From 76d63c03ce4c48d2f551d04834c15e0daa93a406 Mon Sep 17 00:00:00 2001 From: Younes ENNAJI Date: Mon, 2 Mar 2026 03:33:16 +0000 Subject: [PATCH] Reset CSP handler in worker listeners and add related tests --- CHANGELOG.md | 3 + src/Laravel/EventListener/OctaneListener.php | 6 ++ src/Symfony/EventListener/WorkerListener.php | 7 ++ src/Symfony/Resources/config/services.php | 1 + .../Csp/ContentSecurityPolicyHandlerTest.php | 65 +++++++++++++ tests/Prime/Http/ResponseExtensionTest.php | 91 +++++++++++++++++++ .../EventListener/WorkerListenerTest.php | 27 +++++- 7 files changed, 197 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0e4cce2..8e67085d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ * fix [Flasher] Fix FilterCriteria uninitialized property error when constructed with empty array * fix [Flasher] Fix null comparison issues in PriorityCriteria, HopsCriteria, and DelayCriteria that relied on PHP's implicit null-to-0 coercion * fix [Flasher] Add type validation for callable factory return values in NotificationFactoryLocator with descriptive error messages +* fix [Flasher] Add error handling for invalid regex patterns in ResponseExtension::isPathExcluded() +* fix [Flasher] Fix ContentSecurityPolicyHandler parsing of CSP headers with trailing semicolons creating empty directive keys +* fix [Flasher] Add reset() method to ContentSecurityPolicyHandler to fix CSP state leak in long-running processes (Octane, FrankenPHP) ## [v2.1.3](https://github.com/php-flasher/php-flasher/compare/v2.1.2...v2.1.3) - 2025-01-25 diff --git a/src/Laravel/EventListener/OctaneListener.php b/src/Laravel/EventListener/OctaneListener.php index 08992e92..ae7f118b 100644 --- a/src/Laravel/EventListener/OctaneListener.php +++ b/src/Laravel/EventListener/OctaneListener.php @@ -6,6 +6,7 @@ namespace Flasher\Laravel\EventListener; use Flasher\Laravel\Storage\FallbackSession; use Flasher\Prime\EventDispatcher\EventListener\NotificationLoggerListener; +use Flasher\Prime\Http\Csp\ContentSecurityPolicyHandlerInterface; use Laravel\Octane\Events\RequestReceived; final readonly class OctaneListener @@ -17,6 +18,11 @@ final readonly class OctaneListener $listener = $event->sandbox->make('flasher.notification_logger_listener'); $listener->reset(); + // Reset the CSP handler to re-enable CSP for new requests + /** @var ContentSecurityPolicyHandlerInterface $cspHandler */ + $cspHandler = $event->sandbox->make('flasher.csp_handler'); + $cspHandler->reset(); + // Reset the fallback session static storage to prevent notification leakage // when session is not started (e.g., during API requests) FallbackSession::reset(); diff --git a/src/Symfony/EventListener/WorkerListener.php b/src/Symfony/EventListener/WorkerListener.php index e980582f..75771f28 100644 --- a/src/Symfony/EventListener/WorkerListener.php +++ b/src/Symfony/EventListener/WorkerListener.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Flasher\Symfony\EventListener; +use Flasher\Prime\Http\Csp\ContentSecurityPolicyHandlerInterface; use Flasher\Symfony\Storage\FallbackSession; use Symfony\Contracts\Service\ResetInterface; @@ -16,8 +17,14 @@ use Symfony\Contracts\Service\ResetInterface; */ final class WorkerListener implements ResetInterface { + public function __construct( + private readonly ContentSecurityPolicyHandlerInterface $cspHandler, + ) { + } + public function reset(): void { FallbackSession::reset(); + $this->cspHandler->reset(); } } diff --git a/src/Symfony/Resources/config/services.php b/src/Symfony/Resources/config/services.php index a653456a..339ad43b 100644 --- a/src/Symfony/Resources/config/services.php +++ b/src/Symfony/Resources/config/services.php @@ -80,6 +80,7 @@ return static function (ContainerConfigurator $container): void { ->tag('kernel.reset', ['method' => 'reset']) ->set('flasher.worker_listener', WorkerListener::class) + ->args([service('flasher.csp_handler')]) ->tag('kernel.reset', ['method' => 'reset']) ->set('flasher.translation_listener', TranslationListener::class) diff --git a/tests/Prime/Http/Csp/ContentSecurityPolicyHandlerTest.php b/tests/Prime/Http/Csp/ContentSecurityPolicyHandlerTest.php index c2653cb5..8cacf9a3 100644 --- a/tests/Prime/Http/Csp/ContentSecurityPolicyHandlerTest.php +++ b/tests/Prime/Http/Csp/ContentSecurityPolicyHandlerTest.php @@ -454,4 +454,69 @@ final class ContentSecurityPolicyHandlerTest extends TestCase 'csp_style_nonce' => 'noresponsenonce', ], $nonces); } + + public function testResetRestoresCspEnabled(): void + { + // First, disable CSP + $this->cspHandler->disableCsp(); + + // Verify it's disabled by checking updateResponseHeaders returns empty + $removedHeaders = []; + $this->responseMock->method('removeHeader')->willReturnCallback(function ($headerName) use (&$removedHeaders) { + $removedHeaders[] = $headerName; + }); + + $nonces = $this->cspHandler->updateResponseHeaders($this->requestMock, $this->responseMock); + $this->assertSame([], $nonces, 'CSP should be disabled'); + + // Reset the handler + $this->cspHandler->reset(); + + // Now CSP should be enabled again + $this->nonceGeneratorMock->method('generate')->willReturn('resetnonce'); + + $setHeaders = []; + $this->responseMock->method('setHeader')->willReturnCallback(function ($headerName, $value) use (&$setHeaders) { + $setHeaders[$headerName] = $value; + }); + + $nonces = $this->cspHandler->updateResponseHeaders($this->requestMock, $this->responseMock); + + $this->assertNotEmpty($nonces, 'CSP should be enabled after reset'); + $this->assertArrayHasKey('csp_script_nonce', $nonces); + $this->assertArrayHasKey('csp_style_nonce', $nonces); + } + + public function testParseDirectivesWithTrailingSemicolons(): void + { + $this->nonceGeneratorMock->method('generate')->willReturn('testnonce'); + + $this->requestMock->method('hasHeader')->willReturn(false); + + $headers = []; + $this->responseMock->method('hasHeader')->willReturnCallback(function ($name) use (&$headers) { + return isset($headers[$name]); + }); + $this->responseMock->method('getHeader')->willReturnCallback(function ($name) use (&$headers) { + return $headers[$name] ?? null; + }); + $this->responseMock->method('setHeader')->willReturnCallback(function ($name, $value) use (&$headers) { + $headers[$name] = $value; + }); + $this->responseMock->method('removeHeader')->willReturnCallback(function ($name) use (&$headers) { + unset($headers[$name]); + }); + + // CSP with trailing semicolons and empty directives + $headers['Content-Security-Policy'] = "script-src 'self'; ; style-src 'self'; "; + + $nonces = $this->cspHandler->updateResponseHeaders($this->requestMock, $this->responseMock); + + // Should parse correctly without creating empty key entries + $this->assertNotEmpty($nonces); + + // Verify the resulting CSP header doesn't have empty directives + $resultCsp = $headers['Content-Security-Policy']; + $this->assertStringNotContainsString('; ;', $resultCsp); + } } diff --git a/tests/Prime/Http/ResponseExtensionTest.php b/tests/Prime/Http/ResponseExtensionTest.php index 5d27bea0..c4f2d692 100644 --- a/tests/Prime/Http/ResponseExtensionTest.php +++ b/tests/Prime/Http/ResponseExtensionTest.php @@ -458,4 +458,95 @@ final class ResponseExtensionTest extends TestCase $this->assertSame($response, $result); } + + public function testRenderWithInvalidRegexPatternInExcludedPaths(): void + { + $flasher = \Mockery::mock(FlasherInterface::class); + $cspHandler = \Mockery::mock(ContentSecurityPolicyHandlerInterface::class); + $response = \Mockery::mock(ResponseInterface::class); + $htmlResponse = '
Flasher
'; + + $contentBefore = 'content '.HtmlPresenter::BODY_END_PLACE_HOLDER; + + // Create a concrete request class with getUri method + $request = new class implements RequestInterface { + public function isXmlHttpRequest(): bool + { + return false; + } + + public function isHtmlRequestFormat(): bool + { + return true; + } + + public function getUri(): string + { + return '/user/profile'; + } + + public function hasSession(): bool + { + return false; + } + + public function isSessionStarted(): bool + { + return false; + } + + public function hasType(string $type): bool + { + return false; + } + + public function getType(string $type): string|array + { + return []; + } + + public function forgetType(string $type): void + { + } + + public function hasHeader(string $name): bool + { + return false; + } + + public function getHeader(string $name): ?string + { + return null; + } + }; + + $cspHandler->allows()->updateResponseHeaders($request, $response)->andReturn([]); + $flasher->allows()->render('html', [], \Mockery::any())->andReturn($htmlResponse); + + $response->allows([ + 'isSuccessful' => true, + 'isHtml' => true, + 'isRedirection' => false, + 'isAttachment' => false, + 'isJson' => false, + 'getContent' => $contentBefore, + 'setContent' => \Mockery::any(), + ]); + + // Test with invalid regex - should trigger warning but continue + $invalidPatterns = ['[invalid regex']; + + // Suppress the expected warning + $previousHandler = set_error_handler(fn () => true, \E_USER_WARNING); + + try { + $responseExtension = new ResponseExtension($flasher, $cspHandler, $invalidPatterns); + $result = $responseExtension->render($request, $response); + + // Should still render since invalid regex is skipped + $this->assertInstanceOf(ResponseInterface::class, $result); + } finally { + set_error_handler($previousHandler); + } + } } diff --git a/tests/Symfony/EventListener/WorkerListenerTest.php b/tests/Symfony/EventListener/WorkerListenerTest.php index 0b1c22b8..e0ed9cac 100644 --- a/tests/Symfony/EventListener/WorkerListenerTest.php +++ b/tests/Symfony/EventListener/WorkerListenerTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Flasher\Tests\Symfony\EventListener; +use Flasher\Prime\Http\Csp\ContentSecurityPolicyHandlerInterface; use Flasher\Symfony\EventListener\WorkerListener; use Flasher\Symfony\Storage\FallbackSession; use PHPUnit\Framework\TestCase; @@ -11,6 +12,13 @@ use Symfony\Contracts\Service\ResetInterface; final class WorkerListenerTest extends TestCase { + private ContentSecurityPolicyHandlerInterface $cspHandler; + + protected function setUp(): void + { + $this->cspHandler = $this->createMock(ContentSecurityPolicyHandlerInterface::class); + } + protected function tearDown(): void { FallbackSession::reset(); @@ -18,7 +26,7 @@ final class WorkerListenerTest extends TestCase public function testListenerImplementsResetInterface(): void { - $listener = new WorkerListener(); + $listener = new WorkerListener($this->cspHandler); $this->assertInstanceOf(ResetInterface::class, $listener); } @@ -32,8 +40,11 @@ final class WorkerListenerTest extends TestCase // Verify data is stored $this->assertSame(['test_envelope'], $fallbackSession->get('flasher::envelopes')); + // Mock CSP handler to expect reset call + $this->cspHandler->expects($this->once())->method('reset'); + // Reset via WorkerListener - $listener = new WorkerListener(); + $listener = new WorkerListener($this->cspHandler); $listener->reset(); // Verify FallbackSession was reset @@ -42,7 +53,9 @@ final class WorkerListenerTest extends TestCase public function testResetIsIdempotent(): void { - $listener = new WorkerListener(); + $this->cspHandler->expects($this->exactly(3))->method('reset'); + + $listener = new WorkerListener($this->cspHandler); // Should not throw when called multiple times $listener->reset(); @@ -51,4 +64,12 @@ final class WorkerListenerTest extends TestCase $this->assertTrue(true); } + + public function testResetCallsCspHandlerReset(): void + { + $this->cspHandler->expects($this->once())->method('reset'); + + $listener = new WorkerListener($this->cspHandler); + $listener->reset(); + } }