Repository navigation
sec(ci): install golang-migrate via module-hash-pinned go install instead of unverified tarball (closes #434) - #537
Conversation
Replace the unverified `curl | tar` pipe with a two-step download + sha256 verification before extraction. The official sha256sum.txt published with the v4.17.0 release is used as the source of truth. `set -euo pipefail` ensures the step aborts immediately if the checksum check fails. Closes #434
cristim
added
triaged
|
Warning Review limit reached
More reviews will be available in 51 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe workflow file Changesgolang-migrate Download Security Hardening
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
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)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/database-migration.yml (1)
147-157: ⚖️ Poor tradeoffConsider extracting the golang-migrate installation into a composite action or using
go installfor better maintainability.The installation logic is duplicated identically across all three cloud provider jobs (AWS, GCP, Azure). While the current approach significantly improves security, the duplication creates a maintenance burden—any version bump or checksum update must be applied in three places.
Two alternatives to consider:
Extract to a composite action: Create
.github/actions/install-golang-migrate/action.ymlthat accepts version and SHA256 as inputs, reducing duplication and centralizing the installation logic.Use
go install(preferred): As suggested in issue#434, install via a pinned Go module eliminates tarball/checksum management entirely:- name: Install golang-migrate run: | go install -tags 'postgres' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.17.0This leverages Go's module checksums (go.sum) for integrity verification and is already used elsewhere in your CI.
Also applies to: 226-236, 295-305
🤖 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/database-migration.yml around lines 147 - 157, The golang-migrate install block (env MIGRATE_VERSION / MIGRATE_SHA256 and the run: curl|sha256sum|tar|sudo install sequence) is duplicated across provider jobs; replace it by either (A) extracting that logic into a composite action (e.g., .github/actions/install-golang-migrate/action.yml) that accepts MIGRATE_VERSION and MIGRATE_SHA256 inputs and performs the curl/verify/install steps, then call that action from each job; or (B, preferred) remove the tarball steps and replace the run block with a go install invocation that pins the module (e.g., go install -tags 'postgres' github.com/golang-migrate/migrate/v4/cmd/migrate@${MIGRATE_VERSION}) so CI relies on Go module checksums instead of manual tarball download; update the three occurrences (the run blocks referenced) accordingly.
🤖 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.
Nitpick comments:
In @.github/workflows/database-migration.yml:
- Around line 147-157: The golang-migrate install block (env MIGRATE_VERSION /
MIGRATE_SHA256 and the run: curl|sha256sum|tar|sudo install sequence) is
duplicated across provider jobs; replace it by either (A) extracting that logic
into a composite action (e.g.,
.github/actions/install-golang-migrate/action.yml) that accepts MIGRATE_VERSION
and MIGRATE_SHA256 inputs and performs the curl/verify/install steps, then call
that action from each job; or (B, preferred) remove the tarball steps and
replace the run block with a go install invocation that pins the module (e.g.,
go install -tags 'postgres'
github.com/golang-migrate/migrate/v4/cmd/migrate@${MIGRATE_VERSION}) so CI
relies on Go module checksums instead of manual tarball download; update the
three occurrences (the run blocks referenced) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e514ebd-e240-411f-9558-8512c96a3c66
📒 Files selected for processing (1)
.github/workflows/database-migration.yml
…nstall All three cloud provider jobs in database-migration.yml had identical curl/sha256/tar/install blocks. Replaced each with setup-go + go install using the version already pinned in go.mod (v4.19.1), leveraging Go module checksums for integrity and eliminating the manual tarball approach.
|
Addressed the duplicated golang-migrate installation in ece1b9d: replaced all three identical curl/sha256/tar/install blocks with |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
cat .github/workflows/database-migration.ymlLength of output: 12612
A few observations from the review: ✅ What's well done:
Minor note (non-blocking): One thing to verify (if not already done): confirm that Overall the PR is in great shape — this is the cleaner of the two alternatives from ✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
cristim
changed the title
1 participant
Closes #434.
Replaces the unverified
curl -L .../migrate.linux-amd64.tar.gz | tar xvzinstall of golang-migrate (RCE / supply-chain risk on runners holding DB passwords + cloud creds) with:actions/setup-go@v5(go-version-file: go.mod), thengo install -tags 'postgres' github.com/golang-migrate/migrate/v4/cmd/migrate@v4.19.1in all three (AWS/GCP/Azure) migration jobs. Integrity is enforced by the Go toolchain via
go.sum(module hash pinned:h1:OCyb44lFuQfYXYLx1SCxPZQGU7mcaZ7gH9yH4jSFbBA=) + GOSUMDB, which fails closed on mismatch. Version is pinned to@v4.19.1(no@latest), matching the approach already used inci.yml. This is issue #434's option (b) (module-hash-pinned go install), which is at least as strong as the tarball-sha256 option and consistent with existing CI.Note: bumps golang-migrate v4.17.0 -> v4.19.1.
(PR title/body corrected to match the actual implementation: an earlier draft described a tarball-sha256-verify approach that was not what landed.)