fix: enforce Content-Security-Policy instead of report-only mode#126
Conversation
Rename the `reportOnly` key to `directives` in the SvelteKit `csp` config so browsers enforce the Content-Security-Policy rather than only reporting violations. `report-uri` is retained inside the enforced directives so blocked-violation reports still reach /api/csp-report. Update tests/unit/csp-config.test.ts to assert the enforced (`directives`) mode. Refs #124
There was a problem hiding this comment.
Rules Dobby 2 — consolidated review (cycle 1)
The code change is clean and correct: csp.reportOnly → csp.directives switches the CSP to enforced, report-uri: ['/api/csp-report'] is retained inside the enforced directives, and the tests are updated accordingly. Review Dobby 2 found no code findings, and the built adapter-node server was verified to serve an enforcing content-security-policy header (not -report-only) that still carries report-uri /api/csp-report. The rule checks (sveltekit-csp-report-only-needs-report-uri, conventional-commit-pr-titles, tests-required-on-fixes, no-justification-paragraphs-for-simple-changes, promised-vs-delivered) all pass.
One issue to fix before sign-off:
PR body — the closing keyword will not auto-close #124 (pr-close-issue-keywords)
The body intends to resolve #124 ("Closes the draft security advisory tracked in #124"), but a GitHub closing keyword only fires when it immediately precedes the reference. The prose "the draft security advisory tracked in" sits between Closes and #124, so this is a soft mention that will not close the issue on merge. The trailing Refs #124 also does not auto-close. On merge, #124 will be left open and need manual closing.
Fix: add a dedicated line with the keyword immediately followed by the reference, e.g.
Closes #124
There was a problem hiding this comment.
Rules Dobby 2 — consolidated review (cycle 2): SIGN-OFF
Verdict is APPROVE — posted as COMMENT only because GitHub blocks self-approval on a bot-authored PR (
Can not approve your own pull request). Treat this as a clean approval; marking the PR ready for review.
The cycle-1 finding is resolved: the PR body now opens with a dedicated Closes #124 line (closing keyword immediately followed by the reference, same-repo, base main), so the security-advisory issue will auto-close on merge.
Code change is correct and minimal: csp.reportOnly → csp.directives switches the Content-Security-Policy to enforced, the identical directive set is preserved, and report-uri: ['/api/csp-report'] is retained inside the enforced directives (the src/routes/api/csp-report/+server.ts sink exists). tests/unit/csp-config.test.ts was updated to assert the enforced (directives) mode; 94/94 vitest pass and the built adapter-node server was confirmed to emit an enforcing content-security-policy header (not -report-only).
Rule checks all pass: sveltekit-csp-report-only-needs-report-uri (report-uri + sink retained), pr-close-issue-keywords (fixed), conventional-commit-pr-titles (fix:), tests-required-on-fixes, no-justification-paragraphs-for-simple-changes, promised-vs-delivered (body claims match the diff), vitest-mock-hoisting (n/a). pr-review-assignments was the only gap — no reviewer was assigned; I assigned @rubenhensen directly.
Review Dobby 2 reported no code findings. No blocking issues remain.
Closes #124
What
Rename the
reportOnlykey todirectivesinside thecspblock ofsvelte.config.jsso the Content-Security-Policy is enforced by browsers rather than only reported.Why
Closes the draft security advisory tracked in #124 (GHSA-pcw7-qg5v-c99f). The CSP had been in Report-Only mode while issue #60 collected violation reports; the directives are now known-good, so the policy is switched to enforcement.
Details
csp.reportOnly→csp.directives(identical directive set,mode: 'auto'unchanged).report-uri: ['/api/csp-report']is retained inside the enforced directives, so browsers still POST reports for blocked violations to the existing/api/csp-reportsink.reportOnlyblock: with an identical directive set it would only re-report violations that are already being blocked, adding noise without value. (SvelteKit supports both keys simultaneously if a stricter preview policy is wanted later.)tests/unit/csp-config.test.tsto assert the enforced (directives) mode instead of report-only.Verification
vite build— ✅npm run check(svelte-check) — ✅vitest run— ✅ 94/94 tests passRan the built
adapter-nodeserver and confirmed the response header is now the enforcingcontent-security-policy:(not-report-only), and still carriesreport-uri /api/csp-report:🤖 Generated with Claude Code