trailofbits/c-review

C/C++ Security Review

Resolve four parameters, make one Workflow call, return the report. The workflow owns concurrency, retries and result collection.

Use for: native C/C++ userspace — memory safety, integer overflow, races, type confusion, Linux/macOS daemons, Windows services.

Not for: kernel drivers or modules; managed languages (Java, C#, Python, Go, Rust); embedded or bare-metal code with no libc.

Phase 0 — Parameters

Parse any free text on the invocation line (flamenco only, high severity only, use haiku) and pre-fill what it implies. Then make one AskUserQuestion call for whatever is still unresolved. Never silently default a required parameter.

| Parameter | Values | Inferring it from the invocation | |---|---|---| | threat_model | REMOTE / LOCAL_UNPRIVILEGED / BOTH | "remote", "network", "attacker" → REMOTE; "local", "unprivileged" → LOCAL_UNPRIVILEGED; otherwise ask | | worker_model | haiku / sonnet / opus / inherit | An explicit model name. Otherwise ask. inherit uses the session model | | severity_filter | all / medium / high | "all", "every", "noisy" → all; "medium and above" → medium; "high only" → high; otherwise ask | | scope_subpath | repo-relative directory, optional | "X only", "just audit X/" → the matching subdirectory, fuzzy-matched against top-level dirs. Absent → .. Ambiguous → ask |

Two scopes stay separate for the whole run:

• finding_scope_root = scope_subpath (default .) — a finding must live inside it, and it is the tree the unit list is generated from.
• context_roots = . — read freely to establish callers, build flags and reachability. Narrow it to finding_scope_root only if the user explicitly forbids wider reading, and say that reachability confidence drops when you do.

Phase 1 — Resolve paths

root="${CLAUDE_PLUGIN_ROOT:-}"
if [ -z "$root" ] || [ ! -f "$root/workflows/c-review.js" ]; then
  # Fallback for a cache layout that does not set the variable. ~/.claude ONLY — never `.`:
  # `.` is the AUDITED repository, and a tree that vendors or mirrors this marketplace would
  # win the traversal and run its copy of the scripts, with a different question set and
  # nothing saying which copy ran. Let find's stderr through; a missing ~/.claude is a real
  # failure to report, not noise to hide.
  hit="$(find "$HOME/.claude" -path '*/c-review/workflows/c-review.js' -print -quit)"
  root="${hit%/workflows/c-review.js}"
fi
[ -n "$root" ] && [ -f "$root/workflows/c-review.js" ] && echo "PLUGIN ROOT: $root"

Stop if neither resolves, rather than running with an empty path — and say which path you resolved, so a copy other than the installed plugin is visible before eight agents run against it.

# The workflow cannot call Date.now(), so the timestamp is made here.
output_dir="$(pwd)/.c-review-results/$(date -u +%Y%m%dT%H%M%SZ)"
mkdir -p "$output_dir"; echo "$output_dir"

# A Workflow script has no filesystem APIs, and `assemble_findings.py` resolves `--scope`
# against ITS OWN cwd. Resolve it once here and pass BOTH spellings, or the workflow strips
# `src/` from a finding's path while the assembler strips `/repo/src/`, and the two disagree
# about which findings are duplicates of each other.
scope_abs="$(cd "${scope_subpath:-.}" && pwd)" || echo "scope_subpath does not exist"
echo "$scope_abs"

uv must be on PATH: Detect runs the unit enumerator and Assemble runs assemble_findings.py. If uv is missing, say so and stop — the whole review is partitioned from that unit list.

Phase 2 — Run the workflow

Invoking this skill is the opt-in to multi-agent orchestration — call Workflow without asking again. A review of a real codebase also runs past any default workflow size guideline; that guideline is advisory and this is the case it exempts. Do not shrink the fan-out to fit it, and do not substitute hand-spawned Agent calls.

One Workflow call. scriptPath takes the absolute path resolved in Phase 1; args must be a real JSON object, not a JSON-encoded string.

Workflow({
  scriptPath: "<plugin_root>/workflows/c-review.js",
  args: {
    outputDir:        "<output_dir>",
    pluginRoot:       "<plugin_root>",
    threatModel:      "REMOTE",
    severityFilter:   "all",
    findingScopeRoot: "expat/lib",
    findingScopeRootAbs: "/abs/path/to/repo/expat/lib",
    contextRoots:     ".",
    workerModel:      "sonnet"
  }
})

findingScopeRootAbs is the scope_abs from Phase 1 and is not optional in practice: omitted, the workflow tells the assembler no absolute root is known and a finding filed as /repo/expat/lib/xmlparse.c stops merging with the same bug filed as xmlparse.c.

Six further arguments are optional. Omitted, each takes its default; passed with the wrong TYPE, the workflow throws with the field name rather than defaulting. Pass them only when the user asks or when running an evaluation:

| Argument | Default | What it is for | |---|---|---| | maxUnitLines | 150 | Cap on a review unit; a larger function is split at syntactic seams. Raising it reintroduces the saturation the cap prevents | | linesPerAgent | 1500 | Source lines per review agent. A no-op on a small tree — --agent-min (default 4) floors the derived count, so two very different values can produce identical assignments. Use reviewAgents to pin the fan-out | | reviewAgents | derived | Pins the review fan-out, subject to the same floor as the derived count: both are clamped to 4–14, and an explicit value above 14 raises the cap to itself. A value below 4 is raised to 4, and a trailing slice too small to be worth an agent is folded into its neighbour, so the final count can come out one lower than asked | | invariantAudit | false | Adds the shared-state invariant audit to the sweep. A whole extra agent; turn it on for state-machine-heavy targets | | exclude | [] | Array of globs or substrings the unit enumerator skips (each becomes a repeated --exclude). Use when enumeration aborts naming a path it cannot own — a symlink resolving outside the scope root, an unreadable directory — and the excluded paths land in the enumerator's totals as a visible coverage hole, not silence | | benchmarkMode | false | Eval-only. Adds an external-source declaration to reviewer prompts and two schema fields. Changes no finding; leave it off for a real audit |

The workflow validates its own arguments and throws with a named field if one is missing. It runs five phases:

| Phase | Agents | What it does | |---|---|---| | Detect | 1 | Runs enumerate_units.py for the unit list; platform flags from actual API usage; shared-state structs; per bug class, whether any candidate site exists | | Review | 4–14 | One agent per contiguous slice of the unit list. Each returns findings with severity and a ledger row per (unit × question) | | Sweep | 0–2 | The class axis: one agent over every bug class with no entry anywhere that Detect did not rule out. None, and the phase is skipped. Plus the struct-field audit when invariantAudit: true | | Dedup | 0–1 | Only for collisions the assembler cannot merge deterministically. Usually skipped | | Assemble | 1 | Runs assemble_findings.py: ledger gate, deterministic merges, findings.json, REPORT.md, REPORT.sarif |

Around 8–10 agents on a mid-size target.

Phase 3 — Return the report

Read <output_dir>/REPORT.md and return it.

Say once, plainly, next to the findings: no false-positive review ran. Every severity is the reviewer's own (severity_source: "reviewer"), judgeRan is always false, and nothing rejected anything. Expect some of what you are shown to be wrong or out of scope — say so rather than presenting the list as adjudicated, and do not filter it yourself.

Then surface, prominently and separately from the findings, anything in the workflow result that means the run was partial:

| Field | Meaning and what to do | |---|---| | artifactsWritten: false | REPORT.md and REPORT.sarif are missing; the part files under parts/ are intact. Re-assemble by hand (below) — never reconstruct from the tool result | | artifactsWritten: null | The assemble agent returned nothing, so whether the artifacts exist is unknown — the command may have completed and only its structured answer failed. List the output directory before doing anything else | | gateAccepted: false with artifacts written | The failure that looks like success. Artifacts are complete; the coverage gate could not run or rejected the ledger, so the review is assembled and unverified. Do not re-run the assembler — read ledger-gate.json and report the gap. artifactError always carries the reason | | coverage: null | Coverage is unmeasured, not complete. Never report such a run as fully covered; point at ledger-gate.json | | coverage | Report checksSatisfied / checksRequired, and checksCompleted only as "answered" — never "functions reviewed". checksSatisfied below checksCompleted means the gate threw rows out: a coverage-integrity failure. Name the violations | | groupsFailed, agentFailures, notes | That ground was not covered. Do not let a clean report imply it was | | unrecognisedParts | Whole agents' output in no artifact. null means unchecked, not none | | silentClasses / ruledOutClasses / platformDroppedClasses | "Swept and found nothing" / "nothing looked" / "out of scope by configuration". Report separately — only the middle one means a human should look |

Hand re-assembly, when artifactsWritten is false:

uv run --no-project <plugin_root>/scripts/assemble_findings.py --run-dir <output_dir> \
  --threat-model <MODEL> --severity-filter <FILTER> --no-judge \
  --scope <finding_scope_root> --context-roots <context_roots> \
  --worker-model <worker_model> \
  --expect <part-stem>=<finding-count>   # one per part the workflow log names

• --expect is not optional. Without it the allowlist admits every file under parts/, including one nobody dispatched, and the run exits 1 for that reason alone. If you cannot recover the counts from the workflow log, exit 1 is correct and the run is assembled-but-unverified.
• Never drop --no-judge. Dropping it overwrites every reviewer severity with MEDIUM and records judge_ran: true for a run no judge ever saw.
• Six flags cannot be reconstructed at all (--expect-complete, --benchmark-mode, --groups-attempted, --groups-failed, --agent-failure, --external-source), so the document will say agent_failures: [] on a run that may have lost a slice. Report that. Without a units.json the assembler also exits 1: no gate ran.

Finally list the artifacts: findings.json, REPORT.md, REPORT.sarif, units.json, ledger-gate.json, detect.json, and the parts/ and assignments/ directories.

Rationalizations to Reject

• "The run mostly worked, so I'll just present the report." A failed agent is uncovered ground, not a rounding error. Report it next to the findings.
• "Coverage is 80%, that's basically complete." The missing 20% is a list of exact (unit, question) pairs in ledger-gate.json. Name them.
• "I'll write the findings myself instead of running the workflow." Hand-orchestrating costs far more for worse recall. Always call Workflow.
• "The artifacts failed, so I'll reconstruct the report from the tool result." The part files are on disk and the assembler is deterministic. Re-run it.
• "Zero findings, so there is nothing to report." A zero-finding run still produces both artifacts, and zero findings on real C code is itself worth saying out loud.
• "The workflow returned findings, so I can skip reading REPORT.md." The tool result is capped and carries counts, not findings. The report is the artifact.
• "No judge ran, so I should filter the findings myself." No — silently dropping findings reproduces a judge's cost with none of its rigour and leaves the artifact disagreeing with what you said. Report what the pipeline produced, labelled unadjudicated.
• "No judge ran, so I'll present severities as authoritative." Also no. They are one reviewer's opinion.
• "A class-per-agent fan-out would find more." Location is the partition on purpose; the class catalogue is a bounded completeness sweep on top. Do not add one.

Design rationale, the coverage gate's threat model and its known limitations are in AGENTS.md. Read it before changing a prompt or a gate rule.