Skip to content

gates: route names lose their backslash in 3 gates (51 false positives), gate-30 passes having matched nothing, gate-5's 20-line lookback misses real tags #213

Description

@rubenvdlinde

Found while running the gates full-tree against openregister at development (baf4aa9ec) with the runner at main (756fe89). Three independent defects in hydra-gates/scripts/run-hydra-gates.sh, all in the same family: a route name is parsed in a way that silently destroys part of it. Two produce false positives, one produces a falsely-green gate.

Everything below is reproduced against the checked-in runner, not inferred.


1. read without -r deletes the backslash in namespaced route names — 3 call sites

appinfo/routes.php entries are controller#method, and controllers in a sub-namespace are written Settings\ConfigurationSettings#getRbacSettings. Three gates pipe those names into read without -r, which treats \ as an escape and drops it:

run-hydra-gates.sh:1016      while IFS='#' read ctrl method; do          # gate-5  route-auth
run-hydra-gates.sh:1548          | while IFS='#' read _ctrl _method; do  # gate-14 route-reachability
run-hydra-gates.sh:2646          | while IFS='#' read _ctrl _method; do  # gate-30 public-monitoring

Measured:

$ printf 'Settings\\ConfigurationSettings#getRbacSettings\n' | while IFS='#' read ctrl method; do echo "no -r  => [$ctrl]"; done
no -r  => [SettingsConfigurationSettings]
$ printf 'Settings\\ConfigurationSettings#getRbacSettings\n' | while IFS='#' read -r ctrl method; do echo "with -r => [$ctrl]"; done
with -r => [Settings\ConfigurationSettings]

_ctrl_path_from_name then looks for lib/Controller/SettingsConfigurationSettingsController.php, which cannot exist. The real file is lib/Controller/Settings/ConfigurationSettingsController.php.

Corollary: _ctrl_path_from_name's *\\* branch (lines 590–598) — written specifically to map Settings\Foolib/Controller/Settings/FooController.php — is unreachable from either call site. The backslash is destroyed one line before the branch is consulted, so the fix it documents has never executed.

Effect on openregister

openregister's routes.php has 729 unique controller#method names, of which 53 are namespaced.

reported with read -r
gate-14 controller-class-not-found 53 2
gate-5 "NOT JUDGED" (unresolvable) 53 2

51 of 53 are pure false positives — the file exists and the method exists on it.

The 2 survivors are also false positives — second, independent defect

AppHost\Controller\GenericHealth#index and AppHost\Controller\GenericMetrics#index are DI-bound at lib/AppInfo/Application.php:2877 and :2889 under the literal keys 'AppHost\\Controller\\GenericHealthController' / '…GenericMetricsController'.

_di_binds_controller (line 532) always builds the needle with the app namespace prefixed — OCA\OpenRegister\Controller\AppHost\Controller\GenericHealthController. But NC's buildControllerName() does not prefix a route name that already contains a backslash (documented at Application.php:2855-2875, where this exact mismatch previously caused a live 503). So _di_binds_controller cannot match this legitimate binding shape.

Also: gate-14's verdict string is wrong for all 53

Line 1590 reports "53 unrouted method(s) or wrong-target route(s)". Every one of the 53 is rule=controller-class-not-found; zero are missing-route. Invariant 1 genuinely ran and found nothing (138 controller files, 679 Response-returning methods, 0 missing routes). The two invariants should be reported separately — the current template misdescribes the finding.


2. gate-30 public-monitoring reports PASS having matched nothing — falsely green

Independent of the read bug, and worse, because the outcome is a pass.

The extraction regex at line 2643 hard-codes lowercase alternatives:

(metrics|health|liveness|readiness|probe)

openregister's three monitoring routes are capitalised:

appinfo/routes.php:267   'AppHost\Controller\GenericMetrics#index'  -> /api/metrics
appinfo/routes.php:272   'AppHost\Controller\GenericHealth#index'   -> /api/health
appinfo/routes.php:1051  'chatHealth#health'                        -> /api/chat/health

GenericMetrics, GenericHealth, chatHealth — the monitoring word appears only with a capital, and the regex requires it before the #. Zero matches.

Evidence: hydra-gate-public-monitoring.log is 0 bytes and the verdict is PASS. Positive control: the same regex does match a synthetic 'health#index', so the regex works — it simply never sees this repo's routes.

So the gate that exists to stop /api/metrics and /api/health being publicly exposed has never examined openregister's /api/metrics or /api/health, and says PASS. Per ADR-006 these are admin-only; whether they actually are is currently unmeasured here. This is the falsely-green shape, and it is likely fleet-wide wherever a monitoring controller is PascalCase or namespaced.


3. gate-5's 20-line docblock lookback → 2 more false positives

Line 1035 searches for an auth attribute from def_line - 20. Two openregister methods have docblocks longer than 20 lines (inflated by @psalm-return), putting a genuine tag above the window:

file:line method tag present at
lib/Controller/FileExtractionController.php:587 stats :559 @NoAdminRequired, :563 @NoCSRFRequired (docblock 556–586, 31 lines)
lib/Controller/Settings/FileSettingsController.php:323 getFileExtractionStats :301 @NoCSRFRequired (docblock 292–322) — currently hidden by defect 1

The clamp at 1048–1051 already computes prev_close, so widening the window to prev_close+1 .. def_line is safe — it cannot borrow an attribute from a neighbouring method, and it errs in the false-negative direction only.


4. Silent coverage hole of false-negative shape

Because of defect 1, all 10 lib/Controller/Settings/*Controller.php classes — 51 routed methods, the entire admin-settings API surface — were never auth-judged by gate-5 at all. The only trace is the advisory line:

[hydra-gates] gate-5 route-auth: 53 routed entr(ies) NOT JUDGED (controller class unresolvable here)

That line does not start with ^[gate-, so no consumer that parses gate verdicts sees it. An unjudged route is indistinguishable from a passing one downstream.


Suggested fixes

  1. Add -r to read at lines 1016, 1548, 2646. This alone takes gate-14 from 53 → 2 and un-blinds 51 routes for gate-5.
  2. _di_binds_controller (line 532): also try the needle without the OCA\<App>\Controller\ prefix, matching buildControllerName() for backslash-containing route names. Clears the last 2.
  3. Line 1035: replace the fixed def_line - 20 with prev_close+1 .. def_line.
  4. Line 2643: make the alternation case-insensitive, and match the word on either side of #.
  5. Line 1590: report gate-14's two invariants separately.

Impact on the repo that surfaced it

Of gate-5 + gate-14's 64 reported findings against openregister, 54 are gate noise and 10 are real: ScheduledWorkflowController::create/update/destroy, WorkflowEngineController::create/update/destroy/health/available/testHook, WorkflowExecutionController::destroy carry no auth annotation at all, while index/show on the same controllers are @NoAdminRequired. In NC an absent annotation means admin-required, so these are silently admin-only writes — a real asymmetry, but the safe direction, and being reported separately in openregister rather than fixed blind.

Because gate-14 and gate-30 sit on main and every consumer floats hydra-gates-ref at main, fixing these changes verdicts fleet-wide — recommend a consumer re-measure after, not just a merge.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions