diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 6d9ecd1ea555d..a9e29479482cb 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -13,6 +13,7 @@ const { lstat, readlink } = require('node:fs/promises') const { depth } = require('treeverse') const { log, time } = require('proc-log') const { redact } = require('@npmcli/redact') +const { isRegistryResolvedTarball } = require('../registry-resolved-tarball.js') const { OK, @@ -1028,6 +1029,9 @@ This is a one-time fix-up, please be patient... Arborist, resolved: node.resolved, integrity: node.integrity, + // pacote's npa re-parses node.resolved as type=remote, so allowRemote would mis-fire on registry tarballs. + // Override only when we can prove the URL is registry-mediated; see isRegistryResolvedTarball. + ...(isRegistryResolvedTarball(node, this) ? { allowRemote: 'all' } : {}), }) await new Arborist({ ...this.options, path }) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index b099d4d72c6a4..46990b93d4e9c 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -1,10 +1,8 @@ // mixin implementing the reify method const PackageJson = require('@npmcli/package-json') -const hgi = require('hosted-git-info') const npa = require('npm-package-arg') const packageContents = require('@npmcli/installed-package-contents') const pacote = require('pacote') -const { pickRegistry } = require('npm-registry-fetch') const promiseAllRejectLate = require('promise-all-reject-late') const runScript = require('@npmcli/run-script') const { callLimit: promiseCallLimit } = require('promise-call-limit') @@ -32,6 +30,10 @@ const Shrinkwrap = require('../shrinkwrap.js') const { defaultLockfileVersion } = Shrinkwrap const { saveTypeMap, hasSubKey } = require('../add-rm-pkg-deps.js') const { IsolatedNode, IsolatedLink } = require('../isolated-classes.js') +const { + registryResolved: getRegistryResolved, + isRegistryResolvedTarball, +} = require('../registry-resolved-tarball.js') // Part of steps (steps need refactoring before we can do anything about these) const _retireShallowNodes = Symbol.for('retireShallowNodes') @@ -706,9 +708,9 @@ module.exports = cls => class Reifier extends cls { // entirely, since we can't possibly reify it. let res = null if (node.resolved) { - const registryResolved = this.#registryResolved(node.resolved) - if (registryResolved) { - res = `${node.name}@${registryResolved}` + const resolved = getRegistryResolved(node.resolved, this) + if (resolved) { + res = `${node.name}@${resolved}` } } else if (node.package.name && node.version) { res = `${node.package.name}@${node.version}` @@ -748,8 +750,8 @@ module.exports = cls => class Reifier extends cls { e.valid && (e.from?.isProjectRoot || e.from?.isWorkspace) ), // pacote's npa re-parses our `name@URL` spec as type=remote, so allowRemote would mis-fire on registry tarballs. - // Override only when we can prove the URL is registry-mediated; see #isRegistryResolvedTarball. - ...(this.#isRegistryResolvedTarball(node) ? { allowRemote: 'all' } : {}), + // Override only when we can prove the URL is registry-mediated; see isRegistryResolvedTarball. + ...(isRegistryResolvedTarball(node, this) ? { allowRemote: 'all' } : {}), }) // store nodes don't use Node class so node.package doesn't get updated if (node.isInStore) { @@ -982,80 +984,6 @@ module.exports = cls => class Reifier extends cls { return realpathSync(child.path) !== realpathSync(child.realpath) } - // When extracting a registry-resolved package, the spec we hand to pacote is name@URL. - // pacote re-parses that with npa and gets spec.type === 'remote', so without an override the allow-remote gate would fire on every registry tarball (both =none and =root mis-fire). - // Returns true only when we are confident this is a registry-mediated install. - #isRegistryResolvedTarball (node) { - if (!node.resolved || !node.isRegistryDependency) { - return false - } - try { - // Match the effective fetch URL, not the raw lockfile value. - // #registryResolved applies replace-registry-host, rewriting a public-registry pin to the configured proxy/mirror so it matches. - const resolvedURL = new URL(this.#registryResolved(node.resolved)) - // pickRegistry only consults spec.scope, so a bare-name (tag) parse is sufficient and avoids a node.version dependency. - const registry = new URL(pickRegistry(npa(node.name), this.options)) - const registryPath = registry.pathname.replace(/\/?$/, '/') - return resolvedURL.origin === registry.origin && - (registryPath === '/' || resolvedURL.pathname.startsWith(registryPath)) - } catch { - return false - } - } - - #registryResolved (resolved) { - // the default registry url is a magic value meaning "the currently - // configured registry". - // `resolved` must never be falsey. - // - // XXX: use a magic string that isn't also a valid value, like - // ${REGISTRY} or something. This has to be threaded through the - // Shrinkwrap and Node classes carefully, so for now, just treat - // the default reg as the magical animal that it has been. - try { - const resolvedURL = hgi.parseUrl(resolved) - const registryURL = new URL(this.registry) - const registryPath = registryURL.pathname.replace(/\/$/, '') - - let matchURL = null - try { - matchURL = new URL(this.options.replaceRegistryHost) - } catch { - // keep matchURL null - } - - const matchHost = matchURL?.hostname ?? this.options.replaceRegistryHost - const matchPath = matchURL?.pathname.replace(/\/$/, '') ?? null - const hasPathPrefix = (pathname, prefix) => - pathname === prefix || pathname.startsWith(`${prefix}/`) - - const hostMatches = this.options.replaceRegistryHost === 'always' || matchHost === resolvedURL.hostname - const pathMatches = !matchPath || hasPathPrefix(resolvedURL.pathname, matchPath) - - if (!hostMatches || !pathMatches) { - return resolved - } - - resolvedURL.protocol = registryURL.protocol - resolvedURL.hostname = registryURL.hostname - resolvedURL.port = registryURL.port - - if (matchPath) { - // full-URL prefix: swap old path prefix for the registry path - resolvedURL.pathname = registryPath + resolvedURL.pathname.slice(matchPath.length) - } else if (registryPath && !hasPathPrefix(resolvedURL.pathname, registryPath)) { - // host-only: prepend registry path if not already present - resolvedURL.pathname = registryPath + resolvedURL.pathname - } - - return resolvedURL.toString() - } catch { - // if we could not parse the url at all then returning nothing - // here means it will get removed from the tree in the next step - return undefined - } - } - // bundles are *sort of* like shrinkwraps, in that the branch is defined // by the contents of the package. however, in their case, rather than // shipping a virtual tree that must be reified, they ship an entire diff --git a/workspaces/arborist/lib/registry-resolved-tarball.js b/workspaces/arborist/lib/registry-resolved-tarball.js new file mode 100644 index 0000000000000..d20f84cc8d878 --- /dev/null +++ b/workspaces/arborist/lib/registry-resolved-tarball.js @@ -0,0 +1,85 @@ +// Shared by reify.js (#extractOrLink) and build-ideal-tree.js (the +// --complete/--dry-run "crack open" path): both hand pacote a `name@URL` or +// bare URL spec for a registry-hosted tarball. pacote's npa re-parses that +// as spec.type === 'remote', so without an override the allow-remote gate +// would mis-fire on every registry tarball (both allowRemote=none and +// allowRemote=root). isRegistryResolvedTarball() tells the caller when it is +// safe to pass `allowRemote: 'all'` to bypass that false positive. +const hgi = require('hosted-git-info') +const npa = require('npm-package-arg') +const { pickRegistry } = require('npm-registry-fetch') + +// the default registry url is a magic value meaning "the currently +// configured registry". +// `resolved` must never be falsey. +// +// XXX: use a magic string that isn't also a valid value, like +// ${REGISTRY} or something. This has to be threaded through the +// Shrinkwrap and Node classes carefully, so for now, just treat +// the default reg as the magical animal that it has been. +const registryResolved = (resolved, arb) => { + try { + const resolvedURL = hgi.parseUrl(resolved) + const registryURL = new URL(arb.registry) + const registryPath = registryURL.pathname.replace(/\/$/, '') + + let matchURL = null + try { + matchURL = new URL(arb.options.replaceRegistryHost) + } catch { + // keep matchURL null + } + + const matchHost = matchURL?.hostname ?? arb.options.replaceRegistryHost + const matchPath = matchURL?.pathname.replace(/\/$/, '') ?? null + const hasPathPrefix = (pathname, prefix) => + pathname === prefix || pathname.startsWith(`${prefix}/`) + + const hostMatches = arb.options.replaceRegistryHost === 'always' || matchHost === resolvedURL.hostname + const pathMatches = !matchPath || hasPathPrefix(resolvedURL.pathname, matchPath) + + if (!hostMatches || !pathMatches) { + return resolved + } + + resolvedURL.protocol = registryURL.protocol + resolvedURL.hostname = registryURL.hostname + resolvedURL.port = registryURL.port + + if (matchPath) { + // full-URL prefix: swap old path prefix for the registry path + resolvedURL.pathname = registryPath + resolvedURL.pathname.slice(matchPath.length) + } else if (registryPath && !hasPathPrefix(resolvedURL.pathname, registryPath)) { + // host-only: prepend registry path if not already present + resolvedURL.pathname = registryPath + resolvedURL.pathname + } + + return resolvedURL.toString() + } catch { + // if we could not parse the url at all then returning nothing + // here means it will get removed from the tree in the next step + return undefined + } +} + +// Returns true only when we are confident this is a registry-mediated +// install, i.e. it is safe to pass `allowRemote: 'all'` to pacote. +const isRegistryResolvedTarball = (node, arb) => { + if (!node.resolved || !node.isRegistryDependency) { + return false + } + try { + // Match the effective fetch URL, not the raw lockfile value. + // registryResolved() applies replace-registry-host, rewriting a public-registry pin to the configured proxy/mirror so it matches. + const resolvedURL = new URL(registryResolved(node.resolved, arb)) + // pickRegistry only consults spec.scope, so a bare-name (tag) parse is sufficient and avoids a node.version dependency. + const registry = new URL(pickRegistry(npa(node.name), arb.options)) + const registryPath = registry.pathname.replace(/\/?$/, '/') + return resolvedURL.origin === registry.origin && + (registryPath === '/' || resolvedURL.pathname.startsWith(registryPath)) + } catch { + return false + } +} + +module.exports = { registryResolved, isRegistryResolvedTarball } diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 6b1da789deddf..902563c8a8765 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -395,6 +395,22 @@ t.test('bundle deps example 1, complete:true', async t => { }), 'no missing deps, because complete: true, add dep, save bundled') }) +t.test('complete:true cracks open a registry-hosted bundle tarball under allowRemote=none', async t => { + // complete:true is what buildIdealTree runs under for --dry-run and + // --package-lock-only (see reify.js, `complete: this.options.packageLockOnly || this.options.dryRun`). + // The tarball URL handed to pacote.extract here is registry-hosted, but pacote's + // npa re-parses a bare URL as spec.type === 'remote', so without the same + // allowRemote override reify.js applies for its own extract call, this + // mis-fired EALLOWREMOTE on ordinary registry installs (npm/cli#9800). + const path = resolve(fixtures, 'testing-bundledeps-empty') + createRegistry(t, true) + + await t.resolves(buildIdeal(path, { + complete: true, + allowRemote: 'none', + }), 'registry-hosted bundle tarball is allowed under allowRemote=none') +}) + t.test('bundle deps example 2', async t => { // bundled deps at the root level are NOT ignored when building ideal trees const path = resolve(fixtures, 'testing-bundledeps-2')