Skip to content

Close the page-existence oracle on Subject write endpoints#1127

Open
alistair3149 wants to merge 3 commits into
masterfrom
security/1061-write-endpoint-existence-oracle
Open

Close the page-existence oracle on Subject write endpoints#1127
alistair3149 wants to merge 3 commits into
masterfrom
security/1061-write-endpoint-existence-oracle

Conversation

@alistair3149

Copy link
Copy Markdown
Member

Fixes #1061

The four page-id-keyed Subject write endpoints — CreateSubject (main and child), SetMainSubject, SetSubjectsOrdering, all keyed by a dense integer MediaWiki page id — were a page-existence oracle. A caller holding the wiki-global edit right (an ordinary editor) got a 403 for a page they could not edit but a 201/200 no-op for a nonexistent page id, so sweeping page ids distinguished "restricted page exists here" from "no page here". This is the write-side mirror of the read-side oracle #1058 (#1046) closed, and its POST final commit noted these writes were "not yet enumeration-hardened".

The fix

Each of the four endpoints now gates on PageReadAuthorizer::authorizeReadByPageId() — the same binding per-page read check #1058 built — before the write authorization and before any no-op short-circuit. That authorizer already returns false for both a nonexistent page and a page the caller may not read.

page state (caller holds global edit) before after
exists, caller can edit 201 / 200 201 / 200 (unchanged)
exists, caller can read but not edit 403 403 (unchanged — existence already public via GET)
exists, caller may not read (ACL / $wgWhitelistRead) 403 404
nonexistent page id 201 / 200 no-op 404

A page you cannot read and a page that does not exist now return a byte-identical 404 ({"status":"error","message":"Page not found"}), so page ids can no longer be swept to reveal restricted pages. A readable-but-not-editable page still returns 403, because its existence is already public through the read endpoints. The gate runs before the SetMainSubject/SetSubjectsOrdering no-op paths, which previously answered 200 unchanged for a nonexistent page.

Backstop: surface swallowed save failures

SubjectRepository::savePageSubjects returned void and MediaWikiSubjectRepository dropped the PageContentSaver status on the floor (the // TODO: expose failure information), so a save that failed — most notably a page that no longer resolves — was reported to the caller as a successful create/change. savePageSubjects now returns the PageContentSavingStatus, and the three actions turn an ERROR into the same 404. With the read gate in front this is a belt-and-suspenders for the page being deleted between the gate and the save; on its own it stops the write path from claiming success for a write that never landed. updateSubject/deleteSubject keep their void contract.

Scope

In: the four page-id-keyed writes. Out, deliberately: PUT/DELETE /subject/{id} (Replace/Delete) are subject-id-keyed, and Special:NeoJson is title-keyed — page existence by title is already public, so neither is the dense-integer-id oracle this fixes.

Considered, omitted

The backstop maps any post-gate PageContentSavingStatus::ERROR to the same 404, discarding errorMessage — so a rare non-page-gone save failure (an EditFilterMergedContent abort, an edit conflict) on an existing editable page would also read as "Page not found". This is a strict improvement over the prior behaviour (a fabricated 201/200 that claimed a write which never landed), maps uniformly regardless of page existence (no oracle), and is narrowly reachable on these slot-write paths. Distinguishing failure kinds needs a typed reason out of PageContentSaver, which is beyond this issue.

Verification

make cs (phpcs + phpstan) clean. The gate is pinned by action-unit tests proven load-bearing by mutation (neutralising the gate creates the Subject and returns the wrong 403), and by API integration tests asserting byte-identity between the unreadable-page and nonexistent-page 404s on all three endpoints, plus a readable-but-not-editable 403 (mock grants read, denies page edit). The backstop is pinned by an action test that fails the save (InMemorySubjectRepository::failNextSave) and asserts not-found, not created/changed.

Verified live on the dev wiki (per-page read denied on page 30 via a getUserPermissionsErrors hook):

request result
POST /page/30/mainSubject (read-restricted) 404 {"status":"error","message":"Page not found"}
POST /page/999999/mainSubject (nonexistent) byte-identical 404
POST /page/30/childSubjects, PUT /page/30/mainSubject, PUT /page/30/subjectsOrdering 404
POST /page/39/childSubjects as an anon (readable, not editable) 403
POST /page/39/childSubjects as an editor 201 created

CreateSubjectApiTest runs its full ~24-case suite past the dev-stack's Neo4j-connection budget and times out locally; the four gate/backstop cases and the happy path were verified with targeted filters (CI runs the full suite).

Manual check

A sitewide read lockdown cannot exercise these gates — MediaWiki's REST framework rejects all routes on the wiki-global read right first. Use a targeted per-page denial. Append to Docker/LocalSettings.local.php (edit in place with cat >>, replacing it breaks the bind mount):

$wgHooks['getUserPermissionsErrors'][] = static function ( $title, $user, $action, &$result ) {
    if ( $action === 'read' && $title->getDBkey() === 'ACME_Amsterdam_HQ' ) {
        $result = 'badaccess-group0';
        return false;
    }
    return true;
};

As a logged-in ordinary editor (demo data: page 30 = ACME Amsterdam HQ), with a CSRF token:

  • POST <wiki>/w/rest.php/neowiki/v0/page/30/mainSubject (a valid Subject body) → 404 {"status":"error","message":"Page not found"}
  • POST .../page/999999/mainSubject (nonexistent id) → identical 404
  • POST .../page/30/childSubjects, PUT .../page/30/mainSubject, PUT .../page/30/subjectsOrdering → same 404
  • Any page you can read but not edit → 403; any page you can edit → 201/200.

Remove the hook → full behaviour returns.

Directed by @alistair3149; the denial shape (read-gate → 404 mirroring #1058, vs a uniform 403 or the issue's literal 404-for-nonexistent-only) was chosen explicitly in review.
Written by Claude Code, Opus 4.8 (1M context)

🤖 Generated with Claude Code

alistair3149 and others added 3 commits July 21, 2026 17:31
The Create, SetMainSubject and SetSubjectsOrdering endpoints are keyed by
dense integer page ids and answered 403 for a page the caller could not
edit but 201/200 (a no-op) for a nonexistent page id, so a caller holding
the wiki-global edit right could sweep ids to tell restricted pages apart
from absent ones - the write-side mirror of the read oracle #1046 closed.

Each endpoint now gates on PageReadAuthorizer::authorizeReadByPageId before
the write check and before any no-op short-circuit: a page the caller may
not read and a page that does not exist both answer the same 404
(presentPageNotFound), byte-identical, so a hidden page cannot be told
apart from an absent one. A readable-but-not-editable page still answers
403, because its existence is already public through the read endpoints.

Part of #1061.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SubjectRepository::savePageSubjects returned void and MediaWikiSubject-
Repository dropped the PageContentSaver status on the floor (the
"// TODO: expose failure information"), so a save that failed - most
notably a page that no longer resolves - was reported to the caller as a
successful create or change.

savePageSubjects now returns the PageContentSavingStatus, and the Create,
SetMainSubject and SetSubjectsOrdering actions turn an ERROR into the same
not-found response the read gate produces. With that gate in front this is
a backstop for the page being deleted between the gate and the save; on its
own it stops the write path from claiming success for a write that never
landed.

Part of #1061.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The REST API permissions section still said Subject writes answer 403 on
denial. The page-keyed create, main-subject and ordering endpoints now
answer 404 for a page you may not read or that does not exist, so page ids
cannot be swept, and 403 only for a page you may read but not edit.

Part of #1061.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alistair3149
alistair3149 marked this pull request as ready for review July 21, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subject write endpoints oracle page existence (403 vs 201 no-op)

1 participant