From e4e1bddccf1943db4938553f330fccc84dd5f1ee Mon Sep 17 00:00:00 2001 From: Frank Bria <136862992+frankbria@users.noreply.github.com> Date: Sun, 21 Jun 2026 13:04:51 -0700 Subject: [PATCH 1/2] fix(security): shorten JWT lifetime to 24h + add web-UI CSP (#657) Defense-in-depth for the localStorage-stored auth JWT (audit finding M3). Takes the acceptance criteria's "lifetime shortened + CSP added" branch rather than the invasive HttpOnly-cookie refactor (which would break the SSE/WS ?token= design). - auth/manager.py: JWT_LIFETIME_SECONDS default 604800 (7d) -> 86400 (24h); env override unchanged. Tests read the constant dynamically. - web-ui/security-headers.js: CSP + hardening headers. connect-src is built from the same build-time NEXT_PUBLIC_API_URL/WS_URL the app dials, so it matches the real backend; img-src allows only the GitHub avatar host; object-src/base-uri/frame-ancestors locked down. Wired via next.config.js headers(). - Test pins the exfil-relevant directives against silent widening. Known ceiling: script-src keeps 'unsafe-inline'/'unsafe-eval' (Next.js App Router needs them absent a nonce middleware); exfil containment comes from connect-src/img-src/object-src. No refresh token, so re-login after 24h. --- CLAUDE.md | 9 ++++ codeframe/auth/manager.py | 2 +- web-ui/__tests__/security-headers.test.js | 56 +++++++++++++++++++++ web-ui/next.config.js | 7 +++ web-ui/security-headers.js | 60 +++++++++++++++++++++++ 5 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 web-ui/__tests__/security-headers.test.js create mode 100644 web-ui/security-headers.js diff --git a/CLAUDE.md b/CLAUDE.md index ea9f48e1..3097db75 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -304,6 +304,15 @@ REDIS_URL=redis://localhost:6379 CODEFRAME_API_KEY_SECRET= # API key hashing +# Session token lifetime (#657 — defense-in-depth) +JWT_LIFETIME_SECONDS=86400 # JWT validity window; default 24h (was 7d). + # Shorter window limits exposure of the + # localStorage-stored web-UI token. No + # refresh token, so users re-login after + # expiry. The web UI also ships a CSP + # (web-ui/security-headers.js) to contain + # XSS-based token exfiltration. + # Outbound webhook SSRF guard (#656) — default OFF (block) CODEFRAME_ALLOW_PRIVATE_WEBHOOKS=1 # Allow webhook URLs whose host resolves to # private/loopback/link-local/metadata IPs. diff --git a/codeframe/auth/manager.py b/codeframe/auth/manager.py index 42bfc6f3..d4324e68 100644 --- a/codeframe/auth/manager.py +++ b/codeframe/auth/manager.py @@ -68,7 +68,7 @@ def refresh_secret() -> str: # These must match the JWTStrategy defaults from FastAPI Users JWT_ALGORITHM = "HS256" JWT_AUDIENCE = ["fastapi-users:auth"] -JWT_LIFETIME_SECONDS = int(os.getenv("JWT_LIFETIME_SECONDS", "604800")) # 7 days +JWT_LIFETIME_SECONDS = int(os.getenv("JWT_LIFETIME_SECONDS", "86400")) # 24h (#657) # NOTE: The default-secret warning is intentionally NOT emitted at import time. # Importing this module must stay silent so it never leaks onto the CLI (the diff --git a/web-ui/__tests__/security-headers.test.js b/web-ui/__tests__/security-headers.test.js new file mode 100644 index 00000000..514957e3 --- /dev/null +++ b/web-ui/__tests__/security-headers.test.js @@ -0,0 +1,56 @@ +/** + * Security headers / CSP builder tests (#657). + * + * The CSP's exfil-containment value lives in connect-src / img-src / + * object-src / base-uri / frame-ancestors — these tests pin that lockdown so a + * future edit can't silently widen it (e.g. to `connect-src *`). + */ +const { + buildCsp, + buildConnectSrc, + securityHeaders, +} = require('../security-headers'); + +describe('security headers (#657)', () => { + test('CSP locks down the exfil-relevant directives', () => { + const csp = buildCsp({}); + expect(csp).toContain("default-src 'self'"); + expect(csp).toContain("object-src 'none'"); + expect(csp).toContain("base-uri 'self'"); + expect(csp).toContain("frame-ancestors 'none'"); + // The GitHub owner avatar is the only allowed external image host; + // anything else would re-open a GET-based exfil channel. + expect(csp).toContain('https://avatars.githubusercontent.com'); + expect(csp).not.toContain('img-src *'); + }); + + test('connect-src includes self and the configured backend + ws origins', () => { + const cs = buildConnectSrc({ + apiUrl: 'https://api.example.com', + wsUrl: 'wss://api.example.com', + }); + expect(cs).toContain("'self'"); + expect(cs).toContain('https://api.example.com'); + expect(cs).toContain('wss://api.example.com'); + }); + + test('connect-src never falls back to a wildcard', () => { + // Empty env (same-origin API, default ws) must still be a closed list. + const cs = buildConnectSrc({ apiUrl: '', wsUrl: '' }); + expect(cs).not.toContain('*'); + expect(cs).toContain("'self'"); + expect(cs).toContain('ws://localhost:8000'); + }); + + test('securityHeaders ships the CSP plus the hardening header set', () => { + const keys = securityHeaders({}).map((h) => h.key); + expect(keys).toEqual( + expect.arrayContaining([ + 'Content-Security-Policy', + 'X-Content-Type-Options', + 'X-Frame-Options', + 'Referrer-Policy', + ]) + ); + }); +}); diff --git a/web-ui/next.config.js b/web-ui/next.config.js index cd430eae..02dd4351 100644 --- a/web-ui/next.config.js +++ b/web-ui/next.config.js @@ -1,5 +1,12 @@ +const { securityHeaders } = require('./security-headers'); + /** @type {import('next').NextConfig} */ const nextConfig = { + // Defense-in-depth CSP + hardening headers (#657): contains any future XSS + // so an injected script can't exfiltrate the localStorage JWT. + async headers() { + return [{ source: '/:path*', headers: securityHeaders() }]; + }, async rewrites() { return { beforeFiles: [ diff --git a/web-ui/security-headers.js b/web-ui/security-headers.js new file mode 100644 index 00000000..752721f2 --- /dev/null +++ b/web-ui/security-headers.js @@ -0,0 +1,60 @@ +/** + * Content-Security-Policy + hardening headers for the web UI (#657). + * + * Defense-in-depth: the JWT lives in localStorage (the EventSource header + * limitation drove the `?token=` design), so a CSP contains any future XSS by + * locking down where injected JS can send data. connect-src is built from the + * SAME build-time env the app uses for its API/WS calls, so it matches the + * real backend without hardcoding a deploy URL. + * + * Required by next.config.js (CommonJS) — keep this file dependency-free. + */ + +const DEFAULT_WS_URL = 'ws://localhost:8000'; +const AVATAR_HOST = 'https://avatars.githubusercontent.com'; + +/** + * Closed allow-list of origins the browser may talk to. 'self' covers the + * same-origin REST/SSE traffic (NEXT_PUBLIC_API_URL defaults to '' = proxied); + * the WebSocket hooks dial NEXT_PUBLIC_WS_URL (or the localhost default). + */ +function buildConnectSrc({ apiUrl, wsUrl } = {}) { + const sources = new Set(["'self'"]); + if (apiUrl) sources.add(apiUrl); + sources.add(wsUrl || DEFAULT_WS_URL); + return Array.from(sources).join(' '); +} + +function buildCsp(env = process.env) { + const connectSrc = buildConnectSrc({ + apiUrl: env.NEXT_PUBLIC_API_URL, + wsUrl: env.NEXT_PUBLIC_WS_URL, + }); + return [ + "default-src 'self'", + // ponytail: 'unsafe-inline'/'unsafe-eval' are required by the Next.js App + // Router without a per-request nonce middleware (a much larger change). + // Exfil containment comes from connect-src/img-src/object-src below — not + // script-src — so the token can't be POSTed/GET'd to an attacker origin. + "script-src 'self' 'unsafe-inline' 'unsafe-eval'", + "style-src 'self' 'unsafe-inline'", + `img-src 'self' data: blob: ${AVATAR_HOST}`, + "font-src 'self' data:", + `connect-src ${connectSrc}`, + "object-src 'none'", + "base-uri 'self'", + "frame-ancestors 'none'", + "form-action 'self'", + ].join('; '); +} + +function securityHeaders(env = process.env) { + return [ + { key: 'Content-Security-Policy', value: buildCsp(env) }, + { key: 'X-Content-Type-Options', value: 'nosniff' }, + { key: 'X-Frame-Options', value: 'DENY' }, + { key: 'Referrer-Policy', value: 'strict-origin-when-cross-origin' }, + ]; +} + +module.exports = { buildCsp, buildConnectSrc, securityHeaders }; From 1117cdd33ac6e5ec5f8f5c44d1e0bbcfa55533b7 Mon Sep 17 00:00:00 2001 From: Frank Bria <136862992+frankbria@users.noreply.github.com> Date: Sun, 21 Jun 2026 13:28:25 -0700 Subject: [PATCH 2/2] test(security): use ESM import in CSP test to satisfy eslint (#657) --- web-ui/__tests__/security-headers.test.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/web-ui/__tests__/security-headers.test.js b/web-ui/__tests__/security-headers.test.js index 514957e3..5328b19f 100644 --- a/web-ui/__tests__/security-headers.test.js +++ b/web-ui/__tests__/security-headers.test.js @@ -5,11 +5,7 @@ * object-src / base-uri / frame-ancestors — these tests pin that lockdown so a * future edit can't silently widen it (e.g. to `connect-src *`). */ -const { - buildCsp, - buildConnectSrc, - securityHeaders, -} = require('../security-headers'); +import { buildCsp, buildConnectSrc, securityHeaders } from '../security-headers'; describe('security headers (#657)', () => { test('CSP locks down the exfil-relevant directives', () => {