endojs / endo

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

feat(compartment-mapper): Relax package discovery #2642

Closed kriskowal closed 10 hours ago

kriskowal commented 5 days ago

Closes: #2636

Description

The compartment mapper is currently strict about the interpretation of peerDependencies and peerDependenciesMeta.optional as opposed to optionalDependencies and is technically correct, but that correctness and understanding is not evenly distributed among tools in the ecosystem. If the Compartment Mapper instead tolerates the physical absence of expected packages, it defers an error that would be experienced under mapNodeModules to link/load since that might attempt to load a module from the missing package. This is not a particularly observable difference in behavior, but the error message will be slightly less informative.

In this change, we relax the behavior by default and provide a strict option to provide the more informative error and tolerate fewer ecosystem defficiencies.

Security Considerations

None.

Scaling Considerations

None.

Documentation Considerations

News updated.

We do not yet publish API documentation for Compartment Mapper, but the necessary annotations are present when these devices are more public.

Testing Considerations

Includes positive and negative tests for the strict behavior.

Compatibility Considerations

The nature of some errors during bundling may change. Bundling errors are not observed on Agoric’s chain, so that sensitivity should not cause problems.

Upgrade Considerations

None.

kriskowal commented 10 hours ago

Does this have any effect on packages that are optionally required but not otherwise mentioned in package.json?

An example from typescript:

try {
  require('source-map-support')
} catch {}

...where source-map-support is a dev dependency of typescript. So this require would not throw if a) someone has a local working copy of Microsoft/typescript or b) someone already has source-map-support in their node_modules (as they often do).

No, this just tolerates the absence in .../node_modules of packages that are present in package.json, not accommodate the presence in .../node_modules of packages that are absent in package.json

kriskowal commented 10 hours ago

@boneskull We could conceivably add an even sloppier mapNodeModules that recursively lists .../node_modules/{,@/}* and disregards the *dependencies in package.json entirely. With a link hook, we might even be able to make discovery of .../node_modules lazy. In any case, not in scope for this PR, but so you know: that is a possibility.