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

Fix CSP navigation request blocking #10949

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

Fix CSP navigation request blocking #10949

wants to merge 2 commits into from

Conversation

domenic
Copy link
Member

@domenic domenic commented Jan 27, 2025

Closes #10796, by passing along the intended snapshotted source CSP instead of attempting to look up the policy container from the request (which will not work when it's left as "client").

I'm omitting the PR template in favor of treating this as a clear bug fix, as the spec just doesn't work without this.

@mbrodesser-Igalia, can you take a look at this? It will need a corresponding update to the CSP algorithm, which I think you started on in w3c/webappsec-csp#692 (/cc @antosart and @ciaramcmullin). Can you update that PR to take the policy container argument? (Or should we just send the CSP list directly instead of the policy container?)


/browsing-the-web.html ( diff )

Closes #10796, by passing along the intended snapshotted source CSP instead of attempting to look up the policy container from the request (which will not work when it's left as "client").
@mbrodesser-Igalia
Copy link
Member

I currently don't have capacity for this, due to a project switch.

CC @fred-wang

domenic added a commit to domenic/webappsec-csp that referenced this pull request Jan 28, 2025
This allows HTML to pass in the correct CSP list, to help fix whatwg/html#10796.

See also: whatwg/html#10949 and w3c#494.

Closes w3c#692 by superseding it.
domenic added a commit to domenic/webappsec-csp that referenced this pull request Jan 28, 2025
This allows HTML to pass in the correct CSP list, to help fix whatwg/html#10796.

See also: whatwg/html#10949 and w3c#494.

Closes w3c#692 by superseding it.
@domenic
Copy link
Member Author

domenic commented Jan 28, 2025

No worries. I put up a PR to CSP at w3c/webappsec-csp#705 which should take care of it.

Marking "do not merge yet" until that is merged, but this is otherwise ready for review.

@domenic domenic requested a review from domfarolino January 28, 2025 06:45
@antosart
Copy link
Member

You probably already thought about this, but I wanted to ask whether it wouldn't be better to correctly set navigation request's policy container so that it an be used as is by CSP instead of passing the source document csp list explicitly.

@domenic
Copy link
Member Author

domenic commented Jan 29, 2025

You probably already thought about this, but I wanted to ask whether it wouldn't be better to correctly set navigation request's policy container so that it an be used as is by CSP instead of passing the source document csp list explicitly.

I'm honestly unsure. I find the whole thing where "client" is set as the default a bit confusing. If we say that the abstraction boundary is that the fetch spec should be the only one updating the request objects, then probably having the HTML spec mess with the policy container is bad? But maybe the manual navigation handling is so special anyway that we should do it? @annevk any thoughts?

@antosart
Copy link
Member

You probably already thought about this, but I wanted to ask whether it wouldn't be better to correctly set navigation request's policy container so that it an be used as is by CSP instead of passing the source document csp list explicitly.

I'm honestly unsure. I find the whole thing where "client" is set as the default a bit confusing. If we say that the abstraction boundary is that the fetch spec should be the only one updating the request objects, then probably having the HTML spec mess with the policy container is bad? But maybe the manual navigation handling is so special anyway that we should do it? @annevk any thoughts?

I just find it a bit weird that we'd end up having both the directly passed csp list and the request's policy container's csp list and it could end up being slightly confusing. For how I see it, it would be clean for request's policy container to always contain the policies of the context document which is performing the request (both for navigations and for subresource requests). It's convenient to have fetch it at a single place, but the navigation case is so special that that is just not possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants