fix(terminal): unset NODE_CHANNEL_FD for PTY shells on every POSIX host
The PTY runtime exported an empty NODE_CHANNEL_FD to override the daemon's IPC descriptor, because bun-pty merges the native environ back into the child and a JS-only delete does not stick. Node CLIs launched from the shell (opencode, claude) then printed "warn: Failed to parse IPC channel number ''" on exit. The Linux-only env -u ARGV0 wrapper now applies on macOS and Linux and also unsets NODE_CHANNEL_FD, so the variable is gone instead of empty. Testing: runtime and inherited-env tests updated for the POSIX wrapper; verified in the running app that exiting opencode no longer prints the warning.
This commit is contained in:
@@ -12,7 +12,9 @@
|
||||
import { createRequire } from 'node:module';
|
||||
import { existsSync } from 'node:fs';
|
||||
|
||||
const LINUX_ENV_BINARIES = ['/usr/bin/env', '/bin/env'];
|
||||
const POSIX_ENV_BINARIES = ['/usr/bin/env', '/bin/env'];
|
||||
/** Variables a PTY shell must never inherit from the OpenChamber host process. */
|
||||
const PTY_HOST_PRIVATE_VARIABLES = Object.freeze(['ARGV0', 'NODE_CHANNEL_FD']);
|
||||
|
||||
/**
|
||||
* Remove AppImage `ARGV0` from a mutable env object (or `process.env`).
|
||||
@@ -49,26 +51,31 @@ export function clearAppImageArgv0FromProcessEnv() {
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a Linux PTY launch that drops native `ARGV0` before the shell starts.
|
||||
* Resolve a POSIX PTY launch that unsets host-private variables before the
|
||||
* shell starts.
|
||||
*
|
||||
* `bun-pty` merges the OS environ into the child, so deleting `ARGV0` from the
|
||||
* JS env object alone is not enough. Wrapping with `env -u ARGV0` unsets it
|
||||
* before execing the real shell. No-op on non-Linux platforms.
|
||||
* `bun-pty` merges the OS environ into the child, so deleting a variable from
|
||||
* the JS env object alone is not enough: AppImage `ARGV0` came back on Linux,
|
||||
* and the daemon's `NODE_CHANNEL_FD` came back everywhere (Node then warns
|
||||
* "Failed to parse IPC channel number" when a CLI such as opencode exits).
|
||||
* Wrapping with `env -u NAME ...` unsets them before execing the real shell.
|
||||
* No-op on Windows and when no `env` binary is found.
|
||||
*
|
||||
* @param {string} executable
|
||||
* @param {string[]} args
|
||||
* @param {readonly string[]} variables
|
||||
* @returns {{ executable: string, args: string[] }}
|
||||
*/
|
||||
export function resolveLinuxPtyLaunch(executable, args = []) {
|
||||
if (process.platform !== 'linux') {
|
||||
export function resolvePosixPtyLaunch(executable, args = [], variables = PTY_HOST_PRIVATE_VARIABLES) {
|
||||
if (process.platform === 'win32' || variables.length === 0) {
|
||||
return { executable, args };
|
||||
}
|
||||
const envBinary = LINUX_ENV_BINARIES.find((candidate) => existsSync(candidate));
|
||||
const envBinary = POSIX_ENV_BINARIES.find((candidate) => existsSync(candidate));
|
||||
if (!envBinary) {
|
||||
return { executable, args };
|
||||
}
|
||||
return {
|
||||
executable: envBinary,
|
||||
args: ['-u', 'ARGV0', executable, ...args],
|
||||
args: [...variables.flatMap((name) => ['-u', name]), executable, ...args],
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
clearAppImageArgv0FromProcessEnv,
|
||||
resolveLinuxPtyLaunch,
|
||||
resolvePosixPtyLaunch,
|
||||
stripAppImageArgv0Leak,
|
||||
} from './inherited-env.js';
|
||||
|
||||
@@ -46,18 +46,17 @@ describe('clearAppImageArgv0FromProcessEnv', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveLinuxPtyLaunch', () => {
|
||||
it('wraps the shell with env -u ARGV0 on Linux', () => {
|
||||
if (process.platform !== 'linux') return;
|
||||
expect(resolveLinuxPtyLaunch('/bin/zsh', ['-l'])).toEqual({
|
||||
describe('resolvePosixPtyLaunch', () => {
|
||||
it('wraps the shell with env -u for every host-private variable on POSIX', () => {
|
||||
if (process.platform === 'win32') return;
|
||||
expect(resolvePosixPtyLaunch('/bin/zsh', ['-l'])).toEqual({
|
||||
executable: expect.stringMatching(/\/env$/),
|
||||
args: ['-u', 'ARGV0', '/bin/zsh', '-l'],
|
||||
args: ['-u', 'ARGV0', '-u', 'NODE_CHANNEL_FD', '/bin/zsh', '-l'],
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves non-Linux launches unchanged', () => {
|
||||
if (process.platform === 'linux') return;
|
||||
expect(resolveLinuxPtyLaunch('/bin/zsh', ['-l'])).toEqual({
|
||||
it('leaves the launch unchanged with nothing to unset', () => {
|
||||
expect(resolvePosixPtyLaunch('/bin/zsh', ['-l'], [])).toEqual({
|
||||
executable: '/bin/zsh',
|
||||
args: ['-l'],
|
||||
});
|
||||
|
||||
@@ -28,8 +28,8 @@ HTTP remains the authenticated command plane for create, resize, appearance upda
|
||||
- Concurrent creates for one ID are single-flight only when working directory, shell preference, login mode, launch mode, and session purpose match. Command-mode creates must also match the command text, unless the purpose is a project action that is already running for the same resolved `(cwd, actionId)` pair. In that case the runtime returns the existing session and its existing execution identity, even when another client requested a different session ID. Existing IDs cannot be reused for another working directory or another purpose.
|
||||
- Dimensions are bounded to 1-1000 columns and 1-500 rows; input is capped at 64 KiB.
|
||||
- A client may create before its renderer has mounted. It derives an initial size from the container and font metrics (falling back to 80x24 when unavailable), then sends a resize once Ghostty reports its final dimensions. This allows shell startup and renderer initialization to overlap.
|
||||
- PTY children explicitly clear `NODE_CHANNEL_FD`; daemon IPC descriptors are host-private and invalid after PTY descriptor cleanup.
|
||||
- PTY children also strip AppImage `ARGV0` (and other host-private shell vars such as `ELECTRON_RUN_AS_NODE`, `BASH_ENV`, `ENV`, `BASH_XTRACEFD`). An exported `ARGV0` makes zsh rewrite argv[0] for every external command, which breaks Python venv detection and other argv[0]/$0 consumers while leaving `/proc/self/exe` correct. On Linux, PTY spawn is wrapped with `env -u ARGV0` because `bun-pty` merges the native OS environ and would otherwise reintroduce `ARGV0` after a JS-only delete.
|
||||
- PTY children never inherit `NODE_CHANNEL_FD`; daemon IPC descriptors are host-private and invalid after PTY descriptor cleanup, and an inherited value (even empty) makes Node CLIs such as opencode print `Failed to parse IPC channel number` on exit.
|
||||
- PTY children also strip AppImage `ARGV0` (and other host-private shell vars such as `ELECTRON_RUN_AS_NODE`, `BASH_ENV`, `ENV`, `BASH_XTRACEFD`). An exported `ARGV0` makes zsh rewrite argv[0] for every external command, which breaks Python venv detection and other argv[0]/$0 consumers while leaving `/proc/self/exe` correct. On POSIX, PTY spawn is wrapped with `env -u ARGV0 -u NODE_CHANNEL_FD` (`resolvePosixPtyLaunch`) because `bun-pty` merges the native OS environ and would otherwise reintroduce both after a JS-only delete.
|
||||
- `GET /api/terminal/shells` reports shell IDs available on the active server using the same augmented PATH provided to spawned PTYs, plus whether each executable has a supported login-mode argument. `auto` preserves environment/platform fallback order; an explicit unavailable shell fails creation instead of silently running a different shell. Login mode is opt-in and uses only built-in arguments for known shells. Interactive shells still launch as before. Command-mode launches reuse the same environment and login support, but switch argv by shell family: POSIX and Fish use interactive `-c`, Nushell uses `-c`, PowerShell uses `-Command`, and cmd uses `/d /s /c`. Preference changes affect new sessions and explicit restarts, not running PTYs.
|
||||
- PTY data and exit callbacks enter one FIFO queue. The runtime wires those listeners in the same synchronous turn that receives the PTY object. `node-pty` and `bun-pty` both expose the PTY before dispatching registered callbacks. If a backend emitted exit before listener registration, this layer could not recover it, so the wiring stays adjacent to PTY creation.
|
||||
- Scrollback is retained on the server and capped at 512 KiB with UTF-8-safe trimming. Device-status, device-attribute, cursor-position reply, and color-query exchanges are removed from replay history with incomplete control sequences carried across PTY chunks; live output remains byte-for-byte unchanged.
|
||||
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
import { sanitizeTerminalHistoryChunk } from './history.js';
|
||||
import { consumeTerminalThemeQueries, terminalThemeModeReport } from './theme-response.js';
|
||||
import { buildTerminalShellLaunch, createTerminalShellResolver, normalizeTerminalShell } from './shells.js';
|
||||
import { stripAppImageArgv0Leak, resolveLinuxPtyLaunch } from '../inherited-env.js';
|
||||
import { stripAppImageArgv0Leak, resolvePosixPtyLaunch } from '../inherited-env.js';
|
||||
|
||||
const MAX_SESSIONS = 20;
|
||||
const MAX_HISTORY_BYTES = 512 * 1024;
|
||||
@@ -123,15 +123,16 @@ export function createTerminalRuntime({
|
||||
for (const executable of resolvedShell.executables) {
|
||||
try {
|
||||
const env = { ...process.env, PATH: buildAugmentedPath(), TERM: 'xterm-256color', COLORTERM: 'truecolor', COLORFGBG: themeMode === 'light' ? '0;15' : '15;0' };
|
||||
// The daemon's IPC fd is closed inside the PTY. An explicit override is
|
||||
// required because bun-pty also inherits Bun's native process environment.
|
||||
env.NODE_CHANNEL_FD = '';
|
||||
// The daemon's IPC fd is closed inside the PTY; an inherited NODE_CHANNEL_FD
|
||||
// (even an empty one) makes Node CLIs warn about an unparsable IPC channel.
|
||||
delete env.NODE_CHANNEL_FD;
|
||||
delete env.BASH_XTRACEFD; delete env.BASH_ENV; delete env.ENV; delete env.ELECTRON_RUN_AS_NODE;
|
||||
// AppImage exports ARGV0; zsh would otherwise rewrite argv[0] for every command (#2588).
|
||||
// bun-pty also merges the native OS environ, so wrap with `env -u ARGV0` on Linux.
|
||||
stripAppImageArgv0Leak(env);
|
||||
const shellLaunch = buildTerminalShellLaunch(executable, { mode, command, loginShell });
|
||||
const launch = resolveLinuxPtyLaunch(shellLaunch.executable, shellLaunch.args);
|
||||
// bun-pty merges the native OS environ back in, so the POSIX launch is
|
||||
// wrapped with `env -u` for the variables deleted above.
|
||||
const launch = resolvePosixPtyLaunch(shellLaunch.executable, shellLaunch.args);
|
||||
const options = { name: 'xterm-256color', cwd, cols, rows, env };
|
||||
if (process.platform === 'win32') options.useConpty = true;
|
||||
return { process: await provider.spawn(launch.executable, launch.args, options), backend: provider.backend, shell: resolvedShell.id, loginShell };
|
||||
|
||||
@@ -319,12 +319,12 @@ describe('terminal runtime', () => {
|
||||
expect(response.body).toEqual({ sessionId: 'term-1', cols: 120, rows: 40, status: 'running', mode: 'interactive', purpose: { type: 'terminal' } });
|
||||
expect(harness.processes[0].options.cwd).toBe('/repo');
|
||||
expect(harness.processes[0].options.env.COLORFGBG).toBe('0;15');
|
||||
expect(harness.processes[0].options.env.NODE_CHANNEL_FD).toBe('');
|
||||
expect(harness.processes[0].options.env).not.toHaveProperty('NODE_CHANNEL_FD');
|
||||
expect(harness.processes[0].options.env).not.toHaveProperty('ARGV0');
|
||||
expect(harness.processes[0].options.env).not.toHaveProperty('ELECTRON_RUN_AS_NODE');
|
||||
if (process.platform === 'linux') {
|
||||
if (process.platform !== 'win32') {
|
||||
expect(harness.processes[0].shell).toMatch(/\/env$/);
|
||||
expect(harness.processes[0].args.slice(0, 3)).toEqual(['-u', 'ARGV0', expect.any(String)]);
|
||||
expect(harness.processes[0].args.slice(0, 5)).toEqual(['-u', 'ARGV0', '-u', 'NODE_CHANNEL_FD', expect.any(String)]);
|
||||
}
|
||||
harness.processes[0].emitData('\u001b[?2031h\u001b]10;?\u0007\u001b]11;?\u0007\u001b[0c');
|
||||
expect(harness.processes[0].writes).toEqual(['\u001b]10;rgb:1b1b/1b1b/1b1b\u001b\\', '\u001b]11;rgb:fafa/f8f8/f0f0\u001b\\', '\u001b[?1;2c']);
|
||||
@@ -380,7 +380,7 @@ describe('terminal runtime', () => {
|
||||
await harness.routes.post.get('/api/terminal/create')({ body: { sessionId: 'term-argv0', cwd: '/repo', cols: 80, rows: 24 } }, response);
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(harness.processes[0].options.env).not.toHaveProperty('ARGV0');
|
||||
if (process.platform === 'linux') {
|
||||
if (process.platform !== 'win32') {
|
||||
expect(harness.processes[0].shell).toMatch(/\/env$/);
|
||||
expect(harness.processes[0].args[0]).toBe('-u');
|
||||
expect(harness.processes[0].args[1]).toBe('ARGV0');
|
||||
@@ -417,9 +417,9 @@ describe('terminal runtime', () => {
|
||||
const created = createResponse();
|
||||
await harness.routes.post.get('/api/terminal/create')({ body: { sessionId: 'term-shell', cwd: '/repo', shell: 'zsh', loginShell: true } }, created);
|
||||
expect(created.statusCode).toBe(200);
|
||||
if (process.platform === 'linux') {
|
||||
if (process.platform !== 'win32') {
|
||||
expect(harness.processes[0].shell).toMatch(/\/env$/);
|
||||
expect(harness.processes[0].args).toEqual(['-u', 'ARGV0', '/bin/zsh', '-l']);
|
||||
expect(harness.processes[0].args).toEqual(['-u', 'ARGV0', '-u', 'NODE_CHANNEL_FD', '/bin/zsh', '-l']);
|
||||
} else {
|
||||
expect(harness.processes[0].shell).toBe('/bin/zsh');
|
||||
expect(harness.processes[0].args).toEqual(['-l']);
|
||||
@@ -428,9 +428,9 @@ describe('terminal runtime', () => {
|
||||
const restarted = createResponse();
|
||||
await harness.routes.post.get('/api/terminal/:sessionId/restart')({ params: { sessionId: 'term-shell' }, body: { shell: 'bash', loginShell: true } }, restarted);
|
||||
expect(restarted.statusCode).toBe(200);
|
||||
if (process.platform === 'linux') {
|
||||
if (process.platform !== 'win32') {
|
||||
expect(harness.processes[1].shell).toMatch(/\/env$/);
|
||||
expect(harness.processes[1].args).toEqual(['-u', 'ARGV0', '/bin/bash', '-l']);
|
||||
expect(harness.processes[1].args).toEqual(['-u', 'ARGV0', '-u', 'NODE_CHANNEL_FD', '/bin/bash', '-l']);
|
||||
} else {
|
||||
expect(harness.processes[1].shell).toBe('/bin/bash');
|
||||
expect(harness.processes[1].args).toEqual(['-l']);
|
||||
@@ -757,9 +757,9 @@ describe('terminal runtime', () => {
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(response.body).toEqual({ sessionId: 'term-command', cols: 80, rows: 24, status: 'running', mode: 'command', purpose: { type: 'terminal' } });
|
||||
if (process.platform === 'linux') {
|
||||
if (process.platform !== 'win32') {
|
||||
expect(harness.processes[0].shell).toMatch(/\/env$/);
|
||||
expect(harness.processes[0].args).toEqual(['-u', 'ARGV0', '/bin/bash', '-l', '-i', '-c', 'printf ready']);
|
||||
expect(harness.processes[0].args).toEqual(['-u', 'ARGV0', '-u', 'NODE_CHANNEL_FD', '/bin/bash', '-l', '-i', '-c', 'printf ready']);
|
||||
} else {
|
||||
expect(harness.processes[0].args).toEqual(['-l', '-i', '-c', 'printf ready']);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user