From 7291bbb723c2a0230984fd3c5dbb9f98189a113b Mon Sep 17 00:00:00 2001 From: T3ST3ST3R0N Date: Wed, 7 Oct 2026 04:08:04 +0330 Subject: [PATCH 1/2] fix(wireguard): remove the peer when a user loses WireGuard access A partial sync (SyncUser / UpdateUsers / chunked updates) only marked as "touched" the users that survived normalizeUsers, which drops users with empty peer_ips or no WireGuard credentials. The panel sends exactly that shape when it releases a user's WireGuard IPs (for example after the user left every WireGuard group), so the user's existing peer was never removed and kept working until the next Start or full sync. The freed IP could also be handed to another user while the old peer still held it in the peer store. Every user with an email in the request now counts as touched; only the normalized users become desired peers, so the stale peer is diffed out and removed from the device and the peer store. --- backend/wireguard/user_partial_sync.go | 11 +++-- backend/wireguard/user_sync_test.go | 62 ++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/backend/wireguard/user_partial_sync.go b/backend/wireguard/user_partial_sync.go index 656c406a..db41182a 100644 --- a/backend/wireguard/user_partial_sync.go +++ b/backend/wireguard/user_partial_sync.go @@ -21,9 +21,14 @@ func (wg *WireGuard) buildExistingPeersSubsetForTouched(touchedEmails map[string func (wg *WireGuard) syncUsersPartialReconcile(users []*common.User) error { normalizedUsers := normalizeUsers(users) - touchedEmails := make(map[string]struct{}, len(normalizedUsers)) - for _, user := range normalizedUsers { - touchedEmails[user.GetEmail()] = struct{}{} + // Every user in the request is touched, including ones normalizeUsers drops (empty + // peer_ips or no WireGuard credentials): the panel sends those when a user loses + // WireGuard access, and their existing peer must be removed. + touchedEmails := make(map[string]struct{}, len(users)) + for _, user := range users { + if email := user.GetEmail(); email != "" { + touchedEmails[email] = struct{}{} + } } existingSubset := wg.buildExistingPeersSubsetForTouched(touchedEmails) diff --git a/backend/wireguard/user_sync_test.go b/backend/wireguard/user_sync_test.go index 69b29507..f06ccd9f 100644 --- a/backend/wireguard/user_sync_test.go +++ b/backend/wireguard/user_sync_test.go @@ -900,3 +900,65 @@ func mustPeerInfo(email, pubStr string, ips []string) *PeerInfo { AllowedIPs: parsedIPs, } } + +// The panel releases a user's WireGuard IPs (e.g. the user left every WG group) and then +// pushes the user with empty peer_ips or without WireGuard credentials. The stored peer must +// be removed even though the user no longer qualifies as a desired peer. +func TestUpdateUsersRemovesPeerWhenUserLosesWireguardAccess(t *testing.T) { + cases := map[string]*common.Proxy{ + "empty peer_ips": {Wireguard: &common.Wireguard{PeerIps: []string{}}}, + "no wireguard": {}, + } + for name, proxies := range cases { + t.Run(name, func(t *testing.T) { + cfg, err := NewConfig(`{ + "interface_name":"wg-test", + "listen_port":51820, + "address":["10.72.0.1/24"] + }`) + if err != nil { + t.Fatalf("failed to create config: %v", err) + } + + _, key, err := GenerateKeyPair() + if err != nil { + t.Fatalf("failed to generate key: %v", err) + } + if proxies.Wireguard != nil { + proxies.Wireguard.PublicKey = key + } + + ps := NewPeerStore() + ps.ReplaceAll([]*PeerInfo{mustPeerInfo("gone@example.com", key, []string{"10.72.0.2/32"})}) + + var applied []wgtypes.PeerConfig + wg := &WireGuard{ + config: cfg, + peerStore: ps, + statsTracker: stats.New(), + manager: &Manager{ + iFaceName: "wg-test", + client: &fakeWGClient{ + configureDeviceFn: func(interfaceName string, cfg wgtypes.Config) error { + applied = append(applied, cfg.Peers...) + return nil + }, + }, + }, + state: lifecycleRunning, + } + + users := []*common.User{{Email: "gone@example.com", Inbounds: []string{}, Proxies: proxies}} + if err := wg.UpdateUsers(context.Background(), users); err != nil { + t.Fatalf("UpdateUsers failed: %v", err) + } + + if len(applied) != 1 || !applied[0].Remove || applied[0].PublicKey.String() != key { + t.Fatalf("expected exactly one Remove for the stale peer, got %+v", applied) + } + if ps.GetByEmail("gone@example.com") != nil { + t.Fatal("expected the stale peer to be dropped from the peer store") + } + }) + } +} From fb0e71a810a39a6fb413f36ef2a47e15db4575c6 Mon Sep 17 00:00:00 2001 From: T3ST3ST3R0N Date: Wed, 7 Oct 2026 05:34:21 +0330 Subject: [PATCH 2/2] test(wireguard): cover stats cleanup and key handover when a user loses access The stale peer's stats entry must be marked deleted, and a key can move from a user sent with empty peer_ips to another user in the same batch. The handover test fails without the previous commit ("already assigned to user"). --- backend/wireguard/user_sync_test.go | 60 +++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/backend/wireguard/user_sync_test.go b/backend/wireguard/user_sync_test.go index f06ccd9f..4c8d9133 100644 --- a/backend/wireguard/user_sync_test.go +++ b/backend/wireguard/user_sync_test.go @@ -948,6 +948,8 @@ func TestUpdateUsersRemovesPeerWhenUserLosesWireguardAccess(t *testing.T) { state: lifecycleRunning, } + wg.statsTracker.UpdateStatsBatch([]stats.Sample{{PublicKey: key, Email: "gone@example.com", Rx: 10, Tx: 5}}) + users := []*common.User{{Email: "gone@example.com", Inbounds: []string{}, Proxies: proxies}} if err := wg.UpdateUsers(context.Background(), users); err != nil { t.Fatalf("UpdateUsers failed: %v", err) @@ -959,6 +961,64 @@ func TestUpdateUsersRemovesPeerWhenUserLosesWireguardAccess(t *testing.T) { if ps.GetByEmail("gone@example.com") != nil { t.Fatal("expected the stale peer to be dropped from the peer store") } + if entry := wg.statsTracker.GetStatsEntries([]string{key})[key]; entry == nil || !entry.IsDeleted { + t.Fatalf("expected the stale peer's stats entry to be marked deleted, got %+v", entry) + } }) } } + +// A key moves from a user who lost WireGuard access (sent with empty peer_ips) to another user in the +// same batch. The former owner is touched, so the handover is allowed instead of failing the batch. +func TestUpdateUsersAllowsKeyHandoverFromUserWhoLostAccess(t *testing.T) { + cfg, err := NewConfig(`{ + "interface_name":"wg-test", + "listen_port":51820, + "address":["10.73.0.1/24"] + }`) + if err != nil { + t.Fatalf("failed to create config: %v", err) + } + + _, key, err := GenerateKeyPair() + if err != nil { + t.Fatalf("failed to generate key: %v", err) + } + + ps := NewPeerStore() + ps.ReplaceAll([]*PeerInfo{mustPeerInfo("old@example.com", key, []string{"10.73.0.2/32"})}) + + wg := &WireGuard{ + config: cfg, + peerStore: ps, + statsTracker: stats.New(), + manager: &Manager{ + iFaceName: "wg-test", + client: &fakeWGClient{ + configureDeviceFn: func(interfaceName string, cfg wgtypes.Config) error { return nil }, + }, + }, + state: lifecycleRunning, + } + + users := []*common.User{ + {Email: "old@example.com", Proxies: &common.Proxy{Wireguard: &common.Wireguard{PublicKey: key}}}, + { + Email: "new@example.com", + Inbounds: []string{"wg-test"}, + Proxies: &common.Proxy{ + Wireguard: &common.Wireguard{PublicKey: key, PeerIps: []string{"10.73.0.3/32"}}, + }, + }, + } + if err := wg.UpdateUsers(context.Background(), users); err != nil { + t.Fatalf("UpdateUsers failed: %v", err) + } + + if peer := ps.GetByKey(key); peer == nil || peer.Email != "new@example.com" { + t.Fatalf("expected the key to belong to new@example.com, got %+v", peer) + } + if ps.GetByEmail("old@example.com") != nil { + t.Fatal("expected the former owner to have no peer") + } +}