-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: stabilize CSP #12859
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
base: v6
Are you sure you want to change the base?
feat: stabilize CSP #12859
Conversation
✅ Deploy Preview for astro-docs-2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
Thanks @ematipico ! We'll also need to add something to the v6 upgrade guide at https://deploy-preview-12322--astro-docs-2.netlify.app/en/guides/upgrade-to/v6/#experimental-flags This flag can be added to the block of all the flags being removed, then probably just a very similar entry under https://deploy-preview-12322--astro-docs-2.netlify.app/en/guides/upgrade-to/v6/#experimental-features-now-stable to the existing
|
sarah11918
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, @ematipico !
I think the bulk of the work is going to be figuring out how to make the config reference stuff as concise as possible, and I've left a bunch of suggestions about doing that.
Also, the experimental-flags/csp.mdx page can be completely deleted, and should be removed from the Astro sidebar
| Enabling this feature adds additional security to **Astro's handling of processed and bundled scripts and styles** by default, and allows you to further configure these, and additional, content types. | ||
|
|
||
| This experimental CSP feature has some limitations. Inline scripts are not supported out of the box, but you can [provide your own hashes](#hashes) for external and inline scripts. [Astro's view transitions](/en/guides/view-transitions/) using the `<ClientRouter />` are not supported, but you can [consider migrating to the browser native View Transition API](https://events-3bg.pages.dev/jotter/astro-view-transitions/) instead if you are not using Astro's enhancements to the native View Transitions and Navigation APIs. | ||
|
|
||
| :::note | ||
| Due to the nature of the Vite dev server, this feature isn't supported while working in `dev` mode. Instead, you can test this in your Astro project using `build` and `preview`. | ||
| ::: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll have to figure out how much of this we want to say here. Is it all still true with stabilization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the paragraph with a list of limitations
|
Just noting that some of the failed links are on the experimental CSP page (which will be deleted) and I've suggested fixes to the others above. So, those should end up OK! |
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
sarah11918
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This review is called "Death by a thousand indentations" 😆
I am going to commit all of these that are just fixing indentation, and let you review the rest so that it's not quite so overwhelming, @ematipico . But, this looks fantastic! I hope you're happy with it!
| ```js title="astro.config.mjs" | ||
| import { defineConfig } from 'astro/config'; | ||
|
|
||
| export default defineConfig({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be helpful if one of these examples showed e.g. unsafe-inline or something related to the subclasses, just so there's an example of both things!
|
|
||
| Inserts a new valid source to be used for the `script-src` directive. | ||
|
|
||
| ```astro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ```astro | |
| ```astro title="src/pages/index.astro" |
|
|
||
| Adds a new hash to the `script-src` directive. | ||
|
|
||
| ```astro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ```astro | |
| ```astro title="src/pages/index.astro" |
Description (required)
This PR stabilise the CSP APIs.
Moves the CSP APIs and configuration in the proper sections.
I only moved the content, I didn't update the current wording.
Also, I took the liberty to refactor the security section because it became quite lenghty, so I thought it required a proper header now.
For Astro version:
6.0. See astro PR #14946.