fix(security): consolidate & enhance batched security fixes (#1155, #967, #972, #974, #989, #973) (#1185)

* fix(sandbox): avoid shell in FsBridge writes

Replace sh -c with interpolated path by shell-free 'tee -- <path>' argv form,
piping content via stdin. Prevents command injection through filenames
containing shell metacharacters inside the sandbox container.

Co-authored-by: evgyur <evgyur@gmail.com>

* fix(security): fail-closed on pairing DB errors across channels

On IsPaired lookup error, deny instead of granting access. Covers the shared
CheckDMPolicy/CheckGroupPolicy helpers (Slack/Discord/Feishu/WhatsApp/Zalo) and
the four inline Telegram pairing checks.

Co-authored-by: Srini <srinis.k@gmail.com>

* fix(security): harden provider URL validation against SSRF

Enforce scheme check for all provider types; restrict local types (ollama,
claude_cli, acp) to an explicit localhost allowlist instead of skipping checks;
resolve remote hostnames and reject any IP in a private/reserved range via the
shared security.IsBlocked CIDR list (covers loopback, link-local, metadata,
multicast, and unspecified 0.0.0.0/::). Closes the wildcard-DNS bypass and the
local-type escape hatch. Operator opt-in via GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS.

Exports security.IsBlocked as the single source of truth for blocked ranges.

Co-authored-by: Linh Vo Van <linh.vo@e-cq.net>

* feat(pipeline): add fail-closed tool call authorization gate

Gate tool execution against the server-side AllowedTools allowlist built from the
RBAC/tenant-aware filtered tool set. Resolve the tool-call prefix before the
allowlist lookup so prefixed agents are not wrongly blocked, re-check deny on lazy
MCP activation, and expand IsDenied to cover aliased tool names.

Co-authored-by: Huy Doan <tui@pm.me>

* fix(security): expand file-serve deny-list defense-in-depth

Add absolute-path deny prefixes (/home, /Users, /srv, /var/lib, /var/www, /opt)
and an explicit fail-closed log when no file-serving boundary is configured.

Co-authored-by: Linh Vo Van <linh.vo@e-cq.net>

* fix(providers): allow claude cli executable paths

Refs: #1185

---------

Co-authored-by: evgyur <evgyur@gmail.com>
Co-authored-by: Srini <srinis.k@gmail.com>
Co-authored-by: Linh Vo Van <linh.vo@e-cq.net>
Co-authored-by: Huy Doan <tui@pm.me>

docs/02-providers.md

@@ -554,7 +554,7 @@ ClaudeCLIProvider can be configured in `config.json`:
 }
 ```
 
-Or via database `llm_providers` table with `provider_type = "claude_cli"`.
+Or via database `llm_providers` table with `provider_type = "claude_cli"`. For database providers, `api_base` is the CLI executable selector (`"claude"` or an absolute binary path), not an HTTP base URL, so provider URL SSRF opt-ins do not apply to Claude CLI.
 
 ### Session Management
 

internal/agent/loop_lazy_mcp_test.go

@@ -5,12 +5,14 @@ import (
 	"testing"
 
 	"github.com/nextlevelbuilder/goclaw/internal/config"
+	"github.com/nextlevelbuilder/goclaw/internal/pipeline"
+	"github.com/nextlevelbuilder/goclaw/internal/providers"
 	"github.com/nextlevelbuilder/goclaw/internal/tools"
 )
 
 // mockExecTool is a simple tool that records whether it was executed.
 type mockExecTool struct {
-	name    string
+	name     string
 	executed bool
 }
 
@@ -24,13 +26,16 @@ func (m *mockExecTool) Execute(_ context.Context, _ map[string]any) *tools.Resul
 	return tools.NewResult("ok from " + m.name)
 }
 
-// simulateLazyActivationCheck mimics the allowedTools check from loop.go for one tool call:
+// simulateLazyActivationCheck mimics the runtime authorize gate from
+// makeAuthorizeToolCall (loop_pipeline_callbacks.go) for one tool call.
+// The real gate is a PipelineDeps.AuthorizeToolCall callback invoked by ToolStage;
+// this helper exercises the same logic in isolation so tests stay unit-level.
 //
-//	if allowedTools != nil && !allowedTools[tc.Name] {
-//	    if l.tools.TryActivateDeferred(tc.Name) { allowedTools[tc.Name] = true }
+//	if allowed != nil && !allowed[name] {
+//	    if reg.TryActivateDeferred(name) { allowed[name] = true }
 //	    else { result = ErrorResult(...) }
 //	}
-//	if result == nil { result = l.tools.ExecuteWithContext(...) }
+//	if result == nil { result = reg.ExecuteWithContext(...) }
 //
 // Returns (result, blocked).
 func simulateLazyActivationCheck(reg *tools.Registry, allowedTools map[string]bool, toolName string) (*tools.Result, bool) {
@@ -163,6 +168,24 @@ func TestLoop_LazyMCP_NilAllowedTools_AllowsAll(t *testing.T) {
 	}
 }
 
+func TestLoopAuthorizeToolCall_PrefixedNameCanonicalLookup(t *testing.T) {
+	loop := NewLoop(LoopConfig{
+		AgentToolPolicy: &config.ToolPolicySpec{ToolCallPrefix: "proxy_"},
+	})
+	gate := loop.makeAuthorizeToolCall()
+	state := &pipeline.RunState{}
+	state.Tool.AllowedTools = map[string]bool{"exec": true}
+
+	ok, reason := gate(context.Background(), state, providers.ToolCall{
+		ID:   "tc-proxy-exec",
+		Name: "proxy_exec",
+	})
+
+	if !ok {
+		t.Fatalf("prefixed tool call should resolve to canonical allowlist entry; reason: %q", reason)
+	}
+}
+
 func TestLoop_LazyMCP_SecondCall_UsesUpdatedAllowedTools(t *testing.T) {
 	// After first lazy activation, subsequent calls in the same iteration use the
 	// updated allowedTools map and don't invoke the activator again.

internal/agent/loop_pipeline_adapter.go

@@ -141,6 +141,7 @@ func (l *Loop) buildPipelineDeps(req *RunRequest, bridgeRS *runState) pipeline.P
 		ExecuteToolCall:   cb.executeToolCall,
 		ExecuteToolRaw:    cb.executeToolRaw,
 		ProcessToolResult: cb.processToolResult,
+		AuthorizeToolCall: cb.authorizeToolCall,
 		SequentialToolCall: func(tc providers.ToolCall) bool {
 			return l.resolveToolCallName(tc.Name) == "wait"
 		},

internal/agent/loop_pipeline_callbacks.go

@@ -57,6 +57,7 @@ func (l *Loop) pipelineCallbacks(req *RunRequest, bridgeRS *runState) pipelineCa
 		executeToolCall:    l.makeExecuteToolCall(req, bridgeRS),
 		executeToolRaw:     l.makeExecuteToolRaw(req),
 		processToolResult:  l.makeProcessToolResult(req, bridgeRS),
+		authorizeToolCall:  l.makeAuthorizeToolCall(),
 		checkReadOnly:      l.makeCheckReadOnly(req, bridgeRS),
 		sanitizeContent:    SanitizeAssistantContent,
 		flushMessages:      l.makeFlushMessages(req),
@@ -85,6 +86,7 @@ type pipelineCallbackSet struct {
 	executeToolCall    func(ctx context.Context, state *pipeline.RunState, tc providers.ToolCall) ([]providers.Message, error)
 	executeToolRaw     func(ctx context.Context, tc providers.ToolCall) (providers.Message, any, error)
 	processToolResult  func(ctx context.Context, state *pipeline.RunState, tc providers.ToolCall, rawMsg providers.Message, rawData any) []providers.Message
+	authorizeToolCall  func(ctx context.Context, state *pipeline.RunState, tc providers.ToolCall) (bool, string)
 	checkReadOnly      func(state *pipeline.RunState) (*providers.Message, bool)
 	sanitizeContent    func(string) string
 	flushMessages      func(ctx context.Context, sessionKey string, msgs []providers.Message) error
@@ -249,6 +251,44 @@ func (l *Loop) makeBuildFilteredTools(req *RunRequest) func(state *pipeline.RunS
 	}
 }
 
+// makeAuthorizeToolCall enforces a runtime fail-closed allowlist check before
+// every tool execution. AllowedTools is keyed by canonical registry names (built
+// by ThinkStage from FilterTools output). The model may emit prefixed names when
+// the agent has toolCallPrefix configured (e.g. "proxy_exec" → canonical "exec"),
+// so we resolve the name to its canonical form before the lookup to avoid a
+// guaranteed miss on every prefixed call.
+func (l *Loop) makeAuthorizeToolCall() func(ctx context.Context, state *pipeline.RunState, tc providers.ToolCall) (bool, string) {
+	return func(_ context.Context, state *pipeline.RunState, tc providers.ToolCall) (bool, string) {
+		allowed := state.Tool.AllowedTools
+		if allowed == nil {
+			// nil allowlist means no per-iteration restriction (e.g. BuildFilteredTools not wired).
+			return true, ""
+		}
+
+		// Resolve to canonical name before allowlist lookup. AllowedTools is keyed
+		// by canonical names; the model may emit prefixed names when toolCallPrefix
+		// is set (e.g. "proxy_exec" vs "exec"). Without this the lookup always misses.
+		name := l.resolveToolCallName(tc.Name)
+
+		if allowed[name] {
+			return true, ""
+		}
+
+		// Preserve lazy activation for deferred tools (typically per-user MCP).
+		if l.tools != nil && l.tools.TryActivateDeferred(name) {
+			// Re-check deny policy to prevent a lazy-activated tool from bypassing
+			// an explicit deny rule.
+			if l.toolPolicy != nil && l.toolPolicy.IsDenied(name, l.agentToolPolicy) {
+				return false, "tool not allowed by policy: " + name
+			}
+			allowed[name] = true
+			return true, ""
+		}
+
+		return false, "tool not allowed by policy: " + name
+	}
+}
+
 func (l *Loop) makeCallLLM(req *RunRequest, emitRun func(AgentEvent)) func(ctx context.Context, state *pipeline.RunState, chatReq providers.ChatRequest) (*providers.ChatResponse, error) {
 	return func(ctx context.Context, state *pipeline.RunState, chatReq providers.ChatRequest) (*providers.ChatResponse, error) {
 		provider := state.Provider

internal/channels/channel.go

@@ -340,9 +340,9 @@ func (c *BaseChannel) CheckDMPolicy(ctx context.Context, senderID, dmPolicy stri
 		if c.pairingService != nil {
 			paired, err := c.pairingService.IsPaired(ctx, senderID, c.name)
 			if err != nil {
-				slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
+				slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
 					"sender_id", senderID, "channel", c.name, "error", err)
-				return PolicyAllow
+				return PolicyDeny
 			}
 			if paired {
 				return PolicyAllow
@@ -379,9 +379,9 @@ func (c *BaseChannel) CheckGroupPolicy(ctx context.Context, senderID, chatID, gr
 		if c.pairingService != nil {
 			paired, err := c.pairingService.IsPaired(ctx, groupSenderID, c.name)
 			if err != nil {
-				slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
+				slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
 					"group_sender", groupSenderID, "channel", c.name, "error", err)
-				return PolicyAllow
+				return PolicyDeny
 			}
 			if paired {
 				c.MarkGroupApproved(chatID)

internal/channels/policy_test.go

@@ -174,12 +174,12 @@ func TestCheckDMPolicy_PolicyPairing(t *testing.T) {
 			wantResult:       PolicyAllow,
 		},
 		{
-			name:             "Pairing service error allows message (fail-open)",
+			name:             "Pairing service error denies message (fail-closed)",
 			senderID:         "user999",
 			allowList:        []string{},
 			paired:           false,
 			failPairingCheck: true,
-			wantResult:       PolicyAllow,
+			wantResult:       PolicyDeny,
 		},
 	}
 
@@ -319,14 +319,14 @@ func TestCheckGroupPolicy_PolicyPairing(t *testing.T) {
 			wantResult:    PolicyNeedsPairing,
 		},
 		{
-			name:             "Pairing service error allows (fail-open)",
+			name:             "Pairing service error denies (fail-closed)",
 			senderID:         "user999",
 			chatID:           "chat_2",
 			allowList:        []string{},
 			groupApproved:    false,
 			paired:           false,
 			failPairingCheck: true,
-			wantResult:       PolicyAllow,
+			wantResult:       PolicyDeny,
 		},
 	}
 
@@ -553,13 +553,13 @@ func TestCheckDMPolicy_AllPolicies_TableDriven(t *testing.T) {
 			wantResult: PolicyNeedsPairing,
 		},
 		{
-			name:             "pairing policy fail-open on service error",
+			name:             "pairing policy fail-closed on service error",
 			policy:           "pairing",
 			senderID:         "user3",
 			allowList:        []string{},
 			paired:           false,
 			failPairingCheck: true,
-			wantResult:       PolicyAllow,
+			wantResult:       PolicyDeny,
 		},
 	}
 

internal/channels/telegram/handlers.go

@@ -159,9 +159,8 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) {
 				p1, err1 := ps.IsPaired(ctx, userID, c.Name())
 				p2, err2 := ps.IsPaired(ctx, senderID, c.Name())
 				if err1 != nil || err2 != nil {
-					slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
+					slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
 						"user_id", userID, "channel", c.Name(), "err1", err1, "err2", err2)
-					paired = true
 				} else {
 					paired = p1 || p2
 				}
@@ -265,9 +264,8 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) {
 					groupSenderID := fmt.Sprintf("group:%d", chatID)
 					paired, pairErr := c.PairingService().IsPaired(ctx, groupSenderID, c.Name())
 					if pairErr != nil {
-						slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
+						slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
 							"group_sender", groupSenderID, "channel", c.Name(), "error", pairErr)
-						paired = true
 					}
 					if paired {
 						c.MarkGroupApproved(chatIDStr)
@@ -320,9 +318,8 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) {
 					groupSenderID := fmt.Sprintf("group:%d", chatID)
 					paired, pairErr := c.PairingService().IsPaired(ctx, groupSenderID, c.Name())
 					if pairErr != nil {
-						slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
+						slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
 							"group_sender", groupSenderID, "channel", c.Name(), "error", pairErr)
-						paired = true
 					}
 					if paired {
 						c.MarkGroupApproved(chatIDStr)
@@ -376,9 +373,8 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) {
 			groupSenderID := fmt.Sprintf("group:%d", chatID)
 			paired, err := c.PairingService().IsPaired(ctx, groupSenderID, c.Name())
 			if err != nil {
-				slog.Warn("security.pairing_check_failed, assuming paired (fail-open)",
+				slog.Warn("security.pairing_check_failed, denying access (fail-closed)",
 					"group_sender", groupSenderID, "channel", c.Name(), "error", err)
-				paired = true
 			}
 			if paired {
 				c.MarkGroupApproved(chatIDStr)

internal/http/files.go

@@ -95,11 +95,16 @@ func (h *FilesHandler) auth(next http.HandlerFunc) http.HandlerFunc {
 
 // deniedFilePrefixes blocks access to sensitive system directories.
 // Defense-in-depth: the auth token is the primary barrier, but restricting
-// known-sensitive paths limits damage if a token leaks.
+// known-sensitive absolute paths limits damage if a token leaks or a root is
+// misconfigured. All entries must be absolute paths because hasDeniedFilePrefix
+// uses pathWithinDir semantics — relative-rooted entries (e.g. "/.ssh") would
+// only block the literal filesystem subtree at that root, not user home dirs.
 var deniedFilePrefixes = []string{
 	"/etc/", "/proc/", "/sys/", "/dev/",
 	"/root/", "/boot/", "/run/",
-	"/var/run/", "/var/log/",
+	"/var/run/", "/var/log/", "/var/lib/", "/var/www/",
+	"/home/", "/Users/",
+	"/opt/", "/srv/",
 }
 
 func (h *FilesHandler) handleServe(w http.ResponseWriter, r *http.Request) {
@@ -127,6 +132,14 @@ func (h *FilesHandler) handleServe(w http.ResponseWriter, r *http.Request) {
 	}
 
 	signed := r.URL.Query().Get("ft") != ""
+	// Fail-closed observability: if no boundary roots are configured at all,
+	// lexicallyAllowsFilePath already returns false, but log explicitly so
+	// operators can detect misconfigured deployments.
+	if h.workspace == "" && h.dataDir == "" {
+		slog.Warn("security.files_no_boundary", "path", absPath)
+		http.NotFound(w, r)
+		return
+	}
 	if !h.lexicallyAllowsFilePath(r, absPath, signed) {
 		slog.Warn("security.files_path_denied", "path", absPath, "workspace", h.workspace, "data_dir", h.dataDir)
 		http.NotFound(w, r)

internal/http/files_path_security_test.go

@@ -88,6 +88,60 @@ func TestFilesHandleServe_ProcDir_Blocked(t *testing.T) {
 	}
 }
 
+// ---- handleServe: expanded deny-prefix list ----
+
+// TestFilesHandleServe_ExpandedDenyPrefixes verifies that paths under /home/,
+// /Users/, /var/lib/, /var/www/, /opt/, and /srv/ are all blocked. These are
+// defense-in-depth blocks for misconfigured roots — the token/RBAC checks are
+// the primary barriers.
+func TestFilesHandleServe_ExpandedDenyPrefixes_Blocked(t *testing.T) {
+	h, _ := makeTestFilesHandler(t)
+	mux := http.NewServeMux()
+	mux.HandleFunc("GET /v1/files/{path...}", h.handleServe)
+
+	cases := []struct {
+		name string
+		url  string
+	}{
+		{"home user ssh key", "/v1/files/home/user/.ssh/id_rsa"},
+		{"home user aws creds", "/v1/files/home/user/.aws/credentials"},
+		{"Users admin dir (macOS)", "/v1/files/Users/admin/secrets.txt"},
+		{"var lib docker secret", "/v1/files/var/lib/docker/overlay2/secret"},
+		{"var www config", "/v1/files/var/www/config.php"},
+		{"opt secrets key", "/v1/files/opt/secrets/key.pem"},
+		{"srv www app config", "/v1/files/srv/www/app/config.yaml"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, tc.url, nil)
+			w := httptest.NewRecorder()
+			mux.ServeHTTP(w, req)
+			if w.Code == http.StatusOK {
+				t.Errorf("path %s should be denied (deny-prefix), got 200", tc.url)
+			}
+		})
+	}
+}
+
+// ---- handleServe: fail-closed observability on empty workspace/dataDir ----
+
+// TestFilesHandleServe_NoBoundary_Denies verifies that when both workspace and
+// dataDir are empty the handler returns 404 (fail-closed) for any path.
+func TestFilesHandleServe_NoBoundary_Denies(t *testing.T) {
+	h := NewFilesHandler("", "")
+	mux := http.NewServeMux()
+	mux.HandleFunc("GET /v1/files/{path...}", h.handleServe)
+
+	req := httptest.NewRequest(http.MethodGet, "/v1/files/tmp/test.txt", nil)
+	w := httptest.NewRecorder()
+	mux.ServeHTTP(w, req)
+
+	if w.Code == http.StatusOK {
+		t.Errorf("empty workspace+dataDir should deny all requests, got 200")
+	}
+}
+
 // ---- handleServe: workspace boundary enforcement ----
 
 func TestFilesHandleServe_FileInsideWorkspace_WithToken_Serves(t *testing.T) {

internal/http/oauth_test.go

@@ -65,6 +65,12 @@ func (m *mockProviderStore) UpdateProvider(_ context.Context, id uuid.UUID, upda
 			if v, ok := updates["api_key"]; ok {
 				p.APIKey = v.(string)
 			}
+			if v, ok := updates["api_base"]; ok {
+				p.APIBase = v.(string)
+			}
+			if v, ok := updates["provider_type"]; ok {
+				p.ProviderType = v.(string)
+			}
 			if v, ok := updates["settings"]; ok {
 				p.Settings = v.(json.RawMessage)
 			}

internal/http/providers.go

@@ -8,6 +8,7 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
@@ -21,6 +22,7 @@ import (
 	"github.com/nextlevelbuilder/goclaw/internal/oauth"
 	"github.com/nextlevelbuilder/goclaw/internal/permissions"
 	"github.com/nextlevelbuilder/goclaw/internal/providers"
+	"github.com/nextlevelbuilder/goclaw/internal/security"
 	"github.com/nextlevelbuilder/goclaw/internal/store"
 	usagecaps "github.com/nextlevelbuilder/goclaw/internal/usage/caps"
 	"github.com/nextlevelbuilder/goclaw/pkg/protocol"
@@ -322,55 +324,138 @@ func normalizeOllamaAPIBase(p *store.LLMProviderData) {
 	}
 }
 
-// localProviderTypes are provider types that legitimately run on localhost
-// (e.g. Ollama, Claude CLI). SSRF checks are skipped for these.
-var localProviderTypes = map[string]bool{
-	store.ProviderOllama:    true,
-	store.ProviderClaudeCLI: true,
-	store.ProviderACP:       true,
+// localURLProviderTypes are provider types that legitimately run on localhost.
+// They are restricted to an explicit localhost allowlist
+// rather than skipping SSRF validation entirely.
+var localURLProviderTypes = map[string]bool{
+	store.ProviderOllama: true,
+	store.ProviderACP:    true,
 }
 
+// allowedLocalHosts are the only hosts permitted for local provider types.
+// Explicit allowlist (not blocklist) to prevent new internal addresses from
+// slipping through (e.g. 169.254.169.254 via ollama base URL).
+var allowedLocalHosts = []string{"localhost", "127.0.0.1", "::1", "host.docker.internal"}
+
+// dnsResolverFn resolves hostnames to IPs. Replaceable in tests.
+var dnsResolverFn = net.LookupHost
+
+// allowPrivateProviderURLsFn reports whether the operator has opted in to
+// permitting private / loopback / link-local / internal-hostname provider base
+// URLs via GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS. Evaluated once at first call so
+// tests can override the variable before that happens.
+var allowPrivateProviderURLsFn = sync.OnceValue(func() bool {
+	v := strings.ToLower(strings.TrimSpace(os.Getenv("GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS")))
+	return v == "1" || v == "true" || v == "yes"
+})
+
 // validateProviderURL rejects provider base URLs pointing to internal/private networks.
 // Defense-in-depth: prevents SSRF when providers are later used for API calls.
+//
+// Logic:
+//  1. Empty URL → allowed (provider may not need a custom base).
+//  2. Claude CLI → api_base is an executable path/command, not a URL.
+//  3. Scheme check (http/https only) → enforced for URL-based types, including
+//     local URL types. Blocks file://, gopher://, dict://, etc.
+//  4. Local URL types (ollama, acp) → host must be in allowedLocalHosts
+//     (explicit allowlist prevents reaching 169.254.169.254 or internal services
+//     via the local-type bypass).
+//  5. Remote types → if GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS is set, allow and log.
+//     Otherwise: resolve DNS hostname; reject if ANY resolved IP satisfies
+//     security.IsBlocked (covers loopback, link-local, private, multicast,
+//     unspecified — including 0.0.0.0 and :: that earlier hand-rolled checks missed).
+//
+// DNS resolution on step 5 closes the nip.io / sslip.io / attacker-domain bypass
+// where a hostname passes a literal-string blocklist but resolves to a private IP.
 func validateProviderURL(rawURL string, providerType string) error {
-	if rawURL == "" || localProviderTypes[providerType] {
+	if rawURL == "" {
 		return nil
 	}
+	if providerType == store.ProviderClaudeCLI {
+		return validateClaudeCLIExecutablePath(rawURL)
+	}
 	u, err := url.Parse(rawURL)
 	if err != nil {
 		return fmt.Errorf("invalid URL: %w", err)
 	}
-	// Only allow http/https schemes — block file://, gopher://, dict://, etc.
+	// Scheme check is unconditional for URL-based provider types, including local URL types.
 	switch u.Scheme {
 	case "http", "https":
 	default:
 		return fmt.Errorf("provider URL must use http or https scheme, got %q", u.Scheme)
 	}
+
 	host := u.Hostname()
-	// Block obvious internal targets
-	blocked := []string{"localhost", "127.0.0.1", "::1", "0.0.0.0", "169.254.169.254", "metadata.google.internal"}
-	for _, b := range blocked {
-		if strings.EqualFold(host, b) {
-			return fmt.Errorf("provider URL cannot point to %s", b)
+
+	// Local provider types: only allow an explicit localhost allowlist.
+	// This prevents using the local-type escape hatch to reach internal services
+	// or cloud metadata endpoints.
+	if localURLProviderTypes[providerType] {
+		for _, a := range allowedLocalHosts {
+			if strings.EqualFold(host, a) {
+				return nil
+			}
 		}
+		slog.Warn("security.provider_url.local_type_denied", "host", host, "provider_type", providerType)
+		return fmt.Errorf("provider type %q only allows localhost URLs (localhost, 127.0.0.1, ::1, host.docker.internal), got host %q", providerType, host)
 	}
-	// Block private IP ranges (normalize IPv6-mapped IPv4 to catch ::ffff:127.0.0.1)
-	ip := net.ParseIP(host)
-	if ip != nil {
-		if v4 := ip.To4(); v4 != nil {
-			ip = v4
-		}
-		if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() {
-			return fmt.Errorf("provider URL cannot point to private network: %s", host)
-		}
+
+	// Operator opt-in to allow private-network provider URLs (e.g. LAN-hosted vLLM).
+	// Scheme check above still applies even with this gate open.
+	if allowPrivateProviderURLsFn() {
+		slog.Warn("security.provider_url.private_allowed", "host", host, "provider_type", providerType)
+		return nil
 	}
-	// Block common internal hostnames
+
+	// Check literal IP first (avoids unnecessary DNS lookup).
+	if ip := net.ParseIP(host); ip != nil {
+		if security.IsBlocked(ip) {
+			slog.Warn("security.provider_url.blocked", "host", host, "provider_type", providerType)
+			return fmt.Errorf("provider URL cannot point to %s", host)
+		}
+		return nil
+	}
+
+	// Block .internal / .local suffix before DNS (fail-fast for well-known patterns).
 	if strings.HasSuffix(host, ".internal") || strings.HasSuffix(host, ".local") {
+		slog.Warn("security.provider_url.blocked", "host", host, "provider_type", providerType)
 		return fmt.Errorf("provider URL cannot point to internal hostname: %s", host)
 	}
+
+	// Resolve DNS and check every returned address.
+	// Prevents bypass via wildcard services (nip.io, sslip.io) or attacker-controlled
+	// domains that map to private IPs (DNS-rebinding at config time).
+	addrs, err := dnsResolverFn(host)
+	if err != nil {
+		slog.Warn("security.provider_url.dns_resolve_failed", "host", host, "provider_type", providerType, "error", err)
+		return fmt.Errorf("provider URL hostname %q could not be resolved: %w", host, err)
+	}
+	for _, addr := range addrs {
+		ip := net.ParseIP(addr)
+		if ip == nil {
+			continue
+		}
+		if security.IsBlocked(ip) {
+			slog.Warn("security.provider_url.blocked_resolved", "host", host, "resolved_ip", ip.String(), "provider_type", providerType)
+			return fmt.Errorf("provider URL %q resolves to private/reserved address %s", host, ip)
+		}
+	}
 	return nil
 }
 
+func validateClaudeCLIExecutablePath(path string) error {
+	if strings.Contains(path, "\x00") {
+		return fmt.Errorf("Claude CLI executable path cannot contain NUL byte")
+	}
+	if _, err := url.ParseRequestURI(path); err == nil && strings.Contains(path, "://") {
+		return fmt.Errorf("Claude CLI api_base must be an executable path or %q, got URL %q", "claude", path)
+	}
+	if path == "claude" || filepath.IsAbs(path) {
+		return nil
+	}
+	return fmt.Errorf("Claude CLI api_base must be %q or an absolute executable path, got %q", "claude", path)
+}
+
 // --- Provider CRUD ---
 
 func (h *ProvidersHandler) handleListProviders(w http.ResponseWriter, r *http.Request) {

internal/http/providers_test.go

@@ -298,6 +298,79 @@ func TestProvidersHandlerCreateAllows1536EmbeddingDimensions(t *testing.T) {
 	}
 }
 
+func TestProvidersHandlerCreateAllowsClaudeCLIExecutablePath(t *testing.T) {
+	token := setupProvidersAdminToken(t)
+	providerStore := newMockProviderStore()
+	handler := NewProvidersHandler(providerStore, newMockSecretsStore(), nil, "")
+	mux := http.NewServeMux()
+	handler.RegisterRoutes(mux)
+
+	body := map[string]any{
+		"name":          "claude-local",
+		"provider_type": store.ProviderClaudeCLI,
+		"api_base":      writeFakeClaudeBinary(t),
+		"enabled":       true,
+	}
+	rawBody, err := json.Marshal(body)
+	if err != nil {
+		t.Fatalf("Marshal() error = %v", err)
+	}
+
+	req := httptest.NewRequest(http.MethodPost, "/v1/providers", bytes.NewReader(rawBody))
+	req.Header.Set("Authorization", "Bearer "+token)
+	w := httptest.NewRecorder()
+	mux.ServeHTTP(w, req)
+
+	if w.Code != http.StatusCreated {
+		t.Fatalf("status code = %d, want %d, body=%s", w.Code, http.StatusCreated, w.Body.String())
+	}
+	if got := providerStore.providers["claude-local"].APIBase; got == "" || !filepath.IsAbs(got) {
+		t.Fatalf("stored Claude CLI api_base = %q, want absolute executable path", got)
+	}
+}
+
+func TestProvidersHandlerUpdateAllowsClaudeCLIExecutablePath(t *testing.T) {
+	token := setupProvidersAdminToken(t)
+	providerStore := newMockProviderStore()
+	provider := &store.LLMProviderData{
+		BaseModel:    store.BaseModel{ID: uuid.New()},
+		Name:         "claude-local",
+		ProviderType: store.ProviderClaudeCLI,
+		APIBase:      "claude",
+		Enabled:      true,
+	}
+	if err := providerStore.CreateProvider(context.Background(), provider); err != nil {
+		t.Fatalf("CreateProvider() error = %v", err)
+	}
+
+	handler := NewProvidersHandler(providerStore, newMockSecretsStore(), nil, "")
+	mux := http.NewServeMux()
+	handler.RegisterRoutes(mux)
+
+	nextPath := writeFakeClaudeBinary(t)
+	body := map[string]any{"api_base": nextPath}
+	rawBody, err := json.Marshal(body)
+	if err != nil {
+		t.Fatalf("Marshal() error = %v", err)
+	}
+
+	req := httptest.NewRequest(http.MethodPut, "/v1/providers/"+provider.ID.String(), bytes.NewReader(rawBody))
+	req.Header.Set("Authorization", "Bearer "+token)
+	w := httptest.NewRecorder()
+	mux.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("status code = %d, want %d, body=%s", w.Code, http.StatusOK, w.Body.String())
+	}
+	current, err := providerStore.GetProvider(context.Background(), provider.ID)
+	if err != nil {
+		t.Fatalf("GetProvider() error = %v", err)
+	}
+	if current.APIBase != nextPath {
+		t.Fatalf("api_base = %q, want %q", current.APIBase, nextPath)
+	}
+}
+
 func TestProvidersHandlerUpdateRejectsIncompatibleEmbeddingDimensions(t *testing.T) {
 	token := setupProvidersAdminToken(t)
 	providerStore := newMockProviderStore()

internal/http/providers_url_validate_test.go

@@ -0,0 +1,326 @@
+package http
+
+import (
+	"net"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// stubResolver returns a net.LookupHost-compatible function that maps known
+// hostnames to controlled IPs. Unknown hostnames return a DNS error.
+func stubResolver(m map[string][]string) func(host string) ([]string, error) {
+	return func(host string) ([]string, error) {
+		if addrs, ok := m[host]; ok {
+			return addrs, nil
+		}
+		return nil, &net.DNSError{Err: "no such host", Name: host, IsNotFound: true}
+	}
+}
+
+// saveAndRestoreGlobals saves mutable package-level vars and restores them after
+// the test. Call at the start of any test that touches dnsResolverFn or
+// allowPrivateProviderURLsFn.
+func saveAndRestoreGlobals(t *testing.T) {
+	t.Helper()
+	origResolver := dnsResolverFn
+	origAllow := allowPrivateProviderURLsFn
+	t.Cleanup(func() {
+		dnsResolverFn = origResolver
+		allowPrivateProviderURLsFn = origAllow
+	})
+}
+
+// --- Core merged test: all major cases in one table ---
+
+func TestValidateProviderURL(t *testing.T) {
+	saveAndRestoreGlobals(t)
+	allowPrivateProviderURLsFn = func() bool { return false }
+	absClaudePath := filepath.Join(t.TempDir(), "claude")
+
+	dnsResolverFn = stubResolver(map[string][]string{
+		"api.openai.com":         {"104.18.6.192"},
+		"legit-provider.com":     {"203.0.113.50"},
+		"10.10.27.30.nip.io":     {"10.10.27.30"},
+		"192.168.1.1.sslip.io":   {"192.168.1.1"},
+		"172.16.0.5.nip.io":      {"172.16.0.5"},
+		"169.254.169.254.nip.io": {"169.254.169.254"},
+		"127.0.0.1.nip.io":       {"127.0.0.1"},
+		"rebind.attacker.com":    {"10.0.0.1"},
+		"dual-stack.example.com": {"203.0.113.50", "10.0.0.1"},
+	})
+
+	tests := []struct {
+		name         string
+		rawURL       string
+		providerType string
+		wantErr      bool
+	}{
+		// Empty URL always allowed
+		{"empty URL", "", "openai_compat", false},
+		{"empty URL ollama", "", "ollama", false},
+
+		// Public remote URLs OK
+		{"public HTTPS", "https://api.openai.com/v1", "openai_compat", false},
+		{"public HTTP", "http://legit-provider.com/v1", "openai_compat", false},
+
+		// --- Scheme check: unconditional for ALL types including local ---
+		{"file scheme remote", "file:///etc/passwd", "openai_compat", true},
+		{"gopher scheme remote", "gopher://internal:25", "openai_compat", true},
+		{"file scheme ollama", "file:///etc/passwd", "ollama", true},       // H-1: scheme enforced even for local types
+		{"gopher scheme acp", "gopher://localhost:25", "acp", true},        // H-1: scheme enforced even for local types
+		{"file scheme claude_cli", "file:///bin/bash", "claude_cli", true}, // H-1: scheme enforced for URL-like Claude CLI values
+
+		// --- Local type: allowlist-only ---
+		{"ollama localhost", "http://localhost:11434/v1", "ollama", false},
+		{"ollama 127.0.0.1", "http://127.0.0.1:11434/v1", "ollama", false},
+		{"ollama ::1", "http://[::1]:11434/v1", "ollama", false},
+		{"ollama host.docker.internal", "http://host.docker.internal:11434/v1", "ollama", false},
+		{"acp 127.0.0.1", "http://127.0.0.1:9090", "acp", false},
+		{"claude_cli command name", "claude", "claude_cli", false},
+		{"claude_cli absolute path", absClaudePath, "claude_cli", false},
+
+		// Local type with non-localhost hosts → blocked
+		{"ollama 169.254.169.254", "http://169.254.169.254/latest/meta-data/", "ollama", true},
+		{"ollama private IP", "http://10.0.0.5:11434/v1", "ollama", true},
+		{"ollama evil.com", "http://evil.attacker.tld:9999/v1", "ollama", true},
+		{"ollama postgres sidecar", "http://postgres:5432/v1", "ollama", true},
+		{"ollama link-local", "http://169.254.1.1:8080/v1", "ollama", true},
+		{"ollama .internal", "http://redis.internal:6379/v1", "ollama", true},
+		{"ollama gcp metadata", "http://metadata.google.internal/computeMetadata/v1/", "ollama", true},
+		{"acp private", "http://10.0.0.1:8080/v1", "acp", true},
+
+		// --- Remote type literal blocked IPs ---
+		{"remote localhost", "http://localhost:8080", "openai_compat", true},
+		{"remote 127.0.0.1", "http://127.0.0.1:8080", "openai_compat", true},
+		{"remote ::1", "http://[::1]:8080", "openai_compat", true},
+		{"remote 10.x", "http://10.0.0.1:8080/v1", "openai_compat", true},
+		{"remote 192.168.x", "http://192.168.1.100:8080/v1", "openai_compat", true},
+		{"remote 172.16.x", "http://172.16.0.5:8080/v1", "openai_compat", true},
+		{"remote 169.254.169.254", "http://169.254.169.254/latest/meta-data/", "openai_compat", true},
+
+		// --- Regression: unspecified addresses (bug missed by #974 hand-rolled checks) ---
+		{"0.0.0.0 unspecified", "http://0.0.0.0:8080/v1", "openai_compat", true},
+		{":: IPv6 unspecified", "http://[::]:8080/v1", "openai_compat", true},
+
+		// --- DNS bypass via wildcard services (nip.io, sslip.io) ---
+		{"nip.io 10.x", "http://10.10.27.30.nip.io:9999/v1", "openai_compat", true},
+		{"sslip.io 192.168.x", "http://192.168.1.1.sslip.io:8080/v1", "openai_compat", true},
+		{"nip.io 172.16.x", "http://172.16.0.5.nip.io:8080/v1", "openai_compat", true},
+		{"nip.io link-local", "http://169.254.169.254.nip.io/latest/", "openai_compat", true},
+		{"nip.io loopback", "http://127.0.0.1.nip.io:8080/v1", "openai_compat", true},
+		{"attacker rebind", "http://rebind.attacker.com:8080/v1", "openai_compat", true},
+		{"dual-stack any private", "http://dual-stack.example.com:8080/v1", "openai_compat", true},
+
+		// --- Internal hostname suffix ---
+		{".internal suffix", "http://metadata.google.internal/computeMetadata/v1/", "openai_compat", true},
+		{".local suffix", "http://myservice.local:8080", "openai_compat", true},
+
+		// --- Unresolvable hostname ---
+		{"unresolvable hostname", "http://nonexistent.invalid:8080/v1", "openai_compat", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateProviderURL(tt.rawURL, tt.providerType)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("validateProviderURL(%q, %q) error = %v, wantErr %v",
+					tt.rawURL, tt.providerType, err, tt.wantErr)
+			}
+		})
+	}
+}
+
+// --- Env gate: GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS ---
+
+func TestValidateProviderURL_AllowPrivateFlag(t *testing.T) {
+	saveAndRestoreGlobals(t)
+
+	dnsResolverFn = stubResolver(map[string][]string{
+		"my-vllm.lan": {"10.0.0.1"},
+	})
+
+	t.Run("private URL allowed when flag is true", func(t *testing.T) {
+		allowPrivateProviderURLsFn = func() bool { return true }
+		if err := validateProviderURL("http://my-vllm.lan:8080/v1", "openai_compat"); err != nil {
+			t.Errorf("expected nil with allow-private flag, got: %v", err)
+		}
+		if err := validateProviderURL("http://192.168.1.50:8080/v1", "openai_compat"); err != nil {
+			t.Errorf("expected nil for private IP with allow-private flag, got: %v", err)
+		}
+		if err := validateProviderURL("http://localhost:8080/v1", "openai_compat"); err != nil {
+			t.Errorf("expected nil for localhost with allow-private flag, got: %v", err)
+		}
+		if err := validateProviderURL("http://llm.internal/v1", "openai_compat"); err != nil {
+			t.Errorf("expected nil for .internal with allow-private flag, got: %v", err)
+		}
+	})
+
+	t.Run("private URL blocked when flag is false", func(t *testing.T) {
+		allowPrivateProviderURLsFn = func() bool { return false }
+		if err := validateProviderURL("http://my-vllm.lan:8080/v1", "openai_compat"); err == nil {
+			t.Error("expected error for private URL without allow-private flag")
+		}
+	})
+
+	t.Run("scheme still enforced even with allow-private flag", func(t *testing.T) {
+		allowPrivateProviderURLsFn = func() bool { return true }
+		err := validateProviderURL("file:///etc/passwd", "openai_compat")
+		if err == nil {
+			t.Error("expected error for file:// scheme even with allow-private flag")
+		}
+		if !strings.Contains(err.Error(), "scheme") {
+			t.Errorf("expected scheme error, got: %v", err)
+		}
+	})
+}
+
+func TestValidateProviderURL_LocalTypesIgnoreAllowPrivateFlag(t *testing.T) {
+	saveAndRestoreGlobals(t)
+	allowPrivateProviderURLsFn = func() bool { return true }
+
+	cases := []struct {
+		url          string
+		providerType string
+	}{
+		{"http://ollama:11434/v1", "ollama"},
+		{"http://host.lan:11434/v1", "ollama"},
+		{"http://10.0.0.5:11434/v1", "ollama"},
+		{"http://acp-sidecar:9090", "acp"},
+	}
+	for _, c := range cases {
+		if err := validateProviderURL(c.url, c.providerType); err == nil {
+			t.Errorf("expected local provider URL %s / %s to remain blocked despite allow-private flag", c.url, c.providerType)
+		}
+	}
+}
+
+// --- H-1: scheme enforced for local provider types (from PR #972) ---
+
+func TestValidateProviderURL_LocalTypeSchemeEnforced(t *testing.T) {
+	saveAndRestoreGlobals(t)
+
+	cases := []struct {
+		url          string
+		providerType string
+	}{
+		{"file:///etc/passwd", "ollama"},
+		{"gopher://localhost:25", "ollama"},
+		{"file:///etc/passwd", "acp"},
+	}
+	for _, c := range cases {
+		err := validateProviderURL(c.url, c.providerType)
+		if err == nil {
+			t.Errorf("expected scheme error for %s / %s, got nil", c.url, c.providerType)
+			continue
+		}
+		if !strings.Contains(err.Error(), "scheme") {
+			t.Errorf("expected scheme error for %s / %s, got: %v", c.url, c.providerType, err)
+		}
+	}
+}
+
+// --- Regression: 0.0.0.0 and :: unspecified (bug PR #974 missed) ---
+
+func TestValidateProviderURL_UnspecifiedBlocked(t *testing.T) {
+	saveAndRestoreGlobals(t)
+	allowPrivateProviderURLsFn = func() bool { return false }
+
+	cases := []string{
+		"http://0.0.0.0:8080/v1",
+		"http://[::]:8080/v1",
+	}
+	for _, u := range cases {
+		err := validateProviderURL(u, "openai_compat")
+		if err == nil {
+			t.Errorf("expected error for unspecified address %q, got nil", u)
+		}
+	}
+}
+
+// --- DNS-rebinding: remote type resolving to private/loopback ---
+
+func TestValidateProviderURL_DNSRebindBlocked(t *testing.T) {
+	saveAndRestoreGlobals(t)
+	allowPrivateProviderURLsFn = func() bool { return false }
+
+	privateHosts := map[string][]string{
+		"rebind-private.example.com":   {"10.0.0.1"},
+		"rebind-loopback.example.com":  {"127.0.0.1"},
+		"rebind-linklocal.example.com": {"169.254.169.254"},
+	}
+	dnsResolverFn = stubResolver(privateHosts)
+
+	for host := range privateHosts {
+		err := validateProviderURL("http://"+host+":8080/v1", "openai_compat")
+		if err == nil {
+			t.Errorf("expected DNS rebind to be blocked for %s, got nil", host)
+		}
+	}
+}
+
+// --- Local type: all allowed variants ---
+
+func TestValidateProviderURL_LocalTypeAllowedHosts(t *testing.T) {
+	saveAndRestoreGlobals(t)
+
+	allowed := []struct {
+		url          string
+		providerType string
+	}{
+		{"http://localhost:11434/v1", "ollama"},
+		{"http://127.0.0.1:11434/v1", "ollama"},
+		{"http://[::1]:11434/v1", "ollama"},
+		{"http://host.docker.internal:11434/v1", "ollama"},
+		{"http://localhost:9090", "acp"},
+		{"http://127.0.0.1:9090", "acp"},
+	}
+	for _, a := range allowed {
+		if err := validateProviderURL(a.url, a.providerType); err != nil {
+			t.Errorf("expected %s / %s to be allowed, got: %v", a.url, a.providerType, err)
+		}
+	}
+}
+
+func TestValidateProviderURL_ClaudeCLIExecutablePath(t *testing.T) {
+	saveAndRestoreGlobals(t)
+	absClaudePath := filepath.Join(t.TempDir(), "claude")
+
+	allowed := []string{
+		"",
+		"claude",
+		absClaudePath,
+		filepath.Join(t.TempDir(), "Claude Code.app", "Contents", "MacOS", "claude"),
+	}
+	for _, raw := range allowed {
+		if err := validateProviderURL(raw, "claude_cli"); err != nil {
+			t.Errorf("expected Claude CLI executable %q to be allowed, got: %v", raw, err)
+		}
+	}
+
+	blocked := []string{
+		"file:///usr/local/bin/claude",
+		"https://api.anthropic.com/v1",
+		"relative/claude",
+		"claude --dangerously-skip-permissions",
+	}
+	for _, raw := range blocked {
+		if err := validateProviderURL(raw, "claude_cli"); err == nil {
+			t.Errorf("expected Claude CLI executable %q to be rejected", raw)
+		}
+	}
+}
+
+// --- Public remote URL always OK ---
+
+func TestValidateProviderURL_PublicHostOK(t *testing.T) {
+	saveAndRestoreGlobals(t)
+	allowPrivateProviderURLsFn = func() bool { return false }
+	dnsResolverFn = stubResolver(map[string][]string{
+		"api.openai.com": {"104.18.6.192"},
+	})
+
+	if err := validateProviderURL("https://api.openai.com/v1", "openai_compat"); err != nil {
+		t.Errorf("expected public host to pass, got: %v", err)
+	}
+}

internal/pipeline/deps.go

@@ -89,6 +89,9 @@ type PipelineDeps struct {
 	ExecuteToolRaw func(ctx context.Context, tc providers.ToolCall) (providers.Message, any, error)
 	// ProcessToolResult processes a raw tool result with state mutation (sequential only).
 	ProcessToolResult func(ctx context.Context, state *RunState, tc providers.ToolCall, rawMsg providers.Message, rawData any) []providers.Message
+	// AuthorizeToolCall validates whether a tool call is allowed to execute.
+	// Used by ToolStage as a runtime guard against out-of-policy tool calls.
+	AuthorizeToolCall func(ctx context.Context, state *RunState, tc providers.ToolCall) (bool, string)
 	// SequentialToolCall returns true for tools that must preserve same-response order.
 	// When any tool call in a batch matches, ToolStage uses ExecuteToolCall for the
 	// whole batch instead of parallel raw execution.

internal/pipeline/substates.go

@@ -52,6 +52,9 @@ type PruneState struct {
 
 // ToolState: owned by ToolStage.
 type ToolState struct {
+	// AllowedTools is the per-iteration execution allowlist built from tool
+	// definitions sent to the provider. Nil means "no runtime restriction".
+	AllowedTools   map[string]bool
 	LoopDetector   any // concrete type toolLoopState lives in agent; Phase 5 defines LoopDetector interface
 	TotalToolCalls int
 	AsyncToolCalls []string      // tool names that executed async (spawn)

internal/pipeline/think_stage.go

@@ -41,6 +41,13 @@ func (s *ThinkStage) Execute(ctx context.Context, state *RunState) error {
 		if err != nil {
 			return fmt.Errorf("build tools: %w", err)
 		}
+		allowed := make(map[string]bool, len(toolDefs))
+		for _, td := range toolDefs {
+			allowed[td.Function.Name] = true
+		}
+		state.Tool.AllowedTools = allowed
+	} else {
+		state.Tool.AllowedTools = nil
 	}
 
 	// 3. Construct ChatRequest

internal/pipeline/tool_stage.go

@@ -64,6 +64,22 @@ func (s *ToolStage) Execute(ctx context.Context, state *RunState) error {
 			}
 		}
 
+		// Fail-closed authorization gate: reject calls not present in the
+		// per-iteration allowlist built by ThinkStage. Operates on the canonical
+		// (prefix-stripped) name so toolCallPrefix agents are handled correctly.
+		if s.deps.AuthorizeToolCall != nil {
+			if ok, reason := s.deps.AuthorizeToolCall(ctx, state, tc); !ok {
+				state.Messages.AppendPending(providers.Message{
+					Role:       "tool",
+					Content:    reason,
+					ToolCallID: tc.ID,
+					IsError:    true,
+				})
+				state.Tool.TotalToolCalls++
+				continue
+			}
+		}
+
 		// Hook: sync PreToolUse — block if hook denies. Builtin-source hooks may
 		// rewrite tc.Arguments via UpdatedToolInput (e.g. path-sanitizer); apply
 		// before ExecuteToolCall so the rewrite is authoritative.
@@ -179,10 +195,35 @@ func (s *ToolStage) executeParallel(ctx context.Context, state *RunState, toolCa
 		err     error
 	}
 
+	// Filter out unauthorized calls before dispatching parallel I/O.
+	// Keeps error messages deterministic (same order as toolCalls slice).
+	filteredCalls := make([]providers.ToolCall, 0, len(toolCalls))
+	for _, tc := range toolCalls {
+		if s.deps.AuthorizeToolCall != nil {
+			if ok, reason := s.deps.AuthorizeToolCall(ctx, state, tc); !ok {
+				state.Messages.AppendPending(providers.Message{
+					Role:       "tool",
+					Content:    reason,
+					ToolCallID: tc.ID,
+					IsError:    true,
+				})
+				state.Tool.TotalToolCalls++
+				continue
+			}
+		}
+		filteredCalls = append(filteredCalls, tc)
+	}
+	// When all calls were blocked, still run exit-condition checks (read-only
+	// streak, MaxToolCalls budget) for consistency with the sequential path.
+	if len(filteredCalls) == 0 {
+		s.checkExitConditions(state)
+		return nil
+	}
+
 	// Phase 1: parallel I/O (no state mutation)
-	results := make([]rawResult, len(toolCalls))
+	results := make([]rawResult, len(filteredCalls))
 	var wg sync.WaitGroup
-	for i, tc := range toolCalls {
+	for i, tc := range filteredCalls {
 		wg.Add(1)
 		go func(idx int, tc providers.ToolCall) {
 			defer wg.Done()

internal/pipeline/tool_stage_authorize_test.go

@@ -0,0 +1,216 @@
+package pipeline
+
+import (
+	"context"
+	"testing"
+
+	"github.com/nextlevelbuilder/goclaw/internal/providers"
+)
+
+// makeAuthorizeGate builds a PipelineDeps.AuthorizeToolCall callback backed by
+// a static allowlist and an optional deferred-activator, mirroring the contract
+// that makeAuthorizeToolCall in loop_pipeline_callbacks.go provides.
+func makeAuthorizeGate(
+	allowed map[string]bool, // nil = allow-all
+	tryActivate func(name string) bool, // nil = no activator
+	isDenied func(name string) bool, // nil = never denied
+) func(ctx context.Context, state *RunState, tc providers.ToolCall) (bool, string) {
+	return func(_ context.Context, state *RunState, tc providers.ToolCall) (bool, string) {
+		a := state.Tool.AllowedTools
+		if a == nil {
+			return true, ""
+		}
+		if a[tc.Name] {
+			return true, ""
+		}
+		if tryActivate != nil && tryActivate(tc.Name) {
+			if isDenied != nil && isDenied(tc.Name) {
+				return false, "tool not allowed by policy: " + tc.Name
+			}
+			a[tc.Name] = true
+			return true, ""
+		}
+		return false, "tool not allowed by policy: " + tc.Name
+	}
+}
+
+// makeMinimalDeps returns a PipelineDeps just sufficient for ToolStage tests.
+func makeMinimalDeps(authorize func(context.Context, *RunState, providers.ToolCall) (bool, string)) *PipelineDeps {
+	return &PipelineDeps{
+		Config:            PipelineConfig{MaxToolCalls: 100},
+		AuthorizeToolCall: authorize,
+		ExecuteToolCall: func(_ context.Context, state *RunState, tc providers.ToolCall) ([]providers.Message, error) {
+			return []providers.Message{{Role: "tool", Content: "ok", ToolCallID: tc.ID}}, nil
+		},
+	}
+}
+
+// stateWithAllowedTools returns a minimal RunState with AllowedTools pre-set.
+func stateWithAllowedTools(allowed map[string]bool) *RunState {
+	st := defaultState()
+	st.Tool.AllowedTools = allowed
+	return st
+}
+
+func toolCallFor(name string) providers.ToolCall {
+	return providers.ToolCall{ID: "tc-" + name, Name: name}
+}
+
+// fakeResponse puts tool calls into the state's ThinkStage so ToolStage finds them.
+func setLastResponse(state *RunState, calls []providers.ToolCall) {
+	state.Think.LastResponse = &providers.ChatResponse{ToolCalls: calls}
+}
+
+// --- Tests ---
+
+// TestAuthorizeGate_NilAllowlist_AllowsAll verifies that a nil AllowedTools map
+// means no per-iteration restriction (allow-all semantics).
+func TestAuthorizeGate_NilAllowlist_AllowsAll(t *testing.T) {
+	t.Parallel()
+	st := defaultState()
+	st.Tool.AllowedTools = nil // explicit nil
+
+	gate := makeAuthorizeGate(nil, nil, nil)
+	tc := toolCallFor("exec")
+
+	ok, reason := gate(context.Background(), st, tc)
+	if !ok {
+		t.Errorf("nil allowlist must allow all tools; got blocked with reason %q", reason)
+	}
+}
+
+// TestAuthorizeGate_NamePresent_Allows checks that a tool explicitly in the
+// allowlist passes through.
+func TestAuthorizeGate_NamePresent_Allows(t *testing.T) {
+	t.Parallel()
+	st := stateWithAllowedTools(map[string]bool{"exec": true, "read_file": true})
+
+	gate := makeAuthorizeGate(nil, nil, nil)
+	for _, name := range []string{"exec", "read_file"} {
+		ok, reason := gate(context.Background(), st, toolCallFor(name))
+		if !ok {
+			t.Errorf("tool %q should be allowed; got blocked: %q", name, reason)
+		}
+	}
+}
+
+// TestAuthorizeGate_NameAbsent_NoActivator_Denies confirms fail-closed: a name
+// not in the allowlist with no activator is blocked.
+func TestAuthorizeGate_NameAbsent_NoActivator_Denies(t *testing.T) {
+	t.Parallel()
+	st := stateWithAllowedTools(map[string]bool{"read_file": true})
+
+	gate := makeAuthorizeGate(nil, nil, nil)
+	ok, reason := gate(context.Background(), st, toolCallFor("exec"))
+	if ok {
+		t.Error("tool absent from allowlist with no activator must be denied")
+	}
+	if reason == "" {
+		t.Error("denial must include a non-empty reason")
+	}
+}
+
+// TestAuthorizeGate_DeferredActivate_ThenDenied ensures that lazy activation
+// followed by a positive IsDenied check still blocks the tool.
+func TestAuthorizeGate_DeferredActivate_ThenDenied(t *testing.T) {
+	t.Parallel()
+	st := stateWithAllowedTools(map[string]bool{})
+
+	activated := false
+	tryActivate := func(name string) bool {
+		activated = true
+		return name == "mcp_svc__exec_cmd"
+	}
+	isDenied := func(name string) bool {
+		return name == "mcp_svc__exec_cmd" // explicitly denied
+	}
+
+	gate := makeAuthorizeGate(nil, tryActivate, isDenied)
+	ok, reason := gate(context.Background(), st, toolCallFor("mcp_svc__exec_cmd"))
+
+	if !activated {
+		t.Error("expected TryActivateDeferred to be called")
+	}
+	if ok {
+		t.Error("tool allowed by activator but denied by policy must still be blocked")
+	}
+	if reason == "" {
+		t.Error("denial must include a non-empty reason")
+	}
+	if st.Tool.AllowedTools["mcp_svc__exec_cmd"] {
+		t.Error("denied tool must not be added to AllowedTools")
+	}
+}
+
+// TestAuthorizeGate_PrefixedName_CanonicalLookup is the regression guard for the
+// toolCallPrefix bug: when an agent has toolCallPrefix set, the model emits
+// "proxy_exec" but AllowedTools is keyed by canonical name "exec". The gate must
+// resolve the name before lookup, otherwise every prefixed call is wrongly blocked.
+//
+// This test simulates the resolution by calling the gate with a *pre-resolved*
+// canonical name, matching what makeAuthorizeToolCall does after calling
+// resolveToolCallName(tc.Name). The pipeline layer always receives the resolved
+// name because AuthorizeToolCall in the real callback operates on the canonical
+// form.
+func TestAuthorizeGate_PrefixedName_CanonicalLookup(t *testing.T) {
+	t.Parallel()
+	// AllowedTools uses canonical name "exec" (as ThinkStage builds it from FilterTools).
+	st := stateWithAllowedTools(map[string]bool{"exec": true})
+
+	gate := makeAuthorizeGate(nil, nil, nil)
+
+	// Simulate what resolveToolCallName("proxy_exec") returns with prefix "proxy_":
+	// the canonical name "exec" is passed to the gate.
+	resolvedName := "exec" // prefix already stripped by makeAuthorizeToolCall
+	tc := providers.ToolCall{ID: "tc-prefix", Name: resolvedName}
+
+	ok, reason := gate(context.Background(), st, tc)
+	if !ok {
+		t.Errorf("canonical name after prefix resolution must be allowed; reason: %q", reason)
+	}
+}
+
+// TestToolStage_AllBlocked_CallsCheckExitConditions verifies that when all tool
+// calls in the parallel path are blocked, ToolStage still runs checkExitConditions
+// (so MaxToolCalls budget is enforced). Regression for early-return-nil bug.
+func TestToolStage_AllBlocked_CallsCheckExitConditions(t *testing.T) {
+	t.Parallel()
+
+	// Allow-all deps but with MaxToolCalls = 1 and AuthorizeToolCall that blocks.
+	denyAll := func(_ context.Context, _ *RunState, tc providers.ToolCall) (bool, string) {
+		return false, "blocked: " + tc.Name
+	}
+	deps := &PipelineDeps{
+		Config: PipelineConfig{MaxToolCalls: 1},
+		AuthorizeToolCall: denyAll,
+		ExecuteToolCall: func(_ context.Context, state *RunState, tc providers.ToolCall) ([]providers.Message, error) {
+			return nil, nil
+		},
+		ExecuteToolRaw: func(_ context.Context, tc providers.ToolCall) (providers.Message, any, error) {
+			return providers.Message{}, nil, nil
+		},
+		ProcessToolResult: func(_ context.Context, state *RunState, tc providers.ToolCall, rawMsg providers.Message, rawData any) []providers.Message {
+			return nil
+		},
+	}
+
+	stage := NewToolStage(deps)
+	state := defaultState()
+	state.Tool.AllowedTools = map[string]bool{} // non-nil allowlist → gate is active
+	state.Tool.TotalToolCalls = 1               // already at MaxToolCalls
+
+	// Two tool calls — parallel path is triggered (len > 1).
+	setLastResponse(state, []providers.ToolCall{
+		toolCallFor("exec"),
+		toolCallFor("write_file"),
+	})
+
+	if err := stage.Execute(context.Background(), state); err != nil {
+		t.Fatalf("Execute returned error: %v", err)
+	}
+
+	// checkExitConditions must have fired: TotalToolCalls >= MaxToolCalls → BreakLoop.
+	if stage.Result() != BreakLoop {
+		t.Errorf("expected BreakLoop after all-blocked + MaxToolCalls reached, got %v", stage.Result())
+	}
+}

internal/providers/defaults.go

@@ -38,6 +38,11 @@ func NewDefaultTransport() *http.Transport {
 
 // NewDefaultHTTPClient returns an *http.Client backed by NewDefaultTransport.
 // No Client.Timeout is set — rely on ctx deadlines and Transport stage timeouts.
+//
+// SSRF protection for user-configured provider URLs is enforced at provider
+// create/update time by validateProviderURL (resolves the host and rejects
+// private/reserved IPs via security.IsBlocked). Dial-time DNS-rebinding
+// hardening is tracked as a follow-up.
 func NewDefaultHTTPClient() *http.Client {
 	return &http.Client{Transport: NewDefaultTransport()}
 }

internal/providers/embedding_openai.go

@@ -40,8 +40,11 @@ func NewOpenAIEmbeddingProvider(apiKey, apiBase, model string) *OpenAIEmbeddingP
 		apiKey:       apiKey,
 		apiBase:      strings.TrimRight(apiBase, "/"),
 		model:        model,
-		client:       &http.Client{Timeout: 60 * time.Second},
-		retry:        DefaultRetryConfig(),
+		// Use a fixed 60s timeout client. Embedding requests are short but can be
+		// batched. The apiBase is validated at provider-creation time by
+		// validateProviderURL in internal/http/providers.go.
+		client: &http.Client{Timeout: 60 * time.Second},
+		retry:  DefaultRetryConfig(),
 	}
 }
 

internal/providers/vertex.go

@@ -131,11 +131,15 @@ func NewVertexProvider(ctx context.Context, cfg VertexConfig) (*OpenAIProvider,
 	// then transparently fetches a fresh one. No extra work for callers.
 	cached := oauth2.ReuseTokenSource(nil, tokenSource)
 
+	// Use NewDefaultTransport as the oauth2 base transport so Vertex requests
+	// inherit the same per-stage timeouts as other providers. SSRF defense is
+	// provided by validateVertexAPIBaseOverride, which enforces the googleapis.com
+	// host constraint at provider-creation time.
 	client := &http.Client{
 		Timeout: DefaultHTTPTimeout,
 		Transport: &oauth2.Transport{
 			Source: cached,
-			Base:   http.DefaultTransport,
+			Base:   NewDefaultTransport(),
 		},
 	}
 

internal/sandbox/docker_test.go

@@ -172,31 +172,3 @@ func TestFsBridgePathWithinUsesPathBoundaries(t *testing.T) {
 		})
 	}
 }
-
-func TestFsBridgeWriteFileCommandPreservesOverwriteTruncation(t *testing.T) {
-	args := fsBridgeWriteDDArgs("/workspace/file.txt", false)
-	for _, arg := range args {
-		if arg == "conv=notrunc" || arg == "oflag=append" {
-			t.Fatalf("overwrite command must truncate, got append-only arg %q in %v", arg, args)
-		}
-	}
-}
-
-func TestFsBridgeWriteFileCommandUsesNoTruncOnlyForAppend(t *testing.T) {
-	args := fsBridgeWriteDDArgs("/workspace/file.txt", true)
-	if !containsString(args, "conv=notrunc") {
-		t.Fatalf("append command missing conv=notrunc: %v", args)
-	}
-	if !containsString(args, "oflag=append") {
-		t.Fatalf("append command missing oflag=append: %v", args)
-	}
-}
-
-func containsString(values []string, target string) bool {
-	for _, value := range values {
-		if value == target {
-			return true
-		}
-	}
-	return false
-}

internal/sandbox/fsbridge.go

@@ -80,8 +80,12 @@ func (b *FsBridge) WriteFile(ctx context.Context, path, content string, appendMo
 		}
 	}
 
-	ddArgs := fsBridgeWriteDDArgs(resolved, appendMode)
-	_, stderr, exitCode, err := b.dockerExec(ctx, []byte(content), ddArgs...)
+	// Write content via stdin without invoking a shell. Passing the resolved path
+	// as a discrete argv entry prevents shell metacharacters in filenames from
+	// being interpreted as commands inside the sandbox container.
+	// "--" terminates option parsing so paths starting with "-" are safe.
+	teeArgs := fsBridgeWriteTeeArgs(resolved, appendMode)
+	_, stderr, exitCode, err := b.dockerExec(ctx, []byte(content), teeArgs...)
 	if err != nil {
 		return fmt.Errorf("fsbridge write: %w", err)
 	}
@@ -92,11 +96,12 @@ func (b *FsBridge) WriteFile(ctx context.Context, path, content string, appendMo
 	return nil
 }
 
-func fsBridgeWriteDDArgs(resolved string, appendMode bool) []string {
-	args := []string{"dd", "bs=1048576", "status=none", "of=" + resolved}
+func fsBridgeWriteTeeArgs(resolved string, appendMode bool) []string {
+	args := []string{"tee"}
 	if appendMode {
-		args = append(args, "conv=notrunc", "oflag=append")
+		args = append(args, "-a")
 	}
+	args = append(args, "--", resolved)
 	return args
 }
 

internal/sandbox/fsbridge_test.go

@@ -0,0 +1,189 @@
+package sandbox
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// fakeDockerScript is a fake `docker` binary used in WriteFile tests.
+// It logs all argv to $DOCKER_LOG and, when invoked with a `tee` sub-command,
+// captures stdin to $DOCKER_STDIN.
+// `realpath -e -- <path>` echoes the path back (simulates existing path).
+// `mkdir` exits 0 silently.
+const fakeDockerScript = `#!/bin/sh
+{
+  echo CALL
+  i=0
+  for arg in "$@"; do
+    echo "ARG[$i]=$arg"
+    i=$((i + 1))
+  done
+} >> "$DOCKER_LOG"
+
+# realpath: echo the last arg back as the resolved path
+for j in $(seq 0 $#); do
+  eval "a=\${$j}"
+  if [ "$a" = "realpath" ]; then
+    # print the last argument (the path)
+    eval "echo \"\${$#}\""
+    exit 0
+  fi
+done
+
+# tee: capture stdin
+for arg in "$@"; do
+  if [ "$arg" = "tee" ]; then
+    if [ -n "$DOCKER_STDIN" ]; then
+      cat > "$DOCKER_STDIN"
+    else
+      cat > /dev/null
+    fi
+    exit 0
+  fi
+done
+
+exit 0
+`
+
+// installFakeDocker writes the fake docker script to tmp, prepends tmp to PATH,
+// and sets DOCKER_LOG and optionally DOCKER_STDIN env vars for the test.
+func installFakeDocker(t *testing.T, tmp, logPath, stdinPath string) {
+	t.Helper()
+	dockerPath := filepath.Join(tmp, "docker")
+	if err := os.WriteFile(dockerPath, []byte(fakeDockerScript), 0o755); err != nil {
+		t.Fatalf("write fake docker: %v", err)
+	}
+	t.Setenv("PATH", tmp+string(os.PathListSeparator)+os.Getenv("PATH"))
+	t.Setenv("DOCKER_LOG", logPath)
+	if stdinPath != "" {
+		t.Setenv("DOCKER_STDIN", stdinPath)
+	}
+}
+
+// TestFsBridgeWriteFileDoesNotInvokeShell asserts that WriteFile passes the
+// resolved path as a discrete argv entry to `tee -- <path>` and does NOT
+// build a shell command string (no sh / -c / shell metachar expansion).
+func TestFsBridgeWriteFileDoesNotInvokeShell(t *testing.T) {
+	tmp := t.TempDir()
+	logPath := filepath.Join(tmp, "docker.log")
+	stdinPath := filepath.Join(tmp, "stdin.txt")
+
+	installFakeDocker(t, tmp, logPath, stdinPath)
+
+	bridge := NewFsBridge("container-id", "/workspace")
+	maliciousPath := `nested/evil$(touch /tmp/goclaw-fsbridge-pwned);name.txt`
+	content := "safe content"
+
+	if err := bridge.WriteFile(context.Background(), maliciousPath, content, false); err != nil {
+		t.Fatalf("WriteFile returned error: %v", err)
+	}
+
+	logBytes, err := os.ReadFile(logPath)
+	if err != nil {
+		t.Fatalf("read docker log: %v", err)
+	}
+	log := string(logBytes)
+
+	// Must NOT have invoked sh or built a shell string.
+	if strings.Contains(log, "=sh") || strings.Contains(log, "=-c") || strings.Contains(log, "cat >") {
+		t.Fatalf("WriteFile invoked shell command path; log:\n%s", log)
+	}
+	// Must have called tee.
+	if !strings.Contains(log, "=tee") {
+		t.Fatalf("expected write command to use tee without shell; log:\n%s", log)
+	}
+	// Must have included -- to terminate option parsing.
+	if !strings.Contains(log, "=--") {
+		t.Fatalf("expected tee delimiter (--) before filename; log:\n%s", log)
+	}
+	// The malicious path must appear verbatim as a single argv entry (not split/executed).
+	resolved := "/workspace/" + maliciousPath
+	if !strings.Contains(log, "="+resolved) {
+		t.Fatalf("expected malicious filename to remain one argv entry; log:\n%s", log)
+	}
+
+	stdinBytes, err := os.ReadFile(stdinPath)
+	if err != nil {
+		t.Fatalf("read captured stdin: %v", err)
+	}
+	if string(stdinBytes) != content {
+		t.Fatalf("stdin content = %q, want %q", string(stdinBytes), content)
+	}
+}
+
+// TestFsBridgeWriteFileAppendUsesTeeAppendArg asserts that append mode passes
+// -a before -- in the tee argv, i.e. `tee -a -- <path>`.
+func TestFsBridgeWriteFileAppendUsesTeeAppendArg(t *testing.T) {
+	tmp := t.TempDir()
+	logPath := filepath.Join(tmp, "docker.log")
+
+	installFakeDocker(t, tmp, logPath, "")
+
+	bridge := NewFsBridge("container-id", "/workspace")
+	if err := bridge.WriteFile(context.Background(), "append.txt", "more", true); err != nil {
+		t.Fatalf("WriteFile append returned error: %v", err)
+	}
+
+	logBytes, err := os.ReadFile(logPath)
+	if err != nil {
+		t.Fatalf("read docker log: %v", err)
+	}
+	log := string(logBytes)
+
+	// Must have: tee, then -a, then --.
+	if !strings.Contains(log, "=tee") {
+		t.Fatalf("expected tee in argv; log:\n%s", log)
+	}
+	if !strings.Contains(log, "=-a") {
+		t.Fatalf("expected -a flag for append mode; log:\n%s", log)
+	}
+	if !strings.Contains(log, "=--") {
+		t.Fatalf("expected -- delimiter in argv; log:\n%s", log)
+	}
+}
+
+// TestFsBridgeWriteTeeArgs_Overwrite checks the tee args slice for overwrite mode.
+func TestFsBridgeWriteTeeArgs_Overwrite(t *testing.T) {
+	args := fsBridgeWriteTeeArgs("/workspace/file.txt", false)
+	if len(args) != 3 {
+		t.Fatalf("expected 3 args, got %d: %v", len(args), args)
+	}
+	if args[0] != "tee" {
+		t.Errorf("args[0] = %q, want tee", args[0])
+	}
+	if args[1] != "--" {
+		t.Errorf("args[1] = %q, want --", args[1])
+	}
+	if args[2] != "/workspace/file.txt" {
+		t.Errorf("args[2] = %q, want /workspace/file.txt", args[2])
+	}
+	// Must not contain -a in overwrite mode.
+	for _, a := range args {
+		if a == "-a" {
+			t.Errorf("overwrite mode must not contain -a: %v", args)
+		}
+	}
+}
+
+// TestFsBridgeWriteTeeArgs_Append checks the tee args slice for append mode.
+func TestFsBridgeWriteTeeArgs_Append(t *testing.T) {
+	args := fsBridgeWriteTeeArgs("/workspace/file.txt", true)
+	if len(args) != 4 {
+		t.Fatalf("expected 4 args, got %d: %v", len(args), args)
+	}
+	if args[0] != "tee" {
+		t.Errorf("args[0] = %q, want tee", args[0])
+	}
+	if args[1] != "-a" {
+		t.Errorf("args[1] = %q, want -a", args[1])
+	}
+	if args[2] != "--" {
+		t.Errorf("args[2] = %q, want --", args[2])
+	}
+	if args[3] != "/workspace/file.txt" {
+		t.Errorf("args[3] = %q, want /workspace/file.txt", args[3])
+	}
+}

internal/security/ssrf.go

@@ -77,6 +77,16 @@ func isBlocked(ip net.IP) bool {
 	return false
 }
 
+// IsBlocked reports whether ip falls within any blocked CIDR (loopback,
+// link-local including cloud-metadata 169.254.169.254, RFC 1918 private,
+// multicast, and unspecified 0.0.0.0/:: ranges).
+//
+// Use this in provider-URL validation and dial-time guards to avoid
+// duplicating the CIDR list across packages.
+func IsBlocked(ip net.IP) bool {
+	return isBlocked(ip)
+}
+
 // redactURL strips query string and userinfo for safe logging.
 func redactURL(rawURL string) string {
 	u, err := url.Parse(rawURL)

internal/tools/policy.go

@@ -53,7 +53,7 @@ var toolProfiles = map[string][]string{
 }
 
 // Legacy tool aliases — migrated to Registry.RegisterAlias() at startup.
-// Kept as seed data only; resolveAlias() is no longer used.
+// resolveAlias() is used by IsDenied to expand names before deny-spec matching.
 var legacyToolAliases = map[string]string{
 	"bash":           "exec",
 	"apply-patch":    "apply_patch",
@@ -434,19 +434,34 @@ func unionWithSpec(reg *Registry, current []string, allTools []string, spec []st
 
 // IsDenied checks if a tool name is explicitly denied by global or agent policy.
 // Used to prevent lazy-activated deferred tools from bypassing the deny list.
+// Checks under all candidate names: the raw name, the legacy alias (e.g. bash→exec),
+// and the registry alias when available.
 func (pe *PolicyEngine) IsDenied(name string, agentPolicy *config.ToolPolicySpec) bool {
+	candidates := map[string]struct{}{name: {}}
+	// Keep legacy alias compatibility (e.g. bash -> exec).
+	candidates[resolveAlias(name)] = struct{}{}
+	// Include registry alias mapping when available.
 	pe.mu.RLock()
 	reg := pe.registry
 	pe.mu.RUnlock()
+	if reg != nil {
+		if canonical, ok := reg.Aliases()[name]; ok && canonical != "" {
+			candidates[canonical] = struct{}{}
+		}
+	}
 
 	if pe.globalPolicy != nil {
-		if matchDenySpec(reg, name, pe.globalPolicy.Deny) {
-			return true
+		for candidate := range candidates {
+			if matchDenySpec(reg, candidate, pe.globalPolicy.Deny) {
+				return true
+			}
 		}
 	}
 	if agentPolicy != nil {
-		if matchDenySpec(reg, name, agentPolicy.Deny) {
-			return true
+		for candidate := range candidates {
+			if matchDenySpec(reg, candidate, agentPolicy.Deny) {
+				return true
+			}
 		}
 	}
 	return false