Skip to content

fix: unsigned pkg step on macOS bash 3.2; tighten tag and runbook checks - #13

Merged
Keith Oak (keith-oak) merged 2 commits into
mainfrom
fix/macos-app-release-review
Sep 11, 2026
Merged

Keith Oak (keith-oak) merged 2 commits into
mainfrom
fix/macos-app-release-review

Conversation

@keith-oak

Copy link
Copy Markdown
Member

Summary

Follow-up to #12, from the first dry runs and the late Copilot review.

  • Bug: macOS runners run steps with /bin/bash 3.2, where "${sign[@]}" on an empty array under set -u is unbound variable. The pkg step died on every unsigned build (sitrep run 34550186853, timer run 34550199874). It now uses ${sign[@]+"${sign[@]}"} (same for the keychain list), and missing is a string.
  • Tag builds require the v prefix before publishing.
  • Runbook: the Key Vault block runs in an err_exit/pipe_fail subshell inside { } always { }, so a failure stops it and the vault still closes. VAULT="<vault-name>" now parses in zsh. Pkg verification adds stapler validate and spctl --type install.

Review comments not taken

  • "environment resolves in the called workflow's repo": it doesn't. The feat: reusable macOS app release workflow and runbook #12 dry runs waited on each caller's own release environment (sitrep env 21694444568, timer 21694446319), and this repo has no release environment.
  • "productbuild doesn't accept --keychain": man productbuild documents --keychain keychain-path for signed product archives.

Test plan

  • actionlint clean
  • unsigned-path steps (meta → assemble → ad-hoc sign → verify → zip/pkg → checksums → cleanup) run locally under /bin/bash 3.2 with -e -o pipefail
  • dry runs from sitrep and timer re-pinned to the merge commit

The runner's /bin/bash is 3.2, where expanding an empty array under set -u
is an unbound-variable error. That killed the pkg step on every unsigned
build: dry runs of Sitrep and TimeIT both failed there. Empty-safe
expansions replace it, and the missing-secrets list becomes a string.

From review: tag builds now insist on the v prefix, so a broad caller
trigger can't publish an arbitrary tag. The runbook's Key Vault block stops
at the first failure while still closing the vault, its placeholder parses
as zsh, and pkg verification checks the stapled notarisation ticket as well
as the Installer signature.
Copilot AI lite review requested due to automatic review settings September 11, 2026 01:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the macOS release workflow and runbook for Bash 3.2 compatibility, tag validation, Key Vault handling, and package verification.

Changes:

  • Safely handles empty arrays under set -u.
  • Requires v-prefixed release tags.
  • Improves signing-input checks and package verification.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Summary
docs/releasing-macos-apps.md Updates Key Vault handling and package verification instructions.
.github/workflows/macos-app-release.yml Adds Bash 3.2-safe packaging and tag validation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/releasing-macos-apps.md
The cleanup and the closing check now print an explicit failure if the
temporary IP rule can't be removed or confirmed gone, instead of relying on
someone noticing a non-empty rule list.
Copilot AI review requested due to automatic review settings September 11, 2026 01:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

docs/releasing-macos-apps.md:114

  • err_exit is enabled only inside the subshell that starts here, but the preceding az keyvault network-rule add runs outside it. If opening the vault fails, zsh continues into the secret-writing block instead of stopping at the first failure and entering this always cleanup. Move the add command into this try block so its failure is covered by the documented fail-fast/cleanup guarantee.
  setopt err_exit pipe_fail   # stop at the first failure; the always block still closes the vault

docs/releasing-macos-apps.md:114

  • Setting err_exit only inside the ( ... ) subshell stops that subshell, but the parent zsh does not act on its failure status. After a failed az/gh command, cleanup runs and the final vault check can succeed, so the runbook exits 0 despite a partial secret copy. Capture the try-block status and return/exit non-zero after cleanup while retaining the close check.
{ (
  setopt err_exit pipe_fail   # stop at the first failure; the always block still closes the vault

docs/releasing-macos-apps.md:141

  • This is a substring check rather than an exact network-rule check. For example, if MYIP is 1.2.3.4, a different rule such as 11.2.3.45 contains that text and makes a successful removal report as unconfirmed. Match complete lines in rules so the vault-close verification is reliable.
  && [[ $rules != *"$MYIP"* ]] \

@keith-oak
Keith Oak (keith-oak) merged commit baee130 into main Sep 11, 2026
10 checks passed
@keith-oak
Keith Oak (keith-oak) deleted the fix/macos-app-release-review branch September 11, 2026 01:31
Keith Oak (keith-oak) added a commit that referenced this pull request Oct 6, 2026
…cks (#13)

## Summary

Follow-up to #12, from the first dry runs and the late Copilot review.

- **Bug:** macOS runners run steps with `/bin/bash` 3.2, where
`"${sign[@]}"` on an empty array under `set -u` is `unbound variable`.
The pkg step died on every unsigned build (sitrep run 34550186853, timer
run 34550199874). It now uses `${sign[@]+"${sign[@]}"}` (same for the
keychain list), and `missing` is a string.
- Tag builds require the `v` prefix before publishing.
- Runbook: the Key Vault block runs in an `err_exit`/`pipe_fail`
subshell inside `{ } always { }`, so a failure stops it and the vault
still closes. `VAULT="<vault-name>"` now parses in zsh. Pkg verification
adds `stapler validate` and `spctl --type install`.

## Review comments not taken

- *"environment resolves in the called workflow's repo"*: it doesn't.
The #12 dry runs waited on each caller's own `release` environment
(sitrep env 21694444568, timer 21694446319), and this repo has no
`release` environment.
- *"productbuild doesn't accept `--keychain`"*: `man productbuild`
documents `--keychain keychain-path` for signed product archives.

## Test plan

- [x] `actionlint` clean
- [x] unsigned-path steps (meta → assemble → ad-hoc sign → verify →
zip/pkg → checksums → cleanup) run locally under `/bin/bash` 3.2 with
`-e -o pipefail`
- [ ] dry runs from sitrep and timer re-pinned to the merge commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants