From 3f8e7c17334aa4a19cb1d2b69b94a45836431c11 Mon Sep 17 00:00:00 2001
From: Kevin Veen-Birkenbach <kevin@veen.world>
Date: Mon, 1 Sep 2025 09:03:22 +0200
Subject: [PATCH] Refactor CSP filter: - Move default 'unsafe-inline' for
 style-src and style-src-elem into get_csp_flags - Ensure hashes are only
 added if 'unsafe-inline' not in final tokens - Improve comments and structure
 - Extend unit tests to cover default flags, overrides, and final-token logic
 See: https://chatgpt.com/share/68b54520-5cfc-800f-9bac-45093740df78

---
 filter_plugins/csp_filters.py                 | 127 +++++++++++-------
 tests/unit/filter_plugins/test_csp_filters.py | 100 ++++++++++++++
 2 files changed, 178 insertions(+), 49 deletions(-)

diff --git a/filter_plugins/csp_filters.py b/filter_plugins/csp_filters.py
index 74965235..a0af0180 100644
--- a/filter_plugins/csp_filters.py
+++ b/filter_plugins/csp_filters.py
@@ -1,12 +1,15 @@
 from ansible.errors import AnsibleFilterError
 import hashlib
 import base64
-import sys, os
+import sys
+import os
 
+# Ensure module_utils is importable when this filter runs from Ansible
 sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..')))
 from module_utils.config_utils import get_app_conf
 from module_utils.get_url import get_url
 
+
 class FilterModule(object):
     """
     Custom filters for Content Security Policy generation and CSP-related utilities.
@@ -17,10 +20,14 @@ class FilterModule(object):
             'build_csp_header': self.build_csp_header,
         }
 
+    # -------------------------------
+    # Helpers
+    # -------------------------------
+
     @staticmethod
     def is_feature_enabled(applications: dict, feature: str, application_id: str) -> bool:
         """
-        Return True if applications[application_id].features[feature] is truthy.
+        Returns True if applications[application_id].features[feature] is truthy.
         """
         return get_app_conf(
             applications,
@@ -32,6 +39,10 @@ class FilterModule(object):
 
     @staticmethod
     def get_csp_whitelist(applications, application_id, directive):
+        """
+        Returns a list of additional whitelist entries for a given directive.
+        Accepts both scalar and list in config; always returns a list.
+        """
         wl = get_app_conf(
             applications,
             application_id,
@@ -48,28 +59,37 @@ class FilterModule(object):
     @staticmethod
     def get_csp_flags(applications, application_id, directive):
         """
-        Dynamically extract all CSP flags for a given directive and return them as tokens,
-        e.g., "'unsafe-eval'", "'unsafe-inline'", etc.
+        Returns CSP flag tokens (e.g., "'unsafe-eval'", "'unsafe-inline'") for a directive,
+        merging sane defaults with app config.
+        Default: 'unsafe-inline' is enabled for style-src and style-src-elem.
         """
-        flags = get_app_conf(
+        # Defaults that apply to all apps
+        default_flags = {}
+        if directive in ('style-src', 'style-src-elem'):
+            default_flags = {'unsafe-inline': True}
+
+        configured = get_app_conf(
             applications,
             application_id,
             'server.csp.flags.' + directive,
             False,
             {}
         )
-        tokens = []
 
-        for flag_name, enabled in flags.items():
+        # Merge defaults with configured flags (configured overrides defaults)
+        merged = {**default_flags, **configured}
+
+        tokens = []
+        for flag_name, enabled in merged.items():
             if enabled:
                 tokens.append(f"'{flag_name}'")
-
         return tokens
 
     @staticmethod
     def get_csp_inline_content(applications, application_id, directive):
         """
-        Return inline script/style snippets to hash for a given CSP directive.
+        Returns inline script/style snippets to hash for a given directive.
+        Accepts both scalar and list in config; always returns a list.
         """
         snippets = get_app_conf(
             applications,
@@ -87,7 +107,7 @@ class FilterModule(object):
     @staticmethod
     def get_csp_hash(content):
         """
-        Compute the SHA256 hash of the given inline content and return
+        Computes the SHA256 hash of the given inline content and returns
         a CSP token like "'sha256-<base64>'".
         """
         try:
@@ -97,6 +117,10 @@ class FilterModule(object):
         except Exception as exc:
             raise AnsibleFilterError(f"get_csp_hash failed: {exc}")
 
+    # -------------------------------
+    # Main builder
+    # -------------------------------
+
     def build_csp_header(
         self,
         applications,
@@ -106,75 +130,80 @@ class FilterModule(object):
         matomo_feature_name='matomo'
     ):
         """
-        Build the Content-Security-Policy header value dynamically based on application settings.
-        Inline hashes are read from applications[application_id].csp.hashes
+        Builds the Content-Security-Policy header value dynamically based on application settings.
+        - Flags (e.g., 'unsafe-eval', 'unsafe-inline') are read from server.csp.flags.<directive>,
+          with sane defaults applied in get_csp_flags (always 'unsafe-inline' for style-src and style-src-elem).
+        - Inline hashes are read from server.csp.hashes.<directive>.
+        - Whitelists are read from server.csp.whitelist.<directive>.
+        - Inline hashes are added only if the final tokens do NOT include 'unsafe-inline'.
         """
         try:
             directives = [
-                'default-src',      # Fallback source list for all content types not explicitly listed
-                'connect-src',      # Controls allowed URLs for XHR, WebSockets, EventSource, and fetch()
-                'frame-ancestors',  # Restricts which parent frames can embed this page via <iframe>, <object>, <embed>, <applet>
-                'frame-src',        # Controls allowed sources for nested browsing contexts like <iframe>
-                'script-src',       # Controls allowed sources for inline scripts and <script> elements (general script execution)
-                'script-src-elem',  # Controls allowed sources specifically for <script> elements (separate from inline/event handlers)
-                'style-src',        # Controls allowed sources for inline styles and <style>/<link> elements (general styles)
-                'style-src-elem',   # Controls allowed sources specifically for <style> and <link rel="stylesheet"> elements
-                'font-src',         # Controls allowed sources for fonts loaded via @font-face
-                'worker-src',       # Controls allowed sources for web workers, shared workers, and service workers
-                'manifest-src',     # Controls allowed sources for web app manifests
-                'media-src',        # Controls allowed sources for media files like <audio> and <video>
+                'default-src',      # Fallback source list for content types not explicitly listed
+                'connect-src',      # Allowed URLs for XHR, WebSockets, EventSource, fetch()
+                'frame-ancestors',  # Who may embed this page
+                'frame-src',        # Sources for nested browsing contexts (e.g., <iframe>)
+                'script-src',       # Sources for script execution
+                'script-src-elem',  # Sources for <script> elements
+                'style-src',        # Sources for inline styles and <style>/<link> elements
+                'style-src-elem',   # Sources for <style> and <link rel="stylesheet">
+                'font-src',         # Sources for fonts
+                'worker-src',       # Sources for workers
+                'manifest-src',     # Sources for web app manifests
+                'media-src',        # Sources for audio and video
             ]
+
             parts = []
 
             for directive in directives:
                 tokens = ["'self'"]
 
-                # unsafe-eval / unsafe-inline flags
+                # 1) Load flags (includes defaults from get_csp_flags)
                 flags = self.get_csp_flags(applications, application_id, directive)
                 tokens += flags
 
-                if directive in [ 'script-src-elem', 'connect-src', 'style-src-elem' ]:
-                    # Allow fetching from internal CDN as default for all applications
-                    tokens.append(get_url(domains,'web-svc-cdn',web_protocol))
-                
-                if directive in ['script-src-elem', 'connect-src']:
-                    # Matomo integration
-                    if self.is_feature_enabled(applications, matomo_feature_name, application_id):
-                        tokens.append(get_url(domains,'web-app-matomo',web_protocol))
+                # 2) Allow fetching from internal CDN by default for selected directives
+                if directive in ['script-src-elem', 'connect-src', 'style-src-elem']:
+                    tokens.append(get_url(domains, 'web-svc-cdn', web_protocol))
 
-                # ReCaptcha integration: allow loading scripts from Google if feature enabled
+                # 3) Matomo integration if feature is enabled
+                if directive in ['script-src-elem', 'connect-src']:
+                    if self.is_feature_enabled(applications, matomo_feature_name, application_id):
+                        tokens.append(get_url(domains, 'web-app-matomo', web_protocol))
+
+                # 4) ReCaptcha integration (scripts + frames) if feature is enabled
                 if self.is_feature_enabled(applications, 'recaptcha', application_id):
-                    if directive in ['script-src-elem',"frame-src"]:
+                    if directive in ['script-src-elem', 'frame-src']:
                         tokens.append('https://www.gstatic.com')
                         tokens.append('https://www.google.com')
 
+                # 5) Frame ancestors handling (desktop + logout support)
                 if directive == 'frame-ancestors':
-                    # Enable loading via ancestors
                     if self.is_feature_enabled(applications, 'desktop', application_id):
+                        # Allow being embedded by the desktop app domain (and potentially its parent)
                         domain = domains.get('web-app-desktop')[0]
-                        sld_tld = ".".join(domain.split(".")[-2:])  # yields "example.com"
-                        tokens.append(f"{sld_tld}")                 # yields "*.example.com"
-                
+                        sld_tld = ".".join(domain.split(".")[-2:])  # e.g., example.com
+                        tokens.append(f"{sld_tld}")
                     if self.is_feature_enabled(applications, 'logout', application_id):
-                        
-                        # Allow logout via infinito logout proxy
-                        tokens.append(get_url(domains,'web-svc-logout',web_protocol))
-                        
-                        # Allow logout via keycloak app
-                        tokens.append(get_url(domains,'web-app-keycloak',web_protocol))
+                        # Allow embedding via logout proxy and Keycloak app
+                        tokens.append(get_url(domains, 'web-svc-logout', web_protocol))
+                        tokens.append(get_url(domains, 'web-app-keycloak', web_protocol))
 
-                # whitelist
+                # 6) Custom whitelist entries
                 tokens += self.get_csp_whitelist(applications, application_id, directive)
 
-                # only add hashes if 'unsafe-inline' is NOT in flags
-                if "'unsafe-inline'" not in flags:
+                # 7) Add inline content hashes ONLY if final tokens do NOT include 'unsafe-inline'
+                #    (Check tokens, not flags, to include defaults and later modifications.)
+                if "'unsafe-inline'" not in tokens:
                     for snippet in self.get_csp_inline_content(applications, application_id, directive):
                         tokens.append(self.get_csp_hash(snippet))
 
+                # Append directive
                 parts.append(f"{directive} {' '.join(tokens)};")
 
-            # static img-src
+            # 8) Static img-src directive (kept permissive for data/blob and any host)
             parts.append("img-src * data: blob:;")
+
             return ' '.join(parts)
 
         except Exception as exc:
diff --git a/tests/unit/filter_plugins/test_csp_filters.py b/tests/unit/filter_plugins/test_csp_filters.py
index 0b231831..6c528df8 100644
--- a/tests/unit/filter_plugins/test_csp_filters.py
+++ b/tests/unit/filter_plugins/test_csp_filters.py
@@ -222,6 +222,106 @@ class TestCspFilters(unittest.TestCase):
         # Should no longer contain the SLD+TLD
         self.assertNotIn("domain-example.com", header_no)
 
+    def test_flags_default_unsafe_inline_for_styles(self):
+        """
+        get_csp_flags should default to include 'unsafe-inline' for style-src and style-src-elem,
+        even when no explicit flags are configured.
+        """
+        # No explicit flags for app2
+        self.assertIn("'unsafe-inline'", self.filter.get_csp_flags(self.apps, 'app2', 'style-src'))
+        self.assertIn("'unsafe-inline'", self.filter.get_csp_flags(self.apps, 'app2', 'style-src-elem'))
+
+        # Non-style directive should NOT get unsafe-inline by default
+        self.assertNotIn("'unsafe-inline'", self.filter.get_csp_flags(self.apps, 'app2', 'script-src'))
+
+
+    def test_style_src_hashes_suppressed_by_default(self):
+        """
+        Because 'unsafe-inline' is defaulted for style-src, hashes for style-src should NOT be included.
+        """
+        header = self.filter.build_csp_header(self.apps, 'app1', self.domains, web_protocol='https')
+        style_hash = self.filter.get_csp_hash("body { background: #fff; }")
+        self.assertNotIn(style_hash, header)
+
+        # Ensure 'unsafe-inline' actually present in style-src directive
+        tokens = self._get_directive_tokens(header, 'style-src')
+        self.assertIn("'unsafe-inline'", tokens)
+
+
+    def test_style_src_override_disables_inline_and_enables_hashes(self):
+        """
+        If an app explicitly disables 'unsafe-inline' for style-src, then hashes MUST appear.
+        """
+        # Configure override: disable unsafe-inline for style-src
+        self.apps.setdefault('app1', {}).setdefault('server', {}).setdefault('csp', {}).setdefault('flags', {}).setdefault('style-src', {})
+        self.apps['app1']['server']['csp']['flags']['style-src']['unsafe-inline'] = False
+
+        # Also ensure there is a style-src hash to include
+        self.apps['app1']['server']['csp']['hashes']['style-src'] = "body { background: #fff; }"
+
+        header = self.filter.build_csp_header(self.apps, 'app1', self.domains, web_protocol='https')
+
+        # Then the style hash SHOULD be present
+        style_hash = self.filter.get_csp_hash("body { background: #fff; }")
+        self.assertIn(style_hash, header)
+
+        # And 'unsafe-inline' should NOT be present in style-src tokens
+        tokens = self._get_directive_tokens(header, 'style-src')
+        self.assertNotIn("'unsafe-inline'", tokens)
+
+
+    def test_style_src_elem_default_unsafe_inline(self):
+        """
+        style-src-elem should include 'unsafe-inline' by default (from get_csp_flags defaults).
+        """
+        header = self.filter.build_csp_header(self.apps, 'app1', self.domains, web_protocol='https')
+        tokens = self._get_directive_tokens(header, 'style-src-elem')
+        self.assertIn("'unsafe-inline'", tokens)
+
+
+    def test_script_src_hash_behavior_depends_on_unsafe_inline_flag(self):
+        """
+        For script-src:
+        - When unsafe-inline=False (as in app1), hashes SHOULD be included.
+        - If we flip unsafe-inline=True, hashes should NOT be included.
+        """
+        # Baseline (from setUp): app1 script-src has unsafe-inline=False and one hash
+        header = self.filter.build_csp_header(self.apps, 'app1', self.domains, web_protocol='https')
+        script_hash = self.filter.get_csp_hash("console.log('hello');")
+        self.assertIn(script_hash, header)
+
+        # Now toggle unsafe-inline=True and ensure hash disappears
+        self.apps['app1']['server']['csp']['flags']['script-src']['unsafe-inline'] = True
+        header_inline = self.filter.build_csp_header(self.apps, 'app1', self.domains, web_protocol='https')
+        self.assertNotIn(script_hash, header_inline)
+
+        # And 'unsafe-inline' should be present in the script-src tokens now
+        tokens = self._get_directive_tokens(header_inline, 'script-src')
+        self.assertIn("'unsafe-inline'", tokens)
+
+
+    def test_hashes_guard_checks_final_tokens_not_only_flags(self):
+        """
+        Ensure the 'no-hash-when-unsafe-inline' rule is driven by FINAL tokens,
+        not just raw flags: simulate default-provided 'unsafe-inline' (style-src)
+        without explicitly setting it in flags and verify hashes are still suppressed.
+        """
+        # Remove explicit style-src flags entirely to rely solely on defaults
+        self.apps['app1']['server']['csp']['flags'].pop('style-src', None)
+
+        # Provide a style-src hash
+        self.apps['app1']['server']['csp']['hashes']['style-src'] = "body { color: blue; }"
+        style_hash = self.filter.get_csp_hash("body { color: blue; }")
+
+        header = self.filter.build_csp_header(self.apps, 'app1', self.domains, web_protocol='https')
+
+        # Because defaults include 'unsafe-inline' for style-src, the hash MUST NOT appear
+        self.assertNotIn(style_hash, header)
+
+        # And 'unsafe-inline' must appear in final tokens
+        tokens = self._get_directive_tokens(header, 'style-src')
+        self.assertIn("'unsafe-inline'", tokens)
+
 
 if __name__ == '__main__':
     unittest.main()