Skip to content

Commit eadb0aa

Browse files
authored
fix(security): shorten JWT lifetime to 24h + add web-UI CSP (#657) (#709)
* 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. * test(security): use ESM import in CSP test to satisfy eslint (#657)
1 parent 245a502 commit eadb0aa

5 files changed

Lines changed: 129 additions & 1 deletion

File tree

CLAUDE.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,15 @@ REDIS_URL=redis://localhost:6379
304304

305305
CODEFRAME_API_KEY_SECRET=<secret> # API key hashing
306306

307+
# Session token lifetime (#657 — defense-in-depth)
308+
JWT_LIFETIME_SECONDS=86400 # JWT validity window; default 24h (was 7d).
309+
# Shorter window limits exposure of the
310+
# localStorage-stored web-UI token. No
311+
# refresh token, so users re-login after
312+
# expiry. The web UI also ships a CSP
313+
# (web-ui/security-headers.js) to contain
314+
# XSS-based token exfiltration.
315+
307316
# Outbound webhook SSRF guard (#656) — default OFF (block)
308317
CODEFRAME_ALLOW_PRIVATE_WEBHOOKS=1 # Allow webhook URLs whose host resolves to
309318
# private/loopback/link-local/metadata IPs.

codeframe/auth/manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ def refresh_secret() -> str:
6868
# These must match the JWTStrategy defaults from FastAPI Users
6969
JWT_ALGORITHM = "HS256"
7070
JWT_AUDIENCE = ["fastapi-users:auth"]
71-
JWT_LIFETIME_SECONDS = int(os.getenv("JWT_LIFETIME_SECONDS", "604800")) # 7 days
71+
JWT_LIFETIME_SECONDS = int(os.getenv("JWT_LIFETIME_SECONDS", "86400")) # 24h (#657)
7272

7373
# NOTE: The default-secret warning is intentionally NOT emitted at import time.
7474
# Importing this module must stay silent so it never leaks onto the CLI (the
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/**
2+
* Security headers / CSP builder tests (#657).
3+
*
4+
* The CSP's exfil-containment value lives in connect-src / img-src /
5+
* object-src / base-uri / frame-ancestors — these tests pin that lockdown so a
6+
* future edit can't silently widen it (e.g. to `connect-src *`).
7+
*/
8+
import { buildCsp, buildConnectSrc, securityHeaders } from '../security-headers';
9+
10+
describe('security headers (#657)', () => {
11+
test('CSP locks down the exfil-relevant directives', () => {
12+
const csp = buildCsp({});
13+
expect(csp).toContain("default-src 'self'");
14+
expect(csp).toContain("object-src 'none'");
15+
expect(csp).toContain("base-uri 'self'");
16+
expect(csp).toContain("frame-ancestors 'none'");
17+
// The GitHub owner avatar is the only allowed external image host;
18+
// anything else would re-open a GET-based exfil channel.
19+
expect(csp).toContain('https://avatars.githubusercontent.com');
20+
expect(csp).not.toContain('img-src *');
21+
});
22+
23+
test('connect-src includes self and the configured backend + ws origins', () => {
24+
const cs = buildConnectSrc({
25+
apiUrl: 'https://api.example.com',
26+
wsUrl: 'wss://api.example.com',
27+
});
28+
expect(cs).toContain("'self'");
29+
expect(cs).toContain('https://api.example.com');
30+
expect(cs).toContain('wss://api.example.com');
31+
});
32+
33+
test('connect-src never falls back to a wildcard', () => {
34+
// Empty env (same-origin API, default ws) must still be a closed list.
35+
const cs = buildConnectSrc({ apiUrl: '', wsUrl: '' });
36+
expect(cs).not.toContain('*');
37+
expect(cs).toContain("'self'");
38+
expect(cs).toContain('ws://localhost:8000');
39+
});
40+
41+
test('securityHeaders ships the CSP plus the hardening header set', () => {
42+
const keys = securityHeaders({}).map((h) => h.key);
43+
expect(keys).toEqual(
44+
expect.arrayContaining([
45+
'Content-Security-Policy',
46+
'X-Content-Type-Options',
47+
'X-Frame-Options',
48+
'Referrer-Policy',
49+
])
50+
);
51+
});
52+
});

web-ui/next.config.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
1+
const { securityHeaders } = require('./security-headers');
2+
13
/** @type {import('next').NextConfig} */
24
const nextConfig = {
5+
// Defense-in-depth CSP + hardening headers (#657): contains any future XSS
6+
// so an injected script can't exfiltrate the localStorage JWT.
7+
async headers() {
8+
return [{ source: '/:path*', headers: securityHeaders() }];
9+
},
310
async rewrites() {
411
return {
512
beforeFiles: [

web-ui/security-headers.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* Content-Security-Policy + hardening headers for the web UI (#657).
3+
*
4+
* Defense-in-depth: the JWT lives in localStorage (the EventSource header
5+
* limitation drove the `?token=` design), so a CSP contains any future XSS by
6+
* locking down where injected JS can send data. connect-src is built from the
7+
* SAME build-time env the app uses for its API/WS calls, so it matches the
8+
* real backend without hardcoding a deploy URL.
9+
*
10+
* Required by next.config.js (CommonJS) — keep this file dependency-free.
11+
*/
12+
13+
const DEFAULT_WS_URL = 'ws://localhost:8000';
14+
const AVATAR_HOST = 'https://avatars.githubusercontent.com';
15+
16+
/**
17+
* Closed allow-list of origins the browser may talk to. 'self' covers the
18+
* same-origin REST/SSE traffic (NEXT_PUBLIC_API_URL defaults to '' = proxied);
19+
* the WebSocket hooks dial NEXT_PUBLIC_WS_URL (or the localhost default).
20+
*/
21+
function buildConnectSrc({ apiUrl, wsUrl } = {}) {
22+
const sources = new Set(["'self'"]);
23+
if (apiUrl) sources.add(apiUrl);
24+
sources.add(wsUrl || DEFAULT_WS_URL);
25+
return Array.from(sources).join(' ');
26+
}
27+
28+
function buildCsp(env = process.env) {
29+
const connectSrc = buildConnectSrc({
30+
apiUrl: env.NEXT_PUBLIC_API_URL,
31+
wsUrl: env.NEXT_PUBLIC_WS_URL,
32+
});
33+
return [
34+
"default-src 'self'",
35+
// ponytail: 'unsafe-inline'/'unsafe-eval' are required by the Next.js App
36+
// Router without a per-request nonce middleware (a much larger change).
37+
// Exfil containment comes from connect-src/img-src/object-src below — not
38+
// script-src — so the token can't be POSTed/GET'd to an attacker origin.
39+
"script-src 'self' 'unsafe-inline' 'unsafe-eval'",
40+
"style-src 'self' 'unsafe-inline'",
41+
`img-src 'self' data: blob: ${AVATAR_HOST}`,
42+
"font-src 'self' data:",
43+
`connect-src ${connectSrc}`,
44+
"object-src 'none'",
45+
"base-uri 'self'",
46+
"frame-ancestors 'none'",
47+
"form-action 'self'",
48+
].join('; ');
49+
}
50+
51+
function securityHeaders(env = process.env) {
52+
return [
53+
{ key: 'Content-Security-Policy', value: buildCsp(env) },
54+
{ key: 'X-Content-Type-Options', value: 'nosniff' },
55+
{ key: 'X-Frame-Options', value: 'DENY' },
56+
{ key: 'Referrer-Policy', value: 'strict-origin-when-cross-origin' },
57+
];
58+
}
59+
60+
module.exports = { buildCsp, buildConnectSrc, securityHeaders };

0 commit comments

Comments
 (0)