From daa81e8d099f4cc6b2c8b39e73e44072347940c2 Mon Sep 17 00:00:00 2001 From: raul Date: Mon, 6 Apr 2026 22:00:53 +0200 Subject: [PATCH 1/2] fix(shellEnv): scrub AppImage env from shell probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route the env passed to `$SHELL -ilc` through `buildExternalToolEnv()` so AppImage packaging vars (APPIMAGE, APPDIR, ARGV0, OWD, etc.) and `/tmp/.mount_*` entries in PATH / LD_LIBRARY_PATH / XDG_DATA_DIRS are stripped before the probed login shell inherits them. Without this, a Linux AppImage install leaks those vars into the login shell startup. If the user's `.zshrc` / `.bash_profile` exec's a binary by name through PATH (starship version modules, mise activation hooks, oh-my-zsh completion plugins all do some flavor of this), the exec can resolve back into the AppImage mount and re-enter Emdash's main process, which runs this probe again and spawns another shell — a fork bomb. The same remediation helper is already used by ptyManager.ts (rationale comment there introduced by #485) and appIpc.ts (#872). shellEnv.ts was the remaining call site where process.env was passed wholesale to a child shell. Adds a regression test that mocks execSync and asserts the spawned env has AppImage keys and `/tmp/.mount_*` PATH entries scrubbed. Verified by stash-and-rerun: the test fails without the fix and passes with it. Fixes #1679 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/main/utils/__tests__/shellEnv.test.ts | 54 +++++++++++++++++++++++ src/main/utils/shellEnv.ts | 21 ++++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/main/utils/__tests__/shellEnv.test.ts b/src/main/utils/__tests__/shellEnv.test.ts index 50ff2b3e9..187baf5ee 100644 --- a/src/main/utils/__tests__/shellEnv.test.ts +++ b/src/main/utils/__tests__/shellEnv.test.ts @@ -330,6 +330,60 @@ describe('shellEnv', () => { expect(process.env.LC_ALL).toBeUndefined(); }); + it('should strip AppImage env leakage from the probed shell', () => { + // Regression test: shellEnv.ts used to hand `...process.env` wholesale + // to the login shell it spawns for env probing. On Linux AppImage + // installs this meant the child shell saw APPIMAGE, APPDIR, and the + // `/tmp/.mount_*` entries on PATH. If any shell-init plugin (starship, + // mise, oh-my-zsh plugins) exec'd a binary by name through PATH, the + // exec could resolve back into the AppImage mount and re-enter Emdash's + // main process, which would run this probe again — a fork bomb. + // + // The fix routes the probe's env through buildExternalToolEnv() from + // ./childProcessEnv. These assertions lock the behavior in. + const appDir = '/tmp/.mount_emdashAbCd'; + process.env.APPIMAGE = '/home/user/emdash.AppImage'; + process.env.APPDIR = appDir; + process.env.ARGV0 = 'AppRun'; + process.env.OWD = '/tmp'; + process.env.PATH = `/usr/local/bin:${appDir}/usr/bin:/usr/bin`; + process.env.LD_LIBRARY_PATH = `${appDir}/usr/lib:/usr/local/cuda/lib64`; + delete process.env.LANG; + delete process.env.LC_CTYPE; + delete process.env.LC_ALL; + + mockedExecSync.mockImplementation( + shellLookup({ + SSH_AUTH_SOCK: '/detected/socket', + LANG: 'C.UTF-8', + LC_CTYPE: 'C.UTF-8', + LC_ALL: 'C.UTF-8', + }) + ); + + initializeShellEnvironment(); + + // Every execSync invocation in this module should have been handed a + // scrubbed env — no AppImage keys, no mount paths in PATH-like vars. + expect(mockedExecSync).toHaveBeenCalled(); + for (const call of mockedExecSync.mock.calls) { + const options = call[1] as { env?: NodeJS.ProcessEnv } | undefined; + const env = options?.env; + expect(env).toBeDefined(); + if (!env) continue; + expect(env.APPIMAGE).toBeUndefined(); + expect(env.APPDIR).toBeUndefined(); + expect(env.ARGV0).toBeUndefined(); + expect(env.OWD).toBeUndefined(); + expect(env.PATH).toBeDefined(); + expect(env.PATH).not.toContain(appDir); + expect(env.PATH).not.toContain('/tmp/.mount_'); + expect(env.LD_LIBRARY_PATH).toBeDefined(); + expect(env.LD_LIBRARY_PATH).not.toContain(appDir); + expect(env.LD_LIBRARY_PATH).not.toContain('/tmp/.mount_'); + } + }); + it('should drop non-UTF-8 overrides when LANG is already UTF-8', () => { process.env.LANG = 'en_US.UTF-8'; process.env.LC_CTYPE = 'C'; diff --git a/src/main/utils/shellEnv.ts b/src/main/utils/shellEnv.ts index 1df07e0e5..a510a7d62 100644 --- a/src/main/utils/shellEnv.ts +++ b/src/main/utils/shellEnv.ts @@ -8,6 +8,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { stripAnsi } from '@shared/text/stripAnsi'; +import { buildExternalToolEnv } from './childProcessEnv'; import { LOCALE_ENV_VARS, DEFAULT_UTF8_LOCALE, isUtf8Locale } from './locale'; const SHELL_VALUE_START = '__EMDASH_SHELL_VALUE_START__'; @@ -40,13 +41,28 @@ export function getShellEnvVar(varName: string): string | undefined { const shell = process.env.SHELL || (process.platform === 'darwin' ? '/bin/zsh' : '/bin/bash'); // -i = interactive, -l = login shell (sources .zshrc/.bash_profile) + // + // Env: route through buildExternalToolEnv() to strip AppImage-only keys + // (APPIMAGE, APPDIR, ARGV0, ...) and AppImage mount entries from PATH / + // LD_LIBRARY_PATH / XDG_DATA_DIRS before handing them to the child shell. + // + // WHY: Without this, a Linux AppImage install leaks APPIMAGE and the + // `/tmp/.mount_*` PATH entries into the probed login shell. If the user's + // .zshrc / .bash_profile exec's any binary by name through PATH (starship + // version modules, mise activation hooks, oh-my-zsh completion plugins all + // do some flavor of this), the exec can resolve back into the AppImage + // mount, which re-enters Emdash's main process, which runs this probe + // again, which spawns another shell — a fork bomb that OOMs the machine. + // See the matching rationale and helper at + // src/main/utils/childProcessEnv.ts and the ptyManager.ts:1404 comment + // introduced by issue #485 / PR #872. const result = execSync( `${shell} -ilc 'printf "${SHELL_VALUE_START}\\n"; printenv ${varName}; printf "${SHELL_VALUE_END}\\n"; exit 0'`, { encoding: 'utf8', timeout: 5000, env: { - ...process.env, + ...buildExternalToolEnv(process.env), // Prevent oh-my-zsh plugins from breaking output DISABLE_AUTO_UPDATE: 'true', ZSH_TMUX_AUTOSTART: 'false', @@ -234,11 +250,12 @@ function getShellLocaleVars(): Partial> { const printCommands = LOCALE_ENV_VARS.map((v) => `printenv ${v} || echo`).join( '; echo "---"; ' ); + // See getShellEnvVar for the rationale behind buildExternalToolEnv. const result = execSync(`${shell} -ilc '${printCommands}; exit 0'`, { encoding: 'utf8', timeout: 5000, env: { - ...process.env, + ...buildExternalToolEnv(process.env), DISABLE_AUTO_UPDATE: 'true', ZSH_TMUX_AUTOSTART: 'false', ZSH_TMUX_AUTOSTARTED: 'true', From daa4d2aab08effd58cbb9c8ed805fcd4b4d77cec Mon Sep 17 00:00:00 2001 From: raul Date: Mon, 6 Apr 2026 22:13:42 +0200 Subject: [PATCH 2/2] test(shellEnv): filter probe calls before env assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AppImage-env regression test iterated over every mockedExecSync call and asserted `options.env` was defined. That breaks on macOS, where detectSshAuthSock() calls `execSync('launchctl getenv ...')` with no `env` option — the loop would trip the `expect(env).toBeDefined()` assertion on the launchctl call before it ever reached the shell probe. Filter the call list to entries whose command string contains `-ilc` (the flag both getShellEnvVar and getShellLocaleVars use) so the assertions only run against the actual probe calls. Behavior on Linux is unchanged (launchctl branch is skipped there anyway). Verified by reverting shellEnv.ts to HEAD~1 and re-running: the test still fails on `expected '/home/user/emdash.AppImage' to be undefined`, confirming the filter didn't weaken the regression guard. Addresses CodeRabbit review on #1680. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/main/utils/__tests__/shellEnv.test.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/main/utils/__tests__/shellEnv.test.ts b/src/main/utils/__tests__/shellEnv.test.ts index 187baf5ee..0d4ac684b 100644 --- a/src/main/utils/__tests__/shellEnv.test.ts +++ b/src/main/utils/__tests__/shellEnv.test.ts @@ -363,10 +363,18 @@ describe('shellEnv', () => { initializeShellEnvironment(); - // Every execSync invocation in this module should have been handed a - // scrubbed env — no AppImage keys, no mount paths in PATH-like vars. - expect(mockedExecSync).toHaveBeenCalled(); - for (const call of mockedExecSync.mock.calls) { + // Every probe invocation should have been handed a scrubbed env — no + // AppImage keys, no mount paths in PATH-like vars. Filter to the shell + // probe calls specifically: on macOS, detectSshAuthSock() also calls + // `launchctl getenv SSH_AUTH_SOCK` with no `env` option, and we don't + // want that call to trip the `env` assertion below. The probe call + // sites in getShellEnvVar / getShellLocaleVars both use `-ilc`, which + // is a clean discriminator. + const probeCalls = mockedExecSync.mock.calls.filter( + (call) => typeof call[0] === 'string' && call[0].includes('-ilc') + ); + expect(probeCalls.length).toBeGreaterThan(0); + for (const call of probeCalls) { const options = call[1] as { env?: NodeJS.ProcessEnv } | undefined; const env = options?.env; expect(env).toBeDefined();