Switch to govulncheck for Go scanning#2382
Conversation
|
🤖 Created branch: z_pr2382/skitt/govulncheck |
skitt
requested review from
Oats87,
mkolesnik,
sridhargaddam,
tpantelis and
vthapar
as code owners
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 44 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
WalkthroughReplace Anchore-based vulnerability scanning with Go's govulncheck: add Make targets to run and aggregate govulncheck (text and SARIF), install govulncheck in the build image and tools go.mod, update workflows to run govulncheck (including a new periodic SARIF uploader), remove the Anchore report workflow, and ignore generated SARIF. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Actions Runner
participant Repo as Repository (checkout)
participant Make as Makefile (govulncheck.sarif / govulncheck)
participant Govuln as govulncheck (per-module)
participant Upload as upload-sarif Action
Runner->>Repo: checkout branch/ref
Runner->>Make: run `make BUILD_UPX=false govulncheck.sarif`
Make->>Govuln: run `govulncheck -format sarif ./...` (per module)
Govuln-->>Make: return SARIF runs (merged into govulncheck.sarif)
Make-->>Runner: produce `govulncheck.sarif`
Runner->>Upload: upload `govulncheck.sarif` (category: govulncheck-<branch>, include ref/sha)
Upload-->>Runner: upload result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/linting.yml:
- Around line 114-120: The two govulncheck steps named "Run govulncheck
(producing a SARIF report)" and "Run govulncheck (producing a text report and
failing the build if necessary)" only run at the repo root and miss submodule
go.mod files—fix by either replacing both steps to invoke the Makefile target
(run `make govulncheck`) so the Makefile.linting logic enumerates all go.mod
files, or implement a matrix job that sets `with.work-dir` to each module
directory and `with.cache-dependency-path` to the corresponding go.sum for each
matrix entry so the golang/govulncheck-action scans every module; update the
steps' `uses`/`with` accordingly.
In `@Makefile.linting`:
- Around line 36-38: The loop currently does cd "$$module" in the main shell
which changes the working directory for subsequent iterations; run each module
scan in its own subshell so the main loop's cwd is preserved. Replace the
sequence that uses cd "$$module" && govulncheck ./... with a subshell invocation
that runs cd "$$module" and govulncheck inside parentheses, and keep the failure
assignment to the vulns variable (i.e., ensure the || vulns=1 still applies if
the subshell fails). This touches the loop that begins with for module in
$$(find . -name go.mod -printf "%h\n"); do and the commands using cd "$$module",
govulncheck ./..., and the vulns variable.
🪄 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: 0f5316f0-5cd0-4d81-8a2a-4ea68201d012
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
.github/workflows/linting.ymlMakefile.lintingpackage/Dockerfile.shipyard-dapper-basetools/go.mod
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/go.mod (1)
227-227: Consider aligning the tool management pattern with existing tools.The
tooldirective for govulncheck differs from the import-based pattern used for other tools. Intools/tools.go(lines 24-29), golangci-lint, go-mod-outdated, and kind are declared via import statements, but govulncheck lacks a corresponding import. While thetooldirective is the modern Go 1.22+ approach and is fully supported in Go 1.25, the mixed pattern may cause confusion.Consider either:
- Adding a corresponding import to
tools/tools.gofor consistency with existing tools, or- Documenting the intentional shift to
tool-directive-only for new tools🔧 Option 1: Add import to tools/tools.go for consistency
In
tools/tools.go, add:import ( _ "github.com/golangci/golangci-lint/v2/cmd/golangci-lint" _ "github.com/psampaz/go-mod-outdated" _ "sigs.k8s.io/kind/cmd/kind" + _ "golang.org/x/vuln/cmd/govulncheck" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/go.mod` at line 227, The tool declaration for govulncheck in go.mod is inconsistent with the import-based pattern used for other tools; add an underscore import for govulncheck in the same import block in tools/tools.go (e.g., _ "golang.org/x/vuln/cmd/govulncheck") so it follows the existing tools' pattern (golangci-lint, go-mod-outdated, kind), or if you intentionally prefer the new tool directive approach, add a short comment in tools/tools.go explaining that govulncheck is managed via the go 1.22+ tool directive instead of an import to avoid confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tools/go.mod`:
- Line 227: The tool declaration for govulncheck in go.mod is inconsistent with
the import-based pattern used for other tools; add an underscore import for
govulncheck in the same import block in tools/tools.go (e.g., _
"golang.org/x/vuln/cmd/govulncheck") so it follows the existing tools' pattern
(golangci-lint, go-mod-outdated, kind), or if you intentionally prefer the new
tool directive approach, add a short comment in tools/tools.go explaining that
govulncheck is managed via the go 1.22+ tool directive instead of an import to
avoid confusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7612abbf-b037-4a82-9e6a-dd58716b8597
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
.github/workflows/linting.ymlMakefile.lintingpackage/Dockerfile.shipyard-dapper-basetools/go.mod
🚧 Files skipped from review as they are similar to previous changes (3)
- package/Dockerfile.shipyard-dapper-base
- .github/workflows/linting.yml
- Makefile.linting
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/report.yml (1)
23-28:⚠️ Potential issue | 🟠 MajorSARIF scan likely misses submodules when action runs once at repo root.
Line 23 runs
golang/govulncheck-actiona single time with default module scope. In a multi-go.modrepo, this can under-report vulnerabilities compared withMakefile.linting:32-43, which scans every module.Use a module matrix (
work-dir+ matchingcache-dependency-path) or equivalent per-module invocation so SARIF coverage matches actual repo layout.For golang/govulncheck-action v1.0.4, when run once from repository root with default work-dir, does it scan all nested Go modules or only the module containing work-dir? What is the recommended monorepo configuration?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/report.yml around lines 23 - 28, The SARIF scan runs the golang/govulncheck-action only once at the repo root so nested Go modules can be missed; update the workflow step "Run govulncheck (producing a SARIF report)" to invoke golang/govulncheck-action per-module (either via a strategy/matrix over module paths or by iterating modules) and set the action inputs work-dir to each module path and cache-dependency-path to the module's go.sum (or equivalent) so each module is scanned and cached correctly; use the action golang/govulncheck-action@b625fbe08f3bccbe446d94fbf87fcc875a4f50ee and ensure output-format: sarif and output-file remain set for each per-module run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/linting.yml:
- Around line 111-113: Remove the unnecessary permissions entry
"security-events: write" from the permissions block in the linting workflow;
keep only the minimal required scope (e.g., "contents: read") since the job only
runs "make govulncheck" and does not upload SARIF or use security-events, so
delete the "security-events: write" line to tighten permissions.
---
Duplicate comments:
In @.github/workflows/report.yml:
- Around line 23-28: The SARIF scan runs the golang/govulncheck-action only once
at the repo root so nested Go modules can be missed; update the workflow step
"Run govulncheck (producing a SARIF report)" to invoke golang/govulncheck-action
per-module (either via a strategy/matrix over module paths or by iterating
modules) and set the action inputs work-dir to each module path and
cache-dependency-path to the module's go.sum (or equivalent) so each module is
scanned and cached correctly; use the action
golang/govulncheck-action@b625fbe08f3bccbe446d94fbf87fcc875a4f50ee and ensure
output-format: sarif and output-file remain set for each per-module run.
🪄 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: 93df23b8-629b-445e-8e7f-ace21bcc45cb
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
.github/workflows/linting.yml.github/workflows/report.ymlMakefile.lintingpackage/Dockerfile.shipyard-dapper-basetools/go.modtools/tools.go
💤 Files with no reviewable changes (1)
- tools/tools.go
✅ Files skipped from review due to trivial changes (3)
- tools/go.mod
- Makefile.linting
- package/Dockerfile.shipyard-dapper-base
skitt
force-pushed
the
govulncheck
branch
2 times, most recently
from
845ae7a to
19de466
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/periodic.yml:
- Around line 75-88: The workflow checks out the repo into the submariner/ path
but then runs make and uploads govulncheck.sarif from the repo root; update the
"Run govulncheck (producing a SARIF report)" step to run in the checked-out
directory (set working-directory: submariner) and update the "Upload SARIF
report" step to point to the correct SARIF path (sarif_file:
submariner/govulncheck.sarif) or set working-directory: submariner for that
step, keeping the existing step names and the matrix.branch usage intact.
🪄 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: 3ed2d908-c593-4cdb-9d29-fd86a64d8bb4
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.github/workflows/linting.yml.github/workflows/periodic.yml.github/workflows/report.yml.gitignoreMakefile.lintingpackage/Dockerfile.shipyard-dapper-basetools/go.modtools/tools.go
💤 Files with no reviewable changes (2)
- tools/tools.go
- .github/workflows/report.yml
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- tools/go.mod
- Makefile.linting
skitt
force-pushed
the
govulncheck
branch
2 times, most recently
from
cd84b15 to
0a2c7cb
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
@CodeRabbit approve |
✅ Actions performedComments resolved and changes approved. |
|
What about the .grype.yaml? |
Good point, we no longer need it! |
skitt
force-pushed
the
govulncheck
branch
2 times, most recently
from
b224658 to
e6f0630
Compare
|
I think this will affect @dfarrell07 CVE fix agent workflows. |
Removing
|
skitt
force-pushed
the
govulncheck
branch
2 times, most recently
from
2dec214 to
aea922c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile.linting`:
- Around line 48-51: The current loop runs govulncheck per module and appends
multiple full SARIF documents into govulncheck.sarif, producing invalid JSON;
change it to produce one SARIF per module into temporary files (e.g., per
"$module"), ensure govulncheck failures don't break the loop (use || true or
capture exit), clear or recreate govulncheck.sarif before the loop, and then
merge the per-module SARIF files by programmatically combining their "runs"
arrays into a single SARIF object and writing that into govulncheck.sarif;
reference the loop variable module, the govulncheck command, and
govulncheck.sarif when making these changes.
🪄 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: 970f1690-63b1-464b-a669-70b669e750e7
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.github/workflows/linting.yml.github/workflows/periodic.yml.github/workflows/report.yml.gitignoreMakefileMakefile.lintingpackage/Dockerfile.shipyard-dapper-basetools/go.mod
💤 Files with no reviewable changes (1)
- .github/workflows/report.yml
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- tools/go.mod
- .github/workflows/periodic.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- package/Dockerfile.shipyard-dapper-base
- Makefile
- .github/workflows/linting.yml
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai approve |
✅ Actions performedComments resolved and changes approved. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/periodic.yml:
- Around line 81-86: The upload-sarif step ("Upload SARIF report" using
github/codeql-action/upload-sarif) omits ref and sha so SARIFs are attributed to
the workflow trigger ref; add explicit inputs to the step: set ref to the branch
under test (e.g. refs/heads/${{ matrix.branch }}) and set sha to the scanned
commit (e.g. ${{ matrix.sha }} or the matrix commit variable you populate) so
each matrix iteration uploads SARIF tied to that branch/commit.
🪄 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: 9a63c180-9f0d-4002-8b0a-ae01082d2fa3
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.github/workflows/linting.yml.github/workflows/periodic.yml.github/workflows/report.yml.gitignoreMakefileMakefile.lintingpackage/Dockerfile.shipyard-dapper-basetools/go.mod
💤 Files with no reviewable changes (1)
- .github/workflows/report.yml
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- tools/go.mod
🚧 Files skipped from review as they are similar to previous changes (3)
- package/Dockerfile.shipyard-dapper-base
- Makefile.linting
- Makefile
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
govulncheck only reports vulnerabilities in reachable packages, which will greatly reduce the number of false positives. This changes CI linting to use govulncheck, and also provides local govulncheck and govulncheck-sarif targets in the linting Makefile. PR checks no longer upload SARIF reports, and SARIF uploads are changed to periodic rather than per-push. This ensures that security issues are caught in a timely fashion even if no development activity occurs in the repository. See https://words.filippo.io/dependabot/ for background. govulncheck segfaults if it is compressed with UPX, so it is skipped when compressing. Signed-off-by: Stephen Kitt <skitt@redhat.com>
Yes, exactly - we want the CI to fail on actually-relevant CVEs and the CVE workflow to be aware of all CVEs that might be reported by scanners. Thanks @skitt! I had been thinking about this too since the filippo Tweet you linked in a different PR. |
|
🤖 Closed branches: [z_pr2382/skitt/govulncheck] |
5 participants
govulncheck only reports vulnerabilities in reachable packages, which will greatly reduce the number of false positives.
This changes CI linting to use govulncheck, uploading SARIF reports as usual, and also provides a local govulncheck target in the linting Makefile.
See https://words.filippo.io/dependabot/ for background.
Summary by CodeRabbit
New Features
Chores
Chore