Skip to content

Commit 10cfb5c

Browse files
committed
fix(upgrade): replace Bun.mmap with arrayBuffer on macOS to prevent SIGKILL
Bun.mmap() always requests PROT_WRITE regardless of the `shared` flag. On macOS, AMFI code signing enforcement sends an uncatchable SIGKILL for ANY writable mapping (MAP_SHARED or MAP_PRIVATE) on code-signed Mach-O binaries — which all Bun-compiled binaries are. The previous fix (PR #339, { shared: false }) was insufficient because MAP_PRIVATE with PROT_WRITE is also rejected. Sentry telemetry confirmed: zero successful macOS upgrade traces from delta-enabled versions, while Linux worked fine. Use a platform check: on macOS, read the old binary into memory via Bun.file().arrayBuffer() (~100 MB heap, freed after patching). On Linux, keep the zero-heap Bun.mmap() path since ELF binaries have no such restriction.
1 parent becab6f commit 10cfb5c

File tree

2 files changed

+18
-18
lines changed

2 files changed

+18
-18
lines changed

AGENTS.md

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ mock.module("./some-module", () => ({
629629
### Architecture
630630

631631
<!-- lore:019cb8ea-c6f0-75d8-bda7-e32b4e217f92 -->
632-
* **CLI telemetry DSN is public write-onlysafe to embed in install script**: The CLI's own Sentry DSN (\`SENTRY\_CLI\_DSN\` in \`src/lib/constants.ts\`) is a public write-only ingest key (\`1188a86f...@o1.ingest.us.sentry.io/4510776311808000\`). It's already baked into every distributed binary. Safe to hardcode in the bash install script for error reporting via the envelope APIno secrets needed. Opt-out via \`SENTRY\_CLI\_NO\_TELEMETRY=1\` (same env var the binary checks). Envelope endpoint: \`https://o1.ingest.us.sentry.io/api/{PROJECT\_ID}/envelope/\` with \`x-sentry-auth\` header containing the public key.
632+
* **CLI telemetry DSN is public write-onlysafe to embed in install script**: The CLI's Sentry DSN (\`SENTRY\_CLI\_DSN\` in \`src/lib/constants.ts\`) is a public write-only ingest key already baked into every binary. Safe to hardcode in install scripts. Opt-out: \`SENTRY\_CLI\_NO\_TELEMETRY=1\`.
633633

634634
<!-- lore:019c978a-18b5-7a0d-a55f-b72f7789bdac -->
635635
* **cli.sentry.dev is served from gh-pages branch via GitHub Pages**: \`cli.sentry.dev\` is served from gh-pages branch via GitHub Pages. Craft's gh-pages target runs \`git rm -r -f .\` before extracting docs — persist extra files via \`postReleaseCommand\` in \`.craft.yml\`. Install script supports \`--channel nightly\`, downloading from the \`nightly\` release tag directly. version.json is only used by upgrade/version-check flow.
@@ -669,13 +669,13 @@ mock.module("./some-module", () => ({
669669
* **Bun binary build requires SENTRY\_CLIENT\_ID env var**: The build script (\`script/bundle.ts\`) requires \`SENTRY\_CLIENT\_ID\` environment variable and exits with code 1 if missing. When building locally, use \`bun run --env-file=.env.local build\` or set the env var explicitly. The binary build (\`bun run build\`) also needs it. Without it you get: \`Error: SENTRY\_CLIENT\_ID environment variable is required.\`
670670
671671
<!-- lore:019c9776-e3dd-7632-88b8-358a19506218 -->
672-
* **GitHub immutable releases prevent rolling nightly tag pattern**: getsentry/cli has immutable GitHub releases — assets can't be modified and tags can NEVER be reused. Nightly uses per-version tags (e.g., \`0.13.0-dev.1772062077\`) with API-based latest discovery; deletes all existing assets before uploading. Craft minVersion >= 2.21.0 with no \`preReleaseCommand\` silently skips \`bump-version.sh\` if the only target is \`github\`. Fix: explicitly set \`preReleaseCommand: bash scripts/bump-version.sh\`.
672+
* **GitHub immutable releases prevent rolling nightly tag pattern**: getsentry/cli has immutable GitHub releases — assets can't be modified and tags can NEVER be reused. Nightly uses per-version tags (e.g., \`0.13.0-dev.1772062077\`) with API-based latest discovery. Craft with no \`preReleaseCommand\` silently skips \`bump-version.sh\` if the only target is \`github\` — must explicitly set it.
673673
674674
<!-- lore:019cb8c2-d7b5-780c-8a9f-d20001bc198f -->
675-
* **Install script: BSD sed and awk JSON parsing breaks OCI digest extraction**: The install script parses OCI manifests with awk (no jq dependency). Key trap: \`sed 's/},{/}\n{/g'\` doesn't insert newlines on macOS BSD sed (\`\n\` is literal). Also, the first layer shares a line with the config block after \`\[{\` split. Fix: use a single awk pass tracking last-seen \`"digest"\` value, printing it when \`"org.opencontainers.image.title"\` matches target. Works because \`digest\` always precedes \`annotations\` within each OCI layer object. This avoids sed entirely and handles both GNU/BSD awk. The config digest (\`sha256:44136fa...\`) is a 2-byte \`{}\` blob — downloading it instead of the real binary causes \`gunzip: unexpected end of file\`. The install script now has fire-and-forget Sentry telemetry via \`die()\` + ERR trap, which would catch such failures automatically.
675+
* **Install script: BSD sed and awk JSON parsing breaks OCI digest extraction**: The install script parses OCI manifests with awk (no jq). Key trap: BSD sed \`\n\` is literal, not newline. Fix: single awk pass tracking last-seen \`"digest"\`, printing when \`"org.opencontainers.image.title"\` matches target. The config digest (\`sha256:44136fa...\`) is a 2-byte \`{}\` blob — downloading it instead of the real binary causes \`gunzip: unexpected end of file\`.
676676
677677
<!-- lore:019cb963-cb63-722d-9365-b34336f4766d -->
678-
* **macOS SIGKILL on MAP\_SHARED mmap of signed Mach-O binaries**: macOS AMFI (code signing enforcement) sends SIGKILL when \`MAP\_SHARED\` with \`PROT\_WRITE\` is used on a code-signed Mach-O binary. \`Bun.mmap()\` defaults to \`{ shared: true }\` (MAP\_SHARED). In \`src/lib/bspatch.ts\`, \`Bun.mmap(process.execPath)\` kills the process on macOS during delta upgrades because the running CLI binary is ad-hoc signed (all Bun binaries are). Fix: pass \`{ shared: false }\` for MAP\_PRIVATE. Since the mapping is read-only in practice, no COW pages are allocated — identical performance. Linux ELF binaries have no such restriction.
678+
* **macOS SIGKILL on MAP\_SHARED mmap of signed Mach-O binaries**: macOS AMFI (code signing enforcement) sends uncatchable SIGKILL when \`Bun.mmap()\` is used on code-signed Mach-O binaries. \`Bun.mmap()\` always requests PROT\_WRITE regardless of the \`shared\` flag, and macOS rejects ANY writable mapping (MAP\_SHARED or MAP\_PRIVATE) on signed Mach-O. PR #339's MAP\_PRIVATE fix was insufficient. Fixed by replacing \`Bun.mmap(oldPath)\` with \`new Uint8Array(await Bun.file(oldPath).arrayBuffer())\` in bspatch.ts. Costs ~100 MB heap for the old binary but avoids the uncatchable SIGKILL entirely. Sentry telemetry confirmed: zero successful macOS upgrade traces from delta-enabled versions, while Linux worked fine.
679679
680680
<!-- lore:019c969a-1c90-7041-88a8-4e4d9a51ebed -->
681681
* **Multiple mockFetch calls replace each other — use unified mocks for multi-endpoint tests**: Bun test mocking gotchas: (1) \`mockFetch()\` replaces \`globalThis.fetch\` — calling it twice replaces the first mock. Use a single unified fetch mock dispatching by URL pattern. (2) \`mock.module()\` pollutes the module registry for ALL subsequent test files. Tests using it must live in \`test/isolated/\` and run via \`test:isolated\`. (3) For \`Bun.spawn\`, use direct property assignment in \`beforeEach\`/\`afterEach\`.
@@ -688,9 +688,6 @@ mock.module("./some-module", () => ({
688688
689689
### Pattern
690690
691-
<!-- lore:019c9793-fb1c-7986-936e-57949e9a30d0 -->
692-
* **Markdown table structure for marked-terminal: blank header row + separator + data rows**: Markdown tables for marked-terminal: blank header row (\`| | |\`), separator (\`|---|---|\`), then data rows (\`| \*\*Label\*\* | value |\`). Data rows before separator produce malformed output. Escape user content via \`escapeMarkdownCell()\` in \`src/lib/formatters/markdown.ts\` — backslashes first, then pipes. CodeQL flags incomplete escaping as high severity.
693-
694691
<!-- lore:019c972c-9f11-7c0d-96ce-3f8cc2641175 -->
695692
* **Org-scoped SDK calls follow getOrgSdkConfig + unwrapResult pattern**: All org-scoped API calls in src/lib/api-client.ts: (1) call \`getOrgSdkConfig(orgSlug)\` for regional URL + SDK config, (2) spread into SDK function: \`{ ...config, path: { organization\_id\_or\_slug: orgSlug, ... } }\`, (3) pass to \`unwrapResult(result, errorContext)\`. Shared helpers \`resolveAllTargets\`/\`resolveOrgAndProject\` must NOT call \`fetchProjectId\` — commands that need it enrich targets themselves.
696693
@@ -703,5 +700,5 @@ mock.module("./some-module", () => ({
703700
### Preference
704701
705702
<!-- lore:019c9700-0fc3-730c-82c3-a290d5ecc2ea -->
706-
* **CI scripts: prefer jq/sed over node -e for JSON manipulation**: Reviewer (BYK) prefers using standard Unix tools (\`jq\`, \`sed\`, \`awk\`) over \`node -e\` for simple JSON manipulation in CI workflow scripts. For example, reading/modifying package.json version: \`jq -r .version package.json\` to read, \`jq --arg v "$NEW" '.version = $v' package.json > tmp && mv tmp package.json\` to write. This avoids requiring Node.js to be installed in CI steps that only need basic JSON operations, and is more readable for shell-centric workflows.
703+
* **CI scripts: prefer jq/sed over node -e for JSON manipulation**: Prefer \`jq\`/\`sed\`/\`awk\` over \`node -e\` for JSON manipulation in CI scripts. Example: \`jq -r .version package.json\` to read, \`jq --arg v "$NEW" '.version = $v' package.json > tmp && mv tmp package.json\` to write.
707704
<!-- End lore-managed section -->

src/lib/bspatch.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@
55
* TRDIFF10 format (produced by zig-bsdiff with `--use-zstd`). Designed for
66
* minimal memory usage during CLI self-upgrades:
77
*
8-
* - Old binary: memory-mapped via `Bun.mmap()` (0 JS heap)
8+
* - Old binary: `Bun.mmap()` on Linux (0 JS heap), `arrayBuffer()` on macOS
99
* - Diff/extra blocks: streamed via `DecompressionStream('zstd')`
1010
* - Output: written incrementally to disk via `Bun.file().writer()`
1111
* - Integrity: SHA-256 computed inline via `Bun.CryptoHasher`
1212
*
13-
* Total heap usage: ~1-2 MB regardless of binary size.
13+
* Total heap usage: ~1-2 MB on Linux, ~100 MB on macOS (old file in memory).
1414
*
1515
* TRDIFF10 format (from zig-bsdiff):
1616
* ```
@@ -210,7 +210,7 @@ function createZstdStreamReader(compressed: Uint8Array): BufferedStreamReader {
210210
/**
211211
* Apply a TRDIFF10 binary patch with streaming I/O for minimal memory usage.
212212
*
213-
* Uses `Bun.mmap()` for the old file (0 heap), `DecompressionStream('zstd')`
213+
* Uses `Bun.mmap()` (Linux) or `arrayBuffer()` (macOS) for the old file, `DecompressionStream('zstd')`
214214
* for streaming diff/extra blocks (~16 KB buffers), `Bun.file().writer()`
215215
* for disk output, and `Bun.CryptoHasher` for inline SHA-256 verification.
216216
*
@@ -243,13 +243,16 @@ export async function applyPatch(
243243
);
244244
const extraReader = createZstdStreamReader(patchData.subarray(extraStart));
245245

246-
// Memory-map old file: OS manages pages, 0 JS heap allocation.
247-
// MAP_PRIVATE (shared: false) is required on macOS: the kernel's code
248-
// signing enforcement (AMFI) sends SIGKILL when a MAP_SHARED writable
249-
// mapping is created on a code-signed Mach-O binary (which the running
250-
// CLI is). Since we only read from the mapping, no COW pages are ever
251-
// allocated — identical performance to MAP_SHARED.
252-
const oldFile = Bun.mmap(oldPath, { shared: false });
246+
// On macOS, Bun.mmap() triggers an uncatchable SIGKILL from AMFI code
247+
// signing enforcement — it always requests PROT_WRITE, and macOS rejects
248+
// ANY writable mapping (MAP_SHARED or MAP_PRIVATE) on signed Mach-O
249+
// binaries. Fall back to reading into memory (~100 MB heap, freed after
250+
// patching). On Linux, ELF binaries have no such restriction so we keep
251+
// the zero-heap mmap path.
252+
const oldFile =
253+
process.platform === "darwin"
254+
? new Uint8Array(await Bun.file(oldPath).arrayBuffer())
255+
: Bun.mmap(oldPath);
253256

254257
// Streaming output: write directly to disk, no output buffer in memory
255258
const writer = Bun.file(destPath).writer();

0 commit comments

Comments
 (0)