fix(xray): do not fail SyncUser over an empty WireGuard message

backend/xray/config.go

@@ -220,7 +220,8 @@ func (i *Inbound) syncUsers(users []*common.User) {
 
 	case Wireguard:
 		for _, user := range users {
-			if user.GetProxies().GetWireguard() == nil {
+			// No public key means no WireGuard account, as in setupUserAccount.
+			if user.GetProxies().GetWireguard().GetPublicKey() == "" {
 				continue
 			}
 			if slices.Contains(user.Inbounds, i.Tag) {

backend/xray/user.go

@@ -14,8 +14,10 @@ import (
 	"github.com/pasarguard/node/common"
 )
 
-func setupUserAccount(user *common.User) (api.ProxySettings, error) {
-	settings := api.ProxySettings{}
+// setupUserAccount builds the user's account for every protocol it has usable
+// credentials for. wireguardErr reports WireGuard settings that carry a key but
+// are invalid; the other accounts are still set up.
+func setupUserAccount(user *common.User) (settings api.ProxySettings, wireguardErr error) {
 	if user.GetProxies().GetVmess() != nil {
 		if vmessAccount, err := api.NewVmessAccount(user); err == nil {
 			settings.Vmess = vmessAccount
@@ -41,15 +43,17 @@ func setupUserAccount(user *common.User) (api.ProxySettings, error) {
 		settings.Hysteria = api.NewHysteriaAccount(user)
 	}
 
-	if user.GetProxies().GetWireguard() != nil {
-		wgAccount, err := api.NewWireguardAccount(user)
-		if err != nil {
-			return settings, err
+	// Panels send an empty WireGuard message for every user without WireGuard
+	// keys, so a message without a public key means no WireGuard account.
+	if user.GetProxies().GetWireguard().GetPublicKey() != "" {
+		if wgAccount, err := api.NewWireguardAccount(user); err == nil {
+			settings.Wireguard = wgAccount
+		} else {
+			wireguardErr = err
 		}
-		settings.Wireguard = wgAccount
 	}
 
-	return settings, nil
+	return settings, wireguardErr
 }
 
 func inboundFlow(inbound *Inbound) string {
@@ -230,10 +234,7 @@ func (x *Xray) SyncUser(ctx context.Context, user *common.User) error {
 		return err
 	}
 
-	proxySetting, err := setupUserAccount(user)
-	if err != nil {
-		return err
-	}
+	proxySetting, wireguardErr := setupUserAccount(user)
 
 	handler := x.handler
 	inbounds := x.config.InboundConfigs
@@ -247,6 +248,15 @@ func (x *Xray) SyncUser(ctx context.Context, user *common.User) error {
 			continue
 		}
 
+		// Invalid WireGuard settings must not drop the live peer of an inbound
+		// the user stays on; its other inbounds are still applied.
+		if wireguardErr != nil && inbound.Protocol == Wireguard && slices.Contains(userInbounds, inbound.Tag) {
+			err := fmt.Errorf("wireguard user %s on inbound %s: %w", user.GetEmail(), inbound.Tag, wireguardErr)
+			log.Println(err)
+			errMessage.WriteString("\n" + err.Error())
+			continue
+		}
+
 		account, isActive := isActiveInbound(inbound, userInbounds, proxySetting)
 		if isActive && inbound.hasAccount(account) {
 			continue
@@ -260,7 +270,7 @@ func (x *Xray) SyncUser(ctx context.Context, user *common.User) error {
 				}
 			}
 			inbound.updateUser(account)
-			err = handler.AddInboundUser(ctx, inbound.Tag, accountForAPI(inbound, account))
+			err := handler.AddInboundUser(ctx, inbound.Tag, accountForAPI(inbound, account))
 			if err != nil {
 				inbound.removeUser(user.GetEmail())
 				log.Println(err)

backend/xray/user_wireguard_test.go

@@ -6,6 +6,8 @@ import (
 	"strings"
 	"testing"
 
+	"google.golang.org/protobuf/proto"
+
 	"github.com/pasarguard/node/backend/xray/api"
 	"github.com/pasarguard/node/common"
 )
@@ -281,3 +283,155 @@ func TestSyncUserRemovesWireguardPeerWithValidIPOutsideInbound(t *testing.T) {
 		t.Fatal("peer remained cached after leaving the inbound subnet")
 	}
 }
+
+const mixedWireguardInbounds = `{"inbounds":[
+	{"tag":"vless","protocol":"vless","settings":{"clients":[],"decryption":"none"}},
+	{"tag":"wg","protocol":"wireguard","settings":{"secretKey":"","peers":[]}}
+]}`
+
+func newMixedWireguardXray(t *testing.T) (*Xray, *recordingInboundService) {
+	t.Helper()
+
+	x, service := newRecordingXray(t)
+	cfg, err := NewConfig(mixedWireguardInbounds, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	x.config = cfg
+	return x, service
+}
+
+func mixedUser(email string, wireguard *common.Wireguard, inbounds ...string) *common.User {
+	return &common.User{
+		Email:    email,
+		Inbounds: inbounds,
+		Proxies: &common.Proxy{
+			Vless:     &common.Vless{Id: "0b6f2a4e-6a54-4a51-9a3a-6f1e2b1f7c11"},
+			Wireguard: wireguard,
+		},
+	}
+}
+
+func cachedClient(x *Xray, tag, email string) (api.Account, bool) {
+	for _, inbound := range x.config.InboundConfigs {
+		if inbound.Tag == tag {
+			account, ok := inbound.clients[email]
+			return account, ok
+		}
+	}
+	return nil, false
+}
+
+func TestSetupUserAccountTreatsKeylessWireguardAsAbsent(t *testing.T) {
+	// Panels send this message (field 5, length 0) for every user without
+	// WireGuard keys, so it decodes to a non-nil but empty Wireguard.
+	emptyFromWire := &common.Proxy{}
+	if err := proto.Unmarshal([]byte{0x2a, 0x00}, emptyFromWire); err != nil {
+		t.Fatal(err)
+	}
+	if emptyFromWire.GetWireguard() == nil {
+		t.Fatal("expected a non-nil empty WireGuard message")
+	}
+
+	for _, tc := range []struct {
+		name      string
+		wireguard *common.Wireguard
+	}{
+		{"empty message from the wire", emptyFromWire.GetWireguard()},
+		{"peer IPs without a key", &common.Wireguard{PeerIps: []string{"10.0.0.2/32"}}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			settings, err := setupUserAccount(mixedUser("1.alice", tc.wireguard, "vless", "wg"))
+			if err != nil {
+				t.Fatalf("setupUserAccount() error = %v", err)
+			}
+			if settings.Vless == nil {
+				t.Fatal("expected the VLESS account to be set up")
+			}
+			if settings.Wireguard != nil {
+				t.Fatalf("expected no WireGuard account, got %#v", settings.Wireguard)
+			}
+		})
+	}
+}
+
+func TestSyncUserAppliesAUserWithAnEmptyWireguardMessage(t *testing.T) {
+	x, service := newMixedWireguardXray(t)
+	ctx := syncContext(t)
+
+	if err := x.SyncUser(ctx, mixedUser("1.alice", &common.Wireguard{}, "vless", "wg")); err != nil {
+		t.Fatal(err)
+	}
+	assertOperations(t, service.take(), operations("vless:remove", "vless:add", "wg:remove"))
+	if _, ok := cachedClient(x, "vless", "1.alice"); !ok {
+		t.Fatal("VLESS account was not stored")
+	}
+	if _, ok := cachedClient(x, "wg", "1.alice"); ok {
+		t.Fatal("a WireGuard peer was stored without keys")
+	}
+}
+
+func TestSyncUserRemovesAUserWithAnEmptyWireguardMessage(t *testing.T) {
+	x, service := newMixedWireguardXray(t)
+	ctx := syncContext(t)
+	valid := &common.Wireguard{PublicKey: wireguardKey(0x11), PeerIps: []string{"10.0.0.2/32"}}
+
+	if err := x.SyncUser(ctx, mixedUser("1.alice", valid, "vless", "wg")); err != nil {
+		t.Fatal(err)
+	}
+	service.take()
+
+	// A disabled or deleted user arrives with no inbounds and, from panels
+	// that dropped its keys, an empty WireGuard message.
+	if err := x.SyncUser(ctx, mixedUser("1.alice", &common.Wireguard{})); err != nil {
+		t.Fatal(err)
+	}
+	assertOperations(t, service.take(), operations("vless:remove", "wg:remove"))
+	for _, tag := range []string{"vless", "wg"} {
+		if _, ok := cachedClient(x, tag, "1.alice"); ok {
+			t.Fatalf("user remained cached on %s after removal", tag)
+		}
+	}
+}
+
+func TestSyncUserKeepsTheWireguardPeerOfAUserWithValidKeys(t *testing.T) {
+	x, service := newMixedWireguardXray(t)
+	ctx := syncContext(t)
+	valid := &common.Wireguard{PublicKey: wireguardKey(0x11), PeerIps: []string{"10.0.0.2/32"}}
+
+	if err := x.SyncUser(ctx, mixedUser("1.alice", valid, "vless", "wg")); err != nil {
+		t.Fatal(err)
+	}
+	assertOperations(t, service.take(), operations("vless:remove", "vless:add", "wg:add"))
+	account, ok := cachedClient(x, "wg", "1.alice")
+	if !ok || account.(*api.WireguardAccount).PublicKey != strings.Repeat("11", 32) {
+		t.Fatalf("WireGuard peer = %#v, stored = %t", account, ok)
+	}
+}
+
+func TestSyncUserAppliesOtherInboundsWhenWireguardSettingsAreInvalid(t *testing.T) {
+	x, service := newMixedWireguardXray(t)
+	ctx := syncContext(t)
+	valid := &common.Wireguard{PublicKey: wireguardKey(0x11), PeerIps: []string{"10.0.0.2/32"}}
+
+	if err := x.SyncUser(ctx, mixedUser("1.alice", valid, "wg")); err != nil {
+		t.Fatal(err)
+	}
+	service.take()
+
+	invalid := &common.Wireguard{PublicKey: "invalid-key", PeerIps: []string{"10.0.0.2/32"}}
+	if err := x.SyncUser(ctx, mixedUser("1.alice", invalid, "vless", "wg")); err == nil {
+		t.Fatal("expected invalid WireGuard account error")
+	}
+	assertOperations(t, service.take(), operations("vless:remove", "vless:add"))
+	account, ok := cachedClient(x, "wg", "1.alice")
+	if !ok || account.(*api.WireguardAccount).PublicKey != strings.Repeat("11", 32) {
+		t.Fatalf("live WireGuard peer changed after an invalid update: %#v, stored = %t", account, ok)
+	}
+
+	// Leaving every inbound removes the user, even with invalid WireGuard settings.
+	if err := x.SyncUser(ctx, mixedUser("1.alice", invalid)); err != nil {
+		t.Fatal(err)
+	}
+	assertOperations(t, service.take(), operations("vless:remove", "wg:remove"))
+}