Skip to content

Lockfile tamper detector can be silently evaded by placing the changed field outside the entry header's diff-context window #7778

Description

@JSONbored

Problem

scanPackageLockPatch (src/review/lockfile-tamper.ts:115-192, patchLines helper at :64-72) is a heuristic, non-JSON-parsing scanner that tracks which "node_modules/<pkg>": { object it's "inside" purely by watching brace lines as it walks the patch text linearly. patchLines skips @@ hunk-header lines without resetting activeEntry/innerObjectDepth at hunk boundaries. This works only if every hunk containing a resolved/integrity/version change also contains its entry's own opening-brace line within the same hunk's context — but git's default context is only 3 lines, and nothing guarantees the entry's header line survives that window (depends on how many other unchanged fields sit between the entry's { and the changed field; npm's key ordering varies by version and isn't semantically constrained, so it can also be deliberately arranged).

Verified via reproduction (extracted the real parsing logic, fed it a realistic single-hunk diff where integrity is the 4th property inside the entry, i.e. the entry's header line falls outside the 3-line context window):

Result: []   <- the tampered integrity change is silently NOT reported at all

This is a silent false negative in exactly the supply-chain signal this module exists to produce (resolvedOrIntegrityChanged && !versionChanged). The module's own header says it only produces the finding, never decides whether it blocks — but if the ORB gate's review relies on this signal to flag a tampered lockfile, a change positioned to exploit this hunk-boundary gap (whether accidentally or deliberately) evades detection entirely.

Area

src/review/lockfile-tamper.ts (scanPackageLockPatch, patchLines).

Fix

Needs either (a) re-deriving activeEntry for a hunk from a scan of the hunk's leading context lines even without an explicit header in view, or (b) widening the parse to track nesting from the start of the patch rather than resetting per-hunk (the file already maintains a byEntry map keyed by full path across hunks, so the map structure is fine — it's specifically the "which entry is currently open" cursor that isn't robust to a hunk starting without its own header visible).

Notes

Filed maintainer-only per the maintainer's own call: currently self-host only (no live hosted/multi-tenant exposure), and the vulnerable code is already public in this OSS repo either way. Fix directly rather than publishing a public "how to evade the tamper detector" issue.

Resources

  • src/review/lockfile-tamper.ts:64-72 (patchLines), :115-192 (scanPackageLockPatch)

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions