fix(gate-5): route-auth reported a resolution failure as a security finding, and never saw a camelCase route - #162
Merged
Conversation
…inding, and never saw a camelCase route Closes #153. Gate-5 had FOUR defects. Two were reported (#153); the other two this work found, and one of them is the expensive direction. 1. A RESOLUTION FAILURE WAS REPORTED AS A SECURITY FINDING. When lib/Controller/<X>Controller.php did not exist, the gate wrote a line into the failure log and the verdict read "N routed method(s) missing auth attribute". Live on scholiq, whose health/metrics/preferences controllers are ADR-040 AppHost generics registered by \OCA\OpenRegister\AppHost\Bootstrap::register(): the files are absent from the leaf repo BY DESIGN and the attributes live in the openregister package. "I cannot see it" is not "it is absent". A security gate that cries wolf on correct code trains readers to skip the whole tier. 2. IT WAS NOT DIFF-SCOPED. The missing-file branch `continue`d BEFORE the `_in_scope` call, so those findings fired regardless of the diff — measured on a package.json + package-lock.json Dependabot bump. Every scholiq PR was blocked by a finding it did not introduce and could not fix. Per ADR-020 a routed method is now judged when the PR touched EITHER its controller OR appinfo/routes.php (altering a route is exactly when its auth posture must be re-checked). 3. FOUND HERE — camelCase route slugs were invisible. Gate-5 read route names through `'[a-z_]+#…'`, which matches only lowercase and snake_case. Gate-14 already used `'[A-Za-z][A-Za-z0-9_\\]*#…'`. Two gates, two regexes, and the narrower one was the security gate. Measured on scholiq: 14 of 37 routed names matched; the other 23 — `paymentTransaction#callback`, `keyAdmin#generateKey`, `credentialVerify#verify` among them — were never opened, in either direction. Caught because the positive control for this very fix (strip both the attributes AND the docblock tags from a real routed method) still reported PASS. The resolver and the regex are now one shared helper used by both gates. 4. FOUND HERE — the 20-line attribute lookback was not bounded by the start of the method, so it read back over the PREVIOUS member. A short guarded method within 20 lines above an unguarded one donated its `#[NoAdminRequired]` to its neighbour and the unguarded method passed. False NEGATIVE, and invisible, because a pass leaves no log. The window is now clamped to the line after the previous member's closing brace — it can only stop an attribute being borrowed, never hide a genuine one. NO HOLE OPENED. A route naming a class this repo genuinely does not ship is a real defect (ReflectionException 500) — it is just not gate-5's. Gate-14 (route-reachability) used to `continue` past it with the comment "gate-5 already flags this"; it now raises it as `rule=controller-class-not-found`, diff-scoped the same way. Removing the finding from gate-5 without adding it to gate-14 would have created exactly the dead gate this change exists to avoid. AppHost adoption excuses the FIVE generics Bootstrap::register() provides and nothing else. EVIDENCE — scripts/lib/test_gate_route_auth.sh, 25 assertions, every one half of a control pair, auto-discovered by tests/run-helper-suites.sh: unguarded -> FAIL (real IDOR shape + a camelCase slug) guarded -> PASS (same routes, attributes present) apphost -> PASS, 4 entries stated as NOT JUDGED with the reason apphost-unguarded -> FAIL (adoption does not exempt the app's own code) and gate-14 still raises its non-generic missing controller orphan-route -> gate-5 declines, gate-14 raises both reachability shapes + four scoped runs over real git repos: package.json-only diff clean while the finding is demonstrably present in the tree; controller-touched and routes.php-touched diffs both FAIL. MUTATION-TESTED. Three realistic degradations were introduced and the suite went red for each: skipping gate-5 for AppHost apps (6 assertions), widening _apphost_serves to any slug (2), and reverting gate-14 to "gate-5 already flags this" (3). Reverting the lookback clamp fails 4. VERIFIED ON THE REAL REPO. scholiq @ a363b9b, full-tree: gate-5 PASS with the 4 AppHost entries stated as unjudged, gate-14 PASS. Strip the attributes and the docblock tags from CredentialVerifyController::verify and the same run reports `FAIL — 1 … CredentialVerifyController.php:85 method=verify` — so the green is a measurement, not an absence. Every run in the suite is rejected unless its COVERAGE line is present: an abort before the summary leaves the PASS lines on stdout and reads as green. ShellCheck clean.
rubenvdlinde
added a commit
to ConductionNL/scholiq
that referenced
this pull request
Aug 5, 2026
v1.0.1's gate-5 (route-auth) could not tell "this routed method has no auth attribute" from "I could not resolve the controller class", and reported both as the former. It also read route names through `'[a-z_]+#...'`, so every camelCase route slug was invisible to it in either direction, and its 20-line attribute lookback could borrow the PREVIOUS method's attribute. Fixed in ConductionNL/.github#162 (closing #153). v1.2.0 also carries the runner's COVERAGE accounting, which v1.0.1 does not have at all: v1.0.1's summary printed only "N gate(s) failed", so a gate that never ran was indistinguishable from one that passed. EXPECT NEW FINDINGS. gate-5 now sees code it never saw. Red is the honest outcome of a measurement that previously was not taken; nothing here is baselined or suppressed to avoid it.
rubenvdlinde
added a commit
to ConductionNL/procest
that referenced
this pull request
Aug 5, 2026
v1.0.1's gate-5 (route-auth) could not tell "this routed method has no auth attribute" from "I could not resolve the controller class", and reported both as the former. It also read route names through `'[a-z_]+#...'`, so every camelCase route slug was invisible to it in either direction, and its 20-line attribute lookback could borrow the PREVIOUS method's attribute. Fixed in ConductionNL/.github#162 (closing #153). v1.2.0 also carries the runner's COVERAGE accounting, which v1.0.1 does not have at all: v1.0.1's summary printed only "N gate(s) failed", so a gate that never ran was indistinguishable from one that passed. EXPECT NEW FINDINGS. gate-5 now sees code it never saw. Red is the honest outcome of a measurement that previously was not taken; nothing here is baselined or suppressed to avoid it.
rubenvdlinde
added a commit
to ConductionNL/portaliq
that referenced
this pull request
Aug 5, 2026
v1.0.1's gate-5 (route-auth) could not tell "this routed method has no auth attribute" from "I could not resolve the controller class", and reported both as the former. It also read route names through `'[a-z_]+#...'`, so every camelCase route slug was invisible to it in either direction, and its 20-line attribute lookback could borrow the PREVIOUS method's attribute. Fixed in ConductionNL/.github#162 (closing #153). v1.2.0 also carries the runner's COVERAGE accounting, which v1.0.1 does not have at all: v1.0.1's summary printed only "N gate(s) failed", so a gate that never ran was indistinguishable from one that passed. EXPECT NEW FINDINGS. gate-5 now sees code it never saw. Red is the honest outcome of a measurement that previously was not taken; nothing here is baselined or suppressed to avoid it.
rubenvdlinde
added a commit
to ConductionNL/openbuild
that referenced
this pull request
Aug 5, 2026
v1.0.1's gate-5 (route-auth) could not tell "this routed method has no auth attribute" from "I could not resolve the controller class", and reported both as the former. It also read route names through `'[a-z_]+#...'`, so every camelCase route slug was invisible to it in either direction, and its 20-line attribute lookback could borrow the PREVIOUS method's attribute. Fixed in ConductionNL/.github#162 (closing #153). v1.2.0 also carries the runner's COVERAGE accounting, which v1.0.1 does not have at all: v1.0.1's summary printed only "N gate(s) failed", so a gate that never ran was indistinguishable from one that passed. EXPECT NEW FINDINGS. gate-5 now sees code it never saw. Red is the honest outcome of a measurement that previously was not taken; nothing here is baselined or suppressed to avoid it.
This was referenced Aug 5, 2026
Merged
rubenvdlinde
added a commit
to ConductionNL/openbuild
that referenced
this pull request
Aug 5, 2026
* ci(quality): move the hydra-gates pin v1.0.1 -> v1.2.0 v1.0.1's gate-5 (route-auth) could not tell "this routed method has no auth attribute" from "I could not resolve the controller class", and reported both as the former. It also read route names through `'[a-z_]+#...'`, so every camelCase route slug was invisible to it in either direction, and its 20-line attribute lookback could borrow the PREVIOUS method's attribute. Fixed in ConductionNL/.github#162 (closing #153). v1.2.0 also carries the runner's COVERAGE accounting, which v1.0.1 does not have at all: v1.0.1's summary printed only "N gate(s) failed", so a gate that never ran was indistinguishable from one that passed. EXPECT NEW FINDINGS. gate-5 now sees code it never saw. Red is the honest outcome of a measurement that previously was not taken; nothing here is baselined or suppressed to avoid it. * ci(quality): advance the hydra-gates pin v1.2.0 -> v1.3.0 v1.2.0 added the COVERAGE accounting but not the declarations that accounting asks for. v1.0.1/v1.2.0 have no `_skip` vocabulary, so a gate whose prerequisite is absent emits nothing and require-full-coverage (default ON) counts it as DID NOT RUN. v1.3.0 ships 36 not-applicable declarations; measured on doriath PR #160, Hydra Gates went failure -> success on the pin alone. Lands this repo on the same pin as the rest of the fleet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(gate-5): route-auth reported a resolution failure as a security finding, and never saw a camelCase route
Closes #153.
Gate-5 had FOUR defects. Two were reported (#153); the other two this work
found, and one of them is the expensive direction.
A RESOLUTION FAILURE WAS REPORTED AS A SECURITY FINDING.
When lib/Controller/Controller.php did not exist, the gate wrote a line
into the failure log and the verdict read "N routed method(s) missing auth
attribute". Live on scholiq, whose health/metrics/preferences controllers
are ADR-040 AppHost generics registered by
\OCA\OpenRegister\AppHost\Bootstrap::register(): the files are absent from
the leaf repo BY DESIGN and the attributes live in the openregister package.
"I cannot see it" is not "it is absent". A security gate that cries wolf on
correct code trains readers to skip the whole tier.
IT WAS NOT DIFF-SCOPED. The missing-file branch
continued BEFORE the_in_scopecall, so those findings fired regardless of the diff — measuredon a package.json + package-lock.json Dependabot bump. Every scholiq PR was
blocked by a finding it did not introduce and could not fix. Per ADR-020 a
routed method is now judged when the PR touched EITHER its controller OR
appinfo/routes.php (altering a route is exactly when its auth posture must
be re-checked).
FOUND HERE — camelCase route slugs were invisible. Gate-5 read route names
through
'[a-z_]+#…', which matches only lowercase and snake_case. Gate-14already used
'[A-Za-z][A-Za-z0-9_\\]*#…'. Two gates, two regexes, and thenarrower one was the security gate. Measured on scholiq: 14 of 37 routed
names matched; the other 23 —
paymentTransaction#callback,keyAdmin#generateKey,credentialVerify#verifyamong them — were neveropened, in either direction. Caught because the positive control for this
very fix (strip both the attributes AND the docblock tags from a real
routed method) still reported PASS. The resolver and the regex are now one
shared helper used by both gates.
FOUND HERE — the 20-line attribute lookback was not bounded by the start of
the method, so it read back over the PREVIOUS member. A short guarded
method within 20 lines above an unguarded one donated its
#[NoAdminRequired]to its neighbour and the unguarded method passed. FalseNEGATIVE, and invisible, because a pass leaves no log. The window is now
clamped to the line after the previous member's closing brace — it can only
stop an attribute being borrowed, never hide a genuine one.
NO HOLE OPENED. A route naming a class this repo genuinely does not ship is a
real defect (ReflectionException 500) — it is just not gate-5's. Gate-14
(route-reachability) used to
continuepast it with the comment "gate-5already flags this"; it now raises it as
rule=controller-class-not-found,diff-scoped the same way. Removing the finding from gate-5 without adding it to
gate-14 would have created exactly the dead gate this change exists to avoid.
AppHost adoption excuses the FIVE generics Bootstrap::register() provides and
nothing else.
EVIDENCE — scripts/lib/test_gate_route_auth.sh, 25 assertions, every one half
of a control pair, auto-discovered by tests/run-helper-suites.sh:
unguarded -> FAIL (real IDOR shape + a camelCase slug)
guarded -> PASS (same routes, attributes present)
apphost -> PASS, 4 entries stated as NOT JUDGED with the reason
apphost-unguarded -> FAIL (adoption does not exempt the app's own code) and
gate-14 still raises its non-generic missing controller
orphan-route -> gate-5 declines, gate-14 raises both reachability shapes
the finding is demonstrably present in the tree; controller-touched and
routes.php-touched diffs both FAIL.
MUTATION-TESTED. Three realistic degradations were introduced and the suite
went red for each: skipping gate-5 for AppHost apps (6 assertions), widening
_apphost_serves to any slug (2), and reverting gate-14 to "gate-5 already flags
this" (3). Reverting the lookback clamp fails 4.
VERIFIED ON THE REAL REPO. scholiq @ a363b9b, full-tree: gate-5 PASS with the
4 AppHost entries stated as unjudged, gate-14 PASS. Strip the attributes and
the docblock tags from CredentialVerifyController::verify and the same run
reports
FAIL — 1 … CredentialVerifyController.php:85 method=verify— so thegreen is a measurement, not an absence.
Every run in the suite is rejected unless its COVERAGE line is present: an
abort before the summary leaves the PASS lines on stdout and reads as green.
ShellCheck clean.
How to check this yourself
Blast radius, stated honestly
Defects 3 and 4 make gate-5 see code it never saw. Expect new, real findings
in repos whose gate-5 has been quietly green. That is the point of the change,
not a regression — but it means the pin move off
v1.0.1should be treated as ameasurement, not a formality.
🤖 Generated with Claude Code