Skip to content

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

Merged
szachovy merged 2 commits into
masterfrom
fix/task-225-header-duplication
Jul 16, 2026
Merged

Deduplicate HTTP security headers between nginx and Flask-Talisman#169
szachovy merged 2 commits into
masterfrom
fix/task-225-header-duplication

Conversation

@szachovy

Copy link
Copy Markdown
Owner

Summary

  • nginx (services/superset/nginx.conf) and Superset's own Flask-Talisman middleware both independently add Strict-Transport-Security, X-Content-Type-Options, X-Frame-Options, X-XSS-Protection, and Referrer-Policy to every response. add_header doesn't replace upstream headers, it appends — so responses carried duplicates with different values, and per spec/browser handling this wasn't "defense in depth," it was silent misconfiguration:
    • HSTS: RFC 6797 mandates only the first STS header is processed. Superset's weaker copy (no preload) came first, so nginx's preload copy was spec-mandated to be ignored entirely.
    • X-Frame-Options: SAMEORIGIN vs DENY risked comma-joining into an invalid value on some browsers (documented: Safari 6.0.5), which can drop clickjacking protection entirely.
    • Referrer-Policy: UAs take the last valid policy across combined headers — nginx's weaker legacy default (no-referrer-when-downgrade) silently overrode Superset's stricter strict-origin-when-cross-origin.
    • Only nginx's HSTS line carried always, so its other four headers vanished on non-2xx/3xx responses (404, 500) — the response was only still hardened because Talisman's headers are unconditional.

Fix

  • Added proxy_hide_header for all five headers so Talisman's upstream copies never reach the client, and made nginx's own add_header lines (all now with always) the sole source of truth — headers now survive on error pages too.
  • Corrected Referrer-Policy to strict-origin-when-cross-origin. Kept X-Frame-Options: DENY (nginx's existing, stricter value — no dashboard-embedding feature exists anywhere in this repo, so nothing depends on same-origin framing).
  • Left Talisman's TALISMAN_CONFIG/CSP entirely untouched. Reconfiguring Talisman instead would have required replicating Superset's internal default CSP dict verbatim to safely override the monolithic TALISMAN_CONFIG, without ground-truth on the exact defaults shipped in apache/superset:4.0.2 — not worth the risk of silently weakening CSP when the nginx-side fix is fully sufficient.
  • Added CHANGELOG.md entry under ### Fixed.

Test plan

  • Added status_headers() to functional_superset.py, following the existing status_database/status_swarm pattern (auto-runs via the Overlay metaclass on instantiation, no functional.yml changes needed). Curls both / and a nonexistent path, asserts each of the 5 headers appears exactly once with the exact expected value on both — directly encodes this PR's acceptance criteria as a regression test.
  • python3 -m py_compile, flake8, pylint all pass on the modified test file (two pylint import-error warnings are pre-existing and confirmed unrelated by diffing against master).
  • Manually simulated the header-parsing logic against realistic curl -I output.
  • No live docker/cluster access in the environment this was developed in — worth running the new functional test (and nginx -t) against an actual deployment before merge.

szachovy added 2 commits July 16, 2026 12:57
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.
@szachovy
szachovy merged commit 7973cd9 into master Jul 16, 2026
10 of 11 checks passed
@szachovy
szachovy deleted the fix/task-225-header-duplication branch July 16, 2026 12:17
szachovy added a commit that referenced this pull request Jul 16, 2026
Fixes the two root causes behind the header-dedup fix (#169) never reaching production: stale local image cache in pull_or_build_image() and a publish race with no concurrency guard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant