fix(review): lockfile-tamper check loses entry tracking across a nested dependencies sub-object, evadable by key reordering

[JSONbored]

Context

scanPackageLockPatch in src/review/lockfile-tamper.ts (function starts line 114) is a heuristic line-based scanner for package-lock.json diffs that flags the classic supply-chain tell: a resolved/integrity value changed without that same lockfile entry's own "version" field genuinely changing (see the module's header comment for the full rationale).

It tracks "which package entry am I currently inside" with a single currentEntryKey/currentPackageName pair and a sawPackagesEntry flag. The state machine resets currentEntryKey/currentPackageName to null in two places:

• Line 150-151: when an object-header line's key is neither a node_modules/... path nor (once sawPackagesEntry is true) a recognized container key.
• Line 156: unconditionally, on any closing-brace line (} or },).

The bug: a real package-lock.json entry frequently contains its own nested "dependencies" / "devDependencies" / "optionalDependencies" sub-object (this repo's own package-lock.json has many such entries, e.g. apps/loopover-miner-ui/node_modules/@asamuzakjp/css-color). Once inside the "packages" tree (sawPackagesEntry === true), hitting that nested "dependencies": { header falls into the else branch at line 150 (because npmPackageFromNodeModulesPath("dependencies") returns null and CONTAINER_KEYS.has("dependencies") is only checked when !sawPackagesEntry), resetting currentEntryKey/currentPackageName to null without ever restoring them when the nested object closes. Any resolved/integrity/version line for that same outer entry appearing after the nested sub-object in the diff is then silently skipped by the if (!currentEntryKey || !currentPackageName || line.sign === " ") continue; guard (line 158-159) — exactly the fields this module exists to police.

Today's real npm output happens to always order version/resolved/integrity before dependencies within an entry (confirmed against this repo's own package-lock.json), so legitimate diffs don't trip this. But nothing in the parser enforces that ordering, and an attacker crafting a hand-edited tamper diff specifically to evade this exact check would simply reorder the entry's keys so dependencies precedes resolved/integrity — defeating the tamper detector entirely for that entry. test/unit/lockfile-tamper.test.ts has no case with a node_modules/... entry containing a nested dependencies sub-object positioned before its own resolved/integrity/version fields, so this branch is untested today.

Requirements

• Fix scanPackageLockPatch so that a nested "dependencies"/"devDependencies"/"optionalDependencies" sub-object inside a node_modules/... entry does not permanently lose track of that entry — subsequent resolved/integrity/version lines belonging to the same outer entry (appearing after the nested object closes) must still be attributed to it, regardless of key order within the entry.
• The fix must be a parsing/state-tracking correction only — it must not change LockfileTamperCandidate's shape, the module's public API (scanPackageLockPatch's exported callers), or how a detected candidate is turned into a finding/blocker (isConfiguredGateBlocker / gate-mode wiring stays untouched).
• Preserve the existing behavior for the legacy lockfileVersion 1 bare-name "dependencies" container tree (the !sawPackagesEntry branch) exactly as documented in the module's header comment — this fix is scoped to the node_modules/...-keyed lockfileVersion 2/3 path.
• Must not regress the existing entry-keying fix from #2692/#2563 (full node_modules/... path as the map key, not bare package name) — the fix should track entry nesting depth, not just presence/absence of a current key.

Deliverables

• scanPackageLockPatch correctly attributes resolved/integrity/version lines to their owning entry even when a nested dependencies/devDependencies/optionalDependencies sub-object appears earlier in that same entry's diff hunk.
• A regression test in test/unit/lockfile-tamper.test.ts with a synthetic diff where a node_modules/foo entry's dependencies sub-object appears before its resolved/integrity/version lines (key order reversed from real npm output), asserting the tamper signal (resolvedOrIntegrityChanged without a matching versionChanged) is still detected.
• A second test covering the combination noted by the audit: two different nested entries sharing a bare package name (e.g. node_modules/foo and node_modules/bar/node_modules/foo), each also carrying its own dependencies sub-object, to confirm the entry-keying fix from #2692 and this fix compose correctly.

Test Coverage Requirements

Aim for 99%+ Codecov patch coverage (100% including the new branch) on the touched lines in src/review/lockfile-tamper.ts (src/** is in coverage.include). This is a security-relevant regression fix — a dedicated test for the exact bypass scenario (reordered keys defeating detection) is required, not just incidental line coverage.

Expected Outcome

A package-lock.json diff where a tampered entry's resolved/integrity/version lines appear after that entry's own nested dependencies object is now still correctly flagged by the lockfile-tamper check — closing a state-tracking gap that could otherwise let a reordered-key tamper diff slip past this advisory/gate signal undetected.

Links & Resources

• src/review/lockfile-tamper.ts (scanPackageLockPatch, function starting line 114; the reset bug at lines 145-158; entryFor, lines 120-134; CONTAINER_KEYS, near the top of the file)
• test/unit/lockfile-tamper.test.ts (existing test suite)
• Prior related fix: #2563 (original lockfile-tamper-risk gate check) and its #2692 gate-review follow-up (full-path entry keying, documented in this same function's header comment)

[linear-code]

JSO-4020