Resolving a Subject id to its hosting page and gating that page on read is written out at seven
call sites. Application\Rdf\SubjectHostingPageResolver::resolveReadableHostingPage() already is
that sequence — returning null both when no page holds the Subject and when its page is
unreadable — but it has one caller.
The security case for sharing it is not that the code is long. It is two statements. It is that the
gate has been retrofitted onto the Subject write actions across five PRs in seven weeks
(4aba3eb1, aa1a13cd, 64b8822b, 0b9622b6, #1380), each discovering an action the previous one
missed, and that the rationale for it now exists as a 7-line comment copied into two actions and
paraphrased in four more — copies that have already begun to diverge.
Call sites
Absorbable as-is:
| Site |
How it expresses "not there" |
Application\Rdf\RdfSubjectExporter:38 |
returns null (and lives in the resolver's own namespace) |
Application\Queries\ValidateSubjectUpdate\ValidateSubjectUpdateQuery:42 |
throws SubjectNotFoundException |
Application\Actions\ReplaceSubject\ReplaceSubjectAction:50 |
throws SubjectNotFoundException |
Application\Actions\UpdateStatement\UpdateStatementAction:102 |
throws SubjectNotFoundException |
Application\Actions\DeleteSubject\DeleteSubjectAction:27 |
throws SubjectNotFoundException |
Application\Actions\MoveSubject\MoveSubjectAction:47 |
presenter; source page only — its target-page gate is page-id-keyed and unaffected |
Only the read half is shareable. Each caller keeps its own one-line "if null" branch, and the
write half stays inline: Replace and UpdateStatement throw SubjectEditNotAuthorizedException,
Delete throws it with its own message, Move uses a presenter across two pages.
Two constraints that make this more than mechanical
GetSubjectQuery has inverted null semantics, and they contradict ADR 032.
GetSubjectQuery:89's helper is pageIsReadableOrUnresolved(): $pageIdentifiers === null returns
true, so a Subject no page resolves for is served.
ADR 032
says the opposite — "every right a Subject write needs is a right on the page holding it, so with no
page there is nothing to allow", and "null means 'no local page holds this', not 'does not exist'".
Not currently exploitable, on either wiring:
newGetSubjectQuery binds MediaWikiSubjectRepository, whose getSubjects() groups by hosting
page through the same PageIdentifiersLookup, so an unresolvable id loads no content and the
$subject === null branch fires first. The permissive branch is unreachable.
newGetSubjectQueryForRevision binds PointInTimeSubjectLookup, which reads the slot off the
revision with no index — so it can reach the branch, but GetSubjectApi:55 already gated on
revisionPageIsReadable() before constructing the query.
Latent rather than live: bind a SubjectLookup that does not share the index and GET /subject/{id}
begins serving Subjects with no permission check, with nothing in the suite to notice. Inverting it
is one line plus a test, is behaviour-preserving today precisely because the branch is unreachable,
and should land before or with the extraction.
CreateSubjectAction::subjectIdIsInUse():158 must keep the unfiltered lookup. It asks whether a
caller-supplied id collides with any existing Subject, including on pages the caller cannot read.
Point it at a read-filtered resolver and a client can mint an id colliding with a hidden Subject. So
the resolver must be injected alongside PageIdentifiersLookup, never as a replacement binding —
and that is worth a comment at the call site, because "make it consistent" is the obvious wrong move.
Scope
Move SubjectHostingPageResolver out of Application\Rdf\ (it is not RDF-specific; only its
docblock is, written entirely around the concept-URI negotiator), inject it at the six sites above,
collapse each action's pageIdentifiersLookup + readAuthorizer pair into it, and delete the
duplicated rationale in favour of one home on the resolver. Roughly seven production classes,
NeoWikiExtension, and the corresponding test helpers.
Worth knowing before committing to this
It does not make the gate hard to omit. A new action still chooses between the resolver and
PageIdentifiersLookup, and both must stay available because of the CreateSubjectAction
constraint above. The honest benefit is one home for the invariant and one test for it — not
prevention of the next omission.
The cheap partial is available separately and captures most of the maintainability win: delete the
duplicated rationale comments and let PageReadAuthorizer's own docblock own the policy. No
constructor churn, no test changes, no behaviour decisions.
AI-authored — Claude Code, Opus 5 (1M context); written at @alistair3149's request after they questioned the "considered, omitted" note on #1380 and confirmed the page-less-Subject behaviour was unintended; not yet human-reviewed; call sites, line numbers and reachability traced by reading the code and ADR 032, not reproduced against a running wiki.
Resolving a Subject id to its hosting page and gating that page on
readis written out at sevencall sites.
Application\Rdf\SubjectHostingPageResolver::resolveReadableHostingPage()already isthat sequence — returning
nullboth when no page holds the Subject and when its page isunreadable — but it has one caller.
The security case for sharing it is not that the code is long. It is two statements. It is that the
gate has been retrofitted onto the Subject write actions across five PRs in seven weeks
(
4aba3eb1,aa1a13cd,64b8822b,0b9622b6, #1380), each discovering an action the previous onemissed, and that the rationale for it now exists as a 7-line comment copied into two actions and
paraphrased in four more — copies that have already begun to diverge.
Call sites
Absorbable as-is:
Application\Rdf\RdfSubjectExporter:38null(and lives in the resolver's own namespace)Application\Queries\ValidateSubjectUpdate\ValidateSubjectUpdateQuery:42SubjectNotFoundExceptionApplication\Actions\ReplaceSubject\ReplaceSubjectAction:50SubjectNotFoundExceptionApplication\Actions\UpdateStatement\UpdateStatementAction:102SubjectNotFoundExceptionApplication\Actions\DeleteSubject\DeleteSubjectAction:27SubjectNotFoundExceptionApplication\Actions\MoveSubject\MoveSubjectAction:47Only the read half is shareable. Each caller keeps its own one-line "if null" branch, and the
write half stays inline:
ReplaceandUpdateStatementthrowSubjectEditNotAuthorizedException,Deletethrows it with its own message,Moveuses a presenter across two pages.Two constraints that make this more than mechanical
GetSubjectQueryhas invertednullsemantics, and they contradict ADR 032.GetSubjectQuery:89's helper ispageIsReadableOrUnresolved():$pageIdentifiers === nullreturnstrue, so a Subject no page resolves for is served.ADR 032
says the opposite — "every right a Subject write needs is a right on the page holding it, so with no
page there is nothing to allow", and "
nullmeans 'no local page holds this', not 'does not exist'".Not currently exploitable, on either wiring:
newGetSubjectQuerybindsMediaWikiSubjectRepository, whosegetSubjects()groups by hostingpage through the same
PageIdentifiersLookup, so an unresolvable id loads no content and the$subject === nullbranch fires first. The permissive branch is unreachable.newGetSubjectQueryForRevisionbindsPointInTimeSubjectLookup, which reads the slot off therevision with no index — so it can reach the branch, but
GetSubjectApi:55already gated onrevisionPageIsReadable()before constructing the query.Latent rather than live: bind a
SubjectLookupthat does not share the index andGET /subject/{id}begins serving Subjects with no permission check, with nothing in the suite to notice. Inverting it
is one line plus a test, is behaviour-preserving today precisely because the branch is unreachable,
and should land before or with the extraction.
CreateSubjectAction::subjectIdIsInUse():158must keep the unfiltered lookup. It asks whether acaller-supplied id collides with any existing Subject, including on pages the caller cannot read.
Point it at a read-filtered resolver and a client can mint an id colliding with a hidden Subject. So
the resolver must be injected alongside
PageIdentifiersLookup, never as a replacement binding —and that is worth a comment at the call site, because "make it consistent" is the obvious wrong move.
Scope
Move
SubjectHostingPageResolverout ofApplication\Rdf\(it is not RDF-specific; only itsdocblock is, written entirely around the concept-URI negotiator), inject it at the six sites above,
collapse each action's
pageIdentifiersLookup+readAuthorizerpair into it, and delete theduplicated rationale in favour of one home on the resolver. Roughly seven production classes,
NeoWikiExtension, and the corresponding test helpers.Worth knowing before committing to this
It does not make the gate hard to omit. A new action still chooses between the resolver and
PageIdentifiersLookup, and both must stay available because of theCreateSubjectActionconstraint above. The honest benefit is one home for the invariant and one test for it — not
prevention of the next omission.
The cheap partial is available separately and captures most of the maintainability win: delete the
duplicated rationale comments and let
PageReadAuthorizer's own docblock own the policy. Noconstructor churn, no test changes, no behaviour decisions.