diff --git a/pkg/tools/sandbox.go b/pkg/tools/sandbox.go index 3f5d0455f..7f670911b 100644 --- a/pkg/tools/sandbox.go +++ b/pkg/tools/sandbox.go @@ -23,6 +23,7 @@ func IsValidPreset(p Preset) bool { // ExecPolicy defines which commands are allowed for execution. type ExecPolicy struct { AllowPattern string // Prefix-match regex; matched commands are allowed + LocalNetOnly bool // Restrict curl/wget to localhost and RFC 1918 private addresses } // SandboxConfig describes the sandbox isolation policy for a preset. @@ -44,11 +45,13 @@ type SubagentEnvironment struct { } // presetExecPatterns maps presets to command allowlist regexes. +// curl/wget are included where exec is allowed; LocalNetOnly in ExecPolicy +// ensures all curl/wget requests are restricted to localhost and RFC 1918 addresses. var presetExecPatterns = map[Preset]string{ PresetScout: ``, // No exec allowed PresetAnalyst: `^(go\s+(test|vet)|git\s+(log|diff|status)|curl|wget|grep|find)\b`, - PresetCoder: `^(go\s+(test|vet|fmt)|gofmt|goimports|golangci-lint|prettier|eslint|black|ruff|cargo\s+(test|fmt|clippy)|pnpm\s+(test|run\s+(test|lint|format))|bun\s+(test|run\s+(test|lint|format))|uv\s+run\s+)\b`, - PresetWorker: `^(go\s+|pnpm\s+(install|add|run|test|build)|bun\s+(install|add|run|test|build)|uv\s+(run|sync|add|pip\s+install)|pip\s+install|cargo\s+)\b`, + PresetCoder: `^(go\s+(test|vet|fmt)|gofmt|goimports|golangci-lint|prettier|eslint|black|ruff|cargo\s+(test|fmt|clippy)|pnpm\s+(test|run\s+(test|lint|format))|bun\s+(test|run\s+(test|lint|format))|uv\s+run\s+|curl|wget)\b`, + PresetWorker: `^(go\s+|pnpm\s+(install|add|run|test|build)|bun\s+(install|add|run|test|build)|uv\s+(run|sync|add|pip\s+install)|pip\s+install|cargo\s+|curl|wget)\b`, PresetCoordinator: `^(go\s+|pnpm\s+|bun\s+|curl|wget)\b`, } @@ -107,11 +110,14 @@ func SandboxConfigForPreset(p Preset, writeRoot string) SandboxConfig { config.WriteRoot = writeRoot } - // Set ExecPolicy if exec is allowed and pattern is non-empty + // Set ExecPolicy if exec is allowed and pattern is non-empty. + // LocalNetOnly is always true: curl/wget in subagents is for local server + // testing only; external HTTP access goes through the web_fetch tool. if allowed["exec"] { if pattern := presetExecPatterns[p]; pattern != "" { config.ExecPolicy = &ExecPolicy{ AllowPattern: pattern, + LocalNetOnly: true, } } } diff --git a/pkg/tools/sandbox_test.go b/pkg/tools/sandbox_test.go index c63d93e72..74b009f25 100644 --- a/pkg/tools/sandbox_test.go +++ b/pkg/tools/sandbox_test.go @@ -128,6 +128,8 @@ func TestSandboxConfigForPreset_Coder(t *testing.T) { } if config.ExecPolicy == nil { t.Errorf("ExecPolicy: got nil, want non-nil") + } else if !config.ExecPolicy.LocalNetOnly { + t.Errorf("ExecPolicy.LocalNetOnly: got false, want true") } if config.SpawnablePresets != nil { t.Errorf("SpawnablePresets: got non-nil, want nil") @@ -146,6 +148,8 @@ func TestSandboxConfigForPreset_Coordinator(t *testing.T) { } if config.ExecPolicy == nil { t.Errorf("ExecPolicy: got nil, want non-nil") + } else if !config.ExecPolicy.LocalNetOnly { + t.Errorf("ExecPolicy.LocalNetOnly: got false, want true") } if config.SpawnablePresets == nil { t.Errorf("SpawnablePresets: got nil, want non-nil") @@ -186,6 +190,9 @@ func TestPresetExecPatterns_Coder(t *testing.T) { {"go build ./...", false}, {"npm install", false}, {"pnpm test", true}, + // curl/wget are in the allowlist; LocalNetOnly enforcement is at runtime + {"curl http://localhost:3000/health", true}, + {"wget http://127.0.0.1:8080/status", true}, } for _, tt := range tests { diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 9f7091614..f0a3307c4 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -6,6 +6,8 @@ import ( "errors" "fmt" "io" + "net" + "net/url" "os" "os/exec" "path/filepath" @@ -122,6 +124,7 @@ type ExecTool struct { denyPatterns []*regexp.Regexp allowPatterns []*regexp.Regexp restrictToWorkspace bool + localNetOnly bool // restrict curl/wget to localhost + RFC 1918 // Background process management bgMu sync.Mutex @@ -727,6 +730,14 @@ func (t *ExecTool) guardCommand(command, cwd string) string { } } + // Restrict curl/wget to localhost and RFC 1918 private addresses. + // External HTTP access is available via the web_fetch tool. + if t.localNetOnly && isCurlOrWget(cmd) { + if errMsg := checkCurlLocalNet(cmd); errMsg != "" { + return errMsg + } + } + if t.restrictToWorkspace { if strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") { return "Command blocked by safety guard (path traversal detected)" @@ -842,6 +853,54 @@ func (t *ExecTool) SetAllowPatterns(patterns []string) error { return nil } +func (t *ExecTool) SetLocalNetOnly(v bool) { + t.localNetOnly = v +} + +// isCurlOrWget reports whether command is a curl or wget invocation. +func isCurlOrWget(command string) bool { + fields := strings.Fields(command) + if len(fields) == 0 { + return false + } + base := filepath.Base(fields[0]) + return base == "curl" || base == "wget" +} + +// checkCurlLocalNet validates that all http/https URLs in a curl/wget command +// target localhost or RFC 1918 private addresses. +// Returns an error message string, or empty string if the command is allowed. +func checkCurlLocalNet(command string) string { + for _, token := range strings.Fields(command) { + token = strings.Trim(token, "\"'") + if !strings.HasPrefix(token, "http://") && !strings.HasPrefix(token, "https://") { + continue + } + u, err := url.Parse(token) + if err != nil { + continue + } + host := u.Hostname() + if !isLocalHost(host) { + return fmt.Sprintf("Command blocked by safety guard (curl/wget is restricted to localhost and private network; %q is a public address)", host) + } + } + return "" +} + +// isLocalHost reports whether host is localhost or a loopback/RFC 1918 private IP. +// DNS resolution is intentionally avoided to prevent DNS rebinding attacks. +func isLocalHost(host string) bool { + if strings.EqualFold(host, "localhost") { + return true + } + ip := net.ParseIP(host) + if ip == nil { + return false + } + return ip.IsLoopback() || ip.IsPrivate() +} + // SetBgMaxLifetimeForTest overrides bgMaxLifetime for testing purposes. // This is exposed only for tests; the returned function restores the original value. var bgMaxLifetimeOverride time.Duration diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index 6bf7e05a9..20ea7e23e 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -983,3 +983,97 @@ func TestExecTool_Bg_RingBufferOverflow(t *testing.T) { t.Errorf("ring buffer exceeded max size: %d > %d", bufLen, bgRingBufSize) } } + +// TestIsLocalHost verifies localhost and RFC 1918 detection using net package. +func TestIsLocalHost(t *testing.T) { + tests := []struct { + host string + want bool + }{ + // Loopback / localhost + {"localhost", true}, + {"LOCALHOST", true}, + {"127.0.0.1", true}, + {"127.0.0.2", true}, + {"::1", true}, + // RFC 1918 private ranges + {"10.0.0.1", true}, + {"10.255.255.255", true}, + {"172.16.0.1", true}, + {"172.31.255.255", true}, + {"192.168.0.1", true}, + {"192.168.1.100", true}, + // Public addresses + {"8.8.8.8", false}, + {"1.1.1.1", false}, + {"example.com", false}, + {"api.github.com", false}, + // Edge: non-private but routable private-looking address + {"172.15.255.255", false}, // just below 172.16/12 + {"172.32.0.0", false}, // just above 172.31/12 + } + + for _, tt := range tests { + got := isLocalHost(tt.host) + if got != tt.want { + t.Errorf("isLocalHost(%q) = %v, want %v", tt.host, got, tt.want) + } + } +} + +// TestCheckCurlLocalNet verifies URL-level enforcement for curl/wget commands. +func TestCheckCurlLocalNet(t *testing.T) { + tests := []struct { + cmd string + wantErr bool + }{ + // Allowed: localhost and private IPs + {"curl http://localhost:3000/health", false}, + {"curl -v http://127.0.0.1:8080/api/status", false}, + {"wget http://192.168.1.10/file.bin", false}, + {"curl -X POST http://10.0.0.5:9000/webhook", false}, + // Blocked: public addresses + {"curl http://example.com", true}, + {"wget https://releases.github.com/v1.tar.gz", true}, + {"curl http://8.8.8.8/data", true}, + // Allowed: no http URL (e.g. --help, --version — no network access) + {"curl --help", false}, + {"curl --version", false}, + {"wget --help", false}, + } + + for _, tt := range tests { + errMsg := checkCurlLocalNet(tt.cmd) + gotErr := errMsg != "" + if gotErr != tt.wantErr { + t.Errorf("checkCurlLocalNet(%q): gotErr=%v wantErr=%v (msg: %q)", + tt.cmd, gotErr, tt.wantErr, errMsg) + } + } +} + +// TestExecTool_LocalNetOnly verifies curl/wget blocking via SetLocalNetOnly. +func TestExecTool_LocalNetOnly(t *testing.T) { + tool := NewExecTool("", false) + tool.SetLocalNetOnly(true) + + tests := []struct { + cmd string + wantErr bool + }{ + {"curl http://localhost:3000", false}, + {"curl http://example.com", true}, + {"echo hello", false}, // non-curl not affected + } + + ctx := context.Background() + for _, tt := range tests { + result := tool.Execute(ctx, map[string]any{"command": tt.cmd}) + if tt.wantErr && !result.IsError { + t.Errorf("cmd %q: expected blocked, but succeeded", tt.cmd) + } + if !tt.wantErr && result.IsError && strings.Contains(result.ForLLM, "safety guard") { + t.Errorf("cmd %q: expected allowed, but safety guard blocked: %s", tt.cmd, result.ForLLM) + } + } +} diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go index 08cbb0622..bfe8436f8 100644 --- a/pkg/tools/subagent.go +++ b/pkg/tools/subagent.go @@ -312,6 +312,7 @@ func (sm *SubagentManager) buildPresetRegistry(preset Preset, writeRoot string) execTool := sm.execTool if config.ExecPolicy != nil { _ = execTool.SetAllowPatterns([]string{config.ExecPolicy.AllowPattern}) + execTool.SetLocalNetOnly(config.ExecPolicy.LocalNetOnly) } registry.Register(execTool)