From f4b7b08e08cf9624555e43a0eb6c9928dfe3903b Mon Sep 17 00:00:00 2001 From: Sanaei Date: Sat, 1 Aug 2026 15:18:50 +0200 Subject: [PATCH] fix(ldap): stop auto-delete from wiping every client on an empty directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FetchVlessFlags returns (empty map, nil) whenever the bind succeeds but the search yields nothing usable — a renamed OU, a service account that lost read on the user attribute, a filter that stopped matching. The only guard on the destructive half of the sync was `err != nil`, so that answer was read as "every user is gone" and the job detached every client from the configured inbounds, once a minute, for as long as the directory stayed broken. Gate auto-delete behind autoDeleteSafeForFetch: refuse an empty fetch, and refuse one that collapsed below half of the last successful sync, which is a misconfigured directory far more often than real churn. Also stop splitCsv from defaulting an empty string to DefaultTruthyValues. That default belongs to the truthy-value setting, but splitCsv is also what parses ldapInboundTags, so an unconfigured tag list silently resolved to ["true","1","yes","on"]. It only ever bounded the blast radius by accident. --- .../web/job/ldap_auto_delete_guard_test.go | 34 ++++++++++++++++++ internal/web/job/ldap_sync_job.go | 36 +++++++++++++++++-- 2 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 internal/web/job/ldap_auto_delete_guard_test.go diff --git a/internal/web/job/ldap_auto_delete_guard_test.go b/internal/web/job/ldap_auto_delete_guard_test.go new file mode 100644 index 000000000..c4846a262 --- /dev/null +++ b/internal/web/job/ldap_auto_delete_guard_test.go @@ -0,0 +1,34 @@ +package job + +import "testing" + +// A bind that succeeds with an unusable search returns (empty, nil); the only +// guard was `err != nil`, so that answer detached the entire inbound. +func TestAutoDeleteAllowed(t *testing.T) { + cases := []struct { + name string + previous int64 + fetched int + want bool + }{ + {"empty fetch on a fresh job is refused", 0, 0, false}, + {"empty fetch after a healthy sync is refused", 500, 0, false}, + {"full fetch on a fresh job is allowed", 0, 500, true}, + {"steady fetch is allowed", 500, 500, true}, + {"growth is allowed", 500, 900, true}, + {"shrink above the retention floor is allowed", 500, 250, true}, + {"shrink below the retention floor is refused", 500, 249, false}, + {"collapse to a single user is refused", 500, 1, false}, + {"single user with no history is allowed", 0, 1, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + j := NewLdapSyncJob() + j.lastFlagCount.Store(c.previous) + if got := j.autoDeleteSafeForFetch(c.fetched); got != c.want { + t.Fatalf("autoDeleteSafeForFetch(previous=%d, fetched=%d) = %v, want %v", + c.previous, c.fetched, got, c.want) + } + }) + } +} diff --git a/internal/web/job/ldap_sync_job.go b/internal/web/job/ldap_sync_job.go index 25789624e..b5ad7eec3 100644 --- a/internal/web/job/ldap_sync_job.go +++ b/internal/web/job/ldap_sync_job.go @@ -2,6 +2,7 @@ package job import ( "strings" + "sync/atomic" "time" "github.com/mhsanaei/3x-ui/v3/internal/database/model" @@ -12,11 +13,16 @@ import ( var DefaultTruthyValues = []string{"true", "1", "yes", "on"} +// Share of the previous successful fetch a new one must still return to be +// trusted: a sudden collapse means a broken directory far more often than churn. +const ldapAutoDeleteMinRetainPercent = 50 + type LdapSyncJob struct { settingService service.SettingService inboundService service.InboundService clientService service.ClientService xrayService service.XrayService + lastFlagCount atomic.Int64 } // --- Helper functions for mustGet --- @@ -77,7 +83,7 @@ func (j *LdapSyncJob) Run() { UserFilter: mustGetString(j.settingService.GetLdapUserFilter), UserAttr: mustGetString(j.settingService.GetLdapUserAttr), FlagField: mustGetStringOr(j.settingService.GetLdapFlagField, mustGetString(j.settingService.GetLdapVlessField)), - TruthyVals: splitCsv(mustGetString(j.settingService.GetLdapTruthyValues)), + TruthyVals: truthyValuesOrDefault(mustGetString(j.settingService.GetLdapTruthyValues)), Invert: mustGetBool(j.settingService.GetLdapInvertFlag), } @@ -157,7 +163,7 @@ func (j *LdapSyncJob) Run() { // --- Auto delete clients not in LDAP --- autoDelete := mustGetBool(j.settingService.GetLdapAutoDelete) - if autoDelete { + if autoDelete && j.autoDeleteSafeForFetch(len(flags)) { ldapEmailSet := map[string]struct{}{} for e := range flags { ldapEmailSet[e] = struct{}{} @@ -166,11 +172,35 @@ func (j *LdapSyncJob) Run() { j.deleteClientsNotInLDAP(tag, ldapEmailSet) } } + j.lastFlagCount.Store(int64(len(flags))) +} + +// FetchVlessFlags returns (empty, nil) when the bind succeeds but the search +// yields nothing — a renamed OU, a lost read grant — which is not "all gone". +func (j *LdapSyncJob) autoDeleteSafeForFetch(fetched int) bool { + if fetched == 0 { + logger.Warning("LDAP auto-delete skipped: directory returned no usable users") + return false + } + previous := j.lastFlagCount.Load() + if previous > 0 && int64(fetched)*100 < previous*ldapAutoDeleteMinRetainPercent { + logger.Warningf("LDAP auto-delete skipped: fetched %d users, previous successful sync saw %d (below %d%% retention)", + fetched, previous, ldapAutoDeleteMinRetainPercent) + return false + } + return true +} + +func truthyValuesOrDefault(s string) []string { + if vals := splitCsv(s); len(vals) > 0 { + return vals + } + return DefaultTruthyValues } func splitCsv(s string) []string { if s == "" { - return DefaultTruthyValues + return nil } parts := strings.Split(s, ",") out := make([]string, 0, len(parts))