diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3b8aba6f03..b353f62e78 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,6 +18,10 @@ Everything else is wired up thanks to workspaces, so no need to run installs in # Making a Release +* Review the [next release]( +https://github.com/endojs/endo/labels/next-release +) label for additional tasks or pending changes particular to this release. + * Do not release from a Git workspace. In a Git workspace, `.git` is a file and not a directory. At time of writing, Lerna does not account for Git workspaces when it looks diff --git a/packages/eventual-send/package.json b/packages/eventual-send/package.json index 232e1a42da..388018c587 100644 --- a/packages/eventual-send/package.json +++ b/packages/eventual-send/package.json @@ -18,6 +18,12 @@ "lint:types": "tsc", "lint:eslint": "eslint '**/*.js'" }, + "exports": { + "./package.json": "./package.json", + ".": "./src/no-shim.js", + "./shim.js": "./shim.js", + "./utils.js": "./utils.js" + }, "repository": { "type": "git", "url": "git+https://github.com/endojs/endo.git" diff --git a/packages/eventual-send/utils.js b/packages/eventual-send/utils.js new file mode 100644 index 0000000000..4fee534df0 --- /dev/null +++ b/packages/eventual-send/utils.js @@ -0,0 +1 @@ +export { getMethodNames } from './src/local.js'; diff --git a/packages/exo/package.json b/packages/exo/package.json index a0c68bc9ff..6ab75bb960 100644 --- a/packages/exo/package.json +++ b/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" diff --git a/packages/exo/src/exo-tools.js b/packages/exo/src/exo-tools.js index 206ea77fa9..41b83a04be 100644 --- a/packages/exo/src/exo-tools.js +++ b/packages/exo/src/exo-tools.js @@ -1,5 +1,6 @@ -import { E, Far } from '@endo/far'; +import { getMethodNames } from '@endo/eventual-send/utils.js'; import { hasOwnPropertyOf } from '@endo/pass-style'; +import { E, Far } from '@endo/far'; import { listDifference, objectMap, @@ -12,7 +13,6 @@ import { getInterfaceGuardPayload, getCopyMapEntries, } from '@endo/patterns'; - import { GET_INTERFACE_GUARD } from './get-interface.js'; /** @typedef {import('@endo/patterns').Method} Method */ @@ -182,6 +182,9 @@ const defendSyncMethod = (method, methodGuardPayload, label) => { return syncMethod; }; +/** + * @param {MethodGuardPayload} methodGuardPayload + */ const desync = methodGuardPayload => { const { argGuards, @@ -210,22 +213,43 @@ const desync = methodGuardPayload => { }; }; +/** + * @param {(...args: unknown[]) => any} method + * @param {MethodGuardPayload} methodGuardPayload + * @param {string} label + */ const defendAsyncMethod = (method, methodGuardPayload, label) => { const { returnGuard } = methodGuardPayload; + const isRawReturn = isRawGuard(returnGuard); + const { awaitIndexes, rawMethodGuardPayload } = desync(methodGuardPayload); + const matchConfig = buildMatchConfig(rawMethodGuardPayload); + const { asyncMethod } = { // Note purposeful use of `this` and concise method syntax asyncMethod(...args) { - const awaitList = awaitIndexes.map(i => args[i]); + const awaitList = []; + for (const i of awaitIndexes) { + if (i >= args.length) { + break; + } + awaitList.push(args[i]); + } const p = Promise.all(awaitList); const syncArgs = [...args]; - const resultP = E.when(p, awaitedArgs => { - for (let j = 0; j < awaitIndexes.length; j += 1) { - syncArgs[awaitIndexes[j]] = awaitedArgs[j]; - } - const realArgs = defendSyncArgs(syncArgs, rawMethodGuardPayload, label); - return apply(method, this, realArgs); - }); + const resultP = E.when( + p, + /** @param {any[]} awaitedArgs */ awaitedArgs => { + for (let j = 0; j < awaitedArgs.length; j += 1) { + syncArgs[awaitIndexes[j]] = awaitedArgs[j]; + } + const realArgs = defendSyncArgs(syncArgs, matchConfig, label); + return apply(method, this, realArgs); + }, + ); + if (isRawReturn) { + return resultP; + } return E.when(resultP, result => { mustMatch(harden(result), returnGuard, `${label}: result`); return result; @@ -353,21 +377,6 @@ const bindMethod = ( return method; }; -/** - * - * @template {Record} T - * @param {T} behaviorMethods - * @param {InterfaceGuard<{ [M in keyof T]: MethodGuard }>} interfaceGuard - * @returns {T & import('./get-interface.js').GetInterfaceGuard} - */ -const withGetInterfaceGuardMethod = (behaviorMethods, interfaceGuard) => - harden({ - [GET_INTERFACE_GUARD]() { - return interfaceGuard; - }, - ...behaviorMethods, - }); - /** * @template {Record} T * @param {string} tag @@ -384,13 +393,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 | undefined} */ let methodGuards; /** @type {import('@endo/patterns').DefaultGuardType} */ @@ -410,8 +426,6 @@ export const defendPrototype = ( }); defaultGuards = dg; { - const methodNames = ownKeys(behaviorMethods); - assert(methodGuards); const methodGuardNames = ownKeys(methodGuards); const unimplemented = listDifference(methodGuardNames, methodNames); unimplemented.length === 0 || @@ -422,12 +436,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, @@ -439,6 +450,22 @@ export const defendPrototype = ( ); } + if (!hasOwnPropertyOf(prototype, GET_INTERFACE_GUARD)) { + const getInterfaceGuardMethod = { + [GET_INTERFACE_GUARD]() { + // Note: May be `undefined` + 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 & import('./get-interface.js').GetInterfaceGuard} */ ( diff --git a/packages/exo/src/get-interface.js b/packages/exo/src/get-interface.js index 9fd4baf9d0..1657cf3c53 100644 --- a/packages/exo/src/get-interface.js +++ b/packages/exo/src/get-interface.js @@ -4,19 +4,23 @@ * The name of the automatically added default meta-method for * obtaining an exo's interface, if it has one. * + * Intended to be similar to `GET_METHOD_NAMES` from `@endo/pass-style`. + * * TODO Name to be bikeshed. Perhaps even whether it is a - * string or symbol to be bikeshed. + * string or symbol to be bikeshed. See + * https://github.com/endojs/endo/pull/1809#discussion_r1388052454 * * TODO Beware that an exo's interface can change across an upgrade, * so remotes that cache it can become stale. */ -export const GET_INTERFACE_GUARD = Symbol.for('getInterfaceGuard'); +export const GET_INTERFACE_GUARD = '__getInterfaceGuard__'; /** * @template {Record} M * @typedef {{ - * [GET_INTERFACE_GUARD]: () => import('@endo/patterns').InterfaceGuard<{ - * [K in keyof M]: import('@endo/patterns').MethodGuard - * }> + * [GET_INTERFACE_GUARD]: () => + * import('@endo/patterns').InterfaceGuard<{ + * [K in keyof M]: import('@endo/patterns').MethodGuard + * }> | undefined * }} GetInterfaceGuard */ diff --git a/packages/exo/test/test-exo-class-js-class.js b/packages/exo/test/test-exo-class-js-class.js new file mode 100644 index 0000000000..8cc19eae56 --- /dev/null +++ b/packages/exo/test/test-exo-class-js-class.js @@ -0,0 +1,77 @@ +/* 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 { 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); +}); diff --git a/packages/exo/test/test-exo-wobbly-point.js b/packages/exo/test/test-exo-wobbly-point.js new file mode 100644 index 0000000000..59ecc6e380 --- /dev/null +++ b/packages/exo/test/test-exo-wobbly-point.js @@ -0,0 +1,211 @@ +/** + * 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'; + +// eslint-disable-next-line import/order +import { getMethodNames } from '@endo/eventual-send/utils.js'; +import { passStyleOf, Far, GET_METHOD_NAMES } from '@endo/pass-style'; +import { M } from '@endo/patterns'; +import { defineExoClass } from '../src/exo-makers.js'; +import { GET_INTERFACE_GUARD } from '../src/get-interface.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); + +const assertMethodNames = (t, obj, names) => { + t.deepEqual(getMethodNames(obj), names); + t.deepEqual(obj[GET_METHOD_NAMES](), names); +}; + +test('ExoPoint instances', t => { + const pt = makeExoPoint(3, 5); + t.is(passStyleOf(pt), 'remotable'); + t.false(pt instanceof ExoPoint); + assertMethodNames(t, pt, [ + GET_INTERFACE_GUARD, + GET_METHOD_NAMES, + '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'); + assertMethodNames(t, wpt, [ + GET_INTERFACE_GUARD, + GET_METHOD_NAMES, + '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', + }); +}); diff --git a/packages/exo/test/test-heap-classes.js b/packages/exo/test/test-heap-classes.js index a5ef1346a5..c6c3525cf7 100644 --- a/packages/exo/test/test-heap-classes.js +++ b/packages/exo/test/test-heap-classes.js @@ -24,6 +24,19 @@ test('what happens with extra arguments', t => { exo.foo('an extra arg'); }); +const OptionalArrayI = M.interface('OptionalArray', { + foo: M.callWhen().optional(M.arrayOf(M.any())).returns(), +}); + +test('callWhen-guarded method called without optional array argument', async t => { + const exo = makeExo('WithNoOption', OptionalArrayI, { + async foo(arr) { + t.is(arr, undefined); + }, + }); + await t.notThrowsAsync(() => exo.foo()); +}); + const UpCounterI = M.interface('UpCounter', { incr: M.call() // TODO M.number() should not be needed to get a better error message @@ -172,9 +185,7 @@ test('missing interface', t => { message: 'In "makeSayHello" method of (greeterMaker): result: "[Symbol(passStyle)]" property expected: "[Function ]"', }); - t.throws(() => greeterMaker[GET_INTERFACE_GUARD](), { - message: 'greeterMaker[GET_INTERFACE_GUARD] is not a function', - }); + t.is(greeterMaker[GET_INTERFACE_GUARD](), undefined); }); const SloppyGreeterI = M.interface('greeter', {}, { sloppy: true }); @@ -334,7 +345,7 @@ test('naked function call', t => { t.throws(() => gigm(), { message: - 'thisful method "In \\"[Symbol(getInterfaceGuard)]\\" method of (greeter)" called without \'this\' object', + 'thisful method "In \\"__getInterfaceGuard__\\" method of (greeter)" called without \'this\' object', }); t.deepEqual(gigm.bind(greeter)(), GreeterI); }); diff --git a/packages/exo/test/test-non-enumerable-methods.js b/packages/exo/test/test-non-enumerable-methods.js new file mode 100644 index 0000000000..7ed454b162 --- /dev/null +++ b/packages/exo/test/test-non-enumerable-methods.js @@ -0,0 +1,129 @@ +// eslint-disable-next-line import/order +import { test } from './prepare-test-env-ava.js'; + +// eslint-disable-next-line import/order +import { getInterfaceMethodKeys, M } from '@endo/patterns'; +import { defineExoClass } from '../src/exo-makers.js'; +import { GET_INTERFACE_GUARD } from '../src/get-interface.js'; + +const { getPrototypeOf, getOwnPropertyDescriptors, create, fromEntries } = + Object; + +const { ownKeys } = Reflect; + +/** + * Borrowed from https://github.com/endojs/endo/pull/1815 to avoid + * depending on it being merged. TODO If it is merged, then delete this + * copy and import `objectMetaMap` instead. + * + * Like `objectMap`, but at the reflective level of property descriptors + * rather than property values. + * + * Except for hardening, the edge case behavior is mostly the opposite of + * the `objectMap` edge cases. + * * No matter how mutable the original object, the returned object is + * hardened. + * * All own properties of the original are mapped, even if symbol-named + * or non-enumerable. + * * If any of the original properties were accessors, the descriptor + * containing the getter and setter are given to `metaMapFn`. + * * The own properties of the returned are according to the descriptors + * returned by `metaMapFn`. + * * The returned object will always be a plain object whose state is + * only these mapped own properties. It will inherit from the third + * argument if provided, defaulting to `Object.prototype` if omitted. + * + * Because a property descriptor is distinct from `undefined`, we bundle + * mapping and filtering together. When the `metaMapFn` returns `undefined`, + * that property is omitted from the result. + * + * @template {Record} O + * @param {O} original + * @param {( + * desc: TypedPropertyDescriptor, + * key: keyof O + * ) => (PropertyDescriptor | undefined)} metaMapFn + * @param {any} [proto] + * @returns {any} + */ +export const objectMetaMap = ( + original, + metaMapFn, + proto = Object.prototype, +) => { + const descs = getOwnPropertyDescriptors(original); + const keys = ownKeys(original); + + const descEntries = /** @type {[PropertyKey,PropertyDescriptor][]} */ ( + keys + .map(key => [key, metaMapFn(descs[key], key)]) + .filter(([_key, optDesc]) => optDesc !== undefined) + ); + return harden(create(proto, fromEntries(descEntries))); +}; +harden(objectMetaMap); + +const UpCounterI = M.interface('UpCounter', { + incr: M.call() + // TODO M.number() should not be needed to get a better error message + .optional(M.and(M.number(), M.gte(0))) + .returns(M.number()), +}); + +const denumerate = obj => + objectMetaMap( + obj, + desc => ({ ...desc, enumerable: false }), + getPrototypeOf(obj), + ); + +test('test defineExoClass', t => { + const makeUpCounter = defineExoClass( + 'UpCounter', + UpCounterI, + /** @param {number} x */ + (x = 0) => ({ x }), + denumerate({ + incr(y = 1) { + const { state } = this; + state.x += y; + return state.x; + }, + }), + ); + const upCounter = makeUpCounter(3); + t.is(upCounter.incr(5), 8); + t.is(upCounter.incr(1), 9); + t.throws(() => upCounter.incr(-3), { + message: 'In "incr" method of (UpCounter): arg 0?: -3 - Must be >= 0', + }); + // @ts-expect-error bad arg + t.throws(() => upCounter.incr('foo'), { + message: + 'In "incr" method of (UpCounter): arg 0?: string "foo" - Must be a number', + }); + t.deepEqual(upCounter[GET_INTERFACE_GUARD](), UpCounterI); + t.deepEqual(getInterfaceMethodKeys(UpCounterI), ['incr']); + + const symbolic = Symbol.for('symbolic'); + const FooI = M.interface('Foo', { + m: M.call().returns(), + [symbolic]: M.call(M.boolean()).returns(), + }); + t.deepEqual(getInterfaceMethodKeys(FooI), ['m', Symbol.for('symbolic')]); + const makeFoo = defineExoClass( + 'Foo', + FooI, + () => ({}), + denumerate({ + m() {}, + [symbolic]() {}, + }), + ); + const foo = makeFoo(); + t.deepEqual(foo[GET_INTERFACE_GUARD](), FooI); + t.throws(() => foo[symbolic]('invalid arg'), { + message: + 'In "[Symbol(symbolic)]" method of (Foo): arg 0: string "invalid arg" - Must be a boolean', + }); +}); diff --git a/packages/marshal/test/test-marshal-testing.js b/packages/marshal/test/test-marshal-testing.js index 241887b5d8..7721e61efb 100644 --- a/packages/marshal/test/test-marshal-testing.js +++ b/packages/marshal/test/test-marshal-testing.js @@ -1,14 +1,18 @@ import { test } from './prepare-test-env-ava.js'; // eslint-disable-next-line import/order -import { Far, passStyleOf, Remotable } from '@endo/pass-style'; +import { passStyleOf, Remotable } from '@endo/pass-style'; import { makeMarshal } from '../src/marshal.js'; const { create } = Object; -const alice = Far('alice'); -const bob1 = Far('bob'); -const bob2 = Far('bob'); +// Use the lower level `Remotable` rather than `Far` to make an empty +// far object, i.e., one without even the miranda meta methods like +// `GET_METHOD_NAMES`. Such an empty far object should be `t.deepEqual` +// to its remote presences. +const alice = Remotable('Alleged: alice'); +const bob1 = Remotable('Alleged: bob'); +const bob2 = Remotable('Alleged: bob'); const convertValToSlot = val => passStyleOf(val) === 'remotable' ? 'far' : val; diff --git a/packages/pass-style/index.js b/packages/pass-style/index.js index ef54b29c9f..da985131a2 100644 --- a/packages/pass-style/index.js +++ b/packages/pass-style/index.js @@ -24,7 +24,12 @@ export { export { passStyleOf, assertPassable } from './src/passStyleOf.js'; export { makeTagged } from './src/makeTagged.js'; -export { Remotable, Far, ToFarFunction } from './src/make-far.js'; +export { + Remotable, + Far, + ToFarFunction, + GET_METHOD_NAMES, +} from './src/make-far.js'; export { assertRecord, diff --git a/packages/pass-style/package.json b/packages/pass-style/package.json index 60a39fe909..45bf1234db 100644 --- a/packages/pass-style/package.json +++ b/packages/pass-style/package.json @@ -33,6 +33,7 @@ "test": "ava" }, "dependencies": { + "@endo/eventual-send": "^0.17.6", "@endo/promise-kit": "^0.2.60", "@fast-check/ava": "^1.1.5" }, diff --git a/packages/pass-style/src/make-far.js b/packages/pass-style/src/make-far.js index d77ad60c66..0b7b0916b0 100644 --- a/packages/pass-style/src/make-far.js +++ b/packages/pass-style/src/make-far.js @@ -1,5 +1,6 @@ /// +import { getMethodNames } from '@endo/eventual-send/utils.js'; import { assertChecker, PASS_STYLE } from './passStyle-helpers.js'; import { assertIface, getInterfaceOf, RemotableHelper } from './remotable.js'; @@ -128,9 +129,41 @@ export const Remotable = ( }; harden(Remotable); +/** + * The name of the automatically added default meta-method for obtaining a + * list of all methods of an object declared with `Far`, or an object that + * inherits from an object declared with `Far`. + * + * Modeled on `GET_INTERFACE_GUARD` from `@endo/exo`. + * + * TODO Name to be bikeshed. Perhaps even whether it is a + * string or symbol to be bikeshed. See + * https://github.com/endojs/endo/pull/1809#discussion_r1388052454 + * + * HAZARD: Beware that an exo's interface can change across an upgrade, + * so remotes that cache it can become stale. + */ +export const GET_METHOD_NAMES = '__getMethodNames__'; + +/** + * Note that `getMethodNamesMethod` is a thisful method! It must be so that + * it works as expected with far-object inheritance. + * + * @returns {(string|symbol)[]} + */ +const getMethodNamesMethod = harden({ + [GET_METHOD_NAMES]() { + return getMethodNames(this); + }, +})[GET_METHOD_NAMES]; + /** * A concise convenience for the most common `Remotable` use. * + * For far objects (as opposed to far functions), also adds a miranda + * `GET_METHOD_NAMES` method that returns an array of all the method names, + * if there is not yet any method named `GET_METHOD_NAMES`. (Hence "miranda") + * * @template {{}} T * @param {string} farName This name will be prepended with `Alleged: ` * for now to form the `Remotable` `iface` argument. @@ -138,6 +171,16 @@ harden(Remotable); */ export const Far = (farName, remotable = undefined) => { const r = remotable === undefined ? /** @type {T} */ ({}) : remotable; + if (typeof r === 'object' && !(GET_METHOD_NAMES in r)) { + // This test excludes far functions, since we currently consider them + // to only have a call-behavior, with no callable methods. + // Beware: Mutates the input argument! But `Remotable` + // * requires the object to be mutable + // * does further mutations, + // * hardens the mutated object before returning it. + // so this mutation is not unprecedented. But it is surprising! + r[GET_METHOD_NAMES] = getMethodNamesMethod; + } return Remotable(`Alleged: ${farName}`, undefined, r); }; harden(Far); diff --git a/packages/pass-style/test/test-far-class-instances.js b/packages/pass-style/test/test-far-class-instances.js new file mode 100644 index 0000000000..ec5af211a5 --- /dev/null +++ b/packages/pass-style/test/test-far-class-instances.js @@ -0,0 +1,112 @@ +/* eslint-disable class-methods-use-this */ +/* eslint-disable max-classes-per-file */ +import { test } from './prepare-test-env-ava.js'; + +// eslint-disable-next-line import/order +import { getMethodNames } from '@endo/eventual-send/utils.js'; +import { passStyleOf } from '../src/passStyleOf.js'; +import { Far, GET_METHOD_NAMES } from '../src/make-far.js'; + +/** + * Classes whose instances should be Far objects may find it convenient to + * inherit from this base class. Note that the constructor of this base class + * freezes the instance in an empty state, so all is interesting attributes + * can only depend on its identity and what it inherits from. + * This includes private fields, as those are keyed only on + * this object's identity. However, we discourage (but cannot prevent) such + * use of private fields, as they cannot easily be refactored into Exo state. + */ +const FarBaseClass = class FarBaseClass { + constructor() { + harden(this); + } +}; + +Far('FarBaseClass', FarBaseClass.prototype); +harden(FarBaseClass); + +class FarSubclass1 extends FarBaseClass { + double(x) { + return x + x; + } +} + +class FarSubclass2 extends FarSubclass1 { + #y = 0; + + constructor(y) { + super(); + this.#y = y; + } + + doubleAdd(x) { + return this.double(x) + this.#y; + } +} + +const assertMethodNames = (t, obj, names) => { + t.deepEqual(getMethodNames(obj), names); + t.deepEqual(obj[GET_METHOD_NAMES](), names); +}; + +test('far class instances', t => { + const fb = new FarBaseClass(); + t.is(passStyleOf(fb), 'remotable'); + assertMethodNames(t, fb, [GET_METHOD_NAMES, 'constructor']); + + t.assert(new fb.constructor() instanceof FarBaseClass); + t.throws(() => fb.constructor(), { + // TODO message depends on JS engine, and so is a fragile golden test + message: "Class constructor FarBaseClass cannot be invoked without 'new'", + }); + + const fs1 = new FarSubclass1(); + t.is(passStyleOf(fs1), 'remotable'); + t.is(fs1.double(4), 8); + t.assert(new fs1.constructor() instanceof FarSubclass1); + assertMethodNames(t, fs1, [GET_METHOD_NAMES, 'constructor', 'double']); + + const fs2 = new FarSubclass2(3); + t.is(passStyleOf(fs2), 'remotable'); + t.is(fs2.double(4), 8); + t.is(fs2.doubleAdd(4), 11); + assertMethodNames(t, fs2, [ + GET_METHOD_NAMES, + 'constructor', + 'double', + 'doubleAdd', + ]); + + const yField = new WeakMap(); + class FarSubclass3 extends FarSubclass1 { + constructor(y) { + super(); + yField.set(this, y); + } + + doubleAdd(x) { + return this.double(x) + yField.get(this); + } + } + + const fs3 = new FarSubclass3(3); + t.is(passStyleOf(fs3), 'remotable'); + t.is(fs3.double(4), 8); + t.is(fs3.doubleAdd(4), 11); + assertMethodNames(t, fs3, [ + GET_METHOD_NAMES, + 'constructor', + 'double', + 'doubleAdd', + ]); +}); + +test('far class instance hardened empty', t => { + class FarClass4 extends FarBaseClass { + z = 0; + } + t.throws(() => new FarClass4(), { + // TODO message depends on JS engine, and so is a fragile golden test + message: 'Cannot define property z, object is not extensible', + }); +}); diff --git a/packages/pass-style/test/test-far-wobbly-point.js b/packages/pass-style/test/test-far-wobbly-point.js new file mode 100644 index 0000000000..e46060d91c --- /dev/null +++ b/packages/pass-style/test/test-far-wobbly-point.js @@ -0,0 +1,151 @@ +/** + * 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 + */ + +/* eslint-disable class-methods-use-this */ +/* eslint-disable max-classes-per-file */ +import { test } from './prepare-test-env-ava.js'; + +// eslint-disable-next-line import/order +import { getMethodNames } from '@endo/eventual-send/utils.js'; +import { passStyleOf } from '../src/passStyleOf.js'; +import { Far, GET_METHOD_NAMES } from '../src/make-far.js'; + +const { apply } = Reflect; + +/** + * Classes whose instances should be Far objects may find it convenient to + * inherit from this base class. Note that the constructor of this base class + * freezes the instance in an empty state, so all is interesting attributes + * can only depend on its identity and what it inherits from. + * This includes private fields, as those are keyed only on + * this object's identity. However, we discourage (but cannot prevent) such + * use of private fields, as they cannot easily be refactored into Exo state. + */ +class FarBaseClass { + constructor() { + harden(this); + } +} +Far('FarBaseClass', FarBaseClass.prototype); +harden(FarBaseClass); + +class FarPoint extends FarBaseClass { + #x; + + #y; + + constructor(x, y) { + super(); + this.#x = x; + this.#y = y; + } + + toString() { + return `<${this.getX()},${this.getY()}>`; + } + + getX() { + return this.#x; + } + + getY() { + return this.#y; + } + + setY(newY) { + this.#y = newY; + } +} +harden(FarPoint); + +const assertMethodNames = (t, obj, names) => { + t.deepEqual(getMethodNames(obj), names); + t.deepEqual(obj[GET_METHOD_NAMES](), names); +}; + +test('FarPoint instances', t => { + const pt = new FarPoint(3, 5); + t.is(passStyleOf(pt), 'remotable'); + t.assert(pt instanceof FarPoint); + assertMethodNames(t, pt, [ + GET_METHOD_NAMES, + 'constructor', + '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 = new FarPoint(1, 2); + t.is(apply(pt.getX, otherPt, []), 1); +}); + +class FarWobblyPoint extends FarPoint { + #getWobble; + + constructor(x, y, getWobble) { + super(x, y); + this.#getWobble = getWobble; + } + + getX() { + return super.getX() + this.#getWobble(); + } +} +harden(FarWobblyPoint); + +test('FarWobblyPoint inheritance', t => { + let wobble = 0; + const getWobble = () => (wobble += 1); + const wpt = new FarWobblyPoint(3, 5, getWobble); + t.assert(wpt instanceof FarWobblyPoint); + t.assert(wpt instanceof FarPoint); + t.is(passStyleOf(wpt), 'remotable'); + assertMethodNames(t, wpt, [ + GET_METHOD_NAMES, + 'constructor', + '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 = new FarPoint(1, 2); + t.false(otherPt instanceof FarWobblyPoint); + t.throws(() => apply(wpt.getX, otherPt, []), { + // TODO great error message, but is a golden specific to v8 + message: + 'Cannot read private member #getWobble from an object whose class did not declare it', + }); + t.is(apply(wpt.getY, otherPt, []), 2); + + const otherWpt = new FarWobblyPoint(3, 5, () => 1); + t.is(`${otherWpt}`, '<4,5>'); + t.is(apply(wpt.getX, otherWpt, []), 4); + + // This test, though correct, demonstrates a sucurity weakness of + // this approach to JS class inheritance at this + // `@endo/pass-style` / `Far` level of abstraction. The weakness + // is that the overridden method from a superclass can nevertheless + // be directly applied to an instance of the subclass. The + // subclass override did not suppress the use of the overridden + // method as, effectively, part of the subclass' instance's public + // API. + // + // See the corresponding example at + // `test-exo-wobbly-point.js` to see the absence of this vulnerability + // at the Exo level of abstraction. + t.is(apply(otherPt.getX, otherWpt, []), 3); +}); diff --git a/packages/pass-style/test/test-passStyleOf.js b/packages/pass-style/test/test-passStyleOf.js index 9acf2cf6a7..ea6610e5af 100644 --- a/packages/pass-style/test/test-passStyleOf.js +++ b/packages/pass-style/test/test-passStyleOf.js @@ -266,7 +266,8 @@ test('passStyleOf testing remotables', t => { class NonFarBaseClass9 {} class Subclass9 extends NonFarBaseClass9 {} t.throws(() => Far('FarType9', Subclass9.prototype), { - message: 'For now, remotables cannot inherit from anything unusual, in {}', + message: + 'For now, remotables cannot inherit from anything unusual, in {"__getMethodNames__":"[Function __getMethodNames__]"}', }); const unusualTagRecordProtoMessage = diff --git a/packages/patterns/src/patterns/patternMatchers.js b/packages/patterns/src/patterns/patternMatchers.js index 71a5beb8a3..48978f072a 100644 --- a/packages/patterns/src/patterns/patternMatchers.js +++ b/packages/patterns/src/patterns/patternMatchers.js @@ -1664,7 +1664,8 @@ const makePatternKit = () => { ), split: (base, rest = undefined) => { if (passStyleOf(harden(base)) === 'copyArray') { - // @ts-expect-error We know it should be an array + // TODO at-ts-expect-error works locally but not from @endo/exo + // @ts-ignore We know it should be an array return M.splitArray(base, rest && [], rest); } else { return M.splitRecord(base, rest && {}, rest); @@ -1672,7 +1673,8 @@ const makePatternKit = () => { }, partial: (base, rest = undefined) => { if (passStyleOf(harden(base)) === 'copyArray') { - // @ts-expect-error We know it should be an array + // TODO at-ts-expect-error works locally but not from @endo/exo + // @ts-ignore We know it should be an array return M.splitArray([], base, rest); } else { return M.splitRecord({}, base, rest); @@ -1945,7 +1947,8 @@ export const getInterfaceMethodKeys = interfaceGuard => { const { methodGuards, symbolMethodGuards = emptyCopyMap } = getInterfaceGuardPayload(interfaceGuard); /** @type {(string | symbol)[]} */ - // @ts-expect-error inference is too weak to see this is ok + // TODO at-ts-expect-error works locally but not from @endo/exo + // @ts-ignore inference is too weak to see this is ok return harden([ ...Reflect.ownKeys(methodGuards), ...getCopyMapKeys(symbolMethodGuards), diff --git a/packages/ses/src/permits.js b/packages/ses/src/permits.js index 27f989538c..411e73d1be 100644 --- a/packages/ses/src/permits.js +++ b/packages/ses/src/permits.js @@ -1414,6 +1414,7 @@ export const permitted = { race: fn, reject: fn, resolve: fn, + // https://github.com/tc39/proposal-promise-with-resolvers withResolvers: fn, '@@species': getter, },