fix the issues reported by Claude Code
This commit is contained in:
parent
094c6cf134
commit
9c39745e49
2 changed files with 124 additions and 1 deletions
|
|
@ -546,6 +546,13 @@ func NewWebFetchToolWithProxy(maxChars int, proxy string, fetchLimitBytes int64)
|
|||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
if proxy == "" {
|
||||
client.Transport.(*http.Transport).DialContext = newSafeDialer(&net.Dialer{
|
||||
Timeout: 30 * time.Second,
|
||||
KeepAlive: 30 * time.Second,
|
||||
})
|
||||
}
|
||||
if fetchLimitBytes <= 0 {
|
||||
fetchLimitBytes = 10 * 1024 * 1024 // Security Fallback
|
||||
}
|
||||
|
|
@ -587,7 +594,7 @@ func blockPrivateTarget(ctx context.Context, parsedURL *url.URL) error {
|
|||
hostname := parsedURL.Hostname() // strips port and IPv6 brackets
|
||||
addrs, err := net.DefaultResolver.LookupHost(ctx, hostname)
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not resolve host %q", hostname)
|
||||
return fmt.Errorf("DNS resolution failed for host %q: %w", hostname, err)
|
||||
}
|
||||
for _, addr := range addrs {
|
||||
ip := net.ParseIP(addr)
|
||||
|
|
@ -602,6 +609,39 @@ func blockPrivateTarget(ctx context.Context, parsedURL *url.URL) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
func newSafeDialer(base *net.Dialer) func(context.Context, string, string) (net.Conn, error) {
|
||||
return func(ctx context.Context, network, addr string) (net.Conn, error) {
|
||||
host, port, err := net.SplitHostPort(addr)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
addrs, err := net.DefaultResolver.LookupHost(ctx, host)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("DNS resolution failed for host %q: %w", host, err)
|
||||
}
|
||||
for _, a := range addrs {
|
||||
ip := net.ParseIP(a)
|
||||
if ip == nil {
|
||||
continue
|
||||
}
|
||||
if ip.IsLoopback() || ip.IsPrivate() ||
|
||||
ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() {
|
||||
return nil, fmt.Errorf("requests to private/internal addresses are not allowed")
|
||||
}
|
||||
}
|
||||
// Connect directly to the resolved IP to prevent re-resolution at dial time.
|
||||
var lastErr error
|
||||
for _, a := range addrs {
|
||||
conn, err := base.DialContext(ctx, network, net.JoinHostPort(a, port))
|
||||
if err == nil {
|
||||
return conn, nil
|
||||
}
|
||||
lastErr = err
|
||||
}
|
||||
return nil, lastErr
|
||||
}
|
||||
}
|
||||
|
||||
func (t *WebFetchTool) Execute(ctx context.Context, args map[string]any) *ToolResult {
|
||||
urlStr, ok := args["url"].(string)
|
||||
if !ok {
|
||||
|
|
|
|||
|
|
@ -6,8 +6,10 @@ import (
|
|||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
|
@ -699,3 +701,84 @@ func TestWebTool_TavilySearch_Success(t *testing.T) {
|
|||
t.Errorf("Expected 'via Tavily' in output, got: %s", result.ForUser)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBlockPrivateTarget(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
blocked := []struct {
|
||||
name string
|
||||
raw string
|
||||
}{
|
||||
{"loopback IPv4", "http://127.0.0.1/"},
|
||||
{"RFC1918 10/8", "http://10.0.0.1/"},
|
||||
{"RFC1918 172.16/12", "http://172.16.0.1/"},
|
||||
{"RFC1918 192.168/16", "http://192.168.1.1/"},
|
||||
{"link-local", "http://169.254.1.1/"},
|
||||
{"unspecified", "http://0.0.0.0/"},
|
||||
{"loopback IPv6", "http://[::1]/"},
|
||||
{"link-local IPv6", "http://[fe80::1]/"},
|
||||
}
|
||||
for _, tc := range blocked {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
u, _ := url.Parse(tc.raw)
|
||||
if err := blockPrivateTarget(ctx, u); err == nil {
|
||||
t.Errorf("expected blockPrivateTarget to block %s, but it was allowed", tc.raw)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
allowed := []struct {
|
||||
name string
|
||||
raw string
|
||||
}{
|
||||
{"public DNS", "http://8.8.8.8/"},
|
||||
{"RFC5737 doc range", "http://192.0.2.1/"},
|
||||
}
|
||||
for _, tc := range allowed {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
u, _ := url.Parse(tc.raw)
|
||||
if err := blockPrivateTarget(ctx, u); err != nil {
|
||||
t.Errorf("expected blockPrivateTarget to allow %s, got error: %v", tc.raw, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSafeDialer(t *testing.T) {
|
||||
base := &net.Dialer{Timeout: 2 * time.Second}
|
||||
dial := newSafeDialer(base)
|
||||
ctx := context.Background()
|
||||
|
||||
privateAddrs := []struct {
|
||||
name string
|
||||
addr string
|
||||
}{
|
||||
{"loopback", "127.0.0.1:80"},
|
||||
{"RFC1918 10/8", "10.0.0.1:80"},
|
||||
{"RFC1918 192.168/16", "192.168.1.1:80"},
|
||||
}
|
||||
for _, tc := range privateAddrs {
|
||||
t.Run("blocks "+tc.name, func(t *testing.T) {
|
||||
_, err := dial(ctx, "tcp", tc.addr)
|
||||
if err == nil {
|
||||
t.Fatalf("expected error for private addr %s, got nil", tc.addr)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "private/internal") {
|
||||
t.Errorf("expected SSRF error for %s, got: %v", tc.addr, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// 192.0.2.1 (RFC 5737) is public — SSRF check passes, connection fails at
|
||||
// the network level (unreachable host), NOT with an SSRF error.
|
||||
t.Run("allows public RFC5737 IP", func(t *testing.T) {
|
||||
_, err := dial(ctx, "tcp", "192.0.2.1:80")
|
||||
if err == nil {
|
||||
// Unexpected success — the IP is supposed to be unreachable.
|
||||
return
|
||||
}
|
||||
if strings.Contains(err.Error(), "private/internal") {
|
||||
t.Errorf("192.0.2.1 should not be blocked as private, got: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue