fix(exo): allow richer behaviorMethods

packages/exo/package.json

@@ -33,6 +33,7 @@
   },
   "dependencies": {
     "@endo/env-options": "^0.1.4",
+    "@endo/eventual-send": "^0.17.6",
     "@endo/far": "^0.2.22",
     "@endo/pass-style": "^0.1.7",
     "@endo/patterns": "^0.2.6"

packages/exo/src/exo-tools.js

@@ -1,5 +1,5 @@
+import { getMethodNames } from '@endo/eventual-send/src/local.js';
 import { E, Far } from '@endo/far';
-import { hasOwnPropertyOf } from '@endo/pass-style';
 import {
   listDifference,
   objectMap,
@@ -253,21 +253,6 @@ const bindMethod = (
  */
 export const GET_INTERFACE_GUARD = Symbol.for('getInterfaceGuard');
 
-/**
- *
- * @template {Record<PropertyKey, CallableFunction>} T
- * @param {T} behaviorMethods
- * @param {InterfaceGuard<{ [M in keyof T]: MethodGuard }>} interfaceGuard
- * @returns {T}
- */
-const withGetInterfaceGuardMethod = (behaviorMethods, interfaceGuard) =>
-  harden({
-    [GET_INTERFACE_GUARD]() {
-      return interfaceGuard;
-    },
-    ...behaviorMethods,
-  });
-
 /**
  * @template {Record<PropertyKey, CallableFunction>} T
  * @param {string} tag
@@ -285,13 +270,20 @@ export const defendPrototype = (
   interfaceGuard = undefined,
 ) => {
   const prototype = {};
-  if (hasOwnPropertyOf(behaviorMethods, 'constructor')) {
-    // By ignoring any method named "constructor", we can use a
+  const methodNames = getMethodNames(behaviorMethods).filter(
+    // By ignoring any method that seems to be a constructor, we can use a
     // class.prototype as a behaviorMethods.
-    const { constructor: _, ...methods } = behaviorMethods;
-    // @ts-expect-error TS misses that hasOwn check makes this safe
-    behaviorMethods = harden(methods);
-  }
+    key => {
+      if (key !== 'constructor') {
+        return true;
+      }
+      const constructor = behaviorMethods.constructor;
+      return !(
+        constructor.prototype &&
+        constructor.prototype.constructor === constructor
+      );
+    },
+  );
   /** @type {Record<PropertyKey, MethodGuard> | undefined} */
   let methodGuards;
   if (interfaceGuard) {
@@ -307,7 +299,6 @@ export const defendPrototype = (
         fromEntries(getCopyMapEntries(symbolMethodGuards))),
     });
     {
-      const methodNames = ownKeys(behaviorMethods);
       const methodGuardNames = ownKeys(methodGuards);
       const unimplemented = listDifference(methodGuardNames, methodNames);
       unimplemented.length === 0 ||
@@ -318,13 +309,9 @@ export const defendPrototype = (
           Fail`methods ${q(unguarded)} not guarded by ${q(interfaceName)}`;
       }
     }
-    behaviorMethods = withGetInterfaceGuardMethod(
-      behaviorMethods,
-      interfaceGuard,
-    );
   }
 
-  for (const prop of ownKeys(behaviorMethods)) {
+  for (const prop of methodNames) {
     prototype[prop] = bindMethod(
       `In ${q(prop)} method of (${tag})`,
       contextProvider,
@@ -335,6 +322,21 @@ export const defendPrototype = (
     );
   }
 
+  if (interfaceGuard) {
+    const getInterfaceGuardMethod = {
+      [GET_INTERFACE_GUARD]() {
+        return interfaceGuard;
+      },
+    }[GET_INTERFACE_GUARD];
+    prototype[GET_INTERFACE_GUARD] = bindMethod(
+      `In ${q(GET_INTERFACE_GUARD)} method of (${tag})`,
+      contextProvider,
+      getInterfaceGuardMethod,
+      thisfulMethods,
+      undefined,
+    );
+  }
+
   return Far(tag, /** @type {T} */ (prototype));
 };
 harden(defendPrototype);

packages/exo/test/test-exo-class-js-class.js

@@ -0,0 +1,79 @@
+/* eslint-disable max-classes-per-file */
+/* eslint-disable class-methods-use-this */
+// eslint-disable-next-line import/order
+import { test } from './prepare-test-env-ava.js';
+
+import { passStyleOf } from '@endo/pass-style';
+// import { M, getInterfaceGuardPayload } from '@endo/patterns';
+// import { defineExoClass, makeExo } from '../src/exo-makers.js';
+import { M, getInterfaceGuardPayload } from '@endo/patterns';
+import { makeExo, defineExoClass } from '../src/exo-makers.js';
+
+// Based on FarSubclass1 in test-far-class-instances.js
+class DoublerBehaviorClass {
+  double(x) {
+    return x + x;
+  }
+}
+
+const DoublerI = M.interface('Doubler', {
+  double: M.call(M.lte(10)).returns(M.number()),
+});
+
+const doubler = makeExo('doubler', DoublerI, DoublerBehaviorClass.prototype);
+
+test('exo doubler using js classes', t => {
+  t.is(passStyleOf(doubler), 'remotable');
+  t.is(doubler.double(3), 6);
+  t.throws(() => doubler.double('x'), {
+    message: 'In "double" method of (doubler): arg 0: "x" - Must be <= 10',
+  });
+  t.throws(() => doubler.double(), {
+    message:
+      'In "double" method of (doubler): Expected at least 1 arguments: []',
+  });
+  t.throws(() => doubler.double(12), {
+    message: 'In "double" method of (doubler): arg 0: 12 - Must be <= 10',
+  });
+});
+
+// Based on FarSubclass2 in test-far-class-instances.js
+class DoubleAdderBehaviorClass extends DoublerBehaviorClass {
+  doubleAddSelfCall(x) {
+    const {
+      state: { y },
+      self,
+    } = this;
+    return self.double(x) + y;
+  }
+
+  doubleAddSuperCall(x) {
+    const {
+      state: { y },
+    } = this;
+    return super.double(x) + y;
+  }
+}
+
+const DoubleAdderI = M.interface('DoubleAdder', {
+  ...getInterfaceGuardPayload(DoublerI).methodGuards,
+  doubleAddSelfCall: M.call(M.number()).returns(M.number()),
+  doubleAddSuperCall: M.call(M.number()).returns(M.number()),
+});
+
+const makeDoubleAdder = defineExoClass(
+  'doubleAdderClass',
+  DoubleAdderI,
+  y => ({ y }),
+  DoubleAdderBehaviorClass.prototype,
+);
+
+test('exo inheritance self vs super call', t => {
+  const da = makeDoubleAdder(5);
+  t.is(da.doubleAddSelfCall(3), 11);
+  t.throws(() => da.doubleAddSelfCall(12), {
+    message:
+      'In "double" method of (doubleAdderClass): arg 0: 12 - Must be <= 10',
+  });
+  t.is(da.doubleAddSuperCall(12), 29);
+});

packages/exo/test/test-exo-wobbly-point.js

@@ -0,0 +1,205 @@
+/**
+ * Based on the WobblyPoint inheritance examples in
+ * https://storage.googleapis.com/google-code-archive-downloads/v2/code.google.com/google-caja/caja-spec-2007-12-21.pdf
+ * and
+ * test-far-wobbly-point.js
+ */
+
+/* eslint-disable class-methods-use-this */
+/* eslint-disable max-classes-per-file */
+/* eslint-disable-next-line import/order */
+import { test } from './prepare-test-env-ava.js';
+
+// TODO enable import of getMethodNames without deep import
+// eslint-disable-next-line import/order
+import { getMethodNames } from '@endo/eventual-send/src/local.js';
+import { passStyleOf, Far } from '@endo/pass-style';
+import { M } from '@endo/patterns';
+import { defineExoClass } from '../src/exo-makers.js';
+import { GET_INTERFACE_GUARD } from '../src/exo-tools.js';
+
+const { Fail, quote: q } = assert;
+const { apply } = Reflect;
+
+const ExoEmptyI = M.interface('ExoEmpty', {});
+
+class ExoBaseClass {
+  constructor() {
+    Fail`Turn Exo JS classes into Exo classes with defineExoClassFromJSClass: ${q(
+      new.target.name,
+    )}`;
+  }
+
+  static implements = ExoEmptyI;
+
+  static init() {
+    return harden({});
+  }
+}
+
+const defineExoClassFromJSClass = klass =>
+  defineExoClass(klass.name, klass.implements, klass.init, klass.prototype);
+harden(defineExoClassFromJSClass);
+
+const ExoPointI = M.interface('ExoPoint', {
+  toString: M.call().returns(M.string()),
+  getX: M.call().returns(M.gte(0)),
+  getY: M.call().returns(M.number()),
+  setY: M.call(M.number()).returns(),
+});
+
+class ExoAbstractPoint extends ExoBaseClass {
+  static implements = ExoPointI;
+
+  toString() {
+    const { self } = this;
+    return `<${self.getX()},${self.getY()}>`;
+  }
+}
+
+test('cannot make abstract class concrete', t => {
+  t.throws(() => defineExoClassFromJSClass(ExoAbstractPoint), {
+    message:
+      'methods ["getX","getY","setY"] not implemented by "ExoAbstractPoint"',
+  });
+});
+
+class ExoPoint extends ExoAbstractPoint {
+  static init(x, y) {
+    // Heap exos currently use the returned record directly, so
+    // needs to not be frozen for `state.y` to be assignable.
+    // TODO not true for other zones. May make heap zone more like
+    // the others in treatment of `state`.
+    return { x, y };
+  }
+
+  getX() {
+    const {
+      state: { x },
+    } = this;
+    return x;
+  }
+
+  getY() {
+    const {
+      state: { y },
+    } = this;
+    return y;
+  }
+
+  setY(newY) {
+    const { state } = this;
+    state.y = newY;
+  }
+}
+harden(ExoPoint);
+
+const makeExoPoint = defineExoClassFromJSClass(ExoPoint);
+
+test('ExoPoint instances', t => {
+  const pt = makeExoPoint(3, 5);
+  t.is(passStyleOf(pt), 'remotable');
+  t.false(pt instanceof ExoPoint);
+  t.deepEqual(getMethodNames(pt), [
+    GET_INTERFACE_GUARD,
+    'getX',
+    'getY',
+    'setY',
+    'toString',
+  ]);
+  t.is(pt.getX(), 3);
+  t.is(pt.getY(), 5);
+  t.is(`${pt}`, '<3,5>');
+  pt.setY(6);
+  t.is(`${pt}`, '<3,6>');
+
+  const otherPt = makeExoPoint(1, 2);
+  t.is(apply(pt.getX, otherPt, []), 1);
+
+  const negPt = makeExoPoint(-3, 5);
+  t.throws(() => negPt.getX(), {
+    message: 'In "getX" method of (ExoPoint): result: -3 - Must be >= 0',
+  });
+  // `self` calls are guarded
+  t.throws(() => `${negPt}`, {
+    message: 'In "getX" method of (ExoPoint): result: -3 - Must be >= 0',
+  });
+});
+
+class ExoWobblyPoint extends ExoPoint {
+  static init(x, y, getWobble) {
+    return { ...super.init(x, y), getWobble };
+  }
+
+  getX() {
+    const {
+      state: { getWobble },
+    } = this;
+    return super.getX() + getWobble();
+  }
+}
+harden(ExoWobblyPoint);
+
+const makeExoWobblyPoint = defineExoClassFromJSClass(ExoWobblyPoint);
+
+test('FarWobblyPoint inheritance', t => {
+  let wobble = 0;
+  // For heap classes currently, there is no reason to make `getWobble` passable.
+  // But other zones insist on at least passability, and TODO we may eventually
+  // make the heap zone act like this as well.
+  const getWobble = Far('getW', () => (wobble += 1));
+  const wpt = makeExoWobblyPoint(3, 5, getWobble);
+  t.false(wpt instanceof ExoWobblyPoint);
+  t.false(wpt instanceof ExoPoint);
+  t.is(passStyleOf(wpt), 'remotable');
+  t.deepEqual(getMethodNames(wpt), [
+    GET_INTERFACE_GUARD,
+    'getX',
+    'getY',
+    'setY',
+    'toString',
+  ]);
+  t.is(`${wpt}`, '<4,5>');
+  t.is(`${wpt}`, '<5,5>');
+  t.is(`${wpt}`, '<6,5>');
+  wpt.setY(6);
+  t.is(`${wpt}`, '<7,6>');
+
+  const otherPt = makeExoPoint(1, 2);
+  t.false(otherPt instanceof ExoWobblyPoint);
+  t.throws(() => apply(wpt.getX, otherPt, []), {
+    message:
+      '"In \\"getX\\" method of (ExoWobblyPoint)" may only be applied to a valid instance: "[Alleged: ExoPoint]"',
+  });
+  t.throws(() => apply(wpt.getY, otherPt, []), {
+    message:
+      '"In \\"getY\\" method of (ExoWobblyPoint)" may only be applied to a valid instance: "[Alleged: ExoPoint]"',
+  });
+
+  const otherWpt = makeExoWobblyPoint(3, 5, () => 1);
+  t.is(`${otherWpt}`, '<4,5>');
+  t.is(apply(wpt.getX, otherWpt, []), 4);
+
+  // This error behavior shows the absence of the security vulnerability
+  // explained at the corresponding example in `test-far-wobbly-point.js`
+  // for the `@endo/pass-style` / `Far` level of abstraction. At the exo level
+  // of abstraction, a raw-method subclass overriding an inherited superclass
+  // method denies that method to clients of instances of the subclass.
+  // At the same time, this overridden method remains available within
+  // the overriding subclass via unguarded `super` calls.
+  t.throws(() => apply(otherPt.getX, otherWpt, []), {
+    message:
+      '"In \\"getX\\" method of (ExoPoint)" may only be applied to a valid instance: "[Alleged: ExoWobblyPoint]"',
+  });
+
+  const negWpt1 = makeExoWobblyPoint(-3, 5, () => 4);
+  t.is(negWpt1.getX(), 1);
+  // `super` calls are direct, bypassing guards and sharing context
+  t.is(`${negWpt1}`, '<1,5>');
+
+  const negWpt2 = makeExoWobblyPoint(1, 5, () => -4);
+  t.throws(() => `${negWpt2}`, {
+    // `self` calls are guarded
+    message: 'In "getX" method of (ExoWobblyPoint): result: -3 - Must be >= 0',
+  });
+});