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

Drop unnecessary CSP directives for gold view #11605

Merged
merged 2 commits into from
Sep 18, 2024
Merged

Conversation

agjohnson
Copy link
Contributor

@agjohnson agjohnson commented Sep 17, 2024

This does not seem needed, as there is no inline script src in
subscription_detail.html. It seems like maybe we wrote this for
subscription_form.html, which was old.

This conditional was breaking the view for me locally, as we don't have
any CSP directives for script-src locally, we only have these in
production. Because of this, there are no other script-src exceptions.

This does not seem needed, as there is no inline script src in
`subscription_detail.html`. It seems like maybe we wrote this for
`subscription_form.html`, which was old.

This conditional was breaking the view for me locally, as we don't have
any CSP directives for `script-src` locally, we only have these in
production. Because of this, there are no other `script-src` exceptions.
@agjohnson
Copy link
Contributor Author

agjohnson commented Sep 17, 2024

It's not clear if we actually hit an issue with this view in production or not:

I tested a full list of script-src minus "unsafe-inline" and was able to use this view. Before this change, the view was broken locally. We don't seem to actually use inline scripts in this view from what I can tell -- noted at readthedocs/ext-theme#407

@agjohnson agjohnson marked this pull request as ready for review September 17, 2024 21:25
@agjohnson agjohnson requested a review from a team as a code owner September 17, 2024 21:25
@agjohnson agjohnson merged commit 3179613 into main Sep 18, 2024
8 checks passed
@agjohnson agjohnson deleted the agj/gold-view-csp branch September 18, 2024 17:37
@agjohnson
Copy link
Contributor Author

This indeed was not required, I don't get the errors on this view anymore

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

Successfully merging this pull request may close these issues.

2 participants