diff --git a/.changeset/fix-tarball-integrity-dropped.md b/.changeset/fix-tarball-integrity-dropped.md new file mode 100644 index 0000000000..36fdbd01b6 --- /dev/null +++ b/.changeset/fix-tarball-integrity-dropped.md @@ -0,0 +1,6 @@ +--- +"@pnpm/installing.package-requester": patch +"pnpm": patch +--- + +Fix the `integrity` field being dropped from the lockfile entry of a remote (non-registry) https-tarball dependency when an unrelated package is installed afterwards. URL/tarball resolvers do not return an integrity (it is only known after the tarball is downloaded), so when such a dependency was reused from the lockfile without being re-fetched, its integrity was lost. It is now carried over from the existing resolution. With pnpm's lockfile-integrity hardening, the missing integrity made subsequent `--frozen-lockfile` installs fail with `ERR_PNPM_MISSING_TARBALL_INTEGRITY`. [#12001](https://github.com/pnpm/pnpm/issues/12001). diff --git a/installing/package-requester/src/packageRequester.ts b/installing/package-requester/src/packageRequester.ts index 7547a0ec56..93428c270c 100644 --- a/installing/package-requester/src/packageRequester.ts +++ b/installing/package-requester/src/packageRequester.ts @@ -207,6 +207,20 @@ async function resolveAndFetch ( resolution = resolveResult.resolution pkgId = resolveResult.id + // URL/tarball resolvers don't return an integrity, because it is only known + // after the tarball is downloaded. When a package is reused from the lockfile + // without being re-fetched, the freshly resolved resolution has no integrity, + // so carry it over from the current resolution instead of dropping it. + // https://github.com/pnpm/pnpm/issues/12001 + if ( + !updated && + typeof previousIntegrity === 'string' && + !resolution.type && + !(resolution as TarballResolution).integrity + ) { + (resolution as TarballResolution).integrity = previousIntegrity + } + const id = pkgId! if ('type' in resolution && resolution.type === 'directory' && !id.startsWith('file:')) { diff --git a/installing/package-requester/test/index.ts b/installing/package-requester/test/index.ts index f87de972f3..802e73a0af 100644 --- a/installing/package-requester/test/index.ts +++ b/installing/package-requester/test/index.ts @@ -437,6 +437,57 @@ test('force fetch when resolution integrity differs from current package integri expect(files.resolvedFrom).toBe('remote') }) +// https://github.com/pnpm/pnpm/issues/12001 +test('integrity of a tarball dependency is preserved when the resolver returns no integrity', async () => { + const storeDir = temporaryDirectory() + const cafs = createCafsStore(storeDir) + const projectDir = temporaryDirectory() + + const tarball = 'https://example.com/foo/-/foo-1.0.0.tgz' + const integrity = 'sha512-xxzPGZ4P2uN6rROUa5N9Z7zTX6ERuE0hs6GUOc/cKBLF2NqKc16UwqHMt3tFg4CO6EBTE5UecUasg+3jZx3Ckg==' + + // URL/tarball resolvers don't return an integrity, because it is only known + // after the tarball is downloaded. This mimics @pnpm/resolving.tarball-resolver. + const customResolve: typeof resolve = async () => ({ + id: tarball as PkgResolutionId, + normalizedBareSpecifier: tarball, + resolution: { tarball }, + resolvedVia: 'url', + manifest: { + name: 'foo', + version: '1.0.0', + }, + }) + + const requestPackage = createPackageRequester({ + resolve: customResolve, + fetchers, + cafs, + storeDir, + verifyStoreIntegrity: true, + virtualStoreDirMaxLength: 120, + }) + + // The package is already in the lockfile (currentPkg) with its integrity, and + // it is not re-fetched (skipFetch). The integrity from the resolver is absent, + // so it must be carried over from the current package instead of being dropped. + const response = await requestPackage({ alias: 'foo', bareSpecifier: tarball }, { + currentPkg: { + id: tarball as PkgResolutionId, + resolution: { tarball, integrity }, + }, + downloadPriority: 0, + lockfileDir: projectDir, + preferredVersions: {}, + projectDir, + skipFetch: true, + update: false, + }) + + expect(response.body.updated).toBe(false) + expect(response.body.resolution).toStrictEqual({ tarball, integrity }) +}) + test('fetchPackageToStore()', async () => { const storeDir = temporaryDirectory() const cafs = createCafsStore(storeDir) diff --git a/pacquet/crates/cli/tests/tarball_url_dependency.rs b/pacquet/crates/cli/tests/tarball_url_dependency.rs new file mode 100644 index 0000000000..6c23465a7e --- /dev/null +++ b/pacquet/crates/cli/tests/tarball_url_dependency.rs @@ -0,0 +1,134 @@ +//! Integrity preservation for remote https-tarball dependencies. +//! +//! URL/tarball resolvers carry no `integrity` at resolve time — it is +//! only known after the tarball is downloaded and hashed. The risk +//! (pnpm issue [#12001](https://github.com/pnpm/pnpm/issues/12001), +//! fixed upstream in [#12040](https://github.com/pnpm/pnpm/pull/12040)) +//! is that installing an *unrelated* package later rewrites the +//! lockfile while the tarball dependency is reused without being +//! re-fetched, dropping its recorded integrity — which then makes the +//! next `--frozen-lockfile` install fail closed. +//! +//! Reproducing that in pacquet requires resolving a dependency through +//! the [`TarballResolver`], which only claims a bare specifier whose +//! URL does *not* start with the configured registry. A registry-host +//! tarball URL is parsed by the npm resolver instead (see +//! `parse_bare_specifier`), so it carries the registry's integrity from +//! metadata and never exercises the reuse path #12001 is about. +//! +//! Pacquet doesn't support remote (non-registry) https-tarball *direct +//! dependencies* end to end yet, so the scenario below is a +//! [`known_failures`] entry. See that module for the exact gap. + +use assert_cmd::prelude::*; +use command_extra::CommandExtra; +use pacquet_testing_utils::bin::{AddMockedRegistry, CommandTempCwd}; +use std::{fs, path::Path, process::Command}; + +use known_failures::external_tarball_dependency_unsupported; +use pacquet_testing_utils::allow_known_failure; + +fn pacquet_at(workspace: &Path) -> Command { + Command::cargo_bin("pacquet").expect("find the pacquet binary").with_current_dir(workspace) +} + +/// The `integrity:` recorded for a `packages:` entry, e.g. +/// `is-positive@1.0.0`. `None` when the entry is absent or carries no +/// integrity (the #12001 regression). +fn package_integrity(lockfile: &str, package_key: &str) -> Option { + let header = format!("{package_key}:"); + lockfile + .lines() + .skip_while(|line| line.trim() != header) + .take_while(|line| !line.trim_start().starts_with("snapshots:")) + .find_map(|line| line.trim().strip_prefix("integrity:").map(|rest| rest.trim().to_string())) +} + +/// A remote-tarball dependency keeps its integrity when an unrelated +/// dependency is added and the lockfile is rewritten, so the next +/// `--frozen-lockfile` install still succeeds. +#[test] +fn remote_tarball_integrity_survives_unrelated_install() { + allow_known_failure!(external_tarball_dependency_unsupported()); + + let CommandTempCwd { workspace, root, npmrc_info, .. } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { mock_instance, .. } = npmrc_info; + + // The registry is `http://localhost:PORT/`; pointing at the same + // loopback server via `127.0.0.1` keeps the URL from matching the + // registry prefix, so the TarballResolver — not the npm resolver — + // claims it. The tarball is still downloadable from that server. + let tarball = format!( + "{}is-positive/-/is-positive-1.0.0.tgz", + mock_instance.url().replace("localhost", "127.0.0.1"), + ); + let manifest_path = workspace.join("package.json"); + let lockfile_path = workspace.join("pnpm-lock.yaml"); + + fs::write( + &manifest_path, + serde_json::json!({ "dependencies": { "is-positive": tarball } }).to_string(), + ) + .expect("write package.json"); + pacquet_at(&workspace).with_arg("install").assert().success(); + + let lockfile = fs::read_to_string(&lockfile_path).expect("read pnpm-lock.yaml"); + let integrity = package_integrity(&lockfile, "is-positive@1.0.0").unwrap_or_else(|| { + panic!("the fresh install must record an integrity for the tarball dep:\n{lockfile}") + }); + + // Install an unrelated package. This rewrites the lockfile while the + // tarball dependency is reused — the exact #12001 trigger. + fs::write( + &manifest_path, + serde_json::json!({ + "dependencies": { "is-positive": tarball, "@pnpm.e2e/pkg-with-1-dep": "100.0.0" } + }) + .to_string(), + ) + .expect("rewrite package.json with an unrelated dependency"); + pacquet_at(&workspace).with_arg("install").assert().success(); + + let lockfile = fs::read_to_string(&lockfile_path).expect("read pnpm-lock.yaml"); + assert!( + lockfile.contains("@pnpm.e2e/pkg-with-1-dep@100.0.0"), + "the unrelated dependency must be recorded:\n{lockfile}", + ); + assert_eq!( + package_integrity(&lockfile, "is-positive@1.0.0").as_deref(), + Some(integrity.as_str()), + "the tarball dependency's integrity must be preserved verbatim:\n{lockfile}", + ); + + // The frozen install is the symptom #12001 reports: it fails closed + // when the tarball entry has lost its integrity. + pacquet_at(&workspace).with_args(["install", "--frozen-lockfile"]).assert().success(); + + drop((root, mock_instance)); +} + +mod known_failures { + //! Subject-under-test not built yet, stubbed through + //! [`pacquet_testing_utils::allow_known_failure`] so the port exits + //! early instead of masking a real bug. + + use pacquet_testing_utils::known_failure::{KnownFailure, KnownResult}; + + /// A remote, non-registry https-tarball *direct dependency* (e.g. + /// `https://cdn.example.com/foo-1.0.0.tgz`) cannot be installed yet: + /// `TarballResolver` returns no `name_ver` (the name/version live in + /// the tarball's manifest, which pacquet doesn't fetch during + /// resolution), so `dependencies_graph_to_lockfile` panics with + /// `MissingSuffix` building the importer dep path. Until the + /// resolve-time tarball-manifest fetch (and the integrity it + /// computes) lands, pnpm #12001's integrity-preservation-on-reuse + /// isn't reachable here. Registry-host tarball URLs take the npm + /// resolver path instead and already carry integrity from metadata. + /// Tracked in . + pub fn external_tarball_dependency_unsupported() -> KnownResult<()> { + Err(KnownFailure::new( + "remote non-registry https-tarball direct dependencies are unsupported", + )) + } +}