From b54585e5b98cbe1020cf0e286944ef77a9643ddd Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Mon, 26 Oct 2026 14:16:33 +0800 Subject: [PATCH] fix: apply `ContentSecurityPolicy::reportOnly()` to config sources after construction --- system/HTTP/ContentSecurityPolicy.php | 129 ++++++++--------- .../system/HTTP/ContentSecurityPolicyTest.php | 133 ++++++++++++++++++ user_guide_src/source/changelogs/v4.7.6.rst | 1 + 3 files changed, 196 insertions(+), 67 deletions(-) diff --git a/system/HTTP/ContentSecurityPolicy.php b/system/HTTP/ContentSecurityPolicy.php index b668a606e19a..1c40ada588cc 100644 --- a/system/HTTP/ContentSecurityPolicy.php +++ b/system/HTTP/ContentSecurityPolicy.php @@ -66,7 +66,7 @@ class ContentSecurityPolicy /** * The `base-uri` directive restricts the URLs that can be used to specify the document base URL. * - * @var array|string|null + * @var array|string|null */ protected $baseURI = []; @@ -74,35 +74,35 @@ class ContentSecurityPolicy * The `child-src` directive governs the creation of nested browsing contexts as well * as Worker execution contexts. * - * @var array|string + * @var array|string */ protected $childSrc = []; /** * The `connect-src` directive restricts which URLs the protected resource can load using script interfaces. * - * @var array|string + * @var array|string */ protected $connectSrc = []; /** * The `default-src` directive sets a default source list for a number of directives. * - * @var array|string|null + * @var array|string|null */ protected $defaultSrc = []; /** * The `font-src` directive restricts from where the protected resource can load fonts. * - * @var array|string + * @var array|string */ protected $fontSrc = []; /** * The `form-action` directive restricts which URLs can be used as the action of HTML form elements. * - * @var array|string + * @var array|string */ protected $formAction = []; @@ -111,21 +111,21 @@ class ContentSecurityPolicy * the resource using a `frame`, `iframe`, `object`, `embed` or `applet` element, * or equivalent functionality in non-HTML resources. * - * @var array|string + * @var array|string */ protected $frameAncestors = []; /** * The `frame-src` directive restricts the URLs which may be loaded into child navigables. * - * @var array|string + * @var array|string */ protected $frameSrc = []; /** * The `img-src` directive restricts from where the protected resource can load images. * - * @var array|string + * @var array|string */ protected $imageSrc = []; @@ -133,14 +133,14 @@ class ContentSecurityPolicy * The `media-src` directive restricts from where the protected resource can load video, * audio, and associated text tracks. * - * @var array|string + * @var array|string */ protected $mediaSrc = []; /** * The `object-src` directive restricts from where the protected resource can load plugins. * - * @var array|string + * @var array|string */ protected $objectSrc = []; @@ -148,28 +148,28 @@ class ContentSecurityPolicy * The `plugin-types` directive restricts the set of plugins that can be invoked by the * protected resource by limiting the types of resources that can be embedded. * - * @var array|string + * @var array|string */ protected $pluginTypes = []; /** * The `script-src` directive restricts which scripts the protected resource can execute. * - * @var array|string + * @var array|string */ protected $scriptSrc = []; /** * The `style-src` directive restricts which styles the user may applies to the protected resource. * - * @var array|string + * @var array|string */ protected $styleSrc = []; /** * The `sandbox` directive specifies an HTML sandbox policy that the user agent applies to the protected resource. * - * @var array|string + * @var array|string */ protected $sandbox = []; @@ -193,14 +193,14 @@ class ContentSecurityPolicy /** * The `manifest-src` directive restricts the URLs from which application manifests may be loaded. * - * @var array|string + * @var array|string */ protected $manifestSrc = []; /** * The `script-src-elem` directive applies to all script requests and script blocks. * - * @var array|string + * @var array|string */ protected array|string $scriptSrcElem = []; @@ -208,7 +208,7 @@ class ContentSecurityPolicy * The `script-src-attr` directive applies to event handlers and, if present, * it will override the `script-src` directive for relevant checks. * - * @var array|string + * @var array|string */ protected array|string $scriptSrcAttr = []; @@ -216,14 +216,14 @@ class ContentSecurityPolicy * The `style-src-elem` directive governs the behaviour of styles except * for styles defined in inline attributes. * - * @var array|string + * @var array|string */ protected array|string $styleSrcElem = []; /** * The `style-src-attr` directive governs the behaviour of style attributes. * - * @var array|string + * @var array|string */ protected array|string $styleSrcAttr = []; @@ -231,7 +231,7 @@ class ContentSecurityPolicy * The `worker-src` directive restricts the URLs which may be loaded as a `Worker`, * `SharedWorker`, or `ServiceWorker`. * - * @var array|string + * @var array|string */ protected array|string $workerSrc = []; @@ -365,8 +365,8 @@ public function __construct(ContentSecurityPolicyConfig $config) && array_is_list($value) ) { // Config sets these directives as `list|string` - // but we need them as `array` internally. - $this->{$setting} = array_combine($value, array_fill(0, count($value), $this->reportOnly)); + // but we need them as `array` internally. + $this->{$setting} = array_fill_keys($value, null); continue; } @@ -375,11 +375,11 @@ public function __construct(ContentSecurityPolicyConfig $config) } if (! is_array($this->styleSrc)) { - $this->styleSrc = [$this->styleSrc => $this->reportOnly]; + $this->styleSrc = [$this->styleSrc => null]; } if (! is_array($this->scriptSrc)) { - $this->scriptSrc = [$this->scriptSrc => $this->reportOnly]; + $this->scriptSrc = [$this->scriptSrc => null]; } } @@ -885,6 +885,28 @@ public function addReportingEndpoints(array $endpoint): static return $this; } + public function clearDirective(string $directive): void + { + if (! array_key_exists($directive, $this->directives)) { + return; + } + + if ($directive === 'report-uri') { + $this->reportURI = null; + + return; + } + + if ($directive === 'report-to') { + $this->reportURI = null; + $this->reportTo = null; + + return; + } + + $this->{$this->directives[$directive]} = []; + } + /** * DRY method to add an string or array to a class property. * @@ -896,7 +918,7 @@ protected function addOption($options, string $target, ?bool $explicitReporting { // Ensure we have an array to work with... if (is_string($this->{$target})) { - $this->{$target} = $this->stringToOptions($this->{$target}); + $this->{$target} = $this->parseSourceList($this->{$target}); } elseif (! is_array($this->{$target})) { $this->{$target} = []; } @@ -908,28 +930,6 @@ protected function addOption($options, string $target, ?bool $explicitReporting } } - /** - * Converts a string directive value into an array of options. - * - * @return array - */ - private function stringToOptions(string $values): array - { - $valueStr = trim($values); - - if ($valueStr === '') { - return ['' => $this->reportOnly]; - } - - $options = []; - - foreach (preg_split('/\s+/', $valueStr) as $val) { - $options[$val] = $this->reportOnly; - } - - return $options; - } - /** * Scans the body of the request message and replaces any nonce * placeholders with actual nonces, that we'll then add to our @@ -1047,8 +1047,9 @@ protected function buildHeaders(ResponseInterface $response) * Adds a directive and its options to the appropriate header. The $values * array might have options that are geared toward either the regular or the * reportOnly header, since it's viable to have both simultaneously. + * An option flagged `null` follows the current `reportOnly()` setting. * - * @param array|string $values + * @param array|string $values * * @return void */ @@ -1059,7 +1060,7 @@ protected function addToHeader(string $name, $values = null) } if (is_string($values)) { - $values = $this->stringToOptions($values); + $values = $this->parseSourceList($values); } $sources = []; @@ -1078,7 +1079,7 @@ protected function addToHeader(string $name, $values = null) $value = "'{$value}'"; } - if ($reportOnly) { + if ($reportOnly ?? $this->reportOnly) { $reportSources[] = $value; } else { $sources[] = $value; @@ -1102,25 +1103,19 @@ protected function addToHeader(string $name, $values = null) } } - public function clearDirective(string $directive): void + /** + * Converts a string directive value into an array of options. + * + * @return array + */ + private function parseSourceList(string $values): array { - if (! array_key_exists($directive, $this->directives)) { - return; - } - - if ($directive === 'report-uri') { - $this->reportURI = null; - - return; - } - - if ($directive === 'report-to') { - $this->reportURI = null; - $this->reportTo = null; + $valueStr = trim($values); - return; + if ($valueStr === '') { + return ['' => null]; } - $this->{$this->directives[$directive]} = []; + return array_fill_keys(preg_split('/\s+/', $valueStr), null); } } diff --git a/tests/system/HTTP/ContentSecurityPolicyTest.php b/tests/system/HTTP/ContentSecurityPolicyTest.php index 08ee98d0ffe9..75a5cceac55b 100644 --- a/tests/system/HTTP/ContentSecurityPolicyTest.php +++ b/tests/system/HTTP/ContentSecurityPolicyTest.php @@ -114,6 +114,139 @@ public function testReportOnly(): void $this->assertHeaderNotEmitted('Reporting-Endpoints:'); } + public function testReportOnlyAfterConstructionAppliesToConfigSources(): void + { + $config = new CSPConfig(); + $config->reportOnly = false; + $config->scriptSrc = ['self', 'cdn.example.com']; + $config->styleSrc = 'self'; + $config->formAction = 'self'; + + $csp = new ContentSecurityPolicy($config); + $csp->reportOnly(true); + + $response = new Response(new App()); + $response->pretend(true); + $response->setBody('Hello'); + $csp->finalize($response); + + $this->assertFalse($response->hasHeader('Content-Security-Policy')); + + $directives = $this->getCspDirectives($response->getHeaderLine('Content-Security-Policy-Report-Only')); + $this->assertContains("script-src 'self' cdn.example.com", $directives); + $this->assertContains("style-src 'self'", $directives); + $this->assertContains("form-action 'self'", $directives); + } + + public function testReportOnlyDisabledAfterConstructionEnforcesConfigSources(): void + { + $config = new CSPConfig(); + $config->reportOnly = true; + $config->scriptSrc = ['self', 'cdn.example.com']; + + $csp = new ContentSecurityPolicy($config); + $csp->reportOnly(false); + + $response = new Response(new App()); + $response->pretend(true); + $response->setBody('Hello'); + $csp->finalize($response); + + $this->assertFalse($response->hasHeader('Content-Security-Policy-Report-Only')); + $this->assertContains( + "script-src 'self' cdn.example.com", + $this->getCspDirectives($response->getHeaderLine('Content-Security-Policy')), + ); + } + + public function testReportOnlyFromControllerSendsOnlyReportOnlyHeader(): void + { + $config = new CSPConfig(); + $config->reportOnly = false; + $config->upgradeInsecureRequests = true; + $config->defaultSrc = null; + $config->scriptSrc = ['self', 'assets.example.com', 'cdn.example.com']; + $config->scriptSrcElem = ['self', 'assets.example.com', 'cdn.example.com']; + $config->scriptSrcAttr = 'self'; + $config->styleSrc = ['self', 'assets.example.com']; + $config->styleSrcElem = ['self', 'assets.example.com']; + $config->styleSrcAttr = 'self'; + $config->imageSrc = ['self', 'data:', 'images.example.com']; + $config->baseURI = null; + $config->childSrc = ['self']; + $config->connectSrc = ['self', 'assets.example.com']; + $config->fontSrc = ['self', 'assets.example.com']; + $config->formAction = 'self'; + $config->frameAncestors = 'self'; + $config->frameSrc = ['self', 'player.example.com']; + $config->mediaSrc = null; + $config->objectSrc = 'self'; + $config->manifestSrc = null; + $config->workerSrc = []; + $config->pluginTypes = null; + $config->sandbox = null; + $config->autoNonce = false; + + Services::injectMock('csp', new ContentSecurityPolicy($config)); + + $response = new Response(config(App::class)); + $response->pretend(true); + $response->getCSP()->reportOnly(true); + $response->setBody('Hello'); + + ob_start(); + $response->send(); + ob_end_clean(); + + $this->assertFalse($response->hasHeader('Content-Security-Policy')); + $this->assertSame( + [ + "base-uri 'self'", + "child-src 'self'", + "connect-src 'self' assets.example.com", + "default-src 'self'", + "font-src 'self' assets.example.com", + "form-action 'self'", + "frame-ancestors 'self'", + "frame-src 'self' player.example.com", + "img-src 'self' data: images.example.com", + "object-src 'self'", + "script-src 'self' assets.example.com cdn.example.com", + "style-src 'self' assets.example.com", + "script-src-elem 'self' assets.example.com cdn.example.com", + "script-src-attr 'self'", + "style-src-elem 'self' assets.example.com", + "style-src-attr 'self'", + ], + $this->getCspDirectives($response->getHeaderLine('Content-Security-Policy-Report-Only')), + ); + } + + public function testReportOnlyKeepsFlagOfSourcesAddedBeforeIt(): void + { + $config = new CSPConfig(); + $config->reportOnly = false; + $config->scriptSrc = ['self']; + + $csp = new ContentSecurityPolicy($config); + $csp->addScriptSrc('cdn.example.com'); + $csp->reportOnly(true); + + $response = new Response(new App()); + $response->pretend(true); + $response->setBody('Hello'); + $csp->finalize($response); + + $this->assertContains( + 'script-src cdn.example.com', + $this->getCspDirectives($response->getHeaderLine('Content-Security-Policy')), + ); + $this->assertContains( + "script-src 'self'", + $this->getCspDirectives($response->getHeaderLine('Content-Security-Policy-Report-Only')), + ); + } + #[PreserveGlobalState(false)] #[RunInSeparateProcess] public function testExistingCSPHeaderStringIsPreserved(): void diff --git a/user_guide_src/source/changelogs/v4.7.6.rst b/user_guide_src/source/changelogs/v4.7.6.rst index 4c36d122395f..3725f6a7f2cd 100644 --- a/user_guide_src/source/changelogs/v4.7.6.rst +++ b/user_guide_src/source/changelogs/v4.7.6.rst @@ -39,6 +39,7 @@ Bugs Fixed - **Format:** Fixed ``XMLFormatter`` so that an array key that becomes empty after sanitization no longer triggers a ``ValueError``; the fallback tag name ``item`` is used instead. Values that cannot be converted to a string, such as objects not implementing ``Stringable``, now throw a ``FormatException`` instead of a fatal ``Error``. - **Helpers:** Fixed a regression in v4.7.5 where ``set_value()`` threw a ``TypeError`` when the default value was not a string or an array, such as an integer, a float, ``null``, or a ``Stringable`` object. Such values are now converted to a string. - **HTTP:** Fixed a bug in ``ContentSecurityPolicy`` where a numeric source such as ``'0'`` threw a ``TypeError``, and a space-separated directive string in ``Config\\ContentSecurityPolicy`` was emitted as one unquoted source. +- **HTTP:** Fixed a regression in v4.7.0 where calling ``ContentSecurityPolicy::reportOnly()`` after the policy was created did not apply to array sources from ``Config\\ContentSecurityPolicy``, so they stayed in the enforced ``Content-Security-Policy`` header alongside the ``Content-Security-Policy-Report-Only`` header. - **HTTP:** Fixed a bug in ``ContentSecurityPolicy::setDefaultSrc()`` where passing an array of URIs resulted in an invalid ``'Array'`` directive. - **HTTP:** Fixed a bug where ``IncomingRequest`` did not read the request body when PHP's ``post_max_size`` was set to ``0`` or ``-1`` (unlimited). - **HTTP:** Fixed a bug where ``IncomingRequest::isSecure()`` did not recognize ``https`` in the ``X-Forwarded-Proto`` header from a trusted proxy when the value contained uppercase letters or surrounding whitespace, or was represented as a single-element array.