Skip to content

Commit 0cd773a

Browse files
ssddanbrownssddanbrown
committed
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
1 parent dfc91d5 commit 0cd773a

7 files changed

Lines changed: 49 additions & 49 deletions

File tree

‎.env.example.complete‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,9 +402,10 @@ ALLOWED_IFRAME_SOURCES="https://*.draw.io https://*.youtube.com https://*.youtub
402402
ALLOWED_CSS_SOURCES=null
403403

404404
# A list of sources/hostnames that can be loaded as image content within BookStack.
405-
# Space separated if multiple. BookStack host domain is auto-inferred.
405+
# Space separated if multiple. BookStack host domain is auto-inferred, in addition to
406+
# data and blob images, due to their use for various functionality.
406407
# Defaults to a permissive set if not provided.
407-
# Example: ALLOWED_IMAGE_SOURCES="https://images.example.com data:"
408+
# Example: ALLOWED_IMAGE_SOURCES="https://images.example.com"
408409
ALLOWED_IMAGE_SOURCES=null
409410

410411
# A list of the sources/hostnames that can be reached by application SSR calls.

‎app/Config/app.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@
7878
'css_sources' => env('ALLOWED_CSS_SOURCES', null),
7979

8080
// A list of sources/hostnames that can be loaded as image content within BookStack.
81-
// Space separated if multiple. BookStack host domain is auto-inferred.
81+
// Space separated if multiple. BookStack host domain is auto-inferred, in addition to
82+
// data and blob images, due to their use for various functionality.
8283
// If not set, a permissive default set is used to reduce potential breakage.
8384
'image_sources' => env('ALLOWED_IMAGE_SOURCES', null),
8485

‎app/Util/CspService.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,14 @@ protected function getAllowedStyleSources(): array
175175
$sources = array_filter(explode(' ', $configured));
176176
array_unshift($sources, "'self'");
177177

178+
// Ensure 'unsafe-inline' is quoted if present
179+
// This is done as attempting to pass this in env values with quotes can either
180+
// be awkward or cause issues.
181+
$unsafeInlineIndex = array_search('unsafe-inline', $sources, true);
182+
if ($unsafeInlineIndex !== false) {
183+
$sources[$unsafeInlineIndex] = "'unsafe-inline'";
184+
}
185+
178186
return array_values(array_unique($sources));
179187
}
180188

@@ -195,7 +203,7 @@ protected function getAllowedImageSources(): array
195203

196204
if (is_string($configured)) {
197205
$sources = array_filter(explode(' ', $configured));
198-
array_unshift($sources, "'self'");
206+
array_unshift($sources, "'self'", 'blob:', 'data:');
199207

200208
return array_values(array_unique($sources));
201209
}

‎dev/docs/development.md‎

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -31,48 +31,6 @@ BookStack has a large suite of PHP tests to cover application functionality. We
3131

3232
For details about setting-up, running and writing tests please see the [php-testing.md document](php-testing.md).
3333

34-
## Content Security Policy Controls
35-
36-
BookStack enforces a Content Security Policy (CSP) response header to reduce risk from injected content and untrusted embeds.
37-
38-
For backward compatibility, image and CSS controls are intentionally permissive by default, but can be tightened via environment options.
39-
40-
### Related Environment Options
41-
42-
These values are defined in `.env.example.complete`:
43-
44-
- `ALLOWED_CSS_SOURCES`
45-
- Controls allowed `style-src` sources.
46-
- Defaults to a permissive fallback if unset.
47-
- `ALLOWED_IMAGE_SOURCES`
48-
- Controls allowed `img-src` sources.
49-
- Defaults to a permissive fallback if unset.
50-
51-
Values should be space-separated source expressions.
52-
53-
### Example Configurations
54-
55-
Allow Google Fonts CSS and local styles only:
56-
57-
```bash
58-
ALLOWED_CSS_SOURCES="https://fonts.googleapis.com"
59-
```
60-
61-
Allow local images, embedded data images, and a dedicated image CDN:
62-
63-
```bash
64-
ALLOWED_IMAGE_SOURCES="data: https://images.example.com"
65-
```
66-
67-
### Tightening Guidance
68-
69-
When hardening a deployment:
70-
71-
1. Start with defaults to avoid unexpected breakage.
72-
2. Set explicit `ALLOWED_CSS_SOURCES` and `ALLOWED_IMAGE_SOURCES` values for the domains you actually use.
73-
3. Test key workflows (editor, page display, theme assets, external embeds) and browser console CSP warnings.
74-
4. Remove unnecessary protocols and hosts over time.
75-
7634
## Code Standards
7735

7836
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‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
<server name="APP_URL" value="http://bookstack.dev"/>
1919
<server name="APP_TIMEZONE" value="UTC"/>
2020
<server name="APP_DISPLAY_TIMEZONE" value="UTC"/>
21+
<server name="ALLOWED_CSS_SOURCES" value=""/>
22+
<server name="ALLOWED_IMAGE_SOURCES" value=""/>
2123
<server name="ALLOWED_IFRAME_HOSTS" value=""/>
2224
<server name="ALLOWED_IFRAME_SOURCES" value="https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com"/>
2325
<server name="ALLOWED_SSR_HOSTS" value="*"/>

‎readme.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ Big thanks to these companies for supporting the project.
100100
## 🛠️ Development & Testing
101101

102102
Please see our [development docs](dev/docs/development.md) for full details regarding work on the BookStack source code.
103-
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).
104103

105104
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.
106105

‎tests/SecurityHeaderTest.php‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ public function test_frame_src_csp_header_drawio_host_includes_port_if_existing(
153153

154154
public function test_style_src_csp_header_set_to_permissive_defaults_when_not_configured()
155155
{
156+
config()->set('app.css_sources', null);
156157
$resp = $this->get('/');
157158
$header = $this->getCspHeader($resp, 'style-src');
158159

@@ -169,8 +170,29 @@ public function test_style_src_csp_header_can_be_overridden_by_config()
169170
$this->assertEquals("style-src 'self' https://fonts.example.com", $header);
170171
}
171172

173+
public function test_style_src_csp_header_unsafe_inline_value_will_be_auto_quoted()
174+
{
175+
config()->set('app.css_sources', 'unsafe-inline https://css.example.com');
176+
177+
$resp = $this->get('/');
178+
$header = $this->getCspHeader($resp, 'style-src');
179+
180+
$this->assertEquals("style-src 'self' 'unsafe-inline' https://css.example.com", $header);
181+
}
182+
183+
public function test_style_src_can_be_blank_to_set_no_additions()
184+
{
185+
config()->set('app.css_sources', '');
186+
187+
$resp = $this->get('/');
188+
$header = $this->getCspHeader($resp, 'style-src');
189+
190+
$this->assertEquals("style-src 'self'", $header);
191+
}
192+
172193
public function test_img_src_csp_header_set_to_permissive_defaults_when_not_configured()
173194
{
195+
config()->set('app.image_sources', null);
174196
$resp = $this->get('/');
175197
$header = $this->getCspHeader($resp, 'img-src');
176198

@@ -179,12 +201,21 @@ public function test_img_src_csp_header_set_to_permissive_defaults_when_not_conf
179201

180202
public function test_img_src_csp_header_can_be_overridden_by_config()
181203
{
182-
config()->set('app.image_sources', 'https://images.example.com data:');
204+
config()->set('app.image_sources', 'https://images.example.com');
205+
206+
$resp = $this->get('/');
207+
$header = $this->getCspHeader($resp, 'img-src');
183208

209+
$this->assertEquals("img-src 'self' blob: data: https://images.example.com", $header);
210+
}
211+
212+
public function test_img_src_can_be_blank_to_set_no_additions()
213+
{
214+
config()->set('app.image_sources', '');
184215
$resp = $this->get('/');
185216
$header = $this->getCspHeader($resp, 'img-src');
186217

187-
$this->assertEquals("img-src 'self' https://images.example.com data:", $header);
218+
$this->assertEquals("img-src 'self' blob: data:", $header);
188219
}
189220

190221
public function test_cache_control_headers_are_set_on_responses()

0 commit comments

Comments
 (0)