diff --git a/apps/code/src/main/bootstrap.ts b/apps/code/src/main/bootstrap.ts index 24f5e98b49..e6ad13e5ac 100644 --- a/apps/code/src/main/bootstrap.ts +++ b/apps/code/src/main/bootstrap.ts @@ -21,6 +21,18 @@ import os from "node:os"; import path from "node:path"; import { app, crashReporter, protocol } from "electron"; import { fixPath } from "./utils/fixPath"; +import { shouldRefuseInternalChildBoot } from "./utils/internal-child-guard"; + +// The internal-child marker means a workspace-server descendant stripped +// ELECTRON_RUN_AS_NODE and ran `node` or process.execPath; booting a full app +// here would race the single-instance lock and open phantom windows. +if (shouldRefuseInternalChildBoot(app.isPackaged, process.env)) { + process.stderr.write( + "[posthog-code] Refusing to start the desktop app from inside its own " + + "child process tree (expected ELECTRON_RUN_AS_NODE=1).\n", + ); + process.exit(1); +} const isDev = !app.isPackaged; diff --git a/apps/code/src/main/services/workspace-server/service.ts b/apps/code/src/main/services/workspace-server/service.ts index c1e0cb4a98..535837819e 100644 --- a/apps/code/src/main/services/workspace-server/service.ts +++ b/apps/code/src/main/services/workspace-server/service.ts @@ -3,6 +3,7 @@ import { randomBytes } from "node:crypto"; import { createServer } from "node:net"; import path from "node:path"; import { TypedEventEmitter } from "@posthog/shared"; +import { POSTHOG_CODE_INTERNAL_CHILD_ENV } from "@posthog/shared/constants"; import type { WorkspaceConnection } from "@posthog/workspace-client/client"; import { injectable } from "inversify"; import { logger } from "../../utils/logger.js"; @@ -191,6 +192,7 @@ export class WorkspaceServerService extends TypedEventEmitter { + it.each([ + ["packaged app inside the internal child tree", true, "1", true], + ["packaged app with no marker", true, undefined, false], + ["dev build inside the internal child tree", false, "1", false], + ["packaged app with an empty marker", true, "", false], + ] as const)("%s", (_name, isPackaged, marker, expected) => { + expect( + shouldRefuseInternalChildBoot(isPackaged, { + POSTHOG_CODE_INTERNAL_CHILD: marker, + }), + ).toBe(expected); + }); +}); diff --git a/apps/code/src/main/utils/internal-child-guard.ts b/apps/code/src/main/utils/internal-child-guard.ts new file mode 100644 index 0000000000..2fdc254988 --- /dev/null +++ b/apps/code/src/main/utils/internal-child-guard.ts @@ -0,0 +1,10 @@ +import { POSTHOG_CODE_INTERNAL_CHILD_ENV } from "@posthog/shared/constants"; + +// Packaged-only: dev builds must stay launchable from dogfooding trees, where +// an app-spawned agent or terminal runs `pnpm dev` with the marker inherited. +export function shouldRefuseInternalChildBoot( + isPackaged: boolean, + env: NodeJS.ProcessEnv, +): boolean { + return isPackaged && Boolean(env[POSTHOG_CODE_INTERNAL_CHILD_ENV]); +} diff --git a/packages/agent/src/adapters/codex-app-server/local-tools-mcp.test.ts b/packages/agent/src/adapters/codex-app-server/local-tools-mcp.test.ts index 1a65e20dbd..890e1b0bfe 100644 --- a/packages/agent/src/adapters/codex-app-server/local-tools-mcp.test.ts +++ b/packages/agent/src/adapters/codex-app-server/local-tools-mcp.test.ts @@ -58,6 +58,13 @@ describe("buildLocalToolsServer", () => { // Token is forwarded to the child so its own git remote ops authenticate. expect(envNames).toContain("GH_TOKEN"); expect(envNames).toContain("GITHUB_TOKEN"); + // Codex strips ELECTRON_RUN_AS_NODE from its own env, and process.execPath + // is the app binary in packaged installs; without this the server boots + // the full desktop app instead of running the script. + expect(server?.env).toContainEqual({ + name: "ELECTRON_RUN_AS_NODE", + value: "1", + }); const ctxEntry = server?.env.find( (e) => e.name === "POSTHOG_LOCAL_TOOLS_CTX", diff --git a/packages/agent/src/adapters/codex-app-server/local-tools-mcp.ts b/packages/agent/src/adapters/codex-app-server/local-tools-mcp.ts index 4461c1de46..50cd108e23 100644 --- a/packages/agent/src/adapters/codex-app-server/local-tools-mcp.ts +++ b/packages/agent/src/adapters/codex-app-server/local-tools-mcp.ts @@ -39,6 +39,10 @@ function toMcpServerStdio( const env = [ { name: "POSTHOG_LOCAL_TOOLS_CTX", value: ctxBase64 }, { name: "POSTHOG_LOCAL_TOOLS_ENABLED", value: enabledNames.join(",") }, + // Codex spawns this command with ELECTRON_RUN_AS_NODE removed from its own + // env (spawn.ts). In packaged desktop installs process.execPath is the app + // binary, which boots the full app without this var. Inert on real node. + { name: "ELECTRON_RUN_AS_NODE", value: "1" }, ]; if (ctx.token) { // Token also on the child env so its own git remote ops authenticate. diff --git a/packages/agent/src/utils/spawn-env.test.ts b/packages/agent/src/utils/spawn-env.test.ts index 9be1cf2554..3a7879b26a 100644 --- a/packages/agent/src/utils/spawn-env.test.ts +++ b/packages/agent/src/utils/spawn-env.test.ts @@ -1,28 +1,46 @@ import { mkdtempSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; +import { buildNodeShimScript } from "@posthog/shared/node-shim"; import { describe, expect, it } from "vitest"; import { stripElectronNodeShimFromPath } from "./spawn-env"; const EXEC_PATH = "/fake/Electron.app/Contents/MacOS/Electron"; -function makeDir(kind: "shim" | "other-symlink" | "real-node" | "empty") { +function makeDir( + kind: + | "symlink-shim" + | "wrapper-shim" + | "foreign-wrapper" + | "other-symlink" + | "real-node" + | "empty", +) { const dir = mkdtempSync(join(tmpdir(), "spawn-env-")); - if (kind === "shim") symlinkSync(EXEC_PATH, join(dir, "node")); - if (kind === "other-symlink") symlinkSync("/usr/bin/true", join(dir, "node")); - if (kind === "real-node") writeFileSync(join(dir, "node"), ""); + const node = join(dir, "node"); + if (kind === "symlink-shim") symlinkSync(EXEC_PATH, node); + if (kind === "wrapper-shim") + writeFileSync(node, buildNodeShimScript(EXEC_PATH)); + if (kind === "foreign-wrapper") + writeFileSync(node, buildNodeShimScript("/some/other/app")); + if (kind === "other-symlink") symlinkSync("/usr/bin/true", node); + if (kind === "real-node") writeFileSync(node, ""); return dir; } describe("stripElectronNodeShimFromPath", () => { - it("removes only dirs whose node symlinks to the executable", () => { - const shim = makeDir("shim"); + it("removes only dirs whose node aliases the executable", () => { + const symlinkShim = makeDir("symlink-shim"); + const wrapperShim = makeDir("wrapper-shim"); + const foreign = makeDir("foreign-wrapper"); const other = makeDir("other-symlink"); const real = makeDir("real-node"); const empty = makeDir("empty"); - const input = [shim, other, real, empty].join(delimiter); + const input = [symlinkShim, wrapperShim, foreign, other, real, empty].join( + delimiter, + ); expect(stripElectronNodeShimFromPath(input, EXEC_PATH)).toBe( - [other, real, empty].join(delimiter), + [foreign, other, real, empty].join(delimiter), ); }); diff --git a/packages/agent/src/utils/spawn-env.ts b/packages/agent/src/utils/spawn-env.ts index 61a0de1e08..6174ef4aa1 100644 --- a/packages/agent/src/utils/spawn-env.ts +++ b/packages/agent/src/utils/spawn-env.ts @@ -1,14 +1,11 @@ -import { readlinkSync } from "node:fs"; +import { lstatSync, readFileSync, readlinkSync } from "node:fs"; import { delimiter, join } from "node:path"; +import { buildNodeShimScript } from "@posthog/shared/node-shim"; -/** - * Removes PATH entries that alias `node` to the current executable. The desktop - * host prepends a shim dir whose `node` symlinks to Electron so claude-code - * children (which inherit ELECTRON_RUN_AS_NODE=1) can resolve a node binary. - * Codex children have ELECTRON_RUN_AS_NODE deleted, so anything they spawn via - * that shim boots a full Electron app that never exits — wedging codex's - * plugin hooks (and with them every turn) until its 10-minute timeout. - */ +// Removes PATH entries that alias `node` to the current executable, whether a +// legacy symlink or the wrapper script written by ensureNodeShim. Codex +// children must not resolve `node` to Electron's bundled runtime: native +// modules they install target the real node ABI. export function stripElectronNodeShimFromPath( pathValue: string | undefined, execPath: string = process.execPath, @@ -16,13 +13,18 @@ export function stripElectronNodeShimFromPath( if (!pathValue) return pathValue; return pathValue .split(delimiter) - .filter((dir) => { - if (!dir) return false; - try { - return readlinkSync(join(dir, "node")) !== execPath; - } catch { - return true; - } - }) + .filter((dir) => dir && !isElectronNodeShimDir(dir, execPath)) .join(delimiter); } + +function isElectronNodeShimDir(dir: string, execPath: string): boolean { + const shimPath = join(dir, "node"); + try { + if (lstatSync(shimPath).isSymbolicLink()) { + return readlinkSync(shimPath) === execPath; + } + return readFileSync(shimPath, "utf-8") === buildNodeShimScript(execPath); + } catch { + return false; + } +} diff --git a/packages/git/src/operation-manager.ts b/packages/git/src/operation-manager.ts index 38cfa0b612..cca95d4139 100644 --- a/packages/git/src/operation-manager.ts +++ b/packages/git/src/operation-manager.ts @@ -7,9 +7,10 @@ import { AsyncReaderWriterLock } from "./rw-lock"; * child processes spawned by git hooks (e.g. biome via lint-staged) don't * crash trying to initialise GPU subsystems. * - * The agent service symlinks `node → Electron binary` and prepends it to - * PATH. If ELECTRON_RUN_AS_NODE is missing, that binary starts as a full - * Chromium browser (GPU init → SIGTRAP crash). We strip most ELECTRON_/ + * The agent service puts a `node` shim for the Electron binary on PATH (a + * wrapper script that sets ELECTRON_RUN_AS_NODE itself on POSIX, a symlink + * on win32). If the var is missing when the raw binary runs, it starts as a + * full Chromium browser (GPU init → SIGTRAP crash). We strip most ELECTRON_/ * CHROME_ vars but explicitly keep ELECTRON_RUN_AS_NODE=1 so any such * shim still behaves as plain Node.js. * diff --git a/packages/shared/package.json b/packages/shared/package.json index 7a1ee01192..2a413fc909 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -40,6 +40,10 @@ "types": "./dist/mcp-sandbox-proxy.d.ts", "import": "./dist/mcp-sandbox-proxy.js" }, + "./node-shim": { + "types": "./dist/node-shim.d.ts", + "import": "./dist/node-shim.js" + }, "./posthog-property-headers": { "types": "./dist/posthog-property-headers.d.ts", "import": "./dist/posthog-property-headers.js" diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index b0a3336e29..5756947d05 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -9,3 +9,4 @@ export { export const SELF_DRIVING_SETUP_TASK_FLAG = "posthog-code-self-driving-setup-task"; export const BRANCH_PREFIX = "posthog-code/"; +export const POSTHOG_CODE_INTERNAL_CHILD_ENV = "POSTHOG_CODE_INTERNAL_CHILD"; diff --git a/packages/shared/src/node-shim.test.ts b/packages/shared/src/node-shim.test.ts new file mode 100644 index 0000000000..2b087c955d --- /dev/null +++ b/packages/shared/src/node-shim.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from "vitest"; +import { buildNodeShimScript } from "./node-shim"; + +describe("buildNodeShimScript", () => { + it("sets ELECTRON_RUN_AS_NODE and execs the binary", () => { + const script = buildNodeShimScript( + "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code", + ); + expect(script).toContain("#!/bin/sh"); + expect(script).toContain("export ELECTRON_RUN_AS_NODE=1"); + expect(script).toContain( + 'exec "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code" "$@"', + ); + }); + + it("escapes shell-special characters in the binary path", () => { + expect(buildNodeShimScript('/odd/pa"th/$app`bin\\x')).toContain( + 'exec "/odd/pa\\"th/\\$app\\`bin\\\\x" "$@"', + ); + }); +}); diff --git a/packages/shared/src/node-shim.ts b/packages/shared/src/node-shim.ts new file mode 100644 index 0000000000..507bed94b4 --- /dev/null +++ b/packages/shared/src/node-shim.ts @@ -0,0 +1,14 @@ +function escapeForDoubleQuotes(path: string): string { + return path.replace(/([$`"\\])/g, "\\$1"); +} + +// Single source of truth for the PATH `node` shim format: workspace-server +// writes it and the codex spawn path detects it by exact content. +export function buildNodeShimScript(execPath: string): string { + return [ + "#!/bin/sh", + "export ELECTRON_RUN_AS_NODE=1", + `exec "${escapeForDoubleQuotes(execPath)}" "$@"`, + "", + ].join("\n"); +} diff --git a/packages/shared/tsup.config.ts b/packages/shared/tsup.config.ts index 4d3231a785..39884b61a0 100644 --- a/packages/shared/tsup.config.ts +++ b/packages/shared/tsup.config.ts @@ -10,6 +10,7 @@ export default defineConfig({ "src/dismissalReasons.ts", "src/domain-types.ts", "src/mcp-sandbox-proxy.ts", + "src/node-shim.ts", "src/posthog-property-headers.ts", "src/types.ts", ], diff --git a/packages/workspace-server/src/services/agent/agent.test.ts b/packages/workspace-server/src/services/agent/agent.test.ts index 88068a71fe..d60ec7a52b 100644 --- a/packages/workspace-server/src/services/agent/agent.test.ts +++ b/packages/workspace-server/src/services/agent/agent.test.ts @@ -98,6 +98,12 @@ vi.mock("node:fs", async (importOriginal) => { }; }); +const mockNodeShim = vi.hoisted(() => ({ + ensureNodeShim: vi.fn(), +})); + +vi.mock("./node-shim", () => mockNodeShim); + // --- Import after mocks --- import type { RegisteredFolder } from "../folders/schemas"; import { AgentService, buildAutoApproveOutcome } from "./agent"; @@ -347,6 +353,30 @@ describe("AgentService", () => { }); }); + describe("node shim setup", () => { + it("writes the shim once per service and retries after a failure", async () => { + mockNodeShim.ensureNodeShim.mockImplementationOnce(() => { + throw new Error("read-only fs"); + }); + + await service.startSession({ ...baseSessionParams, adapter: "codex" }); + await service.startSession({ + ...baseSessionParams, + taskId: "task-2", + taskRunId: "run-2", + adapter: "codex", + }); + await service.startSession({ + ...baseSessionParams, + taskId: "task-3", + taskRunId: "run-3", + adapter: "codex", + }); + + expect(mockNodeShim.ensureNodeShim).toHaveBeenCalledTimes(2); + }); + }); + describe("idle timeout", () => { function injectSession( svc: AgentService, diff --git a/packages/workspace-server/src/services/agent/agent.ts b/packages/workspace-server/src/services/agent/agent.ts index 77ff3dea7a..77a7a96752 100644 --- a/packages/workspace-server/src/services/agent/agent.ts +++ b/packages/workspace-server/src/services/agent/agent.ts @@ -1,4 +1,4 @@ -import fs, { mkdirSync, symlinkSync } from "node:fs"; +import fs from "node:fs"; import { homedir, tmpdir } from "node:os"; import { isAbsolute, join, relative, resolve, sep } from "node:path"; import { @@ -88,6 +88,7 @@ import { AGENT_REPO_FILES, AGENT_SLEEP_COORDINATOR, } from "./identifiers"; +import { ensureNodeShim } from "./node-shim"; import type { AgentLogger, AgentMcpApps, @@ -1593,19 +1594,7 @@ For git operations while detached: const mockNodeDir = getMockNodeDir(); if (!this.mockNodeReady) { try { - mkdirSync(mockNodeDir, { recursive: true }); - const nodeSymlinkPath = join(mockNodeDir, "node"); - try { - symlinkSync(process.execPath, nodeSymlinkPath); - } catch (err) { - if ( - !(err instanceof Error) || - !("code" in err) || - err.code !== "EEXIST" - ) { - throw err; - } - } + ensureNodeShim(mockNodeDir, process.execPath); this.mockNodeReady = true; } catch (err) { this.log.warn("Failed to setup mock node environment", err); diff --git a/packages/workspace-server/src/services/agent/node-shim.test.ts b/packages/workspace-server/src/services/agent/node-shim.test.ts new file mode 100644 index 0000000000..c790555c6a --- /dev/null +++ b/packages/workspace-server/src/services/agent/node-shim.test.ts @@ -0,0 +1,113 @@ +import { + lstatSync, + mkdtempSync, + readFileSync, + readlinkSync, + rmSync, + statSync, + symlinkSync, + utimesSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { buildNodeShimScript } from "@posthog/shared/node-shim"; +import { afterEach, describe, expect, it } from "vitest"; +import { ensureNodeShim } from "./node-shim"; + +const EXEC_PATH = "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code"; + +describe("ensureNodeShim", () => { + const dirs: string[] = []; + + function makeDir(): string { + const dir = mkdtempSync(join(tmpdir(), "node-shim-test-")); + dirs.push(dir); + return join(dir, "shim"); + } + + afterEach(() => { + while (dirs.length > 0) { + const dir = dirs.pop(); + if (dir) rmSync(dir, { recursive: true, force: true }); + } + }); + + it("writes an executable wrapper that sets ELECTRON_RUN_AS_NODE itself", () => { + const dir = makeDir(); + ensureNodeShim(dir, EXEC_PATH, "darwin"); + + const shim = join(dir, "node"); + const content = readFileSync(shim, "utf-8"); + expect(content).toContain("#!/bin/sh"); + expect(content).toContain("export ELECTRON_RUN_AS_NODE=1"); + expect(content).toContain(`exec "${EXEC_PATH}" "$@"`); + expect(statSync(shim).mode & 0o111).not.toBe(0); + }); + + it("replaces a legacy symlink shim with the wrapper script", () => { + const dir = makeDir(); + ensureNodeShim(dir, EXEC_PATH, "darwin"); + const shim = join(dir, "node"); + rmSync(shim); + symlinkSync(EXEC_PATH, shim); + + ensureNodeShim(dir, EXEC_PATH, "darwin"); + + expect(lstatSync(shim).isSymbolicLink()).toBe(false); + expect(readFileSync(shim, "utf-8")).toBe(buildNodeShimScript(EXEC_PATH)); + }); + + it("rewrites the wrapper when the binary path changes", () => { + const dir = makeDir(); + ensureNodeShim(dir, "/old/location/App", "darwin"); + + ensureNodeShim(dir, EXEC_PATH, "darwin"); + + expect(readFileSync(join(dir, "node"), "utf-8")).toBe( + buildNodeShimScript(EXEC_PATH), + ); + }); + + it("leaves an up-to-date wrapper untouched", () => { + const dir = makeDir(); + ensureNodeShim(dir, EXEC_PATH, "darwin"); + const shim = join(dir, "node"); + const past = new Date("2020-01-01T00:00:00Z"); + utimesSync(shim, past, past); + + ensureNodeShim(dir, EXEC_PATH, "darwin"); + + expect(statSync(shim).mtimeMs).toBe(past.getTime()); + }); + + it("replaces a corrupt shim that is not the expected script", () => { + const dir = makeDir(); + ensureNodeShim(dir, EXEC_PATH, "darwin"); + const shim = join(dir, "node"); + writeFileSync(shim, "not a shim"); + + ensureNodeShim(dir, EXEC_PATH, "darwin"); + + expect(readFileSync(shim, "utf-8")).toBe(buildNodeShimScript(EXEC_PATH)); + }); + + it("keeps the symlink behavior on windows and tolerates an existing link", () => { + const dir = makeDir(); + ensureNodeShim(dir, EXEC_PATH, "win32"); + ensureNodeShim(dir, EXEC_PATH, "win32"); + + const shim = join(dir, "node"); + expect(lstatSync(shim).isSymbolicLink()).toBe(true); + expect(readlinkSync(shim)).toBe(EXEC_PATH); + }); + + it("retargets the win32 symlink when the binary path changes", () => { + const dir = makeDir(); + ensureNodeShim(dir, "/old/location/App.exe", "win32"); + + ensureNodeShim(dir, EXEC_PATH, "win32"); + + expect(readlinkSync(join(dir, "node"))).toBe(EXEC_PATH); + }); +}); diff --git a/packages/workspace-server/src/services/agent/node-shim.ts b/packages/workspace-server/src/services/agent/node-shim.ts new file mode 100644 index 0000000000..6ca6d122f3 --- /dev/null +++ b/packages/workspace-server/src/services/agent/node-shim.ts @@ -0,0 +1,70 @@ +import { + chmodSync, + lstatSync, + mkdirSync, + readFileSync, + readlinkSync, + renameSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { join } from "node:path"; +import { buildNodeShimScript } from "@posthog/shared/node-shim"; + +function isErrnoException(err: unknown): err is NodeJS.ErrnoException { + return err instanceof Error && "code" in err; +} + +// Writes the `node` shim agents resolve via PATH. POSIX shims are wrapper +// scripts that set ELECTRON_RUN_AS_NODE themselves, so a descendant that +// strips the var still gets a node runtime instead of a phantom app boot (a +// bare symlink relied on every layer preserving the env). Self-heals legacy +// symlinks and shims pointing at a moved binary; no-op when already current. +export function ensureNodeShim( + mockNodeDir: string, + execPath: string, + platform: NodeJS.Platform = process.platform, +): void { + mkdirSync(mockNodeDir, { recursive: true }); + const shimPath = join(mockNodeDir, "node"); + + if (platform === "win32") { + // No sh on win32, so the shim stays a symlink; the bootstrap + // internal-child guard is the backstop against phantom boots there. + if (currentSymlinkTarget(shimPath) === execPath) return; + rmSync(shimPath, { force: true }); + try { + symlinkSync(execPath, shimPath); + } catch (err) { + if (!isErrnoException(err) || err.code !== "EEXIST") throw err; + } + return; + } + + const script = buildNodeShimScript(execPath); + if (currentShimContent(shimPath) === script) return; + + const tmpPath = `${shimPath}.${process.pid}.tmp`; + writeFileSync(tmpPath, script); + chmodSync(tmpPath, 0o755); + renameSync(tmpPath, shimPath); +} + +function currentSymlinkTarget(shimPath: string): string | null { + try { + if (!lstatSync(shimPath).isSymbolicLink()) return null; + return readlinkSync(shimPath); + } catch { + return null; + } +} + +function currentShimContent(shimPath: string): string | null { + try { + if (lstatSync(shimPath).isSymbolicLink()) return null; + return readFileSync(shimPath, "utf-8"); + } catch { + return null; + } +} diff --git a/packages/workspace-server/src/services/shell/shell.test.ts b/packages/workspace-server/src/services/shell/shell.test.ts index e63ff12f13..4a5add895a 100644 --- a/packages/workspace-server/src/services/shell/shell.test.ts +++ b/packages/workspace-server/src/services/shell/shell.test.ts @@ -13,6 +13,21 @@ const mockPty = vi.hoisted(() => ({ vi.mock("node-pty", () => mockPty); +const mockExec = vi.hoisted(() => + vi.fn( + ( + _command: string, + _options: unknown, + callback: (error: null, stdout: string, stderr: string) => void, + ) => callback(null, "", ""), + ), +); + +vi.mock("node:child_process", async (importOriginal) => { + const original = await importOriginal(); + return { ...original, exec: mockExec }; +}); + const mockGitQueries = vi.hoisted(() => ({ getCurrentBranch: vi.fn(async () => "feature-branch"), getDefaultBranch: vi.fn(async () => "main"), @@ -153,4 +168,62 @@ describe("ShellService.createSession workspace env", () => { expect(mockPty.spawn).toHaveBeenCalledTimes(1); expect(spawnedEnv().POSTHOG_CODE_WORKSPACE_PATH).toBeUndefined(); }); + + it("strips the internal-child markers the workspace-server runs with", async () => { + // The workspace-server inherits both vars from apps/code (service.ts); a + // user terminal must inherit neither. ELECTRON_RUN_AS_NODE would make + // Electron CLIs run as node; POSTHOG_CODE_INTERNAL_CHILD would trip the + // bootstrap guard so a direct app-binary launch exits(1). + const saved = { + runAsNode: process.env.ELECTRON_RUN_AS_NODE, + internalChild: process.env.POSTHOG_CODE_INTERNAL_CHILD, + }; + process.env.ELECTRON_RUN_AS_NODE = "1"; + process.env.POSTHOG_CODE_INTERNAL_CHILD = "1"; + try { + const { service } = createWorktreeTaskService("/does/not/exist"); + + await service.createSession({ sessionId: "session-1", taskId: "task-1" }); + + expect(spawnedEnv().ELECTRON_RUN_AS_NODE).toBeUndefined(); + expect(spawnedEnv().POSTHOG_CODE_INTERNAL_CHILD).toBeUndefined(); + } finally { + restoreEnv("ELECTRON_RUN_AS_NODE", saved.runAsNode); + restoreEnv("POSTHOG_CODE_INTERNAL_CHILD", saved.internalChild); + } + }); }); + +describe("ShellService.execute", () => { + it("runs commands with the sanitized shell env", async () => { + const saved = { + runAsNode: process.env.ELECTRON_RUN_AS_NODE, + internalChild: process.env.POSTHOG_CODE_INTERNAL_CHILD, + }; + process.env.ELECTRON_RUN_AS_NODE = "1"; + process.env.POSTHOG_CODE_INTERNAL_CHILD = "1"; + try { + const { service } = createService(); + + await service.execute("/repo", "echo hi"); + + const options = mockExec.mock.calls[0][1] as { + env: Record; + }; + expect(options.env.ELECTRON_RUN_AS_NODE).toBeUndefined(); + expect(options.env.POSTHOG_CODE_INTERNAL_CHILD).toBeUndefined(); + expect(options.env.TERM_PROGRAM).toBe("PostHog Code"); + } finally { + restoreEnv("ELECTRON_RUN_AS_NODE", saved.runAsNode); + restoreEnv("POSTHOG_CODE_INTERNAL_CHILD", saved.internalChild); + } + }); +}); + +function restoreEnv(key: string, value: string | undefined): void { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } +} diff --git a/packages/workspace-server/src/services/shell/shell.ts b/packages/workspace-server/src/services/shell/shell.ts index 4b79efb9e2..2e8f3db605 100644 --- a/packages/workspace-server/src/services/shell/shell.ts +++ b/packages/workspace-server/src/services/shell/shell.ts @@ -11,6 +11,7 @@ import { WORKSPACE_SETTINGS_SERVICE, } from "@posthog/platform/workspace-settings"; import { TypedEventEmitter } from "@posthog/shared"; +import { POSTHOG_CODE_INTERNAL_CHILD_ENV } from "@posthog/shared/constants"; import { inject, injectable, preDestroy } from "inversify"; import * as pty from "node-pty"; import { @@ -67,6 +68,12 @@ function buildShellEnv( ): Record { const env = { ...process.env } as Record; + // User-facing shells must not inherit the workspace-server's internal + // markers: ELECTRON_RUN_AS_NODE makes any Electron-based CLI run as node, + // and the internal-child marker makes the packaged app refuse to launch. + delete env.ELECTRON_RUN_AS_NODE; + delete env[POSTHOG_CODE_INTERNAL_CHILD_ENV]; + if (platform() === "darwin" && !process.env.LC_ALL) { const locale = process.env.LC_CTYPE || "en_US.UTF-8"; Object.assign(env, { @@ -372,13 +379,17 @@ export class ShellService extends TypedEventEmitter { execute(cwd: string, command: string): Promise { return new Promise((resolve) => { - exec(command, { cwd, timeout: 60000 }, (error, stdout, stderr) => { - resolve({ - stdout: stdout || "", - stderr: stderr || "", - exitCode: error?.code ?? 0, - }); - }); + exec( + command, + { cwd, timeout: 60000, env: buildShellEnv() }, + (error, stdout, stderr) => { + resolve({ + stdout: stdout || "", + stderr: stderr || "", + exitCode: error?.code ?? 0, + }); + }, + ); }); }