diff --git a/frontend/src/pages/clients/ClientFormModal.tsx b/frontend/src/pages/clients/ClientFormModal.tsx index d457215b9..daab62e8a 100644 --- a/frontend/src/pages/clients/ClientFormModal.tsx +++ b/frontend/src/pages/clients/ClientFormModal.tsx @@ -151,11 +151,6 @@ export function gbToBytes(gb: number): number { return Math.round(gb * 1024 * 1024 * 1024); } -// The quota field displays whole/2-decimal GB, so a byte total that isn't -// aligned to 0.01 GB (e.g. one set via the API or an import) would drift on a -// save that never touched the field. Keep the original byte total when the -// displayed GB still matches it, and only re-derive from GB when the user -// actually changed the value. export function resolveTotalBytes(originalBytes: number | null | undefined, displayedGB: number): number { if (originalBytes != null && displayedGB === bytesToGB(originalBytes)) { return originalBytes; diff --git a/frontend/src/test/outbound-link-parser.test.ts b/frontend/src/test/outbound-link-parser.test.ts index 3ab512b87..44f993527 100644 --- a/frontend/src/test/outbound-link-parser.test.ts +++ b/frontend/src/test/outbound-link-parser.test.ts @@ -252,8 +252,6 @@ describe('parseShadowsocksLink', () => { }); it('decodes URL-safe base64 userinfo (as the emitter writes it)', () => { - // genShadowsocksLink emits Base64.encode(method:password, true) — URL-safe, - // so a password whose encoding contains - or _ must still round-trip. const method = 'aes-256-gcm'; const password = '>>>'; const userinfo = Base64.encode(`${method}:${password}`, true); diff --git a/internal/database/seeder_fastpath_test.go b/internal/database/seeder_fastpath_test.go index fca74a332..13853534b 100644 --- a/internal/database/seeder_fastpath_test.go +++ b/internal/database/seeder_fastpath_test.go @@ -7,11 +7,6 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/database/model" ) -// On a fresh install the fast path marks the one-time migration seeders as done -// without running them. ResetIpLimitNoFail2ban must be in that set: otherwise it -// is skipped on the first boot and runs on the second, where — on a host without -// fail2ban — it destructively zeroes every client's limitIp, including limits the -// admin configured between the two boots. func TestFreshInstallFastPathMarksResetIpLimitSeeder(t *testing.T) { if err := InitDB(filepath.Join(t.TempDir(), "x-ui.db")); err != nil { t.Fatalf("InitDB: %v", err) diff --git a/internal/eventbus/bus.go b/internal/eventbus/bus.go index b9cd34c9a..20627221f 100644 --- a/internal/eventbus/bus.go +++ b/internal/eventbus/bus.go @@ -29,11 +29,12 @@ type subscriber struct { // Producers call Publish (non-blocking) and every event is fanned out to all // subscribers; per-event filtering is the subscriber's responsibility. type Bus struct { - ch chan Event - subs []*subscriber - mu sync.RWMutex - done chan struct{} - wg sync.WaitGroup + ch chan Event + subs []*subscriber + mu sync.RWMutex + done chan struct{} + wg sync.WaitGroup + stopped bool } // New creates a Bus with the given buffer size. Use 0 for DefaultBufferSize. @@ -57,6 +58,9 @@ func New(bufSize int) *Bus { func (b *Bus) Subscribe(id string, handler func(Event)) { b.mu.Lock() defer b.mu.Unlock() + if b.stopped { + return + } for i, s := range b.subs { if s.id == id { close(s.quit) @@ -159,8 +163,13 @@ func safeCall(fn func(Event), e Event) { // Stop shuts down the bus: the dispatch loop and every subscriber worker exit // after finishing any handler already in progress, and any events still buffered -// or queued may be dropped. Safe to call once. +// or queued may be dropped. Safe to call once. After Stop returns, Subscribe is +// a no-op — this also keeps Subscribe's wg.Add from ever racing with Wait below, +// since both are serialized through mu. func (b *Bus) Stop() { + b.mu.Lock() + b.stopped = true + b.mu.Unlock() close(b.done) b.wg.Wait() } diff --git a/internal/eventbus/bus_test.go b/internal/eventbus/bus_test.go index 57a3cce72..7faad6e1b 100644 --- a/internal/eventbus/bus_test.go +++ b/internal/eventbus/bus_test.go @@ -260,3 +260,17 @@ func waitDone(wg *sync.WaitGroup) <-chan struct{} { }() return ch } + +func TestBusSubscribeAfterStopIsNoop(t *testing.T) { + b := New(4) + b.Stop() + + b.Subscribe("late", func(Event) {}) + + b.mu.RLock() + n := len(b.subs) + b.mu.RUnlock() + if n != 0 { + t.Fatalf("Subscribe after Stop registered %d subscriber(s), want 0 (a stopped bus must not accept new subscribers, and must not call wg.Add after wg.Wait has been entered)", n) + } +} diff --git a/internal/sub/clash_service.go b/internal/sub/clash_service.go index bff4c088e..fc07a3f63 100644 --- a/internal/sub/clash_service.go +++ b/internal/sub/clash_service.go @@ -168,16 +168,22 @@ func (s *SubClashService) getProxies(subReq *SubService, inbound *model.Inbound, proxies := make([]map[string]any, 0, len(externalProxies)) for _, ep := range externalProxies { - extPrxy := ep.(map[string]any) + extPrxy, ok := ep.(map[string]any) + if !ok { + continue + } // Expand the host's {{VAR}} remark template for this client (no-op for // the synthetic/legacy entry) before it becomes the proxy name. subReq.renderHostRemark(inbound, client, extPrxy, network) workingInbound := *inbound - workingInbound.Listen = extPrxy["dest"].(string) - workingInbound.Port = int(extPrxy["port"].(float64)) + workingInbound.Listen, _ = extPrxy["dest"].(string) + if port, ok := extPrxy["port"].(float64); ok { + workingInbound.Port = int(port) + } workingStream := cloneStreamForExternalProxy(stream) - switch extPrxy["forceTls"].(string) { + forceTls, _ := extPrxy["forceTls"].(string) + switch forceTls { case "tls": if workingStream["security"] != "tls" { workingStream["security"] = "tls" diff --git a/internal/sub/json_service.go b/internal/sub/json_service.go index 6385b4349..9ac60c2dc 100644 --- a/internal/sub/json_service.go +++ b/internal/sub/json_service.go @@ -177,14 +177,20 @@ func (s *SubJsonService) getConfig(subReq *SubService, inbound *model.Inbound, c network, _ := stream["network"].(string) for _, ep := range externalProxies { - extPrxy := ep.(map[string]any) + extPrxy, ok := ep.(map[string]any) + if !ok { + continue + } // Expand the host's {{VAR}} remark template for this client (no-op for // the synthetic/legacy entry) before it's used as the config remark. subReq.renderHostRemark(inbound, client, extPrxy, network) - inbound.Listen = extPrxy["dest"].(string) - inbound.Port = int(extPrxy["port"].(float64)) + inbound.Listen, _ = extPrxy["dest"].(string) + if port, ok := extPrxy["port"].(float64); ok { + inbound.Port = int(port) + } newStream := cloneStreamForExternalProxy(stream) - switch extPrxy["forceTls"].(string) { + forceTls, _ := extPrxy["forceTls"].(string) + switch forceTls { case "tls": if newStream["security"] != "tls" { newStream["security"] = "tls" @@ -351,13 +357,13 @@ func (s *SubJsonService) realityData(rData map[string]any, clientKey string) map rltyData["spiderX"] = deriveSpiderX(seed, clientKey) shortIds, ok := rData["shortIds"].([]any) if ok && len(shortIds) > 0 { - rltyData["shortId"] = shortIds[random.Num(len(shortIds))].(string) + rltyData["shortId"], _ = shortIds[random.Num(len(shortIds))].(string) } else { rltyData["shortId"] = "" } serverNames, ok := rData["serverNames"].([]any) if ok && len(serverNames) > 0 { - rltyData["serverName"] = serverNames[random.Num(len(serverNames))].(string) + rltyData["serverName"], _ = serverNames[random.Num(len(serverNames))].(string) } else { rltyData["serverName"] = "" } diff --git a/internal/sub/remark_vars_test.go b/internal/sub/remark_vars_test.go index 32276b6a6..b291a0ed5 100644 --- a/internal/sub/remark_vars_test.go +++ b/internal/sub/remark_vars_test.go @@ -131,15 +131,11 @@ func TestExpandRemarkVars_DropUnlimitedSegments(t *testing.T) { func TestExpandRemarkVars_DropEmptySegments(t *testing.T) { inbound := &model.Inbound{Remark: "host"} - // A client with no comment: the {{COMMENT}} segment resolves to empty and - // must be dropped, not left as a trailing "|". noComment := expandCtx(model.Client{}, xray.ClientTraffic{Enable: true}, inbound) if got := expandRemarkVars("{{INBOUND}}|{{COMMENT}}", noComment); got != "host" { t.Errorf("empty comment segment = %q, want %q (no trailing pipe)", got, "host") } - // A decorated empty segment (emoji + empty token) drops whole, not leaving - // a dangling emoji. if got := expandRemarkVars("{{INBOUND}}|📅{{EXPIRE_DATE}}", noComment); got != "host" { t.Errorf("decorated empty segment = %q, want %q", got, "host") } diff --git a/internal/sub/service.go b/internal/sub/service.go index 2fded5d06..7a8712b8d 100644 --- a/internal/sub/service.go +++ b/internal/sub/service.go @@ -993,7 +993,9 @@ func (s *SubService) genHysteriaLink(inbound *model.Inbound, email string) strin alpns, _ := tlsSetting["alpn"].([]any) var alpn []string for _, a := range alpns { - alpn = append(alpn, a.(string)) + if s, ok := a.(string); ok { + alpn = append(alpn, s) + } } if len(alpn) > 0 { params["alpn"] = strings.Join(alpn, ",") @@ -1180,7 +1182,7 @@ func unmarshalStreamSettings(streamSettings string) map[string]any { } func applyPathAndHostParams(settings map[string]any, params map[string]string) { - params["path"] = settings["path"].(string) + params["path"], _ = settings["path"].(string) if host, ok := settings["host"].(string); ok && len(host) > 0 { params["host"] = host } else { @@ -1190,7 +1192,7 @@ func applyPathAndHostParams(settings map[string]any, params map[string]string) { } func applyPathAndHostObj(settings map[string]any, obj map[string]any) { - obj["path"] = settings["path"].(string) + obj["path"], _ = settings["path"].(string) if host, ok := settings["host"].(string); ok && len(host) > 0 { obj["host"] = host } else { @@ -1312,7 +1314,9 @@ func applyShareTLSParams(stream map[string]any, params map[string]string) { alpns, _ := tlsSetting["alpn"].([]any) var alpn []string for _, a := range alpns { - alpn = append(alpn, a.(string)) + if s, ok := a.(string); ok { + alpn = append(alpn, s) + } } if len(alpn) > 0 { params["alpn"] = strings.Join(alpn, ",") @@ -1346,7 +1350,9 @@ func applyVmessTLSParams(stream map[string]any, obj map[string]any) { if len(alpns) > 0 { var alpn []string for _, a := range alpns { - alpn = append(alpn, a.(string)) + if s, ok := a.(string); ok { + alpn = append(alpn, s) + } } obj["alpn"] = strings.Join(alpn, ",") } diff --git a/internal/sub/sub_panic_test.go b/internal/sub/sub_panic_test.go index 8eb9ccb39..334dcb0ce 100644 --- a/internal/sub/sub_panic_test.go +++ b/internal/sub/sub_panic_test.go @@ -11,11 +11,6 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/database/model" ) -// A subscription is built by iterating every client's share link with no -// recover(), so any panic in the link generators 500s the whole subscription -// for every client. Valid-but-unusual stream settings (an empty Reality -// shortIds/serverNames array, a tcp-http header with no request, a grpc block -// missing its keys) must therefore produce a link, not a panic. func TestGetSubsToleratesUnusualStreamSettings(t *testing.T) { gin.SetMode(gin.TestMode) cases := []struct { @@ -27,6 +22,9 @@ func TestGetSubsToleratesUnusualStreamSettings(t *testing.T) { {"tcp http empty path", `{"network":"tcp","security":"none","tcpSettings":{"header":{"type":"http","request":{"path":[]}}}}`}, {"grpc missing keys", `{"network":"grpc","security":"none","grpcSettings":{}}`}, {"empty stream settings", `{}`}, + {"ws missing wsSettings", `{"network":"ws","security":"none"}`}, + {"httpupgrade missing settings", `{"network":"httpupgrade","security":"none"}`}, + {"tls alpn non-string element", `{"network":"tcp","security":"tls","tlsSettings":{"alpn":[123]}}`}, } for i, tc := range cases { @@ -46,9 +44,6 @@ func TestGetSubsToleratesUnusualStreamSettings(t *testing.T) { } } -// The JSON subscription generator for a Hysteria inbound whose StreamSettings -// omit the hysteriaSettings key must not panic (which would 500 the whole JSON -// subscription); the raw generator already tolerates this shape. func TestGetJsonToleratesHysteriaWithoutHysteriaSettings(t *testing.T) { seedSubDB(t) db := database.GetDB() @@ -83,9 +78,21 @@ func TestGetJsonToleratesHysteriaWithoutHysteriaSettings(t *testing.T) { } } -// A Clash subscription must carry the pinned peer certificate SHA-256 when the -// inbound configures one, matching the JSON subscription; dropping it silently -// downgrades certificate pinning for Clash subscribers. +func TestGetJsonToleratesNonStringRealityShortId(t *testing.T) { + seedSubDB(t) + stream := `{"network":"tcp","security":"reality","realitySettings":{"serverNames":["sni.example.com"],"shortIds":[42],"settings":{"publicKey":"pk"}}}` + seedSubInbound(t, "rlty1", "rlty", 46400, 1, stream) + + jsonService := NewSubJsonService("", "", "", NewSubService("")) + out, _, err := jsonService.GetJson("rlty1", "sub.example.com", true) + if err != nil { + t.Fatalf("GetJson: %v", err) + } + if out == "" { + t.Fatal("GetJson returned empty for a reality inbound with a non-string shortId element") + } +} + func TestGetClashEmitsPinnedCertSha256(t *testing.T) { seedSubDB(t) const pin = "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" @@ -100,3 +107,26 @@ func TestGetClashEmitsPinnedCertSha256(t *testing.T) { t.Fatalf("Clash proxy dropped the pinned cert sha256:\n%s", out) } } + +func TestJsonAndClashTolerateExternalProxyMissingPort(t *testing.T) { + seedSubDB(t) + stream := `{"network":"tcp","security":"none","externalProxy":[{"forceTls":"same","dest":"cdn.example.com"}]}` + seedSubInbound(t, "extp1", "extp", 46500, 1, stream) + + jsonService := NewSubJsonService("", "", "", NewSubService("")) + jsonOut, _, err := jsonService.GetJson("extp1", "sub.example.com", true) + if err != nil { + t.Fatalf("GetJson: %v", err) + } + if jsonOut == "" { + t.Fatal("GetJson returned empty for an externalProxy entry missing port") + } + + clashOut, _, err := NewSubClashService(false, "", NewSubService("")).GetClash("extp1", "sub.example.com") + if err != nil { + t.Fatalf("GetClash: %v", err) + } + if clashOut == "" { + t.Fatal("GetClash returned empty for an externalProxy entry missing port") + } +} diff --git a/internal/util/link/outbound.go b/internal/util/link/outbound.go index 22ef6b6e6..af0dbcd0a 100644 --- a/internal/util/link/outbound.go +++ b/internal/util/link/outbound.go @@ -621,11 +621,6 @@ func applyTransport(stream map[string]any, p url.Values) { if m := p.Get("mode"); m != "" { xh["mode"] = m } - // The panel's own emitters carry the advanced xhttp knobs as a - // snake_case x_padding_bytes plus an extra= blob, not as top-level - // camelCase params. Mirror the TS parser's precedence (snake_case alias - // -> extra JSON -> explicit camelCase) so a re-imported share link keeps - // them instead of silently reverting to the stream defaults. if v := p.Get("x_padding_bytes"); v != "" { xh["xPaddingBytes"] = v } diff --git a/internal/util/link/outbound_helpers_test.go b/internal/util/link/outbound_helpers_test.go index 08e91b2c7..f2bab9ab0 100644 --- a/internal/util/link/outbound_helpers_test.go +++ b/internal/util/link/outbound_helpers_test.go @@ -178,8 +178,6 @@ func TestParse_XhttpExtraAndSnakeCaseFields(t *testing.T) { } func TestParse_VmessWSPathWithoutHostKey(t *testing.T) { - // Some generators omit "host" when it equals the address; the path must - // still be honored (matching the TS parser), not dropped to the default. inner := `{"v":"2","add":"h","port":443,"id":"11111111-2222-4333-8444-555555555555","net":"ws","path":"/api","tls":"tls"}` link := "vmess://" + base64.StdEncoding.EncodeToString([]byte(inner)) res, err := ParseLink(link) diff --git a/internal/web/controller/login_limiter.go b/internal/web/controller/login_limiter.go index c25d2404c..6c9edaed2 100644 --- a/internal/web/controller/login_limiter.go +++ b/internal/web/controller/login_limiter.go @@ -110,11 +110,19 @@ func (l *loginLimiter) evictForRoom(now time.Time) { delete(l.attempts, key) } } - if len(l.attempts) >= loginLimitMaxRecords { - for key := range l.attempts { - delete(l.attempts, key) - break + if len(l.attempts) < loginLimitMaxRecords { + return + } + for key, record := range l.attempts { + if now.Before(record.blockedUntil) { + continue } + delete(l.attempts, key) + return + } + for key := range l.attempts { + delete(l.attempts, key) + return } } diff --git a/internal/web/controller/login_limiter_test.go b/internal/web/controller/login_limiter_test.go index 1436241f0..8e8051bfd 100644 --- a/internal/web/controller/login_limiter_test.go +++ b/internal/web/controller/login_limiter_test.go @@ -2,13 +2,11 @@ package controller import ( "strconv" + "strings" "testing" "time" ) -// An unauthenticated attacker can flood /login with fresh usernames, each of -// which seeds a record keyed on that username. The attempts map must stay -// bounded rather than growing until the process OOMs. func TestLoginLimiterBoundsMemoryUnderUsernameFlood(t *testing.T) { limiter := newLoginLimiter(5, 5*time.Minute, 15*time.Minute) for i := 0; i < loginLimitMaxRecords+100; i++ { @@ -24,6 +22,35 @@ func TestLoginLimiterBoundsMemoryUnderUsernameFlood(t *testing.T) { } } +func TestLoginLimiterEvictionSparesActiveBlocks(t *testing.T) { + now := time.Date(2026, 5, 6, 12, 0, 0, 0, time.UTC) + limiter := newLoginLimiter(5, 5*time.Minute, 15*time.Minute) + limiter.now = func() time.Time { return now } + + limiter.mu.Lock() + for i := 0; i < loginLimitMaxRecords-1; i++ { + limiter.attempts["victim-"+strconv.Itoa(i)] = &loginLimitRecord{blockedUntil: now.Add(10 * time.Minute)} + } + limiter.attempts["filler"] = &loginLimitRecord{failures: []time.Time{now}} + limiter.mu.Unlock() + + if _, blocked := limiter.registerFailure("9.9.9.9", "newcomer"); blocked { + t.Fatal("the eviction-triggering failure itself should not be blocked yet") + } + + limiter.mu.Lock() + defer limiter.mu.Unlock() + survivors := 0 + for key, record := range limiter.attempts { + if strings.HasPrefix(key, "victim-") && now.Before(record.blockedUntil) { + survivors++ + } + } + if survivors != loginLimitMaxRecords-1 { + t.Fatalf("eviction under a full map dropped an actively-blocked record: %d/%d victims survived", survivors, loginLimitMaxRecords-1) + } +} + func TestLoginLimiterBlocksAfterConfiguredFailures(t *testing.T) { now := time.Date(2026, 5, 6, 12, 0, 0, 0, time.UTC) limiter := newLoginLimiter(5, 5*time.Minute, 15*time.Minute) diff --git a/internal/web/entity/check_valid_test.go b/internal/web/entity/check_valid_test.go index e126b4729..71da5d7e9 100644 --- a/internal/web/entity/check_valid_test.go +++ b/internal/web/entity/check_valid_test.go @@ -29,13 +29,11 @@ func TestCheckValidSmtpFrom(t *testing.T) { } func TestCheckValidWildcardListenPortConflict(t *testing.T) { - // Same port, both bind all interfaces but spelled differently -> conflict. s := &AllSetting{WebPort: 2053, SubPort: 2053, WebListen: "0.0.0.0", SubListen: ""} if err := s.CheckValid(); err == nil { t.Error("CheckValid must reject the same port bound on 0.0.0.0 and \"\" (both wildcard)") } - // Same port on two distinct specific addresses can coexist and must be allowed. ok := &AllSetting{WebPort: 2053, SubPort: 2053, WebListen: "127.0.0.1", SubListen: "192.168.1.1"} if err := ok.CheckValid(); err != nil { t.Errorf("distinct specific listens on the same port should be allowed: %v", err) diff --git a/internal/web/service/access_log_parse_test.go b/internal/web/service/access_log_parse_test.go index 26dbecf1b..2d57da48e 100644 --- a/internal/web/service/access_log_parse_test.go +++ b/internal/web/service/access_log_parse_test.go @@ -2,10 +2,6 @@ package service import "testing" -// Xray access-log lines carry attacker-influenced content (a client's requested -// destination is logged verbatim) and can be truncated. parseAccessLogFields -// must never panic on a short or malformed line, and must still parse a -// well-formed line correctly. func TestParseAccessLogFields(t *testing.T) { malformed := []string{ "", diff --git a/internal/web/service/client_apply_field_test.go b/internal/web/service/client_apply_field_test.go index 952258208..3f90d3ba7 100644 --- a/internal/web/service/client_apply_field_test.go +++ b/internal/web/service/client_apply_field_test.go @@ -88,9 +88,6 @@ func TestResetClientExpiryTimeByEmail_MultiInbound(t *testing.T) { } } -// A disable-by-email (Telegram bot / LDAP sync) must flip enable on every -// inbound the client is attached to. Patching only the first inbound left the -// client connecting through its siblings' running Xray. func TestSetClientEnableByEmail_MultiInbound(t *testing.T) { dbDir := t.TempDir() t.Setenv("XUI_DB_FOLDER", dbDir) diff --git a/internal/web/service/client_locks_test.go b/internal/web/service/client_locks_test.go index a6d6daeb9..e4284d394 100644 --- a/internal/web/service/client_locks_test.go +++ b/internal/web/service/client_locks_test.go @@ -5,9 +5,6 @@ import ( "time" ) -// lockInbound must not hold the global registry mutex while it waits on a busy -// inbound's own mutex, otherwise one slow client operation on a single inbound -// freezes client mutations on every other inbound panel-wide. func TestLockInboundReleasesRegistryMutexWhileWaiting(t *testing.T) { const id = 990006 held := lockInbound(id) diff --git a/internal/web/service/client_traffic.go b/internal/web/service/client_traffic.go index 78a25cc09..f82bae668 100644 --- a/internal/web/service/client_traffic.go +++ b/internal/web/service/client_traffic.go @@ -210,7 +210,10 @@ func (s *ClientService) ResetAllTraffics() (bool, error) { return res.Error } affected = res.RowsAffected - return tx.Where("1 = 1").Delete(&model.ClientGlobalTraffic{}).Error + if err := tx.Where("1 = 1").Delete(&model.ClientGlobalTraffic{}).Error; err != nil { + return err + } + return tx.Where("1 = 1").Delete(&model.NodeClientTraffic{}).Error }) }) if err != nil { diff --git a/internal/web/service/email/email_deadline_test.go b/internal/web/service/email/email_deadline_test.go index b34b00b84..17d36c423 100644 --- a/internal/web/service/email/email_deadline_test.go +++ b/internal/web/service/email/email_deadline_test.go @@ -6,10 +6,6 @@ import ( "time" ) -// A server that accepts the TCP connection but then never speaks must not block -// the sender goroutine indefinitely: sendPlain arms a connection deadline so the -// SMTP greeting read fails instead of hanging until the OS TCP timeout, long -// after the caller's own 30s budget has passed. func TestSendPlainReturnsOnStalledServer(t *testing.T) { orig := smtpDeadline smtpDeadline = 300 * time.Millisecond diff --git a/internal/web/service/inbound_finalmask_reality_test.go b/internal/web/service/inbound_finalmask_reality_test.go index 62f3b6aa5..13d9097fd 100644 --- a/internal/web/service/inbound_finalmask_reality_test.go +++ b/internal/web/service/inbound_finalmask_reality_test.go @@ -91,10 +91,6 @@ func TestAddInbound_RejectsFinalMaskRealityCombo(t *testing.T) { } } -// AddInbound must always create a new row. The add controller binds the model's -// `id` form field and never clears it, so a client that reuses an existing id -// (e.g. duplicating an inbound fetched from /get) must not silently overwrite -// that stored row via GORM Save's upsert-on-primary-key behavior. func TestAddInbound_IgnoresBoundIdAndCreatesNewRow(t *testing.T) { setupConflictDB(t) svc := &InboundService{} @@ -127,10 +123,6 @@ func TestAddInbound_IgnoresBoundIdAndCreatesNewRow(t *testing.T) { } } -// A WireGuard inbound carrying clients (an imported config, or a node-reconcile -// re-add) must be accepted: WG clients are keyed by their public key and have no -// `id`, so the generic default validation branch wrongly rejected them with -// "empty client ID". func TestAddInbound_AcceptsWireguardClientWithKey(t *testing.T) { setupConflictDB(t) svc := &InboundService{} diff --git a/internal/web/service/inbound_mtproto_txfail_test.go b/internal/web/service/inbound_mtproto_txfail_test.go index c4fa6599b..744140e40 100644 --- a/internal/web/service/inbound_mtproto_txfail_test.go +++ b/internal/web/service/inbound_mtproto_txfail_test.go @@ -11,11 +11,6 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/web/runtime" ) -// A local MTProto inbound edit must not push to the managed sidecar from inside -// the serialized write transaction: that blocks the single traffic-writer -// goroutine on process/network I/O, and a later step failing the transaction -// would leave the sidecar ahead of the rolled-back database. The push belongs in -// the post-commit hook, exactly as the xray branch already does it. func TestUpdateInboundLocalMtprotoDefersPushUntilCommit(t *testing.T) { setupConflictDB(t) @@ -52,9 +47,6 @@ func TestUpdateInboundLocalMtprotoDefersPushUntilCommit(t *testing.T) { } } -// Re-enabling a routed MTProto inbound must request an xray restart: the egress -// SOCKS bridge is only injected for enabled inbounds, so the running config -// needs regenerating or the sidecar dials a bridge that is not there. func TestSetInboundEnableRoutedMtprotoRequestsRestart(t *testing.T) { setupConflictDB(t) diff --git a/internal/web/service/inbound_traffic.go b/internal/web/service/inbound_traffic.go index 7631a0bb0..4b2a2953c 100644 --- a/internal/web/service/inbound_traffic.go +++ b/internal/web/service/inbound_traffic.go @@ -37,9 +37,11 @@ func (s *InboundService) addTrafficLocked(inboundTraffics []*xray.Traffic, clien defer func() { if err != nil { - tx.Rollback() - } else { - tx.Commit() + if rbErr := tx.Rollback().Error; rbErr != nil { + logger.Warning("Error rolling back traffic tx:", rbErr) + } + } else if cErr := tx.Commit().Error; cErr != nil { + logger.Warning("Error committing traffic tx:", cErr) } }() err = s.addInboundTraffic(tx, inboundTraffics) diff --git a/internal/web/service/integration/warp_response_test.go b/internal/web/service/integration/warp_response_test.go index d37ca1f9b..da2ed01b4 100644 --- a/internal/web/service/integration/warp_response_test.go +++ b/internal/web/service/integration/warp_response_test.go @@ -10,9 +10,6 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/database" ) -// A hostile egress proxy (or a MITM on the WARP endpoint) could stream an -// arbitrarily large body; doWarpRequest must cap the read at maxResponseSize so -// the panel cannot be forced into an unbounded allocation. func TestDoWarpRequestCapsResponseBody(t *testing.T) { if err := database.InitDB(filepath.Join(t.TempDir(), "x-ui.db")); err != nil { t.Fatalf("InitDB: %v", err) diff --git a/internal/web/service/key_gen_parse_test.go b/internal/web/service/key_gen_parse_test.go index 68aeb3f69..257813b98 100644 --- a/internal/web/service/key_gen_parse_test.go +++ b/internal/web/service/key_gen_parse_test.go @@ -2,10 +2,6 @@ package service import "testing" -// The x25519 / mldsa65 / mlkem768 key generators parse xray's stdout by fixed -// slice index. A short or reformatted output (a future xray release, or a -// binary that failed silently) must yield an error, never an out-of-range -// panic that 500s the handler. func TestParseXrayKeyPairOutput(t *testing.T) { a, b, err := parseXrayKeyPairOutput("Private key: abc123\nPublic key: def456\n") if err != nil { diff --git a/internal/web/service/outbound_subscription.go b/internal/web/service/outbound_subscription.go index 65e0cc27b..489bff8ac 100644 --- a/internal/web/service/outbound_subscription.go +++ b/internal/web/service/outbound_subscription.go @@ -277,7 +277,6 @@ func (s *OutboundSubscriptionService) RefreshAllEnabled() (int, error) { return refreshed, nil } -// fetchAndStore does the actual network + parse + stability + persist work. // subscriptionFetchClient builds the HTTP client used to fetch a subscription. // A configured panel egress proxy dials the loopback SOCKS bridge (xray handles // the real egress), so its localhost dial must not be SSRF-blocked. A direct @@ -295,6 +294,7 @@ func (s *OutboundSubscriptionService) subscriptionFetchClient(timeout time.Durat } } +// fetchAndStore does the actual network + parse + stability + persist work. func (s *OutboundSubscriptionService) fetchAndStore(sub *model.OutboundSubscription) ([]any, error) { // Re-sanitize on every fetch (handles legacy rows + defense in depth against // any direct DB tampering). Private targets are blocked unless this diff --git a/internal/web/service/outbound_subscription_ssrf_test.go b/internal/web/service/outbound_subscription_ssrf_test.go index 205dccc9a..ddf646489 100644 --- a/internal/web/service/outbound_subscription_ssrf_test.go +++ b/internal/web/service/outbound_subscription_ssrf_test.go @@ -10,9 +10,6 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/util/netsafe" ) -// The direct subscription-fetch client must dial through the SSRF guard so a -// subscription host that resolves to a private/internal address (including a -// DNS-rebinding flip after validation) is blocked at dial time, not connected to. func TestSubscriptionFetchClientBlocksPrivateDial(t *testing.T) { setupSettingTestDB(t) client := (&OutboundSubscriptionService{}).subscriptionFetchClient(5 * time.Second) diff --git a/internal/web/service/reset_writer_test.go b/internal/web/service/reset_writer_test.go index 991fcd6b3..0ffa13192 100644 --- a/internal/web/service/reset_writer_test.go +++ b/internal/web/service/reset_writer_test.go @@ -21,9 +21,6 @@ func (b *blockingResetRuntime) ResetAllTraffics(context.Context) error { return nil } -// A "Reset All Traffics" that propagates to nodes must not hold the single serial -// traffic writer across the remote HTTP calls: each can block for seconds, and -// stalling the writer drops every concurrent poll's traffic deltas. func TestResetAllTrafficsDoesNotBlockWriterOnNodeCall(t *testing.T) { db := initTrafficTestDB(t) resetTrafficWriterForTest(t) diff --git a/internal/web/service/tgbot/tgbot_callback_auth_test.go b/internal/web/service/tgbot/tgbot_callback_auth_test.go index 6b6f2f019..6e8c78e21 100644 --- a/internal/web/service/tgbot/tgbot_callback_auth_test.go +++ b/internal/web/service/tgbot/tgbot_callback_auth_test.go @@ -6,12 +6,6 @@ import ( "github.com/mymmrac/telego" ) -// A non-admin callback must never reach a privileged handler. The second -// callback switch runs outside the isAdmin guard, so without the default-deny -// check a non-admin who can tap an admin's inline button (e.g. in a group) could -// export the database backup or reset all traffic. Here the privileged handler -// would panic on the nil bot/services of a bare Tgbot; the guard must return -// first, so no panic occurs. func TestAnswerCallbackDeniesPrivilegedActionToNonAdmin(t *testing.T) { defer func() { if r := recover(); r != nil { diff --git a/internal/web/service/tgbot/tgbot_delete_after_test.go b/internal/web/service/tgbot/tgbot_delete_after_test.go index be4aa2a02..4a6ca7329 100644 --- a/internal/web/service/tgbot/tgbot_delete_after_test.go +++ b/internal/web/service/tgbot/tgbot_delete_after_test.go @@ -2,10 +2,6 @@ package tgbot import "testing" -// A transient "delete after N seconds" message must not reset the conversation -// state when its timer fires: the user may have advanced to the next wizard step -// (setting a fresh state) within that window, and clearing it would silently -// drop their next input. func TestDeleteMessageAfterDelayKeepsUserState(t *testing.T) { userStateMgr.reset() t.Cleanup(userStateMgr.reset) diff --git a/internal/web/service/tgbot/tgbot_router.go b/internal/web/service/tgbot/tgbot_router.go index 74042adf5..c45fd3c08 100644 --- a/internal/web/service/tgbot/tgbot_router.go +++ b/internal/web/service/tgbot/tgbot_router.go @@ -1128,11 +1128,6 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool } } - // The callbacks below sit outside the isAdmin block above, so a non-admin who - // can see an admin's inline keyboard (for example when the bot runs in a - // group) could otherwise trigger a database backup export, a mass traffic - // reset or client creation. Default-deny: a non-admin may only run the - // per-user client_* callbacks that key off their own Telegram id. if !isAdmin && !isClientSelfCallback(callbackQuery.Data) { return } diff --git a/internal/web/service/traffic_commit_test.go b/internal/web/service/traffic_commit_test.go index a59824601..0a9454dc4 100644 --- a/internal/web/service/traffic_commit_test.go +++ b/internal/web/service/traffic_commit_test.go @@ -11,11 +11,6 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/xray" ) -// The traffic tick stages inbound and client deltas, then runs three best-effort -// maintenance helpers (renew, disable-depleted-clients, disable-depleted-inbounds) -// that are meant to log and continue. A failure in one of them must not roll back -// the already-staged traffic — xray has already advanced its baseline, so a -// rolled-back tick loses that traffic permanently. func TestAddTrafficCommitsDespiteDisableHelperError(t *testing.T) { db := initTrafficTestDB(t) svc := &InboundService{} @@ -53,9 +48,6 @@ func TestAddTrafficCommitsDespiteDisableHelperError(t *testing.T) { } } -// Reset All Client Traffic must re-enable clients that were auto-disabled for -// exceeding their quota, matching every other reset path; otherwise a reset -// leaves depleted clients cut with zero usage. func TestResetAllTrafficsReenablesDepletedClients(t *testing.T) { db := initTrafficTestDB(t) svc := &ClientService{} @@ -76,3 +68,27 @@ func TestResetAllTrafficsReenablesDepletedClients(t *testing.T) { t.Fatal("a depleted client must be re-enabled after Reset All Client Traffic, matching every other reset path") } } + +func TestResetAllTrafficsClearsNodeBaselines(t *testing.T) { + db := initTrafficTestDB(t) + svc := &ClientService{} + + if err := db.Create(&xray.ClientTraffic{InboundId: 1, Email: "spent@x", Enable: true, Up: 60, Down: 60, Total: 100}).Error; err != nil { + t.Fatalf("seed traffic: %v", err) + } + if err := db.Create(&model.NodeClientTraffic{NodeId: 1, Email: "spent@x", Up: 60, Down: 60}).Error; err != nil { + t.Fatalf("seed node baseline: %v", err) + } + + if _, err := svc.ResetAllTraffics(); err != nil { + t.Fatalf("ResetAllTraffics: %v", err) + } + + var cnt int64 + if err := db.Model(&model.NodeClientTraffic{}).Where("email = ?", "spent@x").Count(&cnt).Error; err != nil { + t.Fatalf("count baselines: %v", err) + } + if cnt != 0 { + t.Fatalf("Reset All Client Traffic must clear node baselines like its sibling reset paths, found %d", cnt) + } +} diff --git a/internal/web/service/xray.go b/internal/web/service/xray.go index 2a2a0368f..ba0453c75 100644 --- a/internal/web/service/xray.go +++ b/internal/web/service/xray.go @@ -986,9 +986,6 @@ func (s *XrayService) RestartXray(isForce bool) error { lock.Lock() defer lock.Unlock() logger.Debug("restart Xray, force:", isForce) - // A background reconcile (a pending config-change flag, warp/ldap/outbound - // jobs) must not revive an Xray the admin deliberately stopped; only an - // explicit forced restart clears the manual-stop state. if !isForce && isManuallyStopped.Load() { return nil } diff --git a/internal/web/service/xray_restart_test.go b/internal/web/service/xray_restart_test.go index aa0611612..095213efc 100644 --- a/internal/web/service/xray_restart_test.go +++ b/internal/web/service/xray_restart_test.go @@ -4,9 +4,6 @@ import ( "testing" ) -// A background (non-forced) restart — the pending-config-change cron, warp/ldap/ -// outbound reconcile jobs — must not revive an Xray the admin deliberately -// stopped. Only an explicit forced restart clears the manual-stop state. func TestRestartXrayRespectsManualStop(t *testing.T) { setupSettingTestDB(t) if err := (&SettingService{}).saveSetting("xrayTemplateConfig", "{ not valid json"); err != nil { @@ -22,9 +19,6 @@ func TestRestartXrayRespectsManualStop(t *testing.T) { } } -// When the pending-restart reconcile consumes the need-restart flag but the -// restart itself fails, the flag must be re-armed so the config change is -// retried rather than silently dropped. func TestApplyPendingRestartReArmsFlagOnFailure(t *testing.T) { setupSettingTestDB(t) if err := (&SettingService{}).saveSetting("xrayTemplateConfig", "{ not valid json"); err != nil { diff --git a/internal/xray/api_test.go b/internal/xray/api_test.go index c3ceefe2a..d49fc5433 100644 --- a/internal/xray/api_test.go +++ b/internal/xray/api_test.go @@ -5,9 +5,6 @@ import ( "testing" ) -// RemoveUser must return an error, not panic, when the handler client is not -// initialized — matching every sibling API method. A depletion sweep can reach -// it with a nil client during a restart window where Init(0) failed. func TestRemoveUserGuardsNilHandlerClient(t *testing.T) { err := (&XrayAPI{}).RemoveUser("in-443-tcp", "user@example.com") if err == nil {