From c47c6a73c903965c88de5e8f137f6741945c2d78 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Tue, 25 Aug 2026 11:04:05 +0100 Subject: [PATCH 1/4] Ensure share_errors replaces CSRFs --- .../Concerns/RendersHttpExceptions.php | 20 +++++- src/StaticCaching/Middleware/Cache.php | 6 +- .../SharedErrorsStaticCachingTest.php | 67 +++++++++++++++++++ 3 files changed, 88 insertions(+), 5 deletions(-) diff --git a/src/Exceptions/Concerns/RendersHttpExceptions.php b/src/Exceptions/Concerns/RendersHttpExceptions.php index b983af0777d..c76d1d1e175 100644 --- a/src/Exceptions/Concerns/RendersHttpExceptions.php +++ b/src/Exceptions/Concerns/RendersHttpExceptions.php @@ -13,6 +13,7 @@ use Statamic\Statamic; use Statamic\StaticCaching\Cacher; use Statamic\StaticCaching\Cachers\ApplicationCacher; +use Statamic\StaticCaching\Replacer; use Statamic\View\View; trait RendersHttpExceptions @@ -96,9 +97,22 @@ private function getCachedError(): ?Response $request = Request::createFrom(request())->fakeStaticCacheStatus($status); - return $cacher->hasCachedPage($request) - ? $cacher->getCachedPage($request)->toResponse($request) - : null; + if (! $cacher->hasCachedPage($request)) { + return null; + } + + $response = $cacher->getCachedPage($request)->toResponse($request); + + $this->applyReplacers($response); + + return $response; + } + + private function applyReplacers(Response $response): void + { + collect(config('statamic.static_caching.replacers')) + ->map(fn ($class) => app($class)) + ->each(fn (Replacer $replacer) => $replacer->replaceInCachedResponse($response)); } public static function renderUsing(Closure $callback): void diff --git a/src/StaticCaching/Middleware/Cache.php b/src/StaticCaching/Middleware/Cache.php index 85816471a82..34eddef590d 100644 --- a/src/StaticCaching/Middleware/Cache.php +++ b/src/StaticCaching/Middleware/Cache.php @@ -88,9 +88,9 @@ private function handleRequest($request, Closure $next) $response = $next($request); if ($this->shouldBeCached($request, $response)) { - $this->copyError($request, $response); + $preparedResponse = $this->makeReplacementsAndCacheResponse($request, $response); - $this->makeReplacementsAndCacheResponse($request, $response); + $this->copyError($request, $preparedResponse); $this->nocache->write(); @@ -169,6 +169,8 @@ private function makeReplacementsAndCacheResponse($request, $response) } $this->cacher->cachePage($request, $cachedResponse); + + return $cachedResponse; } private function makeReplacements($response) diff --git a/tests/StaticCaching/SharedErrorsStaticCachingTest.php b/tests/StaticCaching/SharedErrorsStaticCachingTest.php index 71a5daaae27..03ad17a8bca 100644 --- a/tests/StaticCaching/SharedErrorsStaticCachingTest.php +++ b/tests/StaticCaching/SharedErrorsStaticCachingTest.php @@ -9,8 +9,19 @@ use Statamic\StaticCaching\Cachers\ApplicationCacher; use Tests\TestCase; + +use Illuminate\Support\Facades\Cache; +use Orchestra\Testbench\Attributes\DefineEnvironment; +use Statamic\StaticCaching\Replacer; +use Symfony\Component\HttpFoundation\Response; +use Tests\FakesViews; +use Tests\PreventSavingStacheItemsToDisk; + class SharedErrorsStaticCachingTest extends TestCase { + use FakesViews; + use PreventSavingStacheItemsToDisk; + private ApplicationCacher $cacher; protected function setUp(): void @@ -75,4 +86,60 @@ private function shareError(int $status, string $content): void // the framework fires during the real response lifecycle. event(new ResponsePrepared($request, $response)); } + + public function withTestReplacer($app) + { + $app['config']->set('statamic.static_caching.strategy', 'half'); + $app['config']->set('statamic.static_caching.share_errors', true); + $app['config']->set('statamic.static_caching.replacers', array_merge( + $app['config']->get('statamic.static_caching.replacers'), + ['test' => SharedErrorTestReplacer::class] + )); + } + + #[Test] + #[DefineEnvironment('withTestReplacer')] + public function replacers_run_when_serving_a_shared_error() + { + Cache::flush(); + + $this->withStandardFakeViews(); + $this->viewShouldReturnRaw('errors.layout', '{{ template_content }}'); + $this->viewShouldReturnRaw('errors.404', '404 LIVE_VALUE'); + + // First 404 renders live and seeds the shared cache. The live response + // itself is untouched by the prepare step - only the clone that gets + // cached is. + $this->get('/this-does-not-exist') + ->assertNotFound() + ->assertSee('404 LIVE_VALUE', false); + + // A different URL that also 404s is served from the shared cache. + // Regression: previously this replayed whatever copyError() captured + // before any replacer ran, and getCachedError() never ran replacers on + // the way out either - so a stale value (or, for CsrfTokenReplacer in + // production, a frozen CSRF token from a different session) leaked to + // every subsequent visitor forever. + $this->get('/this-also-does-not-exist') + ->assertNotFound() + ->assertSee('404 REPLACED_ON_SERVE', false); + } +} + +class SharedErrorTestReplacer implements Replacer +{ + const PLACEHOLDER = 'TEST_TOKEN_PLACEHOLDER'; + + public function prepareResponseToCache(Response $response, Response $initial) + { + // Mirrors CsrfTokenReplacer's behaviour for the "half" strategy: + // only the clone that gets cached is touched, not $initial (which + // is what's returned to this first request). + $response->setContent(str_replace('LIVE_VALUE', self::PLACEHOLDER, $response->getContent())); + } + + public function replaceInCachedResponse(Response $response) + { + $response->setContent(str_replace(self::PLACEHOLDER, 'REPLACED_ON_SERVE', $response->getContent())); + } } From cf1bfd1c539e1ec91663cc6195622cf1da1f8626 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Tue, 25 Aug 2026 11:06:14 +0100 Subject: [PATCH 2/4] :beer: --- tests/StaticCaching/SharedErrorsStaticCachingTest.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/StaticCaching/SharedErrorsStaticCachingTest.php b/tests/StaticCaching/SharedErrorsStaticCachingTest.php index 03ad17a8bca..6657587e7bd 100644 --- a/tests/StaticCaching/SharedErrorsStaticCachingTest.php +++ b/tests/StaticCaching/SharedErrorsStaticCachingTest.php @@ -4,18 +4,16 @@ use Illuminate\Http\Request; use Illuminate\Routing\Events\ResponsePrepared; +use Illuminate\Support\Facades\Cache; +use Orchestra\Testbench\Attributes\DefineEnvironment; use PHPUnit\Framework\Attributes\Test; use Statamic\Facades\Site; use Statamic\StaticCaching\Cachers\ApplicationCacher; -use Tests\TestCase; - - -use Illuminate\Support\Facades\Cache; -use Orchestra\Testbench\Attributes\DefineEnvironment; use Statamic\StaticCaching\Replacer; use Symfony\Component\HttpFoundation\Response; use Tests\FakesViews; use Tests\PreventSavingStacheItemsToDisk; +use Tests\TestCase; class SharedErrorsStaticCachingTest extends TestCase { From bde70d5848f904e45e2c6e1ff1f626a4476ee1d8 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Wed, 26 Aug 2026 16:42:16 +0100 Subject: [PATCH 3/4] review fixes --- .../Concerns/RendersHttpExceptions.php | 14 +-- src/StaticCaching/Middleware/Cache.php | 14 ++- .../SharedErrorsStaticCachingTest.php | 85 +++++++++++++++++++ 3 files changed, 99 insertions(+), 14 deletions(-) diff --git a/src/Exceptions/Concerns/RendersHttpExceptions.php b/src/Exceptions/Concerns/RendersHttpExceptions.php index c76d1d1e175..f9776b58efe 100644 --- a/src/Exceptions/Concerns/RendersHttpExceptions.php +++ b/src/Exceptions/Concerns/RendersHttpExceptions.php @@ -13,7 +13,6 @@ use Statamic\Statamic; use Statamic\StaticCaching\Cacher; use Statamic\StaticCaching\Cachers\ApplicationCacher; -use Statamic\StaticCaching\Replacer; use Statamic\View\View; trait RendersHttpExceptions @@ -101,18 +100,7 @@ private function getCachedError(): ?Response return null; } - $response = $cacher->getCachedPage($request)->toResponse($request); - - $this->applyReplacers($response); - - return $response; - } - - private function applyReplacers(Response $response): void - { - collect(config('statamic.static_caching.replacers')) - ->map(fn ($class) => app($class)) - ->each(fn (Replacer $replacer) => $replacer->replaceInCachedResponse($response)); + return $cacher->getCachedPage($request)->toResponse($request); } public static function renderUsing(Closure $callback): void diff --git a/src/StaticCaching/Middleware/Cache.php b/src/StaticCaching/Middleware/Cache.php index 34eddef590d..e9d4ac5ff8f 100644 --- a/src/StaticCaching/Middleware/Cache.php +++ b/src/StaticCaching/Middleware/Cache.php @@ -90,6 +90,18 @@ private function handleRequest($request, Closure $next) if ($this->shouldBeCached($request, $response)) { $preparedResponse = $this->makeReplacementsAndCacheResponse($request, $response); + // The clone above is what gets cached, and keeps any replacer placeholders + // (e.g. nocache regions, CSRF tokens) intact for future requests to expand + // per-visitor. Under the ApplicationCacher, prepareResponseToCache() leaves + // those placeholders in the live response untouched too (it's the FileCacher + // that needs them left in place, for client-side JS to resolve), so they need + // expanding here - normally a no-op, except when this content came from a + // shared error cache (see RendersHttpExceptions::getCachedError), whose + // placeholders were never expanded on the way out. + if ($this->cacher instanceof ApplicationCacher) { + $this->makeReplacements($response); + } + $this->copyError($request, $preparedResponse); $this->nocache->write(); @@ -160,7 +172,7 @@ private function attemptToGetCachedResponse($request) } } - private function makeReplacementsAndCacheResponse($request, $response) + private function makeReplacementsAndCacheResponse($request, $response): Response { $cachedResponse = clone $response; diff --git a/tests/StaticCaching/SharedErrorsStaticCachingTest.php b/tests/StaticCaching/SharedErrorsStaticCachingTest.php index 6657587e7bd..04863dada47 100644 --- a/tests/StaticCaching/SharedErrorsStaticCachingTest.php +++ b/tests/StaticCaching/SharedErrorsStaticCachingTest.php @@ -122,6 +122,91 @@ public function replacers_run_when_serving_a_shared_error() ->assertNotFound() ->assertSee('404 REPLACED_ON_SERVE', false); } + + public function shareErrorsWithHalfMeasure($app) + { + $app['config']->set('statamic.static_caching.strategy', 'half'); + $app['config']->set('statamic.static_caching.share_errors', true); + } + + #[Test] + #[DefineEnvironment('shareErrorsWithHalfMeasure')] + public function each_session_gets_its_own_csrf_token_on_a_shared_error() + { + Cache::flush(); + + $this->withStandardFakeViews(); + $this->viewShouldReturnRaw('errors.layout', '{{ template_content }}'); + $this->viewShouldReturnRaw('errors.404', '404 {{ csrf_token }}'); + + // First 404 renders live and seeds the shared cache. + $this->withSession(['_token' => 'session-one-token']) + ->get('/nope-one') + ->assertNotFound() + ->assertSee('404 session-one-token', false); + + // A different URL that also 404s is served from the shared cache, but + // for a different session. It must get its own token, not the one + // frozen into the shared cache by the first visitor. + $this->withSession(['_token' => 'session-two-token']) + ->get('/nope-two') + ->assertNotFound() + ->assertSee('404 session-two-token', false) + ->assertDontSee('session-one-token', false); + + // A repeat hit on that second URL, now cached under its own URL, + // must still resolve session two's current token, not a frozen one. + $this->withSession(['_token' => 'session-two-token']) + ->get('/nope-two') + ->assertNotFound() + ->assertSee('404 session-two-token', false); + } + + #[Test] + #[DefineEnvironment('shareErrorsWithHalfMeasure')] + public function nocache_region_inside_a_shared_error_stays_dynamic_across_repeat_hits() + { + Cache::flush(); + + // Use a tag that outputs something dynamic. It just increments by + // one every time it's rendered. + app()->instance('example_count', 0); + + (new class extends \Statamic\Tags\Tags + { + public static $handle = 'example_count'; + + public function index() + { + $count = app('example_count'); + $count++; + app()->instance('example_count', $count); + + return $count; + } + })::register(); + + $this->withStandardFakeViews(); + $this->viewShouldReturnRaw('errors.layout', '{{ template_content }}'); + $this->viewShouldReturnRaw('errors.404', '404 {{ nocache }}{{ example_count }}{{ /nocache }}'); + + // First 404 renders live and seeds the shared cache. + $this->get('/nope-one')->assertNotFound()->assertSee('404 1', false); + + // A different URL that also 404s is served from the shared cache. + // Its nocache region must render fresh, not replay whatever the + // shared cache captured when it was seeded. + $this->get('/nope-two')->assertNotFound()->assertSee('404 2', false); + + // Repeat hits to that second URL, now cached under its own URL, must + // keep rendering the region fresh too. Before the fix, the first + // request served from the shared cache baked the region's rendered + // output into the per-URL cache entry, so every later hit on that + // URL replayed the same frozen value forever - the same class of + // cross-visitor leak this PR fixes for CSRF tokens. + $this->get('/nope-two')->assertNotFound()->assertSee('404 3', false); + $this->get('/nope-two')->assertNotFound()->assertSee('404 4', false); + } } class SharedErrorTestReplacer implements Replacer From e01823f9fb415bfaa43d836944abe499f941b14e Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Wed, 26 Aug 2026 17:00:46 +0100 Subject: [PATCH 4/4] SymfonyResponse --- src/StaticCaching/Middleware/Cache.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/StaticCaching/Middleware/Cache.php b/src/StaticCaching/Middleware/Cache.php index e9d4ac5ff8f..61afdd05501 100644 --- a/src/StaticCaching/Middleware/Cache.php +++ b/src/StaticCaching/Middleware/Cache.php @@ -22,6 +22,7 @@ use Statamic\StaticCaching\NoCache\Session; use Statamic\StaticCaching\Replacer; use Statamic\StaticCaching\ResponseStatus; +use Symfony\Component\HttpFoundation\Response as SymfonyResponse; use function Statamic\trans as __; @@ -172,7 +173,7 @@ private function attemptToGetCachedResponse($request) } } - private function makeReplacementsAndCacheResponse($request, $response): Response + private function makeReplacementsAndCacheResponse($request, $response): SymfonyResponse { $cachedResponse = clone $response;