Skip to content

fix node-cve: add cross-team component validation safeguards to triage#623

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift-eng:mainfrom
asahay19:component-fix
Jul 17, 2026
Merged

fix node-cve: add cross-team component validation safeguards to triage#623
openshift-merge-bot[bot] merged 2 commits into
openshift-eng:mainfrom
asahay19:component-fix

Conversation

@asahay19

@asahay19 asahay19 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Add cross-team component validation safeguards to node-cve:triage to prevent Node-specific CVE analysis from being posted to non-Node team Jira trackers.

On 2026-07-15, /node-cve:triage --notify-jira posted Node-specific reachability analysis for CVE-2026-27136 to non-Node OCPBUGS trackers (HyperShift, Storage, Networking/PTP, Installer, Monitoring, Cloud providers, Security, Windows Containers, etc.). Root cause: at posting time, trackers were looked up by CVE ID alone, without a component filter, so all of that CVE's multi-team trackers were treated as a single batch to comment on.

This PR adds two independent layers of defense:

query-open-cves :

  • Makes explicit that omitting --component means "all Node team components," never "no filter" — the component in (...) clause in the JQL query is now mandatory in every invocation, whether or not --component was passed.
  • Adds a sanity check after querying: any returned tracker whose component is outside the Node team list is dropped and logged as a warning, to catch JQL construction bugs.

report-findings:

  • Adds a mandatory "Node Team Component Safeguard" validation gate: immediately before posting any comment, each tracker's component is re-checked against the Node team list, independent of whatever Phase 1 already filtered. This is defense-in-depth so a stale cache, manual re-run, or ad-hoc script can't bypass it.
  • Explicitly documents the anti-pattern that caused the incident (CVE-ID-only Jira search, ad-hoc "batch posting scripts" that bypass the normal filtered tracker list) so it isn't repeated.
  • Adds a posting audit log (posting-audit.log) recording every tracker skipped and why, and surfaces a warning in the command summary output when any trackers are skipped.

Both safeguards reference the canonical Node team component list in the node-team shared reference (plugins/node-team/skills/node/references/shared/components.md) rather than duplicating a hardcoded list, so they can't drift out of sync as Node team components change.

Also bumps node-cve to 0.0.6 and syncs marketplace/docs metadata.

Which issue(s) this PR fixes:

N/A — prompted by an internal incident (2026-07-15), not a filed issue.

Special notes for your reviewer:

  • An earlier internal fix proposal suggested hardcoding a 3-component allowlist (Node / CRI-O, Node / kubelet, Node / cri-tools). That list is incomplete — the real Node team component list has ~14 Jira components (CPU manager, Memory manager, Topology manager, Device Manager, Kueue, etc.) plus pscomponent: label mappings (cadvisor, conmon, conmon-rs, cri-tools). This PR instead points both safeguards at the existing canonical shared reference so the safeguard itself doesn't become a new source of drift.
  • These are Claude-instruction skills (Markdown), not executable code, so "validation" is enforced via explicit, repeated instructions plus a posting audit log for visibility — not a unit-testable function. Manual testing is recommended with a known multi-component CVE (e.g. CVE-2026-27136) to confirm only Node trackers receive comments and everything else is logged as skipped.

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Strengthened CVE/Jira safeguards to ensure only validated Node team components are queried and notified.
    • Added clearer handling for mismatched trackers, including user-visible warnings and skip behavior.
    • When Jira notifications are enabled, now generates a posting-audit.log with posted vs. skipped tracker details.
  • Documentation

    • Updated node-cve:triage and related skills with explicit, non-optional component scoping and re-validation guidance.
  • Chores

    • Updated the node-cve plugin version to 0.0.6.

CVE-2026-27136 (a multi-team CVE) had Node-specific analysis posted to
~200 non-Node OCPBUGS trackers (HyperShift, Storage, Networking, etc.)
because a downstream posting step re-searched Jira by CVE ID alone,
without a component filter.

Add two layers of defense: query-open-cves now explicitly requires the
Node team component filter even when --component is omitted (no flag
means "all Node components", never "no filter"), and report-findings
re-validates each tracker's component immediately before posting any
comment, independent of upstream filtering, skipping and audit-logging
anything that isn't a Node team component. Both layers reference the
canonical node-team shared components list rather than a hardcoded
copy, so they can't drift out of sync.

Bump node-cve to 0.0.6 and sync marketplace/docs metadata.

Co-authored-by: Cursor <cursoragent@cursor.com>
@openshift-ci
openshift-ci Bot requested review from mrunalp and rphillips July 15, 2026 14:04
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b17efe3a-bb75-4d27-b1c2-115e9278a2f2

📥 Commits

Reviewing files that changed from the base of the PR and between face5df and 05af321.

📒 Files selected for processing (1)
  • plugins/node-cve/skills/report-findings/SKILL.md

Walkthrough

The node-cve plugin version is updated to 0.0.6. Its documentation now requires Node-team Jira component filtering, validates returned trackers and posting targets, skips non-Node trackers, and records posting results in posting-audit.log.

Changes

Node CVE safeguards

Layer / File(s) Summary
Plugin version alignment
.claude-plugin/marketplace.json, docs/index.html, plugins/node-cve/.claude-plugin/plugin.json
The node-cve version is updated from 0.0.5 to 0.0.6 across plugin metadata and embedded catalog data.
Query-time component scoping
plugins/node-cve/README.md, plugins/node-cve/commands/triage.md, plugins/node-cve/skills/query-open-cves/SKILL.md
Query instructions require Node-team component filters, validate --component, and exclude unexpected returned trackers.
Posting validation and audit
plugins/node-cve/README.md, plugins/node-cve/commands/triage.md, plugins/node-cve/skills/report-findings/SKILL.md
Posting instructions require per-tracker validation, skip and log non-Node trackers, warn on skips, and document posting-audit.log plus the skipped-tracker return field.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: mrunalp, rphillips

🚥 Pre-merge checks | ✅ 10
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding cross-team component validation safeguards to node-cve triage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Real People Names In Style References ✅ Passed Checked the changed docs/skills/metadata files; no real-person names are used as style references or example prompts.
No Assumed Git Remote Names ✅ Passed PASS: The touched node-cve docs use explicit URLs/placeholders and only generic "upstream" prose; no git commands assume origin or upstream remotes.
Git Push Safety Rules ✅ Passed PASS: The PR only updates Jira/docs metadata; changed files contain no git push, force-push, or main/master push instructions.
No Untrusted Mcp Servers ✅ Passed The only changed file is a markdown skill doc; it adds no MCP server installs, npm/npx fetches, or new dependencies.
Ai-Helpers Overlap Detection ✅ Passed No overlapping open PRs touched the node-cve paths, and the closest existing docs (node-bug triage/Jira skills) were below the similarity threshold.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/commands/triage.md`:
- Line 32: Coordinate this documentation change with the implementation in open
PR `#623` before proceeding. Consolidate the overlapping triage-scope updates into
one authoritative change, or adjust this change to avoid duplicating or
conflicting with PR `#623`.
🪄 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: 722d9a9c-63e8-4c46-8a1a-18ac9b717368

📥 Commits

Reviewing files that changed from the base of the PR and between 766a907 and face5df.

📒 Files selected for processing (7)
  • .claude-plugin/marketplace.json
  • docs/index.html
  • plugins/node-cve/.claude-plugin/plugin.json
  • plugins/node-cve/README.md
  • plugins/node-cve/commands/triage.md
  • plugins/node-cve/skills/query-open-cves/SKILL.md
  • plugins/node-cve/skills/report-findings/SKILL.md

Comment thread plugins/node-cve/commands/triage.md
# Node team component list from the shared components reference (link above),
# NOT a hardcoded list.
for tracker_key in $TRACKER_KEYS; do
component=$(jira issue view "$tracker_key" --plain --no-headers --columns COMPONENT | tail -1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two jira issue view calls (lines 99-100) each make a separate API call per tracker. These can be combined into one:

output=$(jira issue view "$tracker_key" --plain --no-headers --columns COMPONENT,LABELS | tail -1)

This halves the validation API calls. Also, the loop has no sleep 1 between calls, but the existing rate limit instruction (line 159) says "sleep 1 second between Jira API calls." Since the agent will likely follow this pseudocode literally, adding sleep 1 inside the loop would prevent 429s. For a CVE with 30 trackers, the current pseudocode fires 60 unthrottled API calls before posting even starts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks sascha. I have Added sleep 1 between validation calls and made posting-audit.log conditional on --notify-jira in the artifacts list.

done
```

A component is considered a Node team component only if it matches an entry in the "Jira Components (OCPBUGS)" list (plus Driver Toolkit, Machine Config Operator) or one of the `pscomponent:` labels in the shared components reference. If a tracker's component cannot be confidently classified as Node team, **skip it and log the reason** — never post "just in case." This check must run even when `--component` was passed, and even if Phase 1 already filtered by component, since this is the last line of defense before an irreversible write to another team's tracker.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pscomponent: label check creates a false-pass risk in the posting-time validation. A tracker with component "Security" (non-Node) but label pscomponent:cri-o would pass this check and get posted to, which is exactly the cross-team contamination this PR prevents.

Phase 1 already filters by component in (...), so any tracker reaching Phase 3 should have a Node team component. The defense-in-depth re-validation should verify the COMPONENT field only, not offer pscomponent labels as an alternative pass condition. The pscomponent labels are for Phase 2 repo mapping (which repo to clone), not for determining tracker ownership.

".work/node-cve/triage-2026-05-20/report.md",
".work/node-cve/triage-2026-05-20/cves.json"
".work/node-cve/triage-2026-05-20/cves.json",
".work/node-cve/triage-2026-05-20/posting-audit.log"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

posting-audit.log is listed unconditionally in the artifacts array, but it's only written when --notify-jira is used (see Step 2, line 163). A run without --notify-jira would have the agent report a nonexistent file. Either note this artifact is conditional on --notify-jira, or move it out of the example and into a note below it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in the latest commit, please check.

Per @saschagrunert's review on PR openshift-eng#623:
- Drop the pscomponent: label as an alternative pass condition in the
  posting-time validation. A non-Node tracker (e.g. component
  "Security") could incidentally carry a pscomponent:cri-o label,
  which would silently reintroduce the cross-team contamination this
  safeguard exists to prevent. Validate the COMPONENT field only.
- Removing the label check also removes the second per-tracker jira
  API call; add sleep 1 between validation calls to match the
  existing rate-limit convention used for posting calls.
- Only list posting-audit.log as an artifact when --notify-jira was
  used, since it's not written otherwise.

Co-authored-by: Cursor <cursoragent@cursor.com>
@saschagrunert

Copy link
Copy Markdown
Contributor

/test images

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 16, 2026
@openshift-ci

openshift-ci Bot commented Jul 16, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: asahay19, saschagrunert

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 16, 2026
@saschagrunert

Copy link
Copy Markdown
Contributor

/retest

@asahay19

Copy link
Copy Markdown
Contributor Author

/retest-required

@saschagrunert

Copy link
Copy Markdown
Contributor

Images CI seems to be broken all over the place, unfortunately I can't fix that now.

@asahay19

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-merge-bot
openshift-merge-bot Bot merged commit 2a19861 into openshift-eng:main Jul 17, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants