diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index db9efa2cf8..cd1b17ce51 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -485,11 +485,13 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) } } - // Use routed session key, but honor pre-set agent-scoped keys (for ProcessDirect/cron) - sessionKey := route.SessionKey - if msg.SessionKey != "" && strings.HasPrefix(msg.SessionKey, "agent:") { - sessionKey = msg.SessionKey - } + // Use custom session key if provided directly via ProcessDirect, else use route's default + // Previously only honored agent: prefixed keys, now honors all custom keys + sessionKey := route.SessionKey + if msg.SessionKey != "" { + sessionKey = msg.SessionKey + } + logger.InfoCF("agent", "Routed message", map[string]any{ diff --git a/pkg/providers/openai_compat/provider.go b/pkg/providers/openai_compat/provider.go index ff9109e969..11e93a876b 100644 --- a/pkg/providers/openai_compat/provider.go +++ b/pkg/providers/openai_compat/provider.go @@ -223,9 +223,15 @@ func parseResponse(body []byte) (*LLMResponse, error) { } if err := json.Unmarshal(body, &apiResponse); err != nil { + // Check if the response is HTML instead of JSON (e.g., wrong api_base URL) + bodyStr := string(body) + if strings.HasPrefix(strings.TrimSpace(bodyStr), "<") { + return nil, fmt.Errorf("API returned HTML instead of JSON. Please check your api_base URL configuration. The server may be returning an error page or the endpoint URL may be incorrect (e.g., missing '/v1' path)") + } return nil, fmt.Errorf("failed to unmarshal response: %w", err) } + if len(apiResponse.Choices) == 0 { return &LLMResponse{ Content: "", diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go index cd8da31959..0ba2db5e30 100644 --- a/pkg/tools/filesystem.go +++ b/pkg/tools/filesystem.go @@ -13,6 +13,29 @@ import ( "github.com/sipeed/picoclaw/pkg/fileutil" ) +// isBinaryFile checks if the file content appears to be binary. +// It checks the first 512 bytes for the ratio of non-printable characters. +func isBinaryFile(content []byte, sampleSize int) bool { + if sampleSize > len(content) { + sampleSize = len(content) + } + if sampleSize == 0 { + return false + } + + sample := content[:sampleSize] + nonPrintable := 0 + for _, b := range sample { + // Check for null byte or control characters (except tab, newline, carriage return) + if b == 0 || (b < 32 && b != 9 && b != 10 && b != 13) { + nonPrintable++ + } + } + + // If more than 30% is non-printable, consider it binary + return float64(nonPrintable)/float64(sampleSize) > 0.3 +} + // validatePath ensures the given path is within the workspace if restrict is true. func validatePath(path, workspace string, restrict bool) (string, error) { if workspace == "" { @@ -127,6 +150,12 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe if err != nil { return ErrorResult(err.Error()) } + + // Check if file is binary + if isBinaryFile(content, 512) { + return ErrorResult("binary file detected. read_file tool does not support reading binary files such as PDF, images, videos, etc.") + } + return NewToolResult(string(content)) } diff --git a/pkg/tools/filesystem_test.go b/pkg/tools/filesystem_test.go index 666004cd40..6235916f17 100644 --- a/pkg/tools/filesystem_test.go +++ b/pkg/tools/filesystem_test.go @@ -520,3 +520,40 @@ func TestWhitelistFs_AllowsMatchingPaths(t *testing.T) { t.Errorf("expected non-whitelisted path to be blocked, got: %s", result.ForLLM) } } +// TestFilesystemTool_ReadFile_BinaryFile verifies rejection of binary files +func TestFilesystemTool_ReadFile_BinaryFile(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.pdf") + // Create binary content with more than 30% non-printable characters + // Start with legitimate content header + pdfContent := []byte("%PDF-1.4\n1 0 obj\n<< /Pages 2 0 R >>\nendobj") + + // Add significant portion of null bytes and control chars (will surpass 30%) + multibytes := make([]byte, 20) + for i := range multibytes { + if i%3 == 0 { + multibytes[i] = 0x00 // null + } else if i%3 == 1 { + multibytes[i] = 0x01 // start of heading control char + } else { + multibytes[i] = 0x02 // start of text control char + } + } + pdfContent = append(pdfContent, multibytes...) + os.WriteFile(testFile, pdfContent, 0o644) + tool := NewReadFileTool("", false) + ctx := context.Background() + args := map[string]any{ + "path": testFile, + } + + result := tool.Execute(ctx, args) + + if !result.IsError { + t.Errorf("Expected error for binary file, got IsError=false") + } + + if !strings.Contains(result.ForLLM, "binary file detected") { + t.Errorf("Expected binary file error message, got: %s", result.ForLLM) + } +} diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index a0c83eb1ee..4a0847c085 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -287,7 +287,50 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult } } +// isURLPath checks if the matched path is part of a URL rather than a file path. +// It detects patterns like "wttr.in/Beijing" or "http://example.com/path" +func isURLPath(command, matchedPath string) bool { + // Find the position of the matched path in the command + idx := strings.Index(command, matchedPath) + if idx == -1 { + return false + } + + // Check what comes before the matched path + // If it's preceded by a dot (.) or //, it's likely a URL + if idx > 0 { + prefix := command[:idx] + trimmed := strings.TrimSpace(prefix) + + // Check for URL patterns: domain.com/ or http:// or https:// + if strings.HasSuffix(trimmed, ".") { + return true + } + if strings.HasSuffix(trimmed, "//") { + return true + } + + // Check if the match starts with / and is preceded by a word character (domain) + if matchedPath[0] == '/' && idx > 0 { + before := command[idx-1] + if before == '.' || before == '/' { + return true + } + } + } + + // Check if the matched path itself looks like a URL path component + // (contains query parameters or fragments) + if strings.Contains(matchedPath, "?") || strings.Contains(matchedPath, "#") || strings.Contains(matchedPath, "&") { + return true + } + + return false +} + + func (t *ExecTool) guardCommand(command, cwd string) string { + cmd := strings.TrimSpace(command) lower := strings.ToLower(cmd) @@ -322,6 +365,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string { } if t.restrictToWorkspace { + // Check for explicit path traversal sequences if strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") { return "Command blocked by safety guard (path traversal detected)" } @@ -334,6 +378,12 @@ func (t *ExecTool) guardCommand(command, cwd string) string { matches := absolutePathPattern.FindAllString(cmd, -1) for _, raw := range matches { + // Skip URL paths (e.g., wttr.in/Beijing in curl commands) + // URL paths typically follow a dot or are part of a URL pattern + if isURLPath(cmd, raw) { + continue + } + p, err := filepath.Abs(raw) if err != nil { continue