SRVKP-12265: Add CVE remediation skill to console-plugin#1136
Open
ankrsinha wants to merge 1 commit into
Open
Conversation
Code Review by Qodo
Context used 1. --fixed-version rejects ranges
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
qodo-code-review
Bot
added
documentation
ankrsinha
changed the title
Collaborator
|
@ankrsinha: This pull request references SRVKP-12265 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. DetailsIn response to this:
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. |
anwesha-palit-redhat
requested review from
arvindk-softwaredev
and removed request for
pratap0007
Signed-off-by: Ankur Sinha <anksinha@redhat.com>
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ankrsinha, anwesha-palit-redhat The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ankrsinha
force-pushed
the
feat/SRVKP-12265
branch
from
fb13e9a to
b3ad0d0
Compare
Contributor
|
New changes are detected. LGTM label has been removed. |
Contributor
|
/agentic_review |
Comment on lines
+84
to
+87
| if (!semver.valid(fixedVersion)) { | ||
| console.error(`Invalid semver version: ${fixedVersion}`); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
1. --fixed-version rejects ranges 📎 Requirement gap ≡ Correctness
scripts/fix-cves/analyze-deps.ts requires --fixed-version to pass semver.valid(), which rejects common Jira/advisory formats like >=4.0.8. This can prevent the skill from correctly determining fixed versions and running analysis as intended.
Agent Prompt
## Issue description
`analyze-deps.ts` rejects `--fixed-version` values unless they are an exact semver, but the skill instructions indicate Jira text may include operators like `>=`.
## Issue Context
The CVE workflow depends on passing fixed-version values extracted from Jira/advisories into `scripts/fix-cves/analyze-deps.ts`.
## Fix Focus Areas
- scripts/fix-cves/analyze-deps.ts[65-89]
- .cursor/skills/fix-cves/SKILL.md[27-37]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
By design, the script always receives a concrete version (e.g. 4.0.8), not a range. The AI agent extracts the version number from Jira text like "fixed in >= 4.0.8" before passing it to the script. The semver.gte() comparison requires a concrete version to work.
Comment on lines
+146
to
+150
| ``` | ||
| | SRVKP | CVE ID | Package | Fixed Version | Status | | ||
| |------------|-----------------|-------------|--------------:|--------------------| | ||
| | SRVKP-1234 | CVE-2024-12345 | micromatch | 4.0.8 | pending | | ||
| ``` |
There was a problem hiding this comment.
2. No workflow validation evidence 📎 Requirement gap ☼ Reliability
The added skill documentation includes only placeholder SRVKP/CVE examples and does not record any validation against real CVE Jira examples and release branches. This leaves the end-to-end workflow correctness unverified per the compliance requirement.
Agent Prompt
## Issue description
The skill documentation does not include any evidence that the workflow was validated against existing/real CVE Jira examples and corresponding release branches.
## Issue Context
Compliance requires demonstrating end-to-end validation and correctness review using known Jira examples and relevant release branches.
## Fix Focus Areas
- .cursor/skills/fix-cves/SKILL.md[10-14]
- .cursor/skills/fix-cves/SKILL.md[139-152]
- .cursor/skills/fix-cves/SKILL.md[510-530]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
The PR already includes a screen recording demo and a Jira comment screenshot as validation.
Comment on lines
+54
to
+149
| function runCmd(cmd: string, args: string[]): string { | ||
| try { | ||
| return execFileSync(cmd, args, { | ||
| encoding: 'utf-8', | ||
| maxBuffer: 10 * 1024 * 1024, | ||
| }); | ||
| } catch (e: any) { | ||
| return e.stdout ?? ''; | ||
| } | ||
| } | ||
|
|
||
| function parseArgs(): CLIArgs { | ||
| const args = process.argv.slice(2); | ||
| let pkg = ''; | ||
| let fixedVersion = ''; | ||
| for (let i = 0; i < args.length; i++) { | ||
| if (args[i] === '--package' && args[i + 1]) pkg = args[++i]; | ||
| else if (args[i] === '--fixed-version' && args[i + 1]) | ||
| fixedVersion = args[++i]; | ||
| } | ||
| if (!pkg || !fixedVersion) { | ||
| console.error( | ||
| 'Usage: analyze-deps.ts --package <name> --fixed-version <ver>', | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| if (!/^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/.test(pkg)) { | ||
| console.error(`Invalid package name: ${pkg}`); | ||
| process.exit(1); | ||
| } | ||
| if (!semver.valid(fixedVersion)) { | ||
| console.error(`Invalid semver version: ${fixedVersion}`); | ||
| process.exit(1); | ||
| } | ||
| return { package: pkg, fixedVersion }; | ||
| } | ||
|
|
||
| /** | ||
| * Run `yarn why <pkg>` and return raw output + parsed dependency chains. | ||
| */ | ||
| function getYarnWhy(pkg: string): { raw: string; chains: string[] } { | ||
| const raw = runCmd('yarn', ['why', pkg]); | ||
| const chains: string[] = []; | ||
| for (const line of raw.split('\n')) { | ||
| const trimmed = line.trim(); | ||
| if (trimmed && !trimmed.startsWith('=') && !trimmed.startsWith('Done')) { | ||
| chains.push(trimmed); | ||
| } | ||
| } | ||
| return { raw, chains }; | ||
| } | ||
|
|
||
| /** | ||
| * Determine if a package is a transitive dependency of any SDK package | ||
| * by inspecting `yarn why` output for SDK package names in the chains. | ||
| */ | ||
| function isTransitiveSDKDep(chains: string[]): boolean { | ||
| return chains.some((chain) => | ||
| SDK_PACKAGES.some((sdk) => chain.includes(sdk)), | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Get the currently installed version from node_modules or yarn.lock. | ||
| */ | ||
| function getCurrentVersion(pkg: string): string | null { | ||
| const nmPath = path.join( | ||
| process.cwd(), | ||
| 'node_modules', | ||
| ...pkg.split('/'), | ||
| 'package.json', | ||
| ); | ||
| if (fs.existsSync(nmPath)) { | ||
| try { | ||
| const pj = JSON.parse(fs.readFileSync(nmPath, 'utf-8')); | ||
| return pj.version ?? null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * Get ALL installed versions of a package across the entire node_modules tree. | ||
| * Catches nested/duplicate copies that the hoisted-only check would miss. | ||
| */ | ||
| 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]; | ||
| } |
There was a problem hiding this comment.
3. Npm ls truncation misclassifies 🐞 Bug ≡ Correctness
runCmd() caps output at 10MB and getAllInstalledVersions() silently returns [] on JSON parse failure, which can cause nested vulnerable copies to be missed and the CVE to be reported as already-remediated based only on the hoisted version. This can produce false assurance and skipped remediation on branches with large dependency trees.
Agent Prompt
## Issue description
`npm ls --json` output can exceed the 10MB buffer and/or be emitted alongside non-fatal errors; today the script suppresses parse errors and treats the result as "no versions found", potentially falling back to a single hoisted version and incorrectly concluding the CVE is already remediated.
## Issue Context
This script is intended to drive automated CVE remediation decisions; missing a nested vulnerable copy is a correctness failure.
## Fix Focus Areas
- scripts/fix-cves/analyze-deps.ts[54-63]
- scripts/fix-cves/analyze-deps.ts[141-149]
- scripts/fix-cves/analyze-deps.ts[269-277]
## Suggested fix approach
- Increase `maxBuffer` significantly (or make it configurable).
- Change `getAllInstalledVersions` to surface parse/truncation failure explicitly (e.g., return `{ versions: string[], error?: string }`).
- In `main()`, if the full-tree scan fails, **do not** fall back to `currentVersion` and mark `already-remediated`; instead emit `strategy: 'triage-needed'` (or a new strategy like `analysis-failed`) with a clear `reason` and include raw stderr/stdout snippets for debugging.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
The full npm ls --all --json output for this project is ~760KB, well under the 10MB buffer. The skill also runs a separate verification step (Phase 2d) that would catch any missed copies.
Comment on lines
+162
to
+169
| ```bash | ||
| git checkout <branch> | ||
| git pull upstream <branch> | ||
| rm -rf node_modules yarn.lock | ||
| yarn install | ||
| ``` | ||
|
|
||
| **Critical**: Always remove both `node_modules` and `yarn.lock` and reinstall to regenerate the dependency graph from scratch and avoid stale resolutions. |
There was a problem hiding this comment.
5. Deletes yarn.lock in workflow 🐞 Bug ☼ Reliability
The skill instructs deleting yarn.lock on every branch sync, which forces a full dependency re-resolution and can upgrade many unrelated packages, creating large/noisy PRs and potentially breaking release branches. This undermines the goal of targeted CVE remediation and makes results harder to review and reproduce.
Agent Prompt
## Issue description
The skill’s “clean install” step removes `yarn.lock`, which broadens changes beyond the intended CVE fixes.
## Issue Context
This repo uses Yarn 4 and checks in `yarn.lock` for reproducible installs.
## Fix Focus Areas
- .cursor/skills/fix-cves/SKILL.md[162-170]
## Suggested fix approach
- Update the workflow to:
- Remove only `node_modules` (optional) but **keep** `yarn.lock`.
- Prefer `yarn install --immutable` right after checkout/pull to ensure the lockfile matches the branch.
- Only regenerate `yarn.lock` as a result of intentional remediation commands (`yarn up ...`, adding `resolutions`, etc.).
- If you still want a “full refresh” mode, document it as an explicit opt-in step, not the default.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
Removing yarn.lock is intentional, tested without removing it - produced false results due to stale resolutions. Clean install is needed for accurate analysis.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Cursor AI skill and a TypeScript analysis script to automate CVE remediation across console-plugin release branches, with Jira integration and FedRAMP image triage.
Files Added
.cursor/skills/fix-cves/SKILL.mdscripts/fix-cves/analyze-deps.tsscripts/fix-cves/tsconfig.jsonts-nodeexecutionSetup
1. Install tools
ghCLIgh auth logingit remote -v— you needupstream(main repo) andorigin(your fork)2. Install Cursor plugins
To install:
Settings → Plugins → Browse Marketplace3. Configure Cursor Agent settings
Go to
Cursor Settings → Agentsand configure the following:Auto-Run Mode
Set to
Allowlist (with Sandbox)or higher.Command Allowlist
Add these commands so the skill can run them without prompting:
MCP Allowlist
Add the following so the skill can interact with Jira automatically:
Fetch Domain Allowlist
Add these domains:
github.com— PR creation and git push (Modes A & B)access.redhat.com,catalog.redhat.com— Red Hat APIs (Mode C)Auto-Run Network Access
sandbox.json + Defaultsworks for most cases. If Red Hat API calls are blocked, switch toAllow All.Usage
Open Cursor Agent chat and use one of the following prompts:
Mode A — Paste Jira descriptions (no Atlassian plugin needed)
Mode B — Fetch from Jira (requires Atlassian plugin)
Mode C — FedRAMP image CVE triage (no plugins needed)
Remediation Strategies
The analysis script inspects the dependency tree and returns one of:
already-remediateddirect-upgradeyarn upparent-upgraderesolutionresolutions(last resort)triage-neededJira Automation (Mode B)
@mentionnotificationsyarn why/npm lsevidence as bash code blocksWorkflow
yarn build/yarn testDemo
Screen recording
CVE-fix-demo.mp4
Screenshot of Jira comment added