Protect api.php and rest.php entry points from crawlers - #56
Conversation
…Execute) entry points - Add CrawlerProtectedApiModules config (default []) for Action API module protection - Add CrawlerProtectedRestPaths config (default []) for REST API path protection via fnmatch glob patterns - Register ApiCheckCanExecute and RestCheckCanExecute hooks in extension.json - Restructure Hooks.php with namespace-block compat stubs so the class implements both new hook interfaces on all supported MW versions; REST protection is silently skipped on MW < 1.42 where RestCheckCanExecute does not fire - Add checkApiModule, isProtectedApiModule, checkRestPath, isProtectedRestPath to CrawlerProtectionService - Add stub interfaces for ApiCheckCanExecuteHook, RestCheckCanExecuteHook, HttpException in namespaced-stubs.php - Add unit tests for all new service methods and hook handlers - Update README with entry-point coverage table and new config documentation Closes #48
There was a problem hiding this comment.
Pull request overview
Adds crawler-protection coverage to non-index.php entry points by wiring Action API and REST API pre-execution hooks into the existing CrawlerProtectionService, with opt-in configuration to avoid breaking existing anonymous consumers.
Changes:
- Register and implement
ApiCheckCanExecuteandRestCheckCanExecutehook handlers that delegate allow/deny decisions toCrawlerProtectionService. - Add new config settings for protected API modules and REST path patterns, plus README documentation for entry-point coverage and configuration.
- Extend unit tests and test stubs to cover the new hook handlers and service methods.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| includes/Hooks.php | Adds API/REST hook handlers and compatibility stubs; delegates to service for allow/deny. |
| includes/CrawlerProtectionService.php | Adds API module and REST path protection logic (check* + isProtected*). |
| extension.json | Registers new hooks and exposes new config keys. |
| README.md | Documents entry-point coverage and new configuration options. |
| tests/phpunit/unit/HooksTest.php | Adds delegation tests for the new hook handlers. |
| tests/phpunit/unit/CrawlerProtectionServiceTest.php | Adds unit tests for API/REST protection logic and configuration. |
| tests/phpunit/namespaced-stubs.php | Adds stub hook interfaces and a REST HttpException stub for unit tests. |
…pe, and agent setup workflow (#61) * Initial plan * Overhaul Copilot instructions with repo-specific CI architecture and testing guidance * Clarify trimmed guidance and Copilot setup workflow intent * Restore comprehensive MediaWiki instruction guidance --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com>
|
@copilot Please address failing pipelines for REL1_39 |
…r REST denial Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com>
…t, enforce minus-x (#59) * Initial plan * ci: hygiene improvements - pin actions, extend matrix, add dependabot * ci: skip minus-x on REL1_39 where MediaWiki does not ship it Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com>
Fixed in b07351d — all CI jobs are now green (run #150). Three failures, all rooted in the compat stub interfaces added to
|
Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.7 to 7.0.1. - [Release notes](https://github.com/actions/checkout/releases) - [Commits](actions/checkout@v4.1.7...v7.0.1) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
@copilot Pull the latest changes from the main branch and then review the code |
…ata-in-api-rest Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com>
Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com>
|
@copilot Please fix these pipeline errors: |
…pcs) Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com>
* i18n: add qqq.json, descriptionmsg, localised raw-denial responses, and CI qqq completeness check (#55) * Initial plan * Add i18n improvements: qqq.json, descriptionmsg, localised raw-denial, CI check * tests: add unit tests for i18n fallback in denyAccess and denyAccessWith418 * tests: refactor i18n fallback tests to AAA format with explicit assertions * fix: use ->plain() instead of ->text() to resolve SecurityCheck-XSS phan errors * fix: suppress SecurityCheck-XSS for trusted message bodies in raw denial --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Protect api.php and rest.php entry points from crawlers (#56) * Initial plan * feat: protect api.php (ApiCheckCanExecute) and rest.php (RestCheckCanExecute) entry points - Add CrawlerProtectedApiModules config (default []) for Action API module protection - Add CrawlerProtectedRestPaths config (default []) for REST API path protection via fnmatch glob patterns - Register ApiCheckCanExecute and RestCheckCanExecute hooks in extension.json - Restructure Hooks.php with namespace-block compat stubs so the class implements both new hook interfaces on all supported MW versions; REST protection is silently skipped on MW < 1.42 where RestCheckCanExecute does not fire - Add checkApiModule, isProtectedApiModule, checkRestPath, isProtectedRestPath to CrawlerProtectionService - Add stub interfaces for ApiCheckCanExecuteHook, RestCheckCanExecuteHook, HttpException in namespaced-stubs.php - Add unit tests for all new service methods and hook handlers - Update README with entry-point coverage table and new config documentation Closes #48 * Operationalize Copilot guidance with repo-specific rules, correct scope, and agent setup workflow (#61) * Initial plan * Overhaul Copilot instructions with repo-specific CI architecture and testing guidance * Clarify trimmed guidance and Copilot setup workflow intent * Restore comprehensive MediaWiki instruction guidance --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: use FNM_PATHNAME for REST path globs, fix docblock, bump version Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * fix: drop core hook interface stubs and use LocalizedHttpException for REST denial Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * ci: pin actions to SHAs, extend matrix to MW 1.44/1.45, add Dependabot, enforce minus-x (#59) * Initial plan * ci: hygiene improvements - pin actions, extend matrix, add dependabot * ci: skip minus-x on REL1_39 where MediaWiki does not ship it Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Bump actions/checkout from 4.1.7 to 7.0.1 (#65) Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.7 to 7.0.1. - [Release notes](https://github.com/actions/checkout/releases) - [Commits](actions/checkout@v4.1.7...v7.0.1) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Merge main; cover query sub-modules and correct REST hook MW version Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * test: extract REST handler stub helper with property docs (MW 1.45 phpcs) Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Jeffrey Wang <jeffw16@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix IP allowlist to use WebRequest::getIP(); add temp-account policy; normalize array configs (#57) * Initial plan * Fix IP allowlist to use WebRequest::getIP(), add TreatTempUsersAsAnon config, normalize array configs - CrawlerProtectionService: derive client IP from $request->getIP() (not $user->getName()) in both checkPerformAction() and checkSpecialPage() - Add $request parameter to checkSpecialPage(); update Hooks.php to pass $special->getContext()->getRequest() - Add CrawlerProtectionTreatTempUsersAsAnon config (default false) and isUserAllowed() helper; temp-account users (isRegistered+isTemp) are treated as anonymous when the flag is true - Pre-normalise all array-valued configs at construction time: coerce scalars to single-element arrays, drop non-string entries, log warnings (prevents fatal on misconfigured scalar values) - Validate CrawlerProtectionAllowedIPs entries at construction time and log a warning for invalid IP/range strings - Inject LoggerInterface via constructor and ServiceWiring.php - Add CrawlerProtectionTreatTempUsersAsAnon to extension.json - Update tests: fix IP tests to use request->getIP(), add $request param to checkSpecialPage() calls, add tests for temp accounts, IP-from- request, and scalar misconfiguration - Add getIP() to WebRequest stub for unit tests - Document new config options in README.md * Operationalize Copilot guidance with repo-specific rules, correct scope, and agent setup workflow (#61) * Initial plan * Overhaul Copilot instructions with repo-specific CI architecture and testing guidance * Clarify trimmed guidance and Copilot setup workflow intent * Restore comprehensive MediaWiki instruction guidance --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Validate IP allowlist entries with IPUtils only; make temp-user mocks version-safe Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * ci: pin actions to SHAs, extend matrix to MW 1.44/1.45, add Dependabot, enforce minus-x (#59) * Initial plan * ci: hygiene improvements - pin actions, extend matrix, add dependabot * ci: skip minus-x on REL1_39 where MediaWiki does not ship it Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * tests: give the Hooks mock context a getRequest() so special-page tests pass Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> Co-authored-by: Jeffrey Wang <jeffw16@users.noreply.github.com> * test: add integration tests and fix denyAccessPretty branch testability (#58) * Initial plan * test: add integration tests and fix denyAccessPretty testability - Replace version_compare(MW_VERSION,...) with method_exists() in ResponseFactory::denyAccessPretty() so both title-setter branches (setPageTitle for MW<1.41, setPageTitleMsg for MW>=1.41) are reachable in tests without redefining the MW_VERSION constant. - Add tests/phpunit/integration/CrawlerProtectionIntegrationTest.php (MediaWikiIntegrationTestCase) covering: * Service-container wiring: both services resolve from the real container * Hook registration: MediaWikiPerformAction and SpecialPageBeforeExecute are registered * Real OutputPage state: denyAccessPretty() sets HTTP 403 on a genuine OutputPage (CI matrix runs REL1_39 + REL1_43, covering both method_exists() branches) * End-to-end behaviour: anonymous users are blocked on protected actions and special pages; registered users are not - Add unit tests for both branches of the new method_exists() logic in ResponseFactoryTest: modern path (setPageTitleMsg called, setPageTitle never) and legacy path (anonymous stub without setPageTitleMsg triggers setPageTitle). - composer.json: restrict standalone `composer phpunit` to unit/ only so integration tests (which need a real MediaWiki install) are not run in isolation; register integration test namespace in autoload-dev. - ci.yml: convert phpunit matrix to explicit include entries and enable pcov coverage on the REL1_43/PHP 8.2 job, passing --coverage-text to phpunit when coverage != none. Closes #50 * Operationalize Copilot guidance with repo-specific rules, correct scope, and agent setup workflow (#61) * Initial plan * Overhaul Copilot instructions with repo-specific CI architecture and testing guidance * Clarify trimmed guidance and Copilot setup workflow intent * Restore comprehensive MediaWiki instruction guidance --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * ci: pin actions to SHAs, extend matrix to MW 1.44/1.45, add Dependabot, enforce minus-x (#59) * Initial plan * ci: hygiene improvements - pin actions, extend matrix, add dependabot * ci: skip minus-x on REL1_39 where MediaWiki does not ship it Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Bump actions/checkout from 4.1.7 to 7.0.1 (#65) Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.7 to 7.0.1. - [Release notes](https://github.com/actions/checkout/releases) - [Commits](actions/checkout@v4.1.7...v7.0.1) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * test: make integration tests version-agnostic for Title/FauxRequest/User Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jeffrey Wang <jeffw16@users.noreply.github.com> * Add CrawlerProtectionShouldDeny hook for bespoke access policy (#62) * Initial plan * Add CrawlerProtectionShouldDeny hook * Merge candidate/1.7.0 and fix unit tests to not construct HookContainer Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * tests: assert the hook still runs for allowlisted IPs Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Jeffrey Wang <jeffw16@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Mark denial responses as noindex,nofollow (#63) * Initial plan * Send X-Robots-Tag on denials and robot policy on pretty denial page * Use WebResponse::header() for X-Robots-Tag on the pretty denial path Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Scope X-Robots-Tag to the pretty denial path only Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Resolve merge conflicts with candidate/1.7.0 and bump to 1.7.1 Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Downgrade version from 1.7.1 to 1.7.0 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Jeffrey Wang <jeffw16@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Use canonical request IP for API/REST allowlist; run denial tests under MediaWiki Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Add opt-in X-Forwarded-For allowlist matching for wikis behind a reverse proxy Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Add missing CrawlerProtectionTrustXForwardedFor key to scalar-config unit tests Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Address cicalese review: denial headers, API 403, hook entry point, docs and tests (#66) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Address cicalese follow-up: signature test names, real ApiMain coverage, REST path docs Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Make ApiMain integration assertions version-agnostic Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> * Soften ApiMain integration test status assertion comments Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: jeffw16 <11380894+jeffw16@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Both
api.phpandrest.phpwere entirely unprotected — a crawler blocked onindex.php?action=historycould trivially fetch identical data viaapi.php?action=query&prop=revisionsor/page/{title}/history. This adds hook-based protection for both entry points with the same bypass semantics (registered users, allowed IPs) as the existingindex.phpprotection.New configuration
$wgCrawlerProtectedApiModules[]$wgCrawlerProtectedRestPaths[]fnmatch()Both default to empty — operators must opt in. This avoids silently breaking existing anonymous API consumers on upgrade.
Example configuration protecting the most crawler-attractive endpoints:
Implementation
ApiCheckCanExecutehook — fires for every API module before execution; denies with a message key when the module name is in the protected list.RestCheckCanExecutehook — fires on MW 1.42+; denies with anHttpException(403)when the request path matches a configured glob pattern. Silently inactive on MW 1.39–1.41 (hook predates those versions).$userfor API,$handler->getAuthority()->getUser()for REST), then delegate the allow/deny decision toCrawlerProtectionService.Hooks.phpis restructured to bracketted namespace blocks so it can conditionally define no-op stub interfaces forApiCheckCanExecuteHookandRestCheckCanExecuteHookon MW versions that don't ship them — allowing the class to unconditionallyimplementboth without fatal errors.CrawlerProtectionServicegainscheckApiModule(),isProtectedApiModule(),checkRestPath(), andisProtectedRestPath().Coverage table (added to README)
index.phppage views / actions / diffsindex.phpSpecial pagesapi.phpAction API modules$wgCrawlerProtectedApiModulesrest.phpREST API paths$wgCrawlerProtectedRestPaths(MW 1.42+)