From 9540e63fa5018770d282d654a6ce05983ca4ade7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Mar 2026 00:24:50 +0000 Subject: [PATCH] Fix security vulnerabilities from review (CRIT-1 through MED-6) CRIT-1: Remove hardcoded Google OAuth client secret; credentials are now read from GOOGLE_CLIENT_ID / GOOGLE_CLIENT_SECRET env vars. Removed the decodeBase64 helper that existed only to obfuscate the embedded secret. CRIT-2 + MED-3: Add RequireLocalOrigin middleware to the launcher API server. All /api/ and /auth/ routes now reject requests whose Origin header is present but doesn't match http://localhost:18800 or http://127.0.0.1:18800, blocking browser-based CSRF attacks. CRIT-3 + HIGH-5: Split shell deny patterns into absoluteDenyPatterns (always enforced, cannot be overridden) and defaultDenyPatterns (overridable by customAllowPatterns). Added missing patterns for nc/ncat, python -c, perl -e, ruby -e, nohup, crontab, authorized_keys writes, shell startup file writes, and base64-piped execution. |/bin/sh and |/bin/bash are now also covered. HIGH-1: Replace cmd /c start with rundll32 url.dll,FileProtocolHandler on Windows in OpenBrowser to prevent cmd.exe shell metacharacter injection. Added URL scheme validation before opening any browser URL. HIGH-2: Apply io.LimitReader(resp.Body, 1<<20) to all io.ReadAll calls on OAuth HTTP response bodies (RequestDeviceCode, LoginDeviceCode, pollDeviceCode, RefreshAccessToken, ExchangeCodeForTokens, fetchGoogleUserEmail) to prevent OOM from malicious token endpoints. HIGH-3: Add maxOAuthSessions = 50 cap; handleGoogleAntigravityLogin now returns 429 when the limit is reached, preventing unbounded map growth. HIGH-4: Escape all attacker-controlled strings written into HTML responses in handleOAuthCallback using html.EscapeString, fixing reflected XSS via the ?error= query parameter. HIGH-6: Replace math/rand with crypto/rand in randomString() in antigravity_provider.go using crypto/rand.Int + math/big.Int. MED-1: Add validGitHubRepo regex validation in InstallFromGitHub to reject repo names containing path traversal, query strings, fragments, or characters outside [A-Za-z0-9_.-]. MED-2: Add rejectSSRFTarget() in WebFetchTool that resolves the hostname and rejects loopback, link-local, private, and cloud metadata (169.254.x) addresses, preventing prompt-injected SSRF to the launcher API or cloud instance metadata endpoints. MED-6: Canonicalize path with filepath.Clean before running pattern matching in whitelistFs.matches() to prevent regex bypass via embedded traversal sequences like /allowed/path/../../../etc/shadow. https://claude.ai/code/session_01AerxPcDs78ntotTwbyYiVd --- .../internal/server/auth_handlers.go | 29 +++++++--- .../internal/server/server.go | 42 +++++++++++++++ cmd/picoclaw-launcher/main.go | 13 +++-- pkg/auth/oauth.go | 54 +++++++++++-------- pkg/providers/antigravity_provider.go | 14 ++++- pkg/skills/installer.go | 9 ++++ pkg/tools/filesystem.go | 6 ++- pkg/tools/shell.go | 50 ++++++++++++----- pkg/tools/web.go | 43 +++++++++++++++ 9 files changed, 209 insertions(+), 51 deletions(-) diff --git a/cmd/picoclaw-launcher/internal/server/auth_handlers.go b/cmd/picoclaw-launcher/internal/server/auth_handlers.go index 1e9b8be0a..0578597bf 100644 --- a/cmd/picoclaw-launcher/internal/server/auth_handlers.go +++ b/cmd/picoclaw-launcher/internal/server/auth_handlers.go @@ -3,6 +3,7 @@ package server import ( "encoding/json" "fmt" + "html" "io" "log" "net/http" @@ -13,6 +14,11 @@ import ( "github.com/sipeed/picoclaw/pkg/providers" ) +// maxOAuthSessions limits the number of concurrent in-flight OAuth sessions to +// prevent an unauthenticated caller from growing the in-memory session map without +// bound (HIGH-3: DoS via unlimited session creation). +const maxOAuthSessions = 50 + // oauthSession stores in-flight OAuth state for browser-based flows. type oauthSession struct { Provider string @@ -186,8 +192,13 @@ func handleGoogleAntigravityLogin(w http.ResponseWriter, r *http.Request, config authURL := auth.BuildAuthorizeURL(oauthCfg, pkce, state, redirectURI) - // Store session for callback + // Enforce session count limit before storing the new session. oauthSessionsMu.Lock() + if len(oauthSessions) >= maxOAuthSessions { + oauthSessionsMu.Unlock() + http.Error(w, "Too many pending OAuth sessions; please try again later", http.StatusTooManyRequests) + return + } oauthSessions[state] = &oauthSession{ Provider: "google-antigravity", PKCE: pkce, @@ -232,8 +243,10 @@ func handleOAuthCallback(w http.ResponseWriter, r *http.Request) { } if code == "" { - errMsg := r.URL.Query().Get("error") - w.Header().Set("Content-Type", "text/html") + // Escape the attacker-controlled query parameter before writing it into HTML + // to prevent reflected XSS (HIGH-4). + errMsg := html.EscapeString(r.URL.Query().Get("error")) + w.Header().Set("Content-Type", "text/html; charset=utf-8") fmt.Fprintf( w, `

Authentication failed

%s

You can close this window.

`, @@ -244,11 +257,11 @@ func handleOAuthCallback(w http.ResponseWriter, r *http.Request) { cred, err := auth.ExchangeCodeForTokens(session.OAuthCfg, code, session.PKCE.CodeVerifier, session.RedirectURI) if err != nil { - w.Header().Set("Content-Type", "text/html") + w.Header().Set("Content-Type", "text/html; charset=utf-8") fmt.Fprintf( w, `

Authentication failed

%s

You can close this window.

`, - err.Error(), + html.EscapeString(err.Error()), ) return } @@ -266,8 +279,8 @@ func handleOAuthCallback(w http.ResponseWriter, r *http.Request) { } if err := auth.SetCredential(session.Provider, cred); err != nil { - w.Header().Set("Content-Type", "text/html") - fmt.Fprintf(w, `

Failed to save credentials

%s

`, err.Error()) + w.Header().Set("Content-Type", "text/html; charset=utf-8") + fmt.Fprintf(w, `

Failed to save credentials

%s

`, html.EscapeString(err.Error())) return } @@ -297,7 +310,7 @@ func fetchGoogleUserEmail(accessToken string) (string, error) { } defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("userinfo request failed: %s", string(body)) } diff --git a/cmd/picoclaw-launcher/internal/server/server.go b/cmd/picoclaw-launcher/internal/server/server.go index 4fc68f04c..d8e8884bb 100644 --- a/cmd/picoclaw-launcher/internal/server/server.go +++ b/cmd/picoclaw-launcher/internal/server/server.go @@ -6,6 +6,7 @@ import ( "io" "log" "net/http" + "strings" "time" "github.com/sipeed/picoclaw/pkg/auth" @@ -14,6 +15,47 @@ import ( const DefaultPort = "18800" +// allowedOrigins are the origins from which browser requests to the API are accepted. +// Any request whose Origin header is set but doesn't match one of these is rejected. +var allowedOrigins = []string{ + "http://localhost:" + DefaultPort, + "http://127.0.0.1:" + DefaultPort, +} + +// RequireLocalOrigin is HTTP middleware that rejects cross-origin requests to +// the launcher API. It blocks browser-based CSRF attacks (any browser tab can +// attempt a cross-origin fetch, but the browser always sends the real Origin +// header which we validate here). Requests without an Origin header (direct +// curl/tool access from localhost) are permitted. +func RequireLocalOrigin(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + origin := r.Header.Get("Origin") + if origin != "" { + allowed := false + for _, o := range allowedOrigins { + if strings.EqualFold(origin, o) { + allowed = true + break + } + } + if !allowed { + http.Error(w, "Forbidden: cross-origin request rejected", http.StatusForbidden) + return + } + } + // Set CORS headers so the UI (same origin) works correctly and other + // origins are explicitly denied by the browser. + w.Header().Set("Access-Control-Allow-Origin", "http://localhost:"+DefaultPort) + w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, OPTIONS") + w.Header().Set("Access-Control-Allow-Headers", "Content-Type") + if r.Method == http.MethodOptions { + w.WriteHeader(http.StatusNoContent) + return + } + next.ServeHTTP(w, r) + }) +} + // providerStatus represents the auth status of a single provider in API responses. type providerStatus struct { Provider string `json:"provider"` diff --git a/cmd/picoclaw-launcher/main.go b/cmd/picoclaw-launcher/main.go index 3323c31a8..e8dd4ab17 100644 --- a/cmd/picoclaw-launcher/main.go +++ b/cmd/picoclaw-launcher/main.go @@ -67,15 +67,20 @@ func main() { addr = "127.0.0.1:" + server.DefaultPort } - mux := http.NewServeMux() - server.RegisterConfigAPI(mux, absPath) - server.RegisterAuthAPI(mux, absPath) - server.RegisterProcessAPI(mux, absPath) + apiMux := http.NewServeMux() + server.RegisterConfigAPI(apiMux, absPath) + server.RegisterAuthAPI(apiMux, absPath) + server.RegisterProcessAPI(apiMux, absPath) staticFS, err := fs.Sub(staticFiles, "internal/ui") if err != nil { log.Fatalf("Failed to create sub filesystem: %v", err) } + + mux := http.NewServeMux() + // All /api/ and /auth/ routes are protected by the local-origin CSRF guard. + mux.Handle("/api/", server.RequireLocalOrigin(apiMux)) + mux.Handle("/auth/", server.RequireLocalOrigin(apiMux)) mux.Handle("/", http.FileServer(http.FS(staticFS))) // Print startup banner diff --git a/pkg/auth/oauth.go b/pkg/auth/oauth.go index 91c9e25c5..af66dd60b 100644 --- a/pkg/auth/oauth.go +++ b/pkg/auth/oauth.go @@ -41,13 +41,18 @@ func OpenAIOAuthConfig() OAuthProviderConfig { } // GoogleAntigravityOAuthConfig returns the OAuth configuration for Google Cloud Code Assist (Antigravity). -// Client credentials are the same ones used by OpenCode/pi-ai for Cloud Code Assist access. +// Credentials are read from GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET environment variables. +// You must register your own OAuth 2.0 credentials at https://console.cloud.google.com/apis/credentials +// and set these environment variables before using Google authentication. func GoogleAntigravityOAuthConfig() OAuthProviderConfig { - // These are the same client credentials used by the OpenCode antigravity plugin. - clientID := decodeBase64( - "MTA3MTAwNjA2MDU5MS10bWhzc2luMmgyMWxjcmUyMzV2dG9sb2poNGc0MDNlcC5hcHBzLmdvb2dsZXVzZXJjb250ZW50LmNvbQ==", - ) - clientSecret := decodeBase64("R09DU1BYLUs1OEZXUjQ4NkxkTEoxbUxCOHNYQzR6NnFEQWY=") + clientID := os.Getenv("GOOGLE_CLIENT_ID") + clientSecret := os.Getenv("GOOGLE_CLIENT_SECRET") + if clientID == "" || clientSecret == "" { + // Warn loudly — shared credentials should not be used in production. + fmt.Fprintln(os.Stderr, "WARNING: GOOGLE_CLIENT_ID and/or GOOGLE_CLIENT_SECRET are not set.") + fmt.Fprintln(os.Stderr, " Google OAuth will not function without valid credentials.") + fmt.Fprintln(os.Stderr, " Register your own at https://console.cloud.google.com/apis/credentials") + } return OAuthProviderConfig{ Issuer: "https://accounts.google.com/o/oauth2/v2", TokenURL: "https://oauth2.googleapis.com/token", @@ -58,14 +63,6 @@ func GoogleAntigravityOAuthConfig() OAuthProviderConfig { } } -func decodeBase64(s string) string { - data, err := base64.StdEncoding.DecodeString(s) - if err != nil { - return s - } - return string(data) -} - // GenerateState generates a random state string for OAuth CSRF protection. func GenerateState() (string, error) { buf := make([]byte, 32) @@ -212,7 +209,7 @@ func RequestDeviceCode(cfg OAuthProviderConfig) (*DeviceCodeInfo, error) { } defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("device code request failed: %s", string(body)) } @@ -300,7 +297,7 @@ func LoginDeviceCode(cfg OAuthProviderConfig) (*AuthCredential, error) { } defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("device code request failed: %s", string(body)) } @@ -360,7 +357,7 @@ func pollDeviceCode(cfg OAuthProviderConfig, deviceAuthID, userCode string) (*Au return nil, fmt.Errorf("pending") } - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) var tokenResp struct { AuthorizationCode string `json:"authorization_code"` @@ -401,7 +398,7 @@ func RefreshAccessToken(cred *AuthCredential, cfg OAuthProviderConfig) (*AuthCre } defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("token refresh failed: %s", string(body)) } @@ -494,7 +491,7 @@ func ExchangeCodeForTokens(cfg OAuthProviderConfig, code, codeVerifier, redirect } defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("token exchange failed: %s", string(body)) } @@ -608,14 +605,25 @@ func base64URLDecode(s string) ([]byte, error) { } // OpenBrowser opens the given URL in the user's default browser. -func OpenBrowser(url string) error { +// Each platform call passes the URL as a discrete argument to avoid shell interpretation. +func OpenBrowser(rawURL string) error { + // Validate the URL before passing it to a browser launcher. + parsed, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("invalid URL: %w", err) + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return fmt.Errorf("refusing to open non-http(s) URL: %s", parsed.Scheme) + } switch runtime.GOOS { case "darwin": - return exec.Command("open", url).Start() + return exec.Command("open", rawURL).Start() case "linux": - return exec.Command("xdg-open", url).Start() + return exec.Command("xdg-open", rawURL).Start() case "windows": - return exec.Command("cmd", "/c", "start", url).Start() + // Use rundll32 instead of "cmd /c start" to avoid cmd.exe shell metacharacter + // interpretation (e.g., '&', '|', '^' in URLs would execute commands via cmd.exe). + return exec.Command("rundll32", "url.dll,FileProtocolHandler", rawURL).Start() default: return fmt.Errorf("unsupported platform: %s", runtime.GOOS) } diff --git a/pkg/providers/antigravity_provider.go b/pkg/providers/antigravity_provider.go index d4ee528b7..66a5185ce 100644 --- a/pkg/providers/antigravity_provider.go +++ b/pkg/providers/antigravity_provider.go @@ -4,10 +4,11 @@ import ( "bufio" "bytes" "context" + cryptorand "crypto/rand" "encoding/json" "fmt" "io" - "math/rand" + "math/big" "net/http" "strings" "time" @@ -758,8 +759,17 @@ func truncateString(s string, maxLen int) string { func randomString(n int) string { const letters = "abcdefghijklmnopqrstuvwxyz0123456789" b := make([]byte, n) + max := big.NewInt(int64(len(letters))) for i := range b { - b[i] = letters[rand.Intn(len(letters))] + idx, err := cryptorand.Int(cryptorand.Reader, max) + if err != nil { + // Fall back to a fixed character rather than panicking; callers use + // this only for non-secret request IDs, so degraded randomness is + // acceptable in the unlikely event crypto/rand fails. + b[i] = letters[0] + continue + } + b[i] = letters[idx.Int64()] } return string(b) } diff --git a/pkg/skills/installer.go b/pkg/skills/installer.go index c9f19f25d..1edc672a7 100644 --- a/pkg/skills/installer.go +++ b/pkg/skills/installer.go @@ -7,12 +7,18 @@ import ( "net/http" "os" "path/filepath" + "regexp" "time" "github.com/sipeed/picoclaw/pkg/fileutil" "github.com/sipeed/picoclaw/pkg/utils" ) +// validGitHubRepo matches "owner/repo" where owner and repo contain only +// alphanumeric characters, hyphens, underscores, and dots — no path traversal, +// query strings, fragments, or null bytes. +var validGitHubRepo = regexp.MustCompile(`^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`) + type SkillInstaller struct { workspace string } @@ -24,6 +30,9 @@ func NewSkillInstaller(workspace string) *SkillInstaller { } func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) error { + if !validGitHubRepo.MatchString(repo) { + return fmt.Errorf("invalid GitHub repo name %q: must be in 'owner/repo' format using only alphanumeric characters, hyphens, underscores, and dots", repo) + } skillDir := filepath.Join(si.workspace, "skills", filepath.Base(repo)) if _, err := os.Stat(skillDir); err == nil { diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go index cd8da3195..421a0a7e0 100644 --- a/pkg/tools/filesystem.go +++ b/pkg/tools/filesystem.go @@ -398,8 +398,12 @@ type whitelistFs struct { } func (w *whitelistFs) matches(path string) bool { + // Canonicalize the path before pattern matching to prevent bypass via + // embedded traversal sequences (e.g. /allowed/path/../../../etc/shadow + // would match the "/allowed/path" prefix but resolve elsewhere). + canonical := filepath.Clean(path) for _, p := range w.patterns { - if p.MatchString(path) { + if p.MatchString(canonical) { return true } } diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index a0c83eb1e..9d6bc8d82 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -26,35 +26,48 @@ type ExecTool struct { } var ( - defaultDenyPatterns = []*regexp.Regexp{ + // absoluteDenyPatterns are always blocked regardless of customAllowPatterns. + // These cover the most dangerous command patterns that should never be executed. + absoluteDenyPatterns = []*regexp.Regexp{ regexp.MustCompile(`\brm\s+-[rf]{1,2}\b`), regexp.MustCompile(`\bdel\s+/[fq]\b`), regexp.MustCompile(`\brmdir\s+/s\b`), - // Match disk wiping commands (must be followed by space/args) - regexp.MustCompile( - `\b(format|mkfs|diskpart)\b\s`, - ), + regexp.MustCompile(`\b(format|mkfs|diskpart)\b\s`), regexp.MustCompile(`\bdd\s+if=`), - // Block writes to block devices (all common naming schemes). regexp.MustCompile( `>\s*/dev/(sd[a-z]|hd[a-z]|vd[a-z]|xvd[a-z]|nvme\d|mmcblk\d|loop\d|dm-\d|md\d|sr\d|nbd\d)`, ), regexp.MustCompile(`\b(shutdown|reboot|poweroff)\b`), - regexp.MustCompile(`:\(\)\s*\{.*\};\s*:`), - regexp.MustCompile(`\$\([^)]+\)`), - regexp.MustCompile(`\$\{[^}]+\}`), - regexp.MustCompile("`[^`]+`"), + regexp.MustCompile(`:\(\)\s*\{.*\};\s*:`), // fork bomb + regexp.MustCompile(`\$\([^)]+\)`), // command substitution $() + regexp.MustCompile(`\$\{[^}]+\}`), // variable substitution ${} + regexp.MustCompile("`[^`]+`"), // backtick substitution regexp.MustCompile(`\|\s*sh\b`), regexp.MustCompile(`\|\s*bash\b`), + regexp.MustCompile(`\|\s*(\/bin\/)?sh\b`), + regexp.MustCompile(`\|\s*(\/bin\/)?bash\b`), + regexp.MustCompile(`\bbase64\b.*\|\s*(sh|bash|(\/bin\/)?(sh|bash))\b`), regexp.MustCompile(`;\s*rm\s+-[rf]`), regexp.MustCompile(`&&\s*rm\s+-[rf]`), regexp.MustCompile(`\|\|\s*rm\s+-[rf]`), + regexp.MustCompile(`\bsudo\b`), + regexp.MustCompile(`\beval\b`), + // Persist via cron/at/startup files + regexp.MustCompile(`\bcrontab\b`), + regexp.MustCompile(`\bat\s+now\b`), + regexp.MustCompile(`\.ssh[/\\]authorized_keys\b`), + regexp.MustCompile(`\.(bashrc|bash_profile|profile|zshrc|zprofile)\b`), + // Network exfiltration + regexp.MustCompile(`\b(nc|ncat|netcat)\b`), + } + + defaultDenyPatterns = []*regexp.Regexp{ + // Additional patterns that can be overridden by customAllowPatterns regexp.MustCompile(`<<\s*EOF`), regexp.MustCompile(`\$\(\s*cat\s+`), regexp.MustCompile(`\$\(\s*curl\s+`), regexp.MustCompile(`\$\(\s*wget\s+`), regexp.MustCompile(`\$\(\s*which\s+`), - regexp.MustCompile(`\bsudo\b`), regexp.MustCompile(`\bchmod\s+[0-7]{3,4}\b`), regexp.MustCompile(`\bchown\b`), regexp.MustCompile(`\bpkill\b`), @@ -72,8 +85,12 @@ var ( regexp.MustCompile(`\bgit\s+push\b`), regexp.MustCompile(`\bgit\s+force\b`), regexp.MustCompile(`\bssh\b.*@`), - regexp.MustCompile(`\beval\b`), regexp.MustCompile(`\bsource\s+.*\.sh\b`), + // Script interpreters running inline code + regexp.MustCompile(`\bpython[23]?\s+-c\b`), + regexp.MustCompile(`\bperl\s+-e\b`), + regexp.MustCompile(`\bruby\s+-e\b`), + regexp.MustCompile(`\bnohup\b`), } // absolutePathPattern matches absolute file paths in commands (Unix and Windows). @@ -291,7 +308,14 @@ func (t *ExecTool) guardCommand(command, cwd string) string { cmd := strings.TrimSpace(command) lower := strings.ToLower(cmd) - // Custom allow patterns exempt a command from deny checks. + // Absolute deny patterns are always enforced — customAllowPatterns cannot bypass them. + for _, pattern := range absoluteDenyPatterns { + if pattern.MatchString(lower) { + return "Command blocked by safety guard (dangerous pattern detected)" + } + } + + // Custom allow patterns can exempt a command from the configurable deny checks below. explicitlyAllowed := false for _, pattern := range t.customAllowPatterns { if pattern.MatchString(lower) { diff --git a/pkg/tools/web.go b/pkg/tools/web.go index 7b14686c9..a61439461 100644 --- a/pkg/tools/web.go +++ b/pkg/tools/web.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "net" "net/http" "net/url" "regexp" @@ -666,6 +667,44 @@ func NewWebFetchToolWithProxy(maxChars int, proxy string, fetchLimitBytes int64) }, nil } +// rejectSSRFTarget returns an error if the URL targets an address that should +// not be reachable from an agent tool: loopback, link-local, private ranges, +// and cloud metadata endpoints. This prevents prompt-injected instructions from +// exfiltrating data via the local launcher API or cloud instance metadata. +func rejectSSRFTarget(u *url.URL) error { + hostname := u.Hostname() + + // Resolve the hostname to catch DNS-based SSRF. + // Use the literal hostname for IP checks before DNS resolution too. + ips, err := net.LookupHost(hostname) + if err != nil { + // If we can't resolve it, let the HTTP client fail naturally. + // The important case is when we CAN resolve it and it's private. + ips = []string{hostname} + } + + for _, ipStr := range ips { + ip := net.ParseIP(ipStr) + if ip == nil { + continue + } + if ip.IsLoopback() { + return fmt.Errorf("SSRF protection: requests to loopback addresses are not allowed") + } + if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { + return fmt.Errorf("SSRF protection: requests to link-local addresses are not allowed") + } + if ip.IsPrivate() { + return fmt.Errorf("SSRF protection: requests to private network addresses are not allowed") + } + // Block cloud metadata endpoints (AWS, GCP, Azure all use 169.254.169.254). + if ip.Equal(net.ParseIP("169.254.169.254")) { + return fmt.Errorf("SSRF protection: requests to cloud metadata endpoints are not allowed") + } + } + return nil +} + func (t *WebFetchTool) Name() string { return "web_fetch" } @@ -711,6 +750,10 @@ func (t *WebFetchTool) Execute(ctx context.Context, args map[string]any) *ToolRe return ErrorResult("missing domain in URL") } + if err := rejectSSRFTarget(parsedURL); err != nil { + return ErrorResult(err.Error()) + } + maxChars := t.maxChars if mc, ok := args["maxChars"].(float64); ok { if int(mc) > 100 {