refactor(sandbox): replace regex exec allowlist with command prefix dictionary
Replace the regex-based presetExecPatterns with presetAllowRules, a dictionary of command prefix strings (e.g. "go test", "pnpm run lint"). Benefits: - Readable at a glance without decoding regex alternations - No regex compilation; matching is a simple word-prefix check - Easy to extend with new commands per preset - Each preset's permissions are an explicit list ExecPolicy.AllowPattern (string) → ExecPolicy.AllowRules ([]string). ExecTool.SetAllowPatterns → ExecTool.SetAllowRules. Tests rewritten to use guardCommand directly instead of regex matching. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
742c53e0f8
commit
31caaf3836
4 changed files with 202 additions and 60 deletions
|
|
@ -22,7 +22,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
|
||||
AllowRules []string // Command prefix allowlist (e.g., "go test", "pnpm run test")
|
||||
LocalNetOnly bool // Restrict curl/wget to localhost and RFC 1918 private addresses
|
||||
}
|
||||
|
||||
|
|
@ -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,
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue