Skip to content

Commit

Permalink
fixup! review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Jan 18, 2025
1 parent 101a3b1 commit 28eeaa9
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 40 deletions.
30 changes: 18 additions & 12 deletions packages/captp/src/trap.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,33 @@ 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 => {
/**
* `freeze` but not `harden` the proxy target so it remains trapping.
* @see /~https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
*/
const target = freeze(() => {});
const handler = TrapProxyHandler(x, trapImpl);
return new Proxy(target, handler);
return new Proxy(funcTarget, handler);
};

const makeTrapGetterProxy = x => {
/**
* `freeze` but not `harden` the proxy target so it remains trapping.
* @see /~https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
*/
const target = freeze(Object.create(null));
const handler = harden({
...baseFreezableProxyHandler,
has(_target, _prop) {
Expand All @@ -89,7 +95,7 @@ export const makeTrap = trapImpl => {
return trapImpl.get(x, prop);
},
});
return new Proxy(target, handler);
return new Proxy(objTarget, handler);
};
Trap.get = makeTrapGetterProxy;

Expand Down
25 changes: 5 additions & 20 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, freeze } = Object;
const { assign, freeze } = Object;

/**
* @import { HandledPromiseConstructor } from './types.js';
Expand Down Expand Up @@ -169,35 +169,20 @@ const makeEGetProxyHandler = (x, HandledPromise) =>

/**
* `freeze` but not `harden` the proxy target so it remains trapping.
* This is safe to share between proxy instances because they are encapsulated
* within the proxy.
* - Before stabilize/suppressTrapping, this is safe
* because they are already frozen, and so they cannot be damaged by the
* proxies that encapsulate them.
* - After stabilize/suppressTrapping, this is safe because the only damage
* that could be done would be by stabilize/suppressTrapping. These proxies
* do not explicitly provide such a trap, and thus will use the default
* behavior which is to refuse to be made non-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.
* This is safe to share between proxy instances because they are encapsulated
* within the proxy.
* - Before stabilize/suppressTrapping, this is safe
* because they are already frozen, and so they cannot be damaged by the
* proxies that encapsulate them.
* - After stabilize/suppressTrapping, this is safe because the only damage
* that could be done would be by stabilize/suppressTrapping. These proxies
* do not explicitly provide such a trap, and thus will use the default
* behavior which is to refuse to be made non-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(create(null));
const objTarget = freeze({ __proto__: null });

/**
* @param {HandledPromiseConstructor} HandledPromise
Expand Down
2 changes: 2 additions & 0 deletions packages/marshal/src/marshal-stringify.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const badArrayHandler = harden({

/**
* `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[]} */ ([]));
Expand Down
12 changes: 9 additions & 3 deletions packages/ses/docs/preparing-for-stabilize.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,18 @@ Draft PR [feat(ses,pass-style): use non-trapping integrity trait for safety #267

## How proxy code should prepare

[#2673](/~https://github.com/endojs/endo/pull/2673) will *by default* produce proxies that refuse to be made non-trapping. An explicit handler trap (whose name is TBD) will need to be explicitly provided to make a proxy that allows itself to be made non-trapping. This is the right default, because proxies on frozen almost-empty objects can still have useful trap behavior for their `get`, `set`, `has`, and `apply` traps. Even on a frozen target
- The `get`, `set`, and `has` traps applied to a non-own property name are still general traps that can have useful trapping behavior.
- The `apply` trap can ignore the target's call behavior and just do its own thing.
[#2673](/~https://github.com/endojs/endo/pull/2673) will *by default* produce proxies that refuse to be made non-trapping. An explicit handler trap (perhaps named `stabilize` or `suppressTrapping`) will need to be explicitly provided to make a proxy that allows itself to be made non-trapping. This is the right default, because proxies on frozen almost-empty objects can still have useful trap behavior for their `get`, `set`, `has`, and `apply` traps. Even on a frozen target
- the `get`, `set`, and `has` traps applied to a non-own property name are still general traps that can have useful trapping behavior.
- the `apply` trap can ignore the target's call behavior and just do its own thing.

However, to prepare for these changes, we need to avoid hardening both such proxies and their targets. We need to avoid hardening their target because this will bypass the traps. We need to avoid hardening the proxy because such proxies will *by default* refuse to be made non-trapping, and thus refuse to be hardened.

Some proxies, such as that returned by `E(...)`, exist only to provide such trapping behavior. Their targets will typically be trivial useless empty frozen objects or almost empty frozen functions. Such frozen targets can be safely shared between multiple proxy instances because they are encapsulated within the proxy.
- Before `stabilize`/`suppressTrapping`, this is safe because they are already frozen, and so they cannot be damaged by the proxies that encapsulate them.
- After `stabilize`/`suppressTrapping`, this is safe because the only damage that could be done would be by `stabilize`/`suppressTrapping`. These proxies do not explicitly provide such a trap, and thus will use the default behavior which is to refuse to be made non-trapping.

Because such trivial targets, when safely encapsulated, can be safely shared, their definitions should typically appear at top level of their module.

## How passable objects should prepare

Although we think of `passStyleOf` as requiring its input to be hardened, `passStyleOf` instead checked that each relevant object is frozen. Manually freezing all objects reachable from a root object had been equivalent to hardening that root object. With these changes, even such manual transitive freezing will not make an object passable. To prepare for these changes, use `harden` explicitly instead.
5 changes: 3 additions & 2 deletions packages/ses/src/sloppy-globals-scope-terminator.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import {
/**
* `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 onlyFrozenObject = freeze(create(null));
const objTarget = freeze({ __proto__: null });

/*
* createSloppyGlobalsScopeTerminator()
Expand Down Expand Up @@ -51,7 +52,7 @@ export const createSloppyGlobalsScopeTerminator = globalObject => {
);

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

Expand Down
7 changes: 4 additions & 3 deletions packages/ses/src/strict-scope-terminator.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ 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/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
*/
const onlyFrozenObject = freeze(create(null));
const objTarget = freeze({ __proto__: null });

/**
* alwaysThrowHandler
Expand All @@ -27,7 +28,7 @@ const onlyFrozenObject = freeze(create(null));
* create one and share it between all Proxy handlers.
*/
export const alwaysThrowHandler = new Proxy(
onlyFrozenObject,
objTarget,
freeze({
get(_shadow, prop) {
Fail`Please report unexpected scope handler trap: ${q(String(prop))}`;
Expand Down Expand Up @@ -94,6 +95,6 @@ export const strictScopeTerminatorHandler = freeze(
);

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

0 comments on commit 28eeaa9

Please sign in to comment.