fix: guard release tag creation - #843
Conversation
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Pull request overview
Separates release preparation from publishing and guards immutable release tags with provenance validation, production approval, and a least-privilege GitHub App.
Changes:
- Adds prepare, publish, and post-release version-sync workflows.
- Adds and tests fail-closed release provenance validation.
- Applies validation to publishers and documents protected repository setup.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/prepare-release.yaml |
Creates version preparation PRs. |
.github/workflows/publish-release.yaml |
Validates and creates protected tags. |
.github/workflows/sync-released-version.yaml |
Syncs minor releases to main. |
.github/workflows/release.yaml |
Adds artifact publication preflight. |
.github/workflows/release-runners.yaml |
Guards runner image publishing. |
.github/workflows/release-pr.yaml |
Removes the overloaded legacy workflow. |
.github/workflows/unit-test.yaml |
Runs validator tests in CI. |
scripts/ci/validate-release.sh |
Validates manifests and release provenance. |
scripts/ci/validate-release_test.sh |
Covers validator acceptance and rejection paths. |
CONTRIBUTING.md |
Documents protected release administration. |
website/docs/release.md |
Updates user-facing release instructions. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 11/11 changed files
- Comments generated: 0
- Review effort level: Balanced
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b3263b455
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f3fd968f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
@codex review |
There was a problem hiding this comment.
Review details
Suppressed comments (1)
CONTRIBUTING.md:297
- The workflows read each client ID from the
varscontext and each private key fromsecrets, but these setup steps do not distinguish environment variables from environment secrets. An operator who stores both values as secrets will leavevars.RELEASE_APP_CLIENT_ID/vars.RELEASE_SYNC_APP_CLIENT_IDempty and token creation will fail. State the storage type explicitly.
- Store `RELEASE_APP_CLIENT_ID` and `RELEASE_APP_PRIVATE_KEY` only in the protected `prod` environment. Require a reviewer on `prod`, disallow administrator bypass, and restrict deployments to `main`. Enable self-review prevention when a second maintainer or reviewer team is available.
- Store `RELEASE_SYNC_APP_CLIENT_ID` and `RELEASE_SYNC_APP_PRIVATE_KEY` only in a separate `version-sync` environment. Restrict it to `main` and disallow administrator bypass. It does not need another reviewer because it runs only after the approved tag job succeeds.
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2231d4df85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Review details
Suppressed comments (2)
scripts/ci/validate-release_test.sh:134
- This fixture omits the fourth
head.shafield, so the validator fails on malformed PR data before reaching the intended merge-ancestry check. Supply a valid head SHA so the test actually proves that an unrelated merge commit is rejected.
unrelated_pr_details=$(printf 'true\trelease-1.2\t%s' "$side_commit")
.github/workflows/prepare-release.yaml:88
create-pull-requestonly calls GitHub's add-labels endpoint; it does not create missing repository labels. Since this label is a new version on every release (the repository currently has version labels only throughv0.22.1), preparing the next version will get a 422 while applying the label, and the resulting unlabeled PR cannot pass the validator. Either create the version label first with the required permission and document that setup, or remove the dynamic-label dependency and bind provenance to the requested version another way.
labels: |
release-pr
${{ inputs.release_version }}
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Balanced
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Review details
Suppressed comments (3)
scripts/ci/plan-release-sync.sh:80
- The planner likewise accepts
version:1.2.3andappVersion:v1.2.3because*permits no YAML separator. It can report a malformed chart as consistent and continue synchronization planning. Require at least one whitespace character after each colon.
chart_version_count=$(awk '/^version:[[:space:]]*/ { count++ } END { print count + 0 }' "$chart_path")
chart_app_version_count=$(awk '/^appVersion:[[:space:]]*/ { count++ } END { print count + 0 }' "$chart_path")
scripts/ci/validate-release.sh:73
- These checks allow zero whitespace after
:, so a line such asversion:1.2.3is accepted as the expected version even though YAML treats it as a scalar rather than the Helm chart'sversionkey. The provenance validator can therefore approve a malformedChart.yamland create a tag whose chart publication fails. Require at least one separator character for both fields.
chart_version_count=$(awk '/^version:[[:space:]]*/ { count++ } END { print count + 0 }' <<<"$chart_content")
chart_app_version_count=$(awk '/^appVersion:[[:space:]]*/ { count++ } END { print count + 0 }' <<<"$chart_content")
scripts/ci/plan-release-sync.sh:120
- Release components are accepted with unbounded digits, but these arithmetic comparisons use Bash's signed machine-width integers. For example,
v9223372036854775808.0.0is valid underparse_versionbut overflows here and can be planned as older thanv1.0.0. Compare decimal components by length and then lexically, asvalidate-release-version.shdoes, and use the same comparison when selecting the pending target.
if ((left_major > right_major || (left_major == right_major && left_minor > right_minor))); then
comparison=1
elif ((left_major < right_major || (left_major == right_major && left_minor < right_minor))); then
comparison=-1
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Balanced
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Review details
Suppressed comments (2)
scripts/ci/plan-release-sync.sh:120
- The accepted semantic-version syntax allows arbitrarily long numeric components, but Bash arithmetic is signed machine-width. For example, with main at
v9223372036854775807.0.0, releasev9223372036854775808.0.0overflows and is treated as older, so the required sync is skipped. The pending-version comparisons below have the same problem. Use the length-then-lexicographic component comparison already used byvalidate-release-version.shrather than(( ... ))for version components.
if ((left_major > right_major || (left_major == right_major && left_minor > right_minor))); then
comparison=1
elif ((left_major < right_major || (left_major == right_major && left_minor < right_minor))); then
comparison=-1
.github/workflows/prepare-release.yaml:182
- The version label is dynamic, but
create-pull-request@v8.1.1only callsissues.addLabels; it does not create missing repository labels. The repository currently has labels throughv0.22.1but no future version labels, and the documented setup does not require operators to create one, so preparing the next version will create an unlabeled PR and then fail while applying the label. Sincevalidate-release.shalso requires this label, either create it explicitly before the PR or stop using per-version labels and identify candidates from the staticrelease-prlabel plus the existing manifest checks.
labels: |
release-pr
${{ inputs.release_version }}
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Balanced
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Review details
Suppressed comments (2)
.github/workflows/release.yaml:164
- An existing release is accepted solely by tag lookup. If that release is a draft or prerelease, this workflow still succeeds, but
reconcile-applater requires.draft == falseand.prerelease == false, so the immutable image tag can never be promoted until someone manually repairs the release. Validate the existing release as a published stable release here (or convert it explicitly) before reporting success.
if gh api \
"repos/${GITHUB_REPOSITORY}/releases/tags/${GITHUB_REF_NAME}" \
--silent 2>/dev/null; then
echo "GitHub release ${GITHUB_REF_NAME} already exists." >>"${GITHUB_STEP_SUMMARY}"
else
gh release create "${GITHUB_REF_NAME}" \
.github/workflows/release.yaml:154
- This new failure point remains after
Publish Helm chart, so a failed publisher can already have added the publicvX.Y.Zchart to GitHub Pages; even on success, that chart is visible before the reconciler creates the corresponding immutable image tag. This contradicts the PR's guarantee that failed publisher attempts cannot change public versions. Stage the chart as an attempt-bound artifact and publish it only from the promotion path after the immutable image version is ready.
This issue also appears on line 159 of the same file.
- name: Create GitHub release
- Files reviewed: 25/25 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
@codex review |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3eb7a3e616
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
@codex review |
There was a problem hiding this comment.
Review details
Suppressed comments (2)
.github/workflows/reconcile-release-latest.yaml:985
- The runner exact-promotion path has the same unrecoverable stale-revision state: after the successful runner publisher triggers this job, a
mainadvance makes its write step fail; rerunning preserves the stalegithub.sha, and manual dispatch skips this job. The manual global reconciler then refuses to proceed because the immutable runner tags were never fully created. Provide a current-mainmanual recovery mode that securely revalidates and promotes the selected successful runner attempt without requiring the already-successful publisher to be rerun.
if: >-
github.ref == 'refs/heads/main' &&
github.event_name == 'workflow_run' &&
github.event.workflow_run.name == 'Release runner images' &&
github.event.workflow_run.conclusion == 'success'
scripts/ci/check-release-alias-floor.sh:294
- The legacy fallback checks the DSSE predicate URI as
critical.type, but actual Cosign simple-signing payloads usecosign container image signaturethere (the URI is a predicate type). Legacy signatures without annotations may also serializeoptionalasnull, not{}. Consequently, the existing pre-guardraillatestimage cannot establish a floor, so the first protected reconciliation fails instead of advancing the alias. Validate the real simple-signing payload shape and update the legacy fixtures accordingly.
jq -e \
--arg digest "$digest" \
--arg image "ghcr.io/${ghcr_repository}@${digest}" '
type == "array" and
any(.[];
.critical.type == "https://sigstore.dev/cosign/sign/v1" and
.critical.image["docker-manifest-digest"] == $digest and
.critical.identity["docker-reference"] == $image and
.optional == {}
)
- Files reviewed: 28/28 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
lint,unit-test, and all other latest workflow runsprodapproval, using a dedicated least-privilege release Applatestmainthrough the release-automation App, a canonical monotonic branch, exact-SHA rechecks, and force-with-lease protectionv0.22.1when an unusable.0tag must remain immutableValidation
scripts/ci/validate-release-version_test.shscripts/ci/validate-release_test.shscripts/ci/plan-release-sync_test.shactionlinton all affected workflowsshellcheckon all release validator/planner scripts and testsv0.22.0candidate is rejected because its manifests containv0.21.0;v0.22.1is rejected because preparation PR chore: Prepare v0.22.1 release #842 was merged without any completed checksRequired repository setup
Before using Prepare release or Publish release:
RELEASE_APP_CLIENT_IDas an environment variable andRELEASE_APP_PRIVATE_KEYas an environment secret inprod; restrictprodtomainand retain its required reviewer/no-admin-bypass policyrelease-automationenvironment restricted tomain, then storeRELEASE_AUTOMATION_APP_CLIENT_IDas an environment variable andRELEASE_AUTOMATION_APP_PRIVATE_KEYas an environment secret therev*tag-creation ruleset with an Always allow bypass only for the release App