Repository navigation
ci(release): pass the GitHub App client secret to the build jobs - #2320
Conversation
The three build jobs that compile `min` now export MINIMAL_GITHUB_APP_CLIENT_SECRET from the repository secret of that name. `min auth login` reads it at compile time for the browser sign-in under the Minimal-published GitHub App; a build without it offers only the device flow. Nightly reuses this workflow with inherited secrets, so it gets the variable too. Nothing reads it until the sign-in lands.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Pass secret to release builds .github/workflows/release.yml |
The AMD64 and ARM64 static build steps and the macOS ARM64 minimal build step receive MINIMAL_GITHUB_APP_CLIENT_SECRET from the workflow secret. |
Priority:
Estimated code review effort: 1 (Trivial) | ~4 minutes
Change: Other
Possibly related issues
- Issue 1501: The release builds now receive the GitHub App client secret described as enabling the browser sign-in path.
- Issue 2276: The release builds now receive the client secret identified as needed for browser sign-in.
Suggested reviewers: bryan-minimal
Merge Risk: 
95b4b
-
The release workflow now passes the GitHub App client secret only to the steps that compile the binary, so release builds can offer browser sign-in. No concrete merge-blocking risk remains; the normal CODEOWNER review of the workflow change still applies.
Pre-merge checks: 
-
Check name Status Explanation Title check The title uses the required Conventional Commit format and clearly describes the primary change: passing the GitHub App client secret to release build jobs. Docstring Coverage No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… 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. Description check The description follows the repository template in substance. It has a Summary-equivalent ('What' and 'Why' sections) that explains the change, the reason for it, and the scope of the secret. It links… Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
Full details: Description check
-
Explanation
The description follows the repository template in substance. It has a Summary-equivalent ('What' and 'Why' sections) that explains the change, the reason for it, and the scope of the secret. It links related issues (
#2263,#1501). The Testing-equivalent 'Checks' section lists actionlint and zizmor runs with results, though it does not paste raw output. The Checklist section is absent: it does not say whether docs were updated, and it does not address the BREAKING CHANGE footer. The PR title is not shown in the provided context, so its Conventional Commit format cannot be checked.
-
✨ 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
A rabbit checks the build step’s nest
A secret joins the release quest
Three targets greet the added key
Then hop along, securely
The workflow runs, neat as can be
Comment @coderabbitai help to get the list of available commands.
|
Reviewed: 2 findings, none blocking at Address the comments on the diff. Clobber: none seen. |
There was a problem hiding this comment.
Two things before I'd stamp this. The bot's HIGH is right about the fact and wrong about the conclusion: an option_env! secret ships in every public binary, so it is public by construction; that is also how gh itself ships its OAuth client secret, and GitHub Apps have no PKCE alternative for the browser flow. So the model is 'the client secret is not a secret', and the PR should say so in the comment above the env line (one sentence), so the next reader does not file the same finding. Second, and cheap: move the env from job level to the cargo build step. At job level every step in the job sees it, the third-party disk-free action included; at step level only the compile does. Nothing else to change.
|
/revise address all comments and review findings. |
|
Round 1 of 10 changed nothing: Noop. The only file in this pull request is .github/workflows/release.yml, and the survey refuses every path under .github/ to this agent. So I changed nothing. The finding has two parts. Part 1 (the secret is compiled into public binaries) does not hold: crates/minimal/src/cmd/auth.rs:24-27 documents BUILT_IN_SECRET as 'The App's public client secret', and the browser flow already uses PKCE (code_challenge S256, code_verifier). GitHub App token exchange requires client_secret even with PKCE, so the design treats it as public and relies on PKCE and the 127.0.0.1 redirect. Part 2 (scope the env to the cargo build step, not the job) is a fair hardening. It needs an edit to .github/workflows/release.yml at the three job env blocks (near lines 194, 339 and 473), which a person with CODEOWNER rights must make. I ran no tests because I changed no file.. This pull request is still open. branch
Updated 2026-10-20T21:35:56.797Z. |
|
Round 1 of 10 on a /revise note: the coder changed nothing. Noop. The only file in this pull request is .github/workflows/release.yml, and the survey refuses every path under .github/ to this agent. So I changed nothing. The finding has two parts. Part 1 (the secret is compiled into public binaries) does not hold: crates/minimal/src/cmd/auth.rs:24-27 documents BUILT_IN_SECRET as 'The App's public client secret', and the browser flow already uses PKCE (code_challenge S256, code_verifier). GitHub App token exchange requires client_secret even with PKCE, so the design treats it as public and relies on PKCE and the 127.0.0.1 redirect. Part 2 (scope the env to the cargo build step, not the job) is a fair hardening. It needs an edit to .github/workflows/release.yml at the three job env blocks (near lines 194, 339 and 473), which a person with CODEOWNER rights must make. I ran no tests because I changed no file. Review threads the coder judged:
|
|
Correcting myself after the revise round's note: the code already does both halves of what I asked for on the model. |
…app-client-secret
Move MINIMAL_GITHUB_APP_CLIENT_SECRET from the job-level env of the three build jobs to the one step in each that compiles `min`: "Build static release binaries" (scripts/dist-build.sh) on Linux amd64 and arm64, and "Build release minimal binary" on macOS. No other step sees it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WhCC5TEBNzPeWgytRsU4rc
|
@bryan-minimal the client secret is now scoped to a step, in 95b4bab. In all three build jobs it is set only on the step that compiles Generated by Claude Code |
|
/review |
There was a problem hiding this comment.
Both asks met in substance: step-scoped since 95b4bab, and the public-by-construction point is in the PR body; the one-line comment above the env can follow. Needed for tonight's nightly so released min offers the browser flow.
What
The three build jobs that compile
min(build-release-linux-amd64,build-release-linux-arm64,build-release-macos-arm64) now passMINIMAL_GITHUB_APP_CLIENT_SECRETfrom the repository secret of that name. The secret is already set.Why
min auth loginsigns in under the Minimal-published GitHub App (client idIv23liv6KetAFJp4Eb5K). The browser flow needs the App's client secret, whichminreads at compile time withoption_env!. A build without it offers only the device flow. This is the sign-in task in the BEP plan, #2263, under epic #1501.envof the one step in each job that compilesmin: "Build static release binaries" on Linux amd64 and arm64, and "Build release minimal binary" on macOS. No other step and no other job sees it.minbinary by design (crates/minimal/src/cmd/auth.rs), and the browser flow uses PKCE.secrets: inherit, so nightly builds get it too.Checks
actionlint .github/workflows/release.yml: clean.zizmor --offline: no findings before or after (26 suppressed, unchanged), checked on the job-level version.Summary by CodeRabbit