From 6d17b669b4fbe287538010891c33e4665dcc632d Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Tue, 2 Jun 2026 15:28:21 +0200 Subject: [PATCH] fix: verify lockfile tarball URL matches registry metadata (#12134) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What The lockfile resolution verifier now confirms that a registry entry pinning an explicit `tarball` URL points at the artifact the registry's own metadata lists for that `name@version`. A mismatch — or any entry that can't be confirmed against the registry — is rejected with `ERR_PNPM_TARBALL_URL_MISMATCH`. ## Why Follow-up to the design discussion on #12122. The verifier checked the age/trust of `name@version` against the registry packument but never bound the lockfile's `tarball` URL to it. For the non-standard entries pnpm preserves a tarball URL for (npm Enterprise, GitHub Packages — see `toLockfileResolution`), pnpm fetches straight from that URL. So a **tampered lockfile could pair a trusted `name@version` with an attacker-chosen tarball URL** (plus a matching integrity for the attacker's bytes); verification passed against the legitimate version while the install fetched the attacker's bytes. Defending a checked-in lockfile is explicitly in this feature's threat model. ## How - For a registry-keyed entry that pins an explicit `tarball`, fetch the packument and assert the URL equals `versions[v].dist.tarball`. The comparison canonicalizes away benign differences — http/https scheme, default ports (`:443`/`:80`), and `%2f` scope-separator encoding (case-insensitive) — so only real mismatches are flagged. The packument is fetched from the user's configured registry (the lockfile's tarball host can't redirect it), and named-registry routing uses the same canonicalization so a scheme/`%2f`-only difference doesn't route to the wrong packument. - **The binding is unconditional.** It runs regardless of `minimumReleaseAge`/`trustPolicy` and is **not** narrowed by their exclude lists, because it guards *integrity*, not *maturity/trust*. Disabling the age/trust policies must not silently disable anti-tamper. (`createNpmResolutionVerifier` therefore always returns a verifier.) - **It is fail-closed.** An entry passes only when the registry metadata affirmatively lists the version with a matching tarball URL. If the metadata can't be fetched, doesn't list the version, or omits `dist.tarball`, the entry is rejected — otherwise a tampered lockfile could smuggle a malicious URL past the check by pointing it at a `name@version` the registry can't vouch for. - **Behavior change:** as a result, an install that re-verifies a lockfile (its content changed since the last verified run, so the verification cache no longer short-circuits) now requires the configured registry to be reachable. `trustLockfile` is the opt-out for environments that treat the on-disk lockfile as already trusted. - **Verification cache.** The policy snapshot records a `tarballUrlBinding` marker and `canTrustPastCheck` requires it, so a cache record written before this rule existed is re-verified rather than trusted (closing an upgrade-time bypass). - Entries with no explicit `tarball` reconstruct the URL from name+version+registry and are inherently bound (no check). `file:`/git-hosted resolutions stay out of scope (#12122). - Threads `nonSemverVersion` to the verifier so URL-keyed tarball deps (a remote `https:` tarball that carries a semver `version` copied from its manifest) are recognized as deliberate non-registry deps and skipped — also fixing a latent release-age over-match on them. The candidate dedupe key includes `nonSemverVersion` so a registry snapshot and a URL-keyed snapshot sharing a `name@version` and serialized resolution stay distinct. Mirrored in pacquet (`create_npm_resolution_verifier`). The dedupe-key change is TS-only: pacquet's candidate `version` comes from the lockfile key suffix, so the two shapes never share a key there. ## Tests - TS: confirmed mismatch → violation; non-standard URL matching metadata → pass; default-port/scheme difference → pass; URL-keyed dep → skipped; URL binding runs (and fails closed) with no age/trust policy configured; `canTrustPastCheck` rejects a cache record lacking the binding marker. Regression-verified (the mismatch test fails when the check is disabled). - pacquet: mirror tests + the no-policy / `minimumReleaseAge: 0` / `trustPolicy: off` cases, default-port/scheme equivalence, and the missing-`tarballUrlBinding` cache rejection. A few install-dispatch / resolution-reuse tests that pin a deliberately bogus tarball URL (or run against an unreachable registry to prove resolution reuse) now set `trustLockfile`, since the always-on fail-closed tarball-URL check would otherwise flag the fixture before the path under test runs. - `clippy --deny warnings`, `fmt`, and `dylint` clean. --- .changeset/tarball-url-binding.md | 13 + .../src/install/verifyLockfileResolutions.ts | 22 +- .../cli/tests/lockfile_resolution_reuse.rs | 12 +- .../src/build_resolution_verifiers.rs | 10 +- .../package-manager/src/install/tests.rs | 30 +- .../src/create_npm_resolution_verifier.rs | 154 +++++++-- .../create_npm_resolution_verifier/tests.rs | 299 ++++++++++++++---- .../src/lookup_context.rs | 25 +- .../src/violation_codes.rs | 1 + resolving/default-resolver/src/index.ts | 8 +- .../src/createNpmResolutionVerifier.ts | 140 ++++++-- resolving/npm-resolver/src/violationCodes.ts | 1 + .../test/createNpmResolutionVerifier.test.ts | 173 +++++++++- resolving/resolver-base/src/index.ts | 9 +- 14 files changed, 716 insertions(+), 181 deletions(-) create mode 100644 .changeset/tarball-url-binding.md diff --git a/.changeset/tarball-url-binding.md b/.changeset/tarball-url-binding.md new file mode 100644 index 0000000000..ff7f3c1a4d --- /dev/null +++ b/.changeset/tarball-url-binding.md @@ -0,0 +1,13 @@ +--- +"@pnpm/resolving.npm-resolver": minor +"@pnpm/resolving.resolver-base": minor +"@pnpm/resolving.default-resolver": patch +"@pnpm/installing.deps-installer": patch +"pnpm": minor +--- + +The lockfile verifier now checks that a registry entry pinning an explicit `tarball` URL points at the artifact the registry's own metadata lists for that `name@version`. Previously a tampered lockfile could pair a trusted `name@version` with an attacker-chosen tarball URL (and a matching integrity for those bytes), so the install fetched the attacker's bytes. A mismatch — or any entry that can't be confirmed against the registry — is rejected with `ERR_PNPM_TARBALL_URL_MISMATCH`. Non-registry resolutions (`file:`, git-hosted, etc.) and registry entries without an explicit tarball URL (the URL is reconstructed from name+version+registry, so it is inherently bound) are unaffected; non-standard registry tarball URLs (npm Enterprise, GitHub Packages) still pass because they match the metadata. + +This binding is unconditional — it runs regardless of `minimumReleaseAge`/`trustPolicy` and is not narrowed by their exclude lists, since it guards integrity rather than maturity/trust. It is **fail-closed**: an entry passes only when the registry metadata affirmatively lists the version with a matching tarball URL. If the metadata can't be fetched, doesn't list the version, or omits `dist.tarball`, the entry is rejected. As a result, an install that re-verifies a lockfile (any install whose lockfile content changed since the last verified run, where the verification cache no longer applies) now requires the configured registry to be reachable. `trustLockfile` is the opt-out for environments that treat the on-disk lockfile as already trusted. + +The `minimumReleaseAge`/`trustPolicy` verification also no longer applies to URL-keyed tarball dependencies (e.g. `https:` tarballs) that carry a semver `version` copied from their manifest — those are deliberate non-registry dependencies. diff --git a/installing/deps-installer/src/install/verifyLockfileResolutions.ts b/installing/deps-installer/src/install/verifyLockfileResolutions.ts index c2ac9a2c2e..74b3b67717 100644 --- a/installing/deps-installer/src/install/verifyLockfileResolutions.ts +++ b/installing/deps-installer/src/install/verifyLockfileResolutions.ts @@ -231,6 +231,7 @@ export async function collectResolutionPolicyViolations ( interface Candidate { name: string version: string + nonSemverVersion?: string resolution: Resolution } @@ -239,21 +240,24 @@ interface Candidate { // therefore appear multiple times. Dedupe so we issue at most one // verification per package version. // -// Include a serialization of `resolution` in the key so two entries that -// share a (name, version) but differ in *what* was resolved (e.g. one -// pinned via npm, another via a git URL under the same alias) don't -// collapse: if the wrong shape wins the dedup, a protocol-scoped -// verifier short-circuits on the surviving entry and the real one is +// Include a serialization of `resolution` and `nonSemverVersion` in the key +// so two entries that share a (name, version) but differ in *what* was +// resolved (e.g. one pinned via npm, another a URL-keyed tarball whose +// snapshot copied the same semver `version` from its manifest) don't +// collapse: `nonSemverVersion` flips whether the npm verifier enforces or +// skips the tarball/policy checks, so if the wrong shape wins the dedup the +// surviving entry is verified under the wrong rules and the real one is // never checked. function collectCandidates (lockfile: LockfileObject): Map { const candidates = new Map() for (const [depPath, snapshot] of Object.entries(lockfile.packages ?? {})) { - const { name, version } = nameVerFromPkgSnapshot(depPath as DepPath, snapshot) + const { name, version, nonSemverVersion } = nameVerFromPkgSnapshot(depPath as DepPath, snapshot) if (!name || !version) continue - const key = `${name}@${version}@${JSON.stringify(snapshot.resolution)}` + const key = `${name}@${version}@${nonSemverVersion ?? ''}@${JSON.stringify(snapshot.resolution)}` candidates.set(key, { name, version, + nonSemverVersion, resolution: snapshot.resolution as Resolution, }) } @@ -268,7 +272,7 @@ async function iterateLockfileViolations ( const violations: ResolutionPolicyViolation[] = [] const limit = pLimit(concurrency ?? DEFAULT_CONCURRENCY) await Promise.all( - Array.from(candidates.values(), ({ name, version, resolution }) => limit(async () => { + Array.from(candidates.values(), ({ name, version, nonSemverVersion, resolution }) => limit(async () => { // Fan out across every active verifier; each handles its own // protocol short-circuit (e.g. the npm verifier returns ok:true for // git resolutions). We stop at the first failure per entry so a @@ -276,7 +280,7 @@ async function iterateLockfileViolations ( // same (name, version). for (const verifier of verifiers) { // eslint-disable-next-line no-await-in-loop - const result = await verifier.verify(resolution, { name, version }) + const result = await verifier.verify(resolution, { name, version, nonSemverVersion }) if (!result.ok) { violations.push({ name, version, resolution, code: result.code, reason: result.reason }) break diff --git a/pacquet/crates/cli/tests/lockfile_resolution_reuse.rs b/pacquet/crates/cli/tests/lockfile_resolution_reuse.rs index fbad9ad773..c94ed79738 100644 --- a/pacquet/crates/cli/tests/lockfile_resolution_reuse.rs +++ b/pacquet/crates/cli/tests/lockfile_resolution_reuse.rs @@ -47,15 +47,15 @@ fn reuses_unchanged_subtree_without_re_resolving_from_the_registry() { CommandTempCwd::init().add_mocked_registry(); let AddMockedRegistry { mock_instance, npmrc_path, .. } = npmrc_info; - // Disable `minimumReleaseAge` so the post-resolution lockfile - // verifier doesn't fetch each entry's metadata from the registry — - // that fetch is a separate concern from resolution reuse, and with - // the default (1 day) it would hit the dead registry regardless of + // Trust the lockfile so the post-resolution verifier doesn't fetch + // each entry's metadata from the registry — that verification is a + // separate concern from resolution reuse, and (now that it always runs + // and fails closed) it would hit the dead registry regardless of // whether resolution was reused, masking what this test proves. let workspace_yaml = workspace.join("pnpm-workspace.yaml"); let existing = fs::read_to_string(&workspace_yaml).expect("read pnpm-workspace.yaml"); - fs::write(&workspace_yaml, format!("{existing}minimumReleaseAge: 0\n")) - .expect("append minimumReleaseAge to pnpm-workspace.yaml"); + fs::write(&workspace_yaml, format!("{existing}trustLockfile: true\n")) + .expect("append trustLockfile to pnpm-workspace.yaml"); // `@pnpm.e2e/pkg-with-1-dep@100.0.0` depends on // `@pnpm.e2e/dep-of-pkg-with-1-dep@^100.0.0`, so the lockfile records diff --git a/pacquet/crates/package-manager/src/build_resolution_verifiers.rs b/pacquet/crates/package-manager/src/build_resolution_verifiers.rs index 6a8e729851..4c06931c94 100644 --- a/pacquet/crates/package-manager/src/build_resolution_verifiers.rs +++ b/pacquet/crates/package-manager/src/build_resolution_verifiers.rs @@ -55,9 +55,9 @@ pub enum BuildVerifiersError { }, } -/// Assemble the verifier list for this install. Returns an empty -/// `Vec` when neither policy is active — the runner short-circuits -/// on an empty list, so the caller doesn't need a separate guard. +/// Assemble the verifier list for this install. The npm verifier is +/// always included — it enforces the tarball-URL binding regardless of +/// policy configuration — so the list is non-empty. /// /// `meta_cache` is the optional per-install packument cache shared /// with the resolver. When provided, the verifier reads it before @@ -123,9 +123,7 @@ pub fn build_resolution_verifiers( now: None, }; - if let Some(verifier) = create_npm_resolution_verifier(opts) { - verifiers.push(Arc::new(verifier)); - } + verifiers.push(Arc::new(create_npm_resolution_verifier(opts))); Ok(verifiers) } diff --git a/pacquet/crates/package-manager/src/install/tests.rs b/pacquet/crates/package-manager/src/install/tests.rs index c33610d557..e1d6dbc7fb 100644 --- a/pacquet/crates/package-manager/src/install/tests.rs +++ b/pacquet/crates/package-manager/src/install/tests.rs @@ -1367,7 +1367,7 @@ async fn warm_reinstall_skips_snapshot_when_current_lockfile_matches() { prefer_frozen_lockfile: None, ignore_manifest_check: false, skip_runtimes: false, - trust_lockfile: false, + trust_lockfile: true, // fixture pins a tripwire tarball URL; skip resolution verification so the tarball-URL check doesn't flag it before the path under test update_checksums: false, is_full_install: true, supported_architectures: None, @@ -1468,7 +1468,7 @@ async fn warm_reinstall_emits_broken_modules_when_dir_is_missing() { prefer_frozen_lockfile: None, ignore_manifest_check: false, skip_runtimes: false, - trust_lockfile: false, + trust_lockfile: true, // fixture pins a tripwire tarball URL; skip resolution verification so the tarball-URL check doesn't flag it before the path under test update_checksums: false, is_full_install: true, supported_architectures: None, @@ -1725,7 +1725,7 @@ async fn warm_reinstall_reports_added_zero_and_emits_no_imported_events() { prefer_frozen_lockfile: None, ignore_manifest_check: false, skip_runtimes: false, - trust_lockfile: false, + trust_lockfile: true, // fixture pins a tripwire tarball URL; skip resolution verification so the tarball-URL check doesn't flag it before the path under test update_checksums: false, is_full_install: true, supported_architectures: None, @@ -1788,6 +1788,12 @@ async fn warm_reinstall_reports_added_zero_and_emits_no_imported_events() { /// network / integrity failure — distinguishable from the early /// `OutdatedLockfile` we expect. /// +/// `trust_lockfile` is on so lockfile-resolution verification is +/// skipped: the unconditional tarball-URL binding check would otherwise +/// flag the fixture's tripwire tarball URL as a `TARBALL_URL_MISMATCH` +/// before the drift gate runs. Verification is orthogonal to the drift +/// check this test exercises. +/// /// [#447]: https://github.com/pnpm/pacquet/issues/447 #[tokio::test] async fn frozen_lockfile_errors_when_manifest_drifts_from_lockfile() { @@ -1824,7 +1830,7 @@ async fn frozen_lockfile_errors_when_manifest_drifts_from_lockfile() { prefer_frozen_lockfile: None, ignore_manifest_check: false, skip_runtimes: false, - trust_lockfile: false, + trust_lockfile: true, update_checksums: false, is_full_install: true, resolved_packages: &Default::default(), @@ -2933,12 +2939,6 @@ async fn frozen_install_silently_swallows_unreachable_optional_tarball() { // Keep retries minimal — 127.0.0.1:1 fails immediately on every // try, but a long retry schedule would dominate the test runtime. config.fetch_retries = 0; - // The lockfile-verification gate is unrelated to what this test - // exercises (optional-tarball swallow path). Disable - // `minimumReleaseAge` so the gate doesn't try to fetch metadata - // for `broken-pkg` against the unreachable default registry - // (which would fail closed with a verifier violation and abort - // the install before the optional-snapshot code path runs). config.minimum_release_age = None; let config = config.leak(); @@ -2958,7 +2958,13 @@ async fn frozen_install_silently_swallows_unreachable_optional_tarball() { prefer_frozen_lockfile: None, ignore_manifest_check: false, skip_runtimes: false, - trust_lockfile: false, + // The lockfile-resolution verifier is unrelated to what this test + // exercises (the optional-tarball swallow path) and now always runs; + // its fail-closed tarball-URL check would otherwise try to fetch + // metadata for `broken-pkg` from the unreachable default registry and + // abort the install before the optional-snapshot code path runs. + // `trust_lockfile` is the opt-out that skips verification entirely. + trust_lockfile: true, update_checksums: false, is_full_install: true, supported_architectures: None, @@ -4791,7 +4797,7 @@ async fn prefer_frozen_lockfile_takes_frozen_path_when_lockfile_is_fresh() { prefer_frozen_lockfile: None, ignore_manifest_check: false, skip_runtimes: false, - trust_lockfile: false, + trust_lockfile: true, // fixture pins a tripwire tarball URL; skip resolution verification so the tarball-URL check doesn't flag it before the path under test update_checksums: false, is_full_install: true, supported_architectures: None, diff --git a/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs b/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs index 7f25fb5346..6b7c754287 100644 --- a/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs +++ b/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs @@ -4,11 +4,13 @@ //! [`createNpmResolutionVerifier.ts`](https://github.com/pnpm/pnpm/blob/2a9bd897bf/resolving/npm-resolver/src/createNpmResolutionVerifier.ts). //! //! The factory takes the install-time policy (cutoff time, exclude -//! patterns, trust policy, named registries) and returns a verifier -//! when at least one policy is active. The verifier inspects each -//! npm-registry-resolved lockfile entry, applies the -//! `minimumReleaseAge` and/or `trustPolicy='no-downgrade'` checks, -//! and surfaces violations through [`ResolutionVerification::Err`]. +//! patterns, trust policy, named registries) and returns a verifier. +//! The verifier inspects each npm-registry-resolved lockfile entry: it +//! always binds the recorded tarball URL to the artifact the registry's +//! metadata lists (an anti-tamper check independent of any policy), and +//! additionally applies the `minimumReleaseAge` and/or +//! `trustPolicy='no-downgrade'` checks when those are configured. +//! Violations surface through [`ResolutionVerification::Err`]. //! //! The publish-timestamp lookup walks a 4-layer fallback chain //! (abbreviated-modified shortcut → local mirror → attestation @@ -39,7 +41,10 @@ use crate::{ named_registry::{build_named_registry_prefixes, pick_registry_for_package}, pick_package::PackageMetaCache, trust_checks::fail_if_trust_downgraded, - violation_codes::{MINIMUM_RELEASE_AGE_VIOLATION_CODE, TRUST_DOWNGRADE_VIOLATION_CODE}, + violation_codes::{ + MINIMUM_RELEASE_AGE_VIOLATION_CODE, TARBALL_URL_MISMATCH_VIOLATION_CODE, + TRUST_DOWNGRADE_VIOLATION_CODE, + }, }; /// Options bundle for [`create_npm_resolution_verifier`]. Mirrors @@ -150,21 +155,18 @@ impl std::fmt::Debug for NpmResolutionVerifier { } } -/// Returns an [`NpmResolutionVerifier`] when at least one policy is -/// active, [`None`] otherwise. The empty case lets the install side -/// skip building a verifier list, which collapses the fan-out to a -/// straight pass — every lockfile entry yields `Ok`. +/// Builds the [`NpmResolutionVerifier`]. It always binds each entry's +/// recorded tarball URL to the artifact the registry's metadata lists (an +/// anti-tamper check independent of any policy), and additionally applies +/// the `minimum_release_age` / `trust_policy='no-downgrade'` checks when +/// those are configured. /// /// Mirrors upstream's /// [`createNpmResolutionVerifier`](https://github.com/pnpm/pnpm/blob/2a9bd897bf/resolving/npm-resolver/src/createNpmResolutionVerifier.ts#L98-L253). pub fn create_npm_resolution_verifier( opts: CreateNpmResolutionVerifierOptions, -) -> Option { +) -> NpmResolutionVerifier { let age_check_active = opts.minimum_release_age.is_some_and(|minutes| minutes > 0); - let trust_check_active = matches!(opts.trust_policy, Some(TrustPolicy::NoDowngrade)); - if !age_check_active && !trust_check_active { - return None; - } let cutoff = if age_check_active { let minutes = opts.minimum_release_age.unwrap_or(0); @@ -196,7 +198,7 @@ pub fn create_npm_resolution_verifier( opts.trust_policy_ignore_after, ); - Some(NpmResolutionVerifier { + NpmResolutionVerifier { minimum_release_age_minutes: opts.minimum_release_age, cutoff, minimum_release_age_exclude: opts.minimum_release_age_exclude, @@ -215,7 +217,7 @@ pub fn create_npm_resolution_verifier( now: opts.now, policy_snapshot, lookup_context: PublishedAtLookupContext::new(), - }) + } } impl ResolutionVerifier for NpmResolutionVerifier { @@ -232,6 +234,13 @@ impl ResolutionVerifier for NpmResolutionVerifier { } fn can_trust_past_check(&self, cached_policy: &serde_json::Map) -> bool { + // The tarball-URL binding is unconditional today; a cached run + // that didn't record it (e.g. written before this rule existed) + // can't be trusted to have enforced it, so force a re-check. + if cached_policy.get("tarballUrlBinding").and_then(JsonValue::as_bool) != Some(true) { + return false; + } + // Maturity: a previously cached run under a larger cutoff // (stricter window) is trustworthy under a smaller current one // — the set of accepted versions is a subset of today's. @@ -299,6 +308,22 @@ impl NpmResolutionVerifier { return ResolutionVerification::Ok; } + let registry = self.pick_registry(ctx.name, tarball_url); + + // A registry entry that pins an explicit tarball URL must point at + // the artifact the registry's own metadata lists. Otherwise a trusted + // name@version could front bytes from an attacker-chosen URL (with a + // matching integrity for those bytes). This binding is unconditional — + // it does not depend on the minimum-release-age / trust policies and + // isn't narrowed by their exclude lists, since it guards integrity + // rather than maturity/trust. + if let Some(url) = tarball_url + && let Some(violation) = + self.run_tarball_url_check(®istry, ctx.name, ctx.version, url).await + { + return violation; + } + let age_applies = self.age_check_active() && !is_excluded(self.minimum_release_age_exclude.as_ref(), ctx.name, ctx.version); let trust_applies = self.trust_check_active() @@ -307,8 +332,6 @@ impl NpmResolutionVerifier { return ResolutionVerification::Ok; } - let registry = self.pick_registry(ctx.name, tarball_url); - if age_applies && let Some(violation) = self.run_age_check(®istry, ctx.name, ctx.version).await { @@ -340,12 +363,15 @@ impl NpmResolutionVerifier { } fn pick_registry(&self, name: &PkgName, tarball_url: Option<&str>) -> String { - if let Some(url) = tarball_url - && let Ok(parsed) = reqwest::Url::parse(url) - { - let normalized = parsed.as_str(); + if let Some(url) = tarball_url { + // Match on the same canonical form the tarball comparison uses, so + // a named-registry tarball that differs from the configured base + // only by scheme or `%2f` encoding still routes to its registry + // instead of falling back (and then failing closed against the + // wrong packument). + let normalized = canonical_tarball_url(url); for prefix in &self.named_registry_prefixes { - if normalized.starts_with(prefix) { + if normalized.starts_with(&canonical_tarball_url(prefix)) { return prefix.clone(); } } @@ -353,6 +379,44 @@ impl NpmResolutionVerifier { pick_registry_for_package(&self.registries, &name.to_string(), None) } + /// Confirm the lockfile-pinned tarball URL is the artifact the + /// registry's own metadata lists for this exact `name@version`. + /// + /// Fail-closed: the entry passes only when the registry metadata + /// affirmatively lists this version with a matching tarball URL. If the + /// metadata can't be fetched, doesn't list the version, or omits + /// `dist.tarball`, the entry can't be confirmed and is rejected — + /// otherwise a tampered lockfile could smuggle a malicious URL past the + /// check by pointing it at a `name@version` the registry can't vouch for. + async fn run_tarball_url_check( + &self, + registry: &str, + name: &PkgName, + version: &str, + lockfile_tarball: &str, + ) -> Option { + let registry_tarball = match self.fetch_abbreviated_meta(registry, name).await { + Ok(Some(meta)) => { + meta.version_tarballs.and_then(|tarballs| tarballs.get(version).cloned()) + } + Ok(None) | Err(_) => None, + }; + match registry_tarball { + Some(url) if same_tarball_url(lockfile_tarball, &url) => None, + Some(url) => Some(ResolutionVerification::Err { + code: TARBALL_URL_MISMATCH_VIOLATION_CODE, + reason: format!( + "has a tarball URL ({lockfile_tarball}) that does not match the registry's published metadata ({url})", + ), + }), + None => Some(ResolutionVerification::Err { + code: TARBALL_URL_MISMATCH_VIOLATION_CODE, + reason: "could not be verified against the registry's published metadata" + .to_string(), + }), + } + } + async fn run_age_check( &self, registry: &str, @@ -530,7 +594,7 @@ impl NpmResolutionVerifier { if parsed.with_timezone(&Utc) >= cutoff { return Ok(None); } - if !meta.version_names.as_ref().is_some_and(|set| set.contains(version)) { + if !meta.version_tarballs.as_ref().is_some_and(|map| map.contains_key(version)) { return Ok(None); } Ok(Some(modified)) @@ -809,6 +873,9 @@ fn build_policy_snapshot( trust_policy_ignore_after: Option, ) -> serde_json::Map { let mut map = serde_json::Map::new(); + // Marks runs that enforced the (unconditional) tarball-URL binding so + // `can_trust_past_check` rejects pre-rule cache records and re-verifies. + map.insert("tarballUrlBinding".to_string(), JsonValue::Bool(true)); map.insert("minimumReleaseAge".to_string(), JsonValue::from(minimum_release_age)); map.insert( "minimumReleaseAgeExclude".to_string(), @@ -923,17 +990,44 @@ fn project_trust_package_version(version: &PackageVersion) -> PackageVersion { } } -/// Pull the `(modified, versionNames)` projection the abbreviated -/// shortcut needs out of a packument document. Works against either -/// the abbreviated or the full form — both carry `modified` and a -/// `versions` map. +/// Pull the `(modified, versionTarballs)` projection the verifier +/// needs out of a packument document. Works against either the +/// abbreviated or the full form — both carry `modified` and a +/// `versions` map with per-version `dist.tarball`. /// /// Mirrors upstream's /// [`projectAbbreviatedMeta`](https://github.com/pnpm/pnpm/blob/2a9bd897bf/resolving/npm-resolver/src/createNpmResolutionVerifier.ts#L702-L707). fn project_abbreviated_meta(meta: &Package) -> crate::lookup_context::AbbreviatedMetaProjection { + let version_tarballs = meta + .versions + .iter() + .map(|(version, manifest)| (version.clone(), manifest.dist.tarball.clone())) + .collect(); crate::lookup_context::AbbreviatedMetaProjection { modified: meta.modified.clone(), - version_names: Some(meta.versions.keys().cloned().collect()), + version_tarballs: Some(version_tarballs), + } +} + +fn same_tarball_url(left: &str, right: &str) -> bool { + canonical_tarball_url(left) == canonical_tarball_url(right) +} + +/// Mirror upstream's `canonicalTarballUrl`: parse-and-reserialize to drop +/// default ports (`:443`/`:80`, what pnpm's `normalizeRegistryUrl` does via +/// `new URL(...).toString()`), decode the `%2f` scoped-name separator, then +/// ignore the scheme — so a benign http/https, default-port, or encoding +/// difference between the lockfile URL and the registry metadata isn't read +/// as tampering. +fn canonical_tarball_url(url: &str) -> String { + let normalized = reqwest::Url::parse(url) + .map_or_else(|_error| url.to_string(), |parsed| parsed.to_string()) + // `%2f` may survive re-serialization in either case; normalize both. + .replace("%2F", "/") + .replace("%2f", "/"); + match normalized.split_once("://") { + Some((_scheme, rest)) => rest.to_string(), + None => normalized, } } diff --git a/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs b/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs index 17016a160b..ebb41589ee 100644 --- a/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs +++ b/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs @@ -156,55 +156,79 @@ fn ctx<'a>(name: &'a PkgName, version: &'a str) -> VerifyCtx<'a> { VerifyCtx { name, version } } -/// `create_npm_resolution_verifier` returns `None` when neither -/// the maturity nor trust check is active — the install side then -/// skips fan-out entirely. -#[test] -fn returns_none_when_no_policy_active() { - let server_url = "https://registry.example/"; - let opts = default_opts(server_url); - assert!(create_npm_resolution_verifier(opts).is_none()); +/// The tarball-URL binding is unconditional: even with no +/// minimumReleaseAge / trustPolicy configured, an entry whose pinned +/// tarball URL doesn't match the registry metadata is rejected. +#[tokio::test] +async fn verifies_tarball_url_when_no_policy_active() { + let mut server = mockito::Server::new_async().await; + let registry = format!("{}/", server.url()); + let server_url = server.url(); + let packument = serde_json::json!({ + "name": "aged-pkg", + "dist-tags": { "latest": "1.0.0" }, + "time": { "1.0.0": "2020-01-01T00:00:00.000Z" }, + "versions": { + "1.0.0": { + "name": "aged-pkg", + "version": "1.0.0", + "dist": { + "integrity": FAKE_INTEGRITY, + "shasum": "0000000000000000000000000000000000000000", + "tarball": format!("{server_url}/aged-pkg/-/aged-pkg-1.0.0.tgz"), + } + } + } + }); + let _meta_mock = server + .mock("GET", "/aged-pkg") + .with_status(200) + .with_body(packument.to_string()) + .create_async() + .await; + // No minimumReleaseAge, no trustPolicy. + let opts = default_opts(®istry); + let verifier = create_npm_resolution_verifier(opts); + let resolution = LockfileResolution::Tarball(TarballResolution { + tarball: "https://attacker.example/aged-pkg-1.0.0.tgz".to_string(), + integrity: Some(fake_integrity()), + git_hosted: None, + path: None, + }); + let result = verifier + .verify(&resolution, ctx(&"aged-pkg".parse::().expect("parse"), "1.0.0")) + .await; + let ResolutionVerification::Err { code, .. } = result else { + panic!("expected Err, got {result:?}"); + }; + assert_eq!(code, "TARBALL_URL_MISMATCH"); } -/// `minimum_release_age = 0` is treated the same as `None` — upstream's -/// `Boolean(opts.minimumReleaseAge)` returns `false` for `0`, so the -/// verifier stays inactive. -#[test] -fn returns_none_when_min_age_is_zero() { - let server_url = "https://registry.example/"; - let mut opts = default_opts(server_url); +/// `minimum_release_age = 0` keeps the age check inactive. The bogus +/// registry URL is a tripwire: a fetch would fail, so the `Ok` result +/// proves the verifier never attempted an age lookup. +#[tokio::test] +async fn min_age_zero_keeps_age_check_inactive() { + let mut opts = default_opts("http://nonexistent.example.invalid/"); opts.minimum_release_age = Some(0); - assert!(create_npm_resolution_verifier(opts).is_none()); + let verifier = create_npm_resolution_verifier(opts); + let result = verifier + .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.0.0")) + .await; + assert_eq!(result, ResolutionVerification::Ok); } -/// `trust_policy = Off` keeps the verifier inactive even when set -/// explicitly. Mirrors upstream's `opts.trustPolicy === 'no-downgrade'` -/// gating. -#[test] -fn returns_none_when_trust_policy_off() { - let server_url = "https://registry.example/"; - let mut opts = default_opts(server_url); +/// `trust_policy = Off` keeps the trust check inactive (same tripwire +/// rationale as the age-check test above). +#[tokio::test] +async fn trust_off_keeps_trust_check_inactive() { + let mut opts = default_opts("http://nonexistent.example.invalid/"); opts.trust_policy = Some(TrustPolicy::Off); - assert!(create_npm_resolution_verifier(opts).is_none()); -} - -/// Setting `minimum_release_age` returns an active verifier even -/// without a trust policy. -#[test] -fn returns_some_when_min_age_set() { - let server_url = "https://registry.example/"; - let mut opts = default_opts(server_url); - opts.minimum_release_age = Some(60 * 24); - assert!(create_npm_resolution_verifier(opts).is_some()); -} - -/// `trust_policy = NoDowngrade` alone activates the verifier. -#[test] -fn returns_some_when_trust_policy_no_downgrade() { - let server_url = "https://registry.example/"; - let mut opts = default_opts(server_url); - opts.trust_policy = Some(TrustPolicy::NoDowngrade); - assert!(create_npm_resolution_verifier(opts).is_some()); + let verifier = create_npm_resolution_verifier(opts); + let result = verifier + .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.0.0")) + .await; + assert_eq!(result, ResolutionVerification::Ok); } /// A git / directory / binary resolution short-circuits to @@ -214,7 +238,7 @@ fn returns_some_when_trust_policy_no_downgrade() { async fn verify_short_circuits_non_registry_resolution() { let mut opts = default_opts("https://registry.example/"); opts.minimum_release_age = Some(60 * 24 * 365); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let directory = LockfileResolution::Directory(pacquet_lockfile::DirectoryResolution { directory: "/some/path".into(), }); @@ -230,7 +254,7 @@ async fn verify_short_circuits_non_registry_resolution() { async fn verify_short_circuits_non_semver_version() { let mut opts = default_opts("https://registry.example/"); opts.minimum_release_age = Some(60 * 24 * 365); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let resolution = registry_resolution(); let name: PkgName = "acme".parse().expect("parse"); let result = verifier.verify(&resolution, ctx(&name, "not-semver")).await; @@ -243,7 +267,7 @@ async fn verify_short_circuits_non_semver_version() { async fn verify_short_circuits_file_tarball_resolution() { let mut opts = default_opts("http://nonexistent.example.invalid/"); opts.minimum_release_age = Some(60 * 24 * 365); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let resolution = LockfileResolution::Tarball(TarballResolution { tarball: "file:vendor/types__my-cool-lib-v1.0.0.tgz".to_string(), integrity: Some(fake_integrity()), @@ -255,6 +279,108 @@ async fn verify_short_circuits_file_tarball_resolution() { assert_eq!(result, ResolutionVerification::Ok); } +/// A registry entry whose pinned tarball URL is not the artifact the +/// registry's metadata lists is rejected before the age check passes it. +/// Guards against a tampered lockfile pairing an aged, trusted +/// name@version with attacker-hosted bytes. +#[tokio::test] +async fn verify_flags_tarball_url_mismatch() { + let mut server = mockito::Server::new_async().await; + let registry = format!("{}/", server.url()); + let server_url = server.url(); + let packument = serde_json::json!({ + "name": "aged-pkg", + "dist-tags": { "latest": "1.0.0" }, + "time": { "1.0.0": "2020-01-01T00:00:00.000Z" }, + "versions": { + "1.0.0": { + "name": "aged-pkg", + "version": "1.0.0", + "dist": { + "integrity": FAKE_INTEGRITY, + "shasum": "0000000000000000000000000000000000000000", + "tarball": format!("{server_url}/aged-pkg/-/aged-pkg-1.0.0.tgz"), + } + } + } + }); + let _meta_mock = server + .mock("GET", "/aged-pkg") + .with_status(200) + .with_body(packument.to_string()) + .create_async() + .await; + let mut opts = default_opts(®istry); + opts.minimum_release_age = Some(60 * 24); + opts.now = Some(now_at("2025-12-01T00:00:00Z")); + let verifier = create_npm_resolution_verifier(opts); + let resolution = LockfileResolution::Tarball(TarballResolution { + tarball: "https://attacker.example/aged-pkg-1.0.0.tgz".to_string(), + integrity: Some(fake_integrity()), + git_hosted: None, + path: None, + }); + let result = verifier + .verify(&resolution, ctx(&"aged-pkg".parse::().expect("parse"), "1.0.0")) + .await; + let ResolutionVerification::Err { code, reason } = result else { + panic!("expected Err, got {result:?}"); + }; + assert_eq!(code, "TARBALL_URL_MISMATCH"); + assert!( + reason.contains("does not match the registry's published metadata"), + "got reason: {reason}", + ); +} + +/// A lockfile URL that differs from the registry metadata only by an +/// explicit default port and the http/https scheme is a benign +/// normalization, not tampering — `same_tarball_url` must canonicalize +/// it away (this is what `canonical_tarball_url`'s URL parse buys over a +/// plain string compare). +#[tokio::test] +async fn tarball_url_default_port_and_scheme_difference_is_a_match() { + let mut server = mockito::Server::new_async().await; + let registry = format!("{}/", server.url()); + // The served metadata lists the artifact on a different host with an + // explicit default port and the http scheme; the lockfile pins the + // canonical https/no-port form of the same URL. + let packument = serde_json::json!({ + "name": "aged-pkg", + "dist-tags": { "latest": "1.0.0" }, + "time": { "1.0.0": "2020-01-01T00:00:00.000Z" }, + "versions": { + "1.0.0": { + "name": "aged-pkg", + "version": "1.0.0", + "dist": { + "integrity": FAKE_INTEGRITY, + "shasum": "0000000000000000000000000000000000000000", + "tarball": "http://registry.npmjs.org:80/aged-pkg/-/aged-pkg-1.0.0.tgz", + } + } + } + }); + let _meta_mock = server + .mock("GET", "/aged-pkg") + .with_status(200) + .with_body(packument.to_string()) + .create_async() + .await; + let opts = default_opts(®istry); + let verifier = create_npm_resolution_verifier(opts); + let resolution = LockfileResolution::Tarball(TarballResolution { + tarball: "https://registry.npmjs.org/aged-pkg/-/aged-pkg-1.0.0.tgz".to_string(), + integrity: Some(fake_integrity()), + git_hosted: None, + path: None, + }); + let result = verifier + .verify(&resolution, ctx(&"aged-pkg".parse::().expect("parse"), "1.0.0")) + .await; + assert_eq!(result, ResolutionVerification::Ok); +} + /// When the exclude policy covers the package, age check skips — /// the version is treated as opted out regardless of its publish /// timestamp. @@ -267,7 +393,7 @@ async fn verify_skips_age_check_when_package_excluded() { opts.minimum_release_age_exclude = Some(create_package_version_policy(["acme".to_string()]).expect("policy")); opts.minimum_release_age_exclude_patterns = vec!["acme".to_string()]; - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let resolution = registry_resolution(); let name: PkgName = "acme".parse().expect("parse"); let result = verifier.verify(&resolution, ctx(&name, "1.0.0")).await; @@ -299,7 +425,7 @@ async fn min_age_pass_when_published_before_cutoff() { let mut opts = default_opts(®istry); opts.minimum_release_age = Some(60 * 24); // 1 day opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let result = verifier .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.0.0")) .await; @@ -328,7 +454,7 @@ async fn min_age_fail_when_published_within_cutoff() { let mut opts = default_opts(®istry); opts.minimum_release_age = Some(60 * 24); // 1 day opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let result = verifier .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.0.0")) .await; @@ -376,7 +502,7 @@ async fn min_age_missing_time_fails_closed_by_default() { let mut opts = default_opts(®istry); opts.minimum_release_age = Some(60 * 24); opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let result = verifier .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.0.0")) .await; @@ -429,7 +555,7 @@ async fn min_age_missing_time_passes_when_ignored() { opts.minimum_release_age = Some(60 * 24); opts.ignore_missing_time_field = true; opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let result = verifier .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.0.0")) .await; @@ -454,7 +580,7 @@ async fn trust_downgrade_publisher_to_provenance_fails() { let mut opts = default_opts(®istry); opts.trust_policy = Some(TrustPolicy::NoDowngrade); opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let result = verifier .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.1.0")) .await; @@ -481,7 +607,7 @@ async fn trust_downgrade_pass_when_no_weaker_evidence() { let mut opts = default_opts(®istry); opts.trust_policy = Some(TrustPolicy::NoDowngrade); opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let result = verifier .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.1.0")) .await; @@ -501,10 +627,29 @@ async fn verify_routes_via_named_registry_prefix() { .expect(1) .create_async() .await; + // The packument lists the same tarball URL the lockfile pins, so the + // tarball-URL binding passes and the test stays focused on registry + // routing. + let packument = serde_json::json!({ + "name": "acme", + "dist-tags": { "latest": "1.0.0" }, + "time": { "1.0.0": "2024-01-01T00:00:00.000Z" }, + "versions": { + "1.0.0": { + "name": "acme", + "version": "1.0.0", + "dist": { + "integrity": FAKE_INTEGRITY, + "shasum": "0000000000000000000000000000000000000000", + "tarball": format!("{server_url}/acme/-/acme-1.0.0.tgz"), + } + } + } + }); let _full_mock = server .mock("GET", "/acme") .with_status(200) - .with_body(min_age_packument_json("acme", "1.0.0", "2024-01-01T00:00:00.000Z").to_string()) + .with_body(packument.to_string()) .expect(1) .create_async() .await; @@ -518,7 +663,7 @@ async fn verify_routes_via_named_registry_prefix() { opts.named_registries = named; opts.minimum_release_age = Some(60 * 24); opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let tarball = LockfileResolution::Tarball(TarballResolution { tarball: format!("{server_url}/acme/-/acme-1.0.0.tgz"), integrity: Some(fake_integrity()), @@ -547,7 +692,7 @@ fn policy_snapshot_records_all_fields_sorted_and_deduped() { opts.trust_policy_exclude = Some(create_package_version_policy(["@scope/foo".to_string()]).expect("policy")); opts.trust_policy_ignore_after = Some(60 * 24 * 30); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let policy = verifier.policy(); assert_eq!(policy.get("minimumReleaseAge").and_then(|value| value.as_u64()), Some(60 * 24)); @@ -575,9 +720,10 @@ fn policy_snapshot_records_all_fields_sorted_and_deduped() { fn can_trust_past_check_accepts_looser_min_age() { let mut opts = default_opts("https://registry.example/"); opts.minimum_release_age = Some(60 * 24); // today: 1 day - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let mut cached = serde_json::Map::new(); + cached.insert("tarballUrlBinding".to_string(), true.into()); cached.insert("minimumReleaseAge".to_string(), (60 * 24 * 7).into()); // past: 7 days cached.insert("minimumReleaseAgeExclude".to_string(), serde_json::Value::Array(vec![])); cached.insert("trustPolicy".to_string(), serde_json::Value::Null); @@ -586,6 +732,25 @@ fn can_trust_past_check_accepts_looser_min_age() { assert!(verifier.can_trust_past_check(&cached)); } +/// A cache record that predates the tarball-URL binding rule (no +/// `tarballUrlBinding` marker) can't be trusted to have enforced it, +/// so it's rejected and forces a re-verification. +#[test] +fn can_trust_past_check_rejects_missing_tarball_url_binding() { + let mut opts = default_opts("https://registry.example/"); + opts.minimum_release_age = Some(60 * 24); + let verifier = create_npm_resolution_verifier(opts); + + // Otherwise-compatible cached policy, but without the binding marker. + let mut cached = serde_json::Map::new(); + cached.insert("minimumReleaseAge".to_string(), (60 * 24 * 7).into()); + cached.insert("minimumReleaseAgeExclude".to_string(), serde_json::Value::Array(vec![])); + cached.insert("trustPolicy".to_string(), serde_json::Value::Null); + cached.insert("trustPolicyExclude".to_string(), serde_json::Value::Array(vec![])); + cached.insert("trustPolicyIgnoreAfter".to_string(), serde_json::Value::Null); + assert!(!verifier.can_trust_past_check(&cached)); +} + /// Tightening the cutoff invalidates the cached run — versions /// that passed under a looser cutoff may now be in the new /// (narrower) window. @@ -593,9 +758,10 @@ fn can_trust_past_check_accepts_looser_min_age() { fn can_trust_past_check_rejects_tighter_min_age() { let mut opts = default_opts("https://registry.example/"); opts.minimum_release_age = Some(60 * 24 * 7); // today: 7 days - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let mut cached = serde_json::Map::new(); + cached.insert("tarballUrlBinding".to_string(), true.into()); cached.insert("minimumReleaseAge".to_string(), (60 * 24).into()); // past: 1 day cached.insert("minimumReleaseAgeExclude".to_string(), serde_json::Value::Array(vec![])); cached.insert("trustPolicy".to_string(), serde_json::Value::Null); @@ -614,9 +780,10 @@ fn can_trust_past_check_rejects_changed_exclude_list() { opts.minimum_release_age_exclude_patterns = vec!["acme".to_string()]; opts.minimum_release_age_exclude = Some(create_package_version_policy(["acme".to_string()]).expect("policy")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let mut cached = serde_json::Map::new(); + cached.insert("tarballUrlBinding".to_string(), true.into()); cached.insert("minimumReleaseAge".to_string(), (60 * 24).into()); cached.insert("minimumReleaseAgeExclude".to_string(), serde_json::Value::Array(vec![])); cached.insert("trustPolicy".to_string(), serde_json::Value::Null); @@ -630,9 +797,10 @@ fn can_trust_past_check_rejects_changed_exclude_list() { fn can_trust_past_check_rejects_changed_trust_policy() { let mut opts = default_opts("https://registry.example/"); opts.trust_policy = Some(TrustPolicy::NoDowngrade); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let mut cached = serde_json::Map::new(); + cached.insert("tarballUrlBinding".to_string(), true.into()); cached.insert("minimumReleaseAge".to_string(), 0.into()); cached.insert("minimumReleaseAgeExclude".to_string(), serde_json::Value::Array(vec![])); cached.insert("trustPolicy".to_string(), serde_json::Value::Null); @@ -648,9 +816,10 @@ fn can_trust_past_check_rejects_changed_ignore_after() { let mut opts = default_opts("https://registry.example/"); opts.trust_policy = Some(TrustPolicy::NoDowngrade); opts.trust_policy_ignore_after = Some(60 * 24 * 14); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let mut cached = serde_json::Map::new(); + cached.insert("tarballUrlBinding".to_string(), true.into()); cached.insert("minimumReleaseAge".to_string(), 0.into()); cached.insert("minimumReleaseAgeExclude".to_string(), serde_json::Value::Array(vec![])); cached.insert("trustPolicy".to_string(), serde_json::Value::String("no-downgrade".into())); @@ -710,7 +879,7 @@ async fn min_age_pass_via_abbreviated_modified_shortcut() { let mut opts = default_opts(®istry); opts.minimum_release_age = Some(60 * 24); // 1 day opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let result = verifier .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.0.0")) .await; @@ -761,7 +930,7 @@ async fn min_age_shortcut_falls_through_when_modified_within_cutoff() { let mut opts = default_opts(®istry); opts.minimum_release_age = Some(60 * 24); // 1 day opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let result = verifier .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "1.0.0")) .await; @@ -816,7 +985,7 @@ async fn min_age_shortcut_falls_through_when_version_not_listed() { let mut opts = default_opts(®istry); opts.minimum_release_age = Some(60 * 24); opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let result = verifier .verify(®istry_resolution(), ctx(&"acme".parse::().expect("parse"), "2.0.0")) .await; @@ -852,7 +1021,7 @@ async fn concurrent_verifications_share_one_fetch() { let mut opts = default_opts(®istry); opts.minimum_release_age = Some(60 * 24); // 1 day opts.now = Some(now_at("2025-12-01T00:00:00Z")); - let verifier = create_npm_resolution_verifier(opts).expect("verifier"); + let verifier = create_npm_resolution_verifier(opts); let name: PkgName = "acme".parse().expect("parse"); let resolution = registry_resolution(); let results = futures_util::future::join_all( diff --git a/pacquet/crates/resolving-npm-resolver/src/lookup_context.rs b/pacquet/crates/resolving-npm-resolver/src/lookup_context.rs index 32f6b3ab8e..487111471e 100644 --- a/pacquet/crates/resolving-npm-resolver/src/lookup_context.rs +++ b/pacquet/crates/resolving-npm-resolver/src/lookup_context.rs @@ -15,10 +15,7 @@ //! the outer mutex is dropped before the await so unrelated keys stay //! unblocked. //! -use std::{ - collections::{HashMap, HashSet}, - sync::Arc, -}; +use std::{collections::HashMap, sync::Arc}; use pacquet_registry::Package; use tokio::sync::{Mutex, OnceCell}; @@ -30,20 +27,24 @@ use tokio::sync::{Mutex, OnceCell}; /// is irrelevant to the policy. pub(crate) type PublishedAtTimeMap = HashMap; -/// Two fields the abbreviated-modified shortcut needs from the -/// packument: the package-level last-modified timestamp and the set -/// of version names the registry currently lists. Projected off the -/// abbreviated packument so the verifier can keep the rest of the -/// document GC-able after the lookup — the full document runs to -/// hundreds of KB per package and OOMs CI runners on multi-thousand -/// entry installs (see [`#11860`](https://github.com/pnpm/pnpm/issues/11860)). +/// The fields the verifier needs from the packument: the +/// package-level last-modified timestamp and a per-version map of +/// `dist.tarball`. The map's keys double as the set of version names +/// the abbreviated-modified shortcut checks; the values feed the +/// tarball-URL binding. Projected off the abbreviated packument so the +/// verifier can keep the rest of the document GC-able after the +/// lookup — the full document runs to hundreds of KB per package and +/// OOMs CI runners on multi-thousand entry installs (see +/// [`#11860`](https://github.com/pnpm/pnpm/issues/11860)); only the +/// short tarball-URL strings are retained. /// /// Mirrors upstream's /// [`AbbreviatedMetaProjection`](https://github.com/pnpm/pnpm/blob/2a9bd897bf/resolving/npm-resolver/src/createNpmResolutionVerifier.ts#L709-L712). #[derive(Debug, Default, Clone)] pub(crate) struct AbbreviatedMetaProjection { pub modified: Option, - pub version_names: Option>, + /// version → `dist.tarball`; key presence means the version is published. + pub version_tarballs: Option>, } /// Slot map of singleflight cells. Outer mutex guards lookup/insert; diff --git a/pacquet/crates/resolving-npm-resolver/src/violation_codes.rs b/pacquet/crates/resolving-npm-resolver/src/violation_codes.rs index 11ef432bfc..52952e0ba4 100644 --- a/pacquet/crates/resolving-npm-resolver/src/violation_codes.rs +++ b/pacquet/crates/resolving-npm-resolver/src/violation_codes.rs @@ -9,3 +9,4 @@ pub const MINIMUM_RELEASE_AGE_VIOLATION_CODE: &str = "MINIMUM_RELEASE_AGE_VIOLATION"; pub const TRUST_DOWNGRADE_VIOLATION_CODE: &str = "TRUST_DOWNGRADE"; +pub const TARBALL_URL_MISMATCH_VIOLATION_CODE: &str = "TARBALL_URL_MISMATCH"; diff --git a/resolving/default-resolver/src/index.ts b/resolving/default-resolver/src/index.ts index e7d592ec62..f023ab97ac 100644 --- a/resolving/default-resolver/src/index.ts +++ b/resolving/default-resolver/src/index.ts @@ -195,9 +195,9 @@ export type ResolutionVerifierFactoryOptions = /** * Companion to {@link createResolver}. Collects the resolver-specific - * verifier factories (today: npm) into a list. Returns an empty array - * when no policy is active — callers can cheaply decide whether to - * iterate at all by checking `verifiers.length`. + * verifier factories (today: npm) into a list. The npm verifier is + * always present — it enforces the tarball-URL binding regardless of + * policy configuration — so the list is non-empty. * * Future protocols (jsr, git, attestation, etc.) plug in here by pushing * their own `ResolutionVerifier` onto the list. Each verifier handles @@ -233,6 +233,6 @@ export function createResolutionVerifiers ( metaCache: opts.metaCache, now: opts.now, }) - if (npmVerifier) verifiers.push(npmVerifier) + verifiers.push(npmVerifier) return verifiers } diff --git a/resolving/npm-resolver/src/createNpmResolutionVerifier.ts b/resolving/npm-resolver/src/createNpmResolutionVerifier.ts index fa619ea695..615fb66e86 100644 --- a/resolving/npm-resolver/src/createNpmResolutionVerifier.ts +++ b/resolving/npm-resolver/src/createNpmResolutionVerifier.ts @@ -17,12 +17,14 @@ import { fetchFullMetadataCached, type FetchFullMetadataCachedOptions, } from './fetchFullMetadataCached.js' +import { normalizeRegistryUrl } from './normalizeRegistryUrl.js' import { BUILTIN_NAMED_REGISTRIES } from './parseBareSpecifier.js' import type { PackageMetaCache } from './pickPackage.js' import { getPkgMirrorPath, loadMeta, warnMissingTimeFieldOnce } from './pickPackage.js' import { failIfTrustDowngraded } from './trustChecks.js' import { MINIMUM_RELEASE_AGE_VIOLATION_CODE, + TARBALL_URL_MISMATCH_VIOLATION_CODE, TRUST_DOWNGRADE_VIOLATION_CODE, } from './violationCodes.js' @@ -95,11 +97,14 @@ export interface CreateNpmResolutionVerifierOptions { } /** - * Returns a `ResolutionVerifier` that re-applies the `minimumReleaseAge` - * and/or `trustPolicy='no-downgrade'` policies to npm-registry-resolved - * lockfile entries, or `undefined` when no policy is active. Pairs with - * `createNpmResolver`: each resolver factory may export a sibling - * verifier factory that the default-resolver combines. + * Returns a `ResolutionVerifier` for npm-registry-resolved lockfile + * entries. It always binds each entry's recorded tarball URL to the + * artifact the registry's metadata lists (an anti-tamper check that does + * not depend on any policy), and additionally re-applies the + * `minimumReleaseAge` and/or `trustPolicy='no-downgrade'` policies when + * those are configured. Pairs with `createNpmResolver`: each resolver + * factory may export a sibling verifier factory that the default-resolver + * combines. * * Designed for fail-closed semantics: if the manifest can't be loaded or * the pinned version is missing from it, the verifier reports a violation @@ -108,12 +113,9 @@ export interface CreateNpmResolutionVerifierOptions { */ export function createNpmResolutionVerifier ( opts: CreateNpmResolutionVerifierOptions -): ResolutionVerifier | undefined { +): ResolutionVerifier { const ageCheckActive = Boolean(opts.minimumReleaseAge) const trustCheckActive = opts.trustPolicy === 'no-downgrade' - // No policy → no verifier. Skipping early keeps the install-side fan-out - // empty when nothing is configured. - if (!ageCheckActive && !trustCheckActive) return undefined const cutoff = ageCheckActive ? (opts.now ?? Date.now()) - opts.minimumReleaseAge! * 60 * 1000 @@ -174,20 +176,36 @@ export function createNpmResolutionVerifier ( const trustPolicy = opts.trustPolicy const trustPolicyIgnoreAfter = opts.trustPolicyIgnoreAfter - const verify: ResolutionVerifier['verify'] = async (resolution, { name, version }) => { + const verify: ResolutionVerifier['verify'] = async (resolution, { name, version, nonSemverVersion }) => { if (!isNpmRegistryResolution(resolution)) return { ok: true } - // Non-semver versions identify URL tarballs, file: refs, git refs, etc. - // Neither the age nor the trust policy applies, and a registry lookup + // URL/git-keyed entries are deliberate non-registry deps. They can still + // carry a semver `version` copied from the resolved manifest, so the + // semver guard below isn't enough on its own — the registry policies and + // the tarball-URL binding don't apply to them, and a registry lookup // would 404. + if (nonSemverVersion != null) return { ok: true } if (!semver.valid(version)) return { ok: true } + const tarballUrl = (resolution as { tarball?: string }).tarball + const registry = pickRegistryForVersion(opts.registries, namedRegistryPrefixes, name, tarballUrl) + + // A registry entry that pins an explicit tarball URL must point at the + // artifact the registry's own metadata lists. Otherwise a trusted + // `name@version` could front bytes from an attacker-chosen URL (with a + // matching integrity for those bytes). This binding is unconditional — + // it does not depend on `minimumReleaseAge`/`trustPolicy` and isn't + // narrowed by their exclude lists, since it guards integrity rather + // than maturity/trust. Registry entries with no tarball URL reconstruct + // it from name+version+registry, so they're inherently bound. + if (typeof tarballUrl === 'string') { + const urlViolation = await runTarballUrlCheck(lookupContext, registry, name, version, tarballUrl) + if (urlViolation) return urlViolation + } + const ageApplies = ageCheckActive && !isExcluded(excludePolicy, name, version) const trustApplies = trustCheckActive && !isExcluded(trustExcludePolicy, name, version) if (!ageApplies && !trustApplies) return { ok: true } - const tarballUrl = (resolution as { tarball?: string }).tarball - const registry = pickRegistryForVersion(opts.registries, namedRegistryPrefixes, name, tarballUrl) - if (ageApplies) { const ageViolation = await runAgeCheck(lookupContext, registry, name, version, cutoff, opts.ignoreMissingTimeField === true) if (ageViolation) return ageViolation @@ -217,6 +235,12 @@ export function createNpmResolutionVerifier ( return { verify, policy: { + // Marks runs that enforced the tarball-URL binding. A cache record + // written before this rule existed lacks the flag, so + // `canTrustPastCheck` rejects it and forces a re-verification that + // applies the binding — otherwise an upgrade could keep trusting a + // lockfile that was only ever age/trust-checked. + tarballUrlBinding: true, minimumReleaseAge, minimumReleaseAgeExclude: sortedMinAgeExcludes, trustPolicy: trustPolicy ?? null, @@ -224,6 +248,10 @@ export function createNpmResolutionVerifier ( trustPolicyIgnoreAfter: trustPolicyIgnoreAfter ?? null, }, canTrustPastCheck: (cached) => { + // The tarball-URL binding is unconditional today; a cached run that + // didn't record it can't be trusted to have enforced it. + if (cached.tarballUrlBinding !== true) return false + // Maturity: a previously cached run under a larger cutoff // (stricter window) is trustworthy under a smaller current one — // its set of accepted versions is a subset of today's. The @@ -319,6 +347,53 @@ async function runAgeCheck ( return undefined } +/** + * Confirm the lockfile-pinned tarball URL is the artifact the registry's + * own metadata lists for this exact `name@version`. + * + * Fail-closed: the entry passes only when the registry metadata + * affirmatively lists this version with a matching tarball URL. If the + * metadata can't be fetched, doesn't list the version, or omits + * `dist.tarball`, the entry can't be confirmed and is rejected — otherwise + * a tampered lockfile could smuggle a malicious URL past the check by + * pointing it at a `name@version` the registry can't vouch for. + */ +async function runTarballUrlCheck ( + context: PublishedAtLookupContext, + registry: string, + name: string, + version: string, + lockfileTarball: string +): Promise<{ ok: false, code: string, reason: string } | undefined> { + const meta = await fetchAbbreviatedMeta(context, registry, name) + const registryTarball = meta?.versionTarballs?.get(version) + if (registryTarball != null && sameTarballUrl(lockfileTarball, registryTarball)) { + return undefined + } + return { + ok: false, + code: TARBALL_URL_MISMATCH_VIOLATION_CODE, + reason: registryTarball == null + ? "could not be verified against the registry's published metadata" + : `has a tarball URL (${lockfileTarball}) that does not match the registry's published metadata (${registryTarball})`, + } +} + +function sameTarballUrl (a: string, b: string): boolean { + return canonicalTarballUrl(a) === canonicalTarballUrl(b) +} + +// Mirror the tolerance toLockfileResolution applies when it decides whether +// a tarball URL is "the expected one": ignore the protocol and `%2f` scope +// encoding so a benign http/https or encoding difference isn't read as +// tampering. The `%2f` match is case-insensitive because `normalizeRegistryUrl` +// (`new URL().toString()`) can upper-case percent-escapes to `%2F`. +function canonicalTarballUrl (url: string): string { + const normalized = normalizeRegistryUrl(url).replace(/%2f/gi, '/') + const schemeEnd = normalized.indexOf('://') + return schemeEnd === -1 ? normalized : normalized.slice(schemeEnd + 3) +} + /** * Run the resolver-time `failIfTrustDowngraded` check against the * pinned lockfile version. The packument is fetched through a @@ -630,7 +705,7 @@ async function tryAbbreviatedModifiedShortcut ( // publish time — but only for versions the registry currently lists. // An unpublished or never-published pin would otherwise pass the gate // on a stale package-level timestamp. - if (!meta?.versionNames?.has(version)) return undefined + if (!meta?.versionTarballs?.has(version)) return undefined return modified } @@ -702,24 +777,33 @@ function validateSharedMeta (meta: PackageMeta | undefined, name: string): Packa return meta } -// Project the abbreviated packument down to the two fields the verifier -// actually reads — package-level `modified` and the set of version names -// for the existence check inside `tryAbbreviatedModifiedShortcut`. The +// Project the abbreviated packument down to the few fields the verifier +// actually reads — package-level `modified`, plus a per-version map of +// `dist.tarball` (whose keys double as the version-existence set for the +// `tryAbbreviatedModifiedShortcut` check and the tarball-URL binding). The // resolver populates the abbreviated mirror with every version's // dependency / engine / dist info, which can run to hundreds of KB per // package and accumulate to many GB across a multi-thousand-entry // lockfile (see #11860). The full document is GC-able as soon as this -// closure returns. +// closure returns; only the short tarball-URL strings are retained. function projectAbbreviatedMeta (meta: PackageMeta): AbbreviatedMetaProjection { + let versionTarballs: Map | undefined + if (meta.versions) { + versionTarballs = new Map() + for (const [version, manifest] of Object.entries(meta.versions)) { + versionTarballs.set(version, manifest.dist?.tarball) + } + } return { modified: meta.modified, - versionNames: meta.versions ? new Set(Object.keys(meta.versions)) : undefined, + versionTarballs, } } interface AbbreviatedMetaProjection { modified?: string - versionNames?: Set + /** version → `dist.tarball`; key presence means the version is published. */ + versionTarballs?: Map } function readLocalMetaTime ( @@ -777,11 +861,13 @@ function pickRegistryForVersion ( // origin we'd otherwise miss. Match the longest prefix so that two named // registries sharing a host but differing by path don't collide. if (tarballUrl) { - const normalized = tryParseUrl(tarballUrl)?.toString() - if (normalized) { - for (const prefix of namedRegistryPrefixes) { - if (normalized.startsWith(prefix)) return prefix - } + // Match on the same canonical form the tarball comparison uses, so a + // named-registry tarball that differs from the configured base only by + // scheme or `%2f` encoding still routes to its registry instead of + // falling back (and then failing closed against the wrong packument). + const normalized = canonicalTarballUrl(tarballUrl) + for (const prefix of namedRegistryPrefixes) { + if (normalized.startsWith(canonicalTarballUrl(prefix))) return prefix } } return pickRegistryForPackage(registries, name) diff --git a/resolving/npm-resolver/src/violationCodes.ts b/resolving/npm-resolver/src/violationCodes.ts index f86b6d9c41..e6aeb59342 100644 --- a/resolving/npm-resolver/src/violationCodes.ts +++ b/resolving/npm-resolver/src/violationCodes.ts @@ -10,3 +10,4 @@ */ export const MINIMUM_RELEASE_AGE_VIOLATION_CODE = 'MINIMUM_RELEASE_AGE_VIOLATION' export const TRUST_DOWNGRADE_VIOLATION_CODE = 'TRUST_DOWNGRADE' +export const TARBALL_URL_MISMATCH_VIOLATION_CODE = 'TARBALL_URL_MISMATCH' diff --git a/resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts b/resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts index 32c007899a..d6a6c3db34 100644 --- a/resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts +++ b/resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts @@ -45,8 +45,34 @@ beforeEach(async () => { await setupMockAgent() }) -test('createNpmResolutionVerifier() returns undefined when no policy is active', () => { - expect(createNpmResolutionVerifier(makeVerifierOpts())).toBeUndefined() +test('createNpmResolutionVerifier() still verifies tarball URLs when no age/trust policy is active', async () => { + // The tarball-URL binding is unconditional, so the verifier exists even + // with no minimumReleaseAge/trustPolicy configured. + const meta = { + name: 'aged-pkg', + 'dist-tags': { latest: '1.0.0' }, + versions: { + '1.0.0': { + name: 'aged-pkg', + version: '1.0.0', + dist: { tarball: 'https://registry.npmjs.org/aged-pkg/-/aged-pkg-1.0.0.tgz', shasum: 'aa' }, + }, + }, + modified: '2020-01-01T00:00:00.000Z', + } + const pool = getMockAgent().get('https://registry.npmjs.org') + pool.intercept({ path: '/aged-pkg', method: 'GET' }).reply(200, meta).persist() + + const verifier = createNpmResolutionVerifier(makeVerifierOpts()) + expect(verifier).toBeDefined() + const result = await verifier.verify( + { + integrity: FAKE_INTEGRITY, + tarball: 'https://attacker.example/aged-pkg-1.0.0.tgz', + } as unknown as Resolution, + { name: 'aged-pkg', version: '1.0.0' } + ) + expect(result).toMatchObject({ ok: false, code: 'TARBALL_URL_MISMATCH' }) }) test('createNpmResolutionVerifier() flags a trustedPublisher → provenance downgrade', async () => { @@ -90,7 +116,7 @@ test('createNpmResolutionVerifier() flags a trustedPublisher → provenance down const verifier = createNpmResolutionVerifier(makeVerifierOpts({ trustPolicy: 'no-downgrade', - }))! + })) expect(verifier).toBeDefined() const result = await verifier.verify(makeTarballResolution('demo', '0.0.2'), { name: 'demo', version: '0.0.2' }) @@ -137,7 +163,7 @@ test('createNpmResolutionVerifier() passes a same-evidence-level version', async const verifier = createNpmResolutionVerifier(makeVerifierOpts({ trustPolicy: 'no-downgrade', - }))! + })) const result = await verifier.verify(makeTarballResolution('demo', '0.0.2'), { name: 'demo', version: '0.0.2' }) expect(result).toEqual({ ok: true }) }) @@ -171,9 +197,13 @@ test('createNpmResolutionVerifier() abbreviated shortcut requires the pinned ver const verifier = createNpmResolutionVerifier(makeVerifierOpts({ minimumReleaseAge: 1440, // 1 day - }))! + })) + // Registry-style resolution (no explicit tarball URL) so the entry + // exercises the age check's abbreviated shortcut rather than the + // tarball-URL binding (which would fail closed on the missing version + // first). const result = await verifier.verify( - makeTarballResolution('unpublished-pkg', '0.0.2'), + { integrity: FAKE_INTEGRITY } as unknown as Resolution, { name: 'unpublished-pkg', version: '0.0.2' } ) expect(result).toMatchObject({ @@ -213,7 +243,7 @@ test('createNpmResolutionVerifier() ignoreMissingTimeField passes the entry when const verifier = createNpmResolutionVerifier(makeVerifierOpts({ minimumReleaseAge: 1440, ignoreMissingTimeField: true, - }))! + })) const result = await verifier.verify( makeTarballResolution('time-free-pkg', '1.0.0'), { name: 'time-free-pkg', version: '1.0.0' } @@ -224,7 +254,7 @@ test('createNpmResolutionVerifier() ignoreMissingTimeField passes the entry when test('createNpmResolutionVerifier() skips file: tarball resolutions', async () => { const verifier = createNpmResolutionVerifier(makeVerifierOpts({ minimumReleaseAge: 1440, - }))! + })) const result = await verifier.verify( { integrity: 'sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==', @@ -235,13 +265,137 @@ test('createNpmResolutionVerifier() skips file: tarball resolutions', async () = expect(result).toEqual({ ok: true }) }) +const FAKE_INTEGRITY = 'sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==' + +test('createNpmResolutionVerifier() flags a lockfile tarball URL that does not match the registry metadata', async () => { + // The version is old enough to clear minimumReleaseAge, but the lockfile + // pins a tarball URL on a host the registry never published to. A tampered + // lockfile could pair an aged, trusted name@version with attacker-hosted + // bytes; the verifier must reject the entry before the age check passes it. + const meta = { + name: 'aged-pkg', + 'dist-tags': { latest: '1.0.0' }, + versions: { + '1.0.0': { + name: 'aged-pkg', + version: '1.0.0', + dist: { tarball: 'https://registry.npmjs.org/aged-pkg/-/aged-pkg-1.0.0.tgz', shasum: 'aa' }, + }, + }, + time: { '1.0.0': '2020-01-01T00:00:00.000Z' }, + modified: '2020-01-01T00:00:00.000Z', + } + const pool = getMockAgent().get('https://registry.npmjs.org') + pool.intercept({ path: '/aged-pkg', method: 'GET' }).reply(200, meta).persist() + + const verifier = createNpmResolutionVerifier(makeVerifierOpts({ + minimumReleaseAge: 1440, + })) + const result = await verifier.verify( + { + integrity: FAKE_INTEGRITY, + tarball: 'https://attacker.example/aged-pkg-1.0.0.tgz', + } as unknown as Resolution, + { name: 'aged-pkg', version: '1.0.0' } + ) + expect(result).toMatchObject({ + ok: false, + code: 'TARBALL_URL_MISMATCH', + }) +}) + +test('createNpmResolutionVerifier() accepts a non-standard tarball URL that matches the registry metadata', async () => { + // npm Enterprise / GitHub Packages serve tarballs from a path the default + // URL template can't reconstruct, so the lockfile keeps the URL. As long + // as it's the URL the registry's own metadata lists, it's legitimate. + const meta = { + name: 'enterprise-pkg', + 'dist-tags': { latest: '1.0.0' }, + versions: { + '1.0.0': { + name: 'enterprise-pkg', + version: '1.0.0', + dist: { tarball: 'https://registry.npmjs.org/enterprise-pkg/download/enterprise-pkg-1.0.0.tgz', shasum: 'aa' }, + }, + }, + time: { '1.0.0': '2020-01-01T00:00:00.000Z' }, + modified: '2020-01-01T00:00:00.000Z', + } + const pool = getMockAgent().get('https://registry.npmjs.org') + pool.intercept({ path: '/enterprise-pkg', method: 'GET' }).reply(200, meta).persist() + + const verifier = createNpmResolutionVerifier(makeVerifierOpts({ + minimumReleaseAge: 1440, + })) + const result = await verifier.verify( + { + integrity: FAKE_INTEGRITY, + tarball: 'https://registry.npmjs.org/enterprise-pkg/download/enterprise-pkg-1.0.0.tgz', + } as unknown as Resolution, + { name: 'enterprise-pkg', version: '1.0.0' } + ) + expect(result).toEqual({ ok: true }) +}) + +test('createNpmResolutionVerifier() treats a default-port / scheme difference as a match', async () => { + // The lockfile URL and the registry metadata differ only by an explicit + // default port and the http/https scheme — benign normalizations, not + // tampering — so `sameTarballUrl` must canonicalize them away. + const meta = { + name: 'aged-pkg', + 'dist-tags': { latest: '1.0.0' }, + versions: { + '1.0.0': { + name: 'aged-pkg', + version: '1.0.0', + dist: { tarball: 'http://registry.npmjs.org:80/aged-pkg/-/aged-pkg-1.0.0.tgz', shasum: 'aa' }, + }, + }, + time: { '1.0.0': '2020-01-01T00:00:00.000Z' }, + modified: '2020-01-01T00:00:00.000Z', + } + const pool = getMockAgent().get('https://registry.npmjs.org') + pool.intercept({ path: '/aged-pkg', method: 'GET' }).reply(200, meta).persist() + + const verifier = createNpmResolutionVerifier(makeVerifierOpts({ + minimumReleaseAge: 1440, + })) + const result = await verifier.verify( + { + integrity: FAKE_INTEGRITY, + tarball: 'https://registry.npmjs.org/aged-pkg/-/aged-pkg-1.0.0.tgz', + } as unknown as Resolution, + { name: 'aged-pkg', version: '1.0.0' } + ) + expect(result).toEqual({ ok: true }) +}) + +test('createNpmResolutionVerifier() skips URL-keyed tarball deps even when they carry a semver version', async () => { + // A remote `https:` tarball dependency keeps a semver `version` copied from + // its manifest, but its lockfile key is the URL (nonSemverVersion). It is a + // deliberate non-registry dep: neither the release-age policy nor the + // registry tarball-URL binding applies, and no registry lookup should fire. + const verifier = createNpmResolutionVerifier(makeVerifierOpts({ + minimumReleaseAge: 1440, + })) + const result = await verifier.verify( + { + integrity: FAKE_INTEGRITY, + tarball: 'https://example.com/foo-1.0.0.tgz', + } as unknown as Resolution, + { name: 'foo', version: '1.0.0', nonSemverVersion: 'https://example.com/foo-1.0.0.tgz' } + ) + expect(result).toEqual({ ok: true }) +}) + test('createNpmResolutionVerifier() canTrustPastCheck rejects when the trust-exclude list shrinks', () => { const verifier = createNpmResolutionVerifier(makeVerifierOpts({ trustPolicy: 'no-downgrade', trustPolicyExclude: ['foo'], - }))! + })) // Same policy → trust. expect(verifier.canTrustPastCheck({ + tarballUrlBinding: true, minimumReleaseAge: 0, minimumReleaseAgeExclude: [], trustPolicy: 'no-downgrade', @@ -250,6 +404,7 @@ test('createNpmResolutionVerifier() canTrustPastCheck rejects when the trust-exc })).toBe(true) // Cached run had a wider exclude list (today's is stricter) → invalidate. expect(verifier.canTrustPastCheck({ + tarballUrlBinding: true, minimumReleaseAge: 0, minimumReleaseAgeExclude: [], trustPolicy: 'no-downgrade', diff --git a/resolving/resolver-base/src/index.ts b/resolving/resolver-base/src/index.ts index d93ae3afff..362bc7a8f8 100644 --- a/resolving/resolver-base/src/index.ts +++ b/resolving/resolver-base/src/index.ts @@ -111,7 +111,14 @@ export type ResolutionVerification = * across registries) name it the same and share the cache slot. */ export interface ResolutionVerifier { - verify: (resolution: Resolution, ctx: { name: string, version: string }) => Promise + /** + * `ctx.nonSemverVersion` is set when the lockfile entry is keyed by a + * non-semver reference (URL tarball, git, etc.) rather than a registry + * `name@version`. Verifiers that only police registry entries use it to + * skip deliberate non-registry deps, which can still carry a semver + * `version` copied from the resolved manifest. + */ + verify: (resolution: Resolution, ctx: { name: string, version: string, nonSemverVersion?: string }) => Promise /** * Snapshot of the policy fields this verifier enforces. Merged with * every other active verifier's `policy` into the cache record. A