diff --git a/projects/packages/waf/changelog/protect-174-waf-skip-rules-without-request-method b/projects/packages/waf/changelog/protect-174-waf-skip-rules-without-request-method new file mode 100644 index 000000000000..c31f8470f2ae --- /dev/null +++ b/projects/packages/waf/changelog/protect-174-waf-skip-rules-without-request-method @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +WAF: Default REQUEST_METHOD to GET when absent (e.g. server-side cron via PHP wrapper or headless browser), preventing false-positive 403 blocks from rule 911100. diff --git a/projects/packages/waf/src/class-waf-runner.php b/projects/packages/waf/src/class-waf-runner.php index 929f118cd7cd..3aeeae375590 100644 --- a/projects/packages/waf/src/class-waf-runner.php +++ b/projects/packages/waf/src/class-waf-runner.php @@ -243,6 +243,14 @@ public static function run() { return; } + // When running via a headless or pseudo-browser (e.g. server-side cron via a PHP + // wrapper that does not report PHP_SAPI as 'cli') REQUEST_METHOD may be absent. + // Default to GET so that rules validating the HTTP method (e.g. rule 911100) do + // not generate a false-positive 403 block. + if ( empty( $_SERVER['REQUEST_METHOD'] ) ) { + $_SERVER['REQUEST_METHOD'] = 'GET'; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized + } + // if something terrible happens during the WAF running, we don't want to interfere with the rest of the site, // so we intercept errors ONLY while the WAF is running, then we remove our handler after the WAF finishes. $display_errors = ini_get( 'display_errors' ); diff --git a/projects/packages/waf/tests/php/unit/WafRunnerTest.php b/projects/packages/waf/tests/php/unit/WafRunnerTest.php index 660e4b3f93e4..f479054ac4e5 100644 --- a/projects/packages/waf/tests/php/unit/WafRunnerTest.php +++ b/projects/packages/waf/tests/php/unit/WafRunnerTest.php @@ -7,6 +7,7 @@ use Automattic\Jetpack\Waf\Waf_Constants; use Automattic\Jetpack\Waf\Waf_Runner; +use PHPUnit\Framework\Attributes\PreserveGlobalState; use PHPUnit\Framework\Attributes\RunInSeparateProcess; /** @@ -51,4 +52,49 @@ public function testRunSetsConstants() { $this->assertSame( '/pseudo/dir/jetpack-waf', JETPACK_WAF_DIR ); $this->assertSame( '/pseudo/dir/../wp-config.php', JETPACK_WAF_WPCONFIG ); } + + /** + * Test that run defaults REQUEST_METHOD to GET when absent and continues to evaluate rules. + * + * This reproduces the scenario where wp-cron.php is executed directly via a PHP + * wrapper (or headless/pseudo-browser) that does not set REQUEST_METHOD, which + * previously caused rule 911100 to fire a 403 because the empty request method was + * not in the allowed-methods list. Defaulting to GET lets the rule pass while still + * allowing the WAF to protect the site. + * + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + #[RunInSeparateProcess] + #[PreserveGlobalState( false )] + public function testRunDefaultsRequestMethodToGetWhenAbsent() { + define( 'ABSPATH', '/pseudo' ); + define( 'WP_CONTENT_DIR', '/pseudo/dir' ); + + // Simulate a non-HTTP execution context: no REQUEST_METHOD present. + unset( $_SERVER['REQUEST_METHOD'] ); + + // Create a temporary rules file that would set a flag if evaluated. + $rules_dir = sys_get_temp_dir() . '/jetpack-waf-test-' . uniqid(); + $rules_file = $rules_dir . '/rules.php'; + mkdir( $rules_dir ); + file_put_contents( $rules_file, 'assertSame( 'GET', $_SERVER['REQUEST_METHOD'], 'REQUEST_METHOD must default to GET when absent.' ); + + // Rules SHOULD have been evaluated since the method is now valid. + $this->assertTrue( defined( 'JETPACK_WAF_RULES_EXECUTED' ), 'WAF rules must be evaluated when REQUEST_METHOD defaults to GET.' ); + + // Clean up. + unlink( $rules_file ); + rmdir( $rules_dir ); + } }