From 4a0d5d09f119d0494fca8f5bca1f6bb85861c9ed Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Wed, 24 Mar 2021 21:53:44 -0700 Subject: [PATCH] fix: Adjust to bug 636 --- packages/ses-ava/test/test-raw-ava-reject.js | 9 ++++--- packages/ses-ava/test/test-raw-ava-throw.js | 9 ++++--- packages/ses/lockdown-options.md | 25 +++++++++++++++++-- .../test-tame-console-unfilteredError.js | 4 +-- ...est-tame-console-unsafe-unfilteredError.js | 4 +-- ...sole-unsafe-unsafeError-unfilteredError.js | 6 +++-- .../test-tame-console-unsafe-unsafeError.js | 11 +++++--- .../test/error/test-tame-console-unsafe.js | 4 +-- .../error/test-tame-console-unsafeError.js | 5 ++-- packages/ses/test/error/test-tame-console.js | 4 +-- 10 files changed, 58 insertions(+), 23 deletions(-) diff --git a/packages/ses-ava/test/test-raw-ava-reject.js b/packages/ses-ava/test/test-raw-ava-reject.js index 990f0df613..474999642b 100644 --- a/packages/ses-ava/test/test-raw-ava-reject.js +++ b/packages/ses-ava/test/test-raw-ava-reject.js @@ -29,15 +29,18 @@ test('raw ava reject console output', t => { Uncommenting the test code above should produce something like the following. This output is all from ava. The stack-like display comes from ava's direct use of the `error.stack` property. Ava bypasses the normal `console`. -For the error message, ava has no access to the non-disclosed -`'NOTICE ME'`, only the redacted `'(a string)'. +For the error message, ava has access only to the `message` string carried +by the error instance, which would normally be redacted to +`'msg (a string)'`. But `errorTaming: 'unsafe'` suppresses that redaction along +with suppressing the redaction of the stack, so the console blabs +`'msg "NOTICE ME"'` instead. ``` raw ava reject console output Rejected promise returned by test. Reason: TypeError { - message: 'msg (a string)', + message: 'msg "NOTICE ME"', } › makeError (file:///Users/markmiller/src/ongithub/agoric/SES-shim/packages/ses/src/error/assert.js:141:17) diff --git a/packages/ses-ava/test/test-raw-ava-throw.js b/packages/ses-ava/test/test-raw-ava-throw.js index b3ddc71432..1ff778c3da 100644 --- a/packages/ses-ava/test/test-raw-ava-throw.js +++ b/packages/ses-ava/test/test-raw-ava-throw.js @@ -24,15 +24,18 @@ test('raw ava throw console output', t => { Uncommenting the test code above should produce something like the following. This output is all from ava. The stack-like display comes from ava's direct use of the `error.stack` property. Ava bypasses the normal `console`. -For the error message, ava has no access to the non-disclosed -`'NOTICE ME'`, only the redacted `'(a string)'. +For the error message, ava has access only to the `message` string carried +by the error instance, which would normally be redacted to +`'msg (a string)'`. But `errorTaming: 'unsafe'` suppresses that redaction along +with suppressing the redaction of the stack, so the console blabs +`'msg "NOTICE ME"'` instead. ``` raw ava throw console output Error thrown in test: TypeError { - message: 'msg (a string)', + message: 'msg "NOTICE ME"', } › makeError (file:///Users/alice/agoric/SES-shim/packages/ses/src/error/assert.js:141:17) diff --git a/packages/ses/lockdown-options.md b/packages/ses/lockdown-options.md index 0aaa3da11d..18fe70e182 100644 --- a/packages/ses/lockdown-options.md +++ b/packages/ses/lockdown-options.md @@ -135,8 +135,12 @@ lockdown(); // consoleTaming defaults to 'safe' lockdown({ consoleTaming: 'safe' }); // Wrap start console to show deep stacks // vs lockdown({ consoleTaming: 'unsafe' }); // Leave original start console in place +// or +lockdown({ + consoleTaming: 'unsafe', // Leave original start console in place + overrideTaming: 'min', // Until https://github.com/endojs/endo/issues/636 +}); ``` - The `consoleTaming: 'unsafe'` setting leaves the original console in place. The `assert` package and error objects will continue to work, but the `console` logging output will not show any of this extra information. @@ -149,6 +153,12 @@ methods violate ocap security. Until we know otherwise, we should assume these are unsafe. Such a raw `console` object should only be handled by very trustworthy code. +Until the bug +[Node console gets confused if .constructor is an accessor (#636)](https://github.com/endojs/endo/issues/636) +is fixed, if you use the `consoleTaming: 'unsafe'` setting and might be running +with the Node `console`, we advise you to also set `overrideTaming: 'min'` so +that no builtin `constructor` properties are turned into accessors. + Examples from [test-deep-send.js](https://github.com/Agoric/agoric-sdk/blob/master/packages/eventual-send/test/test-deep-send.js) of the eventual-send shim: @@ -262,11 +272,22 @@ acts like assert(false, X`literal part ${q(secretData)} with ${q(publicData)}.`); ``` +The `lockdown({ errorTaming: 'unsafe' })` call has this effect by replacing +the global `assert` object with one whose `assert.details` does not redact. +So be sure to sample `assert` and `assert.details` only after such a call to +lockdown: + +```js +lockdown({ errorTaming: 'unsafe' }); + +// Grab `details` only after lockdown +const { details: X, quote: q } = assert; +``` + Like with the stack, the SES shim `console` object always shows the unredacted detailed error message independent of the setting of `errorTaming`. - ## `stackFiltering` Options **Background**: The error stacks shown by many JavaScript engines are diff --git a/packages/ses/test/error/test-tame-console-unfilteredError.js b/packages/ses/test/error/test-tame-console-unfilteredError.js index de29989a21..d2b971d03f 100644 --- a/packages/ses/test/error/test-tame-console-unfilteredError.js +++ b/packages/ses/test/error/test-tame-console-unfilteredError.js @@ -2,12 +2,12 @@ import test from 'ava'; import '../../ses.js'; import { getPrototypeOf } from '../../src/commons.js'; -const { details: d } = assert; - const originalConsole = console; lockdown({ stackFiltering: 'verbose' }); +const { details: d } = assert; + test('console', t => { t.plan(3); diff --git a/packages/ses/test/error/test-tame-console-unsafe-unfilteredError.js b/packages/ses/test/error/test-tame-console-unsafe-unfilteredError.js index 6a336522a0..0c1a78e028 100644 --- a/packages/ses/test/error/test-tame-console-unsafe-unfilteredError.js +++ b/packages/ses/test/error/test-tame-console-unsafe-unfilteredError.js @@ -2,12 +2,12 @@ import test from 'ava'; import '../../ses.js'; import { getPrototypeOf } from '../../src/commons.js'; -const { details: d } = assert; - const originalConsole = console; lockdown({ consoleTaming: 'unsafe', stackFiltering: 'verbose' }); +const { details: d } = assert; + test('console', t => { t.plan(3); diff --git a/packages/ses/test/error/test-tame-console-unsafe-unsafeError-unfilteredError.js b/packages/ses/test/error/test-tame-console-unsafe-unsafeError-unfilteredError.js index 1900ee364b..d929de4425 100644 --- a/packages/ses/test/error/test-tame-console-unsafe-unsafeError-unfilteredError.js +++ b/packages/ses/test/error/test-tame-console-unsafe-unsafeError-unfilteredError.js @@ -2,16 +2,18 @@ import test from 'ava'; import '../../ses.js'; import { getPrototypeOf } from '../../src/commons.js'; -const { details: d } = assert; - const originalConsole = console; lockdown({ consoleTaming: 'unsafe', errorTaming: 'unsafe', stackFiltering: 'verbose', + overrideTaming: 'min', }); +// Grab `details` only after lockdown +const { details: d } = assert; + test('console', t => { t.plan(3); diff --git a/packages/ses/test/error/test-tame-console-unsafe-unsafeError.js b/packages/ses/test/error/test-tame-console-unsafe-unsafeError.js index 2763845d51..a5afee752c 100644 --- a/packages/ses/test/error/test-tame-console-unsafe-unsafeError.js +++ b/packages/ses/test/error/test-tame-console-unsafe-unsafeError.js @@ -2,11 +2,16 @@ import test from 'ava'; import '../../ses.js'; import { getPrototypeOf } from '../../src/commons.js'; -const { details: d } = assert; - const originalConsole = console; -lockdown({ consoleTaming: 'unsafe', errorTaming: 'unsafe' }); +lockdown({ + consoleTaming: 'unsafe', + errorTaming: 'unsafe', + overrideTaming: 'min', +}); + +// Grab `details` only after lockdown +const { details: d } = assert; test('console', t => { t.plan(3); diff --git a/packages/ses/test/error/test-tame-console-unsafe.js b/packages/ses/test/error/test-tame-console-unsafe.js index 7daaad4826..d5fded9b08 100644 --- a/packages/ses/test/error/test-tame-console-unsafe.js +++ b/packages/ses/test/error/test-tame-console-unsafe.js @@ -2,12 +2,12 @@ import test from 'ava'; import '../../ses.js'; import { getPrototypeOf } from '../../src/commons.js'; -const { details: d } = assert; - const originalConsole = console; lockdown({ consoleTaming: 'unsafe' }); +const { details: d } = assert; + test('console', t => { t.plan(3); diff --git a/packages/ses/test/error/test-tame-console-unsafeError.js b/packages/ses/test/error/test-tame-console-unsafeError.js index c757bb9c09..6d43bdaf4a 100644 --- a/packages/ses/test/error/test-tame-console-unsafeError.js +++ b/packages/ses/test/error/test-tame-console-unsafeError.js @@ -2,12 +2,13 @@ import test from 'ava'; import '../../ses.js'; import { getPrototypeOf } from '../../src/commons.js'; -const { details: d } = assert; - const originalConsole = console; lockdown({ errorTaming: 'unsafe' }); +// Grab `details` only after lockdown +const { details: d } = assert; + test('console', t => { t.plan(3); diff --git a/packages/ses/test/error/test-tame-console.js b/packages/ses/test/error/test-tame-console.js index c81c86620b..ea703eedac 100644 --- a/packages/ses/test/error/test-tame-console.js +++ b/packages/ses/test/error/test-tame-console.js @@ -2,12 +2,12 @@ import test from 'ava'; import '../../ses.js'; import { getPrototypeOf } from '../../src/commons.js'; -const { details: d } = assert; - const originalConsole = console; lockdown(); +const { details: d } = assert; + test('console', t => { t.plan(3);