Skip to content

Commit 99baeb6

Browse files
thymikeeclaude
andcommitted
fix(web): launch npm and the managed backend through node, not .cmd shims
On Windows every `--platform web` command failed with `spawn EINVAL`: the managed backend resolved to `node_modules/.bin/agent-browser.cmd` and was spawned with `shell: false`, which Node refuses for `.bat`/`.cmd` since the CVE-2024-27980 fix. `web setup` failed earlier still — a bare `npm` is not spawnable on Windows, where npm ships as `npm.cmd`. `runManagedAgentBrowser` is now the only path that executes the backend. Entry resolution, the Node runtime, the managed environment, and the spawn all live behind it, so setup, doctor, and the provider cannot reintroduce the shim. The entry comes from the installed package's declared `bin` rather than a hard-coded path, which is the part of this worth being precise about. npm is untouched on macOS and Linux, which were never broken: setup still spawns `npm` from PATH. Only Windows resolves npm's own `npm-cli.js` — from an `npm_execpath` that really is npm's launcher, else the copy bundled beside `node` — and fails with the existing actionable TOOL_MISSING when neither is there. Setup also pins `--no-global` so an ambient `npm_config_global` cannot redirect the install out of the managed prefix. The published status shape is unchanged: `binaryPath` still names npm's console shim, now informational rather than the spawned command, and `entryScript` plus `packageDir` are additive. Closes #2022 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LMS3BidXb3F4HSr26vvQmG
1 parent c77bc40 commit 99baeb6

12 files changed

Lines changed: 659 additions & 156 deletions

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,18 @@
3333
`-dev` prerelease marker so the version on `main` never equals a published version (registry
3434
scanners diff the tool surface per version string, and a moving surface under a released number
3535
reads as a republish). `release:prepare` refuses to publish while the `-dev` marker is in place.
36+
- Windows `--platform web` works again. `agent-device web setup` no longer fails with
37+
`npm not found in PATH`, and every web command — including `web doctor` — no longer fails with
38+
`spawn EINVAL`. The managed `agent-browser` backend is now launched as `node <js-entry>` on every
39+
platform instead of through its `node_modules/.bin` console shim, which is a `.cmd` file on
40+
Windows that `child_process.spawn` refuses without a shell (CVE-2024-27980 hardening);
41+
`shell: true` would only trade that for argument-quoting hazards and a `DEP0190` warning on every
42+
command. Setup spawns `npm` from PATH unchanged on macOS and Linux, and runs npm's own
43+
`npm-cli.js` under the current Node only on Windows, where a bare `npm` is not spawnable. A
44+
managed install now counts as present only when the backend package itself is, and
45+
`web setup --json` / `web doctor --json` gain `entryScript` and `packageDir`; the published
46+
`binaryPath` is unchanged and still names npm's console shim, now informational rather than the
47+
spawned command (#2022).
3648
- Parameterized `fill --record-as` protection is now recording-session-scoped instead of
3749
fill-step-scoped (ADR 0017 amendment): a later, unrelated recorded action (`wait`, `is`, `get`) can no
3850
longer re-serialize an app-rendered echo of an already-parameterized value into its own result or

docs/agents/web-backend.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ Web automation uses a managed `agent-browser` backend as an implementation detai
88
- Use `agent-device web doctor` to run the backend health check.
99
- The managed install respects `--state-dir` / `AGENT_DEVICE_STATE_DIR`.
1010
- Web automation requires Node 24+ while the rest of agent-device keeps its Node 22 baseline.
11+
- Every backend call spawns the package's declared `bin` entry with the current Node runtime,
12+
never the `node_modules/.bin` console shim: Windows ships that shim as `.cmd`, which
13+
`child_process.spawn` refuses without a shell (CVE-2024-27980 hardening), and a shell would
14+
reintroduce argument-quoting hazards. `runManagedAgentBrowser` is the only path that executes
15+
the backend. Setup spawns `npm` from PATH as before, except on Windows, where a bare `npm` is
16+
not spawnable and npm's own `npm-cli.js` runs under the current Node instead.
1117

1218
Default first-run flow:
1319

src/__tests__/cli-web.test.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import assert from 'node:assert/strict';
2+
import fs from 'node:fs';
3+
import { test } from 'vitest';
4+
import { runCliCapture } from './cli-capture.ts';
5+
import { mkdtempForTestSync } from './test-utils/tmp-dir.ts';
6+
import { withCommandExecutorOverride } from '../utils/exec.ts';
7+
import {
8+
installFakeManagedAgentBrowser,
9+
withNodeRuntime,
10+
writeFakeManagedAgentBrowserPackage,
11+
} from '../platforms/web/__tests__/test-utils.ts';
12+
13+
type SpawnedCommand = { cmd: string; args: string[] };
14+
15+
// `binaryPath` has been in the published `web setup`/`web doctor` JSON since #833.
16+
// The Windows spawn fix (#2022) moves execution to `node <entryScript>` and adds
17+
// `entryScript`/`packageDir`, but the released field stays in the contract.
18+
test('web doctor --json keeps the published status fields and spawns the JS entry', async () => {
19+
const stateDir = mkdtempForTestSync('agent device cli web doctor ');
20+
try {
21+
const install = installFakeManagedAgentBrowser(stateDir);
22+
const spawned: SpawnedCommand[] = [];
23+
24+
const result = await withCommandExecutorOverride(
25+
async (cmd, args) => {
26+
spawned.push({ cmd, args });
27+
return { stdout: 'ok', stderr: '', exitCode: 0 };
28+
},
29+
async () =>
30+
await runCliCapture(['web', 'doctor', '--json'], {
31+
env: { AGENT_DEVICE_STATE_DIR: stateDir },
32+
}),
33+
);
34+
35+
const status = parseStatus(result.stdout);
36+
assert.equal(status.binaryPath, install.binaryPath);
37+
assert.equal(status.entryScript, install.entryScript);
38+
assert.equal(status.packageDir, install.packageDir);
39+
assert.equal(status.installDir, install.installDir);
40+
assert.equal(status.installed, true);
41+
assert.equal(status.socketDir, undefined);
42+
assert.deepEqual(spawned, [
43+
{ cmd: process.execPath, args: [install.entryScript, 'doctor', '--offline', '--quick'] },
44+
]);
45+
assert.equal(result.calls.length, 0);
46+
} finally {
47+
fs.rmSync(stateDir, { recursive: true, force: true });
48+
}
49+
});
50+
51+
test('web setup --json keeps the published status fields after installing', async () => {
52+
const stateDir = mkdtempForTestSync('agent device cli web setup ');
53+
try {
54+
let install: ReturnType<typeof writeFakeManagedAgentBrowserPackage> | undefined;
55+
let stdout = '';
56+
57+
await withNodeRuntime({ version: '24.13.0' }, async () => {
58+
const result = await withCommandExecutorOverride(
59+
async (_cmd, args) => {
60+
// Stand in for the npm run that writes the managed package tree.
61+
if (args.includes('--prefix')) install = writeFakeManagedAgentBrowserPackage(stateDir);
62+
return { stdout: '', stderr: '', exitCode: 0 };
63+
},
64+
async () =>
65+
await runCliCapture(['web', 'setup', '--json'], {
66+
env: { AGENT_DEVICE_STATE_DIR: stateDir },
67+
}),
68+
);
69+
stdout = result.stdout;
70+
});
71+
72+
const status = parseStatus(stdout);
73+
assert.equal(status.binaryPath, install?.binaryPath);
74+
assert.equal(status.entryScript, install?.entryScript);
75+
assert.equal(status.packageDir, install?.packageDir);
76+
assert.equal(status.installed, true);
77+
assert.equal(status.socketDir, undefined);
78+
} finally {
79+
fs.rmSync(stateDir, { recursive: true, force: true });
80+
}
81+
});
82+
83+
function parseStatus(stdout: string): Record<string, unknown> {
84+
const payload: unknown = JSON.parse(firstJsonDocument(stdout));
85+
assert.ok(isRecord(payload) && payload.success === true, stdout);
86+
const data = payload.data;
87+
assert.ok(isRecord(data), stdout);
88+
const status = data.status;
89+
assert.ok(isRecord(status), stdout);
90+
return status;
91+
}
92+
93+
function isRecord(value: unknown): value is Record<string, unknown> {
94+
return typeof value === 'object' && value !== null;
95+
}
96+
97+
/**
98+
* The CLI dispatches `web` inside its top-level try, and the capture harness
99+
* turns `process.exit` into a throw, so the command's payload is followed by the
100+
* CLI's own report of that synthetic exit. Only the first document is the
101+
* command's own output; real runs exit for real and print it once.
102+
*/
103+
function firstJsonDocument(stdout: string): string {
104+
const lines = stdout.split('\n');
105+
const end = lines.indexOf('}');
106+
assert.ok(end >= 0, stdout);
107+
return lines.slice(0, end + 1).join('\n');
108+
}

src/daemon/handlers/__tests__/session-teardown-resources.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,10 @@ test('daemon session teardown closes an open web session immediately, not on age
351351
// fleet right away, the same way an explicit `session close` does, instead of leaving the
352352
// Chrome processes to agent-browser's own multi-minute idle timer.
353353
expect(mockRunCmd).toHaveBeenCalledTimes(1);
354-
const [, args] = mockRunCmd.mock.calls[0] as [string, string[]];
355-
expect(args).toEqual(['close', '--json', '--session', sessionName]);
354+
const [cmd, args] = mockRunCmd.mock.calls[0] as [string, string[]];
355+
// `node <entry> close ...`: the managed backend never runs through its `.bin` shim (#2022).
356+
expect(cmd).toBe(process.execPath);
357+
expect(args.slice(1)).toEqual(['close', '--json', '--session', sessionName]);
356358
});
357359

358360
test('daemon session teardown surfaces a web close failure through the cleanup-failure channel', async () => {

src/daemon/server/daemon-runtime-web-close-teardown.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,14 @@ test('daemon shutdown closes an open web session immediately, without waiting fo
129129
expect(stderrChunks.join('')).toBe('');
130130
expect(sessionStore.get(session.name)).toBeUndefined();
131131
const closeCall = mockRunCmd.mock.calls.find(([, args]) => (args as string[]).includes('close'));
132-
expect(closeCall?.[1]).toEqual(['close', '--json', '--session', session.name]);
132+
// `node <entry> close ...`: the managed backend never runs through its `.bin` shim (#2022).
133+
expect(closeCall?.[0]).toBe(process.execPath);
134+
expect((closeCall?.[1] as string[] | undefined)?.slice(1)).toEqual([
135+
'close',
136+
'--json',
137+
'--session',
138+
session.name,
139+
]);
133140
});
134141

135142
test('daemon shutdown reports a web close failure on stderr instead of losing it silently', async () => {

src/platforms/web/__tests__/test-utils.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,81 @@ const TEST_AGENT_BROWSER_VERSION = '0.27.1';
88
type FakeManagedAgentBrowserInstall = ReturnType<typeof expectedManagedAgentBrowserInstall>;
99

1010
export function installFakeManagedAgentBrowser(stateDir: string): FakeManagedAgentBrowserInstall {
11+
const install = writeFakeManagedAgentBrowserPackage(stateDir);
12+
fs.writeFileSync(path.join(install.installDir, 'manifest.json'), '{}');
13+
return install;
14+
}
15+
16+
/** The package tree `npm install` leaves behind, without the setup manifest. */
17+
export function writeFakeManagedAgentBrowserPackage(
18+
stateDir: string,
19+
): FakeManagedAgentBrowserInstall {
1120
const install = expectedManagedAgentBrowserInstall(stateDir);
21+
fs.mkdirSync(path.dirname(install.entryScript), { recursive: true });
22+
fs.writeFileSync(install.entryScript, 'process.exit(0)\n');
23+
// npm links a console shim beside the package; the fixture carries it so tests
24+
// prove the shim is never spawned rather than merely absent (#2022).
1225
fs.mkdirSync(path.dirname(install.binaryPath), { recursive: true });
1326
fs.writeFileSync(install.binaryPath, '#!/bin/sh\nexit 0\n');
1427
fs.chmodSync(install.binaryPath, 0o755);
15-
fs.writeFileSync(path.join(install.installDir, 'manifest.json'), '{}');
28+
fs.writeFileSync(
29+
path.join(install.packageDir, 'package.json'),
30+
JSON.stringify({
31+
name: 'agent-browser',
32+
version: TEST_AGENT_BROWSER_VERSION,
33+
bin: { 'agent-browser': './dist/cli.js' },
34+
}),
35+
);
1636
return install;
1737
}
1838

39+
/** npm's own JS launcher, for tests that drive managed setup without a real npm. */
40+
export function writeFakeNpmCliScript(root: string): string {
41+
const npmCliScript = path.join(root, 'node runtime', 'node_modules', 'npm', 'bin', 'npm-cli.js');
42+
fs.mkdirSync(path.dirname(npmCliScript), { recursive: true });
43+
fs.writeFileSync(npmCliScript, 'process.exit(0)\n');
44+
return npmCliScript;
45+
}
46+
47+
/** Reports a different Node runtime to the code under test for one scenario. */
48+
export async function withNodeRuntime(
49+
overrides: { version?: string; platform?: NodeJS.Platform; execPath?: string },
50+
testFn: () => void | Promise<void>,
51+
): Promise<void> {
52+
const restore: (() => void)[] = [];
53+
const override = (target: object, key: string, value: unknown) => {
54+
const original = (target as Record<string, unknown>)[key];
55+
Object.defineProperty(target, key, { value, configurable: true });
56+
restore.push(() => Object.defineProperty(target, key, { value: original, configurable: true }));
57+
};
58+
if (overrides.version !== undefined) {
59+
override(process.versions, 'node', overrides.version);
60+
override(process, 'version', `v${overrides.version}`);
61+
}
62+
if (overrides.platform !== undefined) override(process, 'platform', overrides.platform);
63+
if (overrides.execPath !== undefined) override(process, 'execPath', overrides.execPath);
64+
try {
65+
await testFn();
66+
} finally {
67+
for (const undo of restore.reverse()) undo();
68+
}
69+
}
70+
1971
function expectedManagedAgentBrowserInstall(stateDir: string) {
2072
const installDir = path.join(stateDir, 'tools', 'agent-browser', TEST_AGENT_BROWSER_VERSION);
73+
const packageDir = path.join(installDir, 'package', 'node_modules', 'agent-browser');
2174
return {
2275
version: TEST_AGENT_BROWSER_VERSION,
2376
installDir,
77+
packageDir,
2478
binaryPath: path.join(
2579
installDir,
2680
'package',
2781
'node_modules',
2882
'.bin',
2983
process.platform === 'win32' ? 'agent-browser.cmd' : 'agent-browser',
3084
),
85+
entryScript: path.join(packageDir, 'dist', 'cli.js'),
3186
homeDir: path.join(installDir, 'home'),
3287
runtimeHomeDir:
3388
process.platform === 'win32'
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import fs from 'node:fs';
2+
import path from 'node:path';
3+
import { AppError } from '@agent-device/kernel/errors';
4+
import { runCmd } from '../../utils/exec.ts';
5+
6+
/**
7+
* How the managed backend gets onto disk. Kept apart from the tool module,
8+
* which owns locating and running what this leaves behind.
9+
*/
10+
export async function installManagedAgentBrowserPackage(params: {
11+
packageRoot: string;
12+
packageSpec: string;
13+
timeoutMs: number;
14+
}): Promise<void> {
15+
fs.mkdirSync(params.packageRoot, { recursive: true });
16+
// `--no-global` keeps an ambient `npm_config_global` from redirecting the
17+
// install out of the managed prefix, where the backend entry would be missed.
18+
const npm = npmCommand([
19+
'install',
20+
'--prefix',
21+
params.packageRoot,
22+
'--no-global',
23+
'--no-audit',
24+
'--no-fund',
25+
'--no-save',
26+
params.packageSpec,
27+
]);
28+
await runCmd(npm.command, npm.args, { env: process.env, timeoutMs: params.timeoutMs });
29+
}
30+
31+
export function writeManagedAgentBrowserManifest(params: {
32+
installDir: string;
33+
packageName: string;
34+
version: string;
35+
}): void {
36+
fs.writeFileSync(
37+
path.join(params.installDir, 'manifest.json'),
38+
JSON.stringify(
39+
{
40+
package: params.packageName,
41+
version: params.version,
42+
node: process.version,
43+
installedAt: new Date().toISOString(),
44+
},
45+
null,
46+
2,
47+
),
48+
'utf8',
49+
);
50+
}
51+
52+
/**
53+
* POSIX spawns `npm` from PATH exactly as it always has. Only Windows is
54+
* broken: npm ships as `npm.cmd` there, which `child_process.spawn` refuses
55+
* without a shell since the CVE-2024-27980 fix, so its JS entry runs under the
56+
* current Node runtime instead (#2022).
57+
*/
58+
function npmCommand(args: string[]): { command: string; args: string[] } {
59+
if (process.platform !== 'win32') return { command: 'npm', args };
60+
const npmCliScript = resolveWindowsNpmCliScript(process.env);
61+
if (!npmCliScript) {
62+
throw new AppError('TOOL_MISSING', 'npm not found in PATH', {
63+
nodeExecPath: process.execPath,
64+
hint: 'Install Node.js with npm, or add npm to PATH, and run `agent-device web setup` again.',
65+
});
66+
}
67+
return { command: process.execPath, args: [npmCliScript, ...args] };
68+
}
69+
70+
function resolveWindowsNpmCliScript(env: NodeJS.ProcessEnv): string | undefined {
71+
const advertised = env.npm_execpath?.trim();
72+
if (advertised) {
73+
const scriptPath = path.resolve(advertised);
74+
// pnpm and yarn advertise their own launcher through the same variable.
75+
if (path.basename(scriptPath) === 'npm-cli.js' && isFile(scriptPath)) return scriptPath;
76+
}
77+
const bundled = path.join(
78+
path.dirname(process.execPath),
79+
'node_modules',
80+
'npm',
81+
'bin',
82+
'npm-cli.js',
83+
);
84+
return isFile(bundled) ? bundled : undefined;
85+
}
86+
87+
function isFile(filePath: string): boolean {
88+
try {
89+
return fs.statSync(filePath).isFile();
90+
} catch {
91+
return false;
92+
}
93+
}

src/platforms/web/agent-browser-lifecycle.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ test('records the managed browser daemon and Chrome fleet at the spawn owner', a
283283
const store = { replace: vi.fn(), clear: vi.fn(), read: vi.fn(() => []) };
284284
mockRunCmd.mockResolvedValue({
285285
stdout: [
286-
` 101 1 ${status.binaryPath}`,
286+
` 101 1 ${process.execPath} ${status.entryScript}`,
287287
` 201 101 /Applications/Chromium.app/Contents/MacOS/Chromium ${marker}`,
288288
' 301 201 /Applications/Chromium.app/Contents/Frameworks/Chromium Helper --type=gpu',
289289
' 999 1 /Applications/Google Chrome.app/Contents/MacOS/Google Chrome',
@@ -315,8 +315,10 @@ test('managed process summary includes the recordable agent-browser daemon', ()
315315
installFakeManagedAgentBrowser(stateDir);
316316
const status = getManagedAgentBrowserStatus({ stateDir });
317317
expect(
318-
summarizeManagedAgentBrowserProcesses([{ pid: 101, command: status.binaryPath }], status)
319-
.processes[0]?.reason,
318+
summarizeManagedAgentBrowserProcesses(
319+
[{ pid: 101, command: `${process.execPath} ${status.entryScript}` }],
320+
status,
321+
).processes[0]?.reason,
320322
).toBe('agent-browser-daemon');
321323
} finally {
322324
fs.rmSync(stateDir, { recursive: true, force: true });

0 commit comments

Comments
 (0)