Skip to content

Commit ad2a336

Browse files
robertsLandoclaude
andauthored
fix(sea): match blob generator to target Node version (#247)
* fix(sea): skip mach-O __LINKEDIT patch for SEA binaries The classic pkg flow appends the VFS payload to the end of the binary and uses patchMachOExecutable to extend __LINKEDIT so codesign hashes cover the payload. For SEA, postject creates a dedicated NODE_SEA segment — patching __LINKEDIT on top of that corrupts the SEA blob on macOS arm64 once the payload is non-trivial (reported for NestJS apps in discussion #236, 9 MB enhanced-SEA blob → segfault at LoadSingleExecutableApplication). Split the ad-hoc-sign path used by SEA into signMacOSSeaIfNeeded, which matches the Node.js SEA docs: just codesign, no LINKEDIT patch. signMacOSIfNeeded still patches for the non-SEA producer path. Refs: #236 * refactor(sea): collapse signMacOS{,Sea}IfNeeded behind isSea flag Per review on #247 — a dedicated second function was more code than the branch deserved. Replace it with a single isSea parameter that skips the __LINKEDIT patch when true. Call sites read clearer too. * fix(sea): drop redundant macOS signature pre-strip codesign -f --sign - in signMacOSIfNeeded already force-replaces the existing signature after postject injection, so stripping it first is redundant. On macOS Tahoe 26.x with non-trivial SEA payloads (NestJS, ~9 MB blob) the pre-strip leaves the Mach-O in a state that crashes Node at load time with "v8::ToLocalChecked Empty MaybeLocal" inside node::sea::LoadSingleExecutableApplication. Cross-host Linux-to-macOS builds never pre-stripped and have been confirmed working on the same payload by the reporter, which pins the regression to this step. Refs: discussion #236 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sea): pick blob generator by host/target platform match, not major The SEA prep-blob layout changes between Node patch releases within the same major line (e.g. 22.19/22.20 added fields that break the 22.22 reader), so generating the blob with a host Node whose patch release differs from the downloaded target binary crashes node::sea::FindSingleExecutableResource at startup with EXC_BAD_ACCESS inside BlobDeserializer::ReadArithmetic<unsigned long>. Fix: pick a downloaded target binary whose platform+arch matches the host. That binary is the exact version we inject the blob into, so generator and reader are byte-for-byte version-matched. Fall back to process.execPath only when no target is runnable on the host (true cross-platform builds, e.g. Linux → Windows). Reproduced locally: host Node 22.12.0/22.15.0/22.17.1/22.18.0 → target node22-linux-x64 (downloaded 22.22.2) crashed with exit 139 and a stack trace matching the one reported by @julianpoemp on macOS arm64 in discussion #236. After this fix, every one of those host versions produces a working binary. Refs: discussion #236 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(sea): reuse hostPlatform/hostArch from pkg-fetch pickBlobGeneratorBinary had an inline `process.platform === 'darwin' ? 'macos' : ...` branch and read `process.arch` directly. pkg-fetch's `system` module already exports `hostPlatform` / `hostArch` as the "fancy" values (macos/win/linux) that target descriptors use, and the rest of lib/ consumes them — same translation, one source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(sea): correct inline comments on macOS signing simplifications Earlier commits in this PR (3d932d0 skip __LINKEDIT patch, a24854e drop pre-strip) landed while we believed the macOS arm64 crash in discussion #236 was caused by those steps corrupting the SEA blob. The actual cause turned out to be host/target Node patch-version skew in the blob generator (fixed in 6f456e2), not either of these. The code changes are still valid cleanups — patchMachOExecutable is a no-op on SEA binaries because __LINKEDIT already sits at the file tail, and the codesign pre-strip is redundant because codesign -f --sign - replaces any existing signature — but the inline comments claimed they were fixing corruption bugs that don't exist. Replaced those comments with accurate no-op / redundancy reasoning so future maintainers chasing mach-O or codesign bugs aren't led astray. Code behaviour is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sea): download host-platform node for pure cross-compile blob gen When no target matches the host platform/arch (e.g. Linux host building only a macos-arm64 binary), the previous fallback to process.execPath re-opened the same SEA blob version-skew footgun that 6f456e2 closed for the host-matching case: if the user's local node is on a different patch release than the downloaded target binary, the generated blob crashes node::sea::FindSingleExecutableResource at startup with EXC_BAD_ACCESS (discussion #236). Now we download a host-platform/arch node binary at the same node range as the targets and use it purely as the blob generator — byte-for-byte version-matched with the reader baked into every target. process.execPath is only used as a last-resort fallback if the download itself fails (unsupported host platform such as alpine/musl, no network, etc.), and that path now emits a warning pointing at the exact symptom. Verified: - Linux host 22.12.0 → macos-arm64-only target: previously would silently bake a skewed blob; now logs "No target matches host" + downloads node-v22.22.2-linux-x64 for generation. NODE_SEA filesize matches the known-good build byte-for-byte. - Host-matching and mixed-target flows still take the fast path with no extra download. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sea): hard-fail blob generator when host/target versions skew Replace the silent process.execPath fallback with wasReported when the host node version differs from the resolved target version — the old fallback silently reintroduced the very EXC_BAD_ACCESS crash this flow is meant to prevent (discussion #236). Add resolveTargetNodeVersion as the single source of truth for the resolved target version (reused by getNodejsExecutable): queries the user-provided binary via --version for opts.nodePath, uses process.version for opts.useLocalNode, and falls back to the public registry lookup otherwise. Extract pickMatchingHostTargetIndex as a testable helper and cover it with test-00-sea-picker (exact match, no match, cross-arch, alpine host, empty targets). Drop the now-unused removeMachOExecutableSignature from lib/mach-o.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sea): pin cross-compile blob generator to exact target version Address copilot review on #247: - pickBlobGeneratorBinary: resolve target's concrete patch version up front via resolveTargetNodeVersion, pass it as nodeRange for the host-platform download so host and target resolve to the same patch (unofficial builds / arch-specific availability could otherwise diverge and reintroduce the discussion #236 SEA header skew crash). - Short-circuit to process.execPath when process.version already matches the resolved target version (skip the download). - Drop user-supplied nodePath / useLocalNode when invoking the host download so neither can short-circuit the pinned fetch. - test-00-sea-picker: rewrite the "cross-platform" case comment to describe the real invariant (Linux host, no Linux target), since the target list contains both macos and win. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sea): canonicalize Node version to v-prefix everywhere getNodeVersion's 3-part branch returned bare `22.22.2` while every other producer (nodejs.org/dist, process.version, `node --version`) returns `v22.22.2`. Two concrete consequences: 1. pickBlobGeneratorBinary's `targetVersion === process.version` check would false-negative when the user pins the patch (e.g. node22.22.2 target), dropping into the download path unnecessarily. 2. getNodejsExecutable's archive filename was `node-22.22.2-linux-x64.tar.gz` instead of `node-v22.22.2-...`, so the download would 404 for any patch-pinned range. Fix: always return v-prefixed; strip the prefix in the one place that needs a bare semver (the cross-platform `nodeRange` construction, which is re-parsed by getNodeVersion's regex). Also wire test-00-sea-picker into the npmTests list for parity with test-00-sea — the picker test is a pure unit test that runs in every flavor except only-npm; adding it makes both SEA tests behave the same. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(sea): narrow types and move pickBlobGeneratorBinary doc - Add NodeVersion (`v${number}.${number}.${number}`), NodeOs, NodeArch, NodeRange (`node${string}`) template-literal/union types encoding the invariants the runtime already enforces. - Route getNodeOs / getNodeArch / getNodeVersion / resolveTargetNodeVersion through the narrow types so downstream callers (filename construction, `=== process.version` check, cross-platform `nodeRange` build) carry compile-time guarantees instead of passing bare strings. - Simplify getNodeVersion's version search: drop the `.map → tuple → .find` acrobatics and inline parameter type annotations in favour of a single typed `.find` on the parsed response. - Add explicit `Promise<void>` / `Promise<string>` / `number` return annotations on getNodejsExecutable, bake, signMacOSIfNeeded, assertHostSeaNodeVersion, generateSeaBlob, extract. - Move the 30-line "Strategy" JSDoc back onto pickBlobGeneratorBinary (it had drifted above the pickMatchingHostTargetIndex helper after the helper extraction). No behaviour change — types track existing runtime invariants; the only functional adjustment is swapping `targetVersion.replace(/^v/, '')` for `targetVersion.slice(1)` in the cross-platform `nodeRange` construction, safe because NodeVersion guarantees the leading `v`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(types): hoist Node version/os/arch/range types to lib/types.ts NodeVersion, NodeRange, NodeOs, NodeArch (+ their backing NODE_OSES / NODE_ARCHS const tuples) describe invariants that aren't sea-specific — `lib/index.ts` already builds `node<major>` strings by hand, `lib/fabricator.ts` logs `${arch}` tuples, and `compress_type.ts` interpolates `process.version` into messages. Hoisting them next to `platform` in `lib/types.ts` makes them available for reuse without widening the NodeTarget signature (out of scope here — keeping downstream callers untouched). No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sea): optimize trailing slash removal in manifest key lookup --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent af0c086 commit ad2a336

6 files changed

Lines changed: 324 additions & 98 deletions

File tree

lib/mach-o.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,4 @@ function signMachOExecutable(executable: string) {
7272
}
7373
}
7474

75-
function removeMachOExecutableSignature(executable: string) {
76-
execFileSync('codesign', ['--remove-signature', executable], {
77-
stdio: 'inherit',
78-
});
79-
}
80-
81-
export {
82-
patchMachOExecutable,
83-
removeMachOExecutableSignature,
84-
signMachOExecutable,
85-
};
75+
export { patchMachOExecutable, signMachOExecutable };

0 commit comments

Comments
 (0)