fix(http): restore ACP executable-path validation, bypass SSRF URL check (#1481)

ACP providers use api_base to store an executable command/path, not a URL.
A previous SSRF-hardening refactor incorrectly classified ACP as a local-URL
provider type, causing create/update to reject valid ACP configs with
"provider URL must use http or https scheme".

Changes:
- Remove ProviderACP from localURLProviderTypes.
- Add validateACPExecutablePath mirroring the runtime registration logic in
cmd/gateway_providers.go (allows empty, claude/codex/gemini, or absolute path;
rejects URLs and relative paths).
- Route ACP through the new validator inside validateProviderURL.
- Add unit tests for validateACPExecutablePath.
- Update existing local-type tests that assumed ACP was URL-based.
- Add create/update HTTP handler regression tests for ACP binary, empty binary,
and URL rejection.

Bug-first verification: without the fix, ACP api_base values like "gemini" or
absolute paths fail URL validation, while URLs are incorrectly accepted; with
the fix the behavior is reversed to match the intended executable-path model.

Fixes nextlevelbuilder/goclaw#1481.

internal/http/providers.go

@@ -439,9 +439,10 @@ func normalizeOllamaAPIBase(p *store.LLMProviderData) {
 // localURLProviderTypes are provider types that legitimately run on localhost.
 // They are restricted to an explicit localhost allowlist
 // rather than skipping SSRF validation entirely.
+// ACP is intentionally excluded: its api_base carries an executable command/path,
+// not a URL (see issue #1481).
 var localURLProviderTypes = map[string]bool{
 	store.ProviderOllama: true,
-	store.ProviderACP:    true,
 }
 
 // allowedLocalHosts are the only hosts permitted for local provider types.
@@ -508,6 +509,9 @@ func validateProviderURL(rawURL string, providerType string) error {
 	if providerType == store.ProviderClaudeCLI {
 		return validateClaudeCLIExecutablePath(rawURL)
 	}
+	if providerType == store.ProviderACP {
+		return validateACPExecutablePath(rawURL)
+	}
 	u, err := url.Parse(rawURL)
 	if err != nil {
 		return fmt.Errorf("invalid URL: %w", err)
@@ -595,6 +599,23 @@ func validateClaudeCLIExecutablePath(path string) error {
 	return fmt.Errorf("Claude CLI api_base must be %q or an absolute executable path, got %q", "claude", path)
 }
 
+// validateACPExecutablePath validates that api_base for ACP providers carries a
+// command or absolute executable path, not a URL. This mirrors the runtime
+// registration logic in cmd/gateway_providers.go.
+func validateACPExecutablePath(path string) error {
+	if strings.Contains(path, "\x00") {
+		return fmt.Errorf("ACP binary path cannot contain NUL byte")
+	}
+	if _, err := url.ParseRequestURI(path); err == nil && strings.Contains(path, "://") {
+		return fmt.Errorf("ACP api_base must be an executable path or command, got URL %q", path)
+	}
+	// Keep parity with registerACPFromDB: built-in command names or absolute paths.
+	if path == "claude" || path == "codex" || path == "gemini" || filepath.IsAbs(path) {
+		return nil
+	}
+	return fmt.Errorf("ACP api_base must be %q, %q, %q, or an absolute executable path, got %q", "claude", "codex", "gemini", path)
+}
+
 // --- Provider CRUD ---
 
 func (h *ProvidersHandler) handleListProviders(w http.ResponseWriter, r *http.Request) {

internal/http/providers_test.go

@@ -554,6 +554,179 @@ func TestProvidersHandlerUpdateAllowsClaudeCLIExecutablePath(t *testing.T) {
 	}
 }
 
+// TestProvidersHandlerCreateAllowsACPExecutablePath guards the fix for issue #1481:
+// ACP provider api_base is an executable command/path, not a URL, and must not be
+// rejected by the SSRF URL validator.
+func TestProvidersHandlerCreateAllowsACPExecutablePath(t *testing.T) {
+	token := setupProvidersAdminToken(t)
+	providerStore := newMockProviderStore()
+	handler := NewProvidersHandler(providerStore, newMockSecretsStore(), nil, "")
+	mux := http.NewServeMux()
+	handler.RegisterRoutes(mux)
+
+	body := map[string]any{
+		"name":          "acp-gemini",
+		"provider_type": store.ProviderACP,
+		"api_base":      writeFakeClaudeBinary(t), // any absolute path is acceptable
+		"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["acp-gemini"].APIBase; got == "" || !filepath.IsAbs(got) {
+		t.Fatalf("stored ACP api_base = %q, want absolute executable path", got)
+	}
+}
+
+// TestProvidersHandlerCreateAllowsEmptyACPBinary confirms that an empty api_base
+// is accepted at create time; the runtime path may fall back to config/env.
+func TestProvidersHandlerCreateAllowsEmptyACPBinary(t *testing.T) {
+	token := setupProvidersAdminToken(t)
+	providerStore := newMockProviderStore()
+	handler := NewProvidersHandler(providerStore, newMockSecretsStore(), nil, "")
+	mux := http.NewServeMux()
+	handler.RegisterRoutes(mux)
+
+	body := map[string]any{
+		"name":          "acp-empty",
+		"provider_type": store.ProviderACP,
+		"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())
+	}
+}
+
+// TestProvidersHandlerCreateRejectsACPURL ensures ACP api_base cannot be a URL
+// (it must be an executable path/command).
+func TestProvidersHandlerCreateRejectsACPURL(t *testing.T) {
+	token := setupProvidersAdminToken(t)
+	providerStore := newMockProviderStore()
+	handler := NewProvidersHandler(providerStore, newMockSecretsStore(), nil, "")
+	mux := http.NewServeMux()
+	handler.RegisterRoutes(mux)
+
+	body := map[string]any{
+		"name":          "acp-url",
+		"provider_type": store.ProviderACP,
+		"api_base":      "http://127.0.0.1:9090",
+		"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.StatusBadRequest {
+		t.Fatalf("status code = %d, want %d, body=%s", w.Code, http.StatusBadRequest, w.Body.String())
+	}
+}
+
+// TestProvidersHandlerUpdateAllowsACPExecutablePath guards update-time validation
+// for ACP providers.
+func TestProvidersHandlerUpdateAllowsACPExecutablePath(t *testing.T) {
+	token := setupProvidersAdminToken(t)
+	providerStore := newMockProviderStore()
+	provider := &store.LLMProviderData{
+		BaseModel:    store.BaseModel{ID: uuid.New()},
+		Name:         "acp-local",
+		ProviderType: store.ProviderACP,
+		APIBase:      "gemini",
+		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)
+	}
+}
+
+// TestProvidersHandlerUpdateRejectsACPURL ensures update-time validation rejects
+// a URL-valued api_base for ACP providers.
+func TestProvidersHandlerUpdateRejectsACPURL(t *testing.T) {
+	token := setupProvidersAdminToken(t)
+	providerStore := newMockProviderStore()
+	provider := &store.LLMProviderData{
+		BaseModel:    store.BaseModel{ID: uuid.New()},
+		Name:         "acp-local",
+		ProviderType: store.ProviderACP,
+		APIBase:      "gemini",
+		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)
+
+	body := map[string]any{"api_base": "http://127.0.0.1:9090"}
+	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.StatusBadRequest {
+		t.Fatalf("status code = %d, want %d, body=%s", w.Code, http.StatusBadRequest, w.Body.String())
+	}
+}
+
 func TestProvidersHandlerUpdateMarshalsSettingsForStore(t *testing.T) {
 	token := setupProvidersAdminToken(t)
 	providerStore := newMockProviderStore()

internal/http/providers_url_validate_test.go

@@ -67,11 +67,10 @@ func TestValidateProviderURL(t *testing.T) {
 		{"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 ---
+		// --- Scheme check: unconditional for URL-based types ---
 		{"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 ---
@@ -79,10 +78,18 @@ func TestValidateProviderURL(t *testing.T) {
 		{"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},
 
+		// --- ACP: executable path / command, not a URL (issue #1481) ---
+		{"acp empty", "", "acp", false},
+		{"acp built-in gemini", "gemini", "acp", false},
+		{"acp built-in claude", "claude", "acp", false},
+		{"acp built-in codex", "codex", "acp", false},
+		{"acp URL rejected", "http://127.0.0.1:9090", "acp", true},
+		{"acp private URL rejected", "http://10.0.0.1:8080/v1", "acp", true},
+		{"acp relative rejected", "relative/acp", "acp", true},
+
 		// 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},
@@ -91,7 +98,6 @@ func TestValidateProviderURL(t *testing.T) {
 		{"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},
@@ -189,7 +195,6 @@ func TestValidateProviderURL_LocalTypesIgnoreAllowPrivateFlag(t *testing.T) {
 		{"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 {
@@ -209,7 +214,6 @@ func TestValidateProviderURL_LocalTypeSchemeEnforced(t *testing.T) {
 	}{
 		{"file:///etc/passwd", "ollama"},
 		{"gopher://localhost:25", "ollama"},
-		{"file:///etc/passwd", "acp"},
 	}
 	for _, c := range cases {
 		err := validateProviderURL(c.url, c.providerType)
@@ -275,8 +279,6 @@ func TestValidateProviderURL_LocalTypeAllowedHosts(t *testing.T) {
 		{"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 {
@@ -314,6 +316,38 @@ func TestValidateProviderURL_ClaudeCLIExecutablePath(t *testing.T) {
 	}
 }
 
+func TestValidateProviderURL_ACPExecutablePath(t *testing.T) {
+	saveAndRestoreGlobals(t)
+	absBinary := filepath.Join(t.TempDir(), "gemini")
+
+	allowed := []string{
+		"",
+		"claude",
+		"codex",
+		"gemini",
+		absBinary,
+		filepath.Join(t.TempDir(), "Gemini.app", "Contents", "MacOS", "gemini"),
+	}
+	for _, raw := range allowed {
+		if err := validateProviderURL(raw, "acp"); err != nil {
+			t.Errorf("expected ACP executable %q to be allowed, got: %v", raw, err)
+		}
+	}
+
+	blocked := []string{
+		"file:///usr/local/bin/gemini",
+		"https://api.anthropic.com/v1",
+		"relative/gemini",
+		"gemini --dangerous-flag",
+		"http://localhost:9090",
+	}
+	for _, raw := range blocked {
+		if err := validateProviderURL(raw, "acp"); err == nil {
+			t.Errorf("expected ACP executable %q to be rejected", raw)
+		}
+	}
+}
+
 // --- Public remote URL always OK ---
 
 func TestValidateProviderURL_PublicHostOK(t *testing.T) {
@@ -346,9 +380,6 @@ func TestValidateProviderURL_OllamaAllowedHosts(t *testing.T) {
 		if err := validateProviderURL("http://192.168.3.31:11434/v1", "ollama"); err != nil {
 			t.Errorf("expected configured LAN IP to be allowed, got: %v", err)
 		}
-		if err := validateProviderURL("http://ollama.lan:11434/v1", "acp"); err != nil {
-			t.Errorf("expected configured LAN hostname to be allowed for acp, got: %v", err)
-		}
 		// Case-insensitive host match, matching the existing style in this file.
 		if err := validateProviderURL("http://OLLAMA.LAN:11434/v1", "ollama"); err != nil {
 			t.Errorf("expected case-insensitive host match to be allowed, got: %v", err)