fix(review): track lockfile entry nesting so a nested dependencies object can't hide tamper (#5837)

[shin-core]

Summary

Closes #5837.

scanPackageLockPatch in src/review/lockfile-tamper.ts is the heuristic line-based scanner that flags the
classic supply-chain tell: a resolved/integrity value changed for a package-lock.json entry without that
same entry's own "version" genuinely changing. It tracked "which entry am I inside" with a single flat
currentEntryKey/currentPackageName pointer.

A real node_modules/... entry frequently contains its own nested dependencies /
devDependencies / optionalDependencies sub-object. Once inside the packages tree, hitting that nested
container header fell into the else branch and reset the pointer to null — and it was never restored when
the nested object closed. Any resolved/integrity/version line for the same outer entry appearing after the
nested sub-object was then silently skipped by the current-entry guard.

Today's npm output always orders version/resolved/integrity before dependencies within an entry, so
legitimate diffs never trip this. But nothing enforces that ordering: an attacker hand-crafting a tamper diff
could simply reorder the entry's keys so dependencies precedes resolved/integrity, defeating the tamper
detector entirely for that entry.

Fix: replace the flat pointer with a stack of brace scopes. Each object-header line pushes a frame (an
entry, or null for a container/root/other header); each closing brace pops one. Closing a nested sub-object
therefore restores the enclosing entry as the current one, so subsequent resolved/integrity/version
lines are still attributed to it regardless of key order. This is a parsing/state-tracking correction only:

• Entries stay keyed by their full node_modules/... path (fix(review): stop false-positiving on transitive bumps and workspace links #2692), so two same-named entries under different
  nesting paths remain independent.
• The legacy lockfileVersion-1 bare-name dependencies tree (!sawPackagesEntry branch) is unchanged.
• LockfileTamperCandidate's shape, the module's public API, and the gate/finding wiring
  (isConfiguredGateBlocker) are untouched.

Scope

• The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
• This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
• This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
• I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

• git diff --check
• npm run actionlint
• npm run typecheck
• npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
• npm run test:workers
• npm run build:mcp
• npm run test:mcp-pack
• npm run ui:openapi:check
• npm run ui:lint
• npm run ui:typecheck
• npm run ui:build
• npm audit --audit-level=moderate
• New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

• Ran the full npm run test:ci gate plus npm audit --audit-level=moderate (0 vulnerabilities). scanPackageLockPatch is at 100% branch coverage on the changed lines (unsharded npm run test:coverage); the two new regression tests were confirmed to FAIL against the pre-fix source, so they genuinely pin the bug.

Safety

• No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
• Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
• Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
• API/OpenAPI/MCP behavior is updated and tested where needed.
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
• Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
• Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes on the Safety boxes: this is a backend-only parsing correction to a deterministic supply-chain scanner — no UI, API/OpenAPI, or auth/CORS/session surface. The negative-path testing that applies is the scanner's own: the added tests cover the tamper-detected path (reordered keys), and the existing suite already covers the no-finding legitimate-bump path (unchanged and still passing). The fix strengthens a security check rather than relaxing one.

UI Evidence

Not applicable — backend-only change to a diff parser; no visible UI, frontend, docs, or extension surface.

Notes

• Regression tests (test/unit/lockfile-tamper.test.ts): (1) a node_modules/lodash entry whose dependencies sub-object precedes its resolved/integrity/version lines (key order reversed from real npm output) — asserts the unbumped-integrity tamper signal is still detected; (2) two distinct entries sharing the bare name foo (node_modules/foo and node_modules/bar/node_modules/foo), each with its own dependencies sub-object — the legitimate bump on the top-level foo does not mask the unbumped integrity edit on the nested foo, confirming the fix(review): stop false-positiving on transitive bumps and workspace links #2692 full-path keying and this nesting fix compose. Both new tests were verified to fail on the pre-fix source.

[superagent-security]

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

[superagent-security]

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

[shin-core]

Closing as a duplicate — #5837 was already resolved upstream by #6038 (commit 497505c), which landed while this was in progress. The two fixes are functionally equivalent (nesting-depth tracking so a nested dependencies sub-object no longer drops the outer entry). No action needed.