diff --git a/.agents/skills/daily/SKILL.md b/.agents/skills/daily/SKILL.md index 0a329fa..e9f3faa 100644 --- a/.agents/skills/daily/SKILL.md +++ b/.agents/skills/daily/SKILL.md @@ -41,7 +41,7 @@ Update outcomes ONLY after upstream merge/close, never on open. Stop here if `-- Then read `reports//coverage.json` — the authoritative record of what the scan examined, written every run from the scanners' own meta findings and derived **before** `--ignore-file` suppression. If `complete` is `false`, the finding count is not a clean bill of health: carry the rows verbatim into the post's **Scan coverage** section and state in one sentence what was not examined. This replaces transcribing the Phase 1 preflight by hand; the preflight still runs, because catching a broken Semgrep *before* burning a scan beats reporting it afterwards. ## Phase 4 — Curate -Group by rule family; auto-flag tests/sample/examples/demos/fixtures as candidate-FP. Inspect top 5 real candidates in-repo via `gh api .../contents/`. Write per-finding verdicts, then append them to `reports//verdicts.json` as `source: "curation"` rows — one per rule family with its count, beside the deterministic rows the scanner already wrote. `reason_code` must come from the closed vocabulary in `scanner/verdicts.py:CURATION_REASON_CODES`; reuse or extend the module, never invent a code inline. Evaluate the quality gate boolean. +Group by rule family; auto-flag tests/sample/examples/demos/fixtures as candidate-FP. Inspect top 5 real candidates in-repo via `gh api .../contents/`. Write per-finding verdicts, then append them to `reports//verdicts.json` as `source: "curation"` rows — one per rule family with its count, beside the deterministic rows the scanner already wrote. `reason_code` must come from the closed vocabulary in `scanner/verdicts.py:CURATION_REASON_CODES`; reuse or extend the module, never invent a code inline. `rule` is REQUIRED (name the rule family); `verdict` is one of false-positive/by-design/not-applicable/hardening/real or omitted, and your sentence goes in `detail`; `confirmed-real` is first-party only, upstream version drift is `dependency-currency`. Then validate — `.venv/Scripts/python.exe scanner/run_verdict_check.py --check reports//verdicts.json` must exit 0 — and copy the validated file to `corpus/verdicts/.json`, which is committed (`reports/` is gitignored). Evaluate the quality gate boolean. ## Phase 5 — Publish (gated) Always: write `docs/scans/.md` (including the **Scan coverage** block copied from `reports//coverage.json`, never hand-written) + prepend a row to `docs/index.md` + a bullet to `docs/scan-log.md` (bump both scan counts). If gate TRUE and not strict-norm: file focused de-branded courtesy issue (+ PR for clean one-line fixes). If strict-norm: post-only or one-issue-per-critical. If gate FALSE: post-only. Open `docs:` PR on `elfrost/ai-patchlab`, merge, verify publication via `gh run list --workflow=pages-build-deployment --limit 1` = `success` AND the post returns 200 serving the new text. Do NOT gate on `pages/builds/latest` — on this Actions-published site it reports `errored` even when the deploy succeeded (2026-08-26). diff --git a/.claude/commands/daily.md b/.claude/commands/daily.md index 6ae2423..7c05541 100644 --- a/.claude/commands/daily.md +++ b/.claude/commands/daily.md @@ -119,8 +119,15 @@ Then read `reports//coverage.json`. It is the authoritative record of what 2. Inspect the top 5 real candidates in the actual repo via `gh api repos///contents/` — read the call site, confirm the threat path. 3. Write per-finding verdicts (real / by-design / FP, with the *why*), then **append them to `reports//verdicts.json`** as `source: "curation"` rows — one row per rule family with its count, not one per finding. The scanner has already written its own deterministic rows there (what `--ignore-file` and `--min-severity` removed); add yours beside them. `reason_code` comes from the closed vocabulary in `scanner/verdicts.py:CURATION_REASON_CODES` — `sql-identifier-fp`, `test-or-fixture-path`, `sample-or-demo`, `vendored-code`, `not-reachable`, `mitigated-in-app`, `by-design`, `product-surface`, `domain-noun-collision`, `placeholder-secret`, `active-harm-fp`, `credited-defense`, `confirmed-real`. Reuse a code or add one to the module; never invent one inline, because a long tail of one-off codes counts to one and the corpus stops being countable. + `rule` is REQUIRED on every row — name the rule family (`github-actions-mutable-action-tag`, `sqlalchemy-execute-raw-query`, the CVE id). A row without it can be counted but never acted on, which defeats the file. `verdict` is one of `false-positive` / `by-design` / `not-applicable` / `hardening` / `real`, or omitted — **your sentence goes in `detail`, never in `verdict`**. `confirmed-real` is first-party only; an upstream dependency merely behind a fixed version is `dependency-currency`. +4. **Validate the file before publishing.** It is hand-written JSON, so nothing else enforces the schema: + ```bash + .venv/Scripts/python.exe scanner/run_verdict_check.py --check reports//verdicts.json + ``` + Exit 0 or fix what it lists and re-run. On 2026-09-22 the first run wrote six rows with an empty `rule` and free prose in `verdict`, and nothing caught it — a closed vocabulary is only closed if something closes it. +5. Copy the validated file to `corpus/verdicts/.json` and commit it with the post. `reports/` is gitignored, so a corpus left there lives on one machine and does not survive a clone; `corpus/` is the durable half. This is the file ADR-014 had to reconstruct by hand from 87 archived reports. Writing it as you go is what turns "13th appearance of this FP" into a number that justifies mechanizing the rule. -4. **Evaluate the quality gate:** is there ≥1 real, exploitability-shaped, high-confidence item? Record the boolean — it decides Phase 5 filing. +6. **Evaluate the quality gate:** is there ≥1 real, exploitability-shaped, high-confidence item? Record the boolean — it decides Phase 5 filing. ## Phase 5 — Publish (gated) 1. **Always:** write `docs/scans/.md` from `docs/templates/scan-post.md` — including the **Scan coverage** block, copied from `reports//coverage.json` and never hand-written; prepend a new row to the Scans table in `docs/index.md` **and** a new bullet to `docs/scan-log.md` (the full prose archive), and bump the scan counts in both headers. Three files, every time — on 2026-09-06 the log was found eight entries behind the index because this step only named `index.md`. diff --git a/AGENTS.md b/AGENTS.md index 9f48899..eb6e2c9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,6 +58,9 @@ AI PatchLab is an AI-assisted security remediation toolkit. The MVP focuses on a - `scanner/models.py` - Normalized `Finding` dataclass + severity/confidence enums + `FINDING_FIELDS` - `scanner/recommendations.py` - Deterministic keyword-based recommendation enrichment - `scanner/coverage.py` - Per-scanner coverage derived from meta findings (`ToolCoverage`, `build_coverage`, `EXPECTED_TOOLS`, `is_complete`); feeds `reports/coverage.json` and the report's Scan Coverage block +- `scanner/verdict_corpus.py` - Corpus validation and aggregation (`validate_payload` reports every problem in one pass, `load_corpus` reads `corpus/verdicts/`) +- `scanner/run_verdict_check.py` - CLI: `--check ` (exit 2 on any problem, required by `/daily` Phase 4) and `--summary` (count the whole corpus) +- `corpus/verdicts/` - Committed dismissal corpus, one file per scan; the durable half, since `reports/` is gitignored - `scanner/verdicts.py` - Dismissal records (`VerdictRecord`, `summarize_removed`, `count_by_reason`, `load_records`) with closed `SCANNER_REASON_CODES` / `CURATION_REASON_CODES` vocabularies; feeds `reports/verdicts.json` and the report's Dismissed section - `scanner/report_markdown.py` - Markdown rendering, split out of `report.py` to stay under the 300-line ceiling (`write_markdown_report` is still re-exported from `scanner.report`) - `scanner/confidence.py` - Centralized `Finding.confidence` rules (one function per scanner + `confidence_for_meta_finding` for shared `not-installed` / `scan-error` / etc.) @@ -74,7 +77,7 @@ AI PatchLab is an AI-assisted security remediation toolkit. The MVP focuses on a - `src/main.py` - Legacy entry point (`python -m src.main`) - currently a loguru-wired async stub with TODOs - `.github/workflows/ci.yml` - CI: ruff + black + pytest on Python 3.11 and 3.13 - `reports/disclosures/` - Drafted private disclosure emails awaiting a manual send (gitignored) -- `tests/` - pytest tests (one module per scanner: `test_scanner_foundation.py`, `test_semgrep_scanner.py`, `test_gitleaks_scanner.py`, `test_trivy_scanner.py`, `test_dependency_scan.py`, `test_ai_review.py`, `test_patch_suggestions.py`, `test_recommendations.py`, `test_meta_findings.py`, `test_confidence_field_rules.py`, `test_coverage.py`, `test_verdicts.py`) +- `tests/` - pytest tests (one module per scanner: `test_scanner_foundation.py`, `test_semgrep_scanner.py`, `test_gitleaks_scanner.py`, `test_trivy_scanner.py`, `test_dependency_scan.py`, `test_ai_review.py`, `test_patch_suggestions.py`, `test_recommendations.py`, `test_meta_findings.py`, `test_confidence_field_rules.py`, `test_coverage.py`, `test_verdicts.py`, `test_verdict_corpus.py`) - `tests/conftest.py` - Shared fixtures (`mock_db`, `mock_http_client`, `mock_discord`, `test_config`, session `event_loop`) - `examples/` - Reference patterns to read before implementing - `PRPs/` - Active Product Requirements Prompts @@ -181,6 +184,8 @@ export AI_PATCHLAB_AI_REVIEW_COMMAND=/path/to/ai-review-wrapper - Do not call subprocesses directly from `scanner/scanners/*` - go through the runner module - Every suppression step in `run_scan` records what it removed via `summarize_removed(before, after, reason_code)` - a narrower report must never shrink its own numbers silently (ADR-016) - `reason_code` values are a closed vocabulary in `scanner/verdicts.py`; add a code to the module rather than inventing one at a call site, or the corpus stops being countable +- Curation rows are hand-written JSON, so the dataclass never runs on them - `/daily` Phase 4 MUST get exit 0 from `scanner/run_verdict_check.py --check` before publishing (ADR-017). `rule` is required on every row; `verdict` takes one of five values and the sentence goes in `detail` +- `confirmed-real` is first-party only; an upstream dependency merely behind a fixed version is `dependency-currency` - Coverage is derived from the raw `collect_findings` output **before** `apply_ignore` (`scanner/run_scan.py`) - `--ignore-file` does not exempt meta findings, so deriving it later would let a path pattern hide the fact that a tool never ran - Adding a scanner to `SCANNERS` requires adding its `Finding.tool` value to `scanner/coverage.py:EXPECTED_TOOLS`; `tests/test_coverage.py::TestRegistryDrift` fails until you do - `Finding.confidence` values come from `scanner/confidence.py` - never inline `confidence="high"` / `"medium"` / `"low"` in a scanner adapter; add or reuse a rule function instead @@ -235,7 +240,7 @@ export AI_PATCHLAB_AI_REVIEW_COMMAND=/path/to/ai-review-wrapper - Log architectural decisions in `DECISIONS.md` - Check existing ADRs before making structural changes - Record date, decision, context, and consequences -- Current ADRs of record: ADR-001 scaffold, ADR-002 data stack, ADR-003 placeholder adapters, ADR-004 Gitleaks, ADR-005 Semgrep, ADR-006 recommendation enrichment, ADR-007 patch suggestions, ADR-008 Trivy, ADR-009 pip-audit, ADR-010 disabled-by-default AI review boundary, ADR-011 centralized scanner confidence rules, ADR-012 probabilistic web template fingerprinting boundary, ADR-013 meta findings exempt from severity filtering, ADR-014 field-derived confidence tiers, ADR-015 coverage is a report artifact, ADR-016 dismissals recorded as counted rule families +- Current ADRs of record: ADR-001 scaffold, ADR-002 data stack, ADR-003 placeholder adapters, ADR-004 Gitleaks, ADR-005 Semgrep, ADR-006 recommendation enrichment, ADR-007 patch suggestions, ADR-008 Trivy, ADR-009 pip-audit, ADR-010 disabled-by-default AI review boundary, ADR-011 centralized scanner confidence rules, ADR-012 probabilistic web template fingerprinting boundary, ADR-013 meta findings exempt from severity filtering, ADR-014 field-derived confidence tiers, ADR-015 coverage is a report artifact, ADR-016 dismissals recorded as counted rule families, ADR-017 a closed vocabulary needs a closer ## Known Gotchas - **Do not tag scan posts by keyword inference.** A classifier over post bodies was built and rejected 2026-09-18: validated against 9 posts of known ground truth it gave klavis **6** finding classes where the real finding was dependency CVEs, and got 3 of 9 project families wrong (OpenBiliClaw as "developer tooling", tracecat as "MCP server"). Post bodies discuss false positives and credited defences at length, so matching them tags a clean scan with the class it *dismissed*. On a site whose whole argument is that pattern-matching produces plausible-but-wrong results, publishing plausible-but-wrong tags is self-refuting. Grouping pages are generated from the **curated index table** instead — hand-maintained, verified data diff --git a/CLAUDE.md b/CLAUDE.md index 0544738..f9f73ca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,6 +54,9 @@ This project can optionally include a parallel Codex/OpenAI runtime via `AGENTS. - `scanner/models.py` — Normalized `Finding` dataclass + severity/confidence enums + `FINDING_FIELDS`; `Finding.is_meta` flags scanner-infrastructure findings that `--min-severity` must never drop - `scanner/recommendations.py` — Deterministic keyword-based recommendation enrichment - `scanner/coverage.py` — Per-scanner coverage derived from meta findings (`ToolCoverage`, `build_coverage`, `EXPECTED_TOOLS`, `is_complete`); feeds `reports/coverage.json` and the report's Scan Coverage block +- `scanner/verdict_corpus.py` — Corpus validation and aggregation (`validate_payload` reports every problem in one pass, `load_corpus` reads `corpus/verdicts/`) +- `scanner/run_verdict_check.py` — CLI: `--check ` (exit 2 on any problem, required by `/daily` Phase 4) and `--summary` (count the whole corpus) +- `corpus/verdicts/` — Committed dismissal corpus, one file per scan; the durable half, since `reports/` is gitignored - `scanner/verdicts.py` — Dismissal records (`VerdictRecord`, `summarize_removed`, `count_by_reason`, `load_records`) with closed `SCANNER_REASON_CODES` / `CURATION_REASON_CODES` vocabularies; feeds `reports/verdicts.json` and the report's Dismissed section - `scanner/report_markdown.py` — Markdown rendering, split out of `report.py` to stay under the 300-line ceiling (`write_markdown_report` is still re-exported from `scanner.report`) - `scanner/confidence.py` — Centralized `Finding.confidence` rules (one function per scanner + `confidence_for_meta_finding` for shared `not-installed` / `scan-error` / etc.) @@ -70,7 +73,7 @@ This project can optionally include a parallel Codex/OpenAI runtime via `AGENTS. - `src/main.py` — Legacy point d'entrée (`python -m src.main`) — currently a loguru-wired async stub with TODOs - `.github/workflows/ci.yml` — CI: ruff + black + pytest on Python 3.11 and 3.13 (the 3.13 leg catches stdlib removals such as PEP 594 dropping `cgi`) - `reports/disclosures/` — Drafted private disclosure emails awaiting a manual send (gitignored with the rest of `reports/`) -- `tests/` — Tests pytest (`test_scanner_foundation.py`, `test_semgrep_scanner.py`, `test_gitleaks_scanner.py`, `test_trivy_scanner.py`, `test_dependency_scan.py`, `test_ai_review.py`, `test_patch_suggestions.py`, `test_recommendations.py`, `test_meta_findings.py`, `test_confidence_field_rules.py`, `test_coverage.py`, `test_verdicts.py`) +- `tests/` — Tests pytest (`test_scanner_foundation.py`, `test_semgrep_scanner.py`, `test_gitleaks_scanner.py`, `test_trivy_scanner.py`, `test_dependency_scan.py`, `test_ai_review.py`, `test_patch_suggestions.py`, `test_recommendations.py`, `test_meta_findings.py`, `test_confidence_field_rules.py`, `test_coverage.py`, `test_verdicts.py`, `test_verdict_corpus.py`) - `tests/conftest.py` — Fixtures partagées (`mock_db`, `mock_http_client`, `mock_discord`, `test_config`, session `event_loop`) - `examples/` — Code de référence — LIRE AVANT D'IMPLÉMENTER (api_client, config, discord_alert, mysql, playwright_scraper, scheduler, service) - `PRPs/` — Product Requirements Prompts (actifs) @@ -121,6 +124,8 @@ This project can optionally include a parallel Codex/OpenAI runtime via `AGENTS. - Any finding built with `confidence_for_meta_finding(...)` must also set `is_meta=True` so `--min-severity` cannot drop it - Every suppression step in `run_scan` records what it removed via `summarize_removed(before, after, reason_code)` — a narrower report must never shrink its own numbers silently (ADR-016) - `reason_code` values are a closed vocabulary in `scanner/verdicts.py`; add a code to the module rather than inventing one at a call site, or the corpus stops being countable +- Curation rows are hand-written JSON, so the dataclass never runs on them — `/daily` Phase 4 MUST get exit 0 from `scanner/run_verdict_check.py --check` before publishing (ADR-017). `rule` is required on every row; `verdict` takes one of five values and the sentence goes in `detail` +- `confirmed-real` is first-party only; an upstream dependency merely behind a fixed version is `dependency-currency` - Coverage is derived from the raw `collect_findings` output **before** `apply_ignore` (`scanner/run_scan.py`) — `--ignore-file` does not exempt meta findings, so deriving it later would let a path pattern hide the fact that a tool never ran - Adding a scanner to `SCANNERS` requires adding its `Finding.tool` value to `scanner/coverage.py:EXPECTED_TOOLS`; `tests/test_coverage.py::TestRegistryDrift` fails until you do - `Finding.confidence` values come from `scanner/confidence.py` — never inline `confidence="high"` / `"medium"` / `"low"` in a scanner adapter; add or reuse a rule function instead @@ -286,7 +291,7 @@ $env:AI_PATCHLAB_AI_REVIEW_COMMAND = "C:\tools\ai-review-wrapper.cmd" - Before making a structural decision, check DECISIONS.md for precedent - Use the architect agent (`/architect` or Task tool) for complex decisions - Format: ADR (Architecture Decision Record) — date, decision, context, consequences -- Current ADRs of record: ADR-001 scaffold, ADR-002 data stack, ADR-003 placeholder adapters, ADR-004 Gitleaks, ADR-005 Semgrep, ADR-006 recommendation enrichment, ADR-007 patch suggestions, ADR-008 Trivy, ADR-009 pip-audit, ADR-010 disabled-by-default AI review boundary, ADR-011 centralized scanner confidence rules, ADR-012 probabilistic web template fingerprinting boundary, ADR-013 meta findings exempt from severity filtering, ADR-014 field-derived confidence tiers, ADR-015 coverage is a report artifact, ADR-016 dismissals recorded as counted rule families +- Current ADRs of record: ADR-001 scaffold, ADR-002 data stack, ADR-003 placeholder adapters, ADR-004 Gitleaks, ADR-005 Semgrep, ADR-006 recommendation enrichment, ADR-007 patch suggestions, ADR-008 Trivy, ADR-009 pip-audit, ADR-010 disabled-by-default AI review boundary, ADR-011 centralized scanner confidence rules, ADR-012 probabilistic web template fingerprinting boundary, ADR-013 meta findings exempt from severity filtering, ADR-014 field-derived confidence tiers, ADR-015 coverage is a report artifact, ADR-016 dismissals recorded as counted rule families, ADR-017 a closed vocabulary needs a closer ## Known Gotchas - **Do not tag scan posts by keyword inference.** A classifier over post bodies was built and rejected 2026-09-18: validated against 9 posts of known ground truth it gave klavis **6** finding classes where the real finding was dependency CVEs, and got 3 of 9 project families wrong (OpenBiliClaw as "developer tooling", tracecat as "MCP server"). Post bodies discuss false positives and credited defences at length, so matching them tags a clean scan with the class it *dismissed*. On a site whose whole argument is that pattern-matching produces plausible-but-wrong results, publishing plausible-but-wrong tags is self-refuting. Grouping pages are generated from the **curated index table** instead — hand-maintained, verified data diff --git a/DECISIONS.md b/DECISIONS.md index 3c00627..25cbb38 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -134,6 +134,36 @@ Pour les décisions qui requièrent un round de discussion avant `accepted`. +### ADR-017: A closed vocabulary needs a closer + +**Date:** 2026-09-22 +**Status:** accepted + +**Context:** ADR-016 shipped the dismissal corpus on 2026-09-21. Its first production run, scan #109 (overwirehq/claude-code-telegram) on 2026-09-22, produced a `verdicts.json` with 7 scanner rows and 6 curation rows — the plumbing worked — and every one of the curation rows was invalid. All six had an empty `rule`, and all six carried free prose in `verdict` (`"by-design not-reachable"`, `"FP - parameterized"`, `"real - lockfile refresh"`). Three causes, none of which ADR-016 anticipated: the curation half is hand-written JSON, so `VerdictRecord.__post_init__` never runs and nothing enforces the closed vocabularies; `rule` was documented as part of the family key but never required; and `confirmed-real` was used for 13 dependency-currency findings under a headline of zero real findings. A fourth problem is structural rather than a drift: `reports/` is gitignored, so the corpus lived on one machine and would not survive a clone — ADR-014's measurement only worked because the archived reports happened to still be on disk. + +**Decision Drivers:** +- The corpus is worthless unless it can be counted years later, which is the entire premise of ADR-016 (must-have) +- Enforcement must run where the rows are actually written, not only where the dataclass is constructed (must-have) +- The durable copy must be committed, without un-ignoring `reports/`, which holds raw dumps and unsent private disclosure drafts (must-have) +- A repair list should cost one run, not one run per defect (should-have) + +**Considered Options:** +- **A - Document the rules harder in `/daily`.** Rejected: the instruction already said "one row per rule family" and the first run ignored it. Prose is not enforcement. +- **B - Have the scanner write the curation rows too.** Rejected: the judgement is not available to a subprocess pipeline; only the curating agent has it. +- **C - A validator the daily run must pass, plus a committed corpus directory.** **Selected.** +- **D - Un-ignore `reports/*/verdicts.json` with a negation rule.** Rejected: git does not descend into an ignored directory, so the negation needs a fragile `reports/*` + `!reports/*/` ladder that also risks exposing `reports/disclosures/`. + +**Decision:** Add `scanner/verdict_corpus.py` (`validate_payload`, `load_corpus`) and the `scanner/run_verdict_check.py` CLI. `--check ` reports every problem in every row in one pass and exits 2; `/daily` Phase 4 must see exit 0 before publishing. `rule` becomes required on every record. `dependency-currency` joins the vocabulary and `confirmed-real` is documented as first-party only. The validated file is copied to `corpus/verdicts/.json`, which is committed; `reports/` stays ignored and `reports//verdicts.json` remains the per-run artifact. `scanner/verdicts.py` reached 296 of the 300-line ceiling, so validation and corpus loading moved to the new module. + +**Consequences:** +- Positive: the failure mode was caught on day one by looking at the first real output rather than trusting the green test suite. The unit tests all passed while the production file was entirely invalid, because the tests exercised the constructor and production did not. +- Positive: `--summary` makes the ADR-014 measurement a command instead of an archaeology project. +- Negative: `/daily` gains a step that can fail and block publication. That is intended, but it means a malformed corpus now stops a scan post rather than quietly degrading. +- Negative: the corpus is duplicated — the run artifact under `reports/` and the committed copy under `corpus/`. A copy step can be skipped; the validator cannot detect a file that was never archived. +- Risks: `verdict` is still free-form enough to attract prose, since it duplicates what `reason_code` already says. If the next few runs keep fighting it, remove the field rather than keep validating it. +- The six invalid rows from scan #109 are left as they are. Rewriting them would mean inventing rule identities that were never recorded, and a corpus with one fabricated entry is worth less than one with a known hole. Scan #110 is the first valid entry. +- Amends ADR-016; does not supersede it. + ### ADR-016: Dismissals are recorded as counted rule families, not discarded **Date:** 2026-09-21 diff --git a/ROADMAP.md b/ROADMAP.md index 995b4c8..1d04802 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -75,6 +75,7 @@ - [ ] Timeouts on the semgrep, trivy and gitleaks runners (pip-audit done; the others share the same hang risk) - [ ] Exempt meta findings from `--ignore-file` suppression as well as `--min-severity` - [x] Coverage manifest: `reports/coverage.json` + a `## Scan Coverage` block rendered before the findings, so "never ran" cannot render as "found nothing" (2026/09/18) - ADR-015; published posts carry it via `docs/templates/scan-post.md` +- [x] Enforce the dismissal schema where rows are actually written, and commit the corpus (2026/09/22) - ADR-017; `run_verdict_check.py --check` gates `/daily` Phase 4, `corpus/verdicts/` is versioned - [x] Retain dismissal records with a machine-readable reason instead of deleting suppressed findings (2026/09/21) - ADR-016; `reports//verdicts.json`, scanner rows deterministic, curation rows appended by `/daily` Phase 4 from a closed reason vocabulary ## Phase 4.5 - Polish & Stabilize diff --git a/corpus/verdicts/README.md b/corpus/verdicts/README.md new file mode 100644 index 0000000..23b7f5a --- /dev/null +++ b/corpus/verdicts/README.md @@ -0,0 +1,16 @@ +# Dismissal corpus + +One file per scan: `.json`, copied from `reports//verdicts.json` by +`/daily` Phase 6. + +`reports/` is gitignored - it holds raw scanner dumps and unsent private +disclosure drafts - so a corpus left there lives on one machine and does not +survive a clone. This directory is committed, which is the whole point: ADR-014 +only worked because the archived reports happened to still be on disk. + +Validate one file, or count the corpus: + +```bash +.venv/Scripts/python.exe scanner/run_verdict_check.py --check reports//verdicts.json +.venv/Scripts/python.exe scanner/run_verdict_check.py --summary +``` diff --git a/scanner/run_verdict_check.py b/scanner/run_verdict_check.py new file mode 100644 index 0000000..dfc60ec --- /dev/null +++ b/scanner/run_verdict_check.py @@ -0,0 +1,84 @@ +"""Command line check and summary for the dismissal corpus. + +The curation half of `verdicts.json` is written as JSON by hand during +`/daily` Phase 4, so nothing constructs a `VerdictRecord` and nothing enforces +the closed vocabularies. A closed vocabulary is only closed if something closes +it - on the first production run, scan #109 wrote six curation rows with an +empty `rule` and a `verdict` of `"by-design not-reachable"`, neither of which +the schema allows. This is that enforcement. +""" + +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path + +if __package__ in {None, ""}: + sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from scanner.verdict_corpus import load_corpus, validate_payload +from scanner.verdicts import count_by_reason, total_dismissed + +DEFAULT_CORPUS_DIR = Path("corpus/verdicts") + + +def check_file(path: Path) -> list[str]: + """Return every problem in one verdicts file, or an empty list.""" + try: + payload = json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as exc: + return [f"could not be read: {exc}"] + return validate_payload(payload) + + +def parse_args(argv: list[str] | None = None) -> argparse.Namespace: + """Parse command line arguments.""" + parser = argparse.ArgumentParser(description="Validate and summarize the dismissal corpus.") + mode = parser.add_mutually_exclusive_group(required=True) + mode.add_argument("--check", help="Path to a verdicts.json to validate.") + mode.add_argument( + "--summary", + nargs="?", + const=str(DEFAULT_CORPUS_DIR), + help=f"Count the archived corpus (default: {DEFAULT_CORPUS_DIR}).", + ) + return parser.parse_args(argv) + + +def main(argv: list[str] | None = None) -> int: + """CLI wrapper. Returns 0 when valid, 2 on any problem.""" + args = parse_args(argv) + + if args.check: + path = Path(args.check) + problems = check_file(path) + if problems: + print(f"{path}: {len(problems)} problem(s)", file=sys.stderr) + for problem in problems: + print(f" - {problem}", file=sys.stderr) + return 2 + print(f"{path}: valid") + return 0 + + corpus_dir = Path(args.summary) + if not corpus_dir.is_dir(): + print(f"No corpus directory at {corpus_dir}", file=sys.stderr) + return 2 + + try: + records = load_corpus(corpus_dir) + except (OSError, ValueError, json.JSONDecodeError) as exc: + print(f"Corpus could not be loaded: {exc}", file=sys.stderr) + return 2 + + scans = len(list(corpus_dir.glob("*.json"))) + print(f"Corpus: {scans} scans, {total_dismissed(records)} findings dismissed") + for reason, count in count_by_reason(records).items(): + print(f" {count:>6} {reason}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scanner/verdict_corpus.py b/scanner/verdict_corpus.py new file mode 100644 index 0000000..58d2884 --- /dev/null +++ b/scanner/verdict_corpus.py @@ -0,0 +1,111 @@ +"""Validation and aggregation for the dismissal corpus. + +Split from `scanner.verdicts`, which owns the record and its producers; this +module owns reading a corpus back and proving it is trustworthy. + +The curation half of `verdicts.json` is written as JSON by hand during `/daily` +Phase 4, so nothing constructs a `VerdictRecord` and nothing enforces the closed +vocabularies. A closed vocabulary is only closed if something closes it: on its +first production run, scan #109 wrote six curation rows with an empty `rule` and +free prose in `verdict`. Both are exactly what makes a corpus uncountable later +(ADR-017). +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +from scanner.verdicts import ( + REASON_CODES, + VERDICT_SOURCES, + VERDICTS, + VerdictRecord, + count_by_reason, + load_records, + total_dismissed, +) + + +def validate_payload(payload: dict[str, Any]) -> list[str]: + """Return every problem in a `verdicts.json` payload, as readable sentences. + + `load_records` raises on the first bad row, which is right for a library and + useless for a check run: the curation half of the corpus is written as JSON + by hand, so nothing constructs a `VerdictRecord` and nothing enforces the + closed vocabularies. This collects all problems at once so one run shows the + whole repair list. + + Returns: + An empty list when the payload is valid. + """ + problems: list[str] = [] + rows = payload.get("records") + if not isinstance(rows, list): + return ["`records` is missing or is not a list."] + + for index, row in enumerate(rows): + where = f"record {index}" + if not isinstance(row, dict): + problems.append(f"{where}: not an object.") + continue + + label = f"{where} ({row.get('reason_code', '?')}/{row.get('rule') or 'no rule'})" + # Each field is checked on its own rather than through `VerdictRecord`, + # which raises on the first failure. A repair list that reveals one + # problem per run costs as many runs as there are problems. + if row.get("source") not in VERDICT_SOURCES: + problems.append( + f"{label}: source {row.get('source')!r} is not one of {VERDICT_SOURCES}." + ) + if row.get("reason_code") not in REASON_CODES: + problems.append( + f"{label}: reason_code {row.get('reason_code')!r} is not in the vocabulary." + ) + if not str(row.get("rule", "")).strip(): + problems.append( + f"{label}: rule is empty - name the rule family so the row is actionable." + ) + if not str(row.get("tool", "")).strip(): + problems.append(f"{label}: tool is empty.") + verdict = str(row.get("verdict", "")) + if verdict and verdict not in VERDICTS: + problems.append( + f"{label}: verdict {verdict!r} is not one of {VERDICTS} - " + "the sentence belongs in `detail`." + ) + try: + if int(row.get("count", 0)) < 1: + problems.append(f"{label}: count must be at least 1.") + except (TypeError, ValueError): + problems.append(f"{label}: count {row.get('count')!r} is not a number.") + + if not problems: + records = load_records(payload) + declared = payload.get("total_dismissed") + actual = total_dismissed(records) + if declared is not None and declared != actual: + problems.append(f"`total_dismissed` says {declared} but the rows count {actual}.") + declared_reasons = payload.get("by_reason") + actual_reasons = count_by_reason(records) + if declared_reasons is not None and declared_reasons != actual_reasons: + problems.append( + f"`by_reason` says {declared_reasons} but the rows give {actual_reasons}." + ) + + return problems + + +def load_corpus(directory: Path) -> tuple[VerdictRecord, ...]: + """Load every archived verdict file under `directory`, sorted by name. + + This is the accumulating corpus: one file per scan, committed, so the + measurement that justified ADR-014 becomes a `sum()` rather than an + archaeology project over whatever happens to still be on one machine. + """ + records: list[VerdictRecord] = [] + for path in sorted(directory.glob("*.json")): + payload = json.loads(path.read_text(encoding="utf-8")) + records.extend(load_records(payload)) + return tuple(records) diff --git a/scanner/verdicts.py b/scanner/verdicts.py index b48ee7d..4442480 100644 --- a/scanner/verdicts.py +++ b/scanner/verdicts.py @@ -47,6 +47,7 @@ "placeholder-secret", "active-harm-fp", "credited-defense", + "dependency-currency", "confirmed-real", ) """Closed vocabulary for judgement rows. @@ -55,6 +56,13 @@ long tail of one-off codes counts to one. Each code here is a curation pattern the scan series has hit repeatedly; adding one is cheap, inventing one per scan defeats the file. + +`confirmed-real` is **first-party only**: a defect in the scanned project's own +code, established rather than suspected. An upstream dependency that is merely +behind a fixed version is `dependency-currency`. Scan #109 collapsed the two and +reported 13 `confirmed-real` under a headline of zero real findings; counting +them together across scans would mix "demonstrated vulnerability" with "version +is old", which is the distinction the whole series turns on. """ REASON_CODES = SCANNER_REASON_CODES + CURATION_REASON_CODES @@ -86,6 +94,12 @@ def __post_init__(self) -> None: raise ValueError(f"Unsupported verdict: {self.verdict}") if self.count < 1: raise ValueError(f"A verdict record counts at least one finding: {self.count}") + if not self.rule.strip(): + # Rule identity is what makes the corpus actionable. ADR-014 needed + # "sqlalchemy-execute-raw-query fired 157 times", not "some semgrep + # rules were dismissed"; a row without it can be counted but never + # acted on. + raise ValueError(f"A verdict record names a rule family: {self!r}") def to_dict(self) -> dict[str, Any]: """Return a JSON-serializable verdict row.""" diff --git a/tests/test_verdict_corpus.py b/tests/test_verdict_corpus.py new file mode 100644 index 0000000..5ce08d3 --- /dev/null +++ b/tests/test_verdict_corpus.py @@ -0,0 +1,184 @@ +"""Tests for dismissal-corpus validation and aggregation. + +The curation half of `verdicts.json` is hand-written JSON, so the dataclass +never runs and the closed vocabularies enforce nothing on their own. Scan #109 +proved it on the first production run: six rows with an empty `rule` and free +prose in `verdict`. These tests pin the enforcement that closes that gap, and +the property that made it expensive - a checker that reveals one problem per +run costs as many runs as there are problems (ADR-017). +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from scanner.verdict_corpus import load_corpus, validate_payload +from scanner.verdicts import ( + CURATION_REASON_CODES, + VerdictRecord, + verdicts_payload, +) + + +def _row(**overrides: object) -> dict[str, object]: + """Build a valid raw verdict row, then break exactly one field.""" + row: dict[str, object] = { + "source": "curation", + "reason_code": "by-design", + "tool": "semgrep", + "rule": "github-actions-mutable-action-tag", + "count": 16, + "verdict": "by-design", + "detail": "SHA-pin hardening nudge on CI action refs.", + } + row.update(overrides) + return row + + +def _payload(*rows: dict[str, object]) -> dict[str, object]: + """Wrap raw rows without recomputing the derived totals.""" + return {"records": list(rows)} + + +class TestVocabularyAddition: + """`confirmed-real` is first-party only; version drift has its own code.""" + + def test_dependency_currency_is_a_reason_code(self) -> None: + assert "dependency-currency" in CURATION_REASON_CODES + + def test_confirmed_real_still_exists(self) -> None: + assert "confirmed-real" in CURATION_REASON_CODES + + def test_the_two_are_distinct_codes(self) -> None: + rows = ( + VerdictRecord("curation", "confirmed-real", "semgrep", "a", 1), + VerdictRecord("curation", "dependency-currency", "trivy", "b", 13), + ) + assert verdicts_payload(rows)["by_reason"] == { + "dependency-currency": 13, + "confirmed-real": 1, + } + + +class TestRuleIsRequired: + """A row without a rule family can be counted but never acted on.""" + + def test_record_rejects_an_empty_rule(self) -> None: + with pytest.raises(ValueError, match="names a rule family"): + VerdictRecord("curation", "by-design", "semgrep", "", 1) + + def test_record_rejects_a_whitespace_rule(self) -> None: + with pytest.raises(ValueError, match="names a rule family"): + VerdictRecord("curation", "by-design", "semgrep", " ", 1) + + def test_validator_flags_an_empty_rule(self) -> None: + problems = validate_payload(_payload(_row(rule=""))) + assert any("rule is empty" in problem for problem in problems) + + +class TestValidatePayload: + """One run must show the whole repair list, not the first item of it.""" + + def test_a_good_payload_has_no_problems(self) -> None: + assert validate_payload(_payload(_row())) == [] + + def test_a_generated_payload_round_trips_clean(self) -> None: + rows = (VerdictRecord("scanner", "ignore-pattern", "semgrep", "rule-a", 3),) + assert validate_payload(verdicts_payload(rows)) == [] + + def test_missing_records_key(self) -> None: + assert validate_payload({}) == ["`records` is missing or is not a list."] + + def test_flags_an_unknown_reason_code(self) -> None: + problems = validate_payload(_payload(_row(reason_code="vibes"))) + assert any("not in the vocabulary" in problem for problem in problems) + + def test_flags_an_unknown_source(self) -> None: + problems = validate_payload(_payload(_row(source="intern"))) + assert any("source" in problem for problem in problems) + + def test_flags_free_prose_in_verdict(self) -> None: + problems = validate_payload(_payload(_row(verdict="FP - parameterized"))) + assert any("belongs in `detail`" in problem for problem in problems) + + def test_an_empty_verdict_is_allowed(self) -> None: + assert validate_payload(_payload(_row(verdict=""))) == [] + + def test_flags_a_zero_count(self) -> None: + problems = validate_payload(_payload(_row(count=0))) + assert any("at least 1" in problem for problem in problems) + + def test_flags_a_non_numeric_count(self) -> None: + problems = validate_payload(_payload(_row(count="many"))) + assert any("not a number" in problem for problem in problems) + + def test_flags_an_empty_tool(self) -> None: + problems = validate_payload(_payload(_row(tool=""))) + assert any("tool is empty" in problem for problem in problems) + + def test_reports_every_problem_in_one_row_at_once(self) -> None: + """The regression: scan #109 needed two runs to reveal two defects.""" + problems = validate_payload(_payload(_row(rule="", verdict="FP - parameterized"))) + assert len(problems) == 2 + assert any("rule is empty" in problem for problem in problems) + assert any("belongs in `detail`" in problem for problem in problems) + + def test_reports_problems_across_several_rows(self) -> None: + problems = validate_payload(_payload(_row(rule=""), _row(count=0))) + assert len(problems) == 2 + + def test_flags_a_total_that_disagrees_with_the_rows(self) -> None: + payload = verdicts_payload((VerdictRecord("curation", "by-design", "t", "r", 5),)) + payload["total_dismissed"] = 99 + problems = validate_payload(payload) + assert any("total_dismissed" in problem for problem in problems) + + def test_flags_by_reason_that_disagrees_with_the_rows(self) -> None: + payload = verdicts_payload((VerdictRecord("curation", "by-design", "t", "r", 5),)) + payload["by_reason"] = {"by-design": 4} + problems = validate_payload(payload) + assert any("by_reason" in problem for problem in problems) + + def test_a_row_that_is_not_an_object(self) -> None: + assert validate_payload({"records": ["nope"]}) == ["record 0: not an object."] + + +class TestLoadCorpus: + """The corpus is the point: many scans, one countable total.""" + + def test_an_empty_directory_yields_no_records(self, tmp_path: Path) -> None: + assert load_corpus(tmp_path) == () + + def test_records_accumulate_across_files(self, tmp_path: Path) -> None: + for index, count in enumerate((3, 7)): + rows = (VerdictRecord("curation", "sql-identifier-fp", "semgrep", "rule", count),) + (tmp_path / f"scan-{index}.json").write_text( + json.dumps(verdicts_payload(rows)), encoding="utf-8" + ) + + records = load_corpus(tmp_path) + + assert len(records) == 2 + assert sum(record.count for record in records) == 10 + + def test_files_are_read_in_name_order(self, tmp_path: Path) -> None: + for name, rule in (("b.json", "second"), ("a.json", "first")): + rows = (VerdictRecord("curation", "by-design", "t", rule, 1),) + (tmp_path / name).write_text(json.dumps(verdicts_payload(rows)), encoding="utf-8") + + assert [record.rule for record in load_corpus(tmp_path)] == ["first", "second"] + + def test_a_corrupt_file_fails_loudly(self, tmp_path: Path) -> None: + """A corpus that skips what it cannot parse under-reports its own total.""" + (tmp_path / "bad.json").write_text( + json.dumps({"records": [_row(reason_code="vibes")]}), encoding="utf-8" + ) + with pytest.raises(ValueError, match="reason code"): + load_corpus(tmp_path) + + def test_non_json_files_are_ignored(self, tmp_path: Path) -> None: + (tmp_path / "README.md").write_text("not a corpus file", encoding="utf-8") + assert load_corpus(tmp_path) == ()