gha: Add release branch sync workflow#7091
Conversation
cc66332 to
e802815
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🔴 CRITICAL
Two high-severity bugs completely disable the master-branch safety guard in the Validate step (BRANCH vs $branch case mismatch + trailing space in the comparison string). Several medium-severity issues were also found: git checkout -- '*' won't resolve subdirectory conflicts, sed "//q" uses unescaped regex with slash-injection risk, incorrect boolean logic in conflict detection, missing xargs -r guard, and a DRY_RUN check that fires after merges have already been applied.
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
3 findings in 2 files — 1 high-severity (forced inline), 2 medium-severity (security + logic).
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🔴 CRITICAL
Two confirmed high-severity bugs in new code introduced by this PR.
Findings summary
- scripts/unmerged-tags:32 — Plain
sortsorts version tags lexicographically, causing wrong tag ordering (e.g.docker-v29.10.0beforedocker-v29.9.0). Theawkexit-on-target logic then cuts the list at the wrong point, syncing the wrong set of tags. - .github/workflows/sync-release-branch.yml:80 —
scripts/sync-branch(which runsgit merge) is called before theDRY_RUNcheck, so merges always execute even in dry-run mode. Thedry_runflag only prevents the push, contradicting the stated intent of "print the tag range without merging or pushing".
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
4 medium-severity findings confirmed or likely in the newly added scripts and workflow.
Add a manually dispatched workflow for maintainers to sync a Docker release branch to a selected release tag. The sync-release-branch job checks out the release branch, computes the list of unmerged tags up to the requested tag via scripts/unmerged-tags, merges them in order via scripts/sync-branch using git merge --no-ff (resolving conflicts by taking the tag's content), then pushes the result to a temporary branch. The push-release-branch job runs after manual approval via the docker-releases environment. It verifies that neither the release branch nor the temporary branch moved since the sync job ran before force-advancing the release branch and deleting the temporary branch. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
One finding requires attention before merge.
Lower-confidence findings (not posted inline)
None.
Dismissed findings
scripts/sync-branch:17— Inverted conflict-check logic (DISMISSED: logic is correct; the OR condition correctly guards the force-checkout path).github/workflows/sync-release-branch.yml:60— User-supplied tag input not validated (DISMISSED: workflow_dispatch restricted to maintainers, env var prevents injection, git args are safe).github/workflows/sync-release-branch.yml:80— xargs splitting may mishandle special characters (DISMISSED: git tag names cannot contain whitespace by git's own constraints).github/workflows/sync-release-branch.yml:132— Push uses mutable FETCH_HEAD ref (DISMISSED: no concurrent fetch between verification and push; FETCH_HEAD is stable in this context)
|
|
||
| # Get all docker release tags merged into master but not into release branch | ||
| git tag --merged "$master" --no-merged "$release_branch" \ | ||
| | grep '^v' \ |
There was a problem hiding this comment.
[high] Tag input format in workflow description conflicts with awk stop condition in unmerged-tags
The workflow's own input description says "Tag to sync from, for example docker-v29.6.0", but the repository's actual git tags use v* format (e.g. v29.6.0). This creates two interacting issues:
-
If a maintainer follows the description and supplies
docker-v29.6.0:git rev-parse --verify refs/tags/docker-v29.6.0fails immediately with "Tag does not exist" — the script aborts cleanly but confusingly. -
More critically — the
awkstop condition: even when the correctv29.6.0format is used, theawk -v tag="$tag"on line 38 compares$0 == tagto stop printing at the target tag. This relies on$tagmatching the format thatgit tag --mergedoutputs. If the user ever supplies a tag indocker-v*format (matching the description), the equality check never fires andawkprints all unmerged tags rather than stopping at the intended target — syncing more tags than requested with no error.
The grep '^v' filter is correct for the real tag format; the misleading description is the root of the confusion. Recommend either:
- Update the description to
"for example v29.6.0"to match the actual repo convention, and add a validation/normalization step, or - If
docker-v*is the intended format (e.g. ported from moby where tags differ), fixgrep '^v'togrep '^docker-v'and ensure the tag existence check andawkcomparison are consistent.
| Confidence | Score |
|---|---|
| 🟡 moderate | 57/100 |
Add a manually dispatched workflow for maintainers to sync a docker release branch to a selected docker release tag.
The job checks out the requested release branch, merges the tag with
git merge --no-ff, checks the worktree content out from that tag, stages the result, and pushes the updated branch.The actual push is guarded by the
docker-releasesGitHub environment which requires a manual approval.