ci: bind the release deployment environment to main - #3287
Conversation
375d4fe to
d6d05f8
Compare
The release environment holds the Apple notarization and code signing credentials that .github/workflows/release-desktop.yml consumes, but it carries no protection rules at all. Its only current guard is the job-level branch condition inside release-desktop.yml, which lives in the same file it is meant to protect and so cannot bind the environment on its own. Declare a deployment branch policy that restricts the environment to main. GitHub enforces it outside the workflow, before any job that requests the environment starts. npm-release already carries the same kind of policy, so this reuses an established pattern rather than inventing one. The directive only visits environments named here, so npm-release and copilot keep their current configuration. npm-release is deliberately left undeclared: the underlying call replaces an environment's settings wholesale, and declaring it without restating its reviewer and branch policy would clear them. Generated-by: Claude Code
d6d05f8 to
51572ff
Compare
|
Requesting review from @M4n5ter and @hqhq1025. @M4n5ter — this applies the environment protection pattern from #3192 to the desktop signing channel. @hqhq1025 — mainly the CI and release-path side: whether pinning On validation: the schema and directive semantics were checked against the upstream This touches release credentials, so it is not a self-merge candidate. |
hqhq1025
left a comment
There was a problem hiding this comment.
No actionable findings.
Problem and mechanism
The release GitHub Environment contains five Apple signing and notarization
secrets, but its live configuration currently has no deployment protection
rules. The workflow checks github.ref itself, which a collaborator could
remove in a branch copy before dispatching that branch.
This PR adds an environment-owned custom branch policy for the exact main
branch. GitHub evaluates that policy before the build matrix jobs can start
and access the environment secrets.
Evidence and first-principles assessment
- The live
releaseenvironment has no reviewers, timer, or branch policy, so
the asfyaml full update does not erase existing protection. - Upstream
infrastructure-asfyamlvalidates this shape and emits
protected_branches=false,custom_branch_policies=true, with one
{ name: "main", type: "branch" }policy. - The directive iterates only named environments, so
npm-releaseand
copilotare not modified. - Environment secrets are managed through a separate endpoint and are not part
of this update. - Every observed desktop release workflow run and deployment used
main, so
the policy preserves the established release path. - The PR merges cleanly onto current
main, and required CI is green.
The problem definition and solution follow first principles and Occam's razor:
the credential owner should enforce its own admissible refs, and one exact
environment policy is the smallest mechanism that does so.
Deletion, tests, and architecture
There is no production code or low-value test to delete. No deeper refactor is
needed. The existing workflow-level main checks remain useful defense in
depth, while the environment policy becomes the external credential boundary.
Merge verdict
Approved at 51572ff281e5f8615a9b56c995babb30856db512.
Residual operational checks:
- After merge and ASF Infrastructure application, verify through the GitHub API
thatreleasereports custom branch policies enabled with exactly the
mainbranch policy and that the five environment secret names remain. releasecurrently allows repository administrators to bypass environment
protection. This PR correctly protects ordinary write collaborators; if the
project also wants to remove the administrator break-glass path, that setting
needs a separate manual change or upstream asfyaml support.
Summary
The
releaseGitHub Environment holds the Apple notarization and code signing credentials that.github/workflows/release-desktop.ymlconsumes, and it currently has no protection rules at all. Nothing outside the workflow constrains which ref may deploy with those credentials.The workflow does carry a job-level branch condition, but that condition lives in the same file it is meant to protect, so it cannot serve as the boundary for the environment's secrets. The boundary that holds regardless of the workflow file is the environment's own deployment branch policy, which GitHub evaluates before a job that requests the environment starts.
This declares that policy in
.asf.yaml, restrictingreleasetomain.npm-releasealready carries the same kind of policy, so this reuses an established pattern in this repository rather than introducing a new one.npm-releaseis deliberately left undeclared. The asfyaml directive only visits the environments it is given, sonpm-releaseandcopilotare untouched — but the underlying API call replaces an environment's settings wholesale, so namingnpm-releasehere without restating its existing reviewer and branch policy would clear them. The.asf.yamlcomment records that constraint where the next editor will see it.Verification
Validated against the upstream asfyaml implementation at
apache/infrastructure-asfyaml@main, not against documentation. All of the below was re-run after the final wording of the.asf.yamlcomment:asfyaml.cli:validateon this branch's checkout — reports the file as valid. Negative control: renamingdeployment_branch_policyto a key not in the schema makes the same run fail withunexpected key not in schema, confirming the new block actually reachesASFGitHubFeature.schema. Note for anyone tempted to wire this into CI:validate()prints the error but still exits 0, so it would need an explicit output check to work as a gate._validate_environment_configsfromasfyaml/feature/github/deployment_environments.pyon the parsedenvironmentsmap — no errors. The parsed value is exactly{"release": {"deployment_branch_policy": {"policies": [{"name": "main", "type": "branch"}], "protected_branches": false}}}.dirty_loadof the whole file, thendirty_loadofgithub.as_yaml()) to confirm the comment block does not pollute the foldeddescriptionscalar: 184 characters before and after the round trip, identical.yaml.safe_loaddoes not exercise this path.protected_branches.main.required_status_checks.contextsstill parses as['test']after the round trip.Not run: repository test suites, lint and typecheck. This change touches only
.asf.yaml, which no workspace suite covers and which Biome does not format.Rollout
ASF Infrastructure applies
.asf.yamlon push to the default branch, so the policy takes effect on merge. Applying it issues a full environment update forrelease, which also writes asfyaml's defaults ofwait_timer: 0andprevent_self_review: true. The environment currently has no reviewers, no wait timer and no branch policy, so nothing existing is cleared andprevent_self_reviewis a no-op until a reviewer is ever added. Environment secrets live on a separate endpoint and are not affected. In-flight runs are not affected.AI use
Select exactly one:
Tool(s) and scope: Claude Code (Opus 5) read the upstream asfyaml source to confirm the schema and directive semantics, wrote the
.asf.yamlchange, ran the validation described above, and drafted this description. A second pass by Claude Code (Fable 5) adversarially reviewed the change and independently reproduced each verification step above; its findings corrected the comment wording and prompted the scope section. Both are AI review and neither substitutes for independent human review. The commit carries aGenerated-bytrailer. A human contributor reviews the final diff and owns the merge decision.Checklist
Neither applies: there is no test hook for
.asf.yaml, and Biome does not cover YAML. Validation against the upstream schema and directive is described above.Does this PR entail a change in behavior?
This is a security hardening change to a protected area, so it is not a self-merge candidate and needs independent human review.