Merge commit '4c500055d2c96e84b4cfb225c51be7b4beaf8e1b' into codex/merge-pr-1209

internal/gateway/methods/bitrix_portals.go

@@ -4,8 +4,11 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"log/slog"
+	"net"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/google/uuid"
@@ -13,6 +16,7 @@ import (
 	"github.com/nextlevelbuilder/goclaw/internal/gateway"
 	"github.com/nextlevelbuilder/goclaw/internal/i18n"
 	"github.com/nextlevelbuilder/goclaw/internal/permissions"
+	"github.com/nextlevelbuilder/goclaw/internal/security"
 	"github.com/nextlevelbuilder/goclaw/internal/store"
 	"github.com/nextlevelbuilder/goclaw/pkg/protocol"
 )
@@ -69,9 +73,13 @@ type bitrixPortalView struct {
 // Validation regexes. Kept package-level so they compile once and tests can
 // reference them directly.
 var (
-	// Bitrix24 cloud portal hosts. Matches *.bitrix24.{com,eu,ru,de,fr,jp,in,kz,ua,by}
+	// Bitrix24 cloud portal hosts. Matches *.bitrix24.{com,eu,ru,de,fr,jp,in,kz,ua,by,vn,tr,es,com.br,com.ar}
 	// plus self-hosted *.bitrix.info. Subdomain regex matches DNS label rules.
-	bitrixDomainRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.(bitrix24\.(com|eu|ru|de|fr|jp|in|kz|ua|by)|bitrix\.info)$`)
+	bitrixCloudDomainRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.(bitrix24\.(com|eu|ru|de|fr|jp|in|kz|ua|by|vn|tr|es|com\.br|com\.ar)|bitrix\.info)$`)
+
+	// Valid hostname regex for self-hosted Bitrix24 instances (custom domains).
+	// Accepts any valid FQDN or hostname with optional port (e.g. bx.example.com, portal.internal:8443).
+	selfHostedDomainRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*(\:\d+)?$`)
 
 	// Portal name: lowercase slug used in install state token + channel config
 	// reference. Underscore allowed for legacy CLI-created portals.
@@ -142,10 +150,18 @@ func (m *BitrixPortalsMethods) handleCreate(ctx context.Context, client *gateway
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidRequest, "name: lowercase letters, digits, hyphen, underscore (2-64 chars)")))
 		return
 	}
-	if !bitrixDomainRegex.MatchString(domain) {
-		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidRequest, "domain: must be *.bitrix24.{com,eu,ru,…} or *.bitrix.info")))
+	if !bitrixCloudDomainRegex.MatchString(domain) && !selfHostedDomainRegex.MatchString(domain) {
+		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidRequest, "domain: must be a valid hostname (e.g. *.bitrix24.com, *.bitrix.info, or your self-hosted domain)")))
 		return
 	}
+	// SSRF + port validation for self-hosted domains (cloud domains are
+	// Bitrix-operated and implicitly trusted).
+	if !bitrixCloudDomainRegex.MatchString(domain) {
+		if err := validateSelfHostedDomain(domain); err != nil {
+			client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidRequest, "domain: "+err.Error())))
+			return
+		}
+	}
 	if clientID == "" || clientSecret == "" {
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgRequired, "client_id and client_secret")))
 		return
@@ -352,6 +368,75 @@ func portalRowToView(row store.BitrixPortalData) bitrixPortalView {
 	return v
 }
 
+// lookupHost is the DNS resolver used by validateSelfHostedDomain.
+// Replaced in tests to avoid real network calls and to exercise multi-IP
+// SSRF bypass scenarios.
+var lookupHost = net.LookupHost
+
+// validateSelfHostedDomain checks a self-hosted Bitrix24 domain for SSRF
+// risks and invalid port ranges. Cloud domains (*.bitrix24.*, *.bitrix.info)
+// are Bitrix-operated and implicitly trusted — this function is only called
+// for custom/self-hosted domains.
+//
+// Policy:
+//   - Rejects literal private/loopback/metadata IPs (127.x, 10.x, 192.168.x,
+//     169.254.x, ::1, fc00::, etc.)
+//   - Rejects hostnames where ANY resolved IP is blocked (not just the first)
+//   - Rejects .localhost and .local TLDs (commonly used for local dev)
+//   - Validates port range 1-65535 when a port is specified
+func validateSelfHostedDomain(domain string) error {
+	// Extract host and optional port.
+	host := domain
+	portStr := ""
+	if idx := strings.LastIndex(domain, ":"); idx != -1 {
+		host = domain[:idx]
+		portStr = domain[idx+1:]
+	}
+
+	// Validate port range if present.
+	if portStr != "" {
+		port, err := strconv.Atoi(portStr)
+		if err != nil || port < 1 || port > 65535 {
+			return fmt.Errorf("port must be 1-65535")
+		}
+	}
+
+	// Reject .localhost and .local TLDs (commonly used for local development).
+	lowerHost := strings.ToLower(host)
+	if strings.HasSuffix(lowerHost, ".localhost") || strings.HasSuffix(lowerHost, ".local") || lowerHost == "localhost" {
+		return fmt.Errorf("private/internal hostnames (localhost, .local, .localhost) are not allowed")
+	}
+
+	// If the host is a literal IP, check it against blocked CIDRs.
+	if ip := net.ParseIP(host); ip != nil {
+		if security.IsBlocked(ip) {
+			return fmt.Errorf("IP %s is in a blocked range (loopback/private/metadata)", ip)
+		}
+		return nil
+	}
+
+	// For hostnames, resolve and check ALL returned IPs. DNS result ordering
+	// is not a security boundary — a resolver may return a public IP first
+	// followed by a private one (e.g. split-horizon, fallback addresses).
+	addrs, err := lookupHost(host)
+	if err != nil {
+		return fmt.Errorf("cannot resolve hostname %q", host)
+	}
+	if len(addrs) == 0 {
+		return fmt.Errorf("hostname %q resolved to no addresses", host)
+	}
+	for _, addr := range addrs {
+		ip := net.ParseIP(addr)
+		if ip == nil {
+			return fmt.Errorf("resolved address %q is not a valid IP", addr)
+		}
+		if security.IsBlocked(ip) {
+			return fmt.Errorf("hostname %q resolved to blocked IP %s (loopback/private/metadata)", host, ip)
+		}
+	}
+	return nil
+}
+
 // isDuplicateKeyErr probes a store error for a UNIQUE violation. Kept as a
 // string substring match because the store interface doesn't expose typed
 // duplicate errors and we want consistent behaviour between pg + sqlite

internal/gateway/methods/bitrix_portals_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"strings"
 	"sync"
 	"testing"
@@ -366,17 +367,62 @@ func TestBitrixPortals_Create_HappyPath_ReturnsInstallURL(t *testing.T) {
 func TestBitrixPortals_Create_InvalidDomain(t *testing.T) {
 	tid := uuid.New()
 	m := NewBitrixPortalsMethods(newStubBitrixPortalStore(), newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
-	client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
+
+	cases := []struct {
+		name   string
+		domain string
+	}{
+		{"leading hyphen", "-invalid.com"},
+		{"scheme included", "https://example.com"},
+		{"path included", "example.com/path"},
+		{"space in domain", "exam ple.com"},
+		{"invalid port zero", "example.com:0"},
+		{"invalid port overflow", "example.com:99999"},
+		{"localhost", "localhost"},
+		{"localhost subdomain", "bx.localhost"},
+		{"local TLD", "bx.local"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
+			m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
+				"name":          "p",
+				"domain":        tc.domain,
+				"client_id":     "x",
+				"client_secret": "y",
+			}))
+			resp := readResponse(t, ch)
+			if resp.Error == nil || resp.Error.Code != protocol.ErrInvalidRequest {
+				t.Errorf("expected INVALID_REQUEST for %q, got %+v", tc.domain, resp.Error)
+			}
+		})
+	}
+}
+
+func TestBitrixPortals_Create_SelfHostedDomain(t *testing.T) {
+	tid := uuid.MustParse("22222222-2222-2222-2222-222222222222")
+	pStore := newStubBitrixPortalStore()
+	m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://goclaw.tamgiac.com"))
+
+	// Use a cloud domain (bitrixCloudDomainRegex) for the happy-path test
+	// since it bypasses SSRF DNS validation. Self-hosted SSRF validation
+	// is covered by TestValidateSelfHostedDomain_* tests.
+	client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "admin", 4)
 	m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
-		"name":          "p",
-		"domain":        "not-a-bitrix-domain.com",
-		"client_id":     "x",
-		"client_secret": "y",
+		"name":          "myportal",
+		"domain":        "myportal.bitrix24.com",
+		"client_id":     "local.abc",
+		"client_secret": "secret123",
 	}))
 
 	resp := readResponse(t, ch)
-	if resp.Error == nil || resp.Error.Code != protocol.ErrInvalidRequest {
-		t.Errorf("expected INVALID_REQUEST, got %+v", resp.Error)
+	if resp.Error != nil {
+		t.Fatalf("create with self-hosted domain failed: %+v", resp.Error)
+	}
+	result := resp.Payload.(map[string]any)
+	if result["domain"] != "myportal.bitrix24.com" {
+		t.Errorf("domain = %q, want myportal.bitrix24.com", result["domain"])
 	}
 }
 
@@ -540,22 +586,51 @@ func TestBitrixDomainRegex(t *testing.T) {
 		"my-corp.bitrix24.eu",
 		"a.bitrix24.com",
 		"company.bitrix.info",
+		"mycorp.bitrix24.vn",
+		"portal.bitrix24.tr",
+		"miempresa.bitrix24.es",
+		"empresa.bitrix24.com.br",
 	}
 	bad := []string{
 		"tamgiac.bitrix24",
-		"tamgiac.example.com",
 		"tamgiac.bitrix24.xx",
 		"-bad.bitrix24.com",
 		"UPPER.bitrix24.com", // we lowercase before match
 		"a.b.bitrix24.com",   // multi-level subdomain not allowed
 	}
 	for _, d := range good {
-		if !bitrixDomainRegex.MatchString(d) {
+		if !bitrixCloudDomainRegex.MatchString(d) {
 			t.Errorf("should accept %q", d)
 		}
 	}
 	for _, d := range bad {
-		if bitrixDomainRegex.MatchString(d) {
+		if bitrixCloudDomainRegex.MatchString(d) {
+			t.Errorf("should reject %q", d)
+		}
+	}
+}
+
+func TestSelfHostedDomainRegex(t *testing.T) {
+	good := []string{
+		"bx.example.com",
+		"portal.internal",
+		"bitrix.mycompany.co.uk",
+		"portal.example.com:8443",
+		"bx.corp",
+	}
+	bad := []string{
+		"-bad.example.com",
+		"UPPER.example.com", // we lowercase before match
+		"",
+		"not a domain",
+	}
+	for _, d := range good {
+		if !selfHostedDomainRegex.MatchString(d) {
+			t.Errorf("should accept %q", d)
+		}
+	}
+	for _, d := range bad {
+		if selfHostedDomainRegex.MatchString(d) {
 			t.Errorf("should reject %q", d)
 		}
 	}
@@ -578,6 +653,101 @@ func TestPortalNameRegex(t *testing.T) {
 	}
 }
 
+// ---------------------------------------------------------------------------
+// SSRF + port validation for self-hosted domains
+// ---------------------------------------------------------------------------
+
+func TestValidateSelfHostedDomain_SSRF(t *testing.T) {
+	// These should all be rejected as SSRF risks.
+	blocked := []string{
+		"127.0.0.1",
+		"127.0.0.1:8080",
+		"::1",
+		"10.0.0.1",
+		"10.0.0.1:443",
+		"192.168.1.1",
+		"172.16.0.1",
+		"169.254.169.254", // cloud metadata
+		"0.0.0.0",
+		"localhost",
+		"bx.localhost",
+		"bx.local",
+		"portal.internal.localhost",
+	}
+	for _, d := range blocked {
+		if err := validateSelfHostedDomain(d); err == nil {
+			t.Errorf("should reject %q (SSRF risk)", d)
+		}
+	}
+}
+
+func TestValidateSelfHostedDomain_PortRange(t *testing.T) {
+	badPorts := []string{
+		"example.com:0",
+		"example.com:99999",
+		"example.com:-1",
+		"example.com:abc",
+	}
+	for _, d := range badPorts {
+		if err := validateSelfHostedDomain(d); err == nil {
+			t.Errorf("should reject %q (invalid port)", d)
+		}
+	}
+}
+
+func TestValidateSelfHostedDomain_StubbedDNS(t *testing.T) {
+	// Save and restore the real resolver.
+	orig := lookupHost
+	defer func() { lookupHost = orig }()
+
+	tests := []struct {
+		name    string
+		host    string
+		addrs   []string
+		dnsErr  error
+		wantErr bool
+	}{
+		{"single public IP", "public.example.com", []string{"8.8.8.8"}, nil, false},
+		{"public IP with port", "public.example.com", []string{"93.184.216.34"}, nil, false},
+		{"single private IP", "internal.example.com", []string{"10.0.0.1"}, nil, true},
+		{"DNS resolution failure", "unresolvable.example.com", nil, fmt.Errorf("no such host"), true},
+		{"no addresses returned", "empty.example.com", []string{}, nil, true},
+		{"invalid IP string", "bad.example.com", []string{"not-an-ip"}, nil, true},
+		// Regression: multi-IP SSRF bypass — public IP first, private IP second.
+		// DNS ordering is not a security boundary; must reject if ANY IP is blocked.
+		{"multi-ip public-then-private", "evil.example.com", []string{"8.8.8.8", "10.0.0.1"}, nil, true},
+		{"multi-ip private-then-public", "evil2.example.com", []string{"192.168.1.1", "8.8.4.4"}, nil, true},
+		{"multi-ip all public", "safe.example.com", []string{"8.8.8.8", "8.8.4.4"}, nil, false},
+		{"multi-ip metadata IP", "meta.example.com", []string{"8.8.8.8", "169.254.169.254"}, nil, true},
+		{"multi-ip IPv6 loopback", "ipv6evil.example.com", []string{"2001:db8::1", "::1"}, nil, true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			lookupHost = func(host string) ([]string, error) {
+				if host == tc.host {
+					return tc.addrs, tc.dnsErr
+				}
+				return nil, fmt.Errorf("unexpected host: %s", host)
+			}
+
+			domain := tc.host
+			// Add port for the "public IP with port" case.
+			if tc.name == "public IP with port" {
+				domain = tc.host + ":443"
+			}
+
+			err := validateSelfHostedDomain(domain)
+			if tc.wantErr && err == nil {
+				t.Errorf("expected error for %q, got nil", tc.name)
+			}
+			if !tc.wantErr && err != nil {
+				t.Errorf("expected no error for %q, got: %v", tc.name, err)
+			}
+		})
+	}
+}
+
 // TestIsDuplicateKeyErr covers the two backend error string shapes we map
 // to ALREADY_EXISTS.
 func TestIsDuplicateKeyErr(t *testing.T) {

ui/web/src/pages/channels/bitrix24/bitrix-portal-form-step.tsx

@@ -10,13 +10,72 @@ import { useBitrixPortalCreate } from "./use-bitrix-portals";
 // Validation mirrors the server-side regex in
 // internal/gateway/methods/bitrix_portals.go. Server is authoritative;
 // client validation is purely UX so the operator gets feedback before a
-// round-trip. Pattern intentionally accepts a wide TLD set (Bitrix24 has
-// regional clouds: .com, .eu, .ru, .de, .fr, .jp, .in, .kz, .ua, .by) plus
-// .bitrix.info for self-hosted.
+// round-trip. Pattern accepts Bitrix24 regional clouds (.com, .eu, .ru,
+// .de, .fr, .jp, .in, .kz, .ua, .by, .vn, .tr, .es, .com.br, .com.ar),
+// .bitrix.info self-hosted, plus any valid hostname/port for fully
+// self-hosted custom domains.
 const BITRIX_DOMAIN_RE =
-  /^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.(bitrix24\.(com|eu|ru|de|fr|jp|in|kz|ua|by)|bitrix\.info)$/;
+  /^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.(bitrix24\.(com|eu|ru|de|fr|jp|in|kz|ua|by|vn|tr|es|com\.br|com\.ar)|bitrix\.info)$/;
+const SELF_HOSTED_DOMAIN_RE =
+  /^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*(:\d+)?$/;
 const PORTAL_NAME_RE = /^[a-z0-9][a-z0-9_-]{0,62}[a-z0-9]$/;
 
+// validateSelfHostedDomain mirrors the backend SSRF + port validation.
+// Rejects localhost, .local, .localhost TLDs, literal private/loopback IPs,
+// and invalid port ranges (0, >65535).
+function validateSelfHostedDomain(domain: string): string | null {
+  // Extract host and optional port.
+  let host = domain;
+  let portStr: string | undefined;
+  const colonIdx = domain.lastIndexOf(":");
+  if (colonIdx !== -1) {
+    host = domain.slice(0, colonIdx);
+    portStr = domain.slice(colonIdx + 1);
+  }
+
+  // Validate port range.
+  if (portStr !== undefined) {
+    const port = Number(portStr);
+    if (!Number.isInteger(port) || port < 1 || port > 65535) {
+      return "port must be 1-65535";
+    }
+  }
+
+  // Reject localhost and .local/.localhost TLDs.
+  const lowerHost = host.toLowerCase();
+  if (
+    lowerHost === "localhost" ||
+    lowerHost.endsWith(".localhost") ||
+    lowerHost.endsWith(".local")
+  ) {
+    return "private/internal hostnames (localhost, .local, .localhost) are not allowed";
+  }
+
+  // Reject literal private/loopback IPs.
+  // Simple check for common patterns — backend does full CIDR validation.
+  if (
+    lowerHost === "127.0.0.1" ||
+    lowerHost.startsWith("127.") ||
+    lowerHost === "10.0.0.0" ||
+    lowerHost.startsWith("10.") ||
+    lowerHost.startsWith("192.168.") ||
+    lowerHost.startsWith("172.16.") ||
+    lowerHost.startsWith("172.17.") ||
+    lowerHost.startsWith("172.18.") ||
+    lowerHost.startsWith("172.19.") ||
+    lowerHost.startsWith("172.2") ||
+    lowerHost.startsWith("172.3") ||
+    lowerHost === "169.254.169.254" ||
+    lowerHost.startsWith("169.254.") ||
+    lowerHost === "::1" ||
+    lowerHost === "0.0.0.0"
+  ) {
+    return "IP is in a blocked range (loopback/private/metadata)";
+  }
+
+  return null;
+}
+
 interface BitrixPortalFormStepProps {
   /** Invoked with the server response after bitrix.portals.create succeeds. */
   onSuccess: (createdName: string, installUrl: string, warning?: string) => void;
@@ -53,10 +112,21 @@ export function BitrixPortalFormStep({ onSuccess, onCancel }: BitrixPortalFormSt
         defaultValue: "Use lowercase letters, digits, hyphens, underscores (2-64 chars).",
       });
     }
-    if (!BITRIX_DOMAIN_RE.test(domain.toLowerCase())) {
+    const domainLower = domain.toLowerCase();
+    const isCloud = BITRIX_DOMAIN_RE.test(domainLower);
+    const isSelfHostedSyntax = SELF_HOSTED_DOMAIN_RE.test(domainLower);
+    if (!isCloud && !isSelfHostedSyntax) {
       e.domain = t("bitrix24.create.errors.invalidDomain", {
-        defaultValue: "Must be a valid Bitrix24 portal domain (e.g. mycorp.bitrix24.com).",
+        defaultValue: "Must be a valid hostname (e.g. *.bitrix24.com, *.bitrix.info, or your self-hosted domain).",
       });
+    } else if (!isCloud) {
+      // SSRF + port validation for self-hosted domains.
+      const ssrfErr = validateSelfHostedDomain(domainLower);
+      if (ssrfErr) {
+        e.domain = t("bitrix24.create.errors.invalidDomain", {
+          defaultValue: ssrfErr,
+        });
+      }
     }
     if (!clientId.trim()) e.client_id = t("common.required", { defaultValue: "Required" });
     if (!clientSecret.trim()) e.client_secret = t("common.required", { defaultValue: "Required" });
@@ -121,7 +191,7 @@ export function BitrixPortalFormStep({ onSuccess, onCancel }: BitrixPortalFormSt
           value={domain}
           onChange={(e) => setDomain(e.target.value)}
           onBlur={handleDomainBlur}
-          placeholder="tamgiac.bitrix24.com"
+          placeholder="mycorp.bitrix24.vn or bitrix.example.com"
           autoComplete="off"
           autoFocus
         />