Skip to content

fix(scanner): match CVEs against resolved lockfile version, not manifest floor#79

Merged
LanNguyenSi merged 2 commits into
masterfrom
fix/cve-scanner-resolved-lockfile-version
Jun 21, 2026
Merged

fix(scanner): match CVEs against resolved lockfile version, not manifest floor#79
LanNguyenSi merged 2 commits into
masterfrom
fix/cve-scanner-resolved-lockfile-version

Conversation

@LanNguyenSi

Copy link
Copy Markdown
Owner

What

Fixes the root cause of depsight's CVE false-positive flood: the OSV path sent each npm dependency's declared-range FLOOR (e.g. ^3.27.0 becomes 3.27.0) to OSV instead of the RESOLVED lockfile version, so already-patched repos were flagged as vulnerable.

Why it matters

An org-wide sweep on 2026-06-21 had depsight reporting 273 CVEs (incl. 1 "CRITICAL") across 23 non-archived repos; cross-checking the resolved lockfiles with npm audit showed nearly all already fixed. Concrete: agent-dx declares simple-git ^3.27.0 and was flagged CRITICAL, but the lockfile pins 3.36.0 (safe). The Dependabot path was already correct (GitHub matches lockfile-side); only the OSV path used the floor.

The fix

  • lib/manifest-discovery.ts: new discoverLockfilePaths, parseNpmLockfileContentsList, fetchNpmLockfileResolutions. They fetch the co-located and root package-lock.json (lockfileVersion 1/2/3) and map packageName to its resolved version, keeping the lowest across entries (security-conservative).
  • lib/cve/osv.ts collectDeps: query OSV with the resolved version; fall back to the manifest floor when no lockfile or the package is absent. Resolver failures are caught and degrade to the floor, never aborting the scan.
  • No Prisma migration; Dependabot path untouched; non-npm ecosystems untouched. yarn/pnpm lockfiles fall back to the floor (follow-up).

Review findings addressed

  • Fail-safe: a lockfile-resolver rejection now degrades to the floor instead of returning zero advisories for the whole repo (mutation-verified test).
  • Deep-nested and scoped lockfile keys parse correctly (split, not a greedy regex).
  • Documented residual (MEDIUM, narrow): with per-workspace separate lockfiles, the same dep pinned to different versions across workspaces, and partial lockfile fetch coverage, the global-by-name resolution could substitute a higher (safe) version. It does not occur for standard npm-workspaces with a complete root lockfile. Full per-workspace provenance is tracked in task cac1b6fb.

Verification

  • 222 tests pass (incl. 21 new): resolved-safe, resolved-vulnerable (negative control, real vulns stay flagged), no-lockfile fallback, monorepo, deep-nested keys, fail-safe degrade.
  • tsc clean; eslint clean on changed files.
  • Independent reviewer ran mutation testing: both mutants killed.

Display follow-up (extractVulnRangeInfo shows ranges[0]) is task 6db0ae34.

Refs: CVE sweep 2026-06-21; task a7a4ab3f

nguyen-si-pp and others added 2 commits June 21, 2026 20:56
…est floor

Eliminates false positives in the OSV CVE scanner that occurred when
a repo's package-lock.json had already resolved a dependency to a
patched version that the manifest's declared range floor still fell
inside. Previously, collectDeps stripped the range operator from the
declared spec (e.g. ^10.3.0 → 10.3.0) and sent that floor to OSV;
now it fetches and parses the co-located and root package-lock.json
files, extracts exact resolved versions (lockfileVersion 1/2/3), and
sends those to OSV instead. Falls back to the floor-strip when no
lockfile is present or the package is not listed, so repos without
lockfiles regress nothing.

New pure exports in lib/manifest-discovery.ts:
- discoverLockfilePaths(manifestPaths): string[] — computes which
  package-lock.json paths to probe (root + co-located siblings)
- parseNpmLockfileContentsList(contents): Map<string,string> — parses
  raw lockfile JSON (v1/v2/v3), extracts packageName → resolvedVersion,
  keeps the lowest version across entries (security-conservative)

fetchNpmLockfileResolutions is the thin async wrapper that fetches via
fetchManifestContents and delegates to parseNpmLockfileContentsList.

The Dependabot path (github-advisories.ts) is unaffected.

Co-Authored-By: Lan Nguyen Si <raylsreturn@googlemail.com>
…se, residual docs)

- collectDeps wraps fetchNpmLockfileResolutions in .catch(() => Map) so a resolver failure degrades to the manifest floor instead of aborting the whole npm scan and hiding every advisory (mutation-verified test).
- parseNpmLockfileContentsList takes the name after the LAST node_modules segment (split, not greedy regex) so deeply-nested/scoped keys resolve; skips workspace self-entries.
- Document the cross-workspace partial-coverage residual (full per-workspace provenance is task cac1b6fb).
- Add regression tests: deep-nested key, workspace self-entry, fail-safe degrade.

Co-Authored-By: Lan Nguyen Si <raylsreturn@googlemail.com>
@LanNguyenSi LanNguyenSi merged commit f173398 into master Jun 21, 2026
4 checks passed
@LanNguyenSi LanNguyenSi deleted the fix/cve-scanner-resolved-lockfile-version branch June 21, 2026 19:16
LanNguyenSi added a commit that referenced this pull request Jun 24, 2026
…project floor (#80)

* fix(scanner): resolve Python CVEs against uv.lock/poetry.lock, not pyproject floor

depsight queried OSV with the declared pyproject range floor for direct Python
deps (e.g. jinja2>=3.1 resolves to the 3.1 floor), producing false positives
when the lockfile actually resolves a safe version. Mirror the npm
lockfile-resolution pattern (PR #79) for Python: parse uv.lock and poetry.lock
[[package]] blocks, query OSV with the resolved version, fall back to the
manifest floor when no lockfile lists the dep. PEP 503 name normalization is
applied on both sides of the lookup so my_package and my-package match.

Clears scaffoldkit's 6 floor false positives (jinja2, pydantic). The negative
control keeps genuinely vulnerable resolved versions flagged.

Refs: agent-tasks 2e68c0ab

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(scanner): bound Python lockfile parse to the main [[package]] table

Review finding: parsePythonLockfileContents used last-match-wins and stayed in
the block across all sub-tables. poetry.lock writes sub-tables after the main
table, so a [package.dependencies] constraint keyed literally `name` or
`version` (e.g. `version = ">=2.0"`) overwrote the captured block, queried OSV
with a garbage version, and silently hid the package's real CVEs. Switch to
first-match-wins (the main table lists name/version before any sub-table).
uv.lock inline-table arrays were already safe via the `^` anchor.

Adds two negative-control tests: poetry.lock sub-table keys named name/version
must not corrupt the block, and uv.lock inline-table dependency entries must
not register as standalone packages.

Refs: agent-tasks 2e68c0ab

---------

Co-authored-by: Lan Nguyen Si <nguyen-si@publicplan.de>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@LanNguyenSi LanNguyenSi mentioned this pull request Jun 25, 2026
3 tasks
LanNguyenSi added a commit that referenced this pull request Jun 25, 2026
Bump version 0.5.0 -> 0.5.1 and add the CHANGELOG entry for the changes since
v0.5.0: a scanner-correctness pass that matches CVEs against the resolved
lockfile version instead of the manifest floor for npm (#79) and Python
(uv.lock / poetry.lock, #80), plus a dashboard hero screenshot in the README
(#81).

Refs: depsight v0.5.1 scanner-correctness pass

Co-authored-by: Lan Nguyen Si <nguyen-si@publicplan.de>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants