fix(gate-5,14,30,9): route names, delegated registrars, and a monitoring gate that passed on nothing (#213, #218, #221, #223, #237) - #264
Merged
Conversation
…ing gate that passed on nothing Closes #213. Closes #218. Closes #221. Closes #223. Closes #237. Two defects, five issues, three gates: (a) A shell/regex detail corrupted the input, so the gate measured something other than the code. gate-30's selector alternation is lowercase-only, so GenericMetrics/GenericHealth/chatHealth matched ZERO and it printed PASS over a 0-byte log; it also still carried the `read` without -r that #217 fixed in gates 5 and 14. (b) The gates modelled ONE registration idiom and flagged every other legitimate one: routes supplied by Routes::standard(), a Bootstrap::register() call moved into a registrar, a credential resolved one frame down, a controller whose name merely contains "health". gate-30 can no longer print PASS without having opened a monitoring method: every other outcome is a stated NOT APPLICABLE / SKIPPED with counts, and PASS states how many endpoints it inspected.
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.
Closes #213. Closes #218. Closes #221. Closes #223. Closes #237.
Rebased on #217, which landed the
read -rfix for gates 5 and 14 and explicitly left one shape open (see §1b). Nothing here duplicates it.Two defects, five issues, three gates
(a) A shell or regex detail corrupted the input, so the gate measured something other than the code.
(b) The gates modelled ONE registration idiom and flagged every other legitimate one.
1a. gate-30 selected NOTHING and printed PASS (#213)
The selector alternation is lowercase-only under
grep -E:AppHost\Controller\GenericMetrics#index,genericHealth#index,chatHealth#health— no match. openregister, which owns the fleet's health/metrics engine, reported[gate-30] public-monitoring: PASSover a 0-byte log on every run. Confirmed silently green in four repos.gate-30 also still carried
while IFS='#' read _ctrl _method— the line #217 fixed in gates 5 and 14 — so a namespaced monitoring route was resolved to a path that cannot exist and skipped without a word.1b. The relative DI key #217 flagged and left open (#213)
openregister names two routes with a relative namespace and binds them verbatim:
RouteParser::buildControllerName()appendsControllerto the route-name segment verbatim and does not prefixOCA\<App>\Controller\when the name already contains a backslash — openregister's own comment records the 503 that taught it._di_binds_fq_controlleris scoped toOCA\<App>\…on purpose, so these two were still reported._di_binds_controllernow accepts the bare key for a namespaced name only: for a plain slug the needle would beWidgetController, which any strayWidgetController::classnear aregisterServicecall would satisfy.openregister gate-14: 2 → 0.
2. Routes supplied by
Routes::standard()are not in routes.php (#223)An ADR-040 adopter returns
\OCA\OpenRegister\AppHost\Routes::standard($extra)and receives ten canonical entries it never spells out. gate-14 invariant 1 askedgrep -qF "'slug#method'" appinfo/routes.php— the wrong file, answer always no. Every app shipping its own DashboardController / SettingsController (the supported shape;aliasControllerUnlessLeafDefinesItexists to allow it) was told its working/and/api/settings404.The ten names are an explicit list mirroring
openregister lib/AppHost/Routes.php::canonicalRoutes()+::catchAllRoute()(verified atopenregister@1dcc92cf9, lines 110–126 + 165–176). Not "AppHost adopters are exempt from invariant 1" — see thegadget#runhalf of the fixture.doriath gate-14: 5 → 0.
3. The AppHost call is not required to be in Application.php (#237)
_HYDRA_APPHOSTgreppedlib/AppInfo/Application.phpand nothing else. procest'sBootstrap::register()lives inlib/AppInfo/Registrar/AppHostRegistrar.php— because phpmd asked for the decomposition (procest#717). The evidence is now "some tracked file underlib/calls it", with both conditions required in the same file. #199 fixed a neighbouring symptom and this one kept firing; the fixture pair below is what makes that not repeatable.procest gate-14: 4 → 0.
4. A monitoring word in the name is not a monitoring endpoint (#218)
launchpad's
HealthPingControlleris a per-placement badge: 401 anonymous, thencanViewPlacement()before any work. gate-30's only remedy — add#[PublicPage]— would have published an outbound-ping oracle to anonymous callers.The discriminator is not the name, it is the shape of the route. A Prometheus scraper, a kubelet probe or an uptime monitor has no session and no object in mind: it issues a GET against a fixed URL. So a monitoring endpoint here is a name-matching route that is also an unparameterised GET.
healthPing#show—/api/health-ping/{placementId}, per-object → not judged, reason loggedhealthPing#validate— POST, submits a candidate config → not judged, reason loggedhealth#index—/api/healthGET → still enforced strictlylaunchpad gate-30: FAIL 2 → PASS, with 2 endpoints genuinely inspected.
5. The credential is often one frame down (#221)
#[PublicPage]in Nextcloud means a login is not required — not there is no session. Two shapes were being reported:ApplicationController::create: admin auto-approves, an authenticated non-admin gets a pending row, an anonymous caller is admitted only when the admin has setanonymous_application_registration_enabled. The 401 is a stated policy, not a check against something the annotation forbids._PUBLIC_SESSION_AUTH_RE), which is tested first and still owns the genuine contradiction —requireAdmin()under#[PublicPage], decidesk#44.doriath gate-9: FAIL 1 → PASS.
6. A pre-existing window defect, surfaced by fixing #213
The moment openregister's
Settings\…routes resolved at all, gate-5 reported@NoCSRFRequiredsits on line 301 and the declaration on 323 — a@psalm-returnshape in between — so the 20-line lookback started at 303 and the tag was outside it. Same forFileExtractionController::stats(tags at 559/563, declaration at 587). Both are correctly annotated by gate-5's own accepted-attribute set.New shared
_head_blocktakes the contiguous annotation run OR the 20-line slice, whichever starts earlier, and keeps the previous-member clamp from #153. The window can only grow, so nothing that passes today can start failing; and it still cannot borrow a neighbour's attribute. Used by gate-5 and gate-30.7. gate-30 may no longer print PASS having opened nothing
[ fail -eq 0 ] && PASShad an empty findings log as its only input, and an empty log has two causes that mean opposite things. Now:FAILPASSNOT APPLICABLESKIPPED (structural)--require-full-coverageEvery candidate the run did not judge is written to
hydra-gate-public-monitoring-notes.logwith its reason.Measured — full-tree, both arms, same 5 repos
origin/main(7511fb2, i.e. with #217) vs this branch. Numbers re-read from the runner's own stdout, not from memory.Runner exit: openregister 21→20, launchpad 14→13, procest 21→20, doriath 14→12, openconnector 4→4.
Against the pre-#217 baseline the openregister route pair reads 64 → 10: gate-14 53→0 and gate-5's phantom UNRESOLVED list 53→2 (the two remaining are the real AppHost generics, correctly stated as NOT JUDGED).
Every removal verified on the real file, named:
launchpad lib/Controller/HealthPingController.php—show()401s +canViewPlacement();validate()is a POSTdoriath lib/Controller/ApplicationController.php::create—$this->session->getUser(), then the app-config opt-inprocest lib/AppInfo/Registrar/AppHostRegistrar.php— the delegatedBootstrap::register()openregister lib/AppInfo/Application.php::registerAppHostObservability— the two verbatim DI keysopenregister lib/Controller/Settings/FileSettingsController.php:301/323andlib/Controller/FileExtractionController.php:559/587— the docblock windowRoutes::standard()set —openregister lib/AppHost/Routes.php:110–126+165–176Tests — 30 new shell assertions, 9 new Python, mutation-checked
scripts/lib/test_gate_route_registration.sh(30 assertions, 7 fixtures, auto-discovered bytests/run-helper-suites.sh) + 9 cases intest_check_semantic_auth.py.Every fixture carries its own anti-widening half — a sibling differing by the ONE thing the widening accepts, which must still fail.
delegated-registrar/anddelegated-registrar-absent/are byte-identical apart from a single registrar file.11 mutants, 11 killed, none survived:
read -rrevertedhealthPingreturns as a finding_apphost_supplies_routealways truegadget#rungoes quiet_apphost_servesalways true_head_blockback to a blind 20-line sliceRegression, exit codes read directly (not through a pipe):
Found, NOT fixed here — filed separately
Routes::standard()suppliessettings#update(PUT/api/settings), and doriath'sSettingsControllerhas noupdate(). Because doriath ships its own controller the AppHost alias is skipped, so that route resolves to a method that does not exist. Feeding the canonical ten into gate-5's judging stream and gate-14 invariant 2 would catch it — but that direction adds findings across 16 apps and belongs in its own change with its own measurement, not in a PR whose purpose is removing false positives. Flagging rather than smuggling.