From 7a9f2208d374a7068d52d51ac361b53fd3d1cfce Mon Sep 17 00:00:00 2001 From: Khaliq Date: Tue, 25 Aug 2026 11:42:31 +0200 Subject: [PATCH 1/8] fix(agent37): name an unusable workdir instead of reporting a failed command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `composeScript` emitted `cd || exit 1`, which is indistinguishable from the command's own exit 1. That ambiguity has already produced a wrong finding: a lane pointed an Agent37 instance at `workdir: '/root'`, watched ten unrelated probes all return exit 1, and recorded that `/root` did not exist. It exists — `drwx------ root root`, unreachable by the template's `node` user (uid 1000) — and every one of those exit 1s was the `cd`, not the command. Re-pointed at `/home/node`, all ten passed. The `cd` now writes a marker to stderr and exits a sentinel status, and `runScript` reclassifies that pair into `Agent37WorkdirUnusableError`, which names the instance and the directory. Both signals are required before reclassifying: a command is free to exit 191, and a command is free to print the marker, so neither alone is proof. The emitted line is POSIX `sh` rather than bash, because Agent37's exec plane is dash — measured, not assumed: `${PIPESTATUS[0]}` comes back there as `Bad substitution`. Co-Authored-By: Claude Opus 5 Session-Id: 267222b9-fa00-4f67-9a09-e7cb4021c651 --- src/agent37/index.ts | 3 ++ src/agent37/runtime.test.ts | 65 +++++++++++++++++++++++++++++- src/agent37/runtime.ts | 79 ++++++++++++++++++++++++++++++++++++- src/index.ts | 3 ++ 4 files changed, 147 insertions(+), 3 deletions(-) diff --git a/src/agent37/index.ts b/src/agent37/index.ts index 8645d4f..601be68 100644 --- a/src/agent37/index.ts +++ b/src/agent37/index.ts @@ -1,5 +1,7 @@ export { AGENT37_COMMAND_CAP_MS, + AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, + AGENT37_WORKDIR_UNUSABLE_MARKER, Agent37CommandTimeoutUnsupportedError, Agent37CreateTimeoutUnsupportedError, Agent37EnvValidationError, @@ -7,6 +9,7 @@ export { Agent37MalformedResponseError, Agent37Runtime, Agent37UnknownExitCodeError, + Agent37WorkdirUnusableError, } from "./runtime.js"; export type { Agent37BundleFile, diff --git a/src/agent37/runtime.test.ts b/src/agent37/runtime.test.ts index 00c5124..3ef1398 100644 --- a/src/agent37/runtime.test.ts +++ b/src/agent37/runtime.test.ts @@ -915,7 +915,8 @@ describe("Agent37Runtime.runScript", () => { ); assert.equal( parsed.command, - "cd '/work/repo' || exit 1\nexport TOKEN_NAME='it'\\''s fine'\nnpm test\n", + "cd '/work/repo' || { printf '%s\\n' '__agent37_workdir_unusable__' >&2; exit 191; }\n" + + "export TOKEN_NAME='it'\\''s fine'\nnpm test\n", ); assert.deepEqual(result, { output: "ok\n", stdout: "ok\n", exitCode: 0 }); }); @@ -924,11 +925,71 @@ describe("Agent37Runtime.runScript", () => { const h = harness(() => ({ json: { exit_code: 0, stdout: "", stderr: "" } })); const runtime = makeRuntime(h); await runtime.runScript({ ...RUNNING_HANDLE, workdir: "/from/handle" }, { command: "ls" }); - assert.match(execCommand(h.requests[0] as RecordedRequest), /^cd '\/from\/handle' \|\| exit 1\n/); + assert.match(execCommand(h.requests[0] as RecordedRequest), /^cd '\/from\/handle' \|\| \{ printf /); await runtime.runScript(RUNNING_HANDLE, { command: "ls" }); assert.equal(execCommand(h.requests[1] as RecordedRequest), "ls\n"); }); + it("names an unusable workdir instead of reporting it as a failed command", async () => { + // The regression this guards: ten unrelated probes against `workdir: + // '/root'` all came back exit 1, and the lane concluded /root did not + // exist. It exists — root-owned, mode 0700, unreachable by the template's + // `node` user — and every exit 1 was the `cd`. + const h = harness(() => ({ + json: { + exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, + stdout: "", + stderr: `${pkg.AGENT37_WORKDIR_UNUSABLE_MARKER}\nsh: 1: cd: can't cd to /root\n`, + }, + })); + const runtime = makeRuntime(h); + await assert.rejects( + runtime.runScript(RUNNING_HANDLE, { command: "id", cwd: "/root" }), + (error: unknown) => { + assert.ok(error instanceof pkg.Agent37WorkdirUnusableError); + assert.equal(error.instanceId, "ab12cd34ef"); + assert.equal(error.workdir, "/root"); + assert.match(error.output, /can't cd to \/root/); + return true; + }, + ); + }); + + it("requires BOTH the sentinel status and the marker before reclassifying", async () => { + // Either signal alone belongs to the command, not to the `cd`. A command + // is free to exit 191, and a command is free to print the marker. + const statusOnly = harness(() => ({ + json: { exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, stdout: "", stderr: "boom" }, + })); + const first = await makeRuntime(statusOnly).runScript(RUNNING_HANDLE, { + command: "exit 191", + cwd: "/work", + }); + assert.equal(first.exitCode, pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE); + + const markerOnly = harness(() => ({ + json: { exit_code: 1, stdout: pkg.AGENT37_WORKDIR_UNUSABLE_MARKER, stderr: "" }, + })); + const second = await makeRuntime(markerOnly).runScript(RUNNING_HANDLE, { + command: "echo marker", + cwd: "/work", + }); + assert.equal(second.exitCode, 1); + }); + + it("does not reclassify when no cwd was requested, because no cd was emitted", async () => { + const h = harness(() => ({ + json: { + exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, + stdout: "", + stderr: pkg.AGENT37_WORKDIR_UNUSABLE_MARKER, + }, + })); + const runtime = makeRuntime(h, { defaultHomeDir: "/home/node" }); + const result = await runtime.runScript(RUNNING_HANDLE, { command: "sh -c 'exit 191'" }); + assert.equal(result.exitCode, pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE); + }); + it("reports a nonzero exit as a result, not an error, and combines the streams", async () => { const h = harness(() => ({ json: { exit_code: 2, stdout: "partial", stderr: "boom", truncated: true }, diff --git a/src/agent37/runtime.ts b/src/agent37/runtime.ts index ef08e5d..f8000f9 100644 --- a/src/agent37/runtime.ts +++ b/src/agent37/runtime.ts @@ -300,6 +300,63 @@ export class Agent37UnknownExitCodeError extends Error { } } +/** + * Marker the composed script writes to stderr when its `cd` fails, and the + * exit status it leaves behind. + * + * Both exist because a bare `cd || exit 1` is *indistinguishable from the + * command's own failure*, and that ambiguity has already cost real time: a + * lane pointed an Agent37 instance at `workdir: '/root'`, watched ten + * unrelated probes all return exit 1, and concluded from it that `/root` did + * not exist. It does exist — as `drwx------ root root`, unreachable by the + * template's `node` user — and every one of those exit 1s was the `cd`, not + * the command. Re-pointed at the real home, all ten passed. + * + * The status alone is not proof (a command may legitimately exit 191), and the + * marker alone is not proof (a command may legitimately print it), so + * {@link Agent37Runtime.runScript} requires both before it reclassifies a + * result as a workdir fault. + */ +export const AGENT37_WORKDIR_UNUSABLE_EXIT_CODE = 191; + +/** @see AGENT37_WORKDIR_UNUSABLE_EXIT_CODE */ +export const AGENT37_WORKDIR_UNUSABLE_MARKER = "__agent37_workdir_unusable__"; + +/** + * Raised when the instance could not enter the requested working directory. + * + * This is a *configuration* fault, not a command failure: nothing the caller + * asked to run ever ran. Reporting it as `exitCode: 191` would leave the + * caller to infer that from stderr, which is exactly the inference that got + * made wrongly before. So it is named instead. + * + * The most common cause is a `workdir` that belongs to another user. Agent37's + * template runs as `node` (uid 1000) with `HOME=/home/node`; `/root` exists but + * is mode 0700 and owned by root, so pointing a launch at it fails every + * command on the box. See `docs/agent37.md`. + */ +export class Agent37WorkdirUnusableError extends Error { + /** Instance the command was sent to. */ + readonly instanceId: string; + /** The directory the script could not enter. */ + readonly workdir: string; + /** Whatever the shell said, kept so the underlying reason is not lost. */ + readonly output: string; + + constructor(instanceId: string, workdir: string, output: string) { + super( + `Agent37 could not enter working directory "${workdir}" on instance ` + + `"${instanceId}", so the command never ran. Check that the directory ` + + `exists and is readable by the template's user — the Agent37 template ` + + `runs as "node" with HOME=/home/node, and /root is root-owned mode 0700.`, + ); + this.name = "Agent37WorkdirUnusableError"; + this.instanceId = instanceId; + this.workdir = workdir; + this.output = output; + } +} + /** * Raised when a caller asks for a command lifetime Agent37 cannot enforce. * @@ -780,6 +837,19 @@ export class Agent37Runtime implements SandboxRuntime, WorkflowRuntime { // satisfies it — turning it into an HTTP abort would abandon the response // while the command ran on. const result = await this.execRaw(handle.id, script, options.requestTimeoutMs); + // Reclassify before the result is shaped: a command that never ran must not + // be reported as a command that ran and failed. + if ( + cwd && + result.exit_code === AGENT37_WORKDIR_UNUSABLE_EXIT_CODE && + combineOutput(result.stdout, result.stderr).includes(AGENT37_WORKDIR_UNUSABLE_MARKER) + ) { + throw new Agent37WorkdirUnusableError( + handle.id, + cwd, + combineOutput(result.stdout, result.stderr), + ); + } return { output: combineOutput(result.stdout, result.stderr), ...(result.stdout ? { stdout: result.stdout } : {}), @@ -1208,7 +1278,14 @@ export function composeScript( ): string { const lines: string[] = []; if (options.cwd) { - lines.push(`cd ${shellQuote(options.cwd)} || exit 1`); + // POSIX `sh`, not bash: Agent37's exec plane runs dash, where a bashism + // like ${PIPESTATUS[0]} is a "Bad substitution". `{ …; }` and `printf` are + // both POSIX, so this line runs on either shell. + lines.push( + `cd ${shellQuote(options.cwd)} || { printf '%s\\n' ` + + `${shellQuote(AGENT37_WORKDIR_UNUSABLE_MARKER)} >&2; ` + + `exit ${AGENT37_WORKDIR_UNUSABLE_EXIT_CODE}; }`, + ); } for (const [key, value] of Object.entries(options.env ?? {})) { lines.push(`export ${key}=${shellQuote(value)}`); diff --git a/src/index.ts b/src/index.ts index dd4cb2d..081d1f8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -76,6 +76,8 @@ export type { export { AGENT37_COMMAND_CAP_MS, + AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, + AGENT37_WORKDIR_UNUSABLE_MARKER, Agent37CommandTimeoutUnsupportedError, Agent37CreateTimeoutUnsupportedError, Agent37EnvValidationError, @@ -83,6 +85,7 @@ export { Agent37MalformedResponseError, Agent37Runtime, Agent37UnknownExitCodeError, + Agent37WorkdirUnusableError, } from "./agent37/runtime.js"; export type { Agent37BundleFile, From 2adff0a9f98fc1c38ef445608c5a98ac516ef2c7 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Tue, 25 Aug 2026 11:42:46 +0200 Subject: [PATCH 2/8] feat(bootstrap): close the three measured sandbox bootstrap gaps in userspace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A live Agent37 run found three things missing between a bare image and a box an agent can work on. None of the three needs root, and none of the three is a provider fault. - `buildRelayfileMountLinkShell` puts `relayfile-mount` on PATH from the copy `agent-relay` already vendors as `@relayfile/mount--`. No download. Resolution walks up from the package directory rather than joining a fixed path, because npm is free to hoist that dependency. - `buildGhInstallShell` installs `gh` from the vendor's release tarball into a user-writable directory; measured at about three seconds, moving `gh --version` from exit 127 to exit 0. `gh auth status` then returns exit 1, which is a credential failure and deliberately not this snippet's job. An optional `sha256` is verified before anything is extracted. - `buildClaudeConfigSeedShell` completes Claude Code's first-run onboarding and approves the key the agent will use. A freshly spawned agent came up on the OAuth screen while holding a live key, because a clean `~/.claude.json` had recorded that key's tail under `customApiKeyResponses.rejected`. The seeder moves the tail out of `rejected` into `approved`, so it repairs an already poisoned config rather than only helping a pristine one, and it reads the key from an env var inside the sandbox so no credential is rendered into the built command. Every snippet is POSIX `sh`: Agent37's exec plane is dash. The tests execute the generated shell under `/bin/sh` rather than string-matching it — including a fabricated npm root for the hoisted and nested layouts, a `file://` release tarball, a checksum mismatch, and a home directory whose name contains shell metacharacters. `docs/agent37.md` records the measured template: `defaultHomeDir` must be `/home/node`; `mount | grep relayfile` is empty on a healthy box on both providers and is not a valid test; and `mkdir /opt` is denied on Daytona too, so it is not an Agent37 property and should stop being logged as one. Co-Authored-By: Claude Opus 5 Session-Id: 267222b9-fa00-4f67-9a09-e7cb4021c651 --- docs/agent37.md | 140 +++++++++++++ package.json | 1 + src/bootstrap.test.ts | 443 ++++++++++++++++++++++++++++++++++++++++++ src/bootstrap.ts | 382 ++++++++++++++++++++++++++++++++++++ src/core/index.ts | 3 + 5 files changed, 969 insertions(+) create mode 100644 docs/agent37.md create mode 100644 src/bootstrap.test.ts create mode 100644 src/bootstrap.ts diff --git a/docs/agent37.md b/docs/agent37.md new file mode 100644 index 0000000..2f5bc50 --- /dev/null +++ b/docs/agent37.md @@ -0,0 +1,140 @@ +# Agent37 adapter + +Everything below was measured on a live Agent37 instance on 2026-08-25, on the +provider's default system template. Where a fact is shared with another +provider it says so, because two of the things previously logged as "Agent37 +defects" are not properties of Agent37 at all. + +## The template, and the one setting that matters + +``` +user node (uid 1000, gid 1000) +HOME /home/node +cwd / (the exec plane's default when no cwd is given) +os Debian GNU/Linux 12 (bookworm) +kernel 4.19.0-gvisor +shape 2 vCPU · 4096 MB · 10 GB +shell sh (dash) — NOT bash +``` + +**Construct `Agent37Runtime` with `defaultHomeDir: "/home/node"`.** This is the +single most consequential line in an Agent37 integration. + +`/root` **exists** — as `drwx------ root root` — and the template user cannot +enter it. So a launch that sets `workdir: '/root'` does not fail once, it fails +*every command on the box*, identically: + +``` +$ id; echo PWD=$PWD → exit 1 +sh: 1: cd: can't cd to /root +``` + +Ten unrelated probes were run that way and all ten returned exit 1. Re-pointed +at `/home/node`, all ten passed. Note the correction to an earlier note: `/root` +is **unreachable, not missing**, and the distinction matters because "missing" +sends you looking at the image while "unreachable" sends you to the one line of +caller configuration that actually causes it. + +Since a bare `cd … || exit 1` is indistinguishable from the command's own exit +1, the adapter no longer emits one. A failed `cd` now raises +`Agent37WorkdirUnusableError`, which names the directory and the instance. See +`AGENT37_WORKDIR_UNUSABLE_EXIT_CODE`. + +## The exec plane is dash + +`agent37`'s exec runs POSIX `sh`, not bash. A bashism does not error usefully — +`${PIPESTATUS[0]}` comes back as `Bad substitution`, so a step that looks like +it succeeded quietly did nothing. Anything this package generates for an +Agent37 box is POSIX; anything a caller generates should be too. + +## There is no root, and that is not an Agent37 property + +`sudo` is inert (`effective uid is not 0 … nosuid`) and `mkdir /opt/` +returns `Permission denied`. + +**The same `mkdir /opt` is denied on Daytona**, measured in the same run, so it +should stop being recorded as an Agent37 defect. Nothing needs root: npm's +global prefix is already `/home/node/.npm-global` and already on PATH, and +`/home/node`, `/home/node/.local/bin` and `/tmp` are all writable. + +## What the image does not ship + +| | Agent37 | Daytona | +| --- | --- | --- | +| `node` / `npm` | v24.19.0 / 11.17.0 | present | +| `git`, `curl`, `ssh`, `python3` | present | present | +| `gh` | **absent** — `gh --version` → **exit 127** | **absent** — **exit 127** | +| `relayfile-mount` | **absent from PATH** | `/usr/local/bin/relayfile-mount` | +| `agent-relay` | absent (installs from npm in ~46 s) | present in the image | + +Egress is open: `registry.npmjs.org` and the Agent Relay control plane both +answered `200`. + +Both gaps close in userspace with no root — see `src/bootstrap.ts`: + +- `buildRelayfileMountLinkShell` symlinks the daemon that `agent-relay` already + vendors as `@relayfile/mount--`. No download. +- `buildGhInstallShell` drops a release tarball into `~/.local/bin`; measured at + about three seconds, moving `gh --version` from exit 127 to exit 0. + +`gh auth status` then returns **exit 1** ("not logged into any GitHub hosts"). +Keep the two apart when reporting: a present binary with no credential is a +different failure from a missing binary, and only the second is exit 127. + +## Relayfile mount: `mount | grep` is not a test + +`relayfile-mount` is a **userspace sync daemon**, not a kernel or FUSE mount. +On a completely healthy Agent37 box: + +``` +$ mount | grep -i relayfile → exit 1, no output +``` + +The identical empty result comes back on Daytona, where the mount is in daily +production use. Two lanes drew a false conclusion from this check. + +Test it by moving bytes instead. Measured end to end on Agent37: a file written +inside the instance was read on a laptop (`exit 0`, byte-identical), a file +written on the laptop was read inside the instance (`exit 0`), and a third +machine on the same scope saw both. The daemon's own +`/.relay/state.json` is the honest instrument — `status`, the `files` +map, and `outbox` (`pending` / `failed` / `acked`). + +The gVisor kernel is the reason this shape is right: a FUSE mount is not +available, and a userspace mirror is unaffected. + +## Placing an agent + +A targeted spawn must name its working directory. `worker_cwd` is node-relative +and a `--node` spawn sets none, so without `--cwd` the agent lands at the +broker's project root rather than its workspace — and a tree at a path the +agent was never placed in is indistinguishable, from inside, from a missing +tree. With `--cwd` passed, `readlink /proc//cwd` confirmed the requested +directory for both the broker's PTY process and the agent process. + +Cross-node attach works with no ssh, but needs a **real PTY**: `script` fails on +non-tty stdin with `tcgetattr/ioctl: Operation not supported on socket`. +Allocate one (Python's `pty.fork()` will do) and read the bytes — 18,323 bytes +of live screen came back over a `--mode view` attach. + +Two harness gaps to expect on any fresh box, neither provider-specific: + +- A clean `~/.claude.json` records a valid API key's tail under + `customApiKeyResponses.rejected`, so the agent boots to an OAuth screen while + holding a working credential. `buildClaudeConfigSeedShell` approves the tail + and completes onboarding, and repairs an already-poisoned config. +- `relay node up` may resolve a different workspace than the one + `relay cloud enroll` bound the node to, which makes an in-box roster read + return a single entry — the node's own name. That is a platform issue, not a + provider one; it reproduces on Daytona. + +## Teardown + +Delete is synchronous enough to verify immediately: `destroy` returned in +7,242 ms and `GET /v1/instances` was empty 317 ms later, across three separate +runs with no leaked instance. + +**Daytona is not**: `destroy` returned in 131 ms there and an immediate +`getById` still resolved the sandbox, which was gone from the provider's list +moments later. A read-back straight after delete is not a valid "verified gone" +check on that provider. diff --git a/package.json b/package.json index 986afe9..feaf14c 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,7 @@ }, "files": [ "dist", + "docs/agent37.md", "docs/freestyle.md", "README.md", "LICENSE", diff --git a/src/bootstrap.test.ts b/src/bootstrap.test.ts new file mode 100644 index 0000000..7c4a65b --- /dev/null +++ b/src/bootstrap.test.ts @@ -0,0 +1,443 @@ +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { chmod, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, it } from "node:test"; +import { promisify } from "node:util"; + +import { + GH_RELEASE_BASE_URL, + buildClaudeConfigSeedShell, + buildGhInstallShell, + buildRelayfileMountLinkShell, +} from "./index.js"; + +const run = promisify(execFile); + +/** + * These snippets are shipped to run inside a sandbox, so most of them are + * *executed* here rather than string-matched. A builder whose output only ever + * gets compared to an expected string will keep passing after it stops working + * — which is the failure mode this package has already paid for once, when a + * bad `cd` looked exactly like a bad command. + * + * Everything runs under `sh` (not bash) for the same reason the builders target + * POSIX: Agent37's exec plane is dash. + */ +async function sh( + script: string, + options: { env?: Record; cwd?: string } = {}, +): Promise<{ stdout: string; stderr: string; code: number }> { + try { + const { stdout, stderr } = await run("/bin/sh", ["-c", script], { + env: { PATH: process.env.PATH ?? "", ...options.env }, + ...(options.cwd ? { cwd: options.cwd } : {}), + maxBuffer: 8 * 1024 * 1024, + }); + return { stdout, stderr, code: 0 }; + } catch (error) { + const failure = error as { stdout?: string; stderr?: string; code?: number }; + return { stdout: failure.stdout ?? "", stderr: failure.stderr ?? "", code: failure.code ?? 1 }; + } +} + +async function scratch(): Promise { + return mkdtemp(join(tmpdir(), "sandbox-bootstrap-")); +} + +const PLATFORM_PACKAGE = `@relayfile/mount-${process.platform}-${process.arch}`; + +/** Lay down a fake global npm root containing an `agent-relay` install. */ +async function fakeGlobalRoot( + root: string, + options: { hoisted?: boolean } = {}, +): Promise { + const globalRoot = join(root, "lib", "node_modules"); + // Hoisted: npm lifted the platform package to the top level, beside + // `agent-relay`, instead of nesting it. Both layouts happen in the wild. + const packageHost = options.hoisted + ? globalRoot + : join(globalRoot, "agent-relay"); + const binDir = join(packageHost, "node_modules", PLATFORM_PACKAGE, "bin"); + await mkdir(binDir, { recursive: true }); + await mkdir(join(globalRoot, "agent-relay"), { recursive: true }); + const binary = join(binDir, "relayfile-mount"); + await writeFile(binary, "#!/bin/sh\nprintf 'fake-relayfile-mount\\n'\n"); + await chmod(binary, 0o755); + return globalRoot; +} + +describe("buildRelayfileMountLinkShell", () => { + it("links the vendored binary onto PATH without downloading anything", async () => { + const root = await scratch(); + try { + const globalRoot = await fakeGlobalRoot(root); + const binDir = join(root, "bin"); + const script = buildRelayfileMountLinkShell({ binDir, searchRoots: [globalRoot] }); + + assert.ok( + !/curl|wget|npm install/.test(script), + "the whole point is that the binary is already on the box", + ); + + const result = await sh(script, { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout.trim(), new RegExp(`${PLATFORM_PACKAGE}/bin/relayfile-mount$`)); + + // The link is only useful if invoking the bare name works. + const invoked = await sh("relayfile-mount", { + env: { PATH: `${binDir}:${process.env.PATH ?? ""}` }, + }); + assert.equal(invoked.stdout.trim(), "fake-relayfile-mount"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("finds the binary when npm hoisted it above the agent-relay package", async () => { + const root = await scratch(); + try { + const globalRoot = await fakeGlobalRoot(root, { hoisted: true }); + const binDir = join(root, "bin"); + const result = await sh( + buildRelayfileMountLinkShell({ binDir, searchRoots: [globalRoot] }), + { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }, + ); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout.trim(), /bin\/relayfile-mount$/); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("fails loudly when no vendored binary exists, rather than leaving a broken box", async () => { + const root = await scratch(); + try { + const empty = join(root, "empty"); + await mkdir(empty, { recursive: true }); + const binDir = join(root, "bin"); + const result = await sh( + buildRelayfileMountLinkShell({ binDir, searchRoots: [empty] }), + { env: { PATH: `${binDir}:/usr/bin:/bin` } }, + ); + assert.notEqual(result.code, 0); + assert.match(result.stderr, /relayfile-mount not found/); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("refuses a relative bin directory and a link name with a path separator", () => { + assert.throws(() => buildRelayfileMountLinkShell({ binDir: "bin" }), /absolute path/); + assert.throws( + () => buildRelayfileMountLinkShell({ binDir: "/tmp/bin", linkName: "a/b" }), + /bare filename/, + ); + }); +}); + +describe("buildGhInstallShell", () => { + /** A stand-in for a GitHub release tarball, served over `file://`. */ + async function fakeRelease( + root: string, + version: string, + ): Promise<{ baseUrl: string; sha256: string }> { + const arch = process.arch === "arm64" ? "arm64" : "amd64"; + const name = `gh_${version}_linux_${arch}`; + const stage = join(root, "stage", name, "bin"); + await mkdir(stage, { recursive: true }); + await writeFile(join(stage, "gh"), `#!/bin/sh\nprintf 'gh version ${version}\\n'\n`); + await chmod(join(stage, "gh"), 0o755); + const releases = join(root, "releases", `v${version}`); + await mkdir(releases, { recursive: true }); + const tarball = join(releases, `${name}.tar.gz`); + await run("tar", ["-czf", tarball, "-C", join(root, "stage"), name]); + const { stdout } = await run("/bin/sh", [ + "-c", + `if command -v sha256sum >/dev/null 2>&1; then sha256sum '${tarball}'; else shasum -a 256 '${tarball}'; fi`, + ]); + return { baseUrl: `file://${join(root, "releases")}`, sha256: stdout.trim().split(/\s+/)[0]! }; + } + + it("installs gh into a user directory and leaves it on PATH", async () => { + const root = await scratch(); + try { + const { baseUrl } = await fakeRelease(root, "9.9.9"); + const binDir = join(root, "bin"); + const workDir = join(root, "work"); + const result = await sh( + buildGhInstallShell({ version: "9.9.9", binDir, workDir, releaseBaseUrl: baseUrl }), + { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }, + ); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout, /gh version 9\.9\.9/); + // No root anywhere in the generated shell: that is the whole constraint. + assert.ok(!/\bsudo\b/.test(result.stdout)); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("aborts before extracting when the checksum does not match", async () => { + const root = await scratch(); + try { + const { baseUrl } = await fakeRelease(root, "9.9.9"); + const binDir = join(root, "bin"); + const result = await sh( + buildGhInstallShell({ + version: "9.9.9", + binDir, + workDir: join(root, "work"), + releaseBaseUrl: baseUrl, + sha256: "0".repeat(64), + }), + { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }, + ); + assert.notEqual(result.code, 0); + assert.match(result.stderr, /failed SHA-256 verification/); + const listed = await sh(`ls ${binDir} 2>/dev/null || true`); + assert.equal(listed.stdout.trim(), "", "nothing may be installed from an unverified tarball"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("accepts the matching checksum", async () => { + const root = await scratch(); + try { + const { baseUrl, sha256 } = await fakeRelease(root, "9.9.9"); + const binDir = join(root, "bin"); + const result = await sh( + buildGhInstallShell({ + version: "9.9.9", + binDir, + workDir: join(root, "work"), + releaseBaseUrl: baseUrl, + sha256, + }), + { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }, + ); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout, /gh version 9\.9\.9/); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("defaults to the vendor's own release host and pins no version", () => { + assert.equal(GH_RELEASE_BASE_URL, "https://github.com/cli/cli/releases/download"); + assert.match( + buildGhInstallShell({ version: "2.82.1", binDir: "/home/node/.local/bin" }), + /https:\/\/github\.com\/cli\/cli\/releases\/download/, + ); + assert.throws(() => buildGhInstallShell({ version: "", binDir: "/x" }), /required/); + assert.throws(() => buildGhInstallShell({ version: "v2.82.1", binDir: "/x" }), /leading "v"/); + assert.throws( + () => buildGhInstallShell({ version: "2.82.1", binDir: "/x", sha256: "nope" }), + /64 lowercase hex/, + ); + }); +}); + +describe("buildClaudeConfigSeedShell", () => { + async function seed( + configPath: string, + env: Record, + options: Parameters[0] = { configPath }, + ) { + return sh(buildClaudeConfigSeedShell({ ...options, configPath }), { env }); + } + + it("creates a config that completes onboarding and approves the live key", async () => { + const root = await scratch(); + try { + const configPath = join(root, "home", ".claude.json"); + const result = await seed(configPath, { ANTHROPIC_API_KEY: `sk-ant-${"k".repeat(40)}` }); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout, /apiKeyApproved=1/); + + const cfg = JSON.parse(await readFile(configPath, "utf8")) as Record; + assert.equal(cfg.hasCompletedOnboarding, true); + const responses = cfg.customApiKeyResponses as { approved: string[]; rejected: string[] }; + assert.deepEqual(responses.approved, ["k".repeat(20)]); + assert.deepEqual(responses.rejected, []); + // The credential itself must never reach disk here. + assert.ok(!JSON.stringify(cfg).includes("sk-ant-")); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("never renders the key into the built command", () => { + const script = buildClaudeConfigSeedShell({ configPath: "/home/node/.claude.json" }); + assert.ok(!script.includes("sk-ant")); + assert.match(script, /\$\{ANTHROPIC_API_KEY:-\}/); + }); + + it("repairs a box whose config already rejected the key", async () => { + const root = await scratch(); + try { + const configPath = join(root, ".claude.json"); + const tail = "z".repeat(20); + await writeFile( + configPath, + JSON.stringify({ + machineID: "keep-me", + customApiKeyResponses: { approved: [], rejected: [tail, "other-tail"] }, + }), + ); + const result = await seed(configPath, { ANTHROPIC_API_KEY: `sk-ant-abc${tail}` }); + assert.equal(result.code, 0, result.stderr); + + const cfg = JSON.parse(await readFile(configPath, "utf8")) as Record; + assert.equal(cfg.machineID, "keep-me", "existing state must survive the merge"); + const responses = cfg.customApiKeyResponses as { approved: string[]; rejected: string[] }; + assert.deepEqual(responses.approved, [tail]); + assert.deepEqual( + responses.rejected, + ["other-tail"], + "only this key moves; another key's rejection is not ours to undo", + ); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("still completes onboarding when no key is present, and says so", async () => { + const root = await scratch(); + try { + const configPath = join(root, ".claude.json"); + const result = await seed(configPath, {}); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout, /apiKeyApproved=0/); + const cfg = JSON.parse(await readFile(configPath, "utf8")) as Record; + assert.equal(cfg.hasCompletedOnboarding, true); + assert.equal(cfg.customApiKeyResponses, undefined); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("writes the config 0600 and records the onboarding version when given one", async () => { + const root = await scratch(); + try { + const configPath = join(root, ".claude.json"); + const result = await sh( + buildClaudeConfigSeedShell({ configPath, onboardingVersion: "2.1.245" }), + { env: { ANTHROPIC_API_KEY: "sk-ant-" + "q".repeat(30) } }, + ); + assert.equal(result.code, 0, result.stderr); + const cfg = JSON.parse(await readFile(configPath, "utf8")) as Record; + assert.equal(cfg.lastOnboardingVersion, "2.1.245"); + const mode = await sh(`ls -l ${configPath} | cut -c1-10`); + assert.equal(mode.stdout.trim(), "-rw-------"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("refuses to clobber a config file that is not a JSON object", async () => { + const root = await scratch(); + try { + const configPath = join(root, ".claude.json"); + await writeFile(configPath, "[1,2,3]"); + const result = await seed(configPath, {}); + assert.notEqual(result.code, 0); + assert.match(result.stderr, /not a JSON object/); + assert.equal(await readFile(configPath, "utf8"), "[1,2,3]"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("rejects a relative path and an env var that is not an identifier", () => { + assert.throws(() => buildClaudeConfigSeedShell({ configPath: ".claude.json" }), /absolute/); + assert.throws( + () => buildClaudeConfigSeedShell({ configPath: "/x/.claude.json", apiKeyEnvVar: "a-b" }), + /shell identifier/, + ); + }); + + it("survives a home directory whose name contains shell metacharacters", async () => { + const root = await scratch(); + try { + const home = join(root, "it's $(rm -rf); weird"); + await mkdir(home, { recursive: true }); + const configPath = join(home, ".claude.json"); + const result = await seed(configPath, {}); + assert.equal(result.code, 0, result.stderr); + const cfg = JSON.parse(await readFile(configPath, "utf8")) as Record; + assert.equal(cfg.hasCompletedOnboarding, true); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +}); + +describe("bootstrap snippets are POSIX sh, not bash", () => { + it("uses no bashism that dash would reject", () => { + // Measured: Agent37's exec plane is dash, where `${PIPESTATUS[0]}` is a + // "Bad substitution" — a step that looks fine and silently does nothing. + const scripts = [ + buildRelayfileMountLinkShell({ binDir: "/home/node/.local/bin" }), + buildGhInstallShell({ version: "2.82.1", binDir: "/home/node/.local/bin" }), + buildClaudeConfigSeedShell({ configPath: "/home/node/.claude.json" }), + ]; + for (const script of scripts) { + assert.ok(!script.includes("PIPESTATUS"), script); + assert.ok(!/\[\[/.test(script), script); + assert.ok(!/\blocal\s/.test(script), script); + assert.ok(!/\bfunction\s+\w+\s*\(/.test(script), script); + } + }); + + it("parses under sh -n", async () => { + const root = await scratch(); + try { + const scripts: Record = { + link: buildRelayfileMountLinkShell({ binDir: "/home/node/.local/bin" }), + gh: buildGhInstallShell({ + version: "2.82.1", + binDir: "/home/node/.local/bin", + sha256: "a".repeat(64), + }), + claude: buildClaudeConfigSeedShell({ + configPath: "/home/node/.claude.json", + onboardingVersion: "2.1.245", + }), + }; + for (const [name, script] of Object.entries(scripts)) { + const file = join(root, `${name}.sh`); + await writeFile(file, script); + const result = await sh(`sh -n ${file}`); + assert.equal(result.code, 0, `${name}: ${result.stderr}`); + } + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +}); + +describe("the symlink target survives being a symlink itself", () => { + it("re-links over an existing stale link", async () => { + const root = await scratch(); + try { + const globalRoot = await fakeGlobalRoot(root); + const binDir = join(root, "bin"); + await mkdir(binDir, { recursive: true }); + await symlink("/nonexistent/relayfile-mount", join(binDir, "relayfile-mount")); + const result = await sh( + buildRelayfileMountLinkShell({ binDir, searchRoots: [globalRoot] }), + { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }, + ); + assert.equal(result.code, 0, result.stderr); + const invoked = await sh("relayfile-mount", { + env: { PATH: `${binDir}:${process.env.PATH ?? ""}` }, + }); + assert.equal(invoked.stdout.trim(), "fake-relayfile-mount"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +}); diff --git a/src/bootstrap.ts b/src/bootstrap.ts new file mode 100644 index 0000000..47aee64 --- /dev/null +++ b/src/bootstrap.ts @@ -0,0 +1,382 @@ +/** + * Shell snippets that close the gaps between a bare sandbox image and a box an + * agent can actually work on. + * + * These exist because a measured Agent37 run found three things missing that + * every provider is otherwise expected to have supplied: `relayfile-mount` was + * not on PATH, `gh` was not installed at all (`exit 127`), and a clean + * `~/.claude.json` auto-*rejected* a perfectly valid API key so the agent came + * up on a login screen. None of the three needs root, and none of the three is + * a provider fault — the same `gh` gap is `exit 127` on Daytona too, and the + * Claude config trap is image-independent. + * + * Two constraints shape every snippet here, and both are measured rather than + * assumed: + * + * - **POSIX `sh`, not bash.** Agent37's exec plane runs `dash`. A bashism such + * as `${PIPESTATUS[0]}` fails there with `Bad substitution`, silently + * turning a working step into a broken one. Nothing below uses arrays, + * `[[`, or `local`. + * - **No root.** The Agent37 template user is `node` (uid 1000), `sudo` is + * inert (`effective uid is not 0 … nosuid`), and `mkdir /opt` is denied — + * as it is on Daytona, so that is not an Agent37 property. Everything + * installs under `$HOME`, where npm's global prefix already points. + * + * The contract matches `mount-script.ts`: helpers take primitives, do their own + * shell quoting, and return ready-to-run shell. Callers must not re-quote. + */ + +/** Single-quote a value so no byte in it is parsed as shell syntax. */ +function shellQuote(value: string): string { + return `'${value.replace(/'/g, "'\\''")}'`; +} + +function requireNonEmpty(value: string | undefined, field: string): string { + const trimmed = value?.trim(); + if (!trimmed) { + throw new Error(`${field} is required and must be a non-empty string`); + } + return trimmed; +} + +/** + * Reject a value that would be interpreted rather than used. + * + * Quoting already stops shell injection; this stops a subtler class of bug + * where a newline or a `..` segment produces a snippet that runs cleanly and + * does the wrong thing — the same reasoning `mount-script.ts` applies to + * remote paths. + */ +function requireAbsolutePath(value: string | undefined, field: string): string { + const path = requireNonEmpty(value, field); + if (!path.startsWith("/")) { + throw new Error(`${field} must be an absolute path; got ${JSON.stringify(path)}`); + } + if (path.includes("\n") || path.includes("\0")) { + throw new Error(`${field} must not contain a newline or NUL`); + } + if (path.split("/").includes("..")) { + throw new Error(`${field} must not contain a ".." segment; got ${JSON.stringify(path)}`); + } + return path; +} + +// --------------------------------------------------------------------------- +// relayfile-mount +// --------------------------------------------------------------------------- + +export type RelayfileMountLinkShellOptions = { + /** + * Directory the symlink is created in — for example `/home/node/.local/bin`. + * It is created if absent. It must already be on PATH; this builder does not + * edit a profile, because which profile a sandbox's exec plane reads is + * image-specific. + */ + binDir: string; + /** + * Name to link as. Defaults to `relayfile-mount`, which is the name every + * caller in this package invokes (see `mount-script.ts`). + */ + linkName?: string; + /** + * Roots to search for the vendored binary, in order. Omitted, the snippet + * asks npm for its global root, which is where a `npm install -g agent-relay` + * puts the package. + */ + searchRoots?: readonly string[]; +}; + +/** + * Shell that puts `relayfile-mount` on PATH from the copy already vendored + * inside the installed `agent-relay` package, and verifies the result. + * + * **No download and no root.** `agent-relay` ships the daemon as an optional + * per-platform dependency (`@relayfile/mount--`), so a box that + * has the CLI already has the binary — it just is not on PATH, which is why + * `command -v relayfile-mount` comes back empty on an Agent37 box while + * Daytona's image has it at `/usr/local/bin/relayfile-mount`. + * + * Resolution walks up from the `agent-relay` package directory looking for + * `node_modules/@relayfile/mount--/bin/relayfile-mount`, rather + * than joining a fixed path, because npm is free to hoist that dependency to a + * higher `node_modules`. The walk is the same shape the sandbox image's own + * entrypoint uses to find the broker binary. + * + * The snippet exits non-zero with a specific message when the binary cannot be + * found, so a bootstrap fails loudly instead of leaving a box whose mount will + * fail later for a reason nobody will connect to this step. + */ +export function buildRelayfileMountLinkShell(opts: RelayfileMountLinkShellOptions): string { + const binDir = requireAbsolutePath(opts.binDir, "binDir"); + const linkName = requireNonEmpty(opts.linkName ?? "relayfile-mount", "linkName"); + if (linkName.includes("/")) { + throw new Error(`linkName must be a bare filename; got ${JSON.stringify(linkName)}`); + } + for (const root of opts.searchRoots ?? []) { + requireAbsolutePath(root, "searchRoots entry"); + } + + // `process.argv[1]` carries the roots so no path is interpolated into the JS + // source itself, and the resolver stays one readable expression. + const resolver = + "const fs=require('fs'),path=require('path');" + + "const pkg='@relayfile/mount-'+process.platform+'-'+process.arch;" + + "const roots=process.argv.slice(1).filter(Boolean);" + + "let found='';" + + "for(const root of roots){" + + "let dir=path.join(root,'agent-relay');" + + "if(!fs.existsSync(dir))continue;" + + "for(let i=0;i<10&&!found;i++){" + + "const cand=path.join(dir,'node_modules',pkg,'bin','relayfile-mount');" + + "if(fs.existsSync(cand))found=cand;" + + "const up=path.dirname(dir);" + + "if(up===dir)break;" + + "dir=up;}" + + "if(found)break;}" + + "if(!found)process.exit(1);" + + "process.stdout.write(found);"; + + const rootsExpr = (opts.searchRoots ?? []).map((root) => shellQuote(root)).join(" "); + const npmRoot = '"$(npm root -g 2>/dev/null)"'; + + return [ + `set -e`, + `mkdir -p ${shellQuote(binDir)}`, + `__rf_src=$(node -e ${shellQuote(resolver)} -- ${npmRoot}${rootsExpr ? ` ${rootsExpr}` : ""}) || {`, + ` printf '%s\\n' 'relayfile-mount not found: no @relayfile/mount-- beside the installed agent-relay package. Install agent-relay first.' >&2`, + ` exit 1`, + `}`, + `ln -sf "$__rf_src" ${shellQuote(`${binDir}/${linkName}`)}`, + `command -v ${shellQuote(linkName)} >/dev/null 2>&1 || {`, + ` printf '%s\\n' ${shellQuote(`${linkName} linked into ${binDir} but not on PATH; add ${binDir} to PATH`)} >&2`, + ` exit 1`, + `}`, + `printf '%s\\n' "$__rf_src"`, + ].join("\n"); +} + +// --------------------------------------------------------------------------- +// gh +// --------------------------------------------------------------------------- + +/** + * The GitHub CLI's own published release host. + * + * This package ships no *Agent Relay* endpoint defaults — a base URL for our + * own control plane is always a required argument, because no default is + * correct for every consumer. This is a different thing: it is the upstream + * vendor's canonical download host, the one their install instructions name. + * It stays overridable for callers behind a mirror. + */ +export const GH_RELEASE_BASE_URL = "https://github.com/cli/cli/releases/download"; + +export type GhInstallShellOptions = { + /** + * Release to install, without a leading `v` — for example `2.82.1`. + * Required: a pinned default here would rot silently, and a bootstrap that + * quietly changes which `gh` an agent gets is worse than one that asks. + */ + version: string; + /** + * Directory the `gh` binary is placed in — for example + * `/home/node/.local/bin`. Created if absent, and must be on PATH. + */ + binDir: string; + /** Override the download host. Defaults to {@link GH_RELEASE_BASE_URL}. */ + releaseBaseUrl?: string; + /** + * Expected SHA-256 of the downloaded tarball, lowercase hex. + * + * Optional but recommended: without it the snippet installs an unverified + * binary onto an agent's PATH. It is optional rather than required only + * because the digest is per-version-per-arch, so a caller that pins one arch + * can supply it and a caller that does not cannot. When supplied, a mismatch + * aborts before anything is extracted. + */ + sha256?: string; + /** + * Scratch directory for the download. Defaults to `/tmp`, which is writable + * on both providers measured. + */ + workDir?: string; +}; + +/** + * Shell that installs `gh` into a user-writable directory, no root required. + * + * On a measured Agent37 box this took about three seconds and moved + * `gh --version` from `exit 127` (`gh: not found`) to `exit 0`. Note what it + * does *not* fix: `gh auth status` then returns `exit 1` + * ("not logged into any GitHub hosts"). A present binary with no credential is + * a different failure from a missing binary, and only the first is this + * snippet's job. + * + * The architecture is resolved at run time from `uname -m` rather than baked + * in, so one built command serves amd64 and arm64 images. + */ +export function buildGhInstallShell(opts: GhInstallShellOptions): string { + const version = requireNonEmpty(opts.version, "version"); + if (version.startsWith("v")) { + throw new Error( + `version must not carry a leading "v"; got ${JSON.stringify(version)} — pass "${version.slice(1)}"`, + ); + } + if (!/^[0-9][0-9A-Za-z.+-]*$/.test(version)) { + throw new Error(`version is not a plausible release string: ${JSON.stringify(version)}`); + } + const binDir = requireAbsolutePath(opts.binDir, "binDir"); + const workDir = requireAbsolutePath(opts.workDir ?? "/tmp", "workDir"); + const baseUrl = requireNonEmpty( + opts.releaseBaseUrl ?? GH_RELEASE_BASE_URL, + "releaseBaseUrl", + ).replace(/\/+$/, ""); + const sha256 = opts.sha256?.trim().toLowerCase(); + if (sha256 !== undefined && !/^[0-9a-f]{64}$/.test(sha256)) { + throw new Error("sha256 must be 64 lowercase hex characters"); + } + + const lines = [ + `set -e`, + `mkdir -p ${shellQuote(binDir)} ${shellQuote(workDir)}`, + `__gh_ver=${shellQuote(version)}`, + `case "$(uname -m)" in`, + ` x86_64|amd64) __gh_arch=amd64 ;;`, + ` aarch64|arm64) __gh_arch=arm64 ;;`, + ` *) printf '%s\\n' "unsupported architecture for gh: $(uname -m)" >&2; exit 1 ;;`, + `esac`, + `__gh_name="gh_\${__gh_ver}_linux_\${__gh_arch}"`, + `__gh_tgz=${shellQuote(workDir)}/"\${__gh_name}.tar.gz"`, + `curl -fsSL -o "$__gh_tgz" ${shellQuote(baseUrl)}/"v\${__gh_ver}"/"\${__gh_name}.tar.gz"`, + ]; + + if (sha256) { + // `-c` reads " " — the two spaces are the format, not a + // typo. `sha256sum` is coreutils and `shasum` is the perl tool; a minimal + // image may ship either, so try both rather than assuming. Verified before + // extraction, so a bad payload is never unpacked. + lines.push( + `if command -v sha256sum >/dev/null 2>&1; then __gh_sum="sha256sum"`, + `elif command -v shasum >/dev/null 2>&1; then __gh_sum="shasum -a 256"`, + `else printf '%s\\n' 'no sha256sum or shasum available to verify the gh tarball' >&2; rm -f "$__gh_tgz"; exit 1`, + `fi`, + `printf '%s %s\\n' ${shellQuote(sha256)} "$__gh_tgz" | $__gh_sum -c - >/dev/null 2>&1 || {`, + ` printf '%s\\n' 'gh tarball failed SHA-256 verification; refusing to install' >&2`, + ` rm -f "$__gh_tgz"`, + ` exit 1`, + `}`, + ); + } + + lines.push( + `tar -xzf "$__gh_tgz" -C ${shellQuote(workDir)}`, + `cp ${shellQuote(workDir)}/"\${__gh_name}"/bin/gh ${shellQuote(`${binDir}/gh`)}`, + `chmod 0755 ${shellQuote(`${binDir}/gh`)}`, + `rm -rf "$__gh_tgz" ${shellQuote(workDir)}/"\${__gh_name}"`, + `command -v gh >/dev/null 2>&1 || {`, + ` printf '%s\\n' ${shellQuote(`gh installed into ${binDir} but not on PATH; add ${binDir} to PATH`)} >&2`, + ` exit 1`, + `}`, + `gh --version | head -1`, + ); + + return lines.join("\n"); +} + +// --------------------------------------------------------------------------- +// Claude Code first-run config +// --------------------------------------------------------------------------- + +export type ClaudeConfigSeedShellOptions = { + /** + * Absolute path to the config file — for example `/home/node/.claude.json`. + * It is created if absent and **merged** if present, never overwritten: a + * box may already carry machine ids and migration flags that matter. + */ + configPath: string; + /** + * Environment variable the snippet reads the Anthropic API key from at run + * time. Defaults to `ANTHROPIC_API_KEY`. + * + * The key is read *inside* the sandbox rather than interpolated into the + * command, so no credential is ever rendered into the built string, an argv + * list, or a log — the same ingress rule `mount-script.ts` applies to the + * relayfile token. + */ + apiKeyEnvVar?: string; + /** + * Value recorded as the onboarding version. Optional; omitted, only the + * boolean flag is written. + */ + onboardingVersion?: string; +}; + +/** + * Shell that marks Claude Code's first-run onboarding complete and pre-approves + * the API key the agent will actually use. + * + * **Why the approval half matters.** A freshly spawned agent came up on the + * OAuth sign-in screen while holding a valid key in its environment — a live + * key, verified against the API from inside the same box. The cause was in + * `~/.claude.json`: the key's identifying tail had been recorded under + * `customApiKeyResponses.rejected`, so the CLI declined to use it and fell + * back to interactive login. On a headless box that is a hang, not a prompt. + * + * The identifying tail is the key's last 20 characters, which is what the CLI + * itself stores — never the key. This snippet moves that tail out of + * `rejected` and into `approved`, so a re-run repairs a box that has already + * been poisoned rather than only helping a pristine one. + * + * `hasCompletedOnboarding` is set in the same write because a clean home needs + * both: an approved key still lands on the theme-and-welcome screen without it. + * + * Nothing here is Agent37-specific. It will bite every fresh box on every + * provider. + */ +export function buildClaudeConfigSeedShell(opts: ClaudeConfigSeedShellOptions): string { + const configPath = requireAbsolutePath(opts.configPath, "configPath"); + const apiKeyEnvVar = requireNonEmpty(opts.apiKeyEnvVar ?? "ANTHROPIC_API_KEY", "apiKeyEnvVar"); + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(apiKeyEnvVar)) { + throw new Error( + `apiKeyEnvVar must be a valid shell identifier; got ${JSON.stringify(apiKeyEnvVar)}`, + ); + } + const onboardingVersion = opts.onboardingVersion?.trim(); + if (onboardingVersion !== undefined && onboardingVersion.includes("\n")) { + throw new Error("onboardingVersion must not contain a newline"); + } + + // Read/modify/write in one node process so a concurrent bootstrap step + // cannot interleave between the read and the write. + const script = + "const fs=require('fs'),path=require('path');" + + "const p=process.env.__CLAUDE_CONFIG_PATH;" + + "let cfg={};" + + "try{cfg=JSON.parse(fs.readFileSync(p,'utf8'))||{};}catch(e){" + + "if(e&&e.code!=='ENOENT')throw e;}" + + "if(typeof cfg!=='object'||cfg===null||Array.isArray(cfg))" + + "throw new Error('existing Claude config is not a JSON object: '+p);" + + "cfg.hasCompletedOnboarding=true;" + + "const ver=process.env.__CLAUDE_ONBOARDING_VERSION;" + + "if(ver)cfg.lastOnboardingVersion=ver;" + + "const key=process.env.__CLAUDE_API_KEY||'';" + + "let approved=0;" + + "if(key){" + + "const tail=key.slice(-20);" + + "const r=cfg.customApiKeyResponses;" + + "const prev=(r&&typeof r==='object'&&!Array.isArray(r))?r:{};" + + "const keep=(list)=>Array.isArray(list)?list.filter((v)=>typeof v==='string'&&v!==tail):[];" + + "cfg.customApiKeyResponses={approved:keep(prev.approved).concat([tail])," + + "rejected:keep(prev.rejected)};" + + "approved=1;}" + + "fs.mkdirSync(path.dirname(p),{recursive:true});" + + "fs.writeFileSync(p,JSON.stringify(cfg,null,2),{mode:0o600});" + + "process.stdout.write('claude-config-seeded onboarding=1 apiKeyApproved='+approved+'\\n');"; + + return [ + `set -e`, + `__CLAUDE_CONFIG_PATH=${shellQuote(configPath)} \\`, + ...(onboardingVersion ? [`__CLAUDE_ONBOARDING_VERSION=${shellQuote(onboardingVersion)} \\`] : []), + `__CLAUDE_API_KEY="\${${apiKeyEnvVar}:-}" \\`, + `node -e ${shellQuote(script)}`, + ].join("\n"); +} diff --git a/src/core/index.ts b/src/core/index.ts index 7e1b6b6..57e4762 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -63,3 +63,6 @@ export type { // --- relayfile-mount shell builders ---------------------------------------- export * from "../mount-script.js"; + +// --- sandbox bootstrap shell builders -------------------------------------- +export * from "../bootstrap.js"; From 24a8bf7a732bae9d056e687a763ab4b850efb27c Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 25 Aug 2026 20:36:56 +0200 Subject: [PATCH 3/8] docs(agent37): tag fenced blocks with a language so MD040 passes markdownlint's MD040 rule flags the three unlabeled fences in the newly added doc: the template table (line 10), the /root probe transcript (line 27), and the mount check transcript (line 89). Use text for the table and console for the two command transcripts. Co-Authored-By: Claude Opus 4.7 --- docs/agent37.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/agent37.md b/docs/agent37.md index 2f5bc50..2058eba 100644 --- a/docs/agent37.md +++ b/docs/agent37.md @@ -7,7 +7,7 @@ defects" are not properties of Agent37 at all. ## The template, and the one setting that matters -``` +```text user node (uid 1000, gid 1000) HOME /home/node cwd / (the exec plane's default when no cwd is given) @@ -24,7 +24,7 @@ single most consequential line in an Agent37 integration. enter it. So a launch that sets `workdir: '/root'` does not fail once, it fails *every command on the box*, identically: -``` +```console $ id; echo PWD=$PWD → exit 1 sh: 1: cd: can't cd to /root ``` @@ -86,7 +86,7 @@ different failure from a missing binary, and only the second is exit 127. `relayfile-mount` is a **userspace sync daemon**, not a kernel or FUSE mount. On a completely healthy Agent37 box: -``` +```console $ mount | grep -i relayfile → exit 1, no output ``` From 351761350831a40d3f027551d667f16f570c0c6d Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 25 Aug 2026 20:37:12 +0200 Subject: [PATCH 4/8] fix(bootstrap): tighten PATH resolution, mode 0600 rewrite, and JSON validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six related defects flagged by cubic + codex + coderabbit on #42, all in the userspace bootstrap snippets: - buildRelayfileMountLinkShell honored `searchRoots` in name only: npm's global root was always searched first, so a stale global agent-relay silently won the resolver race over any explicitly named root. When the caller supplies searchRoots, do not consult `npm root -g` at all. - buildRelayfileMountLinkShell rejected linkNames containing `/` but let `.` and `..` through, and `ln -sf src ..` follows the directory and drops the link one level above the bin dir. Reject both. - Both link and gh installers reported success as soon as `command -v ` was non-empty, so a same-named binary earlier on PATH silently won over the one that was just placed. Verify the resolved path equals `${binDir}/${name}` and fail loud otherwise. - `gh --version | head -1` masked a nonzero `gh` exit because dash has no `pipefail` and `head` still exits 0, so a broken install looked fine. Capture the output first, propagate `gh`'s status, then trim to the first line for the printed sanity check. - buildClaudeConfigSeedShell wrapped `JSON.parse` output with `||{}`, which silently coerced bare JSON `null`, `false`, `0`, and `""` to `{}` — the shape validation never saw them and the file was clobbered. Drop the coercion; the existing non-object check catches every invalid form. - `fs.writeFileSync(p, ..., { mode: 0o600 })` only applies the mode when the file is being created, so a rewritten config kept its prior permissions and the API-key tail was left behind mode 0644. Follow the write with an explicit `chmodSync(p, 0o600)`. Tests cover each of the six changes. Co-Authored-By: Claude Opus 4.7 --- src/bootstrap.test.ts | 159 ++++++++++++++++++++++++++++++++++++++++-- src/bootstrap.ts | 72 +++++++++++++++---- 2 files changed, 213 insertions(+), 18 deletions(-) diff --git a/src/bootstrap.test.ts b/src/bootstrap.test.ts index 7c4a65b..e746def 100644 --- a/src/bootstrap.test.ts +++ b/src/bootstrap.test.ts @@ -134,6 +134,53 @@ describe("buildRelayfileMountLinkShell", () => { () => buildRelayfileMountLinkShell({ binDir: "/tmp/bin", linkName: "a/b" }), /bare filename/, ); + // `.` and `..` slip past the `/` check but `ln -sf src ..` follows the + // directory and drops the link one level above the bin dir. Reject both. + assert.throws( + () => buildRelayfileMountLinkShell({ binDir: "/tmp/bin", linkName: "." }), + /"\." or "\.\."/, + ); + assert.throws( + () => buildRelayfileMountLinkShell({ binDir: "/tmp/bin", linkName: ".." }), + /"\." or "\.\."/, + ); + }); + + it("does not consult npm's global root when explicit searchRoots are supplied", () => { + // A stale global `agent-relay` earlier in the resolver's argv would win + // the search over an explicitly named root, contradicting the option's + // documented ordering. + const withRoots = buildRelayfileMountLinkShell({ + binDir: "/home/node/.local/bin", + searchRoots: ["/opt/vendored"], + }); + assert.ok(!/npm root -g/.test(withRoots), "must not fall back to npm root when searchRoots given"); + + const withoutRoots = buildRelayfileMountLinkShell({ binDir: "/home/node/.local/bin" }); + assert.match(withoutRoots, /npm root -g/, "must fall back to npm root when searchRoots omitted"); + }); + + it("fails loud when a different binary shadows the linked one on PATH", async () => { + const root = await scratch(); + try { + const globalRoot = await fakeGlobalRoot(root); + const binDir = join(root, "bin"); + // A prior `relayfile-mount` sits earlier on PATH; the linked binary is + // present at the requested location but PATH order is wrong. The + // snippet must refuse rather than pretend everything is fine. + const shadow = join(root, "shadow"); + await mkdir(shadow, { recursive: true }); + await writeFile(join(shadow, "relayfile-mount"), "#!/bin/sh\nprintf 'shadow\\n'\n"); + await chmod(join(shadow, "relayfile-mount"), 0o755); + const result = await sh( + buildRelayfileMountLinkShell({ binDir, searchRoots: [globalRoot] }), + { env: { PATH: `${shadow}:${binDir}:${process.env.PATH ?? ""}` } }, + ); + assert.notEqual(result.code, 0, "must not report success when PATH resolves to another binary"); + assert.match(result.stderr, /prepend .* to PATH/); + } finally { + await rm(root, { recursive: true, force: true }); + } }); }); @@ -166,14 +213,15 @@ describe("buildGhInstallShell", () => { const { baseUrl } = await fakeRelease(root, "9.9.9"); const binDir = join(root, "bin"); const workDir = join(root, "work"); - const result = await sh( - buildGhInstallShell({ version: "9.9.9", binDir, workDir, releaseBaseUrl: baseUrl }), - { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }, - ); + const script = buildGhInstallShell({ version: "9.9.9", binDir, workDir, releaseBaseUrl: baseUrl }); + // No root anywhere in the generated shell: that is the whole constraint. + // Assert on the shell we built, not on `gh --version` output — the + // stdout scan cannot detect `sudo` and would keep passing after the + // constraint stopped holding. + assert.ok(!/\bsudo\b/.test(script), "no sudo in generated shell"); + const result = await sh(script, { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }); assert.equal(result.code, 0, result.stderr); assert.match(result.stdout, /gh version 9\.9\.9/); - // No root anywhere in the generated shell: that is the whole constraint. - assert.ok(!/\bsudo\b/.test(result.stdout)); } finally { await rm(root, { recursive: true, force: true }); } @@ -238,6 +286,66 @@ describe("buildGhInstallShell", () => { /64 lowercase hex/, ); }); + + it("fails loud when the installed binary cannot run, instead of masking through head", async () => { + // The regression this guards: `gh --version | head -1` used to hide a + // nonzero `gh` exit because dash has no pipefail and `head` still exits + // 0. A build that cannot run on this image now reports failure. + const root = await scratch(); + try { + const arch = process.arch === "arm64" ? "arm64" : "amd64"; + const version = "9.9.9"; + const name = `gh_${version}_linux_${arch}`; + const stage = join(root, "stage", name, "bin"); + await mkdir(stage, { recursive: true }); + // A `gh` that always exits nonzero — stand-in for a binary that cannot + // run on the image (missing loader, wrong arch, corrupted). + await writeFile(join(stage, "gh"), "#!/bin/sh\nexit 1\n"); + await chmod(join(stage, "gh"), 0o755); + const releases = join(root, "releases", `v${version}`); + await mkdir(releases, { recursive: true }); + await run("tar", ["-czf", join(releases, `${name}.tar.gz`), "-C", join(root, "stage"), name]); + const binDir = join(root, "bin"); + const result = await sh( + buildGhInstallShell({ + version, + binDir, + workDir: join(root, "work"), + releaseBaseUrl: `file://${join(root, "releases")}`, + }), + { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }, + ); + assert.notEqual(result.code, 0, "must not mask a broken gh install"); + assert.match(result.stderr, /gh --version failed/); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("fails loud when a different gh is earlier on PATH", async () => { + const root = await scratch(); + try { + const { baseUrl } = await fakeRelease(root, "9.9.9"); + const binDir = join(root, "bin"); + const shadow = join(root, "shadow"); + await mkdir(shadow, { recursive: true }); + await writeFile(join(shadow, "gh"), "#!/bin/sh\nprintf 'shadow gh\\n'\n"); + await chmod(join(shadow, "gh"), 0o755); + const result = await sh( + buildGhInstallShell({ + version: "9.9.9", + binDir, + workDir: join(root, "work"), + releaseBaseUrl: baseUrl, + }), + { env: { PATH: `${shadow}:${binDir}:${process.env.PATH ?? ""}` } }, + ); + assert.notEqual(result.code, 0); + assert.match(result.stderr, /prepend .* to PATH/); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); }); describe("buildClaudeConfigSeedShell", () => { @@ -351,6 +459,45 @@ describe("buildClaudeConfigSeedShell", () => { } }); + it("rejects a bare JSON null, false, 0, or empty string as the config", async () => { + // These used to be coerced to `{}` by `JSON.parse(...)||{}`, so the shape + // validation never saw them and the file was silently clobbered. + for (const payload of ["null", "false", "0", '""']) { + const root = await scratch(); + try { + const configPath = join(root, ".claude.json"); + await writeFile(configPath, payload); + const result = await seed(configPath, {}); + assert.notEqual(result.code, 0, `expected failure on payload ${payload}`); + assert.match(result.stderr, /not a JSON object/); + assert.equal(await readFile(configPath, "utf8"), payload, "invalid config must survive"); + } finally { + await rm(root, { recursive: true, force: true }); + } + } + }); + + it("re-enforces mode 0600 when rewriting an existing permissive config", async () => { + // The regression this guards: `fs.writeFileSync({ mode: 0o600 })` only + // applies the mode when the file is being created. A world-readable + // config kept its permissions after this snippet ran, leaving the API + // key tail behind mode 0644. + const root = await scratch(); + try { + const configPath = join(root, ".claude.json"); + await writeFile(configPath, "{}", { mode: 0o644 }); + await chmod(configPath, 0o644); + const before = await sh(`ls -l ${configPath} | cut -c1-10`); + assert.equal(before.stdout.trim(), "-rw-r--r--"); + const result = await seed(configPath, { ANTHROPIC_API_KEY: "sk-ant-" + "q".repeat(30) }); + assert.equal(result.code, 0, result.stderr); + const after = await sh(`ls -l ${configPath} | cut -c1-10`); + assert.equal(after.stdout.trim(), "-rw-------", "rewrite must tighten mode to 0600"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + it("rejects a relative path and an env var that is not an identifier", () => { assert.throws(() => buildClaudeConfigSeedShell({ configPath: ".claude.json" }), /absolute/); assert.throws( diff --git a/src/bootstrap.ts b/src/bootstrap.ts index 47aee64..54907a9 100644 --- a/src/bootstrap.ts +++ b/src/bootstrap.ts @@ -109,8 +109,12 @@ export type RelayfileMountLinkShellOptions = { export function buildRelayfileMountLinkShell(opts: RelayfileMountLinkShellOptions): string { const binDir = requireAbsolutePath(opts.binDir, "binDir"); const linkName = requireNonEmpty(opts.linkName ?? "relayfile-mount", "linkName"); - if (linkName.includes("/")) { - throw new Error(`linkName must be a bare filename; got ${JSON.stringify(linkName)}`); + // `.` and `..` slip past the `/` check but `ln -sf src ..` follows the + // directory and creates the link one level up. Reject them explicitly. + if (linkName.includes("/") || linkName === "." || linkName === "..") { + throw new Error( + `linkName must be a bare filename other than "." or ".."; got ${JSON.stringify(linkName)}`, + ); } for (const root of opts.searchRoots ?? []) { requireAbsolutePath(root, "searchRoots entry"); @@ -137,20 +141,37 @@ export function buildRelayfileMountLinkShell(opts: RelayfileMountLinkShellOption "process.stdout.write(found);"; const rootsExpr = (opts.searchRoots ?? []).map((root) => shellQuote(root)).join(" "); - const npmRoot = '"$(npm root -g 2>/dev/null)"'; + // Only fall back to npm's global root when the caller did not name their own + // — otherwise a stale global `agent-relay` wins the search over the roots + // that were explicitly requested, contradicting the option's documented + // ordering. + const useNpmFallback = !opts.searchRoots || opts.searchRoots.length === 0; + const argsExpr = useNpmFallback + ? `"$(npm root -g 2>/dev/null)"${rootsExpr ? ` ${rootsExpr}` : ""}` + : rootsExpr; + const linkPath = `${binDir}/${linkName}`; return [ `set -e`, `mkdir -p ${shellQuote(binDir)}`, - `__rf_src=$(node -e ${shellQuote(resolver)} -- ${npmRoot}${rootsExpr ? ` ${rootsExpr}` : ""}) || {`, + `__rf_src=$(node -e ${shellQuote(resolver)} -- ${argsExpr}) || {`, ` printf '%s\\n' 'relayfile-mount not found: no @relayfile/mount-- beside the installed agent-relay package. Install agent-relay first.' >&2`, ` exit 1`, `}`, - `ln -sf "$__rf_src" ${shellQuote(`${binDir}/${linkName}`)}`, - `command -v ${shellQuote(linkName)} >/dev/null 2>&1 || {`, + `ln -sf "$__rf_src" ${shellQuote(linkPath)}`, + // `command -v` alone is not enough: another `${linkName}` earlier on PATH + // wins the lookup and the bare invocation silently runs the wrong binary. + // Verify the resolved path is the one this snippet just created; if PATH + // is misordered we fail loud rather than leaving a box that looks fine. + `__rf_which=$(command -v ${shellQuote(linkName)} 2>/dev/null || true)`, + `if [ -z "$__rf_which" ]; then`, ` printf '%s\\n' ${shellQuote(`${linkName} linked into ${binDir} but not on PATH; add ${binDir} to PATH`)} >&2`, ` exit 1`, - `}`, + `fi`, + `if [ "$__rf_which" != ${shellQuote(linkPath)} ]; then`, + ` printf '%s\\n' ${shellQuote(`${linkName} on PATH resolves to a different binary than ${linkPath}; prepend ${binDir} to PATH`)} >&2`, + ` exit 1`, + `fi`, `printf '%s\\n' "$__rf_src"`, ].join("\n"); } @@ -267,16 +288,33 @@ export function buildGhInstallShell(opts: GhInstallShellOptions): string { ); } + const ghPath = `${binDir}/gh`; lines.push( `tar -xzf "$__gh_tgz" -C ${shellQuote(workDir)}`, - `cp ${shellQuote(workDir)}/"\${__gh_name}"/bin/gh ${shellQuote(`${binDir}/gh`)}`, - `chmod 0755 ${shellQuote(`${binDir}/gh`)}`, + `cp ${shellQuote(workDir)}/"\${__gh_name}"/bin/gh ${shellQuote(ghPath)}`, + `chmod 0755 ${shellQuote(ghPath)}`, `rm -rf "$__gh_tgz" ${shellQuote(workDir)}/"\${__gh_name}"`, - `command -v gh >/dev/null 2>&1 || {`, + // Presence + resolved-path check: an older `gh` earlier on PATH would + // otherwise win the lookup and the caller would keep the pre-existing + // version despite this snippet returning success. + `__gh_which=$(command -v gh 2>/dev/null || true)`, + `if [ -z "$__gh_which" ]; then`, ` printf '%s\\n' ${shellQuote(`gh installed into ${binDir} but not on PATH; add ${binDir} to PATH`)} >&2`, ` exit 1`, + `fi`, + `if [ "$__gh_which" != ${shellQuote(ghPath)} ]; then`, + ` printf '%s\\n' ${shellQuote(`gh on PATH resolves to a different binary than ${ghPath}; prepend ${binDir} to PATH`)} >&2`, + ` exit 1`, + `fi`, + // Capture before piping: `gh --version | head -1` masks a nonzero `gh` + // exit because dash has no pipefail and `head` still returns 0. Verify + // status directly, then trim to the first line for the printed sanity + // check. + `__gh_ver_out=$(gh --version) || {`, + ` printf '%s\\n' 'gh --version failed; installed binary is not runnable' >&2`, + ` exit 1`, `}`, - `gh --version | head -1`, + `printf '%s\\n' "$__gh_ver_out" | head -1`, ); return lines.join("\n"); @@ -347,11 +385,20 @@ export function buildClaudeConfigSeedShell(opts: ClaudeConfigSeedShellOptions): // Read/modify/write in one node process so a concurrent bootstrap step // cannot interleave between the read and the write. + // + // Two subtleties enforced below: + // - `JSON.parse(...)||{}` silently coerces `null`, `false`, `0`, and `""` + // to `{}`, which would let an invalid config be clobbered instead of + // rejected. Let the object-shape validation catch every non-object. + // - `fs.writeFileSync({ mode: 0o600 })` only applies the mode when the + // file is being created; on the repair path the existing perms survive. + // Follow the write with an explicit `chmodSync(0o600)` so the contract + // holds for both first-run and rewrite. const script = "const fs=require('fs'),path=require('path');" + "const p=process.env.__CLAUDE_CONFIG_PATH;" + "let cfg={};" + - "try{cfg=JSON.parse(fs.readFileSync(p,'utf8'))||{};}catch(e){" + + "try{cfg=JSON.parse(fs.readFileSync(p,'utf8'));}catch(e){" + "if(e&&e.code!=='ENOENT')throw e;}" + "if(typeof cfg!=='object'||cfg===null||Array.isArray(cfg))" + "throw new Error('existing Claude config is not a JSON object: '+p);" + @@ -370,6 +417,7 @@ export function buildClaudeConfigSeedShell(opts: ClaudeConfigSeedShellOptions): "approved=1;}" + "fs.mkdirSync(path.dirname(p),{recursive:true});" + "fs.writeFileSync(p,JSON.stringify(cfg,null,2),{mode:0o600});" + + "fs.chmodSync(p,0o600);" + "process.stdout.write('claude-config-seeded onboarding=1 apiKeyApproved='+approved+'\\n');"; return [ From 54e1932bc19db939d14c924fe22aee513869d46d Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 25 Aug 2026 20:37:23 +0200 Subject: [PATCH 5/8] fix(agent37): use a per-invocation nonce for the workdir-unusable sentinel Reclassifying an exit as Agent37WorkdirUnusableError already required BOTH the sentinel exit code and the marker string in the output, on the theory that no honest command would produce both together. That theory is wrong: a command can trivially printf '%s\\n' '__agent37_workdir_unusable__' >&2; exit 191 on a valid cwd where `cd` succeeded, and the runtime would misreport that command as a workdir fault it never was. Close the gap where the collision actually lives: the sentinel line. composeScript now emits a marker of the form `__agent37_workdir_unusable__<32-hex-nonce>` chosen fresh per call, and runScript compares against that exact string. The stable prefix stays exported for humans reading raw output; the nonce guarantees a running command cannot reproduce the full sentinel because it never sees it. New test in Agent37Runtime.runScript proves a command that prints the bare prefix and exits 191 is NOT reclassified. Existing coverage of the "requires both" invariant and of the "no cd, no reclassify" path still holds unchanged. Co-Authored-By: Claude Opus 4.7 --- src/agent37/index.ts | 1 + src/agent37/runtime.test.ts | 48 +++++++++++++++++++++++++++++++----- src/agent37/runtime.ts | 49 +++++++++++++++++++++++++++++++++---- 3 files changed, 87 insertions(+), 11 deletions(-) diff --git a/src/agent37/index.ts b/src/agent37/index.ts index 601be68..d6feac6 100644 --- a/src/agent37/index.ts +++ b/src/agent37/index.ts @@ -10,6 +10,7 @@ export { Agent37Runtime, Agent37UnknownExitCodeError, Agent37WorkdirUnusableError, + makeWorkdirUnusableMarker, } from "./runtime.js"; export type { Agent37BundleFile, diff --git a/src/agent37/runtime.test.ts b/src/agent37/runtime.test.ts index 3ef1398..1814ef5 100644 --- a/src/agent37/runtime.test.ts +++ b/src/agent37/runtime.test.ts @@ -132,6 +132,20 @@ function execCommand(request: RecordedRequest): string { return parsed.command as string; } +/** + * Pull the per-invocation workdir-unusable marker out of a recorded exec body. + * + * Tests that want to simulate a `cd` failure must echo back the exact marker + * the runtime chose for that call — the shared prefix alone will no longer + * trigger reclassification, which is precisely the collision the nonce closes. + */ +function extractWorkdirUnusableMarker(request: RecordedRequest): string { + const cmd = execCommand(request); + const match = cmd.match(/__agent37_workdir_unusable__[0-9a-f]{32}/); + assert.ok(match, `expected a workdir-unusable marker in composed script:\n${cmd}`); + return match[0]; +} + /** Strip comments so a source scan reads code, not the prose explaining it. */ function withoutComments(source: string): string { return source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^[ \t]*\/\/.*$/gm, ""); @@ -913,10 +927,11 @@ describe("Agent37Runtime.runScript", () => { ["command"], "exec takes exactly one field; anything else is rejected by the API", ); - assert.equal( - parsed.command, - "cd '/work/repo' || { printf '%s\\n' '__agent37_workdir_unusable__' >&2; exit 191; }\n" + - "export TOKEN_NAME='it'\\''s fine'\nnpm test\n", + // The sentinel marker carries a per-invocation nonce so command output + // cannot spoof the workdir-unusable signal; everything else is fixed. + assert.match( + parsed.command as string, + /^cd '\/work\/repo' \|\| \{ printf '%s\\n' '__agent37_workdir_unusable__[0-9a-f]{32}' >&2; exit 191; \}\nexport TOKEN_NAME='it'\\''s fine'\nnpm test\n$/, ); assert.deepEqual(result, { output: "ok\n", stdout: "ok\n", exitCode: 0 }); }); @@ -935,11 +950,13 @@ describe("Agent37Runtime.runScript", () => { // '/root'` all came back exit 1, and the lane concluded /root did not // exist. It exists — root-owned, mode 0700, unreachable by the template's // `node` user — and every exit 1 was the `cd`. - const h = harness(() => ({ + const h = harness((request) => ({ json: { exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, stdout: "", - stderr: `${pkg.AGENT37_WORKDIR_UNUSABLE_MARKER}\nsh: 1: cd: can't cd to /root\n`, + // The composed script chose a fresh nonce for this call; echo it back + // so the runtime sees the marker it is actually looking for. + stderr: `${extractWorkdirUnusableMarker(request)}\nsh: 1: cd: can't cd to /root\n`, }, })); const runtime = makeRuntime(h); @@ -977,6 +994,25 @@ describe("Agent37Runtime.runScript", () => { assert.equal(second.exitCode, 1); }); + it("does not reclassify when a command reproduces the fixed prefix and exits 191", async () => { + // Requiring both signals is not enough on its own: a valid command can + // print the bare marker string AND exit 191 in the same run. The + // per-invocation nonce is what makes the pair spoofproof. + const collide = harness(() => ({ + json: { + exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, + stdout: "", + // Marker prefix without the nonce the composed script actually chose. + stderr: `${pkg.AGENT37_WORKDIR_UNUSABLE_MARKER}\ncommand output that happens to include the prefix\n`, + }, + })); + const result = await makeRuntime(collide).runScript(RUNNING_HANDLE, { + command: `printf '%s\\n' '${pkg.AGENT37_WORKDIR_UNUSABLE_MARKER}' >&2; exit 191`, + cwd: "/work", + }); + assert.equal(result.exitCode, pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE); + }); + it("does not reclassify when no cwd was requested, because no cd was emitted", async () => { const h = harness(() => ({ json: { diff --git a/src/agent37/runtime.ts b/src/agent37/runtime.ts index f8000f9..6f90ad5 100644 --- a/src/agent37/runtime.ts +++ b/src/agent37/runtime.ts @@ -1,4 +1,5 @@ import { Buffer } from "node:buffer"; +import { randomUUID } from "node:crypto"; import { writeFile } from "node:fs/promises"; import type { @@ -315,13 +316,34 @@ export class Agent37UnknownExitCodeError extends Error { * The status alone is not proof (a command may legitimately exit 191), and the * marker alone is not proof (a command may legitimately print it), so * {@link Agent37Runtime.runScript} requires both before it reclassifies a - * result as a workdir fault. + * result as a workdir fault. Requiring "both" is not enough on its own either: + * a valid command can print the fixed prefix *and* exit 191 in the same run, + * so {@link composeScript} appends a per-invocation nonce that the running + * command has no way to see. See {@link makeWorkdirUnusableMarker}. */ export const AGENT37_WORKDIR_UNUSABLE_EXIT_CODE = 191; -/** @see AGENT37_WORKDIR_UNUSABLE_EXIT_CODE */ +/** + * Stable prefix for the workdir-unusable sentinel line. + * + * The full marker emitted by any given composed script is this prefix followed + * by a per-invocation nonce, so an unrelated command cannot spoof it. + * + * @see AGENT37_WORKDIR_UNUSABLE_EXIT_CODE + */ export const AGENT37_WORKDIR_UNUSABLE_MARKER = "__agent37_workdir_unusable__"; +/** + * Build a per-invocation workdir-unusable sentinel line. + * + * The prefix stays discoverable to a human reading the raw output, and the + * nonce guarantees the line cannot be reproduced by a command whose own output + * happens to include the prefix. + */ +export function makeWorkdirUnusableMarker(): string { + return `${AGENT37_WORKDIR_UNUSABLE_MARKER}${randomUUID().replace(/-/g, "")}`; +} + /** * Raised when the instance could not enter the requested working directory. * @@ -828,9 +850,14 @@ export class Agent37Runtime implements SandboxRuntime, WorkflowRuntime { assertValidEnv(options.env); } const cwd = options.cwd ?? handle.workdir; + // Per-invocation nonce, generated only when a cd is actually emitted. A + // command that legitimately prints the fixed prefix and exits 191 will + // not carry this suffix, so reclassification cannot collide with it. + const workdirUnusableMarker = cwd ? makeWorkdirUnusableMarker() : undefined; const script = composeScript(options.command, { ...(cwd ? { cwd } : {}), ...(options.env ? { env: options.env } : {}), + ...(workdirUnusableMarker ? { workdirUnusableMarker } : {}), }); // Only `requestTimeoutMs` becomes an abort signal. `timeoutMs` is a command // lifetime, and it got here only because the provider's own cap already @@ -841,8 +868,9 @@ export class Agent37Runtime implements SandboxRuntime, WorkflowRuntime { // be reported as a command that ran and failed. if ( cwd && + workdirUnusableMarker && result.exit_code === AGENT37_WORKDIR_UNUSABLE_EXIT_CODE && - combineOutput(result.stdout, result.stderr).includes(AGENT37_WORKDIR_UNUSABLE_MARKER) + combineOutput(result.stdout, result.stderr).includes(workdirUnusableMarker) ) { throw new Agent37WorkdirUnusableError( handle.id, @@ -1274,16 +1302,27 @@ export function assertValidEnv(env: Record): void { */ export function composeScript( command: string, - options: { cwd?: string; env?: Record } = {}, + options: { + cwd?: string; + env?: Record; + /** + * Sentinel line the `cd` failure branch writes to stderr. Callers wiring + * this into a reclassifier should pass a unique value per invocation (see + * {@link makeWorkdirUnusableMarker}) so command output cannot spoof the + * failure signal. Defaults to the stable prefix for compose-only use. + */ + workdirUnusableMarker?: string; + } = {}, ): string { const lines: string[] = []; if (options.cwd) { // POSIX `sh`, not bash: Agent37's exec plane runs dash, where a bashism // like ${PIPESTATUS[0]} is a "Bad substitution". `{ …; }` and `printf` are // both POSIX, so this line runs on either shell. + const marker = options.workdirUnusableMarker ?? AGENT37_WORKDIR_UNUSABLE_MARKER; lines.push( `cd ${shellQuote(options.cwd)} || { printf '%s\\n' ` + - `${shellQuote(AGENT37_WORKDIR_UNUSABLE_MARKER)} >&2; ` + + `${shellQuote(marker)} >&2; ` + `exit ${AGENT37_WORKDIR_UNUSABLE_EXIT_CODE}; }`, ); } From cdeed3a52bdfce74566f4ad0591b4ed97eee5ae6 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 25 Aug 2026 20:54:20 +0200 Subject: [PATCH 6/8] fix: address the three follow-ups cubic raised on the first round of fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three P2 issues cubic surfaced when it re-reviewed the fix batch on #42: - `binDir` with a trailing slash produced an expected path with `//` which `command -v` never matches, because the shell normalizes the PATH entry it resolves. A perfectly good `relayfile-mount` or `gh` install was reported as shadowed. Strip trailing slashes when constructing `linkPath` / `ghPath`. - The per-invocation workdir-unusable nonce closes the accidental collision, but a hostile command inside the sandbox can still recover the nonce from `/proc/self/cmdline` and forge the exit code + marker pair. Add an out-of-band verification: on the ambiguous 191+marker case, re-issue `cd ` in a fresh exec that runs no user command. If that probe also fails, this really was a workdir fault; otherwise the in-band signals were a spoof and the result stays a plain command failure. The extra exec runs only in the ambiguous case, so the common path pays no cost. Tests cover the trailing-slash accept path for the link builder and add a spoof scenario where the command echoes the recovered nonce and the probe reports the cwd as healthy — the runtime must NOT reclassify. The existing "names an unusable workdir" test now also asserts the probe was sent. Co-Authored-By: Claude Opus 4.7 --- src/agent37/runtime.test.ts | 58 +++++++++++++++++++++++++++++++------ src/agent37/runtime.ts | 27 ++++++++++++++++- src/bootstrap.test.ts | 19 ++++++++++++ src/bootstrap.ts | 14 +++++++-- 4 files changed, 106 insertions(+), 12 deletions(-) diff --git a/src/agent37/runtime.test.ts b/src/agent37/runtime.test.ts index 1814ef5..e9c1a51 100644 --- a/src/agent37/runtime.test.ts +++ b/src/agent37/runtime.test.ts @@ -950,15 +950,22 @@ describe("Agent37Runtime.runScript", () => { // '/root'` all came back exit 1, and the lane concluded /root did not // exist. It exists — root-owned, mode 0700, unreachable by the template's // `node` user — and every exit 1 was the `cd`. - const h = harness((request) => ({ - json: { - exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, - stdout: "", - // The composed script chose a fresh nonce for this call; echo it back - // so the runtime sees the marker it is actually looking for. - stderr: `${extractWorkdirUnusableMarker(request)}\nsh: 1: cd: can't cd to /root\n`, - }, - })); + const h = harness((request, index) => { + if (index === 0) { + // Main script: cd failed, sentinel + marker present. + return { + json: { + exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, + stdout: "", + stderr: `${extractWorkdirUnusableMarker(request)}\nsh: 1: cd: can't cd to /root\n`, + }, + }; + } + // Out-of-band verification probe: cd would also fail here. + return { + json: { exit_code: 1, stdout: "", stderr: "sh: 1: cd: can't cd to /root\n" }, + }; + }); const runtime = makeRuntime(h); await assert.rejects( runtime.runScript(RUNNING_HANDLE, { command: "id", cwd: "/root" }), @@ -970,6 +977,39 @@ describe("Agent37Runtime.runScript", () => { return true; }, ); + // Two exec calls: the composed script, then the out-of-band cd probe. + assert.equal(h.requests.length, 2); + assert.equal(execCommand(h.requests[1] as RecordedRequest), "cd '/root'\n"); + }); + + it("does not reclassify when a command forges the nonce but the workdir is actually usable", async () => { + // The in-band signals are a fast filter, not the whole answer: a hostile + // command can recover the nonce from its own /proc/self/cmdline and print + // it back, then exit 191. The out-of-band cd probe is what closes the + // gap — the probe runs no user command, so the sandbox has no material + // with which to forge its exit code. + const spoof = harness((request, index) => { + if (index === 0) { + return { + json: { + exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, + stdout: "", + stderr: `${extractWorkdirUnusableMarker(request)}\n`, + }, + }; + } + // Probe: cd succeeds; the workdir is fine. + return { json: { exit_code: 0, stdout: "", stderr: "" } }; + }); + const result = await makeRuntime(spoof).runScript(RUNNING_HANDLE, { + command: "grep __agent37_workdir_unusable__ /proc/self/cmdline >&2; exit 191", + cwd: "/work", + }); + assert.equal(result.exitCode, pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE); + // The probe still ran — that is what the fix is; the two exec calls prove + // the runtime did not accept the in-band pair on faith. + assert.equal(spoof.requests.length, 2); + assert.equal(execCommand(spoof.requests[1] as RecordedRequest), "cd '/work'\n"); }); it("requires BOTH the sentinel status and the marker before reclassifying", async () => { diff --git a/src/agent37/runtime.ts b/src/agent37/runtime.ts index 6f90ad5..47bfc02 100644 --- a/src/agent37/runtime.ts +++ b/src/agent37/runtime.ts @@ -866,11 +866,20 @@ export class Agent37Runtime implements SandboxRuntime, WorkflowRuntime { const result = await this.execRaw(handle.id, script, options.requestTimeoutMs); // Reclassify before the result is shaped: a command that never ran must not // be reported as a command that ran and failed. + // + // The in-band signals (exit code + nonce marker) are a fast negative filter + // — if either is absent this cannot be a `cd` failure and we skip the + // out-of-band probe. When both are present, a hostile command could still + // have recovered the nonce from `/proc/self/cmdline` and forged the pair, + // so we confirm by re-issuing the `cd` in a fresh exec that runs *no* + // user command. That probe is a plain shell built-in with no arguments the + // user chose, so it cannot be lied to from inside the sandbox. if ( cwd && workdirUnusableMarker && result.exit_code === AGENT37_WORKDIR_UNUSABLE_EXIT_CODE && - combineOutput(result.stdout, result.stderr).includes(workdirUnusableMarker) + combineOutput(result.stdout, result.stderr).includes(workdirUnusableMarker) && + (await this.probeCwdUnusable(handle.id, cwd)) ) { throw new Agent37WorkdirUnusableError( handle.id, @@ -1066,6 +1075,22 @@ export class Agent37Runtime implements SandboxRuntime, WorkflowRuntime { this.instanceUrls.delete(id); } + /** + * Out-of-band verification that `cd ` cannot succeed on this instance. + * + * Runs a plain `cd` with no user command through a fresh exec. The command + * body carries only the caller's cwd — not the nonce and not their command + * — so the sandbox has no material with which to forge a specific exit + * code, and any nonzero result is the shell's own report of the mount. + * Returns `true` when the `cd` really would have failed (i.e. the + * workdir-unusable reclassification is correct), `false` when the fast + * signals were a spoof or a coincidence. + */ + private async probeCwdUnusable(id: string, cwd: string): Promise { + const probe = await this.execRaw(id, `cd ${shellQuote(cwd)}\n`); + return probe.exit_code !== 0; + } + private async execRaw( id: string, command: string, diff --git a/src/bootstrap.test.ts b/src/bootstrap.test.ts index e746def..efa29f9 100644 --- a/src/bootstrap.test.ts +++ b/src/bootstrap.test.ts @@ -182,6 +182,25 @@ describe("buildRelayfileMountLinkShell", () => { await rm(root, { recursive: true, force: true }); } }); + + it("accepts a binDir with a trailing slash without flagging the install as shadowed", async () => { + // `command -v` normalizes the resolved PATH entry, so a `binDir` of + // `/foo/` would compare against `/foo//name` if we naïvely concatenated + // and the post-link verification would fail a good install. + const root = await scratch(); + try { + const globalRoot = await fakeGlobalRoot(root); + const binDir = join(root, "bin"); + const result = await sh( + buildRelayfileMountLinkShell({ binDir: `${binDir}/`, searchRoots: [globalRoot] }), + { env: { PATH: `${binDir}:${process.env.PATH ?? ""}` } }, + ); + assert.equal(result.code, 0, result.stderr); + assert.match(result.stdout.trim(), /bin\/relayfile-mount$/); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); }); describe("buildGhInstallShell", () => { diff --git a/src/bootstrap.ts b/src/bootstrap.ts index 54907a9..61e78aa 100644 --- a/src/bootstrap.ts +++ b/src/bootstrap.ts @@ -149,7 +149,12 @@ export function buildRelayfileMountLinkShell(opts: RelayfileMountLinkShellOption const argsExpr = useNpmFallback ? `"$(npm root -g 2>/dev/null)"${rootsExpr ? ` ${rootsExpr}` : ""}` : rootsExpr; - const linkPath = `${binDir}/${linkName}`; + // Strip a trailing slash before joining: `command -v` normalizes the PATH + // entry that resolves the lookup, so a `binDir` of `/foo/` would otherwise + // build an expected `linkPath` of `/foo//name` that never matches the + // normalized `/foo/name` and the post-link verification would fail a + // perfectly good install. + const linkPath = `${binDir.replace(/\/+$/, "")}/${linkName}`; return [ `set -e`, @@ -288,7 +293,12 @@ export function buildGhInstallShell(opts: GhInstallShellOptions): string { ); } - const ghPath = `${binDir}/gh`; + // Strip a trailing slash before joining: `command -v` normalizes the PATH + // entry that resolves the lookup, so a `binDir` of `/foo/` would otherwise + // build an expected `ghPath` of `/foo//gh` that never matches the + // normalized `/foo/gh` and the post-install verification would fail a + // perfectly good install. + const ghPath = `${binDir.replace(/\/+$/, "")}/gh`; lines.push( `tar -xzf "$__gh_tgz" -C ${shellQuote(workDir)}`, `cp ${shellQuote(workDir)}/"\${__gh_name}"/bin/gh ${shellQuote(ghPath)}`, From 9f9fb8cc1f079056da152e43773db22dc4f6b906 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 25 Aug 2026 21:15:11 +0200 Subject: [PATCH 7/8] refactor(agent37): reclassify workdir-unusable via a pre-execution probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic raised three related concerns on the previous marker-and-post-probe design: - A user command that mutates its own cwd after entering it (deletes, chmods away access) causes the POST-execution probe to see a broken directory and misclassify a command that actually ran as Agent37WorkdirUnusableError. - When the caller passes `requestTimeoutMs`, the post-probe was unbounded and could outlive the caller's wait budget. - A probe response that omits `exit_code` was treated as a workdir failure (`null !== 0`), so a malformed reply could manufacture a reclassification. The common thread is that no in-band signal — marker, nonce, exit code, or subsequent probe — can survive a user command that has full uid access to the same environment. Move the verification BEFORE the user command runs: on any `cwd`, `runScript` issues a plain `cd ''` through a fresh exec first. If that returns a KNOWN nonzero exit, throw Agent37WorkdirUnusableError immediately and never run the user script. Otherwise proceed with the composed script. This removes the whole class of "user command can spoof or invalidate the classifier" concerns: - The probe runs no user script, so the sandbox has no material with which to interpose on the probe's exit code. - The user command executes after the probe, so anything it does to the cwd afterwards cannot retroactively affect the classification. - Both exec calls carry the caller's `requestTimeoutMs`, so the total wait is bounded consistently with the previous single-exec behavior. - Only a KNOWN nonzero exit triggers reclassification; unknown responses fall through to the user command, whose own outcome speaks. Consequences for the surface: - composeScript no longer accepts a `workdirUnusableMarker` option and no longer emits any in-band marker; the cd guard is now a plain `cd '' || exit 191` so a race between the probe and the exec cannot let the user command run in the shell's inherited directory. - `AGENT37_WORKDIR_UNUSABLE_MARKER` stays exported for backward-compat and is documented as deprecated. `makeWorkdirUnusableMarker` and `probeCwdUnusable` are removed — nothing consumes them. Tests exercise the new shape: pre-probe fires before any user command when a cwd is set; the /root regression case throws with only the probe having run; a hostile command that fakes exit 191 + the deprecated marker prefix is reported as an ordinary command failure; a probe with no exit_code does NOT trigger reclassification; and `requestTimeoutMs` carries through to both exec calls. Co-Authored-By: Claude Opus 4.7 --- src/agent37/index.ts | 1 - src/agent37/runtime.test.ts | 195 ++++++++++++++++-------------------- src/agent37/runtime.ts | 156 +++++++++++------------------ 3 files changed, 145 insertions(+), 207 deletions(-) diff --git a/src/agent37/index.ts b/src/agent37/index.ts index d6feac6..601be68 100644 --- a/src/agent37/index.ts +++ b/src/agent37/index.ts @@ -10,7 +10,6 @@ export { Agent37Runtime, Agent37UnknownExitCodeError, Agent37WorkdirUnusableError, - makeWorkdirUnusableMarker, } from "./runtime.js"; export type { Agent37BundleFile, diff --git a/src/agent37/runtime.test.ts b/src/agent37/runtime.test.ts index e9c1a51..1863610 100644 --- a/src/agent37/runtime.test.ts +++ b/src/agent37/runtime.test.ts @@ -132,19 +132,6 @@ function execCommand(request: RecordedRequest): string { return parsed.command as string; } -/** - * Pull the per-invocation workdir-unusable marker out of a recorded exec body. - * - * Tests that want to simulate a `cd` failure must echo back the exact marker - * the runtime chose for that call — the shared prefix alone will no longer - * trigger reclassification, which is precisely the collision the nonce closes. - */ -function extractWorkdirUnusableMarker(request: RecordedRequest): string { - const cmd = execCommand(request); - const match = cmd.match(/__agent37_workdir_unusable__[0-9a-f]{32}/); - assert.ok(match, `expected a workdir-unusable marker in composed script:\n${cmd}`); - return match[0]; -} /** Strip comments so a source scan reads code, not the prose explaining it. */ function withoutComments(source: string): string { @@ -919,7 +906,13 @@ describe("Agent37Runtime.runScript", () => { env: { TOKEN_NAME: "it's fine" }, }); - const request = h.requests[0] as RecordedRequest; + // Two exec calls when cwd is set: pre-execution cwd probe, then the + // composed script itself. The probe body carries only the cwd — the + // sandbox has no material with which to spoof the workdir signal. + assert.equal(h.requests.length, 2); + assert.equal(execCommand(h.requests[0] as RecordedRequest), "cd '/work/repo'\n"); + + const request = h.requests[1] as RecordedRequest; assert.equal(request.url, `${TEST_BASE_URL}/v1/instances/ab12cd34ef/exec`); const parsed = JSON.parse(bodyText(request)) as Record; assert.deepEqual( @@ -927,45 +920,45 @@ describe("Agent37Runtime.runScript", () => { ["command"], "exec takes exactly one field; anything else is rejected by the API", ); - // The sentinel marker carries a per-invocation nonce so command output - // cannot spoof the workdir-unusable signal; everything else is fixed. - assert.match( - parsed.command as string, - /^cd '\/work\/repo' \|\| \{ printf '%s\\n' '__agent37_workdir_unusable__[0-9a-f]{32}' >&2; exit 191; \}\nexport TOKEN_NAME='it'\\''s fine'\nnpm test\n$/, + // No in-band marker: reclassification is decided by the probe, not by + // scanning the composed script's output. The cd guard is a plain + // `|| exit 191` so a race between the probe and the exec does not let + // the user command run in the shell's inherited directory. + assert.equal( + parsed.command, + "cd '/work/repo' || exit 191\nexport TOKEN_NAME='it'\\''s fine'\nnpm test\n", ); assert.deepEqual(result, { output: "ok\n", stdout: "ok\n", exitCode: 0 }); }); - it("falls back to the handle's workdir and omits cd when there is none", async () => { + it("falls back to the handle's workdir and omits both the probe and the cd when there is none", async () => { const h = harness(() => ({ json: { exit_code: 0, stdout: "", stderr: "" } })); const runtime = makeRuntime(h); + // Handle carries a workdir: probe first, then composed script. await runtime.runScript({ ...RUNNING_HANDLE, workdir: "/from/handle" }, { command: "ls" }); - assert.match(execCommand(h.requests[0] as RecordedRequest), /^cd '\/from\/handle' \|\| \{ printf /); + assert.equal(execCommand(h.requests[0] as RecordedRequest), "cd '/from/handle'\n"); + assert.match( + execCommand(h.requests[1] as RecordedRequest), + /^cd '\/from\/handle' \|\| exit 191\nls\n$/, + ); + // No workdir anywhere: no probe, no cd guard. Bare command only. await runtime.runScript(RUNNING_HANDLE, { command: "ls" }); - assert.equal(execCommand(h.requests[1] as RecordedRequest), "ls\n"); + assert.equal(execCommand(h.requests[2] as RecordedRequest), "ls\n"); }); - it("names an unusable workdir instead of reporting it as a failed command", async () => { + it("names an unusable workdir before running any user command", async () => { // The regression this guards: ten unrelated probes against `workdir: // '/root'` all came back exit 1, and the lane concluded /root did not // exist. It exists — root-owned, mode 0700, unreachable by the template's - // `node` user — and every exit 1 was the `cd`. - const h = harness((request, index) => { - if (index === 0) { - // Main script: cd failed, sentinel + marker present. - return { - json: { - exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, - stdout: "", - stderr: `${extractWorkdirUnusableMarker(request)}\nsh: 1: cd: can't cd to /root\n`, - }, - }; - } - // Out-of-band verification probe: cd would also fail here. - return { - json: { exit_code: 1, stdout: "", stderr: "sh: 1: cd: can't cd to /root\n" }, - }; - }); + // `node` user — and every exit 1 was the `cd`. The pre-execution probe + // catches this before any user command runs. + const h = harness(() => ({ + json: { + exit_code: 1, + stdout: "", + stderr: "sh: 1: cd: can't cd to /root\n", + }, + })); const runtime = makeRuntime(h); await assert.rejects( runtime.runScript(RUNNING_HANDLE, { command: "id", cwd: "/root" }), @@ -977,93 +970,75 @@ describe("Agent37Runtime.runScript", () => { return true; }, ); - // Two exec calls: the composed script, then the out-of-band cd probe. - assert.equal(h.requests.length, 2); - assert.equal(execCommand(h.requests[1] as RecordedRequest), "cd '/root'\n"); + // Only the probe ran — the user command never got a chance to execute. + assert.equal(h.requests.length, 1); + assert.equal(execCommand(h.requests[0] as RecordedRequest), "cd '/root'\n"); }); - it("does not reclassify when a command forges the nonce but the workdir is actually usable", async () => { - // The in-band signals are a fast filter, not the whole answer: a hostile - // command can recover the nonce from its own /proc/self/cmdline and print - // it back, then exit 191. The out-of-band cd probe is what closes the - // gap — the probe runs no user command, so the sandbox has no material - // with which to forge its exit code. - const spoof = harness((request, index) => { + it("does not reclassify when a hostile command tries to fake a workdir failure from inside the sandbox", async () => { + // The probe is the only signal a user command inside the sandbox cannot + // reach — it runs no user script and its exit code comes straight from + // the shell. Whatever a command prints or which code it exits with, if + // the probe said the cwd is usable, the result is an ordinary command + // exit, never `Agent37WorkdirUnusableError`. + const h = harness((_request, index) => { if (index === 0) { - return { - json: { - exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, - stdout: "", - stderr: `${extractWorkdirUnusableMarker(request)}\n`, - }, - }; + // Probe: cd succeeds; the workdir is fine. + return { json: { exit_code: 0, stdout: "", stderr: "" } }; } - // Probe: cd succeeds; the workdir is fine. - return { json: { exit_code: 0, stdout: "", stderr: "" } }; + // Main script: hostile command exits 191 and prints the deprecated + // marker prefix in an attempt to look like a workdir fault. + return { + json: { + exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, + stdout: "", + stderr: `${pkg.AGENT37_WORKDIR_UNUSABLE_MARKER}\nhostile output\n`, + }, + }; }); - const result = await makeRuntime(spoof).runScript(RUNNING_HANDLE, { - command: "grep __agent37_workdir_unusable__ /proc/self/cmdline >&2; exit 191", + const result = await makeRuntime(h).runScript(RUNNING_HANDLE, { + command: `printf '%s\\n' '${pkg.AGENT37_WORKDIR_UNUSABLE_MARKER}' >&2; exit 191`, cwd: "/work", }); assert.equal(result.exitCode, pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE); - // The probe still ran — that is what the fix is; the two exec calls prove - // the runtime did not accept the in-band pair on faith. - assert.equal(spoof.requests.length, 2); - assert.equal(execCommand(spoof.requests[1] as RecordedRequest), "cd '/work'\n"); + assert.match(result.output, /hostile output/); }); - it("requires BOTH the sentinel status and the marker before reclassifying", async () => { - // Either signal alone belongs to the command, not to the `cd`. A command - // is free to exit 191, and a command is free to print the marker. - const statusOnly = harness(() => ({ - json: { exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, stdout: "", stderr: "boom" }, - })); - const first = await makeRuntime(statusOnly).runScript(RUNNING_HANDLE, { - command: "exit 191", - cwd: "/work", + it("does not treat an unknown probe outcome as a workdir failure", async () => { + // A response that omits `exit_code` is unknown, not a `cd` failure. If + // the probe returns nothing conclusive, run the user command anyway and + // let its own outcome speak — spoofing the workdir classifier through a + // malformed probe response must not be possible. + const h = harness((_request, index) => { + if (index === 0) { + // Probe: no exit_code — unknown outcome. + return { json: { stdout: "", stderr: "" } }; + } + return { json: { exit_code: 0, stdout: "ran", stderr: "" } }; }); - assert.equal(first.exitCode, pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE); - - const markerOnly = harness(() => ({ - json: { exit_code: 1, stdout: pkg.AGENT37_WORKDIR_UNUSABLE_MARKER, stderr: "" }, - })); - const second = await makeRuntime(markerOnly).runScript(RUNNING_HANDLE, { - command: "echo marker", + const result = await makeRuntime(h).runScript(RUNNING_HANDLE, { + command: "true", cwd: "/work", }); - assert.equal(second.exitCode, 1); + assert.equal(result.exitCode, 0); + assert.equal(result.output, "ran"); }); - it("does not reclassify when a command reproduces the fixed prefix and exits 191", async () => { - // Requiring both signals is not enough on its own: a valid command can - // print the bare marker string AND exit 191 in the same run. The - // per-invocation nonce is what makes the pair spoofproof. - const collide = harness(() => ({ - json: { - exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, - stdout: "", - // Marker prefix without the nonce the composed script actually chose. - stderr: `${pkg.AGENT37_WORKDIR_UNUSABLE_MARKER}\ncommand output that happens to include the prefix\n`, - }, - })); - const result = await makeRuntime(collide).runScript(RUNNING_HANDLE, { - command: `printf '%s\\n' '${pkg.AGENT37_WORKDIR_UNUSABLE_MARKER}' >&2; exit 191`, + it("passes the caller's requestTimeoutMs to the pre-execution probe", async () => { + // The probe must be bounded by the same request budget as the main exec + // — otherwise `runScript` could outlive the caller's wait limit whenever + // a cwd is set. + const h = harness(() => ({ json: { exit_code: 0, stdout: "", stderr: "" } })); + await makeRuntime(h).runScript(RUNNING_HANDLE, { + command: "true", cwd: "/work", + requestTimeoutMs: 1234, }); - assert.equal(result.exitCode, pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE); - }); - - it("does not reclassify when no cwd was requested, because no cd was emitted", async () => { - const h = harness(() => ({ - json: { - exit_code: pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE, - stdout: "", - stderr: pkg.AGENT37_WORKDIR_UNUSABLE_MARKER, - }, - })); - const runtime = makeRuntime(h, { defaultHomeDir: "/home/node" }); - const result = await runtime.runScript(RUNNING_HANDLE, { command: "sh -c 'exit 191'" }); - assert.equal(result.exitCode, pkg.AGENT37_WORKDIR_UNUSABLE_EXIT_CODE); + assert.equal(h.requests.length, 2); + // AbortController presence on the probe request is the observable proof + // that the timeout was propagated to `execRaw`. + assert.equal((h.requests[0] as RecordedRequest).hasSignal, true, "probe must carry an abort signal"); + assert.equal((h.requests[1] as RecordedRequest).hasSignal, true, "main exec must carry an abort signal"); }); it("reports a nonzero exit as a result, not an error, and combines the streams", async () => { diff --git a/src/agent37/runtime.ts b/src/agent37/runtime.ts index 47bfc02..292d9b9 100644 --- a/src/agent37/runtime.ts +++ b/src/agent37/runtime.ts @@ -1,5 +1,4 @@ import { Buffer } from "node:buffer"; -import { randomUUID } from "node:crypto"; import { writeFile } from "node:fs/promises"; import type { @@ -302,48 +301,36 @@ export class Agent37UnknownExitCodeError extends Error { } /** - * Marker the composed script writes to stderr when its `cd` fails, and the - * exit status it leaves behind. + * Exit status the composed script leaves behind when its own `cd` guard + * trips — i.e. the cwd went away between the pre-execution probe and the + * script's own `cd`, and the user command was not permitted to run in the + * shell's inherited directory instead. * - * Both exist because a bare `cd || exit 1` is *indistinguishable from the - * command's own failure*, and that ambiguity has already cost real time: a - * lane pointed an Agent37 instance at `workdir: '/root'`, watched ten - * unrelated probes all return exit 1, and concluded from it that `/root` did - * not exist. It does exist — as `drwx------ root root`, unreachable by the - * template's `node` user — and every one of those exit 1s was the `cd`, not - * the command. Re-pointed at the real home, all ten passed. + * Reclassification of a run to {@link Agent37WorkdirUnusableError} is + * decided by the out-of-band probe in {@link Agent37Runtime.runScript}, not + * by scanning the result for this code — the code exists so the composed + * script fails visibly instead of silently misdirecting the user command, + * not as a signal a user command could not also produce. The probe is what + * a user command inside the sandbox cannot reach. * - * The status alone is not proof (a command may legitimately exit 191), and the - * marker alone is not proof (a command may legitimately print it), so - * {@link Agent37Runtime.runScript} requires both before it reclassifies a - * result as a workdir fault. Requiring "both" is not enough on its own either: - * a valid command can print the fixed prefix *and* exit 191 in the same run, - * so {@link composeScript} appends a per-invocation nonce that the running - * command has no way to see. See {@link makeWorkdirUnusableMarker}. + * The original bug that shaped this design: a lane pointed an Agent37 + * instance at `workdir: '/root'`, watched ten unrelated probes all return + * exit 1, and concluded from it that `/root` did not exist. It does exist + * — as `drwx------ root root`, unreachable by the template's `node` user — + * and every one of those exit 1s was the `cd`. The pre-execution probe + * catches that class of fault before any user command runs. */ export const AGENT37_WORKDIR_UNUSABLE_EXIT_CODE = 191; /** - * Stable prefix for the workdir-unusable sentinel line. - * - * The full marker emitted by any given composed script is this prefix followed - * by a per-invocation nonce, so an unrelated command cannot spoof it. - * - * @see AGENT37_WORKDIR_UNUSABLE_EXIT_CODE + * @deprecated The workdir-unusable classifier no longer relies on an in-band + * marker string — reclassification is decided by an out-of-band probe. This + * constant is preserved only so callers that pattern-match legacy shell + * output for debugging continue to compile. It is not emitted by + * {@link composeScript}. */ export const AGENT37_WORKDIR_UNUSABLE_MARKER = "__agent37_workdir_unusable__"; -/** - * Build a per-invocation workdir-unusable sentinel line. - * - * The prefix stays discoverable to a human reading the raw output, and the - * nonce guarantees the line cannot be reproduced by a command whose own output - * happens to include the prefix. - */ -export function makeWorkdirUnusableMarker(): string { - return `${AGENT37_WORKDIR_UNUSABLE_MARKER}${randomUUID().replace(/-/g, "")}`; -} - /** * Raised when the instance could not enter the requested working directory. * @@ -850,43 +837,44 @@ export class Agent37Runtime implements SandboxRuntime, WorkflowRuntime { assertValidEnv(options.env); } const cwd = options.cwd ?? handle.workdir; - // Per-invocation nonce, generated only when a cd is actually emitted. A - // command that legitimately prints the fixed prefix and exits 191 will - // not carry this suffix, so reclassification cannot collide with it. - const workdirUnusableMarker = cwd ? makeWorkdirUnusableMarker() : undefined; + // Prove the cwd is usable BEFORE running any user code. This is the only + // signal a user command cannot spoof: the probe runs no user script, so + // whatever is inside the sandbox has nothing to interpose on the probe's + // exit code and no way to mutate the cwd's accessibility before the + // probe sees it. A later `cd` failure inside the user's own exec is + // therefore always the command's own result, never proof that the + // caller's workdir was unusable. + // + // The probe is skipped when no cwd was requested — there is nothing to + // verify — and its own `requestTimeoutMs` matches the main exec so the + // caller's wait budget bounds both halves. + if (cwd) { + const probe = await this.execRaw( + handle.id, + `cd ${shellQuote(cwd)}\n`, + options.requestTimeoutMs, + ); + // Only a KNOWN nonzero exit is proof of a workdir fault. An unknown + // outcome (no exit_code in the response) is exactly that — unknown — + // and treating it as a `cd` failure would let a malformed response + // spoof the reclassification. + if (probe.exit_code !== null && probe.exit_code !== 0) { + throw new Agent37WorkdirUnusableError( + handle.id, + cwd, + combineOutput(probe.stdout, probe.stderr), + ); + } + } const script = composeScript(options.command, { ...(cwd ? { cwd } : {}), ...(options.env ? { env: options.env } : {}), - ...(workdirUnusableMarker ? { workdirUnusableMarker } : {}), }); // Only `requestTimeoutMs` becomes an abort signal. `timeoutMs` is a command // lifetime, and it got here only because the provider's own cap already // satisfies it — turning it into an HTTP abort would abandon the response // while the command ran on. const result = await this.execRaw(handle.id, script, options.requestTimeoutMs); - // Reclassify before the result is shaped: a command that never ran must not - // be reported as a command that ran and failed. - // - // The in-band signals (exit code + nonce marker) are a fast negative filter - // — if either is absent this cannot be a `cd` failure and we skip the - // out-of-band probe. When both are present, a hostile command could still - // have recovered the nonce from `/proc/self/cmdline` and forged the pair, - // so we confirm by re-issuing the `cd` in a fresh exec that runs *no* - // user command. That probe is a plain shell built-in with no arguments the - // user chose, so it cannot be lied to from inside the sandbox. - if ( - cwd && - workdirUnusableMarker && - result.exit_code === AGENT37_WORKDIR_UNUSABLE_EXIT_CODE && - combineOutput(result.stdout, result.stderr).includes(workdirUnusableMarker) && - (await this.probeCwdUnusable(handle.id, cwd)) - ) { - throw new Agent37WorkdirUnusableError( - handle.id, - cwd, - combineOutput(result.stdout, result.stderr), - ); - } return { output: combineOutput(result.stdout, result.stderr), ...(result.stdout ? { stdout: result.stdout } : {}), @@ -1075,22 +1063,6 @@ export class Agent37Runtime implements SandboxRuntime, WorkflowRuntime { this.instanceUrls.delete(id); } - /** - * Out-of-band verification that `cd ` cannot succeed on this instance. - * - * Runs a plain `cd` with no user command through a fresh exec. The command - * body carries only the caller's cwd — not the nonce and not their command - * — so the sandbox has no material with which to forge a specific exit - * code, and any nonzero result is the shell's own report of the mount. - * Returns `true` when the `cd` really would have failed (i.e. the - * workdir-unusable reclassification is correct), `false` when the fast - * signals were a spoof or a coincidence. - */ - private async probeCwdUnusable(id: string, cwd: string): Promise { - const probe = await this.execRaw(id, `cd ${shellQuote(cwd)}\n`); - return probe.exit_code !== 0; - } - private async execRaw( id: string, command: string, @@ -1327,28 +1299,20 @@ export function assertValidEnv(env: Record): void { */ export function composeScript( command: string, - options: { - cwd?: string; - env?: Record; - /** - * Sentinel line the `cd` failure branch writes to stderr. Callers wiring - * this into a reclassifier should pass a unique value per invocation (see - * {@link makeWorkdirUnusableMarker}) so command output cannot spoof the - * failure signal. Defaults to the stable prefix for compose-only use. - */ - workdirUnusableMarker?: string; - } = {}, + options: { cwd?: string; env?: Record } = {}, ): string { const lines: string[] = []; if (options.cwd) { - // POSIX `sh`, not bash: Agent37's exec plane runs dash, where a bashism - // like ${PIPESTATUS[0]} is a "Bad substitution". `{ …; }` and `printf` are - // both POSIX, so this line runs on either shell. - const marker = options.workdirUnusableMarker ?? AGENT37_WORKDIR_UNUSABLE_MARKER; + // Fail-fast guard: if the pre-execution probe passed but the cwd went + // away between the two exec calls, do NOT let the user command run in + // whatever directory the shell inherits — that is exactly the class of + // silent misdirection the workdir-unusable classifier exists to prevent. + // The sentinel exit code stays consistent for debuggability, but the + // caller no longer relies on any in-band marker: reclassification is + // decided by the out-of-band probe in {@link Agent37Runtime.runScript}. + // POSIX `sh`, not bash: Agent37's exec plane runs dash. lines.push( - `cd ${shellQuote(options.cwd)} || { printf '%s\\n' ` + - `${shellQuote(marker)} >&2; ` + - `exit ${AGENT37_WORKDIR_UNUSABLE_EXIT_CODE}; }`, + `cd ${shellQuote(options.cwd)} || exit ${AGENT37_WORKDIR_UNUSABLE_EXIT_CODE}`, ); } for (const [key, value] of Object.entries(options.env ?? {})) { From 8519565e4b57254bc16fe9cc57074c2e48b1d383 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 25 Aug 2026 21:24:19 +0200 Subject: [PATCH 8/8] fix(agent37): share the request budget across the probe and the main exec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic flagged that passing `options.requestTimeoutMs` verbatim to both the pre-execution cwd probe and the composed script's exec lets `runScript` wait up to 2× the caller's timeout on any cwd-required call — the caller's contract says "wait at most requestTimeoutMs", not "wait at most requestTimeoutMs per hop". Capture a single start timestamp when `requestTimeoutMs` is set, then pass the REMAINING budget (`requestTimeoutMs - elapsed`) to each downstream `execRaw`. If the probe consumes the entire budget, floor the second call at 1ms rather than 0 — the client treats `<= 0` as "no timeout at all", which would silently uncancel an already-expired request; 1ms makes the abort fire immediately instead. Co-Authored-By: Claude Opus 4.7 --- src/agent37/runtime.ts | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/agent37/runtime.ts b/src/agent37/runtime.ts index 292d9b9..8502b00 100644 --- a/src/agent37/runtime.ts +++ b/src/agent37/runtime.ts @@ -846,13 +846,30 @@ export class Agent37Runtime implements SandboxRuntime, WorkflowRuntime { // caller's workdir was unusable. // // The probe is skipped when no cwd was requested — there is nothing to - // verify — and its own `requestTimeoutMs` matches the main exec so the - // caller's wait budget bounds both halves. + // verify. + // + // When both the probe and the main exec run, they share ONE request + // budget: `requestTimeoutMs` from the caller is a total wait limit for + // `runScript`, not per-exec. Passing the full timeout to each call in + // sequence would let a two-hop runScript wait up to `2 × requestTimeoutMs`, + // silently exceeding the caller's contract. Instead, capture a monotonic + // start, and pass the remaining budget to each downstream `execRaw`. + // Node's `Date.now()` is a millisecond wall clock; a small skew between + // successive reads is fine because the budget is coarse to begin with. + const startAt = options.requestTimeoutMs !== undefined ? Date.now() : undefined; + const remainingBudgetMs = (): number | undefined => { + if (options.requestTimeoutMs === undefined || startAt === undefined) return undefined; + const remaining = options.requestTimeoutMs - (Date.now() - startAt); + // The client treats `<= 0` as "no timeout at all", which would silently + // uncancel an already-expired request. Floor at 1 so an over-budget + // call still aborts immediately rather than running to completion. + return remaining <= 0 ? 1 : remaining; + }; if (cwd) { const probe = await this.execRaw( handle.id, `cd ${shellQuote(cwd)}\n`, - options.requestTimeoutMs, + remainingBudgetMs(), ); // Only a KNOWN nonzero exit is proof of a workdir fault. An unknown // outcome (no exit_code in the response) is exactly that — unknown — @@ -874,7 +891,7 @@ export class Agent37Runtime implements SandboxRuntime, WorkflowRuntime { // lifetime, and it got here only because the provider's own cap already // satisfies it — turning it into an HTTP abort would abandon the response // while the command ran on. - const result = await this.execRaw(handle.id, script, options.requestTimeoutMs); + const result = await this.execRaw(handle.id, script, remainingBudgetMs()); return { output: combineOutput(result.stdout, result.stderr), ...(result.stdout ? { stdout: result.stdout } : {}),