How to run .cjs shims strict?

[erights]

To write tests for #1655 , I import some shims from core-js. However, core-js is written in common lisp style modules, which by default are evaluated as sloppy code. As a result, even if the shims in question were otherwise safe and compatible with Hardened JS, this aspect makes them grossly unsafe. Fortunately, the SES shim by default will fail to initialize if it encounters apparently primordial sloppy functions. This fails safe.

At #1655 (comment) @mhofman suggests that we run these as strict code. Presumably, to support LavaMoat, we already know how to do that. How should I modify the test cases from that PR?

[naugtur]

Forcing strict mode in node.js used to be done by this monkeypatch: https://github.com/isaacs/use-strict/blob/master/index.js
or by passing an undocumented --use_strict flag.
Both are not recommended in modern node.js apps. We'd need to empirically test if any of them still work.

If we used compartment-mapper, core-js running in a Compartment would be executed in strict mode, but that's obviously pointless as the goal of having core-js is running it prior to lockdown.

In use-cases where a package is supposed to modify intrinsics we're currently in a very uncomfortable situation of having to run it before lockdown and therefore SES has no means of influencing how the end user runs something like core-js.

But I think this issue has some overlap with the topic of vetted shims. If we took the design for vetted shims to run after lockdown modifies intrinsics but before it freezes them, we could also wrap the execution of the vetted shim in a Compartment (or a simplified compartment, as Compartment is not supposed to exist before lockdown finishes) . The result of that would potentially be us controlling whether the code is executed in strict mode.

I'm sure it would work with compartment-mapper present, but I doubt we have the machinery necessary for running 3rdparty packages in a Compartment-ish with just SES.
But I see some hope in vetted shims as a step towards solving this correctly. Worst case scenario, we provide a vetted shim that wraps core-js and cleans up after it as part of the shim not SES itself.

[erights]

Great info!

We actually do have partial support for running other code after all repairs but before hardening.

But neither that nor full vetted shim support may be right for this particular case, since the shim needs to run before permit processing.

I’ll take a better look soon.

[mhofman]

My understanding of vetted shims is that they should run before lockdown, not after. Also compartments can technically exist without lockdown, and I don't think it'd be too crazy to imagine loading vetted shims inside a compartment before lockdown. We still need to define the interaction of vetted shims with lockdown, in particular how their execution is trigger (since they should execute after the ses shim executes, and before lockdown is called) and how they could register permits (and likely how they can access repaired intrinsics).

This comes down to the definition of a vetted shim. It's trusted to be part of the TCB, but do we mandate that vetted shims be compatible with SES? For example do we mandate that they don't introduce strict mode kludges?

Btw the SES shim itself relies on having sloppy mode (to be able to use with), so fully disabling strict mode is not an option. We at the very least need to be able to apply strict mode to loaded cjs modules, but not to evaluated code.

[mhofman]

Reading the internal node source, it looks like the following would still work:

import module from 'node:module';
module.wrapper[0] += '"use strict";'

[naugtur]

My assumption for vetted shims placement is after the part of lockdown that eg. removes unexpected methods/fields from intrinsics, but right before everything gets frozen.
Otherwise, if running vetted shims earlier, we'd require a mechanism for figuring out what not to remove.

injecting use strict should work but the package repository has been archived, so I was wondering if there's fundamental issues with it. I might as well just ask the author.

Here's a different monkeypatch for the same result:
https://github.com/torarvid/auto-strict/blob/master/index.js

[erights]

But neither that nor full vetted shim support may be right for this particular case, since the shim needs to run before permit processing.

I’ll take a better look soon.

Took a look and I was correct. The permit processing happens during the repair phase, not the hardening phase, and so before vetted shims would run. Thus, for purposes of this test, we cannot run core-js as a vetted shim. It must be run before the repair phase.

[erights]

My assumption for vetted shims placement is after the part of lockdown that eg. removes unexpected methods/fields from intrinsics, but right before everything gets frozen.

That is correct.

[erights]

@kriskowal since permit processing and property removal happen during the repair phase before vetted shims would run, perhaps we already do not require that vetted shims (or an envelope) register their additional primordials. The vetting process would then need to include vetting that all primordials added by the vetted shim are fine as is, without the permit processing.

[kriskowal]

@kriskowal since permit processing and property removal happen during the repair phase before vetted shims would run, perhaps we already do not require that vetted shims (or an envelope) register their additional primordials. The vetting process would then need to include vetting that all primordials added by the vetted shim are fine as is, without the permit processing.

I see. The premise is that anything that remains is implicitly permitted. That would certainly be the easy solution! That does still require us to explicitly delimit repair before vetted shims run, which I think is an acceptable cost for the boon of not having a Permit API.

We would still want getIntrinsic to reveal intrinsics that were introduced by repair. I know @mhofman already suggests further machinery for adding to the shared intrinsics for new compartment globals, including inescapable intrinsics (intrinsics that child compartments cannot overshadow). That is a separate concern.

[kriskowal]

In short, exposing repair() is actually a cheap and easy change we can make soon that would allow us to experiment with this approach.

[naugtur]

In short, exposing repair() is actually a cheap and easy change we can make soon that would allow us to experiment with this approach.

Does that mean it'd be used like this?

repair();
myShim();
lockdown();

I assumed vetted shims would be callbacks passed to lockdown options. In that case SES is in control of how they run, at least to some extent. Especially if we'd require them to be modules, but then it's a chicken and egg problem.

[kriskowal]

Yes, @naugtur, your snippet is correct. That would allow existing shims to work largely without modification and gives the host applications some options for how to orchestrate the order in which their applied without complicating the signature of lockdown. So shims can be scripts, modules, or bundled.