diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index d13d8f9ce..f1ffbf872 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -419,21 +419,14 @@ func hasHTTPURLPrefix(command string, start int) bool { } 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]) { + if quote != 0 && (command[i] == quote || command[i] == '\'' || command[i] == '"') { + return i + } + if quote == 0 && (strings.ContainsRune(" \t\r\n", rune(command[i])) || isShellURLDelimiter(command[i])) { + return i + } + if !isHTTPURLChar(command[i]) { return i } } @@ -441,6 +434,24 @@ func findHTTPURLEnd(command string, start int, quote byte) int { return len(command) } +func isHTTPURLChar(ch byte) bool { + switch { + case ch >= 'a' && ch <= 'z': + return true + case ch >= 'A' && ch <= 'Z': + return true + case ch >= '0' && ch <= '9': + return true + } + + switch ch { + case ':', '/', '?', '#', '[', ']', '@', '!', '$', '&', '*', '+', ',', ';', '=', '%', '-', '.', '_', '~': + return true + default: + return false + } +} + func isShellURLDelimiter(ch byte) bool { switch ch { case ';', '|', '&', '<', '>', '(', ')', '`': diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index 0002b02ab..20fe6c53f 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -359,6 +359,28 @@ func TestShellTool_RestrictToWorkspace_URLDoesNotBypassPathChecks(t *testing.T) } } +// TestShellTool_RestrictToWorkspace_URLDoesNotMaskQuotedPaths verifies that a +// URL inside a quoted argument does not hide a later absolute path in the same argument. +func TestShellTool_RestrictToWorkspace_URLDoesNotMaskQuotedPaths(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{ + `python3 -c "print('https://x,' + open('/etc/passwd').read())"`, + `python3 -c "print('https://x,'+open('/etc/passwd').read())"`, + } + + 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 quoted path access 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()