gate-7 accepts a 401 authentication check as an authorisation guard — 4 real IDORs read as PASS (fleet-wide false negative)

[rubenvdlinde]

Summary

check_no_admin_idor.py's _GUARD_BODY_RE accepts Http::STATUS_UNAUTHORIZED as an authorisation guard. But a 401 is an authentication signal ("is anyone logged in?"), not an authorisation one ("may this caller touch this object?").

Nextcloud's idiomatic preamble for a #[NoAdminRequired] endpoint is exactly that check:

$user = $this->userSession->getUser();
if ($user === null) {
    return new JSONResponse(['error' => 'Authentication required'], Http::STATUS_UNAUTHORIZED);
}

Any method that opens this way is exempt from gate-7 for free, however unguarded the direct object reference that follows is. Since the preamble is the house style, this is not a rare shape.

Three-arm control on ONE file, changing only the guard shape

Canonical package ConductionNL/.github @ 81c8c97, checker invoked directly on
pipelinq/lib/Controller/LoyaltyController.php @ 206db4be (a copy in a scratch dir; no checkout modified).

arm	file	gate-7
A — unmodified controller, containing four real IDORs (getAccount, getAccountHistory, getRedemptionOptions, initiateRedemption)	as shipped	0 findings
B — plus plantBareUnguarded(string $accountId): #[NoAdminRequired], reads the account by the URL id, no auth of any kind	+14 lines	1 finding — method=plantBareUnguarded rule=no-auth-guard-in-body
C — plus plantAuthenticatedButUnauthorized(string $accountId): byte-identical body, preceded only by the 401 preamble above	+23 lines	0 findings

Arm B proves the gate is live on this exact file and this exact shape. Arm C, differing from B only by the 401 preamble, proves the preamble is what silences it.

Root cause

hydra-gates/scripts/lib/check_no_admin_idor.py:384-392:

_GUARD_BODY_RE = re.compile(
    r"OCSForbiddenException"
    r"|isAdmin\s*\("
    r"|->\s*(?:authorize|require|ensure)[A-Z][A-Za-z0-9_]*\s*\("
    r"|Http::STATUS_(?:UNAUTHORIZED|FORBIDDEN)"          # <-- UNAUTHORIZED
    r"|(?:statusCode:\s*|,\s*)(?:401|403)\b"             # <-- 401
    ...
)

The real IDORs this masked

All four are in pipelinq/lib/Controller/LoyaltyController.php, all #[NoAdminRequired], all take {accountId} straight off the URL and never scope it to the caller. getAccount's own docblock (:80) states "the caller MUST own the underlying klantId" — a contract written down and never implemented. LoyaltyAccountService::findAccountByKlantAndProgramme() exists as the means to scope it and is never called from the controller.

The worst is not a read leak: POST /api/loyalty/redemption/initiate/{accountId}/{optionId} returns 201, debits the victim's balance, and hands the caller the reward code. Filed against pipelinq separately.

Blast radius

In pipelinq alone: gate-7 reports 0 over the whole controller tree, and 49 of 82 controller files contain both #[NoAdminRequired] and Http::STATUS_UNAUTHORIZED. Every #[NoAdminRequired] method in those files that opens with the preamble is silenced regardless of what it does next. Other apps in the fleet use the same preamble.

Proposed fix — and why it should NOT be merged yet

Minimal and well-motivated: drop UNAUTHORIZED and 401 from _GUARD_BODY_RE, keep FORBIDDEN and 403. A 403 genuinely is an authorisation decision; a 401 is not, by definition.

⚠️ This has the false-RED shape: it will turn gate-7 red in many repos simultaneously, and it is the same sequencing hazard as .github#347, #356 and #358. Filing, not merging. The coordinator should sequence it, and every repo's existing "gate-7 clean" should be treated as unverified until it lands.

🔑 Related, and the reason this went unnoticed: gate-7's known problems so far (.github#353) were all false positives — a guard the regex failed to recognise. This is the opposite and worse shape: a non-guard the regex accepts. A gate whose known failure mode is "too noisy" gets its silences trusted.

[rubenvdlinde]

Control re-run with COMMITTED plants, over the FULL controller tree — same result

The original control ran the checker on a single copied file in a scratch directory. Two objections were available: the plants were not committed, and an isolated file cannot resolve collaborator guards, so the exemption might have come from somewhere else.

Both are now closed. A throwaway git repo containing pipelinq's entire lib/ and src/ at 206db4be, plants committed as their own commit, checker run over all 82 controller files:

arm	commit	gate-7
A — baseline, all 82 controllers, containing the four real IDORs	baseline: pipelinq lib+src at 206db4be	0 findings
B + C — both plants added to LoyaltyController.php in one commit	PLANT: bare-unguarded + authenticated-but-unauthorized	1 finding — lib/Controller/LoyaltyController.php:511 method=plantBareUnguarded rule=no-auth-guard-in-body

Both plants are in the same file, in the same commit, judged in the same run. plantBareUnguarded is reported; plantAuthenticatedButUnauthorized — whose body is byte-identical and which differs only by the no user → 401 preamble in front of it — is not.

That removes every confound: same tree, same commit, same invocation, same file, same collaborator resolution, same lack of a real guard. The only difference between the flagged plant and the unflagged one is the 401 preamble.

// PLANT B — reported
#[NoAdminRequired]
public function plantBareUnguarded(string $accountId): JSONResponse
{
    $account = $this->accountService->getAccount(accountId: $accountId);
    return new JSONResponse($account);
}

// PLANT C — NOT reported
#[NoAdminRequired]
public function plantAuthenticatedButUnauthorized(string $accountId): JSONResponse
{
    $user = $this->userSession->getUser();
    if ($user === null) {
        return new JSONResponse(['error' => …], Http::STATUS_UNAUTHORIZED);   // the only difference
    }

    $account = $this->accountService->getAccount(accountId: $accountId);
    return new JSONResponse($account);
}

[rubenvdlinde]

Fleet measurement — this reproduces, and it is worse than "pipelinq alone"

Independently reproduced with the three-arm committed-plant control, then swept
across all 18 fleet apps. Gate package: canonical ConductionNL/.github @
112d4c9 — pulled fresh, including #368, so none of this is the
#353/#360 false-positive class.

1. Reproduction

Isolated git repo I created (no fleet checkout was written; no plant ever entered
a fleet repository). Subject: a copy of pipelinq/lib/Controller/LoyaltyController.php
@ 67cb89b89. Both plants committed, in the same file, same commit, same run,
with byte-identical bodies apart from the preamble.

arm	canonical gate-7
A — baseline	0
B — plantBareUnguarded($accountId)	1
C — byte-identical body + only the no user → 401 preamble	0

Confirms this issue exactly as filed.

2. There is a third token, not two

#365 names UNAUTHORIZED and 401. _GUARD_BODY_RE has a third alternative
with the same defect:

r"|(?:::|->)\s*(?:forbidden|unauthorized|accessDenied)\s*\("

A $this->responses->unauthorized() helper is a 401 in helper-call spelling.
The fix should drop unauthorized from that alternation too, keeping
forbidden and accessDenied. (My numbers below drop only the two tokens this
issue names, so the delta is exactly attributable to what is filed here.)

3. Fleet sweep

For each app: git archive <ref> lib into an isolated dir, enumerate
lib/Controller/**.php (the same tracked set _enum_tracked feeds the gate), run
the canonical checker and the #365-patched variant over an identical file list.

app	ref	ctrl files	files w/ preamble	canon	patched
zaakafhandelapp	4f67378	25	23	0	89
procest	4061a600e	91	49	0	62
pipelinq	67cb89b89	82	49	0	49
doriath	bfd6da65d	49	31	0	42
shillinq	d63c0d32	83	66	0	35
hermiq	a4eca493	43	28	0	35
docudesk	288f587c	32	25	0	26
openregister	9628dc5d0	139	42	0	21
softwarecatalog	ebc93ff6	21	15	0	15
decidesk	443a77ed	40	18	0	14
openbuild	7e38e7966	21	18	0	10
openconnector	7c1d9c6d	46	32	0	9
opencatalogi	e22d91ab	24	7	0	4
launchpad	e24334b	45	29	0	4
larpingapp	526bbc5f	6	3	0	3
scholiq	39d7b1b	22	17	0	3
portaliq	eebb524	10	1	0	2
nldesign	101869f	12	0	0	0
TOTAL		791	453	0	423

gate-7 reports 0 in all eighteen fleet apps. 453 of 791 controller files
carry the preamble. This is not "pipelinq's house style" — it is the fleet's.

Positive control on the zeros (a zero from a gate looks exactly like an empty
scope): a bare #[NoAdminRequired] IDOR planted into the extracted tree of every
app I was about to call clean took the canonical checker 0 → 1 in nldesign,
launchpad, portaliq, scholiq and openregister. The gate is live everywhere;
the zeros are readings, not absences. All plants removed and verified absent.

⚠️ portaliq's "ALL 58 APPLICABLE GATES PASSED" contains a gate-7 PASS
produced this way. Every "green" claim in this programme inherits it.

4. Hand-verification: ~85% of the delta is a real IDOR

Five agents read the candidates against the actual service/mapper bodies. Of the
methods that take a caller-supplied object id:

app	candidates	REAL	reclassified
zaakafhandelapp	58	55	3 (dead TEST_ARRAY stub routes)
procest	39	33	6
pipelinq	26	23	3
shillinq	24	23	1

Worst confirmed, one per app:

• zaakafhandelapp DELETE /api/objects/{objectType}/{id} — one route, 17
  object types incl. zaken/klanten/besluiten, arbitrary id, straight to
  $mapper->delete(['id' => $id]).
• procest GET /api/kcc/voorblad — raw burgerId off the query string
  returns that citizen's open cases plus contact history; iterating BSN-shaped
  ids walks the resident population.
• pipelinq POST /api/handoff/contract/{id}/send-to-invoicing — mints a real
  invoice in shillinq against a contract the caller never owned.
• shillinq POST /api/vat-returns — injects a statutory Dutch VAT filing
  into any administration; the callee's own docblock says the parameter is
  "Server-resolved".

5. Two things a patched gate-7 will still miss — both produced real findings

1. A format validator whose NAME or DOCBLOCK claims scope.
   TaxReportController::validateAdministration() is entirely
   preg_match('/^[A-Za-z0-9_.\-]{1,64}$/'). Ditto validId(), and
   @param … ADR-005 IDOR-safety docblocks in two more shillinq controllers.
2. A FAKE tenant guard reading instance config rather than the session —
   SepaAuditService.php:391 compares against
   appConfig->getValueString(APP_ID, 'administration_id', '').

Both are guard-shaped and would satisfy any regex looking for an if returning
403/404. Conversely SemanticHandoffController::loadObject() is labelled
per-object guard in prose only.

🔑 Suggested follow-on detector: grep for the claim of a guard (docblock
tokens IDOR, per-object guard, RBAC, ADR-005) and diff it against the
presence of getUID() / a canAccess-style call in the same call tree. In this
sweep that heuristic would have caught four separate false assurances, three of
which are the most dangerous items found — because a future reviewer reads the
comment and marks the endpoint done.

6. Sequencing

I agree this must not be merged into a running wave — it turns 18 repos red at
once. But the sequencing argument cuts the other way on disclosure: every
"gate-7 clean" statement in this programme is currently false, and agents are
still making decisions on them. Suggest landing the fix behind the same
coordinator gate as #347/#361, but updating the gate-7 PASS message
immediately to say that a PASS does not exclude a no user → 401 preamble.

Companion issue filed as .github#372 — gate-7's Pattern 2 tells the reviewer
to look for the guard one layer down, and for most of the fleet OpenRegister is
default-OPEN there, so the second look also comes back clean.

Per-app issues filed: ConductionNL/zaakafhandelapp#347 ·
ConductionNL/procest#801 · ConductionNL/pipelinq#801 · ConductionNL/shillinq#518.

---

Confirmed by: the runs above. Full harness, per-app deltas, candidate lists
and the committed-plant rig: /home/rubenlinde/fleet-board/findings/gate7-fleet-reaudit.md.

[rubenvdlinde]

Live fleet evidence: gate-7 says PASS on pipelinq's honest wide-scope run, over the four real IDORs

Not a local reproduction this time — the actual development → beta release-PR run, which is the widest scope this repo produces:

run 31525663831 · sha 67cb89b89 · Base ref: origin/beta ($HYDRA_GATE_BASE_REF) = 297f4bfb
COVERAGE: 61 of 64 declared gates reported a result (3 not applicable; 61 of 61 applicable gates ran)

gate	verdict
gate-16 spec-coverage	FAIL — 185
gate-19 e2e-coverage	FAIL — 10
gate-61 listener-work-placement	FAIL — 4
gate-57 orphaned-write-capability	FAIL — 3
gate-25 contract-coverage	FAIL — 2
gate-6 orphan-auth	FAIL — 1
🔴 gate-7 no-admin-idor	PASS

Six gates found something. gate-7 found nothing — on a tree containing LoyaltyController::getAccount, getAccountHistory, getRedemptionOptions and initiateRedemption, all #[NoAdminRequired], all taking {accountId} straight off the URL with no ownership check, one of which returns 201 while debiting another customer's points and handing the caller the reward code (ConductionNL/pipelinq#781).

This is not a narrow push scope or a diff artefact: 61 of 61 applicable gates ran, and this is the table a reviewer would read as the app's honest security posture.

So the three-arm control above is not just a laboratory result — the false negative is live, on the widest scope the pipeline produces, in the cell most likely to be trusted.

⚠️ Restating the sequencing point: gate-7 currently reports 0 in this repo. After the fix it will report at least 4 here, and an unknown number elsewhere. That is the false-RED shape — please sequence it rather than merging it into a wave.