Skip to content

fix(upgrade): replace Bun.mmap with arrayBuffer on macOS to prevent SIGKILL #340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BYK merged 1 commit into from
Mar 4, 2026
+18 −18
Merged

fix(upgrade): replace Bun.mmap with arrayBuffer on macOS to prevent SIGKILL #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ mock.module("./some-module", () => ({
### Architecture

<!-- lore:019cb8ea-c6f0-75d8-bda7-e32b4e217f92 -->
* **CLI telemetry DSN is public write-only — safe 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 API — no 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.
* **CLI telemetry DSN is public write-only — safe 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\`.

<!-- lore:019c978a-18b5-7a0d-a55f-b72f7789bdac -->
* **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.
Expand Down Expand Up @@ -669,13 +669,13 @@ mock.module("./some-module", () => ({
* **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.\`

<!-- lore:019c9776-e3dd-7632-88b8-358a19506218 -->
* **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\`.
* **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.

<!-- lore:019cb8c2-d7b5-780c-8a9f-d20001bc198f -->
* **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.
* **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\`.

<!-- lore:019cb963-cb63-722d-9365-b34336f4766d -->
* **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.
* **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.

<!-- lore:019c969a-1c90-7041-88a8-4e4d9a51ebed -->
* **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\`.
Expand All @@ -688,9 +688,6 @@ mock.module("./some-module", () => ({

### Pattern

<!-- lore:019c9793-fb1c-7986-936e-57949e9a30d0 -->
* **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.

<!-- lore:019c972c-9f11-7c0d-96ce-3f8cc2641175 -->
* **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.

Expand All @@ -703,5 +700,5 @@ mock.module("./some-module", () => ({
### Preference

<!-- lore:019c9700-0fc3-730c-82c3-a290d5ecc2ea -->
* **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.
* **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.
<!-- End lore-managed section -->
23 changes: 13 additions & 10 deletions src/lib/bspatch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
* TRDIFF10 format (produced by zig-bsdiff with `--use-zstd`). Designed for
* minimal memory usage during CLI self-upgrades:
*
* - Old binary: memory-mapped via `Bun.mmap()` (0 JS heap)
* - Old binary: `Bun.mmap()` on Linux (0 JS heap), `arrayBuffer()` on macOS
* - Diff/extra blocks: streamed via `DecompressionStream('zstd')`
* - Output: written incrementally to disk via `Bun.file().writer()`
* - Integrity: SHA-256 computed inline via `Bun.CryptoHasher`
*
* Total heap usage: ~1-2 MB regardless of binary size.
* Total heap usage: ~1-2 MB on Linux, ~100 MB on macOS (old file in memory).
*
* TRDIFF10 format (from zig-bsdiff):
* ```
Expand Down Expand Up @@ -210,7 +210,7 @@ function createZstdStreamReader(compressed: Uint8Array): BufferedStreamReader {
/**
* Apply a TRDIFF10 binary patch with streaming I/O for minimal memory usage.
*
* Uses `Bun.mmap()` for the old file (0 heap), `DecompressionStream('zstd')`
* Uses `Bun.mmap()` (Linux) or `arrayBuffer()` (macOS) for the old file, `DecompressionStream('zstd')`
* for streaming diff/extra blocks (~16 KB buffers), `Bun.file().writer()`
* for disk output, and `Bun.CryptoHasher` for inline SHA-256 verification.
*
Expand Down Expand Up @@ -243,13 +243,16 @@ export async function applyPatch(
);
const extraReader = createZstdStreamReader(patchData.subarray(extraStart));

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

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