fix(ses): to workaround #2908 use current console (#2934)

Closes: #2908
Refs: #636 #944 #945 nodejs/node#59456

## Description

Due to timing, reporter.js runs before lockdown replaces the system
console with our causal console. Thus, when `makeReporter` makes a
reporter that uses some global console, it uses the platform's original
global console rather than the causal console. I don't know if this was
purposeful or good enough at the time, so I don't know whether this PR
is a workaround or a proper fix. In any case, it fixes the case of
immediate interest by using the current global console to report rather
than the original one.

### Security Considerations

@gibson042 checked below and found that the `console` property of the
global of the start compartments remains mutable after lockdown. This is
a security hazard, but is consistent with our stance on the privileges
available in the start compartment. In any case, this hazard is not
affected by this PR.

### Scaling Considerations

none
### Documentation Considerations

none
### Testing Considerations

Hard to write a regression test to ensure that a stack is output since
there is no portable agreement on what stacks look like. But not
impossible.

Manually running the test case from #2908 , we get the correct outputs
at #2908 (comment) .

Running that test case from #2908 on Node 22 and 24, we find that the
bug does not manifest there either. The problem is ***only*** on Node
20. I don't understand how that can be. In any case, this PR does fix
the behavior across all these versions.

### Compatibility Considerations

If someone suppresses the `console` replacement with `consoleTaming:
'unsafe'`, then this PR in its current state fails to workaround #2908

If some code depends on reporters using the original `console` rather
than the current one, that code might break or misbehave.

### Upgrade Considerations

none

Author: erights

packages/ses/docs/lockdown.md

@@ -492,19 +492,22 @@ 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
+  - The web platform is distinguished by the existence of `window` or
+    `importScripts` (WebWorker), in which case the current console (at the time
+    of reporting) is used.
+  - The Node.js behavior is to report all warnings to `stderr` visually
+    consistent with use of a console group. To do this, it actually
+    reports using the `console.error` method of the current console (at
+    the time of reporting).
+  - SES will use `print` in the absence of a `console`.
+- The `'console'` option forces the web platform behavior, in which the current
+  console (at time of reporting) is used directly.
+  On Node.js, this results in group labels being reported to `stdout`, because
+  that is the unalterable behavior of Node's `console.group*` methods.
+  The global `console` can be replaced, 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
+  reporting arbitrary diagnostics, rather than also being suited to generate
   machine-readable `stdout`.
 - The `'none'` option mutes warnings.
 

packages/ses/src/reporting.js

@@ -1,13 +1,45 @@
-import { functionBind, globalThis } from './commons.js';
+import { globalThis } from './commons.js';
 import { assert } from './error/assert.js';
 
 /**
  * @import {Reporter, GroupReporter} from './reporting-types.js'
  */
 
+/* eslint-disable @endo/no-polymorphic-call */
+/**
+ * To address https://github.com/endojs/endo/issues/2908,
+ * the `consoleReporter` uses the current `console` rather
+ * than the original one.
+ *
+ * @type {GroupReporter}
+ */
+const consoleReporter = {
+  warn(...args) {
+    globalThis.console.warn(...args);
+  },
+  error(...args) {
+    globalThis.console.error(...args);
+  },
+  ...(globalThis.console?.groupCollapsed
+    ? {
+        groupCollapsed(...args) {
+          globalThis.console.groupCollapsed(...args);
+        },
+      }
+    : undefined),
+  ...(globalThis.console?.groupEnd
+    ? {
+        groupEnd() {
+          globalThis.console.groupEnd();
+        },
+      }
+    : undefined),
+};
+/* eslint-enable @endo/no-polymorphic-call */
+
 /**
  * Creates a suitable reporter for internal errors and warnings out of the
- * Node.js console.error to ensure all messages to go stderr, including the
+ * Node.js console.error to ensure all messages go to stderr, including the
  * group label.
  * Accounts for the extra space introduced by console.error as a delimiter
  * between the indent and subsequent arguments.
@@ -51,18 +83,22 @@ export const chooseReporter = reporting => {
   if (reporting === 'none') {
     return makeReportPrinter(mute);
   }
-  if (
-    reporting === 'console' ||
-    globalThis.window === globalThis ||
-    globalThis.importScripts !== undefined
-  ) {
-    return console;
-  }
   if (globalThis.console !== undefined) {
+    if (
+      reporting === 'console' || // asks for console explicitly
+      globalThis.window === globalThis || // likely on browser
+      globalThis.importScripts !== undefined // likely on worker
+    ) {
+      // reporter just delegates directly to the current console
+      return consoleReporter;
+    }
+    assert(reporting === 'platform');
     // On Node.js, we send all feedback to stderr, regardless of purported level.
-    const console = globalThis.console;
-    const error = functionBind(console.error, console);
-    return makeReportPrinter(error);
+    // This uses `consoleReporter.error` instead of `console.error` because we
+    // want the constructed reporter to use the `console.error` of the current
+    // `console`, not the `console` that was installed when the reporter
+    // was created.
+    return makeReportPrinter(consoleReporter.error);
   }
   if (globalThis.print !== undefined) {
     return makeReportPrinter(globalThis.print);

packages/ses/test/error/_throws-and-logs.js

@@ -12,9 +12,6 @@ import {
 // const internalDebugConsole = console;
 
 const defaultCompareLogs = freeze((t, log, goldenLog) => {
-  // For our internal debugging purposes
-  // internalDebugConsole.log('LOG', log);
-
   t.is(log.length, goldenLog.length, 'wrong log length');
   log.forEach((logRecord, i) => {
     const goldenRecord = goldenLog[i];
@@ -107,6 +104,8 @@ export const assertLogs = freeze((t, thunk, goldenLog, options = {}) => {
     globalThis.console = priorConsole;
     if (checkLogs) {
       const log = takeLog();
+      // For our internal debugging purposes
+      // internalDebugConsole.log('LOG', log);
       compareLogs(t, log, goldenLog);
     }
   }