Repository navigation
Simplify macOS signing failure handling - #398
Conversation
There was a problem hiding this comment.
Pull request overview
Enables strict shell failure handling for the macOS Buildkite signing workflow while preserving credential secrecy and cleanup behavior.
Changes:
- Enables
set -eufor the macOS build step. - Simplifies signing-script validation and failure propagation.
- Adds coverage for caller options and gem-installation failures.
No findings across architecture, security, performance, cross-platform behavior, or tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.buildkite/pipeline.yml |
Enables strict mode before signing setup. |
.buildkite/commands/setup_macos_code_signing.sh |
Simplifies validation and failure handling. |
tests/unit/macos-signing.test.cjs |
Tests strict-mode integration and failures. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -2,17 +2,14 @@ | |||
|
|
|||
| echo "~~~ :apple: Configure macOS code signing" | |||
There was a problem hiding this comment.
I think we should better organize the log sections here and properly group items related to code signing together.
Because right now not only there are 2 sections about code signing interrupted by an install_gems section in the middle, but also the --- from install_gems is a section of higher level than ~~~ but here they're mixed wrong.
I suggest reorganizing things like this instead:
echo "--- :ruby: Install gems"
install_gems
echo "--- :key: Configure macOS code signing"
… the 2 exports, mktemp, 3rd export, cleanup trap, writing of the key, and call to 'fastlane setup_code_signing'
Why
Following up to the discussion on #396, this PR updates the the macOS signing setup script, which is
sourced, so its shebang's-euflags are ignored. We then enable strict mode in the pipeline itself to cover the whole macOS build step.What changes
set -eu.Tests mirror the caller behavior and cover gem installation failures.
How to test this
npm run lint+npm test.Risks and limitations
This affects only macOS CI. Real signing can only be exercised in Buildkite; local tests stub the external signing operations. Strict mode intentionally stops the macOS step on any command failure or unset variable.
Related
Follow-up to #396.
Design decisions and alternatives considered
Strict mode belongs to the caller because it governs the full command sequence. The private key still uses
printenvbecause${VAR:?…}could expose the secret when shell tracing is enabled.Review outcome (required — see AGENTS.md)
0
[fix here]· 0[follow-up]. No findings.