Skip to content

hydra-gates: five false-positive modes (~130 of 276 fleet findings), one with dangerous remediation advice — plus the runner is unsafe to run concurrently #158

Description

@rubenvdlinde

Re-running the gates against the full tree across 21 fleet repos (excluding openregister) produced 276 failing gates. Triaging the authorisation gates line by line — reading every call path — turned up five distinct false-positive modes. Together they account for ~130 of the 276, and one of them ships remediation advice that would introduce a vulnerability if followed.

Measurement: gate script ConductionNL/.github@ad00986 (main, includes #147/#148/#149), ajv installed under hydra-gates/scripts/lib, each repo at its current origin/development, no --scope-to-diff. Each run was executed in its own mount namespace with a private tmpfs /tmp, because the runner writes its detail logs to hardcoded /tmp/hydra-gate-<name>.log paths and reads the counts back out of them — two concurrent runs silently corrupt each other's numbers (see item 6).


1. gate-38 skip-link — fails in 21 of 21 repos

Uniformity across independent inputs points at the instrument, and it is.

The gate greps a root component for a literal <NcContent> / <NcAppContent>. The fleet has migrated to <CnAppRoot> (nc-vue), and CnAppRoot renders <NcContent> internallysrc/components/CnAppRoot/CnAppRoot.vue:60. Every manifest-driven app root therefore fails a check it satisfies.

Second cause in the same gate: it scopes templates/settings/*.php. Those are Nextcloud settings-framework templates; the framework owns the page shell, so such a file can never contain a skip-link and can never pass.

Fix: accept <CnAppRoot> (and any component that transitively renders NcContent) as a shell; drop templates/settings/*.php from scope.

2. gate-9 semantic-auth, rule public-page-annotation-with-auth-body36 of 39 findings, and the advice is dangerous

The gate tells you: "remove #[PublicPage] or remove body auth check". Every one of the 36 is a webhook or public-portal endpoint that correctly bypasses NC session auth and authenticates the caller itself. Following either half of that advice breaks the endpoint or removes its only authentication.

Verified call paths:

  • openconnector PaymentsController::webhook — HMAC signature verify with constant-time compare + timestamp tolerance, 401 before any state change. Its docblock already says "the signature check IS the auth body for this route".
  • portaliq ContributionController::inbox (and 9 siblings) — resolves a portal subject(), 401 when absent; per-row subject + tenant + trust-boundary filtering.
  • Same shape: openconnector DSO / IwmoIjw / Notificaties / NotifyNl / OpenFormulieren / Peppol / StufZkn inbound, procest CaseFederationController ×3, hermiq EgressAuthorize + McpRun, launchpad PublicShareController ×2, doriath ApplicationController::create + ApplicationTokenController::exchange, scholiq PaymentTransactionController::callback, shillinq PortalPaymentInitiationController::initiate, softwarecatalog ×2, petstore ×1.

Fix: the rule must distinguish "public but self-authenticating" (signature verify, bearer/portal token, capability token → correct) from "#[PublicPage] while relying on an IUserSession session check" (→ the real mismatch). Only 3 of the 39 findings are the no-admin-required-annotation-with-admin-body shape the gate was actually built for, and those were real — fixed in openbuild#127.

3. gate-7 no-admin-idor14 of 15 findings false, even after #149

#149 taught the gate to follow delegation; two blind spots remain.

(a) A guard that returns bool/null instead of throwing or returning 401/403. hermiq ×7: AgentVersionController::index/diffloadAccessibleAgent($id, $userId)canUserAccessAgent() (private/owner/invited); AgentsController::index applies canUserAccessAgent() per row inside the result loop. _GUARD_HELPER_NAME_RE requires the name to end in Admin|Access|Permission|Permitted|Owner|Allowed|AuthorisedcanUserAccessAgent ends in Agent, loadAccessibleAgent starts with load; and _HELPER_GUARD_BODY_RE wants a throw or a 401/403/404, which a bool-returning predicate never has.

(b) Endpoints with no object identifier at all. nldesign CatalogController::tokenSets (public catalogue, no args), nldesign ContrastController::evaluate (pure colour math on caller-supplied input), procest AssistantController::availability (boolean feature probe), hermiq AgentsController::create/stats/tools. There is no object to be insecure-direct-referenced; the gate should not ask for a per-object guard where no object is addressed.

doriath CxpRelayController::put/get are a capability-URL relay (the 24-char ISecureRandom pairing id is the bearer credential, payloads are sealed) and SettingsController::getPolicy is a documented read-open-to-all-authenticated. Both false.

The one real finding was docudesk AnonymizationController::updateRelation — genuinely unguarded through four service hops to an unscoped primary-key write. Fixed in docudesk#382. So gate-7's true-positive rate here is 1 in 15 — high enough to be worth keeping, low enough that the noise will train people to skip it.

4. gate-14 route-reachability22 of 30 findings are AppHost delegation

openbuild (7), shillinq (7), doriath (5), planix (3) are reported as having unrouted dashboard#page, dashboard#catchAll, settings#index/create/load, preferences#getPreference/setPreference. All are supplied by OCA\OpenRegister\AppHost\Routes::standard(). The gate parses only the literal array in appinfo/routes.php, so it cannot see routes contributed by the shared builder.

Tell: docudesk passed this gate only because it happens to carry a literal $canonicalRoutes fallback array for the openregister-absent case. Passing should not depend on that.

Fix: when appinfo/routes.php calls Routes::standard(), fold in Routes::canonicalRoutes() before deciding.

The other 8 findings are real and worth keeping — see item 5.

5. gate-6 orphan-auth — 2 of 5 false

  • launchpad LiveTileController::validateSource — a routed controller method (liveTile#validateSource, POST, appinfo/routes.php:557). Its caller is the NC router, not PHP code.
  • pipelinq LogBerichtenboxAdapter::checkMailbox — implements BerichtenboxAdapterInterface::checkMailbox; callers go through the interface.

Fix: exempt routed controller methods and declared-interface implementations.

The other three are real decidesk#60-shaped orphans — implemented, unit-tested, never wired into any production path — and are being filed against their repos: procest BewijsstukService::assertMutable, pipelinq PortalTenantService::isSelfSignupAllowed, pipelinq ZgwCoexistenceValidator::validateWritePath.

6. The runner is not safe to run twice at once

run-hydra-gates.sh writes ~50 detail logs to hardcoded /tmp/hydra-gate-<name>.log and derives each verdict from wc -l on that shared file. Two concurrent runs — trivially easy across repos, or a stray backgrounded run nobody noticed — produce numbers that are plausible but wrong, in both directions.

This bit me during this exercise: a first measurement pass reported softwarecatalog with 7 gate-7 IDOR findings. Running check_no_admin_idor.py directly against the same tree returned zero — the 7 belonged to hermiq, running in parallel. procest moved 22 → 21, softwarecatalog 18 → 16, zaakafhandelapp 8 → 9 once the runs were isolated. Only gate-6 was immune, because it is the one gate already using mktemp.

Fix: derive a per-run log directory once (mktemp -d, or honour TMPDIR) and hang every gate's log off it — the pattern gate-6 already uses. Cheap, and it removes a whole class of "measured, but wrong" from every future fleet sweep.


Suggested priority

  1. item 6 — until it lands, no fleet-wide number from this runner is trustworthy.
  2. item 2 — the remediation text is actively harmful.
  3. items 1, 4 — pure noise, ~43 findings, and they are what makes people stop reading gate output.
  4. items 3, 5 — narrow the guard vocabulary and the scope.

Nothing was suppressed, baselined or excluded anywhere in this exercise.

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