Make bash sandbox network policy configurable (issue #1397)

[dennisonbertram]

Closes #1397

Summary

The bash sandbox previously denied outbound network unconditionally for SandboxScopeWorkspace/SandboxScopeLocal, with no way for a caller to opt back in. This adds a third PermissionConfig axis, network (""/"allow"/"deny", default "allow"), enforced at the OS level (seatbelt on macOS, bubblewrap on Linux), surfaced to the model in its first-turn permissions notice, exposed via harnesscli --sandbox/--network flags, and reported back on bash tool output (sandbox_network).

Security-relevant default change: workspace/local sandbox scopes now allow outbound network by default. Callers that need the old always-deny behavior must set permissions.network: "deny" explicitly.

Scope and issue reconciliation

Followed the coordinator's 2026-09-06 design-decision comment on #1397 exactly:

1. harness.PermissionConfig.Network (json network,omitempty), ""/"allow"/"deny", Validate rejects anything else — done in internal/harness/types.go. Mirrored at the tools layer (tools.NetworkPolicy, ContextKeyNetworkPolicy, WithNetworkPolicy, NetworkPolicyFromContext) in internal/harness/tools/types.go, set alongside the existing sandbox-scope context value in internal/harness/runner_step_engine.go, threaded from internal/harness/runner.go's runStepEngine call (was line 3450 in the issue, now threads htools.NetworkPolicy(effectivePermissions.Network) too).
2. sandbox_darwin.go's seatbeltProfile and sandbox_linux.go's bwrap args gate network confinement on the policy read from ctx; buildSandboxedCommand already received ctx. Filesystem confinement and unrestricted scope are unchanged. SandboxExecResult.NetworkPolicy reports the applied policy; the bash tool result map gains sandbox_network.
3. The permissions statement now includes network=allow|deny and, when denied, the required warning sentence. It is present on the first turn — previously this notice only existed for a continuation whose permissions changed. It's now a per-turn tail message (like working/observational memory and runtime context), not persisted into conversation history, so it doesn't inflate the stored transcript.
4. harnesscli --sandbox / --network populate runCreateRequest.Permissions only when either flag is set; both omitted means no "permissions" key at all, so server defaults apply.
5. Docs updated everywhere they stated network was denied or claimed the wrong default sandbox scope; engineering-log entry added calling out the default change as security-relevant.

Deviation from the literal task description, with reason: CheckSandboxCommand's SandboxScopeLocal heuristic (blocks bare curl/wget/nc/netcat/telnet regardless of the OS-level policy) is a second, string-based gate the issue's contract summary didn't call out explicitly. Left unchanged, it would have kept blocking those commands under the new default even after the OS-level fix, contradicting "workspace/local scopes now allow outbound network by default." Gated it on the same NetworkPolicy parameter (only active when deny) so the two enforcement layers agree. This is a minimal, same-file, same-mechanism fix required to make the stated default change actually true, not a new feature.

Out of scope, confirmed untouched: per-host allowlists, the fetch tool SSRF guard, checkWorkspaceScopeCommand's absolute-path heuristic, docs/runbooks/profile-authoring.md's allow_net_access profile capability (a different mechanism gating ActionFetch/ActionDownload, not bash OS-level sandboxing).

Impact analysis reconciliation

• internal/harness/types.go, internal/harness/tools/types.go: additive fields/types only; no existing field renamed or removed.
• internal/harness/tools/sandbox.go: CheckSandboxCommand gained a parameter. Its only caller is internal/harness/tools/bash_manager.go (updated) and tests (updated); confirmed no external callers via a repo-wide grep.
• internal/harness/tools/sandbox_darwin.go / sandbox_linux.go / sandbox_other.go: seatbeltProfile gained a parameter — its only caller is buildSandboxedCommand in the same file.
• internal/harness/clone.go's buildTurnMessages gained a parameter — its only two callers are in runner_step_engine.go (both updated); no test calls it directly.
• internal/harness/runner.go / runner_step_engine.go: runStepEngine/newStepEngine/stepEngine gained a field/parameter — single call chain, no other callers.
• cmd/harnesscli/main.go: runCreateRequest gained an optional field; startRun/JSON marshaling unaffected for callers that don't set it (confirmed via TestRunOmitsPermissionsWhenNoSandboxOrNetworkFlagSet).
• Existing tests that asserted the old always-deny default for SandboxScopeLocal, or fixed tail-message positions the new per-turn permissions notice shifts by one, were updated in place (working memory / observational memory ordering tests, a trusted-origin system-prompt capture test, a red-team default-deny test) — see the red and green commit messages for the full list and reasoning per test.

Architecture and duplication check

Reused the existing per-run context-injection pattern end to end rather than adding a new one: tools.SandboxScope/ContextKeySandboxScope/WithSandboxScope already existed as the mirror-at-tools-layer pattern for avoiding a harness to tools import cycle; NetworkPolicy follows the identical shape (type, const, context key, With*, *FromContext) right next to it, per the issue's explicit instruction. JobManager.SetNetworkPolicy/networkPolicyForContext mirror the existing SetSandboxScope/sandboxScopeForContext methods exactly. The permissions-notice text reuses the existing permissionsNoticeLines helper for both the (unchanged) continuation-changed notice and the new first-turn/per-turn notice, rather than duplicating the string formatting. No new tool catalog, no second sandbox-building code path, no parallel permissions struct.

Test-first evidence

Red commit: fdf22b52. Green commit: cdbc73dc. Regression commit: a4871c40.

Red command: go vet ./internal/harness/... ./cmd/harnesscli/... (plus go test ./cmd/harnesscli -run 'TestRunParsesSandboxAndNetworkFlagsIntoPermissions|TestRunOmitsPermissionsWhenNoSandboxOrNetworkFlagSet' -v), captured by staging only the test files then stashing the implementation with git stash push --keep-index -u.

Observed failure:

# 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
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

=== RUN   TestRunParsesSandboxAndNetworkFlagsIntoPermissions
    main_permissions_test.go:61: expected exit code 0, got 1
--- FAIL: TestRunParsesSandboxAndNetworkFlagsIntoPermissions (0.00s)

Why the failure proved the missing/incorrect behavior: the compile failures name exactly the symbols the acceptance contract requires (NetworkPolicy, WithNetworkPolicy, SandboxExecResult.NetworkPolicy, PermissionConfig.Network) — they don't exist yet, so no implementation could have accidentally made these tests pass. The CLI failure is a genuine runtime failure (unrecognized --sandbox/--network flags), not an import/compile error.

Green command: go build ./internal/harness/... ./cmd/harnesscli/... then go test ./internal/harness/... ./cmd/harnesscli -race. All green (see Verification evidence).

Verification evidence

Targeted:

go test ./internal/harness/... -run 'TestPermissionConfigNetworkValidation|TestNormalizePermissionConfigDefaultsNetworkToAllow|TestRunnerFirstTurnPermissionsNoticeIncludesNetworkAllow|TestRunnerFirstTurnPermissionsNoticeIncludesNetworkDenyWarning' -v
--- PASS (all 4)

go test ./cmd/harnesscli -run 'TestRunParsesSandboxAndNetworkFlagsIntoPermissions|TestRunOmitsPermissionsWhenNoSandboxOrNetworkFlagSet' -v
--- PASS (both)

Live sandbox integration (real network, real sandbox-exec), both as a Go test and manually against raw seatbelt profiles:

go test ./internal/harness/tools -run TestSandboxWorkspaceScopeNetworkPolicyLiveCurl -v
--- PASS: .../deny (0.02s)
--- PASS: .../allow (0.06s)

$ /usr/bin/sandbox-exec -f allow.sb /bin/bash -lc 'curl -sI -m 5 https://proxy.golang.org -o /dev/null -w "http_code=%{http_code}\n"'
http_code=200
exit=0
$ /usr/bin/sandbox-exec -f deny.sb  /bin/bash -lc 'curl -sI -m 5 https://proxy.golang.org -o /dev/null -w "http_code=%{http_code}\n"'
http_code=000
exit=6

Full regression, race-enabled:

go vet ./internal/harness/... ./cmd/harnesscli/...
(clean, no output)

go test ./internal/harness/... ./cmd/harnesscli -race -count=1
ok  	go-agent-harness/internal/harness	8.592s
ok  	go-agent-harness/internal/harness/tools	18.397s
ok  	go-agent-harness/internal/harness/tools/core	2.836s
ok  	go-agent-harness/internal/harness/tools/deferred	11.848s
ok  	go-agent-harness/internal/harness/tools/descriptions	1.553s
ok  	go-agent-harness/internal/harness/tools/recipe	2.377s
ok  	go-agent-harness/internal/harness/tools/script	4.219s
ok  	go-agent-harness/cmd/harnesscli	8.336s

Cross-compile check for the linux-only sandbox_linux.go/sandbox_linux_test.go (this session is on macOS, so linux tests cannot execute here, only compile-verify):

GOOS=linux GOARCH=amd64 go build ./internal/harness/... ./cmd/harnesscli/...
GOOS=linux GOARCH=amd64 go vet ./internal/harness/... ./cmd/harnesscli/...
GOOS=linux GOARCH=amd64 go test -c -o /dev/null ./internal/harness/tools/
(all clean)

Not verified: actual bubblewrap enforcement on a real Linux host (no such host available in this session) — the linux implementation mirrors the darwin one's now-verified explicit-allow/explicit-deny pattern, and the linux test suite (updated in the red commit) asserts the correct --unshare-net presence/absence, but nobody has run it against a real bwrap binary in this PR.

Real-world manual browser/UI check: not applicable — this is a server/CLI-only change with no UI surface.

Rollout and rollback

No migration or data change. This is a pure default-behavior plus additive-field change, gated entirely by process restart: rebuild harnessd/harnesscli (scripts/install.sh) and restart the daemon to pick it up — merging alone does not change a running process's behavior. Rollback is a plain revert of this PR (or the single commit cdbc73dc plus its docs); no persisted state needs repair since PermissionConfig.Network is per-run, not persisted across restarts. Observability: bash tool results now carry sandbox_network, so an operator can see the applied policy in tool output without extra instrumentation.

Documentation

Updated: website/docs/concepts/tools-and-permissions.md, website/docs/reference/tools-catalog.md, website/docs/reference/glossary.md, website/docs/reference/http-routes.md, website/docs/server/http-api-guide.md, website/docs/reference/cli-flags.md, website/docs/cli/harnesscli.md, docs/logs/engineering-log.md (new dated entry, security-relevant default change called out explicitly).

Checked and left unchanged with reason: website/docs/server/harnessd.md and docs/runbooks/benchmark-smoke.md (named in the task) contain no bash-sandbox-network statements; their "no network" mentions are about the fake provider needing no network calls, unrelated. docs/runbooks/profile-authoring.md's allow_net_access is a distinct profile-capability mechanism (ActionFetch/ActionDownload gating), not this PR's OS-level bash sandbox axis.

Contract checklist

• Linked issue follows the current structured contract and this PR closes it
• Issue acceptance criteria, impact map, and scope were updated when the design changed — N/A, no design change occurred beyond the one documented deviation (CheckSandboxCommand's local-scope heuristic), explained above
• All callers, consumers, sources of truth, and similar abstractions were searched
• No unrelated cleanup, hidden scope growth, duplicated wiring, or parallel abstraction was introduced
• Tests were written first and the expected red failure was observed, or this is a strictly docs-only minor PR
• Targeted checks and the repository-required full regression are green
• Security, compatibility, lifecycle, deployment, observability, documentation, and rollback were reconciled
• Real mouse/keyboard/API/operator behavior was exercised when the change is interaction- or integration-heavy — real sandbox-exec/curl against a live host, both via the Go test suite and manual raw-profile verification

🤖 Generated with Claude Code

https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.