From a0f6a5f409bb5ddc9d9a210a6e387b503fa59f2c Mon Sep 17 00:00:00 2001 From: asahikiko <1225133002@qq.com> Date: Mon, 22 Jun 2026 20:51:05 +0800 Subject: [PATCH] feat(sandbox): add policy contract and harden local execution --- README.md | 4 + docs/security/pr-readiness.md | 37 ++ docs/security/sandbox-current-state-audit.md | 74 ++++ docs/security/sandbox-policy.md | 31 ++ docs/security/threat-model.md | 77 ++++ package.json | 2 + packages/sandbox/src/cube/cube-config.test.ts | 9 +- packages/sandbox/src/factory.ts | 2 +- packages/sandbox/src/index.ts | 16 + .../sandbox/src/local/local-sandbox.test.ts | 162 +++++++- packages/sandbox/src/local/local-sandbox.ts | 348 ++++++++++++++++-- packages/sandbox/src/policy.ts | 165 +++++++++ packages/sandbox/src/types.ts | 7 + 13 files changed, 897 insertions(+), 37 deletions(-) create mode 100644 docs/security/pr-readiness.md create mode 100644 docs/security/sandbox-current-state-audit.md create mode 100644 docs/security/sandbox-policy.md create mode 100644 docs/security/threat-model.md create mode 100644 packages/sandbox/src/policy.ts diff --git a/README.md b/README.md index 77f5ff0..e010e06 100644 --- a/README.md +++ b/README.md @@ -434,6 +434,10 @@ AgentSpace/ ## Documentation +- [Sandbox current-state audit](docs/security/sandbox-current-state-audit.md) +- [Sandbox threat model](docs/security/threat-model.md) +- [Sandbox policy contract](docs/security/sandbox-policy.md) +- [Security PR readiness](docs/security/pr-readiness.md) - [Remote daemon deployment test guide](deploy/REMOTE_DAEMON_TEST.md) - [Founder execution showcase](deploy/FOUNDER_EXECUTION_SHOWCASE.md) - [Remote daemon installer](deploy/install-remote-daemon.sh) diff --git a/docs/security/pr-readiness.md b/docs/security/pr-readiness.md new file mode 100644 index 0000000..93f9abd --- /dev/null +++ b/docs/security/pr-readiness.md @@ -0,0 +1,37 @@ +# PR Readiness + +## PR 1: Sandbox Policy Contract and LocalSandbox Baseline + +- Branch: `security/sandbox-policy-baseline` +- Title: `feat(sandbox): add policy contract and harden trusted local execution` +- Scope: + - Add the sandbox policy contract. + - Harden LocalSandbox path validation, environment construction, output bounds, and cleanup status. + - Add adversarial LocalSandbox tests. + - Document current state, threat model, and policy semantics. +- Platform support: + - Linux/macOS: process-group termination is attempted. + - Windows: direct-child termination is used and documented as a downgrade. +- Known limitations: + - LocalSandbox is still trusted local execution. + - Network isolation is modeled but not enforced in this PR. + - Credential broker, attachment isolation, signed URLs, and runtime tool manifests are later PRs. +- Merge recommendation: draft until the listed validation commands are run in the target CI environment. + +## Validation Log + +Commands run locally on Windows: + +```bash +npm.cmd run test:sandbox +npm.cmd run test:security +git diff --check +git status --short +``` + +- `npm.cmd run test:sandbox`: passed, 19 passed, 1 skipped. The skipped test is the symlink escape test because Windows symlink creation permissions vary by host policy. +- `npm.cmd run test:security`: passed, same sandbox suite coverage. +- `git diff --check`: passed. Git emitted line-ending warnings on Windows but no whitespace errors. +- `npm.cmd run typecheck:deps`: not completed in this checkout. The root script invokes POSIX-style `./apps/web/node_modules/.bin/tsc`, which `cmd.exe` cannot execute, and this fresh checkout does not have `node_modules` installed. + +This PR should remain draft until maintainers or CI run the Linux typecheck and symlink test path. diff --git a/docs/security/sandbox-current-state-audit.md b/docs/security/sandbox-current-state-audit.md new file mode 100644 index 0000000..e78010b --- /dev/null +++ b/docs/security/sandbox-current-state-audit.md @@ -0,0 +1,74 @@ +# Sandbox Current State Audit + +This audit covers the sandbox and daemon execution state before the full multi-PR sandbox hardening roadmap is complete. It is intentionally scoped to concrete code paths in this repository. + +## Current Execution Architecture + +- `packages/daemon/src/provider-runtime.ts` calls `connectSandbox()` for direct provider executions and then calls `sandbox.exec()` with provider CLI commands, task work directories, timeout settings, and provider-specific environment values. +- `packages/sandbox/src/factory.ts` resolves the provider and currently returns either `CubeSandbox.connect(options)` or `new LocalSandbox(options.workDir, options.runtimeId, options.policy)`. +- `packages/sandbox/src/local/local-sandbox.ts` provides trusted local execution using `node:child_process.spawn`. +- AgentRouter executions in `packages/daemon/src/provider-runtime.ts` route Claude, Codex, OpenClaw, and Hermes through `runAgentRouter()` before returning normalized provider output. + +## Current Sandbox Providers + +- `LocalSandbox` is the only provider that executes commands today. +- `CubeSandbox` in `packages/sandbox/src/cube/cube-sandbox.ts` can create, pause, snapshot, and delete Cube sandboxes, but `CubeSandbox.exec()` still throws `CUBE_EXEC_NOT_READY_MESSAGE`. It must not be described as remote isolated execution yet. + +## Current Trust Boundaries + +- `LocalSandbox` is now explicit trusted local execution with path, environment, process, and output safeguards. +- `LocalSandbox` is not a security boundary against malicious code. It still runs provider CLIs on the daemon host. +- Strong isolation is out of scope for this PR and belongs to the later isolated container provider work. + +## Directory and Attachment Boundaries + +- Sandbox file access is rooted at `workDir` and validated in `LocalSandbox.resolveExistingInsideSandbox()` and `LocalSandbox.resolveWritableInsideSandbox()`. +- Attachment storage and download authorization are not changed by this PR. Attachment isolation remains a later PR scope. +- Runtime output artifact collection is not changed by this PR. The new filesystem policy reserves artifact allow patterns for later enforcement. + +## Credential Paths + +- Provider task credentials enter daemon execution through `ProviderTaskOptions.contextEnv` and provider env builders in `packages/daemon/src/provider-runtime.ts`. +- `LocalSandbox.buildSandboxEnv()` no longer inherits the daemon host environment by default. It only keeps a minimal allowlist and explicit credential keys from the sandbox policy. +- This PR does not add a full credential broker. It establishes the policy shape and default local behavior needed for that follow-up. + +## Network Capability + +- `LocalSandbox` does not implement network isolation and does not claim to. +- The policy model records a network policy, but enforcement for `none` and `allowlist` modes is deferred to the isolated container/network PRs. + +## Process Cleanup + +- `LocalSandbox.exec()` starts child processes in a separate process group on non-Windows platforms and terminates the group on timeout or output limit. +- `stop()` and `destroy()` are idempotent. +- Windows keeps a predictable fallback that terminates the direct child process. It does not claim full process-tree cleanup. + +## Audit Capability + +- Structured audit event storage is not implemented in this PR. +- `ExecResult` now carries structured execution outcome fields such as `terminationReason`, `stdoutTruncated`, `stderrTruncated`, and `outputLimitExceeded`, which later audit events can consume. + +## Security Gaps + +- No rootless container provider exists yet. +- No daemon-wide credential broker exists yet. +- Local execution still uses the host kernel, filesystem namespace, and network stack. +- Network policy is modeled but not enforced. +- Attachment signed URL authorization and tenant isolation are unchanged. +- Runtime tool manifests and adapter security manifests are unchanged. + +## Scope of This PR + +- Add `SandboxPolicy` and related policy types. +- Harden trusted local filesystem access, environment construction, output bounds, timeout status, and cleanup behavior. +- Add adversarial LocalSandbox tests. +- Add security documentation for the current state and threat model. + +## Out of Scope + +- Production-grade isolated container execution. +- Network egress proxying. +- Credential broker issuance and revocation. +- Attachment signed URL lifecycle. +- Runtime tool marketplace UI. +- Full audit event persistence. diff --git a/docs/security/sandbox-policy.md b/docs/security/sandbox-policy.md new file mode 100644 index 0000000..bd0ef2a --- /dev/null +++ b/docs/security/sandbox-policy.md @@ -0,0 +1,31 @@ +# Sandbox Policy + +`packages/sandbox/src/policy.ts` defines the policy contract used by sandbox providers. + +## Trust Levels + +- `trusted-local`: executes on the daemon host with safeguards. This is compatible with the current LocalSandbox behavior and is not a security boundary against malicious code. +- `isolated`: reserved for providers that enforce OS/container-level isolation. + +## LocalSandbox Baseline + +`createTrustedLocalSandboxPolicy(workDir)` sets conservative defaults for PR1: + +- absolute sandbox paths are rejected; +- symlinks are rejected by default; +- reads and writes are constrained to the sandbox work directory; +- single-file and total write limits are enforced; +- host environment inheritance is disabled by default; +- `HOME`, `USERPROFILE`, `TMPDIR`, `TMP`, and `TEMP` are rewritten inside the sandbox work directory; +- stdout, stderr, and combined output are bounded; +- timeout and output-limit termination are reported in `ExecResult`. + +## Compatibility + +The public sandbox API remains additive. Existing callers may keep constructing `new LocalSandbox(workDir, runtimeId)` or using `connectSandbox(options)`. + +Callers that need explicit credentials must provide a policy with `credentials.credentialEnvKeys`. Silent inheritance of secrets from the daemon host is intentionally not part of the baseline. + +## Limitations + +The policy model includes network and audit sections, but PR1 only enforces the LocalSandbox filesystem, environment, process timeout, and output controls. Network egress control belongs to the isolated provider and network-isolation PRs. diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md new file mode 100644 index 0000000..bc2f140 --- /dev/null +++ b/docs/security/threat-model.md @@ -0,0 +1,77 @@ +# AgentSpace Sandbox Threat Model + +## Attackers + +- Malicious workspace member. +- Agent controlled by prompt injection. +- Malicious or compromised skill. +- Malicious runtime tool. +- Malicious package install script. +- Compromised provider CLI. +- Agent attempting cross-workspace data access. +- Task attempting to read host credentials. +- Task attempting cloud metadata endpoint access. +- Task attempting CPU, memory, PID, disk, or log exhaustion. +- Task attempting symlink, hardlink, path traversal, or race-condition escape. +- Task attempting to keep background processes alive after completion. +- User attempting attachment access by guessing an attachment identifier. + +## Protected Assets + +- Daemon API tokens. +- Database credentials. +- Google OAuth tokens. +- Provider API keys. +- Workspace documents. +- Attachments. +- Runtime output. +- Agent knowledge. +- Other workspace task directories. +- Host files and host network. +- Internal services. +- Cloud metadata endpoints. +- Sensitive data in logs. + +## Trust Boundaries + +- Browser and API user identity boundary. +- Workspace membership and role boundary. +- Agent/runtime/task boundary. +- Daemon host boundary. +- Sandbox provider boundary. +- Attachment storage boundary. +- Credential injection boundary. +- Provider CLI boundary. + +## Assumptions + +- Maintainers can run daemon hosts with appropriate OS and container runtime controls. +- `LocalSandbox` is for trusted local execution only. +- Strong isolation requires an isolated provider such as a rootless container provider. +- Provider CLIs may read local configuration unless explicitly isolated by policy and runtime setup. + +## Security Goals + +- Make sandbox policy explicit and reviewable. +- Fail closed for path escapes and symlink escapes. +- Avoid default host environment inheritance for LocalSandbox. +- Bound stdout and stderr memory growth. +- Report timeout and output-limit termination explicitly. +- Preserve compatibility for existing callers through additive types and constructor defaults. +- Avoid claiming stronger isolation than the implementation provides. + +## Non-Goals + +- Proving LocalSandbox is safe for malicious code. +- Implementing complete network isolation in PR1. +- Implementing complete credential brokerage in PR1. +- Implementing attachment signed URLs in PR1. +- Implementing commercial marketplace UI in PR1. + +## Residual Risks + +- LocalSandbox still executes on the daemon host. +- Windows cannot guarantee the same process-group cleanup semantics as Linux. +- Existing provider CLIs can still have their own behavior and local config reads. +- Full prevention of TOCTOU attacks requires stronger OS/container primitives. +- Network and attachment isolation require later PRs. diff --git a/package.json b/package.json index 0c9f541..9cc72d6 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,8 @@ "typecheck": "npm run typecheck:deps && npm run typecheck:web:only && npm run typecheck:cli && npm run typecheck:daemon", "lint:web": "npm --prefix apps/web run lint", "test:web": "npm --prefix apps/web run test", + "test:sandbox": "node --experimental-strip-types --test packages/sandbox/src/**/*.test.ts", + "test:security": "npm run test:sandbox", "test:e2e:web": "npm --prefix apps/web run test:e2e", "quality:web": "npm --prefix apps/web run typecheck && npm --prefix apps/web run lint && npm --prefix apps/web run test" } diff --git a/packages/sandbox/src/cube/cube-config.test.ts b/packages/sandbox/src/cube/cube-config.test.ts index 1621f79..ce0514a 100644 --- a/packages/sandbox/src/cube/cube-config.test.ts +++ b/packages/sandbox/src/cube/cube-config.test.ts @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import { resolve } from "node:path"; import test from "node:test"; import { CUBE_MOUNT_WORKDIR_ENV, @@ -21,6 +22,10 @@ function buildOptions(env: NodeJS.ProcessEnv = {}): SandboxConnectOptions { }; } +function expectedResolvedWorkDir(): string { + return resolve(buildOptions().workDir); +} + test("resolveSandboxProvider falls back to local and honors legacy provider env", () => { assert.equal(resolveSandboxProvider(buildOptions()), "local"); assert.equal( @@ -62,7 +67,7 @@ test("resolveCubeSandboxConfig reads E2B-compatible env vars and derives timeout assert.equal(config.mountWorkDir, false); assert.deepEqual(config.metadata, { "agent-space.runtime-id": "runtime-cube-test", - "agent-space.work-dir": "/tmp/agent-space-cube-test", + "agent-space.work-dir": expectedResolvedWorkDir(), }); }); @@ -94,7 +99,7 @@ test("resolveCubeSandboxConfig can mount the daemon workDir into the Cube sandbo assert.equal(config.metadata["agent-space.mount-path"], "/workspace"); assert.equal( config.metadata["host-mount"], - JSON.stringify([{ hostPath: "/tmp/agent-space-cube-test", mountPath: "/workspace", readOnly: false }]), + JSON.stringify([{ hostPath: expectedResolvedWorkDir(), mountPath: "/workspace", readOnly: false }]), ); }); diff --git a/packages/sandbox/src/factory.ts b/packages/sandbox/src/factory.ts index 163d938..ec1c582 100644 --- a/packages/sandbox/src/factory.ts +++ b/packages/sandbox/src/factory.ts @@ -11,5 +11,5 @@ export async function connectSandbox(options: SandboxConnectOptions): Promise { const workDir = await mkdtemp(join(tmpdir(), "agent-space-local-sandbox-stdin-")); - const scriptPath = join(workDir, "interactive.sh"); + const scriptPath = join(workDir, "interactive.mjs"); await writeFile( scriptPath, [ - "#!/bin/sh", - "IFS= read -r first", - "printf '%s\\n' \"first:$first\"", - "IFS= read -r second", - "printf '%s\\n' \"second:$second\"", + "import { createInterface } from 'node:readline';", + "const input = createInterface({ input: process.stdin });", + "let index = 0;", + "input.on('line', (line) => {", + " index += 1;", + " process.stdout.write(`${index === 1 ? 'first' : 'second'}:${line}\\n`);", + " if (index === 2) input.close();", + "});", "", ].join("\n"), "utf8", @@ -26,7 +31,8 @@ test("LocalSandbox.exec can keep stdin open for runtime responses", async () => const sandbox = new LocalSandbox(workDir, "runtime-local-stdin-test"); let wroteFollowup = false; const result = await sandbox.exec({ - command: scriptPath, + command: process.execPath, + args: [scriptPath], input: "hello\n", keepStdinOpen: true, onReady: (controller) => { @@ -47,3 +53,143 @@ test("LocalSandbox.exec can keep stdin open for runtime responses", async () => await rm(workDir, { recursive: true, force: true }); } }); + +test("LocalSandbox rejects path traversal and absolute paths", async () => { + const workDir = await mkdtemp(join(tmpdir(), "agent-space-local-sandbox-path-")); + try { + const sandbox = new LocalSandbox(workDir, "runtime-local-path-test"); + await assert.rejects(() => sandbox.readFile("../../etc/passwd"), /escapes sandbox root|resolves outside/); + await assert.rejects(() => sandbox.writeFile("../../escape.txt", "nope"), /escapes sandbox root/); + await assert.rejects(() => sandbox.readFile(join(workDir, "file.txt")), /Absolute sandbox paths are not allowed/); + } finally { + await rm(workDir, { recursive: true, force: true }); + } +}); + +test("LocalSandbox validates realpath boundaries without prefix confusion", async () => { + const parentDir = await mkdtemp(join(tmpdir(), "agent-space-local-sandbox-prefix-")); + const workDir = join(parentDir, "task"); + const siblingDir = join(parentDir, "task-other"); + await mkdir(workDir, { recursive: true }); + await mkdir(siblingDir, { recursive: true }); + await writeFile(join(siblingDir, "outside.txt"), "outside", "utf8"); + + try { + const sandbox = new LocalSandbox(workDir, "runtime-local-prefix-test"); + await assert.rejects(() => sandbox.readFile("../task-other/outside.txt"), /escapes sandbox root/); + } finally { + await rm(parentDir, { recursive: true, force: true }); + } +}); + +test("LocalSandbox blocks symlink escape by default", { skip: platform === "win32" ? "Windows symlink permissions vary by host policy." : false }, async () => { + const workDir = await mkdtemp(join(tmpdir(), "agent-space-local-sandbox-symlink-")); + const outsideDir = await mkdtemp(join(tmpdir(), "agent-space-local-sandbox-outside-")); + await writeFile(join(outsideDir, "secret.txt"), "must-not-read", "utf8"); + await symlink(outsideDir, join(workDir, "outside-link")); + + try { + const sandbox = new LocalSandbox(workDir, "runtime-local-symlink-test"); + await assert.rejects(() => sandbox.readFile("outside-link/secret.txt"), /symlink|resolves outside/); + await assert.rejects(() => sandbox.writeFile("outside-link/new.txt", "nope"), /symlink|resolves outside/); + } finally { + await rm(workDir, { recursive: true, force: true }); + await rm(outsideDir, { recursive: true, force: true }); + } +}); + +test("LocalSandbox does not inherit host secrets by default", async () => { + const workDir = await mkdtemp(join(tmpdir(), "agent-space-local-sandbox-env-")); + const original = process.env.SANDBOX_TEST_SECRET; + process.env.SANDBOX_TEST_SECRET = "must-not-leak"; + + try { + const sandbox = new LocalSandbox(workDir, "runtime-local-env-test"); + const result = await sandbox.exec({ + command: process.execPath, + args: ["-e", "process.stdout.write(process.env.SANDBOX_TEST_SECRET || 'missing')"], + timeoutMs: 1_000, + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, "missing"); + assert.equal(result.terminationReason, "completed"); + } finally { + if (original === undefined) { + delete process.env.SANDBOX_TEST_SECRET; + } else { + process.env.SANDBOX_TEST_SECRET = original; + } + await rm(workDir, { recursive: true, force: true }); + } +}); + +test("LocalSandbox allows explicit non-sensitive env keys and rewrites HOME/TMP", async () => { + const workDir = await mkdtemp(join(tmpdir(), "agent-space-local-sandbox-env-allow-")); + + try { + const sandbox = new LocalSandbox(workDir, "runtime-local-env-allow-test"); + const result = await sandbox.exec({ + command: process.execPath, + args: [ + "-e", + "process.stdout.write(JSON.stringify({ value: process.env.LANG, home: process.env.HOME, tmp: process.env.TMPDIR }))", + ], + env: { + LANG: "C.UTF-8", + HOME: "/host/home", + TMPDIR: "/host/tmp", + OPENAI_API_KEY: "must-not-leak", + }, + timeoutMs: 1_000, + }); + + const parsed = JSON.parse(result.stdout) as { value: string; home: string; tmp: string }; + assert.equal(parsed.value, "C.UTF-8"); + assert.equal(parsed.home, join(workDir, ".home")); + assert.equal(parsed.tmp, join(workDir, ".tmp")); + assert.doesNotMatch(result.stdout, /must-not-leak/); + } finally { + await rm(workDir, { recursive: true, force: true }); + } +}); + +test("LocalSandbox bounds stdout and reports output-limit termination", async () => { + const workDir = await mkdtemp(join(tmpdir(), "agent-space-local-sandbox-output-")); + const policy = createTrustedLocalSandboxPolicy(workDir); + policy.output.maxStdoutBytes = 16; + policy.output.maxCombinedOutputBytes = 16; + + try { + const sandbox = new LocalSandbox(workDir, "runtime-local-output-test", policy); + const result = await sandbox.exec({ + command: process.execPath, + args: ["-e", "process.stdout.write('x'.repeat(1024))"], + timeoutMs: 1_000, + }); + + assert.equal(result.stdout.length, 16); + assert.equal(result.stdoutTruncated, true); + assert.equal(result.outputLimitExceeded, true); + assert.equal(result.terminationReason, "output_limit"); + } finally { + await rm(workDir, { recursive: true, force: true }); + } +}); + +test("LocalSandbox enforces per-file and total write limits", async () => { + const workDir = await mkdtemp(join(tmpdir(), "agent-space-local-sandbox-write-limit-")); + const policy = createTrustedLocalSandboxPolicy(workDir); + policy.filesystem.maxFileBytes = 4; + policy.filesystem.maxTotalWriteBytes = 6; + + try { + const sandbox = new LocalSandbox(workDir, "runtime-local-write-limit-test", policy); + await assert.rejects(() => sandbox.writeFile("too-large.txt", "12345"), /maxFileBytes/); + await sandbox.writeFile("one.txt", "123"); + await sandbox.writeFile("two.txt", "123"); + await assert.rejects(() => sandbox.writeFile("three.txt", "1"), /maxTotalWriteBytes/); + } finally { + await rm(workDir, { recursive: true, force: true }); + } +}); diff --git a/packages/sandbox/src/local/local-sandbox.ts b/packages/sandbox/src/local/local-sandbox.ts index 99f4275..570d371 100644 --- a/packages/sandbox/src/local/local-sandbox.ts +++ b/packages/sandbox/src/local/local-sandbox.ts @@ -1,38 +1,56 @@ import { spawn } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; -import { access, cp, lstat, mkdir, readdir, readFile, rm, writeFile } from "node:fs/promises"; -import { delimiter, dirname, isAbsolute, join, relative, resolve } from "node:path"; +import { access, cp, lstat, mkdir, readdir, readFile, realpath, rm, writeFile } from "node:fs/promises"; +import { delimiter, dirname, isAbsolute, join, parse, relative, resolve, sep } from "node:path"; import { platform } from "node:process"; import type { ChildProcess } from "node:child_process"; import type { Sandbox } from "../interface.ts"; import type { ExecCommand, ExecResult, FileEntry, SandboxStatus } from "../types.ts"; +import { createTrustedLocalSandboxPolicy, isSensitiveEnvKey, type SandboxPolicy, type SandboxTerminationReason } from "../policy.ts"; -const KILL_GRACE_PERIOD_MS = 5_000; +const WRITE_TOTAL_UNAVAILABLE = -1; +/** + * LocalSandbox provides trusted local execution with path and process + * safeguards. It is not a security boundary against malicious code. + */ export class LocalSandbox implements Sandbox { readonly id: string; - readonly status: SandboxStatus = "active"; private readonly workDir: string; + private readonly policy: SandboxPolicy; private readonly activeChildren = new Set(); + private statusValue: SandboxStatus = "active"; + private stopped = false; + private destroyed = false; + private totalWriteBytes = 0; - constructor(workDir: string, runtimeId: string) { + constructor(workDir: string, runtimeId: string, policy?: SandboxPolicy) { this.workDir = resolve(workDir); + this.policy = policy ?? createTrustedLocalSandboxPolicy(this.workDir); this.id = runtimeId; } + get status(): SandboxStatus { + return this.statusValue; + } + async readFile(path: string): Promise { - return readFile(this.resolveInsideSandbox(path), "utf8"); + this.assertUsable(); + return readFile(await this.resolveExistingInsideSandbox(path, "read"), "utf8"); } async writeFile(path: string, contents: string): Promise { - const absolutePath = this.resolveInsideSandbox(path); - await mkdir(dirname(absolutePath), { recursive: true }); + this.assertUsable(); + this.assertWithinFileWriteLimit(contents); + const absolutePath = await this.resolveWritableInsideSandbox(path); await writeFile(absolutePath, contents, "utf8"); + this.totalWriteBytes += Buffer.byteLength(contents, "utf8"); } async readDir(path: string): Promise { - const absolutePath = this.resolveInsideSandbox(path); + this.assertUsable(); + const absolutePath = await this.resolveExistingInsideSandbox(path, "read"); const entries = await readdir(absolutePath, { withFileTypes: true }); return Promise.all(entries.map(async (entry) => { @@ -50,7 +68,7 @@ export class LocalSandbox implements Sandbox { async exists(path: string): Promise { try { - await access(this.resolveInsideSandbox(path)); + await access(await this.resolveExistingInsideSandbox(path, "read")); return true; } catch { return false; @@ -58,12 +76,18 @@ export class LocalSandbox implements Sandbox { } async exec(command: ExecCommand): Promise { + this.assertUsable(); const startedAt = Date.now(); const resolved = resolveSpawnCommand(command.command); const args = [...resolved.prependArgs, ...(command.args ?? [])]; + const cwd = await this.resolveCommandCwd(command.cwd); + const env = await this.buildSandboxEnv(command.env); + let terminationReason: SandboxTerminationReason = "completed"; + let outputLimitExceeded = false; const child = spawn(resolved.command, args, { - cwd: this.resolveCommandCwd(command.cwd), - env: { ...process.env, ...command.env }, + cwd, + detached: platform !== "win32", + env, stdio: ["pipe", "pipe", "pipe"], }); @@ -87,20 +111,36 @@ export class LocalSandbox implements Sandbox { let stdout = ""; let stderr = ""; let timedOut = false; + let stdoutTruncated = false; + let stderrTruncated = false; let killTimer: NodeJS.Timeout | undefined; let timeout: NodeJS.Timeout | undefined; return await new Promise((resolvePromise, rejectPromise) => { child.stdout.on("data", (chunk) => { const value = String(chunk); - stdout += value; - command.onStdout?.(value); + const append = appendBoundedOutput(stdout, value, this.policy.output.maxStdoutBytes); + stdout = append.value; + stdoutTruncated ||= append.truncated; + command.onStdout?.(append.acceptedChunk); + if (this.outputLimitExceeded(stdout, stderr, stdoutTruncated, stderrTruncated)) { + outputLimitExceeded = true; + terminationReason = "output_limit"; + this.terminateChild(child); + } }); child.stderr.on("data", (chunk) => { const value = String(chunk); - stderr += value; - command.onStderr?.(value); + const append = appendBoundedOutput(stderr, value, this.policy.output.maxStderrBytes); + stderr = append.value; + stderrTruncated ||= append.truncated; + command.onStderr?.(append.acceptedChunk); + if (this.outputLimitExceeded(stdout, stderr, stdoutTruncated, stderrTruncated)) { + outputLimitExceeded = true; + terminationReason = "output_limit"; + this.terminateChild(child); + } }); command.onReady?.(stdinController); @@ -115,10 +155,11 @@ export class LocalSandbox implements Sandbox { if (command.timeoutMs && command.timeoutMs > 0) { timeout = setTimeout(() => { timedOut = true; - child.kill("SIGTERM"); + terminationReason = "timeout"; + this.terminateChild(child); killTimer = setTimeout(() => { - child.kill("SIGKILL"); - }, KILL_GRACE_PERIOD_MS); + this.killChild(child); + }, this.policy.process.killGracePeriodMs); }, command.timeoutMs); } @@ -140,12 +181,18 @@ export class LocalSandbox implements Sandbox { signal: signal ?? undefined, durationMs: Date.now() - startedAt, timedOut, + stdoutTruncated, + stderrTruncated, + outputLimitExceeded, + terminationReason: exitCode === null && terminationReason === "completed" ? "unknown" : terminationReason, }); }); }); } async snapshot(): Promise { + this.assertUsable(); + await this.resolveExistingInsideSandbox(".", "read"); const snapshotDir = join(dirname(this.workDir), ".snapshots"); const snapshotPath = join(snapshotDir, `${this.id}-${Date.now().toString(36)}`); await mkdir(snapshotDir, { recursive: true }); @@ -154,33 +201,282 @@ export class LocalSandbox implements Sandbox { } async stop(): Promise { + if (this.stopped || this.destroyed) { + return; + } + this.stopped = true; for (const child of this.activeChildren) { - child.kill("SIGTERM"); + this.terminateChild(child); } this.activeChildren.clear(); + this.statusValue = "stopped"; } async destroy(): Promise { await this.stop(); + if (this.destroyed) { + return; + } + await this.assertDestroyTargetSafe(); await rm(this.workDir, { recursive: true, force: true }); + this.destroyed = true; + } + + private assertUsable(): void { + if (this.destroyed) { + throw new Error("Sandbox has been destroyed."); + } } - private resolveInsideSandbox(path: string): string { + private rejectUnsafePath(path: string): void { + if (!path || path.trim() === "") { + throw new Error("Sandbox path must not be empty."); + } + if (isAbsolute(path)) { + throw new Error(`Absolute sandbox paths are not allowed: ${path}`); + } + } + + private async resolveExistingInsideSandbox(path: string, accessType: "read" | "write"): Promise { + this.rejectUnsafePath(path); + const absolutePath = resolve(this.workDir, path); + assertPathInside(this.workDir, absolutePath, `Path "${path}" escapes sandbox root.`); + if (!this.policy.filesystem.allowSymlinks) { + await assertNoSymlinkSegments(this.workDir, absolutePath); + } + const realTarget = await realpath(absolutePath); + assertPathInside(this.workDir, realTarget, `Path "${path}" resolves outside sandbox root.`); + this.assertPolicyRootAllowed(realTarget, accessType); + return realTarget; + } + + private async resolveWritableInsideSandbox(path: string): Promise { + this.rejectUnsafePath(path); const absolutePath = resolve(this.workDir, path); - const relativePath = relative(this.workDir, absolutePath); - if (relativePath.startsWith("..")) { - throw new Error(`Path "${path}" escapes sandbox root.`); + assertPathInside(this.workDir, absolutePath, `Path "${path}" escapes sandbox root.`); + if (!this.policy.filesystem.allowSymlinks) { + await assertNoSymlinkSegments(this.workDir, absolutePath); + } + + try { + const stats = await lstat(absolutePath); + if (stats.isSymbolicLink()) { + throw new Error(`Path "${path}" is a symlink and cannot be written.`); + } + } catch (error) { + if (!isMissingPathError(error)) { + throw error; + } } + + const existingParent = await findExistingParent(dirname(absolutePath)); + const realParent = await realpath(existingParent); + assertPathInside(this.workDir, realParent, `Parent for "${path}" resolves outside sandbox root.`); + this.assertPolicyRootAllowed(realParent, "write"); + await mkdir(dirname(absolutePath), { recursive: true }); + const realCreatedParent = await realpath(dirname(absolutePath)); + assertPathInside(this.workDir, realCreatedParent, `Parent for "${path}" resolves outside sandbox root.`); return absolutePath; } - private resolveCommandCwd(cwd: string | undefined): string { + private assertPolicyRootAllowed(target: string, accessType: "read" | "write"): void { + const roots = accessType === "read" + ? this.policy.filesystem.readableRoots + : this.policy.filesystem.writableRoots; + const allowed = roots.some((root) => { + const rootPath = resolve(this.workDir, root); + return isPathInside(rootPath, target); + }); + if (!allowed) { + throw new Error(`Sandbox ${accessType} denied by filesystem policy.`); + } + } + + private assertWithinFileWriteLimit(contents: string): void { + const bytes = Buffer.byteLength(contents, "utf8"); + if (bytes > this.policy.filesystem.maxFileBytes) { + throw new Error(`Sandbox write exceeds maxFileBytes (${this.policy.filesystem.maxFileBytes}).`); + } + if (this.totalWriteBytes !== WRITE_TOTAL_UNAVAILABLE && this.totalWriteBytes + bytes > this.policy.filesystem.maxTotalWriteBytes) { + throw new Error(`Sandbox write exceeds maxTotalWriteBytes (${this.policy.filesystem.maxTotalWriteBytes}).`); + } + } + + private async resolveCommandCwd(cwd: string | undefined): Promise { if (!cwd || cwd === ".") { return this.workDir; } - return isAbsolute(cwd) ? cwd : this.resolveInsideSandbox(cwd); + if (isAbsolute(cwd)) { + throw new Error(`Absolute sandbox cwd is not allowed: ${cwd}`); + } + return this.resolveExistingInsideSandbox(cwd, "read"); + } + + private async buildSandboxEnv(commandEnv: NodeJS.ProcessEnv | undefined): Promise { + const sandboxHome = join(this.workDir, this.policy.environment.sandboxHomeRelativePath); + const sandboxTmp = join(this.workDir, this.policy.environment.sandboxTmpRelativePath); + await mkdir(sandboxHome, { recursive: true }); + await mkdir(sandboxTmp, { recursive: true }); + + const env: NodeJS.ProcessEnv = {}; + const source = this.policy.environment.inheritHostEnv ? process.env : {}; + for (const key of this.policy.environment.allowedEnvKeys) { + const value = source[key] ?? commandEnv?.[key]; + if (typeof value === "string" && !isSensitiveEnvKey(key, this.policy.credentials)) { + env[key] = value; + } + } + + env.HOME = sandboxHome; + env.USERPROFILE = sandboxHome; + env.TMPDIR = sandboxTmp; + env.TMP = sandboxTmp; + env.TEMP = sandboxTmp; + + for (const [key, value] of Object.entries(commandEnv ?? {})) { + if (typeof value !== "string") { + continue; + } + if (isSandboxManagedEnvKey(key)) { + continue; + } + if (this.policy.environment.allowedEnvKeys.includes(key) && !isSensitiveEnvKey(key, this.policy.credentials)) { + env[key] = value; + } + if (this.policy.credentials.credentialEnvKeys.includes(key)) { + env[key] = value; + } + } + + return env; } + + private outputLimitExceeded(stdout: string, stderr: string, stdoutTruncated: boolean, stderrTruncated: boolean): boolean { + return stdoutTruncated + || stderrTruncated + || Buffer.byteLength(stdout, "utf8") + Buffer.byteLength(stderr, "utf8") >= this.policy.output.maxCombinedOutputBytes; + } + + private terminateChild(child: ChildProcess): void { + if (child.pid && platform !== "win32") { + try { + process.kill(-child.pid, "SIGTERM"); + return; + } catch { + // Fall back to killing the direct child below. + } + } + child.kill("SIGTERM"); + } + + private killChild(child: ChildProcess): void { + if (child.pid && platform !== "win32") { + try { + process.kill(-child.pid, "SIGKILL"); + return; + } catch { + // Fall back to killing the direct child below. + } + } + child.kill("SIGKILL"); + } + + private async assertDestroyTargetSafe(): Promise { + const parsed = parse(this.workDir); + if (this.workDir === parsed.root || relative(parsed.root, this.workDir).split(sep).filter(Boolean).length < 2) { + throw new Error(`Refusing to destroy unsafe sandbox root: ${this.workDir}`); + } + const realWorkDir = await realpath(this.workDir); + if (realWorkDir === parsed.root) { + throw new Error(`Refusing to destroy filesystem root: ${this.workDir}`); + } + } +} + +function appendBoundedOutput(current: string, chunk: string, maxBytes: number): { value: string; acceptedChunk: string; truncated: boolean } { + const currentBytes = Buffer.byteLength(current, "utf8"); + const remainingBytes = Math.max(0, maxBytes - currentBytes); + if (remainingBytes <= 0) { + return { value: current, acceptedChunk: "", truncated: chunk.length > 0 }; + } + + const chunkBytes = Buffer.from(chunk, "utf8"); + if (chunkBytes.byteLength <= remainingBytes) { + return { value: current + chunk, acceptedChunk: chunk, truncated: false }; + } + + const acceptedChunk = chunkBytes.subarray(0, remainingBytes).toString("utf8"); + return { + value: current + acceptedChunk, + acceptedChunk, + truncated: true, + }; +} + +function assertPathInside(root: string, target: string, message: string): void { + if (!isPathInside(root, target)) { + throw new Error(message); + } +} + +function isPathInside(root: string, target: string): boolean { + const relativePath = relative(root, target); + return relativePath === "" || (!relativePath.startsWith("..") && !isAbsolute(relativePath)); +} + +async function assertNoSymlinkSegments(root: string, target: string): Promise { + const rootPath = resolve(root); + const targetPath = resolve(target); + const relativePath = relative(rootPath, targetPath); + if (relativePath === "") { + return; + } + let cursor = rootPath; + for (const segment of relativePath.split(/[\\/]/).filter(Boolean)) { + cursor = join(cursor, segment); + try { + const stats = await lstat(cursor); + if (stats.isSymbolicLink()) { + throw new Error(`Sandbox path contains a symlink segment: ${cursor}`); + } + } catch (error) { + if (isMissingPathError(error)) { + return; + } + throw error; + } + } +} + +async function findExistingParent(path: string): Promise { + let cursor = resolve(path); + while (true) { + try { + const stats = await lstat(cursor); + if (!stats.isDirectory()) { + throw new Error(`Sandbox parent is not a directory: ${cursor}`); + } + return cursor; + } catch (error) { + if (!isMissingPathError(error)) { + throw error; + } + const next = dirname(cursor); + if (next === cursor) { + throw error; + } + cursor = next; + } + } +} + +function isMissingPathError(error: unknown): boolean { + return error instanceof Error && "code" in error && error.code === "ENOENT"; +} + +function isSandboxManagedEnvKey(key: string): boolean { + return ["HOME", "USERPROFILE", "TMPDIR", "TMP", "TEMP"].includes(key); } function findExecutableOnPath(command: string): string | null { diff --git a/packages/sandbox/src/policy.ts b/packages/sandbox/src/policy.ts new file mode 100644 index 0000000..ba29eee --- /dev/null +++ b/packages/sandbox/src/policy.ts @@ -0,0 +1,165 @@ +export type SandboxTrustLevel = "trusted-local" | "isolated"; + +export type SandboxTerminationReason = + | "completed" + | "timeout" + | "output_limit" + | "policy_denied" + | "resource_limit" + | "cancelled" + | "unknown"; + +export interface SandboxFilesystemPolicy { + workDir: string; + allowAbsolutePaths: false; + allowSymlinks: boolean; + writableRoots: string[]; + readableRoots: string[]; + maxFileBytes: number; + maxTotalWriteBytes: number; + maxArtifactBytes: number; + allowedArtifactPatterns: string[]; +} + +export interface SandboxProcessPolicy { + timeoutMs: number; + killGracePeriodMs: number; + maxProcesses?: number; + maxMemoryBytes?: number; + maxCpuMillis?: number; + maxStdoutBytes: number; + maxStderrBytes: number; + maxCombinedOutputBytes: number; +} + +export interface SandboxNetworkPolicy { + mode: "none" | "allowlist" | "unrestricted"; + allowedHosts: string[]; + allowedPorts?: number[]; + denyPrivateNetworks: boolean; + denyMetadataEndpoints: true; + logConnections: boolean; +} + +export interface SandboxEnvironmentPolicy { + inheritHostEnv: boolean; + allowedEnvKeys: string[]; + sandboxHomeRelativePath: string; + sandboxTmpRelativePath: string; +} + +export interface SandboxCredentialPolicy { + allowImplicitHostCredentials: boolean; + credentialEnvKeys: string[]; + redactEnvKeyPatterns: string[]; +} + +export interface SandboxOutputPolicy { + maxStdoutBytes: number; + maxStderrBytes: number; + maxCombinedOutputBytes: number; +} + +export interface SandboxAuditPolicy { + emitEvents: boolean; + redactSecrets: boolean; +} + +export interface SandboxPolicy { + trustLevel: SandboxTrustLevel; + filesystem: SandboxFilesystemPolicy; + process: SandboxProcessPolicy; + network: SandboxNetworkPolicy; + environment: SandboxEnvironmentPolicy; + credentials: SandboxCredentialPolicy; + output: SandboxOutputPolicy; + audit: SandboxAuditPolicy; +} + +export const DEFAULT_LOCAL_SANDBOX_TIMEOUT_MS = 12 * 60 * 60 * 1000; +export const DEFAULT_LOCAL_SANDBOX_KILL_GRACE_MS = 5_000; +export const DEFAULT_LOCAL_SANDBOX_MAX_FILE_BYTES = 16 * 1024 * 1024; +export const DEFAULT_LOCAL_SANDBOX_MAX_TOTAL_WRITE_BYTES = 128 * 1024 * 1024; +export const DEFAULT_LOCAL_SANDBOX_MAX_STDOUT_BYTES = 8 * 1024 * 1024; +export const DEFAULT_LOCAL_SANDBOX_MAX_STDERR_BYTES = 8 * 1024 * 1024; +export const DEFAULT_LOCAL_SANDBOX_MAX_COMBINED_OUTPUT_BYTES = 12 * 1024 * 1024; + +export function createTrustedLocalSandboxPolicy(workDir: string): SandboxPolicy { + return { + trustLevel: "trusted-local", + filesystem: { + workDir, + allowAbsolutePaths: false, + allowSymlinks: false, + writableRoots: ["."], + readableRoots: ["."], + maxFileBytes: DEFAULT_LOCAL_SANDBOX_MAX_FILE_BYTES, + maxTotalWriteBytes: DEFAULT_LOCAL_SANDBOX_MAX_TOTAL_WRITE_BYTES, + maxArtifactBytes: DEFAULT_LOCAL_SANDBOX_MAX_FILE_BYTES, + allowedArtifactPatterns: ["runtime-output/**"], + }, + process: { + timeoutMs: DEFAULT_LOCAL_SANDBOX_TIMEOUT_MS, + killGracePeriodMs: DEFAULT_LOCAL_SANDBOX_KILL_GRACE_MS, + maxStdoutBytes: DEFAULT_LOCAL_SANDBOX_MAX_STDOUT_BYTES, + maxStderrBytes: DEFAULT_LOCAL_SANDBOX_MAX_STDERR_BYTES, + maxCombinedOutputBytes: DEFAULT_LOCAL_SANDBOX_MAX_COMBINED_OUTPUT_BYTES, + }, + network: { + mode: "unrestricted", + allowedHosts: [], + denyPrivateNetworks: false, + denyMetadataEndpoints: true, + logConnections: false, + }, + environment: { + inheritHostEnv: false, + allowedEnvKeys: [ + "PATH", + "HOME", + "USERPROFILE", + "TMPDIR", + "TMP", + "TEMP", + "LANG", + "LC_ALL", + "TERM", + "SystemRoot", + "WINDIR", + "COMSPEC", + "PATHEXT", + ], + sandboxHomeRelativePath: ".home", + sandboxTmpRelativePath: ".tmp", + }, + credentials: { + allowImplicitHostCredentials: false, + credentialEnvKeys: [], + redactEnvKeyPatterns: [ + "SECRET", + "TOKEN", + "PASSWORD", + "PASSWD", + "API_KEY", + "ACCESS_KEY", + "DATABASE_URL", + "PRIVATE_KEY", + "CREDENTIAL", + ], + }, + output: { + maxStdoutBytes: DEFAULT_LOCAL_SANDBOX_MAX_STDOUT_BYTES, + maxStderrBytes: DEFAULT_LOCAL_SANDBOX_MAX_STDERR_BYTES, + maxCombinedOutputBytes: DEFAULT_LOCAL_SANDBOX_MAX_COMBINED_OUTPUT_BYTES, + }, + audit: { + emitEvents: false, + redactSecrets: true, + }, + }; +} + +export function isSensitiveEnvKey(key: string, policy: SandboxCredentialPolicy): boolean { + const normalized = key.toUpperCase(); + return policy.redactEnvKeyPatterns.some((pattern) => normalized.includes(pattern.toUpperCase())); +} diff --git a/packages/sandbox/src/types.ts b/packages/sandbox/src/types.ts index 10a8374..e1e12fe 100644 --- a/packages/sandbox/src/types.ts +++ b/packages/sandbox/src/types.ts @@ -24,6 +24,8 @@ export interface ExecController { closeStdin(): void; } +import type { SandboxPolicy, SandboxTerminationReason } from "./policy.ts"; + export interface ExecResult { stdout: string; stderr: string; @@ -31,6 +33,10 @@ export interface ExecResult { signal?: NodeJS.Signals; durationMs: number; timedOut: boolean; + stdoutTruncated?: boolean; + stderrTruncated?: boolean; + outputLimitExceeded?: boolean; + terminationReason?: SandboxTerminationReason; } export interface FileEntry { @@ -48,6 +54,7 @@ export interface SandboxConnectOptions { workDir: string; provider?: SandboxProvider; env?: NodeJS.ProcessEnv; + policy?: SandboxPolicy; } export const SANDBOX_TASK_TIMEOUT_ENV = "AGENT_SPACE_TASK_TIMEOUT_MS";