mirror of
https://github.com/pnpm/pnpm.git
synced 2026-05-13 02:55:56 -04:00
For git-hosted tarballs (`codeload.github.com` / `gitlab.com` / `bitbucket.org`) the fetcher dropped the integrity it computed while downloading, so the lockfile only ever stored the URL. A compromised git host or man-in-the-middle could serve a substituted tarball on subsequent installs and pnpm would install it — the lockfile had no hash to compare against.
This pins the SHA-512 SRI of the raw tarball in the lockfile, in the same `sha512-<base64>` form npm-registry tarballs use. The only difference is the source: for npm we pass through `dist.integrity`, for git we compute it locally from the downloaded buffer. Subsequent installs validate the download against that integrity in the worker (`addTarballToStore` → `parseIntegrity` → hash compare), so a tampered tarball fails with `TarballIntegrityError`.
## Why git-hosted stays on `gitHostedStoreIndexKey`
The lockfile pins integrity for security, but the *store key* for git-hosted resolutions stays on `gitHostedStoreIndexKey(pkgId, { built })` rather than collapsing under the integrity-based key. Reason: git-hosted tarballs are post-processed (`preparePackage` / `packlist`), so the cached file set depends on whether build scripts ran during fetch. The integrity-only key would fold the built and not-built variants into a single slot, letting one overwrite the other and serving the wrong content if `ignoreScripts` was toggled between runs. Keeping git-hosted on the existing key shape preserves that dimension; the integrity is still validated on every fresh download.
## How the routing stays clean
The naive way to express "use gitHostedStoreIndexKey for git-hosted, integrity key for npm" is to call `isGitHostedPkgUrl(resolution.tarball)` everywhere a store key is computed — fragile, scattered, and easy to forget when adding new readers (Copilot caught two of those during review). Instead, a typed annotation: `TarballResolution` gets an optional `gitHosted: boolean` field. The git resolver sets it; the lockfile loader (`convertToLockfileObject`) backfills it for entries written by older pnpm versions; `toLockfileResolution` carries it through on serialize. Every consumer reads `resolution.gitHosted` directly. URL detection lives in exactly two places — the resolver and the loader — instead of seven.
## Changes
### Security fix
- `fetching/tarball-fetcher/src/gitHostedTarballFetcher.ts` — return the `integrity` that the inner remote-tarball fetch already computed (was being silently dropped by the destructure).
### Lockfile schema (additive)
- `@pnpm/lockfile.types` and `@pnpm/resolving.resolver-base` — `TarballResolution` gains optional `gitHosted: boolean`.
- `@pnpm/resolving.git-resolver` — sets `gitHosted: true` on every git-hosted tarball it produces.
- `@pnpm/lockfile.fs` (`convertToLockfileObject`) — backfills the field on load for older lockfiles via inlined URL detection.
- `@pnpm/lockfile.utils` (`toLockfileResolution`, `pkgSnapshotToResolution`) — preserve / read the field.
### Store-key consumers (now one-line typed reads, dropped the URL-sniffing dep)
- `installing/package-requester` (`getFilesIndexFilePath`)
- `store/pkg-finder` (`readPackageFileMap`)
- `modules-mounter/daemon` (`createFuseHandlers`)
- `building/after-install` (side-effects-cache lookup + write)
- `store/commands/storeStatus`
- `installing/deps-installer` (agent-mode store-controller wrapper)
### Fetcher routing
- `fetching/pick-fetcher` — `pickFetcher` prefers `resolution.gitHosted`; URL fallback retained for ad-hoc resolutions.
### Tests
- New integrity-validation test in `tarball-fetcher` (mismatched `integrity` on the resolution must throw `TarballIntegrityError`).
- New git-hosted lookup test in `pkg-finder` asserting routing through `gitHostedStoreIndexKey` even when integrity is present.
- New `toLockfileResolution` test asserting `gitHosted: true` flows through serialization.
- `fromRepo.ts` lockfile snapshot updated for the now-pinned integrity + `gitHosted: true`.
- `git-resolver` tests updated to assert `gitHosted: true` in produced resolutions.
164 lines
5.5 KiB
TypeScript
164 lines
5.5 KiB
TypeScript
import fs from 'node:fs'
|
|
import os from 'node:os'
|
|
import path from 'node:path'
|
|
|
|
import { afterAll, beforeAll, describe, expect, it } from '@jest/globals'
|
|
import type { GitResolution, Resolution, TarballResolution } from '@pnpm/resolving.resolver-base'
|
|
import type { PackageFilesIndex } from '@pnpm/store.cafs'
|
|
import { gitHostedStoreIndexKey, StoreIndex, storeIndexKey } from '@pnpm/store.index'
|
|
import { readPackageFileMap } from '@pnpm/store.pkg-finder'
|
|
|
|
function createFilesIndex (): PackageFilesIndex {
|
|
return {
|
|
algo: 'sha256',
|
|
files: new Map([
|
|
['package.json', { digest: 'abc123', mode: 0o644, size: 0 }],
|
|
['index.js', { digest: 'def456', mode: 0o644, size: 0 }],
|
|
]),
|
|
}
|
|
}
|
|
|
|
function writeCafsFile (storeDir: string, digest: string, content: string): void {
|
|
const dir = path.join(storeDir, 'files', digest.slice(0, 2))
|
|
fs.mkdirSync(dir, { recursive: true })
|
|
fs.writeFileSync(path.join(dir, digest.slice(2)), content)
|
|
}
|
|
|
|
describe('readPackageFileMap', () => {
|
|
let storeDir: string
|
|
let storeIndex: StoreIndex
|
|
|
|
beforeAll(() => {
|
|
storeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pnpm-pkg-finder-test-'))
|
|
storeIndex = new StoreIndex(storeDir)
|
|
})
|
|
|
|
afterAll(() => {
|
|
storeIndex.close()
|
|
fs.rmSync(storeDir, { recursive: true, force: true })
|
|
})
|
|
|
|
const defaultOpts = () => ({
|
|
storeDir,
|
|
storeIndex,
|
|
lockfileDir: '/tmp/project',
|
|
virtualStoreDirMaxLength: 120,
|
|
})
|
|
|
|
it('should resolve registry packages by integrity hash', async () => {
|
|
const integrity = 'sha512-abc123registry'
|
|
const pkgId = 'express@4.18.2'
|
|
const key = storeIndexKey(integrity, pkgId)
|
|
|
|
storeIndex.set(key, createFilesIndex())
|
|
|
|
const resolution: TarballResolution = {
|
|
integrity,
|
|
tarball: 'https://registry.npmjs.org/express/-/express-4.18.2.tgz',
|
|
}
|
|
|
|
const result = await readPackageFileMap(resolution, pkgId, defaultOpts())
|
|
|
|
expect(result).toBeDefined()
|
|
expect(result!.has('package.json')).toBe(true)
|
|
expect(result!.has('index.js')).toBe(true)
|
|
})
|
|
|
|
it('should resolve git-hosted tarball packages with integrity by gitHostedStoreIndexKey', async () => {
|
|
// Git-hosted tarballs are keyed by gitHostedStoreIndexKey to preserve the
|
|
// built/not-built dimension that the integrity-only key would collapse.
|
|
// The lockfile still pins integrity for security; the routing is driven
|
|
// by the `gitHosted` field set by the resolver / lockfile loader.
|
|
const pkgId = 'https://codeload.github.com/stevemao/left-pad/tar.gz/cafe1234'
|
|
const key = gitHostedStoreIndexKey(pkgId, { built: true })
|
|
|
|
storeIndex.set(key, createFilesIndex())
|
|
|
|
const resolution: TarballResolution = {
|
|
integrity: 'sha512-abc123gitHosted',
|
|
tarball: pkgId,
|
|
gitHosted: true,
|
|
}
|
|
|
|
const result = await readPackageFileMap(resolution, pkgId, defaultOpts())
|
|
|
|
expect(result).toBeDefined()
|
|
expect(result!.has('package.json')).toBe(true)
|
|
expect(result!.has('index.js')).toBe(true)
|
|
})
|
|
|
|
it('should resolve git-hosted tarball packages (no type, has tarball)', async () => {
|
|
const pkgId = 'left-pad@https://codeload.github.com/stevemao/left-pad/tar.gz/abc123'
|
|
const key = gitHostedStoreIndexKey(pkgId, { built: true })
|
|
|
|
storeIndex.set(key, createFilesIndex())
|
|
|
|
const resolution = {
|
|
tarball: 'https://codeload.github.com/stevemao/left-pad/tar.gz/abc123',
|
|
} as TarballResolution
|
|
|
|
const result = await readPackageFileMap(resolution, pkgId, defaultOpts())
|
|
|
|
expect(result).toBeDefined()
|
|
expect(result!.has('package.json')).toBe(true)
|
|
expect(result!.has('index.js')).toBe(true)
|
|
})
|
|
|
|
it('should resolve git dependencies with type "git" and return readable file paths', async () => {
|
|
const digest = 'aabbccdd001122'
|
|
const manifestContent = JSON.stringify({
|
|
name: 'left-pad',
|
|
version: '1.3.0',
|
|
license: 'MIT',
|
|
})
|
|
writeCafsFile(storeDir, digest, manifestContent)
|
|
|
|
const pkgId = 'left-pad@git+https://github.com/stevemao/left-pad.git#2fca6157fcca165438e0f9495cf0e5a4e6f71349'
|
|
const filesIndex: PackageFilesIndex = {
|
|
algo: 'sha256',
|
|
files: new Map([
|
|
['package.json', { digest, mode: 0o644, size: 0 }],
|
|
]),
|
|
}
|
|
storeIndex.set(gitHostedStoreIndexKey(pkgId, { built: true }), filesIndex)
|
|
|
|
const resolution: GitResolution = {
|
|
type: 'git',
|
|
repo: 'https://github.com/stevemao/left-pad.git',
|
|
commit: '2fca6157fcca165438e0f9495cf0e5a4e6f71349',
|
|
}
|
|
|
|
const result = await readPackageFileMap(resolution, pkgId, defaultOpts())
|
|
|
|
expect(result).toBeDefined()
|
|
const manifestPath = result!.get('package.json')!
|
|
expect(fs.existsSync(manifestPath)).toBe(true)
|
|
|
|
const parsed = JSON.parse(fs.readFileSync(manifestPath, 'utf8'))
|
|
expect(parsed.name).toBe('left-pad')
|
|
expect(parsed.license).toBe('MIT')
|
|
})
|
|
|
|
it('should throw ENOENT when store index has no entry for a git dependency', async () => {
|
|
const pkgId = 'missing-pkg@git+https://github.com/user/missing-pkg.git#deadbeef'
|
|
|
|
const resolution: GitResolution = {
|
|
type: 'git',
|
|
repo: 'https://github.com/user/missing-pkg.git',
|
|
commit: 'deadbeef',
|
|
}
|
|
|
|
await expect(
|
|
readPackageFileMap(resolution, pkgId, defaultOpts())
|
|
).rejects.toThrow(/package index not found/)
|
|
})
|
|
|
|
it('should return undefined for unknown resolution types', async () => {
|
|
const resolution = { type: 'unknown-type' } as unknown as Resolution
|
|
|
|
const result = await readPackageFileMap(resolution, 'some-pkg@1.0.0', defaultOpts())
|
|
|
|
expect(result).toBeUndefined()
|
|
})
|
|
})
|