fix(hydra-gates): gate-64 read spelling, not code — 2 false REDs and 1 false GREEN - #184
Merged
Merged
Conversation
…1 false GREEN check_apphost_autoload_prelude.py judged ADR-040 compliance from raw text, which was wrong in both directions. FALSE RED — the app id as a class constant has_prelude() required a quoted 'openregister' INSIDE registerAutoloading(...). doriath passes it as self::OPENREGISTER_APP_ID (lib/AppInfo/OpenRegisterAutoloader.php), calls that prelude at Application.php:145 and only references Bootstrap at :150 — correct, and reported as a violation. Constants defined to 'openregister' in the same composition root are now resolved. FALSE RED — comments counted as code The loadApp rule matched doriath's own prose explaining that the code deliberately does NOT use loadApp(). Both occurrences under its lib/AppInfo/ are comments. FALSE GREEN — a commented-OUT prelude counted as a prelude The more dangerous direction, and the reason this is a strip rather than a targeted patch: an app whose prelude had been commented out passed gate-64. All rules now read comment-stripped source; the SUPPRESSION annotation still reads raw text, because it is authored as a comment. `#[` is preserved as a PHP 8 attribute rather than treated as a `#` comment. Measured across the 14 repos that consume these gates: doriath 2 findings -> 0, while procest, openbuild and scholiq still FAIL on genuine adoptions with no prelude anywhere in lib/AppInfo/ — the gate keeps its teeth. 6 tests added. Against the pre-fix checker 4 of them fail, so they are shown capable of failing rather than merely passing.
This was referenced Aug 6, 2026
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
This was referenced Aug 8, 2026
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 was referenced Aug 8, 2026
rubenvdlinde
added a commit
that referenced
this pull request
Aug 9, 2026
…ver a dead interpreter, and three could not see the defect they exist for (#280) * fix(gates 45-55): eleven gates passed over an unopened scope, eight over a dead interpreter, and three could not see the defect they exist for Every gate in this band was given ONE textbook true positive of exactly what it exists to catch, planted in a real fleet repo, then removed again. Where a gate could not fail, it was repaired; where it could, the plant is now a regression test. Measured at package sha 34370f6. ## 1. All eleven reported PASS over a scope they never opened (#242/#240/#258/#268) On a README-only diff against larpingapp, gates 45-55 printed eleven PASS lines and the summary read "53 of 53 applicable gates ran". Not one of them had opened a file. Gates 4/6/7/19/25/28/62/63 have answered the identical situation with NOT APPLICABLE since #268; this band never adopted it. Gates 47 and 48 are the sharper case: they can only answer a question about a CHANGE SET, so on every builder full-repo run in the fleet — no base ref at all — they printed a co-change verdict they had not formed. ## 2. Eight reported PASS over a crashed interpreter (#147/#249/#262) A planted defect only fires when the gate runs, so no plant can see this. With a `python3` on PATH that exits 1 on every call, on a tree carrying real findings: gate-46 PASS — over the 277 unresolved @SPEC findings, across 104 distinct targets, it had reported one run earlier on the same files gate-47 PASS — on the same diff where it had just reported FAIL gate-45/49/50 PASS (`2>/dev/null` discarded status and traceback) gate-51/54/55 PASS (`|| true` discarded the status) gate-52 FAIL — "1 custom-widget finding(s)", a fabricated finding: the helper returned its COUNT as its exit status, the same channel Python uses for a traceback (#209). The count was also clamped to 99 to fit in a byte. It now prints `findings=N` on stdout and exits boolean; no `findings=` line means the helper died. gate-54 was the quietest: its advisory WARN half reads the same log, so a dead helper silenced both halves at once. ## 3. gate-45 was the residue of #272's fix (.github#274) #272 migrated gates 35/40/42/44 off `[ -d src ]` onto `_a11y_has_markup_dir` and left the twelfth member of the family behind. On a templates-only app gate-45 reported NOT APPLICABLE — "this repo ships no frontend" — over a `<style>` block with `transition:` and no reduced-motion fallback, in the same file gate-43 FAILED on in the same run. `na` is the one verdict that removes a gate from coverage accounting. The regression test was already written and gate-45 was excluded from it by name, with a comment explaining why. Removing the name from ARM 4's skip list in test_gate_a11y_markup_scope.sh IS the test; it fails against 34370f6. ## 4. gate-47: prose satisfied it, and a qualified attribute did not `_ANNOTATION_RE` was an unanchored alternation of string literals, and it was wrong in both directions from that one regex — the pairing #269 found in gate-48 and never carried to its sibling. FALSE POSITIVE rewording ONE docblock sentence that merely NAMES the annotation ("becomes `@NoAdminRequired` again, paired with a real ownership check") made the gate demand a test co-change. A gate satisfiable by prose manufactures the appearance of a security review (#191). FALSE NEGATIVE `#[\OCP\AppFramework\Http\Attribute\NoAdminRequired]` was invisible. A commit adding exactly that to a controller — opening an admin-only endpoint to every authenticated user — with no test in the diff reported PASS. Now position-anchored, by the same rule check_csrf_removal.py already used. ## 5. gate-50: a false positive and a false negative in the same regex FALSE NEGATIVE the app-id argument had to be a QUOTED STRING, so every read written the fleet-standard way — `getValueString( Application::APP_ID, 'listing_register', '')` — was invisible. Identical code with `'larpingapp'` FAILED. Same family as #184. 7 security-relevant reads across 5 repos sit behind a constant today. FALSE POSITIVE the empty-compare guard required a closing paren immediately after the empty string, so the correct compound guard `if ($reg === '' || $sch === '')` was reported as unguarded — twice, on code the gate was asking for. A guard that is a boolean `return` rather than an `if` was rejected too. Both directions are now asserted, including the opencatalogi#86 shape that mixes them: one read guarded, the next unguarded two lines later. ## 6. gate-53 did not block the PR that creates larpingapp#286 Reintroducing #286 exactly — the check-in tab deleted from src/manifest.json, `EventRoster` left registered in src/registry.js — reported PASS. Direction 1 of the registry cross-reference stays advisory for LEGACY orphans, correctly: the gate cannot tell "wire it" from "delete it". But when the DIFF ITSELF removed the last reference it can, and that finding now blocks. Pre-existing orphans are untouched (larpingapp carries one today), so this is prevention, not a burn-down list nobody can close. ## Verified working, repaired nothing gate-46 (dangling file, dangling fragment, valid anchor), gate-48 (short and fully-qualified attribute removal; a comment reword correctly stays green), gate-49, gate-51 (title, description and nested items.properties independently), gate-52's ratchet (growth fails, shrink passes), gate-54 (flat, nested and $ref-carrying), gate-55. ## Deliberately NOT enforced `title == key` on a schema property is a real gate-51 defect — the renderer uses `prop.title || key`, so the user sees the raw technical key. Measured across 10 repos: 148 occurrences, ALL of them in softwarecatalog, where they are VNG-standardised element names (`identifier`, `type`, `name`) that must not be renamed. Enforcing it would produce 148 findings with no legitimate end state in the one repo that has them. Reported rather than gated (#252). ## Divergence to reconcile gate-45 now answers an empty in-scope set with `na`; gate-40 answers it with PASS, by a deliberate choice in #272 that cited the invariant test this PR reworks. The invariant now discriminates on the REASON — the applicability table's own phrasing must not appear once its prerequisite holds — so both behaviours are expressible. The family should pick one. ## Testing New: hydra-gates/scripts/lib/test_gate_45_to_55_acceptance.sh — 31 arms across six families, discovered by run-helper-suites.sh. Against the package as merged on main it fails 20 of 31; the 11 that pass are exactly the anti-widening and no-regression controls. Every mutation asserts its anchor is present before it plants. Repos used, chosen for different shapes: larpingapp (register-owning, manifest-driven, ships registry.js), nldesign (PHP templates, no .vue, no register), doriath (ships no phpcs SpecTagSniff — the #246 control, held at 81 findings across 46 targets before and after the plant), openconnector (41 register files). Full package suite: 52 discovered suites pass, 2 quarantined as documented; 60/60 entry-point invariants. * fix(gate-50): the fail-mode window started where the call began, not where it ended The constant-app-id fix in the parent commit made procest's config reads visible for the first time and immediately produced 3 findings on lib/Service/AiService.php — all three false positives, and both causes are ordinary code the window could never have seen: multi-line call PHPCS formats each read across five lines. Two of them plus a blank line put the guard on the ELEVENTH line, one outside a window counted from the line the match BEGAN on. The guard being missed is a textbook `if (empty($registerId) === true || empty($schemaId) === true) { $this->logger->warning(...); return; }` (AiService.php:580, :967). same-line guard `'ai_api_key_set' => ...getValueString(APP_ID, 'ai_api_key', '') !== ''` handles the empty default ON the match line, and the window started after it (AiService.php:710). The window now anchors to the END of the call expression — parentheses balanced forward from the `(` — and includes the remainder of that line. A single-line read keeps exactly the ten lines it always had. Caught by a before/after sweep of 12 fleet repos: 26 of 121 verdicts changed, 25 of them PASS -> NOT APPLICABLE (the truthfulness correction), and this was the only one that changed to FAIL. procest is PASS again, correctly. Three arms added: the multi-line shape, the same-line shape, and the reverse control — the same multi-line shape with the guard DELETED must still FAIL, so the window cannot have been widened until the gate finds nothing. Also: shellcheck SC2181 in gate-45's new status check, and a file-scoped SC2016 suppression for the acceptance suite, whose PHP fixtures are single-quoted on purpose.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 9, 2026
… an empty scope, and gate-10 excused itself from four real findings (#275) * fix(gates): gate-11 was dead fleet-wide, and eight gates passed over an empty scope Every gate in the 1–11 band was put through one acceptance test: plant ONE textbook true positive of exactly what the gate exists to catch, in a real fleet repo, and require the gate to FAIL and NAME it; then remove the plant and require the prior verdict back. Measured against gate package cdfbd7a. gate-11 admin-router was DEAD ----------------------------- It read four hard-coded paths — src/router/index.{js,ts}, src/router.{js,ts}. ONE fleet app of fifteen (softwarecatalog) has a file at any of them; the other fourteen build their router in src/main.js. Proof it was dead rather than merely unexercised: the doriath c7c72e9 defect re-planted verbatim into larpingapp's real router — routes.push({ path: '/settings', component: AdminRoot }) — reported PASS. The identical line in src/router.js reported FAIL. The detection logic was fine; the gate never opened the file. Routers are now discovered (anything under src/ that constructs one), the four legacy paths are kept, and a repo with no router is `na`. The path rule moved into check_admin_router.py, which resolves the enclosing route object: a bare `path: '/settings'` grep would have flagged openconnector's ADR-079 hand-off — the remediation — as the defect. Fleet churn: 0 findings across 16 repos. EIGHT GATES REPORTED PASS OVER A SCOPE THEY NEVER OPENED -------------------------------------------------------- A README-only commit run with --scope-to-diff produced PASS from gates 1, 2, 3, 5, 8, 9, 10 and 11 — four of them authorization gates. Gates 4/6/7 already said NOT APPLICABLE for the identical situation, which is what made the other eight readable as a result rather than an absence. All eight now say `na` with a reason (`na`, not `structural`, per #268: an empty ADR-020 scope is not a gap the author can close). gate-9 semantic-auth flagged the ESCALATION BRANCH -------------------------------------------------- The `!isAdmin` block was searched for a denial token AT ANY DEPTH, conflating two opposite postures: if (!isAdmin) { return 403; } admin IS required -> true positive if (!isAdmin) { if (!owner) 403; } admin NOT required -> false positive In the second a non-admin OWNER proceeds. Live on docudesk SigningController::cancelRequest (its own docudesk#100 fix) and procest InspectionChecklistController::submitResult. The remedy gate-9 printed for both — remove @NoAdminRequired, or switch to #[AuthorizedAdminSetting] — would have made per-user endpoints admin-only and deleted the owner checks' reason to exist. Now only an UNCONDITIONAL denial counts. Fleet: 28 -> 26 findings; the two removed are exactly those, and larpingapp's genuine CharactersController::downloadPdf finding is preserved. gate-2 forbidden-patterns failed in BOTH directions (#184) ----------------------------------------------------------- Six raw-text greps. Invisible: `var_dump ($x)` (PHP allows whitespace before the argument list), `die;` (a LANGUAGE CONSTRUCT, not a function), and `exit` in every form — die's exact synonym, absent from the pattern list, so one name was banned and the other left open. Reported: a comment saying "never use var_dump( here", and the string literal "select dd(x)". Now judged over a comment- and string-masked copy. `: never` exempts exit/die — a TYPE PHP itself checks, deliberately not the @SuppressWarnings docblock sitting above the one fleet instance, because a docblock is the load-bearing prose #196 was about. Fleet churn: 1 -> 1. gate-8 unsafe-auth-resolver used HARD-CODED INDENTATION ------------------------------------------------------- Body extraction terminated on `/^ \}/`, the catch block on `/^ \}/`. On a tab-indented file neither matches, so "the body" ran to end of file and a resolver that correctly RETHROWS was reported as a fail-open because an unrelated cache method further down returned null from its own catch. The apparent detection of tab-indented fail-opens was the same over-capture by luck. Now brace-walked over a comment-masked copy; the procest ZgwService fail-closed shapes stay excluded by construction rather than by indentation. gate-10 initial-state knew only the single-line shape ----------------------------------------------------- `getElementById(...)...\.dataset` requires lookup and read on ONE line, so the TWO-STEP form — what the doriath line becomes after any refactor — plus querySelector and getAttribute('data-*') all reported PASS. Widened along that one axis only; NOT to a bare `.dataset`, which would flag every legitimate event.target.dataset in the fleet. Two measured exclusions, each a rule rather than a waiver: a dataset key the same file also WRITES is the component's own bookkeeping, and data-requesttoken is not IInitialState data (its accessor is getRequestToken(), so gate-10's remedy would not apply). Fleet churn: 0 -> 0. gate-7 no-admin-idor missed the FULLY-QUALIFIED attribute ---------------------------------------------------------- The look-back matched `#[NoAdminRequired` only. Under `#[\OCP\AppFramework\Http\Attribute\NoAdminRequired]` a textbook IDOR fell out of scope and reported PASS. No fleet file uses that spelling today, which is why it needed closing deliberately — a false negative on a security gate leaves no log. WHAT WAS VERIFIED AND LEFT ALONE -------------------------------- gate-3's #269 repair HOLDS in both directions: an inert `$unused = 1;` no longer closes it, and canonical fail-safe delegation no longer trips it. gate-5's #269 repair HOLDS: prose naming an attribute does not satisfy it, the `@auth admin-only <reason>` declaration does, and a too-short reason does not. gate-4 already distinguishes "no advisories" from "audit did not run". gate-6 detects a zero-caller authorization method and goes quiet on one caller. gate-7 is CORRECTLY correlated — .github#160's anti-correlation does not reproduce: a leak FAILs and all four correct-fix shapes PASS. docudesk EmlPreviewController::preview is a genuine IDOR (un-redacted EML by caller-supplied fileId, no ownership check). Regression tests land in run-helper-suites.sh's discovery path and are mutation-checked against WHOLE pre-fix files, not single-site reverts: the pre-fix runner fails 19 assertions of test_gate_1_11_empty_scope_is_na.sh (including the gate-11 dead-gate case), the pre-fix check_semantic_auth.py fails 3, the pre-fix check_no_admin_idor.py fails 1. Full-tree verdicts before/after are IDENTICAL on larpingapp and openregister, and differ on docudesk only by the intended gate-9 false positive. * test(gates): gate-5's empty-diff verdict is `na`, matching the suite's own comment test_gate_route_auth.sh asserted PASS for a package.json-only diff while its own comment said "a PASS here is scoping, not absence" — a fact PASS does not state and NOT APPLICABLE does. Reclassified per #268. 6b/6c remain the controls: the identical finding must still FAIL once the diff touches the controller or appinfo/routes.php. * fix(gates 5,10): a gate declared `na` over the frontend it was built to check, and gate-5 punted to a package it was standing in Three additions after the first commit, all measured against gate package 34370f6 and reported by the runner's own `[hydra-gates] gate package:` line. gate-10 DECLARED `na` OVER A REPO FULL OF ITS OWN SUBJECT MATTER ----------------------------------------------------------------- The surface was `src/` only. nldesign's `src/` holds exactly one file — `manifest.json` — and its entire hand-written frontend lives in `js/`. So gate-10 announced "this repo ships no frontend" over a repo whose `js/admin.js` does precisely what the gate exists to catch: var settingsEl = document.getElementById('nldesign-settings'); var tokenSets = JSON.parse(settingsEl.getAttribute('data-token-sets')); The doriath AdminRoot defect, two-step form, in an ADMIN settings script. Four real findings. `na` REMOVES A GATE FROM COVERAGE ACCOUNTING, so this did not merely under-report — it left the denominator. The surface is now `src/` + `js/` (Nextcloud's conventional shipped-script directory), minus `*.min.js`: a committed bundle is not authored code. Fleet total 0 -> 4, all four in nldesign, all four genuine. AND A SCOPE-BLINDNESS BUG IN MY OWN TWO-STEP RULE -------------------------------------------------- Widening the surface exposed it. The two-step rule matches by NAME across the whole file. In `js/admin.js`, line 1114 binds `var btn = document.getElementById('nldesign-save-btn')`, and that one line put EVERY `btn` in a 1700-line file into scope — including three `forEach(function (btn) {...})` callbacks whose `btn` is the clicked element, i.e. the component's own markup. Four false positives from one binding. Rather than implement JS scoping, an AMBIGUOUS NAME IS DROPPED: if an identifier is ever a function parameter, the file cannot tell the two apart and declines to guess. That can only under-report, which is the right direction for a rule whose false positives would otherwise bury the real finding. 7 raw hits -> 4, and the 4 that remain are the server-rendered reads. gate-5 CALLED A FILE ABSENT FROM INSIDE THE REPOSITORY CONTAINING IT --------------------------------------------------------------------- Nextcloud resolves a route name `A\B\C` against `OCA\<App>\A\B\C`, which PSR-4 maps to `lib/A/B/CController.php`. The resolver rooted every namespaced name at `lib/Controller/` — correct for `Settings\FileSettings`, wrong for `AppHost\Controller\GenericHealth`, whose file is `lib/AppHost/Controller/GenericHealthController.php`. Measured on openregister, the repository that SHIPS those classes. The derived path did not exist, `_apphost_serves` then matched the name, and both entries were filed as "served by the OpenRegister AppHost generic controller (ADR-040); its auth attribute lives in the openregister package and is NOT visible from this repository" inside openregister. The gate punted to another package while standing in it, so `AppHost\Controller\GenericHealth#index` and `GenericMetrics#index` had their auth posture judged by nobody. A derived path is a GUESS; both roots are tried and an existing one wins. Both now resolve, both are correctly annotated (`#[PublicPage]` + `#[NoCSRFRequired]`), and openregister's gate-5 count is unchanged at 10 — this surfaced two unjudged endpoints without inventing a finding. The AppHost CONSUMER control still classifies its four absent generics as ADR-040-unresolved. TWO ACCEPTANCE ARMS A PLANTED DEFECT CANNOT REACH -------------------------------------------------- A plant only fires when the gate RUNS, so two classes of defect are invisible to it. Both are now asserted: ARM 4 an ATTRIBUTE-ONLY change is a change. `_filter_preexisting` (gates 6, 7, 8 in this band) compares a method against the base ref and moves unchanged entries out of the verdict. Before #276 the comparison began at the `function NAME(` line, so ADDING `#[NoAdminRequired]` above an existing unguarded method left the body byte-identical and the finding was suppressed — the single edit that changes a method's auth posture was the one the filter could not see. Verified BOTH ways on one fixture: package cdfbd7a gives `[gate-7] no-admin-idor: PASS` with the finding in `.log.preexisting`; package 34370f6 gives FAIL. #276 fixed the shared helper; this keeps it fixed for gate-7, whose entire scope is decided by that attribute. ARM 5 a BROKEN INTERPRETER is `SKIPPED (wiring)`, never PASS. With python3 exiting 1, gates 2, 3, 10 and 11 all skip. gate-3 is the control: on the same fixture it FAILS a real stub `run()` when python3 works, so the skip is the gate losing a verdict it demonstrably had. TWO REPO SHAPES, NOT ONE ------------------------ gate-11's repair is proven on three: larpingapp (vue-router 3, `new VueRouter`), scholiq (vue-router 4 `createRouter`, AppHost-adopting) and nldesign (no router at all -> `na`, verified honest: no `createRouter`/`new VueRouter` anywhere in the repo). gate-5's resolver is proven on openregister (ships the generics) and an AppHost consumer (does not). Full-tree verdicts vs cdfbd7a: larpingapp and scholiq IDENTICAL; docudesk differs only by the intended gate-9 false-positive removal; nldesign gains gate-10 FAIL (4 real findings) and gate-11 `na` (previously a green over zero bytes). --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
rubenvdlinde
added a commit
that referenced
this pull request
Aug 9, 2026
…could not see the defect they exist to catch Acceptance test applied to all nine gates in the 56-64 band: plant one textbook true positive in a real fleet repo, require the gate to FAIL and NAME it, remove the plant, require the prior verdict back, and require a clean fixture to still pass. Five gates passed unchanged (56, 58, 60, 62, 63). Four did not. AN UNOPENED SCOPE IS NOT A PASS — gates 56, 57, 58, 59, 60, 61 -------------------------------------------------------------- #242/#240 established that a gate must not report PASS over a scope it never opened, and #268 that an empty ADR-020 scope is `na` rather than `structural`. Both were applied to gates 19, 25, 62 and 63 and to nothing else. Measured on shillinq, one docs-only commit, --scope-to-diff: [gate-56] register-handler-resolution: PASS <- 153 registers, 0 opened [gate-57] orphaned-write-capability: PASS <- 316 services, 0 opened [gate-58] e2e-networkidle: PASS <- 60 e2e files, 0 opened [gate-59] unclosable-gate: PASS <- lib/ untouched [gate-60] icon-vocabulary: PASS <- no manifest in the diff [gate-61] listener-work-placement: PASS <- all 15 out of scope [gate-62] store-plane: NOT APPLICABLE (already fixed) [gate-63] settings-surface: NOT APPLICABLE (already fixed) Six gates asserting a verdict about code the run had not looked at, beside two that had already learned not to — and --require-full-coverage cannot see a PASS, so nothing reported that six gates had gone quiet. All six now emit `na` with a reason naming the rule and the count of subjects that exist but were not inspected. GATE 59 COULD NOT RUN A FULL-TREE AUDIT AT ALL ---------------------------------------------- CHANGED_FILES is populated only under --scope-to-diff. Gate 59's guard read `grep -qE '^lib/.*\.php$'` against it unconditionally, so on every unscoped run the guard was false and the gate printed PASS having walked no PHP. That is #240's sentence — "a full-tree audit was the one mode this gate could never reach" — in a gate #240 did not visit. The scoping now applies only when the caller asked for it. Gate 61's unconditional --base is DELIBERATE by contrast (it is about new debt; the fleet's 149-registration backlog is a work-list). Switching it to --all was tried and reverted: the builder runs unscoped, so it would have surfaced the whole backlog as blocking findings on every build. Recorded in the code so the next reader does not repeat it. GATE 59 WAS WRONG IN BOTH DIRECTIONS AT ONCE (#184's shape) ----------------------------------------------------------- It caught the textbook case and then failed on the next four mutations, each reproduced against docudesk: false GREEN a COMMENTED-OUT setter counted as a write. `// TODO: setValueString('app','configuration_version',$v)` closed the finding — and that is the single most likely comment to sit beside a key nobody writes. The gate whose subject is "this guard never closes" was itself closed by a comment promising to close a guard. false GREEN `"key"` in double quotes was invisible on both sides. false POSITIVE read 'key' / write "key" — code that closes its gate correctly reported as never closing it, with no remedy available to the app but changing its quote style. false GREEN a key held in a class constant was invisible on both sides — and a constant is the idiomatic way to write a key used twice, which is the shape a CLOSABLE gate has. Fixed with the established remedies: source_scope.php_mask for the comment regions (offsets preserved; string contents kept, because the key literal is the evidence), both quote styles, and constant resolution that keeps read and write symmetric. The suppression keeps reading raw text because it is authored as a comment — and it now requires the key QUOTED ON THE MARKER'S OWN LINE, because the old four-line window spanned the read and so suppressed whatever was near it rather than what it named. Fleet sweep: 21 repos, zero verdict changes. GATE 57'S MCP SEAM DISSOLVED ON A LEADING BACKSLASH --------------------------------------------------- Both halves of the attribute seam (#200/#215) matched a namespace prefix as `(?:[A-Za-z_]\w*\s*\\+\s*)*` — every segment had to start with a letter. So the fully-qualified spelling, which is how you write a name with no `use` import for it, matched neither: registerServiceAlias('…::app', \OCA\App\Mcp\Impl::class) #[\OCA\OpenRegister\Mcp\Attribute\McpTool(name: 'createLead')] Either miss alone empties the seam and puts every #[McpTool] write method in the app back on the finding list — which is #200 verbatim, a finding whose only remedy is deleting a live, curated MCP write tool. Reproduced on shillinq with the two spellings side by side. The regression test's mutant restores BOTH pre-fix patterns together, because the seam needs both and reverting one alone reads as "the fix changed nothing". Fleet sweep: 21 repos, zero verdict changes — the trap is latent, not live. GATE 64 SAW ONE OF PHP'S THREE WAYS TO NAME A CLASS --------------------------------------------------- Rule 2 required a QUOTED literal, so of these four only the first was a finding; the other three were injected into larpingapp's register() and the gate reported OK for every one: class_exists('OCA\OpenRegister\AppHost\…\GenericHealthController') class_exists(\OCA\OpenRegister\AppHost\…\GenericHealthController::class) use …\GenericHealthController; class_exists(GenericHealthController::class) const AH = 'OCA\OpenRegister\AppHost\…'; class_exists(self::AH) That is #184's lesson in the file where it was learned. All four spellings now resolve through one resolver, so a spelling that escapes it escapes both rules rather than whichever was written second. A bare `use` import stays clean on purpose — `use` is a compile-time alias and does not autoload; flagging it would newly redden four repos for code that works. AND A GREEN GATE-64 WAS NOT EVIDENCE ABOUT THE WIDER PROBE ---------------------------------------------------------- The hard rule is scoped to OCA\OpenRegister\AppHost\, but the autoloader mechanism has nothing to do with AppHost: during register() the whole OCA\OpenRegister\ prefix is absent for any app sorting earlier, so ANY class_exists() on it answers FALSE and everything it guards silently never happens. Measured across apps-extra, no prelude present: larpingapp 3 Event\{DeepLinkRegistration,ObjectCreating,ObjectUpdating} — the last two carry larpingapp's server-authoritative skill-requirement / XP-budget enforcement on character writes, which therefore never registers hermiq 3 flow-node, leaf-provider and shareable-config registration nldesign 1 shareable-config registration Reported as a non-blocking NOTE, printed by the runner, and deliberately not a FAIL: this gate is not diff-scoped, so failing it would block every PR in three repos on code the PR did not touch — the trap check_store_and_settings_surface.py already records. A probe in boot() is NOT noted, because boot() runs after every app has registered. ALSO ---- Gate 64's log path was a hardcoded /tmp/hydra-gate-apphost-autoload-prelude .log — the shared-path non-determinism HYDRA_GATE_LOG_DIR exists to remove, left behind in one gate. TESTS ----- test_check_unclosable_gate.py NEW — gate 59 shipped with no helper suite at all; 23 cases, every arm of the matrix above plus the mutant that proves the comment mask is load-bearing test_gate_5661_empty_scope_is_not_a_pass.sh NEW — 28 assertions over gates 56-61: clean in-scope subject PASSes, empty scope is `na` with a reason and exit 0 under --require-full-coverage, and a planted true positive per gate still FAILs. Against the pre-fix package it reports 13 failures, naming the six PASSes test_check_apphost_autoload_prelude.py +14 cases test_check_orphaned_write_capability.py +5 cases test_check_listener_placement.py EXIT_EMPTY_SCOPE 51 discovered helper suites green (2 quarantined, unchanged). Full-tree run on shillinq before/after: verdicts byte-identical for all nine gates.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 9, 2026
…could not see the defect they exist to catch (#278) * fix(gates 56-64): six gates passed over an unopened scope, and three could not see the defect they exist to catch Acceptance test applied to all nine gates in the 56-64 band: plant one textbook true positive in a real fleet repo, require the gate to FAIL and NAME it, remove the plant, require the prior verdict back, and require a clean fixture to still pass. Five gates passed unchanged (56, 58, 60, 62, 63). Four did not. AN UNOPENED SCOPE IS NOT A PASS — gates 56, 57, 58, 59, 60, 61 -------------------------------------------------------------- #242/#240 established that a gate must not report PASS over a scope it never opened, and #268 that an empty ADR-020 scope is `na` rather than `structural`. Both were applied to gates 19, 25, 62 and 63 and to nothing else. Measured on shillinq, one docs-only commit, --scope-to-diff: [gate-56] register-handler-resolution: PASS <- 153 registers, 0 opened [gate-57] orphaned-write-capability: PASS <- 316 services, 0 opened [gate-58] e2e-networkidle: PASS <- 60 e2e files, 0 opened [gate-59] unclosable-gate: PASS <- lib/ untouched [gate-60] icon-vocabulary: PASS <- no manifest in the diff [gate-61] listener-work-placement: PASS <- all 15 out of scope [gate-62] store-plane: NOT APPLICABLE (already fixed) [gate-63] settings-surface: NOT APPLICABLE (already fixed) Six gates asserting a verdict about code the run had not looked at, beside two that had already learned not to — and --require-full-coverage cannot see a PASS, so nothing reported that six gates had gone quiet. All six now emit `na` with a reason naming the rule and the count of subjects that exist but were not inspected. GATE 59 COULD NOT RUN A FULL-TREE AUDIT AT ALL ---------------------------------------------- CHANGED_FILES is populated only under --scope-to-diff. Gate 59's guard read `grep -qE '^lib/.*\.php$'` against it unconditionally, so on every unscoped run the guard was false and the gate printed PASS having walked no PHP. That is #240's sentence — "a full-tree audit was the one mode this gate could never reach" — in a gate #240 did not visit. The scoping now applies only when the caller asked for it. Gate 61's unconditional --base is DELIBERATE by contrast (it is about new debt; the fleet's 149-registration backlog is a work-list). Switching it to --all was tried and reverted: the builder runs unscoped, so it would have surfaced the whole backlog as blocking findings on every build. Recorded in the code so the next reader does not repeat it. GATE 59 WAS WRONG IN BOTH DIRECTIONS AT ONCE (#184's shape) ----------------------------------------------------------- It caught the textbook case and then failed on the next four mutations, each reproduced against docudesk: false GREEN a COMMENTED-OUT setter counted as a write. `// TODO: setValueString('app','configuration_version',$v)` closed the finding — and that is the single most likely comment to sit beside a key nobody writes. The gate whose subject is "this guard never closes" was itself closed by a comment promising to close a guard. false GREEN `"key"` in double quotes was invisible on both sides. false POSITIVE read 'key' / write "key" — code that closes its gate correctly reported as never closing it, with no remedy available to the app but changing its quote style. false GREEN a key held in a class constant was invisible on both sides — and a constant is the idiomatic way to write a key used twice, which is the shape a CLOSABLE gate has. Fixed with the established remedies: source_scope.php_mask for the comment regions (offsets preserved; string contents kept, because the key literal is the evidence), both quote styles, and constant resolution that keeps read and write symmetric. The suppression keeps reading raw text because it is authored as a comment — and it now requires the key QUOTED ON THE MARKER'S OWN LINE, because the old four-line window spanned the read and so suppressed whatever was near it rather than what it named. Fleet sweep: 21 repos, zero verdict changes. GATE 57'S MCP SEAM DISSOLVED ON A LEADING BACKSLASH --------------------------------------------------- Both halves of the attribute seam (#200/#215) matched a namespace prefix as `(?:[A-Za-z_]\w*\s*\\+\s*)*` — every segment had to start with a letter. So the fully-qualified spelling, which is how you write a name with no `use` import for it, matched neither: registerServiceAlias('…::app', \OCA\App\Mcp\Impl::class) #[\OCA\OpenRegister\Mcp\Attribute\McpTool(name: 'createLead')] Either miss alone empties the seam and puts every #[McpTool] write method in the app back on the finding list — which is #200 verbatim, a finding whose only remedy is deleting a live, curated MCP write tool. Reproduced on shillinq with the two spellings side by side. The regression test's mutant restores BOTH pre-fix patterns together, because the seam needs both and reverting one alone reads as "the fix changed nothing". Fleet sweep: 21 repos, zero verdict changes — the trap is latent, not live. GATE 64 SAW ONE OF PHP'S THREE WAYS TO NAME A CLASS --------------------------------------------------- Rule 2 required a QUOTED literal, so of these four only the first was a finding; the other three were injected into larpingapp's register() and the gate reported OK for every one: class_exists('OCA\OpenRegister\AppHost\…\GenericHealthController') class_exists(\OCA\OpenRegister\AppHost\…\GenericHealthController::class) use …\GenericHealthController; class_exists(GenericHealthController::class) const AH = 'OCA\OpenRegister\AppHost\…'; class_exists(self::AH) That is #184's lesson in the file where it was learned. All four spellings now resolve through one resolver, so a spelling that escapes it escapes both rules rather than whichever was written second. A bare `use` import stays clean on purpose — `use` is a compile-time alias and does not autoload; flagging it would newly redden four repos for code that works. AND A GREEN GATE-64 WAS NOT EVIDENCE ABOUT THE WIDER PROBE ---------------------------------------------------------- The hard rule is scoped to OCA\OpenRegister\AppHost\, but the autoloader mechanism has nothing to do with AppHost: during register() the whole OCA\OpenRegister\ prefix is absent for any app sorting earlier, so ANY class_exists() on it answers FALSE and everything it guards silently never happens. Measured across apps-extra, no prelude present: larpingapp 3 Event\{DeepLinkRegistration,ObjectCreating,ObjectUpdating} — the last two carry larpingapp's server-authoritative skill-requirement / XP-budget enforcement on character writes, which therefore never registers hermiq 3 flow-node, leaf-provider and shareable-config registration nldesign 1 shareable-config registration Reported as a non-blocking NOTE, printed by the runner, and deliberately not a FAIL: this gate is not diff-scoped, so failing it would block every PR in three repos on code the PR did not touch — the trap check_store_and_settings_surface.py already records. A probe in boot() is NOT noted, because boot() runs after every app has registered. ALSO ---- Gate 64's log path was a hardcoded /tmp/hydra-gate-apphost-autoload-prelude .log — the shared-path non-determinism HYDRA_GATE_LOG_DIR exists to remove, left behind in one gate. TESTS ----- test_check_unclosable_gate.py NEW — gate 59 shipped with no helper suite at all; 23 cases, every arm of the matrix above plus the mutant that proves the comment mask is load-bearing test_gate_5661_empty_scope_is_not_a_pass.sh NEW — 28 assertions over gates 56-61: clean in-scope subject PASSes, empty scope is `na` with a reason and exit 0 under --require-full-coverage, and a planted true positive per gate still FAILs. Against the pre-fix package it reports 13 failures, naming the six PASSes test_check_apphost_autoload_prelude.py +14 cases test_check_orphaned_write_capability.py +5 cases test_check_listener_placement.py EXIT_EMPTY_SCOPE 51 discovered helper suites green (2 quarantined, unchanged). Full-tree run on shillinq before/after: verdicts byte-identical for all nine gates. * fix(gates 56-64): six gates passed over an unopened scope, and three could not see the defect they exist to catch Acceptance test applied to all nine gates in the 56-64 band: plant one textbook true positive in a real fleet repo, require the gate to FAIL and NAME it, remove the plant, require the prior verdict back, and require a clean fixture to still pass. Five gates passed unchanged (56, 58, 60, 62, 63). Four did not. AN UNOPENED SCOPE IS NOT A PASS — gates 56, 57, 58, 59, 60, 61 -------------------------------------------------------------- #242/#240 established that a gate must not report PASS over a scope it never opened, and #268 that an empty ADR-020 scope is `na` rather than `structural`. Both were applied to gates 19, 25, 62 and 63 and to nothing else. Measured on shillinq, one docs-only commit, --scope-to-diff: [gate-56] register-handler-resolution: PASS <- 153 registers, 0 opened [gate-57] orphaned-write-capability: PASS <- 316 services, 0 opened [gate-58] e2e-networkidle: PASS <- 60 e2e files, 0 opened [gate-59] unclosable-gate: PASS <- lib/ untouched [gate-60] icon-vocabulary: PASS <- no manifest in the diff [gate-61] listener-work-placement: PASS <- all 15 out of scope [gate-62] store-plane: NOT APPLICABLE (already fixed) [gate-63] settings-surface: NOT APPLICABLE (already fixed) Six gates asserting a verdict about code the run had not looked at, beside two that had already learned not to — and --require-full-coverage cannot see a PASS, so nothing reported that six gates had gone quiet. All six now emit `na` with a reason naming the rule and the count of subjects that exist but were not inspected. GATE 59 COULD NOT RUN A FULL-TREE AUDIT AT ALL ---------------------------------------------- CHANGED_FILES is populated only under --scope-to-diff. Gate 59's guard read `grep -qE '^lib/.*\.php$'` against it unconditionally, so on every unscoped run the guard was false and the gate printed PASS having walked no PHP. That is #240's sentence — "a full-tree audit was the one mode this gate could never reach" — in a gate #240 did not visit. The scoping now applies only when the caller asked for it. Gate 61's unconditional --base is DELIBERATE by contrast (it is about new debt; the fleet's 149-registration backlog is a work-list). Switching it to --all was tried and reverted: the builder runs unscoped, so it would have surfaced the whole backlog as blocking findings on every build. Recorded in the code so the next reader does not repeat it. GATE 59 WAS WRONG IN BOTH DIRECTIONS AT ONCE (#184's shape) ----------------------------------------------------------- It caught the textbook case and then failed on the next four mutations, each reproduced against docudesk: false GREEN a COMMENTED-OUT setter counted as a write. `// TODO: setValueString('app','configuration_version',$v)` closed the finding — and that is the single most likely comment to sit beside a key nobody writes. The gate whose subject is "this guard never closes" was itself closed by a comment promising to close a guard. false GREEN `"key"` in double quotes was invisible on both sides. false POSITIVE read 'key' / write "key" — code that closes its gate correctly reported as never closing it, with no remedy available to the app but changing its quote style. false GREEN a key held in a class constant was invisible on both sides — and a constant is the idiomatic way to write a key used twice, which is the shape a CLOSABLE gate has. Fixed with the established remedies: source_scope.php_mask for the comment regions (offsets preserved; string contents kept, because the key literal is the evidence), both quote styles, and constant resolution that keeps read and write symmetric. The suppression keeps reading raw text because it is authored as a comment — and it now requires the key QUOTED ON THE MARKER'S OWN LINE, because the old four-line window spanned the read and so suppressed whatever was near it rather than what it named. Fleet sweep: 21 repos, zero verdict changes. GATE 57'S MCP SEAM DISSOLVED ON A LEADING BACKSLASH --------------------------------------------------- Both halves of the attribute seam (#200/#215) matched a namespace prefix as `(?:[A-Za-z_]\w*\s*\\+\s*)*` — every segment had to start with a letter. So the fully-qualified spelling, which is how you write a name with no `use` import for it, matched neither: registerServiceAlias('…::app', \OCA\App\Mcp\Impl::class) #[\OCA\OpenRegister\Mcp\Attribute\McpTool(name: 'createLead')] Either miss alone empties the seam and puts every #[McpTool] write method in the app back on the finding list — which is #200 verbatim, a finding whose only remedy is deleting a live, curated MCP write tool. Reproduced on shillinq with the two spellings side by side. The regression test's mutant restores BOTH pre-fix patterns together, because the seam needs both and reverting one alone reads as "the fix changed nothing". Fleet sweep: 21 repos, zero verdict changes — the trap is latent, not live. GATE 64 SAW ONE OF PHP'S THREE WAYS TO NAME A CLASS --------------------------------------------------- Rule 2 required a QUOTED literal, so of these four only the first was a finding; the other three were injected into larpingapp's register() and the gate reported OK for every one: class_exists('OCA\OpenRegister\AppHost\…\GenericHealthController') class_exists(\OCA\OpenRegister\AppHost\…\GenericHealthController::class) use …\GenericHealthController; class_exists(GenericHealthController::class) const AH = 'OCA\OpenRegister\AppHost\…'; class_exists(self::AH) That is #184's lesson in the file where it was learned. All four spellings now resolve through one resolver, so a spelling that escapes it escapes both rules rather than whichever was written second. A bare `use` import stays clean on purpose — `use` is a compile-time alias and does not autoload; flagging it would newly redden four repos for code that works. AND A GREEN GATE-64 WAS NOT EVIDENCE ABOUT THE WIDER PROBE ---------------------------------------------------------- The hard rule is scoped to OCA\OpenRegister\AppHost\, but the autoloader mechanism has nothing to do with AppHost: during register() the whole OCA\OpenRegister\ prefix is absent for any app sorting earlier, so ANY class_exists() on it answers FALSE and everything it guards silently never happens. Measured across apps-extra, no prelude present: larpingapp 3 Event\{DeepLinkRegistration,ObjectCreating,ObjectUpdating} — the last two carry larpingapp's server-authoritative skill-requirement / XP-budget enforcement on character writes, which therefore never registers hermiq 3 flow-node, leaf-provider and shareable-config registration nldesign 1 shareable-config registration Reported as a non-blocking NOTE, printed by the runner, and deliberately not a FAIL: this gate is not diff-scoped, so failing it would block every PR in three repos on code the PR did not touch — the trap check_store_and_settings_surface.py already records. A probe in boot() is NOT noted, because boot() runs after every app has registered. ALSO ---- Gate 64's log path was a hardcoded /tmp/hydra-gate-apphost-autoload-prelude .log — the shared-path non-determinism HYDRA_GATE_LOG_DIR exists to remove, left behind in one gate. TESTS ----- test_check_unclosable_gate.py NEW — gate 59 shipped with no helper suite at all; 23 cases, every arm of the matrix above plus the mutant that proves the comment mask is load-bearing test_gate_5661_empty_scope_is_not_a_pass.sh NEW — 28 assertions over gates 56-61: clean in-scope subject PASSes, empty scope is `na` with a reason and exit 0 under --require-full-coverage, and a planted true positive per gate still FAILs. Against the pre-fix package it reports 13 failures, naming the six PASSes test_check_apphost_autoload_prelude.py +14 cases test_check_orphaned_write_capability.py +5 cases test_check_listener_placement.py EXIT_EMPTY_SCOPE 51 discovered helper suites green (2 quarantined, unchanged). Full-tree run on shillinq before/after: verdicts byte-identical for all nine gates. * fix(gates 56,57,63,64): a crashed checker read as a clean tree, a real class read as absent, and a lazy closure read as an eager one Second pass over the 56-64 band, planting in a SECOND repo of a different shape per gate. Four more defects, all measured against gate package 48c88ba. A CRASHED CHECKER REPORTED PASS — gates 56 and 57 -------------------------------------------------- Both invoked their helper as `>> log 2>/dev/null || true` and then derived the verdict from `wc -l` on the log. Stderr discarded, exit status discarded, empty log — so a checker that never started reported PASS. Measured on shillinq with a python3 shim that exits 1 for exactly these two helpers: [gate-56] register-handler-resolution: PASS <- 153 registers [gate-57] orphaned-write-capability: PASS <- 316 services and gate-57 had reported 20 real findings over that same tree on the previous run. That is gate-40's defect verbatim. Both helpers ALWAYS exit 0 when they run, by design (#209 — the count goes to stdout, never into the exit byte), so a non-zero exit can only be a crash and never a finding count. Both now emit SKIPPED (wiring) and keep the stderr on disk. Every gate in this band was re-checked for the count-as-exit-status defect found in gates 19, 26 and 52: none of the nine has it. 56 and 57 put the count on stdout and the runner counts lines; 58/59/60/61/62/63/64 return status codes only. A REAL CLASS READ AS ABSENT — gate 56 -------------------------------------- The PSR-4 path guess handles the conventional layout; the fallback walk over lib/ is what finds a type living where PSR-4 does NOT predict — a DI-bound registration, a type not named after its file. That is the shape gate-30 was caught mis-resolving (`AppHost\Controller\GenericHealth` PSR-4-maps to lib/Controller/AppHost/Controller/… while openregister DI-binds it to lib/AppHost/Controller/). The walk matched `class` only, with at most ONE modifier. So every other declaration form was a FALSE POSITIVE on a type that genuinely exists — and the action `guard-class-not-found` invites is to write the class a second time. Measured, each against a real declaration at a non-conventional path: enum ProbeState: string { … } -> guard-class-not-found interface ProbeContract { … } -> guard-class-not-found trait ProbeTrait { … } -> guard-class-not-found final readonly class ReadonlyProbe -> guard-class-not-found `final readonly` is ordinary PHP 8.2. Only the DECLARATION FORMS widen; the line anchor that keeps docblock prose out is unchanged and asserted in both directions. A LAZY CLOSURE READ AS AN EAGER REFERENCE — gate 64 ---------------------------------------------------- This module's header has always said lazy service closures that merely MENTION an AppHost class are deliberately NOT flagged, because their bodies run at resolution time. Both rules ran over the whole file, so they did not. Measured on launchpad — deliberately a DIFFERENT repo shape from larpingapp. launchpad's whole composition root resolves OpenRegister lazily inside closures (it already does this for AppHost\Observability\ManifestLoader); that is the documented leaf pattern and the reason launchpad is green. A closure body naming Bootstrap reported byte-identically to an eager `Bootstrap::register($context, …)`. The gate would have failed the one repo doing it correctly, for doing it correctly, and the only remedy is to stop writing the lazy form. Anonymous and arrow-function bodies are now blanked before both rules. Named methods are untouched (`public function register(` has an identifier between `function` and `(`), and an eager reference AFTER or BETWEEN closures is still caught — both asserted. GATE 63 ON A CONTROLLER-ONLY DIFF — the reason was an overclaim --------------------------------------------------------------- Every BLOCKING rule in this gate reads src/manifest.json, src/manifest.d/ or src/menu-layout.json. The two rules that touch lib/Settings are WARNs and never fail it. So a PR that changes a settings CONTROLLER, or adds or deletes a lib/Settings/*Admin.php section, lands in the empty-scope branch — and the line read "this PR introduces no settings placement (ADR-079) to judge", which on such a diff is false. The author changed the settings surface; this gate does not adjudicate that half of it. NOT WIDENED, on purpose. Reading the controller was tried and reverted, and check_store_and_settings_surface.py records why: a gate that only RUNS when a manifest changed and then judges code the PR never touched "blocked EVERY manifest-touching PR in that repo, permanently". The verdict stays `na` — the correct category, since no change the author could make puts a manifest into a diff that does not touch one. What changes is that the line now names which half it looked at, and when the diff contains lib/Settings or a settings controller it says so explicitly, so `na` cannot be read as a clearance for the change the author actually made. VERIFIED, BOTH ARMS OF THE #270 CONTRACT ----------------------------------------- * empty ADR-020 scope -> NOT APPLICABLE, exit 0 under --require-full-coverage (measured on the controller-only fixture above) * genuine structural gap -> SKIPPED (structural), exit 98 (test_gate_empty_scope_never_passes.sh ARM 4, gate-33 with --axe-enabled and no report) And gate-60's three states, all asserted: real finding · SKIPPED (wiring) naming the missing dependency · clean pass. FLEET SWEEPS — 21 apps-extra repos, old helper vs new ------------------------------------------------------ gate-56 zero verdict changes gate-64 zero verdict changes; the three pre-existing FAILs (openbuild, procest, scholiq) survive, and the three NOTEs (larpingapp, hermiq, nldesign) are unchanged TESTS ----- test_gate_crashed_checker_is_not_a_finding.sh +2 gates, 5 assertions. Two arms: the plants must FAIL with a working interpreter, and the SAME tree must report SKIPPED (wiring) with a dead one — otherwise "always skip" would pass. Against 48c88ba it reports the two PASSes. test_check_register_handler_resolution.py +8 cases incl. the mutant that restores the pre-fix pattern and requires all four forms to go back to not-found. test_check_apphost_autoload_prelude.py +9 cases, closure and anti-widening arms. REPOS PLANTED IN, by gate -------------------------- 56 shillinq (register-owning, 153 register.d files) + a synthetic DI-bound/non-conventional-path fixture 57 shillinq (316 services) + fixture 58 shillinq (60 e2e files); fleet-wide check that no live networkidle call in any repo lives outside tests/e2e/ — 193 inside, 0 outside 59 docudesk (the repo the gate was written against) + fixture 60 shillinq (233 manifests, fake MDI package) + fixture 61 shillinq (15 post-event registrations) 62 shillinq 63 shillinq + a controller-only fixture 64 larpingapp (eager class_exists composition root, 3 live probes) AND launchpad (lazy-closure composition root, 0 eager references) NOT MINE, REPORTED NOT FIXED ----------------------------- test_gate_45_to_55_acceptance.sh fails 4 gate-53 assertions identically on pristine main (48c88ba) and on this branch — the suite expects blocking behaviour for pre-existing orphans that #250/#260/#280 deliberately made advisory. Out of this band; flagged rather than touched.
This was referenced Aug 9, 2026
rubenvdlinde
added a commit
that referenced
this pull request
Aug 13, 2026
…ng-literal-as-evidence axis in 16 gates (#424) (#438) * fix(gates): harden markup_mask's comment delimiter, then consolidate seven private copies (#424) STEP 1 — HARDEN. source_scope treated `<!--` as a comment opener wherever it appeared. `<p>{{ '<!--' }}</p>` … `<img alt="">` … `{{ '-->' }}` blanked the markup between them and gates 35/36 went green over live markup. Replaced the `<!--.*?-->` regex with html_comment_spans(), a scanner that knows a quoted attribute value and a Vue {{ }} interpolation are not comment scope, and that <script>/<style> bodies are raw text. Unterminated openers fall back to 'this was text' rather than swallowing the file — over-blanking is the failure mode. STEP 2 — CONSOLIDATE, only after step 1. Six a11y checkers (gates 37, 39, 40, 42, 43, 44), php_template_scope (gate 41) and check_orphaned_write_capability each carried their own copy of the same regex with the same hole. All eight now call source_scope.mask_html_comments. Also closes the string-literal-as-evidence axis in three PHP gates: gate-8 php_mask(blank_strings=True) — nothing it matches is ever a string, and blanking also repairs the brace walker. gate-59 two masks, one coordinate system: the CALL from a strings-blanked anchor, the config KEY from the string-preserving text. gate-64 same split; strip_comments() (the #184 state machine) now delegates to php_mask, removing a second PHP comment dialect. Every arm labelled EVIDENCE or CONTROL by measurement, not by assertion. * fix(gates): close the string-literal-as-evidence axis in gates 10, 11, 13, 18, 50, 53 (#424) gate-10 / gate-11 TWO MASKS, ONE COORDINATE SYSTEM. Both read evidence out of a literal — the attribute name in getAttribute('data-x'), the '/settings' path, the import specifier — so blanking string contents would delete the evidence. Instead the pattern runs on the string-preserving mask and source_scope.starts_in_code() asks the ANCHOR whether the match START is code. gate-11's route-object brace walk moves to the anchor too, so a '}' inside a literal stops truncating the object the anti-widening guard reads. gate-13 the runner's inline PYMI masked comments with a FOURTH private <!--.*?--> and never masked literals, so 'use <NcDialog> …' in <script> reported an inline modal. Now scopes to vue_markup_mask — the SFC's rendered template — which is the question the rule actually asks. Not a weakening: an inline modal is by definition written in the template. gate-18 tokens were matched against json.dumps(rule), so a rule whose own description WARNED AGAINST the legacy dialect was reported as it, three times. Now walks the structure: three KEY tokens against keys, '@self.' against machine values, with per-locale/documentation fields excluded. Gate had NO helper suite; test_check_notification_dialect.py adds 14 arms. gate-50 #420 hand-rolled _strip_php_comments inside run-hydra-gates.sh, a fourth PHP comment dialect. Deleted; the block calls php_mask. Verified behaviour-preserving over 2481 fleet PHP files (one difference, and it is php_mask being right: a CSS /* inside a string is not a comment). gate-53 stripJsComments was two regexes with no string awareness, so the /* in glob: '/*.vue' opened a block comment that deleted later registry entries — and, when the swallowed span was brace-unbalanced, made parsed:false skip the whole cross-reference check silently. New scripts/lib/js_scope.js is the node port of source_scope.js_comment_mask, asserted BYTE-IDENTICAL to it over a corpus plus this package's own .js sources. * test(source_scope): arms for starts_in_code, the anchoring predicate (#424) --------- 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.
Found while unpinning
hydra-gates-refacross the 14 caller repos: doriath went red on gate-64 with 2 findings. Neither is real.What was wrong
check_apphost_autoload_prelude.pyjudged ADR-040 compliance from raw text, which was wrong in both directions.False RED 1 — the app id as a class constant
has_prelude()required a quoted'openregister'insideregisterAutoloading(...). doriath passes it asself::OPENREGISTER_APP_ID:lib/AppInfo/OpenRegisterAutoloader.php:55—private const OPENREGISTER_APP_ID = 'openregister';lib/AppInfo/OpenRegisterAutoloader.php:89—\OC_App::registerAutoloading(self::OPENREGISTER_APP_ID, $path);lib/AppInfo/Application.php:145— calls that preludelib/AppInfo/Application.php:150— only then referencesBootstrapThat is exactly what ADR-040 asks for. Substituting the literal for the constant flips
has_prelude()fromFalsetoTruewith no other change — the gate was reading how the id is spelled, not what the code does.False RED 2 — comments counted as code
The
loadApprule matched doriath's own prose explaining that the code deliberately does not use it. Both occurrences underlib/AppInfo/are comments (Application.php:131,OpenRegisterAutoloader.php:69). Documentation of the correct choice was reported as the wrong one.False GREEN 3 — a commented-OUT prelude counted as a prelude
The more dangerous direction, found while writing the tests: an app whose prelude had been commented out passed gate-64. This is why the fix strips comments generally rather than patching the two observed symptoms.
The fix
#[is preserved as a PHP 8 attribute, not treated as a#comment.'openregister'in the same composition root are resolved.Evidence it still bites
Measured across all 14 consumer repos:
6 tests added. Against the pre-fix checker 4 of them fail, so they are shown capable of failing rather than merely passing. The existing 14 tests are unchanged and still pass.