Skip to content

Commit d99f0a4

Browse files
committed
fix(compartment-mapper): mapNodeModules should not crawl ESM modules from CJS entrypoints
This changes the logic in mapNodeModules() so that when it reads the `package.json` of the entrypoint, it checks for `type === 'module'`. If this is set, then `import` is added to the `conditions`. If it is not set, then `require` is added to the conditions. The conditions always contain `default`; this has not changed. This prevents CJS modules from referencing modules in the `CompartmentMapDescriptor` which they would not otherwise load (and in fact _cannot_ load for some value of "_cannot_").
1 parent 60190cd commit d99f0a4

File tree

10 files changed

+92
-2
lines changed

10 files changed

+92
-2
lines changed

packages/compartment-mapper/NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ User-visible changes to `@endo/compartment-mapper`:
1010
occur _if and only if_ dynamic requires are enabled _and_ a policy is
1111
provided.
1212
- Improved error messaging for policy enforcement failures.
13+
- Fixed a bug where `mapNodeModules()` would traverse ESM exports even when
14+
provided a CJS entrypoint.
1315

1416
# v1.6.2 (2025-06-17)
1517

packages/compartment-mapper/src/node-modules.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,6 @@ const graphPackages = async (
704704
const packageDescriptor = allegedPackageDescriptor;
705705

706706
conditions = new Set(conditions || []);
707-
conditions.add('import');
708707
conditions.add('default');
709708
conditions.add('endo');
710709

@@ -1098,14 +1097,21 @@ export const mapNodeModules = async (
10981097
packageDescriptorLocation,
10991098
moduleSpecifier,
11001099
} = await search(readPowers, moduleLocation, { log });
1101-
1100+
conditions = conditions ?? new Set();
1101+
11021102
const packageDescriptor = /** @type {typeof parseLocatedJson<unknown>} */ (
11031103
parseLocatedJson
11041104
)(packageDescriptorText, packageDescriptorLocation);
11051105

11061106
assertPackageDescriptor(packageDescriptor);
11071107
assertFileUrlString(packageLocation);
11081108

1109+
if (packageDescriptor.type === 'module') {
1110+
conditions.add('import');
1111+
} else {
1112+
conditions.add('require');
1113+
}
1114+
11091115
return compartmentMapForNodeModules(
11101116
readPowers,
11111117
packageLocation,

packages/compartment-mapper/test/fixtures-dual-module/node_modules/app-esm/index.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/compartment-mapper/test/fixtures-dual-module/node_modules/app-esm/package.json

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/compartment-mapper/test/fixtures-dual-module/node_modules/app/index.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/compartment-mapper/test/fixtures-dual-module/node_modules/app/package.json

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/compartment-mapper/test/fixtures-dual-module/node_modules/dual/index.cjs

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/compartment-mapper/test/fixtures-dual-module/node_modules/dual/index.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/compartment-mapper/test/fixtures-dual-module/node_modules/dual/package.json

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/compartment-mapper/test/map-node-modules.test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,3 +158,35 @@ test('mapNodeModules() should not consider peerDependenciesMeta without correspo
158158
}
159159
});
160160
}
161+
162+
test('mapNodeModules() should not traverse ESM exports for CJS entrypoints', async t => {
163+
const moduleLocation = new URL(
164+
'fixtures-dual-module/node_modules/app/index.js',
165+
import.meta.url,
166+
).href;
167+
const readPowers = makeReadPowers({ fs, url });
168+
const compartmentMap = await mapNodeModules(readPowers, moduleLocation);
169+
170+
t.is(
171+
Object.values(compartmentMap.compartments).find(
172+
compartment => compartment.name === 'dual',
173+
)?.modules?.dual?.module,
174+
'./index.cjs',
175+
);
176+
});
177+
178+
test('mapNodeModules() should prefer ESM exports for ESM entrypoints', async t => {
179+
const moduleLocation = new URL(
180+
'fixtures-dual-module/node_modules/app-esm/index.js',
181+
import.meta.url,
182+
).href;
183+
const readPowers = makeReadPowers({ fs, url });
184+
const compartmentMap = await mapNodeModules(readPowers, moduleLocation);
185+
186+
t.is(
187+
Object.values(compartmentMap.compartments).find(
188+
compartment => compartment.name === 'dual',
189+
)?.modules?.dual?.module,
190+
'./index.js',
191+
);
192+
});

0 commit comments

Comments
 (0)