Skip to content

Commit

Permalink
refactor: prepare for non-trapping integrity trait (#2679)
Browse files Browse the repository at this point in the history
Closes: #XXXX
Refs: Agoric/agoric-sdk#10795

## Description

Prepare for anticipated introduction and use of the non-trapping
integrity trait as explained at
/~https://github.com/endojs/endo/blob/b12eb434b6672f0ceae41be55aac7f24c4562b7b/packages/ses/docs/preparing-for-stabilize.md

These preparations must work now, before these traits are introduced,
and should continue to work after these traits are introduced and used.

### Security Considerations

Some things that had been deeply frozen automatically by `harden` are
now manually frozen by explicit calls to `freeze`. We need to review
these carefully to ensure that nothing has inadvertently be left
unfrozen as a result of the changes in this PR.

Some proxies will become unhardenable, but they will still be hardenable
as of now, so mistaken hardenings will not be detected.

### Scaling Considerations

For this PR by itself, none. Using the shim-based implementation of the
non-trapping trait will have scaling consequences:
#2675

### Documentation Considerations


/~https://github.com/endojs/endo/blob/b12eb434b6672f0ceae41be55aac7f24c4562b7b/packages/ses/docs/preparing-for-stabilize.md
will need to be reflected in developer docs.

### Testing Considerations

Since this PR by itself should be a pure refactor with no observable
changes, there is nothing to test at this stage. The testing burden will
come with #2675 to see how adequate
these preparations were.

### Compatibility Considerations

The point. This changes to coding patterns that should be compat both
with the current status quo and with
#2675

### Upgrade Considerations

As a pure refactor, none.
  • Loading branch information
erights authored Jan 19, 2025
1 parent 630592d commit 07ff084
Show file tree
Hide file tree
Showing 11 changed files with 191 additions and 71 deletions.
22 changes: 20 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 @@ -56,14 +58,30 @@ const TrapProxyHandler = (x, trapImpl) => {
});
};

/**
* `freeze` but not `harden` the proxy target so it remains trapping.
* Thus, it should not be shared outside this module.
*
* @see /~https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
*/
const funcTarget = freeze(() => {});

/**
* `freeze` but not `harden` the proxy target so it remains trapping.
* Thus, it should not be shared outside this module.
*
* @see /~https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
*/
const objTarget = freeze({ __proto__: null });

/**
* @param {import('./types.js').TrapImpl} trapImpl
* @returns {import('./ts-types.js').Trap}
*/
export const makeTrap = trapImpl => {
const Trap = x => {
const handler = TrapProxyHandler(x, trapImpl);
return harden(new Proxy(() => {}, handler));
return new Proxy(funcTarget, handler);
};

const makeTrapGetterProxy = x => {
Expand All @@ -77,7 +95,7 @@ export const makeTrap = trapImpl => {
return trapImpl.get(x, prop);
},
});
return new Proxy(Object.create(null), handler);
return new Proxy(objTarget, handler);
};
Trap.get = makeTrapGetterProxy;

Expand Down
32 changes: 22 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, freeze } = Object;

/**
* @import { HandledPromiseConstructor } from './types.js';
Expand Down Expand Up @@ -167,6 +167,23 @@ const makeEGetProxyHandler = (x, HandledPromise) =>
get: (_target, prop) => HandledPromise.get(x, prop),
});

/**
* `freeze` but not `harden` the proxy target so it remains trapping.
* Thus, it should not be shared outside this module.
*
* @see /~https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
*/
const funcTarget = freeze(() => {});

/**
/**
* `freeze` but not `harden` the proxy target so it remains trapping.
* Thus, it should not be shared outside this module.
*
* @see /~https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
*/
const objTarget = freeze({ __proto__: null });

/**
* @param {HandledPromiseConstructor} HandledPromise
*/
Expand All @@ -183,7 +200,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 +213,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 +238,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/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
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
16 changes: 14 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,14 @@ const badArrayHandler = harden({
},
});

const badArray = harden(new Proxy(harden([]), badArrayHandler));
/**
* `freeze` but not `harden` the proxy target so it remains trapping.
* Thus, it should not be shared outside this module.
*
* @see /~https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
*/
const arrayTarget = freeze(/** @type {any[]} */ ([]));
const badArray = new Proxy(arrayTarget, badArrayHandler);

const { serialize, unserialize } = makeMarshal(
doNotConvertValToSlot,
Expand All @@ -48,7 +57,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/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
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
Loading

0 comments on commit 07ff084

Please sign in to comment.