From 80c0ada654cb2837d52e682172349e1bc1c64677 Mon Sep 17 00:00:00 2001 From: vian Date: Fri, 6 Nov 2026 00:55:04 +0530 Subject: [PATCH 01/14] fix(yarn): resolve workspace globs with shared discovery --- src/parsers/yarn-lock.ts | 37 +++++-------------- src/scanner.ts | 12 ++++-- src/utils/package-json.ts | 3 +- tests/parsers/yarn-workspace-globs.test.ts | 43 ++++++++++++++++++++++ tests/scanner-cache.test.ts | 40 ++++++++++++++++++++ 5 files changed, 102 insertions(+), 33 deletions(-) create mode 100644 tests/parsers/yarn-workspace-globs.test.ts diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index a458f88d2..f24d39667 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -4,6 +4,7 @@ import { parse as parseYarnLock } from "yarn-lockfile"; import type { PackageRef } from "../types.js"; import { looksLikeVersion, normalizeRawVersion } from "../utils/version.js"; import { upsertPackage, MAX_PATHS_PER_PACKAGE } from "./utils.js"; +import { resolveWorkspacePackageJsonPaths } from "../utils/package-json.js"; const MAX_PATH_DEPTH = 10; @@ -119,35 +120,15 @@ function readWorkspacePackages(filePath: string, rootPkg?: any): { name: string; if (!patterns.length) return []; const result: { name: string; pkg: any }[] = []; - for (const pattern of patterns) { - const parts = pattern.split("/"); - const lastPart = parts[parts.length - 1]; - const baseParts = parts.slice(0, -1); - - let wsDirs: string[]; - if (lastPart === "*") { - const baseDir = path.join(dir, ...baseParts); - try { - wsDirs = fs.readdirSync(baseDir, { withFileTypes: true }) - .filter(d => d.isDirectory()) - .map(d => [...baseParts, d.name].join("/")); - } catch { - continue; - } - } else { - wsDirs = [pattern]; - } - - for (const wsDir of wsDirs) { - let wsPkg: any; - try { - wsPkg = JSON.parse(fs.readFileSync(path.join(dir, wsDir, "package.json"), "utf8")); - } catch { - continue; - } - if (!wsPkg.name) continue; - result.push({ name: wsPkg.name, pkg: wsPkg }); + for (const packageJsonPath of resolveWorkspacePackageJsonPaths(dir, patterns)) { + let wsPkg: any; + try { + wsPkg = JSON.parse(fs.readFileSync(packageJsonPath, "utf8")); + } catch { + continue; } + if (!wsPkg.name) continue; + result.push({ name: wsPkg.name, pkg: wsPkg }); } return result; diff --git a/src/scanner.ts b/src/scanner.ts index 578dc2ede..422ef83ca 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -471,6 +471,7 @@ export async function scanPackages( saveCache(cache, cacheDirOverride, options.debug ? log : undefined); } + const workspaceMemberNames = readWorkspaceMemberNames(context?.scanFilePath); const findings: Finding[] = mergeResultsByPackage(results).map(result => { // Collapse advisory records OSV marks as the same vulnerability before // anything downstream counts them, so the terminal, JSON, SARIF, both SBOM @@ -487,6 +488,7 @@ export async function scanPackages( dependencyPaths, result.pkg.name, context?.directDependencyNames, + workspaceMemberNames, ); const selectedFix = selectFixedVersion( vulnerabilities, @@ -567,7 +569,6 @@ export async function scanPackages( ? createNpmTransitiveGraphFromLockfile(context.scanFilePath, log, packages.length) : null; const lockParentGraph = loadLockParentGraph(context?.scanSource, context?.scanFilePath, log); - const workspaceMemberNames = readWorkspaceMemberNames(context?.scanFilePath); const npmWorkspaceMap = (() => { try { return isNpmLockSource(context?.scanSource) && context?.scanFilePath @@ -1138,15 +1139,18 @@ function classifyRelationship( paths: string[][], packageName?: string, directDependencyNames?: ReadonlySet | null, + workspaceMemberNames: ReadonlySet = new Set(), ): "direct" | "transitive" | "unknown" { if (packageName && directDependencyNames?.has(packageName)) { // The package name is declared as a direct dependency. But when multiple versions // of the same package are installed (one direct, one transitive), the name alone // is not enough - we need to verify this specific installed version is the direct - // one. A direct install always has at least one path of length 2 (["project", name]). - // If all paths are longer, this is a different (transitive) version of the package. + // one. A direct install has a path ["project", name], or + // ["project", workspaceName, name] for a declared workspace member. + // Longer paths describe a different (transitive) installed version. if (paths.length === 0) return "direct"; // no path data - trust the name - const hasDirectPath = paths.some(p => p.length <= 2); + const hasDirectPath = paths.some(p => p.length <= 2 + || (p.length === 3 && workspaceMemberNames.has(p[1]!))); return hasDirectPath ? "direct" : "transitive"; } if (paths.length === 0) return "unknown"; diff --git a/src/utils/package-json.ts b/src/utils/package-json.ts index 71b020b00..2ebd891c1 100644 --- a/src/utils/package-json.ts +++ b/src/utils/package-json.ts @@ -110,7 +110,8 @@ function readWorkspacePatterns(manifest: Record, projectPath: s return []; } -function resolveWorkspacePackageJsonPaths(projectPath: string, patterns: string[]): string[] { +/** Expands workspace patterns to unique existing member manifests. */ +export function resolveWorkspacePackageJsonPaths(projectPath: string, patterns: string[]): string[] { const matches = new Set(); for (const pattern of patterns) { diff --git a/tests/parsers/yarn-workspace-globs.test.ts b/tests/parsers/yarn-workspace-globs.test.ts new file mode 100644 index 000000000..776c32241 --- /dev/null +++ b/tests/parsers/yarn-workspace-globs.test.ts @@ -0,0 +1,43 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { loadPackages } from "../../src/parsers/index.js"; +import { buildYarnWorkspaceMap } from "../../src/parsers/yarn-lock.js"; +import { buildSuggestedFixCommandPlan } from "../../src/remediation/fix-commands.js"; +import type { Finding } from "../../src/types.js"; +import { removeDir } from "../test-utils.js"; + +const patterns = ["packages/*/apps/*", "packages/**", "packages/team/apps/w*", "packages/team/apps/web/"]; + +describe.each(["classic", "berry"])("Yarn %s workspace globs", (format) => { + it.each(patterns)("scopes fixes and dependency paths for %s", (pattern) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cve-yarn-glob-")); + try { + const member = path.join(dir, "packages/team/apps/web"); + fs.mkdirSync(member, { recursive: true }); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ + name: "root", private: true, workspaces: [pattern], + })); + fs.writeFileSync(path.join(member, "package.json"), JSON.stringify({ + name: "web", dependencies: { lodash: "4.17.20" }, + })); + const lockPath = path.join(dir, "yarn.lock"); + fs.writeFileSync(lockPath, format === "classic" + ? 'lodash@4.17.20:\n version "4.17.20"\n' + : '__metadata:\n version: 8\n\n"lodash@npm:4.17.20":\n version: 4.17.20\n resolution: "lodash@npm:4.17.20"\n'); + + expect(buildYarnWorkspaceMap(lockPath).get("lodash")).toEqual(["web"]); + const input = loadPackages(dir, false, 4); + const pkg = input.packages.find(p => p.name === "lodash")!; + expect(pkg.paths).toEqual([["project", "web", "lodash"]]); + const finding: Finding = { + pkg, vulnerabilities: [{ id: "GHSA-test" }], severity: "high", cveAliases: [], + relationship: "direct", dependencyPaths: pkg.paths ?? [], firstFixedVersion: "4.17.21", + }; + const plan = buildSuggestedFixCommandPlan([finding], input); + expect(plan?.command).toBe("yarn workspace web add lodash@4.17.21"); + } finally { + removeDir(dir); + } + }); +}); diff --git a/tests/scanner-cache.test.ts b/tests/scanner-cache.test.ts index 9b0480f2b..7b1975a7a 100644 --- a/tests/scanner-cache.test.ts +++ b/tests/scanner-cache.test.ts @@ -455,6 +455,46 @@ describe("scanPackages cache behavior", () => { } }); + it.each([false, true])("classifies workspace paths without treating a nested version as direct: %s", async (transitive) => { + const cacheDir = createTempCacheDir(); + const pkg = createPackage("diff", "4.0.2"); + const member = path.join(cacheDir, "packages/team/apps/web"); + fs.mkdirSync(member, { recursive: true }); + fs.writeFileSync(path.join(cacheDir, "package.json"), JSON.stringify({ workspaces: ["packages/*/apps/*"] })); + fs.writeFileSync(path.join(member, "package.json"), JSON.stringify({ name: "web", dependencies: { diff: "4.0.2" } })); + pkg.paths = transitive ? [["project", "web", "parent", "diff"]] : [["project", "web", "diff"]]; + const detail: OsvVuln = { + id: "OSV-DIRECT", + affected: [{ ranges: [{ events: [{ fixed: "4.0.4" }] }] }], + }; + + queryBatchMock.mockResolvedValue([ + { + package: pkg.name, + version: pkg.version, + vulnerabilities: [{ id: "OSV-DIRECT" }], + }, + ]); + getVulnMock.mockResolvedValue(detail); + fetchMock.mockResolvedValue({ + ok: true, + json: async () => ({ versions: { "4.0.2": {}, "4.0.4": {} } }), + }); + + try { + const { findings } = await scanPackages([pkg], 100, createOptions(cacheDir), { + directDependencyNames: new Set(["diff", "typescript"]), + scanSource: "yarn-lock", + scanFilePath: path.join(cacheDir, "yarn.lock"), + }); + + expect(findings).toHaveLength(1); + expect(findings[0]?.relationship).toBe(transitive ? "transitive" : "direct"); + } finally { + removeDir(cacheDir); + } + }); + it("keeps published fixed-version hints unconfirmed when advisory ranges are incomplete", async () => { const cacheDir = createTempCacheDir(); const pkg = createPackage("tar", "2.0.0"); From cfce8c5332f01309e206377a4c668aacb2ee66c3 Mon Sep 17 00:00:00 2001 From: vian Date: Fri, 6 Nov 2026 01:10:33 +0530 Subject: [PATCH 02/14] fix(yarn): address root fixes and workspace identity review --- src/parsers/yarn-lock.ts | 12 ++++- src/remediation/fix-commands.ts | 6 ++- src/scanner.ts | 12 +++-- src/types.ts | 2 + src/utils/fix-runner.ts | 4 +- tests/fix-runner-apply.test.ts | 12 +++++ tests/parsers/yarn-workspace-globs.test.ts | 54 ++++++++++++++++++++++ tests/scanner-cache.test.ts | 4 +- 8 files changed, 94 insertions(+), 12 deletions(-) diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index f24d39667..6cdabfe30 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -90,7 +90,10 @@ function readRootPackageJson(filePath: string): any | null { export function buildYarnWorkspaceMap(filePath: string): Map { const map = new Map(); - for (const { name: wsName, pkg: wsPkg } of readWorkspacePackages(filePath)) { + const members = readWorkspacePackages(filePath); + const root = readRootPackageJson(filePath); + const manifests = root && members.length > 0 ? [{ name: ".", pkg: root }, ...members] : members; + for (const { name: wsName, pkg: wsPkg } of manifests) { for (const depSection of ["dependencies", "optionalDependencies", "devDependencies"]) { const deps = wsPkg[depSection]; if (!deps || typeof deps !== "object") continue; @@ -127,7 +130,7 @@ function readWorkspacePackages(filePath: string, rootPkg?: any): { name: string; } catch { continue; } - if (!wsPkg.name) continue; + if (!wsPkg?.name) continue; result.push({ name: wsPkg.name, pkg: wsPkg }); } @@ -209,6 +212,10 @@ function resolveDepSeeds( selectorToKey.get(`${depName}@${depRange}`) ?? selectorToKey.get(`${depName}@npm:${depRange}`); if (resolved) { + if (pathPrefix.length === 2) { + const direct = map.get(resolved); + if (direct) direct.workspaceDirect = true; + } seeds.push({ key: resolved, path: pathPrefix, dev }); continue; } @@ -218,6 +225,7 @@ function resolveDepSeeds( const name = String(depName); const key = `${name}@${fallbackVersion}`; upsertPackage(map, { name, version: fallbackVersion, ecosystem: "npm", paths: [] }); + if (pathPrefix.length === 2) map.get(key)!.workspaceDirect = true; addTaggedPath(taggedPathsByKey, key, [...pathPrefix, name]); // Unresolved fallback prod deps are not graph nodes, so record their key as // production-reachable directly (mirrors the resolved production roots). diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index ff22fc6c7..669b50d30 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -204,7 +204,7 @@ export function buildSuggestedFixCommandPlan( isUpgradeTarget(finding.pkg.version, directTarget) && validatedFieldOk ) { - const pkgWorkspaces = workspaceMap.get(packageNameKey(finding.pkg))?.filter(w => w !== ".") ?? []; + const pkgWorkspaces = workspaceMap.get(packageNameKey(finding.pkg))?.filter(w => packageManager === "yarn" || w !== ".") ?? []; upsertTarget(targetsByKey, finding, { package: finding.pkg.name, ...(finding.pkg.registryName ? { registryName: finding.pkg.registryName } : {}), @@ -1105,7 +1105,9 @@ function buildWorkspaceInstallCommands( commands.push(`yarn add${flag} ${pkgArgs}`); } else { for (const ws of workspaces) { - commands.push(`yarn workspace ${shellQuote(ws)} add${flag} ${pkgArgs}`); + commands.push(ws === "." + ? `yarn add -W${flag} ${pkgArgs}` + : `yarn workspace ${shellQuote(ws)} add${flag} ${pkgArgs}`); } } } else if (packageManager === "bun") { diff --git a/src/scanner.ts b/src/scanner.ts index 422ef83ca..7950bc6ff 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -471,7 +471,6 @@ export async function scanPackages( saveCache(cache, cacheDirOverride, options.debug ? log : undefined); } - const workspaceMemberNames = readWorkspaceMemberNames(context?.scanFilePath); const findings: Finding[] = mergeResultsByPackage(results).map(result => { // Collapse advisory records OSV marks as the same vulnerability before // anything downstream counts them, so the terminal, JSON, SARIF, both SBOM @@ -488,7 +487,7 @@ export async function scanPackages( dependencyPaths, result.pkg.name, context?.directDependencyNames, - workspaceMemberNames, + result.pkg.workspaceDirect === true, ); const selectedFix = selectFixedVersion( vulnerabilities, @@ -569,6 +568,7 @@ export async function scanPackages( ? createNpmTransitiveGraphFromLockfile(context.scanFilePath, log, packages.length) : null; const lockParentGraph = loadLockParentGraph(context?.scanSource, context?.scanFilePath, log); + const workspaceMemberNames = readWorkspaceMemberNames(context?.scanFilePath); const npmWorkspaceMap = (() => { try { return isNpmLockSource(context?.scanSource) && context?.scanFilePath @@ -1139,18 +1139,20 @@ function classifyRelationship( paths: string[][], packageName?: string, directDependencyNames?: ReadonlySet | null, - workspaceMemberNames: ReadonlySet = new Set(), + workspaceDirect = false, ): "direct" | "transitive" | "unknown" { if (packageName && directDependencyNames?.has(packageName)) { // The package name is declared as a direct dependency. But when multiple versions // of the same package are installed (one direct, one transitive), the name alone // is not enough - we need to verify this specific installed version is the direct // one. A direct install has a path ["project", name], or - // ["project", workspaceName, name] for a declared workspace member. + // ["project", workspaceName, name] with parser-confirmed workspace identity. + // A registry parent can share a workspace name, so the display path alone + // cannot prove that this resolved version is a workspace dependency. // Longer paths describe a different (transitive) installed version. if (paths.length === 0) return "direct"; // no path data - trust the name const hasDirectPath = paths.some(p => p.length <= 2 - || (p.length === 3 && workspaceMemberNames.has(p[1]!))); + || (p.length === 3 && workspaceDirect)); return hasDirectPath ? "direct" : "transitive"; } if (paths.length === 0) return "unknown"; diff --git a/src/types.ts b/src/types.ts index 7b0befbdb..6d413c6ee 100644 --- a/src/types.ts +++ b/src/types.ts @@ -21,6 +21,8 @@ export type PackageRef = { ecosystem: string; dev?: boolean; paths?: string[][]; + /** This resolved version is declared by a Yarn workspace member, independent of display paths. */ + workspaceDirect?: boolean; resolvedUrl?: string; /** * SPDX license expression as declared by the package, when the lockfile diff --git a/src/utils/fix-runner.ts b/src/utils/fix-runner.ts index 7dbcc27c5..92f848605 100644 --- a/src/utils/fix-runner.ts +++ b/src/utils/fix-runner.ts @@ -187,7 +187,7 @@ function buildInstallApplyArgv( packageManager: SuggestedFixCommandPlan["packageManager"], target: SuggestedFixTarget, ): string[][] { - const workspaces = (target.workspaces ?? []).filter(w => w !== "."); + const workspaces = (target.workspaces ?? []).filter(w => packageManager === "yarn" || w !== "."); const pkgArg = installPackageSpec(target); const isDev = Boolean(target.isDev); @@ -220,7 +220,7 @@ function buildInstallApplyArgv( return [["yarn", ...args]]; } return workspaces.map(ws => { - const args = ["workspace", ws, "add"]; + const args = ws === "." ? ["add", "-W"] : ["workspace", ws, "add"]; if (isDev) args.push("-D"); args.push(pkgArg); return ["yarn", ...args]; diff --git a/tests/fix-runner-apply.test.ts b/tests/fix-runner-apply.test.ts index b7d621f74..2c573af4d 100644 --- a/tests/fix-runner-apply.test.ts +++ b/tests/fix-runner-apply.test.ts @@ -443,3 +443,15 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { }); }); + +it.each([false, true])("applies Yarn fixes to root and member with matching dependency flags: %s", async (isDev) => { + const { argvs, run } = recordingRunner(); + const yarnPlan = plan([target({ package: "lodash", workspaces: [".", "web"], isDev })]); + yarnPlan.packageManager = "yarn"; + await applyFixesIfRequested({ plan: yarnPlan, projectPath: "/tmp/proj", totalFindings: 1, options, runCommand: run }); + const flag = isDev ? ["-D"] : []; + expect(argvs).toEqual([ + ["yarn", "add", "-W", ...flag, "lodash@1.0.1"], + ["yarn", "workspace", "web", "add", ...flag, "lodash@1.0.1"], + ]); +}); diff --git a/tests/parsers/yarn-workspace-globs.test.ts b/tests/parsers/yarn-workspace-globs.test.ts index 776c32241..bc10b3517 100644 --- a/tests/parsers/yarn-workspace-globs.test.ts +++ b/tests/parsers/yarn-workspace-globs.test.ts @@ -41,3 +41,57 @@ describe.each(["classic", "berry"])("Yarn %s workspace globs", (format) => { } }); }); + +it.each([false, true])("fixes root and member declarations together (dev: %s)", (dev) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cve-yarn-root-")); + try { + fs.mkdirSync(path.join(dir, "packages/web"), { recursive: true }); + const deps = { [dev ? "devDependencies" : "dependencies"]: { lodash: "4.17.20" } }; + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ private: true, workspaces: ["packages/*"], ...deps })); + fs.writeFileSync(path.join(dir, "packages/web/package.json"), JSON.stringify({ name: "web", ...deps })); + fs.writeFileSync(path.join(dir, "yarn.lock"), 'lodash@4.17.20:\n version "4.17.20"\n'); + const input = loadPackages(dir, false, 4); + const pkg = input.packages[0]!; + const finding: Finding = { + pkg, vulnerabilities: [{ id: "GHSA-test" }], severity: "high", cveAliases: [], + relationship: "direct", dependencyPaths: pkg.paths ?? [], firstFixedVersion: "4.17.21", + }; + const plan = buildSuggestedFixCommandPlan([finding], input); + const flag = dev ? " -D" : ""; + expect(plan?.command).toBe(`yarn add -W${flag} lodash@4.17.21 && yarn workspace web add${flag} lodash@4.17.21`); + expect(plan?.targets[0]?.workspaces).toEqual([".", "web"]); + } finally { removeDir(dir); } +}); + +it("skips a null manifest and still finds valid members", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cve-yarn-null-")); + try { + for (const name of ["bad", "web"]) fs.mkdirSync(path.join(dir, "packages", name), { recursive: true }); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ private: true, workspaces: ["packages/*"] })); + fs.writeFileSync(path.join(dir, "packages/bad/package.json"), "null"); + fs.writeFileSync(path.join(dir, "packages/web/package.json"), JSON.stringify({ name: "web", dependencies: { lodash: "4.17.20" } })); + fs.writeFileSync(path.join(dir, "yarn.lock"), 'lodash@4.17.20:\n version "4.17.20"\n'); + expect(buildYarnWorkspaceMap(path.join(dir, "yarn.lock")).get("lodash")).toEqual(["web"]); + expect(loadPackages(dir, false, 4).packages[0]?.workspaceDirect).toBe(true); + } finally { removeDir(dir); } +}); + +it.each(["classic", "berry"])("distinguishes registry and workspace parents named web in Yarn %s", (format) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cve-yarn-identity-")); + try { + fs.mkdirSync(path.join(dir, "packages/web"), { recursive: true }); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ private: true, workspaces: ["packages/*"], dependencies: { web: "1.0.0" } })); + fs.writeFileSync(path.join(dir, "packages/web/package.json"), JSON.stringify({ name: "web", dependencies: { diff: "2.0.0" } })); + fs.writeFileSync(path.join(dir, "yarn.lock"), format === "classic" + ? 'web@1.0.0:\n version "1.0.0"\n dependencies:\n diff "1.0.0"\n\ndiff@1.0.0:\n version "1.0.0"\n\ndiff@2.0.0:\n version "2.0.0"\n' + : '__metadata:\n version: 8\n\n"web@npm:1.0.0":\n version: 1.0.0\n resolution: "web@npm:1.0.0"\n dependencies:\n diff: 1.0.0\n\n"diff@npm:1.0.0":\n version: 1.0.0\n resolution: "diff@npm:1.0.0"\n\n"diff@npm:2.0.0":\n version: 2.0.0\n resolution: "diff@npm:2.0.0"\n'); + const packages = loadPackages(dir, false, 4).packages; + const nested = packages.find(p => p.name === "diff" && p.version === "1.0.0"); + const direct = packages.find(p => p.name === "diff" && p.version === "2.0.0"); + // Display paths collide, but only the workspace's resolved version is direct. + expect(nested?.paths).toEqual([["project", "web", "diff"]]); + expect(direct?.paths).toEqual(nested?.paths); + expect(nested?.workspaceDirect).not.toBe(true); + expect(direct?.workspaceDirect).toBe(true); + } finally { removeDir(dir); } +}); diff --git a/tests/scanner-cache.test.ts b/tests/scanner-cache.test.ts index 7b1975a7a..e6b391254 100644 --- a/tests/scanner-cache.test.ts +++ b/tests/scanner-cache.test.ts @@ -462,7 +462,9 @@ describe("scanPackages cache behavior", () => { fs.mkdirSync(member, { recursive: true }); fs.writeFileSync(path.join(cacheDir, "package.json"), JSON.stringify({ workspaces: ["packages/*/apps/*"] })); fs.writeFileSync(path.join(member, "package.json"), JSON.stringify({ name: "web", dependencies: { diff: "4.0.2" } })); - pkg.paths = transitive ? [["project", "web", "parent", "diff"]] : [["project", "web", "diff"]]; + // Both display paths are identical; only the resolved workspace identity differs. + pkg.paths = [["project", "web", "diff"]]; + pkg.workspaceDirect = !transitive; const detail: OsvVuln = { id: "OSV-DIRECT", affected: [{ ranges: [{ events: [{ fixed: "4.0.4" }] }] }], From 10a113d6e8e55b9c233f8b7e7c5b55d95e5af94a Mon Sep 17 00:00:00 2001 From: vian Date: Fri, 6 Nov 2026 01:21:57 +0530 Subject: [PATCH 03/14] fix(yarn): preserve resolved version and transitive coverage --- src/parsers/yarn-lock.ts | 25 +++++-- src/remediation/fix-commands.ts | 20 +++++- src/scanner.ts | 7 +- src/types.ts | 4 ++ src/utils/fix-runner.ts | 2 +- tests/fix-runner-apply.test.ts | 8 +++ tests/parsers/yarn-workspace-globs.test.ts | 44 +++++++++++- tests/scanner-cache.test.ts | 81 ++++++++++++++++++++++ 8 files changed, 179 insertions(+), 12 deletions(-) diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index 6cdabfe30..8092c8166 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -161,7 +161,7 @@ function collectYarnPaths( map: Map, taggedPathsByKey: Map, ): void { - const queue = seeds.map(seed => ({ key: seed.key, path: seed.path })); + const queue = seeds.map(seed => ({ key: seed.key, path: seed.path, depth: 0 })); const visitedStates = new Set(); let head = 0; @@ -171,11 +171,18 @@ function collectYarnPaths( if (!ref) continue; const nextPath = [...current.path, ref.name]; + // Preserve reachability before display-path de-duplication: registry and + // workspace parents can produce exactly the same printable path. + const pkg = map.get(ref.key); + if (pkg && current.depth > 0) { + const paths = pkg.yarnTransitivePaths ??= []; + if (paths.length < MAX_PATHS_PER_PACKAGE && !paths.some(p => pathsEqual(p, nextPath))) paths.push(nextPath); + } const stateKey = `${ref.key}>${nextPath.join(">")}`; if (visitedStates.has(stateKey)) continue; visitedStates.add(stateKey); - // Only record paths for packages that exist in the lockfile map, matching prior behavior. + // Only record paths for packages that exist in the lockfile map. if (map.has(ref.key)) addTaggedPath(taggedPathsByKey, ref.key, nextPath); if (nextPath.length >= MAX_PATH_DEPTH) continue; @@ -185,7 +192,7 @@ function collectYarnPaths( } for (const child of graph.get(ref.key) ?? []) { - queue.push({ key: child, path: nextPath }); + queue.push({ key: child, path: nextPath, depth: current.depth + 1 }); } } } @@ -216,6 +223,12 @@ function resolveDepSeeds( const direct = map.get(resolved); if (direct) direct.workspaceDirect = true; } + const direct = map.get(resolved); + if (direct) { + const workspaces = direct.yarnDirectWorkspaces ??= []; + const workspace = pathPrefix[1] ?? "."; + if (!workspaces.includes(workspace)) workspaces.push(workspace); + } seeds.push({ key: resolved, path: pathPrefix, dev }); continue; } @@ -225,7 +238,11 @@ function resolveDepSeeds( const name = String(depName); const key = `${name}@${fallbackVersion}`; upsertPackage(map, { name, version: fallbackVersion, ecosystem: "npm", paths: [] }); - if (pathPrefix.length === 2) map.get(key)!.workspaceDirect = true; + const direct = map.get(key)!; + if (pathPrefix.length === 2) direct.workspaceDirect = true; + const workspaces = direct.yarnDirectWorkspaces ??= []; + const workspace = pathPrefix[1] ?? "."; + if (!workspaces.includes(workspace)) workspaces.push(workspace); addTaggedPath(taggedPathsByKey, key, [...pathPrefix, name]); // Unresolved fallback prod deps are not graph nodes, so record their key as // production-reachable directly (mirrors the resolved production roots). diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index 669b50d30..af0751e49 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -1,4 +1,5 @@ import path from "node:path"; +import { safeReadText } from "../utils/file.js"; import type { Finding, ScanInput, SeverityLabel, ChainHop } from "../types.js"; import { severityOrder } from "../constants.js"; import { compareVersions, looksLikeVersion } from "../utils/version.js"; @@ -8,7 +9,7 @@ import { calculatePathCoverage, formatDependencyPath } from "../utils/path-cover import { pluralize } from "../utils/string.js"; import { buildNpmWorkspaceMap } from "../parsers/package-lock.js"; import { buildPnpmWorkspaceMap } from "../parsers/pnpm-lock.js"; -import { buildYarnWorkspaceMap } from "../parsers/yarn-lock.js"; +import { buildYarnWorkspaceMap, isYarnBerry } 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"; @@ -53,6 +54,7 @@ export type SuggestedFixTarget = { reason: string; command?: string; workspaces?: string[]; + yarnBerry?: boolean; fixVersionPublishedAt?: string | null; cooldownWarning?: CooldownWarning | null; coverage?: "complete" | "partial"; @@ -104,6 +106,8 @@ export function buildSuggestedFixCommandPlan( const packageManager = packageManagerForSource(scanInput.source); if (!packageManager) return null; const offline = !!options?.offline; + const yarnBerry = packageManager === "yarn" && !!scanInput.filePath + && isYarnBerry(safeReadText(scanInput.filePath) ?? ""); let workspaceMap: Map = new Map(); if (scanInput.filePath) { @@ -204,7 +208,8 @@ export function buildSuggestedFixCommandPlan( isUpgradeTarget(finding.pkg.version, directTarget) && validatedFieldOk ) { - const pkgWorkspaces = workspaceMap.get(packageNameKey(finding.pkg))?.filter(w => packageManager === "yarn" || w !== ".") ?? []; + const pkgWorkspaces = (packageManager === "yarn" && finding.pkg.yarnDirectWorkspaces + ? finding.pkg.yarnDirectWorkspaces : workspaceMap.get(packageNameKey(finding.pkg)))?.filter(w => packageManager === "yarn" || w !== ".") ?? []; upsertTarget(targetsByKey, finding, { package: finding.pkg.name, ...(finding.pkg.registryName ? { registryName: finding.pkg.registryName } : {}), @@ -261,6 +266,14 @@ export function buildSuggestedFixCommandPlan( continue; } + if (finding.relationship === "transitive" && finding.pkg.yarnDirectWorkspaces?.length + && finding.pkg.yarnTransitivePaths?.length) { + skippedByKey.set(`direct:${packageInstanceKey(finding.pkg)}`, { + package: finding.pkg.name, version: finding.pkg.version, relationship: "direct", + reason: `Also declared directly in ${finding.pkg.yarnDirectWorkspaces.join(", ")}. Update these declarations separately; parent remediation covers only the transitive paths. Rescan afterward.`, + }); + } + if (finding.relationship === "unknown") { skippedByKey.set(`unknown:${packageInstanceKey(finding.pkg)}`, { package: finding.pkg.name, @@ -503,6 +516,7 @@ export function buildSuggestedFixCommandPlan( } continue; } + if (packageManager === "yarn") target.yarnBerry = yarnBerry; if (target.command && target.workspaces?.length) { const scoped = buildWorkspaceInstallCommands([target], packageManager); if (scoped.length > 0) target.command = scoped.join(" && "); @@ -1106,7 +1120,7 @@ function buildWorkspaceInstallCommands( } else { for (const ws of workspaces) { commands.push(ws === "." - ? `yarn add -W${flag} ${pkgArgs}` + ? `yarn add${targets[0]?.yarnBerry ? "" : " -W"}${flag} ${pkgArgs}` : `yarn workspace ${shellQuote(ws)} add${flag} ${pkgArgs}`); } } diff --git a/src/scanner.ts b/src/scanner.ts index 7950bc6ff..703a97e75 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -482,8 +482,11 @@ export async function scanPackages( const severity = maxSeverity(vulnerabilities); const cveAliases = extractCveAliases(vulnerabilities); - const dependencyPaths = result.pkg.paths ?? []; - const relationship = classifyRelationship( + // Mixed Yarn reachability must retain parent remediation; direct declarations + // are carried separately on the resolved package for the fix-plan warning. + const mixedYarn = !!result.pkg.yarnDirectWorkspaces?.length && !!result.pkg.yarnTransitivePaths?.length; + const dependencyPaths = mixedYarn ? result.pkg.yarnTransitivePaths! : result.pkg.paths ?? []; + const relationship = mixedYarn ? "transitive" : classifyRelationship( dependencyPaths, result.pkg.name, context?.directDependencyNames, diff --git a/src/types.ts b/src/types.ts index 6d413c6ee..d961e6242 100644 --- a/src/types.ts +++ b/src/types.ts @@ -23,6 +23,10 @@ export type PackageRef = { paths?: string[][]; /** This resolved version is declared by a Yarn workspace member, independent of display paths. */ workspaceDirect?: boolean; + /** Yarn declarations resolving to this exact package version (root is "."). */ + yarnDirectWorkspaces?: string[]; + /** Paths reached through a registry dependency rather than a declaration seed. */ + yarnTransitivePaths?: string[][]; resolvedUrl?: string; /** * SPDX license expression as declared by the package, when the lockfile diff --git a/src/utils/fix-runner.ts b/src/utils/fix-runner.ts index 92f848605..7b9daad55 100644 --- a/src/utils/fix-runner.ts +++ b/src/utils/fix-runner.ts @@ -220,7 +220,7 @@ function buildInstallApplyArgv( return [["yarn", ...args]]; } return workspaces.map(ws => { - const args = ws === "." ? ["add", "-W"] : ["workspace", ws, "add"]; + const args = ws === "." ? ["add", ...(target.yarnBerry ? [] : ["-W"])] : ["workspace", ws, "add"]; if (isDev) args.push("-D"); args.push(pkgArg); return ["yarn", ...args]; diff --git a/tests/fix-runner-apply.test.ts b/tests/fix-runner-apply.test.ts index 2c573af4d..75e5d9de9 100644 --- a/tests/fix-runner-apply.test.ts +++ b/tests/fix-runner-apply.test.ts @@ -455,3 +455,11 @@ it.each([false, true])("applies Yarn fixes to root and member with matching depe ["yarn", "workspace", "web", "add", ...flag, "lodash@1.0.1"], ]); }); + +it("omits the Classic-only root flag when applying a Berry target", async () => { + const { argvs, run } = recordingRunner(); + const yarnPlan = plan([target({ package: "lodash", workspaces: [".", "web"], yarnBerry: true })]); + yarnPlan.packageManager = "yarn"; + await applyFixesIfRequested({ plan: yarnPlan, projectPath: "/tmp/proj", totalFindings: 1, options, runCommand: run }); + expect(argvs).toEqual([["yarn", "add", "lodash@1.0.1"], ["yarn", "workspace", "web", "add", "lodash@1.0.1"]]); +}); diff --git a/tests/parsers/yarn-workspace-globs.test.ts b/tests/parsers/yarn-workspace-globs.test.ts index bc10b3517..d56d6b901 100644 --- a/tests/parsers/yarn-workspace-globs.test.ts +++ b/tests/parsers/yarn-workspace-globs.test.ts @@ -76,18 +76,24 @@ it("skips a null manifest and still finds valid members", () => { } finally { removeDir(dir); } }); -it.each(["classic", "berry"])("distinguishes registry and workspace parents named web in Yarn %s", (format) => { +it.each(["classic", "berry"].flatMap(format => [false, true].map(shared => ({ format, shared }))))("distinguishes registry/workspace paths in $format (shared version: $shared)", ({ format, shared }) => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cve-yarn-identity-")); try { fs.mkdirSync(path.join(dir, "packages/web"), { recursive: true }); fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ private: true, workspaces: ["packages/*"], dependencies: { web: "1.0.0" } })); - fs.writeFileSync(path.join(dir, "packages/web/package.json"), JSON.stringify({ name: "web", dependencies: { diff: "2.0.0" } })); + fs.writeFileSync(path.join(dir, "packages/web/package.json"), JSON.stringify({ name: "web", dependencies: { diff: shared ? "1.0.0" : "2.0.0" } })); fs.writeFileSync(path.join(dir, "yarn.lock"), format === "classic" ? 'web@1.0.0:\n version "1.0.0"\n dependencies:\n diff "1.0.0"\n\ndiff@1.0.0:\n version "1.0.0"\n\ndiff@2.0.0:\n version "2.0.0"\n' : '__metadata:\n version: 8\n\n"web@npm:1.0.0":\n version: 1.0.0\n resolution: "web@npm:1.0.0"\n dependencies:\n diff: 1.0.0\n\n"diff@npm:1.0.0":\n version: 1.0.0\n resolution: "diff@npm:1.0.0"\n\n"diff@npm:2.0.0":\n version: 2.0.0\n resolution: "diff@npm:2.0.0"\n'); const packages = loadPackages(dir, false, 4).packages; const nested = packages.find(p => p.name === "diff" && p.version === "1.0.0"); const direct = packages.find(p => p.name === "diff" && p.version === "2.0.0"); + if (shared) { + expect(nested?.workspaceDirect).toBe(true); + expect(nested?.yarnDirectWorkspaces).toEqual(["web"]); + expect(nested?.yarnTransitivePaths).toEqual([["project", "web", "diff"]]); + return; + } // Display paths collide, but only the workspace's resolved version is direct. expect(nested?.paths).toEqual([["project", "web", "diff"]]); expect(direct?.paths).toEqual(nested?.paths); @@ -95,3 +101,37 @@ it.each(["classic", "berry"])("distinguishes registry and workspace parents name expect(direct?.workspaceDirect).toBe(true); } finally { removeDir(dir); } }); + +it.each(["classic", "berry"])("selects only declarations resolving to the vulnerable version in %s", (format) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cve-yarn-versions-")); + try { + fs.mkdirSync(path.join(dir, "packages/web"), { recursive: true }); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ private: true, workspaces: ["packages/*"], dependencies: { lodash: "4.18.1" } })); + fs.writeFileSync(path.join(dir, "packages/web/package.json"), JSON.stringify({ name: "web", dependencies: { lodash: "4.17.20" } })); + fs.writeFileSync(path.join(dir, "yarn.lock"), format === "classic" + ? 'lodash@4.18.1:\n version "4.18.1"\n\nlodash@4.17.20:\n version "4.17.20"\n' + : '__metadata:\n version: 8\n\n"lodash@npm:4.18.1":\n version: 4.18.1\n resolution: "lodash@npm:4.18.1"\n\n"lodash@npm:4.17.20":\n version: 4.17.20\n resolution: "lodash@npm:4.17.20"\n'); + const input = loadPackages(dir, false, 4); + const pkg = input.packages.find(p => p.version === "4.17.20")!; + const finding: Finding = { pkg, vulnerabilities: [{ id: "GHSA-test" }], severity: "high", cveAliases: [], relationship: "direct", dependencyPaths: pkg.paths ?? [], firstFixedVersion: "4.17.21" }; + expect(buildSuggestedFixCommandPlan([finding], input)?.command).toBe("yarn workspace web add lodash@4.17.21"); + expect(pkg.yarnDirectWorkspaces).toEqual(["web"]); + // The root is separately attributed to the newer resolved version. + const rootPkg = input.packages.find(p => p.version === "4.18.1")!; + expect(rootPkg.yarnDirectWorkspaces).toEqual(["."]); + const rootFinding = { ...finding, pkg: rootPkg, firstFixedVersion: "4.19.0" }; + expect(buildSuggestedFixCommandPlan([rootFinding], input)?.command).toBe(`yarn add${format === "classic" ? " -W" : ""} lodash@4.19.0`); + } finally { removeDir(dir); } +}); + +it("retains parent remediation and reports the direct declaration on mixed findings", () => { + const finding: Finding = { + pkg: { name: "diff", version: "1.0.0", ecosystem: "npm", yarnDirectWorkspaces: ["web"], yarnTransitivePaths: [["project", "parent", "diff"]] }, + vulnerabilities: [{ id: "GHSA-test" }], severity: "high", cveAliases: [], relationship: "transitive", + dependencyPaths: [["project", "parent", "diff"]], firstFixedVersion: "2.0.0", + recommendedParentUpgrade: { package: "parent", currentVersion: "1.0.0", targetVersion: "2.0.0", viaPath: ["project", "parent", "diff"], vulnerablePackage: "diff", confidence: "verified", reason: "parent@2 resolves diff@2" }, + }; + const plan = buildSuggestedFixCommandPlan([finding], { mode: "resolved-lockfile", source: "yarn-lock", filePath: null, packages: [finding.pkg], notes: [], warnings: [], skippedDependencies: [] }); + expect(plan?.targets).toEqual(expect.arrayContaining([expect.objectContaining({ package: "parent", kind: "parent-upgrade" })])); + expect(plan?.skipped).toEqual(expect.arrayContaining([expect.objectContaining({ relationship: "direct", reason: expect.stringContaining("Also declared directly in web") })])); +}); diff --git a/tests/scanner-cache.test.ts b/tests/scanner-cache.test.ts index e6b391254..309574028 100644 --- a/tests/scanner-cache.test.ts +++ b/tests/scanner-cache.test.ts @@ -497,6 +497,87 @@ describe("scanPackages cache behavior", () => { } }); + it("preserves parent remediation for a version also declared by a workspace", async () => { + const cacheDir = createTempCacheDir(); + const pkg = createPackage("diff", "4.0.2"); + pkg.workspaceDirect = true; + pkg.yarnDirectWorkspaces = ["web"]; + pkg.paths = [["project", "web", "diff"], ["project", "registry-parent", "diff"]]; + pkg.yarnTransitivePaths = [["project", "registry-parent", "diff"]]; + const detail: OsvVuln = { + id: "OSV-DIRECT", + affected: [{ ranges: [{ events: [{ fixed: "4.0.4" }] }] }], + }; + + queryBatchMock.mockResolvedValue([ + { + package: pkg.name, + version: pkg.version, + vulnerabilities: [{ id: "OSV-DIRECT" }], + }, + ]); + getVulnMock.mockResolvedValue(detail); + fetchMock.mockResolvedValue({ + ok: true, + json: async () => ({ versions: { "4.0.2": {}, "4.0.4": {} } }), + }); + + try { + const { findings } = await scanPackages([pkg], 100, createOptions(cacheDir), { + directDependencyNames: new Set(["diff", "typescript"]), + }); + + expect(findings).toHaveLength(1); + expect(findings[0]?.relationship).toBe("transitive"); + expect(findings[0]?.dependencyPaths).toEqual([["project", "registry-parent", "diff"]]); + expect(findings[0]?.pkg.yarnDirectWorkspaces).toEqual(["web"]); + } finally { + removeDir(cacheDir); + } + }); + + it.each([false, true])("classifies workspace paths without treating a nested version as direct: %s", async (transitive) => { + const cacheDir = createTempCacheDir(); + const pkg = createPackage("diff", "4.0.2"); + const member = path.join(cacheDir, "packages/team/apps/web"); + fs.mkdirSync(member, { recursive: true }); + fs.writeFileSync(path.join(cacheDir, "package.json"), JSON.stringify({ workspaces: ["packages/*/apps/*"] })); + fs.writeFileSync(path.join(member, "package.json"), JSON.stringify({ name: "web", dependencies: { diff: "4.0.2" } })); + // Both display paths are identical; only the resolved workspace identity differs. + pkg.paths = [["project", "web", "diff"]]; + pkg.workspaceDirect = !transitive; + const detail: OsvVuln = { + id: "OSV-DIRECT", + affected: [{ ranges: [{ events: [{ fixed: "4.0.4" }] }] }], + }; + + queryBatchMock.mockResolvedValue([ + { + package: pkg.name, + version: pkg.version, + vulnerabilities: [{ id: "OSV-DIRECT" }], + }, + ]); + getVulnMock.mockResolvedValue(detail); + fetchMock.mockResolvedValue({ + ok: true, + json: async () => ({ versions: { "4.0.2": {}, "4.0.4": {} } }), + }); + + try { + const { findings } = await scanPackages([pkg], 100, createOptions(cacheDir), { + directDependencyNames: new Set(["diff", "typescript"]), + scanSource: "yarn-lock", + scanFilePath: path.join(cacheDir, "yarn.lock"), + }); + + expect(findings).toHaveLength(1); + expect(findings[0]?.relationship).toBe(transitive ? "transitive" : "direct"); + } finally { + removeDir(cacheDir); + } + }); + it("keeps published fixed-version hints unconfirmed when advisory ranges are incomplete", async () => { const cacheDir = createTempCacheDir(); const pkg = createPackage("tar", "2.0.0"); From 7bf583fdaa8b6ecfec5389cc9518bfc381b45bae Mon Sep 17 00:00:00 2001 From: vian Date: Fri, 6 Nov 2026 01:35:50 +0530 Subject: [PATCH 04/14] fix(yarn): retain affected root declarations in parent upgrades --- src/remediation/fix-commands.ts | 17 +++++++++--- tests/parsers/yarn-workspace-globs.test.ts | 31 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index af0751e49..879acbda7 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -126,6 +126,16 @@ export function buildSuggestedFixCommandPlan( } } + // Use the same resolved declaration selection for direct and parent targets. + // Yarn's root is a real target, but only for the version being upgraded. + function workspacesFor(name: string, version: string): string[] { + const resolved = packageManager === "yarn" + ? scanInput.packages.find(pkg => packageNameKey(pkg) === name && pkg.version === version) + : undefined; + const workspaces = resolved?.yarnDirectWorkspaces ?? workspaceMap.get(name) ?? []; + return workspaces.filter(workspace => packageManager === "yarn" || workspace !== "."); + } + const devLookup = new Map(); for (const pkg of scanInput.packages) { devLookup.set(`${pkg.name}@${pkg.version}`, pkg.dev ?? false); @@ -208,8 +218,7 @@ export function buildSuggestedFixCommandPlan( isUpgradeTarget(finding.pkg.version, directTarget) && validatedFieldOk ) { - const pkgWorkspaces = (packageManager === "yarn" && finding.pkg.yarnDirectWorkspaces - ? finding.pkg.yarnDirectWorkspaces : workspaceMap.get(packageNameKey(finding.pkg)))?.filter(w => packageManager === "yarn" || w !== ".") ?? []; + const pkgWorkspaces = workspacesFor(packageNameKey(finding.pkg), finding.pkg.version); upsertTarget(targetsByKey, finding, { package: finding.pkg.name, ...(finding.pkg.registryName ? { registryName: finding.pkg.registryName } : {}), @@ -376,7 +385,7 @@ export function buildSuggestedFixCommandPlan( const chainForParentUpgrade = finding.chainResolution?.directDep === finding.recommendedParentUpgrade.package ? finding.chainResolution : null; - const parentUpgradeWorkspaces = workspaceMap.get(finding.recommendedParentUpgrade.package)?.filter(w => w !== ".") ?? []; + const parentUpgradeWorkspaces = workspacesFor(finding.recommendedParentUpgrade.package, finding.recommendedParentUpgrade.currentVersion); upsertTarget(targetsByKey, finding, { package: finding.recommendedParentUpgrade.package, currentVersion: finding.recommendedParentUpgrade.currentVersion, @@ -440,7 +449,7 @@ export function buildSuggestedFixCommandPlan( if (finding.chainResolution) { 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 !== ".") ?? []; + const chainWorkspaces = workspacesFor(finding.chainResolution.directDep, finding.chainResolution.directDepCurrentVersion); upsertTarget(targetsByKey, finding, { package: finding.chainResolution.directDep, currentVersion: finding.chainResolution.directDepCurrentVersion, diff --git a/tests/parsers/yarn-workspace-globs.test.ts b/tests/parsers/yarn-workspace-globs.test.ts index d56d6b901..2fa7e92af 100644 --- a/tests/parsers/yarn-workspace-globs.test.ts +++ b/tests/parsers/yarn-workspace-globs.test.ts @@ -135,3 +135,34 @@ it("retains parent remediation and reports the direct declaration on mixed findi expect(plan?.targets).toEqual(expect.arrayContaining([expect.objectContaining({ package: "parent", kind: "parent-upgrade" })])); expect(plan?.skipped).toEqual(expect.arrayContaining([expect.objectContaining({ relationship: "direct", reason: expect.stringContaining("Also declared directly in web") })])); }); + +it.each(["classic", "berry"].flatMap(format => [false, true].flatMap(chain => [false, true].map(newerRoot => ({ format, chain, newerRoot }))))) +("scopes $format parent upgrades (chain: $chain, newer root: $newerRoot)", ({ format, chain, newerRoot }) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cve-yarn-parent-")); + try { + fs.mkdirSync(path.join(dir, "packages/web"), { recursive: true }); + const rootVersion = newerRoot ? "3.0.0" : "1.0.0"; + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ private: true, workspaces: ["packages/*"], dependencies: { parent: rootVersion } })); + fs.writeFileSync(path.join(dir, "packages/web/package.json"), JSON.stringify({ name: "web", dependencies: { parent: "1.0.0" } })); + const versions = newerRoot ? ["1.0.0", "3.0.0"] : ["1.0.0"]; + const lock = versions.map(v => format === "classic" + ? `parent@${v}:\n version "${v}"\n` + : `"parent@npm:${v}":\n version: ${v}\n resolution: "parent@npm:${v}"\n`).join("\n"); + fs.writeFileSync(path.join(dir, "yarn.lock"), (format === "berry" ? "__metadata:\n version: 8\n\n" : "") + lock); + const input = loadPackages(dir, false, 4); + const finding: Finding = { + pkg: { name: "diff", version: "1.0.0", ecosystem: "npm" }, vulnerabilities: [{ id: "GHSA-test" }], + severity: "high", cveAliases: [], relationship: "transitive", dependencyPaths: [["project", "parent", "diff"]], firstFixedVersion: "2.0.0", + ...(chain ? { + chainResolution: { directDep: "parent", directDepCurrentVersion: "1.0.0", targetVersion: "2.0.0", chain: [], safeVersion: "2.0.0", command: "yarn add parent@2.0.0", coveredPaths: 1, totalPaths: 1 }, + } : { + recommendedParentUpgrade: { package: "parent", currentVersion: "1.0.0", targetVersion: "2.0.0", viaPath: ["project", "parent", "diff"], vulnerablePackage: "diff", confidence: "verified", reason: "parent@2 resolves diff@2" }, + }), + }; + const plan = buildSuggestedFixCommandPlan([finding], input); + const memberCommand = "yarn workspace web add parent@2.0.0"; + const rootCommand = `yarn add${format === "classic" ? " -W" : ""} parent@2.0.0`; + expect(plan?.targets[0]?.workspaces).toEqual(newerRoot ? ["web"] : [".", "web"]); + expect(plan?.command).toBe(newerRoot ? memberCommand : `${rootCommand} && ${memberCommand}`); + } finally { removeDir(dir); } +}); From 11296973a0ae22181ae8bc29c0b0907d264aed18 Mon Sep 17 00:00:00 2001 From: vian Date: Fri, 6 Nov 2026 13:04:23 +0530 Subject: [PATCH 05/14] fix(remediation): select chain parent version from affected path --- src/remediation/transitive-chain-resolver.ts | 117 +++++++++++-------- src/scanner.ts | 5 +- tests/transitive-chain-resolver.test.ts | 77 ++++++++++-- 3 files changed, 133 insertions(+), 66 deletions(-) diff --git a/src/remediation/transitive-chain-resolver.ts b/src/remediation/transitive-chain-resolver.ts index 36d409bf4..1458ca0f8 100644 --- a/src/remediation/transitive-chain-resolver.ts +++ b/src/remediation/transitive-chain-resolver.ts @@ -1,15 +1,16 @@ -import type { Finding, ChainHop, ChainResolution } from "../types.js"; +import type { Finding, ChainHop, ChainResolution, PackageRef } from "../types.js"; import type { SuggestedFixPackageManager } from "./fix-commands.js"; import { commandPrefix } from "./fix-commands.js"; import { fetchPackument } from "./npm-registry.js"; import { compareVersions, looksLikeVersion, isPreReleaseVersion, satisfiesRange } from "../utils/version.js"; +import type { LockParentGraph } from "./npm-transitive-resolution.js"; import { highestVersionSatisfyingRange } from "./npm-transitive-resolution.js"; export async function resolveChainFix( finding: Finding, - installedVersions: Map, + packages: PackageRef[], packageManager: SuggestedFixPackageManager, - options: { offline: boolean }, + options: { offline: boolean; workspaceMemberNames?: ReadonlySet; parentGraph?: LockParentGraph | null }, ): Promise { if (options.offline) return null; if (finding.relationship !== "transitive") return null; @@ -17,56 +18,70 @@ export async function resolveChainFix( const fixedVersion = finding.validatedFirstFixedVersion ?? finding.firstFixedVersion; if (!fixedVersion) return null; - const uniqueDirectDeps = [...new Set( - finding.dependencyPaths - .filter(path => path.length >= 3) - .map(path => path[1]), - )]; - - for (const directDepName of uniqueDirectDeps) { - const directDepCurrentVersion = installedVersions.get(directDepName); - if (!directDepCurrentVersion) continue; - - const packument = await fetchPackument(directDepName); - if (!packument?.versions) continue; - - const candidateVersions = Object.keys(packument.versions) - .filter(looksLikeVersion) - .filter(v => !isPreReleaseVersion(v)) - .filter(v => compareVersions(v, directDepCurrentVersion) > 0) - .sort(compareVersions); - - const pathsForDep = finding.dependencyPaths.filter(p => p[1] === directDepName); - - for (const candidateVersion of candidateVersions) { - const candidateDeps = packument.versions[candidateVersion]?.dependencies ?? {}; - // hops = everything after the direct dep and before (and including) the vulnerable package - // For path ["root", "express", "send", "mime-types"]: - // directDep = path[1] = "express" - // hops = path.slice(2) = ["send", "mime-types"] - const hops = pathsForDep[0].slice(2); - - const traced = await traceChain( - candidateDeps, - hops, - finding.pkg.version, - fixedVersion, - ); - if (!traced) continue; - - return { - directDep: directDepName, - directDepCurrentVersion, - targetVersion: candidateVersion, - chain: traced.chain, - safeVersion: traced.safeVersion, - command: `${commandPrefix(packageManager)} ${directDepName}@${candidateVersion}`, - coveredPaths: 1, - totalPaths: finding.dependencyPaths.length, - }; + for (const affectedPath of finding.dependencyPaths) { + const directIndices = options.workspaceMemberNames?.has(affectedPath[1]) ? [1, 2] : [1]; + for (const directIndex of directIndices) { + if (affectedPath.length <= directIndex + 1) continue; + const directDepName = affectedPath[directIndex]; + const prefix = affectedPath.slice(0, directIndex + 1); + let candidates = packages.filter(pkg => pkg.name === directDepName + && pkg.paths?.some(path => path.length === prefix.length && path.every((part, i) => part === prefix[i]))); + // Follow real lockfile edges backwards from the vulnerable version. Display + // paths alone can be identical for several installed parent versions. + if (options.parentGraph) { + const graph = options.parentGraph; + let nodes = [...graph.nodeIdsFor(finding.pkg.name, finding.pkg.version)]; + for (const name of affectedPath.slice(directIndex, -1).reverse()) { + nodes = [...new Set(nodes.flatMap(id => [...graph.parentsFor(id)]))] + .filter(id => graph.getNode(id)?.name === name); + } + const versions = new Set(nodes.map(id => graph.getNode(id)?.version)); + candidates = candidates.filter(pkg => versions.has(pkg.version)); + } + const versions = [...new Set(candidates.map(pkg => pkg.version))]; + // No name-only fallback: missing or ambiguous path identity cannot safely + // select a version floor or a declaring workspace. + if (versions.length !== 1) continue; + const directDepCurrentVersion = versions[0]; + + const packument = await fetchPackument(directDepName); + if (!packument?.versions) continue; + + const candidateVersions = Object.keys(packument.versions) + .filter(looksLikeVersion) + .filter(v => !isPreReleaseVersion(v)) + .filter(v => compareVersions(v, directDepCurrentVersion) > 0) + .sort(compareVersions); + + for (const candidateVersion of candidateVersions) { + const candidateDeps = packument.versions[candidateVersion]?.dependencies ?? {}; + // hops = everything after the direct dep and before (and including) the vulnerable package + // For path ["root", "express", "send", "mime-types"]: + // directDep = path[1] = "express" + // hops = path.slice(2) = ["send", "mime-types"] + const hops = affectedPath.slice(directIndex + 1); + + const traced = await traceChain( + candidateDeps, + hops, + finding.pkg.version, + fixedVersion, + ); + if (!traced) continue; + + return { + directDep: directDepName, + directDepCurrentVersion, + targetVersion: candidateVersion, + chain: traced.chain, + safeVersion: traced.safeVersion, + command: `${commandPrefix(packageManager)} ${directDepName}@${candidateVersion}`, + coveredPaths: 1, + totalPaths: finding.dependencyPaths.length, + }; + } } } - return null; } diff --git a/src/scanner.ts b/src/scanner.ts index 703a97e75..505fd9b1b 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -815,7 +815,6 @@ export async function scanPackages( // Chain fix resolution: pre-compute validated chain for transitive findings const chainPm = packageManagerForSource(context?.scanSource); if (chainPm) { - const installedVersions = new Map(packages.map(p => [p.name, p.version])); await Promise.all( findings .filter(f => f.relationship === "transitive" && f.dependencyPaths?.length && !aliasRemediationFindings.has(f)) @@ -823,9 +822,9 @@ export async function scanPackages( try { finding.chainResolution = await resolveChainFix( finding, - installedVersions, + packages, chainPm, - { offline }, + { offline, workspaceMemberNames, parentGraph: lockParentGraph }, ); } catch (error) { chainResolutionFailureCount += 1; diff --git a/tests/transitive-chain-resolver.test.ts b/tests/transitive-chain-resolver.test.ts index e736f07e2..e7a4a94ca 100644 --- a/tests/transitive-chain-resolver.test.ts +++ b/tests/transitive-chain-resolver.test.ts @@ -1,5 +1,6 @@ import { jest } from "@jest/globals"; -import type { Finding } from "../src/types.js"; +import type { Finding, PackageRef } from "../src/types.js"; +import { buildSuggestedFixCommandPlan } from "../src/remediation/fix-commands.js"; import { resolveChainFix } from "../src/remediation/transitive-chain-resolver.js"; import { clearPackumentCache } from "../src/remediation/npm-registry.js"; @@ -20,6 +21,10 @@ function makeFinding(overrides: Partial = {}): Finding { } as Finding; } +function installed(entries: Array<[string, string]> = []): PackageRef[] { + return entries.map(([name, version]) => ({ name, version, ecosystem: "npm", paths: [["root", name]] })); +} + beforeEach(() => { fetchMock.mockReset(); clearPackumentCache(); @@ -29,7 +34,7 @@ describe("resolveChainFix", () => { it("returns null when offline", async () => { const result = await resolveChainFix( makeFinding(), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "npm", { offline: true }, ); @@ -40,7 +45,7 @@ describe("resolveChainFix", () => { it("returns null when finding is not transitive", async () => { const result = await resolveChainFix( makeFinding({ relationship: "direct" }), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "npm", { offline: false }, ); @@ -50,7 +55,7 @@ describe("resolveChainFix", () => { it("returns null when direct dep is not in installedVersions", async () => { const result = await resolveChainFix( makeFinding(), - new Map(), + installed(), "npm", { offline: false }, ); @@ -61,7 +66,7 @@ describe("resolveChainFix", () => { fetchMock.mockResolvedValue({ ok: false, status: 404, json: async () => ({}) }); const result = await resolveChainFix( makeFinding(), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "npm", { offline: false }, ); @@ -102,7 +107,7 @@ describe("resolveChainFix", () => { makeFinding({ dependencyPaths: [["root", "express", "mime-types"]], }), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "npm", { offline: false }, ); @@ -159,7 +164,7 @@ describe("resolveChainFix", () => { const result = await resolveChainFix( makeFinding(), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "npm", { offline: false }, ); @@ -213,7 +218,7 @@ describe("resolveChainFix", () => { const result = await resolveChainFix( makeFinding(), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "npm", { offline: false }, ); @@ -250,7 +255,7 @@ describe("resolveChainFix", () => { const result = await resolveChainFix( makeFinding({ dependencyPaths: [["root", "express", "mime-types"]] }), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "pnpm", { offline: false }, ); @@ -296,7 +301,7 @@ describe("resolveChainFix", () => { firstFixedVersion: "2.0.0", validatedFirstFixedVersion: "2.0.0", }), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "npm", { offline: false }, ); @@ -344,7 +349,7 @@ describe("resolveChainFix", () => { firstFixedVersion: "2.0.0", validatedFirstFixedVersion: "2.0.0", }), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "npm", { offline: false }, ); @@ -403,7 +408,7 @@ describe("resolveChainFix", () => { firstFixedVersion: "2.0.0", validatedFirstFixedVersion: "2.0.0", }), - new Map([["express", "4.17.0"]]), + installed([["express", "4.17.0"]]), "npm", { offline: false }, ); @@ -416,3 +421,51 @@ describe("resolveChainFix", () => { expect(result!.command).toBe("npm install express@4.19.0"); }); }); + +it.each([false, true].flatMap(member => [false, true].map(reverse => ({ member, reverse })))) +("uses the affected parent's version and workspace (member: $member, reverse: $reverse)", async ({ member, reverse }) => { + fetchMock.mockImplementation(async (url: unknown) => ({ + ok: true, + json: async () => ({ versions: String(url).includes("/parent") ? { + "1.0.0": { dependencies: { diff: "1.0.0" } }, + "2.0.0": { dependencies: { diff: "2.0.0" } }, + "3.0.0": { dependencies: { diff: "2.0.0" } }, + } : { "1.0.0": {}, "2.0.0": {} } }), + })); + const prefix = member ? ["project", "web", "parent"] : ["project", "parent"]; + const packages: PackageRef[] = [ + { name: "parent", version: "1.0.0", ecosystem: "npm", paths: [prefix], yarnDirectWorkspaces: [member ? "web" : "."] }, + { name: "parent", version: "3.0.0", ecosystem: "npm", paths: [member ? ["project", "parent"] : ["project", "web", "parent"]], yarnDirectWorkspaces: [member ? "." : "web"] }, + ]; + if (reverse) packages.reverse(); + const finding = makeFinding({ pkg: { name: "diff", version: "1.0.0", ecosystem: "npm" }, dependencyPaths: [[...prefix, "diff"]] }); + const result = await resolveChainFix(finding, packages, "yarn", { offline: false, workspaceMemberNames: new Set(["web"]) }); + expect(result?.directDepCurrentVersion).toBe("1.0.0"); + expect(result?.targetVersion).toBe("2.0.0"); + finding.chainResolution = result; + const plan = buildSuggestedFixCommandPlan([finding], { source: "yarn-lock", mode: "resolved-lockfile", filePath: null, packages, notes: [], warnings: [], skippedDependencies: [] }); + expect(plan?.command).toBe(member ? "yarn workspace web add parent@2.0.0" : "yarn add -W parent@2.0.0"); +}); + +it("does not guess when two parent versions have the same display path", async () => { + const finding = makeFinding({ dependencyPaths: [["root", "express", "mime-types"]] }); + expect(await resolveChainFix(finding, installed([["express", "4.17.0"], ["express", "4.20.0"]]), "npm", { offline: false })).toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); +}); + +it("uses graph edges to disambiguate parent versions sharing a display path", async () => { + fetchMock.mockImplementation(async (url: unknown) => ({ ok: true, json: async () => ({ versions: String(url).includes("/express") + ? { "4.17.0": {}, "4.19.0": { dependencies: { "mime-types": "2.0.0" } }, "4.20.0": {} } + : { "2.0.0": {} } }) })); + const finding = makeFinding({ dependencyPaths: [["root", "express", "mime-types"]] }); + const result = await resolveChainFix(finding, installed([["express", "4.17.0"], ["express", "4.20.0"]]), "npm", { + offline: false, + parentGraph: { + nodeIdsFor: () => ["vulnerable"], + parentsFor: id => id === "vulnerable" ? ["affected-parent"] : [], + getNode: id => id === "affected-parent" ? { name: "express", version: "4.17.0" } : { name: "mime-types", version: "1.0.0" }, + }, + }); + expect(result?.directDepCurrentVersion).toBe("4.17.0"); + expect(result?.targetVersion).toBe("4.19.0"); +}); From 8316db5bcec0698754986118df8097910ada2e56 Mon Sep 17 00:00:00 2001 From: bhushan4work Date: Fri, 6 Nov 2026 19:10:02 +0530 Subject: [PATCH 06/14] fix(remediation): emit yarn up instead of yarn upgrade for Yarn Berry (#1537) --- src/remediation/fix-commands.ts | 20 ++-- tests/fixture-scan.test.ts | 37 +++++++ .../fix-commands-workspace-scoping.test.ts | 102 ++++++++++++++++++ 3 files changed, 153 insertions(+), 6 deletions(-) diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index 879acbda7..112ff90ab 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -1,3 +1,4 @@ +import fs from "node:fs"; import path from "node:path"; import { safeReadText } from "../utils/file.js"; import type { Finding, ScanInput, SeverityLabel, ChainHop } from "../types.js"; @@ -110,6 +111,7 @@ export function buildSuggestedFixCommandPlan( && isYarnBerry(safeReadText(scanInput.filePath) ?? ""); let workspaceMap: Map = new Map(); + let isBerry = false; if (scanInput.filePath) { try { if (packageManager === "npm" && isNpmLockSource(scanInput.source)) { @@ -118,6 +120,11 @@ export function buildSuggestedFixCommandPlan( workspaceMap = buildPnpmWorkspaceMap(scanInput.filePath); } else if (packageManager === "yarn" && scanInput.source === "yarn-lock") { workspaceMap = buildYarnWorkspaceMap(scanInput.filePath); + try { + isBerry = isYarnBerry(fs.readFileSync(scanInput.filePath, "utf8")); + } catch { + // best-effort + } } else if (packageManager === "bun" && scanInput.source === "bun-lock") { workspaceMap = buildBunWorkspaceMap(scanInput.filePath); } @@ -431,7 +438,7 @@ export function buildSuggestedFixCommandPlan( const lockfileRefreshCommand = packageManager === "pnpm" ? buildPnpmLockfileRefreshCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces) : packageManager === "yarn" - ? buildYarnUpgradeCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces) + ? buildYarnUpgradeCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces, isBerry) : buildBunUpdateCommand(finding.pkg.name); upsertTarget(targetsByKey, finding, parentUpdateTarget(finding, urgent, { package: finding.pkg.name, @@ -519,7 +526,7 @@ export function buildSuggestedFixCommandPlan( } else if (packageManager === "pnpm") { target.command = buildPnpmLockfileRefreshCommand(updatePkg, target.workspaces); } else if (packageManager === "yarn") { - target.command = buildYarnUpgradeCommand(updatePkg, target.workspaces); + target.command = buildYarnUpgradeCommand(updatePkg, target.workspaces, isBerry); } else if (packageManager === "bun") { target.command = buildBunUpdateCommand(updatePkg); } @@ -608,11 +615,12 @@ function buildPnpmLockfileRefreshCommand(pkg: string, workspaces?: string[]): st return `pnpm --filter ${shellQuote("./" + workspace)} update --no-save ${pkgQ}`; } -function buildYarnUpgradeCommand(pkg: string, workspaces?: string[]): string { +function buildYarnUpgradeCommand(pkg: string, workspaces?: string[], isBerry = false): string { const pkgQ = shellQuote(pkg); - const nonRoot = (workspaces ?? []).filter(w => w !== "."); - if (nonRoot.length === 0) return `yarn upgrade ${pkgQ}`; - return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} upgrade ${pkgQ}`).join(" && "); + const subCmd = isBerry ? "up" : "upgrade"; + const nonRoot = [...new Set((workspaces ?? []).filter(w => w !== "."))]; + if (nonRoot.length === 0) return `yarn ${subCmd} ${pkgQ}`; + return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} ${subCmd} ${pkgQ}`).join(" && "); } function buildBunUpdateCommand(pkg: string): string { diff --git a/tests/fixture-scan.test.ts b/tests/fixture-scan.test.ts index d5510e456..a5f3529ca 100644 --- a/tests/fixture-scan.test.ts +++ b/tests/fixture-scan.test.ts @@ -121,6 +121,43 @@ describe("fixture remediation scans", () => { }, ); + itWithFixture( + "yarn-berry", + "suggests yarn up for yarn-berry within-range transitive instead of yarn upgrade", + () => { + const scanInput = loadFixture("yarn-berry"); + const finding = findingFor(scanInput, "follow-redirects", { + relationship: "transitive", + firstFixedVersion: "1.15.0", + dependencyPaths: [["project", "axios", "follow-redirects"]], + recommendedNpmTransitiveRemediation: { + kind: "update-parent-within-range", + package: "axios", + currentVersion: "0.21.1", + targetChildVersion: "1.15.0", + viaPath: ["project", "axios", "follow-redirects"], + reason: "follow-redirects can be refreshed within axios's declared range.", + }, + }); + + const plan = buildSuggestedFixCommandPlan([finding], scanInput); + + expect(plan?.packageManager).toBe("yarn"); + expect(plan?.command).toBe("yarn up follow-redirects"); + expect(plan?.targets).toEqual([ + expect.objectContaining({ + package: "follow-redirects", + kind: "parent-update", + }), + ]); + expect(plan?.targets).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ package: "axios", kind: "parent-upgrade" }), + ]), + ); + }, + ); + describe("exact-pinned-intermediate", () => { let originalFetch: typeof fetch; const fetchMock = jest.fn(); diff --git a/tests/remediation/fix-commands-workspace-scoping.test.ts b/tests/remediation/fix-commands-workspace-scoping.test.ts index 2aa48fc17..6dbc2194b 100644 --- a/tests/remediation/fix-commands-workspace-scoping.test.ts +++ b/tests/remediation/fix-commands-workspace-scoping.test.ts @@ -86,6 +86,108 @@ describe("buildSuggestedFixCommandPlan - pnpm workspace command scoping", () => }); }); +function yarnScanInput(filePath: string): ScanInput { + return { + mode: "resolved-lockfile", + source: "yarn-lock", + filePath, + packages: [], + notes: [], + warnings: [], + skippedDependencies: [], + }; +} + +function writeYarnLock(content: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-ws-")); + const file = path.join(dir, "yarn.lock"); + fs.writeFileSync(file, content); + return file; +} + +function yarnWithinRangeRefreshFinding(workspaces?: string[]): Finding { + return { + pkg: { name: "js-cookie", version: "3.0.1", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-qjx8-664m-686j" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["project", "@aws-amplify/core", "js-cookie"]], + relationship: "transitive", + firstFixedVersion: "3.0.5", + recommendedNpmTransitiveRemediation: { + kind: "update-parent-within-range", + package: "js-cookie", + currentVersion: "3.0.1", + targetChildVersion: "3.0.5", + viaPath: ["project", "@aws-amplify/core", "js-cookie"], + reason: "@aws-amplify/core@6.16.1 already allows js-cookie@3.0.5 within the current dependency range", + workspaces, + }, + }; +} + +describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { + it("keeps yarn upgrade unchanged on Yarn Classic (v1) without workspaces", () => { + const lock = writeYarnLock("# yarn lockfile v1\n"); + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + const target = findFixTargetForFinding(plan!, finding); + expect(target?.command).toBe("yarn upgrade js-cookie"); + expect(plan?.command).toBe("yarn upgrade js-cookie"); + }); + + it("keeps yarn upgrade unchanged on Yarn Classic (v1) even with workspace members", () => { + const lock = writeYarnLock("# yarn lockfile v1\n"); + const finding = yarnWithinRangeRefreshFinding(["client"]); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + const target = findFixTargetForFinding(plan!, finding); + expect(target?.command).toBe("yarn workspace client upgrade js-cookie"); + expect(plan?.command).toBe("yarn workspace client upgrade js-cookie"); + }); + + it("emits yarn up on Yarn Berry (v2+) when there is no workspace", () => { + const lock = writeYarnLock("__metadata:\n version: 8\n cacheKey: 10c0\n"); + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + const target = findFixTargetForFinding(plan!, finding); + expect(target?.command).toBe("yarn up js-cookie"); + expect(plan?.command).toBe("yarn up js-cookie"); + }); + + it("emits yarn up on Yarn Berry (v2+) when the package is at the workspace root", () => { + const lock = writeYarnLock("__metadata:\n version: 8\n cacheKey: 10c0\n"); + const finding = yarnWithinRangeRefreshFinding(["."]); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + const target = findFixTargetForFinding(plan!, finding); + expect(target?.command).toBe("yarn up js-cookie"); + expect(plan?.command).toBe("yarn up js-cookie"); + }); + + it("scopes a within-range refresh to a single member with yarn workspace up on Yarn Berry", () => { + const lock = writeYarnLock("__metadata:\n version: 8\n cacheKey: 10c0\n"); + const finding = yarnWithinRangeRefreshFinding(["client"]); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + const target = findFixTargetForFinding(plan!, finding); + expect(target?.command).toBe("yarn workspace client up js-cookie"); + expect(plan?.command).toBe("yarn workspace client up js-cookie"); + }); + + it("scopes within-range refresh to multiple members with chained yarn workspace commands on Yarn Berry", () => { + const lock = writeYarnLock("__metadata:\n version: 8\n cacheKey: 10c0\n"); + const finding = yarnWithinRangeRefreshFinding(["client", "admin"]); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + const target = findFixTargetForFinding(plan!, finding); + expect(target?.command).toBe("yarn workspace client up js-cookie && yarn workspace admin up js-cookie"); + expect(plan?.command).toBe("yarn workspace client up js-cookie && yarn workspace admin up js-cookie"); + }); +}); + // A transitive finding whose fix is a breaking parent upgrade (install), where // the parent package is a direct dependency of a workspace member. function pnpmParentUpgradeFinding(parent: string, currentVersion: string, targetVersion: string): Finding { From c9d68606949eec180dcda609936abc1b921d0f55 Mon Sep 17 00:00:00 2001 From: bhushan4work Date: Fri, 6 Nov 2026 22:05:23 +0530 Subject: [PATCH 07/14] fix(remediation): address CodeRabbit review findings for Yarn Berry fixes (#1537) --- src/parsers/yarn-lock.ts | 89 ++++++++++ src/remediation/fix-commands.ts | 54 +++++- src/utils/fix-runner.ts | 30 +++- tests/fix-runner-apply.test.ts | 158 +++++++++++++++++- tests/fixture-scan.test.ts | 6 +- .../fix-commands-workspace-scoping.test.ts | 115 +++++++++++-- 6 files changed, 426 insertions(+), 26 deletions(-) diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index 8092c8166..0d87ea16e 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import semver from "semver"; import { parse as parseYarnLock } from "yarn-lockfile"; import type { PackageRef } from "../types.js"; import { looksLikeVersion, normalizeRawVersion } from "../utils/version.js"; @@ -141,6 +142,94 @@ export function isYarnBerry(content: string): boolean { return content.startsWith('__metadata:') || content.includes('\n__metadata:'); } +export type YarnBerryInfo = { + isBerry: boolean; + supportsRecursiveUp: boolean; +}; + +export function detectYarnBerryInfo( + filePath: string | null | undefined, + lockContent: string, +): YarnBerryInfo { + if (!isYarnBerry(lockContent)) { + return { isBerry: false, supportsRecursiveUp: false }; + } + + if (filePath) { + const projectDir = path.dirname(filePath); + try { + const pkgJsonPath = path.join(projectDir, "package.json"); + if (fs.existsSync(pkgJsonPath)) { + const pkgJson = JSON.parse(fs.readFileSync(pkgJsonPath, "utf8")); + if (typeof pkgJson.packageManager === "string") { + const match = pkgJson.packageManager.match(/^yarn@(\d+)\.(\d+)(?:\.(\d+))?/); + if (match) { + const major = parseInt(match[1], 10); + const minor = parseInt(match[2], 10); + if (major > 3 || (major === 3 && minor >= 1)) { + return { isBerry: true, supportsRecursiveUp: true }; + } + if (major === 2 || (major === 3 && minor === 0)) { + return { isBerry: true, supportsRecursiveUp: false }; + } + } + } + if (typeof pkgJson.engines?.yarn === "string") { + const coerced = semver.coerce(pkgJson.engines.yarn); + if (coerced) { + if (semver.gte(coerced, "3.1.0")) { + return { isBerry: true, supportsRecursiveUp: true }; + } + if (semver.gte(coerced, "2.0.0") && semver.lt(coerced, "3.1.0")) { + return { isBerry: true, supportsRecursiveUp: false }; + } + } + } + } + } catch { + // best-effort + } + + for (const rcName of [".yarnrc.yml", ".yarnrc.yaml"]) { + try { + const rcPath = path.join(projectDir, rcName); + if (fs.existsSync(rcPath)) { + const rcContent = fs.readFileSync(rcPath, "utf8"); + const match = rcContent.match(/yarn-(\d+)\.(\d+)(?:\.(\d+))?/); + if (match) { + const major = parseInt(match[1], 10); + const minor = parseInt(match[2], 10); + if (major > 3 || (major === 3 && minor >= 1)) { + return { isBerry: true, supportsRecursiveUp: true }; + } + if (major === 2 || (major === 3 && minor === 0)) { + return { isBerry: true, supportsRecursiveUp: false }; + } + } + } + } catch { + // best-effort + } + } + } + + // Fallback to lockfile metadata version: + // __metadata: + // version: 8 + const metaMatch = lockContent.match(/__metadata:[\s\S]*?version:\s*["']?(\d+)["']?/); + if (metaMatch) { + const lockVersion = parseInt(metaMatch[1], 10); + if (lockVersion <= 5) { + return { isBerry: true, supportsRecursiveUp: false }; + } + if (lockVersion >= 7) { + return { isBerry: true, supportsRecursiveUp: true }; + } + } + + return { isBerry: true, supportsRecursiveUp: true }; +} + function pathsEqual(left: string[], right: string[]): boolean { if (left.length !== right.length) return false; return left.every((value, index) => value === right[index]); diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index 112ff90ab..2350c2db5 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -10,7 +10,7 @@ import { calculatePathCoverage, formatDependencyPath } from "../utils/path-cover import { pluralize } from "../utils/string.js"; import { buildNpmWorkspaceMap } from "../parsers/package-lock.js"; import { buildPnpmWorkspaceMap } from "../parsers/pnpm-lock.js"; -import { buildYarnWorkspaceMap, isYarnBerry } from "../parsers/yarn-lock.js"; +import { buildYarnWorkspaceMap, isYarnBerry, detectYarnBerryInfo } 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"; @@ -66,8 +66,12 @@ export type SuggestedFixTarget = { chainProof?: ChainHop[]; chainSafeVersion?: string; chainVulnerablePackage?: string; + yarnVariant?: YarnVariant; + yarnBerryRecursiveUp?: boolean; }; +export type YarnVariant = "classic" | "berry"; + export type SuggestedFixSkip = { package: string; version: string; @@ -83,6 +87,8 @@ export type SuggestedFixSkip = { export type SuggestedFixCommandPlan = { packageManager: SuggestedFixPackageManager; + yarnVariant?: YarnVariant; + yarnBerryRecursiveUp?: boolean; sourceLabel: string; command: string | null; sections: Array<{ @@ -112,6 +118,7 @@ export function buildSuggestedFixCommandPlan( let workspaceMap: Map = new Map(); let isBerry = false; + let yarnBerryRecursiveUp = true; if (scanInput.filePath) { try { if (packageManager === "npm" && isNpmLockSource(scanInput.source)) { @@ -119,12 +126,15 @@ export function buildSuggestedFixCommandPlan( } else if (packageManager === "pnpm" && scanInput.source === "pnpm-lock") { workspaceMap = buildPnpmWorkspaceMap(scanInput.filePath); } else if (packageManager === "yarn" && scanInput.source === "yarn-lock") { - workspaceMap = buildYarnWorkspaceMap(scanInput.filePath); try { - isBerry = isYarnBerry(fs.readFileSync(scanInput.filePath, "utf8")); + const lockContent = fs.readFileSync(scanInput.filePath, "utf8"); + const berryInfo = detectYarnBerryInfo(scanInput.filePath, lockContent); + isBerry = berryInfo.isBerry; + yarnBerryRecursiveUp = berryInfo.supportsRecursiveUp; } catch { // best-effort } + workspaceMap = buildYarnWorkspaceMap(scanInput.filePath); } else if (packageManager === "bun" && scanInput.source === "bun-lock") { workspaceMap = buildBunWorkspaceMap(scanInput.filePath); } @@ -435,10 +445,19 @@ export function buildSuggestedFixCommandPlan( finding.relationship === "transitive" && finding.recommendedNpmTransitiveRemediation?.kind === "update-parent-within-range" ) { + if (packageManager === "yarn" && isBerry && !yarnBerryRecursiveUp) { + skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { + package: finding.pkg.name, + version: finding.pkg.version, + relationship: finding.relationship, + reason: `Yarn Berry prior to 3.1 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; upgrade Yarn to 3.1+ or update the parent dependency.`, + }); + continue; + } const lockfileRefreshCommand = packageManager === "pnpm" ? buildPnpmLockfileRefreshCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces) : packageManager === "yarn" - ? buildYarnUpgradeCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces, isBerry) + ? buildYarnUpgradeCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces, isBerry, yarnBerryRecursiveUp) : buildBunUpdateCommand(finding.pkg.name); upsertTarget(targetsByKey, finding, parentUpdateTarget(finding, urgent, { package: finding.pkg.name, @@ -447,6 +466,8 @@ export function buildSuggestedFixCommandPlan( reason: `${finding.pkg.name}@${finding.pkg.version} can be refreshed to ${finding.recommendedNpmTransitiveRemediation.targetChildVersion}+; no parent upgrade needed.`, command: lockfileRefreshCommand, workspaces: finding.recommendedNpmTransitiveRemediation?.workspaces, + yarnVariant: packageManager === "yarn" ? (isBerry ? "berry" : "classic") : undefined, + yarnBerryRecursiveUp: packageManager === "yarn" && isBerry ? yarnBerryRecursiveUp : undefined, })); continue; } @@ -526,7 +547,9 @@ export function buildSuggestedFixCommandPlan( } else if (packageManager === "pnpm") { target.command = buildPnpmLockfileRefreshCommand(updatePkg, target.workspaces); } else if (packageManager === "yarn") { - target.command = buildYarnUpgradeCommand(updatePkg, target.workspaces, isBerry); + target.command = buildYarnUpgradeCommand(updatePkg, target.workspaces, isBerry, yarnBerryRecursiveUp); + target.yarnVariant = isBerry ? "berry" : "classic"; + target.yarnBerryRecursiveUp = isBerry ? yarnBerryRecursiveUp : undefined; } else if (packageManager === "bun") { target.command = buildBunUpdateCommand(updatePkg); } @@ -557,6 +580,8 @@ export function buildSuggestedFixCommandPlan( const plan: SuggestedFixCommandPlan = { packageManager, + yarnVariant: packageManager === "yarn" ? (isBerry ? "berry" : "classic") : undefined, + yarnBerryRecursiveUp: packageManager === "yarn" && isBerry ? yarnBerryRecursiveUp : undefined, sourceLabel: packageManagerSourceLabel(scanInput), command, sections, @@ -615,10 +640,19 @@ function buildPnpmLockfileRefreshCommand(pkg: string, workspaces?: string[]): st return `pnpm --filter ${shellQuote("./" + workspace)} update --no-save ${pkgQ}`; } -function buildYarnUpgradeCommand(pkg: string, workspaces?: string[], isBerry = false): string { +function buildYarnUpgradeCommand( + pkg: string, + workspaces?: string[], + isBerry = false, + yarnBerryRecursiveUp = true, +): string { const pkgQ = shellQuote(pkg); - const subCmd = isBerry ? "up" : "upgrade"; const nonRoot = [...new Set((workspaces ?? []).filter(w => w !== "."))]; + if (!isBerry) { + if (nonRoot.length === 0) return `yarn upgrade ${pkgQ}`; + return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} upgrade ${pkgQ}`).join(" && "); + } + const subCmd = yarnBerryRecursiveUp ? "up -R" : "up"; if (nonRoot.length === 0) return `yarn ${subCmd} ${pkgQ}`; return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} ${subCmd} ${pkgQ}`).join(" && "); } @@ -803,6 +837,8 @@ function parentUpdateTarget( command: string; isDev?: boolean; workspaces?: string[]; + yarnVariant?: YarnVariant; + yarnBerryRecursiveUp?: boolean; }, ): SuggestedFixTarget { const namesParent = args.package !== finding.pkg.name; @@ -828,6 +864,8 @@ function parentUpdateTarget( usage: finding.usage ?? null, isDev: args.isDev, workspaces: workspaces.length > 0 ? workspaces : undefined, + yarnVariant: args.yarnVariant, + yarnBerryRecursiveUp: args.yarnBerryRecursiveUp, }; } @@ -884,6 +922,8 @@ function upsertTarget( chainProof: existing.chainProof ?? next.chainProof, chainSafeVersion: existing.chainSafeVersion ?? next.chainSafeVersion, chainVulnerablePackage: existing.chainVulnerablePackage ?? next.chainVulnerablePackage, + yarnVariant: existing.yarnVariant ?? next.yarnVariant, + yarnBerryRecursiveUp: existing.yarnBerryRecursiveUp ?? next.yarnBerryRecursiveUp, childPackage: existing.childPackage ?? next.childPackage, childTargetVersion: higherVersion(existing.childTargetVersion, next.childTargetVersion) ?? existing.childTargetVersion diff --git a/src/utils/fix-runner.ts b/src/utils/fix-runner.ts index 7b9daad55..e009bf3dc 100644 --- a/src/utils/fix-runner.ts +++ b/src/utils/fix-runner.ts @@ -9,7 +9,7 @@ import { pluralize } from "./string.js"; import { isBreakingUpgrade } from "./version.js"; import { installPackageSpec } from "./package-identity.js"; import { formatParentUpdateChildNote } from "../remediation/parent-update-child-note.js"; -import type { SuggestedFixCommandPlan, SuggestedFixTarget } from "../remediation/fix-commands.js"; +import type { SuggestedFixCommandPlan, SuggestedFixTarget, YarnVariant } from "../remediation/fix-commands.js"; import type { DebugLogger } from "../output/debug.js"; const OUTPUT_LIMIT = 8 * 1024; @@ -173,12 +173,16 @@ const EMPTY_UNAPPLIED: UnappliedBreakdown = { // 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, + options?: { + yarnVariant?: YarnVariant; + yarnBerryRecursiveUp?: boolean; + }, ): string[][] { if (target.kind === "parent-update") { - return buildParentUpdateApplyArgv(packageManager, target); + return buildParentUpdateApplyArgv(packageManager, target, options); } return buildInstallApplyArgv(packageManager, target); } @@ -237,9 +241,13 @@ function buildInstallApplyArgv( return [["bun", ...args]]; } -function buildParentUpdateApplyArgv( +export function buildParentUpdateApplyArgv( packageManager: SuggestedFixCommandPlan["packageManager"], target: SuggestedFixTarget, + options?: { + yarnVariant?: YarnVariant; + yarnBerryRecursiveUp?: boolean; + }, ): string[][] { // 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. @@ -270,6 +278,15 @@ function buildParentUpdateApplyArgv( } if (packageManager === "yarn") { + const isBerry = (options?.yarnVariant ?? target.yarnVariant) === "berry"; + const recursiveUp = (options?.yarnBerryRecursiveUp ?? target.yarnBerryRecursiveUp) !== false; + if (isBerry) { + const upArgs = recursiveUp ? ["up", "-R", updatePkg] : ["up", updatePkg]; + if (nonRoot.length === 0) { + return [["yarn", ...upArgs]]; + } + return nonRoot.map(ws => ["yarn", "workspace", ws, ...upArgs]); + } if (nonRoot.length === 0) { return [["yarn", "upgrade", updatePkg]]; } @@ -356,7 +373,10 @@ export async function applyFixesIfRequested(params: { const seenCommands = new Set(); for (let i = 0; i < applyTargets.length; i++) { const target = applyTargets[i]; - for (const commandParts of buildApplyCommands(params.plan.packageManager, target)) { + for (const commandParts of buildApplyCommands(params.plan.packageManager, target, { + yarnVariant: params.plan.yarnVariant, + yarnBerryRecursiveUp: params.plan.yarnBerryRecursiveUp, + })) { const commandLine = commandParts.join(" "); const retryHint = `\n${formatRetryHint(commandParts, params.projectPath)}\nCheck that \`${params.plan.packageManager}\` is available and you have write access to node_modules.\nIf running in CI, ensure the install step has already run before cve-lite --fix.`; if (seenCommands.has(commandLine)) continue; diff --git a/tests/fix-runner-apply.test.ts b/tests/fix-runner-apply.test.ts index 75e5d9de9..65dc336d3 100644 --- a/tests/fix-runner-apply.test.ts +++ b/tests/fix-runner-apply.test.ts @@ -23,7 +23,10 @@ function target(overrides: Partial): SuggestedFixTarget { }; } -function plan(targets: SuggestedFixTarget[]): SuggestedFixCommandPlan { +function plan( + targets: SuggestedFixTarget[], + overrides?: Partial, +): SuggestedFixCommandPlan { return { packageManager: "npm", sourceLabel: "package-lock.json", @@ -33,6 +36,7 @@ function plan(targets: SuggestedFixTarget[]): SuggestedFixCommandPlan { skipped: [], coveredFindingCount: 0, totalFindingCount: 0, + ...overrides, }; } @@ -442,6 +446,158 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { expect(argvs.map(a => a[0])).toEqual(["npm"]); }); + describe("Yarn variant --fix execution", () => { + it("executes yarn up -R for a root within-range refresh on Yarn Berry (v3.1+)", async () => { + const { calls, argvs, run } = recordingRunner(); + await applyFixesIfRequested({ + plan: plan( + [ + target({ + kind: "parent-update", + package: "follow-redirects", + currentVersion: "1.14.0", + targetVersion: "1.16.0", + command: "yarn up -R follow-redirects", + }), + ], + { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: true }, + ), + projectPath: "/tmp/proj", + totalFindings: 1, + options, + runCommand: run, + }); + + expect(calls).toEqual(["yarn up -R follow-redirects"]); + expect(argvs).toEqual([["yarn", "up", "-R", "follow-redirects"]]); + }); + + it("executes yarn workspace up -R for a workspace within-range refresh on Yarn Berry (v3.1+)", async () => { + const { calls, argvs, run } = recordingRunner(); + await applyFixesIfRequested({ + plan: plan( + [ + target({ + kind: "parent-update", + package: "follow-redirects", + currentVersion: "1.14.0", + targetVersion: "1.16.0", + workspaces: ["packages/client"], + command: "yarn workspace packages/client up -R follow-redirects", + }), + ], + { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: true }, + ), + projectPath: "/tmp/proj", + totalFindings: 1, + options, + runCommand: run, + }); + + expect(calls).toEqual(["yarn workspace packages/client up -R follow-redirects"]); + expect(argvs).toEqual([["yarn", "workspace", "packages/client", "up", "-R", "follow-redirects"]]); + }); + + it("preserves yarn upgrade for a root within-range refresh on Yarn Classic", async () => { + const { calls, argvs, run } = recordingRunner(); + await applyFixesIfRequested({ + plan: plan( + [ + target({ + kind: "parent-update", + package: "follow-redirects", + currentVersion: "1.14.0", + targetVersion: "1.16.0", + command: "yarn upgrade follow-redirects", + }), + ], + { packageManager: "yarn", yarnVariant: "classic" }, + ), + projectPath: "/tmp/proj", + totalFindings: 1, + options, + runCommand: run, + }); + + expect(calls).toEqual(["yarn upgrade follow-redirects"]); + expect(argvs).toEqual([["yarn", "upgrade", "follow-redirects"]]); + }); + + it("preserves yarn workspace upgrade for a workspace within-range refresh on Yarn Classic", async () => { + const { calls, argvs, run } = recordingRunner(); + await applyFixesIfRequested({ + plan: plan( + [ + target({ + kind: "parent-update", + package: "follow-redirects", + currentVersion: "1.14.0", + targetVersion: "1.16.0", + workspaces: ["packages/client"], + command: "yarn workspace packages/client upgrade follow-redirects", + }), + ], + { packageManager: "yarn", yarnVariant: "classic" }, + ), + projectPath: "/tmp/proj", + totalFindings: 1, + options, + runCommand: run, + }); + + expect(calls).toEqual(["yarn workspace packages/client upgrade follow-redirects"]); + expect(argvs).toEqual([["yarn", "workspace", "packages/client", "upgrade", "follow-redirects"]]); + }); + + it("executes yarn up without -R for earlier Berry when recursiveUp is false", async () => { + const { calls, argvs, run } = recordingRunner(); + await applyFixesIfRequested({ + plan: plan( + [ + target({ + kind: "parent-update", + package: "follow-redirects", + currentVersion: "1.14.0", + targetVersion: "1.16.0", + command: "yarn up follow-redirects", + }), + ], + { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: false }, + ), + projectPath: "/tmp/proj", + totalFindings: 1, + options, + runCommand: run, + }); + + expect(calls).toEqual(["yarn up follow-redirects"]); + expect(argvs).toEqual([["yarn", "up", "follow-redirects"]]); + }); + + it("executes yarn add for direct dependencies on Yarn Berry", async () => { + const { calls, run } = recordingRunner(); + await applyFixesIfRequested({ + plan: plan( + [ + target({ + kind: "direct", + package: "axios", + currentVersion: "0.21.1", + targetVersion: "0.33.0", + workspaces: ["packages/api"], + }), + ], + { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: true }, + ), + projectPath: "/tmp/proj", + totalFindings: 1, + options, + runCommand: run, + }); + + expect(calls).toEqual(["yarn workspace packages/api add axios@0.33.0"]); + }); + }); }); it.each([false, true])("applies Yarn fixes to root and member with matching dependency flags: %s", async (isDev) => { diff --git a/tests/fixture-scan.test.ts b/tests/fixture-scan.test.ts index a5f3529ca..a76070cd2 100644 --- a/tests/fixture-scan.test.ts +++ b/tests/fixture-scan.test.ts @@ -123,7 +123,7 @@ describe("fixture remediation scans", () => { itWithFixture( "yarn-berry", - "suggests yarn up for yarn-berry within-range transitive instead of yarn upgrade", + "suggests yarn up -R for yarn-berry within-range transitive instead of yarn upgrade", () => { const scanInput = loadFixture("yarn-berry"); const finding = findingFor(scanInput, "follow-redirects", { @@ -143,7 +143,9 @@ describe("fixture remediation scans", () => { const plan = buildSuggestedFixCommandPlan([finding], scanInput); expect(plan?.packageManager).toBe("yarn"); - expect(plan?.command).toBe("yarn up follow-redirects"); + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(true); + expect(plan?.command).toBe("yarn up -R follow-redirects"); expect(plan?.targets).toEqual([ expect.objectContaining({ package: "follow-redirects", diff --git a/tests/remediation/fix-commands-workspace-scoping.test.ts b/tests/remediation/fix-commands-workspace-scoping.test.ts index 6dbc2194b..8a0db9279 100644 --- a/tests/remediation/fix-commands-workspace-scoping.test.ts +++ b/tests/remediation/fix-commands-workspace-scoping.test.ts @@ -147,34 +147,36 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.command).toBe("yarn workspace client upgrade js-cookie"); }); - it("emits yarn up on Yarn Berry (v2+) when there is no workspace", () => { + it("emits yarn up -R on Yarn Berry (v3.1+) when there is no workspace", () => { const lock = writeYarnLock("__metadata:\n version: 8\n cacheKey: 10c0\n"); const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); const target = findFixTargetForFinding(plan!, finding); - expect(target?.command).toBe("yarn up js-cookie"); - expect(plan?.command).toBe("yarn up js-cookie"); + expect(target?.command).toBe("yarn up -R js-cookie"); + expect(plan?.command).toBe("yarn up -R js-cookie"); + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(true); }); - it("emits yarn up on Yarn Berry (v2+) when the package is at the workspace root", () => { + it("emits yarn up -R on Yarn Berry (v3.1+) when the package is at the workspace root", () => { const lock = writeYarnLock("__metadata:\n version: 8\n cacheKey: 10c0\n"); const finding = yarnWithinRangeRefreshFinding(["."]); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); const target = findFixTargetForFinding(plan!, finding); - expect(target?.command).toBe("yarn up js-cookie"); - expect(plan?.command).toBe("yarn up js-cookie"); + expect(target?.command).toBe("yarn up -R js-cookie"); + expect(plan?.command).toBe("yarn up -R js-cookie"); }); - it("scopes a within-range refresh to a single member with yarn workspace up on Yarn Berry", () => { + it("scopes a within-range refresh to a single member with yarn workspace up -R on Yarn Berry", () => { const lock = writeYarnLock("__metadata:\n version: 8\n cacheKey: 10c0\n"); const finding = yarnWithinRangeRefreshFinding(["client"]); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); const target = findFixTargetForFinding(plan!, finding); - expect(target?.command).toBe("yarn workspace client up js-cookie"); - expect(plan?.command).toBe("yarn workspace client up js-cookie"); + expect(target?.command).toBe("yarn workspace client up -R js-cookie"); + expect(plan?.command).toBe("yarn workspace client up -R js-cookie"); }); it("scopes within-range refresh to multiple members with chained yarn workspace commands on Yarn Berry", () => { @@ -183,8 +185,99 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); const target = findFixTargetForFinding(plan!, finding); - expect(target?.command).toBe("yarn workspace client up js-cookie && yarn workspace admin up js-cookie"); - expect(plan?.command).toBe("yarn workspace client up js-cookie && yarn workspace admin up js-cookie"); + expect(target?.command).toBe("yarn workspace client up -R js-cookie && yarn workspace admin up -R js-cookie"); + expect(plan?.command).toBe("yarn workspace client up -R js-cookie && yarn workspace admin up -R js-cookie"); + }); + + it("detects Berry before building workspace map even if workspace map construction fails", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-berry-ws-err-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 8\n cacheKey: 10c0\n"); + // Invalid package.json with workspaces that causes workspace map parsing issues + const rootPkg = path.join(dir, "package.json"); + fs.writeFileSync(rootPkg, JSON.stringify({ workspaces: ["packages/*"] })); + const badPkgDir = path.join(dir, "packages", "bad"); + fs.mkdirSync(badPkgDir, { recursive: true }); + fs.writeFileSync(path.join(badPkgDir, "package.json"), "NOT_JSON"); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.packageManager).toBe("yarn"); + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(true); + expect(plan?.command).toBe("yarn up -R js-cookie"); + }); + + it("skips transitive within-range refresh on older Berry (< 3.1) with an explanatory note", () => { + // Lockfile metadata version 5 (Yarn 2) + const lock = writeYarnLock("__metadata:\n version: 5\n cacheKey: 10c0\n"); + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.packageManager).toBe("yarn"); + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(false); + expect(plan?.command).toBeNull(); + expect(plan?.targets).toHaveLength(0); + expect(plan?.skipped).toEqual([ + expect.objectContaining({ + package: "js-cookie", + relationship: "transitive", + reason: expect.stringContaining("Yarn Berry prior to 3.1 does not support recursive lockfile refresh"), + }), + ]); + }); + + it("skips transitive within-range refresh when packageManager specifies Yarn Berry < 3.1", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-v2-pkg-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 6\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@2.4.3" })); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(false); + expect(plan?.command).toBeNull(); + expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.1"); + }); + + it("supports recursive upgrade when packageManager specifies Yarn 3.1+", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-v31-pkg-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 6\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@3.1.1" })); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(true); + expect(plan?.command).toBe("yarn up -R js-cookie"); + }); + + it("still allows direct package upgrades on older Yarn Berry releases", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-v2-direct-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 5\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@2.4.3" })); + + const directFinding: Finding = { + pkg: { name: "lodash", version: "4.17.20", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-lodash" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["project", "lodash"]], + relationship: "direct", + firstFixedVersion: "4.17.21", + }; + + const plan = buildSuggestedFixCommandPlan([directFinding], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.command).toBe("yarn add lodash@4.17.21"); }); }); From 883827eb490d0b37f0f3835f04326b5a1748c793 Mon Sep 17 00:00:00 2001 From: bhushan4work Date: Fri, 6 Nov 2026 22:39:56 +0530 Subject: [PATCH 08/14] fix(remediation): refine Yarn Berry 3.0+ detection and project-level recursive fix execution (#1537) --- src/parsers/yarn-lock.ts | 66 ++++----- src/remediation/fix-commands.ts | 31 +++-- src/utils/fix-runner.ts | 14 +- tests/fix-runner-apply.test.ts | 42 +++++- .../fix-commands-workspace-scoping.test.ts | 127 +++++++++++++++--- 5 files changed, 209 insertions(+), 71 deletions(-) diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index 0d87ea16e..6b223bcdc 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -1,6 +1,5 @@ import fs from "node:fs"; import path from "node:path"; -import semver from "semver"; import { parse as parseYarnLock } from "yarn-lockfile"; import type { PackageRef } from "../types.js"; import { looksLikeVersion, normalizeRawVersion } from "../utils/version.js"; @@ -144,9 +143,21 @@ export function isYarnBerry(content: string): boolean { export type YarnBerryInfo = { isBerry: boolean; - supportsRecursiveUp: boolean; + supportsRecursiveUp: boolean | "ambiguous"; }; +export function extractYarnPathFromYarnRc(content: string): string | null { + for (const line of content.split(/\r?\n/)) { + const trimmed = line.trim(); + if (trimmed.startsWith("#")) continue; + const match = trimmed.match(/^yarnPath:\s*["']?([^#"'\s]+)["']?/); + if (match) { + return match[1]; + } + } + return null; +} + export function detectYarnBerryInfo( filePath: string | null | undefined, lockContent: string, @@ -165,22 +176,10 @@ export function detectYarnBerryInfo( const match = pkgJson.packageManager.match(/^yarn@(\d+)\.(\d+)(?:\.(\d+))?/); if (match) { const major = parseInt(match[1], 10); - const minor = parseInt(match[2], 10); - if (major > 3 || (major === 3 && minor >= 1)) { + if (major >= 3) { return { isBerry: true, supportsRecursiveUp: true }; } - if (major === 2 || (major === 3 && minor === 0)) { - return { isBerry: true, supportsRecursiveUp: false }; - } - } - } - if (typeof pkgJson.engines?.yarn === "string") { - const coerced = semver.coerce(pkgJson.engines.yarn); - if (coerced) { - if (semver.gte(coerced, "3.1.0")) { - return { isBerry: true, supportsRecursiveUp: true }; - } - if (semver.gte(coerced, "2.0.0") && semver.lt(coerced, "3.1.0")) { + if (major === 2) { return { isBerry: true, supportsRecursiveUp: false }; } } @@ -195,15 +194,17 @@ export function detectYarnBerryInfo( const rcPath = path.join(projectDir, rcName); if (fs.existsSync(rcPath)) { const rcContent = fs.readFileSync(rcPath, "utf8"); - const match = rcContent.match(/yarn-(\d+)\.(\d+)(?:\.(\d+))?/); - if (match) { - const major = parseInt(match[1], 10); - const minor = parseInt(match[2], 10); - if (major > 3 || (major === 3 && minor >= 1)) { - return { isBerry: true, supportsRecursiveUp: true }; - } - if (major === 2 || (major === 3 && minor === 0)) { - return { isBerry: true, supportsRecursiveUp: false }; + const yarnPath = extractYarnPathFromYarnRc(rcContent); + if (yarnPath) { + const match = yarnPath.match(/yarn-(\d+)\.(\d+)(?:\.(\d+))?/); + if (match) { + const major = parseInt(match[1], 10); + if (major >= 3) { + return { isBerry: true, supportsRecursiveUp: true }; + } + if (major === 2) { + return { isBerry: true, supportsRecursiveUp: false }; + } } } } @@ -219,15 +220,20 @@ export function detectYarnBerryInfo( const metaMatch = lockContent.match(/__metadata:[\s\S]*?version:\s*["']?(\d+)["']?/); if (metaMatch) { const lockVersion = parseInt(metaMatch[1], 10); - if (lockVersion <= 5) { - return { isBerry: true, supportsRecursiveUp: false }; - } - if (lockVersion >= 7) { + // Version 5 and higher support recursive upgrades (yarn up -R) + if (lockVersion >= 5) { return { isBerry: true, supportsRecursiveUp: true }; } + // Version 4 is ambiguous: shared by Yarn 2.4.3 and Yarn 3.0.0 + if (lockVersion === 4) { + return { isBerry: true, supportsRecursiveUp: "ambiguous" }; + } + // Versions < 4 are early Berry v2 + return { isBerry: true, supportsRecursiveUp: false }; } - return { isBerry: true, supportsRecursiveUp: true }; + // Without metadata version or exact version configuration, capability is ambiguous + return { isBerry: true, supportsRecursiveUp: "ambiguous" }; } function pathsEqual(left: string[], right: string[]): boolean { diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index 2350c2db5..def2e632a 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -67,7 +67,7 @@ export type SuggestedFixTarget = { chainSafeVersion?: string; chainVulnerablePackage?: string; yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean; + yarnBerryRecursiveUp?: boolean | "ambiguous"; }; export type YarnVariant = "classic" | "berry"; @@ -88,7 +88,7 @@ export type SuggestedFixSkip = { export type SuggestedFixCommandPlan = { packageManager: SuggestedFixPackageManager; yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean; + yarnBerryRecursiveUp?: boolean | "ambiguous"; sourceLabel: string; command: string | null; sections: Array<{ @@ -118,7 +118,7 @@ export function buildSuggestedFixCommandPlan( let workspaceMap: Map = new Map(); let isBerry = false; - let yarnBerryRecursiveUp = true; + let yarnBerryRecursiveUp: boolean | "ambiguous" = true; if (scanInput.filePath) { try { if (packageManager === "npm" && isNpmLockSource(scanInput.source)) { @@ -445,12 +445,21 @@ export function buildSuggestedFixCommandPlan( finding.relationship === "transitive" && finding.recommendedNpmTransitiveRemediation?.kind === "update-parent-within-range" ) { - if (packageManager === "yarn" && isBerry && !yarnBerryRecursiveUp) { + if (packageManager === "yarn" && isBerry && yarnBerryRecursiveUp === false) { skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { package: finding.pkg.name, version: finding.pkg.version, relationship: finding.relationship, - reason: `Yarn Berry prior to 3.1 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; upgrade Yarn to 3.1+ or update the parent dependency.`, + reason: `Yarn Berry prior to 3.0 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; upgrade Yarn to 3.0+ or update the parent dependency.`, + }); + continue; + } + if (packageManager === "yarn" && isBerry && yarnBerryRecursiveUp === "ambiguous") { + skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { + package: finding.pkg.name, + version: finding.pkg.version, + relationship: finding.relationship, + reason: `Yarn Berry version cannot be determined reliably (lockfile metadata version 4 is shared by Yarn 2 and 3); specify packageManager in package.json or yarnPath in .yarnrc.yml to enable automatic refresh.`, }); continue; } @@ -644,7 +653,7 @@ function buildYarnUpgradeCommand( pkg: string, workspaces?: string[], isBerry = false, - yarnBerryRecursiveUp = true, + yarnBerryRecursiveUp: boolean | "ambiguous" = true, ): string { const pkgQ = shellQuote(pkg); const nonRoot = [...new Set((workspaces ?? []).filter(w => w !== "."))]; @@ -652,9 +661,11 @@ function buildYarnUpgradeCommand( if (nonRoot.length === 0) return `yarn upgrade ${pkgQ}`; return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} upgrade ${pkgQ}`).join(" && "); } - const subCmd = yarnBerryRecursiveUp ? "up -R" : "up"; - if (nonRoot.length === 0) return `yarn ${subCmd} ${pkgQ}`; - return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} ${subCmd} ${pkgQ}`).join(" && "); + if (yarnBerryRecursiveUp === true) { + return `yarn up -R ${pkgQ}`; + } + if (nonRoot.length === 0) return `yarn up ${pkgQ}`; + return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} up ${pkgQ}`).join(" && "); } function buildBunUpdateCommand(pkg: string): string { @@ -838,7 +849,7 @@ function parentUpdateTarget( isDev?: boolean; workspaces?: string[]; yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean; + yarnBerryRecursiveUp?: boolean | "ambiguous"; }, ): SuggestedFixTarget { const namesParent = args.package !== finding.pkg.name; diff --git a/src/utils/fix-runner.ts b/src/utils/fix-runner.ts index e009bf3dc..6dece6c7a 100644 --- a/src/utils/fix-runner.ts +++ b/src/utils/fix-runner.ts @@ -178,7 +178,7 @@ export function buildApplyCommands( target: SuggestedFixTarget, options?: { yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean; + yarnBerryRecursiveUp?: boolean | "ambiguous"; }, ): string[][] { if (target.kind === "parent-update") { @@ -246,7 +246,7 @@ export function buildParentUpdateApplyArgv( target: SuggestedFixTarget, options?: { yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean; + yarnBerryRecursiveUp?: boolean | "ambiguous"; }, ): string[][] { // Always build from structured fields. Never tokenize target.command: plan @@ -279,13 +279,15 @@ export function buildParentUpdateApplyArgv( if (packageManager === "yarn") { const isBerry = (options?.yarnVariant ?? target.yarnVariant) === "berry"; - const recursiveUp = (options?.yarnBerryRecursiveUp ?? target.yarnBerryRecursiveUp) !== false; + const recursiveUp = (options?.yarnBerryRecursiveUp ?? target.yarnBerryRecursiveUp) === true; + if (isBerry && recursiveUp) { + return [["yarn", "up", "-R", updatePkg]]; + } if (isBerry) { - const upArgs = recursiveUp ? ["up", "-R", updatePkg] : ["up", updatePkg]; if (nonRoot.length === 0) { - return [["yarn", ...upArgs]]; + return [["yarn", "up", updatePkg]]; } - return nonRoot.map(ws => ["yarn", "workspace", ws, ...upArgs]); + return nonRoot.map(ws => ["yarn", "workspace", ws, "up", updatePkg]); } if (nonRoot.length === 0) { return [["yarn", "upgrade", updatePkg]]; diff --git a/tests/fix-runner-apply.test.ts b/tests/fix-runner-apply.test.ts index 65dc336d3..56a1c4db2 100644 --- a/tests/fix-runner-apply.test.ts +++ b/tests/fix-runner-apply.test.ts @@ -472,7 +472,7 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { expect(argvs).toEqual([["yarn", "up", "-R", "follow-redirects"]]); }); - it("executes yarn workspace up -R for a workspace within-range refresh on Yarn Berry (v3.1+)", async () => { + it("executes project-level yarn up -R exactly once for within-range refresh on Yarn Berry 3.0+ across multiple workspaces", async () => { const { calls, argvs, run } = recordingRunner(); await applyFixesIfRequested({ plan: plan( @@ -482,8 +482,8 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { package: "follow-redirects", currentVersion: "1.14.0", targetVersion: "1.16.0", - workspaces: ["packages/client"], - command: "yarn workspace packages/client up -R follow-redirects", + workspaces: ["packages/client", "packages/admin"], + command: "yarn up -R follow-redirects", }), ], { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: true }, @@ -494,8 +494,40 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { runCommand: run, }); - expect(calls).toEqual(["yarn workspace packages/client up -R follow-redirects"]); - expect(argvs).toEqual([["yarn", "workspace", "packages/client", "up", "-R", "follow-redirects"]]); + expect(calls).toEqual(["yarn up -R follow-redirects"]); + expect(argvs).toEqual([["yarn", "up", "-R", "follow-redirects"]]); + }); + + it("executes workspace-scoped yarn up for non-recursive Berry updates across members", async () => { + const { calls, argvs, run } = recordingRunner(); + await applyFixesIfRequested({ + plan: plan( + [ + target({ + kind: "parent-update", + package: "follow-redirects", + currentVersion: "1.14.0", + targetVersion: "1.16.0", + workspaces: ["packages/client", "packages/admin"], + command: "yarn workspace packages/client up follow-redirects && yarn workspace packages/admin up follow-redirects", + }), + ], + { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: false }, + ), + projectPath: "/tmp/proj", + totalFindings: 1, + options, + runCommand: run, + }); + + expect(calls).toEqual([ + "yarn workspace packages/client up follow-redirects", + "yarn workspace packages/admin up follow-redirects", + ]); + expect(argvs).toEqual([ + ["yarn", "workspace", "packages/client", "up", "follow-redirects"], + ["yarn", "workspace", "packages/admin", "up", "follow-redirects"], + ]); }); it("preserves yarn upgrade for a root within-range refresh on Yarn Classic", async () => { diff --git a/tests/remediation/fix-commands-workspace-scoping.test.ts b/tests/remediation/fix-commands-workspace-scoping.test.ts index 8a0db9279..87e58863f 100644 --- a/tests/remediation/fix-commands-workspace-scoping.test.ts +++ b/tests/remediation/fix-commands-workspace-scoping.test.ts @@ -169,24 +169,24 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.command).toBe("yarn up -R js-cookie"); }); - it("scopes a within-range refresh to a single member with yarn workspace up -R on Yarn Berry", () => { + it("emits project-level yarn up -R on Yarn Berry (v3.0+) even when the package is scoped to a workspace member", () => { const lock = writeYarnLock("__metadata:\n version: 8\n cacheKey: 10c0\n"); const finding = yarnWithinRangeRefreshFinding(["client"]); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); const target = findFixTargetForFinding(plan!, finding); - expect(target?.command).toBe("yarn workspace client up -R js-cookie"); - expect(plan?.command).toBe("yarn workspace client up -R js-cookie"); + expect(target?.command).toBe("yarn up -R js-cookie"); + expect(plan?.command).toBe("yarn up -R js-cookie"); }); - it("scopes within-range refresh to multiple members with chained yarn workspace commands on Yarn Berry", () => { + it("emits project-level yarn up -R exactly once on Yarn Berry (v3.0+) across multiple workspaces", () => { const lock = writeYarnLock("__metadata:\n version: 8\n cacheKey: 10c0\n"); const finding = yarnWithinRangeRefreshFinding(["client", "admin"]); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); const target = findFixTargetForFinding(plan!, finding); - expect(target?.command).toBe("yarn workspace client up -R js-cookie && yarn workspace admin up -R js-cookie"); - expect(plan?.command).toBe("yarn workspace client up -R js-cookie && yarn workspace admin up -R js-cookie"); + expect(target?.command).toBe("yarn up -R js-cookie"); + expect(plan?.command).toBe("yarn up -R js-cookie"); }); it("detects Berry before building workspace map even if workspace map construction fails", () => { @@ -209,31 +209,84 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.command).toBe("yarn up -R js-cookie"); }); - it("skips transitive within-range refresh on older Berry (< 3.1) with an explanatory note", () => { - // Lockfile metadata version 5 (Yarn 2) + it("recognizes lockfile metadata version 5 as supporting recursive upgrade (Yarn 3.0+)", () => { const lock = writeYarnLock("__metadata:\n version: 5\n cacheKey: 10c0\n"); const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.packageManager).toBe("yarn"); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(false); + expect(plan?.yarnBerryRecursiveUp).toBe(true); + expect(plan?.command).toBe("yarn up -R js-cookie"); + }); + + it("treats metadata version 4 as ambiguous and skips transitive refresh with explanation", () => { + const lock = writeYarnLock("__metadata:\n version: 4\n cacheKey: 10c0\n"); + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.packageManager).toBe("yarn"); + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); expect(plan?.command).toBeNull(); expect(plan?.targets).toHaveLength(0); expect(plan?.skipped).toEqual([ expect.objectContaining({ package: "js-cookie", relationship: "transitive", - reason: expect.stringContaining("Yarn Berry prior to 3.1 does not support recursive lockfile refresh"), + reason: expect.stringContaining("lockfile metadata version 4 is shared by Yarn 2 and 3"), }), ]); }); - it("skips transitive within-range refresh when packageManager specifies Yarn Berry < 3.1", () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-v2-pkg-")); + it("never mistakes engines.yarn ranges for installed versions and keeps metadata 4 ambiguous", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-engines-range-")); const lock = path.join(dir, "yarn.lock"); - fs.writeFileSync(lock, "__metadata:\n version: 6\n cacheKey: 10c0\n"); - fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@2.4.3" })); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ engines: { yarn: ">=2" } })); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); + expect(plan?.command).toBeNull(); + expect(plan?.skipped[0]?.reason).toContain("lockfile metadata version 4 is shared by Yarn 2 and 3"); + }); + + it("restricts .yarnrc.yml version detection strictly to the yarnPath value, ignoring comments and plugins", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-yarnrc-path-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + fs.writeFileSync( + path.join(dir, ".yarnrc.yml"), + [ + "# yarnPath: .yarn/releases/yarn-2.4.3.cjs", + "plugins:", + " - path: .yarn/plugins/@yarnpkg/plugin-interactive-tools-2.4.3.cjs", + 'yarnPath: ".yarn/releases/yarn-3.0.0.cjs"', + ].join("\n"), + ); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(true); + expect(plan?.command).toBe("yarn up -R js-cookie"); + }); + + it("detects older Berry from yarnPath in .yarnrc.yml and skips transitive refresh with 3.0+ note", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-yarnrc-v2-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + fs.writeFileSync( + path.join(dir, ".yarnrc.yml"), + [ + "# yarn-3.0.0 mention in comment", + 'yarnPath: ".yarn/releases/yarn-2.4.3.cjs"', + ].join("\n"), + ); const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); @@ -241,14 +294,14 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.yarnVariant).toBe("berry"); expect(plan?.yarnBerryRecursiveUp).toBe(false); expect(plan?.command).toBeNull(); - expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.1"); + expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); }); - it("supports recursive upgrade when packageManager specifies Yarn 3.1+", () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-v31-pkg-")); + it("supports recursive upgrade when packageManager specifies Yarn 3.0 explicitly", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-v30-pkg-")); const lock = path.join(dir, "yarn.lock"); - fs.writeFileSync(lock, "__metadata:\n version: 6\n cacheKey: 10c0\n"); - fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@3.1.1" })); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@3.0.0" })); const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); @@ -258,10 +311,44 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.command).toBe("yarn up -R js-cookie"); }); + it("skips transitive within-range refresh on older Berry (< 3.0, metadata version < 4) with an explanatory note", () => { + const lock = writeYarnLock("__metadata:\n version: 3\n cacheKey: 10c0\n"); + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.packageManager).toBe("yarn"); + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(false); + expect(plan?.command).toBeNull(); + expect(plan?.targets).toHaveLength(0); + expect(plan?.skipped).toEqual([ + expect.objectContaining({ + package: "js-cookie", + relationship: "transitive", + reason: expect.stringContaining("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"), + }), + ]); + }); + + it("skips transitive within-range refresh when packageManager specifies Yarn Berry 2.x", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-v2-pkg-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 5\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@2.4.3" })); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(false); + expect(plan?.command).toBeNull(); + expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); + }); + it("still allows direct package upgrades on older Yarn Berry releases", () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-v2-direct-")); const lock = path.join(dir, "yarn.lock"); - fs.writeFileSync(lock, "__metadata:\n version: 5\n cacheKey: 10c0\n"); + fs.writeFileSync(lock, "__metadata:\n version: 3\n cacheKey: 10c0\n"); fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@2.4.3" })); const directFinding: Finding = { From 252f1eb897720b772fa3a3359caaa2448838a90a Mon Sep 17 00:00:00 2001 From: bhushan4work Date: Sat, 7 Nov 2026 21:32:00 +0530 Subject: [PATCH 09/14] fix(remediation): address maintainer feedback on Yarn Berry fixes (#1537) --- src/parsers/yarn-lock.ts | 33 ++- src/remediation/fix-commands.ts | 89 ++++---- src/utils/fix-runner.ts | 13 +- tests/fix-runner-apply.test.ts | 57 ----- .../fix-commands-workspace-scoping.test.ts | 194 ++++++++++++++++++ 5 files changed, 248 insertions(+), 138 deletions(-) diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index 6b223bcdc..03a102af8 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -148,9 +148,7 @@ export type YarnBerryInfo = { export function extractYarnPathFromYarnRc(content: string): string | null { for (const line of content.split(/\r?\n/)) { - const trimmed = line.trim(); - if (trimmed.startsWith("#")) continue; - const match = trimmed.match(/^yarnPath:\s*["']?([^#"'\s]+)["']?/); + const match = line.match(/^yarnPath:\s*["']?([^#"'\s]+)["']?/); if (match) { return match[1]; } @@ -158,6 +156,12 @@ export function extractYarnPathFromYarnRc(content: string): string | null { return null; } +function berryInfoFromMajor(major: number): YarnBerryInfo | null { + if (major >= 3) return { isBerry: true, supportsRecursiveUp: true }; + if (major === 2) return { isBerry: true, supportsRecursiveUp: false }; + return null; +} + export function detectYarnBerryInfo( filePath: string | null | undefined, lockContent: string, @@ -175,13 +179,8 @@ export function detectYarnBerryInfo( if (typeof pkgJson.packageManager === "string") { const match = pkgJson.packageManager.match(/^yarn@(\d+)\.(\d+)(?:\.(\d+))?/); if (match) { - const major = parseInt(match[1], 10); - if (major >= 3) { - return { isBerry: true, supportsRecursiveUp: true }; - } - if (major === 2) { - return { isBerry: true, supportsRecursiveUp: false }; - } + const info = berryInfoFromMajor(parseInt(match[1], 10)); + if (info) return info; } } } @@ -193,18 +192,12 @@ export function detectYarnBerryInfo( try { const rcPath = path.join(projectDir, rcName); if (fs.existsSync(rcPath)) { - const rcContent = fs.readFileSync(rcPath, "utf8"); - const yarnPath = extractYarnPathFromYarnRc(rcContent); + const yarnPath = extractYarnPathFromYarnRc(fs.readFileSync(rcPath, "utf8")); if (yarnPath) { - const match = yarnPath.match(/yarn-(\d+)\.(\d+)(?:\.(\d+))?/); + const match = path.basename(yarnPath).match(/yarn-(\d+)\.(\d+)(?:\.(\d+))?/); if (match) { - const major = parseInt(match[1], 10); - if (major >= 3) { - return { isBerry: true, supportsRecursiveUp: true }; - } - if (major === 2) { - return { isBerry: true, supportsRecursiveUp: false }; - } + const info = berryInfoFromMajor(parseInt(match[1], 10)); + if (info) return info; } } } diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index def2e632a..0eb890206 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -10,7 +10,7 @@ import { calculatePathCoverage, formatDependencyPath } from "../utils/path-cover import { pluralize } from "../utils/string.js"; import { buildNpmWorkspaceMap } from "../parsers/package-lock.js"; import { buildPnpmWorkspaceMap } from "../parsers/pnpm-lock.js"; -import { buildYarnWorkspaceMap, isYarnBerry, detectYarnBerryInfo } from "../parsers/yarn-lock.js"; +import { buildYarnWorkspaceMap, detectYarnBerryInfo } 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"; @@ -66,8 +66,6 @@ export type SuggestedFixTarget = { chainProof?: ChainHop[]; chainSafeVersion?: string; chainVulnerablePackage?: string; - yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean | "ambiguous"; }; export type YarnVariant = "classic" | "berry"; @@ -446,39 +444,41 @@ export function buildSuggestedFixCommandPlan( finding.recommendedNpmTransitiveRemediation?.kind === "update-parent-within-range" ) { if (packageManager === "yarn" && isBerry && yarnBerryRecursiveUp === false) { - skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { - package: finding.pkg.name, - version: finding.pkg.version, - relationship: finding.relationship, - reason: `Yarn Berry prior to 3.0 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; upgrade Yarn to 3.0+ or update the parent dependency.`, - }); - continue; - } - if (packageManager === "yarn" && isBerry && yarnBerryRecursiveUp === "ambiguous") { - skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { + if (!finding.chainResolution) { + skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { + package: finding.pkg.name, + version: finding.pkg.version, + relationship: finding.relationship, + reason: `Yarn Berry prior to 3.0 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; upgrade Yarn to 3.0+ or update the parent dependency.`, + }); + continue; + } + } else if (packageManager === "yarn" && isBerry && yarnBerryRecursiveUp === "ambiguous") { + if (!finding.chainResolution) { + skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { + package: finding.pkg.name, + version: finding.pkg.version, + relationship: finding.relationship, + reason: `Yarn Berry version cannot be determined reliably (lockfile metadata version 4 is shared by Yarn 2 and 3); specify packageManager in package.json or yarnPath in .yarnrc.yml to enable automatic refresh.`, + }); + continue; + } + } else { + const lockfileRefreshCommand = packageManager === "pnpm" + ? buildPnpmLockfileRefreshCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces) + : packageManager === "yarn" + ? buildYarnUpgradeCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces, isBerry) + : buildBunUpdateCommand(finding.pkg.name); + upsertTarget(targetsByKey, finding, parentUpdateTarget(finding, urgent, { package: finding.pkg.name, - version: finding.pkg.version, - relationship: finding.relationship, - reason: `Yarn Berry version cannot be determined reliably (lockfile metadata version 4 is shared by Yarn 2 and 3); specify packageManager in package.json or yarnPath in .yarnrc.yml to enable automatic refresh.`, - }); + currentVersion: finding.pkg.version, + childTargetVersion: finding.recommendedNpmTransitiveRemediation.targetChildVersion, + reason: `${finding.pkg.name}@${finding.pkg.version} can be refreshed to ${finding.recommendedNpmTransitiveRemediation.targetChildVersion}+; no parent upgrade needed.`, + command: lockfileRefreshCommand, + workspaces: finding.recommendedNpmTransitiveRemediation?.workspaces, + })); continue; } - const lockfileRefreshCommand = packageManager === "pnpm" - ? buildPnpmLockfileRefreshCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces) - : packageManager === "yarn" - ? buildYarnUpgradeCommand(finding.pkg.name, finding.recommendedNpmTransitiveRemediation.workspaces, isBerry, yarnBerryRecursiveUp) - : buildBunUpdateCommand(finding.pkg.name); - upsertTarget(targetsByKey, finding, parentUpdateTarget(finding, urgent, { - package: finding.pkg.name, - currentVersion: finding.pkg.version, - childTargetVersion: finding.recommendedNpmTransitiveRemediation.targetChildVersion, - reason: `${finding.pkg.name}@${finding.pkg.version} can be refreshed to ${finding.recommendedNpmTransitiveRemediation.targetChildVersion}+; no parent upgrade needed.`, - command: lockfileRefreshCommand, - workspaces: finding.recommendedNpmTransitiveRemediation?.workspaces, - yarnVariant: packageManager === "yarn" ? (isBerry ? "berry" : "classic") : undefined, - yarnBerryRecursiveUp: packageManager === "yarn" && isBerry ? yarnBerryRecursiveUp : undefined, - })); - continue; } const primaryParent = getPrimaryParent(finding); @@ -556,9 +556,7 @@ export function buildSuggestedFixCommandPlan( } else if (packageManager === "pnpm") { target.command = buildPnpmLockfileRefreshCommand(updatePkg, target.workspaces); } else if (packageManager === "yarn") { - target.command = buildYarnUpgradeCommand(updatePkg, target.workspaces, isBerry, yarnBerryRecursiveUp); - target.yarnVariant = isBerry ? "berry" : "classic"; - target.yarnBerryRecursiveUp = isBerry ? yarnBerryRecursiveUp : undefined; + target.command = buildYarnUpgradeCommand(updatePkg, target.workspaces, isBerry); } else if (packageManager === "bun") { target.command = buildBunUpdateCommand(updatePkg); } @@ -653,19 +651,14 @@ function buildYarnUpgradeCommand( pkg: string, workspaces?: string[], isBerry = false, - yarnBerryRecursiveUp: boolean | "ambiguous" = true, ): string { const pkgQ = shellQuote(pkg); - const nonRoot = [...new Set((workspaces ?? []).filter(w => w !== "."))]; - if (!isBerry) { - if (nonRoot.length === 0) return `yarn upgrade ${pkgQ}`; - return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} upgrade ${pkgQ}`).join(" && "); - } - if (yarnBerryRecursiveUp === true) { + if (isBerry) { return `yarn up -R ${pkgQ}`; } - if (nonRoot.length === 0) return `yarn up ${pkgQ}`; - return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} up ${pkgQ}`).join(" && "); + const nonRoot = [...new Set((workspaces ?? []).filter(w => w !== "."))]; + if (nonRoot.length === 0) return `yarn upgrade ${pkgQ}`; + return nonRoot.map(ws => `yarn workspace ${shellQuote(ws)} upgrade ${pkgQ}`).join(" && "); } function buildBunUpdateCommand(pkg: string): string { @@ -848,8 +841,6 @@ function parentUpdateTarget( command: string; isDev?: boolean; workspaces?: string[]; - yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean | "ambiguous"; }, ): SuggestedFixTarget { const namesParent = args.package !== finding.pkg.name; @@ -875,8 +866,6 @@ function parentUpdateTarget( usage: finding.usage ?? null, isDev: args.isDev, workspaces: workspaces.length > 0 ? workspaces : undefined, - yarnVariant: args.yarnVariant, - yarnBerryRecursiveUp: args.yarnBerryRecursiveUp, }; } @@ -933,8 +922,6 @@ function upsertTarget( chainProof: existing.chainProof ?? next.chainProof, chainSafeVersion: existing.chainSafeVersion ?? next.chainSafeVersion, chainVulnerablePackage: existing.chainVulnerablePackage ?? next.chainVulnerablePackage, - yarnVariant: existing.yarnVariant ?? next.yarnVariant, - yarnBerryRecursiveUp: existing.yarnBerryRecursiveUp ?? next.yarnBerryRecursiveUp, childPackage: existing.childPackage ?? next.childPackage, childTargetVersion: higherVersion(existing.childTargetVersion, next.childTargetVersion) ?? existing.childTargetVersion diff --git a/src/utils/fix-runner.ts b/src/utils/fix-runner.ts index 6dece6c7a..29aa0962d 100644 --- a/src/utils/fix-runner.ts +++ b/src/utils/fix-runner.ts @@ -173,7 +173,7 @@ const EMPTY_UNAPPLIED: UnappliedBreakdown = { // 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. -export function buildApplyCommands( +function buildApplyCommands( packageManager: SuggestedFixCommandPlan["packageManager"], target: SuggestedFixTarget, options?: { @@ -278,16 +278,9 @@ export function buildParentUpdateApplyArgv( } if (packageManager === "yarn") { - const isBerry = (options?.yarnVariant ?? target.yarnVariant) === "berry"; - const recursiveUp = (options?.yarnBerryRecursiveUp ?? target.yarnBerryRecursiveUp) === true; - if (isBerry && recursiveUp) { - return [["yarn", "up", "-R", updatePkg]]; - } + const isBerry = options?.yarnVariant === "berry"; if (isBerry) { - if (nonRoot.length === 0) { - return [["yarn", "up", updatePkg]]; - } - return nonRoot.map(ws => ["yarn", "workspace", ws, "up", updatePkg]); + return [["yarn", "up", "-R", updatePkg]]; } if (nonRoot.length === 0) { return [["yarn", "upgrade", updatePkg]]; diff --git a/tests/fix-runner-apply.test.ts b/tests/fix-runner-apply.test.ts index 56a1c4db2..6d2266eb5 100644 --- a/tests/fix-runner-apply.test.ts +++ b/tests/fix-runner-apply.test.ts @@ -498,38 +498,6 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { expect(argvs).toEqual([["yarn", "up", "-R", "follow-redirects"]]); }); - it("executes workspace-scoped yarn up for non-recursive Berry updates across members", async () => { - const { calls, argvs, run } = recordingRunner(); - await applyFixesIfRequested({ - plan: plan( - [ - target({ - kind: "parent-update", - package: "follow-redirects", - currentVersion: "1.14.0", - targetVersion: "1.16.0", - workspaces: ["packages/client", "packages/admin"], - command: "yarn workspace packages/client up follow-redirects && yarn workspace packages/admin up follow-redirects", - }), - ], - { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: false }, - ), - projectPath: "/tmp/proj", - totalFindings: 1, - options, - runCommand: run, - }); - - expect(calls).toEqual([ - "yarn workspace packages/client up follow-redirects", - "yarn workspace packages/admin up follow-redirects", - ]); - expect(argvs).toEqual([ - ["yarn", "workspace", "packages/client", "up", "follow-redirects"], - ["yarn", "workspace", "packages/admin", "up", "follow-redirects"], - ]); - }); - it("preserves yarn upgrade for a root within-range refresh on Yarn Classic", async () => { const { calls, argvs, run } = recordingRunner(); await applyFixesIfRequested({ @@ -581,31 +549,6 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { expect(argvs).toEqual([["yarn", "workspace", "packages/client", "upgrade", "follow-redirects"]]); }); - it("executes yarn up without -R for earlier Berry when recursiveUp is false", async () => { - const { calls, argvs, run } = recordingRunner(); - await applyFixesIfRequested({ - plan: plan( - [ - target({ - kind: "parent-update", - package: "follow-redirects", - currentVersion: "1.14.0", - targetVersion: "1.16.0", - command: "yarn up follow-redirects", - }), - ], - { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: false }, - ), - projectPath: "/tmp/proj", - totalFindings: 1, - options, - runCommand: run, - }); - - expect(calls).toEqual(["yarn up follow-redirects"]); - expect(argvs).toEqual([["yarn", "up", "follow-redirects"]]); - }); - it("executes yarn add for direct dependencies on Yarn Berry", async () => { const { calls, run } = recordingRunner(); await applyFixesIfRequested({ diff --git a/tests/remediation/fix-commands-workspace-scoping.test.ts b/tests/remediation/fix-commands-workspace-scoping.test.ts index 87e58863f..fedb56e6e 100644 --- a/tests/remediation/fix-commands-workspace-scoping.test.ts +++ b/tests/remediation/fix-commands-workspace-scoping.test.ts @@ -366,6 +366,200 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn add lodash@4.17.21"); }); + + it("extracts Yarn version from the basename of yarnPath, ignoring versioned parent directories", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-basename-v3-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + // Parent directory has 2.4.3, but basename is yarn-3.1.0.cjs + fs.writeFileSync( + path.join(dir, ".yarnrc.yml"), + 'yarnPath: "/opt/yarn-2.4.3/releases/yarn-3.1.0.cjs"\n', + ); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(true); + expect(plan?.command).toBe("yarn up -R js-cookie"); + }); + + it("extracts older Berry version from basename even if parent directory has a higher version", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-basename-v2-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + // Parent directory has 3.1.0, but basename is yarn-2.4.3.cjs + fs.writeFileSync( + path.join(dir, ".yarnrc.yml"), + 'yarnPath: "/opt/yarn-3.1.0/releases/yarn-2.4.3.cjs"\n', + ); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(false); + expect(plan?.command).toBeNull(); + expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); + }); + + it("falls back to lockfile metadata when yarnPath basename has no recognizable version", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-no-version-basename-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 5\n cacheKey: 10c0\n"); + fs.writeFileSync( + path.join(dir, ".yarnrc.yml"), + 'yarnPath: ".yarn/releases/custom-yarn.cjs"\n', + ); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + // Falls back to metadata version 5 which supports recursive up + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(true); + expect(plan?.command).toBe("yarn up -R js-cookie"); + }); + + it("ignores nested or indented yarnPath settings and falls back to metadata", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-nested-path-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + fs.writeFileSync( + path.join(dir, ".yarnrc.yml"), + [ + "custom:", + ' yarnPath: ".yarn/releases/yarn-3.0.0.cjs"', + "plugins:", + " - path: .yarn/plugins/@yarnpkg/plugin-3.0.0.cjs", + ].join("\n"), + ); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + // Top-level yarnPath is absent; falls back to metadata version 4 (ambiguous) + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); + expect(plan?.command).toBeNull(); + expect(plan?.skipped[0]?.reason).toContain("lockfile metadata version 4 is shared by Yarn 2 and 3"); + }); + + it("proceeds to parent upgrade via chain resolution when Berry recursive up is ambiguous", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-chain-res-ambiguous-")); + const lock = path.join(dir, "yarn.lock"); + // Metadata version 4, no packageManager field + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ name: "my-app" })); + + const findingWithChain: Finding = { + pkg: { name: "follow-redirects", version: "1.14.0", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-follow-redirects" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["project", "axios", "follow-redirects"]], + relationship: "transitive", + firstFixedVersion: "1.14.8", + recommendedNpmTransitiveRemediation: { + kind: "update-parent-within-range", + package: "axios", + currentVersion: "0.21.1", + targetChildVersion: "1.14.8", + reason: "axios permits follow-redirects 1.14.8", + }, + chainResolution: { + directDep: "axios", + directDepCurrentVersion: "0.21.1", + targetVersion: "0.21.4", + safeVersion: "1.14.8", + chain: [ + { name: "axios", version: "0.21.4", range: "^1.14.8", next: "follow-redirects" }, + { name: "follow-redirects", version: "1.14.8" }, + ], + command: "yarn add axios@0.21.4", + coveredPaths: 1, + totalPaths: 1, + }, + }; + + const plan = buildSuggestedFixCommandPlan([findingWithChain], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); + expect(plan?.command).toBe("yarn add axios@0.21.4"); + expect(plan?.targets).toHaveLength(1); + expect(plan?.targets[0]?.package).toBe("axios"); + expect(plan?.targets[0]?.targetVersion).toBe("0.21.4"); + expect(plan?.targets[0]?.kind).toBe("parent-upgrade"); + expect(plan?.skipped).toHaveLength(0); + + // Finding without chain resolution remains skipped + const findingWithoutChain: Finding = { + ...findingWithChain, + chainResolution: undefined, + }; + const skippedPlan = buildSuggestedFixCommandPlan([findingWithoutChain], yarnScanInput(lock)); + expect(skippedPlan?.command).toBeNull(); + expect(skippedPlan?.targets).toHaveLength(0); + expect(skippedPlan?.skipped).toHaveLength(1); + expect(skippedPlan?.skipped[0]?.package).toBe("follow-redirects"); + expect(skippedPlan?.skipped[0]?.reason).toContain("lockfile metadata version 4 is shared by Yarn 2 and 3"); + }); + + it("proceeds to parent upgrade via chain resolution when Berry recursive up is false", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-chain-res-false-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 3\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@2.4.3" })); + + const findingWithChain: Finding = { + pkg: { name: "follow-redirects", version: "1.14.0", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-follow-redirects" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["project", "axios", "follow-redirects"]], + relationship: "transitive", + firstFixedVersion: "1.14.8", + recommendedNpmTransitiveRemediation: { + kind: "update-parent-within-range", + package: "axios", + currentVersion: "0.21.1", + targetChildVersion: "1.14.8", + reason: "axios permits follow-redirects 1.14.8", + }, + chainResolution: { + directDep: "axios", + directDepCurrentVersion: "0.21.1", + targetVersion: "0.21.4", + safeVersion: "1.14.8", + chain: [ + { name: "axios", version: "0.21.4", range: "^1.14.8", next: "follow-redirects" }, + { name: "follow-redirects", version: "1.14.8" }, + ], + command: "yarn add axios@0.21.4", + coveredPaths: 1, + totalPaths: 1, + }, + }; + + const plan = buildSuggestedFixCommandPlan([findingWithChain], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(false); + expect(plan?.command).toBe("yarn add axios@0.21.4"); + expect(plan?.targets[0]?.package).toBe("axios"); + expect(plan?.skipped).toHaveLength(0); + + const findingWithoutChain: Finding = { + ...findingWithChain, + chainResolution: undefined, + }; + const skippedPlan = buildSuggestedFixCommandPlan([findingWithoutChain], yarnScanInput(lock)); + expect(skippedPlan?.command).toBeNull(); + expect(skippedPlan?.targets).toHaveLength(0); + expect(skippedPlan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); + }); }); // A transitive finding whose fix is a breaking parent upgrade (install), where From 617e284b0bd8a36eddf5f4038fa5ce9e6d409462 Mon Sep 17 00:00:00 2001 From: bhushan4work Date: Sat, 7 Nov 2026 21:49:27 +0530 Subject: [PATCH 10/14] test(remediation): fix fixture types in workspace scoping tests (#1537) --- tests/remediation/fix-commands-workspace-scoping.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/remediation/fix-commands-workspace-scoping.test.ts b/tests/remediation/fix-commands-workspace-scoping.test.ts index fedb56e6e..535c8fe3c 100644 --- a/tests/remediation/fix-commands-workspace-scoping.test.ts +++ b/tests/remediation/fix-commands-workspace-scoping.test.ts @@ -466,6 +466,7 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { package: "axios", currentVersion: "0.21.1", targetChildVersion: "1.14.8", + viaPath: ["project", "axios", "follow-redirects"], reason: "axios permits follow-redirects 1.14.8", }, chainResolution: { @@ -474,7 +475,7 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { targetVersion: "0.21.4", safeVersion: "1.14.8", chain: [ - { name: "axios", version: "0.21.4", range: "^1.14.8", next: "follow-redirects" }, + { name: "axios", version: "0.21.4" }, { name: "follow-redirects", version: "1.14.8" }, ], command: "yarn add axios@0.21.4", @@ -526,6 +527,7 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { package: "axios", currentVersion: "0.21.1", targetChildVersion: "1.14.8", + viaPath: ["project", "axios", "follow-redirects"], reason: "axios permits follow-redirects 1.14.8", }, chainResolution: { @@ -534,7 +536,7 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { targetVersion: "0.21.4", safeVersion: "1.14.8", chain: [ - { name: "axios", version: "0.21.4", range: "^1.14.8", next: "follow-redirects" }, + { name: "axios", version: "0.21.4" }, { name: "follow-redirects", version: "1.14.8" }, ], command: "yarn add axios@0.21.4", From 3056eecfa589eded7487c89fe08373160e009a47 Mon Sep 17 00:00:00 2001 From: bhushan4work Date: Sat, 7 Nov 2026 22:08:05 +0530 Subject: [PATCH 11/14] fix(parsers): respect yarnPath precedence and ignorePath in Berry detection (#1537) --- src/parsers/yarn-lock.ts | 68 ++++++++--- tests/parsers/yarn-lock.test.ts | 110 +++++++++++++++++- .../fix-commands-workspace-scoping.test.ts | 45 ++++++- 3 files changed, 201 insertions(+), 22 deletions(-) diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index 03a102af8..b24938e15 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -156,6 +156,16 @@ export function extractYarnPathFromYarnRc(content: string): string | null { return null; } +export function extractIgnorePathFromYarnRc(content: string): boolean { + for (const line of content.split(/\r?\n/)) { + const match = line.match(/^ignorePath:\s*["']?true["']?(?:\s*(?:#.*)?)?$/i); + if (match) { + return true; + } + } + return false; +} + function berryInfoFromMajor(major: number): YarnBerryInfo | null { if (major >= 3) return { isBerry: true, supportsRecursiveUp: true }; if (major === 2) return { isBerry: true, supportsRecursiveUp: false }; @@ -172,6 +182,44 @@ export function detectYarnBerryInfo( if (filePath) { const projectDir = path.dirname(filePath); + + // 1. Check for active yarnPath in .yarnrc.yml / .yarnrc.yaml + let activeYarnPath: string | null = null; + let hasActiveYarnPath = false; + + for (const rcName of [".yarnrc.yml", ".yarnrc.yaml"]) { + try { + const rcPath = path.join(projectDir, rcName); + if (fs.existsSync(rcPath)) { + const rcContent = fs.readFileSync(rcPath, "utf8"); + const yarnPath = extractYarnPathFromYarnRc(rcContent); + if (yarnPath) { + const ignorePath = extractIgnorePathFromYarnRc(rcContent); + if (!ignorePath) { + activeYarnPath = yarnPath; + hasActiveYarnPath = true; + } + } + break; + } + } catch { + // best-effort + } + } + + if (hasActiveYarnPath && activeYarnPath) { + const match = path.basename(activeYarnPath).match(/yarn-(\d+)\.(\d+)(?:\.(\d+))?/); + if (match) { + const info = berryInfoFromMajor(parseInt(match[1], 10)); + if (info) return info; + } + // If an active yarnPath exists but its version cannot be determined, + // preserve an unknown/ambiguous capability state. Do not fall back to a + // conflicting packageManager version or assume recursive support. + return { isBerry: true, supportsRecursiveUp: "ambiguous" }; + } + + // 2. When no yarnPath is configured (or ignorePath: true), check packageManager try { const pkgJsonPath = path.join(projectDir, "package.json"); if (fs.existsSync(pkgJsonPath)) { @@ -187,27 +235,9 @@ export function detectYarnBerryInfo( } catch { // best-effort } - - for (const rcName of [".yarnrc.yml", ".yarnrc.yaml"]) { - try { - const rcPath = path.join(projectDir, rcName); - if (fs.existsSync(rcPath)) { - const yarnPath = extractYarnPathFromYarnRc(fs.readFileSync(rcPath, "utf8")); - if (yarnPath) { - const match = path.basename(yarnPath).match(/yarn-(\d+)\.(\d+)(?:\.(\d+))?/); - if (match) { - const info = berryInfoFromMajor(parseInt(match[1], 10)); - if (info) return info; - } - } - } - } catch { - // best-effort - } - } } - // Fallback to lockfile metadata version: + // 3. Fallback to lockfile metadata version: // __metadata: // version: 8 const metaMatch = lockContent.match(/__metadata:[\s\S]*?version:\s*["']?(\d+)["']?/); diff --git a/tests/parsers/yarn-lock.test.ts b/tests/parsers/yarn-lock.test.ts index 1009310ea..61487e1b1 100644 --- a/tests/parsers/yarn-lock.test.ts +++ b/tests/parsers/yarn-lock.test.ts @@ -7,7 +7,12 @@ import { loadNpmLockGraph } from "../../src/parsers/npm-lock-graph.js"; 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 { + loadFromYarnLock, + detectYarnBerryInfo, + extractYarnPathFromYarnRc, + extractIgnorePathFromYarnRc, +} from "../../src/parsers/yarn-lock.js"; import { removeDir } from "../test-utils.js"; function createTempProjectDir(): string { @@ -688,4 +693,107 @@ follow-redirects@^1.10.0: } }); + describe("Yarn Berry detection & configuration precedence", () => { + it("gives active yarnPath precedence over packageManager", () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + fs.writeFileSync(lockPath, "__metadata:\n version: 4\n cacheKey: 10c0\n", "utf8"); + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ packageManager: "yarn@3.8.0" }), + "utf8", + ); + // Active yarnPath is Yarn 2, conflicting with packageManager Yarn 3 + fs.writeFileSync( + path.join(projectDir, ".yarnrc.yml"), + 'yarnPath: ".yarn/releases/yarn-2.4.3.cjs"\n', + "utf8", + ); + + try { + const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); + expect(info.isBerry).toBe(true); + expect(info.supportsRecursiveUp).toBe(false); + } finally { + removeDir(projectDir); + } + }); + + it("respects ignorePath: true and falls back to packageManager version", () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + fs.writeFileSync(lockPath, "__metadata:\n version: 4\n cacheKey: 10c0\n", "utf8"); + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ packageManager: "yarn@3.8.0" }), + "utf8", + ); + fs.writeFileSync( + path.join(projectDir, ".yarnrc.yml"), + 'yarnPath: ".yarn/releases/yarn-2.4.3.cjs"\nignorePath: true\n', + "utf8", + ); + + try { + const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); + expect(info.isBerry).toBe(true); + expect(info.supportsRecursiveUp).toBe(true); + } finally { + removeDir(projectDir); + } + }); + + it("preserves ambiguous state when active yarnPath version cannot be determined", () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + // Even if lockfile has metadata 5 and packageManager has 3.8.0, active yarnPath with unknown version must stay ambiguous + fs.writeFileSync(lockPath, "__metadata:\n version: 5\n cacheKey: 10c0\n", "utf8"); + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ packageManager: "yarn@3.8.0" }), + "utf8", + ); + fs.writeFileSync( + path.join(projectDir, ".yarnrc.yml"), + 'yarnPath: ".yarn/releases/custom-yarn-bundle.cjs"\n', + "utf8", + ); + + try { + const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); + expect(info.isBerry).toBe(true); + expect(info.supportsRecursiveUp).toBe("ambiguous"); + } finally { + removeDir(projectDir); + } + }); + + it("uses packageManager when yarnPath is absent", () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + fs.writeFileSync(lockPath, "__metadata:\n version: 4\n cacheKey: 10c0\n", "utf8"); + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ packageManager: "yarn@3.8.0" }), + "utf8", + ); + + try { + const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); + expect(info.isBerry).toBe(true); + expect(info.supportsRecursiveUp).toBe(true); + } finally { + removeDir(projectDir); + } + }); + + it("extracts ignorePath strictly from top-level setting, ignoring comments and nested keys", () => { + expect(extractIgnorePathFromYarnRc("ignorePath: true")).toBe(true); + expect(extractIgnorePathFromYarnRc('ignorePath: "true"')).toBe(true); + expect(extractIgnorePathFromYarnRc("ignorePath: false")).toBe(false); + expect(extractIgnorePathFromYarnRc("# ignorePath: true")).toBe(false); + expect(extractIgnorePathFromYarnRc(" ignorePath: true")).toBe(false); + }); + }); + }); diff --git a/tests/remediation/fix-commands-workspace-scoping.test.ts b/tests/remediation/fix-commands-workspace-scoping.test.ts index 535c8fe3c..a5ed6f50c 100644 --- a/tests/remediation/fix-commands-workspace-scoping.test.ts +++ b/tests/remediation/fix-commands-workspace-scoping.test.ts @@ -404,10 +404,12 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); }); - it("falls back to lockfile metadata when yarnPath basename has no recognizable version", () => { + it("preserves ambiguous state when active yarnPath version cannot be determined", () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-no-version-basename-")); const lock = path.join(dir, "yarn.lock"); + // Even with metadata version 5 and packageManager yarn@3.8.0, active yarnPath with unknown version must stay ambiguous fs.writeFileSync(lock, "__metadata:\n version: 5\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@3.8.0" })); fs.writeFileSync( path.join(dir, ".yarnrc.yml"), 'yarnPath: ".yarn/releases/custom-yarn.cjs"\n', @@ -416,7 +418,46 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - // Falls back to metadata version 5 which supports recursive up + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); + expect(plan?.command).toBeNull(); + expect(plan?.skipped[0]?.reason).toContain("Yarn Berry version cannot be determined reliably"); + }); + + it("gives active yarnPath precedence over packageManager when versions conflict", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-precedence-conflict-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + // packageManager says 3.8.0, but active yarnPath is 2.4.3 + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@3.8.0" })); + fs.writeFileSync( + path.join(dir, ".yarnrc.yml"), + 'yarnPath: ".yarn/releases/yarn-2.4.3.cjs"\n', + ); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.yarnVariant).toBe("berry"); + expect(plan?.yarnBerryRecursiveUp).toBe(false); + expect(plan?.command).toBeNull(); + expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); + }); + + it("respects ignorePath: true in .yarnrc.yml so packageManager determines active version", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-ignore-path-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@3.8.0" })); + fs.writeFileSync( + path.join(dir, ".yarnrc.yml"), + 'yarnPath: ".yarn/releases/yarn-2.4.3.cjs"\nignorePath: true\n', + ); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + // yarnPath ignored due to ignorePath: true, packageManager yarn@3.8.0 wins expect(plan?.yarnVariant).toBe("berry"); expect(plan?.yarnBerryRecursiveUp).toBe(true); expect(plan?.command).toBe("yarn up -R js-cookie"); From d43ed407937a5644325d592c42bf23a0b16adcd8 Mon Sep 17 00:00:00 2001 From: bhushan4work Date: Sun, 8 Nov 2026 22:06:18 +0530 Subject: [PATCH 12/14] fix(remediation): simplify Yarn Berry detection and improve recursive refresh guidance (#1537) --- src/parsers/yarn-lock.ts | 56 +++------- src/remediation/fix-commands.ts | 8 +- src/utils/fix-runner.ts | 5 +- tests/fix-runner-apply.test.ts | 6 +- tests/fixture-scan.test.ts | 1 - tests/parsers/yarn-lock.test.ts | 101 ++++++++++++++++-- .../fix-commands-workspace-scoping.test.ts | 30 +++--- 7 files changed, 127 insertions(+), 80 deletions(-) diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index b24938e15..660ad5642 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import { parse as parseYaml } from "yaml"; import { parse as parseYarnLock } from "yarn-lockfile"; import type { PackageRef } from "../types.js"; import { looksLikeVersion, normalizeRawVersion } from "../utils/version.js"; @@ -146,26 +147,23 @@ export type YarnBerryInfo = { supportsRecursiveUp: boolean | "ambiguous"; }; -export function extractYarnPathFromYarnRc(content: string): string | null { - for (const line of content.split(/\r?\n/)) { - const match = line.match(/^yarnPath:\s*["']?([^#"'\s]+)["']?/); - if (match) { - return match[1]; +function readActiveYarnPath(projectDir: string): string | null { + try { + const rcPath = path.join(projectDir, ".yarnrc.yml"); + if (!fs.existsSync(rcPath)) return null; + const content = fs.readFileSync(rcPath, "utf8"); + const parsed = parseYaml(content); + if (!parsed || typeof parsed !== "object") return null; + if (parsed.ignorePath === true || parsed.ignorePath === "true") return null; + if (typeof parsed.yarnPath === "string" && parsed.yarnPath.trim()) { + return parsed.yarnPath.trim(); } + } catch { + // best-effort } return null; } -export function extractIgnorePathFromYarnRc(content: string): boolean { - for (const line of content.split(/\r?\n/)) { - const match = line.match(/^ignorePath:\s*["']?true["']?(?:\s*(?:#.*)?)?$/i); - if (match) { - return true; - } - } - return false; -} - function berryInfoFromMajor(major: number): YarnBerryInfo | null { if (major >= 3) return { isBerry: true, supportsRecursiveUp: true }; if (major === 2) return { isBerry: true, supportsRecursiveUp: false }; @@ -183,31 +181,9 @@ export function detectYarnBerryInfo( if (filePath) { const projectDir = path.dirname(filePath); - // 1. Check for active yarnPath in .yarnrc.yml / .yarnrc.yaml - let activeYarnPath: string | null = null; - let hasActiveYarnPath = false; - - for (const rcName of [".yarnrc.yml", ".yarnrc.yaml"]) { - try { - const rcPath = path.join(projectDir, rcName); - if (fs.existsSync(rcPath)) { - const rcContent = fs.readFileSync(rcPath, "utf8"); - const yarnPath = extractYarnPathFromYarnRc(rcContent); - if (yarnPath) { - const ignorePath = extractIgnorePathFromYarnRc(rcContent); - if (!ignorePath) { - activeYarnPath = yarnPath; - hasActiveYarnPath = true; - } - } - break; - } - } catch { - // best-effort - } - } - - if (hasActiveYarnPath && activeYarnPath) { + // 1. Check for active yarnPath in .yarnrc.yml + const activeYarnPath = readActiveYarnPath(projectDir); + if (activeYarnPath) { const match = path.basename(activeYarnPath).match(/yarn-(\d+)\.(\d+)(?:\.(\d+))?/); if (match) { const info = berryInfoFromMajor(parseInt(match[1], 10)); diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index 0eb890206..bdfa030e3 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -86,7 +86,6 @@ export type SuggestedFixSkip = { export type SuggestedFixCommandPlan = { packageManager: SuggestedFixPackageManager; yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean | "ambiguous"; sourceLabel: string; command: string | null; sections: Array<{ @@ -111,8 +110,6 @@ export function buildSuggestedFixCommandPlan( const packageManager = packageManagerForSource(scanInput.source); if (!packageManager) return null; const offline = !!options?.offline; - const yarnBerry = packageManager === "yarn" && !!scanInput.filePath - && isYarnBerry(safeReadText(scanInput.filePath) ?? ""); let workspaceMap: Map = new Map(); let isBerry = false; @@ -449,7 +446,7 @@ export function buildSuggestedFixCommandPlan( package: finding.pkg.name, version: finding.pkg.version, relationship: finding.relationship, - reason: `Yarn Berry prior to 3.0 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; upgrade Yarn to 3.0+ or update the parent dependency.`, + reason: `Yarn Berry prior to 3.0 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; run yarn set version stable followed by yarn up -R ${finding.pkg.name} or update the parent dependency.`, }); continue; } @@ -562,7 +559,7 @@ export function buildSuggestedFixCommandPlan( } continue; } - if (packageManager === "yarn") target.yarnBerry = yarnBerry; + if (packageManager === "yarn") target.yarnBerry = isBerry; if (target.command && target.workspaces?.length) { const scoped = buildWorkspaceInstallCommands([target], packageManager); if (scoped.length > 0) target.command = scoped.join(" && "); @@ -588,7 +585,6 @@ export function buildSuggestedFixCommandPlan( const plan: SuggestedFixCommandPlan = { packageManager, yarnVariant: packageManager === "yarn" ? (isBerry ? "berry" : "classic") : undefined, - yarnBerryRecursiveUp: packageManager === "yarn" && isBerry ? yarnBerryRecursiveUp : undefined, sourceLabel: packageManagerSourceLabel(scanInput), command, sections, diff --git a/src/utils/fix-runner.ts b/src/utils/fix-runner.ts index 29aa0962d..05e083fa6 100644 --- a/src/utils/fix-runner.ts +++ b/src/utils/fix-runner.ts @@ -178,7 +178,6 @@ function buildApplyCommands( target: SuggestedFixTarget, options?: { yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean | "ambiguous"; }, ): string[][] { if (target.kind === "parent-update") { @@ -241,12 +240,11 @@ function buildInstallApplyArgv( return [["bun", ...args]]; } -export function buildParentUpdateApplyArgv( +function buildParentUpdateApplyArgv( packageManager: SuggestedFixCommandPlan["packageManager"], target: SuggestedFixTarget, options?: { yarnVariant?: YarnVariant; - yarnBerryRecursiveUp?: boolean | "ambiguous"; }, ): string[][] { // Always build from structured fields. Never tokenize target.command: plan @@ -370,7 +368,6 @@ export async function applyFixesIfRequested(params: { const target = applyTargets[i]; for (const commandParts of buildApplyCommands(params.plan.packageManager, target, { yarnVariant: params.plan.yarnVariant, - yarnBerryRecursiveUp: params.plan.yarnBerryRecursiveUp, })) { const commandLine = commandParts.join(" "); const retryHint = `\n${formatRetryHint(commandParts, params.projectPath)}\nCheck that \`${params.plan.packageManager}\` is available and you have write access to node_modules.\nIf running in CI, ensure the install step has already run before cve-lite --fix.`; diff --git a/tests/fix-runner-apply.test.ts b/tests/fix-runner-apply.test.ts index 6d2266eb5..d64e460f8 100644 --- a/tests/fix-runner-apply.test.ts +++ b/tests/fix-runner-apply.test.ts @@ -460,7 +460,7 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { command: "yarn up -R follow-redirects", }), ], - { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: true }, + { packageManager: "yarn", yarnVariant: "berry" }, ), projectPath: "/tmp/proj", totalFindings: 1, @@ -486,7 +486,7 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { command: "yarn up -R follow-redirects", }), ], - { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: true }, + { packageManager: "yarn", yarnVariant: "berry" }, ), projectPath: "/tmp/proj", totalFindings: 1, @@ -562,7 +562,7 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { workspaces: ["packages/api"], }), ], - { packageManager: "yarn", yarnVariant: "berry", yarnBerryRecursiveUp: true }, + { packageManager: "yarn", yarnVariant: "berry" }, ), projectPath: "/tmp/proj", totalFindings: 1, diff --git a/tests/fixture-scan.test.ts b/tests/fixture-scan.test.ts index a76070cd2..92343657e 100644 --- a/tests/fixture-scan.test.ts +++ b/tests/fixture-scan.test.ts @@ -144,7 +144,6 @@ describe("fixture remediation scans", () => { expect(plan?.packageManager).toBe("yarn"); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(true); expect(plan?.command).toBe("yarn up -R follow-redirects"); expect(plan?.targets).toEqual([ expect.objectContaining({ diff --git a/tests/parsers/yarn-lock.test.ts b/tests/parsers/yarn-lock.test.ts index 61487e1b1..69fa23fce 100644 --- a/tests/parsers/yarn-lock.test.ts +++ b/tests/parsers/yarn-lock.test.ts @@ -10,8 +10,6 @@ import { loadFromPnpmLock } from "../../src/parsers/pnpm-lock.js"; import { loadFromYarnLock, detectYarnBerryInfo, - extractYarnPathFromYarnRc, - extractIgnorePathFromYarnRc, } from "../../src/parsers/yarn-lock.js"; import { removeDir } from "../test-utils.js"; @@ -787,12 +785,99 @@ follow-redirects@^1.10.0: } }); - it("extracts ignorePath strictly from top-level setting, ignoring comments and nested keys", () => { - expect(extractIgnorePathFromYarnRc("ignorePath: true")).toBe(true); - expect(extractIgnorePathFromYarnRc('ignorePath: "true"')).toBe(true); - expect(extractIgnorePathFromYarnRc("ignorePath: false")).toBe(false); - expect(extractIgnorePathFromYarnRc("# ignorePath: true")).toBe(false); - expect(extractIgnorePathFromYarnRc(" ignorePath: true")).toBe(false); + it("correctly handles comments, quoted values, and inline comments in .yarnrc.yml", () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + fs.writeFileSync(lockPath, "__metadata:\n version: 4\n cacheKey: 10c0\n", "utf8"); + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ packageManager: "yarn@2.4.3" }), + "utf8", + ); + fs.writeFileSync( + path.join(projectDir, ".yarnrc.yml"), + '# Global settings\nyarnPath: \'.yarn/releases/yarn-3.8.0.cjs\' # active berry release\n# ignorePath: true\n', + "utf8", + ); + + try { + const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); + expect(info.isBerry).toBe(true); + expect(info.supportsRecursiveUp).toBe(true); + } finally { + removeDir(projectDir); + } + }); + + it("respects string quoted ignorePath: \"true\" in .yarnrc.yml", () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + fs.writeFileSync(lockPath, "__metadata:\n version: 4\n cacheKey: 10c0\n", "utf8"); + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ packageManager: "yarn@3.8.0" }), + "utf8", + ); + fs.writeFileSync( + path.join(projectDir, ".yarnrc.yml"), + 'yarnPath: ".yarn/releases/yarn-2.4.3.cjs"\nignorePath: "true"\n', + "utf8", + ); + + try { + const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); + expect(info.isBerry).toBe(true); + expect(info.supportsRecursiveUp).toBe(true); + } finally { + removeDir(projectDir); + } + }); + + it("does not probe unsupported .yarnrc.yaml", () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + fs.writeFileSync(lockPath, "__metadata:\n version: 4\n cacheKey: 10c0\n", "utf8"); + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ packageManager: "yarn@3.8.0" }), + "utf8", + ); + // Unsupported .yarnrc.yaml should be ignored + fs.writeFileSync( + path.join(projectDir, ".yarnrc.yaml"), + 'yarnPath: ".yarn/releases/yarn-2.4.3.cjs"\n', + "utf8", + ); + + try { + const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); + expect(info.isBerry).toBe(true); + // packageManager 3.8.0 wins because .yarnrc.yaml was not probed + expect(info.supportsRecursiveUp).toBe(true); + } finally { + removeDir(projectDir); + } + }); + + it("does not infer installed version from engines.yarn range", () => { + const projectDir = createTempProjectDir(); + const lockPath = path.join(projectDir, "yarn.lock"); + // Lockfile metadata 4 is ambiguous + fs.writeFileSync(lockPath, "__metadata:\n version: 4\n cacheKey: 10c0\n", "utf8"); + fs.writeFileSync( + path.join(projectDir, "package.json"), + JSON.stringify({ engines: { yarn: ">=3.0.0" } }), + "utf8", + ); + + try { + const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); + expect(info.isBerry).toBe(true); + // Must stay ambiguous, not inferred as true from engines range + expect(info.supportsRecursiveUp).toBe("ambiguous"); + } finally { + removeDir(projectDir); + } }); }); diff --git a/tests/remediation/fix-commands-workspace-scoping.test.ts b/tests/remediation/fix-commands-workspace-scoping.test.ts index a5ed6f50c..2971e75fd 100644 --- a/tests/remediation/fix-commands-workspace-scoping.test.ts +++ b/tests/remediation/fix-commands-workspace-scoping.test.ts @@ -156,7 +156,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(target?.command).toBe("yarn up -R js-cookie"); expect(plan?.command).toBe("yarn up -R js-cookie"); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(true); }); it("emits yarn up -R on Yarn Berry (v3.1+) when the package is at the workspace root", () => { @@ -205,7 +204,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.packageManager).toBe("yarn"); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(true); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -216,7 +214,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.packageManager).toBe("yarn"); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(true); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -227,7 +224,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.packageManager).toBe("yarn"); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); expect(plan?.command).toBeNull(); expect(plan?.targets).toHaveLength(0); expect(plan?.skipped).toEqual([ @@ -249,7 +245,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("lockfile metadata version 4 is shared by Yarn 2 and 3"); }); @@ -272,7 +267,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(true); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -292,9 +286,10 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(false); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); + expect(plan?.skipped[0]?.reason).toContain("yarn set version stable"); + expect(plan?.skipped[0]?.reason).toContain("yarn up -R js-cookie"); }); it("supports recursive upgrade when packageManager specifies Yarn 3.0 explicitly", () => { @@ -307,7 +302,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(true); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -318,7 +312,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.packageManager).toBe("yarn"); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(false); expect(plan?.command).toBeNull(); expect(plan?.targets).toHaveLength(0); expect(plan?.skipped).toEqual([ @@ -328,6 +321,8 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { reason: expect.stringContaining("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"), }), ]); + expect(plan?.skipped[0]?.reason).toContain("yarn set version stable"); + expect(plan?.skipped[0]?.reason).toContain("yarn up -R js-cookie"); }); it("skips transitive within-range refresh when packageManager specifies Yarn Berry 2.x", () => { @@ -340,9 +335,10 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(false); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); + expect(plan?.skipped[0]?.reason).toContain("yarn set version stable"); + expect(plan?.skipped[0]?.reason).toContain("yarn up -R js-cookie"); }); it("still allows direct package upgrades on older Yarn Berry releases", () => { @@ -381,7 +377,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(true); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -399,9 +394,10 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(false); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); + expect(plan?.skipped[0]?.reason).toContain("yarn set version stable"); + expect(plan?.skipped[0]?.reason).toContain("yarn up -R js-cookie"); }); it("preserves ambiguous state when active yarnPath version cannot be determined", () => { @@ -419,7 +415,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("Yarn Berry version cannot be determined reliably"); }); @@ -439,9 +434,10 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(false); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); + expect(plan?.skipped[0]?.reason).toContain("yarn set version stable"); + expect(plan?.skipped[0]?.reason).toContain("yarn up -R js-cookie"); }); it("respects ignorePath: true in .yarnrc.yml so packageManager determines active version", () => { @@ -459,7 +455,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { // yarnPath ignored due to ignorePath: true, packageManager yarn@3.8.0 wins expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(true); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -482,7 +477,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { // Top-level yarnPath is absent; falls back to metadata version 4 (ambiguous) expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("lockfile metadata version 4 is shared by Yarn 2 and 3"); }); @@ -528,7 +522,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([findingWithChain], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe("ambiguous"); expect(plan?.command).toBe("yarn add axios@0.21.4"); expect(plan?.targets).toHaveLength(1); expect(plan?.targets[0]?.package).toBe("axios"); @@ -589,7 +582,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([findingWithChain], yarnScanInput(lock)); expect(plan?.yarnVariant).toBe("berry"); - expect(plan?.yarnBerryRecursiveUp).toBe(false); expect(plan?.command).toBe("yarn add axios@0.21.4"); expect(plan?.targets[0]?.package).toBe("axios"); expect(plan?.skipped).toHaveLength(0); @@ -602,6 +594,8 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(skippedPlan?.command).toBeNull(); expect(skippedPlan?.targets).toHaveLength(0); expect(skippedPlan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); + expect(skippedPlan?.skipped[0]?.reason).toContain("yarn set version stable"); + expect(skippedPlan?.skipped[0]?.reason).toContain("yarn up -R follow-redirects"); }); }); From 59be07834964eae51d8f568a2eb62f80d2874778 Mon Sep 17 00:00:00 2001 From: bhushan4work Date: Mon, 9 Nov 2026 07:25:56 +0530 Subject: [PATCH 13/14] fix(remediation): address maintainer review on Yarn Berry fixes (#1537) --- src/parsers/yarn-lock.ts | 37 +++-- src/remediation/fix-commands.ts | 68 ++++----- src/utils/fix-runner.ts | 16 +-- tests/fix-runner-apply.test.ts | 15 +- tests/fixture-scan.test.ts | 2 +- tests/parsers/yarn-lock.test.ts | 2 + .../fix-commands-workspace-scoping.test.ts | 133 +++++++++++++++--- 7 files changed, 175 insertions(+), 98 deletions(-) diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index 660ad5642..0700720ab 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -145,8 +145,18 @@ export function isYarnBerry(content: string): boolean { export type YarnBerryInfo = { isBerry: boolean; supportsRecursiveUp: boolean | "ambiguous"; + ambiguityReason?: "yarnPath" | "metadata-v4"; }; +function readPackageManagerYarnMajor(filePath: string): number | null { + const root = readRootPackageJson(filePath); + if (typeof root?.packageManager === "string") { + const match = root.packageManager.match(/^yarn@(\d+)\.(\d+)(?:\.(\d+))?/); + if (match) return parseInt(match[1], 10); + } + return null; +} + function readActiveYarnPath(projectDir: string): string | null { try { const rcPath = path.join(projectDir, ".yarnrc.yml"); @@ -189,27 +199,14 @@ export function detectYarnBerryInfo( const info = berryInfoFromMajor(parseInt(match[1], 10)); if (info) return info; } - // If an active yarnPath exists but its version cannot be determined, - // preserve an unknown/ambiguous capability state. Do not fall back to a - // conflicting packageManager version or assume recursive support. - return { isBerry: true, supportsRecursiveUp: "ambiguous" }; + return { isBerry: true, supportsRecursiveUp: "ambiguous", ambiguityReason: "yarnPath" }; } // 2. When no yarnPath is configured (or ignorePath: true), check packageManager - try { - const pkgJsonPath = path.join(projectDir, "package.json"); - if (fs.existsSync(pkgJsonPath)) { - const pkgJson = JSON.parse(fs.readFileSync(pkgJsonPath, "utf8")); - if (typeof pkgJson.packageManager === "string") { - const match = pkgJson.packageManager.match(/^yarn@(\d+)\.(\d+)(?:\.(\d+))?/); - if (match) { - const info = berryInfoFromMajor(parseInt(match[1], 10)); - if (info) return info; - } - } - } - } catch { - // best-effort + const pmMajor = readPackageManagerYarnMajor(filePath); + if (pmMajor !== null) { + const info = berryInfoFromMajor(pmMajor); + if (info) return info; } } @@ -225,14 +222,14 @@ export function detectYarnBerryInfo( } // Version 4 is ambiguous: shared by Yarn 2.4.3 and Yarn 3.0.0 if (lockVersion === 4) { - return { isBerry: true, supportsRecursiveUp: "ambiguous" }; + return { isBerry: true, supportsRecursiveUp: "ambiguous", ambiguityReason: "metadata-v4" }; } // Versions < 4 are early Berry v2 return { isBerry: true, supportsRecursiveUp: false }; } // Without metadata version or exact version configuration, capability is ambiguous - return { isBerry: true, supportsRecursiveUp: "ambiguous" }; + return { isBerry: true, supportsRecursiveUp: "ambiguous", ambiguityReason: "metadata-v4" }; } function pathsEqual(left: string[], right: string[]): boolean { diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index bdfa030e3..6b573874e 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -1,4 +1,3 @@ -import fs from "node:fs"; import path from "node:path"; import { safeReadText } from "../utils/file.js"; import type { Finding, ScanInput, SeverityLabel, ChainHop } from "../types.js"; @@ -10,7 +9,7 @@ import { calculatePathCoverage, formatDependencyPath } from "../utils/path-cover import { pluralize } from "../utils/string.js"; import { buildNpmWorkspaceMap } from "../parsers/package-lock.js"; import { buildPnpmWorkspaceMap } from "../parsers/pnpm-lock.js"; -import { buildYarnWorkspaceMap, detectYarnBerryInfo } from "../parsers/yarn-lock.js"; +import { buildYarnWorkspaceMap, detectYarnBerryInfo, type YarnBerryInfo } 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"; @@ -68,8 +67,6 @@ export type SuggestedFixTarget = { chainVulnerablePackage?: string; }; -export type YarnVariant = "classic" | "berry"; - export type SuggestedFixSkip = { package: string; version: string; @@ -85,7 +82,6 @@ export type SuggestedFixSkip = { export type SuggestedFixCommandPlan = { packageManager: SuggestedFixPackageManager; - yarnVariant?: YarnVariant; sourceLabel: string; command: string | null; sections: Array<{ @@ -114,6 +110,7 @@ export function buildSuggestedFixCommandPlan( let workspaceMap: Map = new Map(); let isBerry = false; let yarnBerryRecursiveUp: boolean | "ambiguous" = true; + let yarnBerryAmbiguityReason: YarnBerryInfo["ambiguityReason"]; if (scanInput.filePath) { try { if (packageManager === "npm" && isNpmLockSource(scanInput.source)) { @@ -121,14 +118,11 @@ export function buildSuggestedFixCommandPlan( } else if (packageManager === "pnpm" && scanInput.source === "pnpm-lock") { workspaceMap = buildPnpmWorkspaceMap(scanInput.filePath); } else if (packageManager === "yarn" && scanInput.source === "yarn-lock") { - try { - const lockContent = fs.readFileSync(scanInput.filePath, "utf8"); - const berryInfo = detectYarnBerryInfo(scanInput.filePath, lockContent); - isBerry = berryInfo.isBerry; - yarnBerryRecursiveUp = berryInfo.supportsRecursiveUp; - } catch { - // best-effort - } + const lockContent = safeReadText(scanInput.filePath) ?? ""; + const berryInfo = detectYarnBerryInfo(scanInput.filePath, lockContent); + isBerry = berryInfo.isBerry; + yarnBerryRecursiveUp = berryInfo.supportsRecursiveUp; + yarnBerryAmbiguityReason = berryInfo.ambiguityReason; workspaceMap = buildYarnWorkspaceMap(scanInput.filePath); } else if (packageManager === "bun" && scanInput.source === "bun-lock") { workspaceMap = buildBunWorkspaceMap(scanInput.filePath); @@ -440,23 +434,18 @@ export function buildSuggestedFixCommandPlan( finding.relationship === "transitive" && finding.recommendedNpmTransitiveRemediation?.kind === "update-parent-within-range" ) { - if (packageManager === "yarn" && isBerry && yarnBerryRecursiveUp === false) { + if (packageManager === "yarn" && isBerry && (yarnBerryRecursiveUp === false || yarnBerryRecursiveUp === "ambiguous")) { if (!finding.chainResolution) { + const reason = yarnBerryRecursiveUp === false + ? `Yarn Berry prior to 3.0 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; run \`yarn set version stable\`, then \`yarn up -R ${finding.pkg.name}\` or update the parent dependency.` + : yarnBerryAmbiguityReason === "yarnPath" + ? "Yarn Berry version cannot be determined reliably from yarnPath in .yarnrc.yml; check the yarnPath binary or update the parent dependency." + : "Yarn Berry version cannot be determined reliably (lockfile metadata version 4 is shared by Yarn 2 and 3); specify packageManager in package.json or yarnPath in .yarnrc.yml to enable automatic refresh."; skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { package: finding.pkg.name, version: finding.pkg.version, relationship: finding.relationship, - reason: `Yarn Berry prior to 3.0 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; run yarn set version stable followed by yarn up -R ${finding.pkg.name} or update the parent dependency.`, - }); - continue; - } - } else if (packageManager === "yarn" && isBerry && yarnBerryRecursiveUp === "ambiguous") { - if (!finding.chainResolution) { - skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { - package: finding.pkg.name, - version: finding.pkg.version, - relationship: finding.relationship, - reason: `Yarn Berry version cannot be determined reliably (lockfile metadata version 4 is shared by Yarn 2 and 3); specify packageManager in package.json or yarnPath in .yarnrc.yml to enable automatic refresh.`, + reason, }); continue; } @@ -473,6 +462,7 @@ export function buildSuggestedFixCommandPlan( reason: `${finding.pkg.name}@${finding.pkg.version} can be refreshed to ${finding.recommendedNpmTransitiveRemediation.targetChildVersion}+; no parent upgrade needed.`, command: lockfileRefreshCommand, workspaces: finding.recommendedNpmTransitiveRemediation?.workspaces, + yarnBerry: packageManager === "yarn" ? isBerry : undefined, })); continue; } @@ -546,6 +536,7 @@ export function buildSuggestedFixCommandPlan( // Parent-update commands are rebuilt from the (possibly merged) workspace list // so the printed plan matches what --fix will spawn. for (const target of targets) { + if (packageManager === "yarn") target.yarnBerry = isBerry; if (target.kind === "parent-update") { const updatePkg = target.childPackage ?? target.package; if (packageManager === "npm") { @@ -559,7 +550,6 @@ export function buildSuggestedFixCommandPlan( } continue; } - if (packageManager === "yarn") target.yarnBerry = isBerry; if (target.command && target.workspaces?.length) { const scoped = buildWorkspaceInstallCommands([target], packageManager); if (scoped.length > 0) target.command = scoped.join(" && "); @@ -584,7 +574,6 @@ export function buildSuggestedFixCommandPlan( const plan: SuggestedFixCommandPlan = { packageManager, - yarnVariant: packageManager === "yarn" ? (isBerry ? "berry" : "classic") : undefined, sourceLabel: packageManagerSourceLabel(scanInput), command, sections, @@ -688,17 +677,19 @@ export function findFixTargetForFinding( const target = plan.targets.find(item => { if (finding.recommendedNpmTransitiveRemediation?.kind === "update-parent-within-range") { const rem = finding.recommendedNpmTransitiveRemediation; - if (item.kind !== "parent-update") return false; - if (item.package !== rem.package && item.package !== finding.pkg.name) return false; - // Parent-named parent-update: match only on child identity. Falling - // through to targetVersion compares the parent's version to the child's - // destination and can attach the wrong sibling. - if (item.childPackage) { - return item.childPackage === finding.pkg.name - && item.childTargetVersion === rem.targetChildVersion; + if (item.kind === "parent-update") { + if (item.package !== rem.package && item.package !== finding.pkg.name) return false; + // Parent-named parent-update: match only on child identity. Falling + // through to targetVersion compares the parent's version to the child's + // destination and can attach the wrong sibling. + if (item.childPackage) { + return item.childPackage === finding.pkg.name + && item.childTargetVersion === rem.targetChildVersion; + } + // Self-refresh / lockfile refresh: destination stays on targetVersion. + return item.targetVersion === rem.targetChildVersion; } - // Self-refresh / lockfile refresh: destination stays on targetVersion. - return item.targetVersion === rem.targetChildVersion; + if (!finding.chainResolution) return false; } if (finding.recommendedParentUpgrade) { @@ -837,6 +828,7 @@ function parentUpdateTarget( command: string; isDev?: boolean; workspaces?: string[]; + yarnBerry?: boolean; }, ): SuggestedFixTarget { const namesParent = args.package !== finding.pkg.name; @@ -862,6 +854,7 @@ function parentUpdateTarget( usage: finding.usage ?? null, isDev: args.isDev, workspaces: workspaces.length > 0 ? workspaces : undefined, + yarnBerry: args.yarnBerry, }; } @@ -897,6 +890,7 @@ function upsertTarget( const merged: SuggestedFixTarget = { ...existing, + yarnBerry: next.yarnBerry ?? existing.yarnBerry, urgent: existing.urgent || next.urgent, severity: severityOrder[next.severity] > severityOrder[existing.severity] ? next.severity : existing.severity, adjusted: existing.adjusted || next.adjusted, diff --git a/src/utils/fix-runner.ts b/src/utils/fix-runner.ts index 05e083fa6..5852b7af5 100644 --- a/src/utils/fix-runner.ts +++ b/src/utils/fix-runner.ts @@ -9,7 +9,7 @@ import { pluralize } from "./string.js"; import { isBreakingUpgrade } from "./version.js"; import { installPackageSpec } from "./package-identity.js"; import { formatParentUpdateChildNote } from "../remediation/parent-update-child-note.js"; -import type { SuggestedFixCommandPlan, SuggestedFixTarget, YarnVariant } from "../remediation/fix-commands.js"; +import type { SuggestedFixCommandPlan, SuggestedFixTarget } from "../remediation/fix-commands.js"; import type { DebugLogger } from "../output/debug.js"; const OUTPUT_LIMIT = 8 * 1024; @@ -176,12 +176,9 @@ const EMPTY_UNAPPLIED: UnappliedBreakdown = { function buildApplyCommands( packageManager: SuggestedFixCommandPlan["packageManager"], target: SuggestedFixTarget, - options?: { - yarnVariant?: YarnVariant; - }, ): string[][] { if (target.kind === "parent-update") { - return buildParentUpdateApplyArgv(packageManager, target, options); + return buildParentUpdateApplyArgv(packageManager, target); } return buildInstallApplyArgv(packageManager, target); } @@ -243,9 +240,6 @@ function buildInstallApplyArgv( function buildParentUpdateApplyArgv( packageManager: SuggestedFixCommandPlan["packageManager"], target: SuggestedFixTarget, - options?: { - yarnVariant?: YarnVariant; - }, ): string[][] { // 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. @@ -276,7 +270,7 @@ function buildParentUpdateApplyArgv( } if (packageManager === "yarn") { - const isBerry = options?.yarnVariant === "berry"; + const isBerry = Boolean(target.yarnBerry); if (isBerry) { return [["yarn", "up", "-R", updatePkg]]; } @@ -366,9 +360,7 @@ export async function applyFixesIfRequested(params: { const seenCommands = new Set(); for (let i = 0; i < applyTargets.length; i++) { const target = applyTargets[i]; - for (const commandParts of buildApplyCommands(params.plan.packageManager, target, { - yarnVariant: params.plan.yarnVariant, - })) { + for (const commandParts of buildApplyCommands(params.plan.packageManager, target)) { const commandLine = commandParts.join(" "); const retryHint = `\n${formatRetryHint(commandParts, params.projectPath)}\nCheck that \`${params.plan.packageManager}\` is available and you have write access to node_modules.\nIf running in CI, ensure the install step has already run before cve-lite --fix.`; if (seenCommands.has(commandLine)) continue; diff --git a/tests/fix-runner-apply.test.ts b/tests/fix-runner-apply.test.ts index d64e460f8..92fd246ec 100644 --- a/tests/fix-runner-apply.test.ts +++ b/tests/fix-runner-apply.test.ts @@ -457,10 +457,11 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { package: "follow-redirects", currentVersion: "1.14.0", targetVersion: "1.16.0", + yarnBerry: true, command: "yarn up -R follow-redirects", }), ], - { packageManager: "yarn", yarnVariant: "berry" }, + { packageManager: "yarn" }, ), projectPath: "/tmp/proj", totalFindings: 1, @@ -483,10 +484,11 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { currentVersion: "1.14.0", targetVersion: "1.16.0", workspaces: ["packages/client", "packages/admin"], + yarnBerry: true, command: "yarn up -R follow-redirects", }), ], - { packageManager: "yarn", yarnVariant: "berry" }, + { packageManager: "yarn" }, ), projectPath: "/tmp/proj", totalFindings: 1, @@ -508,10 +510,11 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { package: "follow-redirects", currentVersion: "1.14.0", targetVersion: "1.16.0", + yarnBerry: false, command: "yarn upgrade follow-redirects", }), ], - { packageManager: "yarn", yarnVariant: "classic" }, + { packageManager: "yarn" }, ), projectPath: "/tmp/proj", totalFindings: 1, @@ -534,10 +537,11 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { currentVersion: "1.14.0", targetVersion: "1.16.0", workspaces: ["packages/client"], + yarnBerry: false, command: "yarn workspace packages/client upgrade follow-redirects", }), ], - { packageManager: "yarn", yarnVariant: "classic" }, + { packageManager: "yarn" }, ), projectPath: "/tmp/proj", totalFindings: 1, @@ -560,9 +564,10 @@ describe("applyFixesIfRequested - within-range refresh auto-apply", () => { currentVersion: "0.21.1", targetVersion: "0.33.0", workspaces: ["packages/api"], + yarnBerry: true, }), ], - { packageManager: "yarn", yarnVariant: "berry" }, + { packageManager: "yarn" }, ), projectPath: "/tmp/proj", totalFindings: 1, diff --git a/tests/fixture-scan.test.ts b/tests/fixture-scan.test.ts index 92343657e..818637e22 100644 --- a/tests/fixture-scan.test.ts +++ b/tests/fixture-scan.test.ts @@ -143,12 +143,12 @@ describe("fixture remediation scans", () => { const plan = buildSuggestedFixCommandPlan([finding], scanInput); expect(plan?.packageManager).toBe("yarn"); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn up -R follow-redirects"); expect(plan?.targets).toEqual([ expect.objectContaining({ package: "follow-redirects", kind: "parent-update", + yarnBerry: true, }), ]); expect(plan?.targets).not.toEqual( diff --git a/tests/parsers/yarn-lock.test.ts b/tests/parsers/yarn-lock.test.ts index 69fa23fce..06d13340b 100644 --- a/tests/parsers/yarn-lock.test.ts +++ b/tests/parsers/yarn-lock.test.ts @@ -761,6 +761,7 @@ follow-redirects@^1.10.0: const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); expect(info.isBerry).toBe(true); expect(info.supportsRecursiveUp).toBe("ambiguous"); + expect(info.ambiguityReason).toBe("yarnPath"); } finally { removeDir(projectDir); } @@ -875,6 +876,7 @@ follow-redirects@^1.10.0: expect(info.isBerry).toBe(true); // Must stay ambiguous, not inferred as true from engines range expect(info.supportsRecursiveUp).toBe("ambiguous"); + expect(info.ambiguityReason).toBe("metadata-v4"); } finally { removeDir(projectDir); } diff --git a/tests/remediation/fix-commands-workspace-scoping.test.ts b/tests/remediation/fix-commands-workspace-scoping.test.ts index 2971e75fd..23820bf7a 100644 --- a/tests/remediation/fix-commands-workspace-scoping.test.ts +++ b/tests/remediation/fix-commands-workspace-scoping.test.ts @@ -155,7 +155,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const target = findFixTargetForFinding(plan!, finding); expect(target?.command).toBe("yarn up -R js-cookie"); expect(plan?.command).toBe("yarn up -R js-cookie"); - expect(plan?.yarnVariant).toBe("berry"); }); it("emits yarn up -R on Yarn Berry (v3.1+) when the package is at the workspace root", () => { @@ -203,7 +202,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.packageManager).toBe("yarn"); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -213,7 +211,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.packageManager).toBe("yarn"); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -223,7 +220,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.packageManager).toBe("yarn"); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBeNull(); expect(plan?.targets).toHaveLength(0); expect(plan?.skipped).toEqual([ @@ -244,7 +240,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("lockfile metadata version 4 is shared by Yarn 2 and 3"); }); @@ -266,7 +261,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -285,7 +279,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); expect(plan?.skipped[0]?.reason).toContain("yarn set version stable"); @@ -301,7 +294,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -311,7 +303,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); expect(plan?.packageManager).toBe("yarn"); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBeNull(); expect(plan?.targets).toHaveLength(0); expect(plan?.skipped).toEqual([ @@ -334,7 +325,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); expect(plan?.skipped[0]?.reason).toContain("yarn set version stable"); @@ -359,7 +349,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([directFinding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn add lodash@4.17.21"); }); @@ -376,7 +365,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -393,18 +381,18 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); expect(plan?.skipped[0]?.reason).toContain("yarn set version stable"); expect(plan?.skipped[0]?.reason).toContain("yarn up -R js-cookie"); }); - it("preserves ambiguous state when active yarnPath version cannot be determined", () => { + it("identifies yarnPath uncertainty when active yarnPath version cannot be determined even with lockfile v8 and packageManager", () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-no-version-basename-")); const lock = path.join(dir, "yarn.lock"); - // Even with metadata version 5 and packageManager yarn@3.8.0, active yarnPath with unknown version must stay ambiguous - fs.writeFileSync(lock, "__metadata:\n version: 5\n cacheKey: 10c0\n"); + // Even with metadata version 8 and packageManager yarn@3.8.0, active yarnPath with unknown version must stay ambiguous + // and identify yarnPath uncertainty rather than reporting metadata version 4 or recommending packageManager. + fs.writeFileSync(lock, "__metadata:\n version: 8\n cacheKey: 10c0\n"); fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ packageManager: "yarn@3.8.0" })); fs.writeFileSync( path.join(dir, ".yarnrc.yml"), @@ -414,9 +402,24 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBeNull(); - expect(plan?.skipped[0]?.reason).toContain("Yarn Berry version cannot be determined reliably"); + expect(plan?.skipped[0]?.reason).toContain("from yarnPath in .yarnrc.yml"); + expect(plan?.skipped[0]?.reason).not.toContain("metadata version 4"); + expect(plan?.skipped[0]?.reason).not.toContain("packageManager"); + }); + + it("reports metadata-v4 ambiguity when lockfile is version 4 without active yarnPath", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-meta4-ambig-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n version: 4\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ name: "my-app" })); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.command).toBeNull(); + expect(plan?.skipped[0]?.reason).toContain("lockfile metadata version 4 is shared by Yarn 2 and 3"); + expect(plan?.skipped[0]?.reason).toContain("specify packageManager in package.json or yarnPath in .yarnrc.yml"); }); it("gives active yarnPath precedence over packageManager when versions conflict", () => { @@ -433,7 +436,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const finding = yarnWithinRangeRefreshFinding(); const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("Yarn Berry prior to 3.0 does not support recursive lockfile refresh"); expect(plan?.skipped[0]?.reason).toContain("yarn set version stable"); @@ -454,7 +456,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); // yarnPath ignored due to ignorePath: true, packageManager yarn@3.8.0 wins - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn up -R js-cookie"); }); @@ -476,7 +477,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); // Top-level yarnPath is absent; falls back to metadata version 4 (ambiguous) - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("lockfile metadata version 4 is shared by Yarn 2 and 3"); }); @@ -521,7 +521,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([findingWithChain], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn add axios@0.21.4"); expect(plan?.targets).toHaveLength(1); expect(plan?.targets[0]?.package).toBe("axios"); @@ -581,7 +580,6 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { const plan = buildSuggestedFixCommandPlan([findingWithChain], yarnScanInput(lock)); - expect(plan?.yarnVariant).toBe("berry"); expect(plan?.command).toBe("yarn add axios@0.21.4"); expect(plan?.targets[0]?.package).toBe("axios"); expect(plan?.skipped).toHaveLength(0); @@ -597,6 +595,95 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(skippedPlan?.skipped[0]?.reason).toContain("yarn set version stable"); expect(skippedPlan?.skipped[0]?.reason).toContain("yarn up -R follow-redirects"); }); + + it("accounts for all 3 of 3 findings on a Yarn 2 copy of examples/yarn-berry", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn2-berry-all-3-")); + const lock = path.join(dir, "yarn.lock"); + fs.copyFileSync(path.resolve("examples/yarn-berry/yarn.lock"), lock); + const pkgJson = JSON.parse(fs.readFileSync(path.resolve("examples/yarn-berry/package.json"), "utf8")); + pkgJson.packageManager = "yarn@2.4.3"; + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify(pkgJson, null, 2), "utf8"); + + // Findings matching examples/yarn-berry: + // 1. axios direct finding + const axiosFinding: Finding = { + pkg: { name: "axios", version: "0.21.1", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-cph5-m8f7-6c5x" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["project", "axios"]], + relationship: "direct", + firstFixedVersion: "0.21.2", + validatedFirstFixedVersion: "0.21.4", + }; + + // 2. lodash direct finding + const lodashFinding: Finding = { + pkg: { name: "lodash", version: "4.17.20", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-35jh-r3h4-6jhm" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["project", "lodash"]], + relationship: "direct", + firstFixedVersion: "4.17.21", + validatedFirstFixedVersion: "4.17.21", + }; + + // 3. follow-redirects transitive finding (child of axios) + const followRedirectsFinding: Finding = { + pkg: { name: "follow-redirects", version: "1.14.0", ecosystem: "npm" }, + vulnerabilities: [{ id: "GHSA-74fj-2j2h-c42q" }], + severity: "high", + cveAliases: [], + dependencyPaths: [["project", "axios", "follow-redirects"]], + relationship: "transitive", + firstFixedVersion: "1.14.8", + recommendedNpmTransitiveRemediation: { + kind: "update-parent-within-range", + package: "axios", + currentVersion: "0.21.1", + targetChildVersion: "1.14.8", + viaPath: ["project", "axios", "follow-redirects"], + reason: "axios permits follow-redirects 1.14.8", + }, + chainResolution: { + directDep: "axios", + directDepCurrentVersion: "0.21.1", + targetVersion: "0.21.4", + safeVersion: "1.14.8", + chain: [ + { name: "axios", version: "0.21.4" }, + { name: "follow-redirects", version: "1.14.8" }, + ], + command: "yarn add axios@0.21.4", + coveredPaths: 1, + totalPaths: 1, + }, + }; + + const findings = [axiosFinding, lodashFinding, followRedirectsFinding]; + const plan = buildSuggestedFixCommandPlan(findings, yarnScanInput(lock)); + + expect(plan).not.toBeNull(); + // Must account for all 3 of 3 findings without empty skipped list hiding a missing finding + expect(plan?.totalFindingCount).toBe(3); + expect(plan?.coveredFindingCount).toBe(3); + expect(plan?.skipped).toHaveLength(0); + + // Each finding must have a matching fix target + expect(findFixTargetForFinding(plan!, axiosFinding)).toEqual( + expect.objectContaining({ package: "axios", targetVersion: "0.21.4", kind: "direct" }), + ); + expect(findFixTargetForFinding(plan!, lodashFinding)).toEqual( + expect.objectContaining({ package: "lodash", targetVersion: "4.17.21", kind: "direct" }), + ); + expect(findFixTargetForFinding(plan!, followRedirectsFinding)).toEqual( + expect.objectContaining({ package: "axios", targetVersion: "0.21.4", kind: "direct" }), + ); + + // Command suggested for follow-redirects routes to the parent upgrade + expect(findSuggestedCommandForFinding(plan!, followRedirectsFinding)).toBe("yarn add axios@0.21.4"); + }); }); // A transitive finding whose fix is a breaking parent upgrade (install), where From 1ac1f93c01d5ca9bf288e09ec06546aa0d63ee55 Mon Sep 17 00:00:00 2001 From: bhushan4work Date: Mon, 9 Nov 2026 17:54:18 +0530 Subject: [PATCH 14/14] fix(remediation): remove redundant yarnBerry assignments and clarify ambiguity reasons (#1537) --- src/parsers/yarn-lock.ts | 4 ++-- src/remediation/fix-commands.ts | 12 +++++------- tests/parsers/yarn-lock.test.ts | 15 +++++++++++++++ .../fix-commands-workspace-scoping.test.ts | 16 ++++++++++++++++ 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/parsers/yarn-lock.ts b/src/parsers/yarn-lock.ts index 0700720ab..cfcc8d853 100644 --- a/src/parsers/yarn-lock.ts +++ b/src/parsers/yarn-lock.ts @@ -145,7 +145,7 @@ export function isYarnBerry(content: string): boolean { export type YarnBerryInfo = { isBerry: boolean; supportsRecursiveUp: boolean | "ambiguous"; - ambiguityReason?: "yarnPath" | "metadata-v4"; + ambiguityReason?: "yarnPath" | "metadata-v4" | "missing-metadata"; }; function readPackageManagerYarnMajor(filePath: string): number | null { @@ -229,7 +229,7 @@ export function detectYarnBerryInfo( } // Without metadata version or exact version configuration, capability is ambiguous - return { isBerry: true, supportsRecursiveUp: "ambiguous", ambiguityReason: "metadata-v4" }; + return { isBerry: true, supportsRecursiveUp: "ambiguous", ambiguityReason: "missing-metadata" }; } function pathsEqual(left: string[], right: string[]): boolean { diff --git a/src/remediation/fix-commands.ts b/src/remediation/fix-commands.ts index 6b573874e..1142269b8 100644 --- a/src/remediation/fix-commands.ts +++ b/src/remediation/fix-commands.ts @@ -439,8 +439,10 @@ export function buildSuggestedFixCommandPlan( const reason = yarnBerryRecursiveUp === false ? `Yarn Berry prior to 3.0 does not support recursive lockfile refresh (yarn up -R) for transitive dependencies; run \`yarn set version stable\`, then \`yarn up -R ${finding.pkg.name}\` or update the parent dependency.` : yarnBerryAmbiguityReason === "yarnPath" - ? "Yarn Berry version cannot be determined reliably from yarnPath in .yarnrc.yml; check the yarnPath binary or update the parent dependency." - : "Yarn Berry version cannot be determined reliably (lockfile metadata version 4 is shared by Yarn 2 and 3); specify packageManager in package.json or yarnPath in .yarnrc.yml to enable automatic refresh."; + ? "Yarn Berry version cannot be determined reliably from yarnPath in .yarnrc.yml; check the active version with `yarn --version` or update the parent dependency." + : yarnBerryAmbiguityReason === "metadata-v4" + ? "Yarn Berry version cannot be determined reliably (lockfile metadata version 4 is shared by Yarn 2 and 3); specify packageManager in package.json or yarnPath in .yarnrc.yml to enable automatic refresh." + : "Yarn Berry version cannot be determined reliably (lockfile metadata version is missing); specify packageManager in package.json or yarnPath in .yarnrc.yml to enable automatic refresh."; skippedByKey.set(`${finding.relationship}:${packageInstanceKey(finding.pkg)}`, { package: finding.pkg.name, version: finding.pkg.version, @@ -462,7 +464,6 @@ export function buildSuggestedFixCommandPlan( reason: `${finding.pkg.name}@${finding.pkg.version} can be refreshed to ${finding.recommendedNpmTransitiveRemediation.targetChildVersion}+; no parent upgrade needed.`, command: lockfileRefreshCommand, workspaces: finding.recommendedNpmTransitiveRemediation?.workspaces, - yarnBerry: packageManager === "yarn" ? isBerry : undefined, })); continue; } @@ -544,7 +545,7 @@ export function buildSuggestedFixCommandPlan( } else if (packageManager === "pnpm") { target.command = buildPnpmLockfileRefreshCommand(updatePkg, target.workspaces); } else if (packageManager === "yarn") { - target.command = buildYarnUpgradeCommand(updatePkg, target.workspaces, isBerry); + target.command = buildYarnUpgradeCommand(updatePkg, target.workspaces, target.yarnBerry); } else if (packageManager === "bun") { target.command = buildBunUpdateCommand(updatePkg); } @@ -828,7 +829,6 @@ function parentUpdateTarget( command: string; isDev?: boolean; workspaces?: string[]; - yarnBerry?: boolean; }, ): SuggestedFixTarget { const namesParent = args.package !== finding.pkg.name; @@ -854,7 +854,6 @@ function parentUpdateTarget( usage: finding.usage ?? null, isDev: args.isDev, workspaces: workspaces.length > 0 ? workspaces : undefined, - yarnBerry: args.yarnBerry, }; } @@ -890,7 +889,6 @@ function upsertTarget( const merged: SuggestedFixTarget = { ...existing, - yarnBerry: next.yarnBerry ?? existing.yarnBerry, urgent: existing.urgent || next.urgent, severity: severityOrder[next.severity] > severityOrder[existing.severity] ? next.severity : existing.severity, adjusted: existing.adjusted || next.adjusted, diff --git a/tests/parsers/yarn-lock.test.ts b/tests/parsers/yarn-lock.test.ts index 06d13340b..24ac9d5ce 100644 --- a/tests/parsers/yarn-lock.test.ts +++ b/tests/parsers/yarn-lock.test.ts @@ -881,6 +881,21 @@ follow-redirects@^1.10.0: removeDir(projectDir); } }); + + it("reports missing-metadata ambiguity when lockfile lacks metadata version", () => { + const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-meta-missing-parser-")); + const lockPath = path.join(projectDir, "yarn.lock"); + fs.writeFileSync(lockPath, "__metadata:\n cacheKey: 10c0\n", "utf8"); + + try { + const info = detectYarnBerryInfo(lockPath, fs.readFileSync(lockPath, "utf8")); + expect(info.isBerry).toBe(true); + expect(info.supportsRecursiveUp).toBe("ambiguous"); + expect(info.ambiguityReason).toBe("missing-metadata"); + } finally { + removeDir(projectDir); + } + }); }); }); diff --git a/tests/remediation/fix-commands-workspace-scoping.test.ts b/tests/remediation/fix-commands-workspace-scoping.test.ts index 23820bf7a..c52afc968 100644 --- a/tests/remediation/fix-commands-workspace-scoping.test.ts +++ b/tests/remediation/fix-commands-workspace-scoping.test.ts @@ -404,10 +404,26 @@ describe("buildSuggestedFixCommandPlan - yarn lockfile refresh scoping", () => { expect(plan?.command).toBeNull(); expect(plan?.skipped[0]?.reason).toContain("from yarnPath in .yarnrc.yml"); + expect(plan?.skipped[0]?.reason).toContain("check the active version with `yarn --version`"); expect(plan?.skipped[0]?.reason).not.toContain("metadata version 4"); expect(plan?.skipped[0]?.reason).not.toContain("packageManager"); }); + it("reports missing-metadata ambiguity when lockfile has no metadata version", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-meta-missing-")); + const lock = path.join(dir, "yarn.lock"); + fs.writeFileSync(lock, "__metadata:\n cacheKey: 10c0\n"); + fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ name: "my-app" })); + + const finding = yarnWithinRangeRefreshFinding(); + const plan = buildSuggestedFixCommandPlan([finding], yarnScanInput(lock)); + + expect(plan?.command).toBeNull(); + expect(plan?.skipped[0]?.reason).toContain("lockfile metadata version is missing"); + expect(plan?.skipped[0]?.reason).not.toContain("metadata version 4"); + expect(plan?.skipped[0]?.reason).toContain("specify packageManager in package.json or yarnPath in .yarnrc.yml"); + }); + it("reports metadata-v4 ambiguity when lockfile is version 4 without active yarnPath", () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "yarn-meta4-ambig-")); const lock = path.join(dir, "yarn.lock");