fix: deployment-surface hardening — Permissions-Policy, font pipeline, CSP headers - #278
Merged
Conversation
…on CF Pages + Docker Permissions-Policy: microphone=() is the empty allowlist — it disallows the microphone for every origin, including same-origin. Voice (hooks/useMicLevel.ts, hooks/useSpeechRecognition.ts) calls getUserMedia/SpeechRecognition from same-origin code, so this silently killed Whisper STT, push-to-talk, and the mic-level meter on Cloudflare Pages and the Docker/nginx image. Fixed to microphone=(self) in both public/_headers and nginx.conf, with a regression test (tests/unit/deploymentHeaders.test.ts) that locks the value and asserts parity between the two hosts. Documented the full per-host header matrix (including GitHub Pages' inability to set any response header at all) in docs/DEPLOYMENT.md.
…to Sans GR does not exist) "Noto Sans GR" does not exist at Google Fonts — the combined CSS2 request for Noto+Sans+JP/KR/GR 400'd as a whole, silently dropping the JP and KR font requests alongside the invalid GR one. Japanese, Korean, and Greek got no webfont in the web build, and the request would have been CSP-blocked in the Tauri build regardless (font-src there never allowed fonts.gstatic.com). Self-hosts CJK/Greek via @fontsource, matching the existing Arabic/Hebrew pattern: @fontsource/noto-sans-jp, noto-sans-kr, noto-sans-sc (zh is Simplified Chinese per i18n/locales.ts), and the base @fontsource/noto-sans package's `greek` subset for el (replacing the nonexistent "Noto Sans GR" family entirely). Removes the Google Fonts <link>/preconnect tags and the fonts.googleapis.com/fonts.gstatic.com CSP allowances from index.html. New tests/unit/fontPipeline.test.ts asserts no live reference to a Google Fonts host or the dead family name remains, and that every --font-ui-* family in index.css has a matching @fontsource import. Verified pnpm run build + bundle:budget stay green — @fontsource CSS registers @font-face rules against separate .woff2 assets, so it doesn't inflate the JS chunks the budget checks.
…rrect the ADR ADR-0004's Consequences section claimed "the host (Vercel/CF) tightens CSP further via HTTP response headers in production" — this was false: none of vercel.json, public/_headers, or nginx.conf set a Content-Security-Policy header, so the accepted connect-src residual risk had no compensating control, and GitHub Pages (the canonical upstream mirror) cannot set one at all regardless. Adds a real Content-Security-Policy header to all three hosts that can set one, identical to the index.html meta CSP so there's nothing to diverge on. connect-src is unchanged (ADR-0004's BYOK tradeoff still applies there); the actual new hardening is frame-ancestors 'none', which only takes effect as a header and was previously inert in the meta tag. Corrects ADR-0004 and docs/SECURITY-THREAT-MODEL.md to state the real control (and the GitHub Pages limitation) instead of the false claim, and extends tests/unit/csp.test.ts to assert the three header CSPs exist, match each other, and are never looser than the meta CSP for any shared directive.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request self-hosts CJK and Greek fonts, removes Google Fonts references from CSP and HTML, adds aligned deployment security headers with regression tests, updates security documentation, and refreshes README test and i18n metrics. ChangesSelf-hosted font pipeline
Deployment security headers
README project metrics
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
tests/unit/fontPipeline.test.ts (1)
113-118: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the Greek subset import explicitly.
Noto Sanscurrently passes with any@fontsource/noto-sans/import, including a Latin-only stylesheet. Require agreek-*import for this family so removal of Greek coverage fails the regression test.Proposed fix
+const REQUIRED_IMPORT_PREFIXES: Readonly<Record<string, string>> = { + 'Noto Sans': "'`@fontsource/noto-sans/greek-`", +}; + // e.g. "Noto Sans JP" -> `@fontsource/noto-sans-jp`; "Noto Sans" -> `@fontsource/noto-sans` const pkg = `@fontsource/${family.toLowerCase().replace(/\s+/g, '-')}`; +const importPrefix = REQUIRED_IMPORT_PREFIXES[family] ?? `'${pkg}/`; expect( - indexTsx.includes(`'${pkg}/`), + indexTsx.includes(importPrefix),🤖 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 `@tests/unit/fontPipeline.test.ts` around lines 113 - 118, Update the font import assertion in the relevant font pipeline test so the Noto Sans family specifically requires an `@fontsource/noto-sans/greek-`* import, rather than accepting any Noto Sans stylesheet. Preserve the existing generic package assertion for other families and continue allowing configured system fallbacks.vercel.json (1)
25-29: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSecurity Misconfiguration (CWE-16)
Reachability: External
CSP header addition looks correct; consider closing the Permissions-Policy gap on Vercel.
The added CSP string matches
public/_headersandnginx.confbyte-for-byte (as asserted by the newtests/unit/csp.test.tssuite). However, this file still has noPermissions-Policyheader at all, while Cloudflare Pages and the Docker/nginx image now both restrictcamera=()/geolocation=()and allowmicrophone=(self).docs/DEPLOYMENT.md's new table documents this as "not set" for Vercel, but it means a Vercel deployment doesn't get the same camera/geolocation hardening as the other two hosts that can set headers.🛡️ Proposed addition for host parity
{ "key": "X-Frame-Options", "value": "DENY" }, { "key": "Referrer-Policy", "value": "strict-origin-when-cross-origin" }, + { "key": "Permissions-Policy", "value": "camera=(), microphone=(self), geolocation=()" }, { "key": "Content-Security-Policy",🤖 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 `@vercel.json` around lines 25 - 29, Add a Permissions-Policy header to the Vercel headers configuration, matching the policy used by Cloudflare Pages and nginx: disable camera and geolocation while allowing microphone for self. Keep the existing CSP and other security headers unchanged.tests/unit/deploymentHeaders.test.ts (1)
1-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate config-parsing helpers across the two new deployment-header test suites. Both files independently implement the same "narrow a regex match, then read
public/_headers/nginx.confand regex-extract a header value" pattern — the shared root cause is a missing common test utility for parsing these deployment config files.
tests/unit/deploymentHeaders.test.ts#L1-L38:group1,headersPolicyValue(), andnginxPolicyValue()duplicate the existing helpers/pattern fromtests/unit/csp.test.ts; import a sharedgroup1plus genericextractHeaderValue(source, headerName)from a newtests/utils/module instead.tests/unit/csp.test.ts#L25-L28,102-155: keepgroup1here (or move it to the shared module) and havevercelCsp()/headersCsp()/nginxHeaderCsp()reuse the same generic extractor so both suites read from one source of truth.🤖 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 `@tests/unit/deploymentHeaders.test.ts` around lines 1 - 38, Introduce a shared tests/utils helper containing group1 and generic extractHeaderValue(source, headerName), then update tests/unit/deploymentHeaders.test.ts (lines 1-38) to import and use them for both policy readers. Update tests/unit/csp.test.ts (lines 25-28 and 102-155) to reuse the same extractor in vercelCsp(), headersCsp(), and nginxHeaderCsp(), keeping group1 shared or locally only if appropriate.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@docs/DEPLOYMENT.md`:
- Around line 116-130: Correct the CSP explanation in the “Header invariants per
host” section: replace the claim that the header wins and makes the meta CSP
misleading with the accurate behavior that HTTP and meta CSP policies are both
enforced, using the most restrictive applicable rules. Preserve or clarify the
existing directive-specific exception that meta CSP cannot enforce
frame-ancestors and sandbox, which require headers.
In `@index.css`:
- Around line 33-39: The multiline QNBS-v3 comment format must be replaced with
the required single-line format: update the adjacent comment in index.css lines
33-39 to one /* QNBS-v3: … */ comment, and update the adjacent block comment in
index.tsx lines 35-51 to one // QNBS-v3: [Grund / Impact / Kreativer Mehrwert]
comment.
In `@nginx.conf`:
- Around line 30-36: Update the nginx child location blocks for static assets
and sw.js to preserve the server-level security headers when add_header
Cache-Control is present. Either duplicate X-Frame-Options,
X-Content-Type-Options, Referrer-Policy, Permissions-Policy, and
Content-Security-Policy in those blocks, or use add_header_inherit merge when
nginx 1.29.3+ is guaranteed.
In `@README.md`:
- Line 501: Update the README test-directory metric near the unit tree entry to
accurately describe the scope counted by scripts/sync-readme-metrics.mjs: either
identify the tests, components, and package test directories included, or
replace the value with a tests/unit-only count.
---
Nitpick comments:
In `@tests/unit/deploymentHeaders.test.ts`:
- Around line 1-38: Introduce a shared tests/utils helper containing group1 and
generic extractHeaderValue(source, headerName), then update
tests/unit/deploymentHeaders.test.ts (lines 1-38) to import and use them for
both policy readers. Update tests/unit/csp.test.ts (lines 25-28 and 102-155) to
reuse the same extractor in vercelCsp(), headersCsp(), and nginxHeaderCsp(),
keeping group1 shared or locally only if appropriate.
In `@tests/unit/fontPipeline.test.ts`:
- Around line 113-118: Update the font import assertion in the relevant font
pipeline test so the Noto Sans family specifically requires an
`@fontsource/noto-sans/greek-`* import, rather than accepting any Noto Sans
stylesheet. Preserve the existing generic package assertion for other families
and continue allowing configured system fallbacks.
In `@vercel.json`:
- Around line 25-29: Add a Permissions-Policy header to the Vercel headers
configuration, matching the policy used by Cloudflare Pages and nginx: disable
camera and geolocation while allowing microphone for self. Keep the existing CSP
and other security headers unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad06f798-5f92-41e1-b664-ded24a15d493
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
README.mddocs/DEPLOYMENT.mddocs/SECURITY-THREAT-MODEL.mddocs/adr/0004-csp-connect-src-byok-tradeoff.mdindex.cssindex.htmlindex.tsxnginx.confpackage.jsonpublic/_headerstests/unit/csp.test.tstests/unit/deploymentHeaders.test.tstests/unit/fontPipeline.test.tsvercel.json
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- CodeQL (high severity, "incomplete multi-character sanitization"): make tests/unit/fontPipeline.test.ts's stripComments() loop until a fixed point instead of a single regex pass, so a comment marker nested inside another can't survive removal. - CodeRabbit: vercel.json had no Permissions-Policy header at all — Vercel is the primary host, so it never got the microphone=(self) fix from the prior commit. Added it, matching Cloudflare Pages/nginx, and extended deploymentHeaders.test.ts to assert parity across all three hosts. - CodeRabbit: nginx's asset-caching and /sw.js location blocks each set their own Cache-Control via add_header, which (per nginx's inheritance rule) drops every server-level add_header for requests matching them — X-Frame-Options, CSP, and Permissions-Policy were silently absent on those paths. Duplicated the security headers into both blocks. - CodeRabbit: tightened the fontPipeline "Noto Sans" family check to require an actual `greek-` subset import, not just any noto-sans stylesheet. - CodeRabbit: corrected docs/DEPLOYMENT.md's CSP-precedence claim — a header and meta CSP are enforced simultaneously (strictest per-directive wins), not "the header wins and the meta tag is ignored"; frame-ancestors is the one directive that only works as a header at all. - CodeRabbit: corrected README.md's test-file-count line, which attributed the repo-wide count (tests/, components/, packages/*/tests/) to the tests/unit/ folder specifically. - CodeRabbit (nitpick): extracted the duplicated header-parsing helpers from csp.test.ts and deploymentHeaders.test.ts into tests/utils/deploymentConfigParsers.ts. - QNBS-v3 convention: condensed three multi-line comments (index.html, index.css, index.tsx) introduced by the previous commit into single lines. Also found and reverted an unrelated pre-existing bug surfaced while investigating this: scripts/sync-readme-metrics.mjs sums per-file leaf counts (2854) while scripts/check-i18n-keys.mjs deduplicates via a Set (2844) — 10 keys are defined in both `settings.json` and `common.json`/`dashboard.json` for the same locale, and diverge in translation for several non-EN locales (the `es` copies are literally in German). Kept README at the canonical 2844; the duplicate-key cleanup itself is a separate, larger fix and out of scope here — noted for follow-up.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@tests/unit/deploymentHeaders.test.ts`:
- Around line 37-50: Update the Permissions-Policy assertions in the microphone
and camera/geolocation tests to parse each policy into directives and compare
the exact values for microphone=(self), camera=(), and geolocation=(). Replace
substring-based toContain checks so broader or duplicate directives cannot pass,
while retaining validation across vercelPolicyValue(), headersPolicyValue(), and
nginxPolicyValue().
In `@tests/utils/deploymentConfigParsers.ts`:
- Around line 14-40: Insert a single-line `// QNBS-v3: ...` rationale comment
immediately before the helper block beginning with `extractHeadersFileValue`,
covering all three shared deployment-config parser helpers. Do not alter the
parsing logic or add additional markers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a3cef1c-77b1-4738-b60c-d9504e3d1db5
📒 Files selected for processing (11)
README.mddocs/DEPLOYMENT.mdindex.cssindex.htmlindex.tsxnginx.conftests/unit/csp.test.tstests/unit/deploymentHeaders.test.tstests/unit/fontPipeline.test.tstests/utils/deploymentConfigParsers.tsvercel.json
🚧 Files skipped from review as they are similar to previous changes (6)
- vercel.json
- index.html
- index.tsx
- tests/unit/csp.test.ts
- nginx.conf
- tests/unit/fontPipeline.test.ts
Merged
6 tasks
- CodeQL (still flagged after the first do-while fix): split
stripComments() into two independent stripToFixedPoint() calls, one per
regex, each shaped exactly like CodeQL's own recommended remediation
example — the previous version chained both replace() calls before
checking the fixed point, which likely obscured the loop invariant from
its dataflow analysis for one of the two patterns.
- CodeRabbit: deploymentHeaders.test.ts's Permissions-Policy assertions used
toContain('microphone=(self)'), which would also pass for a broader
allowlist (`microphone=(self https://evil.example)`) or a duplicated
directive where a later, looser value is the one actually in effect. Added
parsePermissionsPolicy() (directive -> value map, last occurrence wins) to
the shared test utils and switched to exact per-directive assertions.
- CodeRabbit: added the missing QNBS-v3 marker to
tests/utils/deploymentConfigParsers.ts, and escaped the header-name
argument before interpolating it into `new RegExp(...)` — it's always a
fixed string literal at every current call site, never untrusted input,
but escaping costs nothing and satisfies static analysis that can't verify
that invariant across call sites on its own.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
qnbs
enabled auto-merge (squash)
July 28, 2026 21:15
3 tasks
qnbs
added a commit
that referenced
this pull request
Jul 28, 2026
#282) Merging PR #278 broke main: the push-triggered CI run's "Attest build provenance" step failed with "Too many subjects specified (>1024)". This step never runs on pull_request events (only push to main), so the PR's own checks looked fully green and the regression only surfaced after merge. Root cause: PR #278 self-hosts CJK fonts (@fontsource noto-sans-jp/kr/sc) to fix the broken Google Fonts request. Those packages ship one small .woff2 file per Unicode-range subset for efficient lazy-loading — 2278 font files in dist/ alone (2150 Noto-branded), against 195 everything else, well past actions/attest-build-provenance's 1024-subject limit. Fonts are vendored static assets, not application code that needs per-file SLSA provenance — excluding woff/woff2 from subject-path still attests every JS/CSS/HTML/JSON/wasm file that actually makes up the deployed application.
8 tasks
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
2 participants
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.
Summary
Three P0 fixes, all in the gap between build artifact and deployment surface (no existing CI gate — E2E/Lighthouse run against
vite preview, where none of_headers/nginx.conf/Tauri CSP apply):microphone=()(empty allowlist) silently killed Voice (Whisper STT, push-to-talk, mic-level meter) on Cloudflare Pages + the Docker/nginx image, since it blocks same-origingetUserMedia/SpeechRecognitiontoo. Fixed tomicrophone=(self)in both hosts.Noto Sans GRdoesn't exist at Google Fonts — the combined CSS2 request 400'd, silently dropping the JP/KR requests alongside it, and it would've been CSP-blocked in Tauri regardless. Self-hosts CJK (JP/KR/SC) + Greek via@fontsource, matching the existing Arabic/Hebrew pattern; drops the Google Fonts CDN entirely.vercel.json/_headers/nginx.confset one. Adds a realContent-Security-Policyheader (identical to the meta CSP) to all three; the actual new hardening isframe-ancestors 'none', inert as a meta tag. Corrects the ADR andSECURITY-THREAT-MODEL.md, and documents that GitHub Pages (the canonical mirror) cannot set any response header at all.Each fix carries its own regression test:
tests/unit/deploymentHeaders.test.ts(new),tests/unit/fontPipeline.test.ts(new),tests/unit/csp.test.ts(extended).Test plan
pnpm run lint— 0 diagnosticsnode scripts/check-suppressions.mjs— 52/52, unchangednpx tsgo --project tsconfig.tsgo.json --noEmit --checkers 4— 0 errorspnpm run token:audit— 160 ≤ 165 baselinepnpm run test:run— 527 files / 6310 tests passing (incl. the 3 new/extended files, 19 tests)pnpm run build && pnpm run bundle:budget— green, fonts don't inflate JS chunks🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores