.claude/tasks/0023-bench-tool-isolation-guard.md

@@ -10,8 +10,22 @@ The guard is a callback that inspects every proposed shell command and permits o
 
 ## Acceptance criteria
 
-- [ ] Each arm's allow-listed binary passes the guard; a foreign binary is denied.
+- [x] Each arm's allow-listed binary passes the guard; a foreign binary is denied.
-- [ ] An absolute-path invocation of a foreign binary is denied.
+- [x] An absolute-path invocation of a foreign binary is denied.
-- [ ] An interpreter-based fetch attempt (e.g. driving an HTTP request through a language runtime) is denied.
+- [x] An interpreter-based fetch attempt (e.g. driving an HTTP request through a language runtime) is denied.
-- [ ] A curated per-arm PATH is produced exposing only that arm's allowed binary.
+- [x] A curated per-arm PATH is produced exposing only that arm's allowed binary.
-- [ ] Unit tests cover the allowed-binary, foreign-binary, absolute-path, and interpreter-fetch cases per arm.
+- [x] Unit tests cover the allowed-binary, foreign-binary, absolute-path, and interpreter-fetch cases per arm.
+
+## Implementation Notes
+
+The guard lives in `bench/guard.ts` and exposes a small interface over a deliberately deep implementation:
+
+- `guardCommand(arm, command)` — the authoritative guard, returning `{ allowed: true }` or `{ allowed: false, reason }`.
+- `provisionArmBin(arm, binDir, locate?)` — populates a per-arm bin directory with a single symlink to the arm's binary (empty for `gitea-mcp`); `locate` is an injectable resolver so tests stay host-independent.
+- `ARM_BINARY` and `HARMLESS_BINARIES` — the per-arm allow-listed binary (`null` for the shell-disabled `gitea-mcp` arm) and the curated set of harmless read/text/flow utilities.
+
+Depth added beyond the literal criteria, invited by ADR 0016 ("isolation strength rests on the completeness of the guard's deny rules"): the guard checks *every* binary a command would reach, not just the leading token, via a hand-rolled shell command parser (`extractCommands`) that handles pipelines, `;`/`&&`/`||` sequences, subshells, `$(...)` and backtick substitutions, process substitutions, redirections (including `2>&1` and `&>` forms), and leading `NAME=value` assignments. This closes pipe-hiding and substitution-hiding evasions in addition to the named absolute-path and interpreter-fetch cases. Path-qualified invocations are refused even for the arm's own binary, since the curated PATH is meant to resolve it by name and a path-qualified form is a symlink/copy evasion vector.
+
+The `gitea-mcp` arm runs with the shell disabled entirely (no allow-listed binary), so `guardCommand` denies every shell command for it with a shell-disabled reason, and `provisionArmBin` exposes nothing.
+
+Tests are colocated in `bench/guard.test.ts` (31 tests) and run via `npm run test:bench`, kept out of the `src/` coverage tier.

bench/README.md

@@ -33,8 +33,9 @@ The raw component breakdown is retained on every sample so the data can be re-we
 
 - `result.ts` — the immutable result-record shape and its tags (arm, task, tier, trial, timestamp).
 - `store.ts` — the append-only, per-cell sample store that accumulates result records.
+- `guard.ts` — the authoritative tool-isolation guard plus the curated per-arm bin directory that backs it.
 
-Later slices add the tool-isolation guard, the seed provisioning, the arm scaffolding, the single-cell runner, the task suite, the run-loop CLI, and the aggregator.
+Later slices add the seed provisioning, the arm scaffolding, the single-cell runner, the task suite, the run-loop CLI, and the aggregator.
 
 ## Tests
 

bench/guard.test.ts

@@ -0,0 +1,222 @@
+import { mkdtempSync, readdirSync, readlinkSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import type { Arm } from "./result.js";
+import { guardCommand, provisionArmBin } from "./guard.js";
+
+/**
+ * Behavior: each shell-driving arm's own allow-listed binary passes the guard.
+ *
+ * The (arm, command) pairs below are independent literals — the benchmark spec
+ * and ADR 0016 fix which binary each arm drives — rather than values derived
+ * from the module under test, so the assertion stays a genuine check.
+ */
+const allowedForOwnBinary: ReadonlyArray<{ arm: Arm; command: string }> = [
+  { arm: "gitea-axi", command: "gitea-axi issue list --state open" },
+  { arm: "tea", command: "tea issues list --output json" },
+  { arm: "raw-api", command: "curl -s https://host/api/v1/repos/o/r/issues" },
+];
+
+/**
+ * Behavior: a foreign binary — one belonging to a different arm — is denied.
+ *
+ * Only one binary is allow-listed per arm (ADR 0016), so driving another arm's
+ * tool must be refused. Each pair is an independent literal: a shell-driving
+ * arm paired with a command whose executable is a foreign binary.
+ */
+const foreignBinaryForArm: ReadonlyArray<{ arm: Arm; command: string; foreign: string }> = [
+  { arm: "gitea-axi", command: "curl -s https://host/api/v1/repos", foreign: "curl" },
+  { arm: "tea", command: "gitea-axi issue list", foreign: "gitea-axi" },
+  { arm: "raw-api", command: "tea issues list --output json", foreign: "tea" },
+];
+
+/**
+ * Behavior: an absolute-path invocation of a foreign binary is denied.
+ *
+ * Naming a foreign binary by absolute path sidesteps the curated PATH, so the
+ * guard must still refuse it (ADR 0016). Each pair is an independent literal:
+ * a shell-driving arm paired with an absolute-path invocation of another arm's
+ * binary.
+ */
+const foreignAbsolutePathForArm: ReadonlyArray<{ arm: Arm; command: string }> = [
+  { arm: "gitea-axi", command: "/usr/bin/curl -s https://host/api/v1/repos" },
+  { arm: "tea", command: "/opt/bin/gitea-axi issue list" },
+  { arm: "raw-api", command: "/usr/local/bin/tea issues list" },
+];
+
+/**
+ * Behavior: an interpreter-based fetch attempt is denied.
+ *
+ * Reaching the API over HTTP through a language runtime's HTTP client is a
+ * foreign path for every arm — including raw-api, whose only allowed binary is
+ * curl, not an interpreter (ADR 0016). Each pair is an independent literal: a
+ * shell-driving arm paired with an interpreter invocation that fetches over HTTP.
+ */
+const interpreterFetchForArm: ReadonlyArray<{ arm: Arm; command: string }> = [
+  {
+    arm: "gitea-axi",
+    command: `python3 -c "import urllib.request as u; u.urlopen('https://host/api/v1')"`,
+  },
+  {
+    arm: "tea",
+    command: `node -e "fetch('https://host/api/v1').then(r => r.text())"`,
+  },
+  {
+    arm: "raw-api",
+    command: `ruby -e "require 'net/http'; Net::HTTP.get(URI('https://host/api/v1'))"`,
+  },
+];
+
+describe("guardCommand", () => {
+  it.each(allowedForOwnBinary)(
+    "permits the $arm arm to run its own allow-listed binary",
+    ({ arm, command }) => {
+      expect(guardCommand(arm, command)).toEqual({ allowed: true });
+    },
+  );
+
+  it.each(foreignBinaryForArm)(
+    "denies the $arm arm a command driving the foreign binary $foreign",
+    ({ arm, command, foreign }) => {
+      const decision = guardCommand(arm, command);
+      expect(decision.allowed).toBe(false);
+      // A genuine foreign-binary denial names the offending binary in its reason,
+      // distinguishing it from denial for some unrelated cause.
+      if (decision.allowed === false) {
+        expect(decision.reason).toContain(foreign);
+      }
+    },
+  );
+
+  it("denies a foreign binary reached downstream of a pipe, not only the leading command", () => {
+    // The tea arm's own binary leads the line and is allow-listed, but a foreign
+    // interpreter (python3) is reached after the pipe. The guard must inspect
+    // every binary the command reaches, not just the first token.
+    const decision = guardCommand("tea", 'tea issues list | python3 -c "import urllib.request"');
+
+    expect(decision.allowed).toBe(false);
+    if (decision.allowed === false) {
+      expect(decision.reason).toContain("python3");
+    }
+  });
+
+  it.each(foreignAbsolutePathForArm)(
+    "denies the $arm arm a foreign binary named by absolute path",
+    ({ arm, command }) => {
+      const decision = guardCommand(arm, command);
+      expect(decision.allowed).toBe(false);
+    },
+  );
+
+  it.each(interpreterFetchForArm)(
+    "denies the $arm arm an interpreter-based fetch over HTTP",
+    ({ arm, command }) => {
+      const decision = guardCommand(arm, command);
+      expect(decision.allowed).toBe(false);
+    },
+  );
+
+  // The gitea-mcp arm reaches Gitea only through its attached MCP tools; its shell
+  // is disabled entirely, so no command — not even another arm's allow-listed
+  // binary or a bare harmless utility — may run (ADR 0016). Independent literals.
+  it.each([
+    "gitea-axi issue list",
+    "tea issues list",
+    "curl https://host",
+    "ls",
+  ])("denies the gitea-mcp arm every shell command, including %j", (command) => {
+    const decision = guardCommand("gitea-mcp", command);
+    expect(decision.allowed).toBe(false);
+    if (decision.allowed === false) {
+      // The denial is about the shell being off for this arm, not an ordinary
+      // foreign-binary rejection.
+      expect(decision.reason).toMatch(/shell/i);
+    }
+  });
+
+  it("denies a foreign binary hidden inside a command substitution", () => {
+    // The leading curl is allow-listed for raw-api, but python3 hides inside the
+    // $(...) substitution. The guard must look inside substitutions, not just at
+    // the top-level command.
+    const decision = guardCommand(
+      "raw-api",
+      `curl -s $(python3 -c "print('https://host')")/api/v1/repos`,
+    );
+
+    expect(decision.allowed).toBe(false);
+  });
+
+  // The arm's own binary alongside curated harmless utilities (jq, head — which
+  // cannot reach the network or execute code) is permitted, and shell plumbing
+  // like a 2>&1 redirection must not be mistaken for a foreign command (ADR 0016).
+  it.each([
+    { arm: "raw-api" as Arm, command: "curl -s https://host/api/v1/repos 2>&1 | head -n 5" },
+    { arm: "tea" as Arm, command: "tea issues list --output json | jq '.[].number'" },
+  ])(
+    "permits the $arm arm's binary piped through a curated read-only utility",
+    ({ arm, command }) => {
+      expect(guardCommand(arm, command)).toEqual({ allowed: true });
+    },
+  );
+
+  it("permits a leading NAME=value assignment before the arm's binary", () => {
+    // The runner passes the API token via a leading environment assignment; the
+    // command is the binary that follows, not the assignment itself.
+    expect(guardCommand("gitea-axi", "TOKEN=secret gitea-axi issue list")).toEqual({
+      allowed: true,
+    });
+  });
+
+  it("denies a path-qualified invocation of the arm's own binary", () => {
+    // The curated PATH resolves the arm's binary by name; a path-qualified form
+    // sidesteps that and could resolve to something else via a symlink or copy,
+    // so it is refused even for the arm's own allow-listed binary.
+    const decision = guardCommand("tea", "/usr/bin/tea issues list");
+
+    expect(decision.allowed).toBe(false);
+  });
+});
+
+describe("provisionArmBin", () => {
+  let binDir: string;
+
+  beforeEach(() => {
+    binDir = mkdtempSync(join(tmpdir(), "gitea-axi-armbin-"));
+  });
+
+  afterEach(() => {
+    rmSync(binDir, { recursive: true, force: true });
+  });
+
+  // Fake resolver so the test never depends on binaries present on the host.
+  const locate = (binary: string) => `/fake/prefix/${binary}`;
+
+  // Independent literals: each shell arm's one allow-listed binary (ADR 0016),
+  // NOT read back from ARM_BINARY.
+  const shellArmBinary: ReadonlyArray<{ arm: Arm; binary: string }> = [
+    { arm: "gitea-axi", binary: "gitea-axi" },
+    { arm: "tea", binary: "tea" },
+    { arm: "raw-api", binary: "curl" },
+  ];
+
+  it.each(shellArmBinary)(
+    "exposes only the $arm arm's allow-listed binary $binary as a symlink",
+    ({ arm, binary }) => {
+      provisionArmBin(arm, binDir, locate);
+
+      expect(readdirSync(binDir)).toEqual([binary]);
+      expect(readlinkSync(join(binDir, binary))).toBe(`/fake/prefix/${binary}`);
+    },
+  );
+
+  it("exposes nothing for the gitea-mcp arm, whose shell is disabled", () => {
+    provisionArmBin("gitea-mcp", binDir, locate);
+
+    expect(readdirSync(binDir)).toEqual([]);
+  });
+
+  it("throws when the arm's binary cannot be located", () => {
+    expect(() => provisionArmBin("tea", binDir, () => null)).toThrow(/tea/);
+  });
+});

bench/guard.ts

@@ -0,0 +1,322 @@
+import { accessSync, constants, mkdirSync, symlinkSync } from "node:fs";
+import { join } from "node:path";
+import type { Arm } from "./result.js";
+
+/**
+ * The single binary each arm's agent is allowed to invoke through the shell.
+ * `null` marks an arm that runs with the shell disabled entirely (gitea-mcp,
+ * which reaches Gitea only through its attached MCP tools and so has no shell
+ * leakage surface at all).
+ */
+export const ARM_BINARY: Record<Arm, string | null> = {
+  "gitea-axi": "gitea-axi",
+  tea: "tea",
+  "gitea-mcp": null,
+  "raw-api": "curl",
+};
+
+/**
+ * The curated set of harmless utilities any arm may reach in addition to its own
+ * allow-listed binary. Every entry is a read/text/flow utility that cannot reach
+ * the network or execute arbitrary code. The set deliberately excludes anything
+ * that can launch another program or open a socket — language interpreters
+ * (python, node, ruby, perl, php, lua), shells (sh, bash, zsh), program-launching
+ * wrappers (env, xargs, find, timeout, nohup, nice), code-capable text tools
+ * (sed, awk), and every network tool (curl, wget, nc, ssh, git). Those are the
+ * evasion surface the guard exists to close, so none of them is "harmless".
+ */
+export const HARMLESS_BINARIES: ReadonlySet<string> = new Set([
+  "cat", "head", "tail", "wc", "cut", "tr", "sort", "uniq", "comm",
+  "grep", "egrep", "fgrep", "diff", "echo", "printf", "ls", "pwd", "cd",
+  "mkdir", "rmdir", "tee", "test", "[", "true", "false", "basename",
+  "dirname", "seq", "sleep", "date", "nl", "rev", "tac", "fold", "column",
+  "expr", "jq",
+]);
+
+/** The guard's verdict on one proposed shell command. */
+export type GuardDecision = { allowed: true } | { allowed: false; reason: string };
+
+/**
+ * The authoritative tool-isolation guard. Inspects a proposed shell command and
+ * permits it only if every binary it would reach is either the active arm's one
+ * allow-listed binary or a curated harmless utility. Foreign binaries,
+ * absolute-path evasions, and interpreter-based fetch tricks are denied.
+ */
+export function guardCommand(arm: Arm, command: string): GuardDecision {
+  const allowed = ARM_BINARY[arm];
+  if (allowed === null) {
+    return {
+      allowed: false,
+      reason: `the ${arm} arm runs with the shell disabled; only its MCP tools are available`,
+    };
+  }
+  const commands = extractCommands(command);
+  if (commands.length === 0) {
+    return { allowed: false, reason: "no command was found to run" };
+  }
+  for (const name of commands) {
+    if (name.includes("/")) {
+      const base = name.slice(name.lastIndexOf("/") + 1);
+      return {
+        allowed: false,
+        reason: `path-qualified command "${name}" is not permitted; invoke "${base}" by name so the ${arm} arm's curated PATH governs which binary resolves`,
+      };
+    }
+    if (name === allowed) {
+      continue;
+    }
+    if (HARMLESS_BINARIES.has(name)) {
+      continue;
+    }
+    return {
+      allowed: false,
+      reason: `"${name}" is not permitted for the ${arm} arm; only "${allowed}" and curated read-only utilities are allowed`,
+    };
+  }
+  return { allowed: true };
+}
+
+/** Whether `word` is a leading `NAME=value` environment assignment, not a command. */
+function isAssignment(word: string): boolean {
+  return /^[A-Za-z_][A-Za-z0-9_]*=/.test(word);
+}
+
+/**
+ * Extract every command name a shell command line would reach — across pipelines,
+ * sequences (`;`, `&&`, `||`), subshells, command substitutions (`$(...)` and
+ * backticks), and process substitutions. Leading `NAME=value` assignments and
+ * redirections (including forms like `2>&1`) are skipped so only genuine command
+ * names are returned. The guard checks every returned name, which is what stops a
+ * foreign binary from hiding downstream of a pipe or inside a substitution.
+ */
+function extractCommands(command: string): string[] {
+  const s = command;
+  const len = s.length;
+  const cur = { i: 0 };
+  const names: string[] = [];
+
+  const isSpace = (c: string | undefined): boolean => c === " " || c === "\t" || c === "\r";
+  const isWordBreak = (c: string | undefined): boolean =>
+    c === undefined ||
+    isSpace(c) ||
+    c === "\n" ||
+    c === "|" ||
+    c === "&" ||
+    c === ";" ||
+    c === "(" ||
+    c === ")" ||
+    c === "{" ||
+    c === "}" ||
+    c === "<" ||
+    c === ">" ||
+    c === "`";
+
+  function skipSpaces(): void {
+    while (cur.i < len && isSpace(s[cur.i])) cur.i++;
+  }
+
+  // Read one word starting at cur.i, honoring single/double quotes and backslash
+  // escapes, and recursing into any `$(...)` / backtick substitution embedded in
+  // a double-quoted span so its command names are captured too.
+  function readWord(): string {
+    let w = "";
+    while (cur.i < len) {
+      const c = s[cur.i];
+      if (c === "'") {
+        cur.i++;
+        while (cur.i < len && s[cur.i] !== "'") {
+          w += s[cur.i];
+          cur.i++;
+        }
+        cur.i++;
+        continue;
+      }
+      if (c === '"') {
+        cur.i++;
+        while (cur.i < len && s[cur.i] !== '"') {
+          if (s[cur.i] === "\\") {
+            w += s[cur.i + 1] ?? "";
+            cur.i += 2;
+            continue;
+          }
+          if (s[cur.i] === "$" && s[cur.i + 1] === "(") {
+            cur.i += 2;
+            parseSequence(")");
+            continue;
+          }
+          if (s[cur.i] === "`") {
+            cur.i++;
+            parseSequence("`");
+            continue;
+          }
+          w += s[cur.i];
+          cur.i++;
+        }
+        cur.i++;
+        continue;
+      }
+      if (c === "\\") {
+        w += s[cur.i + 1] ?? "";
+        cur.i += 2;
+        continue;
+      }
+      if (c === "$" && s[cur.i + 1] === "(") break;
+      if (isWordBreak(c)) break;
+      w += c;
+      cur.i++;
+    }
+    return w;
+  }
+
+  // Consume a redirection operator (cur.i is at `<` or `>`) and its target, so
+  // neither the operator nor the target file/descriptor is mistaken for a command.
+  function consumeRedirection(): void {
+    cur.i++; // the leading < or >
+    if (s[cur.i] === ">" || s[cur.i] === "<") cur.i++; // >>, <<, <>
+    if (s[cur.i] === "&") cur.i++; // >&, <& (duplicate a descriptor)
+    skipSpaces();
+    if (s[cur.i] === "-") {
+      cur.i++; // close a descriptor: >&-
+      return;
+    }
+    if (cur.i < len && s[cur.i] === "$" && s[cur.i + 1] === "(") {
+      cur.i += 2;
+      parseSequence(")");
+      return;
+    }
+    if (cur.i < len && !isWordBreak(s[cur.i])) {
+      readWord(); // discard the redirection target
+    }
+  }
+
+  // Parse a run of commands until end of input, or until `closer` (`)` for a
+  // subshell/substitution, `` ` `` for a backtick substitution) is reached.
+  function parseSequence(closer: string | null): void {
+    let expectCommand = true;
+    while (cur.i < len) {
+      skipSpaces();
+      const c = s[cur.i];
+      if (c === undefined) break;
+      if (closer !== null && c === closer) {
+        cur.i++;
+        return;
+      }
+      if (c === "\n" || c === ";") {
+        cur.i++;
+        expectCommand = true;
+        continue;
+      }
+      if (c === "&") {
+        if (s[cur.i + 1] === ">") {
+          cur.i++; // `&>` / `&>>` redirect both streams
+          consumeRedirection();
+          continue;
+        }
+        cur.i++;
+        if (s[cur.i] === "&") cur.i++; // && vs background &
+        expectCommand = true;
+        continue;
+      }
+      if (c === "|") {
+        cur.i++;
+        if (s[cur.i] === "|" || s[cur.i] === "&") cur.i++; // ||, |&
+        expectCommand = true;
+        continue;
+      }
+      if (c === "(") {
+        cur.i++;
+        parseSequence(")");
+        expectCommand = false;
+        continue;
+      }
+      if (c === "{") {
+        cur.i++;
+        expectCommand = true;
+        continue;
+      }
+      if (c === "}") {
+        cur.i++;
+        continue;
+      }
+      if (c === "`") {
+        if (closer === "`") {
+          cur.i++;
+          return;
+        }
+        cur.i++;
+        parseSequence("`");
+        expectCommand = false;
+        continue;
+      }
+      if (c === "$" && s[cur.i + 1] === "(") {
+        cur.i += 2;
+        parseSequence(")");
+        expectCommand = false;
+        continue;
+      }
+      if (c === "<" || c === ">") {
+        consumeRedirection();
+        continue;
+      }
+      const word = readWord();
+      if (/^\d+$/.test(word) && (s[cur.i] === "<" || s[cur.i] === ">")) {
+        consumeRedirection(); // a file-descriptor prefix, e.g. 2>&1
+        continue;
+      }
+      if (word.length === 0) continue;
+      if (expectCommand) {
+        if (isAssignment(word)) continue; // NAME=value prefix; the command follows
+        names.push(word);
+        expectCommand = false;
+      }
+    }
+  }
+
+  parseSequence(null);
+  return names;
+}
+
+/**
+ * Provision a curated per-arm bin directory that exposes only the arm's one
+ * allow-listed binary, as a convenience layer behind the authoritative guard.
+ *
+ * The directory is populated with a single symlink named after the arm's binary
+ * pointing at its resolved absolute path, so prepending the directory to PATH
+ * lets the arm's tool resolve by name while no foreign binary is reachable that
+ * way. The gitea-mcp arm has no shell binary, so its directory is left empty.
+ * `locate` resolves a binary name to an absolute path (defaulting to a search of
+ * the real PATH); a test injects a deterministic fake.
+ */
+export function provisionArmBin(
+  arm: Arm,
+  binDir: string,
+  locate: (binary: string) => string | null = locateOnPath,
+): void {
+  mkdirSync(binDir, { recursive: true });
+  const binary = ARM_BINARY[arm];
+  if (binary === null) {
+    return; // gitea-mcp: the shell is disabled, so nothing is exposed.
+  }
+  const target = locate(binary);
+  if (target === null) {
+    throw new Error(
+      `cannot provision the ${arm} arm: its binary "${binary}" was not found on PATH`,
+    );
+  }
+  symlinkSync(target, join(binDir, binary));
+}
+
+/** Resolve `binary` to the absolute path of the first executable of that name on PATH. */
+function locateOnPath(binary: string): string | null {
+  for (const dir of (process.env.PATH ?? "").split(":")) {
+    if (dir === "") continue;
+    const candidate = join(dir, binary);
+    try {
+      accessSync(candidate, constants.X_OK);
+      return candidate;
+    } catch {
+      // Not here (or not executable); keep looking.
+    }
+  }
+  return null;
+}