feat(pass-style,exo): exo boundary only throws throwables

packages/exo/NEWS.md

@@ -1,5 +1,9 @@
 User-visible changes in `@endo/exo`:
 
+# Next release
+
+- A call to an exo will only throw a throwable, i.e., a Passable without capabilities, i.e., without Remotables or Promises. It will consist only of copy data and Passable errors. Passable errors themselves cannot contain capabilities, and so are throwable. An async exo `callWhen` method will likewise only reject with a throwable reason. Both contraints help security reviews, since experience shows it is too hard for reviewers to be adequately vigilant about capabilities communicated over the implicit exceptional control flow pathways.
+
 # v0.2.6 (2023-09-11)
 
 - Adds support for symbol-keyed methods in interface guards, e.g.

packages/exo/src/exo-tools.js

@@ -1,5 +1,5 @@
 import { getMethodNames } from '@endo/eventual-send/utils.js';
-import { hasOwnPropertyOf } from '@endo/pass-style';
+import { hasOwnPropertyOf, toThrowable } from '@endo/pass-style';
 import { E, Far } from '@endo/far';
 import {
   mustMatch,
@@ -164,14 +164,18 @@ const defendSyncMethod = (
   const { syncMethod } = {
     // Note purposeful use of `this` and concise method syntax
     syncMethod(...syncArgs) {
-      const context = getContext(this);
-      // Only harden args and return value if not dealing with a raw value guard.
-      const realArgs = defendSyncArgs(syncArgs, matchConfig, label);
-      const result = apply(behaviorMethod, context, realArgs);
-      if (!isRawReturn) {
-        mustMatch(harden(result), returnGuard, `${label}: result`);
+      try {
+        const context = getContext(this);
+        // Only harden args and return value if not dealing with a raw value guard.
+        const realArgs = defendSyncArgs(syncArgs, matchConfig, label);
+        const result = apply(behaviorMethod, context, realArgs);
+        if (!isRawReturn) {
+          mustMatch(harden(result), returnGuard, `${label}: result`);
+        }
+        return result;
+      } catch (thrownThing) {
+        throw toThrowable(thrownThing);
       }
-      return result;
     },
   };
   return syncMethod;
@@ -251,13 +255,16 @@ const defendAsyncMethod = (
           return apply(behaviorMethod, context, realArgs);
         },
       );
-      if (isRawReturn) {
-        return resultP;
-      }
-      return E.when(resultP, result => {
-        mustMatch(harden(result), returnGuard, `${label}: result`);
-        return result;
-      });
+      return E.when(resultP, fulfillment => {
+        if (!isRawReturn) {
+          mustMatch(harden(fulfillment), returnGuard, `${label}: result`);
+        }
+        return fulfillment;
+      }).catch(reason =>
+        // Done is a chained `.catch` rather than an onRejected clause of the
+        // `E.when` above in case the `mustMatch` throws.
+        Promise.reject(toThrowable(reason)),
+      );
     },
   };
   return asyncMethod;

packages/exo/test/test-exo-only-throwables.js

@@ -0,0 +1,51 @@
+import test from '@endo/ses-ava/prepare-endo.js';
+
+import { makeError } from '@endo/errors';
+import { Far, isPassable } from '@endo/pass-style';
+import { M } from '@endo/patterns';
+import { makeExo } from '../src/exo-makers.js';
+
+const { defineProperty } = Object;
+
+const thrower = makeExo(
+  'Thrower',
+  M.interface('Thrower', {
+    throw: M.call(M.raw()).returns(M.any()),
+    reject: M.callWhen(M.raw()).returns(M.any()),
+  }),
+  {
+    throw(val) {
+      throw val;
+    },
+    reject(val) {
+      throw val;
+    },
+  },
+);
+
+test('exo only throwables', async t => {
+  const e = makeError('test error', undefined, {
+    sanitize: false,
+  });
+
+  // Remotables cannot be in passable errors or throwables
+  defineProperty(e, 'foo', { value: Far('Foo', {}) });
+
+  let caught;
+  try {
+    thrower.throw(e);
+  } catch (thrown) {
+    caught = thrown;
+  }
+  t.false(isPassable(e));
+  t.true(isPassable(caught));
+  t.log('throw caught', caught);
+
+  try {
+    await thrower.reject(e);
+  } catch (thrown) {
+    caught = thrown;
+  }
+  t.true(isPassable(caught));
+  t.log('reject caught', caught);
+});

packages/pass-style/NEWS.md

@@ -1,5 +1,9 @@
 User-visible changes in `@endo/pass-style`:
 
+# Next release
+
+- Adds `toThrowable` as a generalization of `toPassableError` that also admits copy data containing passable errors, but still without passable caps, i.e, without remotables or promises. This is in support of the exo boundary throwing only throwables, to ease security review.
+
 # v1.3.0 (2024-03-19)
 
 - Exports `isWellFormedString` and `assertWellFormedString`. Unfortunately the [standard `String.prototype.isWellFormed`](https://tc39.es/proposal-is-usv-string/) first coerces its input to string, leading it to claim that some non-strings are well-formed strings. By contrast, `isWellFormedString` and `assertWellFormedString` will not judge any non-strings to be well-formed strings.

packages/pass-style/index.js

@@ -29,6 +29,7 @@ export {
   isPassable,
   assertPassable,
   toPassableError,
+  toThrowable,
 } from './src/passStyleOf.js';
 
 export { makeTagged } from './src/makeTagged.js';

packages/pass-style/src/passStyleOf.js

@@ -14,6 +14,7 @@ import {
   checkRecursivelyPassableErrorPropertyDesc,
   checkRecursivelyPassableError,
   getErrorConstructor,
+  isErrorLike,
 } from './error.js';
 import { RemotableHelper } from './remotable.js';
 
@@ -22,15 +23,15 @@ import { assertSafePromise } from './safe-promise.js';
 import { assertPassableString } from './string.js';
 
 /** @import {PassStyleHelper} from './internal-types.js' */
-/** @import {Passable} from './types.js' */
+/** @import {CopyArray, CopyRecord, CopyTagged, Passable} from './types.js' */
 /** @import {PassStyle} from './types.js' */
 /** @import {PassStyleOf} from './types.js' */
 /** @import {PrimitiveStyle} from './types.js' */
 
 /** @typedef {Exclude<PassStyle, PrimitiveStyle | "promise">} HelperPassStyle */
 
 const { ownKeys } = Reflect;
-const { isFrozen, getOwnPropertyDescriptors } = Object;
+const { isFrozen, getOwnPropertyDescriptors, values } = Object;
 
 /**
  * @param {PassStyleHelper[]} passStyleHelpers
@@ -262,10 +263,14 @@ const isPassableErrorPropertyDesc = (name, desc) =>
   checkRecursivelyPassableErrorPropertyDesc(name, desc, passStyleOf);
 
 /**
- * Return a passable error that propagates the diagnostic info of the
- * original, and is linked to the original as a note.
- * `toPassableError` hardens the argument before checking if it is already
- * a passable error. If it is, then `toPassableError` returns the argument.
+ * After hardening, if `err` is a passable error, return it.
+ *
+ * Otherwise, return a new passable error that propagates the diagnostic
+ * info of the original, and is linked to the original as a note.
+ *
+ * TODO Adopt a more flexible notion of passable error, in which
+ * a passable error can contain other own data properties with
+ * throwable values.
  *
  * @param {Error} err
  * @returns {Error}
@@ -304,3 +309,75 @@ export const toPassableError = err => {
   return newError;
 };
 harden(toPassableError);
+
+/**
+ * After hardening, if `specimen` is throwable, return it.
+ * A specimen is throwable iff it is Passable and contains no PassableCaps,
+ * i.e., no Remotables or Promises.
+ * IOW, if it contains only copy-data and passable errors.
+ *
+ * Otherwise, if `specimen` is *almost* throwable, for example, it is
+ * an error that can be made throwable by `toPassableError`, then
+ * return `specimen` converted to a throwable.
+ *
+ * Otherwise, throw a diagnostic indicating a failure to coerce.
+ *
+ * This is in support of the exo boundary throwing only throwables, to ease
+ * security review.
+ *
+ * TODO Adopt a more flexitble notion of throwable, in which
+ * data containers containing non-passable errors can themselves be coerced
+ * to throwable by coercing to a similar containers containing
+ * the results of coercing those errors to passable errors.
+ *
+ * @param {unknown} specimen
+ * @returns {Passable<never, Error>}
+ */
+export const toThrowable = specimen => {
+  harden(specimen);
+  if (isErrorLike(specimen)) {
+    return toPassableError(/** @type {Error} */ (specimen));
+  }
+  // Note that this step will fail if `specimen` would be a passable container
+  // except that it contains non-passable errors that could be converted.
+  // This will need to be fixed to do the TODO above.
+  const passStyle = passStyleOf(specimen);
+  if (isObject(specimen)) {
+    switch (passStyle) {
+      case 'copyArray': {
+        const elements = /** @type {CopyArray} */ (specimen);
+        for (const element of elements) {
+          element === toThrowable(element) ||
+            Fail`nested toThrowable coercion not yet supported ${element}`;
+        }
+        break;
+      }
+      case 'copyRecord': {
+        const rec = /** @type {CopyRecord} */ (specimen);
+        for (const val of values(rec)) {
+          val === toThrowable(val) ||
+            Fail`nested toThrowable coercion not yet supported ${val}`;
+        }
+        break;
+      }
+      case 'tagged': {
+        const tg = /** @type {CopyTagged} */ (specimen);
+        const { payload } = tg;
+        payload === toThrowable(payload) ||
+          Fail`nested toThrowable coercion not yet supported ${payload}`;
+        break;
+      }
+      case 'error': {
+        const er = /** @type {Error} */ (specimen);
+        er === toThrowable(er) ||
+          Fail`nested toThrowable coercion not yet supported ${er}`;
+        break;
+      }
+      default: {
+        throw Fail`A ${q(passStyle)} is not throwable: ${specimen}`;
+      }
+    }
+  }
+  return /** @type {Passable<never,never>} */ (specimen);
+};
+harden(toThrowable);

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

@@ -6,7 +6,12 @@ import {
   passStyleOf,
   isPassable,
   toPassableError,
+  toThrowable,
 } from '../src/passStyleOf.js';
+import { Far } from '../src/make-far.js';
+import { makeTagged } from '../src/makeTagged.js';
+
+const { defineProperty } = Object;
 
 test('style of extended errors', t => {
   const e1 = Error('e1');
@@ -29,10 +34,14 @@ test('style of extended errors', t => {
   }
 });
 
-test('toPassableError rejects unfrozen errors', t => {
+test('toPassableError, toThrowable', t => {
   const e = makeError('test error', undefined, {
     sanitize: false,
   });
+
+  // Remotables cannot be in passable errors or throwables
+  defineProperty(e, 'foo', { value: Far('Foo', {}) });
+
   // I include this test because I was recently surprised that the errors
   // made by `makeError` are not frozen, and therefore not passable.
   // Since then, we changed `makeError` to make reasonable effort
@@ -45,12 +54,56 @@ test('toPassableError rejects unfrozen errors', t => {
   // is a passable error.
   const e2 = toPassableError(e);
 
+  t.true(Object.isFrozen(e));
+  t.false(isPassable(e));
+
   t.true(Object.isFrozen(e2));
   t.true(isPassable(e2));
 
-  // May not be true on all platforms, depending on what "extraneous"
-  // properties the host added to the error before `makeError` returned it.
-  // If this fails, please let us know. See the doccomment on the
-  // `sanitizeError` function is the ses-shim's `assert.js`.
-  t.is(e, e2);
+  t.not(e, e2);
+  t.log('passable', e2);
+
+  t.is(e2, toThrowable(e2));
+  t.deepEqual(toThrowable(e), e2);
+
+  const notYetCoercable = harden([e]);
+  // Note: eventually `toThrowable(notYetCoercable)` should return
+  // a throwable singleton copyArray containing a toThrowable(e), i.e.,
+  // an error like e2.
+  t.throws(() => toThrowable(notYetCoercable), {
+    message: 'Passable Error has extra unpassed property "foo"',
+  });
+
+  const throwable = harden([e2, { e2 }, makeTagged('e2', e2)]);
+  t.is(throwable, toThrowable(throwable));
+});
+
+/**
+ * Copied from
+ * https://github.com/Agoric/agoric-sdk/blob/286302a192b9eb2e222faa08479f496645bb7b9a/packages/internal/src/upgrade-api.js#L25-L39
+ * to verify that an UpgradeDisconnection object is throwable, as we need it
+ * to be.
+ *
+ * Makes an Error-like object for use as the rejection reason of promises
+ * abandoned by upgrade.
+ *
+ * @param {string} upgradeMessage
+ * @param {number} toIncarnationNumber
+ * @returns {UpgradeDisconnection}
+ */
+const makeUpgradeDisconnection = (upgradeMessage, toIncarnationNumber) =>
+  harden({
+    name: 'vatUpgraded',
+    upgradeMessage,
+    incarnationNumber: toIncarnationNumber,
+  });
+
+/**
+ * Copied from
+ * https://github.com/Agoric/agoric-sdk/blob/286302a192b9eb2e222faa08479f496645bb7b9a/packages/internal/test/test-upgrade-api.js#L9
+ */
+const disconnection = makeUpgradeDisconnection('vat upgraded', 2);
+
+test('UpgradeDisconnection is throwable', t => {
+  t.is(toThrowable(disconnection), disconnection);
 });