Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ intensive.

# Configuration

* `$wgCrawlerProtectedActions` - array of actions to protect (default: `[ 'history' ]`).
Actions specified in this array will be denied for anonymous users.
Set to an empty array `[]` to disable `action=`-based restrictions for anonymous users (other checks
such as `type=revision`, `diff`, or `oldid` may still block requests).
* `$wgCrawlerProtectedSpecialPages` - array of special pages to protect
(default: `[ 'mobilediff', 'recentchangeslinked', 'whatlinkshere' ]`).
Supported values are special page names or their aliases regardless of case.
Expand Down
5 changes: 5 additions & 0 deletions extension.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
"SpecialPageBeforeExecute": "main"
},
"config": {
"CrawlerProtectedActions": {
"value": [
"history"
]
},
"CrawlerProtectedSpecialPages": {
"value": [
"mobilediff",
Expand Down
7 changes: 5 additions & 2 deletions includes/Hooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class Hooks implements MediaWikiPerformActionHook, SpecialPageBeforeExecuteHook
* Block sensitive page views for anonymous users via MediaWikiPerformAction.
* Handles:
* - ?type=revision
* - ?action=history
* - ?action=<configurable actions>
* - ?diff=1234
* - ?oldid=1234
*
Expand All @@ -70,11 +70,14 @@ public function onMediaWikiPerformAction(
$diffId = (int)$request->getVal( 'diff' );
$oldId = (int)$request->getVal( 'oldid' );

$config = MediaWikiServices::getInstance()->getMainConfig();
$protectedActions = $config->get( 'CrawlerProtectedActions' );

if (
!$user->isRegistered()
&& (
$type === 'revision'
|| $action === 'history'
|| in_array( $action, $protectedActions, true )
Comment on lines +73 to +80
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CrawlerProtectedActions is now configurable, but there are no tests covering non-default configuration values (e.g. empty array allowing action=history, or protecting an additional action). Adding such cases would ensure the new config wiring is actually exercised and remains stable across MediaWiki versions.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied all feedback changes in commit 91c86bc:

  1. Fixed getVal() mocks to use willReturnCallback for proper single-argument matching
  2. Removed unnecessary test skips from history action tests (they now work in both environments)
  3. Made stub config controllable via MediaWikiServices::$testProtectedActions static property
  4. Added 3 new tests for non-default configurations:
    • testEmptyProtectedActionsAllowsHistory - verifies empty array allows history
    • testCustomProtectedActionBlocks - verifies custom actions are blocked
    • testCustomProtectedActionAllowsOtherActions - verifies non-configured actions are allowed
  5. Clarified README wording that empty array only disables action-based restrictions

All 26 tests pass and code follows MediaWiki standards.

|| $diffId > 0
|| $oldId > 0
)
Expand Down
8 changes: 8 additions & 0 deletions tests/phpunit/namespaced-stubs.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ class MediaWikiServices {
/** @var bool Control CrawlerProtectionUse418 config for testing */
public static $testUse418 = false;

/** @var array|null Control CrawlerProtectedActions config for testing */
public static $testProtectedActions = null;

/**
* @return MediaWikiServices
*/
Expand Down Expand Up @@ -130,6 +133,11 @@ public function getMainConfig() {
* @return mixed
*/
public function get( $name ) {
if ( $name === 'CrawlerProtectedActions' ) {
return MediaWikiServices::$testProtectedActions !== null
? MediaWikiServices::$testProtectedActions
: [ 'history' ];
}
Comment on lines +136 to +140
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

namespaced-stubs.php hardcodes CrawlerProtectedActions to [ 'history' ], which makes it impossible to unit-test the new configurability (e.g., verifying that an empty array allows action=history, or that another action can be protected). Consider making this stub value controllable (e.g., via a static property) so tests can cover non-default configurations.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feedback has already been addressed in commit 91c86bc. The changes include:

  • Added MediaWikiServices::$testProtectedActions static property to make stub configuration controllable
  • Fixed mocks to use willReturnCallback for proper parameter matching
  • Removed unnecessary test skips
  • Added 3 new tests covering empty array and custom action configurations
  • Updated README wording to clarify empty array behavior

All 26 tests pass ✅

if ( $name === 'CrawlerProtectedSpecialPages' ) {
return [
'RecentChangesLinked',
Expand Down
239 changes: 238 additions & 1 deletion tests/phpunit/unit/HooksTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,13 @@ public static function setUpBeforeClass(): void {
*/
protected function tearDown(): void {
parent::tearDown();
// Reset the test config flag (only exists in stub environment)
// Reset the test config flags (only exists in stub environment)
if ( property_exists( '\MediaWiki\MediaWikiServices', 'testUse418' ) ) {
\MediaWiki\MediaWikiServices::$testUse418 = false;
}
if ( property_exists( '\MediaWiki\MediaWikiServices', 'testProtectedActions' ) ) {
\MediaWiki\MediaWikiServices::$testProtectedActions = null;
}
// Only reset if the method exists (in our test stubs)
if ( method_exists( '\MediaWiki\MediaWikiServices', 'resetForTesting' ) ) {
\MediaWiki\MediaWikiServices::resetForTesting();
Expand Down Expand Up @@ -160,6 +163,240 @@ public function testNonRevisionTypeAlwaysAllowed() {
$this->assertTrue( $result );
}

/**
* @covers ::onMediaWikiPerformAction
*/
public function testHistoryActionBlocksAnonymous() {
$output = $this->createMock( self::$outputPageClassName );

$request = $this->createMock( self::$webRequestClassName );
$request->method( 'getVal' )->willReturnCallback( static function ( $key, $default = null ) {
if ( $key === 'action' ) {
return 'history';
}
return $default;
} );

$user = $this->createMock( self::$userClassName );
$user->method( 'isRegistered' )->willReturn( false );

$article = $this->createMock( self::$articleClassName );
$title = $this->createMock( self::$titleClassName );
$wiki = $this->createMock( self::$actionEntryPointClassName );

$runner = $this->getMockBuilder( Hooks::class )
->onlyMethods( [ 'denyAccess' ] )
->getMock();
$runner->expects( $this->once() )->method( 'denyAccess' );

$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
$this->assertFalse( $result );
}

/**
* @covers ::onMediaWikiPerformAction
*/
public function testHistoryActionAllowsLoggedIn() {
$output = $this->createMock( self::$outputPageClassName );

$request = $this->createMock( self::$webRequestClassName );
$request->method( 'getVal' )->willReturnCallback( static function ( $key, $default = null ) {
if ( $key === 'action' ) {
return 'history';
}
return $default;
} );

$user = $this->createMock( self::$userClassName );
$user->method( 'isRegistered' )->willReturn( true );

$article = $this->createMock( self::$articleClassName );
$title = $this->createMock( self::$titleClassName );
$wiki = $this->createMock( self::$actionEntryPointClassName );

$runner = $this->getMockBuilder( Hooks::class )
->onlyMethods( [ 'denyAccess' ] )
->getMock();
$runner->expects( $this->never() )->method( 'denyAccess' );

$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
$this->assertTrue( $result );
}

/**
* @covers ::onMediaWikiPerformAction
*/
public function testEmptyProtectedActionsAllowsHistory() {
// Skip this test in MediaWiki environment - it requires service container
if ( !property_exists( '\MediaWiki\MediaWikiServices', 'testProtectedActions' ) ) {
$this->markTestSkipped(
'Test requires stub MediaWikiServices with testProtectedActions. Skipped in MediaWiki environment.'
);
}

Comment on lines +230 to +236
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These history-action tests are skipped whenever the real MediaWikiServices is present, but Hooks::onMediaWikiPerformAction() now always uses MediaWikiServices::getInstance()->getMainConfig(), and the other onMediaWikiPerformAction tests in this file are not skipped. This skip will unnecessarily drop coverage in a real MediaWiki test run; consider removing it (or only skipping when CrawlerProtectedActions cannot be read).

Suggested change
// Skip this test in MediaWiki environment - it requires service container
if ( !property_exists( '\MediaWiki\MediaWikiServices', 'testUse418' ) ) {
$this->markTestSkipped(
'Test requires stub MediaWikiServices. Skipped in MediaWiki unit test environment.'
);
}

Copilot uses AI. Check for mistakes.
// Set empty array to allow all actions
\MediaWiki\MediaWikiServices::$testProtectedActions = [];

$output = $this->createMock( self::$outputPageClassName );

$request = $this->createMock( self::$webRequestClassName );
$request->method( 'getVal' )->willReturnCallback( static function ( $key, $default = null ) {
if ( $key === 'action' ) {
return 'history';
}
return $default;
} );

$user = $this->createMock( self::$userClassName );
$user->method( 'isRegistered' )->willReturn( false );

$article = $this->createMock( self::$articleClassName );
$title = $this->createMock( self::$titleClassName );
$wiki = $this->createMock( self::$actionEntryPointClassName );

$runner = $this->getMockBuilder( Hooks::class )
->onlyMethods( [ 'denyAccess' ] )
->getMock();
$runner->expects( $this->never() )->method( 'denyAccess' );

$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
$this->assertTrue( $result );
}

/**
* @covers ::onMediaWikiPerformAction
*/
public function testCustomProtectedActionBlocks() {
// Skip this test in MediaWiki environment - it requires service container
if ( !property_exists( '\MediaWiki\MediaWikiServices', 'testProtectedActions' ) ) {
$this->markTestSkipped(
'Test requires stub MediaWikiServices with testProtectedActions. Skipped in MediaWiki environment.'
);
}

// Set custom protected actions
\MediaWiki\MediaWikiServices::$testProtectedActions = [ 'edit', 'delete' ];

$output = $this->createMock( self::$outputPageClassName );

$request = $this->createMock( self::$webRequestClassName );
$request->method( 'getVal' )->willReturnCallback( static function ( $key, $default = null ) {
if ( $key === 'action' ) {
return 'edit';
}
return $default;
} );

$user = $this->createMock( self::$userClassName );
$user->method( 'isRegistered' )->willReturn( false );

$article = $this->createMock( self::$articleClassName );
$title = $this->createMock( self::$titleClassName );
$wiki = $this->createMock( self::$actionEntryPointClassName );

$runner = $this->getMockBuilder( Hooks::class )
->onlyMethods( [ 'denyAccess' ] )
->getMock();
$runner->expects( $this->once() )->method( 'denyAccess' );

$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
$this->assertFalse( $result );
}

/**
* @covers ::onMediaWikiPerformAction
*/
public function testCustomProtectedActionAllowsOtherActions() {
// Skip this test in MediaWiki environment - it requires service container
if ( !property_exists( '\MediaWiki\MediaWikiServices', 'testProtectedActions' ) ) {
$this->markTestSkipped(
'Test requires stub MediaWikiServices with testProtectedActions. Skipped in MediaWiki environment.'
);
}

// Set custom protected actions that don't include 'history'
\MediaWiki\MediaWikiServices::$testProtectedActions = [ 'edit', 'delete' ];

$output = $this->createMock( self::$outputPageClassName );

$request = $this->createMock( self::$webRequestClassName );
$request->method( 'getVal' )->willReturnCallback( static function ( $key, $default = null ) {
if ( $key === 'action' ) {
return 'history';
}
return $default;
} );

$user = $this->createMock( self::$userClassName );
$user->method( 'isRegistered' )->willReturn( false );

$article = $this->createMock( self::$articleClassName );
$title = $this->createMock( self::$titleClassName );
$wiki = $this->createMock( self::$actionEntryPointClassName );

$runner = $this->getMockBuilder( Hooks::class )
->onlyMethods( [ 'denyAccess' ] )
->getMock();
$runner->expects( $this->never() )->method( 'denyAccess' );

$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
$this->assertTrue( $result );
}

/**
* @covers ::onMediaWikiPerformAction
*/
public function testDiffParameterBlocksAnonymous() {
$output = $this->createMock( self::$outputPageClassName );

$request = $this->createMock( self::$webRequestClassName );
$request->method( 'getVal' )->willReturnMap( [
[ 'diff', null, '1234' ],
] );

$user = $this->createMock( self::$userClassName );
$user->method( 'isRegistered' )->willReturn( false );

$article = $this->createMock( self::$articleClassName );
$title = $this->createMock( self::$titleClassName );
$wiki = $this->createMock( self::$actionEntryPointClassName );

$runner = $this->getMockBuilder( Hooks::class )
->onlyMethods( [ 'denyAccess' ] )
->getMock();
$runner->expects( $this->once() )->method( 'denyAccess' );

$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
$this->assertFalse( $result );
}

/**
* @covers ::onMediaWikiPerformAction
*/
public function testOldidParameterBlocksAnonymous() {
$output = $this->createMock( self::$outputPageClassName );

$request = $this->createMock( self::$webRequestClassName );
$request->method( 'getVal' )->willReturnMap( [
[ 'oldid', null, '5678' ],
] );

$user = $this->createMock( self::$userClassName );
$user->method( 'isRegistered' )->willReturn( false );

$article = $this->createMock( self::$articleClassName );
$title = $this->createMock( self::$titleClassName );
$wiki = $this->createMock( self::$actionEntryPointClassName );

$runner = $this->getMockBuilder( Hooks::class )
->onlyMethods( [ 'denyAccess' ] )
->getMock();
$runner->expects( $this->once() )->method( 'denyAccess' );

$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
$this->assertFalse( $result );
}

/**
* @covers ::onSpecialPageBeforeExecute
* @dataProvider provideBlockedSpecialPages
Expand Down