Skip to content

Enforce per-page read permission on the Subject, Layout, and RDF read endpoints#1058

Merged
JeroenDeDauw merged 17 commits into
masterfrom
per-page-read-gates
Jul 17, 2026
Merged

Enforce per-page read permission on the Subject, Layout, and RDF read endpoints#1058
JeroenDeDauw merged 17 commits into
masterfrom
per-page-read-gates

Conversation

@alistair3149

@alistair3149 alistair3149 commented Jul 16, 2026

Copy link
Copy Markdown
Member

Fixes #1046

The Subject, Layout, RDF, and schema-name read endpoints answered with data from pages the caller may not read. MediaWiki's REST framework checks only the wiki-global read right, so a fully private wiki was protected wholesale while partially restricted setups (page protection with read-restriction extensions, restricted namespaces, read whitelists, permission-filtered farms) leaked both Subject data and page existence. The POST /subject/{id}/validate endpoint additionally oracled Subject existence outright (404 for absent, 200 for present).

Every read path now enforces the caller's per-page read permission via Authority::authorizeRead( 'read', $title ) — the binding check that runs the expensive permission hook ACL extensions rely on. The pre-existing Schema and Mapping gates used probablyCan, which core documents as unfit for access control (it skips that hook), and carried a comment claiming the inner lookup applies a per-user read check; RevisionRecord::FOR_THIS_USER filters revision-deletion suppression only, so no such check existed. Both gates are upgraded and the comment corrected.

Denials are deliberately indistinguishable from absent data. Page and revision IDs are sequential integers, and readable Subjects hand out the IDs of Subjects they reference, so any distinguishable denial (a 403, a different message) is a sweepable oracle over which pages are restricted. Each endpoint routes denial through its existing miss shape, byte-identical:

Endpoint Denied response
GET /subject/{id} (incl. expand=) 200 {"subject":null}
GET /subject/{id}?revisionId=N (page of N unreadable) 404 Revision not found: N
GET /page/{id}/subjects (incl. expand=) 200 + empty subject map
GET /page/{id}/rdf 404 No NeoWiki data found for page: <id>
POST /subject/{id}/validate 404, same message as an absent Subject
GET /layout/{name} 200 {"layout":null}
GET /schema/{name} 200 {"schema":null}
GET /layouts, GET /schemas, GET /schema-names/{search} denied rows omitted

Referenced Subjects on unreadable pages are omitted from expand=relations responses; the readable page's own relation statements still show the opaque target IDs, which are that page's content. Every denial is logged at info level to the NeoWiki channel with page and performer, so the suppressed error stays diagnosable server-side. This diverges from core REST's 403 convention on purpose: core's endpoints are title-keyed and page existence by title is already public there; ours are keyed by dense integer IDs and by Subject IDs that leak through relations.

Architecturally this completes the #1003 split with a third quadrant: SubjectReadAuthorizer (binding, side-effect-free apart from rate-limit accounting — no read bucket exists by default) joins SubjectPermissionHints and SubjectWriteAuthorizer on AuthorityBasedSubjectAuthorizer, and every PageId-keyed gate goes through it. Unlike the write side there is no global-right fallback: content is only reachable through a resolved page, so an unresolvable page denies. Name-keyed Persistence lookups (Layout, schema names, the Schema/Mapping caches) hold an Authority directly, as CachingSchemaLookup already did. Where a Subject's owning page does not resolve in the graph, GetSubjectQuery treats it as readable — that case is reachable only from its ?revisionId= branch, whose page the handler has already authorized — while GetPageSubjectsQuery, where the case is unreachable, fails closed so a future non-graph lookup cannot silently bypass the gate. The validate endpoint now resolves the owning page once and denies before reading any slot content, which also removed a duplicate graph query, as did the single resolve serving both the gate and expand=page in GetSubjectQuery.

The issue's premise that the read paths "apply RevDel audience checks (FOR_THIS_USER)" was wrong in a way that matters: the Subject paths pass no audience at all (default FOR_PUBLIC, which is stricter), and FOR_THIS_USER was never a read gate anywhere. The issue text should be corrected accordingly.

Verification

Every gate is pinned by tests that were proven load-bearing by mutation during development: flipping the denying authority to permissive (or reverting a gate to probablyCan, or changing the 'read' action string) fails a test for each gate. Denial tests assert byte-identity with the absent shape, not just status codes. make cs (phpcs + phpstan) clean; all touched suites green, including broad filter=Subject (545 tests) and filter=Schema (286 tests).

Verified end-to-end on the dev wiki (anonymous caller, one page denied via a getUserPermissionsErrors hook, control page unrestricted): the full denied matrix above reproduced exactly, the control page stayed fully visible, schema-name enumeration omitted the denied Schema, and lifting the restriction restored full responses.

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:

  1. Append to Docker/LocalSettings.local.php (edit the file in place, e.g. 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;
    };
    No container restart needed.
  2. As an anonymous caller (demo data: page 30 = ACME Amsterdam HQ, main Subject s1demo7aaaaaaa4):
    • GET <wiki>/w/rest.php/neowiki/v0/page/30/subjects200 {"pageId":30,"mainSubjectId":null,"subjects":[]}
    • GET .../page/30/rdf404 No NeoWiki data found for page: 30
    • GET .../subject/s1demo7aaaaaaa4200 {"subject":null}
    • POST .../subject/s1demo7aaaaaaa4/validate (body {"label":"x","statements":{}}) → 404 Subject not found: s1demo7aaaaaaa4
    • Any other page with Subjects → unaffected.
  3. Remove the hook → full data returns immediately.

Deferred

Documented in the new docs/api/rest-api.md Permissions section; each has its own follow-up issue.

Directed by @alistair3149; gate method, scope, and denial shape were each decided explicitly in review with them, including the deliberate divergence from core REST's 403 convention.
Context: the NeoWiki codebase, MediaWiki Authority/PermissionManager/REST internals, the #101 audit history, and multi-agent implementation with per-task spec + quality reviews, mutation-tested gates, an independent final review, and a live smoke test.
Written by Claude Code, Fable 5

🤖 Generated with Claude Code

alistair3149 and others added 12 commits July 16, 2026 12:08
Third interface on AuthorityBasedSubjectAuthorizer, delegating to
Authority::authorizeRead( 'read', $title ): the full per-title check
including the expensive ACL hook that probablyCan skips. Reads have no
global-right fallback - an unresolvable page denies. Denials log to the
NeoWiki channel. Groundwork for #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GET /page/{id}/rdf read the Subject slot and page properties with no
per-title check, exporting any page's data to anyone holding the
wiki-global read right. Denials reuse the exact no-data 404 so
unreadable pages are indistinguishable from pages without NeoWiki data.
CLI dumps (maintenance/DumpRdf.php) stay unfiltered. Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GET /page/{id}/subjects returned every Subject on any page to anyone
holding the wiki-global read right. A denied page now takes exactly the
no-Subjects path (200 + empty map), and referenced Subjects on
unreadable pages are omitted, so page ids cannot be swept to map
restricted pages. Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GET /subject/{id} returned Subject data from unreadable pages. A denied
Subject now takes exactly the absent-Subject path (200 {"subject":null}),
a denied revision page answers like a nonexistent revision, and denied
referenced Subjects are omitted. One page resolve now serves both the
gate and expand=page, removing the former duplicate graph query.
Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
POST /subject/{id}/validate oracled Subject existence (404 vs 200) and
validated against the Subject's Schema regardless of page readability.
A denied Subject now throws the same SubjectNotFoundException as an
absent one, so the two are byte-identical. The query is built at
request time so it can carry the caller's Authority. Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WikiPageLayoutLookup relied on the revision audience check, which
filters revision deletion only and never checks read on the title. It
now gates with authorizeRead before fetching; a denied Layout is null,
identical to an absent one, on both GET /layout/{name} and the
GET /layouts item list. Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The empty-search branch of DatabaseSchemaNameLookup listed NS_SCHEMA
straight from the page table with no permission check, and the search
branch depended on engine-specific visibility rules. Both branches now
share a binding per-title read filter; filtered names are absent, like
Schemas that do not exist. getSchemaCount() is a known, deferred count
oracle. Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
probablyCan is documented by core as unfit for access control: it skips
the expensive permission hook that ACL extensions use, so on a
permission-filtered wiki the gate silently passed users it should deny.
Also corrects the false comment claiming the inner lookup applies a
per-user read check - the revision audience check filters revision
deletion only. Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Name the Cypher endpoint as a second per-page-filter exception, stop
claiming write denials reveal nothing (page-id-keyed writes are not yet
enumeration-hardened), and make three gate comments state their actual
situation and accepted trade-offs. Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add the missing verb/action pin test for DatabaseSchemaNameLookup (the
one gate a probablyCan regression would not have failed), resolve the
validate endpoint's owning page once instead of twice (no slot content
is loaded for denied Subjects anymore), and drop the redundant
'NeoWiki:' prefix from denial-log messages - the channel already names
the extension. Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GetPageSubjectsQuery only ever receives Subjects from the graph-backed
repository, which cannot return a Subject without resolving its page,
so the null case is unreachable today. It previously followed the
sibling queries' null-means-readable rule for uniformity; that default
would silently serve ungated data if a non-graph SubjectLookup were
ever wired in. Unresolvable now means omitted - the endpoint's normal
absence shape. GetSubjectQuery deliberately keeps null readable: its
revision branch reaches null for handler-authorized pages. Part of #1046.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CachingMappingLookupTest created a real Mapping page via createMapping()
without declaring the page table used or clearing the graph, so its
committed rows and advanced revision ids leaked into later @group
Database tests in the same process and made their graph projection
resolve stale revisions - deterministically breaking
MediaWikiSubjectRepositoryTest::testGetSubjectReturnsSubject downstream
(green in isolation, red in the full suite; CI-only).

Rewritten as a pure unit test mirroring its schema twin
CachingSchemaLookupTest::testGateUsesBindingAuthorizeRead: mocked
Title/TitleFactory/Authority, hash-backed cache, no DB. It still pins
the binding authorizeRead verb, the 'read' action string, and the
denial log. Part of #1046.
Drop the enumeration rationale, the wiki-global-shortcut framing, the
per-endpoint denial-shape table, and the temporary not-yet caveats. Keep
the one fact a client author needs: a denied read looks like absent data,
so treat not-found as maybe-not-permitted. Part of #1046.
…#1071)

* Test the read gate on the Schema name search branch

Both existing denial tests call getSchemaNamesMatching with an empty
search, so they only cover getFirstSchemaNames. Deleting filterReadable
from the search branch -- the branch GET /schema-names/{search} actually
uses -- passed the whole suite.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Correct the REST permissions documentation

The section claimed page protection and restricted namespaces restrict
reads. Neither does: for the read action MediaWiki runs only
checkPermissionHooks, checkReadPermissions and checkUserBlock, read is not
one of the default $wgRestrictionTypes, and $wgNamespaceProtection is only
consulted by checkSpecialsAndNSPermissions, which reads skip. Per-page read
restrictions come from $wgWhitelistRead or a permission extension.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JeroenDeDauw and others added 2 commits July 17, 2026 13:11
The read gate was one decision -- "may this Authority read this page?" --
written five times: AuthorityBasedSubjectAuthorizer, CachingSchemaLookup,
CachingMappingLookup, WikiPageLayoutLookup and DatabaseSchemaNameLookup each
called authority->authorizeRead( 'read', $title ), and four followed it with
the same five-line denial log. Three classes grew a LoggerInterface only to
host that block, four test classes each had their own copy of a
testGateUsesBindingAuthorizeRead, and three of those carried the comment
"Mirrors AuthorityBasedSubjectAuthorizerTest::testDeniedReadIsLogged".

That copying had already drifted once: both caching lookups used probablyCan,
an advisory hint verb, for a binding gate, and both had to be fixed by hand.

SubjectReadAuthorizer could not absorb the other four because of its name: it
holds no Subject semantics -- it is newTitle() plus authorizeRead -- and two of
its callers gate an RDF export and a revision's page, not a Subject. Schemas,
Layouts and Mappings live on pages and need the same check, but a thing called
SubjectReadAuthorizer cannot plausibly gate a Schema page.

Rename it to PageReadAuthorizer, move it off AuthorityBasedSubjectAuthorizer
into AuthorityBasedPageReadAuthorizer, and give it the two entry points its
callers need: by PageId for the Application queries, which resolve a Subject to
a page through the graph, and by Title for the Persistence lookups, which
already resolved one for their own content fetch. SubjectContentRepository sets
the precedent for keying one page two ways. The verb and the denial log are now
pinned once, in the authorizer's own test.

Denials from the Schema name filter are now logged like every other denial,
where before that one call site deliberately stayed silent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GetSubjectQuery and GetPageSubjectsQuery each had a private pageIsReadable
with the same signature and the opposite meaning: one treats an unresolved
page as readable, the other as denied. Both needed a docblock to say so, and
one of those docblocks existed mainly to point at the other.

GetPageSubjectsQuery's helper had a single call site and, by its own docblock,
guarded a case that cannot happen today. Inline it: the fail-closed check is
three tokens, and a named helper plus eight lines of prose is not warranted by
"if a SubjectLookup that bypasses the graph is ever wired in".

That leaves one such check, in GetSubjectQuery, where null does not mean
"readable" but "no page to gate". Name it pageIsReadableOrUnresolved, which is
what it decides, and the docblock only has to explain why unresolved is
allowed there.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@JeroenDeDauw
JeroenDeDauw merged commit 1aa1620 into master Jul 17, 2026
6 checks passed
@JeroenDeDauw
JeroenDeDauw deleted the per-page-read-gates branch July 17, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security alert('xss');

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enforce per-page read permission on Subject, Layout, and RDF read endpoints

3 participants