endojs / endo

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

fix(bundle-source): Add a nonce to bundle-source scratch files #1762

Closed kriskowal closed 1 year ago

kriskowal commented 1 year ago

Description

The bundle-source tool writes out to temporary scratch files to avoid race conditions between concurrent runs. The scratch file currently includes the PID of the bundle-source process so they do not collide and to make it easy to determine whether the scratch file can be deleted if bundle-source is interrupted before renaming the scratch file.

This does not account for use of the bundle cache as a library, where multiple bundle caches may exist concurrently in a single process, making the PID insufficient to disambiguate. Also, the PID argument is optional, so a nonce can disambiguate if a PID is not available. The new scratch file has both the PID and nonce, like x.100.1000.json or x.no-pid.2000.json.

Security Considerations

Not to my knowledge.

Scaling Considerations

This change includes concurrency tests for both the tool and library usage of the bundler.

Documentation Considerations

This change does not alter the usage of the bundle-source command. NEWS.md includes a suggestion to use the new pid argument of the cache maker, which should otherwise be discovered on first use. We do not otherwise currently have documentation for the cache library in README.md, so discoverability is limited. This is probably fine for now.

Testing Considerations

This adds testing for the cache command and library usage.

Upgrade Considerations

This should not impact upgrades.

kriskowal commented 1 year ago

@dckc FYI, cache maker options pid and nonce exist and have reasonable defaults.

michaelfig commented 1 year ago

Can you please also adjust the behaviour of Agoric/agoric-sdk#8314 to provide process.pid instead of crypto.randomInt to the bundler (letting the bundler generate the nonce)?