Repository navigation
Refuse on the API what demo mode refuses in the interface - #831
Merged
Merged
Conversation
Demo mode makes an instance refuse to change or copy itself. The web enforces it in five config actions and in UserForm; nothing on the API surface mentioned demo mode anywhere. That gap matters more than a missing guard usually would, because a demo deployment is the one place where the caller is *meant* to hold administrator credentials — they are published so people can try it. The ACL therefore stops nobody: sign in as the demo admin, mint a token, and the API ran the backup, the export, or the user change the interface had just refused. A visitor who changed the demo admin's password ended the demo for everyone after them. Four endpoints gain the guard, through one shared method on the API ControllerBase rather than four copies: config/backup, config/export, and the user edit and delete paths. The demo account's id moves from a private constant in UserForm to User::DEMO_ADMIN_ID, so the two doors compare against the same value. The refusal is narrowed to that one account on the user endpoints — every other user on a demo stays editable and removable, which is most of what there is to try — and DemoModeTest pairs each refusal with the same call succeeding on an ordinary user, so none of them can be satisfied by an endpoint that simply stopped working. Removing the guard fails exactly the four refusals and leaves the control passing.
This was referenced Aug 20, 2026
blaipr
added a commit
that referenced
this pull request
Aug 23, 2026
Every way of getting an installation out of a demo refuses — the backup and both of its downloads, the config import, the encryption save and refresh, and the REST export since #831 — except the web export, both halves of it. A demo publishes its administrator's credentials by design, so the ACL in front of these stops nobody and the demo check is the whole boundary. The export holds the same installation the backup is guarded for: every account's encrypted secret and its key, and, when no export password is given, the name, login, URL and notes of every account in the clear. #831 fixed the REST door by comparing it against the five web config actions that had the guard. The web export was not among those five, so comparing against them could not reveal it: the gap was in the list being compared to. Both halves are guarded, because guarding only the creation leaves an export made before the instance became a demo fetchable, which is a way around it. The download refusal asserts an empty body rather than a message, which is what the backup downloads beside it already answer on a demo: these actions are typed CALLBACK, so a refusal that is not a callable renders as nothing. That blank page is a pre-existing wart the three share, verified against the untouched downloadBackupApp rather than assumed, and left alone. What the test pins is that the export is not handed over. CLAUDE.md corrects #841, which said any signed-in user could delete another user's notification on the grounds that Acl returns true for the notification actions unconditionally. NOTIFICATION_VIEW, NOTIFICATION_SEARCH and NOTIFICATION_CHECK are in that list; NOTIFICATION_DELETE is not in the switch at all and falls through to the deny, so only isAdminApp ever reached those methods. The ownership check remains right as defence in depth, but it was not the reachable hole it was described as.
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.
The gap
Demo mode makes an instance refuse to change or copy itself. The web enforces it in five config
actions and in
UserForm:ConfigBackup/FileBackupController·DownloadBackupDbControllerConfigImport/ImportControllerConfigEncryption/SaveController·RefreshControllerUserForm— edit / delete / change password of the demo accountconfig/backupconfig/exportusers/{id}PUT / DELETEgrep -rn isDemoEnabled src/Infrastructure/Adapter/In/Api/returned nothing: the API surface didnot mention demo mode anywhere.
Why this one matters more than a missing guard usually would
A demo deployment is the one place where the caller is meant to hold administrator credentials —
they are published so people can try the thing. The ACL therefore stops nobody, and the demo guard
is the whole boundary. Sign in as the demo admin, mint a token, call the API, and it did the backup,
the export or the user change the interface had just refused one click earlier. A visitor who
changed the demo admin's password ended the demo for everyone who came after.
The change
One shared
denyOnDemo()on the APIControllerBaserather than four copies, plus adenyOnDemoUser(int $id)that narrows it to the published account. Wired intoconfig/backup,config/export, and the user edit and delete paths — the complete set: there is no APIpassword-change endpoint for users, so the web's third
isDemo()call site has no second door.The demo account's id moves from a private constant in
UserFormtoUser::DEMO_ADMIN_ID, so thetwo doors compare against the same value instead of each holding a copy.
The user refusals are deliberately narrow. Every other user on a demo stays editable and removable —
trying that is most of the point of running a demo.
Test
DemoModeTest(5 tests, realApiTestCasedispatch with a real token, demo mode written into theconfig the API actually reads). Each refusal asserts both the error and the state — the row is
unchanged, no dump or export file is written — and is paired with the same call succeeding on an
ordinary user, so none of them can be satisfied by an endpoint that simply stopped working.
Mutation-checked: removing the guard fails exactly the four refusals, at the assertion that says the
API refused, and leaves the control passing.
PHPStan level 6 and PHPCS clean.
Also
CLAUDE.mdgains "The same rule, asked at the other door" in the defects section. This is thefourth finding from that lens — after the custom-field masking, the password-policy lifetime on edit,
and the search-paging clamp — and it had no entry, while the pattern it describes keeps producing.