Merge branch 'phase3-hard-cutover' (SOCKS5 hot-apply fix)

internal/xray/hot_diff.go

@@ -136,6 +136,10 @@ func diffInbounds(oldCfg, newCfg *Config, diff *HotDiff) bool {
 			logger.Debug("hot diff: inbound [", oldIb.Tag, "] is a TPROXY target; a gRPC add reports success but does not reliably bind a working listener, forcing a full restart instead of a hot swap")
 			return false
 		}
+		if exists && (inboundUsesSocksAccounts(oldIb) || inboundUsesSocksAccounts(newIb)) {
+			logger.Debug("hot diff: inbound [", oldIb.Tag, "] is a password-auth SOCKS5 inbound (e.g. internal/amneziawgnet's per-peer relay); a gRPC remove+add reports success but was observed in production to silently drop an account, forcing a full restart instead of a hot swap")
+			return false
+		}
 		diff.RemovedInboundTags = append(diff.RemovedInboundTags, oldIb.Tag)
 		if exists {
 			raw, err := json.Marshal(newIb)
@@ -157,6 +161,10 @@ func diffInbounds(oldCfg, newCfg *Config, diff *HotDiff) bool {
 			logger.Debug("hot diff: new inbound [", newIb.Tag, "] is a TPROXY target (e.g. internal/amneziawg's Xray egress bridge); a gRPC add reports success but does not reliably bind a working listener, forcing a full restart instead of a hot add")
 			return false
 		}
+		if inboundUsesSocksAccounts(newIb) {
+			logger.Debug("hot diff: new inbound [", newIb.Tag, "] is a password-auth SOCKS5 inbound (e.g. internal/amneziawgnet's per-peer relay); forcing a full restart instead of a hot add, same reasoning as the existing-inbound case above")
+			return false
+		}
 		raw, err := json.Marshal(newIb)
 		if err != nil {
 			return false
@@ -296,6 +304,41 @@ func inboundUsesTproxy(ib *InboundConfig) bool {
 	return stream.Sockopt.Tproxy != "" && stream.Sockopt.Tproxy != "off"
 }
 
+// inboundUsesSocksAccounts reports whether an inbound is a password-auth
+// SOCKS5 inbound with one or more named accounts -- the shape
+// internal/amneziawgnet's per-inbound relay (internal/web/service/xray.go's
+// injectAmneziawgnetSocks) is the only generator of in this fork; every
+// other SOCKS5 bridge this fork builds (panel egress, per-node egress,
+// mtproto egress) uses "noauth" with no per-account identity at all, so this
+// check can't accidentally rope in one of those lower-churn bridges.
+//
+// Real production incident, not a theoretical concern: a single client
+// edit under an AmneziaWG inbound left this inbound's settings unchanged in
+// every way relevant to accounts.user was already correct in the freshly
+// regenerated config, yet the account for a peer whose email contained
+// non-ASCII characters silently vanished from the running Xray process
+// after a gRPC remove+add hot swap -- while a full process restart (reading
+// the same JSON straight from disk) always produced the correct account
+// list. socks isn't in userDiffableProtocols (that only covers vless/vmess/
+// trojan, which use a wholly different clients+email shape, not
+// accounts+user), so without this check any settings drift on this inbound
+// -- even one unrelated to the account list itself -- falls through to the
+// generic remove+add path and can reproduce the same silent drop. Forcing a
+// full restart here is the same defensive choice already made above for
+// REALITY and TPROXY.
+func inboundUsesSocksAccounts(ib *InboundConfig) bool {
+	if ib == nil || ib.Protocol != "socks" || len(ib.Settings) == 0 {
+		return false
+	}
+	var settings struct {
+		Auth string `json:"auth"`
+	}
+	if err := json.Unmarshal(ib.Settings, &settings); err != nil {
+		return false
+	}
+	return settings.Auth == "password"
+}
+
 func inboundHasReverseClient(ib *InboundConfig) bool {
 	if ib == nil {
 		return false

internal/xray/hot_diff_test.go

@@ -436,3 +436,78 @@ func TestComputeHotDiff_TproxyStreamChangeNeedsRestart(t *testing.T) {
 		t.Fatal("a TPROXY bridge's port change must force a full restart, not a gRPC hot swap")
 	}
 }
+
+// TestComputeHotDiff_SocksAccountsSettingsChangeNeedsRestart reproduces a
+// real incident: editing one client under an AmneziaWG inbound left its
+// relay's settings.json byte-different (a new account list) while Xray was
+// already running. A gRPC remove+add reported success but silently dropped
+// an account with a non-ASCII email; a full restart always produced the
+// correct account list. This must force a restart, not a hot swap.
+func TestComputeHotDiff_SocksAccountsSettingsChangeNeedsRestart(t *testing.T) {
+	relayIb := InboundConfig{
+		Listen:   json_util.RawMessage(`"127.0.0.1"`),
+		Port:     65110,
+		Protocol: "socks",
+		Tag:      "in-443-udp",
+		Settings: json_util.RawMessage(`{"auth":"password","udp":true,"accounts":[{"user":"Роутер_awg","pass":"p"}]}`),
+	}
+	oldCfg := makeHotConfig()
+	oldCfg.InboundConfigs = append(oldCfg.InboundConfigs, relayIb)
+	newCfg := makeHotConfig()
+	changedIb := relayIb
+	changedIb.Settings = json_util.RawMessage(`{"auth":"password","udp":true,"accounts":[{"user":"Роутер_awg","pass":"p"},{"user":"Майфун🛟","pass":"p"}]}`)
+	newCfg.InboundConfigs = append(newCfg.InboundConfigs, changedIb)
+
+	if _, ok := ComputeHotDiff(oldCfg, newCfg); ok {
+		t.Fatal("a password-auth SOCKS5 relay's account-list change must force a full restart, not a gRPC hot swap")
+	}
+}
+
+// TestComputeHotDiff_NewSocksAccountsInboundNeedsRestart mirrors the TPROXY
+// new-inbound test above: a brand-new AmneziaWG relay inbound must also
+// force a restart, for the same reliability reason.
+func TestComputeHotDiff_NewSocksAccountsInboundNeedsRestart(t *testing.T) {
+	oldCfg := makeHotConfig()
+	newCfg := makeHotConfig()
+	newCfg.InboundConfigs = append(newCfg.InboundConfigs, InboundConfig{
+		Listen:   json_util.RawMessage(`"127.0.0.1"`),
+		Port:     65110,
+		Protocol: "socks",
+		Tag:      "in-443-udp",
+		Settings: json_util.RawMessage(`{"auth":"password","udp":true,"accounts":[{"user":"Майфун🛟","pass":"p"}]}`),
+	})
+
+	if _, ok := ComputeHotDiff(oldCfg, newCfg); ok {
+		t.Fatal("adding a new password-auth SOCKS5 relay inbound must force a full restart, not a gRPC hot add")
+	}
+}
+
+// TestComputeHotDiff_NoauthSocksBridgeStaysHot confirms the check above is
+// scoped to password-auth accounts specifically: this fork's other SOCKS5
+// bridges (panel egress, per-node egress, mtproto egress) use "noauth" with
+// no per-account identity, have no history of this failure mode, and must
+// keep using the ordinary remove+add hot path rather than pay for an
+// unnecessary restart on every port/tag change.
+func TestComputeHotDiff_NoauthSocksBridgeStaysHot(t *testing.T) {
+	bridgeIb := InboundConfig{
+		Listen:   json_util.RawMessage(`"127.0.0.1"`),
+		Port:     62790,
+		Protocol: "socks",
+		Tag:      "panel-egress",
+		Settings: json_util.RawMessage(`{"auth":"noauth","udp":false}`),
+	}
+	oldCfg := makeHotConfig()
+	oldCfg.InboundConfigs = append(oldCfg.InboundConfigs, bridgeIb)
+	newCfg := makeHotConfig()
+	changedIb := bridgeIb
+	changedIb.Port = 62791
+	newCfg.InboundConfigs = append(newCfg.InboundConfigs, changedIb)
+
+	diff, ok := ComputeHotDiff(oldCfg, newCfg)
+	if !ok {
+		t.Fatal("a noauth SOCKS5 bridge's port change should stay hot-appliable")
+	}
+	if len(diff.RemovedInboundTags) != 1 || len(diff.AddedInbounds) != 1 {
+		t.Fatalf("expected a plain remove+add for the changed bridge, got %+v", diff)
+	}
+}