fix(pacquet): always record a tarball integrity in the lockfile (#13549)

A registry version that carried no `dist.integrity` was recorded in the
lockfile with a bare tarball URL and no hash, and `--lockfile-only` never
computed one — so the very first `pnpm install --frozen-lockfile` over
that lockfile failed with `ERR_PNPM_MISSING_TARBALL_INTEGRITY`. Two
independent gaps produced it, and both are closed here.

First, `dist.shasum` was ignored. pnpm's `getIntegrity` promotes the
legacy hex digest to a `sha1-` SRI string when `dist.integrity` is
absent, which is what pins every version published before subresource
integrity — the whole of `https://node-registry.bit.cloud/`, for one.
pacquet parsed the field and dropped it, leaving those versions unpinned
and diverging from the lockfile the TypeScript CLI writes for the same
graph. `dist_integrity` now mirrors `getIntegrity`, including the
`ERR_PNPM_INVALID_TARBALL_INTEGRITY` failure for a shasum that is not a
hex digest. It also restores the stronger of the two behaviours: a
registry-published hash pins the bytes, where hashing whatever arrived
pins nothing.

Second, `--lockfile-only` skipped the `PrefetchingResolver` wholesale,
and that wrapper owns `populate_missing_integrity` — the only path that
hashes a tarball no registry hash covers. Background prefetching and
integrity completion are now separate concerns: the wrapper always sits
in the resolver chain, and `prefetch_downloads` turns off only the
speculative download. A run that materializes nothing (`--lockfile-only`)
or only part of the graph (a filtered workspace selection) still fetches
the few tarballs whose hash the lockfile cannot be written without, which
is what pnpm does with `resolutionNeedsFetch` under `dryRun`.

Closes pnpm/pnpm#13547
This commit is contained in:
Zoltan Kochan authored and GitHub committed 2026-08-01 12:37:10 +02:00
1 parent a99b8526a0
commit bd26366c79
10 files changed
+366 -38

No files matched your search

@@ -0,0 +1,5 @@
---
"pacquet": patch
---
A registry dependency is now always recorded in `pnpm-lock.yaml` with an integrity hash, including under `--lockfile-only`. Packages from a registry that publishes no subresource-integrity metadata — `https://node-registry.bit.cloud/`, for one — were recorded without one, so the next `pnpm install --frozen-lockfile` failed with `ERR_PNPM_MISSING_TARBALL_INTEGRITY` [#13547](https://github.com/pnpm/pnpm/issues/13547).
+72
View File
@@ -13,6 +13,7 @@ use assert_cmd::prelude::*;
use command_extra::CommandExtra;
use pacquet_testing_utils::{
bin::{AddMockedRegistry, CommandTempCwd},
fixtures::minimal_tarball,
fs::get_all_files,
};
use std::{fs, path::Path, process::Command};
@@ -370,3 +371,74 @@ fn lockfile_only_updates_importers_when_a_project_is_added() {
drop((root, mock_instance));
}
/// A registry version that pins nothing — neither `dist.integrity` nor
/// `dist.shasum` — must be hashed even under `--lockfile-only`, or the
/// lockfile it writes cannot be installed
/// (<https://github.com/pnpm/pnpm/issues/13547>).
#[test]
fn computes_the_integrity_of_an_unpinned_tarball() {
let CommandTempCwd { root, workspace, .. } = CommandTempCwd::init();
let mut registry = mockito::Server::new();
let tarball_path = "/unpinned/-/unpinned-1.0.0.tgz";
let packument = serde_json::json!({
"name": "unpinned",
"dist-tags": { "latest": "1.0.0" },
"modified": "2020-01-15T12:00:00.000Z",
"time": { "1.0.0": "2020-01-10T08:30:00.000Z" },
"versions": {
"1.0.0": {
"name": "unpinned",
"version": "1.0.0",
"dist": { "tarball": format!("{}{tarball_path}", registry.url()) },
},
},
});
let packument_mock =
registry.mock("GET", "/unpinned").with_body(packument.to_string()).create();
let tarball_mock = registry
.mock("GET", tarball_path)
.with_body(minimal_tarball("unpinned", "1.0.0"))
.expect_at_least(1)
.create();
write_registry_workspace(&workspace, &registry.url());
fs::write(
workspace.join("package.json"),
serde_json::json!({ "dependencies": { "unpinned": "1.0.0" } }).to_string(),
)
.expect("write package.json");
pacquet_at(&workspace).with_args(["install", "--lockfile-only"]).assert().success();
let lockfile = fs::read_to_string(workspace.join("pnpm-lock.yaml")).expect("read lockfile");
assert!(
lockfile.contains("resolution: {integrity: sha512-"),
"the unpinned tarball's integrity must be computed and recorded:\n{lockfile}",
);
assert!(
!workspace.join("node_modules").exists(),
"node_modules must not be created by --lockfile-only",
);
packument_mock.assert();
tarball_mock.assert();
pacquet_at(&workspace).with_args(["install", "--frozen-lockfile"]).assert().success();
assert!(
workspace.join("node_modules/unpinned/package.json").exists(),
"the frozen install must materialize the unpinned dependency",
);
drop(root);
}
/// `.npmrc` + `pnpm-workspace.yaml` for a registry the test hosts
/// itself. [`AddMockedRegistry`] writes the same pair, but only ever for
/// the shared pnpr fixture registry.
fn write_registry_workspace(workspace: &Path, registry: &str) {
fs::write(workspace.join(".npmrc"), format!("registry={registry}/\n")).expect("write .npmrc");
fs::write(
workspace.join("pnpm-workspace.yaml"),
"storeDir: ../pacquet-store\ncacheDir: ../pacquet-cache\nenableGlobalVirtualStore: false\n",
)
.expect("write pnpm-workspace.yaml");
}
@@ -1048,37 +1048,28 @@ impl<DependencyGroupList> InstallWithFreshLockfile<'_, DependencyGroupList> {
// download's `pnpm:progress` emits route through the same
// reporter the install pass uses. See
// `prefetching_resolver.rs` for the full design rationale.
//
// Skipped entirely under `--lockfile-only`: that path writes only
// `pnpm-lock.yaml` and must not touch the store, so resolution
// runs through the bare chain with no background download. This is
// the `dryRun: opts.lockfileOnly` path.
let resolver: Box<dyn Resolver> = if lockfile_only || filtered_isolated {
inner_resolver
} else {
Box::new(PrefetchingResolver::<Reporter>::new(
inner_resolver,
PrefetchContext {
http_client: &http_client_arc,
mem_cache: &tarball_mem_cache,
store_index: store_index_ref,
store_index_writer: Some(&store_index_writer),
verified_files_cache: &verified_files_cache,
config,
requester,
supported_architectures,
progress_reported: &progress_reported,
},
))
};
let resolver: Box<dyn Resolver> = Box::new(PrefetchingResolver::<Reporter>::new(
inner_resolver,
PrefetchContext {
http_client: &http_client_arc,
mem_cache: &tarball_mem_cache,
store_index: store_index_ref,
store_index_writer: Some(&store_index_writer),
verified_files_cache: &verified_files_cache,
config,
requester,
supported_architectures,
progress_reported: &progress_reported,
prefetch_downloads: !lockfile_only && !filtered_isolated,
},
));
// The pnpr server resolves `--lockfile-only` and reports each
// resolved tarball to the client as it lands, so the client can
// fetch in parallel with the server's resolution. Wrap the chain
// last so the observer sees every resolve regardless of whether
// the prefetcher above is in play (it isn't under
// `--lockfile-only`, which is the pnpr resolve path). A no-op for
// every local install (`resolution_observer` is `None`).
// last so the observer sees each resolve as the wrapper above
// leaves it, integrity included. A no-op for every local install
// (`resolution_observer` is `None`).
let resolver: Box<dyn Resolver> = match resolution_observer {
Some(observer) => Box::new(crate::ObservingResolver::new(resolver, observer)),
None => resolver,
@@ -1741,11 +1732,10 @@ impl<DependencyGroupList> InstallWithFreshLockfile<'_, DependencyGroupList> {
};
// `--lockfile-only`: the graph is resolved, so build and write
// `pnpm-lock.yaml` and return before any materialization. No
// tarball was prefetched (the resolver ran without the
// `PrefetchingResolver` wrapper), so the store is untouched and
// there is no `node_modules`, `.modules.yaml`, or current
// lockfile — a lockfile-only resolve pass.
// `pnpm-lock.yaml` and return before any materialization. Nothing
// was prefetched, and there is no `node_modules`,
// `.modules.yaml`, or current lockfile — a lockfile-only resolve
// pass.
if lockfile_only {
let freshly_resolved = build_fresh_lockfile(FreshLockfileBuildOptions {
config,
@@ -21,6 +21,11 @@
//! prefetch is already done) or briefly blocks on the `Notify` (the
//! prefetch is still in flight). Errors are surfaced to the install
//! path as `TarballError::SiblingFetchFailed`.
//!
//! That prefetch is speculative, and a run may switch it off
//! ([`PrefetchContext::prefetch_downloads`]). Hashing a resolution
//! that carries no integrity is not speculative — the lockfile records
//! that hash — so it runs either way.
use crate::{
install_package_from_registry::{extract_tarball, manifest_file_count, manifest_unpacked_size},
@@ -70,6 +75,10 @@ pub struct PrefetchContext<'a> {
/// consults the set so prefetch progress is visible immediately
/// without being counted again.
pub progress_reported: &'a SharedReportedProgressKeys,
/// Whether a resolved tarball is prefetched. `false` for a run
/// whose install pass will never ask for those bytes, so the store
/// isn't filled with tarballs nobody installs.
pub prefetch_downloads: bool,
}
/// Owned, `'static`-friendly clones of [`PrefetchContext`] stored on
@@ -109,6 +118,7 @@ struct OwnedFetchCtx {
/// edge downloads and computes the integrity; later edges await the
/// same cell instead of fetching the URL again.
integrity_cache: Arc<DashMap<String, Arc<OnceCell<Integrity>>>>,
prefetch_downloads: bool,
}
/// Wraps an inner [`Resolver`] and, after each successful resolve that
@@ -149,6 +159,7 @@ impl<Reporter: self::Reporter + 'static> PrefetchingResolver<Reporter> {
requester,
supported_architectures,
progress_reported,
prefetch_downloads,
} = prefetch_ctx;
let ctx = OwnedFetchCtx {
http_client: Arc::clone(http_client),
@@ -169,6 +180,7 @@ impl<Reporter: self::Reporter + 'static> PrefetchingResolver<Reporter> {
progress_reported: SharedReportedProgressKeys::clone(progress_reported),
spawned_urls: Arc::new(DashSet::new()),
integrity_cache: Arc::new(DashMap::new()),
prefetch_downloads,
};
PrefetchingResolver { inner, ctx, _phantom: PhantomData }
}
@@ -380,7 +392,9 @@ impl<Reporter: self::Reporter + 'static> Resolver for PrefetchingResolver<Report
let mut result = self.inner.resolve(wanted_dependency, opts).await?;
if let Some(result_mut) = result.as_mut() {
self.populate_missing_integrity(result_mut).await?;
if !self.should_skip_prefetch(wanted_dependency, result_mut) {
if self.ctx.prefetch_downloads
&& !self.should_skip_prefetch(wanted_dependency, result_mut)
{
self.maybe_kickoff_download(result_mut);
}
}
@@ -109,6 +109,14 @@ impl Resolver for FixedResolver {
fn resolver_with_inner(
dir: &Path,
inner: Box<dyn Resolver>,
) -> PrefetchingResolver<SilentReporter> {
resolver_with_prefetch(dir, inner, true)
}
fn resolver_with_prefetch(
dir: &Path,
inner: Box<dyn Resolver>,
prefetch_downloads: bool,
) -> PrefetchingResolver<SilentReporter> {
let mut config = Config::new();
config.store_dir = dir.join("store").into();
@@ -129,6 +137,7 @@ fn resolver_with_inner(
requester: "/project",
supported_architectures: None,
progress_reported: &SharedReportedProgressKeys::default(),
prefetch_downloads,
},
)
}
@@ -298,3 +307,87 @@ async fn keeps_prefetch_for_required_manifest() {
assert!(!resolver.should_skip_prefetch(&wanted, &result));
}
fn integrity_pinned_result(tarball_url: &str) -> ResolveResult {
let mut result =
result_with_manifest("pinned", json!({ "name": "pinned", "version": "1.0.0" }));
result.resolution = LockfileResolution::Tarball(TarballResolution {
integrity: Some("sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==".parse().unwrap()),
tarball: tarball_url.to_string(),
git_hosted: None,
path: None,
});
result
}
/// <https://github.com/pnpm/pnpm/issues/13547>
#[tokio::test]
async fn populates_integrity_with_prefetching_off() {
let dir = tempdir().unwrap();
let mut server = mockito::Server::new_async().await;
let tarball_path = "/unpinned-1.0.0.tgz";
let get_mock = server
.mock("GET", tarball_path)
.with_status(200)
.with_body(minimal_tarball("unpinned", "1.0.0"))
.expect(1)
.create_async()
.await;
let mut result = result_with_manifest("unpinned", json!({}));
result.resolution = LockfileResolution::Tarball(TarballResolution {
integrity: None,
tarball: format!("{}{tarball_path}", server.url()),
git_hosted: None,
path: None,
});
let resolver = resolver_with_prefetch(dir.path(), Box::new(FixedResolver { result }), false);
let resolved = resolver
.resolve(&WantedDependency::default(), &ResolveOptions::default())
.await
.expect("resolve succeeds")
.expect("resolver returns a result");
let LockfileResolution::Tarball(tarball) = resolved.resolution else {
panic!("expected tarball resolution");
};
assert!(tarball.integrity.is_some(), "an unpinned tarball still needs its integrity");
get_mock.assert_async().await;
}
#[tokio::test]
async fn skips_the_background_download_with_prefetching_off() {
let dir = tempdir().unwrap();
let tarball_url = "https://registry.example/pinned-1.0.0.tgz";
let resolver = resolver_with_prefetch(
dir.path(),
Box::new(FixedResolver { result: integrity_pinned_result(tarball_url) }),
false,
);
resolver
.resolve(&WantedDependency::default(), &ResolveOptions::default())
.await
.expect("resolve succeeds")
.expect("resolver returns a result");
assert!(resolver.ctx.spawned_urls.is_empty(), "no download may be claimed");
}
#[tokio::test]
async fn claims_the_background_download_with_prefetching_on() {
let dir = tempdir().unwrap();
let tarball_url = "https://registry.example/pinned-1.0.0.tgz";
let resolver = resolver_with_inner(
dir.path(),
Box::new(FixedResolver { result: integrity_pinned_result(tarball_url) }),
);
resolver
.resolve(&WantedDependency::default(), &ResolveOptions::default())
.await
.expect("resolve succeeds")
.expect("resolver returns a result");
assert!(resolver.ctx.spawned_urls.contains(tarball_url), "the download must be claimed");
}
@@ -33,6 +33,7 @@ reqwest = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
sha2 = { workspace = true }
ssri = { workspace = true }
tokio = { workspace = true }
tracing = { workspace = true }
@@ -43,7 +44,6 @@ pacquet-resolving-local-resolver = { workspace = true }
futures-util = { workspace = true }
mockito = { workspace = true }
pretty_assertions = { workspace = true }
ssri = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = ["macros", "rt"] }
@@ -2,6 +2,7 @@
use derive_more::{Display, Error};
use miette::Diagnostic;
use pacquet_network::redact_and_sanitize;
/// Failure to fetch a registry metadata document. Used by
/// [`crate::fetch_full_metadata()`] and
@@ -167,5 +168,30 @@ pub struct GuardRepickLimitError {
pub reason: String,
}
/// Raised when a registry version carries no `dist.integrity` and its
/// `dist.shasum` is not a hex digest, so no SRI string can be derived
/// from it.
///
/// Both fields are quoted registry metadata, so [`Self::new`] redacts
/// and sanitizes them — see [`FetchMetadataError`] for why.
#[derive(Debug, Display, Error, Diagnostic)]
#[display(r#"Tarball "{tarball}" has invalid shasum specified in its metadata: {shasum}"#)]
#[diagnostic(code(ERR_PNPM_INVALID_TARBALL_INTEGRITY))]
pub struct InvalidTarballIntegrityError {
#[error(not(source))]
pub tarball: String,
pub shasum: String,
}
impl InvalidTarballIntegrityError {
#[must_use]
pub fn new(tarball: &str, shasum: &str) -> Self {
InvalidTarballIntegrityError {
tarball: redact_and_sanitize(tarball),
shasum: redact_and_sanitize(shasum),
}
}
}
#[cfg(test)]
mod tests;
@@ -44,7 +44,7 @@ pub use create_npm_resolution_verifier::{
CreateNpmResolutionVerifierOptions, DistStats, NpmResolutionVerifier, ObservedDistStats,
create_npm_resolution_verifier, observed_dist_stats_sink,
};
pub use errors::FetchMetadataError;
pub use errors::{FetchMetadataError, InvalidTarballIntegrityError};
pub use fetch_attestation_published_at::{FetchAttestationOptions, fetch_attestation_published_at};
pub use fetch_full_metadata::{
FetchFullMetadataOptions, FetchFullMetadataOutcome, fetch_full_metadata,
@@ -30,16 +30,17 @@ use node_semver::Version;
use pacquet_config::{TrustPolicy, version_policy::PackageVersionPolicy};
use pacquet_lockfile::{LockfileResolution, PkgName, PkgNameVer, TarballResolution};
use pacquet_network::{AuthHeaders, RetryOpts, ThrottledClient, redact_and_sanitize};
use pacquet_registry::{Package, PackageVersion, RangeSpecStyle};
use pacquet_registry::{Package, PackageDistribution, PackageVersion, RangeSpecStyle};
use pacquet_resolving_resolver_base::{
LatestInfo, LatestQuery, NoMatchingVersionError, PackageVersionGuardDecision,
RegistryResponseError, RegistryResponseErrorOptions, ResolutionPolicyViolation, ResolveError,
ResolveFuture, ResolveLatestFuture, ResolveOptions, ResolveResult, Resolver, UpdateBehavior,
WantedDependency, WorkspacePackages, parse_packument_timestamp,
};
use ssri::{Algorithm, Integrity};
use crate::{
errors::{AllVersionsBlockedError, GuardRepickLimitError},
errors::{AllVersionsBlockedError, GuardRepickLimitError, InvalidTarballIntegrityError},
named_registry::pick_registry_for_package,
parse_bare_specifier::{parse_bare_specifier, parse_jsr_specifier_to_registry_package_spec},
pick_package::{
@@ -814,7 +815,7 @@ pub(crate) fn build_resolve_result(
// the URL the install path needs.
let resolution = LockfileResolution::Tarball(TarballResolution {
tarball: picked.dist.tarball.clone(),
integrity: picked.dist.integrity.clone(),
integrity: dist_integrity(&picked.dist)?,
git_hosted: None,
path: None,
});
@@ -861,6 +862,24 @@ pub(crate) fn build_resolve_result(
})
}
/// The integrity a registry version's `dist` pins its tarball with.
///
/// A registry predating subresource integrity publishes only the legacy
/// `dist.shasum` hex digest, which pins the bytes just as well, so it is
/// promoted to its `sha1-` SRI form. `None` when the version pins
/// nothing at all.
fn dist_integrity(dist: &PackageDistribution) -> Result<Option<Integrity>, ResolveError> {
if let Some(integrity) = &dist.integrity {
return Ok(Some(integrity.clone()));
}
let Some(shasum) = dist.shasum.as_deref().filter(|shasum| !shasum.is_empty()) else {
return Ok(None);
};
Integrity::from_hex(shasum, Algorithm::Sha1).map(Some).map_err(|_| {
Box::new(InvalidTarballIntegrityError::new(&dist.tarball, shasum)) as ResolveError
})
}
/// The `(specifier, pin)` pair a manifest-ready specifier is computed
/// from, or `None` when there is nothing to compute: the caller did not
/// ask for one (`ResolveOptions::calc_specifier`), or `spec` already
@@ -18,6 +18,7 @@ use serde_json::json;
use tempfile::TempDir;
use crate::{
errors::InvalidTarballIntegrityError,
npm_resolver::NpmResolver,
pick_package::{
InMemoryPackageMetaCache, shared_packument_fetch_locker, shared_picked_manifest_cache,
@@ -1539,3 +1540,111 @@ async fn jsr_specifier_suppresses_latest_when_published_by_holds_back_raw_latest
assert_eq!(result.name_ver.as_ref().expect("name_ver").suffix.to_string(), "1.0.0");
assert!(result.latest.is_none(), "immature dist-tags.latest suppresses the hint");
}
/// A packument whose `dist` carries only the legacy `shasum`, the shape
/// behind <https://github.com/pnpm/pnpm/issues/13547>.
fn shasum_only_package_body(shasum: &str) -> String {
json!({
"name": "acme",
"dist-tags": { "latest": "1.0.0" },
"modified": "2025-01-15T12:00:00.000Z",
"versions": {
"1.0.0": {
"name": "acme",
"version": "1.0.0",
"dist": {
"shasum": shasum,
"tarball": "https://registry/acme-1.0.0.tgz",
},
},
},
})
.to_string()
}
#[tokio::test]
async fn shasum_only_metadata_resolves_to_a_sha1_integrity() {
let mut server = mockito::Server::new_async().await;
let _mock = server
.mock("GET", "/acme")
.with_status(200)
.with_body(shasum_only_package_body("e21bf1d18b7ce29d1cd45f6d8e0e8bcd0a4ca8ba"))
.create_async()
.await;
let registry = format!("{}/", server.url());
let (resolver, _tempdir) = build_resolver(&registry);
let wanted =
WantedDependency { alias: Some("acme".to_string()), ..WantedDependency::default() };
let result = resolver.resolve(&wanted, &ResolveOptions::default()).await.unwrap().unwrap();
let LockfileResolution::Tarball(tarball) = &result.resolution else {
panic!("expected a tarball resolution, got {:?}", result.resolution);
};
assert_eq!(
tarball.integrity.as_ref().map(ToString::to_string).as_deref(),
Some("sha1-4hvx0Yt84p0c1F9tjg6LzQpMqLo="),
);
}
#[tokio::test]
async fn unparsable_shasum_fails_the_resolve() {
let mut server = mockito::Server::new_async().await;
let _mock = server
.mock("GET", "/acme")
.with_status(200)
.with_body(shasum_only_package_body("not-a-hex-digest"))
.create_async()
.await;
let registry = format!("{}/", server.url());
let (resolver, _tempdir) = build_resolver(&registry);
let wanted =
WantedDependency { alias: Some("acme".to_string()), ..WantedDependency::default() };
let error = resolver
.resolve(&wanted, &ResolveOptions::default())
.await
.expect_err("an unusable shasum must fail the resolve");
let error = error.downcast_ref::<InvalidTarballIntegrityError>().expect("integrity error");
assert_eq!(error.shasum, "not-a-hex-digest");
assert_eq!(error.tarball, "https://registry/acme-1.0.0.tgz");
}
#[tokio::test]
async fn invalid_shasum_error_redacts_registry_metadata() {
let mut server = mockito::Server::new_async().await;
let body = json!({
"name": "acme",
"dist-tags": { "latest": "1.0.0" },
"modified": "2025-01-15T12:00:00.000Z",
"versions": {
"1.0.0": {
"name": "acme",
"version": "1.0.0",
"dist": {
"shasum": "not\u{7}-a-hex-digest",
"tarball": "https://user:hunter2@registry/acme-1.0.0.tgz",
},
},
},
})
.to_string();
let _mock = server.mock("GET", "/acme").with_status(200).with_body(body).create_async().await;
let registry = format!("{}/", server.url());
let (resolver, _tempdir) = build_resolver(&registry);
let wanted =
WantedDependency { alias: Some("acme".to_string()), ..WantedDependency::default() };
let error = resolver
.resolve(&wanted, &ResolveOptions::default())
.await
.expect_err("an unusable shasum must fail the resolve")
.to_string();
assert!(!error.contains("hunter2"), "inline credentials must not reach the message: {error}");
assert!(
!error.chars().any(char::is_control),
"control characters must not reach the message: {error:?}",
);
}