From 42ece2aceb9d351e426d2ec828b17616278f39ae Mon Sep 17 00:00:00 2001 From: LC Date: Mon, 15 Jun 2026 16:16:25 +0800 Subject: [PATCH] fix(web): improve launcher allowlist bypass diagnostics (#3126) --- web/backend/launcherconfig/config.go | 42 ++++- web/backend/launcherconfig/config_test.go | 64 +++++++- web/backend/main.go | 67 ++++++++ web/backend/main_test.go | 181 ++++++++++++++++++++++ 4 files changed, 347 insertions(+), 7 deletions(-) diff --git a/web/backend/launcherconfig/config.go b/web/backend/launcherconfig/config.go index 3873af4fa..692136f8b 100644 --- a/web/backend/launcherconfig/config.go +++ b/web/backend/launcherconfig/config.go @@ -20,17 +20,28 @@ const ( // Config stores launch parameters for the web backend service. type Config struct { - Port int `json:"port"` - Public bool `json:"public"` - AllowedCIDRs []string `json:"allowed_cidrs,omitempty"` - AllowLocalhostBypass bool `json:"allow_localhost_bypass"` - TrustedProxyCIDRs []string `json:"trusted_proxy_cidrs,omitempty"` - DashboardPasswordHash string `json:"dashboard_password_hash,omitempty"` + Port int `json:"port"` + Public bool `json:"public"` + AllowedCIDRs []string `json:"allowed_cidrs,omitempty"` + AllowLocalhostBypass bool `json:"allow_localhost_bypass"` + AllowLocalhostBypassSource BoolFieldSource `json:"-"` + TrustedProxyCIDRs []string `json:"trusted_proxy_cidrs,omitempty"` + DashboardPasswordHash string `json:"dashboard_password_hash,omitempty"` // LegacyLauncherToken is read only for one-time migration from the removed // token login flow. Save always clears it so new configs do not persist it. LegacyLauncherToken string `json:"launcher_token,omitempty"` } +// BoolFieldSource tracks whether a JSON boolean field was omitted, explicitly +// provided, or explicitly set to null. This is only used for diagnostics. +type BoolFieldSource uint8 + +const ( + BoolFieldAbsent BoolFieldSource = iota + BoolFieldPresent + BoolFieldNull +) + // Default returns default launcher settings. func Default() Config { return Config{Port: DefaultPort, Public: false, AllowLocalhostBypass: true} @@ -98,6 +109,7 @@ func Load(path string, fallback Config) (Config, error) { } cfg := fallback + cfg.AllowLocalhostBypassSource = detectBoolFieldSource(data, "allow_localhost_bypass") if err := json.Unmarshal(data, &cfg); err != nil { return Config{}, err } @@ -111,6 +123,24 @@ func Load(path string, fallback Config) (Config, error) { return cfg, nil } +func detectBoolFieldSource(data []byte, field string) BoolFieldSource { + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return BoolFieldAbsent + } + + value, ok := raw[field] + if !ok { + return BoolFieldAbsent + } + + if string(value) == "null" { + return BoolFieldNull + } + + return BoolFieldPresent +} + // Save writes launcher settings to disk. func Save(path string, cfg Config) error { cfg.AllowedCIDRs = NormalizeCIDRs(cfg.AllowedCIDRs) diff --git a/web/backend/launcherconfig/config_test.go b/web/backend/launcherconfig/config_test.go index 83dbea37e..090aaa6f0 100644 --- a/web/backend/launcherconfig/config_test.go +++ b/web/backend/launcherconfig/config_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "runtime" "testing" ) @@ -78,7 +79,11 @@ func TestSaveAndLoadRoundTrip(t *testing.T) { if err != nil { t.Fatalf("Stat() error = %v", err) } - if perm := stat.Mode().Perm(); perm != 0o600 { + if perm := stat.Mode().Perm(); runtime.GOOS == "windows" { + if perm&0o200 == 0 { + t.Fatalf("file perm = %o, want owner-writable on Windows", perm) + } + } else if perm != 0o600 { t.Fatalf("file perm = %o, want 600", perm) } } @@ -111,6 +116,63 @@ func TestLoadDefaultsAllowLocalhostBypassForLegacyConfig(t *testing.T) { if !got.AllowLocalhostBypass { t.Fatal("allow_localhost_bypass = false, want true for legacy config") } + if got.AllowLocalhostBypassSource != BoolFieldAbsent { + t.Fatalf("allow_localhost_bypass source = %v, want %v", got.AllowLocalhostBypassSource, BoolFieldAbsent) + } +} + +func TestLoadTracksAllowLocalhostBypassSource(t *testing.T) { + tests := []struct { + name string + body string + wantValue bool + wantSource BoolFieldSource + }{ + { + name: "explicit true", + body: `{"port":18800,"allow_localhost_bypass":true}`, + wantValue: true, + wantSource: BoolFieldPresent, + }, + { + name: "explicit false", + body: `{"port":18800,"allow_localhost_bypass":false}`, + wantValue: false, + wantSource: BoolFieldPresent, + }, + { + name: "explicit null keeps fallback behavior", + body: `{"port":18800,"allow_localhost_bypass":null}`, + wantValue: true, + wantSource: BoolFieldNull, + }, + { + name: "omitted uses fallback", + body: `{"port":18800}`, + wantValue: true, + wantSource: BoolFieldAbsent, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "launcher-config.json") + if err := os.WriteFile(path, []byte(tt.body), 0o600); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + got, err := Load(path, Default()) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if got.AllowLocalhostBypass != tt.wantValue { + t.Fatalf("allow_localhost_bypass = %t, want %t", got.AllowLocalhostBypass, tt.wantValue) + } + if got.AllowLocalhostBypassSource != tt.wantSource { + t.Fatalf("allow_localhost_bypass source = %v, want %v", got.AllowLocalhostBypassSource, tt.wantSource) + } + }) + } } func TestValidateRejectsInvalidPort(t *testing.T) { diff --git a/web/backend/main.go b/web/backend/main.go index 400902c56..698e582e4 100644 --- a/web/backend/main.go +++ b/web/backend/main.go @@ -339,6 +339,64 @@ func firstNonEmpty(values ...string) string { return "" } +type launcherAllowlistBypassLogDecision struct { + level logger.LogLevel + message string + emit bool +} + +func launcherBindMayExposeBeyondLoopback(hostInput string, public bool) bool { + normalizedHostInput := strings.TrimSpace(hostInput) + if normalizedHostInput == "" { + return public + } + + for token := range strings.SplitSeq(normalizedHostInput, ",") { + token = strings.TrimSpace(token) + if token == "" { + continue + } + if token == "*" || netbind.IsUnspecifiedHost(token) { + return true + } + if strings.EqualFold(token, "localhost") || netbind.IsLoopbackHost(token) { + continue + } + return true + } + + return false +} + +func launcherAllowlistBypassLogPolicy( + hostInput string, + public bool, + cfg launcherconfig.Config, +) launcherAllowlistBypassLogDecision { + if !launcherBindMayExposeBeyondLoopback(hostInput, public) || len(cfg.AllowedCIDRs) == 0 { + return launcherAllowlistBypassLogDecision{} + } + + switch cfg.AllowLocalhostBypassSource { + case launcherconfig.BoolFieldPresent: + if cfg.AllowLocalhostBypass { + return launcherAllowlistBypassLogDecision{ + level: logger.INFO, + emit: true, + message: "Launcher public access uses allowed_cidrs with allow_localhost_bypass=true; same-host proxies or tunnels can bypass CIDR restrictions", + } + } + case launcherconfig.BoolFieldNull: + return launcherAllowlistBypassLogDecision{ + level: logger.WARN, + emit: true, + message: "Launcher public access uses allowed_cidrs with allow_localhost_bypass=null; default localhost bypass remains enabled, so same-host proxies or tunnels can bypass CIDR restrictions", + } + } + + return launcherAllowlistBypassLogDecision{} +} + func main() { port := flag.String("port", "18800", "Port to listen on") host := flag.String("host", "", "Host to listen on (overrides -public when set)") @@ -491,6 +549,15 @@ func main() { logger.InfoC("web", "Ignoring -public because launcher host was explicitly set") } + if decision := launcherAllowlistBypassLogPolicy(hostInput, effectivePublic, launcherCfg); decision.emit { + switch decision.level { + case logger.WARN: + logger.WarnC("web", decision.message) + default: + logger.InfoC("web", decision.message) + } + } + portNum, err := strconv.Atoi(effectivePort) if err != nil || portNum < 1 || portNum > 65535 { if err == nil { diff --git a/web/backend/main_test.go b/web/backend/main_test.go index aea02927e..5795a46e6 100644 --- a/web/backend/main_test.go +++ b/web/backend/main_test.go @@ -11,7 +11,9 @@ import ( "testing" "time" + "github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/netbind" + "github.com/sipeed/picoclaw/web/backend/launcherconfig" "github.com/sipeed/picoclaw/web/backend/middleware" ) @@ -132,6 +134,185 @@ func TestResolveLauncherHostInput(t *testing.T) { } } +func TestLauncherAllowlistBypassLogPolicy(t *testing.T) { + tests := []struct { + name string + hostInput string + public bool + cfg launcherconfig.Config + wantEmit bool + wantLevel logger.LogLevel + wantMessage string + }{ + { + name: "explicit true logs info", + public: true, + cfg: launcherconfig.Config{ + AllowedCIDRs: []string{"192.168.1.0/24"}, + AllowLocalhostBypass: true, + AllowLocalhostBypassSource: launcherconfig.BoolFieldPresent, + }, + wantEmit: true, + wantLevel: logger.INFO, + wantMessage: "Launcher public access uses allowed_cidrs with allow_localhost_bypass=true; same-host proxies or tunnels can bypass CIDR restrictions", + }, + { + name: "explicit null logs warn", + public: true, + cfg: launcherconfig.Config{ + AllowedCIDRs: []string{"192.168.1.0/24"}, + AllowLocalhostBypass: true, + AllowLocalhostBypassSource: launcherconfig.BoolFieldNull, + }, + wantEmit: true, + wantLevel: logger.WARN, + wantMessage: "Launcher public access uses allowed_cidrs with allow_localhost_bypass=null; default localhost bypass remains enabled, so same-host proxies or tunnels can bypass CIDR restrictions", + }, + { + name: "omitted field does not log", + public: true, + cfg: launcherconfig.Config{ + AllowedCIDRs: []string{"192.168.1.0/24"}, + AllowLocalhostBypass: true, + AllowLocalhostBypassSource: launcherconfig.BoolFieldAbsent, + }, + wantEmit: false, + }, + { + name: "not public does not log", + public: false, + cfg: launcherconfig.Config{ + AllowedCIDRs: []string{"192.168.1.0/24"}, + AllowLocalhostBypass: true, + AllowLocalhostBypassSource: launcherconfig.BoolFieldPresent, + }, + wantEmit: false, + }, + { + name: "empty cidrs does not log", + public: true, + cfg: launcherconfig.Config{ + AllowLocalhostBypass: true, + AllowLocalhostBypassSource: launcherconfig.BoolFieldPresent, + }, + wantEmit: false, + }, + { + name: "explicit false does not log", + public: true, + cfg: launcherconfig.Config{ + AllowedCIDRs: []string{"192.168.1.0/24"}, + AllowLocalhostBypass: false, + AllowLocalhostBypassSource: launcherconfig.BoolFieldPresent, + }, + wantEmit: false, + }, + { + name: "explicit any-host override still logs", + hostInput: "0.0.0.0", + public: false, + cfg: launcherconfig.Config{ + AllowedCIDRs: []string{"192.168.1.0/24"}, + AllowLocalhostBypass: true, + AllowLocalhostBypassSource: launcherconfig.BoolFieldPresent, + }, + wantEmit: true, + wantLevel: logger.INFO, + wantMessage: "Launcher public access uses allowed_cidrs with allow_localhost_bypass=true; same-host proxies or tunnels can bypass CIDR restrictions", + }, + { + name: "explicit ipv6 any-host override still logs", + hostInput: "::", + public: false, + cfg: launcherconfig.Config{ + AllowedCIDRs: []string{"192.168.1.0/24"}, + AllowLocalhostBypass: true, + AllowLocalhostBypassSource: launcherconfig.BoolFieldPresent, + }, + wantEmit: true, + wantLevel: logger.INFO, + wantMessage: "Launcher public access uses allowed_cidrs with allow_localhost_bypass=true; same-host proxies or tunnels can bypass CIDR restrictions", + }, + { + name: "explicit hostname override logs", + hostInput: "example.com", + public: false, + cfg: launcherconfig.Config{ + AllowedCIDRs: []string{"192.168.1.0/24"}, + AllowLocalhostBypass: true, + AllowLocalhostBypassSource: launcherconfig.BoolFieldPresent, + }, + wantEmit: true, + wantLevel: logger.INFO, + wantMessage: "Launcher public access uses allowed_cidrs with allow_localhost_bypass=true; same-host proxies or tunnels can bypass CIDR restrictions", + }, + { + name: "explicit loopback override does not log", + hostInput: "127.0.0.1", + public: false, + cfg: launcherconfig.Config{ + AllowedCIDRs: []string{"192.168.1.0/24"}, + AllowLocalhostBypass: true, + AllowLocalhostBypassSource: launcherconfig.BoolFieldPresent, + }, + wantEmit: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := launcherAllowlistBypassLogPolicy(tt.hostInput, tt.public, tt.cfg) + if got.emit != tt.wantEmit { + t.Fatalf("emit = %t, want %t", got.emit, tt.wantEmit) + } + if !tt.wantEmit { + return + } + if got.level != tt.wantLevel { + t.Fatalf("level = %v, want %v", got.level, tt.wantLevel) + } + if got.message != tt.wantMessage { + t.Fatalf("message = %q, want %q", got.message, tt.wantMessage) + } + }) + } +} + +func TestLauncherBindMayExposeBeyondLoopback(t *testing.T) { + tests := []struct { + name string + hostInput string + public bool + want bool + }{ + {name: "default loopback bind", public: false, want: false}, + {name: "default public bind", public: true, want: true}, + {name: "localhost override", hostInput: "localhost", want: false}, + {name: "ipv4 loopback override", hostInput: "127.0.0.1", want: false}, + {name: "ipv6 loopback override", hostInput: "::1", want: false}, + {name: "ipv4 wildcard override", hostInput: "0.0.0.0", want: true}, + {name: "ipv6 wildcard override", hostInput: "::", want: true}, + {name: "star override", hostInput: "*", want: true}, + {name: "hostname override", hostInput: "example.com", want: true}, + {name: "lan ip override", hostInput: "192.168.1.2", want: true}, + {name: "mixed hosts with non-loopback", hostInput: "127.0.0.1,192.168.1.2", want: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := launcherBindMayExposeBeyondLoopback(tt.hostInput, tt.public); got != tt.want { + t.Fatalf( + "launcherBindMayExposeBeyondLoopback(%q, %t) = %t, want %t", + tt.hostInput, + tt.public, + got, + tt.want, + ) + } + }) + } +} + func TestLauncherConsoleHosts(t *testing.T) { t.Run("default loopback shows localhost only", func(t *testing.T) { hosts := launcherConsoleHostsWithLocalAddrs(