From d3b49e828bd0ea2344079d2b0d205e4633624bee Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Fri, 1 Sep 2023 14:53:50 -0700 Subject: [PATCH 1/4] feat(compartment-mapper): add policy-related types This adds types for policies and exports them from the `compartment-mapper` entry point. It also updates some usages of existing and new types. --- .../compartment-mapper/demo/policy/index.mjs | 7 +- packages/compartment-mapper/src/import.js | 2 +- .../compartment-mapper/src/node-modules.js | 2 +- .../compartment-mapper/src/policy-format.js | 85 ++++++++----- packages/compartment-mapper/src/policy.js | 116 ++++++++++-------- packages/compartment-mapper/src/types.js | 89 ++++++++++++-- packages/compartment-mapper/types.d.ts | 3 + 7 files changed, 216 insertions(+), 88 deletions(-) diff --git a/packages/compartment-mapper/demo/policy/index.mjs b/packages/compartment-mapper/demo/policy/index.mjs index 1fb2b743ec..940b8aae77 100644 --- a/packages/compartment-mapper/demo/policy/index.mjs +++ b/packages/compartment-mapper/demo/policy/index.mjs @@ -8,7 +8,11 @@ lockdown({ import fs from 'fs'; -import { importLocation, makeArchive, parseArchive } from '../../index.js'; +import { + importLocation, + makeArchive, + parseArchive, +} from '@endo/compartment-mapper'; const readPower = async location => fs.promises.readFile(new URL(location).pathname); @@ -18,6 +22,7 @@ const entrypointPath = new URL('./app.js', import.meta.url).href; const ApiSubsetOfBuffer = harden({ from: Buffer.from }); const options = { + /** @type {import('@endo/compartment-mapper').Policy} */ policy: { defaultAttenuator: '@endo/compartment-mapper-demo-lavamoat-style-attenuator', diff --git a/packages/compartment-mapper/src/import.js b/packages/compartment-mapper/src/import.js index 7be89bd312..1bb6221c5a 100644 --- a/packages/compartment-mapper/src/import.js +++ b/packages/compartment-mapper/src/import.js @@ -115,7 +115,7 @@ export const loadLocation = async (readPowers, moduleLocation, options) => { * @param {ReadFn | ReadPowers} readPowers * @param {string} moduleLocation * @param {ExecuteOptions & ArchiveOptions} [options] - * @returns {Promise} the object of the imported modules exported + * @returns {Promise} the object of the imported modules exported * names. */ export const importLocation = async ( diff --git a/packages/compartment-mapper/src/node-modules.js b/packages/compartment-mapper/src/node-modules.js index 056070f12a..e6771ace34 100644 --- a/packages/compartment-mapper/src/node-modules.js +++ b/packages/compartment-mapper/src/node-modules.js @@ -631,7 +631,7 @@ const translateGraph = ( name, path, }, - packagePolicy, + /** @type {import('./types.js').PackagePolicy} */ (packagePolicy), ) ) { moduleDescriptors[localPath] = { diff --git a/packages/compartment-mapper/src/policy-format.js b/packages/compartment-mapper/src/policy-format.js index 958ca2fb1f..9dc7ca3150 100644 --- a/packages/compartment-mapper/src/policy-format.js +++ b/packages/compartment-mapper/src/policy-format.js @@ -1,8 +1,5 @@ // @ts-check -/** @typedef {import('./types.js').AttenuationDefinition} AttenuationDefinition */ -/** @typedef {import('./types.js').UnifiedAttenuationDefinition} UnifiedAttenuationDefinition */ - const { entries, keys } = Object; const { isArray } = Array; const q = JSON.stringify; @@ -10,14 +7,18 @@ const q = JSON.stringify; const ATTENUATOR_KEY = 'attenuate'; const ATTENUATOR_PARAMS_KEY = 'params'; const WILDCARD_POLICY_VALUE = 'any'; -const POLICY_FIELDS_LOOKUP = ['builtins', 'globals', 'packages']; +const POLICY_FIELDS_LOOKUP = /** @type {const} */ ([ + 'builtins', + 'globals', + 'packages', +]); /** * - * @param {object} packagePolicy - * @param {string} field + * @param {import('./types.js').PackagePolicy} packagePolicy + * @param {'builtins'|'globals'|'packages'} field * @param {string} itemName - * @returns {boolean | object} + * @returns {boolean | import('./types.js').AttenuationDefinition} */ export const policyLookupHelper = (packagePolicy, field, itemName) => { if (!POLICY_FIELDS_LOOKUP.includes(field)) { @@ -34,38 +35,42 @@ export const policyLookupHelper = (packagePolicy, field, itemName) => { if (packagePolicy[field] === WILDCARD_POLICY_VALUE) { return true; } - if (packagePolicy[field][itemName]) { - return packagePolicy[field][itemName]; + + const value = /** @type {import('./types.js').AttenuationDefinition} */ ( + packagePolicy[field] + ); + if (itemName in value) { + return value[itemName]; } return false; }; /** - * Checks if the policy value is set to wildcard to allow everything + * Type guard; checks if the policy value is set to the wildcard value to allow everything * - * @param {any} policyValue - * @returns {boolean} + * @param {unknown} policyValue + * @returns {policyValue is import('./types.js').WildcardPolicy} */ export const isAllowingEverything = policyValue => policyValue === WILDCARD_POLICY_VALUE; /** - * - * @param {AttenuationDefinition} potentialDefinition - * @returns {boolean} + * Type guard for `AttenuationDefinition` + * @param {unknown} allegedDefinition + * @returns {allegedDefinition is import('./types.js').AttenuationDefinition} */ -export const isAttenuationDefinition = potentialDefinition => { +export const isAttenuationDefinition = allegedDefinition => { return ( - (typeof potentialDefinition === 'object' && - typeof potentialDefinition[ATTENUATOR_KEY] === 'string') || // object with attenuator name - isArray(potentialDefinition) // params for default attenuator + (typeof allegedDefinition === 'object' && + typeof allegedDefinition?.[ATTENUATOR_KEY] === 'string') || // object with attenuator name + isArray(allegedDefinition) // params for default attenuator ); }; /** * - * @param {AttenuationDefinition} attenuationDefinition - * @returns {UnifiedAttenuationDefinition} + * @param {import('./types.js').AttenuationDefinition} attenuationDefinition + * @returns {import('./types.js').UnifiedAttenuationDefinition} */ export const getAttenuatorFromDefinition = attenuationDefinition => { if (!isAttenuationDefinition(attenuationDefinition)) { @@ -90,32 +95,49 @@ export const getAttenuatorFromDefinition = attenuationDefinition => { } }; +// TODO: should be a type guard const isRecordOf = (item, predicate) => { if (typeof item !== 'object' || item === null || isArray(item)) { return false; } return entries(item).every(([key, value]) => predicate(value, key)); }; + +/** + * Type guard for `boolean` + * @param {unknown} item + * @returns {item is boolean} + */ const isBoolean = item => typeof item === 'boolean'; + +// TODO: should be a type guard const predicateOr = (...predicates) => item => predicates.some(p => p(item)); + +/** + * @param {unknown} item + * @returns {item is import('./types.js').PolicyItem} + */ const isPolicyItem = item => item === undefined || item === WILDCARD_POLICY_VALUE || isRecordOf(item, isBoolean); /** + * This asserts (i.e., throws) that `allegedPackagePolicy` is a valid `PackagePolicy`. + * + * Mild-mannered during the day, but fights crime at night as a type guard. * - * @param {unknown} allegedPackagePolicy - * @param {string} path - * @param {string} [url] - * @returns {void} + * @param {unknown} allegedPackagePolicy - Alleged `PackagePolicy` to test + * @param {string} path - Path in the `Policy` object; used for error messages only + * @param {string} [url] - URL of the policy file; used for error messages only + * @returns {allegedPackagePolicy is import('./types.js').PackagePolicy} */ export const assertPackagePolicy = (allegedPackagePolicy, path, url) => { if (allegedPackagePolicy === undefined) { - return; + return true; } const inUrl = url ? ` in ${q(url)}` : ''; @@ -169,16 +191,20 @@ export const assertPackagePolicy = (allegedPackagePolicy, path, url) => { builtins, })}${inUrl}`, ); + return true; }; /** + * This asserts (i.e., throws) that `allegedPolicy` is a valid `Policy` + * + * It also moonlights as a type guard. * - * @param {unknown} allegedPolicy - * @returns {void} + * @param {unknown} allegedPolicy - Alleged `Policy` to test + * @returns {allegedPolicy is import('./types.js').Policy} */ export const assertPolicy = allegedPolicy => { if (allegedPolicy === undefined) { - return; + return true; } const policy = Object(allegedPolicy); assert( @@ -206,4 +232,5 @@ export const assertPolicy = allegedPolicy => { for (const [key, value] of entries(resources)) { assertPackagePolicy(value, `policy.resources["${key}"]`); } + return true; }; diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js index 8604bcb2ac..16ae040c9b 100644 --- a/packages/compartment-mapper/src/policy.js +++ b/packages/compartment-mapper/src/policy.js @@ -1,20 +1,10 @@ // @ts-check -/** @typedef {import('./types.js').PackageNamingKit} PackageNamingKit */ -/** @typedef {import('./types.js').AttenuationDefinition} AttenuationDefinition */ -/** @typedef {import('./types.js').FullAttenuationDefinition} FullAttenuationDefinition */ -/** @typedef {import('./types.js').ImplicitAttenuationDefinition} ImplicitAttenuationDefinition */ -/** @typedef {import('./types.js').Attenuator} Attenuator */ -/** @typedef {import('./types.js').DeferredAttenuatorsProvider} DeferredAttenuatorsProvider */ -/** @typedef {import('./types.js').CompartmentDescriptor} CompartmentDescriptor */ -// get StaticModuleRecord from the ses package's types -/** @typedef {import('ses').ThirdPartyStaticModuleInterface} ThirdPartyStaticModuleInterface */ - import { - policyLookupHelper, - isAttenuationDefinition, getAttenuatorFromDefinition, isAllowingEverything, + isAttenuationDefinition, + policyLookupHelper, } from './policy-format.js'; const { create, entries, values, assign, keys, freeze } = Object; @@ -46,8 +36,15 @@ const selectiveCopy = (from, to, list) => { return to; }; +/** + * Parses an attenuation definition for attenuator names + * + * Note: this function is recursive + * @param {string[]} attenuators - List of attenuator names; may be mutated + * @param {import('./types.js').AttenuationDefinition|import('./types.js').Policy} policyFragment + */ const collectAttenuators = (attenuators, policyFragment) => { - if (policyFragment.attenuate) { + if ('attenuate' in policyFragment) { attenuators.push(policyFragment.attenuate); } for (const value of values(policyFragment)) { @@ -58,11 +55,12 @@ const collectAttenuators = (attenuators, policyFragment) => { }; const attenuatorsCache = new WeakMap(); + /** * Goes through policy and lists all attenuator specifiers used. * Memoization keyed on policy object reference * - * @param {object} policy + * @param {import('./types.js').Policy} [policy] * @returns {Array} attenuators */ export const detectAttenuators = policy => { @@ -83,7 +81,7 @@ export const detectAttenuators = policy => { /** * Generates a string identifying a package for policy lookup purposes. * - * @param {PackageNamingKit} namingKit + * @param {import('./types.js').PackageNamingKit} namingKit * @returns {string} */ const generateCanonicalName = ({ isEntry = false, name, path }) => { @@ -97,11 +95,11 @@ const generateCanonicalName = ({ isEntry = false, name, path }) => { }; /** - * Verifies if a module identified by namingKit can be a dependency of a package per packagePolicy. - * packagePolicy is required, when policy is not set, skipping needs to be handled by the caller. + * Verifies if a module identified by `namingKit` can be a dependency of a package per `packagePolicy`. + * `packagePolicy` is required, when policy is not set, skipping needs to be handled by the caller. * - * @param {PackageNamingKit} namingKit - * @param {any} packagePolicy + * @param {import('./types.js').PackageNamingKit} namingKit + * @param {import('./types.js').PackagePolicy} packagePolicy * @returns {boolean} */ export const dependencyAllowedByPolicy = (namingKit, packagePolicy) => { @@ -116,9 +114,9 @@ export const dependencyAllowedByPolicy = (namingKit, packagePolicy) => { /** * Returns the policy applicable to the canonicalName of the package * - * @param {PackageNamingKit} namingKit - a key in the policy resources spec is derived frm these - * @param {object|undefined} policy - user supplied policy - * @returns {object|undefined} packagePolicy if policy was specified + * @param {import('./types.js').PackageNamingKit} namingKit - a key in the policy resources spec is derived frm these + * @param {import('./types.js').Policy} [policy] - user supplied policy + * @returns {import('./types.js').PackagePolicy|undefined} packagePolicy if policy was specified */ export const getPolicyForPackage = (namingKit, policy) => { if (!policy) { @@ -142,21 +140,27 @@ export const getPolicyForPackage = (namingKit, policy) => { } }; +/** + * Get list of globals from package policy + * @param {import('./types.js').PackagePolicy} [packagePolicy] + * @returns {Array} + */ const getGlobalsList = packagePolicy => { - if (!packagePolicy.globals) { + if (!packagePolicy?.globals) { return []; } - return entries(packagePolicy.globals) + return entries(packagePolicy?.globals) .filter(([_key, value]) => value) .map(([key, _vvalue]) => key); }; const GLOBAL_ATTENUATOR = 'attenuateGlobals'; const MODULE_ATTENUATOR = 'attenuateModule'; + /** - * - * @param {AttenuationDefinition} attenuationDefinition - * @param {DeferredAttenuatorsProvider} attenuatorsProvider + * Imports attenuator per its definition and provider + * @param {import('./types.js').AttenuationDefinition} attenuationDefinition + * @param {import('./types.js').DeferredAttenuatorsProvider} attenuatorsProvider * @param {string} attenuatorExportName * @returns {Promise} */ @@ -183,10 +187,10 @@ const importAttenuatorForDefinition = async ( }; /** - * + * Makes an async provider for attenuators * @param {Record} compartments - * @param {Record} compartmentDescriptors - * @returns {DeferredAttenuatorsProvider} + * @param {Record} compartmentDescriptors + * @returns {import('./types.js').DeferredAttenuatorsProvider} */ export const makeDeferredAttenuatorsProvider = ( compartments, @@ -210,7 +214,7 @@ export const makeDeferredAttenuatorsProvider = ( /** * * @param {string} attenuatorSpecifier - * @returns {Promise} + * @returns {Promise} */ importAttenuator = async attenuatorSpecifier => { if (!attenuatorSpecifier) { @@ -232,10 +236,11 @@ export const makeDeferredAttenuatorsProvider = ( }; /** + * Attenuates the `globalThis` object * * @param {object} options - * @param {DeferredAttenuatorsProvider} options.attenuators - * @param {AttenuationDefinition} options.attenuationDefinition + * @param {import('./types.js').DeferredAttenuatorsProvider} options.attenuators + * @param {import('./types.js').AttenuationDefinition} options.attenuationDefinition * @param {object} options.globalThis * @param {object} options.globals */ @@ -274,8 +279,8 @@ async function attenuateGlobalThis({ * * @param {object} globalThis * @param {object} globals - * @param {object} packagePolicy - * @param {DeferredAttenuatorsProvider} attenuators + * @param {import('./types.js').PackagePolicy} packagePolicy + * @param {import('./types.js').DeferredAttenuatorsProvider} attenuators * @param {Array} pendingJobs * @param {string} name * @returns {void} @@ -328,37 +333,49 @@ export const attenuateGlobals = ( freezeGlobalThisUnlessOptedOut(); }; +/** + * @param {string} [errorHint] + * @returns {string} + */ const diagnoseModulePolicy = errorHint => { if (!errorHint) { return ''; } return ` (info: ${errorHint})`; }; + +/** + * Options for {@link enforceModulePolicy} + * @typedef EnforceModulePolicyOptions + * @property {boolean} [exit] - Whether it is an exit module + * @property {string} [errorHint] - Error hint message + */ + /** * Throws if importing of the specifier is not allowed by the policy * * @param {string} specifier * @param {import('./types.js').CompartmentDescriptor} compartmentDescriptor - * @param {object} [info] + * @param {EnforceModulePolicyOptions} [options] */ export const enforceModulePolicy = ( specifier, compartmentDescriptor, - info = {}, + { exit, errorHint } = {}, ) => { const { policy, modules, label } = compartmentDescriptor; if (!policy) { return; } - if (!info.exit) { + if (!exit) { if (!modules[specifier]) { throw Error( `Importing ${q(specifier)} in ${q( label, )} was not allowed by packages policy ${q( policy.packages, - )}${diagnoseModulePolicy(info.errorHint)}`, + )}${diagnoseModulePolicy(errorHint)}`, ); } return; @@ -368,18 +385,18 @@ export const enforceModulePolicy = ( throw Error( `Importing ${q(specifier)} was not allowed by policy builtins:${q( policy.builtins, - )}${diagnoseModulePolicy(info.errorHint)}`, + )}${diagnoseModulePolicy(errorHint)}`, ); } }; /** - * + * Attenuates a module * @param {object} options - * @param {DeferredAttenuatorsProvider} options.attenuators - * @param {AttenuationDefinition} options.attenuationDefinition - * @param {ThirdPartyStaticModuleInterface} options.originalModuleRecord - * @returns {Promise} + * @param {import('./types.js').DeferredAttenuatorsProvider} options.attenuators + * @param {import('./types.js').AttenuationDefinition} options.attenuationDefinition + * @param {import('ses').ThirdPartyStaticModuleInterface} options.originalModuleRecord + * @returns {Promise} */ async function attenuateModule({ attenuators, @@ -409,14 +426,15 @@ async function attenuateModule({ }, }); } + /** * Throws if importing of the specifier is not allowed by the policy * * @param {string} specifier - exit module name - * @param {ThirdPartyStaticModuleInterface} originalModuleRecord - reference to the exit module - * @param {object} policy - local compartment policy - * @param {DeferredAttenuatorsProvider} attenuators - a key-value where attenuations can be found - * @returns {Promise} - the attenuated module + * @param {import('ses').ThirdPartyStaticModuleInterface} originalModuleRecord - reference to the exit module + * @param {import('./types.js').PackagePolicy} policy - local compartment policy + * @param {import('./types.js').DeferredAttenuatorsProvider} attenuators - a key-value where attenuations can be found + * @returns {Promise} - the attenuated module */ export const attenuateModuleHook = async ( specifier, diff --git a/packages/compartment-mapper/src/types.js b/packages/compartment-mapper/src/types.js index e10d9c6a49..9e8f60504c 100644 --- a/packages/compartment-mapper/src/types.js +++ b/packages/compartment-mapper/src/types.js @@ -146,7 +146,7 @@ export {}; /** * @callback ExecuteFn * @param {ExecuteOptions} [options] - * @returns {Promise} + * @returns {Promise} */ /** @@ -399,16 +399,19 @@ export {}; */ /** + * An object representing a full attenuation definition. * @typedef {object} FullAttenuationDefinition - * @property {string} name - * @property {Array} params + * @property {string} attenuate - The type of attenuation. + * @property {ImplicitAttenuationDefinition} params - The parameters for the attenuation. */ /** - * @typedef {Array} ImplicitAttenuationDefinition + * An array of any type representing an implicit attenuation definition. + * @typedef {[any, ...any[]]} ImplicitAttenuationDefinition */ /** + * A type representing an attenuation definition, which can be either a full or implicit definition. * @typedef {FullAttenuationDefinition | ImplicitAttenuationDefinition} AttenuationDefinition */ @@ -420,12 +423,84 @@ export {}; */ /** - * @typedef {object} Attenuator - * @property {Function} [attenuateGlobals] - * @property {Function} [attenuateModule] + * @template {[any, ...any[]]} [GlobalParams=[any, ...any[]]] + * @template {[any, ...any[]]} [ModuleParams=[any, ...any[]]] + * @typedef Attenuator + * @property {GlobalAttenuatorFn} [attenuateGlobals] + * @property {ModuleAttenuatorFn} [attenuateModule] + */ + +/** + * @template {[any, ...any[]]} [Params=[any, ...any[]]] + * @callback GlobalAttenuatorFn + * @param {Params} params + * @param {Record} originalObject + * @param {Record} globalThis + * @returns {void} + * @todo Unsure if we can do much typing of `originalObject` and `globalThis` here. + */ + +/** + * @template {[any, ...any[]]} [Params=[any, ...any[]]] + * @template [T=unknown] + * @template [U=T] + * @callback ModuleAttenuatorFn + * @param {Params} params + * @param {T} ns + * @returns {U} */ /** * @typedef {object} DeferredAttenuatorsProvider * @property {(attenuatorSpecifier: string|null) => Promise} import */ + +/** + * A type representing a wildcard policy, which can be 'any'. + * @typedef {'any'} WildcardPolicy + */ + +/** + * A type representing a property policy, which is a record of string keys and boolean values. + * @typedef {Record} PropertyPolicy + */ + +/** + * A type representing a policy item, which can be a {@link WildcardPolicy wildcard policy}, a property policy, `undefined`, or defined by an attenuator + * @template [T=void] + * @typedef {WildcardPolicy|PropertyPolicy|T} PolicyItem + */ + +/** + * An object representing a nested attenuation definition. + * @typedef {Record} NestedAttenuationDefinition + */ + +/** + * An object representing a base package policy. + * @template [PackagePolicyItem=void] + * @template [GlobalsPolicyItem=void] + * @template [BuiltinsPolicyItem=void] + * @typedef {object} PackagePolicy + * @property {string} [defaultAttenuator] - The default attenuator. + * @property {PolicyItem} [packages] - The policy item for packages. + * @property {PolicyItem|AttenuationDefinition} [globals] - The policy item or full attenuation definition for globals. + * @property {PolicyItem|NestedAttenuationDefinition} [builtins] - The policy item or nested attenuation definition for builtins. + * @property {boolean} [noGlobalFreeze] - Whether to disable global freeze. + */ + +/** + * An object representing a base policy. + * @template [PackagePolicyItem=void] + * @template [GlobalsPolicyItem=void] + * @template [BuiltinsPolicyItem=void] + * @typedef {object} Policy + * @property {Record>} resources - The package policies for the resources. + * @property {string} [defaultAttenuator] - The default attenuator. + * @property {PackagePolicy} [entry] - The package policy for the entry. + */ + +/** + * Any object. All objects. Not `null`, though. + * @typedef {Record} SomeObject + */ diff --git a/packages/compartment-mapper/types.d.ts b/packages/compartment-mapper/types.d.ts index 9ce6f12e4b..e480609c63 100644 --- a/packages/compartment-mapper/types.d.ts +++ b/packages/compartment-mapper/types.d.ts @@ -14,3 +14,6 @@ export { export { search } from './src/search.js'; export { compartmentMapForNodeModules } from './src/node-modules.js'; export { makeBundle, writeBundle } from './src/bundle.js'; + +// eslint-disable-next-line import/export -- ESLint doesn't understand this +export type * from './src/types.js'; From ed6f23e5957dd89b9732aa087d99c10a58f56c26 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Thu, 14 Dec 2023 14:23:41 -0800 Subject: [PATCH 2/4] fix(compartment-mapper): handle implicit policy/packagePolicy ...as requested by @naugtur. also change some return types to use TS' `asserts` keyword. --- .../compartment-mapper/src/node-modules.js | 29 ++++++++++--------- .../compartment-mapper/src/policy-format.js | 10 +++---- packages/compartment-mapper/src/policy.js | 21 ++++++++++++-- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/packages/compartment-mapper/src/node-modules.js b/packages/compartment-mapper/src/node-modules.js index e6771ace34..9f94eb9d5f 100644 --- a/packages/compartment-mapper/src/node-modules.js +++ b/packages/compartment-mapper/src/node-modules.js @@ -42,18 +42,18 @@ * @typedef {Record} CommonDependencyDescriptors */ +import { pathCompare } from './compartment-map.js'; import { inferExportsAndAliases } from './infer-exports.js'; -import { searchDescriptor } from './search.js'; import { parseLocatedJson } from './json.js'; -import { unpackReadPowers } from './powers.js'; +import { join } from './node-module-specifier.js'; +import { assertPolicy } from './policy-format.js'; import { - getPolicyForPackage, ATTENUATORS_COMPARTMENT, dependencyAllowedByPolicy, + getPolicyForPackage, } from './policy.js'; -import { join } from './node-module-specifier.js'; -import { pathCompare } from './compartment-map.js'; -import { assertPolicy } from './policy-format.js'; +import { unpackReadPowers } from './powers.js'; +import { searchDescriptor } from './search.js'; const { assign, create, keys, values } = Object; @@ -566,7 +566,7 @@ const graphPackages = async ( * @param {Graph} graph * @param {Set} tags - build tags about the target environment * for selecting relevant exports, e.g., "browser" or "node". - * @param {object|undefined} policy + * @param {import('./types.js').Policy} [policy] * @returns {CompartmentMapDescriptor} */ const translateGraph = ( @@ -626,13 +626,14 @@ const translateGraph = ( const localPath = join(dependencyName, exportPath); if ( !policy || - dependencyAllowedByPolicy( - { - name, - path, - }, - /** @type {import('./types.js').PackagePolicy} */ (packagePolicy), - ) + (packagePolicy && + dependencyAllowedByPolicy( + { + name, + path, + }, + packagePolicy, + )) ) { moduleDescriptors[localPath] = { compartment: packageLocation, diff --git a/packages/compartment-mapper/src/policy-format.js b/packages/compartment-mapper/src/policy-format.js index 9dc7ca3150..226d1d328e 100644 --- a/packages/compartment-mapper/src/policy-format.js +++ b/packages/compartment-mapper/src/policy-format.js @@ -133,11 +133,11 @@ const isPolicyItem = item => * @param {unknown} allegedPackagePolicy - Alleged `PackagePolicy` to test * @param {string} path - Path in the `Policy` object; used for error messages only * @param {string} [url] - URL of the policy file; used for error messages only - * @returns {allegedPackagePolicy is import('./types.js').PackagePolicy} + * @returns {asserts allegedPackagePolicy is import('./types.js').PackagePolicy|undefined} */ export const assertPackagePolicy = (allegedPackagePolicy, path, url) => { if (allegedPackagePolicy === undefined) { - return true; + return; } const inUrl = url ? ` in ${q(url)}` : ''; @@ -191,7 +191,6 @@ export const assertPackagePolicy = (allegedPackagePolicy, path, url) => { builtins, })}${inUrl}`, ); - return true; }; /** @@ -200,11 +199,11 @@ export const assertPackagePolicy = (allegedPackagePolicy, path, url) => { * It also moonlights as a type guard. * * @param {unknown} allegedPolicy - Alleged `Policy` to test - * @returns {allegedPolicy is import('./types.js').Policy} + * @returns {asserts allegedPolicy is import('./types.js').Policy|undefined} */ export const assertPolicy = allegedPolicy => { if (allegedPolicy === undefined) { - return true; + return; } const policy = Object(allegedPolicy); assert( @@ -232,5 +231,4 @@ export const assertPolicy = allegedPolicy => { for (const [key, value] of entries(resources)) { assertPackagePolicy(value, `policy.resources["${key}"]`); } - return true; }; diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js index 16ae040c9b..14cd05b644 100644 --- a/packages/compartment-mapper/src/policy.js +++ b/packages/compartment-mapper/src/policy.js @@ -114,10 +114,27 @@ export const dependencyAllowedByPolicy = (namingKit, packagePolicy) => { /** * Returns the policy applicable to the canonicalName of the package * - * @param {import('./types.js').PackageNamingKit} namingKit - a key in the policy resources spec is derived frm these + * @overload + * @param {import('./types.js').PackageNamingKit} namingKit - a key in the policy resources spec is derived from these + * @param {import('./types.js').Policy} policy - user supplied policy + * @returns {import('./types.js').PackagePolicy} packagePolicy if policy was specified + */ + +/** + * Returns `undefined` + * + * @overload + * @param {import('./types.js').PackageNamingKit} namingKit - a key in the policy resources spec is derived from these * @param {import('./types.js').Policy} [policy] - user supplied policy * @returns {import('./types.js').PackagePolicy|undefined} packagePolicy if policy was specified */ + +/** + * Returns the policy applicable to the canonicalName of the package + * + * @param {import('./types.js').PackageNamingKit} namingKit - a key in the policy resources spec is derived from these + * @param {import('./types.js').Policy} [policy] - user supplied policy + */ export const getPolicyForPackage = (namingKit, policy) => { if (!policy) { return undefined; @@ -149,7 +166,7 @@ const getGlobalsList = packagePolicy => { if (!packagePolicy?.globals) { return []; } - return entries(packagePolicy?.globals) + return entries(packagePolicy.globals) .filter(([_key, value]) => value) .map(([key, _vvalue]) => key); }; From ece09e253f48416e465a948c0e85ee6d866d7358 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Thu, 4 Jan 2024 11:12:51 -0800 Subject: [PATCH 3/4] fix(compartment-mapper): throw if policy/packagePolicy mismatch The logic forbids it, but type inference doesn't necessarily--better to be safe. --- packages/compartment-mapper/src/node-modules.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/compartment-mapper/src/node-modules.js b/packages/compartment-mapper/src/node-modules.js index 9f94eb9d5f..10e497297d 100644 --- a/packages/compartment-mapper/src/node-modules.js +++ b/packages/compartment-mapper/src/node-modules.js @@ -612,6 +612,12 @@ const translateGraph = ( policy, ); + /* c8 ignore next */ + if (policy && !packagePolicy) { + // this should never happen + throw new TypeError('Unexpectedly falsy package policy'); + } + /** * @param {string} dependencyName * @param {string} packageLocation From 9b6ef8d1047617be061bcb9f7be65e66c52e8f6b Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Tue, 9 Jan 2024 15:58:34 -0800 Subject: [PATCH 4/4] chore(compartment-mapper): lint fixes --- packages/compartment-mapper/src/policy-format.js | 9 +++++---- packages/compartment-mapper/src/policy.js | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/compartment-mapper/src/policy-format.js b/packages/compartment-mapper/src/policy-format.js index 226d1d328e..3d18f41cc1 100644 --- a/packages/compartment-mapper/src/policy-format.js +++ b/packages/compartment-mapper/src/policy-format.js @@ -60,10 +60,11 @@ export const isAllowingEverything = policyValue => * @returns {allegedDefinition is import('./types.js').AttenuationDefinition} */ export const isAttenuationDefinition = allegedDefinition => { - return ( - (typeof allegedDefinition === 'object' && - typeof allegedDefinition?.[ATTENUATOR_KEY] === 'string') || // object with attenuator name - isArray(allegedDefinition) // params for default attenuator + return Boolean( + (allegedDefinition && + typeof allegedDefinition === 'object' && + typeof allegedDefinition[ATTENUATOR_KEY] === 'string') || // object with attenuator name + isArray(allegedDefinition), // params for default attenuator ); }; diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js index 14cd05b644..02dc1b6f9c 100644 --- a/packages/compartment-mapper/src/policy.js +++ b/packages/compartment-mapper/src/policy.js @@ -163,7 +163,7 @@ export const getPolicyForPackage = (namingKit, policy) => { * @returns {Array} */ const getGlobalsList = packagePolicy => { - if (!packagePolicy?.globals) { + if (!packagePolicy || !packagePolicy.globals) { return []; } return entries(packagePolicy.globals)