-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
CSS and JSON module scripts and CSP #7233
Comments
This definitely seems strange to me and like something that should be fixed - that CSS imports should be Given that import assertions are mandated perhaps that gives us the flexibility to use that to infer the CSP policy as well? Alternatively I see the CSP spec does have a response phase, so the alternative could be do perform all module CSP checking in the response phase perhaps? I hope we can solve this as it would be highly beneficial to lay the way for |
All of them end up "being" JS modules, right? What are the benefits of distinguishing them based on the source media type rather than the destination type? |
So that some file allowed to be imported as a CSS module can't be simultaneously imported as JS and start running arbitrary scripts. The same way we wanted to specify the type of module where it's used, it's important for CSP to be able to specify & restrict how a given content can be interpreted. |
If CSP can restrict it, then is there still a need for an import assertion? |
@ljharb AFAIK a CSP can only express constraints which apply after the type/usage is known - e.g. |
Does anyone have any reasonable objections for treating the import assertion as a CSP policy type indicator? |
Yes. There are plenty of websites where CSP isn't deployed or can't be deployed for various reasons. We need a way to ensure CSS or JSON files wouldn't start parsing as JS specially on a server with content based MIME type detection. |
I'm not super familiar with this feature but I'd like to see it get included in interop23: web-platform-tests/interop#236 |
It would be helpful to get some input from those intimately familiar with CSP and module scripts. I think what would end up doing the right thing is that the Import Assertion changes the request destination. That in turn will impact which CSP policy ends up applying. @antosart @mikewest @arturjanc do you think that's workable? |
Just to double-check my assumptions: my understanding is that importing a module requires specifying a type assertion, and that this assertion ensures that a module with a declared CSS/JSON type will not be executed as script. If so, then it would make sense for non-script imports to be governed by other CSP directives (probably If changing the request destination has this effect, then this sounds quite reasonable to me. |
I agree with Artur's take. Shifting the |
I'm relatively neutral, but given that CSS/JSON modules
it somehow looks reasonable to load them as CSS/JSON, not as scripts. As for implementation, I suppose in Chromium this is to load CSS/JSON modules using e.g. |
It was pointed out to me that this really stretches the meaning of assertion. As now the assertion directly drives the way we fetch. And there's other features people brought up that would potentially stretch it even further (such as importing text as a string, an ArrayBuffer, or a Blob). Perhaps changes on the TC39 side are still possible here that would make this less jarring? |
Wouldn't changing the destination for CSS module scripts to "style" change the Accept header (in fetch step 13.2)? If the assertion changes how the request is made or how the response is processed (beyond simply rejecting or accepting the response), then we've gone beyond "assertions" and are violating part of the specification
If browsers feel that CSS modules should have a "style" destination (which seems totally reasonable to me on the face of it!) and if this contradicts the import assertions specification, we may need to go back and think more about the definition of import assertions at TC39. I filed tc39/proposal-import-attributes#125 to discuss this topic. I hope that we don't need to make changes to import assertions, given that they are already shipping in a browser and at Stage 3, but this question is really core to the use cases import assertions were initially designed for, and the syntax and semantics that were designed based on that. We're going to discuss this HTML integration issue in TC39's next fortnightly call around modules. If any fetch/HTML experts are available for this discussion, it would be very helpful; I'm happy to forward the invitation. |
Depending on the time I might be able to attend. |
I put this on the agenda to discuss more officially requesting TC39 changing the syntax as this change would no longer make assertion an applicable term. |
It also seems to undermine the use case that went to stages 2 and 3 - so the entire feature may need to be reevaluated. |
Yes, this is fundamental change. So if it happens, we should at least move the stage back to stage-2. Or given that the thing becomes changed from assertion, we may need a new proposal instead. |
Hi folks, after discussing internally, the chromium and V8 position is:
Our thinking is that it's important to recall that when we did import assertions, there was a real risk that without a proper syntax proposal, the demand for the capability was strong enough that the ecosystem was going to invent micro DSLs within the module specifier to convey type information. This demand is still as strong now as it was then, evidenced by the tools' quick adoption of this feature and some apparent violations of the "no reinterpretation" restriction. Similarly, a micro DSL world is just as undesirable now as it was then, and unshipping the feature puts us at risk of that again (on top of the usual pains of possibly breaking users). We interpret the strong demand and the tooling ecosystem's disregard of the "no reinterpretation" restriction as a solid signal that the ideal path forward is to relax that restriction instead of disabling the feature. Finally, I wonder if we're overindexing on one particular prescriptive meaning of the word "assert". The proposed HTML behavior is actually well within the lay English definition of "assert", though it deviates from its common usage in programming as a function name that aborts execution when checking for a condition. Given that this is novel syntax, I'm inclined to squint and consider it a different shade of meaning of the word "assert". |
Would it not be enough for CSS to be loaded in an inactive document, which is already guarded from execution, IIRC? For example, with DocumentFragment imports, I use For CSS modules, my transcompilation workaround for Webkit and Firefox both use an inactive document to create the Edit: Related w3c/DOM-Parsing#65 |
See recent changes to https://github.com/tc39/proposal-import-attributes. Notably: * Import attributes can affect how a module is loaded and can be part of the cache key. This removes the willful violation HTML had in its usage of import assertions, and unblocks #7233. * The keyword has been changed from assert to with. Due to existing support in Chrome/Node.js/Deno the assert keyword will stick around for a while, and the V8 team will investigate the possibility of removing it.
@syg After this week's TC39 meeting can you share the Chromium/V8 team's current thinking? I'm assuming V8 will add support for |
Chrome is investigating whether it can even remove it based on useage: https://bugs.chromium.org/p/v8/issues/detail?id=13856. |
I've honestly never pushed anything publicly that didn't transcompile CSS imports to string => CSSStyleSheet/HTMLSyleElement. Unless Chromium's counter checks source maps (if they even exist), I doubt the counter will tell us much. I find CSS imports as mostly a pre-bundled thing currently. I can't imagine anything public-facing outside of maybe electron / chrome apps. Since I've been having a rough time trying to get external tools (eg: bundlephobia, webpack, esbuild) to work with them, I'm in the process of dropping them all in my dev code since. Since we're switching actual syntax, then I feel it leads to an appearance of instability. I will likely wait for it to ship on Firefox, before proselytizing their usage around the JS community. Back to template literals, for now. |
I opened #9486 implementing a fix for this, if anyone wants to review it :) |
Builds on whatwg/fetch#1691. Tests: web-platform-tests/wpt#41665. Fixes #7233.
WebAssembly/esm-integration#56 made me wonder what happens for CSS and JSON module scripts and it seems that since destination is "
script
", CSP will usescript-src
for them. Is that really what we want?cc @lweichselbaum @antosart @mikewest @guybedford @dandclark @whatwg/modules
The text was updated successfully, but these errors were encountered: