fix(gate-38): a mount point is not a page root, and the remedy it demanded regressed a11y (#214, #216, #227) - #247
Merged
Conversation
…anded regressed a11y (#214, #216, #227) WCAG 2.4.1 is a property of a DOCUMENT. gate-38 was asking it of two shapes that own no document, and reporting all of them. templates/**/*.php Nextcloud's Template renderer SUBSTITUTES an app template into core's page. Measured across the fleet: all 30 app templates are fragments, NOT ONE emits <html>, <head> or <body>. The typical body is literally `<div id="procest-settings"></div>`. 8 templates across 6 apps failed this way. AdminRoot.vue rendered INTO core's Settings page, inside the section core already built. A <CnAdminSettingsShell> is a stack of <NcSettingsSection>s, not an app shell (#227). In both cases the only way to close the finding was to add a SECOND "skip to content" anchor, pointing into the middle of a page that already has one and announced to every screen-reader user ahead of core's real one — a WCAG 2.4.1 regression demanded by a WCAG 2.4.1 gate. WHAT STILL FAILS - src/App.vue / src/views/App.vue with a bespoke shell - **/*Root.vue that is not a settings surface - a PHP template that DOES own the document and has no bypass mechanism. This WIDENS the PHP arm: it used to look only at templates/settings/, and now asks every app template the question. The classifier is scripts/lib/php_template_scope.py, not a grep, and that is load-bearing. The first cut WAS a grep, and the first fixture it met defeated it: the fixture's own explanatory comment contained `<html>`, so a bare mount point classified as a page root and the fix silently did nothing. Same defect as gate-64 — a checker that greps a string literal misses every constant and matches every comment, failing both ways at once. Classification now runs on emitted markup with PHP regions and HTML comments removed, and both comment shapes are pinned in test_php_template_scope.py. The same trap bit the FIXTURE twice: the AdminRoot.vue fixture's comment spelled out the wrapper element it does not have, the gate matched the COMMENT and accepted the file, and the "#227 is fixed" assertion passed IDENTICALLY with the fix reverted. Caught by mutation-checking, not by reading. All three mutations now go red: * classifier forced to "owns document" -> fragment assertions fail * settings-surface exclusion removed -> #227 assertion fails * PHP arm removed -> the standalone.php TP is lost Tests: 4 new fixtures (a document-owning template with and without a bypass anchor; a mount point; a settings surface), 4 new assertions in test_gate_monitoring_and_skiplink.sh (16/16 green), and a new 15-case test_php_template_scope.py. The "not reported" assertions run in the `accidental/` tree, where the gate is demonstrably firing in the same run, so an absence is a decision and never an empty scope. A missing helper now reports SKIP-wiring, not PASS (#147): without the classifier every template reads as a fragment and the whole PHP arm would evaporate into a green. Refs #214, #216, #227
…e-19's leaked `set -e` Adopts the gate-19 / #249 signalling convention, and fixes two ways the PHP arm could have gone falsely green. 1. THE EXIT BYTE WAS THE ANSWER, AND A CRASH SHARES ITS VALUE. `--owns-document` returned 0=owns / 1=fragment. A helper that CRASHES also exits 1. Every template would have classified as a fragment, the whole PHP arm would have evaporated, and gate-38 would have reported PASS having inspected nothing. The answer now comes from STDOUT — one `--classify` call for the whole set, printing `<path>: page-root|fragment` — and a non-zero exit is a wiring failure, reported as SKIPPED with stderr KEPT rather than discarded. Also one python process instead of one per template. 2. A LATENT BUG IN main(), found by the assertion above. gate-19's block (line ~1901) turns `set -e` ON and leaves it on for every gate after it, though this script's header sets only `set -u`. The first run of the crash test did not report a falsely-green gate — it reported NO GATE AT ALL: the non-zero helper killed the entire runner mid-sweep, 21 later gates silently unreported, and the run ended on the abort guard. The call is now wrapped in `set +e` with the caller's flag restored. Tests: 2 new wiring assertions (classifier MISSING -> SKIPPED; classifier CRASHING -> SKIPPED), 18/18 green. The crash assertion is the one that found the `set -e` leak; without it that failure mode is invisible, because an aborted run's PASS lines read exactly like a clean run's. Merged origin/main rather than rebasing: the branch is shared and the fleet force-push guard is right to refuse a history rewrite.
…adopt the errexit invariant TWO SCOPE DEFINITIONS BECOME ONE (#225 / #261). This arm had its own `find templates appinfo/templates -name '*.php'`, which is `_a11y_markup_files` minus its exclusions — so a generated `phpmetrics/` or a `vendor/` template would have been audited HERE and nowhere else in the a11y family. gate-38 now filters the shared enumeration to `*.php`. There is deliberately no third definition. ERREXIT (#243). Dropped the `case $- in *e*) … set -e` restore dance. main established that errexit is OFF for the whole script and nothing may turn it on — a trailing `set -e` is an unconditional ENABLE, not a restore. My workaround was written before that landed and is now obsolete; the underlying bug is fixed upstream, and test_gate_errexit_discipline.sh enforces the rule. Verified after the merge: skip-link control pairs 18/18, errexit discipline ALL PASS.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…224, #226, #230, #235, #236, #266) (#269) * fix(gates): nine checkers matched prose, not code — one shared scope, 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. * fix(gate-34,gate-48): a guard is not a second dialog, and an FQCN attribute 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 * fix(source_scope): `</script bar>` ends a script, and the mask must know 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
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…o full of markup, and three reported PASS over a crashed checker (#272) * fix(gates 35,40,42,44): four a11y gates excused themselves from a repo full of markup, and three reported PASS over a crashed checker Measured at package sha cdfbd7a against opencatalogi (93 .vue) and nldesign (zero .vue, one PHP template), one textbook true positive planted per gate in BOTH — the asymmetry that made #225/#261 possible. All 11 gates in the 34-44 band fired and named the plant in both arms, and all returned to their exact prior verdict on removal. Two defects survive that. 1. FOUR GATES GO `na` ON A TEMPLATES-ONLY REPO Gates 35, 40, 42 and 44 still guarded on `[ -d src ]` while 34/36/37/39/43 had moved to `_a11y_has_markup_dir`, and the central applicability table listed the whole family under `[ -d src ]`. On a repo with a `templates/` full of markup and no `src/`, same run, same files: gate-34/36/37/38/39/41/43 ran; four of them FAILED on the plants gate-35/40/42/44 NOT APPLICABLE — "this repo ships no frontend, so there is no .vue/.js/.ts source for this gate to inspect" `na` is the one verdict that removes a gate from coverage accounting, and the reason was contradicted by the same run's own output three lines above it. No fleet app is templates-only today; nldesign is one `rm` away, since its `src/` holds a single `manifest.json` — the exact shape that made twelve gates pass over nothing in #225. The guards now call `_a11y_has_markup_dir`, and the applicability declaration calls THE SAME FUNCTION rather than restating it, so the two cannot drift again. No third scope definition was added. 2. A CRASHED CHECKER REPORTED PASS (#147 / #249) — gates 40, 42, 44 With a `python3` on PATH that exits 1 on every call, run against opencatalogi: gate-40 PASS gate-42 PASS gate-44 PASS <- the three inline ones gate-34/37/38/39/41/43 SKIPPED (wiring) <- the six behind a helper gate-40 printed PASS over the 13 real findings it had reported one run earlier. gate-40 discarded its status with `2>/dev/null || true`; 42 and 44 ran per-file inline heredocs and never had one. 42 and 44 move to scripts/lib/check_link_text.py and scripts/lib/check_autocomplete.py — one interpreter for the whole file set, findings on stdout, exit code as a status — and 40 gains the same return-code guard. FOUND WHILE WRITING THE TESTS * gate-44 judged an input on the FIRST of name/id/v-model and stopped, so `<input id="e" type="text" name="email">` — the plainest textbook case this gate has — passed. Fleet effect, measured across 15 repos: openregister 0 -> 1 (an OpenAI Organization ID field), pipelinq 4 -> 5 (a "Colleague email" field). Both genuine, nothing lost. * gates 35, 36 and 44 read attribute values out of DOUBLE QUOTES ONLY. `tabindex='5'`, `alt=''` and `name='telephone'` render identically and reported PASS in both arms. Zero occurrences in the fleet today, which is why they could sit there indefinitely. * `[^>]*` in gates 42 and 44: a `>` inside an attribute value is not the end of a tag — the parse that hid 19 buttons from gate-39 (#259, #198, #236). * gates 42 and 44 scanned RAW text, so a commented-out `<a>click here</a>` or `<input name="email">` counted. That is gate-64's defect (#184), the one gate-38 (#247) and gate-41 (#266) each shipped a fix for. MEASURED AFTER, NOT ONLY BEFORE * 15 repos, gates 34-44, before vs after: every verdict and every finding count identical except the two new gate-44 true positives above. The rewrites of 42 and 44 removed nothing. * opencatalogi and nldesign return to their exact pre-plant baselines. * ARM 4 of test_gate_a11y_markup_scope.sh was mutation-checked: reverting gate-42's guard to `[ -d src ]` turns it red with the finding it was written for. TESTS * scripts/lib/test_check_link_text.py, test_check_autocomplete.py — 32 assertions; every relaxation ships with the true positive it must not swallow, comment/script exclusions ship with their positive control, and each ends with the whole PRE-FIX checker replayed as the mutant, asserting it answers DIFFERENTLY on every fixture. * test_gate_a11y_helper_wiring.sh gains gates 39, 40, 42, 44 (39 was wired correctly but never listed, so nothing held it to that) — 70 assertions. * test_gate_a11y_markup_scope.sh gains ARM 4, the templates-only repo. * Full discovered suite: 49 passed, 0 failed, 2 pre-existing quarantines. tests/test-hydra-gates-bin.sh: 59 passed, 0 failed. * fix(test): SC2194 — the case word was the constant, not the subject `case " 38 45 " in *" ${_g} "*)` matches a constant against a pattern built from the variable, which is the comparison written backwards. It happened to work, and ShellCheck is right that it reads as a mistake. Verified with shellcheck 0.10.0 at full severity: clean.
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.
Closes #214. Closes #216. Closes #227.
The defect
WCAG 2.4.1 (Bypass Blocks) is a property of a document. gate-38 was asking it of two shapes that own no document:
templates/**/*.phpTemplaterenderer substitutes the template into core's page. Measured: all 30 app templates in the fleet are fragments — not one emits<html>,<head>or<body>. The typical body is literally<div id="procest-settings"></div>.AdminRoot.vue<CnAdminSettingsShell>is a stack of<NcSettingsSection>s, not an app shell (#227).8 templates across 6 apps failed this way (nldesign, opencatalogi, docudesk, larpingapp, procest ×2, openbuild), plus 2
AdminRoot.vue.The remedy was an a11y regression
The only way to close the finding was to add a second "skip to content" anchor pointing into the middle of a page that already has one — announced to every screen-reader user ahead of core's real one. A WCAG 2.4.1 regression demanded by a WCAG 2.4.1 gate. That is why these findings could never be closed honestly.
What still fails
src/App.vue/src/views/App.vuewith a bespoke shell**/*Root.vuethat is not a settings surfacetemplates/settings/, and now asks every app template the question. A standalone/PublicPage template is covered where before it was not.The classifier is not a grep, and that is load-bearing
The first cut was
grep -iE '<(html|body)\b', and the first fixture it met defeated it: the fixture's own explanatory comment contained<html>, so a bare mount point classified as a page root and the fix silently did nothing. Same defect as gate-64 — a checker that greps a string literal misses every constant and matches every comment, failing both ways at once.scripts/lib/php_template_scope.pyclassifies emitted markup, with PHP regions and HTML comments removed. Both comment shapes are pinned in tests.The same trap bit the fixture, twice
The
AdminRoot.vuefixture's comment spelled out the wrapper element it does not have. The gate matched the comment, accepted the file, and the "#227 is fixed" assertion passed identically with the fix reverted. Caught by mutation-checking, not by reading.Mutation check — all three go red
standalone.phptrue positive is lostTests
test_gate_monitoring_and_skiplink.sh— 16/16 greentest_php_template_scope.py— 15 casesEvery "not reported" assertion runs in the
accidental/tree, where the gate is demonstrably firing in the same run, so an absence is a decision and never an empty scope.A missing helper now reports SKIP-wiring, not PASS (#147).