diff --git a/system/HTTP/ContentSecurityPolicy.php b/system/HTTP/ContentSecurityPolicy.php index f15a85df32d9..a7c67647823b 100644 --- a/system/HTTP/ContentSecurityPolicy.php +++ b/system/HTTP/ContentSecurityPolicy.php @@ -427,9 +427,7 @@ public function getScriptNonce(): string */ public function finalize(ResponseInterface $response) { - if ($this->autoNonce) { - $this->generateNonces($response); - } + $this->generateNonces($response); $this->buildHeaders($response); } @@ -892,6 +890,10 @@ protected function addOption($options, string $target, ?bool $explicitReporting */ protected function generateNonces(ResponseInterface $response) { + if ($this->enabled() && ! $this->autoNonce) { + return; + } + $body = (string) $response->getBody(); if ($body === '') { @@ -905,6 +907,10 @@ protected function generateNonces(ResponseInterface $response) $pattern = sprintf('/(%s|%s)/', preg_quote($this->styleNonceTag, '/'), preg_quote($this->scriptNonceTag, '/')); $body = preg_replace_callback($pattern, function ($match) use ($jsonEscape): string { + if (! $this->enabled()) { + return ''; + } + $nonce = $match[0] === $this->styleNonceTag ? $this->getStyleNonce() : $this->getScriptNonce(); $attr = 'nonce="' . $nonce . '"'; @@ -923,6 +929,10 @@ protected function generateNonces(ResponseInterface $response) */ protected function buildHeaders(ResponseInterface $response) { + if (! $this->enabled()) { + return; + } + $response->setHeader('Content-Security-Policy', []); $response->setHeader('Content-Security-Policy-Report-Only', []); $response->setHeader('Reporting-Endpoints', []); diff --git a/system/HTTP/ResponseTrait.php b/system/HTTP/ResponseTrait.php index 211663bc7987..3063e44116fb 100644 --- a/system/HTTP/ResponseTrait.php +++ b/system/HTTP/ResponseTrait.php @@ -367,11 +367,7 @@ public function send() { // If we're enforcing a Content Security Policy, // we need to give it a chance to build out it's headers. - if ($this->CSP->enabled()) { - $this->CSP->finalize($this); - } else { - $this->body = str_replace(['{csp-style-nonce}', '{csp-script-nonce}'], '', $this->body ?? ''); - } + $this->CSP->finalize($this); $this->sendHeaders(); $this->sendCookies(); diff --git a/tests/system/HTTP/ResponseTest.php b/tests/system/HTTP/ResponseTest.php index 84408df0e5a5..38c349117cbb 100644 --- a/tests/system/HTTP/ResponseTest.php +++ b/tests/system/HTTP/ResponseTest.php @@ -577,4 +577,150 @@ public function testPretendOutput(): void $this->assertSame('Happy days', $actual); } + + public function testSendRemovesDefaultNoncePlaceholdersWhenCSPDisabled(): void + { + $config = new App(); + $config->CSPEnabled = false; + + $response = new Response($config); + $response->pretend(true); + + $body = ''; + $response->setBody($body); + + ob_start(); + $response->send(); + $actual = ob_get_clean(); + + // Nonce placeholders should be removed when CSP is disabled + $this->assertIsString($actual); + $this->assertStringNotContainsString('{csp-script-nonce}', $actual); + $this->assertStringNotContainsString('{csp-style-nonce}', $actual); + $this->assertStringContainsString('', $actual); + $this->assertStringContainsString('', $actual); + } + + public function testSendRemovesCustomNoncePlaceholdersWhenCSPDisabled(): void + { + $appConfig = new App(); + $appConfig->CSPEnabled = false; + + // Create custom CSP config with custom nonce tags + $cspConfig = new \Config\ContentSecurityPolicy(); + $cspConfig->scriptNonceTag = '{custom-script-tag}'; + $cspConfig->styleNonceTag = '{custom-style-tag}'; + + Services::injectMock('csp', new ContentSecurityPolicy($cspConfig)); + + $response = new Response($appConfig); + $response->pretend(true); + + $body = ''; + $response->setBody($body); + + ob_start(); + $response->send(); + $actual = ob_get_clean(); + + // Custom nonce placeholders should be removed when CSP is disabled + $this->assertIsString($actual); + $this->assertStringNotContainsString('{custom-script-tag}', $actual); + $this->assertStringNotContainsString('{custom-style-tag}', $actual); + $this->assertStringContainsString('', $actual); + $this->assertStringContainsString('', $actual); + } + + public function testSendNoEffectWhenBodyEmptyAndCSPDisabled(): void + { + $config = new App(); + $config->CSPEnabled = false; + + $response = new Response($config); + $response->pretend(true); + + $body = ''; + $response->setBody($body); + + ob_start(); + $response->send(); + $actual = ob_get_clean(); + + $this->assertIsString($actual); + $this->assertSame('', $actual); + } + + public function testSendNoEffectWithNoPlaceholdersAndCSPDisabled(): void + { + $config = new App(); + $config->CSPEnabled = false; + + $response = new Response($config); + $response->pretend(true); + + $body = 'Test

No placeholders here

'; + $response->setBody($body); + + ob_start(); + $response->send(); + $actual = ob_get_clean(); + + // Body should be unchanged when there are no placeholders and CSP is disabled + $this->assertIsString($actual); + $this->assertSame($body, $actual); + } + + public function testSendRemovesMultiplePlaceholdersWhenCSPDisabled(): void + { + $config = new App(); + $config->CSPEnabled = false; + + $response = new Response($config); + $response->pretend(true); + + $body = ''; + $response->setBody($body); + + ob_start(); + $response->send(); + $actual = ob_get_clean(); + + // All nonce placeholders should be removed when CSP is disabled + $this->assertIsString($actual); + $this->assertStringNotContainsString('{csp-script-nonce}', $actual); + $this->assertStringNotContainsString('{csp-style-nonce}', $actual); + $this->assertStringContainsString('', $actual); + $this->assertStringContainsString('', $actual); + $this->assertStringContainsString('', $actual); + $this->assertStringContainsString('', $actual); + } + + public function testSendRemovesPlaceholdersWhenBothCSPAndAutoNonceAreDisabled(): void + { + $appConfig = new App(); + $appConfig->CSPEnabled = false; + + // Create custom CSP config with custom nonce tags + $cspConfig = new \Config\ContentSecurityPolicy(); + $cspConfig->autoNonce = false; + + Services::injectMock('csp', new ContentSecurityPolicy($cspConfig)); + + $response = new Response($appConfig); + $response->pretend(true); + + $body = ''; + $response->setBody($body); + + ob_start(); + $response->send(); + $actual = ob_get_clean(); + + // Custom nonce placeholders should be removed when CSP is disabled + $this->assertIsString($actual); + $this->assertStringNotContainsString('{csp-script-nonce}', $actual); + $this->assertStringNotContainsString('{csp-style-nonce}', $actual); + $this->assertStringContainsString('', $actual); + $this->assertStringContainsString('', $actual); + } } diff --git a/user_guide_src/source/changelogs/v4.7.1.rst b/user_guide_src/source/changelogs/v4.7.1.rst index 8a94a731a413..c3b872c8611b 100644 --- a/user_guide_src/source/changelogs/v4.7.1.rst +++ b/user_guide_src/source/changelogs/v4.7.1.rst @@ -32,6 +32,7 @@ Deprecations Bugs Fixed ********** +- **ContentSecurityPolicy:** Fixed a bug where custom CSP tags were not removed from generated HTML when CSP was disabled. The method now ensures that all custom CSP tags are removed from the generated HTML. - **ContentSecurityPolicy:** Fixed a bug where ``generateNonces()`` produces corrupted JSON responses by replacing CSP nonce placeholders with unescaped double quotes. The method now automatically JSON-escapes nonce attributes when the response Content-Type is JSON. - **Session:** Fixed a bug in ``MemcachedHandler`` where the constructor incorrectly threw an exception when ``savePath`` was not empty.