feat(ses): Add XS variant of shim #2471

kriskowal · 2024-09-27T04:59:17Z

Closes: #2251

Description

To take advantage of native XS compartments while retaining backward compatibility and parity with the SES shim, we introduce an xs specific Compartment adapter that requires two levels of opt-in.

The SES shim must be bundled with the xs package export/import condition.
The constructor of a Compartment must specify the __native__ option to sacrifice the ability to use precompiled module sources (as generated by @endo/module-source without the xs package export/import condition) and instead use adapted native ModuleSource (as generated by @endo/module-source with the xs package export/import condition). This allows @endo/import-bundle, for example, to use the JSON serialization of a precompiled module source that is captured in a bundle and also opt-in for __native__ treatment if the archive/bundle contains original sources.

This change introduces an XS-specific shim that is an adapter for Compartment and lockdown, and also papers over parity gaps like the XS Object.freeze second boolean argument. The adapter creates parallel native and shim (virtual) compartment trees, where any individual Compartment can elect to use the native or shim variant for a child Compartment.

We have not yet found a workable design that obviates the need for the __native__ opt-in. Such a design would need to create an adapter from precompiled module sources to XS’s virtual module source protocol. To do that would require native module to emit notifications for the mutation of exported live bindings and also require the native Compartment evaluate method to accept an argument like the shim’s __moduleGlobalLexicals__.

Security Considerations

Uncountably numerous. Among them, with the __native__ option, censorship does not occur, so dynamic import and direct eval are possible.

Scaling Considerations

The native ModuleSource makes it practical to defer module parsing to runtime, and should improve the performance of execution as well.

Documentation Considerations

The NEWS.md qualifies these changes as under "incubation". When the shape of these changes settles, the NEWS will need to reiterate the final user facing API in README.md and NEWS.md. With the __native__ option, censorship does not occur, so dynamic import and direct eval are possible.

Testing Considerations

This change contains a token of xst testing that is exercised in CI with test:xs. This demonstrates the use of @endo/compartment-mapper/bundle.js to thread the xs package export/import condition and generate a script that can xst can run directly. This gives us some modest confidence that lockdown works and demonstrates the __native__ feature but does not provide sufficient confidence of parity for the gamut of Compartment usage, both legacy and XS, for all of the accepted module descriptors and other Compartment features. This is an exercise that will begin with a subsequent change that introduces hardened262, a comprehensive parity checking framework for the full cross-product of [ SES on Node.js, SES on XS, and XS stand-alone ] ⨉ [ Lockdown, not Lockdown ] ⨉ [ Compartment, no Compartment ] ⨉ [ Sloppy, Strict, Module ].

Compatibility Considerations

This change preserves all existing usage and introduces an unstable alternate version of SES for XS that requires two layers of opt-in. The use of the xs condition introduces an adapter for Compartment that may not have full parity with the underlying implementation, and requires additional testing. The __native__ option elects to break some usage (precompiled moduels) in favor of others (dynamic import, direct eval, top-level-await).

Upgrade Considerations

In order to realize these changes on the Agoric chain will likely require a more recent version of XS and switching the bundle format for the lockdown/bootstrap script for xsnap swingset workers.

kriskowal · 2024-10-31T17:48:52Z

packages/ses/src-xs/index.js

+    const modules = delegateNative
+      ? fromEntries(
+          arrayMap(
+            entries(options.modules ?? {}),
+            ([specifier, descriptor]) => [
+              specifier,
+              adaptModuleDescriptor(descriptor, specifier, undefined),
+            ],
+          ),
+        )
+      : {};


Add comment explaining that this module is only used for nativeOptions and the shim options pass options.modules thru unaltered. Arguably, the {} at the end could be anything, but {} satisfies the type.

kriskowal · 2024-10-31T18:13:47Z

packages/import-bundle/NEWS.md

+Experimental:
+
+- Adds a `__native__: true` option that indicates that the application will
+  fall through to the native implementation of Compartment, currently only
+  available on XS, which lacks support for precompiled module sources (as exist
+  in many archived applications, particularly Agoric smart contract bundles)
+  and instead supports loading modules from original sources (which is not
+  possible at runtime on XS).


An alternative design that I am interested in considering is to instead create a new moduleFormat called endoNativeZipBase64 that implies this __native__ flag. Whether it can be overridden is worth consideration. This would imply a change in bundle-source such that the existing --no-transforms or a new --native flags would switch the moduleFormat. We can create bundles with -T,--no-transforms today that cannot be imported without this __native__ flag on XS, but once this PR lands, can be imported without the __native__ flag on Node.js.

I don't understand

loading modules from original sources (which is not possible at runtime on XS)

What about this is not possible on XS?

SES on XS, both before this change and after this change (but without the __native__ option set), cannot load a module from original sources without the assistance of the optional ModuleSource shim.

In any case, this prose needs some work, but I’ve also moved this commit out of this PR since I intend to investigate the alternate design using an "endoNativeZipBase64" format. The MetaMask folks reminded me that the web will still benefit from pre-compiled bundles, so those should remain the default and we should refrain from changing the default even with a major version bump on bundle-source.

erights

Review so far

erights · 2024-11-04T00:08:56Z

packages/compartment-mapper/NEWS.md

+Experimental:
+
+- Adds a `__native__: true` option to all paths to import, that indicates that
+  the application will fall through to the native implementation of
+  Compartment, currently only available on XS, which lacks support for
+  precompiled module sources (as exist in many archived applications,
+  particularly Agoric smart contract bundles) and instead supports loading
+  modules from original sources (which is not possible at runtime on XS).
+


Did you mean to include this under "v1.3.0" rather than "Next version"?

erights · 2024-11-04T00:12:40Z

packages/import-bundle/NEWS.md

+Experimental:
+
+- Adds a `__native__: true` option that indicates that the application will
+  fall through to the native implementation of Compartment, currently only
+  available on XS, which lacks support for precompiled module sources (as exist
+  in many archived applications, particularly Agoric smart contract bundles)
+  and instead supports loading modules from original sources (which is not
+  possible at runtime on XS).


I don't understand

loading modules from original sources (which is not possible at runtime on XS)

What about this is not possible on XS?

erights · 2024-11-04T00:14:32Z

packages/ses/NEWS.md

+
+Incubating: Please do not rely on these features as they are under development
+and subject to breaking changes that will not be signaled by semver.
+
 - Adds permits for `ModuleSource`, either the native implementation or from
  `@endo/module-source/shim.js`.
+- Adds support for an XS-specific variant of the SES shim that is triggered
+  with the `xs` package export condition.
+  This version of SES preserves all the features of `Compartment` provided
+  uniquely by the SES shim, but with the `__native__` constructor option,
+  loses support for importing precompiled module records and gains support
+  for native `ModuleSource`.


If you truly want to add/change an entry in an older NEWS section, could you add something to the "Next release" section to make clear what is new?

Unintentional: moved up.

erights · 2024-11-04T00:21:06Z

packages/compartment-mapper/src/bundle.js

        throw Error(
          `Cannot bundle: encountered deferredError ${deferredError}`,


Keep the errors connected re causal console log output

Suggested change

throw Error(

`Cannot bundle: encountered deferredError ${deferredError}`,

Fail`Cannot bundle: encountered deferredError ${deferredError}`,

Also, consider for other asserts. Unless we're trying to avoid our assertion support and style in this package for some reason. Are we? Why?

Re-raising @erights's comment:

Keep the errors connected re causal console log output

Suggested change

throw Error(

`Cannot bundle: encountered deferredError ${deferredError}`,

Fail`Cannot bundle: encountered deferredError ${deferredError}`,

Also, consider for other asserts. Unless we're trying to avoid our assertion support and style in this package for some reason. Are we? Why?

erights · 2024-11-04T00:37:07Z

packages/ses/src-xs/index.js

+import '../src/assert-shim.js';
+import '../src/console-shim.js';
+import { repairIntrinsics } from '../src/lockdown.js';
+
+import {
+  makeCompartmentConstructor as makeShimCompartmentConstructor,
+  compartmentOptions,
+} from '../src/compartment.js';


Minor:

This file is intended to be an xs-flavored alternative to ses/index.js. Two minor changes would help a reader see and understand the parallelism:

Move ses/index.js to ses/src/index.js, leaving behind a forwarding stub if appropriate.

Rearrange the import order in that file to
import './assert-shim.js'; import './console-shim.js'; import './lockdown-shim.js'; import './compartment-shim.js';
so the reader doesn't need to worry about whether the difference in import order is significant.

Might it even make sense to break this file up into a ses/src-xs/lockdown-shim.js and ses/src-xs/compartment-shim.js, and then change ses/src-xs/index.js to be that much more similar?

I’ve taken this advice. You will find more files in src-xs on your next review.

erights · 2024-11-04T00:52:32Z

packages/ses/src-xs/index.js

+);
+
+globalThis.lockdown = options => {
+  const hardenIntrinsics = repairIntrinsics(options);


I notice that this PR does not modify lockdown.js which defines the repairIntrinsics being called here. Nor does it modify tame-harden.js or make-hardener.js used by repairIntrinsics to make the harden function. But presumably, on XS we want to use only the XS native harden. How does that happen?

We use native harden due to an existing fall through to globalThis.harden in makeHarden. We also continue to respect the lockdown option to make harden a no-op. I’ve elected to continue using our lockdown variant so-far, since it is parameterized, can conduct repairs to the XS intrinsics if any become necessary, and support enablements. I’m at least leaving native lockdown out of scope for this run at native Compartment and ModuleSource.

erights · 2024-11-05T20:44:09Z

packages/ses/src-xs/index.js

+  hardenIntrinsics();
+  // Replace global Compartment with a version that is hardened and hardens
+  // transitive child Compartment.
+  globalThis.Compartment = FERAL_EVAL(compartmentShim)(


This both reevaluates compartmentShim and reapplies its completion value, which is the big function defined by inline text above, to similar arguments, with the main (only?) difference being noHarden vs harden. While this is all fine, so no change necessarily suggested, it does make me wonder whether we could evaluate the function only once and have all needed differences come only from the application arguments?

I’ve posted a change that eliminates the eval entirely. It’s now just a function adaptCompartmentConstructors(NativeCompartmentConstructor, ShimCompartmentConstructor, maybeHarden). I look forward to possibly remembering why the eval was necessary, but it probably wasn’t actually.

erights

Review complete on everything but src-xs/index.js

This looks good to me in broad outline, but I have not yet found the time to review it in detail. @gibson042 , can you complete this? If you approve this one file, I'm willing to approve the whole thing. Thanks!

gibson042

This seems nicely threaded to me.

gibson042 · 2024-11-26T03:25:43Z

packages/compartment-mapper/NEWS.md

@@ -23,13 +23,43 @@ User-visible changes to `@endo/compartment-mapper`:
  originally intended. To those who expected the previous behavior: if you
  exist, please exercise caution when upgrading.

+Experimental:


Should this be moved up to a "Next version" above v1.4.0?

gibson042 · 2024-11-26T03:32:10Z

packages/compartment-mapper/src/bundle.js

        throw Error(
          `Cannot bundle: encountered deferredError ${deferredError}`,


Re-raising @erights's comment:

Keep the errors connected re causal console log output

Suggested change

throw Error(

`Cannot bundle: encountered deferredError ${deferredError}`,

Fail`Cannot bundle: encountered deferredError ${deferredError}`,

Also, consider for other asserts. Unless we're trying to avoid our assertion support and style in this package for some reason. Are we? Why?

gibson042 · 2024-11-26T03:44:14Z

packages/ses/src-xs/commons.js

+  uncurryThis,
+} from '../src/commons.js';
+
+export const NativeStartCompartment = globalThis.Compartment;


Not NativeCompartment?

Making the Start distinction avoids a shadowing problem elsewhere, when a child compartment has its own constructors.

gibson042 · 2024-11-26T04:12:52Z

packages/ses/src-xs/compartment-shim.js

+    for (const transform of transforms) {
+      source = transform(source);
+    }


Do the imports from commons.js mean that this code needs to be robust against a tampered environment? If so, then it probably shouldn't use of.

Suggested change

for (const transform of transforms) {

source = transform(source);

}

for (let i = 0; i < transforms.length; i += 1) {

source = transforms[i](source);

}

gibson042 · 2024-11-26T04:14:24Z

packages/ses/src-xs/compartment.js

+      ? fromEntries(
+          arrayMap(
+            entries(options.modules ?? {}),
+            ([specifier, descriptor]) => [


Do the imports from commons.js mean that this code needs to be robust against a tampered environment? If so, then

Suggested change

([specifier, descriptor]) => [

({ 0: specifier, 1: descriptor }) => [

gibson042 · 2024-11-26T04:15:49Z

packages/ses/src-xs/compartment.js

+  }
+  // eslint-disable-next-line no-underscore-dangle
+  if (source.__syncModuleProgram__) {
+    throw new SyntaxError(


I'm pretty sure @erights prefers universal avoidance of new with built-in error constructors.

gibson042 · 2024-11-26T04:28:36Z

packages/ses/src-xs/index.js

+// We disable this behavior to encourage use of `harden` for portable Hardened
+// JavaScript.
+/** @param {object} object */
+Object.freeze = object => freeze(object);


Let's maintain Object.freeze.name.

Suggested change

Object.freeze = object => freeze(object);

Object.freeze = ({ freeze: object => freeze(object) }).freeze;

gibson042 · 2024-11-26T04:29:01Z

packages/ses/src-xs/lockdown-shim.js

@@ -0,0 +1,26 @@
+/**
+ * @module Alters the XS implementation of Lockdown to be backward compatible


Suggested change

* @module Alters the XS implementation of Lockdown to be backward compatible

* @module Alters the XS implementation of lockdown to be backward compatible

gibson042 · 2024-11-26T04:38:25Z

packages/ses/src-xs/lockdown-shim.js

+globalThis.lockdown = options => {
+  const hardenIntrinsics = repairIntrinsics(options);
+  hardenIntrinsics();
+  // Replace global Compartment with a version that is hardened and hardens
+  // transitive child Compartment.
+  globalThis.Compartment = adaptCompartmentConstructors(
+    NativeStartCompartment,
+    ShimStartCompartment,
+    harden,
+  );
+};


Suggested change

globalThis.lockdown = options => {

const hardenIntrinsics = repairIntrinsics(options);

hardenIntrinsics();

// Replace global Compartment with a version that is hardened and hardens

// transitive child Compartment.

globalThis.Compartment = adaptCompartmentConstructors(

NativeStartCompartment,

ShimStartCompartment,

harden,

);

};

const lockdown = options => {

const hardenIntrinsics = repairIntrinsics(options);

hardenIntrinsics();

// Replace global Compartment with a version that is hardened and hardens

// transitive child Compartment.

globalThis.Compartment = adaptCompartmentConstructors(

NativeStartCompartment,

ShimStartCompartment,

harden,

);

};

globalThis.lockdown = lockdown;

…runtime modules

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 8ddbfc3 to 70beef1 Compare September 27, 2024 05:23

kriskowal force-pushed the kriskowal-ses-xs-shim branch 2 times, most recently from 5e2a457 to edb5c78 Compare September 27, 2024 05:34

kriskowal requested review from erights and gibson042 September 27, 2024 15:17

kriskowal mentioned this pull request Sep 27, 2024

Using XS native Compartments for Agoric Vat Workers #400

Open

29 tasks

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 70beef1 to 5342ea3 Compare October 9, 2024 06:04

kriskowal force-pushed the kriskowal-ses-xs-shim branch from edb5c78 to 08138db Compare October 9, 2024 06:04

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 5342ea3 to 607b4ee Compare October 17, 2024 22:54

kriskowal force-pushed the kriskowal-ses-xs-shim branch from 08138db to a4f3663 Compare October 17, 2024 22:54

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 607b4ee to a02438f Compare October 18, 2024 00:05

kriskowal force-pushed the kriskowal-ses-xs-shim branch from a4f3663 to 920ddd8 Compare October 18, 2024 00:05

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from a02438f to 82a89a8 Compare October 18, 2024 00:50

kriskowal force-pushed the kriskowal-ses-xs-shim branch from 920ddd8 to ae04c9e Compare October 18, 2024 00:50

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 82a89a8 to 84a2fb7 Compare October 18, 2024 01:18

kriskowal force-pushed the kriskowal-ses-xs-shim branch from ae04c9e to f2e4a22 Compare October 18, 2024 01:18

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 84a2fb7 to c1cdb57 Compare October 18, 2024 05:40

kriskowal force-pushed the kriskowal-ses-xs-shim branch from f2e4a22 to aca33ea Compare October 18, 2024 05:40

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from c1cdb57 to 7312722 Compare October 18, 2024 19:32

kriskowal force-pushed the kriskowal-ses-xs-shim branch from aca33ea to 47fa5f2 Compare October 18, 2024 19:32

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 7312722 to f8dd207 Compare October 18, 2024 19:49

kriskowal force-pushed the kriskowal-ses-xs-shim branch from 47fa5f2 to 10ef65c Compare October 18, 2024 19:49

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from f8dd207 to 13eb35a Compare October 18, 2024 20:01

kriskowal force-pushed the kriskowal-ses-xs-shim branch from 10ef65c to b45a4e0 Compare October 18, 2024 20:01

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 13eb35a to 9f9b0ae Compare October 18, 2024 20:28

kriskowal force-pushed the kriskowal-ses-xs-shim branch from b45a4e0 to 63ce6b4 Compare October 18, 2024 20:28

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 9f9b0ae to f6a6bac Compare October 18, 2024 20:33

kriskowal force-pushed the kriskowal-ses-xs-shim branch from 63ce6b4 to 8d472d9 Compare October 18, 2024 20:33

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from f6a6bac to 69152ea Compare October 18, 2024 20:39

kriskowal force-pushed the kriskowal-ses-xs-shim branch from 8d472d9 to dff1b9c Compare October 18, 2024 20:39

kriskowal force-pushed the kriskowal-ses-xs-shim branch 2 times, most recently from 3417669 to 6d709e7 Compare October 30, 2024 22:55

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from fa10b9b to 9e16181 Compare October 30, 2024 22:55

kriskowal commented Oct 31, 2024

View reviewed changes

erights reviewed Nov 4, 2024

View reviewed changes

erights self-requested a review November 4, 2024 00:26

erights reviewed Nov 4, 2024

View reviewed changes

erights self-requested a review November 4, 2024 00:52

erights reviewed Nov 5, 2024

View reviewed changes

erights self-requested a review November 5, 2024 20:49

erights reviewed Nov 12, 2024

View reviewed changes

kriskowal force-pushed the kriskowal-xs-module-source-shim branch from 9e16181 to 53e5109 Compare November 14, 2024 21:02

kriskowal force-pushed the kriskowal-ses-xs-shim branch from 6d709e7 to f388ca5 Compare November 14, 2024 21:02

Base automatically changed from kriskowal-xs-module-source-shim to master November 14, 2024 21:20

kriskowal force-pushed the kriskowal-ses-xs-shim branch 3 times, most recently from b8149b7 to 52598c1 Compare November 15, 2024 01:23

gibson042 reviewed Nov 26, 2024

View reviewed changes

gibson042 approved these changes Nov 26, 2024

View reviewed changes

kriskowal force-pushed the kriskowal-ses-xs-shim branch 3 times, most recently from c67795d to da44206 Compare December 6, 2024 22:40

kriskowal added 4 commits December 6, 2024 14:52

feat(ses): Add XS variant of shim

f6c8456

feat(compartment-mapper): Export all parsers

c5f5d43

feat(compartment-mapper): Thread native flag to opt-in for native XS …

aa1e77f

…runtime modules

chore(compartment-mapper): Improve makeBundle diagnostics

bd79347

kriskowal force-pushed the kriskowal-ses-xs-shim branch from da44206 to bd79347 Compare December 6, 2024 22:54

kriskowal merged commit 335bbba into master Dec 6, 2024
15 checks passed

kriskowal deleted the kriskowal-ses-xs-shim branch December 6, 2024 23:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ses): Add XS variant of shim #2471

feat(ses): Add XS variant of shim #2471

kriskowal commented Sep 27, 2024

kriskowal Oct 31, 2024

kriskowal Oct 31, 2024 •

edited

Loading

erights Nov 4, 2024

kriskowal Nov 15, 2024

erights left a comment

erights Nov 4, 2024

erights Nov 4, 2024

erights Nov 4, 2024

kriskowal Nov 15, 2024

erights Nov 4, 2024

gibson042 Nov 26, 2024

erights Nov 4, 2024

erights Nov 4, 2024

kriskowal Nov 15, 2024

erights Nov 4, 2024

kriskowal Nov 15, 2024

erights Nov 5, 2024

kriskowal Nov 15, 2024

erights left a comment

gibson042 left a comment

gibson042 Nov 26, 2024

gibson042 Nov 26, 2024

gibson042 Nov 26, 2024

kriskowal Dec 6, 2024

gibson042 Nov 26, 2024

gibson042 Nov 26, 2024

gibson042 Nov 26, 2024

gibson042 Nov 26, 2024

gibson042 Nov 26, 2024

gibson042 Nov 26, 2024

		throw Error(
		`Cannot bundle: encountered deferredError ${deferredError}`,

	throw Error(
	`Cannot bundle: encountered deferredError ${deferredError}`,
	Fail`Cannot bundle: encountered deferredError ${deferredError}`,

	([specifier, descriptor]) => [
	({ 0: specifier, 1: descriptor }) => [

	Object.freeze = object => freeze(object);
	Object.freeze = ({ freeze: object => freeze(object) }).freeze;

		@@ -0,0 +1,26 @@
		/**
		* @module Alters the XS implementation of Lockdown to be backward compatible

feat(ses): Add XS variant of shim #2471

feat(ses): Add XS variant of shim #2471

Conversation

kriskowal commented Sep 27, 2024

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Compatibility Considerations

Upgrade Considerations

Choose a reason for hiding this comment

kriskowal Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erights left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erights left a comment

Choose a reason for hiding this comment

gibson042 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kriskowal Oct 31, 2024 •

edited

Loading