fix(security): add SSRF protection to WebFetchTool, warn on plaintext HTTP in provider
Block requests to internal/private networks (loopback, link-local, RFC1918, IPv6 ULA) in WebFetchTool to prevent SSRF attacks targeting cloud metadata and internal services. Log a warning when HTTPProvider is configured with plain http:// API base, as API keys may be transmitted without encryption. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
4dfb331560
commit
c735e9cb2f
3 changed files with 95 additions and 1 deletions
|
|
@ -19,6 +19,7 @@ import (
|
||||||
|
|
||||||
"github.com/sipeed/picoclaw/pkg/auth"
|
"github.com/sipeed/picoclaw/pkg/auth"
|
||||||
"github.com/sipeed/picoclaw/pkg/config"
|
"github.com/sipeed/picoclaw/pkg/config"
|
||||||
|
"github.com/sipeed/picoclaw/pkg/logger"
|
||||||
)
|
)
|
||||||
|
|
||||||
type HTTPProvider struct {
|
type HTTPProvider struct {
|
||||||
|
|
@ -41,6 +42,14 @@ func NewHTTPProvider(apiKey, apiBase, proxy string) *HTTPProvider {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if strings.HasPrefix(apiBase, "http://") &&
|
||||||
|
!strings.Contains(apiBase, "localhost") &&
|
||||||
|
!strings.Contains(apiBase, "127.0.0.1") {
|
||||||
|
logger.WarnCF("provider", "API base uses plain HTTP — API keys may be transmitted without encryption", map[string]interface{}{
|
||||||
|
"api_base": apiBase,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
return &HTTPProvider{
|
return &HTTPProvider{
|
||||||
apiKey: apiKey,
|
apiKey: apiKey,
|
||||||
apiBase: strings.TrimRight(apiBase, "/"),
|
apiBase: strings.TrimRight(apiBase, "/"),
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@ import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
"regexp"
|
"regexp"
|
||||||
|
|
@ -267,6 +268,7 @@ func (t *WebSearchTool) Execute(ctx context.Context, args map[string]interface{}
|
||||||
|
|
||||||
type WebFetchTool struct {
|
type WebFetchTool struct {
|
||||||
maxChars int
|
maxChars int
|
||||||
|
allowLoopback bool
|
||||||
}
|
}
|
||||||
|
|
||||||
func NewWebFetchTool(maxChars int) *WebFetchTool {
|
func NewWebFetchTool(maxChars int) *WebFetchTool {
|
||||||
|
|
@ -304,6 +306,47 @@ func (t *WebFetchTool) Parameters() map[string]interface{} {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (t *WebFetchTool) setAllowLoopback(allow bool) {
|
||||||
|
t.allowLoopback = allow
|
||||||
|
}
|
||||||
|
|
||||||
|
// isBlockedHost returns true if the hostname resolves to a private/internal IP.
|
||||||
|
func (t *WebFetchTool) isBlockedHost(hostname string) bool {
|
||||||
|
var ips []net.IP
|
||||||
|
|
||||||
|
if ip := net.ParseIP(hostname); ip != nil {
|
||||||
|
ips = append(ips, ip)
|
||||||
|
} else {
|
||||||
|
addrs, err := net.LookupHost(hostname)
|
||||||
|
if err != nil {
|
||||||
|
// If we can't resolve, block by default for safety
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
for _, addr := range addrs {
|
||||||
|
if ip := net.ParseIP(addr); ip != nil {
|
||||||
|
ips = append(ips, ip)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, ip := range ips {
|
||||||
|
if ip.IsLoopback() && !t.allowLoopback {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
if ip.Equal(net.IPv4zero) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
if ip.IsLinkLocalUnicast() {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
if ip.IsPrivate() {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
func (t *WebFetchTool) Execute(ctx context.Context, args map[string]interface{}) *ToolResult {
|
func (t *WebFetchTool) Execute(ctx context.Context, args map[string]interface{}) *ToolResult {
|
||||||
urlStr, ok := args["url"].(string)
|
urlStr, ok := args["url"].(string)
|
||||||
if !ok {
|
if !ok {
|
||||||
|
|
@ -323,6 +366,10 @@ func (t *WebFetchTool) Execute(ctx context.Context, args map[string]interface{})
|
||||||
return ErrorResult("missing domain in URL")
|
return ErrorResult("missing domain in URL")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if t.isBlockedHost(parsedURL.Hostname()) {
|
||||||
|
return ErrorResult("URL blocked: requests to internal/private networks are not allowed")
|
||||||
|
}
|
||||||
|
|
||||||
maxChars := t.maxChars
|
maxChars := t.maxChars
|
||||||
if mc, ok := args["maxChars"].(float64); ok {
|
if mc, ok := args["maxChars"].(float64); ok {
|
||||||
if int(mc) > 100 {
|
if int(mc) > 100 {
|
||||||
|
|
|
||||||
|
|
@ -19,6 +19,7 @@ func TestWebTool_WebFetch_Success(t *testing.T) {
|
||||||
defer server.Close()
|
defer server.Close()
|
||||||
|
|
||||||
tool := NewWebFetchTool(50000)
|
tool := NewWebFetchTool(50000)
|
||||||
|
tool.setAllowLoopback(true)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
args := map[string]interface{}{
|
args := map[string]interface{}{
|
||||||
"url": server.URL,
|
"url": server.URL,
|
||||||
|
|
@ -55,6 +56,7 @@ func TestWebTool_WebFetch_JSON(t *testing.T) {
|
||||||
defer server.Close()
|
defer server.Close()
|
||||||
|
|
||||||
tool := NewWebFetchTool(50000)
|
tool := NewWebFetchTool(50000)
|
||||||
|
tool.setAllowLoopback(true)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
args := map[string]interface{}{
|
args := map[string]interface{}{
|
||||||
"url": server.URL,
|
"url": server.URL,
|
||||||
|
|
@ -146,6 +148,7 @@ func TestWebTool_WebFetch_Truncation(t *testing.T) {
|
||||||
defer server.Close()
|
defer server.Close()
|
||||||
|
|
||||||
tool := NewWebFetchTool(1000) // Limit to 1000 chars
|
tool := NewWebFetchTool(1000) // Limit to 1000 chars
|
||||||
|
tool.setAllowLoopback(true)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
args := map[string]interface{}{
|
args := map[string]interface{}{
|
||||||
"url": server.URL,
|
"url": server.URL,
|
||||||
|
|
@ -211,6 +214,7 @@ func TestWebTool_WebFetch_HTMLExtraction(t *testing.T) {
|
||||||
defer server.Close()
|
defer server.Close()
|
||||||
|
|
||||||
tool := NewWebFetchTool(50000)
|
tool := NewWebFetchTool(50000)
|
||||||
|
tool.setAllowLoopback(true)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
args := map[string]interface{}{
|
args := map[string]interface{}{
|
||||||
"url": server.URL,
|
"url": server.URL,
|
||||||
|
|
@ -254,3 +258,37 @@ func TestWebTool_WebFetch_MissingDomain(t *testing.T) {
|
||||||
t.Errorf("Expected domain error message, got ForLLM: %s", result.ForLLM)
|
t.Errorf("Expected domain error message, got ForLLM: %s", result.ForLLM)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestWebFetchTool_SSRFBlocking verifies that requests to internal/private networks are blocked
|
||||||
|
func TestWebFetchTool_SSRFBlocking(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
url string
|
||||||
|
}{
|
||||||
|
{"loopback IPv4", "http://127.0.0.1/secret"},
|
||||||
|
{"localhost", "http://localhost/admin"},
|
||||||
|
{"cloud metadata", "http://169.254.169.254/latest/meta-data/"},
|
||||||
|
{"loopback IPv6", "http://[::1]/internal"},
|
||||||
|
{"private 10.x", "http://10.0.0.1/internal"},
|
||||||
|
{"private 192.168.x", "http://192.168.1.1/admin"},
|
||||||
|
{"private 172.16.x", "http://172.16.0.1/internal"},
|
||||||
|
}
|
||||||
|
|
||||||
|
tool := NewWebFetchTool(50000)
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
args := map[string]interface{}{
|
||||||
|
"url": tc.url,
|
||||||
|
}
|
||||||
|
result := tool.Execute(ctx, args)
|
||||||
|
if !result.IsError {
|
||||||
|
t.Errorf("Expected SSRF block for %s, but request was allowed", tc.url)
|
||||||
|
}
|
||||||
|
if !strings.Contains(result.ForLLM, "URL blocked") {
|
||||||
|
t.Errorf("Expected 'URL blocked' message for %s, got: %s", tc.url, result.ForLLM)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue