Repository navigation
feat(go-pr-analysis): let callers gate on govulncheck instead of only logging it - #947
Conversation
… logging it The "Run govulncheck" step carried a bare `continue-on-error: true`, so a known vulnerability in a consumer's dependency graph left the Security job green and the finding sat in a job log nobody opens on a passing check. Removing the flag outright would be a break for every repository on the channel: the Go vulnerability database moves independently of the pull request, so a newly published CVE in a common dependency would turn them all red at once, on pull requests touching nothing related — the same shape of breakage x/vuln v1.8.0 already caused here. So the step now branches on the exit code instead. Findings are always annotated: a warning by default, an error — failing the job — when the new `fail_on_govulncheck` input is true. The default keeps today's behaviour, and a repository that has cleaned its graph can opt into the gate on its own schedule. Closes #910
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe Go analysis and validation workflows add a Changesgovulncheck enforcement
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~12 minutes Change: Feature · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: allowing callers to make govulncheck findings fail the workflow instead of only logging them. |
| Description check | |
The description follows the required template, explains the behavior change and compatibility impact, identifies testing performed, and links the change to issue #910. It clearly states that no real c… |
| Linked Issues check | |
Issue #910 requires govulncheck findings to be able to fail CI instead of being hidden by continue-on-error: true. The PR adds fail_on_govulncheck to both reusable workflows, forwards the input, r… |
| Out of Scope Changes check | |
The changes are limited to the two Go reusable workflow interfaces, govulncheck result handling, and documentation for the new input. These changes directly implement issue #910. The documented adviso… |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
Full details: Description check
-
Explanation
The description follows the required template, explains the behavior change and compatibility impact, identifies testing performed, and links the change to issue
#910. It clearly states that no real caller workflow run was completed.
Full details: Linked Issues check
-
Explanation
Issue
#910requires govulncheck findings to be able to fail CI instead of being hidden bycontinue-on-error: true. The PR addsfail_on_govulncheckto both reusable workflows, forwards the input, reports findings as warnings when false, and reports errors that fail the Security job when true. The default remainsfalse, which preserves compatibility while providing the requested opt-in gate. The PR also reports YAML andactionlintvalidation.
Full details: Out of Scope Changes check
-
Explanation
The changes are limited to the two Go reusable workflow interfaces, govulncheck result handling, and documentation for the new input. These changes directly implement issue
#910. The documented advisory behavior for GoSec and Trivy remains unchanged. No unrelated change is demonstrated.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
lerian-studio
added
size/XS
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
🔍 Lint Analysis
|
🛡️ CodeQL Analysis ResultsLanguages analyzed: ✅ No security issues found. 🔍 View full scan logs | 🛡️ Security tab |
|
@coderabbitai review |
lerian-studio
added
the
review-ready
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
2 participants
GitHub Actions Shared Workflows
Description
go-pr-analysis.ymlran govulncheck with a barecontinue-on-error: true. A known vulnerability in a consumer's dependency graph therefore left the Security job green, and the finding stayed in a job log nobody opens on a passing check — every Go repository on the channel read "security passed" without proof that it had.Removing the flag outright would be a break for every consumer at once. The Go vulnerability database moves independently of the pull request, so the day a reachable CVE lands in a common dependency, every repository on the channel goes red on pull requests that touch nothing related — the same shape of breakage
x/vulnv1.8.0 already caused here, as thegovulncheck_versioninput description records.So the step branches on the exit code instead of swallowing it:
fail_on_govulncheckinput (boolean,default: false) ingo-pr-analysis.yml, forwarded bygo-pr-validation.yml.false(today's behaviour): the job still passes, but the finding is now emitted as a::warning::annotation instead of being buried in the log.true: the finding is emitted as::error::and the Security job fails, which propagates throughchecks_passedto the Go Analysis gate.A repository opts into the gate once its dependency graph is clean, which is the sequence the issue asks for — run the analysis against the consumers and fix or accept what it reports before flipping anything globally.
Out of scope, deliberately:
make sec/ gosec-no-failin the same job and the Trivyexit-code: '0'inpr-security-scan.ymlremain advisory, and the default here staysfalse. Flipping the channel-wide default is a separate, evidence-backed decision.Type of Change
feat: New workflow or new input/output/step in an existing workflowfix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)perf: Performance improvement (e.g. caching, parallelism, reduced steps)refactor: Internal restructuring with no behavior changedocs: Documentation only (README, docs/, inline comments)ci: Changes to self-CI (workflows under.github/workflows/that run on this repo)chore: Dependency bumps, config updates, maintenancetest: Adding or updating testsBREAKING CHANGE: Callers must update their configuration after this PRBreaking Changes
None.
fail_on_govulncheckdefaults tofalse, which is exactly the pass/fail behaviour the step had before. The only observable difference for a caller that changes nothing is a warning annotation where there was previously silence.Testing
@this-branchor the beta tagactionlintclean on both modified workflows. The caller-side value isfail_on_govulncheck: trueon a repository with a known finding; this PR's own self-CI exercises the default path.Caller repo / workflow run:
Related Issues
Closes #910