From 9721c36e55bf77c6628f8f80df78cc791d5cf65d Mon Sep 17 00:00:00 2001 From: danmobot <270805001+danmobot@users.noreply.github.com> Date: Tue, 23 Jun 2026 02:53:14 +0100 Subject: [PATCH 1/2] fix(exec): keep deny patterns active for custom allow rules --- pkg/providers/openai_compat/provider.go | 1 + pkg/tools/shell.go | 43 ++++++++++++------------- pkg/tools/shell_test.go | 34 +++++++++++++++++++ 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/pkg/providers/openai_compat/provider.go b/pkg/providers/openai_compat/provider.go index c12a8348f..6a1b73d8a 100644 --- a/pkg/providers/openai_compat/provider.go +++ b/pkg/providers/openai_compat/provider.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "io" + "log" "maps" "net/http" "net/url" diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 85bdcb8c3..3d5a97fe8 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -1135,36 +1135,35 @@ func expandPowerShellEnvVars(cmd string) string { }) } +func (t *ExecTool) commandMatchesAllowPattern(lower string) bool { + for _, pattern := range t.allowPatterns { + if pattern.MatchString(lower) { + return true + } + } + for _, pattern := range t.customAllowPatterns { + if pattern.MatchString(lower) { + return true + } + } + return false +} + func (t *ExecTool) guardCommand(command, cwd string) string { cmd := strings.TrimSpace(command) lower := strings.ToLower(cmd) - // Custom allow patterns exempt a command from deny checks. - explicitlyAllowed := false - for _, pattern := range t.customAllowPatterns { + // Deny patterns always apply, even when a command matches a custom allow rule. + // Custom allow rules can permit a command, but must not disable secret-safety + // deny rules such as jq env access checks (#3079). + for _, pattern := range t.denyPatterns { if pattern.MatchString(lower) { - explicitlyAllowed = true - break + return "Command blocked by safety guard (dangerous pattern detected)" } } - if !explicitlyAllowed { - for _, pattern := range t.denyPatterns { - if pattern.MatchString(lower) { - return "Command blocked by safety guard (dangerous pattern detected)" - } - } - } - - if len(t.allowPatterns) > 0 { - allowed := false - for _, pattern := range t.allowPatterns { - if pattern.MatchString(lower) { - allowed = true - break - } - } - if !allowed { + if len(t.allowPatterns) > 0 || len(t.customAllowPatterns) > 0 { + if !t.commandMatchesAllowPattern(lower) { return "Command blocked by safety guard (not in allowlist)" } } diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index cdc78b7fc..5312283b0 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -1936,3 +1936,37 @@ func TestShellTool_SchemelessURLDetection(t *testing.T) { } } } + +func TestShellTool_CustomAllowDoesNotBypassDenyPatterns(t *testing.T) { + cfg := &config.Config{} + cfg.Tools.Exec.EnableDenyPatterns = true + cfg.Tools.Exec.CustomAllowPatterns = []string{`^jq\b`} + cfg.Tools.Exec.CustomDenyPatterns = []string{`\$env\b`, `(^|[^.$a-z0-9_])env([^a-z0-9_]|$)`} + + tool, err := NewExecToolWithConfig(t.TempDir(), false, cfg) + if err != nil { + t.Fatalf("NewExecToolWithConfig() error: %v", err) + } + + got := tool.guardCommand(`jq -n '$ENV.PICOCLAW_VARIANT_CANARY'`, t.TempDir()) + if !strings.Contains(got, "dangerous pattern detected") { + t.Fatalf("custom allow should not bypass deny patterns, got: %q", got) + } +} + +func TestShellTool_CustomAllowStillPermitsSafeMatch(t *testing.T) { + cfg := &config.Config{} + cfg.Tools.Exec.EnableDenyPatterns = true + cfg.Tools.Exec.CustomAllowPatterns = []string{`^jq\b`} + cfg.Tools.Exec.CustomDenyPatterns = []string{`\$env\b`, `(^|[^.$a-z0-9_])env([^a-z0-9_]|$)`} + + tool, err := NewExecToolWithConfig(t.TempDir(), false, cfg) + if err != nil { + t.Fatalf("NewExecToolWithConfig() error: %v", err) + } + + got := tool.guardCommand(`jq -n '"ok"'`, t.TempDir()) + if got != "" { + t.Fatalf("safe custom-allowed command should pass guard, got: %q", got) + } +} From ff247b63ac815dce10c0a799caed53194d8193ac Mon Sep 17 00:00:00 2001 From: danmobot <270805001+danmobot@users.noreply.github.com> Date: Tue, 23 Jun 2026 20:47:15 +0100 Subject: [PATCH 2/2] fix(exec): preserve additive custom allow behavior --- pkg/tools/shell.go | 2 +- pkg/tools/shell_test.go | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 3d5a97fe8..95690f163 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -1162,7 +1162,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string { } } - if len(t.allowPatterns) > 0 || len(t.customAllowPatterns) > 0 { + if len(t.allowPatterns) > 0 { if !t.commandMatchesAllowPattern(lower) { return "Command blocked by safety guard (not in allowlist)" } diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index 5312283b0..a3399ede1 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -1970,3 +1970,20 @@ func TestShellTool_CustomAllowStillPermitsSafeMatch(t *testing.T) { t.Fatalf("safe custom-allowed command should pass guard, got: %q", got) } } + +func TestShellTool_CustomAllowDoesNotBecomeStrictAllowlist(t *testing.T) { + cfg := &config.Config{} + cfg.Tools.Exec.EnableDenyPatterns = true + cfg.Tools.Exec.CustomAllowPatterns = []string{`^jq\b`} + cfg.Tools.Exec.CustomDenyPatterns = []string{`\$env\b`, `(^|[^.$a-z0-9_])env([^a-z0-9_]|$)`} + + tool, err := NewExecToolWithConfig(t.TempDir(), false, cfg) + if err != nil { + t.Fatalf("NewExecToolWithConfig() error: %v", err) + } + + got := tool.guardCommand("ls", t.TempDir()) + if got != "" { + t.Fatalf("custom allow patterns should not become a strict allowlist, got: %q", got) + } +}