diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 0a6f19190..a0b7a5c8e 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -336,36 +336,35 @@ func (t *ExecTool) guardCommand(command, cwd string) string { return "" } - matches := absolutePathPattern.FindAllString(cmd, -1) + // Web URL schemes whose path components (starting with //) should be exempt + // from workspace sandbox checks. file: is intentionally excluded so that + // file:// URIs are still validated against the workspace boundary. + webSchemes := []string{"http:", "https:", "ftp:", "ftps:", "sftp:", "ssh:", "git:"} + + matchIndices := absolutePathPattern.FindAllStringIndex(cmd, -1) + + for _, loc := range matchIndices { + raw := cmd[loc[0]:loc[1]] - for _, raw := range matches { // Skip URL path components that look like they're from web URLs. // When a URL like "https://github.com" is parsed, the regex captures // "//github.com" as a match (the path portion after "https:"). - // These double-slash prefixes indicate URL paths, not file system paths. - // However, we must NOT skip file:// URIs as they could escape the sandbox. - // Only skip if preceded by a web URL scheme (http:, https:, ftp:, etc.). - if strings.HasPrefix(raw, "//") { - // Check if this // path is preceded by a web URL scheme - // by looking for patterns like "http://", "https://", "ftp://" before the match - idx := strings.Index(cmd, raw) - if idx > 0 { - // Look for the scheme prefix (e.g., "https:") before the // - before := cmd[:idx] - // Check if it ends with a web URL scheme followed by colon - // Web schemes: http, https, ftp, ftps, sftp, ssh, git - webSchemes := []string{"http:", "https:", "ftp:", "ftps:", "sftp:", "ssh:", "git:"} - isWebURL := false - for _, scheme := range webSchemes { - if strings.HasSuffix(before, scheme) { - isWebURL = true - break - } - } - if isWebURL { - continue + // Use the exact match position (loc[0]) so that duplicate //path substrings + // in the same command are each evaluated at their own position. + if strings.HasPrefix(raw, "//") && loc[0] > 0 { + before := cmd[:loc[0]] + isWebURL := false + + for _, scheme := range webSchemes { + if strings.HasSuffix(before, scheme) { + isWebURL = true + break } } + + if isWebURL { + continue + } } p, err := filepath.Abs(raw) diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index 5878fc4b3..6f439fce8 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -515,3 +515,29 @@ func TestShellTool_FileURISandboxing(t *testing.T) { } } } + +// TestShellTool_URLBypassPrevented verifies that a command cannot bypass the workspace +// sandbox by smuggling a real path after a URL that contains the same //path substring. +// e.g. "echo https://etc/passwd && cat //etc/passwd" must still be blocked. +func TestShellTool_URLBypassPrevented(t *testing.T) { + tmpDir := t.TempDir() + tool, err := NewExecTool(tmpDir, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + // The path //etc/passwd appears twice: once as the host part of an https URL + // and once as a real (escaped) absolute path. The guard must block the command + // because the second occurrence is a genuine out-of-workspace path. + blockedCommands := []string{ + "echo https://etc/passwd && cat //etc/passwd", + "curl https://host/file && ls //etc", + } + + for _, cmd := range blockedCommands { + result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") { + t.Errorf("bypass attempt should be blocked: %q\n got: %s", cmd, result.ForLLM) + } + } +}