diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 2ea58b259..fe9cb1366 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -79,6 +79,10 @@ 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. @@ -327,7 +331,9 @@ func (t *ExecTool) guardCommand(command, cwd string) string { } if t.restrictToWorkspace { - if strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") { + sanitizedCmd := stripHTTPURLs(cmd) + + if strings.Contains(sanitizedCmd, "..\\") || strings.Contains(sanitizedCmd, "../") { return "Command blocked by safety guard (path traversal detected)" } @@ -336,7 +342,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string { return "" } - matches := absolutePathPattern.FindAllString(cmd, -1) + matches := absolutePathPattern.FindAllString(sanitizedCmd, -1) for _, raw := range matches { p, err := filepath.Abs(raw) @@ -362,6 +368,12 @@ func (t *ExecTool) guardCommand(command, cwd string) string { return "" } +func stripHTTPURLs(command string) string { + return httpURLPattern.ReplaceAllStringFunc(command, func(match string) string { + return strings.Repeat(" ", len(match)) + }) +} + func (t *ExecTool) SetTimeout(timeout time.Duration) { t.timeout = timeout } diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index a6abca8ea..0f825cdd2 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -312,6 +312,29 @@ func TestShellTool_RestrictToWorkspace(t *testing.T) { } } +// TestShellTool_RestrictToWorkspace_AllowsHTTPURLs verifies that HTTP(S) URLs +// are not mistaken for absolute filesystem paths when workspace restriction is active. +func TestShellTool_RestrictToWorkspace_AllowsHTTPURLs(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 "http://example.com"`, + `echo "https://test"`, + `echo "https://example.com/path/../still-a-url"`, + } + + for _, cmd := range commands { + result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + if result.IsError { + t.Fatalf("expected HTTP(S) URL to be allowed, command=%q error=%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()