From 45cf81cfe68f113ec8e2b8c2285ddf2cdc294798 Mon Sep 17 00:00:00 2001 From: SashaMIT Date: Sun, 9 Aug 2026 14:00:38 +0700 Subject: [PATCH] fix(wecom): use CreateSafeHTTPClient for media downloads WeCom mediaClient was a plain http.Client, so inbound storeRemoteMedia and outbound downloadRemoteMediaToTemp followed redirects onto loopback/private hosts. Build the client via CreateSafeHTTPClient and ValidateSafeHTTPURL before fetch (sibling of channel DownloadFile #3322). --- pkg/channels/wecom/media.go | 7 +++++ pkg/channels/wecom/media_test.go | 45 ++++++++++++++++++++++++++++++++ pkg/channels/wecom/wecom.go | 10 ++++++- 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pkg/channels/wecom/media.go b/pkg/channels/wecom/media.go index 1494b5793a..bb69307763 100644 --- a/pkg/channels/wecom/media.go +++ b/pkg/channels/wecom/media.go @@ -22,6 +22,7 @@ import ( "github.com/sipeed/picoclaw/pkg/bus" "github.com/sipeed/picoclaw/pkg/media" + "github.com/sipeed/picoclaw/pkg/utils" ) const ( @@ -281,6 +282,9 @@ func (c *WeComChannel) storeRemoteMedia( return "", fmt.Errorf("no media store available") } + if err := utils.ValidateSafeHTTPURL(resourceURL, nil, nil); err != nil { + return "", fmt.Errorf("download media: %w", err) + } req, err := http.NewRequestWithContext(ctx, http.MethodGet, resourceURL, nil) if err != nil { return "", fmt.Errorf("create request: %w", err) @@ -418,6 +422,9 @@ func (c *WeComChannel) downloadRemoteMediaToTemp( ctx context.Context, resourceURL, fallbackName string, ) (string, string, string, error) { + if err := utils.ValidateSafeHTTPURL(resourceURL, nil, nil); err != nil { + return "", "", "", fmt.Errorf("download media: %w", err) + } req, err := http.NewRequestWithContext(ctx, http.MethodGet, resourceURL, nil) if err != nil { return "", "", "", fmt.Errorf("create request: %w", err) diff --git a/pkg/channels/wecom/media_test.go b/pkg/channels/wecom/media_test.go index d5307e5d2b..89183c541b 100644 --- a/pkg/channels/wecom/media_test.go +++ b/pkg/channels/wecom/media_test.go @@ -6,11 +6,14 @@ import ( "encoding/base64" "io" "net/http" + "net/http/httptest" "strings" "testing" + "time" basechannels "github.com/sipeed/picoclaw/pkg/channels" "github.com/sipeed/picoclaw/pkg/media" + "github.com/sipeed/picoclaw/pkg/utils" ) func TestStoreRemoteMedia_DetectsJPEGContentTypeFromBody(t *testing.T) { @@ -163,6 +166,48 @@ func TestStoreRemoteMedia_PreservesSuffixFromContentDisposition(t *testing.T) { } } +func TestStoreRemoteMedia_BlocksPrivateRedirect(t *testing.T) { + t.Parallel() + + privateHit := false + private := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + privateHit = true + _, _ = w.Write([]byte("SECRET")) + })) + t.Cleanup(private.Close) + + public := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, private.URL+"/secret", http.StatusFound) + })) + t.Cleanup(public.Close) + + client, err := utils.CreateSafeHTTPClient(utils.SafeHTTPClientOptions{ + Timeout: 5 * time.Second, + }) + if err != nil { + t.Fatalf("CreateSafeHTTPClient: %v", err) + } + + store := media.NewFileMediaStore() + ch := &WeComChannel{ + BaseChannel: basechannels.NewBaseChannel("wecom", nil, nil, nil), + mediaClient: client, + } + ch.SetMediaStore(store) + + _, err = ch.storeRemoteMedia(context.Background(), "test-scope", "msg-ssrf", public.URL, "", "") + if err == nil { + t.Fatal("expected storeRemoteMedia to reject redirect to private host") + } + if privateHit { + t.Fatal("private target was reached via redirect") + } + if !strings.Contains(err.Error(), "private or local") && + !strings.Contains(err.Error(), "blocked private") { + t.Fatalf("unexpected error: %v", err) + } +} + func decodeTestBase64(t *testing.T, value string) []byte { t.Helper() diff --git a/pkg/channels/wecom/wecom.go b/pkg/channels/wecom/wecom.go index f6252af4ca..f46ba0d302 100644 --- a/pkg/channels/wecom/wecom.go +++ b/pkg/channels/wecom/wecom.go @@ -18,6 +18,7 @@ import ( "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/identity" "github.com/sipeed/picoclaw/pkg/logger" + "github.com/sipeed/picoclaw/pkg/utils" ) const ( @@ -124,6 +125,13 @@ func NewChannel(bc *config.Channel, cfg *config.WeComSettings, messageBus *bus.M channels.WithReasoningChannelID(bc.ReasoningChannelID), ) + mediaClient, err := utils.CreateSafeHTTPClient(utils.SafeHTTPClientOptions{ + Timeout: wecomMediaTimeout, + }) + if err != nil { + return nil, fmt.Errorf("create wecom media http client: %w", err) + } + ch := &WeComChannel{ BaseChannel: base, config: cfg, @@ -131,7 +139,7 @@ func NewChannel(bc *config.Channel, cfg *config.WeComSettings, messageBus *bus.M turns: make(map[string][]wecomTurn), recent: newRecentMessageSet(wecomRecentMessageMax), routes: newReqIDStore(""), - mediaClient: &http.Client{Timeout: wecomMediaTimeout}, + mediaClient: mediaClient, } ch.SetOwner(ch) return ch, nil