mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 16:17:25 +00:00
fix: harden media URL handling
This commit is contained in:
1 parent
fef98b4dd7
commit
1dea4a21cf
6 files changed
+87
-16
No files matched your search
@@ -10,6 +10,7 @@ import (
|
||||
"strings"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/providers"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/security"
|
||||
usagecaps "github.com/nextlevelbuilder/goclaw/internal/usage/caps"
|
||||
)
|
||||
|
||||
@@ -62,7 +63,7 @@ func (t *ReadImageTool) SetUsageCapService(svc *usagecaps.Service) {
|
||||
func (t *ReadImageTool) Name() string { return "read_image" }
|
||||
|
||||
func (t *ReadImageTool) Description() string {
|
||||
return "Analyze images using vision AI. Works with: (1) images sent by the user (<media:image> tags), (2) workspace/generated image files (pass a file path)."
|
||||
return "Analyze images using vision AI. Works with images sent by the user, workspace/generated image files, or public HTTP/HTTPS image URLs."
|
||||
}
|
||||
|
||||
func (t *ReadImageTool) Parameters() map[string]any {
|
||||
@@ -111,6 +112,9 @@ func (t *ReadImageTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
}
|
||||
images = fileImages
|
||||
} else if imgURL != "" {
|
||||
if _, _, err := security.Validate(imgURL); err != nil {
|
||||
return ErrorResult(fmt.Sprintf("Invalid image URL: %v", err))
|
||||
}
|
||||
images = []providers.ImageContent{{
|
||||
URL: imgURL,
|
||||
}}
|
||||
@@ -153,6 +157,15 @@ func (t *ReadImageTool) callProvider(ctx context.Context, cp credentialProvider,
|
||||
prompt := GetParamString(params, "prompt", "Describe this image in detail.")
|
||||
images, _ := params["images"].([]providers.ImageContent)
|
||||
|
||||
for _, img := range images {
|
||||
if img.URL == "" {
|
||||
continue
|
||||
}
|
||||
if _, _, err := security.Validate(img.URL); err != nil {
|
||||
return nil, nil, fmt.Errorf("invalid image URL: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Anthropic Claude does not support URL references and requires base64-encoded image data.
|
||||
if providerName == "anthropic" || providerName == "claude-cli" {
|
||||
for _, img := range images {
|
||||
|
||||
@@ -26,6 +26,22 @@ func TestReadImage_BothPathAndUrl_Error(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadImage_PrivateURL_Error(t *testing.T) {
|
||||
tool := NewReadImageTool(nil)
|
||||
|
||||
res := tool.Execute(context.Background(), map[string]any{
|
||||
"prompt": "describe this",
|
||||
"url": "http://127.0.0.1/image.png",
|
||||
})
|
||||
|
||||
if !res.IsError {
|
||||
t.Fatalf("expected error for private image URL")
|
||||
}
|
||||
if !strings.Contains(res.ForLLM, "Invalid image URL") {
|
||||
t.Errorf("unexpected error message: %s", res.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadImage_AnthropicURL_Error(t *testing.T) {
|
||||
tool := NewReadImageTool(nil)
|
||||
|
||||
@@ -33,7 +49,7 @@ func TestReadImage_AnthropicURL_Error(t *testing.T) {
|
||||
"prompt": "describe this",
|
||||
"images": []providers.ImageContent{
|
||||
{
|
||||
URL: "https://example.com/image.png",
|
||||
URL: "https://93.184.216.34/image.png",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
@@ -4,11 +4,13 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/providers"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/security"
|
||||
usagecaps "github.com/nextlevelbuilder/goclaw/internal/usage/caps"
|
||||
)
|
||||
|
||||
@@ -32,6 +34,8 @@ func MediaVideoRefsFromCtx(ctx context.Context) []providers.MediaRef {
|
||||
// videoMaxBytes is the max file size for video analysis (100MB).
|
||||
const videoMaxBytes = 100 * 1024 * 1024
|
||||
|
||||
const videoURLPinnedIPParam = "_pinned_ip"
|
||||
|
||||
// videoProviderPriority is the order in which providers are tried for video analysis.
|
||||
// OpenAI excluded — no native video upload in chat completions.
|
||||
var videoProviderPriority = []string{"gemini", "openrouter"}
|
||||
@@ -101,13 +105,17 @@ func (t *ReadVideoTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
|
||||
var data []byte
|
||||
var videoMime string
|
||||
var pinnedIP net.IP
|
||||
|
||||
if videoURL != "" {
|
||||
// Infer MIME type from URL extension
|
||||
ext := filepath.Ext(videoURL)
|
||||
if idx := strings.Index(ext, "?"); idx != -1 {
|
||||
ext = ext[:idx]
|
||||
validatedURL, validatedIP, err := security.Validate(videoURL)
|
||||
if err != nil {
|
||||
return ErrorResult(fmt.Sprintf("Invalid video URL: %v", err))
|
||||
}
|
||||
pinnedIP = validatedIP
|
||||
|
||||
// Infer MIME type from URL extension
|
||||
ext := filepath.Ext(validatedURL.Path)
|
||||
videoMime = mimeFromVideoExt(ext)
|
||||
} else {
|
||||
videoPath, mime, err := t.resolveVideoFile(ctx, mediaID)
|
||||
@@ -139,6 +147,9 @@ func (t *ReadVideoTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
chain[i].Params["data"] = data
|
||||
chain[i].Params["url"] = videoURL
|
||||
chain[i].Params["mime"] = videoMime
|
||||
if pinnedIP != nil {
|
||||
chain[i].Params[videoURLPinnedIPParam] = pinnedIP
|
||||
}
|
||||
}
|
||||
|
||||
chainResult, err := ExecuteWithChain(ctx, chain, t.registry, t.callProvider)
|
||||
|
||||
@@ -5,12 +5,14 @@ import (
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/providers"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/security"
|
||||
)
|
||||
|
||||
// resolveVideoFile finds the video file path from context MediaRefs.
|
||||
@@ -62,12 +64,19 @@ func (t *ReadVideoTool) resolveVideoFile(ctx context.Context, mediaID string) (p
|
||||
|
||||
// callProvider dispatches video analysis to the appropriate provider API.
|
||||
// Gemini: uses File API (upload → poll → file_data in generateContent).
|
||||
// Others: falls back to base64 or URL in image_url (OpenRouter routes to Gemini which handles video).
|
||||
// Others: falls back to base64 or URL in video_url (OpenRouter routes to Gemini which handles video).
|
||||
func (t *ReadVideoTool) callProvider(ctx context.Context, cp credentialProvider, providerName, model string, params map[string]any) ([]byte, *providers.Usage, error) {
|
||||
prompt := GetParamString(params, "prompt", "Analyze this video and describe its contents.")
|
||||
data, _ := params["data"].([]byte)
|
||||
videoURL, _ := params["url"].(string)
|
||||
mime := GetParamString(params, "mime", "video/mp4")
|
||||
pinnedIP, _ := params[videoURLPinnedIPParam].(net.IP)
|
||||
if videoURL != "" && pinnedIP == nil {
|
||||
var err error
|
||||
if _, pinnedIP, err = security.Validate(videoURL); err != nil {
|
||||
return nil, nil, fmt.Errorf("invalid video URL: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Gemini: use File API (requires credentials).
|
||||
ptype := GetParamString(params, "_provider_type", providerTypeFromName(providerName))
|
||||
@@ -87,18 +96,19 @@ func (t *ReadVideoTool) callProvider(ctx context.Context, cp credentialProvider,
|
||||
|
||||
if videoURL != "" {
|
||||
slog.Info("read_video: streaming URL directly to Gemini File API", "provider", providerName, "model", model, "url", videoURL)
|
||||
|
||||
|
||||
// Send GET request to fetch the stream.
|
||||
req, getErr := http.NewRequestWithContext(ctx, "GET", videoURL, nil)
|
||||
reqCtx := security.WithPinnedIP(ctx, pinnedIP)
|
||||
req, getErr := http.NewRequestWithContext(reqCtx, "GET", videoURL, nil)
|
||||
if getErr != nil {
|
||||
if reservation != nil {
|
||||
reservation.Reconcile(ctx, nil, getErr)
|
||||
}
|
||||
return nil, nil, fmt.Errorf("failed to create GET request for video URL: %w", getErr)
|
||||
}
|
||||
|
||||
// No global Timeout on http.Client to allow piping large streams.
|
||||
client := &http.Client{}
|
||||
|
||||
// Use the shared SSRF-safe client so DNS stays pinned during streaming.
|
||||
client := security.NewSafeClient(0)
|
||||
httpResp, getErr := client.Do(req)
|
||||
if getErr != nil {
|
||||
if reservation != nil {
|
||||
|
||||
@@ -6,6 +6,8 @@ import (
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/security"
|
||||
)
|
||||
|
||||
type mockCredentialProvider struct {
|
||||
@@ -34,8 +36,27 @@ func TestReadVideo_BothMediaIdAndUrl_Error(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadVideo_PrivateURL_Error(t *testing.T) {
|
||||
tool := NewReadVideoTool(nil, nil)
|
||||
|
||||
res := tool.Execute(context.Background(), map[string]any{
|
||||
"prompt": "describe this video",
|
||||
"url": "http://127.0.0.1/video.mp4",
|
||||
})
|
||||
|
||||
if !res.IsError {
|
||||
t.Fatalf("expected error for private video URL")
|
||||
}
|
||||
if !strings.Contains(res.ForLLM, "Invalid video URL") {
|
||||
t.Errorf("unexpected error message: %s", res.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadVideo_GeminiURL_Validation(t *testing.T) {
|
||||
// 1. Trường hợp không có Content-Length (ContentLength <= 0)
|
||||
security.SetAllowLoopbackForTest(true)
|
||||
defer security.SetAllowLoopbackForTest(false)
|
||||
|
||||
// Missing Content-Length should fail before upload.
|
||||
ts1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Transfer-Encoding", "chunked")
|
||||
w.Write([]byte("chunked data mock video"))
|
||||
@@ -59,7 +80,7 @@ func TestReadVideo_GeminiURL_Validation(t *testing.T) {
|
||||
t.Errorf("unexpected error for missing Content-Length: %v", err)
|
||||
}
|
||||
|
||||
// 2. Trường hợp Content-Length vượt quá 2 GB
|
||||
// Content-Length over the Gemini File API limit should fail before upload.
|
||||
ts2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Length", "2147483649") // 2GB + 1 byte
|
||||
w.WriteHeader(http.StatusOK)
|
||||
@@ -80,7 +101,7 @@ func TestReadVideo_GeminiURL_Validation(t *testing.T) {
|
||||
t.Errorf("unexpected error for limit exceed: %v", err)
|
||||
}
|
||||
|
||||
// 3. Trường hợp HTTP status code lỗi (ví dụ 404)
|
||||
// Non-2xx status should be reported before upload.
|
||||
ts3 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}))
|
||||
|
||||
@@ -254,7 +254,7 @@ func resolveRemoteCDP(remoteURL string) (string, error) {
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return "", fmt.Errorf("/json/version returned HTTP %d", resp.StatusCode)
|
||||
return "", fmt.Errorf("query /json/version at %s returned HTTP %d", versionURL, resp.StatusCode)
|
||||
}
|
||||
|
||||
var ver struct {
|
||||
|
||||
Reference in new issue
Block a user