[Bug] Desktop leaks ELECTRON_RUN_AS_NODE into every child process - Electron apps launched from a DSH shell start as Node

[huiselichang]

Summary

apps/desktop deliberately runs the Harness under Electron with ELECTRON_RUN_AS_NODE=1. That variable is a process-wide selector, not data: Electron treats it as present-or-absent (even "" / 0 / false enable Node mode). It is not scrubbed from the environment handed to child processes, so it is inherited by shell tools, terminal (PTY) sessions, MCP servers, LSP servers, subagent CLIs, etc.

Consequence: any Electron application launched from a DSH shell starts as a plain Node process — no window, and its main process typically throws (app is undefined, require('electron') returns a path string). This breaks every Electron project a developer tries to start (npm run dev, electron ., electron-vite, electron-forge, or an already-packaged .exe), and the failure is silent (exit 0 or a confusing TypeError), so it costs a lot of time to diagnose.

Reproduction

Windows, DSH desktop (nightly). Inside a DSH shell, with any Electron project:

$env:ELECTRON_RUN_AS_NODE     # -> 1
npx electron --version        # -> v24.x  (Electron's bundled Node) instead of the Electron version, e.g. v44.4.1
npm run dev                   # -> no window; main process dies with:
                              #    TypeError: Cannot read properties of undefined (reading 'isPackaged')

Also reproduces with a packaged build: launching the built *.exe from a DSH shell exits immediately with code 0 and no window. Launching the same .exe from Explorer, or after clearing the variable in the shell, works normally.

Workaround that fixes it (on the user side only):

Remove-Item Env:ELECTRON_RUN_AS_NODE -ErrorAction SilentlyContinue
npm run dev

Note: $env:ELECTRON_RUN_AS_NODE = '' does not work in PowerShell, and cross-env ELECTRON_RUN_AS_NODE= cannot work either, because Electron checks presence rather than value — only actually removing the variable helps.

Current behavior

Every child spawned through the shared scrub base inherits the selector:

• packages/subprocess/subprocess/src/index.ts — SENSITIVE_ENV_PATTERN = /KEY|PASSWORD|SECRET|TOKEN/i and scrubbedParentEnv(); only credential-shaped names and DSH_* are dropped, so ELECTRON_RUN_AS_NODE passes through.
• packages/subprocess/subprocess-local/src/spawn.ts / runner-launch.ts build every ordinary child env from that base.
• Shell and terminal layer on the same base: packages/shell/pwsh-local/src/index.ts (env: { ...ENV_OVERRIDES, ...spec.env, ...spec.dshEnv }) and packages/terminal/terminal-bash/src/index.ts ("The subprocess provider supplies its own scrubbed ambient base…").
• Same base is used by MCP/LSP/subagent/open-in-app spawn paths.

So the leak is not specific to one tool: it is the environment policy of the whole subprocess seam.

Expected behavior

Children should not inherit the harness's own Node-mode selector. Every deliberate consumer already passes it explicitly in env, so nothing depends on inheritance:

• apps/desktop/src/node-environment.ts (desktopNodeEnvironment() returns { ...environment, ELECTRON_RUN_AS_NODE: '1', … })
• apps/desktop-host/src/index.ts (package manager launch)
• apps/desktop/scripts/node-bin/node.cmd / node (sets it in the launcher itself)
• packages/host/open-in-app/src/resolver.ts (Electron-based CLIs)
• packages/experimental/speech-to-text-sensevoice/src/recognizer.ts

And there is already a precedent for treating it as something to strip: packages/ptc-runtime/ptc-runtime-node/src/index.ts filters it out of the child environment, with the documented rationale "The host preserves ELECTRON_RUN_AS_NODE only for child startup so the Desktop executable runs the Node bootstrap; the bootstrap removes the selector before evaluating model code" (see that package's README and tests/host-failures.spec.ts, which asserts the variable is absent).

Suggested fix

Make the shared scrub tombstone the selector, matching the ptc-runtime-node precedent:

// packages/subprocess/subprocess/src/index.ts
const TOMBSTONED_ENV_NAMES = new Set(['ELECTRON_RUN_AS_NODE']) // process-wide selector; must not be inherited

if (value !== undefined
    && !SENSITIVE_ENV_PATTERN.test(key)
    && !key.toUpperCase().startsWith(DSH_ENV_PREFIX)
    && !TOMBSTONED_ENV_NAMES.has(key.toUpperCase())) env[key] = value

Explicit env layers merge after the scrub (childEnv(extra)), so all deliberate uses above keep working unchanged — only passive inheritance is cut. NODE_OPTIONS is the same class of variable and is worth considering together.

Environment

• DSH desktop, Windows 11 (nightly channel); observed on the installed build and on master @ 0.2.0-rc.1 (source-verified).
• PowerShell 7.6.
• Affected Electron version in the failing project: any (verified with Electron 44.4.1; the selector is long-standing Electron behavior).

[argszero]

The diagnosis is right, and the presence-semantics detail ('' / 0 / false all enable Node mode, so only removal works) is the part most workarounds get wrong. One checkable refinement before this lands: the list of "every deliberate consumer already passes it explicitly" is missing the two sites where the harness launches its own runners, and those two are exactly where the selector is load-bearing.

The two omissions

Both runner invocations are [process.execPath, …], and neither passes the variable:

• packages/subprocess/subprocess-local/src/runner-launch.ts:29-42 — spawnRunnerInvocation() returns [process.execPath, <runner.js>] (or the --import tsx source form).
• packages/sandbox/sandbox-local/src/index.ts — windowsAclRunnerInvocation() returns [process.execPath, builtEntry] for the ACL runner.

Why a tombstone in the shared scrub breaks them

On Windows the subprocess runner is launched through launchWindowsJob() (packages/subprocess/subprocess-local/src/index.ts:192-194, chosen by selectContainmentMode()), and its environment is built by runnerEnvironment():

// runner-launch.ts:68-86
const env = childEnv()                       // <- the scrub you are changing
for (const name of Object.keys(env)) {       // strips only RUNNER_CONTROL_ENV_PREFIXES
  if (RUNNER_CONTROL_ENV_PREFIXES.some(p => normalized.startsWith(p))) Reflect.deleteProperty(env, name)
}
return { ...env, [SUBPROCESS_RUNNER_ENV]: selection, SYSTEMD_LOG_TARGET: 'null', ... }

childEnv() is scrubbedParentEnv() plus the explicit layer, and this explicit layer adds only SUBPROCESS_RUNNER_ENV, SYSTEMD_LOG_TARGET and (source form) TSX_TSCONFIG_PATH — not ELECTRON_RUN_AS_NODE. So the selector reaches the runner by inheritance alone. Add TOMBSTONED_ENV_NAMES to scrubbedParentEnv() and the child [Electron.exe, runner.js] starts with no selector — which is to say it starts as an application, not as Node, and runner.js never executes.

Independent measurement, same host, three days later

#8193 captured the Desktop process tree live (30 ms polling of Win32_Process). Chain 1 is the harness host successfully launching that very runner:

[10:30:07.307] pid=52860 DeepSeek Harness.exe   parent=56976(DeepSeek Harness.exe)
    cmd: "...\app.asar\dsh\node_modules\@deepseek-ai\dsh-subprocess-local\lib\runner.js"
         -- "...\pwsh.exe" -NoLogo -NoProfile -NonInteractive -Command "..."
[10:30:08.066] pid=62288 pwsh.exe               parent=52860(DeepSeek Harness.exe)   ← ran fine

DeepSeek Harness.exe is the Electron image running runner.js, so the selector was present in that child's environment. It is inherited, and the source of the inheritance is the runtime's own process.env: apps/desktop/src/host-process.ts:199 spawns the harness host with desktopNodeEnvironment(...), which sets ELECTRON_RUN_AS_NODE: '1' (apps/desktop/src/node-environment.ts:12-17). One fix for the leak, two silent breakages at the sites above — and "silent" in your sense: non-zero-free, no diagnostic.

Suggested adjustment

Keep the tombstone in the shared base (it is the right place for "not inherited"), and preserve the selector explicitly where a runner is the child — one entry in runnerEnvironment()'s explicit layer, the way SUBPROCESS_RUNNER_ENV already gets there, plus the same for the sandbox ACL runner path. That keeps your property intact: explicit layers merge after the scrub, so every deliberate use survives and only passive inheritance is cut. Without those two entries the change is not "only passive inheritance is cut".

Worth noting the precedent you cite is deliberately weaker than the suggested tombstone, and in a useful way: packages/ptc-runtime/ptc-runtime-node/src/index.ts:228-231 builds a per-spawn tombstone map (key -> undefined) for the ambient names including ELECTRON_RUN_AS_NODE, with the comment "Electron needs its Node-mode selector until bootstrap; the child then removes it with other ambient values" — i.e. the selector is allowed to reach the child's startup, and removed at the target boundary. That is the same shape as the adjustment above, applied per spawner instead of globally.

On NODE_OPTIONS: partly handled already, narrowly. RUNNER_CONTROL_ENV_PREFIXES = ['NODE_', 'TSX_'] (runner-launch.ts:23) strips NODE_* from the runner's environment — the runner deliberately does not get the ambient NODE_OPTIONS — but ordinary children still do. So it is worth considering together, as you say, with the same question of which consumers depend on it.

Related, not a fix for this. @argszero/cordis-plugin-sandbox-grant-advisor@0.7.1 (npm, source) carries the reader-facing half of the coupling above: its native-init advisory tells a stuck Desktop session not to unset ELECTRON_RUN_AS_NODE as a workaround, because the desktop's runner is that Electron binary. It observes tools/post-execute and changes no environment itself — it is a diagnosis, so it does not address the leak you report; it only documents the same constraint from the other side.