sec-review-team

Files (86)

• SKILL.md
• consolidation-template.md · docs/
• output-contract.md · docs/
• scanner-coverage.md · docs/
• tradeoffs.md · docs/
• README.md · fixtures/
• ci.yml · fixtures/cicd-github-actions-vulnerable/.github/workflows/
• deploy.yml · fixtures/cicd-github-actions-vulnerable/.github/workflows/
• README-FIXTURE.md · fixtures/cicd-github-actions-vulnerable/
• expected-coverage.jsonl · fixtures/cicd-github-actions-vulnerable/
• expected-findings.jsonl · fixtures/cicd-github-actions-vulnerable/
• README-FIXTURE.md · fixtures/concurrency-race-vulnerable/
• auth.js · fixtures/concurrency-race-vulnerable/
• expected-coverage.jsonl · fixtures/concurrency-race-vulnerable/
• expected-findings.jsonl · fixtures/concurrency-race-vulnerable/
• package.json · fixtures/concurrency-race-vulnerable/
• session.js · fixtures/concurrency-race-vulnerable/
• wallet.js · fixtures/concurrency-race-vulnerable/
• Dockerfile · fixtures/container-docker-vulnerable/
• README-FIXTURE.md · fixtures/container-docker-vulnerable/
• docker-compose.yml · fixtures/container-docker-vulnerable/
• expected-coverage.jsonl · fixtures/container-docker-vulnerable/
• expected-findings.jsonl · fixtures/container-docker-vulnerable/
• README-FIXTURE.md · fixtures/desktop-tauri-vulnerable/
• expected-coverage.jsonl · fixtures/desktop-tauri-vulnerable/
• expected-findings.jsonl · fixtures/desktop-tauri-vulnerable/
• Cargo.toml · fixtures/desktop-tauri-vulnerable/src-tauri/
• default.json · fixtures/desktop-tauri-vulnerable/src-tauri/capabilities/
• schema.sql · fixtures/desktop-tauri-vulnerable/src-tauri/src/db/
• lib.rs · fixtures/desktop-tauri-vulnerable/src-tauri/src/
• tauri.conf.json · fixtures/desktop-tauri-vulnerable/src-tauri/
• README-FIXTURE.md · fixtures/iac-terraform-vulnerable/
• expected-coverage.jsonl · fixtures/iac-terraform-vulnerable/
• expected-findings.jsonl · fixtures/iac-terraform-vulnerable/
• iam.tf · fixtures/iac-terraform-vulnerable/
• main.tf · fixtures/iac-terraform-vulnerable/
• network.tf · fixtures/iac-terraform-vulnerable/
• terraform.tfvars · fixtures/iac-terraform-vulnerable/
• variables.tf · fixtures/iac-terraform-vulnerable/
• README-FIXTURE.md · fixtures/llm-agent-python-vulnerable/
• agent.py · fixtures/llm-agent-python-vulnerable/
• expected-coverage.jsonl · fixtures/llm-agent-python-vulnerable/
• expected-findings.jsonl · fixtures/llm-agent-python-vulnerable/
• rag.py · fixtures/llm-agent-python-vulnerable/
• requirements.txt · fixtures/llm-agent-python-vulnerable/
• README-FIXTURE.md · fixtures/privacy-telemetry-vulnerable/
• app.js · fixtures/privacy-telemetry-vulnerable/
• expected-coverage.jsonl · fixtures/privacy-telemetry-vulnerable/
• expected-findings.jsonl · fixtures/privacy-telemetry-vulnerable/
• index.html · fixtures/privacy-telemetry-vulnerable/
• package.json · fixtures/privacy-telemetry-vulnerable/
• README-FIXTURE.md · fixtures/python-django-vulnerable/
• expected-coverage.jsonl · fixtures/python-django-vulnerable/
• expected-findings.jsonl · fixtures/python-django-vulnerable/
• views.py · fixtures/python-django-vulnerable/myapp/
• settings.py · fixtures/python-django-vulnerable/myproject/
• requirements.txt · fixtures/python-django-vulnerable/
• run-harness.sh · fixtures/
• README-FIXTURE.md · fixtures/webapp-express-vulnerable/
• expected-coverage.jsonl · fixtures/webapp-express-vulnerable/
• …and 26 more

SKILL.md (first lines)

# Multi-Agent Security Review Team — Orchestration

**What this optimizes for:** two complementary guarantees, in this order of priority. First, coverage-of-absence — a "clean" verdict on any category must cite the searches that back it, never a bare assertion. Second, verification-of-positives — a finding only reaches the report after surviving independent adversarial validation (Step 4.5) against a category-appropriate proof standard, with self-reported specialist confidence no longer sufficient on its own. "Done" means every roster specialist's owned categories are accounted for (checked-clean, checked-issues-found, or explicitly not-checked with a reason), and every surviving finding in the report is validator-confirmed, not merely raised.

Follow these steps. Review-only. Never apply fixes.

## Asking the user

This rule covers every question this skill asks the user: a confirmation, a choice, a missing value, a numbered decision list, or a stop-and-ask. It applies even where a later step says "ask the user" or "ask exactly once". The user may watch the decisions pane rather than the chat, so a question that only goes to chat can go unanswered.

- **When `mcp__decision-tracker__decision` is in your tool list** (loaded, or named as a deferred tool; load a deferred one with `ToolSearch` first), log each question before you ask it:
  1. Call it with `action: "open"`, the question (ending in `?`), and `options` when the answers are a closed set. Set `required: false` when the work can go on without an answer. A numbered list of questions opens one decision per item. Keep each returned `id`.
  2. Ask the question in chat as this skill says. The open call does not replace the chat question.
  3. When the user answers, in chat or as a `Decision Dn: <answer>` message from the plugin, call the tool with `action: "close"`, the `id` and the answer.
  4. When the change the answer asked for is in place, call it with `action: "implement"`, the `id` and a `path:line`.
- **When the tool is absent**, ask in chat only.
- An `AskUserQuestion` call is logged on its own. Do not open it again.
- This rule changes where a question is logged. It does not change when this skill asks, or how many times.

## Step 0: Pre-run confirmation

The skill's largest user-facing safety gate. Never spawn specialists without showing this block (unless `--yes` / `auto-approve` is passed).

### 0.1 Parse arguments

- **Target path** — absolute path. If missing, ask: *"What repo should I review? (absolute path)"* Verify it exists and is readable.
- **Scope** — one of:
  - `all` — full tree
  - `<subdir>` — a subdirectory, e.g. `src/api/`
  - `diff vs main` (or `diff vs <branch>`) — only files changed on current branch vs target branch
  If missing, ask: *"What scope? Full tree, a subdirectory, or a diff?"*
- **Flags:**
  - `--yes` / `-y` — skip confirmation entirely. Default is to confirm.
  - `--lite` / `-l` — run only the baseline 4 (`secrets-crypto-auditor`, `dependency-supplychain-auditor`, `silent-failure-hunter`, `data-exposure-auditor`) plus `auth-authz-auditor` and `input-validation-auditor` when the stack roster would otherwise include them (5–6 agent subset). Auto-activated when scope is a diff with ≤25 changed files, same threshold as `code-review-team`.
  - `--full` — force a fresh baseline scan, ignoring any prior run history for this target (see Step 0.1a). Without this flag, a target with prior history runs in re-review mode automatically.

### 0.1a Detect prior run (re-review mode)

Every run writes its artifacts under a run-scoped directory, `RUN_DIR = <TARGET>/.planning/security-review/runs/<run_id>/` (`run_id` = an ISO-timestamp-derived slug), so a later run never overwrites an earlier one. Runs accumulate a top-level ledger at `<TARGET>/.planning/security-review/known-findings.jsonl` — one record per validator-confirmed `root_issue` ever reported for this target (`root_issue`, `title`, `severity`, `first_seen_run_id`, `first_seen_date`, `file`, `evidence_snippet` — the latter two exist solely so a later run can cheaply re-verify the issue is still actually present before reporting it as "still open"; see Step 4.5's ledger-append and re-verification logic).

1. Check whether `<TARGET>/.planning/security-review/known-findings.jsonl` exists.
2. **Exists, and `--full` was not passed** → `re_review_mode = true`. This run skips `root_issue`s already in the ledger and suppresses all `low`/`info`-severity findings from the report (Step 4.5's re-review filter, `re_review_mode` branch — sec-review-team's severity floor for "nit-equivalent" is `low`/`info`, mirroring `code-review-team`'s nit suppression). Announce this in the Step 0.7 confirmation block.
3. **Missing, or `--full` was passed** → `re_review_mode = false`. This is a baseline scan; every validator-confirmed finding is eligible for the report, and the ledger is (re)built from this run's results.

All `RUN_DIR`-relative paths referenced in Steps 0.9 through 5 below resolve against the `RUN_DIR` computed here — the ledger itself is the one exception, always written at the top-level `<TARGET>/.planning/security-review/` regardless of `run_id`.

### 0.2 Enumerate scope

Use git-aware commands (fall back to `find` if not a git repo):

```bash
# File count + line count (scoped)
cd "<TARGET>"
if [[ "<SCOPE>" == "all" ]]; then
  FILE_COUNT=$(git ls-files | wc -l)
  LINE_COUNT=$(git ls-files | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}')
elif [[ "<SCOPE>" == diff* ]]; then
  BASE="${SCOPE#diff vs }"; BASE="${BASE:-main}"
  FILES=$(git diff --name-only "$BASE"...HEAD)
  FILE_COUNT=$(echo "$FILES" | wc -l)
  LINE_COUNT=$(echo "$FILES" | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}')
else
  FILE_COUNT=$(git ls-files "<SCOPE>" | wc -l)
  LINE_COUNT=$(git ls-files "<SCOPE>" | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}')
fi
```

Cap `LINE_COUNT` at 500k (anything larger: warn and suggest scope narrowing).

### 0.3 Detect stack signals

Read the highest-signal manifests first (cheapest): `README.md` top 50 lines, `package.json`, `Cargo.toml`, `pyproject.toml`, `go.mod`, `Gemfile`, `tauri.conf.json`, `Dockerfile`, `.github/workflows/` (existence only), `requirements.txt`. Two or three file-reads is enough.

Apply each rule in `stack-signals.md` against what you observe. Collect matching signals (multiple can match). Example: a Tauri note app emits `desktop` + `desktop-with-webview`.

### 0.4 Pick default roster

Per `stack-signals.md`: union the specialist sets across all matching stack signals. Always include the baseline 4 (`secrets-crypto-auditor`, `dependency-supplychain-auditor`, `silent-failure-hunter`, `data-exposure-auditor`).

**`--lite` mode** (explicit flag or diff ≤25 files, per 0.1): restrict the roster to the baseline 4 plus `auth-authz-auditor` and `input-validation-auditor` — but only if the stack-signal union would have included them anyway. Do not add either specialist in lite mode if the full-roster resolution wouldn't have included it (e.g. a pure `iac`-only target stays baseline-only in lite mode, not baseline+auth+input-validation). All other stack-specific additions (`iac-auditor`, `container-runtime-auditor`, `frontend-security-auditor`, etc.) are dropped in lite mode. `--lite` never drops `input-validation-auditor` or `auth-authz-auditor` when they'd otherwise be in roster — mirroring `code-review-team`'s decision to keep `security-reviewer` mandatory in its lite roster, on the same reasoning: a missed injection or authz bug outweighs the tokens saved by cutting these two.

### 0.5 Detect scanner availability

For each scanner in Step 2.5's pre-pass list, run `command -v <name>`. Flag available vs missing. Missing scanners don't block — they just reduce pre-pass coverage; dependent specialists will work in manual mode.

### 0.6 Estimate runtime + tokens + cost

**Historical baseline** (from the CBP-060 reference run against `recall`, executed on Opus 4.7 — kept as a labeled historical data point, not a current default):

- **Input tokens per specialist:** `~min(40k, 30 + 0.02 × LINE_COUNT)` k tokens — small repos float near 30k overhead; large repos grow linearly with source lines but the agent self-caps reads.
- **Output tokens per specialist:** `~5–10k` regardless of repo size (findings + prose).
- **Total tokens:** `input-validation-auditor` and `auth-authz-auditor` each run as a loop-until-dry consensus fan-out when in roster (Step 3a, `MIN_PASSES = 2` .. `MAX_PASSES = 8`), not a single pass. Since the actual pass count (`N`) isn't known until the loop terminates, estimate with `N` at its expected midpoint (`~4`, i.e. `N-1 = 3` extra specialist-equivalents) for a planning estimate, and note the true range spans `MIN_PASSES` to `MAX_PASSES`: `Total tokens ≈ (N_specialists + 3 × N_consensus_specialists_in_roster) × (input + output)`, worst case `(N_specialists + 7 × N_consensus_specialists_in_roster) × (input + output)` if every consensus specialist runs the full 8-pass cap. The Step 4.5 validator adds roughly one more agent call per surviving finding group after consolidation — this can't be sized until findings exist, so note it as "+validator passes (post-consolidation, ~1 call per finding group)" rather than folding it into this estimate.
- **Runtime:** longest-path specialist `≈ 3 min` baseline; add `0.6 sec / file read`. Dependency auditor is typically slowest (`~20–30 min` on a repo with a 700-package `node_modules`). Report estimate as a range: `longest × 1.1` to `longest × 1.5`. The two consensus specialists run their first 2 passes concurrently, then any further passes one at a time until dry or capped — so wall-clock adds roughly one to a few specialists' worth of time depending on how quickly they dry up, not a fixed multiple — plus a consolidation/tally step afterward (~30s each).
- **Cost (compute at run time, not from the historical baseline above):** look up the current model tier's per-MTok input/output pricing (e.g. via the `claude-api` skill's model catalog or the Models API) for whatever model the orchestrator is actually running on, then compute `(input_tokens × price_in + output_tokens × price_out) / 1_000_000`, using the consensus-adjusted token total above plus the validator caveat. Print `$X.XX` along with the pricing assumption used (model name + rate) so the estimate doesn't silently rot next generation.

### 0.7 Render confirmation block

```
╭─ sec-review-team ──────────────────────────────────────────╮
│                                                            │
│  Target:     <absolute path>                               │
│  Scope:      <scope>                                       │
│              <N> files, <N> lines                          │
│                                                            │
│  Stack:      <detected signals, comma-separated>           │
│  Model:      <current model ID>                            │
│  Mode:       <full | lite>                                 │
│  Run mode:   <baseline | re-review (vs run <prior_run_id>)>│
│                                                            │
│  Specialists (<N> in roster):                              │
│    ✓ secrets-crypto-auditor                                │
│    ✓ dependency-supplychain-auditor                        │
│    ✓ silent-failure-hunter                                 │
│    ✓ data-exposure-auditor                                 │
│    ✓ auth-authz-auditor          (loop-until-dry consensus) │
│    ✓ input-validation-auditor    (loop-until-dry consensus) │
│    ✓ <stack-specific additions>  (full mode only)          │
│                                                            │
│  Pre-pass scanners:                                        │

From tbeack/fsad_playbook, MIT.

More from tbeack/fsad_playbook