* fix(clients): surface bulk-reset auto-enable failures
BulkResetTraffic re-enables a disabled client before resetting its
traffic, but discarded the s.Update result with `_, _ =`, so a failed
re-enable was silent: the client stayed disabled with nothing logged,
unlike the single-client ResetTraffic path which already warns on the
same call. Check the error and log a warning to match, and add a
regression test covering BulkResetTraffic's previously-untested
re-enable path.
* ci: update Go toolchain for govulncheck
---------
Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
* fix(clients): parse only settings.clients across protocols
Several inbound settings readers decoded the whole settings object into map[string][]model.Client. Real protocol settings include scalar keys such as VLESS decryption and Hysteria version, so that shape can fail before callers reach settings.clients or leave them relying on decoder side effects.
Add one shared helper that extracts only the clients field through json.RawMessage, then use it from GetClients, SearchClientTraffic and the IP-limit job fallback paths. Regression tests cover VLESS and Hysteria settings with scalar protocol fields.
* fix(clients): reject empty inbound settings
* fix(sync): mark node dirty inside the mutation transaction
ConfigDirty is currently set by MarkNodeDirty AFTER the mutation, on a
separate DB handle outside the mutation's transaction. A crash or error
between the committed change and the mark leaves a committed config
change that never reconciles to the node (silent drift). Add
MarkNodeDirtyTx(tx, id) and call it inside each mutation's transaction so
the dirty mark commits atomically with the change.
* fix(test): initialize DB in TestResolveInboundAddress and group gorm import
Two CI failures on this branch:
- race (-shuffle=on): TestResolveInboundAddress reaches resolveInboundAddress -> configuredPublicHost -> GetSubDomain, which reads the global DB. The test never initialized one, relying on another sub-package test to do so first; under shuffle it ran first and nil-dereferenced gorm. Call initSubDB(t) so it is self-sufficient (empty DB yields an empty subDomain, so the subscriber-host fallback still holds).
- golangci goimports: gorm.io/gorm was grouped with the github.com/mhsanaei/3x-ui local imports in node_dirty_test.go. Move it into the third-party group.
* fix(settings): require server-side 2fa for sensitive changes
* fix(lint): group third-party imports separately from local (goimports)
golangci-lint goimports flagged setting.go and setting_security_test.go because xlzd/gotp and gorm.io/gorm were mixed into the github.com/mhsanaei/3x-ui local-prefix group. Move them into the third-party group so the local imports stand alone.
The external subscription fetcher read the remote body with a plain
io.LimitReader, silently truncating at 2 MiB and decoding whatever
prefix arrived (possibly a half share link). Detect the overflow with
the established N+1 pattern and return an error so the caller serves the
last cached value instead of a corrupted partial list.
Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
* perf(xray): compile log/traffic regexps once at package scope
GetTraffic recompiled two stats regexps on every traffic tick, and LogWriter.Write
recompiled two more on every log line. Hoist all four to package-level vars so they
compile once at load instead of per call on hot paths.
* fix(xray): guard LogWriter.lastLine against the GetResult reader race
Write is driven by the Xray process goroutine while Process.GetResult
reads lastLine from the caller's goroutine, so the unsynchronized field
is a data race under `go test -race`. Add an RWMutex and route every
write through setLastLine; GetResult reads via LastLine().
* fix(xray): bound handler gRPC calls with a deadline
AddInbound, DelInbound and the AddUser AlterInbound call used
context.Background(), so a hung core connection could block the caller
indefinitely (for example while the process restart lock is held). Give
them a 10s deadline (handlerRPCTimeout) and a nil-client guard, matching
the other handler operations.
getSetting (WHERE key=?) runs on nearly every subscription request and job
tick and had no index, so each lookup full-scans the settings table past the
large xrayTemplateConfig blob. Add an index on settings.key; AutoMigrate
creates it on existing DBs too. Includes a HasIndex test.
GetTraffic recompiled two stats regexps on every traffic tick, and LogWriter.Write
recompiled two more on every log line. Hoist all four to package-level vars so they
compile once at load instead of per call on hot paths.
Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
NodeService.Delete dropped the node row (and its per-node child rows) without
checking for inbounds still referencing it via node_id, leaving orphaned
inbounds with a dangling node_id that confuse node sync, subscriptions and
cleanup. Refuse the delete with a clear error when inbounds are still attached,
and remove the per-node child rows before the node row inside one transaction.
Delete stays tolerant of a missing node row so it can still clean up orphaned
rows. Regression test covers the blocked and clean-delete paths.
The public subscription http.Server set no timeouts, leaving the most exposed
listener open to slow-header/Slowloris exhaustion. Mirror the panel server
timeouts already used in internal/web/web.go.
Remote node HTTP responses were read with an unbounded io.ReadAll, so a
broken or hostile node could force the master panel to buffer an arbitrarily
large body. The single Remote.do choke point that all node calls funnel
through now:
- validates the HTTP status before reading any success payload (a non-OK
body is only read up to a small bounded diagnostic snippet, so a node
cannot make the master buffer a large body just to return an error);
- fast-fails on an honestly-declared oversize Content-Length;
- reads the success body through readCappedBody, an io.LimitReader cap
(64 MiB) that rejects oversize with a typed error.
The 64 MiB cap bounds one response's wire/decompressed size; it is documented
as not a process-wide memory bound (endpoint-specific caps and a concurrency
budget remain follow-ups).
Tests cover the cap+1 boundary, an oversize streamed body, a normal envelope,
and non-OK status precedence.
A panic in a goroutine without a recover takes the whole panel down. The
per-node heartbeat and traffic-sync goroutines run remote network I/O for
each node with no panic isolation, so one misbehaving node could crash the
master.
Add common.GoRecover(name, fn), which runs fn in a goroutine guarded by a
recover that logs the panic with a stack trace instead of crashing, and use
it for the per-node heartbeat, traffic-sync and global-push goroutines. The
deferred WaitGroup/semaphore releases still run during panic unwind, so the
group never stalls. Other background goroutines can adopt the same helper.
The scheduler was created without a panic recovery wrapper, so a panic in any
scheduled job (traffic write, IP check, etc.) propagated up and could take down
the whole panel process. Wrap jobs with cron.Recover so a panic is logged and
the scheduler keeps running.
* fix(xray): verify the release archive checksum before installing
UpdateXray downloaded the Xray-core release zip and installed the binary
from it after only a TLS fetch, an HTTP-200 check and a size cap — the
archive itself was never verified, so a corrupted or tampered release
asset would be extracted and run as the panel's xray binary.
Verify the downloaded archive against the SHA2-256 published in the
release's .dgst sidecar (which XTLS ships next to every asset) before
installing, and abort the update on mismatch, a missing/short SHA2-256
entry, or an unreachable .dgst. The digest parser and fetch are covered by
tests, including the real .dgst line format ("SHA2-256= <hex>").
* address review: clearer warning + re-download guidance on checksum mismatch
Per review feedback on the PR: on a SHA-256 mismatch, surface a plain-language
warning that the downloaded archive is corrupted or differs from the official
release and that the user should exit and re-download, instead of a terse
"checksum mismatch" error. The install still aborts so a mismatched binary is
never run; the message now tells the user the safe next step.
The process cmd, done and exitErr fields were written by Start/startCommand and
the waitForCommand goroutine while IsRunning/GetErr/GetResult/Stop read them
concurrently from other goroutines (the status endpoint and the check-xray
job) — a data race. Guard them with a RWMutex: writers take the write lock;
readers snapshot under the read lock and run any blocking syscall
(Wait/Signal/Kill) on the local copy without holding it. IsRunning now uses the
done channel as the exit signal instead of reading cmd.ProcessState, which
races with cmd.Wait. Adds a -race regression test.
* fix(subscriptions): avoid shared mutable state during generation
* fix(subscriptions): serve external-link-only subs in JSON/Clash; load remark settings per request
The ForRequest refactor added an early `len(inbounds) == 0` return to
GetJson/GetClash that fired before external links were fetched, so a
subscription whose only entries are external links (or whose inbounds are
all disabled) rendered empty in the JSON and Clash formats. Drop the
premature check — the existing inbounds+externalLinks empty guard already
covers the truly-empty case.
Also load datepicker/emailInRemark in PrepareForRequest rather than only in
getSubs, so JSON and Clash remarks honor these settings instead of seeing
the zero values (emailInRemark previously depended on the shared-state leak
this PR fixes).
Add a regression test covering an external-link-only sub across both formats.
---------
Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
* feat(web): cap request body size on state-changing routes
* fix(web): exempt importDB from request body size cap
The 10 MiB body cap was applied globally, which would break database
restore (/panel/api/server/importDB) on any panel whose SQLite backup
exceeds the limit. Make MaxBodyBytes accept exempt path suffixes and
pass importDB through uncapped; the cap still covers all other
state-changing routes. Add a test for the skip-suffix behavior.
---------
Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
n0ctal