diff --git a/README.md b/README.md index 9b0a6dd..45e4f0c 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,29 @@ pull request: Details and rationale: [`docs/CONTRIBUTING.md`](docs/CONTRIBUTING.md). +## Current results + +Two scanners measured against the corpus so far: + +| Scanner | True positives | False positives | Character | +|---|---|---|---| +| **ThreatCrush** v0.1.0 | 15.6% (12/77) | **0.0%** (0/39) | Secrets only — credentials yes, code vulnerabilities no | +| **Semgrep** 1.172.0 (`p/security-audit`, `p/secrets`) | 27.3% (21/77) | 7.7% (3/39) | Code patterns yes, secrets shapes largely no | + +They are close to mirror images, which is the point of keeping a fixed corpus: +the numbers are comparable because the answer key does not move. + +Two caveats before quoting either figure: + +- **Dead-code guards depress the true-positive rate.** Every payload sits behind + `if (false)`, so analyzers that prune unreachable branches under-report. Part + of both scores is that, not a genuine detection gap. +- **Secrets fixtures are degraded.** GitHub push protection rejected the + originals, so the Slack fixtures measure pattern detection rather than + validated-credential detection. + +Both are documented in [`docs/SCANNER_INTEGRATION.md`](docs/SCANNER_INTEGRATION.md). + ## Running it yourself ```bash diff --git a/docs/SCANNER_INTEGRATION.md b/docs/SCANNER_INTEGRATION.md index 24cb2d3..946097c 100644 --- a/docs/SCANNER_INTEGRATION.md +++ b/docs/SCANNER_INTEGRATION.md @@ -154,8 +154,9 @@ Ways to handle it, in order of preference: Do not commit the result, and do not run the corpus after doing this. -Track this as PRD Open Question 1 — it is the single biggest threat to the -validity of the coverage numbers. +This remains the single biggest threat to the validity of every coverage number +in this repository. It is unrelated to PRD Open Question 1, which asked whether +the CLI supports PR-level scanning — answered below: it does not. --- @@ -249,17 +250,76 @@ were removed from the workflow on 2026-08-01 and replaced with the npm install above. `threatcrush.com` and `sh1pt.com` themselves do resolve; only those subdomains are absent. -### The CLI's flags are unverified +### The CLI's real interface -The scan invocation in the workflow — +The PRD specified `threatcrush scan --format sarif --output … --config … +--fail-on critical,high --verbose`. **None of those options exist.** From the +published bundle: +```js +.command("scan").description("Scan codebase for vulnerabilities and secrets") + .argument("[path]", "Path to scan", ".") ``` -threatcrush scan --format sarif --output … --config … --fail-on critical,high + +A path, and nothing else. Two consequences: + +- **PRD Open Question 1 is answered: no.** There is no native pull-request + scanning. The workflow scans the `vulns` directory and lets the coverage + validator do the per-file attribution. +- **There is no SARIF output.** `scripts/threatcrush-to-sarif.py` converts the + CLI's text output instead, which is what keeps the Security-tab upload, the + coverage validator and the PR comment tool-agnostic. + +The workflow keeps a **Record the CLI interface** step that prints `--help` for +the CLI and its `scan` subcommand into every run log, so a future release that +changes the interface shows up immediately rather than silently scoring zero. + +#### Output format + +Findings arrive as multi-line blocks, not one per line: + +``` + CRITICAL AWS Access Key + File: secrets/aws-credentials-hardcoded.env:23 + Info: Possible AWS Access Key detected + Code: **************** ``` -— comes from the PRD, and has never been checked against a real binary. The -workflow therefore includes a **Record the CLI interface** step that prints -`threatcrush --help` and `threatcrush scan --help` into the run log. Read that -log and correct the scan step from the evidence before relying on any coverage -number. If the flags differ, the scan step fails soft, so the run still reports -"scanner did not run" rather than a misleading zero-findings result. +Three details the converter has to handle, each of which caused a real bug: + +| Detail | Why it matters | +|---|---| +| Paths are relative to the scan root | `secrets/x.env`, not `vulns/secrets/x.env`. Unprefixed, every finding scores as "outside corpus" and the true-positive rate reads 0% even though the scan worked. Hence `--path-prefix`. | +| Severity is bare for `CRITICAL`, bracketed for `[HIGH]`/`[MEDIUM]`/`[LOW]` | A single regex shape misses half the findings. | +| Whole-file findings report line `:0` | SARIF requires `startLine >= 1`; the converter clamps. | +| `Code:` lines are redacted excerpts | Matching them double-counts every finding, so they are skipped. | + +Real CLI output is committed at `tests/fixtures/threatcrush-scan-output.txt`, so +the parser is tested against observed behaviour rather than assumption. + +The converter **fails closed**. If it cannot recognise the output it exits +non-zero and dumps the first 40 lines, and the workflow fails the job. Emitting +empty SARIF instead would report "0 findings" — indistinguishable from a clean +scan, and a silent false negative on a repository that exists to contain +findings. + +### Baseline: ThreatCrush + +Measured against this corpus (CLI v0.1.0, package `@profullstack/threatcrush`): + +| Metric | Result | +|---|---| +| True positive rate | 15.6% (12/77) | +| False positive rate | **0.0% (0/39)** | +| Unattributed findings | 0 | + +**ThreatCrush is a secrets scanner.** It detected the AWS access key, Stripe +key, GitHub token, Slack token, both database URLs and the committed `.env` +file, and did not attempt the code-level classes — no SQL injection, XSS, SSRF, +command injection, deserialisation or prototype pollution was reported. The +missing 65 are almost entirely those categories. + +A 0% false-positive rate against the 39-line control group is a genuinely good +result and the more interesting half of this measurement: it flagged no correct +implementation. Read the two baselines together — Semgrep covers code patterns +and misses secrets shapes, ThreatCrush is the mirror image.