-
Notifications
You must be signed in to change notification settings - Fork 206
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(ses): temp patch in Endo 2355,2359 line-number workaround (#9711)
closes: #8662 refs: endojs/endo#2355 endojs/endo#2348 ## Description endojs/endo#2355 makes it possible to see accurate line numbers into TypeScript Ava tests run under Node, which would fix #8662 as of the next endo-release-agoric-sdk-sync cycle. To get this benefit before then, this PR turns endojs/endo#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 endojs/endo#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 endojs/endo#2355 ### Testing Considerations This PR effectively serves as a test both for endojs/endo#2355 and for the corresponding patch in this PR. ### Upgrade Considerations none.
- Loading branch information
Showing
4 changed files
with
184 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |