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

Add SRI support #199

Closed
wants to merge 2 commits into from
Closed

Add SRI support #199

wants to merge 2 commits into from

Conversation

botandrose
Copy link
Contributor

@botandrose botandrose commented Oct 22, 2023

Summary

This is a proof-of-concept for adding optional Sub-Resource Integrity support via a new --integrity command line option, and a new :integrity key in config/importmap.rb:

bin/importmap pin md5 --integrity

When this option is enabled, the SRI checksum is calculated and added to config/importmap.rb:

pin "md5", to: "https://cdn.jsdelivr.net/npm/md5@2.3.0/md5.js", integrity: "sha384-oqVuAfXRKap7fdgcCY5uykM6+R9GqQ8K/uxy9rx7HNQlGYl1kPzQho1wx4JwY8wC"

Which ultimately adds it to the HTML importmap, with two ESMS options enabled:

<script type="esms-options">{ "polyfillEnable": true, "enforceIntegrity": true }</script>
<script type="importmap-shim">...</script>
<link rel="modulepreload-shim" href="https://cdn.jsdelivr.net/npm/md5@2.3.0/md5.js" integrity="sha384-oqVuAfXRKap7fdgcCY5uykM6+R9GqQ8K/uxy9rx7HNQlGYl1kPzQho1wx4JwY8wC" />

More discussion here: #122

Drawbacks

  • Note that this requires the polyfillEnable: true option to be turned on so that the shim is used for ALL clients. Otherwise, this integrity checking is bypassed when using native implementations, e.g. Chrome. So there is a performance/security trade-off here.
  • The previous also requires that we append "-shim" to the relevant script[type] and link[rel] attributes. This breaks some other tooling that is looking for script[type='importmap'] exactly, e.g. { eagerLoadControllersFrom } from "@hotwired/stimulus-loading".

TODO:

  • More tests. This is mostly a proof-of-concept spike with only a few narrow unit tests, so I want to make sure that this plays well in all contexts.
  • Propshaft integration. Currently failing in CI.
  • Investigate whether or not its possible to work with ESMS to avoid appending "-shim" to script[type] and link[rel] attributes? ESMS has progressed since I dove deep last year, maybe this has changed.
  • Add documentation

Thoughts?

@dhh
Copy link
Member

dhh commented Jan 1, 2024

Version 2.0 is dropping the shim and also vendoring all pins. So I don't think this is relevant any longer. Appreciate you exploring it, though!

@dhh dhh closed this Jan 1, 2024
@botandrose
Copy link
Contributor Author

@dhh Ah, yeah, without the shim, we can't do SRI! While I still don't love the idea of checking all JS deps into the project repo, I do appreciate your ongoing push to make the JavaScript story in Rails simpler with less moving parts. Onward!

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.

3 participants