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/package.json

@@ -46,7 +46,8 @@
     "@endo/eventual-send": "workspace:^",
     "@endo/nat": "workspace:^",
     "@endo/pass-style": "workspace:^",
-    "@endo/promise-kit": "workspace:^"
+    "@endo/promise-kit": "workspace:^",
+    "ses": "workspace:^"
   },
   "devDependencies": {
     "@endo/init": "workspace:^",

packages/marshal/src/encodeToCapData.js

@@ -6,6 +6,9 @@
 // encodes to CapData, a JSON-representable data structure, and leaves it to
 // the caller (`marshal.js`) to stringify it.
 
+import { X, Fail, q } from '@endo/errors';
+import { freezeOrSuppressTrapping } from 'ses/nonTrappingShimAdapter.js';
+
 import {
   passStyleOf,
   isErrorLike,
@@ -17,7 +20,6 @@ import {
   nameForPassableSymbol,
   passableSymbolForName,
 } from '@endo/pass-style';
-import { X, Fail, q } from '@endo/errors';
 
 /** @import {Passable, RemotableObject} from '@endo/pass-style' */
 /** @import {Encoding, EncodingUnion} from './types.js' */
@@ -30,7 +32,6 @@ const {
   is,
   entries,
   fromEntries,
-  freeze,
 } = Object;
 
 /**
@@ -176,10 +177,11 @@ 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 `freezeOrSuppressTrapping` 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));
+            // as input in another call to `encodeToCapData`.
+            result.rest = encodeToCapDataRecur(freezeOrSuppressTrapping(rest));
           }
           return result;
         }

packages/pass-style/package.json

@@ -37,7 +37,8 @@
     "@endo/env-options": "workspace:^",
     "@endo/errors": "workspace:^",
     "@endo/eventual-send": "workspace:^",
-    "@endo/promise-kit": "workspace:^"
+    "@endo/promise-kit": "workspace:^",
+    "ses": "workspace:^"
   },
   "devDependencies": {
     "@endo/init": "workspace:^",

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

@@ -4,6 +4,7 @@
 /** @import {PassStyle} from './types.js' */
 
 import { X, q } from '@endo/errors';
+import { isFrozenOrIsNonTrapping } from 'ses/nonTrappingShimAdapter.js';
 
 const { isArray } = Array;
 const { prototype: functionPrototype } = Function;
@@ -167,6 +168,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}`)) &&
+      (isFrozenOrIsNonTrapping(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

@@ -2,6 +2,7 @@
 
 /// <reference types="ses"/>
 
+import { isFrozenOrIsNonTrapping } from 'ses/nonTrappingShimAdapter.js';
 import { isPromise } from '@endo/promise-kit';
 import { X, Fail, q, annotateError, makeError } from '@endo/errors';
 import {
@@ -163,14 +164,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 (!isFrozenOrIsNonTrapping(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);
@@ -197,8 +201,12 @@ const makePassStyleOf = passStyleHelpers => {
           return 'remotable';
         }
         case 'function': {
-          isFrozen(inner) ||
-            Fail`Cannot pass non-frozen objects like ${inner}. Use harden()`;
+          if (!isFrozenOrIsNonTrapping(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`;
           assertValid(remotableHelper, inner, passStyleOfRecur);

packages/pass-style/src/remotable.js

@@ -1,6 +1,7 @@
 /// <reference types="ses"/>
 
 import { Fail, q } from '@endo/errors';
+import { isFrozenOrIsNonTrapping } from 'ses/nonTrappingShimAdapter.js';
 import {
   assertChecker,
   canBeMethod,
@@ -24,7 +25,6 @@ const { ownKeys } = Reflect;
 const { isArray } = Array;
 const {
   getPrototypeOf,
-  isFrozen,
   prototype: objectPrototype,
   getOwnPropertyDescriptors,
 } = Object;
@@ -154,7 +154,7 @@ const checkRemotable = (val, check) => {
   if (confirmedRemotables.has(val)) {
     return true;
   }
-  if (!isFrozen(val)) {
+  if (!isFrozenOrIsNonTrapping(val)) {
     return (
       !!check && CX(check)`cannot serialize non-frozen objects like ${val}`
     );

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

@@ -2,15 +2,87 @@
 
 import { isPromise } from '@endo/promise-kit';
 import { q } from '@endo/errors';
-import { assertChecker, hasOwnPropertyOf, CX } from './passStyle-helpers.js';
+import { isFrozenOrIsNonTrapping } from 'ses/nonTrappingShimAdapter.js';
+import {
+  assertChecker,
+  hasOwnPropertyOf,
+  CX,
+  isObject,
+} from './passStyle-helpers.js';
 
 /** @import {Checker} from './types.js' */
 
-const { isFrozen, getPrototypeOf, getOwnPropertyDescriptor } = Object;
+const { getPrototypeOf, getOwnPropertyDescriptor } = Object;
 const { ownKeys } = Reflect;
-const { toStringTag } = Symbol;
 
 /**
+ * Explicitly tolerate symbol-named non-configurable non-writable data
+ * property whose value is obviously harmless, such as a primitive value.
+ *
+ * The motivations are to tolerate `@@toStringTag` and those properties
+ * that might be added by Node's async_hooks. Thus, beyond primitives, the
+ * only values that must be tolerated are those safe values that might be
+ * added by async_hooks.
+ *
+ * At the time of this writing, Node's async_hooks contains the
+ * following code, which we need to tolerate if safe:
+ *
+ * ```js
+ * function destroyTracking(promise, parent) {
+ *   trackPromise(promise, parent);
+ *   const asyncId = promise[async_id_symbol];
+ *   const destroyed = { destroyed: false };
+ *   promise[destroyedSymbol] = destroyed;
+ *   registerDestroyHook(promise, asyncId, destroyed);
+ * }
+ * ```
+ *
+ * @param {object} obj
+ * @param {string|symbol} key
+ * @param {Checker} check
+ */
+const checkSafeOwnKeyOf = (obj, key, check) => {
+  const desc = getOwnPropertyDescriptor(obj, key);
+  assert(desc);
+  const quoteKey = q(String(key));
+  if (!hasOwnPropertyOf(desc, 'value')) {
+    return CX(
+      check,
+    )`Own ${quoteKey} must be a data property, not an accessor: ${obj}`;
+  }
+  const { value, writable, configurable } = desc;
+  if (writable) {
+    return CX(check)`Own ${quoteKey} must not be writable: ${obj}`;
+  }
+  if (configurable) {
+    return CX(check)`Own ${quoteKey} must not be configurable: ${obj}`;
+  }
+  if (!isObject(value)) {
+    return true;
+  }
+
+  if (
+    typeof value === 'object' &&
+    value !== null &&
+    isFrozenOrIsNonTrapping(value) &&
+    getPrototypeOf(value) === Object.prototype
+  ) {
+    const subKeys = ownKeys(value);
+    if (subKeys.length === 0) {
+      return true;
+    }
+
+    if (subKeys.length === 1 && subKeys[0] === 'destroyed') {
+      return checkSafeOwnKeyOf(value, 'destroyed', check);
+    }
+  }
+  return CX(
+    check,
+  )`Unexpected Node async_hooks additions: ${obj}[${quoteKey}] is ${value}`;
+};
+
+/**
+ * @see https://github.com/endojs/endo/issues/2700
  * @param {Promise} pr The value to examine
  * @param {Checker} check
  * @returns {pr is Promise} Whether it is a safe promise
@@ -22,96 +94,15 @@ const checkPromiseOwnKeys = (pr, check) => {
     return true;
   }
 
-  /**
-   * This excludes those symbol-named own properties that are also found on
-   * `Promise.prototype`, so that overrides of these properties can be
-   * explicitly tolerated if they pass the `checkSafeOwnKey` check below.
-   * In particular, we wish to tolerate
-   *   * An overriding `toStringTag` non-enumerable data property
-   *     with a string value.
-   *   * Those own properties that might be added by Node's async_hooks.
-   */
-  const unknownKeys = keys.filter(
-    key => typeof key !== 'symbol' || !hasOwnPropertyOf(Promise.prototype, key),
-  );
+  const stringKeys = keys.filter(key => typeof key !== 'symbol');
 
-  if (unknownKeys.length !== 0) {
+  if (stringKeys.length !== 0) {
     return CX(
       check,
-    )`${pr} - Must not have any own properties: ${q(unknownKeys)}`;
+    )`${pr} - Must not have any string-named own properties: ${q(stringKeys)}`;
   }
 
-  /**
-   * Explicitly tolerate a `toStringTag` symbol-named non-enumerable
-   * data property whose value is a string. Otherwise, tolerate those
-   * symbol-named properties that might be added by NodeJS's async_hooks,
-   * if they obey the expected safety properties.
-   *
-   * At the time of this writing, Node's async_hooks contains the
-   * following code, which we can safely tolerate
-   *
-   * ```js
-   * function destroyTracking(promise, parent) {
-   *   trackPromise(promise, parent);
-   *   const asyncId = promise[async_id_symbol];
-   *   const destroyed = { destroyed: false };
-   *   promise[destroyedSymbol] = destroyed;
-   *   registerDestroyHook(promise, asyncId, destroyed);
-   * }
-   * ```
-   *
-   * @param {string|symbol} key
-   */
-  const checkSafeOwnKey = key => {
-    if (key === toStringTag) {
-      // TODO should we also enforce anything on the contents of the string,
-      // such as that it must start with `'Promise'`?
-      const tagDesc = getOwnPropertyDescriptor(pr, toStringTag);
-      assert(tagDesc !== undefined);
-      return (
-        (hasOwnPropertyOf(tagDesc, 'value') ||
-          CX(
-            check,
-          )`Own @@toStringTag must be a data property, not an accessor: ${q(tagDesc)}`) &&
-        (typeof tagDesc.value === 'string' ||
-          CX(
-            check,
-          )`Own @@toStringTag value must be a string: ${q(tagDesc.value)}`) &&
-        (!tagDesc.enumerable ||
-          CX(check)`Own @@toStringTag must not be enumerable: ${q(tagDesc)}`)
-      );
-    }
-    const val = pr[key];
-    if (val === undefined || typeof val === 'number') {
-      return true;
-    }
-    if (
-      typeof val === 'object' &&
-      val !== null &&
-      isFrozen(val) &&
-      getPrototypeOf(val) === Object.prototype
-    ) {
-      const subKeys = ownKeys(val);
-      if (subKeys.length === 0) {
-        return true;
-      }
-
-      if (
-        subKeys.length === 1 &&
-        subKeys[0] === 'destroyed' &&
-        val.destroyed === false
-      ) {
-        return true;
-      }
-    }
-    return CX(
-      check,
-    )`Unexpected Node async_hooks additions to promise: ${pr}.${q(
-      String(key),
-    )} is ${val}`;
-  };
-
-  return keys.every(checkSafeOwnKey);
+  return keys.every(key => checkSafeOwnKeyOf(pr, key, check));
 };
 
 /**
@@ -132,7 +123,7 @@ const checkPromiseOwnKeys = (pr, check) => {
  */
 const checkSafePromise = (pr, check) => {
   return (
-    (isFrozen(pr) || CX(check)`${pr} - Must be frozen`) &&
+    (isFrozenOrIsNonTrapping(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(