diff --git a/packages/cli/src/ai-context/__tests__/sources.spec.ts b/packages/cli/src/ai-context/__tests__/sources.spec.ts new file mode 100644 index 00000000..f928cb05 --- /dev/null +++ b/packages/cli/src/ai-context/__tests__/sources.spec.ts @@ -0,0 +1,47 @@ +import { readdir, readFile } from 'node:fs/promises' +import { dirname, join, sep } from 'node:path' +import { fileURLToPath } from 'node:url' + +import { describe, expect, it } from 'vitest' + +const aiContextDir = join(dirname(fileURLToPath(import.meta.url)), '..') + +// Only the labelled markers. A bare `=======` line is also a legal setext +// heading underline, so flagging it would fail this suite on a valid file. +const MARKER = /^(<{7}|>{7})( |$)/m + +describe('AI context sources', () => { + it('ships no unresolved merge conflict markers', async () => { + // Everything here is copied into dist verbatim and then either served + // to users by the skills command or written into scaffolded projects, + // and the parts most likely to carry a bad merge are parsed by nothing + // that would notice: markdown by no tool at all, and + // onboarding-boilerplate by neither tsc (tsconfig.json excludes the + // directory) nor vitest (vitest.config.mts excludes it), with its + // `__checks__` specs also outside eslint's reach. A botched resolution + // there ships to users with every other check still green. + // + // Negative control first: a matcher that stopped matching would leave + // this suite green with a real marker in the tree, which is the whole + // failure this guard exists to prevent. + expect('<<<<<<< HEAD\nx\n=======\ny\n>>>>>>> theirs\n').toMatch(MARKER) + expect('# A heading\n=======\n\nnormal text\n').not.toMatch(MARKER) + + const entries = await readdir(aiContextDir, { recursive: true, withFileTypes: true }) + const files = entries + .filter(entry => entry.isFile()) + .map(entry => join(entry.parentPath, entry.name)) + // Tests are not shipped, and this file holds a marker on purpose: + // scanning it would make the guard's own fixture trip it. + .filter(file => !file.includes(`${sep}__tests__${sep}`)) + // Guard the guard: a directory walk that silently finds nothing would + // pass forever. + expect(files.length).toBeGreaterThan(20) + + for (const file of files) { + const content = (await readFile(file, 'utf8')).replace(/\r\n/g, '\n') + expect(content, `${file} contains a merge conflict marker`) + .not.toMatch(MARKER) + } + }) +}) diff --git a/packages/cli/src/ai-context/references/configure-playwright-checks.md b/packages/cli/src/ai-context/references/configure-playwright-checks.md index 2ffb1615..79d85fc4 100644 --- a/packages/cli/src/ai-context/references/configure-playwright-checks.md +++ b/packages/cli/src/ai-context/references/configure-playwright-checks.md @@ -16,7 +16,8 @@ - In a workspace (monorepo) whose code bundle covers only part of the workspace, the bundled lockfile is pruned automatically: the CLI regenerates it (via `pnpm install --lockfile-only` / `npm install --package-lock-only` / `bun install --lockfile-only` / `yarn install --mode=update-lockfile` in a temp dir) so it only references the packages actually in the bundle — otherwise the remote install would try to fetch dependencies of workspace members that were omitted or shipped as dependency-free placeholder manifests, which fails outright for private packages. Supported for `pnpm-lock.yaml` versions 6/9, `package-lock.json` versions 2/3, the text `bun.lock` version 1 and Yarn Berry `yarn.lock` files (for bun projects, keep registry configuration in `.npmrc`, which bun reads: `bunfig.toml` is not carried into the regeneration — recorded resolutions keep their URLs, but whenever bun declines to reuse the lockfile — it is out of date with a manifest, or a workspace member's name collides with a registry dependency — bun re-resolves those entries against the wrong registry, disclosing the package names to it (typically the public registry), and pruning rejects the result with a warning; for yarn projects, `.yarnrc.yml` is likewise not carried into the regeneration, which is safe because Berry lockfiles are registry-agnostic and the regeneration reuses recorded resolutions without the network — settings like `approvedGitRepositories` and `npmScopes` only affect new resolutions, which pruning never performs — and the regeneration runs with yarn's network access disabled outright, since it never needs it: a lockfile that is out of date with a manifest then fails fast with a warning instead of resolving the missing package against the wrong registry and disclosing its name; yarn's hardened mode is disabled for the same reason; `yarn patch` files under `.yarn/patches` are bundled automatically because the regeneration reads them; a `yarn` binary that resolves to Yarn Classic on a Berry project is refused before it can run, because Classic would silently perform a full install); when a bundled lockfile over-describes a partial-workspace bundle but pruning cannot run — other lockfile formats, Yarn Classic v1 lockfiles, a `yarn` binary that resolves to Yarn Classic on a Berry project (set the `packageManager` field so Corepack provisions Yarn 2+), bun's binary `bun.lockb` (regenerate a text lockfile with `bun install --save-text-lockfile`), the package manager binary not being installed on the machine running the CLI, `excludeLinksFromLockfile`, a recorded pnpmfile checksum without a bundled pnpmfile, a workspace member whose version cannot be determined, among others — the original lockfile ships unchanged and the CLI prints a note saying so. Other skips are silent: nothing to prune (the bundle contains the full workspace, or regeneration produced identical bytes), no bundled lockfile to prune, or pruning disabled via `CHECKLY_LOCKFILE_PRUNE=0`; silent skip reasons are visible via `DEBUG='checkly:cli:services:check-parser:*'`. When pruning runs but cannot produce a provably pruned copy of the original — the lockfile is out of date with a `package.json`, the package manager could not run or timed out, the lockfile could not be read or written, or verification failed, among others — the original ships unchanged with a warning. For pnpm projects that patch a dependency (`patchedDependencies` in `pnpm-workspace.yaml` or in the root `package.json`'s `pnpm` field), pruning also filters the patches: a bundle carries the whole map but only part of the workspace, so a patch whose package belongs to an unbundled member would apply to nothing, which pnpm rejects whenever it resolves the bundle (the CLI hits this while regenerating the lockfile). Once the lockfile has been pruned, the declarations it shows no longer apply are removed from the bundled config, their patch files are left out of the bundle, and the matching entries are removed from the bundled lockfile, so the three agree. Declarations the project's own lockfile never recorded are left alone (the pnpm that wrote it may not read the declaration site), as are projects that declare patches in both places at once, since pnpm honors only one of them and which one depends on the pnpm version, and patch files kept outside the conventional `patches/` directory (their declaration is still removed; only the file stays). Leaving a declaration in place is safe as long as the bundled lockfile still records it — pnpm only rejects an unused patch when it re-resolves — but a bundle whose config declares a patch its lockfile does not record can fail the remote install with `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH` or `ERR_PNPM_UNUSED_PATCH`, so the CLI prints a note naming those declarations; refresh the lockfile with your own install to clear it. Patch files no declaration references are bundled as-is. Set `CHECKLY_LOCKFILE_PRUNE=0` to disable pruning. - Checkly caches installed dependencies between runs, keyed off the workspace's lock file, every workspace member's `package.json` and `.npmrc` (whether or not the member is in the bundle), bundled pnpmfile contents, and the resolved `bundle.packages.embed` tarball set (filtered to what the pruned lockfile still references when pruning applied) — plus, as additional inputs, any synthesized placeholder manifests shipped in the bundle and the pruned lockfile when pruning applied. Because the bundle-specific inputs follow the bundle, the key can change without a file edit — e.g. when a different set of workspace members ends up in the bundle. To force a reinstall declaratively, set `caching.dependencyCache.version` (a string or a safe integer) at the top level of `checkly.config.ts` (not per check — one code bundle serves all Playwright Check Suites) and change its value whenever the cache should be invalidated; scheduled checks pick up the change on the next `checkly deploy`. Unset or empty-string values leave the cache key unchanged, so a dynamic value such as `version: process.env.DEPENDENCY_CACHE_VERSION` is safe when the variable is not always set. For a one-off reinstall during an ad-hoc run, use the `--refresh-cache` flag available on the run/test commands (`checkly test`, `checkly pw-test`, `checkly trigger`, `checkly checks run`) instead; the config value is the persistent knob that also applies to deployed, scheduled checks. - In Checkly CLI v8.0.0 and later, `include` patterns resolve relative to the Playwright config directory, not the project root. If `playwrightConfigPath` points to a subdirectory, adjust `include` globs. Example: `playwrightConfigPath: "./e2e/playwright.config.ts"` with a root fixture at `fixtures/data.json` needs `include: ["../fixtures/data.json"]`. -- If dependencies come from a private registry that Checkly's infrastructure cannot reach (for example an intranet-only Nexus mirror), list them in `bundle.packages.embed` in `checkly.config.ts` — a top-level section: `bundle: { packages: { embed: ['@acme/private-utils', 'legacy-private-pkg@2.1.0'] } }`. Each entry is a package name (embeds every version found in the lockfile) or an exact `name@version` pin; names may contain `*` wildcards (`@acme/*`, `acme-*`, `@acme/*-utils`) where each `*` matches any run of characters except `/` (never crossing the scope separator); a `!` prefix (`!@acme/legacy`, `!@acme/*`, `!legacy@2.1.0`) turns an entry into an exclusion that removes the packages it matches from what the entries *before* it selected, so entries apply in order — `['@acme/*', '!@acme/legacy']` embeds the whole scope except `@acme/legacy`, while the reverse order embeds the whole scope because the exclusion runs before anything has been selected; as long as a spec matches at least one registry package, matches that cannot be embedded are skipped (workspace members silently, git/file/URL dependencies and integrity-less entries with a warning since the runner must fetch those itself), while a spec whose only matches cannot be embedded — or that matches nothing at all — is an error, except that exclusions never error (one that removes nothing is a no-op) and removing every package an earlier entry selected also silences that entry — no error, and no skip warning even for packages it matched but did not exclude, so use `DEBUG='checkly:cli:services:embedded-packages'` to see what such an entry reached; because exclusions only subtract, a list of nothing but `!` entries selects nothing, and a configuration whose entries select no packages at all is reported as a warning (packages dropped later by lockfile pruning are covered by the pruning note above); a pattern embeds every lockfile version of every package it matches, so scope it to the packages the runner genuinely cannot fetch. List every unreachable package by name, including private packages that only appear as transitive dependencies of other private packages — dependencies of listed packages are not embedded automatically. The CLI resolves entries against the workspace-root lockfile (`pnpm-lock.yaml`, `package-lock.json`, the text `bun.lock` or a Yarn Berry `yarn.lock` — Yarn Classic v1 lockfiles are not supported), reuses tarballs from local caches (its own, then npm's) or downloads them from the registry configured in `.npmrc` (only `.npmrc` — bun or yarn users whose registry credentials live solely in `bunfig.toml` or `.yarnrc.yml` must duplicate them into `.npmrc`, or downloads fail with an auth error), verifies each against the lockfile's integrity hash, and ships them inside the code bundle at `.checkly/embedded-packages/*.tgz`, where the runner serves them through a local registry during install. Yarn Berry lockfiles record no npm tarball integrity (Berry checksums cover yarn's own cache format), so the CLI resolves the tarball integrity from the registry's package metadata instead — one small metadata request per embedded package on every deploy (the per-version route, falling back to the full packument), even when the tarballs themselves come from a warm cache, so a yarn embed needs registry reachability at deploy time even on a warm cache. When the bundled lockfile is pruned to the code bundle's contents (see the pruning bullet above), the embedded set follows it: packages the pruned lockfile no longer references — dependencies of workspace members that are not part of the bundle — are neither embedded nor downloaded, even if an entry matches them. If a package unexpectedly stops being embedded, the usual cause is that only a workspace member outside the bundle depends on it, in which case the runner never installs it and nothing is wrong; if the checks genuinely need it, make the depending member part of the bundle (import it from check code) rather than disabling pruning — `CHECKLY_LOCKFILE_PRUNE=0` restores the unfiltered set but reintroduces the over-describing lockfile that pruning exists to prevent, so treat it as a last resort. Downloads are cached under the workspace root's `node_modules/.cache/checkly` (in a monorepo that is the repo root, not the member package; override with `CHECKLY_CACHE_DIR`; a per-user cache dir is the fallback when the project location isn't writable), so nothing lands in the project outside `node_modules`. CI setups that cache `node_modules` — or platforms that preserve `node_modules/.cache` — persist the tarballs automatically; otherwise persist `CHECKLY_CACHE_DIR` in CI to avoid re-downloading (note `npm ci` deletes `node_modules` wholesale, unlike incremental pnpm installs). The machine running `checkly deploy`/`test` needs registry access on a cold cache — but only for the tarballs actually shipped, not for pruned-away ones. Changing the resolved set of embedded packages invalidates the runner's dependency cache, so the next run reinstalls with the new tarballs. Applies to Playwright Check Suites only, not browser or multistep checks. +- If dependencies come from a private registry that Checkly's infrastructure cannot reach (for example an intranet-only Nexus mirror), list them in `bundle.packages.embed` in `checkly.config.ts` — a top-level section: `bundle: { packages: { embed: ['@acme/private-utils', 'legacy-private-pkg@2.1.0'] } }`. Each entry is a package name (embeds every version found in the lockfile) or an exact `name@version` pin; names may contain `*` wildcards (`@acme/*`, `acme-*`, `@acme/*-utils`) where each `*` matches any run of characters except `/` (never crossing the scope separator); a `!` prefix (`!@acme/legacy`, `!@acme/*`, `!legacy@2.1.0`) turns an entry into an exclusion that removes the packages it matches from what the entries *before* it selected, so entries apply in order — `['@acme/*', '!@acme/legacy']` embeds the whole scope except `@acme/legacy`, while the reverse order embeds the whole scope because the exclusion runs before anything has been selected; as long as a spec matches at least one registry package, matches that cannot be embedded are skipped (workspace members silently, git/file/URL dependencies and integrity-less entries with a warning since the runner must fetch those itself), while a spec whose only matches cannot be embedded — or that matches nothing at all — is an error, except that exclusions never error (one that removes nothing is a no-op) and removing every package an earlier entry selected also silences that entry — no error, and no skip warning even for packages it matched but did not exclude, so use `DEBUG='checkly:cli:services:embedded-packages'` to see what such an entry reached; because exclusions only subtract, a list of nothing but `!` entries selects nothing, and a configuration whose entries select no packages at all is reported as a warning (packages dropped later by lockfile pruning are covered by the pruning note above); a pattern embeds every lockfile version of every package it matches, so scope it to the packages the runner genuinely cannot fetch. List every unreachable package by name, including private packages that only appear as transitive dependencies of other private packages — dependencies of listed packages are not embedded automatically. The CLI resolves entries against the workspace-root lockfile (`pnpm-lock.yaml`, `package-lock.json`, the text `bun.lock` or a Yarn Berry `yarn.lock` — Yarn Classic v1 lockfiles are not supported), reuses tarballs from local caches (its own, then npm's) or downloads them from the registry configured in `.npmrc` (see the credentials bullet below), verifies each against the lockfile's integrity hash, and ships them inside the code bundle at `.checkly/embedded-packages/*.tgz`, where the runner serves them through a local registry during install. Yarn Berry lockfiles record no npm tarball integrity (Berry checksums cover yarn's own cache format), so the CLI resolves the tarball integrity from the registry's package metadata instead — one small metadata request per embedded package on every deploy (the per-version route, falling back to the full packument), even when the tarballs themselves come from a warm cache, so a yarn embed needs registry reachability at deploy time even on a warm cache. When the bundled lockfile is pruned to the code bundle's contents (see the pruning bullet above), the embedded set follows it: packages the pruned lockfile no longer references — dependencies of workspace members that are not part of the bundle — are neither embedded nor downloaded, even if an entry matches them. If a package unexpectedly stops being embedded, the usual cause is that only a workspace member outside the bundle depends on it, in which case the runner never installs it and nothing is wrong; if the checks genuinely need it, make the depending member part of the bundle (import it from check code) rather than disabling pruning — `CHECKLY_LOCKFILE_PRUNE=0` restores the unfiltered set but reintroduces the over-describing lockfile that pruning exists to prevent, so treat it as a last resort. Downloads are cached under the workspace root's `node_modules/.cache/checkly` (in a monorepo that is the repo root, not the member package; override with `CHECKLY_CACHE_DIR`; a per-user cache dir is the fallback when the project location isn't writable), so nothing lands in the project outside `node_modules`. CI setups that cache `node_modules` — or platforms that preserve `node_modules/.cache` — persist the tarballs automatically; otherwise persist `CHECKLY_CACHE_DIR` in CI to avoid re-downloading (note `npm ci` deletes `node_modules` wholesale, unlike incremental pnpm installs). The machine running `checkly deploy`/`test` needs registry access on a cold cache — but only for the tarballs actually shipped, not for pruned-away ones. Changing the resolved set of embedded packages invalidates the runner's dependency cache, so the next run reinstalls with the new tarballs. Applies to Playwright Check Suites only, not browser or multistep checks. +- Credentials for a private registry (`bundle.packages.embed`) must be scoped to a registry — the `//host/path/:_authToken` form, not a bare `_authToken` at the top of the file, which npm itself rejects with `ERR_INVALID_AUTH` and `npm config fix`. They come from `npm_config_*` environment variables, the project, workspace-root and user `.npmrc` files, and pnpm's global `auth.ini` — where `pnpm login` writes tokens on pnpm 11+, outranking the user `.npmrc` for pnpm projects and acting as a fallback for others. Scope-qualified keys (`//registry.example.com/:@acme:_authToken`, the spelling `pnpm login --scope=@acme` writes) are honored for packages in that scope in any project, whichever package manager it uses, and every scoped key is tried before any unscoped one regardless of path depth; pnpm's alternative spelling (`//registry.example.com/@acme/:_authToken`) is honored after the unscoped keys. A key that cannot be used is never quietly swapped for a different one: an unset `${VAR}` fails the download and names the variable, and a key left blank counts as absent for its own credential but still masks that same key elsewhere, matching npm. Keys that could not have applied to the request at all are skipped, visible only under `DEBUG='checkly:cli:services:embedded-packages'`. A `registry` that is blank or otherwise unusable is a broken setting rather than an absent one, so the download fails and names the key instead of falling back to the public registry and disclosing a private package name to it, though a blank `@scope:registry` does fall back to a usable global `registry`. bun or yarn users whose registry credentials live solely in `bunfig.toml` or `.yarnrc.yml` must duplicate them into `.npmrc` or set `npm_config_*`, or downloads fail with an auth error. A download that fails to authenticate names where the credentials came from — a config key and its file, an environment variable, the lockfile that recorded the URL, or the registry that issued it — so tell the user to read the error rather than guessing which source to edit. ## Install troubleshooting diff --git a/packages/cli/src/services/checkly-config-loader.ts b/packages/cli/src/services/checkly-config-loader.ts index 596c22ff..b20d0140 100644 --- a/packages/cli/src/services/checkly-config-loader.ts +++ b/packages/cli/src/services/checkly-config-loader.ts @@ -145,11 +145,16 @@ export type ChecklyConfig = { * lockfiles record no npm tarball integrity, so it is resolved from * the registry's package metadata instead — one small metadata * request per embedded package on every deploy, even with a warm - * cache. Downloads read registry credentials from `.npmrc` only; - * bun or yarn users whose credentials live solely in `bunfig.toml` - * or `.yarnrc.yml` must duplicate them into `.npmrc` — referencing - * them through environment variables (`${NPM_TOKEN}`), never as - * plaintext, because `.npmrc` is uploaded with the code bundle. When the bundled lockfile is pruned to the code + * cache. Downloads read registry credentials from `npm_config_*` + * environment variables, the project, workspace-root and user + * `.npmrc` files, and pnpm's global `auth.ini` (where `pnpm login` + * writes tokens on pnpm 11 and later), including the + * scope-qualified keys `pnpm login --scope` writes. bun or yarn + * users whose credentials live solely in `bunfig.toml` or + * `.yarnrc.yml` must duplicate them into `.npmrc` or set + * `npm_config_*` — referencing a token through an environment + * variable (`${NPM_TOKEN}`), never as plaintext, because `.npmrc` + * is uploaded with the code bundle. When the bundled lockfile is pruned to the code * bundle's contents, the embedded set follows it: packages the pruned * lockfile no longer references — dependencies of workspace members * that are not part of the bundle — are neither embedded nor diff --git a/packages/cli/src/services/embedded-packages/__tests__/diagnostics.spec.ts b/packages/cli/src/services/embedded-packages/__tests__/diagnostics.spec.ts new file mode 100644 index 00000000..3bbce208 --- /dev/null +++ b/packages/cli/src/services/embedded-packages/__tests__/diagnostics.spec.ts @@ -0,0 +1,213 @@ +import { describe, it, expect } from 'vitest' + +import { UNPRINTABLE_URL, describeUnusableUrlOrigin, downloadFailureHint, redactUrl } from '../diagnostics.js' +import { ConfigOrigin, LoadedNpmrcConfig } from '../npmrc.js' + +function npmrc (overrides: Partial = {}): LoadedNpmrcConfig { + return { + config: new Map(), + files: ['/ws/.npmrc', '/home/u/.npmrc'], + unreadable: [], + origins: new Map(), + ...overrides, + } +} + +describe('redactUrl()', () => { + // Only scheme and host survive. The path goes too: some registries take + // a token as a path segment, and the package name and version the path + // encodes are already stated separately in every message that shows a URL. + it.each([ + ['userinfo', 'https://user:tok@nexus.local/npm/foo.tgz', 'https://nexus.local'], + ['a port with userinfo', 'https://u:p@nexus.local:8443/npm/f.tgz', 'https://nexus.local:8443'], + ['a query, which may hold a pre-signed signature', + 'https://cdn.example.com/f.tgz?X-Amz-Signature=deadbeef', 'https://cdn.example.com'], + ['a fragment', 'https://cdn.example.com/f.tgz#tok=deadbeef', 'https://cdn.example.com'], + ['a token in a path segment', 'https://nexus.local/s3cret-token/npm/f.tgz', 'https://nexus.local'], + // The WHATWG parser strips surrounding whitespace, so this is a normal + // parse rather than one of the malformed shapes below. + ['userinfo, ignoring leading whitespace', ' https://user:tok@nexus.local/x', 'https://nexus.local'], + ])('keeps only scheme and host, dropping %s', (_label, input, expected) => { + expect(redactUrl(input)).toBe(expected) + }) + + // Each of these leaked a credential through an earlier string-surgery + // implementation. None can be redacted reliably — telling userinfo from a + // path in a malformed string needs a parser — so none is echoed at all. + it.each([ + ['no scheme and no slashes', 'admin:s3cret@nexus.local/npm/'], + ['an @ inside the password', '//user:p@ss@nexus.local/npm/'], + ['a scheme with an out-of-range port', 'https://user:tok@nexus.local:99999/x'], + ['whitespace inside the credential', 'https://user:pa ss@nexus.local:99999/x'], + ['an extra leading slash', '///user:tok@nexus.local/x'], + ['an unencoded slash in the password', '//user:pa/ss@nexus.local/x'], + ['a scheme-less host and port', 'nexus.local:8443/npm/@acme/foo.tgz'], + // Withheld for the same reason it cannot be requested: what may be + // echoed and what may be fetched are one rule. + ['a scheme nothing here fetches', 'ftp://user:tok@nexus.local/x'], + ])('withholds an unusable URL with %s', (_label, input) => { + const redacted = redactUrl(input) + expect(redacted).toBe(UNPRINTABLE_URL) + for (const secret of ['s3cret', 'tok', 'p@ss', 'pa ss', 'pa/ss']) { + expect(redacted).not.toContain(secret) + } + }) +}) + +describe('downloadFailureHint()', () => { + it('says nothing for a status authentication cannot explain', () => { + expect(downloadFailureHint(500, undefined, npmrc())).toBe('') + }) + + // The URL in the message is the one that was requested, so a hop has to + // be reported whatever the status — otherwise the message names a host + // that produced nothing. + it('reports a redirect for a status authentication cannot explain', () => { + expect(downloadFailureHint(500, undefined, npmrc(), { host: 'cdn.example.com' })) + .toContain(`redirected to 'cdn.example.com', which is what answered`) + }) + + it('does not claim the redirect target answered when nothing did', () => { + // No status means no response at all — a reset or a timeout. + const hint = downloadFailureHint(undefined, undefined, npmrc(), { host: 'cdn.example.com' }) + expect(hint).toContain(`redirected to 'cdn.example.com' before it failed`) + expect(hint).not.toContain('is what answered') + }) + + it('names every source consulted when no credentials matched', () => { + const hint = downloadFailureHint(401, undefined, npmrc()) + expect(hint).toContain('npm_config_* environment variables') + expect(hint).toContain('/ws/.npmrc') + expect(hint).toContain('/home/u/.npmrc') + }) + + it('hedges a 404 as a possible authorization failure', () => { + expect(downloadFailureHint(404, undefined, npmrc())) + .toMatch(/may answer 404 for a package you are not authorized to see/) + }) + + it('names the file a rejected credential came from', () => { + const origins = new Map([ + ['//nexus.local/:_authToken', { kind: 'file', path: '/home/u/.npmrc' }], + ]) + const hint = downloadFailureHint(401, { from: 'config', keys: ['//nexus.local/:_authToken'] }, npmrc({ origins })) + expect(hint).toContain(`'//nexus.local/:_authToken' in '/home/u/.npmrc'`) + expect(hint).toMatch(/were rejected/) + }) + + it('names an environment variable by its verbatim spelling', () => { + const origins = new Map([ + ['registry', { kind: 'env', variable: 'NPM_CONFIG_REGISTRY' }], + ]) + const hint = downloadFailureHint(401, { from: 'config', keys: ['registry'] }, npmrc({ origins })) + expect(hint).toContain(`the 'NPM_CONFIG_REGISTRY' environment variable`) + expect(hint).not.toContain('npm_config_registry') + }) + + it('reports both halves of a username/password pair separately', () => { + const origins = new Map([ + ['//h/:username', { kind: 'file', path: '/ws/.npmrc' }], + ['//h/:_password', { kind: 'env', variable: 'npm_config_//h/:_password' }], + ]) + const hint = downloadFailureHint(401, { from: 'config', keys: ['//h/:username', '//h/:_password'] }, npmrc({ origins })) + expect(hint).toContain(`'//h/:username' in '/ws/.npmrc'`) + expect(hint).toContain(`the 'npm_config_//h/:_password' environment variable`) + }) + + it('blames a redirect that dropped the credentials, not the credentials', () => { + const hint = downloadFailureHint( + 404, + { from: 'config', keys: ['//h/:_authToken'] }, + npmrc(), + { host: 'cdn.example.com', credentialsDropped: true }, + ) + expect(hint).toContain(`redirected to 'cdn.example.com'`) + expect(hint).not.toMatch(/were rejected/) + }) + + // Without this the reader is told to configure credentials for a host + // that never asked for any. + it('mentions a redirect even when no credentials were configured', () => { + const hint = downloadFailureHint(404, undefined, npmrc(), { host: 'cdn.example.com' }) + expect(hint).toContain(`redirected to 'cdn.example.com'`) + expect(hint).toContain('which is what answered') + }) + + it('mentions a redirect that carried the credentials through', () => { + const hint = downloadFailureHint( + 401, + { from: 'config', keys: ['//h/:_authToken'] }, + npmrc(), + { host: 'other.example.com' }, + ) + expect(hint).toMatch(/were rejected/) + expect(hint).toContain(`redirect to 'other.example.com'`) + }) + + it('reports a config file that could not be read', () => { + const hint = downloadFailureHint(401, undefined, npmrc({ unreadable: ['/home/u/pnpm/auth.ini'] })) + expect(hint).toContain('/home/u/pnpm/auth.ini') + expect(hint).toMatch(/could not be read/) + }) + + it('never repeats a credential value', () => { + // The hint is assembled from keys and paths only; nothing in its inputs + // carries a value, and this pins that the config map is not consulted. + const config = new Map([['//h/:_authToken', 'super-secret']]) + const hint = downloadFailureHint(401, { from: 'config', keys: ['//h/:_authToken'] }, npmrc({ config })) + expect(hint).not.toContain('super-secret') + }) +}) + +describe('describeUnusableUrlOrigin()', () => { + const origins = new Map([ + ['registry', { kind: 'file', path: '/ws/.npmrc' }], + ]) + + it('sends the reader to the lockfile when the lockfile recorded the URL', () => { + const described = describeUnusableUrlOrigin({ lockfile: '/ws/pnpm-lock.yaml' }, npmrc()) + expect(described).toContain('recorded in \'/ws/pnpm-lock.yaml\'') + expect(described).not.toContain('registry') + }) + + it('does not send the reader to a registry setting that does not exist', () => { + // The metadata branch also covers the case where nothing configures a + // registry, where telling someone to check "the setting" is a dead end. + const described = describeUnusableUrlOrigin({ metadata: {} }, npmrc()) + expect(described).toContain('nothing here configures a registry') + expect(described).not.toContain('check that the setting') + }) + + it('says a metadata URL is the registry\'s to correct, not the project\'s', () => { + const described = describeUnusableUrlOrigin({ metadata: { registryKey: 'registry' } }, npmrc({ origins })) + expect(described).toContain('package metadata served by') + expect(described).toContain('\'registry\' in \'/ws/.npmrc\'') + // Not the wording used when the project itself composed the URL. + expect(described).not.toContain('malformed package name') + }) +}) + +describe('credentials carried by a URL', () => { + const origins = new Map([ + ['registry', { kind: 'file', path: '/ws/.npmrc' }], + ]) + + it('names the registry URL once, not twice', () => { + // An earlier revision nested 'the registry configured by' inside 'the + // registry URL configured by', which read as gibberish. + const hint = downloadFailureHint(401, { from: 'url', origin: { registryKey: 'registry' } }, npmrc({ origins })) + expect(hint).toContain('came from the URL of the registry configured by \'registry\' in \'/ws/.npmrc\'.') + expect(hint).not.toContain('configured by the registry configured by') + }) + + it('says a metadata-supplied URL was issued by the registry, not configured locally', () => { + const sent = { from: 'url', origin: { metadata: { registryKey: 'registry' } } } as const + const hint = downloadFailureHint(401, sent, npmrc({ origins })) + expect(hint).toContain('issued by that registry rather than configured here') + }) + + it('names the lockfile for a URL it recorded', () => { + const sent = { from: 'url', origin: { lockfile: '/ws/yarn.lock' } } as const + expect(downloadFailureHint(401, sent, npmrc())).toContain('recorded in \'/ws/yarn.lock\'') + }) +}) diff --git a/packages/cli/src/services/embedded-packages/__tests__/lockfile-packages.spec.ts b/packages/cli/src/services/embedded-packages/__tests__/lockfile-packages.spec.ts index d3c88463..319820b2 100644 --- a/packages/cli/src/services/embedded-packages/__tests__/lockfile-packages.spec.ts +++ b/packages/cli/src/services/embedded-packages/__tests__/lockfile-packages.spec.ts @@ -6,6 +6,7 @@ import { describe, it, expect } from 'vitest' import { UnsupportedLockfileError, + isPnpmLockfile, loadLockfilePackages, parseBunLockfilePackages, parseLockfilePackagesContent, @@ -14,6 +15,23 @@ import { parseYarnLockfilePackages, } from '../lockfile-packages.js' +describe('isPnpmLockfile()', () => { + it('recognizes a pnpm lockfile by basename', () => { + expect(isPnpmLockfile('pnpm-lock.yaml')).toBe(true) + expect(isPnpmLockfile(path.join('/ws', 'pnpm-lock.yaml'))).toBe(true) + }) + + it('rejects other lockfiles', () => { + expect(isPnpmLockfile(path.join('/ws', 'package-lock.json'))).toBe(false) + expect(isPnpmLockfile(path.join('/ws', 'yarn.lock'))).toBe(false) + expect(isPnpmLockfile(path.join('/ws', 'bun.lock'))).toBe(false) + }) + + it('does not match a directory that merely contains the name', () => { + expect(isPnpmLockfile(path.join('/ws', 'pnpm-lock.yaml', 'nested.json'))).toBe(false) + }) +}) + describe('parsePnpmLockfilePackages()', () => { it('parses v9 registry entries', () => { const { registry, excluded } = parsePnpmLockfilePackages(` diff --git a/packages/cli/src/services/embedded-packages/__tests__/materializer.spec.ts b/packages/cli/src/services/embedded-packages/__tests__/materializer.spec.ts index 821d774c..9e887896 100644 --- a/packages/cli/src/services/embedded-packages/__tests__/materializer.spec.ts +++ b/packages/cli/src/services/embedded-packages/__tests__/materializer.spec.ts @@ -677,6 +677,56 @@ packages: {} expect(requests[0].authorization).toBe('Bearer secret') }) + it('sends a scope-qualified credential for a package in that scope', async () => { + // The key `pnpm login --scope=@acme` writes. It only resolves if the + // package being downloaded reaches the credential lookup. + await fs.writeFile(path.join(workspaceRoot, '.npmrc'), [ + `registry=${serverUrl}`, + `//127.0.0.1:${(server.address() as AddressInfo).port}/:@acme:_authToken=scoped-secret`, + ].join('\n')) + + await materializeAll(makeMaterializer(['@acme/foo'])) + expect(requests[0].authorization).toBe('Bearer scoped-secret') + }) + + // Both cases deliberately put a *different* token in each file: with a + // token in only one of them, either ordering resolves the same + // credential and the test could not detect inverted precedence. + // XDG_CONFIG_HOME pins pnpm's config dir on every platform, so neither + // test has to branch on the real process.platform. + const writeCompetingTokens = async () => { + const nerfDart = `//127.0.0.1:${(server.address() as AddressInfo).port}/` + await fs.mkdir(path.join(homedir, 'pnpm'), { recursive: true }) + await fs.writeFile(path.join(homedir, 'pnpm', 'auth.ini'), `${nerfDart}:_authToken=pnpm-token\n`) + await fs.writeFile(path.join(homedir, '.npmrc'), `${nerfDart}:_authToken=npmrc-token\n`) + return { CHECKLY_CACHE_DIR: cacheDir, XDG_CONFIG_HOME: homedir } + } + + it('prefers the pnpm auth file over the user .npmrc for a pnpm lockfile', async () => { + const env = await writeCompetingTokens() + + await materializeAll(makeMaterializer(['bar@2.0.0'], { env })) + expect(requests[0].authorization).toBe('Bearer pnpm-token') + }) + + it('prefers the user .npmrc over the pnpm auth file for an npm lockfile', async () => { + const env = await writeCompetingTokens() + + // No `resolved` field: npm lockfiles normally carry one, and it would + // be used verbatim, sending the request to the real registry instead + // of this test's server. + const npmLockfilePath = path.join(workspaceRoot, 'package-lock.json') + await fs.writeFile(npmLockfilePath, JSON.stringify({ + lockfileVersion: 3, + packages: { + 'node_modules/bar': { version: '2.0.0', integrity: barIntegrity }, + }, + })) + + await materializeAll(makeMaterializer(['bar@2.0.0'], { lockfilePath: npmLockfilePath, env })) + expect(requests[0].authorization).toBe('Bearer npmrc-token') + }) + it('prefers a lockfile-recorded tarball URL over the derived one', async () => { await fs.writeFile(lockfilePath, ` lockfileVersion: '9.0' @@ -694,6 +744,23 @@ packages: expect(requests[0].url).toBe('/custom/path/bar-2.0.0.tgz') }) + it('blames the lockfile, not the registry config, for an unusable recorded URL', async () => { + // The advice has to match the source: sending someone to fix a + // registry setting that is already correct wastes the whole message. + await fs.writeFile(lockfilePath, ` +lockfileVersion: '9.0' +packages: + bar@2.0.0: + resolution: {integrity: ${barIntegrity}, tarball: 'https://'} +`) + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + expect(error.message).toMatch(/tarball URL.*is not a valid URL/s) + expect(error.message).toContain(`recorded in '${lockfilePath}'`) + expect(error.message).not.toContain('registry') + expect(requests).toHaveLength(0) + }) + it('fails with a clear error on an integrity mismatch', async () => { server.removeAllListeners('request') server.on('request', (req, res) => res.end('tampered content')) @@ -713,6 +780,291 @@ packages: .rejects.toThrow(/Failed to download embedded package 'secured@1\.0\.0'.*HTTP 401.*credentials/s) }) + describe('authentication hints', () => { + const securedLockfile = ` +lockfileVersion: '9.0' +packages: + secured@1.0.0: + resolution: {integrity: ${barIntegrity}} +` + + it('names every consulted config file when no credentials matched', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + + // XDG_CONFIG_HOME pins auth.ini's location: the production code + // uses the real process.platform, whose default differs per OS. + const error = await materializeAll(makeMaterializer(['secured'], { + env: { CHECKLY_CACHE_DIR: cacheDir, XDG_CONFIG_HOME: homedir }, + })).catch(err => err) + expect(error.message).toMatch(/No credentials for this registry were found in/) + expect(error.message).toContain(path.join(workspaceRoot, '.npmrc')) + expect(error.message).toContain(path.join(homedir, '.npmrc')) + // pnpm's auth.ini is named alongside the .npmrc files, so a pnpm + // user is not told to edit a file their credentials do not live in. + expect(error.message).toContain(path.join(homedir, 'pnpm', 'auth.ini')) + }) + + it('names the file a rejected credential came from', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + const workspaceNpmrc = path.join(workspaceRoot, '.npmrc') + const nerfDart = `//127.0.0.1:${(server.address() as AddressInfo).port}/` + await fs.writeFile(workspaceNpmrc, [ + `registry=${serverUrl}`, + `${nerfDart}:_authToken=wrong-token`, + ].join('\n')) + + const error = await materializeAll(makeMaterializer(['secured'])).catch(err => err) + expect(error.message).toMatch(/credentials sent for this registry were rejected/) + expect(error.message).toMatch(/expired token/) + // Naming the exact key and file is the whole point: several files + // can supply a credential, and "yours was rejected" without saying + // which one leaves the reader as stuck as a bare status code. + expect(error.message).toContain(`'${nerfDart}:_authToken' in '${workspaceNpmrc}'`) + // Never the credential itself. + expect(error.message).not.toContain('wrong-token') + }) + + it('names the environment variable when the credential came from one', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + const { port } = server.address() as AddressInfo + const envKey = `npm_config_//127.0.0.1:${port}/:_authToken` + + const error = await materializeAll(makeMaterializer(['secured'], { + env: { CHECKLY_CACHE_DIR: cacheDir, [envKey]: 'env-token' }, + })).catch(err => err) + // The stored key has the npm_config_ prefix stripped, so the hint + // must name the variable itself or it names nothing searchable. + expect(error.message).toContain(`the '${envKey}' environment variable`) + expect(error.message).not.toContain('env-token') + }) + + it('names an uppercase environment variable by its real spelling', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + // Shells and CI systems routinely uppercase these. The config map + // case-folds the key, so echoing the key would print a name that + // does not exist in the environment. + const envKey = 'NPM_CONFIG_REGISTRY' + + const error = await materializeAll(makeMaterializer(['secured'], { + env: { CHECKLY_CACHE_DIR: cacheDir, [envKey]: `http://user:pass@127.0.0.1:${ + (server.address() as AddressInfo).port}/` }, + })).catch(err => err) + expect(error.message).toContain(`the '${envKey}' environment variable`) + expect(error.message).not.toContain('npm_config_registry') + expect(error.message).not.toContain('pass@') + }) + + it('attributes credentials in a lockfile-recorded URL to the lockfile', async () => { + const { port } = server.address() as AddressInfo + // npm lockfiles record a `resolved` URL verbatim, and it can carry + // userinfo — in which case no config key is to blame for it. + await fs.writeFile(lockfilePath, ` +lockfileVersion: '9.0' +packages: + secured@1.0.0: + resolution: {integrity: ${barIntegrity}, tarball: http://user:pass@127.0.0.1:${port}/secured/-/secured-1.0.0.tgz} +`) + + const error = await materializeAll(makeMaterializer(['secured'])).catch(err => err) + expect(error.message).toContain(`came from the tarball URL recorded in '${lockfilePath}'`) + expect(error.message).not.toContain('pass@') + }) + + it('blames a cross-host redirect rather than the credentials', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + const { port } = server.address() as AddressInfo + const workspaceNpmrc = path.join(workspaceRoot, '.npmrc') + await fs.writeFile(workspaceNpmrc, [ + `registry=${serverUrl}`, + `//127.0.0.1:${port}/:_authToken=good-token`, + ].join('\n')) + // A second port on the same address: follow-redirects compares the + // host INCLUDING the port, so this is a different host to it and + // the header is stripped — deterministic, with no DNS involved. + const cdn = http.createServer((req, res) => { + requests.push({ url: req.url!, authorization: req.headers.authorization }) + res.statusCode = 404 + res.end('not found') + }) + await new Promise(resolve => cdn.listen(0, '127.0.0.1', resolve)) + const cdnPort = (cdn.address() as AddressInfo).port + + server.removeAllListeners('request') + server.on('request', (req, res) => { + requests.push({ url: req.url!, authorization: req.headers.authorization }) + res.statusCode = 302 + res.setHeader('location', `http://127.0.0.1:${cdnPort}${req.url!}`) + res.end() + }) + + try { + const error = await materializeAll(makeMaterializer(['secured'])).catch(err => err) + // The redirect target never received the token, so saying it was + // rejected would send the reader to rotate a working credential. + expect(error.message).toContain(`redirected to '127.0.0.1:${cdnPort}'`) + expect(error.message).toMatch(/dropped rather than forwarded/) + expect(error.message).not.toMatch(/were rejected/) + // The attribution survives, so the reader still learns which + // source the original host was given. + expect(error.message).toContain(`'//127.0.0.1:${port}/:_authToken' in '${workspaceNpmrc}'`) + expect(requests[0].authorization).toBe('Bearer good-token') + expect(requests[1]?.authorization).toBeUndefined() + } finally { + await new Promise((resolve, reject) => + cdn.close(err => err ? reject(err) : resolve())) + } + }) + + it('still blames the credentials when a redirect keeps them', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + const { port } = server.address() as AddressInfo + const workspaceNpmrc = path.join(workspaceRoot, '.npmrc') + await fs.writeFile(workspaceNpmrc, [ + `registry=${serverUrl}`, + `//127.0.0.1:${port}/:_authToken=good-token`, + ].join('\n')) + // A same-host redirect keeps the Authorization header, so the + // credentials really were seen and rejected. Deriving the drop from + // host comparison rather than observing it would misreport this. + server.removeAllListeners('request') + server.on('request', (req, res) => { + requests.push({ url: req.url!, authorization: req.headers.authorization }) + if (req.url === '/secured/-/secured-1.0.0.tgz') { + res.statusCode = 302 + res.setHeader('location', `http://127.0.0.1:${port}/moved/secured.tgz`) + res.end() + return + } + res.statusCode = 401 + res.end('unauthorized') + }) + + const error = await materializeAll(makeMaterializer(['secured'])).catch(err => err) + expect(error.message).toMatch(/were rejected/) + // Asserted positively: the credentials survived the hop, so the + // message must say they were carried through it rather than + // dropped. A negative assertion here passed on the coincidence + // that the two sentences differ by one word. + expect(error.message).toMatch(/carried through a redirect to/) + expect(error.message).not.toMatch(/dropped rather than forwarded/) + expect(requests[1]?.authorization).toBe('Bearer good-token') + }) + + it('lists the environment channel among the places it looked', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + + const error = await materializeAll(makeMaterializer(['secured'])).catch(err => err) + // npm_config_* outranks every file, so omitting it would send the + // reader to edit files that a set variable would override anyway. + expect(error.message).toContain(`'npm_config_* environment variables'`) + }) + + it('names both files when a username/password pair is split across them', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + const workspaceNpmrc = path.join(workspaceRoot, '.npmrc') + const userNpmrc = path.join(homedir, '.npmrc') + const nerfDart = `//127.0.0.1:${(server.address() as AddressInfo).port}/` + await fs.writeFile(workspaceNpmrc, [ + `registry=${serverUrl}`, + `${nerfDart}:username=alice`, + ].join('\n')) + // The password — the half that actually expires — lives elsewhere. + await fs.writeFile(userNpmrc, `${nerfDart}:_password=${Buffer.from('secret').toString('base64')}\n`) + + const error = await materializeAll(makeMaterializer(['secured'])).catch(err => err) + expect(error.message).toContain(`'${nerfDart}:username' in '${workspaceNpmrc}'`) + expect(error.message).toContain(`'${nerfDart}:_password' in '${userNpmrc}'`) + expect(error.message).not.toContain('secret') + }) + + it('explains a 404 that credentials did not unlock', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + const workspaceNpmrc = path.join(workspaceRoot, '.npmrc') + const nerfDart = `//127.0.0.1:${(server.address() as AddressInfo).port}/` + await fs.writeFile(workspaceNpmrc, [ + `registry=${serverUrl}`, + `${nerfDart}:_authToken=insufficient-token`, + ].join('\n')) + // A registry that hides packages the caller may not see answers 404 + // even once credentials are presented. + server.removeAllListeners('request') + server.on('request', (req, res) => { + requests.push({ url: req.url!, authorization: req.headers.authorization }) + res.statusCode = 404 + res.end('not found') + }) + + const error = await materializeAll(makeMaterializer(['secured'])).catch(err => err) + expect(error.message).toMatch(/HTTP 404/) + expect(error.message).toMatch(/Credentials were sent but did not grant access/) + expect(error.message).toContain(`'${nerfDart}:_authToken' in '${workspaceNpmrc}'`) + expect(error.message).not.toContain('insufficient-token') + }) + + // A registry that hides unauthorized packages behind a 404 is the + // case that reads as "package does not exist" without this hint. + it('explains a 404 as a possible authorization failure', async () => { + await fs.writeFile(lockfilePath, ` +lockfileVersion: '9.0' +packages: + missing@1.0.0: + resolution: {integrity: ${barIntegrity}} +`) + + const error = await materializeAll(makeMaterializer(['missing'])).catch(err => err) + expect(error.message).toMatch(/HTTP 404/) + expect(error.message).toMatch(/may answer 404 for a package you are not authorized to see/) + }) + + it('reports a config file that exists but could not be read', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + const authIni = path.join(homedir, 'pnpm', 'auth.ini') + await fs.mkdir(path.dirname(authIni), { recursive: true }) + await fs.writeFile(authIni, 'registry=https://unreadable.example.com/\n') + await fs.chmod(authIni, 0o000) + try { + await fs.readFile(authIni, 'utf8') + return // Running as root: permission bits do not apply. + } catch { + // Expected: the file is genuinely unreadable. + } + + const error = await materializeAll(makeMaterializer(['secured'], { + env: { CHECKLY_CACHE_DIR: cacheDir, XDG_CONFIG_HOME: homedir }, + })).catch(err => err) + expect(error.message).toContain(authIni) + expect(error.message).toMatch(/could not be read/) + }) + + // axios sends userinfo credentials itself and drops the Authorization + // header when it does, so reporting the config entry would name a + // credential that never left the process. + it('attributes credentials embedded in the registry URL to the key that configured it', async () => { + await fs.writeFile(lockfilePath, securedLockfile) + const workspaceNpmrc = path.join(workspaceRoot, '.npmrc') + const { port } = server.address() as AddressInfo + await fs.writeFile(workspaceNpmrc, [ + `registry=http://user:pass@127.0.0.1:${port}/`, + `//127.0.0.1:${port}/:_authToken=unused-token`, + ].join('\n')) + + const error = await materializeAll(makeMaterializer(['secured'])).catch(err => err) + // The whole clause: an earlier revision nested 'the registry + // configured by' inside 'the registry URL configured by', and a + // prefix match did not notice. + expect(error.message).toMatch(/came from the URL of the registry configured by '[^']+' in '[^']+'\.$/) + expect(error.message).toContain(`'registry' in '${workspaceNpmrc}'`) + expect(error.message).not.toContain('unused-token') + expect(error.message).not.toContain('pass@') + + // The precedence rule rests on axios sending the URL's credentials + // and dropping the Authorization header. Assert the wire, not just + // the wording, so a change in that behaviour fails here. + expect(requests[0].authorization) + .toBe(`Basic ${Buffer.from('user:pass').toString('base64')}`) + }) + }) + it('refuses to materialize when the plan has issues', async () => { await expect(materializeAll(makeMaterializer(['no-such-package']))) .rejects.toThrow(EmbeddedPackageError) @@ -740,11 +1092,49 @@ packages: expect(requests).toHaveLength(1) }) - it('fails with a clear error for a registry URL without a protocol', async () => { - await fs.writeFile(path.join(workspaceRoot, '.npmrc'), 'registry=nexus.local/repository/npm/\n') + it.each([ + // Each breaks a different one of the three rules, and the message + // states all three rather than guessing which: telling the reader of + // a `file:` URL to add a protocol sends them looking for one it has. + ['no protocol', 'nexus.local/repository/npm/'], + ['no host, so the package name would become one', 'https://'], + ['one slash, so the package name would become the host', 'https:/'], + ['a scheme nothing here can fetch', 'ftp://nexus.local/npm/'], + ['a scheme that never has a host', 'file:///srv/npm-mirror/'], + // A query absorbs whatever is appended to it, so the package path + // would vanish into it and every request would hit the root. + ['a query', 'https://nexus.local/repository/npm/?token=abc'], + ['a fragment', 'https://nexus.local/repository/npm/#tok'], + ['a bare query delimiter', 'https://nexus.local/repository/npm/?'], + ['a bare fragment delimiter', 'https://nexus.local/repository/npm/#'], + ])('refuses a registry with %s', async (_label, registry) => { + // `https://` composes into `https://bar/-/bar-2.0.0.tgz`, whose host + // is the package name — a real host somebody else may own. + await fs.writeFile(path.join(workspaceRoot, '.npmrc'), `registry=${registry}\n`) + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + expect(error.message).toMatch(/registry URL.*is not usable/s) + expect(error.message).toMatch(/must be an absolute http or https URL with a host/) + expect(error.message).toContain(`'registry' in '${path.join(workspaceRoot, '.npmrc')}'`) + expect(requests).toHaveLength(0) + }) - await expect(materializeAll(makeMaterializer(['bar@2.0.0']))) - .rejects.toThrow(/is not a valid URL.*registry/s) + // Each of these registry values produces a URL the parser cannot make + // sense of, so redaction falls back to string surgery. Every one of + // them leaked a credential at some point during development. + it.each([ + ['an @ in the password', '//user:p@ss@nexus.local/npm/', ['ss@', 'user:']], + ['no protocol and no leading slashes', 'admin:s3cret@nexus.local/npm/', ['s3cret', 'admin:']], + ['a scheme with an out-of-range port', 'https://user:tok@nexus.local:99999/npm/', ['tok@', 'user:']], + ['whitespace inside the credential', 'https://user:pa ss@nexus.local:99999/npm/', ['pa ss', 'user:']], + ])('redacts credentials from an unparseable registry URL with %s', async (_label, registry, forbidden) => { + await fs.writeFile(path.join(workspaceRoot, '.npmrc'), `registry=${registry}\n`) + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + expect(error.message).toMatch(/is not usable/) + for (const secret of forbidden) { + expect(error.message).not.toContain(secret) + } }) it('redacts registry credentials from download error messages', async () => { @@ -912,11 +1302,147 @@ __metadata: await writeYarnLockfile() serveMetadata({}) - await expect(materializeAll(makeMaterializer(['bar@2.0.0']))) - .rejects.toThrow(/provides no usable integrity hash/) + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + // Not "no usable integrity hash": there was no metadata at all, and a + // private registry answers 404 for packages the caller may not see. + expect(error.message).toMatch(/has no metadata for embedded package/) + expect(error.message).not.toMatch(/provides no usable integrity hash/) + expect(error.message).toMatch(/No credentials for this registry were found/) expect(requests.map(request => request.url)).toEqual(['/bar/2.0.0', '/bar']) }) + it('names the rejected credentials when neither metadata route exists', async () => { + await writeYarnLockfile() + const { port } = server.address() as AddressInfo + await fs.writeFile(path.join(workspaceRoot, '.npmrc'), [ + `registry=${serverUrl}`, + `//127.0.0.1:${port}/:_authToken=stale`, + ].join('\n')) + serveMetadata({}) + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + expect(error.message).toContain(`'//127.0.0.1:${port}/:_authToken'`) + expect(error.message).not.toContain('stale') + }) + + it('does not blame credentials when the metadata answers without this version', async () => { + // The registry accepted the request and simply lacks the version, so + // an authentication hint would send the reader to rotate a token the + // registry had just honoured. + await writeYarnLockfile() + const { port } = server.address() as AddressInfo + await fs.writeFile(path.join(workspaceRoot, '.npmrc'), [ + `registry=${serverUrl}`, + `//127.0.0.1:${port}/:_authToken=works`, + ].join('\n')) + serveMetadata({ '/bar': { versions: { '1.0.0': { dist: {} } } } }) + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + expect(error.message).toMatch(/does not describe embedded package 'bar@2.0.0'/) + expect(error.message).not.toMatch(/[Cc]redentials/) + expect(error.message).not.toMatch(/has no metadata/) + }) + + it('reports a config file it could not read when the metadata has nothing', async () => { + // A skipped `auth.ini` is invisible otherwise, and it is the likeliest + // thing to be missing when a private package cannot be resolved at all. + await writeYarnLockfile() + const authIni = path.join(homedir, 'pnpm', 'auth.ini') + await fs.mkdir(path.dirname(authIni), { recursive: true }) + await fs.writeFile(authIni, '//127.0.0.1/:_authToken=unreadable\n') + await fs.chmod(authIni, 0o000) + try { + await fs.readFile(authIni, 'utf8') + // Root ignores the permission bits, and Windows honours only the + // write bit, so there is nothing unreadable to report. Probing + // beats testing the platform: it is the read that has to fail. + return + } catch { + // Expected: the file is genuinely unreadable. + } + // The registry must ANSWER without the version: a 404 from both + // routes takes the other branch, which already said this. + serveMetadata({ '/bar': { versions: { '1.0.0': { dist: {} } } } }) + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'], { + env: { CHECKLY_CACHE_DIR: cacheDir, XDG_CONFIG_HOME: homedir }, + })).catch(err => err) + expect(error.message).toMatch(/does not describe embedded package/) + expect(error.message).toMatch(/could not be read/) + expect(error.message).toContain(authIni) + }) + + it('treats an explicit null dist as absent and falls back to the packument', async () => { + // A registry may answer with `"dist": null` rather than omitting it. + // Read literally that is neither absent nor usable, and it once both + // suppressed this fallback and crashed on the property read below. + await writeYarnLockfile() + serveMetadata({ + '/bar/2.0.0': { dist: null }, + '/bar': { versions: { '2.0.0': { dist: { integrity: barIntegrity, tarball: `${serverUrl}bar.tgz` } } } }, + '/bar.tgz': barTarball, + }) + + const tarballs = await materializeAll(makeMaterializer(['bar@2.0.0'])) + expect(tarballs).toHaveLength(1) + expect(requests.map(request => request.url)).toContain('/bar') + }) + + it('refuses a host-less registry before requesting metadata', async () => { + // `https://` composes into `https://bar/2.0.0`, whose host is the + // package name — a real host somebody else may own. The composed form + // parses, so only checking the registry URL itself catches it. + await writeYarnLockfile() + await fs.writeFile(path.join(workspaceRoot, '.npmrc'), 'registry=https://\n') + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + expect(error.message).toMatch(/registry URL.*is not usable/s) + expect(requests).toHaveLength(0) + }) + + it('attributes credentials in a metadata-supplied tarball URL to the registry', async () => { + // The registry minted them into the URL its own metadata returned, so + // they are in no file the reader can open — naming the registry + // config line would send them somewhere that holds no credentials. + await writeYarnLockfile() + const { port } = server.address() as AddressInfo + serveMetadata({ + '/bar/2.0.0': { + dist: { integrity: barIntegrity, tarball: `http://svc:tok@127.0.0.1:${port}/bar/-/bar-2.0.0.tgz` }, + }, + '/bar/-/bar-2.0.0.tgz': 401, + }) + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + expect(error.message).toContain('returned in this package\'s metadata') + expect(error.message).toContain('issued by that registry rather than configured here') + // The userinfo itself must never appear; 'tok' alone would match the + // word "token" in the hint's own wording. + expect(error.message).not.toContain('svc:') + expect(error.message).not.toContain('tok@') + }) + + it('blames the registry URL for credentials it carries when metadata is rejected', async () => { + // axios sends userinfo from the URL itself and drops the + // Authorization header when it does, so reporting no credentials + // would tell the reader to add what was in fact sent and rejected. + await writeYarnLockfile() + const { port } = server.address() as AddressInfo + await fs.writeFile( + path.join(workspaceRoot, '.npmrc'), + `registry=http://ci-user:tok@127.0.0.1:${port}/\n`, + ) + serveMetadata({ '/bar/2.0.0': 401 }) + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + // The whole clause: an earlier revision nested 'the registry + // configured by' inside 'the registry URL configured by', and a + // prefix match did not notice. + expect(error.message).toMatch(/came from the URL of the registry configured by '[^']+' in '[^']+'\.$/) + expect(error.message).not.toMatch(/No credentials for this registry were found/) + expect(error.message).not.toContain('ci-user') + }) + it('fails with a clear error when the metadata provides no usable hash', async () => { await writeYarnLockfile() serveMetadata({ @@ -943,6 +1469,62 @@ __metadata: expect(requests[0]).toMatchObject({ url: '/bar/2.0.0', authorization: 'Bearer secret' }) }) + it('sends a scope-qualified credential with the metadata request', async () => { + // The metadata route resolves credentials separately from the + // download, so it needs its own proof that the package name reaches + // the lookup — a scoped key resolves only if it does. + await writeYarnLockfile() + const { port } = server.address() as AddressInfo + await fs.writeFile(path.join(workspaceRoot, '.npmrc'), [ + `registry=${serverUrl}`, + `//127.0.0.1:${port}/:@acme:_authToken=scoped-secret`, + ].join('\n')) + serveMetadata({ + '/@acme/foo/1.2.3': { dist: { integrity: fooIntegrity, tarball: `${serverUrl}@acme/foo/-/foo-1.2.3.tgz` } }, + '/@acme/foo/-/foo-1.2.3.tgz': fooTarball, + }) + + await materializeAll(makeMaterializer(['@acme/foo'])) + expect(requests[0]).toMatchObject({ url: '/@acme/foo/1.2.3', authorization: 'Bearer scoped-secret' }) + }) + + // The metadata route resolves its own registry URL, so the guard on it + // needs its own coverage: the tarball-path cases above run on a pnpm + // lockfile and never reach this code. + it.each([ + ['registry', 'registry=nexus.local/repository/npm/', 'bar@2.0.0'], + // The key named must be the one at fault, not the global default. + ['@acme:registry', '@acme:registry=nexus.local/npm/', '@acme/foo'], + // Parses, but as the opaque scheme `admin:` with no host, so nothing + // can separate the credential from a path — it is withheld entirely. + ['registry', 'registry=admin:s3cret@nexus.local/npm/', 'bar@2.0.0'], + ])('refuses an unusable %s before requesting metadata', async (key, line, spec) => { + await writeYarnLockfile() + await fs.writeFile(path.join(workspaceRoot, '.npmrc'), `${line}\n`) + + const error = await materializeAll(makeMaterializer([spec])).catch(err => err) + expect(error.message).toMatch(/registry URL.*is not usable/s) + expect(error.message).toContain(`'${key}' in '${path.join(workspaceRoot, '.npmrc')}'`) + expect(error.message).not.toContain('s3cret') + expect(error.message).not.toContain('admin:') + expect(requests).toHaveLength(0) + }) + + it('blames the registry, not the lockfile, for an unusable metadata tarball URL', async () => { + // Yarn plans learn the tarball URL from the registry's own metadata, + // so a bad one is not something the project can fix in its lockfile. + await writeYarnLockfile() + serveMetadata({ + '/bar/2.0.0': { dist: { integrity: barIntegrity, tarball: 'https://' } }, + }) + + const error = await materializeAll(makeMaterializer(['bar@2.0.0'])).catch(err => err) + expect(error.message).toMatch(/tarball URL.*is not a valid URL/s) + expect(error.message).toContain('package metadata served by') + expect(error.message).not.toContain(lockfilePath) + expect(error.message).not.toContain('malformed package name') + }) + it('still resolves metadata on a warm cache, but skips the download', async () => { // The caches are keyed by integrity, which for yarn plans is only // learnable from the registry — so the (small) metadata roundtrip diff --git a/packages/cli/src/services/embedded-packages/__tests__/npmrc.spec.ts b/packages/cli/src/services/embedded-packages/__tests__/npmrc.spec.ts index 8370a4ef..76576069 100644 --- a/packages/cli/src/services/embedded-packages/__tests__/npmrc.spec.ts +++ b/packages/cli/src/services/embedded-packages/__tests__/npmrc.spec.ts @@ -11,8 +11,9 @@ import { loadNpmrcConfig, npmrcConfigFromEnv, parseNpmrc, + pnpmAuthIniPath, resolveAuthHeader, - resolveRegistryUrl, + resolveRegistry, } from '../npmrc.js' describe('parseNpmrc()', () => { @@ -52,26 +53,148 @@ describe('loadNpmrcConfig()', () => { await fs.rm(dir, { recursive: true, force: true }) }) + it('lets a blank credential mask the same key in a lower-precedence file', async () => { + // Deliberate parity with npm and pnpm, which both keep blank values + // read from files. Skipping blanks during the merge would let a working + // lower-precedence token through, but then a project that blanks an + // entry on purpose — to force anonymous access — would have the + // developer's personal token sent instead, which npm would never do. + // (Nothing writes these blanks automatically: `npm logout` deletes the + // lines. They come from hand edits, or a script writing an absent + // secret.) + const blank = path.join(dir, 'blank.npmrc') + const working = path.join(dir, 'working.npmrc') + await fs.writeFile(blank, '//nexus.local/:_authToken=\n') + await fs.writeFile(working, '//nexus.local/:_authToken=works\n') + + const { config } = await loadNpmrcConfig([{ path: blank }, { path: working }], {}) + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})).toBeUndefined() + + const { config: reversed } = await loadNpmrcConfig([{ path: working }, { path: blank }], {}) + expect(resolveAuthHeader(reversed, 'https://nexus.local/foo', 'foo', {})?.header).toBe('Bearer works') + }) + + it('does not reach past a blank into another file, even under a different spelling', async () => { + // The case the same-spelling test cannot catch: the two files disagree + // on capitalisation, so the merge keeps both keys and a naive + // case-fallback would send the personal token the project deliberately + // blanked out. npm would go anonymous here. + const blank = path.join(dir, 'blank-cased.npmrc') + const personal = path.join(dir, 'personal-cased.npmrc') + await fs.writeFile(blank, '//nexus.local/:_authToken=\n') + await fs.writeFile(personal, '//nexus.local/:_authtoken=personal-token\n') + + const { config } = await loadNpmrcConfig([{ path: blank }, { path: personal }], {}) + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})).toBeUndefined() + }) + it('gives earlier files precedence and merges the rest', async () => { - const config = await loadNpmrcConfig([ - path.join(dir, 'project.npmrc'), - path.join(dir, 'user.npmrc'), + const { config } = await loadNpmrcConfig([ + { path: path.join(dir, 'project.npmrc') }, + { path: path.join(dir, 'user.npmrc') }, ], {}) expect(config.get('registry')).toBe('https://project.example.com/') expect(config.get('//user.example.com/:_authToken')).toBe('user-token') }) it('skips missing files', async () => { - const config = await loadNpmrcConfig([ - path.join(dir, 'does-not-exist.npmrc'), - path.join(dir, 'project.npmrc'), + const { config } = await loadNpmrcConfig([ + { path: path.join(dir, 'does-not-exist.npmrc') }, + { path: path.join(dir, 'project.npmrc') }, + ], {}) + expect(config.get('registry')).toBe('https://project.example.com/') + }) + + it('lets the pnpm auth file win over the user .npmrc when it ranks higher', async () => { + const authIni = path.join(dir, 'auth.ini') + const userNpmrc = path.join(dir, 'competing-user.npmrc') + await fs.writeFile(authIni, '//registry.example.com/:_authToken=pnpm-token\n') + await fs.writeFile(userNpmrc, '//registry.example.com/:_authToken=npmrc-token\n') + + const { config: preferred } = await loadNpmrcConfig([{ path: authIni, optional: true }, { path: userNpmrc }], {}) + expect(preferred.get('//registry.example.com/:_authToken')).toBe('pnpm-token') + + const { config: notPreferred } = await loadNpmrcConfig([{ path: userNpmrc }, { path: authIni, optional: true }], {}) + expect(notPreferred.get('//registry.example.com/:_authToken')).toBe('npmrc-token') + }) + + it('fails on an unreadable required file but skips an unreadable optional one', async () => { + const unreadable = path.join(dir, 'unreadable.npmrc') + await fs.writeFile(unreadable, 'registry=https://unreadable.example.com/\n') + await fs.chmod(unreadable, 0o000) + try { + // Running as root defeats permission bits entirely, so only assert + // when the mode actually denies this process. + await fs.readFile(unreadable, 'utf8') + return + } catch { + // Expected: the file is genuinely unreadable. + } + + await expect(loadNpmrcConfig([{ path: unreadable }], {})).rejects.toThrow(/Unable to read npm configuration/) + + const { config, unreadable: skipped } = await loadNpmrcConfig([ + { path: unreadable, optional: true }, + { path: path.join(dir, 'project.npmrc') }, ], {}) expect(config.get('registry')).toBe('https://project.example.com/') + // Reported rather than merely skipped, so a later authentication + // failure can say the file was found but not used. + expect(skipped).toEqual([unreadable]) + }) + + it('records which source supplied each key', async () => { + const projectNpmrc = path.join(dir, 'project.npmrc') + const userNpmrc = path.join(dir, 'user.npmrc') + const { origins } = await loadNpmrcConfig( + [{ path: projectNpmrc }, { path: userNpmrc }], + { 'npm_config_//env.example.com/:_authToken': 'env-token' }, + ) + + expect(origins.get('registry')).toEqual({ kind: 'file', path: projectNpmrc }) + expect(origins.get('//user.example.com/:_authToken')).toEqual({ kind: 'file', path: userNpmrc }) + expect(origins.get('//env.example.com/:_authToken')) + .toEqual({ kind: 'env', variable: 'npm_config_//env.example.com/:_authToken' }) + }) + + it('names the environment variable verbatim, whatever its case', async () => { + // The stored key is case-folded, so only the verbatim variable name is + // something the user can search their environment for. + const { origins } = await loadNpmrcConfig([], { NPM_CONFIG_REGISTRY: 'https://env.example.com/' }) + expect(origins.get('registry')).toEqual({ kind: 'env', variable: 'NPM_CONFIG_REGISTRY' }) + expect(origins.get('REGISTRY')).toEqual({ kind: 'env', variable: 'NPM_CONFIG_REGISTRY' }) + }) + + it('reports the config files consulted', async () => { + const projectNpmrc = path.join(dir, 'project.npmrc') + const { files } = await loadNpmrcConfig([{ path: projectNpmrc }], {}) + expect(files).toEqual([projectNpmrc]) + }) + + it('records the origin under the key spelling that actually matched', async () => { + const lowercased = path.join(dir, 'lowercased.npmrc') + await fs.writeFile(lowercased, '//nexus.local/:_authtoken=lower-token\n') + + const { config, origins } = await loadNpmrcConfig([{ path: lowercased }], {}) + const auth = resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {}) + + // resolveAuthHeader asks for the canonical `_authToken` spelling but + // matches the lowercase one; the reported key has to be the spelling + // present in origins, or the source cannot be named. + expect(auth?.header).toBe('Bearer lower-token') + expect(origins.get(auth!.keys[0])).toEqual({ kind: 'file', path: lowercased }) + }) + + it('does not report missing files as unreadable', async () => { + const { unreadable } = await loadNpmrcConfig([ + { path: path.join(dir, 'does-not-exist.npmrc'), optional: true }, + ], {}) + expect(unreadable).toEqual([]) }) it('gives npm_config_* environment variables precedence over files', async () => { - const config = await loadNpmrcConfig( - [path.join(dir, 'project.npmrc')], + const { config } = await loadNpmrcConfig( + [{ path: path.join(dir, 'project.npmrc') }], { npm_config_registry: 'https://env.example.com/' }, ) expect(config.get('registry')).toBe('https://env.example.com/') @@ -94,35 +217,124 @@ describe('npmrcConfigFromEnv()', () => { const config = npmrcConfigFromEnv({ 'npm_config_//nexus.local/:_authToken': 'env-secret', }) - expect(resolveAuthHeader(config, 'https://nexus.local/foo', {})).toBe('Bearer env-secret') + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})?.header).toBe('Bearer env-secret') }) }) describe('defaultNpmrcPaths()', () => { it('orders context dir before workspace root before home', () => { - expect(defaultNpmrcPaths('/ws', '/home/user', '/ws/packages/a')).toEqual([ - path.join('/ws/packages/a', '.npmrc'), - path.join('/ws', '.npmrc'), - path.join('/home/user', '.npmrc'), + expect(defaultNpmrcPaths({ + workspaceRoot: '/ws', + homedir: '/home/user', + contextDir: '/ws/packages/a', + })).toEqual([ + { path: path.join('/ws/packages/a', '.npmrc') }, + { path: path.join('/ws', '.npmrc') }, + { path: path.join('/home/user', '.npmrc') }, ]) }) it('deduplicates when the context dir is the workspace root', () => { - expect(defaultNpmrcPaths('/ws', '/home/user', '/ws')).toEqual([ - path.join('/ws', '.npmrc'), - path.join('/home/user', '.npmrc'), + expect(defaultNpmrcPaths({ + workspaceRoot: '/ws', + homedir: '/home/user', + contextDir: '/ws', + })).toEqual([ + { path: path.join('/ws', '.npmrc') }, + { path: path.join('/home/user', '.npmrc') }, + ]) + }) + + it('ranks the pnpm auth file above the user .npmrc when preferred', () => { + expect(defaultNpmrcPaths({ + workspaceRoot: '/ws', + homedir: '/home/user', + pnpmAuthFile: '/cfg/pnpm/auth.ini', + pnpmAuthFilePreferred: true, + })).toEqual([ + { path: path.join('/ws', '.npmrc') }, + { path: '/cfg/pnpm/auth.ini', optional: true }, + { path: path.join('/home/user', '.npmrc') }, + ]) + }) + + it('ranks the pnpm auth file below the user .npmrc when not preferred', () => { + expect(defaultNpmrcPaths({ + workspaceRoot: '/ws', + homedir: '/home/user', + pnpmAuthFile: '/cfg/pnpm/auth.ini', + pnpmAuthFilePreferred: false, + })).toEqual([ + { path: path.join('/ws', '.npmrc') }, + { path: path.join('/home/user', '.npmrc') }, + { path: '/cfg/pnpm/auth.ini', optional: true }, + ]) + }) + + it('omits the pnpm auth file when none is given', () => { + expect(defaultNpmrcPaths({ workspaceRoot: '/ws', homedir: '/home/user' })).toEqual([ + { path: path.join('/ws', '.npmrc') }, + { path: path.join('/home/user', '.npmrc') }, ]) }) }) -describe('resolveRegistryUrl()', () => { +describe('pnpmAuthIniPath()', () => { + const home = path.sep === '/' ? '/home/user' : 'C:\\Users\\user' + + it('uses macOS preferences on darwin', () => { + expect(pnpmAuthIniPath({}, 'darwin', home)) + .toBe(path.join(home, 'Library', 'Preferences', 'pnpm', 'auth.ini')) + }) + + it('uses ~/.config on linux', () => { + expect(pnpmAuthIniPath({}, 'linux', home)).toBe(path.join(home, '.config', 'pnpm', 'auth.ini')) + }) + + it('uses LOCALAPPDATA on win32', () => { + expect(pnpmAuthIniPath({ LOCALAPPDATA: 'C:\\LocalAppData' }, 'win32', home)) + .toBe(path.join('C:\\LocalAppData', 'pnpm', 'config', 'auth.ini')) + }) + + it('falls back to ~/.config on win32 without LOCALAPPDATA', () => { + expect(pnpmAuthIniPath({}, 'win32', home)).toBe(path.join(home, '.config', 'pnpm', 'auth.ini')) + }) + + it('prefers XDG_CONFIG_HOME on every platform', () => { + for (const platform of ['darwin', 'linux', 'win32'] as NodeJS.Platform[]) { + expect(pnpmAuthIniPath({ XDG_CONFIG_HOME: '/xdg' }, platform, home)) + .toBe(path.join('/xdg', 'pnpm', 'auth.ini')) + } + }) + + it('ignores an empty XDG_CONFIG_HOME', () => { + expect(pnpmAuthIniPath({ XDG_CONFIG_HOME: '' }, 'linux', home)) + .toBe(path.join(home, '.config', 'pnpm', 'auth.ini')) + }) + + // An empty value must not be joined as-is: that would yield a relative + // path and read credentials from the current working directory. + it('ignores an empty LOCALAPPDATA', () => { + expect(pnpmAuthIniPath({ LOCALAPPDATA: '' }, 'win32', home)) + .toBe(path.join(home, '.config', 'pnpm', 'auth.ini')) + }) + + // pnpm consults PNPM_HOME for its data and state directories, never for + // the config directory that holds auth.ini. + it('ignores PNPM_HOME', () => { + expect(pnpmAuthIniPath({ PNPM_HOME: '/pnpm-home' }, 'linux', home)) + .toBe(path.join(home, '.config', 'pnpm', 'auth.ini')) + }) +}) + +describe('resolveRegistry()', () => { it('defaults to the public registry', () => { - expect(resolveRegistryUrl(new Map(), 'some-package')).toBe(DEFAULT_REGISTRY_URL) + expect(resolveRegistry(new Map(), 'some-package').url).toBe(DEFAULT_REGISTRY_URL) }) it('uses the registry entry and appends a trailing slash', () => { const config = parseNpmrc('registry=https://nexus.local/repository/npm') - expect(resolveRegistryUrl(config, 'some-package')).toBe('https://nexus.local/repository/npm/') + expect(resolveRegistry(config, 'some-package').url).toBe('https://nexus.local/repository/npm/') }) it('prefers a scoped registry for scoped packages', () => { @@ -130,19 +342,87 @@ describe('resolveRegistryUrl()', () => { 'registry=https://nexus.local/repository/npm/', '@acme:registry=https://nexus.local/repository/npm-private/', ].join('\n')) - expect(resolveRegistryUrl(config, '@acme/private-utils')).toBe('https://nexus.local/repository/npm-private/') - expect(resolveRegistryUrl(config, 'some-package')).toBe('https://nexus.local/repository/npm/') + expect(resolveRegistry(config, '@acme/private-utils').url).toBe('https://nexus.local/repository/npm-private/') + expect(resolveRegistry(config, 'some-package').url).toBe('https://nexus.local/repository/npm/') }) it('expands ${VAR} references from the environment', () => { const config = parseNpmrc('registry=${MY_REGISTRY}') - expect(resolveRegistryUrl(config, 'some-package', { MY_REGISTRY: 'https://example.com' })) + expect(resolveRegistry(config, 'some-package', { MY_REGISTRY: 'https://example.com' }).url) .toBe('https://example.com/') }) it('throws a clear error for unset ${VAR} references', () => { const config = parseNpmrc('registry=${MY_UNSET_REGISTRY}') - expect(() => resolveRegistryUrl(config, 'some-package', {})).toThrow(NpmrcEnvVarError) + expect(() => resolveRegistry(config, 'some-package', {})).toThrow(NpmrcEnvVarError) + }) + + it('does not fall back to the public registry when the configured one is blank', () => { + // A blank registry is a broken setting, not an absent one. Silently + // using the public registry would send private package names to it; + // the unusable URL and the key that produced it let the caller report + // which entry to fix. `${VAR}` set to the empty string is how a missing + // CI secret usually arrives. + for (const config of [ + parseNpmrc('registry='), + parseNpmrc('registry=${EMPTY_REGISTRY}'), + ]) { + // Reported as unusable rather than handed back as a URL nothing can + // fetch: the caller cannot compose onto it by accident. + expect(resolveRegistry(config, '@acme/private-utils', { EMPTY_REGISTRY: '' })) + .toEqual({ usable: false, key: 'registry' }) + } + }) + + it('does not fall back to the public registry when a scoped registry is blank', () => { + const config = parseNpmrc('@acme:registry=') + expect(resolveRegistry(config, '@acme/private-utils', {})) + .toEqual({ usable: false, key: '@acme:registry' }) + }) + + it('falls back to the global registry when the scoped one is blank', () => { + // npm and pnpm both read a blank `@scope:registry` as unset. The + // fallback here is the user's own private registry, so refusing it + // would fail a configuration both package managers install from. + const config = parseNpmrc([ + 'registry=https://nexus.local/repository/npm/', + '@acme:registry=${EMPTY_REGISTRY}', + ].join('\n')) + expect(resolveRegistry(config, '@acme/private-utils', { EMPTY_REGISTRY: '' }).url) + .toBe('https://nexus.local/repository/npm/') + }) + + it('keeps the blank scoped registry when the global one is unusable', () => { + // With nothing usable to fall back to, the blank entry is kept: the + // caller reports which key to fix instead of defaulting to the public + // registry and disclosing a private package name to it. + const config = parseNpmrc([ + 'registry=', + '@acme:registry=', + ].join('\n')) + expect(resolveRegistry(config, '@acme/private-utils', {})) + .toEqual({ usable: false, key: '@acme:registry' }) + }) + + it('keeps a blank scoped registry when the global one references an unset variable', () => { + // The global entry is no more usable than a missing one, and reporting + // it would name a key that is not the one in use. + const config = parseNpmrc([ + '@acme:registry=', + 'registry=${UNSET_REGISTRY}', + ].join('\n')) + expect(resolveRegistry(config, '@acme/private-utils', {})) + .toEqual({ usable: false, key: '@acme:registry' }) + }) + + it('ignores a blank npm_config_registry rather than failing on it', () => { + // The exception the blank-registry hard failure depends on: a pipeline + // exporting the variable from an unset secret must still reach the + // default registry, not an unusable URL that aborts every download. + // Empty environment values never reach the config, matching npm. + const registry = resolveRegistry(npmrcConfigFromEnv({ npm_config_registry: '' }), 'foo', {}) + expect(registry.url).toBe(DEFAULT_REGISTRY_URL) + expect(registry.key).toBeUndefined() }) it('ignores unset ${VAR} references in entries that are not used', () => { @@ -150,37 +430,78 @@ describe('resolveRegistryUrl()', () => { 'registry=https://nexus.local/repository/npm/', '//unrelated.example.com/:_authToken=${SOME_UNSET_TOKEN}', ].join('\n')) - expect(resolveRegistryUrl(config, 'some-package', {})).toBe('https://nexus.local/repository/npm/') - expect(resolveAuthHeader(config, 'https://nexus.local/repository/npm/foo', {})).toBeUndefined() + expect(resolveRegistry(config, 'some-package', {}).url).toBe('https://nexus.local/repository/npm/') + expect(resolveAuthHeader(config, 'https://nexus.local/repository/npm/foo', 'foo', {})).toBeUndefined() }) }) describe('resolveAuthHeader()', () => { it('matches an _authToken by nerf dart', () => { const config = parseNpmrc('//nexus.local/repository/npm-private/:_authToken=secret') - const header = resolveAuthHeader( + const auth = resolveAuthHeader( config, 'https://nexus.local/repository/npm-private/@acme/foo/-/foo-1.0.0.tgz', + '@acme/foo', {}, ) - expect(header).toBe('Bearer secret') + expect(auth?.header).toBe('Bearer secret') + // The matched key is reported so a rejected credential can be traced + // back to the file that supplied it. + expect(auth?.keys).toEqual(['//nexus.local/repository/npm-private/:_authToken']) + }) + + it('reports both halves of a username/_password pair', () => { + const config = parseNpmrc([ + '//nexus.local/:username=user', + `//nexus.local/:_password=${Buffer.from('pass').toString('base64')}`, + ].join('\n')) + // Precedence is per key, so the two halves can come from different + // files; naming only the username would point at the half that is not + // secret and cannot expire. + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})?.keys) + .toEqual(['//nexus.local/:username', '//nexus.local/:_password']) }) it('walks the URL path upward to find host-level credentials', () => { const config = parseNpmrc('//nexus.local/:_authToken=host-secret') - const header = resolveAuthHeader(config, 'https://nexus.local/repository/npm/foo/-/foo-1.0.0.tgz', {}) - expect(header).toBe('Bearer host-secret') + const auth = resolveAuthHeader(config, 'https://nexus.local/repository/npm/foo/-/foo-1.0.0.tgz', 'foo', {}) + expect(auth?.header).toBe('Bearer host-secret') }) it('includes the port in the nerf dart', () => { const config = parseNpmrc('//nexus.local:8443/:_authToken=port-secret') - expect(resolveAuthHeader(config, 'https://nexus.local:8443/foo/-/foo-1.0.0.tgz', {})).toBe('Bearer port-secret') - expect(resolveAuthHeader(config, 'https://nexus.local/foo/-/foo-1.0.0.tgz', {})).toBeUndefined() + expect(resolveAuthHeader(config, 'https://nexus.local:8443/foo/-/foo-1.0.0.tgz', 'foo', {})?.header) + .toBe('Bearer port-secret') + expect(resolveAuthHeader(config, 'https://nexus.local/foo/-/foo-1.0.0.tgz', 'foo', {})).toBeUndefined() }) it('supports pre-encoded _auth as Basic', () => { const config = parseNpmrc('//nexus.local/:_auth=dXNlcjpwYXNz') - expect(resolveAuthHeader(config, 'https://nexus.local/foo', {})).toBe('Basic dXNlcjpwYXNz') + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})?.header).toBe('Basic dXNlcjpwYXNz') + }) + + it('prefers a username/_password pair over a legacy _auth at the same dart', () => { + // npm's getCredentialsByURI order: _authToken, then the pair, then + // _auth. A stale `_auth` left behind by an older CI image beside a pair + // written later must not win, or `checkly deploy` authenticates as + // somebody `npm install` stopped using. + const config = parseNpmrc([ + `//nexus.local/:_auth=${Buffer.from('stale:stale').toString('base64')}`, + '//nexus.local/:username=user', + `//nexus.local/:_password=${Buffer.from('works').toString('base64')}`, + ].join('\n')) + const auth = resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {}) + expect(auth?.header).toBe(`Basic ${Buffer.from('user:works').toString('base64')}`) + expect(auth?.keys).toEqual(['//nexus.local/:username', '//nexus.local/:_password']) + }) + + it('falls through to _auth when no pair can form', () => { + const config = parseNpmrc([ + '//nexus.local/:username=user', + '//nexus.local/:_auth=dXNlcjpwYXNz', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})?.header) + .toBe('Basic dXNlcjpwYXNz') }) it('supports username and base64 _password as Basic', () => { @@ -188,18 +509,353 @@ describe('resolveAuthHeader()', () => { '//nexus.local/:username=user', `//nexus.local/:_password=${Buffer.from('pass').toString('base64')}`, ].join('\n')) - expect(resolveAuthHeader(config, 'https://nexus.local/foo', {})) + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})?.header) .toBe(`Basic ${Buffer.from('user:pass').toString('base64')}`) }) it('expands ${VAR} tokens from the environment', () => { const config = parseNpmrc('//nexus.local/:_authToken=${NPM_TOKEN}') - expect(resolveAuthHeader(config, 'https://nexus.local/foo', { NPM_TOKEN: 'env-secret' })) + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', { NPM_TOKEN: 'env-secret' })?.header) .toBe('Bearer env-secret') }) it('returns undefined without matching credentials', () => { const config = parseNpmrc('//other.example.com/:_authToken=secret') - expect(resolveAuthHeader(config, 'https://nexus.local/foo', {})).toBeUndefined() + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})).toBeUndefined() + }) + + it.each([ + ['does not parse', 'nexus.local/foo'], + ['parses without a host', 'admin:s3cret@nexus.local/foo'], + ['names a scheme nothing here fetches', 'ftp://nexus.local/foo'], + ])('sends no credentials to a URL that %s', (_label, url) => { + // Callers check the URL before requesting it, so this is belt and + // braces — but the safe answer for one that could not be validated is + // to send nothing, not to throw or to hand a token to a scheme this + // CLI never fetches over. + const config = parseNpmrc('//nexus.local/:_authToken=secret') + expect(resolveAuthHeader(config, url, 'foo', {})).toBeUndefined() + }) + + it('matches a scope-qualified _authToken', () => { + // The spelling `pnpm login --scope=@acme` writes. + const config = parseNpmrc('//nexus.local/:@acme:_authToken=scoped-secret') + const auth = resolveAuthHeader(config, 'https://nexus.local/@acme/foo/-/foo-1.0.0.tgz', '@acme/foo', {}) + expect(auth?.header).toBe('Bearer scoped-secret') + expect(auth?.keys).toEqual(['//nexus.local/:@acme:_authToken']) + }) + + it('supports scope-qualified _auth and username/_password', () => { + const config = parseNpmrc([ + '//nexus.local/:@acme:_auth=dXNlcjpwYXNz', + '//other.local/:@acme:username=user', + `//other.local/:@acme:_password=${Buffer.from('pass').toString('base64')}`, + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/@acme/foo', '@acme/foo', {})?.header) + .toBe('Basic dXNlcjpwYXNz') + expect(resolveAuthHeader(config, 'https://other.local/@acme/foo', '@acme/foo', {})?.header) + .toBe(`Basic ${Buffer.from('user:pass').toString('base64')}`) + }) + + it('prefers a scope-qualified key over an unscoped one at the same depth', () => { + const config = parseNpmrc([ + '//nexus.local/:_authToken=unscoped-secret', + '//nexus.local/:@acme:_authToken=scoped-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/@acme/foo', '@acme/foo', {})?.header) + .toBe('Bearer scoped-secret') + }) + + it('exhausts the scoped walk before considering any unscoped key', () => { + // pnpm walks the whole scoped table first, so a shallow scoped key wins + // over a deeper unscoped one instead of the two interleaving by depth. + const config = parseNpmrc([ + '//nexus.local/repository/npm/:_authToken=deep-unscoped-secret', + '//nexus.local/:@acme:_authToken=shallow-scoped-secret', + ].join('\n')) + const url = 'https://nexus.local/repository/npm/@acme/foo/-/foo-1.0.0.tgz' + expect(resolveAuthHeader(config, url, '@acme/foo', {})?.header).toBe('Bearer shallow-scoped-secret') + }) + + it('falls back to an unscoped key when the scope has none', () => { + const config = parseNpmrc([ + '//nexus.local/:_authToken=unscoped-secret', + '//nexus.local/:@other:_authToken=other-scope-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/@acme/foo', '@acme/foo', {})?.header) + .toBe('Bearer unscoped-secret') + }) + + it('never sends a scoped credential for a package outside that scope', () => { + // A scoped token belongs to one organisation, so an unscoped package + // must not borrow it. + const config = parseNpmrc('//nexus.local/:@acme:_authToken=scoped-secret') + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})).toBeUndefined() + expect(resolveAuthHeader(config, 'https://nexus.local/@other/foo', '@other/foo', {})).toBeUndefined() + }) + + it('treats a name with no scope separator as unscoped', () => { + // `@acme` alone is a malformed package name, not a scope: reading it as + // one would send @acme's credential to a package that is not in it. + const config = parseNpmrc('//nexus.local/:@acme:_authToken=scoped-secret') + expect(resolveAuthHeader(config, 'https://nexus.local/@acme', '@acme', {})).toBeUndefined() + }) + + it('never pairs a scoped username with an unscoped password', () => { + // Both halves must come from the same key prefix: combining them would + // send a credential that neither entry describes. + const encoded = Buffer.from('pass').toString('base64') + const config = parseNpmrc([ + '//nexus.local/:@acme:username=scoped-user', + '//nexus.local/:username=unscoped-user', + `//nexus.local/:_password=${encoded}`, + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/@acme/foo', '@acme/foo', {})?.keys) + .toEqual(['//nexus.local/:username', '//nexus.local/:_password']) + }) + + it('reports an unset ${VAR} in a scope-qualified key instead of silently using another', () => { + // Probing a scoped key makes an unexpandable one fatal where it was + // previously never read. That is deliberate: the config asks for that + // scope's token specifically, and quietly sending a different one would + // authenticate as the wrong identity. The error names the key and the + // variable, which is what the reader has to fix. + const config = parseNpmrc([ + '//nexus.local/:@acme:_authToken=${ACME_TOKEN}', + '//nexus.local/:_authToken=unscoped-secret', + ].join('\n')) + expect(() => resolveAuthHeader(config, 'https://nexus.local/@acme/foo', '@acme/foo', {})) + .toThrow(NpmrcEnvVarError) + // A package outside the scope never reads that key, so it still resolves. + expect(resolveAuthHeader(config, 'https://nexus.local/bar', 'bar', {})?.header) + .toBe('Bearer unscoped-secret') + }) + + it('treats a blank credential as absent rather than sending it', () => { + // An entry emptied instead of deleted must not shadow a working + // credential further along the walk — least of all a scope-qualified + // one, which outranks every unscoped key at every depth. + const config = parseNpmrc([ + '//nexus.local/:@acme:_authToken=', + '//nexus.local/:_authToken=working-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/@acme/foo', '@acme/foo', {})?.header) + .toBe('Bearer working-secret') + }) + + it('treats a blank _auth as absent and falls through to a shallower dart', () => { + // The blank has to sit where `_auth` would actually be consulted, and + // the working credential out of that prefix's reach — a pair beside it + // would win on order alone and the test could not fail. + const config = parseNpmrc([ + '//nexus.local/repository/npm/:_auth=', + '//nexus.local/:_authToken=works', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/repository/npm/foo', 'foo', {})?.header) + .toBe('Bearer works') + }) + + it('treats a blank username as absent and falls through to a shallower dart', () => { + // Half a pair is no pair: the blank username must not combine with the + // password beside it, nor stop the walk before the working key above. + const config = parseNpmrc([ + '//nexus.local/repository/npm/:username=', + `//nexus.local/repository/npm/:_password=${Buffer.from('pass').toString('base64')}`, + '//nexus.local/:_authToken=host-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/repository/npm/foo', 'foo', {})?.header) + .toBe('Bearer host-secret') + }) + + it('never sends a username with a blank password', () => { + // The regression this guards is a credential on the wire, not a missing + // one: pairing the username with an empty password would send + // `Basic :` and read as a rejected login rather than a + // misconfiguration. + const config = parseNpmrc([ + '//nexus.local/:username=user', + '//nexus.local/:_password=', + '//nexus.local/:_authToken=', + '//other.local/:_authToken=elsewhere', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})).toBeUndefined() + }) + + it('lets a blank value mask the other spelling of the same key', () => { + // Reading past a blank spelling to the other one is what lets a blank + // in a higher-precedence file reach a token in a lower-precedence one, + // so the first spelling that exists settles the key. npm looks up one + // spelling and goes anonymous on a blank; so does this. + const config = parseNpmrc([ + '//nexus.local/:_authToken=', + '//nexus.local/:_authtoken=real-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})).toBeUndefined() + }) + + it('still matches a lowercase spelling when it is the only one present', () => { + // Masking is about a blank, not about the spelling: with nothing under + // the canonical name, the other spelling is still the key. + const config = parseNpmrc('//nexus.local/:_authtoken=real-secret') + const auth = resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {}) + expect(auth?.header).toBe('Bearer real-secret') + expect(auth?.keys).toEqual(['//nexus.local/:_authtoken']) + }) + + it('matches the path form of a scope-qualified key', () => { + // pnpm strips a trailing scope segment off the key and binds the + // credential to the registry above it, so the key covers packages in + // that scope wherever the registry serves them. + const config = parseNpmrc('//npm.pkg.github.com/@acme/:_authToken=path-form-secret') + const url = 'https://npm.pkg.github.com/download/@acme/foo/1.0.0/abcdef' + expect(resolveAuthHeader(config, url, '@acme/foo', {})?.header).toBe('Bearer path-form-secret') + }) + + it('does not lend a path-form key to another scope', () => { + const config = parseNpmrc('//npm.pkg.github.com/@acme/:_authToken=path-form-secret') + const url = 'https://npm.pkg.github.com/download/@other/foo/1.0.0/abcdef' + expect(resolveAuthHeader(config, url, '@other/foo', {})).toBeUndefined() + }) + + it('prefers the colon form over the path form at the same nerf dart', () => { + const config = parseNpmrc([ + '//nexus.local/@acme/:_authToken=path-form-secret', + '//nexus.local/:@acme:_authToken=colon-form-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/x/@acme/foo', '@acme/foo', {})?.header) + .toBe('Bearer colon-form-secret') + }) + + it('lets a deeper unscoped key win over the path form', () => { + // The path form is spelled exactly like a nerf dart for the path + // `/@acme/`, which is how npm, yarn and bun read it. Ranking it below + // the unscoped walk keeps a setup that authenticates today sending the + // same credential it sends today. + const config = parseNpmrc([ + '//nexus.local/@acme/:_authToken=path-form-secret', + '//nexus.local/repository/npm/:_authToken=deeper-unscoped-secret', + ].join('\n')) + const url = 'https://nexus.local/repository/npm/@acme/foo/-/foo-1.0.0.tgz' + expect(resolveAuthHeader(config, url, '@acme/foo', {})?.header).toBe('Bearer deeper-unscoped-secret') + }) + + it('skips a path-form key whose ${VAR} is unset when the URL never touches that path', () => { + // `//nexus.local/@acme/` is not a prefix of this URL, so under every + // reading but pnpm's it says nothing about this request. Failing here + // would abort a download that would otherwise have gone out. + const config = parseNpmrc('//nexus.local/@acme/:_authToken=${UNSET_TOKEN}') + const url = 'https://nexus.local/repository/npm/@acme/foo/-/foo-1.0.0.tgz' + expect(resolveAuthHeader(config, url, '@acme/foo', {})).toBeUndefined() + }) + + it('still fails on an unset ${VAR} when the scope path is part of the URL', () => { + // Here the same key IS a nerf dart of the request, which every package + // manager reads as applying to it, so the missing variable is fatal as + // it would be for any other applicable key. The tolerance above is not + // a blanket rule about the spelling. + const config = parseNpmrc([ + '//nexus.local/@acme/:_authToken=${UNSET_TOKEN}', + '//nexus.local/:_authToken=working-secret', + ].join('\n')) + const url = 'https://nexus.local/@acme/foo/-/foo-1.0.0.tgz' + expect(() => resolveAuthHeader(config, url, '@acme/foo', {})).toThrow(NpmrcEnvVarError) + }) + + it('still uses a working key when a path-form one cannot be expanded', () => { + const config = parseNpmrc([ + '//nexus.local/@acme/:_authToken=${UNSET_TOKEN}', + '//nexus.local/:_authToken=working-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/x/@acme/foo', '@acme/foo', {})?.header) + .toBe('Bearer working-secret') + }) + + it('keeps a usable credential kind beside an unexpandable one at a path-form prefix', () => { + // The tolerance is per key: one entry referencing a missing variable + // must not discard the credential configured next to it. + const config = parseNpmrc([ + '//nexus.local/@acme/:_authToken=${UNSET_TOKEN}', + '//nexus.local/@acme/:_auth=dXNlcjpwYXNz', + ].join('\n')) + const url = 'https://nexus.local/repository/npm/@acme/foo/-/foo-1.0.0.tgz' + expect(resolveAuthHeader(config, url, '@acme/foo', {})?.header).toBe('Basic dXNlcjpwYXNz') + }) + + it('skips a _password whose ${VAR} is unset when no username sits beside it', () => { + // Half a pair can never produce a credential, so expanding it would + // fail the download over a key that was never going to be used. + const config = parseNpmrc([ + '//nexus.local/:@acme:_password=${UNSET_PASSWORD}', + '//nexus.local/:_authToken=working-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/@acme/foo', '@acme/foo', {})?.header) + .toBe('Bearer working-secret') + }) + + it('skips a username whose ${VAR} is unset when no password sits beside it', () => { + // The mirror of the _password case: a leftover username from a setup + // that moved to a token must not be fatal either. + const config = parseNpmrc([ + '//nexus.local/repository/npm/:username=${UNSET_USER}', + '//nexus.local/:_authToken=working-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/repository/npm/foo', 'foo', {})?.header) + .toBe('Bearer working-secret') + }) + + it('skips an unexpandable half of a pair whose other half is blank', () => { + // A blank half is absent by the same rule an omitted one is, so the + // pair can never form and the surviving half must not be expanded. + const config = parseNpmrc([ + '//nexus.local/repository/npm/:username=', + '//nexus.local/repository/npm/:_password=${UNSET_PASSWORD}', + '//nexus.local/:_authToken=working-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/repository/npm/foo', 'foo', {})?.header) + .toBe('Bearer working-secret') + }) + + it('treats a ${VAR} that expands to the empty string as absent', () => { + // How a missing CI secret usually arrives: the variable exists, its + // value does not. + const config = parseNpmrc([ + '//nexus.local/repository/npm/:_authToken=${CI_TOKEN}', + '//nexus.local/:_authToken=working-secret', + ].join('\n')) + expect(resolveAuthHeader(config, 'https://nexus.local/repository/npm/foo', 'foo', { CI_TOKEN: '' })?.header) + .toBe('Bearer working-secret') + }) + + it('skips an unexpandable half whose partner expands to blank', () => { + // The blank arrives through a variable rather than a literal, which is + // the same thing once expanded — so the pair cannot form and the unset + // variable in the other half must not abort the download. + const config = parseNpmrc([ + '//nexus.local/repository/npm/:username=${CI_USER}', + '//nexus.local/repository/npm/:_password=${CI_PASSWORD}', + '//nexus.local/:_authToken=working-secret', + ].join('\n')) + const url = 'https://nexus.local/repository/npm/foo/-/foo-1.0.0.tgz' + expect(resolveAuthHeader(config, url, 'foo', { CI_USER: '' })?.header).toBe('Bearer working-secret') + }) + + it('reports the missing variable when the pair was real', () => { + // Both halves are usable references, so the user meant a pair and one + // variable is genuinely missing — worth naming rather than skipping. + const config = parseNpmrc([ + '//nexus.local/:username=${CI_USER}', + '//nexus.local/:_password=${CI_PASSWORD}', + ].join('\n')) + expect(() => resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', { CI_USER: 'user' })) + .toThrow(NpmrcEnvVarError) + }) + + it('still reports an unset ${VAR} in a _password that completes a pair', () => { + const config = parseNpmrc([ + '//nexus.local/:username=user', + '//nexus.local/:_password=${UNSET_PASSWORD}', + ].join('\n')) + expect(() => resolveAuthHeader(config, 'https://nexus.local/foo', 'foo', {})) + .toThrow(NpmrcEnvVarError) }) }) diff --git a/packages/cli/src/services/embedded-packages/__tests__/url.spec.ts b/packages/cli/src/services/embedded-packages/__tests__/url.spec.ts new file mode 100644 index 00000000..64f60c09 --- /dev/null +++ b/packages/cli/src/services/embedded-packages/__tests__/url.spec.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest' + +import { parseComposableUrl, parseFetchableUrl } from '../url.js' + +describe('parseFetchableUrl()', () => { + it.each([ + ['a plain https URL', 'https://nexus.local/repository/npm/'], + ['a port', 'https://nexus.local:8443/npm/'], + ['http', 'http://127.0.0.1:4873/'], + // Fetchable, and redaction drops these before anything is echoed. + ['userinfo', 'https://user:tok@nexus.local/npm/'], + ['a query, which a request may legitimately carry', 'https://cdn.example.com/f.tgz?sig=abc'], + ])('accepts %s', (_label, url) => { + expect(parseFetchableUrl(url)?.host).not.toBe(undefined) + }) + + it.each([ + ['a bare host', 'nexus.local/repository/npm/'], + ['a scheme with nothing after it', 'https://'], + // Parses as the opaque scheme `admin:` with no host, leaving the + // credential in what looks like a path. + ['userinfo with no scheme', 'admin:s3cret@nexus.local/npm/'], + ['a scheme nothing here fetches', 'ftp://nexus.local/npm/'], + ['a scheme that never has a host', 'file:///srv/npm-mirror/'], + ['an empty string', ''], + ])('rejects %s', (_label, url) => { + expect(parseFetchableUrl(url)).toBeUndefined() + }) +}) + +describe('parseComposableUrl()', () => { + it('accepts a URL a path can be appended to', () => { + expect(parseComposableUrl('https://nexus.local/repository/npm/')?.host).toBe('nexus.local') + }) + + it.each([ + // Each of these absorbs whatever is appended, so the composed path + // would never reach the server. + ['a query', 'https://nexus.local/npm/?token=abc'], + ['a bare query delimiter', 'https://nexus.local/npm/?'], + ['a fragment', 'https://nexus.local/npm/#tok'], + ['a bare fragment delimiter', 'https://nexus.local/npm/#'], + ])('rejects %s', (_label, url) => { + expect(parseComposableUrl(url)).toBeUndefined() + }) + + it('rejects everything an unfetchable URL is rejected for', () => { + expect(parseComposableUrl('ftp://nexus.local/npm/')).toBeUndefined() + expect(parseComposableUrl('nexus.local/npm/')).toBeUndefined() + }) +}) diff --git a/packages/cli/src/services/embedded-packages/diagnostics.ts b/packages/cli/src/services/embedded-packages/diagnostics.ts new file mode 100644 index 00000000..f43f41bb --- /dev/null +++ b/packages/cli/src/services/embedded-packages/diagnostics.ts @@ -0,0 +1,283 @@ +import { LoadedNpmrcConfig, NPM_CONFIG_ENV_PREFIX } from './npmrc.js' +import { FETCHABLE_URL_REQUIREMENT, parseFetchableUrl } from './url.js' + +/** + * Joins up to 8 items, appending `N more` for the rest — the + * uniform truncation for user-facing lists of packages, versions and + * reasons. + */ +export function capList (items: string[], separator: string, overflow: string): string { + const shown = items.slice(0, 8).join(separator) + return items.length > 8 ? `${shown}${overflow}${items.length - 8} more` : shown +} + +/** Quotes and joins a list of names for a message. */ +export function quotedList (names: string[]): string { + return capList(names.map(name => `'${name}'`), ', ', ' and ') +} + +/** Stand-in for a URL that cannot be shown without risking a credential. */ +export const UNPRINTABLE_URL = '(withheld: unparseable and may contain credentials)' + +/** + * Rebuilds a URL from the parts that cannot carry a credential, so it can + * safely appear in error messages and logs. + * + * Only scheme and host survive. Userinfo, path, query and fragment are + * dropped by construction rather than stripped: a registry URL may embed a + * token in its userinfo, a pre-signed CDN URL puts its signature in the + * query, and some registries take a token as a PATH segment + * (`https://host//npm/`). The path is the component least worth + * keeping anyway — every message that shows a URL already names the + * package and version separately, which is what the path encodes. + * + * A string that does not parse, parses without a host, or names a scheme + * nothing here fetches over yields the placeholder instead — deliberately + * the same rule as what may be requested, so a URL is echoable exactly when + * it was fetchable. Earlier revisions tried to redact such strings with + * a regex and leaked a credential four times over as many review rounds — + * through the first `@` only, through an empty userinfo, through a + * host-less parse, and through an authority that did not start at offset + * zero. Nothing short of a parser can tell userinfo from a path in a + * malformed string, so this follows the precedent already set for proxy + * URLs in `rest/errors.ts` and declines to echo it at all. Callers name the + * configuration key and file that produced the value instead, which is + * where the reader would go to look anyway. + */ +export function redactUrl (url: string): string { + // `origin` is deliberately not used to rebuild it: that is the string + // "null" for non-special schemes. + const parsed = parseFetchableUrl(url) + return parsed === undefined ? UNPRINTABLE_URL : `${parsed.protocol}//${parsed.host}` +} + +/** + * Where the credentials sent with a request came from: a nerf-darted + * config entry, or userinfo embedded in the URL itself. + * + * The `url` case carries a whole `UrlOrigin` rather than mirroring its + * members, because the two must agree on every variant and a hand-kept + * copy did not: a variant added to `UrlOrigin` alone once reported + * registry-issued credentials as coming from a config line that contained + * none. Reusing the type makes the compiler enforce what review had to. + */ +export type SentCredentials = + | { from: 'config', keys: string[] } + | { from: 'url', origin: UrlOrigin } + +/** + * Where a tarball URL came from: the lockfile recorded it verbatim, the + * registry returned it in this package's metadata, or the CLI built it from + * a registry. `registryKey` is absent when nothing configured one and the + * public npm registry was assumed. + * + * Read both to attribute credentials the URL itself carries and to say + * which setting produced a URL nothing can be fetched from — and the two + * answers differ, so the variants are not interchangeable: a metadata URL + * is the registry's to fix, not the project's. + */ +export type UrlOrigin = + | RecordedUrlOrigin + | { registryKey?: string } + +/** + * Names the registry a URL came from, by the key that configured it or as + * the assumed default when nothing did. + */ +function describeRegistrySource (registryKey: string | undefined, npmrc: LoadedNpmrcConfig): string { + return registryKey !== undefined + ? `the registry configured by ${describeConfigKeys([registryKey], npmrc)}` + : 'the default registry' +} + +/** + * A URL this CLI did not compose: something else handed it over already + * formed, so it can be unusable however it likes. + * + * A URL the CLI builds itself cannot be. The registry it is built from is + * checked first, and appending a path to a URL that parses with a host + * leaves both intact — so those origins never reach a message about an + * unusable URL, and are not in this type. + */ +export type RecordedUrlOrigin = + | { lockfile: string } + | { metadata: { registryKey?: string } } + +/** + * Says where an unusable URL came from, and therefore whose it is to fix. + * The advice has to match the branch: telling someone to correct their + * registry setting when the bad value came out of the lockfile sends them + * to a setting that is already correct. + */ +export function describeUnusableUrlOrigin (origin: RecordedUrlOrigin, npmrc: LoadedNpmrcConfig): string { + if ('lockfile' in origin) { + return `It was recorded in '${origin.lockfile}', whose tarball URL for this package` + + ` is not ${FETCHABLE_URL_REQUIREMENT}.` + } + const registry = describeRegistrySource(origin.metadata.registryKey, npmrc) + const whose = origin.metadata.registryKey !== undefined + ? `so check that the setting points at the registry you meant before taking it up with whoever runs it.` + : `and nothing here configures a registry, so this came from the public one.` + return `It came from the package metadata served by ${registry}, which returned a tarball URL` + + ` that is not ${FETCHABLE_URL_REQUIREMENT}. The URL is the registry's to correct, ${whose}` +} + +/** What a redirect did to the credentials on a request. */ +export interface RedirectOutcome { + /** The host the request was last redirected to. */ + host?: string + /** True when credentials were sent but did not survive the hop. */ + credentialsDropped?: boolean +} + +/** + * Names config keys and where they came from, for a hint sentence. Keys are + * listed individually because precedence is per key: the halves of a + * `username`/`_password` pair can come from different sources. + */ +export function describeConfigKeys (keys: string[], npmrc: LoadedNpmrcConfig): string { + const described = keys.map(key => { + const origin = npmrc.origins.get(key) + switch (origin?.kind) { + // Name the variable verbatim: the stored key has the `npm_config_` + // prefix stripped and may be case-folded, so echoing it would name + // nothing the user can search their environment for. + case 'env': + return `the '${origin.variable}' environment variable` + case 'file': + return `'${key}' in '${origin.path}'` + default: + return `'${key}'` + } + }) + return capList(described, ', ', ' and ') +} + +/** + * The places a credential could have been configured, highest precedence + * first. The environment channel is always consulted and outranks every + * file, so a list that omits it sends people to edit files that cannot win. + */ +function describeConfigSources (npmrc: LoadedNpmrcConfig): string { + return quotedList([ + `${NPM_CONFIG_ENV_PREFIX}* environment variables`, + ...npmrc.files, + ]) +} + +/** + * Names where the credentials on a request came from, as a sentence tail + * ending in a full stop. + */ +function describeSentCredentials (sent: SentCredentials, npmrc: LoadedNpmrcConfig): string { + if (sent.from === 'config') { + return `${describeConfigKeys(sent.keys, npmrc)}.` + } + const { origin } = sent + if ('lockfile' in origin) { + return `the tarball URL recorded in '${origin.lockfile}'.` + } + if ('metadata' in origin) { + return `the tarball URL that ${describeRegistrySource(origin.metadata.registryKey, npmrc)} returned` + + ` in this package's metadata, so they were issued by that registry rather than configured here.` + } + return `the URL of ${describeRegistrySource(origin.registryKey, npmrc)}.` +} + +/** + * Explains an HTTP failure that authentication could account for. + * + * 404 gets the same treatment as 401/403 because registries routinely hide + * packages the caller is not authorized to see behind a 404 — npmjs does — + * which otherwise reads as "this package does not exist" and sends people + * looking in entirely the wrong place. The wording stays hedged for 404, + * where a genuinely missing package is equally likely. + * + * Whenever credentials were sent, the message names where they came from. + * They can come from any of several files or an environment variable, so + * "your credentials were rejected" without saying which source supplied + * them leaves the reader exactly as stuck as a bare 404. + */ +export function downloadFailureHint ( + status: number | undefined, + sent: SentCredentials | undefined, + npmrc: LoadedNpmrcConfig, + redirect: RedirectOutcome = {}, +): string { + if (status !== 401 && status !== 403 && status !== 404) { + // The URL in the message is the one that was requested; whatever the + // status, say so when something else handled it. Without a status + // nothing answered at all, so the hop is reported without claiming it + // produced the failure. + if (redirect.host === undefined) { + return '' + } + return status === undefined + ? ` The request was redirected to '${redirect.host}' before it failed.` + : ` The request was redirected to '${redirect.host}', which is what answered.` + } + + const sentences: string[] = [] + + if (redirect.credentialsDropped === true) { + // Never say the credentials were rejected: the host that answered never + // saw them, and blaming them sends the reader to rotate a working + // token. A redirect to an unauthenticated CDN is normal for GitHub + // Packages and for Artifactory or Nexus fronted by object storage, so + // this is often not the fault at all — hence the pointer to the + // redirect target rather than a verdict about it. + sentences.push( + `The request was redirected to '${redirect.host}', and the credentials were dropped rather` + + ` than forwarded there, so that host answered without them. This is normal when a registry` + + ` redirects to a pre-signed URL; look at what that host returned.`, + ) + if (sent !== undefined) { + sentences.push(`The credentials the original host received came from ${describeSentCredentials(sent, npmrc)}`) + } + } else if (sent === undefined) { + sentences.push(`No credentials for this registry were found in ${describeConfigSources(npmrc)}.`) + if (status === 404) { + sentences.push( + `A registry may answer 404 for a package you are not authorized to see, so the package` + + ` may exist but be invisible without credentials.`, + ) + } + if (redirect.host !== undefined) { + // Without this the reader is told to configure credentials for a host + // that never asked for any, when the status came from elsewhere. + sentences.push(`Note that the request was redirected to '${redirect.host}', which is what answered.`) + } + } else { + if (status === 404) { + sentences.push( + `Credentials were sent but did not grant access, so either the package does not exist` + + ` or the credentials do not cover it.`, + ) + } else { + sentences.push(`The credentials sent for this registry were rejected — an expired token fails this way.`) + } + sentences.push(`They came from ${describeSentCredentials(sent, npmrc)}`) + if (redirect.host !== undefined) { + sentences.push(`They were carried through a redirect to '${redirect.host}', which is what answered.`) + } + } + + return ` ${sentences.join(' ')}${describeUnreadableConfig(npmrc)}` +} + +/** + * Names any config file that existed but could not be read, as a sentence + * with a leading space or the empty string. + * + * Worth saying on any failure a credential could explain, not only the ones + * that got as far as a status code: a skipped `auth.ini` is invisible + * otherwise, and it is the likeliest thing to be missing when a private + * package cannot be resolved at all. + */ +export function describeUnreadableConfig (npmrc: LoadedNpmrcConfig): string { + if (npmrc.unreadable.length === 0) { + return '' + } + return ` Note that ${quotedList(npmrc.unreadable)} could not be read, so any credentials it holds` + + ` were not used.` +} diff --git a/packages/cli/src/services/embedded-packages/lockfile-packages.ts b/packages/cli/src/services/embedded-packages/lockfile-packages.ts index 2a0475d1..2a63e37e 100644 --- a/packages/cli/src/services/embedded-packages/lockfile-packages.ts +++ b/packages/cli/src/services/embedded-packages/lockfile-packages.ts @@ -67,6 +67,16 @@ export class UnsupportedLockfileError extends Error { } } +const PNPM_LOCKFILE = 'pnpm-lock.yaml' + +/** + * Whether a lockfile is pnpm's. Callers use this to decide which package + * manager's credential conventions apply to the project. + */ +export function isPnpmLockfile (lockfilePath: string): boolean { + return path.basename(lockfilePath) === PNPM_LOCKFILE +} + /** * Enumerates every package entry in a lockfile, classified into embeddable * registry packages and excluded (git/file/link/integrity-less) entries. @@ -86,7 +96,7 @@ export async function loadLockfilePackages (lockfilePath: string): PromiseN more` for the rest — the - * uniform truncation for user-facing lists of packages, versions and - * reasons. - */ -function capList (items: string[], separator: string, overflow: string): string { - const shown = items.slice(0, 8).join(separator) - return items.length > 8 ? `${shown}${overflow}${items.length - 8} more` : shown -} - -/** - * Removes userinfo credentials from a URL so it can be safely included in - * error messages and logs (a registry URL may embed a token). + * Wraps an axios error from a registry request in an EmbeddedPackageError, + * appending the HTTP status and whatever the caller's hint makes of it. + * `message` is the action-specific prefix (e.g. "Failed to download …"). */ -function redactUrl (url: string): string { - try { - const parsed = new URL(url) - parsed.username = '' - parsed.password = '' - return parsed.toString() - } catch { - // Not parseable as a URL (e.g. a scheme-less registry entry) — strip - // anything that looks like a userinfo segment before displaying it. - return url.replace(/(^|\/\/)[^/@\s]+@/, '$1') - } +function registryHttpError ( + err: any, + message: string, + hint: (status: number | undefined) => string = () => '', +): EmbeddedPackageError { + const status = err?.response?.status + const statusHint = status !== undefined ? ` (HTTP ${status})` : '' + return new EmbeddedPackageError(`${message}${statusHint}.${hint(status)}`, { cause: err }) } /** - * Wraps an axios error from a registry request in an EmbeddedPackageError, - * appending the HTTP status and, for 401/403, a credentials hint. `message` - * is the action-specific prefix (e.g. "Failed to download …"). + * Whether a URL carries credentials in its userinfo component. axios sends + * those itself — and drops any `Authorization` header when it does — so a + * failure hint that only consulted the npm config would contradict what was + * actually on the wire. + * + * Only called with a URL the caller has already parsed successfully. */ -function registryHttpError (err: any, message: string): EmbeddedPackageError { - const status = err?.response?.status - const statusHint = status !== undefined ? ` (HTTP ${status})` : '' - const authHint = status === 401 || status === 403 - ? ` Check that your .npmrc contains valid credentials for this registry.` - : '' - return new EmbeddedPackageError(`${message}${statusHint}.${authHint}`, { cause: err }) +function hasUrlCredentials (url: string): boolean { + const parsed = new URL(url) + return parsed.username !== '' || parsed.password !== '' } /** @@ -221,17 +231,24 @@ export class EmbeddedPackagesMaterializer { return [] } - // Safe to assert: a missing lockfile is a plan issue, and issues abort - // above. - const npmrcConfig = await loadNpmrcConfig(defaultNpmrcPaths( - this.#projectRoot!, - this.#homedir, - this.#options.contextDir, - ), this.#env) + // Safe to assert both: a missing lockfile is a plan issue, and issues + // abort above. + const lockfilePath = this.#options.lockfilePath! + const pnpmAuthFile = pnpmAuthIniPath(this.#env, process.platform, this.#homedir) + const pnpmAuthFilePreferred = isPnpmLockfile(lockfilePath) + debug('pnpm auth file %s (preferred: %s)', pnpmAuthFile, pnpmAuthFilePreferred) + + const npmrc = await loadNpmrcConfig(defaultNpmrcPaths({ + workspaceRoot: this.#projectRoot!, + homedir: this.#homedir, + contextDir: this.#options.contextDir, + pnpmAuthFile, + pnpmAuthFilePreferred, + }), this.#env) const queue = new PQueue({ concurrency: DOWNLOAD_CONCURRENCY }) return await queue.addAll(tarballs.map(tarball => async (): Promise => { - const { filePath, integrity } = await this.#obtainTarball(tarball, npmrcConfig) + const { filePath, integrity } = await this.#obtainTarball(tarball, npmrc) return { ...tarball, integrity, @@ -476,18 +493,86 @@ export class EmbeddedPackagesMaterializer { } } + /** + * Resolves the registry a package comes from, refusing one nothing can be + * fetched from. + * + * The registry URL is checked before anything is composed onto it: + * `registry=https://` composes into `https:///...`, which + * parses cleanly with the package name as its HOST, so the request would + * go to whatever host bears that name. A query or fragment is refused for + * the mirror-image reason — it absorbs the path instead of the host. + */ + #resolveFetchableRegistry ( + tarball: PlannedTarball, + npmrc: LoadedNpmrcConfig, + ): UsableRegistry { + const registry = resolveRegistry(npmrc.config, tarball.name, this.#env) + if (registry.usable) { + return registry + } + + // One sentence covering every way it can fail — it parses or it does + // not, it has a host or it does not, its scheme is fetchable or it is + // not, it carries a query or it does not — because splitting them + // produced advice that was wrong for the case it did not cover: + // `file:///srv/mirror/` is absolute and has a protocol, and being told + // to add one sends the reader nowhere. + // + // The value is not echoed, for the same reason a composed URL is not: + // one this malformed could carry a credential anywhere in it. + throw new EmbeddedPackageError( + `The registry URL for embedded package '${tarball.name}@${tarball.version}' is not usable:` + + ` it must be ${COMPOSABLE_URL_REQUIREMENT}.` + + ` It is configured by ${describeConfigKeys([registry.key], npmrc)}.`, + ) + } + + /** + * Rejects a tarball URL nothing can be fetched from, naming whoever + * handed it over. Only a URL this CLI did not compose can get here — see + * `RecordedUrlOrigin`. + * + * The offending value is deliberately not echoed: it is unusable by + * definition here, so nothing can reliably tell a credential in it from a + * path. Naming the source is both safe and more useful — that is where + * the reader goes to fix it. + */ + #assertFetchableTarballUrl ( + url: string, + tarball: PlannedTarball, + npmrc: LoadedNpmrcConfig, + origin: RecordedUrlOrigin, + ): void { + if (parseFetchableUrl(url) !== undefined) { + return + } + + throw new EmbeddedPackageError( + `The tarball URL for embedded package '${tarball.name}@${tarball.version}'` + + ` is not a valid URL. ${describeUnusableUrlOrigin(origin, npmrc)}`, + ) + } + async #obtainTarball ( tarball: PlannedTarball, - npmrcConfig: NpmrcConfig, + npmrc: LoadedNpmrcConfig, ): Promise<{ filePath: string, integrity: string }> { let { integrity, tarballUrl } = tarball + // Set when the URL below came from package metadata rather than the + // lockfile, so a failure blames the registry that served it instead of + // a lockfile that never mentioned it. + let metadataOrigin: RecordedUrlOrigin | undefined if (integrity === undefined) { // yarn.lock plans carry no SRI tarball integrity (Berry checksums // hash yarn's own cache archive); resolve it from the registry's // per-version metadata before the caches can be consulted. - const dist = await this.#resolveDistFromRegistry(tarball, npmrcConfig) + const dist = await this.#resolveDistFromRegistry(tarball, npmrc) integrity = dist.integrity - tarballUrl ??= dist.tarballUrl + if (tarballUrl === undefined && dist.tarballUrl !== undefined) { + tarballUrl = dist.tarballUrl + metadataOrigin = { metadata: { registryKey: dist.registryKey } } + } } const cached = await this.#cache.get(integrity) @@ -502,16 +587,29 @@ export class EmbeddedPackagesMaterializer { return { filePath: await this.#cache.put(integrity, fromNpmCacache), integrity } } - const url = tarballUrl ?? this.#deriveTarballUrl(tarball, npmrcConfig) - if (!URL.canParse(url)) { - throw new EmbeddedPackageError( - `The tarball URL for embedded package '${tarball.name}@${tarball.version}'` - + ` is not a valid URL: '${redactUrl(url)}'. Check the 'registry' configuration` - + ` in your .npmrc (it must be an absolute URL including the protocol).`, - ) + // Where the URL came from decides who to blame for credentials embedded + // in it: a lockfile-recorded URL is the lockfile's, a derived one + // belongs to whichever config key configured the registry. + let url: string + let urlOrigin: UrlOrigin + if (tarballUrl !== undefined) { + url = tarballUrl + // Safe to assert: a missing lockfile is a plan issue, and materialize + // aborts on issues before any tarball is obtained. + const recorded = metadataOrigin ?? { lockfile: this.#options.lockfilePath! } + // Only a URL handed over already formed can be unusable: the one the + // else-branch composes is built on a registry checked beforehand. + this.#assertFetchableTarballUrl(url, tarball, npmrc, recorded) + urlOrigin = recorded + } else { + const registry = this.#resolveFetchableRegistry(tarball, npmrc) + const basename = tarball.name.split('/').pop() + url = `${registry.url}${tarball.name}/-/${basename}-${tarball.version}.tgz` + urlOrigin = { registryKey: registry.key } } + debug('%s@%s: downloading from %s', tarball.name, tarball.version, redactUrl(url)) - const content = await this.#download(tarball, url, npmrcConfig) + const content = await this.#download(tarball, url, npmrc, urlOrigin) if (!verifyIntegrity(content, integrity)) { // For yarn.lock plans the integrity came from the registry's own @@ -545,25 +643,58 @@ export class EmbeddedPackagesMaterializer { */ async #resolveDistFromRegistry ( tarball: PlannedTarball, - npmrcConfig: NpmrcConfig, - ): Promise<{ integrity: string, tarballUrl?: string }> { - const registryUrl = resolveRegistryUrl(npmrcConfig, tarball.name, this.#env) - const versionUrl = `${registryUrl}${tarball.name}/${tarball.version}` - const packumentUrl = `${registryUrl}${tarball.name}` + npmrc: LoadedNpmrcConfig, + ): Promise<{ integrity: string, tarballUrl?: string, registryKey?: string }> { + // Both routes are this URL plus the package name, so checking it covers + // them and the composed forms need no guard of their own. + const registry = this.#resolveFetchableRegistry(tarball, npmrc) + const versionUrl = `${registry.url}${tarball.name}/${tarball.version}` + const packumentUrl = `${registry.url}${tarball.name}` // Per-version route: dist is at the document root. - const perVersion = await this.#fetchMetadataDist(tarball, npmrcConfig, versionUrl, data => data?.dist) - // Packument fallback (only when the per-version route was absent, not - // when it answered with unusable data): dist is nested per version. - const dist = perVersion ?? await this.#fetchMetadataDist( - tarball, npmrcConfig, packumentUrl, data => data?.versions?.[tarball.version]?.dist, + const perVersion = await this.#fetchMetadataDist( + tarball, npmrc, versionUrl, registry.key, data => data?.dist, ) + // Packument fallback (only when the per-version route yielded no dist, + // whether it 404'd or answered without one): dist is nested per version. + const packument = perVersion?.dist !== undefined + ? undefined + : await this.#fetchMetadataDist( + tarball, npmrc, packumentUrl, registry.key, data => data?.versions?.[tarball.version]?.dist, + ) + const dist = perVersion?.dist ?? packument?.dist + + if (dist === undefined) { + // Distinguish "the registry has nothing for us" from "it answered but + // this version is not in it": only the first can be an authorization + // failure, since a private registry hides packages the caller may not + // see behind a 404, and claiming so for the second sends the reader to + // rotate a token the registry just accepted. + const answered = perVersion !== undefined || packument !== undefined + if (answered) { + throw new EmbeddedPackageError( + `The registry metadata at '${redactUrl(versionUrl)}' does not describe embedded package` + + ` '${tarball.name}@${tarball.version}', so its integrity could not be resolved.` + + ` The version may have been unpublished, the registry may serve only some versions, or` + + ` something in front of it — a proxy or an SSO gateway — may have answered instead of` + + ` the registry.${describeUnreadableConfig(npmrc)}`, + ) + } + + const auth = resolveAuthHeader(npmrc.config, versionUrl, tarball.name, this.#env) + const sent = this.#sentCredentials(versionUrl, { registryKey: registry.key }, auth) + throw new EmbeddedPackageError( + `The registry at '${redactUrl(versionUrl)}' has no metadata for embedded package` + + ` '${tarball.name}@${tarball.version}', so its integrity could not be resolved.` + + downloadFailureHint(404, sent, npmrc), + ) + } // Modern publishes carry an SRI `integrity`; very old ones only a hex // sha1 `shasum`, which converts to a (weaker but supported) SRI hash. - const integrity = typeof dist?.integrity === 'string' && dist.integrity !== '' + const integrity = typeof dist.integrity === 'string' && dist.integrity !== '' ? dist.integrity as string - : typeof dist?.shasum === 'string' && /^[0-9a-f]{40}$/.test(dist.shasum) + : typeof dist.shasum === 'string' && /^[0-9a-f]{40}$/.test(dist.shasum) ? `sha1-${Buffer.from(dist.shasum, 'hex').toString('base64')}` : undefined if (integrity === undefined) { @@ -576,43 +707,65 @@ export class EmbeddedPackagesMaterializer { return { integrity, - // Same guard as the lockfile-recorded URLs: only absolute http(s) - // URLs are usable for downloading. - tarballUrl: typeof dist?.tarball === 'string' && /^https?:/.test(dist.tarball) + registryKey: registry.key, + // The same cheap prefilter the lockfile readers apply, and no more: + // anything that survives it is checked properly by + // `#assertFetchableTarballUrl`, which reports a URL the registry + // returned rather than silently composing a different one. + tarballUrl: typeof dist.tarball === 'string' && /^https?:/.test(dist.tarball) ? dist.tarball as string : undefined, } } /** - * Fetches one metadata URL and extracts its `dist` via `select`. Returns - * undefined on a 404 (so the caller can try another route); any other - * failure — auth, network, malformed response — throws, because retrying - * a different route would only mask it. + * Who supplied the credentials on a request, for the failure message. + * + * Userinfo embedded in the URL wins: axios sends that itself and drops + * the Authorization header when it does, so naming the config entry + * would name credentials that never reached the wire. A URL with no + * userinfo falls back to the config keys — including one derived from the + * default registry, whose URL carries none by construction. + */ + #sentCredentials ( + url: string, + origin: UrlOrigin, + auth: ResolvedAuth | undefined, + ): SentCredentials | undefined { + if (hasUrlCredentials(url)) { + return { from: 'url', origin } + } + return auth !== undefined ? { from: 'config', keys: auth.keys } : undefined + } + + /** + * Fetches one metadata URL and extracts its `dist` via `select`. + * + * Returns undefined on a 404 — distinct from `{ dist: undefined }`, which + * means the route answered but carried nothing usable. The caller needs + * both apart: only a route that never answered can be an authorization + * failure. Any other failure — auth, network, malformed response — throws, + * because retrying a different route would only mask it. */ async #fetchMetadataDist ( tarball: PlannedTarball, - npmrcConfig: NpmrcConfig, + npmrc: LoadedNpmrcConfig, url: string, + registryKey: string | undefined, select: (data: any) => any, - ): Promise { - if (!URL.canParse(url)) { - throw new EmbeddedPackageError( - `The registry metadata URL for embedded package '${tarball.name}@${tarball.version}'` - + ` is not a valid URL: '${redactUrl(url)}'. Check the 'registry' configuration` - + ` in your .npmrc (it must be an absolute URL including the protocol).`, - ) - } - const authHeader = resolveAuthHeader(npmrcConfig, url, this.#env) + ): Promise<{ dist: any } | undefined> { + const auth = resolveAuthHeader(npmrc.config, url, tarball.name, this.#env) + // This URL is always one the CLI built from the registry. + const sent = this.#sentCredentials(url, { registryKey }, auth) debug('%s@%s: resolving integrity from %s', tarball.name, tarball.version, redactUrl(url)) try { const response = await axios.get(url, assignProxy(url, { headers: { - ...(authHeader !== undefined ? { authorization: authHeader } : {}), + ...(auth !== undefined ? { authorization: auth.header } : {}), }, timeout: DOWNLOAD_TIMEOUT_MS, })) - return select(response.data) + return { dist: select(response.data) ?? undefined } } catch (err: any) { if (err?.response?.status === 404) { return undefined @@ -621,18 +774,37 @@ export class EmbeddedPackagesMaterializer { err, `Failed to fetch registry metadata for embedded package` + ` '${tarball.name}@${tarball.version}' from '${redactUrl(url)}'`, + status => downloadFailureHint(status, sent, npmrc), ) } } - #deriveTarballUrl (tarball: PlannedTarball, npmrcConfig: NpmrcConfig): string { - const registryUrl = resolveRegistryUrl(npmrcConfig, tarball.name, this.#env) - const basename = tarball.name.split('/').pop() - return `${registryUrl}${tarball.name}/-/${basename}-${tarball.version}.tgz` - } - - async #download (tarball: PlannedTarball, url: string, npmrcConfig: NpmrcConfig): Promise { - const authHeader = resolveAuthHeader(npmrcConfig, url, this.#env) + async #download ( + tarball: PlannedTarball, + url: string, + npmrc: LoadedNpmrcConfig, + urlOrigin: UrlOrigin, + ): Promise { + const auth = resolveAuthHeader(npmrc.config, url, tarball.name, this.#env) + + const sent = this.#sentCredentials(url, urlOrigin, auth) + + // A redirect can make the credentials moot: follow-redirects drops + // confidential headers rather than hand them to another host, so + // whatever answered never saw them and "they were rejected" would be + // wrong. Tarball downloads redirect to CDNs routinely. + // + // Observed, not predicted: the drop happens before `beforeRedirect` + // runs and mutates the very options handed to it, so the hook can see + // what actually survived. Re-deriving the library's rule would get + // subdomain redirects (which keep the header) and protocol downgrades + // (which drop it regardless of host) wrong, and would rot silently if + // the policy ever changed. + // + // The hop itself is recorded even when nothing was sent: whatever + // answered is then not the host the reader configured, and telling them + // to add credentials for a host that never asked is its own dead end. + const redirect: RedirectOutcome = {} try { const response = await axios.get(url, assignProxy(url, { @@ -643,7 +815,23 @@ export class EmbeddedPackagesMaterializer { // otherwise make axios gunzip it, breaking integrity verification // with a misleading "different artifact" error. 'accept-encoding': 'identity', - ...(authHeader !== undefined ? { authorization: authHeader } : {}), + ...(auth !== undefined ? { authorization: auth.header } : {}), + }, + beforeRedirect: (options: { host?: string, auth?: string | null, headers?: Record }) => { + redirect.host = options.host + if (sent === undefined) { + return + } + const keptHeader = Object.keys(options.headers ?? {}) + .some(header => header.toLowerCase() === 'authorization') + // `!= null` rather than `!== undefined`: the legacy URL path + // yields `null` here, and treating that as "credentials survived" + // would fail open on the very check meant to catch a drop. + const keptUrlAuth = options.auth != null && options.auth !== '' + // Assigned rather than latched: a later hop back to the original + // origin restores URL credentials, and reporting them as dropped + // would send the reader to inspect the wrong host. + redirect.credentialsDropped = !keptHeader && !keptUrlAuth }, timeout: DOWNLOAD_TIMEOUT_MS, maxContentLength: MAX_TARBALL_BYTES, @@ -654,6 +842,7 @@ export class EmbeddedPackagesMaterializer { err, `Failed to download embedded package '${tarball.name}@${tarball.version}'` + ` from '${redactUrl(url)}'`, + status => downloadFailureHint(status, sent, npmrc, redirect), ) } } diff --git a/packages/cli/src/services/embedded-packages/npmrc.ts b/packages/cli/src/services/embedded-packages/npmrc.ts index b74574d0..4560ce90 100644 --- a/packages/cli/src/services/embedded-packages/npmrc.ts +++ b/packages/cli/src/services/embedded-packages/npmrc.ts @@ -2,8 +2,55 @@ import fs from 'node:fs/promises' import os from 'node:os' import path from 'node:path' +import Debug from 'debug' + +import { parseComposableUrl, parseFetchableUrl } from './url.js' + +const debug = Debug('checkly:cli:services:embedded-packages') + export const DEFAULT_REGISTRY_URL = 'https://registry.npmjs.org/' +/** + * A configuration file to merge, and how hard to insist on reading it. + * `optional` files are skipped with a log line when they exist but cannot + * be read, for files the user did not choose to put there themselves. + */ +export interface NpmrcFile { + path: string + optional?: boolean +} + +/** The prefix that marks an environment variable as npm configuration. */ +export const NPM_CONFIG_ENV_PREFIX = 'npm_config_' + +/** + * Where a config value came from. Structured rather than a display string: + * an environment variable is named by its verbatim spelling, which is what + * the user can actually search for — the key stored in the config map has + * the prefix stripped and may be case-folded. + */ +export type ConfigOrigin = + | { kind: 'file', path: string } + | { kind: 'env', variable: string } + +export interface LoadedNpmrcConfig { + config: NpmrcConfig + /** The config files consulted, highest precedence first. */ + files: string[] + /** + * Optional files that could not be read, and were therefore skipped. Any + * credentials they hold went unused, which is worth saying out loud when + * a download later fails to authenticate. + */ + unreadable: string[] + /** + * Which source each key came from. A credential that a registry rejects + * is far easier to fix when the error can name where it came from, which + * the merged map alone cannot say. + */ + origins: Map +} + /** * Merged `.npmrc` configuration: a flat key → raw value map. Values keep * any `${VAR}` references unexpanded until they're actually used, so an @@ -14,8 +61,10 @@ export type NpmrcConfig = Map export class NpmrcEnvVarError extends Error { constructor (key: string, varName: string) { super( - `The .npmrc value for '${key}' references the environment variable` - + ` '${varName}', which is not set`, + // Not necessarily an .npmrc: the value may equally have come from + // pnpm's auth.ini or an npm_config_* environment variable. + `The npm configuration value for '${key}' references the environment` + + ` variable '${varName}', which is not set`, ) this.name = 'NpmrcEnvVarError' } @@ -53,32 +102,55 @@ export function parseNpmrc (content: string): NpmrcConfig { } /** - * Extracts npm configuration from `npm_config_*` environment variables - * (e.g. `npm_config_registry`, commonly set in CI and by package managers - * running lifecycle scripts). In npm's precedence order these sit above - * every `.npmrc` file. The prefix is matched case-insensitively; the key - * is stored both verbatim and lowercased, because plain keys are written - * in any case (`NPM_CONFIG_REGISTRY`) while nerf-darted auth keys carry a - * case-sensitive spelling (`npm_config_//host/:_authToken`). + * Every `npm_config_*` variable in an environment, as the config key it + * carries plus the variable's verbatim name. The prefix is matched + * case-insensitively; the name is kept because only it is something the + * user can search their environment for. */ -export function npmrcConfigFromEnv (env: NodeJS.ProcessEnv): NpmrcConfig { - const config: NpmrcConfig = new Map() - - const prefix = 'npm_config_' - for (const [name, value] of Object.entries(env)) { - if (value === undefined || !name.toLowerCase().startsWith(prefix)) { +function* npmConfigEnvEntries ( + env: NodeJS.ProcessEnv, +): Generator<{ key: string, value: string, variable: string }> { + for (const [variable, value] of Object.entries(env)) { + if (value === undefined || !variable.toLowerCase().startsWith(NPM_CONFIG_ENV_PREFIX)) { continue } - const key = name.slice(prefix.length) + const key = variable.slice(NPM_CONFIG_ENV_PREFIX.length) // npm drops env config entries with empty values rather than treating // them as set-to-empty. if (key === '' || value === '') { continue } - config.set(key, value) - if (!config.has(key.toLowerCase())) { - config.set(key.toLowerCase(), value) - } + yield { key, value, variable } + } +} + +/** + * Records an env-derived entry under both the verbatim key and, unless one + * is already present, its lowercased alias. Shared so that the config map + * and the origins map cannot drift apart: they must key identically, or a + * value resolves while its origin does not. + */ +function setEnvEntry (map: Map, key: string, value: T): void { + map.set(key, value) + if (!map.has(key.toLowerCase())) { + map.set(key.toLowerCase(), value) + } +} + +/** + * Extracts npm configuration from `npm_config_*` environment variables + * (e.g. `npm_config_registry`, commonly set in CI and by package managers + * running lifecycle scripts). In npm's precedence order these sit above + * every `.npmrc` file. The key is stored both verbatim and lowercased, + * because plain keys are written in any case (`NPM_CONFIG_REGISTRY`) while + * nerf-darted auth keys carry a case-sensitive spelling + * (`npm_config_//host/:_authToken`). + */ +export function npmrcConfigFromEnv (env: NodeJS.ProcessEnv): NpmrcConfig { + const config: NpmrcConfig = new Map() + + for (const { key, value } of npmConfigEnvEntries(env)) { + setEnvEntry(config, key, value) } return config @@ -91,12 +163,20 @@ export function npmrcConfigFromEnv (env: NodeJS.ProcessEnv): NpmrcConfig { * Missing files are skipped. */ export async function loadNpmrcConfig ( - filePaths: string[], + files: NpmrcFile[], env: NodeJS.ProcessEnv = process.env, -): Promise { +): Promise { const merged: NpmrcConfig = npmrcConfigFromEnv(env) + const unreadable: string[] = [] + const origins = new Map() - for (const filePath of filePaths) { + // Keyed exactly as the config map above, so every spelling that resolves + // a value can also name the variable the user actually set. + for (const { key, variable } of npmConfigEnvEntries(env)) { + setEnvEntry(origins, key, { kind: 'env', variable }) + } + + for (const { path: filePath, optional = false } of files) { let content: string try { content = await fs.readFile(filePath, 'utf8') @@ -106,35 +186,118 @@ export async function loadNpmrcConfig ( } // An unreadable .npmrc (e.g. bad permissions) must not silently drop // registry credentials — that would surface later as a baffling 401. - throw new Error(`Unable to read npm configuration from '${filePath}'`, { cause: err }) + // Optional files belong to another tool rather than to this project, + // so an unreadable one must not take the whole command down with it. + // It is still recorded so an authentication failure can say the file + // was skipped. Note this covers more than bad permissions on the file + // itself — an unsearchable parent directory lands here too — so the + // reported wording must not claim the file exists. + if (!optional) { + throw new Error(`Unable to read npm configuration from '${filePath}'`, { cause: err }) + } + debug('skipping unreadable optional config %s: %s', filePath, (err as Error).message) + unreadable.push(filePath) + continue } for (const [key, value] of parseNpmrc(content)) { if (!merged.has(key)) { merged.set(key, value) + origins.set(key, { kind: 'file', path: filePath }) + } + } + } + + return { config: merged, files: files.map(file => file.path), unreadable, origins } +} + +/** + * The file pnpm keeps its global registry credentials in. pnpm 11 stopped + * writing them to `.npmrc`: `pnpm login` writes `auth.ini` in pnpm's global + * config directory instead, so a logged-in pnpm user looks unauthenticated + * to anything that only reads `.npmrc`. + * + * The directory resolution mirrors pnpm's own `getConfigDir` branch for + * branch. Note that `PNPM_HOME` is deliberately NOT consulted: pnpm uses it + * for the data and state directories, never for the config directory. + */ +export function pnpmAuthIniPath ( + env: NodeJS.ProcessEnv, + platform: NodeJS.Platform, + homedir: string, +): string { + const xdgConfigHome = env.XDG_CONFIG_HOME + if (xdgConfigHome !== undefined && xdgConfigHome !== '') { + return path.join(xdgConfigHome, 'pnpm', 'auth.ini') + } + switch (platform) { + case 'darwin': + return path.join(homedir, 'Library', 'Preferences', 'pnpm', 'auth.ini') + case 'win32': { + const localAppData = env.LOCALAPPDATA + if (localAppData !== undefined && localAppData !== '') { + return path.join(localAppData, 'pnpm', 'config', 'auth.ini') } + return path.join(homedir, '.config', 'pnpm', 'auth.ini') } + default: + return path.join(homedir, '.config', 'pnpm', 'auth.ini') } +} - return merged +export interface NpmrcPathsOptions { + /** Workspace root, whose `.npmrc` is consulted. */ + workspaceRoot: string + /** + * The directory the Checkly project lives in (a workspace member in a + * monorepo), whose `.npmrc` takes precedence over the workspace root's. + */ + contextDir?: string + homedir?: string + /** pnpm's global `auth.ini`. Consulted whenever it is provided. */ + pnpmAuthFile?: string + /** + * True when the project's lockfile is pnpm's, which is when `auth.ini` + * outranks the user `.npmrc` — matching pnpm's own precedence. For any + * other package manager it ranks below. + * + * Note that precedence applies per key, as it does in npm and pnpm, not + * per registry: a lower-ranked file's `_authToken` still wins over a + * higher-ranked file's `username`/`_password` for the same registry, + * because the credential kinds are distinct keys and `resolveAuthHeader` + * prefers a token over basic auth. npm's own config cascade behaves the + * same way, and a scope-qualified key beats an unscoped one for the same + * registry for the same reason. + */ + pnpmAuthFilePreferred?: boolean } /** - * The `.npmrc` locations relevant to a project, in npm's precedence order: + * The configuration files relevant to a project, highest precedence first: * the directory the Checkly project lives in (the nearest project config, - * which may be a workspace member), the workspace root, then the - * user-level file. (npm's global and builtin configs are not consulted.) - */ -export function defaultNpmrcPaths ( - workspaceRoot: string, - homedir = os.homedir(), - contextDir?: string, -): string[] { - const paths = [ - ...(contextDir !== undefined ? [path.join(contextDir, '.npmrc')] : []), - path.join(workspaceRoot, '.npmrc'), - path.join(homedir, '.npmrc'), + * which may be a workspace member), the workspace root, then pnpm's global + * `auth.ini` and the user-level `.npmrc` in whichever order the project's + * package manager implies. (npm's global and builtin configs are not + * consulted.) + */ +export function defaultNpmrcPaths (options: NpmrcPathsOptions): NpmrcFile[] { + const { workspaceRoot, contextDir, homedir = os.homedir(), pnpmAuthFile, pnpmAuthFilePreferred } = options + + const userNpmrc: NpmrcFile = { path: path.join(homedir, '.npmrc') } + // Not a file this project chose to have, so an unreadable one is skipped + // rather than failing the command. + const authIni: NpmrcFile[] = pnpmAuthFile !== undefined + ? [{ path: pnpmAuthFile, optional: true }] + : [] + + const files: NpmrcFile[] = [ + ...(contextDir !== undefined ? [{ path: path.join(contextDir, '.npmrc') }] : []), + { path: path.join(workspaceRoot, '.npmrc') }, + ...(pnpmAuthFilePreferred === true ? [...authIni, userNpmrc] : [userNpmrc, ...authIni]), ] - return [...new Set(paths)] + + // Dedupe by path, keeping the highest-precedence occurrence: `new Set` on + // the records themselves would compare by identity and never match. + return files.filter((file, index) => files.findIndex(other => other.path === file.path) === index) } function expandValue (key: string, value: string, env: NodeJS.ProcessEnv): string { @@ -147,72 +310,381 @@ function expandValue (key: string, value: string, env: NodeJS.ProcessEnv): strin }) } -function getExpanded (config: NpmrcConfig, key: string, env: NodeJS.ProcessEnv): string | undefined { - const value = config.get(key) ?? config.get(key.toLowerCase()) - if (value === undefined) { +/** + * Looks a key up, falling back to its lowercased spelling, and reports + * which spelling actually matched. Callers that trace a value back to the + * file it came from need the matched key, not the one they asked for: only + * the former is a key in `LoadedNpmrcConfig`'s `origins`. + */ +function getExpandedEntry ( + config: NpmrcConfig, + key: string, + env: NodeJS.ProcessEnv, +): { value: string, key: string } | undefined { + // npm matches config keys case-insensitively, so both spellings count. + for (const candidate of key === key.toLowerCase() ? [key] : [key, key.toLowerCase()]) { + const value = config.get(candidate) + if (value !== undefined) { + return { value: expandValue(candidate, value, env), key: candidate } + } + } + return undefined +} + +/** + * The same lookup for a credential, where a blank value counts as no value + * at all: an entry left empty rather than deleted — by a token rotation, or + * a logout that clears the line — must not shadow a credential that still + * works. npm and pnpm both test credential values for truthiness for the + * same reason. + * + * What it falls through to is another *key*: another credential kind at the + * same prefix, or a shallower nerf dart. That is as far as it goes, and + * deliberately so — it matches npm, whose `hasAuth` likewise only tries + * other credential kinds at the same or a shallower dart. It does not fall + * through to the other case spelling of the same key, which would reach + * past a blank into a different file. + * + * It does not reach the same key in a lower-precedence file: the merge in + * `loadNpmrcConfig` is first-writer-wins per key, so a blank `_authToken` + * in a project `.npmrc` still masks a working one in `~/.npmrc`. Skipping + * blanks during the merge would fix that, and was tried, but it makes this + * CLI send a credential npm and pnpm would not — they keep blank values + * read from files — so a project that deliberately blanks an entry to force + * anonymous access would have the developer's personal token sent instead. + * + * Deliberately confined to credentials. A blank `registry` is a broken + * setting rather than an absent one, and treating it as absent would fall + * back to the public registry and send private package names to it. That + * holds for values read from files; a blank `npm_config_registry` never + * reaches the config at all, because `npmConfigEnvEntries` drops empty + * environment values at load, matching npm. + * + * A blank value and an unset `${VAR}` are deliberately not the same thing, + * here as in npm and pnpm: a blank value is an entry that exists and holds + * nothing, while an unset variable is a reference to something that does + * not exist — a typo, or a secret missing from the environment — which + * `expandValue` reports by name rather than papering over. + */ +function getCredentialEntry ( + config: NpmrcConfig, + key: string, + env: NodeJS.ProcessEnv, +): { value: string, key: string } | undefined { + // `getExpandedEntry` already stops at the first spelling that exists, so + // mapping its blank result to undefined is the whole difference. + const entry = getExpandedEntry(config, key, env) + return entry?.value === '' ? undefined : entry +} + +/** + * The scope of a package name (`@acme/foo` → `@acme`), or undefined when + * the name is unscoped. A leading `@` with no slash is not a scope: it is a + * malformed name, and treating it as one would silently truncate it. + */ +function packageScope (packageName: string): string | undefined { + if (!packageName.startsWith('@')) { return undefined } - return expandValue(key, value, env) + const separator = packageName.indexOf('/') + return separator > 1 ? packageName.slice(0, separator) : undefined +} + +/** + * The registry a package resolves to, or the reason nothing can be + * requested from it. + * + * A configured value is never quietly replaced by a default — resolving a + * private package against the public registry would disclose its name — so + * a broken one has to be reported rather than substituted. That is a + * discriminated result rather than an unusable URL string because the + * caller composes a path onto `url` and requests it: with + * `registry=https://` the composed URL parses with the PACKAGE NAME as its + * host, and the request, along with any credential nerf-darted to it, goes + * to whoever owns that name. A rule that can send a token somewhere + * unintended belongs in the type rather than in a comment a future caller + * may not read. + * + * `key` names the entry to blame, absent only when nothing configured a + * registry and the public npm one was assumed — which is always usable. + */ +export type ResolvedRegistry = UsableRegistry | { usable: false, key: string } + +/** A registry a request can actually be composed for and sent to. */ +export interface UsableRegistry { + usable: true + url: string + key?: string } /** - * Resolves the registry URL for a package name: the `@scope:registry` entry - * if the package is scoped and one exists, the `registry` entry otherwise, - * falling back to the public npm registry. Always ends with a slash. + * Resolves the registry for a package name: the `@scope:registry` entry if + * the package is scoped and one exists, the `registry` entry otherwise, + * falling back to the public npm registry. */ -export function resolveRegistryUrl ( +export function resolveRegistry ( config: NpmrcConfig, packageName: string, env: NodeJS.ProcessEnv = process.env, -): string { - let registry: string | undefined +): ResolvedRegistry { + let registry: { value: string, key: string } | undefined + + const scope = packageScope(packageName) + if (scope !== undefined) { + registry = getExpandedEntry(config, `${scope}:registry`, env) + + // npm and pnpm both treat a blank `@scope:registry` as unset and use the + // global `registry`, whatever that points at — including the public + // registry, if that is what the project configured. Only a usable value + // counts as the fallback: with nothing to fall back to, the blank entry + // is kept, so the caller reports the key to fix instead of quietly + // assuming the public registry nobody configured. + if (registry?.value === '') { + // An unexpandable global entry is no more usable than a missing one, + // and reporting it would name a key that is not the one in use, so it + // counts as nothing to fall back to. + const fallback = attempt(() => getExpandedEntry(config, 'registry', env)) + if (fallback.error !== undefined) { + debug('ignoring unusable global registry while %s is blank: %s', registry.key, fallback.error.message) + } + + if (fallback.value !== undefined && fallback.value.value !== '') { + debug('%s is blank, falling back to the registry configured by %s', registry.key, fallback.value.key) + registry = fallback.value + } + } + } + + registry ??= getExpandedEntry(config, 'registry', env) + + if (registry === undefined) { + return { usable: true, url: DEFAULT_REGISTRY_URL } + } + + // The trailing slash goes on first: it is part of what a registry URL + // means here, and `//host/npm` composes differently from `//host/npm/`. + const url = registry.value.endsWith('/') ? registry.value : `${registry.value}/` + return parseComposableUrl(url) !== undefined + ? { usable: true, url, key: registry.key } + : { usable: false, key: registry.key } +} + +export interface ResolvedAuth { + /** The `Authorization` header value to send. */ + header: string + /** + * Every config key that contributed, in the spelling that matched. + * Paired with `LoadedNpmrcConfig`'s `origins`, these name the file or + * environment variable a rejected credential came from — indispensable + * once several sources can supply one. `username` + `_password` yields + * two keys rather than one: because precedence is + * per key, the halves routinely come from different files, and the + * password (the half that actually expires) is the one worth naming. + */ + keys: string[] +} + +/** + * The nerf darts a URL's credentials may be keyed by, deepest path first: + * `https://host/a/b` yields `//host/a/b/`, `//host/a/`, `//host/`. The path + * is walked upward because a credential configured for a registry root + * also applies to everything served beneath it. + */ +function nerfDarts (url: URL): string[] { + const segments = url.pathname.split('/').filter(segment => segment !== '') + + const darts: string[] = [] + for (let depth = segments.length; depth >= 0; depth--) { + const prefix = segments.slice(0, depth).map(segment => `${segment}/`).join('') + darts.push(`//${url.host}/${prefix}`) + } + return darts +} + +/** + * The credentials configured under one key prefix, in npm's own order: + * `_authToken` (Bearer), then `username` + `_password` (base64-encoded, per + * npm convention), then `_auth` (pre-encoded Basic). + * + * A prefix is a nerf dart, optionally qualified by a scope + * (`//host/:@acme`). Both halves of a `username` + `_password` pair must + * live under the same prefix: pairing a scoped username with an unscoped + * password would send a credential neither entry describes. + * + * pnpm's `tokenHelper` is deliberately absent from this list. It names an + * external command to run for a token, and running a command found in a + * config file is a decision well beyond resolving a credential. A user who + * has only that configured resolves no credential here and fails as if + * none were configured. + */ +function credentialsAt ( + config: NpmrcConfig, + prefix: string, + env: NodeJS.ProcessEnv, + { skipUnexpandable = false }: { skipUnexpandable?: boolean } = {}, +): ResolvedAuth | undefined { + // `skipUnexpandable` is per key, not per prefix: one entry referencing a + // variable that is not set says nothing about the other credential kinds + // configured beside it. + const entry = (kind: string) => { + const key = `${prefix}:${kind}` + try { + return getCredentialEntry(config, key, env) + } catch (err) { + if (skipUnexpandable && err instanceof NpmrcEnvVarError) { + debug('skipping credential %s: %s', key, err.message) + return undefined + } + throw err + } + } + + const authToken = entry('_authToken') + if (authToken !== undefined) { + return { header: `Bearer ${authToken.value}`, keys: [authToken.key] } + } + + // The pair outranks `_auth`, which is npm's order (`getCredentialsByURI` + // tries `_authToken`, then `username` + `_password`, then `_auth`) and + // matters when a legacy `_auth` line has been left behind beside a pair + // written later: npm authenticates with the pair, and so must this. + // + // Neither half is worth failing over alone — a leftover `username` from a + // setup that moved to a token must not abort a download that a credential + // further along the walk would have authenticated. Whether a half is + // usable is only knowable after expanding it, since a `${VAR}` set to the + // empty string is as absent as a literal blank, so both are attempted and + // an unexpandable one is held rather than thrown. + const username = attempt(() => entry('username')) + const password = attempt(() => entry('_password')) + + if (username.value !== undefined && password.value !== undefined) { + const decodedPassword = Buffer.from(password.value.value, 'base64').toString('utf8') + const encoded = Buffer.from(`${username.value.value}:${decodedPassword}`, 'utf8').toString('base64') + return { header: `Basic ${encoded}`, keys: [username.value.key, password.value.key] } + } + + // Both halves are here in some form, so the pair was meant and a variable + // is genuinely missing — worth naming rather than falling through to a + // credential the user did not intend to use. + const unexpandable = username.error ?? password.error + if (unexpandable !== undefined) { + if (found(username) && found(password)) { + throw unexpandable + } + // Dropped because its other half never materialised, so no pair was + // ever going to form here. Traceable rather than silent: "the CLI + // ignored my entry" is the report this explains. + debug('ignoring half a credential pair at %s: %s', prefix, unexpandable.message) + } - if (packageName.startsWith('@')) { - const scope = packageName.slice(0, packageName.indexOf('/')) - registry = getExpanded(config, `${scope}:registry`, env) + const auth = entry('_auth') + if (auth !== undefined) { + return { header: `Basic ${auth.value}`, keys: [auth.key] } } - registry ??= getExpanded(config, 'registry', env) - registry ??= DEFAULT_REGISTRY_URL + return undefined +} + +/** Whether a held lookup produced anything at all, usable or not. */ +function found (attempted: { value?: unknown, error?: NpmrcEnvVarError }): boolean { + return attempted.value !== undefined || attempted.error !== undefined +} - return registry.endsWith('/') ? registry : `${registry}/` +/** + * Runs a lookup, holding an `NpmrcEnvVarError` instead of raising it so the + * caller can decide whether the key it names was ever going to be used. + */ +function attempt (lookup: () => T): { value?: T, error?: NpmrcEnvVarError } { + try { + return { value: lookup() } + } catch (err) { + if (err instanceof NpmrcEnvVarError) { + return { error: err } + } + throw err + } } /** - * Resolves the `Authorization` header value applicable to a URL, matching - * npm's "nerf dart" scheme: credentials are keyed by the registry URL minus - * its protocol (`//host/path/:_authToken=...`). The URL's path is walked - * upward so credentials configured for a registry root also apply to - * tarball URLs beneath it. Supports `_authToken` (Bearer), `_auth` - * (pre-encoded Basic), and `username` + `_password` (base64-encoded, per - * npm convention). Returns undefined when no credentials match. + * Resolves the `Authorization` header applicable to a URL, matching npm's + * "nerf dart" scheme: credentials are keyed by the registry URL minus its + * protocol (`//host/path/:_authToken=...`). + * + * `pnpm login --scope=@acme` writes a scope-qualified key instead + * (`//host/:@acme:_authToken=...`, or equivalently `//host/@acme/:_authToken`), + * so the package being downloaded decides which keys apply. Mirroring pnpm, + * every scoped key is tried before any unscoped one — a shallow scoped + * credential outranks a deeper unscoped one, rather than the two being + * interleaved by depth. An unscoped package never falls back to a scoped + * key: a scoped token belongs to one organisation by construction, and + * reusing it elsewhere would send that organisation's credential somewhere + * it was never meant to go. + * + * Scope-qualified keys are honored for every project, not only pnpm ones: + * npm, yarn, bun and pnpm 10 ignore the spelling entirely, but writing one + * is an unambiguous statement of which token that scope should use, and + * refusing to read it would leave a user whose only login is + * `pnpm login --scope` unauthenticated for exactly the packages the key + * names. + * + * Returns undefined when no credentials match. */ export function resolveAuthHeader ( config: NpmrcConfig, url: string, + packageName: string, env: NodeJS.ProcessEnv = process.env, -): string | undefined { - const parsed = new URL(url) - - const segments = parsed.pathname.split('/').filter(segment => segment !== '') - for (let depth = segments.length; depth >= 0; depth--) { - const nerfDart = `//${parsed.host}/${segments.slice(0, depth).map(segment => `${segment}/`).join('')}` +): ResolvedAuth | undefined { + // The same rule the request itself has to pass. Callers check it first, + // so this is belt and braces — but the safe answer for a URL that could + // not be validated is no credentials rather than a thrown parse error. + const parsed = parseFetchableUrl(url) + if (parsed === undefined) { + return undefined + } - const authToken = getExpanded(config, `${nerfDart}:_authToken`, env) - if (authToken !== undefined) { - return `Bearer ${authToken}` - } + const darts = nerfDarts(parsed) + const scope = packageScope(packageName) - const auth = getExpanded(config, `${nerfDart}:_auth`, env) - if (auth !== undefined) { - return `Basic ${auth}` + const scoped = scope !== undefined ? darts.map(dart => `${dart}:${scope}`) : [] + for (const prefix of [...scoped, ...darts]) { + const credentials = credentialsAt(config, prefix, env) + if (credentials !== undefined) { + return credentials } + } - const username = getExpanded(config, `${nerfDart}:username`, env) - const password = getExpanded(config, `${nerfDart}:_password`, env) - if (username !== undefined && password !== undefined) { - const decodedPassword = Buffer.from(password, 'base64').toString('utf8') - return `Basic ${Buffer.from(`${username}:${decodedPassword}`, 'utf8').toString('base64')}` + // pnpm accepts a second spelling, `//host/@acme/:_authToken`, which it + // binds to the registry the scope segment was stripped from rather than to + // the path — so it covers every `@acme` package on that host, including + // tarball URLs that never mention the scope at that depth, as GitHub + // Packages' `/download/@acme/...` URLs do not. + // + // It is tried last rather than with the colon form, which is where pnpm + // ranks it. The spelling is indistinguishable from an ordinary nerf dart + // for the path `/@acme/`, which is exactly how npm, yarn and bun read it, + // so giving it pnpm's rank would let it outrank a deeper unscoped key that + // authenticates a working setup today. + // + // Tried last it can only supply a credential where nothing else matched, + // and an unexpandable one is skipped rather than fatal — but only when + // this walk is the sole reading of that key. For a URL whose path does + // contain the scope, such as the `${registry}/@acme/foo/-/…` this CLI + // composes, `//host/@acme/` is an ordinary nerf dart the unscoped walk + // above already visited, and there it is a key that plainly applies to + // this request, so a variable missing from it is fatal like any other. + // The tolerance is for the other shape — GitHub Packages' `/download/…`, + // or a CDN URL — where the key names a location this request never + // touches and must not abort a download that would otherwise go out. + // + // A skip is only visible on the debug channel, so a download that then + // fails to authenticate reports finding no credentials at all. + const pathForm = scope !== undefined ? darts.map(dart => `${dart}${scope}/`) : [] + for (const prefix of pathForm) { + const credentials = credentialsAt(config, prefix, env, { skipUnexpandable: true }) + if (credentials !== undefined) { + return credentials } } diff --git a/packages/cli/src/services/embedded-packages/url.ts b/packages/cli/src/services/embedded-packages/url.ts new file mode 100644 index 00000000..1bd0654f --- /dev/null +++ b/packages/cli/src/services/embedded-packages/url.ts @@ -0,0 +1,61 @@ +/** + * Parses a URL that something is about to be fetched from, or returns + * undefined when nothing can be. + * + * The scheme has to be one this CLI actually fetches over: `ftp://host/x` + * parses and has a host, but axios cannot retrieve it, and tarball URLs + * read out of lockfiles and registry metadata are already held to the same + * http(s) rule. That also settles the shapes a bare parse lets through, + * such as `admin:s3cret@nexus.local/x` — the opaque scheme `admin:` with an + * empty host, and a credential sitting in what looks like a path. The host + * is checked as well, belt and braces: the WHATWG parser refuses a + * host-less `https://` today, and this rule means "a request can be made", + * which needs somewhere to send it. + * + * This decides both whether a request can be made and whether the value is + * safe to echo, which must be the same rule: a URL that reaches the network + * is one `redactUrl` can rebuild from parts that cannot carry a secret. + */ +export function parseFetchableUrl (url: string): URL | undefined { + let parsed: URL + try { + parsed = new URL(url) + } catch { + return undefined + } + if (parsed.host === '' || (parsed.protocol !== 'http:' && parsed.protocol !== 'https:')) { + return undefined + } + return parsed +} + +/** + * The rule `parseFetchableUrl` enforces, as a sentence for a message. Kept + * beside the check so the two cannot drift into describing different rules. + */ +export const FETCHABLE_URL_REQUIREMENT = 'an absolute http or https URL with a host' + +/** + * Parses a URL that a path will be appended to, or returns undefined when + * appending would not do what it looks like. + * + * Everything `parseFetchableUrl` requires, plus no query and no fragment: + * those swallow whatever follows them. `https://host/npm/?token=abc` with + * `bar/-/bar-2.0.0.tgz` appended parses with its pathname still `/npm/` and + * the whole package path inside the query, so the request would go to the + * registry root and fail later as something that reads like the registry's + * fault. Excluding them is what makes "appending a path leaves the host and + * the path intact" true rather than nearly true. + */ +export function parseComposableUrl (url: string): URL | undefined { + // Tested on the raw string, not on `search` and `hash`: a URL ending in a + // bare `?` or `#` parses with both of those empty, yet still absorbs + // whatever is appended after the delimiter. + if (/[?#]/.test(url)) { + return undefined + } + return parseFetchableUrl(url) +} + +/** The rule `parseComposableUrl` enforces, as a sentence for a message. */ +export const COMPOSABLE_URL_REQUIREMENT = `${FETCHABLE_URL_REQUIREMENT}, carrying no query or fragment`