diff --git a/packages/compartment-mapper/src/archive.js b/packages/compartment-mapper/src/archive.js index f0f32d8a16..8a68940d98 100644 --- a/packages/compartment-mapper/src/archive.js +++ b/packages/compartment-mapper/src/archive.js @@ -330,26 +330,26 @@ const digestLocation = async (powers, moduleLocation, options) => { const { compartments, - entry: { module: entryModuleSpecifier }, + entry: { module: entryModuleSpecifier, compartment: entryCompartmentName }, } = compartmentMap; /** @type {Sources} */ const sources = Object.create(null); - const compartmentExitModuleImportHook = exitModuleImportHookMaker({ + const consolidatedExitModuleImportHook = exitModuleImportHookMaker({ modules: exitModules, exitModuleImportHook, }); - const makeImportHook = makeImportHookMaker({ - readPowers: read, - baseLocation: packageLocation, + const makeImportHook = makeImportHookMaker(read, packageLocation, { sources, compartmentDescriptors: compartments, - exitModuleImportHook: compartmentExitModuleImportHook, archiveOnly: true, computeSha512, searchSuffixes, + entryCompartmentName, + entryModuleSpecifier, + exitModuleImportHook: consolidatedExitModuleImportHook, }); // Induce importHook to record all the necessary modules to import the given module specifier. const { compartment, attenuatorsCompartment } = link(compartmentMap, { diff --git a/packages/compartment-mapper/src/bundle.js b/packages/compartment-mapper/src/bundle.js index 9a9f20b164..8bf66ed4d4 100644 --- a/packages/compartment-mapper/src/bundle.js +++ b/packages/compartment-mapper/src/bundle.js @@ -45,14 +45,12 @@ const parserForLanguage = { /** * @param {Record} compartmentDescriptors * @param {Record} compartmentSources - * @param {Record} compartmentResolvers * @param {string} entryCompartmentName * @param {string} entryModuleSpecifier */ const sortedModules = ( compartmentDescriptors, compartmentSources, - compartmentResolvers, entryCompartmentName, entryModuleSpecifier, ) => { @@ -71,7 +69,6 @@ const sortedModules = ( } seen.add(key); - const resolve = compartmentResolvers[compartmentName]; const source = compartmentSources[compartmentName][moduleSpecifier]; if (source !== undefined) { const { record, parser, deferredError } = source; @@ -85,6 +82,9 @@ const sortedModules = ( /** @type {PrecompiledStaticModuleInterface} */ (record); const resolvedImports = Object.create(null); for (const importSpecifier of [...imports, ...reexports]) { + // If we ever support another module resolution algorithm, that + // should be indicated in the compartment descriptor by name and the + // corresponding behavior selected here. const resolvedSpecifier = resolve(importSpecifier, moduleSpecifier); resolvedImports[importSpecifier] = recur( compartmentName, @@ -165,8 +165,8 @@ function getBundlerKitForModule(module) { * @param {ModuleTransforms} [options.moduleTransforms] * @param {boolean} [options.dev] * @param {Set} [options.tags] - * @param {Array} [options.searchSuffixes] * @param {object} [options.commonDependencies] + * @param {Array} [options.searchSuffixes] * @returns {Promise} */ export const makeBundle = async (read, moduleLocation, options) => { @@ -206,15 +206,16 @@ export const makeBundle = async (read, moduleLocation, options) => { /** @type {Sources} */ const sources = Object.create(null); - const makeImportHook = makeImportHookMaker({ - readPowers: read, - baseLocation: packageLocation, + const makeImportHook = makeImportHookMaker(read, packageLocation, { sources, compartmentDescriptors: compartments, searchSuffixes, + entryCompartmentName, + entryModuleSpecifier, }); + // Induce importHook to record all the necessary modules to import the given module specifier. - const { compartment, resolvers } = link(compartmentMap, { + const { compartment } = link(compartmentMap, { resolve, makeImportHook, moduleTransforms, @@ -225,7 +226,6 @@ export const makeBundle = async (read, moduleLocation, options) => { const { modules, aliases } = sortedModules( compartmentMap.compartments, sources, - resolvers, entryCompartmentName, entryModuleSpecifier, ); diff --git a/packages/compartment-mapper/src/import-hook.js b/packages/compartment-mapper/src/import-hook.js index 88e05ff66f..f3de34e202 100644 --- a/packages/compartment-mapper/src/import-hook.js +++ b/packages/compartment-mapper/src/import-hook.js @@ -15,6 +15,7 @@ /** @typedef {import('./types.js').ExitModuleImportHook} ExitModuleImportHook */ import { attenuateModuleHook, enforceModulePolicy } from './policy.js'; +import { resolve } from './node-module-specifier.js'; import { unpackReadPowers } from './powers.js'; // q, as in quote, for quoting strings in error messages. @@ -63,7 +64,6 @@ const nodejsConventionSearchSuffixes = [ ]; /** - * * @param {object} params * @param {Record=} params.modules * @param {ExitModuleImportHook=} params.exitModuleImportHook @@ -96,35 +96,60 @@ export const exitModuleImportHookMaker = ({ }; /** + * @param {ReadFn|ReadPowers} readPowers + * @param {string} baseLocation * @param {object} options - * @param {ReadFn|ReadPowers} options.readPowers - * @param {string} options.baseLocation - * @param {Sources=} options.sources - * @param {Record=} options.compartmentDescriptors - * @param {ExitModuleImportHook=} options.exitModuleImportHook - * @param {boolean=} options.archiveOnly - * @param {HashFn=} options.computeSha512 - * @param {Array=} options.searchSuffixes - Suffixes to search if the unmodified specifier is not found. + * @param {Sources} [options.sources] + * @param {Record} [options.compartmentDescriptors] + * @param {boolean} [options.archiveOnly] + * @param {HashFn} [options.computeSha512] + * @param {Array} [options.searchSuffixes] - Suffixes to search if the + * unmodified specifier is not found. * Pass [] to emulate Node.js’s strict behavior. * The default handles Node.js’s CommonJS behavior. - * Unlike Node.js, the Compartment Mapper lifts CommonJS up, more like a bundler, - * and does not attempt to vary the behavior of resolution depending on the - * language of the importing module. + * Unlike Node.js, the Compartment Mapper lifts CommonJS up, more like a + * bundler, and does not attempt to vary the behavior of resolution depending + * on the language of the importing module. + * @param {string} options.entryCompartmentName + * @param {string} options.entryModuleSpecifier + * @param {ExitModuleImportHook} [options.exitModuleImportHook] * @returns {ImportHookMaker} */ -export const makeImportHookMaker = ({ +export const makeImportHookMaker = ( readPowers, baseLocation, - sources = Object.create(null), - compartmentDescriptors = Object.create(null), - exitModuleImportHook = undefined, - archiveOnly = false, - computeSha512 = undefined, - searchSuffixes = nodejsConventionSearchSuffixes, -}) => { - // Set of specifiers for modules whose parser is not using heuristics to determine imports - const strictlyRequired = new Set(); - // per-assembly: + { + sources = Object.create(null), + compartmentDescriptors = Object.create(null), + archiveOnly = false, + computeSha512 = undefined, + searchSuffixes = nodejsConventionSearchSuffixes, + entryCompartmentName, + entryModuleSpecifier, + exitModuleImportHook = undefined, + }, +) => { + // Set of specifiers for modules (scoped to compartment) whose parser is not + // using heuristics to determine imports. + /** @type {Map>} compartment name ->* module specifier */ + const strictlyRequired = new Map([ + [entryCompartmentName, new Set([entryModuleSpecifier])], + ]); + + /** + * @param {string} compartmentName + */ + const strictlyRequiredForCompartment = compartmentName => { + let compartmentStrictlyRequired = strictlyRequired.get(compartmentName); + if (compartmentStrictlyRequired !== undefined) { + return compartmentStrictlyRequired; + } + compartmentStrictlyRequired = new Set(); + strictlyRequired.set(compartmentName, compartmentStrictlyRequired); + return compartmentStrictlyRequired; + }; + + // per-compartment: /** @type {ImportHookMaker} */ const makeImportHook = ({ packageLocation, @@ -154,7 +179,7 @@ export const makeImportHookMaker = ({ // defer, because importing from esm makes it strictly required. // Note that ultimately a situation may arise, with exit modules, where the module never reaches importHook but // its imports do. In that case the notion of strictly required is no longer boolean, it's true,false,noidea. - if (strictlyRequired.has(specifier)) { + if (strictlyRequiredForCompartment(packageLocation).has(specifier)) { throw error; } // Return a place-holder that'd throw an error if executed @@ -323,10 +348,11 @@ export const makeImportHookMaker = ({ sha512, }; if (!shouldDeferError(parser)) { - getImportsFromRecord(record).forEach( - strictlyRequired.add, - strictlyRequired, - ); + for (const importSpecifier of getImportsFromRecord(record)) { + strictlyRequiredForCompartment(packageLocation).add( + resolve(importSpecifier, moduleSpecifier), + ); + } } return record; diff --git a/packages/compartment-mapper/src/import.js b/packages/compartment-mapper/src/import.js index c22c387095..7be89bd312 100644 --- a/packages/compartment-mapper/src/import.js +++ b/packages/compartment-mapper/src/import.js @@ -85,13 +85,13 @@ export const loadLocation = async (readPowers, moduleLocation, options) => { modules, exitModuleImportHook, }); - const makeImportHook = makeImportHookMaker({ - readPowers, - baseLocation: packageLocation, + const makeImportHook = makeImportHookMaker(readPowers, packageLocation, { compartmentDescriptors: compartmentMap.compartments, - exitModuleImportHook: compartmentExitModuleImportHook, - archiveOnly: false, searchSuffixes, + archiveOnly: false, + entryCompartmentName: packageLocation, + entryModuleSpecifier: moduleSpecifier, + exitModuleImportHook: compartmentExitModuleImportHook, }); const { compartment, pendingJobsPromise } = link(compartmentMap, { makeImportHook, diff --git a/packages/compartment-mapper/src/link.js b/packages/compartment-mapper/src/link.js index 53fca39217..f4aee2e7aa 100644 --- a/packages/compartment-mapper/src/link.js +++ b/packages/compartment-mapper/src/link.js @@ -345,9 +345,6 @@ export const link = ( compartmentDescriptors, ); - /** @type {Record} */ - const resolvers = Object.create(null); - const pendingJobs = []; for (const [compartmentName, compartmentDescriptor] of entries( @@ -383,6 +380,9 @@ export const link = ( } }; + // If we ever need an alternate resolution algorithm, it should be + // indicated in the compartment descriptor and a behavior selected here. + const resolveHook = resolve; const importHook = makeImportHook({ packageLocation: location, packageName: name, @@ -398,8 +398,6 @@ export const link = ( modules, scopes, ); - const resolveHook = resolve; - resolvers[compartmentName] = resolve; const compartment = new Compartment(Object.create(null), undefined, { resolveHook, @@ -437,7 +435,6 @@ export const link = ( return { compartment, compartments, - resolvers, attenuatorsCompartment, pendingJobsPromise: promiseAllSettled(pendingJobs).then( /** @param {PromiseSettledResult[]} results */ results => { diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/left/index.cjs b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/left/index.cjs new file mode 100644 index 0000000000..a2a1da4955 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/left/index.cjs @@ -0,0 +1,4 @@ +try { + // Does not exist in parent directory. + require('../inconsistent.js'); +} catch {} diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/main.js b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/main.js new file mode 100644 index 0000000000..6055001a72 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/main.js @@ -0,0 +1,2 @@ +import './left/index.cjs'; +import './right/child/index.mjs'; diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/package.json b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/package.json new file mode 100644 index 0000000000..7c44008c73 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/package.json @@ -0,0 +1,10 @@ +{ + "name": "app", + "version": "1.0.0", + "type": "module", + "main": "./main.js", + "scripts": { + "preinstall": "echo DO NOT INSTALL TEST FIXTURES; exit -1" + } +} + diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/right/child/index.mjs b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/right/child/index.mjs new file mode 100644 index 0000000000..f2ab2d7081 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/right/child/index.mjs @@ -0,0 +1,2 @@ +// Does exist in the parent directory and is strictly retained. +import '../inconsistent.js'; diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/right/inconsistent.js b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/right/inconsistent.js new file mode 100644 index 0000000000..d154322f94 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-directories/right/inconsistent.js @@ -0,0 +1 @@ +// Exists in this directory! diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/main.js b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/main.js new file mode 100644 index 0000000000..67b1d9974b --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/main.js @@ -0,0 +1,2 @@ +import 'left'; +import 'right'; diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/left/main.js b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/left/main.js new file mode 100644 index 0000000000..a5a7846a3f --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/left/main.js @@ -0,0 +1,4 @@ +if (false) { + // Does not exist in this package, so not strictly required. + require('./inconsistent.js'); +} diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/left/package.json b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/left/package.json new file mode 100644 index 0000000000..3920c67a87 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/left/package.json @@ -0,0 +1,8 @@ +{ + "name": "left", + "version": "1.0.0", + "main": "./main.js", + "scripts": { + "preinstall": "echo DO NOT INSTALL TEST FIXTURES; exit -1" + } +} diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/right/inconsistent.js b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/right/inconsistent.js new file mode 100644 index 0000000000..d44ac2a996 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/right/inconsistent.js @@ -0,0 +1 @@ +// A file with this name exists in this package. diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/right/main.js b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/right/main.js new file mode 100644 index 0000000000..edd2ef0ce2 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/right/main.js @@ -0,0 +1 @@ +import './inconsistent.js'; // Exists in this package! diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/right/package.json b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/right/package.json new file mode 100644 index 0000000000..625814d656 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/node_modules/right/package.json @@ -0,0 +1,9 @@ +{ + "name": "right", + "version": "1.0.0", + "type": "module", + "main": "./main.js", + "scripts": { + "preinstall": "echo DO NOT INSTALL TEST FIXTURES; exit -1" + } +} diff --git a/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/package.json b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/package.json new file mode 100644 index 0000000000..36e540fc31 --- /dev/null +++ b/packages/compartment-mapper/test/fixtures-strictly-inconsistent-packages/package.json @@ -0,0 +1,14 @@ +{ + "name": "app", + "version": "1.0.0", + "type": "module", + "main": "./main.js", + "dependencies": { + "left": "^1.0.0", + "right": "^1.0.0" + }, + "scripts": { + "preinstall": "echo DO NOT INSTALL TEST FIXTURES; exit -1" + } +} + diff --git a/packages/compartment-mapper/test/test-missing-entry.js b/packages/compartment-mapper/test/test-missing-entry.js new file mode 100644 index 0000000000..f53cff9bdd --- /dev/null +++ b/packages/compartment-mapper/test/test-missing-entry.js @@ -0,0 +1,23 @@ +import 'ses'; +import test from 'ava'; +import path from 'path'; +import fs from 'fs'; +import url from 'url'; +import crypto from 'crypto'; + +import { makeAndHashArchive } from '../archive.js'; +import { makeReadPowers } from '../node-powers.js'; + +const readPowers = makeReadPowers({ fs, url, crypto }); + +test('missing entry', async t => { + const entry = url.pathToFileURL( + path.resolve('i-solemnly-swear-i-do-not-exist.js'), + ); + await t.throwsAsync( + makeAndHashArchive(readPowers, entry, {}).then(() => {}), + { + message: /Failed to load/, + }, + ); +}); diff --git a/packages/compartment-mapper/test/test-strictly-inconsistent.js b/packages/compartment-mapper/test/test-strictly-inconsistent.js new file mode 100644 index 0000000000..11a6315468 --- /dev/null +++ b/packages/compartment-mapper/test/test-strictly-inconsistent.js @@ -0,0 +1,23 @@ +import 'ses'; +import test from 'ava'; +import { scaffold } from './scaffold.js'; + +scaffold( + 'inconsistently strictly required between directories', + test, + new URL('fixtures-strictly-inconsistent-directories/main.js', import.meta.url) + .href, + t => { + t.pass(); + }, + 1, +); + +scaffold( + 'inconsistently strictly required between packages', + test, + new URL('fixtures-strictly-inconsistent-packages/main.js', import.meta.url) + .href, + t => t.pass(), + 1, +);