Skip to content

Commit 52c7ed6

Browse files
committed
policy: support conditions for redirects
1 parent 7a1220a commit 52c7ed6

File tree

9 files changed

+387
-38
lines changed

9 files changed

+387
-38
lines changed

doc/api/policy.md

+23-11
Original file line numberDiff line numberDiff line change
@@ -124,24 +124,25 @@ replaced.
124124

125125
```json
126126
{
127-
"builtins": [],
128127
"resources": {
129128
"./app/checked.js": {
130129
"dependencies": {
131130
"fs": true,
132-
"os": "./app/node_modules/alt-os"
131+
"os": "./app/node_modules/alt-os",
132+
"http": { "import": true }
133133
}
134134
}
135135
}
136136
}
137137
```
138138

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

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

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

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

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

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

168+
Similar to `"exports"` in `package.json` dependencies can also be specified to
169+
be objects containing conditions which branch how dependencies are loaded. In
170+
the above example `"http"` will be allowed when the `"import"` condition is
171+
part of loading it.
172+
173+
A value of `null` for the resolved value will cause the resolution to fail.
174+
This can be used to ensure some kinds dynamic access are explicitly prevented.
175+
176+
Unknown values for the resolved module location will cause failure, but are
177+
not guaranteed to be forwards compatible.
178+
167179
#### Example: Patched dependency
168180

169181
Redirected dependencies can provide attenuated or modified functionality as fits

lib/internal/errors.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -1215,7 +1215,8 @@ E('ERR_MANIFEST_ASSERT_INTEGRITY',
12151215
return msg;
12161216
}, Error);
12171217
E('ERR_MANIFEST_DEPENDENCY_MISSING',
1218-
'Manifest resource %s does not list %s as a dependency specifier',
1218+
'Manifest resource %s does not list %s as a dependency specifier for ' +
1219+
'conditions: %s',
12191220
Error);
12201221
E('ERR_MANIFEST_INTEGRITY_MISMATCH',
12211222
'Manifest resource %s has multiple entries but integrity lists do not match',

lib/internal/modules/cjs/helpers.js

+19-5
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
'use strict';
22

33
const {
4+
ArrayPrototypeJoin,
45
ObjectDefineProperty,
56
ObjectPrototypeHasOwnProperty,
67
SafeMap,
8+
SafeSet,
79
} = primordials;
810
const {
911
ERR_MANIFEST_DEPENDENCY_MISSING,
@@ -16,10 +18,16 @@ const path = require('path');
1618
const { pathToFileURL, fileURLToPath } = require('internal/url');
1719
const { URL } = require('url');
1820

21+
const { getOptionValue } = require('internal/options');
22+
const userConditions = getOptionValue('--conditions');
23+
1924
let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
2025
debug = fn;
2126
});
2227

28+
// TODO: Use this set when resolving pkg#exports conditions in loader.js.
29+
const cjsConditions = new SafeSet(['require', 'node', ...userConditions]);
30+
2331
function loadNativeModule(filename, request) {
2432
const mod = NativeModule.map.get(filename);
2533
if (mod) {
@@ -38,11 +46,12 @@ function makeRequireFunction(mod, redirects) {
3846

3947
let require;
4048
if (redirects) {
41-
const { resolve, reaction } = redirects;
4249
const id = mod.filename || mod.id;
43-
require = function require(path) {
50+
const conditions = cjsConditions;
51+
const { resolve, reaction } = redirects;
52+
require = function require(specifier) {
4453
let missing = true;
45-
const destination = resolve(path);
54+
const destination = resolve(specifier, conditions);
4655
if (destination === true) {
4756
missing = false;
4857
} else if (destination) {
@@ -66,9 +75,13 @@ function makeRequireFunction(mod, redirects) {
6675
}
6776
}
6877
if (missing) {
69-
reaction(new ERR_MANIFEST_DEPENDENCY_MISSING(id, path));
78+
reaction(new ERR_MANIFEST_DEPENDENCY_MISSING(
79+
id,
80+
specifier,
81+
ArrayPrototypeJoin([...conditions], ', ')
82+
));
7083
}
71-
return mod.require(path);
84+
return mod.require(specifier);
7285
};
7386
} else {
7487
require = function require(path) {
@@ -168,6 +181,7 @@ function normalizeReferrerURL(referrer) {
168181

169182
module.exports = {
170183
addBuiltinLibsToObject,
184+
cjsConditions,
171185
loadNativeModule,
172186
makeRequireFunction,
173187
normalizeReferrerURL,

lib/internal/modules/cjs/loader.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ const {
4747
RegExpPrototypeTest,
4848
SafeMap,
4949
SafeWeakMap,
50-
SafeSet,
5150
String,
5251
StringPrototypeIndexOf,
5352
StringPrototypeMatch,
@@ -76,6 +75,7 @@ const {
7675
makeRequireFunction,
7776
normalizeReferrerURL,
7877
stripBOM,
78+
cjsConditions,
7979
loadNativeModule
8080
} = require('internal/modules/cjs/helpers');
8181
const { getOptionValue } = require('internal/options');
@@ -85,7 +85,6 @@ const manifest = getOptionValue('--experimental-policy') ?
8585
require('internal/process/policy').manifest :
8686
null;
8787
const { compileFunction } = internalBinding('contextify');
88-
const userConditions = getOptionValue('--conditions');
8988

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

1005-
const cjsConditions = new SafeSet(['require', 'node', ...userConditions]);
10061004
Module._resolveFilename = function(request, parent, isMain, options) {
10071005
if (NativeModule.canBeRequiredByUsers(request)) {
10081006
return request;

lib/internal/modules/esm/resolve.js

+25
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ const {
3131
Stats,
3232
} = require('fs');
3333
const { getOptionValue } = require('internal/options');
34+
const manifest = getOptionValue('--experimental-policy') ?
35+
require('internal/process/policy').manifest :
36+
null;
3437
const { sep, relative } = require('path');
3538
const preserveSymlinks = getOptionValue('--preserve-symlinks');
3639
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
@@ -42,6 +45,7 @@ const {
4245
ERR_INVALID_MODULE_SPECIFIER,
4346
ERR_INVALID_PACKAGE_CONFIG,
4447
ERR_INVALID_PACKAGE_TARGET,
48+
ERR_MANIFEST_DEPENDENCY_MISSING,
4549
ERR_MODULE_NOT_FOUND,
4650
ERR_PACKAGE_IMPORT_NOT_DEFINED,
4751
ERR_PACKAGE_PATH_NOT_EXPORTED,
@@ -764,6 +768,27 @@ function resolveAsCommonJS(specifier, parentURL) {
764768

765769
function defaultResolve(specifier, context = {}, defaultResolveUnused) {
766770
let { parentURL, conditions } = context;
771+
if (manifest) {
772+
const redirects = manifest.getRedirector(parentURL);
773+
if (redirects) {
774+
const { resolve, reaction } = redirects;
775+
const destination = resolve(specifier, new SafeSet(conditions));
776+
let missing = true;
777+
if (destination === true) {
778+
missing = false;
779+
} else if (destination) {
780+
const href = destination.href;
781+
return { url: href };
782+
}
783+
if (missing) {
784+
reaction(new ERR_MANIFEST_DEPENDENCY_MISSING(
785+
parentURL,
786+
specifier,
787+
ArrayPrototypeJoin([...conditions], ', '))
788+
);
789+
}
790+
}
791+
}
767792
let parsed;
768793
try {
769794
parsed = new URL(specifier);

lib/internal/policy/manifest.js

+80-18
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,15 @@ const {
77
ObjectCreate,
88
ObjectEntries,
99
ObjectFreeze,
10+
ObjectKeys,
1011
ObjectSetPrototypeOf,
1112
RegExpPrototypeTest,
1213
SafeMap,
1314
uncurryThis,
1415
} = primordials;
16+
const {
17+
compositeKey
18+
} = require('internal/util/compositekey');
1519
const {
1620
canBeRequiredByUsers
1721
} = require('internal/bootstrap/loaders').NativeModule;
@@ -70,13 +74,21 @@ class Manifest {
7074
*/
7175
#integrities = new SafeMap();
7276
/**
73-
* @type {Map<string, (specifier: string) => true | URL>}
77+
* @type {
78+
Map<
79+
string,
80+
(specifier: string, conditions: Set<string>) => true | null | URL
81+
>
82+
}
7483
*
7584
* Used to find where a dependency is located.
7685
*
7786
* This stores functions to lazily calculate locations as needed.
7887
* `true` is used to signify that the location is not specified
7988
* by the manifest and default resolution should be allowed.
89+
*
90+
* The functions return `null` to signify that a dependency is
91+
* not found
8092
*/
8193
#dependencies = new SafeMap();
8294
/**
@@ -158,36 +170,83 @@ class Manifest {
158170
dependencyMap = ObjectCreate(null);
159171
}
160172
if (typeof dependencyMap === 'object' && !ArrayIsArray(dependencyMap)) {
173+
function searchDependencies(target, conditions) {
174+
if (
175+
target &&
176+
typeof target === 'object' &&
177+
!ArrayIsArray(target)
178+
) {
179+
const keys = ObjectKeys(target);
180+
for (let i = 0; i < keys.length; i++) {
181+
const key = keys[i];
182+
if (conditions.has(key)) {
183+
const ret = searchDependencies(target[key], conditions);
184+
if (ret != null) {
185+
return ret;
186+
}
187+
}
188+
}
189+
} else if (typeof target === 'string') {
190+
return target;
191+
} else if (target === true) {
192+
return target;
193+
} else {
194+
throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD(
195+
resourceHREF,
196+
'dependencies');
197+
}
198+
return null;
199+
}
200+
// This is used so we don't traverse this every time
201+
// in theory we can delete parts of the dep map once this is populated
202+
const localMappings = new SafeMap();
161203
/**
162-
* @returns {true | URL}
204+
* @returns {true | null | URL}
163205
*/
164-
const dependencyRedirectList = (toSpecifier) => {
165-
if (toSpecifier in dependencyMap !== true) {
206+
const dependencyRedirectList = (specifier, conditions) => {
207+
const key = compositeKey([localMappings, specifier, ...conditions]);
208+
if (localMappings.has(key)) {
209+
return localMappings.get(key);
210+
}
211+
if (specifier in dependencyMap !== true) {
212+
localMappings.set(key, null);
166213
return null;
167214
}
168-
const to = dependencyMap[toSpecifier];
169-
if (to === true) {
215+
const target = searchDependencies(
216+
dependencyMap[specifier],
217+
conditions);
218+
if (target === true) {
219+
localMappings.set(key, true);
170220
return true;
171221
}
172-
if (parsedURLs.has(to)) {
173-
return parsedURLs.get(to);
174-
} else if (canBeRequiredByUsers(to)) {
175-
const href = `node:${to}`;
222+
if (typeof target !== 'string') {
223+
localMappings.set(key, null);
224+
return null;
225+
}
226+
if (parsedURLs.has(target)) {
227+
const parsed = parsedURLs.get(target);
228+
localMappings.set(key, parsed);
229+
return parsed;
230+
} else if (canBeRequiredByUsers(target)) {
231+
const href = `node:${target}`;
176232
const resolvedURL = new URL(href);
177-
parsedURLs.set(to, resolvedURL);
233+
parsedURLs.set(target, resolvedURL);
178234
parsedURLs.set(href, resolvedURL);
235+
localMappings.set(key, resolvedURL);
179236
return resolvedURL;
180-
} else if (RegExpPrototypeTest(kRelativeURLStringPattern, to)) {
181-
const resolvedURL = new URL(to, manifestURL);
237+
} else if (RegExpPrototypeTest(kRelativeURLStringPattern, target)) {
238+
const resolvedURL = new URL(target, manifestURL);
182239
const href = resourceURL.href;
183-
parsedURLs.set(to, resolvedURL);
240+
parsedURLs.set(target, resolvedURL);
184241
parsedURLs.set(href, resolvedURL);
242+
localMappings.set(key, resolvedURL);
185243
return resolvedURL;
186244
}
187-
const resolvedURL = new URL(to);
188-
const href = resourceURL.href;
189-
parsedURLs.set(to, resolvedURL);
245+
const resolvedURL = new URL(target);
246+
const href = resolvedURL.href;
247+
parsedURLs.set(target, resolvedURL);
190248
parsedURLs.set(href, resolvedURL);
249+
localMappings.set(key, resolvedURL);
191250
return resolvedURL;
192251
};
193252
dependencies.set(resourceHREF, dependencyRedirectList);
@@ -208,7 +267,10 @@ class Manifest {
208267
const dependencies = this.#dependencies;
209268
if (dependencies.has(requester)) {
210269
return {
211-
resolve: (to) => dependencies.get(requester)(`${to}`),
270+
resolve: (specifier, conditions) => dependencies.get(requester)(
271+
`${specifier}`,
272+
conditions
273+
),
212274
reaction: this.#reaction
213275
};
214276
}

0 commit comments

Comments
 (0)