From 7e80e9cb94e7e4182745388eeccfcf6573f6a470 Mon Sep 17 00:00:00 2001 From: vian <85208522+vianmangal@users.noreply.github.com> Date: Tue, 10 Nov 2026 04:11:42 +0530 Subject: [PATCH 01/21] Merge commit from fork * fix(security): restrict repository registry destinations * test: use typed fetch responses in registry tests * docs: describe registry destination trust boundary * fix(security): enforce registry policy in override audits (cherry picked from commit fe909d68aaef1c640639f3fdf9a88d5aace4690a) --- src/overrides/parsing/registry.ts | 89 +++++------------- src/remediation/npm-registry.ts | 14 ++- src/scan/override-audit.ts | 10 ++- src/scanner.ts | 7 ++ src/utils/checked-address-policy.ts | 48 ++++++++++ src/utils/network.ts | 1 + src/utils/osv-fetch.ts | 64 +++---------- src/utils/registry-config.ts | 14 ++- src/utils/registry-fetch.ts | 49 ++++++++++ tests/npm-registry.test.ts | 46 +++++++--- tests/override-audit.test.ts | 31 +++++++ tests/overrides/admitted-scan.test.ts | 2 + tests/registry-fetch.test.ts | 115 ++++++++++++++++++++++++ tests/utils/registry-config.test.ts | 35 ++++++++ website/docs/corporate-proxy.md | 23 +++++ website/docs/security-assurance-case.md | 4 +- 16 files changed, 412 insertions(+), 140 deletions(-) create mode 100644 src/utils/checked-address-policy.ts create mode 100644 src/utils/registry-fetch.ts create mode 100644 tests/registry-fetch.test.ts diff --git a/src/overrides/parsing/registry.ts b/src/overrides/parsing/registry.ts index 855bedbb9..c2a53b994 100644 --- a/src/overrides/parsing/registry.ts +++ b/src/overrides/parsing/registry.ts @@ -1,5 +1,6 @@ import type { RegistryDistTags } from '../context.js'; import type { FetchLike } from '../../types.js'; +import { fetchRegistryMetadata } from '../../utils/registry-fetch.js'; import { compareVersions, isPreReleaseVersion } from '../../utils/version.js'; /** @@ -55,21 +56,12 @@ export interface RegistryClientOptions { baseUrl?: string; /** Per-request timeout in ms. Default 5000. */ timeoutMs?: number; - /** Inject a fetch implementation for tests. */ + /** Private access is only allowed for trusted user/environment configuration. */ + allowPrivate?: boolean; + /** Fetch implementation for built-in or trusted registries. */ fetchImpl?: FetchLike; } -/** - * Fetch the `dist-tags` map for `pkgName` from the npm registry. - * - * Returns null on any failure (network error, non-200, parse error, timeout). - * The caller surfaces a `skippedDetectors` note when this returns null for a - * rule that depends on registry data. - * - * Per scan, callers should batch-fetch all needed packages in parallel and - * cache the result on `Context.registryDistTags`. This function is single-shot; - * caching/batching is the scanner's responsibility. - */ /** Shared request shape for both lookups. */ async function packumentOutcome( pkgName: string, @@ -78,19 +70,18 @@ async function packumentOutcome( ): Promise> { const baseUrl = opts.baseUrl ?? 'https://registry.npmjs.org'; const timeoutMs = opts.timeoutMs ?? 5000; - const fetchImpl = opts.fetchImpl ?? fetch; - const url = `${baseUrl}/${encodeRegistryPath(pkgName)}`; + const url = `${baseUrl.replace(/\/+$/, "")}/${encodeRegistryPath(pkgName)}`; const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), timeoutMs); try { return await classify( - () => fetchImpl(url, { + () => fetchRegistryMetadata(baseUrl, url, opts.allowPrivate ?? false, { signal: controller.signal, headers: { Accept: 'application/vnd.npm.install-v1+json', 'User-Agent': 'override-audit-cli', }, - }) as Promise, + }, opts.fetchImpl), parse, ); } finally { @@ -115,34 +106,23 @@ async function versionsOutcome(pkgName: string, opts: RegistryClientOptions) { }); } +/** + * Fetch the `dist-tags` map for `pkgName` from the npm registry. + * + * Returns null on any failure (network error, non-200, parse error, timeout). + * The caller surfaces a `skippedDetectors` note when this returns null for a + * rule that depends on registry data. + * + * Per scan, callers should batch-fetch all needed packages in parallel and + * cache the result on `Context.registryDistTags`. This function is single-shot; + * caching/batching is the scanner's responsibility. + */ export async function fetchDistTags( pkgName: string, opts: RegistryClientOptions = {}, ): Promise { - const baseUrl = opts.baseUrl ?? 'https://registry.npmjs.org'; - const timeoutMs = opts.timeoutMs ?? 5000; - const fetchImpl = opts.fetchImpl ?? fetch; - - const url = `${baseUrl}/${encodeRegistryPath(pkgName)}`; - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), timeoutMs); - - try { - const res = await fetchImpl(url, { - signal: controller.signal, - headers: { - Accept: 'application/vnd.npm.install-v1+json', - 'User-Agent': 'override-audit-cli', - }, - }); - if (!res.ok) return null; - const body = await res.json() as { 'dist-tags'?: Record }; - return body['dist-tags'] ?? null; - } catch { - return null; - } finally { - clearTimeout(timer); - } + const outcome = await distTagsOutcome(pkgName, opts); + return outcome.kind === "ok" ? outcome.value : null; } /** @@ -179,33 +159,8 @@ export async function fetchVersions( pkgName: string, opts: RegistryClientOptions = {}, ): Promise { - const baseUrl = opts.baseUrl ?? 'https://registry.npmjs.org'; - const timeoutMs = opts.timeoutMs ?? 5000; - const fetchImpl = opts.fetchImpl ?? fetch; - - const url = `${baseUrl}/${encodeRegistryPath(pkgName)}`; - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), timeoutMs); - - try { - const res = await fetchImpl(url, { - signal: controller.signal, - headers: { - Accept: 'application/vnd.npm.install-v1+json', - 'User-Agent': 'override-audit-cli', - }, - }); - if (!res.ok) return null; - const body = await res.json() as { versions?: Record }; - if (!body.versions) return null; - return Object.keys(body.versions) - .filter((v) => !isPreReleaseVersion(v)) - .sort(compareVersions); - } catch { - return null; - } finally { - clearTimeout(timer); - } + const outcome = await versionsOutcome(pkgName, opts); + return outcome.kind === "ok" ? outcome.value : null; } /** diff --git a/src/remediation/npm-registry.ts b/src/remediation/npm-registry.ts index d72093e32..9e75aa73b 100644 --- a/src/remediation/npm-registry.ts +++ b/src/remediation/npm-registry.ts @@ -3,6 +3,7 @@ import type { OsvVuln } from "../types.js"; import type { DebugLogger } from "../output/debug.js"; import { registryForPackage, type RegistryConfig } from "../utils/registry-config.js"; import type { AdvisorySource } from "../advisory/advisory-source.js"; +import { fetchRegistryMetadata, RegistryNetworkPolicyError } from "../utils/registry-fetch.js"; import { AsyncLocalStorage } from "node:async_hooks"; export type Packument = { @@ -40,7 +41,7 @@ export type PublishedFixVersionResolution = { type PackumentFetchResult = | { status: "resolved"; packument: Packument } | { status: "not-found"; packument: null } - | { status: "transient-failure"; packument: null }; + | { status: "transient-failure"; packument: null; policyBlocked?: boolean }; const packumentCache = new Map>(); const DEFAULT_NPM_REGISTRY_URL = "https://registry.npmjs.org"; @@ -60,6 +61,7 @@ function nextRegistryBatchId(): string { export type RegistryFetchObserver = { onTransientFailure?: (name: string) => void; + onPolicyBlocked?: () => void; onResolved?: (name: string) => void; }; @@ -80,6 +82,7 @@ function notifyPackumentObserver(result: PackumentFetchResult, packageName: stri const observer = observerStorage.getStore(); if (result.status === "transient-failure") { observer?.onTransientFailure?.(packageName); + if (result.policyBlocked) observer?.onPolicyBlocked?.(); } else { observer?.onResolved?.(packageName); } @@ -122,6 +125,7 @@ let registryConfig: RegistryConfig | null = null; */ export function configureNpmRegistry(config: RegistryConfig | null): void { registryConfig = config; + clearPackumentCache(); } /** @@ -163,7 +167,7 @@ async function doFetchPackument(packageName: string): Promise(); + let registryPolicyBlocked = false; const remediationFailures = new Map(); let chainResolutionFailureCount = 0; let directCount = 0; @@ -623,6 +624,7 @@ export async function scanPackages( await runWithObserver({ onTransientFailure(name) { packumentFailures.add(name); }, + onPolicyBlocked() { registryPolicyBlocked = true; }, }, async () => { const npmRemediationCache = new Map(); const parentUpgradeCache = new Map(); @@ -841,6 +843,11 @@ export async function scanPackages( } }); + if (registryPolicyBlocked && registryConfig.sourceFile) { + const source = path.basename(registryConfig.sourceFile); + console.warn(`Warning: The registry from the project's ${source} was blocked because repository settings cannot use plain HTTP or private addresses. Set NPM_CONFIG_REGISTRY to a trusted registry for this scan.`); + } + const severityCounts = countBySeverity(findings); log("Findings classified", { total: findings.length, diff --git a/src/utils/checked-address-policy.ts b/src/utils/checked-address-policy.ts new file mode 100644 index 000000000..bb33877f5 --- /dev/null +++ b/src/utils/checked-address-policy.ts @@ -0,0 +1,48 @@ +import dns from "node:dns"; +import { isIP, type LookupFunction } from "node:net"; +import { isPrivateOrReservedIp, stripIpv6Brackets } from "./validate-url.js"; + +export function createCheckedAddressPolicy(options: { + allowPrivate: boolean; + invalidAddressMessage: string; + privateAddressError: () => Error; + missingFamilyMessage: string; +}): { lookup: LookupFunction; checkLiteral: (hostname: string) => void } { + function checkAddresses(addresses: dns.LookupAddress[]): void { + if (!addresses.length || addresses.some(({ address }) => !isIP(address))) { + throw new Error(options.invalidAddressMessage); + } + if (!options.allowPrivate && addresses.some(({ address }) => isPrivateOrReservedIp(address))) { + throw options.privateAddressError(); + } + } + + // Validate all DNS answers and return only those same addresses to the socket. + const lookup: LookupFunction = (hostname, requestOptions, callback) => { + dns.lookup(hostname, { all: true }, (error, addresses) => { + if (error) { + callback(error, "", 0); + return; + } + try { + checkAddresses(addresses); + const family = requestOptions.family; + const candidates = family ? addresses.filter(address => address.family === family) : addresses; + if (!candidates.length) throw new Error(options.missingFamilyMessage); + if (requestOptions.all) callback(null, candidates); + else callback(null, candidates[0].address, candidates[0].family); + } catch (failure) { + callback(failure instanceof Error ? failure : new Error(String(failure)), "", 0); + } + }); + }; + + function checkLiteral(hostname: string): void { + // Node bypasses lookup callbacks for IP literals. + const address = stripIpv6Brackets(hostname); + const family = isIP(address); + if (family) checkAddresses([{ address, family }]); + } + + return { lookup, checkLiteral }; +} diff --git a/src/utils/network.ts b/src/utils/network.ts index ca934e00d..aaedf64ab 100644 --- a/src/utils/network.ts +++ b/src/utils/network.ts @@ -221,6 +221,7 @@ export function createCertAwareFetch(caCertPath?: string, lookup?: LookupFunctio headers, agent: getAgent(), lookup, + signal: init?.signal ?? undefined, }; const req = https.request(reqOptions, res => { diff --git a/src/utils/osv-fetch.ts b/src/utils/osv-fetch.ts index 14aa2f740..fb34a9ce1 100644 --- a/src/utils/osv-fetch.ts +++ b/src/utils/osv-fetch.ts @@ -1,48 +1,8 @@ -import dns from "node:dns"; import { FLAG_ALLOW_PRIVATE_OSV_URL, FLAG_OSV_URL } from "../cli/flags.js"; -import { isIP, type LookupFunction } from "node:net"; import type { FetchLike } from "../types.js"; +import { createCheckedAddressPolicy } from "./checked-address-policy.js"; import { createCertAwareFetch } from "./network.js"; -import { isOfficialOsv, isPrivateOrReservedIp, stripIpv6Brackets } from "./validate-url.js"; - -function rejectPrivateAddresses(addresses: dns.LookupAddress[], allowPrivate: boolean): void { - if (addresses.length === 0 || addresses.some(address => !isIP(address.address))) { - throw new Error(`${FLAG_OSV_URL} did not resolve to a valid IP address`); - } - if (!allowPrivate && addresses.some(address => isPrivateOrReservedIp(address.address))) { - throw new Error( - `${FLAG_OSV_URL} resolves to a private/reserved IP at connection time. ` + - `Use ${FLAG_ALLOW_PRIVATE_OSV_URL} for internal mirrors.`, - ); - } -} - -/** Return only the addresses that were checked, without another resolver call. */ -function createOsvLookup(allowPrivate: boolean): LookupFunction { - return (hostname, options, callback) => { - dns.lookup(hostname, { all: true }, (error, addresses) => { - if (error) { - callback(error, "", 0); - return; - } - try { - rejectPrivateAddresses(addresses, allowPrivate); - const family = options.family; - const candidates = family ? addresses.filter(address => address.family === family) : addresses; - if (candidates.length === 0) { - throw new Error(`${FLAG_OSV_URL} has no address for the requested IP family`); - } - if (options.all) { - callback(null, candidates); - } else { - callback(null, candidates[0].address, candidates[0].family); - } - } catch (failure) { - callback(failure as Error, "", 0); - } - }); - }; -} +import { isOfficialOsv } from "./validate-url.js"; /** Protect only the configured OSV origin; registry and other fetches keep their transport. */ export function createOsvFetch(osvUrl: string, allowPrivate: boolean, caCertPath?: string): FetchLike { @@ -51,7 +11,16 @@ export function createOsvFetch(osvUrl: string, allowPrivate: boolean, caCertPath return fallback; } // Parse lazily: validateOptions owns invalid-URL diagnostics before any request. - const protectedFetch = createCertAwareFetch(caCertPath, createOsvLookup(allowPrivate)); + const addressPolicy = createCheckedAddressPolicy({ + allowPrivate, + invalidAddressMessage: `${FLAG_OSV_URL} did not resolve to a valid IP address`, + privateAddressError: () => new Error( + `${FLAG_OSV_URL} resolves to a private/reserved IP at connection time. ` + + `Use ${FLAG_ALLOW_PRIVATE_OSV_URL} for internal mirrors.`, + ), + missingFamilyMessage: `${FLAG_OSV_URL} has no address for the requested IP family`, + }); + const protectedFetch = createCertAwareFetch(caCertPath, addressPolicy.lookup); return async (input, init) => { const target = new URL(osvUrl); const url = new URL(typeof input === "string" ? input : input instanceof URL ? input.href : input.url); @@ -61,14 +30,9 @@ export function createOsvFetch(osvUrl: string, allowPrivate: boolean, caCertPath if (url.protocol !== "https:") { throw new Error(`${FLAG_OSV_URL} requires https:// scheme`); } - // Node bypasses lookup callbacks for literals, so enforce the same policy here. - const hostname = stripIpv6Brackets(url.hostname); - const family = isIP(hostname); - if (family) { - rejectPrivateAddresses([{ address: hostname, family }], allowPrivate); - } + addressPolicy.checkLiteral(url.hostname); // The HTTPS transport never follows redirects. It retains the hostname for // Host, SNI and certificate validation, and validates DNS for each new socket. - return protectedFetch(input, init); + return protectedFetch(url, init); }; } diff --git a/src/utils/registry-config.ts b/src/utils/registry-config.ts index ba40b9c01..df987313a 100644 --- a/src/utils/registry-config.ts +++ b/src/utils/registry-config.ts @@ -31,6 +31,8 @@ export type RegistryConfig = { scopes: Record; /** The file the registry came from, for reporting. Null when nothing was found. */ sourceFile: string | null; + /** Only user/environment configuration may authorize internal destinations. */ + allowPrivate?: boolean; }; export type RegistryConfigPackageManager = "npm" | "pnpm" | "yarn" | "bun"; @@ -132,7 +134,7 @@ export function resolveRegistryConfig(args: { const envRegistry = env.NPM_CONFIG_REGISTRY || env.npm_config_registry; if (envRegistry) { - return { registry: envRegistry, scopes: {}, sourceFile: "NPM_CONFIG_REGISTRY" }; + return { registry: envRegistry, scopes: {}, sourceFile: "NPM_CONFIG_REGISTRY", allowPrivate: true }; } const config = args.packageManager ? MANAGER_CONFIG[args.packageManager] : undefined; @@ -152,6 +154,9 @@ export function resolveRegistryConfig(args: { registry: parsed.registry || NPM_PUBLIC_REGISTRY, scopes: parsed.scopes, sourceFile: file, + // A project scanned at home (or a home config linked into the project) + // must not promote repository-owned settings to user authorization. + allowPrivate: dir === homeDir && !isInsideProject(file, args.projectPath), }; } @@ -168,3 +173,10 @@ export function registryForPackage(config: RegistryConfig, packageName: string): } return config.registry; } + +function isInsideProject(file: string, projectPath: string): boolean { + try { + const relative = path.relative(fs.realpathSync(projectPath), fs.realpathSync(file)); + return relative === "" || (!relative.startsWith(`..${path.sep}`) && relative !== ".." && !path.isAbsolute(relative)); + } catch { return true; } // Uncertain ownership must not authorize private access. +} diff --git a/src/utils/registry-fetch.ts b/src/utils/registry-fetch.ts new file mode 100644 index 000000000..192c67197 --- /dev/null +++ b/src/utils/registry-fetch.ts @@ -0,0 +1,49 @@ +import type { FetchLike } from "../types.js"; +import { createCheckedAddressPolicy } from "./checked-address-policy.js"; +import { createCertAwareFetch } from "./network.js"; + +export class RegistryNetworkPolicyError extends Error {} + +const publicRegistryPolicy = createCheckedAddressPolicy({ + allowPrivate: false, + invalidAddressMessage: "Repository registry resolves to a private/reserved or invalid address", + privateAddressError: () => new RegistryNetworkPolicyError("Repository registry resolves to a private/reserved address"), + missingFamilyMessage: "Registry has no address for the requested IP family", +}); + +/** Repository settings select metadata sources, not permission to access internal services. */ +export async function fetchRegistryMetadata( + base: string, + requestUrl: string, + allowPrivate: boolean, + init?: RequestInit, + fetchImpl?: FetchLike, +): Promise { + const registry = new URL(base); + if (registry.username || registry.password || registry.search || registry.hash || base.includes("?") || base.includes("#")) { + throw new Error("Registry URL must not contain credentials, a query or a fragment"); + } + if (registry.protocol === "http:" && !allowPrivate) { + throw new RegistryNetworkPolicyError("Repository registry requires HTTPS"); + } + if (registry.protocol !== "https:" && registry.protocol !== "http:") { + throw new Error("Registry URL must use HTTP or HTTPS"); + } + const target = new URL(requestUrl); + if (target.origin !== registry.origin) throw new Error("Registry request changed origin"); + + const ca = process.env.NODE_EXTRA_CA_CERTS; + const official = registry.origin === "https://registry.npmjs.org"; + let transport: FetchLike; + if (allowPrivate || official) { + transport = fetchImpl ?? (ca && registry.protocol === "https:" ? createCertAwareFetch(ca) : fetch); + } else { + publicRegistryPolicy.checkLiteral(target.hostname); + transport = createCertAwareFetch(ca, publicRegistryPolicy.lookup); + } + + // Use the canonical URL so alternate HTTPS spellings still use the checked transport. + const response = await transport(target.href, { ...init, redirect: "error" }); + if (response.status >= 300 && response.status < 400) throw new Error("Registry redirects are not permitted"); + return response; +} diff --git a/tests/npm-registry.test.ts b/tests/npm-registry.test.ts index cc913aae7..611ebfa13 100644 --- a/tests/npm-registry.test.ts +++ b/tests/npm-registry.test.ts @@ -1,16 +1,13 @@ import { jest } from "@jest/globals"; import { clearPackumentCache, configureNpmRegistry, fetchPackument } from "../src/remediation/npm-registry.js"; -const fetchMock = jest.fn(); -global.fetch = fetchMock as unknown as typeof fetch; +const fetchMock = jest.fn(); +global.fetch = fetchMock; function mockRegistry(versions: string[] = []) { - fetchMock.mockResolvedValue({ - ok: true, - json: async () => ({ - versions: Object.fromEntries(versions.map((v) => [v, {}])), - }), - }); + fetchMock.mockResolvedValue(new Response(JSON.stringify({ + versions: Object.fromEntries(versions.map(v => [v, {}])), + }))); } describe("fetchPackument", () => { @@ -61,11 +58,7 @@ describe("fetchPackument", () => { }); it("returns null for non-ok responses without caching null as rejection", async () => { - fetchMock.mockResolvedValue({ - ok: false, - status: 404, - statusText: "Not Found", - }); + fetchMock.mockResolvedValue(new Response(null, { status: 404, statusText: "Not Found" })); const result = await fetchPackument("no-such-pkg"); expect(result).toBeNull(); @@ -104,6 +97,7 @@ describe("fetchPackument registry selection", () => { registry: "https://nexus.internal/repository/npm-group/", scopes: {}, sourceFile: "/tmp/.npmrc", + allowPrivate: true, }); await fetchPackument("lodash"); @@ -118,9 +112,35 @@ describe("fetchPackument registry selection", () => { registry: "https://nexus.internal/npm/", scopes: { "@acme": "https://acme.internal/npm/" }, sourceFile: "/tmp/.npmrc", + allowPrivate: true, }); await fetchPackument("@acme/widget"); expect(String(fetchMock.mock.calls[0]?.[0])).toContain("https://acme.internal/npm/"); }); }); + + +describe("repository registry request boundary", () => { + afterEach(() => { configureNpmRegistry(null); }); + it.each(["http://127.0.0.1/", "https://127.0.0.1/", "https://mirror.test/action?query=yes#"])("blocks %s before fetching", async registry => { + fetchMock.mockReset(); + configureNpmRegistry({ registry, scopes: {}, sourceFile: "/project/.npmrc" }); + await expect(fetchPackument("axios")).resolves.toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + it("blocks untrusted scoped destinations too", async () => { + fetchMock.mockReset(); + configureNpmRegistry({ registry: "https://registry.npmjs.org", scopes: { "@acme": "http://127.0.0.1/" }, sourceFile: "/project/.npmrc" }); + await expect(fetchPackument("@acme/widget")).resolves.toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + it("does not reuse packuments after the configured source changes", async () => { + mockRegistry(["1.0.0"]); + configureNpmRegistry(null); + expect(await fetchPackument("axios")).not.toBeNull(); + configureNpmRegistry({ registry: "http://127.0.0.1/", scopes: {}, sourceFile: "/project/.npmrc" }); + expect(await fetchPackument("axios")).toBeNull(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/override-audit.test.ts b/tests/override-audit.test.ts index 4a74fd8d3..92a155a1a 100644 --- a/tests/override-audit.test.ts +++ b/tests/override-audit.test.ts @@ -3,6 +3,8 @@ // reverted with the whole suite green. import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { runOverrideAudit } from "../src/scan/override-audit.js"; @@ -64,6 +66,35 @@ describe("runOverrideAudit (#1222)", () => { expect(spy.urls).toEqual([]); }); + it("blocks a project-owned loopback registry for both network rules", async () => { + project(); + let connections = 0; + const server = createServer((_req, res) => { + res.setHeader("Connection", "close"); + res.end(JSON.stringify({ "dist-tags": { latest: "1.2.8" }, versions: { "1.2.8": {} } })); + }); + server.on("connection", () => { connections++; }); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const port = (server.address() as AddressInfo).port; + const keys = ["NPM_CONFIG_REGISTRY", "npm_config_registry"] as const; + const original = keys.map(key => process.env[key]); + try { + for (const key of keys) delete process.env[key]; + writeFileSync(join(dir, ".npmrc"), `registry=http://127.0.0.1:${port}/\n`); + const outcome = await runOverrideAudit({ ...base(), checkNetwork: true, scanSource: "package-lock" }); + expect(connections).toBe(0); + const failed = outcome.skipped.filter(s => /registry lookups failed/.test(s.reason)); + expect(failed.map(s => s.ruleId).sort()).toEqual(["OA007", "OA010"]); + } finally { + for (const [index, key] of keys.entries()) { + const value = original[index]; + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + await new Promise(resolve => server.close(() => resolve())); + } + }); + it("reports OA007 and OA010 as skipped when the opt-in is absent", async () => { project(); const outcome = await runOverrideAudit({ ...base(), checkNetwork: false }); diff --git a/tests/overrides/admitted-scan.test.ts b/tests/overrides/admitted-scan.test.ts index e475ca05b..137c097ef 100644 --- a/tests/overrides/admitted-scan.test.ts +++ b/tests/overrides/admitted-scan.test.ts @@ -14,6 +14,7 @@ function ctxOf(entries: OverrideEntry[]): OverrideContext { packageManager: 'pnpm', overrideEntries: entries, lockfilePackageNames: new Set(), + lockfileVersions: new Map(), installedVersions: new Map(), installedCopies: new Map(), parentDeclarations: new Map(), @@ -71,6 +72,7 @@ function advisoryStub( const registry = (versionsByPkg: Record) => ({ baseUrl: 'https://registry.test', + allowPrivate: true, fetchImpl: fetchStub(versionsByPkg) as any, }); diff --git a/tests/registry-fetch.test.ts b/tests/registry-fetch.test.ts new file mode 100644 index 000000000..fed85c18c --- /dev/null +++ b/tests/registry-fetch.test.ts @@ -0,0 +1,115 @@ +import dns from "node:dns"; +import https from "node:https"; +import fs from "node:fs"; +import { fileURLToPath } from "node:url"; +import type { AddressInfo, LookupFunction } from "node:net"; +import { jest } from "@jest/globals"; +import { fetchRegistryMetadata } from "../src/utils/registry-fetch.js"; + +const cert = fileURLToPath(new URL("./fixtures/tls/osv-test-cert.pem", import.meta.url)); +const key = fileURLToPath(new URL("./fixtures/tls/osv-test-key.pem", import.meta.url)); +const publicAddress = { address: "93.184.216.34", family: 4 }; +const privateAddress = { address: "127.0.0.1", family: 4 }; +function answer(addresses: dns.LookupAddress[]) { + return jest.spyOn(dns, "lookup").mockImplementation((( + _host: string, _options: dns.LookupOptions, + callback: (error: Error | null, result: dns.LookupAddress[]) => void, + ) => callback(null, addresses)) as typeof dns.lookup); +} + +describe("registry destination policy", () => { + let server: https.Server; + let base: string; + let connections = 0; + let requests: string[] = []; + const previousCa = process.env.NODE_EXTRA_CA_CERTS; + beforeAll(async () => { + server = https.createServer({ key: fs.readFileSync(key), cert: fs.readFileSync(cert) }, (req, res) => { + requests.push(req.url ?? ""); + res.setHeader("Connection", "close"); + if (req.url === "/redirect") res.writeHead(302, { Location: "/unexpected" }); + res.end('{"versions":{"1.0.0":{}}}'); + }); + server.on("connection", () => { connections++; }); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + base = `https://mirror.test:${(server.address() as AddressInfo).port}`; + }); + beforeEach(() => { connections = 0; requests = []; delete process.env.NODE_EXTRA_CA_CERTS; }); + afterEach(() => { jest.restoreAllMocks(); }); + afterAll(async () => { + if (previousCa === undefined) delete process.env.NODE_EXTRA_CA_CERTS; + else process.env.NODE_EXTRA_CA_CERTS = previousCa; + await new Promise(resolve => server.close(() => resolve())); + }); + + it.each(["127.0.0.1", "[::1]", "[::ffff:127.0.0.1]"])("rejects repository IP literal %s without connecting", async host => { + const url = `https://${host}`; + await expect(fetchRegistryMetadata(url, `${url}/axios`, false)).rejects.toThrow("private/reserved"); + expect(connections).toBe(0); + }); + it("rejects repository HTTP even when a hostname resolves to a private address", async () => { + const lookup = answer([privateAddress]); + const fetch = jest.spyOn(globalThis, "fetch").mockResolvedValue(new Response("{}")); + const url = base.replace("https:", "http:"); + await expect(fetchRegistryMetadata(url, `${url}/axios`, false)).rejects.toThrow("requires HTTPS"); + expect(fetch).not.toHaveBeenCalled(); + expect(lookup).not.toHaveBeenCalled(); + }); + it("keeps alternate HTTPS spellings on the checked transport", async () => { + answer([privateAddress]); + const fetch = jest.spyOn(globalThis, "fetch").mockResolvedValue(new Response("{}")); + const url = base.replace("https:", "HTTPS:"); + await expect(fetchRegistryMetadata(url, `${url}/axios`, false)).rejects.toThrow("private/reserved"); + expect(fetch).not.toHaveBeenCalled(); + expect(connections).toBe(0); + }); + it.each(["https://user:password@mirror.test", "https://mirror.test/?action=x", "https://mirror.test/#", "https://mirror.test/?"])("rejects non-registry URL components: %s", async url => { + await expect(fetchRegistryMetadata(url, url + "/axios", true)).rejects.toThrow("Registry URL must not contain"); + expect(connections).toBe(0); + }); + it.each([[privateAddress], [publicAddress, privateAddress], []])("rejects unsafe DNS answers %j at connection time", async (...addresses) => { + answer(addresses); + await expect(fetchRegistryMetadata(base, base + "/axios", false)).rejects.toThrow("private/reserved"); + expect(connections).toBe(0); + }); + it("gives the connector the checked public addresses without another lookup", async () => { + const lookup = answer([publicAddress]); + jest.spyOn(https, "request").mockImplementation(((options: https.RequestOptions) => { + const listeners: Record void> = {}; + return { + on(event: string, listener: (error: Error) => void) { listeners[event] = listener; return this; }, + end() { + (options.lookup as LookupFunction)("mirror.test", { all: true }, (error, addresses) => { + expect(error).toBeNull(); expect(addresses).toEqual([publicAddress]); + listeners.error(new Error("connector reached")); + }); + }, + }; + }) as typeof https.request); + await expect(fetchRegistryMetadata(base, base + "/axios", false)).rejects.toThrow("connector reached"); + expect(lookup).toHaveBeenCalledTimes(1); + }); + it("supports a user-authorized HTTPS mirror with a custom CA", async () => { + answer([privateAddress]); process.env.NODE_EXTRA_CA_CERTS = cert; + const response = await fetchRegistryMetadata(base, base + "/axios", true); + expect(await response.json()).toEqual({ versions: { "1.0.0": {} } }); + expect(requests).toEqual(["/axios"]); + }); + it("refuses redirects even from a user-authorized mirror", async () => { + answer([privateAddress]); process.env.NODE_EXTRA_CA_CERTS = cert; + await expect(fetchRegistryMetadata(base, base + "/redirect", true)).rejects.toThrow("redirects"); + expect(requests).toEqual(["/redirect"]); + }); + it("retains TLS trust checks for authorized mirrors", async () => { + answer([privateAddress]); process.env.NODE_EXTRA_CA_CERTS = cert; + // Different hostname from the fixture must still fail validation. + const target = base.replace("mirror.test", "other.test"); + await expect(fetchRegistryMetadata(target, target + "/axios", true)).rejects.toThrow(/hostname|altnames/i); + expect(requests).toHaveLength(0); + }); + it("uses the default registry transport with redirects disabled", async () => { + const fetch = jest.spyOn(globalThis, "fetch").mockResolvedValue(new Response("{}")); + await fetchRegistryMetadata("https://registry.npmjs.org", "https://registry.npmjs.org/axios", false); + expect(fetch).toHaveBeenCalledWith("https://registry.npmjs.org/axios", { redirect: "error" }); + }); +}); diff --git a/tests/utils/registry-config.test.ts b/tests/utils/registry-config.test.ts index f9d851a33..66b16a4cc 100644 --- a/tests/utils/registry-config.test.ts +++ b/tests/utils/registry-config.test.ts @@ -183,3 +183,38 @@ describe("packageManagerForSource", () => { expect(packageManagerForSource(undefined)).toBeNull(); }); }); + + +describe("registry configuration ownership", () => { + it("does not authorize project registry destinations", () => { + for (const packageManager of ["npm", "pnpm", "yarn"] as const) { + const config = resolveRegistryConfig({ + projectPath: tmpProject({ ".npmrc": "registry=http://127.0.0.1/", ".yarnrc.yml": "npmRegistryServer: http://127.0.0.1/" }), + packageManager, env: {}, homeDir: tmpProject({}), + }); + expect(config.allowPrivate).toBe(false); + } + }); + it("authorizes a separately owned user registry", () => { + const config = resolveRegistryConfig({ projectPath: tmpProject({}), packageManager: "npm", env: {}, + homeDir: tmpProject({ ".npmrc": "registry=http://127.0.0.1/" }) }); + expect(config.allowPrivate).toBe(true); + }); + it("does not trust the home file when home is the scanned project", () => { + const project = tmpProject({ ".npmrc": "registry=http://127.0.0.1/" }); + expect(resolveRegistryConfig({ projectPath: project, homeDir: project, packageManager: "npm", env: {} }).allowPrivate).toBe(false); + }); + it("does not trust a home config symlink into the scanned project", () => { + const project = tmpProject({ "repo-config": "registry=http://127.0.0.1/" }); + const home = tmpProject({}); + const userConfig = path.join(home, ".npmrc"); + fs.symlinkSync(path.join(project, "repo-config"), userConfig); + const config = resolveRegistryConfig({ projectPath: project, homeDir: home, packageManager: "npm", env: {} }); + expect(config.sourceFile).toBe(userConfig); + expect(config.registry).toBe("http://127.0.0.1/"); + expect(config.allowPrivate).toBe(false); + }); + it("accepts an explicit environment registry", () => { + expect(resolveRegistryConfig({ projectPath: tmpProject({}), packageManager: "npm", env: { NPM_CONFIG_REGISTRY: "http://127.0.0.1/" } }).allowPrivate).toBe(true); + }); +}); diff --git a/website/docs/corporate-proxy.md b/website/docs/corporate-proxy.md index b8943afc6..75ac079a5 100644 --- a/website/docs/corporate-proxy.md +++ b/website/docs/corporate-proxy.md @@ -130,3 +130,26 @@ cve-lite . --offline-db /path/to/advisories.db ``` See [Offline Advisory DB](./offline-advisory-db.md) for the full offline workflow. + + +## Registry metadata destinations + +Package metadata lookups honor project `.npmrc` and `.yarnrc.yml` registry settings, +including scoped registries. Repository-supplied destinations require HTTPS and +public addresses. DNS answers are checked for each connection and redirects are +refused. Registry URLs must not contain credentials, query strings or fragments. + +For a trusted internal registry, configure the registry in your user-level config +outside the scanned project, or explicitly set `NPM_CONFIG_REGISTRY` for the scan: + +```sh +NPM_CONFIG_REGISTRY=https://registry.internal/npm/ cve-lite . +``` + +The environment override selects the default registry for all metadata lookups. +A user-level config can select scoped registries when the project supplies no +registry settings. A home config linked into the scanned project is treated as +repository configuration. HTTP is supported only for explicitly trusted registry +configuration; redirects remain disabled. Registry credentials are not read. +Blocked metadata requests leave remediation validation incomplete; they do not +silently switch validation to the public registry. diff --git a/website/docs/security-assurance-case.md b/website/docs/security-assurance-case.md index fdfab415d..6971b5ad4 100644 --- a/website/docs/security-assurance-case.md +++ b/website/docs/security-assurance-case.md @@ -103,7 +103,7 @@ The project applies the [Saltzer and Schroeder](https://en.wikipedia.org/wiki/Sa - **Open design.** The codebase is fully open source under the MIT license. Advisory data comes from the public OSV project. Release signatures are publicly verifiable. The security model in this document is itself part of that openness. - **Separation of privilege.** Three independent signing mechanisms protect releases: the project lead's GPG key (source tag), GitHub Actions Sigstore attestations (release tarball), and the npm registry's automatic signature (npm-installed package). Compromising one does not compromise the others. - **Least common mechanism.** Local-first scans hold no server-side state. Offline mode in particular is fully isolated — no shared cache, no analytics, no telemetry of any kind. -- **Least privilege.** The CLI does not write to the user's source tree by default. `--fix` mode is explicit, scoped to the package manager's own install command, and rescans afterward. The CLI never opens network connections to anything other than the hardcoded advisory and registry endpoints documented above. +- **Least privilege.** The CLI does not write to the user's source tree by default. `--fix` mode is explicit, scoped to the package manager's own install command, and rescans afterward. Advisory endpoints default to the documented services. Repository-supplied registry destinations require HTTPS and public addresses checked at connection time; user/environment configuration can explicitly authorize internal mirrors. Registry redirects are refused. - **Psychological acceptability.** Output is a copy-and-run fix command, not a list of CVE IDs requiring further triage. The tool is meant to be used, not avoided. ## Common implementation weaknesses countered @@ -121,7 +121,7 @@ Mapped against the [OWASP Top 10 (2021)](https://owasp.org/Top10/) — the categ | A07: Identification and Authentication Failures | Not applicable | The CLI has no authentication surface. | | A08: Software and Data Integrity Failures | Countered | Release tarballs are signed via Sigstore Artifact Attestations. Git tags are GPG-signed by the project lead. Withdrawn OSV advisories are filtered at ingest. Sync replaces the advisory database atomically rather than partially updating it. | | A09: Security Logging and Monitoring Failures | Partially countered | CodeQL alerts on every push, the self-scan workflow flags new advisories in the project's own dependencies on every push, and the GitHub release workflow emits build provenance. The project does not centrally log scan invocations because none happen on project-controlled infrastructure — scans run on each user's local machine. | -| A10: Server-Side Request Forgery | Substantially mitigated | The CLI makes outbound HTTP requests to its default advisory and registry endpoints and can optionally use a user-supplied OSV-compatible endpoint via `--osv-url`. That flag enforces HTTPS and validates DNS resolution against private, non-global, and SSRF-sensitive special-purpose ranges (RFC 1918, loopback, link-local, multicast, CGNAT, translation and tunneling ranges, etc.) by default. For custom endpoints, each new HTTPS connection resolves and validates all DNS answers and passes those same checked addresses to the connector, including when a custom CA is configured. Private IP literals are checked separately because Node.js skips DNS lookup for them. TLS certificate and hostname validation remain enabled, and OSV redirects are not followed. Using `--allow-private-osv-url` bypasses this protection and should only be used with trusted internal mirrors. | +| A10: Server-Side Request Forgery | Substantially mitigated | The CLI makes outbound HTTP requests to its default advisory and registry endpoints and can optionally use a user-supplied OSV-compatible endpoint via `--osv-url`. That flag enforces HTTPS and validates DNS resolution against private, non-global, and SSRF-sensitive special-purpose ranges (RFC 1918, loopback, link-local, multicast, CGNAT, translation and tunneling ranges, etc.) by default. For custom endpoints, each new HTTPS connection resolves and validates all DNS answers and passes those same checked addresses to the connector, including when a custom CA is configured. Private IP literals are checked separately because Node.js skips DNS lookup for them. TLS certificate and hostname validation remain enabled, and OSV redirects are not followed. Using `--allow-private-osv-url` bypasses this protection and should only be used with trusted internal mirrors. Registry metadata has a separate policy: repository-supplied URLs require HTTPS and checked public addresses, URL credentials/query/fragment components are rejected, and redirects are refused. Internal registry access requires user/environment configuration outside the scanned project. | ## Limitations and explicit non-goals From 82b0499dd03b6015f1a82cb6e4cb14679a9d6599 Mon Sep 17 00:00:00 2001 From: vian <85208522+vianmangal@users.noreply.github.com> Date: Tue, 10 Nov 2026 04:12:12 +0530 Subject: [PATCH 02/21] Merge commit from fork * fix(security): quote subfolder paths in remediation commands * fix(security): quote override remediation paths (cherry picked from commit 4d6e38ff9c9db500a08137284617222b98cd8876) --- src/output/override-fix-command.ts | 12 ++-- src/remediation/fix-commands.ts | 9 +-- src/utils/shell-quote.ts | 13 +++++ tests/fix-commands-subfolder.test.ts | 64 +++++++++++++++++++++ tests/output/override-findings-html.test.ts | 39 +++++++------ 5 files changed, 107 insertions(+), 30 deletions(-) create mode 100644 src/utils/shell-quote.ts diff --git a/src/output/override-fix-command.ts b/src/output/override-fix-command.ts index 6dec1b973..94ee0a056 100644 --- a/src/output/override-fix-command.ts +++ b/src/output/override-fix-command.ts @@ -1,12 +1,10 @@ import { OVERRIDES_COMMAND } from "../constants.js"; +import { quoteShellPath } from "../utils/shell-quote.js"; -// The folder a fix command targets, relative to the scanned project root -// ("packages/a"), or empty for the root itself. Never pass a cwd-relative or -// absolute path: the HTML report is opened from an unknown directory, so only a -// project-relative target stays runnable when it is copied out. +// HTML callers pass a project-relative path, and terminal callers pass a +// cwd-relative path. An empty target or "." needs no positional argument. export function overrideCommandTargetSuffix(commandTarget?: string): string { - const normalized = commandTarget?.replace(/\\/g, "/"); - return normalized && normalized !== "." ? ` ${normalized}` : ""; + return commandTarget && commandTarget !== "." ? ` ${quoteShellPath(commandTarget)}` : ""; } export function buildOverrideFixCommand( @@ -14,5 +12,5 @@ export function buildOverrideFixCommand( commandTarget?: string, ): string { const suffix = overrideCommandTargetSuffix(commandTarget); - return runnableCommand.replace(OVERRIDES_COMMAND, `cve-lite${suffix} overrides`); + return runnableCommand.replace(OVERRIDES_COMMAND, () => `cve-lite${suffix} overrides`); } diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index ff22fc6c7..5e80a27fb 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -6,6 +6,7 @@ import { getPrimaryParent } from "../utils/finding.js"; import { isNpmLockSource, packageManagerForSource } from "../utils/package-manager-source.js"; import { calculatePathCoverage, formatDependencyPath } from "../utils/path-coverage.js"; import { pluralize } from "../utils/string.js"; +import { quoteShellPath, shellQuote } from "../utils/shell-quote.js"; import { buildNpmWorkspaceMap } from "../parsers/package-lock.js"; import { buildPnpmWorkspaceMap } from "../parsers/pnpm-lock.js"; import { buildYarnWorkspaceMap } from "../parsers/yarn-lock.js"; @@ -551,7 +552,7 @@ export function buildSuggestedFixCommandPlan( }).length; if (options?.subfolder) { - const prefix = `cd ${options.subfolder} && `; + const prefix = `cd ${quoteShellPath(options.subfolder)} && `; if (plan.command) plan.command = prefix + plan.command; for (const section of plan.sections) { section.command = prefix + section.command; @@ -596,12 +597,6 @@ function buildBunUpdateCommand(pkg: string): string { return `bun update ${shellQuote(pkg)}`; } -/** Quote a token for human-facing plan strings that users may paste into a shell. */ -function shellQuote(token: string): string { - if (/^[A-Za-z0-9_@%+./:=,-]+$/.test(token)) return token; - return "'" + token.replace(/'/g, "'\\''") + "'"; -} - export function commandPrefix(packageManager: SuggestedFixPackageManager): string { if (packageManager === "npm") return "npm install"; if (packageManager === "pnpm") return "pnpm add"; diff --git a/src/utils/shell-quote.ts b/src/utils/shell-quote.ts new file mode 100644 index 000000000..cccb94572 --- /dev/null +++ b/src/utils/shell-quote.ts @@ -0,0 +1,13 @@ +import path from "node:path"; + +/** Quote a token for a command the user may paste into a POSIX shell. */ +export function shellQuote(token: string): string { + if (/^[A-Za-z0-9_@%+./:=,-]+$/.test(token)) return token; + return "'" + token.replace(/'/g, "'\\''") + "'"; +} + +export function quoteShellPath(directory: string): string { + // Preserve literal backslashes in POSIX names and keep ordinary Windows paths unquoted. + const normalized = path.sep === "\\" ? directory.replace(/\\/g, "/") : directory; + return shellQuote(normalized.startsWith("-") ? `./${normalized}` : normalized); +} diff --git a/tests/fix-commands-subfolder.test.ts b/tests/fix-commands-subfolder.test.ts index 552b3c080..ee8cdaa34 100644 --- a/tests/fix-commands-subfolder.test.ts +++ b/tests/fix-commands-subfolder.test.ts @@ -1,4 +1,10 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { jest } from "@jest/globals"; import { buildSuggestedFixCommandPlan } from "../src/remediation/fix-commands.js"; +import { buildOverrideFixCommand, overrideCommandTargetSuffix } from "../src/output/override-fix-command.js"; import type { Finding, ScanInput } from "../src/types.js"; const minimalScanInput: ScanInput = { @@ -54,4 +60,62 @@ describe("buildSuggestedFixCommandPlan with subfolder", () => { expect(section.command).not.toMatch(/^cd /); } }); + + it("keeps ordinary Windows nested paths unquoted", () => { + const separator = jest.replaceProperty(path, "sep", "\\"); + try { + const plan = buildSuggestedFixCommandPlan([minimalFinding], minimalScanInput, { subfolder: "packages\\app" }); + expect(plan?.command).toBe("cd packages/app && npm install axios@1.8.0"); + } finally { + separator.restore(); + } + }); +}); + +// Execute the generated strings: output snapshots alone cannot prove shell safety. +describe("subfolder commands preserve literal paths", () => { + const shells = ["/bin/sh", "/bin/bash", "/bin/zsh"].filter(shell => fs.existsSync(shell)); + if (shells.length === 0) { + it.skip("requires an installed POSIX shell", () => {}); + return; + } + it.each(shells)("quotes paths when executed by %s", shell => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "cvel-command-path-")); + try { + const bin = path.join(root, "bin"); + const caller = path.join(root, "caller"); + fs.mkdirSync(bin); + fs.mkdirSync(caller); + fs.writeFileSync(path.join(bin, "cve-lite"), '#!/bin/sh\n[ "$2" = overrides ] && cd "$1" && pwd -P\n', { mode: 0o700 }); + const env = { ...process.env, PATH: `${bin}${path.delimiter}${process.env.PATH ?? ""}`, CDPATH: "" }; + const overrideFix = "cve-lite overrides --fix --rule OA001"; + for (const subfolder of [ + "ordinary", "two words", "single'quote", 'double"quote', + "pkg$(touch marker)", "pkg`touch marker`", "pkg; touch marker; #", + "nl\ntouch marker", "pkg && touch marker", "pkg | touch marker", + "pkg$&", "literal\\backslash", "-", "-P", "nested/with space", + ]) { + fs.mkdirSync(path.join(root, subfolder), { recursive: true }); + const plan = buildSuggestedFixCommandPlan([minimalFinding], minimalScanInput, { subfolder }); + if (!plan?.command) throw new Error("Expected a generated command"); + const commands = [ + ...[plan.command, ...plan.sections.map(section => section.command)].map(command => ({ command, cwd: root })), + { command: buildOverrideFixCommand(overrideFix, subfolder), cwd: root }, + { command: buildOverrideFixCommand(overrideFix, `../${subfolder}`), cwd: caller }, + { command: `cve-lite${overrideCommandTargetSuffix(subfolder)} overrides --fix`, cwd: root }, + ]; + for (const { command, cwd } of commands) { + // Both stubs report the chosen directory without applying any fixes. + const run = spawnSync(shell, ["-c", 'npm() { pwd -P; }; ' + command], { + cwd, env, encoding: "utf8", timeout: 5000, + }); + expect(run.status).toBe(0); + expect(run.stdout.trim()).toBe(fs.realpathSync(path.join(root, subfolder))); + expect(fs.existsSync(path.join(cwd, "marker"))).toBe(false); + } + } + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); }); diff --git a/tests/output/override-findings-html.test.ts b/tests/output/override-findings-html.test.ts index 3c6ba4952..25fe45166 100644 --- a/tests/output/override-findings-html.test.ts +++ b/tests/output/override-findings-html.test.ts @@ -1,3 +1,5 @@ +import path from "node:path"; +import { jest } from "@jest/globals"; import { renderOverrideFindingsHtml } from "../../src/output/override-findings-html.js"; import type { OverrideFinding } from "../../src/overrides/types.js"; @@ -94,20 +96,25 @@ describe("renderOverrideFindingsHtml", () => { }); it("normalises a Windows subfolder separator in the command target", () => { - const html = renderOverrideFindingsHtml( - [ - f({ - fix: { - type: "rfc6902", - patch: [], - runnableCommand: "cve-lite overrides --fix --rule OA001", - }, - }), - ], - "packages\\a", - ); - expect(html).toContain("cve-lite packages/a overrides --fix --rule OA001"); - expect(html).not.toContain("packages\\a"); + const separator = jest.replaceProperty(path, "sep", "\\"); + try { + const html = renderOverrideFindingsHtml( + [ + f({ + fix: { + type: "rfc6902", + patch: [], + runnableCommand: "cve-lite overrides --fix --rule OA001", + }, + }), + ], + "packages\\a", + ); + expect(html).toContain("cve-lite packages/a overrides --fix --rule OA001"); + expect(html).not.toContain("packages\\a"); + } finally { + separator.restore(); + } }); it("emits a bare command when the panel describes the project root", () => { @@ -149,9 +156,9 @@ describe("renderOverrideFindingsHtml", () => { it("sorts critical findings before high findings", () => { const html = renderOverrideFindingsHtml([ f({ ruleId: "OA010", severity: "high" }), - f({ ruleId: "OA020", severity: "critical", package: { name: "lodash" } }), + f({ ruleId: "OA008", severity: "critical", package: { name: "lodash" } }), ]); - const critIdx = html.indexOf("OA020"); + const critIdx = html.indexOf("OA008"); const highIdx = html.indexOf("OA010"); expect(critIdx).toBeLessThan(highIdx); }); From 293c3bb55d1a3cc101b9088d2953bd8b08c1756f Mon Sep 17 00:00:00 2001 From: Alex Powell <51252677+alex-powell@users.noreply.github.com> Date: Tue, 10 Nov 2026 09:33:55 +1030 Subject: [PATCH 03/21] Merge commit from fork Yarn 1.22 records an alias as "util-lib@npm:lodash@4.17.20". The classic parser kept the text before the final @, so a scan queried util-lib@npm:lodash and reported no advisories for the lodash that was actually installed. Co-authored-by: Alex Powell (cherry picked from commit 8b4800cebf9d52b7f7780c95e88417b97a42a563) --- src/parsers/yarn-lock-graph.ts | 46 ++++-- src/parsers/yarn-lock.ts | 123 ++++++++++++--- tests/parsers/yarn-lock.test.ts | 257 ++++++++++++++++++++++++++++++++ 3 files changed, 387 insertions(+), 39 deletions(-) diff --git a/src/parsers/yarn-lock-graph.ts b/src/parsers/yarn-lock-graph.ts index 2aef0162a..b2ac9cb23 100644 --- a/src/parsers/yarn-lock-graph.ts +++ b/src/parsers/yarn-lock-graph.ts @@ -1,6 +1,6 @@ import fs from "node:fs"; import { parse as parseYarnLock } from "yarn-lockfile"; -import { isYarnBerry, parseBerryDependencies, parseYarnPackageKey } from "./yarn-lock.js"; +import { isYarnBerry, parseBerryDependencies, parseYarnClassicSelectorKey, parseYarnPackageKey } from "./yarn-lock.js"; /** * The dependency graph of a `yarn.lock`, exposed for SBOM edge building. @@ -16,7 +16,7 @@ import { isYarnBerry, parseBerryDependencies, parseYarnPackageKey } from "./yarn * truncated paths loses most of the graph on a large tree: measured on * `examples/twenty`, 4239 of 5452 packages had no parent edge at all. See #1109. */ -export type YarnGraphNode = { name: string; version: string }; +export type YarnGraphNode = { name: string; version: string; registryName?: string }; export type YarnLockGraph = { nodeIdsFor(name: string, version: string | null): readonly string[]; @@ -30,6 +30,8 @@ type YarnIndex = { selectorToKey: Map; /** Canonical key to the canonical keys it depends on. */ graph: Map; + /** Registry package for a Yarn v1 alias, keyed by the install-name canonical key. */ + registryNameByKey: Map; }; const EMPTY: readonly string[] = Object.freeze([]); @@ -86,7 +88,17 @@ function buildBerryIndex(content: string): YarnIndex { graph.set(canonicalKey, [...children]); } - return { selectorToKey, graph }; + return { selectorToKey, graph, registryNameByKey: new Map() }; +} + +function classicCanonicalKey(selectorKey: string, version: string): { key: string; selectors: string[]; registryName?: string } | null { + const identity = parseYarnClassicSelectorKey(selectorKey); + if (!identity) return null; + return { + key: `${identity.name}@${version}`, + selectors: identity.selectors, + ...(identity.registryName ? { registryName: identity.registryName } : {}), + }; } /** @@ -96,29 +108,27 @@ function buildBerryIndex(content: string): YarnIndex { function buildClassicIndex(parsed: any): YarnIndex { const selectorToKey = new Map(); const graph = new Map(); + const registryNameByKey = new Map(); const entries = Object.entries(parsed?.object ?? {}); for (const [selectorKey, meta] of entries) { const version = meta?.version; if (!version) continue; - const selectors = String(selectorKey).split(",").map(part => part.trim()).filter(Boolean); - const atIndex = selectors[0]?.lastIndexOf("@") ?? -1; - if (atIndex <= 0) continue; + const entry = classicCanonicalKey(selectorKey, version); + if (!entry) continue; - const canonicalKey = `${selectors[0]!.slice(0, atIndex)}@${version}`; - for (const selector of selectors) selectorToKey.set(selector, canonicalKey); + if (entry.registryName) registryNameByKey.set(entry.key, entry.registryName); + for (const selector of entry.selectors) selectorToKey.set(selector, entry.key); } for (const [selectorKey, meta] of entries) { const version = meta?.version; if (!version) continue; - const selectors = String(selectorKey).split(",").map(part => part.trim()).filter(Boolean); - const atIndex = selectors[0]?.lastIndexOf("@") ?? -1; - if (atIndex <= 0) continue; + const entry = classicCanonicalKey(selectorKey, version); + if (!entry) continue; - const canonicalKey = `${selectors[0]!.slice(0, atIndex)}@${version}`; const children = new Set(); // An optional dependency is installed whenever it can be, so its parent // constrains the installed copy as much as a regular one does. @@ -129,10 +139,10 @@ function buildClassicIndex(parsed: any): YarnIndex { if (childKey) children.add(childKey); } } - graph.set(canonicalKey, [...children]); + graph.set(entry.key, [...children]); } - return { selectorToKey, graph }; + return { selectorToKey, graph, registryNameByKey }; } /** @@ -180,7 +190,13 @@ export function loadYarnLockGraph(filePath: string): YarnLockGraph | null { parentsFor: nodeId => parentsByKey.get(nodeId) ?? EMPTY, getNode: nodeId => { const parsedKey = parseYarnPackageKey(nodeId); - return parsedKey ? { name: parsedKey.name, version: parsedKey.version } : null; + if (!parsedKey) return null; + const registryName = index.registryNameByKey.get(nodeId); + return { + name: parsedKey.name, + version: parsedKey.version, + ...(registryName ? { registryName } : {}), + }; }, }; } diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index a458f88d2..c856815c1 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -2,6 +2,7 @@ import fs from "node:fs"; import path from "node:path"; import { parse as parseYarnLock } from "yarn-lockfile"; import type { PackageRef } from "../types.js"; +import { npmAliasRegistryName, packageInstanceKey } from "../utils/package-identity.js"; import { looksLikeVersion, normalizeRawVersion } from "../utils/version.js"; import { upsertPackage, MAX_PATHS_PER_PACKAGE } from "./utils.js"; @@ -171,6 +172,83 @@ export function parseYarnPackageKey(key: string): { key: string; name: string; v return { key, name, version }; } +/** + * Yarn v1 records an alias as `@npm:@`. + * + * `yarn 1.22` quotes that key. A scoped target keeps the leading `@` after the + * protocol (`frames@npm:@babel/code-frame@7.23.5`), and a scoped alias keeps + * its own (`@my/frames@npm:@babel/code-frame@7.23.5`). The range in the key is + * what was requested. The installed version is the lockfile `version` field, + * so this only returns names. + * + * A normal key (`lodash@4.17.20`, `@babel/code-frame@^7.23.4`) has no `@npm:` + * marker. Splitting those on the final `@` is still the package name. Doing + * that to an alias key yields `util-lib@npm:lodash`, which is not a package + * OSV knows about. + */ +export function parseYarnClassicDescriptor(selector: string): { name: string; registryName?: string } | null { + const marker = "@npm:"; + const markerAt = selector.indexOf(marker); + if (markerAt > 0) { + const name = selector.slice(0, markerAt); + const target = selector.slice(markerAt + marker.length); + if (!name || !target) return null; + // One definition of an alias target, shared with npm lockfiles: a later `@` + // starts the requested range, and the leading `@` of `@scope/pkg` does not. + const registryName = npmAliasRegistryName("npm:" + target); + if (!registryName) return null; + // `lodash@npm:lodash@4.17.20` is the same package under its own name. + if (registryName === name) return { name }; + return { name, registryName }; + } + + const atIndex = selector.lastIndexOf("@"); + if (atIndex <= 0) return null; + const name = selector.slice(0, atIndex); + if (!name) return null; + return { name }; +} + +export type YarnClassicIdentity = { + name: string; + registryName?: string; + selectors: string[]; +}; + +/** Every descriptor in one Yarn v1 entry, and the package the first one names. */ +export function parseYarnClassicSelectorKey(selectorKey: string): YarnClassicIdentity | null { + const selectors = String(selectorKey).split(",").map(part => part.trim()).filter(Boolean); + const identity = selectors[0] ? parseYarnClassicDescriptor(selectors[0]) : null; + if (!identity) return null; + return { + name: identity.name, + ...(identity.registryName ? { registryName: identity.registryName } : {}), + selectors, + }; +} + +type YarnClassicEntry = YarnClassicIdentity & { + version: string; + /** Map and path-graph key. Includes the registry name when this is an alias. */ + instanceKey: string; +}; + +function readYarnClassicEntry(selectorKey: string, meta: { version?: unknown }): YarnClassicEntry | null { + const version = meta?.version; + if (typeof version !== "string" || version.length === 0) return null; + const identity = parseYarnClassicSelectorKey(selectorKey); + if (!identity) return null; + return { + ...identity, + version, + instanceKey: packageInstanceKey({ + name: identity.name, + registryName: identity.registryName, + version, + }), + }; +} + function collectYarnPaths( seeds: Seed[], graph: Map, @@ -186,7 +264,12 @@ function collectYarnPaths( const ref = parseYarnPackageKey(current.key); if (!ref) continue; - const nextPath = [...current.path, ref.name]; + // An aliased entry's canonical key is `alias\0registry@version`. The path + // shows the name that was installed, which is the part before the NUL. + const installName = ref.name.split("\0")[0]; + if (!installName) continue; + + const nextPath = [...current.path, installName]; const stateKey = `${ref.key}>${nextPath.join(">")}`; if (visitedStates.has(stateKey)) continue; visitedStates.add(stateKey); @@ -371,35 +454,27 @@ function loadFromYarnClassicLock(parsed: any, filePath: string): PackageRef[] { const selectorToKey = new Map(); for (const [selectorKey, meta] of Object.entries(parsed.object)) { - const version = meta?.version; - if (!version) continue; - - const selectors = String(selectorKey).split(",").map(selector => selector.trim()).filter(Boolean); - const firstSelector = selectors[0]; - const atIndex = firstSelector.lastIndexOf("@"); - if (atIndex <= 0) continue; - - const name = firstSelector.slice(0, atIndex); - const canonicalKey = `${name}@${version}`; + const entry = readYarnClassicEntry(selectorKey, meta); + if (!entry) continue; - for (const selector of selectors) { - selectorToKey.set(selector, canonicalKey); + for (const selector of entry.selectors) { + selectorToKey.set(selector, entry.instanceKey); } - upsertPackage(map, { name, version, ecosystem: "npm", paths: [], resolvedUrl: meta?.resolved as string | undefined }); + upsertPackage(map, { + name: entry.name, + version: entry.version, + ecosystem: "npm", + paths: [], + resolvedUrl: meta?.resolved as string | undefined, + ...(entry.registryName ? { registryName: entry.registryName } : {}), + }); } for (const [selectorKey, meta] of Object.entries(parsed.object)) { - const version = meta?.version; - if (!version) continue; - - const selectors = String(selectorKey).split(",").map(selector => selector.trim()).filter(Boolean); - const firstSelector = selectors[0]; - const atIndex = firstSelector.lastIndexOf("@"); - if (atIndex <= 0) continue; + const entry = readYarnClassicEntry(selectorKey, meta); + if (!entry) continue; - const name = firstSelector.slice(0, atIndex); - const canonicalKey = `${name}@${version}`; const depKeys = new Set(); const deps = meta?.dependencies; @@ -410,7 +485,7 @@ function loadFromYarnClassicLock(parsed: any, filePath: string): PackageRef[] { } } - graph.set(canonicalKey, [...depKeys]); + graph.set(entry.instanceKey, [...depKeys]); } const taggedPathsByKey = new Map(); diff --git a/tests/parsers/yarn-lock.test.ts b/tests/parsers/yarn-lock.test.ts index 1009310ea..df5169012 100644 --- a/tests/parsers/yarn-lock.test.ts +++ b/tests/parsers/yarn-lock.test.ts @@ -1,6 +1,8 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { jest } from "@jest/globals"; +import { parseArgs } from "../../src/cli/args.js"; import { loadPackages } from "../../src/parsers/index.js"; import { loadFromBunLock } from "../../src/parsers/bun-lock.js"; import { loadNpmLockGraph } from "../../src/parsers/npm-lock-graph.js"; @@ -8,6 +10,13 @@ import { loadFromPackageJson } from "../../src/parsers/package-json.js"; import { loadFromPackageLock } from "../../src/parsers/package-lock.js"; import { loadFromPnpmLock } from "../../src/parsers/pnpm-lock.js"; import { loadFromYarnLock } from "../../src/parsers/yarn-lock.js"; +import { loadYarnLockGraph } from "../../src/parsers/yarn-lock-graph.js"; +import { buildSuggestedFixCommandPlan, findSuggestedCommandForFinding } from "../../src/remediation/fix-commands.js"; +import { clearPackumentCache } from "../../src/remediation/npm-registry.js"; +import { scanPackages } from "../../src/scanner.js"; +import type { FetchLike, Finding, OsvVuln, ParsedOptions, ScanInput } from "../../src/types.js"; +import { formatPackageName } from "../../src/output/formatters.js"; +import { registryPackageName } from "../../src/utils/package-identity.js"; import { removeDir } from "../test-utils.js"; function createTempProjectDir(): string { @@ -688,4 +697,252 @@ follow-redirects@^1.10.0: } }); + // Yarn 1.22.22 writes an alias as "@npm:@", quoting the + // key. A scoped target keeps its leading @ after the protocol: + // "frames@npm:@babel/code-frame@7.23.5". A plain key has no "@npm:" marker. + it("reads Yarn v1 npm: aliases as the registry package and leaves ordinary keys unchanged", () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ + dependencies: { + "util-lib": "npm:lodash@^4.17.15", + frames: "npm:@babel/code-frame@7.23.5", + "@my/frames": "npm:@babel/code-frame@7.23.5", + plain: "npm:@babel/code-frame", + lodash: "4.17.20", + "@babel/code-frame": "^7.23.4", + }, + }), + "utf8", + ); + fs.writeFileSync( + lockPath, + [ + "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.", + "# yarn lockfile v1", + "", + "", + '"util-lib@npm:lodash@^4.17.15":', + ' version "4.17.20"', + ' resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.20.tgz#b44a9b6297bcb698f1c51a3545a2b3b368d59c52"', + "", + '"frames@npm:@babel/code-frame@7.23.5":', + ' version "7.23.5"', + ' resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.23.5.tgz#abc"', + "", + '"@my/frames@npm:@babel/code-frame@7.23.5":', + ' version "7.23.5"', + ' resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.23.5.tgz#abc"', + "", + "lodash@4.17.20:", + ' version "4.17.20"', + ' resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.20.tgz#def"', + "", + // Same package requested through its own npm: alias. Yarn still writes + // the descriptor; it is not a second installed package. + '"lodash@npm:lodash@4.17.20":', + ' version "4.17.20"', + ' resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.20.tgz#def"', + "", + '"@babel/code-frame@^7.23.4":', + ' version "7.23.5"', + ' resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.23.5.tgz#abc"', + "", + // Yarn 1.22 omits the range when the manifest alias is `npm:@scope/pkg`. + '"plain@npm:@babel/code-frame":', + ' version "8.0.6"', + ' resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-8.0.6.tgz#abc"', + " dependencies:", + ' js-tokens "^10.0.0"', + "", + "js-tokens@^10.0.0:", + ' version "10.0.0"', + ' resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-10.0.0.tgz#def"', + "", + ].join("\n"), + "utf8", + ); + + try { + const packages = loadFromYarnLock(lockPath); + const byInstall = (name: string) => packages.find(pkg => pkg.name === name); + + expect(byInstall("util-lib")).toMatchObject({ + name: "util-lib", + registryName: "lodash", + version: "4.17.20", + }); + expect(registryPackageName(byInstall("util-lib")!)).toBe("lodash"); + expect(byInstall("frames")).toMatchObject({ + name: "frames", + registryName: "@babel/code-frame", + version: "7.23.5", + }); + expect(byInstall("@my/frames")).toMatchObject({ + name: "@my/frames", + registryName: "@babel/code-frame", + version: "7.23.5", + }); + const lodashEntries = packages.filter(pkg => pkg.name === "lodash"); + expect(lodashEntries).toHaveLength(1); + expect(lodashEntries[0]).toMatchObject({ name: "lodash", version: "4.17.20" }); + expect(lodashEntries[0]?.registryName).toBeUndefined(); + const selfAliasFinding: Finding = { + pkg: lodashEntries[0]!, + severity: "high", + relationship: "direct", + vulnerabilities: [], + cveAliases: [], + dependencyPaths: [["project", "lodash"]], + firstFixedVersion: "4.17.21", + }; + const selfAliasInput: ScanInput = { + mode: "resolved-lockfile", + source: "yarn-lock", + filePath: lockPath, + packages: [lodashEntries[0]!], + notes: [], + warnings: [], + skippedDependencies: [], + }; + const selfAliasPlan = buildSuggestedFixCommandPlan([selfAliasFinding], selfAliasInput, { offline: true }); + expect(findSuggestedCommandForFinding(selfAliasPlan!, selfAliasFinding)).toBe("yarn add lodash@4.17.21"); + expect(byInstall("@babel/code-frame")).toMatchObject({ + name: "@babel/code-frame", + version: "7.23.5", + }); + expect(byInstall("@babel/code-frame")?.registryName).toBeUndefined(); + expect(byInstall("plain")).toMatchObject({ + name: "plain", + registryName: "@babel/code-frame", + version: "8.0.6", + paths: [["project", "plain"]], + }); + expect(byInstall("js-tokens")?.paths).toEqual([["project", "plain", "js-tokens"]]); + + const graph = loadYarnLockGraph(lockPath)!; + expect(graph.getNode("util-lib@4.17.20")).toEqual({ + name: "util-lib", + version: "4.17.20", + registryName: "lodash", + }); + expect(graph.getNode("lodash@4.17.20")).toEqual({ + name: "lodash", + version: "4.17.20", + }); + expect(graph.getNode("frames@7.23.5")).toEqual({ + name: "frames", + version: "7.23.5", + registryName: "@babel/code-frame", + }); + } finally { + removeDir(projectDir); + } + }); + + it("reports the lodash advisory for a Yarn 1.22 alias of lodash@4.17.20", async () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + const advisoryId = "GHSA-35jh-r3h4-6jhm"; + const advisory: OsvVuln = { + id: advisoryId, + aliases: [], + summary: "lodash command injection", + database_specific: { severity: "HIGH" }, + affected: [{ + package: { name: "lodash", ecosystem: "npm" }, + ranges: [{ type: "ECOSYSTEM", events: [{ introduced: "0" }, { fixed: "4.17.21" }] }], + }], + }; + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ + name: "yarn-alias-demo", + version: "1.0.0", + dependencies: { "util-lib": "npm:lodash@4.17.20" }, + }), + "utf8", + ); + // Produced by yarn 1.22.22 from "util-lib": "npm:lodash@4.17.20". + fs.writeFileSync( + lockPath, + [ + "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.", + "# yarn lockfile v1", + "", + "", + '"util-lib@npm:lodash@4.17.20":', + ' version "4.17.20"', + ' resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.20.tgz#b44a9b6297bcb698f1c51a3545a2b3b368d59c52"', + " integrity sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA==", + "", + ].join("\n"), + "utf8", + ); + + clearPackumentCache(); + const requests: Array<{ url: string; body: unknown }> = []; + const fetchSpy = jest.spyOn(globalThis, "fetch").mockImplementation(async (input, init) => { + const url = String(input); + const body = init?.body ? JSON.parse(String(init.body)) : null; + requests.push({ url, body }); + if (url.endsWith("/v1/querybatch")) { + const queries = (body as { queries: Array<{ package: { name: string }; version: string }> }).queries; + return Response.json({ + results: queries.map(query => ({ + vulns: query.package.name === "lodash" && query.version === "4.17.20" ? [{ id: advisoryId }] : [], + })), + }); + } + if (url.endsWith(`/v1/vulns/${advisoryId}`)) return Response.json(advisory); + if (url.endsWith("/security/advisories/bulk")) return Response.json({}); + if (url.endsWith("/lodash")) { + return Response.json({ + versions: { "4.17.20": {}, "4.17.21": {} }, + time: { "4.17.21": "2021-02-15T00:00:00.000Z" }, + }); + } + return new Response("{}", { status: 404, headers: { "content-type": "application/json" } }); + }); + + try { + const packages = loadFromYarnLock(lockPath); + const installed = packages.find(pkg => pkg.name === "util-lib"); + expect(installed).toMatchObject({ + name: "util-lib", + registryName: "lodash", + version: "4.17.20", + }); + expect(registryPackageName(installed!)).toBe("lodash"); + + const options: ParsedOptions = { + ...parseArgs([]).options, + cacheDir: projectDir, + json: true, + noCache: true, + osvUrl: "https://osv.test", + }; + const { findings } = await scanPackages(packages, 100, options, { + directDependencyNames: new Set(["util-lib"]), + scanSource: "yarn-lock", + scanFilePath: lockPath, + }, undefined, globalThis.fetch as FetchLike); + + expect(findings).toHaveLength(1); + expect(findings[0]?.pkg.version).toBe("4.17.20"); + expect(registryPackageName(findings[0]!.pkg)).toBe("lodash"); + expect(formatPackageName(findings[0]!.pkg)).toBe("util-lib (lodash)"); + expect(findings[0]?.vulnerabilities.map(vuln => vuln.id)).toEqual([advisoryId]); + expect(requests.find(request => request.url.endsWith("/v1/querybatch"))?.body).toEqual({ + queries: [{ package: { name: "lodash", ecosystem: "npm" }, version: "4.17.20" }], + }); + } finally { + fetchSpy.mockRestore(); + clearPackumentCache(); + removeDir(projectDir); + } + }); + }); From fc144bab21cf9eac4181f54e99e409af3f69b120 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Mon, 9 Nov 2026 18:08:32 -0500 Subject: [PATCH 04/21] release: v1.41.1 Version bump only. The three security fixes are already in this branch, carried over from main. --- package-lock.json | 4 ++-- package.json | 2 +- website/docusaurus.config.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 83e5782f5..7815ce164 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cve-lite-cli", - "version": "1.41.0", + "version": "1.41.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cve-lite-cli", - "version": "1.41.0", + "version": "1.41.1", "license": "MIT", "dependencies": { "better-sqlite3": "^12.8.0", diff --git a/package.json b/package.json index fd3b26b5b..829c6e103 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cve-lite-cli", - "version": "1.41.0", + "version": "1.41.1", "description": "Developer-friendly CLI for scanning JS/TS projects for dependency vulnerabilities using local lockfiles and OSV", "type": "module", "bin": { diff --git a/website/docusaurus.config.ts b/website/docusaurus.config.ts index f7ada5d7f..9a484a394 100644 --- a/website/docusaurus.config.ts +++ b/website/docusaurus.config.ts @@ -2,7 +2,7 @@ import {themes as prismThemes} from 'prism-react-renderer'; import type {Config} from '@docusaurus/types'; import type * as Preset from '@docusaurus/preset-classic'; -const latestVersion = 'v1.41.0'; +const latestVersion = 'v1.41.1'; const config: Config = { title: 'CVE Lite CLI', From 804954bd9a82f99b6bbd228b0a44b0577338ec02 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Mon, 9 Nov 2026 18:08:32 -0500 Subject: [PATCH 05/21] docs: changelog for v1.41.1 --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9222147f6..e89276636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to CVE Lite CLI will be documented in this file. ## [Unreleased] +## [1.41.1] - 2026-11-09 + +> **Upgrade note.** This release contains three security fixes and nothing else. If your project's own `.npmrc` or `.yarnrc.yml` points at an internal or plain-HTTP registry, metadata lookups to it are now refused; set the registry in your user-level config or with `NPM_CONFIG_REGISTRY` for the scan instead. See the corporate proxy docs. + +### Security + +- Yarn classic (v1) projects that install a package through an npm alias, such as `"util-lib": "npm:lodash@4.17.20"`, are now scanned under the real package name. The alias was looked up under a name that does not exist, so a vulnerable package reported "No known vulnerabilities found" and passed `--fail-on`. Fix commands keep the alias, for example `yarn add util-lib@npm:lodash@4.18.1` (GHSA-c8j3-f9pr-q5jg). Reported and fixed by @alex-powell. +- Folder names are now quoted in the copyable `cd` fix commands printed in the terminal and the HTML report, including override fix commands. A repository folder named with shell syntax could otherwise run a command when the suggestion was pasted (GHSA-5r8q-h59x-3m7v). Reported and fixed by @vianmangal. +- Registry addresses taken from the scanned repository's `.npmrc` or `.yarnrc.yml` must now use HTTPS and resolve to public addresses, checked for every connection, and redirects are refused, both for remediation lookups and for the override audit. A repository could otherwise make the scanner send requests to internal addresses. Internal registries can still be used through user-level configuration or `NPM_CONFIG_REGISTRY` (GHSA-2v68-94gw-jr4g). Reported and fixed by @vianmangal, and found independently by @Nachiket-Roy. + ## [1.41.0] - 2026-11-05 > **Upgrade note.** This release contains a security fix for custom `--osv-url` endpoints (GHSA-mrvp-qgq4-c66g); upgrade if you point the scanner at your own OSV mirror. The minimum supported Node.js version is now 22, and CI tests Node 22 alongside Node 24. SARIF locations from a subdirectory scan now carry the folder path, so if you add that prefix yourself, drop that step or the path doubles. Projects that ship an `npm-shrinkwrap.json` now get fix commands and complete SBOM dependency links where they previously got neither. From e668a621b9876a9f8aa03c2a89a75a1b7ff54ed6 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Tue, 10 Nov 2026 07:28:17 -0500 Subject: [PATCH 06/21] fix(registry): trust npm-exported registry only when it matches user config npm run and npx export a project's own .npmrc registry to the scanner as lowercase npm_config_registry, which was treated as an explicit user setting and granted private access. The lowercase spelling, and any spelling on Windows, is now trusted only when it is the public registry or matches the user's home config. --- src/utils/registry-config.ts | 44 +++++++++++++++++++++++++++-- tests/override-audit.test.ts | 4 ++- tests/utils/registry-config.test.ts | 33 +++++++++++++++++++++- 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/utils/registry-config.ts b/src/utils/registry-config.ts index df987313a..c803ccc8c 100644 --- a/src/utils/registry-config.ts +++ b/src/utils/registry-config.ts @@ -128,16 +128,24 @@ export function resolveRegistryConfig(args: { packageManager: RegistryConfigPackageManager | null; env?: NodeJS.ProcessEnv; homeDir?: string; + platform?: NodeJS.Platform; }): RegistryConfig { const env = args.env ?? process.env; const homeDir = args.homeDir ?? os.homedir(); + const config = args.packageManager ? MANAGER_CONFIG[args.packageManager] : undefined; + const envRegistry = env.NPM_CONFIG_REGISTRY || env.npm_config_registry; if (envRegistry) { - return { registry: envRegistry, scopes: {}, sourceFile: "NPM_CONFIG_REGISTRY", allowPrivate: true }; + const explicit = Boolean(env.NPM_CONFIG_REGISTRY) && (args.platform ?? process.platform) !== "win32"; + return { + registry: envRegistry, + scopes: {}, + sourceFile: env.NPM_CONFIG_REGISTRY ? "NPM_CONFIG_REGISTRY" : "npm_config_registry", + allowPrivate: explicit || isUserRegistry(envRegistry, config, homeDir, args.projectPath), + }; } - const config = args.packageManager ? MANAGER_CONFIG[args.packageManager] : undefined; if (!config) return { registry: NPM_PUBLIC_REGISTRY, scopes: {}, sourceFile: null }; for (const dir of [args.projectPath, homeDir]) { @@ -163,6 +171,38 @@ export function resolveRegistryConfig(args: { return { registry: NPM_PUBLIC_REGISTRY, scopes: {}, sourceFile: null }; } +/** + * npm and npx export every setting they read, including a project's own `.npmrc`, to + * child processes as lowercase `npm_config_*` variables. So a lowercase registry + * variable may be repository-owned, and only earns trust when it is the public + * registry or the one in the user's own config. Windows environment names ignore + * case, so there even the uppercase spelling cannot be told apart. + */ +function isUserRegistry( + registry: string, + config: { file: string; format: "ini" | "yaml" } | undefined, + homeDir: string, + projectPath: string, +): boolean { + const target = normaliseRegistry(registry); + if (target === normaliseRegistry(NPM_PUBLIC_REGISTRY)) return true; + if (!config) return false; + const file = path.join(homeDir, config.file); + const contents = readIfPresent(file); + if (contents === null || isInsideProject(file, projectPath)) return false; + const home = (config.format === "ini" ? fromIni(contents) : fromYaml(contents)).registry; + return Boolean(home) && normaliseRegistry(home as string) === target; +} + +function normaliseRegistry(registry: string): string { + try { + const url = new URL(registry); + return `${url.origin}${url.pathname.replace(/\/+$/, "")}`; + } catch { + return registry; + } +} + export { packageManagerForSource } from "./package-manager-source.js"; /** The registry a single package resolves through, honouring its scope. */ diff --git a/tests/override-audit.test.ts b/tests/override-audit.test.ts index 92a155a1a..8fd3264be 100644 --- a/tests/override-audit.test.ts +++ b/tests/override-audit.test.ts @@ -66,7 +66,8 @@ describe("runOverrideAudit (#1222)", () => { expect(spy.urls).toEqual([]); }); - it("blocks a project-owned loopback registry for both network rules", async () => { + // npm run and npx export the project's .npmrc as a lowercase npm_config_registry. + it.each([false, true])("blocks a project-owned loopback registry for both network rules (exported by npm: %s)", async exportedByNpm => { project(); let connections = 0; const server = createServer((_req, res) => { @@ -81,6 +82,7 @@ describe("runOverrideAudit (#1222)", () => { try { for (const key of keys) delete process.env[key]; writeFileSync(join(dir, ".npmrc"), `registry=http://127.0.0.1:${port}/\n`); + if (exportedByNpm) process.env.npm_config_registry = `http://127.0.0.1:${port}/`; const outcome = await runOverrideAudit({ ...base(), checkNetwork: true, scanSource: "package-lock" }); expect(connections).toBe(0); const failed = outcome.skipped.filter(s => /registry lookups failed/.test(s.reason)); diff --git a/tests/utils/registry-config.test.ts b/tests/utils/registry-config.test.ts index 66b16a4cc..e399e87b6 100644 --- a/tests/utils/registry-config.test.ts +++ b/tests/utils/registry-config.test.ts @@ -215,6 +215,37 @@ describe("registry configuration ownership", () => { expect(config.allowPrivate).toBe(false); }); it("accepts an explicit environment registry", () => { - expect(resolveRegistryConfig({ projectPath: tmpProject({}), packageManager: "npm", env: { NPM_CONFIG_REGISTRY: "http://127.0.0.1/" } }).allowPrivate).toBe(true); + expect(resolveRegistryConfig({ projectPath: tmpProject({}), homeDir: tmpProject({}), packageManager: "npm", platform: "linux", env: { NPM_CONFIG_REGISTRY: "http://127.0.0.1/" } }).allowPrivate).toBe(true); + }); + // npm run and npx re-export the project's own .npmrc as npm_config_registry. + it("does not trust a lowercase registry exported from the project's .npmrc", () => { + const config = resolveRegistryConfig({ + projectPath: tmpProject({ ".npmrc": "registry=http://127.0.0.1/\n" }), + homeDir: tmpProject({}), + packageManager: "npm", + platform: "linux", + env: { npm_config_registry: "http://127.0.0.1/" }, + }); + expect(config.registry).toBe("http://127.0.0.1/"); + expect(config.sourceFile).toBe("npm_config_registry"); + expect(config.allowPrivate).toBe(false); + }); + it("trusts a lowercase registry that matches the user's own config", () => { + const config = resolveRegistryConfig({ + projectPath: tmpProject({}), + homeDir: tmpProject({ ".npmrc": "registry=http://127.0.0.1/npm/\n" }), + packageManager: "npm", + platform: "linux", + env: { npm_config_registry: "http://127.0.0.1/npm" }, + }); + expect(config.allowPrivate).toBe(true); + }); + it("trusts a lowercase registry that is the public registry", () => { + const config = resolveRegistryConfig({ projectPath: tmpProject({}), homeDir: tmpProject({}), packageManager: "npm", platform: "linux", env: { npm_config_registry: NPM_PUBLIC_REGISTRY } }); + expect(config.allowPrivate).toBe(true); + }); + it("does not trust an uppercase registry on Windows, where npm's lowercase export reads the same", () => { + const config = resolveRegistryConfig({ projectPath: tmpProject({}), homeDir: tmpProject({}), packageManager: "npm", platform: "win32", env: { NPM_CONFIG_REGISTRY: "http://127.0.0.1/" } }); + expect(config.allowPrivate).toBe(false); }); }); From 905b8c3abe25d70519a17baa2658ebf209eb67c4 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Tue, 10 Nov 2026 07:28:17 -0500 Subject: [PATCH 07/21] refactor(registry): use the shared public registry constant in registry fetch --- src/utils/registry-fetch.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/utils/registry-fetch.ts b/src/utils/registry-fetch.ts index 192c67197..311dab9b4 100644 --- a/src/utils/registry-fetch.ts +++ b/src/utils/registry-fetch.ts @@ -1,3 +1,4 @@ +import { NPM_PUBLIC_REGISTRY } from "../constants.js"; import type { FetchLike } from "../types.js"; import { createCheckedAddressPolicy } from "./checked-address-policy.js"; import { createCertAwareFetch } from "./network.js"; @@ -33,7 +34,7 @@ export async function fetchRegistryMetadata( if (target.origin !== registry.origin) throw new Error("Registry request changed origin"); const ca = process.env.NODE_EXTRA_CA_CERTS; - const official = registry.origin === "https://registry.npmjs.org"; + const official = registry.origin === new URL(NPM_PUBLIC_REGISTRY).origin; let transport: FetchLike; if (allowPrivate || official) { transport = fetchImpl ?? (ca && registry.protocol === "https:" ? createCertAwareFetch(ca) : fetch); From 5359a0cb452ec75cfe0f51788070893103b8773e Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Tue, 10 Nov 2026 07:28:18 -0500 Subject: [PATCH 08/21] test(registry): return a fresh Response for each mocked fetch --- tests/npm-registry.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/npm-registry.test.ts b/tests/npm-registry.test.ts index 611ebfa13..f0c105ffe 100644 --- a/tests/npm-registry.test.ts +++ b/tests/npm-registry.test.ts @@ -5,7 +5,7 @@ const fetchMock = jest.fn(); global.fetch = fetchMock; function mockRegistry(versions: string[] = []) { - fetchMock.mockResolvedValue(new Response(JSON.stringify({ + fetchMock.mockImplementation(async () => new Response(JSON.stringify({ versions: Object.fromEntries(versions.map(v => [v, {}])), }))); } @@ -58,7 +58,7 @@ describe("fetchPackument", () => { }); it("returns null for non-ok responses without caching null as rejection", async () => { - fetchMock.mockResolvedValue(new Response(null, { status: 404, statusText: "Not Found" })); + fetchMock.mockImplementation(async () => new Response(null, { status: 404, statusText: "Not Found" })); const result = await fetchPackument("no-such-pkg"); expect(result).toBeNull(); From b729f810296e5e008fadf48c170d4eb476983520 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Tue, 10 Nov 2026 07:28:18 -0500 Subject: [PATCH 09/21] docs: note npm-exported registries in v1.41.1 changelog and proxy docs --- CHANGELOG.md | 4 ++-- website/docs/corporate-proxy.md | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e89276636..2e2ff7617 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,13 @@ All notable changes to CVE Lite CLI will be documented in this file. ## [1.41.1] - 2026-11-09 -> **Upgrade note.** This release contains three security fixes and nothing else. If your project's own `.npmrc` or `.yarnrc.yml` points at an internal or plain-HTTP registry, metadata lookups to it are now refused; set the registry in your user-level config or with `NPM_CONFIG_REGISTRY` for the scan instead. See the corporate proxy docs. +> **Upgrade note.** This release contains three security fixes and nothing else. If your project's own `.npmrc` or `.yarnrc.yml` points at an internal or plain-HTTP registry, metadata lookups to it are now refused; set the registry in your user-level config or with `NPM_CONFIG_REGISTRY` for the scan instead. On Windows, use the user-level config. See the corporate proxy docs. ### Security - Yarn classic (v1) projects that install a package through an npm alias, such as `"util-lib": "npm:lodash@4.17.20"`, are now scanned under the real package name. The alias was looked up under a name that does not exist, so a vulnerable package reported "No known vulnerabilities found" and passed `--fail-on`. Fix commands keep the alias, for example `yarn add util-lib@npm:lodash@4.18.1` (GHSA-c8j3-f9pr-q5jg). Reported and fixed by @alex-powell. - Folder names are now quoted in the copyable `cd` fix commands printed in the terminal and the HTML report, including override fix commands. A repository folder named with shell syntax could otherwise run a command when the suggestion was pasted (GHSA-5r8q-h59x-3m7v). Reported and fixed by @vianmangal. -- Registry addresses taken from the scanned repository's `.npmrc` or `.yarnrc.yml` must now use HTTPS and resolve to public addresses, checked for every connection, and redirects are refused, both for remediation lookups and for the override audit. A repository could otherwise make the scanner send requests to internal addresses. Internal registries can still be used through user-level configuration or `NPM_CONFIG_REGISTRY` (GHSA-2v68-94gw-jr4g). Reported and fixed by @vianmangal, and found independently by @Nachiket-Roy. +- Registry addresses taken from the scanned repository's `.npmrc` or `.yarnrc.yml` must now use HTTPS and resolve to public addresses, checked for every connection, and redirects are refused, both for remediation lookups and for the override audit. This also applies when npm or npx passes the repository's registry on to the scanner as `npm_config_registry`. A repository could otherwise make the scanner send requests to internal addresses. Internal registries can still be used through user-level configuration or `NPM_CONFIG_REGISTRY` (GHSA-2v68-94gw-jr4g). Reported and fixed by @vianmangal, and found independently by @Nachiket-Roy. ## [1.41.0] - 2026-11-05 diff --git a/website/docs/corporate-proxy.md b/website/docs/corporate-proxy.md index 75ac079a5..4d8d5823c 100644 --- a/website/docs/corporate-proxy.md +++ b/website/docs/corporate-proxy.md @@ -147,6 +147,10 @@ NPM_CONFIG_REGISTRY=https://registry.internal/npm/ cve-lite . ``` The environment override selects the default registry for all metadata lookups. +npm and npx pass the project's own `.npmrc` registry on to the scanner as +lowercase `npm_config_registry`, so that spelling is trusted only when it is the +public registry or matches your user-level config. On Windows, where variable +names ignore case, use the user-level config rather than the environment. A user-level config can select scoped registries when the project supplies no registry settings. A home config linked into the scanned project is treated as repository configuration. HTTP is supported only for explicitly trusted registry From 4a719ad7ee80d0872f18b15ff68c33a1ce413394 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Tue, 10 Nov 2026 07:38:39 -0500 Subject: [PATCH 10/21] fix(registry): say how to trust a blocked registry, including on Windows --- src/scanner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scanner.ts b/src/scanner.ts index 20b7ff264..cb2e53f37 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -845,7 +845,7 @@ export async function scanPackages( if (registryPolicyBlocked && registryConfig.sourceFile) { const source = path.basename(registryConfig.sourceFile); - console.warn(`Warning: The registry from the project's ${source} was blocked because repository settings cannot use plain HTTP or private addresses. Set NPM_CONFIG_REGISTRY to a trusted registry for this scan.`); + console.warn(`Warning: The registry from ${source} was blocked because repository settings cannot use plain HTTP or private addresses. To trust it, set it in your user-level config (~/.npmrc or ~/.yarnrc.yml), or with NPM_CONFIG_REGISTRY outside Windows.`); } const severityCounts = countBySeverity(findings); From 0a3b1f4dd9d19a179b710631a6f2cb69251a7fb6 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Tue, 10 Nov 2026 08:17:24 -0500 Subject: [PATCH 11/21] fix(registry): give a recovery step the resolver honours when a registry is blocked A project registry takes precedence over the user's home config, so pointing users at their home config alone does not unblock a scan. --- src/scanner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scanner.ts b/src/scanner.ts index cb2e53f37..878692bfb 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -845,7 +845,7 @@ export async function scanPackages( if (registryPolicyBlocked && registryConfig.sourceFile) { const source = path.basename(registryConfig.sourceFile); - console.warn(`Warning: The registry from ${source} was blocked because repository settings cannot use plain HTTP or private addresses. To trust it, set it in your user-level config (~/.npmrc or ~/.yarnrc.yml), or with NPM_CONFIG_REGISTRY outside Windows.`); + console.warn(`Warning: The registry from ${source} was blocked because repository settings cannot use plain HTTP or private addresses. To use it, set NPM_CONFIG_REGISTRY for the scan, or on Windows remove the registry from the project's config and set it in your user-level config (~/.npmrc or ~/.yarnrc.yml).`); } const severityCounts = countBySeverity(findings); From c3b910abd678159c727c785f194ff26e5f52ef85 Mon Sep 17 00:00:00 2001 From: Rosai Date: Thu, 12 Nov 2026 20:41:54 +0530 Subject: [PATCH 12/21] Merge commit from fork --- src/output/formatters.ts | 9 +- src/output/html-reporter.ts | 3 +- .../detectors/pd001-override-only-phantom.ts | 5 +- .../pd002-transitive-only-phantom.ts | 5 +- src/overrides/detectors/phantom-utils.ts | 9 + src/remediation/fix-commands.ts | 16 +- src/remediation/transitive-chain-resolver.ts | 4 +- src/scanner.ts | 3 +- tests/shell-command-quoting.test.ts | 299 ++++++++++++++++++ 9 files changed, 338 insertions(+), 15 deletions(-) create mode 100644 tests/shell-command-quoting.test.ts diff --git a/src/output/formatters.ts b/src/output/formatters.ts index 5e7714d4b..63bb47722 100644 --- a/src/output/formatters.ts +++ b/src/output/formatters.ts @@ -14,6 +14,7 @@ import { inferSeverity } from "../osv/severity.js"; import { getPrimaryParent, getRootDependencies, isUnconfirmedFix } from "../utils/finding.js"; import { calculatePathCoverage, formatDependencyPath } from "../utils/path-coverage.js"; import { pluralize, isOutputStdout } from "../utils/string.js"; +import { shellQuote } from "../utils/shell-quote.js"; import { hasMaliciousAdvisory } from "../utils/vuln.js"; import { computePrioritySignal } from "../utils/priority-signal.js"; import { computeContextualSignals } from "../utils/contextual-signals.js"; @@ -157,10 +158,12 @@ export function getRecommendedAction(finding: Finding): string { // a degenerate length-2 chain (project -> vulnerable, common when the // package is hoisted to top-level node_modules). The advice is the same // either way - inspect the lockfile or run ` ls` to find the parent. + const inspectMessage = + `No parent dependency was identified for ${finding.pkg.name} in the lockfile. Run \`npm ls ${shellQuote(finding.pkg.name)}\` (or your package manager equivalent) to find which package pulls it in`; if (directTarget) { - return `No parent dependency was identified for ${finding.pkg.name} in the lockfile. Run \`npm ls ${finding.pkg.name}\` (or your package manager equivalent) to find which package pulls it in, then upgrade toward ${directTarget}+.`; + return `${inspectMessage}, then upgrade toward ${directTarget}+.`; } - return `No parent dependency was identified for ${finding.pkg.name} in the lockfile. Run \`npm ls ${finding.pkg.name}\` (or your package manager equivalent) to find which package pulls it in.`; + return `${inspectMessage}.`; } export function summarizeRisk(finding: Finding): string { @@ -286,7 +289,7 @@ export function summarizeNextAction(finding: Finding): string { return `Upgrade ${parent} - no safe version identified. Find a release resolving ${finding.pkg.name} to ${directTarget}+.`; } if (directTarget) { - return `No parent identified in the lockfile. Run \`npm ls ${finding.pkg.name}\` to find the parent, then upgrade toward ${directTarget}+.`; + return `No parent identified in the lockfile. Run \`npm ls ${shellQuote(finding.pkg.name)}\` to find the parent, then upgrade toward ${directTarget}+.`; } return `Inspect the parent dependency chain for ${finding.pkg.name} and choose the safest available upgrade.`; } diff --git a/src/output/html-reporter.ts b/src/output/html-reporter.ts index b3876569e..ec3bf0b0e 100644 --- a/src/output/html-reporter.ts +++ b/src/output/html-reporter.ts @@ -6,6 +6,7 @@ import { serializeFinding, summarizeNextAction, summarizeRisk, formatCooldownWar import { loadLogoDataUri } from "./logos.js"; import { isBreakingUpgrade } from "../utils/version.js"; import { pluralize } from "../utils/string.js"; +import { shellQuote } from "../utils/shell-quote.js"; import { MAL_GIT_SOURCE_PINNED_DISPLAY, MAL_GIT_SOURCE_FLOATING_DISPLAY, severityOrder } from "../constants.js"; import { hasMaliciousAdvisory } from "../utils/vuln.js"; import { renderOverrideFindingsHtml } from "./override-findings-html.js"; @@ -835,7 +836,7 @@ function renderTransitiveContextCol(finding: SerializedFinding): string { return `

Context

✕ No parent identified -

Run npm ls ${escapeHtml(finding.package)} to find which package pulls it in.

+

Run npm ls ${escapeHtml(shellQuote(finding.package))} to find which package pulls it in.

`; } diff --git a/src/overrides/detectors/pd001-override-only-phantom.ts b/src/overrides/detectors/pd001-override-only-phantom.ts index 839799607..b192f3033 100644 --- a/src/overrides/detectors/pd001-override-only-phantom.ts +++ b/src/overrides/detectors/pd001-override-only-phantom.ts @@ -1,6 +1,6 @@ import type { OverrideContext } from "../context.js"; import type { OverrideFinding } from "../types.js"; -import { getDeclaredPackages, installCmd, undeclaredImportFiles } from "./phantom-utils.js"; +import { formatPhantomInstallCommand, getDeclaredPackages, undeclaredImportFiles } from "./phantom-utils.js"; const RULE_ID = "PD001" as const; @@ -10,7 +10,6 @@ export function detect(ctx: OverrideContext): OverrideFinding[] { const declared = getDeclaredPackages(ctx.packageJson); const overrideNames = new Set(ctx.overrideEntries.map((e) => e.packageName)); - const cmd = installCmd(ctx.packageManager); const findings: OverrideFinding[] = []; for (const [pkgName, importedIn] of ctx.importedPackageNames) { @@ -29,7 +28,7 @@ export function detect(ctx: OverrideContext): OverrideFinding[] { details: `Imported in: ${shown.join(", ")}${extra}. ` + `In pnpm strict mode (default on Vercel), override-only packages are not importable and will cause a deploy failure. ` + - `Run: ${cmd} ${pkgName}`, + `Run: ${formatPhantomInstallCommand(ctx.packageManager, pkgName)}`, references: ["https://github.com/OWASP/cve-lite-cli/blob/main/docs/rules/PD001.md"], }); } diff --git a/src/overrides/detectors/pd002-transitive-only-phantom.ts b/src/overrides/detectors/pd002-transitive-only-phantom.ts index e805cd8e8..23e8c81ed 100644 --- a/src/overrides/detectors/pd002-transitive-only-phantom.ts +++ b/src/overrides/detectors/pd002-transitive-only-phantom.ts @@ -1,6 +1,6 @@ import type { OverrideContext } from "../context.js"; import type { OverrideFinding } from "../types.js"; -import { getDeclaredPackages, installCmd, undeclaredImportFiles } from "./phantom-utils.js"; +import { formatPhantomInstallCommand, getDeclaredPackages, undeclaredImportFiles } from "./phantom-utils.js"; const RULE_ID = "PD002" as const; @@ -10,7 +10,6 @@ export function detect(ctx: OverrideContext): OverrideFinding[] { const declared = getDeclaredPackages(ctx.packageJson); const overrideNames = new Set(ctx.overrideEntries.map((e) => e.packageName)); - const cmd = installCmd(ctx.packageManager); const findings: OverrideFinding[] = []; for (const [pkgName, importedIn] of ctx.importedPackageNames) { @@ -30,7 +29,7 @@ export function detect(ctx: OverrideContext): OverrideFinding[] { details: `Imported in: ${shown.join(", ")}${extra}. ` + `If the parent package drops or changes this dependency, your code will break without warning. ` + - `Run: ${cmd} ${pkgName}`, + `Run: ${formatPhantomInstallCommand(ctx.packageManager, pkgName)}`, references: ["https://github.com/OWASP/cve-lite-cli/blob/main/docs/rules/PD002.md"], }); } diff --git a/src/overrides/detectors/phantom-utils.ts b/src/overrides/detectors/phantom-utils.ts index ae7ca1e6d..36c160f19 100644 --- a/src/overrides/detectors/phantom-utils.ts +++ b/src/overrides/detectors/phantom-utils.ts @@ -1,4 +1,5 @@ import type { OverrideContext, WorkspaceMember } from "../context.js"; +import { shellQuote } from "../../utils/shell-quote.js"; export function getDeclaredPackages(packageJson: Record): Set { const declared = new Set(); @@ -52,3 +53,11 @@ export function installCmd(pm: OverrideContext["packageManager"]): string { if (pm === "bun") return "bun add"; return "npm install"; } + +/** Format a copyable phantom-dependency install command with shell-quoted package name. */ +export function formatPhantomInstallCommand( + pm: OverrideContext["packageManager"], + pkgName: string, +): string { + return `${installCmd(pm)} ${shellQuote(pkgName)}`; +} diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index 5e80a27fb..052d89bc0 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -439,7 +439,10 @@ export function buildSuggestedFixCommandPlan( adjusted: false, adjustmentNote: null, reason: buildParentUpgradeReason(finding, coverage), - command: finding.chainResolution.command, + command: buildPackageInstallCommand( + packageManager, + `${finding.chainResolution.directDep}@${finding.chainResolution.targetVersion}`, + ), chainProof: finding.chainResolution.chain, chainSafeVersion: finding.chainResolution.safeVersion, chainVulnerablePackage: finding.pkg.name, @@ -604,6 +607,15 @@ export function commandPrefix(packageManager: SuggestedFixPackageManager): strin return "yarn add"; } +/** Format a single install command with a shell-quoted package spec. */ +export function buildPackageInstallCommand( + packageManager: SuggestedFixPackageManager, + packageSpec: string, + flag = "", +): string { + return `${commandPrefix(packageManager)}${flag} ${shellQuote(packageSpec)}`; +} + function devFlag(packageManager: SuggestedFixPackageManager): string { return packageManager === "bun" ? "--dev" : "-D"; } @@ -704,7 +716,7 @@ export function buildTargetCommand( } if (target.command) return target.command; const flag = target.isDev ? ` ${devFlag(packageManager)}` : ""; - return `${commandPrefix(packageManager)}${flag} ${shellQuote(installPackageSpec(target))}`; + return buildPackageInstallCommand(packageManager, installPackageSpec(target), flag); } export function findSuggestedCommandForFinding( diff --git a/src/remediation/transitive-chain-resolver.ts b/src/remediation/transitive-chain-resolver.ts index 36d409bf4..d964e10ac 100644 --- a/src/remediation/transitive-chain-resolver.ts +++ b/src/remediation/transitive-chain-resolver.ts @@ -1,6 +1,6 @@ import type { Finding, ChainHop, ChainResolution } from "../types.js"; import type { SuggestedFixPackageManager } from "./fix-commands.js"; -import { commandPrefix } from "./fix-commands.js"; +import { buildPackageInstallCommand } from "./fix-commands.js"; import { fetchPackument } from "./npm-registry.js"; import { compareVersions, looksLikeVersion, isPreReleaseVersion, satisfiesRange } from "../utils/version.js"; import { highestVersionSatisfyingRange } from "./npm-transitive-resolution.js"; @@ -60,7 +60,7 @@ export async function resolveChainFix( targetVersion: candidateVersion, chain: traced.chain, safeVersion: traced.safeVersion, - command: `${commandPrefix(packageManager)} ${directDepName}@${candidateVersion}`, + command: buildPackageInstallCommand(packageManager, `${directDepName}@${candidateVersion}`), coveredPaths: 1, totalPaths: finding.dependencyPaths.length, }; diff --git a/src/scanner.ts b/src/scanner.ts index 878692bfb..ad9393322 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -42,6 +42,7 @@ import { buildPnpmWorkspaceMap } from "./parsers/pnpm-lock.js"; import { buildNpmWorkspaceMap } from "./parsers/package-lock.js"; import { buildBunWorkspaceMap } from "./parsers/bun-lock.js"; import { pluralize } from "./utils/string.js"; +import { shellQuote } from "./utils/shell-quote.js"; import path from "node:path"; import { type DebugLogger } from "./output/debug.js"; import { resolveRegistryConfig } from "./utils/registry-config.js"; @@ -686,7 +687,7 @@ export async function scanPackages( updateAnalysisProgress("resolving remediation for", `${finding.pkg.name}@${finding.pkg.version}`); if (aliasRemediationFindings.has(finding)) { - const aliasNote = `Automatic parent remediation is unavailable for paths containing an npm alias. Run \`npm ls ${finding.pkg.name}\` to inspect the declaring dependency, then rescan after updating it.`; + const aliasNote = `Automatic parent remediation is unavailable for paths containing an npm alias. Run \`npm ls ${shellQuote(finding.pkg.name)}\` to inspect the declaring dependency, then rescan after updating it.`; finding.fixVersionValidationNote = [finding.fixVersionValidationNote, aliasNote].filter(Boolean).join(" "); continue; } diff --git a/tests/shell-command-quoting.test.ts b/tests/shell-command-quoting.test.ts new file mode 100644 index 000000000..49b15ecb6 --- /dev/null +++ b/tests/shell-command-quoting.test.ts @@ -0,0 +1,299 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { jest } from "@jest/globals"; +import { getRecommendedAction, summarizeNextAction } from "../src/output/formatters.js"; +import { buildReportData, renderHtmlReport } from "../src/output/html-reporter.js"; +import { detect as detectPd001 } from "../src/overrides/detectors/pd001-override-only-phantom.js"; +import { detect as detectPd002 } from "../src/overrides/detectors/pd002-transitive-only-phantom.js"; +import { clearPackumentCache } from "../src/remediation/npm-registry.js"; +import { buildSuggestedFixCommandPlan, findSuggestedCommandForFinding } from "../src/remediation/fix-commands.js"; +import { resolveChainFix } from "../src/remediation/transitive-chain-resolver.js"; +import { scanPackages } from "../src/scanner.js"; +import { parseArgs } from "../src/cli/args.js"; +import { NULL_AUDIT_LOG } from "../src/audit-log/index.js"; +import type { OverrideContext, OverrideEntry } from "../src/overrides/context.js"; +import type { FetchLike, Finding, PackageRef, ScanInput } from "../src/types.js"; + +const NOOP_LOGGER = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} } as any; + +const INJECTED_NAMES = [ + "two words", + "single'quote", + 'double"quote', + "pkg$(touch marker)", + "pkg`touch marker`", + "pkg; touch marker #", + "nl\ntouch marker", + "pkg && touch marker", + "pkg | touch marker", + "pkg$&", + "literal\\backslash", +]; + +function makeTransitiveFinding(pkgName: string, fixedVersion: string | null = "1.0.1"): Finding { + return { + pkg: { name: pkgName, version: "1.0.0", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-test-0000-0001" }], + severity: "high", + cveAliases: [], + dependencyPaths: [], + relationship: "transitive", + firstFixedVersion: fixedVersion, + validatedFirstFixedVersion: fixedVersion, + }; +} + +function makeOverrideContext( + pkgName: string, + pm: OverrideContext["packageManager"], + mode: "pd001" | "pd002", +): OverrideContext { + const overrideEntries: OverrideEntry[] = mode === "pd001" + ? [{ key: pkgName, packageName: pkgName, value: "1.0.0", path: ["overrides", pkgName], container: "overrides" }] + : []; + return { + projectPath: "/x", + packageJson: { name: "x", version: "1.0.0" }, + packageJsonRaw: "{}", + packageManager: pm, + overrideEntries, + lockfilePackageNames: new Set(mode === "pd002" ? [pkgName] : []), + installedVersions: new Map(), + installedCopies: new Map(), + parentDeclarations: new Map(), + registryDistTags: new Map(), + skippedDetectors: [], + importedPackageNames: new Map([[pkgName, ["src/index.ts"]]]), + auditLog: NULL_AUDIT_LOG, + logger: NOOP_LOGGER, + }; +} + +function extractBetween(text: string, prefix: string, suffix: string): string { + const start = text.indexOf(prefix); + if (start === -1) throw new Error(`Prefix not found in: ${text}`); + const from = start + prefix.length; + if (!suffix) return text.slice(from); + const end = text.indexOf(suffix, from); + if (end === -1) throw new Error(`Suffix not found in: ${text}`); + return text.slice(from, end); +} + +function unescapeHtmlEntities(s: string): string { + return s + .replace(/"/g, '"') + .replace(/>/g, ">") + .replace(/</g, "<") + .replace(/&/g, "&"); +} + +function runShellCommand(shell: string, cwd: string, command: string): string { + // Shadow pm commands with functions printing the package spec argument safely. + const prelude = [ + 'npm() { if [ "$1" = "ls" ] || [ "$1" = "install" ]; then printf "%s" "$2"; fi; }', + 'pnpm() { if [ "$1" = "add" ]; then printf "%s" "$2"; fi; }', + 'yarn() { if [ "$1" = "add" ]; then printf "%s" "$2"; fi; }', + 'bun() { if [ "$1" = "add" ]; then printf "%s" "$2"; fi; }', + ].join("; "); + const res = spawnSync(shell, ["-c", `${prelude}; ${command}`], { + cwd, + encoding: "utf8", + timeout: 5000, + }); + expect(res.status).toBe(0); + expect(fs.existsSync(path.join(cwd, "marker"))).toBe(false); + return res.stdout; +} + +describe("copyable inspection and remediation commands keep ordinary tokens unquoted", () => { + it("preserves ordinary and scoped package names without quotes", () => { + expect(getRecommendedAction(makeTransitiveFinding("lodash", "4.18.1"))).toContain("Run `npm ls lodash`"); + expect(getRecommendedAction(makeTransitiveFinding("@scope/pkg", null))).toContain("Run `npm ls @scope/pkg`"); + expect(summarizeNextAction(makeTransitiveFinding("@scope/pkg", "1.0.1"))).toContain("Run `npm ls @scope/pkg`"); + expect(detectPd001(makeOverrideContext("@scope/pkg", "npm", "pd001"))[0]!.details).toContain("Run: npm install @scope/pkg"); + expect(detectPd002(makeOverrideContext("lodash", "pnpm", "pd002"))[0]!.details).toContain("Run: pnpm add lodash"); + }); +}); + +// Execute generated commands in real POSIX shells to prove metacharacters stay literal arguments. +describe("copyable inspection and remediation commands prevent shell injection", () => { + const shells = ["/bin/sh", "/bin/bash", "/bin/zsh"].filter((s) => fs.existsSync(s)); + if (shells.length === 0) { + it.skip("requires an installed POSIX shell", () => {}); + return; + } + + it.each(shells)("quotes formatter, HTML, and phantom commands safely in %s", (shell) => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "cvel-shell-quote-")); + try { + for (const name of INJECTED_NAMES) { + const withFix = makeTransitiveFinding(name, "1.0.1"); + const noFix = makeTransitiveFinding(name, null); + + const recWithFix = extractBetween( + getRecommendedAction(withFix), + "the lockfile. Run `", + "` (or your package manager equivalent)", + ); + const recNoFix = extractBetween( + getRecommendedAction(noFix), + "the lockfile. Run `", + "` (or your package manager equivalent)", + ); + const nextWithFix = extractBetween( + summarizeNextAction(withFix), + "No parent identified in the lockfile. Run `", + "` to find the parent, then upgrade", + ); + + const html = renderHtmlReport(buildReportData({ + projectPath: "/x", + cliVersion: "1.41.1", + packageManager: "npm", + lockfileSource: "package-lock.json", + packageCount: 1, + findings: [noFix], + suggestedFixCommands: null, + notes: [], + warnings: [], + skippedDependencies: [], + })); + const htmlCmd = unescapeHtmlEntities(extractBetween(html, "Run ", " to find which package pulls it in.")); + + const pd1Cmd = extractBetween(detectPd001(makeOverrideContext(name, "npm", "pd001"))[0]!.details, "Run: ", ""); + const pd2Cmd = extractBetween(detectPd002(makeOverrideContext(name, "pnpm", "pd002"))[0]!.details, "Run: ", ""); + + for (const command of [recWithFix, recNoFix, nextWithFix, htmlCmd, pd1Cmd, pd2Cmd]) { + expect(runShellCommand(shell, tmp, command)).toBe(name); + } + } + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + it.each(shells)("quotes transitive npm alias notes safely in %s", async (shell) => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "cvel-alias-quote-")); + try { + const options = { ...parseArgs([]).options, cacheDir: tmp, json: true, osvUrl: "https://osv.test" }; + const fetchMock: FetchLike = async (input) => { + const url = String(input); + if (url.endsWith("/v1/querybatch")) { + return Response.json({ results: [{ vulns: [{ id: "GHSA-test-alias" }] }] }); + } + if (url.endsWith("/v1/vulns/GHSA-test-alias")) { + return Response.json({ + id: "GHSA-test-alias", + affected: [{ package: { name: "real-pkg", ecosystem: "npm" }, ranges: [{ type: "ECOSYSTEM", events: [{ introduced: "0" }, { fixed: "1.0.1" }] }] }], + }); + } + if (url.endsWith("/real-pkg")) { + return Response.json({ versions: { "1.0.0": {}, "1.0.1": {} }, time: { "1.0.1": "2020-01-01T00:00:00.000Z" } }); + } + return new Response(null, { status: 404 }); + }; + + for (const name of INJECTED_NAMES) { + clearPackumentCache(); + const pkgRef: PackageRef = { + name, + registryName: "real-pkg", + version: "1.0.0", + ecosystem: "npm", + paths: [["project", "parent", name]], + }; + const { findings } = await scanPackages([pkgRef], 100, options, { directDependencyNames: new Set(["parent"]) }, undefined, fetchMock); + expect(findings).toHaveLength(1); + const note = findings[0]!.fixVersionValidationNote ?? ""; + const command = extractBetween(note, "Run `", "` to inspect the declaring dependency"); + expect(runShellCommand(shell, tmp, command)).toBe(name); + } + } finally { + clearPackumentCache(); + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + it.each(shells)("quotes +build version metadata in chain resolution and ignores untrusted prebuilt chain commands in %s", async (shell) => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "cvel-chain-quote-")); + const extraCaPrev = process.env.NODE_EXTRA_CA_CERTS; + delete process.env.NODE_EXTRA_CA_CERTS; + clearPackumentCache(); + + const badVer = "1.0.1+x;touch marker #"; + const fetchSpy = jest.spyOn(globalThis, "fetch").mockImplementation(async (input) => { + const url = String(input); + if (url.endsWith("/parent-dep")) { + return Response.json({ + versions: { + "1.0.0": { dependencies: { "leaf-dep": "1.0.0" } }, + [badVer]: { dependencies: { "leaf-dep": "^1.0.1" } }, + }, + }); + } + if (url.endsWith("/leaf-dep")) { + return Response.json({ + versions: { + "1.0.0": { dependencies: {} }, + "1.0.1": { dependencies: {} }, + }, + }); + } + return new Response(null, { status: 404 }); + }); + + try { + const finding: Finding = { + pkg: { name: "leaf-dep", version: "1.0.0", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-test-chain" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["project", "parent-dep", "leaf-dep"]], + relationship: "transitive", + firstFixedVersion: "1.0.1", + validatedFirstFixedVersion: "1.0.1", + }; + const chain = await resolveChainFix(finding, new Map([["parent-dep", "1.0.0"]]), "npm", { offline: false }); + expect(chain).not.toBeNull(); + expect(runShellCommand(shell, tmp, chain!.command)).toBe(`parent-dep@${badVer}`); + + // Verify fix-commands rebuilds target.command instead of copying an unquoted chainResolution.command. + const tamperedFinding: Finding = { + ...finding, + chainResolution: { + directDep: "parent-dep", + directDepCurrentVersion: "1.0.0", + targetVersion: badVer, + chain: [], + safeVersion: "1.0.1", + command: `npm install parent-dep@${badVer}`, + coveredPaths: 1, + totalPaths: 1, + }, + }; + const scanInput: ScanInput = { + mode: "resolved-lockfile", + source: "package-lock", + filePath: path.join(tmp, "package-lock.json"), + packages: [], + notes: [], + warnings: [], + skippedDependencies: [], + }; + const plan = buildSuggestedFixCommandPlan([tamperedFinding], scanInput); + expect(plan).not.toBeNull(); + const perFindingCmd = findSuggestedCommandForFinding(plan!, tamperedFinding); + expect(perFindingCmd).not.toBeNull(); + expect(runShellCommand(shell, tmp, plan!.targets[0]!.command!)).toBe(`parent-dep@${badVer}`); + expect(runShellCommand(shell, tmp, perFindingCmd!)).toBe(`parent-dep@${badVer}`); + } finally { + fetchSpy.mockRestore(); + clearPackumentCache(); + if (extraCaPrev !== undefined) process.env.NODE_EXTRA_CA_CERTS = extraCaPrev; + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); +}); + From b4f13a9072e54d47d52294e19ccba1fa737cd826 Mon Sep 17 00:00:00 2001 From: Rosai Date: Thu, 12 Nov 2026 20:42:49 +0530 Subject: [PATCH 13/21] Merge commit from fork * fix(npm): resolve registry identities for overrides and peerDependencies aliases * fix(npm): gate peer alias resolution and extract alias module --- src/overrides/parsing/package-json.ts | 2 +- src/parsers/npm-alias-resolution.ts | 324 ++++++++++++++++++++++++++ src/parsers/npm-lock-graph.ts | 146 ++++++++---- src/utils/advisory.ts | 2 +- src/utils/package-json.ts | 1 + tests/e2e/npm-alias.test.ts | 172 ++++++++++++++ tests/parsers/npm-alias.test.ts | 309 ++++++++++++++++++++++++ tests/utils/package-json.test.ts | 20 +- 8 files changed, 929 insertions(+), 47 deletions(-) create mode 100644 src/parsers/npm-alias-resolution.ts diff --git a/src/overrides/parsing/package-json.ts b/src/overrides/parsing/package-json.ts index 292946c16..23d8c40fc 100644 --- a/src/overrides/parsing/package-json.ts +++ b/src/overrides/parsing/package-json.ts @@ -53,7 +53,7 @@ export function bareName(overrideKey: string): string { * or -1 if there is none. For scoped names the leading `@` (index 0) is the * scope, not a specifier, so we look past it. */ -function specifierAt(selector: string): number { +export function specifierAt(selector: string): number { if (selector.startsWith('@')) { return selector.indexOf('@', 1); } diff --git a/src/parsers/npm-alias-resolution.ts b/src/parsers/npm-alias-resolution.ts new file mode 100644 index 000000000..ded9c84f5 --- /dev/null +++ b/src/parsers/npm-alias-resolution.ts @@ -0,0 +1,324 @@ +import fs from "node:fs"; +import path from "node:path"; +import type { NpmLockNode } from "../types.js"; +import { bareName, specifierAt } from "../overrides/parsing/package-json.js"; +import { isGitSource } from "../utils/advisory.js"; +import { isRecord } from "../utils/package-json.js"; +import { npmAliasRegistryName } from "../utils/package-identity.js"; +import { isValidRange, satisfiesRange } from "../utils/version.js"; + +export type RawRootLockMetadata = { + dependencies?: Record; + devDependencies?: Record; + optionalDependencies?: Record; + peerDependencies?: Record; + overrides?: Record; +}; + +export type NpmLockAliasResolutionContext = { + filePath: string; + rootMeta: RawRootLockMetadata | null; + rawLegacyDependencies: unknown; + rootEntryNodeIds: ReadonlySet; + rootRanges: Readonly>; + peerRangesByChildNodeId: ReadonlyMap; + manifestNameByNodeId: ReadonlyMap; + resolvedUrlByNodeId: ReadonlyMap; + ancestorVersionByNodeId: ReadonlyMap; + nodesById: ReadonlyMap; + parentNodeIdsByChildNodeId: ReadonlyMap>; + rangeByParentNodeId: ReadonlyMap>; + peerFallbackEdgeKeys: ReadonlySet; +}; + +type OverrideSelector = { + name: string; + range?: string; +}; + +type OverrideAliasRule = { + parentChain: OverrideSelector[]; + target: OverrideSelector; + registryName: string; +}; + +/** + * Resolve true npm registry identities onto lockfile graph nodes without + * re-attributing non-registry git/tarball packages or un-installed peers. + */ +export function resolveNpmLockAliasIdentities(ctx: NpmLockAliasResolutionContext): void { + const overrideRules = loadOverrideAliasRules(ctx.filePath, ctx.rootMeta); + const legacyAliasByPath = collectLegacyAliasRegistryNames(ctx.rawLegacyDependencies); + + for (const node of ctx.nodesById.values()) { + const registryName = resolveNodeRegistryName(node, ctx, overrideRules, legacyAliasByPath); + if (registryName) { + node.registryName = registryName; + } + } +} + +/** Format the canonical key tracking a fallback edge added from `peerDependencies`. */ +export function peerFallbackEdgeKey(parentNodeId: string, childNodeId: string): string { + return `${parentNodeId}\u0000${childNodeId}`; +} + +function collectRegularDeclaredRanges(node: NpmLockNode, ctx: NpmLockAliasResolutionContext): string[] { + const ranges: string[] = []; + if (ctx.rootEntryNodeIds.has(node.id)) { + const rootRange = ctx.rootRanges[node.name]; + if (rootRange) ranges.push(rootRange); + } + for (const parentId of ctx.parentNodeIdsByChildNodeId.get(node.id) ?? []) { + if (ctx.peerFallbackEdgeKeys.has(peerFallbackEdgeKey(parentId, node.id))) continue; + const range = ctx.rangeByParentNodeId.get(parentId)?.get(node.name); + if (range) ranges.push(range); + } + return ranges; +} + +function resolveNodeRegistryName( + node: NpmLockNode, + ctx: NpmLockAliasResolutionContext, + overrideRules: OverrideAliasRule[], + legacyAliasByPath: Map, +): string | null { + const regularDeclaredRanges = collectRegularDeclaredRanges(node, ctx); + const manifestName = ctx.manifestNameByNodeId.get(node.id); + const resolvedUrl = ctx.resolvedUrlByNodeId.get(node.id); + + // 1. Explicit `npm:` spec on root or regular parent dependencies. + const explicitAlias = regularDeclaredRanges + .map(npmAliasRegistryName) + .find((name) => name && name !== node.name); + if (explicitAlias) return explicitAlias; + + // 2. Lockfile v2 legacy `dependencies` entry. + const legacyAlias = legacyAliasByPath.get(node.packagePath); + if (legacyAlias && legacyAlias !== node.name && (!manifestName || manifestName === legacyAlias)) { + return legacyAlias; + } + + // Remaining paths rely on a differing installed manifest `name`. + if (!manifestName || manifestName === node.name) return null; + + // 3. Peer-derived `npm:` alias: only trust when the node's lockfile manifest `name` + // matches the alias target, so `--legacy-peer-deps` or optional peers cannot + // substitute an alias identity onto a different real package installed in that slot. + if ( + !isGitSource({ resolvedUrl }) && + (regularDeclaredRanges.length === 0 || regularDeclaredRanges.every(looksLikeRegistryVersionSpec)) + ) { + const peerRanges = ctx.peerRangesByChildNodeId.get(node.id) ?? []; + const peerAlias = peerRanges + .map(npmAliasRegistryName) + .find((name) => name === manifestName); + if (peerAlias) return peerAlias; + } + + // 4. Configured `overrides` (package.json or root lockfile metadata). + if (!isGitSource({ resolvedUrl })) { + const matchedRule = overrideRules.find( + (rule) => + rule.target.name === node.name && + rule.registryName === manifestName && + matchesAncestorChain(node.id, rule.parentChain, ctx), + ); + if (matchedRule) return matchedRule.registryName; + } + + // 5. Standalone v3 lockfiles omit `overrides` and replace the child spec only in + // node metadata; accept standard registry tarball layouts (including private + // mirrors) when every regular parent spec is a registry range/tag (#1450). + if ( + !isGitSource({ resolvedUrl }) && + regularDeclaredRanges.length > 0 && + regularDeclaredRanges.every(looksLikeRegistryVersionSpec) && + isRegistryTarballForPackage(resolvedUrl, manifestName, node.version) + ) { + return manifestName; + } + + return null; +} + +function loadOverrideAliasRules( + lockfilePath: string, + rootMeta: RawRootLockMetadata | null, +): OverrideAliasRule[] { + const pkgJson = readJsonObject(path.join(path.dirname(lockfilePath), "package.json")); + const rootManifestDeps = collectRootManifestDeps(rootMeta, pkgJson); + const rules: OverrideAliasRule[] = []; + walkOverrideAliasRules(pkgJson?.overrides, [], rootManifestDeps, rules); + walkOverrideAliasRules(rootMeta?.overrides, [], rootManifestDeps, rules); + return rules; +} + +function readJsonObject(filePath: string): Record | null { + try { + const parsed = JSON.parse(fs.readFileSync(filePath, "utf8")); + return isRecord(parsed) ? parsed : null; + } catch { + return null; + } +} + +function collectRootManifestDeps( + rootMeta: RawRootLockMetadata | null, + pkgJson: Record | null, +): Record { + const deps: Record = {}; + const sections = ["peerDependencies", "devDependencies", "optionalDependencies", "dependencies"] as const; + for (const source of [rootMeta as Record | null, pkgJson]) { + if (!source) continue; + for (const sectionName of sections) { + const section = source[sectionName]; + if (!isRecord(section)) continue; + for (const [name, spec] of Object.entries(section)) { + if (typeof spec === "string" && spec) deps[name] = spec; + } + } + } + return deps; +} + +function walkOverrideAliasRules( + node: unknown, + ancestors: OverrideSelector[], + rootManifestDeps: Record, + rules: OverrideAliasRule[], +): void { + if (!isRecord(node)) return; + + for (const [key, value] of Object.entries(node)) { + if (key === ".") { + if (typeof value === "string" && ancestors.length > 0) { + const registryName = resolveOverrideAliasRegistryName(value, rootManifestDeps); + const target = ancestors[ancestors.length - 1]; + if (registryName && target.name) { + rules.push({ + parentChain: ancestors.slice(0, -1), + target, + registryName, + }); + } + } + continue; + } + + const selector = splitOverrideSelector(key); + if (!selector.name) continue; + + if (typeof value === "string") { + const registryName = resolveOverrideAliasRegistryName(value, rootManifestDeps); + if (registryName) { + rules.push({ parentChain: ancestors, target: selector, registryName }); + } + } else if (isRecord(value)) { + walkOverrideAliasRules(value, [...ancestors, selector], rootManifestDeps, rules); + } + } +} + +function splitOverrideSelector(key: string): OverrideSelector { + const trimmed = key.trim(); + const at = specifierAt(trimmed); + if (at === -1) return { name: trimmed }; + const name = bareName(trimmed); + const range = trimmed.slice(at + 1).trim(); + return range ? { name, range } : { name }; +} + +function resolveOverrideAliasRegistryName( + rawSpec: string, + rootManifestDeps: Record, +): string | null { + const trimmed = rawSpec.trim(); + const resolved = trimmed.startsWith("$") ? rootManifestDeps[trimmed.slice(1)] : trimmed; + return npmAliasRegistryName(resolved); +} + +function matchesAncestorChain( + childNodeId: string, + parentChain: OverrideSelector[], + ctx: Pick, +): boolean { + if (parentChain.length === 0) return true; + + const visited = new Set(); + const stack: Array<{ currentId: string; chainIndex: number }> = []; + for (const parentId of ctx.parentNodeIdsByChildNodeId.get(childNodeId) ?? []) { + stack.push({ currentId: parentId, chainIndex: parentChain.length - 1 }); + } + + while (stack.length > 0) { + const { currentId, chainIndex } = stack.pop()!; + const visitKey = `${currentId}\u0000${chainIndex}`; + if (visited.has(visitKey)) continue; + visited.add(visitKey); + + const parentNode = ctx.nodesById.get(currentId); + if (!parentNode) continue; + + const selector = parentChain[chainIndex]; + const parentVersion = parentNode.version ?? ctx.ancestorVersionByNodeId.get(currentId) ?? null; + const selectorMatches = + parentNode.name === selector.name && + (!selector.range || + !isValidRange(selector.range) || + (parentVersion !== null && satisfiesRange(parentVersion, selector.range))); + + if (selectorMatches) { + if (chainIndex === 0) return true; + for (const grandParentId of ctx.parentNodeIdsByChildNodeId.get(currentId) ?? []) { + stack.push({ currentId: grandParentId, chainIndex: chainIndex - 1 }); + } + } + + // Nested npm overrides apply through intermediate transitive dependencies. + for (const grandParentId of ctx.parentNodeIdsByChildNodeId.get(currentId) ?? []) { + stack.push({ currentId: grandParentId, chainIndex }); + } + } + + return false; +} + +function collectLegacyAliasRegistryNames(deps: unknown): Map { + const byPath = new Map(); + const walk = (entries: unknown, prefix: string): void => { + if (!isRecord(entries)) return; + for (const [name, meta] of Object.entries(entries)) { + if (!isRecord(meta)) continue; + const packagePath = prefix ? `${prefix}/node_modules/${name}` : `node_modules/${name}`; + const registryName = npmAliasRegistryName(meta.version); + if (registryName) byPath.set(packagePath, registryName); + if (meta.dependencies) walk(meta.dependencies, packagePath); + } + }; + walk(deps, ""); + return byPath; +} + +function looksLikeRegistryVersionSpec(spec: string): boolean { + const trimmed = spec.trim(); + if (!trimmed) return false; + if (trimmed.startsWith("npm:")) return true; + if (trimmed.includes(":") || trimmed.includes("/")) return false; + return isValidRange(trimmed) || /^[A-Za-z0-9._-]+$/.test(trimmed); +} + +function isRegistryTarballForPackage( + resolvedUrl: string | undefined, + manifestName: string, + version: string | null, +): boolean { + if (!resolvedUrl || !version) return false; + const cleanUrl = resolvedUrl.split(/[?#]/, 1)[0]; + const unscoped = manifestName.startsWith("@") + ? manifestName.slice(manifestName.indexOf("/") + 1) + : manifestName; + if (!unscoped) return false; + return cleanUrl.endsWith(`/${manifestName}/-/${unscoped}-${version}.tgz`); +} + diff --git a/src/parsers/npm-lock-graph.ts b/src/parsers/npm-lock-graph.ts index 3e65b51c3..4e1cfc915 100644 --- a/src/parsers/npm-lock-graph.ts +++ b/src/parsers/npm-lock-graph.ts @@ -1,18 +1,19 @@ import fs from "node:fs"; import type { NpmLockGraph, NpmLockNode } from "../types.js"; import { unique } from "../utils/array.js"; -import { npmAliasRegistryName } from "../utils/package-identity.js"; - -type RawLockPackage = { +import { isRecord } from "../utils/package-json.js"; +import { + peerFallbackEdgeKey, + resolveNpmLockAliasIdentities, + type RawRootLockMetadata, +} from "./npm-alias-resolution.js"; + +type RawLockPackage = RawRootLockMetadata & { name?: string; version?: string; dev?: boolean; link?: boolean; resolved?: string; - dependencies?: Record; - devDependencies?: Record; - optionalDependencies?: Record; - peerDependencies?: Record; }; export function loadNpmLockGraph( @@ -28,6 +29,9 @@ export function loadNpmLockGraph( const dependencyRangesByNodeId = new Map>(); const resolutionBasePathsByNodeId = new Map(); const peerRangesByNodeId = new Map>(); + const manifestNameByNodeId = new Map(); + const resolvedUrlByNodeId = new Map(); + const ancestorVersionByNodeId = new Map(); const parentNodeIdsByChildNodeId = new Map>(); const childNodeIdsByParentNodeId = new Map>(); const rangeByParentNodeId = new Map>(); @@ -68,9 +72,20 @@ export function loadNpmLockGraph( dev: !!meta?.dev, }); nodeIdByPackagePath.set(packagePath, id); + if (typeof meta?.name === "string" && meta.name) { + manifestNameByNodeId.set(id, meta.name); + } + if (typeof meta?.resolved === "string" && meta.resolved) { + resolvedUrlByNodeId.set(id, meta.resolved); + } const keySet = nodeIdsByPackageKey.get(packageKey) ?? new Set(); keySet.add(id); nodeIdsByPackageKey.set(packageKey, keySet); + const depSource = resolveDependencySource(meta, rawPackages); + const effectiveVersion = version ?? (typeof depSource?.version === "string" ? depSource.version : undefined); + if (effectiveVersion) { + ancestorVersionByNodeId.set(id, effectiveVersion); + } // Workspace / linked packages (link:true, resolved -> a workspace dir) are // installed with their devDependencies, like the root, and are the true roots // of their own dependency subtrees. Collect their full deps (incl. devDeps) and @@ -79,17 +94,18 @@ export function loadNpmLockGraph( dependencyRangesByNodeId.set( id, isLinkNode - ? collectRootDependencyRanges(resolveDependencySource(meta, rawPackages)) - : collectDependencyRanges(resolveDependencySource(meta, rawPackages)), + ? collectRootDependencyRanges(depSource) + : collectDependencyRanges(depSource), ); resolutionBasePathsByNodeId.set(id, resolveBasePaths(packagePath, meta)); - const peerRanges = collectPeerRanges(resolveDependencySource(meta, rawPackages)); + const peerRanges = collectPeerRanges(depSource); if (Object.keys(peerRanges).length > 0) peerRangesByNodeId.set(id, peerRanges); if (isLinkNode) workspaceEntryNodeIds.push(id); } const rootMeta = isRecord(rawPackages[""]) ? rawPackages[""] : null; const rootRanges = collectRootDependencyRanges(rootMeta); + const rootPeerRanges = collectPeerRanges(rootMeta); const entryPackages = resolveEdgesForParent( "", rootRanges, @@ -114,7 +130,14 @@ export function loadNpmLockGraph( const entryNodeIds = unique([...entryPackages, ...workspaceEntryNodeIds]); - applyPeerDependencyFallback({ + applyRootPeerFallback({ + rootPeerRanges, + entryNodeIds, + nodeIdByPackagePath, + parentNodeIdsByChildNodeId, + }); + + const peerFallbackEdgeKeys = applyPeerDependencyFallback({ nodesById, entryNodeIds, peerRangesByNodeId, @@ -125,21 +148,29 @@ export function loadNpmLockGraph( rangeByParentNodeId, }); - // A different manifest name also occurs for git/tarball installs. Only an - // npm: declaration establishes a registry alias for the resolved graph slot. - const rootEntryNodeIds = new Set(entryPackages); - for (const node of nodesById.values()) { - const declaredRanges = [ - rootEntryNodeIds.has(node.id) ? rootRanges[node.name] : undefined, - ...[...(parentNodeIdsByChildNodeId.get(node.id) ?? [])] - .map(parentId => rangeByParentNodeId.get(parentId)?.get(node.name)), - ]; - const registryName = declaredRanges.map(npmAliasRegistryName) - .find(name => name && name !== node.name); - if (registryName) { - node.registryName = registryName; - } - } + const peerRangesByChildNodeId = collectPeerRangesByChild( + nodesById, + rootPeerRanges, + peerRangesByNodeId, + resolutionBasePathsByNodeId, + nodeIdByPackagePath, + ); + + resolveNpmLockAliasIdentities({ + filePath, + rootMeta, + rawLegacyDependencies: raw?.dependencies, + rootEntryNodeIds: new Set(entryPackages), + rootRanges, + peerRangesByChildNodeId, + manifestNameByNodeId, + resolvedUrlByNodeId, + ancestorVersionByNodeId, + nodesById, + parentNodeIdsByChildNodeId, + rangeByParentNodeId, + peerFallbackEdgeKeys, + }); if (options?.includePaths !== false) { const MAX_PATHS_PER_NODE = 5; @@ -183,12 +214,7 @@ export function loadNpmLockGraph( childNodeIdsByParentNodeId: setsToArrays(childNodeIdsByParentNodeId), rangeByParentNodeId, pathSetByNodeId, - peerRangesByChildNodeId: collectPeerRangesByChild( - nodesById, - peerRangesByNodeId, - resolutionBasePathsByNodeId, - nodeIdByPackagePath, - ), + peerRangesByChildNodeId, }); } @@ -256,28 +282,60 @@ function createGraph(args: { }; } +function appendPeerRangesForBasePaths( + peerRanges: Record, + basePaths: string[], + nodeIdByPackagePath: Map, + rangesByChild: Map, +): void { + for (const [peerName, range] of Object.entries(peerRanges)) { + const childPackagePath = resolveDependencyPackagePath(basePaths, peerName, nodeIdByPackagePath); + const childNodeId = childPackagePath ? nodeIdByPackagePath.get(childPackagePath) : undefined; + if (!childNodeId) continue; + rangesByChild.set(childNodeId, [...(rangesByChild.get(childNodeId) ?? []), range]); + } +} + // npm refuses an install whose version breaks a peer requirement, so every // peer range on an installed copy limits what it can be moved to, whether or // not the peer fallback below turned that requirement into an edge. function collectPeerRangesByChild( nodesById: Map, + rootPeerRanges: Record, peerRangesByNodeId: Map>, resolutionBasePathsByNodeId: Map, nodeIdByPackagePath: Map, ): Map { const rangesByChild = new Map(); + appendPeerRangesForBasePaths(rootPeerRanges, [""], nodeIdByPackagePath, rangesByChild); for (const [nodeId, peerRanges] of peerRangesByNodeId) { const basePaths = resolutionBasePathsByNodeId.get(nodeId) ?? [nodesById.get(nodeId)?.packagePath ?? nodeId]; - for (const [peerName, range] of Object.entries(peerRanges)) { - const childPackagePath = resolveDependencyPackagePath(basePaths, peerName, nodeIdByPackagePath); - const childNodeId = childPackagePath ? nodeIdByPackagePath.get(childPackagePath) : undefined; - if (!childNodeId) continue; - rangesByChild.set(childNodeId, [...(rangesByChild.get(childNodeId) ?? []), range]); - } + appendPeerRangesForBasePaths(peerRanges, basePaths, nodeIdByPackagePath, rangesByChild); } return rangesByChild; } +// npm 7+ auto-installs root peerDependencies unless suppressed; keep otherwise-orphaned +// root peer targets anchored at "project" so paths and aliases remain intact. +function applyRootPeerFallback(args: { + rootPeerRanges: Record; + entryNodeIds: string[]; + nodeIdByPackagePath: Map; + parentNodeIdsByChildNodeId: Map>; +}): void { + const entrySet = new Set(args.entryNodeIds); + for (const peerName of Object.keys(args.rootPeerRanges)) { + const childPackagePath = resolveDependencyPackagePath([""], peerName, args.nodeIdByPackagePath); + if (!childPackagePath) continue; + const childNodeId = args.nodeIdByPackagePath.get(childPackagePath); + if (!childNodeId || entrySet.has(childNodeId)) continue; + if ((args.parentNodeIdsByChildNodeId.get(childNodeId)?.size ?? 0) > 0) continue; + + args.entryNodeIds.push(childNodeId); + entrySet.add(childNodeId); + } +} + function resolveEdgesForParent( parentPackagePath: string, dependencyRanges: Record, @@ -327,7 +385,7 @@ function applyPeerDependencyFallback(args: { parentNodeIdsByChildNodeId: Map>; childNodeIdsByParentNodeId: Map>; rangeByParentNodeId: Map>; -}): void { +}): Set { const { nodesById, entryNodeIds, @@ -337,13 +395,14 @@ function applyPeerDependencyFallback(args: { parentNodeIdsByChildNodeId, } = args; + const fallbackEdgeKeys = new Set(); const entryNodeIdSet = new Set(entryNodeIds); const orphanNodeIds = new Set(); for (const nodeId of nodesById.keys()) { const hasParent = (parentNodeIdsByChildNodeId.get(nodeId)?.size ?? 0) > 0; if (!hasParent && !entryNodeIdSet.has(nodeId)) orphanNodeIds.add(nodeId); } - if (orphanNodeIds.size === 0) return; + if (orphanNodeIds.size === 0) return fallbackEdgeKeys; for (const node of nodesById.values()) { const peerRanges = peerRangesByNodeId.get(node.id); @@ -356,8 +415,11 @@ function applyPeerDependencyFallback(args: { if (!childNodeId || !orphanNodeIds.has(childNodeId)) continue; addParentChildEdge(node.id, childNodeId, peerName, range, args); + fallbackEdgeKeys.add(peerFallbackEdgeKey(node.id, childNodeId)); } } + + return fallbackEdgeKeys; } function addParentChildEdge( @@ -489,10 +551,6 @@ function setsToArrays(map: Map>): Map { return result; } -function isRecord(value: unknown): value is Record { - return !!value && typeof value === "object"; -} - function buildPackageKey(name: string, version: string | null): string { return `${name}@${version ?? ""}`; } diff --git a/src/utils/advisory.ts b/src/utils/advisory.ts index 09b1ff26f..b34f0fce4 100644 --- a/src/utils/advisory.ts +++ b/src/utils/advisory.ts @@ -19,7 +19,7 @@ const GIT_SOURCE_PREFIXES = [ "git://", ]; -export function isGitSource(pkg: PackageRef): boolean { +export function isGitSource(pkg: Pick): boolean { return ( pkg.resolvedUrl !== undefined && pkg.resolvedUrl !== "" && diff --git a/src/utils/package-json.ts b/src/utils/package-json.ts index 71b020b00..4e2ac4723 100644 --- a/src/utils/package-json.ts +++ b/src/utils/package-json.ts @@ -71,6 +71,7 @@ function addDependencyNamesFromManifest( const sections: Array | undefined> = [ isRecord(manifest.dependencies) ? manifest.dependencies : undefined, isRecord(manifest.optionalDependencies) ? manifest.optionalDependencies : undefined, + isRecord(manifest.peerDependencies) ? manifest.peerDependencies : undefined, ]; if (!prodOnly) { sections.push(isRecord(manifest.devDependencies) ? manifest.devDependencies : undefined); diff --git a/tests/e2e/npm-alias.test.ts b/tests/e2e/npm-alias.test.ts index ca5342fbe..4b15b1b87 100644 --- a/tests/e2e/npm-alias.test.ts +++ b/tests/e2e/npm-alias.test.ts @@ -66,6 +66,10 @@ describe("non-registry npm dependencies (#1450)", () => { it.each([ { spec: "github:me/fork", resolved: "git+ssh://git@github.com/me/fork.git#abc123" }, { spec: "https://example.test/fork.tgz", resolved: "https://example.test/fork.tgz" }, + { + spec: `https://example.test/${REGISTRY_PACKAGE}/-/${REGISTRY_PACKAGE}-1.0.0.tgz`, + resolved: `https://example.test/${REGISTRY_PACKAGE}/-/${REGISTRY_PACKAGE}-1.0.0.tgz`, + }, ])("does not substitute registry advisories or fixes for $spec", ({ spec, resolved }) => { const dir = mkProject({ "package.json": { name: "fork-app", dependencies: { "my-fork": spec } }, @@ -86,3 +90,171 @@ describe("non-registry npm dependencies (#1450)", () => { } }); }); + +describe("npm overrides and peerDependencies alias detection", () => { + function scan(dir: string, args: string[]) { + const result = runCli([dir, "--offline-db", SEEDED_DB, ...args], { cwd: dir }); + const files = readdirSync(dir).filter(f => f.startsWith("cve-lite-scan-") && f.endsWith(".json")); + expect(files).toHaveLength(1); + return { result, report: JSON.parse(readFileSync(join(dir, files[0]), "utf8")) }; + } + + it("detects vulnerable override alias without tarball fallback and writes registry purl to SBOM", () => { + const projectFiles = { + "package.json": { + name: "override-alias-app", + dependencies: { wrapper: "1.0.0" }, + overrides: { benign: `npm:${REGISTRY_PACKAGE}@1.0.0` }, + }, + "package-lock.json": { + lockfileVersion: 3, + packages: { + "": { name: "override-alias-app", dependencies: { wrapper: "1.0.0" } }, + "node_modules/wrapper": { version: "1.0.0", dependencies: { benign: "2.0.0" } }, + "node_modules/benign": { + name: REGISTRY_PACKAGE, + version: "1.0.0", + }, + }, + }, + }; + const dir = mkProject(projectFiles); + try { + const { result, report } = scan(dir, ["--json", "--fail-on", "high"]); + expect(result.status).toBe(1); + expect(report.findings).toEqual([expect.objectContaining({ + package: "benign", + registryPackage: REGISTRY_PACKAGE, + severity: "high", + relationship: "transitive", + })]); + } finally { + rmProject(dir); + } + + const sbomDir = mkProject(projectFiles); + try { + const sbom = scan(sbomDir, ["--sbom", "cyclonedx", "--sbom-inventory-only"]); + expect(sbom.result.status).toBe(0); + expect(sbom.report.components).toContainEqual(expect.objectContaining({ + name: REGISTRY_PACKAGE, + purl: `pkg:npm/${REGISTRY_PACKAGE}@1.0.0`, + })); + } finally { + rmProject(sbomDir); + } + }); + + it("detects vulnerable root and transitive peerDependencies aliases", () => { + const dir = mkProject({ + "package.json": { + name: "peer-alias-app", + dependencies: { host: "1.0.0", "other-user": "1.0.0" }, + peerDependencies: { "root-peer": `npm:${REGISTRY_PACKAGE}@1.0.0` }, + }, + "package-lock.json": { + lockfileVersion: 3, + packages: { + "": { + name: "peer-alias-app", + dependencies: { host: "1.0.0", "other-user": "1.0.0" }, + peerDependencies: { "root-peer": `npm:${REGISTRY_PACKAGE}@1.0.0` }, + }, + "node_modules/host": { + version: "1.0.0", + peerDependencies: { "shared-peer": `npm:${REGISTRY_PACKAGE}@1.0.0` }, + }, + "node_modules/other-user": { + version: "1.0.0", + dependencies: { "shared-peer": "*" }, + }, + "node_modules/root-peer": { name: REGISTRY_PACKAGE, version: "1.0.0", peer: true }, + "node_modules/shared-peer": { name: REGISTRY_PACKAGE, version: "1.0.0" }, + }, + }, + }); + try { + const { result, report } = scan(dir, ["--json", "--fail-on", "high"]); + expect(result.status).toBe(1); + expect(report.findings).toHaveLength(2); + const rootFinding = report.findings.find((f: { package: string }) => f.package === "root-peer"); + const sharedFinding = report.findings.find((f: { package: string }) => f.package === "shared-peer"); + expect(rootFinding).toMatchObject({ + package: "root-peer", + registryPackage: REGISTRY_PACKAGE, + severity: "high", + relationship: "direct", + dependencyPaths: [["project", "root-peer"]], + }); + expect(rootFinding.nextAction ?? "").not.toContain("No parent"); + expect(sharedFinding).toMatchObject({ + package: "shared-peer", + registryPackage: REGISTRY_PACKAGE, + severity: "high", + relationship: "transitive", + dependencyPaths: [["project", "other-user", "shared-peer"]], + }); + } finally { + rmProject(dir); + } + }); + + it("preserves real installed packages when root peerDependencies aliases are skipped (--legacy-peer-deps)", () => { + const falseNegDir = mkProject({ + "package.json": { + name: "legacy-peer-fn", + dependencies: { wrapper: "1.0.0" }, + peerDependencies: { [REGISTRY_PACKAGE]: "npm:cve-lite-scenario-clean@1.0.0" }, + }, + "package-lock.json": { + lockfileVersion: 3, + packages: { + "": { + name: "legacy-peer-fn", + dependencies: { wrapper: "1.0.0" }, + peerDependencies: { [REGISTRY_PACKAGE]: "npm:cve-lite-scenario-clean@1.0.0" }, + }, + "node_modules/wrapper": { version: "1.0.0", dependencies: { [REGISTRY_PACKAGE]: "1.0.0" } }, + [`node_modules/${REGISTRY_PACKAGE}`]: { version: "1.0.0" }, + }, + }, + }); + const falsePosDir = mkProject({ + "package.json": { + name: "legacy-peer-fp", + dependencies: { wrapper: "1.0.0" }, + peerDependencies: { "cve-lite-scenario-clean": `npm:${REGISTRY_PACKAGE}@1.0.0` }, + }, + "package-lock.json": { + lockfileVersion: 3, + packages: { + "": { + name: "legacy-peer-fp", + dependencies: { wrapper: "1.0.0" }, + peerDependencies: { "cve-lite-scenario-clean": `npm:${REGISTRY_PACKAGE}@1.0.0` }, + }, + "node_modules/wrapper": { version: "1.0.0", dependencies: { "cve-lite-scenario-clean": "1.0.0" } }, + "node_modules/cve-lite-scenario-clean": { version: "1.0.0" }, + }, + }, + }); + try { + const vulnScan = scan(falseNegDir, ["--json", "--fail-on", "high"]); + expect(vulnScan.result.status).toBe(1); + expect(vulnScan.report.findings).toEqual([expect.objectContaining({ + package: REGISTRY_PACKAGE, + severity: "high", + relationship: "transitive", + dependencyPaths: [["project", "wrapper", REGISTRY_PACKAGE]], + })]); + expect(vulnScan.report.findings[0].registryPackage).toBeUndefined(); + + const cleanScan = scan(falsePosDir, ["--json", "--fail-on", "high"]); + expect(cleanScan.result.status).toBe(0); + expect(cleanScan.report.findings).toEqual([]); + } finally { + rmProject(falseNegDir); + rmProject(falsePosDir); + } + }); +}); diff --git a/tests/parsers/npm-alias.test.ts b/tests/parsers/npm-alias.test.ts index 18f13cd55..8c1e24f2e 100644 --- a/tests/parsers/npm-alias.test.ts +++ b/tests/parsers/npm-alias.test.ts @@ -104,3 +104,312 @@ describe("npm aliases in legacy lockfiles", () => { } }); }); + +describe("npm overrides and peerDependencies aliases", () => { + let dir: string; + beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), "cve-lite-npm-alias-ext-")); }); + afterEach(() => { removeDir(dir); }); + + function writeProject(manifest: Record | null, lockfile: Record): string { + if (manifest) { + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify(manifest)); + } + const lockPath = path.join(dir, "package-lock.json"); + fs.writeFileSync(lockPath, JSON.stringify(lockfile)); + return lockPath; + } + + it.each([2, 3])("resolves top-level, versioned, dot (.), and $ref overrides without tarball fallback in lockfile v%s", (lockfileVersion) => { + const lockPath = writeProject( + { + name: "app", + dependencies: { + debug: "4.3.4", + helper: "1.0.0", + "dot-target": "1.0.0", + "my-axios": "npm:axios@0.21.1", + }, + overrides: { + "ms@2.1.2": "npm:lodash@4.17.20", + "@scope/tool@^1.0.0": "npm:@scope/real@2.0.0", + "dot-target": { + ".": "npm:lodash@4.17.20", + child: "1.0.0", + }, + helper: "$my-axios", + }, + }, + { + lockfileVersion, + packages: { + "": { dependencies: { debug: "4.3.4", helper: "1.0.0", "dot-target": "1.0.0", "my-axios": "npm:axios@0.21.1" } }, + "node_modules/debug": { version: "4.3.4", dependencies: { ms: "2.1.2", "@scope/tool": "^1.0.0" } }, + "node_modules/ms": { name: "lodash", version: "4.17.20" }, + "node_modules/@scope/tool": { name: "@scope/real", version: "2.0.0" }, + "node_modules/dot-target": { + name: "lodash", + version: "4.17.20", + resolved: "https://packages.example.test/custom-layout/dot-target.tgz", + }, + "node_modules/helper": { name: "axios", version: "0.21.1" }, + "node_modules/my-axios": { name: "axios", version: "0.21.1" }, + }, + }, + ); + + const packages = loadFromPackageLock(lockPath, false); + expect(packages.find(p => p.name === "ms")).toMatchObject({ + registryName: "lodash", + version: "4.17.20", + paths: [["project", "debug", "ms"]], + }); + expect(packages.find(p => p.name === "@scope/tool")).toMatchObject({ + registryName: "@scope/real", + version: "2.0.0", + paths: [["project", "debug", "@scope/tool"]], + }); + expect(packages.find(p => p.name === "dot-target")).toMatchObject({ + registryName: "lodash", + version: "4.17.20", + paths: [["project", "dot-target"]], + }); + expect(packages.find(p => p.name === "helper")).toMatchObject({ + registryName: "axios", + version: "0.21.1", + paths: [["project", "helper"]], + }); + }); + + it("scopes nested and dot overrides to matching parent chains and ignores un-overridden siblings", () => { + const lockPath = writeProject( + { + name: "app", + dependencies: { "parent-v1": "1.0.0", "parent-v2": "2.0.0", "benign-helper": "1.0.0", "nested-dot": "1.0.0" }, + overrides: { + "parent-v1@^1.0.0": { + "benign-helper": "npm:axios@0.21.1", + "nested-dot": { ".": "npm:@scope/real@2.0.0", sub: "1.0.0" }, + }, + "parent-v2@^1.0.0": { "benign-helper": "npm:axios@0.21.1" }, + }, + }, + { + lockfileVersion: 3, + packages: { + "": { dependencies: { "parent-v1": "1.0.0", "parent-v2": "2.0.0", "benign-helper": "1.0.0", "nested-dot": "1.0.0" } }, + "node_modules/benign-helper": { version: "1.0.0" }, + "node_modules/nested-dot": { version: "1.0.0" }, + "node_modules/parent-v1": { version: "1.0.0", dependencies: { "benign-helper": "1.0.0", "nested-dot": "1.0.0" } }, + "node_modules/parent-v1/node_modules/benign-helper": { name: "axios", version: "0.21.1" }, + "node_modules/parent-v1/node_modules/nested-dot": { name: "@scope/real", version: "2.0.0" }, + "node_modules/parent-v2": { version: "2.0.0", dependencies: { "benign-helper": "1.0.0", "nested-dot": "1.0.0" } }, + "node_modules/parent-v2/node_modules/benign-helper": { name: "axios", version: "0.21.1" }, + "node_modules/parent-v2/node_modules/nested-dot": { name: "@scope/real", version: "2.0.0" }, + }, + }, + ); + + const graph = loadNpmLockGraph(lockPath); + expect(graph.getNode("node_modules/benign-helper")?.registryName).toBeUndefined(); + expect(graph.getNode("node_modules/parent-v1/node_modules/benign-helper")?.registryName).toBe("axios"); + expect(graph.getNode("node_modules/parent-v2/node_modules/benign-helper")?.registryName).toBeUndefined(); + expect(graph.getNode("node_modules/nested-dot")?.registryName).toBeUndefined(); + expect(graph.getNode("node_modules/parent-v1/node_modules/nested-dot")?.registryName).toBe("@scope/real"); + expect(graph.getNode("node_modules/parent-v2/node_modules/nested-dot")?.registryName).toBeUndefined(); + }); + + it("recovers v3 overrides from standalone registry tarballs and v2 legacy deps without sibling package.json", () => { + const v3Standalone = writeProject(null, { + lockfileVersion: 3, + packages: { + "": { dependencies: { debug: "4.3.4" } }, + "node_modules/debug": { version: "4.3.4", dependencies: { ms: "2.1.2" } }, + "node_modules/ms": { + name: "lodash", + version: "4.17.20", + resolved: "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz", + }, + }, + }); + expect(loadFromPackageLock(v3Standalone, false).find(p => p.name === "ms")).toMatchObject({ + registryName: "lodash", + version: "4.17.20", + }); + + const v2NoManifest = writeProject(null, { + lockfileVersion: 2, + packages: { + "": { dependencies: { debug: "4.3.4" } }, + "node_modules/debug": { version: "4.3.4", dependencies: { ms: "2.1.2" } }, + "node_modules/ms": { name: "@scope/real", version: "1.2.3" }, + }, + dependencies: { + debug: { version: "4.3.4", requires: { ms: "2.1.2" } }, + ms: { version: "npm:@scope/real@1.2.3" }, + }, + }); + expect(loadFromPackageLock(v2NoManifest, false).find(p => p.name === "ms")).toMatchObject({ + registryName: "@scope/real", + version: "1.2.3", + }); + }); + + it("ignores stale overrides, mismatched manifest names, git-hosted overrides, and direct non-registry URL specs", () => { + const lockPath = writeProject( + { + name: "app", + dependencies: { + ms: "2.1.2", + mismatched: "1.0.0", + "git-override": "1.0.0", + "codeload-override": "1.0.0", + "my-fork": "https://example.test/lodash/-/lodash-4.17.10.tgz", + }, + overrides: { + ms: "npm:lodash@4.17.20", + mismatched: "npm:lodash@4.17.20", + "git-override": "npm:lodash@4.17.20", + "codeload-override": "npm:lodash@4.17.20", + }, + }, + { + lockfileVersion: 3, + packages: { + "": { + dependencies: { + ms: "2.1.2", + mismatched: "1.0.0", + "git-override": "1.0.0", + "codeload-override": "1.0.0", + "my-fork": "https://example.test/lodash/-/lodash-4.17.10.tgz", + }, + }, + "node_modules/ms": { version: "2.1.2" }, + "node_modules/mismatched": { name: "other-pkg", version: "4.17.20" }, + "node_modules/git-override": { + name: "lodash", + version: "4.17.20", + resolved: "git+ssh://git@github.com/evil/fork.git#abc123", + }, + "node_modules/codeload-override": { + name: "lodash", + version: "4.17.20", + resolved: "https://codeload.github.com/evil/lodash/-/lodash-4.17.20.tgz", + }, + "node_modules/my-fork": { + name: "lodash", + version: "4.17.10", + resolved: "https://example.test/lodash/-/lodash-4.17.10.tgz", + }, + }, + }, + ); + + const packages = loadFromPackageLock(lockPath, false); + expect(packages.find(p => p.name === "ms")?.registryName).toBeUndefined(); + expect(packages.find(p => p.name === "mismatched")?.registryName).toBeUndefined(); + expect(packages.find(p => p.name === "git-override")?.registryName).toBeUndefined(); + expect(packages.find(p => p.name === "codeload-override")?.registryName).toBeUndefined(); + expect(packages.find(p => p.name === "my-fork")?.registryName).toBeUndefined(); + }); + + it("resolves root, workspace, orphan, and non-orphan peerDependencies aliases", () => { + const lockPath = writeProject(null, { + lockfileVersion: 3, + packages: { + "": { + dependencies: { "regular-user": "1.0.0", "peer-declarer": "1.0.0" }, + peerDependencies: { "root-peer": "npm:lodash@4.17.20" }, + }, + "packages/web": { + version: "1.0.0", + peerDependencies: { "ws-peer": "npm:@scope/ws-real@2.0.0" }, + }, + "node_modules/web": { resolved: "packages/web", link: true }, + "node_modules/regular-user": { + version: "1.0.0", + dependencies: { "shared-peer": "*" }, + peerDependencies: { "orphan-peer": "npm:axios@0.21.1" }, + }, + "node_modules/peer-declarer": { + version: "1.0.0", + peerDependencies: { "shared-peer": "npm:minimist@1.2.5" }, + }, + "node_modules/root-peer": { + name: "lodash", + version: "4.17.20", + peer: true, + dependencies: { "root-peer-child": "1.0.0" }, + }, + "node_modules/root-peer-child": { version: "1.0.0", peer: true }, + "node_modules/ws-peer": { name: "@scope/ws-real", version: "2.0.0", peer: true }, + "node_modules/orphan-peer": { name: "axios", version: "0.21.1", peer: true }, + "node_modules/shared-peer": { name: "minimist", version: "1.2.5" }, + }, + }); + + const packages = loadFromPackageLock(lockPath, false); + expect(packages.find(p => p.name === "root-peer")).toMatchObject({ + registryName: "lodash", + version: "4.17.20", + paths: [["project", "root-peer"]], + }); + expect(packages.find(p => p.name === "root-peer-child")?.paths).toEqual([ + ["project", "root-peer", "root-peer-child"], + ]); + expect(packages.find(p => p.name === "ws-peer")).toMatchObject({ + registryName: "@scope/ws-real", + version: "2.0.0", + paths: [["project", "web", "ws-peer"]], + }); + expect(packages.find(p => p.name === "orphan-peer")).toMatchObject({ + registryName: "axios", + version: "0.21.1", + paths: [["project", "regular-user", "orphan-peer"]], + }); + expect(packages.find(p => p.name === "shared-peer")).toMatchObject({ + registryName: "minimist", + version: "1.2.5", + paths: [["project", "regular-user", "shared-peer"]], + }); + }); + + it("ignores peer-derived aliases when the installed node does not match (--legacy-peer-deps / optional peer)", () => { + const lockPath = writeProject( + { + name: "legacy-peer-app", + dependencies: { debug: "2.6.0", host: "1.0.0" }, + peerDependencies: { ms: "npm:left-pad@1.3.0" }, + }, + { + lockfileVersion: 3, + packages: { + "": { + name: "legacy-peer-app", + dependencies: { debug: "2.6.0", host: "1.0.0" }, + peerDependencies: { ms: "npm:left-pad@1.3.0" }, + }, + "node_modules/debug": { version: "2.6.0", dependencies: { ms: "0.7.2" } }, + "node_modules/ms": { + version: "0.7.2", + resolved: "https://registry.npmjs.org/ms/-/ms-0.7.2.tgz", + }, + "node_modules/host": { + version: "1.0.0", + peerDependencies: { "opt-peer": "npm:left-pad@1.3.0" }, + peerDependenciesMeta: { "opt-peer": { optional: true } }, + }, + "node_modules/opt-peer": { name: "other-pkg", version: "1.0.0" }, + }, + }, + ); + + const packages = loadFromPackageLock(lockPath, false); + expect(packages.find(p => p.name === "ms")).toMatchObject({ + version: "0.7.2", + paths: [["project", "debug", "ms"]], + }); + expect(packages.find(p => p.name === "ms")?.registryName).toBeUndefined(); + expect(packages.find(p => p.name === "opt-peer")?.registryName).toBeUndefined(); + }); +}); diff --git a/tests/utils/package-json.test.ts b/tests/utils/package-json.test.ts index b460c5c2f..bf5c3cc99 100644 --- a/tests/utils/package-json.test.ts +++ b/tests/utils/package-json.test.ts @@ -1,7 +1,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { hasOverrideEntries } from "../../src/utils/package-json.js"; +import { hasOverrideEntries, readDirectDependencyNames } from "../../src/utils/package-json.js"; function makeTempDir(): string { return fs.mkdtempSync(path.join(os.tmpdir(), "pkg-json-test-")); @@ -65,3 +65,21 @@ describe("hasOverrideEntries", () => { expect(hasOverrideEntries(dir)).toBe(false); }); }); + +describe("readDirectDependencyNames", () => { + it("includes dependencies, optionalDependencies, peerDependencies, and non-prod devDependencies", () => { + const dir = makeTempDir(); + try { + writePackageJson(dir, { + dependencies: { a: "1.0.0" }, + optionalDependencies: { b: "1.0.0" }, + peerDependencies: { c: "npm:lodash@4.17.20" }, + devDependencies: { d: "1.0.0" }, + }); + expect([...(readDirectDependencyNames(dir, false) ?? [])].sort()).toEqual(["a", "b", "c", "d"]); + expect([...(readDirectDependencyNames(dir, true) ?? [])].sort()).toEqual(["a", "b", "c"]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); From c536e9bf90fde5d290c71c02c6952a2383bc40cf Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Thu, 12 Nov 2026 09:27:27 -0600 Subject: [PATCH 14/21] release: v1.41.2 --- package-lock.json | 4 ++-- package.json | 2 +- website/docusaurus.config.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7815ce164..de893c7af 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cve-lite-cli", - "version": "1.41.1", + "version": "1.41.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cve-lite-cli", - "version": "1.41.1", + "version": "1.41.2", "license": "MIT", "dependencies": { "better-sqlite3": "^12.8.0", diff --git a/package.json b/package.json index 829c6e103..3c64ea71f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cve-lite-cli", - "version": "1.41.1", + "version": "1.41.2", "description": "Developer-friendly CLI for scanning JS/TS projects for dependency vulnerabilities using local lockfiles and OSV", "type": "module", "bin": { diff --git a/website/docusaurus.config.ts b/website/docusaurus.config.ts index 9a484a394..8fd30bace 100644 --- a/website/docusaurus.config.ts +++ b/website/docusaurus.config.ts @@ -2,7 +2,7 @@ import {themes as prismThemes} from 'prism-react-renderer'; import type {Config} from '@docusaurus/types'; import type * as Preset from '@docusaurus/preset-classic'; -const latestVersion = 'v1.41.1'; +const latestVersion = 'v1.41.2'; const config: Config = { title: 'CVE Lite CLI', From 421ca7748f4dedda2a76f41913d0345f431cbcfc Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Thu, 12 Nov 2026 09:27:27 -0600 Subject: [PATCH 15/21] docs: changelog for v1.41.2 Also corrects the 1.41.1 release date and its upgrade note, which told users to set the registry in their user-level config without saying the project's own config has to drop it first. --- CHANGELOG.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e2ff7617..c5212e390 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,18 @@ All notable changes to CVE Lite CLI will be documented in this file. ## [Unreleased] -## [1.41.1] - 2026-11-09 +## [1.41.2] - 2026-11-12 -> **Upgrade note.** This release contains three security fixes and nothing else. If your project's own `.npmrc` or `.yarnrc.yml` points at an internal or plain-HTTP registry, metadata lookups to it are now refused; set the registry in your user-level config or with `NPM_CONFIG_REGISTRY` for the scan instead. On Windows, use the user-level config. See the corporate proxy docs. +> **Upgrade note.** This release contains two security fixes and nothing else. + +### Security + +- Package names read from a lockfile or `package.json` are now quoted in every copyable command the scanner prints: the `npm ls` inspection hints, the install commands for phantom dependencies (PD001 and PD002), and chain-resolution and fix-plan commands, in the terminal, the HTML report and JSON. A repository could name a package with shell syntax so that pasting the suggested command ran something else (GHSA-9v96-p5c5-2frh). Reported and fixed by @Nachiket-Roy. +- Packages installed through an npm alias in `overrides` or `peerDependencies` are now scanned under their real registry name, including nested, versioned and `$` references in `overrides` and lockfile v2 and v3. They were looked up under the alias, so a vulnerable package reported clean and passed `--fail-on`. A peer alias is only applied when the installed package really is the aliased one, so a peer that npm did not install cannot relabel another package (GHSA-6vp2-w78m-6gf7). Reported and fixed by @Nachiket-Roy. + +## [1.41.1] - 2026-11-10 + +> **Upgrade note.** This release contains three security fixes and nothing else. If your project's own `.npmrc` or `.yarnrc.yml` points at an internal or plain-HTTP registry, metadata lookups to it are now refused. To use it for a scan, set `NPM_CONFIG_REGISTRY`, or remove the registry from the project's config and set it in your user-level config (`~/.npmrc` or `~/.yarnrc.yml`); on Windows, use the user-level config. See the corporate proxy docs. ### Security From 09c338f9effe0428a9c0f13fd70a23051148be6e Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Thu, 12 Nov 2026 23:16:25 -0500 Subject: [PATCH 16/21] Merge commit from fork * fix(security): add a check for names that are unsafe in any common shell shellQuote is POSIX single-quote quoting. cmd.exe ignores single quotes and PowerShell reads the POSIX escape for a quote differently, so a name with & | < > ^ % ! quotes, a backtick or a control character can still act there. isSafeInEveryShell flags those, and npmLsCommand returns no command for them. * fix(security): withhold fix commands that are unsafe to paste in some shell A target whose package, version or workspace name fails the check gets no command and each affected finding is listed as skipped with the reason, under its own package so a parent-update target cannot hide the child. A folder name that fails the check withholds every command for that folder. * fix(security): withhold unsafe inspection, phantom and override commands The npm ls hints, the PD001 and PD002 install commands and the override --fix command give a plain instruction instead of a command when the package or folder name is not safe to paste into every shell. * test: expect no command for names unsafe in some shell Names that only POSIX quoting can neutralise now get no command; the real-shell execution checks stay for the rest. * test: pin which names withhold a command and how skips are reported Covers the shell check on both sides, an unsafe package next to a safe one, an unsafe folder naming each affected package including a child refreshed through its parent, a folder with only a space, and the override command. * test: fill the override context fields the shell quoting test omitted The context was missing three fields and passed possibly-undefined details to a string helper; both were type errors carried from the test's original version, which the CI check now counts because this branch edits the file. --- src/output/formatters.ts | 10 +- src/output/html-reporter.ts | 9 +- src/output/override-findings-terminal.ts | 6 +- src/output/override-fix-command.ts | 10 +- .../detectors/pd001-override-only-phantom.ts | 4 +- .../pd002-transitive-only-phantom.ts | 4 +- src/overrides/detectors/phantom-utils.ts | 11 +- src/remediation/fix-commands.ts | 57 +++++++++- src/scanner.ts | 6 +- src/utils/shell-quote.ts | 21 ++++ tests/fix-commands-subfolder.test.ts | 14 ++- tests/shell-command-quoting.test.ts | 30 +++++- tests/utils/shell-safe-commands.test.ts | 100 ++++++++++++++++++ 13 files changed, 255 insertions(+), 27 deletions(-) create mode 100644 tests/utils/shell-safe-commands.test.ts diff --git a/src/output/formatters.ts b/src/output/formatters.ts index 63bb47722..6adf8e82c 100644 --- a/src/output/formatters.ts +++ b/src/output/formatters.ts @@ -14,7 +14,7 @@ import { inferSeverity } from "../osv/severity.js"; import { getPrimaryParent, getRootDependencies, isUnconfirmedFix } from "../utils/finding.js"; import { calculatePathCoverage, formatDependencyPath } from "../utils/path-coverage.js"; import { pluralize, isOutputStdout } from "../utils/string.js"; -import { shellQuote } from "../utils/shell-quote.js"; +import { npmLsCommand, UNSAFE_LS_INSTRUCTION } from "../utils/shell-quote.js"; import { hasMaliciousAdvisory } from "../utils/vuln.js"; import { computePrioritySignal } from "../utils/priority-signal.js"; import { computeContextualSignals } from "../utils/contextual-signals.js"; @@ -158,8 +158,10 @@ export function getRecommendedAction(finding: Finding): string { // a degenerate length-2 chain (project -> vulnerable, common when the // package is hoisted to top-level node_modules). The advice is the same // either way - inspect the lockfile or run ` ls` to find the parent. + const lsCommand = npmLsCommand(finding.pkg.name); + const lookup = lsCommand ? `Run \`${lsCommand}\` (or your package manager equivalent)` : UNSAFE_LS_INSTRUCTION; const inspectMessage = - `No parent dependency was identified for ${finding.pkg.name} in the lockfile. Run \`npm ls ${shellQuote(finding.pkg.name)}\` (or your package manager equivalent) to find which package pulls it in`; + `No parent dependency was identified for ${finding.pkg.name} in the lockfile. ${lookup} to find which package pulls it in`; if (directTarget) { return `${inspectMessage}, then upgrade toward ${directTarget}+.`; } @@ -289,7 +291,9 @@ export function summarizeNextAction(finding: Finding): string { return `Upgrade ${parent} - no safe version identified. Find a release resolving ${finding.pkg.name} to ${directTarget}+.`; } if (directTarget) { - return `No parent identified in the lockfile. Run \`npm ls ${shellQuote(finding.pkg.name)}\` to find the parent, then upgrade toward ${directTarget}+.`; + const lsCommand = npmLsCommand(finding.pkg.name); + const lookup = lsCommand ? `Run \`${lsCommand}\`` : UNSAFE_LS_INSTRUCTION; + return `No parent identified in the lockfile. ${lookup} to find the parent, then upgrade toward ${directTarget}+.`; } return `Inspect the parent dependency chain for ${finding.pkg.name} and choose the safest available upgrade.`; } diff --git a/src/output/html-reporter.ts b/src/output/html-reporter.ts index ec3bf0b0e..abcecd9fb 100644 --- a/src/output/html-reporter.ts +++ b/src/output/html-reporter.ts @@ -6,7 +6,7 @@ import { serializeFinding, summarizeNextAction, summarizeRisk, formatCooldownWar import { loadLogoDataUri } from "./logos.js"; import { isBreakingUpgrade } from "../utils/version.js"; import { pluralize } from "../utils/string.js"; -import { shellQuote } from "../utils/shell-quote.js"; +import { npmLsCommand, UNSAFE_LS_INSTRUCTION } from "../utils/shell-quote.js"; import { MAL_GIT_SOURCE_PINNED_DISPLAY, MAL_GIT_SOURCE_FLOATING_DISPLAY, severityOrder } from "../constants.js"; import { hasMaliciousAdvisory } from "../utils/vuln.js"; import { renderOverrideFindingsHtml } from "./override-findings-html.js"; @@ -836,7 +836,7 @@ function renderTransitiveContextCol(finding: SerializedFinding): string { return `

Context

✕ No parent identified -

Run npm ls ${escapeHtml(shellQuote(finding.package))} to find which package pulls it in.

+

${lsLookupHtml(finding.package)} to find which package pulls it in.

`; } @@ -1145,3 +1145,8 @@ export function openInBrowser(filePath: string): void { // Best-effort only: report creation should not fail if the OS open command is unavailable. } } + +function lsLookupHtml(packageName: string): string { + const lsCommand = npmLsCommand(packageName); + return lsCommand ? `Run ${escapeHtml(lsCommand)}` : escapeHtml(UNSAFE_LS_INSTRUCTION); +} diff --git a/src/output/override-findings-terminal.ts b/src/output/override-findings-terminal.ts index e52601e22..00cb78898 100644 --- a/src/output/override-findings-terminal.ts +++ b/src/output/override-findings-terminal.ts @@ -6,7 +6,7 @@ import type { SeverityLabel } from "../types.js"; import { chalk, stripAnsi } from "../utils/chalk.js"; import { formatSeverityLabel, SEVERITY_ORDER } from "../utils/severity.js"; import { pluralize } from "../utils/string.js"; -import { buildOverrideFixCommand, overrideCommandTargetSuffix } from "./override-fix-command.js"; +import { buildOverrideFixCommand, isPasteableOverrideTarget, overrideCommandTargetSuffix } from "./override-fix-command.js"; const COL_RULE_MAX = 8; const COL_PKG_MAX = 30; @@ -96,7 +96,7 @@ export function renderOverrideFindings( if (f.fix?.runnableCommand && !seenRules.has(f.ruleId)) { seenRules.add(f.ruleId); const cmd = buildOverrideFixCommand(f.fix.runnableCommand, commandTarget); - lines.push(chalk.bold.cyan(`> ${cmd}`)); + if (cmd) lines.push(chalk.bold.cyan(`> ${cmd}`)); } } @@ -107,7 +107,7 @@ export function renderOverrideFindings( const allFixableRules = new Set( findings.filter(f => f.fix?.runnableCommand).map(f => f.ruleId), ); - if (allFixableRules.size >= 2) { + if (allFixableRules.size >= 2 && isPasteableOverrideTarget(commandTarget)) { const fixAllCmd = `cve-lite${projectInfix} overrides --fix`; lines.push("💡 " + chalk.gray(`Tip: run ${fixAllCmd} to apply all auto-fixable fixes in one shot.`)); lines.push(""); diff --git a/src/output/override-fix-command.ts b/src/output/override-fix-command.ts index 94ee0a056..d6e332782 100644 --- a/src/output/override-fix-command.ts +++ b/src/output/override-fix-command.ts @@ -1,5 +1,5 @@ import { OVERRIDES_COMMAND } from "../constants.js"; -import { quoteShellPath } from "../utils/shell-quote.js"; +import { isSafeInEveryShell, quoteShellPath } from "../utils/shell-quote.js"; // HTML callers pass a project-relative path, and terminal callers pass a // cwd-relative path. An empty target or "." needs no positional argument. @@ -7,10 +7,16 @@ export function overrideCommandTargetSuffix(commandTarget?: string): string { return commandTarget && commandTarget !== "." ? ` ${quoteShellPath(commandTarget)}` : ""; } +/** False when the folder name is not safe to paste into every shell, so no command should be printed for it. */ +export function isPasteableOverrideTarget(commandTarget?: string): boolean { + return !commandTarget || isSafeInEveryShell(commandTarget); +} + export function buildOverrideFixCommand( runnableCommand: string, commandTarget?: string, -): string { +): string | null { + if (!isPasteableOverrideTarget(commandTarget)) return null; const suffix = overrideCommandTargetSuffix(commandTarget); return runnableCommand.replace(OVERRIDES_COMMAND, () => `cve-lite${suffix} overrides`); } diff --git a/src/overrides/detectors/pd001-override-only-phantom.ts b/src/overrides/detectors/pd001-override-only-phantom.ts index b192f3033..09196fb1a 100644 --- a/src/overrides/detectors/pd001-override-only-phantom.ts +++ b/src/overrides/detectors/pd001-override-only-phantom.ts @@ -1,6 +1,6 @@ import type { OverrideContext } from "../context.js"; import type { OverrideFinding } from "../types.js"; -import { formatPhantomInstallCommand, getDeclaredPackages, undeclaredImportFiles } from "./phantom-utils.js"; +import { getDeclaredPackages, phantomInstallInstruction, undeclaredImportFiles } from "./phantom-utils.js"; const RULE_ID = "PD001" as const; @@ -28,7 +28,7 @@ export function detect(ctx: OverrideContext): OverrideFinding[] { details: `Imported in: ${shown.join(", ")}${extra}. ` + `In pnpm strict mode (default on Vercel), override-only packages are not importable and will cause a deploy failure. ` + - `Run: ${formatPhantomInstallCommand(ctx.packageManager, pkgName)}`, + phantomInstallInstruction(ctx.packageManager, pkgName), references: ["https://github.com/OWASP/cve-lite-cli/blob/main/docs/rules/PD001.md"], }); } diff --git a/src/overrides/detectors/pd002-transitive-only-phantom.ts b/src/overrides/detectors/pd002-transitive-only-phantom.ts index 23e8c81ed..203eb955b 100644 --- a/src/overrides/detectors/pd002-transitive-only-phantom.ts +++ b/src/overrides/detectors/pd002-transitive-only-phantom.ts @@ -1,6 +1,6 @@ import type { OverrideContext } from "../context.js"; import type { OverrideFinding } from "../types.js"; -import { formatPhantomInstallCommand, getDeclaredPackages, undeclaredImportFiles } from "./phantom-utils.js"; +import { getDeclaredPackages, phantomInstallInstruction, undeclaredImportFiles } from "./phantom-utils.js"; const RULE_ID = "PD002" as const; @@ -29,7 +29,7 @@ export function detect(ctx: OverrideContext): OverrideFinding[] { details: `Imported in: ${shown.join(", ")}${extra}. ` + `If the parent package drops or changes this dependency, your code will break without warning. ` + - `Run: ${formatPhantomInstallCommand(ctx.packageManager, pkgName)}`, + phantomInstallInstruction(ctx.packageManager, pkgName), references: ["https://github.com/OWASP/cve-lite-cli/blob/main/docs/rules/PD002.md"], }); } diff --git a/src/overrides/detectors/phantom-utils.ts b/src/overrides/detectors/phantom-utils.ts index 36c160f19..66338a8be 100644 --- a/src/overrides/detectors/phantom-utils.ts +++ b/src/overrides/detectors/phantom-utils.ts @@ -1,5 +1,5 @@ import type { OverrideContext, WorkspaceMember } from "../context.js"; -import { shellQuote } from "../../utils/shell-quote.js"; +import { isSafeInEveryShell, shellQuote } from "../../utils/shell-quote.js"; export function getDeclaredPackages(packageJson: Record): Set { const declared = new Set(); @@ -54,10 +54,13 @@ export function installCmd(pm: OverrideContext["packageManager"]): string { return "npm install"; } -/** Format a copyable phantom-dependency install command with shell-quoted package name. */ -export function formatPhantomInstallCommand( +/** The closing instruction for a phantom-dependency finding: a copyable install command, or a plain step when the name is not safe in every shell. */ +export function phantomInstallInstruction( pm: OverrideContext["packageManager"], pkgName: string, ): string { - return `${installCmd(pm)} ${shellQuote(pkgName)}`; + if (!isSafeInEveryShell(pkgName)) { + return "Declare it in package.json (no command is shown because the name is not safe to paste into every shell)."; + } + return `Run: ${installCmd(pm)} ${shellQuote(pkgName)}`; } diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index 052d89bc0..a348abc48 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -6,7 +6,7 @@ import { getPrimaryParent } from "../utils/finding.js"; import { isNpmLockSource, packageManagerForSource } from "../utils/package-manager-source.js"; import { calculatePathCoverage, formatDependencyPath } from "../utils/path-coverage.js"; import { pluralize } from "../utils/string.js"; -import { quoteShellPath, shellQuote } from "../utils/shell-quote.js"; +import { isSafeInEveryShell, quoteShellPath, shellQuote } from "../utils/shell-quote.js"; import { buildNpmWorkspaceMap } from "../parsers/package-lock.js"; import { buildPnpmWorkspaceMap } from "../parsers/pnpm-lock.js"; import { buildYarnWorkspaceMap } from "../parsers/yarn-lock.js"; @@ -475,7 +475,7 @@ export function buildSuggestedFixCommandPlan( }); } - const targets = [...targetsByKey.values()].sort((a, b) => { + const sortedTargets = [...targetsByKey.values()].sort((a, b) => { const sevDelta = severityOrder[b.severity] - severityOrder[a.severity]; if (sevDelta !== 0) return sevDelta; @@ -487,6 +487,7 @@ export function buildSuggestedFixCommandPlan( if (kindDelta !== 0) return kindDelta; return a.package.localeCompare(b.package); }); + const targets = withholdShellUnsafeTargets(sortedTargets, findings, skippedByKey, options?.subfolder); // Some install targets carry an explicit `command` built without workspace // context (e.g. a chain-resolution command). This raw field is exposed in JSON @@ -825,6 +826,58 @@ export function planVersionForFinding( return target.targetVersion; } +export const UNSAFE_PACKAGE_REASON = + "The package or workspace name contains characters that are not safe to paste into every shell, so no command is offered. Check the name in your lockfile before running anything for it."; +export const UNSAFE_FOLDER_REASON = + "The folder name contains characters that are not safe to paste into every shell, so no command is offered for this folder."; + +// Every token a target's printed command is built from. A token that would +// still act in cmd.exe or PowerShell despite our quoting withholds the command. +function unsafeShellReason(target: SuggestedFixTarget): string | null { + const tokens = [ + target.package, + target.registryName, + target.targetVersion, + target.childPackage, + target.childTargetVersion, + ...(target.workspaces ?? []), + ]; + return tokens.every(token => token === undefined || isSafeInEveryShell(token)) ? null : UNSAFE_PACKAGE_REASON; +} + +// Drops targets whose printed command could not be pasted safely into every +// shell, and records a skip for each finding that loses its command, under the +// finding's own package so a parent-update target named after its parent cannot +// hide the child it was refreshing. +function withholdShellUnsafeTargets( + sortedTargets: SuggestedFixTarget[], + findings: Finding[], + skippedByKey: Map, + subfolder: string | undefined, +): SuggestedFixTarget[] { + const unsafeFolder = !!subfolder && !isSafeInEveryShell(subfolder); + const kept: SuggestedFixTarget[] = []; + const withheld = new Map(); + for (const target of sortedTargets) { + const reason = unsafeFolder ? UNSAFE_FOLDER_REASON : unsafeShellReason(target); + if (reason) withheld.set(target, reason); + else kept.push(target); + } + if (withheld.size === 0) return kept; + const withheldPlan = { targets: [...withheld.keys()] } as SuggestedFixCommandPlan; + for (const finding of findings) { + const target = findFixTargetForFinding(withheldPlan, finding); + if (!target) continue; + skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { + package: finding.pkg.name, + version: finding.pkg.version, + relationship: finding.relationship, + reason: withheld.get(target)!, + }); + } + return kept; +} + function upsertTarget( targetsByKey: Map, finding: Finding, diff --git a/src/scanner.ts b/src/scanner.ts index ad9393322..ee4408875 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -42,7 +42,7 @@ import { buildPnpmWorkspaceMap } from "./parsers/pnpm-lock.js"; import { buildNpmWorkspaceMap } from "./parsers/package-lock.js"; import { buildBunWorkspaceMap } from "./parsers/bun-lock.js"; import { pluralize } from "./utils/string.js"; -import { shellQuote } from "./utils/shell-quote.js"; +import { npmLsCommand, UNSAFE_LS_INSTRUCTION } from "./utils/shell-quote.js"; import path from "node:path"; import { type DebugLogger } from "./output/debug.js"; import { resolveRegistryConfig } from "./utils/registry-config.js"; @@ -687,7 +687,9 @@ export async function scanPackages( updateAnalysisProgress("resolving remediation for", `${finding.pkg.name}@${finding.pkg.version}`); if (aliasRemediationFindings.has(finding)) { - const aliasNote = `Automatic parent remediation is unavailable for paths containing an npm alias. Run \`npm ls ${shellQuote(finding.pkg.name)}\` to inspect the declaring dependency, then rescan after updating it.`; + const lsCommand = npmLsCommand(finding.pkg.name); + const lookup = lsCommand ? `Run \`${lsCommand}\`` : UNSAFE_LS_INSTRUCTION; + const aliasNote = `Automatic parent remediation is unavailable for paths containing an npm alias. ${lookup} to inspect the declaring dependency, then rescan after updating it.`; finding.fixVersionValidationNote = [finding.fixVersionValidationNote, aliasNote].filter(Boolean).join(" "); continue; } diff --git a/src/utils/shell-quote.ts b/src/utils/shell-quote.ts index cccb94572..8eb06173c 100644 --- a/src/utils/shell-quote.ts +++ b/src/utils/shell-quote.ts @@ -6,8 +6,29 @@ export function shellQuote(token: string): string { return "'" + token.replace(/'/g, "'\\''") + "'"; } +// shellQuote only protects POSIX shells. cmd.exe ignores single quotes, so +// & | < > ^ % ! and double quotes still act there, and PowerShell reads the +// POSIX escape for a quote inside a token differently, as it does a backtick. +// Spaces are left alone: in cmd.exe they only split an argument, they cannot +// run anything. +const UNSAFE_IN_SOME_SHELL = /[&|<>^%!"'`\u0000-\u001f\u007f]/; + +/** True when a quoted token stays inert if pasted into any common shell, Windows included. */ +export function isSafeInEveryShell(token: string): boolean { + return !UNSAFE_IN_SOME_SHELL.test(token); +} + export function quoteShellPath(directory: string): string { // Preserve literal backslashes in POSIX names and keep ordinary Windows paths unquoted. const normalized = path.sep === "\\" ? directory.replace(/\\/g, "/") : directory; return shellQuote(normalized.startsWith("-") ? `./${normalized}` : normalized); } + +/** Shown in place of an `npm ls` command when the package name is not safe to paste into every shell. */ +export const UNSAFE_LS_INSTRUCTION = + "Look it up with your package manager's ls command (no command is shown because the name is not safe to paste into every shell)"; + +/** `npm ls ` for a pasteable inspection hint, or null when the name is not safe in every shell. */ +export function npmLsCommand(packageName: string): string | null { + return isSafeInEveryShell(packageName) ? `npm ls ${shellQuote(packageName)}` : null; +} diff --git a/tests/fix-commands-subfolder.test.ts b/tests/fix-commands-subfolder.test.ts index ee8cdaa34..9c74f05c6 100644 --- a/tests/fix-commands-subfolder.test.ts +++ b/tests/fix-commands-subfolder.test.ts @@ -3,7 +3,8 @@ import os from "node:os"; import path from "node:path"; import { spawnSync } from "node:child_process"; import { jest } from "@jest/globals"; -import { buildSuggestedFixCommandPlan } from "../src/remediation/fix-commands.js"; +import { buildSuggestedFixCommandPlan, UNSAFE_FOLDER_REASON } from "../src/remediation/fix-commands.js"; +import { isSafeInEveryShell } from "../src/utils/shell-quote.js"; import { buildOverrideFixCommand, overrideCommandTargetSuffix } from "../src/output/override-fix-command.js"; import type { Finding, ScanInput } from "../src/types.js"; @@ -97,11 +98,18 @@ describe("subfolder commands preserve literal paths", () => { ]) { fs.mkdirSync(path.join(root, subfolder), { recursive: true }); const plan = buildSuggestedFixCommandPlan([minimalFinding], minimalScanInput, { subfolder }); + if (!isSafeInEveryShell(subfolder)) { + // Not pasteable into cmd.exe or PowerShell: no command for this folder at all. + expect(plan?.command ?? null).toBeNull(); + expect(plan?.skipped.map(skip => skip.reason)).toContain(UNSAFE_FOLDER_REASON); + expect(buildOverrideFixCommand(overrideFix, subfolder)).toBeNull(); + continue; + } if (!plan?.command) throw new Error("Expected a generated command"); const commands = [ ...[plan.command, ...plan.sections.map(section => section.command)].map(command => ({ command, cwd: root })), - { command: buildOverrideFixCommand(overrideFix, subfolder), cwd: root }, - { command: buildOverrideFixCommand(overrideFix, `../${subfolder}`), cwd: caller }, + { command: buildOverrideFixCommand(overrideFix, subfolder)!, cwd: root }, + { command: buildOverrideFixCommand(overrideFix, `../${subfolder}`)!, cwd: caller }, { command: `cve-lite${overrideCommandTargetSuffix(subfolder)} overrides --fix`, cwd: root }, ]; for (const { command, cwd } of commands) { diff --git a/tests/shell-command-quoting.test.ts b/tests/shell-command-quoting.test.ts index 49b15ecb6..5286d487f 100644 --- a/tests/shell-command-quoting.test.ts +++ b/tests/shell-command-quoting.test.ts @@ -11,6 +11,7 @@ import { clearPackumentCache } from "../src/remediation/npm-registry.js"; import { buildSuggestedFixCommandPlan, findSuggestedCommandForFinding } from "../src/remediation/fix-commands.js"; import { resolveChainFix } from "../src/remediation/transitive-chain-resolver.js"; import { scanPackages } from "../src/scanner.js"; +import { isSafeInEveryShell, UNSAFE_LS_INSTRUCTION } from "../src/utils/shell-quote.js"; import { parseArgs } from "../src/cli/args.js"; import { NULL_AUDIT_LOG } from "../src/audit-log/index.js"; import type { OverrideContext, OverrideEntry } from "../src/overrides/context.js"; @@ -62,6 +63,9 @@ function makeOverrideContext( lockfilePackageNames: new Set(mode === "pd002" ? [pkgName] : []), installedVersions: new Map(), installedCopies: new Map(), + lockfileVersions: new Map(), + admittedVersions: new Map(), + admittedVulnerable: new Map(), parentDeclarations: new Map(), registryDistTags: new Map(), skippedDetectors: [], @@ -132,6 +136,23 @@ describe("copyable inspection and remediation commands prevent shell injection", const withFix = makeTransitiveFinding(name, "1.0.1"); const noFix = makeTransitiveFinding(name, null); + if (!isSafeInEveryShell(name)) { + // Not pasteable into cmd.exe or PowerShell: no command at all, a plain instruction instead. + const texts = [getRecommendedAction(withFix), getRecommendedAction(noFix), summarizeNextAction(withFix)]; + for (const text of texts) { + expect(text).toContain(UNSAFE_LS_INSTRUCTION); + expect(text).not.toContain("npm ls"); + } + for (const details of [ + detectPd001(makeOverrideContext(name, "npm", "pd001"))[0]!.details, + detectPd002(makeOverrideContext(name, "pnpm", "pd002"))[0]!.details, + ]) { + expect(details).toContain("no command is shown"); + expect(details).not.toContain("Run: "); + } + continue; + } + const recWithFix = extractBetween( getRecommendedAction(withFix), "the lockfile. Run `", @@ -162,8 +183,8 @@ describe("copyable inspection and remediation commands prevent shell injection", })); const htmlCmd = unescapeHtmlEntities(extractBetween(html, "Run ", " to find which package pulls it in.")); - const pd1Cmd = extractBetween(detectPd001(makeOverrideContext(name, "npm", "pd001"))[0]!.details, "Run: ", ""); - const pd2Cmd = extractBetween(detectPd002(makeOverrideContext(name, "pnpm", "pd002"))[0]!.details, "Run: ", ""); + const pd1Cmd = extractBetween(detectPd001(makeOverrideContext(name, "npm", "pd001"))[0]!.details ?? "", "Run: ", ""); + const pd2Cmd = extractBetween(detectPd002(makeOverrideContext(name, "pnpm", "pd002"))[0]!.details ?? "", "Run: ", ""); for (const command of [recWithFix, recNoFix, nextWithFix, htmlCmd, pd1Cmd, pd2Cmd]) { expect(runShellCommand(shell, tmp, command)).toBe(name); @@ -207,6 +228,11 @@ describe("copyable inspection and remediation commands prevent shell injection", const { findings } = await scanPackages([pkgRef], 100, options, { directDependencyNames: new Set(["parent"]) }, undefined, fetchMock); expect(findings).toHaveLength(1); const note = findings[0]!.fixVersionValidationNote ?? ""; + if (!isSafeInEveryShell(name)) { + expect(note).toContain(UNSAFE_LS_INSTRUCTION); + expect(note).not.toContain("npm ls"); + continue; + } const command = extractBetween(note, "Run `", "` to inspect the declaring dependency"); expect(runShellCommand(shell, tmp, command)).toBe(name); } diff --git a/tests/utils/shell-safe-commands.test.ts b/tests/utils/shell-safe-commands.test.ts new file mode 100644 index 000000000..bbdfd90aa --- /dev/null +++ b/tests/utils/shell-safe-commands.test.ts @@ -0,0 +1,100 @@ +import { buildSuggestedFixCommandPlan, findSuggestedCommandForFinding, UNSAFE_FOLDER_REASON, UNSAFE_PACKAGE_REASON } from "../../src/remediation/fix-commands.js"; +import { buildOverrideFixCommand } from "../../src/output/override-fix-command.js"; +import { isSafeInEveryShell, npmLsCommand } from "../../src/utils/shell-quote.js"; +import type { Finding, ScanInput } from "../../src/types.js"; + +const scanInput: ScanInput = { + mode: "resolved-lockfile", + source: "package-lock", + filePath: "/fake/project/package-lock.json", + packages: [], + notes: [], + warnings: [], + skippedDependencies: [], +}; + +function directFinding(name: string): Finding { + return { + pkg: { name, version: "1.0.0", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-test-direct" }], + severity: "high", + cveAliases: [], + dependencyPaths: [[name]], + relationship: "direct", + firstFixedVersion: "1.0.1", + validatedFirstFixedVersion: "1.0.1", + }; +} + +// A child refreshed through its parent: the plan files this target under the +// parent's name, which is what once let two skips collide. +const childFinding: Finding = { + pkg: { name: "form-data", version: "4.0.0", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-test-child" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["axios", "form-data"]], + relationship: "transitive", + firstFixedVersion: "4.0.6", + validatedFirstFixedVersion: "4.0.6", + recommendedNpmTransitiveRemediation: { + kind: "update-parent-within-range", + package: "axios", + currentVersion: "1.0.0", + targetChildVersion: "4.0.6", + viaPath: ["project", "axios", "form-data"], + reason: "axios already allows form-data@4.0.6", + }, +}; + +describe("isSafeInEveryShell", () => { + it.each(["lodash", "@scope/pkg", "two words", "pkg; touch marker #", "pkg$(touch marker)", "1.0.1+build.5", "literal\\backslash"])( + "accepts %j, which POSIX quoting keeps inert and cmd.exe and PowerShell leave alone", + token => expect(isSafeInEveryShell(token)).toBe(true), + ); + + it.each(["z & calc", "a|b", "ab", "a^b", "%PATH%", "a!b", "single'quote", 'double"quote', "a`b", "nl\nx", "tab\tx"])( + "rejects %j, which still acts in cmd.exe or PowerShell despite quoting", + token => expect(isSafeInEveryShell(token)).toBe(false), + ); + + it("gives an npm ls command only for names safe in every shell", () => { + expect(npmLsCommand("@scope/pkg")).toBe("npm ls @scope/pkg"); + expect(npmLsCommand("two words")).toBe("npm ls 'two words'"); + expect(npmLsCommand("z & calc")).toBeNull(); + }); +}); + +describe("fix plan with names unsafe in some shell", () => { + it("withholds the command for an unsafe package name and keeps the rest", () => { + const plan = buildSuggestedFixCommandPlan([directFinding("axios"), directFinding("z & calc")], scanInput); + expect(plan?.command).toBe("npm install axios@1.0.1"); + expect(plan?.command).not.toContain("calc"); + expect(plan?.skipped).toContainEqual(expect.objectContaining({ package: "z & calc", relationship: "direct", reason: UNSAFE_PACKAGE_REASON })); + expect(plan?.coveredFindingCount).toBe(1); + expect(findSuggestedCommandForFinding(plan!, directFinding("z & calc"))).toBeNull(); + }); + + it("withholds every command for an unsafe folder and names each affected package", () => { + const plan = buildSuggestedFixCommandPlan([directFinding("axios"), childFinding], scanInput, { subfolder: "R&D" }); + expect(plan?.command).toBeNull(); + expect(plan?.sections).toEqual([]); + expect(plan?.coveredFindingCount).toBe(0); + const skipped = plan?.skipped.map(skip => `${skip.package}@${skip.version} ${skip.relationship}: ${skip.reason}`); + expect(skipped).toEqual(expect.arrayContaining([ + `axios@1.0.0 direct: ${UNSAFE_FOLDER_REASON}`, + `form-data@4.0.0 transitive: ${UNSAFE_FOLDER_REASON}`, + ])); + }); + + it("keeps commands for a folder whose name only has a space", () => { + const plan = buildSuggestedFixCommandPlan([directFinding("axios")], scanInput, { subfolder: "web ui" }); + expect(plan?.command).toBe("cd 'web ui' && npm install axios@1.0.1"); + expect(plan?.skipped).toEqual([]); + }); + + it("gives no override fix command for an unsafe folder", () => { + expect(buildOverrideFixCommand("cve-lite overrides --fix", "R&D")).toBeNull(); + expect(buildOverrideFixCommand("cve-lite overrides --fix", "web ui")).toBe("cve-lite 'web ui' overrides --fix"); + }); +}); From 5ae6a605a64ac716eebfd05c3f88c03b5bf30ee0 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Thu, 12 Nov 2026 22:16:52 -0600 Subject: [PATCH 17/21] release: v1.41.3 --- package-lock.json | 4 ++-- package.json | 2 +- website/docusaurus.config.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index de893c7af..fdd82b281 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cve-lite-cli", - "version": "1.41.2", + "version": "1.41.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cve-lite-cli", - "version": "1.41.2", + "version": "1.41.3", "license": "MIT", "dependencies": { "better-sqlite3": "^12.8.0", diff --git a/package.json b/package.json index 3c64ea71f..2bf8b8349 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cve-lite-cli", - "version": "1.41.2", + "version": "1.41.3", "description": "Developer-friendly CLI for scanning JS/TS projects for dependency vulnerabilities using local lockfiles and OSV", "type": "module", "bin": { diff --git a/website/docusaurus.config.ts b/website/docusaurus.config.ts index 8fd30bace..832df76e8 100644 --- a/website/docusaurus.config.ts +++ b/website/docusaurus.config.ts @@ -2,7 +2,7 @@ import {themes as prismThemes} from 'prism-react-renderer'; import type {Config} from '@docusaurus/types'; import type * as Preset from '@docusaurus/preset-classic'; -const latestVersion = 'v1.41.2'; +const latestVersion = 'v1.41.3'; const config: Config = { title: 'CVE Lite CLI', From 12f068edf4155601c8f8d5245639426f03669afc Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Thu, 12 Nov 2026 22:16:52 -0600 Subject: [PATCH 18/21] docs: changelog for v1.41.3 --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5212e390..92c091454 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to CVE Lite CLI will be documented in this file. ## [Unreleased] +## [1.41.3] - 2026-11-13 + +> **Upgrade note.** This release contains one security fix and nothing else. + +### Security + +- A copyable command is no longer printed for a package, workspace or folder name that would still run something if pasted into Windows `cmd.exe` or PowerShell. The quoting added in 1.41.1 and 1.41.2 only protects POSIX shells such as bash, zsh and sh, so a name containing `&`, `|`, `<`, `>`, `^`, `%`, `!`, a quote, a backtick or a control character now gets no command in the fix plan, the `npm ls` hints, the PD001 and PD002 install hints or the override `--fix` command; the finding is listed as skipped with the reason, and the fix count drops to match. Ordinary names, including folder names with spaces, are unchanged (GHSA-x5r8-cxwg-fhm4). + ## [1.41.2] - 2026-11-12 > **Upgrade note.** This release contains two security fixes and nothing else. From b7c25674521f5030774ada78f489968df92aeb3f Mon Sep 17 00:00:00 2001 From: Savio Dsouza Date: Thu, 12 Nov 2026 21:37:14 +0530 Subject: [PATCH 19/21] fix(security): suppress option-shaped package names in fix commands (#1493) * Update shellQuote function for token quoting Modify shellQuote function to force quote tokens starting with '-' to prevent misinterpretation as options. * Export buildApplyCommands and add isOptionShaped function * Add tests for option-shaped package names in commands * Refactor command handling and shell quoting logic Refactor shellQuote function to simplify token validation and adjust command handling for option-shaped names. * Add suppressedCount to notAutoApplied object * Add suppressedCount to FixExecutionResult Added suppressedCount to track suppressed findings and updated related logic. * Implement test for npm alias option suppression Add test to suppress fix command for npm alias resembling an option * Fix import paths and update test cases Updated import paths to use .js extensions and fixed a typo in the test case. * Update src/utils/fix-runner.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Add check for option-shaped command package names * Refactor imports and export commandPackageName function * Refactor option handling in fix-runner.ts * Fix typo in test assertion from 'tBeNull' to 'toBeNull' * Update package count logic and summary output Refactor package count calculation to exclude suppressed targets and update summary message accordingly. * Check for suppressed targets in fix command plan * Remove isOptionShapedName function Removed isOptionShapedName function to simplify command handling. * Change condition check from isOptionShaped to isOptionShapedName * Implement isOptionShapedName function Add a function to check if a package name is option-shaped. * Enhance suggested fix command plan output logic * Update fix-commands.ts * Make suppressedCount optional and update checks * Add validation for target command flags Ensure target commands do not include registry or ignore-scripts flags. * Reorder imports in printers.ts for clarity * Refactor imports and clean up unused function Reorder imports and remove unused commandPackageName function. * Reorder imports in fix-runner.ts * Implement commandPackageName function Add commandPackageName function to determine package name based on command type. * Export formatFixCommandSummary function * Refactor command return values in fix-commands.ts Update command handling to return empty string or null based on conditions. * Fix formatting issues in fix-runner.ts * Add tests for command plan and summary formatting * Add newline at end of runner-option-injection test file Fix missing newline at end of runner-option-injection.test.ts * Change return type of buildSectionTitle and buildCommandForTargets * Update assertions for solo command in tests * Refactor fix application logic and update note message Updated condition to check runnable targets and modified note message for clarity. * Change test expectation for suppressed count Update the test to check for a specific note message instead of suppressed count. * Clean up runner-option-injection test formatting Remove unnecessary blank line in test case. * Refactor command plan checks in printers.ts Removed unnecessary checks for suppressed targets in command plan handling. * Refactor fix command logic for option-shaped names Removed references to commandPackageName for option-shaped names to prevent offering fix commands for them. * Replace suppressedCount with optionShapedNameCount * Refactor fix-runner to remove suppressedCount * Add optionShapedNameCount to test cases * Remove unused variable 'hasSuppressedTargets' Removed unused variable 'hasSuppressedTargets' from multiple locations in printers.ts. * Refactor command functions to return strings * Refactor noFixCount calculation in fix-runner.ts * Fix test for validateOptions with createPr option * Update severity levels in test cases * Rename test for npm alias option suppression * Remove test for option-shaped target omission Removed test case for omitting option-shaped targets from the displayed plan. * Update function return types to include null * Refactor upsertTarget to manage command-shaped names Refactor upsertTarget function to handle command-shaped package names and update related logic for skipped targets. * Add checks for option-shaped names in yarn commands * Add test for unused recommendation not skipping fix * Add test for yarn workspace command behavior * Add loop to process ordered findings * Clean up test case by removing blank line Remove unnecessary blank line in test case for npm alias. * Refactor buildCommandForTargets function Remove early return for empty targets array. * Add tests for option-shaped skips in summary * Update axios version and enhance npm alias tests Updated targetChildVersion for axios and adjusted expectations in tests to reflect the new version. Added tests to skip npm operations when names resemble command-line options. * Remove yarn workspace command tests for invalid names Removed test cases that check for yarn workspace command generation with names starting with '-' * Simplify yarn handling in fix-runner.ts Remove unnecessary checks for option-shaped names in yarn package manager handling. * Update notAutoApplied structure in test cases Added 'optionShapedNameCount' to notAutoApplied object in multiple test cases. * Fix test case for option-shaped skip counting * Update tests/fix-runner-summary.test.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: make optionShapedNameCount optional so fixtures match main The field is only set by categorizeUnappliedTargets, so readers treat an absent value as 0. That lets create-pr.test.ts and fix-result-json.test.ts stay exactly as they are on main, keeping this PR to the option-shaped name guard. * test: cover single-dash option-shaped names such as -rf Every existing case used a double-dash name, so a check narrowed to startsWith("--") would still pass. Adds -rf at the planner and argv levels so that mutation fails. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- src/output/printers.ts | 3 +- src/remediation/fix-commands.ts | 28 +++- src/scan/single-scan.ts | 1 + src/utils/fix-runner.ts | 41 +++-- src/utils/package-identity.ts | 16 ++ tests/fix-runner-summary.test.ts | 37 ++++- tests/remediation/npm-alias-fixes.test.ts | 160 ++++++++++++++++++++ tests/utils/runner-option-injection.test.ts | 41 +++++ 8 files changed, 303 insertions(+), 24 deletions(-) create mode 100644 tests/utils/runner-option-injection.test.ts diff --git a/src/output/printers.ts b/src/output/printers.ts index f1e604122..37128ff33 100644 --- a/src/output/printers.ts +++ b/src/output/printers.ts @@ -1158,7 +1158,7 @@ function colorFixSectionTitle( return chalk.gray(title); } -function formatFixCommandSummary( +export function formatFixCommandSummary( plan: ReturnType, ) { if (!plan) return ""; @@ -1178,7 +1178,6 @@ function formatFixCommandSummary( const packageLabel = packageCount === 1 ? "package" : "packages"; const sectionLabel = sectionCount === 1 ? "group" : "groups"; - return `${sectionCount} command ${sectionLabel} ready across ${packageCount} ${packageLabel}${severitySummary ? ` (${severitySummary})` : ""}.`; } diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index a348abc48..bf20e857f 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -13,8 +13,7 @@ import { buildYarnWorkspaceMap } from "../parsers/yarn-lock.js"; import { buildBunWorkspaceMap } from "../parsers/bun-lock.js"; import { readConfiguredCooldown, cooldownWarningFor, type CooldownWarning } from "./cooldown.js"; import { isVerifiedUpgrade } from "./parent-upgrade.js"; -import { installPackageSpec, packageInstanceKey, packageNameKey, registryPackageName } from "../utils/package-identity.js"; - +import { commandPackageName, installPackageSpec, isOptionShapedName, packageInstanceKey, packageNameKey, registryPackageName } from "../utils/package-identity.js"; // Rendered next to a parent-upgrade fix target whose recommendedParentUpgrade // could not be verified end-to-end (parent-upgrade.ts confidence: "unverified"). // Shared across terminal and HTML output so the wording stays in sync. @@ -77,6 +76,7 @@ export type SuggestedFixSkip = { * matching the reason text. */ advisoryHintUnavailable?: boolean; + optionShapedName?: boolean; }; export type SuggestedFixCommandPlan = { @@ -206,7 +206,7 @@ export function buildSuggestedFixCommandPlan( validatedFieldOk ) { const pkgWorkspaces = workspaceMap.get(packageNameKey(finding.pkg))?.filter(w => w !== ".") ?? []; - upsertTarget(targetsByKey, finding, { + upsertTarget(targetsByKey, skippedByKey, finding, { package: finding.pkg.name, ...(finding.pkg.registryName ? { registryName: finding.pkg.registryName } : {}), currentVersion: finding.pkg.version, @@ -277,7 +277,7 @@ export function buildSuggestedFixCommandPlan( finding.relationship === "transitive" && finding.recommendedNpmTransitiveRemediation?.kind === "update-parent-within-range" ) { - upsertTarget(targetsByKey, finding, parentUpdateTarget(finding, urgent, { + upsertTarget(targetsByKey, skippedByKey, finding, parentUpdateTarget(finding, urgent, { package: finding.recommendedNpmTransitiveRemediation.package, currentVersion: finding.recommendedNpmTransitiveRemediation.currentVersion, childTargetVersion: finding.recommendedNpmTransitiveRemediation.targetChildVersion, @@ -324,7 +324,7 @@ export function buildSuggestedFixCommandPlan( ? finding.chainResolution : null; const npmParentUpgradeWorkspaces = workspaceMap.get(finding.recommendedNpmTransitiveRemediation.package)?.filter(w => w !== ".") ?? []; - upsertTarget(targetsByKey, finding, { + upsertTarget(targetsByKey, skippedByKey, finding, { package: finding.recommendedNpmTransitiveRemediation.package, currentVersion: finding.recommendedNpmTransitiveRemediation.currentVersion, targetVersion: finding.recommendedNpmTransitiveRemediation.targetVersion, @@ -365,7 +365,7 @@ export function buildSuggestedFixCommandPlan( ? finding.chainResolution : null; const parentUpgradeWorkspaces = workspaceMap.get(finding.recommendedParentUpgrade.package)?.filter(w => w !== ".") ?? []; - upsertTarget(targetsByKey, finding, { + upsertTarget(targetsByKey, skippedByKey, finding, { package: finding.recommendedParentUpgrade.package, currentVersion: finding.recommendedParentUpgrade.currentVersion, targetVersion: finding.recommendedParentUpgrade.targetVersion, @@ -412,7 +412,7 @@ export function buildSuggestedFixCommandPlan( : packageManager === "yarn" ? buildYarnUpgradeCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces) : buildBunUpdateCommand(finding.pkg.name); - upsertTarget(targetsByKey, finding, parentUpdateTarget(finding, urgent, { + upsertTarget(targetsByKey, skippedByKey, finding, parentUpdateTarget(finding, urgent, { package: finding.pkg.name, currentVersion: finding.pkg.version, childTargetVersion: finding.recommendedNpmTransitiveRemediation.targetChildVersion, @@ -429,7 +429,7 @@ export function buildSuggestedFixCommandPlan( const isDev = devLookup.get(`${finding.chainResolution.directDep}@${finding.chainResolution.directDepCurrentVersion}`) ?? false; const coverage = calculatePathCoverage(finding.dependencyPaths, [finding.chainResolution.directDep]); const chainWorkspaces = workspaceMap.get(finding.chainResolution.directDep)?.filter(w => w !== ".") ?? []; - upsertTarget(targetsByKey, finding, { + upsertTarget(targetsByKey, skippedByKey, finding, { package: finding.chainResolution.directDep, currentVersion: finding.chainResolution.directDepCurrentVersion, targetVersion: finding.chainResolution.targetVersion, @@ -880,9 +880,21 @@ function withholdShellUnsafeTargets( function upsertTarget( targetsByKey: Map, + skippedByKey: Map, finding: Finding, next: SuggestedFixTarget, ): void { + const operand = commandPackageName(next); + if (isOptionShapedName(operand)) { + skippedByKey.set(`option-shaped:${packageInstanceKey(finding.pkg)}`, { + package: finding.pkg.name, + version: finding.pkg.version, + relationship: finding.relationship, + reason: `Package name "${operand}" looks like a command-line option, so no fix command is offered.`, + optionShapedName: true, + }); + return; + } const key = targetKey(finding, next); const existing = targetsByKey.get(key); if (!existing) { diff --git a/src/scan/single-scan.ts b/src/scan/single-scan.ts index 1d59415aa..d9e8a0651 100644 --- a/src/scan/single-scan.ts +++ b/src/scan/single-scan.ts @@ -405,6 +405,7 @@ export async function handleSingleFolderScan(params: SingleFolderScanParams): Pr parentUpgradeCount: fixResult?.parentUpgradeCount ?? 0, breakingUpgradeCount: fixResult?.breakingUpgradeCount ?? 0, noFixCount: fixResult?.noFixCount ?? 0, + optionShapedNameCount: fixResult?.optionShapedNameCount ?? 0, }, }; writeFixResultJson(fixResultJson, projectPath); diff --git a/src/utils/fix-runner.ts b/src/utils/fix-runner.ts index de2fc2351..97f5d90a8 100644 --- a/src/utils/fix-runner.ts +++ b/src/utils/fix-runner.ts @@ -7,7 +7,7 @@ import type { ParsedOptions } from "../types.js"; import type { SeverityLabel } from "../types.js"; import { pluralize } from "./string.js"; import { isBreakingUpgrade } from "./version.js"; -import { installPackageSpec } from "./package-identity.js"; +import { installPackageSpec, isOptionShapedName } from "./package-identity.js"; import { formatParentUpdateChildNote } from "../remediation/parent-update-child-note.js"; import type { SuggestedFixCommandPlan, SuggestedFixTarget } from "../remediation/fix-commands.js"; import type { DebugLogger } from "../output/debug.js"; @@ -55,6 +55,10 @@ export type UnappliedBreakdown = { breakingUpgradeCount: number; // Findings with no available fix (skip records). noFixCount: number; + // Findings whose package name looks like a command-line option, so no fix cmd + // is offered for them. Optional so fixtures written before the field existed + // keep compiling; every reader treats an absent value as 0. + optionShapedNameCount?: number; }; export type FixExecutionResult = { @@ -91,7 +95,8 @@ export function categorizeUnappliedTargets(plan: SuggestedFixCommandPlan): Unapp return { parentUpgradeCount, breakingUpgradeCount, - noFixCount: plan.skipped.length, + noFixCount: plan.skipped.filter(skip => !skip.optionShapedName).length, + optionShapedNameCount: plan.skipped.filter(skip => skip.optionShapedName).length, }; } @@ -99,13 +104,14 @@ const EMPTY_UNAPPLIED: UnappliedBreakdown = { parentUpgradeCount: 0, breakingUpgradeCount: 0, noFixCount: 0, + optionShapedNameCount: 0, }; // Build argv lists for --fix without round-tripping through a shell string. // Workspace paths and package tokens are passed as whole argv elements to spawn // (no shell), so a directory name containing `&&` cannot become a second executable. // Human-facing plan strings still come from buildTargetCommand. -function buildApplyCommands( +export function buildApplyCommands( packageManager: SuggestedFixCommandPlan["packageManager"], target: SuggestedFixTarget, ): string[][] { @@ -121,6 +127,7 @@ function buildInstallApplyArgv( ): string[][] { const workspaces = (target.workspaces ?? []).filter(w => w !== "."); const pkgArg = installPackageSpec(target); + if (isOptionShapedName(pkgArg)) return []; const isDev = Boolean(target.isDev); if (packageManager === "npm") { @@ -176,6 +183,7 @@ function buildParentUpdateApplyArgv( // Always build from structured fields. Never tokenize target.command: plan // strings may be shell-quoted for display, and must not be re-parsed into argv. const updatePkg = target.childPackage ?? target.package; + if (isOptionShapedName(updatePkg)) return []; const workspaces = target.workspaces ?? []; const nonRoot = workspaces.filter(w => w !== "."); @@ -263,9 +271,12 @@ export async function applyFixesIfRequested(params: { const applyTargets = params.plan.targets.filter( target => target.kind === "direct" || target.kind === "parent-update", ); + const packageManager = params.plan.packageManager; + const runnableTargets = applyTargets.filter( + target => buildApplyCommands(packageManager, target).length > 0, + ); const unapplied = categorizeUnappliedTargets(params.plan); - - if (applyTargets.length === 0) { + if (runnableTargets.length === 0) { return { appliedFixCount: 0, appliedWithinRangeRefreshCount: 0, @@ -276,7 +287,7 @@ export async function applyFixesIfRequested(params: { } const commandLabel = commandLabelForPackageManager(params.plan.packageManager); - const total = applyTargets.length; + const total = runnableTargets.length; const spinner = createSpinner( `Applying safe package fixes with ${commandLabel}... 0/${total}`, params.options, @@ -287,8 +298,8 @@ export async function applyFixesIfRequested(params: { // parents that refresh the same child can still share `npm update `. // Spawn each command once; the displayed plan already dedupes the same way. const seenCommands = new Set(); - for (let i = 0; i < applyTargets.length; i++) { - const target = applyTargets[i]; + for (let i = 0; i < runnableTargets.length; i++) { + const target = runnableTargets[i]; for (const commandParts of buildApplyCommands(params.plan.packageManager, target)) { const commandLine = commandParts.join(" "); if (seenCommands.has(commandLine)) continue; @@ -310,13 +321,13 @@ export async function applyFixesIfRequested(params: { } } } - spinner.succeed(`Applied ${applyTargets.length} package ${pluralize(applyTargets.length, "fix", "fixes")} with ${commandLabel}`); + spinner.succeed(`Applied ${runnableTargets.length} package ${pluralize(runnableTargets.length, "fix", "fixes")} with ${commandLabel}`); return { - appliedFixCount: applyTargets.length, - appliedWithinRangeRefreshCount: applyTargets.filter(target => target.kind === "parent-update").length, + appliedFixCount: runnableTargets.length, + appliedWithinRangeRefreshCount: runnableTargets.filter(target => target.kind === "parent-update").length, ...unapplied, - applied: applyTargets.map(target => ({ + applied: runnableTargets.map(target => ({ package: target.package, from: target.currentVersion ?? "unknown", to: target.targetVersion, @@ -356,7 +367,7 @@ export function printFixModeSummary(params: { : ""; console.log(`- Applied fixes: ${chalk.green(String(result.appliedFixCount))}${chalk.gray(refreshNote)}`); - const notAutoApplied = result.parentUpgradeCount + result.noFixCount; + const notAutoApplied = result.parentUpgradeCount + result.noFixCount + (result.optionShapedNameCount ?? 0); console.log(`- Not auto-applied: ${chalk.yellow(String(notAutoApplied))}`); if (result.parentUpgradeCount > 0) { const breakingNote = result.breakingUpgradeCount > 0 @@ -367,6 +378,10 @@ export function printFixModeSummary(params: { if (result.noFixCount > 0) { console.log(` - No fix available: ${chalk.yellow(String(result.noFixCount))}`); } + const optionShapedNameCount = result.optionShapedNameCount ?? 0; + if (optionShapedNameCount > 0) { + console.log(` - Package name looks like an option (no command offered): ${chalk.yellow(String(optionShapedNameCount))}`); + } console.log(`- Findings before fix: ${chalk.white(String(params.findingsBeforeFix))}`); console.log(`- Remaining findings after fix: ${chalk.white(String(params.findingsAfterFix))}`); console.log( diff --git a/src/utils/package-identity.ts b/src/utils/package-identity.ts index d67e6a6b8..a4edeebf4 100644 --- a/src/utils/package-identity.ts +++ b/src/utils/package-identity.ts @@ -35,6 +35,22 @@ export function installPackageSpec(target: { package: string; targetVersion: str return `${target.package}@${version}`; } +/** + * A package name beginning with "-" would be read as an option by the package + * manager (npm never allows such a name), so fix commands that would name one + * are suppressed rather than printed or run. + */ +export function isOptionShapedName(name: string): boolean { + return name.startsWith("-"); +} + +/** +* The package name a fix command would name. For a parent-update the command +* names the child being refreshed; otherwise it names the package itself. +*/ +export function commandPackageName(target: { kind: string; package: string; childPackage?: string }): string { + return target.kind === "parent-update" ? (target.childPackage ?? target.package) : target.package; +} /** npm lockfile v1 stores an alias's resolved name and version in `version`. */ export function parseNpmAliasVersion(value: unknown): { registryName: string; version: string } | null { if (typeof value !== "string" || !value.startsWith(NPM_ALIAS_PROTOCOL)) { diff --git a/tests/fix-runner-summary.test.ts b/tests/fix-runner-summary.test.ts index a0a17966c..c2bf67aaf 100644 --- a/tests/fix-runner-summary.test.ts +++ b/tests/fix-runner-summary.test.ts @@ -62,6 +62,20 @@ describe("categorizeUnappliedTargets", () => { expect(result.breakingUpgradeCount).toBe(1); }); + it("counts an option-shaped skip once, separately from ordinary no-fix skips", () => { + const mixed: SuggestedFixCommandPlan = { + ...plan([], 0), + skipped: [ + { package: "no-fix", version: "1.0.0", relationship: "transitive" as const, reason: "no fix" }, + { package: "--registry=http://127.0.0.1:9/", version: "1.0.0", relationship: "direct" as const, reason: "looks like an option", optionShapedName: true }, + ], + }; + const result = categorizeUnappliedTargets(mixed); + + expect(result.noFixCount).toBe(1); + expect(result.optionShapedNameCount).toBe(1); + }); + it("counts skip records as no-fix", () => { const result = categorizeUnappliedTargets(plan([ target({ kind: "direct", package: "applied-direct" }), @@ -75,7 +89,7 @@ describe("categorizeUnappliedTargets", () => { }); describe("printFixModeSummary", () => { - const noSeverity: Record = { critical: 0, high: 0, medium: 0, low: 0, unknown: 0 }; + const noSeverity: Record = { none: 0, critical: 0, high: 0, medium: 0, low: 0, unknown: 0 }; function capture(result: FixExecutionResult, before: number, after: number): string { const spy = jest.spyOn(console, "log").mockImplementation(() => {}); @@ -87,6 +101,23 @@ describe("printFixModeSummary", () => { } } + it("adds the option-shaped count to the total and lists it on its own line", () => { + const out = capture({ + appliedFixCount: 0, + appliedWithinRangeRefreshCount: 0, + applied: [], + note: null, + parentUpgradeCount: 0, + breakingUpgradeCount: 0, + noFixCount: 1, + optionShapedNameCount: 1, + }, 2, 2); + + expect(out).toContain("Not auto-applied: 2"); + expect(out).toContain("No fix available: 1"); + expect(out).toContain("Package name looks like an option (no command offered): 1"); + }); + it("renders an honest categorized breakdown with no 'v1 skip' jargon", () => { const out = capture({ appliedFixCount: 0, @@ -96,6 +127,7 @@ describe("printFixModeSummary", () => { parentUpgradeCount: 27, breakingUpgradeCount: 27, noFixCount: 1, + optionShapedNameCount: 0, }, 45, 45); expect(out).toContain("Not auto-applied: 28"); @@ -115,6 +147,7 @@ describe("printFixModeSummary", () => { parentUpgradeCount: 0, breakingUpgradeCount: 0, noFixCount: 0, + optionShapedNameCount: 0, }, 5, 1); expect(out).toContain("Applied fixes: 4"); @@ -133,6 +166,7 @@ describe("printFixModeSummary", () => { parentUpgradeCount: 0, breakingUpgradeCount: 0, noFixCount: 0, + optionShapedNameCount: 0, }, 2, 0); expect(out).toContain("refresh form-data to 4.0.6"); @@ -152,6 +186,7 @@ describe("printFixModeSummary", () => { parentUpgradeCount: 2, breakingUpgradeCount: 0, noFixCount: 0, + optionShapedNameCount: 0, }, 3, 2); expect(out).toContain("Parent upgrades (review + test): 2"); diff --git a/tests/remediation/npm-alias-fixes.test.ts b/tests/remediation/npm-alias-fixes.test.ts index b26f25c8b..b8df2be89 100644 --- a/tests/remediation/npm-alias-fixes.test.ts +++ b/tests/remediation/npm-alias-fixes.test.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import { buildSuggestedFixCommandPlan, findSuggestedCommandForFinding } from "../../src/remediation/fix-commands.js"; import { applyFixesIfRequested } from "../../src/utils/fix-runner.js"; +import { formatFixCommandSummary } from "../../src/output/printers.js"; import type { Finding, ScanInput } from "../../src/types.js"; import { removeDir } from "../test-utils.js"; import { parseArgs } from "../../src/cli/args.js"; @@ -77,4 +78,163 @@ describe("npm alias fix plans and execution (#1449)", () => { ["npm", "install", "-w", "packages/second", "client@npm:@scope/two@1.0.1"], ])); }); + it("skips an npm alias whose name looks like an option instead of offering a command", async () => { + const f = finding("--registry=http://127.0.0.1:9/", "axios"); + const plan = buildSuggestedFixCommandPlan([f], scanInput([f]), { offline: true }); + expect(plan).not.toBeNull(); + expect(plan!.targets).toHaveLength(0); + expect(plan!.skipped).toHaveLength(1); + expect(plan!.skipped[0].package).toBe("--registry=http://127.0.0.1:9/"); + expect(plan!.skipped[0].optionShapedName).toBe(true); + expect(plan!.skipped[0].reason).toMatch(/looks like a command-line option/); + expect(plan!.command).toBeNull(); + expect(findSuggestedCommandForFinding(plan!, f)).toBeNull(); + + const calls: string[][] = []; + const result = await applyFixesIfRequested({ + plan, projectPath: dir, totalFindings: 1, options: FIX_OPTIONS, + runCommand: async (command, argv) => { + calls.push([command, ...argv]); + return { status: 0, error: null }; + }, + }); + expect(calls).toEqual([]); + expect(result.appliedFixCount).toBe(0); + + expect(result.optionShapedNameCount).toBe(1); + expect(result.note).toMatch(/No fixes were auto-applied/); + }); + + it("skips a finding whose recommended parent upgrade is option-shaped", () => { + const f = finding("lodash", "lodash"); + f.relationship = "transitive"; + f.recommendedParentUpgrade = { + package: "--registry=http://127.0.0.1:9/", + currentVersion: "1.0.0", + targetVersion: "1.0.1", + viaPath: ["project", "--registry=http://127.0.0.1:9/", "lodash"], + vulnerablePackage: "lodash", + confidence: "verified", + reason: "parent upgrade", + }; + const plan = buildSuggestedFixCommandPlan([f], scanInput([f]), { offline: true }); + expect(plan).not.toBeNull(); + expect(plan!.targets).toHaveLength(0); + expect(plan!.skipped[0].optionShapedName).toBe(true); + expect(plan!.skipped[0].reason).toMatch(/looks like a command-line option/); + }); + + it("does not skip a fix when only an unused recommendation is option-shaped", () => { + const f = finding("lodash", "lodash"); + f.relationship = "transitive"; + f.recommendedNpmTransitiveRemediation = { + kind: "update-parent-within-range", + package: "axios", + currentVersion: "0.21.1", + viaPath: ["project", "axios", "lodash"], + reason: "parent always permits a safe child", + targetChildVersion: "1.0.1", + }; + f.recommendedParentUpgrade = { + package: "--registry=http://127.0.0.1:9/", + currentVersion: "1.0.0", + targetVersion: "1.0.1", + viaPath: ["project", "--registry=http://127.0.0.1:9/", "lodash"], + vulnerablePackage: "lodash", + confidence: "verified", + reason: "parent upgrade", + }; + const plan = buildSuggestedFixCommandPlan([f], scanInput([f]), { offline: true }); + expect(plan).not.toBeNull(); + expect(plan!.skipped.filter(skip => skip.optionShapedName)).toHaveLength(0); + expect(plan!.targets).toHaveLength(1); + expect(plan!.targets[0].kind).toBe("parent-update"); + expect(plan!.targets[0].package).toBe("axios"); + expect(plan!.targets[0].childPackage).toBe("lodash"); + expect(plan!.targets[0].childTargetVersion).toBe("1.0.1"); + }); + + it("skips an npm within-range refresh when the child name looks like an option", () => { + const f = finding("--registry=http://127.0.0.1:9/", "axios"); + f.relationship = "transitive"; + f.recommendedNpmTransitiveRemediation = { + kind: "update-parent-within-range", + package: "axios", + currentVersion: "0.21.1", + viaPath: ["project", "axios", "--registry=http://127.0.0.1:9/"], + reason: "parent already permits a safe child", + targetChildVersion: "0.33.0", + }; + const plan = buildSuggestedFixCommandPlan([f], scanInput([f]), { offline: true }); + expect(plan).not.toBeNull(); + // The command names the child, so an option-shaped child gets no command. + expect(plan!.targets).toHaveLength(0); + expect(plan!.skipped[0].optionShapedName).toBe(true); + expect(plan!.skipped[0].reason).toMatch(/looks like a command-line option/); + }); + + it("skips a chain resolution when the direct dependency looks like an option", () => { + const f = finding("lodash", "lodash"); + f.relationship = "transitive"; + f.dependencyPaths = [["project", "--registry=http://127.0.0.1:9/", "lodash"]]; + f.chainResolution = { + directDep: "--registry=http://127.0.0.1:9/", + directDepCurrentVersion: "1.0.0", + targetVersion: "1.0.1", + chain: [], + safeVersion: "1.0.1", + command: "npm install --registry=http://127.0.0.1:9/@1.0.1", + coveredPaths: 1, + totalPaths: 1, + }; + const plan = buildSuggestedFixCommandPlan([f], scanInput([f]), { offline: true }); + expect(plan).not.toBeNull(); + // The target carries an explicit command; the check must still catch it. + expect(plan!.targets).toHaveLength(0); + expect(plan!.skipped[0].optionShapedName).toBe(true); + }); + + it("skips an npm parent upgrade when the parent name looks like an option", () => { + const f = finding("lodash", "lodash"); + f.relationship = "transitive"; + f.recommendedNpmTransitiveRemediation = { + kind: "upgrade-parent-to-version", + package: "--registry=http://127.0.0.1:9/", + currentVersion: "1.0.0", + targetVersion: "2.0.0", + viaPath: ["project", "--registry=http://127.0.0.1:9/", "lodash"], + reason: "upgrade the parent", + targetChildVersion: "1.0.1", + }; + const plan = buildSuggestedFixCommandPlan([f], scanInput([f]), { offline: true }); + expect(plan).not.toBeNull(); + expect(plan!.targets).toHaveLength(0); + expect(plan!.skipped[0].optionShapedName).toBe(true); + }); + + it("skips a single-dash alias name such as -rf", () => { + const f = finding("-rf", "axios"); + const plan = buildSuggestedFixCommandPlan([f], scanInput([f]), { offline: true }); + expect(plan).not.toBeNull(); + expect(plan!.targets).toHaveLength(0); + expect(plan!.skipped[0].optionShapedName).toBe(true); + expect(plan!.skipped[0].reason).toMatch(/looks like a command-line option/); + }); + + it("never prints an option-shaped name in the plan command, its sections, or the summary count", () => { + const evil = finding("--registry=http://127.0.0.1:9/", "axios"); + const normal = finding("lodash", "lodash"); + + const solo = buildSuggestedFixCommandPlan([evil], scanInput([evil]), { offline: true }); + expect(solo).not.toBeNull(); + expect(solo!.command).toBeNull(); + expect(solo!.sections).toHaveLength(0); + expect(formatFixCommandSummary(solo!)).toMatch(/^0 command groups ready across 0 packages/); + + const mixed = buildSuggestedFixCommandPlan([evil, normal], scanInput([evil, normal]), { offline: true }); + expect(mixed).not.toBeNull(); + expect(mixed!.command ?? "").not.toContain("--registry"); + expect(JSON.stringify(mixed!.sections)).not.toContain("--registry"); + expect(formatFixCommandSummary(mixed!)).toMatch(/^1 command group ready across 1 package/); + }); }); diff --git a/tests/utils/runner-option-injection.test.ts b/tests/utils/runner-option-injection.test.ts new file mode 100644 index 000000000..66a1219aa --- /dev/null +++ b/tests/utils/runner-option-injection.test.ts @@ -0,0 +1,41 @@ +import { buildApplyCommands } from "../../src/utils/fix-runner.js"; +import { buildTargetCommand, type SuggestedFixTarget } from "../../src/remediation/fix-commands.js"; + +function target(overrides: Partial): SuggestedFixTarget { + return { + kind: "direct", + package: "lodash", + targetVersion: "4.17.21", + ...overrides, + } as SuggestedFixTarget; +} + +// A package name taken from an untrusted lockfile must never be read as an option +// by the package manager (CWE-88, argument injection). +describe("option-shaped package names in fix commands", () => { + it("does not build an install command when the package name starts with '-'", () => { + expect(buildApplyCommands("npm", target({ package: "--ignore-scripts=false" }))).toEqual([]); + expect(buildApplyCommands("pnpm", target({ package: "--registry=https://attacker.example/" }))).toEqual([]); + expect(buildApplyCommands("yarn", target({ package: "--ignore-scripts=false" }))).toEqual([]); + expect(buildApplyCommands("bun", target({ package: "--ignore-scripts=false" }))).toEqual([]); + }); + + it("does not build an install command for a single-dash name such as -rf", () => { + expect(buildApplyCommands("npm", target({ package: "-rf" }))).toEqual([]); + expect(buildApplyCommands("pnpm", target({ package: "-rf" }))).toEqual([]); + expect(buildApplyCommands("yarn", target({ package: "-rf" }))).toEqual([]); + expect(buildApplyCommands("bun", target({ package: "-rf" }))).toEqual([]); + }); + + it("does not build a parent-update command when the child name starts with '-'", () => { + const t = target({ kind: "parent-update", package: "axios", childPackage: "--ignore-scripts=false" }); + expect(buildApplyCommands("npm", t)).toEqual([]); + }); + + it("still builds a normal install command for a well-formed name", () => { + expect(buildApplyCommands("npm", target({ package: "lodash", targetVersion: "4.17.21" }))).toEqual([ + ["npm", "install", "lodash@4.17.21"], + ]); + expect(buildTargetCommand(target({ package: "lodash" }), "npm")).toBe("npm install lodash@4.17.21"); + }); +}); From 95e7e20d407635ad0f754b3664d779689c398b07 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Fri, 13 Nov 2026 10:36:16 -0600 Subject: [PATCH 20/21] release: v1.41.4 --- package-lock.json | 4 ++-- package.json | 2 +- website/docusaurus.config.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index fdd82b281..64534de88 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cve-lite-cli", - "version": "1.41.3", + "version": "1.41.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cve-lite-cli", - "version": "1.41.3", + "version": "1.41.4", "license": "MIT", "dependencies": { "better-sqlite3": "^12.8.0", diff --git a/package.json b/package.json index 2bf8b8349..4e8ab0daf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cve-lite-cli", - "version": "1.41.3", + "version": "1.41.4", "description": "Developer-friendly CLI for scanning JS/TS projects for dependency vulnerabilities using local lockfiles and OSV", "type": "module", "bin": { diff --git a/website/docusaurus.config.ts b/website/docusaurus.config.ts index 832df76e8..c52152f15 100644 --- a/website/docusaurus.config.ts +++ b/website/docusaurus.config.ts @@ -2,7 +2,7 @@ import {themes as prismThemes} from 'prism-react-renderer'; import type {Config} from '@docusaurus/types'; import type * as Preset from '@docusaurus/preset-classic'; -const latestVersion = 'v1.41.3'; +const latestVersion = 'v1.41.4'; const config: Config = { title: 'CVE Lite CLI', From 1d0c9ff18a744756516b3cdebc94ad6e55bd4020 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Fri, 13 Nov 2026 10:36:16 -0600 Subject: [PATCH 21/21] docs: changelog for v1.41.4 --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92c091454..0dc6c0657 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to CVE Lite CLI will be documented in this file. ## [Unreleased] +## [1.41.4] - 2026-11-13 + +> **Upgrade note.** This release contains one security fix and nothing else. + +### Security + +- A package name that looks like a command-line option, such as an npm alias named `--registry=https://...` or `--ignore-scripts=false`, is no longer put into a printed fix command or run by `--fix`. Since 1.41.0 such an alias is scanned under the real package it points at, so it could produce a finding whose fix command the package manager read as a setting, for example pointing `npm install` at another registry. The finding is now listed as skipped with the reason (GHSA-5583-5qpr-q69x). Reported and fixed by @S3DFX-CYBER. + ## [1.41.3] - 2026-11-13 > **Upgrade note.** This release contains one security fix and nothing else.