Add subresource integrity support for ES modules, through importmaps

[yoavweiss]

Based on a proposal by @guybedford.

SRI support for ES module imports would enable using them in documents that require SRI for certain scripts for security or privacy reasons.

• At least two implementers are interested (and none opposed):
  • Chromium - I'm implementing this.
  • WebKit - supportive, I'm implementing.
• Tests are written and can be reviewed and commented upon at:
  • Dynamic module imports.
  • Static module imports.
  • Non import modules
  • No referencing script
  • SW Request.integrity
• Implementation bugs are filed:
  • Chromium: https://issues.chromium.org/issues/334251999
  • Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1892199
  • WebKit: https://bugs.webkit.org/show_bug.cgi?id=272884
  • Deno: Add support for subresource integrity in imported module scripts denoland/deno#23521
  • Node.js: Add support for subresource integrity in imported module scripts nodejs/node#52662
• MDN issue is filed: Add support for subresource integrity in imported module scripts mdn/mdn#541
• The top of this comment includes a clear commit message to use.

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

---

/infrastructure.html ( diff )
/links.html ( diff )
/scripting.html ( diff )
/webappapis.html ( diff )

[domenic]

Some initial comments:

• I'd envisioned the keys being URLs, not specifiers. https://github.com/guybedford/import-maps-extensions?tab=readme-ov-file#integrity isn't super-clear (/cc @guybedford) but the extra indirection of the specifiers doesn't seem like a great idea to me here. I'm open to being persuaded though.

• Make sure that when you do the actual integrity checking, it applies to all imports, not just dynamic ones. (It looks like there's no integrity checking in the spec PR yet, but in the Chromium draft CL WPTs I only see dynamic import tests.)

[yoavweiss]

Thanks for taking a look!

• I'd envisioned the keys being URLs, not specifiers. https://github.com/guybedford/import-maps-extensions?tab=readme-ov-file#integrity isn't super-clear (/cc @guybedford) but the extra indirection of the specifiers doesn't seem like a great idea to me here. I'm open to being persuaded though.

That's a good point, and I'm currently using the resolved URLs as the keys to the internal lookup, which is admittedly a bit awkward. Moving to use the URLs explicitly makes sense to me, and will simplify the code a bit.

• It looks like there's no integrity checking in the spec PR yet

Good point, I missed adding it. Will do!

• in the Chromium draft CL WPTs I only see dynamic import tests

Yeah, I was planning on testing static imports as well (but ran out of power on the flight :P). Will add that before this will all be ready for review.

[yoavweiss]

@domenic - I believe this is now ready for review. Also, I marked Chromium as supportive given that I'm implementing this, but let me know if that requires more explicit support from the Chrome team.

[yoavweiss]

(Also, PR preview doesn't seem to update to the latest version for some reason)

[domenic]

Exciting!

[yoavweiss]

Thanks for reviewing!! Addressing some of the comments, to clear things up for the rest :)

[hiroshige-g]

The explainer says:

The "integrity" for any module request is looked up from this import maps integrity attribute whenever there is no integrity specified.
<script type="module" integrity="..."> integrity attribute on a module script will take precedence over the import map integrity.
The import map integrity will only apply to modules and not other assets.

But in the current PR (as well as the draft CL) the import-map-originating hashes are NOT applied to

• <script src="a.js" type="module">
• <link rel=modulepreload href="a.js">

Is this intentional, or spec/impl should be fixed?

[yoavweiss]

Is this intentional, or spec/impl should be fixed?

The functionality is intentional (as discussed with @domenic), but I missed the fact that the explainer doesn't align with what we decided on. But I guess we could also modify the spec/impl to handle that case.

@domenic - WDYT?

[domenic]

I slightly lean toward the version we have now where it only applies to "imports", not other "module loads". But, I don't feel strongly. @guybedford and other @whatwg/modules people, do you have thoughts?

[guybedford]

Has been great to follow this PR, excited to see this one.

I think supporting the import map integrity for top-level script loads would be useful. If we are to consider the import map as the source of truth, then updating the import map is enough to update all usage, without having to update every module script reference point. So it would be a net reduction in workflow friction. Implementation would be as described where explicit integrity on a tag acts as an override. If we ever support explicit integrity in import attributes one might expect similar there as well so it isn't necessarily unique to the top-level.

If going in that direction, preloads could be nice to support as well, to avoid needing to prune the import map integrity to the non-preloaded graph when generating preloads.

While we are working through edge cases - I assume this all applies equally to JSON imports? It would be good to ensure that is tested as well.

[domenic]

Pretty much all minor editorial things at this point, although we have a normative discussion about the interaction with other entry points too.

[domenic]

I think supporting the import map integrity for top-level script loads would be useful. If we are to consider the import map as the source of truth, then updating the import map is enough to update all usage, without having to update every module script reference point. So it would be a net reduction in workflow friction. Implementation would be as described where explicit integrity on a tag acts as an override. If we ever support explicit integrity in import attributes one might expect similar there as well so it isn't necessarily unique to the top-level.

I find this relatively compelling and in the absence of any counterarguments I'd switch to weakly being in favor of supporting this case.

I think the parts of the spec to modify are just the call sites of "fetch an external module script graph" and "fetch a modulepreload module script graph".

If going in that direction, preloads could be nice to support as well, to avoid needing to prune the import map integrity to the non-preloaded graph when generating preloads.

To be clear, I think we should support modulepreload, but not preload. (Or prefetch, or fetch(), or other call sites.)

If this is too confusing then maybe it's reason to reconsider... After all, we need to be cautious about @hiroshige-g's concern at https://groups.google.com/a/chromium.org/g/blink-dev/c/O2UR3kb-HcI/m/tWgtOEixAQAJ .

While we are working through edge cases - I assume this all applies equally to JSON imports? It would be good to ensure that is tested as well.

+1. It should work per spec, and adding one or two tests for JSON and CSS module scripts would be great.

[yoavweiss]

Thanks!!

[yoavweiss]

+1. It should work per spec, and adding one or two tests for JSON and CSS module scripts would be great.

I agree, but it seems like CSS and JSON module scripts are not at all tested with import maps (unless I'm missing something).

[yoavweiss]

I think the parts of the spec to modify are just the call sites of "fetch an external module script graph" and "fetch a modulepreload module script graph".

It might be cleaner to override the integrity metadata inside these calls in case it's the empty string (rather than at the call sites). WDYT?

[domenic]

It might be cleaner to override the integrity metadata inside these calls in case it's the empty string (rather than at the call sites). WDYT?

I think only the call sites will know whether the attribute is absent (should fall back) or the empty string (should probably use the empty string)?

[yoavweiss]

@annevk - any blockers/concerns for landing this?