diff --git a/CHANGELOG.md b/CHANGELOG.md index 185dcb0301..3905b928f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,18 @@ `-dev` prerelease marker so the version on `main` never equals a published version (registry scanners diff the tool surface per version string, and a moving surface under a released number reads as a republish). `release:prepare` refuses to publish while the `-dev` marker is in place. +- Windows `--platform web` works again. `agent-device web setup` no longer fails with + `npm not found in PATH`, and every web command — including `web doctor` — no longer fails with + `spawn EINVAL`. The managed `agent-browser` backend is now launched as `node ` on every + platform instead of through its `node_modules/.bin` console shim, which is a `.cmd` file on + Windows that `child_process.spawn` refuses without a shell (CVE-2024-27980 hardening); + `shell: true` would only trade that for argument-quoting hazards and a `DEP0190` warning on every + command. Setup spawns `npm` from PATH unchanged on macOS and Linux, and runs npm's own + `npm-cli.js` under the current Node only on Windows, where a bare `npm` is not spawnable. A + managed install now counts as present only when the backend package itself is, and + `web setup --json` / `web doctor --json` gain `entryScript` and `packageDir`; the published + `binaryPath` is unchanged and still names npm's console shim, now informational rather than the + spawned command (#2022). - Parameterized `fill --record-as` protection is now recording-session-scoped instead of fill-step-scoped (ADR 0017 amendment): a later, unrelated recorded action (`wait`, `is`, `get`) can no longer re-serialize an app-rendered echo of an already-parameterized value into its own result or diff --git a/docs/agents/web-backend.md b/docs/agents/web-backend.md index 657f1878ca..f8efb009db 100644 --- a/docs/agents/web-backend.md +++ b/docs/agents/web-backend.md @@ -8,6 +8,12 @@ Web automation uses a managed `agent-browser` backend as an implementation detai - Use `agent-device web doctor` to run the backend health check. - The managed install respects `--state-dir` / `AGENT_DEVICE_STATE_DIR`. - Web automation requires Node 24+ while the rest of agent-device keeps its Node 22 baseline. +- Every backend call spawns the package's declared `bin` entry with the current Node runtime, + never the `node_modules/.bin` console shim: Windows ships that shim as `.cmd`, which + `child_process.spawn` refuses without a shell (CVE-2024-27980 hardening), and a shell would + reintroduce argument-quoting hazards. `runManagedAgentBrowser` is the only path that executes + the backend. Setup spawns `npm` from PATH as before, except on Windows, where a bare `npm` is + not spawnable and npm's own `npm-cli.js` runs under the current Node instead. Default first-run flow: diff --git a/src/__tests__/cli-web.test.ts b/src/__tests__/cli-web.test.ts new file mode 100644 index 0000000000..9f74552a03 --- /dev/null +++ b/src/__tests__/cli-web.test.ts @@ -0,0 +1,108 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import { test } from 'vitest'; +import { runCliCapture } from './cli-capture.ts'; +import { mkdtempForTestSync } from './test-utils/tmp-dir.ts'; +import { withCommandExecutorOverride } from '../utils/exec.ts'; +import { + installFakeManagedAgentBrowser, + withNodeRuntime, + writeFakeManagedAgentBrowserPackage, +} from '../platforms/web/__tests__/test-utils.ts'; + +type SpawnedCommand = { cmd: string; args: string[] }; + +// `binaryPath` has been in the published `web setup`/`web doctor` JSON since #833. +// The Windows spawn fix (#2022) moves execution to `node ` and adds +// `entryScript`/`packageDir`, but the released field stays in the contract. +test('web doctor --json keeps the published status fields and spawns the JS entry', async () => { + const stateDir = mkdtempForTestSync('agent device cli web doctor '); + try { + const install = installFakeManagedAgentBrowser(stateDir); + const spawned: SpawnedCommand[] = []; + + const result = await withCommandExecutorOverride( + async (cmd, args) => { + spawned.push({ cmd, args }); + return { stdout: 'ok', stderr: '', exitCode: 0 }; + }, + async () => + await runCliCapture(['web', 'doctor', '--json'], { + env: { AGENT_DEVICE_STATE_DIR: stateDir }, + }), + ); + + const status = parseStatus(result.stdout); + assert.equal(status.binaryPath, install.binaryPath); + assert.equal(status.entryScript, install.entryScript); + assert.equal(status.packageDir, install.packageDir); + assert.equal(status.installDir, install.installDir); + assert.equal(status.installed, true); + assert.equal(status.socketDir, undefined); + assert.deepEqual(spawned, [ + { cmd: process.execPath, args: [install.entryScript, 'doctor', '--offline', '--quick'] }, + ]); + assert.equal(result.calls.length, 0); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +test('web setup --json keeps the published status fields after installing', async () => { + const stateDir = mkdtempForTestSync('agent device cli web setup '); + try { + let install: ReturnType | undefined; + let stdout = ''; + + await withNodeRuntime({ version: '24.13.0' }, async () => { + const result = await withCommandExecutorOverride( + async (_cmd, args) => { + // Stand in for the npm run that writes the managed package tree. + if (args.includes('--prefix')) install = writeFakeManagedAgentBrowserPackage(stateDir); + return { stdout: '', stderr: '', exitCode: 0 }; + }, + async () => + await runCliCapture(['web', 'setup', '--json'], { + env: { AGENT_DEVICE_STATE_DIR: stateDir }, + }), + ); + stdout = result.stdout; + }); + + const status = parseStatus(stdout); + assert.equal(status.binaryPath, install?.binaryPath); + assert.equal(status.entryScript, install?.entryScript); + assert.equal(status.packageDir, install?.packageDir); + assert.equal(status.installed, true); + assert.equal(status.socketDir, undefined); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +function parseStatus(stdout: string): Record { + const payload: unknown = JSON.parse(firstJsonDocument(stdout)); + assert.ok(isRecord(payload) && payload.success === true, stdout); + const data = payload.data; + assert.ok(isRecord(data), stdout); + const status = data.status; + assert.ok(isRecord(status), stdout); + return status; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} + +/** + * The CLI dispatches `web` inside its top-level try, and the capture harness + * turns `process.exit` into a throw, so the command's payload is followed by the + * CLI's own report of that synthetic exit. Only the first document is the + * command's own output; real runs exit for real and print it once. + */ +function firstJsonDocument(stdout: string): string { + const lines = stdout.split('\n'); + const end = lines.indexOf('}'); + assert.ok(end >= 0, stdout); + return lines.slice(0, end + 1).join('\n'); +} diff --git a/src/daemon/handlers/__tests__/session-teardown-resources.test.ts b/src/daemon/handlers/__tests__/session-teardown-resources.test.ts index d8f21d1b6e..c23b334d03 100644 --- a/src/daemon/handlers/__tests__/session-teardown-resources.test.ts +++ b/src/daemon/handlers/__tests__/session-teardown-resources.test.ts @@ -351,8 +351,10 @@ test('daemon session teardown closes an open web session immediately, not on age // fleet right away, the same way an explicit `session close` does, instead of leaving the // Chrome processes to agent-browser's own multi-minute idle timer. expect(mockRunCmd).toHaveBeenCalledTimes(1); - const [, args] = mockRunCmd.mock.calls[0] as [string, string[]]; - expect(args).toEqual(['close', '--json', '--session', sessionName]); + const [cmd, args] = mockRunCmd.mock.calls[0] as [string, string[]]; + // `node close ...`: the managed backend never runs through its `.bin` shim (#2022). + expect(cmd).toBe(process.execPath); + expect(args.slice(1)).toEqual(['close', '--json', '--session', sessionName]); }); test('daemon session teardown surfaces a web close failure through the cleanup-failure channel', async () => { diff --git a/src/daemon/server/daemon-runtime-web-close-teardown.test.ts b/src/daemon/server/daemon-runtime-web-close-teardown.test.ts index a12e81c39e..84f390a100 100644 --- a/src/daemon/server/daemon-runtime-web-close-teardown.test.ts +++ b/src/daemon/server/daemon-runtime-web-close-teardown.test.ts @@ -129,7 +129,14 @@ test('daemon shutdown closes an open web session immediately, without waiting fo expect(stderrChunks.join('')).toBe(''); expect(sessionStore.get(session.name)).toBeUndefined(); const closeCall = mockRunCmd.mock.calls.find(([, args]) => (args as string[]).includes('close')); - expect(closeCall?.[1]).toEqual(['close', '--json', '--session', session.name]); + // `node close ...`: the managed backend never runs through its `.bin` shim (#2022). + expect(closeCall?.[0]).toBe(process.execPath); + expect((closeCall?.[1] as string[] | undefined)?.slice(1)).toEqual([ + 'close', + '--json', + '--session', + session.name, + ]); }); test('daemon shutdown reports a web close failure on stderr instead of losing it silently', async () => { diff --git a/src/platforms/web/__tests__/test-utils.ts b/src/platforms/web/__tests__/test-utils.ts index e4641d1d68..d5bf887119 100644 --- a/src/platforms/web/__tests__/test-utils.ts +++ b/src/platforms/web/__tests__/test-utils.ts @@ -8,19 +8,73 @@ const TEST_AGENT_BROWSER_VERSION = '0.27.1'; type FakeManagedAgentBrowserInstall = ReturnType; export function installFakeManagedAgentBrowser(stateDir: string): FakeManagedAgentBrowserInstall { + const install = writeFakeManagedAgentBrowserPackage(stateDir); + fs.writeFileSync(path.join(install.installDir, 'manifest.json'), '{}'); + return install; +} + +/** The package tree `npm install` leaves behind, without the setup manifest. */ +export function writeFakeManagedAgentBrowserPackage( + stateDir: string, +): FakeManagedAgentBrowserInstall { const install = expectedManagedAgentBrowserInstall(stateDir); + fs.mkdirSync(path.dirname(install.entryScript), { recursive: true }); + fs.writeFileSync(install.entryScript, 'process.exit(0)\n'); + // npm links a console shim beside the package; the fixture carries it so tests + // prove the shim is never spawned rather than merely absent (#2022). fs.mkdirSync(path.dirname(install.binaryPath), { recursive: true }); fs.writeFileSync(install.binaryPath, '#!/bin/sh\nexit 0\n'); fs.chmodSync(install.binaryPath, 0o755); - fs.writeFileSync(path.join(install.installDir, 'manifest.json'), '{}'); + fs.writeFileSync( + path.join(install.packageDir, 'package.json'), + JSON.stringify({ + name: 'agent-browser', + version: TEST_AGENT_BROWSER_VERSION, + bin: { 'agent-browser': './dist/cli.js' }, + }), + ); return install; } +/** npm's own JS launcher, for tests that drive managed setup without a real npm. */ +export function writeFakeNpmCliScript(root: string): string { + const npmCliScript = path.join(root, 'node runtime', 'node_modules', 'npm', 'bin', 'npm-cli.js'); + fs.mkdirSync(path.dirname(npmCliScript), { recursive: true }); + fs.writeFileSync(npmCliScript, 'process.exit(0)\n'); + return npmCliScript; +} + +/** Reports a different Node runtime to the code under test for one scenario. */ +export async function withNodeRuntime( + overrides: { version?: string; platform?: NodeJS.Platform; execPath?: string }, + testFn: () => void | Promise, +): Promise { + const restore: (() => void)[] = []; + const override = (target: object, key: string, value: unknown) => { + const original = (target as Record)[key]; + Object.defineProperty(target, key, { value, configurable: true }); + restore.push(() => Object.defineProperty(target, key, { value: original, configurable: true })); + }; + if (overrides.version !== undefined) { + override(process.versions, 'node', overrides.version); + override(process, 'version', `v${overrides.version}`); + } + if (overrides.platform !== undefined) override(process, 'platform', overrides.platform); + if (overrides.execPath !== undefined) override(process, 'execPath', overrides.execPath); + try { + await testFn(); + } finally { + for (const undo of restore.reverse()) undo(); + } +} + function expectedManagedAgentBrowserInstall(stateDir: string) { const installDir = path.join(stateDir, 'tools', 'agent-browser', TEST_AGENT_BROWSER_VERSION); + const packageDir = path.join(installDir, 'package', 'node_modules', 'agent-browser'); return { version: TEST_AGENT_BROWSER_VERSION, installDir, + packageDir, binaryPath: path.join( installDir, 'package', @@ -28,6 +82,7 @@ function expectedManagedAgentBrowserInstall(stateDir: string) { '.bin', process.platform === 'win32' ? 'agent-browser.cmd' : 'agent-browser', ), + entryScript: path.join(packageDir, 'dist', 'cli.js'), homeDir: path.join(installDir, 'home'), runtimeHomeDir: process.platform === 'win32' diff --git a/src/platforms/web/agent-browser-install.ts b/src/platforms/web/agent-browser-install.ts new file mode 100644 index 0000000000..0d41e710bf --- /dev/null +++ b/src/platforms/web/agent-browser-install.ts @@ -0,0 +1,93 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { AppError } from '@agent-device/kernel/errors'; +import { runCmd } from '../../utils/exec.ts'; + +/** + * How the managed backend gets onto disk. Kept apart from the tool module, + * which owns locating and running what this leaves behind. + */ +export async function installManagedAgentBrowserPackage(params: { + packageRoot: string; + packageSpec: string; + timeoutMs: number; +}): Promise { + fs.mkdirSync(params.packageRoot, { recursive: true }); + // `--no-global` keeps an ambient `npm_config_global` from redirecting the + // install out of the managed prefix, where the backend entry would be missed. + const npm = npmCommand([ + 'install', + '--prefix', + params.packageRoot, + '--no-global', + '--no-audit', + '--no-fund', + '--no-save', + params.packageSpec, + ]); + await runCmd(npm.command, npm.args, { env: process.env, timeoutMs: params.timeoutMs }); +} + +export function writeManagedAgentBrowserManifest(params: { + installDir: string; + packageName: string; + version: string; +}): void { + fs.writeFileSync( + path.join(params.installDir, 'manifest.json'), + JSON.stringify( + { + package: params.packageName, + version: params.version, + node: process.version, + installedAt: new Date().toISOString(), + }, + null, + 2, + ), + 'utf8', + ); +} + +/** + * POSIX spawns `npm` from PATH exactly as it always has. Only Windows is + * broken: npm ships as `npm.cmd` there, which `child_process.spawn` refuses + * without a shell since the CVE-2024-27980 fix, so its JS entry runs under the + * current Node runtime instead (#2022). + */ +function npmCommand(args: string[]): { command: string; args: string[] } { + if (process.platform !== 'win32') return { command: 'npm', args }; + const npmCliScript = resolveWindowsNpmCliScript(process.env); + if (!npmCliScript) { + throw new AppError('TOOL_MISSING', 'npm not found in PATH', { + nodeExecPath: process.execPath, + hint: 'Install Node.js with npm, or add npm to PATH, and run `agent-device web setup` again.', + }); + } + return { command: process.execPath, args: [npmCliScript, ...args] }; +} + +function resolveWindowsNpmCliScript(env: NodeJS.ProcessEnv): string | undefined { + const advertised = env.npm_execpath?.trim(); + if (advertised) { + const scriptPath = path.resolve(advertised); + // pnpm and yarn advertise their own launcher through the same variable. + if (path.basename(scriptPath) === 'npm-cli.js' && isFile(scriptPath)) return scriptPath; + } + const bundled = path.join( + path.dirname(process.execPath), + 'node_modules', + 'npm', + 'bin', + 'npm-cli.js', + ); + return isFile(bundled) ? bundled : undefined; +} + +function isFile(filePath: string): boolean { + try { + return fs.statSync(filePath).isFile(); + } catch { + return false; + } +} diff --git a/src/platforms/web/agent-browser-lifecycle.test.ts b/src/platforms/web/agent-browser-lifecycle.test.ts index ad0ca2bd44..762d822b7e 100644 --- a/src/platforms/web/agent-browser-lifecycle.test.ts +++ b/src/platforms/web/agent-browser-lifecycle.test.ts @@ -283,7 +283,7 @@ test('records the managed browser daemon and Chrome fleet at the spawn owner', a const store = { replace: vi.fn(), clear: vi.fn(), read: vi.fn(() => []) }; mockRunCmd.mockResolvedValue({ stdout: [ - ` 101 1 ${status.binaryPath}`, + ` 101 1 ${process.execPath} ${status.entryScript}`, ` 201 101 /Applications/Chromium.app/Contents/MacOS/Chromium ${marker}`, ' 301 201 /Applications/Chromium.app/Contents/Frameworks/Chromium Helper --type=gpu', ' 999 1 /Applications/Google Chrome.app/Contents/MacOS/Google Chrome', @@ -315,8 +315,10 @@ test('managed process summary includes the recordable agent-browser daemon', () installFakeManagedAgentBrowser(stateDir); const status = getManagedAgentBrowserStatus({ stateDir }); expect( - summarizeManagedAgentBrowserProcesses([{ pid: 101, command: status.binaryPath }], status) - .processes[0]?.reason, + summarizeManagedAgentBrowserProcesses( + [{ pid: 101, command: `${process.execPath} ${status.entryScript}` }], + status, + ).processes[0]?.reason, ).toBe('agent-browser-daemon'); } finally { fs.rmSync(stateDir, { recursive: true, force: true }); diff --git a/src/platforms/web/agent-browser-provider.test.ts b/src/platforms/web/agent-browser-provider.test.ts index a3c165b574..fb79ddc882 100644 --- a/src/platforms/web/agent-browser-provider.test.ts +++ b/src/platforms/web/agent-browser-provider.test.ts @@ -1,6 +1,5 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; -import path from 'node:path'; import { beforeEach, expect, test, vi } from 'vitest'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; @@ -22,7 +21,7 @@ import { withCommandExecutorOverride, type ExecResult } from '../../utils/exec.t import { AppError } from '@agent-device/kernel/errors'; import { buildSelectorChainForNode, resolveRecordedTarget } from '@agent-device/selectors'; import { attachRefs } from '@agent-device/kernel/snapshot'; -import { installFakeManagedAgentBrowser } from './__tests__/test-utils.ts'; +import { installFakeManagedAgentBrowser, withNodeRuntime } from './__tests__/test-utils.ts'; import type { OwnedProcessRecordStore } from '../../utils/owned-process-record.ts'; type AgentBrowserCall = { @@ -104,8 +103,8 @@ test('agent-browser provider runs provider-startup cleanup before the first mana { session: 'web-session', openWebSessionNames: () => [] }, async (provider) => { await withCommandExecutorOverride( - async (cmd, args) => { - events.push(`${path.basename(cmd)} ${args[0] ?? ''}`); + async (_cmd, args) => { + events.push(`agent-browser ${agentBrowserCliArgs(args)[0] ?? ''}`); return jsonResult({ success: true, data: {} }); }, async () => await provider.open('https://example.test'), @@ -291,7 +290,7 @@ test('agent-browser provider dumps session network requests', async () => { await withManagedAgentBrowserProvider({ session: 'web-session' }, async (provider) => { const calls: AgentBrowserCall[] = []; const executor = async (cmd: string, args: string[]): Promise => { - calls.push({ cmd, args }); + recordAgentBrowserCall(calls, cmd, args); return jsonResult({ success: true, data: { @@ -357,7 +356,7 @@ test('agent-browser provider probes page audio through eval', async () => { const calls: AgentBrowserCall[] = []; const audio = await withCommandExecutorOverride( async (cmd, args) => { - calls.push({ cmd, args }); + recordAgentBrowserCall(calls, cmd, args); return jsonResult({ success: true, data: { @@ -422,9 +421,9 @@ test('agent-browser provider generated audio probe script samples streams discov const calls: AgentBrowserCall[] = []; const page = createAudioProbeScriptPage(); const executor = async (cmd: string, args: string[]): Promise => { - calls.push({ cmd, args }); - assert.equal(args[0], 'eval'); - const script = args[1]; + const cliArgs = recordAgentBrowserCall(calls, cmd, args); + assert.equal(cliArgs[0], 'eval'); + const script = cliArgs[1]; if (typeof script !== 'string') throw new Error('Expected generated eval script'); if (calls.length === 2) page.audio.srcObject = page.stream; return jsonResult({ @@ -478,7 +477,7 @@ test('agent-browser provider surfaces stale ref failures during requested snapsh () => withCommandExecutorOverride( async (_cmd, args) => { - if (args[0] === 'snapshot') { + if (agentBrowserCliArgs(args)[0] === 'snapshot') { return jsonResult({ success: true, data: { @@ -502,7 +501,7 @@ test('agent-browser provider surfaces stale ref failures during requested snapsh test('agent-browser provider adds doctor guidance for missing binary and invalid JSON', async () => { // Pin a web-supported Node version so the missing-binary path yields the // setup hint instead of the Node upgrade hint on Node <24 hosts. - await withNodeRuntimeVersion('24.0.0', async () => { + await withNodeRuntime({ version: '24.0.0' }, async () => { const missingStateDir = mkdtempForTestSync('agent-device-web-provider-missing-'); try { const provider = createAgentBrowserWebProvider({ stateDir: missingStateDir }); @@ -536,7 +535,7 @@ test('agent-browser provider adds doctor guidance for missing binary and invalid }); test('agent-browser provider preserves Node version guidance for missing managed backend', async () => { - await withNodeRuntimeVersion('22.19.0', async () => { + await withNodeRuntime({ version: '22.19.0' }, async () => { const missingStateDir = mkdtempForTestSync('agent-device-web-provider-node-'); try { const provider = createAgentBrowserWebProvider({ stateDir: missingStateDir }); @@ -561,56 +560,52 @@ async function withManagedAgentBrowserProvider( openWebSessionNames?: () => readonly string[]; ownedProcessRecords?: OwnedProcessRecordStore; }, - testFn: (provider: ReturnType) => void | Promise, + testFn: ( + provider: ReturnType, + install: ReturnType, + ) => void | Promise, ): Promise { - const stateDir = mkdtempForTestSync('agent-device-web-provider-'); + const stateDir = mkdtempForTestSync('agent device web provider '); try { - installFakeManagedAgentBrowser(stateDir); + const install = installFakeManagedAgentBrowser(stateDir); const provider = createAgentBrowserWebProvider({ ...options, stateDir }); - await testFn(provider); + await testFn(provider, install); } finally { fs.rmSync(stateDir, { recursive: true, force: true }); } } -async function withNodeRuntimeVersion( - version: string, - testFn: () => void | Promise, -): Promise { - const originalNodeVersion = process.versions.node; - const originalProcessVersion = process.version; - Object.defineProperty(process.versions, 'node', { value: version, configurable: true }); - Object.defineProperty(process, 'version', { value: `v${version}`, configurable: true }); - try { - await testFn(); - } finally { - Object.defineProperty(process.versions, 'node', { - value: originalNodeVersion, - configurable: true, - }); - Object.defineProperty(process, 'version', { - value: originalProcessVersion, - configurable: true, - }); - } -} - function recordingExecutor(calls: AgentBrowserCall[]) { return async (cmd: string, args: string[]): Promise => { - calls.push({ cmd, args }); + recordAgentBrowserCall(calls, cmd, args); return jsonResult({ success: true, data: {} }); }; } +/** + * The backend runs as `node `; that shape is owned + * and asserted by agent-browser-tool. Here it is only stripped, so provider + * tests read as the agent-browser CLI arguments they are about. + */ +function agentBrowserCliArgs(args: string[]): string[] { + return args.slice(1); +} + +function recordAgentBrowserCall(calls: AgentBrowserCall[], cmd: string, args: string[]): string[] { + const cliArgs = agentBrowserCliArgs(args); + calls.push({ cmd, args: cliArgs }); + return cliArgs; +} + function snapshotExecutor(calls: AgentBrowserCall[]) { return async (cmd: string, args: string[], options: { allowFailure?: boolean }) => { - calls.push({ cmd, args }); - if (args[0] === 'snapshot') return snapshotPayload(); - if (args.slice(0, 3).join(' ') === 'get box @e3') { + const cliArgs = recordAgentBrowserCall(calls, cmd, args); + if (cliArgs[0] === 'snapshot') return snapshotPayload(); + if (cliArgs.slice(0, 3).join(' ') === 'get box @e3') { assert.equal(options.allowFailure, true); return jsonResult({ success: false, error: 'No box for element' }, 1); } - if (args[0] === 'get' && args[1] === 'box') return boxPayload(args[2]); + if (cliArgs[0] === 'get' && cliArgs[1] === 'box') return boxPayload(cliArgs[2]); return jsonResult({ success: true, data: {} }); }; } diff --git a/src/platforms/web/agent-browser-provider.ts b/src/platforms/web/agent-browser-provider.ts index fcf5f1481b..1bea9bf46c 100644 --- a/src/platforms/web/agent-browser-provider.ts +++ b/src/platforms/web/agent-browser-provider.ts @@ -1,4 +1,4 @@ -import { execFailureDetails, runCmd } from '../../utils/exec.ts'; +import { execFailureDetails } from '../../utils/exec.ts'; import { AppError } from '@agent-device/kernel/errors'; import { emitDiagnostic } from '../../utils/diagnostics.ts'; import { sleep } from '../../utils/timeouts.ts'; @@ -19,7 +19,7 @@ import type { WebProvider, WebSnapshotOptions, WebSnapshotResult } from './provi import { getManagedAgentBrowserStatus, mapManagedAgentBrowserError, - resolveAgentBrowserTool, + runManagedAgentBrowser, } from './agent-browser-tool.ts'; import { cleanupManagedAgentBrowserOrphansForProviderStartup, @@ -263,10 +263,9 @@ async function runAgentBrowserCommand( const status = getManagedAgentBrowserStatus({ stateDir: options.stateDir }); try { await cleanupProviderStartupOrphans(options); - const tool = await resolveAgentBrowserTool({ stateDir: options.stateDir }); - const result = await runCmd(tool.command, cliArgs, { + const result = await runManagedAgentBrowser(cliArgs, { + stateDir: options.stateDir, allowFailure: true, - env: tool.env, timeoutMs: AGENT_BROWSER_TIMEOUT_MS, signal, }); diff --git a/src/platforms/web/agent-browser-tool.test.ts b/src/platforms/web/agent-browser-tool.test.ts index d6cf13803f..0e1b88e3de 100644 --- a/src/platforms/web/agent-browser-tool.test.ts +++ b/src/platforms/web/agent-browser-tool.test.ts @@ -1,17 +1,29 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; -import { test } from 'vitest'; -import { resolveAgentBrowserTool } from './agent-browser-tool.ts'; -import { installFakeManagedAgentBrowser } from './__tests__/test-utils.ts'; +import { test, vi } from 'vitest'; +import { + getManagedAgentBrowserStatus, + runManagedAgentBrowser, + setupManagedAgentBrowser, +} from './agent-browser-tool.ts'; +import { + installFakeManagedAgentBrowser, + withNodeRuntime, + writeFakeManagedAgentBrowserPackage, + writeFakeNpmCliScript, +} from './__tests__/test-utils.ts'; import { AppError } from '@agent-device/kernel/errors'; +import { withCommandExecutorOverride } from '../../utils/exec.ts'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; -test('managed agent-browser tool reports actionable guidance when install is missing', async () => { +type SpawnedCommand = { cmd: string; args: string[] }; + +test('managed agent-browser reports actionable guidance when install is missing', async () => { const stateDir = mkdtempForTestSync('agent-device-web-tool-'); try { await assert.rejects( - () => resolveAgentBrowserTool({ stateDir }), + () => runManagedAgentBrowser(['doctor'], { stateDir, timeoutMs: 1_000 }), (error: unknown) => error instanceof AppError && error.code === 'TOOL_MISSING' && @@ -23,22 +35,207 @@ test('managed agent-browser tool reports actionable guidance when install is mis } }); -test('managed agent-browser tool uses short runtime home for backend state', async () => { +// The `node_modules/.bin` shim is a `.cmd` on Windows, which `child_process.spawn` +// rejects with EINVAL unless a shell is used (CVE-2024-27980 hardening). The +// fixture writes that shim, so this proves it exists and is still not the +// spawned command — including from an install path containing spaces. +test('managed agent-browser runs its JS entry with the current Node runtime, not the shim', async () => { + const stateDir = mkdtempForTestSync('agent device web tool '); + try { + const install = installFakeManagedAgentBrowser(stateDir); + const spawned: SpawnedCommand[] = []; + + await withCommandExecutorOverride( + async (cmd, args) => { + spawned.push({ cmd, args }); + return { stdout: '', stderr: '', exitCode: 0 }; + }, + async () => + await runManagedAgentBrowser(['snapshot', '--json'], { stateDir, timeoutMs: 1_000 }), + ); + + assert.ok(stateDir.includes(' '), 'the fixture must exercise a path containing spaces'); + assert.ok(fs.existsSync(install.binaryPath), 'the shim npm links must exist'); + assert.deepEqual(spawned, [ + { cmd: process.execPath, args: [install.entryScript, 'snapshot', '--json'] }, + ]); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +test('managed agent-browser passes the managed runtime home and socket dir to the backend', async () => { + const stateDir = mkdtempForTestSync('agent-device-web-tool-'); + try { + const install = installFakeManagedAgentBrowser(stateDir); + let env: NodeJS.ProcessEnv | undefined; + + await withCommandExecutorOverride( + async (_cmd, _args, options) => { + env = options.env; + return { stdout: '', stderr: '', exitCode: 0 }; + }, + async () => await runManagedAgentBrowser(['doctor'], { stateDir, timeoutMs: 1_000 }), + ); + + assert.equal(env?.HOME, install.runtimeHomeDir); + assert.equal(env?.AGENT_BROWSER_SOCKET_DIR, install.socketDir); + assert.equal(env?.AGENT_BROWSER_IDLE_TIMEOUT_MS, '300000'); + assert.match(env?.AGENT_BROWSER_ARGS ?? '', /^--agent-device-managed-web=[a-f0-9]{16}$/); + assert.notEqual(install.runtimeHomeDir, install.homeDir); + assert.ok(fs.existsSync(install.runtimeHomeDir)); + assert.ok(fs.existsSync(install.socketDir)); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +test('managed agent-browser status ignores a bin shim without the backend package', () => { const stateDir = mkdtempForTestSync('agent-device-web-tool-'); try { - const status = installFakeManagedAgentBrowser(stateDir); - - const tool = await resolveAgentBrowserTool({ stateDir }); - - assert.equal(tool.command, status.binaryPath); - assert.equal(tool.env?.HOME, status.runtimeHomeDir); - assert.equal(tool.env?.AGENT_BROWSER_SOCKET_DIR, status.socketDir); - assert.equal(tool.env?.AGENT_BROWSER_IDLE_TIMEOUT_MS, '300000'); - assert.match(tool.env?.AGENT_BROWSER_ARGS ?? '', /^--agent-device-managed-web=[a-f0-9]{16}$/); - assert.notEqual(status.runtimeHomeDir, status.homeDir); - assert.ok(fs.existsSync(status.runtimeHomeDir)); - assert.ok(fs.existsSync(status.socketDir)); + const installDir = path.join(stateDir, 'tools', 'agent-browser', '0.27.1'); + const binDir = path.join(installDir, 'package', 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + fs.writeFileSync(path.join(binDir, 'agent-browser'), '#!/bin/sh\nexit 0\n'); + fs.writeFileSync(path.join(binDir, 'agent-browser.cmd'), '@echo off\n'); + fs.writeFileSync(path.join(installDir, 'manifest.json'), '{}'); + + const status = getManagedAgentBrowserStatus({ stateDir }); + + assert.equal(status.entryScript, undefined); + assert.equal(status.installed, false); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +// npm itself is `npm.cmd` on Windows, so setup runs npm's JS entry there. POSIX +// keeps spawning `npm` from PATH, which was never broken. +for (const scenario of [ + { + name: 'posix keeps spawning npm from PATH', + platform: 'linux' as const, + advertiseNpm: false, + bundleNpm: false, + expected: 'bare-npm' as const, + }, + { + name: 'windows uses the npm launcher advertised through npm_execpath', + platform: 'win32' as const, + advertiseNpm: true, + bundleNpm: false, + expected: 'advertised' as const, + }, + { + name: 'windows falls back to the npm bundled beside node', + platform: 'win32' as const, + advertiseNpm: false, + bundleNpm: true, + expected: 'bundled' as const, + }, + { + name: 'windows without npm fails with actionable guidance', + platform: 'win32' as const, + advertiseNpm: false, + bundleNpm: false, + expected: 'missing' as const, + }, +]) { + test(`managed agent-browser setup resolves npm: ${scenario.name}`, async () => { + const stateDir = mkdtempForTestSync('agent device web setup '); + try { + const advertised = writeFakeNpmCliScript(path.join(stateDir, 'advertised')); + const nodeDir = path.join(stateDir, 'Program Files', 'nodejs'); + const bundled = path.join(nodeDir, 'node_modules', 'npm', 'bin', 'npm-cli.js'); + if (scenario.bundleNpm) { + fs.mkdirSync(path.dirname(bundled), { recursive: true }); + fs.writeFileSync(bundled, 'process.exit(0)\n'); + } + if (scenario.advertiseNpm) vi.stubEnv('npm_execpath', advertised); + else vi.stubEnv('npm_execpath', ''); + const spawned: SpawnedCommand[] = []; + + await withNodeRuntime( + { version: '24.13.0', platform: scenario.platform, execPath: path.join(nodeDir, 'node') }, + async () => { + const run = async () => + await withCommandExecutorOverride( + async (cmd, args) => { + spawned.push({ cmd, args }); + if (args.includes('install') && args.includes('--prefix')) { + writeFakeManagedAgentBrowserPackage(stateDir); + } + return { stdout: '', stderr: '', exitCode: 0 }; + }, + async () => await setupManagedAgentBrowser({ stateDir }), + ); + if (scenario.expected === 'missing') { + await assert.rejects( + run, + (error: unknown) => + error instanceof AppError && + error.code === 'TOOL_MISSING' && + error.message === 'npm not found in PATH' && + typeof error.details?.hint === 'string', + ); + return; + } + await run(); + }, + ); + + if (scenario.expected === 'missing') { + assert.deepEqual(spawned, [], 'no command may be spawned when npm cannot be resolved'); + return; + } + const npmSpawn = spawned[0]; + const expectedNpm = scenario.expected === 'advertised' ? advertised : bundled; + if (scenario.expected === 'bare-npm') { + assert.equal(npmSpawn?.cmd, 'npm'); + assert.equal(npmSpawn?.args[0], 'install'); + } else { + assert.equal(npmSpawn?.cmd, path.join(nodeDir, 'node')); + assert.equal(npmSpawn?.args[0], expectedNpm); + assert.equal(npmSpawn?.args[1], 'install'); + } + assert.deepEqual(npmSpawn?.args.slice(npmSpawn.args.indexOf('install')), [ + 'install', + '--prefix', + path.join(stateDir, 'tools', 'agent-browser', '0.27.1', 'package'), + '--no-global', + '--no-audit', + '--no-fund', + '--no-save', + 'agent-browser@0.27.1', + ]); + } finally { + vi.unstubAllEnvs(); + fs.rmSync(stateDir, { recursive: true, force: true }); + } + }); +} + +test('managed agent-browser setup reports an install that produced no entry', async () => { + const stateDir = mkdtempForTestSync('agent-device-web-setup-empty-'); + vi.stubEnv('npm_execpath', writeFakeNpmCliScript(stateDir)); + try { + await withNodeRuntime({ version: '24.13.0' }, async () => { + await withCommandExecutorOverride( + // npm succeeds without writing the package, as an install redirected + // out of the managed prefix would. + async () => ({ stdout: '', stderr: '', exitCode: 0 }), + async () => + await assert.rejects( + () => setupManagedAgentBrowser({ stateDir }), + (error: unknown) => + error instanceof AppError && + error.code === 'TOOL_MISSING' && + error.message === 'Managed web backend install produced no runnable entry.', + ), + ); + }); } finally { + vi.unstubAllEnvs(); fs.rmSync(stateDir, { recursive: true, force: true }); } }); diff --git a/src/platforms/web/agent-browser-tool.ts b/src/platforms/web/agent-browser-tool.ts index 3dac1e61ee..93a70d47ba 100644 --- a/src/platforms/web/agent-browser-tool.ts +++ b/src/platforms/web/agent-browser-tool.ts @@ -2,8 +2,12 @@ import fs from 'node:fs'; import crypto from 'node:crypto'; import os from 'node:os'; import path from 'node:path'; -import { runCmd } from '../../utils/exec.ts'; +import { runCmd, type ExecResult } from '../../utils/exec.ts'; import { AppError, asAppError } from '@agent-device/kernel/errors'; +import { + installManagedAgentBrowserPackage, + writeManagedAgentBrowserManifest, +} from './agent-browser-install.ts'; import { acquireProcessLock } from '../../utils/process-lock.ts'; import { readProcessStartTime } from '../../utils/host-process.ts'; import { @@ -18,16 +22,15 @@ const MINIMUM_WEB_NODE_MAJOR = 24; const SETUP_TIMEOUT_MS = 5 * 60_000; const DOCTOR_TIMEOUT_MS = 60_000; -export type AgentBrowserTool = { - command: string; - env?: NodeJS.ProcessEnv; -}; - export type AgentBrowserToolStatus = { version: string; stateDir: string; installDir: string; + packageDir: string; + /** npm's console shim. Published since #833; informational — never spawned (#2022). */ binaryPath: string; + /** Backend JS entry, undefined until the managed package is installed. */ + entryScript: string | undefined; homeDir: string; runtimeHomeDir: string; socketDir: string; @@ -36,15 +39,25 @@ export type AgentBrowserToolStatus = { nodeSupported: boolean; }; -export async function resolveAgentBrowserTool(options: { +export type ManagedAgentBrowserRunOptions = { stateDir?: string; -}): Promise { - const status = getManagedAgentBrowserStatus(options); - if (status.installed) { - return createManagedTool(status); - } + timeoutMs: number; + allowFailure?: boolean; + signal?: AbortSignal; +}; - throw missingManagedToolError(status); +/** + * The only way the managed backend is executed. Entry resolution, the Node + * runtime, the managed environment, and the spawn all live behind this call, so + * no caller can reintroduce the `.bin` shim that Windows cannot spawn (#2022). + */ +export async function runManagedAgentBrowser( + args: readonly string[], + options: ManagedAgentBrowserRunOptions, +): Promise { + const status = getManagedAgentBrowserStatus({ stateDir: options.stateDir }); + if (!status.installed) throw missingManagedToolError(status); + return await spawnManagedAgentBrowser(status, args, options); } export async function setupManagedAgentBrowser(options: { @@ -67,12 +80,23 @@ export async function setupManagedAgentBrowser(options: { const freshStatus = getManagedAgentBrowserStatus(options); if (freshStatus.installed) return freshStatus; fs.mkdirSync(freshStatus.installDir, { recursive: true }); - await installAgentBrowserPackage(freshStatus); - await runManagedAgentBrowser(freshStatus, ['install'], { timeoutMs: SETUP_TIMEOUT_MS }); - await runManagedAgentBrowser(freshStatus, ['doctor', '--offline', '--quick'], { + await installManagedAgentBrowserPackage({ + packageRoot: path.join(freshStatus.installDir, 'package'), + packageSpec: `${AGENT_BROWSER}@${MANAGED_AGENT_BROWSER_VERSION}`, + timeoutMs: SETUP_TIMEOUT_MS, + }); + // The backend entry only exists once npm has written the package. + const installedStatus = getManagedAgentBrowserStatus(options); + if (!installedStatus.entryScript) throw unusableInstallError(installedStatus); + await spawnManagedAgentBrowser(installedStatus, ['install'], { timeoutMs: SETUP_TIMEOUT_MS }); + await spawnManagedAgentBrowser(installedStatus, ['doctor', '--offline', '--quick'], { timeoutMs: DOCTOR_TIMEOUT_MS, }); - writeManifest(freshStatus); + writeManagedAgentBrowserManifest({ + installDir: installedStatus.installDir, + packageName: AGENT_BROWSER, + version: MANAGED_AGENT_BROWSER_VERSION, + }); return getManagedAgentBrowserStatus(options); } finally { await release(); @@ -86,7 +110,7 @@ export async function doctorManagedAgentBrowser(options: { if (!status.installed) { throw missingManagedToolError(status); } - const result = await runManagedAgentBrowser(status, ['doctor', '--offline', '--quick'], { + const result = await spawnManagedAgentBrowser(status, ['doctor', '--offline', '--quick'], { timeoutMs: DOCTOR_TIMEOUT_MS, allowFailure: true, }); @@ -98,17 +122,21 @@ export function getManagedAgentBrowserStatus(options: { }): AgentBrowserToolStatus { const stateDir = options.stateDir ?? process.env.AGENT_DEVICE_STATE_DIR ?? defaultStateDir(); const installDir = path.join(stateDir, 'tools', 'agent-browser', MANAGED_AGENT_BROWSER_VERSION); + const packageDir = resolveManagedPackageDir(installDir); const binaryPath = resolveManagedBinaryPath(installDir); + const entryScript = resolveManagedEntryScript(packageDir); const homeDir = path.join(installDir, 'home'); const runtimeHomeDir = resolveManagedRuntimeHomeDir(installDir); const socketDir = resolveManagedSocketDir(installDir); - const installed = isExecutable(binaryPath) && hasManifest(installDir); + const installed = entryScript !== undefined && hasManifest(installDir); const nodeMajor = Number.parseInt(process.versions.node.split('.')[0] ?? '0', 10); return { version: MANAGED_AGENT_BROWSER_VERSION, stateDir, installDir, + packageDir, binaryPath, + entryScript, homeDir, runtimeHomeDir, socketDir, @@ -118,41 +146,19 @@ export function getManagedAgentBrowserStatus(options: { }; } -function createManagedTool(status: AgentBrowserToolStatus): AgentBrowserTool { - if (!status.installed) throw missingManagedToolError(status); - return { - command: status.binaryPath, - env: managedAgentBrowserEnv(status, process.env), - }; -} - -async function installAgentBrowserPackage(status: AgentBrowserToolStatus): Promise { - const packageRoot = path.join(status.installDir, 'package'); - fs.mkdirSync(packageRoot, { recursive: true }); - await runCmd( - 'npm', - [ - 'install', - '--prefix', - packageRoot, - '--no-audit', - '--no-fund', - '--no-save', - `${AGENT_BROWSER}@${MANAGED_AGENT_BROWSER_VERSION}`, - ], - { env: process.env, timeoutMs: SETUP_TIMEOUT_MS }, - ); -} - -async function runManagedAgentBrowser( +// `node `, never the `node_modules/.bin` shim: that shim is a `.cmd` on +// Windows, which spawn refuses without a shell since CVE-2024-27980 (#2022). +async function spawnManagedAgentBrowser( status: AgentBrowserToolStatus, - args: string[], - options: { timeoutMs: number; allowFailure?: boolean }, -) { - return await runCmd(status.binaryPath, args, { + args: readonly string[], + options: Omit, +): Promise { + if (!status.entryScript) throw missingManagedToolError(status); + return await runCmd(process.execPath, [status.entryScript, ...args], { allowFailure: options.allowFailure, env: managedAgentBrowserEnv(status, process.env), timeoutMs: options.timeoutMs, + signal: options.signal, }); } @@ -190,35 +196,54 @@ function ensureRuntimeHomeDir(status: AgentBrowserToolStatus): void { } } -function writeManifest(status: AgentBrowserToolStatus): void { - fs.writeFileSync( - path.join(status.installDir, 'manifest.json'), - JSON.stringify( - { - package: AGENT_BROWSER, - version: MANAGED_AGENT_BROWSER_VERSION, - node: process.version, - installedAt: new Date().toISOString(), - }, - null, - 2, - ), - 'utf8', - ); -} - function hasManifest(installDir: string): boolean { return fs.existsSync(path.join(installDir, 'manifest.json')); } +/** + * The backend's declared `bin` entry. Read from the installed manifest rather + * than hard-coded, because the path inside the package is the package's to + * choose (`bin/agent-browser.js` in 0.27.1). + */ +function resolveManagedEntryScript(packageDir: string): string | undefined { + let bin: unknown; + try { + const manifest: unknown = JSON.parse( + fs.readFileSync(path.join(packageDir, 'package.json'), 'utf8'), + ); + bin = + typeof manifest === 'object' && manifest !== null + ? (manifest as { bin?: unknown }).bin + : undefined; + } catch { + return undefined; + } + const declaredPath = + typeof bin === 'string' + ? bin + : typeof bin === 'object' && bin !== null + ? (bin as Record)[AGENT_BROWSER] + : undefined; + if (typeof declaredPath !== 'string') return undefined; + const entryScript = path.resolve(packageDir, declaredPath); + return isFile(entryScript) ? entryScript : undefined; +} + +function isFile(filePath: string): boolean { + try { + return fs.statSync(filePath).isFile(); + } catch { + return false; + } +} + +function resolveManagedPackageDir(installDir: string): string { + return path.join(installDir, 'package', 'node_modules', AGENT_BROWSER); +} + function resolveManagedBinaryPath(installDir: string): string { - const packageRoot = path.join(installDir, 'package'); - return path.join( - packageRoot, - 'node_modules', - '.bin', - process.platform === 'win32' ? 'agent-browser.cmd' : 'agent-browser', - ); + const shim = process.platform === 'win32' ? `${AGENT_BROWSER}.cmd` : AGENT_BROWSER; + return path.join(installDir, 'package', 'node_modules', '.bin', shim); } function missingManagedToolError(status: AgentBrowserToolStatus): AppError { @@ -232,6 +257,17 @@ function missingManagedToolError(status: AgentBrowserToolStatus): AppError { }); } +// npm exited 0 but left no runnable entry: reported apart from the +// not-installed-yet case, whose "run web setup" hint is useless in `web setup`. +function unusableInstallError(status: AgentBrowserToolStatus): AppError { + return new AppError('TOOL_MISSING', 'Managed web backend install produced no runnable entry.', { + version: MANAGED_AGENT_BROWSER_VERSION, + installDir: status.installDir, + packageDir: status.packageDir, + hint: `Remove ${status.installDir} and run \`agent-device web setup\` again.`, + }); +} + function assertWebNodeSupported(nodeMajor: number): void { if (nodeMajor >= MINIMUM_WEB_NODE_MAJOR) return; throw new AppError('UNSUPPORTED_OPERATION', 'Web automation requires Node 24 or newer.', { @@ -256,15 +292,6 @@ function isNoEntryError(error: unknown): boolean { return typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT'; } -function isExecutable(filePath: string): boolean { - try { - fs.accessSync(filePath, fs.constants.X_OK); - return true; - } catch { - return false; - } -} - function defaultStateDir(): string { return path.join(process.env.HOME ?? process.cwd(), '.agent-device'); }