From 5e70a6e5f1a398c8c3e861064a71a31c2953b961 Mon Sep 17 00:00:00 2001 From: naugtur Date: Thu, 26 Oct 2023 14:29:28 +0200 Subject: [PATCH 1/6] test(compartment-mapper): add a test case for package exports support in policy attenuators --- .../node_modules/@ohmyscope/bob/package.json | 4 +++ .../fixtures-policy/node_modules/app/index.js | 3 ++- .../node_modules/myattenuator/package.json | 3 +++ .../compartment-mapper/test/test-policy.js | 25 +++++++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/compartment-mapper/test/fixtures-policy/node_modules/@ohmyscope/bob/package.json b/packages/compartment-mapper/test/fixtures-policy/node_modules/@ohmyscope/bob/package.json index 403578c640..cd78a27338 100644 --- a/packages/compartment-mapper/test/fixtures-policy/node_modules/@ohmyscope/bob/package.json +++ b/packages/compartment-mapper/test/fixtures-policy/node_modules/@ohmyscope/bob/package.json @@ -6,6 +6,10 @@ "dependencies": { "alice": "1.0.0" }, + "exports": { + ".": "./index.js", + "./nested-export": "./index.js" + }, "scripts": { "preinstall": "echo DO NOT INSTALL TEST FIXTURES; exit -1" } diff --git a/packages/compartment-mapper/test/fixtures-policy/node_modules/app/index.js b/packages/compartment-mapper/test/fixtures-policy/node_modules/app/index.js index 330734f100..b3d836ee01 100644 --- a/packages/compartment-mapper/test/fixtures-policy/node_modules/app/index.js +++ b/packages/compartment-mapper/test/fixtures-policy/node_modules/app/index.js @@ -1,7 +1,8 @@ import { alice, carol } from 'alice/alice.js'; import bob from './bob.cjs'; import scopedBob from '@ohmyscope/bob'; +import nestedScopedBob from '@ohmyscope/bob/nested-export'; import builtins from './builtinsWithPolicy.js'; import builtins2 from 'alice/secondaryBuitins'; -export { alice, bob, carol, builtins, builtins2, scopedBob }; +export { alice, bob, carol, builtins, builtins2, scopedBob, nestedScopedBob }; diff --git a/packages/compartment-mapper/test/fixtures-policy/node_modules/myattenuator/package.json b/packages/compartment-mapper/test/fixtures-policy/node_modules/myattenuator/package.json index 5b7c3d3b18..d3590f94da 100644 --- a/packages/compartment-mapper/test/fixtures-policy/node_modules/myattenuator/package.json +++ b/packages/compartment-mapper/test/fixtures-policy/node_modules/myattenuator/package.json @@ -4,6 +4,9 @@ "main": "./index.js", "type": "module", "dependencies": {}, + "exports": { + "./attenuate": "./index.js" + }, "scripts": { "preinstall": "echo DO NOT INSTALL TEST FIXTURES; exit -1" } diff --git a/packages/compartment-mapper/test/test-policy.js b/packages/compartment-mapper/test/test-policy.js index 3f63566711..5c37d79ced 100644 --- a/packages/compartment-mapper/test/test-policy.js +++ b/packages/compartment-mapper/test/test-policy.js @@ -98,6 +98,7 @@ const defaultExpectations = { redPill: 'undefined', purplePill: 'number', }, + nestedScopedBob: { scoped: 1 }, scopedBob: { scoped: 1 }, builtins: '{"a":1,"b":2,"default":{"a":1,"b":2}}', builtins2: '{"c":3,"default":{"c":3}}', @@ -306,6 +307,15 @@ const errorAttenuatorForAllGlobals = recursiveEdit((key, obj) => { } }); +const nestedAttenuator = recursiveEdit((key, obj) => { + if (key === 'attenuate') { + obj[key] = 'myattenuator/attenuate' + } + if (key === 'resources') { + obj[key]['myattenuator/attenuate'] = obj[key].myattenuator + } +}); + scaffold( 'policy - globals attenuator', test, @@ -403,3 +413,18 @@ scaffold( }, }, ); + +scaffold( + 'policy - nested export in attenuator', + test, + fixture, + combineAssertions( + makeResultAssertions(defaultExpectations), + assertNoPolicyBypassImport, + ), + 2, // expected number of assertions + { + addGlobals: globals, + policy: nestedAttenuator(policy), + }, +); \ No newline at end of file From 5d3a711b3f307f4e16221bba76b219b6f46dcab9 Mon Sep 17 00:00:00 2001 From: naugtur Date: Thu, 2 Nov 2023 12:29:06 +0100 Subject: [PATCH 2/6] fix(compartment-mapper): fix archive producing invalid compartment maps --- packages/compartment-mapper/src/archive.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/compartment-mapper/src/archive.js b/packages/compartment-mapper/src/archive.js index 17f10f75d1..5e9644aa90 100644 --- a/packages/compartment-mapper/src/archive.js +++ b/packages/compartment-mapper/src/archive.js @@ -145,13 +145,16 @@ const translateCompartmentMap = (compartments, sources, compartmentRenames) => { if (compartment.modules) { for (const name of keys(compartmentModules).sort()) { const module = compartmentModules[name]; - if (module.compartment !== undefined) { + if ( + module.compartment !== undefined && + compartmentRenames[module.compartment] !== undefined + ) { modules[name] = { ...module, compartment: compartmentRenames[module.compartment], }; } else { - modules[name] = module; + // If compartment is not defined, the module must not be part of the compartment map. } } } From bc5a0aed2796ca601d1970f42defdd37b08cc85b Mon Sep 17 00:00:00 2001 From: naugtur Date: Thu, 2 Nov 2023 12:29:45 +0100 Subject: [PATCH 3/6] fix(compartment-mapper): policy - allow any packages imported in the attenuators compartment --- packages/compartment-mapper/src/policy.js | 5 +---- packages/compartment-mapper/test/test-policy.js | 6 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js index 042c5c0001..12146b9083 100644 --- a/packages/compartment-mapper/src/policy.js +++ b/packages/compartment-mapper/src/policy.js @@ -154,10 +154,7 @@ export const getPolicyForPackage = (namingKit, policy) => { if (canonicalName === ATTENUATORS_COMPARTMENT) { return { defaultAttenuator: policy.defaultAttenuator, - packages: detectAttenuators(policy).reduce((packages, specifier) => { - packages[specifier] = true; - return packages; - }, {}), + packages: 'any', }; } if (policy.resources && policy.resources[canonicalName]) { diff --git a/packages/compartment-mapper/test/test-policy.js b/packages/compartment-mapper/test/test-policy.js index 5c37d79ced..b2a80aa110 100644 --- a/packages/compartment-mapper/test/test-policy.js +++ b/packages/compartment-mapper/test/test-policy.js @@ -309,10 +309,10 @@ const errorAttenuatorForAllGlobals = recursiveEdit((key, obj) => { const nestedAttenuator = recursiveEdit((key, obj) => { if (key === 'attenuate') { - obj[key] = 'myattenuator/attenuate' + obj[key] = 'myattenuator/attenuate'; } if (key === 'resources') { - obj[key]['myattenuator/attenuate'] = obj[key].myattenuator + obj[key]['myattenuator/attenuate'] = obj[key].myattenuator; } }); @@ -427,4 +427,4 @@ scaffold( addGlobals: globals, policy: nestedAttenuator(policy), }, -); \ No newline at end of file +); From b03efc27dbd0a3f9fb3182f89f27895c87de4e56 Mon Sep 17 00:00:00 2001 From: naugtur Date: Thu, 23 Nov 2023 15:16:10 +0100 Subject: [PATCH 4/6] feat(compartment-mapper): allow skipping powerless packages in policy resources WIP --- packages/compartment-mapper/src/archive.js | 7 +- .../compartment-mapper/src/import-archive.js | 1 + .../compartment-mapper/src/import-hook.js | 1 + packages/compartment-mapper/src/link.js | 26 ++--- .../compartment-mapper/src/node-modules.js | 5 - packages/compartment-mapper/src/policy.js | 106 +++--------------- .../test/snapshots/test-policy.js.md | 84 +------------- .../test/snapshots/test-policy.js.snap | Bin 919 -> 624 bytes .../compartment-mapper/test/test-policy.js | 83 +++++--------- 9 files changed, 65 insertions(+), 248 deletions(-) diff --git a/packages/compartment-mapper/src/archive.js b/packages/compartment-mapper/src/archive.js index 5e9644aa90..17f10f75d1 100644 --- a/packages/compartment-mapper/src/archive.js +++ b/packages/compartment-mapper/src/archive.js @@ -145,16 +145,13 @@ const translateCompartmentMap = (compartments, sources, compartmentRenames) => { if (compartment.modules) { for (const name of keys(compartmentModules).sort()) { const module = compartmentModules[name]; - if ( - module.compartment !== undefined && - compartmentRenames[module.compartment] !== undefined - ) { + if (module.compartment !== undefined) { modules[name] = { ...module, compartment: compartmentRenames[module.compartment], }; } else { - // If compartment is not defined, the module must not be part of the compartment map. + modules[name] = module; } } } diff --git a/packages/compartment-mapper/src/import-archive.js b/packages/compartment-mapper/src/import-archive.js index df61f0664a..cc51a9b34b 100644 --- a/packages/compartment-mapper/src/import-archive.js +++ b/packages/compartment-mapper/src/import-archive.js @@ -95,6 +95,7 @@ const makeArchiveImportHookMaker = ( // module is a "builtin" module and the policy needs to be enforced. enforceModulePolicy(moduleSpecifier, compartmentDescriptor, { exit: true, + errorHint: 'The module was not in the archive and an attempt was made to load it as a builtin', }); const record = await exitModuleImportHook(moduleSpecifier); if (record) { diff --git a/packages/compartment-mapper/src/import-hook.js b/packages/compartment-mapper/src/import-hook.js index e3a58029f8..6bba911e77 100644 --- a/packages/compartment-mapper/src/import-hook.js +++ b/packages/compartment-mapper/src/import-hook.js @@ -220,6 +220,7 @@ export const makeImportHookMaker = ( // hook returns something. Otherwise, we need to fall back to the 'cannot find' error below. enforceModulePolicy(moduleSpecifier, compartmentDescriptor, { exit: true, + errorHint: 'The module was not in the compartment map and an attempt was made to load it as a builtin', }); if (archiveOnly) { // Return a place-holder. diff --git a/packages/compartment-mapper/src/link.js b/packages/compartment-mapper/src/link.js index 3ae1fcaf75..8decfda31b 100644 --- a/packages/compartment-mapper/src/link.js +++ b/packages/compartment-mapper/src/link.js @@ -19,7 +19,6 @@ import { parseExtension } from './extension.js'; import { enforceModulePolicy, ATTENUATORS_COMPARTMENT, - diagnoseMissingCompartmentError, attenuateGlobals, makeDeferredAttenuatorsProvider, } from './policy.js'; @@ -236,6 +235,7 @@ const makeModuleMapHook = ( // archive goes through foreignModuleSpecifier for local modules too enforceModulePolicy(moduleSpecifier, compartmentDescriptor, { exit: false, + errorHint: `TODO`, // TODO: add a testcase for this and provide a helpful hint }); } @@ -244,12 +244,7 @@ const makeModuleMapHook = ( throw Error( `Cannot import from missing compartment ${q( foreignCompartmentName, - )}${diagnoseMissingCompartmentError({ - moduleSpecifier, - compartmentDescriptor, - foreignModuleSpecifier, - foreignCompartmentName, - })}`, + )}}`, ); } return foreignCompartment.module(foreignModuleSpecifier); @@ -279,20 +274,19 @@ const makeModuleMapHook = ( throw Error( `Cannot import from missing compartment ${q( foreignCompartmentName, - )}${diagnoseMissingCompartmentError({ - moduleSpecifier, - compartmentDescriptor, - foreignModuleSpecifier, - foreignCompartmentName, - })}`, + )}`, ); } - // Despite all non-exit modules not allowed by policy being dropped - // while building the graph, this check is necessary because module - // is written back to the compartment map below. enforceModulePolicy(scopePrefix, compartmentDescriptor, { exit: false, + errorHint: ` There is an alias defined that causes another package to be imported as ${q( + moduleSpecifier, + )}. Package ${q(moduleSpecifier)} resolves to ${q( + foreignModuleSpecifier, + )} in ${q( + foreignCompartmentName, + )}.`, }); // The following line is weird. // Information is flowing backward. diff --git a/packages/compartment-mapper/src/node-modules.js b/packages/compartment-mapper/src/node-modules.js index 611634faa2..1c5cff649b 100644 --- a/packages/compartment-mapper/src/node-modules.js +++ b/packages/compartment-mapper/src/node-modules.js @@ -611,11 +611,6 @@ const translateGraph = ( }, policy, ); - // do not include compartments for packages not covered by policy - if (policy && !packagePolicy) { - // eslint-disable-next-line no-continue - continue; - } /** * @param {string} dependencyName diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js index 12146b9083..5392c079f9 100644 --- a/packages/compartment-mapper/src/policy.js +++ b/packages/compartment-mapper/src/policy.js @@ -113,29 +113,6 @@ export const dependencyAllowedByPolicy = (namingKit, packagePolicy) => { return !!policyLookupHelper(packagePolicy, 'packages', canonicalName); }; -const validateDependencies = (policy, canonicalName) => { - const packages = policy.resources[canonicalName].packages; - if (!packages || isAllowingEverything(packages)) { - return; - } - - const packageNames = keys(packages); - const attenuators = detectAttenuators(policy); - // Join attenuators with packageNames into a Set to deduplicate and check if all are listed in policy.resources - const allSpecifiers = new Set([...packageNames, ...attenuators]); - for (const specifier of allSpecifiers) { - if (!(specifier in policy.resources)) { - throw Error( - `Package ${q(specifier)} is allowed for ${q( - canonicalName, - )} to import but its policy is not defined. Please add a policy for ${q( - specifier, - )}`, - ); - } - } -}; - /** * Returns the policy applicable to the canonicalName of the package * @@ -157,14 +134,11 @@ export const getPolicyForPackage = (namingKit, policy) => { packages: 'any', }; } - if (policy.resources && policy.resources[canonicalName]) { - validateDependencies(policy, canonicalName); + if (policy.resources && policy.resources[canonicalName] !== undefined) { return policy.resources[canonicalName]; } else { - console.warn( - `No policy for '${canonicalName}', omitting from compartment map.`, - ); - return undefined; + // Allow skipping policy entries for packages with no powers. + return {}; } }; @@ -354,6 +328,14 @@ export const attenuateGlobals = ( freezeGlobalThisUnlessOptedOut(); }; + +const diagnoseModulePolicy = (errorHint) => { + if(!errorHint) { + return ''; + } + return ` (${errorHint})`; +} + /** * Throws if importing of the specifier is not allowed by the policy * @@ -361,18 +343,18 @@ export const attenuateGlobals = ( * @param {object} compartmentDescriptor * @param {object} [info] */ -export const enforceModulePolicy = (specifier, compartmentDescriptor, info) => { +export const enforceModulePolicy = (specifier, compartmentDescriptor, info = {}) => { const { policy, modules } = compartmentDescriptor; if (!policy) { return; } - + if (!info.exit) { if (!modules[specifier]) { throw Error( `Importing ${q(specifier)} was not allowed by policy packages:${q( policy.packages, - )}`, + )}${diagnoseModulePolicy(info.errorHint)}`, ); } return; @@ -382,7 +364,7 @@ export const enforceModulePolicy = (specifier, compartmentDescriptor, info) => { throw Error( `Importing ${q(specifier)} was not allowed by policy builtins:${q( policy.builtins, - )}`, + )}${diagnoseModulePolicy(info.errorHint)}`, ); } }; @@ -456,61 +438,3 @@ export const attenuateModuleHook = async ( originalModuleRecord, }); }; - -const padDiagnosis = text => ` (${text})`; -/** - * Provide dignostic information for a missing compartment error - * - * @param {object} args - * @param {string} args.moduleSpecifier - * @param {object} args.compartmentDescriptor - * @param {string} args.foreignModuleSpecifier - * @param {string} args.foreignCompartmentName - * @returns {string} - */ -export const diagnoseMissingCompartmentError = ({ - moduleSpecifier, - compartmentDescriptor, - foreignModuleSpecifier, - foreignCompartmentName, -}) => { - const { policy, name, scopes } = compartmentDescriptor; - - if (policy) { - if (!policy.packages) { - return padDiagnosis( - `There were no allowed packages specified in policy for ${q(name)}`, - ); - } - if (name === ATTENUATORS_COMPARTMENT) { - return padDiagnosis( - `Attenuator ${q( - moduleSpecifier, - )} was imported but there is no policy resources entry defined for it.`, - ); - } - - const scopeNames = entries(scopes) - .filter(([_name, scope]) => scope.compartment === foreignCompartmentName) - .map(([scopeName]) => scopeName); - if (scopeNames.length === 1 && scopeNames[0] === moduleSpecifier) { - return padDiagnosis( - `Package ${q( - moduleSpecifier, - )} is missing. Are you sure there is an entry in policy resources specified for it?`, - ); - } else { - return padDiagnosis( - `Package ${q(moduleSpecifier)} resolves to ${q( - foreignModuleSpecifier, - )} in ${q( - foreignCompartmentName, - )} which seems disallowed by policy. There is likely an override defined that causes another package to be imported as ${q( - moduleSpecifier, - )}.`, - ); - } - } - // Omit diagnostics when parent package had no policy - it means there was no policy. - return ''; -}; diff --git a/packages/compartment-mapper/test/snapshots/test-policy.js.md b/packages/compartment-mapper/test/snapshots/test-policy.js.md index 633219e399..ffbb95e400 100644 --- a/packages/compartment-mapper/test/snapshots/test-policy.js.md +++ b/packages/compartment-mapper/test/snapshots/test-policy.js.md @@ -4,113 +4,41 @@ The actual snapshot is saved in `test-policy.js.snap`. Generated by [AVA](https://avajs.dev). -## policy - insufficient policy detected early / loadLocation - -> Snapshot 1 - - 'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"' - -## policy - insufficient policy detected early / importLocation - -> Snapshot 1 - - 'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"' - -## policy - insufficient policy detected early / makeArchive / parseArchive - -> Snapshot 1 - - 'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"' - -## policy - insufficient policy detected early / makeArchive / parseArchive with a prefix - -> Snapshot 1 - - 'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"' - -## policy - insufficient policy detected early / writeArchive / loadArchive - -> Snapshot 1 - - 'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"' - -## policy - insufficient policy detected early / writeArchive / importArchive - -> Snapshot 1 - - 'Package "alice>carol" is allowed for "alice" to import but its policy is not defined. Please add a policy for "alice>carol"' - -## policy - malfunction resulting in missing compartment / loadLocation - -> Snapshot 1 - - 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)' - -## policy - malfunction resulting in missing compartment / importLocation - -> Snapshot 1 - - 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)' - -## policy - malfunction resulting in missing compartment / makeArchive / parseArchive - -> Snapshot 1 - - 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)' - -## policy - malfunction resulting in missing compartment / makeArchive / parseArchive with a prefix - -> Snapshot 1 - - 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)' - -## policy - malfunction resulting in missing compartment / writeArchive / loadArchive - -> Snapshot 1 - - 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)' - -## policy - malfunction resulting in missing compartment / writeArchive / importArchive - -> Snapshot 1 - - 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/" (Package "carol" is missing. Are you sure there is an entry in policy resources specified for it?)' - ## policy - attack - browser alias / loadLocation > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' ## policy - attack - browser alias / importLocation > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' ## policy - attack - browser alias / makeArchive / parseArchive > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' ## policy - attack - browser alias / makeArchive / parseArchive with a prefix > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' ## policy - attack - browser alias / writeArchive / loadArchive > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' ## policy - attack - browser alias / writeArchive / importArchive > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Cannot import from missing compartment "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" (Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/" which seems disallowed by policy. There is likely an override defined that causes another package to be imported as "dan".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' ## policy - attenuator error aggregation / loadLocation diff --git a/packages/compartment-mapper/test/snapshots/test-policy.js.snap b/packages/compartment-mapper/test/snapshots/test-policy.js.snap index 2c881675d20163da1c0df07fffe1f7bf6965035e..ba57a1335e5a5dedf48b9dd5cd2c5c339ef9e6e2 100644 GIT binary patch literal 624 zcmV-$0+0PcRzVahX^5o_@f{WJ@mo}P3*C~ zWW8(cuIogIL+^b9PUT5Ewv)7+oYd;Ul5>dc?98{{%x_nj>tv}+v~pMf$V;zi9TY~% zXah4q?=f2O!_u0{kp)x|9fY6^#@|hZUK%Z(#=>RByUXi1Mvp(He;-etPU_F}$>iCi ztLYb%iem7lNrc?Qr6LeP*lrN~ass4=f}b(vg@nWBVHgBK7@534>vPh6mSa(n4Lvy@ zCUWgdOK#Sh4Yi5sXDjSNjtd2xyn<54WYtFM6cYAOC(L1y7smSfHxZ*2P;p7xcrH$v zift}8uQW8SfzdCKSPo9p#tZlv}GK=RtFR*UdS{6@9Q# zCNGHJcvrW39hnc3`L{mIp^`oWgu-&&>+u8(@&p`k>nkh$og+}Q8*lxPnZG}pdFyi~wUjo)TldWV=0DcYr(cy>qH=rHTBMQX&VO#?tBk9;U1tvI zHYed#!3~W_wErd|-`u(ra}bMt9-YF1+sy|x5T}PJM`(?D5M}0YobHao+e0aI&eS7^ zJ{&$C%HgP)cR);z?%cz%-Z44r&!lrE9SQXzbnvY?YGxdn^kMSpo=m!Cx{=C#sN4Wp K2qMWP6#xJTkuxp; literal 919 zcmV;I18Dp~RzV*Yb3{0as=n+f5|EahysCaSu`ctY_w%-?#RTdKLDB^oRQU{U2z! z5qQx}q~QJ-c7Vq^4MXm8j1A=9258VX7yvRQ#^8aFEO;w@X1I*Gj#;7y(&)>pAYg`l z_;!2i?w0-Q+}XO<`FhIyA?qVJjDq2w&y*Ana1BfdIijecRQZn%7zsQ|q%zP;4RE82 z8fbki4V4n|7y}nh1u~7mf&f^t<~mTGrI5E9mHv^kcQ5_fSAiXB%k>X}byWO^H#QCI=*hxWaB=?|{8!-{4v2Slapirm6 z@VHM+fQV~t-}*8lADhUwqvCG-v2!`C^p@%zGA>9EiLe(!B!g6taM$B;fEVr;?buXF z(U6cH?F^NS{wT6}6}I8CUuE65rIqZjspmDgu&)rtG6hYM!3>aIttn!l>}s4f zYC???%T)QOK_}?*kmsJ^=Eb9e@Zq9_D;`KKSKd9M!dvnBEV_u;f9zB;e`$83b`o45%TFn@L|{veAJ11JH<(hJfp0n(K{o54dnv7)0{us53m^I$VAM5&r*%nN78z#d`{|^KpDO2*)?J(Jt~u^jwdSAJPn+$hz1n`Nny*?%ZMLJHPCE+Ylo=ZPC;`2) zeqUifJBU}$cF$&f^q_MjWRD45Mzuk*utfEB7M{eDv&!EIX+OVa`aewB6V{2w*VQ>) zaE{CCa2t-P1GEWTXM4WL`Jj9Qk;wt|;uaL1&r6|lK598M;qY=^4z*^CHDXe`b2rDj tU~<}=N#!}cCDeq_+`FdMS+`} @@ -159,58 +166,6 @@ scaffold( }, ); -scaffold( - 'policy - insufficient policy detected early', - test, - fixture, - assertTestAlwaysThrows, - 2, // expected number of assertions - { - shouldFailBeforeArchiveOperations: true, - onError: (t, { error }) => { - t.regex(error.message, /carol.*policy.*add/); - t.snapshot(sanitizePaths(error.message)); - }, - addGlobals: globals, - policy: { - resources: { - '': { - ...policy.resources[''], - }, - alice: { - ...policy.resources.alice, - }, - }, - }, - tags: new Set(['browser']), - }, -); - -scaffold( - 'policy - malfunction resulting in missing compartment', - test, - fixture, - assertTestAlwaysThrows, - 2, // expected number of assertions - { - shouldFailBeforeArchiveOperations: true, - onError: (t, { error }) => { - t.regex(error.message, /carol.*is missing.*policy/); - t.snapshot(sanitizePaths(error.message)); - }, - addGlobals: globals, - policy: { - entry: policy.entry, - resources: { - ...policy.resources, - // not something that can would normally be specified, but passes policy validation while triggering an error later. - 'alice>carol': undefined, - }, - }, - tags: new Set(['browser']), - }, -); - scaffold( 'policy - attack - browser alias', test, @@ -220,7 +175,7 @@ scaffold( { shouldFailBeforeArchiveOperations: true, onError: (t, { error }) => { - t.regex(error.message, /dan.*hackity.*disallowed/); + t.regex(error.message, /dan.*alias.*hackity/); t.snapshot(sanitizePaths(error.message)); }, addGlobals: globals, @@ -279,6 +234,15 @@ const recursiveEdit = editor => originalPolicy => { }; return recur(policyToAlter); }; +const mutationEdit = editor => originalPolicy => { + const policyToAlter = JSON.parse(JSON.stringify(originalPolicy)); + editor(policyToAlter) + return policyToAlter; +} + +const skipCarol = mutationEdit((policyToAlter) => { + policyToAlter.resources['alice>carol'] = undefined; +}); const addAttenuatorForAllGlobals = recursiveEdit((key, obj) => { if (key === 'globals') { @@ -316,6 +280,19 @@ const nestedAttenuator = recursiveEdit((key, obj) => { } }); + +scaffold( + 'policy - allow skipping policy entries for powerless compartments', + test, + fixture, + makeResultAssertions(powerlessCarolExpectations), + 1, // expected number of assertions + { + addGlobals: globals, + policy: skipCarol(policy), + }, +); + scaffold( 'policy - globals attenuator', test, From d19afd2f9a194f554c7218f7190b0070d3e9278a Mon Sep 17 00:00:00 2001 From: naugtur Date: Fri, 24 Nov 2023 16:03:56 +0100 Subject: [PATCH 5/6] fix(compartment-mapper): correct error interpretations, negative policy enforcement test --- .../compartment-mapper/src/import-archive.js | 4 +- .../compartment-mapper/src/import-hook.js | 4 +- packages/compartment-mapper/src/link.js | 13 ++-- packages/compartment-mapper/src/policy.js | 26 ++++---- .../test/snapshots/test-policy.js.md | 60 ++++++++++++++---- .../test/snapshots/test-policy.js.snap | Bin 624 -> 788 bytes .../compartment-mapper/test/test-policy.js | 40 +++++++++--- 7 files changed, 108 insertions(+), 39 deletions(-) diff --git a/packages/compartment-mapper/src/import-archive.js b/packages/compartment-mapper/src/import-archive.js index cc51a9b34b..c3248cfdad 100644 --- a/packages/compartment-mapper/src/import-archive.js +++ b/packages/compartment-mapper/src/import-archive.js @@ -95,7 +95,9 @@ const makeArchiveImportHookMaker = ( // module is a "builtin" module and the policy needs to be enforced. enforceModulePolicy(moduleSpecifier, compartmentDescriptor, { exit: true, - errorHint: 'The module was not in the archive and an attempt was made to load it as a builtin', + errorHint: `Blocked in loading. ${q( + moduleSpecifier, + )} was not in the archive and an attempt was made to load it as a builtin`, }); const record = await exitModuleImportHook(moduleSpecifier); if (record) { diff --git a/packages/compartment-mapper/src/import-hook.js b/packages/compartment-mapper/src/import-hook.js index 6bba911e77..af58c33b70 100644 --- a/packages/compartment-mapper/src/import-hook.js +++ b/packages/compartment-mapper/src/import-hook.js @@ -220,7 +220,9 @@ export const makeImportHookMaker = ( // hook returns something. Otherwise, we need to fall back to the 'cannot find' error below. enforceModulePolicy(moduleSpecifier, compartmentDescriptor, { exit: true, - errorHint: 'The module was not in the compartment map and an attempt was made to load it as a builtin', + errorHint: `Blocked in loading. ${q( + moduleSpecifier, + )} was not in the compartment map and an attempt was made to load it as a builtin`, }); if (archiveOnly) { // Return a place-holder. diff --git a/packages/compartment-mapper/src/link.js b/packages/compartment-mapper/src/link.js index 8decfda31b..302e439379 100644 --- a/packages/compartment-mapper/src/link.js +++ b/packages/compartment-mapper/src/link.js @@ -231,11 +231,14 @@ const makeModuleMapHook = ( return undefined; // fall through to import hook } if (foreignModuleSpecifier !== undefined) { + // archive goes through foreignModuleSpecifier for local modules too if (!moduleSpecifier.startsWith('./')) { - // archive goes through foreignModuleSpecifier for local modules too + // This code path seems to only be reached on subsequent imports of the same specifier in the same compartment. + // The check should be redundant and is only left here out of abundance of caution. enforceModulePolicy(moduleSpecifier, compartmentDescriptor, { exit: false, - errorHint: `TODO`, // TODO: add a testcase for this and provide a helpful hint + errorHint: + 'This check should not be reachable. If you see this error, please file an issue.', }); } @@ -280,11 +283,9 @@ const makeModuleMapHook = ( enforceModulePolicy(scopePrefix, compartmentDescriptor, { exit: false, - errorHint: ` There is an alias defined that causes another package to be imported as ${q( + errorHint: `Blocked in linking. ${q( moduleSpecifier, - )}. Package ${q(moduleSpecifier)} resolves to ${q( - foreignModuleSpecifier, - )} in ${q( + )} is part of the compartment map and resolves to ${q( foreignCompartmentName, )}.`, }); diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js index 5392c079f9..e0eb1488ae 100644 --- a/packages/compartment-mapper/src/policy.js +++ b/packages/compartment-mapper/src/policy.js @@ -328,31 +328,35 @@ export const attenuateGlobals = ( freezeGlobalThisUnlessOptedOut(); }; - -const diagnoseModulePolicy = (errorHint) => { - if(!errorHint) { +const diagnoseModulePolicy = errorHint => { + if (!errorHint) { return ''; } - return ` (${errorHint})`; -} - + return ` (info: ${errorHint})`; +}; /** * Throws if importing of the specifier is not allowed by the policy * * @param {string} specifier - * @param {object} compartmentDescriptor + * @param {import('./types.js').CompartmentDescriptor} compartmentDescriptor * @param {object} [info] */ -export const enforceModulePolicy = (specifier, compartmentDescriptor, info = {}) => { - const { policy, modules } = compartmentDescriptor; +export const enforceModulePolicy = ( + specifier, + compartmentDescriptor, + info = {}, +) => { + const { policy, modules, label } = compartmentDescriptor; if (!policy) { return; } - + if (!info.exit) { if (!modules[specifier]) { throw Error( - `Importing ${q(specifier)} was not allowed by policy packages:${q( + `Importing ${q(specifier)} in ${q( + label, + )} was not allowed by packages policy ${q( policy.packages, )}${diagnoseModulePolicy(info.errorHint)}`, ); diff --git a/packages/compartment-mapper/test/snapshots/test-policy.js.md b/packages/compartment-mapper/test/snapshots/test-policy.js.md index ffbb95e400..a9c594bb84 100644 --- a/packages/compartment-mapper/test/snapshots/test-policy.js.md +++ b/packages/compartment-mapper/test/snapshots/test-policy.js.md @@ -4,41 +4,77 @@ The actual snapshot is saved in `test-policy.js.snap`. Generated by [AVA](https://avajs.dev). -## policy - attack - browser alias / loadLocation +## policy - attack - browser alias - with alias hint / loadLocation > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" in "eve-v1.0.0" was not allowed by packages policy {"dan":true} (info: Blocked in linking. "dan" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' -## policy - attack - browser alias / importLocation +## policy - attack - browser alias - with alias hint / importLocation > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" in "eve-v1.0.0" was not allowed by packages policy {"dan":true} (info: Blocked in linking. "dan" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' -## policy - attack - browser alias / makeArchive / parseArchive +## policy - attack - browser alias - with alias hint / makeArchive / parseArchive > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" in "eve-v1.0.0" was not allowed by packages policy {"dan":true} (info: Blocked in linking. "dan" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' -## policy - attack - browser alias / makeArchive / parseArchive with a prefix +## policy - attack - browser alias - with alias hint / makeArchive / parseArchive with a prefix > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" in "eve-v1.0.0" was not allowed by packages policy {"dan":true} (info: Blocked in linking. "dan" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' -## policy - attack - browser alias / writeArchive / loadArchive +## policy - attack - browser alias - with alias hint / writeArchive / loadArchive > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" in "eve-v1.0.0" was not allowed by packages policy {"dan":true} (info: Blocked in linking. "dan" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' -## policy - attack - browser alias / writeArchive / importArchive +## policy - attack - browser alias - with alias hint / writeArchive / importArchive > Snapshot 1 - 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" was not allowed by policy packages:{"dan":true} ( There is an alias defined that causes another package to be imported as "dan". Package "dan" resolves to "." in "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' + 'Failed to load module "./attack.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (1 underlying failures: Importing "dan" in "eve-v1.0.0" was not allowed by packages policy {"dan":true} (info: Blocked in linking. "dan" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/hackity/".)' + +## policy - disallowed package with error hint / loadLocation + +> Snapshot 1 + + 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (2 underlying failures: Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".), Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".)' + +## policy - disallowed package with error hint / importLocation + +> Snapshot 1 + + 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (2 underlying failures: Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".), Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".)' + +## policy - disallowed package with error hint / makeArchive / parseArchive + +> Snapshot 1 + + 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (2 underlying failures: Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".), Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".)' + +## policy - disallowed package with error hint / makeArchive / parseArchive with a prefix + +> Snapshot 1 + + 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (2 underlying failures: Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".), Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".)' + +## policy - disallowed package with error hint / writeArchive / loadArchive + +> Snapshot 1 + + 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (2 underlying failures: Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".), Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".)' + +## policy - disallowed package with error hint / writeArchive / importArchive + +> Snapshot 1 + + 'Failed to load module "./index.js" in package "file://.../compartment-mapper/test/fixtures-policy/node_modules/app/" (2 underlying failures: Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".), Importing "carol" in "alice-v1.0.0" was not allowed by packages policy {"alice>carol":false} (info: Blocked in linking. "carol" is part of the compartment map and resolves to "file://.../compartment-mapper/test/fixtures-policy/node_modules/alice/node_modules/carol/".)' ## policy - attenuator error aggregation / loadLocation diff --git a/packages/compartment-mapper/test/snapshots/test-policy.js.snap b/packages/compartment-mapper/test/snapshots/test-policy.js.snap index ba57a1335e5a5dedf48b9dd5cd2c5c339ef9e6e2..2848cf1ebcbe596d2dc05c8b1ee51aa5bb4ec330 100644 GIT binary patch literal 788 zcmV+v1MB=jRzVNdv4+vZSQ+?QPT&D;shpbvwkXuef zBsI}y=O2p*00000000B+na_*UKorNf74htU@W??{*e1K89)gG#f31jk5(Gh}$)p)O znF%w~rd2$6^y+`GUi=UI`+P~7ZVQz(X)6S#r%W>Q=DqiMU((6Dibh-p6aDiSHOz22 zf09bhf&~nK7(;>yUW}B?G*v)2BO1pUGx2f|Ghx63E=l-C2E;Ham=+|}u{8SoRTvUO zKJ?{Zty(9?tU|IK64pcxZ)n|&CwGznL zS=D!k`MU!!FJJJMdUvS%E9xQBRbW-V3*J;IsdoFH-v6_~_jmuFd>Q6{ek{EAYVig{ zNnQfRat<0IE|xqlZTJ!4x`uVN+kd8AxVd?&@Fd57X0x%DjY)GG z*d~Wp&E(MJ)Mus9m13~c*d~qlhmuC!;n)hLE2UwDa)5V8lWz!C8eJ&{D~)Z^c)gc2 zYCg4Cadf2|tT?ubahX^5o_@f{WJ@mo}P3*C~ zWW8(cuIogIL+^b9PUT5Ewv)7+oYd;Ul5>dc?98{{%x_nj>tv}+v~pMf$V;zi9TY~% zXah4q?=f2O!_u0{kp)x|9fY6^#@|hZUK%Z(#=>RByUXi1Mvp(He;-etPU_F}$>iCi ztLYb%iem7lNrc?Qr6LeP*lrN~ass4=f}b(vg@nWBVHgBK7@534>vPh6mSa(n4Lvy@ zCUWgdOK#Sh4Yi5sXDjSNjtd2xyn<54WYtFM6cYAOC(L1y7smSfHxZ*2P;p7xcrH$v zift}8uQW8SfzdCKSPo9p#tZlv}GK=RtFR*UdS{6@9Q# zCNGHJcvrW39hnc3`L{mIp^`oWgu-&&>+u8(@&p`k>nkh$og+}Q8*lxPnZG}pdFyi~wUjo)TldWV=0DcYr(cy>qH=rHTBMQX&VO#?tBk9;U1tvI zHYed#!3~W_wErd|-`u(ra}bMt9-YF1+sy|x5T}PJM`(?D5M}0YobHao+e0aI&eS7^ zJ{&$C%HgP)cR);z?%cz%-Z44r&!lrE9SQXzbnvY?YGxdn^kMSpo=m!Cx{=C#sN4Wp K2qMWP6#xJTkuxp; diff --git a/packages/compartment-mapper/test/test-policy.js b/packages/compartment-mapper/test/test-policy.js index e9b6515fa1..0203b620c0 100644 --- a/packages/compartment-mapper/test/test-policy.js +++ b/packages/compartment-mapper/test/test-policy.js @@ -113,11 +113,14 @@ const anyExpectations = { const powerlessCarolExpectations = { namespace: { ...defaultExpectations.namespace, - carol: { bluePill: 'undefined', redPill: 'undefined', purplePill: 'undefined' }, + carol: { + bluePill: 'undefined', + redPill: 'undefined', + purplePill: 'undefined', + }, }, }; - const makeResultAssertions = expectations => async (t, { namespace }) => { @@ -167,7 +170,7 @@ scaffold( ); scaffold( - 'policy - attack - browser alias', + 'policy - attack - browser alias - with alias hint', test, fixtureAttack, assertTestAlwaysThrows, @@ -175,7 +178,8 @@ scaffold( { shouldFailBeforeArchiveOperations: true, onError: (t, { error }) => { - t.regex(error.message, /dan.*alias.*hackity/); + t.regex(error.message, /dan.*resolves.*hackity/); + // see the snapshot for the error hint in the message t.snapshot(sanitizePaths(error.message)); }, addGlobals: globals, @@ -236,14 +240,18 @@ const recursiveEdit = editor => originalPolicy => { }; const mutationEdit = editor => originalPolicy => { const policyToAlter = JSON.parse(JSON.stringify(originalPolicy)); - editor(policyToAlter) + editor(policyToAlter); return policyToAlter; -} +}; -const skipCarol = mutationEdit((policyToAlter) => { +const skipCarol = mutationEdit(policyToAlter => { policyToAlter.resources['alice>carol'] = undefined; }); +const disallowCarol = mutationEdit(policyToAlter => { + policyToAlter.resources.alice.packages['alice>carol'] = false; +}); + const addAttenuatorForAllGlobals = recursiveEdit((key, obj) => { if (key === 'globals') { obj[key] = { @@ -280,7 +288,6 @@ const nestedAttenuator = recursiveEdit((key, obj) => { } }); - scaffold( 'policy - allow skipping policy entries for powerless compartments', test, @@ -293,6 +300,23 @@ scaffold( }, ); +scaffold( + 'policy - disallowed package with error hint', + test, + fixture, + assertTestAlwaysThrows, + 2, // expected number of assertions + { + shouldFailBeforeArchiveOperations: true, + onError: (t, { error }) => { + t.regex(error.message, /Importing.*carol.*in.*alice.*not allowed/i); + t.snapshot(sanitizePaths(error.message)); + }, + addGlobals: globals, + policy: disallowCarol(policy), + }, +); + scaffold( 'policy - globals attenuator', test, From db2545b1d9b436baad597eea79154d9052e391f7 Mon Sep 17 00:00:00 2001 From: naugtur Date: Tue, 12 Dec 2023 09:11:07 +0100 Subject: [PATCH 6/6] fix(compartment-mapper): Turn empty policy into a null-prototype object --- packages/compartment-mapper/src/node-modules.js | 2 +- packages/compartment-mapper/src/policy-format.js | 2 +- packages/compartment-mapper/src/policy.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/compartment-mapper/src/node-modules.js b/packages/compartment-mapper/src/node-modules.js index 1c5cff649b..056070f12a 100644 --- a/packages/compartment-mapper/src/node-modules.js +++ b/packages/compartment-mapper/src/node-modules.js @@ -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} policy + * @param {object|undefined} policy * @returns {CompartmentMapDescriptor} */ const translateGraph = ( diff --git a/packages/compartment-mapper/src/policy-format.js b/packages/compartment-mapper/src/policy-format.js index 5f2f57e353..958ca2fb1f 100644 --- a/packages/compartment-mapper/src/policy-format.js +++ b/packages/compartment-mapper/src/policy-format.js @@ -122,7 +122,7 @@ export const assertPackagePolicy = (allegedPackagePolicy, path, url) => { const packagePolicy = Object(allegedPackagePolicy); assert( allegedPackagePolicy === packagePolicy && !isArray(allegedPackagePolicy), - `${path} must be an object, got ${allegedPackagePolicy}${inUrl}`, + `${path} must be an object, got ${q(allegedPackagePolicy)}${inUrl}`, ); const { packages, diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js index e0eb1488ae..8604bcb2ac 100644 --- a/packages/compartment-mapper/src/policy.js +++ b/packages/compartment-mapper/src/policy.js @@ -17,7 +17,7 @@ import { isAllowingEverything, } from './policy-format.js'; -const { entries, values, assign, keys, freeze } = Object; +const { create, entries, values, assign, keys, freeze } = Object; const q = JSON.stringify; /** @@ -138,7 +138,7 @@ export const getPolicyForPackage = (namingKit, policy) => { return policy.resources[canonicalName]; } else { // Allow skipping policy entries for packages with no powers. - return {}; + return create(null); } };