Add exact-pinned-intermediate regression fixture (Discussion #528 #2)

[Ayush7614]

Adds examples/exact-pinned-intermediate/ for Discussion #528 fixture ** Fixture 2**.

Summary

• Intermediate parent (body-parser) exact-pins qs@6.14.2 in the lockfile — not a semver range like ~6.14.0
• CVE Lite must not suggest npm update qs (within-range lockfile refresh)
• Correct fix: parent upgrade on express

Chain

express@4.22.1 → body-parser@1.20.4 → qs@6.14.2

• Lockfile declares "qs": "6.14.2" (exact) on body-parser — only that version satisfies the pin
• npm overrides isolate the deep path and pin vulnerable versions (same technique as deep-chain-no-fix)

Verified scan output

npm run build
node dist/index.js examples/exact-pinned-intermediate --verbose

Parsed 69 packages from package-lock.json
Found 1 package (1 CVE) with known OSV matches
1 medium finding: qs@6.14.2 (transitive via express → body-parser → qs)
Fix command: npm install express@4.22.2
Does NOT suggest: npm update qs

What changed

• examples/exact-pinned-intermediate/package.json + package-lock.json
• examples/readme.md — fixture table + scan command
• tests/fixture-scan.test.ts — asserts parent upgrade, not npm update qs

Contrast with related fixtures

Fixture	Parent declares	Expected fix
wrong-parent / pnpm-within-range	Range that covers the fix	npm update / pnpm update on vulnerable pkg
deep-chain-no-fix	Range that does not cover the fix	Parent upgrade
exact-pinned-intermediate	Exact pin (no range)	Parent upgrade — within-range refresh must not apply

Test plan

• node dist/index.js examples/exact-pinned-intermediate --verbose → npm install express@4.22.2, not npm update qs
• Fixture documented in examples/readme.md
• CI (npm test)

Closes Discussion #528 item fixture 2 (exact-pinned-intermediate).

cc: @sonukapoor

[sonukapoor]

The fixture itself is solid - this is exactly the edge case #528 item #2 describes, and the lockfile structure correctly isolates the nested vulnerable qs path. A few things to fix:

Removed test block - the multiple-versions-same-pkg test block was dropped with no explanation. That fixture is assigned to @coder-Yash886 in Discussion #528 and has a sister PR (#633) in flight - removing its test coverage here needs a clear reason. Could you either restore it or explain why it was removed?

readme.md regressions - three entries disappeared: payload, presenton, and the detailed twenty entry. These look like a rebase accident rather than intentional removals. Please restore them.

Hardcoded reason string - the recommendedParentUpgrade.reason in the test is manually authored, so it won't catch regressions if the scanner's actual output changes. Consider either deriving it from a real scanPackages() call or dropping the assertion on that field.

[Ayush7614]

Thanks for the review, @sonukapoor — addressed in 3cb5f78:

1. Restored multiple-versions-same-pkg test block — accidental removal during the fixture branch work; left assigned to @coder-Yash886 / PR Fixture(#11)/multiple versions same pkg #633 unchanged.
2. Restored readme.md regressions — re-added the twenty snapshot section, payload / presenton local-only rows, and the corrected strapi entry (rebase accident).
3. Dropped hardcoded reason — removed the manually authored recommendedParentUpgrade.reason from the exact-pinned-intermediate mock; the test still asserts the fix command and target kinds.

Ready for another look.

[sonukapoor]

The fixture structure is clean and the requirePackage version parameter improvement in the second commit is a nice touch.

Two things need to be addressed:

There's a TypeScript compile error: recommendedParentUpgrade in the test input is missing the required reason field. Every other test that uses this type includes it (e.g. the transitive-only test has reason: "Upgrade lint-staged..."). The field doesn't need to be meaningful for the assertion - just add reason: "Parent upgrade for exact-pinned qs@6.14.2 via express." or similar.

The test doesn't actually exercise the edge case it's designed to catch. The scenario where exact pinning produces a different outcome than a semver range requires a fix version that falls within ~6.14.0 but outside =6.14.2 - for example 6.14.3. With firstFixedVersion: "6.15.2", both the range fixture and the exact-pinned fixture follow the same code path so the test can't distinguish between them. Please update the fixture and fix version so the test actually catches the regression it claims to cover.

Branch is behind main - please rebase with git fetch origin && git rebase origin/main && git push --force-with-lease.

[Ayush7614]

Thanks again, @sonukapoor — both points addressed in 97c3376:

1. TypeScript compile error — re-added the required reason field to recommendedParentUpgrade in the exact-pinned-intermediate mock: reason: "Parent upgrade for exact-pinned qs path (body-parser pins qs@6.14.2).". It's there to satisfy the type; the assertions still focus on the fix command and target kinds.

2. Edge case now actually exercised — changed firstFixedVersion/validatedFirstFixedVersion from 6.15.2 → 6.14.3. 6.14.3 is inside ~6.14.0 but outside the pinned =6.14.2, so the exact-pinned fixture now diverges from the within-range refresh path and the test genuinely catches the regression. With 6.15.2 both paths were identical, as you noted.

Rebasing onto latest main next (git fetch && git rebase origin/main && git push --force-with-lease) and will ping when it's clean.

[Ayush7614]

Rebased onto latest main (now ahead 3 / behind 0). All three commits replayed cleanly with no conflicts, and the compile fix + 6.14.3 edge-case version are intact. Ready for another look, @sonukapoor.

[sonukapoor]

This branch has fallen behind main. Would you mind rebasing? (git fetch upstream && git rebase upstream/main should do it - let me know if you hit any conflicts.)

[sonukapoor]

Thanks for putting this together. The lockfile structure looks correct - body-parser@1.20.4 has "qs": "6.14.2" as an exact pin in the lockfile (not ~6.14.2 or ^6.14.2), which is exactly the scenario you're trying to capture. The readme update is clean and consistent. A couple of things to address before this lands:

The test doesn't actually exercise the exact-pin detection. The regression this is meant to guard against (Discussion #528) is that the scanner incorrectly routes to npm update qs when body-parser exact-pins the vulnerable version. But the test hardcodes recommendedParentUpgrade in the overrides, then calls buildSuggestedFixCommandPlan directly. That means the fix-command builder's routing is tested (given a pre-classified finding, produce npm install express@4.22.2), but the scanner's ability to detect the exact pin and produce recommendedParentUpgrade is never called. If the analysis layer produced recommendedNpmTransitiveRemediation.kind = "update-parent-within-range" for this lockfile, this test would still pass. The existing transitive-only test already covers the routing behavior being tested here.

To actually guard the regression, the test needs to run the analysis layer against this fixture - something like calling resolveNpmTransitiveRemediation directly on the loaded scanInput and asserting it produces recommendedParentUpgrade (not update-parent-within-range). Or, if a scanner-level test is out of scope for this PR, the description and PR title should say so - "adds a lockfile fixture for manual testing and documents the exact-pin scenario" rather than "regression fixture."

The package.json is identical to deep-chain-no-fix. Both fixtures have the same overrides block (body-parser["."] = "1.20.4", body-parser.qs = "6.14.2", express.qs = "6.15.2") and the same direct dependency (express: 4.22.1). The difference is in the lockfile only: deep-chain-no-fix has "qs": "~6.14.0" in body-parser's dependency field, while this fixture has "qs": "6.14.2" (exact pin). That's a genuinely different scenario, so both fixtures make sense - but it would help to call out the distinction explicitly in the description field. Something like: "...unlike deep-chain-no-fix which uses a tilde range, this fixture uses a bare version string, meaning no update within the installed parent version can resolve it."

Minor: the confidence: "best-effort" in the test overrides doesn't match what the real scanner produces for this routing path. When the scanner routes through upgrade-parent-to-version, it sets confidence: "exact-direct-child" (scanner.ts:450). buildSuggestedFixCommandPlan doesn't check confidence, so this doesn't break anything, but if the test is meant to represent a realistic finding shape, it should match.

[Ayush7614]

Thanks @sonukapoor — addressed in 74415b3:

1. Analysis layer now exercised — the test loads the real lockfile graph and calls resolveNpmTransitiveRemediation on the fixture. It asserts the exact pin is not misclassified as update-parent-within-range (returns null for this deep chain), then derives recommendedParentUpgrade from resolveRecommendedParentUpgrade before asserting the fix command. No more hardcoded remediation overrides.

2. Fixture distinction documented — updated the exact-pinned-intermediate description (and examples/readme.md row) to call out that it shares the same package.json/overrides as deep-chain-no-fix, but the lockfile uses a bare 6.14.2 exact pin instead of ~6.14.0, so no in-range refresh can resolve qs.

3. Confidence matches scanner output — recommendedParentUpgrade is now derived from the resolver; for this deep-chain path the scanner sets confidence: "best-effort" via findUpgradeForImmediateIntermediate (the exact-direct-child confidence applies to the upgrade-parent-to-version branch in scanner.ts:450, which this 3-level chain does not hit).

Rebased onto latest main (ahead 4 / behind 0). Ready for another look.

[sonukapoor]

The regression logic is sound — I traced the full code path and the test would genuinely catch an exact-pin being misclassified as a within-range refresh. The mock is complete: resolveNpmTransitiveRemediation reads from the graph without network calls, and resolveRecommendedParentUpgrade only fetches the express packument which is correctly mocked. A few things to fix before this merges.

[sonukapoor]

packageNameFromRegistryUrl and mockPackumentsByPackage (below) are already defined in tests/parent-upgrade.test.ts. Two copies of the same logic, and they've already diverged: this version adds .split("/")[0] after slicing the registry prefix, which breaks scoped packages — @scope/pkg becomes just "@scope". The fixture only uses qs and express so the bug is dormant here, but it's a regression waiting to happen. Please extract both helpers to a shared file (e.g. tests/helpers/registry-mock.ts) and import from there in both test files.

[sonukapoor]

This assertion is redundant — null can't have a kind property, so if line 212 passes, this one is trivially true. Drop it; the claim is "returns null", not "returns something that isn't update-parent-within-range".

[sonukapoor]

The test asserts null but doesn't confirm why. The causal chain is: the graph stores body-parser's qs range as "6.14.2" (exact pin), versionSatisfiesRange("6.14.3", "6.14.2") returns false, no in-range target exists. Worth adding a short assertion or comment confirming the graph holds the range as "6.14.2" and not "~6.14.0" — otherwise if someone regenerates the fixture lockfile from scratch (which would produce a tilde range), the test breaks for the wrong reason without making the bug obvious.

[sonukapoor]

The lockfile is hand-crafted — body-parser's qs range was manually changed from ~6.14.0 to 6.14.2, it was not generated by npm install. Worth noting in the description or in examples/readme.md so future contributors know not to regenerate it blindly.

[Ayush7614]

Thanks @sonukapoor — addressed the latest review in b05e2ca:

• Shared registry mock helpers — extracted packageNameFromRegistryUrl and mockPackumentsByPackage to tests/helpers/registry-mock.ts; both fixture-scan.test.ts and parent-upgrade.test.ts import from there. Uses the correct scoped-package logic (no .split("/")[0]).
• Redundant assertion removed — dropped expect(remediation?.kind).not.toBe("update-parent-within-range"); the test now asserts remediation is null.
• Graph range assertion added — confirms graph.rangeFor(body-parser, "qs") is "6.14.2" (hand-crafted exact pin, not ~6.14.0) before calling the resolver.
• Hand-crafted lockfile documented — noted in examples/exact-pinned-intermediate/package.json description and examples/readme.md.

Rebased onto latest main (ahead 5 / behind 0). CI running.