endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
804 stars 71 forks source link

refactor(ses): refactor console init and test cleanup #1941

Closed erights closed 8 months ago

erights commented 8 months ago

closes: #XXXX refs: #1345 #1942

Description

A flaw in tame-console.js made it hard to use throws-and-logs.js to test the console output that happens during lockdown. The flaw is that tame-console.js read the value of the global console variable at initialization, rather than when tameConsole() is called. This PR moves that initialization code into tameConsole, which is highly unlikely to break any working code.

Using that fix, this PR also tests the current behavior that #1345 complains about. A later PR #1942 will fix that problem, building on this PR so that the difference is itself tested.

Security Considerations

Prior to this PR, tame-console.js used console and print as ambient globals without a /* global ... */ annotation. For console this is not surprising, as we generally assume this is ambiently available. But for print it is surprising. Do we have a bug in our lint diagnostic framework? Attn @kriskowal

In any case, this PR changes those specifically to globalThis.console and globalThis.print, where that globalThis is itself obtained from commons.js.

Moving the sampling of these globals into tameConsole() enables them to be changed before tameConsole() samples them, as we now intend. This should have zero security implications as these will still be sampled before lockdown() returns.

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

Moving the sampling of console this way should enable other uses of throws-and-logs.js for testing console output that would not work before this PR.

Upgrade Considerations

none.