From ac88da5f5661e8c366b7956edd064729dc9b86e3 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Fri, 19 Jul 2024 01:55:20 -0700 Subject: [PATCH] fix(ses): temp patch in Endo 2355,2359 line-number workaround (#9711) closes: https://github.com/Agoric/agoric-sdk/issues/8662 refs: https://github.com/endojs/endo/pull/2355 https://github.com/endojs/endo/issues/2348 ## Description https://github.com/endojs/endo/pull/2355 makes it possible to see accurate line numbers into TypeScript Ava tests run under Node, which would fix https://github.com/Agoric/agoric-sdk/issues/8662 as of the next endo-release-agoric-sdk-sync cycle. To get this benefit before then, this PR turns https://github.com/endojs/endo/pull/2355 into an agoric-sdk patch of endo. This PR also adds a test case to demonstrate that the fix works, and updates the `env.md` file to document the new environment variable for enabling this behavior. ### Security Considerations See Security Consideration of https://github.com/endojs/endo/pull/2355 . In short, this feature sacrifices security for a better test-debug experience, which is fine for ***development only***. ### Scaling Considerations none ### Documentation Considerations See Documentation Considerations of https://github.com/endojs/endo/pull/2355 ### Testing Considerations This PR effectively serves as a test both for https://github.com/endojs/endo/pull/2355 and for the corresponding patch in this PR. ### Upgrade Considerations none. --- .../test/snapshots/xsnap-store.test.js.md | 8 +- .../test/snapshots/xsnap-store.test.js.snap | Bin 503 -> 501 bytes patches/@endo+lockdown+1.0.7.patch | 13 ++ patches/ses+1.5.0.patch | 167 ++++++++++++++++++ 4 files changed, 184 insertions(+), 4 deletions(-) create mode 100644 patches/@endo+lockdown+1.0.7.patch create mode 100644 patches/ses+1.5.0.patch diff --git a/packages/SwingSet/test/snapshots/xsnap-store.test.js.md b/packages/SwingSet/test/snapshots/xsnap-store.test.js.md index db827766e84..52a1b44b201 100644 --- a/packages/SwingSet/test/snapshots/xsnap-store.test.js.md +++ b/packages/SwingSet/test/snapshots/xsnap-store.test.js.md @@ -20,8 +20,8 @@ Generated by [AVA](https://avajs.dev). { compressSeconds: 0, dbSaveSeconds: 0, - hash: 'bd10d954d29157e1ab9a068b6bdeb8bb7157489e11dc71b2961f4cab9a7d135c', - uncompressedSize: 827051, + hash: '30d2d6ce649f1f3b9d5dd48404ee32c4ff0cc9935b7aa4498c0bcd218eedca71', + uncompressedSize: 827651, } > after use of harden() - sensitive to SES-shim, XS, and supervisor @@ -29,6 +29,6 @@ Generated by [AVA](https://avajs.dev). { compressSeconds: 0, dbSaveSeconds: 0, - hash: 'ba3a51da404ab14b6366df8966df683bc4f8ffd99f2dc12aabfa3b7728f39952', - uncompressedSize: 827211, + hash: '46d4988aa5eed7354684b76362fa8b6049e4467139e064a5b73fa8e80bed26a7', + uncompressedSize: 827811, } diff --git a/packages/SwingSet/test/snapshots/xsnap-store.test.js.snap b/packages/SwingSet/test/snapshots/xsnap-store.test.js.snap index 1c7da1cef30459e48cf574ae8fd528cc684c0f08..566a5d8b8112bafeab8b9c08dbbff55c0c6d638a 100644 GIT binary patch literal 501 zcmVRzVD0cO;Qk${Bz>(mZsT%ni)BlEl2A3L} zTWj^>9v_Pc00000000Bk(y?yaFc1ddW9+0E8pKBW032I1E@X)ksZ8n6xr3H;Jo1h; zTUjzh+6cN8-MaM=l0Hfwrms;rMm%H)(lK0)Jp4EuKJdodws&9S_ghXOwS4{Vs*!JH z^P!Qk!&O}LDM}nzwte?`mUvi7;%y0Ch$*5BuD}ii~oq99OFLb6?n+N+M-crkIP| zDep7q>bfYcMpUXcF0+pExnU-Udj3SP(ap;p*!2ba#KYZZw0)oCOh)cvNMXY=_1pd| zF2eF$UNz?uJCbp|;$ahFKRh>ERgtQVK`fa`7p1CF8ne2Yc%@FKLfPA|Cm1CP>#8!>z9{} zcp;kCjYyqarA42Ua07|9@7~Wc4NHNvIbs)6jy~>FUv3axcJKIf#2&HD;*a^^Ib-aC zF%}qOUm0V67-OgWXv&Y?!Uyu1^Zj))nJ|9L_?Yn%cFGT)@)6^QQ-0$69#&*cT&d)jk}_MX2uJb7Q4p<5S3sGB0wbND_?4@0@w;P1u~{IMbPM#3l8iY6wRqJ z4zW^LgOUDD^U1o~4NBO=Pqdw3^!wWZwGOCV#V+RP+CA)p14B7STPZB8tTagn*3D}J zH6Vcj(6Z9XSdtRGmQdJQ2IV&?jk2un3-;^#lP%cwxeJ+w+s_DnpT$fh>Qc<{ibU== z^;ufP<(ar_&V=hwr1gr1tC;%X!P&afRT7<=D+f|RUDh=QV>f$kO7JR}5YXCCpqGVn t5L^jb7bcX}R>eJS-~8CwzS`QZ6Nx^Eg&UB%XV3qK_%C2qyz@H*002N)^y~lt diff --git a/patches/@endo+lockdown+1.0.7.patch b/patches/@endo+lockdown+1.0.7.patch new file mode 100644 index 00000000000..819e4c9dcac --- /dev/null +++ b/patches/@endo+lockdown+1.0.7.patch @@ -0,0 +1,13 @@ +diff --git a/node_modules/@endo/lockdown/commit-debug.js b/node_modules/@endo/lockdown/commit-debug.js +index 9595aa0..03e02a2 100644 +--- a/node_modules/@endo/lockdown/commit-debug.js ++++ b/node_modules/@endo/lockdown/commit-debug.js +@@ -21,7 +21,7 @@ lockdown({ + // NOTE TO REVIEWERS: If you see the following line commented out, + // this may be a development accident that should be fixed before merging. + // +- errorTaming: 'unsafe', ++ errorTaming: 'unsafe-debug', + + // The default `{stackFiltering: 'concise'}` setting usually makes for a + // better debugging experience, by severely reducing the noisy distractions diff --git a/patches/ses+1.5.0.patch b/patches/ses+1.5.0.patch new file mode 100644 index 00000000000..b2c37f96972 --- /dev/null +++ b/patches/ses+1.5.0.patch @@ -0,0 +1,167 @@ +diff --git a/node_modules/ses/src/error/tame-error-constructor.js b/node_modules/ses/src/error/tame-error-constructor.js +index 2788c42..db59952 100644 +--- a/node_modules/ses/src/error/tame-error-constructor.js ++++ b/node_modules/ses/src/error/tame-error-constructor.js +@@ -7,6 +7,7 @@ import { + setPrototypeOf, + getOwnPropertyDescriptor, + defineProperty, ++ getOwnPropertyDescriptors, + } from '../commons.js'; + import { NativeErrors } from '../permits.js'; + import { tameV8ErrorConstructor } from './tame-v8-error-constructor.js'; +@@ -29,12 +30,17 @@ const tamedMethods = { + return ''; + }, + }; ++let initialGetStackString = tamedMethods.getStackString; + + export default function tameErrorConstructor( + errorTaming = 'safe', + stackFiltering = 'concise', + ) { +- if (errorTaming !== 'safe' && errorTaming !== 'unsafe') { ++ if ( ++ errorTaming !== 'safe' && ++ errorTaming !== 'unsafe' && ++ errorTaming !== 'unsafe-debug' ++ ) { + throw TypeError(`unrecognized errorTaming ${errorTaming}`); + } + if (stackFiltering !== 'concise' && stackFiltering !== 'verbose') { +@@ -42,9 +48,9 @@ export default function tameErrorConstructor( + } + const ErrorPrototype = FERAL_ERROR.prototype; + +- const platform = +- typeof FERAL_ERROR.captureStackTrace === 'function' ? 'v8' : 'unknown'; + const { captureStackTrace: originalCaptureStackTrace } = FERAL_ERROR; ++ const platform = ++ typeof originalCaptureStackTrace === 'function' ? 'v8' : 'unknown'; + + const makeErrorConstructor = (_ = {}) => { + // eslint-disable-next-line no-shadow +@@ -122,6 +128,45 @@ export default function tameErrorConstructor( + }, + }); + ++ if (errorTaming === 'unsafe-debug' && platform === 'v8') { ++ // This case is a kludge to work around ++ // https://github.com/endojs/endo/issues/1798 ++ // https://github.com/endojs/endo/issues/2348 ++ // https://github.com/Agoric/agoric-sdk/issues/8662 ++ ++ defineProperties(InitialError, { ++ prepareStackTrace: { ++ get() { ++ return FERAL_ERROR.prepareStackTrace; ++ }, ++ set(newPrepareStackTrace) { ++ FERAL_ERROR.prepareStackTrace = newPrepareStackTrace; ++ }, ++ enumerable: false, ++ configurable: true, ++ }, ++ captureStackTrace: { ++ value: FERAL_ERROR.captureStackTrace, ++ writable: true, ++ enumerable: false, ++ configurable: true, ++ }, ++ }); ++ ++ const descs = getOwnPropertyDescriptors(InitialError); ++ defineProperties(SharedError, { ++ stackTraceLimit: descs.stackTraceLimit, ++ prepareStackTrace: descs.prepareStackTrace, ++ captureStackTrace: descs.captureStackTrace, ++ }); ++ ++ return { ++ '%InitialGetStackString%': initialGetStackString, ++ '%InitialError%': InitialError, ++ '%SharedError%': SharedError, ++ }; ++ } ++ + // The default SharedError much be completely powerless even on v8, + // so the lenient `stackTraceLimit` accessor does nothing on all + // platforms. +@@ -171,7 +216,6 @@ export default function tameErrorConstructor( + }); + } + +- let initialGetStackString = tamedMethods.getStackString; + if (platform === 'v8') { + initialGetStackString = tameV8ErrorConstructor( + FERAL_ERROR, +@@ -179,7 +223,7 @@ export default function tameErrorConstructor( + errorTaming, + stackFiltering, + ); +- } else if (errorTaming === 'unsafe') { ++ } else if (errorTaming === 'unsafe' || errorTaming === 'unsafe-debug') { + // v8 has too much magic around their 'stack' own property for it to + // coexist cleanly with this accessor. So only install it on non-v8 + +diff --git a/node_modules/ses/src/lockdown.js b/node_modules/ses/src/lockdown.js +index 107b5d0..dd709e5 100644 +--- a/node_modules/ses/src/lockdown.js ++++ b/node_modules/ses/src/lockdown.js +@@ -58,7 +58,7 @@ import { tameFauxDataProperties } from './tame-faux-data-properties.js'; + + /** @import {LockdownOptions} from '../types.js' */ + +-const { Fail, details: d, quote: q } = assert; ++const { Fail, details: X, quote: q } = assert; + + /** @type {Error=} */ + let priorRepairIntrinsics; +@@ -200,7 +200,7 @@ export const repairIntrinsics = (options = {}) => { + priorRepairIntrinsics === undefined || + // eslint-disable-next-line @endo/no-polymorphic-call + assert.fail( +- d`Already locked down at ${priorRepairIntrinsics} (SES_ALREADY_LOCKED_DOWN)`, ++ X`Already locked down at ${priorRepairIntrinsics} (SES_ALREADY_LOCKED_DOWN)`, + TypeError, + ); + // See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_ALREADY_LOCKED_DOWN.md +@@ -298,7 +298,7 @@ export const repairIntrinsics = (options = {}) => { + * @type {((error: any) => string | undefined) | undefined} + */ + let optGetStackString; +- if (errorTaming !== 'unsafe') { ++ if (errorTaming === 'safe') { + optGetStackString = intrinsics['%InitialGetStackString%']; + } + const consoleRecord = tameConsole( +@@ -318,13 +318,17 @@ export const repairIntrinsics = (options = {}) => { + // There doesn't seem to be a cleaner way to reach it. + hostIntrinsics.SafeMap = getPrototypeOf( + // eslint-disable-next-line no-underscore-dangle +- /** @type {any} */ (consoleRecord.console)._times, ++ /** @type {any} */(consoleRecord.console)._times, + ); + } + + // @ts-ignore assert is absent on globalThis type def. +- if (errorTaming === 'unsafe' && globalThis.assert === assert) { +- // If errorTaming is 'unsafe' we replace the global assert with ++ if ( ++ (errorTaming === 'unsafe' || errorTaming === 'unsafe-debug') && ++ globalThis.assert === assert ++ ) { ++ // If errorTaming is 'unsafe' or 'unsafe-debug' we replace the ++ // global assert with + // one whose `details` template literal tag does not redact + // unmarked substitution values. IOW, it blabs information that + // was supposed to be secret from callers, as an aid to debugging +@@ -391,7 +395,7 @@ export const repairIntrinsics = (options = {}) => { + priorHardenIntrinsics === undefined || + // eslint-disable-next-line @endo/no-polymorphic-call + assert.fail( +- d`Already locked down at ${priorHardenIntrinsics} (SES_ALREADY_LOCKED_DOWN)`, ++ X`Already locked down at ${priorHardenIntrinsics} (SES_ALREADY_LOCKED_DOWN)`, + TypeError, + ); + // See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_ALREADY_LOCKED_DOWN.md