Skip to content

fix: Hardcoded CSP Nonce Tags in ResponseTrait #9937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
patel-vansh wants to merge 10 commits into
base: develop
Choose a base branch
from
+131 −5
Open

fix: Hardcoded CSP Nonce Tags in ResponseTrait #9937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
19380aa
Created a method to clear nonce placeholders.
patel-vansh Feb 8, 2026
489d201
Added tests
patel-vansh Feb 8, 2026
466930a
cs-fix
patel-vansh Feb 8, 2026
ca867f7
Fix static analysis errors
patel-vansh Feb 8, 2026
e42537f
Fixed `Argument #1 ($text) must be of type string, null given` error
patel-vansh Feb 8, 2026
65b1a04
Apply code suggestions
patel-vansh Feb 10, 2026
ea893a9
Fix the failing test case
patel-vansh Feb 10, 2026
77d5eed
Merge branch 'develop' into fix/hardcoded-csp-tags
patel-vansh Feb 11, 2026
030f4a0
Remove unused function
patel-vansh Feb 11, 2026
db3d0bd
Updated docs
patel-vansh Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions system/HTTP/ContentSecurityPolicy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,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 . '"';

Expand All @@ -923,6 +927,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', []);
Expand Down
6 changes: 1 addition & 5 deletions system/HTTP/ResponseTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

@paulbalandan paulbalandan Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, if CSP is disabled and auto-nonce is also disabled, then replacement with '' would not happen. Can you add a test where $CSPEnabled is false and $autoNonce is also false where the response body will still be correctly replaced with empty strings on the tags?


$this->sendHeaders();
$this->sendCookies();
Expand Down
121 changes: 121 additions & 0 deletions tests/system/HTTP/ResponseTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use DateTimeZone;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Group;
use ReflectionClass;

/**
* @internal
Expand Down Expand Up @@ -577,4 +578,124 @@ 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 = '<html><script {csp-script-nonce}>console.log("test")</script><style {csp-style-nonce}>.test{}</style></html>';
$response->setBody($body);

ob_start();
$response->send();
$actual = ob_get_contents();
ob_end_clean();
Comment on lines +595 to +596
Copy link
Member

@paulbalandan paulbalandan Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do ob_get_clean() instead?


// Nonce placeholders should be removed when CSP is disabled
$this->assertStringNotContainsString('{csp-script-nonce}', (string) $actual);
Copy link
Member

@paulbalandan paulbalandan Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an assertIsString() test for the $actual so you don't need the string cast here down.

$this->assertStringNotContainsString('{csp-style-nonce}', (string) $actual);
$this->assertStringContainsString('<script >console.log("test")</script>', (string) $actual);
$this->assertStringContainsString('<style >.test{}</style>', (string) $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}';

$response = new Response($appConfig);
$response->pretend(true);

// Inject the custom CSP config
$reflection = new ReflectionClass($response);
$cspProperty = $reflection->getProperty('CSP');
$cspProperty->setValue($response, new ContentSecurityPolicy($cspConfig));

$body = '<html><script {custom-script-tag}>test()</script><style {custom-style-tag}>.x{}</style></html>';
$response->setBody($body);

ob_start();
$response->send();
$actual = ob_get_contents();
ob_end_clean();
Comment on lines +628 to +629
Copy link
Member

@paulbalandan paulbalandan Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ob_get_clean() as shortcut?


// Custom nonce placeholders should be removed when CSP is disabled
$this->assertStringNotContainsString('{custom-script-tag}', (string) $actual);
Copy link
Member

@paulbalandan paulbalandan Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add assertIsString to remove the string cast.

$this->assertStringNotContainsString('{custom-style-tag}', (string) $actual);
$this->assertStringContainsString('<script >test()</script>', (string) $actual);
$this->assertStringContainsString('<style >.x{}</style>', (string) $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_contents();
ob_end_clean();

$this->assertSame('', (string) $actual);
Comment on lines +651 to +654
Copy link
Member

@paulbalandan paulbalandan Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ob_get_clean and add assertIsString.

}

public function testSendNoEffectWithNoPlaceholdersAndCSPDisabled(): void
{
$config = new App();
$config->CSPEnabled = false;

$response = new Response($config);
$response->pretend(true);

$body = '<html><head><title>Test</title></head><body><p>No placeholders here</p></body></html>';
$response->setBody($body);

ob_start();
$response->send();
$actual = ob_get_contents();
ob_end_clean();

// Body should be unchanged when there are no placeholders and CSP is disabled
$this->assertSame($body, (string) $actual);
Comment on lines +670 to +674
Copy link
Member

@paulbalandan paulbalandan Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ob_get_clean and add assertIsString.

}

public function testSendRemovesMultiplePlaceholdersWhenCSPDisabled(): void
{
$config = new App();
$config->CSPEnabled = false;

$response = new Response($config);
$response->pretend(true);

$body = '<html><script {csp-script-nonce}>console.log("test")</script><script {csp-script-nonce}>console.log("test2")</script><style {csp-style-nonce}>.test{}</style><style {csp-style-nonce}>.test2{}</style></html>';
$response->setBody($body);

ob_start();
$response->send();
$actual = ob_get_contents();
ob_end_clean();

// All nonce placeholders should be removed when CSP is disabled
$this->assertStringNotContainsString('{csp-script-nonce}', (string) $actual);
Comment on lines +690 to +694
Copy link
Member

@paulbalandan paulbalandan Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ob_get_clean and add assertIsString.

$this->assertStringNotContainsString('{csp-style-nonce}', (string) $actual);
$this->assertStringContainsString('<script >console.log("test")</script>', (string) $actual);
$this->assertStringContainsString('<script >console.log("test2")</script>', (string) $actual);
$this->assertStringContainsString('<style >.test{}</style>', (string) $actual);
$this->assertStringContainsString('<style >.test2{}</style>', (string) $actual);
}
}
1 change: 1 addition & 0 deletions user_guide_src/source/changelogs/v4.7.1.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Loading