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

Update CSM destinations to CSP Level 3. #1536 #1536

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

Conversation

ToniWilliams1
Copy link

@ToniWilliams1 ToniWilliams1 commented Nov 12, 2022

  • At least two implementers are interested (and none opposed):

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

@annevk
Copy link
Member

annevk commented Nov 14, 2022

CSP still defines style-src and such too. If you're looking to contribute I recommend starting with good first issues.

@annevk annevk closed this Nov 14, 2022
@ToniWilliams1
Copy link
Author

I actually did update the CSP specifications for the entire row, sorry for not clarifying that

@annevk
Copy link
Member

annevk commented Nov 14, 2022

That's alright, I looked at the entire PR, but none of the changes look correct to me.

@ToniWilliams1
Copy link
Author

ToniWilliams1 commented Nov 14, 2022

Got it, thank you for the update. I would still love to work on this issue, should this also update the additional destinations such as "frame"AND "iFrame" since they both return "frame-src"? From what I understand, the issue asks to update the list to Content Security Policy Level 3. The ref (https://w3c.github.io/webappsec-csp/#effective-directive-for-a-request) shows destinations such as 'script' to return 'script-src-elem' and more from the CSP level 3 list. If there is something I am not understanding, please let me know. Thank you!

@annevk
Copy link
Member

annevk commented Nov 14, 2022

@ToniWilliams1 apologies, I missed #1466. Next time please mention the issue you are attempting to fix.

We probably don't want to remove script-src however, but instead add script-src-elem to it.

@annevk annevk reopened this Nov 14, 2022
PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
fetch.bs Outdated
@@ -1654,11 +1654,11 @@ not always relevant and might require different behavior.
<code>cursor</code>, CSS' <code>list-style-image</code>, …
<tr>
<td>"<code>audioworklet</code>"
<td><code>script-src</code>
<td><code>script-src-elem</code>
Copy link
Member

Choose a reason for hiding this comment

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

Let's list both, separated by a comma. Also, it seems you forgot the script-src for the script element below.

Copy link
Author

Choose a reason for hiding this comment

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

I've now listed both style-src and style-src-elem. Also added the script-src for the script element below.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see that reflected when I click on "Files changed" above.

Copy link
Author

Choose a reason for hiding this comment

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

is this issue still open?

fetch.bs Outdated
@@ -1678,11 +1678,11 @@ not always relevant and might require different behavior.
<td><code>Federated Credential Management requests</code>
<tr>
<td>"<code>worker</code>"
<td><code>child-src</code>, <code>script-src</code>, <code>worker-src</code>
<td><code>worker-src</code>, <code>script-src</code>, <code>worker-src</code>
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this change. It already lists worker-src and it should continue to list child-src.

fetch.bs Outdated
<td><code>Worker</code>
<tr>
<td>"<code>style</code>"
<td><code>style-src</code>
<td><code>style-src-elem</code>
Copy link
Member

Choose a reason for hiding this comment

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

Let's list both here, as with script, separated by a comma.

Copy link
Author

Choose a reason for hiding this comment

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

I've now added both style-src and stly-src-elem

@ToniWilliams1 ToniWilliams1 marked this pull request as draft November 14, 2022 15:49
@ToniWilliams1 ToniWilliams1 changed the title Update CSM destinations to CSP Level 3. Update CSM destinations to CSP Level 3. #1536 Nov 14, 2022
@ToniWilliams1 ToniWilliams1 marked this pull request as ready for review November 14, 2022 16:18
@ToniWilliams1 ToniWilliams1 marked this pull request as draft December 26, 2022 21:16
@ToniWilliams1 ToniWilliams1 marked this pull request as ready for review December 26, 2022 21:17
@annevk
Copy link
Member

annevk commented Jan 3, 2023

Hey @ToniWilliams1 if you go to "Files changed" above you'll find that you have still changed two files. You will need to undo the changes to PULL_REQUEST_TEMPLATE.md.

There's also still one script-src case where you only listed one variant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants