Skip to content

Commit 948bbe0

Browse files
committed
Agent mode phase 1: real sandbox and permission system
Agent mode's only protection against run_command/run_code/background commands was a regex blocklist against command text — no OS-level isolation, no resource limits, and the code's own comments already admitted it "can't catch everything a shell is capable of." This adds two independent layers on top of it: - Network policy (100% enforceable on every platform, since it just refuses to run the tool at all): a new networkToolsEnabled setting gates web_search, fetch_url, http_request, capture_page_screenshot, and the three GitHub tools in executeTool(). Separately, run_command/run_code/ start_background_command gained a per-call `network` argument (default false) so the model has to explicitly ask for network access on a command-by-command basis, visible in the approval card like any other argument. - OS-level containment (app/src/command-sandbox.ts): on Linux, wraps the command with bubblewrap (bwrap) when installed — confines filesystem writes to the workspace and denies network unless the network argument above was set. On macOS, generates a sandbox-exec profile doing the same (built into macOS, no install needed). On Windows there's no lightweight equivalent primitive (Windows Sandbox needs Pro/Enterprise and is a full VM-like container; Job Objects/restricted tokens don't confine filesystem or network) — stays on the blocklist plus the resource limits below, and the new Settings sandbox-status row says so honestly rather than implying protection that isn't there. Every call site falls back to running unwrapped when no mechanism is available, never a hard failure. Deliberately does NOT offer a plain `unshare --net` fallback on Linux: creating a network namespace that way commonly requires privileges a normal user doesn't have, which would make sandboxing *break* commands instead of just not confining them. - Resource limits (app/src/resource-monitor.ts, cross-platform via pidusage): a safety net against a runaway process — configurable memory and CPU caps, generous defaults, killing the whole process tree (app/src/process-tree.ts, reused from phase 0) on breach. Complements the existing 60s run_command timeout, which never applied to background commands at all. New Settings section (Chat & Prompts → Agent runtime) exposes all of this: the network-tools toggle, the resource limit inputs, and a read-only status row reporting exactly what's enforced on the current OS. 34 new tests across command-sandbox.test.ts, resource-monitor.test.ts, and process-tree.test.ts, plus 2 new agent-tools.test.ts cases for the workspace-scoped background-task cleanup from phase 0.
1 parent 9470dfa commit 948bbe0

13 files changed

Lines changed: 644 additions & 12 deletions

app/package-lock.json

Lines changed: 41 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
"devDependencies": {
7979
"@types/node": "^26.1.1",
8080
"@types/pdf-parse": "^1.1.5",
81+
"@types/pidusage": "^2.0.5",
8182
"concurrently": "^10.0.3",
8283
"cross-env": "^10.1.0",
8384
"electron": "^43.1.1",
@@ -91,6 +92,7 @@
9192
"ffmpeg-static": "^5.3.0",
9293
"node-llama-cpp": "^3.19.1",
9394
"pdf-parse": "^2.4.5",
95+
"pidusage": "^4.0.1",
9496
"tesseract.js": "^5.1.1"
9597
}
9698
}

app/src/agent-tools.ts

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import type { ToolDefinition } from "./providers/types";
88
import { getAccountToken } from "./accounts";
99
import { capturePageScreenshot } from "./browser-capture";
1010
import { killProcessTree } from "./process-tree";
11+
import { applySandbox } from "./command-sandbox";
12+
import { monitorProcess } from "./resource-monitor";
13+
import * as settingsStore from "./settings-store";
1114

1215
const execAsync = promisify(exec);
1316

@@ -131,40 +134,43 @@ export const AGENT_TOOLS: ToolDefinition[] = [
131134
{
132135
name: "run_command",
133136
description:
134-
"Execute a shell command in the workspace (or a subdirectory of it) and return its stdout/stderr/exit code. Use for builds, tests, git, npm, etc. Commands that could affect the system outside the workspace (deleting elsewhere, shutting down the machine, privilege escalation, etc.) are rejected.",
137+
"Execute a shell command in the workspace (or a subdirectory of it) and return its stdout/stderr/exit code. Use for builds, tests, git, npm, etc. Commands that could affect the system outside the workspace (deleting elsewhere, shutting down the machine, privilege escalation, etc.) are rejected. Runs inside an OS-level sandbox confined to the workspace where the platform supports it (Linux with bubblewrap installed, macOS always) — on Windows this containment isn't available and only the command-text checks above apply.",
135138
parameters: {
136139
type: "object",
137140
properties: {
138141
command: { type: "string", description: "The shell command to run." },
139142
cwd: { type: "string", description: 'Working directory for the command, relative to the workspace root. Defaults to "."' },
143+
network: { type: "boolean", description: "Whether this command needs network access (e.g. npm install, curl). Defaults to false — most commands don't need it." },
140144
},
141145
required: ["command"],
142146
},
143147
},
144148
{
145149
name: "run_code",
146150
description:
147-
"Run a Python or JavaScript code snippet in the workspace and return its stdout/stderr/exit code. A convenience over run_command for multi-line code (no shell-quoting to worry about) — it is not a sandbox: the code runs with the same permissions as run_command and is subject to the same safety checks.",
151+
"Run a Python or JavaScript code snippet in the workspace and return its stdout/stderr/exit code. A convenience over run_command for multi-line code (no shell-quoting to worry about) — subject to the same sandboxing (where available) and safety checks as run_command.",
148152
parameters: {
149153
type: "object",
150154
properties: {
151155
language: { type: "string", enum: ["python", "javascript"], description: "Which interpreter to run the code with." },
152156
code: { type: "string", description: "The full source code to execute." },
153157
cwd: { type: "string", description: 'Working directory, relative to the workspace root. Defaults to "."' },
158+
network: { type: "boolean", description: "Whether this code needs network access. Defaults to false." },
154159
},
155160
required: ["language", "code"],
156161
},
157162
},
158163
{
159164
name: "start_background_command",
160165
description:
161-
"Start a long-running command (dev server, build watcher, long test run) in the background and return immediately with a task id. Use get_background_output to check on it later and stop_background_command when done — unlike run_command, this doesn't block or time out. Subject to the same safety checks as run_command.",
166+
"Start a long-running command (dev server, build watcher, long test run) in the background and return immediately with a task id. Use get_background_output to check on it later and stop_background_command when done — unlike run_command, this doesn't block or time out. Subject to the same safety checks and sandboxing as run_command.",
162167
parameters: {
163168
type: "object",
164169
properties: {
165170
command: { type: "string", description: "The shell command to run." },
166171
cwd: { type: "string", description: 'Working directory, relative to the workspace root. Defaults to "."' },
167172
name: { type: "string", description: "Short human-readable label for this task (e.g. \"dev server\")." },
173+
network: { type: "boolean", description: "Whether this command needs network access (e.g. a dev server that fetches data). Defaults to false." },
168174
},
169175
required: ["command"],
170176
},
@@ -784,24 +790,45 @@ function formatCommandResult(stdout: string, stderr: string, exitCode: number |
784790
return parts.join("\n\n");
785791
}
786792

787-
export async function runCommand(workspaceRoot: string, command: string, relativeCwd = "."): Promise<string> {
793+
export async function runCommand(
794+
workspaceRoot: string,
795+
command: string,
796+
relativeCwd = ".",
797+
network = false
798+
): Promise<string> {
788799
const dangerReason = findDangerousCommandReason(command);
789800
if (dangerReason) throw new Error(dangerReason);
790801

791802
const cwd = resolveSafePath(workspaceRoot, relativeCwd);
803+
const wrappedCommand = applySandbox(command, { workspaceRoot, allowNetwork: network });
804+
const settings = settingsStore.getSettings();
805+
let stopMonitor = () => {};
792806
try {
793-
const { stdout, stderr } = await execAsync(command, {
807+
const execPromise = execAsync(wrappedCommand, {
794808
cwd,
795809
timeout: COMMAND_TIMEOUT_MS,
796810
maxBuffer: 10 * 1024 * 1024,
797811
});
812+
// execPromise.child is a documented feature of promisify(exec) — the
813+
// returned promise carries the underlying ChildProcess, which is the
814+
// only way to get its pid for resource-monitor.ts to watch.
815+
if (execPromise.child.pid) {
816+
stopMonitor = monitorProcess(
817+
execPromise.child.pid,
818+
{ maxMemoryMB: settings.sandboxMaxMemoryMB, maxCpuPercent: settings.sandboxMaxCpuPercent },
819+
() => execPromise.child.kill()
820+
);
821+
}
822+
const { stdout, stderr } = await execPromise;
798823
return formatCommandResult(stdout, stderr, 0);
799824
} catch (err) {
800825
const e = err as { stdout?: string; stderr?: string; code?: number; killed?: boolean; message: string };
801826
if (e.killed) {
802827
return `Command timed out after ${COMMAND_TIMEOUT_MS / 1000}s.\n\n${formatCommandResult(e.stdout ?? "", e.stderr ?? "", e.code ?? null)}`;
803828
}
804829
return formatCommandResult(e.stdout ?? "", e.stderr ?? e.message, e.code ?? null);
830+
} finally {
831+
stopMonitor();
805832
}
806833
}
807834

@@ -815,7 +842,8 @@ export async function runCode(
815842
workspaceRoot: string,
816843
language: "python" | "javascript",
817844
code: string,
818-
relativeCwd = "."
845+
relativeCwd = ".",
846+
network = false
819847
): Promise<string> {
820848
const dangerReason = findDangerousCommandReason(code);
821849
if (dangerReason) throw new Error(dangerReason);
@@ -825,7 +853,7 @@ export async function runCode(
825853
fs.writeFileSync(tmpFile, code);
826854
try {
827855
const interpreter = language === "python" ? "python3" : "node";
828-
return await runCommand(workspaceRoot, `${interpreter} "${tmpFile}"`, relativeCwd);
856+
return await runCommand(workspaceRoot, `${interpreter} "${tmpFile}"`, relativeCwd, network);
829857
} finally {
830858
fs.rmSync(tmpFile, { force: true });
831859
}
@@ -856,7 +884,8 @@ export function startBackgroundCommand(
856884
workspaceRoot: string,
857885
command: string,
858886
relativeCwd = ".",
859-
name?: string
887+
name?: string,
888+
network = false
860889
): { taskId: string; name: string } {
861890
const dangerReason = findDangerousCommandReason(command);
862891
if (dangerReason) throw new Error(dangerReason);
@@ -866,12 +895,13 @@ export function startBackgroundCommand(
866895
}
867896

868897
const cwd = resolveSafePath(workspaceRoot, relativeCwd);
898+
const wrappedCommand = applySandbox(command, { workspaceRoot, allowNetwork: network });
869899
// detached so the shell becomes its own process group leader — lets
870900
// killProcessTree() below signal the whole group (shell + whatever it
871901
// spawned, e.g. `npm run dev` spawning `node`) instead of just the shell
872902
// itself, which is all a plain .kill() would reach. No effect on Windows,
873903
// where killProcessTree uses `taskkill /t` instead.
874-
const child = spawn(command, {
904+
const child = spawn(wrappedCommand, {
875905
cwd,
876906
shell: true,
877907
stdio: ["ignore", "pipe", "pipe"],
@@ -900,7 +930,16 @@ export function startBackgroundCommand(
900930
task.output += `\n[failed to start: ${err.message}]`;
901931
task.exitCode = -1;
902932
});
933+
const settings = settingsStore.getSettings();
934+
const stopMonitor = child.pid
935+
? monitorProcess(
936+
child.pid,
937+
{ maxMemoryMB: settings.sandboxMaxMemoryMB, maxCpuPercent: settings.sandboxMaxCpuPercent },
938+
(reason) => append(Buffer.from(`\n[background task stopped: ${reason}]`))
939+
)
940+
: () => {};
903941
child.on("exit", (code) => {
942+
stopMonitor();
904943
task.exitCode = code ?? -1;
905944
});
906945
backgroundTasks.set(id, task);
@@ -1357,7 +1396,25 @@ export async function githubReadFile(repository: string, filePath: string, ref?:
13571396
return content.length > MAX_READ_CHARS ? `${content.slice(0, MAX_READ_CHARS)}\n\n[truncated]` : content;
13581397
}
13591398

1399+
// Tools that reach the network — gated by settings.networkToolsEnabled as a
1400+
// baseline that's 100% enforceable on every platform (refusing to run the
1401+
// tool at all, rather than trying to block network access after the fact,
1402+
// which is what command-sandbox.ts's per-call `network` argument does for
1403+
// run_command/run_code/start_background_command instead).
1404+
const NETWORK_TOOLS = new Set([
1405+
"web_search",
1406+
"fetch_url",
1407+
"http_request",
1408+
"capture_page_screenshot",
1409+
"github_list_repositories",
1410+
"github_repository_tree",
1411+
"github_read_file",
1412+
]);
1413+
13601414
export async function executeTool(workspaceRoot: string, name: string, args: Record<string, unknown>): Promise<unknown> {
1415+
if (NETWORK_TOOLS.has(name) && settingsStore.getSettings().networkToolsEnabled === false) {
1416+
throw new Error(`Network access for agent tools is turned off in Settings — "${name}" can't run.`);
1417+
}
13611418
switch (name) {
13621419
case "read_file":
13631420
return readFile(
@@ -1391,17 +1448,18 @@ export async function executeTool(workspaceRoot: string, name: string, args: Rec
13911448
case "search_files":
13921449
return searchFiles(workspaceRoot, String(args.query ?? ""), args.path ? String(args.path) : ".");
13931450
case "run_command":
1394-
return runCommand(workspaceRoot, String(args.command ?? ""), args.cwd ? String(args.cwd) : ".");
1451+
return runCommand(workspaceRoot, String(args.command ?? ""), args.cwd ? String(args.cwd) : ".", args.network === true);
13951452
case "run_code": {
13961453
const language = args.language === "python" ? "python" : "javascript";
1397-
return runCode(workspaceRoot, language, String(args.code ?? ""), args.cwd ? String(args.cwd) : ".");
1454+
return runCode(workspaceRoot, language, String(args.code ?? ""), args.cwd ? String(args.cwd) : ".", args.network === true);
13981455
}
13991456
case "start_background_command":
14001457
return startBackgroundCommand(
14011458
workspaceRoot,
14021459
String(args.command ?? ""),
14031460
args.cwd ? String(args.cwd) : ".",
1404-
args.name ? String(args.name) : undefined
1461+
args.name ? String(args.name) : undefined,
1462+
args.network === true
14051463
);
14061464
case "get_background_output":
14071465
return getBackgroundOutput(String(args.task_id ?? ""));

app/src/command-sandbox.test.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
import { describe, it, expect } from "vitest";
2+
import { detectSandboxCapabilities, wrapCommand, applySandbox } from "./command-sandbox";
3+
4+
const has = (available: string[]) => (cmd: string) => available.includes(cmd);
5+
6+
describe("detectSandboxCapabilities", () => {
7+
it("prefers bubblewrap on Linux when available", () => {
8+
expect(detectSandboxCapabilities("linux", has(["bwrap"]))).toEqual({
9+
filesystemConfinement: true,
10+
networkDenial: true,
11+
mechanism: "bubblewrap",
12+
});
13+
});
14+
15+
it("reports no containment on Linux without bubblewrap", () => {
16+
expect(detectSandboxCapabilities("linux", has([]))).toEqual({
17+
filesystemConfinement: false,
18+
networkDenial: false,
19+
mechanism: "none",
20+
});
21+
});
22+
23+
it("uses sandbox-exec on macOS when available", () => {
24+
expect(detectSandboxCapabilities("darwin", has(["sandbox-exec"]))).toEqual({
25+
filesystemConfinement: true,
26+
networkDenial: true,
27+
mechanism: "sandbox-exec",
28+
});
29+
});
30+
31+
it("reports no containment on macOS without sandbox-exec", () => {
32+
expect(detectSandboxCapabilities("darwin", has([]))).toEqual({
33+
filesystemConfinement: false,
34+
networkDenial: false,
35+
mechanism: "none",
36+
});
37+
});
38+
39+
it("always reports no containment on Windows, regardless of PATH", () => {
40+
expect(detectSandboxCapabilities("win32", has(["bwrap", "sandbox-exec"]))).toEqual({
41+
filesystemConfinement: false,
42+
networkDenial: false,
43+
mechanism: "none",
44+
});
45+
});
46+
});
47+
48+
describe("wrapCommand", () => {
49+
it("returns null when no sandbox mechanism is available", () => {
50+
expect(wrapCommand("echo hi", { workspaceRoot: "/ws", allowNetwork: false }, "win32", has([]))).toBeNull();
51+
expect(wrapCommand("echo hi", { workspaceRoot: "/ws", allowNetwork: false }, "linux", has([]))).toBeNull();
52+
});
53+
54+
it("builds a bubblewrap invocation confining writes to the workspace, network denied by default", () => {
55+
const wrapped = wrapCommand("npm test", { workspaceRoot: "/home/user/project", allowNetwork: false }, "linux", has(["bwrap"]));
56+
expect(wrapped?.command).toBe("bwrap");
57+
expect(wrapped?.args).toContain("--unshare-all");
58+
expect(wrapped?.args).not.toContain("--share-net");
59+
expect(wrapped?.args).toEqual(expect.arrayContaining(["--bind", "/home/user/project", "/home/user/project"]));
60+
expect(wrapped?.args.slice(-3)).toEqual(["sh", "-c", "npm test"]);
61+
});
62+
63+
it("adds --share-net to the bubblewrap invocation when network is explicitly allowed", () => {
64+
const wrapped = wrapCommand("npm install", { workspaceRoot: "/home/user/project", allowNetwork: true }, "linux", has(["bwrap"]));
65+
expect(wrapped?.args).toContain("--share-net");
66+
});
67+
68+
it("builds a sandbox-exec invocation with a profile scoped to the workspace", () => {
69+
const wrapped = wrapCommand("npm test", { workspaceRoot: "/Users/me/project", allowNetwork: false }, "darwin", has(["sandbox-exec"]));
70+
expect(wrapped?.command).toBe("sandbox-exec");
71+
expect(wrapped?.args[0]).toBe("-p");
72+
expect(wrapped?.args[1]).toContain('(allow file-write* (subpath "/Users/me/project"))');
73+
expect(wrapped?.args[1]).toContain("(deny network*)");
74+
expect(wrapped?.args.slice(-3)).toEqual(["sh", "-c", "npm test"]);
75+
});
76+
77+
it("allows network in the sandbox-exec profile when requested", () => {
78+
const wrapped = wrapCommand("curl example.com", { workspaceRoot: "/Users/me/project", allowNetwork: true }, "darwin", has(["sandbox-exec"]));
79+
expect(wrapped?.args[1]).toContain("(allow network*)");
80+
expect(wrapped?.args[1]).not.toContain("(deny network*)");
81+
});
82+
});
83+
84+
describe("applySandbox", () => {
85+
it("returns the command unchanged when no sandbox mechanism is available", () => {
86+
expect(applySandbox("echo hi", { workspaceRoot: "/ws", allowNetwork: false }, "win32", has(["bwrap"]))).toBe("echo hi");
87+
});
88+
89+
it("folds a bubblewrap-wrapped command into a single shell string ending in the original command", () => {
90+
const result = applySandbox("npm test", { workspaceRoot: "/home/user/project", allowNetwork: false }, "linux", has(["bwrap"]));
91+
expect(result.startsWith("'bwrap' ")).toBe(true);
92+
expect(result).toContain("'/home/user/project'");
93+
expect(result.endsWith("'sh' '-c' 'npm test'")).toBe(true);
94+
});
95+
96+
it("shell-quotes a workspace path containing a single quote so it can't break out of the wrapper", () => {
97+
const result = applySandbox("echo hi", { workspaceRoot: "/home/user/it's-a-project", allowNetwork: false }, "linux", has(["bwrap"]));
98+
// A raw unescaped single quote here would terminate the shell string
99+
// early and let the rest of the path be interpreted as commands.
100+
expect(result).toContain("'/home/user/it'\\''s-a-project'");
101+
});
102+
});

0 commit comments

Comments
 (0)