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

@@ -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.

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),
 

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));
         }

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.

phpunit.xml

@@ -18,6 +18,8 @@
     <server name="APP_URL" value="http://bookstack.dev"/>
     <server name="APP_TIMEZONE" value="UTC"/>
     <server name="APP_DISPLAY_TIMEZONE" value="UTC"/>
+    <server name="ALLOWED_CSS_SOURCES" value=""/>
+    <server name="ALLOWED_IMAGE_SOURCES" value=""/>
     <server name="ALLOWED_IFRAME_HOSTS" value=""/>
     <server name="ALLOWED_IFRAME_SOURCES" value="https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com"/>
     <server name="ALLOWED_SSR_HOSTS" value="*"/>

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.
 

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()