test(gates): repo-shaped acceptance suite — scope matrix, exit-code semantics, coverage ratchet - #359
Merged
Merged
Conversation
…emantics, coverage ratchet
We keep proving gates work by hand, one planted true positive at a time, in
every app. This builds the suite instead.
WHY UNIT TESTS ARE NOT ENOUGH
-----------------------------
gate-7 has 86 unit tests. All 86 passed. The gate was still wrong: its regex
was self-consistent, and what it could not see was a real repository spelling
its auth predicates verb-object (`canUserAccessAgent`). Three more from the
same sweep are invisible to a unit test by construction — gate-61 is a SCOPE
bug, gate-4's INPUT was hidden by .gitignore, and gate-19 returned its finding
count as an exit STATUS (a byte holds 255, so 256 findings would have exited
0 = PASS). None of those live in a checker function; they live in the wrapper,
the filesystem, and the process boundary.
WHAT LANDS
----------
Five suites, all auto-discovered by tests/run-helper-suites.sh (no workflow
edit — that is the point of discovery):
test_gate_acceptance_matrix.sh generalises never-green-over-nothing. Each
bundle declares planted/clean expectations
in expect.conf; the planted arm must FAIL
**and NAME the planted subject**, the clean
arm must PASS. NOT APPLICABLE over a fixture
built to trigger the gate is a suite FAILURE.
test_gate_scope_matrix.sh the same tree at push / full / diff scope.
test_gate_exit_code_semantics.sh a byte holds 255.
test_gate19_coverage_credibility.sh a tag is not a test.
test_gate7_verb_object_guards.sh .github#353, with an anti-dead-test control.
THE COVERAGE RATCHET
--------------------
Coverage is COMPUTED from the bundles and diffed against the runner's own
declared inventory — never a hand-maintained list, which is the failure mode
this package has already been bitten by twice. Every declared gate must be
either fixtured or carry a reasoned row in UNCOVERED.md (52 gates, each with a
category: no-fixture-yet / needs-diff / needs-external / advisory-only).
The list can only shrink: a listed gate that gains a fixture is a hard failure,
and so is a declared gate in neither place. 12 of 64 gates now have
planted/clean coverage driven through the real bin/hydra-gates.
DEFECTS REPRODUCED AND PINNED (each asserted as a live KNOWN DEFECT, which
fails loudly the moment the behaviour changes, so no entry can rot):
.github#347 gate-61 reproduced from scratch in a synthetic repo. Tree held
constant, only the scope flag changed: `--all` finds and names
the listener; a diff that touches it FAILs; `--full` reports
NOT APPLICABLE citing "the diff against origin/development", on
a run whose own preamble says "Base ref: n/a — --full requested".
.github#356 ONE `@e2e exclude` in a requirement body exempted all THREE
scenarios beneath it; the gate printed "PASS — 0 reference(s) in
e2e suite". Scenario-level exclusion is fixtured as a control, so
a fix that over-corrects is caught too.
.github#343 a file-level `@e2e` tag credited 2 scenarios in a file whose only
test is `test.skip(true, …)`. The honest arm and the counterfeit
arm emit BYTE-IDENTICAL verdicts — that is the finding.
.github#209 repaired in the MESSAGE, not the VERDICT: the count is parsed
from stdout, but PASS/FAIL still branches on $?. Latent (no
shipped checker returns a count) and now pinned by a general
invariant: no gate may report PASS while its own log states FAIL.
Also generalised, gate-agnostically: a NOT APPLICABLE that claims a diff
EXCLUDED something is invalid on a run that computed no diff. Gates 29/47/48
decline honestly on --full and are asserted as the good pattern; gates 6/7/8/9
give a right verdict with a wrong reason and are recorded.
Nothing was weakened to make this green, and no held PR was merged.
…SC2126 The ShellCheck job failed on four `note:` findings. The action itself reported success; a wrapper step then runs `exit 1` on ANY finding, severity included. Three were SC1091 "Not following: ./gate_fixture_support.sh was not specified as input". The `# shellcheck source=` directives were already present and correct -- they simply had no effect, because external sourcing was never enabled. `.shellcheckrc` now sets `external-sources=true`, which turns those directives on rather than suppressing the diagnostic, so a genuinely missing or misspelled helper path is still reported. The shellcheck workflow already lists `.shellcheckrc` in its `paths:` trigger, so the file was expected to exist and did not. The fourth was SC2126 (`grep|wc -l` -> `grep -c`). Taking the naive suggestion would have changed behaviour: `grep -c` prints 0 but exits 1 when nothing matches, where `| wc -l` exits 0. Verified both arms: match: OLD val=2 rc=0 | NEW val=2 rc=0 no match: OLD val=0 rc=0 | NEW val=0 rc=0 `|| true` restores the original exit status exactly. This matters here because the surrounding assertion is precisely the one whose own comment records that stderr noise can pass while the assertion still prints PASS. Suite re-run after the change: 9 passed, 0 failed, 1 known live defect.
external-sources=true made ShellCheck follow the `# shellcheck source=`
directives, and the diagnostic changed rather than disappearing:
before: Not following: ./gate_fixture_support.sh was not specified as
input (see shellcheck -x)
after: Not following: ./gate_fixture_support.sh: openBinaryFile: does
not exist (No such file or directory)
That second message is progress, not a regression -- it means the
directive was being honoured, but resolved against the process working
directory. The CI action invokes shellcheck from the repository root, so
`./gate_fixture_support.sh` resolved to <repo-root>/gate_fixture_support.sh.
source-path=SCRIPTDIR resolves the directive relative to the directory of
the script carrying it, which keeps the existing directives correct
whether a script is run from the repo root, from its own directory, or by
the suite runner.
SC2126 is already resolved by the previous commit and no longer appears.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 11, 2026
Closes #361. gate-16 is diff-scoped by design (ADR-020). On a full-repo run there is no diff, so the runner's unguarded BASE_REF let the checker fall back to origin/development -- which on development diffs the branch against itself. Result: '# count=0' -> PASS over a scope nothing was read from. Worse than the sibling #347: gate-61's NOT APPLICABLE is excluded from the verdict, but gate-16's PASS COUNTS toward 'N of N applicable gates ran'. It does not hide a number, it falsifies a verdict. Caught live on openregister#2422, where gates 19/25/26 printed NOT APPLICABLE and gate-16 printed PASS four lines apart over the same empty scope. Measured in eight repos, one tree each, changing only this input: pipelinq 0 / 185 (beta) / 1466 (report) shillinq 0 / 808 (empty-tree base) docudesk 0 / 290 (beta) / 471 openregister 0 / 232 (beta) / 234 decidesk 0 / 5 (beta) / 91 procest 0 / 113 (beta) / 445 larpingapp 0 / 0 / 0 <- genuinely clean, committed plant fired portaliq 0 / 0 (beta) / 2 (main) / 5 (report) zaakafhandel 0 / 0 / 0 / 0 <- 551 @SPEC tags behind it The three zeros are EARNED, not fictional -- each proven with a committed plant that moved every arm by exactly +1 and named itself. So this is not a 'every repo hides debt' claim. DESIGN The checker now ALWAYS runs, on every scope, and the two conditions are evaluated in the order that keeps them distinguishable: 1. WIRING -- did it finish? (no terminal '# count=' marker -> SKIPPED) 2. SCOPE -- is its answer meaningful? (full run -> NOT APPLICABLE) A crash must never be reported as an empty scope, and an empty scope must never be reported as a pass. Deliberately NOT a literal copy of gate-19's else-branch: gate-19 falls back to a FULL SWEEP, which for gate-16 would flag the entire legacy @SPEC surface -- the wrong contract per ADR-020, and a false RED in every repo. TWO ERRORS THIS PR MADE, BOTH CAUGHT BY CONTROLS - The first draft passed 'scope' to _skip. Anything outside na|structural|wiring is treated as an internal-error FAIL -- it would have produced exactly the false RED this change prevents. - The first draft also skipped the invocation entirely on a full run, so a CRASHED checker became invisible at full scope. The package's own test_gate_crashed_checker_is_not_a_finding.sh caught it: FAIL - gate-16 spec-coverage: verdict is 'NOT' - expected SKIPPED (wiring) That test now passes again, with the wiring branch reached first. That the acceptance suite (#359) caught a regression in a fix authored by the same programme is the strongest available evidence it works.
rubenvdlinde
pushed a commit
that referenced
this pull request
Aug 12, 2026
…t can reach is public" The second of gate-7's two blinding exemptions. `#[PublicPage]` was skipped unconditionally, commented "intentionally open to unauthenticated callers" — a claim about the CALLER, where the gate's question is about the OBJECT. It carried no stated reason, so there was nothing to audit. REPRODUCED at package 57bcb2b on a nine-arm rig, one file, one commit, one run: a byte-identical IDOR plant carrying `#[PublicPage]` — including an UNAUTHENTICATED WRITE to an arbitrary `$id` — reported PASS. AND THE EXEMPTION IS THE SMALLER HALF. Deleting it and changing nothing else moves only the arm that carries BOTH attributes. A `#[PublicPage]`-only method never reaches it: the `_NO_ADMIN_RE` scope filter drops it one branch earlier. Across the fleet's lib/Controller trees that is 267 of 357 public methods — 74% — invisible to any edit of the exemption. So the scope filter widens too. It is demonstrably not safe by construction. opencatalogi#856, merged as 963f832a, was exactly this shape: `GET /api/themes/{id}` was `@PublicPage` and its `find()` carried no register and no schema, so it resolved the caller's identifier in ANY register on the instance and served an anonymous caller a municipal `zaak` status record. THE PREDICATE IS NOT THE `#[NoAdminRequired]` ONE. There is no current user to compare an owner against, so ownership guards do not transfer. A public handler must CONSTRAIN THE LOOKUP TO A SCOPE IT DECLARES PUBLIC rather than resolve the identifier globally. Five clears, each derived from a shape read in a fleet app: a non-caller argument that carries a value; a callee declaring publicness as a complete CamelCase segment; a receiver already scoped as state; a container resolution whose result constrains a later lookup and whose miss is refused; and one resolved delegation hop. Plus a publicness or scope predicate answered with a refusal, per selector rather than per call. TWO-DIRECTIONAL CONTROL, on the real endpoint rather than only the rig: opencatalogi `ThemesController::show` is FLAGGED at f6e86ab and SILENT at 963f832 — same file, same method, same checker, the security fix in between. ALSO HERE, and load-bearing: a helper was guard-bearing if its body matched a bare `throw`, matched against source WITH COMMENTS IN IT. hermiq's `loadOwnedAgent()` contains no throw statement at all — the word appears only in a comment explaining why it CATCHES one — and that cleared all four routed methods calling it, and cleared opencatalogi's `getObjectService()` for every caller in the app, which is why Pattern 8 could not see the endpoint that motivates it. The body test now reads comment-free text and a `throw` must name an authorisation exception; an ownership comparison answered with `null` is recognised instead, so no true clear was lost (hermiq stays at 20, doriath at 5). `TemplateResponse` no longer clears a `@PublicPage` method: its stated reason is that NC guarantees a session, and `@PublicPage` is the annotation that turns that guarantee off. Findings get their own rule name, `publicpage-unscoped-object-lookup`, because the remedy differs. `@no-admin-idor-exempt <reason>` still exempts — a reason-bearing exemption replacing a reasonless blanket one. BLAST RADIUS, 18 canonical clones, identical file lists: 307 -> 315. Six `publicpage-unscoped-object-lookup` (openregister 3, pipelinq 1, portaliq 1, softwarecatalog 1) and two from the helper-evidence repair (opencatalogi 1, launchpad 1). doriath — the app that gets scoping right — stays at 5. hermiq stays at 20. Tests: 113 -> 130 unit tests, 9 of the 17 new ones red against the pre-fix checker. New repo-shaped acceptance bundle `publicpage-scope` (#359), verified to go RED on `method=arbitraryId` against the pre-fix checker, so it proves the fix rather than merely passing.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 12, 2026
) (#373) * fix(gate-7): an authentication check is not an authorisation guard (#365) gate-7 reported 0 in ALL EIGHTEEN fleet apps. Every one of those zeros was produced by a checker that accepts "is anyone logged in?" as an answer to "may THIS caller touch THIS object?". Reproduced with a three-arm committed-plant control — same file, same commit, same run, byte-identical data-access bodies, only the preamble differing: bare unguarded method 1 finding + a no-user 401 preamble, nothing else 0 findings + a real per-object ownership check 0 findings (correct) Scale at the canonical package 57bcb2b: 0 reported fleet-wide while 453 of 791 controller files carry that preamble. Positive-controlled in all 18 apps by planting a bare IDOR into each extracted tree: the checker went 0 to 1 in every one, so each zero is a measurement and not an empty scope. Under NoAdminRequired the framework has already rejected the anonymous caller before the method runs, so the preamble cannot even fail. It was nonetheless silencing the gate through three guard regexes at once. THE FIX IS NOT "DROP 401 FROM THE REGEX", which is what the issue proposes. That repair is wrong in both directions, and both are now pinned by tests: too wide - a real ownership comparison that answers 401 instead of 403 is a genuine guard, and deleting the token makes it a false positive. False positives are how gate-7 lost its credibility and therefore how its silences came to be believed. too narrow - the same authentication clause answering 403 keeps clearing. One token of edit, made by someone chasing a green cell, buys the silence back by making the response worse. So authentication-ness is treated as a property of the CONDITION, not of the status code. An if whose condition tests only whether a caller identity is absent, and whose consequent refuses, is blanked to same-length whitespace before any guard pattern runs. Offsets, spans and line numbers are unchanged. Three controls keep the blanking narrow: absence polarity only, so a presence test wrapping a whole method body is never eaten; argument-free operands only, so a guard call and object data are never read as an identity; and the consequent must refuse. Two companion patterns ship in the same change because removing the clear is what EXPOSES them, and shipping without them would have manufactured 45 false positives in doriath alone - an app whose real gate-7 exposure is zero: Pattern 6 session-identity hand-off. Every call receiving a caller-supplied value must also receive a session-derived identity, so the object reference is resolved under a scope the caller cannot forge. An ALL-quantifier, not ANY: one unscoped data call still reports. Pattern 7 in-body ownership comparison, whatever status it answers with, including the deliberate 404 chosen so a 403 cannot become an existence oracle. That sentence was already in this gate's own FAIL message; it was advice to a human and is now code. Fleet effect, measured over canonical development clones: 0 becomes 327 findings across 18 apps. Pattern 6 suppresses 86 and Pattern 7 suppresses 2 of what the naive repair would have reported. The audit's hand-verified real findings all survive: shillinq arbitrary-file-read download, pipelinq semantic handoff, docudesk cross-register preview, decidesk motion history, opencatalogi unscoped search, zaakafhandelapp object routes. Coverage: a new authn-vs-authz acceptance bundle drives the control through the real wrapper. Both arms KEEP the 401 preamble, so the clean arm pins that the preamble is ignored rather than punished. Its expect.conf names method=preamble rather than the file, because the bare arm would name the file under the old checker too - verified by running the bundle against the pre-fix checker, where it goes red on exactly that assertion. Four unit tests that encoded the defect are inverted with the reason recorded in each; one of them, the decidesk citizenAction case, sits beside staffAction in the same collaborator and only staffAction is an authorisation guard. 113 unit tests pass, 74 helper suites pass, 61 entry-point tests pass. The one red suite, test_gate_45_to_55_acceptance.sh, fails identically on the unmodified canonical clone: ajv is not resolvable in this environment. * fix(gate-7): "may be called without a session" is not "every object it can reach is public" The second of gate-7's two blinding exemptions. `#[PublicPage]` was skipped unconditionally, commented "intentionally open to unauthenticated callers" — a claim about the CALLER, where the gate's question is about the OBJECT. It carried no stated reason, so there was nothing to audit. REPRODUCED at package 57bcb2b on a nine-arm rig, one file, one commit, one run: a byte-identical IDOR plant carrying `#[PublicPage]` — including an UNAUTHENTICATED WRITE to an arbitrary `$id` — reported PASS. AND THE EXEMPTION IS THE SMALLER HALF. Deleting it and changing nothing else moves only the arm that carries BOTH attributes. A `#[PublicPage]`-only method never reaches it: the `_NO_ADMIN_RE` scope filter drops it one branch earlier. Across the fleet's lib/Controller trees that is 267 of 357 public methods — 74% — invisible to any edit of the exemption. So the scope filter widens too. It is demonstrably not safe by construction. opencatalogi#856, merged as 963f832a, was exactly this shape: `GET /api/themes/{id}` was `@PublicPage` and its `find()` carried no register and no schema, so it resolved the caller's identifier in ANY register on the instance and served an anonymous caller a municipal `zaak` status record. THE PREDICATE IS NOT THE `#[NoAdminRequired]` ONE. There is no current user to compare an owner against, so ownership guards do not transfer. A public handler must CONSTRAIN THE LOOKUP TO A SCOPE IT DECLARES PUBLIC rather than resolve the identifier globally. Five clears, each derived from a shape read in a fleet app: a non-caller argument that carries a value; a callee declaring publicness as a complete CamelCase segment; a receiver already scoped as state; a container resolution whose result constrains a later lookup and whose miss is refused; and one resolved delegation hop. Plus a publicness or scope predicate answered with a refusal, per selector rather than per call. TWO-DIRECTIONAL CONTROL, on the real endpoint rather than only the rig: opencatalogi `ThemesController::show` is FLAGGED at f6e86ab and SILENT at 963f832 — same file, same method, same checker, the security fix in between. ALSO HERE, and load-bearing: a helper was guard-bearing if its body matched a bare `throw`, matched against source WITH COMMENTS IN IT. hermiq's `loadOwnedAgent()` contains no throw statement at all — the word appears only in a comment explaining why it CATCHES one — and that cleared all four routed methods calling it, and cleared opencatalogi's `getObjectService()` for every caller in the app, which is why Pattern 8 could not see the endpoint that motivates it. The body test now reads comment-free text and a `throw` must name an authorisation exception; an ownership comparison answered with `null` is recognised instead, so no true clear was lost (hermiq stays at 20, doriath at 5). `TemplateResponse` no longer clears a `@PublicPage` method: its stated reason is that NC guarantees a session, and `@PublicPage` is the annotation that turns that guarantee off. Findings get their own rule name, `publicpage-unscoped-object-lookup`, because the remedy differs. `@no-admin-idor-exempt <reason>` still exempts — a reason-bearing exemption replacing a reasonless blanket one. BLAST RADIUS, 18 canonical clones, identical file lists: 307 -> 315. Six `publicpage-unscoped-object-lookup` (openregister 3, pipelinq 1, portaliq 1, softwarecatalog 1) and two from the helper-evidence repair (opencatalogi 1, launchpad 1). doriath — the app that gets scoping right — stays at 5. hermiq stays at 20. Tests: 113 -> 130 unit tests, 9 of the 17 new ones red against the pre-fix checker. New repo-shaped acceptance bundle `publicpage-scope` (#359), verified to go RED on `method=arbitraryId` against the pre-fix checker, so it proves the fix rather than merely passing. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
We keep proving gates work by hand, one planted true positive at a time, in every app. This builds the suite instead.
The argument this is built around: gate-7 has 86 unit tests, they all passed, and the gate was still wrong. Its regex was self-consistent; what it could not see was a real repo naming its auth predicates verb-object (
canUserAccessAgent). Only a repo-shaped fixture driven through the real wrapper catches that. Same for gate-61 (a scope bug), gate-4 (.gitignorehid its input), gate-19 (returned its finding count as an exit status — 256 findings would have exited 0 = PASS). None of those live in a checker function.What lands
Five suites, auto-discovered by
tests/run-helper-suites.sh— no workflow edit, which is the point of discovery.test_gate_acceptance_matrix.shNOT APPLICABLEover a fixture built to trigger the gate is a suite FAILUREtest_gate_scope_matrix.shtest_gate_exit_code_semantics.shtest_gate19_coverage_credibility.shtest_gate7_verb_object_guards.sh#353, with an anti-dead-test controlThe coverage ratchet
Coverage is computed from the bundles and diffed against the runner's own declared inventory — never a hand-maintained list, the failure mode this package has already been bitten by twice. Every declared gate is either fixtured or carries a reasoned row in
UNCOVERED.md(52 gates, categorisedno-fixture-yet/needs-diff/needs-external/advisory-only). The list can only shrink. 12 of 64 gates now have planted/clean coverage through the realbin/hydra-gates.Defects reproduced and pinned
Each is asserted as a live
KNOWN DEFECTthat fails loudly the moment the behaviour changes, so no entry can rot into a permanent excuse. None of the held PRs were merged.#347gate-61 — reproduced from scratch in a synthetic repo. Tree constant, only the scope flag changed:--allfinds and names the listener; a diff touching it FAILs;--fullreportsNOT APPLICABLE — the diff against 'origin/development' put every post-event registration out of scope, on a run whose own preamble readsBase ref: n/a — --full requested.#356— ONE@e2e excludein a requirement body exempted all THREE scenarios beneath it:PASS — 0 reference(s) in e2e suite. Scenario-level exclusion is fixtured as a control, so a fix that over-corrects is caught too.#343+ the never-falsetest.skip— a file-level tag credited 2 scenarios in a file whose only test istest.skip(true, …). The honest arm and the counterfeit arm emit byte-identical verdicts. That is the finding.#209— repaired in the message, not the verdict: the count is parsed from stdout, butPASS/FAILstill branches on$?. Latent (no shipped checker returns a count) and now pinned by a general invariant: no gate may report PASS while its own log states FAIL.hasPermission()andcanAccess()guards are still reported as unguarded IDOR, before and after#353. The regex requires a non-empty segment between theis/has/can/mayprefix and the auth token, so the token can never be first. (The#353message states "canAccess matched"; measured against both regexes, it never did.)Generalised gate-agnostically: a
NOT APPLICABLEthat claims a diff EXCLUDED something is invalid on a run that computed no diff. Gates 29/47/48 decline honestly on--fulland are asserted as the good pattern; gates 6/7/8/9 give a right verdict with a wrong reason and are recorded.Proving the suite can fail
A suite for gates is subject to exactly the disease it diagnoses, so each control was broken deliberately and observed red, then restored.
check_listener_placement.py's registration regex#347landing (--allon unscoped runs)#347appears to be FIXED — flip this assertion"UNCOVERED.mdUNCOVERED.md#353relaxationBreakage F runs on every execution, not once: the
#353author's first "now passes" fixture passed identically under the old regex, so the suite re-derives that proof each run.Not weakened
No baselines, no excludes-to-dodge, no deleted or skipped tests, no widened thresholds. Gates that genuinely cannot be fixture-tested are listed in
UNCOVERED.mdwith the exact blocking dependency named, rather than given a fixture that passes vacuously.