From 6e92951912ffde3603437cc2f0a8c31879467ff7 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Tue, 2 Jan 2024 16:18:17 -0500 Subject: [PATCH 1/4] fix(ses): Support an incomplete shimmed globalEnv.process Fixes #1917 --- packages/ses/src/error/tame-console.js | 24 ++++++++++++------- .../ses/test/test-lockdown-shimmed-process.js | 22 +++++++++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 packages/ses/test/test-lockdown-shimmed-process.js diff --git a/packages/ses/src/error/tame-console.js b/packages/ses/src/error/tame-console.js index 000f08207f..f240044404 100644 --- a/packages/ses/src/error/tame-console.js +++ b/packages/ses/src/error/tame-console.js @@ -95,21 +95,29 @@ export const tameConsole = ( /* eslint-disable @endo/no-polymorphic-call */ // Node.js - if (errorTrapping !== 'none' && globalThis.process !== undefined) { - globalThis.process.on('uncaughtException', error => { + const globalProcess = globalThis.process; + if ( + errorTrapping !== 'none' && + globalProcess && + typeof globalProcess === 'object' && + typeof globalProcess.on === 'function' + ) { + globalProcess.on('uncaughtException', error => { // causalConsole is born frozen so not vulnerable to method tampering. ourConsole.error(error); if (errorTrapping === 'platform' || errorTrapping === 'exit') { - globalThis.process.exit(globalThis.process.exitCode || -1); + globalProcess.exit(globalProcess.exitCode || -1); } else if (errorTrapping === 'abort') { - globalThis.process.abort(); + globalProcess.abort(); } }); } if ( unhandledRejectionTrapping !== 'none' && - globalThis.process !== undefined + globalProcess && + typeof globalProcess === 'object' && + typeof globalProcess.on === 'function' ) { const handleRejection = reason => { // 'platform' and 'report' just log the reason. @@ -119,9 +127,9 @@ export const tameConsole = ( const h = makeRejectionHandlers(handleRejection); if (h) { // Rejection handlers are supported. - globalThis.process.on('unhandledRejection', h.unhandledRejectionHandler); - globalThis.process.on('rejectionHandled', h.rejectionHandledHandler); - globalThis.process.on('exit', h.processTerminationHandler); + globalProcess.on('unhandledRejection', h.unhandledRejectionHandler); + globalProcess.on('rejectionHandled', h.rejectionHandledHandler); + globalProcess.on('exit', h.processTerminationHandler); } } diff --git a/packages/ses/test/test-lockdown-shimmed-process.js b/packages/ses/test/test-lockdown-shimmed-process.js new file mode 100644 index 0000000000..2465043e9d --- /dev/null +++ b/packages/ses/test/test-lockdown-shimmed-process.js @@ -0,0 +1,22 @@ +/* global globalThis */ + +import test from 'ava'; +import '../index.js'; + +test('shimmed globalThis.process', t => { + const process = {}; + Object.defineProperty(globalThis, 'process', { + value: process, + configurable: false, + writable: false, + }); + t.is(globalThis.process, process); + t.is(globalThis.process.on, undefined); + lockdown({ + consoleTaming: 'safe', + errorTrapping: 'platform', + unhandledRejectionTrapping: 'report', + }); + t.is(globalThis.process, process); + t.is(globalThis.process.on, undefined); +}); From bc1e2dc383f7d6ec874886b26656d072cc1b0197 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Tue, 2 Jan 2024 16:21:30 -0500 Subject: [PATCH 2/4] chore: Make process/window testing more uniform --- packages/env-options/src/env-options.js | 31 ++++++++++++------------- packages/ses/src/error/fatal-assert.js | 2 +- packages/ses/src/error/tame-console.js | 25 +++++++++----------- packages/ses/src/tame-domains.js | 10 ++++---- 4 files changed, 31 insertions(+), 37 deletions(-) diff --git a/packages/env-options/src/env-options.js b/packages/env-options/src/env-options.js index b0047eee15..ae4d3e5fb0 100644 --- a/packages/env-options/src/env-options.js +++ b/packages/env-options/src/env-options.js @@ -61,22 +61,21 @@ export const makeEnvironmentCaptor = aGlobal => { /** @type {string} */ let setting = defaultSetting; - const globalProcess = aGlobal.process; - if (globalProcess && typeof globalProcess === 'object') { - const globalEnv = globalProcess.env; - if (globalEnv && typeof globalEnv === 'object') { - if (optionName in globalEnv) { - arrayPush(capturedEnvironmentOptionNames, optionName); - const optionValue = globalEnv[optionName]; - // eslint-disable-next-line @endo/no-polymorphic-call - typeof optionValue === 'string' || - Fail`Environment option named ${q( - optionName, - )}, if present, must have a corresponding string value, got ${q( - optionValue, - )}`; - setting = optionValue; - } + const globalProcess = aGlobal.process || undefined; + const globalEnv = + (typeof globalProcess === 'object' && globalProcess.env) || undefined; + if (typeof globalEnv === 'object') { + if (optionName in globalEnv) { + arrayPush(capturedEnvironmentOptionNames, optionName); + const optionValue = globalEnv[optionName]; + // eslint-disable-next-line @endo/no-polymorphic-call + typeof optionValue === 'string' || + Fail`Environment option named ${q( + optionName, + )}, if present, must have a corresponding string value, got ${q( + optionValue, + )}`; + setting = optionValue; } } return setting; diff --git a/packages/ses/src/error/fatal-assert.js b/packages/ses/src/error/fatal-assert.js index 219144be80..29d1d0dc18 100644 --- a/packages/ses/src/error/fatal-assert.js +++ b/packages/ses/src/error/fatal-assert.js @@ -10,7 +10,7 @@ let abandon; // below). Currently it only checks for the `process.abort` or `process.exit` // found on Node. It should also sniff for a vat terminating function expected // to be found within the start compartment of SwingSet vats. What else? -if (typeof process === 'object') { +if (typeof process === 'object' && process) { abandon = process.abort || process.exit; } let raise; diff --git a/packages/ses/src/error/tame-console.js b/packages/ses/src/error/tame-console.js index f240044404..ab24de45af 100644 --- a/packages/ses/src/error/tame-console.js +++ b/packages/ses/src/error/tame-console.js @@ -95,10 +95,9 @@ export const tameConsole = ( /* eslint-disable @endo/no-polymorphic-call */ // Node.js - const globalProcess = globalThis.process; + const globalProcess = globalThis.process || undefined; if ( errorTrapping !== 'none' && - globalProcess && typeof globalProcess === 'object' && typeof globalProcess.on === 'function' ) { @@ -112,10 +111,8 @@ export const tameConsole = ( } }); } - if ( unhandledRejectionTrapping !== 'none' && - globalProcess && typeof globalProcess === 'object' && typeof globalProcess.on === 'function' ) { @@ -134,25 +131,25 @@ export const tameConsole = ( } // Browser + const globalWindow = globalThis.window || undefined; if ( errorTrapping !== 'none' && - globalThis.window !== undefined && - globalThis.window.addEventListener !== undefined + typeof globalWindow === 'object' && + typeof globalWindow.addEventListener === 'function' ) { - globalThis.window.addEventListener('error', event => { + globalWindow.addEventListener('error', event => { event.preventDefault(); // 'platform' and 'report' just log the reason. ourConsole.error(event.error); if (errorTrapping === 'exit' || errorTrapping === 'abort') { - globalThis.window.location.href = `about:blank`; + globalWindow.location.href = `about:blank`; } }); } - if ( unhandledRejectionTrapping !== 'none' && - globalThis.window !== undefined && - globalThis.window.addEventListener !== undefined + typeof globalWindow === 'object' && + typeof globalWindow.addEventListener === 'function' ) { const handleRejection = reason => { ourConsole.error('SES_UNHANDLED_REJECTION:', reason); @@ -161,17 +158,17 @@ export const tameConsole = ( const h = makeRejectionHandlers(handleRejection); if (h) { // Rejection handlers are supported. - globalThis.window.addEventListener('unhandledrejection', event => { + globalWindow.addEventListener('unhandledrejection', event => { event.preventDefault(); h.unhandledRejectionHandler(event.reason, event.promise); }); - globalThis.window.addEventListener('rejectionhandled', event => { + globalWindow.addEventListener('rejectionhandled', event => { event.preventDefault(); h.rejectionHandledHandler(event.promise); }); - globalThis.window.addEventListener('beforeunload', _event => { + globalWindow.addEventListener('beforeunload', _event => { h.processTerminationHandler(); }); } diff --git a/packages/ses/src/tame-domains.js b/packages/ses/src/tame-domains.js index d14e07a884..7b5adcbe4c 100644 --- a/packages/ses/src/tame-domains.js +++ b/packages/ses/src/tame-domains.js @@ -17,12 +17,10 @@ export function tameDomains(domainTaming = 'safe') { } // Protect against the hazard presented by Node.js domains. - if (typeof globalThis.process === 'object' && globalThis.process !== null) { + const globalProcess = globalThis.process || undefined; + if (typeof globalProcess === 'object') { // Check whether domains were initialized. - const domainDescriptor = getOwnPropertyDescriptor( - globalThis.process, - 'domain', - ); + const domainDescriptor = getOwnPropertyDescriptor(globalProcess, 'domain'); if (domainDescriptor !== undefined && domainDescriptor.get !== undefined) { // The domain descriptor on Node.js initially has value: null, which // becomes a get, set pair after domains initialize. @@ -37,7 +35,7 @@ export function tameDomains(domainTaming = 'safe') { // The domain module merely throws an exception when it attempts to define // the domain property of the process global during its initialization. // We have no better recourse because Node.js uses defineProperty too. - defineProperty(globalThis.process, 'domain', { + defineProperty(globalProcess, 'domain', { value: null, configurable: false, writable: false, From 5d637d046fdc6354cef25514aea4e3aa37fa4792 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Tue, 2 Jan 2024 20:40:10 -0500 Subject: [PATCH 3/4] feat(ses): Fail fast when a required process.exit or process.abort method is missing --- packages/ses/src/error/tame-console.js | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/ses/src/error/tame-console.js b/packages/ses/src/error/tame-console.js index ab24de45af..257bc61b3e 100644 --- a/packages/ses/src/error/tame-console.js +++ b/packages/ses/src/error/tame-console.js @@ -1,6 +1,7 @@ // @ts-check import { + // Using TypeError minimizes risk of exposing the feral Error constructor TypeError, apply, defineProperty, @@ -101,13 +102,27 @@ export const tameConsole = ( typeof globalProcess === 'object' && typeof globalProcess.on === 'function' ) { + let terminate; + if (errorTrapping === 'platform' || errorTrapping === 'exit') { + const { exit } = globalProcess; + if (typeof exit !== 'function') { + // There is a function-valued process.on but no function-valued process.exit; + // fail early without caring whether errorTrapping is "platform" only by default. + throw TypeError('missing process.exit'); + } + terminate = () => exit(globalProcess.exitCode || -1); + } else if (errorTrapping === 'abort') { + terminate = globalProcess.abort; + if (typeof terminate !== 'function') { + throw TypeError('missing process.abort'); + } + } + globalProcess.on('uncaughtException', error => { // causalConsole is born frozen so not vulnerable to method tampering. ourConsole.error(error); - if (errorTrapping === 'platform' || errorTrapping === 'exit') { - globalProcess.exit(globalProcess.exitCode || -1); - } else if (errorTrapping === 'abort') { - globalProcess.abort(); + if (terminate) { + terminate(); } }); } From 08cbae53bce12aef401f1536ececc3ff47caa1e7 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Tue, 2 Jan 2024 20:42:30 -0500 Subject: [PATCH 4/4] style(ses): DRY out a failFast helper function --- packages/ses/src/error/tame-console.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/ses/src/error/tame-console.js b/packages/ses/src/error/tame-console.js index 257bc61b3e..33fac1c122 100644 --- a/packages/ses/src/error/tame-console.js +++ b/packages/ses/src/error/tame-console.js @@ -14,6 +14,10 @@ import { makeRejectionHandlers } from './unhandled-rejection.js'; import './types.js'; import './internal-types.js'; +const failFast = message => { + throw TypeError(message); +}; + const wrapLogger = (logger, thisArg) => freeze((...args) => apply(logger, thisArg, args)); @@ -61,9 +65,9 @@ export const tameConsole = ( unhandledRejectionTrapping = 'report', optGetStackString = undefined, ) => { - if (consoleTaming !== 'safe' && consoleTaming !== 'unsafe') { - throw TypeError(`unrecognized consoleTaming ${consoleTaming}`); - } + consoleTaming === 'safe' || + consoleTaming === 'unsafe' || + failFast(`unrecognized consoleTaming ${consoleTaming}`); let loggedErrorHandler; if (optGetStackString === undefined) { @@ -105,17 +109,13 @@ export const tameConsole = ( let terminate; if (errorTrapping === 'platform' || errorTrapping === 'exit') { const { exit } = globalProcess; - if (typeof exit !== 'function') { - // There is a function-valued process.on but no function-valued process.exit; - // fail early without caring whether errorTrapping is "platform" only by default. - throw TypeError('missing process.exit'); - } + // If there is a function-valued process.on but no function-valued process.exit, + // fail early without caring whether errorTrapping is "platform" only by default. + typeof exit === 'function' || failFast('missing process.exit'); terminate = () => exit(globalProcess.exitCode || -1); } else if (errorTrapping === 'abort') { terminate = globalProcess.abort; - if (typeof terminate !== 'function') { - throw TypeError('missing process.abort'); - } + typeof terminate === 'function' || failFast('missing process.abort'); } globalProcess.on('uncaughtException', error => {