From 0a775d9fb97083500823557911c12e9f32b73e56 Mon Sep 17 00:00:00 2001 From: Sakurapainting Date: Thu, 2 Apr 2026 18:02:31 +0800 Subject: [PATCH 1/7] feat(gateway): add bind fallback and CIDR policy enforcement --- config/config.example.json | 4 +- pkg/channels/manager.go | 22 ++- pkg/config/defaults.go | 9 +- pkg/config/gateway.go | 9 +- pkg/gateway/gateway.go | 62 ++++++- pkg/gateway/network_policy.go | 266 +++++++++++++++++++++++++++++ pkg/gateway/network_policy_test.go | 200 ++++++++++++++++++++++ web/backend/api/config.go | 10 ++ web/backend/api/config_test.go | 25 +++ 9 files changed, 587 insertions(+), 20 deletions(-) create mode 100644 pkg/gateway/network_policy.go create mode 100644 pkg/gateway/network_policy_test.go diff --git a/config/config.example.json b/config/config.example.json index f0cce6d72..2989c0726 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -465,9 +465,11 @@ }, "gateway": { "_comment": "Default log level is set to 'fatal'. Other available options are 'debug', 'info', 'warn' and 'error'.", + "_comment_allowed_cidrs": "Optional CIDR allowlist for gateway HTTP endpoints. Empty means no CIDR restriction unless automatic fallback enables a generated local allowlist.", "host": "127.0.0.1", "port": 18790, "hot_reload": false, - "log_level": "fatal" + "log_level": "fatal", + "allowed_cidrs": [] } } diff --git a/pkg/channels/manager.go b/pkg/channels/manager.go index 239448a1c..00210eb65 100644 --- a/pkg/channels/manager.go +++ b/pkg/channels/manager.go @@ -98,6 +98,10 @@ type asyncTask struct { cancel context.CancelFunc } +// HTTPMiddleware wraps an HTTP handler and may return an error if middleware +// configuration is invalid. +type HTTPMiddleware func(http.Handler) (http.Handler, error) + // RecordPlaceholder registers a placeholder message for later editing. // Implements PlaceholderRecorder. func (m *Manager) RecordPlaceholder(channel, chatID, placeholderID string) { @@ -436,7 +440,7 @@ func (m *Manager) initChannels(channels *config.ChannelsConfig) error { // SetupHTTPServer creates a shared HTTP server with the given listen address. // It registers health endpoints from the health server and discovers channels // that implement WebhookHandler and/or HealthChecker to register their handlers. -func (m *Manager) SetupHTTPServer(addr string, healthServer *health.Server) { +func (m *Manager) SetupHTTPServer(addr string, healthServer *health.Server, middlewares ...HTTPMiddleware) error { m.mux = newDynamicServeMux() // Register health endpoints @@ -447,12 +451,26 @@ func (m *Manager) SetupHTTPServer(addr string, healthServer *health.Server) { // Discover and register webhook handlers and health checkers m.registerHTTPHandlersLocked() + handler := http.Handler(m.mux) + for idx, mw := range middlewares { + if mw == nil { + continue + } + wrapped, err := mw(handler) + if err != nil { + return fmt.Errorf("apply HTTP middleware #%d: %w", idx, err) + } + handler = wrapped + } + m.httpServer = &http.Server{ Addr: addr, - Handler: m.mux, + Handler: handler, ReadTimeout: 30 * time.Second, WriteTimeout: 30 * time.Second, } + + return nil } // registerHTTPHandlersLocked registers webhook and health-check handlers for diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 39cdb89e6..6f1690b51 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -358,10 +358,11 @@ func DefaultConfig() *Config { }, }, Gateway: GatewayConfig{ - Host: "127.0.0.1", - Port: 18790, - HotReload: false, - LogLevel: DefaultGatewayLogLevel, + Host: "127.0.0.1", + Port: 18790, + HotReload: false, + LogLevel: DefaultGatewayLogLevel, + AllowedCIDRs: nil, }, Tools: ToolsConfig{ FilterSensitiveData: true, diff --git a/pkg/config/gateway.go b/pkg/config/gateway.go index e9f4085d3..7dc620fc8 100644 --- a/pkg/config/gateway.go +++ b/pkg/config/gateway.go @@ -10,10 +10,11 @@ import ( const DefaultGatewayLogLevel = "warn" type GatewayConfig struct { - Host string `json:"host" env:"PICOCLAW_GATEWAY_HOST"` - Port int `json:"port" env:"PICOCLAW_GATEWAY_PORT"` - HotReload bool `json:"hot_reload" env:"PICOCLAW_GATEWAY_HOT_RELOAD"` - LogLevel string `json:"log_level,omitempty" env:"PICOCLAW_LOG_LEVEL"` + Host string `json:"host" env:"PICOCLAW_GATEWAY_HOST"` + Port int `json:"port" env:"PICOCLAW_GATEWAY_PORT"` + HotReload bool `json:"hot_reload" env:"PICOCLAW_GATEWAY_HOT_RELOAD"` + LogLevel string `json:"log_level,omitempty" env:"PICOCLAW_LOG_LEVEL"` + AllowedCIDRs []string `json:"allowed_cidrs,omitempty"` } func canonicalGatewayLogLevel(level logger.LogLevel) string { diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 8065a0795..9e27f8dec 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -3,10 +3,12 @@ package gateway import ( "context" "fmt" + "net" "os" "os/signal" "path/filepath" "sort" + "strconv" "strings" "sync" "sync/atomic" @@ -64,6 +66,9 @@ type services struct { DeviceService *devices.Service HealthServer *health.Server VoiceAgentCancel context.CancelFunc + ListenHost string + ListenAddr string + EffectiveCIDRs []string manualReloadChan chan struct{} reloading atomic.Bool authToken string @@ -204,7 +209,11 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error runningServices.HealthServer.SetReloadFunc(reloadTrigger) agentLoop.SetReloadFunc(reloadTrigger) - fmt.Printf("✓ Gateway started on %s:%d\n", cfg.Gateway.Host, cfg.Gateway.Port) + if runningServices.ListenHost == gatewayFallbackBindHost { + fmt.Printf("✓ Gateway started (all interfaces, port %d)\n", cfg.Gateway.Port) + } else { + fmt.Printf("✓ Gateway started on %s\n", runningServices.ListenAddr) + } fmt.Println("Press Ctrl+C to stop") ctx, cancel := context.WithCancel(context.Background()) @@ -265,6 +274,9 @@ func preCheckConfig(cfg *config.Config) error { if cfg.Gateway.Port <= 0 || cfg.Gateway.Port > 65535 { return fmt.Errorf("invalid gateway port: %d, port must be between 1 and 65535", cfg.Gateway.Port) } + if _, err := normalizeAndValidateCIDRs(cfg.Gateway.AllowedCIDRs); err != nil { + return fmt.Errorf("invalid gateway allowed_cidrs: %w", err) + } return nil } @@ -377,10 +389,39 @@ func setupAndStartServices( fmt.Println("⚠ Warning: No channels enabled") } - addr := fmt.Sprintf("%s:%d", cfg.Gateway.Host, cfg.Gateway.Port) + listenDecision, err := resolveGatewayListenDecision(cfg.Gateway.Host, cfg.Gateway.Port, cfg.Gateway.AllowedCIDRs) + if err != nil { + if fms, ok := runningServices.MediaStore.(*media.FileMediaStore); ok { + fms.Stop() + } + return nil, fmt.Errorf("error resolving gateway listen host: %w", err) + } + if listenDecision.AutoFallback { + logger.WarnCF("gateway", "Loopback bind failed, fallback to all interfaces with CIDR allowlist", map[string]any{ + "configured_host": cfg.Gateway.Host, + "bind_host": listenDecision.BindHost, + "port": cfg.Gateway.Port, + "allowed_cidrs": listenDecision.AllowedCIDRs, + "reason": listenDecision.FallbackReason, + }) + } + + addr := net.JoinHostPort(listenDecision.BindHost, strconv.Itoa(cfg.Gateway.Port)) + runningServices.ListenHost = listenDecision.BindHost + runningServices.ListenAddr = addr + runningServices.EffectiveCIDRs = listenDecision.AllowedCIDRs runningServices.authToken = authToken - runningServices.HealthServer = health.NewServer(cfg.Gateway.Host, cfg.Gateway.Port, authToken) - runningServices.ChannelManager.SetupHTTPServer(addr, runningServices.HealthServer) + runningServices.HealthServer = health.NewServer(listenDecision.BindHost, cfg.Gateway.Port, authToken) + if err = runningServices.ChannelManager.SetupHTTPServer( + addr, + runningServices.HealthServer, + newCIDRAllowlistMiddleware(listenDecision.AllowedCIDRs), + ); err != nil { + if fms, ok := runningServices.MediaStore.(*media.FileMediaStore); ok { + fms.Stop() + } + return nil, fmt.Errorf("error setting up shared HTTP server: %w", err) + } if err = runningServices.ChannelManager.StartAll(context.Background()); err != nil { return nil, fmt.Errorf("error starting channels: %w", err) @@ -396,11 +437,14 @@ func setupAndStartServices( voiceAgent.Start(vaCtx) } - fmt.Printf( - "✓ Health endpoints available at http://%s:%d/health, /ready and /reload (POST)\n", - cfg.Gateway.Host, - cfg.Gateway.Port, - ) + if runningServices.ListenHost == gatewayFallbackBindHost { + fmt.Printf("✓ Health endpoints available on all interfaces at port %d (/health, /ready and /reload POST)\n", cfg.Gateway.Port) + } else { + fmt.Printf("✓ Health endpoints available at http://%s/health, /ready and /reload (POST)\n", runningServices.ListenAddr) + } + if len(runningServices.EffectiveCIDRs) > 0 { + fmt.Printf("✓ Gateway CIDR allowlist enabled: %s\n", strings.Join(runningServices.EffectiveCIDRs, ", ")) + } stateManager := state.NewManager(cfg.WorkspacePath()) runningServices.DeviceService = devices.NewService(devices.Config{ diff --git a/pkg/gateway/network_policy.go b/pkg/gateway/network_policy.go new file mode 100644 index 000000000..7827dd8b4 --- /dev/null +++ b/pkg/gateway/network_policy.go @@ -0,0 +1,266 @@ +package gateway + +import ( + "errors" + "fmt" + "net" + "net/http" + "sort" + "strconv" + "strings" + + "github.com/sipeed/picoclaw/pkg/channels" +) + +const ( + gatewayDefaultLoopbackHost = "127.0.0.1" + gatewayFallbackBindHost = "0.0.0.0" +) + +type gatewayListenDecision struct { + BindHost string + Port int + AllowedCIDRs []string + AutoFallback bool + FallbackReason string +} + +var ( + probeGatewayBind = probeTCPBind + discoverGatewayCIDRs = discoverLocalInterfaceCIDRs +) + +func resolveGatewayListenDecision(configuredHost string, port int, configuredCIDRs []string) (*gatewayListenDecision, error) { + host := strings.TrimSpace(configuredHost) + if host == "" { + host = gatewayDefaultLoopbackHost + } + + normalizedCIDRs, err := normalizeAndValidateCIDRs(configuredCIDRs) + if err != nil { + return nil, fmt.Errorf("invalid gateway allowed_cidrs: %w", err) + } + + bindErr := probeGatewayBind(host, port) + if bindErr == nil { + return &gatewayListenDecision{ + BindHost: host, + Port: port, + AllowedCIDRs: normalizedCIDRs, + }, nil + } + + if !isLoopbackHost(host) { + return nil, fmt.Errorf("bind %s:%d failed: %w", host, port, bindErr) + } + + discoveredCIDRs, discoverErr := discoverGatewayCIDRs() + if discoverErr != nil { + return nil, fmt.Errorf( + "loopback bind %s:%d failed: %w; interface discovery failed: %v", + host, + port, + bindErr, + discoverErr, + ) + } + if len(discoveredCIDRs) == 0 { + return nil, fmt.Errorf( + "loopback bind %s:%d failed: %w; no non-loopback interface CIDRs discovered", + host, + port, + bindErr, + ) + } + + fallbackCIDRs := normalizedCIDRs + if len(fallbackCIDRs) == 0 { + fallbackCIDRs = discoveredCIDRs + } + if len(fallbackCIDRs) == 0 { + return nil, fmt.Errorf("loopback bind %s:%d failed: %w; fallback allowlist is empty", host, port, bindErr) + } + + if fallbackBindErr := probeGatewayBind(gatewayFallbackBindHost, port); fallbackBindErr != nil { + return nil, fmt.Errorf( + "loopback bind %s:%d failed: %w; fallback bind %s:%d failed: %v", + host, + port, + bindErr, + gatewayFallbackBindHost, + port, + fallbackBindErr, + ) + } + + return &gatewayListenDecision{ + BindHost: gatewayFallbackBindHost, + Port: port, + AllowedCIDRs: fallbackCIDRs, + AutoFallback: true, + FallbackReason: fmt.Sprintf("loopback bind %s:%d failed, fallback to %s:%d with CIDR allowlist", host, port, gatewayFallbackBindHost, port), + }, nil +} + +func normalizeAndValidateCIDRs(cidrs []string) ([]string, error) { + if len(cidrs) == 0 { + return nil, nil + } + + seen := make(map[string]struct{}, len(cidrs)) + out := make([]string, 0, len(cidrs)) + for _, raw := range cidrs { + trimmed := strings.TrimSpace(raw) + if trimmed == "" { + continue + } + _, ipNet, err := net.ParseCIDR(trimmed) + if err != nil { + return nil, fmt.Errorf("invalid CIDR %q: %w", trimmed, err) + } + canonical := ipNet.String() + if _, ok := seen[canonical]; ok { + continue + } + seen[canonical] = struct{}{} + out = append(out, canonical) + } + if len(out) == 0 { + return nil, nil + } + sort.Strings(out) + return out, nil +} + +func discoverLocalInterfaceCIDRs() ([]string, error) { + ifaces, err := net.Interfaces() + if err != nil { + return nil, err + } + + out := make([]string, 0, len(ifaces)) + seen := make(map[string]struct{}) + ifaceErrs := make([]error, 0) + + for _, iface := range ifaces { + if iface.Flags&net.FlagUp == 0 || iface.Flags&net.FlagLoopback != 0 { + continue + } + + addrs, err := iface.Addrs() + if err != nil { + ifaceErrs = append(ifaceErrs, fmt.Errorf("%s: %w", iface.Name, err)) + continue + } + + for _, addr := range addrs { + ipNet := toIPNet(addr) + if ipNet == nil || ipNet.IP == nil { + continue + } + ip := ipNet.IP + if ip.IsLoopback() || ip.IsUnspecified() || ip.IsMulticast() || ip.IsInterfaceLocalMulticast() || ip.IsLinkLocalMulticast() || ip.IsLinkLocalUnicast() { + continue + } + + masked := ip.Mask(ipNet.Mask) + if masked == nil { + continue + } + canonical := (&net.IPNet{IP: masked, Mask: ipNet.Mask}).String() + if _, ok := seen[canonical]; ok { + continue + } + seen[canonical] = struct{}{} + out = append(out, canonical) + } + } + + sort.Strings(out) + if len(out) == 0 && len(ifaceErrs) > 0 { + return nil, errors.Join(ifaceErrs...) + } + return out, nil +} + +func toIPNet(addr net.Addr) *net.IPNet { + switch v := addr.(type) { + case *net.IPNet: + return v + case *net.IPAddr: + if v.IP == nil { + return nil + } + bits := 128 + if v.IP.To4() != nil { + bits = 32 + } + return &net.IPNet{IP: v.IP, Mask: net.CIDRMask(bits, bits)} + default: + return nil + } +} + +func probeTCPBind(host string, port int) error { + addr := net.JoinHostPort(host, strconv.Itoa(port)) + ln, err := net.Listen("tcp", addr) + if err != nil { + return err + } + _ = ln.Close() + return nil +} + +func isLoopbackHost(host string) bool { + normalized := strings.TrimSpace(strings.ToLower(host)) + if normalized == "localhost" { + return true + } + ip := net.ParseIP(normalized) + return ip != nil && ip.IsLoopback() +} + +func newCIDRAllowlistMiddleware(allowedCIDRs []string) channels.HTTPMiddleware { + effectiveCIDRs := append([]string(nil), allowedCIDRs...) + return func(next http.Handler) (http.Handler, error) { + if len(effectiveCIDRs) == 0 { + return next, nil + } + + nets := make([]*net.IPNet, 0, len(effectiveCIDRs)) + for _, cidr := range effectiveCIDRs { + _, ipNet, err := net.ParseCIDR(cidr) + if err != nil { + return nil, fmt.Errorf("invalid CIDR %q: %w", cidr, err) + } + nets = append(nets, ipNet) + } + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ip := clientIPFromRemoteAddr(r.RemoteAddr) + if ip == nil { + http.Error(w, "Forbidden", http.StatusForbidden) + return + } + if ip.IsLoopback() { + next.ServeHTTP(w, r) + return + } + for _, ipNet := range nets { + if ipNet.Contains(ip) { + next.ServeHTTP(w, r) + return + } + } + http.Error(w, "Forbidden", http.StatusForbidden) + }), nil + } +} + +func clientIPFromRemoteAddr(remoteAddr string) net.IP { + host := remoteAddr + if h, _, err := net.SplitHostPort(remoteAddr); err == nil { + host = h + } + return net.ParseIP(strings.TrimSpace(host)) +} diff --git a/pkg/gateway/network_policy_test.go b/pkg/gateway/network_policy_test.go new file mode 100644 index 000000000..7c92fc729 --- /dev/null +++ b/pkg/gateway/network_policy_test.go @@ -0,0 +1,200 @@ +package gateway + +import ( + "errors" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "testing" +) + +func TestNormalizeAndValidateCIDRs(t *testing.T) { + got, err := normalizeAndValidateCIDRs([]string{" 192.168.1.20/24 ", "10.0.0.0/8", "192.168.1.0/24"}) + if err != nil { + t.Fatalf("normalizeAndValidateCIDRs() error = %v", err) + } + want := []string{"10.0.0.0/8", "192.168.1.0/24"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("normalizeAndValidateCIDRs() = %v, want %v", got, want) + } +} + +func TestNormalizeAndValidateCIDRsInvalid(t *testing.T) { + _, err := normalizeAndValidateCIDRs([]string{"bad-cidr"}) + if err == nil { + t.Fatal("normalizeAndValidateCIDRs() expected error for invalid CIDR") + } +} + +func TestResolveGatewayListenDecisionLoopbackFallbackUsesConfiguredCIDRs(t *testing.T) { + origProbe := probeGatewayBind + origDiscover := discoverGatewayCIDRs + t.Cleanup(func() { + probeGatewayBind = origProbe + discoverGatewayCIDRs = origDiscover + }) + + probeGatewayBind = func(host string, _ int) error { + switch host { + case "127.0.0.1": + return errors.New("loopback unavailable") + case gatewayFallbackBindHost: + return nil + default: + return nil + } + } + discoverGatewayCIDRs = func() ([]string, error) { + return []string{"192.168.50.0/24"}, nil + } + + decision, err := resolveGatewayListenDecision("127.0.0.1", 18790, []string{"10.0.0.0/8"}) + if err != nil { + t.Fatalf("resolveGatewayListenDecision() error = %v", err) + } + if !decision.AutoFallback { + t.Fatal("decision.AutoFallback = false, want true") + } + if decision.BindHost != gatewayFallbackBindHost { + t.Fatalf("decision.BindHost = %q, want %q", decision.BindHost, gatewayFallbackBindHost) + } + wantCIDRs := []string{"10.0.0.0/8"} + if !reflect.DeepEqual(decision.AllowedCIDRs, wantCIDRs) { + t.Fatalf("decision.AllowedCIDRs = %v, want %v", decision.AllowedCIDRs, wantCIDRs) + } +} + +func TestResolveGatewayListenDecisionLoopbackFallbackUsesDiscoveredCIDRs(t *testing.T) { + origProbe := probeGatewayBind + origDiscover := discoverGatewayCIDRs + t.Cleanup(func() { + probeGatewayBind = origProbe + discoverGatewayCIDRs = origDiscover + }) + + probeGatewayBind = func(host string, _ int) error { + switch host { + case "localhost": + return errors.New("loopback unavailable") + case gatewayFallbackBindHost: + return nil + default: + return nil + } + } + discoverGatewayCIDRs = func() ([]string, error) { + return []string{"192.168.1.0/24", "10.0.0.0/8"}, nil + } + + decision, err := resolveGatewayListenDecision("localhost", 18790, nil) + if err != nil { + t.Fatalf("resolveGatewayListenDecision() error = %v", err) + } + if !decision.AutoFallback { + t.Fatal("decision.AutoFallback = false, want true") + } + wantCIDRs := []string{"192.168.1.0/24", "10.0.0.0/8"} + if !reflect.DeepEqual(decision.AllowedCIDRs, wantCIDRs) { + t.Fatalf("decision.AllowedCIDRs = %v, want %v", decision.AllowedCIDRs, wantCIDRs) + } +} + +func TestResolveGatewayListenDecisionNonLoopbackFailure(t *testing.T) { + origProbe := probeGatewayBind + origDiscover := discoverGatewayCIDRs + t.Cleanup(func() { + probeGatewayBind = origProbe + discoverGatewayCIDRs = origDiscover + }) + + probeGatewayBind = func(host string, _ int) error { + if host == "192.0.2.1" { + return errors.New("cannot assign requested address") + } + return nil + } + discoverGatewayCIDRs = func() ([]string, error) { + return []string{"10.0.0.0/8"}, nil + } + + _, err := resolveGatewayListenDecision("192.0.2.1", 18790, nil) + if err == nil { + t.Fatal("resolveGatewayListenDecision() expected error") + } + if !strings.Contains(err.Error(), "bind 192.0.2.1:18790 failed") { + t.Fatalf("error = %q, want bind failure", err.Error()) + } +} + +func TestResolveGatewayListenDecisionLoopbackFailureNoDiscoveredCIDRs(t *testing.T) { + origProbe := probeGatewayBind + origDiscover := discoverGatewayCIDRs + t.Cleanup(func() { + probeGatewayBind = origProbe + discoverGatewayCIDRs = origDiscover + }) + + probeGatewayBind = func(host string, _ int) error { + switch host { + case "127.0.0.1": + return errors.New("loopback unavailable") + case gatewayFallbackBindHost: + return nil + default: + return nil + } + } + discoverGatewayCIDRs = func() ([]string, error) { + return nil, nil + } + + _, err := resolveGatewayListenDecision("127.0.0.1", 18790, nil) + if err == nil { + t.Fatal("resolveGatewayListenDecision() expected error") + } + if !strings.Contains(err.Error(), "no non-loopback interface CIDRs discovered") { + t.Fatalf("error = %q, want no-interface-cidr failure", err.Error()) + } +} + +func TestCIDRAllowlistMiddleware(t *testing.T) { + mw := newCIDRAllowlistMiddleware([]string{"192.168.1.0/24"}) + h, err := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + if err != nil { + t.Fatalf("middleware error = %v", err) + } + + tests := []struct { + name string + remoteAddr string + wantStatus int + }{ + {name: "inside cidr", remoteAddr: "192.168.1.99:1234", wantStatus: http.StatusOK}, + {name: "loopback", remoteAddr: "127.0.0.1:1234", wantStatus: http.StatusOK}, + {name: "outside cidr", remoteAddr: "10.0.0.7:1234", wantStatus: http.StatusForbidden}, + {name: "malformed", remoteAddr: "not-an-ip", wantStatus: http.StatusForbidden}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/health", nil) + req.RemoteAddr = tt.remoteAddr + h.ServeHTTP(rec, req) + if rec.Code != tt.wantStatus { + t.Fatalf("status = %d, want %d", rec.Code, tt.wantStatus) + } + }) + } +} + +func TestCIDRAllowlistMiddlewareInvalidCIDR(t *testing.T) { + mw := newCIDRAllowlistMiddleware([]string{"bad-cidr"}) + _, err := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + if err == nil { + t.Fatal("middleware expected error for invalid CIDR") + } +} diff --git a/web/backend/api/config.go b/web/backend/api/config.go index 5490b4e18..743954436 100644 --- a/web/backend/api/config.go +++ b/web/backend/api/config.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "io" + "net" "net/http" "regexp" "strings" @@ -279,6 +280,15 @@ func validateConfig(cfg *config.Config) []string { if cfg.Gateway.Port != 0 && (cfg.Gateway.Port < 1 || cfg.Gateway.Port > 65535) { errs = append(errs, fmt.Sprintf("gateway.port %d is out of valid range (1-65535)", cfg.Gateway.Port)) } + for index, cidr := range cfg.Gateway.AllowedCIDRs { + trimmed := strings.TrimSpace(cidr) + if trimmed == "" { + continue + } + if _, _, err := net.ParseCIDR(trimmed); err != nil { + errs = append(errs, fmt.Sprintf("gateway.allowed_cidrs[%d] is not a valid CIDR: %v", index, err)) + } + } // Pico channel: token required when enabled if cfg.Channels.Pico.Enabled && cfg.Channels.Pico.Token.String() == "" { diff --git a/web/backend/api/config_test.go b/web/backend/api/config_test.go index a90145f3c..938ec5c18 100644 --- a/web/backend/api/config_test.go +++ b/web/backend/api/config_test.go @@ -173,6 +173,31 @@ func TestHandlePatchConfig_AllowsInvalidExecRegexPatternsWhenExecDisabled(t *tes } } +func TestHandlePatchConfig_RejectsInvalidGatewayAllowedCIDRs(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + req := httptest.NewRequest(http.MethodPatch, "/api/config", bytes.NewBufferString(`{ + "gateway": { + "allowed_cidrs": ["bad-cidr"] + } + }`)) + req.Header.Set("Content-Type", "application/json") + + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusBadRequest, rec.Body.String()) + } + if !bytes.Contains(rec.Body.Bytes(), []byte("gateway.allowed_cidrs")) { + t.Fatalf("expected validation error mentioning gateway.allowed_cidrs, body=%s", rec.Body.String()) + } +} + // setupPicoEnabledEnv creates a test environment with Pico channel enabled and // its token stored only in .security.yml (not in the JSON payload). func setupPicoEnabledEnv(t *testing.T) (string, func()) { From e0db7fde0dedfcfca6ab528cf66a61bb6972793e Mon Sep 17 00:00:00 2001 From: Sakurapainting Date: Thu, 2 Apr 2026 18:24:36 +0800 Subject: [PATCH 2/7] chore(lint): fix golines formatting in gateway files --- pkg/config/gateway.go | 8 ++++---- pkg/gateway/gateway.go | 10 ++++++++-- pkg/gateway/network_policy.go | 25 ++++++++++++++++++------- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/pkg/config/gateway.go b/pkg/config/gateway.go index 7dc620fc8..3d22568c8 100644 --- a/pkg/config/gateway.go +++ b/pkg/config/gateway.go @@ -10,10 +10,10 @@ import ( const DefaultGatewayLogLevel = "warn" type GatewayConfig struct { - Host string `json:"host" env:"PICOCLAW_GATEWAY_HOST"` - Port int `json:"port" env:"PICOCLAW_GATEWAY_PORT"` - HotReload bool `json:"hot_reload" env:"PICOCLAW_GATEWAY_HOT_RELOAD"` - LogLevel string `json:"log_level,omitempty" env:"PICOCLAW_LOG_LEVEL"` + Host string `json:"host" env:"PICOCLAW_GATEWAY_HOST"` + Port int `json:"port" env:"PICOCLAW_GATEWAY_PORT"` + HotReload bool `json:"hot_reload" env:"PICOCLAW_GATEWAY_HOT_RELOAD"` + LogLevel string `json:"log_level,omitempty" env:"PICOCLAW_LOG_LEVEL"` AllowedCIDRs []string `json:"allowed_cidrs,omitempty"` } diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 9e27f8dec..7e9e66cbc 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -438,9 +438,15 @@ func setupAndStartServices( } if runningServices.ListenHost == gatewayFallbackBindHost { - fmt.Printf("✓ Health endpoints available on all interfaces at port %d (/health, /ready and /reload POST)\n", cfg.Gateway.Port) + fmt.Printf( + "✓ Health endpoints available on all interfaces at port %d (/health, /ready and /reload POST)\n", + cfg.Gateway.Port, + ) } else { - fmt.Printf("✓ Health endpoints available at http://%s/health, /ready and /reload (POST)\n", runningServices.ListenAddr) + fmt.Printf( + "✓ Health endpoints available at http://%s/health, /ready and /reload (POST)\n", + runningServices.ListenAddr, + ) } if len(runningServices.EffectiveCIDRs) > 0 { fmt.Printf("✓ Gateway CIDR allowlist enabled: %s\n", strings.Join(runningServices.EffectiveCIDRs, ", ")) diff --git a/pkg/gateway/network_policy.go b/pkg/gateway/network_policy.go index 7827dd8b4..4adf0580d 100644 --- a/pkg/gateway/network_policy.go +++ b/pkg/gateway/network_policy.go @@ -30,7 +30,11 @@ var ( discoverGatewayCIDRs = discoverLocalInterfaceCIDRs ) -func resolveGatewayListenDecision(configuredHost string, port int, configuredCIDRs []string) (*gatewayListenDecision, error) { +func resolveGatewayListenDecision( + configuredHost string, + port int, + configuredCIDRs []string, +) (*gatewayListenDecision, error) { host := strings.TrimSpace(configuredHost) if host == "" { host = gatewayDefaultLoopbackHost @@ -94,11 +98,17 @@ func resolveGatewayListenDecision(configuredHost string, port int, configuredCID } return &gatewayListenDecision{ - BindHost: gatewayFallbackBindHost, - Port: port, - AllowedCIDRs: fallbackCIDRs, - AutoFallback: true, - FallbackReason: fmt.Sprintf("loopback bind %s:%d failed, fallback to %s:%d with CIDR allowlist", host, port, gatewayFallbackBindHost, port), + BindHost: gatewayFallbackBindHost, + Port: port, + AllowedCIDRs: fallbackCIDRs, + AutoFallback: true, + FallbackReason: fmt.Sprintf( + "loopback bind %s:%d failed, fallback to %s:%d with CIDR allowlist", + host, + port, + gatewayFallbackBindHost, + port, + ), }, nil } @@ -159,7 +169,8 @@ func discoverLocalInterfaceCIDRs() ([]string, error) { continue } ip := ipNet.IP - if ip.IsLoopback() || ip.IsUnspecified() || ip.IsMulticast() || ip.IsInterfaceLocalMulticast() || ip.IsLinkLocalMulticast() || ip.IsLinkLocalUnicast() { + if ip.IsLoopback() || ip.IsUnspecified() || ip.IsMulticast() || + ip.IsInterfaceLocalMulticast() || ip.IsLinkLocalMulticast() || ip.IsLinkLocalUnicast() { continue } From 69f8a1f63051f2ca1f8882330e92e7c184c081df Mon Sep 17 00:00:00 2001 From: Sakurapainting Date: Thu, 2 Apr 2026 18:36:50 +0800 Subject: [PATCH 3/7] fix(gateway): address fallback review feedback --- docs/configuration.md | 34 +++++++++++++++ pkg/config/gateway.go | 8 ++-- pkg/gateway/gateway.go | 15 ++++++- pkg/gateway/network_policy.go | 68 ++++++++++++++++++++---------- pkg/gateway/network_policy_test.go | 24 ++++++++++- pkg/pid/pidfile.go | 35 +++++++++++++++ pkg/pid/pidfile_test.go | 36 ++++++++++++++++ 7 files changed, 192 insertions(+), 28 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 7a5902f58..952385701 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -49,6 +49,40 @@ When omitted, the default is `warn`. Supported values: `debug`, `info`, `warn`, You can also override this with the environment variable `PICOCLAW_LOG_LEVEL`. +### Gateway Host Fallback And CIDR Allowlist + +`gateway.host` defaults to `127.0.0.1`. + +When `gateway.host` is a loopback address (`127.0.0.1`, `::1`, or `localhost`) and bind fails (for example, on boards where loopback is unavailable), PicoClaw automatically: + +1. Falls back to bind on `0.0.0.0`. +2. Enforces a CIDR allowlist for gateway HTTP endpoints. +3. Discovers local interface CIDRs only when `gateway.allowed_cidrs` is empty. + +CIDR sources in fallback mode: + +- If `gateway.allowed_cidrs` is configured, that list is used. +- If `gateway.allowed_cidrs` is empty, discovered local CIDRs are used. +- If no non-loopback CIDR can be discovered, gateway startup fails. + +Loopback clients are always allowed for local administration. + +> **Reverse proxy note:** CIDR checks use connection `RemoteAddr`. If you place the gateway behind a local reverse proxy/tunnel (so requests arrive as loopback), those requests are treated as loopback and pass CIDR filtering at this layer. + +Example: + +```json +{ + "gateway": { + "host": "127.0.0.1", + "port": 18790, + "allowed_cidrs": [ + "192.168.1.0/24", + "10.0.0.0/8" + ] + } +} +``` ### Workspace Layout PicoClaw stores data in your configured workspace (default: `~/.picoclaw/workspace`): diff --git a/pkg/config/gateway.go b/pkg/config/gateway.go index 3d22568c8..22c76cbd7 100644 --- a/pkg/config/gateway.go +++ b/pkg/config/gateway.go @@ -10,10 +10,10 @@ import ( const DefaultGatewayLogLevel = "warn" type GatewayConfig struct { - Host string `json:"host" env:"PICOCLAW_GATEWAY_HOST"` - Port int `json:"port" env:"PICOCLAW_GATEWAY_PORT"` - HotReload bool `json:"hot_reload" env:"PICOCLAW_GATEWAY_HOT_RELOAD"` - LogLevel string `json:"log_level,omitempty" env:"PICOCLAW_LOG_LEVEL"` + Host string `json:"host" env:"PICOCLAW_GATEWAY_HOST"` + Port int `json:"port" env:"PICOCLAW_GATEWAY_PORT"` + HotReload bool `json:"hot_reload" env:"PICOCLAW_GATEWAY_HOT_RELOAD"` + LogLevel string `json:"log_level,omitempty" env:"PICOCLAW_LOG_LEVEL"` AllowedCIDRs []string `json:"allowed_cidrs,omitempty"` } diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 7e9e66cbc..0f10cf9a3 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -190,6 +190,19 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error return err } + if pidData.Host != runningServices.ListenHost || pidData.Port != cfg.Gateway.Port { + updatedPidData, updateErr := pid.UpdatePidFileEndpoint(homePath, runningServices.ListenHost, cfg.Gateway.Port) + if updateErr != nil { + logger.WarnCF("gateway", "Failed to sync pid listen endpoint", map[string]any{ + "error": updateErr.Error(), + "listen_host": runningServices.ListenHost, + "listen_port": cfg.Gateway.Port, + }) + } else { + pidData = updatedPidData + } + } + // Setup manual reload channel for /reload endpoint manualReloadChan := make(chan struct{}, 1) runningServices.manualReloadChan = manualReloadChan @@ -210,7 +223,7 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error agentLoop.SetReloadFunc(reloadTrigger) if runningServices.ListenHost == gatewayFallbackBindHost { - fmt.Printf("✓ Gateway started (all interfaces, port %d)\n", cfg.Gateway.Port) + fmt.Printf("✓ Gateway started (all interfaces, port %d)\n", pidData.Port) } else { fmt.Printf("✓ Gateway started on %s\n", runningServices.ListenAddr) } diff --git a/pkg/gateway/network_policy.go b/pkg/gateway/network_policy.go index 4adf0580d..f6441861c 100644 --- a/pkg/gateway/network_policy.go +++ b/pkg/gateway/network_policy.go @@ -19,7 +19,6 @@ const ( type gatewayListenDecision struct { BindHost string - Port int AllowedCIDRs []string AutoFallback bool FallbackReason string @@ -49,7 +48,6 @@ func resolveGatewayListenDecision( if bindErr == nil { return &gatewayListenDecision{ BindHost: host, - Port: port, AllowedCIDRs: normalizedCIDRs, }, nil } @@ -58,23 +56,27 @@ func resolveGatewayListenDecision( return nil, fmt.Errorf("bind %s:%d failed: %w", host, port, bindErr) } - discoveredCIDRs, discoverErr := discoverGatewayCIDRs() - if discoverErr != nil { - return nil, fmt.Errorf( - "loopback bind %s:%d failed: %w; interface discovery failed: %v", - host, - port, - bindErr, - discoverErr, - ) - } - if len(discoveredCIDRs) == 0 { - return nil, fmt.Errorf( - "loopback bind %s:%d failed: %w; no non-loopback interface CIDRs discovered", - host, - port, - bindErr, - ) + var discoveredCIDRs []string + if len(normalizedCIDRs) == 0 { + var discoverErr error + discoveredCIDRs, discoverErr = discoverGatewayCIDRs() + if discoverErr != nil { + return nil, fmt.Errorf( + "loopback bind %s:%d failed: %w; interface discovery failed: %v", + host, + port, + bindErr, + discoverErr, + ) + } + if len(discoveredCIDRs) == 0 { + return nil, fmt.Errorf( + "loopback bind %s:%d failed: %w; no non-loopback interface CIDRs discovered", + host, + port, + bindErr, + ) + } } fallbackCIDRs := normalizedCIDRs @@ -99,7 +101,6 @@ func resolveGatewayListenDecision( return &gatewayListenDecision{ BindHost: gatewayFallbackBindHost, - Port: port, AllowedCIDRs: fallbackCIDRs, AutoFallback: true, FallbackReason: fmt.Sprintf( @@ -174,11 +175,17 @@ func discoverLocalInterfaceCIDRs() ([]string, error) { continue } - masked := ip.Mask(ipNet.Mask) + mask := ipNet.Mask + if len(mask) == 0 { + continue + } + ip = normalizeIPForMask(ip, mask) + + masked := ip.Mask(mask) if masked == nil { continue } - canonical := (&net.IPNet{IP: masked, Mask: ipNet.Mask}).String() + canonical := (&net.IPNet{IP: masked, Mask: mask}).String() if _, ok := seen[canonical]; ok { continue } @@ -194,6 +201,20 @@ func discoverLocalInterfaceCIDRs() ([]string, error) { return out, nil } +func normalizeIPForMask(ip net.IP, mask net.IPMask) net.IP { + switch len(mask) { + case net.IPv4len: + if ip4 := ip.To4(); ip4 != nil { + return ip4 + } + case net.IPv6len: + if ip16 := ip.To16(); ip16 != nil { + return ip16 + } + } + return ip +} + func toIPNet(addr net.Addr) *net.IPNet { switch v := addr.(type) { case *net.IPNet: @@ -253,6 +274,9 @@ func newCIDRAllowlistMiddleware(allowedCIDRs []string) channels.HTTPMiddleware { http.Error(w, "Forbidden", http.StatusForbidden) return } + // Loopback is always allowed for local administration. + // When deployed behind a local reverse proxy/tunnel, forwarded + // external traffic may still appear as loopback at this layer. if ip.IsLoopback() { next.ServeHTTP(w, r) return diff --git a/pkg/gateway/network_policy_test.go b/pkg/gateway/network_policy_test.go index 7c92fc729..b8c63aa42 100644 --- a/pkg/gateway/network_policy_test.go +++ b/pkg/gateway/network_policy_test.go @@ -2,6 +2,7 @@ package gateway import ( "errors" + "net" "net/http" "net/http/httptest" "reflect" @@ -35,6 +36,8 @@ func TestResolveGatewayListenDecisionLoopbackFallbackUsesConfiguredCIDRs(t *test discoverGatewayCIDRs = origDiscover }) + discoverCalled := false + probeGatewayBind = func(host string, _ int) error { switch host { case "127.0.0.1": @@ -46,7 +49,8 @@ func TestResolveGatewayListenDecisionLoopbackFallbackUsesConfiguredCIDRs(t *test } } discoverGatewayCIDRs = func() ([]string, error) { - return []string{"192.168.50.0/24"}, nil + discoverCalled = true + return nil, errors.New("must not be called when configured CIDRs are provided") } decision, err := resolveGatewayListenDecision("127.0.0.1", 18790, []string{"10.0.0.0/8"}) @@ -63,6 +67,9 @@ func TestResolveGatewayListenDecisionLoopbackFallbackUsesConfiguredCIDRs(t *test if !reflect.DeepEqual(decision.AllowedCIDRs, wantCIDRs) { t.Fatalf("decision.AllowedCIDRs = %v, want %v", decision.AllowedCIDRs, wantCIDRs) } + if discoverCalled { + t.Fatal("discoverGatewayCIDRs() called unexpectedly") + } } func TestResolveGatewayListenDecisionLoopbackFallbackUsesDiscoveredCIDRs(t *testing.T) { @@ -198,3 +205,18 @@ func TestCIDRAllowlistMiddlewareInvalidCIDR(t *testing.T) { t.Fatal("middleware expected error for invalid CIDR") } } + +func TestNormalizeIPForMaskIPv4MappedIPv6(t *testing.T) { + ip := net.ParseIP("::ffff:192.168.10.20") + mask := net.CIDRMask(24, 32) + + normalized := normalizeIPForMask(ip, mask) + if got := normalized.String(); got != "192.168.10.20" { + t.Fatalf("normalizeIPForMask() = %q, want %q", got, "192.168.10.20") + } + + masked := normalized.Mask(mask) + if got := (&net.IPNet{IP: masked, Mask: mask}).String(); got != "192.168.10.0/24" { + t.Fatalf("masked network = %q, want %q", got, "192.168.10.0/24") + } +} diff --git a/pkg/pid/pidfile.go b/pkg/pid/pidfile.go index 69d02bc65..273b8a57e 100644 --- a/pkg/pid/pidfile.go +++ b/pkg/pid/pidfile.go @@ -99,6 +99,41 @@ func WritePidFile(homePath, host string, port int) (*PidFileData, error) { return data, nil } +// UpdatePidFileEndpoint updates host/port in the current process pid file +// without rotating the auth token. +func UpdatePidFileEndpoint(homePath, host string, port int) (*PidFileData, error) { + pidMu.Lock() + defer pidMu.Unlock() + + pidPath := pidFilePath(homePath) + data, err := readPidFileUnlocked(pidPath) + if err != nil { + return nil, fmt.Errorf("failed to read pid file: %w", err) + } + if data.PID != os.Getpid() { + return nil, fmt.Errorf("pid file belongs to another process: %d", data.PID) + } + + data.Host = host + data.Port = port + + raw, err := json.MarshalIndent(data, "", " ") + if err != nil { + return nil, fmt.Errorf("failed to marshal pid file: %w", err) + } + + tmp := pidPath + ".tmp" + if err := os.WriteFile(tmp, raw, 0o600); err != nil { + return nil, fmt.Errorf("failed to write pid file: %w", err) + } + if err := os.Rename(tmp, pidPath); err != nil { + os.Remove(tmp) + return nil, fmt.Errorf("failed to rename pid file: %w", err) + } + + return data, nil +} + // ReadPidFileWithCheck reads the PID file and additionally checks if // the recorded process is still alive. Returns nil if the file is // missing, unreadable, or the process has exited. diff --git a/pkg/pid/pidfile_test.go b/pkg/pid/pidfile_test.go index 921f590ad..1545d8a61 100644 --- a/pkg/pid/pidfile_test.go +++ b/pkg/pid/pidfile_test.go @@ -120,6 +120,42 @@ func TestWritePidFileOverwrite(t *testing.T) { } } +func TestUpdatePidFileEndpoint(t *testing.T) { + dir := tmpDir(t) + + data, err := WritePidFile(dir, "127.0.0.1", 18790) + if err != nil { + t.Fatalf("WritePidFile failed: %v", err) + } + + updated, err := UpdatePidFileEndpoint(dir, "0.0.0.0", 18888) + if err != nil { + t.Fatalf("UpdatePidFileEndpoint failed: %v", err) + } + if updated.Host != "0.0.0.0" { + t.Fatalf("updated.Host = %q, want %q", updated.Host, "0.0.0.0") + } + if updated.Port != 18888 { + t.Fatalf("updated.Port = %d, want 18888", updated.Port) + } + if updated.Token != data.Token { + t.Fatal("UpdatePidFileEndpoint must not rotate token") + } +} + +func TestUpdatePidFileEndpointDifferentPID(t *testing.T) { + dir := tmpDir(t) + + other := PidFileData{PID: 99999999, Token: "deadbeef12345678deadbeef12345678", Host: "127.0.0.1", Port: 18790} + raw, _ := json.MarshalIndent(other, "", " ") + os.WriteFile(filepath.Join(dir, pidFileName), raw, 0o600) + + _, err := UpdatePidFileEndpoint(dir, "0.0.0.0", 18888) + if err == nil { + t.Fatal("expected error when pid file belongs to another process") + } +} + // TestWritePidFileStalePID writes a PID file with a non-running PID, then // verifies WritePidFile cleans it up and writes a new one. func TestWritePidFileStalePID(t *testing.T) { From bf89c4101f07c6dbb478816db99a783e68ee2f9f Mon Sep 17 00:00:00 2001 From: Sakurapainting Date: Thu, 2 Apr 2026 18:58:18 +0800 Subject: [PATCH 4/7] fix(gateway): harden fallback CIDR policy and share allowlist --- docs/configuration.md | 6 +- pkg/gateway/gateway.go | 4 +- pkg/gateway/network_policy.go | 130 +++++++++++++----- pkg/gateway/network_policy_test.go | 128 ++++++++++++++++- pkg/netpolicy/allowlist.go | 81 +++++++++++ pkg/netpolicy/allowlist_test.go | 71 ++++++++++ web/backend/middleware/access_control.go | 41 ++---- web/backend/middleware/access_control_test.go | 18 +++ 8 files changed, 398 insertions(+), 81 deletions(-) create mode 100644 pkg/netpolicy/allowlist.go create mode 100644 pkg/netpolicy/allowlist_test.go diff --git a/docs/configuration.md b/docs/configuration.md index 952385701..e255b2891 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -57,13 +57,13 @@ When `gateway.host` is a loopback address (`127.0.0.1`, `::1`, or `localhost`) a 1. Falls back to bind on `0.0.0.0`. 2. Enforces a CIDR allowlist for gateway HTTP endpoints. -3. Discovers local interface CIDRs only when `gateway.allowed_cidrs` is empty. +3. Discovers private local interface CIDRs only when `gateway.allowed_cidrs` is empty. CIDR sources in fallback mode: - If `gateway.allowed_cidrs` is configured, that list is used. -- If `gateway.allowed_cidrs` is empty, discovered local CIDRs are used. -- If no non-loopback CIDR can be discovered, gateway startup fails. +- If `gateway.allowed_cidrs` is empty, discovered private CIDRs are used (`10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`, `100.64.0.0/10`, `fc00::/7`). +- If no private non-loopback CIDR can be discovered, gateway startup fails. On public-only hosts, configure `gateway.allowed_cidrs` explicitly. Loopback clients are always allowed for local administration. diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 0f10cf9a3..c06ecaae8 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -222,7 +222,7 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error runningServices.HealthServer.SetReloadFunc(reloadTrigger) agentLoop.SetReloadFunc(reloadTrigger) - if runningServices.ListenHost == gatewayFallbackBindHost { + if isWildcardBindHost(runningServices.ListenHost) { fmt.Printf("✓ Gateway started (all interfaces, port %d)\n", pidData.Port) } else { fmt.Printf("✓ Gateway started on %s\n", runningServices.ListenAddr) @@ -450,7 +450,7 @@ func setupAndStartServices( voiceAgent.Start(vaCtx) } - if runningServices.ListenHost == gatewayFallbackBindHost { + if isWildcardBindHost(runningServices.ListenHost) { fmt.Printf( "✓ Health endpoints available on all interfaces at port %d (/health, /ready and /reload POST)\n", cfg.Gateway.Port, diff --git a/pkg/gateway/network_policy.go b/pkg/gateway/network_policy.go index f6441861c..4e8469f57 100644 --- a/pkg/gateway/network_policy.go +++ b/pkg/gateway/network_policy.go @@ -10,11 +10,13 @@ import ( "strings" "github.com/sipeed/picoclaw/pkg/channels" + "github.com/sipeed/picoclaw/pkg/netpolicy" ) const ( gatewayDefaultLoopbackHost = "127.0.0.1" - gatewayFallbackBindHost = "0.0.0.0" + gatewayFallbackBindHostV4 = "0.0.0.0" + gatewayFallbackBindHostV6 = "::" ) type gatewayListenDecision struct { @@ -27,6 +29,14 @@ type gatewayListenDecision struct { var ( probeGatewayBind = probeTCPBind discoverGatewayCIDRs = discoverLocalInterfaceCIDRs + + fallbackPrivateCIDRs = mustParseCIDRs( + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + "100.64.0.0/10", + "fc00::/7", + ) ) func resolveGatewayListenDecision( @@ -71,7 +81,7 @@ func resolveGatewayListenDecision( } if len(discoveredCIDRs) == 0 { return nil, fmt.Errorf( - "loopback bind %s:%d failed: %w; no non-loopback interface CIDRs discovered", + "loopback bind %s:%d failed: %w; no private non-loopback interface CIDRs discovered", host, port, bindErr, @@ -87,32 +97,73 @@ func resolveGatewayListenDecision( return nil, fmt.Errorf("loopback bind %s:%d failed: %w; fallback allowlist is empty", host, port, bindErr) } - if fallbackBindErr := probeGatewayBind(gatewayFallbackBindHost, port); fallbackBindErr != nil { + fallbackCandidates := fallbackBindCandidates(host) + fallbackHost, fallbackBindErr := probeFallbackBindCandidates(fallbackCandidates, port) + if fallbackBindErr != nil { return nil, fmt.Errorf( - "loopback bind %s:%d failed: %w; fallback bind %s:%d failed: %v", + "loopback bind %s:%d failed: %w; fallback bind candidates %v failed: %v", host, port, bindErr, - gatewayFallbackBindHost, - port, + fallbackCandidates, fallbackBindErr, ) } return &gatewayListenDecision{ - BindHost: gatewayFallbackBindHost, + BindHost: fallbackHost, AllowedCIDRs: fallbackCIDRs, AutoFallback: true, FallbackReason: fmt.Sprintf( "loopback bind %s:%d failed, fallback to %s:%d with CIDR allowlist", host, port, - gatewayFallbackBindHost, + fallbackHost, port, ), }, nil } +func fallbackBindCandidates(configuredLoopbackHost string) []string { + lower := strings.TrimSpace(strings.ToLower(configuredLoopbackHost)) + if lower == "localhost" { + return []string{gatewayFallbackBindHostV6, gatewayFallbackBindHostV4} + } + + ip := net.ParseIP(lower) + if ip != nil && ip.To4() == nil { + return []string{gatewayFallbackBindHostV6, gatewayFallbackBindHostV4} + } + + return []string{gatewayFallbackBindHostV4, gatewayFallbackBindHostV6} +} + +func probeFallbackBindCandidates(candidates []string, port int) (string, error) { + errList := make([]error, 0, len(candidates)) + for _, host := range candidates { + err := probeGatewayBind(host, port) + if err == nil { + return host, nil + } + errList = append(errList, fmt.Errorf("%s:%d: %w", host, port, err)) + } + + if len(errList) == 0 { + return "", fmt.Errorf("no fallback bind candidates") + } + + return "", errors.Join(errList...) +} + +func isWildcardBindHost(host string) bool { + switch strings.TrimSpace(host) { + case gatewayFallbackBindHostV4, gatewayFallbackBindHostV6: + return true + default: + return false + } +} + func normalizeAndValidateCIDRs(cidrs []string) ([]string, error) { if len(cidrs) == 0 { return nil, nil @@ -180,6 +231,9 @@ func discoverLocalInterfaceCIDRs() ([]string, error) { continue } ip = normalizeIPForMask(ip, mask) + if !isSafeFallbackIP(ip) { + continue + } masked := ip.Mask(mask) if masked == nil { @@ -201,6 +255,30 @@ func discoverLocalInterfaceCIDRs() ([]string, error) { return out, nil } +func mustParseCIDRs(cidrs ...string) []*net.IPNet { + parsed := make([]*net.IPNet, 0, len(cidrs)) + for _, cidr := range cidrs { + _, ipNet, err := net.ParseCIDR(cidr) + if err != nil { + panic(fmt.Sprintf("invalid built-in fallback CIDR %q: %v", cidr, err)) + } + parsed = append(parsed, ipNet) + } + return parsed +} + +func isSafeFallbackIP(ip net.IP) bool { + if ip == nil { + return false + } + for _, ipNet := range fallbackPrivateCIDRs { + if ipNet.Contains(ip) { + return true + } + } + return false +} + func normalizeIPForMask(ip net.IP, mask net.IPMask) net.IP { switch len(mask) { case net.IPv4len: @@ -255,47 +333,23 @@ func isLoopbackHost(host string) bool { func newCIDRAllowlistMiddleware(allowedCIDRs []string) channels.HTTPMiddleware { effectiveCIDRs := append([]string(nil), allowedCIDRs...) return func(next http.Handler) (http.Handler, error) { - if len(effectiveCIDRs) == 0 { + allowlist, err := netpolicy.NewIPAllowlist(effectiveCIDRs) + if err != nil { + return nil, err + } + if allowlist.IsOpen() { return next, nil } - nets := make([]*net.IPNet, 0, len(effectiveCIDRs)) - for _, cidr := range effectiveCIDRs { - _, ipNet, err := net.ParseCIDR(cidr) - if err != nil { - return nil, fmt.Errorf("invalid CIDR %q: %w", cidr, err) - } - nets = append(nets, ipNet) - } - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ip := clientIPFromRemoteAddr(r.RemoteAddr) - if ip == nil { - http.Error(w, "Forbidden", http.StatusForbidden) - return - } // Loopback is always allowed for local administration. // When deployed behind a local reverse proxy/tunnel, forwarded // external traffic may still appear as loopback at this layer. - if ip.IsLoopback() { + if allowlist.AllowsRemoteAddr(r.RemoteAddr) { next.ServeHTTP(w, r) return } - for _, ipNet := range nets { - if ipNet.Contains(ip) { - next.ServeHTTP(w, r) - return - } - } http.Error(w, "Forbidden", http.StatusForbidden) }), nil } } - -func clientIPFromRemoteAddr(remoteAddr string) net.IP { - host := remoteAddr - if h, _, err := net.SplitHostPort(remoteAddr); err == nil { - host = h - } - return net.ParseIP(strings.TrimSpace(host)) -} diff --git a/pkg/gateway/network_policy_test.go b/pkg/gateway/network_policy_test.go index b8c63aa42..e803b990c 100644 --- a/pkg/gateway/network_policy_test.go +++ b/pkg/gateway/network_policy_test.go @@ -42,7 +42,7 @@ func TestResolveGatewayListenDecisionLoopbackFallbackUsesConfiguredCIDRs(t *test switch host { case "127.0.0.1": return errors.New("loopback unavailable") - case gatewayFallbackBindHost: + case gatewayFallbackBindHostV4, gatewayFallbackBindHostV6: return nil default: return nil @@ -60,8 +60,8 @@ func TestResolveGatewayListenDecisionLoopbackFallbackUsesConfiguredCIDRs(t *test if !decision.AutoFallback { t.Fatal("decision.AutoFallback = false, want true") } - if decision.BindHost != gatewayFallbackBindHost { - t.Fatalf("decision.BindHost = %q, want %q", decision.BindHost, gatewayFallbackBindHost) + if decision.BindHost != gatewayFallbackBindHostV4 { + t.Fatalf("decision.BindHost = %q, want %q", decision.BindHost, gatewayFallbackBindHostV4) } wantCIDRs := []string{"10.0.0.0/8"} if !reflect.DeepEqual(decision.AllowedCIDRs, wantCIDRs) { @@ -84,7 +84,7 @@ func TestResolveGatewayListenDecisionLoopbackFallbackUsesDiscoveredCIDRs(t *test switch host { case "localhost": return errors.New("loopback unavailable") - case gatewayFallbackBindHost: + case gatewayFallbackBindHostV4, gatewayFallbackBindHostV6: return nil default: return nil @@ -146,7 +146,7 @@ func TestResolveGatewayListenDecisionLoopbackFailureNoDiscoveredCIDRs(t *testing switch host { case "127.0.0.1": return errors.New("loopback unavailable") - case gatewayFallbackBindHost: + case gatewayFallbackBindHostV4, gatewayFallbackBindHostV6: return nil default: return nil @@ -160,11 +160,77 @@ func TestResolveGatewayListenDecisionLoopbackFailureNoDiscoveredCIDRs(t *testing if err == nil { t.Fatal("resolveGatewayListenDecision() expected error") } - if !strings.Contains(err.Error(), "no non-loopback interface CIDRs discovered") { + if !strings.Contains(err.Error(), "no private non-loopback interface CIDRs discovered") { t.Fatalf("error = %q, want no-interface-cidr failure", err.Error()) } } +func TestResolveGatewayListenDecisionLoopbackFallbackIPv6Preferred(t *testing.T) { + origProbe := probeGatewayBind + origDiscover := discoverGatewayCIDRs + t.Cleanup(func() { + probeGatewayBind = origProbe + discoverGatewayCIDRs = origDiscover + }) + + probeGatewayBind = func(host string, _ int) error { + switch host { + case "::1": + return errors.New("loopback unavailable") + case gatewayFallbackBindHostV6: + return nil + case gatewayFallbackBindHostV4: + return errors.New("should not probe IPv4 when IPv6 fallback succeeds") + default: + return nil + } + } + discoverGatewayCIDRs = func() ([]string, error) { + return []string{"192.168.1.0/24"}, nil + } + + decision, err := resolveGatewayListenDecision("::1", 18790, nil) + if err != nil { + t.Fatalf("resolveGatewayListenDecision() error = %v", err) + } + if decision.BindHost != gatewayFallbackBindHostV6 { + t.Fatalf("decision.BindHost = %q, want %q", decision.BindHost, gatewayFallbackBindHostV6) + } +} + +func TestResolveGatewayListenDecisionLoopbackFallbackTriesSecondaryWildcard(t *testing.T) { + origProbe := probeGatewayBind + origDiscover := discoverGatewayCIDRs + t.Cleanup(func() { + probeGatewayBind = origProbe + discoverGatewayCIDRs = origDiscover + }) + + probeGatewayBind = func(host string, _ int) error { + switch host { + case "::1": + return errors.New("loopback unavailable") + case gatewayFallbackBindHostV6: + return errors.New("ipv6 wildcard unavailable") + case gatewayFallbackBindHostV4: + return nil + default: + return nil + } + } + discoverGatewayCIDRs = func() ([]string, error) { + return []string{"192.168.1.0/24"}, nil + } + + decision, err := resolveGatewayListenDecision("::1", 18790, nil) + if err != nil { + t.Fatalf("resolveGatewayListenDecision() error = %v", err) + } + if decision.BindHost != gatewayFallbackBindHostV4 { + t.Fatalf("decision.BindHost = %q, want %q", decision.BindHost, gatewayFallbackBindHostV4) + } +} + func TestCIDRAllowlistMiddleware(t *testing.T) { mw := newCIDRAllowlistMiddleware([]string{"192.168.1.0/24"}) h, err := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -220,3 +286,53 @@ func TestNormalizeIPForMaskIPv4MappedIPv6(t *testing.T) { t.Fatalf("masked network = %q, want %q", got, "192.168.10.0/24") } } + +func TestFallbackBindCandidates(t *testing.T) { + tests := []struct { + name string + host string + want []string + }{ + { + name: "ipv4 loopback", + host: "127.0.0.1", + want: []string{gatewayFallbackBindHostV4, gatewayFallbackBindHostV6}, + }, + {name: "ipv6 loopback", host: "::1", want: []string{gatewayFallbackBindHostV6, gatewayFallbackBindHostV4}}, + {name: "localhost", host: "localhost", want: []string{gatewayFallbackBindHostV6, gatewayFallbackBindHostV4}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := fallbackBindCandidates(tt.host) + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("fallbackBindCandidates() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestIsSafeFallbackIP(t *testing.T) { + tests := []struct { + name string + ip string + want bool + }{ + {name: "rfc1918 a", ip: "10.1.2.3", want: true}, + {name: "rfc1918 b", ip: "172.20.1.1", want: true}, + {name: "rfc1918 c", ip: "192.168.1.1", want: true}, + {name: "cgnat", ip: "100.64.2.3", want: true}, + {name: "ipv6 ula", ip: "fd12::1", want: true}, + {name: "public ipv4", ip: "8.8.8.8", want: false}, + {name: "public ipv6", ip: "2001:4860:4860::8888", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ip := net.ParseIP(tt.ip) + if got := isSafeFallbackIP(ip); got != tt.want { + t.Fatalf("isSafeFallbackIP(%q) = %v, want %v", tt.ip, got, tt.want) + } + }) + } +} diff --git a/pkg/netpolicy/allowlist.go b/pkg/netpolicy/allowlist.go new file mode 100644 index 000000000..61c274dbe --- /dev/null +++ b/pkg/netpolicy/allowlist.go @@ -0,0 +1,81 @@ +package netpolicy + +import ( + "fmt" + "net" + "strings" +) + +// IPAllowlist evaluates whether a remote address is allowed by CIDR policy. +// Loopback addresses are always allowed for local administration. +type IPAllowlist struct { + nets []*net.IPNet +} + +// NewIPAllowlist parses CIDR rules and constructs an allowlist checker. +// Empty CIDRs means unrestricted policy. +func NewIPAllowlist(allowedCIDRs []string) (*IPAllowlist, error) { + if len(allowedCIDRs) == 0 { + return &IPAllowlist{}, nil + } + + nets := make([]*net.IPNet, 0, len(allowedCIDRs)) + for _, cidr := range allowedCIDRs { + _, ipNet, err := net.ParseCIDR(cidr) + if err != nil { + return nil, fmt.Errorf("invalid CIDR %q: %w", cidr, err) + } + nets = append(nets, ipNet) + } + + return &IPAllowlist{nets: nets}, nil +} + +// IsOpen reports whether the allowlist has no restrictions. +func (a *IPAllowlist) IsOpen() bool { + return a == nil || len(a.nets) == 0 +} + +// AllowsRemoteAddr checks whether RemoteAddr is permitted. +func (a *IPAllowlist) AllowsRemoteAddr(remoteAddr string) bool { + if a.IsOpen() { + return true + } + + ip := ClientIPFromRemoteAddr(remoteAddr) + return a.AllowsIP(ip) +} + +// AllowsIP checks whether an IP is permitted. +func (a *IPAllowlist) AllowsIP(ip net.IP) bool { + if ip == nil { + return false + } + if ip.IsLoopback() { + return true + } + if a.IsOpen() { + return true + } + + for _, ipNet := range a.nets { + if ipNet.Contains(ip) { + return true + } + } + return false +} + +// ClientIPFromRemoteAddr parses the IP component from net/http RemoteAddr. +func ClientIPFromRemoteAddr(remoteAddr string) net.IP { + host := strings.TrimSpace(remoteAddr) + if h, _, err := net.SplitHostPort(host); err == nil { + host = h + } + // Strip IPv6 zone identifier (for example: fe80::1%eth0). + if i := strings.LastIndex(host, "%"); i != -1 { + host = host[:i] + } + + return net.ParseIP(strings.TrimSpace(host)) +} diff --git a/pkg/netpolicy/allowlist_test.go b/pkg/netpolicy/allowlist_test.go new file mode 100644 index 000000000..ebfff8f81 --- /dev/null +++ b/pkg/netpolicy/allowlist_test.go @@ -0,0 +1,71 @@ +package netpolicy + +import ( + "testing" +) + +func TestIPAllowlistOpenPolicy(t *testing.T) { + allowlist, err := NewIPAllowlist(nil) + if err != nil { + t.Fatalf("NewIPAllowlist() error = %v", err) + } + if !allowlist.IsOpen() { + t.Fatal("allowlist should be open for empty CIDRs") + } + if !allowlist.AllowsRemoteAddr("203.0.113.7:1234") { + t.Fatal("open policy should allow any remote address") + } +} + +func TestIPAllowlistAllowsInsideCIDR(t *testing.T) { + allowlist, err := NewIPAllowlist([]string{"192.168.1.0/24"}) + if err != nil { + t.Fatalf("NewIPAllowlist() error = %v", err) + } + + if !allowlist.AllowsRemoteAddr("192.168.1.8:1234") { + t.Fatal("allowlist should allow address inside CIDR") + } + if allowlist.AllowsRemoteAddr("10.0.0.8:1234") { + t.Fatal("allowlist should reject address outside CIDR") + } +} + +func TestIPAllowlistAlwaysAllowsLoopback(t *testing.T) { + allowlist, err := NewIPAllowlist([]string{"192.168.1.0/24"}) + if err != nil { + t.Fatalf("NewIPAllowlist() error = %v", err) + } + + if !allowlist.AllowsRemoteAddr("127.0.0.1:1234") { + t.Fatal("loopback should always be allowed") + } +} + +func TestClientIPFromRemoteAddrIPv6Zone(t *testing.T) { + ip := ClientIPFromRemoteAddr("[fe80::1%eth0]:1234") + if ip == nil { + t.Fatal("ClientIPFromRemoteAddr() returned nil") + } + if got := ip.String(); got != "fe80::1" { + t.Fatalf("ClientIPFromRemoteAddr() = %q, want %q", got, "fe80::1") + } +} + +func TestNewIPAllowlistInvalidCIDR(t *testing.T) { + _, err := NewIPAllowlist([]string{"bad-cidr"}) + if err == nil { + t.Fatal("NewIPAllowlist() expected error for invalid CIDR") + } +} + +func TestIPAllowlistWithZoneAddressInCIDR(t *testing.T) { + allowlist, err := NewIPAllowlist([]string{"fe80::/10"}) + if err != nil { + t.Fatalf("NewIPAllowlist() error = %v", err) + } + + if !allowlist.AllowsRemoteAddr("[fe80::2%eth0]:1234") { + t.Fatal("allowlist should accept IPv6 link-local with zone") + } +} diff --git a/web/backend/middleware/access_control.go b/web/backend/middleware/access_control.go index 159d60c3e..4abe40222 100644 --- a/web/backend/middleware/access_control.go +++ b/web/backend/middleware/access_control.go @@ -1,58 +1,35 @@ package middleware import ( - "fmt" - "net" "net/http" "strings" + + "github.com/sipeed/picoclaw/pkg/netpolicy" ) // IPAllowlist restricts access to requests from configured CIDR ranges. // Loopback addresses are always allowed for local administration. // Empty CIDR list means no restriction. func IPAllowlist(allowedCIDRs []string, next http.Handler) (http.Handler, error) { - if len(allowedCIDRs) == 0 { + allowlist, err := netpolicy.NewIPAllowlist(allowedCIDRs) + if err != nil { + return nil, err + } + + if allowlist.IsOpen() { return next, nil } - nets := make([]*net.IPNet, 0, len(allowedCIDRs)) - for _, cidr := range allowedCIDRs { - _, ipNet, err := net.ParseCIDR(cidr) - if err != nil { - return nil, fmt.Errorf("invalid CIDR %q: %w", cidr, err) - } - nets = append(nets, ipNet) - } - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ip := clientIPFromRemoteAddr(r.RemoteAddr) - if ip == nil { - rejectByPolicy(w, r) - return - } - if ip.IsLoopback() { + if allowlist.AllowsRemoteAddr(r.RemoteAddr) { next.ServeHTTP(w, r) return } - for _, ipNet := range nets { - if ipNet.Contains(ip) { - next.ServeHTTP(w, r) - return - } - } rejectByPolicy(w, r) }), nil } -func clientIPFromRemoteAddr(remoteAddr string) net.IP { - host := remoteAddr - if h, _, err := net.SplitHostPort(remoteAddr); err == nil { - host = h - } - return net.ParseIP(host) -} - func rejectByPolicy(w http.ResponseWriter, r *http.Request) { if strings.HasPrefix(r.URL.Path, "/api/") { w.Header().Set("Content-Type", "application/json") diff --git a/web/backend/middleware/access_control_test.go b/web/backend/middleware/access_control_test.go index 259fd4a4c..12920fdf4 100644 --- a/web/backend/middleware/access_control_test.go +++ b/web/backend/middleware/access_control_test.go @@ -84,3 +84,21 @@ func TestIPAllowlist_InvalidCIDR(t *testing.T) { t.Fatal("IPAllowlist() expected error for invalid CIDR") } } + +func TestIPAllowlist_AllowsIPv6ZoneAddress(t *testing.T) { + h, err := IPAllowlist([]string{"fe80::/10"}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + if err != nil { + t.Fatalf("IPAllowlist() error = %v", err) + } + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "[fe80::1%eth0]:1234" + h.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } +} From a86705340f16424205d0cb5700b8663d02ebe151 Mon Sep 17 00:00:00 2001 From: Sakurapainting Date: Thu, 2 Apr 2026 19:37:29 +0800 Subject: [PATCH 5/7] fix(netpolicy): normalize allowlist CIDR inputs --- docs/configuration.md | 2 +- pkg/netpolicy/allowlist.go | 18 ++++++++++++++- pkg/netpolicy/allowlist_test.go | 39 +++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index e255b2891..617a2da7a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -62,7 +62,7 @@ When `gateway.host` is a loopback address (`127.0.0.1`, `::1`, or `localhost`) a CIDR sources in fallback mode: - If `gateway.allowed_cidrs` is configured, that list is used. -- If `gateway.allowed_cidrs` is empty, discovered private CIDRs are used (`10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`, `100.64.0.0/10`, `fc00::/7`). +- If `gateway.allowed_cidrs` is empty, PicoClaw discovers CIDR networks from local interfaces and uses only those that fall within private address ranges (for example, within `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`, `100.64.0.0/10`, `fc00::/7`). - If no private non-loopback CIDR can be discovered, gateway startup fails. On public-only hosts, configure `gateway.allowed_cidrs` explicitly. Loopback clients are always allowed for local administration. diff --git a/pkg/netpolicy/allowlist.go b/pkg/netpolicy/allowlist.go index 61c274dbe..ccc12e8d5 100644 --- a/pkg/netpolicy/allowlist.go +++ b/pkg/netpolicy/allowlist.go @@ -19,15 +19,31 @@ func NewIPAllowlist(allowedCIDRs []string) (*IPAllowlist, error) { return &IPAllowlist{}, nil } + seen := make(map[string]struct{}, len(allowedCIDRs)) nets := make([]*net.IPNet, 0, len(allowedCIDRs)) - for _, cidr := range allowedCIDRs { + for _, rawCIDR := range allowedCIDRs { + cidr := strings.TrimSpace(rawCIDR) + if cidr == "" { + continue + } + _, ipNet, err := net.ParseCIDR(cidr) if err != nil { return nil, fmt.Errorf("invalid CIDR %q: %w", cidr, err) } + + canonical := ipNet.String() + if _, ok := seen[canonical]; ok { + continue + } + seen[canonical] = struct{}{} nets = append(nets, ipNet) } + if len(nets) == 0 { + return &IPAllowlist{}, nil + } + return &IPAllowlist{nets: nets}, nil } diff --git a/pkg/netpolicy/allowlist_test.go b/pkg/netpolicy/allowlist_test.go index ebfff8f81..b360f7483 100644 --- a/pkg/netpolicy/allowlist_test.go +++ b/pkg/netpolicy/allowlist_test.go @@ -69,3 +69,42 @@ func TestIPAllowlistWithZoneAddressInCIDR(t *testing.T) { t.Fatal("allowlist should accept IPv6 link-local with zone") } } + +func TestNewIPAllowlistTrimsSkipsAndDedups(t *testing.T) { + allowlist, err := NewIPAllowlist([]string{ + " 192.168.1.8/24 ", + "", + "192.168.1.0/24", + " ", + "10.0.0.0/8", + }) + if err != nil { + t.Fatalf("NewIPAllowlist() error = %v", err) + } + if allowlist.IsOpen() { + t.Fatal("allowlist should not be open") + } + if len(allowlist.nets) != 2 { + t.Fatalf("len(allowlist.nets) = %d, want 2", len(allowlist.nets)) + } + + if !allowlist.AllowsRemoteAddr("192.168.1.22:1234") { + t.Fatal("allowlist should allow deduplicated 192.168.1.0/24 CIDR") + } + if !allowlist.AllowsRemoteAddr("10.9.8.7:1234") { + t.Fatal("allowlist should allow 10.0.0.0/8 CIDR") + } + if allowlist.AllowsRemoteAddr("203.0.113.7:1234") { + t.Fatal("allowlist should reject outside CIDR") + } +} + +func TestNewIPAllowlistAllEmptyEntries(t *testing.T) { + allowlist, err := NewIPAllowlist([]string{"", " ", "\t"}) + if err != nil { + t.Fatalf("NewIPAllowlist() error = %v", err) + } + if !allowlist.IsOpen() { + t.Fatal("allowlist should be open when all entries are empty") + } +} From bf93470977086971edc7dc8ad238842684d14787 Mon Sep 17 00:00:00 2001 From: Sakurapainting Date: Thu, 2 Apr 2026 20:02:15 +0800 Subject: [PATCH 6/7] fix(gateway): prefer loopback fallback before wildcard --- pkg/gateway/network_policy.go | 39 ++++++++++++++++ pkg/gateway/network_policy_test.go | 72 ++++++++++++++++++++++++++++++ pkg/netpolicy/allowlist.go | 2 +- web/backend/api/config.go | 19 +++++++- web/backend/api/config_test.go | 37 +++++++++++++++ 5 files changed, 167 insertions(+), 2 deletions(-) diff --git a/pkg/gateway/network_policy.go b/pkg/gateway/network_policy.go index 4e8469f57..0937f499a 100644 --- a/pkg/gateway/network_policy.go +++ b/pkg/gateway/network_policy.go @@ -66,6 +66,24 @@ func resolveGatewayListenDecision( return nil, fmt.Errorf("bind %s:%d failed: %w", host, port, bindErr) } + // Before widening exposure via wildcard bind, try other common loopback hosts. + for _, altHost := range alternativeLoopbackHosts(host) { + if err := probeGatewayBind(altHost, port); err == nil { + return &gatewayListenDecision{ + BindHost: altHost, + AllowedCIDRs: normalizedCIDRs, + AutoFallback: true, + FallbackReason: fmt.Sprintf( + "loopback bind %s:%d failed, fallback to loopback host %s:%d", + host, + port, + altHost, + port, + ), + }, nil + } + } + var discoveredCIDRs []string if len(normalizedCIDRs) == 0 { var discoverErr error @@ -124,6 +142,27 @@ func resolveGatewayListenDecision( }, nil } +func alternativeLoopbackHosts(configuredHost string) []string { + configured := strings.TrimSpace(strings.ToLower(configuredHost)) + candidates := []string{"127.0.0.1", "::1", "localhost"} + out := make([]string, 0, len(candidates)) + seen := make(map[string]struct{}, len(candidates)) + + for _, candidate := range candidates { + normalized := strings.TrimSpace(strings.ToLower(candidate)) + if normalized == configured { + continue + } + if _, ok := seen[normalized]; ok { + continue + } + seen[normalized] = struct{}{} + out = append(out, candidate) + } + + return out +} + func fallbackBindCandidates(configuredLoopbackHost string) []string { lower := strings.TrimSpace(strings.ToLower(configuredLoopbackHost)) if lower == "localhost" { diff --git a/pkg/gateway/network_policy_test.go b/pkg/gateway/network_policy_test.go index e803b990c..ed8545591 100644 --- a/pkg/gateway/network_policy_test.go +++ b/pkg/gateway/network_policy_test.go @@ -42,6 +42,8 @@ func TestResolveGatewayListenDecisionLoopbackFallbackUsesConfiguredCIDRs(t *test switch host { case "127.0.0.1": return errors.New("loopback unavailable") + case "::1", "localhost": + return errors.New("alternative loopback unavailable") case gatewayFallbackBindHostV4, gatewayFallbackBindHostV6: return nil default: @@ -84,6 +86,8 @@ func TestResolveGatewayListenDecisionLoopbackFallbackUsesDiscoveredCIDRs(t *test switch host { case "localhost": return errors.New("loopback unavailable") + case "127.0.0.1", "::1": + return errors.New("alternative loopback unavailable") case gatewayFallbackBindHostV4, gatewayFallbackBindHostV6: return nil default: @@ -146,6 +150,8 @@ func TestResolveGatewayListenDecisionLoopbackFailureNoDiscoveredCIDRs(t *testing switch host { case "127.0.0.1": return errors.New("loopback unavailable") + case "::1", "localhost": + return errors.New("alternative loopback unavailable") case gatewayFallbackBindHostV4, gatewayFallbackBindHostV6: return nil default: @@ -177,6 +183,8 @@ func TestResolveGatewayListenDecisionLoopbackFallbackIPv6Preferred(t *testing.T) switch host { case "::1": return errors.New("loopback unavailable") + case "127.0.0.1", "localhost": + return errors.New("alternative loopback unavailable") case gatewayFallbackBindHostV6: return nil case gatewayFallbackBindHostV4: @@ -210,6 +218,8 @@ func TestResolveGatewayListenDecisionLoopbackFallbackTriesSecondaryWildcard(t *t switch host { case "::1": return errors.New("loopback unavailable") + case "127.0.0.1", "localhost": + return errors.New("alternative loopback unavailable") case gatewayFallbackBindHostV6: return errors.New("ipv6 wildcard unavailable") case gatewayFallbackBindHostV4: @@ -231,6 +241,47 @@ func TestResolveGatewayListenDecisionLoopbackFallbackTriesSecondaryWildcard(t *t } } +func TestResolveGatewayListenDecisionLoopbackUsesAlternativeBeforeWildcard(t *testing.T) { + origProbe := probeGatewayBind + origDiscover := discoverGatewayCIDRs + t.Cleanup(func() { + probeGatewayBind = origProbe + discoverGatewayCIDRs = origDiscover + }) + + discoverCalled := false + probeGatewayBind = func(host string, _ int) error { + switch host { + case "127.0.0.1": + return errors.New("configured loopback unavailable") + case "::1": + return nil + case "localhost", gatewayFallbackBindHostV4, gatewayFallbackBindHostV6: + return errors.New("must not be probed after alternative loopback succeeds") + default: + return nil + } + } + discoverGatewayCIDRs = func() ([]string, error) { + discoverCalled = true + return nil, errors.New("must not be called when alternative loopback succeeds") + } + + decision, err := resolveGatewayListenDecision("127.0.0.1", 18790, nil) + if err != nil { + t.Fatalf("resolveGatewayListenDecision() error = %v", err) + } + if decision.BindHost != "::1" { + t.Fatalf("decision.BindHost = %q, want %q", decision.BindHost, "::1") + } + if len(decision.AllowedCIDRs) != 0 { + t.Fatalf("decision.AllowedCIDRs = %v, want empty", decision.AllowedCIDRs) + } + if discoverCalled { + t.Fatal("discoverGatewayCIDRs() called unexpectedly") + } +} + func TestCIDRAllowlistMiddleware(t *testing.T) { mw := newCIDRAllowlistMiddleware([]string{"192.168.1.0/24"}) h, err := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -312,6 +363,27 @@ func TestFallbackBindCandidates(t *testing.T) { } } +func TestAlternativeLoopbackHosts(t *testing.T) { + tests := []struct { + name string + host string + want []string + }{ + {name: "configured ipv4", host: "127.0.0.1", want: []string{"::1", "localhost"}}, + {name: "configured ipv6", host: "::1", want: []string{"127.0.0.1", "localhost"}}, + {name: "configured localhost", host: "localhost", want: []string{"127.0.0.1", "::1"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := alternativeLoopbackHosts(tt.host) + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("alternativeLoopbackHosts() = %v, want %v", got, tt.want) + } + }) + } +} + func TestIsSafeFallbackIP(t *testing.T) { tests := []struct { name string diff --git a/pkg/netpolicy/allowlist.go b/pkg/netpolicy/allowlist.go index ccc12e8d5..f5910e280 100644 --- a/pkg/netpolicy/allowlist.go +++ b/pkg/netpolicy/allowlist.go @@ -13,7 +13,7 @@ type IPAllowlist struct { } // NewIPAllowlist parses CIDR rules and constructs an allowlist checker. -// Empty CIDRs means unrestricted policy. +// Empty CIDR list means unrestricted policy. func NewIPAllowlist(allowedCIDRs []string) (*IPAllowlist, error) { if len(allowedCIDRs) == 0 { return &IPAllowlist{}, nil diff --git a/web/backend/api/config.go b/web/backend/api/config.go index 743954436..b1b887ec3 100644 --- a/web/backend/api/config.go +++ b/web/backend/api/config.go @@ -280,15 +280,32 @@ func validateConfig(cfg *config.Config) []string { if cfg.Gateway.Port != 0 && (cfg.Gateway.Port < 1 || cfg.Gateway.Port > 65535) { errs = append(errs, fmt.Sprintf("gateway.port %d is out of valid range (1-65535)", cfg.Gateway.Port)) } + + // Normalize and validate gateway allowed CIDRs: + // - trim whitespace + // - drop empty entries + // - canonicalize via ipNet.String() + // - deduplicate canonical CIDRs + normalizedCIDRs := make([]string, 0, len(cfg.Gateway.AllowedCIDRs)) + seenCIDRs := make(map[string]struct{}, len(cfg.Gateway.AllowedCIDRs)) for index, cidr := range cfg.Gateway.AllowedCIDRs { trimmed := strings.TrimSpace(cidr) if trimmed == "" { continue } - if _, _, err := net.ParseCIDR(trimmed); err != nil { + _, ipNet, err := net.ParseCIDR(trimmed) + if err != nil { errs = append(errs, fmt.Sprintf("gateway.allowed_cidrs[%d] is not a valid CIDR: %v", index, err)) + continue } + canonical := ipNet.String() + if _, exists := seenCIDRs[canonical]; exists { + continue + } + seenCIDRs[canonical] = struct{}{} + normalizedCIDRs = append(normalizedCIDRs, canonical) } + cfg.Gateway.AllowedCIDRs = normalizedCIDRs // Pico channel: token required when enabled if cfg.Channels.Pico.Enabled && cfg.Channels.Pico.Token.String() == "" { diff --git a/web/backend/api/config_test.go b/web/backend/api/config_test.go index 938ec5c18..8e4a66122 100644 --- a/web/backend/api/config_test.go +++ b/web/backend/api/config_test.go @@ -198,6 +198,43 @@ func TestHandlePatchConfig_RejectsInvalidGatewayAllowedCIDRs(t *testing.T) { } } +func TestHandlePatchConfig_NormalizesGatewayAllowedCIDRs(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + req := httptest.NewRequest(http.MethodPatch, "/api/config", bytes.NewBufferString(`{ + "gateway": { + "allowed_cidrs": [" 192.168.1.20/24 ", "", "192.168.1.0/24", " 10.0.0.0/8 ", " "] + } + }`)) + req.Header.Set("Content-Type", "application/json") + + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + + want := []string{"192.168.1.0/24", "10.0.0.0/8"} + if len(cfg.Gateway.AllowedCIDRs) != len(want) { + t.Fatalf("len(gateway.allowed_cidrs) = %d, want %d", len(cfg.Gateway.AllowedCIDRs), len(want)) + } + for i, cidr := range want { + if cfg.Gateway.AllowedCIDRs[i] != cidr { + t.Fatalf("gateway.allowed_cidrs[%d] = %q, want %q", i, cfg.Gateway.AllowedCIDRs[i], cidr) + } + } +} + // setupPicoEnabledEnv creates a test environment with Pico channel enabled and // its token stored only in .security.yml (not in the JSON payload). func setupPicoEnabledEnv(t *testing.T) (string, func()) { From 2cceda1ad4ace78a8ce66f4ec2aa95743c60f458 Mon Sep 17 00:00:00 2001 From: Sakurapainting Date: Thu, 2 Apr 2026 20:48:31 +0800 Subject: [PATCH 7/7] fix: update doc mention --- docs/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration.md b/docs/configuration.md index 617a2da7a..ac44bc2af 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -55,7 +55,7 @@ You can also override this with the environment variable `PICOCLAW_LOG_LEVEL`. When `gateway.host` is a loopback address (`127.0.0.1`, `::1`, or `localhost`) and bind fails (for example, on boards where loopback is unavailable), PicoClaw automatically: -1. Falls back to bind on `0.0.0.0`. +1. Falls back to bind on a wildcard address (`0.0.0.0` or `::`, depending on configuration). 2. Enforces a CIDR allowlist for gateway HTTP endpoints. 3. Discovers private local interface CIDRs only when `gateway.allowed_cidrs` is empty.