Allow anonymous revision metadata queries to bypass crawler protection

README.md

@@ -62,6 +62,35 @@ addresses in `$wgCrawlerProtectionAllowedIPs` are always permitted.
   ```php
   $wgCrawlerProtectedApiModules = [ 'compare', 'parse', 'revisions', 'recentchanges', 'backlinks' ];
   ```
+* `$wgCrawlerProtectionAllowRevisionMetadata` - when `true` (default), an
+  anonymous `action=query` request is not denied on account of `'revisions'`
+  being in `$wgCrawlerProtectedApiModules` if it only asks for the timestamp
+  of the latest revision of a single page. This keeps
+  [Page Previews](https://www.mediawiki.org/wiki/Extension:Popups) working
+  for anonymous readers: its preview request is
+  `action=query&prop=info|extracts|pageimages|revisions|info&rvprop=timestamp&titles=<page>`
+  (plus `formatversion`, `redirects`, `inprop`, and the `ex*` and `pi*`
+  parameters of TextExtracts and PageImages), and without this exemption every
+  preview fails with "There was an issue displaying this preview." A request
+  is let through only when all of the following hold:
+  - `revisions` is the only protected module in the request; any other
+    protected module (or a protected `query`) still denies it;
+  - it names exactly one page through a single `titles` or `pageids` value,
+    and uses neither `generator` nor `revids`;
+  - `rvprop` is present and contains only `timestamp`. A missing `rvprop`
+    does not qualify, because the API then returns its default props, which
+    include the user and the edit summary;
+  - no other `rv*` parameter is present (`rvlimit`, `rvstart`, `rvend`,
+    `rvstartid`, `rvendid`, `rvdir`, `rvuser`, `rvexcludeuser`, `rvtag`,
+    `rvcontinue`, `rvslots`, `rvsection`, `rvparse`, `rvexpandtemplates`,
+    `rvgeneratexml`, `rvdiffto`, `rvdifftotext`, `rvdifftotextpst`,
+    `rvcontentformat` and `rvcontentformat-{slot}`), since each of them pages
+    through revisions or reads revision content.
+
+  Multi-value parameters are recognised in both the `|` and the
+  leading-`\x1f` form. Set to `false` to deny every anonymous request that
+  involves a protected `revisions` module. The `CrawlerProtectionShouldDeny`
+  hook still receives the final decision and can override it.
 * `$wgCrawlerProtectedRestPaths` - array of REST API path glob patterns to
   block for anonymous users (default: `[]`). Each pattern is tested with
   `fnmatch()` with the `FNM_PATHNAME` flag, so `*` matches any single path

extension.json

@@ -1,7 +1,7 @@
 {
     "name": "CrawlerProtection",
     "author": "[https://mywikis.com MyWikis LLC]",
-    "version": "1.7.0",
+    "version": "1.8.0",
     "url": "https://www.mediawiki.org/wiki/Extension:CrawlerProtection",
     "descriptionmsg": "crawlerprotection-desc",
     "type": "hook",
@@ -70,6 +70,9 @@
             "value": [],
             "merge_strategy": "provide_default"
         },
+        "CrawlerProtectionAllowRevisionMetadata": {
+            "value": true
+        },
         "CrawlerProtectionProtectRevisions": {
             "value": true
         },

includes/CrawlerProtectionService.php

@@ -53,11 +53,22 @@ class CrawlerProtectionService {
 		'CrawlerProtectedRestPaths',
 		'CrawlerProtectedSpecialPages',
 		'CrawlerProtectionAllowedIPs',
+		'CrawlerProtectionAllowRevisionMetadata',
 		'CrawlerProtectionProtectRevisions',
 		'CrawlerProtectionTreatTempUsersAsAnon',
 		'CrawlerProtectionTrustXForwardedFor',
 	];
 
+	/**
+	 * Values of the "rvprop" parameter that $wgCrawlerProtectionAllowRevisionMetadata
+	 * lets through. Page Previews (Popups) asks for "timestamp" only; every other
+	 * value exposes revision content, its author or edit summary, or more than
+	 * the bare metadata the previews need.
+	 *
+	 * @var string[]
+	 */
+	private const REVISION_METADATA_PROPS = [ 'timestamp' ];
+
 	/** @var ServiceOptions */
 	private ServiceOptions $options;
 
@@ -328,6 +339,10 @@ public function checkApiModules( array $moduleNames, $user, $request = null ): b
 					break;
 				}
 			}
+
+			if ( $shouldDeny && $this->isRevisionMetadataQuery( $moduleNames, $request ) ) {
+				$shouldDeny = false;
+			}
 		}
 
 		$this->hookRunner->onCrawlerProtectionShouldDeny(
@@ -352,6 +367,99 @@ public function checkApiModules( array $moduleNames, $user, $request = null ): b
 		return true;
 	}
 
+	/**
+	 * Determine whether an action=query request only asks the "revisions"
+	 * module for metadata of the latest revision of a single page, which
+	 * $wgCrawlerProtectionAllowRevisionMetadata lets anonymous users through.
+	 *
+	 * Page Previews (Popups) sends action=query&prop=...|revisions&rvprop=timestamp
+	 * for one title, so protecting "revisions" would otherwise break previews
+	 * for anonymous readers. The rules are deliberately narrow:
+	 * - "revisions" must be the only protected module in the request;
+	 * - exactly one page is named by "titles" or "pageids", and neither a
+	 *   generator nor "revids" is used, so the request cannot fan out;
+	 * - "rvprop" is given explicitly and holds only REVISION_METADATA_PROPS,
+	 *   because without it the API falls back to props that include the
+	 *   comment and user;
+	 * - no other "rv" parameter is present, since all of them page through
+	 *   revisions or read revision content (rvlimit, rvslots, rvsection, ...).
+	 *
+	 * @param string[] $moduleNames Module names involved in the request
+	 * @param WebRequest|null $request
+	 * @return bool
+	 */
+	private function isRevisionMetadataQuery( array $moduleNames, $request ): bool {
+		if ( $request === null
+			|| !$this->options->get( 'CrawlerProtectionAllowRevisionMetadata' )
+			|| strtolower( $moduleNames[0] ?? '' ) !== 'query'
+		) {
+			return false;
+		}
+
+		$protectedModules = array_unique( array_map(
+			'strtolower',
+			array_filter( $moduleNames, [ $this, 'isProtectedApiModule' ] )
+		) );
+		if ( array_values( $protectedModules ) !== [ 'revisions' ] ) {
+			return false;
+		}
+
+		if ( $request->getVal( 'generator' ) !== null || $request->getVal( 'revids' ) !== null ) {
+			return false;
+		}
+
+		$titles = $request->getVal( 'titles' );
+		$pageIds = $request->getVal( 'pageids' );
+		if ( $titles !== null && $pageIds !== null ) {
+			return false;
+		}
+		$pageTarget = $titles ?? $pageIds;
+		if ( $pageTarget === null ) {
+			return false;
+		}
+		$pages = self::splitApiMultiValue( $pageTarget );
+		if ( count( $pages ) !== 1 || $pages[0] === '' ) {
+			return false;
+		}
+
+		$revisionProps = $request->getVal( 'rvprop' );
+		if ( $revisionProps === null ) {
+			return false;
+		}
+		foreach ( self::splitApiMultiValue( $revisionProps ) as $prop ) {
+			if ( !in_array( $prop, self::REVISION_METADATA_PROPS, true ) ) {
+				return false;
+			}
+		}
+
+		foreach ( array_keys( $request->getValues() ) as $name ) {
+			$name = strtolower( (string)$name );
+			if ( strncmp( $name, 'rv', 2 ) === 0 && $name !== 'rvprop' ) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
+	/**
+	 * Split a multi-value Action API parameter into its values.
+	 *
+	 * Like ApiBase, this uses "\x1f" as the separator when the value starts
+	 * with that character, and "|" otherwise. Values are returned as sent:
+	 * they are neither trimmed nor filtered for empty strings.
+	 *
+	 * @since 1.8.0
+	 * @param string $value
+	 * @return string[]
+	 */
+	public static function splitApiMultiValue( string $value ): array {
+		if ( substr( $value, 0, 1 ) === "\x1f" ) {
+			return explode( "\x1f", substr( $value, 1 ) );
+		}
+		return explode( '|', $value );
+	}
+
 	/**
 	 * Determine whether the given API module name is in the
 	 * configured list of protected modules.

includes/Hooks.php

@@ -172,10 +172,7 @@ private function getApiModuleNames( $module ): array {
 			if ( $value === null || $value === '' ) {
 				continue;
 			}
-			// MediaWiki uses "\x1f" as the separator when a multi-value
-			// parameter starts with that character, and "|" otherwise.
-			$separator = substr( $value, 0, 1 ) === "\x1f" ? "\x1f" : '|';
-			foreach ( explode( $separator, $value ) as $subModule ) {
+			foreach ( CrawlerProtectionService::splitApiMultiValue( $value ) as $subModule ) {
 				$subModule = trim( $subModule );
 				if ( $subModule !== '' ) {
 					$names[] = $subModule;

tests/phpunit/integration/CrawlerProtectionIntegrationTest.php

@@ -72,6 +72,7 @@ private function overrideCrawlerProtectionConfig( array $overrides = [] ): void
 			'CrawlerProtectedSpecialPages'         => [ 'whatlinkshere', 'recentchangeslinked' ],
 			'CrawlerProtectedQueryParams'          => [ 'target' ],
 			'CrawlerProtectionAllowedIPs'          => [],
+			'CrawlerProtectionAllowRevisionMetadata' => true,
 			'CrawlerProtectionProtectRevisions'    => true,
 			'CrawlerProtectionTreatTempUsersAsAnon' => false,
 			'CrawlerProtectionTrustXForwardedFor'  => false,
@@ -763,4 +764,80 @@ public function testApiCheckCanExecuteAllowsRegisteredUserOnProtectedModule(): v
 			'A registered user must not be denied'
 		);
 	}
+
+	/**
+	 * A Page Previews (Popups) style request for the timestamp of a single
+	 * page must reach the API even though "revisions" is protected.
+	 *
+	 * TextExtracts and PageImages are not installed here, so only the core
+	 * modules of the Popups "prop" list are requested.
+	 *
+	 * @covers \MediaWiki\Extension\CrawlerProtection\Hooks::onApiCheckCanExecute
+	 * @covers \MediaWiki\Extension\CrawlerProtection\CrawlerProtectionService::checkApiModules
+	 */
+	public function testApiCheckCanExecuteAllowsAnonymousRevisionMetadataQuery(): void {
+		// Arrange
+		$this->overrideCrawlerProtectionConfig( [
+			'CrawlerProtectedApiModules' => [ 'revisions' ],
+		] );
+		$this->useWebModeServiceInContainer();
+		$api = $this->makeApiMain(
+			[
+				'action' => 'query',
+				'prop' => 'info|revisions|info',
+				'formatversion' => '2',
+				'redirects' => '1',
+				'rvprop' => 'timestamp',
+				'inprop' => 'url',
+				'titles' => 'Main Page',
+			],
+			$this->makeAnonUser()
+		);
+
+		// Act
+		$api->execute();
+
+		// Assert
+		$this->assertArrayHasKey(
+			'query',
+			$api->getResult()->getResultData( [], [ 'Strip' => 'all' ] ),
+			'A revision-metadata query must execute normally'
+		);
+	}
+
+	/**
+	 * The same request asking for revision content must still be denied.
+	 *
+	 * @covers \MediaWiki\Extension\CrawlerProtection\Hooks::onApiCheckCanExecute
+	 * @covers \MediaWiki\Extension\CrawlerProtection\CrawlerProtectionService::checkApiModules
+	 */
+	public function testApiCheckCanExecuteDeniesAnonymousRevisionContentQuery(): void {
+		// Arrange
+		$this->overrideCrawlerProtectionConfig( [
+			'CrawlerProtectedApiModules' => [ 'revisions' ],
+		] );
+		$this->useWebModeServiceInContainer();
+		$api = $this->makeApiMain(
+			[
+				'action' => 'query',
+				'prop' => 'info|revisions',
+				'rvprop' => 'timestamp|content',
+				'titles' => 'Main Page',
+			],
+			$this->makeAnonUser()
+		);
+
+		// Act
+		$denied = false;
+		try {
+			$api->execute();
+		} catch ( \Exception $e ) {
+			$denied = method_exists( $e, 'getStatusValue' )
+				// @phan-suppress-next-line PhanUndeclaredMethod ApiUsageException only
+				&& $e->getStatusValue()->hasMessage( 'crawlerprotection-accessdenied-text' );
+		}
+
+		// Assert
+		$this->assertTrue( $denied, 'A query for revision content must be denied' );
+	}
 }

tests/phpunit/namespaced-stubs.php

@@ -178,6 +178,13 @@ public function getVal( $name, $default = null ) {
 			return $default;
 		}
 
+		/**
+		 * @return array
+		 */
+		public function getValues() {
+			return [];
+		}
+
 		public function getIP(): string {
 			return '127.0.0.1';
 		}