You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ci(release): scope and lock the signing keychain - #3066
build-release-macos-arm64 and sign-macos-artifacts set environment: release.
All three keychain unlocks pass KC_PW on standard input, not as an argument. pipefail keeps a refused unlock a step failure, because the default bash -e shell does not set it.
Each signing job locks the keychain in a final if: always() step.
The docs add a step to cutting a release: list the new release/X.Y.Z branch in the signing runner group.
Before and after merge
The release environment exists and admits main and release/*. KC_PW is still a repository secret, which these jobs can read until it is set on the environment. So this PR does not depend on that move.
Signed runs from any other branch stop working once this merges. The last one was a feature branch on 2026-10-31.
Verification
actionlint and zizmor pass on release.yml. just lint-prose is clean on both docs.
On a throwaway keychain, with a password containing ", \, a space and $: printf '%s\n' "$PW" | security unlock-keychain "$K" exits 0 for the right password and 51 for a wrong one.
Not verified: no release has run this workflow. The environment rule blocks a dry run from this branch, so the first proof is a dry run from main after merge.
Release signing jobs now use the release environment and stricter branch and runner controls. Signing keychains are locked after each job, and unlock steps pass passwords through standard input.
Documentation
Updated release and runner setup guidance clarifies which branches and runners can run signing jobs, how to configure signing and notarization jobs, and where release credentials are provided.
The signing jobs read KC_PW as a repository secret, passed it to
security on the command line, and left the keychain unlocked when they
ended.
Both jobs now set `environment: release`, which holds the secret and
admits only main and release/* branches. The password goes to security
on standard input, with pipefail set so a refused unlock still fails
the step. The last step of each job locks the keychain on any outcome.
The docs add the release branch step for the signing runner group.
No actionable comments were generated in the recent review.
Walkthrough
The macOS release workflow now uses the release environment for signing jobs, pipes KC_PW to keychain unlock commands, and locks the keychain after those jobs. Internal runbooks document runner-group, branch admission, and environment requirements.
Changes: macOS release signing
Layer / File(s)
Summary
Signing job access and keychain handling .github/workflows/release.yml, docs/internal/macos-runner-setup.md, docs/internal/release-pipeline.md
Signing jobs use the release environment. Unlock commands pipe KC_PW to security unlock-keychain, and unconditional cleanup steps lock the keychain. The runbooks describe runner-group restrictions, branch admission requirements, and the source of the notary keychain password.
gominimal/minimal#2823: Adds Developer ID signing and notarization to the macOS release job.
gominimal/minimal#2232: Documents releases and hotfixes from persistent release branches.
Suggested reviewers:bryan-minimal, jtnkminimal
Merge Risk: Minimal · up to f06d7
No merge-blocking risk was established for the documented release path.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title describes the release signing keychain changes.
Description check
The description covers the changes, related issue, and verification. It is mostly complete. Use the template headings, add the checklist, and include lint output.
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
Vale (3.24.0)
{
"Line": 13,
"Path": ".vale.ini",
"Text": "Style 'ste' isn't on the StylesPath (/styles). If it's a package, run vale sync.",
"Code": "E201",
"Span": 23
}
A rabbit checks the signing gate Then pipes the password, neat and straight The keychain locks when tasks are through Release branches pass the checks they’re due The runner rests beneath the moon
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Essentials
Run ID: fce76e81-b7d8-4c3c-b5c6-c24b3a7a1048
Commits
Reviewing files that changed from the base of the PR and between 803dbe7 and f06d7e1.
Files selected for processing: 1
docs/internal/release-pipeline.md
Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.
The reason will be displayed to describe this comment to others. Learn more.
Review of 803dbe7. I found no blockers. The workflow change is small and correct as far as I can check without a Mac.
I checked these live settings read-only:
The release environment exists. Its branch policies are main and release/*.
KC_PW is set on that environment.
The only caller, nightly.yml, runs on main, so it is admitted.
The runner doc says there is one runner per Mac, so the new lock step cannot race another job on the same keychain.
Should-fix
1. The repository-level KC_PW still exists, so the environment does not restrict the secret yet. (release.yml:618, :702, :944; docs/internal/release-pipeline.md:62)
gh api repos/gominimal/minimal/actions/secrets still lists KC_PW beside the new environment secret.
Any job on any branch that leaves out environment: still gets secrets.KC_PW. The runner group still limits who can reach the keychain on the Mac.
So the title and the doc line "The release environment holds KC_PW" are not true yet.
The PR body also says KC_PW is still only a repository secret until it is set on the environment. That is out of date, because the environment secret now exists.
Fix: after the first dry run from main passes, run gh secret delete KC_PW --repo gominimal/minimal, then update the PR body.
2. The doc's runner-group PATCH can reset the group's access settings. (docs/internal/release-pipeline.md:69-70)
The jq builds only {name, selected_workflows} and PATCHes that.
The "Update a self-hosted runner group" endpoint documents restricted_to_workflows and allows_public_repositories with Default: false. If omitted fields fall back to those defaults rather than keeping their current values:
restricted_to_workflows becomes false, and every workflow on every branch can reach the signing Mac. That undoes the control this PR documents.
allows_public_repositories becomes false. This repo is public, so the signing jobs then wait in the queue.
This is a hypothesis from the documented defaults. I could not read runner group 3, which needs admin:org, and I did not run the PATCH.
Fix: carry the fields through, then have an org owner check the group with a GET:
set -o pipefail does nothing here. (release.yml:625, :710, :949)
The last command in each pipeline is security, so bash -e already fails the step when the unlock is refused.
I checked with bash -e -c 'printf "%s\n" pw | sh -c "cat >/dev/null; exit 51"; echo reached', which exits 51 without printing reached.
Drop it, or keep it and correct the reason in the PR body.
The stdin unlock goes through getpass(). (release.yml:626, :711, :950)
Without -p, security unlock-keychain reads with getpass(). On macOS that cuts the password at 128 characters.
It also reads /dev/tty instead of stdin when the process has a controlling terminal.
Confirm that KC_PW is 128 characters or fewer. A runner started with ./run.sh from Terminal, instead of the LaunchAgent, would wait on the tty.
I have not tested this on macOS.
docs/internal/release-pipeline.md:66 names the wrong permission. It says the PATCH "needs an organization owner". The API asks for an org admin, or the "runners and runner groups" permission, which is what macos-runner-setup.md:145 already says.
docs/internal/release-pipeline.md:472 gives no branch for the dry run. It says to test with a dry_run run. After this PR, a dry run from a feature branch fails the two signing jobs on the environment's branch policy. Add "from main or a release/ branch".
The tracking issue is wrong. "Related: #2812" is closed and is about Secure Enclave keys for BEP. This hardening has no tracking issue.
Not checked
actionlint, zizmor and Vale are not installed here.
Runner group 3's live config, which needs admin:org.
Anything that needs a real Mac:
the stdin unlock
lock-keychain when the earlier steps never ran
whether a locked keychain that is still on the search list makes a later job on the signing Mac show a GUI unlock prompt
The first dry run from main after merge is the first real proof.
This PR edits .github/workflows/, which is frozen and owner-gated. @mitodrummer signs off on that separately.
The reason will be displayed to describe this comment to others. Learn more.
Sign-off from @mitodrummer, the owner, on the .github/workflows/release.yml change, which is frozen and CODEOWNER-gated. Before or right after merge, see the two should-fixes in the review above: delete the repo-level KC_PW once the first dry run from main passes, and carry the access fields through the runner-group PATCH in the doc.
Backports the release.yml part of #3066. Both signing jobs set
`environment: release`, the unlock reads the password from standard
input with pipefail set, and each job ends by locking the keychain.
The one conflict is the build job timeout. This branch keeps its own
60 minutes and its comment, because it does not build bep.
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
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.
Change
build-release-macos-arm64andsign-macos-artifactssetenvironment: release.KC_PWon standard input, not as an argument.pipefailkeeps a refused unlock a step failure, because the defaultbash -eshell does not set it.if: always()step.release/X.Y.Zbranch in thesigningrunner group.Before and after merge
releaseenvironment exists and admitsmainandrelease/*.KC_PWis still a repository secret, which these jobs can read until it is set on the environment. So this PR does not depend on that move.Verification
actionlintandzizmorpass onrelease.yml.just lint-proseis clean on both docs.",\, a space and$:printf '%s\n' "$PW" | security unlock-keychain "$K"exits 0 for the right password and 51 for a wrong one.Not verified: no release has run this workflow. The environment rule blocks a dry run from this branch, so the first proof is a dry run from
mainafter merge.Related: #2812.
Summary by CodeRabbit