fix(gates): nine checkers matched prose, not code (#191, #196, #220, #224, #226, #230, #235, #236, #266) - #269
Merged
Merged
Conversation
… nine gates Every gate below decided a question about CODE by grepping the raw bytes of a file. Prose is made of the same bytes, so each one failed in BOTH directions at once — the shape first written down in #184: "a checker that greps a STRING LITERAL misses every constant and matches every comment." #191 gate-48 a REMOVED COMMENT naming `#[NoCSRFRequired]` read as a removed attribute. nldesign red for one rewritten docblock sentence. #196 gate-5 a docblock saying `#[NoAdminRequired]` is deliberately NOT used SATISFIED the auth gate. A false NEGATIVE on a security gate, and a pass leaves no log. #220 gate-31 an `<img>` in a JSDoc comment in <script> (launchpad). #235 gate-31 the same, 3 of 3 findings on openbuild. #224 gate-34 false RED on a comment AND false GREEN on window['confirm'](). #226 gate-3 a run() delegating to one helper read as a stub, and the gate was closable by an inert `$unused = 1;`. #230 gate-58 a comment WARNING AGAINST networkidle counted as a use of it. #236 gate-12 `<NcSelect[^>]*>` truncated at the `>` of `option =>`. #236 gate-32 a comment describing the `<div @click>` an element replaced scored as that `<div @click>`. #266 gate-41 a PHP comment mentioning `<html>` made a mount point a page root. ONE SCOPE, NOT NINE ------------------- scripts/lib/source_scope.py generalises the two precedents that already got this right — #184's PHP stripper (which knows `#` opens a comment but `#[` opens an attribute) and #249's gate-19 tokeniser (blank once, PRESERVE OFFSETS, keep string delimiters). Every mask returns a same-length string, so a gate can report a line number computed on the mask and read a suppression marker out of the ORIGINAL at that line — which matters because every suppression marker in this package lives in a comment. Gate-19 keeps its own copy of the JS tokeniser; a drift test asserts the two byte-identical over a corpus and over this package's own .js sources, and asserts the keyword sets equal — the corpus alone SURVIVED deleting "await" from one set, so the corpus alone was not enough. #196 SHIPS WITH A DECLARATION, NOT JUST A TIGHTENING ----------------------------------------------------- Admin-only is expressed in Nextcloud by the ABSENCE of an attribute, and absence is the only thing gate-5 reports. Closing the false negative alone would have converted it into a PERMANENT false positive on correct code, with no legitimate way to satisfy the gate. So `@auth admin-only <reason>` joins the `@spec exclude` family. Making bare absence sufficient was considered and rejected: it would empty the gate completely. MEASURED, NOT ASSUMED --------------------- - 3 fixtures from #226's table, the 4 arms from #224, the nldesign line from #191 and the larpingapp line from #230, all verbatim. - Every relaxation is paired with the true positive it must not swallow, and every wiring is covered both ways: a MISSING helper and a CRASHING helper must report SKIPPED, never PASS (#147, #245, #249). gate-5 additionally runs a positive control on the mask itself, because a mask that silently returns its input is invisible to `[ -f helper ]` and puts the gate straight back into the false negative. - A nested `<template #default>` slot regression was caught by measurement before landing: a lazy `(.*?)` ended the SFC template at the first slot close and deleted a real finding at openconnector EditMapping.vue:376. Boundaries are found by depth now, and there is a test. Closes #191, #196, #220, #224, #226, #230, #235, #266 Refs #236 (parts 1 and 2; part 3 was already fixed by #247) Supersedes #219, whose gate-12 helper is carried here with its 17 tests.
…ribute is one
Both found by MEASURING the fix rather than by reading the issues.
gate-34 — 7 defects reported as 14 findings
------------------------------------------
The first cut accepted any `window.confirm` REFERENCE, called or not, so on
openbuild every native dialog was reported twice:
const ok = typeof window !== 'undefined' && window.confirm <- guard
? window.confirm(t('openbuild', 'Delete this automation?')) <- call
A feature-detection guard is a truthiness test, not a second native dialog,
and inflating a security-adjacent count is its own false report (#254: a count
is not a defect count). A reference now counts only when it is an ALIAS — a
binding whose call site is elsewhere and therefore invisible:
const c = window.confirm counts
const { confirm } = window counts
x && window.confirm ? … : … does not
openbuild: 7 before, 7 after, same seven lines.
The anchor also lost a character it should never have had. Written
`=\s*window\s*[.\[]` it CONSUMED the `window` that follows, and `finditer`
returns non-overlapping matches — so `const r = window.confirm('x')` matched
only the alias rule, failed it because a `(` follows, and reported NOTHING. A
real call dropped by an anchor one character too greedy. It is a lookahead
now, and there is a test.
gate-48 — the old regex could not see a fully-qualified attribute
-----------------------------------------------------------------
Running #191's arm 2 end-to-end through the runner reported PASS on a genuine
removal of
- #[\OCP\AppFramework\Http\Attribute\NoCSRFRequired]
because the pre-fix pattern alternated on the literal `#[NoCSRFRequired]`.
A false NEGATIVE hiding behind the false positive #191 reported — the same
both-ways failure as every other gate in this change. The new bracket-bounded
rule matches it.
Refs #191, #224
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…now it
CodeQL raised py/bad-tag-filter (HIGH) against this branch, and it is right.
r'<script(\s[^>]*)?>(.*?)</script\s*>'
does not match `</script bar>` or `</script\t\n foo>`, both of which an HTML
parser treats as the end of the element. When the close is spelled that way
the block regex fails to match AT ALL, the script body is never
comment-masked, and a JSDoc `<img>` inside it is scanned as markup — #235
reintroduced by the mask written to fix it. `</style …>` had the same hole.
⚠️ THE FIRST TEST FOR THIS SURVIVED THE MUTANT. It exercised
`vue_markup_mask`, which keeps `<template>` spans and never goes through
`_SCRIPT_BLOCK` at all, so reverting the regex changed nothing and the suite
stayed green. The assertion now runs through `html_markup_mask` and
`script_mask`, the two functions that actually use it, and the reverted regex
kills both. A mutation test that does not kill is not evidence — it is a
second thing to check.
Refs #235
This was referenced Aug 8, 2026
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.
Nine checkers decided a question about code by grepping the raw bytes of a file. Prose is made of the same bytes, so every one of them failed in both directions at once — the shape #184 named: "a checker that greps a STRING LITERAL misses every constant and matches every comment."
#[NoCSRFRequired]— nldesign red for one rewritten docblock sentence#[NoAdminRequired]is deliberately NOT used satisfied the auth gate<img>in a JSDoc comment in<script>(launchpad)window['confirm']()run()delegating to one helper read as a stub; gate closable by$unused = 1;networkidlecounted as a use of it<NcSelect[^>]*>truncated at the>ofoption =><div @click>an element replaced, scored as that element<html>made a mount point a page root#236part 3 (gate-38 ontemplates/settings/admin.php) needs no change — #247 already fixed it; measured PASS on softwarecatalog in both arms, because that template is a fragment.One scope definition, not nine
scripts/lib/source_scope.pygeneralises the two precedents that already got this right, rather than inventing a third dialect:#opens a comment but#[opens an attribute.Every mask returns a same-length string. That is what lets a gate report a line number computed on the mask and read a suppression marker out of the original at that line — which matters because every suppression marker in this package lives in a comment.
Gate-19 keeps its own copy of the JS tokeniser (moving 180 lines out from under the suite that proves it correct is a bad trade mid-change). A drift test asserts the two byte-identical over a corpus, over this package's own
.jssources, and asserts the keyword sets equal — the corpus alone survived deleting"await"from one set, so the corpus alone was not enough.#196 ships with a declaration, not just a tightening
Admin-only is expressed in Nextcloud by the absence of an attribute, and absence is the only thing gate-5 reports.
#[NoAdminRequired]widens; so doesAuthorizedAdminSetting(to delegated admins). Closing the false negative alone would have converted a silent false negative into a permanent false positive on correct code — pipelinq alone would pull 17 routed methods into scope with no legitimate fix available. So the tightening lands with a declaration in the@spec excludefamily:Making bare absence sufficient was considered and rejected: absence is the only thing gate-5 reports, so accepting it would empty the gate completely. gate-9 (semantic-auth) still owns whether the declared posture matches the body. Documented in
hydra-gates/README.md.appinfo/routes.php— so the fleet impact is per-PR, not a one-off wave.Evidence
Planted true positives, both directions, on the real files named in the issues. Read from the runner's stdout, never its exit byte.
<img>— the tell from #235networkidleis not usedinput-labelwritten after:reduce="(o) => …"run(), one file with norun()at allNotificationDispatchJob— 530 lines, 11 private methods:class="{ clickable: … total_errors > 0 }"— the>inside the attribute value truncated the tag, so@clickwas never seen[ -d src ]gated them out of a templates-only repo; now_a11y_has_markup_dir(residual #225)#196, end-to-end on the fixture built from openconnector's
ProductSubscriptionsController: pre-fixmainreports 1 finding (analytics);subscribe(), whose only difference is a docblock sentence naming#[NoAdminRequired], passes. Post-fix reports 2. The suite asserts the count, because FAIL alone would have been FAIL before the fix too.#191, end-to-end through the runner, three arms:
#[\OCP\…\NoCSRFRequired]attribute removed* @NoCSRFRequireddocblock tag removedTests
tests/run-helper-suites.sh).test_source_scope.py(41),test_check_markup_a11y.py(25),test_check_js_call_sites.py(32),test_check_stub_run_body.py(20),test_check_csrf_removal.py(14),test_check_nc_select_labels.py(19, carried from fix(gate-12): the element ended at the arrow of:reduce="(o) => o.id"— 18 of 18 findings false on scholiq #219). Extended:test_gate_route_auth.sh(36),test_php_template_scope.py(23),test_gate_a11y_helper_wiring.sh(50).SKIPPED (wiring), never PASS (fix(gates): 16 gates reported PASS when their helper never ran — 2 are authorization gates #147, gate-17 reports 'FAIL — 0 pass-through method(s)' when python3 dies with 'Argument list too long' — the crash IS the full-tree measurement #245, fix(gate-19): read the test file with a parser, not three regexes (#234, #239, #244) #249). gate-5 additionally runs a positive control on the mask itself — a mask that silently returns its input is invisible to[ -f helper ]and would put the gate straight back into the false negative.old in sourceasserted before each: reverting each gate's fix to its pre-fix expression — the whole pre-fix behaviour, not one site — kills its suite. 7 of 7 mutants killed. Two earlier mutants onsource_scopekilled too, and one survived (deleting a regex keyword), which is why the drift test now compares the keyword sets directly.Caught by measurement before landing
<template…>(.*?)</template>ended the SFC template at the first nested slot close and deleted a real finding at openconnectorEditMapping.vue:376. Boundaries are found by depth now.const r = window.confirm('x').Closes #191, #196, #220, #224, #226, #230, #235, #266
Refs #236 (parts 1 and 2; part 3 already fixed by #247)
Supersedes #219 — its gate-12 helper and 17 tests are carried here, rebased onto current
mainand converged onto the shared scope.🤖 Generated with Claude Code