Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Policy conditions #34414

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions doc/api/policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,24 +124,25 @@ replaced.

```json
{
"builtins": [],
"resources": {
"./app/checked.js": {
"dependencies": {
"fs": true,
"os": "./app/node_modules/alt-os"
"os": "./app/node_modules/alt-os",
"http": { "import": true }
}
}
}
}
```

The dependencies are keyed by the requested string specifier and have values
of either `true` or a string pointing to a module that will be resolved.
The dependencies are keyed by the requested specifier string and have values
of either `true`, `null`, a string pointing to a module that will be resolved,
or a conditions object.

The specifier string does not perform any searching and must match exactly
what is provided to the `require()`. Therefore, multiple specifiers may be
needed in the policy if `require()` uses multiple different strings to point
what is provided to the `require()` or `import`. Therefore, multiple specifiers
may be needed in the policy if it uses multiple different strings to point
to the same module (such as excluding the extension).

If the value of the redirection is `true` the default searching algorithms will
Expand All @@ -150,20 +151,31 @@ be used to find the module.
If the value of the redirection is a string, it will be resolved relative to
the manifest and then immediately be used without searching.

Any specifier string that is `require()`ed and not listed in the dependencies
will result in an error according to the policy.
Any specifier string that is attempted to resolved and not listed in the
dependencies will result in an error according to the policy.

Redirection will not prevent access to APIs through means such as direct access
to `require.cache` and/or through `module.constructor` which allow access to
loading modules. Policy redirection only affect specifiers to `require()`.
Other means such as to prevent undesired access to APIs through variables are
necessary to lock down that path of loading modules.
loading modules. Policy redirection only affect specifiers to `require()` and
`import`. Other means such as to prevent undesired access to APIs through
variables are necessary to lock down that path of loading modules.

A boolean value of `true` for the dependencies map can be specified to allow a
module to load any specifier without redirection. This can be useful for local
development and may have some valid usage in production, but should be used
only with care after auditing a module to ensure its behavior is valid.

Similar to `"exports"` in `package.json` dependencies can also be specified to
be objects containing conditions which branch how dependencies are loaded. In
the above example `"http"` will be allowed when the `"import"` condition is
part of loading it.

A value of `null` for the resolved value will cause the resolution to fail.
This can be used to ensure some kinds dynamic access are explicitly prevented.

Unknown values for the resolved module location will cause failure, but are
not guaranteed to be forwards compatible.

#### Example: Patched dependency

Redirected dependencies can provide attenuated or modified functionality as fits
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1215,7 +1215,8 @@ E('ERR_MANIFEST_ASSERT_INTEGRITY',
return msg;
}, Error);
E('ERR_MANIFEST_DEPENDENCY_MISSING',
'Manifest resource %s does not list %s as a dependency specifier',
'Manifest resource %s does not list %s as a dependency specifier for ' +
'conditions: %s',
Error);
E('ERR_MANIFEST_INTEGRITY_MISMATCH',
'Manifest resource %s has multiple entries but integrity lists do not match',
Expand Down
24 changes: 19 additions & 5 deletions lib/internal/modules/cjs/helpers.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
'use strict';

const {
ArrayPrototypeJoin,
ObjectDefineProperty,
ObjectPrototypeHasOwnProperty,
SafeMap,
SafeSet,
} = primordials;
const {
ERR_MANIFEST_DEPENDENCY_MISSING,
Expand All @@ -16,10 +18,16 @@ const path = require('path');
const { pathToFileURL, fileURLToPath } = require('internal/url');
const { URL } = require('url');

const { getOptionValue } = require('internal/options');
const userConditions = getOptionValue('--conditions');

let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
debug = fn;
});

// TODO: Use this set when resolving pkg#exports conditions in loader.js.
const cjsConditions = new SafeSet(['require', 'node', ...userConditions]);

function loadNativeModule(filename, request) {
const mod = NativeModule.map.get(filename);
if (mod) {
Expand All @@ -38,11 +46,12 @@ function makeRequireFunction(mod, redirects) {

let require;
if (redirects) {
const { resolve, reaction } = redirects;
const id = mod.filename || mod.id;
require = function require(path) {
const conditions = cjsConditions;
const { resolve, reaction } = redirects;
require = function require(specifier) {
let missing = true;
const destination = resolve(path);
const destination = resolve(specifier, conditions);
if (destination === true) {
missing = false;
} else if (destination) {
Expand All @@ -66,9 +75,13 @@ function makeRequireFunction(mod, redirects) {
}
}
if (missing) {
reaction(new ERR_MANIFEST_DEPENDENCY_MISSING(id, path));
reaction(new ERR_MANIFEST_DEPENDENCY_MISSING(
id,
specifier,
ArrayPrototypeJoin([...conditions], ', ')
));
}
return mod.require(path);
return mod.require(specifier);
};
} else {
require = function require(path) {
Expand Down Expand Up @@ -168,6 +181,7 @@ function normalizeReferrerURL(referrer) {

module.exports = {
addBuiltinLibsToObject,
cjsConditions,
loadNativeModule,
makeRequireFunction,
normalizeReferrerURL,
Expand Down
4 changes: 1 addition & 3 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ const {
RegExpPrototypeTest,
SafeMap,
SafeWeakMap,
SafeSet,
String,
StringPrototypeIndexOf,
StringPrototypeMatch,
Expand Down Expand Up @@ -76,6 +75,7 @@ const {
makeRequireFunction,
normalizeReferrerURL,
stripBOM,
cjsConditions,
loadNativeModule
} = require('internal/modules/cjs/helpers');
const { getOptionValue } = require('internal/options');
Expand All @@ -85,7 +85,6 @@ const manifest = getOptionValue('--experimental-policy') ?
require('internal/process/policy').manifest :
null;
const { compileFunction } = internalBinding('contextify');
const userConditions = getOptionValue('--conditions');

// Whether any user-provided CJS modules had been loaded (executed).
// Used for internal assertions.
Expand Down Expand Up @@ -1002,7 +1001,6 @@ Module._load = function(request, parent, isMain) {
return module.exports;
};

const cjsConditions = new SafeSet(['require', 'node', ...userConditions]);
Module._resolveFilename = function(request, parent, isMain, options) {
if (NativeModule.canBeRequiredByUsers(request)) {
return request;
Expand Down
25 changes: 25 additions & 0 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ const {
Stats,
} = require('fs');
const { getOptionValue } = require('internal/options');
const manifest = getOptionValue('--experimental-policy') ?
require('internal/process/policy').manifest :
null;
const { sep, relative } = require('path');
const preserveSymlinks = getOptionValue('--preserve-symlinks');
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
Expand All @@ -42,6 +45,7 @@ const {
ERR_INVALID_MODULE_SPECIFIER,
ERR_INVALID_PACKAGE_CONFIG,
ERR_INVALID_PACKAGE_TARGET,
ERR_MANIFEST_DEPENDENCY_MISSING,
ERR_MODULE_NOT_FOUND,
ERR_PACKAGE_IMPORT_NOT_DEFINED,
ERR_PACKAGE_PATH_NOT_EXPORTED,
Expand Down Expand Up @@ -764,6 +768,27 @@ function resolveAsCommonJS(specifier, parentURL) {

function defaultResolve(specifier, context = {}, defaultResolveUnused) {
let { parentURL, conditions } = context;
if (manifest) {
const redirects = manifest.getRedirector(parentURL);
if (redirects) {
const { resolve, reaction } = redirects;
const destination = resolve(specifier, new SafeSet(conditions));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the copy necessarily because of potential mutations? If so, would it make sense to freeze the conditions set? Or is that a perf concern?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to keep the signature of resolve consistent

let missing = true;
if (destination === true) {
missing = false;
} else if (destination) {
const href = destination.href;
return { url: href };
}
if (missing) {
reaction(new ERR_MANIFEST_DEPENDENCY_MISSING(
parentURL,
specifier,
ArrayPrototypeJoin([...conditions], ', '))
);
}
}
}
let parsed;
try {
parsed = new URL(specifier);
Expand Down
98 changes: 80 additions & 18 deletions lib/internal/policy/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@ const {
ObjectCreate,
ObjectEntries,
ObjectFreeze,
ObjectKeys,
ObjectSetPrototypeOf,
RegExpPrototypeTest,
SafeMap,
uncurryThis,
} = primordials;
const {
compositeKey
} = require('internal/util/compositekey');
const {
canBeRequiredByUsers
} = require('internal/bootstrap/loaders').NativeModule;
Expand Down Expand Up @@ -70,13 +74,21 @@ class Manifest {
*/
#integrities = new SafeMap();
/**
* @type {Map<string, (specifier: string) => true | URL>}
* @type {
Map<
string,
(specifier: string, conditions: Set<string>) => true | null | URL
>
}
*
* Used to find where a dependency is located.
*
* This stores functions to lazily calculate locations as needed.
* `true` is used to signify that the location is not specified
* by the manifest and default resolution should be allowed.
*
* The functions return `null` to signify that a dependency is
* not found
*/
#dependencies = new SafeMap();
/**
Expand Down Expand Up @@ -158,36 +170,83 @@ class Manifest {
dependencyMap = ObjectCreate(null);
}
if (typeof dependencyMap === 'object' && !ArrayIsArray(dependencyMap)) {
function searchDependencies(target, conditions) {
if (
target &&
typeof target === 'object' &&
!ArrayIsArray(target)
) {
const keys = ObjectKeys(target);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
if (conditions.has(key)) {
const ret = searchDependencies(target[key], conditions);
if (ret != null) {
return ret;
}
}
}
} else if (typeof target === 'string') {
return target;
} else if (target === true) {
return target;
} else {
throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD(
resourceHREF,
'dependencies');
}
return null;
}
// This is used so we don't traverse this every time
// in theory we can delete parts of the dep map once this is populated
const localMappings = new SafeMap();
/**
* @returns {true | URL}
* @returns {true | null | URL}
*/
const dependencyRedirectList = (toSpecifier) => {
if (toSpecifier in dependencyMap !== true) {
const dependencyRedirectList = (specifier, conditions) => {
const key = compositeKey([localMappings, specifier, ...conditions]);
if (localMappings.has(key)) {
return localMappings.get(key);
}
if (specifier in dependencyMap !== true) {
localMappings.set(key, null);
return null;
}
const to = dependencyMap[toSpecifier];
if (to === true) {
const target = searchDependencies(
dependencyMap[specifier],
conditions);
if (target === true) {
localMappings.set(key, true);
return true;
}
if (parsedURLs.has(to)) {
return parsedURLs.get(to);
} else if (canBeRequiredByUsers(to)) {
const href = `node:${to}`;
if (typeof target !== 'string') {
localMappings.set(key, null);
return null;
}
if (parsedURLs.has(target)) {
const parsed = parsedURLs.get(target);
localMappings.set(key, parsed);
return parsed;
} else if (canBeRequiredByUsers(target)) {
const href = `node:${target}`;
const resolvedURL = new URL(href);
parsedURLs.set(to, resolvedURL);
parsedURLs.set(target, resolvedURL);
parsedURLs.set(href, resolvedURL);
localMappings.set(key, resolvedURL);
return resolvedURL;
} else if (RegExpPrototypeTest(kRelativeURLStringPattern, to)) {
const resolvedURL = new URL(to, manifestURL);
} else if (RegExpPrototypeTest(kRelativeURLStringPattern, target)) {
const resolvedURL = new URL(target, manifestURL);
const href = resourceURL.href;
parsedURLs.set(to, resolvedURL);
parsedURLs.set(target, resolvedURL);
parsedURLs.set(href, resolvedURL);
localMappings.set(key, resolvedURL);
return resolvedURL;
}
const resolvedURL = new URL(to);
const href = resourceURL.href;
parsedURLs.set(to, resolvedURL);
const resolvedURL = new URL(target);
const href = resolvedURL.href;
parsedURLs.set(target, resolvedURL);
parsedURLs.set(href, resolvedURL);
localMappings.set(key, resolvedURL);
return resolvedURL;
};
dependencies.set(resourceHREF, dependencyRedirectList);
Expand All @@ -208,7 +267,10 @@ class Manifest {
const dependencies = this.#dependencies;
if (dependencies.has(requester)) {
return {
resolve: (to) => dependencies.get(requester)(`${to}`),
resolve: (specifier, conditions) => dependencies.get(requester)(
`${specifier}`,
conditions
),
reaction: this.#reaction
};
}
Expand Down
Loading