layer 0: HostEnsureCanCompileStrings integration #76
Conversation
| @@ -548,8 +550,11 @@ location: https://tc39.es/proposal-compartments/ | |||
| 1. Else, if Type(_importMeta_) is not Object, then | |||
| 1. Throw a *TypeError* exception. | |||
| 1. Perform ? RequireInternalSlot(_moduleSource_, [[ModuleSource]]). | |||
| 1. Let _evalRealm_ be the current Realm Record. | |||
| 1. NOTE: Module Instances created from this constructor must use a valid source. | |||
| 1. Perform ? HostEnsureCanCompileStrings(_evalRealm_). | |||
There was a problem hiding this comment.
Creating a ModuleSource is not problematic, but creating a Module Instance from that source is, in which case we call the host.
There was a problem hiding this comment.
@nicolo-ribaudo this is important so we can serialize and share a ModuleSource instance with another realm, and evaluate it there if possible without requiring string evaluation privileges on the realm creating the source.
There was a problem hiding this comment.
Also I believe the Host operation has the wrong (or misleading) name since the problem is not really compilation IMO, but evaluation. We should get some clarifications from implementers.
There was a problem hiding this comment.
This is somehow like you create a new Function via eval and call it in a CSP-enabled Realm. It's OK. You should put the check in the ModuleSource constructor.
There was a problem hiding this comment.
Creating a ModuleSource is not problematic
I just realized that this is problematic: for layer 1, hosts must parse the string passed to ModuleSource to provide the static analysis info, and thus it requires them to ship a complete parser.
This host hook is not just about security, it's about avoiding shipping big chunks of code (parser, interpreter) that are almost never used on resource-constrained engines.
| 1. Let _O_ be ? OrdinaryCreateFromConstructor(NewTarget, *"%Module.prototype%"*, « [[Module]], [[ModuleSourceInstance]], [[ImportHook]] »). | ||
| 1. Let _moduleRecord_ to ! CreateModuleRecord(_moduleSource_.[[ModuleSource]]). | ||
| 1. Let _moduleRecord_ to ! CreateModuleRecord(_evalRealm_, _moduleSource_.[[ModuleSource]]). |
There was a problem hiding this comment.
reusing the _evalRealm_ here, but also it makes sense that when a module record is created, it can be associated to a realm, and the caller of the abstract operation to be responsible for providing such realm. This opens the door for more advanced use-cases like transferring instances across realms.
| @@ -548,8 +550,11 @@ location: https://tc39.es/proposal-compartments/ | |||
| 1. Else, if Type(_importMeta_) is not Object, then | |||
| 1. Throw a *TypeError* exception. | |||
| 1. Perform ? RequireInternalSlot(_moduleSource_, [[ModuleSource]]). | |||
| 1. Let _evalRealm_ be the current Realm Record. | |||
There was a problem hiding this comment.
Does this mean the [[Realm]] from the current Execution Context?
My feeling is that we should be getting [[Realm]] from an internal slot of the %Module% constructor, patterned after %Function%.
There was a problem hiding this comment.
Yes, this is the pattern used by Function constructor (https://tc39.es/ecma262/#sec-function-p1-p2-pn-body), which just calls https://tc39.es/ecma262/#sec-createdynamicfunction, and this abstract operation gets the current realm.
| @@ -548,8 +550,11 @@ location: https://tc39.es/proposal-compartments/ | |||
| 1. Else, if Type(_importMeta_) is not Object, then | |||
| 1. Throw a *TypeError* exception. | |||
| 1. Perform ? RequireInternalSlot(_moduleSource_, [[ModuleSource]]). | |||
| 1. Let _evalRealm_ be the current Realm Record. | |||
| 1. NOTE: Module Instances created from this constructor must use a valid source. | |||
| 1. Perform ? HostEnsureCanCompileStrings(_evalRealm_). | |||
There was a problem hiding this comment.
This should be called in ModuleSource constructor, because create a new module instance from a compiled source is safe, but convert a string to a ModuleSource is unsafe.
There was a problem hiding this comment.
This is a good point... but the problem that I see here is that csp can completely block the parsing of a source... let's chat more about it. Ideally, we can decorate the source record to conditionally call this host hook. cc @nicolo-ribaudo
There was a problem hiding this comment.
csp can completely block the parsing of a source
Yes, that's how CSP works.
There was a problem hiding this comment.
Yes, I understand that's how it works today, but if we are going to extend the ModuleSource API to expose the metadata of the parsed source, you will not be able to parse it in the first place even though you don't plan to evaluate it. E.g.: a bundler.
This PR introduces the host integration to prevent code from being evaluated when needed.