Repository navigation
fix(security): tighten CSP and add hardening headers (OWASP A02) - #52
Merged
Merged
Conversation
ZAP baseline findings (10055, 10063, 90004, 10037): - Replace the catch-all default-src CSP with explicit directives: default-src 'self', object-src 'none', base-uri/form-action/ frame-ancestors 'self', same-origin scripts and workers, Google Fonts only for style/font. 'unsafe-eval' is now dev-only. - Add Permissions-Policy denying unused features. - Add Cross-Origin-Opener-Policy and Cross-Origin-Resource-Policy (same-origin). COEP intentionally omitted: it would block third-party camera images, HLS video and embedded players. - poweredByHeader: false (drops X-Powered-By). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
carbon-evolution
added a commit
that referenced
this pull request
Oct 18, 2026
- A01 Broken Access Control: Medium -> Low. Recon scanner closed by default (token / explicit opt-in), client IP from forwarding headers only with TRUST_PROXY_HEADERS (#51). - A06 Insecure Design: Medium -> Low. Analytics opt-in, visitor-IP event removed (#50); recon no longer anonymous by default. - A02 Security Misconfiguration: stays Medium (improved). Hardening headers and a tighter CSP without unsafe-eval (#52); ZAP on main now 0 High / 3 Medium / 1 Low / 4 Info (was 0/5/5/5). Remaining Medium items are script/style 'unsafe-inline' and wildcard media sources. - A07: scanner access token compared in constant time. - Overall: 2 Medium, 8 Low (was 4 Medium, 6 Low); Scorecard 8.0. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes two OWASP A01 (Broken Access Control) findings.
SCANNER_KEYwas set,/api/scannerlet any visitor make the server scan arbitrary public hosts, and the targets saw the operator's IP.clientIp()(guard.ts) andgetClientIp()(ssrf-guard.ts) readx-forwarded-for/x-real-ip/cf-connecting-ipas sent by the client. Anyone could get around per-IP rate limits and forge the IP in the audit log.What Next.js 16 route handlers can see (tested, not assumed)
I added a temporary route that echoed the request headers (removed before commit), ran it under
next start -p 3022(Next 16.4.2), and called it with curl:curl 127.0.0.1:3022x-forwarded-for: ::ffff:127.0.0.1,x-forwarded-host/port/protofilled inX-Forwarded-For: 6.6.6.6,X-Real-IP: 7.7.7.7,CF-Connecting-IP: 8.8.8.8,X-Forwarded-Host: evil.comx-forwarded-for: 6.6.6.6, no socket address addedHost: localhost:3022from anywhereThe cause is in
next/dist/server/base-server.js:req.headers['x-forwarded-for'] ??= socket.remoteAddress. App-router handlers have norequest.ip, and the socket address only shows up when the client didn't send its own XFF. So without a trusted proxy, no header can tell a handler where a request really came from. A proxy-free check like "allow if the request is from loopback" can always be faked by a non-browser client.Design and threat model
Scanner gate (
src/lib/scanner-access.ts, applied insrc/app/api/scanner/route.ts)A request is allowed, in this order, when:
SCANNER_ALLOW_ANONYMOUS=true: the operator explicitly makes the scanner public (documented as not recommended).SCANNER_ACCESS_TOKENis set and the request sends it inX-Scanner-Token. The comparison usescrypto.timingSafeEqualon SHA-256 digests, so differing lengths don't leak timing either. A custom header can't be attached cross-origin without a CORS preflight, so another site can't use a token holder's browser to run scans.NODE_ENV === 'development'(npm run dev/npm run osiris) and the request looks like it came from loopback:x-real-ip/cf-connecting-ip/forwardedheaderSec-Fetch-Siteis absent,same-originornoneAnything else is refused:
{auth:"token"}when a token is configured (the UI then prompts for it).The gate runs after the rate limit, so token guesses are throttled too.
Trade-off for rule 3: because of what the table above shows, the loopback check uses headers the client controls. It does block browser-based abuse: DNS rebinding (wrong Host), a LAN visitor's browser (wrong Host), and a cross-site page firing requests at
localhost:3000(Sec-Fetch-Site). It does not stop a deliberate non-browser client on the same network that forges those headers. That's why the rule only applies tonext dev, which should never be publicly exposed anyway. Every production build (next start, Docker, standalone) needs rule 1 or 2. I considered loopback detection for production builds and rejected it: the handler can't learn the socket address, and Docker requests arrive from the bridge gateway, not loopback.Recon panel (
OsintPanel.tsx): on a 401 from/api/scanner, the panel:Trusted client IP (
src/lib/client-ip.ts)There is now one helper.
guard.clientIpandssrf-guard.getClientIpboth re-export it, so all callers keep their signatures. Behaviour byTRUST_PROXY_HEADERSvalue:'unknown'. Per-IP limits become one shared bucket per endpoint. That's a hard cap on abuse instead of a limit anyone can bypass.true/1: uses the rightmost XFF entry, which is the address the nearest trusted proxy appended. The old code took the first entry, and that one is client-supplied even behind nginx.N: N proxies in a chain; uses the Nth entry from the right.cloudflare: usescf-connecting-ip.Values that aren't IP addresses become
'unknown', which also stops log injection through the header.src/app/api/geo/route.tsstill reads the headers on its own. That route only geolocates the visitor for their own map, so a forged value only affects the person forging it; I left it unchanged.Also fixed
type=constructorresolved throughObject.prototypeand only failed later, atAbortSignal.timeout(undefined)(502).New env vars (appended at the end of
.env.example)SCANNER_ACCESS_TOKENX-Scanner-TokenSCANNER_ALLOW_ANONYMOUStrue= any visitor can run scans (not recommended)TRUST_PROXY_HEADERStrue/<N>/cloudflare. Set it only when the app port is reachable solely through that proxyREADME and DOCKER.md explain all three.
nginx/nginx.confnow has a comment explaining that its$proxy_add_x_forwarded_forappends the real client address and OSIRIS reads that rightmost entry whenTRUST_PROXY_HEADERS=true. The nginx config itself is unchanged: it already overwritesX-Real-IPand appends to XFF.SCANNER_ACCESS_TOKENand give it to trusted users, or the recon scans will return 403.npm run osiris/npm run devon localhost needs no change.TRUST_PROXY_HEADERS=trueto keep per-client rate limits. Without it, all clients share one bucket. Note thatdocker-compose.ymlalso publishes the app onOSIRIS_PORTdirectly. Only enable trust if that port isn't reachable from outside, otherwise clients can bypass nginx and forge the header.Tests
src/app/api/scanner-access.test.ts(25 tests). It lives next toroute-hardening.test.tsbecause.gitignore'sscanner/entry ignores test files insideapi/scanner/. Covered:auth:"token"), no fetch;next dev+ loopback → proxied;src/lib/client-ip.test.ts(20 tests, run against bothguard.clientIpandssrf-guard.getClientIp): headers ignored for unset / '' / false / 0 / nonsense;true→ rightmost entry;2→ second from the right; fallback to X-Real-IP;cloudflare; non-IP values rejected;ip:portnormalised.Against the previous code, 28 of the 45 new tests fail:
type=constructor;Full suite:
npx vitest run(with the usual 4 infra excludes) → 111/111 pass. Also passing:tsc --noEmit(0 errors),eslint .(0 errors; OsintPanel still has 39 warnings, same as before),npm run build.Live evidence (production build,
next start -p 3022)The sidecar wasn't running, so
SCANNER_URLpointed at an unused port. In these results, 502 "Scanner unreachable / fetch failed" means the request passed the gate and reached the upstream-fetch stage.curl /api/scanner?target=93.184.215.14&type=headers→ 403 "Scanner disabled for remote clients"Host: localhost+X-Forwarded-For: 127.0.0.1→ 403SCANNER_ACCESS_TOKENset:TRUST_PROXY_HEADERS=true:XFF: 1.1.1.1, 203.0.113.$i(rightmost entry varies) → six 502s, separate bucketsXFF: 198.51.100.$i, 203.0.113.77(only the forged leftmost entry varies) → 502 ×5, then 429next dev -p 3022, no token:Host: osiris.example.com→ 403Sec-Fetch-Site: cross-site→ 403sessionStorage(osiris.scannerAccessToken), andlocalStoragewas empty. The next scan sentX-Scanner-Tokenautomatically with no prompt; it returned 429 because my earlier tries had used up the 5/min scanner budget.🤖 Generated with Claude Code