fixup! review suggestions

Author: erights

packages/eventual-send/src/E.js

@@ -167,20 +167,42 @@ const makeEGetProxyHandler = (x, HandledPromise) =>
     get: (_target, prop) => HandledPromise.get(x, prop),
   });
 
+/**
+ * `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.
+ *
+ * @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.
+ *
+ * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+ */
+const objTarget = freeze(create(null));
+
 /**
  * @param {HandledPromiseConstructor} HandledPromise
  */
 const makeE = HandledPromise => {
-  /**
-   * `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 funcTarget = freeze(() => {});
-  /**
-   * `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 objTarget = freeze(create(null));
   return harden(
     assign(
       /**

packages/pass-style/test/passStyleOf.test.js

@@ -110,13 +110,17 @@ test('some passStyleOf rejections', t => {
 });
 
 /**
- * For testing purposes, makes a TagRecord-like object with
+ * For testing purposes, makes a *non-frozen* TagRecord-like object with
  * non-enumerable PASS_STYLE and Symbol.toStringTag properties.
  * A valid Remotable must inherit from a valid TagRecord.
+ * - Before stabilize/suppressTrapping, a valid TagRecord must be frozen.
+ * - After stabilize/suppressTrapping, a valid TagRecord must also be
+ *   stable/non-trapping, for example, because it was hardened.
  *
  * @param {string} [tag]
  * @param {object|null} [proto]
  * @returns {{ [PASS_STYLE]: 'remotable', [Symbol.toStringTag]: string }}
+ * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
  */
 const makeTagishRecord = (tag = 'Remotable', proto = undefined) => {
   return Object.create(proto === undefined ? Object.prototype : proto, {
@@ -193,16 +197,21 @@ test('passStyleOf testing remotables', t => {
 
   const tagRecord1 = harden(makeTagishRecord('Alleged: manually constructed'));
   /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const farObj1 = harden({ __proto__: tagRecord1 });
   t.is(passStyleOf(farObj1), 'remotable');
 
   const tagRecord2 = makeTagishRecord('Alleged: tagRecord not hardened');
   /**
+   * Do not freeze `tagRecord2` in order to test that an object with
+   * a non-frozen __proto__ is not passable.
+   *
    * TODO In order to run this test before we have explicit support for a
    * non-trapping integrity trait, 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
+   * because that would also harden `__proto__`. So we will need to
    * explicitly make this non-trapping, which we cannot yet express.
    * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
    *
@@ -220,24 +229,34 @@ test('passStyleOf testing remotables', t => {
 
   const tagRecord3 = harden(makeTagishRecord('Alleged: both manually frozen'));
   /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   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 */
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const farObj4 = harden({ __proto__: tagRecord4 });
   t.is(passStyleOf(farObj4), 'remotable');
 
   const tagRecord5 = harden(makeTagishRecord('Not alleging'));
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   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'));
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const farObjProto6 = harden({ __proto__: tagRecord6 });
   /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const farObj6 = harden({ __proto__: farObjProto6 });
   t.is(passStyleOf(farObj6), 'remotable', 'tagRecord grandproto is accepted');
 
@@ -292,6 +311,8 @@ test('passStyleOf testing remotables', t => {
   const tagRecordA1 = harden(
     makeTagishRecord('Alleged: null-proto tagRecord proto', null),
   );
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const farObjA1 = harden({ __proto__: tagRecordA1 });
   t.throws(
     () => passStyleOf(farObjA1),
@@ -302,7 +323,11 @@ test('passStyleOf testing remotables', t => {
   const tagRecordA2 = harden(
     makeTagishRecord('Alleged: null-proto tagRecord grandproto', null),
   );
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const farObjProtoA2 = harden({ __proto__: tagRecordA2 });
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const farObjA2 = harden({ __proto__: farObjProtoA2 });
   t.throws(
     () => passStyleOf(farObjA2),
@@ -317,7 +342,11 @@ test('passStyleOf testing remotables', t => {
   const fauxTagRecordB = harden(
     makeTagishRecord('Alleged: manually constructed', harden({})),
   );
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const farObjProtoB = harden({ __proto__: fauxTagRecordB });
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const farObjB = harden({ __proto__: farObjProtoB });
   t.throws(() => passStyleOf(farObjB), {
     message:
@@ -329,6 +358,8 @@ test('passStyleOf testing remotables', t => {
   );
   Object.defineProperty(farObjProtoWithExtra, 'extra', { value: () => {} });
   harden(farObjProtoWithExtra);
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const badFarObjExtraProtoProp = harden({ __proto__: farObjProtoWithExtra });
   t.throws(() => passStyleOf(badFarObjExtraProtoProp), {
     message: 'Unexpected properties on Remotable Proto ["extra"]',
@@ -378,11 +409,17 @@ test('remotables - safety from the gibson042 attack', t => {
   );
 
   /**
+   * Do not freeze `mercurialProto` in order to test that an object with
+   * a non-frozen __proto__ is not passable. Once we have support for
+   * non-trapping, we should generalize this test (or add a new one) where
+   * `mercurialProto` is frozen but still trapping. This test would then
+   * test that a valid TagRecord must be non-trapping.
+   *
    * TODO In order to run this test before we have explicit support for a
    * non-trapping integrity trait, 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
+   * because that would also harden `__proto__`. So we will need to
    * explicitly make this non-trapping, which we cannot yet express.
    * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
    */
@@ -448,12 +485,16 @@ test('Allow toStringTag overrides', t => {
   t.is(`${q(alice)}`, '"[DebugName: Allison]"');
 
   /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const carol = harden({ __proto__: alice });
   t.is(passStyleOf(carol), 'remotable');
   t.is(`${carol}`, '[object DebugName: Allison]');
   t.is(`${q(carol)}`, '"[DebugName: Allison]"');
 
   /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
+  // TODO Once we have support for explicit non-trapping, this should change
+  // from `harden` to `suppressTrapping` or `stabilize`.
   const bob = harden({
     __proto__: carol,
     [Symbol.toStringTag]: 'DebugName: Robert',