-
Notifications
You must be signed in to change notification settings - Fork 5
fix(security): shorten JWT lifetime to 24h + add web-UI CSP (#657) #709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+129
−1
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
e4e1bdd
fix(security): shorten JWT lifetime to 24h + add web-UI CSP (#657)
frankbria 1117cdd
test(security): use ESM import in CSP test to satisfy eslint (#657)
frankbria 5cecd05
Merge remote-tracking branch 'origin/main' into fix/657-jwt-lifetime-csp
frankbria File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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', | ||
| ]) | ||
| ); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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(' '); | ||
| } | ||
|
|
||
| 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 }; | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid shipping
localhostinconnect-srcfor production defaults.Line 24 currently adds
ws://localhost:8000wheneverNEXT_PUBLIC_WS_URLis 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