diff --git a/pkg/tools/sandbox.go b/pkg/tools/sandbox.go index 7f670911b..4a6306393 100644 --- a/pkg/tools/sandbox.go +++ b/pkg/tools/sandbox.go @@ -22,8 +22,8 @@ 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 + AllowRules []string // Command prefix allowlist (e.g., "go test", "pnpm run test") + LocalNetOnly bool // Restrict curl/wget to localhost and RFC 1918 private addresses } // SandboxConfig describes the sandbox isolation policy for a preset. @@ -44,15 +44,43 @@ type SubagentEnvironment struct { ContextFiles []string // Files to provide as context } -// presetExecPatterns maps presets to command allowlist regexes. +// presetAllowRules maps presets to command prefix allowlists. +// Each entry is a command prefix: the first N words of the executed command +// must match exactly. e.g. "go test" allows "go test ./..." but not "go build". +// A single word like "curl" allows any arguments. // 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+|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`, +var presetAllowRules = map[Preset][]string{ + PresetScout: nil, // No exec allowed + PresetAnalyst: { + "go test", "go vet", + "git log", "git diff", "git status", + "curl", "wget", "grep", "find", + }, + PresetCoder: { + "go test", "go vet", "go fmt", + "gofmt", "goimports", "golangci-lint", + "prettier", "eslint", "black", "ruff", + "cargo test", "cargo fmt", "cargo clippy", + "pnpm test", "pnpm run test", "pnpm run lint", "pnpm run format", + "bun test", "bun run test", "bun run lint", "bun run format", + "uv run", + "curl", "wget", + }, + PresetWorker: { + "go", + "pnpm install", "pnpm add", "pnpm run", "pnpm test", "pnpm build", + "bun install", "bun add", "bun run", "bun test", "bun build", + "uv run", "uv sync", "uv add", "uv pip install", + "pip install", + "cargo", + "curl", "wget", + }, + PresetCoordinator: { + "go", + "pnpm", "bun", + "curl", "wget", + }, } // presetSpawnablePresets maps presets to which presets they can spawn. @@ -110,13 +138,13 @@ 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 rules are defined. // 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 != "" { + if rules := presetAllowRules[p]; len(rules) > 0 { config.ExecPolicy = &ExecPolicy{ - AllowPattern: pattern, + AllowRules: rules, LocalNetOnly: true, } } diff --git a/pkg/tools/sandbox_test.go b/pkg/tools/sandbox_test.go index 74b009f25..3d4b37788 100644 --- a/pkg/tools/sandbox_test.go +++ b/pkg/tools/sandbox_test.go @@ -1,7 +1,6 @@ package tools import ( - "regexp" "testing" ) @@ -167,17 +166,15 @@ func TestSandboxConfigForPreset_Coordinator(t *testing.T) { } } -// TestPresetExecPatterns_Coder validates coder exec allowlist. -func TestPresetExecPatterns_Coder(t *testing.T) { - pattern, ok := presetExecPatterns[PresetCoder] - if !ok || pattern == "" { - t.Fatalf("coder pattern missing or empty") +// TestPresetAllowRules_Coder validates coder exec allowlist. +func TestPresetAllowRules_Coder(t *testing.T) { + rules := presetAllowRules[PresetCoder] + if len(rules) == 0 { + t.Fatalf("coder rules missing or empty") } - re, err := regexp.Compile(pattern) - if err != nil { - t.Fatalf("failed to compile pattern: %v", err) - } + exec := NewExecTool(t.TempDir(), true) + exec.SetAllowRules(rules) tests := []struct { cmd string @@ -185,35 +182,50 @@ func TestPresetExecPatterns_Coder(t *testing.T) { }{ {"go test ./...", true}, {"go vet ./...", true}, + {"go fmt ./...", true}, {"gofmt -w file.go", true}, {"golangci-lint run", true}, - {"go build ./...", false}, - {"npm install", false}, + {"cargo test", true}, + {"cargo fmt", true}, + {"cargo clippy", true}, {"pnpm test", true}, + {"pnpm run test", true}, + {"pnpm run lint", true}, + {"pnpm run format", true}, + {"bun test", true}, + {"bun run test", true}, + {"uv run pytest", 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}, + // blocked + {"go build ./...", false}, + {"npm install", false}, + {"pnpm install", false}, + {"pnpm run build", false}, + {"cargo build", false}, + {"pwd", false}, + {"ls", false}, } for _, tt := range tests { - gotOK := re.MatchString(tt.cmd) + result := exec.guardCommand(tt.cmd, t.TempDir()) + gotOK := result == "" if gotOK != tt.wantOK { - t.Errorf("cmd %q: got %v, want %v", tt.cmd, gotOK, tt.wantOK) + t.Errorf("cmd %q: got allowed=%v, want %v (guard: %q)", tt.cmd, gotOK, tt.wantOK, result) } } } -// TestPresetExecPatterns_Analyst validates analyst exec allowlist. -func TestPresetExecPatterns_Analyst(t *testing.T) { - pattern, ok := presetExecPatterns[PresetAnalyst] - if !ok || pattern == "" { - t.Fatalf("analyst pattern missing or empty") +// TestPresetAllowRules_Analyst validates analyst exec allowlist. +func TestPresetAllowRules_Analyst(t *testing.T) { + rules := presetAllowRules[PresetAnalyst] + if len(rules) == 0 { + t.Fatalf("analyst rules missing or empty") } - re, err := regexp.Compile(pattern) - if err != nil { - t.Fatalf("failed to compile pattern: %v", err) - } + exec := NewExecTool(t.TempDir(), true) + exec.SetAllowRules(rules) tests := []struct { cmd string @@ -223,16 +235,103 @@ func TestPresetExecPatterns_Analyst(t *testing.T) { {"go vet ./...", true}, {"git log --oneline", true}, {"git diff HEAD", true}, + {"git status", true}, {"grep pattern file", true}, + {"find . -name '*.go'", true}, {"curl http://example.com", true}, + // blocked {"go build ./...", false}, {"npm install", false}, + {"git push", false}, + {"git checkout", false}, + {"pwd", false}, + {"ls", false}, } for _, tt := range tests { - gotOK := re.MatchString(tt.cmd) + result := exec.guardCommand(tt.cmd, t.TempDir()) + gotOK := result == "" if gotOK != tt.wantOK { - t.Errorf("cmd %q: got %v, want %v", tt.cmd, gotOK, tt.wantOK) + t.Errorf("cmd %q: got allowed=%v, want %v (guard: %q)", tt.cmd, gotOK, tt.wantOK, result) + } + } +} + +// TestPresetAllowRules_Worker validates worker exec allowlist. +func TestPresetAllowRules_Worker(t *testing.T) { + rules := presetAllowRules[PresetWorker] + if len(rules) == 0 { + t.Fatalf("worker rules missing or empty") + } + + exec := NewExecTool(t.TempDir(), true) + exec.SetAllowRules(rules) + + tests := []struct { + cmd string + wantOK bool + }{ + {"go build ./...", true}, + {"go test ./...", true}, + {"pnpm install", true}, + {"pnpm add lodash", true}, + {"pnpm run dev", true}, + {"bun install", true}, + {"bun build", true}, + {"uv run pytest", true}, + {"uv sync", true}, + {"uv pip install flask", true}, + {"pip install flask", true}, + {"cargo build", true}, + {"cargo test", true}, + {"curl http://localhost:8080", true}, + // blocked + {"npm install", false}, + {"pwd", false}, + } + + for _, tt := range tests { + result := exec.guardCommand(tt.cmd, t.TempDir()) + gotOK := result == "" + if gotOK != tt.wantOK { + t.Errorf("cmd %q: got allowed=%v, want %v (guard: %q)", tt.cmd, gotOK, tt.wantOK, result) + } + } +} + +// TestPresetAllowRules_Coordinator validates coordinator exec allowlist. +func TestPresetAllowRules_Coordinator(t *testing.T) { + rules := presetAllowRules[PresetCoordinator] + if len(rules) == 0 { + t.Fatalf("coordinator rules missing or empty") + } + + exec := NewExecTool(t.TempDir(), true) + exec.SetAllowRules(rules) + + tests := []struct { + cmd string + wantOK bool + }{ + {"go build ./...", true}, + {"go test ./...", true}, + {"pnpm install", true}, + {"pnpm run dev", true}, + {"bun install", true}, + {"bun run dev", true}, + {"curl http://localhost:8080", true}, + {"wget http://127.0.0.1:3000", true}, + // blocked + {"npm install", false}, + {"cargo build", false}, + {"pwd", false}, + } + + for _, tt := range tests { + result := exec.guardCommand(tt.cmd, t.TempDir()) + gotOK := result == "" + if gotOK != tt.wantOK { + t.Errorf("cmd %q: got allowed=%v, want %v (guard: %q)", tt.cmd, gotOK, tt.wantOK, result) } } } diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 4aced8b45..232976985 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -122,7 +122,7 @@ type ExecTool struct { workingDir string timeout time.Duration denyPatterns []*regexp.Regexp - allowPatterns []*regexp.Regexp + allowRules [][]string // pre-split command prefix allowlist restrictToWorkspace bool localNetOnly bool // restrict curl/wget to localhost + RFC 1918 @@ -217,7 +217,7 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf workingDir: workingDir, timeout: 5 * time.Minute, denyPatterns: denyPatterns, - allowPatterns: nil, + allowRules: nil, restrictToWorkspace: restrict, bgProcesses: make(map[string]*bgProcess), bgCtx: bgCtx, @@ -708,22 +708,15 @@ func (t *ExecTool) guardCommand(command, cwd string) string { } } - if len(t.allowPatterns) > 0 { - allowed := false - for _, pattern := range t.allowPatterns { - if pattern.MatchString(lower) { - allowed = true - break - } - } - if !allowed { + if len(t.allowRules) > 0 { + if !matchAllowRules(lower, t.allowRules) { var b strings.Builder b.WriteString("Command blocked: not in allowlist [") - for i, p := range t.allowPatterns { + for i, rule := range t.allowRules { if i > 0 { b.WriteByte(',') } - b.WriteString(p.String()) + b.WriteString(strings.Join(rule, " ")) } b.WriteByte(']') return b.String() @@ -847,16 +840,38 @@ func (t *ExecTool) SetRestrictToWorkspace(restrict bool) { t.restrictToWorkspace = restrict } -func (t *ExecTool) SetAllowPatterns(patterns []string) error { - t.allowPatterns = make([]*regexp.Regexp, 0, len(patterns)) - for _, p := range patterns { - re, err := regexp.Compile(p) - if err != nil { - return fmt.Errorf("invalid allow pattern %q: %w", p, err) +// SetAllowRules sets the command prefix allowlist. +// Each rule is a space-separated command prefix (e.g. "go test", "pnpm run lint"). +// A command is allowed if its first N words match any rule's N words exactly. +func (t *ExecTool) SetAllowRules(rules []string) { + t.allowRules = make([][]string, 0, len(rules)) + for _, r := range rules { + words := strings.Fields(strings.ToLower(r)) + if len(words) > 0 { + t.allowRules = append(t.allowRules, words) } - t.allowPatterns = append(t.allowPatterns, re) } - return nil +} + +// matchAllowRules checks if cmd matches any prefix in the allowlist. +func matchAllowRules(cmd string, rules [][]string) bool { + cmdWords := strings.Fields(cmd) + for _, ruleWords := range rules { + if len(cmdWords) < len(ruleWords) { + continue + } + match := true + for i, rw := range ruleWords { + if cmdWords[i] != rw { + match = false + break + } + } + if match { + return true + } + } + return false } func (t *ExecTool) SetLocalNetOnly(v bool) { diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go index 33938c200..d37ac60f5 100644 --- a/pkg/tools/subagent.go +++ b/pkg/tools/subagent.go @@ -300,7 +300,7 @@ func (sm *SubagentManager) buildPresetRegistry(preset Preset, writeRoot string) // Register exec and bg_monitor if allowed. // Each subagent gets its own ExecTool to avoid mutating the shared instance's - // allowPatterns (which would leak sandbox restrictions to the conductor). + // allowRules (which would leak sandbox restrictions to the conductor). if config.AllowedTools["exec"] { execWorkDir := writeRoot if execWorkDir == "" { @@ -312,7 +312,7 @@ func (sm *SubagentManager) buildPresetRegistry(preset Preset, writeRoot string) return registry } if config.ExecPolicy != nil { - _ = execTool.SetAllowPatterns([]string{config.ExecPolicy.AllowPattern}) + execTool.SetAllowRules(config.ExecPolicy.AllowRules) execTool.SetLocalNetOnly(config.ExecPolicy.LocalNetOnly) } registry.Register(execTool)