UN-2238 [FIX] Enforce the frontend CSP (out of report-only mode) - #2245
Deepak-Kesavan wants to merge 3 commits into
Conversation
Sweep of the report-only policy on a live deployment (browsing + per-directive probes) turned up hosts the shipped bundle loads but no directive allows: - style-src/font-src: cdn.jsdelivr.net (Monaco CSS + codicon) - img-src: cdn.jsdelivr.net (emoji-datasource), ProductFruits, GTM, GA, reCAPTCHA assets, q.stripe.com - media-src: cdn.productfruits.com (new directive; default-src had no blob:) - connect-src: unpkg.com, api.productfruits.com, GA4 regional endpoints, reCAPTCHA api2, m.stripe.network - frame-src: googletagmanager ns.html, m.stripe.network ProductFruits' animations.css was the one violation observed in normal use; the rest belong to code paths and flows that were not exercised. Drops the bare `wss:` wildcard: socket.io connects to window.location.origin (GetStaticData getBaseUrl) and 'self' covers same-origin ws/wss per CSP3, verified with a ws:// probe against nginx serving this policy. Adds .claude/skills/csp-check so the policy can be re-checked against a build or a deployment when a frontend dependency changes.
 |
Renames the header from Content-Security-Policy-Report-Only to Content-Security-Policy. The policy value is byte-identical to the one the previous commit landed after the origin sweep -- the only change is that a resource from an unlisted origin is now blocked rather than logged. That sweep predated the shadcn/React 19 frontend migration, so it was rechecked against both bundles before flipping: - the OSS build (`bun run build`, 111 chunks): every external host it references is in the policy - the live us-central deployment (363 chunks, which includes the cloud plugin code the OSS build does not have): same The one host either scan turned up that no directive allows is react.dev, which appears only in React 19's error-message links and is never fetched -- added to scan_origins.py's IGNORED set rather than to the policy. Also refreshes the csp-check skill: it described a report-only policy and a `npm run build`/frontend/dist layout that is now `bun run build`/frontend/build.
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
Unstract test resultsPer-group results
Critical paths
|
praveen-formido
left a comment
There was a problem hiding this comment.
Review findings (correctness pass).
| # connect-src carries no `wss:` wildcard: socket.io connects to window.location.origin | ||
| # (frontend/src/helpers/GetStaticData.js getBaseUrl), and 'self' covers same-origin | ||
| # ws/wss per CSP3. | ||
| add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cdn.jsdelivr.net https://unpkg.com https://eu.i.posthog.com https://eu-assets.i.posthog.com https://www.googletagmanager.com https://www.google.com/recaptcha/ https://www.gstatic.com/recaptcha/ https://js.stripe.com https://app.productfruits.com; style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://app.productfruits.com; img-src 'self' data: blob: https://eu.i.posthog.com https://eu-assets.i.posthog.com https://cdn.jsdelivr.net https://app.productfruits.com https://cdn.productfruits.com https://www.googletagmanager.com https://www.google-analytics.com https://www.google.com/recaptcha/ https://www.gstatic.com/recaptcha/ https://q.stripe.com; font-src 'self' data: https://cdn.jsdelivr.net; connect-src 'self' blob: https://cdn.jsdelivr.net https://unpkg.com https://eu.i.posthog.com https://eu-assets.i.posthog.com https://www.googletagmanager.com https://www.google-analytics.com https://region1.google-analytics.com https://analytics.google.com https://www.google.com/recaptcha/ https://api.stripe.com https://m.stripe.network https://app.productfruits.com https://api.productfruits.com; frame-src 'self' https://www.google.com/recaptcha/ https://recaptcha.google.com https://www.googletagmanager.com https://js.stripe.com https://hooks.stripe.com https://m.stripe.network; media-src 'self' blob: https://cdn.productfruits.com; worker-src 'self' blob: https://unpkg.com https://cdn.jsdelivr.net; object-src 'none'; base-uri 'self'; form-action 'self' https://checkout.stripe.com; frame-ancestors 'self'" always; |
There was a problem hiding this comment.
Enforced img-src blocks operator-configured branding URLs. generate-runtime-config.sh exposes VITE_CUSTOM_LOGO_URL / VITE_FAVICON_PATH (and the REACT_APP_* fallbacks) to config.logoUrl / config.favicon. The on-prem Helm values in unstract-cloud say REACT_APP_CUSTOM_LOGO_URL: "" # Replace with logo URL. If an operator sets it to an absolute URL on their own host, for example https://cdn.acme.com/logo.svg, the <img> in TopNavBar.jsx and the <link rel=icon> set in index.jsx are both governed by img-src, which only allows 'self' and the listed vendor hosts. Once the header enforces, the logo is blocked, onError falls back to the default logo, and the favicon silently reverts. The bundle scan cannot see this because the value is supplied at runtime. Either document that these must be same-origin paths, or derive an img-src addition from the same env vars when the config is generated.
| # connect-src carries no `wss:` wildcard: socket.io connects to window.location.origin | ||
| # (frontend/src/helpers/GetStaticData.js getBaseUrl), and 'self' covers same-origin | ||
| # ws/wss per CSP3. | ||
| add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cdn.jsdelivr.net https://unpkg.com https://eu.i.posthog.com https://eu-assets.i.posthog.com https://www.googletagmanager.com https://www.google.com/recaptcha/ https://www.gstatic.com/recaptcha/ https://js.stripe.com https://app.productfruits.com; style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://app.productfruits.com; img-src 'self' data: blob: https://eu.i.posthog.com https://eu-assets.i.posthog.com https://cdn.jsdelivr.net https://app.productfruits.com https://cdn.productfruits.com https://www.googletagmanager.com https://www.google-analytics.com https://www.google.com/recaptcha/ https://www.gstatic.com/recaptcha/ https://q.stripe.com; font-src 'self' data: https://cdn.jsdelivr.net; connect-src 'self' blob: https://cdn.jsdelivr.net https://unpkg.com https://eu.i.posthog.com https://eu-assets.i.posthog.com https://www.googletagmanager.com https://www.google-analytics.com https://region1.google-analytics.com https://analytics.google.com https://www.google.com/recaptcha/ https://api.stripe.com https://m.stripe.network https://app.productfruits.com https://api.productfruits.com; frame-src 'self' https://www.google.com/recaptcha/ https://recaptcha.google.com https://www.googletagmanager.com https://js.stripe.com https://hooks.stripe.com https://m.stripe.network; media-src 'self' blob: https://cdn.productfruits.com; worker-src 'self' blob: https://unpkg.com https://cdn.jsdelivr.net; object-src 'none'; base-uri 'self'; form-action 'self' https://checkout.stripe.com; frame-ancestors 'self'" always; |
There was a problem hiding this comment.
GA4 endpoints are pinned to specific hosts, which breaks once the policy enforces. connect-src lists region1.google-analytics.com and analytics.google.com. gtag also sends to region1.analytics.google.com, and when Google Signals or ads features are on it sends to *.g.doubleclick.net and www.google.com/g/collect, /ccm/collect. Only /recaptcha/ is allowed on www.google.com. Google's published GA4 CSP guidance uses https://*.google-analytics.com https://*.analytics.google.com https://*.googletagmanager.com for this reason. The GTM container is also edited outside the repo, so a tag someone adds later in the GTM UI (Signals, Ads conversion, a Custom HTML tag) is dropped with no report collector to show it. Report-only mode hid this; enforcing turns it into silently lost analytics. Consider the documented wildcards for the Google analytics hosts.
| ```bash | ||
| cd .claude/skills/csp-check/scripts | ||
| python3 extract_policy.py # what the policy says today | ||
| python3 scan_origins.py --url https://us-central.unstract.com # or --dist frontend/build |
There was a problem hiding this comment.
--dist frontend/build after cd .claude/skills/csp-check/scripts gives a false pass. Run from this directory, the relative path resolves to scripts/frontend/build, which does not exist. read_dist calls Path.rglob on it, and rglob on a missing directory returns [] without raising. The script then prints Scanned 0 bundle files followed by Every external host referenced by the bundle appears in the policy. and exits 0. That is exactly the gate this skill exists to provide, and it passes vacuously. Suggested fix: use --dist ../../../../frontend/build in the doc, and in scan_origins.py exit non-zero when args.dist is not a directory or when files is empty.
| } | ||
|
|
||
|
|
||
| def read_dist(dist: Path) -> dict[str, str]: |
There was a problem hiding this comment.
Silent empty scan (the code side of the SKILL.md issue). read_dist does not check that dist exists or contains anything. read_deployment has the same problem: every chunk fetch failure is only printed to stderr (for example an auth wall, a redirect to login, or a CDN 403). In both cases found ends up empty, uncovered is empty, and main prints "Every external host ... appears in the policy" with exit code 0. Fail when len(files) == 0, and treat fetch errors as a non-zero exit, or at least a warning that changes the exit code. Separately, only /assets/*.js|css is scanned. Hosts that reach the page through index.html or /config/runtime-config.js (the custom logo and favicon URLs, for example) are never checked.
| for sources in directives.values(): | ||
| for source in sources: | ||
| if source.startswith("https://"): | ||
| hosts.add(source[len("https://") :].split("/")[0]) |
There was a problem hiding this comment.
allowed_hosts drops the path from path-scoped sources, so the scanner over-reports coverage. https://www.google.com/recaptcha/ becomes www.google.com, and every bundle reference to https://www.google.com/... is then marked allowed. The same applies to www.gstatic.com. The browser only permits /recaptcha/... on those hosts. So a new dependency that fetches https://www.google.com/g/collect, or a Google Maps or fonts path on www.gstatic.com, passes the static check and is then blocked by the enforcing policy. Either keep the path and match URL prefixes (URL_RE would need to capture the path), or mark path-scoped hosts as "needs probe" rather than allowed.
What
Takes the frontend CSP out of report-only mode, which is what UN-2238 asks for. Two commits:
wss:wildcard fromconnect-src. Adds.claude/skills/csp-check— scripts + procedure for re-checking the policy against a build or a running deployment (the "way to check compatibility of new features with the CSP policy" Hari asked for in the ticket).Content-Security-Policy-Report-OnlytoContent-Security-Policy. The policy value is byte-identical across that commit — the only change is that an unlisted origin is now blocked rather than logged.Why
UN-2238. CSP shipped in report-only mode with no report collector, so violations only ever reached each user's browser console. The ticket's exit condition is "once there are no more violations, we can take CSP out of report mode" — commit 1 closes the violations, commit 2 does the flip.
How
Commit 1 — what the sweep added
Two methods: browsing while recording
securitypolicyviolation(landing, dashboard, prompt studio incl. PDF viewer, API/ETL/task, logs, users, LLM settings, platform settings, profile, manual review), plus per-directive probes for hosts the bundle references whose code paths were not exercised. Each host below traces to a chunk in the deployed bundle.style-srccdn.jsdelivr.net,app.productfruits.comeditor.main.css; ProductFruits CSS (observed in normal use)font-srccdn.jsdelivr.netcodicon.ttfimg-srccdn.jsdelivr.net,app.productfruits.com,cdn.productfruits.com,www.googletagmanager.com,www.google-analytics.com,www.google.com/recaptcha/,www.gstatic.com/recaptcha/,q.stripe.comconnect-srcunpkg.com,api.productfruits.com,www.googletagmanager.com,region1.google-analytics.com,analytics.google.com,www.google.com/recaptcha/,m.stripe.networkframe-srcwww.googletagmanager.com,m.stripe.networkns.html; Stripe.jsmedia-src'self' blob: cdn.productfruits.comdefault-src 'self', noblob:)Commit 2 — rechecking before the flip
That sweep predates the shadcn / React 19 frontend migration (359 files), so the policy was rechecked against both bundles:
bun run build, 111 chunks) — every external host it references is in the policy.The one host either scan turned up that no directive allows is
react.dev, which appears only in React 19's error-message links and is never fetched. It went intoscan_origins.py'sIGNOREDset, not into the policy.The skill's docs were also refreshed: they described a report-only policy and an
npm run build/frontend/distlayout that is nowbun run build/frontend/build.Can this PR break any existing features. If yes, please list possible items. If no, please explain why.
Yes — this is the commit where a policy gap stops being a console message and starts being a broken feature. What was checked, and what is left:
animations.css), which commit 1 allows.window.location.href = ${origin}/api/v1/login— a navigation, whichform-actiondoes not govern. App forms are ReactonSubmithandlers with no native submit./api/v1/...paths and socket.io connects towindow.location.origin;'self'covers both, including same-origin ws/wss under CSP3.frame-ancestors 'self'was already enforced in practice by the existingX-Frame-Options: SAMEORIGIN.VITE_CUSTOM_LOGO_URLandVITE_FAVICON_PATH(viagenerate-runtime-config.sh) let a deployment point the logo/favicon at an arbitrary URL. Underimg-src, an external logo host now has to be added to the policy or the logo will not render. Same-origin values are unaffected..claude/skills/csp-checkcheck 3 is the procedure.Rollback is a one-line revert of commit 2.
Database Migrations
Env Config
VITE_CUSTOM_LOGO_URL/VITE_FAVICON_PATHnote above for an existing var whose behaviour now has a CSP constraint.Relevant Docs
.claude/skills/csp-check/SKILL.mdRelated Issues or PRs
Dependencies Versions
Notes on Testing
scan_origins.py --dist frontend/buildagainst a local production build of this branch: clean, 111 chunks.scan_origins.py --url https://us-central.unstract.com: clean, 363 chunks.extract_policy.pyparses the flipped header and reports 13 directives, and the policy string is byte-identical to the pre-flip one (verified by diffing the twoadd_headervalues).docker runofnginx:alpinewith thisnginx.conf— config accepted, header emitted verbatim; 48 per-directive browser probes against it — every listed origin loads with no report, and all 4 controls to a disallowed host were reported. The conf's directive structure is unchanged since, only the header name differs. Docker was not available in this round, so that check was not re-run.