feat: add multi-path-same-vuln regression fixture (#528) by Ayush7614 · Pull Request #761 · OWASP/cve-lite-cli

Ayush7614 · 2026-06-26T07:01:39Z

Summary

Adds examples/multi-path-same-vuln/ for discussion Help wanted: edge case lockfile fixtures for regression testing #528 fixture 3
Same qs advisory via two paths with different parent ranges — each gets the correct fix command
Regression test in tests/fixture-scan.test.ts

Scan output

node dist/index.js examples/multi-path-same-vuln --verbose

Produces:

npm update qs for qs@6.15.1 via body-parser (within-range)
npm install express@4.22.2 for qs@6.14.2 via express (parent upgrade)

Test plan

npm test -- tests/fixture-scan.test.ts -t multi-path-same-vuln
node dist/index.js examples/multi-path-same-vuln --verbose

Closes fixture 3 from #528

sonukapoor · 2026-06-26T07:45:23Z

Hey @Ayush7614 - this branch is a bit behind main. Could you rebase against main and force-push? Happy to review once it's up to date.

Ayush7614 · 2026-06-26T07:48:39Z

@sonukapoor Rebased onto latest main and force-pushed — should be up to date now. Ready for review whenever you have a moment.

sonukapoor · 2026-06-26T23:44:43Z

Hey, this branch is behind main - could you rebase against the latest main and push? Happy to pick this up for review once it's up to date.

Ayush7614 · 2026-06-27T08:45:36Z

@sonukapoor Rebased onto the latest main again (picked up #755, #756, and the docs fix) and force-pushed. Regression test still passes locally — ready for review.

sonukapoor

Clean regression fixture. The two-path scenario is correctly modelled, the mock packument setup is consistent with the existing pattern, and both remediations are explicitly asserted - update-parent-within-range for the body-parser path, upgrade-parent-to-version for the direct express path. Approving.

Craft a minimal express lockfile where qs@6.15.1 and qs@6.14.2 share the same advisory but need different fix commands — npm update qs for the body-parser within-range path and npm install express@4.22.2 for the direct express parent upgrade.

Ayush7614 · 2026-06-29T14:42:59Z

cc: @sonukapoor

sonukapoor

Thanks for adding this fixture - the scenario is exactly what #528 fixture 3 is meant to cover, and the test structure mirrors the exact-pinned-intermediate describe block cleanly. A few things need attention before this is ready to merge.

The negative assertions are missing

The most important regression guard here is not just "the right command was generated" - it's also "the wrong command was NOT generated". The exact-pinned-intermediate and wrong-parent tests both assert .not.toContain(...) to confirm the wrong remediation path wasn't taken. Right now if resolveNpmTransitiveRemediation returned "upgrade-parent-to-version" for the within-range path, the test would still pass. Please add:

expect(withinRangePlan?.command).not.toContain("npm install express");
expect(parentUpgradePlan?.command).not.toContain("npm update qs");

The overrides field in package.json needs a note in the description

The description field doesn't mention that this lockfile is controlled by overrides. The other fixtures that pin specific versions via overrides call this out explicitly and warn against running npm install (since regenerating would produce a different lockfile and break the test). Worth adding the same note here.

readme.md description is slightly misleading

"direct express path" reads as if qs itself is a direct dependency. Both findings are transitive - I'd reword to "express > qs path expects npm install express@4.22.2" for clarity.

sonukapoor · 2026-06-30T10:55:48Z

+          scanInput,
+        );
+
+        expect(withinRangePlan?.command).toBe("npm update qs");


Missing a negative guard here. Add expect(withinRangePlan?.command).not.toContain("npm install express") after this line - otherwise a future change that generates the parent-upgrade command instead of npm update would still pass this test.

sonukapoor · 2026-06-30T10:55:48Z

+        );
+
+        expect(withinRangePlan?.command).toBe("npm update qs");
+        expect(parentUpgradePlan?.command).toBe("npm install express@4.22.2");


Same here - add expect(parentUpgradePlan?.command).not.toContain("npm update qs") to guard against the remediation type flipping.

sonukapoor · 2026-06-30T10:55:48Z

+  "dependencies": {
+    "express": "4.22.1"
+  },
+  "overrides": {


The description field above doesn't mention that this lockfile is controlled by overrides. The other fixtures that hand-craft their lockfiles (e.g. exact-pinned-intermediate) explicitly warn: "do not regenerate with npm install". Worth adding that here for the same reason - anyone who runs npm install will get a different lockfile and wonder why the test broke.

sonukapoor · 2026-06-30T10:55:48Z

 | `direct-and-transitive` | npm | Mixed direct and transitive vulnerability output. |
 | `deep-chain-no-fix` | npm | 3-level chain where the intermediate parent range does not cover the fix — expects a parent upgrade, not `npm update`. |
 | `exact-pinned-intermediate` | npm | Same overrides as `deep-chain-no-fix`, but body-parser uses a bare `6.14.2` exact pin (not `~6.14.0`) — no in-range refresh is possible; expects a parent upgrade, not `npm update`. Lockfile is hand-crafted (qs range changed from `~6.14.0`; do not regenerate with `npm install`). |
+| `multi-path-same-vuln` | npm | Same vulnerable package (`qs`) via two intermediate parents with different ranges — `body-parser` path expects `npm update qs`, direct `express` path expects `npm install express@4.22.2`. |


"direct express path" reads as if qs itself is a direct project dependency. Both findings are transitive. I'd reword to something like: "express > qs path expects npm install express@4.22.2".

Add negative regression guards, document overrides in the fixture description, and clarify readme wording for the express > qs path.

Ayush7614 · 2026-06-30T11:08:59Z

@sonukapoor Thanks for the detailed review — addressed all three points in 5ccdb2f:

Negative assertions — added .not.toContain("npm install express") on the within-range plan and .not.toContain("npm update qs") on the parent-upgrade plan
package.json description — now notes the lockfile is controlled by overrides and warns not to regenerate with npm install
readme.md wording — reworded to express > qs path (both findings are transitive)

Regression test still passes locally. Ready for another look.

sonukapoor

All three concerns addressed - negative assertions added, overrides documented in package.json description with the regeneration warning, and readme fixed to show the full path. Good to merge.

sonukapoor · 2026-06-30T12:15:13Z

Merged - thank you @Ayush7614!

Ayush7614 requested a review from sonukapoor June 26, 2026 07:02

Ayush7614 force-pushed the feat/multi-path-same-vuln branch from bb98fe3 to 02ada3d Compare June 26, 2026 07:47

Ayush7614 force-pushed the feat/multi-path-same-vuln branch from 02ada3d to bfda0d0 Compare June 27, 2026 08:45

sonukapoor approved these changes Jun 28, 2026

View reviewed changes

Ayush7614 force-pushed the feat/multi-path-same-vuln branch from bfda0d0 to 6d394eb Compare June 29, 2026 14:39

sonukapoor requested changes Jun 30, 2026

View reviewed changes

fix: address review feedback for multi-path-same-vuln fixture

5ccdb2f

Add negative regression guards, document overrides in the fixture description, and clarify readme wording for the express > qs path.

sonukapoor approved these changes Jun 30, 2026

View reviewed changes

sonukapoor merged commit 6775e6a into OWASP:main Jun 30, 2026
6 checks passed

Uh oh!

Uh oh!

Conversation

Ayush7614 commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Scan output

Test plan

Uh oh!

sonukapoor commented Jun 26, 2026

Uh oh!

Ayush7614 commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonukapoor commented Jun 26, 2026

Uh oh!

Ayush7614 commented Jun 27, 2026

Uh oh!

sonukapoor left a comment

Choose a reason for hiding this comment

Uh oh!

Ayush7614 commented Jun 29, 2026

Uh oh!

sonukapoor left a comment

Choose a reason for hiding this comment

Uh oh!

sonukapoor Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

sonukapoor Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

sonukapoor Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

sonukapoor Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Ayush7614 commented Jun 30, 2026

Uh oh!

sonukapoor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonukapoor commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Ayush7614 commented Jun 26, 2026 •

edited

Loading

Ayush7614 commented Jun 26, 2026 •

edited

Loading