diff --git a/app/Config/app.php b/app/Config/app.php index acd27e98c..7aa94b4f2 100644 --- a/app/Config/app.php +++ b/app/Config/app.php @@ -42,17 +42,17 @@ return [ // Even when overridden the WYSIWYG editor may still escape script content. 'allow_content_scripts' => env('ALLOW_CONTENT_SCRIPTS', false), - // Control the behaviour of page content filtering. + // Control the behaviour of content filtering, primarily used for page content. // This setting is a collection of characters which represent different available filters: - // - j - Filter out JavaScript based content - // - h - Filter out unexpected, potentially dangerous, HTML elements + // - j - Filter out JavaScript and unknown binary data based content + // - h - Filter out unexpected, and potentially dangerous, HTML elements // - f - Filter out unexpected form elements // - a - Run content through a more complex allow-list filter // This defaults to using all filters, unless ALLOW_CONTENT_SCRIPTS is set to true in which case no filters are used. // Note: These filters are a best attempt, and may not be 100% effective. They are typically a layer used in addition to other security measures. // TODO - Add to example env // TODO - Remove allow_content_scripts option above - 'content_filtering' => env('CONTENT_FILTERING', env('ALLOW_CONTENT_SCRIPTS', false) === true ? '' : 'jfha'), + 'content_filtering' => env('APP_CONTENT_FILTERING', env('ALLOW_CONTENT_SCRIPTS', false) === true ? '' : 'jhfa'), // Allow server-side fetches to be performed to potentially unknown // and user-provided locations. Primarily used in exports when loading diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index f8a061739..4f72e7c49 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -341,7 +341,8 @@ class PageContent $contentId = $this->page->id; $contentTime = $this->page->updated_at?->timestamp ?? time(); $appVersion = AppVersion::get(); - return "page-content-cache::{$appVersion}::{$contentId}::{$contentTime}::{$contentHash}"; + $filterConfig = config('app.content_filtering') ?? ''; + return "page-content-cache::{$filterConfig}::{$appVersion}::{$contentId}::{$contentTime}::{$contentHash}"; } /** diff --git a/phpunit.xml b/phpunit.xml index 8a7ab9cb7..94fc002b7 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -34,6 +34,7 @@ + diff --git a/tests/Entity/PageContentFilteringTest.php b/tests/Entity/PageContentFilteringTest.php index e1295034d..8103fae1d 100644 --- a/tests/Entity/PageContentFilteringTest.php +++ b/tests/Entity/PageContentFilteringTest.php @@ -22,6 +22,8 @@ class PageContentFilteringTest extends TestCase public function test_more_complex_content_script_escaping_scenarios() { + config()->set('app.content_filtering', 'j'); + $checks = [ "

Some script

", "

Some script

", @@ -47,6 +49,8 @@ class PageContentFilteringTest extends TestCase public function test_js_and_base64_src_urls_are_removed() { + config()->set('app.content_filtering', 'j'); + $checks = [ '', '', @@ -89,6 +93,8 @@ class PageContentFilteringTest extends TestCase public function test_javascript_uri_links_are_removed() { + config()->set('app.content_filtering', 'j'); + $checks = [ ''; + $page->save(); + + $this->asEditor()->get($page->getUrl())->assertSee('dont-see-this', false); + + config()->set('app.content_filtering', 'f'); + $this->get($page->getUrl())->assertDontSee('dont-see-this', false); + } + public function test_form_actions_with_javascript_are_removed() { + config()->set('app.content_filtering', 'j'); + $checks = [ '', 'Click me', @@ -139,6 +160,8 @@ class PageContentFilteringTest extends TestCase public function test_form_elements_are_removed() { + config()->set('app.content_filtering', 'f'); + $checks = [ '

thisisacattofind

thisdogshouldnotbefound
', '

thisisacattofind

', @@ -182,6 +205,8 @@ TESTCASE public function test_form_attributes_are_removed() { + config()->set('app.content_filtering', 'f'); + $withinSvgSample = <<<'TESTCASE' @@ -229,6 +254,8 @@ TESTCASE; public function test_metadata_redirects_are_removed() { + config()->set('app.content_filtering', 'h'); + $checks = [ '', '', @@ -253,6 +280,8 @@ TESTCASE; public function test_page_inline_on_attributes_removed_by_default() { + config()->set('app.content_filtering', 'j'); + $this->asEditor(); $page = $this->entities->page(); $script = '

Hello

'; @@ -267,6 +296,8 @@ TESTCASE; public function test_more_complex_inline_on_attributes_escaping_scenarios() { + config()->set('app.content_filtering', 'j'); + $checks = [ '

Hello

', '

Hello

', @@ -308,6 +339,8 @@ TESTCASE; public function test_svg_script_usage_is_removed() { + config()->set('app.content_filtering', 'j'); + $checks = [ '', '', @@ -350,4 +383,46 @@ TESTCASE; $pageView->assertSee($script, false); $pageView->assertDontSee('

Hello

', false); } + + public function test_non_content_filtering_is_controlled_by_config() + { + config()->set('app.content_filtering', 'h'); + $page = $this->entities->page(); + $html = <<<'HTML' + +

inbetweenpsection

+ + +superbeans! + +HTML; + + $page->html = $html; + $page->save(); + + $resp = $this->asEditor()->get($page->getUrl()); + $resp->assertDontSee('superbeans', false); + $resp->assertSee('inbetweenpsection', false); + } + + public function test_non_content_filtering() + { + config()->set('app.content_filtering', 'h'); + } + + public function test_allow_list_filtering_is_controlled_by_config() + { + config()->set('app.content_filtering', ''); + $page = $this->entities->page(); + $page->html = '
Hello!
'; + $page->save(); + + $resp = $this->asEditor()->get($page->getUrl()); + $resp->assertSee('style="position: absolute; left: 0;color:#00FFEE;"', false); + + config()->set('app.content_filtering', 'a'); + $resp = $this->get($page->getUrl()); + $resp->assertDontSee('style="position: absolute; left: 0;color:#00FFEE;"', false); + $resp->assertSee('style="color:#00FFEE;"', false); + } } diff --git a/tests/Unit/ConfigTest.php b/tests/Unit/ConfigTest.php index 7795a861a..63dd04fb1 100644 --- a/tests/Unit/ConfigTest.php +++ b/tests/Unit/ConfigTest.php @@ -170,6 +170,20 @@ class ConfigTest extends TestCase } } + public function test_content_filtering_defaults_to_enabled() + { + $this->runWithEnv(['APP_CONTENT_FILTERING' => null, 'ALLOW_CONTENT_SCRIPTS' => null], function () { + $this->assertEquals('jhfa', config('app.content_filtering')); + }); + } + + public function test_allow_content_scripts_disables_content_filtering() + { + $this->runWithEnv(['APP_CONTENT_FILTERING' => null, 'ALLOW_CONTENT_SCRIPTS' => 'true'], function () { + $this->assertEquals('', config('app.content_filtering')); + }); + } + /** * Set an environment variable of the given name and value * then check the given config key to see if it matches the given result.