Make CVE fix skill self-contained - #2443
Conversation
|
🤖 Created branch: z_pr2443/dfarrell07/fix-state-expiring-ignores |
|
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:
WalkthroughAdds optional auto-expiring CVE ignores, self-fix worktree support, version-aware stdlib and package handling, environment isolation for builds, and updated CVE review guidance. ChangesCVE fix workflow updates
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
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 |
facbc6d to
4ca5179
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@scripts/cve/review-prompt.md`:
- Around line 31-35: Run markdownlint on the changed markdown file
(scripts/cve/review-prompt.md) and fix any lint issues before committing;
specifically ensure line lengths, list formatting, code span/backtick usage, and
emphasis conventions comply with the project's markdownlint rules. Re-run
markdownlint until it reports no errors, then re-stage the file and update the
PR; reference the content that mentions CVE_SCRIPTS, ignore.sh, and STATE_FILE
while making the fixes to avoid altering meaning.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c784d464-67ee-473d-803b-9a21a0d4c6c9
📒 Files selected for processing (4)
.agents/workflows/cve-fix.mdscripts/cve/ignore.shscripts/cve/lib.shscripts/cve/review-prompt.md
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 `@scripts/cve/fix-stdlib.sh`:
- Around line 34-35: The script currently forces GOTOOLCHAIN=auto even when the
caller explicitly set GOTOOLCHAIN (e.g., local); change the condition so we only
set GOTOOLCHAIN=auto when it is unset. Replace the test [[ "${GOTOOLCHAIN:-}" !=
*auto* ]] with an explicit unset check (e.g., [[ -z "${GOTOOLCHAIN:-}" ]]) while
keeping the existing OLD_MINOR==NEW_MINOR guard so that only when OLD_MINOR and
NEW_MINOR match and GOTOOLCHAIN is not set do we export GOTOOLCHAIN=auto.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5ceaeef5-5f71-4101-958e-add487377300
📒 Files selected for processing (1)
scripts/cve/fix-stdlib.sh
dfarrell07
changed the title
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further |
|
🤖 Closed branches: [z_pr2443/dfarrell07/fix-state-expiring-ignores] |
|
🤖 Created branch: z_pr2443/dfarrell07/fix-state-expiring-ignores |
8dfbdb2 to
bad9688
Compare
dfarrell07
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/cve/fix-stdlib.sh (1)
63-65: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDon't override an explicitly configured
GOTOOLCHAINvalue.This condition matches explicit values like
GOTOOLCHAIN=local, forcingautoeven when the caller intentionally set a different mode. Per the official Go documentation,GOTOOLCHAIN=localmeans the go command always runs the bundled toolchain—overriding this silently breaks offline and reproducible build environments. Additionally, the override at line 63 causes the host Go check at line 70 to be skipped (sinceautomatches*auto*), masking insufficient host Go versions.This was previously flagged in an earlier review commit and remains unaddressed.
🔧 Proposed fix
# Compiler already has the fix version — bump go.mod to match -if [[ "${GOTOOLCHAIN:-}" != *auto* ]]; then +if [[ -z "${GOTOOLCHAIN:-}" ]]; then export GOTOOLCHAIN=auto fi🤖 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 `@scripts/cve/fix-stdlib.sh` around lines 63 - 65, The GOTOOLCHAIN handling in the stdlib fix script is overriding explicit caller configuration, especially GOTOOLCHAIN=local, by forcing auto. Update the check around the GOTOOLCHAIN export so it only sets auto when GOTOOLCHAIN is unset, and preserves any explicitly provided value in the script’s main flow before the host Go version check. Use the existing GOTOOLCHAIN guard logic near the compiler/version setup to keep the host Go check effective.
🤖 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.
Duplicate comments:
In `@scripts/cve/fix-stdlib.sh`:
- Around line 63-65: The GOTOOLCHAIN handling in the stdlib fix script is
overriding explicit caller configuration, especially GOTOOLCHAIN=local, by
forcing auto. Update the check around the GOTOOLCHAIN export so it only sets
auto when GOTOOLCHAIN is unset, and preserves any explicitly provided value in
the script’s main flow before the host Go version check. Use the existing
GOTOOLCHAIN guard logic near the compiler/version setup to keep the host Go
check effective.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9722505b-a40f-46e6-a8b8-c89a59052616
📒 Files selected for processing (10)
.agents/workflows/cve-fix.mdscripts/cve/detect.shscripts/cve/fix-all.shscripts/cve/fix-package.shscripts/cve/fix-stdlib.shscripts/cve/ignore.shscripts/cve/lib.shscripts/cve/review-prompt.mdscripts/cve/scan.shscripts/cve/test-lib.sh
✅ Files skipped from review due to trivial changes (2)
- .agents/workflows/cve-fix.md
- scripts/cve/review-prompt.md
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/cve/ignore.sh
bad9688 to
3b153bb
Compare
eea6171 to
992b8db
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
992b8db to
702d58f
Compare
dfarrell07
changed the title
702d58f to
c1baaea
Compare
Move scripts/cve/ into skills/cve-fix/ so the skill is portable. Add verification subagents (yaml-integrity, fix-availability, scan-regression, existing-entries) with a verify-fix retry loop. - repos.yaml with paths for name-based repo resolution - Makefile with fix, all, clean, test targets - detect.sh resolves bare names via repos.yaml - insert_grype_ignore dup detection: grep -A4 (was -A2) - fix-availability: use real CVE IDs, skip already-bumped packages Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
c1baaea to
e22d525
Compare
Remove all Submariner-specific assumptions. The skill now works for any Go project with zero config — just go.mod and grype. - Add .cve-fix.yaml support for project-specific overrides (build-image, build/clean/test commands, k8s-version-guard) - Rename SHIPYARD_* → BUILD_* throughout - Auto-detect build/test/clean commands from Makefile - Fall back to host Go when no build image configured - K8s version guard is opt-in via config - Upstream org detected from git remote - Remove hardcoded quay.io/submariner image references - existing-entries.sh reads skip list from .grype.yaml exclude section instead of hardcoding CVE IDs Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
Move no-fix CVE handling from the LLM review agent to the deterministic fix loop. CVEs with no fix version are now auto- ignored with --no-fix (auto-expires when a fix is published). The review agent is now conditional (skipped when all CVEs are handled deterministically) and focused on diagnosing exit-2 (breaking change) and exit-3 (CVE persists) failures only. Also filter scan-regression gate to go-module CVEs only (npm/Python CVEs are out of scope) and use lowercase worktree paths for Docker compatibility. Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
3 participants
Move scripts/cve/ into skills/cve-fix/ so the skill is portable.
Add verification subagents (yaml-integrity, fix-availability,
scan-regression, existing-entries) with a verify-fix retry loop.