fix(csp): drop script-src unsafe-inline, tighten CDN, disable Vite polyfill - #107
Open
DavidHLP wants to merge 4 commits into
Open
fix(csp): drop script-src unsafe-inline, tighten CDN, disable Vite polyfill#107DavidHLP wants to merge 4 commits into
DavidHLP wants to merge 4 commits into
Conversation
- script-src 'self' (was 'self' 'unsafe-inline') — the project ships zero inline scripts (index.html uses only external src= for theme-bootstrap.js + main.ts; PWA uses registerSW external module per console/src/pwa-register.ts). unsafe-inline neutralized CSP's XSS defense for no benefit (HIGH 4). - style-src: narrow https://cdn.jsdelivr.net to /npm/@fontsource/ (actual font source per shared/theme/src/typography.css:42-45) - style-src: add https://cdnjs.cloudflare.com/ajax/libs/highlight.js + /KaTeX/ (console/index.html:17,21 loads these — thana's CSP omitted cdnjs, which would block code-highlighting + math rendering under CSP) - font-src: narrow https://cdn.jsdelivr.net to /npm/@fontsource/ - style-src keeps 'unsafe-inline' (Vue scoped style + Tailwind v4 runtime injection require it; industry consensus for style is acceptable) Verified: nginx -t passes via official template mechanism on both console + management; envsubst correctly renders connect-src with injected API_ORIGIN (confirms CR HIGH 7 was correctly invalidated). Fixes #101
…t-free Vite's default build.modulePreload.polyfill=true injects a small inline <script type="module"> into the built index.html. That is the only inline script in the entire production bundle (all author code is external: index.html uses src= for theme-bootstrap.js + main.ts; PWA uses the external registerSW module). With the prior commit's CSP script-src 'self', the polyfill would be blocked — harmless on modern browsers (which support <link rel=modulepreload> natively) but it produces CSP violation noise in the browser console and undermines the "zero inline scripts" claim at the build-output level. Disable the polyfill. UltiCode already targets only modern browsers (Vite 8 / Vue 3.5 / Tailwind v4 / Monaco all require ES2020+), so the polyfill serves no real audience. Combined with df5fe20, the production bundle now has zero inline scripts and CSP `script-src 'self'` applies cleanly. Refs #101
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a0e8b009f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| # Security headers (server-level — inherited by all locations) | ||
| add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; img-src 'self' data: https:; font-src 'self' https://cdn.jsdelivr.net data:; connect-src 'self' ${API_ORIGIN}; frame-ancestors 'self'" always; | ||
| add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net/npm/@fontsource/ https://cdnjs.cloudflare.com/ajax/libs/highlight.js/ https://cdnjs.cloudflare.com/ajax/libs/KaTeX/; img-src 'self' data: https:; font-src 'self' https://cdn.jsdelivr.net/npm/@fontsource/ data:; connect-src 'self' ${API_ORIGIN}; frame-ancestors 'self'" always; |
There was a problem hiding this comment.
For the console page, console/index.html loads KaTeX CSS from https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.16.9/katex.min.css, and that stylesheet references its font files under the same cdnjs KaTeX path. This CSP now allows the stylesheet in style-src, but font-src still only allows self, jsdelivr, and data URLs, so browsers will reject those KaTeX font requests under CSP and math rendering can remain degraded in production. Add the cdnjs KaTeX font path to font-src as well, or stop loading the external KaTeX stylesheet.
Useful? React with 👍 / 👎.
console's `pnpm audit --prod --audit-level high` failed on this PR's CI: markdown-it > linkify-it <=5.0.0 has a high-severity ReDoS in the LinkifyIt#match scan (quadratic complexity). Patched in linkify-it >=5.0.1. markdown-it ^14.1.1's linkify-it range already allows 5.0.1, but the lockfile was pinned at 5.0.0. Add an explicit pnpm.overrides entry — matching the existing pattern (dompurify, lodash-es, protocol-buffers-schema, vite, form-data) — to force ^5.0.1 across the dependency tree. Pre-existing on main; exposed here only because this PR touches console/ files and is the first of the 5 fix branches to trigger the console audit job (the others don't modify console/). Verified locally: pnpm audit --prod --audit-level high → 0 high (was 1), 10 remaining (4 low + 6 moderate, below threshold). Lockfile diff minimal (linkify-it + its direct dependents only, +11/-9). Refs #101
Follow-up to df5fe20 (caught on self-review of all PRs). console/index.html:21 loads katex.min.css from cdnjs.cloudflare.com. That stylesheet declares @font-face with relative url() — e.g. url(fonts/KaTeX_AMS-Regular.woff2) — so browsers fetch KaTeX math fonts from https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.16.9/fonts/. The prior font-src ('self' + jsdelivr/npm/@fontsource/ + data:) would have blocked these, making math glyphs fall back to system fonts. Add the precise cdnjs KaTeX prefix. management/ intentionally keeps the narrower font-src: its index.html has no cdnjs <link> and no KaTeX-from-CDN dependency (least privilege). highlight.js github-dark CSS verified to contain no url()/@font-face (pure color rules), so no cdnjs/highlight.js entry is needed in font-src. Refs #101
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
1 participant
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.
script-srcback to'self'(project ships zero author-written inline scripts)/npm/@fontsource/(actual font source pershared/theme/src/typography.css:42-45)console/index.html:17,21loads highlight.js + KaTeX — thana's CSP omitted these, breaking code highlighting + math rendering under CSP)modulePreload.polyfill=false(the only inline script Vite injects at build time)Verified:
nginx -tpasses via the official template mechanism; envsubst rendersconnect-srcwith the injectedAPI_ORIGIN.Closes #101