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: img csp nonce #459

Merged
merged 1 commit into from
Dec 4, 2024
Merged

fix: img csp nonce #459

merged 1 commit into from
Dec 4, 2024

Conversation

yordis
Copy link
Contributor

@yordis yordis commented Dec 3, 2024

Context

I noticed that my CSP configuration was not working, and then I realized that the value of nonce for these images was empty.

Digging into it, I also found this mdn/sprints#3650 (comment)

Tested

  • Arc
  • Chrome
  • Safari
  • Firefox

@josevalim
Copy link
Member

Thank you for the report. The best solution is probably to move the image into CSS, either into assets, or some inline style:

<style nonce="your-generated-nonce">
  .data-image {
    background-image: url('data:image/png;base64,...');
  }
</style>
<div class="data-image"></div>

@yordis
Copy link
Contributor Author

yordis commented Dec 3, 2024

@josevalim will do that, no problem. Aside from that, do you have any clue how could be done without CSS? I tried to find the answer but I failed. Asking to document here the learning for the next person, or in cases we couldn't move it to CSS

@josevalim
Copy link
Member

The only other option I found was allowing data attributes in the CSP definition, but we probably don't want to impose that.

@yordis yordis force-pushed the fix-img-csp branch 2 times, most recently from 71789cb to 5c74a98 Compare December 3, 2024 21:09
@yordis yordis marked this pull request as ready for review December 3, 2024 21:11
@yordis
Copy link
Contributor Author

yordis commented Dec 3, 2024

@josevalim ready for review

@josevalim
Copy link
Member

@yordis can you please take a look at the tests? Thanks! I also think we should probably remove the img nonce from our docs and similar. It is not used anywhere anyway.

@josevalim josevalim merged commit d284dab into phoenixframework:main Dec 4, 2024
4 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

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