Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Enable adapter-cloudflare to specify Content-Security-Policy in _headers file #13290

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

OptoCloud
Copy link

This will add the configured CSP directives to cloudflares _header file so that they get served with every request, no matter if its a static file or server rendered, these CSP headers will compound with the tags.

Tools like Google CSP Evaluator Seems to only pick up on CSP headers, so serving statically rendered sites will lead it to show that no CSP is defined.

This will also make sure that all files served are under the CSP that is defined in the svelte config


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

Copy link

changeset-bot bot commented Jan 8, 2025

🦋 Changeset detected

Latest commit: 0c0ad89

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@sveltejs/adapter-cloudflare Minor
@sveltejs/kit Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Rich-Harris
Copy link
Member

preview: https://svelte-dev-git-preview-kit-13290-svelte.vercel.app/

this is an automated message

@OptoCloud OptoCloud changed the title Enable adapter-cloudflare to specify Content-Security-Policy in _header file feat: Enable adapter-cloudflare to specify Content-Security-Policy in _header file Jan 8, 2025
@OptoCloud OptoCloud changed the title feat: Enable adapter-cloudflare to specify Content-Security-Policy in _header file feat: Enable adapter-cloudflare to specify Content-Security-Policy in _headers file Jan 8, 2025
generateCspHeaderValue() {
if (!config.kit.csp.directives) return null;

const csp = new Csp(config.kit.csp, { prerender: false });
Copy link
Member

@eltigerchino eltigerchino Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be generatePrerenderedCspHeaderValue() instead and prerender should be true? In the Cloudflare adapter case, it's primarily being used when serving prerendered pages.

When mode is 'auto', SvelteKit will use nonces for dynamically rendered pages and hashes for prerendered pages. Using nonces with prerendered pages is insecure and therefore forbidden.

https://svelte.dev/docs/kit/configuration#csp

Also, I'm not sure if this would work. Don't we need to add the script and style sources before we generate the header?

csp.add_script(init_app);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think you're right, but I don't think it would matter as the CSP instance here will not be taking any script or style tags as input (unless prerendered pages are analyzed and their hashes are added to it).
Also on second thought the cloudflare _headers csp would need to allow unsafe-inline for script and style directives unless we do that analysis...
At that point I'm not sure if this PR is worth it 🤔

@eltigerchino eltigerchino added the feature / enhancement New feature or request label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants