mirror of https://github.com/garrytan/gstack.git
fix(setup): truncate Bun trailing padding before codesign, stop false-alarm warning
On Apple Silicon, `./setup` warned "codesign failed for browse/dist/find-browse (binary may not run on Apple Silicon)" and the same for bin/gstack-global-discover. The binaries actually run fine — the warning was a false alarm. Root cause: `bun build --compile` leaves ~19KB of trailing zero-padding AFTER the Mach-O LC_CODE_SIGNATURE region for these two binaries (browse/design/pdf have 0 trailing bytes). macOS codesign requires the signature to be the last content and extend to EOF, so the padding makes `codesign --remove-signature` fail with "internal error in Code Signing subsystem" and `codesign -s - -f` fail with "main executable failed strict validation". The re-sign fails, leaving Bun's original adhoc linker-signed signature in place — which still satisfies the kernel's exec-time check, so the binary runs despite `codesign --verify` being unhappy. Fix (two parts): 1. Truncate trailing bytes past LC_CODE_SIGNATURE (computed via otool dataoff+ datasize) before signing. Verified: the identical `codesign -s - -f` that failed on the full file succeeds on the truncated file, and `codesign --verify --strict` then passes. Degrades safely if otool is unavailable. 2. When re-sign still fails, probe the binary and reserve the scary "may not run" warning for genuine SIGKILL (exit 137); otherwise emit an informational note. Probe is set -e safe (`|| _probe_rc=$?`). Adds two regression tests to test/setup-codesign.test.ts (truncation logic + SIGKILL-gated warning). Both fail on the pre-fix setup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Brandon Lipman
parent
a6fb31726c
commit
7e07964d13
34
setup
34
setup
|
|
@ -321,14 +321,46 @@ if [ "$NEEDS_BUILD" -eq 1 ]; then
|
|||
# macOS kills with SIGKILL (exit 137). The two-step remove+re-sign is
|
||||
# required because a naive `codesign -s - -f` fails when the existing
|
||||
# signature block is corrupt. This is idempotent and costs <1s.
|
||||
#
|
||||
# Some binaries (observed: find-browse, gstack-global-discover) also carry
|
||||
# trailing zero-padding AFTER the Mach-O LC_CODE_SIGNATURE region. macOS
|
||||
# codesign requires the signature to be the last content and extend to EOF,
|
||||
# so the padding triggers "main executable failed strict validation" on
|
||||
# re-sign (and "internal error in Code Signing subsystem" on remove). We
|
||||
# truncate that trailing slack to the end of LC_CODE_SIGNATURE first, which
|
||||
# lets the identical re-sign succeed. The binary runs either way: Bun's
|
||||
# adhoc code-page signature satisfies the kernel's exec check even when
|
||||
# `codesign --verify` is unhappy, so a re-sign failure only warns when the
|
||||
# binary is genuinely SIGKILL'd on exec (exit 137).
|
||||
# See: https://github.com/garrytan/gstack/issues/997
|
||||
if [ "$(uname -s)" = "Darwin" ] && [ "$(uname -m)" = "arm64" ]; then
|
||||
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
|
||||
# Strip any trailing bytes past LC_CODE_SIGNATURE so codesign can re-sign.
|
||||
# otool prints the signature's dataoff+datasize; if the file is larger,
|
||||
# the extra bytes are Bun padding that breaks strict validation.
|
||||
_sig_end=$(otool -l "$_bin_path" 2>/dev/null | awk '/LC_CODE_SIGNATURE/{f=1} f&&/dataoff/{o=$2} f&&/datasize/{print o+$2; exit}')
|
||||
_fsize=$(stat -f%z "$_bin_path" 2>/dev/null)
|
||||
if [ -n "$_sig_end" ] && [ -n "$_fsize" ] && [ "$_sig_end" -gt 0 ] 2>/dev/null && [ "$_sig_end" -lt "$_fsize" ] 2>/dev/null; then
|
||||
_trunc_tmp=$(mktemp 2>/dev/null) || _trunc_tmp=""
|
||||
if [ -n "$_trunc_tmp" ] && head -c "$_sig_end" "$_bin_path" > "$_trunc_tmp" 2>/dev/null; then
|
||||
cat "$_trunc_tmp" > "$_bin_path" && chmod +x "$_bin_path"
|
||||
fi
|
||||
[ -n "$_trunc_tmp" ] && rm -f "$_trunc_tmp"
|
||||
fi
|
||||
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)"
|
||||
# Re-sign failed. Only warn if the binary genuinely cannot execute
|
||||
# (SIGKILL = exit 137). Otherwise Bun's adhoc code-page signature still
|
||||
# runs fine and the codesign --verify miss is cosmetic. set -e safe.
|
||||
_probe_rc=0
|
||||
"$_bin_path" --help >/dev/null 2>&1 || _probe_rc=$?
|
||||
if [ "$_probe_rc" -eq 137 ]; then
|
||||
log "warning: codesign failed for $_bin and it is SIGKILL'd on exec (exit 137) — it may not run on Apple Silicon"
|
||||
else
|
||||
log "note: codesign could not re-sign $_bin, but it executes fine (Bun adhoc signature); continuing"
|
||||
fi
|
||||
fi
|
||||
done
|
||||
fi
|
||||
|
|
|
|||
|
|
@ -1,78 +1,105 @@
|
|||
import { describe, test, expect } from 'bun:test';
|
||||
import { spawnSync } from 'child_process';
|
||||
import * as path from 'path';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import { describe, test, expect } from "bun:test";
|
||||
import { spawnSync } from "child_process";
|
||||
import * as path from "path";
|
||||
import * as fs from "fs";
|
||||
import * as os from "os";
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const SETUP_SCRIPT = path.join(ROOT, 'setup');
|
||||
const ROOT = path.resolve(import.meta.dir, "..");
|
||||
const SETUP_SCRIPT = path.join(ROOT, "setup");
|
||||
|
||||
describe('setup: Apple Silicon codesign', () => {
|
||||
test('setup script contains codesign block for Darwin arm64', () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8');
|
||||
describe("setup: Apple Silicon codesign", () => {
|
||||
test("setup script contains codesign block for Darwin arm64", () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, "utf-8");
|
||||
// Verify the codesign guard checks both Darwin and arm64
|
||||
expect(content).toContain('$(uname -s)" = "Darwin"');
|
||||
expect(content).toContain('$(uname -m)" = "arm64"');
|
||||
// Verify remove-then-resign two-step pattern
|
||||
expect(content).toContain('codesign --remove-signature');
|
||||
expect(content).toContain('codesign -s - -f');
|
||||
expect(content).toContain("codesign --remove-signature");
|
||||
expect(content).toContain("codesign -s - -f");
|
||||
});
|
||||
|
||||
test('codesign block covers all compiled binaries', () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8');
|
||||
test("codesign block covers all compiled binaries", () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, "utf-8");
|
||||
// Extract the binaries from the codesign for-loop
|
||||
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
|
||||
expect(binaries).toContain('browse/dist/browse');
|
||||
expect(binaries).toContain('browse/dist/find-browse');
|
||||
expect(binaries).toContain('design/dist/design');
|
||||
expect(binaries).toContain('bin/gstack-global-discover');
|
||||
expect(binaries).toContain("browse/dist/browse");
|
||||
expect(binaries).toContain("browse/dist/find-browse");
|
||||
expect(binaries).toContain("design/dist/design");
|
||||
expect(binaries).toContain("bin/gstack-global-discover");
|
||||
});
|
||||
|
||||
test('codesign block is inside the NEEDS_BUILD=1 branch', () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8');
|
||||
test("codesign block is inside the NEEDS_BUILD=1 branch", () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, "utf-8");
|
||||
// The codesign block should appear after the build command and before the
|
||||
// `if [ ! -x "$BROWSE_BIN" ]` guard that checks the build succeeded. The
|
||||
// setup script invokes the build via `bun_cmd run build` (not literal
|
||||
// `bun run build`) so the wrapper can route through asdf/volta/etc;
|
||||
// matching the wrapped form keeps this test stable across that indirection.
|
||||
const buildIdx = content.indexOf('bun_cmd run build');
|
||||
const codesignIdx = content.indexOf('codesign --remove-signature');
|
||||
const browseCheckIdx = content.indexOf('gstack setup failed: browse binary missing');
|
||||
const buildIdx = content.indexOf("bun_cmd run build");
|
||||
const codesignIdx = content.indexOf("codesign --remove-signature");
|
||||
const browseCheckIdx = content.indexOf(
|
||||
"gstack setup failed: browse binary missing",
|
||||
);
|
||||
expect(buildIdx).toBeGreaterThan(-1);
|
||||
expect(codesignIdx).toBeGreaterThan(buildIdx);
|
||||
expect(browseCheckIdx).toBeGreaterThan(codesignIdx);
|
||||
});
|
||||
|
||||
test('codesign block is idempotent (skips missing binaries)', () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8');
|
||||
test("codesign block is idempotent (skips missing binaries)", () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, "utf-8");
|
||||
// The loop must guard with a file-existence + executable check before codesigning
|
||||
expect(content).toContain('[ -f "$_bin_path" ] && [ -x "$_bin_path" ] || continue');
|
||||
expect(content).toContain(
|
||||
'[ -f "$_bin_path" ] && [ -x "$_bin_path" ] || continue',
|
||||
);
|
||||
});
|
||||
|
||||
test('codesign failure is a warning, not a fatal error', () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8');
|
||||
test("codesign failure is a warning, not a fatal error", () => {
|
||||
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');
|
||||
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 --remove-signature "$_bin_path" 2>/dev/null || true',
|
||||
);
|
||||
});
|
||||
|
||||
test('codesign shell snippet is syntactically valid', () => {
|
||||
test("codesign block truncates trailing data past LC_CODE_SIGNATURE", () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, "utf-8");
|
||||
// Bun --compile can leave zero-padding after the signature region, which
|
||||
// breaks `codesign -s - -f` with "main executable failed strict
|
||||
// validation". Setup must compute the signature end via otool and truncate
|
||||
// the file to it before signing.
|
||||
expect(content).toContain("LC_CODE_SIGNATURE");
|
||||
expect(content).toContain('head -c "$_sig_end"');
|
||||
expect(content).toContain('"$_sig_end" -lt "$_fsize"');
|
||||
});
|
||||
|
||||
test("re-sign failure only warns when the binary is SIGKILLed (exit 137)", () => {
|
||||
const content = fs.readFileSync(SETUP_SCRIPT, "utf-8");
|
||||
// A failed re-sign is not fatal and not always a real problem: Bun's adhoc
|
||||
// code-page signature still satisfies the kernel. Setup must probe the
|
||||
// binary and reserve the scary "may not run" warning for exit 137.
|
||||
expect(content).toContain('"$_probe_rc" -eq 137');
|
||||
// The probe must be set -e safe (|| _probe_rc=$?), since setup runs set -e.
|
||||
expect(content).toContain("|| _probe_rc=$?");
|
||||
});
|
||||
|
||||
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 content = fs.readFileSync(SETUP_SCRIPT, "utf-8");
|
||||
const match = content.match(
|
||||
/# macOS Apple Silicon: ad-hoc codesign[\s\S]*?done\n\s*fi/
|
||||
/# macOS Apple Silicon: ad-hoc codesign[\s\S]*?done\n\s*fi/,
|
||||
);
|
||||
expect(match).toBeTruthy();
|
||||
const snippet = match![0];
|
||||
// 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], {
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
const result = spawnSync("bash", ["-n", "-c", testScript], {
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
timeout: 5000,
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
|
|
|
|||
Loading…
Reference in New Issue