Add fix-cve skill to golang plugin for Go CVE patching - #470
Conversation
openshift-ci
Bot
added
the
needs-rebase
WalkthroughThis PR adds a complete ChangesCVE Patching Command and Skill
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
openshift-ci
Bot
removed
the
needs-rebase
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@plugins/deps/commands/fix-cve.md`:
- Around line 12-14: The fenced code blocks containing the directive
"/deps:fix-cve module=\"google.golang.org/grpc\" fix-version=... cve=...
ticket=..." lack language identifiers and trigger markdownlint MD040; update
each fence (including the occurrences around the shown diff and at the other
locations noted, e.g., lines 54-61 and 65-67) to include a language tag such as
"text" (i.e., change ``` to ```text) so the blocks are properly identified while
leaving the directive content unchanged.
- Around line 44-45: Update the "PR creation" step of the fix-cve command to add
explicit push/PR safety gates: require an interactive confirmation before any
git push or PR creation (no implicit pushes), block pushes/PRs to main or master
branches unless a specific override is explicitly provided and confirmed,
prohibit any force-push unless a clearly named allow-force flag (e.g.,
--allow-force) is set and confirmed interactively, and require explicit remote
selection (do not assume "origin") or prompt the user to confirm the remote;
document the new flags and prompts in the PR creation section of fix-cve.md and
ensure the implementation checks these guards before running git push or
creating a PR.
In `@plugins/deps/skills/fix-cve/SKILL.md`:
- Around line 67-70: The markdown in SKILL.md has multiple fenced code blocks
missing language identifiers (MD040); update each triple-backtick fence shown
around the examples (the blocks containing "// <CVE number> replace
<original-module> => <fork-module> <fork-version>", the "fix(deps): <action>
<module-name> to fix <CVE>" example, the "[<branch>] <TICKET>: fix <CVE> by
<action> <module-name>" line, and the /deps:fix-cve
module="google.golang.org/grpc" ... example) to include a language tag such as
text (e.g., change ``` to ```text) for all affected ranges (67-70, 142-149,
161-163, 194-201) so the markdown linter MD040 is satisfied.
- Around line 158-170: The PR guidance in Step 6 of SKILL.md currently allows
push/PR actions without safety checks; update the Step 6 text and any associated
workflow logic so that before any push/PR the tool must prompt and obtain
explicit user permission, explicitly forbid pushing to branch names "main" or
"master", disallow any force-push semantics, and avoid assuming remote names
(require the user to confirm or supply the remote). Locate and modify the Step 6
section and any code that auto-executes git push/pr actions referenced by the PR
format examples to add these checks and clear user prompts.
🪄 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: 6fca6ae4-351d-423e-b12c-bd548794ca3f
📒 Files selected for processing (7)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/deps/.claude-plugin/plugin.jsonplugins/deps/README.mdplugins/deps/commands/fix-cve.mdplugins/deps/skills/fix-cve/SKILL.md
| "version": "0.1.2" | ||
| }, | ||
| { | ||
| "name": "deps", |
There was a problem hiding this comment.
The footprint is really small, does it need its own plugin? Could it go to a new cve plugin, or maybe golang ? I expect we'll have more in this space
There was a problem hiding this comment.
I’ve put it in there, because I though it’s not a pure CVE plugin perse, it’s not fixing a CVE, but patching deps for a CVE fix xD. W/e is fine to me tbh :). Let me know to reformulate the best suitable way.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ error (plugin-owners-required): Plugin 'golang' has an OWNERS file but is still in the exclude list — remove it from the exclusion
There was a problem hiding this comment.
THere's an exclusion in .skillsaw.yaml if you'll add an OWNERS for this plugin
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/golang/skills/fix-cve/SKILL.md (1)
82-85: ⚡ Quick winConsider a more precise grep pattern to identify affected modules.
The current pattern
grep "<module>" path/to/go.modmay match the module name in comments, as part of another module's name, or in unrelated contexts. A more targeted pattern would reduce false positives and unnecessary processing.♻️ Suggested improvement
-For each `go.mod`, check if it references the target module: -```bash -grep "<module>" path/to/go.mod -``` +For each `go.mod`, check if it actually depends on or replaces the target module: +```bash +grep -E "^\s*(require|replace).*<module>" path/to/go.mod +```This anchors the search to
requireorreplacedirectives at the start of lines (after optional whitespace), avoiding matches in comments or as substrings of other module paths.🤖 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/golang/skills/fix-cve/SKILL.md` around lines 82 - 85, Update the grep pattern used to detect references to the target module in go.mod so it only matches dependency directives, not comments or substrings: replace the current naive grep of "<module>" with a anchored regex that looks for require or replace directives (e.g., grep -E "^\s*(require|replace).*<module>" path/to/go.mod) so the check targets actual require/replace lines in go.mod rather than any occurrence of "<module>".
🤖 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 1724-1728: Replace the placeholder description "|" for the skill
with id "fix-cve" / name "fix-cve" by adding a clear, actionable description
that explains what the skill does, when to use it, expected inputs and outputs,
and any preconditions; locate the JSON object containing "id": "fix-cve" and
update its "description" field to a meaningful sentence or two similar in level
of detail to nearby golang plugin entries so users understand the command's
purpose and usage.
---
Nitpick comments:
In `@plugins/golang/skills/fix-cve/SKILL.md`:
- Around line 82-85: Update the grep pattern used to detect references to the
target module in go.mod so it only matches dependency directives, not comments
or substrings: replace the current naive grep of "<module>" with a anchored
regex that looks for require or replace directives (e.g., grep -E
"^\s*(require|replace).*<module>" path/to/go.mod) so the check targets actual
require/replace lines in go.mod rather than any occurrence of "<module>".
🪄 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: f9587b71-b2e1-4aaa-a953-62128116b363
📒 Files selected for processing (8)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/golang/.claude-plugin/plugin.jsonplugins/golang/OWNERSplugins/golang/README.mdplugins/golang/commands/fix-cve.mdplugins/golang/skills/fix-cve/SKILL.md
✅ Files skipped from review due to trivial changes (5)
- plugins/golang/OWNERS
- plugins/golang/.claude-plugin/plugin.json
- plugins/golang/commands/fix-cve.md
- plugins/golang/README.md
- PLUGINS.md
There was a problem hiding this comment.
THere's an exclusion in .skillsaw.yaml if you'll add an OWNERS for this plugin
There was a problem hiding this comment.
Claude and Cursor have both de-emphasized commands in favor of skills, as it usually doesn't make sense to split them. I have also seen issues where skills and commands with the same name have Claude confused, it may make sense to just publish this as one skill.
Skills are invocable as a "/" command for some time in Claude.
There was a problem hiding this comment.
Makes sense. Consolidating both :)
|
One optional comment, but the lint needs to be fixed about OWNERS, otherwise lgtm. Thank you! |
ab76093 to
d4be312
Compare
Signed-off-by: Jose Parrill <jparrill@redhat.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
jparrill
changed the title
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill, stbenjam 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 |
openshift-ci
Bot
added
the
approved
2 participants
Summary
/golang:fix-cveskill that automates patching Go module dependencies to fix CVEsBackground
This skill was born from hands-on CVE patching work on the hypershift repo:
Test plan
make lint— passes clean (0 errors, 0 warnings)make update— PLUGINS.md and docs/data.json regenerated correctly/golang:fix-cveappears in the skill list after plugin installation🤖 Generated with Claude Code