Address review findings on 1.7.0: denial headers, API status code, hook entry points

[Copilot]

Follow-up to @cicalese's review of #64. The findings were: denial responses were only marked noindex on the HTML path, Action API denials returned 200 OK, and CrawlerProtectionShouldDeny was shipping as @stable to implement with an index.php-only signature.

Denial responses

• denyAccessRaw() now sends X-Robots-Tag: noindex,nofollow, covering the raw-403 and 418 strategies — the ones operators actually enable under crawler load.
• New ResponseFactory::markDenied( $response, ?int $statusCode ) marks denials that aren't rendered through OutputPage. Core answers an ApiCheckCanExecute veto with dieWithError() and no HTTP code, so ApiMain::handleException() reads getCode() === 0 and emits 200 OK; checkApiModules() now sets 403 explicitly. REST keeps its 403 from LocalizedHttpException and only gains the robot directive.

CrawlerProtectionShouldDeny

Added a string $entryPoint argument (constants ENTRY_POINT_INDEX / ENTRY_POINT_API / ENTRY_POINT_REST) before the interface stabilizes, and wired the hook into the API and REST paths rather than deferring them. $specialPageName is always null for api/rest; $request is documented nullable.

$wgHooks['CrawlerProtectionShouldDeny'][] = static function (
	$user, $request, $entryPoint, $specialPageName, &$shouldDeny
) {
	if ( $entryPoint === 'api' && $request && $request->getHeader( 'X-My-Crawler-Token' ) === $secret ) {
		$shouldDeny = false;
	}
};

Docs

• README no longer implies the allowlist change fixes reverse-proxy resolution; it states that WebRequest::getIP() behaves as it does everywhere in MediaWiki, including reporting the proxy address when the proxy is unregistered.
• The $wgCrawlerProtectionTrustXForwardedFor warning now requires a proxy that unconditionally sets or appends the header, names HAProxy's option forwardfor if-none as the gap, and gives correct HAProxy/nginx directives.
• The PR description's two claims about the allowlist "bug fix" still need rewording; suggested text is in the review reply.

Tests

• Integration: ApiCheckCanExecute / RestCheckCanExecute registration, a reflection check of handler signatures against core's hook interfaces (the class deliberately doesn't implement them, so PHP can't catch drift), and end-to-end API/REST service coverage. A real ApiMain::execute() isn't viable — PHPUnit runs as CLI, where the container service is built with cliMode = true.
• Unit: hook entry-point propagation, markDenied() behavior, and assertNotSame( '', … ) alongside the previously tautological i18n fallback assertions.

Smaller items

• fnmatch() guarded with function_exists(); absent, no REST path is treated as protected and a warning is logged.
• normalizeArrayConfig() no longer warns twice for a single non-string scalar.
• check-i18n-qqq.sh rewritten in PHP (drops the Python dependency) and now also fails on qqq keys orphaned from en.json.
• composer phpunit scoped to tests/phpunit/unit/, matching the documented behavior; added composer phpunit:integration. The CI job still runs the full directory.

[Copilot]

Pull request overview

This PR is a follow-up to prior review feedback, tightening denial behavior across all entry points (index.php/API/REST) by ensuring crawler directives are consistently applied, Action API denials use an explicit 403, and the CrawlerProtectionShouldDeny hook is extended to cover API/REST with an entry-point identifier.

Changes:

• Add ResponseFactory::markDenied() and use it from API/REST denial paths to send X-Robots-Tag: noindex,nofollow, plus an explicit 403 for Action API denials.
• Extend CrawlerProtectionShouldDeny with an $entryPoint argument (constants for index/api/rest) and run it for API and REST paths.
• Expand unit/integration tests and update docs/tooling (including phpunit script scoping and the i18n qqq checker rewrite).

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
includes/ResponseFactory.php	Adds consistent robot directives for non-OutputPage denials via markDenied() and extends raw denial headers.
includes/CrawlerProtectionService.php	Runs the should-deny hook for API/REST entry points and marks API/REST denials with robots header (and 403 for API).
includes/HookRunner.php	Updates hook runner signature to pass the new $entryPoint argument.
includes/Hook/CrawlerProtectionShouldDenyHook.php	Extends the hook contract with entry-point constants and a new $entryPoint parameter.
tests/phpunit/unit/ResponseFactoryTest.php	Adds unit coverage for markDenied() and strengthens raw-denial fallback assertions.
tests/phpunit/unit/CrawlerProtectionServiceTest.php	Updates hook-handler signatures and adds unit tests for API/REST hook propagation and denial header marking.
tests/phpunit/namespaced-stubs.php	Extends the WebResponse stub with statusHeader() for tests.
tests/phpunit/integration/CrawlerProtectionIntegrationTest.php	Adds integration coverage for API/REST hook registration and basic end-to-end API/REST denial behavior.
README.md	Updates denial/header behavior documentation and clarifies reverse-proxy/XFF trust guidance; documents new hook signature.
TESTING.md	Documents separate unit vs integration test commands.
composer.json	Scopes composer phpunit to unit tests and adds phpunit:integration.
.github/scripts/check-i18n-qqq.sh	Rewrites the i18n qqq checker in PHP and adds orphaned-key detection.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

includes/Hook/CrawlerProtectionShouldDenyHook.php:67

• The hook docblock currently claims $user/$request are always the namespaced classes (\MediaWiki\User\User, \MediaWiki\Request\WebRequest). Across supported MediaWiki versions these can be the legacy global classes (\User, \WebRequest), so the doc can mislead hook implementers into adding incompatible type hints. Document both possible types to reflect actual runtime values.

	 * @param \MediaWiki\User\User $user The user making the request
	 * @param \MediaWiki\Request\WebRequest|null $request The current request,
	 *   or null when the entry point cannot supply one

includes/CrawlerProtectionService.php:440

• When fnmatch() is unavailable, this warning will be emitted on every call to isProtectedRestPath(). Under crawler load that can create very high log volume, even though the message is static. Consider emitting it only once (per PHP process) to avoid flooding logs while still surfacing the configuration problem.

		if ( !function_exists( 'fnmatch' ) ) {
			$this->logger->warning(
				'CrawlerProtection: fnmatch() is unavailable, so ' .
					'CrawlerProtectedRestPaths cannot be evaluated.'
			);