Skip to content

Commit

Permalink
fix: Adjust to bug 636
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Mar 25, 2021
1 parent 145595c commit 4a0d5d0
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 23 deletions.
9 changes: 6 additions & 3 deletions packages/ses-ava/test/test-raw-ava-reject.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 6 additions & 3 deletions packages/ses-ava/test/test-raw-ava-throw.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 23 additions & 2 deletions packages/ses/lockdown-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/ses/test/error/test-tame-console-unfilteredError.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
11 changes: 8 additions & 3 deletions packages/ses/test/error/test-tame-console-unsafe-unsafeError.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions packages/ses/test/error/test-tame-console-unsafe.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
5 changes: 3 additions & 2 deletions packages/ses/test/error/test-tame-console-unsafeError.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions packages/ses/test/error/test-tame-console.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 4a0d5d0

Please sign in to comment.