feat(ses,pass-style): use non-trapping integrity trait for safety

packages/captp/src/captp.js

@@ -47,7 +47,7 @@ const reverseSlot = slot => {
 };
 
 /**
- * @typedef {object} CapTPImportExportTables 
+ * @typedef {object} CapTPImportExportTables
  * @property {(value: any) => CapTPSlot} makeSlotForValue
  * @property {(slot: CapTPSlot, iface: string | undefined) => any} makeValueForSlot
  * @property {(slot: CapTPSlot) => boolean} hasImport
@@ -58,12 +58,12 @@ const reverseSlot = slot => {
  * @property {(slot: CapTPSlot, value: any) => void} markAsExported
  * @property {(slot: CapTPSlot) => void} deleteExport
  * @property {() => void} didDisconnect
- 
+ *
  * @typedef {object} MakeCapTPImportExportTablesOptions
  * @property {boolean} gcImports
  * @property {(slot: CapTPSlot) => void} releaseSlot
  * @property {(slot: CapTPSlot) => RemoteKit} makeRemoteKit
- 
+ *
  * @param {MakeCapTPImportExportTablesOptions} options
  * @returns {CapTPImportExportTables}
  */

packages/marshal/src/encodeToCapData.js

@@ -30,7 +30,8 @@ const {
   is,
   entries,
   fromEntries,
-  freeze,
+  // @ts-expect-error TS doesn't see this on ObjectConstructor
+  suppressTrapping,
 } = Object;
 
 /**
@@ -176,10 +177,10 @@ export const makeEncodeToCapData = (encodeOptions = {}) => {
             // We harden the entire capData encoding before we return it.
             // `encodeToCapData` requires that its input be Passable, and
             // therefore hardened.
-            // The `freeze` here is needed anyway, because the `rest` is
+            // The `suppressTrapping` here is needed anyway, because the `rest` is
             // freshly constructed by the `...` above, and we're using it
             // as imput in another call to `encodeToCapData`.
-            result.rest = encodeToCapDataRecur(freeze(rest));
+            result.rest = encodeToCapDataRecur(suppressTrapping(rest));
           }
           return result;
         }

packages/pass-style/src/passStyle-helpers.js

@@ -11,8 +11,10 @@ const {
   getOwnPropertyDescriptor,
   getPrototypeOf,
   hasOwnProperty: objectHasOwnProperty,
-  isFrozen,
   prototype: objectPrototype,
+  isFrozen,
+  // @ts-expect-error TS does not yet have `isNonTrapping` on ObjectConstructor
+  isNonTrapping,
 } = Object;
 const { apply } = Reflect;
 const { toStringTag: toStringTagSymbol } = Symbol;
@@ -167,6 +169,9 @@ const makeCheckTagRecord = checkProto => {
           CX(check)`A non-object cannot be a tagRecord: ${tagRecord}`)) &&
       (isFrozen(tagRecord) ||
         (!!check && CX(check)`A tagRecord must be frozen: ${tagRecord}`)) &&
+      (isNonTrapping(tagRecord) ||
+        (!!check &&
+          CX(check)`A tagRecord must be non-trapping: ${tagRecord}`)) &&
       (!isArray(tagRecord) ||
         (!!check && CX(check)`An array cannot be a tagRecord: ${tagRecord}`)) &&
       checkPassStyle(

packages/pass-style/src/passStyleOf.js

@@ -31,7 +31,13 @@ import { assertPassableString } from './string.js';
 /** @typedef {Exclude<PassStyle, PrimitiveStyle | "promise">} HelperPassStyle */
 
 const { ownKeys } = Reflect;
-const { isFrozen, getOwnPropertyDescriptors, values } = Object;
+const {
+  getOwnPropertyDescriptors,
+  values,
+  isFrozen,
+  // @ts-expect-error TS does not yet have `isNonTrapping` on ObjectConstructor
+  isNonTrapping,
+} = Object;
 
 /**
  * @param {PassStyleHelper[]} passStyleHelpers
@@ -143,14 +149,17 @@ const makePassStyleOf = passStyleHelpers => {
           if (inner === null) {
             return 'null';
           }
-          if (!isFrozen(inner)) {
-            assert.fail(
-              // TypedArrays get special treatment in harden()
-              // and a corresponding special error message here.
-              isTypedArray(inner)
-                ? X`Cannot pass mutable typed arrays like ${inner}.`
-                : X`Cannot pass non-frozen objects like ${inner}. Use harden()`,
-            );
+          if (!isNonTrapping(inner)) {
+            if (!isFrozen(inner)) {
+              throw assert.fail(
+                // TypedArrays get special treatment in harden()
+                // and a corresponding special error message here.
+                isTypedArray(inner)
+                  ? X`Cannot pass mutable typed arrays like ${inner}.`
+                  : X`Cannot pass non-frozen objects like ${inner}. Use harden()`,
+              );
+            }
+            throw Fail`Cannot pass trapping objects like ${inner}`;
           }
           if (isPromise(inner)) {
             assertSafePromise(inner);
@@ -177,8 +186,12 @@ const makePassStyleOf = passStyleHelpers => {
           return 'remotable';
         }
         case 'function': {
-          isFrozen(inner) ||
-            Fail`Cannot pass non-frozen objects like ${inner}. Use harden()`;
+          if (!isNonTrapping(inner)) {
+            if (!isFrozen(inner)) {
+              throw Fail`Cannot pass non-frozen objects like ${inner}. Use harden()`;
+            }
+            throw Fail`Cannot pass trapping objects like ${inner}. Use harden()`;
+          }
           typeof inner.then !== 'function' ||
             Fail`Cannot pass non-promise thenables`;
           remotableHelper.assertValid(inner, passStyleOfRecur);

packages/pass-style/src/remotable.js

@@ -24,9 +24,10 @@ const { ownKeys } = Reflect;
 const { isArray } = Array;
 const {
   getPrototypeOf,
-  isFrozen,
   prototype: objectPrototype,
   getOwnPropertyDescriptors,
+  // @ts-expect-error TS does not yet have `isNonTrapping` on ObjectConstructor
+  isNonTrapping,
 } = Object;
 
 /**
@@ -154,7 +155,7 @@ const checkRemotable = (val, check) => {
   if (confirmedRemotables.has(val)) {
     return true;
   }
-  if (!isFrozen(val)) {
+  if (!isNonTrapping(val)) {
     return (
       !!check && CX(check)`cannot serialize non-frozen objects like ${val}`
     );

packages/pass-style/src/safe-promise.js

@@ -6,7 +6,12 @@ import { assertChecker, hasOwnPropertyOf, CX } from './passStyle-helpers.js';
 
 /** @import {Checker} from './types.js' */
 
-const { isFrozen, getPrototypeOf, getOwnPropertyDescriptor } = Object;
+const {
+  getPrototypeOf,
+  getOwnPropertyDescriptor,
+  // @ts-expect-error TS does not yet have `isNonTrapping` on ObjectConstructor
+  isNonTrapping,
+} = Object;
 const { ownKeys } = Reflect;
 const { toStringTag } = Symbol;
 
@@ -88,7 +93,7 @@ const checkPromiseOwnKeys = (pr, check) => {
     if (
       typeof val === 'object' &&
       val !== null &&
-      isFrozen(val) &&
+      isNonTrapping(val) &&
       getPrototypeOf(val) === Object.prototype
     ) {
       const subKeys = ownKeys(val);
@@ -132,7 +137,7 @@ const checkPromiseOwnKeys = (pr, check) => {
  */
 const checkSafePromise = (pr, check) => {
   return (
-    (isFrozen(pr) || CX(check)`${pr} - Must be frozen`) &&
+    (isNonTrapping(pr) || CX(check)`${pr} - Must be frozen`) &&
     (isPromise(pr) || CX(check)`${pr} - Must be a promise`) &&
     (getPrototypeOf(pr) === Promise.prototype ||
       CX(check)`${pr} - Must inherit from Promise.prototype: ${q(

packages/pass-style/test/passStyleOf.test.js

@@ -13,35 +13,7 @@ const harden = /** @type {import('ses').Harden & { isFake?: boolean }} */ (
   global.harden
 );
 
-const { getPrototypeOf, defineProperty, freeze } = Object;
-/**
- * Local alias of `harden` to eventually be switched to whatever applies
- * the suppress-trapping integrity trait. For the shim at
- * https://github.com/endojs/endo/pull/2673
- * that is `suppressTrapping`, which is why we choose that name for the
- * placeholder here. But it is a separate definition so these aliased uses
- * do not yet depend on the final name.
- *
- * TODO Once we do have support for an explicit `suppressTrapping` operation,
- * we should import that instead, and if necessary rename all uses to that
- * operation's final name.
- */
-const hardenToBeSuppressTrapping = harden;
-
-/**
- * Local alias of `harden` to eventually be switched to whatever applies
- * the suppress-trapping integrity trait. For the shim at
- * https://github.com/endojs/endo/pull/2673
- * that is `suppressTrapping`, which is why we choose that name for the
- * placeholder here. But it is a separate definition so these aliased uses
- * do not yet depend on the final name.
- *
- * TODO Once we do have support for an explicit `suppressTrapping` operation,
- * we should import that instead, and if necessary rename all uses to that
- * operation's final name.
- */
-const freezeToBeSuppressTrapping = freeze;
-
+const { getPrototypeOf, defineProperty, suppressTrapping } = Object;
 const { ownKeys } = Reflect;
 
 test('passStyleOf basic success cases', t => {
@@ -224,23 +196,15 @@ test('passStyleOf testing remotables', t => {
   t.is(passStyleOf(Far('foo', () => 'far function')), 'remotable');
 
   const tagRecord1 = harden(makeTagishRecord('Alleged: manually constructed'));
-  const farObj1 = hardenToBeSuppressTrapping({ __proto__: tagRecord1 });
+  const farObj1 = suppressTrapping({ __proto__: tagRecord1 });
   t.is(passStyleOf(farObj1), 'remotable');
 
   const tagRecord2 = makeTagishRecord('Alleged: tagRecord not hardened');
   /**
    * Do not freeze `tagRecord2` in order to test that an object with
    * a non-frozen __proto__ is not passable.
-   *
-   * TODO In order to run this test before we have explicit support for a
-   * non-trapping integrity trait, we have to `freeze` here but not `harden`.
-   * However, once we do have that support, and `passStyleOf` checks that
-   * its argument is also non-trapping, we still need to avoid `harden`
-   * because that would also harden `__proto__`. So we will need to
-   * explicitly make this non-trapping, which we cannot yet express.
-   * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
    */
-  const farObj2 = freezeToBeSuppressTrapping({ __proto__: tagRecord2 });
+  const farObj2 = suppressTrapping({ __proto__: tagRecord2 });
   if (harden.isFake) {
     t.is(passStyleOf(farObj2), 'remotable');
   } else {
@@ -251,23 +215,23 @@ test('passStyleOf testing remotables', t => {
   }
 
   const tagRecord3 = harden(makeTagishRecord('Alleged: both manually frozen'));
-  const farObj3 = hardenToBeSuppressTrapping({ __proto__: tagRecord3 });
+  const farObj3 = suppressTrapping({ __proto__: tagRecord3 });
   t.is(passStyleOf(farObj3), 'remotable');
 
   const tagRecord4 = harden(makeTagishRecord('Remotable'));
-  const farObj4 = hardenToBeSuppressTrapping({ __proto__: tagRecord4 });
+  const farObj4 = suppressTrapping({ __proto__: tagRecord4 });
   t.is(passStyleOf(farObj4), 'remotable');
 
   const tagRecord5 = harden(makeTagishRecord('Not alleging'));
-  const farObj5 = hardenToBeSuppressTrapping({ __proto__: tagRecord5 });
+  const farObj5 = suppressTrapping({ __proto__: tagRecord5 });
   t.throws(() => passStyleOf(farObj5), {
     message:
       /For now, iface "Not alleging" must be "Remotable" or begin with "Alleged: " or "DebugName: "; unimplemented/,
   });
 
   const tagRecord6 = harden(makeTagishRecord('Alleged: manually constructed'));
-  const farObjProto6 = hardenToBeSuppressTrapping({ __proto__: tagRecord6 });
-  const farObj6 = hardenToBeSuppressTrapping({ __proto__: farObjProto6 });
+  const farObjProto6 = suppressTrapping({ __proto__: tagRecord6 });
+  const farObj6 = suppressTrapping({ __proto__: farObjProto6 });
   t.is(passStyleOf(farObj6), 'remotable', 'tagRecord grandproto is accepted');
 
   // Our current agoric-sdk plans for far classes are to create a class-like
@@ -321,7 +285,7 @@ test('passStyleOf testing remotables', t => {
   const tagRecordA1 = harden(
     makeTagishRecord('Alleged: null-proto tagRecord proto', null),
   );
-  const farObjA1 = hardenToBeSuppressTrapping({ __proto__: tagRecordA1 });
+  const farObjA1 = suppressTrapping({ __proto__: tagRecordA1 });
   t.throws(
     () => passStyleOf(farObjA1),
     { message: unusualTagRecordProtoMessage },
@@ -331,8 +295,8 @@ test('passStyleOf testing remotables', t => {
   const tagRecordA2 = harden(
     makeTagishRecord('Alleged: null-proto tagRecord grandproto', null),
   );
-  const farObjProtoA2 = hardenToBeSuppressTrapping({ __proto__: tagRecordA2 });
-  const farObjA2 = hardenToBeSuppressTrapping({ __proto__: farObjProtoA2 });
+  const farObjProtoA2 = suppressTrapping({ __proto__: tagRecordA2 });
+  const farObjA2 = suppressTrapping({ __proto__: farObjProtoA2 });
   t.throws(
     () => passStyleOf(farObjA2),
     { message: unusualTagRecordProtoMessage },
@@ -346,10 +310,10 @@ test('passStyleOf testing remotables', t => {
   const fauxTagRecordB = harden(
     makeTagishRecord('Alleged: manually constructed', harden({})),
   );
-  const farObjProtoB = hardenToBeSuppressTrapping({
+  const farObjProtoB = suppressTrapping({
     __proto__: fauxTagRecordB,
   });
-  const farObjB = hardenToBeSuppressTrapping({ __proto__: farObjProtoB });
+  const farObjB = suppressTrapping({ __proto__: farObjProtoB });
   t.throws(() => passStyleOf(farObjB), {
     message:
       'cannot serialize Remotables with non-methods like "Symbol(passStyle)" in "[Alleged: manually constructed]"',
@@ -360,7 +324,7 @@ test('passStyleOf testing remotables', t => {
   );
   Object.defineProperty(farObjProtoWithExtra, 'extra', { value: () => {} });
   harden(farObjProtoWithExtra);
-  const badFarObjExtraProtoProp = hardenToBeSuppressTrapping({
+  const badFarObjExtraProtoProp = suppressTrapping({
     __proto__: farObjProtoWithExtra,
   });
   t.throws(() => passStyleOf(badFarObjExtraProtoProp), {
@@ -425,7 +389,7 @@ test('remotables - safety from the gibson042 attack', t => {
    * explicitly make this non-trapping, which we cannot yet express.
    * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
    */
-  const makeInput = () => freeze({ __proto__: mercurialProto });
+  const makeInput = () => suppressTrapping({ __proto__: mercurialProto });
   const input1 = makeInput();
   const input2 = makeInput();
 
@@ -486,12 +450,12 @@ test('Allow toStringTag overrides', t => {
   t.is(`${alice}`, '[object DebugName: Allison]');
   t.is(`${q(alice)}`, '"[DebugName: Allison]"');
 
-  const carol = hardenToBeSuppressTrapping({ __proto__: alice });
+  const carol = suppressTrapping({ __proto__: alice });
   t.is(passStyleOf(carol), 'remotable');
   t.is(`${carol}`, '[object DebugName: Allison]');
   t.is(`${q(carol)}`, '"[DebugName: Allison]"');
 
-  const bob = hardenToBeSuppressTrapping({
+  const bob = suppressTrapping({
     __proto__: carol,
     [Symbol.toStringTag]: 'DebugName: Robert',
   });

packages/ses/package.json

@@ -85,7 +85,8 @@
     "postpack": "git clean -f '*.d.ts*' '*.tsbuildinfo'"
   },
   "dependencies": {
-    "@endo/env-options": "workspace:^"
+    "@endo/env-options": "workspace:^",
+    "@endo/non-trapping-shim": "^0.1.0"
   },
   "devDependencies": {
     "@endo/compartment-mapper": "workspace:^",

packages/ses/src/commons.js

@@ -14,6 +14,8 @@
 /* global globalThis */
 /* eslint-disable no-restricted-globals */
 
+import '@endo/non-trapping-shim/shim.js';
+
 // We cannot use globalThis as the local name since it would capture the
 // lexical name.
 const universalThis = globalThis;
@@ -75,6 +77,11 @@ export const {
   setPrototypeOf,
   values,
   fromEntries,
+  // https://github.com/endojs/endo/pull/2673
+  // @ts-expect-error TS does not yet have this on ObjectConstructor.
+  isNonTrapping,
+  // @ts-expect-error TS does not yet have this on ObjectConstructor.
+  suppressTrapping,
 } = Object;
 
 export const {
@@ -125,6 +132,9 @@ export const {
   ownKeys,
   preventExtensions: reflectPreventExtensions,
   set: reflectSet,
+  // https://github.com/endojs/endo/pull/2673
+  isNonTrapping: reflectIsNonTrapping,
+  suppressTrapping: reflectSuppressTrapping,
 } = Reflect;
 
 export const { isArray, prototype: arrayPrototype } = Array;

packages/ses/src/error/assert.js

@@ -293,7 +293,7 @@
   } = descs;
 
   const restNames = ownKeys(restDescs);
   if (restNames.length >= 1) {
     for (const name of restNames) {
       delete error[name];
     }
@@ -305,7 +305,6 @@
     );
   }
   for (const name of ownKeys(error)) {
-    // @ts-expect-error TS still confused by symbols as property names
     const desc = descs[name];
     if (desc && objectHasOwnProperty(desc, 'get')) {
       defineProperty(error, name, {