fix(gates): 46/40/28/9/7 were majority-false, and the runner corrupted its own measurements - #175
Merged
Merged
Conversation
…d its own measurements
Two gates produced more false findings than real ones, which made a
fleet burn-down UN-MERGEABLE: the gates are diff- AND file-scoped, so
touching a file drags in its pre-existing findings, and when the residue
is entirely false there is no honest way to green the PR. Two burn-down
PRs (opencatalogi#808, docudesk#385) are stuck in exactly that state.
Measured across 21 fleet repos at origin/development, against v1.4.0:
gate-46 spec-anchor-existence 1,995 -> 918 (-54%)
gate-40 form-label-association 1,211 -> 517 (-57%)
gate-9 semantic-auth 45 -> 11 (-76%)
gate-7 no-admin-idor 32 -> 26 (-19%, partial)
Every relaxation ships with the true-positive case it must not swallow.
Four new suites (gate-46, gate-40, gate-28, gate-9) and 8 new gate-7
cases, all discovered automatically by tests/run-helper-suites.sh. Each
was mutation-checked: making the relaxed predicate always-true fails the
suite, and so does making it always-false.
gate-46 — 1,077 findings cleared, none of them evidence about a spec
* the `:`-tail rule accepted only EQUALITY where the full-heading rule
accepted a PREFIX, so `### Requirement: REQ-001: List zaken` rejected
`#REQ-001`
* a requirement id in trailing parens/brackets was invisible:
`### Requirement: Payment Provider Adapter Interface (REQ-PAY-001)`
rejected `#REQ-PAY-001`. Lifted tokens match by EQUALITY only, so
`#REQ` and `#REQ-PAY` still do not resolve
* an id before the colon — `#### Scenario REQ-BIE-004-01: Cron triggers`
— likewise. Only tokens CONTAINING A DIGIT are lifted; without that
every word of every heading becomes an anchor
* `- [~]` and `- [-]` checkboxes were invisible to the task rule, which
ALSO shifted every positional `#task-N` after them. A wrong positional
resolution reports PASS against a different task — worse than the
missing anchor it replaced
* `openspec/specs/x.md` and `openspec/specs/x/spec.md` are the same spec
* `#scenario`/`#requirement` no longer prefix-match every heading of
that level; `#webhooks` against `## Webhooks (Task 2.9 of giant)`
still resolves, by equality against the bracket-stripped heading
gate-40 — 694 cleared, and its advice was an a11y REGRESSION
The only way to satisfy it on `<NcCheckboxRadioSwitch>Installed apps
only</NcCheckboxRadioSwitch>` was to add `aria-label`, which OVERRIDES
the visible label and breaks speech-input users. 463 findings were that
shape. Also: implicit `<label>` wrapping (268), bound `:id`/`:for`
pairs matched by expression (56), and markup inside comments and
<script> blocks (5). Replaces a flatten-the-newlines regex with a real
tag walker, so nesting and slots are visible. A self-closed switch with
no slot and no prop — docudesk Settings.vue:41 — is still reported.
Also ~40x faster: one python process, not one per .vue file.
gate-28 — the NUL byte is worse than filed (#171)
Depending on the grep implementation, a raw 0x00 makes it either a
false RED (GNU grep <=3.4 prints "Binary file X matches" on stdout, so
`awk '{print $3}'` reads the FILE PATH as the licence) or a false GREEN
(ugrep / GNU grep >=3.5 print nothing to stdout, so the gate `continue`s
and NEVER CHECKS THE FILE). Verified: an `@license AGPL-3.0-or-later`
hidden behind a NUL passed silently. Reading bytes in python removes
the class. Also collects EVERY declaration — `@license` tags and
`SPDX-License-Identifier:` lines alike — instead of the first `@license`
only, which is how 174 files carried an AGPL claim behind a green gate.
Identifiers inside string literals stay test data, not claims.
gate-9 — its remediation would have INTRODUCED the vulnerability
"remove #[PublicPage] or remove body auth check": the first breaks the
endpoint (middleware rejects the remote caller before the controller
runs), the second deletes its only authentication. 34 of 45 findings
were webhook/portal/federation endpoints that correctly bypass session
auth and authenticate from the REQUEST. Returning 401/403 is not, on
its own, evidence of a session dependency. A #[PublicPage] method that
tests the SESSION still fires, under a rule name that says so.
gate-7 — it was ANTI-CORRELATED with the property it checks (#160)
On a multi-tenant codebase a tenancy guard refuses with 404 ON PURPOSE,
because a 403 leaks another tenant's object ids. gate-7 excluded bare
throws, so it flagged exactly the code that got tenancy right — and
FlowController::state() reported identically before and after its real
IDOR was fixed. Adds a tenancy signal requiring BOTH a comparison
against a session-derived scope AND a refusal. Partial: 6 of the ~17
openregister false positives clear; the rest need collaborator-hop work.
THE RUNNER CORRUPTED ITS OWN MEASUREMENTS
61 gates wrote to hardcoded /tmp/hydra-gate-<name>.log and derived
verdicts by `wc -l` on them. Exactly one used mktemp. Demonstrated with
two concurrent runs on different repos at v1.4.0: petstore reported
"gate-46 FAIL - 26 unresolved targets - see /tmp/hydra-gate-spec-anchor-
existence.log" while that file contained ZERO lines, app-versions having
truncated it; both repos' gate-40 verdicts (1 and 7) pointed at one file
holding 7 lines. Had the truncation landed before the `wc -l`, petstore
would have reported PASS over 26 real findings. Now: one private
directory per invocation, printed once, TMPDIR honoured, and the run
REFUSES (exit 97) rather than falling back to a shared path. Same fix in
tests/run-helper-suites.sh, test_gate_route_auth.sh and
test_check_manifest.sh, which had the same defect: route-auth reported 7
failures under the harness and 0 standalone.
A RESOLVING DIFF BASE IS NOT A USABLE ONE
shillinq's `development` run finished in 22 seconds, all green. Cause:
on a push to a mainline branch `origin/development` IS HEAD, so the diff
is empty by construction and every gate passes over nothing. Verified at
c64e9fe — 52 gates PASS scoped, 18 FAIL unscoped. Now refused with exit
99, alongside a merge-base check for shallow checkouts, and the diff's
exit code is read directly rather than through a `||` chain that cannot
tell "no changes" from "could not run".
The first draft of that block used `set +e`/`set -e`, which does not
restore state — it enabled errexit for the remaining 3,700 lines and
aborted every scoped run right after the scope line. Caught by
test_gate_route_auth.sh. Replaced with `&& rc=0 || rc=$?`.
ALSO
* 19 helper lookups re-resolved `dirname "${BASH_SOURCE[0]}"` AFTER the
`cd "${APP_DIR}"`, against the warning at the top of the file. One of
them (gate-17) aborted the entire suite when the runner was invoked by
a relative path — 46 gates never ran.
* gates 46/40/28 now _skip(wiring) when their helper is missing, rather
than passing over an unread file set (#147).
* quality.yml floats on @main while the package it drives is PINNED, and
it executes paths inside that package BY NAME. That interface broke
three times in one day (#168). Pinning both halves from one tag is a
human call; until then a preflight names the desync instead of letting
it surface as an unexplained gate failure.
This was referenced Aug 5, 2026
added 2 commits
August 5, 2026 23:33
CodeQL `py/bad-tag-filter`, high, on the terminator-trimming pattern introduced in the previous commit. Nothing here sanitises markup — it trims a comment terminator off a licence identifier — but a half-known comment syntax is still half-known: `<!--SPDX-License-Identifier: EUPL-1.2--!>` would have yielded `EUPL-1.2--!` as the licence and reported drift on a correct header. Tested both ways: the value is trimmed for `*/`, `-->` and `--!>`, and a genuinely wrong licence is still wrong after trimming.
Three reconciliations, each caught by an existing assertion rather than by reading the diff: * gate-28 — #172 gave this gate a four-way skip taxonomy (`na` for no lib/, `na` for no composer.json, `structural` for a composer.json without a `license`, `structural` for nothing carrying a tag) and made PASS conditional on having actually compared a file. Moving the read into a helper collapsed all of that into one `na`, undoing it. The taxonomy is restored verbatim; the helper now returns the compared-file count on stderr so `_lt_checked` still gates the PASS. The one state that short-circuits it is a MISSING HELPER — which #172's chain would have called `structural`, i.e. a claim about the REPOSITORY, when the repository is fine and the gate is broken. Those must not wear the same words. * gates 40 and 46 — an empty in-scope file set is ordinary ADR-020 diff scoping, not "not applicable". tests/test-hydra-gates-bin.sh asserts that no src-guarded gate reports NOT APPLICABLE while src/ exists, and gate-40 was tripping it. Answering differently from every sibling gate drifts the applicability table away from the guards it mirrors — the one way that table could hide a live gate. * quality.yml — #173 added a preflight for the FLAG direction of the floating-caller / pinned-callee desync (does the pinned runner understand the coverage flag?). This branch adds one for the PATH direction (does the pinned package contain the files the workflow executes by name?). Both are kept: they are different halves of the same interface, and #168 broke the half #173 does not cover. And one contract change, made explicit rather than incidental: test-hydra-gates-bin.sh's "empty diff exits 0" fixture set BASE_SHA to HEAD, so it was really asserting "scoping a commit against itself exits 0" — the shillinq shape. Those are different facts and only one is legitimate. The original assertion now runs against a real base-behind-HEAD empty diff and still expects 0; a new assertion covers base == HEAD, expects 99, and checks that NO gate printed PASS. Verified on the merged tree: hydra-gates entry-point tests: 36 passed, 0 failed (was 31 passed, 3 failed) helper suites: 23 passed, 0 failed gate-46 fleet: 1,995 -> 918 gate-40 fleet: 1,211 -> 517 (unchanged by the merge)
Contributor
Author
Update — rebased onto
|
…/tmp path
Two CI failures, both caused by this branch, both worth the detour.
* `hydra-gates-package.yml` asserts that a gate reporting FAIL also WROTE
the evidence naming the offending file — a good assertion, reading a
hardcoded /tmp/hydra-gate-<name>.log. That is the defect this branch
exists to remove, one level up: with a shared path the check could read a
DIFFERENT run's log and pass, or a truncated one and fail, and neither
outcome would say anything about the fixture. It now reads the directory
the run announces on its own first line, and fails loudly if no such line
appeared — because an unattributable verdict is not a verdict.
Reproduced locally against the fixed runner: exit 2, both gates named,
both injected files named in their own run's logs.
* ShellCheck SC2086 x4 on the `${_TCM_LOG}` uses added to
test_check_manifest.sh. Quoted.
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
2 participants
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
1,726 real findings across the fleet cannot be burned down until these gates are honest. The gates are diff- AND file-scoped, so touching a file drags in all of its pre-existing findings — and when the residue is entirely false positives there is no honest way to green the PR. Two burn-down PRs (opencatalogi#808, docudesk#385) are open and stuck in exactly that state.
The sharpest example: clearing gate-40's residue would mean adding
aria-labeltoNcCheckboxRadioSwitchelements that already name themselves from their default slot.aria-labeloverrides the visible label. The only way to close an accessibility gate was to ship an accessibility regression.Measured, 21 fleet repos at
origin/development, baseline = v1.4.0 (46cc1c8)Note these are not the 2,630 / 1,365 in #158: those predate
gh_slugify(#165), which is in v1.4.0 and in no earlier tag. Measured honestly against whatmainactually does today.Never make a gate pass by making it blind
Every relaxation ships paired with the true-positive case it must not swallow. Four new suites (46, 40, 28, 9) plus 8 new gate-7 cases, all auto-discovered by
tests/run-helper-suites.sh.Each relaxed predicate was mutation-checked in both directions:
has_anchor→ always True:id/:for→ "is bound at all"The last pair is the one that matters: the gate-7 suite pins both edges, so neither a mute nor a revert can pass it.
The runner corrupted its own measurements
61 gates wrote to hardcoded
/tmp/hydra-gate-<name>.logand derived their verdicts bywc -lon them. Exactly one usedmktemp, and its comment said why.Two concurrent runs on different repos, v1.4.0:
petstore's 26 findings were truncated away by app-versions. Had the truncation landed before the
wc -linstead of after, petstore would have reported PASS over 26 real findings. Same test on this branch: two private directories, each containing exactly what its own run reported (26/0 and 0/1).The package's own test suite had it too —
test_gate_route_auth.shreported 7 failures under the harness and 0 standalone. Fixed there as well.A resolving diff base is not a usable one
shillinq's
developmentrun finished in 22 seconds, all green. Not a shallow checkout: on a push to a mainline branchorigin/developmentISHEAD, so the diff is empty by construction and every gate passes over nothing. Verified atc64e9fe— 52 gates PASS scoped, 18 FAIL unscoped.Now refused (exit 99), with a merge-base check for genuine shallow checkouts, and the diff's exit code read directly rather than through a
||chain that cannot tell "no changes" from "could not run".gate-28: the NUL byte is worse than #171 filed
Depending on the grep implementation a raw
0x00produces either outcome:Binary file X matcheson stdout →awk '{print $3}'reads the file path as the licence → false REDcontinues and never checks the file → false GREENVerified: an
@license AGPL-3.0-or-laterhidden behind a NUL passed silently. Live NULs remain indoriath/src/import/model.jsandopenbuild/src/services/manifestValidation/documentAttachments.js(filereports both asdata).The gate also read only the first
@licenseand ignoredSPDX-License-Identifier:entirely — how 174 files carried an AGPL claim behind a green gate. Now every declaration is collected; identifiers inside string literals stay test data (nldesign'sMarianneFontTest.php).gate-9: the advice would have introduced the vulnerability
The first breaks the endpoint — NC middleware rejects the remote caller before the controller runs. The second deletes its only authentication. There was no correct action a developer could take on 34 of 45 findings. Returning 401/403 is not, on its own, evidence of a session dependency; it is what a self-authenticating public endpoint does.
A
#[PublicPage]method that tests the session still fires, under a rule name that says so, with advice that never tells anyone to open an endpoint. Asserted on the string itself, inRemediationTextIsSafe.gate-7: anti-correlated with the property it checks
On a multi-tenant codebase the tenancy guard refuses with 404 on purpose, because a 403 leaks another tenant's object ids. gate-7 excluded bare throws, so it flagged exactly the code that got tenancy right — and
FlowController::state()reported identically before and after its real IDOR was fixed.Adds a signal requiring both a comparison against a session-derived scope and a refusal. Partial: 6 of ~17 openregister false positives clear. The rest need collaborator-hop work — left for a follow-up rather than guessed at, because this is a security gate.
Also
dirname "${BASH_SOURCE[0]}"after thecd "${APP_DIR}", against the warning at the top of the file. One (gate-17) aborted the whole suite when the runner was invoked by a relative path — 46 gates never ran._skip(wiring)when their helper is missing rather than passing over an unread file set (fix(gates): 16 gates reported PASS when their helper never ran — 2 are authorization gates #147)..vue.quality.ymlfloats on@mainwhile the package it drives is PINNED, and executes paths inside it by name. That interface broke three times in one day (fix(quality): re-land axe DOM scoping, with the runner in a file — a 22 KBrun:step is what made the workflow unresolvable #168). Pinning both halves from one tag is a human call — it changes how 22 repos receive gate fixes. Until then a preflight names the desync instead of letting it surface as an unexplained gate failure. Largest singlerun:step in the file is 8,533 B, well under the ~19 KB limit.Verification
22 suites green (v1.4.0: 17 passing, 1 failing). 3 concurrent harness runs: 0 failures each — on v1.4.0 one of two overlapping runs failed.
Release
Ready to tag v1.5.0, not v1.4.x. New refusal conditions (exit 97, exit 99 on base==HEAD) turn previously-passing runs into failures, and gates emit new rule names. That must not arrive as a patch. The fleet pins v1.3.0, so anyone bumping to a v1.4.x would silently also receive v1.4.0's unrolled
gh_slugifyverdict change — one minor makes the whole delta legible in one release note.Not rolled to the fleet. Reported ready.
Closes #158 (items 2 and 6), closes #171. Partially addresses #160 (items 2 and 3; item 1 partial). Addresses #159's
@main-vs-pinned split with a named preflight, not a resolution.