Skip to content

fix(security): add missing HTTP security headers across all surfaces (#549) - #641

Merged
vybe merged 1 commit into
devfrom
feature/549-security-headers
May 5, 2026
Merged

fix(security): add missing HTTP security headers across all surfaces (#549)#641
vybe merged 1 commit into
devfrom
feature/549-security-headers

Conversation

@dolho

@dolho dolho commented May 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #549. UnderDefense pentest 3.4.2 flagged Trinity for missing the standard HTTP security header set. Production nginx already had most via security-headers.conf; the gap was on three surfaces this PR addresses.

Changes

Surface Before After
FastAPI middleware (src/backend/main.py) 4 headers (X-Content-Type-Options, Referrer-Policy, Permissions-Policy, Cross-Origin-Resource-Policy) + X-Frame-Options DENY, Cross-Origin-Opener-Policy same-origin, conditional HSTS (gated on X-Forwarded-Proto: https or request.url.scheme == "https")
Vite dev server (src/frontend/vite.config.js) Zero security headers — npm run dev served the SPA with nothing New configureServer plugin mirrors prod with dev-only 'unsafe-inline' 'unsafe-eval' on script-src for Vite HMR
Production nginx (security-headers.conf + nginx.conf) 8 headers, HSTS deliberately absent (comment said "ops should add at LB") HSTS now emitted via map $http_x_forwarded_proto $hsts_header — fires only when upstream TLS terminator signals HTTPS, nginx skips emission on plain HTTP

Design choices

  • CSP NOT set on FastAPI responses/api/* returns JSON, not rendered HTML. A strict CSP on JSON is meaningless and would block legitimate Swagger UI script execution from the docs route.
  • HSTS gated on actual HTTPS — emitting HSTS over HTTP is misleading (browsers ignore per RFC 6797) and can pin a stale policy. Both the FastAPI middleware and the nginx map check the upstream X-Forwarded-Proto so it works whether uvicorn is direct-facing (--proxy-headers) or behind cloudflare/traefik/external-LB.
  • HSTS NOT set in Vite dev — would force the dev box's hostname to HTTPS in the browser HSTS cache and break subsequent HTTP work on the same port.
  • X-Frame-Options DENY for FastAPI (API responses should never be iframed); SAMEORIGIN preserved for the SPA (could legitimately be embedded in same-origin contexts).
  • Cross-Origin-Embedder-Policy still omitted everywhererequire-corp would break future cross-origin resources and no SharedArrayBuffer use case justifies the cost. Existing comment in security-headers.conf documents this intentional choice.
  • Server: uvicorn already suppressed via --no-server-header flag in both Dockerfile CMD and docker-compose.yml command. No change needed.

Tests

tests/test_security_headers.py extended (now 5 tests, all pass against the live local stack):

./.venv/bin/pytest tests/test_security_headers.py -v
# 5 passed in 0.38s
  • existing: test_api_response_has_security_headers extended with X-Frame-Options + COOP assertions
  • existing: test_server_header_stripped
  • existing: test_cors_preflight_still_works
  • new: test_hsts_absent_on_plain_http
  • new: test_hsts_set_when_x_forwarded_proto_https

Manual verification

# FastAPI middleware
curl -sI http://localhost:8000/health | grep -iE "frame|origin|cont|ref|perm|hsts"
# 7 headers, no HSTS (correct — HTTP)

curl -sI -H "X-Forwarded-Proto: https" http://localhost:8000/health | grep -i hsts
# strict-transport-security: max-age=31536000; includeSubDomains

# Vite dev server
curl -sI http://localhost:8001 | grep -iE "frame|origin|cont|ref|perm|csp"
# 8 headers including dev CSP

# nginx config syntax (lints inside the trinity-agent-network so upstream resolves)
docker run --rm --network trinity-agent-network \
  -v "./src/frontend/nginx.conf:/etc/nginx/conf.d/default.conf:ro" \
  -v "./src/frontend/security-headers.conf:/etc/nginx/security-headers.conf:ro" \
  nginx:alpine nginx -t
# nginx: configuration file /etc/nginx/nginx.conf test is successful

UI smoke (Vite dev frontend at http://localhost:8001) loaded without console errors after restart. Production CSP unchanged from current shipping config — only HSTS added.

Acceptance criteria

  • All standard security headers added to nginx config (already mostly there; HSTS the gap)
  • Equivalent headers added to FastAPI middleware for non-nginx-fronted deployments and direct API access
  • Equivalent headers added to Vite dev server so devs catch CSP regressions before prod
  • Server header stripped (already via --no-server-header; verified)
  • Headers present in curl -I output on backend, Vite dev, and nginx (verified)
  • CSP allows the Vue.js frontend to function (existing prod CSP unchanged; dev CSP relaxed for HMR per Vite expectation)
  • HSTS only applied when traffic is HTTPS (gated on X-Forwarded-Proto in both FastAPI and nginx)

🤖 Generated with Claude Code

…549)

UnderDefense pentest 3.4.2 flagged Trinity as missing the standard
security header set. Most production headers were already in place via
nginx security-headers.conf but three surfaces needed work:

1. FastAPI middleware (src/backend/main.py): API responses (Swagger,
   direct curl, /docs) lacked X-Frame-Options, Cross-Origin-Opener-Policy,
   and conditional Strict-Transport-Security. Added all three. CSP is
   intentionally NOT set on /api/* responses — they're JSON, not rendered,
   and a strict CSP would block legitimate Swagger UI interactions.

2. Vite dev server (src/frontend/vite.config.js): `npm run dev` served
   the SPA with zero security headers, so devs only saw CSP regressions
   in production. New configureServer plugin mirrors security-headers.conf
   (with the dev-only relaxation of script-src 'unsafe-inline'
   'unsafe-eval' for Vite HMR + module preloads). HSTS deliberately NOT
   emitted in dev — would force the dev box's hostname to HTTPS in the
   browser HSTS cache and break subsequent HTTP work.

3. Production nginx (src/frontend/security-headers.conf + nginx.conf):
   added Strict-Transport-Security gated on X-Forwarded-Proto from the
   upstream TLS terminator (cloudflare / external LB). Implemented via
   `map $http_x_forwarded_proto $hsts_header` so the conditional is
   evaluated once per request. nginx skips emission when the mapped
   value is empty (default branch).

The FastAPI HSTS check honours both request.url.scheme and
X-Forwarded-Proto so it works whether uvicorn is direct-facing or
behind a proxy with --proxy-headers.

X-Frame-Options uses DENY on FastAPI (API responses should never be
iframed) and the existing SAMEORIGIN on the frontend (SPA could
legitimately be embedded in same-origin contexts).

Cross-Origin-Embedder-Policy intentionally still omitted across the
board: require-corp would break any future cross-origin resources and
no SharedArrayBuffer use case justifies the cost.

Tests in tests/test_security_headers.py extended with assertions for
X-Frame-Options, COOP, HSTS-absent-on-HTTP, and HSTS-present-on-
forwarded-HTTPS. All 5 pass against a live local stack.

Verified manually: curl -I against backend (:8000), Vite dev (:8001),
and nginx -t syntax check on the production config inside the Trinity
docker network.

Closes #549.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dolho
dolho requested a review from vybe May 4, 2026 10:20

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr — clean security fix, all three surfaces covered, tests pass. No blocking issues. Approving for merge.

@vybe
vybe merged commit 92bb5aa into dev May 5, 2026
2 checks passed
vybe pushed a commit that referenced this pull request Jul 2, 2026
The Files-tab preview builds blob: URLs (URL.createObjectURL) and renders them
per type — text/download via fetch() (connect-src), <img> (img-src),
<video>/<audio> (media-src), and PDF <embed> (object-src + frame-src, since
Chrome renders the embedded PDF in an internal viewer frame). blob: is not
covered by 'self', so each directive must list it explicitly. Only img-src did,
so images previewed while text/media/PDF failed ("Failed to load text content" /
Chrome's "This content is blocked"). Latent since the CSP landed (#549/#641);
#784's Content-Disposition fix never touched the real cause, so it stayed broken
in every nginx-served build and under the dev Vite CSP alike.

- Add blob: to connect-src/media-src/object-src/frame-src in BOTH CSP copies
  (security-headers.conf prod nginx + vite.config.js dev), kept in sync
- Add tests/unit/test_1400_csp_blob_preview.py: asserts blob: in each directive
  across both files and that the two files stay in sync (fails on the old CSP)
- Document the bug class in docs/memory/learnings.md and the file-manager
  feature flow

Verified locally across text, image (PNG+SVG), audio, and PDF previews.

Fixes #1400

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

2 participants