fix: tame Error constructor

packages/ses/src/tame-global-error-object.js

@@ -10,6 +10,44 @@ export const NativeErrors = [
   URIError,
 ];
 
+// Whitelist names from https://v8.dev/docs/stack-trace-api
+// Whitelisting only the names used by error-stack-shim/src/v8StackFrames
+// callSiteToFrame to shim the error stack proposal.
+const safeV8CallSiteMethodNames = [
+  // suppress 'getThis' definitely
+  'getTypeName',
+  // suppress 'getFunction' definitely
+  'getFunctionName',
+  'getMethodName',
+  'getFileName',
+  'getLineNumber',
+  'getColumnNumber',
+  'getEvalOrigin',
+  // suppress 'isTopLevel' for now
+  'isEval',
+  // suppress 'isNative' for now
+  'isConstructor',
+  'isAsync',
+  // suppress 'isPromiseAll' for now
+  // suppress 'getPromiseIndex' for now
+
+  // Additional names found by experiment, absent from
+  // https://v8.dev/docs/stack-trace-api
+
+  // suppress 'getPosition' for now
+  // suppress 'getScriptNameOrSourceURL' for now
+  'toString', // TODO replace to use only whitelisted info
+];
+
+// TODO this is a ridiculously expensive way to attenuate callsites.
+// Before that matters, we should switch to a reasonable representation.
+const safeV8CallSiteFacet = callSite => {
+  const methodEntry = name => [name, () => callSite[name]()];
+  return Object.fromEntries(safeV8CallSiteMethodNames.map(methodEntry));
+};
+
+const safeV8SST = sst => sst.map(safeV8CallSiteFacet);
+
 export default function tameGlobalErrorObject(errorTaming = 'safe') {
   if (errorTaming !== 'safe' && errorTaming !== 'unsafe') {
     throw new Error(`unrecognized errorTaming ${errorTaming}`);
@@ -40,7 +78,12 @@ export default function tameGlobalErrorObject(errorTaming = 'safe') {
 
   // Use concise methods to obtain named functions without constructors.
   const tamedMethods = {
-    captureStackTrace(error, optFn = undefined) {
+    // The optional `optFn` argument is for cutting off the bottom of
+    // the stack --- for capturing the stack only above the topmost
+    // call to that function. Since this isn't the "real" captureStackTrace
+    // but instead calls the real one, if no other cutoff is provided,
+    // we cut this one off.
+    captureStackTrace(error, optFn = tamedMethods.captureStackTrace) {
       if (
         errorTaming === 'unsafe' &&
         typeof originalError.captureStackTrace === 'function'
@@ -53,7 +96,52 @@ export default function tameGlobalErrorObject(errorTaming = 'safe') {
     },
   };
 
+  // A prepareFn is a prepareStackTrace function.
+  // An sst is a `structuredStackTrace`, which is an array of
+  // callsites.
+  // A user prepareFn is a prepareFn defined by a client of this API,
+  // and provided by assigning to `Error.prepareStackTrace`.
+  // A user prepareFn should only receive an attenuated sst, which
+  // is an array of attenuated callsites.
+  // A system prepareFn is the prepareFn created by this module to
+  // be installed on the real `Error` constructor, to receive
+  // an original sst, i.e., an array of unattenuated callsites.
+  // An input prepareFn is a function the user assigns to
+  // `Error.prepareStackTrace`, which might be a user prepareFn or
+  // a system prepareFn previously obtained by reading
+  // `Error.prepareStackTrace`.
+
+  // A weakset branding some functions as system prepareFns, all of which
+  // must be defined by this module, since they can receive an
+  // unattenuated sst.
+  const systemPrepareFnSet = new WeakSet();
+
+  const systemPrepareFnFor = inputPrepareFn => {
+    if (systemPrepareFnSet.has(inputPrepareFn)) {
+      return inputPrepareFn;
+    }
+    // Use concise methods to obtain named functions without constructors.
+    const systemMethods = {
+      prepareStackTrace(error, sst) {
+        return inputPrepareFn(error, safeV8SST(sst));
+      },
+    };
+    systemPrepareFnSet.add(systemMethods.prepareStackTrace);
+    return systemMethods.prepareStackTrace;
+  };
+
   const ErrorPrototype = originalError.prototype;
+  if (typeof originalError.captureStackTrace === 'function') {
+    // Define captureStackTrace only on v8
+    defineProperties(tamedError, {
+      captureStackTrace: {
+        value: tamedMethods.captureStackTrace,
+        writable: true,
+        enumerable: false,
+        configurable: true,
+      },
+    });
+  }
   defineProperties(tamedError, {
     length: { value: 1 },
     prototype: {
@@ -62,12 +150,6 @@ export default function tameGlobalErrorObject(errorTaming = 'safe') {
       enumerable: false,
       configurable: false,
     },
-    captureStackTrace: {
-      value: tamedMethods.captureStackTrace,
-      writable: true,
-      enumerable: false,
-      configurable: true,
-    },
     stackTraceLimit: {
       get() {
         if (
@@ -77,8 +159,15 @@ export default function tameGlobalErrorObject(errorTaming = 'safe') {
           // originalError.stackTraceLimit is only on v8
           return originalError.stackTraceLimit;
         }
-        return 0;
+        return undefined;
       },
+      // https://v8.dev/docs/stack-trace-api#compatibility advises that
+      // programmers can "always" set `Error.stackTraceLimit` and
+      // `Error.prepareStackTrace` even on non-v8 platforms. On non-v8
+      // it will have no effect, but this advise only makes sense
+      // if the assignment itself does not fail, which it would
+      // if `Error` were naively frozen. Hence, we add setters that
+      // accept but ignore the assignment on non-v8 platforms.
       set(newLimit) {
         if (
           errorTaming === 'unsafe' &&
@@ -97,6 +186,37 @@ export default function tameGlobalErrorObject(errorTaming = 'safe') {
       enumerable: false,
       configurable: true,
     },
+    prepareStackTrace: {
+      get() {
+        if (errorTaming === 'unsafe') {
+          return originalError.prepareStackTrace;
+        }
+        // By returning undefined, hopefully this means the VM will next consult
+        // originalError.prepareStackTrace, even on node despite
+        // https://bugs.chromium.org/p/v8/issues/detail?id=10551#c3
+        // or, if absent, fallback to the default behavior.
+        return undefined;
+      },
+      set(inputPrepareStackTraceFn) {
+        if (errorTaming === 'unsafe') {
+          if (typeof inputPrepareStackTraceFn === 'function') {
+            const systemPrepareFn = systemPrepareFnFor(
+              inputPrepareStackTraceFn,
+            );
+            originalError.prepareStackTrace = systemPrepareFn;
+          } else {
+            delete originalError.prepareStackTrace;
+          }
+          // We place the useless return on the next line to ensure
+          // that anything we place after the if in the future only
+          // happens if the then-case does not.
+          // eslint-disable-next-line no-useless-return
+          return;
+        }
+      },
+      enumerable: false,
+      configurable: true,
+    },
   });
 
   // TODO uncomment. See TODO note above
@@ -111,7 +231,7 @@ export default function tameGlobalErrorObject(errorTaming = 'safe') {
     },
     stackTraceLimit: {
       get() {
-        return 0;
+        return undefined;
       },
       set(_) {
         // ignore
@@ -120,6 +240,16 @@ export default function tameGlobalErrorObject(errorTaming = 'safe') {
       enumerable: false,
       configurable: true,
     },
+    prepareStackTrace: {
+      get() {
+        return undefined;
+      },
+      set(_) {
+        // ignore
+      },
+      enumerable: false,
+      configurable: true,
+    },
   });
   */
 

packages/ses/src/whitelist.js

@@ -379,8 +379,10 @@ export default {
     prototype: 'ErrorPrototype',
     // Non standard, v8 only, used by tap
     captureStackTrace: fn,
-    // Non standard, v8 only, used by tap
+    // Non standard, v8 only, used by tap, tamed to accessor
     stackTraceLimit: accessor,
+    // Non standard, v8 only, used by several, tamed to accessor
+    prepareStackTrace: accessor,
   },
 
   ErrorPrototype: {

packages/ses/test/error-manipulation.test.js

@@ -0,0 +1,167 @@
+/* global lockdown */
+import test from 'tape';
+import '../src/main.js';
+
+// TODO test Error API in
+//    * non - start compartments
+//    * with { errorTaming: 'safe' }
+//    * on non-v8
+lockdown({ errorTaming: 'unsafe' });
+
+// depd (https://github.com/dougwilson/nodejs-depd) uses a stack trace to
+// determine the call site of a deprecated function
+
+function simulateDepd() {
+  function prepareObjectStackTrace(obj, stack) {
+    return stack;
+  }
+
+  function callSiteLocation(callSite) {
+    let file = callSite.getFileName() || '<anonymous>';
+    const line = callSite.getLineNumber();
+    const colm = callSite.getColumnNumber();
+    if (callSite.isEval()) {
+      file = `${callSite.getEvalOrigin()}, ${file}`;
+    }
+    const site = [file, line, colm];
+    site.callSite = callSite;
+    site.name = callSite.getFunctionName();
+    return site;
+  }
+
+  function getStack() {
+    const limit = Error.stackTraceLimit;
+    const obj = {};
+    const prep = Error.prepareStackTrace;
+    Error.prepareStackTrace = prepareObjectStackTrace;
+    Error.stackTraceLimit = Math.max(10, limit);
+    // capture the stack
+    Error.captureStackTrace(obj);
+    // slice this function off the top
+    const stack = obj.stack.slice(1);
+    Error.prepareStackTrace = prep;
+    Error.stackTraceLimit = limit;
+    return stack;
+  }
+
+  function middle() {
+    return getStack();
+  }
+
+  const site = callSiteLocation(middle()[0]);
+  return site.name;
+}
+
+test('Error compatibility - depd', t => {
+  // the Start Compartment should support this sort of manipulation
+  const name = simulateDepd();
+  t.equal(name, 'middle');
+
+  // however a new Compartment should not
+  // const c = new Compartment({ console });
+  // const sim = c.evaluate(`(${simulateDepd})`);
+  // t.throws(() => sim(), /Cannot add property prepareStackTrace, object is not extensible/);
+
+  t.end();
+});
+
+// callstack (https://github.com/nailgun/node-callstack#readme) returns a
+// stack as a list of strings, by reading Error().stack
+function simulateCallstack() {
+  function callstack() {
+    return new Error().stack.split('\n').splice(2);
+  }
+  function middle() {
+    return callstack();
+  }
+  return middle();
+}
+
+test('Error compatibility - callstack', t => {
+  const stack = simulateCallstack();
+  // TODO: upgrade to tape 5.x for t.match
+  // t.match(stack[0], /at middle/, '"middle" found in callstack() output');
+  t.notEqual(
+    stack[0].search(/at middle/),
+    -1,
+    '"middle" found in callstack() output',
+  );
+
+  t.end();
+});
+
+// callsite (https://www.npmjs.com/package/callsite) returns a list of stack
+// frames, obtained by replacing Error.prepareStackTrace .
+function simulateCallsite() {
+  function callsite() {
+    const orig = Error.prepareStackTrace;
+    Error.prepareStackTrace = (_, stack) => stack;
+    const err = new Error();
+
+    // note: the upstream `callsite` library uses
+    // `Error.captureStackTrace(err, arguments.callee);`
+    // We test a fixed version, to exercise compatibility with the SES
+    // unsafe-tamed Error object, even though the original uses a
+    // sloppy mode (non-strict mode) `arguments.callee` that cannot
+    // work in module code or under SES.
+    Error.captureStackTrace(err, callsite);
+    const { stack } = err;
+    Error.prepareStackTrace = orig;
+    return stack;
+  }
+
+  function middle() {
+    return callsite();
+  }
+
+  return middle()[0].getFunctionName();
+}
+
+test('Error compatibility - callsite', t => {
+  const name = simulateCallsite();
+  t.equal(name, 'middle');
+
+  t.end();
+});
+
+// callsites from
+// https://github.com/sindresorhus/callsites/blob/master/index.js
+// triggers prepareStackTrace by accessing the `.stack` property
+// of an error, rather than calling `captureStackTrace`.
+function simulateCallsites() {
+  function callsites() {
+    const orig = Error.prepareStackTrace;
+    Error.prepareStackTrace = (_, stack) => stack;
+    const stack = new Error().stack.slice(1);
+    Error.prepareStackTrace = orig;
+    return stack;
+  }
+
+  function middle() {
+    return callsites();
+  }
+
+  return middle()[0].getFunctionName();
+}
+
+test('Error compatibility - callsites', t => {
+  const name = simulateCallsites();
+  t.equal(name, 'middle');
+
+  t.end();
+});
+
+test('Error compatibility - save and restore prepareStackTrace', t => {
+  const pst1 = (_err, _sst) => 'x';
+  Error.prepareStackTrace = pst1;
+  t.equal(new Error().stack, 'x');
+  const pst2 = Error.prepareStackTrace;
+  t.notEqual(pst1, pst2);
+  t.equal(pst2({}, []), 'x');
+  Error.prepareStackTrace = pst2;
+  t.equal(new Error().stack, 'x');
+  const pst3 = Error.prepareStackTrace;
+  t.equal(pst2, pst3);
+
+  t.end();
+});