review-code-security

name	review-code-security
description	Scan implemented code for concrete security anti-patterns. Companion to /review-plan-security.
model	opus
effort	high

Scan the specified code for security anti-patterns using the same three axes as /review-plan-security. Use after implementation to verify the plan's security goals hold in the actual code.

When to Use

Use when the implementation touches external input, secrets handling, or third-party integrations. Pass a file path, diff, or describe the code to review.

Rules

• Report each finding with: file/location, pattern category, and recommended fix.
• Note context for potential false positives (test fixtures, comments, examples).
• For commit-time auto-scanning of hard secrets (AWS/LLM API keys, PEM keys, GitHub/Slack tokens, .env commits), see docs/scan-outbound.md — those are covered automatically.
• Sibling sweep: enumerate functions, logic, or patterns the change touches that belong to a class; flag siblings as MUST / OPTIONAL / NA for same treatment.

Procedure

RCS-1. Open the concern round (Bash): bash "$AGENTS_CONFIG_DIR/skills/review-code-security/scripts/open-concern-round.sh" — prints ROUND, PLANS_DIR, SESSION_ID, and the [PRIOR CONCERNS START]…[PRIOR CONCERNS END] block both producers receive. ROUND=0 means the ledger is unavailable: run RCS-2 and RCS-3, report the NOT-STAGED line, skip the ledger close-out in ## Completion.

RCS-2. Delegate scan to security-scanner, issued together with the RCS-3 quality gates per skills/_shared/subagent-concurrency.md SC-P (independent — both are read-only over the merge-base diff and write no shared target):

Agent({ subagent_type: "security-scanner", prompt: JSON.stringify({
  topic: "security review", context: SCAN_TARGET,
  artifact_dir: PLANS_DIR, prior_concerns: PRIOR_BLOCK
}) })

Pass the RCS-1 block verbatim as prior_concerns; omit the key when RCS-1 printed none. On failed status: surface summary + artifact_path to user. Output: ## Security Review: PERFORMED|FAILED (1 line) + artifact_path pointer. Read report only on failure or explicit user request.

RCS-3. Quality gates (Bash, issued with RCS-2 per SC-P): CONCERN_LEDGER_ROUND=<ROUND> bash "$AGENTS_CONFIG_DIR/skills/review-code-security/scripts/run-quality-gates.sh" — resolves merge-base and runs the ledger-wrapped codex reviewer plus 7 lint gates; the wrapper stages the reviewer's own delta. Each gate is advisory; non-zero exit is a warning, not a blocker. When the output carries any ## <gate>: NOT FOUND line, append (N gates NOT FOUND) to the ## Security Review: line so the reader sees the sweep was incomplete.

Patterns by Axis

Axis 1: Information Leakage

Axis scope: OWASP ASVS V8 (Data Protection), V6 (Stored Cryptography)

Automated coverage: AWS/LLM API keys, PEM private keys, GitHub/Slack tokens, and .env file commits are auto-detected by scan-outbound.sh at pre-commit. Source integrity is also covered: zero-width chars (U+200B/C/D, U+FEFF) and Bidi override chars (U+202D/E, U+2066–2069) are detected as [zero-width] / [bidi-override] (Trojan Source, CVE-2021-42574). This axis focuses on context-dependent leaks automation cannot catch.

Pattern	What to look for	Risk
Generic secret hardcoded	(?i)(secret|password|token)\s*=\s*["'][^"']{8,}["'] in source	Leaked credential
Logging sensitive data	log.*password, print.*token, stack traces with secrets	Exposure via logs
.env not gitignored	.env absent from .gitignore	Secret leak on push
Temp files with secrets	Scripts writing secrets to /tmp/ or similar	Filesystem exposure

Axis 2: Third-Party Access

Axis scope: OWASP MCP Top 10 (MCP03 Excessive Permissions, MCP04 Tool Poisoning), LLM Top 10 (LLM03 Supply Chain)

Pattern	What to look for	Risk
Unpinned dependency	"latest" in package.json, bare package name in requirements.txt	Supply chain (LLM03)
Unvalidated LLM/MCP output	Agent output used directly in eval, shell call, or DB query	Prompt injection → RCE
Excessive MCP permissions	Tool requesting file system / network beyond task scope	MCP03
Tool Poisoning	MCP tool descriptions containing instruction overrides (ignore previous, system commands)	MCP04
Rug Pull	MCP server behavior changes after approval — use trusted, auditable publishers only	MCP09
Return Value Injection	Tool return value fed directly into a prompt or eval without validation	MCP05 / LLM01

Axis 3: External Access

Axis scope: OWASP WSTG (Input Validation), CWE Top 25 #2 (CWE-79 XSS), #3 (CWE-89 SQL Injection)

Pattern	Regex hint	CWE
Shell injection	eval.*\$, unquoted $VAR in command position	CWE-78
Path traversal	../ combined with user-controlled variable	CWE-22
SQL injection	String concatenation in SQL ("SELECT " + var)	CWE-89
Open redirect	Redirect to URL from user input without allowlist	CWE-601
XSS	Unsanitized user input rendered as HTML	CWE-79
Instruction override in input	Untrusted input containing ignore previous, you are now, system: forwarded to LLM as context	LLM01
Base64 obfuscation	Base64 string from untrusted input decoded and passed to LLM/shell	LLM01

Completion

After reporting findings, once both RCS-2 and RCS-3 finish, close the ledger round via skills/review-code-security/scripts/close-concern-round.sh. Schema and severity vocabulary: skills/_shared/concern-ledger.md.

1. Run: bash "$AGENTS_CONFIG_DIR/skills/review-code-security/scripts/close-concern-round.sh" <ROUND> <PLANS_DIR> <SESSION_ID> security-scanner <COMPLETE|PARTIAL|ABSENT> <artifact_path> — stages the scanner's delta (mapping its status: onto the exec label), reduces, finalizes, and verifies via check-finalized, retrying finalize once on a transient failure. Append (N unresolved concerns), derived from its UNRESOLVED= line, to the ## Security Review: line.
2. CHECK=ok → run (as a standalone Bash command — no pipes, no && chaining): echo "<<WORKFLOW_MARK_STEP_review_security_complete>>"
3. CHECK=FINALIZE-FAILED → do not emit the completion sentinel; report the FINALIZE-FAILED reason and the recovered-copy path instead.

Relationship to Other Tools

• /review-plan-security — architecture-level checklist (run before implementation begins)
• scan-outbound.sh — auto-detects hard secrets and private info at pre-commit (see docs/scan-outbound.md)