chore(ci): reduce the blast radius of the Homebrew tap token (#318)
## Summary
All seven findings from the #313 security review, addressed in `update_homebrew_formula.yml`, with the behavior PR #313 verified left intact. The four release-asset / formula-stanza states still behave exactly as that PR documented, re-verified by executing the committed step bodies.
## How the tap token was scoped
The job cloned the tap from `https://x-access-token:<token>@github.com/lablup/homebrew-tap.git`. Git records that url verbatim as `origin` in `homebrew-tap/.git/config`, so the push token sat in the workspace for the rest of the job, readable by every later step and by anything those steps ran, third-party actions included.
`lablup/homebrew-tap` is public, and has to be for `brew tap lablup/tap` to work, so the clone needs no credential at all and is now anonymous. Only the push carries one:
```
git \
-c credential.helper= \
-c 'credential.helper=!f() { if [ "$1" = get ]; then printf "username=x-access-token\npassword=%s\n" "$HOMEBREW_TAP_TOKEN"; fi; }; f' \
push origin main
```
The token lives in exactly one place: the environment of that one command, declared in the step's `env:`.
**Not disk.** No remote url carries it, and nothing writes it to a config file.
**Not argv,** which is what rules out `-c http.extraheader="Authorization: Basic $(...)"`, the usual replacement. Command lines are readable through `ps`, and a step that turns on `set -x` prints the expanded line into the workflow log. Here the helper string is single-quoted, so bash hands git the name `$HOMEBREW_TAP_TOKEN` and expands nothing. `set -x` prints the same name, and xtrace does not propagate into the shell git spawns for the helper (bash does not export `SHELLOPTS`), so the helper's own `printf` is not traced either. Both halves are asserted by the tests, and the `http.extraheader` variant fails them: it authenticates fine and leaks the raw token into the trace.
**Not the keychain.** The empty `-c credential.helper=` first resets the helper list, dropping the runner image's global helper (osxkeychain on macOS). Without it, git hands the credential to that helper for storage after a successful authentication, which is disk by another route. The helper here answers `get` only, so git's post-authentication `store` call is a no-op, and the `if` makes it exit 0 so git does not treat the silence as a failed helper.
## The seven findings
| # | Finding | Change |
|---|---|---|
| 1 | Token persists in `homebrew-tap/.git/config` | Anonymous clone; credential scoped to the single `git push` through an env-reading `credential.helper`, as above |
| 2 | `permissions: contents: write` is broader than needed | `contents: read`, which is what the `gh api` reads need and which zeroes every other scope |
| 3 | No `concurrency:` group | `group: update-homebrew-formula`, a constant because the contended resource is the tap, `cancel-in-progress: false` so an interrupted run cannot land between `git commit` and `git push` |
| 4 | Release asset list read without pagination | Resolve the release id, then list through the paginated `/releases/{id}/assets` collection with `--paginate` and `per_page=100` |
| 5 | Artifacts downloaded into the tap working tree | Downloaded to `$RUNNER_TEMP/all-smi-artifacts` |
| 6 | Checksums and urls validated independently | Validated as `(url, sha256)` pairs, read out of the formula the same way `set_artifact` writes them |
| 7 | `packaging` environment gates nothing | **Deferred to a human decision, deliberately.** See below |
One drive-by: `actions/checkout` now runs with `persist-credentials: false`. By default it writes `GITHUB_TOKEN` into `.git/config` as an `http.extraheader`, which is the same shape of problem as finding 1. No step performs a git operation in that checkout, so nothing needs it.
### On finding 4, precisely
I could not reproduce truncation of the embedded `assets` array: `electron/electron` returns all 76 of its assets inline, `denoland/deno` all 56. So the array is not visibly capped at any count this repo will reach soon, and our releases currently publish 18 assets, not the 28 the issue cites.
The change still stands on its own terms. The embedded array carries no pagination controls and no `Link` header, so its completeness is not something the API documents, whereas `/releases/{id}/assets` is a documented paginated collection. This workflow reads absence from that list as a decision rather than as missing data: a truncated list is a silent "the release publishes no Intel zip", which either skips an artifact that exists or trips the version-skew refusal in the next step. Depending on undocumented completeness for that is the part worth removing.
### On finding 6, precisely
Checking url and checksum independently accepts a formula in which two artifacts have exchanged checksums. Every checksum is one the run computed, every url names the right release, and the counts are right, so nothing before `brew install` notices; `brew install` then fetches one artifact and verifies it against another's checksum. Swapped is exactly what a rewrite that matched one stanza and wrote into another produces, which is the failure the whole update step is built around avoiding. The test suite includes a control that runs the pre-#316 checks over a swapped formula and confirms they accept it.
### On finding 7, the recorded decision
**The `packaging` environment was not configured, on purpose, and this was not an oversight.** It still has `protection_rules: []` and `deployment_branch_policy: null`, so naming it scopes `HOMEBREW_TAP_TOKEN` to this job but gates nothing.
Adding required reviewers or a deployment branch policy is a repository administration change rather than a workflow change, and it decides who is able to ship a release. That is a call for a maintainer to make and apply, not something to do quietly inside a hardening PR. The issue's acceptance criterion accepts either configuring it or recording an explicit decision not to; this is the recording. The reasoning is also written into a comment on the job, next to `environment: packaging`, so it stays with the thing it describes.
Until someone decides otherwise, the environment is bookkeeping, and what actually limits the token's reach is everything else in this PR.
## Tests
`tests/homebrew-formula-workflow/` is the #313 harness, committed this time so it is repeatable. Step bodies are read out of the committed YAML by step name, so the thing under test is the file that ships rather than a copy that drifts.
```bash
./tests/homebrew-formula-workflow/run-workflow-steps.sh
```
69 assertions, all passing, entirely offline. Nothing in it contacts GitHub or the real tap.
- [x] **state 1** (asset present, stanza present): both macOS stanzas rewritten, each with its own checksum, 4 sha256 stanzas, validation green
- [x] **state 2** (asset absent, stanza absent): `::notice::`, macOS section gains nothing, 3 sha256 stanzas, validation green
- [x] **state 3** (asset present, stanza absent): `::warning::`, the other three still update, validation green at 3, and the final gate is present in the YAML so the job ends red after the push
- [x] **state 4** (asset absent, stanza present): refused in the update step over version skew, before anything could be pushed, and no state recorded
- [x] malformed tap with two Intel stanzas: refused by `set_artifact`'s unchanged uniqueness assertion
- [x] **swapped checksums**: rejected by the new pairing guard, naming the pair that does not exist, with a control assertion confirming the pre-#316 independent checks accept the same file
- [x] stale url left at v0.25.0: rejected
- [x] traversal-shaped tag (`v0.0.0/../../../attacker/repo/...`): refused before any download, #313's guard intact
- [x] artifacts land in `$RUNNER_TEMP`; the tap working tree is `git status --porcelain`-clean afterwards and has no `tmp/`
- [x] asset listing reaches `/releases/{id}/assets` with `--paginate`; an Intel zip sitting last in a 205-name list is still found. The `gh` stub refuses an assets listing without `--paginate` and refuses to serve the embedded array at all, so reverting either fails here
- [x] **the push, executed for real** against a local authenticating Git smart-HTTP server backed by `git http-backend`: the push succeeds, the bare repo receives `bump: all-smi to v0.26.0`, and the server records `Authorization: Basic base64(x-access-token:<token>)`
- [x] after that push: the token appears in no file anywhere under the workspace, not in the clone's `.git/config`, and not in the `set -x` trace, which shows `$HOMEBREW_TAP_TOKEN` unexpanded instead. A pre-poisoned global `credential.helper=store` is neither consulted nor written to
- [x] re-running the push step with no formula change: "nothing to push", exit 0, server never contacted
- [x] structural checks: no step body interpolates a `${{ }}` expression, `permissions` is `contents: read` and nothing else, a `concurrency` group exists, `secrets.HOMEBREW_TAP_TOKEN` is read exactly once into exactly one step's `env:`, and no shell line builds an `x-access-token:` url
- [x] **negative controls**: three deliberately broken copies of the workflow were run through the harness. Reverting the pagination fails 8 assertions, restoring the token-in-clone-url fails the shape check, and swapping the credential scoping for `-c http.extraheader` fails 4, including the trace leak. The tests are not vacuous
- [x] `actionlint` on the workflow: clean, exit 0. `main` currently reports one `SC2086` info, which this PR fixes in passing (`>> $GITHUB_ENV` is now quoted). The one `SC2016` the credential helper attracts is suppressed inline, with the reason, because the unexpanded variable is the point
- [x] `shellcheck -x` on the harness scripts: clean. `python3 -m py_compile` on the Python: clean
- [x] `cargo metadata`: the new `tests/` subdirectory adds no cargo test target (no `.rs` files), so `cargo test` is unaffected
- [x] `make homebrew-formula-workflow` from `tests/`: the new target runs the suite and appears in `make help`
- [x] the harness leaves no background process behind, on a full run and on a SIGINT partway through. It did at first: the server pid was assigned inside a `$(...)`, so it never reached the caller and `stop_git_server` killed nothing
- [x] CI on this PR: Test Suite pass, license/cla pass, Docker Build Check skipped
## Not verified in this environment
- [ ] **A dry run against a real tag.** Not reachable autonomously: it means publishing a release or dispatching the real workflow, and the push target is a production tap. The push path is covered against a local git server instead, which proves the credential mechanics but not GitHub's acceptance of this particular token.
- [ ] **Nothing was pushed to `lablup/homebrew-tap`.** No release, tag, or workflow dispatch was created either.
- [ ] **No GitHub repository settings were changed.** The `packaging` environment is untouched; see finding 7.
- [ ] **`brew install` on a real Intel and Apple Silicon Mac.** Unchanged from #313, which also left this open.
- [ ] **Real GitHub API pagination.** The `--paginate` behavior is exercised against a stub. Live calls confirmed the endpoint and filter shape return the same 18 asset names as the embedded array for `v0.25.0`, but no release exists with enough assets to force a second page.
- [ ] **`concurrency` under an actual overlapping release.** The group is declared and parses; two genuinely concurrent releases were not staged.
Closes #316 J
Jeongkyu Shin committed
7c5b321ab0c3adaa526102cf268ba099db028d62
Parent: 02b2e6d
Committed by GitHub <noreply@github.com>
on 8/5/2026, 9:43:20 AM