From a22f0a4e7bc5c1156b25792099033516d8a82005 Mon Sep 17 00:00:00 2001 From: ardaaltinors Date: Thu, 12 Mar 2026 10:34:56 +0300 Subject: [PATCH] fix(cli): address review feedback on native binary detection and postinstall - Read only first 4096 bytes of binary header instead of entire file - Add error logging to all catch blocks with specific failure messages - Separate copy vs dlopen catch blocks in postinstall Strategy 1 - Add archCount sanity cap (max 30) for fat Mach-O parsing - Distinguish timeout vs rebuild failure in Strategy 2 --- scripts/native-binary-compat.mjs | 73 +++++++++++++++++--------------- scripts/postinstall.mjs | 36 +++++++++------- 2 files changed, 59 insertions(+), 50 deletions(-) diff --git a/scripts/native-binary-compat.mjs b/scripts/native-binary-compat.mjs index 74fa21ad..1b5ab353 100644 --- a/scripts/native-binary-compat.mjs +++ b/scripts/native-binary-compat.mjs @@ -1,8 +1,11 @@ -import { existsSync, readFileSync } from "node:fs"; +import { existsSync, openSync, readSync, closeSync } from "node:fs"; export const PUBLISHED_BUILD_PLATFORM = "linux"; export const PUBLISHED_BUILD_ARCH = "x64"; +const HEADER_SIZE = 4096; +const MAX_FAT_ARCH_COUNT = 30; + function mapElfMachine(machine) { switch (machine) { case 62: @@ -44,11 +47,11 @@ function readUInt32(buffer, offset, littleEndian) { return littleEndian ? buffer.readUInt32LE(offset) : buffer.readUInt32BE(offset); } +const ELF_MAGIC = 0x7f454c46; + function detectElfTarget(buffer) { if (buffer.length < 20) return null; - if (buffer[0] !== 0x7f || buffer[1] !== 0x45 || buffer[2] !== 0x4c || buffer[3] !== 0x46) { - return null; - } + if (buffer.readUInt32BE(0) !== ELF_MAGIC) return null; const littleEndian = buffer[5] !== 2; const arch = mapElfMachine(readUInt16(buffer, 18, littleEndian)); @@ -57,35 +60,37 @@ function detectElfTarget(buffer) { return { platform: "linux", architectures: [arch] }; } +const THIN_MACH_MAGIC = new Map([ + [0xfeedface, false], + [0xfeedfacf, false], + [0xcefaedfe, true], + [0xcffaedfe, true], +]); +const FAT_MACH_MAGIC = new Map([ + [0xcafebabe, false], + [0xcafebabf, false], + [0xbebafeca, true], + [0xbfbafeca, true], +]); + function detectMachTarget(buffer) { if (buffer.length < 8) return null; const magic = buffer.readUInt32BE(0); - const thinMagic = new Map([ - [0xfeedface, false], - [0xfeedfacf, false], - [0xcefaedfe, true], - [0xcffaedfe, true], - ]); - const fatMagic = new Map([ - [0xcafebabe, false], - [0xcafebabf, false], - [0xbebafeca, true], - [0xbfbafeca, true], - ]); - if (thinMagic.has(magic)) { - const littleEndian = thinMagic.get(magic); + if (THIN_MACH_MAGIC.has(magic)) { + const littleEndian = THIN_MACH_MAGIC.get(magic); const arch = mapMachCpuType(readUInt32(buffer, 4, littleEndian)); if (!arch) return null; return { platform: "darwin", architectures: [arch] }; } - if (!fatMagic.has(magic)) return null; + if (!FAT_MACH_MAGIC.has(magic)) return null; - const littleEndian = fatMagic.get(magic); + const littleEndian = FAT_MACH_MAGIC.get(magic); const isFat64 = magic === 0xcafebabf || magic === 0xbfbafeca; const archCount = readUInt32(buffer, 4, littleEndian); + if (archCount > MAX_FAT_ARCH_COUNT) return null; const entrySize = isFat64 ? 32 : 20; const architectures = new Set(); @@ -102,18 +107,11 @@ function detectMachTarget(buffer) { function detectPeTarget(buffer) { if (buffer.length < 0x40) return null; - if (buffer[0] !== 0x4d || buffer[1] !== 0x5a) return null; + if (buffer.readUInt16LE(0) !== 0x5a4d) return null; const peHeaderOffset = buffer.readUInt32LE(0x3c); if (peHeaderOffset + 6 > buffer.length) return null; - if ( - buffer[peHeaderOffset] !== 0x50 || - buffer[peHeaderOffset + 1] !== 0x45 || - buffer[peHeaderOffset + 2] !== 0x00 || - buffer[peHeaderOffset + 3] !== 0x00 - ) { - return null; - } + if (buffer.readUInt32LE(peHeaderOffset) !== 0x00004550) return null; const arch = mapPeMachine(buffer.readUInt16LE(peHeaderOffset + 4)); if (!arch) return null; @@ -121,16 +119,23 @@ function detectPeTarget(buffer) { } export function detectNativeBinaryTarget(buffer) { - return detectElfTarget(buffer) ?? detectMachTarget(buffer) ?? detectPeTarget(buffer) ?? null; + return detectElfTarget(buffer) ?? detectMachTarget(buffer) ?? detectPeTarget(buffer); } export function readNativeBinaryTarget(binaryPath) { if (!existsSync(binaryPath)) return null; + let fd; try { - return detectNativeBinaryTarget(readFileSync(binaryPath)); - } catch { + fd = openSync(binaryPath, "r"); + const buffer = Buffer.alloc(HEADER_SIZE); + const bytesRead = readSync(fd, buffer, 0, HEADER_SIZE, 0); + return detectNativeBinaryTarget(buffer.subarray(0, bytesRead)); + } catch (err) { + console.warn(` ⚠️ Could not read native binary at ${binaryPath}: ${err.message}`); return null; + } finally { + if (fd !== undefined) closeSync(fd); } } @@ -145,14 +150,14 @@ export function isNativeBinaryCompatible( return false; } } else if (runtimePlatform !== PUBLISHED_BUILD_PLATFORM || runtimeArch !== PUBLISHED_BUILD_ARCH) { - // Unknown binary layout on a non-build platform is too risky to treat as compatible. return false; } try { dlopen({ exports: {} }, binaryPath); return true; - } catch { + } catch (err) { + console.warn(` ⚠️ Native binary dlopen failed: ${err.message}`); return false; } } diff --git a/scripts/postinstall.mjs b/scripts/postinstall.mjs index d8379f6b..12fd4436 100644 --- a/scripts/postinstall.mjs +++ b/scripts/postinstall.mjs @@ -20,6 +20,8 @@ import { existsSync, copyFileSync, mkdirSync } from "node:fs"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; +import { PUBLISHED_BUILD_PLATFORM, PUBLISHED_BUILD_ARCH } from "./native-binary-compat.mjs"; + const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); const ROOT = join(__dirname, ".."); @@ -43,23 +45,18 @@ const rootBinary = join( ); if (!existsSync(join(ROOT, "app", "node_modules", "better-sqlite3"))) { - // No standalone app directory — nothing to do (dev install, not npm global) process.exit(0); } -// The published binary is compiled for linux-x64. On any other platform/arch, -// always replace it — dlopen alone is unreliable because macOS can load an -// incompatible binary without throwing (the exact bug fixed in #312). -const BUILD_PLATFORM = "linux"; -const BUILD_ARCH = "x64"; -const platformMatch = process.platform === BUILD_PLATFORM && process.arch === BUILD_ARCH; +const platformMatch = + process.platform === PUBLISHED_BUILD_PLATFORM && process.arch === PUBLISHED_BUILD_ARCH; if (platformMatch) { try { process.dlopen({ exports: {} }, appBinary); process.exit(0); - } catch { - // Same platform but binary still incompatible (e.g. Node.js ABI mismatch) + } catch (err) { + console.warn(` ⚠️ Bundled binary incompatible despite platform match: ${err.message}`); } } @@ -70,18 +67,21 @@ if (existsSync(rootBinary)) { try { mkdirSync(dirname(appBinary), { recursive: true }); copyFileSync(rootBinary, appBinary); + } catch (err) { + console.warn(` ⚠️ Failed to copy binary: ${err.message}`); + } - // Verify the copied binary loads + try { process.dlopen({ exports: {} }, appBinary); console.log(" ✅ Native module fixed successfully!\n"); process.exit(0); - } catch { - // Copy succeeded but binary still doesn't load — fall through + } catch (err) { + console.warn(` ⚠️ Copied binary failed to load: ${err.message}`); } } // Strategy 2: Fall back to npm rebuild (may work if build tools are available) -console.log(" ⚠️ Root binary not available, attempting npm rebuild..."); +console.log(" ⚠️ Root binary not available or incompatible, attempting npm rebuild..."); try { const { execSync } = await import("node:child_process"); @@ -91,12 +91,16 @@ try { timeout: 120_000, }); - // Verify rebuild worked process.dlopen({ exports: {} }, appBinary); console.log(" ✅ Native module rebuilt successfully!\n"); process.exit(0); -} catch { - // Rebuild failed or binary still incompatible +} catch (err) { + const isTimeout = err.killed || err.signal === "SIGTERM"; + if (isTimeout) { + console.warn(" ⚠️ npm rebuild timed out after 120s."); + } else { + console.warn(` ⚠️ npm rebuild failed: ${err.message}`); + } } // If nothing worked, warn but don't fail the install — let the package stay