diff --git a/.kiro/specs/contributor-scan/requirements.md b/.kiro/specs/contributor-scan/requirements.md index 666b915..933cec2 100644 --- a/.kiro/specs/contributor-scan/requirements.md +++ b/.kiro/specs/contributor-scan/requirements.md @@ -194,3 +194,65 @@ tables and queries can be checked for regressions that pair scoring cannot see. 3. The system shall treat an unexplained loss of a previously-entailed finding as a regression to be investigated, not a number to be accepted. 4. The baseline shall record the engine version, because a CPG engine change moves every verdict it produces. + +--- + +## Proposed amendments from group 2 (drafted 2026-09-09, NOT approved) + +Group 2 was specified as the phase that can reshape the rest, and it did. These are the changes it argues for +at the requirements level rather than the task level. They are recorded here unapproved; the tasks already +merged into group 2 implement what was within the approved requirements, and these are the parts that are not. + +### Amendment A — Req 5 should state what a report may repeat + +**Why.** libpng emitted 198 findings, 163 of them a static pattern asserting that `memcpy` and `strcpy` are +*present* in a library that uses them correctly throughout. Req 5 says the output must be one a contributor +can act on; it does not say anything about volume, so nothing in the spec was violated by a report no +contributor would read. + +**Proposed criterion.** *The report shall be proportionate to the evidence behind a finding: a rule whose only +evidence is a text match shall be summarised once it exceeds a stated number of sites, and a finding a +proposer reasoned about shall keep its detail whatever its rung. Nothing shall be dropped from +`findings.json`, which remains the record a baseline is compared against.* + +Implemented in task 2.11 for pattern matches. **Not implemented, and the reason this is an amendment rather +than a closed task:** the same argument applies to the model layer's own suspicion band, which is 38 of vampi's +60 findings — an enumerated candidate that the graph could not decide and an LLM affirmed. That is the same +epistemic class as a pattern match with an opinion attached, and treating it as a reasoned claim is what +leaves 51 sections on a 520-line application. + +### Amendment B — a family that cannot decide a class shall say so + +**Why.** `c/memory` produced zero true positives across an 89k-line C library and three distinct classes of +false one. The reason is structural: it models string functions, and libpng overflows through arithmetic. A +silent scan currently reads as a clean bill of health, and the stated goal is that an LLM will help a +developer dive in — so it will fluently explain a silence it has no basis for. + +**Proposed criterion.** *Where a family's abstraction cannot decide a weakness class, the system shall state +the limit in the report rather than be silent about it, and no scan shall present the absence of findings as +evidence of absence.* + +This is worse than noise because it is quiet, and it is the one failure mode the rung ladder does not already +guard against. + +### Amendment C — Req 4 should require a checkout that can produce a true positive + +**Why.** Req 4.1 asks for "at least one known-vulnerable repository checkout at a vulnerable commit", and +libpng satisfies it while being incapable of demonstrating detection: its CVE is out of family, and no +version of it can be otherwise, because the library never calls a sink any family models. The requirement is +met and the intent is not. + +**Proposed criterion.** *At least one pinned checkout shall carry a known vulnerability that a family can +decide in principle, so that a scan producing nothing is distinguishable from an engine that cannot see.* + +vampi satisfies this for Python. Nothing satisfies it for C, and 2.15 exists to find out whether anything can. + +### What group 2 confirmed rather than changed + +* **Req 8's overfitting warning earned its place twice.** It stopped a rule being extended until libpng was + clean (2.13), and it forced the access-control precision work to be measured on pairs nobody tuned against + (2.6), where the leak fell 42.9% → 0.0% with pair-correct unchanged. +* **Req 3.3's honesty counters were the load-bearing part.** `regions_unjudged` is what made a budget that + stopped free work visible at all. +* **Req 11.2's byte-identical gates held through every one of nine defect fixes** — which says the same thing + from the other side: the corpus could not have caught any of them. diff --git a/.kiro/specs/contributor-scan/spec.json b/.kiro/specs/contributor-scan/spec.json index e0fc92b..81b0d59 100644 --- a/.kiro/specs/contributor-scan/spec.json +++ b/.kiro/specs/contributor-scan/spec.json @@ -1,7 +1,7 @@ { "feature_name": "contributor-scan", "created_at": "2026-09-08T20:00:00Z", - "updated_at": "2026-09-09T09:30:00Z", + "updated_at": "2026-09-09T12:00:00Z", "language": "en", "phase": "implementation", "approvals": { @@ -22,5 +22,5 @@ "depends_on": [ "model-grounded-detection" ], - "approval_note": "2026-09-08 brief drafted after the maintainer asked what the spec state was and whether anything was forgotten. It was: model-grounded-detection is 16/16 complete and its arbiter is measured at 86.7% pair-correct / 6.7% leak, but the model layer has ZERO production callers -- ousast scan does not use it, so the feature is complete and unshipped at once. That is a scope gap in the predecessor (it had no task saying 'make this reachable', and its byte-identical-gates requirement is satisfied precisely because nothing is wired). This spec covers scan integration, whole-repo scanning against known vulnerable repositories, the deferred memory-family decision, and records the pipeline, ops and corpus work delivered outside any spec. Not approved; requirements not generated. ; 2026-09-08 requirements generated at the maintainer's direction ('we follow the spec and build integration afterwards'). ; 2026-09-09 requirements approved and design generated ('let us continue with these'): the model layer joins as one stage inside MAP beside the obligations checker, one CPG per scan, regions from entry points, a total budget rather than a per-region cap. Phase 2 (the first run on a pinned real repository) can invalidate the rest and comes before shipping. ; 2026-09-09 tasks approved by the maintainer ('approve the tasks and start group 1'); implementing group 1 (regions, driver, MAP stage)." + "approval_note": "2026-09-08 brief drafted after the maintainer asked what the spec state was and whether anything was forgotten. It was: model-grounded-detection is 16/16 complete and its arbiter is measured at 86.7% pair-correct / 6.7% leak, but the model layer has ZERO production callers -- ousast scan does not use it, so the feature is complete and unshipped at once. That is a scope gap in the predecessor (it had no task saying 'make this reachable', and its byte-identical-gates requirement is satisfied precisely because nothing is wired). This spec covers scan integration, whole-repo scanning against known vulnerable repositories, the deferred memory-family decision, and records the pipeline, ops and corpus work delivered outside any spec. Not approved; requirements not generated. ; 2026-09-08 requirements generated at the maintainer's direction ('we follow the spec and build integration afterwards'). ; 2026-09-09 requirements approved and design generated ('let us continue with these'): the model layer joins as one stage inside MAP beside the obligations checker, one CPG per scan, regions from entry points, a total budget rather than a per-region cap. Phase 2 (the first run on a pinned real repository) can invalidate the rest and comes before shipping. ; 2026-09-09 tasks approved by the maintainer ('approve the tasks and start group 1'); implementing group 1 (regions, driver, MAP stage). ; 2026-09-09 group 2 complete except 2.8/2.9/2.13-2.16, and it reshaped the phase as intended. Nine engine defects found by measurement and none by review, gates byte-identical through every fix -- which says the pair corpus could not have caught any of them. Three requirements amendments are DRAFTED AND UNAPPROVED at the end of requirements.md: (A) Req 5 should state what a report may repeat, because libpng emitted 198 findings with nothing in the spec forbidding it, and the model layer's own suspicion band is still 38 of vampi's 60; (B) a family that cannot decide a class must say so, because c/memory scored zero true positives on 89k lines of C for a structural reason and a silent scan reads as a clean bill of health; (C) Req 4.1 is satisfied by a checkout that cannot demonstrate detection at all, so it should require one that can. Separately, two task groups appended to tasks.md without passing requirements or design have been LIFTED OUT to the new finding-feedback-loop spec, where they belong." } diff --git a/.kiro/specs/contributor-scan/tasks.md b/.kiro/specs/contributor-scan/tasks.md index 2efc3d1..07ea3c5 100644 --- a/.kiro/specs/contributor-scan/tasks.md +++ b/.kiro/specs/contributor-scan/tasks.md @@ -411,131 +411,15 @@ - Group 5 is deliberately last. The corpus is not the binding constraint today — reachability is — and 198 of 238 OWASP pairs were left unvendored precisely because volume without readability makes every number worse. -## Group 7 — Stop repeating the same false positive +--- -Three false-positive classes were found by hand today, and two of them were the *same defect in a different -matcher*: `resolve` matched inside `resolveUrl`, `user` inside `users`, `gets` inside `fgets`. Each was fixed -locally, and each next one survived because nobody carried the fix across. That is the thing to automate -- -not the judgement, the propagation. +## Lifted out of this spec (2026-09-09) -The constraint that shapes every task here: **no LLM-authored patterns or policy.** A model may propose a -hypothesis and draft a change; only a deterministic measurement may adopt one. And any loop that learns from -the corpus must respect the train/holdout split, because the last one did not: five of eleven candidate -shapes were taught by the holdout pairs they then "recovered", turning a -0.059 Youden into +0.059. +Two groups drafted here -- a counter-example ledger, and per-repository evolutionary prompting so developers +can commit dismissals and evolve the harness -- were appended directly to this file without passing +requirements or design. That was a workflow error and they are a separate feature, not a refinement of scan +integration. They now live in `.kiro/specs/finding-feedback-loop/`, at brief, unapproved. -- [ ] 7.1 One matcher, asserted to be the only one - - The three bugs above existed because three modules each grew their own token-matching rule. There is now - one bounded matcher in `taint.sc` and one in `dominance.sc`, still separate copies of the same logic. - - Make it one definition the queries share, and add a test that fails when a second appears -- a grep-level - invariant is fine and is what would have caught the third instance. - - Observable: a test that fails if any query matches a spec token with a bare `contains` or `startsWith`. - - _Requirements: 8.1_ - -- [ ] 7.2 A counter-example ledger - - When a finding is contradicted -- by a maintainer, by the judge with coverage, or by a declared contract - like an OpenAPI `security` block -- record it: the site, the rung, the witness, the spec tokens that - produced it, and the commit. That artifact is the durable part; today's three fixes left no trace a - future regression could be checked against. - - Each entry becomes a regression case: this site, at this commit, must not be reported at that rung. - `fgets(buf, 256, f)` and VAmPI's `register_user` are the first two. - - Observable: the ledger is committed, the cases run in CI offline, and re-introducing the substring match - fails a test rather than a repository scan four weeks later. - - _Requirements: 8.1, 10.3_ - - _Depends: 2.9_ - -- [ ] 7.3 Group the ledger by cause, and let a model propose the cause - - Twenty-six identical findings were one defect; three separate bugs were one bug class. A deterministic - pass should group counter-examples by what they share -- same spec token, same matcher clause, same - family, same witness shape -- and report the groups by size. - - **This is where the model earns its place, and only here.** Given a group, ask it for a hypothesis: what - do these share, and what change would remove all of them? It drafts; it does not decide. The output is a - proposed fact-table or matcher edit with a rationale, written to a review queue -- never to the ruleset, - and never to the declared policy file. - - Observable: run it against today's three fixes with the fixes reverted, and record how many it groups - correctly and what it proposes. A model that cannot rediscover a known cause should not be trusted with - an unknown one. - - _Requirements: 8.1, 8.4_ - - _Depends: 7.2_ - -- [ ] 7.4 Adopt only what a split-respecting measurement confirms - - A proposed edit is adopted only if it removes its counter-examples AND does not lose pair-corpus recall, - measured on the **train** split with the holdout untouched. The predecessor's improve lever admitted a - shape because it "recovers a currently missed holdout pair", which is how a -0.059 Youden was reported - as +0.059. - - The gates already exist and must stay byte-identical across an adoption, or the adoption is a - regression wearing a fix's clothes. - - Observable: an adoption record per accepted edit -- counter-examples removed, train recall before and - after, holdout untouched and unread, gates identical. - - _Requirements: 8.1, 8.2, 10.2, 10.3_ - - _Depends: 7.3_ - -- [ ] 7.5 Report what the loop cost as well as what it bought - - A false-positive reducer that quietly trades recall is worse than none, and the trade is invisible unless - it is measured on both sides. Today's 2.6 removed three leaks and lost no correct detection; that is the - shape of an acceptable trade and it was only knowable because both numbers were taken. - - Observable: every adoption records precision and recall on both repositories and the pair corpus, and a - net-negative adoption is reverted automatically rather than argued about. - - _Requirements: 8.2, 10.3_ - - _Depends: 7.4_ - -## Group 8 — Per-repository state a developer owns - -> "an evolution (gepa, simba) style prompting with a declarative and persistent approach when evaluating -> findings per target repo? so that devs can commit state and evolve the harness / dismiss findings" - -Yes, and Group 7 is half of it already -- 7.2's counter-example ledger is the persistent state, 7.3 is a model -proposing from it, 7.4 is a split-respecting adoption. What this adds is the part that makes it a developer's -rather than a maintainer's: the state lives **in the target repository**, committed beside the code, and it is -declarative. - -**The boundary that decides the design.** A reflective optimiser may evolve a **prompt**, because a prompt is -how we ask a question. It may not author **patterns or policy** -- the fact tables and the rulesets stay -human-authored, and the tool never writes the declared policy file. So GEPA-style evolution has exactly one -legitimate target here: the judge and residual questions in `model/pipeline.py`, which are already the -LLM-facing surface. Evolving those against a repository's own dismissals is sound. Evolving sink tables from -them is the thing this project has refused from the start, and the closed-loop leak is why it is right to. - -- [ ] 8.1 A dismissal file the developer owns - - `.openultrasast/dismissals.toml` in the TARGET repository: finding id or site, the rung it was dismissed - at, a reason, and who. Committed with the code, reviewed like code, and diffable -- a dismissal without a - reason is how a suppression file becomes a graveyard nobody can audit. - - The tool READS it and never writes it, which is the same rule the declared policy file already follows. - `ousast scan` reports dismissed findings as dismissed with their reason rather than hiding them, so a - stale dismissal is visible rather than silent. - - Observable: a scan of vampi with two dismissals reports 3 findings and 2 dismissed-with-reason, and the - file is untouched by the run. - - _Requirements: 5.1, 5.2_ - -- [ ] 8.2 Dismissals are an evaluation set, per repository - - A dismissal is a labelled example: this site, at this rung, is not a finding here. Together with the - known vulnerabilities a `benchmarks/repos/` recipe already declares, that is a per-repository eval set - with both classes in it -- which is exactly what an optimiser needs and what a suppression list normally - throws away. - - Split it the way the pair corpus is split. A dismissal that teaches the prompt that recovers it is the - leak measured on 2026-09-05, where five of eleven candidate shapes were taught by the holdout pairs they - then recovered and a -0.059 Youden was reported as +0.059. - - Observable: the eval set builds from a recipe plus a dismissal file, with a declared train/holdout split - and a test that the holdout is never read during optimisation. - - _Requirements: 8.2, 10.2_ - - _Depends: 8.1_ - -- [ ] 8.3 Evolve the QUESTION, never the facts - - GEPA/SIMBA-style reflective evolution over `CANDIDATE_QUESTION` and `residual_question`, scored on 8.2's - train split: fewer dismissed sites affirmed, no known vulnerability lost. - - Hard boundaries, and they are the point of putting this in its own group: the optimiser proposes prompt - text only; it never edits a fact table, a ruleset or the declared policy file; an evolved prompt is - committed by a human like any other change; and the three gates stay byte-identical or the evolution is - a regression wearing a fix's clothes. - - Observable: an evolution record per accepted prompt -- train score before and after, holdout untouched, - known vulnerabilities still found, gates identical, and the diff of the prompt. - - _Requirements: 8.1, 8.2, 10.3_ - - _Depends: 8.2, 7.4_ - -- [ ] 8.4 Say what the harness learned, in the repository it learned it in - - Per-repository state is only useful if a developer can see what it did. A committed record of which - dismissals shaped the current prompt, and what that cost on the shared corpus, is what keeps this from - becoming a per-repo tuning that nobody can reason about. - - Observable: `ousast repos` or the report shows, for a target with evolved state, what was learned and - what it was measured against. - - _Requirements: 5.2, 10.4_ - - _Depends: 8.3_ +Three requirements amendments this group argues for are drafted at the end of `requirements.md`, also +unapproved: what a report may repeat, a family stating a class it cannot decide, and Req 4.1 requiring a +checkout that can actually demonstrate detection. diff --git a/.kiro/specs/finding-feedback-loop/brief.md b/.kiro/specs/finding-feedback-loop/brief.md new file mode 100644 index 0000000..b5867fc --- /dev/null +++ b/.kiro/specs/finding-feedback-loop/brief.md @@ -0,0 +1,88 @@ +# Brief: finding-feedback-loop — what happens after a finding is judged wrong + +**Status:** brief only, unapproved. Successor scope lifted out of `contributor-scan`, which is mid-implementation +and does not cover this. + +## Why this is its own spec + +Two task groups were drafted into `contributor-scan/tasks.md` on 2026-09-09 without passing through +requirements or design. That was a workflow error: they are not refinements of scan integration, they are a +different feature — a loop that learns from findings a human rejected — and writing them as tasks skipped the +two phases that decide whether the feature is shaped right. They are lifted here to be specified properly. + +## The gap this exists to close + +Nine engine defects were found in one session against two repositories. **Three of them were the same bug in +three different matchers**: a spec token matched as a substring rather than as a word — `resolve` inside +`resolveUrl` in the sanitizer matcher, `user` inside `users` in the discharger matcher, `gets` inside `fgets` +in the sink matcher. Each was found by a measurement, fixed locally, and left no trace. The third survived +until a 89k-line C library made it visible as 26 identical false positives. + +Nothing in the system remembers that a finding was wrong. There is no artifact recording *this site, at this +rung, is not a defect, and here is why*, so the same class recurs and each recurrence costs another +measurement to find. + +Two audiences want that artifact for different reasons: + +* **A maintainer** wants the propagation automated. Two of the three matcher bugs needed no judgement at all, + only the fix carried across. +* **A developer** wants to dismiss a finding in their own repository, have it stay dismissed, and have the + reason reviewable beside the code. Today they have no way to say so, and a tool that cannot be told it is + wrong is one they stop running. + +## What the maintainer asked for + +> "is there a way for an evolution (gepa, simba) style prompting with a declarative and persistent approach +> when evaluating findings per target repo? so that devs can commit state and evolve the harness / dismiss +> findings" + +And, as the standing test for all of it: + +> "it must find the bugs, otherwise it's SAST noise. most devs know codeql / semgrep have dataflow +> capabilities but never use it" + +## The boundary that shapes the design + +This project has refused LLM-authored patterns and policy from the start, and a closed-loop leak measured on +2026-09-05 is why that is not negotiable: the improve lever admitted candidate shapes because they "recover a +currently missed holdout pair", five of eleven were taught by the holdout pairs they then recovered, and a +holdout Youden of **−0.059 was reported as +0.059**. + +So a reflective optimiser has exactly one legitimate target here: + +| may be evolved | may not be evolved | +| --- | --- | +| the judge and residual **questions** — how we ask | fact tables, rulesets, the declared policy file — what we claim | + +A prompt is how a question is put; a fact table is an assertion about the world. GEPA/SIMBA-style evolution +over `CANDIDATE_QUESTION` and `residual_question`, scored against a repository's own labelled dismissals, is +sound. The same machinery writing sink tables is the thing that produced the leak. + +## What the shape looks like + +* **Per-repository, declarative, committed by the developer.** A dismissal file in the *target* repository — + site, rung, reason, author — read by the tool and never written by it, the same rule the declared policy + file already follows. Reviewed like code, diffable, and a dismissal without a reason is how a suppression + list becomes a graveyard nobody can audit. +* **Dismissals are labels, not suppressions.** With the known vulnerabilities a `benchmarks/repos/` recipe + already declares, a dismissal file gives a per-repository evaluation set with *both* classes in it — which + is what an optimiser needs and what a suppression list normally throws away. +* **A ledger for the maintainer.** Counter-examples grouped by what they share (same token, same matcher + clause, same witness shape), so a recurring cause is visible as a cause rather than as five separate bugs. +* **Split-respecting adoption.** Train split only, holdout never read, the three gates byte-identical, and + precision *and* recall reported on both sides — an adoption that quietly trades recall is worse than none. + +## The acceptance test for the model's role + +Revert the three matcher fixes and run the grouping: does it rediscover the known causes? A model that cannot +recover a cause we already know should not be trusted to propose one we do not. + +## Known limits to carry forward + +* The pair corpus could not see any of the nine defects; whole-repository measurement is what found them, and + this loop inherits that. Its evaluation set has to include repositories, not only pairs. +* Only two repositories exist today (one Python, one C), and the C one has never produced a true positive. + An optimiser tuned on that evidence base would be tuned on very little. +* `contributor-scan` Req 8 already warns about the +16% overfitting gap between the slice a change was + developed against and one nobody tuned against. Per-repository evolution makes that risk structural rather + than incidental. diff --git a/.kiro/specs/finding-feedback-loop/spec.json b/.kiro/specs/finding-feedback-loop/spec.json new file mode 100644 index 0000000..91296ee --- /dev/null +++ b/.kiro/specs/finding-feedback-loop/spec.json @@ -0,0 +1,15 @@ +{ + "feature_name": "finding-feedback-loop", + "created_at": "2026-09-09T12:00:00Z", + "updated_at": "2026-09-09T12:00:00Z", + "language": "en", + "phase": "brief", + "approvals": { + "requirements": { "generated": false, "approved": false }, + "design": { "generated": false, "approved": false }, + "tasks": { "generated": false, "approved": false } + }, + "ready_for_implementation": false, + "depends_on": ["contributor-scan"], + "approval_note": "2026-09-09 brief drafted. Scope lifted out of contributor-scan, where two task groups (a counter-example ledger and per-repository evolutionary prompting) had been appended directly to tasks.md without passing requirements or design -- a workflow error, and they are a separate feature rather than a refinement of scan integration. Prompted by the maintainer asking for GEPA/SIMBA-style evolution with declarative persistent per-repository state so developers can commit dismissals and evolve the harness. The design boundary is that an optimiser may evolve the QUESTION (judge and residual prompts) and never the FACTS (tables, rulesets, declared policy), because the closed-loop leak measured 2026-09-05 turned a -0.059 holdout Youden into +0.059 by teaching from the pairs it then recovered. Not approved; requirements not generated." +}