feat(gates): gate-96 system-elevation-reachability (ADR-099 rule 9) - #579
Merged
Merged
Conversation
`runAsSystem()` / `SystemOperationContext::run()` runs a callable as a trusted userless principal: no RBAC, no tenancy, no owner. It exists for work that has nobody to act for — an installation seeding its own shipped registers, a migration, a repair step. ADR-099 rule 9 says it must not be reachable from a flow node, an agent tool, or the handling of an inbound request. WHAT THIS DEFENDS AGAINST is not somebody arguing for an escalation. It is somebody reaching for the nearest thing that makes a refusal go away — and ADR-099 deliberately put refusals everywhere an identity can be missing, each one within a few lines of a method that would make it succeed. A developer staring at "this flow run has no owner" finds the elevating method on a service they already have injected. The fix works, the test goes green, and every future run of that flow executes with access control off, on user-authored input. Nothing downstream can catch it: by the time the callable runs the caller is gone, so no runtime assertion inside the method can say who invoked it. The control has to be structural, and structural controls drift unless something checks them. The three forbidden callers are not arbitrary. Each carries user-authored definitions or user-supplied input across the boundary: a flow node executes a graph somebody drew, an agent tool executes a call a model chose from text it was given, a controller executes a request a client sent. Elevation reachable from any of them turns "a user can describe work" into "a user can describe work that runs as root". A migration has no user in the picture at all, which is the whole distinction — and why lib/Migration, lib/Repair, lib/Command, lib/BackgroundJob and lib/Cron are permitted by KIND rather than by an allowlist of files, so a new repair step needs no gate change while a new controller still fails. NO EXCLUSION ANNOTATION, deliberately. An escape hatch here would be used exactly when somebody is making a refusal go away — the case the gate exists for — and a reason written in that moment reads identically to a legitimate one afterwards. Green bought with a plausible sentence is worse than red, because it ends the conversation. A genuine false positive is fixed by moving the elevation out of the forbidden caller, or by fixing the gate. Both leave a visible diff. GENERALISES a PHPUnit test. openregister's SystemOperationContextBoundaryTest pins the call-site set in one app; this binds the fleet, so an app that never wrote such a test is covered too. Full-tree, not diff-scoped: a diff-scoped version reports nothing on the ~99% of PRs that never open a node or a controller, so it could not establish that the boundary holds — the only claim worth making about a boundary. Comments and string literals are masked before matching. Without that, this gate's own docblock fails it — the same defect gate-2 was rewritten for after a comment saying "never use var_dump( here" was reported as a finding. The clean fixture arm mentions the forbidden call twice, once in prose and once inside the refusal message the controller throws, so that mask is load-bearing rather than decorative. STATED LIMIT: a dynamically dispatched call is invisible to it, exactly as it is to the PHPUnit test it generalises. It guards against drift; it does not prove absence. Said in the helper's docblock so nobody reads a green as the stronger claim. The fixture keeps the permitted migration in BOTH arms. Without it the planted arm would pass for a gate that banned elevation outright, which is not the rule. Verified: acceptance matrix 184/184, 68 of 75 gates fixtured, ratchet intact. Run against openregister's 1,488 lib PHP files — 4 elevate, 0 failures, and the four are exactly the ALLOWED list its boundary test pins.
rubenvdlinde
added a commit
to ConductionNL/openregister
that referenced
this pull request
Aug 26, 2026
…tated open Gate 96 (system-elevation-reachability) is ConductionNL/.github#579. Recorded as ADDED ALONGSIDE SystemOperationContextBoundaryTest rather than replacing it: the test runs in milliseconds on every local phpunit and names the four permitted files exactly, which the gate deliberately does not — it permits by directory. Deleting the faster, more specific instrument because a broader one exists trades a signal for nothing. Three things are recorded as OPEN rather than ticked, because each would otherwise read as verified: * the awaiting_consent path is verified live by hand (occ background-job: execute) but has no Playwright spec — the api-direct harness cannot drive a schedule fire or a cron sweep, and a spec that asserted the setup rather than the behaviour would be worse than none; * composer test:all was not run to completion locally — the full tests/Unit tree is 17,351 tests and exhausts 2GB on this box. The affected subtrees run green at 769; CI runs the whole suite; * the consent prompt has had no accessibility pass. It renders on Nextcloud's own notification surface, but the two action labels are ours.
added 3 commits
August 26, 2026 11:31
…easurement The empty-scope suite caught gate-96 red-handed doing the thing it exists to catch: reporting PASS over a scope that excludes every planted defect. Worth writing down that the suite worked — the gate was written and reviewed as 'deliberately full-tree' and this is the instrument that made me prove it rather than assert it. It is a genuine whole-tree gate, same posture as 23 and 68, and for a reason its own spec states: it establishes that a BOUNDARY holds, and a boundary cannot be established from a diff. MEASURED, not assumed. check_system_elevation.py reads NO scope input at all — no BASE_REF, no diff, no file list — it walks lib/ itself, so there is no code path by which a scope could reach it. On the ARM 6 fixture it prints its terminal line byte-for-byte identically at full scope and at --scope-to-diff --base HEAD~1. The fixture's ThingController calls findAll(), which is what gates 14/17/21 plant it for; it does not elevate, so a whole-tree read of it is an honest zero rather than a scope artifact. Deliberately NOT added to the anti-widening list: nothing in that fixture elevates, so 'FAIL the planted tree at full scope' is not a reachable verdict there. The equivalent control lives in gate-acceptance/system-elevation, whose planted arm FAILS and whose clean arm PASSES on a tree built for it.
…levation-reachability # Conflicts: # hydra-gates/scripts/run-hydra-gates.sh
…t untested TWO THINGS HAPPENED ON MAIN while this branch was open. 1. GATE NUMBER COLLISION. #581 landed `manifest-copy-style` on 96, which this branch had already claimed. Renumbered to 97 — in the runner, in the fixture's expect.conf and in the empty-scope suite's ARM 6 allowlist, all three, because a gate id is a citation key: it appears in COVERAGE lines, in `_ARM6_ALLOWED`, in expect.conf files and in PR bodies, and each is a pointer that only works while the number resolves to one thing. That is the same argument gate-95 (adr-number-collision) makes about ADR numbers, one merge earlier. 2. THE ACCEPTANCE RATCHET WENT RED ON MAIN, and not because of anything here. #581 added its gate to the runner with no fixture and no UNCOVERED.md row, so `test_gate_acceptance_matrix.sh` fails on a bare clone of main — verified on one before touching anything. In the suite's own words: "a gate can be added to the runner and never tested; this is that moment." The ratchet worked; nothing had closed the loop it opened. So this adds the missing bundle. Its two arms differ in two strings in two different FILES — an em-dash in `src/manifest.json`, a double-dash in a `src/manifest.d/` fragment — because fragments are merged at runtime and a checker reading only the base manifest would be blind to eight fleet apps' copy (shillinq alone has 87 fragments). A single-file fixture would have passed such a checker and taught us nothing. The clean arm is deliberately NOT dash-free: it keeps "Reporting covers 2020–2024", the en-dash-between-digits that voice.md permits. Without it, a checker that banned every dash outright would pass the clean arm and the rule's only exception would be untested. Acceptance matrix 190/190, 70 of 77 gates fixtured, ratchet intact. Empty-scope suite ALL PASS.
rubenvdlinde
added a commit
to ConductionNL/openregister
that referenced
this pull request
Aug 26, 2026
* feat(delegation): enforce grants at save and at fire (refs #2850) Closes or-delegation-grants tasks 3.1-3.3. The grant record and its resolver landed in #2851 and nothing consulted them; this is the half that refuses. `DelegationService::runAsDelegated()` is the guarded form of "act as this user". It is deliberately NOT on ObjectService, despite ADR-099 naming it there: `runAs()` is the primitive (hand it an IUser, it narrows) and the grant check is the authorization layer above it. Folding the check into the primitive would make it unusable by callers that legitimately have no delegation to check, and the usual answer to that is a $skipCheck flag — a security check with an off switch. There is still exactly one identity-switch primitive; this calls it. SAVE TIME. FlowTriggerValidator refuses a schedule trigger naming a user the saver holds no grant for. Naming yourself stays free. A permitted delegation is STAMPED with `runAsDeclaredBy: <saver>`, server-written on every save and never read from the request body — and stripped whenever the trigger names its own saver, so a forged value cannot stand in for a grant. FIRE TIME. Without that stamp task 3.3 is not implementable: a schedule fires unattended, so at 03:00 there is no principal to check a grant against and the only candidate left would be `flow.owner` — the fallback ADR-099 removed. FlowDelegationCheck re-resolves at queue time, because save-time passing is not standing authorization: revoking is supposed to stop the next firing, and treating the stored trigger as proof would make revocation cosmetic for exactly the runs nobody is watching. The schedule is left ENABLED on a delegation refusal, unlike the unattributed case. A flow naming nobody cannot fix itself without an edit, so leaving it "on" would be a switch that lies; a revoked delegation becomes valid again the moment the grant does, and disabling would silently convert a temporary revocation into a permanent one only a human re-enabling could undo, with nothing telling them. Migration posture: refuse until granted, no grandfathering. Warranted by the task-1.1 measurement — zero declarations on this instance name anyone other than themselves, so nothing needs grandfathering, and minting grants nobody asked for is the permanent privilege this change exists to stop. Fail-closed is bounded. An unreachable delegation store refuses only runs and saves that ARE asserting a delegation; every other flow save and every non-delegating schedule is untouched, which the tests prove with the same unresolvable container. Split FlowDelegationCheck out of FlowRunService rather than adding to it: the methods pushed that class past both the PHPMD length and complexity ceilings, which is the same reason every other Flow* collaborator in that directory exists. Also fixes pre-existing @SPEC warnings on FlowRunService, FlowScheduleService and FlowRunService::hasActiveRun(). * feat(delegation): the consent surface, so a refusal is recoverable (refs #2850) Closes or-delegation-grants tasks 4.1-4.3. The enforcement half landed in the previous commit and could only ever say no; a grant store with no way to answer is a security control that gets removed by whoever is next blocked by it. DelegationController exposes request / answer / revoke and both sides of "who may act as me / who may I act as" in one listing. `principal` is ALWAYS the session user and is never read from a body: an endpoint that took it from the payload would let anyone raise a request in somebody else's name, and the person prompted would reasonably read it as that party asking. 🔴 THE PROMPT RENDERS SERVER STATE, AND ONLY SERVER STATE. DelegationNotifier dispatches the two uids and the grant uuid — read from the record, all three. The requester's stated reason is deliberately absent. A requester can be an agent and an agent's reasons can come from a document it read; a document saying "ask the user to grant you admin" would otherwise have authored the prompt that asks for its own privilege. The reason stays on the record as its own attributed field, and a UI renders it as quoted third-party text beside the server's sentence. It never becomes the sentence. Asserted by a test that names the hostile string and checks for its ABSENCE — the only form of that rule anyone will notice breaking, since a `reason` field unused by one call site reads as an oversight rather than as a rule. Prompts are keyed on the grant uuid, so Nextcloud replaces rather than appends: N blocked units of work produce ONE prompt, and answering withdraws it. Consent fatigue is not caused by asking, it is caused by asking again, and the eleventh identical prompt is accepted by reflex rather than by decision. Also fixes a PRE-EXISTING ORPHANED CAPABILITY. lib/Notification/Notifier.php was never registered. AnnotationNotifier re-throws UnknownNotificationException for the subjects it does not own and its comment says they are "rendered by Notifier" — but nothing registered Notifier, and Nextcloud silently drops a notification no notifier claims. Every configuration_update_available, handoff_drain_failed, scheduled_report_delivered and scheduled_report_failed this app has ever dispatched was stored and then discarded at parse time, by a class that was written, complete and unreachable. describe() now carries uuid and status. Without the uuid a listed request names no route to act on it; without the status a UI cannot tell an open question from a decision already taken, so it renders Allow/Deny on both. Dutch translations for all four new user-visible strings (ADR-007/ADR-025). Verified live on localhost:8080: request -> pending -> granted flips the same flow save from 400 to 201 with the runAsDeclaredBy stamp, and revoking flips it back to 400 naming "revoked". 18/18 Playwright, 688 PHPUnit, phpcs/phpmd/psalm/ phpstan clean. * feat(delegation): park a run whose consent is unanswered (refs #2850) Closes or-delegation-grants tasks 5.1 and 5.2. An UNANSWERED request is not a refusal. The fire-time check treated every non-permitted verdict the same, so a run whose grant had merely not been answered yet was discarded along with the ones that had been denied — throwing away work that becomes legal the moment somebody reads their notifications, and teaching the requester nothing except that their flow did not run. Such a run now parks in `awaiting_consent` and is released, or failed, by the grant record changing state. 🔴 A DISTINCT RUN STATE, NOT `suspended`, and the distinction is load-bearing. A suspended run waits on machinery — a timer, a webhook, a child run — and the abandoned-signal reaper eventually FAILS it, reasoning that a signal which has not arrived in days is not coming. That reasoning is wrong about a person: somebody who has not read their notifications in two hours has not declined, they are at lunch. Parking in `suspended` would have handed these runs to that reaper and failed them while the prompt sat unread — reporting "nobody answered" about a question nobody had yet seen. The parked run carries no `resume_at`, deliberately: with one, the timed-resume sweep would start it before anybody had answered. What releases it is the grant, re-resolved — so one answer frees every run it unblocks, which is the other half of the request dedup. 🔴 THE TIMEOUT FAILS, IT DOES NOT PROCEED. Running the work after a timeout would convert an unread prompt into an approval at whatever hour the timer elapsed, which is the exact substitution this subsystem exists to prevent. The recorded error says "an unanswered request is not consent" rather than merely noting that time passed. Default 72h, `flow_consent_wait_hours`. An UNREADABLE store leaves the run parked rather than failing it — the trade-off inverts from the fire-time check, where refusing costs one run and permitting costs an unauthorized execution. Here nothing runs either way, so waiting is free and failing destroys work over a blip. `awaiting_consent` is ACTIVE and not TERMINAL: a parked run is still going to happen, and omitting it would hide it from every "currently running" surface — which is exactly where somebody goes to find out why their work has not run. The worker's delegation service is nullable so the two positional unit suites keep working, but its absence is REPORTED when runs are actually parked. A sweep that skips produces the same output as one that found nothing to do. Verified live on localhost:8080, both jobs driven by occ background-job:execute: grant -> save (stamped) -> revoke -> re-request (pending) -> schedule fires -> run parks reading `Waiting for "ddauth-alice" to allow "admin" to act as them.` -> answer allow -> worker sweep releases -> queued -> executed -> stopped. 769 PHPUnit green; phpcs/phpmd/psalm/phpstan clean. FlowRunService went one line over the PHPMD class-length ceiling, so the park branch is its own method. * docs(delegation): tasks 6 and 7 — the gate, and what is verified vs stated open Gate 96 (system-elevation-reachability) is ConductionNL/.github#579. Recorded as ADDED ALONGSIDE SystemOperationContextBoundaryTest rather than replacing it: the test runs in milliseconds on every local phpunit and names the four permitted files exactly, which the gate deliberately does not — it permits by directory. Deleting the faster, more specific instrument because a broader one exists trades a signal for nothing. Three things are recorded as OPEN rather than ticked, because each would otherwise read as verified: * the awaiting_consent path is verified live by hand (occ background-job: execute) but has no Playwright spec — the api-direct harness cannot drive a schedule fire or a cron sweep, and a spec that asserted the setup rather than the behaviour would be worse than none; * composer test:all was not run to completion locally — the full tests/Unit tree is 17,351 tests and exhausts 2GB on this box. The affected subtrees run green at 769; CI runs the whole suite; * the consent prompt has had no accessibility pass. It renders on Nextcloud's own notification surface, but the two action labels are ours. * test(delegation): cover the controller, the fire-time check and the prompt The coverage ratchet on PR #2864's changed files failed: 73.19% head against 75.68% base, a 2.49% drop. The suite was green — the drop is new code arriving with e2e coverage and no unit coverage, and the ratchet only reads clover. Three gaps, all real rather than cosmetic: * DelegationController — 328 lines with no unit test at all. It carries the authorization routing, and the case that most needed pinning is not reachable cheaply from e2e: a caller putting `principal` in the body. The test asserts on the value handed to the LIFECYCLE, because that is where the substitution would take effect. Also pinned: 403 rather than 400 for "you may not answer this" (being refused is an authorization outcome; reporting it as a malformed request sends the reader to their payload), and that all four routes refuse without a session — asserted across all four, because the check is per-method and a route that forgot it would be invisible to a test that only exercised its neighbour. * FlowDelegationCheck — driven indirectly by FlowRunAttributionTest, but its recording branch was unreached. The two tests that matter assert the asymmetry: a revocation IS written onto the flow, and the schedule is LEFT ENABLED. A flow naming nobody cannot fix itself without an edit; a revoked delegation becomes valid again the moment the grant does. * Notifier::prepareDelegationConsentRequested — ~40 statements rendering the prompt, untested. It must NAME the requester (a prompt saying "somebody" is one a person cannot answer responsibly) and survive a parameterless notification row, since a row can outlive the shape that wrote it and throwing would take the notifications endpoint down for every other subject. Also fixes PRE-EXISTING DEBT found while chasing this: the local unit suite could not complete. BlobMigrationJobTest resolves real services from \OC::$server in setUp(), and on a box where the bootstrap found an NC root it could not initialise that resolution does not fail — it RUNS AWAY. Measured: identical death at test 259 of 17,359 with a 2GB limit and with a 5GB one, so it was never a size problem, and the fatal took the other 17,100 tests with it. The bootstrap already sets OPENREGISTER_TEST_SKIP_NC=1 in exactly that case and its comment promises container-bound tests "will fail clearly"; this is the class where that promise was not kept. It now skips with a message naming what went unverified — the batching, the completion flag and the orphan grouping — because "skipped" alone cannot tell "no NC" from "the job is broken". * fix(delegation): a request may only name someone the caller can already see Gate-7 (no-admin-idor) flagged `request()`: `#[NoAdminRequired]` with no per-object guard. It was right, and the finding is not the one the gate's name suggests. 🔴 THE ENDPOINT WAS A USER-EXISTENCE ORACLE. Any authenticated user could POST a uid and read the status code as an answer — 201 for a real account, 404 for an invented one. And it would have been a NEW oracle: Nextcloud governs enumeration through its own sharing settings, so an endpoint that answers around them has removed a control rather than added a feature. I introduced that while adding an existence check for a good reason (a pending request naming nobody can never be answered, so it would sit until it expired while its requester waited for a prompt no account could receive). Both facts now return ONE response. "Not someone you may ask" is all a requester needs and all they are entitled to; the difference between "no such person" and "not in your organisation" is exactly what an oracle is built out of. A test asserts the two are indistinguishable down to the uid the caller already supplied. ORGANISATION is the boundary because it is the fleet's tenancy unit — the same one scoping every register, schema and run. A delegation that crossed it would let one tenant's user request rights inside another's. An administrator is exempt: answering across tenants is what they are for, and that exemption is its own test, so a controller that refused every cross-organisation request could not pass this file. An unreadable organisation list REFUSES. Treating it as "no restriction" would re-open the enumeration surface wholesale, and it is the fail-open shape this subsystem has already been bitten by twice. One test note worth keeping: `Organisation::getUsers()` is a MAGIC method served by Entity::__call, so createMock() cannot stub it — it answers "Method name is not configured". The fixture uses a real entity with setUsers(). 16 tests / 29 assertions green; gate-7 clean on the file; phpcs, phpmd, psalm and phpstan clean; 18/18 Playwright still green live. * feat(delegation): an accessible consent prompt, and the parking path in e2e Two of the three things this change had recorded as open, closed. Both were recorded honestly and both turned out to be smaller than the note implied. 🔴 THE PROMPT NOW RENDERS AS A PERSON, NOT A TOKEN. `setRichSubject` / `setRichMessage` carry the requester as a `user` parameter, so a client renders a real user reference — display name, avatar, semantics a screen reader can announce as a person — and `setParsedSubject` / `setParsedMessage` keep the plain-text sentence every other surface reads. Both, and neither optional: NC's own isValidParsed() refuses a notification with only rich text, and a client without rich rendering shows nothing without the parsed half. A test asserts the two say the SAME sentence, because a rich subject reading differently from its fallback would mean two people looking at one security decision on different clients were answering different questions. A uid is an identifier, not a name. Asking somebody to grant rights to `j.devries3` when the person they know is "Jan de Vries" is harder for every reader and materially harder for one who cannot glance at a face beside it, so the display name is resolved and asserted — with the uid kept as the rich parameter's `id`, which is how a client resolves the avatar. The action labels stay "Allow" and "Deny" rather than becoming OK/Cancel. Out of context — which is how a screen reader reaches a button list — "OK" is meaningless and "Allow" is not. The message states the consequence BEFORE the controls, because a reader meets them in reading order.⚠️ I WAS WRONG THAT THE HARNESS COULD NOT DRIVE THIS. I recorded `awaiting_consent` as untestable in e2e because "the api-direct harness cannot drive a schedule fire or a cron sweep". `flow-schedule.spec.ts` has been doing exactly that since it was written — `occ background-job:execute` through the dev container, skipping when unreachable. delegation-parking.spec.ts follows that pattern: park on an unanswered request, assert the run says WHO it waits on, then answer and assert the sweep releases it into work that actually runs. Also fixes PRE-EXISTING DEBT the same file revealed: flow-schedule.spec.ts matched `Cron\FlowScheduleWorker`, and #2870 moved that job to `BackgroundJob\`. A matcher pinned to a namespace stops finding its job after a move — which does not FAIL the spec, it SKIPS it, so the whole scheduled-trigger path would have gone unverified with nothing red to say so. Both specs now match the class basename, which survives a namespace move.⚠️ NOT RUN LIVE. Docker is down on this box, and resolveContainer() refuses the shared `nextcloud` container by design, so this spec skips here regardless. The path it covers was verified by hand earlier today against a live instance — grant, save, revoke, re-request, schedule tick, park, answer, sweep, executed — and the spec encodes that sequence. Said plainly rather than reported as green. Dutch translations for all three new strings. 20 tests / 42 assertions in the Notifier suite; phpcs, phpmd, phpstan and the l10n parity check clean. * docs(delegation): close task 2.3 as not applicable, and say why Ticked as DECIDED rather than left open. An open box invites somebody to 'finish' it by making the grant an object — and that is exactly the circularity task 2.2 exists to prevent: x-openregister-lifecycle and x-openregister-notifications are properties of a register schema evaluated by the object layer, so a grant declaring either would be read through object RBAC, and resolving a delegation would require the delegation. The two tasks cannot both be satisfied. 2.2 is the one holding a security property, so it wins, permanently — this is not a deferral and the box should not read like one. The notification half IS delivered, by 4.1, and the same argument explains why it is imperative rather than declarative: the dialect fires on OBJECT lifecycle events and a grant has none. or-delegation-grants now has no unticked task.
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
1 participant
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.
runAsSystem()/SystemOperationContext::run()runs a callable as a trusted userless principal — no RBAC, no tenancy, no owner. ADR-099 rule 9 says it must not be reachable from a flow node, an agent tool, or the handling of an inbound request.What this actually defends against
Not somebody arguing for an escalation. Somebody reaching for the nearest thing that makes a refusal go away.
That reach is predictable, because ADR-099 deliberately put refusals everywhere an identity can be missing — a schedule trigger naming nobody, a delegation without a grant, an agent tool acting for an absent user — and every one of those sits within a few lines of a method that would make it succeed. A developer staring at
this flow run has no ownerfinds the elevating method on a service they already have injected. The fix works, the test goes green, and every future run of that flow executes with access control off, on user-authored input.Nothing downstream can catch it: by the time the callable runs, the caller is gone, so no runtime assertion inside the method can say who invoked it. The control has to be structural — and structural controls drift unless something checks them.
Why those three callers
They are not arbitrary. Each carries user-authored definitions or user-supplied input across the boundary:
Elevation reachable from any of them turns "a user can describe work" into "a user can describe work that runs as root". A migration has no user in the picture at all — which is the whole distinction, and why
lib/Migration/,lib/Repair/,lib/Command/,lib/BackgroundJob/andlib/Cron/are permitted by kind rather than by an allowlist of files: a new repair step needs no gate change, while a new controller still fails.🔴 No exclusion annotation, deliberately
Most gates here take a reason-bearing
@gate exclude <why>. This one does not, and that is the point rather than an omission.An escape hatch on this rule would be used exactly when somebody is making a refusal go away — the case the gate exists for — and a reason written in that moment ("needed for the migration path") reads identically to a legitimate one to every reviewer afterwards. Green bought with a plausible sentence is worse than red, because it ends the conversation.
A genuine false positive is fixed by moving the elevation out of the forbidden caller, or by fixing this gate. Both leave a visible diff. A comment does not.
Generalises a PHPUnit test
openregister's
SystemOperationContextBoundaryTestpins the call-site set in one app. This binds the fleet, so an app that never wrote such a test is covered too. It replaces the per-app test as the fleet control (the test can stay as a faster local signal).Stated limit
A dynamically dispatched call —
$svc->{$m}(), a callable in a variable, a container lookup by string — is invisible to it, exactly as it is to the PHPUnit test it generalises. It guards against drift; it does not prove absence. That is written into the helper's docblock so nobody reads a green as the stronger claim. The control that actually holds the line is that the elevating service is not injected into node, tool or endpoint classes.Comments and strings are masked
Without the mask, this gate's own docblock fails it — the same defect gate-2 was rewritten for after a comment saying
never use var_dump( hereand the string"select dd(x)"were both reported as findings.The
clean/fixture arm mentions the forbidden call twice — once in a docblock sentence, once inside the refusal message the controller throws — so the mask is load-bearing rather than decorative. A raw-text grep fails that arm.The fixture also keeps the permitted migration in both arms, elevating in both. Without it, the planted arm would pass for a gate that banned elevation outright, which is not the rule; such a gate would look identical on
planted/and be wrong about the only distinction that matters.Verification
bash -non the runnerlib/**/*.php)ALLOWEDlist its boundary test pinsFull-tree, not diff-scoped: a diff-scoped version reports nothing on the ~99% of PRs that never open a node or a controller, so it could not establish that the boundary holds — which is the only claim worth making about a boundary.
Gate 96, not 95 — #TBD (
adr-number-collision) is in flight on that number.🤖 Generated with Claude Code