diff --git a/.changeset/config-deps-path-traversal.md b/.changeset/config-deps-path-traversal.md new file mode 100644 index 0000000000..04c0f2285c --- /dev/null +++ b/.changeset/config-deps-path-traversal.md @@ -0,0 +1,6 @@ +--- +"@pnpm/installing.env-installer": patch +"pnpm": patch +--- + +Security: validate config dependency names and versions from the env lockfile (`pnpm-lock.yaml`) before using them to build filesystem paths. A committed lockfile with a traversal-shaped `configDependencies` name (such as `../../PWNED`) or version (such as `../../../PWNED`) could previously cause `pnpm install` to create symlinks or write package files outside `node_modules/.pnpm-config` and the store. Names must now be valid npm package names and versions must be exact semver versions; the same validation is applied to optional subdependencies of config dependencies, and to the legacy workspace-manifest format before any lockfile is written. See [GHSA-qrv3-253h-g69c](https://github.com/pnpm/pnpm/security/advisories/GHSA-qrv3-253h-g69c). diff --git a/Cargo.lock b/Cargo.lock index 1fcee437e5..138ffe8c0c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3750,6 +3750,7 @@ dependencies = [ "pacquet-registry", "pacquet-reporter", "pacquet-resolving-npm-resolver", + "pacquet-resolving-parse-wanted-dependency", "pacquet-resolving-resolver-base", "pacquet-store-dir", "pacquet-tarball", diff --git a/installing/env-installer/src/assertValidConfigDepVersion.ts b/installing/env-installer/src/assertValidConfigDepVersion.ts new file mode 100644 index 0000000000..00418711dd --- /dev/null +++ b/installing/env-installer/src/assertValidConfigDepVersion.ts @@ -0,0 +1,15 @@ +import { PnpmError } from '@pnpm/error' +import semver from 'semver' + +// A config-dep version becomes a store path segment (`//`), +// so reject non-semver values to keep a traversal-shaped version from escaping +// the store root. +export function assertValidConfigDepVersion (name: string, version: string): void { + if (semver.valid(version) == null) { + throw new PnpmError( + 'INVALID_CONFIG_DEP_VERSION', + `The config dependency "${name}" has an invalid version "${version}"`, + { hint: 'A config dependency version must be an exact semver version.' } + ) + } +} diff --git a/installing/env-installer/src/index.ts b/installing/env-installer/src/index.ts index 7e182c4d3d..6463563f01 100644 --- a/installing/env-installer/src/index.ts +++ b/installing/env-installer/src/index.ts @@ -2,3 +2,4 @@ export { installConfigDeps, type InstallConfigDepsOpts } from './installConfigDe export { resolveAndInstallConfigDeps, type ResolveAndInstallConfigDepsOpts } from './resolveAndInstallConfigDeps.js' export { resolveConfigDeps, type ResolveConfigDepsOpts } from './resolveConfigDeps.js' export { isPackageManagerResolved, resolvePackageManagerIntegrities, type ResolvePackageManagerIntegritiesOpts } from './resolvePackageManagerIntegrities.js' +export { verifyEnvLockfile } from './verifyEnvLockfile.js' diff --git a/installing/env-installer/src/installConfigDeps.ts b/installing/env-installer/src/installConfigDeps.ts index 5d7540e92a..7a8f0afa53 100644 --- a/installing/env-installer/src/installConfigDeps.ts +++ b/installing/env-installer/src/installConfigDeps.ts @@ -16,6 +16,7 @@ import { symlinkDir } from 'symlink-dir' import { migrateConfigDepsToLockfile } from './migrateConfigDeps.js' import type { NormalizedConfigDep, NormalizedSubdep } from './parseIntegrity.js' +import { verifyEnvLockfile } from './verifyEnvLockfile.js' export interface InstallConfigDepsOpts { frozenLockfile?: boolean @@ -133,6 +134,7 @@ async function normalizeForInstall ( ): Promise> { // If it's a EnvLockfile object (has lockfileVersion), use it directly if (isEnvLockfile(configDepsOrLockfile)) { + verifyEnvLockfile(configDepsOrLockfile) return normalizeFromLockfile(configDepsOrLockfile, opts.registries) } @@ -140,6 +142,7 @@ async function normalizeForInstall ( // Try to read the env lockfile first. const envLockfile = await readEnvLockfile(opts.rootDir) if (envLockfile) { + verifyEnvLockfile(envLockfile) return normalizeFromLockfile(envLockfile, opts.registries) } diff --git a/installing/env-installer/src/migrateConfigDeps.ts b/installing/env-installer/src/migrateConfigDeps.ts index 4f8b1ac0cd..28ff250611 100644 --- a/installing/env-installer/src/migrateConfigDeps.ts +++ b/installing/env-installer/src/migrateConfigDeps.ts @@ -1,13 +1,14 @@ import { pickRegistryForPackage } from '@pnpm/config.pick-registry-for-package' import { writeSettings } from '@pnpm/config.writer' import { PnpmError } from '@pnpm/error' -import { createEnvLockfile, writeEnvLockfile } from '@pnpm/lockfile.fs' +import { createEnvLockfile } from '@pnpm/lockfile.fs' import { toLockfileResolution } from '@pnpm/lockfile.utils' import type { ConfigDependencies, ConfigDependencySpecifiers, Registries } from '@pnpm/types' import getNpmTarballUrl from 'get-npm-tarball-url' import type { NormalizedConfigDep } from './parseIntegrity.js' import { parseIntegrity } from './parseIntegrity.js' +import { writeVerifiedEnvLockfile } from './writeVerifiedEnvLockfile.js' interface MigrateOpts { registries: Registries @@ -26,6 +27,9 @@ export async function migrateConfigDepsToLockfile ( opts: MigrateOpts ): Promise> { const envLockfile = createEnvLockfile() + // Null-prototype so a `__proto__` name lands as an own key verifyEnvLockfile + // sees, not a silent prototype mutation. + envLockfile.importers['.'].configDependencies = Object.create(null) const cleanSpecifiers: ConfigDependencySpecifiers = {} const normalizedDeps: Record = {} @@ -89,17 +93,14 @@ export async function migrateConfigDepsToLockfile ( } } - // Write the new env lockfile and clean up workspace manifest - await Promise.all([ - writeEnvLockfile(opts.rootDir, envLockfile), - writeSettings({ - rootProjectManifestDir: opts.rootDir, - workspaceDir: opts.rootDir, - updatedSettings: { - configDependencies: cleanSpecifiers, - }, - }), - ]) + await writeVerifiedEnvLockfile(opts.rootDir, envLockfile) + await writeSettings({ + rootProjectManifestDir: opts.rootDir, + workspaceDir: opts.rootDir, + updatedSettings: { + configDependencies: cleanSpecifiers, + }, + }) return normalizedDeps } diff --git a/installing/env-installer/src/resolveAndInstallConfigDeps.ts b/installing/env-installer/src/resolveAndInstallConfigDeps.ts index a8601b9da1..366b13ebf5 100644 --- a/installing/env-installer/src/resolveAndInstallConfigDeps.ts +++ b/installing/env-installer/src/resolveAndInstallConfigDeps.ts @@ -4,7 +4,6 @@ import { createEnvLockfile, type EnvLockfile, readEnvLockfile, - writeEnvLockfile, } from '@pnpm/lockfile.fs' import { toLockfileResolution } from '@pnpm/lockfile.utils' import { createGetAuthHeaderByURI } from '@pnpm/network.auth-header' @@ -17,6 +16,7 @@ import { installConfigDeps, type InstallConfigDepsOpts } from './installConfigDe import { parseIntegrity } from './parseIntegrity.js' import { pruneEnvLockfile } from './pruneEnvLockfile.js' import { resolveOptionalSubdeps } from './resolveOptionalSubdeps.js' +import { writeVerifiedEnvLockfile } from './writeVerifiedEnvLockfile.js' export type ResolveAndInstallConfigDepsOpts = CreateFetchFromRegistryOptions & ResolverFactoryOptions & InstallConfigDepsOpts & { rootDir: string @@ -92,7 +92,7 @@ export async function resolveAndInstallConfigDeps ( if (depsToResolve.length === 0) { if (lockfileChanged) { - await writeEnvLockfile(opts.rootDir, envLockfile) + await writeVerifiedEnvLockfile(opts.rootDir, envLockfile) } await installConfigDeps(envLockfile, opts) return @@ -143,6 +143,6 @@ export async function resolveAndInstallConfigDeps ( pruneEnvLockfile(envLockfile) - await writeEnvLockfile(opts.rootDir, envLockfile) + await writeVerifiedEnvLockfile(opts.rootDir, envLockfile) await installConfigDeps(envLockfile, opts) } diff --git a/installing/env-installer/src/resolveConfigDeps.ts b/installing/env-installer/src/resolveConfigDeps.ts index a800fd5658..c3433746e2 100644 --- a/installing/env-installer/src/resolveConfigDeps.ts +++ b/installing/env-installer/src/resolveConfigDeps.ts @@ -5,7 +5,6 @@ import { createEnvLockfile, type EnvLockfile, readEnvLockfile, - writeEnvLockfile, } from '@pnpm/lockfile.fs' import { toLockfileResolution } from '@pnpm/lockfile.utils' import { createGetAuthHeaderByURI } from '@pnpm/network.auth-header' @@ -17,6 +16,7 @@ import type { ConfigDependencies, ConfigDependencySpecifiers, RegistryConfig } f import { installConfigDeps, type InstallConfigDepsOpts } from './installConfigDeps.js' import { pruneEnvLockfile } from './pruneEnvLockfile.js' import { resolveOptionalSubdeps } from './resolveOptionalSubdeps.js' +import { writeVerifiedEnvLockfile } from './writeVerifiedEnvLockfile.js' export type ResolveConfigDepsOpts = CreateFetchFromRegistryOptions & ResolverFactoryOptions & InstallConfigDepsOpts & { configDependencies?: ConfigDependencies @@ -81,17 +81,15 @@ export async function resolveConfigDeps (configDeps: string[], opts: ResolveConf pruneEnvLockfile(envLockfile) - await Promise.all([ - writeSettings({ - ...opts, - rootProjectManifestDir: opts.rootDir, - workspaceDir: opts.rootDir, - updatedSettings: { - configDependencies: configDependencySpecifiers, - }, - }), - writeEnvLockfile(opts.rootDir, envLockfile), - ]) + await writeVerifiedEnvLockfile(opts.rootDir, envLockfile) + await writeSettings({ + ...opts, + rootProjectManifestDir: opts.rootDir, + workspaceDir: opts.rootDir, + updatedSettings: { + configDependencies: configDependencySpecifiers, + }, + }) await installConfigDeps(envLockfile, opts) } diff --git a/installing/env-installer/src/resolvePackageManagerIntegrities.ts b/installing/env-installer/src/resolvePackageManagerIntegrities.ts index d0a00e686c..9d14c07eef 100644 --- a/installing/env-installer/src/resolvePackageManagerIntegrities.ts +++ b/installing/env-installer/src/resolvePackageManagerIntegrities.ts @@ -1,4 +1,4 @@ -import { convertToLockfileFile, createEnvLockfile, readEnvLockfile, writeEnvLockfile } from '@pnpm/lockfile.fs' +import { convertToLockfileFile, createEnvLockfile, readEnvLockfile } from '@pnpm/lockfile.fs' import { pruneSharedLockfile } from '@pnpm/lockfile.pruner' import type { EnvLockfile } from '@pnpm/lockfile.types' import type { StoreController } from '@pnpm/store.controller' @@ -6,6 +6,7 @@ import type { DepPath, ProjectId, Registries } from '@pnpm/types' import { convertToLockfileEnvObject } from './pruneEnvLockfile.js' import { resolveManifestDependencies } from './resolveManifestDependencies.js' +import { writeVerifiedEnvLockfile } from './writeVerifiedEnvLockfile.js' export interface ResolvePackageManagerIntegritiesOpts { envLockfile?: EnvLockfile @@ -93,7 +94,7 @@ export async function resolvePackageManagerIntegrities ( envLockfile.snapshots = prunedFile.snapshots ?? {} if (save) { - await writeEnvLockfile(opts.rootDir, envLockfile) + await writeVerifiedEnvLockfile(opts.rootDir, envLockfile) } } return envLockfile diff --git a/installing/env-installer/src/verifyEnvLockfile.ts b/installing/env-installer/src/verifyEnvLockfile.ts new file mode 100644 index 0000000000..9c37184934 --- /dev/null +++ b/installing/env-installer/src/verifyEnvLockfile.ts @@ -0,0 +1,24 @@ +import { assertValidDependencyAliases } from '@pnpm/installing.deps-resolver' +import type { EnvLockfile } from '@pnpm/lockfile.fs' + +import { assertValidConfigDepVersion } from './assertValidConfigDepVersion.js' + +// Offline structural gate for the env lockfile, mirroring the alias/shape +// checks `verifyLockfileResolutions` runs over the main lockfile. Config +// dependency and optional-subdependency names and versions become store path +// segments, so reject any that isn't a valid npm name / exact semver before a +// path is built from them. +export function verifyEnvLockfile (envLockfile: EnvLockfile): void { + const configDeps = envLockfile.importers['.']?.configDependencies + assertValidDependencyAliases(configDeps, 'The configDependencies in pnpm-lock.yaml') + if (configDeps == null) return + for (const [name, { version }] of Object.entries(configDeps)) { + assertValidConfigDepVersion(name, version) + const optionalDeps = envLockfile.snapshots[`${name}@${version}`]?.optionalDependencies + if (optionalDeps == null) continue + assertValidDependencyAliases(optionalDeps, `The optionalDependencies of config dependency "${name}" in pnpm-lock.yaml`) + for (const [subdepName, subdepVersion] of Object.entries(optionalDeps)) { + assertValidConfigDepVersion(subdepName, subdepVersion) + } + } +} diff --git a/installing/env-installer/src/writeVerifiedEnvLockfile.ts b/installing/env-installer/src/writeVerifiedEnvLockfile.ts new file mode 100644 index 0000000000..e67920adf9 --- /dev/null +++ b/installing/env-installer/src/writeVerifiedEnvLockfile.ts @@ -0,0 +1,10 @@ +import { type EnvLockfile, writeEnvLockfile } from '@pnpm/lockfile.fs' + +import { verifyEnvLockfile } from './verifyEnvLockfile.js' + +// Persist an env lockfile only after verifying it, so no code path can write +// one carrying an invalid config-dependency name or version. +export async function writeVerifiedEnvLockfile (rootDir: string, envLockfile: EnvLockfile): Promise { + verifyEnvLockfile(envLockfile) + await writeEnvLockfile(rootDir, envLockfile) +} diff --git a/installing/env-installer/test/installConfigDeps.ts b/installing/env-installer/test/installConfigDeps.ts index c20a55c528..6b6ee2dce4 100644 --- a/installing/env-installer/test/installConfigDeps.ts +++ b/installing/env-installer/test/installConfigDeps.ts @@ -24,6 +24,22 @@ function makeEnvLockfile (deps: Record { prepareEmpty() const { storeController, storeDir } = createTempStore() @@ -83,6 +99,210 @@ test('configuration dependency is installed from env lockfile', async () => { expect(fs.existsSync('node_modules/.pnpm-config/@pnpm.e2e/foo/package.json')).toBeFalsy() }) +test('a config dependency with a path-traversal name in the env lockfile is rejected', async () => { + prepareEmpty() + const { storeController, storeDir } = createTempStore() + + const maliciousName = '../../PWNED' + const lockfile = makeEnvLockfile({ + [maliciousName]: { version: '1.0.0', integrity: getIntegrity('@pnpm.e2e/foo', '100.0.0') }, + }) + + await expect(installConfigDeps(lockfile, { + registries: { + default: registry, + }, + rootDir: process.cwd(), + store: storeController, + storeDir, + })).rejects.toThrow('invalid name') + + expect(containsEntryNamed(process.cwd(), 'PWNED')).toBe(false) + expect(containsEntryNamed(storeDir, 'PWNED')).toBe(false) +}) + +test('an optional subdep with a path-traversal name in the env lockfile is rejected', async () => { + prepareEmpty() + const { storeController, storeDir } = createTempStore() + + const parentName = '@pnpm.e2e/foo' + const parentVersion = '100.0.0' + const maliciousSubdepName = '../../PWNED_SUBDEP' + const subdepVersion = '1.0.0' + + const lockfile = createEnvLockfile() + const parentKey = `${parentName}@${parentVersion}` + lockfile.importers['.'].configDependencies[parentName] = { specifier: parentVersion, version: parentVersion } + lockfile.packages[parentKey] = { resolution: { integrity: getIntegrity(parentName, parentVersion) } } + lockfile.snapshots[parentKey] = { + optionalDependencies: { [maliciousSubdepName]: subdepVersion }, + } + lockfile.packages[`${maliciousSubdepName}@${subdepVersion}`] = { + resolution: { integrity: getIntegrity('@pnpm.e2e/bar', '100.0.0') }, + } + + await expect(installConfigDeps(lockfile, { + registries: { + default: registry, + }, + rootDir: process.cwd(), + store: storeController, + storeDir, + })).rejects.toThrow('invalid name') + + expect(containsEntryNamed(process.cwd(), 'PWNED_SUBDEP')).toBe(false) + expect(containsEntryNamed(storeDir, 'PWNED_SUBDEP')).toBe(false) +}) + +test('a config dependency named __proto__ in the env lockfile is rejected', async () => { + prepareEmpty() + const { storeController, storeDir } = createTempStore() + + const lockfile = createEnvLockfile() + // JSON.parse makes `__proto__` an own enumerable key (as on-disk parsing does); + // a plain object literal would set the prototype and hide it. + lockfile.importers['.'].configDependencies = JSON.parse('{"__proto__":{"specifier":"1.0.0","version":"1.0.0"}}') + lockfile.packages['__proto__@1.0.0'] = { resolution: { integrity: getIntegrity('@pnpm.e2e/foo', '100.0.0') } } + lockfile.snapshots['__proto__@1.0.0'] = {} + + await expect(installConfigDeps(lockfile, { + registries: { + default: registry, + }, + rootDir: process.cwd(), + store: storeController, + storeDir, + })).rejects.toThrow('invalid name') + + expect(containsEntryNamed(process.cwd(), '__proto__')).toBe(false) +}) + +test('an optional subdep named __proto__ in the env lockfile is rejected', async () => { + prepareEmpty() + const { storeController, storeDir } = createTempStore() + + const parentName = '@pnpm.e2e/foo' + const parentVersion = '100.0.0' + const lockfile = createEnvLockfile() + const parentKey = `${parentName}@${parentVersion}` + lockfile.importers['.'].configDependencies[parentName] = { specifier: parentVersion, version: parentVersion } + lockfile.packages[parentKey] = { resolution: { integrity: getIntegrity(parentName, parentVersion) } } + // JSON.parse so `__proto__` is an own enumerable key. + lockfile.snapshots[parentKey] = { optionalDependencies: JSON.parse('{"__proto__":"1.0.0"}') } + lockfile.packages['__proto__@1.0.0'] = { resolution: { integrity: getIntegrity('@pnpm.e2e/bar', '100.0.0') } } + + await expect(installConfigDeps(lockfile, { + registries: { + default: registry, + }, + rootDir: process.cwd(), + store: storeController, + storeDir, + })).rejects.toThrow('invalid name') + + expect(containsEntryNamed(process.cwd(), '__proto__')).toBe(false) + expect(containsEntryNamed(storeDir, '__proto__')).toBe(false) +}) + +test('an invalid config dependency name in the workspace manifest is rejected before any lockfile is written', async () => { + prepareEmpty() + const { storeController, storeDir } = createTempStore() + + const integrity = getIntegrity('@pnpm.e2e/foo', '100.0.0') + const configDeps: Record = { + '../../PWNED': `100.0.0+${integrity}`, + } + + await expect(installConfigDeps(configDeps, { + registries: { + default: registry, + }, + rootDir: process.cwd(), + store: storeController, + storeDir, + })).rejects.toThrow('invalid name') + + expect(fs.existsSync('pnpm-lock.yaml')).toBe(false) + expect(containsEntryNamed(process.cwd(), 'PWNED')).toBe(false) +}) + +test('an invalid config dependency version in the workspace manifest is rejected before any lockfile is written', async () => { + prepareEmpty() + const { storeController, storeDir } = createTempStore() + + const integrity = getIntegrity('@pnpm.e2e/foo', '100.0.0') + const configDeps: Record = { + '@pnpm.e2e/foo': `../../../PWNED+${integrity}`, + } + + await expect(installConfigDeps(configDeps, { + registries: { + default: registry, + }, + rootDir: process.cwd(), + store: storeController, + storeDir, + })).rejects.toThrow('invalid version') + + expect(fs.existsSync('pnpm-lock.yaml')).toBe(false) + expect(containsEntryNamed(process.cwd(), 'PWNED')).toBe(false) +}) + +test('a config dependency with a path-traversal version in the env lockfile is rejected', async () => { + prepareEmpty() + const { storeController, storeDir } = createTempStore() + + const maliciousVersion = '../../../PWNED' + const lockfile = makeEnvLockfile({ + '@pnpm.e2e/foo': { version: maliciousVersion, integrity: getIntegrity('@pnpm.e2e/foo', '100.0.0') }, + }) + + await expect(installConfigDeps(lockfile, { + registries: { + default: registry, + }, + rootDir: process.cwd(), + store: storeController, + storeDir, + })).rejects.toThrow('invalid version') + + expect(containsEntryNamed(process.cwd(), 'PWNED')).toBe(false) + expect(containsEntryNamed(storeDir, 'PWNED')).toBe(false) +}) + +test('an optional subdep with a path-traversal version in the env lockfile is rejected', async () => { + prepareEmpty() + const { storeController, storeDir } = createTempStore() + + const parentName = '@pnpm.e2e/foo' + const parentVersion = '100.0.0' + const subdepName = '@pnpm.e2e/bar' + const maliciousVersion = '../../../PWNED' + + const lockfile = createEnvLockfile() + const parentKey = `${parentName}@${parentVersion}` + lockfile.importers['.'].configDependencies[parentName] = { specifier: parentVersion, version: parentVersion } + lockfile.packages[parentKey] = { resolution: { integrity: getIntegrity(parentName, parentVersion) } } + lockfile.snapshots[parentKey] = { + optionalDependencies: { [subdepName]: maliciousVersion }, + } + lockfile.packages[`${subdepName}@${maliciousVersion}`] = { + resolution: { integrity: getIntegrity(subdepName, '100.0.0') }, + } + + await expect(installConfigDeps(lockfile, { + registries: { + default: registry, + }, + rootDir: process.cwd(), + store: storeController, + storeDir, + })).rejects.toThrow('invalid version') + + expect(containsEntryNamed(process.cwd(), 'PWNED')).toBe(false) + expect(containsEntryNamed(storeDir, 'PWNED')).toBe(false) +}) + test('optional subdep matching the current platform is installed and symlinked next to parent', async () => { prepareEmpty() const { storeController, storeDir } = createTempStore() diff --git a/pacquet/crates/env-installer/Cargo.toml b/pacquet/crates/env-installer/Cargo.toml index b819bc0083..35292ebab3 100644 --- a/pacquet/crates/env-installer/Cargo.toml +++ b/pacquet/crates/env-installer/Cargo.toml @@ -11,19 +11,20 @@ license.workspace = true repository.workspace = true [dependencies] -pacquet-config = { workspace = true } -pacquet-diagnostics = { workspace = true } -pacquet-fs = { workspace = true } -pacquet-graph-hasher = { workspace = true } -pacquet-lockfile = { workspace = true } -pacquet-network = { workspace = true } -pacquet-package-is-installable = { workspace = true } -pacquet-package-manager = { workspace = true } -pacquet-reporter = { workspace = true } -pacquet-resolving-resolver-base = { workspace = true } -pacquet-store-dir = { workspace = true } -pacquet-tarball = { workspace = true } -pacquet-workspace-state = { workspace = true } +pacquet-config = { workspace = true } +pacquet-diagnostics = { workspace = true } +pacquet-fs = { workspace = true } +pacquet-graph-hasher = { workspace = true } +pacquet-lockfile = { workspace = true } +pacquet-network = { workspace = true } +pacquet-package-is-installable = { workspace = true } +pacquet-package-manager = { workspace = true } +pacquet-reporter = { workspace = true } +pacquet-resolving-parse-wanted-dependency = { workspace = true } +pacquet-resolving-resolver-base = { workspace = true } +pacquet-store-dir = { workspace = true } +pacquet-tarball = { workspace = true } +pacquet-workspace-state = { workspace = true } derive_more = { workspace = true } futures-util = { workspace = true } diff --git a/pacquet/crates/env-installer/src/errors.rs b/pacquet/crates/env-installer/src/errors.rs index 657752410b..dad60f0983 100644 --- a/pacquet/crates/env-installer/src/errors.rs +++ b/pacquet/crates/env-installer/src/errors.rs @@ -41,6 +41,29 @@ pub enum ConfigDepError { #[diagnostic(code(ERR_PNPM_ENV_LOCKFILE_CORRUPTED))] EnvLockfileCorrupted { message: String }, + /// Mirrors pnpm's `INVALID_DEPENDENCY_NAME`, thrown by + /// [`assertValidDependencyAliases`](https://github.com/pnpm/pnpm/blob/main/installing/deps-resolver/src/validateDependencyAlias.ts). + #[display(r"{description} contains a dependency with an invalid name: {name:?}")] + #[diagnostic( + code(ERR_PNPM_INVALID_DEPENDENCY_NAME), + help( + "A dependency name must be a valid npm package name — a single `name` or `@scope/name` \ + consisting of URL-friendly characters, with no leading `.` or `_`, and not equal to \ + reserved names such as `node_modules`." + ) + )] + InvalidDependencyName { description: String, name: String }, + + /// A config-dependency version is a store path segment + /// (`//`), so a non-semver value is rejected to keep a + /// traversal-shaped version from escaping the store root. + #[display(r#"The config dependency "{name}" has an invalid version "{version}""#)] + #[diagnostic( + code(ERR_PNPM_INVALID_CONFIG_DEP_VERSION), + help("A config dependency version must be an exact semver version.") + )] + InvalidConfigDepVersion { name: String, version: String }, + #[display("Failed to resolve config dependency {spec}: {error}")] #[diagnostic(code(ERR_PNPM_BAD_CONFIG_DEP))] Resolve { diff --git a/pacquet/crates/env-installer/src/install_config_deps.rs b/pacquet/crates/env-installer/src/install_config_deps.rs index fde0b72797..e00b5a547b 100644 --- a/pacquet/crates/env-installer/src/install_config_deps.rs +++ b/pacquet/crates/env-installer/src/install_config_deps.rs @@ -10,6 +10,7 @@ use crate::{ ConfigDepError, NormalizedConfigDep, NormalizedSubdep, options::ConfigDepsInstallOptions, + verify_env_lockfile::verify_env_lockfile, }; use pacquet_graph_hasher::{ calc_global_virtual_store_path_with_subdeps, calc_leaf_global_virtual_store_path, @@ -39,6 +40,7 @@ pub async fn install_config_deps( env_lockfile: &EnvLockfile, opts: &ConfigDepsInstallOptions<'_>, ) -> Result<(), ConfigDepError> { + verify_env_lockfile(env_lockfile)?; let normalized = normalize_from_lockfile(env_lockfile, opts)?; let global_virtual_store_dir = opts.store_dir.links(); let config_modules_dir = opts.root_dir.join("node_modules").join(".pnpm-config"); diff --git a/pacquet/crates/env-installer/src/lib.rs b/pacquet/crates/env-installer/src/lib.rs index c4d938d801..84716c3a0b 100644 --- a/pacquet/crates/env-installer/src/lib.rs +++ b/pacquet/crates/env-installer/src/lib.rs @@ -23,6 +23,7 @@ mod prune; mod resolve_and_install_config_deps; mod resolve_optional_subdeps; mod resolve_package_manager_integrities; +mod verify_env_lockfile; pub use errors::ConfigDepError; pub use install_config_deps::install_config_deps; @@ -34,6 +35,7 @@ pub use resolve_optional_subdeps::resolve_optional_subdeps; pub use resolve_package_manager_integrities::{ is_package_manager_resolved, resolve_package_manager_integrities, }; +pub use verify_env_lockfile::{verify_env_lockfile, write_verified_env_lockfile}; #[cfg(test)] mod tests; diff --git a/pacquet/crates/env-installer/src/resolve_and_install_config_deps.rs b/pacquet/crates/env-installer/src/resolve_and_install_config_deps.rs index 3bda87e19d..02835ed2c4 100644 --- a/pacquet/crates/env-installer/src/resolve_and_install_config_deps.rs +++ b/pacquet/crates/env-installer/src/resolve_and_install_config_deps.rs @@ -13,6 +13,7 @@ use crate::{ ConfigDepError, install_config_deps::install_config_deps, options::ConfigDepsInstallOptions, parse_integrity::parse_integrity, prune::prune_env_lockfile, resolve_optional_subdeps::resolve_optional_subdeps, + verify_env_lockfile::write_verified_env_lockfile, }; use pacquet_lockfile::{ EnvLockfile, LockfileResolution, PackageKey, PackageMetadata, SnapshotEntry, @@ -109,7 +110,7 @@ pub async fn resolve_and_install_config_deps( // Migration and/or removal changed the lockfile; prune any // now-orphaned packages/snapshots before writing. prune_env_lockfile(&mut env_lockfile); - env_lockfile.write(opts.root_dir).map_err(ConfigDepError::WriteLockfile)?; + write_verified_env_lockfile(&env_lockfile, opts.root_dir)?; } return install_config_deps::(&env_lockfile, opts).await; } @@ -119,7 +120,7 @@ pub async fn resolve_and_install_config_deps( } prune_env_lockfile(&mut env_lockfile); - env_lockfile.write(opts.root_dir).map_err(ConfigDepError::WriteLockfile)?; + write_verified_env_lockfile(&env_lockfile, opts.root_dir)?; install_config_deps::(&env_lockfile, opts).await } diff --git a/pacquet/crates/env-installer/src/resolve_package_manager_integrities.rs b/pacquet/crates/env-installer/src/resolve_package_manager_integrities.rs index 945f3190df..73a15d18be 100644 --- a/pacquet/crates/env-installer/src/resolve_package_manager_integrities.rs +++ b/pacquet/crates/env-installer/src/resolve_package_manager_integrities.rs @@ -4,6 +4,7 @@ use crate::{ options::ConfigDepsInstallOptions, prune::prune_env_lockfile, resolve_optional_subdeps::resolution_has_integrity, + verify_env_lockfile::write_verified_env_lockfile, }; use pacquet_lockfile::{ EnvLockfile, PackageKey, PkgName, PkgVerPeer, SnapshotDepRef, SnapshotEntry, @@ -93,7 +94,7 @@ pub async fn resolve_package_manager_integrities( } prune_env_lockfile(&mut env_lockfile); - env_lockfile.write(opts.root_dir).map_err(ConfigDepError::WriteLockfile) + write_verified_env_lockfile(&env_lockfile, opts.root_dir) } #[must_use] diff --git a/pacquet/crates/env-installer/src/tests.rs b/pacquet/crates/env-installer/src/tests.rs index 82cedd1f3f..5508e96c3c 100644 --- a/pacquet/crates/env-installer/src/tests.rs +++ b/pacquet/crates/env-installer/src/tests.rs @@ -1,6 +1,6 @@ use crate::{ - ConfigDepError, ConfigDepsInstallOptions, is_package_manager_resolved, prune_env_lockfile, - resolve_and_install_config_deps, resolve_package_manager_integrities, + ConfigDepError, ConfigDepsInstallOptions, install_config_deps, is_package_manager_resolved, + prune_env_lockfile, resolve_and_install_config_deps, resolve_package_manager_integrities, }; use pacquet_lockfile::{ EnvLockfile, LockfileResolution, PackageKey, PackageMetadata, RegistryResolution, @@ -369,6 +369,277 @@ async fn rejects_optional_subdep_with_non_exact_version() { ); } +/// Recursively search `dir` for an entry named `name`, without following +/// symlinks (so it can't loop through the dir links a successful install leaves). +fn contains_entry_named(dir: &Path, name: &str) -> bool { + let Ok(entries) = std::fs::read_dir(dir) else { + return false; + }; + for entry in entries.flatten() { + if entry.file_name() == name { + return true; + } + if entry.file_type().is_ok_and(|file_type| file_type.is_dir()) + && contains_entry_named(&entry.path(), name) + { + return true; + } + } + false +} + +#[tokio::test] +async fn rejects_config_dep_with_path_traversal_name() { + let harness = harness(); + let (resolver, _cache) = build_resolver(&harness.registry_url); + let root = TempDir::new().unwrap(); + + // Resolve a legit config dep, then re-key its entry under a traversal-shaped + // name to mimic a malicious committed lockfile. + let mut config_deps = BTreeMap::new(); + config_deps.insert("@pnpm.e2e/foo".to_string(), clean_spec("100.0.0")); + resolve_and_install_config_deps::( + &config_deps, + &resolver, + &options(&harness, root.path(), false), + ) + .await + .unwrap(); + + let mut env = EnvLockfile::read(root.path()).unwrap().expect("env lockfile written"); + let spec = env.root_importer_mut().config_dependencies.remove("@pnpm.e2e/foo").unwrap(); + let malicious_name = "../../PWNED_CFGDEP".to_string(); + env.root_importer_mut().config_dependencies.insert(malicious_name.clone(), spec.clone()); + let legit_key: PackageKey = "@pnpm.e2e/foo@100.0.0".parse().unwrap(); + let pkg = env.packages[&legit_key].clone(); + let malicious_key: PackageKey = format!("{malicious_name}@{}", spec.version).parse().unwrap(); + env.packages.insert(malicious_key, pkg); + + let error = install_config_deps::(&env, &options(&harness, root.path(), false)) + .await + .expect_err("a traversal-shaped config dep name must be rejected"); + assert!( + matches!(error, ConfigDepError::InvalidDependencyName { .. }), + "unexpected error: {error:?}", + ); + + assert!(!contains_entry_named(root.path(), "PWNED_CFGDEP")); + assert!(!contains_entry_named(&harness.store_dir.links(), "PWNED_CFGDEP")); +} + +#[tokio::test] +async fn rejects_optional_subdep_with_path_traversal_name() { + let harness = harness(); + let (resolver, _cache) = build_resolver(&harness.registry_url); + let root = TempDir::new().unwrap(); + + let mut config_deps = BTreeMap::new(); + config_deps.insert("@pnpm.e2e/foo".to_string(), clean_spec("100.0.0")); + resolve_and_install_config_deps::( + &config_deps, + &resolver, + &options(&harness, root.path(), false), + ) + .await + .unwrap(); + + let mut env = EnvLockfile::read(root.path()).unwrap().expect("env lockfile written"); + let parent_key: PackageKey = "@pnpm.e2e/foo@100.0.0".parse().unwrap(); + let pkg = env.packages[&parent_key].clone(); + let malicious_name = "../../PWNED_SUBDEP".to_string(); + let malicious_key: PackageKey = format!("{malicious_name}@100.0.0").parse().unwrap(); + env.packages.insert(malicious_key, pkg); + let subdep_name: pacquet_lockfile::PkgName = malicious_name.parse().unwrap(); + let subdep_ref: SnapshotDepRef = "100.0.0".parse().unwrap(); + env.snapshots.entry(parent_key).or_default().optional_dependencies = + Some(std::iter::once((subdep_name, subdep_ref)).collect()); + + let error = install_config_deps::(&env, &options(&harness, root.path(), false)) + .await + .expect_err("a traversal-shaped optional subdep name must be rejected"); + assert!( + matches!(error, ConfigDepError::InvalidDependencyName { .. }), + "unexpected error: {error:?}", + ); + + assert!(!contains_entry_named(root.path(), "PWNED_SUBDEP")); + assert!(!contains_entry_named(&harness.store_dir.links(), "PWNED_SUBDEP")); +} + +/// `__proto__` is an invalid npm name (leading `_`); Rust's string-keyed maps +/// reject it with none of the null-prototype handling the JS side needs. +#[tokio::test] +async fn rejects_config_dep_named_dunder_proto() { + let harness = harness(); + let (resolver, _cache) = build_resolver(&harness.registry_url); + let root = TempDir::new().unwrap(); + + let mut config_deps = BTreeMap::new(); + config_deps.insert("@pnpm.e2e/foo".to_string(), clean_spec("100.0.0")); + resolve_and_install_config_deps::( + &config_deps, + &resolver, + &options(&harness, root.path(), false), + ) + .await + .unwrap(); + + let mut env = EnvLockfile::read(root.path()).unwrap().expect("env lockfile written"); + let spec = env.root_importer_mut().config_dependencies.remove("@pnpm.e2e/foo").unwrap(); + let malicious_name = "__proto__".to_string(); + env.root_importer_mut().config_dependencies.insert(malicious_name.clone(), spec.clone()); + let legit_key: PackageKey = "@pnpm.e2e/foo@100.0.0".parse().unwrap(); + let pkg = env.packages[&legit_key].clone(); + let malicious_key: PackageKey = format!("{malicious_name}@{}", spec.version).parse().unwrap(); + env.packages.insert(malicious_key, pkg); + + let error = install_config_deps::(&env, &options(&harness, root.path(), false)) + .await + .expect_err("a config dep named __proto__ must be rejected"); + assert!( + matches!(error, ConfigDepError::InvalidDependencyName { .. }), + "unexpected error: {error:?}", + ); +} + +#[tokio::test] +async fn rejects_invalid_manifest_config_dep_name_before_writing_lockfile() { + let harness = harness(); + let (resolver, _cache) = build_resolver(&harness.registry_url); + let root = TempDir::new().unwrap(); + + let mut config_deps = BTreeMap::new(); + config_deps.insert( + "../../PWNED".to_string(), + ConfigDependency::VersionWithIntegrity("100.0.0+sha512-deadbeef".to_string()), + ); + + let error = resolve_and_install_config_deps::( + &config_deps, + &resolver, + &options(&harness, root.path(), false), + ) + .await + .expect_err("an invalid manifest config dep name must be rejected"); + assert!( + matches!(error, ConfigDepError::InvalidDependencyName { .. }), + "unexpected error: {error:?}", + ); + + assert!(!root.path().join("pnpm-lock.yaml").exists()); +} + +#[tokio::test] +async fn rejects_invalid_manifest_config_dep_version_before_writing_lockfile() { + let harness = harness(); + let (resolver, _cache) = build_resolver(&harness.registry_url); + let root = TempDir::new().unwrap(); + + let integrity = integrity_of(&resolver, "@pnpm.e2e/foo", "100.0.0").await; + let mut config_deps = BTreeMap::new(); + config_deps.insert( + "@pnpm.e2e/foo".to_string(), + ConfigDependency::VersionWithIntegrity(format!("../../../PWNED+{integrity}")), + ); + + let error = resolve_and_install_config_deps::( + &config_deps, + &resolver, + &options(&harness, root.path(), false), + ) + .await + .expect_err("an invalid manifest config dep version must be rejected"); + assert!( + matches!(error, ConfigDepError::InvalidConfigDepVersion { .. }), + "unexpected error: {error:?}", + ); + + assert!(!root.path().join("pnpm-lock.yaml").exists()); +} + +#[tokio::test] +async fn rejects_config_dep_with_path_traversal_version() { + let harness = harness(); + let (resolver, _cache) = build_resolver(&harness.registry_url); + let root = TempDir::new().unwrap(); + + let mut config_deps = BTreeMap::new(); + config_deps.insert("@pnpm.e2e/foo".to_string(), clean_spec("100.0.0")); + resolve_and_install_config_deps::( + &config_deps, + &resolver, + &options(&harness, root.path(), false), + ) + .await + .unwrap(); + + let mut env = EnvLockfile::read(root.path()).unwrap().expect("env lockfile written"); + let malicious_version = "../../../PWNED"; + env.root_importer_mut().config_dependencies.get_mut("@pnpm.e2e/foo").unwrap().version = + malicious_version.to_string(); + let legit_key: PackageKey = "@pnpm.e2e/foo@100.0.0".parse().unwrap(); + let pkg = env.packages[&legit_key].clone(); + let malicious_key: PackageKey = format!("@pnpm.e2e/foo@{malicious_version}").parse().unwrap(); + env.packages.insert(malicious_key, pkg); + + let error = install_config_deps::(&env, &options(&harness, root.path(), false)) + .await + .expect_err("a traversal-shaped config dep version must be rejected"); + assert!( + matches!(error, ConfigDepError::InvalidConfigDepVersion { .. }), + "unexpected error: {error:?}", + ); + // Pin the message format (guards against a doubled/dropped quote). + let message = error.to_string(); + assert_eq!( + message, + r#"The config dependency "@pnpm.e2e/foo" has an invalid version "../../../PWNED""#, + ); + + assert!(!contains_entry_named(root.path(), "PWNED")); + assert!(!contains_entry_named(&harness.store_dir.links(), "PWNED")); +} + +#[tokio::test] +async fn rejects_optional_subdep_with_path_traversal_version() { + let harness = harness(); + let (resolver, _cache) = build_resolver(&harness.registry_url); + let root = TempDir::new().unwrap(); + + let mut config_deps = BTreeMap::new(); + config_deps.insert("@pnpm.e2e/foo".to_string(), clean_spec("100.0.0")); + resolve_and_install_config_deps::( + &config_deps, + &resolver, + &options(&harness, root.path(), false), + ) + .await + .unwrap(); + + let mut env = EnvLockfile::read(root.path()).unwrap().expect("env lockfile written"); + let parent_key: PackageKey = "@pnpm.e2e/foo@100.0.0".parse().unwrap(); + let pkg = env.packages[&parent_key].clone(); + let malicious_version = "../../../PWNED"; + let subdep_name = "@pnpm.e2e/bar"; + let malicious_key: PackageKey = format!("{subdep_name}@{malicious_version}").parse().unwrap(); + env.packages.insert(malicious_key, pkg); + let subdep_name_parsed: pacquet_lockfile::PkgName = subdep_name.parse().unwrap(); + let subdep_ref: SnapshotDepRef = malicious_version.parse().unwrap(); + env.snapshots.entry(parent_key).or_default().optional_dependencies = + Some(std::iter::once((subdep_name_parsed, subdep_ref)).collect()); + + let error = install_config_deps::(&env, &options(&harness, root.path(), false)) + .await + .expect_err("a traversal-shaped optional subdep version must be rejected"); + assert!( + matches!(error, ConfigDepError::InvalidConfigDepVersion { .. }), + "unexpected error: {error:?}", + ); + + assert!(!contains_entry_named(root.path(), "PWNED")); + assert!(!contains_entry_named(&harness.store_dir.links(), "PWNED")); +} + #[tokio::test] async fn frozen_lockfile_rejects_new_config_dep() { let harness = harness(); diff --git a/pacquet/crates/env-installer/src/verify_env_lockfile.rs b/pacquet/crates/env-installer/src/verify_env_lockfile.rs new file mode 100644 index 0000000000..01374c0283 --- /dev/null +++ b/pacquet/crates/env-installer/src/verify_env_lockfile.rs @@ -0,0 +1,75 @@ +//! Offline structural gate for the env lockfile, mirroring pnpm's +//! [`verifyEnvLockfile`](https://github.com/pnpm/pnpm/blob/main/installing/env-installer/src/verifyEnvLockfile.ts) +//! and the always-on alias/shape checks `verifyLockfileResolutions` runs over +//! the main lockfile. + +use crate::ConfigDepError; +use pacquet_lockfile::{EnvLockfile, PackageKey}; +use pacquet_resolving_parse_wanted_dependency::is_valid_old_npm_package_name; +use std::path::Path; + +/// Persist an env lockfile only after verifying it, so no code path can write +/// one carrying an invalid config-dependency name or version. +pub fn write_verified_env_lockfile( + env_lockfile: &EnvLockfile, + root_dir: &Path, +) -> Result<(), ConfigDepError> { + verify_env_lockfile(env_lockfile)?; + env_lockfile.write(root_dir).map_err(ConfigDepError::WriteLockfile) +} + +/// Reject config-dependency and optional-subdep names/versions before they +/// build store paths (`//`): names must be valid npm +/// package names, versions exact semver — otherwise a traversal-shaped value +/// would escape the install roots. +pub fn verify_env_lockfile(env_lockfile: &EnvLockfile) -> Result<(), ConfigDepError> { + let Some(importer) = env_lockfile.importers.get(EnvLockfile::ROOT_IMPORTER_KEY) else { + return Ok(()); + }; + for (name, spec) in &importer.config_dependencies { + assert_valid_name(name, "The configDependencies in pnpm-lock.yaml")?; + assert_valid_version(name, &spec.version)?; + + let Ok(key) = format!("{name}@{}", spec.version).parse::() else { + continue; + }; + let Some(optionals) = env_lockfile + .snapshots + .get(&key) + .and_then(|snapshot| snapshot.optional_dependencies.as_ref()) + else { + continue; + }; + let description = + format!("The optionalDependencies of config dependency \"{name}\" in pnpm-lock.yaml"); + for (subdep_name, dep_ref) in optionals { + let subdep_name = subdep_name.to_string(); + assert_valid_name(&subdep_name, &description)?; + let version = dep_ref.ver_peer().map(ToString::to_string).unwrap_or_default(); + assert_valid_version(&subdep_name, &version)?; + } + } + Ok(()) +} + +fn assert_valid_name(name: &str, description: &str) -> Result<(), ConfigDepError> { + if is_valid_old_npm_package_name(name) { + Ok(()) + } else { + Err(ConfigDepError::InvalidDependencyName { + description: description.to_string(), + name: name.to_string(), + }) + } +} + +fn assert_valid_version(name: &str, version: &str) -> Result<(), ConfigDepError> { + if version.parse::().is_err() { + Err(ConfigDepError::InvalidConfigDepVersion { + name: name.to_string(), + version: version.to_string(), + }) + } else { + Ok(()) + } +}