Add workflow to automatically bump Go toolchain#5010
Conversation
d5f3558 to
78c6c02
Compare
simonfaltum
left a comment
There was a problem hiding this comment.
Review Swarm: Add workflow to automatically bump Go toolchain
Reviewers: Isaac + Cursor | Rounds: 2 (independent review + cross-check)
0 Critical | 4 Major | 3 Nit | 2 Suggestion
Major findings
- Script injection via
${{ inputs.version }}interpolated directly into shell (line 40-41, 72) - Auto-created PR won't trigger CI because
peter-evans/create-pull-requestwith defaultGITHUB_TOKENsuppresses downstream workflows (line 79-80) - No null guard when
jqreturnsnullfor an EOL'd Go minor series (line 44-47) - Missing third
go.modatbundle/internal/tf/codegen/go.modwhich also has atoolchaindirective (line 71-72)
Suggestions (not blocking)
- Feed ordering assumption:
[...][0].versionassumes go.dev returns newest first. Consider sorting explicitly. - No build verification: consider adding
go build ./...orgo mod verifybefore PR creation, especially given the CI-not-triggering issue.
See inline comments for details and suggested fixes.
| if [ -n "${{ inputs.version }}" ]; then | ||
| toolchain=${{ inputs.version }} |
There was a problem hiding this comment.
[Major] Script injection via ${{ inputs.version }} (Found by: Isaac, Confirmed by: Cursor)
inputs.version is interpolated directly into the run: script at YAML-render time. This is the canonical GitHub Actions script-injection pattern. While only write-access users can trigger workflow_dispatch, a compromised account gets arbitrary shell on the protected runner with contents: write + pull-requests: write.
Suggestion: pass via env: and validate format:
env:
INPUT_VERSION: ${{ inputs.version }}
run: |
if [ -n "$INPUT_VERSION" ]; then
if ! echo "$INPUT_VERSION" | grep -qE '^go[0-9]+\.[0-9]+\.[0-9]+$'; then
echo "Invalid version format: $INPUT_VERSION"
exit 1
fi
toolchain="$INPUT_VERSION"
else
...
fiSame treatment needed for the go mod edit step at line 72.
There was a problem hiding this comment.
Fixed. Input is now passed via env: and validated against ^go[0-9]+\.[0-9]+\.[0-9]+$. Same treatment for the go mod edit step.
| toolchain=$( | ||
| curl -fsSL 'https://go.dev/dl/?mode=json' | | ||
| jq -r --arg minor "go${minor}." '[.[] | select(.version | startswith($minor))][0].version' | ||
| ) |
There was a problem hiding this comment.
[Major] No guard when jq returns null for EOL'd minor series (Found by: Both reviewers independently)
If the current minor is dropped from go.dev/dl/?mode=json (Go only keeps the two most recent minors), jq returns null. This flows into GITHUB_OUTPUT, the check step sees go1.25.7 != null, declares an update needed, and go mod edit -toolchain=null fails with a misleading error. Scheduled runs become recurring failures.
Suggestion: use // empty and guard:
toolchain=$(curl -fsSL 'https://go.dev/dl/?mode=json' | \
jq -r --arg minor "go${minor}." \
'[.[] | select(.version | startswith($minor))][0].version // empty')
if [ -z "$toolchain" ]; then
echo "No release found for go${minor}.x - minor series may be EOL."
exit 1
fiThere was a problem hiding this comment.
Fixed. Added // empty to the jq filter and an explicit guard that fails the step if no release is found.
| for dir in . ./tools; do | ||
| (cd "$dir" && go mod edit -toolchain=${{ steps.latest.outputs.toolchain }}) |
There was a problem hiding this comment.
[Major] Misses third go.mod at bundle/internal/tf/codegen/ (Found by: Cursor, Confirmed by: Isaac who verified the file exists with toolchain go1.25.7)
The loop only updates ./go.mod and ./tools/go.mod, but bundle/internal/tf/codegen/go.mod also has a toolchain directive. That module will drift after every automated bump, undermining the CVE-patch goal.
Suggestion: discover dynamically:
while IFS= read -r modfile; do
dir=$(dirname "$modfile")
if grep -q '^toolchain' "$modfile"; then
(cd "$dir" && go mod edit -toolchain="$TOOLCHAIN")
fi
done < <(git ls-files '**/go.mod' 'go.mod')There was a problem hiding this comment.
Fixed. Replaced hardcoded directory list with dynamic discovery via git ls-files. Updates any go.mod that has a toolchain directive.
| - name: Create pull request | ||
| if: steps.check.outputs.needed == 'true' && inputs.version == '' |
There was a problem hiding this comment.
[Major] Auto-created PR will not trigger CI (Found by: Isaac, Confirmed by: Cursor)
peter-evans/create-pull-request with the default GITHUB_TOKEN will not trigger downstream workflows on the created PR (GitHub's safeguard against recursive triggers). Every auto-generated toolchain-bump PR lands with zero CI signal, so reviewers merge blind or have to close/reopen manually.
Suggestion: supply a token: input from a GitHub App installation token or PAT with repo scope:
with:
token: ${{ steps.app-token.outputs.token }}
branch: auto/bump-go-toolchain
...If not available, at minimum note in the PR body that CI did not run.
There was a problem hiding this comment.
Acknowledged. This is a known limitation of GITHUB_TOKEN. Fixing it requires a GitHub App or PAT, which is out of scope for this PR. Reviewers can push an empty commit to trigger CI.
|
|
||
| Release notes: https://go.dev/doc/devel/release#${{ steps.latest.outputs.toolchain }} | ||
| reviewers: simonfaltum,andrewnester,anton-107,denik,janniklasrose,pietern,shreyas-goenka | ||
| labels: dependencies |
There was a problem hiding this comment.
[Nit] Hardcoded individual reviewers (Found by: Isaac, Confirmed by: Cursor)
Pinning seven usernames means every team change requires a workflow edit. Consider using team-reviewers: <org>/<team-slug> (supported by peter-evans/create-pull-request), or rely on CODEOWNERS + branch protection.
There was a problem hiding this comment.
Intentional. team-reviewers requires org-level token access which the default GITHUB_TOKEN doesn't have.
| run: | | ||
| toolchain=$(grep '^toolchain' go.mod | awk '{print $2}') |
There was a problem hiding this comment.
[Nit] No empty guard on grep result (Found by: Isaac, cross-review)
If the toolchain directive is ever removed from go.mod, grep '^toolchain' returns empty, minor becomes empty, and the jq filter would match all Go releases across all minors, potentially triggering an unintended minor version upgrade PR. Consider failing fast if toolchain is empty.
There was a problem hiding this comment.
Won't fix. If the toolchain directive is removed from go.mod, the workflow failing is the correct behavior.
Runs daily at 05:00 UTC. Checks for new patch releases of the current Go minor series via the Go download API and creates a PR when an update is available. Supports workflow_dispatch with an optional version override to test the flow without creating a PR. Co-authored-by: Isaac
Summary
Add a GitHub Actions workflow that automatically bumps the Go toolchain to the latest patch release. This ensures CVE fixes in the Go toolchain are picked up promptly.
https://go.dev/dl/?mode=jsonfor the latest patch of the current minor seriestoolchaindirective in bothgo.modandtools/go.modworkflow_dispatchwith an optional version override for testing (skips PR creation)Successful run: https://github.com/databricks/cli/actions/runs/24562914491
Example PR: #5009
Test plan
go1.25.7 → go1.25.9updatego mod editupdates bothgo.modandtools/go.mod