| 1 | --- |
| 2 | name: security-review |
| 3 | description: Review a change, module, or surface for exploitable defects — trust boundaries, authn/authz, injection, secret exposure, filesystem and network reach, dependency risk. Use when the user asks for a security review, audit, or vulnerability check of concrete code. Not for general code review, lint, or compliance paperwork. |
| 4 | invocation: model+user |
| 5 | --- |
| 6 | |
| 7 | # Security Review |
| 8 | |
| 9 | Produce findings a reviewer can verify, not a vibes pass. Every finding |
| 10 | names the file, the reachable path that makes it real, and the fix. |
| 11 | |
| 12 | ## Scope the review first |
| 13 | |
| 14 | - **What is under review:** a diff, a module, a plugin bundle, a network |
| 15 | surface. Say the boundary out loud before reading. |
| 16 | - **Trust boundaries:** where untrusted input enters (HTTP handlers, MCP |
| 17 | tool args, file parsers, CLI flags, env vars, rendered content) and where |
| 18 | authority is exercised (fs writes, network egress, process spawn, |
| 19 | credential reads, signing). |
| 20 | - **Prerequisites:** a checked-out tree and the project's own test runner. |
| 21 | Ask for credentials only if a live path genuinely needs them; never read |
| 22 | secrets from the environment or keychain yourself. |
| 23 | |
| 24 | ## Procedure |
| 25 | |
| 26 | 1. **Map entry points and sinks.** `rg` for the handlers, deserializers, |
| 27 | and exec/fs/net calls in scope. Follow data from entry to sink before |
| 28 | judging it. |
| 29 | 2. **Authn/authz.** Every mutating or sensitive handler checks identity and |
| 30 | object-level authorization. Look for checks that exist on one path but |
| 31 | not its sibling, and for checks done on the client only. |
| 32 | 3. **Injection.** Command lines, SQL, template eval, shell expansion, |
| 33 | path joins under user influence, and markup that will render later — |
| 34 | including generated HTML/markdown that carries repo content into a |
| 35 | browser surface. |
| 36 | 4. **Secrets.** `rg` for token/key/secret patterns and `git log -p` the |
| 37 | diff for credentials. Also check what gets *logged* or embedded in |
| 38 | receipts, exports, or error messages. |
| 39 | 5. **Dependencies.** Run the project's audit gate if it exists |
| 40 | (`cargo audit`, `npm audit`, `osv-scanner`) — report versions and CVEs, |
| 41 | not "deps look old". |
| 42 | 6. **Denial and abuse paths.** Unbounded reads/allocations, missing |
| 43 | timeouts on network calls, resource leaks in error paths, retry storms. |
| 44 | 7. **Verify a finding before reporting it.** Trace the real call path or |
| 45 | write a minimal proof. A finding that "looks suspicious" but has no |
| 46 | reachable path is a note, not a finding. |
| 47 | |
| 48 | ## Findings format |
| 49 | |
| 50 | For each: severity (exploitability × impact), file:line, the reachable |
| 51 | path, a one-paragraph explanation, and the fix. Order by severity. Then a |
| 52 | short "checked and clean" list naming what was audited and cleared — the |
| 53 | scope statement means something only if the clear list is honest. |
| 54 | |
| 55 | ## Recovery and limits |
| 56 | |
| 57 | - If you cannot prove reachability, downgrade the claim and say what |
| 58 | evidence is missing. |
| 59 | - Do not claim a formal audit, certification, or absence of |
| 60 | vulnerabilities. This review finds defects; it does not prove none exist. |
| 61 | - Never fix-and-stay-quiet on a security finding in someone else's in-flight |
| 62 | code — report it first. |
| 63 | |
| 64 | ## Completion criteria |
| 65 | |
| 66 | - Every entry point in scope was traced to its sinks. |
| 67 | - Findings carry file:line + reachability + fix; the clear list names what |
| 68 | was actually checked. |
| 69 | - Severity ordering is defensible by exploitability, not by how loudly the |
| 70 | code smells. |
| 71 |