Skip to content

Commit

Permalink
fix(compartment-mapper): properly handle absolute paths
Browse files Browse the repository at this point in the history 
- improve error messages
- better tests to support the `node-gyp-build` use-case
- create `makeSyncReadPowers()` as not to befoul things depending on what `makeReadPowers()` does
- remove `dynamic` flag from policy; if we need it, we can use `options`
  • Loading branch information
@boneskull
boneskull committed Jul 26, 2024
1 parent 3f597ad commit 0963553
Show file tree
Hide file tree
Showing 34 changed files with 479 additions and 173 deletions.
100 changes: 67 additions & 33 deletions packages/compartment-mapper/src/import-hook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@

// @ts-check

/** @import {ImportHook} from 'ses' */
/** @import {ImportHook, ThirdPartyStaticModuleInterface} from 'ses' */
/** @import {ImportNowHook} from 'ses' */
/** @import {StaticModuleType} from 'ses' */
/** @import {RedirectStaticModuleInterface} from 'ses' */
/** @import {MakeImportNowHookMakerOptions} from './types.js' */
/** @import {MakeImportNowHookMakerOptions, ModuleDescriptor} from './types.js' */
/** @import {ReadFn} from './types.js' */
/** @import {SyncReadPowers} from './types.js' */
/** @import {ReadPowers} from './types.js' */
Expand All @@ -24,7 +24,11 @@
/** @import {SourceMapHook} from './types.js' */
/** @import {ImportNowHookMaker} from './types.js' */

import { attenuateModuleHook, enforceModulePolicy } from './policy.js';
import {
attenuateModuleHook,
ATTENUATORS_COMPARTMENT,
enforceModulePolicy,
} from './policy.js';
import { resolve } from './node-module-specifier.js';
import { unpackReadPowers } from './powers.js';

Expand All @@ -41,7 +45,7 @@ const { apply } = Reflect;
*/
const freeze = Object.freeze;

const entries = Object.entries;
const { entries, keys } = Object;

const { hasOwnProperty } = Object.prototype;
/**
Expand Down Expand Up @@ -93,7 +97,7 @@ export const exitModuleImportHookMaker = ({
const ns = modules[specifier];
return Object.freeze({
imports: [],
exports: ns ? Object.keys(ns) : [],
exports: ns ? keys(ns) : [],
execute: moduleExports => {
moduleExports.default = ns;
Object.assign(moduleExports, ns);
Expand All @@ -117,8 +121,8 @@ export const exitModuleImportHookMaker = ({
* @param {HashFn} [options.computeSha512]
* @param {Array<string>} [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.
* Pass [] to emulate Node.js' strict behavior.
* The default handles Node.js' 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.
Expand Down Expand Up @@ -412,7 +416,7 @@ export const makeImportHookMaker = (
* @param {SyncReadPowers} readPowers
* @param {string} baseLocation
* @param {MakeImportNowHookMakerOptions} options
* @returns {ImportNowHookMaker|undefined}
* @returns {ImportNowHookMaker}
*/
export function makeImportNowHookMaker(
readPowers,
Expand Down Expand Up @@ -471,8 +475,12 @@ export function makeImportNowHookMaker(
packageLocation = resolveLocation(packageLocation, baseLocation);
const packageSources = sources[packageLocation] || Object.create(null);
sources[packageLocation] = packageSources;
const { modules: moduleDescriptors = Object.create(null) } =
compartmentDescriptor;
const {
modules:
moduleDescriptors = /** @type {Record<string, ModuleDescriptor>} */ (
Object.create(null)
),
} = compartmentDescriptor;
compartmentDescriptor.modules = moduleDescriptors;

let { policy } = compartmentDescriptor;
Expand All @@ -497,35 +505,59 @@ export function makeImportNowHookMaker(

/** @type {ImportNowHook} */
const importNowHook = moduleSpecifier => {
// defensive; this should be caught earlier
if (!policy.dynamic) {
throw new Error(
`Dynamic require not allowed in compartment ${q(compartmentDescriptor.name)}`,
);
}

if (isAbsolute(moduleSpecifier)) {
if (exitModuleImportNowHook) {
// this hook is responsible for ensuring that the moduleSpecifier actually refers to an exit module
const record = exitModuleImportNowHook(
moduleSpecifier,
packageLocation,
);

if (!record) {
throw new Error(`Could not import module: ${moduleSpecifier}`);
let shouldCallExitModuleImportNowHook = true;
for (const [someDescriptorName, someDescriptor] of entries(
compartmentDescriptors,
)) {
if (someDescriptorName !== ATTENUATORS_COMPARTMENT) {
const moduleSpecifierUrl = resolveLocation(
moduleSpecifier,
packageLocation,
);
if (
!compartmentDescriptor.location.startsWith(moduleSpecifierUrl)
) {
if (moduleSpecifierUrl.startsWith(someDescriptor.location)) {
if (compartmentDescriptor.name in someDescriptor.modules) {
return {
specifier: moduleSpecifier,
compartment: compartments[someDescriptorName],
};
} else if (compartmentDescriptor === someDescriptor) {
shouldCallExitModuleImportNowHook = false;
}
}
}
}
return record;
}
if (shouldCallExitModuleImportNowHook) {
if (exitModuleImportNowHook) {
// this hook is responsible for ensuring that the moduleSpecifier actually refers to an exit module
const record = exitModuleImportNowHook(
moduleSpecifier,
packageLocation,
);

throw new Error(`Could not import module: ${moduleSpecifier}`);
if (!record) {
throw new Error(`Could not import module: ${q(moduleSpecifier)}`);
}

return record;
}
throw new Error(
`Could not import module: ${q(moduleSpecifier)}; try providing an importNowHook`,
);
}
}

// Collate candidate locations for the moduleSpecifier,
// to support Node.js conventions and similar.
const candidates = [moduleSpecifier];
for (const candidateSuffix of searchSuffixes) {
candidates.push(`${moduleSpecifier}${candidateSuffix}`);
if (!searchSuffixes.some(suffix => moduleSpecifier.endsWith(suffix))) {
for (const candidateSuffix of searchSuffixes) {
candidates.push(`${moduleSpecifier}${candidateSuffix}`);
}
}

for (const candidateSpecifier of candidates) {
Expand Down Expand Up @@ -566,14 +598,14 @@ export function makeImportNowHookMaker(
candidateSpecifier,
packageLocation,
);

// eslint-disable-next-line no-await-in-loop
/** @type {Uint8Array} */
let moduleBytes;
try {
moduleBytes = readSync(moduleLocation);
} catch (err) {
if (err && err.code === 'ENOENT') {
// might be an exit module. use the fallback `exitModuleImportNowHook` to import it
// eslint-disable-next-line no-continue
continue;
}
Expand Down Expand Up @@ -662,13 +694,15 @@ export function makeImportNowHookMaker(
);

if (!record) {
throw new Error(`Could not import module: ${moduleSpecifier}`);
throw new Error(`Could not import module: ${q(moduleSpecifier)}`);
}

return record;
}

throw new Error(`Could not import module: ${moduleSpecifier}`);
throw new Error(
`Could not import module: ${q(moduleSpecifier)}; try providing an importNowHook`,
);
};

return importNowHook;
Expand Down
4 changes: 2 additions & 2 deletions packages/compartment-mapper/src/import-lite.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ const { assign, create, freeze } = Object;
/**
* Returns `true` if `value` is a {@link SyncImportLocationOptions}.
*
* The only requirement here is that `moduleTransforms` is _not_ present in
* `value`; `importNowHook` is optional.
* The only requirements here are:
* - `moduleTransforms` _is not_ present in `value`
*
* @param {ImportLocationOptions|SyncImportLocationOptions} value
* @returns {value is SyncImportLocationOptions}
Expand Down
49 changes: 23 additions & 26 deletions packages/compartment-mapper/src/link.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
/** @import {LinkOptions} from './types.js' */
/** @import {ERef} from '@endo/eventual-send' */

import { resolve as resolveFallback } from './node-module-specifier.js';
import { parseExtension } from './extension.js';
import { resolve as resolveFallback } from './node-module-specifier.js';
import {
enforceModulePolicy,
ATTENUATORS_COMPARTMENT,
attenuateGlobals,
enforceModulePolicy,
makeDeferredAttenuatorsProvider,
} from './policy.js';

Expand Down Expand Up @@ -453,16 +453,11 @@ const makeModuleMapHook = (
* @returns {value is SyncLinkOptions}
*/
const isSyncOptions = value =>
!value || (typeof value === 'object' && !('moduleTransforms' in value));

/**
* Checks if the compartment descriptor's policy allows dynamic requires.
*
* @param {CompartmentDescriptor} value
* @returns {boolean}
*/
const isDynamicRequireAllowed = value =>
value.policy && value.policy.dynamic === true;
!value ||
(typeof value === 'object' &&
!('moduleTransforms' in value) &&
'makeImportNowHook' in value &&
typeof value.makeImportNowHook === 'function');

/**
* Assemble a DAG of compartments as declared in a compartment map starting at
Expand Down Expand Up @@ -586,16 +581,19 @@ export const link = (
for (const [compartmentName, compartmentDescriptor] of entries(
compartmentDescriptors,
)) {
// TODO: The default assignments seem to break type inference
const {
location,
name,
modules = create(null),
parsers: languageForExtensionOverrides = {},
types: languageForModuleSpecifierOverrides = {},
scopes = create(null),
} = compartmentDescriptor;

// this is for retaining the correct type inference about these values
// without use of `let`
const { scopes: _scopes, modules: _modules } = compartmentDescriptor;
const modules = _modules || create(null);
const scopes = _scopes || create(null);

// Capture the default.
// The `moduleMapHook` writes back to the compartment map.
compartmentDescriptor.modules = modules;
Expand Down Expand Up @@ -650,30 +648,29 @@ export const link = (
});
/** @type {ImportNowHook | undefined} */
let importNowHook;
if (
isSync &&
makeImportNowHook &&
compartmentDescriptor !== entryCompartmentDescriptor
) {
if (isDynamicRequireAllowed(compartmentDescriptor)) {
// `parse` must be sync here

if (isSync) {
if (entryCompartmentDescriptor !== compartmentDescriptor) {
const syncParse = /** @type {ParseFn} */ (parse);
importNowHook = makeImportNowHook({
importNowHook = /** @type {ImportNowHookMaker} */ (makeImportNowHook)({
entryCompartmentDescriptor,
packageLocation: location,
packageName: name,
parse: syncParse,
compartments,
});
} else {
// it's possible for this to still be called, but
// the error message would be cryptic otherwise.
// should never happen
importNowHook = () => {
throw new Error(
`Dynamic require not allowed in compartment ${q(compartmentDescriptor.name)}`,
'importNowHook should not be called by entry compartment',
);
};
}
} else {
importNowHook = () => {
throw new Error('Provided read powers do not support dynamic requires');
};
}

const moduleMapHook = makeModuleMapHook(
Expand Down
47 changes: 37 additions & 10 deletions packages/compartment-mapper/src/node-powers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const fakeIsAbsolute = () => false;
* @param {UrlAPI} [args.url]
* @param {CryptoAPI} [args.crypto]
* @param {PathAPI} [args.path]
* @returns {MaybeReadPowers & SyncReadPowers}
* @returns {MaybeReadPowers}
*/
const makeReadPowersSloppy = ({
fs,
Expand Down Expand Up @@ -98,14 +98,6 @@ const makeReadPowersSloppy = ({
}
};

/**
* @type {ReadSyncFn}
*/
const readSync = location => {
const filepath = fileURLToPath(location);
return fs.readFileSync(filepath);
};

/**
* @type {MaybeReadFn}
*/
Expand Down Expand Up @@ -155,7 +147,7 @@ const makeReadPowersSloppy = ({
}
};

/** @type {HashFn=} */
/** @type {HashFn | undefined} */
const computeSha512 = crypto
? bytes => {
const hash = crypto.createHash('sha512');
Expand All @@ -172,7 +164,42 @@ const makeReadPowersSloppy = ({
canonical,
computeSha512,
requireResolve,
isAbsolute,
};
};

/**
* Creates {@link ReadPowers} for dynamic module support
*
* @param {object} args
* @param {FsAPI} args.fs
* @param {UrlAPI} [args.url]
* @param {CryptoAPI} [args.crypto]
* @param {PathAPI} [args.path]
* @returns {MaybeReadPowers & SyncReadPowers}
*/
export const makeSyncReadPowers = ({
fs,
url = undefined,
crypto = undefined,
path = undefined,
}) => {
const powers = makeReadPowersSloppy({ fs, url, crypto, path });
const fileURLToPath = powers.fileURLToPath || fakeFileURLToPath;
const isAbsolute = powers.isAbsolute || fakeIsAbsolute;

/**
* @type {ReadSyncFn}
*/
const readSync = location => {
const filepath = fileURLToPath(location);
return fs.readFileSync(filepath);
};

return {
...powers,
readSync,
fileURLToPath,
isAbsolute,
};
};
Expand Down
Loading

0 comments on commit 0963553

Please sign in to comment.