fix(gate-7): an authentication check is not an authorisation guard (#365)

[rubenvdlinde]

Closes #365.

Note on wording: this description deliberately avoids reproducing verdict-shaped
lines. The workflow exports the PR body into the gates log, so a pasted verdict
line reappears there and reads as a finding of the very run meant to prove the
fix. Counts below are given in prose and in a table instead.

What was wrong

gate-7 reported zero in all eighteen fleet apps. Every one of those zeros came
from a checker that accepts "is anyone logged in?" as an answer to "may this
caller touch this object?".

Reproduced before touching anything, with a three-arm committed-plant control —
same file, same commit, same run, byte-identical data-access bodies, only the
preamble differing:

arm	canonical gate-7
bare unguarded method	1
the same body, preceded only by a no user -> 401 preamble	0
the same body, plus a real per-object ownership check	0 (correct)

Scale, measured against the canonical package at 57bcb2b: zero reported
fleet-wide, while 453 of 791 controller files carry that preamble.

Positive-controlled in all 18 apps. A zero from a gate looks exactly like an
empty scope, so a bare #[NoAdminRequired] IDOR was planted into each extracted
tree and the canonical checker re-run: it moved from 0 to 1 in every single one,
then every plant was removed and verified absent. Each zero is a measurement.

Under #[NoAdminRequired] Nextcloud's middleware has already rejected the
anonymous caller before the method runs, so that preamble cannot even fail. It
was nonetheless silencing the gate, through three separate guard patterns.

Why the fix is not "drop 401 from the regex"

That is what #365 proposes, and it is wrong in both directions. Both are now
pinned by tests.

Too wide. A genuine per-object decision that answers with the wrong status —

if ($account['ownerId'] !== $user->getUID()) {
    return new JSONResponse([], Http::STATUS_UNAUTHORIZED);
}

— is a real guard. Deleting the token turns it into a false positive, and false
positives (#353, #360) are precisely how gate-7 lost its credibility and how its
silences came to be believed.

Too narrow. The same authentication clause answering 403 keeps clearing.
One token of edit, made by someone chasing a green cell, buys the silence back by
making the response worse.

So this treats authentication-ness as a property of the condition, not of the
status code. An if whose condition tests only whether a caller identity is
absent, and whose consequent refuses, is blanked to same-length whitespace
before any guard pattern runs. Byte offsets, spans and reported line numbers are
unchanged; only the text the guard patterns get to see is.

Three controls keep the blanking narrow, each closing a way it could have eaten a
real guard:

1. Polarity — absence tests only. if ($user !== null) { ...whole body... }
   is a wrapper, and blanking it would erase every guard inside.
2. Argument-free operands only — $this->access->canAccess($id, $user) and
   $account['ownerId'] can never be read as an identity, however they are
   spelled. The first is a guard; the second is object data. This one was caught
   by the existing suite, which went red on the first draft.
3. The consequent must refuse. A conditional that merely computes is left
   alone.

Identity recognition is deliberately spelling-agnostic. #365's own re-audit built
the opposite mistake into its triage tool — three enumerated spellings of "who is
the caller", sessionUserId() matching none — and manufactured 19 false positives
from that gap.

Two companion patterns, and why they ship in the same PR

Removing the clear is what exposes them. Shipping without them would have
reported 45 findings in doriath alone, an app whose real gate-7 exposure was
hand-verified as zero.

• Pattern 6 — session-identity hand-off. Every call that receives a
  caller-supplied value must also receive a session-derived identity, so the
  object reference is resolved under a scope the caller cannot forge. An
  ALL-quantifier, not ANY: one unscoped data call still reports, so an audit-log
  line cannot clear a real IDOR. Suppresses 86 fleet-wide.
• Pattern 7 — in-body ownership comparison, whatever status it answers with,
  including the 404 chosen deliberately so a 403 cannot become an existence
  oracle. That sentence was already printed by this gate's own failure guidance;
  it was advice to a human and is now code. Suppresses 2 fleet-wide, and both
  are endpoints an earlier audit named specifically as over-reports.

Blast radius

Canonical development clones from GitHub, not local checkouts — three local app
directories turned out to have no .git at all and silently resolved the parent
repository's ref.

app	ref	ctrl files	files with the preamble	today	with this PR
zaakafhandelapp	bef3a7c	25	23	0	86
pipelinq	ccf0ff3	82	49	0	52
shillinq	5d31898	83	66	0	40
procest	347f2a4	91	49	0	32
hermiq	7cd256c	43	28	0	20
docudesk	67d5201	32	25	0	19
decidesk	445776b	40	18	0	17
softwarecatalog	ea4dc7e	21	15	0	14
openregister	df815c3	139	42	0	10
opencatalogi	f6e86ab	24	7	0	9
openbuild	6772ff6	21	18	0	9
doriath	bfd6da6	49	31	0	5
scholiq	d2a0a1c	22	17	0	4
launchpad	19e9b49	45	29	0	3
larpingapp	9704abd	6	3	0	3
openconnector	6da5dfb	46	32	0	2
portaliq	65cafbd	10	1	0	2
nldesign	8e49697	12	0	0	0
total		791	453	0	327

This is a candidate count, not an exposure count. An earlier hand-audit of a
similar candidate list found per-app precision ranging from 95% to 0%. Read the
findings; do not quote 327 as a defect count.

Spot-checked that the fix does not swallow the real ones: shillinq's
arbitrary-file-read download endpoint, its SEPA mandate export, pipelinq's
semantic-handoff routes, docudesk's cross-register preview, decidesk's motion
history, opencatalogi's unscoped search, and zaakafhandelapp's object routes are
all still reported. procest's and openconnector's counts are low because both
landed real guards earlier today; the guards were read and confirmed rather than
assumed.

Coverage added

A new authn-vs-authz bundle in the gate-acceptance matrix drives the control
through the real wrapper. Both arms keep the 401 preamble, so the clean arm
pins that the preamble is ignored, not punished — without that, "flag anything
containing a 401" would pass. Its expect.conf requires the finding to name
method=preamble rather than the file path, because the bare arm names the file
under the old checker too; a file-level assertion would have been satisfied by a
checker with the defect intact.

Proven able to fail: running that bundle against the pre-fix checker turns it
red, on exactly that assertion, with the driver reporting that the gate failed for
some other reason and the fixture therefore proves nothing.

Four existing unit tests encoded the defect as correct behaviour and are inverted,
each carrying the reason. The sharpest is the decidesk citizenAction() case: it
sits beside staffAction() in one collaborator, and only staffAction() is an
authorisation guard — the 401 arm was borrowing credibility from the 403 arm next
door.

Verification

• 113 unit tests for this checker, green, up from 95.
• 74 helper suites green, 2 quarantined as documented.
• 61 entry-point tests green.
• One helper suite is red — the gates 45-to-55 acceptance suite — and it is red
  identically on an unmodified canonical clone: ajv is not resolvable in this
  environment and the suite refuses to run rather than emit a false verdict. Not
  caused by this change.

Not verified

• No HTTP probe against a running instance. Every claim about a specific app's
  endpoints here rests on reading source.
• The residual Pattern 6 admits: a callee that accepts the identity argument and
  ignores it. This gate sees one method body by contract, so that residual is
  symmetrical with the guidance already printed with its findings, and the
  ALL-quantifier narrows it rather than removing it.

Merge

Deliberately not merged. It turns a green cell red in all eighteen apps at
once, and the sequencing is Ruben's call.

---

Extension, 2026-08-12 — gate-7's SECOND blinding exemption: #[PublicPage]

Same note on wording: prose and tables, no verdict-shaped lines.

What was wrong

#[PublicPage] was skipped unconditionally, commented "intentionally open to
unauthenticated callers". That is a claim about the caller, where the gate's
question is about the object. The exemption carried no stated reason, so
there was nothing to audit.

Reproduced first, on a nine-arm rig — one file, one commit, one run, predictions
written before the run. A byte-identical IDOR plant carrying #[PublicPage],
including an unauthenticated write to an arbitrary $id, cleared the gate.

It is not safe by construction. opencatalogi#856, merged as 963f832a, was
exactly this shape: GET /api/themes/{id} was @PublicPage and its find()
carried no register and no schema, so it resolved the caller's identifier in any
register on the instance and served an anonymous caller a municipal zaak status
record.

🔑 The exemption is the smaller half of the blinding

Deleting it and changing nothing else moves one arm in nine. A
#[PublicPage] method carrying no #[NoAdminRequired] never reaches the
exemption — the scope filter drops it one branch earlier. Across the fleet that
is 267 of 357 public controller methods (74 %), including all 78 of
procest's. So the scope filter widens too.

The predicate

There is no current user, so ownership guards do not transfer. A public handler
must constrain the lookup to a scope it declares public rather than resolve
the identifier globally. In scope: a caller-supplied scalar reaches a selection
call — array $data is a payload (portaliq's public forms are untouched), and a
$token/$secret is the authorisation, not a selector. Cleared when every such
value reaches a constrained selection, by five spellings each read out of a fleet
app: a non-caller argument that carries a value (a bare literal such as
_rbac: true does not constrain — 963f832a's own message says why); a
callee declaring publicness as a complete CamelCase segment, so
getPublicationById is not mistaken for one; a receiver already scoped as state;
a container resolution whose result constrains a later lookup and whose miss is
refused; and one resolved delegation hop. Judged per selector, not per call,
because "verify in scope, then act" is the commonest correct public shape.

Two-directional control, on a real endpoint

opencatalogi ThemesController::show — same file, same method, same checker,
the shipped security fix in between — is reported at f6e86ab and is silent at
963f832. PublicationsController::show/uses/used/index, fixed by the
same commit, also go silent.

Also here, and load-bearing

A same-class helper counted as guard-bearing if its body matched a bare throw,
matched against source with comments in it. hermiq's loadOwnedAgent()
contains no throw statement at all — the word appears only in a comment
explaining why it catches one — and that cleared all four routed methods
calling it. opencatalogi's getObjectService(), a service locator that throws
when OpenRegister is absent, cleared every caller in the app, which is why the
new pattern could not see the endpoint that motivates it. The body test now reads
comment-free text, a throw must name an authorisation exception, and an
ownership comparison answered with null is recognised instead — so no true
clear is lost. hermiq is unmoved; so is doriath.

TemplateResponse no longer clears a @PublicPage method: its stated reason is
that Nextcloud guarantees a session, and @PublicPage is the annotation that
turns that guarantee off.

Blast radius

18 canonical clones on current development, identical file lists. The fleet
total moves by eight: six under the new rule name
publicpage-unscoped-object-lookup (openregister 3, pipelinq 1, portaliq 1,
softwarecatalog 1) and two from the helper-evidence repair (opencatalogi 1,
launchpad 1). doriath — the app that gets scoping right — is unmoved, and so is
hermiq.

All eight were hand-read. One is a live exposure: openregister
NamesController::show resolves any identifier anonymously and returns its name,
which is #856's shape one field narrower. One is a real pre-existing #365-class
finding surfaced by the helper repair (opencatalogi ListingsController::index).
The other six are endpoints whose publicness is a claim — a public catalogue
keyed by a public identifier. The gate cannot tell those from an object store
without knowing which schemas are public, and that is the point: the remedy is
@no-admin-idor-exempt <reason>, which turns a reasonless blanket exemption into
an auditable one.

Coverage

Unit tests 113 → 130; nine of the seventeen new ones are red against the
pre-fix checker. New repo-shaped acceptance bundle publicpage-scope under
#359's matrix — both arms keep #[PublicPage] on every method, so the clean
arm pins that the annotation is ignored, not punished, and the subject is a
method name rather than a file path. Verified able to fail: run against the
pre-fix checker, the bundle goes red on exactly that assertion.

Not verified

No HTTP probe against a running instance — every app verdict rests on reading
source. Pattern 2's premise (that OpenRegister's RBAC decides for an anonymous
caller) is contradicted in prose by 963f832a's own commit message and remains
unaudited; withholding it from @PublicPage methods changes nothing on
openregister today, so it is not what masks those endpoints. Two pre-existing
weaknesses are named but not fixed here: scan_file still matches its guard
patterns against comment-bearing text, and an exception-to-status mapping inside
a catch block still reads as an authorisation decision.

[rubenvdlinde]

CI settled: 16 pass, 15 skipping, none failing, none pending — count stable at 31 across eleven consecutive polls, since a check list read too early is trivially all-non-pending.

The job that matters is Package invariants, which runs the helper suites: seventy-five green, two quarantined as documented, none red. test_gate_acceptance_matrix.sh is among them, so the new authn-vs-authz bundle is enforced from here on.

One correction to the description: the gates 45-to-55 acceptance suite, reported above as red locally, is green in CI. It refuses to run when ajv is unresolvable rather than emit a verdict it cannot support, so the local red was the machine and not the change — and CI is what establishes that rather than my having said so.

Still deliberately not merged.