diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index a570ac9ec..f30a68404 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -112,8 +112,70 @@ var ( "/dev/stdout": true, "/dev/stderr": true, } + + quotedHeredocStartPattern = regexp.MustCompile(`<<-?\s*(?:'([^'\s]+)'|"([^"\s]+)")`) ) +func stripQuotedHeredocBodies(command string) string { + sanitized := command + searchFrom := 0 + + for { + loc := quotedHeredocStartPattern.FindStringSubmatchIndex(sanitized[searchFrom:]) + if loc == nil { + return sanitized + } + + matchStart := searchFrom + loc[0] + matchEnd := searchFrom + loc[1] + delimStart := searchFrom + loc[2] + delimEnd := searchFrom + loc[3] + if delimStart == searchFrom-1 || delimEnd == searchFrom-1 { + delimStart = searchFrom + loc[4] + delimEnd = searchFrom + loc[5] + } + delim := sanitized[delimStart:delimEnd] + allowTabs := strings.Contains(sanitized[matchStart:matchEnd], "<<-") + + newlineRel := strings.IndexByte(sanitized[matchEnd:], '\n') + if newlineRel == -1 { + return sanitized + } + bodyStart := matchEnd + newlineRel + 1 + lineStart := bodyStart + foundEnd := false + + for lineStart <= len(sanitized) { + lineEnd := len(sanitized) + if nextNewline := strings.IndexByte(sanitized[lineStart:], '\n'); nextNewline >= 0 { + lineEnd = lineStart + nextNewline + } + + line := sanitized[lineStart:lineEnd] + compare := strings.TrimSuffix(line, "\r") + if allowTabs { + compare = strings.TrimLeft(compare, "\t") + } + if compare == delim { + sanitized = sanitized[:bodyStart] + "[quoted heredoc omitted]\n" + sanitized[lineStart:] + searchFrom = bodyStart + len("[quoted heredoc omitted]\n") + foundEnd = true + break + } + + if lineEnd == len(sanitized) { + lineStart = len(sanitized) + 1 + } else { + lineStart = lineEnd + 1 + } + } + + if !foundEnd { + return sanitized + } + } +} + func NewExecTool(workingDir string, restrict bool, allowPaths ...[]*regexp.Regexp) (*ExecTool, error) { return NewExecToolWithConfig(workingDir, restrict, nil, allowPaths...) } @@ -1022,6 +1084,7 @@ func (t *ExecTool) executeSendKeys(args map[string]any) *ToolResult { func (t *ExecTool) guardCommand(command, cwd string) string { cmd := strings.TrimSpace(command) lower := strings.ToLower(cmd) + lowerForDeny := strings.ToLower(stripQuotedHeredocBodies(cmd)) // Custom allow patterns exempt a command from deny checks. explicitlyAllowed := false @@ -1034,7 +1097,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string { if !explicitlyAllowed { for _, pattern := range t.denyPatterns { - if pattern.MatchString(lower) { + if pattern.MatchString(lowerForDeny) { return "Command blocked by safety guard (dangerous pattern detected)" } } diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index a8de2f4c9..f5232cab7 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -181,6 +181,37 @@ func TestShellTool_DangerousCommand_KillBlocked(t *testing.T) { } } +func TestShellTool_BackticksInsideQuotedHeredocAreAllowed(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + command := "gh pr comment 2763 --body-file - <<'TXT'\n" + + "Fixed `pkg/tools/integration/web_test.go` and `brave` is now expected.\n" + + "TXT" + + guardError := tool.guardCommand(command, "") + if guardError != "" { + t.Fatalf("quoted heredoc body should not be blocked, got: %s", guardError) + } +} + +func TestShellTool_BackticksOutsideQuotedHeredocRemainBlocked(t *testing.T) { + tool, err := NewExecTool("", false) + require.NoError(t, err) + + result := tool.Execute(context.Background(), map[string]any{ + "action": "run", + "command": "echo `whoami`", + }) + + if !result.IsError { + t.Fatal("expected raw backtick command substitution to be blocked") + } + if !strings.Contains(result.ForLLM, "blocked") { + t.Fatalf("expected blocked message, got: %s", result.ForLLM) + } +} + // TestShellTool_MissingCommand verifies error handling for missing command func TestShellTool_MissingCommand(t *testing.T) { tool, err := NewExecTool("", false)