-
Notifications
You must be signed in to change notification settings - Fork 313
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: meta based csp #228
base: main
Are you sure you want to change the base?
feat: meta based csp #228
Conversation
Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
|
|
|
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.
- track violations in RUM
- check for timing, violating resources might get loaded before CSP becomes effective
/** | ||
* Loads everything that doesn't need to be delayed. | ||
* @param {Element} doc The container element | ||
*/ | ||
async function loadLazy(doc) { | ||
await setCSP(); |
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'd check if this is effective in blocking eagerly loaded resources that violate the CSP.
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 am not sure that i am really that worried about the CSPs protecting block code...
to me this probably has to be in place before we execute untrusted code, eg. martech stack, so one could argue that this is in the wrong place altogether. so to a certain extent i think i am saying i don't even know when this should be loaded, so i am not really sure that an await
really makes a difference here...
i am somewhat doubtful of the allowlist based approach altogether, so maybe a nonce based strict-dynamic
approach would be the right thing here, but i definitely don't know enough about the repercussions of that vis-a-vis the somewhat questionable benefit of CSPs around XSS attacks on franklin sites.
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.
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.
... 99.34% of hosts with CSP use policies that offer no benefit against XSS.
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.
Revisiting this, I think this is right, but only the first step. The next step should be @chicharr's lightweight tag management controlling the CSP
Co-authored-by: Lars Trieloff <[email protected]>
|
|
For the record: |
Used this approach across multiple projects. Works great. I would like for this to be default in boilerplate to encourage folks to use CSP. If someone adds something to the martech stack via a tag manager (Launch/GTM) or some iframe in the content via the Embed block, then they have to explicitly allow that in the CSP in code and create a PR which would run PSI checks against the site to ensure the new plugin is not impacting page performance. This will force better practices and ensure that it is easy to still maintain performance while also adding additional libraries to martech. |
setting a
CSP
that is only transported over the wire once and then cached on the client.manages the
CSP
in an easy to read JSON filehttps://main--helix-project-boilerplate--adobe.hlx.live/
vs.
https://meta-csp--helix-project-boilerplate--adobe.hlx.live/?rum=1