From 0cd773a5d3d07b1299660c22a3f65456b6aa28d0 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 17 May 2026 18:33:23 +0100 Subject: [PATCH] CSP Headers: Review of #6071 - Removed extra non-needed docs in repo - Tweaked some wording. - Added extra test scenarios. - Added options to phpunit default env. - Added auto-quote-handling for unsafe-inline CSS rule. For #6033 --- .env.example.complete | 5 +++-- app/Config/app.php | 3 ++- app/Util/CspService.php | 10 ++++++++- dev/docs/development.md | 42 ------------------------------------ phpunit.xml | 2 ++ readme.md | 1 - tests/SecurityHeaderTest.php | 35 ++++++++++++++++++++++++++++-- 7 files changed, 49 insertions(+), 49 deletions(-) diff --git a/.env.example.complete b/.env.example.complete index 6a7f9db65..ed0cac5f4 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -402,9 +402,10 @@ ALLOWED_IFRAME_SOURCES="https://*.draw.io https://*.youtube.com https://*.youtub ALLOWED_CSS_SOURCES=null # A list of sources/hostnames that can be loaded as image content within BookStack. -# Space separated if multiple. BookStack host domain is auto-inferred. +# Space separated if multiple. BookStack host domain is auto-inferred, in addition to +# data and blob images, due to their use for various functionality. # Defaults to a permissive set if not provided. -# Example: ALLOWED_IMAGE_SOURCES="https://images.example.com data:" +# Example: ALLOWED_IMAGE_SOURCES="https://images.example.com" ALLOWED_IMAGE_SOURCES=null # A list of the sources/hostnames that can be reached by application SSR calls. diff --git a/app/Config/app.php b/app/Config/app.php index 5536e9abd..12f285ea9 100644 --- a/app/Config/app.php +++ b/app/Config/app.php @@ -78,7 +78,8 @@ return [ 'css_sources' => env('ALLOWED_CSS_SOURCES', null), // A list of sources/hostnames that can be loaded as image content within BookStack. - // Space separated if multiple. BookStack host domain is auto-inferred. + // Space separated if multiple. BookStack host domain is auto-inferred, in addition to + // data and blob images, due to their use for various functionality. // If not set, a permissive default set is used to reduce potential breakage. 'image_sources' => env('ALLOWED_IMAGE_SOURCES', null), diff --git a/app/Util/CspService.php b/app/Util/CspService.php index a0e1faadf..68c544ff0 100644 --- a/app/Util/CspService.php +++ b/app/Util/CspService.php @@ -175,6 +175,14 @@ class CspService $sources = array_filter(explode(' ', $configured)); array_unshift($sources, "'self'"); + // Ensure 'unsafe-inline' is quoted if present + // This is done as attempting to pass this in env values with quotes can either + // be awkward or cause issues. + $unsafeInlineIndex = array_search('unsafe-inline', $sources, true); + if ($unsafeInlineIndex !== false) { + $sources[$unsafeInlineIndex] = "'unsafe-inline'"; + } + return array_values(array_unique($sources)); } @@ -195,7 +203,7 @@ class CspService if (is_string($configured)) { $sources = array_filter(explode(' ', $configured)); - array_unshift($sources, "'self'"); + array_unshift($sources, "'self'", 'blob:', 'data:'); return array_values(array_unique($sources)); } diff --git a/dev/docs/development.md b/dev/docs/development.md index 16f168f9c..2c73a0256 100644 --- a/dev/docs/development.md +++ b/dev/docs/development.md @@ -31,48 +31,6 @@ BookStack has a large suite of PHP tests to cover application functionality. We For details about setting-up, running and writing tests please see the [php-testing.md document](php-testing.md). -## Content Security Policy Controls - -BookStack enforces a Content Security Policy (CSP) response header to reduce risk from injected content and untrusted embeds. - -For backward compatibility, image and CSS controls are intentionally permissive by default, but can be tightened via environment options. - -### Related Environment Options - -These values are defined in `.env.example.complete`: - -- `ALLOWED_CSS_SOURCES` - - Controls allowed `style-src` sources. - - Defaults to a permissive fallback if unset. -- `ALLOWED_IMAGE_SOURCES` - - Controls allowed `img-src` sources. - - Defaults to a permissive fallback if unset. - -Values should be space-separated source expressions. - -### Example Configurations - -Allow Google Fonts CSS and local styles only: - -```bash -ALLOWED_CSS_SOURCES="https://fonts.googleapis.com" -``` - -Allow local images, embedded data images, and a dedicated image CDN: - -```bash -ALLOWED_IMAGE_SOURCES="data: https://images.example.com" -``` - -### Tightening Guidance - -When hardening a deployment: - -1. Start with defaults to avoid unexpected breakage. -2. Set explicit `ALLOWED_CSS_SOURCES` and `ALLOWED_IMAGE_SOURCES` values for the domains you actually use. -3. Test key workflows (editor, page display, theme assets, external embeds) and browser console CSP warnings. -4. Remove unnecessary protocols and hosts over time. - ## Code Standards We use tools to manage code standards and formatting within the project. If submitting a PR, formatting as per our project standards would help for clarity but don't worry too much about using/understanding these tools as we can always address issues at a later stage when they're picked up by our automated tools. diff --git a/phpunit.xml b/phpunit.xml index 94fc002b7..52c5e7de8 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -18,6 +18,8 @@ + + diff --git a/readme.md b/readme.md index e231ad9f0..e9e5d197b 100644 --- a/readme.md +++ b/readme.md @@ -100,7 +100,6 @@ Big thanks to these companies for supporting the project. ## 🛠️ Development & Testing Please see our [development docs](dev/docs/development.md) for full details regarding work on the BookStack source code. -For details on Content Security Policy controls (including image and CSS source options), see the **Content Security Policy Controls** section in the [development docs](dev/docs/development.md). If you're just looking to customize or extend your own BookStack instance, take a look at our [Hacking BookStack documentation page](https://www.bookstackapp.com/docs/admin/hacking-bookstack/) for details on various options to achieve this without altering the BookStack source code. diff --git a/tests/SecurityHeaderTest.php b/tests/SecurityHeaderTest.php index 126a85f23..8badc39b9 100644 --- a/tests/SecurityHeaderTest.php +++ b/tests/SecurityHeaderTest.php @@ -153,6 +153,7 @@ class SecurityHeaderTest extends TestCase public function test_style_src_csp_header_set_to_permissive_defaults_when_not_configured() { + config()->set('app.css_sources', null); $resp = $this->get('/'); $header = $this->getCspHeader($resp, 'style-src'); @@ -169,8 +170,29 @@ class SecurityHeaderTest extends TestCase $this->assertEquals("style-src 'self' https://fonts.example.com", $header); } + public function test_style_src_csp_header_unsafe_inline_value_will_be_auto_quoted() + { + config()->set('app.css_sources', 'unsafe-inline https://css.example.com'); + + $resp = $this->get('/'); + $header = $this->getCspHeader($resp, 'style-src'); + + $this->assertEquals("style-src 'self' 'unsafe-inline' https://css.example.com", $header); + } + + public function test_style_src_can_be_blank_to_set_no_additions() + { + config()->set('app.css_sources', ''); + + $resp = $this->get('/'); + $header = $this->getCspHeader($resp, 'style-src'); + + $this->assertEquals("style-src 'self'", $header); + } + public function test_img_src_csp_header_set_to_permissive_defaults_when_not_configured() { + config()->set('app.image_sources', null); $resp = $this->get('/'); $header = $this->getCspHeader($resp, 'img-src'); @@ -179,12 +201,21 @@ class SecurityHeaderTest extends TestCase public function test_img_src_csp_header_can_be_overridden_by_config() { - config()->set('app.image_sources', 'https://images.example.com data:'); + config()->set('app.image_sources', 'https://images.example.com'); $resp = $this->get('/'); $header = $this->getCspHeader($resp, 'img-src'); - $this->assertEquals("img-src 'self' https://images.example.com data:", $header); + $this->assertEquals("img-src 'self' blob: data: https://images.example.com", $header); + } + + public function test_img_src_can_be_blank_to_set_no_additions() + { + config()->set('app.image_sources', ''); + $resp = $this->get('/'); + $header = $this->getCspHeader($resp, 'img-src'); + + $this->assertEquals("img-src 'self' blob: data:", $header); } public function test_cache_control_headers_are_set_on_responses()