fix(lockdown): change commit-debug.js to unsafe-debug #2359
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BREAKING: Importing
@endo/lockdown/commit-debug.js
will now endanger integrity as well, in order to get better line numbers.@endo/lockdown/commit-debug.js
should normally be used only for development/debugging/testing scenarios, where it is not potentially in contact with any genuinely malicious code. For those cases, there is no BREAKING change here. But we're flagging a potentially breaking change in case anyone is importing@endo/lockdown/commit-debug.js
in scenarios where potentially malicious code is a concern. Such uses should be revised to avoid settingerrorTaming: 'unsafe-debug'
.Closes: #XXXX
Refs: #2355 Agoric/agoric-sdk#9711
Description
In #2355 , we introduced a new
errorTaming
setting,'unsafe-debug'
, to sacrifice more security for a better debugging experience. In many ways it would have been more convenient to modify'unsafe'
to do this, rather than introduce a new setting. But we did not because the'unsafe'
setting was already documented as threatening only confidentiality, not integrity. Many production scenarios don't need the'safe'
level of confidentiality defense, and so have been usingerrorTaming: 'unsafe'
in production, as shown below. Thus, a less-safe form had to be a new mode, so the loss of safety was purely opt-in.However, for uses that were clearly development-only uses, especially those that are explicitly about testing/debugging, they often inherit these lockdown settings from a long chain of imports bottoming out in
Since this import is explicitly named something-debug, and since none of the production uses of
errorTaming: 'unsafe'
that we are aware of inherit it fromendo/lockdown/commit-debug.js
, it seems we could recover the convenience for those testing/debugging uses by modifying this file in place, with an acceptable security risk. This PR does so.Some existing uses of
errorTaming: 'unsafe'
Some of these are in production code
https://github.com/LavaMoat/LavaMoat/blob/1cb057b281d46d2564872f53c2769bf4c4cb0ba5/packages/webpack/src/plugin.js#L54
https://github.com/LavaMoat/LavaMoat/blob/1cb057b281d46d2564872f53c2769bf4c4cb0ba5/packages/core/src/kernelTemplate.js#L60
https://github.com/LavaMoat/LavaMoat/blob/1cb057b281d46d2564872f53c2769bf4c4cb0ba5/packages/webpack/src/plugin.js#L54
https://github.com/MetaMask/metamask-extension/blob/7b3450a294a2fe5751726a9c33d6fa0b564f03dd/app/scripts/lockdown-run.js#L6
https://github.com/MetaMask/snaps/blob/0a265dcf9de73a16a7b50cc47681fe6da179383a/packages/snaps-utils/src/eval-worker.ts#L14
https://github.com/MetaMask/snaps/blob/0a265dcf9de73a16a7b50cc47681fe6da179383a/packages/snaps-execution-environments/src/common/lockdown/lockdown.ts#L15
From the psm.inter.trade console
![psm inter trade-debug-console](https://private-user-images.githubusercontent.com/273868/349297417-cd01501c-9a0c-43d6-a529-67fd1ca3ae43.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkyMjgxMTQsIm5iZiI6MTczOTIyNzgxNCwicGF0aCI6Ii8yNzM4NjgvMzQ5Mjk3NDE3LWNkMDE1MDFjLTlhMGMtNDNkNi1hNTI5LTY3ZmQxY2EzYWU0My5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwMjEwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDIxMFQyMjUwMTRaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT0xODdhZDQyZThhZThiZjQ2M2JmNGZkNWQyYmIwNDc2MDY0NWQwZDg3N2YxZDAxZmY2MmQ5NTQwMjY4YWZmMWI5JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.L-ME2LblzP1OAKMq30q_5_ufHE5ta4ZORYMVyZ-ir68)
Security Considerations
As explained above. If there are any production uses that get their lockdown settings by importing
@endo/lockdown/commit-debug.js
, they will experience a silent loss of security when they upgrade to depend on this more recent version. We are not aware of any such cases. Because of the explicit "debug" in the name, we expect such cases to be rare. But we have no way to confirm they do not exist.Reviewers, please let me know if you'd like me to change from a
fix:
to afix!
because of this silent loss of security on upgrading the dependency.Scaling Considerations
none
Documentation Considerations
#2355 documents the differences between
'unsafe'
and'unsafe-debug'
for both security and functionality.endo/lockdown/commit-debug.js
has changed in place, explaining the difference.Testing Considerations
Most of our tests inherit their
lockdown
settings from importing@endo/lockdown/commit-debug.js
, so these test would experience both the security loss --- which should not matter for testing --- and functionality gain of seeing correct line-numbers into transpiled code, such as TypeScript sources.Compatibility Considerations
For development purposes, practically none, since the loss of security in question is unlikely to matter. The improvement in line numbers will help developers looking at stack traces, but is unlikely to affect any code.
This change is not compatible with production code that imports
@endo/lockdown/commit-debug.js
. But as the name indicates, production code should not have been importing this file anyway.Upgrade Considerations
No upgrade issues
Include
*BREAKING*:
in the commit message with migration instructions for any breaking change.Update
NEWS.md
for user-facing changes.