From 85962e1429145c892b97bda50f8650ec7196a3e6 Mon Sep 17 00:00:00 2001 From: RealDiligent Date: Wed, 15 Jul 2026 15:22:32 +0800 Subject: [PATCH 1/2] fix: resolve lockfile-tamper entry tracking loss across nested deps Track inner object depth inside an active package entry so nested dependencies sub-objects no longer permanently drop resolved/integrity attribution when those fields appear after the sub-object in the diff. Closes #5837 Co-authored-by: Cursor --- src/review/lockfile-tamper.ts | 27 ++++++++++------ test/unit/lockfile-tamper.test.ts | 53 +++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/review/lockfile-tamper.ts b/src/review/lockfile-tamper.ts index cffa90f22b..b317b9fe95 100644 --- a/src/review/lockfile-tamper.ts +++ b/src/review/lockfile-tamper.ts @@ -113,8 +113,8 @@ function versionChanged(removed: string | undefined, added: string | undefined): * actively maintained repo's lockfile and is out of scope here. */ function scanPackageLockPatch(path: string, patch: string): LockfileTamperCandidate[] { const byEntry = new Map(); - let currentEntryKey: string | null = null; - let currentPackageName: string | null = null; + let activeEntry: { entryKey: string; packageName: string } | null = null; + let innerObjectDepth = 0; let sawPackagesEntry = false; const entryFor = (entryKey: string, packageName: string): MutableCandidate => { @@ -140,22 +140,29 @@ function scanPackageLockPatch(path: string, patch: string): LockfileTamperCandid const key = objectHeader[1]!; const nodeModulesPackage = npmPackageFromNodeModulesPath(key); if (nodeModulesPackage) { - currentEntryKey = key; - currentPackageName = nodeModulesPackage; + activeEntry = { entryKey: key, packageName: nodeModulesPackage }; + innerObjectDepth = 0; sawPackagesEntry = true; + } else if (activeEntry) { + innerObjectDepth++; } else if (!sawPackagesEntry && !CONTAINER_KEYS.has(key)) { - currentEntryKey = key; - currentPackageName = key; + activeEntry = { entryKey: key, packageName: key }; + innerObjectDepth = 0; } else { - currentEntryKey = null; - currentPackageName = null; + activeEntry = null; + innerObjectDepth = 0; } continue; } if (body === "}" || body.startsWith("},")) { - currentEntryKey = null; - currentPackageName = null; + if (innerObjectDepth > 0) { + innerObjectDepth--; + } else { + activeEntry = null; + } } + const currentEntryKey = activeEntry?.entryKey ?? null; + const currentPackageName = activeEntry?.packageName ?? null; if (!currentEntryKey || !currentPackageName || line.sign === " ") continue; const resolvedMatch = /^"resolved"\s*:\s*"([^"]*)"/.exec(body); diff --git a/test/unit/lockfile-tamper.test.ts b/test/unit/lockfile-tamper.test.ts index 7fc3dbc441..0e9e6fad5e 100644 --- a/test/unit/lockfile-tamper.test.ts +++ b/test/unit/lockfile-tamper.test.ts @@ -368,4 +368,57 @@ describe("lockfileTamperRiskFinding", () => { expect(finding?.detail).toContain("evil-pkg"); expect(finding?.detail).toContain("outside registry.npmjs.org"); }); + + // #5837: a nested "dependencies" sub-object inside a node_modules/... entry must not permanently drop + // tracking — resolved/integrity/version lines AFTER that sub-object must still attribute to the outer entry. + it("still flags tampered resolved/integrity when a nested dependencies sub-object precedes those fields (#5837)", () => { + const lockPatch = [ + '@@ -1,14 +1,14 @@', + ' "node_modules/foo": {', + ' "dependencies": {', + ' "bar": {', + ' "version": "1.0.0"', + ' }', + ' },', + '- "version": "1.0.0",', + '- "resolved": "https://registry.npmjs.org/foo/-/foo-1.0.0.tgz",', + '- "integrity": "sha512-old=="', + '+ "version": "1.0.0",', + '+ "resolved": "https://registry.npmjs.org/foo/-/foo-1.0.0.tgz",', + '+ "integrity": "sha512-tampered=="', + ' },', + ].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch)]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("foo"); + }); + + it("composes nested-dependencies tracking with full-path entry keying for two nested copies of the same package (#5837)", () => { + const lockPatch = [ + '@@ -1,24 +1,24 @@', + ' "node_modules/foo": {', + ' "dependencies": {', + ' "left-pad": {', + ' "version": "1.0.0"', + ' }', + ' },', + ' "version": "1.0.0",', + '- "integrity": "sha512-old1=="', + '+ "integrity": "sha512-new1=="', + ' },', + ' "node_modules/bar/node_modules/foo": {', + ' "devDependencies": {', + ' "left-pad": {', + ' "version": "2.0.0"', + ' }', + ' },', + ' "version": "2.0.0",', + '- "integrity": "sha512-old2=="', + '+ "integrity": "sha512-tampered2=="', + ' },', + ].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch)]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("foo"); + }); }); From e4c484904daa06ff014cae1ea49d14d3bc92b1ff Mon Sep 17 00:00:00 2001 From: RealDiligent Date: Wed, 15 Jul 2026 15:34:03 +0800 Subject: [PATCH 2/2] test: cover packages wrapper and optionalDependencies nesting paths Co-authored-by: Cursor --- test/unit/lockfile-tamper.test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/unit/lockfile-tamper.test.ts b/test/unit/lockfile-tamper.test.ts index 0e9e6fad5e..5493132c57 100644 --- a/test/unit/lockfile-tamper.test.ts +++ b/test/unit/lockfile-tamper.test.ts @@ -421,4 +421,24 @@ describe("lockfileTamperRiskFinding", () => { expect(finding).not.toBeNull(); expect(finding?.detail).toContain("foo"); }); + + it("tracks tamper signals inside a packages root wrapper and through optionalDependencies sub-objects", () => { + const lockPatch = [ + '@@ -1,12 +1,12 @@', + ' "packages": {', + ' "node_modules/lodash": {', + ' "optionalDependencies": {', + ' "left-pad": {', + ' "version": "1.0.0"', + ' }', + ' },', + '- "integrity": "sha512-old=="', + '+ "integrity": "sha512-tampered=="', + ' }', + ' },', + ].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch)]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("lodash"); + }); });