From bcb85adee603ccac68f675e94de4f7aa34e99aff Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Mon, 22 Mar 2021 10:28:05 -0500 Subject: [PATCH] policy: canonicalize before resolving specifiers PR-URL: https://github.com/nodejs/node/pull/37863 Co-authored-by: Antoine du Hamel Co-authored-by: James M Snell Reviewed-By: Guy Bedford --- doc/api/errors.md | 7 + doc/api/policy.md | 165 +++- lib/internal/errors.js | 3 + lib/internal/policy/manifest.js | 836 +++++++++++------- test/fixtures/policy/canonicalize.mjs | 5 + ...endencies-missing-policy-default-true.json | 13 + .../dependencies-redirect-builtin-policy.json | 2 +- .../dependencies-redirect-policy.json | 2 +- ...ncies-redirect-unknown-builtin-policy.json | 2 +- ...ependencies-scopes-relative-specifier.json | 12 + test/parallel/test-policy-dependencies.js | 41 +- .../test-policy-scopes-dependencies.js | 35 + 12 files changed, 772 insertions(+), 351 deletions(-) create mode 100644 test/fixtures/policy/canonicalize.mjs create mode 100644 test/fixtures/policy/dependencies/dependencies-missing-policy-default-true.json create mode 100644 test/fixtures/policy/dependencies/dependencies-scopes-relative-specifier.json diff --git a/doc/api/errors.md b/doc/api/errors.md index fcbe849d035de9..8a5d3e5daf21f2 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1615,6 +1615,13 @@ A policy manifest resource had an invalid value for one of its fields. Update the manifest entry to match in order to resolve this error. See the documentation for [policy][] manifests for more information. + +### `ERR_MANIFEST_INVALID_SPECIFIER` + +A policy manifest resource had an invalid value for one of its dependency +mappings. Update the manifest entry to match to resolve this error. See the +documentation for [policy][] manifests for more information. + ### `ERR_MANIFEST_PARSE_POLICY` diff --git a/doc/api/policy.md b/doc/api/policy.md index 6cca07f8ba59b2..01b3eba0a33084 100644 --- a/doc/api/policy.md +++ b/doc/api/policy.md @@ -57,7 +57,7 @@ by defining an "onerror" field in a policy manifest. The following values are available to change the behavior: * `"exit"`: will exit the process immediately. - No cleanup code will be allowed to run. + No cleanup code will be allowed to run. * `"log"`: will log the error at the site of the failure. * `"throw"`: will throw a JS error at the site of the failure. This is the default. @@ -80,9 +80,9 @@ compatible with the browser [integrity attribute](https://www.w3.org/TR/SRI/#the-integrity-attribute) associated with absolute URLs. -When using `require()` all resources involved in loading are checked for -integrity if a policy manifest has been specified. If a resource does not match -the integrity listed in the manifest, an error will be thrown. +When using `require()` or `import` all resources involved in loading are checked +for integrity if a policy manifest has been specified. If a resource does not +match the integrity listed in the manifest, an error will be thrown. An example policy file that would allow loading a file `checked.js`: @@ -107,7 +107,7 @@ and hash fragment. `./a.js?b` will not be used when attempting to load `./a.js` and vice versa. To generate integrity strings, a script such as -`printf "sha384-$(cat checked.js | openssl dgst -sha384 -binary | base64)"` +`node -e 'process.stdout.write("sha256-");process.stdin.pipe(crypto.createHash("sha256").setEncoding("base64")).pipe(process.stdout)' < FILE` can be used. Integrity can be specified as the boolean value `true` to accept any @@ -140,13 +140,37 @@ The dependencies are keyed by the requested specifier string and have values of either `true`, `null`, a string pointing to a module to be resolved, or a conditions object. -The specifier string does not perform any searching and must match exactly -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). +The specifier string does not perform any searching and must match exactly what +is provided to the `require()` or `import` except for a canonicalization step. +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 are -used to find the module. +Specifier strings are canonicalized but not resolved prior to be used for +matching in order to have some compatibility with import maps, for example if a +resource `file:///C:/app/server.js` was given the following redirection from a +policy located at `file:///C:/app/policy.json`: + +```json +{ + "resources": { + "file:///C:/app/utils.js": { + "dependencies": { + "./utils.js": "./utils-v2.js" + } + } + } +} +``` + +Any specifier used to load `file:///C:/app/utils.js` would then be intercepted +and redirected to `file:///C:/app/utils-v2.js` instead regardless of using an +absolute or relative specifier. However, if a specifier that is not an absolute +or relative URL string is used, it would not be intercepted. So, if an import +such as `import('#utils')` was used, it would not be intercepted. + +If the value of the redirection is `true`, a "dependencies" field at the top of +the policy file will be used. If that field at the top of the policy file is +`true` the default node searching algorithms are used to find the module. If the value of the redirection is a string, it is resolved relative to the manifest and then immediately used without searching. @@ -207,7 +231,8 @@ is found by recursively reducing the resource URL by removing segments for hash fragment. This leads to the eventual reduction of the URL to its origin. If the URL is non-special the scope will be located by the URL's origin. If no scope is found for the origin or in the case of opaque origins, a protocol -string can be used as a scope. +string can be used as a scope. If no scope is found for the URL's protocol, a +final empty string `""` scope will be used. Note, `blob:` URLs adopt their origin from the path they contain, and so a scope of `"blob:https://nodejs.org"` will have no effect since no URL can have an @@ -216,6 +241,61 @@ origin of `blob:https://nodejs.org`; URLs starting with thus `https:` for its protocol scope. For opaque origin `blob:` URLs they will have `blob:` for their protocol scope since they do not adopt origins. +#### Example + +```json +{ + "scopes": { + "file:///C:/app/": {}, + "file:": {}, + "": {} + } +} +``` + +Given a file located at `file:///C:/app/bin/main.js`, the following scopes would +be checked in order: + +1. `"file:///C:/app/bin/"` + +This determines the policy for all file based resources within +`"file:///C:/app/bin/"`. This is not in the `"scopes"` field of the policy and +would be skipped. Adding this scope to the policy would cause it to be used +prior to the `"file:///C:/app/"` scope. + +2. `"file:///C:/app/"` + +This determines the policy for all file based resources within +`"file:///C:/app/"`. This is in the `"scopes"` field of the policy and it would +determine the policy for the resource at `file:///C:/app/bin/main.js`. If the +scope has `"cascade": true`, any unsatisfied queries about the resource would +delegate to the next relevant scope for `file:///C:/app/bin/main.js`, `"file:"`. + +3. `"file:///C:/"` + +This determines the policy for all file based resources within `"file:///C:/"`. +This is not in the `"scopes"` field of the policy and would be skipped. It would +not be used for `file:///C:/app/bin/main.js` unless `"file:///"` is set to +cascade or is not in the `"scopes"` of the policy. + +4. `"file:///"` + +This determines the policy for all file based resources on the `localhost`. This +is not in the `"scopes"` field of the policy and would be skipped. It would not +be used for `file:///C:/app/bin/main.js` unless `"file:///"` is set to cascade +or is not in the `"scopes"` of the policy. + +5. `"file:"` + +This determines the policy for all file based resources. It would not be used +for `file:///C:/app/bin/main.js` unless `"file:///"` is set to cascade or is not +in the `"scopes"` of the policy. + +6. `""` + +This determines the policy for all resources. It would not be used for +`file:///C:/app/bin/main.js` unless `"file:"` is set to cascade. + #### Integrity using scopes Setting an integrity to `true` on a scope will set the integrity for any @@ -284,5 +364,64 @@ The following example, would allow access to `fs` for all `data:` resources: } ``` -[relative-URL string]: https://url.spec.whatwg.org/#relative-url-with-fragment-string +#### Example: [import maps][] emulation + +Given an import map: + +```json +{ + "imports": { + "react": "./app/node_modules/react/index.js" + }, + "scopes": { + "./ssr/": { + "react": "./app/node_modules/server-side-react/index.js" + } + } +} +``` + +```json +{ + "dependencies": true, + "scopes": { + "": { + "cascade": true, + "dependencies": { + "react": "./app/node_modules/react/index.js" + } + }, + "./ssr/": { + "cascade": true, + "dependencies": { + "react": "./app/node_modules/server-side-react/index.js" + } + } + } +} +``` + +Import maps assume you can get any resource by default. This means +`"dependencies"` at the top level of the policy should be set to `true`. +Policies require this to be opt-in since it enables all resources of the +application cross linkage which doesn't make sense for many scenarios. They also +assume any given scope has access to any scope above its allowed dependencies; +all scopes emulating import maps must set `"cascade": true`. + +Import maps only have a single top level scope for their "imports". So for +emulating `"imports"` use the `""` scope. For emulating `"scopes"` use the +`"scopes"` in a similar manner to how `"scopes"` works in import maps. + +Caveats: Policies do not use string matching for various finding of scope. They +do URL traversals. This means things like `blob:` and `data:` URLs might not be +entirely interoperable between the two systems. For example import maps can +partially match a `data:` or `blob:` URL by partitioning the URL on a `/` +character, policies intentionally cannot. For `blob:` URLs import map scopes do +not adopt the origin of the `blob:` URL. + +Additionally, import maps only work on `import` so it may be desirable to add a +`"import"` condition to all dependency mappings. + +[import maps]: https://url.spec.whatwg.org/#relative-url-with-fragment-string +[relative-url string]: https://url.spec.whatwg.org/#relative-url-with-fragment-string [special schemes]: https://url.spec.whatwg.org/#special-scheme diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 2e0cc23a2b2929..91d03d087420ba 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1266,6 +1266,9 @@ E('ERR_MANIFEST_INTEGRITY_MISMATCH', E('ERR_MANIFEST_INVALID_RESOURCE_FIELD', 'Manifest resource %s has invalid property value for %s', TypeError); +E('ERR_MANIFEST_INVALID_SPECIFIER', + 'Manifest resource %s has invalid dependency mapping %s', + TypeError); E('ERR_MANIFEST_TDZ', 'Manifest initialization has not yet run', Error); E('ERR_MANIFEST_UNKNOWN_ONERROR', 'Manifest specified unknown error behavior "%s".', diff --git a/lib/internal/policy/manifest.js b/lib/internal/policy/manifest.js index a8420343db1a3c..42ff63e87dc9cf 100644 --- a/lib/internal/policy/manifest.js +++ b/lib/internal/policy/manifest.js @@ -1,12 +1,15 @@ 'use strict'; +// #region imports const { ArrayIsArray, + ArrayPrototypeSort, ObjectCreate, ObjectEntries, ObjectFreeze, ObjectKeys, ObjectSetPrototypeOf, + RegExpPrototypeExec, RegExpPrototypeTest, SafeMap, SafeSet, @@ -15,13 +18,10 @@ const { Symbol, uncurryThis, } = primordials; -const { - canBeRequiredByUsers -} = require('internal/bootstrap/loaders').NativeModule; const { ERR_MANIFEST_ASSERT_INTEGRITY, - ERR_MANIFEST_INTEGRITY_MISMATCH, ERR_MANIFEST_INVALID_RESOURCE_FIELD, + ERR_MANIFEST_INVALID_SPECIFIER, ERR_MANIFEST_UNKNOWN_ONERROR, } = require('internal/errors').codes; let debug = require('internal/util/debuglog').debuglog('policy', (fn) => { @@ -37,12 +37,13 @@ const HashDigest = uncurryThis(crypto.Hash.prototype.digest); const BufferToString = uncurryThis(Buffer.prototype.toString); const kRelativeURLStringPattern = /^\.{0,2}\//; const { getOptionValue } = require('internal/options'); -const shouldAbortOnUncaughtException = - getOptionValue('--abort-on-uncaught-exception'); +const shouldAbortOnUncaughtException = getOptionValue( + '--abort-on-uncaught-exception' +); const { abort, exit, _rawDebug } = process; +// #endregion -const kTerminate = () => null; - +// #region constants // From https://url.spec.whatwg.org/#special-scheme const kSpecialSchemes = new SafeSet([ 'file:', @@ -53,7 +54,14 @@ const kSpecialSchemes = new SafeSet([ 'wss:', ]); +/** + * @type {symbol} + */ const kCascade = Symbol('cascade'); +/** + * @type {symbol} + */ +const kFallThrough = Symbol('fall through'); function REACTION_THROW(error) { throw error; @@ -70,31 +78,306 @@ function REACTION_EXIT(error) { function REACTION_LOG(error) { _rawDebug(error.stack); } + +// #endregion + +// #region DependencyMapperInstance +class DependencyMapperInstance { + /** + * @type {string} + */ + href; + /** + * @type {DependencyMap | undefined} + */ + #dependencies; + /** + * @type {PatternDependencyMap | undefined} + */ + #patternDependencies; + /** + * @type {DependencyMapperInstance | null | undefined} + */ + #parentDependencyMapper; + /** + * @type {boolean} + */ + #normalized = false; + /** + * @type {boolean} + */ + cascade; + /** + * @type {boolean} + */ + allowSameHREFScope; + /** + * @param {string} parentHREF + * @param {DependencyMap | undefined} dependencies + * @param {boolean} cascade + * @param {boolean} allowSameHREFScope + */ + constructor( + parentHREF, + dependencies, + cascade = false, + allowSameHREFScope = false) { + this.href = parentHREF; + if (dependencies === kFallThrough || + dependencies === undefined || + dependencies === null) { + this.#dependencies = dependencies; + this.#patternDependencies = undefined; + } else { + const patterns = []; + for (const { 0: key } of ObjectEntries(dependencies)) { + if (StringPrototypeEndsWith(key, '*')) { + const target = RegExpPrototypeExec(/^([^*]*)\*([^*]*)$/); + if (!target) { + throw new ERR_MANIFEST_INVALID_SPECIFIER( + this.href, + target + + ', pattern needs to have a single' + + 'trailing "*" in target'); + } + const prefix = target[1]; + const suffix = target[2]; + patterns.push([ + target.slice(0, -1), + [prefix, suffix], + ]); + } + } + ArrayPrototypeSort(patterns, (a, b) => { + return a[0] < b[0] ? -1 : 1; + }); + this.#dependencies = dependencies; + this.#patternDependencies = patterns; + } + this.cascade = cascade; + this.allowSameHREFScope = allowSameHREFScope; + ObjectFreeze(this); + } + /** + * + * @param {string} normalizedSpecifier + * @param {Set} conditions + * @param {Manifest} manifest + * @returns {URL | typeof kFallThrough | null} + */ + _resolveAlreadyNormalized(normalizedSpecifier, conditions, manifest) { + let dependencies = this.#dependencies; + debug(this.href, 'resolving', normalizedSpecifier); + if (dependencies === kFallThrough) return true; + if (dependencies !== undefined && typeof dependencies === 'object') { + const normalized = this.#normalized; + if (normalized !== true) { + /** + * @type {Record} + */ + const normalizedDependencyMap = ObjectCreate(null); + for (let specifier in dependencies) { + const target = dependencies[specifier]; + specifier = canonicalizeSpecifier(specifier, manifest.href); + normalizedDependencyMap[specifier] = target; + } + ObjectFreeze(normalizedDependencyMap); + dependencies = normalizedDependencyMap; + this.#dependencies = normalizedDependencyMap; + this.#normalized = true; + } + debug(dependencies); + if (normalizedSpecifier in dependencies === true) { + const to = searchDependencies( + this.href, + dependencies[normalizedSpecifier], + conditions + ); + debug({ to }); + if (to === true) { + return true; + } + let ret; + if (parsedURLs && parsedURLs.has(to)) { + ret = parsedURLs.get(to); + } else if (RegExpPrototypeTest(kRelativeURLStringPattern, to)) { + ret = resolve(to, manifest.href); + } else { + ret = resolve(to); + } + return ret; + } + } + const { cascade } = this; + if (cascade !== true) { + return null; + } + let parentDependencyMapper = this.#parentDependencyMapper; + if (parentDependencyMapper === undefined) { + parentDependencyMapper = manifest.getScopeDependencyMapper( + this.href, + this.allowSameHREFScope + ); + this.#parentDependencyMapper = parentDependencyMapper; + } + if (parentDependencyMapper === null) { + return null; + } + return parentDependencyMapper._resolveAlreadyNormalized( + normalizedSpecifier, + conditions, + manifest + ); + } +} + +const kArbitraryDependencies = new DependencyMapperInstance( + 'arbitrary dependencies', + kFallThrough, + false, + true +); +const kNoDependencies = new DependencyMapperInstance( + 'no dependencies', + null, + false, + true +); /** + * @param {string} href + * @param {JSONDependencyMap} dependencies + * @param {boolean} cascade + * @param {Map} store + */ +const insertDependencyMap = ( + href, + dependencies, + cascade, + allowSameHREFScope, + store +) => { + if (cascade !== undefined && typeof cascade !== 'boolean') { + throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD(href, 'cascade'); + } + if (dependencies === true) { + store.set(href, kArbitraryDependencies); + return; + } + if (dependencies === null || dependencies === undefined) { + store.set( + href, + cascade ? + new DependencyMapperInstance(href, null, true, allowSameHREFScope) : + kNoDependencies + ); + return; + } + if (objectButNotArray(dependencies)) { + store.set( + href, + new DependencyMapperInstance( + href, + dependencies, + cascade, + allowSameHREFScope + ) + ); + return; + } + throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD(href, 'dependencies'); +}; +/** + * Finds the longest key within `this.#scopeDependencies` that covers a + * specific HREF + * @param {string} href + * @param {ScopeStore} scopeStore + * @returns {null | string} + */ +function findScopeHREF(href, scopeStore, allowSame) { + let protocol; + if (href !== '') { + // default URL parser does some stuff to special urls... skip if this is + // just the protocol + if (RegExpPrototypeTest(/^[^:]*[:]$/, href)) { + protocol = href; + } else { + let currentURL = new URL(href); + const normalizedHREF = currentURL.href; + protocol = currentURL.protocol; + // Non-opaque blobs adopt origins + if (protocol === 'blob:' && currentURL.origin !== 'null') { + currentURL = new URL(currentURL.origin); + protocol = currentURL.protocol; + } + // Only a few schemes are hierarchical + if (kSpecialSchemes.has(currentURL.protocol)) { + // Make first '..' act like '.' + if (!StringPrototypeEndsWith(currentURL.pathname, '/')) { + currentURL.pathname += '/'; + } + let lastHREF; + let currentHREF = currentURL.href; + do { + if (scopeStore.has(currentHREF)) { + if (allowSame || currentHREF !== normalizedHREF) { + return currentHREF; + } + } + lastHREF = currentHREF; + currentURL = new URL('..', currentURL); + currentHREF = currentURL.href; + } while (lastHREF !== currentHREF); + } + } + } + if (scopeStore.has(protocol)) { + if (allowSame || protocol !== href) return protocol; + } + if (scopeStore.has('')) { + if (allowSame || '' !== href) return ''; + } + return null; +} +// #endregion + +/** + * @typedef {Record | typeof kFallThrough} DependencyMap + * @typedef {Array<[string, [string, string]]>} PatternDependencyMap + * @typedef {Record | null | true} JSONDependencyMap + */ +/** + * @typedef {Map} ScopeStore * @typedef {(specifier: string) => true | URL} DependencyMapper - * @typedef {Record | true} DependencyMap - * @typedef {true | string | SRI[]} Integrity + * @typedef {boolean | string | SRI[] | typeof kCascade} Integrity */ class Manifest { + #defaultDependencies; /** - * @type {Map} - * - * Used to compare a resource to the content body at the resource. - * `true` is used to signify that all integrities are allowed, otherwise, - * SRI strings are parsed to compare with the body. + * @type {string} + */ + href; + /** + * @type {(err: Error) => void} * - * Separate from #resourceDependencies due to conflicts with things like - * `blob:` being both a scope and a resource potentially as well as - * `file:` being parsed to `file:///` instead of remaining host neutral. + * Performs default action for what happens when a manifest encounters + * a violation such as abort()ing or exiting the process, throwing the error, + * or logging the error. */ - #scopeDependencies = new SafeMap(); + #reaction; /** - * @type {Map} + * @type {Map} * - * Used to allow arbitrary loading within a scope + * 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 */ - #scopeIntegrities = new SafeMap(); + #resourceDependencies = new SafeMap(); /** * @type {Map} * @@ -109,26 +392,23 @@ class Manifest { */ #resourceIntegrities = new SafeMap(); /** - * @type {Map} - * - * Used to find where a dependency is located. + * @type {ScopeStore} * - * 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. + * Used to compare a resource to the content body at the resource. + * `true` is used to signify that all integrities are allowed, otherwise, + * SRI strings are parsed to compare with the body. * - * The functions return `null` to signify that a dependency is - * not found + * Separate from #resourceDependencies due to conflicts with things like + * `blob:` being both a scope and a resource potentially as well as + * `file:` being parsed to `file:///` instead of remaining host neutral. */ - #resourceDependencies = new SafeMap(); + #scopeDependencies = new SafeMap(); /** - * @type {(err: Error) => void} + * @type {Map} * - * Performs default action for what happens when a manifest encounters - * a violation such as abort()ing or exiting the process, throwing the error, - * or logging the error. + * Used to allow arbitrary loading within a scope */ - #reaction = null; + #scopeIntegrities = new SafeMap(); /** * `obj` should match the policy file format described in the docs * it is expected to not have prototype pollution issues either by reassigning @@ -137,17 +417,16 @@ class Manifest { * `manifestURL` is a URL to resolve relative locations against. * * @param {object} obj - * @param {string} manifestURL + * @param {string} manifestHREF */ - constructor(obj, manifestURL) { + constructor(obj, manifestHREF) { + this.href = manifestHREF; const scopes = this.#scopeDependencies; - scopes.set(null, kTerminate); - scopes.set(undefined, kTerminate); const integrities = this.#resourceIntegrities; - const dependencies = this.#resourceDependencies; + const resourceDependencies = this.#resourceDependencies; let reaction = REACTION_THROW; - if (obj.onerror) { + if (objectButNotArray(obj) && 'onerror' in obj) { const behavior = obj.onerror; if (behavior === 'throw') { } else if (behavior === 'exit') { @@ -160,319 +439,96 @@ class Manifest { } this.#reaction = reaction; - const jsonResourcesEntries = ObjectEntries(obj.resources ?? {}); - const jsonScopesEntries = ObjectEntries(obj.scopes ?? {}); - - function searchDependencies(resourceHREF, 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( - resourceHREF, - 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; - } - - /** - * @param {string} resourceHREF - * @param {{[key: string]: string | true}} dependencyMap - * @param {boolean} cascade - * @returns {DependencyMapper} - */ - const createDependencyMapper = ( - resourceHREF, - dependencyMap, - cascade - ) => { - let parentDeps; - return (toSpecifier, conditions) => { - if (toSpecifier in dependencyMap !== true) { - if (cascade === true) { - /** @type {string | null} */ - let scopeHREF = resourceHREF; - if (typeof parentDeps === 'undefined') { - do { - scopeHREF = this.#findScopeHREF(scopeHREF); - if (scopeHREF === resourceHREF) { - scopeHREF = null; - } - if (scopes.has(scopeHREF)) { - break; - } - } while ( - scopeHREF !== null - ); - parentDeps = scopes.get(scopeHREF); - } - return parentDeps(toSpecifier); - } - return null; - } - const to = searchDependencies( - resourceHREF, - dependencyMap[toSpecifier], - conditions); - if (to === true) { - return true; - } - if (parsedURLs.has(to)) { - return parsedURLs.get(to); - } else if (canBeRequiredByUsers(to)) { - const href = `node:${to}`; - const resolvedURL = new URL(href); - parsedURLs.set(to, resolvedURL); - parsedURLs.set(href, resolvedURL); - return resolvedURL; - } else if (RegExpPrototypeTest(kRelativeURLStringPattern, to)) { - const resolvedURL = new URL(to, manifestURL); - const href = resourceHREF; - parsedURLs.set(to, resolvedURL); - parsedURLs.set(href, resolvedURL); - return resolvedURL; - } - const resolvedURL = new URL(to); - const href = resourceHREF; - parsedURLs.set(to, resolvedURL); - parsedURLs.set(href, resolvedURL); - return resolvedURL; - }; - }; - - /** - * Stores URLs keyed by string specifier relative to the manifest - * @type {Map} - */ - const parsedURLs = new SafeMap(); + const jsonResourcesEntries = ObjectEntries( + obj.resources ?? ObjectCreate(null) + ); + const jsonScopesEntries = ObjectEntries(obj.scopes ?? ObjectCreate(null)); + const defaultDependencies = obj.dependencies ?? ObjectCreate(null); - /** - * Resolves a valid url string against the manifest - * @param {string} originalHREF - * @returns {string} - */ - const resolve = (originalHREF) => { - if (parsedURLs.has(originalHREF)) { - return parsedURLs.get(originalHREF).href; - } else if ( - RegExpPrototypeTest(kRelativeURLStringPattern, originalHREF) - ) { - const resourceURL = new URL(originalHREF, manifestURL); - const resourceHREF = resourceURL.href; - parsedURLs.set(originalHREF, resourceURL); - parsedURLs.set(resourceURL.href, resourceURL); - return resourceHREF; - } - const resourceURL = new URL(originalHREF); - const resourceHREF = resourceURL.href; - parsedURLs.set(originalHREF, resourceURL); - return resourceHREF; - }; - - /** - * @param {string} resourceHREF - * @param {DependencyMap} dependencyMap - * @param {boolean} cascade - * @param {Map} store - */ - const insertDependencyMap = ( - resourceHREF, - dependencyMap, - cascade, - store - ) => { - if (cascade !== undefined && typeof cascade !== 'boolean') { - throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD( - resourceHREF, - 'cascade'); - } - if (dependencyMap === null || dependencyMap === undefined) { - dependencyMap = ObjectCreate(null); - } - if ( - typeof dependencyMap === 'object' && - !ArrayIsArray(dependencyMap) - ) { - const dependencyRedirectList = createDependencyMapper( - resourceHREF, - dependencyMap, - cascade); - store.set(resourceHREF, dependencyRedirectList); - return; - } else if (dependencyMap === true) { - const arbitraryDependencies = /** @type {()=>true} */() => true; - store.set(resourceHREF, arbitraryDependencies); - return; - } - throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD( - resourceHREF, - 'dependencies'); - }; - /** - * Does a special allowance for scopes to be non-valid URLs - * that are only protocol strings - * @param {string} resourceHREF - * @returns {string} - */ - const protocolOrResolve = (resourceHREF) => { - if (StringPrototypeEndsWith(resourceHREF, ':')) { - // URL parse will trim these anyway, save the compute - resourceHREF = StringPrototypeReplace( - resourceHREF, - // eslint-disable-next-line - /^[\x00-\x1F\x20]|\x09\x0A\x0D|[\x00-\x1F\x20]$/g, - '' - ); - if (RegExpPrototypeTest(/^[a-zA-Z][a-zA-Z+\-.]*:$/, resourceHREF)) { - return resourceHREF; - } - } - return resolve(resourceHREF); - }; + this.#defaultDependencies = new DependencyMapperInstance( + 'default', + defaultDependencies === true ? kFallThrough : defaultDependencies, + false + ); for (let i = 0; i < jsonResourcesEntries.length; i++) { - const { 0: originalHREF, 1: resourceDescriptor } = - jsonResourcesEntries[i]; - const cascade = resourceDescriptor.cascade; - const dependencyMap = resourceDescriptor.dependencies; - const resourceHREF = resolve(originalHREF); + const { 0: originalHREF, 1: descriptor } = jsonResourcesEntries[i]; + const { cascade, dependencies, integrity } = descriptor; + const href = resolve(originalHREF, manifestHREF).href; - const integrity = resourceDescriptor.integrity; if (typeof integrity !== 'undefined') { debug('Manifest contains integrity for resource %s', originalHREF); - if (integrities.has(resourceHREF)) { - if (integrities.get(resourceHREF) !== integrity) { - throw new ERR_MANIFEST_INTEGRITY_MISMATCH(resourceHREF); - } - } if (typeof integrity === 'string') { - integrities.set(resourceHREF, integrity); + integrities.set(href, integrity); } else if (integrity === true) { - integrities.set(resourceHREF, true); + integrities.set(href, true); } else { - throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD( - resourceHREF, - 'integrity'); + throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD(href, 'integrity'); } } else { - integrities.set(resourceHREF, cascade ? kCascade : null); + integrities.set(href, cascade === true ? kCascade : false); } - insertDependencyMap(resourceHREF, dependencyMap, cascade, dependencies); + insertDependencyMap( + href, + dependencies, + cascade, + true, + resourceDependencies + ); } const scopeIntegrities = this.#scopeIntegrities; for (let i = 0; i < jsonScopesEntries.length; i++) { - const { 0: originalHREF, 1: scopeDescriptor } = jsonScopesEntries[i]; - const integrity = scopeDescriptor.integrity; - const cascade = scopeDescriptor.cascade; - const dependencyMap = scopeDescriptor.dependencies; - const resourceHREF = protocolOrResolve(originalHREF); + const { 0: originalHREF, 1: descriptor } = jsonScopesEntries[i]; + const { cascade, dependencies, integrity } = descriptor; + const href = emptyOrProtocolOrResolve(originalHREF, manifestHREF); if (typeof integrity !== 'undefined') { debug('Manifest contains integrity for scope %s', originalHREF); - if (scopeIntegrities.has(resourceHREF)) { - if (scopeIntegrities.get(resourceHREF) !== integrity) { - throw new ERR_MANIFEST_INTEGRITY_MISMATCH(resourceHREF); - } - } if (integrity === true) { - scopeIntegrities.set(resourceHREF, true); + scopeIntegrities.set(href, true); } else { - throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD( - resourceHREF, - 'integrity'); + throw new ERR_MANIFEST_INVALID_RESOURCE_FIELD(href, 'integrity'); } } else { - scopeIntegrities.set(resourceHREF, cascade ? kCascade : null); + scopeIntegrities.set(href, cascade === true ? kCascade : false); } - insertDependencyMap(resourceHREF, dependencyMap, cascade, scopes); + insertDependencyMap(href, dependencies, cascade, false, scopes); } ObjectFreeze(this); } - /** - * Finds the longest key within `this.#scopeDependencies` that covers a - * specific HREF - * @param {string} href - * @returns {null | string} - */ - #findScopeHREF = (href) => { - let currentURL = new URL(href); - let protocol = currentURL.protocol; - // Non-opaque blobs adopt origins - if (protocol === 'blob:' && currentURL.origin !== 'null') { - currentURL = new URL(currentURL.origin); - protocol = currentURL.protocol; - } - // Only a few schemes are hierarchical - if (kSpecialSchemes.has(currentURL.protocol)) { - // Make first '..' act like '.' - if (!StringPrototypeEndsWith(currentURL.pathname, '/')) { - currentURL.pathname += '/'; - } - let lastHREF; - let currentHREF = currentURL.href; - do { - if (this.#scopeDependencies.has(currentHREF)) { - return currentHREF; - } - lastHREF = currentHREF; - currentURL = new URL('..', currentURL); - currentHREF = currentURL.href; - } while (lastHREF !== currentHREF); - } - if (this.#scopeDependencies.has(protocol)) { - return protocol; - } - return null; - } - - #createResolver = (resolve) => { - return { - resolve: (to, conditions) => resolve(`${to}`, conditions), - reaction: this.#reaction - }; - } - /** * @param {string} requester + * @returns {{resolve: any, reaction: (err: any) => void}} */ getDependencyMapper(requester) { const requesterHREF = `${requester}`; const dependencies = this.#resourceDependencies; - if (dependencies.has(requesterHREF)) { - return this.#createResolver( - dependencies.get(requesterHREF) || - (() => null) - ); - } - const scopes = this.#scopeDependencies; - if (scopes.size !== 0) { - const scopeHREF = this.#findScopeHREF(requesterHREF); - if (typeof scopeHREF === 'string') { - return this.#createResolver(scopes.get(scopeHREF)); - } - } - return this.#createResolver(() => null); + /** + * @type {DependencyMapperInstance} + */ + const instance = ( + dependencies.has(requesterHREF) ? + dependencies.get(requesterHREF) ?? null : + this.getScopeDependencyMapper(requesterHREF, true) + ) ?? this.#defaultDependencies; + return { + resolve: (specifier, conditions) => { + const normalizedSpecifier = canonicalizeSpecifier( + specifier, + requesterHREF + ); + const result = instance._resolveAlreadyNormalized( + normalizedSpecifier, + conditions, + this + ); + if (result === kFallThrough) return true; + return result; + }, + reaction: this.#reaction, + }; } assertIntegrity(url, content) { @@ -496,10 +552,7 @@ class Manifest { if (ArrayIsArray(integrityEntries)) { // Avoid clobbered Symbol.iterator for (let i = 0; i < integrityEntries.length; i++) { - const { - algorithm, - value: expected - } = integrityEntries[i]; + const { algorithm, value: expected } = integrityEntries[i]; const hash = createHash(algorithm); // TODO(tniessen): the content should not be passed as a string in the // first place, see https://github.com/nodejs/node/issues/39707 @@ -509,10 +562,7 @@ class Manifest { timingSafeEqual(digest, expected)) { return true; } - realIntegrities.set( - algorithm, - BufferToString(digest, 'base64') - ); + realIntegrities.set(algorithm, BufferToString(digest, 'base64')); } } @@ -521,7 +571,7 @@ class Manifest { this.#reaction(error); } } - let scope = this.#findScopeHREF(href); + let scope = findScopeHREF(href, this.#scopeIntegrities, true); while (scope !== null) { if (this.#scopeIntegrities.has(scope)) { const entry = this.#scopeIntegrities.get(scope); @@ -532,8 +582,8 @@ class Manifest { break; } } - const nextScope = this.#findScopeHREF(new URL('..', scope)); - if (!nextScope || nextScope === scope) { + const nextScope = findScopeHREF(scope, this.#scopeDependencies, false); + if (!nextScope) { break; } scope = nextScope; @@ -541,6 +591,24 @@ class Manifest { const error = new ERR_MANIFEST_ASSERT_INTEGRITY(url, realIntegrities); this.#reaction(error); } + /** + * @param {string} href + * @param {boolean} allowSameHREFScope + * @returns {DependencyMapperInstance | null} + */ + getScopeDependencyMapper(href, allowSameHREFScope) { + if (href === null) { + return this.#defaultDependencies; + } + /** @type {string | null} */ + const scopeHREF = findScopeHREF( + href, + this.#scopeDependencies, + allowSameHREFScope + ); + if (scopeHREF === null) return this.#defaultDependencies; + return this.#scopeDependencies.get(scopeHREF); + } } // Lock everything down to avoid problems even if reference is leaked somehow @@ -549,3 +617,107 @@ ObjectSetPrototypeOf(Manifest.prototype, null); ObjectFreeze(Manifest); ObjectFreeze(Manifest.prototype); module.exports = ObjectFreeze({ Manifest }); + +// #region URL utils + +/** + * Attempts to canonicalize relative URL strings against a base URL string + * Does not perform I/O + * If not able to canonicalize, returns the original specifier + * + * This effectively removes the possibility of the return value being a relative + * URL string + * @param {string} specifier + * @param {string} base + * @returns {string} + */ +function canonicalizeSpecifier(specifier, base) { + try { + if (RegExpPrototypeTest(kRelativeURLStringPattern, specifier)) { + return resolve(specifier, base).href; + } + return resolve(specifier).href; + } catch {} + return specifier; +} + +/** + * Does a special allowance for scopes to be non-valid URLs + * that are only protocol strings or the empty string + * @param {string} resourceHREF + * @param {string} [base] + * @returns {string} + */ +const emptyOrProtocolOrResolve = (resourceHREF, base) => { + if (resourceHREF === '') return ''; + if (StringPrototypeEndsWith(resourceHREF, ':')) { + // URL parse will trim these anyway, save the compute + resourceHREF = StringPrototypeReplace( + resourceHREF, + // eslint-disable-next-line + /^[\x00-\x1F\x20]|\x09\x0A\x0D|[\x00-\x1F\x20]$/g, + '' + ); + if (RegExpPrototypeTest(/^[a-zA-Z][a-zA-Z+\-.]*:$/, resourceHREF)) { + return resourceHREF; + } + } + return resolve(resourceHREF, base).href; +}; + +/** + * @type {Map} + */ +let parsedURLs; +/** + * Resolves a valid url string and uses the parsed cache to avoid double parsing + * costs. + * @param {string} originalHREF + * @param {string} [base] + * @returns {Readonly} + */ +const resolve = (originalHREF, base) => { + parsedURLs = parsedURLs ?? new SafeMap(); + if (parsedURLs.has(originalHREF)) { + return parsedURLs.get(originalHREF); + } else if (RegExpPrototypeTest(kRelativeURLStringPattern, originalHREF)) { + const resourceURL = new URL(originalHREF, base); + parsedURLs.set(resourceURL.href, resourceURL); + return resourceURL; + } + const resourceURL = new URL(originalHREF); + parsedURLs.set(originalHREF, resourceURL); + return resourceURL; +}; + +// #endregion + +/** + * @param {any} o + * @returns {o is object} + */ +function objectButNotArray(o) { + return o && typeof o === 'object' && !ArrayIsArray(o); +} + +function searchDependencies(href, target, conditions) { + if (objectButNotArray(target)) { + const keys = ObjectKeys(target); + for (let i = 0; i < keys.length; i++) { + const key = keys[i]; + if (conditions.has(key)) { + const ret = searchDependencies(href, 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(href, 'dependencies'); + } + return null; +} diff --git a/test/fixtures/policy/canonicalize.mjs b/test/fixtures/policy/canonicalize.mjs new file mode 100644 index 00000000000000..64e7cd117ad7c0 --- /dev/null +++ b/test/fixtures/policy/canonicalize.mjs @@ -0,0 +1,5 @@ +import resolveAsFS from './dep.js'; +import fs from 'fs'; + +let correct = resolveAsFS === fs && typeof resolveAsFS === 'object'; +process.exit(correct ? 0 : 1); diff --git a/test/fixtures/policy/dependencies/dependencies-missing-policy-default-true.json b/test/fixtures/policy/dependencies/dependencies-missing-policy-default-true.json new file mode 100644 index 00000000000000..10ab862d2b0cc0 --- /dev/null +++ b/test/fixtures/policy/dependencies/dependencies-missing-policy-default-true.json @@ -0,0 +1,13 @@ +{ + "dependencies": true, + "resources": { + "../parent.js": { + "cascade": true, + "integrity": true + }, + "../dep.js": { + "cascade": true, + "integrity": true + } + } +} \ No newline at end of file diff --git a/test/fixtures/policy/dependencies/dependencies-redirect-builtin-policy.json b/test/fixtures/policy/dependencies/dependencies-redirect-builtin-policy.json index 437228a2e50277..7d2715f96a5092 100644 --- a/test/fixtures/policy/dependencies/dependencies-redirect-builtin-policy.json +++ b/test/fixtures/policy/dependencies/dependencies-redirect-builtin-policy.json @@ -3,7 +3,7 @@ "../parent.js": { "integrity": true, "dependencies": { - "./dep.js": "node:util" + "../dep.js": "node:util" } } } diff --git a/test/fixtures/policy/dependencies/dependencies-redirect-policy.json b/test/fixtures/policy/dependencies/dependencies-redirect-policy.json index 993a683f10a38f..c5b73403d5f864 100644 --- a/test/fixtures/policy/dependencies/dependencies-redirect-policy.json +++ b/test/fixtures/policy/dependencies/dependencies-redirect-policy.json @@ -3,7 +3,7 @@ "../parent.js": { "integrity": true, "dependencies": { - "./dep.js": "../dep.js" + "../dep.js": "../dep.js" } }, "../dep.js": { diff --git a/test/fixtures/policy/dependencies/dependencies-redirect-unknown-builtin-policy.json b/test/fixtures/policy/dependencies/dependencies-redirect-unknown-builtin-policy.json index db2046c6d36f07..a2bcb2676223d3 100644 --- a/test/fixtures/policy/dependencies/dependencies-redirect-unknown-builtin-policy.json +++ b/test/fixtures/policy/dependencies/dependencies-redirect-unknown-builtin-policy.json @@ -3,7 +3,7 @@ "../parent.js": { "integrity": true, "dependencies": { - "./dep.js": "node:404" + "../dep.js": "node:404" } } } diff --git a/test/fixtures/policy/dependencies/dependencies-scopes-relative-specifier.json b/test/fixtures/policy/dependencies/dependencies-scopes-relative-specifier.json new file mode 100644 index 00000000000000..7ce0b0262e48a8 --- /dev/null +++ b/test/fixtures/policy/dependencies/dependencies-scopes-relative-specifier.json @@ -0,0 +1,12 @@ +{ + "scopes": { + "file:": { + "integrity": true, + "cascade": true, + "dependencies": { + "fs": "node:fs", + "../dep.js": "node:fs" + } + } + } +} diff --git a/test/parallel/test-policy-dependencies.js b/test/parallel/test-policy-dependencies.js index 157da8295266db..decc164dadd253 100644 --- a/test/parallel/test-policy-dependencies.js +++ b/test/parallel/test-policy-dependencies.js @@ -16,12 +16,13 @@ const dep = fixtures.path('policy', 'parent.js'); 'policy', 'dependencies', 'dependencies-redirect-policy.json'); - const { status } = spawnSync( + const { status, stderr, stdout } = spawnSync( process.execPath, [ '--experimental-policy', depPolicy, dep, ] ); + console.log('%s\n%s', stderr, stdout); assert.strictEqual(status, 0); } { @@ -55,12 +56,13 @@ const dep = fixtures.path('policy', 'parent.js'); 'policy', 'dependencies', 'dependencies-wildcard-policy.json'); - const { status } = spawnSync( + const { status, stderr, stdout } = spawnSync( process.execPath, [ '--experimental-policy', depPolicy, dep, ] ); + console.log('%s\n%s', stderr, stdout); assert.strictEqual(status, 0); } { @@ -76,6 +78,19 @@ const dep = fixtures.path('policy', 'parent.js'); ); assert.strictEqual(status, 1); } +{ + const depPolicy = fixtures.path( + 'policy', + 'dependencies', + 'dependencies-missing-policy-default-true.json'); + const { status } = spawnSync( + process.execPath, + [ + '--experimental-policy', depPolicy, dep, + ] + ); + assert.strictEqual(status, 0); +} { const depPolicy = fixtures.path( 'policy', @@ -107,5 +122,25 @@ const dep = fixtures.path('policy', 'parent.js'); assert.match( `${stderr}`, /SyntaxError: Named export 'doesNotExist' not found\./, - 'Should give the real SyntaxError and position'); + new Error('Should give the real SyntaxError and position')); +} +{ + const depPolicy = fixtures.path( + 'policy', + 'dependencies', + 'dependencies-scopes-relative-specifier.json'); + const { status } = spawnSync( + process.execPath, + [ + '--experimental-policy', + depPolicy, + fixtures.path('policy', 'canonicalize.mjs'), + ] + ); + assert.strictEqual( + status, + 0, + new Error( + 'policies should canonicalize specifiers by default prior to matching') + ); } diff --git a/test/parallel/test-policy-scopes-dependencies.js b/test/parallel/test-policy-scopes-dependencies.js index 53c44886146bfe..f4b93a08e58c47 100644 --- a/test/parallel/test-policy-scopes-dependencies.js +++ b/test/parallel/test-policy-scopes-dependencies.js @@ -38,6 +38,41 @@ const assert = require('assert'); ); } } + { + const manifest = new Manifest({ + scopes: { + '': { + dependencies: true + } + } + }); + + for (const href of baseURLs) { + assert.strictEqual( + manifest.getDependencyMapper(href).resolve('fs'), + true + ); + } + } + { + const manifest = new Manifest({ + scopes: { + '': { + dependencies: true + }, + 'file:': { + cascade: true + } + } + }); + + for (const href of baseURLs) { + assert.strictEqual( + manifest.getDependencyMapper(href).resolve('fs'), + true + ); + } + } { const manifest = new Manifest({ scopes: {