Fix JWT secret validation and deployment guidance#24
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPinning and configuration updates: default APP_BASE_URL changed to use port 8080; Go and golang-migrate tool versions pinned to 1.24.7 and v4.19.1 across Dockerfile, Makefile, and CI; CI JWT_SECRET value adjusted; a unit test’s token-tampering logic was modified; docs updated to reflect version pins. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (4 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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
🧹 Nitpick comments (1)
Makefile (1)
77-79: Consider pinning all setup tools consistently.
migrateis pinned to v4.19.1, butgoimportsandswaguse@latest, allowing setup to drift over time. Stable versions are available (goimports v0.43.0, swag v1.16.3) and could be pinned for reproducibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 77 - 79, The Makefile currently pins migrate to v4.19.1 but installs goimports and swag with `@latest`, which allows unpredictable changes; update the go install lines for golang.org/x/tools/cmd/goimports and github.com/swaggo/swag/cmd/swag to use fixed versions (e.g., goimports v0.43.0 and swag v1.16.3) instead of `@latest` so all setup tools (migrate, goimports, swag) are consistently pinned for reproducible builds.
🤖 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/ci.yml:
- Around line 49-50: The workflow currently downloads and moves the migrate
binary without verifying integrity; update the CI steps that call curl and sudo
mv (the lines downloading migrate.linux-amd64.tar.gz and moving migrate) to also
download the corresponding sha256sum.txt from the same GitHub release, verify
the tarball with sha256sum -c (or an equivalent sha256 check) before extracting,
and only proceed to tar xvz and sudo mv migrate /usr/local/bin/ if the checksum
verification succeeds; ensure you reference the release's sha256sum.txt and the
archive name migrate.linux-amd64.tar.gz when adding the verification step.
---
Nitpick comments:
In `@Makefile`:
- Around line 77-79: The Makefile currently pins migrate to v4.19.1 but installs
goimports and swag with `@latest`, which allows unpredictable changes; update the
go install lines for golang.org/x/tools/cmd/goimports and
github.com/swaggo/swag/cmd/swag to use fixed versions (e.g., goimports v0.43.0
and swag v1.16.3) instead of `@latest` so all setup tools (migrate, goimports,
swag) are consistently pinned for reproducible builds.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac1f425b-9ad2-4f38-b971-af13463c0364
📒 Files selected for processing (6)
.env.example.github/workflows/ci.ymlCONTRIBUTING.mdDockerfileMakefileREADME.md
Comment on lines
+49
to
50
| curl -L https://github.com/golang-migrate/migrate/releases/download/v4.19.1/migrate.linux-amd64.tar.gz | tar xvz | ||
| sudo mv migrate /usr/local/bin/ |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
#!/bin/bash
# Verify release assets available for the pinned tag
set -euo pipefail
gh api repos/golang-migrate/migrate/releases/tags/v4.19.1 --jq '.assets[].name'Repository: osama1998H/uniauth
Repository: osama1998H/uniauth
Exit code: 1
stdout:
{
"message": "Bad credentials",
"documentation_url": "https://docs.github.com/rest",
"status": "401"
}
stderr:
gh: Bad credentials (HTTP 401)
Script executed:
# Check migrate v4.19.1 release assets via GitHub API
curl -s "https://api.github.com/repos/golang-migrate/migrate/releases/tags/v4.19.1" | jq -r '.assets[] | .name' 2>/dev/null | head -20Repository: osama1998H/uniauth
Repository: osama1998H/uniauth
Exit code: 0
stdout:
migrate-4.19.1.zip
migrate.darwin-amd64.tar.gz
migrate.darwin-arm64.tar.gz
migrate.linux-386.deb
migrate.linux-386.tar.gz
migrate.linux-amd64.deb
migrate.linux-amd64.tar.gz
migrate.linux-arm64.deb
migrate.linux-arm64.tar.gz
migrate.linux-armv7.deb
migrate.linux-armv7.tar.gz
migrate.windows-386.zip
migrate.windows-amd64.zip
migrate.windows-arm64.zip
migrate.windows-armv7.zip
sha256sum.txt
Script executed:
# Also inspect the actual workflow file to see full context around lines 49-50
cat -n .github/workflows/ci.yml | sed -n '40,60p'Repository: osama1998H/uniauth
Repository: osama1998H/uniauth
Exit code: 0
stdout:
40 - uses: actions/checkout@v4
41
42 - uses: actions/setup-go@v5
43 with:
44 go-version-file: go.mod
45 cache: true
46
47 - name: Install migrate
48 run: |
49 curl -L https://github.com/golang-migrate/migrate/releases/download/v4.19.1/migrate.linux-amd64.tar.gz | tar xvz
50 sudo mv migrate /usr/local/bin/
51
52 - name: Run migrations
53 run: migrate -path migrations -database "postgres://uniauth:uniauth@localhost:5432/uniauth_test?sslmode=disable" up
54
55 - name: Run tests
56 env:
57 DATABASE_URL: postgres://uniauth:uniauth@localhost:5432/uniauth_test?sslmode=disable
58 REDIS_URL: redis://localhost:6379/0
59 JWT_SECRET: test-secret-key-for-ci-32-charsx
60 run: go test ./... -race -timeout 120s
Fix checksum verification for migrate binary download.
The workflow downloads the migrate binary without integrity verification, which creates a supply-chain risk. The GitHub release provides a sha256sum.txt file for verification.
🔐 Suggested hardening patch
- curl -L https://github.com/golang-migrate/migrate/releases/download/v4.19.1/migrate.linux-amd64.tar.gz | tar xvz
- sudo mv migrate /usr/local/bin/
+ MIGRATE_VERSION=v4.19.1
+ ARCHIVE=migrate.linux-amd64.tar.gz
+ curl -fsSLo "$ARCHIVE" "https://github.com/golang-migrate/migrate/releases/download/${MIGRATE_VERSION}/${ARCHIVE}"
+ curl -fsSLo sha256sum.txt "https://github.com/golang-migrate/migrate/releases/download/${MIGRATE_VERSION}/sha256sum.txt"
+ grep " ${ARCHIVE}$" sha256sum.txt | sha256sum -c -
+ tar -xzf "$ARCHIVE" migrate
+ sudo install -m 0755 migrate /usr/local/bin/migrate📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| curl -L https://github.com/golang-migrate/migrate/releases/download/v4.19.1/migrate.linux-amd64.tar.gz | tar xvz | |
| sudo mv migrate /usr/local/bin/ | |
| MIGRATE_VERSION=v4.19.1 | |
| ARCHIVE=migrate.linux-amd64.tar.gz | |
| curl -fsSLo "$ARCHIVE" "https://github.com/golang-migrate/migrate/releases/download/${MIGRATE_VERSION}/${ARCHIVE}" | |
| curl -fsSLo sha256sum.txt "https://github.com/golang-migrate/migrate/releases/download/${MIGRATE_VERSION}/sha256sum.txt" | |
| grep " ${ARCHIVE}$" sha256sum.txt | sha256sum -c - | |
| tar -xzf "$ARCHIVE" migrate | |
| sudo install -m 0755 migrate /usr/local/bin/migrate |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 49 - 50, The workflow currently
downloads and moves the migrate binary without verifying integrity; update the
CI steps that call curl and sudo mv (the lines downloading
migrate.linux-amd64.tar.gz and moving migrate) to also download the
corresponding sha256sum.txt from the same GitHub release, verify the tarball
with sha256sum -c (or an equivalent sha256 check) before extracting, and only
proceed to tar xvz and sudo mv migrate /usr/local/bin/ if the checksum
verification succeeds; ensure you reference the release's sha256sum.txt and the
archive name migrate.linux-amd64.tar.gz when adding the verification step.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
JWT_SECRETvalues during config loadJWT_SECRETvalues in Docker Compose and update docs to useopenssl rand -hex 32Type of change
Related issues
Changes
JWT_SECRETcheck frommainand fail fast in config loading instead.env.example, and scaling docs to document generating one shared secret and reusing it across instancesTesting
go test ./...)docker compose upSummary by CodeRabbit
Chores
Documentation
Tests