fix(gates 2, 21, 35, 36): three false positives and one blind spot, both arms fixtured - #380
Merged
Merged
Conversation
…ach with both arms fixtured A gate that cries wolf gets its silences believed. gate-7 reported zero IDORs in all eighteen apps for months because its known failure mode was false POSITIVES, so everyone learned to distrust its findings and therefore to trust its quiet. Three of the four repairs below are false-positive removals, which makes them security work by that route. Each defect was reproduced first on a purpose-built fixture driven through the real wrapper, and each repair is proven in BOTH directions with the prediction written before the run. gate-2 forbidden-patterns — `private function exit()` was reported as a call. `die` and `exit` are semi-reserved in PHP: illegal as free function names, legal as method names. The construct pattern saw the parameter list's `(` and flagged the one position where the identifier is definitionally not a call, while `$this->exit()`, `self::exit()` and `$exitCode` were already silent. The name offsets of every `function` header are now computed and skipped. `exit;` inside that method's body still fires, and so does `exit(0)` inside a method merely named `exitEarly()`. gate-21 conflict-markers — a seven-character Markdown setext H1 underline is byte-identical to git's separator under the anchored seven-equals pattern. In Markdown only, a bare separator with no angle-bracket marker anywhere in the same file is now read as a heading; every other file type is untouched, which is where all three incidents this gate was built for actually happened. A second, independent defect in the same block: the enumeration named `appinfo` twice, so every file under it was walked twice and a conflicted routes.php — one of those three incidents — was counted as two files. The `find`-based enumeration is deliberately KEPT rather than aligned to `git ls-files`, with the reason written into the block: an unstaged conflicted file is a real state on shared checkouts, and switching would narrow the true-positive class to buy tidiness. gate-35 img-alt-empty-only — the noun list was word-boundary anchored, and `U` is a word character, so `avatarUrl`, `thumbnailUrl`, `photoUrl`, `pictureUrl` and `avatar_url` could not match. On a seven-image probe where every image was a violation the gate found three. The source expression is now tokenised on camelCase and punctuation boundaries and compared for token EQUALITY. Equality rather than substring is the load-bearing choice: a relaxed substring match lights up `photographerBio`, `gallery.pictures` and `thumbnails`, all of which are fine, and that is how the next gate earns a reputation for crying wolf. gate-36 tabindex-positive — the last raw grep in the family reported a comment warning against positive tabindex as a positive tabindex, the same shape as the gate-32 defect where the better a team documents a rule the redder its repo goes. Comments are now masked. Deliberately comments only, not the markup scope: focus order is a property of the rendered DOM whoever emitted it, so `<script>`, PHP code and `.js` files all stay in scope. Removing noise by removing detection is not a repair. gates 35 and 36 move into check_markup_a11y.py, joining gates 31 and 32 for the same reason those moved. MEASUREMENT. On one fixture tree, changing only the gate package: gate-2 three findings to two, gate-21 five to three, gate-35 three to seven, gate-36 three to one. On openregister and pipelinq at their development tips, all four gates report clean before and after; with committed true positives planted in both repositories the four gates behave identically in both, and the pre-repair package misses the planted image entirely while reporting the planted comment. LIVE EXPOSURE IS ZERO FOR ALL FOUR, positive-controlled across all eighteen fleet apps: no method named after a language construct, no seven-character setext underline, no positive tabindex value, and seventeen image-tag occurrences fleet-wide of which three are documentation prose and exactly one carries an empty alt — correctly silent before and after. These repairs buy trustworthiness rather than closing a defect. Do not read urgency into them. ACCEPTANCE COVERAGE. Two new repo-shaped bundles, debug-and-conflict and a11y-noise, take the ratchet from thirteen fixtured gates to seventeen and their four rows leave UNCOVERED.md. Two expect.conf rows per gate, because the driver grades a verdict plus a named subject and never a count: with every true positive of a gate in one file, a repair that switches a whole rule off still produces a failure naming that file, and the suite stays green. That was measured while authoring these bundles, not assumed. Eight deliberate breakages were run, each verified to have landed before the result was believed, and each turns the suite red — including both over-corrections, exempting all Markdown separators and widening the noun match to a substring. Also corrected in UNCOVERED.md: a note claiming the auth-guards bundle was an empty stub, untrue since that bundle was finished.
…th sides Three hunks in run-hydra-gates.sh (gates 35 and 36) and one in UNCOVERED.md. Both sides were fixing the same class from opposite ends, so neither side is discarded: gates 35 + 36 — this branch replaces the raw greps with check_markup_a11y.py, because `grep -rnE` read the COMMENT documenting a rule as a violation of it. main meanwhile added `.github#374`'s "count what was opened" invariant, using `_iae_inspected` / `_tp_inspected` counters precisely because a grep hands back no file list. The helper takes an explicit file array, so that array IS the corpus and the count: #374's invariant is kept structurally, and the counters it needed are gone with the greps that needed them. Noted at both sites so the next reader does not restore a counter that no longer has a job. The empty-scope path now goes through main's shared `_skip_empty_scope` rather than the reason string this branch hand-wrote. That string named ADR-020 as the rule in force ("the diff touches none under ADR-020"), and #378 made diff scoping OPT-IN — so on every full-scope run it would have stated the wrong cause for the skip. `_skip_empty_scope` distinguishes "narrowed with --scope-to-diff" from "the tree contains no such file", which is the whole point of the distinction. Keeping this branch's wording would have shipped a correct skip carrying a false explanation — the same defect as #377's stale provenance note, one merge later. For gate-36 the subject string is main's "frontend source file (...vue|js|ts|php|html)": .js/.ts are in this gate's corpus and NOT in `_a11y_markup_files`, a distinction #374 was explicit about preserving. UNCOVERED.md — both sides correct the same stale claim about `auth-guards/`. Combined: main's superseding record (auth-guards is real, authn-vs-authz joins it, and the two assert gate-7 from opposite directions) plus this branch's general principle, which is why the old text is struck through rather than deleted — a bundle with no expect.conf is not coverage, and the driver hard-fails it. Full helper suite after resolution: 80 discovered, 78 passed, 2 quarantined, 0 failed, "ALL discovered helper suites PASSED".
rubenvdlinde
added a commit
that referenced
this pull request
Aug 12, 2026
…a subject (#404) Three of the ten acceptance bundles could not detect the regression they exist to prevent. All ten go red on a FULL revert -- which is the test anybody actually runs, and which is why nobody noticed. Three were blind to a PARTIAL one, and one of those was blind to precisely the defect it was built for. `_names` is `grep -qF`. gate-7's log line is `path:LINE method=NAME rule=RULE`, so `method=preamble` is CONTAINED IN `method=preambleForbiddenCode`. Reverting only the 401 half of #365 removed preamble() from the log entirely and the suite printed `PASS -- gate-7 NAMES 'method=preamble'` and `ALL PASS`. #380's remedy -- one mechanism per named file -- would not have caught that: all three plants legitimately belong in one controller, because the fixture's whole argument is that the arms differ only by the preamble. The real invariant is that a subject must be unsatisfiable by every OTHER finding the bundle can produce. A method name is a prefix of every longer method name, so a name alone is never a discriminator. Every subject is now closed on the right by its ` rule=...` / ` calls a service method` field, one row per arm. A trailing space would NOT have worked: `read` strips trailing IFS whitespace from the last field, so `method=preamble ` collapses back and the bundle stays vacuous while looking fixed. The anchor has to be a token. Internal spaces DO survive, which is why the driver needed no change. authn-vs-authz 1 -> 3 rows (bare / 401 preamble / 403 preamble) auth-guards 1 -> 2 rows (was the FILE PATH, which both plants name) exception-translation 1 -> 2 rows (destroy header-binding / purge docblock) comment-silenced-guard NEW 3 mechanisms, 2 files, 2 rows The new bundle closes the third of gate-7's three proven blindnesses, which had no repo-shaped fixture at all -- only unit tests, i.e. exactly the layer this suite's own header argues is insufficient ("gate-7 has 86 unit tests. All 86 passed. The gate was still wrong."). Its three mechanisms revert separately: A the body test reads comment-free text. 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. B a `throw` must NAME an authorisation exception. opencatalogi's getObjectService(), in 36 controller files across 7 apps, throws when OpenRegister is absent and so cleared every caller. C an ownership comparison answered with `null` IS recognised, so the narrowing costs no true clear. Visible only in the clean/ arm. Both arms keep the comment and keep the locator verbatim. #373 did not make prose illegal, it made prose uncountable; an arm that also deleted the sentence would have passed against the broken checker too. Twelve probes, twelve predictions written before the run, twelve matched exactly -- count and the identity of every failing assertion. 52 -> 70 assertions, 0 failed. Probes 4 and 7 are the two the vacuity audit measured at 28 passed / 0 failed, green over the exact defect; they are now one failure each, naming the lost plant. Probe 9b is its own result: gate-7's routed-method scan still reads comment-bearing text, so an explanatory docblock in a fixture can clear the very method the fixture plants. With mechanism A reverted, deleting ONLY the two comment lines restored the finding -- so the silencer is the comment and nothing else in the file. Every per-method docblock in the new fixture is austere for that reason. Not closed, and said so in the bundle header: `scan_file` still matches _GUARD_BODY_RE against comment-bearing text, so a comment in a ROUTED method containing Http::STATUS_FORBIDDEN / `, 403)` still clears it. This bundle pins the helper path only. a11y-noise and debug-and-conflict (#380) have not been revert-audited by anyone. Coverage is unchanged at 22 of 65 gates fixtured. Only hydra-gates/scripts/test-fixtures/gate-acceptance/ is touched. No checker, no runner, no driver. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
rubenvdlinde
added a commit
that referenced
this pull request
Aug 12, 2026
…he subject too (#409) * test(gate-acceptance): a sibling finding in the SAME file satisfies the subject too Revert-audits the last two bundles nobody had ever tested. #404 closed asking for exactly this, on the grounds that #380's remedy -- one mechanism per named file -- is necessary but not sufficient. debug-and-conflict is SOUND and is not changed. Four independent reverts, four predictions written first, four matched: dropping \s* from _CALL_RX reddens the Debugger.php row alone (120/1); narrowing _CONSTRUCT_RX to (?=[(]) reddens the Terminator.php row alone (120/1) -- disjoint findings, which is what proves the two rows separable; exempting all Markdown reddens conflicted.md (120/1); and reverting the setext discriminator reddens the CLEAN arm instead (119/2), so both directions are covered. a11y-noise was NOT. It went green over the exact regression it exists to pin. FacesCamel.vue was authored to hold "ONLY the forms the \b-anchored noun list could NOT see", but avatarUrl and avatar_url are TWO code paths, not one: _CAMEL_1/_CAMEL_2 versus _NON_ALNUM. Reverting camelCase splitting alone left avatar_url firing, which kept the file in the log, which satisfied grep -qF: 9 findings -> 5 and the suite printed ALL PASS. Two more of the same shape -- dropping plain `src` support (9 -> 8) and single-quoted tabindex (3 -> 2) -- were also green. Every FULL revert was caught, which is the test anybody runs, and is why this survived three audits. The invariant needs one more turn. #404's defect was substring containment BETWEEN subjects; this one is sibling findings INSIDE one subject. The subject is a file path, and a file path is satisfied by every finding in that file. The construction that is safe by definition is a named file carrying exactly ONE finding: any mechanism it depends on empties the file when lost. Where one finding cannot state the claim, every finding must ride the same code path -- and "same code path" means read the checker, not the fixture's comment. a11y-noise 4 -> 7 rows, corpus UNCHANGED (still 9 + 3 findings, redistributed) NounsCamel.vue camelCase only NounsSnake.vue avatar_url only NounsPlain.vue the \b-visible nouns NounsLiteral.vue the literal src only FocusDouble.vue tabindex="5" FocusSingle.vue tabindex='12' All three formerly-green probes now redden exactly one row each, naming it, so which row fails tells you which mechanism was lost. render-helpers.js verified independently: removing js/ts from gate-36's find reddens that row alone. The planted files were renamed because Faces.vue IS a prefix of FacesCamel.vue and only the .vue extension separated the two subjects -- relying on an extension as the terminator is relying on an accident. Pairwise containment is now checked against the real log text, positive-controlled on #404's own corpus first, since a grep -qF that cannot match reports "no containment" for free. Also corrects a false sentence in debug-and-conflict/expect.conf claiming a regression "fails here on the COUNT". The driver never reads a count. Same wrong sentence #404 corrected in auth-guards -- found now in two bundles by two authors, so it is a recurring mental model, not a typo. Fixed in place because it was load-bearing for somebody's confidence. And closes the registry class #405 identified: COVERED-ELSEWHERE.md credits coverage by gate NUMBER while naming the providing suite in PROSE, and nothing ever resolved that name to a file -- though those rows feed "58 of 65 gates fixtured". All 19 named suites exist today, so the check pins a true state rather than reporting a backlog. Built fail-closed: fewer than 10 extracted names FAILS rather than reporting success over an empty list, because a check over zero names passes for free, which is the defect it exists to catch. Proven able to fail by renaming one credited suite -> 130/1. Thirteen probes, thirteen predictions written first, thirteen matched on count AND on the identity of every failing assertion. Every revert quoted its actual changed line from git diff -U0; Edit only, no sed/awk/perl/python. Checkers and runner restored after every probe and byte-identical to main. assertions 121 -> 131 passed / 0 failed * fix(gate-acceptance): scope the suite-name extraction, and drop a backtick ShellCheck reads as a substitution Two corrections to the COVERED-ELSEWHERE.md resolution check, both found by CI rather than by me. ShellCheck flagged SC2016 on the extraction pattern. The pattern anchored on the backticks the suite names are written in, and a backtick inside a single-quoted string reads to ShellCheck as a command substitution. Dropped the anchor. That is also more correct: some rows carry a scripts/lib/ prefix INSIDE the backticks, which a backtick-anchored pattern would have missed.⚠️ The first ShellCheck run had FAILED WITHOUT RUNNING -- the action could not unpack its download ("does not look like a tar archive") -- so it never saw this. A peer PR hit the identical transient failure eleven minutes earlier and went green on re-run. Re-running it here surfaced a REAL finding in my own file that the infrastructure failure had masked. A red that is the harness and a red that is the code are worth telling apart in both directions. Then the broadened pattern counted 21 suites where 20 are credited: the intro prose names this driver itself. Scoped to table rows, which is where the credit is actually granted, so the number the check reports means what it says. The negative control was RE-RUN against the changed extraction rather than inherited from the earlier version -- renaming a credited suite still reddens exactly one assertion (130/1). Merges main, which moved twice under this branch and touched the same driver (#401's verdict-shape matcher) and the same registry (gates 47 and 48). 60 of 65 gates fixtured -- 131 passed / 0 failed --------- 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
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.
Fixes three false positives and one blind spot found by the gate audit. Do not merge without Ruben — a gate change moves the verdict for all eighteen apps at once.
Why this is security work
A gate that cries wolf gets its silences believed. That is not a slogan: gate-7 reported zero IDORs in all eighteen apps for months because its known failure mode was false positives, so every agent who met it learned to distrust its findings and therefore to trust its quiet. One hundred and sixty-seven real ones sat in the tree. Three of the four repairs here remove a false positive, and that is the route by which they matter.
The four
gate-2 forbidden-patterns flagged
private function exit().dieandexitare semi-reserved in PHP — illegal as free function names, legal as method names — so the construct pattern saw the parameter list's opening bracket and reported the one position where the identifier is definitionally not a call. The call sites$this->exit()andself::exit()and the property$exitCodewere already silent, so the gate was quiet everywhere it might have been right and loud in the one place it could not be. Fixed by computing the name offsets of everyfunctionheader and skipping a construct match that starts exactly there. A construct in that method's body still fires, and so does one in a method merely namedexitEarly().gate-21 conflict-markers flagged a seven-character Markdown setext H1 underline, which is byte-identical to git's separator under the anchored pattern. In Markdown only, a bare separator with no angle-bracket marker anywhere in the same file is now read as a heading. Every other file type is untouched, which is where all three incidents this gate was written for actually happened. A second, independent defect in the same block: the enumeration named
appinfotwice, sofindwalked it twice and a conflictedappinfo/routes.php— one of those three incidents — was reported as two files.The brief asked whether this gate's use of
findrather thangit ls-files, unique in gates one to twenty-one, is intended. Decision: keep it, and say so in the code. An unstaged conflicted file is a real state on shared checkouts, and switching to the tracked-file enumeration would narrow the true-positive class to buy consistency. The reason is now written into the block rather than left as an unexplained asymmetry.gate-35 img-alt-empty-only was blind. The noun list was word-boundary anchored, and
Uis a word character, soavatarUrl,thumbnailUrl,photoUrlandpictureUrlcould not match; underscore is a word character too, soavatar_urlwas invisible for the same reason. On a seven-image probe in which every image was a violation, the gate found three. The source expression is now tokenised on camelCase and punctuation boundaries and compared for token equality. Equality rather than substring is the load-bearing choice — a relaxed substring match lights upphotographerBio,gallery.picturesandthumbnails, all of which are correct code, and that is exactly how the next gate earns the reputation this PR exists to undo.gate-36 tabindex-positive, the last raw grep in its family, reported a comment warning against positive tabindex as a positive tabindex — the gate-32 shape, where the better a team documents a rule the redder its repository goes. Comments are now masked. Deliberately comments only, not the markup scope: focus order is a property of the rendered DOM whoever emitted it, so script blocks, PHP code and
.jsfiles all stay in scope. Removing the noise by removing detection would not be a repair.gates 35 and 36 move into
check_markup_a11y.py, joining 31 and 32 for the reason those moved.Measurement
On one fixture tree with only the gate package changed, gate-2 goes from three findings to two, gate-21 from five to three, gate-35 from three to seven, gate-36 from three to one.
On openregister and pipelinq, both at their
developmenttips, all four gates are clean before and after the change — so the before/after on real repositories is zero to zero, and a repair that had silently narrowed detection would be invisible in that number alone. The load-bearing measurement is therefore the planted one: with true positives committed into both repositories, the pre-repair package misses the planted image entirely and reports the planted comment, while the repaired package names the real subject in each of the four gates, identically in both apps.Live exposure is zero for all four defects, positive-controlled across all eighteen fleet apps. No method is named after a language construct; there is no seven-character setext underline; there is no positive tabindex value anywhere; and there are seventeen image-tag occurrences fleet-wide, of which three are documentation prose and exactly one carries an empty alt, correctly silent both before and after. These repairs buy trustworthiness, not a closed defect. Please do not read urgency into them.
Acceptance coverage
Two new repo-shaped bundles under
gate-acceptance/. The ratchet moves from thirteen fixtured gates to seventeen and the four rows leaveUNCOVERED.md.There are two
expect.confrows per gate, because the driver grades a verdict plus a named subject and never a count. With all of a gate's true positives in a single file, switching a whole rule off still produces a failure naming that file and the suite stays green — measured while authoring these bundles, not assumed. Each row now names a file holding exactly one mechanism.Eight deliberate breakages were run, each verified to have landed before its result was believed, and each turns the suite red. They include both over-corrections a reviewer should worry about: exempting every Markdown separator, and widening the noun match to a substring. One prediction was wrong — substituting the markup scope for the comments-only mask reddens the clean arm rather than the planted one, because the extension dispatch sends
.jssomewhere that does not understand//— and that correction is recorded in the fixture rather than tidied away.The full helper-suite runner reports seventy-four passing, two quarantined, and one failing. The one failure is
test_gate_45_to_55_acceptance.sh, which fails identically on the unmodified base withajv is not resolvable; the two logs differ only in the absolute path. It is the knownneeds-externaldependency, not this change.