Skip to content

Pin the Schema name search read gate and correct the permissions docs#1071

Draft
JeroenDeDauw wants to merge 2 commits into
per-page-read-gatesfrom
pr-1058-review-fixes
Draft

Pin the Schema name search read gate and correct the permissions docs#1071
JeroenDeDauw wants to merge 2 commits into
per-page-read-gatesfrom
pr-1058-review-fixes

Conversation

@JeroenDeDauw

Copy link
Copy Markdown
Member

Follows-up to #1058

Two fixes from an AI review of #1058. Both are small. The review's more significant findings are deliberately not in this branch — they need your call, and are reported separately.

Test the read gate on the Schema name search branch

Both denial tests in DatabaseSchemaNameLookupTest call getSchemaNamesMatching with an empty search, so they only cover getFirstSchemaNames. Deleting filterReadable from the search branch — the one GET /schema-names/{search} actually uses — passed the full suite. The production code is correct; the gate was simply unpinned. The new test fails without the gate and passes with it.

Correct the REST permissions documentation

Two inaccuracies:

  • It claimed page protection and restricted namespaces apply to 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.
  • It stated without qualification that read endpoints enforce per-page read. GET /subject-labels applies no per-page filter. Verified on a dev wiki with one page read-denied: GET /subject/{id} returns {"subject":null} while GET /subject-labels still returns that Subject's id and label. totalRows likewise still counts rows omitted from the list.

This partially restores caveats that 4cb79db trimmed as "temporary not-yet caveats". Flagging rather than assuming your intent: #1058's description says the deferred gaps are "Documented in the new docs/api/rest-api.md Permissions section", and the trim removed exactly that, which left the remaining blanket claim false. If you prefer different wording, the only constraint is that the section not assert a guarantee /subject-labels does not provide.

Verification

make cs clean; full suite green (1689 tests). New test proven load-bearing by mutation.

AI-authored — Claude Code, Opus 4.8 (max); produced by a /pr-review run of #1058 for @JeroenDeDauw, no revisions; diff not yet human-reviewed; new test mutation-tested, make cs clean, full PHPUnit suite green locally, docs claims checked against MediaWiki 1.43 core and reproduced live on a dev wiki.

JeroenDeDauw and others added 2 commits July 17, 2026 02:37
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 Fable 5 <noreply@anthropic.com>
The Permissions section claimed that page protection and restricted
namespaces apply to 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.

It also stated without qualification that read endpoints enforce per-page
read permission. GET /subject-labels applies no per-page filter, so on a
restricted wiki it still answers with the labels and ids of Subjects on
pages the caller may not read, and the totalRows counts still include rows
omitted from the list. Restore those caveats, which #1046 lists as
documented here and which each have a follow-up issue.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant