From 6018c57cdddff2e805d505dbd8276e3bb74219a2 Mon Sep 17 00:00:00 2001 From: TallblokeUK Date: Tue, 29 Sep 2026 08:46:36 +0100 Subject: [PATCH 1/4] fix: stop safe mode restarting current user resolution --- CHANGELOG.md | 1 + src/php/Integration/Evaluate_Functions.php | 31 +++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e94b17f42..7381d5d68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ * Fixed featured Community Cloud snippets failing to load with some cloud API responses. (PRO) * Fixed bulk actions in Community Cloud running against an empty selection, so selected snippets were never downloaded. (PRO) +* Fixed safe mode failing on sites running a plugin that builds URLs while WordPress is identifying the current user. ## [3.10.2] (2026-09-01) diff --git a/src/php/Integration/Evaluate_Functions.php b/src/php/Integration/Evaluate_Functions.php index 9034ab062..0dbd858be 100644 --- a/src/php/Integration/Evaluate_Functions.php +++ b/src/php/Integration/Evaluate_Functions.php @@ -68,7 +68,28 @@ public function is_safe_mode_query_var_set(): bool { * @return bool */ public function is_safe_mode_requested(): bool { - return $this->is_safe_mode_query_var_set() && code_snippets()->current_user_can(); + static $resolving = false; + + if ( ! $this->is_safe_mode_query_var_set() ) { + return false; + } + + // Asking for a capability makes WordPress resolve the current user, + // which runs third-party callbacks on determine_current_user. One that + // builds a URL arrives back here before that resolution has finished, + // and answering it again would restart it, recursing until the request + // ran out of memory. No user is known yet at that point, so it is no. + if ( $resolving ) { + return false; + } + + $resolving = true; + + try { + return code_snippets()->current_user_can(); + } finally { + $resolving = false; + } } /** @@ -81,6 +102,14 @@ public function is_safe_mode_requested(): bool { public function add_safe_mode_query_var( $url ): string { $url = is_string( $url ) ? $url : ''; + // A URL built before WordPress has settled on a user belongs to that + // resolution rather than to anything a visitor follows, so it is left + // alone. Safe mode links are generated while a page renders, which is + // long after this point. + if ( ! did_action( 'set_current_user' ) ) { + return $url; + } + return $this->is_safe_mode_requested() ? add_query_arg( 'snippets-safe-mode', true, $url ) : $url; From 0d9875d3e908f068d4a7493b1ac6bb993dbd456e Mon Sep 17 00:00:00 2001 From: TallblokeUK Date: Tue, 29 Sep 2026 08:46:45 +0100 Subject: [PATCH 2/4] test: cover safe mode during current user resolution --- .../Evaluate_Functions_Safe_Mode_Test.php | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/tests/unit/Integration/Evaluate_Functions_Safe_Mode_Test.php b/tests/unit/Integration/Evaluate_Functions_Safe_Mode_Test.php index 1f86df8ac..8535cba57 100644 --- a/tests/unit/Integration/Evaluate_Functions_Safe_Mode_Test.php +++ b/tests/unit/Integration/Evaluate_Functions_Safe_Mode_Test.php @@ -12,6 +12,36 @@ */ class Evaluate_Functions_Safe_Mode_Test extends UnitTestCase { + /** + * Calls to the current-user filter after which it stops building URLs. + * + * Past anything a correct implementation produces, but low enough that a + * regression fails on the count rather than exhausting the memory limit and + * taking the whole test run down with it. + */ + private const RESOLUTION_LIMIT = 5; + + /** + * How many times the current-user filter was called during a test. + * + * @var int + */ + private int $resolution_count = 0; + + /** + * The last URL built while the current user was being resolved. + * + * @var string|null + */ + private ?string $resolved_url = null; + + /** + * User the current-user filter resolves to. + * + * @var int|false + */ + private $user_to_resolve = false; + /** * Set up before each test. * @@ -20,6 +50,9 @@ class Evaluate_Functions_Safe_Mode_Test extends UnitTestCase { public function set_up() { parent::set_up(); unset( $_REQUEST['snippets-safe-mode'] ); + $this->resolution_count = 0; + $this->resolved_url = null; + $this->user_to_resolve = false; } /** @@ -29,9 +62,42 @@ public function set_up() { */ public function tear_down() { unset( $_REQUEST['snippets-safe-mode'] ); + remove_filter( 'determine_current_user', [ $this, 'resolve_user_through_a_url' ], 15 ); parent::tear_down(); } + /** + * Stand in for a plugin that builds a URL while the current user is being resolved. + * + * WooCommerce does this: its REST authentication runs on determine_current_user + * and calls home_url() to work out whether the request is for one of its routes. + * + * @param int|false $user_id Resolved user, from an unknown earlier callback. + * + * @return int|false + */ + public function resolve_user_through_a_url( $user_id ) { + ++$this->resolution_count; + + if ( $this->resolution_count < self::RESOLUTION_LIMIT ) { + $this->resolved_url = home_url( '/' ); + } + + return false === $this->user_to_resolve ? $user_id : $this->user_to_resolve; + } + + /** + * Force WordPress to resolve the current user again on the next capability check. + * + * @return void + */ + private function require_user_resolution(): void { + global $current_user; + + // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- emptying the cached user is the only way to make WordPress resolve one again, which is the whole point of these tests. + $current_user = null; + } + /** * The query var check must not touch capabilities. * @@ -88,4 +154,90 @@ public function test_execution_callback_tolerates_a_null_from_an_earlier_callbac $this->assertFalse( $evaluate->disable_snippet_execution( null ) ); $this->assertTrue( $evaluate->disable_snippet_execution( true ) ); } + + /** + * Resolving the current user must not re-enter the capability check. + * + * A plugin that builds a URL on determine_current_user reaches the URL + * filter before WordPress knows who the user is. Checking the capability + * again there restarts the resolution that is already running, which + * recurses until the request exhausts its memory limit. Any visitor can + * trigger it by putting the query var on a front-end URL. + * + * @return void + */ + public function test_url_filter_does_not_restart_user_resolution(): void { + $_REQUEST['snippets-safe-mode'] = '1'; + + $evaluate = new Evaluate_Functions( code_snippets()->db ); + + add_filter( 'determine_current_user', [ $this, 'resolve_user_through_a_url' ], 15 ); + $this->require_user_resolution(); + + $evaluate->disable_snippet_execution( true ); + + $this->assertSame( + 1, + $this->resolution_count, + 'the current user should be resolved once per request, not once per URL built while resolving' + ); + } + + /** + * A URL built during resolution comes back untouched. + * + * The user the request will turn out to belong to is not known while that + * URL is being built, so safe mode cannot yet be part of it, even when the + * user being resolved does hold the capability. + * + * @return void + */ + public function test_url_built_during_user_resolution_is_unchanged(): void { + $_REQUEST['snippets-safe-mode'] = '1'; + $this->user_to_resolve = $this->factory()->user->create( [ 'role' => 'administrator' ] ); + + $evaluate = new Evaluate_Functions( code_snippets()->db ); + + add_filter( 'determine_current_user', [ $this, 'resolve_user_through_a_url' ], 15 ); + $this->require_user_resolution(); + + $evaluate->disable_snippet_execution( true ); + + $this->assertIsString( $this->resolved_url ); + $this->assertStringNotContainsString( 'snippets-safe-mode', $this->resolved_url ); + } + + /** + * Safe mode still reaches the URLs an administrator follows. + * + * @return void + */ + public function test_query_var_is_added_for_a_user_with_the_capability(): void { + $_REQUEST['snippets-safe-mode'] = '1'; + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $evaluate = new Evaluate_Functions( code_snippets()->db ); + + $this->assertStringContainsString( + 'snippets-safe-mode=1', + $evaluate->add_safe_mode_query_var( 'https://example.org/wp-admin/' ) + ); + } + + /** + * Safe mode stays out of the URLs a visitor without the capability follows. + * + * @return void + */ + public function test_query_var_is_not_added_for_a_user_without_the_capability(): void { + $_REQUEST['snippets-safe-mode'] = '1'; + wp_set_current_user( 0 ); + + $evaluate = new Evaluate_Functions( code_snippets()->db ); + + $this->assertSame( + 'https://example.org/', + $evaluate->add_safe_mode_query_var( 'https://example.org/' ) + ); + } } From f86b8b1120f0d8e08ab23dc8a152f75ef44d2b17 Mon Sep 17 00:00:00 2001 From: lightbulbman Date: Mon, 19 Oct 2026 16:38:04 +0100 Subject: [PATCH 3/4] fix: carry safe mode on admin links only, without checking the user for each URL --- src/php/Integration/Evaluate_Functions.php | 44 +++-------------- .../Evaluate_Functions_Safe_Mode_Test.php | 49 ++++++------------- 2 files changed, 23 insertions(+), 70 deletions(-) diff --git a/src/php/Integration/Evaluate_Functions.php b/src/php/Integration/Evaluate_Functions.php index 0dbd858be..dd01920c7 100644 --- a/src/php/Integration/Evaluate_Functions.php +++ b/src/php/Integration/Evaluate_Functions.php @@ -39,10 +39,9 @@ public function __construct( DB $db ) { // plugins are still being included, which is before WordPress loads // pluggable.php, so a capability check at this point would call an // undefined wp_get_current_user() and take the whole request down. - // The capability is checked in the callback instead, which never runs - // before URLs are being generated. + // Site URLs are left alone: callers append paths to them, REST URLs + // among them, and a query var in the middle breaks the result. if ( $this->is_safe_mode_query_var_set() ) { - add_filter( 'home_url', [ $this, 'add_safe_mode_query_var' ] ); add_filter( 'admin_url', [ $this, 'add_safe_mode_query_var' ] ); } } @@ -68,28 +67,7 @@ public function is_safe_mode_query_var_set(): bool { * @return bool */ public function is_safe_mode_requested(): bool { - static $resolving = false; - - if ( ! $this->is_safe_mode_query_var_set() ) { - return false; - } - - // Asking for a capability makes WordPress resolve the current user, - // which runs third-party callbacks on determine_current_user. One that - // builds a URL arrives back here before that resolution has finished, - // and answering it again would restart it, recursing until the request - // ran out of memory. No user is known yet at that point, so it is no. - if ( $resolving ) { - return false; - } - - $resolving = true; - - try { - return code_snippets()->current_user_can(); - } finally { - $resolving = false; - } + return $this->is_safe_mode_query_var_set() && code_snippets()->current_user_can(); } /** @@ -102,17 +80,11 @@ public function is_safe_mode_requested(): bool { public function add_safe_mode_query_var( $url ): string { $url = is_string( $url ) ? $url : ''; - // A URL built before WordPress has settled on a user belongs to that - // resolution rather than to anything a visitor follows, so it is left - // alone. Safe mode links are generated while a page renders, which is - // long after this point. - if ( ! did_action( 'set_current_user' ) ) { - return $url; - } - - return $this->is_safe_mode_requested() ? - add_query_arg( 'snippets-safe-mode', true, $url ) : - $url; + // No capability is checked here: doing so resolves the current user, + // and a plugin that builds a URL while that is under way would arrive + // back here and restart it. Snippets are only skipped for a user with + // the capability, which is checked where they are evaluated. + return add_query_arg( 'snippets-safe-mode', true, $url ); } /** diff --git a/tests/unit/Integration/Evaluate_Functions_Safe_Mode_Test.php b/tests/unit/Integration/Evaluate_Functions_Safe_Mode_Test.php index 8535cba57..00b6927f1 100644 --- a/tests/unit/Integration/Evaluate_Functions_Safe_Mode_Test.php +++ b/tests/unit/Integration/Evaluate_Functions_Safe_Mode_Test.php @@ -71,6 +71,7 @@ public function tear_down() { * * WooCommerce does this: its REST authentication runs on determine_current_user * and calls home_url() to work out whether the request is for one of its routes. + * An admin URL is built as well, as that is the one safe mode still filters. * * @param int|false $user_id Resolved user, from an unknown earlier callback. * @@ -81,6 +82,7 @@ public function resolve_user_through_a_url( $user_id ) { if ( $this->resolution_count < self::RESOLUTION_LIMIT ) { $this->resolved_url = home_url( '/' ); + admin_url( '/' ); } return false === $this->user_to_resolve ? $user_id : $this->user_to_resolve; @@ -184,37 +186,33 @@ public function test_url_filter_does_not_restart_user_resolution(): void { } /** - * A URL built during resolution comes back untouched. - * - * The user the request will turn out to belong to is not known while that - * URL is being built, so safe mode cannot yet be part of it, even when the - * user being resolved does hold the capability. + * Site URLs are left alone, as callers append paths to them and a query var + * in the middle breaks the result. * * @return void */ - public function test_url_built_during_user_resolution_is_unchanged(): void { + public function test_site_urls_do_not_carry_the_query_var(): void { $_REQUEST['snippets-safe-mode'] = '1'; - $this->user_to_resolve = $this->factory()->user->create( [ 'role' => 'administrator' ] ); + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); $evaluate = new Evaluate_Functions( code_snippets()->db ); - add_filter( 'determine_current_user', [ $this, 'resolve_user_through_a_url' ], 15 ); - $this->require_user_resolution(); - - $evaluate->disable_snippet_execution( true ); - - $this->assertIsString( $this->resolved_url ); - $this->assertStringNotContainsString( 'snippets-safe-mode', $this->resolved_url ); + $this->assertFalse( has_filter( 'home_url', [ $evaluate, 'add_safe_mode_query_var' ] ) ); + $this->assertStringNotContainsString( 'snippets-safe-mode', home_url( '/' ) ); + $this->assertStringNotContainsString( 'snippets-safe-mode', rest_url( 'code-snippets/v1/snippets' ) ); } /** - * Safe mode still reaches the URLs an administrator follows. + * Safe mode is carried on to the admin URLs of a request that asked for it. + * + * Whether snippets are then skipped depends on the capability, which is + * checked where they are evaluated rather than for every URL built. * * @return void */ - public function test_query_var_is_added_for_a_user_with_the_capability(): void { + public function test_query_var_is_added_to_admin_urls(): void { $_REQUEST['snippets-safe-mode'] = '1'; - wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); + wp_set_current_user( 0 ); $evaluate = new Evaluate_Functions( code_snippets()->db ); @@ -223,21 +221,4 @@ public function test_query_var_is_added_for_a_user_with_the_capability(): void { $evaluate->add_safe_mode_query_var( 'https://example.org/wp-admin/' ) ); } - - /** - * Safe mode stays out of the URLs a visitor without the capability follows. - * - * @return void - */ - public function test_query_var_is_not_added_for_a_user_without_the_capability(): void { - $_REQUEST['snippets-safe-mode'] = '1'; - wp_set_current_user( 0 ); - - $evaluate = new Evaluate_Functions( code_snippets()->db ); - - $this->assertSame( - 'https://example.org/', - $evaluate->add_safe_mode_query_var( 'https://example.org/' ) - ); - } } From cad6397f269a3c05382ec70f5d037a3817c18608 Mon Sep 17 00:00:00 2001 From: lightbulbman Date: Mon, 19 Oct 2026 16:38:08 +0100 Subject: [PATCH 4/4] docs: describe the safe mode URL change in the changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92b1352cb..9abe7d4ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,7 +74,8 @@ snippet running under file-based execution. * Fixed the snippet editor reporting a save as failed and undelivered when no response was received, which it cannot determine. -* Fixed safe mode failing on sites running a plugin that builds URLs while WordPress is identifying the current user. +* Fixed safe mode failing on sites running a plugin that builds URLs while WordPress is identifying the current user, + and breaking REST API URLs. Safe mode is now carried on admin links only. ## [3.10.2] (2026-09-01)