diff --git a/adapters/claude/adapter.sh b/adapters/claude/adapter.sh index 2e29fe25..8a259540 100644 --- a/adapters/claude/adapter.sh +++ b/adapters/claude/adapter.sh @@ -49,8 +49,10 @@ fi # --restricted no user/project/local settings files; --settings # (the gate hook) still applies; no code-running # tool unless --tools names it; file tools confined -# to the working directory. Defence in depth: the -# S0 lines above don't depend on it. +# to the working directory; no CLAUDE.md file or +# auto-memory in the prompt. The S0 lines above +# don't depend on it, but it is what keeps founder +# and repository memory out; a test fails without it. # --tools the built-in tool limit (S0 line 5); empty = none # --allowedTools the same tools plus the mosaic MCP server, so # --permission-mode dontAsk nothing waits on a prompt and anything else is denied diff --git a/packages/bus/README.md b/packages/bus/README.md index 29b4add7..fff96910 100644 --- a/packages/bus/README.md +++ b/packages/bus/README.md @@ -63,6 +63,10 @@ underlying Broker's test-level binding API to bypass runtime process checks. A rebind of a run that already has `session.ended` refuses with `run-ended`, also after a restart, and a refused bind leaves the run unbound. +A request that carries a safe-integer `id` gets the same `id` on its reply, +refusals included, so the host can match each reply to its request +(`packages/cli/README.md`). A request without one gets a reply without one. + S6 adds launch ops on the same IPC channel, one reply each (`{ok:true,result}` or `{ok:false,error}`), none of them socket verbs: diff --git a/packages/bus/src/process.mjs b/packages/bus/src/process.mjs index bf0ce4f1..68276a03 100644 --- a/packages/bus/src/process.mjs +++ b/packages/bus/src/process.mjs @@ -10,6 +10,9 @@ function launchOp(m) { if (m.op === 'endLaunch') return runtime.endLaunch(m.record); return runtime.credentialStatus(m.business, m.role); } +// The host's request id comes back on the reply, so a late reply can't +// answer a later request (host.mjs). A message without one gets none. +const tag = (message, reply) => (Number.isSafeInteger(message?.id) ? { ...reply, id: message.id } : reply); let runtime, booted = false, closing = false; @@ -33,18 +36,18 @@ if (!process.send) { try { if (message?.op === 'bindLaunch' && runtime) { try { - process.send({ ok: true, launch: runtime.bindLaunch(message.record) }); + process.send(tag(message, { ok: true, launch: runtime.bindLaunch(message.record) })); } catch (e) { - process.send({ ok: false, error: e instanceof BusError ? e.code : 'bind-refused' }); + process.send(tag(message, { ok: false, error: e instanceof BusError ? e.code : 'bind-refused' })); } return; } - // S6 launcher ops, one reply each; the host sends them one at a time like bindLaunch. + // S6 launcher ops, one reply each, like bindLaunch. if (LAUNCH_OPS.has(message?.op) && runtime) { try { - process.send({ ok: true, result: launchOp(message) }); + process.send(tag(message, { ok: true, result: launchOp(message) })); } catch (e) { - process.send({ ok: false, error: e instanceof BusError ? e.code : 'launch-op-refused' }); + process.send(tag(message, { ok: false, error: e instanceof BusError ? e.code : 'launch-op-refused' })); } return; } @@ -69,7 +72,7 @@ if (!process.send) { } process.send({ ok: true, path: runtime.path, launches: runtime.launches, readers: runtime.readers }); } catch (e) { - process.send?.({ ok: false, error: e instanceof BusError ? e.code : 'startup-refused' }, () => + process.send?.(tag(message, { ok: false, error: e instanceof BusError ? e.code : 'startup-refused' }), () => close(2), ); } diff --git a/packages/bus/tests/end-launch.test.mjs b/packages/bus/tests/end-launch.test.mjs index 574129b7..68720d71 100644 --- a/packages/bus/tests/end-launch.test.mjs +++ b/packages/bus/tests/end-launch.test.mjs @@ -175,6 +175,11 @@ test('broker process: launch ops authorize role.launch, record refusals and end assert.deepEqual((await ask(child, { op: 'credentialStatus', business: 'demo', role: 'pm' })).result, []); const end = await ask(child, { op: 'endLaunch', record: { business: 'demo', run: 'pm-1', reason: 'stopped', exitCode: 0 } }); assert.deepEqual(end, { ok: true, result: { released: true } }); + // The host's request id comes back on every launch-op and bind reply, refusals too. + assert.equal((await ask(child, { op: 'identity', cap: cto, id: 7 })).id, 7); + assert.deepEqual(await ask(child, { op: 'identity', cap: 'f'.repeat(64), id: 8 }), { ok: false, error: 'unauthenticated', id: 8 }); + assert.equal((await ask(child, { op: 'bindLaunch', record: record('coder', 'coder-1'), id: 9 })).id, 9); + assert.deepEqual(await ask(child, { op: 'bindLaunch', record: record('coder', 'coder-1'), id: 10 }), { ok: false, error: 'duplicate-run', id: 10 }); await assert.rejects(new Client({ path: ready.path, cap: pm }).call('agents'), /unauthenticated/); const trail = await reader.call('trail', { subject: 'pm-1' }); assert.deepEqual( diff --git a/packages/cli/README.md b/packages/cli/README.md index be9e6531..32977afa 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -62,6 +62,9 @@ mosaic launches list [--json] - `launches off` and `on` are the broker's `launch.revoke` and `launch.restore`. While off, every `role.launch` refuses with `launch-revoked`; running sessions keep running. Stop them with `stop`. + `bus start --pm` doesn't ask the broker (`launchPm` skips + `authorizeLaunch`): the human launches the PM, so `launches off` doesn't + stop it. The other checks in "Sessions" below still apply. - `stop` and `launches list` read `/bus-host/sessions.json`, not the bus (see "Sessions" below). `stop` refuses inside an agent run; `launches list` does not, because the file is readable to the same user @@ -106,8 +109,12 @@ uses: `bindLaunch(record)` binds a launched run's process identity one of the broker process's launch ops (`identity`, `authorizeLaunch`, `refuse`, `endLaunch`, `credentialStatus`); `beforeClose(fn)` runs `fn` before the broker closes, so the launcher stops its sessions first. -Requests to the broker process go one at a time, because its replies carry -no request id. +Requests to the broker process go one at a time. Each carries an id, and +the broker echoes it on the reply. If a reply doesn't arrive within 10 s, or +arrives with an id no request is waiting for, the channel is broken: the +waiting request refuses with `broker-channel-broken`, so does every later +one, and the host stops with exit 1 for the unit to restart. A late reply +never answers a later request, and a launch waiting on one refuses. SIGTERM or SIGINT stops the notifier after its poll in flight, then closes the broker and removes `host.json`. If either child dies on its own, the @@ -166,7 +173,9 @@ directory; `launches/.jsonl` (0600, append-only) logs every launch, refusal and end; `workspaces///` (0700) is kept across launches; `bus-host/sessions.json` (0600) lists the running sessions. -When the host closes, the launcher stops taking requests, sends SIGTERM to +When the host closes, the launcher stops taking requests and drops every +open `launch.sock` connection, so a client that never ends its side can't +hold the close. It sends SIGTERM to every runner (again each second, see "Limits"), waits up to 30 s, then SIGKILLs what is left and ends those launches as `killed`. diff --git a/packages/cli/src/host.mjs b/packages/cli/src/host.mjs index 35c44f58..0073091b 100644 --- a/packages/cli/src/host.mjs +++ b/packages/cli/src/host.mjs @@ -25,6 +25,7 @@ const NOTIFIER = fileURLToPath(new URL("./notifier-process.mjs", import.meta.url export const BOOT_TIMEOUT_MS = 30000; const START_TIMEOUT_MS = 15000; const CLOSE_TIMEOUT_MS = 20000; +const REQUEST_TIMEOUT_MS = 10000; export const hostDir = (dataRoot) => join(dataRoot, "bus-host"); export const hostFile = (dataRoot) => join(hostDir(dataRoot), "host.json"); @@ -94,8 +95,9 @@ export function watchChildren(children, onDeath) { // boot: the {op:'boot'} config from bootConfig(). notifier: null, or // {binding, base?, pollMs?}; base and pollMs exist for the tests, and the -// command line never sets them. Resolves once both children are up. -export async function startHost({ boot, business, notifier = null, bootTimeoutMs = BOOT_TIMEOUT_MS, log = (l) => process.stderr.write(`mosaic-bus: ${l}\n`) }) { +// command line never sets them, nor requestTimeoutMs. Resolves once both +// children are up. +export async function startHost({ boot, business, notifier = null, bootTimeoutMs = BOOT_TIMEOUT_MS, requestTimeoutMs = REQUEST_TIMEOUT_MS, log = (l) => process.stderr.write(`mosaic-bus: ${l}\n`) }) { const dataRoot = boot.dataRoot; const prior = readHostState(dataRoot); if (prior?.live) throw new CliError(`a bus host already runs for ${prior.business} (pid ${prior.pid})`, 3); @@ -152,14 +154,52 @@ export async function startHost({ boot, business, notifier = null, bootTimeoutMs const done = new Promise((r) => (finish = r)); const hooks = []; - // Replies from process.mjs carry no request id, so requests go one at a time. + // Requests go one at a time, each with an id that process.mjs echoes. A + // reply that misses the wait, or carries an id no request is waiting for, + // breaks the channel: the waiting request and every later one refuse, and + // the host stops with exit 1 so the unit restarts it. A late reply can + // never answer a later request. let queue = Promise.resolve(); + let seq = 0; + let pending = null; + let broken = null; + const fail = (code, text) => Object.assign(new CliError(text, 1), { code }); + function breakChannel(why) { + if (broken) return; + broken = why; + if (pending) { + clearTimeout(pending.timer); + pending.reject(fail("broker-channel-broken", `broker channel broken: ${why}`)); + pending = null; + } + if (stopping) return; + log(`broker channel broken (${why}); stopping the host`); + close(1); + } + broker.on("message", (m) => { + if (pending && m?.id === pending.id) { + clearTimeout(pending.timer); + const { resolve } = pending; + pending = null; + resolve(m); + } else breakChannel(pending ? "reply for another request" : "reply with no request waiting"); + }); + broker.once("exit", () => { + if (!pending) return; + clearTimeout(pending.timer); + pending.reject(fail("broker-exited", "broker exited before it replied")); + pending = null; + }); function request(message, { what, pick }) { const run = queue.then(async () => { - if (closing || !broker.connected) throw Object.assign(new CliError("bus host is closing", 1), { code: "host-closing" }); - const r = firstReply(broker, 10000, "broker"); - broker.send(message); - const m = await r; + if (broken) throw fail("broker-channel-broken", `broker channel broken: ${broken}`); + if (closing || !broker.connected) throw fail("host-closing", "bus host is closing"); + const id = ++seq; + const m = await new Promise((resolve, reject) => { + const timer = setTimeout(() => breakChannel(`no reply within ${Math.round(requestTimeoutMs / 1000)} s`), requestTimeoutMs); + pending = { id, resolve, reject, timer }; + broker.send({ ...message, id }); + }); if (m?.ok !== true) { const code = typeof m?.error === "string" ? m.error : `${what}-refused`; throw Object.assign(new CliError(`${what} refused: ${code}`, 3), { code }); diff --git a/packages/cli/src/launcher.mjs b/packages/cli/src/launcher.mjs index ec01a2c3..b07ae333 100644 --- a/packages/cli/src/launcher.mjs +++ b/packages/cli/src/launcher.mjs @@ -83,8 +83,8 @@ export function claudeVersion(env = process.env) { } // host: startHost()'s handle. tracker: true when the broker has task verbs for -// the business. adapters, namespace, sessionDefaults and versions exist for -// the tests; the command line never sets them. +// the business. adapters, namespace, sessionDefaults, versions and +// stopTimeoutMs exist for the tests; the command line never sets them. export function createLauncher({ host, system, @@ -94,6 +94,7 @@ export function createLauncher({ namespace = true, sessionDefaults = {}, versions = null, + stopTimeoutMs = STOP_TIMEOUT_MS, log = (l) => process.stderr.write(`mosaic-bus: ${l}\n`), }) { const dataRoot = system.dataRoot; @@ -114,6 +115,7 @@ export function createLauncher({ let chain = Promise.resolve(); let closed = false; let server = null; + const sockets = new Set(); const record = (entry) => { try { @@ -335,6 +337,8 @@ export function createLauncher({ rmSync(path); } server = createServer((socket) => { + sockets.add(socket); + socket.once("close", () => sockets.delete(socket)); let buf = ""; let answered = false; const reply = (obj) => { @@ -377,7 +381,11 @@ export function createLauncher({ async function close() { closed = true; if (server) { - await new Promise((r) => server.close(r)); + // server.close waits for every connection, and a client that never + // ends its side (or never sends) would hold it; so they go first. + const stopped = new Promise((r) => server.close(r)); + for (const socket of sockets) socket.destroy(); + await stopped; rmSync(launchSocketPath(dataRoot), { force: true }); } await chain; @@ -399,7 +407,7 @@ export function createLauncher({ if (startTimeOf(s.pid) === s.startTime) process.kill(s.pid, "SIGKILL"); } catch {} } - }, STOP_TIMEOUT_MS); + }, stopTimeoutMs); await Promise.all(pending); // The exit handlers run on the same tick as the close events; let them finish. while (children.size) await new Promise((r) => setTimeout(r, 20)); diff --git a/packages/cli/tests/host.test.mjs b/packages/cli/tests/host.test.mjs index f8f11ec9..f8914479 100644 --- a/packages/cli/tests/host.test.mjs +++ b/packages/cli/tests/host.test.mjs @@ -203,6 +203,65 @@ test("a second host for the same data root refuses with exit 3 while the first r assert.equal(await host.close(0), 0); }); +test("a broker reply that misses the wait breaks the channel: the late reply answers nothing, later requests refuse, the host exits 1", { timeout: 30000 }, async (t) => { + const root = tmp(t); + const f = fixture(root); + makeDeployment(root); + const boot = bootConfig({ system: loadSystem({ env: f.env }), businessId: "acme", env: f.env }); + const logs = []; + const host = await startHost({ boot, business: "acme", requestTimeoutMs: 1000, log: (l) => logs.push(l) }); + // Hooks run in order: the broker resumes before the close, which a + // stopped broker would hold. + t.after(() => { + try { + process.kill(host.pids.broker, "SIGCONT"); + } catch {} + }); + t.after(() => host.close(0)); + const record = (role, run) => ({ business: "acme", role, run, harness: "pi", pid: process.pid, startTime: startTimeOf(process.pid) }); + const coder = await host.bindLaunch(record("coder", "coder-run")); + assert.equal((await host.op({ op: "identity", cap: coder.cap })).role, "coder"); + + // Stall the broker with a request waiting and a bind queued behind it. + process.kill(host.pids.broker, "SIGSTOP"); + const code = (p) => p.then((v) => ({ answered: v }), (e) => e.code); + const first = code(host.op({ op: "identity", cap: "bogus" })); + const second = code(host.bindLaunch(record("reviewer", "reviewer-run"))); + assert.equal(await first, "broker-channel-broken"); + assert.equal(await second, "broker-channel-broken", "a queued request never reaches the broker"); + process.kill(host.pids.broker, "SIGCONT"); + + // The broker answers the stalled request late; nothing takes that reply. + assert.equal(await host.done, 1); + assert.ok(logs.some((l) => /^broker channel broken \(no reply within 1 s\); stopping the host$/.test(l)), logs.join("\n")); + assert.equal(await code(host.op({ op: "identity", cap: coder.cap })), "broker-channel-broken"); +}); + +test("a broker reply with another request's id, or none, breaks the channel and the host exits 1", { timeout: 30000 }, async (t) => { + // The broker echoes whatever id it's sent, so changing the id on the way + // out gives the host the reply a confused broker would send. + let what, tamper; + spySends(t, (m) => m?.cap === "tamper" && tamper(m)); + for ([what, tamper] of [ + ["another id", (m) => (m.id += 1000)], + ["no id", (m) => delete m.id], + ]) { + const root = tmp(t); + const f = fixture(root); + makeDeployment(root); + const boot = bootConfig({ system: loadSystem({ env: f.env }), businessId: "acme", env: f.env }); + const logs = []; + const host = await startHost({ boot, business: "acme", log: (l) => logs.push(l) }); + t.after(() => host.close(0)); + const record = (role, run) => ({ business: "acme", role, run, harness: "pi", pid: process.pid, startTime: startTimeOf(process.pid) }); + const coder = await host.bindLaunch(record("coder", "coder-run")); + await assert.rejects(host.op({ op: "identity", cap: "tamper" }), (e) => e.code === "broker-channel-broken", what); + await assert.rejects(host.op({ op: "identity", cap: coder.cap }), (e) => e.code === "broker-channel-broken", what); + assert.equal(await host.done, 1, what); + assert.ok(logs.includes("broker channel broken (reply for another request); stopping the host"), `${what}: ${logs.join("\n")}`); + } +}); + test("a notifier that refuses stops the broker and the host refuses with exit 3", async (t) => { const root = tmp(t); const f = fixture(root); diff --git a/packages/cli/tests/launcher.test.mjs b/packages/cli/tests/launcher.test.mjs index b0c0ba09..75ee91b8 100644 --- a/packages/cli/tests/launcher.test.mjs +++ b/packages/cli/tests/launcher.test.mjs @@ -53,7 +53,7 @@ function prepare(t, more = (doc) => doc) { return { root, f, adapter }; } -async function setup(t, more = (doc) => doc, { root, f, adapter } = prepare(t, more)) { +async function setup(t, more = (doc) => doc, { root, f, adapter } = prepare(t, more), options = {}) { const system = loadSystem({ env: f.env }); const boot = bootConfig({ system, businessId: "acme", env: f.env }); const logs = []; @@ -68,6 +68,7 @@ async function setup(t, more = (doc) => doc, { root, f, adapter } = prepare(t, m sessionDefaults: { pollInterval: 100 }, versions: { pi: "0.85.1", claude: null }, log: (l) => logs.push(l), + ...options, }); await launcher.listen(); } catch (e) { @@ -267,6 +268,16 @@ for (const exited of [false, true]) { // The broker child exits on the lost IPC channel; the session runs on. await until(() => !existsSync(join(f.dataRoot, "bus", "writer.lock"))); assert.equal(startTimeOf(left.runnerPid), left.runnerStartTime); + if (!exited) { + // Stopped, the leftover neither polls the dead broker nor answers + // SIGTERM: only the next host's SIGKILL ends it. + for (const pid of [left.pid, left.runnerPid]) process.kill(pid, "SIGSTOP"); + t.after(() => { + for (const [pid, start] of [[left.pid, left.startTime], [left.runnerPid, left.runnerStartTime]]) { + if (startTimeOf(pid) === start) process.kill(pid, "SIGKILL"); + } + }); + } if (exited) { await until(() => startTimeOf(left.pid) === null); assert.match(readFileSync(runnerLog, "utf8"), /broker unreachable after 30 tries[\s\S]*exiting 23/); @@ -309,3 +320,75 @@ test("a malformed sessions.json refuses the host start with exit 3 and stays as assert.equal(readFileSync(file, "utf8"), "{"); assert.equal(existsSync(join(given.f.dataRoot, "bus", "writer.lock")), false, "the host closed"); }); + +const within = (p, ms, what) => Promise.race([p, new Promise((_, reject) => setTimeout(() => reject(new Error(`${what} took over ${ms} ms`)), ms).unref())]); + +test("an over-long launch request is refused at once, not at the 10 s idle timeout", async (t) => { + const { f } = await setup(t); + const s = connect(launchSocketPath(f.dataRoot)); + t.after(() => s.destroy()); + let buf = ""; + s.on("data", (b) => (buf += b)); + s.write("x".repeat(5000)); + await within(once(s, "end"), 3000, "the refusal"); + assert.deepEqual(JSON.parse(buf), { ok: false, error: "invalid-request" }); +}); + +test("a launch client that never closes its side doesn't hold the host's close", { timeout: 30000 }, async (t) => { + const { f, close } = await setup(t); + const path = launchSocketPath(f.dataRoot); + // One got its reply and never ends; one never sends anything. + const answered = connect({ path, allowHalfOpen: true }); + const silent = connect({ path, allowHalfOpen: true }); + const connected = once(silent, "connect"); + const gone = Promise.all([once(answered, "close"), once(silent, "close")]); + const drop = () => { + answered.destroy(); + silent.destroy(); + }; + t.after(drop); + let buf = ""; + answered.on("data", (b) => (buf += b)); + answered.write("not json\n"); + await once(answered, "end"); + assert.deepEqual(JSON.parse(buf), { ok: false, error: "invalid-request" }); + await within(connected, 2000, "connect"); + // On a failure the clients go, so the host can still close and the test + // fails instead of hanging. + const code = await within(close(), 5000, "close").catch((e) => { + drop(); + throw e; + }); + assert.equal(code, 0); + assert.equal(existsSync(path), false); + answered.end(); + silent.end(); + await within(gone, 2000, "the clients' close"); +}); + +test("a runner that ignores SIGTERM is killed when the host closes", { skip, timeout: 60000 }, async (t) => { + const { f, launcher, close } = await setup(t, undefined, undefined, { stopTimeoutMs: 1000 }); + const pm = await launcher.launchPm(); + const runnerLog = join(f.dataRoot, "launches", "acme", pm.run, "runner.log"); + await until(() => existsSync(runnerLog) && readFileSync(runnerLog, "utf8").includes("claimed by run")); + const [s] = readSessions(f.dataRoot); + // Stopped from outside its namespace, it can't act on SIGTERM. + process.kill(s.runnerPid, "SIGSTOP"); + const kill = () => { + if (startTimeOf(s.runnerPid) === s.runnerStartTime) process.kill(s.runnerPid, "SIGKILL"); + }; + t.after(kill); + const started = Date.now(); + const code = await within(close(), 15000, "close").catch((e) => { + kill(); + throw e; + }); + assert.equal(code, 0); + assert.ok(Date.now() - started >= 1000, "it waited for the stop timeout"); + assert.equal(startTimeOf(s.pid), null); + assert.equal(startTimeOf(s.runnerPid), null); + assert.deepEqual(readSessions(f.dataRoot), []); + const ends = log(f).filter((e) => e.event === "end" && e.run === pm.run); + assert.equal(ends.length, 1); + assert.equal(ends[0].signal, "SIGKILL"); +}); diff --git a/packages/harness/README.md b/packages/harness/README.md index bc2dcc85..79ff829a 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -26,10 +26,13 @@ bundle's `manifest.json` names the lines it relies on (`reliesOn`): | 4. gate hang | the runner's wall clock plus the `agent_end` marker | `timeout -k 2 10 \|\| exit 2`, hook timeout 20 | | 5. bash | limited only by the tool limit | limited only by the tool limit | -Claude Code also runs with `--restricted` (no user, project or local -settings; file tools confined to the working directory). That is defence -in depth: none of the S0 lines depend on it, and no test treats it as the -layer that holds. +Claude Code also runs with `--restricted`: no user, project or local +settings, file tools confined to the working directory, and no `CLAUDE.md` +file (user, parent or workspace) or auto-memory in the prompt. None of the +S0 lines depend on it, and the gate doesn't rely on it for paths. It is the +layer that keeps founder and repository memory out of the session's +prompt, though, so `claude-session.test.mjs` fails if the adapter stops +passing it, and shows the memory reaching the model without it. ## The bundle @@ -74,7 +77,10 @@ refusal comes back to the model as `refused: `. policy's built-in tools and typed tools pass, anything else is blocked, and every path argument of a file tool (and a `find`/`Glob` pattern) must resolve inside the workspace after symlinks and Pi's own path -normalisation. Pi calls it from the extension, Claude Code from +normalisation. A path that reaches a dangling symlink, at any depth, is +refused even when the link points inside: a write through it would create +the target wherever it names, and Pi's `write` makes the missing +directories first. Pi calls it from the extension, Claude Code from `claude-gate.mjs`. ## The runner @@ -148,7 +154,14 @@ These are limits, not bugs, and nothing in this package claims otherwise. - **Resolution** runs without the project layer, there is no skills source (bundles list none), and the resolved `thinking` level is recorded in the manifest but not applied to either harness. -- **`--restricted`** (Claude Code) is an extra layer, not one the S0 lines - prove. +- **`--restricted`** (Claude Code) is not one of the S0 lines. It is what + keeps `CLAUDE.md` files and auto-memory out of the prompt (above). +- **The gate checks a path when the call is made.** A link created or + changed between the check and the tool's own open (by `bash`, or by + another process of the same user) isn't seen. That is the same reach as + `bash` itself. +- **The system prompt is in argv** for both adapters, so the same UID can + read it in `/proc//cmdline`; the PID namespace hides it from other + sessions. It holds no secret. - **Pi adapter paths** with spaces break its unquoted `-e` and skill splitting; the launcher's paths don't contain spaces. diff --git a/packages/harness/src/gate.mjs b/packages/harness/src/gate.mjs index 322a84ce..47cc029e 100644 --- a/packages/harness/src/gate.mjs +++ b/packages/harness/src/gate.mjs @@ -8,7 +8,7 @@ // and anything bash can reach, the session can reach. See the README's // limits. -import { existsSync, realpathSync } from "node:fs"; +import { lstatSync, realpathSync } from "node:fs"; import { homedir } from "node:os"; import { basename, dirname, isAbsolute, join, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; @@ -41,17 +41,25 @@ function normalise(p) { return s; } -// Realpath of the nearest existing ancestor, with the missing tail kept. +// Realpath of the nearest ancestor that exists as a name, with the missing +// tail kept. lstat, not exists: a dangling symlink exists as a name, and a +// write through it would create its target wherever that is. So a path that +// reaches a dangling symlink, at any depth, can't be checked and is refused. function real(p) { let head = p; const tail = []; - while (!existsSync(head)) { + while (!lstatSync(head, { throwIfNoEntry: false })) { const up = dirname(head); if (up === head) break; tail.unshift(basename(head)); head = up; } - return join(realpathSync(head), ...tail); + try { + return join(realpathSync(head), ...tail); + } catch (error) { + if (error.code === "ENOENT") throw Object.assign(new Error("dangling symlink"), { code: "dangling-symlink" }); + throw error; + } } export function insideWorkspace(workspace, p) { @@ -87,6 +95,7 @@ export function decide(policy, tool, input) { try { if (!insideWorkspace(policy.workspace, value)) return no(`${tool} path is outside the workspace: ${value}`); } catch (error) { + if (error.code === "dangling-symlink") return no(`${tool} path goes through a dangling symlink: ${value}`); return no(`${tool} path can't be checked: ${error.code ?? error.message}`); } return { allow: true }; diff --git a/packages/harness/src/runner.mjs b/packages/harness/src/runner.mjs index e2e02d4d..ccf6ca43 100644 --- a/packages/harness/src/runner.mjs +++ b/packages/harness/src/runner.mjs @@ -256,16 +256,16 @@ export async function main(runDir, { stdin = process.stdin, env = process.env, l process.on("SIGTERM", stop); process.on("SIGINT", stop); - let session, cap; + let session, cap, policy; try { session = { ...DEFAULTS, ...JSON.parse(readFileSync(join(runDir, "session.json"), "utf8")), runDir }; cap = JSON.parse(await readLine(stdin)).cap; if (typeof cap !== "string" || !/^[0-9a-f]{64}$/.test(cap)) throw new Error("no capability on stdin"); + policy = JSON.parse(readFileSync(session.bundle.policy, "utf8")); } catch (e) { log(`runner: ${e.message}`); return EXIT.usage; } - const policy = JSON.parse(readFileSync(session.bundle.policy, "utf8")); if (stopping) { log("runner: stopped before claim"); return EXIT.stopped; diff --git a/packages/harness/tests/claude-session.test.mjs b/packages/harness/tests/claude-session.test.mjs index 823a31ee..e45c5d69 100644 --- a/packages/harness/tests/claude-session.test.mjs +++ b/packages/harness/tests/claude-session.test.mjs @@ -1,13 +1,14 @@ // The host's claude CLI through adapters/claude with the bundle's hook and // MCP server, against the scripted Messages API. Scratch CLAUDE_CONFIG_DIR // and HOME, a dummy key, nonessential traffic off: nothing reaches a real -// model or the user's Claude configuration. Skipped when claude isn't on PATH. +// model or the user's Claude configuration. Skipped when claude isn't on PATH, +// except the argv check, which uses a stand-in claude. import { test } from "node:test"; import assert from "node:assert/strict"; import { spawn, spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { chmodSync, existsSync, mkdirSync, readFileSync, realpathSync, writeFileSync } from "node:fs"; +import { basename, dirname, join } from "node:path"; import { buildBundle } from "../src/bundle.mjs"; import { adapterEnv } from "../src/runner.mjs"; import { fakeToolSocket, mockAnthropic, REPO, resolvedFor, scratch } from "./helpers.mjs"; @@ -57,9 +58,9 @@ async function session(t, script, tools = ["read", "bash"]) { return { dir, workspace, api, sock, s, env, manifest }; } -function turn(env, request, cwd) { +function turn(env, request, cwd, adapter = ADAPTER) { return new Promise((resolve) => { - const child = spawn("/bin/sh", [ADAPTER], { cwd, env: { ...env, MOSAIC_REQUEST: request }, stdio: ["ignore", "pipe", "pipe"], detached: true }); + const child = spawn("/bin/sh", [adapter], { cwd, env: { ...env, MOSAIC_REQUEST: request }, stdio: ["ignore", "pipe", "pipe"], detached: true }); let stdout = ""; let stderr = ""; child.stdout.on("data", (b) => (stdout += b)); @@ -136,6 +137,67 @@ test("claude: a second turn resumes the first turn's session", { skip }, async ( assert.equal(readFileSync(join(s.sessionDir, "claude-session-id"), "utf8"), id); }); +// --restricted is what keeps CLAUDE.md files and auto-memory out of the +// session's prompt (README "Limits"). This one runs without claude: a +// stand-in records the adapter's argv. +test("claude adapter: --restricted is always passed", (t) => { + const dir = scratch(t); + mkdirSync(join(dir, "bin")); + writeFileSync(join(dir, "bin", "claude"), '#!/bin/sh\nprintf "%s\\n" "$@" > "$ARGV_OUT"\necho ok\n'); + chmodSync(join(dir, "bin", "claude"), 0o755); + for (const f of ["prompt.md", "settings.json", "mcp.json"]) writeFileSync(join(dir, f), "{}"); + const r = spawnSync("/bin/sh", [ADAPTER], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + env: { + PATH: `${join(dir, "bin")}:/usr/bin:/bin`, + ARGV_OUT: join(dir, "argv"), + MOSAIC_SYSTEM_PROMPT_FILE: join(dir, "prompt.md"), + MOSAIC_REQUEST: "x", + MOSAIC_WORKSPACE: join(dir, "ws"), + MOSAIC_SESSION_DIR: join(dir, "session"), + MOSAIC_MODEL: "claude-sonnet-5-5", + MOSAIC_CLAUDE_SETTINGS: join(dir, "settings.json"), + MOSAIC_CLAUDE_MCP_CONFIG: join(dir, "mcp.json"), + }, + }); + assert.equal(r.status, 0, r.stderr); + const argv = readFileSync(join(dir, "argv"), "utf8").split("\n"); + assert.ok(argv.includes("--restricted"), argv.join(" ")); + assert.ok(!argv.includes("--bare"), argv.join(" ")); +}); + +test("claude: CLAUDE.md files and auto-memory don't reach the model; without --restricted they do", { skip }, async (t) => { + const { dir, workspace, env } = await session(t, []); + const slug = realpathSync(workspace).replace(/[/.]/g, "-"); + const plant = { + "MARKER-USER": [join(env.HOME, ".claude", "CLAUDE.md"), join(env.CLAUDE_CONFIG_DIR, "CLAUDE.md")], + "MARKER-PARENT": [join(dir, "CLAUDE.md")], + "MARKER-WS": [join(workspace, "CLAUDE.md")], + "MARKER-MEMORY": [join(env.HOME, ".claude", "projects", slug, "memory", "MEMORY.md"), join(env.CLAUDE_CONFIG_DIR, "projects", slug, "memory", "MEMORY.md")], + }; + for (const [marker, files] of Object.entries(plant)) { + for (const f of files) { + mkdirSync(dirname(f), { recursive: true }); + writeFileSync(f, `${marker}\n`); + } + } + const seen = async (adapter) => { + const api = await mockAnthropic(t, {}); + const r = await turn({ ...env, ANTHROPIC_BASE_URL: api.url, MOSAIC_SESSION_DIR: join(dir, `session-${basename(adapter)}`) }, "x", workspace, adapter); + assert.equal(r.code, 0, r.stderr); + const all = api.requests.map((x) => x.raw).join("\n"); + assert.match(all, /## This session/); + return Object.keys(plant).filter((m) => all.includes(m)); + }; + assert.deepEqual(await seen(ADAPTER), []); + // The control: the same adapter without --restricted lets all four in. + const loose = join(dir, "adapter-loose.sh"); + writeFileSync(loose, readFileSync(ADAPTER, "utf8").replace(" --restricted \\\n", "")); + assert.notEqual(readFileSync(loose, "utf8"), readFileSync(ADAPTER, "utf8")); + assert.deepEqual(await seen(loose), Object.keys(plant)); +}); + test("claude: a missing hook or MCP file refuses before claude starts", { skip }, async (t) => { const { dir, api, workspace, env } = await session(t, []); for (const k of ["MOSAIC_CLAUDE_SETTINGS", "MOSAIC_CLAUDE_MCP_CONFIG", "MOSAIC_SYSTEM_PROMPT_FILE"]) { diff --git a/packages/harness/tests/gate.test.mjs b/packages/harness/tests/gate.test.mjs index 66e00bb3..c32394c2 100644 --- a/packages/harness/tests/gate.test.mjs +++ b/packages/harness/tests/gate.test.mjs @@ -79,6 +79,29 @@ test("a symlink inside the workspace that points out is outside", (t) => { blocked(decide(policy, "write", { path: "link/new.txt" }), /outside the workspace/); }); +test("a dangling symlink is refused at any depth, in both harnesses", (t) => { + for (const [harness, tool, field] of [["pi", "write", "path"], ["claude-code", "Write", "file_path"]]) { + const { dir, workspace, policy } = setup(t, harness, ["read", "write"]); + mkdirSync(join(dir, "outside")); + // notes.md names a file that doesn't exist yet, outside; dangling names a + // directory that doesn't; inner points inside but at nothing. + symlinkSync(join(dir, "outside", "planted.txt"), join(workspace, "notes.md")); + symlinkSync(join(dir, "nowhere"), join(workspace, "dangling")); + symlinkSync(join(workspace, "later.txt"), join(workspace, "inner")); + for (const rel of ["notes.md", "dangling/x", "dangling/deep/x", "inner"]) { + blocked(decide(policy, tool, { [field]: rel }), /goes through a dangling symlink/); + blocked(decide(policy, tool, { [field]: join(workspace, rel) }), /goes through a dangling symlink/); + } + // Once the target exists, the usual realpath rule decides. + writeFileSync(join(dir, "outside", "planted.txt"), "p"); + blocked(decide(policy, tool, { [field]: "notes.md" }), /outside the workspace/); + writeFileSync(join(workspace, "later.txt"), "l"); + allowed(decide(policy, tool, { [field]: "inner" })); + // A file used as a directory can't be checked. + blocked(decide(policy, tool, { [field]: "a.txt/x" }), /can't be checked: ENOTDIR/); + } +}); + test("claude path fields per tool", (t) => { const { workspace, policy } = setup(t, "claude-code", ["read", "write", "edit", "grep", "find"]); allowed(decide(policy, "Read", { file_path: join(workspace, "a.txt") })); diff --git a/packages/harness/tests/helpers.mjs b/packages/harness/tests/helpers.mjs index 19f13a64..96c1923f 100644 --- a/packages/harness/tests/helpers.mjs +++ b/packages/harness/tests/helpers.mjs @@ -79,7 +79,7 @@ export async function mockAnthropic(t, { script = [], done = (r) => `DONE ${JSON const results = toolResults(body.messages); const step = tools.length ? script[results.length] : null; const r = step ? { kind: "tool", id: `toolu_${results.length + 1}`, name: step.name, input: step.input } : { kind: "text", text: tools.length ? done(results) : "mock" }; - requests.push({ tools, results, system: body.system, answer: r }); + requests.push({ tools, results, system: body.system, answer: r, raw }); const content = r.kind === "tool" ? { type: "tool_use", id: r.id, name: r.name, input: {} } : { type: "text", text: "" }; const delta = r.kind === "tool" ? { type: "input_json_delta", partial_json: JSON.stringify(r.input) } : { type: "text_delta", text: r.text }; const stop = r.kind === "tool" ? "tool_use" : "end_turn"; diff --git a/packages/harness/tests/pi-session.test.mjs b/packages/harness/tests/pi-session.test.mjs index 8fc8b4e4..0486857f 100644 --- a/packages/harness/tests/pi-session.test.mjs +++ b/packages/harness/tests/pi-session.test.mjs @@ -4,7 +4,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { spawn } from "node:child_process"; -import { existsSync, mkdirSync, readdirSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, readdirSync, symlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { buildBundle } from "../src/bundle.mjs"; import { adapterEnv } from "../src/runner.mjs"; @@ -96,6 +96,30 @@ test("pi: typed tools reach the socket, the gate blocks, agent_end writes the ma assert.ok(readdirSync(s.sessionDir).length > 0); }); +test("pi: a write through a dangling symlink is blocked, and nothing appears outside", async (t) => { + const { dir, workspace, s, env } = await session(t, [ + { name: "write", input: { path: "notes.md", content: "planted\n" } }, + { name: "write", input: { path: "gone/x.md", content: "planted\n" } }, + { name: "write", input: { path: "fine.md", content: "inside\n" } }, + ]); + mkdirSync(join(dir, "outside")); + symlinkSync(join(dir, "outside", "planted.txt"), join(workspace, "notes.md")); + symlinkSync(join(dir, "outside", "made"), join(workspace, "gone")); + const r = await turn(env, "Message 1 from jason, class REQUEST:\n\nwrite your notes", workspace); + assert.equal(r.code, 0, r.stderr); + const results = JSON.parse(r.stdout.trim().slice("ANSWER ".length)); + assert.equal(results.length, 3); + assert.equal(results[0][0], true); + assert.match(results[0][1], /dangling symlink: notes\.md/); + assert.equal(results[1][0], true); + assert.match(results[1][1], /dangling symlink: gone\/x\.md/); + assert.equal(results[2][0], false); + assert.ok(existsSync(join(workspace, "fine.md")), "the write inside landed"); + assert.deepEqual(readdirSync(join(dir, "outside")), []); + assert.ok(!existsSync(join(dir, "outside", "made"))); + assert.ok(existsSync(s.turnMarker)); +}); + test("pi: a missing extension refuses before any model call", async (t) => { const { dir, api, s, env } = await session(t, []); const r = await turn({ ...env, MOSAIC_EXTENSIONS: join(dir, "missing.mjs") }, "x", s.workspace); diff --git a/packages/harness/tests/runner.test.mjs b/packages/harness/tests/runner.test.mjs index 77027f36..25b4d8a9 100644 --- a/packages/harness/tests/runner.test.mjs +++ b/packages/harness/tests/runner.test.mjs @@ -110,15 +110,22 @@ async function setup(t, { session = {}, policy = {}, tools } = {}) { return { dir, runDir, store, broker, server, cap, pm, call, launches, pids: join(dir, "pids") }; } -function startRunner(runDir, cap, env = {}) { +// A runner that hasn't exited after a minute is killed, so a test that +// expected an exit fails on the code instead of hanging. +function startRunner(runDir, cap, env = {}, input = cap === null ? "" : JSON.stringify({ cap }) + "\n") { const child = spawn(process.execPath, [RUNNER, runDir], { stdio: ["pipe", "ignore", "pipe"], env: { PATH: process.env.PATH, ...env }, }); let stderr = ""; child.stderr.on("data", (b) => (stderr += b)); - child.stdin.end(cap === null ? "" : JSON.stringify({ cap }) + "\n"); - const exited = once(child, "exit").then(([code, signal]) => ({ code, signal, stderr })); + child.stdin.on("error", () => {}); + child.stdin.end(input); + const clock = setTimeout(() => child.kill("SIGKILL"), 60_000); + const exited = once(child, "exit").then(([code, signal]) => { + clearTimeout(clock); + return { code, signal, stderr }; + }); return { child, exited, stderr: () => stderr }; } @@ -308,5 +315,23 @@ test("no capability, or a malformed one, on stdin exits 2", async (t) => { assert.equal((await startRunner(ctx.runDir, null).exited).code, EXIT.usage); assert.equal((await startRunner(ctx.runDir, "not-hex").exited).code, EXIT.usage); assert.equal((await startRunner(join(ctx.dir, "nope"), ctx.cap).exited).code, EXIT.usage); + // A valid capability inside an over-long line: the 4096-byte cap refuses it. + const long = await startRunner(ctx.runDir, ctx.cap, {}, JSON.stringify({ cap: ctx.cap, pad: "x".repeat(5000) }) + "\n").exited; + assert.equal(long.code, EXIT.usage, long.stderr); + assert.match(long.stderr, /stdin too large/); + assert.equal(ctx.store.get("SELECT 1 FROM role_claims WHERE role='coder'"), undefined); +}); + +test("a missing or malformed policy exits 2 before the claim", async (t) => { + const ctx = await setup(t); + const policy = join(ctx.runDir, "bundle", "policy.json"); + writeFileSync(policy, "{"); + const bad = await startRunner(ctx.runDir, ctx.cap).exited; + assert.equal(bad.code, EXIT.usage, bad.stderr); + assert.match(bad.stderr, /^runner: /m); + rmSync(policy); + const missing = await startRunner(ctx.runDir, ctx.cap).exited; + assert.equal(missing.code, EXIT.usage, missing.stderr); + assert.match(missing.stderr, /runner: ENOENT/); assert.equal(ctx.store.get("SELECT 1 FROM role_claims WHERE role='coder'"), undefined); });