feat: hardening de tools — deny-by-default, SSRF, path traversal, allowlists

Cambios de seguridad en las 4 herramientas de agentes: - tools/file: deny-by-default (AllowedPaths vacío = todo denegado), resolución de symlinks con EvalSymlinks, protección contra path traversal (../) y confusión de prefijos (/opt vs /opt1234) - tools/ssh: nuevo AllowedCommands allowlist (complementa ForbiddenCommands), validación de sintaxis shell (bloquea pipes, subshells, redirects, chains) - tools/http: protección SSRF bloqueando IPs privadas, loopback, link-local, metadata (169.254.169.254). Validación de dominio case-insensitive. - tools/matrix: nuevo parámetro AllowedRooms para restringir rooms destino - internal/config/schema: AllowedCommands en SSHToolCfg, MatrixToolCfg nueva - agents/runtime: pasa MatrixToolCfg al constructor de matrix_send Parte de issue 0019 (prompt injection hardening). Feature flag OFF. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-07 19:17:00 +00:00
parent 71a009f890
commit 4e7aa95adb
6 changed files with 195 additions and 21 deletions
@@ -12,7 +12,8 @@ import (
 )

 // NewReadFile creates a read_file tool that reads local files.
-// Validates paths against cfg.AllowedPaths when non-empty.
+// Deny-by-default: if AllowedPaths is empty, all reads are rejected.
+// Resolves symlinks and normalizes paths to prevent traversal attacks.
 func NewReadFile(cfg config.FileOpsCfg) tools.Tool {
 	return tools.Tool{
 		Def: tools.Def{
@@ -53,18 +54,54 @@ func NewReadFile(cfg config.FileOpsCfg) tools.Tool {
 	}
 }

+// validatePath checks that absPath is under one of the allowed paths.
+// Deny-by-default: if allowedPaths is empty, no paths are allowed.
+// Resolves symlinks to prevent traversal via ../  or symlink escapes.
 func validatePath(absPath string, allowedPaths []string) error {
 	if len(allowedPaths) == 0 {
-		return nil
+		return fmt.Errorf("read_file: no allowed paths configured, all reads denied")
 	}
+
+	// Resolve symlinks on the requested path to get the real path.
+	// If the file doesn't exist yet, resolve the parent directory.
+	realPath, err := resolveReal(absPath)
+	if err != nil {
+		return fmt.Errorf("read_file: cannot resolve path %q: %w", absPath, err)
+	}
+
 	for _, allowed := range allowedPaths {
 		a, err := filepath.Abs(allowed)
 		if err != nil {
 			continue
 		}
-		if strings.HasPrefix(absPath, a) {
+		// Resolve symlinks on the allowed path too.
+		realAllowed, err := resolveReal(a)
+		if err != nil {
+			continue
+		}
+		// Ensure the real path is strictly under the allowed directory.
+		// Add trailing separator to prevent /opt matching /opt1234.
+		if strings.HasPrefix(realPath, realAllowed+string(filepath.Separator)) || realPath == realAllowed {
 			return nil
 		}
 	}
 	return fmt.Errorf("path %q not under any allowed path", absPath)
 }
+
+// resolveReal resolves symlinks for a path.
+// If the exact path doesn't exist, it resolves the deepest existing ancestor
+// and appends the remaining segments, preventing partial traversal.
+func resolveReal(path string) (string, error) {
+	real, err := filepath.EvalSymlinks(path)
+	if err == nil {
+		return filepath.Clean(real), nil
+	}
+	// Path doesn't exist — resolve parent and append base.
+	parent := filepath.Dir(path)
+	base := filepath.Base(path)
+	realParent, err := filepath.EvalSymlinks(parent)
+	if err != nil {
+		return "", err
+	}
+	return filepath.Clean(filepath.Join(realParent, base)), nil
+}
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"net/url"
 	"strings"
@@ -14,7 +15,8 @@ import (
 )

 // NewHTTPGet creates an http_get tool that performs GET requests.
-// Validates URLs against cfg.AllowedDomains when non-empty.
+// Validates URLs against cfg.AllowedDomains (deny-by-default if non-empty)
+// and blocks requests to internal/private IP ranges (SSRF protection).
 func NewHTTPGet(cfg config.HTTPToolCfg) tools.Tool {
 	timeout := cfg.Timeout
 	if timeout == 0 {
@@ -35,8 +37,8 @@ func NewHTTPGet(cfg config.HTTPToolCfg) tools.Tool {
 			if rawURL == "" {
 				return tools.Result{Err: fmt.Errorf("http_get: url is required")}
 			}
-			if err := validateDomain(rawURL, cfg.AllowedDomains); err != nil {
-				return tools.Result{Err: err}
+			if err := validateURL(rawURL, cfg.AllowedDomains); err != nil {
+				return tools.Result{Err: fmt.Errorf("http_get: %w", err)}
 			}

 			req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil)
@@ -61,7 +63,7 @@ func NewHTTPGet(cfg config.HTTPToolCfg) tools.Tool {
 }

 // NewHTTPPost creates an http_post tool that performs POST requests with a JSON body.
-// Validates URLs against cfg.AllowedDomains when non-empty.
+// Validates URLs against cfg.AllowedDomains and blocks internal IPs.
 func NewHTTPPost(cfg config.HTTPToolCfg) tools.Tool {
 	timeout := cfg.Timeout
 	if timeout == 0 {
@@ -87,8 +89,8 @@ func NewHTTPPost(cfg config.HTTPToolCfg) tools.Tool {
 			if bodyStr == "" {
 				return tools.Result{Err: fmt.Errorf("http_post: body is required")}
 			}
-			if err := validateDomain(rawURL, cfg.AllowedDomains); err != nil {
-				return tools.Result{Err: err}
+			if err := validateURL(rawURL, cfg.AllowedDomains); err != nil {
+				return tools.Result{Err: fmt.Errorf("http_post: %w", err)}
 			}

 			req, err := http.NewRequestWithContext(ctx, http.MethodPost, rawURL, strings.NewReader(bodyStr))
@@ -113,21 +115,84 @@ func NewHTTPPost(cfg config.HTTPToolCfg) tools.Tool {
 	}
 }

-// validateDomain checks that the URL's host is in the allowed list.
-// If allowedDomains is empty, all domains are allowed.
-func validateDomain(rawURL string, allowedDomains []string) error {
-	if len(allowedDomains) == 0 {
-		return nil
-	}
+// validateURL checks domain allowlist and blocks internal IPs (SSRF protection).
+func validateURL(rawURL string, allowedDomains []string) error {
 	u, err := url.Parse(rawURL)
 	if err != nil {
 		return fmt.Errorf("invalid url: %w", err)
 	}
+
 	host := u.Hostname()
+	if host == "" {
+		return fmt.Errorf("url has no host")
+	}
+
+	// SSRF protection: block internal/private IPs and localhost.
+	if err := rejectInternalHost(host); err != nil {
+		return err
+	}
+
+	// Domain allowlist (if configured).
+	if err := validateDomain(host, allowedDomains); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// validateDomain checks that the host is in the allowed list.
+// If allowedDomains is empty, all domains are allowed.
+func validateDomain(host string, allowedDomains []string) error {
+	if len(allowedDomains) == 0 {
+		return nil
+	}
+	lower := strings.ToLower(host)
 	for _, d := range allowedDomains {
-		if host == d {
+		if lower == strings.ToLower(d) {
 			return nil
 		}
 	}
 	return fmt.Errorf("domain %q not in allowed list", host)
 }
+
+// rejectInternalHost blocks requests to localhost, private IPs, and link-local addresses.
+func rejectInternalHost(host string) error {
+	lower := strings.ToLower(host)
+	if lower == "localhost" {
+		return fmt.Errorf("requests to localhost are blocked")
+	}
+
+	ip := net.ParseIP(host)
+	if ip == nil {
+		// Not an IP literal — could be a domain. Resolve it.
+		ips, err := net.LookupIP(host)
+		if err != nil {
+			return nil // let the HTTP client handle DNS errors
+		}
+		for _, resolved := range ips {
+			if isPrivateIP(resolved) {
+				return fmt.Errorf("domain %q resolves to private IP %s", host, resolved)
+			}
+		}
+		return nil
+	}
+
+	if isPrivateIP(ip) {
+		return fmt.Errorf("requests to private IP %s are blocked", ip)
+	}
+	return nil
+}
+
+// isPrivateIP returns true for loopback, private, link-local, and metadata IPs.
+func isPrivateIP(ip net.IP) bool {
+	return ip.IsLoopback() ||
+		ip.IsPrivate() ||
+		ip.IsLinkLocalUnicast() ||
+		ip.IsLinkLocalMulticast() ||
+		isMetadataIP(ip)
+}
+
+// isMetadataIP checks for cloud metadata service IPs (169.254.169.254).
+func isMetadataIP(ip net.IP) bool {
+	return ip.Equal(net.ParseIP("169.254.169.254"))
+}
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"

+	"github.com/enmanuel/agents/internal/config"
 	"github.com/enmanuel/agents/tools"
 )

@@ -15,7 +16,8 @@ type MatrixSender interface {
 }

 // NewMatrixSend creates a matrix_send tool that sends a message to a Matrix room.
-func NewMatrixSend(sender MatrixSender) tools.Tool {
+// If AllowedRooms is configured, only those room IDs can be targeted.
+func NewMatrixSend(sender MatrixSender, cfg config.MatrixToolCfg) tools.Tool {
 	return tools.Tool{
 		Def: tools.Def{
 			Name:        "matrix_send",
@@ -32,6 +34,10 @@ func NewMatrixSend(sender MatrixSender) tools.Tool {
 				return tools.Result{Err: fmt.Errorf("matrix_send: room_id and message are required")}
 			}

+			if err := validateRoom(roomID, cfg.AllowedRooms); err != nil {
+				return tools.Result{Err: err}
+			}
+
 			if err := sender.SendMarkdown(ctx, roomID, message); err != nil {
 				return tools.Result{Err: fmt.Errorf("matrix_send: %w", err)}
 			}
@@ -40,3 +46,17 @@ func NewMatrixSend(sender MatrixSender) tools.Tool {
 		},
 	}
 }
+
+// validateRoom checks that roomID is in the allowed list.
+// If allowedRooms is empty, all rooms are allowed.
+func validateRoom(roomID string, allowedRooms []string) error {
+	if len(allowedRooms) == 0 {
+		return nil
+	}
+	for _, r := range allowedRooms {
+		if roomID == r {
+			return nil
+		}
+	}
+	return fmt.Errorf("matrix_send: room %q not in allowed rooms list", roomID)
+}
@@ -12,7 +12,9 @@ import (
 )

 // NewSSHCommand creates an ssh_command tool that executes remote commands via SSH.
-// Validates targets against cfg.AllowedTargets and commands against cfg.ForbiddenCommands.
+// Validates targets against AllowedTargets (deny-by-default if non-empty),
+// commands against AllowedCommands allowlist (if non-empty, only those prefixes permitted),
+// and against ForbiddenCommands blocklist as a second defense layer.
 func NewSSHCommand(cfg config.SSHToolCfg, exec *shellssh.Executor) tools.Tool {
 	return tools.Tool{
 		Def: tools.Def{
@@ -33,7 +35,13 @@ func NewSSHCommand(cfg config.SSHToolCfg, exec *shellssh.Executor) tools.Tool {
 			if err := validateTarget(target, cfg.AllowedTargets); err != nil {
 				return tools.Result{Err: err}
 			}
-			if err := validateCommand(command, cfg.ForbiddenCommands); err != nil {
+			if err := validateAllowedCommand(command, cfg.AllowedCommands); err != nil {
+				return tools.Result{Err: err}
+			}
+			if err := validateForbiddenCommand(command, cfg.ForbiddenCommands); err != nil {
+				return tools.Result{Err: err}
+			}
+			if err := validateCommandSyntax(command); err != nil {
 				return tools.Result{Err: err}
 			}

@@ -73,7 +81,23 @@ func validateTarget(target string, allowed []string) error {
 	return fmt.Errorf("ssh target %q not in allowed list", target)
 }

-func validateCommand(command string, forbidden []string) error {
+// validateAllowedCommand checks that the command starts with one of the allowed prefixes.
+// If the allowlist is empty, all commands pass this check (blocklist still applies).
+func validateAllowedCommand(command string, allowed []string) error {
+	if len(allowed) == 0 {
+		return nil
+	}
+	lower := strings.ToLower(command)
+	for _, a := range allowed {
+		if strings.HasPrefix(lower, strings.ToLower(a)) {
+			return nil
+		}
+	}
+	return fmt.Errorf("ssh command not in allowed commands list")
+}
+
+// validateForbiddenCommand checks that the command does not contain any forbidden patterns.
+func validateForbiddenCommand(command string, forbidden []string) error {
 	lower := strings.ToLower(command)
 	for _, f := range forbidden {
 		if strings.Contains(lower, strings.ToLower(f)) {
@@ -82,3 +106,25 @@ func validateCommand(command string, forbidden []string) error {
 	}
 	return nil
 }
+
+// validateCommandSyntax rejects commands with suspicious shell constructs
+// that could be used to bypass restrictions: pipes to external services,
+// subshells, and output redirection.
+func validateCommandSyntax(command string) error {
+	suspicious := []string{
+		"|",  // pipe (can exfiltrate output)
+		"$(", // command substitution
+		"`",  // backtick substitution
+		">>", // append redirection
+		">",  // output redirection
+		"&&", // command chaining
+		"||", // command chaining
+		";",  // command separator
+	}
+	for _, s := range suspicious {
+		if strings.Contains(command, s) {
+			return fmt.Errorf("ssh command contains disallowed shell syntax %q", s)
+		}
+	}
+	return nil
+}