diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index 5c723179..4c0ce3a0 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -6160,3 +6160,77 @@ Skipped creating separate issues for Op/EventMsg protocol (already covered by SS (`TestSandboxWorkspaceScopeNetworkPolicyLiveCurl`) curls `https://proxy.golang.org` through the actual seatbelt sandbox and asserts success under allow, failure under deny. + +# 2026-09-06 (Issue #1399 sandbox toolchain cache dirs) + +- Prior behavior: `SandboxScopeWorkspace` confined writes to the workspace + root alone (plus a handful of device nodes on darwin). Any language + toolchain invoked via `bash` under workspace scope — `go build`/`go test`, + `npm install`, `cargo build` — writes to its build cache and a scratch + directory under the process temp dir or a per-user cache dir, neither of + which is the workspace, so every such command failed with + `failed to initialize build cache ... operation not permitted` (darwin + seatbelt) or the equivalent bwrap "Operation not permitted" (Linux, + since `/tmp` was only ever read-only bound there). +- Change: `toolchainWritableDirs()` (`internal/harness/tools/toolchain_dirs.go`) + computes, fresh on every call, the set of per-user temp/cache roots a + toolchain needs: `os.TempDir()`, `os.UserCacheDir()`, `~/.cache` (created + if missing), `$GOCACHE` (if set and existing), the Go module cache + (`$GOMODCACHE`, else `$GOPATH/pkg`, else `~/go/pkg`, created if missing), + `~/.npm`, `~/.cargo/registry`, `~/.cargo/git`. Every entry is + symlink-canonicalized (`filepath.EvalSymlinks`) the same way the workspace + root already is, so it lines up with the kernel-resolved paths darwin's + seatbelt `(subpath ...)` predicate and Linux's bwrap binds actually match + against. `$HOME` itself is never opened up — only these specific narrow + subdirectories, and only the two noted above are created rather than + merely detected. +- `seatbeltProfile` (darwin) now emits an additional + `(allow file-write* (subpath ...))` line per directory for + `SandboxScopeWorkspace` only (`SandboxScopeLocal` already has a blanket + `(allow file-write*)`, so it does not need these). `buildSandboxedCommand` + (Linux) now `--bind`s each directory read-write, after the read-only root + bind and before the workspace bind; `/tmp` is deliberately no longer + explicitly `--ro-bind`ed there, since `os.TempDir()` is frequently exactly + `/tmp` (TMPDIR unset) and a read-only bind of the same path would shadow + the later read-write bind for that directory. `/var/tmp` stays read-only + unless it happens to be one of the toolchain writable dirs. +- `checkWorkspaceScopeCommand`'s string heuristic (defense-in-depth, not the + primary OS-level enforcement) now treats an absolute-path token as + in-scope if it falls under the workspace OR any `toolchainWritableDirs()` + entry, reusing `canonicalizePathAllowingMissing`/`pathWithinRoot` from + `common_paths.go` rather than re-deriving symlink-safe containment + checking. It also now expands a leading `~/` token against the home + directory before the containment check, so `~/.ssh/id_rsa` is still + correctly rejected (previously `~`-prefixed tokens were silently skipped + entirely, since `filepath.IsAbs` does not recognize `~` as absolute). +- `SandboxExecResult` gains a `WritableDirs []string` field so the bash tool + result map carries a `sandbox_writable_dirs` key (populated only for + workspace scope, where it's meaningful); the "Permissions for this run: + ..." notice injected into the model's context (issue #1397's line) now + appends "For this run, temp and per-user cache directories are writable." + when `sandbox=workspace`, so the model does not misdiagnose a legitimate + cache-dir write as a permissions problem it must route around. +- Existing test fallout, both expected consequences of legitimately widening + what workspace scope permits: `TestSandboxWorkspaceScopeBlocksWriteOutsideWorkspaceAtOSLevel` + and `TestSandboxWorkspaceScopeEnforcesFilePaths` both used to prove an + "outside the workspace" write was rejected by writing under `os.TempDir()` + or a sibling of the workspace's `t.TempDir()` parent (itself under + `os.TempDir()`) — both are now legitimately writable, so both tests were + repointed at `/var/tmp` (never a toolchain writable dir) to keep proving a + real boundary exists. `TestCheckSandboxCommandWorkspaceScope`'s + cross-platform-ambiguous `"ls /tmp"` case (would flip from rejected to + accepted on any host where `TMPDIR` is unset, since `os.TempDir()` then + equals literal `/tmp`) was replaced with `"ls /usr/local/bin/x"`, one of + the contract's explicit still-rejected examples. +- Regression/integration: `TestSandboxWorkspaceScopeToolchainCanBuildAndTest` + creates a throwaway Go module inside the workspace and runs + `go env GOCACHE && go build ./... && go test ./...` through the real + darwin seatbelt sandbox with no env var overrides — confirmed to fail with + the pre-fix code (`operation not permitted` creating the build work dir) + and pass after; `TestSandboxWorkspaceScopeAllowsMktempDir` does the same + for `mktemp -d`. Both are skipped when no OS-level sandbox mechanism is + available; the Linux bwrap bind-flag test + (`TestBuildSandboxedCommandLinuxIncludesToolchainWritableDirs`) cannot + execute on this darwin worktree and is verified by + `GOOS=linux go vet ./internal/harness/tools/` for syntax/type correctness + only — it needs a real Linux CI run to prove behavior. diff --git a/internal/harness/runner.go b/internal/harness/runner.go index 35567567..a98a97a1 100644 --- a/internal/harness/runner.go +++ b/internal/harness/runner.go @@ -7460,16 +7460,28 @@ func buildContinuationPolicyNotice(srcAllowed, currentAllowed []string, srcPerms // permissionsNoticeLines renders the permissions statement injected into the // model's context: one line reporting the current sandbox/approval/network -// axes, plus (when network is denied) a warning that dependency installs -// will fail rather than letting the model silently substitute a different -// design (issue #1397). Used both for the first-turn notice (always present) -// and the continuation notice (present only when permissions changed). +// axes, plus (when sandbox is "workspace") a note that temp and per-user +// cache directories are writable, plus (when network is denied) a warning +// that dependency installs will fail rather than letting the model silently +// substitute a different design (issue #1397). Used both for the +// first-turn notice (always present) and the continuation notice (present +// only when permissions changed). func permissionsNoticeLines(perms PermissionConfig) []string { network := perms.Network if network == "" { network = NetworkPolicyAllow } lines := []string{fmt.Sprintf("Permissions for this run: sandbox=%s, approval=%s, network=%s.", perms.Sandbox, perms.Approval, network)} + if perms.Sandbox == SandboxScopeWorkspace { + // Issue #1399: workspace scope confines writes to the workspace + // plus a handful of per-user temp/cache directories a language + // toolchain needs (go build/test, npm, cargo). Told explicitly so + // the model does not misdiagnose a legitimate cache-dir write as a + // permissions problem it must route around. "local"/"unrestricted" + // already permit unrestricted filesystem writes, so this line + // would be noise there and is omitted. + lines = append(lines, "For this run, temp and per-user cache directories are writable.") + } if network == NetworkPolicyDeny { lines = append(lines, "Outbound network is blocked for this run: dependency installs will fail; report the blocker instead of substituting a different design.") } diff --git a/internal/harness/runner_writable_dirs_notice_test.go b/internal/harness/runner_writable_dirs_notice_test.go new file mode 100644 index 00000000..dcfc5ac2 --- /dev/null +++ b/internal/harness/runner_writable_dirs_notice_test.go @@ -0,0 +1,74 @@ +package harness + +import "testing" + +// TestRunnerFirstTurnPermissionsNoticeIncludesWritableCacheDirsForWorkspace +// verifies that under SandboxScopeWorkspace the permissions notice tells the +// model that temp and per-user cache directories are writable (issue +// #1399), so it does not misdiagnose a `go build`/`npm install` cache +// failure as a permissions problem it must additionally route around. +func TestRunnerFirstTurnPermissionsNoticeIncludesWritableCacheDirsForWorkspace(t *testing.T) { + t.Parallel() + + provider := &capturingProvider{turns: []CompletionResult{{Content: "done"}}} + runner := NewRunner(provider, NewRegistry(), RunnerConfig{ + DefaultModel: "gpt-5-nano", + MaxSteps: 2, + DefaultAgentIntent: "general", + }) + + run, err := runner.StartRun(RunRequest{ + Prompt: "hello", + Permissions: &PermissionConfig{Sandbox: SandboxScopeWorkspace, Approval: ApprovalPolicyNone, Network: NetworkPolicyAllow}, + }) + if err != nil { + t.Fatalf("start run: %v", err) + } + if _, err := collectRunEvents(t, runner, run.ID); err != nil { + t.Fatalf("collect events: %v", err) + } + + if len(provider.calls) != 1 { + t.Fatalf("expected one provider call, got %d", len(provider.calls)) + } + if !anyMessageContains(provider.calls[0].Messages, "temp and per-user cache directories are writable") { + t.Fatalf("expected first-turn messages to mention writable temp/cache dirs for workspace scope, got %+v", provider.calls[0].Messages) + } +} + +// TestRunnerFirstTurnPermissionsNoticeOmitsWritableCacheDirsForUnrestricted +// is a regression test for issue #1399, guarding a different angle than the +// positive test above: "local" and "unrestricted" sandbox scope already +// permit unrestricted filesystem writes, so calling out temp/cache dirs +// specifically there would be noise (and would misleadingly suggest those +// scopes are MORE restricted than they actually are). If a future change +// stopped gating the new sentence on Sandbox == SandboxScopeWorkspace, this +// test would fail by finding the sentence present for unrestricted scope. +func TestRunnerFirstTurnPermissionsNoticeOmitsWritableCacheDirsForUnrestricted(t *testing.T) { + t.Parallel() + + provider := &capturingProvider{turns: []CompletionResult{{Content: "done"}}} + runner := NewRunner(provider, NewRegistry(), RunnerConfig{ + DefaultModel: "gpt-5-nano", + MaxSteps: 2, + DefaultAgentIntent: "general", + }) + + run, err := runner.StartRun(RunRequest{ + Prompt: "hello", + Permissions: &PermissionConfig{Sandbox: SandboxScopeUnrestricted, Approval: ApprovalPolicyNone}, + }) + if err != nil { + t.Fatalf("start run: %v", err) + } + if _, err := collectRunEvents(t, runner, run.ID); err != nil { + t.Fatalf("collect events: %v", err) + } + + if len(provider.calls) != 1 { + t.Fatalf("expected one provider call, got %d", len(provider.calls)) + } + if anyMessageContains(provider.calls[0].Messages, "temp and per-user cache directories are writable") { + t.Fatalf("expected unrestricted-scope notice to omit the writable-cache-dirs sentence, got %+v", provider.calls[0].Messages) + } +} diff --git a/internal/harness/tools/bash_manager.go b/internal/harness/tools/bash_manager.go index ad2a0c58..2ab90c39 100644 --- a/internal/harness/tools/bash_manager.go +++ b/internal/harness/tools/bash_manager.go @@ -289,6 +289,9 @@ func (m *JobManager) runForeground(ctx context.Context, command string, timeoutS if sbResult.NetworkPolicy != "" { result["sandbox_network"] = string(sbResult.NetworkPolicy) } + if len(sbResult.WritableDirs) > 0 { + result["sandbox_writable_dirs"] = sbResult.WritableDirs + } return result, nil } @@ -448,6 +451,9 @@ func (m *JobManager) runBackground(ctx context.Context, command string, timeoutS if sbResult.NetworkPolicy != "" { result["sandbox_network"] = string(sbResult.NetworkPolicy) } + if len(sbResult.WritableDirs) > 0 { + result["sandbox_writable_dirs"] = sbResult.WritableDirs + } return result, nil } diff --git a/internal/harness/tools/sandbox.go b/internal/harness/tools/sandbox.go index f3da3f21..4ed3a53f 100644 --- a/internal/harness/tools/sandbox.go +++ b/internal/harness/tools/sandbox.go @@ -53,38 +53,58 @@ func CheckSandboxCommand(scope SandboxScope, network NetworkPolicy, workspaceRoo } // checkWorkspaceScopeCommand blocks bash commands that appear to target paths -// outside the workspace. It inspects: -// - Absolute paths embedded in the command. +// outside the workspace and outside toolchainWritableDirs() (issue #1399). +// It inspects: +// - Absolute paths embedded in the command (including "~/..." tokens, +// expanded against the home directory before the containment check). // - "cd .." or "cd ../../" style path escapes. -// - /etc, /tmp, /var, /usr, /home, /root usage (paths outside workspace). +// - /etc, /tmp, /var, /usr, /home, /root usage, EXCEPT when the token +// falls under one of toolchainWritableDirs() — e.g. a GOTMPDIR or +// GOCACHE override pointed at the process temp dir or a per-user cache +// dir, which language toolchains legitimately need under workspace +// scope. func checkWorkspaceScopeCommand(workspaceRoot, command string) error { - // Resolve workspace root for comparison. + // Resolve workspace root for comparison, canonicalizing symlinks the + // same way the extra writable roots below already are (and the same + // way the OS-level sandbox builders already resolve the workspace + // root) so a token pointing at the same location through a symlink + // (e.g. macOS's /var -> /private/var) is not falsely rejected. absRoot, err := filepath.Abs(workspaceRoot) if err != nil { absRoot = workspaceRoot } absRoot = filepath.Clean(absRoot) + if resolved, err := filepath.EvalSymlinks(absRoot); err == nil { + absRoot = resolved + } + + roots := append([]string{absRoot}, toolchainWritableDirs()...) + + home, homeErr := os.UserHomeDir() - // Detect absolute paths in the command that escape the workspace. - // We look for patterns like /something where /something is NOT under absRoot. - // Simple heuristic: split on whitespace and check each token that looks like - // an absolute path. + // Detect absolute paths in the command that escape the workspace and + // every toolchain writable root. Simple heuristic: split on whitespace + // and check each token that looks like an absolute (or home-relative + // "~/...") path. tokens := strings.Fields(command) for _, tok := range tokens { // Strip leading quotes and common shell metacharacters. cleaned := strings.TrimLeft(tok, `"'`) cleaned = strings.TrimRight(cleaned, `"';`) + if homeErr == nil && (cleaned == "~" || strings.HasPrefix(cleaned, "~/")) { + cleaned = filepath.Join(home, strings.TrimPrefix(cleaned, "~")) + } if !filepath.IsAbs(cleaned) { continue } candidate := filepath.Clean(cleaned) - rel, relErr := filepath.Rel(absRoot, candidate) - if relErr != nil { - continue + if resolved, err := canonicalizePathAllowingMissing(candidate); err == nil { + candidate = resolved } - if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - return fmt.Errorf("sandbox violation: absolute path %q escapes workspace %q", cleaned, absRoot) + if pathUnderAnyRoot(candidate, roots) { + continue } + return fmt.Errorf("sandbox violation: absolute path %q escapes workspace %q", cleaned, absRoot) } // Detect "cd .." patterns that escape the workspace. @@ -96,6 +116,17 @@ func checkWorkspaceScopeCommand(workspaceRoot, command string) error { return nil } +// pathUnderAnyRoot reports whether candidate lies within any of roots, +// reusing pathWithinRoot's component-wise (not string-prefix) comparison. +func pathUnderAnyRoot(candidate string, roots []string) bool { + for _, root := range roots { + if pathWithinRoot(candidate, root) { + return true + } + } + return false +} + // checkLocalScopeCommand blocks outbound network commands. func checkLocalScopeCommand(command string) error { for _, pattern := range networkRestrictedPatterns { @@ -143,6 +174,13 @@ type SandboxExecResult struct { // output always reflects the effective policy rather than leaving the // caller to infer it. NetworkPolicy NetworkPolicy + // WritableDirs lists the extra per-user temp/cache roots (beyond the + // workspace itself) that were opened up for writes under + // SandboxScopeWorkspace (issue #1399), e.g. os.TempDir(), the Go build + // and module caches. Empty for scopes where it is not meaningful + // ("local"/"unrestricted" already permit unrestricted filesystem + // writes). + WritableDirs []string } // resolveSandboxUnavailable is called by the platform-specific diff --git a/internal/harness/tools/sandbox_darwin.go b/internal/harness/tools/sandbox_darwin.go index 8338a1a7..8f7a1bc1 100644 --- a/internal/harness/tools/sandbox_darwin.go +++ b/internal/harness/tools/sandbox_darwin.go @@ -79,7 +79,11 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo cleanup := func() { os.Remove(f.Name()) } cmd := exec.CommandContext(ctx, sandboxExecBinary, "-f", f.Name(), "/bin/bash", "-lc", command) - return cmd, cleanup, SandboxExecResult{Applied: true, Mechanism: "seatbelt", NetworkPolicy: network}, nil + var writableDirs []string + if scope == SandboxScopeWorkspace { + writableDirs = toolchainWritableDirs() + } + return cmd, cleanup, SandboxExecResult{Applied: true, Mechanism: "seatbelt", NetworkPolicy: network, WritableDirs: writableDirs}, nil default: return nil, nil, SandboxExecResult{}, fmt.Errorf("unknown sandbox scope %q", scope) } @@ -90,10 +94,14 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo // SandboxScopeWorkspace: reads are allowed broadly (needed for coreutils, // dynamic linking, terminfo, locale data, etc. without hand-maintaining an // allowlist of every system path a shell invocation might touch); writes are -// confined to workspaceRoot plus the handful of device nodes a non-interactive -// bash needs (/dev/null, /dev/tty, /dev/zero, /dev/dtracehelper). +// confined to workspaceRoot, toolchainWritableDirs() (issue #1399 — the +// per-user temp dir and cache directories a language toolchain writes to +// even for a workspace-scoped build/test), plus the handful of device nodes +// a non-interactive bash needs (/dev/null, /dev/tty, /dev/zero, +// /dev/dtracehelper). // -// SandboxScopeLocal: filesystem access (read and write) is unconfined. +// SandboxScopeLocal: filesystem access (read and write) is unconfined, so it +// does not need (and does not get) the toolchainWritableDirs() lines. // // Both scopes' network access follows the network policy (issue #1397): // under "(deny default)", every operation — including network — is denied @@ -108,6 +116,9 @@ func seatbeltProfile(scope SandboxScope, workspaceRoot string, network NetworkPo switch scope { case SandboxScopeWorkspace: b.WriteString(fmt.Sprintf("(allow file-write* (subpath %s))\n", seatbeltQuote(workspaceRoot))) + for _, dir := range toolchainWritableDirs() { + b.WriteString(fmt.Sprintf("(allow file-write* (subpath %s))\n", seatbeltQuote(dir))) + } b.WriteString(`(allow file-write-data (literal "/dev/null") (literal "/dev/tty") (literal "/dev/dtracehelper") (literal "/dev/zero"))` + "\n") b.WriteString(`(allow file-ioctl (literal "/dev/null") (literal "/dev/tty"))` + "\n") case SandboxScopeLocal: diff --git a/internal/harness/tools/sandbox_darwin_test.go b/internal/harness/tools/sandbox_darwin_test.go index 341c8c08..376017bd 100644 --- a/internal/harness/tools/sandbox_darwin_test.go +++ b/internal/harness/tools/sandbox_darwin_test.go @@ -4,6 +4,8 @@ package tools import ( "context" + "fmt" + "os" "strings" "testing" ) @@ -78,3 +80,53 @@ func TestBuildSandboxedCommandDarwinNetworkPolicyFromContext(t *testing.T) { t.Errorf("expected unrestricted scope to skip sandboxing, got mechanism %q", res4.Mechanism) } } + +// TestSeatbeltProfileIncludesToolchainWritableDirs verifies (issue #1399) +// that the darwin seatbelt profile for SandboxScopeWorkspace emits a +// "(allow file-write* (subpath ...))" line for each of +// toolchainWritableDirs(), not just the workspace root, so language +// toolchains (go build/test, npm, cargo) can write to their per-user +// temp/cache dirs. SandboxScopeLocal already emits a blanket +// "(allow file-write*)" so it does not need — and should not gain — these +// per-dir lines. +func TestSeatbeltProfileIncludesToolchainWritableDirs(t *testing.T) { + writableDirs := toolchainWritableDirs() + if len(writableDirs) == 0 { + t.Fatal("test precondition failed: toolchainWritableDirs() returned no directories on this host") + } + + profile := seatbeltProfile(SandboxScopeWorkspace, t.TempDir(), NetworkPolicyAllow) + for _, dir := range writableDirs { + want := fmt.Sprintf("(allow file-write* (subpath %s))", seatbeltQuote(dir)) + if !strings.Contains(profile, want) { + t.Errorf("expected workspace-scope profile to contain %q for toolchain dir %q, got:\n%s", want, dir, profile) + } + } + + localProfile := seatbeltProfile(SandboxScopeLocal, t.TempDir(), NetworkPolicyAllow) + for _, dir := range writableDirs { + want := fmt.Sprintf("(allow file-write* (subpath %s))", seatbeltQuote(dir)) + if strings.Contains(localProfile, want) { + t.Errorf("expected local-scope profile (already unrestricted) NOT to also emit a per-dir subpath rule for %q, got:\n%s", dir, localProfile) + } + } +} + +// TestBuildSandboxedCommandDarwinReportsWritableDirs verifies that +// buildSandboxedCommand surfaces the toolchain writable dirs on +// SandboxExecResult.WritableDirs for workspace scope, so the bash tool +// result map (bash_manager.go) can report them to the caller. +func TestBuildSandboxedCommandDarwinReportsWritableDirs(t *testing.T) { + workspace := t.TempDir() + _, cleanup, res, err := buildSandboxedCommand(context.Background(), SandboxScopeWorkspace, workspace, "echo hi") + if err != nil { + t.Fatalf("buildSandboxedCommand: %v", err) + } + defer cleanup() + if len(res.WritableDirs) == 0 { + t.Fatalf("expected SandboxExecResult.WritableDirs to be non-empty for workspace scope, got %v", res.WritableDirs) + } + if !containsDir(t, res.WritableDirs, os.TempDir()) { + t.Errorf("expected SandboxExecResult.WritableDirs to include os.TempDir() (%q), got %v", os.TempDir(), res.WritableDirs) + } +} diff --git a/internal/harness/tools/sandbox_linux.go b/internal/harness/tools/sandbox_linux.go index 635ea0a6..f4e6a430 100644 --- a/internal/harness/tools/sandbox_linux.go +++ b/internal/harness/tools/sandbox_linux.go @@ -65,6 +65,7 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo if network == NetworkPolicyDeny { args = append(args, "--unshare-net") } + var writableDirs []string if scope == SandboxScopeWorkspace { // Bind the whole root filesystem read-only, then punch a // read-write hole for the workspace only. Separate mounts @@ -72,10 +73,19 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo // picked up by a "/" bind and must be bound explicitly so // writes there are also confined. args = append(args, "--ro-bind", "/", "/") - for _, extra := range []string{"/tmp", "/var/tmp"} { - if _, statErr := os.Stat(extra); statErr == nil { - args = append(args, "--ro-bind", extra, extra) - } + // /var/tmp stays read-only unless it is itself one of the + // toolchain writable dirs (rare, but handled below); /tmp is + // deliberately NOT ro-bound here — os.TempDir() (bound + // read-write below) is frequently exactly "/tmp" (TMPDIR + // unset), and a ro-bind of the same path would shadow the + // later read-write bind and leave the process temp dir + // unwritable (issue #1399). + if _, statErr := os.Stat("/var/tmp"); statErr == nil { + args = append(args, "--ro-bind", "/var/tmp", "/var/tmp") + } + writableDirs = toolchainWritableDirs() + for _, dir := range writableDirs { + args = append(args, "--bind", dir, dir) } args = append(args, "--bind", absRoot, absRoot) } else { @@ -84,7 +94,7 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo args = append(args, "--", "/bin/bash", "-lc", command) cmd := exec.CommandContext(ctx, bwrapPath, args...) - return cmd, noop, SandboxExecResult{Applied: true, Mechanism: "bubblewrap", NetworkPolicy: network}, nil + return cmd, noop, SandboxExecResult{Applied: true, Mechanism: "bubblewrap", NetworkPolicy: network, WritableDirs: writableDirs}, nil default: return nil, nil, SandboxExecResult{}, fmt.Errorf("unknown sandbox scope %q", scope) } diff --git a/internal/harness/tools/sandbox_linux_test.go b/internal/harness/tools/sandbox_linux_test.go index 37804064..4bea5d32 100644 --- a/internal/harness/tools/sandbox_linux_test.go +++ b/internal/harness/tools/sandbox_linux_test.go @@ -137,6 +137,52 @@ func TestBuildSandboxedCommandLinuxNetworkPolicy(t *testing.T) { } } +// TestBuildSandboxedCommandLinuxIncludesToolchainWritableDirs verifies +// (issue #1399) that the bwrap invocation for SandboxScopeWorkspace binds +// each of toolchainWritableDirs() read-write ("--bind dir dir"), after the +// read-only root binds and before the workspace bind, so language +// toolchains (go build/test, npm, cargo) can write to their per-user +// temp/cache dirs even though those live outside the workspace. Before this +// change /tmp was only ever ro-bound, so the process temp dir ended up +// read-only under workspace scope. +func TestBuildSandboxedCommandLinuxIncludesToolchainWritableDirs(t *testing.T) { + // Not parallel: fakeBwrapOnPath rewrites the process-global PATH via + // t.Setenv, which the testing package forbids in parallel tests. + fakeBwrapOnPath(t) + + writableDirs := toolchainWritableDirs() + if len(writableDirs) == 0 { + t.Fatal("test precondition failed: toolchainWritableDirs() returned no directories on this host") + } + + cmd, cleanup, res, err := buildSandboxedCommand(context.Background(), SandboxScopeWorkspace, t.TempDir(), "echo hi") + if err != nil { + t.Fatalf("buildSandboxedCommand: %v", err) + } + defer cleanup() + + args := bwrapArgsBeforeDoubleDash(cmd) + for _, dir := range writableDirs { + if !containsArgPair(args, "--bind", dir, dir) { + t.Errorf("expected bwrap args to contain \"--bind %s %s\", got: %s", dir, dir, strings.Join(args, " ")) + } + } + if len(res.WritableDirs) == 0 { + t.Fatalf("expected SandboxExecResult.WritableDirs to be non-empty for workspace scope, got %v", res.WritableDirs) + } +} + +// containsArgPair reports whether args contains flag immediately followed by +// src then dst (bwrap's "--bind src dst" triple). +func containsArgPair(args []string, flag, src, dst string) bool { + for i := 0; i+2 < len(args); i++ { + if args[i] == flag && args[i+1] == src && args[i+2] == dst { + return true + } + } + return false +} + // TestSandboxLinuxPIDNamespaceHidesHostProcesses guards #785 at the OS level: // a process inside the sandbox must not be able to signal a host canary // process nor read its /proc environ. Skipped on hosts without a usable diff --git a/internal/harness/tools/sandbox_test.go b/internal/harness/tools/sandbox_test.go index bdc9258e..ab75b7fa 100644 --- a/internal/harness/tools/sandbox_test.go +++ b/internal/harness/tools/sandbox_test.go @@ -7,6 +7,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strings" "testing" "time" ) @@ -106,9 +107,15 @@ func TestCheckSandboxCommandWorkspaceScope(t *testing.T) { absWorkspace, _ := filepath.Abs(workspace) // Commands with absolute paths outside the workspace should be blocked. + // "ls /tmp" is deliberately NOT included here: on hosts where TMPDIR is + // unset, os.TempDir() (and so toolchainWritableDirs(), issue #1399) + // resolves to exactly "/tmp", which would make this assertion + // host-dependent; TestCheckWorkspaceScopeCommandToolchainWritableDirs + // covers that acceptance case directly via os.TempDir() instead of a + // hardcoded path. outsideAbsPaths := []string{ "cat /etc/passwd", - "ls /tmp", + "ls /usr/local/bin/x", "rm /var/log/messages", } for _, cmd := range outsideAbsPaths { @@ -157,8 +164,12 @@ func TestSandboxWorkspaceScopeEnforcesFilePaths(t *testing.T) { workspace := t.TempDir() absWorkspace, _ := filepath.Abs(workspace) - // Writing to a path outside the workspace via absolute path should be blocked. - outsideFile := filepath.Join(filepath.Dir(absWorkspace), "outside.txt") + // Writing to a path outside the workspace via absolute path should be + // blocked. /var/tmp (not a sibling directory under os.TempDir()) is + // used deliberately: issue #1399 opens up os.TempDir() itself for + // writes under workspace scope, so a sibling of the workspace's t.TempDir() + // parent would no longer prove an escape. + outsideFile := filepath.Join("/var/tmp", "harness-sandbox-outside-test.txt") cmd := "echo secret > " + outsideFile if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, cmd); err == nil { t.Errorf("workspace scope: expected error for write to %q, got nil", outsideFile) @@ -182,6 +193,41 @@ func TestCheckSandboxCommandUnknownScope(t *testing.T) { } } +// TestCheckWorkspaceScopeCommandToolchainWritableDirs verifies (issue #1399) +// that checkWorkspaceScopeCommand no longer flags absolute-path tokens that +// fall under one of toolchainWritableDirs()'s roots — e.g. a GOTMPDIR or +// GOCACHE override pointed at the process temp dir or per-user cache dir — +// while still rejecting genuinely out-of-scope system paths. +func TestCheckWorkspaceScopeCommandToolchainWritableDirs(t *testing.T) { + workspace := t.TempDir() + absWorkspace, err := filepath.Abs(workspace) + if err != nil { + t.Fatal(err) + } + + tempFile := filepath.Join(os.TempDir(), "harness-1399-gotmpdir-probe") + accepted := []string{ + "ls " + os.TempDir(), + "echo hi > " + tempFile, + } + for _, cmd := range accepted { + if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, cmd); err != nil { + t.Errorf("expected command %q referencing a toolchain-writable dir to be accepted, got error: %v", cmd, err) + } + } + + rejected := []string{ + "cat /etc/passwd", + "ls /usr/local/bin/x", + "cat ~/.ssh/id_rsa", + } + for _, cmd := range rejected { + if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, cmd); err == nil { + t.Errorf("expected command %q to still be rejected as a sandbox violation, got nil", cmd) + } + } +} + // TestJobManagerSandboxScopeWorkspace verifies that commands blocked by the // workspace sandbox scope are rejected by JobManager.runForeground. func TestJobManagerSandboxScopeWorkspace(t *testing.T) { @@ -319,7 +365,12 @@ func TestSandboxWorkspaceScopeBlocksWriteOutsideWorkspaceAtOSLevel(t *testing.T) mgr := NewJobManager(absWorkspace, nil) mgr.SetSandboxScope(SandboxScopeWorkspace) - target := filepath.Join(os.TempDir(), fmt.Sprintf("harness-sandbox-proof-%d", time.Now().UnixNano())) + // /var/tmp, not os.TempDir(), is the "outside" location here: issue + // #1399 deliberately opens up os.TempDir() (and a handful of per-user + // cache dirs) for writes under workspace scope, so a proof of + // OS-level confinement needs a destination outside every one of those + // toolchain-writable roots to still demonstrate a real boundary. + target := filepath.Join("/var/tmp", fmt.Sprintf("harness-sandbox-proof-%d", time.Now().UnixNano())) _ = os.Remove(target) defer os.Remove(target) @@ -488,3 +539,163 @@ func TestJobManagerRunForegroundReportsSandboxNetworkInResult(t *testing.T) { }) } } + +// TestSandboxWorkspaceScopeToolchainCanBuildAndTest is the integration proof +// for issue #1399: under SandboxScopeWorkspace, with the REAL Go toolchain +// and no env var overrides steering GOCACHE/GOTMPDIR/GOMODCACHE into the +// workspace, `go build ./...` and `go test ./...` must succeed against a +// throwaway module created inside the workspace. Before this change this +// failed with "failed to initialize build cache ... operation not +// permitted" (build cache lives under the per-user cache dir) or "creating +// work dir: mkdir /var/folders/...: operation not permitted" (Go's scratch +// dir lives under the process temp dir) — neither is the workspace, so +// neither the darwin seatbelt profile nor the Linux bwrap binds covered +// them. Skipped when no OS-level sandbox mechanism is available. +func TestSandboxWorkspaceScopeToolchainCanBuildAndTest(t *testing.T) { + if !osSandboxAvailable(t) { + t.Skip("no OS-level sandbox mechanism (seatbelt/bubblewrap) available on this host") + } + if _, err := exec.LookPath("go"); err != nil { + t.Skip("go toolchain not available on this host") + } + + workspace := t.TempDir() + absWorkspace, err := filepath.Abs(workspace) + if err != nil { + t.Fatal(err) + } + + files := map[string]string{ + "go.mod": "module sandboxcachetest\n\ngo 1.21\n", + "main.go": `package main + +func add(a, b int) int { return a + b } + +func main() { println(add(2, 3)) } +`, + "main_test.go": `package main + +import "testing" + +func TestAdd(t *testing.T) { + if add(2, 3) != 5 { + t.Fatal("add(2,3) != 5") + } +} +`, + } + for name, content := range files { + if err := os.WriteFile(filepath.Join(absWorkspace, name), []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + + mgr := NewJobManager(absWorkspace, nil) + mgr.SetSandboxScope(SandboxScopeWorkspace) + + result, err := mgr.RunForeground(context.Background(), "go env GOCACHE && go build ./... && go test ./...", 90, "") + if err != nil { + t.Fatalf("run foreground: %v", err) + } + output, _ := result["output"].(string) + exitCode, _ := result["exit_code"].(int) + if exitCode != 0 { + t.Fatalf("expected go build/test to succeed under workspace sandbox with no env overrides, got exit_code=%d output=%q", exitCode, output) + } + if !strings.Contains(output, "ok") { + t.Errorf("expected go test output to report \"ok\", got: %q", output) + } +} + +// TestSandboxWorkspaceScopeAllowsMktempDir is the second integration proof +// required by issue #1399: `mktemp -d` (which creates a directory under the +// process temp dir, not the workspace) must succeed under +// SandboxScopeWorkspace with no env overrides. +func TestSandboxWorkspaceScopeAllowsMktempDir(t *testing.T) { + if !osSandboxAvailable(t) { + t.Skip("no OS-level sandbox mechanism (seatbelt/bubblewrap) available on this host") + } + + workspace := t.TempDir() + mgr := NewJobManager(workspace, nil) + mgr.SetSandboxScope(SandboxScopeWorkspace) + + result, err := mgr.RunForeground(context.Background(), "mktemp -d", 10, "") + if err != nil { + t.Fatalf("run foreground: %v", err) + } + exitCode, _ := result["exit_code"].(int) + output, _ := result["output"].(string) + if exitCode != 0 { + t.Fatalf("expected \"mktemp -d\" to succeed under workspace sandbox, got exit_code=%d output=%q", exitCode, output) + } + if strings.TrimSpace(output) == "" { + t.Errorf("expected \"mktemp -d\" to print the created directory path, got empty output") + } +} + +// TestJobManagerRunForegroundReportsSandboxWritableDirsInResult is a +// regression test for issue #1399: the bash tool result map must surface +// which extra writable roots were opened up under workspace scope +// (result["sandbox_writable_dirs"]), so an operator/model inspecting a run's +// tool output can see why writes outside the literal workspace succeeded. +func TestJobManagerRunForegroundReportsSandboxWritableDirsInResult(t *testing.T) { + if !osSandboxAvailable(t) { + t.Skip("no OS-level sandbox mechanism (seatbelt/bubblewrap) available on this host") + } + t.Parallel() + + workspace := t.TempDir() + mgr := NewJobManager(workspace, nil) + mgr.SetSandboxScope(SandboxScopeWorkspace) + + result, err := mgr.RunForeground(context.Background(), "echo hi", 5, "") + if err != nil { + t.Fatalf("RunForeground: %v", err) + } + dirs, ok := result["sandbox_writable_dirs"].([]string) + if !ok || len(dirs) == 0 { + t.Fatalf(`expected result["sandbox_writable_dirs"] to be a non-empty []string, got %#v`, result["sandbox_writable_dirs"]) + } + if !containsDir(t, dirs, os.TempDir()) { + t.Errorf(`expected result["sandbox_writable_dirs"] to include os.TempDir() (%q), got %v`, os.TempDir(), dirs) + } +} + +// TestSandboxWorkspaceScopeGOCACHEOverrideIsWritableEndToEnd is a +// regression test for issue #1399: it is a different angle from the other +// integration tests above (which only exercise the default os.TempDir() +// path) — it points GOCACHE at a custom directory via the real process +// environment and proves, through the real OS-level sandbox mechanism, that +// a write there succeeds. If a future change stopped reading $GOCACHE in +// toolchainWritableDirs(), or stopped threading its result into the darwin +// seatbelt profile / Linux bwrap binds, this test would fail with an +// "operation not permitted" exit code exactly like the original bug report, +// independent of the other tests that only cover the unconfigured default. +func TestSandboxWorkspaceScopeGOCACHEOverrideIsWritableEndToEnd(t *testing.T) { + if !osSandboxAvailable(t) { + t.Skip("no OS-level sandbox mechanism (seatbelt/bubblewrap) available on this host") + } + + customGocache := t.TempDir() + t.Setenv("GOCACHE", customGocache) + + workspace := t.TempDir() + mgr := NewJobManager(workspace, nil) + mgr.SetSandboxScope(SandboxScopeWorkspace) + + marker := filepath.Join(customGocache, "sandbox-1399-marker") + command := "echo written > " + marker + result, err := mgr.RunForeground(context.Background(), command, 10, "") + if err != nil { + t.Fatalf("RunForeground: %v", err) + } + exitCode, _ := result["exit_code"].(int) + output, _ := result["output"].(string) + if exitCode != 0 { + t.Fatalf("expected write to custom GOCACHE dir to succeed under workspace sandbox, got exit_code=%d output=%q", exitCode, output) + } + if _, statErr := os.Stat(marker); statErr != nil { + t.Fatalf("expected marker file %q to exist after the sandboxed write, got stat error: %v", marker, statErr) + } +} diff --git a/internal/harness/tools/toolchain_dirs.go b/internal/harness/tools/toolchain_dirs.go new file mode 100644 index 00000000..f1f9a54f --- /dev/null +++ b/internal/harness/tools/toolchain_dirs.go @@ -0,0 +1,121 @@ +package tools + +import ( + "os" + "path/filepath" +) + +// toolchainWritableDirs returns the per-user temp and cache directories that +// language-toolchain invocations (go build/test, npm, cargo) need to write +// to even though nothing else about them lives inside the workspace (issue +// #1399). Without these, a `go build` run under SandboxScopeWorkspace fails +// with "failed to initialize build cache ... operation not permitted" (the +// build cache lives under the per-user cache dir) or "creating work dir: +// mkdir /var/folders/...: operation not permitted" (Go's scratch dir lives +// under the process temp dir) — neither of those roots is the workspace, so +// neither darwin's seatbelt "(subpath ...)" allow-list nor Linux's bwrap +// binds cover them today. +// +// Included roots (only when they already exist, unless noted): +// - os.TempDir() — honors TMPDIR. +// - os.UserCacheDir() — honors XDG_CACHE_HOME / ~/Library/Caches. +// - ~/.cache, when different from the above — CREATED if missing, since a +// fresh machine may not have it yet and it is one of the two +// conventional cache homes this function is allowed to create. +// - $GOCACHE, if the env var is set and the directory already exists. +// - The Go module cache: $GOMODCACHE if set, else $GOPATH/pkg, else +// ~/go/pkg — CREATED if missing (the other of the two directories this +// function is allowed to create). +// - ~/.npm, ~/.cargo/registry, ~/.cargo/git — only when they already +// exist. +// +// $HOME itself is never included: only these specific, narrow +// subdirectories are opened up for writes. +// +// Every returned path is symlink-canonicalized (filepath.EvalSymlinks) the +// same way the workspace root already is in buildSandboxedCommand, so the +// darwin seatbelt profile's "(subpath ...)" match (which operates on the +// kernel's resolved path, not a symlinked one) and the Linux bwrap +// "--bind src dst" invocation actually cover what gets written to at +// runtime. +// +// Computed fresh on every call (not cached) so it reflects the calling +// process's current environment (TMPDIR, GOCACHE, GOMODCACHE, GOPATH, +// XDG_CACHE_HOME) rather than a snapshot taken at process start — tests +// exercise this via t.Setenv, and a long-lived harnessd should not need a +// restart for env changes to take effect here. +func toolchainWritableDirs() []string { + var dirs []string + seen := make(map[string]bool) + + addExisting := func(p string) { + if p == "" { + return + } + if _, err := os.Stat(p); err != nil { + return + } + addCanonicalDir(&dirs, seen, p) + } + addCreated := func(p string) { + if p == "" { + return + } + if err := os.MkdirAll(p, 0o755); err != nil { + return + } + addCanonicalDir(&dirs, seen, p) + } + + addExisting(os.TempDir()) + + if cacheDir, err := os.UserCacheDir(); err == nil { + addExisting(cacheDir) + } + + home, homeErr := os.UserHomeDir() + if homeErr == nil { + addCreated(filepath.Join(home, ".cache")) + } + + addExisting(os.Getenv("GOCACHE")) + + modCache := os.Getenv("GOMODCACHE") + if modCache == "" { + gopath := os.Getenv("GOPATH") + if gopath == "" && homeErr == nil { + gopath = filepath.Join(home, "go") + } + if gopath != "" { + modCache = filepath.Join(gopath, "pkg") + } + } + addCreated(modCache) + + if homeErr == nil { + addExisting(filepath.Join(home, ".npm")) + addExisting(filepath.Join(home, ".cargo", "registry")) + addExisting(filepath.Join(home, ".cargo", "git")) + } + + return dirs +} + +// addCanonicalDir resolves p to an absolute, symlink-canonicalized path +// (falling back to the Abs/Clean form if EvalSymlinks fails) and appends it +// to *dirs unless already present. +func addCanonicalDir(dirs *[]string, seen map[string]bool, p string) { + abs, err := filepath.Abs(p) + if err != nil { + abs = p + } + abs = filepath.Clean(abs) + if resolved, err := filepath.EvalSymlinks(abs); err == nil { + abs = resolved + } + if seen[abs] { + return + } + seen[abs] = true + *dirs = append(*dirs, abs) +} diff --git a/internal/harness/tools/toolchain_dirs_test.go b/internal/harness/tools/toolchain_dirs_test.go new file mode 100644 index 00000000..2a816a16 --- /dev/null +++ b/internal/harness/tools/toolchain_dirs_test.go @@ -0,0 +1,240 @@ +package tools + +import ( + "os" + "path/filepath" + "testing" +) + +// containsDir reports whether dirs contains want after canonicalizing both +// sides the same way (Abs + best-effort EvalSymlinks), so tests do not have +// to hand-resolve platform-specific symlinks (e.g. /tmp -> /private/tmp on +// macOS) themselves. +func containsDir(t *testing.T, dirs []string, want string) bool { + t.Helper() + wantResolved := resolveForTest(t, want) + for _, d := range dirs { + if d == wantResolved || d == want { + return true + } + } + return false +} + +func resolveForTest(t *testing.T, p string) string { + t.Helper() + abs, err := filepath.Abs(p) + if err != nil { + return p + } + if resolved, err := filepath.EvalSymlinks(abs); err == nil { + return resolved + } + return abs +} + +// TestToolchainWritableDirsIncludesTempDir verifies that os.TempDir() (which +// honors TMPDIR) is always included, since Go's own scratch directory for +// `go build`/`go test` lives there. +func TestToolchainWritableDirsIncludesTempDir(t *testing.T) { + dirs := toolchainWritableDirs() + if !containsDir(t, dirs, os.TempDir()) { + t.Errorf("expected toolchainWritableDirs() to include os.TempDir() (%q), got %v", os.TempDir(), dirs) + } +} + +// TestToolchainWritableDirsCreatesModuleCacheWhenMissing verifies the Go +// module cache (GOMODCACHE, or GOPATH/pkg, or ~/go/pkg as the final +// fallback) is created if it does not already exist, per the contract: this +// is one of only two directories toolchainWritableDirs is allowed to create +// rather than merely detect. +func TestToolchainWritableDirsCreatesModuleCacheWhenMissing(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("GOPATH", "") + t.Setenv("GOMODCACHE", "") + + wantModCache := filepath.Join(home, "go", "pkg") + if _, err := os.Stat(wantModCache); err == nil { + t.Fatalf("test setup invariant violated: %q already exists", wantModCache) + } + + dirs := toolchainWritableDirs() + + if _, err := os.Stat(wantModCache); err != nil { + t.Fatalf("expected toolchainWritableDirs() to create %q, got stat error: %v", wantModCache, err) + } + if !containsDir(t, dirs, wantModCache) { + t.Errorf("expected toolchainWritableDirs() to include the created module cache %q, got %v", wantModCache, dirs) + } +} + +// TestToolchainWritableDirsRespectsGOMODCACHEOverride verifies an explicit +// GOMODCACHE override is used (and created if missing) instead of the +// GOPATH/pkg or ~/go/pkg fallback chain. +func TestToolchainWritableDirsRespectsGOMODCACHEOverride(t *testing.T) { + home := t.TempDir() + override := filepath.Join(t.TempDir(), "custom-modcache") + t.Setenv("HOME", home) + t.Setenv("GOPATH", "") + t.Setenv("GOMODCACHE", override) + + dirs := toolchainWritableDirs() + + if _, err := os.Stat(override); err != nil { + t.Fatalf("expected toolchainWritableDirs() to create GOMODCACHE override %q, got stat error: %v", override, err) + } + if !containsDir(t, dirs, override) { + t.Errorf("expected toolchainWritableDirs() to include GOMODCACHE override %q, got %v", override, dirs) + } + defaultModCache := filepath.Join(home, "go", "pkg") + if containsDir(t, dirs, defaultModCache) { + t.Errorf("expected toolchainWritableDirs() NOT to also include the default module cache %q when GOMODCACHE is set, got %v", defaultModCache, dirs) + } +} + +// TestToolchainWritableDirsIncludesExistingGOCACHE verifies an explicit +// GOCACHE is included when it already exists on disk. +func TestToolchainWritableDirsIncludesExistingGOCACHE(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + gocache := t.TempDir() + t.Setenv("GOCACHE", gocache) + + dirs := toolchainWritableDirs() + if !containsDir(t, dirs, gocache) { + t.Errorf("expected toolchainWritableDirs() to include existing GOCACHE %q, got %v", gocache, dirs) + } +} + +// TestToolchainWritableDirsExcludesMissingGOCACHE verifies GOCACHE is only +// included (and never created) when it already exists — unlike the module +// cache, GOCACHE is not one of the two directories the contract allows this +// function to create. +func TestToolchainWritableDirsExcludesMissingGOCACHE(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + missing := filepath.Join(t.TempDir(), "does-not-exist-gocache") + t.Setenv("GOCACHE", missing) + + dirs := toolchainWritableDirs() + if containsDir(t, dirs, missing) { + t.Errorf("expected toolchainWritableDirs() NOT to include a nonexistent GOCACHE %q, got %v", missing, dirs) + } + if _, err := os.Stat(missing); err == nil { + t.Errorf("expected toolchainWritableDirs() NOT to create a missing GOCACHE %q, but it now exists", missing) + } +} + +// TestToolchainWritableDirsCreatesDotCacheWhenMissing verifies ~/.cache is +// created if missing — the other directory (besides the module cache) the +// contract allows this function to create. +func TestToolchainWritableDirsCreatesDotCacheWhenMissing(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CACHE_HOME", "") + + wantDotCache := filepath.Join(home, ".cache") + if _, err := os.Stat(wantDotCache); err == nil { + t.Fatalf("test setup invariant violated: %q already exists", wantDotCache) + } + + dirs := toolchainWritableDirs() + + if _, err := os.Stat(wantDotCache); err != nil { + t.Fatalf("expected toolchainWritableDirs() to create ~/.cache (%q), got stat error: %v", wantDotCache, err) + } + if !containsDir(t, dirs, wantDotCache) { + t.Errorf("expected toolchainWritableDirs() to include the created ~/.cache %q, got %v", wantDotCache, dirs) + } +} + +// TestToolchainWritableDirsIncludesNpmAndCargoWhenPresent verifies ~/.npm, +// ~/.cargo/registry, and ~/.cargo/git are included when they already exist, +// since those are the per-user cache roots npm and cargo write to. +func TestToolchainWritableDirsIncludesNpmAndCargoWhenPresent(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + npm := filepath.Join(home, ".npm") + cargoRegistry := filepath.Join(home, ".cargo", "registry") + cargoGit := filepath.Join(home, ".cargo", "git") + for _, d := range []string{npm, cargoRegistry, cargoGit} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatalf("mkdir %q: %v", d, err) + } + } + + dirs := toolchainWritableDirs() + for _, want := range []string{npm, cargoRegistry, cargoGit} { + if !containsDir(t, dirs, want) { + t.Errorf("expected toolchainWritableDirs() to include existing %q, got %v", want, dirs) + } + } +} + +// TestToolchainWritableDirsExcludesNpmAndCargoWhenAbsent verifies npm/cargo +// dirs that do not exist are skipped rather than created (unlike ~/.cache +// and the module cache). +func TestToolchainWritableDirsExcludesNpmAndCargoWhenAbsent(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + dirs := toolchainWritableDirs() + for _, absent := range []string{ + filepath.Join(home, ".npm"), + filepath.Join(home, ".cargo", "registry"), + filepath.Join(home, ".cargo", "git"), + } { + if containsDir(t, dirs, absent) { + t.Errorf("expected toolchainWritableDirs() NOT to include nonexistent %q, got %v", absent, dirs) + } + } +} + +// TestToolchainWritableDirsNoBlanketHome guards against a regression to +// blanket $HOME write access: the returned list must never contain the home +// directory itself, only the specific narrow subdirectories the contract +// names. +func TestToolchainWritableDirsNoBlanketHome(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + dirs := toolchainWritableDirs() + homeResolved := resolveForTest(t, home) + for _, d := range dirs { + if d == home || d == homeResolved { + t.Fatalf("toolchainWritableDirs() must never include $HOME itself, got %v", dirs) + } + } +} + +// TestToolchainWritableDirsCanonicalizesSymlinks verifies each returned +// directory is symlink-canonicalized (filepath.EvalSymlinks), the same way +// the workspace root already is, so the darwin seatbelt "(subpath ...)" +// match (which operates on the kernel's resolved path) and Linux's bwrap +// bind source/target actually cover what gets written to at runtime. +func TestToolchainWritableDirsCanonicalizesSymlinks(t *testing.T) { + realDir := t.TempDir() + linkParent := t.TempDir() + link := filepath.Join(linkParent, "tmp-link") + if err := os.Symlink(realDir, link); err != nil { + t.Skipf("symlink not supported on this filesystem: %v", err) + } + t.Setenv("HOME", t.TempDir()) + t.Setenv("TMPDIR", link) + + dirs := toolchainWritableDirs() + + resolvedReal := resolveForTest(t, realDir) + found := false + for _, d := range dirs { + if d == link { + t.Errorf("expected toolchainWritableDirs() to return the symlink-resolved form of TMPDIR, got the raw symlink path %q", d) + } + if d == resolvedReal { + found = true + } + } + if !found { + t.Errorf("expected toolchainWritableDirs() to include the symlink-resolved TMPDIR %q, got %v", resolvedReal, dirs) + } +} diff --git a/website/docs/concepts/tools-and-permissions.md b/website/docs/concepts/tools-and-permissions.md index e65e8fb0..d7bab1a9 100644 --- a/website/docs/concepts/tools-and-permissions.md +++ b/website/docs/concepts/tools-and-permissions.md @@ -122,6 +122,8 @@ The sandbox scope controls what the agent's `bash` tool can access. **`"workspace"`** — Bash commands that reference absolute paths outside the workspace or attempt `cd ..` escapes are rejected. This is a defence-in-depth heuristic, not a kernel-level filesystem jail — it tokenizes the command for out-of-workspace absolute paths and matches `cd ..` patterns. This is the default when `permissions` is omitted. +Writes are also permitted to a small set of per-user temp and cache directories (issue #1399): the process temp dir (`os.TempDir()`, i.e. `TMPDIR`), the OS per-user cache dir (`os.UserCacheDir()`), `~/.cache`, the Go build/module caches (`GOCACHE`, `GOMODCACHE`, or `GOPATH/pkg`/`~/go/pkg` as a fallback), `~/.npm`, and `~/.cargo/registry`/`~/.cargo/git`. Without this, `go build`/`go test`, `npm install`, and `cargo build` all fail under workspace scope with an "operation not permitted" error the moment they try to write their build cache or a scratch directory — none of those roots is the workspace itself. `$HOME` itself is never opened up wholesale, only these specific subdirectories, and only when they already exist (the Go module cache and `~/.cache` are created if missing). + This scope is recommended for untrusted prompts operating on a bounded codebase. @@ -139,7 +141,7 @@ The agent can read and write any path on the host filesystem and run arbitrary s -Source: `internal/harness/types.go`. +Source: `internal/harness/types.go`, `internal/harness/tools/toolchain_dirs.go`, `internal/harness/tools/sandbox_darwin.go`, `internal/harness/tools/sandbox_linux.go`. ### Network policy diff --git a/website/docs/reference/glossary.md b/website/docs/reference/glossary.md index c15c8791..d1892a2b 100644 --- a/website/docs/reference/glossary.md +++ b/website/docs/reference/glossary.md @@ -80,7 +80,7 @@ See: [Subagents and profiles](/docs/integrations/subagents-and-profiles) A constraint that limits what the agent's shell and file tools can reach. The three levels, set via `permissions.sandbox` in a `RunRequest`, are: -- `"workspace"` — the `bash` tool can only access paths inside the workspace directory (default) +- `"workspace"` — the `bash` tool can only access paths inside the workspace directory, plus a small set of per-user temp/cache directories a language toolchain needs (`os.TempDir()`, Go build/module caches, `~/.npm`, `~/.cargo`; issue #1399) (default) - `"local"` — filesystem access is unrestricted - `"unrestricted"` — no restrictions