surshar / claude-code

Claude Code is an agentic coding tool that lives in your terminal, understands your codebase, and helps you code faster by executing routine tasks, explaining complex code, and handling git workflows - all through natural language commands.

Built by Anthropic · github.com/anthropics

claude-code / plugins / security-guidance / hooks / review_api.py

Mohamed HegazyMerge pull request #62586 from mhegazy/security-guidance-v2 ccadef7 · on May 26, 2026 History

398 lines · 24.2 KBRaw

1	"""Public review API for the security-guidance agentic commit reviewer.
2
3	This module is the importable surface for callers that want to run the
4	same two-stage agentic security review as the CC plugin (investigate →
5	self-refute) without going through the CC hook protocol. External
6	agentic harnesses can import this directly so their commit reviewer uses
7	the exact prompts, schemas, and filters the plugin uses.
8
9	``security_reminder_hook.py`` imports every symbol below; the hook
10	script's own underscored names are aliases. Keep this file free of CC
11	hook-event coupling (no stdin parsing, no env-var feature gates, no
12	``debug_log``/state-file IO) so non-CC callers can import it without
13	side effects.
14	"""
15	from __future__ import annotations
16
17	import json
18	import os
19	from typing import Any
20
21	import extensibility
22
23	# ---------------------------------------------------------------------------
24	# Diff capping
25	# ---------------------------------------------------------------------------
26
27	DIFF_PER_FILE_BYTES = int(os.environ.get("DIFF_PER_FILE_BYTES", "80000"))
28	DIFF_TOTAL_BYTES = int(os.environ.get("DIFF_TOTAL_BYTES", "400000"))
29
30
31	def cap_diff_for_prompt(
32	files: list[tuple[str, str]],
33	) -> tuple[list[tuple[str, str]], int]:
34	"""Cap per-file and total diff bytes; return (capped_files, bytes_dropped).
35
36	Truncation markers are written inside the content so the reviewer
37	knows the file is incomplete.
38	"""
39	out: list[tuple[str, str]] = []
40	dropped = 0
41	total = 0
42	for fp, content in files:
43	if len(content) > DIFF_PER_FILE_BYTES:
44	dropped += len(content) - DIFF_PER_FILE_BYTES
45	content = (
46	content[:DIFF_PER_FILE_BYTES]
47	+ "\n... [truncated by security-guidance: file exceeds per-file byte cap]"
48	)
49	room = DIFF_TOTAL_BYTES - total
50	if room <= 0:
51	dropped += len(content)
52	out.append(
53	(fp, "[omitted by security-guidance: total diff byte cap reached]")
54	)
55	continue
56	if len(content) > room:
57	dropped += len(content) - room
58	content = (
59	content[:room]
60	+ "\n... [truncated by security-guidance: total diff byte cap reached]"
61	)
62	total += len(content)
63	out.append((fp, content))
64	return out, dropped
65
66
67	# ---------------------------------------------------------------------------
68	# Stage 1 — investigate
69	# ---------------------------------------------------------------------------
70
71	AGENTIC_INVESTIGATE_SYSTEM = """You are a senior application-security engineer performing a deep security review of a code change. You have read-only filesystem tools (Read, Grep, Glob) scoped to the repository — USE THEM AGGRESSIVELY. The diff alone is not enough.
72
73	The #1 cause of missed vulnerabilities is not reading the file that contains them. Before any analysis: Read EVERY changed file in full (not just the diff hunks). Then Grep for the changed function/class names to find callers. A vulnerability that requires cross-file context is still your responsibility.
74
75	METHOD:
76
77	Phase 1 — Map entry points and sinks touched by this change.
78	Entry points: HTTP handlers/routes, RPC methods, CLI args, webhook receivers, message consumers, file/upload handlers, OAuth callbacks, GitHub Actions inputs, MCP tools, hook handlers, IPC receivers (main/privileged process handling messages from a sandboxed/renderer/less-privileged process).
79	Sinks: shell/exec/subprocess, SQL/ORM raw, eval/new Function, filesystem paths (open/read/write/unlink), outbound HTTP (SSRF), HTML render/innerHTML, deserialization (pickle/yaml/json with object_hook), template engines, subprocess env, IAM/RBAC bindings, dynamic code/plugin/extension loaders (any API that loads+executes code from a path), log/telemetry/metrics dimensions (only when value matches a PII shape — email, token, free-text field; NOT a static enum/type name), cache-control / Vary headers (cache poisoning), DDL that drops a constraint/FK/trigger (referential-integrity), response bodies/headers, prompts sent to LLMs.
80	For each changed file, Grep for the function/class names in the diff to find their callers and what data reaches them.
81
82	Phase 2 — Trace data flow.
83	For every value that reaches a sink, determine whether it is attacker-influenceable. Read upstream: where does the variable come from? Is there validation/sanitization between source and sink? Check sibling handlers in the same file — if they enforce a check this one omits, the omission IS the finding. Cross-component flows (input enters in module A, dangerous operation in module B) are where the high-value findings live; follow them.
84	FOLLOW RETURNS: when a changed function builds a tainted value (command string, SQL, URL, path, template) and RETURNS it rather than executing locally, the sink is in a CALLER — Grep for the function name and read the call sites before deciding it's safe.
85	SIBLING-PATH GATE PARITY: when + lines add a guard/check/tenant-scope/visibility-filter/invalidation/cleanup to ONE branch, ONE handler, or ONE layer, enumerate ALL sibling branches, early-returns, error/except paths, and peer handlers in the same router/service that touch the same resource — report any that lack an equivalent gate. ONLY emit when (a) both the guarded path AND the sibling reach a state-changing or boundary-crossing sink, AND (b) the sibling's input is controllable by a different principal than the guard checks for. Skip if the file has a "generated / DO NOT EDIT" header or lives under generated/openapi/autogen.
86
87	Phase 2b — Parser/validator differentials (a top miss category).
88	When the change adds or modifies parsing, validation, normalization, or matching logic (regexes, URL/path parsers, allowlists, content-type checks, decoders, AST/shell parsers), ask: does an input exist that the validator ACCEPTS but the downstream consumer interprets differently? Look for: unanchored/partial regexes; case/encoding/unicode normalization mismatches; URL parsers that disagree on userinfo/host/path; allowlists checked with substring/startswith; decoders that accept malformed input; quoting/escaping the parser strips but the consumer doesn't. The finding is the differential itself — name both sides.
89
90	Phase 2c — High-miss patterns. Check ONLY against + lines in the diff — do NOT flag pre-existing code you read while exploring.
91	- SENSITIVE-TO-OBSERVABILITY: a + line emits to a log/trace/span/metric/exception-message sink. Trace EVERY field (including URLs, paths, error-object .message, f-string vars, **kwargs) to its source and flag credentials, PII, customer content, or model free-text reaching the sink — especially on error/except branches where happy-path redaction is bypassed and external-service error messages can echo URL-embedded secrets. Skip if: a sanitizer wraps the value at the call site; the log is gated by a debug/dev env flag; or the value is static request metadata (method/path/host).
92	- IaC OMITTED ARG: a + line instantiates a Terraform/Pulumi/CDK module and OMITS an optional security-relevant arg — read the module's variables and check whether the default is the secure value.
93	- CI/CD TRUST: + lines add or change a GitHub Actions trigger to workflow_dispatch / repository_dispatch / pull_request_target without a branches: filter, AND the job reads secrets or has write permissions.
94	- ALLOWLIST SEMANTIC ESCAPE: + lines add an entry to a safe-command/safe-endpoint/capability allowlist OR add a `\|\|` disjunct to a permission matcher OR edit a validator that gates exec/eval/subprocess. Verify no allowed entry achieves a denied effect via its arguments, flags, abbreviations, side-channels (DNS, config-write, env), or scope mismatch vs. enforcement (e.g., allowlist matches argv[0] but consumer reads full argv).
95	- OVER-BROAD GRANT: when + lines add a principal/identity to a broad-scope permission (global/service-wide allowlist, standing admin role binding, reuse of another principal's credential), check whether the SAME changed file or its immediate module already exposes a narrower-scope mechanism for the same need (per-resource/per-RPC allowlist, break-glass/2PC role, dedicated principal). If it does, the broad grant is the finding. Do NOT flag if no narrower mechanism is visible in the changed files.
96	- STALE IDENTITY MAPPING: + lines change teardown/unregister of an identity primitive (hostname/DNS, IP, service route, lease, auth token, service-registry entry) where a window leaves it resolvable to the wrong tenant. NOT in-process data caches.
97	- CONTROL REGRESSION: when - lines DELETE a fail-closed validator (allowlist returning False by default, _is_safe_*, deny-by-default) and + lines replace it with a single condition, the replacement IS the finding.
98	- FAIL-OPEN STATE DRIFT: when a security decision reads parsed/cached/tracked/callback state, verify error, cancellation, TOCTOU, cache-skew, and unhandled-variant paths do not yield a default that skips enforcement — broad-except→pass, unwrap_or({}), missing-finally cleanup, ignored verifier params, or stale validator maps all fail open. The finding is the path where the fallback value is the allow outcome. Also: when + lines compare against a security threshold, check whether the EXACT boundary value yields the permissive branch; when an error path triggers retry/redelivery, check whether the retry can emit a decision that overrides a stricter first decision; when sync logic reads persisted state, check whether state surviving a data wipe causes destructive sync.
99	- SECURITY-REGISTRY FANOUT: when + lines add a new entity (field, enum value, credential type, alias, model variant, port, scope), Grep unchanged files for every security registry keyed on that entity class — sanitizer field-lists, redaction sets, revocation handlers, strip denylists, capability allowlists, translation maps — and flag if the new entry is missing from any. Conversely, when + lines ADD entries to such a registry, Grep for where that registry is consumed and verify each new entry's literal matches the consumer's key format (namespace prefix, case, composite key) — a mismatched entry is a silent no-op that defeats the control.
100	- GATE/ACTION FIELD MISMATCH: when + lines add or modify an authorization/policy check, identify which request field(s) the gate reads vs which field(s) the downstream operation uses to select the target resource. If they differ (gate checks `parent`, action derives target from `name`; gate checks org A, action writes to org from a separate param), the gate is bypassable.
101	- RESOURCE-BOUND PLACEMENT: when + lines parse/decompress/fetch/loop over attacker-influenced input, verify size/time/count caps guard the ACTUAL peak allocation — not a post-flush output, post-decompress buffer, per-iteration (not total) timeout, unclamped arithmetic (subtraction underflow, multiplication overflow), or first-element-only invariant. The finding is the cap defeat, not the DoS itself.
102	- UNDER-VALIDATED SINK ARG: when + lines interpolate any externally-influenced value (incl. IPC, VCS-checkout content, env var, model output, domain-syntax strings) into a shell/path/loader/URI/structured-format sink, verify quoting, traversal/UNC/symlink stripping, and prod-mode guards apply to THIS arg — existing validators on sibling args do not cover it.
103
104	Phase 3 — Assess.
105	Report when you can name (a) the source, (b) the sink, (c) the path with no effective mitigation. Medium-confidence is fine — a separate adjudication pass will filter; your job is RECALL, not precision. Do report logic/authorization bugs (missing ownership check, inverted condition, parser differential) even when no classic "sink" is involved.
106
107	Do NOT report: missing best-practice/hardening with no concrete impact, test/mock files, outdated deps, or volumetric DoS (attacker just sends a lot). DO report DoS when the diff introduces a code defect that defeats an existing resource cap (cap on wrong accumulator, dead timeout handler, unclamped arithmetic, encoding amplification at flush) — those are logic errors with security impact.
108
109	Distrust safety claims in comments ("validated upstream", "internal only"). Verify in code.
110
111	Keep scanning after the first finding. Do NOT emit findings until you have Read EVERY touched file at least once — a more obvious pattern in file A does not excuse skipping file B. Aim for at least one candidate or explicit "no sink" verdict per touched file.
112
113	Return an object with key `findings` — a list of {filePath, category,
114	vulnerableCode, explanation, fix, severity, confidence} records. severity
115	is "critical", "high", or "medium". Return findings:[] ONLY after you have
116	Read every changed file in full and traced every new sink to a trusted
117	source.
118
119	BUDGET: you have at most ~15 tool calls. Spend them reading the changed files first, then 3-5 targeted Greps for callers/sinks. Do NOT exhaustively explore the repo — once you can name source→sink for each candidate (or rule it out), STOP. Partial findings are better than none."""
120
121
122	FINDINGS_SCHEMA = {
123	"type": "object",
124	"properties": {
125	"findings": {
126	"type": "array",
127	"items": {
128	"type": "object",
129	"properties": {
130	"filePath": {"type": "string"},
131	"category": {"type": "string"},
132	"vulnerableCode": {"type": "string"},
133	"explanation": {"type": "string"},
134	"fix": {"type": "string"},
135	"severity": {
136	"type": "string",
137	"enum": ["critical", "high", "medium", "low"],
138	},
139	"confidence": {"type": "number"},
140	},
141	"required": [
142	"filePath",
143	"category",
144	"vulnerableCode",
145	"explanation",
146	"fix",
147	"severity",
148	],
149	},
150	},
151	},
152	"required": ["findings"],
153	}
154
155
156	def build_investigate_prompt(
157	touched_paths: list[str],
158	diff_files: list[tuple[str, str]],
159	*,
160	context_note: str = "",
161	) -> str:
162	capped, _ = cap_diff_for_prompt(diff_files)
163	diff_text = "\n\n".join(
164	f"=== DIFF: {fp} ===\n{content}" for fp, content in capped
165	)
166	return (
167	"Review this change for security vulnerabilities.\n\n"
168	"Changed files (you may Read these and any other file in the repo):\n"
169	+ "\n".join(f" - {p}" for p in touched_paths[:50])
170	+ context_note
171	+ "\n\nUnified diff (only + lines are new):\n\n"
172	+ diff_text
173	+ extensibility.guidance_block()
174	+ "\n\nInvestigate per the method in your instructions, then return "
175	"the findings list."
176	)
177
178
179	# ---------------------------------------------------------------------------
180	# Stage 2 — self-refute
181	# ---------------------------------------------------------------------------
182
183	AGENTIC_REFUTE_SYSTEM = (
184	"You adversarially verify security findings. You have "
185	"Read/Grep over the repo. Default = SURVIVES unless you "
186	"find concrete refuting evidence."
187	)
188
189
190	SURVIVED_SCHEMA = {
191	"type": "object",
192	"properties": {
193	"survived": {"type": "array", "items": {"type": "integer"}},
194	"refuted": {
195	"type": "array",
196	"items": {
197	"type": "object",
198	"properties": {
199	"idx": {"type": "integer"},
200	"reason": {"type": "string"},
201	},
202	"required": ["idx", "reason"],
203	},
204	},
205	},
206	"required": ["survived"],
207	}
208
209
210	def build_refute_prompt(candidates: list[dict[str, Any]], diff_text: str) -> str:
211	return (
212	"You previously flagged these candidate vulnerabilities:\n\n"
213	+ json.dumps(candidates, indent=2)
214	+ "\n\nDIFF:\n" + diff_text[:8000]
215	+ "\n\nNow adversarially try to DISPROVE each one. For each "
216	"candidate, FIRST identify the attacker (who controls the "
217	"input) and the victim (who is harmed). REFUTE if the only "
218	"victim is the attacker themselves on their own machine. KEEP "
219	"if the attacker is a legitimate user/tenant but the impact "
220	"reaches other users/tenants, shared infra, or server-side "
221	"resources.\n\n"
222	"DIFF-ANCHOR: candidates are sorted `in_diff` first, then "
223	"`off_diff`. Process them in order. `in_diff` candidates "
224	"use the standard KEEP/REFUTE bar above. `off_diff` "
225	"candidates require STRICTER evidence: you must identify "
226	"the specific +/- line in the diff that ENABLES the "
227	"off-diff sink (a removed guard, a new caller, a changed "
228	"argument feeding it). If you cannot name that enabling "
229	"diff line, REFUTE the off_diff candidate. Additionally, "
230	"REFUTE any off_diff candidate whose sink is already "
231	"covered by a surviving in_diff candidate.\n\n"
232	"Then Read the cited file and refute with cited file:line "
233	"evidence if ANY of these holds:\n"
234	"- PRE-EXISTING: the cited vulnerableCode does NOT appear on "
235	"any + line in the DIFF block above — it is unchanged context "
236	"in a touched file. The diff did not introduce it.\n"
237	"- A sanitizer/validator/authz check prevents the described "
238	"exploit.\n"
239	"- The sink is non-dangerous: typed-schema decoder (msgspec/"
240	"pydantic, not pickle/yaml), hardcoded https://<host>/ URL "
241	"with non-:path params, autogen client stub, value is "
242	"statically number/boolean.\n"
243	"- NO PRIVILEGE BOUNDARY: attacker == victim. The input "
244	"comes from env var / CLI arg / $HOME dotfile / HKCU / "
245	"~/Library prefs / OS-user config — and the process runs at "
246	"the same privilege as whoever writes that source. Also: "
247	"the 'allow' decision is advisory self-gating returned to "
248	"the same caller; or the prefix/suffix check is a secondary "
249	"filter behind a parent-domain pin.\n"
250	" NEVER apply NO-PRIVILEGE-BOUNDARY to: SSRF/outbound-"
251	"network sinks; LLM-agent capability gates (PreToolUse/"
252	"PostToolUse hooks, bash allow/denylists, workspace path "
253	"jails — the model is the attacker, the user is the "
254	"victim); data-exposure findings (CWE-200/359/532, secrets-"
255	"in-logs — the question is who READS the sink, not who "
256	"controls the input); project-working-directory config "
257	"(.claude/settings, .vscode/, package.json scripts — repo "
258	"author ≠ repo cloner); cross-process metadata sources "
259	"(psutil.Process(...), /proc/<pid>/* — different process "
260	"owner is a different principal).\n"
261	"- TRUSTED-HEADER NAMESPACE: the flagged header is from a "
262	"namespace the same handler already trusts for actor "
263	"identity/authz (e.g. control-plane-injected X-Amzn-*).\n"
264	"- FRONTEND-ONLY GATE: the loosened check is in frontend "
265	"code AND the backend handler independently enforces it.\n"
266	"- DELEGATED VALIDATION: the unvalidated credential is "
267	"immediately forwarded to an upstream that validates.\n"
268	"- THROWAWAY-CODE: all touched files live under scripts/, "
269	"dev/, tools/, examples/, testdata/, fixtures/, or behind "
270	"a __main__ dev guard.\n"
271	"- CONTROL MOVED TO LIBRARY: the diff removes a security "
272	"control AND bumps a dependency that documents providing "
273	"that control — the control was delegated, not removed.\n"
274	"- Config/feature-flag gates the path with no per-request "
275	"user control over the gate value.\n"
276	"- Protective-control polarity: the change loosens a guard "
277	"around a PROTECTIVE control (prompt/audit/confirm).\n"
278	"Do NOT speculate — refute only with cited evidence. Default "
279	"= SURVIVES.\n\n"
280	"Return `survived` — the indices of candidates you could NOT "
281	"refute — and `refuted` — {idx, reason} records for each you "
282	"did. An empty `survived` means every candidate was refuted."
283	)
284
285
286	# ---------------------------------------------------------------------------
287	# Mechanical filters and rendering
288	# ---------------------------------------------------------------------------
289
290
291	def tag_diff_anchor(
292	candidates: list[dict[str, Any]], diff_text: str
293	) -> list[dict[str, Any]]:
294	"""SOFT diff-intersect: tag each candidate ``_diff_anchor: "in_diff" \|
295	"off_diff"`` and sort in_diff first; do NOT drop.
296
297	Investigate reads full files and often cites pre-existing patterns in
298	unchanged context (the largest false-positive source). Hard-dropping
299	those also discards correct findings whose sink is off-diff but
300	enabled by an in-diff change. The refute pass's DIFF-ANCHOR block
301	keys on the ``_diff_anchor`` tag to apply stricter evidence to
302	off_diff candidates instead of dropping them.
303
304	Mutates ``candidates`` in place; returns it for chaining.
305	"""
306	added = [
307	ln[1:]
308	for ln in diff_text.splitlines()
309	if ln.startswith("+") and not ln.startswith("+++")
310	]
311	removed = [
312	ln[1:]
313	for ln in diff_text.splitlines()
314	if ln.startswith("-") and not ln.startswith("---")
315	]
316
317	def _norm(s: str) -> str:
318	return " ".join(t for t in " ".join(s.split()).split() if len(t) > 2)
319
320	added_norm = _norm("\n".join(added))
321	removed_norm = _norm("\n".join(removed))
322
323	def _intersects(cand: dict[str, Any]) -> bool:
324	vc = _norm(" ".join(str(cand.get("vulnerableCode") or "").split()))
325	if len(vc) < 8:
326	return True
327	toks = vc.split()
328	for i in range(max(1, len(toks) - 2)):
329	if " ".join(toks[i : i + 3]) in added_norm:
330	return True
331	for ln in added:
332	ln_n = _norm(ln)
333	if len(ln_n) >= 8 and ln_n in vc:
334	return True
335	if len(added) < len(removed):
336	for i in range(max(1, len(toks) - 2)):
337	if " ".join(toks[i : i + 3]) in removed_norm:
338	return True
339	return False
340
341	for c in candidates:
342	c["_diff_anchor"] = "in_diff" if _intersects(c) else "off_diff"
343	candidates.sort(key=lambda c: c.get("_diff_anchor") != "in_diff")
344	return candidates
345
346
347	_SEVERITY_ORDER = {"critical": 0, "high": 1, "medium": 2, "low": 3}
348
349
350	def filter_by_severity(
351	findings: list[dict[str, Any]], *, include_medium: bool = True
352	) -> list[dict[str, Any]]:
353	"""Medium-included is the validated default; the model's investigate-stage
354	severity is conservative and dropping mediums before self-refute filters
355	out most real findings.
356	Pass ``include_medium=False`` for the old high/critical-only behavior.
357	"""
358	keep = ("critical", "high", "medium") if include_medium else ("critical", "high")
359	out = [
360	v
361	for v in findings
362	if str(v.get("severity", "medium")).strip().lower() in keep
363	]
364	out.sort(key=lambda v: _SEVERITY_ORDER.get(v.get("severity", "medium"), 2))
365	return out
366
367
368	def format_findings(findings: list[dict[str, Any]]) -> str:
369	"""Render findings as the same text block the CC plugin emits to Claude."""
370	by_file: dict[str, list[dict[str, Any]]] = {}
371	for v in findings:
372	by_file.setdefault(v.get("filePath", "unknown"), []).append(v)
373	lines = [
374	"Security Review: Potential vulnerabilities detected",
375	"",
376	f"Affected files: {', '.join(by_file)}",
377	"The following issues were flagged by automated security review. "
378	"Address each, or briefly note why it doesn't apply. Valid reasons "
379	"to proceed without changes: the user explicitly asked for this and "
380	"you've already surfaced the security tradeoffs, or the pattern "
381	"isn't actually exploitable in this context. Do not dismiss "
382	"findings solely because the service is internal-only — internal "
383	"services are common SSRF/IDOR targets:",
384	"",
385	]
386	n = 1
387	for fp, vs in by_file.items():
388	lines.append(f" {fp}:")
389	for v in vs:
390	sev = (v.get("severity") or "medium").upper()
391	lines.append(
392	f" {n}. [{sev}] [{v.get('category', 'Unknown')}] "
393	f"{v.get('vulnerableCode', 'N/A')}"
394	)
395	lines.append(f" Suggested fix: {v.get('fix', 'N/A')}")
396	lines.append("")
397	n += 1
398	return "\n".join(lines)