feat(ses): Lockdown reporting option (#2613)

Closes: #2608 ## Description This change introduces a `"reporting"` option to `lockdown` and `repairIntrinsics` that determines the means by which SES will send warnings to diagnostic tools like the web or node `console`, `print`, or nowhere at all. ### Security Considerations None ### Scaling Considerations None ### Documentation Considerations Relevant documentation added to lockdown options and NEWS. ### Testing Considerations This adjusts the existing test to use the `"reporting": "console"` option, since it already verifies that behavior and consequently is suitable for verifying the web behavior while standing on Node.js. This adds a new test that verifies that reporting with the `"platform"` default behavior on node generates no output on stderr and confirms the presence of expected indented and non-indented messages, while being resilient to additional intrinsics being added to the platform. ### Compatibility Considerations None. ### Upgrade Considerations None.
endojs · Oct 28, 2024 · b7c9f16 · b7c9f16
2 parents d6c6c69 + cf7f299
commit b7c9f16
Show file tree

Hide file tree

Showing 12 changed files with 322 additions and 63 deletions.
diff --git a/packages/ses/NEWS.md b/packages/ses/NEWS.md
@@ -5,6 +5,24 @@ User-visible changes in `ses`:
 - Permit [Promise.try](https://github.com/tc39/proposal-promise-try),
   since it has reached Stage 4.
 
+- Adds a `reporting` option to `lockdown` and `repairIntrinsics`.
+
+  The default behavior is `"platform"` which will detect the platform and
+  report warnings according to whether a web `console`, Node.js `console`, or
+  `print` are available.
+  The web platform is distinguished by the existence of `window` or
+  `importScripts` (WebWorker).
+  The Node.js behavior is to report all warnings to `stderr` visually
+  consistent with use of a console group.
+  SES will use `print` in the absence of a `console`.
+  Captures the platform `console` at the time `lockdown` or `repairIntrinsics`
+  are called, not at the time `ses` initializes.
+
+  The `"console"` option forces the web platform behavior.
+  On Node.js, this results in group labels being reported to `stdout`.
+
+  The `"none"` option mutes warnings.
+
 # v1.9.0 (2024-10-10)
 
 - On platforms without

diff --git a/packages/ses/docs/lockdown.md b/packages/ses/docs/lockdown.md
@@ -27,6 +27,7 @@ Each option is explained in its own section below.
 | `consoleTaming`                  | `'safe'`         | `'unsafe'`                             | deep stacks                ([details](#consoletaming-options)) |
 | `errorTaming`                    | `'safe'`         | `'unsafe'` `'unsafe-debug'`            | `errorInstance.stack`      ([details](#errortaming-options)) |
 | `errorTrapping`                  | `'platform'`     | `'exit'` `'abort'` `'report'` `'none'` | handling of uncaught exceptions ([details](#errortrapping-options)) |
+| `reporting`                      | `'platform'`     | `'console'` `'none'`                   | where to report warnings ([details](#reporting-options))
 | `unhandledRejectionTrapping`     | `'report'`       | `'none'`                               | handling of finalized unhandled rejections ([details](#unhandledrejectiontrapping-options)) |
 | `evalTaming`                     | `'safeEval'`     | `'unsafeEval'` `'noEval'`              | `eval` and `Function` of the start compartment ([details](#evaltaming-options)) |
 | `stackFiltering`                 | `'concise'`      | `'verbose'`                            | deep stacks signal/noise   ([details](#stackfiltering-options)) |
@@ -47,6 +48,7 @@ for threading environment variables into a JavaScript program.
 | `consoleTaming`                  | `LOCKDOWN_CONSOLE_TAMING`                    |                       |
 | `errorTaming`                    | `LOCKDOWN_ERROR_TAMING`                      |                       |
 | `errorTrapping`                  | `LOCKDOWN_ERROR_TRAPPING`                    |                       |
+| `reporting`                      | `LOCKDOWN_REPORTING`                         |                       |
 | `unhandledRejectionTrapping`     | `LOCKDOWN_UNHANDLED_REJECTION_TRAPPING`      |                       |
 | `evalTaming`                     | `LOCKDOWN_EVAL_TAMING`                       |                       |
 | `stackFiltering`                 | `LOCKDOWN_STACK_FILTERING`                   |                       |
@@ -459,6 +461,53 @@ the container to exit explicitly, and we highly recommend setting
 - `'none'`: do not install traps for uncaught exceptions. Errors are likely to
   appear as `{}` when they are reported by the default trap.
 
+## `reporting` Options
+
+**Background**: Lockdown and `repairIntrinsics` report warnings if they
+encounter unexpected but repairable variations on the shared intrinsics, which
+regularly occurs if the version of `ses` predates the introduction of new
+language features.
+With the `reporting` option, an application can mute or control the direction
+of these warnings.
+
+```js
+lockdown(); // reporting defaults to 'platform'
+// or
+lockdown({ reporting: 'platform' });
+// vs
+lockdown({ reporting: 'console' });
+// vs
+lockdown({ reporting: 'none' });
+```
+
+If `lockdown` does not receive an `reporting` option, it will respect
+`process.env.LOCKDOWN_REPORTING`.
+
+```console
+LOCKDOWN_REPORTING=platform
+LOCKDOWN_REPORTING=console
+LOCKDOWN_REPORTING=none
+```
+
+- The default behavior is `'platform'` which will detect the platform and
+  report warnings according to whether a web `console`, Node.js `console`, or
+  `print` are available.
+  The web platform is distinguished by the existence of `window` or
+  `importScripts` (WebWorker).
+  The Node.js behavior is to report all warnings to `stderr` visually
+  consistent with use of a console group.
+  SES will use `print` in the absence of a `console`.
+  Captures the platform `console` at the time `lockdown` or `repairIntrinsics`
+  are called, not at the time `ses` initializes.
+- The `'console'` option forces the web platform behavior.
+  On Node.js, this results in group labels being reported to `stdout`.
+  The global `console` can be replaced before `lockdown` so using this option
+  will drive use of `console.groupCollapsed`, `console.groupEnd`,
+  `console.warn`, and `console.error` assuming that console is suited for
+  reporting arbitrary diagnostics rather than also being suited to generate
+  machine-readable `stdout`.
+- The `'none'` option mutes warnings.
+
 ## `unhandledRejectionTrapping` Options
 
 **Background**: Same concerns as `errorTrapping`, but in addition, SES will

diff --git a/packages/ses/src/enable-property-overrides.js b/packages/ses/src/enable-property-overrides.js
@@ -23,6 +23,8 @@ import {
   severeEnablements,
 } from './enablements.js';
 
+/** @import {Reporter} from './reporting-types.js' */
+
 /**
  * For a special set of properties defined in the `enablement` whitelist,
  * `enablePropertyOverrides` ensures that the effect of freezing does not
@@ -75,11 +77,13 @@ import {
  *
  * @param {Record<string, any>} intrinsics
  * @param {'min' | 'moderate' | 'severe'} overrideTaming
+ * @param {Reporter} reporter
  * @param {Iterable<string | symbol>} [overrideDebug]
  */
 export default function enablePropertyOverrides(
   intrinsics,
   overrideTaming,
+  { warn },
   overrideDebug = [],
 ) {
   const debugProperties = new Set(overrideDebug);
@@ -109,8 +113,7 @@ export default function enablePropertyOverrides(
               this[prop] = newValue;
             } else {
               if (isDebug) {
-                // eslint-disable-next-line @endo/no-polymorphic-call
-                console.error(TypeError(`Override property ${prop}`));
+                warn(TypeError(`Override property ${prop}`));
               }
               defineProperty(this, prop, {
                 value: newValue,

diff --git a/packages/ses/src/lockdown.js b/packages/ses/src/lockdown.js
@@ -58,6 +58,7 @@ import { tameSymbolConstructor } from './tame-symbol-constructor.js';
 import { tameFauxDataProperties } from './tame-faux-data-properties.js';
 import { tameRegeneratorRuntime } from './tame-regenerator-runtime.js';
 import { shimArrayBufferTransfer } from './shim-arraybuffer-transfer.js';
+import { reportInGroup, chooseReporter } from './reporting.js';
 
 /** @import {LockdownOptions} from '../types.js' */
 
@@ -162,19 +163,24 @@ export const repairIntrinsics = (options = {}) => {
 
   const {
     errorTaming = getenv('LOCKDOWN_ERROR_TAMING', 'safe'),
-    errorTrapping = /** @type {"platform" | "none" | "report" | "abort" | "exit" | undefined} */ (
+    errorTrapping = /** @type {"platform" | "none" | "report" | "abort" | "exit"} */ (
       getenv('LOCKDOWN_ERROR_TRAPPING', 'platform')
     ),
-    unhandledRejectionTrapping = /** @type {"none" | "report" | undefined} */ (
+    reporting = /** @type {"platform" | "console" | "none"} */ (
+      getenv('LOCKDOWN_REPORTING', 'platform')
+    ),
+    unhandledRejectionTrapping = /** @type {"none" | "report"} */ (
       getenv('LOCKDOWN_UNHANDLED_REJECTION_TRAPPING', 'report')
     ),
     regExpTaming = getenv('LOCKDOWN_REGEXP_TAMING', 'safe'),
     localeTaming = getenv('LOCKDOWN_LOCALE_TAMING', 'safe'),
 
-    consoleTaming = /** @type {'unsafe' | 'safe' | undefined} */ (
+    consoleTaming = /** @type {'unsafe' | 'safe'} */ (
       getenv('LOCKDOWN_CONSOLE_TAMING', 'safe')
     ),
-    overrideTaming = getenv('LOCKDOWN_OVERRIDE_TAMING', 'moderate'),
+    overrideTaming = /** @type {'moderate' | 'min' | 'severe'} */ (
+      getenv('LOCKDOWN_OVERRIDE_TAMING', 'moderate')
+    ),
     stackFiltering = getenv('LOCKDOWN_STACK_FILTERING', 'concise'),
     domainTaming = getenv('LOCKDOWN_DOMAIN_TAMING', 'safe'),
     evalTaming = getenv('LOCKDOWN_EVAL_TAMING', 'safeEval'),
@@ -208,6 +214,8 @@ export const repairIntrinsics = (options = {}) => {
   extraOptionsNames.length === 0 ||
     Fail`lockdown(): non supported option ${q(extraOptionsNames)}`;
 
+  const reporter = chooseReporter(reporting);
+
   priorRepairIntrinsics === undefined ||
     // eslint-disable-next-line @endo/no-polymorphic-call
     assert.fail(
@@ -363,7 +371,16 @@ export const repairIntrinsics = (options = {}) => {
   // Remove non-standard properties.
   // All remaining function encountered during whitelisting are
   // branded as honorary native functions.
-  whitelistIntrinsics(intrinsics, markVirtualizedNativeFunction);
+  reportInGroup(
+    'SES Removing unpermitted intrinsics',
+    reporter,
+    groupReporter =>
+      whitelistIntrinsics(
+        intrinsics,
+        markVirtualizedNativeFunction,
+        groupReporter,
+      ),
+  );
 
   // Initialize the powerful initial global, i.e., the global of the
   // start compartment, from the intrinsics.
@@ -425,7 +442,14 @@ export const repairIntrinsics = (options = {}) => {
     // therefore before vetted shims rather than afterwards. It is not
     // clear yet which is better.
     // @ts-ignore enablePropertyOverrides does its own input validation
-    enablePropertyOverrides(intrinsics, overrideTaming, overrideDebug);
+    reportInGroup('SES Enabling property overrides', reporter, groupReporter =>
+      enablePropertyOverrides(
+        intrinsics,
+        overrideTaming,
+        groupReporter,
+        overrideDebug,
+      ),
+    );
     if (legacyRegeneratorRuntimeTaming === 'unsafe-ignore') {
       tameRegeneratorRuntime();
     }

diff --git a/packages/ses/src/permits-intrinsics.js b/packages/ses/src/permits-intrinsics.js
@@ -62,29 +62,24 @@ import {
   symbolKeyFor,
 } from './commons.js';
 
+/**
+ * @import {Reporter} from './reporting-types.js'
+ */
+
 /**
  * whitelistIntrinsics()
  * Removes all non-allowed properties found by recursively and
  * reflectively walking own property chains.
  *
  * @param {object} intrinsics
  * @param {(object) => void} markVirtualizedNativeFunction
+ * @param {Reporter} reporter
  */
 export default function whitelistIntrinsics(
   intrinsics,
   markVirtualizedNativeFunction,
+  { warn, error },
 ) {
-  let groupStarted = false;
-  const inConsoleGroup = (level, ...args) => {
-    if (!groupStarted) {
-      // eslint-disable-next-line @endo/no-polymorphic-call
-      console.groupCollapsed('Removing unpermitted intrinsics');
-      groupStarted = true;
-    }
-    // eslint-disable-next-line @endo/no-polymorphic-call
-    return console[level](...args);
-  };
-
   // These primitives are allowed for permits.
   const primitives = ['undefined', 'boolean', 'number', 'string', 'symbol'];
 
@@ -294,7 +289,7 @@ export default function whitelistIntrinsics(
         // that we are removing it so we know to look into it, as happens when
         // the language evolves new features to existing intrinsics.
         if (subPermit !== false) {
-          inConsoleGroup('warn', `Removing ${subPath}`);
+          warn(`Removing ${subPath}`);
         }
         try {
           delete obj[prop];
@@ -303,32 +298,22 @@ export default function whitelistIntrinsics(
             if (typeof obj === 'function' && prop === 'prototype') {
               obj.prototype = undefined;
               if (obj.prototype === undefined) {
-                inConsoleGroup(
-                  'warn',
-                  `Tolerating undeletable ${subPath} === undefined`,
-                );
+                warn(`Tolerating undeletable ${subPath} === undefined`);
                 // eslint-disable-next-line no-continue
                 continue;
               }
             }
-            inConsoleGroup('error', `failed to delete ${subPath}`, err);
+            error(`failed to delete ${subPath}`, err);
           } else {
-            inConsoleGroup('error', `deleting ${subPath} threw`, err);
+            error(`deleting ${subPath} threw`, err);
           }
           throw err;
         }
       }
     }
   }
 
-  try {
-    // Start path with 'intrinsics' to clarify that properties are not
-    // removed from the global object by the whitelisting operation.
-    visitProperties('intrinsics', intrinsics, permitted);
-  } finally {
-    if (groupStarted) {
-      // eslint-disable-next-line @endo/no-polymorphic-call
-      console.groupEnd();
-    }
-  }
+  // Start path with 'intrinsics' to clarify that properties are not
+  // removed from the global object by the whitelisting operation.
+  visitProperties('intrinsics', intrinsics, permitted);
 }
diff --git a/packages/ses/src/reporting-types.d.ts b/packages/ses/src/reporting-types.d.ts
@@ -0,0 +1,13 @@
+/* eslint-disable no-restricted-globals */
+
+export type Reporter = {
+  warn: (...message: Array<any>) => void;
+  error: (...message: Array<string>) => void;
+};
+
+export type GroupReporter = Reporter & {
+  groupCollapsed: (label: string) => void;
+  groupEnd: () => void;
+};
+
+// Console implements GroupReporter