diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 2edd0accb..0a6f19190 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -339,12 +339,33 @@ func (t *ExecTool) guardCommand(command, cwd string) string { matches := absolutePathPattern.FindAllString(cmd, -1) for _, raw := range matches { - // Skip URL path components that look like they're from URLs. + // 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, "//") { - continue + // 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 + } + } } p, err := filepath.Abs(raw) diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index cb5f1a305..153185b47 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -472,3 +472,46 @@ func TestShellTool_URLsNotBlocked(t *testing.T) { } } } + +// TestShellTool_FileURISandboxing verifies that file:// URIs that escape the +// workspace are still blocked, even though other URLs are allowed (issue #1254). +func TestShellTool_FileURISandboxing(t *testing.T) { + tmpDir := t.TempDir() + tool, err := NewExecTool(tmpDir, true) + if err != nil { + t.Fatalf("unable to configure exec tool: %s", err) + } + + // These file:// URIs should be blocked if they reference paths outside the workspace. + // Unlike web URLs (http://, https://, ftp://), file:// URIs can be used to escape the sandbox. + blockedCommands := []string{ + "cat file:///etc/passwd", + "cat file:///etc/hosts", + "cat file:///root/.ssh/id_rsa", + } + + 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("file:// URI outside workspace should be blocked: %s", cmd) + } + } + + // These file:// URIs should be allowed if they reference paths inside the workspace. + // Create a test file inside the temp directory + testFile := filepath.Join(tmpDir, "test.txt") + if err := os.WriteFile(testFile, []byte("test content"), 0644); err != nil { + t.Fatalf("failed to create test file: %s", err) + } + + allowedCommands := []string{ + "cat file://" + testFile, + } + + for _, cmd := range allowedCommands { + result := tool.Execute(context.Background(), map[string]any{"command": cmd}) + if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") { + t.Errorf("file:// URI inside workspace should be allowed: %s\n error: %s", cmd, result.ForLLM) + } + } +}