From 0d5e54bc94748fbff4ff0c0debaefa69e03a3e10 Mon Sep 17 00:00:00 2001 From: Craig Weber Date: Fri, 20 Dec 2024 14:26:13 -0500 Subject: [PATCH 1/2] fix: support CSP configuration as sets Fixes several exceptions that'd be thrown when trying to use Python sets as config values, rather than tuples or lists. --- csp/tests/test_middleware.py | 12 ++++++++++++ csp/tests/test_utils.py | 24 ++++++++++++++++++++++++ csp/utils.py | 14 ++++++++------ 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/csp/tests/test_middleware.py b/csp/tests/test_middleware.py index e2e7e54..d3011cf 100644 --- a/csp/tests/test_middleware.py +++ b/csp/tests/test_middleware.py @@ -33,6 +33,18 @@ def test_both_headers() -> None: assert HEADER_REPORT_ONLY in response +@override_settings( + CONTENT_SECURITY_POLICY={"DIRECTIVES": {"default-src": {"example.com"}}}, + CONTENT_SECURITY_POLICY_REPORT_ONLY={"DIRECTIVES": {"default-src": {SELF}}}, +) +def test_directives_configured_as_sets() -> None: + request = rf.get("/") + response = HttpResponse() + mw.process_response(request, response) + assert HEADER in response + assert HEADER_REPORT_ONLY in response + + def test_exempt() -> None: request = rf.get("/") response = HttpResponse() diff --git a/csp/tests/test_utils.py b/csp/tests/test_utils.py index 74d5c4f..b0e8ebd 100644 --- a/csp/tests/test_utils.py +++ b/csp/tests/test_utils.py @@ -55,6 +55,11 @@ def test_default_src() -> None: policy = build_policy() policy_eq("default-src example.com example2.com", policy) +@override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {"default-src": {"example.com", "example2.com"}}}) +def test_default_src_is_set() -> None: + policy = build_policy() + policy_eq("default-src example.com example2.com", policy) + @override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {"script-src": ["example.com"]}}) def test_script_src() -> None: @@ -212,6 +217,25 @@ def test_replace_string() -> None: policy_eq("default-src 'self'; img-src example2.com", policy) +@override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {"img-src": ("example.com",)}}) +def test_update_set() -> None: + """ + GitHub issue #40 - given project settings as a tuple, and + an update/replace with a string, concatenate correctly. + """ + policy = build_policy(update={"img-src": {"example2.com"}}) + policy_eq("default-src 'self'; img-src example.com example2.com", policy) + + +@override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {"img-src": ("example.com",)}}) +def test_replace_set() -> None: + """ + Demonstrate that GitHub issue #40 doesn't affect replacements + """ + policy = build_policy(replace={"img-src": {"example2.com"}}) + policy_eq("default-src 'self'; img-src example2.com", policy) + + @override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {"form-action": ["example.com"]}}) def test_form_action() -> None: policy = build_policy() diff --git a/csp/utils.py b/csp/utils.py index 2b4ec19..449caf5 100644 --- a/csp/utils.py +++ b/csp/utils.py @@ -98,13 +98,13 @@ def build_policy( v = config[k] if v is not None: v = copy.copy(v) - if not isinstance(v, (list, tuple)): + if not isinstance(v, (list, tuple, set)): v = (v,) csp[k] = v for k, v in update.items(): if v is not None: - if not isinstance(v, (list, tuple)): + if not isinstance(v, (list, tuple, set)): v = (v,) if csp.get(k) is None: csp[k] = v @@ -117,10 +117,12 @@ def build_policy( for key, value in csp.items(): # Check for boolean directives. - if len(value) == 1 and isinstance(value[0], bool): - if value[0] is True: - policy_parts[key] = "" - continue + if len(value) == 1: + val = list(value)[0] + if isinstance(val, bool): + if value[0] is True: + policy_parts[key] = "" + continue if NONCE in value: if nonce: value = [f"'nonce-{nonce}'" if v == NONCE else v for v in value] From 55e3ea0668b8ef63a4b8feb6fb8850d0a78eff71 Mon Sep 17 00:00:00 2001 From: Craig Weber Date: Tue, 14 Jan 2025 18:34:51 -0500 Subject: [PATCH 2/2] fix: remove duplicate values from CSP directives --- csp/tests/test_templatetags.py | 2 +- csp/tests/test_utils.py | 30 ++++++++++++++++++++++++++++++ csp/utils.py | 20 ++++++++++++-------- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/csp/tests/test_templatetags.py b/csp/tests/test_templatetags.py index 308175d..4aa79e8 100644 --- a/csp/tests/test_templatetags.py +++ b/csp/tests/test_templatetags.py @@ -39,7 +39,7 @@ def test_async_attribute_with_falsey(self) -> None: {% script src="foo.com/bar.js" async=False %} {% endscript %}""" - expected = '" + expected = '' self.assert_template_eq(*self.process_templates(tpl, expected)) diff --git a/csp/tests/test_utils.py b/csp/tests/test_utils.py index b0e8ebd..03c6888 100644 --- a/csp/tests/test_utils.py +++ b/csp/tests/test_utils.py @@ -55,6 +55,7 @@ def test_default_src() -> None: policy = build_policy() policy_eq("default-src example.com example2.com", policy) + @override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {"default-src": {"example.com", "example2.com"}}}) def test_default_src_is_set() -> None: policy = build_policy() @@ -337,6 +338,35 @@ def test_only_nonce_in_value() -> None: policy_eq("default-src 'nonce-abc123'", policy) +@override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {"img-src": ["example.com", "example.com"]}}) +def test_deduplicate_values() -> None: + """ + GitHub issue #40 - given project settings as a tuple, and + an update/replace with a string, concatenate correctly. + """ + policy = build_policy() + policy_eq("default-src 'self'; img-src example.com", policy) + + +@override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {"img-src": ["example.com", "example.com"]}}) +def test_deduplicate_values_update() -> None: + """ + GitHub issue #40 - given project settings as a tuple, and + an update/replace with a string, concatenate correctly. + """ + policy = build_policy(update={"img-src": "example.com"}) + policy_eq("default-src 'self'; img-src example.com", policy) + + +@override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {"img-src": ("example.com",)}}) +def test_deduplicate_values_replace() -> None: + """ + Demonstrate that GitHub issue #40 doesn't affect replacements + """ + policy = build_policy(replace={"img-src": ["example2.com", "example2.com"]}) + policy_eq("default-src 'self'; img-src example2.com", policy) + + def test_boolean_directives() -> None: for directive in ["upgrade-insecure-requests", "block-all-mixed-content"]: with override_settings(CONTENT_SECURITY_POLICY={"DIRECTIVES": {directive: True}}): diff --git a/csp/utils.py b/csp/utils.py index 449caf5..e42bfbe 100644 --- a/csp/utils.py +++ b/csp/utils.py @@ -98,13 +98,18 @@ def build_policy( v = config[k] if v is not None: v = copy.copy(v) - if not isinstance(v, (list, tuple, set)): + if isinstance(v, set): + v = sorted(v) + if not isinstance(v, (list, tuple)): v = (v,) csp[k] = v for k, v in update.items(): if v is not None: - if not isinstance(v, (list, tuple, set)): + v = copy.copy(v) + if isinstance(v, set): + v = sorted(v) + if not isinstance(v, (list, tuple)): v = (v,) if csp.get(k) is None: csp[k] = v @@ -117,12 +122,10 @@ def build_policy( for key, value in csp.items(): # Check for boolean directives. - if len(value) == 1: - val = list(value)[0] - if isinstance(val, bool): - if value[0] is True: - policy_parts[key] = "" - continue + if len(value) == 1 and isinstance(value[0], bool): + if value[0] is True: + policy_parts[key] = "" + continue if NONCE in value: if nonce: value = [f"'nonce-{nonce}'" if v == NONCE else v for v in value] @@ -130,6 +133,7 @@ def build_policy( # Strip the `NONCE` sentinel value if no nonce is provided. value = [v for v in value if v != NONCE] + value = list(dict.fromkeys(value)) # Deduplicate policy_parts[key] = " ".join(value) if report_uri: