Repository navigation
Conversation
A wiki page that transcludes a protected special page, such as
{{Special:RecentChanges/10}}, was answered with the denial for every
viewer: in miser mode MediaWiki runs a transcluded special page as an
anonymous user whoever views the page, and with
$wgCrawlerProtectionRawDenial the denial replaces the whole page.
A transcluded special page runs with the arguments the wiki page gives
it, not with the visitor's request, so it is no longer checked. The
README names the requests that render client-supplied wikitext, such
as action=parse with text, and how to protect them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_e56c0dc3-48ae-44a3-accc-c8db51e0fcb5
jeffw16
left a comment
There was a problem hiding this comment.
The fix is correct and in the right place. When the parser renders a transcluded special page, it builds a new request from the wiki page's arguments, with no real client IP (it reads as 127.0.0.1). For pages whose output can be cached (by default, in miser mode), it also runs the page as an anonymous user. So the check always failed, even for logged-in visitors and allowlisted IPs. With $wgCrawlerProtectionRawDenial, denyAccessRaw() then calls die() in the middle of rendering and kills the whole page view. With the "pretty" denial page, the "access denied" text ends up in the page body and can be stored in the parser cache. including() has existed far longer than REL1_39, so there's no compatibility risk, and CI is green across the matrix.
A few points before merging:
1. This loosens the default protection. Two of the three special pages protected by default, whatlinkshere and recentchangeslinked, can be transcluded. Before this PR, an anonymous request such as api.php?action=parse&text={{Special:WhatLinksHere/Foo|limit=500}} was denied, although only as a side effect of the bug. Now it goes through unless the operator adds parse to $wgCrawlerProtectedApiModules, which is empty by default. The README discloses this, but a default install is now more open than in 1.7.0. I'd call this out in the release notes and consider bumping to 1.8.0 rather than 1.7.1. Protecting parse by default isn't a good answer either, because it would break anonymous previews.
2. The README list of bypass paths is incomplete. Besides action=parse and Special:ExpandTemplates, anonymous edit previews, the REST /transform/wikitext/to/html endpoint, and extensions such as TemplateSandbox can also render client-supplied wikitext. I've left an inline suggestion that rewords the paragraph for clarity and covers these.
3. The CrawlerProtectionShouldDeny hook no longer fires for transclusions. Code that relies on that hook can't see or overrule this bypass. That's probably acceptable, but it should be documented on the hook. Alternatively, the bypass could go through the hook with $shouldDeny = false.
4. Tests (optional). The unit test covers the hook logic. An integration test that renders a page containing {{Special:WhatLinksHere/...}} and checks it isn't denied would catch future changes in how core builds the transclusion's request.
There's also a small wording fix inline for the doc comment in Hooks.php.
Generated by Claude Code
Skipping the check for transcluded special pages also lets every request that renders wikitext supplied by the client, such as action=parse with text, run a protected special page, which 1.7.0 denied. The skip therefore only applies with $wgCrawlerProtectionAllowTranscludedSpecialPages set to true, so a default install keeps the behaviour of 1.7.0, and the version becomes 1.8.0. The README documents the option, lists more requests that render client-supplied wikitext with how to protect them, and states that CrawlerProtectionShouldDeny does not run for transcluded special pages while the option is on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0af9e55d-60c0-4046-b4d4-c9f357d779c4
|
Thanks for the careful review. 65a7734 addresses it:
|
Adds
$wgCrawlerProtectionAllowTranscludedSpecialPages(defaultfalse). Whentrue, special pages transcluded into a wiki page, such as{{Special:RecentChanges/10}}, are not checked. Without it, a wiki page that transcludes a protected special page shows the denial instead: to anonymous viewers, and in miser mode, where MediaWiki runs a transcluded special page as an anonymous user, to every viewer; with$wgCrawlerProtectionRawDenialthe denial replaces the whole page. The option lets requests that render client-supplied wikitext, such asaction=parsewithtext, run a protected special page, as the README spells out, so it is off by default and the version becomes 1.8.0.Human input
Goal and shape: goal from @kghbln, to fix a wiki whose pages went blank this way; @kghbln chose to fix it here rather than with a
CrawlerProtectionShouldDenyhandler on that wiki. After the review, @kghbln made the exemption an option, off by default. The technical design within that is Claude's.Outcome: not reviewed by @kghbln yet. Left for you: the change and the README wording.
Claude's calls
Hooks::onSpecialPageBeforeExecute, which asks the newCrawlerProtectionService::allowsTranscludedSpecialPages();CrawlerProtectionShouldDenydoes not run for a transcluded special page while the option is on, documented on the hook interface and in the README, since running it with$shouldDeny = falsewould let a handler deny in the middle of rendering again, deciding on a stand-in anonymous user; the README names the REST transform endpoint rather than a pattern.FauxRequest), whichincluding()states directly./v1/…paths fromcoreRoutes.jsontoRestCheckCanExecuteunchanged, as no module namedv1is registered, which would keep the README's example/page/*/historyfrom matching. Read from core's code only and not touched here; details in the README thread.The change
extension.json:$wgCrawlerProtectionAllowTranscludedSpecialPages, defaultfalse; version 1.8.0.includes/CrawlerProtectionService.php:allowsTranscludedSpecialPages().includes/Hooks.php:onSpecialPageBeforeExecutereturnstruefor a transcluded special page while the option is on.README.mdandincludes/Hook/CrawlerProtectionShouldDenyHook.php: the option, the requests it opens up and how to close them, and the hook not running for transcluded special pages.HooksTestcovers both settings;CrawlerProtectionIntegrationTestrenders{{Special:WhatLinksHere/Test}}with and without the option.$wgCrawlerProtectionAllowTranscludedSpecialPages: set it totruewhen wiki pages transclude a protected special page, which otherwise shows the denial; the README lists the requests it opens up.Verification evidence
Hooks.php, the option-on test fails: the special page's list is missing..phpcs.xmlon the six changed PHP files: clean, with mediawiki-codesniffer 45.0.0 as resolved in the MediaWiki tree (the repo pins 43.0.0)..github/scripts/check-version-bump.sh upstream/main HEAD: "Version bump 1.7.0 -> 1.8.0 is valid."recentchangesandallpagesprotected and raw denial on: anonymousapi.php?action=parse&text={{Special:Recentchanges/10}}and{{Special:AllPages/Widget:}}answer HTTP 403 with the raw denial text.Parser::braceSubstitution(REL1_43) runs a transcluded special page asUser::newFromName( '127.0.0.1', false )unlessmaxIncludeCacheTime()is 0, whichSpecialPagereturns only outside miser mode.SpecialPageFactory::executePathsetsincluding()beforerun()on REL1_39 and REL1_43.