From ce2d645f616f87d99808743676702d58b3fb392f Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Thu, 2 Jan 2025 10:12:44 -0800 Subject: [PATCH] refactor: prepare for non-trapping integrity level --- packages/captp/src/trap.js | 16 ++++- packages/eventual-send/src/E.js | 25 ++++--- packages/eventual-send/src/handled-promise.js | 3 + .../far/test/marshal-far-function.test.js | 2 +- packages/marshal/src/marshal-stringify.js | 14 +++- .../marshal/test/marshal-far-function.test.js | 2 +- packages/pass-style/test/passStyleOf.test.js | 65 +++++++++---------- packages/ses/src/commons.js | 6 -- .../src/sloppy-globals-scope-terminator.js | 10 ++- packages/ses/src/strict-scope-terminator.js | 12 +++- 10 files changed, 95 insertions(+), 60 deletions(-) diff --git a/packages/captp/src/trap.js b/packages/captp/src/trap.js index 5b7c81afda..b49cc2f2fa 100644 --- a/packages/captp/src/trap.js +++ b/packages/captp/src/trap.js @@ -1,5 +1,7 @@ // Lifted mostly from `@endo/eventual-send/src/E.js`. +const { freeze } = Object; + /** * Default implementation of Trap for near objects. * @@ -62,11 +64,21 @@ const TrapProxyHandler = (x, trapImpl) => { */ export const makeTrap = trapImpl => { const Trap = x => { + /** + * `freeze` but not `harden` the proxy target so it remains trapping. + * @see https://github.com/tc39/proposal-stabilize + */ + const target = freeze(() => {}); const handler = TrapProxyHandler(x, trapImpl); - return harden(new Proxy(() => {}, handler)); + return new Proxy(target, handler); }; const makeTrapGetterProxy = x => { + /** + * `freeze` but not `harden` the proxy target so it remains trapping. + * @see https://github.com/tc39/proposal-stabilize + */ + const target = freeze(Object.create(null)); const handler = harden({ ...baseFreezableProxyHandler, has(_target, _prop) { @@ -77,7 +89,7 @@ export const makeTrap = trapImpl => { return trapImpl.get(x, prop); }, }); - return new Proxy(Object.create(null), handler); + return new Proxy(target, handler); }; Trap.get = makeTrapGetterProxy; diff --git a/packages/eventual-send/src/E.js b/packages/eventual-send/src/E.js index a114e3ccb2..484c12a406 100644 --- a/packages/eventual-send/src/E.js +++ b/packages/eventual-send/src/E.js @@ -2,7 +2,7 @@ import { trackTurns } from './track-turns.js'; import { makeMessageBreakpointTester } from './message-breakpoints.js'; const { details: X, quote: q, Fail, error: makeError } = assert; -const { assign, create } = Object; +const { assign, create, freeze } = Object; /** * @import { HandledPromiseConstructor } from './types.js'; @@ -171,6 +171,16 @@ const makeEGetProxyHandler = (x, HandledPromise) => * @param {HandledPromiseConstructor} HandledPromise */ const makeE = HandledPromise => { + /** + * `freeze` but not `harden` the proxy target so it remains trapping. + * @see https://github.com/tc39/proposal-stabilize + */ + const funcTarget = freeze(() => {}); + /** + * `freeze` but not `harden` the proxy target so it remains trapping. + * @see https://github.com/tc39/proposal-stabilize + */ + const objTarget = freeze(create(null)); return harden( assign( /** @@ -183,7 +193,7 @@ const makeE = HandledPromise => { * @returns {ECallableOrMethods>} method/function call proxy */ // @ts-expect-error XXX typedef - x => harden(new Proxy(() => {}, makeEProxyHandler(x, HandledPromise))), + x => new Proxy(funcTarget, makeEProxyHandler(x, HandledPromise)), { /** * E.get(x) returns a proxy on which you can get arbitrary properties. @@ -196,11 +206,8 @@ const makeE = HandledPromise => { * @returns {EGetters>} property get proxy * @readonly */ - get: x => - // @ts-expect-error XXX typedef - harden( - new Proxy(create(null), makeEGetProxyHandler(x, HandledPromise)), - ), + // @ts-expect-error XXX typedef + get: x => new Proxy(objTarget, makeEGetProxyHandler(x, HandledPromise)), /** * E.resolve(x) converts x to a handled promise. It is @@ -224,9 +231,7 @@ const makeE = HandledPromise => { */ sendOnly: x => // @ts-expect-error XXX typedef - harden( - new Proxy(() => {}, makeESendOnlyProxyHandler(x, HandledPromise)), - ), + new Proxy(funcTarget, makeESendOnlyProxyHandler(x, HandledPromise)), /** * E.when(x, res, rej) is equivalent to diff --git a/packages/eventual-send/src/handled-promise.js b/packages/eventual-send/src/handled-promise.js index 17785b3529..ab4ec0bba2 100644 --- a/packages/eventual-send/src/handled-promise.js +++ b/packages/eventual-send/src/handled-promise.js @@ -309,6 +309,9 @@ export const makeHandledPromise = () => { if (proxyOpts) { const { handler: proxyHandler, + // The proxy target can be frozen but should not be hardened + // so it remains trapping. + // See https://github.com/tc39/proposal-stabilize target: proxyTarget, revokerCallback, } = proxyOpts; diff --git a/packages/far/test/marshal-far-function.test.js b/packages/far/test/marshal-far-function.test.js index c86c2b1fb6..28d544e6c6 100644 --- a/packages/far/test/marshal-far-function.test.js +++ b/packages/far/test/marshal-far-function.test.js @@ -58,7 +58,7 @@ test('Data can contain far functions', t => { const arrow = Far('arrow', a => a + 1); t.is(passStyleOf(harden({ x: 8, foo: arrow })), 'copyRecord'); const mightBeMethod = a => a + 1; - t.throws(() => passStyleOf(freeze({ x: 8, foo: mightBeMethod })), { + t.throws(() => passStyleOf(harden({ x: 8, foo: mightBeMethod })), { message: /Remotables with non-methods like "x" /, }); }); diff --git a/packages/marshal/src/marshal-stringify.js b/packages/marshal/src/marshal-stringify.js index e581cdefed..c592afd5a0 100644 --- a/packages/marshal/src/marshal-stringify.js +++ b/packages/marshal/src/marshal-stringify.js @@ -5,6 +5,8 @@ import { makeMarshal } from './marshal.js'; /** @import {Passable} from '@endo/pass-style' */ +const { freeze } = Object; + /** @type {import('./types.js').ConvertValToSlot} */ const doNotConvertValToSlot = val => Fail`Marshal's stringify rejects presences and promises ${val}`; @@ -23,7 +25,12 @@ const badArrayHandler = harden({ }, }); -const badArray = harden(new Proxy(harden([]), badArrayHandler)); +/** + * `freeze` but not `harden` the proxy target so it remains trapping. + * @see https://github.com/tc39/proposal-stabilize + */ +const arrayTarget = freeze(/** @type {any[]} */ ([])); +const badArray = new Proxy(arrayTarget, badArrayHandler); const { serialize, unserialize } = makeMarshal( doNotConvertValToSlot, @@ -48,7 +55,10 @@ harden(stringify); */ const parse = str => unserialize( - harden({ + // `freeze` but not `harden` since the `badArray` proxy and its target + // must remain trapping. + // See https://github.com/tc39/proposal-stabilize + freeze({ body: str, slots: badArray, }), diff --git a/packages/marshal/test/marshal-far-function.test.js b/packages/marshal/test/marshal-far-function.test.js index b546b11621..223fdd27e5 100644 --- a/packages/marshal/test/marshal-far-function.test.js +++ b/packages/marshal/test/marshal-far-function.test.js @@ -60,7 +60,7 @@ test('Data can contain far functions', t => { const arrow = Far('arrow', a => a + 1); t.is(passStyleOf(harden({ x: 8, foo: arrow })), 'copyRecord'); const mightBeMethod = a => a + 1; - t.throws(() => passStyleOf(freeze({ x: 8, foo: mightBeMethod })), { + t.throws(() => passStyleOf(harden({ x: 8, foo: mightBeMethod })), { message: /Remotables with non-methods like "x" /, }); }); diff --git a/packages/pass-style/test/passStyleOf.test.js b/packages/pass-style/test/passStyleOf.test.js index d09cd55260..f8b292d978 100644 --- a/packages/pass-style/test/passStyleOf.test.js +++ b/packages/pass-style/test/passStyleOf.test.js @@ -13,7 +13,7 @@ const harden = /** @type {import('ses').Harden & { isFake?: boolean }} */ ( global.harden ); -const { getPrototypeOf, defineProperty } = Object; +const { getPrototypeOf, defineProperty, freeze } = Object; const { ownKeys } = Reflect; test('passStyleOf basic success cases', t => { @@ -193,16 +193,22 @@ test('passStyleOf testing remotables', t => { const tagRecord1 = harden(makeTagishRecord('Alleged: manually constructed')); /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */ - const farObj1 = harden({ - __proto__: tagRecord1, - }); + const farObj1 = harden({ __proto__: tagRecord1 }); t.is(passStyleOf(farObj1), 'remotable'); const tagRecord2 = makeTagishRecord('Alleged: tagRecord not hardened'); - /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */ - const farObj2 = Object.freeze({ - __proto__: tagRecord2, - }); + /** + * TODO In order to run this test before we have explicit support for a + * non-trapping integrity level, 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 hardden `__proto__`. So we will need to + * explicitly make this non-trapping, which we cannot yet express. + * @see https://github.com/tc39/proposal-stabilize + * + * @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 + */ + const farObj2 = freeze({ __proto__: tagRecord2 }); if (harden.isFake) { t.is(passStyleOf(farObj2), 'remotable'); } else { @@ -212,39 +218,27 @@ test('passStyleOf testing remotables', t => { }); } - const tagRecord3 = Object.freeze( - makeTagishRecord('Alleged: both manually frozen'), - ); + const tagRecord3 = harden(makeTagishRecord('Alleged: both manually frozen')); /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */ - const farObj3 = Object.freeze({ - __proto__: tagRecord3, - }); + const farObj3 = harden({ __proto__: tagRecord3 }); t.is(passStyleOf(farObj3), 'remotable'); const tagRecord4 = harden(makeTagishRecord('Remotable')); /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */ - const farObj4 = harden({ - __proto__: tagRecord4, - }); + const farObj4 = harden({ __proto__: tagRecord4 }); t.is(passStyleOf(farObj4), 'remotable'); const tagRecord5 = harden(makeTagishRecord('Not alleging')); - const farObj5 = harden({ - __proto__: tagRecord5, - }); + const farObj5 = harden({ __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 = harden({ - __proto__: tagRecord6, - }); + const farObjProto6 = harden({ __proto__: tagRecord6 }); /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */ - const farObj6 = harden({ - __proto__: farObjProto6, - }); + const farObj6 = harden({ __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 @@ -323,12 +317,8 @@ test('passStyleOf testing remotables', t => { const fauxTagRecordB = harden( makeTagishRecord('Alleged: manually constructed', harden({})), ); - const farObjProtoB = harden({ - __proto__: fauxTagRecordB, - }); - const farObjB = harden({ - __proto__: farObjProtoB, - }); + const farObjProtoB = harden({ __proto__: fauxTagRecordB }); + const farObjB = harden({ __proto__: farObjProtoB }); t.throws(() => passStyleOf(farObjB), { message: 'cannot serialize Remotables with non-methods like "Symbol(passStyle)" in "[Alleged: manually constructed]"', @@ -387,7 +377,16 @@ test('remotables - safety from the gibson042 attack', t => { }, ); - const makeInput = () => Object.freeze({ __proto__: mercurialProto }); + /** + * TODO In order to run this test before we have explicit support for a + * non-trapping integrity level, 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 hardden `__proto__`. So we will need to + * explicitly make this non-trapping, which we cannot yet express. + * @see https://github.com/tc39/proposal-stabilize + */ + const makeInput = () => freeze({ __proto__: mercurialProto }); const input1 = makeInput(); const input2 = makeInput(); diff --git a/packages/ses/src/commons.js b/packages/ses/src/commons.js index 5925c17eae..5211fa075d 100644 --- a/packages/ses/src/commons.js +++ b/packages/ses/src/commons.js @@ -269,12 +269,6 @@ export const finalizationRegistryUnregister = export const getConstructorOf = fn => reflectGet(getPrototypeOf(fn), 'constructor'); -/** - * immutableObject - * An immutable (frozen) empty object that is safe to share. - */ -export const immutableObject = freeze(create(null)); - /** * isObject tests whether a value is an object. * Today, this is equivalent to: diff --git a/packages/ses/src/sloppy-globals-scope-terminator.js b/packages/ses/src/sloppy-globals-scope-terminator.js index 01e3860856..590fdf86ed 100644 --- a/packages/ses/src/sloppy-globals-scope-terminator.js +++ b/packages/ses/src/sloppy-globals-scope-terminator.js @@ -3,7 +3,6 @@ import { create, freeze, getOwnPropertyDescriptors, - immutableObject, reflectSet, } from './commons.js'; import { @@ -11,6 +10,13 @@ import { alwaysThrowHandler, } from './strict-scope-terminator.js'; +/** + * `freeze` but not `harden` the proxy target so it remains trapping. + * Thus, it should not be shared outside this module. + * @see https://github.com/tc39/proposal-stabilize + */ +const onlyFrozenObject = freeze(create(null)); + /* * createSloppyGlobalsScopeTerminator() * strictScopeTerminatorHandler manages a scopeTerminator Proxy which serves as @@ -45,7 +51,7 @@ export const createSloppyGlobalsScopeTerminator = globalObject => { ); const sloppyGlobalsScopeTerminator = new Proxy( - immutableObject, + onlyFrozenObject, sloppyGlobalsScopeTerminatorHandler, ); diff --git a/packages/ses/src/strict-scope-terminator.js b/packages/ses/src/strict-scope-terminator.js index 7257afccd1..b8749a5298 100644 --- a/packages/ses/src/strict-scope-terminator.js +++ b/packages/ses/src/strict-scope-terminator.js @@ -7,12 +7,18 @@ import { freeze, getOwnPropertyDescriptors, globalThis, - immutableObject, } from './commons.js'; import { assert } from './error/assert.js'; const { Fail, quote: q } = assert; +/** + * `freeze` but not `harden` the proxy target so it remains trapping. + * Thus, it should not be shared outside this module. + * @see https://github.com/tc39/proposal-stabilize + */ +const onlyFrozenObject = freeze(create(null)); + /** * alwaysThrowHandler * This is an object that throws if any property is called. It's used as @@ -21,7 +27,7 @@ const { Fail, quote: q } = assert; * create one and share it between all Proxy handlers. */ export const alwaysThrowHandler = new Proxy( - immutableObject, + onlyFrozenObject, freeze({ get(_shadow, prop) { Fail`Please report unexpected scope handler trap: ${q(String(prop))}`; @@ -88,6 +94,6 @@ export const strictScopeTerminatorHandler = freeze( ); export const strictScopeTerminator = new Proxy( - immutableObject, + onlyFrozenObject, strictScopeTerminatorHandler, );