SRVKP-12532: CVE skill enhancement by preserving yarn.lock during analysis and adding npm ls evidence

[ankrsinha]

Summary

Improve CVE skill accuracy by preserving the committed lockfile during analysis and capturing full dependency evidence.

Changes

Skill (SKILL.md)

• Phase 2a clean install: Changed from rm -rf node_modules yarn.lock to rm -rf node_modules dist - analysis now runs against the exact dependency versions committed on the branch, preventing false positives from fresh re-resolution
• Resolution strategy: Lockfile regeneration (rm -rf node_modules yarn.lock && yarn install) now only happens after explicitly applying resolutions to package.json
• Evidence reference: Updated comment templates to reference both yarnWhyRaw and npmLsRaw fields

Analysis script (analyze-deps.ts)

• Added npmLsRaw field to the script output capturing npm ls --all <pkg> output
• Jira comments now include both yarn why and npm ls evidence instead of only yarn why

Why

Previously the skill removed yarn.lock before analysis, which caused Yarn to re-resolve all dependencies from scratch. This could report a CVE as already-remediated when the committed lockfile still pinned the vulnerable version. Additionally, Jira comments only included yarn why output, missing the full dependency tree from npm ls.

Assisted-by: Claude 4.6 high

[openshift-ci-robot]

@ankrsinha: This pull request references SRVKP-12532 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Improve CVE skill accuracy by preserving the committed lockfile during analysis and capturing full dependency evidence.

Changes

Skill (SKILL.md)

• Phase 2a clean install: Changed from rm -rf node_modules yarn.lock to rm -rf node_modules dist - analysis now runs against the exact dependency versions committed on the branch, preventing false positives from fresh re-resolution
• Resolution strategy: Lockfile regeneration (rm -rf node_modules yarn.lock && yarn install) now only happens after explicitly applying resolutions to package.json
• Evidence reference: Updated comment templates to reference both yarnWhyRaw and npmLsRaw fields

Analysis script (analyze-deps.ts)

• Added npmLsRaw field to the script output capturing npm ls --all <pkg> output
• Jira comments now include both yarn why and npm ls evidence instead of only yarn why

Why

Previously the skill removed yarn.lock before analysis, which caused Yarn to re-resolve all dependencies from scratch. This could report a CVE as already-remediated when the committed lockfile still pinned the vulnerable version. Additionally, Jira comments only included yarn why output, missing the full dependency tree from npm ls.

Assisted-by: Claude 4.6 high

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ankrsinha
Once this PR has been reviewed and has the lgtm label, please assign vdemeester for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[qodo-code-review]

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 SRVKP-12532 - Partially compliant Compliant requirements: Preserve the committed yarn.lock during initial branch setup so dependency analysis reflects the actual deployed/committed state. Only regenerate yarn.lock after explicitly applying resolutions changes. Non-compliant requirements: Improve accuracy of dependency analysis and reduce noise/unrelated version changes in fix PRs caused by full re-resolution. Requires further human verification: Improve accuracy of dependency analysis and reduce noise/unrelated version changes in fix PRs caused by full re-resolution.

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Robustness npm ls often exits non-zero when it encounters missing/invalid peer deps or extraneous packages; ensure runCmd('npm', ...) behavior won’t crash the script when npm ls returns a failure exit code, since npmLsRaw capture uses the non-JSON form and is not wrapped in a try/catch. function main(): void { const args = parseArgs(); const { raw: yarnWhyRaw, chains } = getYarnWhy(args.package); const npmLsRaw = runCmd('npm', ['ls', '--all', args.package]).trimEnd(); const currentVersion = getCurrentVersion(args.package); // Full-tree check: verify ALL installed copies satisfy the fix, not just the // hoisted one. Falls back to the hoisted version if npm ls returns nothing. Output Size Capturing full npm ls --all <pkg> output into npmLsRaw can be very large and may bloat JSON output / logs / Jira comments; consider truncation/size limits or capturing the JSON output already fetched in getAllInstalledVersions to avoid duplicate and potentially massive evidence blobs. function getAllInstalledVersions(pkg: string): string[] { const output = runCmd('npm', ['ls', '--all', pkg, '--json']); const versions = new Set<string>(); try { const tree = JSON.parse(output); findVersions(tree, pkg, versions); } catch {} return [...versions]; } Error Handling JSON parsing failures in getAllInstalledVersions are silently ignored (catch {}), which can hide legitimate issues (e.g., npm ls warnings mixed into stdout); consider recording parse errors into the result (or at least preserving raw output for that codepath) so reviewers can understand why the full-tree check fell back. function getAllInstalledVersions(pkg: string): string[] { const output = runCmd('npm', ['ls', '--all', pkg, '--json']); const versions = new Set<string>(); try { const tree = JSON.parse(output); findVersions(tree, pkg, versions); } catch {} return [...versions];