From 9e7d9676fb0fb7747c772b29517ca82bf7c16a87 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 15 May 2026 09:52:28 -0700 Subject: [PATCH] fix(setup): surface codesign failures, verify signatures, exit non-zero on Apple Silicon Closes #1254. Three changes to the codesign loop in setup (lines 288-303): 1. Capture codesign stderr to a tempfile and print it via the log helper. The previous `2>/dev/null` swallowed every diagnostic, so users hit SIGKILL minutes later with no breadcrumb explaining what went wrong. 2. Run `codesign --verify --strict` after signing to catch silent corruption. The reporter's repro had 5/5 binaries unsigned despite a clean setup exit; the verify step makes that case visible at setup time. 3. Track failures and exit 1 when any binary fails to sign or verify. The previous `log "warning: ..."` returned 0 so setup silently succeeded; now it fails loud per the CLAUDE.md "never silent failures" guidance. The Darwin+arm64 guard is unchanged. Other platforms see no behavior change. The codesign machinery itself is fine (the reporter's manual `codesign --remove-signature && codesign -s - -f` recipe works on the same machine); only the wrapper needed hardening. test/setup-codesign.test.ts: 6 pass / 0 fail. Old assertions about 'warning, not fatal' replaced with assertions for the new fail-loud behavior. Existing for-loop coverage expanded to assert all 5 binaries (the setup loop already had 5; the test only checked 4). Reported by @lucascaro. --- setup | 41 ++++++++++++++++++++++++++++++++++--- test/setup-codesign.test.ts | 29 +++++++++++++++----------- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/setup b/setup index b51fed83df..f2e856a953 100755 --- a/setup +++ b/setup @@ -292,14 +292,49 @@ if [ "$NEEDS_BUILD" -eq 1 ]; then # signature block is corrupt. This is idempotent and costs <1s. # See: https://github.com/garrytan/gstack/issues/997 if [ "$(uname -s)" = "Darwin" ] && [ "$(uname -m)" = "arm64" ]; then + _codesign_failures=0 + _codesign_err="$(mktemp)" + _print_codesign_err() { + if [ -s "$_codesign_err" ]; then + while IFS= read -r _codesign_line; do + log " $_codesign_line" + done < "$_codesign_err" + else + log " (no stderr output)" + fi + } + for _bin in browse/dist/browse browse/dist/find-browse design/dist/design make-pdf/dist/pdf bin/gstack-global-discover; do _bin_path="$SOURCE_GSTACK_DIR/$_bin" [ -f "$_bin_path" ] && [ -x "$_bin_path" ] || continue - codesign --remove-signature "$_bin_path" 2>/dev/null || true - if ! codesign -s - -f "$_bin_path" 2>/dev/null; then - log "warning: codesign failed for $_bin (binary may not run on Apple Silicon)" + + : > "$_codesign_err" + if ! codesign --remove-signature "$_bin_path" 2>"$_codesign_err"; then + log "warning: codesign --remove-signature failed for $_bin:" + _print_codesign_err + fi + + : > "$_codesign_err" + if ! codesign -s - -f "$_bin_path" 2>"$_codesign_err"; then + log "error: codesign failed for $_bin:" + _print_codesign_err + _codesign_failures=$((_codesign_failures + 1)) + continue + fi + + : > "$_codesign_err" + if ! codesign --verify --strict "$_bin_path" 2>"$_codesign_err"; then + log "error: codesign verification failed for $_bin:" + _print_codesign_err + _codesign_failures=$((_codesign_failures + 1)) fi done + + rm -f "$_codesign_err" + if [ "$_codesign_failures" -gt 0 ]; then + echo "gstack setup failed: $_codesign_failures binaries did not codesign. Apple Silicon will SIGKILL them on first run. See output above for codesign errors." >&2 + exit 1 + fi fi # macOS: install coreutils for `gtimeout` (Codex hang protection in /codex + /autoplan). diff --git a/test/setup-codesign.test.ts b/test/setup-codesign.test.ts index 1ac7a4982c..76ec1fe378 100644 --- a/test/setup-codesign.test.ts +++ b/test/setup-codesign.test.ts @@ -24,10 +24,11 @@ describe('setup: Apple Silicon codesign', () => { const forMatch = content.match(/for _bin in ([^;]+);/); expect(forMatch).toBeTruthy(); const binaries = forMatch![1].trim().split(/\s+/); - // All four compiled binaries from `bun run build` must be covered + // All compiled binaries from `bun run build` must be covered expect(binaries).toContain('browse/dist/browse'); expect(binaries).toContain('browse/dist/find-browse'); expect(binaries).toContain('design/dist/design'); + expect(binaries).toContain('make-pdf/dist/pdf'); expect(binaries).toContain('bin/gstack-global-discover'); }); @@ -49,23 +50,27 @@ describe('setup: Apple Silicon codesign', () => { expect(content).toContain('[ -f "$_bin_path" ] && [ -x "$_bin_path" ] || continue'); }); - test('codesign failure is a warning, not a fatal error', () => { + test('codesign failures surface stderr, verify signatures, and fail setup', () => { const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); - // On codesign failure, log a warning but don't exit - expect(content).toContain('warning: codesign failed for'); - // Should NOT have `set -e` causing exit on codesign failure - // (the `|| true` after --remove-signature and the if-guard around -s - -f handle this) - expect(content).toContain('codesign --remove-signature "$_bin_path" 2>/dev/null || true'); + expect(content).toContain('_codesign_err="$(mktemp)"'); + expect(content).toContain('codesign --remove-signature "$_bin_path" 2>"$_codesign_err"'); + expect(content).toContain('codesign -s - -f "$_bin_path" 2>"$_codesign_err"'); + expect(content).toContain('codesign --verify --strict "$_bin_path" 2>"$_codesign_err"'); + expect(content).toContain('_print_codesign_err'); + expect(content).toContain('_codesign_failures=$((_codesign_failures + 1))'); + expect(content).toContain('gstack setup failed: $_codesign_failures binaries did not codesign'); + expect(content).toContain('exit 1'); + expect(content).not.toContain('codesign --remove-signature "$_bin_path" 2>/dev/null || true'); }); test('codesign shell snippet is syntactically valid', () => { // Extract the codesign block and validate it parses as bash const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); - const match = content.match( - /# macOS Apple Silicon: ad-hoc codesign[\s\S]*?done\n\s*fi/ - ); - expect(match).toBeTruthy(); - const snippet = match![0]; + const start = content.indexOf('# macOS Apple Silicon: ad-hoc codesign'); + const end = content.indexOf('# macOS: install coreutils', start); + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + const snippet = content.slice(start, end); // Wrap in a function to make it a complete script, then syntax-check const testScript = `#!/usr/bin/env bash\nset -e\n_test_fn() {\n${snippet}\n}\n`; const result = spawnSync('bash', ['-n', '-c', testScript], {