fix(review): preserve lockfile-tamper entry tracking across nested dependencies (#5837)

[RealDiligent]

Summary

Fixes #5837

scanPackageLockPatch permanently lost track of the active node_modules/... entry when a nested dependenciessub-object appeared before that entry'\''s ownresolved/integrity/version` lines. An attacker could reorder keys in a hand-edited diff to evade the lockfile-tamper check.

Root cause

Entering a nested dependencies: {header hit the reset branch and clearedcurrentEntryKeywithout restoring it when the nested object closed. Subsequentresolved/integrity` lines were silently skipped.

Fix approach

• Track the active package entry separately from inner object nesting depth
• Increment depth on any sub-object header inside an active entry; only clear the entry when its own closing brace is reached (depth zero)
• Regression tests for reordered-key bypass and two nested copies sharing a bare package name

Impact

Tamper diffs with dependencies before resolved/integrity are now correctly flagged when the lockfile-integrity gate is enabled.

Risk / tradeoffs

• Scoped to parsing state only — no API or finding-shape changes
• Legacy lockfileVersion 1 bare-name path unchanged (!sawPackagesEntry branch)

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.17%. Comparing base (696597b) to head (e4c4849).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #6038       +/-   ##
===========================================
+ Coverage   44.58%   95.17%   +50.58%     
===========================================
  Files         592      592               
  Lines       46966    46971        +5     
  Branches    15005    15009        +4     
===========================================
+ Hits        20942    44704    +23762     
+ Misses      22096     1511    -20585     
+ Partials     3928      756     -3172     

Flag	Coverage Δ
shard-1	43.95% <46.66%> (?)
shard-2	36.55% <100.00%> (?)
shard-3	31.98% <0.00%> (?)
shard-4	31.94% <0.00%> (?)
shard-5	32.53% <0.00%> (?)
shard-6	44.63% <0.00%> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/review/lockfile-tamper.ts	100.00% <100.00%> (+95.55%)	⬆️

... and 474 files with indirect coverage changes

[loopover-orb]

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

_{Review updated: 2026-07-15 07:43:23 UTC}

2 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

• safe to merge

Review summary
This fixes a real state-tracking bug in scanPackageLockPatch where a nested `dependencies`/`devDependencies`/`optionalDependencies` sub-object inside a `node_modules/...` entry cleared `currentEntryKey`, silently dropping resolved/integrity/version lines that appeared after it — exactly the bypass described in #5837. The new `activeEntry` + `innerObjectDepth` model correctly distinguishes 'closing my own object' (depth 0, clears the entry) from 'closing a nested sub-object' (depth>0, decrements only), and the three new tests exercise the actual bypass shape (nested deps before resolved/integrity, two nested copies of the same bare name, and a `packages` root wrapper with `optionalDependencies`). I traced the depth increments/decrements through all three test fixtures by hand and they land correctly.

Nits — 4 non-blocking

• src/review/lockfile-tamper.ts: the `else if (activeEntry) { innerObjectDepth++; }` branch increments depth for ANY non-node_modules sub-object header while an entry is active, including a header that itself matches `npmPackageFromNodeModulesPath` only when no `activeEntry` is set yet — worth a one-line comment noting that real npm v2/v3 lockfiles never nest a `node_modules/...` path inside another entry's body, since that's the implicit assumption keeping this correct.
• test/unit/lockfile-tamper.test.ts: consider adding a case with two levels of nested sub-objects sharing a bare key name (e.g. `dependencies` containing another `dependencies`) to lock in the depth-counter behavior beyond the single-level nesting shown here.
• Extract the depth bookkeeping (object-header branch + closing-brace branch) into a small named helper or state machine comment block in lockfile-tamper.ts, since the four-way branch on object headers is dense and would benefit from inline comments explaining each case (new entry / nested sub-object / legacy bare entry / reset).
• Add a changelog/comment note near CONTAINER_KEYS clarifying that it now only affects entry acquisition, not depth tracking, since a reader diffing old vs new logic could otherwise think CONTAINER_KEYS still gates nested headers.

Signal	Result	Evidence
Code review	✅ No blockers	1 reviewer
Linked issue	✅ Linked	#5837
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 344 registered-repo PR(s), 165 merged, 29 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	RealDiligent; Gittensor profile; 344 PR(s), 29 issue(s).
Gate result	✅ Passing	No configured blocker found.
Improvement	✅ Minor	risk: clean · value: minor · LLM: significant

Linked issue satisfaction

Addressed
The diff replaces the single currentEntryKey/currentPackageName reset with an activeEntry object plus innerObjectDepth counter, so nested dependencies/devDependencies/optionalDependencies sub-objects only increment depth rather than clearing the active entry, restoring attribution once the nested object closes while leaving the legacy !sawPackagesEntry path untouched. It also adds the two required

Review context

• Author: RealDiligent
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: not available
• Official Gittensor activity: 344 PR(s), 29 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Keep the PR focused and include validation evidence before maintainer review.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

• Re-run LoopOver review

[loopover-orb]

LoopOver approves — the gate is satisfied and CI is green.