From fe2e6f407057b62736fd93cfd77ec2f3e5df4d20 Mon Sep 17 00:00:00 2001 From: naugtur Date: Mon, 18 Mar 2024 13:50:29 +0100 Subject: [PATCH 1/2] feat(core): add external package name fallback function to options in generatePolicy, use it in webpack --- packages/core/src/generatePolicy.js | 24 +++++++++++++++---- .../webpack/src/buildtime/policyGenerator.js | 4 ++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/core/src/generatePolicy.js b/packages/core/src/generatePolicy.js index 5643060971..6e634eb04e 100644 --- a/packages/core/src/generatePolicy.js +++ b/packages/core/src/generatePolicy.js @@ -348,7 +348,11 @@ function createModuleInspector(opts) { /** * @type {GeneratePolicyFn} */ - function generatePolicy({ policyOverride, includeDebugInfo = false }) { + function generatePolicy({ + policyOverride, + includeDebugInfo = false, + moduleToPackageFallback, + }) { /** @type {import('./schema').Resources} */ const resources = {} /** @@ -375,6 +379,7 @@ function createModuleInspector(opts) { const packageDeps = aggregateDeps({ packageModules, moduleIdToModuleRecord, + moduleToPackageFallback, }) if (packageDeps.length) { packages = Object.fromEntries( @@ -453,10 +458,15 @@ function createModuleInspector(opts) { * string, * import('./moduleRecord').LavamoatModuleRecord * > + * moduleToPackageFallback?: (specifier: string) => string | undefined * }} opts * @returns */ -function aggregateDeps({ packageModules, moduleIdToModuleRecord }) { +function aggregateDeps({ + packageModules, + moduleIdToModuleRecord, + moduleToPackageFallback = guessPackageName, +}) { const deps = new Set() // get all dep package from the "packageModules" collection of modules Object.values(packageModules).forEach((moduleRecord) => { @@ -477,7 +487,8 @@ function aggregateDeps({ packageModules, moduleIdToModuleRecord }) { return } // moduleRecord missing, guess package name - const packageName = guessPackageName(requestedName) + const packageName = + moduleToPackageFallback(requestedName) || `` deps.add(packageName) } ) @@ -494,13 +505,13 @@ function aggregateDeps({ packageModules, moduleIdToModuleRecord }) { * because resolution was skipped for that module * * @param {string} requestedName - * @returns {string} + * @returns {string | undefined} */ function guessPackageName(requestedName) { const isNotPackageName = requestedName.startsWith('/') || requestedName.startsWith('.') if (isNotPackageName) { - return `` + return } // resolving is skipped so guess package name const pathParts = requestedName.split('/') @@ -530,7 +541,9 @@ function getDefaultPaths(policyName) { * @callback GeneratePolicyFn * @param {Partial & { * policyOverride?: import('./schema').LavaMoatPolicyOverrides + * moduleToPackageFallback?: (value: string) => string | undefined * }} opts + * * @returns {import('./schema').LavaMoatPolicy * | import('./schema').LavaMoatPolicyDebug} */ @@ -545,6 +558,7 @@ function getDefaultPaths(policyName) { * @typedef ModuleInspectorOptions * @property {(value: string) => boolean} isBuiltin * @property {boolean} [includeDebugInfo] + * @property {(specifier: string) => string | undefined} [moduleToPackageFallback] */ /** diff --git a/packages/webpack/src/buildtime/policyGenerator.js b/packages/webpack/src/buildtime/policyGenerator.js index 82e13eade1..f4bc2d8d83 100644 --- a/packages/webpack/src/buildtime/policyGenerator.js +++ b/packages/webpack/src/buildtime/policyGenerator.js @@ -73,6 +73,10 @@ module.exports = { const moduleInspector = createModuleInspector({ isBuiltin: () => false, includeDebugInfo: false, + // If the specifier is requested as a dependency in importMap but was never passed to inspectModule, its package name will be looked up here. + // This is a workaround to inconsistencies in how webpack represents connections. We're not aware of any security implications of this, since the package is already resolved clearly and this is only a part of policy generation, not runtime. + moduleToPackageFallback: (specifier) => + getPackageNameForModulePath(canonicalNameMap, specifier), }) return { From e38f10c693a19b04bf47db5ab71be378d1a7054a Mon Sep 17 00:00:00 2001 From: naugtur Date: Wed, 20 Mar 2024 20:00:51 +0100 Subject: [PATCH 2/2] chore(core): improved types --- packages/core/src/generatePolicy.js | 30 ++++++++++--------- .../webpack/src/buildtime/policyGenerator.js | 8 +++-- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/core/src/generatePolicy.js b/packages/core/src/generatePolicy.js index 6e634eb04e..70d4e5183e 100644 --- a/packages/core/src/generatePolicy.js +++ b/packages/core/src/generatePolicy.js @@ -449,18 +449,21 @@ function createModuleInspector(opts) { } /** - * @param {{ - * packageModules: Record< - * string, - * import('./moduleRecord').LavamoatModuleRecord - * > - * moduleIdToModuleRecord: Map< - * string, - * import('./moduleRecord').LavamoatModuleRecord - * > - * moduleToPackageFallback?: (specifier: string) => string | undefined - * }} opts - * @returns + * @callback ModuleToPackageFallbackFn + * @param {string} requestedName + * @returns {string | undefined} + */ + +/** + * @typedef {Object} AggregateDepsOptions + * @property {Record} packageModules + * @property {Map} moduleIdToModuleRecord + * @property {ModuleToPackageFallbackFn} [moduleToPackageFallback] + */ + +/** + * @param {AggregateDepsOptions} opts + * @returns {string[]} */ function aggregateDeps({ packageModules, @@ -504,8 +507,7 @@ function aggregateDeps({ * For when you encounter a `requestedName` that was not inspected, likely * because resolution was skipped for that module * - * @param {string} requestedName - * @returns {string | undefined} + * @type {ModuleToPackageFallbackFn} */ function guessPackageName(requestedName) { const isNotPackageName = diff --git a/packages/webpack/src/buildtime/policyGenerator.js b/packages/webpack/src/buildtime/policyGenerator.js index f4bc2d8d83..c07dd38a11 100644 --- a/packages/webpack/src/buildtime/policyGenerator.js +++ b/packages/webpack/src/buildtime/policyGenerator.js @@ -52,7 +52,7 @@ module.exports = { if (policyFromOptions) { // TODO: avoid loading the policy file if policyFromOptions is present final = policyFromOptions - } else { + } else if (policy) { final = applyOverride(policy) } if (emit) { @@ -67,14 +67,16 @@ module.exports = { // load policy file // load overrides - // generate policy if requested and write to file + // generate policy if requested and write to fileWe're not aware of any // merge result with overrides // return that and emit snapshot const moduleInspector = createModuleInspector({ isBuiltin: () => false, includeDebugInfo: false, // If the specifier is requested as a dependency in importMap but was never passed to inspectModule, its package name will be looked up here. - // This is a workaround to inconsistencies in how webpack represents connections. We're not aware of any security implications of this, since the package is already resolved clearly and this is only a part of policy generation, not runtime. + // This is a workaround to inconsistencies in how webpack represents connections. + // Specifically what happened to surface this issue: `connections` in webpack contain a module entry that's identified by the path to its index.mjs entrypoint while the index.js entrypoint is actually used in the bundle, so inspector doesn't have a cached entry for this one and without the fallback handler would return + // There should be no security implications of this, since the package is already resolved clearly and this is only a part of policy generation, not runtime. moduleToPackageFallback: (specifier) => getPackageNameForModulePath(canonicalNameMap, specifier), })