Add node-cve plugin for daily Node team CVE triage#488
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds a complete ChangesNode CVE Plugin
Sequence Diagram(s)The high-level query-analyze-report flow is captured in the Phase diagram above; individual skill orchestration is not separately diagrammed as each skill performs a well-scoped role documented in its SKILL.md. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
863abca to
39c6760
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/node-cve/.claude-plugin/plugin.json`:
- Around line 5-7: The plugin.json currently has an "author" object; replace
that object with the required string value so the author field is exactly
"github.com/openshift-eng" (i.e., change the "author" entry in plugin.json from
an object to the literal string "github.com/openshift-eng").
In `@plugins/node-cve/commands/triage.md`:
- Around line 153-158: The Summary block currently lists four outcome buckets
but the classification elsewhere defines five; update the Summary in the
triage.md file to include a dedicated "PRESENT_NOT_EXPLOITABLE" entry (count and
list) alongside "Reachable", "Present but not reachable", "Not affected", and
"Uncertain" so that totals align with the 5-way classification referenced at
lines with "PRESENT_NOT_EXPLOITABLE"; mirror formatting used for other buckets
and ensure downstream tooling reading the Summary can rely on the same bucket
name "PRESENT_NOT_EXPLOITABLE".
- Around line 214-216: Replace the real-person names in the triage examples for
CVE-2026-41326, CVE-2026-32281, and CVE-2026-35469 with neutral role-based
placeholders (e.g., "<assignee>" or "Unassigned") so lines that currently show
"Prabhakar P.", "(unassigned)", and "Shannon P." become standardized
placeholders; update the three CVE rows in triage.md accordingly and scan the
surrounding example rows to ensure no other real names remain.
- Around line 63-67: The JQL output doesn't include due date but later steps
(lines referenced 160 and 226) require "due within 7 days"; update the jira
query invocation (the jira issue list command with --columns
KEY,SUMMARY,STATUS,ASSIGNEE,LABELS) to include the Jira due date field (e.g.,
add duedate or DUE depending on the CLI) and then update the parsing/processing
instructions that reference "due within 7 days" (lines 160 and 226) to consume
that new due date column and apply the date comparison, or alternatively remove
the due-date reporting/highlighting requirements if you prefer not to include
due dates. Ensure you change both the --columns list and the downstream
parsing/requirements so they agree.
- Around line 209-221: The summary output block in the triage markdown is using
an unlabeled fenced code block which triggers MD040; update the fence used for
the summary table (the triple-backtick block labeled "Node CVE Triage -
YYYY-MM-DD") to specify a language of text by replacing the opening ``` with
```text so the block is recognized as plain text; locate the unlabeled fence in
the triage.md summary output section and change only the opening fence to
```text (leave the contents and closing fence unchanged).
In `@plugins/node-cve/README.md`:
- Line 101: Replace the typo "Node / Device Manage" in the README table with
"Node / Device Manager"; locate the table row containing the exact string "Node
/ Device Manage" and update that cell so the component name reads "Node / Device
Manager" for consistency.
In `@plugins/node-cve/skills/query-open-cves/SKILL.md`:
- Line 40: The JIRA query in the SKILL.md command (the jira issue list line)
doesn't include the due date, so update the command to add the due date column
(e.g., include DUE or duedate in the --columns list) and then update the parsing
contract described in "Step 3" to extract and validate that due-date field
(ensure the parser expects the new column name and treats empty/malformed dates
safely for the 7-day urgency check).
In `@plugins/node-cve/skills/report-findings/SKILL.md`:
- Around line 27-35: Add a separate metric row for the "Present not exploitable"
classification (PRESENT_NOT_EXPLOITABLE) to the report summary table so it
appears alongside Reachable/Not affected/etc., and update the report
aggregation/listing logic that computes counts and builds the lists (the
codepath that currently references PRESENT_NOT_EXPLOITABLE elsewhere) to
increment and surface this metric rather than collapsing it into another bucket;
ensure "Total unique CVEs" and any per-category lists include the
PRESENT_NOT_EXPLOITABLE items so totals remain accurate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fd0ac4b7-b25f-4902-83b3-41f1f4ad07ad
📒 Files selected for processing (10)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/node-cve/.claude-plugin/plugin.jsonplugins/node-cve/OWNERSplugins/node-cve/README.mdplugins/node-cve/commands/triage.mdplugins/node-cve/skills/analyze-cve-repos/SKILL.mdplugins/node-cve/skills/query-open-cves/SKILL.mdplugins/node-cve/skills/report-findings/SKILL.md
39c6760 to
2d3207d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/data.json (1)
1699-1699: ⚡ Quick winUse generic placeholders instead of specific example values in synopsis.
The
argument_hintandsynopsisuse a specific example value"Node / CRI-O"instead of a generic placeholder. For consistency with other commands in this file, consider using a generic placeholder like<name>or<component>.Similarly,
[--days 7]might be misinterpreted as showing a default value, when it appears to be an example. Consider[--days N]to match the pattern used elsewhere (e.g., line 115, line 696).📝 Suggested revision
- "argument_hint": "[--component <name>] [--notify-jira] [--notify-slack] [--days N]", + "argument_hint": "[--component <name>] [--notify-jira] [--notify-slack] [--days N]", "description": "Triage all open CVEs for OpenShift Node team components with reachability analysis", "example": "", "name": "triage", - "synopsis": "/node-cve:triage [--component \"Node / CRI-O\"] [--notify-jira] [--notify-slack] [--days 7]" + "synopsis": "/node-cve:triage [--component <name>] [--notify-jira] [--notify-slack] [--days N]"Also applies to: 1703-1703
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/data.json` at line 1699, The synopsis/argument_hint entries use a concrete example value ("Node / CRI-O") and a specific example for days ("[--days 7]"); update the "argument_hint" and corresponding "synopsis" fields to use generic placeholders instead—e.g., replace "Node / CRI-O" with "<component>" or "<name>" and change "[--days 7]" to "[--days N]" so they match the generic placeholder style used elsewhere and avoid implying a default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/node-cve/skills/analyze-cve-repos/SKILL.md`:
- Around line 46-52: The git clone example in SKILL.md does not enforce the
required 600s cutoff; wrap the git clone of .work/node-cve/repos/<repo-name> in
a timeout wrapper (e.g., the system timeout utility) so the clone is killed if
it exceeds 600 seconds, and explicitly document that callers must check the
clone exit status and classify the repo as UNCERTAIN on timeout/failure; update
the example in SKILL.md (the snippet containing the git clone command) to show
the timeout usage and the follow-up check that maps non-zero/timeout exit codes
to UNCERTAIN.
In `@plugins/node-cve/skills/query-open-cves/SKILL.md`:
- Around line 49-54: Step 3 currently extracts issue key, CVE, OCP version,
status and assignee but omits parsing the issue labels; update Step 3 to
explicitly parse the issue's labels field and include it on the output record
(preserving the labels array or normalized keys), ensuring pscomponent:* labels
are retained for Phase 2 repo mapping; apply the same label extraction change
wherever the Step 3 logic is duplicated (the other occurrence referenced in the
diff) so downstream mapping can rely on label-based repo mapping.
---
Nitpick comments:
In `@docs/data.json`:
- Line 1699: The synopsis/argument_hint entries use a concrete example value
("Node / CRI-O") and a specific example for days ("[--days 7]"); update the
"argument_hint" and corresponding "synopsis" fields to use generic placeholders
instead—e.g., replace "Node / CRI-O" with "<component>" or "<name>" and change
"[--days 7]" to "[--days N]" so they match the generic placeholder style used
elsewhere and avoid implying a default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a5b92478-40c0-4c0e-9838-5268b51a9847
📒 Files selected for processing (10)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/node-cve/.claude-plugin/plugin.jsonplugins/node-cve/OWNERSplugins/node-cve/README.mdplugins/node-cve/commands/triage.mdplugins/node-cve/skills/analyze-cve-repos/SKILL.mdplugins/node-cve/skills/query-open-cves/SKILL.mdplugins/node-cve/skills/report-findings/SKILL.md
✅ Files skipped from review due to trivial changes (5)
- plugins/node-cve/OWNERS
- plugins/node-cve/.claude-plugin/plugin.json
- plugins/node-cve/skills/report-findings/SKILL.md
- PLUGINS.md
- plugins/node-cve/README.md
2d3207d to
36405c1
Compare
c377f69 to
7afa9a5
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/data.json`:
- Around line 1696-1728: The change edits the auto-generated docs/data.json (the
"node-cve" entry and its "commands"/"skills"), which must not be modified by
hand; revert the manual edits to docs/data.json and instead update the plugin
metadata in plugins/<name>/.claude-plugin/plugin.json (bump the plugin "version"
if code changes) and the command descriptions/examples in the plugin's markdown
frontmatter, then run make update (which calls scripts/build-website.py) to
regenerate docs/data.json so the "node-cve" commands, synopsis, and skills are
produced correctly.
In `@plugins/node-cve/skills/analyze-cve-repos/SKILL.md`:
- Around line 44-45: Change the triage fallback behavior: do not fall back to
upstream when a downstream fork or branch is missing; instead mark the tracker
as UNCERTAIN and skip further analysis. Update the logic described by the
sentence "If the downstream fork or branch does not exist, fall back to the
upstream repo's equivalent branch" in the analyze-cve-repos skill (SKILL.md) so
it instead classifies as UNCERTAIN and aborts analysis for that tracker; ensure
any code paths or functions implementing the downstream-to-upstream fallback
(the downstream branch/fork existence check and fallback decision) are adjusted
to return an UNCERTAIN classification and stop processing rather than switching
source repositories.
In `@plugins/node-cve/skills/query-open-cves/SKILL.md`:
- Around line 56-70: The deduplication step currently collapses multiple
affected components into a single "component" field causing loss of coverage;
update the logic described in "Step 4: Deduplicate by CVE ID" to preserve all
affected components (e.g., use "components": [] or a map from tracker key to
component) instead of a single "component" value, and ensure the generated
record for each CVE includes either an array "components" listing every unique
component seen across tracker_keys or a per-tracker mapping (retain
"tracker_keys" and attach component info) so multi-component CVEs are not lost.
- Around line 39-41: The jira CLI query string used in SKILL.md must include
COMPONENT in the --columns flag and the downstream parsing must extract that
column into the returned record; update the command invocation (the jira issue
list ... --columns KEY,SUMMARY,STATUS,ASSIGNEE,LABELS) to add COMPONENT and
modify the parsing/record-construction logic that consumes the CLI output so it
reads the COMPONENT field (in the same position as the other columns) and
assigns it to the record's "component" key so the returned record is fully
reconstructible (apply the same change to the other occurrence of the
query/parse block).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5b6da372-2d00-4106-8256-ba7eeed3b3b3
📒 Files selected for processing (10)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/node-cve/.claude-plugin/plugin.jsonplugins/node-cve/OWNERSplugins/node-cve/README.mdplugins/node-cve/commands/triage.mdplugins/node-cve/skills/analyze-cve-repos/SKILL.mdplugins/node-cve/skills/query-open-cves/SKILL.mdplugins/node-cve/skills/report-findings/SKILL.md
✅ Files skipped from review due to trivial changes (6)
- plugins/node-cve/.claude-plugin/plugin.json
- plugins/node-cve/OWNERS
- plugins/node-cve/README.md
- plugins/node-cve/skills/report-findings/SKILL.md
- PLUGINS.md
- plugins/node-cve/commands/triage.md
8021c31 to
ef7eb8c
Compare
ef7eb8c to
d3ba3dd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
plugins/node-cve/skills/analyze-cve-repos/SKILL.md (1)
8-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove upstream fallback from analysis source selection.
This reintroduces the downstream-accuracy risk: if downstream fork/branch is missing, analysis should classify
UNCERTAINand stop, not switch to upstream source-of-truth.Also applies to: 19-20, 44-44
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/node-cve/skills/analyze-cve-repos/SKILL.md` at line 8, Update the analysis source-selection logic to remove any upstream fallback so that when a downstream fork/branch is missing the analysis for that CVE-branch immediately classifies the result as UNCERTAIN and stops; locate and change the behavior currently implementing "downstream with upstream fallback" (references: the node-cve:triage Phase 2 selection logic and any helpers that choose repository source) to instead treat absent downstream sources as terminal UNCERTAIN outcomes, and apply the same change to the other two occurrences noted (the blocks corresponding to lines referenced as 19-20 and 44-44 in the diff).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/node-cve/skills/query-open-cves/SKILL.md`:
- Around line 80-85: The Step 5 heading in SKILL.md claims "Identify unassigned
and urgent CVEs" but the text only implements unassigned logic; either rename
the heading to "Identify unassigned CVEs" (or "Identify unassigned CVEs (Status
= New)") or add explicit urgency criteria to the step (for example: define what
"urgent" means—Status = "New", CVSS score threshold, or age in days—and list the
checks). Update the Step 5 title and the bulleted criteria so the heading and
the implemented logic match exactly (refer to the Step 5 section in SKILL.md to
locate and modify the title and bullets).
---
Duplicate comments:
In `@plugins/node-cve/skills/analyze-cve-repos/SKILL.md`:
- Line 8: Update the analysis source-selection logic to remove any upstream
fallback so that when a downstream fork/branch is missing the analysis for that
CVE-branch immediately classifies the result as UNCERTAIN and stops; locate and
change the behavior currently implementing "downstream with upstream fallback"
(references: the node-cve:triage Phase 2 selection logic and any helpers that
choose repository source) to instead treat absent downstream sources as terminal
UNCERTAIN outcomes, and apply the same change to the other two occurrences noted
(the blocks corresponding to lines referenced as 19-20 and 44-44 in the diff).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 40675941-9d3a-433a-aff9-fb9127cb5502
📒 Files selected for processing (10)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/node-cve/.claude-plugin/plugin.jsonplugins/node-cve/OWNERSplugins/node-cve/README.mdplugins/node-cve/commands/triage.mdplugins/node-cve/skills/analyze-cve-repos/SKILL.mdplugins/node-cve/skills/query-open-cves/SKILL.mdplugins/node-cve/skills/report-findings/SKILL.md
✅ Files skipped from review due to trivial changes (6)
- plugins/node-cve/OWNERS
- .claude-plugin/marketplace.json
- plugins/node-cve/skills/report-findings/SKILL.md
- plugins/node-cve/README.md
- plugins/node-cve/commands/triage.md
- PLUGINS.md
caa04ce to
499237c
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: saschagrunert 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 |
499237c to
1524532
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/node-cve/skills/analyze-cve-repos/SKILL.md`:
- Line 8: Update the skill summary sentence that currently reads "prefers
downstream forks with upstream fallback" to reflect the actual implementation:
remove the upstream-fallback claim and state that analysis prefers downstream
forks and will classify a CVE-branch as "Uncertain" when the downstream fork or
branch does not exist (no automatic fallback to upstream). Ensure the revised
sentence mentions the preference for downstream forks and the Uncertain
classification behavior to match the logic implemented in the analyze step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7184c506-fc2e-4d65-b49d-2de808bee6df
📒 Files selected for processing (9)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/node-cve/.claude-plugin/plugin.jsonplugins/node-cve/OWNERSplugins/node-cve/README.mdplugins/node-cve/commands/triage.mdplugins/node-cve/skills/analyze-cve-repos/SKILL.mdplugins/node-cve/skills/query-open-cves/SKILL.mdplugins/node-cve/skills/report-findings/SKILL.md
✅ Files skipped from review due to trivial changes (4)
- plugins/node-cve/OWNERS
- plugins/node-cve/skills/report-findings/SKILL.md
- plugins/node-cve/README.md
- plugins/node-cve/skills/query-open-cves/SKILL.md
1524532 to
c657361
Compare
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
c657361 to
b05fab5
Compare
What this PR does / why we need it:
New
node-cveplugin that automates CVE triage for OpenShift Node team components./node-cve:triagequeries all open OCPBUGS Vulnerability issues across 10 Node team components, deduplicates by CVE ID, clones affected repositories at version-specific release branches, and uses Claude to analyze source code for reachability. Optionally posts findings to Jira trackers and sends a Slack summary.claude --printand OpenShift CronJob schedulingWhich issue(s) this PR fixes:
Special notes for your reviewer:
This plugin adds 3 skills (query-open-cves, analyze-cve-repos, report-findings) orchestrated by the
triagecommand. The analysis spawns parallel agents for each CVE and branch combination. No external tooling beyondjiraCLI andgitis required; reachability analysis is performed by Claude reading source code directly.Checklist:
Summary by CodeRabbit
/node-cve:triagecommand with configurable component filtering and analysis periods