From da805810d474299a9cba9dc3947c181ce03b1659 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 18 Jul 2026 01:00:48 -0400 Subject: [PATCH] fix(desktop): restore exec bit on node-pty spawn-helper for dev terminals node-pty's published npm tarball ships the POSIX `spawn-helper` with mode 0644 (no exec bit). node-pty `posix_spawnp`s that helper on macOS/Linux, so a non-executable copy fails every embedded-terminal spawn with `Error: posix_spawnp failed.`. Packaged builds are unaffected because stage-native-deps.mjs chmods the staged copy, but the dev flow (`npm run dev` -> `electron .`) resolves node-pty straight from node_modules, which nothing chmods -- so the first terminal in dev always dies. Restore the exec bit once, lazily, right before the first spawn, via a small DI-testable helper. Idempotent: already-executable copies (packaged builds) are left untouched, and stat/chmod failures are collected and logged rather than thrown so terminal startup never breaks. --- apps/desktop/electron/main.ts | 35 +++++ .../electron/spawn-helper-perms.test.ts | 140 ++++++++++++++++++ apps/desktop/electron/spawn-helper-perms.ts | 113 ++++++++++++++ 3 files changed, 288 insertions(+) create mode 100644 apps/desktop/electron/spawn-helper-perms.test.ts create mode 100644 apps/desktop/electron/spawn-helper-perms.ts diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 692862356b14..b5efc018c2d1 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -113,6 +113,7 @@ import { SESSION_WINDOW_MIN_HEIGHT, SESSION_WINDOW_MIN_WIDTH } from './session-windows' +import { ensureSpawnHelperExecutable } from './spawn-helper-perms' import { nativeOverlayWidth as computeNativeOverlayWidth, macTitleBarOverlayHeight } from './titlebar-overlay-width' import { resolveBehindCount, shouldCountCommits } from './update-count' import { readLiveUpdateMarker, writeUpdateMarker } from './update-marker' @@ -8663,7 +8664,41 @@ ipcMain.handle('hermes:git:scanRepos', async (_event, roots, options) => { } }) +// node-pty's published tarball ships the POSIX `spawn-helper` without an exec +// bit; the dev flow resolves node-pty straight from node_modules (nothing +// chmods it there), so the first terminal spawn dies with `posix_spawnp +// failed`. Restore the bit once, lazily, right before the first spawn. Packaged +// builds already stage an executable copy, so this is a no-op there. +let _spawnHelperEnsured = false + +function ensureNodePtySpawnHelper() { + if (_spawnHelperEnsured || IS_WINDOWS) { + return + } + + _spawnHelperEnsured = true + + try { + const nodePtyRoot = path.dirname(require.resolve('node-pty/package.json')) + const { fixed, errors } = ensureSpawnHelperExecutable(nodePtyRoot) + + for (const helperPath of fixed) { + rememberLog(`[terminal] restored +x on node-pty spawn-helper: ${helperPath}`) + } + + for (const failure of errors) { + rememberLog(`[terminal] could not chmod spawn-helper ${failure.path}: ${failure.error}`) + } + } catch (error) { + rememberLog( + `[terminal] spawn-helper exec check skipped: ${error instanceof Error ? error.message : String(error)}` + ) + } +} + ipcMain.handle('hermes:terminal:start', async (event, payload = {}) => { + ensureNodePtySpawnHelper() + const id = crypto.randomUUID() const { args, command, name } = terminalShellCommand() const cwd = safeTerminalCwd(payload?.cwd) diff --git a/apps/desktop/electron/spawn-helper-perms.test.ts b/apps/desktop/electron/spawn-helper-perms.test.ts new file mode 100644 index 000000000000..8bb11cd1acbf --- /dev/null +++ b/apps/desktop/electron/spawn-helper-perms.test.ts @@ -0,0 +1,140 @@ +import assert from 'node:assert/strict' +import { join } from 'node:path' + +import { test } from 'vitest' + +import { + ensureSpawnHelperExecutable, + needsExecBit, + spawnHelperCandidates, + type SpawnHelperFs, + withExecBits +} from './spawn-helper-perms' + +interface FakeFile { + mode: number + statThrows?: boolean + chmodThrows?: boolean +} + +function fakeFs( + files: Record, + dirs: Record = {} +): SpawnHelperFs & { chmods: { path: string; mode: number }[] } { + const chmods: { path: string; mode: number }[] = [] + + return { + chmods, + existsSync(path) { + return path in files || path in dirs + }, + readdirSync(path) { + return dirs[path] ?? [] + }, + statSync(path) { + const file = files[path] + + if (!file || file.statThrows) { + throw new Error(`stat failed: ${path}`) + } + + return { mode: file.mode } + }, + chmodSync(path, mode) { + const file = files[path] + + if (file?.chmodThrows) { + throw new Error(`chmod failed: ${path}`) + } + + chmods.push({ path, mode }) + + if (file) { + file.mode = mode + } + } + } +} + +test('needsExecBit / withExecBits treat any missing exec bit as non-executable', () => { + assert.equal(needsExecBit(0o644), true) + assert.equal(needsExecBit(0o755), false) + // Partial exec bits (owner only) still count as needing repair. + assert.equal(needsExecBit(0o744), true) + // Preserves read/write bits while adding exec for all three classes. + assert.equal(withExecBits(0o644), 0o755) + assert.equal(withExecBits(0o600), 0o711) +}) + +test('candidates cover every prebuild dir plus build/Release', () => { + const root = '/pkg/node-pty' + + const fs = fakeFs( + {}, + { [join(root, 'prebuilds')]: ['darwin-arm64', 'darwin-x64', 'linux-x64'] } + ) + + assert.deepEqual(spawnHelperCandidates(root, fs), [ + join(root, 'prebuilds', 'darwin-arm64', 'spawn-helper'), + join(root, 'prebuilds', 'darwin-x64', 'spawn-helper'), + join(root, 'prebuilds', 'linux-x64', 'spawn-helper'), + join(root, 'build', 'Release', 'spawn-helper') + ]) +}) + +test('chmods only the non-executable spawn-helpers, leaving 0755 copies alone', () => { + const root = '/pkg/node-pty' + const arm = join(root, 'prebuilds', 'darwin-arm64', 'spawn-helper') + const x64 = join(root, 'prebuilds', 'darwin-x64', 'spawn-helper') + + const fs = fakeFs( + { + [arm]: { mode: 0o644 }, + [x64]: { mode: 0o755 } + }, + { [join(root, 'prebuilds')]: ['darwin-arm64', 'darwin-x64'] } + ) + + const result = ensureSpawnHelperExecutable(root, fs) + + assert.deepEqual(result.fixed, [arm]) + assert.deepEqual(result.errors, []) + assert.deepEqual(fs.chmods, [{ path: arm, mode: 0o755 }]) +}) + +test('missing spawn-helpers are skipped without error', () => { + const root = '/pkg/node-pty' + const fs = fakeFs({}, { [join(root, 'prebuilds')]: ['darwin-arm64'] }) + + const result = ensureSpawnHelperExecutable(root, fs) + + assert.deepEqual(result.fixed, []) + assert.deepEqual(result.errors, []) + assert.deepEqual(fs.chmods, []) +}) + +test('chmod failures are collected, not thrown', () => { + const root = '/pkg/node-pty' + const arm = join(root, 'prebuilds', 'darwin-arm64', 'spawn-helper') + + const fs = fakeFs( + { [arm]: { mode: 0o644, chmodThrows: true } }, + { [join(root, 'prebuilds')]: ['darwin-arm64'] } + ) + + const result = ensureSpawnHelperExecutable(root, fs) + + assert.deepEqual(result.fixed, []) + assert.equal(result.errors.length, 1) + assert.equal(result.errors[0].path, arm) +}) + +test('no prebuilds dir (Windows layout) is a clean no-op', () => { + const root = '/pkg/node-pty' + const fs = fakeFs({}, {}) + + const result = ensureSpawnHelperExecutable(root, fs) + + assert.deepEqual(result.fixed, []) + assert.deepEqual(result.errors, []) +}) diff --git a/apps/desktop/electron/spawn-helper-perms.ts b/apps/desktop/electron/spawn-helper-perms.ts new file mode 100644 index 000000000000..6dfd484dc808 --- /dev/null +++ b/apps/desktop/electron/spawn-helper-perms.ts @@ -0,0 +1,113 @@ +// node-pty ships its POSIX `spawn-helper` inside the published npm tarball with +// mode 0644 (no exec bit). node-pty `posix_spawnp`s that helper on macOS/Linux, +// so a non-executable helper fails every terminal spawn with +// `Error: posix_spawnp failed.`. Packaged builds are covered because +// stage-native-deps.mjs chmods the staged copy, but the dev flow +// (`npm run dev` → `electron .`) resolves node-pty straight from +// `node_modules/`, which nobody chmods. This restores the exec bits at runtime, +// best-effort, so both dev and any environment that stripped the bit keep +// working. Idempotent: files that are already executable are left untouched. + +import { + chmodSync as realChmodSync, + existsSync as realExistsSync, + readdirSync as realReaddirSync, + statSync as realStatSync +} from 'node:fs' +import { join } from 'node:path' + +const EXEC_BITS = 0o111 + +export interface SpawnHelperFs { + existsSync(path: string): boolean + readdirSync(path: string): string[] + statSync(path: string): { mode: number } + chmodSync(path: string, mode: number): void +} + +export interface EnsureSpawnHelperResult { + fixed: string[] + errors: { path: string; error: string }[] +} + +const defaultFs: SpawnHelperFs = { + existsSync: realExistsSync, + readdirSync: (path: string) => realReaddirSync(path), + statSync: (path: string) => realStatSync(path), + chmodSync: realChmodSync +} + +// True when any of the owner/group/other execute bits are missing. +export function needsExecBit(mode: number): boolean { + return (mode & EXEC_BITS) !== EXEC_BITS +} + +// Preserve existing permission bits, adding execute for owner/group/other. +export function withExecBits(mode: number): number { + return mode | EXEC_BITS +} + +// Every place a `spawn-helper` can live under a node-pty package root: one per +// bundled prebuild (`prebuilds/-/`) plus a locally compiled +// `build/Release/` copy. Windows layouts have no spawn-helper, so the list is +// naturally empty there. +export function spawnHelperCandidates( + nodePtyRoot: string, + fs: Pick = defaultFs +): string[] { + const candidates: string[] = [] + const prebuilds = join(nodePtyRoot, 'prebuilds') + + if (fs.existsSync(prebuilds)) { + for (const entry of fs.readdirSync(prebuilds)) { + candidates.push(join(prebuilds, entry, 'spawn-helper')) + } + } + + candidates.push(join(nodePtyRoot, 'build', 'Release', 'spawn-helper')) + + return candidates +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} + +// Best-effort: ensure every existing spawn-helper under `nodePtyRoot` is +// executable. Never throws — missing files are skipped, and chmod/stat failures +// are collected so the caller can log them without breaking terminal startup. +export function ensureSpawnHelperExecutable( + nodePtyRoot: string, + fs: SpawnHelperFs = defaultFs +): EnsureSpawnHelperResult { + const result: EnsureSpawnHelperResult = { fixed: [], errors: [] } + + for (const path of spawnHelperCandidates(nodePtyRoot, fs)) { + if (!fs.existsSync(path)) { + continue + } + + let mode: number + + try { + mode = fs.statSync(path).mode + } catch (error) { + result.errors.push({ path, error: errorMessage(error) }) + + continue + } + + if (!needsExecBit(mode)) { + continue + } + + try { + fs.chmodSync(path, withExecBits(mode)) + result.fixed.push(path) + } catch (error) { + result.errors.push({ path, error: errorMessage(error) }) + } + } + + return result +}