Skip to content

Commit 8403118

Browse files
bmeckBethGriggs
authored andcommitted
policy: support conditions for redirects
PR-URL: #34414 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Guy Bedford <[email protected]>
1 parent 698cae7 commit 8403118

File tree

9 files changed

+387
-38
lines changed

9 files changed

+387
-38
lines changed

doc/api/policy.md

Lines changed: 23 additions & 11 deletions
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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1205,7 +1205,8 @@ E('ERR_MANIFEST_ASSERT_INTEGRITY',
12051205
return msg;
12061206
}, Error);
12071207
E('ERR_MANIFEST_DEPENDENCY_MISSING',
1208-
'Manifest resource %s does not list %s as a dependency specifier',
1208+
'Manifest resource %s does not list %s as a dependency specifier for ' +
1209+
'conditions: %s',
12091210
Error);
12101211
E('ERR_MANIFEST_INTEGRITY_MISMATCH',
12111212
'Manifest resource %s has multiple entries but integrity lists do not match',

lib/internal/modules/cjs/helpers.js

Lines changed: 19 additions & 5 deletions
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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ const {
4444
ReflectSet,
4545
RegExpPrototypeTest,
4646
SafeMap,
47-
SafeSet,
4847
String,
4948
StringPrototypeMatch,
5049
StringPrototypeSlice,
@@ -71,6 +70,7 @@ const {
7170
makeRequireFunction,
7271
normalizeReferrerURL,
7372
stripBOM,
73+
cjsConditions,
7474
loadNativeModule
7575
} = require('internal/modules/cjs/helpers');
7676
const { getOptionValue } = require('internal/options');
@@ -81,7 +81,6 @@ const manifest = getOptionValue('--experimental-policy') ?
8181
require('internal/process/policy').manifest :
8282
null;
8383
const { compileFunction } = internalBinding('contextify');
84-
const userConditions = getOptionValue('--conditions');
8584

8685
// Whether any user-provided CJS modules had been loaded (executed).
8786
// Used for internal assertions.
@@ -803,7 +802,6 @@ Module._load = function(request, parent, isMain) {
803802
return module.exports;
804803
};
805804

806-
const cjsConditions = new SafeSet(['require', 'node', ...userConditions]);
807805
Module._resolveFilename = function(request, parent, isMain, options) {
808806
if (NativeModule.canBeRequiredByUsers(request)) {
809807
return request;

lib/internal/modules/esm/resolve.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ const {
3030
Stats,
3131
} = require('fs');
3232
const { getOptionValue } = require('internal/options');
33+
const manifest = getOptionValue('--experimental-policy') ?
34+
require('internal/process/policy').manifest :
35+
null;
3336
const { sep, relative } = require('path');
3437
const preserveSymlinks = getOptionValue('--preserve-symlinks');
3538
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
@@ -41,6 +44,7 @@ const {
4144
ERR_INVALID_MODULE_SPECIFIER,
4245
ERR_INVALID_PACKAGE_CONFIG,
4346
ERR_INVALID_PACKAGE_TARGET,
47+
ERR_MANIFEST_DEPENDENCY_MISSING,
4448
ERR_MODULE_NOT_FOUND,
4549
ERR_PACKAGE_IMPORT_NOT_DEFINED,
4650
ERR_PACKAGE_PATH_NOT_EXPORTED,
@@ -710,6 +714,27 @@ function resolveAsCommonJS(specifier, parentURL) {
710714

711715
function defaultResolve(specifier, context = {}, defaultResolveUnused) {
712716
let { parentURL, conditions } = context;
717+
if (manifest) {
718+
const redirects = manifest.getRedirector(parentURL);
719+
if (redirects) {
720+
const { resolve, reaction } = redirects;
721+
const destination = resolve(specifier, new SafeSet(conditions));
722+
let missing = true;
723+
if (destination === true) {
724+
missing = false;
725+
} else if (destination) {
726+
const href = destination.href;
727+
return { url: href };
728+
}
729+
if (missing) {
730+
reaction(new ERR_MANIFEST_DEPENDENCY_MISSING(
731+
parentURL,
732+
specifier,
733+
ArrayPrototypeJoin([...conditions], ', '))
734+
);
735+
}
736+
}
737+
}
713738
let parsed;
714739
try {
715740
parsed = new URL(specifier);

lib/internal/policy/manifest.js

Lines changed: 80 additions & 18 deletions
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)