Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,15 @@ REDIS_URL=redis://localhost:6379

CODEFRAME_API_KEY_SECRET=<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.
Expand Down
2 changes: 1 addition & 1 deletion codeframe/auth/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 52 additions & 0 deletions web-ui/__tests__/security-headers.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* 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 *`).
*/
import { buildCsp, buildConnectSrc, securityHeaders } from '../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',
])
);
});
});
7 changes: 7 additions & 0 deletions web-ui/next.config.js
Original file line number Diff line number Diff line change
@@ -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: [
Expand Down
60 changes: 60 additions & 0 deletions web-ui/security-headers.js
Original file line number Diff line number Diff line change
@@ -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(' ');
Comment on lines +21 to +25

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid shipping localhost in connect-src for production defaults.

Line 24 currently adds ws://localhost:8000 whenever NEXT_PUBLIC_WS_URL is unset, and this policy is then applied globally. In production, that unnecessarily opens an extra exfil destination (any local listener) if XSS occurs.

Suggested hardening
 function buildConnectSrc({ apiUrl, wsUrl } = {}) {
   const sources = new Set(["'self'"]);
   if (apiUrl) sources.add(apiUrl);
-  sources.add(wsUrl || DEFAULT_WS_URL);
+  if (wsUrl) {
+    sources.add(wsUrl);
+  } else if (process.env.NODE_ENV !== 'production') {
+    sources.add(DEFAULT_WS_URL);
+  }
   return Array.from(sources).join(' ');
 }

Also applies to: 29-32

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web-ui/security-headers.js` around lines 21 - 25, The buildConnectSrc
function unconditionally adds DEFAULT_WS_URL (which is localhost:8000) to the
connect-src policy when wsUrl is not explicitly provided, creating an
unnecessary security risk in production by exposing a local exfil destination.
Fix this by modifying the line that adds wsUrl to only include it when
explicitly provided, removing the fallback to DEFAULT_WS_URL. Apply the same fix
to the similar pattern mentioned at lines 29-32 to ensure no default localhost
addresses are shipped in production security headers.

}

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 };
Loading