endojs / endo

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

support dynamic requires #2310

Open boneskull opened 3 weeks ago

boneskull commented 3 weeks ago

Description

Security Considerations

Dynamically requiring an exit module (e.g., a Node.js builtin) requires a user-defined hook, which has the same security considerations as a user-defined exit module hook.

Swapping out a dependency (source-map-js for source-map) incurs risk.

Scaling Considerations

n/a

Documentation Considerations

Should be announced as a user-facing feature

Testing Considerations

I've added some fixtures and tested around the conditionals I've added, but am open to any suggestions for additional coverage.

Compatibility Considerations

This increases ecosystem compatibility considerably; use of dynamic require in the ecosystem is not rare.

For example, most packages which ship a native module will be using dynamic require, because the filepath of the build artifact is dependent upon the platform and architecture.

Upgrade Considerations

Dynamic imports cannot be used without providing a readSync() and isAbsolute() in the readPowers parameter. Both readSync() and isAbsolute() are now generated by makeReadPowersSloppy(), which means that any consumer using this method who previously expected a dynamic require to fail will now receive a different error (presumably due to the missing policy item).

To avoid this, I could create a separate function to provide a ReadPowers including readSync() and isAbsolute(), instead of changing makeReadPowersSloppy; please advise.

Otherwise, everything else should be backwards-compatible, as long as source-map-js does as it says on the tin.

Users of @endo/evasive-transform may note that native modules are neither downloaded/compiled (due to the switch from source-map to source-map-js).

boneskull commented 2 weeks ago

If anyone can point me in the direction of why the tests are failing, that'd be helpful; otherwise I'll just plug at it. Plugged.

boneskull commented 1 week ago

Going to extract the changes to @endo/evasive-transform into a separate PR.

boneskull commented 1 week ago

Ref: https://github.com/endojs/endo/pull/2332

cc @kriskowal

boneskull commented 1 week ago

Once #2332 is merged, I can rebase this onto master, which should eliminate the commit from this PR's history. So let's sit on it until then.