diff --git a/framework/cli/src/lib/bridge.ts b/framework/cli/src/lib/bridge.ts index bfa173b..2156810 100644 --- a/framework/cli/src/lib/bridge.ts +++ b/framework/cli/src/lib/bridge.ts @@ -52,7 +52,7 @@ import { closeSync, existsSync, openSync, readSync, rmSync, statSync } from "nod import { join } from "node:path"; import type { A2AppClient, Task } from "@a2app/sdk"; import type { HarnessProfile, Route } from "./harness.js"; -import { PROMPT_PLACEHOLDER, resolveOnPath } from "./harness.js"; +import { PROMPT_PLACEHOLDER, launchSpec, resolveOnPath } from "./harness.js"; import { fetchWithTimeout, pollHealth } from "./net.js"; import { killTreeForce, spawnBackgroundShell } from "./proc.js"; import { writeFileAtomic } from "./home.js"; @@ -343,6 +343,7 @@ async function deliverHeadless( cwd: string, timeoutMs: number, onHeartbeat: () => Promise, + heartbeatMs: number = HEARTBEAT_MS, ): Promise { const started = Date.now(); const resolved = resolveOnPath(route.command); @@ -355,11 +356,22 @@ async function deliverHeadless( ms: 0, }; } + const launch = launchSpec(resolved); + if ("error" in launch) { + return { ok: false, code: "harness_spawn_failed", completes: true, detail: launch.error, ms: 0 }; + } const useStdin = route.input === "stdin"; - const args = useStdin ? route.args : route.args.map((a) => a.split(PROMPT_PLACEHOLDER).join(prompt)); - - return new Promise((resolve) => { - const child = spawn(resolved, args, { + const args = [ + ...launch.prefixArgs, + ...(useStdin ? route.args : route.args.map((a) => a.split(PROMPT_PLACEHOLDER).join(prompt))), + ]; + + // spawn() can throw synchronously (EINVAL, ENAMETOOLONG) before any 'error' + // event exists to catch it. Escaping here used to leave the task claimed + // with nobody running it, until the app redelivered it to exhaustion. + let child: ReturnType; + try { + child = spawn(launch.file, args, { cwd: route.cwd !== undefined && route.cwd !== "app" ? route.cwd : cwd, shell: false, stdio: [useStdin ? "pipe" : "ignore", "pipe", "pipe"], @@ -373,6 +385,17 @@ async function deliverHeadless( // terminal popping up in front of the user for each delivered task. windowsHide: true, }); + } catch (err) { + return { + ok: false, + code: "harness_spawn_failed", + completes: true, + detail: `${route.command} could not be started: ${(err as Error).message}`, + ms: Date.now() - started, + }; + } + + return new Promise((resolve) => { let out = ""; let errOut = ""; let settled = false; @@ -443,7 +466,7 @@ async function deliverHeadless( ms: Date.now() - started, }); }); - }, HEARTBEAT_MS); + }, heartbeatMs); }); } @@ -493,6 +516,8 @@ export interface BridgeContext { route: Route; prompt: PromptContext; taskTimeoutMs: number; + /** how often a running delivery re-asserts its claim; defaults to HEARTBEAT_MS */ + heartbeatMs?: number; /** null means every capability; otherwise only these are delivered */ capabilities: string[] | null; dryRun: boolean; @@ -522,9 +547,17 @@ export async function deliverAndSettle(ctx: BridgeContext, task: Task): Promise< // should see the task move the moment it is picked up, not when it finishes. await ctx.client.progressTask(task.id, { step: `delivered to ${ctx.profile.id} (${ctx.route.mode})`, percent: 0 }); - /** Keep the claim alive; false means the app no longer agrees it is ours. */ + /** + * Keep the claim alive; false means the app no longer agrees it is ours. + * + * It carries no step. An agent that reports its own ("Writing triage notes") + * is telling the person watching what it is doing, and a heartbeat every 20 s + * that replaced it with "running in claude" made the app flip back and forth + * between the two. With no step sent, the step stays whatever was said last: + * the agent's, or "delivered to …" until it says anything. + */ const heartbeat = async (): Promise => { - const res = await ctx.client.progressTask(task.id, { step: `running in ${ctx.profile.id}` }).catch(() => null); + const res = await ctx.client.progressTask(task.id, {}).catch(() => null); if (res === null) return true; // a transient network blip is not a lost claim return res.ok; }; @@ -538,6 +571,7 @@ export async function deliverAndSettle(ctx: BridgeContext, task: Task): Promise< ctx.project.dir, ctx.route.timeoutMs ?? ctx.taskTimeoutMs, heartbeat, + ctx.heartbeatMs, ); break; case "inbound": diff --git a/framework/cli/src/lib/harness.ts b/framework/cli/src/lib/harness.ts index acc2909..87b8a59 100644 --- a/framework/cli/src/lib/harness.ts +++ b/framework/cli/src/lib/harness.ts @@ -40,8 +40,8 @@ * property of this computer, so they live in the framework home and are shared * by every app on it. */ -import { accessSync, constants, existsSync, statSync } from "node:fs"; -import { delimiter, isAbsolute, join } from "node:path"; +import { accessSync, constants, existsSync, readFileSync, statSync } from "node:fs"; +import { delimiter, dirname, isAbsolute, join } from "node:path"; import { homePath } from "./home.js"; import { readJsonFile } from "./json.js"; import { fetchWithTimeout } from "./net.js"; @@ -146,7 +146,27 @@ export interface HarnessConfig { * runs. */ export const BUILT_IN_HARNESSES: readonly HarnessProfile[] = [ - { id: "claude", name: "Claude Code", routes: [{ mode: "headless", command: "claude", args: ["-p", PROMPT_PLACEHOLDER] }] }, + // Headless Claude Code has nobody to approve a command, so without a grant + // every `a2app` call the prompt asks for is refused, and a determined agent + // goes looking for another way in (reading the token, editing the adapter's + // state file). The grant is exactly the CLI the prompt names, and file edits + // are denied: a delivered task is done through the app's API, not its files. + // The variadic tool lists come before `-p` so they cannot swallow the prompt. + { + id: "claude", + name: "Claude Code", + routes: [ + { + mode: "headless", + command: "claude", + args: [ + "--allowedTools", "Bash(a2app:*)", + "--disallowedTools", "Edit", "Write", "NotebookEdit", + "-p", PROMPT_PLACEHOLDER, + ], + }, + ], + }, { id: "codex", name: "Codex CLI", routes: [{ mode: "headless", command: "codex", args: ["exec", PROMPT_PLACEHOLDER] }] }, { id: "gemini", name: "Gemini CLI", routes: [{ mode: "headless", command: "gemini", args: ["-p", PROMPT_PLACEHOLDER] }] }, { id: "aider", name: "Aider", routes: [{ mode: "headless", command: "aider", args: ["--message", PROMPT_PLACEHOLDER] }] }, @@ -343,6 +363,71 @@ export function resolveOnPath(command: string, env: NodeJS.ProcessEnv = process. return null; } +/** + * How to start a resolved command without a shell. + * + * Most commands are executables and start as themselves. Windows is the + * exception that matters: an npm-installed CLI (`claude`, `codex`, `gemini`) + * is reached through a `.cmd` wrapper, and Node refuses to spawn a batch file + * without a shell (it throws EINVAL; CVE-2024-27980). Running it through + * `cmd.exe` is not an option here — the prompt carries app content, and cmd.exe + * would interpret it. So the wrapper is read instead of run: npm's shims say + * exactly which script or binary they start, and that is spawned directly, + * with the arguments still passed as an array. + * + * A batch file that is not an npm shim is refused with a reason, never run. + */ +export type LaunchSpec = { file: string; prefixArgs: string[] } | { error: string }; + +export function launchSpec(resolved: string, nodePath: string = process.execPath): LaunchSpec { + if (process.platform !== "win32" || !/\.(cmd|bat)$/i.test(resolved)) return { file: resolved, prefixArgs: [] }; + let text: string; + try { + text = readFileSync(resolved, "utf8"); + } catch (err) { + return { error: `${resolved} could not be read: ${(err as Error).message}` }; + } + return shimTarget(text, dirname(resolved), nodePath, existsSync) ?? { + error: + `${resolved} is a batch file, and the bridge never runs a harness through a shell (the prompt carries ` + + `app content). Point the route's command at the real executable instead.`, + }; +} + +/** + * What a cmd shim starts, or null if `text` is not one. Three shapes: + * npm's node-script shim (`"%_prog%" "%dp0%\…\cli.js" %*`, run with the shim's + * bundled node.exe when present, else this node), npm's native-binary shim + * (`"%dp0%\…\x.exe" %*`), and a one-line node wrapper of the kind installers + * write by hand (`node "%~dp0launcher.js" %*` — Pi's `pi.cmd`). The last is + * matched strictly: apart from `@echo off`, blank lines, comments and + * setlocal/endlocal, that line must be the whole file, so nothing else the + * batch file does is skipped by running its target directly. Exported for + * tests. + */ +export function shimTarget( + text: string, + dir: string, + nodePath: string, + exists: (p: string) => boolean, +): { file: string; prefixArgs: string[] } | null { + const at = (rel: string): string => join(dir, ...rel.split("\\").filter((s) => s !== "")); + const script = /"%_prog%"\s+"%dp0%\\([^"]+)"\s+%\*/.exec(text); + if (script) { + const bundled = join(dir, "node.exe"); + return { file: exists(bundled) ? bundled : nodePath, prefixArgs: [at(script[1]!)] }; + } + const binary = /"%dp0%\\([^"]+\.exe)"\s+%\*/i.exec(text); + if (binary) return { file: at(binary[1]!), prefixArgs: [] }; + const lines = text + .split(/\r?\n/) + .map((l) => l.trim()) + .filter((l) => l !== "" && !/^@?echo\s+off$/i.test(l) && !/^(?:rem\b|::)/i.test(l) && !/^@?(?:setlocal|endlocal)$/i.test(l)); + const wrapper = lines.length === 1 ? /^@?"?node(?:\.exe)?"?\s+"%~dp0([^"%]+)"\s+%\*$/i.exec(lines[0]!) : null; + if (wrapper) return { file: nodePath, prefixArgs: [at(wrapper[1]!)] }; + return null; +} + /* ------------------------------------------------------------- the ladder */ /** One rung, and why it is or is not usable here. */ @@ -376,14 +461,19 @@ async function inspectRoute(route: Route): Promise { } case "headless": { const resolved = resolveOnPath(route.command); - return { - mode: "headless", - available: resolved !== null, - detail: - resolved !== null - ? `${route.command} → ${resolved}` - : `${route.command} is not on PATH — install it, or point a route at it by absolute path`, - }; + if (resolved === null) { + return { + mode: "headless", + available: false, + detail: `${route.command} is not on PATH — install it, or point a route at it by absolute path`, + }; + } + // Found is not the same as startable: a batch wrapper the bridge cannot + // run without a shell has to be reported here, not discovered at delivery. + const launch = launchSpec(resolved); + if ("error" in launch) return { mode: "headless", available: false, detail: launch.error }; + const via = launch.file === resolved ? "" : ` (runs ${[launch.file, ...launch.prefixArgs].join(" ")})`; + return { mode: "headless", available: true, detail: `${route.command} → ${resolved}${via}` }; } case "gateway": { const up = await probe(route.health); diff --git a/framework/cli/test/bridge.test.mjs b/framework/cli/test/bridge.test.mjs index a5ee2fe..7875da3 100644 --- a/framework/cli/test/bridge.test.mjs +++ b/framework/cli/test/bridge.test.mjs @@ -935,6 +935,163 @@ await withApp([], async ({ port }) => { remove(dir); }); +/* --------------------- Windows: an npm .cmd shim is read, never run by cmd.exe */ + +// npm puts every CLI it installs on Windows (claude, codex, gemini) behind a +// .cmd wrapper. Node refuses to spawn one without a shell, and a shell would +// interpret the prompt. So the bridge reads what the shim starts and runs that. +{ + const { shimTarget } = await import(pathToFileURL(resolve(here, "..", "dist", "lib", "harness.js")).href); + const NODE_SHIM = [ + "@ECHO off", + "IF EXIST \"%dp0%\\node.exe\" (", + " SET \"_prog=%dp0%\\node.exe\"", + ") ELSE (", + " SET \"_prog=node\"", + ")", + 'endLocal & goto #_undefined_# 2>NUL || title %COMSPEC% & "%_prog%" "%dp0%\\node_modules\\@anthropic-ai\\claude-code\\cli.js" %*', + ].join("\r\n"); + const dir = join(tmpdir(), "npm-bin"); + check( + "an npm node shim runs its script with this node", + shimTarget(NODE_SHIM, dir, "NODE", () => false), + { file: "NODE", prefixArgs: [join(dir, "node_modules", "@anthropic-ai", "claude-code", "cli.js")] }, + ); + check( + "…or with the node.exe bundled beside it", + shimTarget(NODE_SHIM, dir, "NODE", () => true)?.file, + join(dir, "node.exe"), + ); + check( + "an npm binary shim runs the binary", + shimTarget('@ECHO off\r\n"%dp0%\\node_modules\\x\\bin\\x.exe" %*\r\n', dir, "NODE", () => false), + { file: join(dir, "node_modules", "x", "bin", "x.exe"), prefixArgs: [] }, + ); + check("any other batch file is not a shim", shimTarget("@echo off\r\ncall other.bat %*\r\n", dir, "NODE", () => false), null); + // Pi's pi.cmd, verbatim: a one-line node wrapper an installer wrote by hand. + check( + "a one-line node wrapper runs its script with this node", + shimTarget('@ECHO off\r\nnode "%~dp0pi-launcher.js" %*\r\n', dir, "NODE", () => false), + { file: "NODE", prefixArgs: [join(dir, "pi-launcher.js")] }, + ); + check( + "…and may name a script below it", + shimTarget('@echo off\r\nsetlocal\r\nrem launcher\r\nnode.exe "%~dp0lib\\cli.js" %*\r\n', dir, "NODE", () => false)?.prefixArgs, + [join(dir, "lib", "cli.js")], + ); + check( + "a wrapper that does anything else is not one", + shimTarget('@echo off\r\nset FOO=1\r\nnode "%~dp0pi-launcher.js" %*\r\n', dir, "NODE", () => false), + null, + ); +} + +if (process.platform === "win32") { + const PAYLOAD = '" & echo PWNED > pwned.txt & | ^ %PATH% "'; + + await withApp([task("tsk_shim", "summarize", { title: PAYLOAD })], async ({ port, state }) => { + const dir = makeAppDir(port); + makeHarness(dir); + // The shape npm writes, pointing at the fake harness script. + const shim = join(dir, "fakeharness.cmd"); + writeFileSync( + shim, + '@ECHO off\r\nSETLOCAL\r\nSET "_prog=node"\r\nendLocal & "%_prog%" "%dp0%\\fake-harness.mjs" %*\r\n', + ); + const home = makeHome([{ id: "fake", routes: [{ mode: "headless", command: shim, args: ["{prompt}"] }] }], "fake"); + + const status = firstJson((await cli(AGENT_APP, [dir, "bridge"], { A2APP_HOME: home })).stdout); + ok("an npm shim is reported as startable", status?.ladder?.[0]?.available === true); + + const res = await cli(AGENT_APP, [dir, "bridge", "start", "--once"], { A2APP_HOME: home }); + check("a harness behind an npm .cmd shim is delivered to", res.code, 0); + check("…and the task completes", state.get("tsk_shim").status, "completed"); + const argv = existsSync(join(dir, "argv.json")) ? JSON.parse(readFileSync(join(dir, "argv.json"), "utf8")) : []; + check("…with the prompt still exactly one argument", argv.length, 1); + // The prompt renders the payload as JSON, so its quotes arrive escaped. + ok("…carrying cmd.exe metacharacters verbatim", String(argv[0]).includes(JSON.stringify(PAYLOAD).slice(1, -1))); + ok("…and no shell ran them", !existsSync(join(dir, "pwned.txt")) && !existsSync("pwned.txt")); + rmSync(home, { recursive: true, force: true }); + rmSync(dir, { recursive: true, force: true }); + }); + + await withApp([task("tsk_bat", "summarize", {})], async ({ port, state }) => { + const dir = makeAppDir(port); + const bat = join(dir, "wrapper.bat"); + writeFileSync(bat, "@echo off\r\ncall something-else %*\r\n"); + const home = makeHome([{ id: "fake", routes: [{ mode: "headless", command: bat, args: ["{prompt}"] }] }], "fake"); + + const status = firstJson((await cli(AGENT_APP, [dir, "bridge"], { A2APP_HOME: home })).stdout); + ok("a batch file that is not a shim is reported as not startable", status?.ladder?.[0]?.available === false); + + // Refused before anything is claimed, so the task stays free for a listener + // that can run it, instead of sitting claimed until redelivery runs out. + const res = await cli(AGENT_APP, [dir, "bridge", "start", "--once"], { A2APP_HOME: home }); + ok("the bridge refuses to start", res.code !== 0 && firstJson(res.stdout)?.supported === false); + check("…and never claims the task", state.get("tsk_bat").status, "submitted"); + rmSync(home, { recursive: true, force: true }); + rmSync(dir, { recursive: true, force: true }); + }); +} + +/* ------------------------ the heartbeat keeps the claim, not the conversation */ + +// An agent that reports its own step is telling the person watching the app +// what it is doing. A heartbeat that overwrote it every 20 s with "running in +// " made the app flip between the two for the whole run. +{ + const { deliverAndSettle } = await import(pathToFileURL(resolve(here, "..", "dist", "lib", "bridge.js")).href); + const dir = mkdtempSync(join(tmpdir(), "a2app-bridge-beat-")); + const slow = join(dir, "slow-harness.mjs"); + writeFileSync(slow, "setTimeout(() => process.exit(0), 700);\n"); + const progress = []; + const client = { + progressTask: async (_id, body) => (progress.push(body), { ok: true, status: 200, json: {} }), + getTask: async () => ({ ok: true, status: 200, json: { status: "working" } }), + completeTask: async () => ({ ok: true, status: 200, json: {} }), + }; + await deliverAndSettle( + { + project: { dir }, + client, + profile: { id: "fake", name: "Fake", routes: [] }, + route: { mode: "headless", command: process.execPath, args: [slow, "{prompt}"] }, + prompt: { appRef: dir, appName: "Beat", appId: "beat", closesOnExit: true, cwdIsApp: true }, + taskTimeoutMs: 10_000, + heartbeatMs: 100, + capabilities: null, + dryRun: false, + }, + task("tsk_beat", "summarize", {}), + ); + ok("the run announced who picked it up", String(progress[0]?.step ?? "").startsWith("delivered to fake")); + const beats = progress.slice(1); + ok("the claim was kept alive while it ran", beats.length >= 2); + check("…by heartbeats that carry no step", beats.filter((b) => "step" in b).length, 0); + rmSync(dir, { recursive: true, force: true }); +} + +/* ------------------- a harness that cannot be started fails the task at once */ + +// spawn() throws synchronously for some failures (EINVAL on Windows batch +// files, a NUL byte in an argument everywhere), before any 'error' event +// exists. That used to escape the delivery and leave the task claimed with +// nobody running it. +await withApp([task("tsk_throw", "summarize", {})], async ({ port, state }) => { + const dir = makeAppDir(port); + const harness = makeHarness(dir); + const home = makeHome( + [{ id: "fake", routes: [{ mode: "headless", command: process.execPath, args: [harness, "{prompt}", "bad\u0000arg"] }] }], + "fake", + ); + const res = await cli(AGENT_APP, [dir, "bridge", "start", "--once"], { A2APP_HOME: home }); + check("a harness that cannot be started fails the pass", res.code, 1); + check("…and the task, at once", state.get("tsk_throw").status, "failed"); + check("…saying it could not be started", state.get("tsk_throw").reason, "harness_spawn_failed"); + rmSync(home, { recursive: true, force: true }); + rmSync(dir, { recursive: true, force: true }); +}); + /* -------------------------------------------------------------------- report */ if (failures.length > 0) {