diff --git a/scripts/command-smoke.sh b/scripts/command-smoke.sh index 0cd644f9..d0ba5603 100644 --- a/scripts/command-smoke.sh +++ b/scripts/command-smoke.sh @@ -22,7 +22,8 @@ # Usage: [WORKOS_API_KEY=sk_...] sh command-smoke.sh /path/to/workos set -u -BIN="$1" +# Keep the binary address valid when a check changes working directory. +BIN="$(cd "$(dirname "$1")" && pwd)/$(basename "$1")" fails=0 pass() { echo " ok: $1"; } @@ -33,9 +34,9 @@ fail() { # Sandbox the config/home so host auth state can never leak in; the Windows # binary reads USERPROFILE, which needs a Windows-style path under Git Bash. -SANDBOX=$(mktemp -d) +SANDBOX=$(mktemp -d) || exit 1 if command -v cygpath >/dev/null 2>&1; then - USERPROFILE=$(cygpath -w "$SANDBOX") + USERPROFILE=$(cygpath -w "$SANDBOX") || exit 1 else USERPROFILE="$SANDBOX" fi @@ -110,6 +111,52 @@ case "$err" in esac if [ "$code" -eq 1 ] && [ "$json_ok" -eq 1 ]; then pass "unknown command exits 1 with structured error"; else fail "unknown command contract (exit $code, want 1): $err"; fi +# Doctor must use installed tools, not shims planted in its project directory. +# This runs against the shipped Bun binary on native Windows release runners, +# where CWD-first lookup is implicit. Do not emulate it with "." in POSIX PATH: +# Bun 1.3.x resolves relative PATH entries before applying the child's cwd. +probe_project="$SANDBOX/untrusted project" +probe_tools="$SANDBOX/installed tools" +probe_marker="$SANDBOX/planted-ran" +mkdir -p "$probe_project" "$probe_tools" "$SANDBOX/.claude" "$SANDBOX/.codex" +printf '%s\n' '{"name":"probe-project","private":true}' >"$probe_project/package.json" +printf '%s\n' '{}' >"$probe_project/package-lock.json" +if command -v cygpath >/dev/null 2>&1; then + probe_path="$probe_tools:$PATH" + WORKOS_EXEC_MARKER=$(cygpath -w "$probe_marker") || exit 1 + for tool in node npm claude codex; do + printf '@echo off\r\necho v98.76.54\r\n' >"$probe_tools/$tool.cmd" + printf '@echo off\r\necho planted> "%%WORKOS_EXEC_MARKER%%"\r\necho v0.0.0\r\n' >"$probe_project/$tool.bat" + done +else + probe_path="$probe_tools:$PATH" + WORKOS_EXEC_MARKER="$probe_marker" + for tool in node npm claude codex; do + printf '#!/bin/sh\necho v98.76.54\n' >"$probe_tools/$tool" + printf '#!/bin/sh\necho planted > "$WORKOS_EXEC_MARKER"\necho v0.0.0\n' >"$probe_project/$tool" + chmod +x "$probe_tools/$tool" "$probe_project/$tool" + done +fi +export WORKOS_EXEC_MARKER +out=$(cd "$probe_project" && PATH="$probe_path" "$BIN" doctor --skip-api --skip-ai --json 2>"$SANDBOX/doctor-stderr") +code=$? +# An otherwise-empty project may correctly produce diagnostic errors, exit 1. +case "$out" in + *'"nodeVersion": "v98.76.54"'*'"packageManagerVersion": "v98.76.54"'*) json_ok=1 ;; + *) json_ok=0 ;; +esac +if [ "$code" -le 1 ] && [ "$json_ok" -eq 1 ] && [ ! -e "$probe_marker" ]; then + pass "doctor ignores repo-local binaries and runs installed tool shims" +else + fail "doctor tool isolation (exit $code): $out $(cat "$SANDBOX/doctor-stderr")" +fi +# Prove the MCP availability probes actually ran, rather than passing because +# the fake clients were not detected in this platform's home directory. +case "$out" in + *'"agent": "Claude Code"'*'"agent": "Codex"'*) pass "doctor probes both installed MCP clients" ;; + *) fail "doctor did not probe both MCP clients" ;; +esac + # ---- Authenticated commands (opt-in via WORKOS_API_KEY) ---- if [ -n "$SMOKE_API_KEY" ]; then # On failure, surface the CLI's structured stderr error — it never diff --git a/src/utils/exec-file.spec.ts b/src/utils/exec-file.spec.ts new file mode 100644 index 00000000..6105c891 --- /dev/null +++ b/src/utils/exec-file.spec.ts @@ -0,0 +1,133 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { access, mkdtemp, mkdir, readFile, realpath, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { delimiter, join } from 'node:path'; +import { execFileNoThrow } from './exec-file.js'; +import { IS_WINDOWS } from './platform.js'; + +let root: string; +let project: string; +let tools: string; +let originalCwd: string; +let env: NodeJS.ProcessEnv; + +async function tool(directory: string, name: string, body: string): Promise { + await writeFile( + join(directory, `${name}${IS_WINDOWS ? '.cmd' : ''}`), + `${IS_WINDOWS ? '@echo off\r\n' : '#!/bin/sh\n'}${body}\n`, + { mode: 0o755 }, + ); +} + +beforeEach(async () => { + originalCwd = process.cwd(); + root = await mkdtemp(join(tmpdir(), 'exec-file-test-')); + project = join(root, 'untrusted project'); + tools = join(root, 'installed tools'); + await mkdir(project); + await mkdir(tools); + // Windows searches CWD implicitly. On POSIX, a relative PATH entry gives + // us the same regression signal without mocking spawn or the platform. + env = { + ...Object.fromEntries(Object.entries(process.env).filter(([key]) => key.toLowerCase() !== 'path')), + PATH: `${IS_WINDOWS ? '' : `.${delimiter}`}${tools}`, + PATHEXT: '.COM;.EXE;.BAT;.CMD', + EXEC_TEST_MARKER: join(root, 'planted-ran'), + }; + process.chdir(project); +}); + +afterEach(async () => { + vi.unstubAllEnvs(); + process.chdir(originalCwd); + await rm(root, { recursive: true, force: true }); +}); + +describe('execFileNoThrow', () => { + it.each(['node', 'npm', 'bun', 'claude', 'codex'])('does not execute a repo-local %s shim', async (name) => { + await tool(tools, name, 'echo trusted'); + await tool( + project, + name, + IS_WINDOWS + ? 'echo planted> "%EXEC_TEST_MARKER%"\r\necho untrusted' + : 'echo planted > "$EXEC_TEST_MARKER"\necho untrusted', + ); + + const result = await execFileNoThrow(name, ['--version'], { env }); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(''); + expect(result.stdout.trim()).toBe('trusted'); + await expect(access(env.EXEC_TEST_MARKER!)).rejects.toMatchObject({ code: 'ENOENT' }); + expect(process.cwd()).toBe(await realpath(project)); + }); + + it('uses a fresh directory per call and removes it after the process closes', async () => { + await tool(tools, 'probe', IS_WINDOWS ? 'cd' : 'pwd'); + const first = await execFileNoThrow('probe', [], { env }); + const second = await execFileNoThrow('probe', [], { env }); + + expect(first.status).toBe(0); + expect(second.status).toBe(0); + expect(first.stdout.trim()).not.toBe(second.stdout.trim()); + for (const result of [first, second]) { + await expect(access(result.stdout.trim())).rejects.toMatchObject({ code: 'ENOENT' }); + } + }); + + it('preserves an explicit working directory for trusted project commands', async () => { + await writeFile(join(project, 'input.txt'), 'project input\n'); + await tool( + tools, + 'probe', + IS_WINDOWS ? 'type input.txt' : 'while IFS= read -r line; do echo "$line"; done < input.txt', + ); + + const result = await execFileNoThrow('probe', [], { cwd: project, env }); + + expect(result.status).toBe(0); + expect(result.stdout.trim()).toBe('project input'); + expect(await readFile(join(project, 'input.txt'), 'utf8')).toBe('project input\n'); + }); + + it('captures nonzero exits and cleans up the working directory', async () => { + await tool(tools, 'probe', IS_WINDOWS ? 'cd\r\necho failure 1>&2\r\nexit /b 7' : 'pwd\necho failure >&2\nexit 7'); + + const result = await execFileNoThrow('probe', [], { env }); + + expect(result.status).toBe(7); + expect(result.stderr.trim()).toBe('failure'); + await expect(access(result.stdout.trim())).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('does not fall back to a repo-local shim when the tool is missing from PATH', async () => { + await tool(project, 'workos-nonexistent-test-tool', 'echo untrusted'); + const result = await execFileNoThrow('workos-nonexistent-test-tool', [], { env }); + expect(result.status).not.toBe(0); + expect(result.stdout).toBe(''); + expect(result.stderr).not.toBe(''); + }); + + it('fails closed when it cannot create the isolated directory', async () => { + await tool(tools, 'probe', 'echo ran'); + for (const key of ['TMPDIR', 'TMP', 'TEMP']) vi.stubEnv(key, join(root, 'missing')); + + const result = await execFileNoThrow('probe', [], { env }); + + expect(result.status).toBe(1); + expect(result.stdout).toBe(''); + expect(result.stderr).toContain('ENOENT'); + }); + + it('cleans up after a timed-out process', async () => { + // Shell builtins only, so there is no surviving grandchild holding pipes. + await tool(tools, 'probe', IS_WINDOWS ? 'cd\r\n:loop\r\ngoto loop' : 'pwd\nwhile :; do :; done'); + + const result = await execFileNoThrow('probe', [], { env, timeout: 500 }); + + expect(result.status).not.toBe(0); + expect(result.stdout.trim()).not.toBe(''); + await expect(access(result.stdout.trim())).rejects.toMatchObject({ code: 'ENOENT' }); + }); +}); diff --git a/src/utils/exec-file.ts b/src/utils/exec-file.ts index b4d16fb6..195d3a1d 100644 --- a/src/utils/exec-file.ts +++ b/src/utils/exec-file.ts @@ -1,4 +1,7 @@ import { spawn } from 'node:child_process'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { SPAWN_OPTS } from './platform.js'; export interface ExecResult { @@ -8,6 +11,7 @@ export interface ExecResult { } export interface ExecOptions { + /** Explicitly opt into a trusted project's working directory. */ cwd?: string; timeout?: number; env?: NodeJS.ProcessEnv; @@ -15,42 +19,53 @@ export interface ExecOptions { /** * Execute a command without throwing on non-zero exit codes. + * Defaults to a fresh directory so Windows shell lookup cannot execute a + * repo-local shim, and host probes do not load project-local configuration. * Returns { status, stdout, stderr } for all outcomes. */ -export function execFileNoThrow(command: string, args: string[], options: ExecOptions = {}): Promise { - return new Promise((resolve) => { - const child = spawn(command, args, { - cwd: options.cwd, - env: options.env ?? process.env, - timeout: options.timeout, - ...SPAWN_OPTS, - }); - - let stdout = ''; - let stderr = ''; - - child.stdout?.on('data', (data) => { - stdout += data.toString(); - }); - - child.stderr?.on('data', (data) => { - stderr += data.toString(); - }); - - child.on('close', (code) => { - resolve({ - status: code ?? 1, - stdout, - stderr, - }); - }); +export async function execFileNoThrow(command: string, args: string[], options: ExecOptions = {}): Promise { + try { + const isolatedCwd = options.cwd === undefined ? await mkdtemp(join(tmpdir(), 'workos-exec-')) : undefined; + try { + return await new Promise((resolve) => { + const child = spawn(command, args, { + cwd: options.cwd ?? isolatedCwd, + env: options.env ?? process.env, + timeout: options.timeout, + ...SPAWN_OPTS, + }); + + let stdout = ''; + let stderr = ''; + + child.stdout?.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr?.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + resolve({ + status: code ?? 1, + stdout, + stderr, + }); + }); - child.on('error', (err) => { - resolve({ - status: 1, - stdout, - stderr: err.message, + child.on('error', (err) => { + resolve({ + status: 1, + stdout, + stderr: err.message, + }); + }); }); - }); - }); + } finally { + if (isolatedCwd) await rm(isolatedCwd, { recursive: true, force: true, maxRetries: 3 }); + } + } catch (error) { + return { status: 1, stdout: '', stderr: error instanceof Error ? error.message : String(error) }; + } }