diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index fe9cb1366..d13d8f9ce 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -79,10 +79,6 @@ var ( // absolutePathPattern matches absolute file paths in commands (Unix and Windows). absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`) - // httpURLPattern matches HTTP(S) URLs so they can be excluded from path-based - // workspace checks. URLs are command arguments, not local filesystem paths. - httpURLPattern = regexp.MustCompile(`(?i)\bhttps?://[^\s"'<>]+`) - // safePaths are kernel pseudo-devices that are always safe to reference in // commands, regardless of workspace restriction. They contain no user data // and cannot cause destructive writes. @@ -369,9 +365,89 @@ func (t *ExecTool) guardCommand(command, cwd string) string { } func stripHTTPURLs(command string) string { - return httpURLPattern.ReplaceAllStringFunc(command, func(match string) string { - return strings.Repeat(" ", len(match)) - }) + sanitized := []byte(command) + quote := byte(0) + + for i := 0; i < len(command); i++ { + ch := command[i] + + switch quote { + case '\'': + if ch == '\'' { + quote = 0 + continue + } + case '"': + if ch == '\\' && i+1 < len(command) { + i++ + continue + } + if ch == '"' { + quote = 0 + continue + } + default: + if ch == '\\' && i+1 < len(command) { + i++ + continue + } + if ch == '\'' || ch == '"' { + quote = ch + continue + } + } + + if !hasHTTPURLPrefix(command, i) { + continue + } + + end := findHTTPURLEnd(command, i, quote) + for j := i; j < end; j++ { + sanitized[j] = ' ' + } + i = end - 1 + } + + return string(sanitized) +} + +func hasHTTPURLPrefix(command string, start int) bool { + return len(command)-start >= len("http://") && + (strings.EqualFold(command[start:start+len("http://")], "http://") || + (len(command)-start >= len("https://") && + strings.EqualFold(command[start:start+len("https://")], "https://"))) +} + +func findHTTPURLEnd(command string, start int, quote byte) int { + if quote != 0 { + for i := start; i < len(command); i++ { + if quote == '"' && command[i] == '\\' && i+1 < len(command) { + i++ + continue + } + if command[i] == quote { + return i + } + } + return len(command) + } + + for i := start; i < len(command); i++ { + if strings.ContainsRune(" \t\r\n", rune(command[i])) || isShellURLDelimiter(command[i]) { + return i + } + } + + return len(command) +} + +func isShellURLDelimiter(ch byte) bool { + switch ch { + case ';', '|', '&', '<', '>', '(', ')', '`': + return true + default: + return false + } } func (t *ExecTool) SetTimeout(timeout time.Duration) { diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index 0f825cdd2..0002b02ab 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -325,6 +325,8 @@ func TestShellTool_RestrictToWorkspace_AllowsHTTPURLs(t *testing.T) { `echo "http://example.com"`, `echo "https://test"`, `echo "https://example.com/path/../still-a-url"`, + `echo "https://example.com?a=1&b=/still-a-url"`, + `echo 'https://example.com;/bin/sh'`, } for _, cmd := range commands { @@ -335,6 +337,28 @@ func TestShellTool_RestrictToWorkspace_AllowsHTTPURLs(t *testing.T) { } } +// TestShellTool_RestrictToWorkspace_URLDoesNotBypassPathChecks verifies that +// URLs do not hide real paths that should still be blocked by the workspace guard. +func TestShellTool_RestrictToWorkspace_URLDoesNotBypassPathChecks(t *testing.T) { + tmpDir := t.TempDir() + tool, err := NewExecTool(tmpDir, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + commands := []string{ + `echo https://example.com /etc/passwd`, + `echo https://x;/bin/sh -c whoami`, + } + + for _, cmd := range commands { + result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + if !result.IsError || !strings.Contains(result.ForLLM, "blocked") { + t.Fatalf("expected command to stay blocked, command=%q output=%s", cmd, result.ForLLM) + } + } +} + // TestShellTool_DevNullAllowed verifies that /dev/null redirections are not blocked (issue #964). func TestShellTool_DevNullAllowed(t *testing.T) { tmpDir := t.TempDir()