fix(ses): enablements grow monotonically (#3129)

Closes: #XXXX
Refs: #XXXX

## Description

Problem originally reported by @boneskull .

`overrideTaming: 'moderate'` includes `overrideTaming: 'min'`.

Previously `overrideTaming: 'min'` correctly enabled
`Iterator.prototype.constructor` to be overridden by assignment, but due
to an oversight, `overrideTaming: 'moderate'` did not. Now it does.

To make such mistakes less likely, this PR also adopts a style where all
records within larger enablements triple-dot the corresponding record
from a smaller enablement, if present.

### Security Considerations

Beyond fixing an observable bug, none. Everything is equally safe before
and after this PR.

### Scaling Considerations

none
### Documentation Considerations

none
### Testing Considerations

Tested.
Tests also modified to follow enablements being tested.
New test that the larger enablements are relaxations (as defined in
packages/ses/test/enable-property-overrides-relaxation.test.js) of
smaller enablements.

### Compatibility Considerations

Iterator appears starting in Node 22. In my first attempt, tests broke
in Node 22 because of this.

### Upgrade Considerations

none.

Author: erights

.changeset/yummy-mangos-behave.md

@@ -0,0 +1,9 @@
+---
+'ses': minor
+---
+
+`overrideTaming: 'moderate'` includes `overrideTaming: 'min'`.
+
+Previously `overrideTaming: 'min'` correctly enabled `Iterator.prototype.constructor` to be overridden by assignment, but due to an oversight, `overrideTaming: 'moderate'` did not. Now it does.
+
+To make such mistakes less likely, this PR also adopts a style where all records within larger enablements triple-dot the corresponding record from a smaller enablement, if present.

packages/ses/src/enablements.js

@@ -95,38 +95,39 @@ export const moderateEnablements = {
   ...minEnablements,
 
   '%ObjectPrototype%': {
-    toString: true,
+    ...minEnablements['%ObjectPrototype%'],
     valueOf: true,
   },
 
-  '%ArrayPrototype%': {
-    toString: true,
-    push: true, // set by "Google Analytics"
-    concat: true, // set by mobx generated code (old TS compiler?)
-    [iteratorSymbol]: true, // set by mobx generated code (old TS compiler?)
-  },
-
-  '%IteratorPrototype%': {
-    [iteratorSymbol]: true, // is sometimes used in custom iterators and generators implementations eg. @rive-app/canvas
-  },
-
   // Function.prototype has no 'prototype' property to enable.
   // Function instances have their own 'name' and 'length' properties
   // which are configurable and non-writable. Thus, they are already
   // non-assignable anyway.
   '%FunctionPrototype%': {
+    ...minEnablements['%FunctionPrototype%'],
     constructor: true, // set by "regenerator-runtime"
     bind: true, // set by "underscore", "express"
-    toString: true, // set by "rollup"
   },
 
   '%ErrorPrototype%': {
+    ...minEnablements['%ErrorPrototype%'],
     constructor: true, // set by "fast-json-patch", "node-fetch"
     message: true,
-    name: true, // set by "precond", "ava", "node-fetch", "node 14"
     toString: true, // set by "bluebird"
   },
 
+  '%IteratorPrototype%': {
+    ...minEnablements['%IteratorPrototype%'],
+    [iteratorSymbol]: true, // is sometimes used in custom iterators and generators implementations eg. @rive-app/canvas
+  },
+
+  '%ArrayPrototype%': {
+    toString: true,
+    push: true, // set by "Google Analytics"
+    concat: true, // set by mobx generated code (old TS compiler?)
+    [iteratorSymbol]: true, // set by mobx generated code (old TS compiler?)
+  },
+
   '%TypeErrorPrototype%': {
     constructor: true, // set by "readable-stream"
     message: true, // set by "tape"

packages/ses/test/enable-property-overrides-default-unsafeError.test.js

@@ -10,11 +10,11 @@ lockdown({
 
 test('property overrides default with unsafe errorTaming', t => {
   overrideTester(t, 'Error', Error(), [
+    'name',
+    'stack',
     'constructor',
     'message',
-    'name',
     'toString',
-    'stack',
   ]);
   overrideTester(t, 'TypeError', TypeError(), [
     'constructor',

packages/ses/test/enable-property-overrides-default.test.js

@@ -10,27 +10,35 @@ lockdown({
 
 test('enablePropertyOverrides - on', t => {
   overrideTester(t, 'Object', {}, ['toString', 'valueOf']);
-  // We allow 'length' *not* because it is in enablements; it is not;
-  // but because each array instance has its own.
-  overrideTester(
-    t,
-    'Array',
-    [],
-    ['toString', 'length', 'push', 'concat', Symbol.iterator],
-  );
   // eslint-disable-next-line func-names, prefer-arrow-callback
   overrideTester(t, 'Function', function () {}, [
+    'toString',
     'constructor',
     'bind',
-    'toString',
   ]);
   overrideTester(t, 'Error', Error(), [
+    'name',
+    'stack',
     'constructor',
     'message',
-    'name',
     'toString',
-    'stack',
   ]);
+  if (typeof Iterator !== 'undefined') {
+    overrideTester(t, 'Iterator', Object.create(Iterator.prototype), [
+      'toString',
+      'constructor',
+      Symbol.toStringTag,
+      Symbol.iterator,
+    ]);
+  }
+  // We allow 'length' *not* because it is in enablements; it is not;
+  // but because each array instance has its own.
+  overrideTester(
+    t,
+    'Array',
+    [],
+    ['toString', 'length', 'push', 'concat', Symbol.iterator],
+  );
   overrideTester(t, 'TypeError', TypeError(), [
     'constructor',
     'message',
@@ -39,9 +47,4 @@ test('enablePropertyOverrides - on', t => {
   // eslint-disable-next-line func-names, prefer-arrow-callback
   overrideTester(t, 'Promise', new Promise(function () {}), ['constructor']);
   overrideTester(t, 'JSON', JSON);
-  if (typeof Iterator !== 'undefined') {
-    overrideTester(t, 'Iterator', Object.create(Iterator.prototype), [
-      Symbol.iterator,
-    ]);
-  }
 });

packages/ses/test/enable-property-overrides-min.test.js

@@ -12,4 +12,15 @@ test('enablePropertyOverrides - on', t => {
   overrideTester(t, 'Object', {}, ['toString']);
   overrideTester(t, 'Function', () => {}, ['toString']);
   overrideTester(t, 'Error', Error(), ['name', 'stack']);
+  if (typeof Iterator !== 'undefined') {
+    overrideTester(t, 'Iterator', Object.create(Iterator.prototype), [
+      'toString',
+      'constructor',
+      Symbol.toStringTag,
+    ]);
+  }
+
+  // We allow 'length' *not* because it is in enablements; it is not;
+  // but because each array instance has its own.
+  overrideTester(t, 'Array', [], ['length']);
 });

packages/ses/test/enable-property-overrides-relaxation.test.js

@@ -0,0 +1,114 @@
+/* eslint-disable no-nested-ternary */
+
+import '../index.js';
+import test from 'ava';
+import {
+  minEnablements,
+  moderateEnablements,
+  severeEnablements,
+} from '../src/enablements.js';
+
+/**
+ * @import {ExecutionContext} from 'ava';
+ */
+
+/**
+ * Matches any non-empty string that consists exclusively of ASCII
+ * letter/digit/underscore/percent sign and does not start with a digit.
+ * Percent sign is allowed for unnamed primordials such as `%ObjectPrototype%`.
+ */
+const identifierLikePatt = /^[a-zA-Z_%][a-zA-Z_%0-9]*$/i;
+
+const builtinSymbols = /** @type {Map<symbol, string>} */ (
+  new Map(
+    Reflect.ownKeys(Symbol).flatMap(key => {
+      const value = Symbol[key];
+      return typeof value === 'symbol' ? [[value, key]] : [];
+    }),
+  )
+);
+
+/** @type {(symbol: symbol) => string} */
+const stringifySymbol = symbol => {
+  const builtinSymbolName = builtinSymbols.get(symbol);
+  if (builtinSymbolName) {
+    return builtinSymbolName.match(identifierLikePatt)
+      ? `Symbol.${builtinSymbolName}`
+      : `Symbol[${JSON.stringify(builtinSymbolName)}]`;
+  }
+  const key = Symbol.keyFor(symbol);
+  return key !== undefined
+    ? `Symbol.for(${JSON.stringify(key)})`
+    : `Symbol(${JSON.stringify(symbol.description)})`;
+};
+
+/**
+ * Assert that some enablement value is a valid relaxation of a base enablement.
+ * `true` may be relaxed only to `true`, "*" may be relaxed to `true` or "*",
+ * and a base record may be relaxed to `true`, "*", or a record that includes a
+ * superset of the base properties in which each property of the relaxation is
+ * either absent from the base element or is (recursively) a valid
+ * relaxation of the corresponding base enablement.
+ *
+ * @param {ExecutionContext} t
+ * @param {any} base
+ * @param {any} relaxation
+ * @param {string} [path]
+ */
+const assertEnablementsRelaxation = (t, base, relaxation, path = '') => {
+  // Relaxing to `true` is always acceptable.
+  if (relaxation === true) return;
+
+  // Otherwise, relaxation must either preserve `true`/"*" or be recursively
+  // acceptable.
+  if (base === true || base === '*') {
+    t.is(
+      relaxation,
+      base,
+      `relaxation must preserve ${JSON.stringify(base)} at ${path || 'top-level'}`,
+    );
+    return;
+  }
+
+  t.is(
+    base === null ? 'null' : typeof base,
+    'object',
+    `base enablement at ${path || 'top-level'} must be \`true\`, "*", or a record`,
+  );
+  if (relaxation === '*') return;
+  t.is(
+    relaxation === null ? 'null' : typeof relaxation,
+    'object',
+    `relaxed enablement at ${path || 'top-level'} must be \`true\`, "*", or a record`,
+  );
+
+  const baseKeys = Reflect.ownKeys(base);
+  const relaxationKeys = Reflect.ownKeys(relaxation);
+  const missingKeys = baseKeys.filter(k => !relaxationKeys.includes(k));
+  t.deepEqual(
+    missingKeys,
+    [],
+    `relaxation must not omit base properties at ${path || 'top-level'}`,
+  );
+  for (const key of baseKeys) {
+    const pathSuffix =
+      typeof key === 'symbol'
+        ? `[${stringifySymbol(key)}]`
+        : key.match(identifierLikePatt)
+          ? `.${key}`
+          : `[${JSON.stringify(key)}]`;
+    const subPath = `${path}${pathSuffix}`.replace(/^[.]/, '');
+    const relaxationEnablement = Object.hasOwn(relaxation, key)
+      ? relaxation[key]
+      : undefined;
+    assertEnablementsRelaxation(t, base[key], relaxationEnablement, subPath);
+  }
+};
+
+test('moderateEnablements relaxes minEnablements', t => {
+  assertEnablementsRelaxation(t, minEnablements, moderateEnablements);
+});
+
+test('severeEnablements relaxes moderateEnablements', t => {
+  assertEnablementsRelaxation(t, moderateEnablements, severeEnablements);
+});