fix(security): close four authorisation holes the full-tree Hydra gates surfaced

[rubenvdlinde]

Why

Enabling the Hydra gates here (#2335) produced a green tick that certified nothing. The gates are diff-scoped, and the enablement PR touched three CI files:

[hydra-gates] Scope: diff vs origin/development — 3 changed file(s)

58 gates "passed" in about a second having read no lib/, no src/, no routes.php.

Re-run against the full tree, with ajv resolvable as CI has it, under an isolated /tmp: 19 gates fail at f1614ce67. Gates 6/7/8/9 — the ADR-005 authorisation family — account for 27 findings. I read the call path for every one. Four are real. All four are the same shape: an authorisation decision one path makes and a neighbouring path does not.

The four

1. FlowController::state() was an unguarded cross-organisation read.
GET /api/flow/{flowId}/state is @NoAdminRequired and handed the client-supplied uuid straight to FlowStateMapper::findByFlow(), which applies no organisation scoping at all. Any authenticated user could read any other tenant's flow state — arbitrary data written by flow nodes: slot holders, external identifiers, run bookkeeping — by naming its uuid. It was also the one method in the class that broke the invariant stated in the class's own file header: "Every CRUD method goes through FlowService, never FlowMapper." Now resolved through FlowService::find() first, so a flow the caller may not see 404s exactly like one that does not exist.

2. FederatedConfigController::bundle() was publish() without publish()'s gate.
Same serialise() call, same bytes, no canPublish() check — so anyone the publish gate refused could ask for the export directly. Underneath it, two serialisers read past the caller's tenant: GenericObjectShareableConfigType::serialise() passed _rbac: false, _multitenancy: false (engine escape hatches, on a path a request reaches), and FlowShareableConfigType::serialise() went to FlowMapper::findByUuid() directly. All three fixed.

3. An object-scope federated share reached further than it granted.
buildScopeConfig() pins filters['uuid'] on the collection endpoint, but object() / updateObject() / deleteObject() took {id} from the URL and never compared it to the grant. A token for one object read, overwrote or deleted any object in the same register/schema — confidential ones included, because applyShareVisibility() deliberately skips the confidentiality filter for object scope. The item path was strictly wider than the list path that guards it. scopeCoversObject() closes it; register/schema/query scopes are untouched. The refusal is a 404, not a 403, so a scoped token is not an enumeration oracle.

4. A bulk row whose schema failed to load was written with no RBAC check.
resolveSafeguardSchema() swallowed every \Throwable into a bare null — which is also what a legitimate mixed-schema batch looks like, and that branch passes the row through unchecked. The write still went ahead, because the single-schema fast path re-resolves $schema downstream and never consults the safeguard's opinion. "Could not resolve" now refuses. resolveSafeguardRegister() is deleted rather than fixed: its result was assigned to $defaultRegister and never read.

Evidence

16 new tests across 5 files. Each was run with lib/ reverted and confirmed to fail, for the right reason — not merely to error:

Test	Failure without the fix
state refuses a foreign flow	FlowStateMapper::findByFlow('someone-elses-flow') was called
federation object-scope (×4)	the read/save/delete reached ObjectService
bundle gate	serialise() was reached
bulk safeguard	the smuggled row came back as passed, RBAC never consulted
flow serialise scoping	findByUuid on the unscoped mapper, called 1 time

Every refusal test is paired with a positive control, so none of them is satisfiable by an endpoint that refuses everything.

• Full unit suite: 16023 tests, same 29 errors / 2 failures as the untouched baseline measured on this same container (16007 tests) — i.e. zero regressions, all pre-existing (Doriath fixtures + two unrelated).
• phpcs, phpmd, psalm, phpstan: clean on every changed file.
• No suppression, no baseline entry, no .skip, no weakened assertion, no @spec exclude anywhere. Nothing added to phpmd.baseline.xml.

Also fixes one pre-existing gate-49 finding in a file this change already touches (setServeContext() had no @throws).

Gates, before and after

Diff-scoped as CI runs it, two gates remain red — both false positives, both reported upstream rather than worked around: ConductionNL/.github#160.

• gate-7 still flags FlowController::state() — the method this PR fixed. Fix a bug with uuid's that have integer-values #149 taught it to follow delegation, but only through an explicit deny (401/403/isAdmin/require*). OpenRegister's tenancy guard deliberately throws DoesNotExistException instead, because a 403 would leak the existence of another tenant's object. The gate's verdict did not change when the defect did; it would go green if I made the code leak. 17 of its 18 full-tree findings are this pattern.
• gate-9 flags the six #[PublicPage] federation endpoints. They are public because the caller is another server; the bearer share token is the credential. Its advice — "remove #[PublicPage] or remove body auth check" — breaks federation or opens it. Pre-existing on development.

gate-8 now passes (was 2). gate-16 caught that the changed methods carried no @spec — the federation serving surface had no spec at all, so this writes the change it implements rather than tagging something adjacent.

The upstream issue also reports a third defect found on the way: the fixed /tmp/hydra-gate-*.log paths make two parallel runs on one host contaminate each other's verdicts. Same sha, minutes apart, gate-39 flipped PASS↔FAIL and gate-7 read 18 vs 25. That is why the previously-circulated "24 failures" figure is wrong — it was measured under a shared /tmp. Under a private one the run is byte-identical across repeats.

Needs a product decision (not changed here)

FederationController::createShare is #[NoAdminRequired] with no group gate — any organisation member can mint an outgoing federated share exposing that org's register data to an external instance. The organisation is server-stamped, so it is not an IDOR, but it is a broader grant than federated_config_publish_groups gives the config-sharing path. Deliberate asymmetry, or an oversight?

[rubenvdlinde]

Full triage — every gate-6/7/8/9 finding, with the call path

Measured full-tree at f1614ce67 (before this PR), .github@main (which has #149), ajv resolvable, under an isolated /tmp. Identical at the pinned v1.0.1 — #149 changed nothing here.

gate-6 orphan-auth — 1 finding, 0 real

Finding

Verdict

NotificationReadState::isRead

False positive. Not an authorisation method — an in-memory read/unread tracker keyed (userId, notificationId), matched on the is* predicate heuristic. It is genuinely dead (the whole class has zero callers, tests included; the DB-backed NotificationReadStateEntity is the live one), but that is dead code, not a missing check. Not touched here — deleting a spec-mirroring primitive is a separate call.

gate-7 no-admin-idor — 18 findings, 2 real

Finding	Verdict	Call path
FlowController::state	🔴 REAL — fixed	went straight to FlowStateMapper::findByFlow($clientUuid); that mapper has no organisation filter
FederatedConfigController::bundle	🔴 REAL — fixed	no gate → FederatedConfigService::bundle → IShareableConfigType::serialise, whose implementations used _rbac:false,_multitenancy:false and FlowMapper::findByUuid
FlowController::{index,show,create,update,destroy,run}	FP	all six → FlowService, which applies activeOrganisation() on every read and belongsTo() in find(); save/delete/run all route through find()
FlowController::{eventCatalog,nodeCatalog,validate}	FP	no object reference at all — static catalogues, a scope-filtered palette, and a stateless preflight of the submitted body
FederationController::shares	FP	→ FederationShareService::listShares → FederatedShareMapper::findAll → applyOrganisationFilter() (MultiTenancyTrait)
FederationController::revokeShare	FP (3 hops)	→ setStatus → updateFromArray → find(int $id) → applyOrganisationFilter(); another org's share id raises DoesNotExistException → 404
FederationController::createShare	FP	organisation is server-stamped from activeOrganisationUuid(), never read from the payload — see the product question below
FederatedConfigController::{types,discover,fetch,publicKey}	FP	types is a static catalogue; discover/fetch hit GitHub with the caller's own stored credential; publicKey returns a public key

gate-8 unsafe-auth-resolver — 2 findings, 1 real (gate now PASSES)

Finding	Verdict
SaveObjects::resolveSafeguardSchema	🔴 REAL — fixed. The catch(\Throwable){return null} fed a caller whose null branch does $passed[] = $sanitised; continue; — i.e. skips the per-row PermissionHandler::hasPermission entirely. The write still proceeded, because the single-schema fast path re-resolves $schema downstream. Textbook CWE-863.
SaveObjects::resolveSafeguardRegister	Not exploitable — but genuinely dead. Its result was assigned to $defaultRegister and never read anywhere in the file. Deleted rather than fixed.

gate-9 semantic-auth — 6 findings, 0 real as stated (but the triage found a real defect underneath)

All six are FederationController::{objects,object,createObject,updateObject,deleteObject,meta}, rule public-page-annotation-with-auth-body. All six are false positives: these are #[PublicPage] because the caller is another Nextcloud server with no local session, and the bearer share token in the URL is the sole credential — the same shape NC core uses for public share links. The gate's advice ("remove #[PublicPage] or remove body auth check") breaks federation or opens it; there is no correct action.

However, reading those six call paths is how finding 3 in the PR body turned up: object(), updateObject() and deleteObject() resolved the token correctly and then never compared {id} to the share's objectUri. Gate-9 pointed at the right file for the wrong reason.

Counter-example held in mind

The dead catch (NotAuthorizedException) in the Merge/Transition controllers was not re-litigated: MergeService passes _rbac: true on every call, so that is misleading error semantics (403s surfacing as 404s), not a hole. Unchanged.

[github-actions]

Quality Report — ConductionNL/openregister @ f7f2c04

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-specs		✅
test-l10n		❌
composer			✅	✅ 173/173
npm			✅	✅ 713/713
PHPUnit					✅
Newman					✅
Playwright					✅
Hydra gates					❌

---

Quality workflow — 2026-08-05 13:43 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/openregister @ f7f2c04

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-specs		✅
test-l10n		❌
composer			✅	✅ 173/173
npm			✅	✅ 713/713
PHPUnit					✅
Newman					✅
Playwright					✅
Hydra gates					❌

---

Quality workflow — 2026-08-05 14:00 UTC

Download the full PDF report from the workflow artifacts.