CSS and JSON module scripts and CSP
annevk opened this issue · 24 comments
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 use script-src for them. Is that really what we want?
cc @lweichselbaum @antosart @mikewest @guybedford @dandclark @whatwg/modules
This definitely seems strange to me and like something that should be fixed - that CSS imports should be style-src and JSON imports should be something else but not script-src since they don't represent any execution. There is also no escalation possibility here by design since the import './data.json' assert { type: 'json' } assertion syntax being mandated guarantees that we know the policy in play.
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 wasm-src as well and avoid the permanent need for a non-granular unsafe-inline-wasm in policies.
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?
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. style-src 'self' says that all self-origin URL resources can be interpreted as stylesheets, not that they will be / must be.
Does anyone have any reasonable objections for treating the import assertion as a CSP policy type indicator?
If CSP can restrict it, then is there still a need for an import assertion?
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
Is there any rough consensus in this issue?
Is there anything I can change in chromium that would help?
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 style-src and connect-src for CSS and JSON respectively?).
If changing the request destination has this effect, then this sounds quite reasonable to me.
I agree with Artur's take. Shifting the destination of the requests sounds like a reasonable way of reflecting the risk associated with each request type if each request type solidly enforces content-type matches. I'd be comfortable with that approach.
I'm relatively neutral, but given that CSS/JSON modules
- have
Content-Typeenforcement, - are always parsed as CSS/JSON, and
- don't trigger another module to load (e.g. CSS/JSON modules can't have any submodules and thus can't trigger any JS loading),
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. CSSStylesheetResource instead of ScriptResource.
This is more consistent, not only in that applying style-src CSP to CSS modules, but also in that ScriptResource always loads scripts (currently ScriptResource can load CSS modules).
This makes module loading code path less uniform though: module loading code should switch between resource types and be careful about subtle differences (if any) between them.
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
moduleRequest.[[Assertions]] must not influence the interpretation of the module or the module specifier; instead, it may be used to determine whether the algorithm completes normally or with an abrupt completion.
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:
- We prefer the proposed HTML semantics, i.e. allow
Acceptheaders and have CSP integration work for CSS modules. These semantics seem intuitive and do the right thing to us -- the "no reinterpretation" ECMAScript restriction ought to be relaxed to allow for CSP integration, but still be strong enough for the security requirements. - This feature has been Stage 3 in TC39 for a while, and has been shipping in Chrome since 91 and V8 9.1 (which was released May 2021). Given how long it's been out in Chrome as well as the tooling ecosystem in Node/Deno and bundlers, we are not going to unship the feature on short notice. Moreover we prefer to not unship, but to change the behavior to the proposed semantics.
- For the same reason above, it'd be ideal if we didn't have to change the
assertkeyword either. If the developer response to theassertkeyword is overwhelmingly negative, we think the next step should be to assess how widespread the use is in the tooling ecosystem and possibilities of migration.
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 document.implementation.createHTMLDocument(). because <img src={url}> would be parsed and cause network fetchs before the template can be interpolated. If HTML Imports becomes finalized, I hope that becomes standard because I wouldn't be able to use it for importing uninterpolated templates if it'll start parsing on the current (active) document.
For CSS modules, my transcompilation workaround for Webkit and Firefox both use an inactive document to create the CSSStyleSheet. CSP throws up errors when I try to add an HTMLStyleElement to a shadowRoot as it should. And I no problems with adoptedStyleSheets. I'm not sure where the chain of trust would be broken. Pardon my ignorance, but are we trying to guard against CSS Modules being loaded before it can be applied to adoptedStyleSheets? Is that the goal?
Edit: Related w3c/DOM-Parsing#65
after discussing internally, the chromium and V8 position
@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 with; any idea of a rough timeline for that? And any idea on when or whether V8 will drop assert?
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 :)