Skip to content

Commit

Permalink
refactor: prepare for non-trapping integrity level
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Jan 2, 2025
1 parent b23724f commit 526e404
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 60 deletions.
16 changes: 14 additions & 2 deletions packages/captp/src/trap.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Lifted mostly from `@endo/eventual-send/src/E.js`.

const { freeze } = Object;

/**
* Default implementation of Trap for near objects.
*
Expand Down Expand Up @@ -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) {
Expand All @@ -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;

Expand Down
25 changes: 15 additions & 10 deletions packages/eventual-send/src/E.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
/**
Expand All @@ -183,7 +193,7 @@ const makeE = HandledPromise => {
* @returns {ECallableOrMethods<RemoteFunctions<T>>} 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.
Expand All @@ -196,11 +206,8 @@ const makeE = HandledPromise => {
* @returns {EGetters<LocalRecord<T>>} 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
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions packages/eventual-send/src/handled-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/far/test/marshal-far-function.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" /,
});
});
Expand Down
14 changes: 12 additions & 2 deletions packages/marshal/src/marshal-stringify.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { makeMarshal } from './marshal.js';

/** @import {Passable} from '@endo/pass-style' */

const { freeze } = Object;

/** @type {import('./types.js').ConvertValToSlot<any>} */
const doNotConvertValToSlot = val =>
Fail`Marshal's stringify rejects presences and promises ${val}`;
Expand All @@ -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,
Expand All @@ -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,
}),
Expand Down
2 changes: 1 addition & 1 deletion packages/marshal/test/marshal-far-function.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" /,
});
});
Expand Down
65 changes: 32 additions & 33 deletions packages/pass-style/test/passStyleOf.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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]"',
Expand Down Expand Up @@ -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();

Expand Down
6 changes: 0 additions & 6 deletions packages/ses/src/commons.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 8 additions & 2 deletions packages/ses/src/sloppy-globals-scope-terminator.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,20 @@ import {
create,
freeze,
getOwnPropertyDescriptors,
immutableObject,
reflectSet,
} from './commons.js';
import {
strictScopeTerminatorHandler,
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
Expand Down Expand Up @@ -45,7 +51,7 @@ export const createSloppyGlobalsScopeTerminator = globalObject => {
);

const sloppyGlobalsScopeTerminator = new Proxy(
immutableObject,
onlyFrozenObject,
sloppyGlobalsScopeTerminatorHandler,
);

Expand Down
12 changes: 9 additions & 3 deletions packages/ses/src/strict-scope-terminator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))}`;
Expand Down Expand Up @@ -88,6 +94,6 @@ export const strictScopeTerminatorHandler = freeze(
);

export const strictScopeTerminator = new Proxy(
immutableObject,
onlyFrozenObject,
strictScopeTerminatorHandler,
);

0 comments on commit 526e404

Please sign in to comment.