diff --git a/src/main/wsl/__fixtures__/wsl-invocation-allowlist.txt b/src/main/wsl/__fixtures__/wsl-invocation-allowlist.txt new file mode 100644 index 00000000000..6fa44e9de5f --- /dev/null +++ b/src/main/wsl/__fixtures__/wsl-invocation-allowlist.txt @@ -0,0 +1,27 @@ +# Files that spawn wsl.exe directly instead of through src/main/wsl/wsl-runner.ts. +# Enforced by ../wsl-invocation-boundary.test.ts. This list only shrinks -- its +# length is the W3 goalpost. Removing an entry means the file now goes through +# the runner, not that its call was tidied in place. +main/agent-hooks/wsl-hook-relay-launch.ts +main/claude-accounts/runtime-auth-service.ts +main/claude-accounts/service.ts +main/cli/wsl-cli-installer.ts +main/codex-accounts/runtime-home-service.ts +main/codex-accounts/service.ts +main/codex/codex-state-db-backfill-recovery.ts +main/codex/codex-trust-grant-host.ts +main/codex/codex-wsl-hook-install-plan.ts +main/git/wsl-git-read-environment.ts +main/hooks.ts +main/ipc/filesystem-watcher-wsl.ts +main/ipc/preflight-wsl-agent-detection.ts +main/ipc/preflight-wsl-command.ts +main/local-worktree-filesystem.ts +main/skills/claude-plugin-skill-sources-wsl.ts +main/skills/skill-discovery-wsl.ts +main/skills/skill-provider-runtime-roots.ts +main/skills/skill-wsl-install-filesystem.ts +main/skills/skill-wsl-provider-detection.ts +main/wsl-availability.ts +main/wsl-unc-delete.ts +main/wsl.ts diff --git a/src/main/wsl/wsl-executable-path.ts b/src/main/wsl/wsl-executable-path.ts new file mode 100644 index 00000000000..5767f7b0a79 --- /dev/null +++ b/src/main/wsl/wsl-executable-path.ts @@ -0,0 +1,20 @@ +import { existsSync } from 'node:fs' +import { win32 as pathWin32 } from 'node:path' + +/** + * Absolute path to `wsl.exe`. + * + * Why not the bare name: spawning by name resolves through the child's PATH, + * which a Group Policy, a stripped Electron environment or a shadowing entry + * can point somewhere else — the same class of failure W1 fixed for PowerShell + * (#15749). System32 is the launcher Microsoft ships; the Store package + * forwards through it. + */ +export function resolveWslExecutablePath(): string { + const systemRoot = process.env.SystemRoot || 'C:\\Windows' + const absolute = pathWin32.join(systemRoot, 'System32', 'wsl.exe') + // Why the fallback: a host that has WSL somewhere else still deserves a + // working call, and PATH resolution is strictly better than a path we know + // is wrong. + return existsSync(absolute) ? absolute : 'wsl.exe' +} diff --git a/src/main/wsl/wsl-guest-environment.test.ts b/src/main/wsl/wsl-guest-environment.test.ts new file mode 100644 index 00000000000..1e797d9d88e --- /dev/null +++ b/src/main/wsl/wsl-guest-environment.test.ts @@ -0,0 +1,138 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const runProcessMock = vi.hoisted(() => vi.fn()) +vi.mock('../../shared/child-process/run-process', () => ({ runProcess: runProcessMock })) +vi.mock('./wsl-executable-path', () => ({ resolveWslExecutablePath: () => 'wsl.exe' })) + +import { + getWslGuestEnvironment, + invalidateWslGuestEnvironment, + peekWslGuestEnvironment +} from './wsl-guest-environment' + +/** Echo a well-formed payload back inside whatever fence the probe generated. */ +function respondWithPayload(payload: string, code = 0): void { + runProcessMock.mockImplementation(async (spec: { args: string[] }) => { + const script = spec.args.at(-1) ?? '' + const begin = /__ORCA_WSL_CAPTURE_BEGIN_[a-z0-9]+__/.exec(script)?.[0] ?? '' + const end = /__ORCA_WSL_CAPTURE_END_[a-z0-9]+__/.exec(script)?.[0] ?? '' + return { + code, + signal: null, + stdout: `distro banner\n${begin}${payload}${end}`, + stderr: '', + timedOut: false + } + }) +} + +const GOOD = ['/home/u/.nvm/bin:/usr/bin', '/home/u', '/usr/bin/env'].join('\0') + +beforeEach(() => { + runProcessMock.mockReset() + invalidateWslGuestEnvironment() +}) +afterEach(() => invalidateWslGuestEnvironment()) + +describe('probing', () => { + it('reads PATH, HOME and env out of a banner-polluted stdout', async () => { + respondWithPayload(GOOD) + expect(await getWslGuestEnvironment('Ubuntu')).toEqual({ + path: '/home/u/.nvm/bin:/usr/bin', + home: '/home/u', + envBinary: '/usr/bin/env' + }) + }) + + it('collapses a concurrent burst into one probe', async () => { + // Teardown and detection both fan out; 32 login shells per burst is the + // cost this cache exists to avoid. + respondWithPayload(GOOD) + await Promise.all(Array.from({ length: 32 }, () => getWslGuestEnvironment('Ubuntu'))) + expect(runProcessMock).toHaveBeenCalledTimes(1) + }) + + it('keeps distros isolated', async () => { + respondWithPayload(GOOD) + await getWslGuestEnvironment('Ubuntu') + await getWslGuestEnvironment('Debian') + expect(runProcessMock).toHaveBeenCalledTimes(2) + }) +}) + +describe('bad answers are not cached as good ones', () => { + it.each([ + ['a relative HOME', ['/usr/bin', 'home/u', '/usr/bin/env'].join('\0')], + ['a PATH with a newline', ['/usr/bin\nx', '/home/u', '/usr/bin/env'].join('\0')], + ['a truncated payload', '/usr/bin'], + ['a relative env binary', ['/usr/bin', '/home/u', 'env'].join('\0')] + ])('rejects %s', async (_case, payload) => { + respondWithPayload(payload) + expect(await getWslGuestEnvironment('Ubuntu')).toBeNull() + expect(peekWslGuestEnvironment('Ubuntu')).toBeUndefined() + }) + + it('treats a missing fence as a failed probe, not an empty PATH', async () => { + runProcessMock.mockResolvedValue({ + code: 0, + signal: null, + stdout: 'only a banner, no fence', + stderr: '', + timedOut: false + }) + expect(await getWslGuestEnvironment('Ubuntu')).toBeNull() + }) +}) + +describe('transient versus permanent failure', () => { + it('does not retry a distro that cannot produce env', async () => { + // 127 is the probe's own "no usable env". Retrying that forever turns a + // probe into a poller. + runProcessMock.mockResolvedValue({ + code: 127, + signal: null, + stdout: '', + stderr: '', + timedOut: false + }) + await getWslGuestEnvironment('Ubuntu') + await getWslGuestEnvironment('Ubuntu') + expect(runProcessMock).toHaveBeenCalledTimes(1) + }) + + it('retries after the window when the probe timed out', async () => { + vi.useFakeTimers() + try { + runProcessMock.mockResolvedValue({ + code: null, + signal: null, + stdout: '', + stderr: '', + timedOut: true + }) + expect(await getWslGuestEnvironment('Ubuntu')).toBeNull() + expect(await getWslGuestEnvironment('Ubuntu')).toBeNull() + expect(runProcessMock).toHaveBeenCalledTimes(1) + + vi.setSystemTime(Date.now() + 31_000) + respondWithPayload(GOOD) + expect(await getWslGuestEnvironment('Ubuntu')).not.toBeNull() + expect(runProcessMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) +}) + +describe('invalidation', () => { + it('re-probes after the caller invalidates, so a newly installed tool appears', async () => { + // A user who installs nvm inside a running distro would otherwise keep the + // pre-install PATH until Orca restarts, and read that as the detection bug + // this cache exists to fix. + respondWithPayload(GOOD) + await getWslGuestEnvironment('Ubuntu') + invalidateWslGuestEnvironment('Ubuntu') + respondWithPayload(['/opt/new/bin', '/home/u', '/usr/bin/env'].join('\0')) + expect((await getWslGuestEnvironment('Ubuntu'))?.path).toBe('/opt/new/bin') + }) +}) diff --git a/src/main/wsl/wsl-guest-environment.ts b/src/main/wsl/wsl-guest-environment.ts new file mode 100644 index 00000000000..cf21b373c14 --- /dev/null +++ b/src/main/wsl/wsl-guest-environment.ts @@ -0,0 +1,172 @@ +import { runProcess } from '../../shared/child-process/run-process' +import { + buildWslCapturedLoginShellCommand, + buildWslExecArgs +} from '../../shared/wsl-login-shell-command' +import { resolveWslExecutablePath } from './wsl-executable-path' + +/** + * The login-shell environment of a distro, probed once and cached. + * + * Why this exists: a probe needs the user's real PATH (nvm, mise and asdf all + * install into rc files), but paying for a login shell on every probe is what + * made `cli:getWslInstallStatus` time out behind a blocking `~/.profile` + * (#14288) and what makes WSL git operations lag (#9768). Probe the login shell + * exactly once per distro, then run everything else with no shell at all. + * + * Generalised from `src/main/git/wsl-git-read-environment.ts`, which is the one + * WSL caller that already got this right. + */ + +export type WslGuestEnvironment = { + /** Login-shell PATH, as the user's own terminal would see it. */ + path: string + home: string + /** Absolute path to `env`, used to run programs without a shell. */ + envBinary: string +} + +const PROBE_TIMEOUT_MS = 10_000 +const PROBE_MAX_OUTPUT_BYTES = 64 * 1024 +/** + * Why retry a timeout but not a malformed answer: a stopped distro or a + * momentarily wedged `wsl.exe` recovers on its own, while a distro that cannot + * produce a POSIX PATH will not start doing so. Retrying the second forever is + * how a probe becomes a poller. + */ +const TRANSIENT_RETRY_MS = 30_000 + +type ProbeOutcome = + | { kind: 'resolved'; environment: WslGuestEnvironment } + | { kind: 'rejected' } + | { kind: 'transient' } + +const inFlight = new Map>() +const resolved = new Map() +const retryAfter = new Map() + +/** A payload that is not three absolute, single-line POSIX values is a failed probe. */ +function parseProbePayload(payload: string | null): WslGuestEnvironment | null { + if (payload === null) { + return null + } + const [path = '', home = '', envBinary = ''] = payload.split('\0') + const isCleanAbsolute = (value: string): boolean => + value.startsWith('/') && !value.includes('\n') && !value.includes('\r') + if (!path.includes('/') || path.length > 32_768 || path.includes('\n')) { + return null + } + if (!isCleanAbsolute(home) || !isCleanAbsolute(envBinary)) { + return null + } + return { path, home, envBinary } +} + +async function probeGuestEnvironment(distro: string | undefined): Promise { + // Why resolve `env` rather than assume /usr/bin/env: it is /usr/bin/env on + // Debian, Ubuntu, Fedora and Arch, but the probe costs nothing extra here and + // a distro that puts it elsewhere would otherwise fail every later call. + const script = [ + '_orca_env=$(command -v env 2>/dev/null || true)', + 'case "$_orca_env" in /*) [ -x "$_orca_env" ] || exit 127 ;; *) exit 127 ;; esac', + `printf '%s\\0%s\\0%s' "$PATH" "$HOME" "$_orca_env"` + ].join('\n') + const captured = buildWslCapturedLoginShellCommand(script) + const result = await runProcess({ + program: resolveWslExecutablePath(), + args: buildWslExecArgs(distro, ['sh', '-c', captured.command]), + timeoutMs: PROBE_TIMEOUT_MS, + maxOutputBytes: PROBE_MAX_OUTPUT_BYTES + }) + if (result.timedOut) { + return { kind: 'transient' } + } + if (result.code !== 0) { + // 127 is our own "no usable env"; anything else is the distro being + // unavailable, which is worth retrying. + return result.code === 127 ? { kind: 'rejected' } : { kind: 'transient' } + } + const environment = parseProbePayload(captured.readStdout(result.stdout)) + return environment ? { kind: 'resolved', environment } : { kind: 'rejected' } +} + +function cacheKey(distro: string | undefined): string { + return distro ?? '' +} + +/** + * The distro's login-shell environment, or null when it cannot be established. + * + * Null is "we could not ask", never "the distro has no PATH" — callers fall + * back to the interactive lane rather than running with an empty environment. + */ +export function getWslGuestEnvironment( + distro: string | undefined +): Promise { + const key = cacheKey(distro) + const retry = retryAfter.get(key) + if (retry !== undefined && Date.now() >= retry) { + inFlight.delete(key) + retryAfter.delete(key) + } + const existing = inFlight.get(key) + if (existing) { + return existing + } + // Why store the promise before awaiting: a 32-wide burst during teardown must + // collapse into one probe, not 32 login shells. + const probe = probeGuestEnvironment(distro).then((outcome) => { + if (inFlight.get(key) !== probe) { + return outcome.kind === 'resolved' ? outcome.environment : null + } + if (outcome.kind === 'resolved') { + resolved.set(key, outcome.environment) + retryAfter.delete(key) + return outcome.environment + } + if (outcome.kind === 'transient') { + retryAfter.set(key, Date.now() + TRANSIENT_RETRY_MS) + } + return null + }) + inFlight.set(key, probe) + return probe +} + +/** + * Drop a distro's cached environment. + * + * Why callers need this: a user who installs nvm inside a running distro would + * otherwise keep the pre-install PATH until Orca restarts, and read that as the + * same detection bug this cache exists to fix. + */ +export function invalidateWslGuestEnvironment(distro?: string): void { + if (distro === undefined) { + inFlight.clear() + resolved.clear() + retryAfter.clear() + return + } + const key = cacheKey(distro) + inFlight.delete(key) + resolved.delete(key) + retryAfter.delete(key) +} + +/** Test-only: the cached value without probing. */ +export function peekWslGuestEnvironment( + distro: string | undefined +): WslGuestEnvironment | undefined { + return resolved.get(cacheKey(distro)) +} + +/** Test-only: pretend a distro has already been probed. */ +export function seedWslGuestEnvironmentForTests( + distro: string | undefined, + environment: WslGuestEnvironment +): void { + const key = cacheKey(distro) + inFlight.set(key, Promise.resolve(environment)) + resolved.set(key, environment) + retryAfter.delete(key) +} diff --git a/src/main/wsl/wsl-invocation-boundary.test.ts b/src/main/wsl/wsl-invocation-boundary.test.ts new file mode 100644 index 00000000000..e2fc6440b3e --- /dev/null +++ b/src/main/wsl/wsl-invocation-boundary.test.ts @@ -0,0 +1,101 @@ +import { readdirSync, readFileSync, statSync } from 'node:fs' +import { join, relative, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' + +/** + * Every `wsl.exe` spawn must go through `runWslProcess`. + * + * Why a guard and not review: five decisions have to be made on each call + * (separator, shell, stdout fencing, WSLENV, payload transport), each is + * invisible in a diff, and each has shipped wrong. `wsl-exec-mode-separator` + * already guards one of the five — this guards the call itself, so the other + * four cannot be re-decided per site. + * + * The allowlist is the W3 migration worklist and only shrinks. Its length is + * the workstream's measured goalpost. + */ +const ALLOWLIST: readonly string[] = readFileSync( + join(__dirname, '__fixtures__', 'wsl-invocation-allowlist.txt'), + 'utf8' +) + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.length > 0 && !line.startsWith('#')) + +const SOURCE_ROOT = resolve(__dirname, '../..') +// Why the trailing slash: a bare 'main/wsl' prefix also exempts main/wsl.ts, +// main/wsl-availability.ts and main/wsl-unc-delete.ts -- three files that spawn +// wsl.exe directly. Caught by testing the guard against a planted call site. +const OWNER_DIRECTORY = 'main/wsl/' +const IGNORED = new Set(['node_modules', 'dist', 'out', 'build', '.git', '__fixtures__']) +/** + * A spawn site: the `wsl.exe` literal sits directly in a spawn-family call, or + * in a `program:` field. A bare mention (a shell-name constant, a type, a + * comment) is not a call and is not the guard's business. + */ +const SPAWN_OPENER = + /(?:spawn|spawnSync|execFile|execFileSync|exec|fork|runProcess|spawnProcess|execFileAsync|execAsync)\s*\(\s*$|program:\s*$/ + +function isTestFile(path: string): boolean { + return ( + /\.(?:test|spec)\.tsx?$/.test(path) || + /(?:test-harness|test-utils|test-setup|test-fixture|repro)/.test(path) + ) +} + +function collectSourceFiles(root: string): string[] { + const found: string[] = [] + for (const entry of readdirSync(root)) { + if (IGNORED.has(entry) || entry.startsWith('.')) { + continue + } + const path = join(root, entry) + if (statSync(path).isDirectory()) { + found.push(...collectSourceFiles(path)) + continue + } + if (/\.tsx?$/.test(entry)) { + found.push(path) + } + } + return found +} + +function findSpawnSites(): string[] { + const offenders = new Set() + for (const path of collectSourceFiles(SOURCE_ROOT)) { + const relativePath = relative(SOURCE_ROOT, path).replace(/\\/g, '/') + if (isTestFile(relativePath) || relativePath.startsWith(OWNER_DIRECTORY)) { + continue + } + const source = readFileSync(path, 'utf8') + for (const match of source.matchAll(/['"]wsl\.exe['"]/g)) { + // Collapse the preceding whitespace so a call broken across lines by the + // formatter still reads as one opener. + const preceding = source.slice(Math.max(0, match.index - 60), match.index) + if (SPAWN_OPENER.test(preceding.replace(/\s+/g, ' ').replace(/ $/, ''))) { + offenders.add(relativePath) + } + } + } + return [...offenders].sort() +} + +describe('wsl.exe is spawned through one runner', () => { + const offenders = findSpawnSites() + + it('finds the call sites it is meant to guard', () => { + // Guards against a detection change quietly emptying the scan, which would + // make the assertions below pass without checking anything. + expect(offenders.length + ALLOWLIST.length).toBeGreaterThanOrEqual(20) + }) + + it('adds no new direct wsl.exe spawn', () => { + expect(offenders.filter((path) => !ALLOWLIST.includes(path))).toEqual([]) + }) + + it('carries no stale allowlist entry', () => { + // A migrated file must leave the list, or the goalpost stops moving. + expect(ALLOWLIST.filter((path) => !offenders.includes(path))).toEqual([]) + }) +}) diff --git a/src/main/wsl/wsl-runner.test.ts b/src/main/wsl/wsl-runner.test.ts new file mode 100644 index 00000000000..2c014fb9e86 --- /dev/null +++ b/src/main/wsl/wsl-runner.test.ts @@ -0,0 +1,216 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const runProcessMock = vi.hoisted(() => vi.fn()) +vi.mock('../../shared/child-process/run-process', () => ({ runProcess: runProcessMock })) +vi.mock('./wsl-executable-path', () => ({ + resolveWslExecutablePath: () => 'C:\\Windows\\System32\\wsl.exe' +})) + +import { runWslProcess } from './wsl-runner' +import { + invalidateWslGuestEnvironment, + seedWslGuestEnvironmentForTests +} from './wsl-guest-environment' + +const ENVIRONMENT = { + path: '/home/u/.nvm/bin:/usr/bin', + home: '/home/u', + envBinary: '/usr/bin/env' +} + +function lastArgv(): string[] { + return runProcessMock.mock.calls.at(-1)?.[0].args as string[] +} + +beforeEach(() => { + runProcessMock.mockReset() + runProcessMock.mockResolvedValue({ + code: 0, + signal: null, + stdout: '', + stderr: '', + timedOut: false + }) + invalidateWslGuestEnvironment() +}) + +afterEach(() => { + invalidateWslGuestEnvironment() +}) + +describe('separator', () => { + it.each([ + ['probe', 'probe'], + ['interactive', 'interactive'] + ] as const)('uses --exec and never -- on the %s lane', async (_name, lane) => { + // Why this is pinned on both lanes: under `--`, wsl.exe expands $name in + // every forwarded argument before the guest runs -- even with no shell in + // the command -- so a script means something other than what it says + // (#12964). No escaping on our side is a reliable substitute. + seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) + await runWslProcess({ lane, program: '/usr/bin/git', args: ['status'] }) + expect(lastArgv()).toContain('--exec') + expect(lastArgv()).not.toContain('--') + }) + + it('passes the distro before --exec', async () => { + seedWslGuestEnvironmentForTests('Ubuntu', ENVIRONMENT) + await runWslProcess({ lane: 'probe', distro: 'Ubuntu', program: '/bin/true' }) + expect(lastArgv().slice(0, 3)).toEqual(['-d', 'Ubuntu', '--exec']) + }) +}) + +describe('probe lane', () => { + it('runs with the cached login PATH and no shell', async () => { + // The whole point: the user's real PATH without paying for -- or being + // blocked by -- a login shell on every call (#14288, #9768). + seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) + await runWslProcess({ lane: 'probe', program: 'codex', args: ['--version'] }) + expect(lastArgv()).toEqual([ + '--exec', + '/usr/bin/env', + 'PATH=/home/u/.nvm/bin:/usr/bin', + 'HOME=/home/u', + 'codex', + '--version' + ]) + }) + + it('falls back to the interactive lane when the distro cannot be probed', async () => { + // "We could not ask" must not become "run with no PATH" -- that would turn + // an unknown into a wrong answer. + runProcessMock.mockResolvedValue({ + code: 1, + signal: null, + stdout: '', + stderr: 'distro is stopped', + timedOut: false + }) + await runWslProcess({ lane: 'probe', program: 'codex' }) + const argv = lastArgv() + expect(argv).toContain('--exec') + expect(argv.slice(-2, -1)).toEqual(['-c']) + expect(argv.at(-1)).toContain('_orca_wsl_shell') + }) +}) + +describe('interactive lane', () => { + it('always fences stdout, even when the caller ignores it', async () => { + // Stock Ubuntu writes its rc hint to stdout, so an unfenced parse reads the + // banner as data (#11327, #11823). A caller that starts parsing later must + // not have to remember to opt in. + runProcessMock.mockImplementation(async (spec: { args: string[] }) => { + const script = spec.args.at(-1) ?? '' + const begin = /__ORCA_WSL_CAPTURE_BEGIN_[a-z0-9]+__/.exec(script)?.[0] ?? '' + const end = /__ORCA_WSL_CAPTURE_END_[a-z0-9]+__/.exec(script)?.[0] ?? '' + return { + code: 0, + signal: null, + stdout: `Ubuntu banner: run a command as administrator\n${begin}payload${end}`, + stderr: '', + timedOut: false + } + }) + const result = await runWslProcess({ lane: 'interactive', program: 'claude' }) + expect(result.stdout).toBe('payload') + }) +}) + +describe('scripts', () => { + it('delivers a script on stdin through sh -s, not in argv', async () => { + // A script on stdin has no quoting boundary for its own quotes to escape + // from. That is what the base64 and eval wrappers were working around + // (#14292), and why this is the only supported way to run one. + seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) + const script = `case "$x" in a) echo 'it'\\''s fine';; esac` + await runWslProcess({ lane: 'probe', script, args: ['/tmp/root'] }) + expect(runProcessMock.mock.calls.at(-1)?.[0].input).toBe(script) + expect(lastArgv()).toEqual([ + '--exec', + '/usr/bin/env', + 'PATH=/home/u/.nvm/bin:/usr/bin', + 'HOME=/home/u', + 'sh', + '-s', + '--', + '/tmp/root' + ]) + expect(lastArgv().join(' ')).not.toContain('case') + }) +}) + +describe('WSLENV', () => { + it('adds every propagated key so the value actually crosses the boundary', async () => { + // Unset, a Windows-side variable silently never reaches the guest (#12557). + seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) + await runWslProcess({ + lane: 'probe', + program: '/bin/true', + env: { GITLAB_HOST: 'git.example.com', GH_TOKEN: 't' } + }) + const env = runProcessMock.mock.calls.at(-1)?.[0].env as NodeJS.ProcessEnv + expect(env.GITLAB_HOST).toBe('git.example.com') + expect(env.WSLENV?.split(':')).toEqual(expect.arrayContaining(['GITLAB_HOST', 'GH_TOKEN'])) + }) + + it('leaves the host environment alone when nothing is propagated', async () => { + seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) + await runWslProcess({ lane: 'probe', program: '/bin/true' }) + expect(runProcessMock.mock.calls.at(-1)?.[0].env).toBeUndefined() + }) +}) + +describe('guest cwd', () => { + it('cds inside the guest rather than passing a Windows cwd to wsl.exe', async () => { + seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) + await runWslProcess({ lane: 'probe', program: '/usr/bin/git', cwd: '/home/u/repo' }) + expect(runProcessMock.mock.calls.at(-1)?.[0].cwd).toBeUndefined() + expect(lastArgv()).toContain('/home/u/repo') + expect(lastArgv()).toContain('sh') + }) + + it.each([['C:\\repo'], ['relative/path']])('rejects %s as a guest cwd', async (cwd) => { + // A Windows path here means a mistake further up; converting it silently + // hides that. + seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) + await expect(runWslProcess({ lane: 'probe', program: '/bin/true', cwd })).rejects.toThrow( + /guest path/ + ) + }) +}) + +describe('program is a binary, not a shell string', () => { + it.each([['sh -c echo hi'], ['a; b'], ['a | b'], ['a && b'], ['echo $HOME'], ['a > b']])( + 'rejects %s', + async (program) => { + await expect(runWslProcess({ lane: 'probe', program })).rejects.toThrow(/single binary/) + } + ) + + it('allows a guest path containing a space', async () => { + // --exec passes the program as one argv element, so a space is harmless; + // rejecting it would fail legitimate installs under a spaced directory. + seedWslGuestEnvironmentForTests(undefined, ENVIRONMENT) + await expect( + runWslProcess({ lane: 'probe', program: '/home/u/my tools/codex' }) + ).resolves.toBeDefined() + }) +}) + +describe('a script never rides the interactive lane', () => { + it('runs sh -s even when the distro cannot be probed', async () => { + // The login shell owns stdin, and the script arrives on stdin. If the shell + // consumed it, `sh -s` would read EOF, run nothing and exit 0 -- a silent + // wrong answer, worse than the degraded PATH this avoids. + runProcessMock.mockResolvedValue({ + code: 1, + signal: null, + stdout: '', + stderr: 'distro is stopped', + timedOut: false + }) + await runWslProcess({ lane: 'probe', script: 'echo hi' }) + expect(lastArgv()).toEqual(['--exec', 'sh', '-s', '--']) + expect(runProcessMock.mock.calls.at(-1)?.[0].input).toBe('echo hi') + }) +}) diff --git a/src/main/wsl/wsl-runner.ts b/src/main/wsl/wsl-runner.ts new file mode 100644 index 00000000000..f1df1d76391 --- /dev/null +++ b/src/main/wsl/wsl-runner.ts @@ -0,0 +1,205 @@ +import { addWslEnvKeys } from '../../shared/wsl-env' +import { runProcess } from '../../shared/child-process/run-process' +import { + buildWslCapturedLoginShellCommand, + buildWslExecArgs, + quotePosixShell +} from '../../shared/wsl-login-shell-command' +import { getWslGuestEnvironment, type WslGuestEnvironment } from './wsl-guest-environment' +import { resolveWslExecutablePath } from './wsl-executable-path' + +/** + * The single place Orca runs a program inside WSL. + * + * Five decisions have to be made on every `wsl.exe` call, each has a right + * answer, and each has shipped wrong: + * + * - **Separator.** `--` makes wsl.exe expand `$name` in every forwarded + * argument before the guest runs, even with no shell in the command, so + * `awk '{print $2}'` loses its field reference (#12964). Always `--exec`. + * - **Shell.** A login shell on a probe path sources `~/.profile`, so one + * blocking line eats the whole timeout (#14288) and every call pays startup + * (#9768). No login shell on a user-facing path means PATH does not match the + * user's terminal, so nvm-installed agents read as absent (#9725, #7563, + * #8366). Hence two lanes, chosen explicitly. + * - **Fencing.** An interactive login shell runs the distro rc, and stock + * Ubuntu writes its "run as administrator" hint to *stdout*, so anything + * parsing that stream reads the banner as data (#11327, #11823). + * - **WSLENV.** Unset, a Windows-side variable silently never crosses into the + * guest (#12557). + * - **Payload.** Scripts go in on stdin. A script on stdin has no quoting + * boundary to escape from, which is what the base64 and `eval` wrappers were + * working around (#14292). + */ + +export type WslLane = + /** No shell. Cached login PATH/HOME, applied via `env`. */ + | 'probe' + /** Login shell, always fenced. For anything whose PATH must match the user's terminal. */ + | 'interactive' + +/** + * What to run: a single binary, or a script. + * + * Why a union rather than an optional `script`: a script has to arrive on + * stdin, which means the program must be a shell reading stdin. Left to the + * caller that is a footgun — and the wrappers this replaces exist because it + * was never made explicit. `script` makes the runner supply `sh -s` itself. + */ +export type WslCommand = + | { program: string; args?: readonly string[]; script?: never } + | { script: string; args?: readonly string[]; program?: never } + +export type WslSpec = WslCommand & { + /** Undefined selects the distro's default. */ + distro?: string + /** + * Required, with no default. The wrong lane is the most common WSL defect in + * this tree, and a default lets a call site pick it by omission. + */ + lane: WslLane + /** Guest (POSIX) path. */ + cwd?: string + /** Host variables to propagate into the guest; sets WSLENV automatically. */ + env?: Readonly> + timeoutMs?: number + maxOutputBytes?: number + signal?: AbortSignal +} + +export type WslResult = { + code: number | null + /** Payload only — on the interactive lane the rc banner is removed by the fence. */ + stdout: string + stderr: string + timedOut: boolean +} + +export const DEFAULT_WSL_TIMEOUT_MS = 30_000 + +function assertGuestPath(cwd: string): void { + // Why reject rather than convert: a caller passing a Windows path here has + // usually made a different mistake further up, and silently translating it + // hides that. + if (!cwd.startsWith('/')) { + throw new Error(`WSL cwd must be a guest path, received ${cwd}`) + } +} + +function assertNotShellString(program: string): void { + // Why: the base64 and eval wrappers exist because this boundary was never + // enforced. `script` is the supported way to run a script. + // + // Why metacharacters and not whitespace: a guest binary may legitimately live + // under a path containing a space, and --exec passes it as one argv element, + // so a space is harmless. A `;` or `|` means the caller is building a command + // line, which is the thing being prevented. + if (/[;&|<>$`\n\r]/.test(program) || /^\S+\s+-/.test(program)) { + throw new Error(`WSL program must be a single binary, received ${program}`) + } +} + +/** Host env plus the WSLENV entries that let it cross the boundary. */ +function buildHostEnv(env: WslSpec['env']): NodeJS.ProcessEnv | undefined { + if (!env || Object.keys(env).length === 0) { + return undefined + } + const merged: NodeJS.ProcessEnv = { ...process.env, ...env } + addWslEnvKeys(merged, Object.keys(env)) + return merged +} + +/** + * `cd` into the guest cwd before the program. + * + * Why not `runProcess`'s `cwd`: that is a *Windows* directory for `wsl.exe`, + * not a guest one. Passing a guest path there fails, and passing the UNC form + * makes wsl.exe start in a network location. + */ +function withGuestCwd(cwd: string | undefined, argv: readonly string[]): string[] { + if (!cwd) { + return [...argv] + } + assertGuestPath(cwd) + return ['sh', '-c', 'cd "$1" || exit 1; shift; exec "$@"', 'orca-wsl', cwd, ...argv] +} + +/** `sh -s --` for a script on stdin, otherwise the program itself. */ +function guestCommandArgv(spec: WslSpec): string[] { + return spec.script !== undefined + ? ['sh', '-s', '--', ...(spec.args ?? [])] + : [spec.program, ...(spec.args ?? [])] +} + +/** Shell-free argv, with the cached environment applied when one is available. */ +function buildGuestArgv(environment: WslGuestEnvironment | null, spec: WslSpec): string[] { + const command = guestCommandArgv(spec) + const argv = environment + ? [environment.envBinary, `PATH=${environment.path}`, `HOME=${environment.home}`, ...command] + : command + return withGuestCwd(spec.cwd, argv) +} + +function buildInteractiveArgv(spec: WslSpec): { + argv: string[] + readStdout: (stdout: string) => string +} { + // Why the whole invocation goes through the fence: a caller that does not + // parse stdout today may start tomorrow, and the banner is invisible until + // then. + const quoted = guestCommandArgv(spec).map(quotePosixShell).join(' ') + const body = spec.cwd ? `cd ${quotePosixShell(spec.cwd)} || exit 1\n${quoted}` : quoted + const captured = buildWslCapturedLoginShellCommand(body) + return { + argv: ['sh', '-c', captured.command], + readStdout: (stdout: string) => captured.readStdout(stdout) ?? '' + } +} + +/** + * Run a program inside WSL. + * + * Falls back from the probe lane to the interactive lane when the distro's + * environment cannot be probed — an unprobed distro is "we could not ask", and + * running with no PATH at all would turn that into a wrong answer. + */ +export async function runWslProcess(spec: WslSpec): Promise { + if (spec.program !== undefined) { + assertNotShellString(spec.program) + } + if (spec.cwd) { + assertGuestPath(spec.cwd) + } + + const environment = spec.lane === 'probe' ? await getWslGuestEnvironment(spec.distro) : null + // Why a script never takes the interactive lane: the login shell owns stdin, + // and the script is delivered on stdin. If the shell consumes it first, the + // inner `sh -s` reads EOF, runs nothing, and exits 0 -- a silent wrong answer, + // which is strictly worse than the degraded PATH avoided by taking this path. + // A script therefore always runs as `--exec sh -s --`, with the cached + // environment applied when there is one. + const lane = + environment === null && spec.script === undefined + ? // Why fall back rather than run with no PATH: an unprobed distro is + // "we could not ask", and answering with an empty environment turns + // that into a wrong answer. + ({ kind: 'interactive', ...buildInteractiveArgv(spec) } as const) + : ({ kind: 'probe', argv: buildGuestArgv(environment, spec) } as const) + + const result = await runProcess({ + program: resolveWslExecutablePath(), + args: buildWslExecArgs(spec.distro, lane.argv), + env: buildHostEnv(spec.env), + input: spec.script, + timeoutMs: spec.timeoutMs ?? DEFAULT_WSL_TIMEOUT_MS, + maxOutputBytes: spec.maxOutputBytes, + signal: spec.signal + }) + + return { + code: result.code, + stdout: lane.kind === 'interactive' ? lane.readStdout(result.stdout) : result.stdout, + stderr: result.stderr, + timedOut: result.timedOut + } +} diff --git a/src/main/wsl/wsl-runner.wsl.test.ts b/src/main/wsl/wsl-runner.wsl.test.ts new file mode 100644 index 00000000000..d1f89a23927 --- /dev/null +++ b/src/main/wsl/wsl-runner.wsl.test.ts @@ -0,0 +1,125 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { runWslProcess } from './wsl-runner' +import { invalidateWslGuestEnvironment } from './wsl-guest-environment' +import { runProcess } from '../../shared/child-process/run-process' +import { resolveWslExecutablePath } from './wsl-executable-path' + +/** + * The assertions no unit test can make: a real distro, a real login shell, a + * real rc banner. + * + * Gated behind an env var and win32 because it mutates the distro's `~/.profile` + * to reproduce #14288. Run with: + * ORCA_REAL_WSL_RUNNER_TEST=1 pnpm vitest run src/main/wsl/wsl-runner.wsl.test.ts + */ +const DISTRO = process.env.ORCA_WSL_TEST_DISTRO ?? 'Ubuntu-24.04' +const enabled = process.platform === 'win32' && process.env.ORCA_REAL_WSL_RUNNER_TEST === '1' +const describeOnWsl = enabled ? describe : describe.skip + +const PROFILE = '/tmp/orca-wsl-runner-profile.bak' + +async function guest(script: string): Promise { + const result = await runProcess({ + program: resolveWslExecutablePath(), + args: ['-d', DISTRO, '--exec', 'sh', '-c', script], + timeoutMs: 30_000 + }) + return result.stdout.trim() +} + +describeOnWsl('runWslProcess against a real distro', () => { + beforeAll(async () => { + // Save whatever profile exists, then make it block for far longer than the + // probe budget -- this is #14288 reproduced, not simulated. + await guest(`cp -f "$HOME/.profile" ${PROFILE} 2>/dev/null || true`) + await guest(`printf '\\nsleep 60\\n' >> "$HOME/.profile"`) + invalidateWslGuestEnvironment() + }, 120_000) + + afterAll(async () => { + await guest(`cp -f ${PROFILE} "$HOME/.profile" 2>/dev/null || rm -f "$HOME/.profile"`) + await guest(`rm -f ${PROFILE}`) + invalidateWslGuestEnvironment() + }, 120_000) + + it('probe lane survives a ~/.profile that blocks for a minute', async () => { + // The first call pays one login shell (which the blocking profile stalls, + // so it times out and the lane degrades); every later call must not. + // What must never happen is the probe lane inheriting the stall per call. + const started = Date.now() + const result = await runWslProcess({ + lane: 'probe', + distro: DISTRO, + program: '/bin/echo', + args: ['orca-probe-ok'], + timeoutMs: 15_000 + }) + const elapsed = Date.now() - started + expect(result.stdout.trim()).toContain('orca-probe-ok') + expect(elapsed).toBeLessThan(20_000) + }, 60_000) + + it('second probe-lane call does not pay the login shell again', async () => { + const started = Date.now() + await runWslProcess({ lane: 'probe', distro: DISTRO, program: '/bin/true', timeoutMs: 15_000 }) + expect(Date.now() - started).toBeLessThan(5_000) + }, 30_000) + + it('interactive lane strips the distro banner from parsed stdout', async () => { + // Stock Ubuntu writes its rc hint to stdout. Anything parsing that stream + // reads the banner as data unless the fence removes it (#11327, #11823). + const result = await runWslProcess({ + lane: 'interactive', + distro: DISTRO, + program: '/bin/echo', + args: ['ORCA_PAYLOAD'], + timeoutMs: 60_000 + }) + expect(result.stdout.trim()).toBe('ORCA_PAYLOAD') + }, 90_000) + + it('a script with quotes and $ arrives byte-identical', async () => { + // Both hazards in one payload: `--` would expand $2 host-side, and the + // base64/eval wrappers broke on the embedded quotes (#12964, #14292). + const script = [ + `printf '%s\\n' "$1"`, + `echo 'it'\\''s fine'`, + `echo "x" | awk '{print $1}'` + ].join('\n') + const result = await runWslProcess({ + lane: 'probe', + distro: DISTRO, + script, + args: ['ORCA_ARG'], + timeoutMs: 30_000 + }) + expect( + result.stdout + .split('\n') + .map((l) => l.trim()) + .filter(Boolean) + ).toEqual(['ORCA_ARG', "it's fine", 'x']) + }, 60_000) + + it('propagated env crosses the boundary via WSLENV', async () => { + const result = await runWslProcess({ + lane: 'probe', + distro: DISTRO, + script: 'printf %s "$ORCA_WSLENV_PROBE"', + env: { ORCA_WSLENV_PROBE: 'crossed' }, + timeoutMs: 30_000 + }) + expect(result.stdout.trim()).toBe('crossed') + }, 60_000) + + it('runs in the requested guest cwd', async () => { + const result = await runWslProcess({ + lane: 'probe', + distro: DISTRO, + program: '/bin/pwd', + cwd: '/tmp', + timeoutMs: 30_000 + }) + expect(result.stdout.trim()).toBe('/tmp') + }, 60_000) +})