rust-review

---
name: rust-review
description: Performs comprehensive Rust security review for safe/unsafe boundary issues, memory safety in unsafe blocks, concurrency hazards, panic-induced DoS, FFI safety, and async runtime mistakes. Use when auditing Rust crates, services, or libraries — particularly those with `unsafe`, FFI, or concurrent code.
allowed-tools: Agent AskUserQuestion SendMessage TaskCreate TaskUpdate TaskList Read Write Bash
---

# Rust Security Review

Runs in the main conversation (invoke via `/rust-review:rust-review`). Orchestrator owns the `Task*` ledger as bookkeeping for retries; workers and judges have no Task tools. Workers and judges are named plugin subagents (`rust-review:rust-review-worker`, `rust-review:rust-review-dedup-judge`, `rust-review:rust-review-fp-judge`); tool sets are declared in `plugins/rust-review/agents/*.md`. Findings are exchanged via markdown-with-YAML files in a shared output directory.

## When to Use

Rust application/library security review: safe/unsafe boundary auditing, memory safety in `unsafe` blocks, concurrency hazards, panic-induced DoS on servers, FFI safety, async-runtime mistakes.

## When NOT to Use

- Pure-C / pure-C++ codebases — use `c-review` instead.
- Smart contracts (Solana programs / NEAR contracts / Ink!) — use `solana-vulnerability-scanner` or the contract-specific skill.
- Kernel-mode Rust drivers without userspace allocator — coverage is incomplete; flag as advisory only.
- Secrets/key memory hygiene (zeroization, `Zeroize`/`ZeroizeOnDrop`/`secrecy` usage, lingering stack/heap copies) — use the `zeroize-audit` skill; rust-review does not cover memory zeroization.

## Subagents

| Subagent type | Purpose | Tool set |
|---|---|---|
| `rust-review:rust-review-worker` | Run assigned cluster, write findings | Read, Write, Edit, Bash |
| `rust-review:rust-review-dedup-judge` | Merge duplicates (runs **first**) | Read, Write, Edit, Glob |
| `rust-review:rust-review-fp-judge` | FP + severity + final reports (runs **second**) | Read, Write, Edit, Bash |

Tools come from each agent's frontmatter at spawn time. The orchestrator's `Task*`/`Agent`/`Bash`/etc. come from this skill's `allowed-tools`. **Search-tool / `Bash` interaction:** in current Claude Code, an agent granted `Bash` is **not** also granted the dedicated `Glob` **or `Grep`** tools (the calls return `No such tool available`; the harness expects `find`/`grep`/`rg` via `Bash` instead). So only the dedup-judge — the one agent that holds **no** `Bash` — uses `Glob`; the worker, fp-judge, and the orchestrator resolve and search paths with `Read` / `Bash` `find` / `rg` / `grep` / `test -f` instead. Because the cluster/finder prompt seeds are written in ripgrep regex syntax (`\s`, `\d`, `\b`), `Bash`-holding agents must run them with **`rg`**. If `rg` is not installed its call fails *loudly* (`command not found`) — fall back to `grep -E` with POSIX classes (`\s`→`[[:space:]]`, `\d`→`[[:digit:]]`, drop `\b`), never a raw-`\s` `grep` whose *silent* empty becomes a bad `cleared`. Do **not** reintroduce `Glob`/`Grep` into a `Bash`-holding agent's protocol.

---

## Architecture

```
coordinator: write context.md → build_run_plan.py → TaskCreate × M
          → spawn primer (foreground) → spawn M workers (parallel)
          → classify Phase-7 outcomes + write findings-index.txt
          → dedup-judge → fp-judge → report safety net (SARIF + REPORT.md) → return REPORT.md
```

Output directory contains: `context.md`, `plan.json`, `worker-prompts/`, `findings/`, `findings-index.d/` (per-worker shards), `findings-index.txt`, `coverage/` (per-worker coverage-gate files), `run-summary.md`, `dedup-summary.md`, `fp-summary.md`, `REPORT.md`, `REPORT.sarif`.

**Path convention:** every later phase shells out to `${RUST_REVIEW_PLUGIN_ROOT}/scripts/*.py`, so resolve that variable first to the plugin directory that contains `prompts/clusters/unsafe-boundary.md` (and `scripts/build_run_plan.py`). Try in order, first hit wins:

1. **Native Claude Code** — `${CLAUDE_PLUGIN_ROOT}`, accepted if `Bash: ls "${CLAUDE_PLUGIN_ROOT}/prompts/clusters/unsafe-boundary.md"` resolves.
2. **Codex** — `${CODEX_PLUGIN_ROOT}` (set it the same way if that var is present and resolves the marker).
3. **Fallback search** — covers Codex installs under `~/.codex`, Claude installs under `~/.claude`, and a local checkout / repo run: `Bash: find ~/.claude ~/.codex . -path '*/plugins/rust-review/prompts/clusters/unsafe-boundary.md' -print -quit 2>/dev/null`. Take the match and strip the trailing `/prompts/clusters/unsafe-boundary.md` to get the root (the home dirs are searched before `.` so an installed copy wins over any vendored copy in the audited repo).

Set `RUST_REVIEW_PLUGIN_ROOT` to the resolved root. If all three fail, **abort** with a message naming the roots searched — do not enter Phase 4 with an empty variable (every `uv run --no-project "${RUST_REVIEW_PLUGIN_ROOT}/scripts/..."` call would fail with a confusing path error).

**Scope convention:** keep two scopes separate throughout the run:

- `finding_scope_root` — the user-requested audit subtree. Workers may only file findings whose vulnerable location is inside this subtree.
- `context_roots` — read-only repo roots/files workers and judges may inspect to verify reachability, callers, wrappers, build flags, mitigations, and threat-model details. Default to `.` unless the user explicitly forbids broader context. Reading context outside `finding_scope_root` is allowed; filing findings there is not.

---

## Rationalizations to Reject

- **"`unsafe` is rare, so hand-skip the memory-safety cluster."** Don't edit the cluster list — set `has_unsafe` accurately in Phase 1 and let `build_run_plan.py` decide. Every memory-safety bug class (UAF, double-free, uninitialized reads, `Vec::set_len`, union UB) requires `unsafe`, so the planner runs the **whole** `memory-safety` cluster when `has_unsafe=true` and correctly omits it when `false` — there is no "run it anyway." The **unsafe-boundary** cluster is different: it has no `requires` and always runs (consolidated; its safety-doc and `repr(C)` hygiene apply to FFI declarations even without visible `unsafe { }` blocks).
- **"The compiler caught it."** The borrow checker proves absence of safe-code data races; it proves nothing about unsafe blocks, panic reachability, ABBA deadlocks, atomic-load/store sequencing, or FFI ABI mismatch.
- **"`unwrap()` is fine if it's `// SAFETY: documented infallible`."** `// SAFETY:` documents `unsafe` operations, not infallibility claims. An `unwrap()` on documented-infallible input is still risky if the documentation is wrong — file as low severity and let the FP judge decide.
- **"`has_unsafe=false` so skip the run."** Pure safe-Rust crates still have panic-DoS, atomic races, drop-panics, and trait-implementation hazards. Run the always-on clusters.
- **"Background spawns parallelize the workers."** They do not — `Agent` calls in a single assistant message already run concurrently. `run_in_background=true` defeats the Phase 6a primer cache, so every worker pays full cache-creation (`cache_read_input_tokens=0`) and the ~15 K-token primer is wasted M times. Default: omit `run_in_background` from worker spawns.
- **"I'll re-derive the cluster list / paths / pass prefixes inline instead of running `build_run_plan.py`."** The script is the only authority for selection and rendering. Paraphrasing it drops fields that the worker self-check requires, producing `worker-N abort: spawn prompt malformed`. Always run the script and `Read plan.json`.
- **"The run partially succeeded — I'll just write `REPORT.md` from what completed."** Hiding partial runs behind a successful report is a correctness bug. If any Phase-5 cluster task is not `completed`, surface it prominently in `run-summary.md` and the final response.
- **"Zero findings — skip Phase 8."** Always run both judges and Phase 8b: dedup-judge writes a minimal no-op `dedup-summary.md` on an empty index, fp-judge writes empty `REPORT.md`/`REPORT.sarif`, and Phase 8b's SARIF generator emits `results: []` for the empty case. SARIF consumers depend on a stable artifact set.
- **"`Bash: ls README*` is fine for the preflight."** Under zsh, an unmatched glob aborts the whole compound command before `2>/dev/null` runs. Use `find` (never fails on no-match) — and not `Glob`, which is unavailable to an agent that also holds `Bash`.

---

## Orchestration Workflow

Run these phases **in the main conversation**.

### Phase 0: Parameter Collection

**Entry:** skill invoked. **Exit:** `threat_model`, `worker_model`, `severity_filter` resolved; `scope_subpath` resolved or set to `"."`; `finding_scope_root=scope_subpath`; `context_roots` resolved.

The skill is invoked directly (no command wrapper). Parse any free-text arguments the user passed on the `/rust-review:rust-review` line (e.g. `flamenco only`, `high severity only`, `use haiku`) and pre-fill the answers they imply — then ask for any missing required parameters with **one** `AskUserQuestion` call. Never silently default the required parameters.

Required parameters:

| Parameter | Values | How to infer from args |
|---|---|---|
| `threat_model` | `REMOTE` / `LOCAL_UNPRIVILEGED` / `BOTH` | Words like "remote", "network", "attacker" → `REMOTE`; "local", "unprivileged" → `LOCAL_UNPRIVILEGED`; otherwise ask. |
| `worker_model` | `haiku` / `sonnet` / `opus` | Explicit model name in args. Otherwise ask (no silent default). |
| `severity_filter` | `all` / `medium` / `high` | "all", "every", "noisy" → `all`; "medium and above" → `medium`; "high only", "criticals only" → `high`. Otherwise ask — **no silent default**. |
| `scope_subpath` | repo-relative directory (optional) | Phrases like "X only", "just audit X/", "review subdirectory X" → `src/X/` or the matching subdir. Apply fuzzy matching against top-level subdirectories of the repo. If absent, set `"."`; if ambiguous, ask. |

Call `AskUserQuestion` exactly once with only unresolved required parameters (`threat_model`, `worker_model`, `severity_filter`) plus `scope_subpath` only when the user explicitly requested a narrowed scope but it is ambiguous. If the required parameters were all pre-filled and scope is absent or resolved, skip the question.

After resolving `scope_subpath`, set `finding_scope_root="${scope_subpath:-.}"`. Set `context_roots="."` by default so workers can verify callers/build settings outside a narrowed subtree without filing out-of-scope findings. If the user explicitly asks to forbid broader context, set `context_roots="${finding_scope_root}"` and note that reachability confidence may be lower.

### Phase 1: Prerequisites

**Entry:** Phase 0 complete. **Exit:** `has_unsafe`, `has_ffi`, `has_concurrency`, `has_async`, `has_packed_repr`, `has_fs_io` flags determined. Abort with a clear message if no `*.rs` files exist under `${finding_scope_root}`.

First confirm `uv` is present (`command -v uv`) — every helper script from Phase 4 onward runs
through it. If missing, **abort** and tell the user to install it (`brew install uv` or
the official installer); nothing downstream can run without it.

Probe within `${finding_scope_root:-.}` with the `Bash` commands below (non-empty output ⇒ flag true). The dedicated `Grep`/`Glob` tools are unavailable to this orchestrator because it holds `Bash` — use `grep`/`rg`/`find` via `Bash`. (The probe regexes use `\s`/`\b`; if your `grep` lacks GNU `\s` support, run them with `rg -uu` — which honors `\s` and still searches ignored files — or, if `rg` is not installed either, replace `\s`→`[[:space:]]` and drop `\b`. Widening is safe here: a false-positive capability flag only adds a harmless extra worker, whereas a missed match would skip a whole pass.)

```bash
# Rust source presence (precondition)
find "${finding_scope_root:-.}" -name '*.rs' -print -quit

# has_unsafe
grep -rlE '\bunsafe\s+(extern|fn|impl|trait)\b|\bunsafe\s*\{' --include='*.rs' "${finding_scope_root:-.}" | head -1

# has_ffi
grep -rlE 'extern\s+"(C|system|stdcall|cdecl|win64|sysv64|aapcs|fastcall|thiscall|vectorcall|efiapi)(-unwind)?"|\bextern\s+fn\b|extern\s+\{|#\[repr\((C|transparent)\b|\b(CString|CStr)\b|use\s+(libc|core::ffi|std::ffi|std::os::raw|cty)|\blibc::|\b(bindgen|cbindgen)\b|\bc_void\b' --include='*.rs' "${finding_scope_root:-.}" | head -1

# has_concurrency
grep -rlE '\b(std::(thread|sync)|parking_lot::|crossbeam|rayon::|tokio::sync|core::sync::atomic|std::sync::atomic|Atomic[A-Za-z0-9_]*|UnsafeCell|static\s+mut|unsafe\s+impl\s+(Send|Sync)|memmap2::|Mmap(Options|Mut)?|MAP_SHARED|shm_open|mmap\s*\(|memfd_create|shared_memory|raw_sync|CreateFileMapping|MapViewOfFile|once_cell|sigaction|signal_hook|nix::sys::signal|libc::signal|libc::sigaction)' --include='*.rs' "${finding_scope_root:-.}" | head -1

# has_async
grep -rlE '\basync\s+(fn|move|\{)|\.await\b|tokio::|async_std::|futures::' --include='*.rs' "${finding_scope_root:-.}" | head -1

# has_packed_repr (outer #[repr(...packed...)] and inner #![repr(...packed...)])
grep -rlE '#!?\[repr\([^]]*packed' --include='*.rs' "${finding_scope_root:-.}" | head -1

# has_fs_io (path types / construction)
grep -rlE '\bPathBuf\b|\bPath\b' --include='*.rs' "${finding_scope_root:-.}" | head -1

# has_fs_io (fs module and file APIs)
grep -rlE '\bfs::|\bFile::(open|create)\b|OpenOptions|\.exists\(\)|\.metadata\(|symlink_metadata|read_dir|read_to_string' --include='*.rs' "${finding_scope_root:-.}" | head -1
```

As with the other flags, non-empty output (from either probe) means the flag is true. These detectors are intentionally conservative: when in doubt they set the flag true, because a false-positive flag only costs a harmless extra worker while a false-negative would skip a real pass. `has_fs_io` keys on path types (`PathBuf`/`Path`, which also covers `&Path` parameters and bare `Path::` calls) and filesystem anchors (`fs::`/`File::`/`OpenOptions`/`read_dir`/…) rather than the bare `.join(`/`.push(` calls — path construction is reached via the path-type anchors, so leaving join/push out of the gate avoids matching unrelated iterator/`JoinHandle` joins and `Vec::push` that would make the gate fire on nearly every crate.

Note for `Cargo.toml`: also probe for `[dependencies] tokio`, `async-std`, etc., to set `has_async=true` even if the scope subpath has no `.await` yet (library crates often re-export).

Also probe `Cargo.toml` presence (informational — note in `run-summary.md` whether the audit was over a Cargo workspace, single crate, or loose `.rs` files):

```bash
# context_roots may be comma-separated (build_run_plan.py treats it as a list),
# so probe each root rather than passing "a,b" as one (nonexistent) path.
echo "${context_roots:-.}" | tr ',' '\n' | while IFS= read -r root; do
  find "${root:-.}" -name 'Cargo.toml' -print -quit
done | head -1
```

### Phase 2: Output Directory

**Entry:** Phase 1 flags set. **Exit:** absolute `output_dir` resolved; `${output_dir}/findings/` and `${output_dir}/coverage/` exist.

Resolve an absolute path for `output_dir` (default: `$(pwd)/.rust-review-results/$(date -u +%Y%m%dT%H%M%SZ)/`):

```bash
mkdir -p "${output_dir}/findings" "${output_dir}/coverage"
```

The `coverage/` subdirectory holds per-worker coverage-gate audit files (`coverage/worker-{N}.md`). Workers write to it instead of embedding the table in their reply — see `agents/rust-review-worker.md` step 5.

### Phase 3: Codebase Context

**Entry:** `${output_dir}` exists. **Exit:** `${output_dir}/context.md` written.

Skim `README.{md,rst,txt}` and any build/manifest file (`Cargo.toml`, `Cargo.lock`, `rust-toolchain.toml`, `build.rs`) — preflight with `find` (via `Bash`) before any `Read` (a `Read` on a missing file aborts the turn; `Glob` is unavailable to this orchestrator because it holds `Bash`). Do **not** use `Bash: ls README*` for the preflight: under zsh, an unmatched glob aborts the whole compound command before `2>/dev/null` runs. Use `find . -maxdepth 2 -name 'README*' -o -name 'Cargo.toml' -o -name 'rust-toolchain.toml' -o -name 'build.rs'`, which never fails on no-match.

Write `${output_dir}/context.md` with: YAML frontmatter (`threat_model`, `severity_filter`, `scope_subpath`, `finding_scope_root`, `context_roots`, `has_unsafe`, `has_ffi`, `has_concurrency`, `has_async`, `has_packed_repr`, `has_fs_io`, `output_dir`, `cargo_manifest` as `workspace`/`single-crate`/`absent` plus path when present), then a short markdown body with five sections — **Purpose** (1-3 sentences), **Scope** (what's in `finding_scope_root`, and that findings outside it are out of scope), **Entry points** (where untrusted data enters: network, files, CLI, IPC, `serde` deserialization, FFI inputs), **Trust boundaries** (sandboxed vs trusted peers vs arbitrary remote), **Existing hardening** (fuzzing harnesses, MIRI runs, `clippy::pedantic`, `cargo-deny`, `cargo-audit`).

### Phase 4: Build Run Plan (deterministic)

**Entry:** capability flags + `threat_model` known; `${output_dir}/findings/` exists. **Exit:** `${output_dir}/plan.json` and `${output_dir}/worker-prompts/*.txt` written; `M = worker_count` known.

Selection, filtering, path resolution, and spawn-prompt rendering are **delegated to the script** to keep spawn prompts complete and consistent:

```bash
uv run --no-project "${RUST_REVIEW_PLUGIN_ROOT}/scripts/build_run_plan.py" \
  --plugin-root "${RUST_REVIEW_PLUGIN_ROOT}" --output-dir "${output_dir}" \
  --threat-model "${threat_model}" --severity-filter "${severity_filter}" \
  --scope-subpath "${finding_scope_root:-.}" --context-roots "${context_roots:-.}" \
  --has-unsafe "${has_unsafe}" --has-ffi "${has_ffi}" \
  --has-concurrency "${has_concurrency}" --has-async "${has_async}" \
  --has-packed-repr "${has_packed_repr}" --has-fs-io "${has_fs_io}" \
  --max-passes-per-worker 4
```

The script writes `plan.json` + `worker-prompts/worker-N.txt` + (if `--cache-primer=true`, the default) `worker-prompts/cache-primer.txt`, and prints a JSON summary on stdout. Exits non-zero on any missing prompt — surface the message and stop. With the default `--max-passes-per-worker 4` the planner selects ~8 clusters → **M ≈ 13 workers** for pure safe Rust (no FFI / concurrency / async; `info-disclosure` is always on), ~10 clusters → **M ≈ 15** for concurrent safe Rust, and ~15 clusters → **M ≈ 23** for full Rust (unsafe + FFI + concurrency + async, plus `input-os-safety` when `has_fs_io` and `layout-safety` when `has_packed_repr`). M is the post-chunk worker count (`plan.workers.length`), so it runs above the cluster count — chunking splits multi-pass **non-consolidated** clusters (e.g. `panic-dos`, `memory-safety`), while the two **consolidated** clusters (`unsafe-boundary`, `concurrency-locking`) are never chunked: one worker each builds the shared inventory once and runs all its phases. `recursion-dos` is one pass per worker. After it returns, `Read plan.json` for the structured selection — never re-derive filtering or paths.

`--max-passes-per-worker N` caps the per-worker pass count. The planner deterministically splits any **non-consolidated** cluster with more than `N` passes into `ceil(K/N)` contiguous chunks; each chunk becomes its own `rust-review-worker` spawn with a `-{i}`-suffixed `cluster_id` (e.g. `panic-dos-1`, `panic-dos-2`). **Consolidated clusters (`unsafe-boundary`, `concurrency-locking`) are exempt — never chunked, regardless of pass count or override — so one worker builds their shared Phase-A inventory once and runs every phase** (chunking a consolidated cluster would force each chunk to rebuild that inventory, which workers skip in practice). The shared prompt-cache prefix and `Cluster prompt:` path are byte-identical across chunks, so the cache primer still warms every worker. Default 4 is calibrated against the heavy-tail clusters in `manifest.json`. Some output-heavy non-consolidated clusters declare a smaller manifest-level `max_passes_per_worker` override so each expensive pass gets its own worke

… (truncated)