ci: add vuln scanning + module verify, and streamline the test workflow#409
Conversation
Add a vulncheck job that runs govulncheck against the dependency graph, reporting only vulnerabilities reachable from our code, and a `go mod verify` step in the build job to catch tampering of the restored module cache before builds trust it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe CI workflow is restructured to consolidate Go test jobs into a single parameterized matrix, removing intermediate job dependencies. A new ChangesCI Test Suite and Security Consolidation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Confidence Score: 5/5This looks safe to merge.
Reviews (3): Last reviewed commit: "ci: pin govulncheck to v1.3.0" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 137-140: The vulncheck job is missing a timeout-minutes
specification which could cause it to hang indefinitely if govulncheck
encounters issues. Add a timeout-minutes field to the vulncheck job definition
(at the same indentation level as name, runs-on, and steps) with a reasonable
timeout value to prevent the job from running until the default 360-minute
limit.
- Around line 141-147: Pin the actions/checkout and actions/setup-go actions to
their full commit SHA hashes instead of version tags (currently `@v6`) to prevent
tag-hijacking attacks. Additionally, add persist-credentials: false to the
actions/checkout step to prevent the GITHUB_TOKEN from being accessible to
potentially malicious code. These changes address the supply-chain security gaps
flagged by static analysis tools.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f75effec-766f-47a8-8ec2-d834203817f4
📒 Files selected for processing (1)
.github/workflows/test.yml
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@v6 | ||
| with: | ||
| go-version: ${{ env.GO_VERSION }} | ||
| cache: true |
There was a problem hiding this comment.
Harden action references and disable credential persistence.
The static analysis tool flagged supply-chain security gaps: actions are not pinned to commit hashes and credentials are persisted to disk. In a PR adding supply-chain controls, these gaps should be addressed.
🔒 Hardening recommendations
- Pin actions to commit SHA hashes instead of tags to prevent tag-hijacking attacks
- Set
persist-credentials: falseto prevent the GITHUB_TOKEN from being accessible to malicious code
steps:
- - uses: actions/checkout@v6
+ - uses: actions/checkout@v6 # Pin to commit SHA: actions/checkout@<commit-hash>
+ with:
+ persist-credentials: false
- name: Set up Go
- uses: actions/setup-go@v6
+ uses: actions/setup-go@v6 # Pin to commit SHA: actions/setup-go@<commit-hash>
with:
go-version: ${{ env.GO_VERSION }}
cache: trueTo get commit hashes: visit the action repository's releases page and copy the full commit SHA for the desired version.
Note: The same pattern exists throughout this workflow. Consider hardening all jobs in a follow-up PR to establish consistent supply-chain security practices.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 141-141: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 141-141: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 144-144: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/test.yml around lines 141 - 147, Pin the actions/checkout
and actions/setup-go actions to their full commit SHA hashes instead of version
tags (currently `@v6`) to prevent tag-hijacking attacks. Additionally, add
persist-credentials: false to the actions/checkout step to prevent the
GITHUB_TOKEN from being accessible to potentially malicious code. These changes
address the supply-chain security gaps flagged by static analysis tools.
Source: Linters/SAST tools
The unit, e2e, integration, and contract jobs shared identical checkout/setup-go/cache boilerplate and differed only in the test command, so they now run as a single matrix job. Check names are preserved (Unit Tests, E2E Tests, Integration Tests, Contract Replay Tests) so required status checks keep matching. The build job no longer waits on the test jobs — compiling the binary doesn't depend on tests passing, so it now runs in parallel and shortens the critical path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on the vulncheck job: - Add timeout-minutes: 10 so a hung govulncheck can't run to the 360-minute default. - Add a second govulncheck run with -tags=swagger to cover the swagger-tagged production build (swagger_enabled.go), which the default tag set excludes. The tests/* suites behind e2e/integration/ contract tags are intentionally not scanned — they aren't shipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SantiagoDePolonia
changed the title
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 @.github/workflows/test.yml:
- Around line 127-130: The govulncheck tool installation uses `@latest` which
makes CI behavior non-reproducible and susceptible to upstream changes. In the
run block where govulncheck is installed, replace the `@latest` version specifier
with an explicit version pin like `@v1.3.0` in the go install
golang.org/x/vuln/cmd/govulncheck command on line 128. This ensures consistent
and reproducible CI behavior, and allows intentional updates to be made when
necessary rather than drifting with upstream releases.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 75da3ad2-5b3e-4164-8bcb-c8d59e01cf4c
📒 Files selected for processing (1)
.github/workflows/test.yml
Replace @latest with an explicit version so the vulnerability gate is reproducible and doesn't drift with upstream releases. v1.3.0 is the current latest; bump intentionally when updating. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/test.yml (1)
42-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a job-level timeout to
go-tests.
go-testscurrently has notimeout-minutes, so a hung command can run until the platform default limit. Add an explicit job timeout (for example, 20–30 minutes) to bound failures.Also applies to: 67-68
🤖 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 @.github/workflows/test.yml around lines 42 - 47, The go-tests job lacks a timeout-minutes configuration, which means hung commands can run until the platform default limit. Add a timeout-minutes field to the go-tests job definition at the job level (alongside the existing runs-on, strategy, and other top-level job properties) with a value between 20 and 30 minutes to establish an explicit upper bound on execution time.
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
26-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin all GitHub Actions to commit SHAs and disable checkout credential persistence.
This supply-chain hardening gap is still present:
uses:references are tag-based andactions/checkoutdoes not setpersist-credentials: false. Please harden all jobs consistently.#!/bin/bash # Verify unpinned action refs and checkout credential persistence in this workflow. set -euo pipefail FILE=".github/workflows/test.yml" echo "== Tag-based uses refs (not SHA-pinned) ==" rg -n 'uses:\s*[^@]+@v?[0-9]+' "$FILE" || true echo echo "== checkout steps missing persist-credentials: false ==" python - <<'PY' import re, pathlib p = pathlib.Path(".github/workflows/test.yml") lines = p.read_text().splitlines() for i,l in enumerate(lines, start=1): if re.search(r'uses:\s*actions/checkout@', l): window = "\n".join(lines[i:i+8]) # next ~8 lines if "persist-credentials: false" not in window: print(f"Line {i}: checkout without persist-credentials: false") PYAlso applies to: 59-63, 82-86, 96-100, 113-117, 136-144, 157-161
🤖 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 @.github/workflows/test.yml around lines 26 - 27, The GitHub Actions workflow has two security hardening issues that need to be fixed across all workflow jobs. First, replace all tag-based action references (like `@v6`, `@v4`) with their corresponding commit SHAs for both actions/checkout and actions/setup-go to prevent supply-chain attacks. Second, add `persist-credentials: false` configuration to every actions/checkout step to disable credential persistence. These changes must be applied consistently to all occurrences throughout the workflow file, including the ones at lines 59-63, 82-86, 96-100, 113-117, 136-144, and 157-161, not just the ones shown in the diff.
🤖 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.
Outside diff comments:
In @.github/workflows/test.yml:
- Around line 42-47: The go-tests job lacks a timeout-minutes configuration,
which means hung commands can run until the platform default limit. Add a
timeout-minutes field to the go-tests job definition at the job level (alongside
the existing runs-on, strategy, and other top-level job properties) with a value
between 20 and 30 minutes to establish an explicit upper bound on execution
time.
---
Duplicate comments:
In @.github/workflows/test.yml:
- Around line 26-27: The GitHub Actions workflow has two security hardening
issues that need to be fixed across all workflow jobs. First, replace all
tag-based action references (like `@v6`, `@v4`) with their corresponding commit SHAs
for both actions/checkout and actions/setup-go to prevent supply-chain attacks.
Second, add `persist-credentials: false` configuration to every actions/checkout
step to disable credential persistence. These changes must be applied
consistently to all occurrences throughout the workflow file, including the ones
at lines 59-63, 82-86, 96-100, 113-117, 136-144, and 157-161, not just the ones
shown in the diff.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 58a47c76-9e48-4b5f-b254-8cff1244b1c4
📒 Files selected for processing (1)
.github/workflows/test.yml
2 participants
Summary
Consolidated CI improvements to
test.yml(originally split across #409 + #410, now combined here):Supply-chain controls
vulncheckjob runsgovulncheck ./...plus a second-tags=swaggerpass, so the swagger-tagged production build (swagger_enabled.go) — excluded by the default tag set — is also scanned. Thetests/*suites behinde2e/integration/contracttags are intentionally not scanned; they aren't in the shipped binary.timeout-minutes: 10on the vulncheck job so a hung scan can't run to the 360-minute default.go mod verifystep in thebuildjob — verifies the restored module cache againstgo.sumbefore the build trusts it.Workflow simplification
Unit Tests,E2E Tests,Integration Tests,Contract Replay Tests) collapse into a singlego-testsmatrix. Check names are preserved, so required-status-check rules keep matching.buildjob is no longer gated behind the test jobs — compiling the binary doesn't depend on tests passing, so it runs in parallel and shortens the critical path.User-visible impact
None at runtime — CI-only. Faster feedback and a vulnerability gate on PRs.
Notes
go mod verifychecks cache ↔ go.sum; the go.sum ↔ upstream trust is covered byGOSUMDB(on by default) + PR review ofgo.sumdiffs.persist-credentials: false(flagged by zizmor/CodeRabbit) is a repo-wide pattern and will be addressed in a dedicated follow-up PR across all workflows.🤖 Generated with Claude Code
Summary by CodeRabbit
Chores