From fdf22b520dd3ab393b51d3079f717c0ba66de263 Mon Sep 17 00:00:00 2001 From: Dennison Date: Sun, 6 Sep 2026 09:54:33 -0400 Subject: [PATCH 1/4] test(red): TASK-1397 failing tests for sandbox network policy Behavioral tests added ahead of implementation for issue #1397: - PermissionConfig.Network validation (allow/deny/empty valid, anything else rejected) and normalizePermissionConfig defaulting to allow. - tools.NetworkPolicy context plumbing, seatbeltProfile (darwin) and bwrap args (linux) gated by network policy instead of always denying network for workspace/local scope, and CheckSandboxCommand's local-scope heuristic gated the same way. - Runner first-turn permissions notice includes "network=allow|deny" and, when denied, the "Outbound network is blocked..." warning sentence. - CLI --sandbox/--network flags populate a "permissions" object on the run-create request body; omitted when neither flag is set. - A live, real-network integration test that curls https://proxy.golang.org through the actual seatbelt sandbox under both policies. None of this compiles yet: the tests reference NetworkPolicy, its constants, WithNetworkPolicy, SandboxExecResult.NetworkPolicy, the new seatbeltProfile/CheckSandboxCommand signatures, and PermissionConfig.Network, none of which exist before the next commit. Existing tests were also updated in place to assert the new default (network allowed unless denied) where they previously asserted the old always-deny behavior for SandboxScopeLocal, and to account for the new permissions-notice tail message. go vet output (internal/harness, internal/harness/tools): # go-agent-harness/internal/harness/tools [go-agent-harness/internal/harness/tools.test] internal/harness/tools/sandbox_darwin_test.go:19:50: undefined: NetworkPolicyDeny internal/harness/tools/sandbox_darwin_test.go:19:50: too many arguments in call to seatbeltProfile have (SandboxScope, string, unknown type) want (SandboxScope, string) internal/harness/tools/sandbox_darwin_test.go:24:49: undefined: NetworkPolicyAllow internal/harness/tools/sandbox_darwin_test.go:40:9: undefined: WithNetworkPolicy internal/harness/tools/sandbox_darwin_test.go:46:9: res.NetworkPolicy undefined (type SandboxExecResult has no field or method NetworkPolicy) # go-agent-harness/internal/harness vet: internal/harness/permission_config_test.go:80:66: unknown field Network in struct literal of type PermissionConfig go test output (cmd/harnesscli, does not depend on new symbols so it compiles and fails at runtime instead): === RUN TestRunParsesSandboxAndNetworkFlagsIntoPermissions main_permissions_test.go:61: expected exit code 0, got 1 --- FAIL: TestRunParsesSandboxAndNetworkFlagsIntoPermissions (0.00s) === RUN TestRunOmitsPermissionsWhenNoSandboxOrNetworkFlagSet --- PASS: TestRunOmitsPermissionsWhenNoSandboxOrNetworkFlagSet (0.00s) FAIL These tests will pass after the implementation in the next commit. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5 --- cmd/harnesscli/main_permissions_test.go | 143 ++++++++++++++++++ internal/harness/permission_config_test.go | 41 +++++ .../runner_continuerun_security_test.go | 2 + .../harness/runner_network_notice_test.go | 84 ++++++++++ internal/harness/runner_redteam_test.go | 4 + internal/harness/runner_task_complete_test.go | 12 +- internal/harness/runner_test.go | 6 +- .../runner_working_memory_sqlite_test.go | 13 +- .../harness/runner_working_memory_test.go | 19 ++- internal/harness/tools/sandbox_darwin_test.go | 80 ++++++++++ internal/harness/tools/sandbox_linux_test.go | 128 ++++++++++++---- internal/harness/tools/sandbox_test.go | 135 +++++++++++++---- 12 files changed, 594 insertions(+), 73 deletions(-) create mode 100644 cmd/harnesscli/main_permissions_test.go create mode 100644 internal/harness/runner_network_notice_test.go create mode 100644 internal/harness/tools/sandbox_darwin_test.go diff --git a/cmd/harnesscli/main_permissions_test.go b/cmd/harnesscli/main_permissions_test.go new file mode 100644 index 000000000..5dc79ad66 --- /dev/null +++ b/cmd/harnesscli/main_permissions_test.go @@ -0,0 +1,143 @@ +package main + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" +) + +// TestRunParsesSandboxAndNetworkFlagsIntoPermissions verifies that --sandbox +// and --network populate a "permissions" object on the run-create request +// body (issue #1397). When neither flag is set, "permissions" must be +// entirely absent so the server falls back to its own defaults. +func TestRunParsesSandboxAndNetworkFlagsIntoPermissions(t *testing.T) { + var rawBody []byte + mux := http.NewServeMux() + mux.HandleFunc("/v1/runs", func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + body, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("read request body: %v", err) + } + rawBody = body + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = io.WriteString(w, `{"run_id":"run_perm","status":"queued"}`) + }) + mux.HandleFunc("/v1/runs/run_perm/events", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/event-stream; charset=utf-8") + _, _ = io.WriteString(w, "event: run.completed\n") + _, _ = io.WriteString(w, "data: {\"id\":\"e1\",\"run_id\":\"run_perm\",\"type\":\"run.completed\"}\n\n") + }) + ts := httptest.NewServer(mux) + defer ts.Close() + + origRequestClient := requestHTTPClient + origStreamClient := streamHTTPClient + origStdout := stdout + origStderr := stderr + defer func() { + requestHTTPClient = origRequestClient + streamHTTPClient = origStreamClient + stdout = origStdout + stderr = origStderr + }() + + requestHTTPClient = ts.Client() + streamHTTPClient = ts.Client() + stdout = &bytes.Buffer{} + stderr = &bytes.Buffer{} + + code := run([]string{ + "-base-url=" + ts.URL, + "-prompt=do work", + "-sandbox=local", + "-network=deny", + }) + if code != 0 { + t.Fatalf("expected exit code 0, got %d", code) + } + + var body map[string]any + if err := json.Unmarshal(rawBody, &body); err != nil { + t.Fatalf("decode captured request body: %v; raw=%s", err, rawBody) + } + permsRaw, ok := body["permissions"] + if !ok { + t.Fatalf("expected \"permissions\" key in request body, got: %s", rawBody) + } + perms, ok := permsRaw.(map[string]any) + if !ok { + t.Fatalf("expected \"permissions\" to be an object, got: %T (%v)", permsRaw, permsRaw) + } + if perms["sandbox"] != "local" { + t.Errorf("expected permissions.sandbox=%q, got %v", "local", perms["sandbox"]) + } + if perms["network"] != "deny" { + t.Errorf("expected permissions.network=%q, got %v", "deny", perms["network"]) + } + if approval, ok := perms["approval"]; !ok || approval != "" { + t.Errorf("expected permissions.approval to be present and empty (matching the struct's json tag), got %v (present=%v)", approval, ok) + } +} + +// TestRunOmitsPermissionsWhenNoSandboxOrNetworkFlagSet verifies that a plain +// run request (no --sandbox, no --network) sends no "permissions" field at +// all, leaving the server's own defaults in effect (issue #1397). +func TestRunOmitsPermissionsWhenNoSandboxOrNetworkFlagSet(t *testing.T) { + var rawBody []byte + mux := http.NewServeMux() + mux.HandleFunc("/v1/runs", func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + body, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("read request body: %v", err) + } + rawBody = body + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = io.WriteString(w, `{"run_id":"run_noperm","status":"queued"}`) + }) + mux.HandleFunc("/v1/runs/run_noperm/events", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/event-stream; charset=utf-8") + _, _ = io.WriteString(w, "event: run.completed\n") + _, _ = io.WriteString(w, "data: {\"id\":\"e1\",\"run_id\":\"run_noperm\",\"type\":\"run.completed\"}\n\n") + }) + ts := httptest.NewServer(mux) + defer ts.Close() + + origRequestClient := requestHTTPClient + origStreamClient := streamHTTPClient + origStdout := stdout + origStderr := stderr + defer func() { + requestHTTPClient = origRequestClient + streamHTTPClient = origStreamClient + stdout = origStdout + stderr = origStderr + }() + + requestHTTPClient = ts.Client() + streamHTTPClient = ts.Client() + stdout = &bytes.Buffer{} + stderr = &bytes.Buffer{} + + code := run([]string{ + "-base-url=" + ts.URL, + "-prompt=do work", + }) + if code != 0 { + t.Fatalf("expected exit code 0, got %d", code) + } + + var body map[string]any + if err := json.Unmarshal(rawBody, &body); err != nil { + t.Fatalf("decode captured request body: %v; raw=%s", err, rawBody) + } + if _, ok := body["permissions"]; ok { + t.Errorf("expected no \"permissions\" key when neither flag is set, got: %s", rawBody) + } +} diff --git a/internal/harness/permission_config_test.go b/internal/harness/permission_config_test.go index b602b153f..ccb3ca0fa 100644 --- a/internal/harness/permission_config_test.go +++ b/internal/harness/permission_config_test.go @@ -70,6 +70,47 @@ func TestPermissionConfigValidation(t *testing.T) { } } +// TestPermissionConfigNetworkValidation checks that ValidatePermissionConfig +// accepts the empty/allow/deny NetworkPolicy values and rejects anything +// else (issue #1397). +func TestPermissionConfigNetworkValidation(t *testing.T) { + t.Parallel() + + valid := []PermissionConfig{ + {Sandbox: SandboxScopeWorkspace, Approval: ApprovalPolicyNone, Network: ""}, + {Sandbox: SandboxScopeWorkspace, Approval: ApprovalPolicyNone, Network: NetworkPolicyAllow}, + {Sandbox: SandboxScopeWorkspace, Approval: ApprovalPolicyNone, Network: NetworkPolicyDeny}, + } + for _, cfg := range valid { + if err := ValidatePermissionConfig(cfg); err != nil { + t.Errorf("expected valid network %q to pass, got: %v", cfg.Network, err) + } + } + + badCfg := PermissionConfig{Sandbox: SandboxScopeWorkspace, Approval: ApprovalPolicyNone, Network: "maybe"} + if err := ValidatePermissionConfig(badCfg); err == nil { + t.Error("expected invalid network \"maybe\" to fail, but got nil error") + } +} + +// TestNormalizePermissionConfigDefaultsNetworkToAllow verifies that an empty +// Network field normalizes to NetworkPolicyAllow, matching the safety-biased +// default documented on PermissionConfig (issue #1397): workspace/local +// scopes allow outbound network unless a caller explicitly opts into deny. +func TestNormalizePermissionConfigDefaultsNetworkToAllow(t *testing.T) { + t.Parallel() + + got := normalizePermissionConfig(PermissionConfig{Sandbox: SandboxScopeWorkspace, Approval: ApprovalPolicyNone}) + if got.Network != NetworkPolicyAllow { + t.Errorf("expected normalized empty network to default to %q, got %q", NetworkPolicyAllow, got.Network) + } + + got = normalizePermissionConfig(PermissionConfig{Sandbox: SandboxScopeWorkspace, Approval: ApprovalPolicyNone, Network: NetworkPolicyDeny}) + if got.Network != NetworkPolicyDeny { + t.Errorf("expected explicit deny to be preserved, got %q", got.Network) + } +} + // TestPermissionConfigToLegacy verifies backward-compatible mapping from // PermissionConfig to the legacy ToolApprovalMode. func TestPermissionConfigToLegacy(t *testing.T) { diff --git a/internal/harness/runner_continuerun_security_test.go b/internal/harness/runner_continuerun_security_test.go index c1eb971b3..4c81a3ab5 100644 --- a/internal/harness/runner_continuerun_security_test.go +++ b/internal/harness/runner_continuerun_security_test.go @@ -199,6 +199,7 @@ func TestContinueRunWithOptions_OverridesAllowedToolsAndPermissions(t *testing.T overridePerms := PermissionConfig{ Sandbox: SandboxScopeLocal, Approval: ApprovalPolicyAll, + Network: NetworkPolicyAllow, } run2, err := runner.ContinueRunWithOptions(run1.ID, ContinueRunRequest{ Prompt: "follow up", @@ -512,6 +513,7 @@ func TestContinueRunPropagatesPermissions(t *testing.T) { wantPerms := PermissionConfig{ Sandbox: SandboxScopeWorkspace, Approval: ApprovalPolicyDestructive, + Network: NetworkPolicyAllow, } run1, err := runner.StartRun(RunRequest{ Prompt: "initial", diff --git a/internal/harness/runner_network_notice_test.go b/internal/harness/runner_network_notice_test.go new file mode 100644 index 000000000..4e58de307 --- /dev/null +++ b/internal/harness/runner_network_notice_test.go @@ -0,0 +1,84 @@ +package harness + +import ( + "strings" + "testing" +) + +// TestRunnerFirstTurnPermissionsNoticeIncludesNetworkAllow verifies that the +// permissions statement injected into the very first turn's messages +// includes the network axis (issue #1397). Previously this notice only +// appeared on a continuation whose permissions changed, so a model never +// learned its network policy on turn one. +func TestRunnerFirstTurnPermissionsNoticeIncludesNetworkAllow(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"}) + 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, "network=allow") { + t.Fatalf("expected first-turn messages to include a network=allow permissions notice, got %+v", provider.calls[0].Messages) + } +} + +// TestRunnerFirstTurnPermissionsNoticeIncludesNetworkDenyWarning verifies +// that when a run's PermissionConfig sets Network: NetworkPolicyDeny, the +// first-turn permissions notice both reports network=deny and warns the +// model that dependency installs will fail rather than letting it silently +// substitute a different design (issue #1397). +func TestRunnerFirstTurnPermissionsNoticeIncludesNetworkDenyWarning(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: NetworkPolicyDeny}, + }) + 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, "network=deny") { + t.Fatalf("expected first-turn messages to include a network=deny permissions notice, got %+v", provider.calls[0].Messages) + } + wantWarning := "Outbound network is blocked for this run: dependency installs will fail; report the blocker instead of substituting a different design." + if !anyMessageContains(provider.calls[0].Messages, wantWarning) { + t.Fatalf("expected first-turn messages to include the network-deny warning sentence, got %+v", provider.calls[0].Messages) + } +} + +func anyMessageContains(messages []Message, substr string) bool { + for _, msg := range messages { + if strings.Contains(msg.Content, substr) { + return true + } + } + return false +} diff --git a/internal/harness/runner_redteam_test.go b/internal/harness/runner_redteam_test.go index 5168dfc9c..c81b82003 100644 --- a/internal/harness/runner_redteam_test.go +++ b/internal/harness/runner_redteam_test.go @@ -330,6 +330,10 @@ func TestRedTeam_SandboxNetwork_BlocksCurl(t *testing.T) { Permissions: &PermissionConfig{ Sandbox: SandboxScopeLocal, Approval: ApprovalPolicyNone, + // Issue #1397 changed the default so local scope allows outbound + // network; this red-team scenario exercises the still-available + // explicit-deny path, not the new default. + Network: NetworkPolicyDeny, }, }) if err != nil { diff --git a/internal/harness/runner_task_complete_test.go b/internal/harness/runner_task_complete_test.go index cb212c034..185a8b22c 100644 --- a/internal/harness/runner_task_complete_test.go +++ b/internal/harness/runner_task_complete_test.go @@ -291,10 +291,14 @@ func TestRunForkedSkill_TrustedOriginIgnoresTamperedMetadataForInheritedPolicy(t provider := &funcProvider{fn: func(_ context.Context, req CompletionRequest) (CompletionResult, error) { captureMu.Lock() defer captureMu.Unlock() - for _, message := range req.Messages { - if message.Role == "system" { - capturedSystemPrompt = message.Content - } + // buildTurnMessages always places the static system prompt (if any) + // at index 0, ahead of history and the volatile tail (working/ + // observational memory, dynamic rules, plan-mode guidance, the + // permissions notice added in issue #1397, and runtime context) — + // several of which are also role=system. Only the leading message + // is the system prompt under test here. + if len(req.Messages) > 0 && req.Messages[0].Role == "system" { + capturedSystemPrompt = req.Messages[0].Content } return CompletionResult{Content: "child done"}, nil }} diff --git a/internal/harness/runner_test.go b/internal/harness/runner_test.go index 09e7bd2c0..e537cf4f3 100644 --- a/internal/harness/runner_test.go +++ b/internal/harness/runner_test.go @@ -633,9 +633,11 @@ func TestRunnerInjectsMemorySnippetAndEmitsMemoryEvents(t *testing.T) { } // Volatile blocks (including observational memory) are injected at the tail, // after the conversation history, so the cached prefix is not invalidated. + // The permissions notice (issue #1397) is appended after memory on every + // turn, so the memory snippet is now second-to-last rather than last. msgs0 := provider.calls[0].Messages - if len(msgs0) < 1 || msgs0[len(msgs0)-1].Content != "test" { - t.Fatalf("expected injected memory snippet at the tail of the first request: %+v", msgs0) + if len(msgs0) < 2 || msgs0[len(msgs0)-2].Content != "test" { + t.Fatalf("expected injected memory snippet second-to-last (before the permissions notice) in the first request: %+v", msgs0) } requireEventOrder(t, events, "memory.observe.started", "memory.observe.completed", "run.completed") } diff --git a/internal/harness/runner_working_memory_sqlite_test.go b/internal/harness/runner_working_memory_sqlite_test.go index 5c42c648b..8bd92b2b6 100644 --- a/internal/harness/runner_working_memory_sqlite_test.go +++ b/internal/harness/runner_working_memory_sqlite_test.go @@ -171,11 +171,18 @@ func injectedMemorySnippetForRun(t *testing.T, store workingmemory.Store, req Ru } msgs := provider.calls[0].Messages // Working/observational memory is injected as a system message at the tail - // (after the history) for cache friendliness; return the last system message. + // (after the history) for cache friendliness; return the last such + // message. The permissions notice (issue #1397) is appended after memory + // on every turn, so it is skipped here rather than mistaken for the + // memory injection. for i := len(msgs) - 1; i >= 0; i-- { - if msgs[i].Role == "system" { - return msgs[i].Content + if msgs[i].Role != "system" { + continue } + if strings.HasPrefix(msgs[i].Content, "Permissions for this run:") { + continue + } + return msgs[i].Content } return "" } diff --git a/internal/harness/runner_working_memory_test.go b/internal/harness/runner_working_memory_test.go index cfda0d6a3..317e66ea3 100644 --- a/internal/harness/runner_working_memory_test.go +++ b/internal/harness/runner_working_memory_test.go @@ -100,15 +100,20 @@ func TestRunnerInjectsWorkingMemoryBeforeObservationalMemory(t *testing.T) { t.Fatal("expected provider call") } messages := provider.calls[0].Messages - if len(messages) < 3 { - t.Fatalf("message count = %d, want at least 3", len(messages)) + if len(messages) < 4 { + t.Fatalf("message count = %d, want at least 4", len(messages)) } // Volatile blocks are injected at the tail (after history) for cache - // friendliness, with working memory still ordered before observational memory. - if !strings.Contains(messages[len(messages)-2].Content, "") { - t.Fatalf("second-to-last message = %q, want working-memory snippet", messages[len(messages)-2].Content) + // friendliness, with working memory still ordered before observational + // memory. The permissions notice (issue #1397) is appended after both, + // so it is now the last tail message on every turn including this one. + if !strings.Contains(messages[len(messages)-3].Content, "") { + t.Fatalf("third-to-last message = %q, want working-memory snippet", messages[len(messages)-3].Content) } - if !strings.Contains(messages[len(messages)-1].Content, "") { - t.Fatalf("last message = %q, want observational-memory snippet", messages[len(messages)-1].Content) + if !strings.Contains(messages[len(messages)-2].Content, "") { + t.Fatalf("second-to-last message = %q, want observational-memory snippet", messages[len(messages)-2].Content) + } + if !strings.Contains(messages[len(messages)-1].Content, "Permissions for this run") { + t.Fatalf("last message = %q, want permissions notice", messages[len(messages)-1].Content) } } diff --git a/internal/harness/tools/sandbox_darwin_test.go b/internal/harness/tools/sandbox_darwin_test.go new file mode 100644 index 000000000..341c8c086 --- /dev/null +++ b/internal/harness/tools/sandbox_darwin_test.go @@ -0,0 +1,80 @@ +//go:build darwin + +package tools + +import ( + "context" + "strings" + "testing" +) + +// TestSeatbeltProfileNetworkPolicy verifies that seatbeltProfile emits +// "(deny network*)" only for SandboxScopeWorkspace/SandboxScopeLocal, and +// only when the caller asks for it (issue #1397). Before this change the +// seatbelt profile always denied network regardless of caller intent. +func TestSeatbeltProfileNetworkPolicy(t *testing.T) { + t.Parallel() + + for _, scope := range []SandboxScope{SandboxScopeWorkspace, SandboxScopeLocal} { + profile := seatbeltProfile(scope, t.TempDir(), NetworkPolicyDeny) + if !strings.Contains(profile, "(deny network*)") { + t.Errorf("scope %q, network=deny: expected profile to contain \"(deny network*)\", got:\n%s", scope, profile) + } + + profile = seatbeltProfile(scope, t.TempDir(), NetworkPolicyAllow) + if strings.Contains(profile, "(deny network*)") { + t.Errorf("scope %q, network=allow: expected profile to omit \"(deny network*)\", got:\n%s", scope, profile) + } + } +} + +// TestBuildSandboxedCommandDarwinNetworkPolicyFromContext verifies that +// buildSandboxedCommand reads the network policy from ctx (issue #1397) and +// reports the applied policy on SandboxExecResult so bash tool output can +// surface it. +func TestBuildSandboxedCommandDarwinNetworkPolicyFromContext(t *testing.T) { + t.Parallel() + + workspace := t.TempDir() + + ctx := WithNetworkPolicy(context.Background(), NetworkPolicyDeny) + _, cleanup, res, err := buildSandboxedCommand(ctx, SandboxScopeWorkspace, workspace, "echo hi") + if err != nil { + t.Fatalf("buildSandboxedCommand: %v", err) + } + defer cleanup() + if res.NetworkPolicy != NetworkPolicyDeny { + t.Errorf("expected SandboxExecResult.NetworkPolicy=%q, got %q", NetworkPolicyDeny, res.NetworkPolicy) + } + + ctx = WithNetworkPolicy(context.Background(), NetworkPolicyAllow) + _, cleanup2, res2, err := buildSandboxedCommand(ctx, SandboxScopeWorkspace, workspace, "echo hi") + if err != nil { + t.Fatalf("buildSandboxedCommand: %v", err) + } + defer cleanup2() + if res2.NetworkPolicy != NetworkPolicyAllow { + t.Errorf("expected SandboxExecResult.NetworkPolicy=%q, got %q", NetworkPolicyAllow, res2.NetworkPolicy) + } + + // No network policy on the context at all must default to allow. + _, cleanup3, res3, err := buildSandboxedCommand(context.Background(), SandboxScopeWorkspace, workspace, "echo hi") + if err != nil { + t.Fatalf("buildSandboxedCommand: %v", err) + } + defer cleanup3() + if res3.NetworkPolicy != NetworkPolicyAllow { + t.Errorf("expected default SandboxExecResult.NetworkPolicy=%q, got %q", NetworkPolicyAllow, res3.NetworkPolicy) + } + + // Unrestricted scope is untouched by the network policy: no seatbelt + // profile is generated at all. + _, cleanup4, res4, err := buildSandboxedCommand(context.Background(), SandboxScopeUnrestricted, workspace, "echo hi") + if err != nil { + t.Fatalf("buildSandboxedCommand: %v", err) + } + defer cleanup4() + if res4.Mechanism != "none" { + t.Errorf("expected unrestricted scope to skip sandboxing, got mechanism %q", res4.Mechanism) + } +} diff --git a/internal/harness/tools/sandbox_linux_test.go b/internal/harness/tools/sandbox_linux_test.go index c02e21316..378040646 100644 --- a/internal/harness/tools/sandbox_linux_test.go +++ b/internal/harness/tools/sandbox_linux_test.go @@ -13,30 +13,56 @@ import ( "testing" ) -// TestBuildSandboxedCommandLinuxIsolatesPIDAndIPC guards #785 at the -// argument level: the bwrap invocation must unshare the PID and IPC -// namespaces (and start a new session) in addition to the pre-existing -// network unshare, for both confinement scopes. Without --unshare-pid a -// sandboxed process can signal every same-UID host process and read host -// /proc//environ (API keys); darwin's seatbelt already restricts -// signals to self. -func TestBuildSandboxedCommandLinuxIsolatesPIDAndIPC(t *testing.T) { - // Not parallel: this test rewrites the process-global PATH via - // t.Setenv, which the testing package forbids in parallel tests. - // - // A fake bwrap on PATH is sufficient: the test only inspects the - // assembled argv, it never executes it. +// bwrapArgsBeforeDoubleDash returns the bwrap argv up to (not including) the +// "--" separator, so tests can inspect which flags were assembled without +// tripping over the wrapped command itself. +func bwrapArgsBeforeDoubleDash(cmd *exec.Cmd) []string { + var args []string + for _, a := range cmd.Args { + if a == "--" { + break + } + args = append(args, a) + } + return args +} + +func containsArg(args []string, want string) bool { + for _, a := range args { + if a == want { + return true + } + } + return false +} + +// fakeBwrapOnPath installs a no-op bwrap binary on PATH so tests can inspect +// the assembled argv without actually creating a namespace sandbox. Not safe +// to use from a parallel subtest: t.Setenv forbids it. +func fakeBwrapOnPath(t *testing.T) { + t.Helper() dir := t.TempDir() fakeBwrap := filepath.Join(dir, "bwrap") if err := os.WriteFile(fakeBwrap, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { t.Fatal(err) } t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +// TestBuildSandboxedCommandLinuxIsolatesPIDAndIPC guards #785 at the +// argument level: the bwrap invocation must unshare the PID and IPC +// namespaces (and start a new session), for both confinement scopes, +// regardless of network policy. Without --unshare-pid a sandboxed process +// can signal every same-UID host process and read host /proc//environ +// (API keys); darwin's seatbelt already restricts signals to self. +func TestBuildSandboxedCommandLinuxIsolatesPIDAndIPC(t *testing.T) { + // Not parallel: fakeBwrapOnPath rewrites the process-global PATH via + // t.Setenv, which the testing package forbids in parallel tests. + fakeBwrapOnPath(t) for _, scope := range []SandboxScope{SandboxScopeWorkspace, SandboxScopeLocal} { scope := scope t.Run(string(scope), func(t *testing.T) { - t.Parallel() cmd, cleanup, res, err := buildSandboxedCommand(context.Background(), scope, t.TempDir(), "echo hi") if err != nil { t.Fatalf("buildSandboxedCommand: %v", err) @@ -46,22 +72,9 @@ func TestBuildSandboxedCommandLinuxIsolatesPIDAndIPC(t *testing.T) { t.Fatalf("expected sandbox to be applied, got %+v", res) } - var bwrapArgs []string - for _, a := range cmd.Args { - if a == "--" { - break - } - bwrapArgs = append(bwrapArgs, a) - } - for _, want := range []string{"--unshare-pid", "--unshare-ipc", "--new-session", "--unshare-net", "--die-with-parent"} { - found := false - for _, a := range bwrapArgs { - if a == want { - found = true - break - } - } - if !found { + bwrapArgs := bwrapArgsBeforeDoubleDash(cmd) + for _, want := range []string{"--unshare-pid", "--unshare-ipc", "--new-session", "--die-with-parent"} { + if !containsArg(bwrapArgs, want) { t.Errorf("expected %q in bwrap args before \"--\", got: %s", want, strings.Join(bwrapArgs, " ")) } } @@ -69,6 +82,61 @@ func TestBuildSandboxedCommandLinuxIsolatesPIDAndIPC(t *testing.T) { } } +// TestBuildSandboxedCommandLinuxNetworkPolicy verifies that --unshare-net is +// added only when the network policy read from ctx is deny (issue #1397). +// Before this change bwrap always unshared the network namespace regardless +// of caller intent. +func TestBuildSandboxedCommandLinuxNetworkPolicy(t *testing.T) { + // Not parallel: fakeBwrapOnPath rewrites the process-global PATH via + // t.Setenv, which the testing package forbids in parallel tests. + fakeBwrapOnPath(t) + + for _, scope := range []SandboxScope{SandboxScopeWorkspace, SandboxScopeLocal} { + scope := scope + t.Run(string(scope)+"/deny", func(t *testing.T) { + ctx := WithNetworkPolicy(context.Background(), NetworkPolicyDeny) + cmd, cleanup, res, err := buildSandboxedCommand(ctx, scope, t.TempDir(), "echo hi") + if err != nil { + t.Fatalf("buildSandboxedCommand: %v", err) + } + defer cleanup() + if !containsArg(bwrapArgsBeforeDoubleDash(cmd), "--unshare-net") { + t.Errorf("expected --unshare-net in bwrap args when network=deny, got: %s", strings.Join(bwrapArgsBeforeDoubleDash(cmd), " ")) + } + if res.NetworkPolicy != NetworkPolicyDeny { + t.Errorf("expected SandboxExecResult.NetworkPolicy=%q, got %q", NetworkPolicyDeny, res.NetworkPolicy) + } + }) + t.Run(string(scope)+"/allow", func(t *testing.T) { + ctx := WithNetworkPolicy(context.Background(), NetworkPolicyAllow) + cmd, cleanup, res, err := buildSandboxedCommand(ctx, scope, t.TempDir(), "echo hi") + if err != nil { + t.Fatalf("buildSandboxedCommand: %v", err) + } + defer cleanup() + if containsArg(bwrapArgsBeforeDoubleDash(cmd), "--unshare-net") { + t.Errorf("expected no --unshare-net in bwrap args when network=allow, got: %s", strings.Join(bwrapArgsBeforeDoubleDash(cmd), " ")) + } + if res.NetworkPolicy != NetworkPolicyAllow { + t.Errorf("expected SandboxExecResult.NetworkPolicy=%q, got %q", NetworkPolicyAllow, res.NetworkPolicy) + } + }) + t.Run(string(scope)+"/default_is_allow", func(t *testing.T) { + cmd, cleanup, res, err := buildSandboxedCommand(context.Background(), scope, t.TempDir(), "echo hi") + if err != nil { + t.Fatalf("buildSandboxedCommand: %v", err) + } + defer cleanup() + if containsArg(bwrapArgsBeforeDoubleDash(cmd), "--unshare-net") { + t.Errorf("expected no --unshare-net in bwrap args by default, got: %s", strings.Join(bwrapArgsBeforeDoubleDash(cmd), " ")) + } + if res.NetworkPolicy != NetworkPolicyAllow { + t.Errorf("expected default SandboxExecResult.NetworkPolicy=%q, got %q", NetworkPolicyAllow, res.NetworkPolicy) + } + }) + } +} + // 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 cdd5a0bf6..d54be9587 100644 --- a/internal/harness/tools/sandbox_test.go +++ b/internal/harness/tools/sandbox_test.go @@ -43,37 +43,48 @@ func TestCheckSandboxCommandUnrestricted(t *testing.T) { "cd /etc && cat passwd", } for _, cmd := range commands { - if err := CheckSandboxCommand(SandboxScopeUnrestricted, workspace, cmd); err != nil { + if err := CheckSandboxCommand(SandboxScopeUnrestricted, NetworkPolicyDeny, workspace, cmd); err != nil { t.Errorf("unrestricted scope: unexpected error for command %q: %v", cmd, err) } } // Empty scope is also unrestricted. for _, cmd := range commands { - if err := CheckSandboxCommand("", workspace, cmd); err != nil { + if err := CheckSandboxCommand("", NetworkPolicyDeny, workspace, cmd); err != nil { t.Errorf("empty scope: unexpected error for command %q: %v", cmd, err) } } } +// TestCheckSandboxCommandLocalScope verifies that SandboxScopeLocal's +// network heuristic is gated by NetworkPolicy (issue #1397): explicit deny +// still blocks curl/wget/nc/netcat/telnet, but the new default (allow, both +// as an explicit value and as the empty zero value) does not. func TestCheckSandboxCommandLocalScope(t *testing.T) { t.Parallel() workspace := t.TempDir() - blocked := []string{ + networkCommands := []string{ "curl https://example.com", "wget http://example.com", "nc -l 1234", "netcat example.com 80", "telnet example.com", } - for _, cmd := range blocked { - if err := CheckSandboxCommand(SandboxScopeLocal, workspace, cmd); err == nil { - t.Errorf("local scope: expected error for command %q, got nil", cmd) + for _, cmd := range networkCommands { + if err := CheckSandboxCommand(SandboxScopeLocal, NetworkPolicyDeny, workspace, cmd); err == nil { + t.Errorf("local scope, network=deny: expected error for command %q, got nil", cmd) + } + } + for _, network := range []NetworkPolicy{NetworkPolicyAllow, ""} { + for _, cmd := range networkCommands { + if err := CheckSandboxCommand(SandboxScopeLocal, network, workspace, cmd); err != nil { + t.Errorf("local scope, network=%q: unexpected error for command %q: %v", network, cmd, err) + } } } - // Local scope allows filesystem operations. + // Local scope allows filesystem operations regardless of network policy. allowed := []string{ "ls /tmp", "cat /etc/hosts", @@ -81,7 +92,7 @@ func TestCheckSandboxCommandLocalScope(t *testing.T) { "go test ./...", } for _, cmd := range allowed { - if err := CheckSandboxCommand(SandboxScopeLocal, workspace, cmd); err != nil { + if err := CheckSandboxCommand(SandboxScopeLocal, NetworkPolicyDeny, workspace, cmd); err != nil { t.Errorf("local scope: unexpected error for command %q: %v", cmd, err) } } @@ -101,14 +112,14 @@ func TestCheckSandboxCommandWorkspaceScope(t *testing.T) { "rm /var/log/messages", } for _, cmd := range outsideAbsPaths { - if err := CheckSandboxCommand(SandboxScopeWorkspace, absWorkspace, cmd); err == nil { + if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, cmd); err == nil { t.Errorf("workspace scope: expected error for command %q with outside absolute path, got nil", cmd) } } // Commands entirely within the workspace should be allowed. insideCmd := "ls " + absWorkspace - if err := CheckSandboxCommand(SandboxScopeWorkspace, absWorkspace, insideCmd); err != nil { + if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, insideCmd); err != nil { t.Errorf("workspace scope: unexpected error for in-workspace command %q: %v", insideCmd, err) } @@ -119,7 +130,7 @@ func TestCheckSandboxCommandWorkspaceScope(t *testing.T) { "cd ../ ", } for _, cmd := range cdEscape { - if err := CheckSandboxCommand(SandboxScopeWorkspace, absWorkspace, cmd); err == nil { + if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, cmd); err == nil { t.Errorf("workspace scope: expected error for cd-escape command %q, got nil", cmd) } } @@ -132,7 +143,7 @@ func TestCheckSandboxCommandWorkspaceScope(t *testing.T) { "cat notes.txt", } for _, cmd := range safeCommands { - if err := CheckSandboxCommand(SandboxScopeWorkspace, absWorkspace, cmd); err != nil { + if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, cmd); err != nil { t.Errorf("workspace scope: unexpected error for safe command %q: %v", cmd, err) } } @@ -149,14 +160,14 @@ func TestSandboxWorkspaceScopeEnforcesFilePaths(t *testing.T) { // Writing to a path outside the workspace via absolute path should be blocked. outsideFile := filepath.Join(filepath.Dir(absWorkspace), "outside.txt") cmd := "echo secret > " + outsideFile - if err := CheckSandboxCommand(SandboxScopeWorkspace, absWorkspace, cmd); err == nil { + if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, cmd); err == nil { t.Errorf("workspace scope: expected error for write to %q, got nil", outsideFile) } // Writing inside the workspace is fine. insideFile := filepath.Join(absWorkspace, "inside.txt") cmd2 := "echo hello > " + insideFile - if err := CheckSandboxCommand(SandboxScopeWorkspace, absWorkspace, cmd2); err != nil { + if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, cmd2); err != nil { t.Errorf("workspace scope: unexpected error for write to %q: %v", insideFile, err) } } @@ -166,7 +177,7 @@ func TestCheckSandboxCommandUnknownScope(t *testing.T) { t.Parallel() workspace := t.TempDir() - if err := CheckSandboxCommand("badscope", workspace, "echo hi"); err == nil { + if err := CheckSandboxCommand("badscope", NetworkPolicyAllow, workspace, "echo hi"); err == nil { t.Error("expected error for unknown sandbox scope, got nil") } } @@ -201,7 +212,7 @@ func TestJobManagerSandboxScopeWorkspace(t *testing.T) { } // TestJobManagerSandboxScopeLocal verifies that network commands are blocked -// under SandboxScopeLocal. +// under SandboxScopeLocal when the network policy is explicitly deny. func TestJobManagerSandboxScopeLocal(t *testing.T) { t.Parallel() @@ -212,7 +223,7 @@ func TestJobManagerSandboxScopeLocal(t *testing.T) { mgr := NewJobManager(workspace, nil) mgr.SetSandboxScope(SandboxScopeLocal) - ctx := context.Background() + ctx := WithNetworkPolicy(context.Background(), NetworkPolicyDeny) // curl should be blocked. _, err := mgr.RunForeground(ctx, "curl https://example.com", 5, "") @@ -230,6 +241,25 @@ func TestJobManagerSandboxScopeLocal(t *testing.T) { } } +// TestJobManagerSandboxScopeLocalAllowsNetworkByDefault verifies the default +// behavior change from issue #1397: with no network policy configured on +// either the JobManager or the context, SandboxScopeLocal no longer rejects +// curl before it runs (the pre-execution heuristic check must not fire). +func TestJobManagerSandboxScopeLocalAllowsNetworkByDefault(t *testing.T) { + t.Parallel() + + workspace, _ := os.MkdirTemp("", "sandbox-test") + defer os.RemoveAll(workspace) + + mgr := NewJobManager(workspace, nil) + mgr.SetSandboxScope(SandboxScopeLocal) + + _, err := mgr.RunForeground(context.Background(), "curl --version", 5, "") + if err != nil { + t.Errorf("expected curl not to be rejected under default (allow) network policy, got error: %v", err) + } +} + func TestJobManagerContextSandboxScopeOverridesDefault(t *testing.T) { t.Parallel() @@ -262,9 +292,10 @@ func TestJobManagerContextSandboxScopeBlocksBackgroundCommand(t *testing.T) { mgr.SetSandboxScope(SandboxScopeUnrestricted) ctx := WithSandboxScope(context.Background(), SandboxScopeLocal) + ctx = WithNetworkPolicy(ctx, NetworkPolicyDeny) if _, err := mgr.RunBackgroundWithContext(ctx, "curl https://example.com", 5, ""); err == nil { - t.Fatal("expected local sandbox override to block background network command") + t.Fatal("expected local sandbox override with network=deny to block background network command") } } @@ -299,7 +330,7 @@ func TestSandboxWorkspaceScopeBlocksWriteOutsideWorkspaceAtOSLevel(t *testing.T) // The heuristic layer must NOT catch this obfuscated escape — that is // what makes this a proof of OS-level enforcement rather than a // duplicate of the existing string-matching tests above. - if err := CheckSandboxCommand(SandboxScopeWorkspace, absWorkspace, command); err != nil { + if err := CheckSandboxCommand(SandboxScopeWorkspace, NetworkPolicyAllow, absWorkspace, command); err != nil { t.Fatalf("expected heuristic to miss the obfuscated escape (so the OS layer is what's under test), got error: %v", err) } @@ -314,12 +345,13 @@ func TestSandboxWorkspaceScopeBlocksWriteOutsideWorkspaceAtOSLevel(t *testing.T) } } -// TestSandboxLocalScopeBlocksObfuscatedNetworkAtOSLevel proves that -// local-scope network denial is enforced by the OS, not by regex matching -// against the command string: "curl" is assembled from two shell variables -// so the literal substring "curl" never appears in the command, defeating -// the \bcurl\b pattern in checkLocalScopeCommand. The request must still -// fail because the OS layer denies network operations outright. +// TestSandboxLocalScopeBlocksObfuscatedNetworkAtOSLevel proves that, when the +// network policy is deny, local-scope network denial is enforced by the OS, +// not by regex matching against the command string: "curl" is assembled from +// two shell variables so the literal substring "curl" never appears in the +// command, defeating the \bcurl\b pattern in checkLocalScopeCommand. The +// request must still fail because the OS layer denies network operations +// outright. func TestSandboxLocalScopeBlocksObfuscatedNetworkAtOSLevel(t *testing.T) { if !osSandboxAvailable(t) { t.Skip("no OS-level sandbox mechanism (seatbelt/bubblewrap) available on this host") @@ -328,14 +360,15 @@ func TestSandboxLocalScopeBlocksObfuscatedNetworkAtOSLevel(t *testing.T) { workspace := t.TempDir() mgr := NewJobManager(workspace, nil) mgr.SetSandboxScope(SandboxScopeLocal) + ctx := WithNetworkPolicy(context.Background(), NetworkPolicyDeny) command := `A=cur; B=l; "$A$B" -s -m 5 https://example.com -o /dev/null -w '%{http_code}'` - if err := CheckSandboxCommand(SandboxScopeLocal, workspace, command); err != nil { + if err := CheckSandboxCommand(SandboxScopeLocal, NetworkPolicyDeny, workspace, command); err != nil { t.Fatalf("expected heuristic to miss the obfuscated network command (so the OS layer is what's under test), got error: %v", err) } - result, err := mgr.RunForeground(context.Background(), command, 10, "") + result, err := mgr.RunForeground(ctx, command, 10, "") if err != nil { // A hard exec failure also demonstrates the network call never // succeeded; only a clean success (exit 0) would be a problem. @@ -373,3 +406,51 @@ func TestResolveSandboxUnavailableFailsClosedWhenStrict(t *testing.T) { t.Fatal("expected an error when strict mode is enabled and the mechanism is unavailable") } } + +// TestSandboxWorkspaceScopeNetworkPolicyLiveCurl proves end-to-end, with a +// real network request, that workspace-scope network confinement follows +// PermissionConfig.Network (issue #1397): curl to a real host fails under +// network=deny and succeeds under network=allow. Skipped when no OS-level +// sandbox mechanism (seatbelt/bubblewrap) or no curl binary is available. +// The allow-case is skipped rather than failed when the host itself has no +// route to the internet, since that is an environment limitation, not a +// sandbox defect. +func TestSandboxWorkspaceScopeNetworkPolicyLiveCurl(t *testing.T) { + if !osSandboxAvailable(t) { + t.Skip("no OS-level sandbox mechanism (seatbelt/bubblewrap) available on this host") + } + if _, err := exec.LookPath("curl"); err != nil { + t.Skip("curl not available on this host") + } + + workspace := t.TempDir() + mgr := NewJobManager(workspace, nil) + mgr.SetSandboxScope(SandboxScopeWorkspace) + + const command = `curl -sI -m 10 https://proxy.golang.org` + + t.Run("deny", func(t *testing.T) { + t.Parallel() + ctx := WithNetworkPolicy(context.Background(), NetworkPolicyDeny) + result, _ := mgr.RunForeground(ctx, command, 15, "") + if result == nil { + t.Fatal("expected non-nil result") + } + if exitCode, _ := result["exit_code"].(int); exitCode == 0 { + t.Errorf("expected curl to fail under network=deny, got exit_code 0; result=%v", result) + } + }) + + t.Run("allow", func(t *testing.T) { + t.Parallel() + ctx := WithNetworkPolicy(context.Background(), NetworkPolicyAllow) + result, err := mgr.RunForeground(ctx, command, 15, "") + if err != nil { + t.Skipf("curl exec failed under network=allow (host likely has no route to the internet): %v", err) + } + exitCode, _ := result["exit_code"].(int) + if exitCode != 0 { + t.Skipf("curl exited %d under network=allow (host likely has no route to the internet); result=%v", exitCode, result) + } + }) +} From cdbc73dcd6f6724082d993d72cfb76f29d513a70 Mon Sep 17 00:00:00 2001 From: Dennison Date: Sun, 6 Sep 2026 09:55:40 -0400 Subject: [PATCH 2/4] feat: TASK-1397 make bash sandbox network policy configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implementation for tests added in fdf22b52. Summary (issue #1397): workspace/local sandbox scopes now allow outbound network by default. A new PermissionConfig.Network axis ("" / "allow" / "deny") lets a caller explicitly deny it; the bash sandbox enforces the resolved policy at the OS level (seatbelt on darwin, bubblewrap on linux), the model is told the policy on every turn including the first, and the CLI can set it via flags. - internal/harness/types.go: NetworkPolicy type + PermissionConfig.Network field (json "network,omitempty"); ValidatePermissionConfig rejects anything other than "", "allow", "deny"; DefaultPermissionConfig sets Network: NetworkPolicyAllow explicitly. - internal/harness/tools/types.go: mirrored tools.NetworkPolicy type, ContextKeyNetworkPolicy, WithNetworkPolicy, NetworkPolicyFromContext — same import-cycle-avoidance pattern as SandboxScope. - internal/harness/tools/sandbox.go: SandboxExecResult gains a NetworkPolicy field so bash tool output reports the applied policy; CheckSandboxCommand takes a NetworkPolicy parameter and only applies the SandboxScopeLocal curl/wget/nc/netcat/telnet heuristic when it is deny. - internal/harness/tools/sandbox_darwin.go: seatbeltProfile takes the network policy and emits an explicit "(allow network*)" for allow/default or "(deny network*)" for deny — under seatbelt's "(deny default)", merely omitting a deny rule still leaves network denied, so the allow case needs its own explicit rule. Confirmed with a real curl against https://proxy.golang.org via /usr/bin/sandbox-exec: exit 0 (HTTP 200) under allow, exit 6 (could not resolve host) under deny. - internal/harness/tools/sandbox_linux.go: --unshare-net is only added to the bwrap invocation when network is deny. - internal/harness/tools/sandbox_other.go: reports the resolved NetworkPolicy on SandboxExecResult for parity on unimplemented platforms; behavior otherwise unchanged (degrades to heuristic/fail-closed as before). - internal/harness/tools/bash_manager.go: JobManager gets SetNetworkPolicy/networkPolicyForContext (mirroring SandboxScope), wired into runForeground/runBackground; bash tool results gain a "sandbox_network" field. - internal/harness/runner.go: runStepEngine/newStepEngine/stepEngine thread the resolved tools.NetworkPolicy alongside SandboxScope from effectivePermissions down to the per-tool-call context (htools.WithNetworkPolicy); normalizePermissionConfig defaults an empty Network to allow; permissionsNoticeLines() renders "Permissions for this run: sandbox=%s, approval=%s, network=%s." plus, when denied, "Outbound network is blocked for this run: dependency installs will fail; report the blocker instead of substituting a different design." — reused by both the existing continuation-changed notice and the new per-turn notice below. - internal/harness/runner_step_engine.go / clone.go: buildTurnMessages gains a permissionsNotice tail parameter, computed once from the run's effective sandbox/approval/network and re-appended to every turn's wire message list (never persisted into conversation history, unlike the continuation-changed notice), so the model is told its network policy starting on turn one — not only when a continuation's permissions change. - cmd/harnesscli/main.go: --sandbox (workspace|local|unrestricted) and --network (allow|deny) flags populate runCreateRequest.Permissions (*harness.PermissionConfig, json "permissions,omitempty") only when either flag is set; both flags unset omits "permissions" entirely so the server's own defaults apply. Existing tests were updated where they asserted the previous always-deny default for SandboxScopeLocal, or fixed-position tail-message assumptions that the new per-turn permissions notice message shifts by one (working memory / observational memory / trusted-origin system-prompt capture tests) — see the red commit for the full list and reasoning. go vet output (internal/harness, internal/harness/tools, cmd/harnesscli): clean, no output. go test -race output (internal/harness/..., cmd/harnesscli): ok go-agent-harness/internal/harness 8.362s ok go-agent-harness/internal/harness/tools 18.140s ok go-agent-harness/internal/harness/tools/core (cached) ok go-agent-harness/internal/harness/tools/deferred (cached) ok go-agent-harness/internal/harness/tools/descriptions (cached) ok go-agent-harness/internal/harness/tools/recipe (cached) ok go-agent-harness/internal/harness/tools/script (cached) ok go-agent-harness/cmd/harnesscli 6.861s Behavioral tests covered: PermissionConfig.Network validation, darwin seatbelt/linux bwrap network gating, first-turn permissions notice (allow and deny), CLI --sandbox/--network flags. Files changed: cmd/harnesscli/main.go, internal/harness/clone.go, internal/harness/runner.go, internal/harness/runner_step_engine.go, internal/harness/tools/bash_manager.go, internal/harness/tools/sandbox.go, internal/harness/tools/sandbox_darwin.go, internal/harness/tools/sandbox_linux.go, internal/harness/tools/sandbox_other.go, internal/harness/tools/types.go, internal/harness/types.go Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5 --- cmd/harnesscli/main.go | 33 ++++++++++++++------ internal/harness/clone.go | 21 ++++++++----- internal/harness/runner.go | 31 ++++++++++++++++--- internal/harness/runner_step_engine.go | 20 ++++++++++-- internal/harness/tools/bash_manager.go | 38 ++++++++++++++++++++--- internal/harness/tools/sandbox.go | 19 ++++++++++-- internal/harness/tools/sandbox_darwin.go | 38 +++++++++++++++++------ internal/harness/tools/sandbox_linux.go | 16 ++++++++-- internal/harness/tools/sandbox_other.go | 7 ++++- internal/harness/tools/types.go | 39 ++++++++++++++++++++++++ internal/harness/types.go | 33 +++++++++++++++++++- 11 files changed, 252 insertions(+), 43 deletions(-) diff --git a/cmd/harnesscli/main.go b/cmd/harnesscli/main.go index 7f373d4fd..34882b392 100644 --- a/cmd/harnesscli/main.go +++ b/cmd/harnesscli/main.go @@ -81,15 +81,16 @@ var ( ) type runCreateRequest struct { - Prompt string `json:"prompt"` - Model string `json:"model,omitempty"` - SystemPrompt string `json:"system_prompt,omitempty"` - AgentIntent string `json:"agent_intent,omitempty"` - TaskContext string `json:"task_context,omitempty"` - PromptProfile string `json:"prompt_profile,omitempty"` - PromptExtensions *runCreatePromptSettings `json:"prompt_extensions,omitempty"` - WorkspacePath string `json:"workspace_path,omitempty"` - PlanMode bool `json:"plan_mode,omitempty"` + Prompt string `json:"prompt"` + Model string `json:"model,omitempty"` + SystemPrompt string `json:"system_prompt,omitempty"` + AgentIntent string `json:"agent_intent,omitempty"` + TaskContext string `json:"task_context,omitempty"` + PromptProfile string `json:"prompt_profile,omitempty"` + PromptExtensions *runCreatePromptSettings `json:"prompt_extensions,omitempty"` + WorkspacePath string `json:"workspace_path,omitempty"` + PlanMode bool `json:"plan_mode,omitempty"` + Permissions *harness.PermissionConfig `json:"permissions,omitempty"` } type runCreatePromptSettings struct { @@ -153,6 +154,8 @@ func run(args []string) int { promptCustom := flags.String("prompt-custom", "", "custom prompt extension text") workspace := flags.String("workspace", "", "workspace directory for this run (defaults to current working directory)") planMode := flags.Bool("plan-mode", false, "start the run in enforced read-only plan mode") + sandbox := flags.String("sandbox", "", "sandbox scope for this run: workspace, local, or unrestricted (default: server default)") + network := flags.String("network", "", "network policy for the bash sandbox: allow or deny (default: server default, currently allow)") enableTUI := flags.Bool("tui", false, "launch interactive BubbleTea TUI (experimental)") resume := flags.String("resume", "", "resume an existing conversation by ID in the TUI (implies --tui)") listProfiles := flags.Bool("list-profiles", false, "list available profiles and exit") @@ -194,6 +197,17 @@ func run(args []string) int { } } + // permissions is populated only when the caller sets --sandbox and/or + // --network; leaving both flags unset omits "permissions" from the + // request body entirely, so the server's own defaults apply (issue #1397). + var permissions *harness.PermissionConfig + if strings.TrimSpace(*sandbox) != "" || strings.TrimSpace(*network) != "" { + permissions = &harness.PermissionConfig{ + Sandbox: harness.SandboxScope(*sandbox), + Network: harness.NetworkPolicy(*network), + } + } + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() @@ -207,6 +221,7 @@ func run(args []string) int { PromptExtensions: extensions, WorkspacePath: workspacePath, PlanMode: *planMode, + Permissions: permissions, }) if err != nil { fmt.Fprintf(stderr, "harnesscli: start run: %v\n", err) diff --git a/internal/harness/clone.go b/internal/harness/clone.go index c14473dcb..eaef26b10 100644 --- a/internal/harness/clone.go +++ b/internal/harness/clone.go @@ -82,17 +82,24 @@ func copyMessages(msgs []Message) []Message { // conversation history — so the provider-visible prefix (tools + system + // history) only ever grows by appending and stays a valid cached prefix across // steps. All volatile per-step content (working memory, observational memory, -// dynamic rules, plan-mode guidance, and the runtime context, which carries the -// step number, token and cost totals, and a timestamp) is placed at the tail, -// after the history, where it can change every step without invalidating the -// cached prefix. Empty snippets are skipped. -func (r *Runner) buildTurnMessages(systemPrompt string, messages []Message, workingMemory, observationalMemory, ruleContent, planModeGuidance, runtimeContext string) []Message { - tm := make([]Message, 0, len(messages)+6) +// dynamic rules, plan-mode guidance, the permissions notice, and the runtime +// context, which carries the step number, token and cost totals, and a +// timestamp) is placed at the tail, after the history, where it can change +// every step without invalidating the cached prefix. Empty snippets are +// skipped. +// +// permissionsNotice reports the run's sandbox/approval/network policy +// (issue #1397). It is recomputed and re-appended every turn rather than +// persisted into conversation history, so it always reaches the model +// (including turn one) without inflating the stored transcript the way a +// history message would. +func (r *Runner) buildTurnMessages(systemPrompt string, messages []Message, workingMemory, observationalMemory, ruleContent, planModeGuidance, permissionsNotice, runtimeContext string) []Message { + tm := make([]Message, 0, len(messages)+7) if systemPrompt != "" { tm = append(tm, Message{Role: "system", Content: systemPrompt}) } tm = append(tm, copyMessages(messages)...) - for _, tail := range []string{workingMemory, observationalMemory, ruleContent, planModeGuidance, runtimeContext} { + for _, tail := range []string{workingMemory, observationalMemory, ruleContent, planModeGuidance, permissionsNotice, runtimeContext} { if strings.TrimSpace(tail) != "" { tm = append(tm, Message{Role: "system", Content: tail}) } diff --git a/internal/harness/runner.go b/internal/harness/runner.go index ad9c61a45..35567567a 100644 --- a/internal/harness/runner.go +++ b/internal/harness/runner.go @@ -2112,8 +2112,8 @@ func (r *Runner) runPreflight(ctx context.Context, runID string, req RunRequest) }, nil } -func (r *Runner) runStepEngine(ctx context.Context, runID string, req RunRequest, preflight *runPreflightResult, effectiveMaxSteps int, effectiveMaxTurns int, runForkDepth int, effectiveApprovalPolicy ApprovalPolicy, effectiveSandboxScope htools.SandboxScope) { - newStepEngine(r, ctx, runID, req, preflight, effectiveMaxSteps, effectiveMaxTurns, runForkDepth, effectiveApprovalPolicy, effectiveSandboxScope).run() +func (r *Runner) runStepEngine(ctx context.Context, runID string, req RunRequest, preflight *runPreflightResult, effectiveMaxSteps int, effectiveMaxTurns int, runForkDepth int, effectiveApprovalPolicy ApprovalPolicy, effectiveSandboxScope htools.SandboxScope, effectiveNetworkPolicy htools.NetworkPolicy) { + newStepEngine(r, ctx, runID, req, preflight, effectiveMaxSteps, effectiveMaxTurns, runForkDepth, effectiveApprovalPolicy, effectiveSandboxScope, effectiveNetworkPolicy).run() } func mapPromptExtensions(input *PromptExtensions) systemprompt.Extensions { @@ -3447,7 +3447,7 @@ func (r *Runner) execute(runID string, req RunRequest) { // Captured once from req to avoid repeated lock acquisitions in the step loop. runForkDepth := req.ForkDepth - r.runStepEngine(ctx, runID, req, preflight, effectiveMaxSteps, effectiveMaxTurns, runForkDepth, effectiveApprovalPolicy, htools.SandboxScope(effectivePermissions.Sandbox)) + r.runStepEngine(ctx, runID, req, preflight, effectiveMaxSteps, effectiveMaxTurns, runForkDepth, effectiveApprovalPolicy, htools.SandboxScope(effectivePermissions.Sandbox), htools.NetworkPolicy(effectivePermissions.Network)) return } @@ -7424,6 +7424,11 @@ func normalizePermissionConfig(p PermissionConfig) PermissionConfig { if p.Approval == "" { p.Approval = ApprovalPolicyNone } + if p.Network == "" { + // Default: outbound network is allowed (issue #1397). See + // DefaultPermissionConfig in types.go. + p.Network = NetworkPolicyAllow + } p.Rules = copyPermissionRuleSet(p.Rules) return p } @@ -7448,11 +7453,29 @@ func buildContinuationPolicyNotice(srcAllowed, currentAllowed []string, srcPerms } } if permsChanged { - lines = append(lines, fmt.Sprintf("Permissions for this run: sandbox=%s, approval=%s.", currentPerms.Sandbox, currentPerms.Approval)) + lines = append(lines, permissionsNoticeLines(currentPerms)...) } return strings.Join(lines, "\n") } +// 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). +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 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.") + } + return lines +} + func stringSlicesEqual(a, b []string) bool { if len(a) != len(b) { return false diff --git a/internal/harness/runner_step_engine.go b/internal/harness/runner_step_engine.go index 4551fa7ca..8f5c3a419 100644 --- a/internal/harness/runner_step_engine.go +++ b/internal/harness/runner_step_engine.go @@ -67,9 +67,10 @@ type stepEngine struct { runForkDepth int effectiveApprovalPolicy ApprovalPolicy effectiveSandboxScope htools.SandboxScope + effectiveNetworkPolicy htools.NetworkPolicy } -func newStepEngine(r *Runner, ctx context.Context, runID string, req RunRequest, preflight *runPreflightResult, effectiveMaxSteps int, effectiveMaxTurns int, runForkDepth int, effectiveApprovalPolicy ApprovalPolicy, effectiveSandboxScope htools.SandboxScope) *stepEngine { +func newStepEngine(r *Runner, ctx context.Context, runID string, req RunRequest, preflight *runPreflightResult, effectiveMaxSteps int, effectiveMaxTurns int, runForkDepth int, effectiveApprovalPolicy ApprovalPolicy, effectiveSandboxScope htools.SandboxScope, effectiveNetworkPolicy htools.NetworkPolicy) *stepEngine { return &stepEngine{ runner: r, ctx: ctx, @@ -81,6 +82,7 @@ func newStepEngine(r *Runner, ctx context.Context, runID string, req RunRequest, runForkDepth: runForkDepth, effectiveApprovalPolicy: effectiveApprovalPolicy, effectiveSandboxScope: effectiveSandboxScope, + effectiveNetworkPolicy: effectiveNetworkPolicy, } } @@ -95,6 +97,7 @@ func (se *stepEngine) run() { runForkDepth := se.runForkDepth effectiveApprovalPolicy := se.effectiveApprovalPolicy effectiveSandboxScope := se.effectiveSandboxScope + effectiveNetworkPolicy := se.effectiveNetworkPolicy // rc is this run's config snapshot, captured at run creation. It is // immutable for the run's lifetime, so per-step reads stay stable even @@ -139,6 +142,16 @@ func (se *stepEngine) run() { } consecutiveEmptyResponses := 0 + // permissionsNotice reports this run's sandbox/approval/network policy to + // the model every turn, including the first (issue #1397). It is static + // for the run's lifetime — the three axes are fixed at run start — so it + // is computed once here rather than inside the per-step loop. + permissionsNotice := strings.Join(permissionsNoticeLines(PermissionConfig{ + Sandbox: SandboxScope(effectiveSandboxScope), + Approval: effectiveApprovalPolicy, + Network: NetworkPolicy(effectiveNetworkPolicy), + }), "\n") + emitCausalGraph := func(lastStep int) { if causalBuilder == nil { return @@ -295,7 +308,7 @@ func (se *stepEngine) run() { } } planModeGuidance := r.planModePromptBlock(runID) - turnMessages := r.buildTurnMessages(systemPrompt, messages, workingMemorySnippet, memorySnippetForSnapshot, injectedRuleContent.String(), planModeGuidance, runtimeContext) + turnMessages := r.buildTurnMessages(systemPrompt, messages, workingMemorySnippet, memorySnippetForSnapshot, injectedRuleContent.String(), planModeGuidance, permissionsNotice, runtimeContext) if rc.AutoCompactEnabled && rc.ModelContextWindow > 0 { estimated := 0 @@ -325,7 +338,7 @@ func (se *stepEngine) run() { } messages = compactedMsgs r.stepSetMessages(runID, messages) - turnMessages = r.buildTurnMessages(systemPrompt, messages, workingMemorySnippet, memorySnippetForSnapshot, injectedRuleContent.String(), planModeGuidance, runtimeContext) + turnMessages = r.buildTurnMessages(systemPrompt, messages, workingMemorySnippet, memorySnippetForSnapshot, injectedRuleContent.String(), planModeGuidance, permissionsNotice, runtimeContext) r.emit(runID, EventAutoCompactCompleted, map[string]any{ "before_tokens": estimated, "after_tokens": afterTokens, @@ -1271,6 +1284,7 @@ func (se *stepEngine) run() { toolCtx = htools.WithAskUserQuestionPendingNotifier(toolCtx, pendingNotifier) } toolCtx = htools.WithSandboxScope(toolCtx, effectiveSandboxScope) + toolCtx = htools.WithNetworkPolicy(toolCtx, effectiveNetworkPolicy) // Extra directory roots granted on the run request (TUI /add-dir) // ride the same per-call context so file-tool confinement permits // them in addition to the workspace root. diff --git a/internal/harness/tools/bash_manager.go b/internal/harness/tools/bash_manager.go index 4d12698d7..ad2a0c580 100644 --- a/internal/harness/tools/bash_manager.go +++ b/internal/harness/tools/bash_manager.go @@ -123,7 +123,8 @@ type JobManager struct { ttl time.Duration maxOutputBytes int now func() time.Time - sandboxScope SandboxScope // optional sandbox enforcement + sandboxScope SandboxScope // optional sandbox enforcement + networkPolicy NetworkPolicy // optional network policy fallback (issue #1397) events JobEvents } @@ -161,6 +162,13 @@ func (m *JobManager) SetSandboxScope(scope SandboxScope) { m.sandboxScope = scope } +// SetNetworkPolicy configures the fallback network policy enforced for +// commands run via this JobManager when the per-call context carries none +// (issue #1397). It is safe to call before any commands are launched. +func (m *JobManager) SetNetworkPolicy(policy NetworkPolicy) { + m.networkPolicy = policy +} + func (m *JobManager) runForeground(ctx context.Context, command string, timeoutSeconds int, workingDir string) (map[string]any, error) { if timeoutSeconds <= 0 { timeoutSeconds = 30 @@ -169,7 +177,8 @@ func (m *JobManager) runForeground(ctx context.Context, command string, timeoutS timeoutSeconds = 300 } scope := m.sandboxScopeForContext(ctx) - if err := CheckSandboxCommand(scope, m.root, command); err != nil { + network := m.networkPolicyForContext(ctx) + if err := CheckSandboxCommand(scope, network, m.root, command); err != nil { return nil, err } workDir, err := resolveWorkingDir(m.root, workingDir) @@ -177,7 +186,7 @@ func (m *JobManager) runForeground(ctx context.Context, command string, timeoutS return nil, err } - timeoutCtx, cancel := context.WithTimeout(ctx, time.Duration(timeoutSeconds)*time.Second) + timeoutCtx, cancel := context.WithTimeout(WithNetworkPolicy(ctx, network), time.Duration(timeoutSeconds)*time.Second) defer cancel() cmd, sandboxCleanup, sbResult, err := buildSandboxedCommand(timeoutCtx, scope, m.root, command) @@ -277,6 +286,9 @@ func (m *JobManager) runForeground(ctx context.Context, command string, timeoutS if sbResult.Warning != "" { result["sandbox_warning"] = sbResult.Warning } + if sbResult.NetworkPolicy != "" { + result["sandbox_network"] = string(sbResult.NetworkPolicy) + } return result, nil } @@ -288,13 +300,15 @@ func (m *JobManager) runBackground(ctx context.Context, command string, timeoutS timeoutSeconds = 3600 } scope := m.sandboxScopeForContext(ctx) - if err := CheckSandboxCommand(scope, m.root, command); err != nil { + network := m.networkPolicyForContext(ctx) + if err := CheckSandboxCommand(scope, network, m.root, command); err != nil { return nil, err } workDir, err := resolveWorkingDir(m.root, workingDir) if err != nil { return nil, err } + ctx = WithNetworkPolicy(ctx, network) m.cleanupExpired() @@ -431,6 +445,9 @@ func (m *JobManager) runBackground(ctx context.Context, command string, timeoutS if sbResult.Warning != "" { result["sandbox_warning"] = sbResult.Warning } + if sbResult.NetworkPolicy != "" { + result["sandbox_network"] = string(sbResult.NetworkPolicy) + } return result, nil } @@ -689,6 +706,19 @@ func (m *JobManager) sandboxScopeForContext(ctx context.Context) SandboxScope { return m.sandboxScope } +// networkPolicyForContext resolves the effective network policy: an explicit +// per-call context value wins, falling back to the JobManager-level default, +// and finally to NetworkPolicyAllow (issue #1397's safety-biased default). +func (m *JobManager) networkPolicyForContext(ctx context.Context) NetworkPolicy { + if policy, ok := NetworkPolicyFromContext(ctx); ok && policy != "" { + return policy + } + if m.networkPolicy != "" { + return m.networkPolicy + } + return NetworkPolicyAllow +} + func resolveWorkingDir(workspaceRoot, workingDir string) (string, error) { if strings.TrimSpace(workingDir) == "" { return filepath.Abs(workspaceRoot) diff --git a/internal/harness/tools/sandbox.go b/internal/harness/tools/sandbox.go index 2667c6b48..f3da3f21d 100644 --- a/internal/harness/tools/sandbox.go +++ b/internal/harness/tools/sandbox.go @@ -30,12 +30,21 @@ var networkRestrictedPatterns = []*regexp.Regexp{ // are blocked. // // For SandboxScopeUnrestricted (or empty), no additional checks are applied. -func CheckSandboxCommand(scope SandboxScope, workspaceRoot, command string) error { +// +// network gates the SandboxScopeLocal heuristic: it only rejects network +// commands when network is NetworkPolicyDeny. An empty network (or +// NetworkPolicyAllow) is the default and lets network commands proceed — +// the real enforcement for that case is simply the absence of any OS-level +// network restriction (see buildSandboxedCommand). +func CheckSandboxCommand(scope SandboxScope, network NetworkPolicy, workspaceRoot, command string) error { switch scope { case SandboxScopeWorkspace: return checkWorkspaceScopeCommand(workspaceRoot, command) case SandboxScopeLocal: - return checkLocalScopeCommand(command) + if network == NetworkPolicyDeny { + return checkLocalScopeCommand(command) + } + return nil case SandboxScopeUnrestricted, "": return nil default: @@ -128,6 +137,12 @@ type SandboxExecResult struct { // Warning is non-empty when confinement degraded to heuristic-only // enforcement and should be surfaced to the caller. Warning string + // NetworkPolicy is the network policy actually applied when building the + // command (issue #1397): "allow" or "deny". It is populated even for + // scopes/mechanisms that do not enforce it at the OS level, so bash tool + // output always reflects the effective policy rather than leaving the + // caller to infer it. + NetworkPolicy NetworkPolicy } // resolveSandboxUnavailable is called by the platform-specific diff --git a/internal/harness/tools/sandbox_darwin.go b/internal/harness/tools/sandbox_darwin.go index 7ace0fdf1..8338a1a71 100644 --- a/internal/harness/tools/sandbox_darwin.go +++ b/internal/harness/tools/sandbox_darwin.go @@ -20,17 +20,27 @@ const sandboxExecBinary = "/usr/bin/sandbox-exec" // appropriate for scope. The returned cleanup func must be called once the // command has finished running (Run/Wait returned) to remove the temporary // profile file. +// +// The network policy is read from ctx (NetworkPolicyFromContext), defaulting +// to NetworkPolicyAllow when absent, and only affects the seatbelt profile +// generated for SandboxScopeWorkspace/SandboxScopeLocal; SandboxScopeUnrestricted +// is unaffected (issue #1397). func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoot, command string) (*exec.Cmd, func(), SandboxExecResult, error) { noop := func() {} + network, _ := NetworkPolicyFromContext(ctx) + if network == "" { + network = NetworkPolicyAllow + } switch scope { case SandboxScopeUnrestricted, "": - return exec.CommandContext(ctx, "/bin/bash", "-lc", command), noop, SandboxExecResult{Applied: false, Mechanism: "none"}, nil + return exec.CommandContext(ctx, "/bin/bash", "-lc", command), noop, SandboxExecResult{Applied: false, Mechanism: "none", NetworkPolicy: network}, nil case SandboxScopeWorkspace, SandboxScopeLocal: if _, statErr := os.Stat(sandboxExecBinary); statErr != nil { res, err := resolveSandboxUnavailable(scope, "seatbelt", fmt.Sprintf("%s not found: %v", sandboxExecBinary, statErr)) if err != nil { return nil, nil, SandboxExecResult{}, err } + res.NetworkPolicy = network return exec.CommandContext(ctx, "/bin/bash", "-lc", command), noop, res, nil } @@ -52,7 +62,7 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo absRoot = resolvedRoot } - profile := seatbeltProfile(scope, absRoot) + profile := seatbeltProfile(scope, absRoot, network) f, err := os.CreateTemp("", "harness-sandbox-*.sb") if err != nil { return nil, nil, SandboxExecResult{}, fmt.Errorf("sandbox: create seatbelt profile: %w", err) @@ -69,7 +79,7 @@ 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"}, nil + return cmd, cleanup, SandboxExecResult{Applied: true, Mechanism: "seatbelt", NetworkPolicy: network}, nil default: return nil, nil, SandboxExecResult{}, fmt.Errorf("unknown sandbox scope %q", scope) } @@ -81,12 +91,18 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo // 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); all network -// operations are denied. +// bash needs (/dev/null, /dev/tty, /dev/zero, /dev/dtracehelper). // -// SandboxScopeLocal: filesystem access (read and write) is unconfined; -// network operations are denied. -func seatbeltProfile(scope SandboxScope, workspaceRoot string) string { +// SandboxScopeLocal: filesystem access (read and write) is unconfined. +// +// Both scopes' network access follows the network policy (issue #1397): +// under "(deny default)", every operation — including network — is denied +// unless explicitly allowed, so NetworkPolicyAllow (the default) emits an +// explicit "(allow network*)" rather than merely omitting a deny rule; a +// bare omission would still leave network denied by the profile's default. +// NetworkPolicyDeny emits the equivalent explicit deny for clarity, since +// deny-by-default already covers it. +func seatbeltProfile(scope SandboxScope, workspaceRoot string, network NetworkPolicy) string { var b strings.Builder b.WriteString("(version 1)\n(deny default)\n(allow process-fork)\n(allow process-exec)\n(allow signal (target self))\n(allow sysctl-read)\n(allow mach-lookup)\n(allow iokit-open)\n(allow file-read*)\n") switch scope { @@ -97,7 +113,11 @@ func seatbeltProfile(scope SandboxScope, workspaceRoot string) string { case SandboxScopeLocal: b.WriteString("(allow file-write*)\n") } - b.WriteString("(deny network*)\n") + if network == NetworkPolicyDeny { + b.WriteString("(deny network*)\n") + } else { + b.WriteString("(allow network*)\n") + } return b.String() } diff --git a/internal/harness/tools/sandbox_linux.go b/internal/harness/tools/sandbox_linux.go index e069772e3..635ea0a63 100644 --- a/internal/harness/tools/sandbox_linux.go +++ b/internal/harness/tools/sandbox_linux.go @@ -20,11 +20,18 @@ import ( // sandboxing tool that gives us real mount-namespace filesystem // confinement and network-namespace isolation without adding a Go // dependency. +// The network policy is read from ctx (NetworkPolicyFromContext), defaulting +// to NetworkPolicyAllow when absent; --unshare-net is only added to the bwrap +// invocation when the resolved policy is deny (issue #1397). func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoot, command string) (*exec.Cmd, func(), SandboxExecResult, error) { noop := func() {} + network, _ := NetworkPolicyFromContext(ctx) + if network == "" { + network = NetworkPolicyAllow + } switch scope { case SandboxScopeUnrestricted, "": - return exec.CommandContext(ctx, "/bin/bash", "-lc", command), noop, SandboxExecResult{Applied: false, Mechanism: "none"}, nil + return exec.CommandContext(ctx, "/bin/bash", "-lc", command), noop, SandboxExecResult{Applied: false, Mechanism: "none", NetworkPolicy: network}, nil case SandboxScopeWorkspace, SandboxScopeLocal: bwrapPath, lookErr := exec.LookPath("bwrap") if lookErr != nil { @@ -32,6 +39,7 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo if err != nil { return nil, nil, SandboxExecResult{}, err } + res.NetworkPolicy = network return exec.CommandContext(ctx, "/bin/bash", "-lc", command), noop, res, nil } @@ -42,7 +50,6 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo args := []string{ "--die-with-parent", - "--unshare-net", // Isolate PID and IPC namespaces so sandboxed processes can // neither signal same-UID host processes nor read host // /proc//environ (API keys) — parity with darwin seatbelt's @@ -55,6 +62,9 @@ func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoo "--proc", "/proc", "--dev", "/dev", } + if network == NetworkPolicyDeny { + args = append(args, "--unshare-net") + } if scope == SandboxScopeWorkspace { // Bind the whole root filesystem read-only, then punch a // read-write hole for the workspace only. Separate mounts @@ -74,7 +84,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"}, nil + return cmd, noop, SandboxExecResult{Applied: true, Mechanism: "bubblewrap", NetworkPolicy: network}, nil default: return nil, nil, SandboxExecResult{}, fmt.Errorf("unknown sandbox scope %q", scope) } diff --git a/internal/harness/tools/sandbox_other.go b/internal/harness/tools/sandbox_other.go index 15e736cb8..43dc5f782 100644 --- a/internal/harness/tools/sandbox_other.go +++ b/internal/harness/tools/sandbox_other.go @@ -15,14 +15,19 @@ import ( func buildSandboxedCommand(ctx context.Context, scope SandboxScope, workspaceRoot, command string) (*exec.Cmd, func(), SandboxExecResult, error) { noop := func() {} cmd := exec.CommandContext(ctx, "/bin/bash", "-lc", command) + network, _ := NetworkPolicyFromContext(ctx) + if network == "" { + network = NetworkPolicyAllow + } switch scope { case SandboxScopeUnrestricted, "": - return cmd, noop, SandboxExecResult{Applied: false, Mechanism: "none"}, nil + return cmd, noop, SandboxExecResult{Applied: false, Mechanism: "none", NetworkPolicy: network}, nil case SandboxScopeWorkspace, SandboxScopeLocal: res, err := resolveSandboxUnavailable(scope, "os-sandbox", "no OS-level sandbox mechanism implemented for this platform") if err != nil { return nil, nil, SandboxExecResult{}, err } + res.NetworkPolicy = network return cmd, noop, res, nil default: return nil, nil, SandboxExecResult{}, fmt.Errorf("unknown sandbox scope %q", scope) diff --git a/internal/harness/tools/types.go b/internal/harness/tools/types.go index 738f40767..9d92e4e71 100644 --- a/internal/harness/tools/types.go +++ b/internal/harness/tools/types.go @@ -43,6 +43,22 @@ const ( SandboxScopeUnrestricted SandboxScope = "unrestricted" ) +// NetworkPolicy mirrors harness.NetworkPolicy at the tools layer for the same +// import-cycle reason as SandboxScope above. It controls whether the bash +// sandbox denies outbound network access, independent of the sandbox scope's +// filesystem confinement (issue #1397). +type NetworkPolicy string + +const ( + // NetworkPolicyAllow permits outbound network access from the bash + // sandbox. This is the default: an empty NetworkPolicy is treated as + // allow everywhere it is read. + NetworkPolicyAllow NetworkPolicy = "allow" + // NetworkPolicyDeny blocks outbound network access from the bash + // sandbox at the OS level (seatbelt on darwin, bubblewrap on linux). + NetworkPolicyDeny NetworkPolicy = "deny" +) + type PolicyInput struct { ToolName string `json:"tool_name"` Action Action `json:"action"` @@ -581,6 +597,7 @@ const ContextKeyTranscriptReader contextKey = "transcript_reader" const ContextKeyOutputStreamer contextKey = "output_streamer" const ContextKeyMessageReplacer contextKey = "message_replacer" const ContextKeySandboxScope contextKey = "sandbox_scope" +const ContextKeyNetworkPolicy contextKey = "network_policy" const ContextKeyPlanModeGate contextKey = "plan_mode_gate" const contextKeyRecipeStepAuthorizer contextKey = "recipe_step_authorizer" const contextKeyAskUserQuestionPendingNotifier contextKey = "ask_user_question_pending_notifier" @@ -642,6 +659,15 @@ func WithSandboxScope(ctx context.Context, scope SandboxScope) context.Context { return context.WithValue(ctx, ContextKeySandboxScope, scope) } +// WithNetworkPolicy returns a context with the given network policy set for +// the current tool execution. Nil contexts are promoted to Background. +func WithNetworkPolicy(ctx context.Context, policy NetworkPolicy) context.Context { + if ctx == nil { + ctx = context.Background() + } + return context.WithValue(ctx, ContextKeyNetworkPolicy, policy) +} + // ContextKeyExtraAllowedRoots carries the per-run extra directory roots a // caller granted (harness.RunRequest.ExtraDirs, TUI /add-dir). File-tool // confinement (ConfineWorkspacePath via ResolveWorkspacePathConfined) permits @@ -795,6 +821,19 @@ func SandboxScopeFromContext(ctx context.Context) (SandboxScope, bool) { return v, ok } +// NetworkPolicyFromContext retrieves the effective network policy override +// from the tool execution context. Callers that need a concrete default +// (rather than the raw ok bool) should treat a missing or empty value as +// NetworkPolicyAllow, matching the safety-biased default documented on +// NetworkPolicyAllow. +func NetworkPolicyFromContext(ctx context.Context) (NetworkPolicy, bool) { + if ctx == nil { + return "", false + } + v, ok := ctx.Value(ContextKeyNetworkPolicy).(NetworkPolicy) + return v, ok +} + // CronClient provides access to the cron scheduler daemon. var ErrCronJobNotFound = errors.New("cron job not found") var ErrCronJobConflict = errors.New("cron job update conflict") diff --git a/internal/harness/types.go b/internal/harness/types.go index c72b01bd5..77aeda2d1 100644 --- a/internal/harness/types.go +++ b/internal/harness/types.go @@ -877,10 +877,31 @@ const ( ApprovalPolicyAll ApprovalPolicy = "all" ) -// PermissionConfig combines sandbox scope and approval policy. +// NetworkPolicy controls whether the bash sandbox permits outbound network +// access, independent of SandboxScope's filesystem confinement (issue #1397). +type NetworkPolicy string + +const ( + // NetworkPolicyAllow permits outbound network access from the bash + // sandbox. This is the default: an empty NetworkPolicy is treated as + // allow. + NetworkPolicyAllow NetworkPolicy = "allow" + // NetworkPolicyDeny blocks outbound network access from the bash + // sandbox at the OS level (seatbelt on darwin, bubblewrap on linux). + NetworkPolicyDeny NetworkPolicy = "deny" +) + +// PermissionConfig combines sandbox scope, approval policy, and network +// policy. type PermissionConfig struct { Sandbox SandboxScope `json:"sandbox"` Approval ApprovalPolicy `json:"approval"` + // Network controls outbound network access from the bash sandbox. + // Empty defaults to NetworkPolicyAllow: workspace/local scopes permit + // outbound network unless a caller explicitly opts into deny (issue + // #1397; previously the bash sandbox always denied network for these + // two scopes). + Network NetworkPolicy `json:"network,omitempty"` // Rules applies fine-grained effects to matching tool calls. A nil or empty // rule set leaves the legacy two-axis permission behavior unchanged. Rules *PermissionRuleSet `json:"rules,omitempty"` @@ -901,6 +922,7 @@ func DefaultPermissionConfig() PermissionConfig { return PermissionConfig{ Sandbox: SandboxScopeWorkspace, Approval: ApprovalPolicyNone, + Network: NetworkPolicyAllow, } } @@ -939,6 +961,15 @@ func ValidatePermissionConfig(p PermissionConfig) error { default: return fmt.Errorf("invalid approval policy %q: must be one of none, destructive, all", p.Approval) } + switch p.Network { + case NetworkPolicyAllow, NetworkPolicyDeny: + // valid + case "": + // empty defaults to allow (see DefaultPermissionConfig and + // normalizePermissionConfig) — also valid at validation time + default: + return fmt.Errorf("invalid network policy %q: must be one of allow, deny", p.Network) + } if err := ValidatePermissionRules(permissionRulesFromSet(p.Rules)); err != nil { return err } From a4871c40fa7ca4f6be415d24660b568e047166e8 Mon Sep 17 00:00:00 2001 From: Dennison Date: Sun, 6 Sep 2026 09:58:02 -0400 Subject: [PATCH 3/4] test(regression): TASK-1397 regression coverage for sandbox network policy Regression tests added that would fail if the change in cdbc73dc is reverted: - TestRedTeam_SandboxNetwork_DefaultPermissionsAllowCurl (internal/harness/runner_redteam_test.go): drives a real bash tool call through the full runner -> step engine -> tool context path with NO explicit Permissions on the RunRequest, and asserts curl is NOT rejected as a sandbox violation. This exercises the end-to-end plumbing (not just the unit-level seatbeltProfile/CheckSandboxCommand functions already covered by the red commit), so it would catch a regression where the runner stopped threading the resolved network policy from effectivePermissions into the tool execution context, or where normalizePermissionConfig stopped defaulting Network to allow. - TestJobManagerRunForegroundReportsSandboxNetworkInResult (internal/harness/tools/sandbox_test.go): asserts the bash tool result map's "sandbox_network" field matches the network policy actually applied, for both allow and deny, under a real OS-level sandbox. Catches a regression where SandboxExecResult.NetworkPolicy stopped being threaded into the JobManager result map. Full test suite output (internal/harness/..., cmd/harnesscli, -race): ok go-agent-harness/internal/harness 7.945s ok go-agent-harness/internal/harness/tools 18.497s ok go-agent-harness/internal/harness/tools/core 2.917s ok go-agent-harness/internal/harness/tools/deferred 12.696s ok go-agent-harness/internal/harness/tools/descriptions 1.628s ok go-agent-harness/internal/harness/tools/recipe 2.520s ok go-agent-harness/internal/harness/tools/script 4.397s ok go-agent-harness/cmd/harnesscli 7.709s (2498 individual `--- PASS` lines across both packages under -v, 0 FAIL) go vet ./internal/harness/... ./cmd/harnesscli/...: clean, no output. Regression scenarios covered: - Default permissions (no Permissions field at all) allow outbound network for a real bash tool call end to end. - The bash tool result surfaces the actually-applied network policy for both allow and deny. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5 --- internal/harness/runner_redteam_test.go | 73 +++++++++++++++++++++++++ internal/harness/tools/sandbox_test.go | 34 ++++++++++++ 2 files changed, 107 insertions(+) diff --git a/internal/harness/runner_redteam_test.go b/internal/harness/runner_redteam_test.go index c81b82003..abfe6f8d1 100644 --- a/internal/harness/runner_redteam_test.go +++ b/internal/harness/runner_redteam_test.go @@ -378,6 +378,79 @@ func TestRedTeam_SandboxNetwork_BlocksCurl(t *testing.T) { } } +// TestRedTeam_SandboxNetwork_DefaultPermissionsAllowCurl is a regression test +// for issue #1397's default change: a real bash tool call under a run with NO +// explicit Permissions (so SandboxScopeWorkspace + the default NetworkPolicy) +// must NOT be rejected as a sandbox violation. Before this change, the +// runner's default sandbox scope always denied outbound network for bash +// regardless of caller intent; if that regression were reintroduced (e.g. a +// future edit to normalizePermissionConfig or the step engine's context +// wiring dropped the network policy), this test would start failing with +// "sandbox violation" on the curl call the way TestRedTeam_SandboxNetwork_BlocksCurl +// above intentionally does for an explicit deny. +func TestRedTeam_SandboxNetwork_DefaultPermissionsAllowCurl(t *testing.T) { + t.Parallel() + + workspace := t.TempDir() + registry := NewDefaultRegistryWithOptions(workspace, DefaultRegistryOptions{ + ApprovalMode: ToolApprovalModeFullAuto, + SandboxScope: SandboxScopeWorkspace, + }) + + provider := &continuationProvider{ + turns: []CompletionResult{ + { + ToolCalls: []ToolCall{{ + ID: "call_curl_default", + Name: "bash", + Arguments: `{"command":"curl --version"}`, + }}, + }, + {Content: "done"}, + }, + } + + runner := NewRunner(provider, registry, RunnerConfig{ + DefaultModel: "test-model", + MaxSteps: 4, + }) + + // No Permissions field at all: exercises the server's real default, not + // an explicitly requested policy. + run, err := runner.StartRun(RunRequest{Prompt: "check curl"}) + if err != nil { + t.Fatalf("StartRun: %v", err) + } + + events, err := collectRunEvents(t, runner, run.ID) + if err != nil { + t.Fatalf("collectRunEvents: %v", err) + } + + var completedSeen bool + for _, ev := range events { + if ev.Type != EventToolCallCompleted { + continue + } + if callID, _ := ev.Payload["call_id"].(string); callID != "call_curl_default" { + continue + } + completedSeen = true + errField, _ := ev.Payload["error"].(string) + if strings.Contains(errField, "sandbox violation") { + t.Errorf("tool.call.completed error = %q, want no sandbox violation under default permissions", errField) + } + } + if !completedSeen { + t.Fatalf("expected tool.call.completed for call_curl_default, events=%v", eventTypes(events)) + } + + payload := toolMessagePayload(t, runner, run.ID, "bash") + if errMsg, _ := payload["error"].(string); strings.Contains(errMsg, "sandbox violation") { + t.Errorf("bash tool result error = %v, want no sandbox violation under default permissions", payload["error"]) + } +} + // TestRedTeam_SandboxNetworkRegexBypasses_NotEnforced DOCUMENTS (does not claim a // guarantee for) the known regex-only bypasses of the SandboxScopeLocal network // filter. The filter is a set of regexes over the raw command string diff --git a/internal/harness/tools/sandbox_test.go b/internal/harness/tools/sandbox_test.go index d54be9587..bdc9258ea 100644 --- a/internal/harness/tools/sandbox_test.go +++ b/internal/harness/tools/sandbox_test.go @@ -454,3 +454,37 @@ func TestSandboxWorkspaceScopeNetworkPolicyLiveCurl(t *testing.T) { } }) } + +// TestJobManagerRunForegroundReportsSandboxNetworkInResult is a regression +// test for issue #1397: the bash tool result map must surface which network +// policy was actually applied (result["sandbox_network"]), for both allow +// and deny, so an operator inspecting a run's tool output can see the policy +// without cross-referencing the run's permissions separately. If a future +// change stopped threading SandboxExecResult.NetworkPolicy into the result +// map (bash_manager.go), this test would fail by finding the key absent or +// mismatched, independent of whether the command itself succeeded. +func TestJobManagerRunForegroundReportsSandboxNetworkInResult(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) + + for _, policy := range []NetworkPolicy{NetworkPolicyAllow, NetworkPolicyDeny} { + policy := policy + t.Run(string(policy), func(t *testing.T) { + t.Parallel() + ctx := WithNetworkPolicy(context.Background(), policy) + result, err := mgr.RunForeground(ctx, "echo hi", 5, "") + if err != nil { + t.Fatalf("RunForeground: %v", err) + } + if got := result["sandbox_network"]; got != string(policy) { + t.Errorf("result[\"sandbox_network\"] = %v, want %q", got, string(policy)) + } + }) + } +} From 6ea0e2dcadec331bc8ef44cd03c8409ed8132fda Mon Sep 17 00:00:00 2001 From: Dennison Date: Sun, 6 Sep 2026 10:00:46 -0400 Subject: [PATCH 4/4] docs: TASK-1397 document the bash sandbox network policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updates every doc page that stated bash network was unconditionally denied for workspace/local sandbox scope, or claimed the wrong default sandbox scope (unrestricted instead of workspace), to match the implementation in cdbc73dc: - website/docs/concepts/tools-and-permissions.md: rewrites the permission model section for three axes (sandbox, network, approval), adds a Network policy tabbed section, and calls out the security-relevant default change explicitly. - website/docs/reference/tools-catalog.md: corrects the sandbox-scope default (workspace, not unrestricted) and adds the network-policy table. - website/docs/reference/glossary.md: corrects the sandbox-scope default and documents the network axis. - website/docs/reference/http-routes.md: corrects the RunRequest example body's default permissions and field notes. - website/docs/server/http-api-guide.md: adds "network" to the permissions JSON examples, corrects the omitted-permissions default, and documents the network field/default in the field table. - website/docs/reference/cli-flags.md, website/docs/cli/harnesscli.md: document the new --sandbox/--network flags. - docs/logs/engineering-log.md: new entry describing the change, the seatbeltProfile "(deny default)" gotcha (an omitted deny rule does not become an allow — an explicit "(allow network*)" is required), and calling out the default change as security-relevant. website/docs/server/harnessd.md and docs/runbooks/benchmark-smoke.md were checked (per the task) but contain no bash-sandbox-network statements to update; docs/runbooks/profile-authoring.md's "network-denied profile" reference is a different mechanism (the allow_net_access profile capability gating ActionFetch/ActionDownload) and is out of scope. Docs-only change; no rebuild required. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5 --- docs/logs/engineering-log.md | 58 +++++++++++++++++++ website/docs/cli/harnesscli.md | 2 + .../docs/concepts/tools-and-permissions.md | 54 ++++++++++++----- website/docs/reference/cli-flags.md | 2 + website/docs/reference/glossary.md | 8 ++- website/docs/reference/http-routes.md | 8 ++- website/docs/reference/tools-catalog.md | 20 +++++-- website/docs/server/http-api-guide.md | 13 +++-- 8 files changed, 135 insertions(+), 30 deletions(-) diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index 3614ae925..5c7231796 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -6102,3 +6102,61 @@ Skipped creating separate issues for Op/EventMsg protocol (already covered by SS client.go. Live verification against DeepSeek via OpenRouter is a follow-up for whoever holds the API key; this PR only proves the fake/unit/integration paths. + +# 2026-09-06 (Issue #1397 bash sandbox network policy — SECURITY-RELEVANT DEFAULT CHANGE) + +- Prior behavior: `SandboxScopeWorkspace` and `SandboxScopeLocal` unconditionally + denied outbound network from the `bash` tool (`(deny network*)` in the darwin + seatbelt profile, `--unshare-net` always in the linux bwrap invocation), with + no way for a caller to opt back in. A run that legitimately needed to install + a dependency or call an API from a sandboxed `bash` call had no way to do so + short of dropping to `SandboxScopeUnrestricted`, which also drops filesystem + confinement. +- Change: `harness.PermissionConfig` gains a third axis, `Network` (json + `network`, values `""`/`"allow"`/`"deny"`, default `"allow"`), mirrored at + the tools layer as `tools.NetworkPolicy` with a context key + (`ContextKeyNetworkPolicy`), `WithNetworkPolicy`, and + `NetworkPolicyFromContext`, set alongside the existing sandbox-scope context + value wherever the runner sets up a tool call's execution context + (`runner_step_engine.go`). **SECURITY-RELEVANT DEFAULT CHANGE:** workspace + and local sandbox scopes now allow outbound network by default; callers that + need the old always-deny behavior must set `permissions.network` to + `"deny"` explicitly. `SandboxScopeUnrestricted` is unaffected (it was never + network-confined). +- Gotcha found while implementing the darwin side: seatbelt's `(deny default)` + means every operation — including network — is denied unless explicitly + allowed. The first implementation attempt just omitted the `(deny network*)` + line for the allow case and left it there; that still left curl unable to + resolve DNS (`exit 6`) because omitting a deny rule under `(deny default)` + does not become an allow. The fix emits an explicit `(allow network*)` line + for the allow case (and an explicit `(deny network*)` for deny, which is + redundant with the default but kept for clarity/robustness against a future + default-policy change). Confirmed directly with `sandbox-exec` against a + real host (`https://proxy.golang.org`): `http_code=200` under an + `(allow network*)` profile, `exit=6` ("could not resolve host") under + `(deny network*)`. +- `SandboxExecResult` gains a `NetworkPolicy` field so the bash tool result map + carries a `sandbox_network` key reporting the policy actually applied, + independent of the mechanism (`seatbelt`, `bubblewrap`, `none`, or + `unavailable`). +- The permissions statement injected into the model's context + ("Permissions for this run: sandbox=%s, approval=%s, network=%s.") now + includes the network axis and, when denied, an explicit warning that + dependency installs will fail and the model should report the blocker + rather than substitute a design. It is now appended to every turn's wire + message list (not persisted into conversation history, unlike the existing + continuation-changed notice) so it reaches the model starting on turn one — + previously this line only appeared on a continuation whose permissions + changed from the source run, never on a fresh run's first turn. +- `harnesscli` gains `--sandbox` and `--network` flags on the one-shot run + path, populating `permissions` on the run-create request only when either + flag is set (both omitted → server defaults, matching the new default + above). +- Regression: `TestRedTeam_SandboxNetwork_DefaultPermissionsAllowCurl` drives a + real bash tool call through the full runner with no explicit `Permissions` + and asserts curl is not rejected; `TestJobManagerRunForegroundReportsSandboxNetworkInResult` + asserts the `sandbox_network` result field matches the applied policy for + both allow and deny under a real OS-level sandbox; a live integration test + (`TestSandboxWorkspaceScopeNetworkPolicyLiveCurl`) curls + `https://proxy.golang.org` through the actual seatbelt sandbox and asserts + success under allow, failure under deny. diff --git a/website/docs/cli/harnesscli.md b/website/docs/cli/harnesscli.md index edc9ed808..ab5b06700 100644 --- a/website/docs/cli/harnesscli.md +++ b/website/docs/cli/harnesscli.md @@ -71,6 +71,8 @@ The `run_id=` / `terminal_event=` stdout lines are unchanged by this mapping. Se | `-prompt-custom` | `""` | Custom prompt extension text | | `-workspace` | cwd | Workspace directory for this run (sent as `workspace_path`; see the callout below) | | `-plan-mode` | `false` | Start the run in enforced read-only plan mode (`plan_mode` in the request); see [Enforced Plan Mode](/docs/concepts/configuration) | +| `-sandbox` | `""` (server default: `workspace`) | Sandbox scope: `workspace`, `local`, or `unrestricted` (`permissions.sandbox` in the request) | +| `-network` | `""` (server default: `allow`) | Network policy for the `bash` sandbox: `allow` or `deny` (`permissions.network` in the request; issue #1397) | | `-resume` | `""` | Resume an existing conversation by ID in the TUI; implies `-tui` | | `-tui` | `false` | Launch the interactive BubbleTea TUI (requires a real terminal) | | `-list-profiles` | `false` | List available profiles and exit | diff --git a/website/docs/concepts/tools-and-permissions.md b/website/docs/concepts/tools-and-permissions.md index 2e5c8f04b..e65e8fb06 100644 --- a/website/docs/concepts/tools-and-permissions.md +++ b/website/docs/concepts/tools-and-permissions.md @@ -97,13 +97,14 @@ LSP tools (`lsp_diagnostics`, `lsp_references`) are defined but are **not** incl ## Permission model -Every run operates under a `PermissionConfig` with two independent axes: **sandbox scope** and **approval policy**. +Every run operates under a `PermissionConfig` with three independent axes: **sandbox scope**, **network policy**, and **approval policy**. ```go -// internal/harness/types.go:693-696 +// internal/harness/types.go type PermissionConfig struct { Sandbox SandboxScope `json:"sandbox"` Approval ApprovalPolicy `json:"approval"` + Network NetworkPolicy `json:"network,omitempty"` } ``` @@ -111,36 +112,63 @@ type PermissionConfig struct { The sandbox scope controls what the agent's `bash` tool can access. - + - unrestricted - local workspace + local + unrestricted + + +**`"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. + +This scope is recommended for untrusted prompts operating on a bounded codebase. + + + + +**`"local"`** — Filesystem access (read and write) is unrestricted. + + -**`"unrestricted"`** — No filesystem restrictions. This is the default when `permissions` is omitted. +**`"unrestricted"`** — No filesystem restrictions. The agent can read and write any path on the host filesystem and run arbitrary shell commands. - + + +Source: `internal/harness/types.go`. + +### Network policy + +The network policy controls whether the agent's `bash` tool can reach the network, independent of the sandbox scope's filesystem confinement (issue #1397). -**`"local"`** — Filesystem access is unrestricted, but outbound network commands (`curl`, `wget`, `nc`, `netcat`, `telnet`) are blocked inside `bash`. + + + allow + deny + + -Use this when you want to prevent exfiltration over the network while still allowing full local filesystem access. +**`"allow"`** — Outbound network access from `bash` is unrestricted. This is the default: an omitted or empty `network` field behaves the same as `"allow"`. Applies to both `"workspace"` and `"local"` sandbox scope; `"unrestricted"` scope was never network-confined. - + -**`"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. Network access is unrestricted under this scope. +**`"deny"`** — Outbound network access from `bash` is blocked at the OS level (a seatbelt `(deny network*)` profile rule on macOS, `bwrap --unshare-net` on Linux) for `"workspace"` and `"local"` sandbox scope. `"unrestricted"` scope ignores this field — it has no network confinement at any setting. -This scope is recommended for untrusted prompts operating on a bounded codebase. +When denied, the model is told in its permissions notice that dependency installs will fail and to report the blocker rather than substitute a different design. -Source: `internal/harness/types.go:670–677`. +Source: `internal/harness/types.go`, `internal/harness/tools/sandbox_darwin.go`, `internal/harness/tools/sandbox_linux.go`. + + +Before this change (issue #1397), `"workspace"` and `"local"` sandbox scope always denied `bash` network access unconditionally — there was no way to opt back in for a run that legitimately needed to install a dependency or call an API. The default is now `"allow"`; set `network: "deny"` explicitly for a run that must not reach the network. + ### Approval policy diff --git a/website/docs/reference/cli-flags.md b/website/docs/reference/cli-flags.md index 700413590..da02e0111 100644 --- a/website/docs/reference/cli-flags.md +++ b/website/docs/reference/cli-flags.md @@ -79,6 +79,8 @@ Source: `cmd/harnesscli/main.go:123` | `-prompt-custom` | string | `""` | Custom prompt extension text appended to the prompt. | | `-workspace` | string | `""` (resolves to cwd) | Workspace directory for this run. Resolved via `os.Getwd()` when empty, sent as `workspace_path`, and honored by the server when it is an absolute path to an existing directory (tools are rooted there). | | `-plan-mode` | bool | `false` | Start the run in enforced read-only plan mode (sent as `plan_mode`). | +| `-sandbox` | string | `""` (server default: `workspace`) | Sandbox scope for this run: `workspace`, `local`, or `unrestricted`. Sent as `permissions.sandbox`; omitted from the request body when neither `-sandbox` nor `-network` is set (issue #1397). | +| `-network` | string | `""` (server default: `allow`) | Network policy for the `bash` sandbox: `allow` or `deny`. Sent as `permissions.network`; omitted from the request body when neither `-sandbox` nor `-network` is set (issue #1397). | | `-resume` | string | `""` | Resume an existing conversation by ID in the TUI; implies `-tui`. | | `-tui` | bool | `false` | Launch the interactive BubbleTea TUI. Requires a real terminal — fails with an error if stdout is a pipe. | | `-list-profiles` | bool | `false` | Fetch and print available profiles, then exit. | diff --git a/website/docs/reference/glossary.md b/website/docs/reference/glossary.md index c199c8ffd..c15c87913 100644 --- a/website/docs/reference/glossary.md +++ b/website/docs/reference/glossary.md @@ -80,9 +80,11 @@ 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: -- `"unrestricted"` — no restrictions (default) -- `"local"` — filesystem access is unrestricted, but outbound network commands (`curl`, `wget`, `nc`, etc.) are blocked in the `bash` tool -- `"workspace"` — the `bash` tool can only access paths inside the workspace directory +- `"workspace"` — the `bash` tool can only access paths inside the workspace directory (default) +- `"local"` — filesystem access is unrestricted +- `"unrestricted"` — no restrictions + +Outbound network access from the `bash` tool is a separate axis, `permissions.network` (issue #1397): `"allow"` (default) or `"deny"`, enforced at the OS level for `"workspace"`/`"local"` scope. See: [HTTP API guide](/docs/server/http-api-guide) diff --git a/website/docs/reference/http-routes.md b/website/docs/reference/http-routes.md index bfba16565..977d6b904 100644 --- a/website/docs/reference/http-routes.md +++ b/website/docs/reference/http-routes.md @@ -375,8 +375,9 @@ Source: `internal/harness/types.go`. "profile": "", "parent_context_handoff": null, "permissions": { - "sandbox": "unrestricted", - "approval": "none" + "sandbox": "workspace", + "approval": "none", + "network": "allow" }, "role_models": { "primary": "", @@ -397,8 +398,9 @@ Selected field notes: - `max_steps` and `max_turns`: `0` means unlimited — there is no default step cap; negative values are rejected. - `max_cost_usd`: `0` means unlimited; the run emits `run.cost_limit_reached` on breach (run still completes normally). - `denied_tools` lists tool names that must never be offered to or callable from this run, even if `allowed_tools` or an activated skill would otherwise grant them. -- `permissions.sandbox`: `"unrestricted"` (default), `"local"`, or `"workspace"`. +- `permissions.sandbox`: `"workspace"` (default), `"local"`, or `"unrestricted"`. - `permissions.approval`: `"none"` (default), `"destructive"`, or `"all"`. +- `permissions.network` (issue #1397): `"allow"` (default) or `"deny"` — controls whether the `bash` tool can reach the network under `"workspace"`/`"local"` sandbox scope; `"unrestricted"` scope is unaffected. - `rules` applies fine-grained allow/ask/deny effects to tool calls; evaluated together with `permissions.rules`, with `rules` appended after. - `initiator_api_key_prefix` is server-populated from the auth context — it is never accepted from the request body. diff --git a/website/docs/reference/tools-catalog.md b/website/docs/reference/tools-catalog.md index 29a1c8db5..cff050c8a 100644 --- a/website/docs/reference/tools-catalog.md +++ b/website/docs/reference/tools-catalog.md @@ -399,18 +399,25 @@ When `allowed_tools` is non-empty, only the listed names **plus** `AlwaysAvailab ## Default permissions (security) -By default, tools run without any sandboxing and without any approval prompts. This is appropriate for trusted development environments and automated pipelines where you control the workspace. For anything handling untrusted input, explicitly set a `permissions` policy. +By default, tools run sandbox-confined to the workspace directory and without any approval prompts (no sandboxing at all requires explicitly opting into `"unrestricted"`). Bash's outbound network access is unrestricted by default too — set `network: "deny"` to block it. For anything handling untrusted input, review the `permissions` policy explicitly rather than relying on defaults. -The `PermissionConfig` struct (source: `internal/harness/types.go`) controls two independent axes: +The `PermissionConfig` struct (source: `internal/harness/types.go`) controls three independent axes: **Sandbox scope** (`sandbox` field): | Value | Behavior | |-------|----------| -| `"unrestricted"` | No restrictions. **Default.** | -| `"local"` | Filesystem access unrestricted; outbound network commands (`curl`, `wget`, `nc`, `netcat`, `telnet`) blocked in bash. | -| `"workspace"` | Bash can only access paths inside the workspace directory. | +| `"workspace"` | Bash can only access paths inside the workspace directory. **Default.** | +| `"local"` | Filesystem access unrestricted. | +| `"unrestricted"` | No restrictions. | + +**Network policy** (`network` field, issue #1397): + +| Value | Behavior | +|-------|----------| +| `"allow"` | Bash's outbound network access is unrestricted. **Default** (an omitted or empty `network` field behaves the same as `"allow"`). | +| `"deny"` | Bash's outbound network access is denied at the OS level (seatbelt on macOS, bubblewrap on Linux) for `"workspace"` and `"local"` sandbox scopes. `"unrestricted"` scope is never network-confined regardless of this field. | **Approval policy** (`approval` field): @@ -427,7 +434,8 @@ To harden a run, pass a `permissions` object in the run request: "prompt": "...", "permissions": { "sandbox": "workspace", - "approval": "destructive" + "approval": "destructive", + "network": "deny" } } ``` diff --git a/website/docs/server/http-api-guide.md b/website/docs/server/http-api-guide.md index bf89aaaf3..1d1fb2f05 100644 --- a/website/docs/server/http-api-guide.md +++ b/website/docs/server/http-api-guide.md @@ -215,7 +215,8 @@ Fill in the fields below and copy the generated JSON body or curl command. The b "fallback_providers": ["anthropic"], "permissions": { "sandbox": "workspace", - "approval": "destructive" + "approval": "destructive", + "network": "deny" }, "mcp_servers": [ {"name": "my-mcp", "url": "http://localhost:9000"} @@ -286,25 +287,27 @@ Fill in the fields below and copy the generated JSON body or curl command. The b ### Permissions -The `permissions` field controls the two-axis permission model. It is an object — **not** top-level fields: +The `permissions` field controls the three-axis permission model. It is an object — **not** top-level fields: ```json { "permissions": { "sandbox": "workspace", - "approval": "destructive" + "approval": "destructive", + "network": "deny" } } ``` -`sandbox` and `approval` are nested inside the `permissions` object. They are not top-level `RunRequest` fields. Omitting the `permissions` block entirely is equivalent to `{"sandbox": "unrestricted", "approval": "none"}` — the agent runs unsandboxed with no approval gate. See [Tools and Permissions](/docs/concepts/tools-and-permissions) for a full explanation of what each value means. +`sandbox`, `approval`, and `network` are nested inside the `permissions` object. They are not top-level `RunRequest` fields. Omitting the `permissions` block entirely is equivalent to `{"sandbox": "workspace", "approval": "none", "network": "allow"}` — the agent is workspace-confined with no approval gate, and its `bash` tool can reach the network. See [Tools and Permissions](/docs/concepts/tools-and-permissions) for a full explanation of what each value means. | Field | Valid values | Default | |---|---|---| -| `sandbox` | `"unrestricted"`, `"local"`, `"workspace"` | `"unrestricted"` | +| `sandbox` | `"workspace"`, `"local"`, `"unrestricted"` | `"workspace"` | | `approval` | `"none"`, `"destructive"`, `"all"` | `"none"` | +| `network` (issue #1397) | `"allow"`, `"deny"` | `"allow"` | ### What the server sets (never send this)