Deduplicate HTTP security headers between nginx and Flask-Talisman (#169)

nginx and Superset's own Flask-Talisman middleware both independently
added Strict-Transport-Security, X-Content-Type-Options, X-Frame-Options,
X-XSS-Protection, and Referrer-Policy to every response. nginx's add_header
doesn't replace upstream headers, it appends, so responses carried
duplicates with different values - per spec/browser handling this silently
dropped nginx's preload flag on HSTS (RFC 6797: only the first STS header
is processed), risked invalidating X-Frame-Options entirely on some
browsers when comma-joined, and silently downgraded Referrer-Policy to
nginx's weaker legacy default (UAs take the last policy across combined
headers). Only nginx's HSTS line carried `always`, so its other four
headers vanished on non-2xx/3xx responses (404, 500), relying entirely on
Talisman as an unconditional fallback.

Use proxy_hide_header to strip Talisman's upstream copies and let nginx's
own add_header lines (all now with `always`) be the sole source of these
five headers. Corrects Referrer-Policy to strict-origin-when-cross-origin.
Left Talisman's CSP and all other config untouched - reconfiguring
Talisman instead would have required replicating Superset's internal
default CSP dict verbatim to safely override TALISMAN_CONFIG.

Add a functional test (status_headers) that curls both a normal path and
a nonexistent one, asserting each header appears exactly once with the
expected value on both.

Author: szachovy

CHANGELOG.md

@@ -39,6 +39,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+* Deduplicated HTTP security headers between nginx and Flask-Talisman: nginx now hides Talisman's upstream
+  copies via `proxy_hide_header` and is the sole source of `Strict-Transport-Security`, `X-Frame-Options`,
+  `X-XSS-Protection`, `X-Content-Type-Options`, and `Referrer-Policy`, applied with `always` so they persist
+  on error responses too. Corrected `Referrer-Policy` to `strict-origin-when-cross-origin`.
 * Redacted MySQL password from `superset` entrypoint's `test_db` output and disabled `-x` tracing around it,
   preventing plaintext password exposure in `docker logs`/`docker service logs`.
 * Upload `.py` source instead of `.pyc` bytecode to decouple host Python version. (#38, #41)

services/superset/nginx.conf

@@ -21,11 +21,17 @@ http {
         ssl_prefer_server_ciphers on;
         ssl_session_cache shared:SSL:10m;
 
+        proxy_hide_header Strict-Transport-Security;
+        proxy_hide_header X-Content-Type-Options;
+        proxy_hide_header X-Frame-Options;
+        proxy_hide_header X-XSS-Protection;
+        proxy_hide_header Referrer-Policy;
+
         add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;
-        add_header X-Content-Type-Options nosniff;
-        add_header X-Frame-Options DENY;
-        add_header X-XSS-Protection "1; mode=block";
-        add_header Referrer-Policy "no-referrer-when-downgrade";
+        add_header X-Content-Type-Options nosniff always;
+        add_header X-Frame-Options DENY always;
+        add_header X-XSS-Protection "1; mode=block" always;
+        add_header Referrer-Policy "strict-origin-when-cross-origin" always;
 
         server_tokens off;
         tcp_nopush on;

tests/testsuite/roles/testing/files/functional_superset.py

@@ -294,6 +294,42 @@ def status_swarm(self) -> None:
             swarm_info["ControlAvailable"] is True, \
             "The testing localhost is supposed to be a Swarm manager, but it is not"
 
+    @decorators.Overlay.run_selected_methods_once
+    def status_headers(self) -> None:
+        expected_headers = {
+            "strict-transport-security": "max-age=31536000; includeSubDomains; preload",
+            "x-content-type-options": "nosniff",
+            "x-frame-options": "DENY",
+            "x-xss-protection": "1; mode=block",
+            "referrer-policy": "strict-origin-when-cross-origin",
+        }
+        for path in ("/", "/this-path-does-not-exist-xyz"):
+            command = f"""
+                curl \
+                    --cacert /app/server_certificate.pem \
+                    --silent \
+                    --head \
+                    https://{self.virtual_ip_address}{path}
+            """
+            response = self.run_command_on_the_container(command).decode("utf-8")
+            response_lines = [line.strip() for line in response.splitlines() if ":" in line]
+            for header_name, expected_value in expected_headers.items():
+                matches = [
+                    line.split(":", 1)[1].strip()
+                    for line in response_lines
+                    if line.split(":", 1)[0].strip().lower() == header_name
+                ]
+                assert \
+                    len(matches) == 1, \
+                    f"""Expected exactly one {header_name} header on {path}, found {len(matches)}: {matches}
+                        \nCommand: {command!r}\nReturned: {response!r}
+                    """
+                assert \
+                    matches[0] == expected_value, \
+                    f"""Expected {header_name} to be {expected_value!r} on {path}, got {matches[0]!r}
+                        \nCommand: {command!r}\nReturned: {response!r}
+                    """
+
     def run_query(self) -> float:
         payload = '{"database_id": 1, "runAsync": true, "sql": "SELECT * FROM superset.logs;"}'
         command = f"""