* fix(ci): keep a refused Claude credential from reddening a PR
An expired subscription ends the claude-code-action step with exit 0, so the
classifier that exists for "the API refused this run" never sees it -- its
condition is a failed step -- and the final "posted nothing" step reddens the
pull request although nothing is wrong with the repository.
Verified against five real runs (35159059540, 35184688775, 35185722358,
35186543654, 35187380192): step 8 success, step 10 found no cause, step 11
failure, transcript {"error":"oauth_org_not_allowed"} plus a result entry with
api_error_status 403. A usage-limited run carries 429 and a rejected
rate_limit_event, and a real review carries is_error false with no status, so
the 401/403 test fires on the refused credential alone.
* fix(ci): stop a refused credential reddening the issue analysis
The same exit-0 refusal reaches this workflow's "posted no reply" check, which
fails for the same reason and shows up as seven failed runs in a day. It never
attaches to a pull request -- the trigger excludes them -- so this is the same
step and the same 401/403 transcript test applied where the refusal lands.
Reported only as a warning annotation: nothing was analysed, and there is no
comment worth posting about a credential the maintainer has to renew.
* fix(nodes): say which half of node mTLS failed, and say it as an error
A configured client CA bundle that will not parse produced the same
warning as a settings read that failed, and both read as though mTLS
were merely unavailable. It is not: the node API silently stops
accepting client certificates, callers fall back to a bearer token or
lose their only credential, and the one line saying so is a warning at
boot.
Report it at error level, and distinguish the two causes rather than
attributing a storage fault to the operator's certificate bundle.
NodeMtlsClientCAPool now tags the parse failure with
ErrNodeMtlsTrustBundleInvalid; its message text is unchanged, so
anything matching on the existing string still matches.
Startup is deliberately left alone. Refusing to boot was considered and
rejected: the bundle is one of two equal credentials here, a panel that
will not start takes the proxies and the subscription server with it,
and bundles written before the stricter validation landed in #6188 are
already stored, editable only through the panel that would no longer
come up.
The tests pin the tag on an unusable bundle and its absence on an unset
one; without the tag the first goes red.
* test(nodes): drop a duplicate node mTLS trust-bundle test
TestNodeMtlsClientCAPoolLeavesUnsetBundleUntagged asserted only that an
unset nodeMtlsClientCAPem yields (nil, nil). That path returns before the
line the sentinel change touched, so the test was green with and without
ErrNodeMtlsTrustBundleInvalid, and TestNodeMtlsClientCAPool already pins
the same two assertions on the same fixture. A test that passes either way
certifies nothing and then gets cited as coverage for the sentinel.
TestNodeMtlsClientCAPoolTagsAnInvalidBundle, which does go red without the
sentinel, stays as the regression guard.
---------
Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
* fix(panel): accept 2FA codes from adjacent TOTP windows
CheckUser compared only gotp.Now(), so a code submitted at the end of
its 30s window (or with slight client/server clock drift) failed with
'invalid 2fa code', while the immediate retry in the next window
succeeded. Accept current +/-1 window, the standard TOTP skew
tolerance.
Fixes MHSanaei/3x-ui#6535
* fix(panel): share TOTP skew tolerance with VerifyTwoFactorCode
Move the +/-1 window helper to internal/util/totp so both 2FA
acceptance points use it: login (CheckUser) and disable/rebind plus
username/password changes (VerifyTwoFactorCode). Also shrink comments
to the 2-line house rule and anchor the unit test mid-window to avoid
a step-boundary flake.
Addresses review on #6546 (MEDIUM + 2 LOWs).
* fix(tgbot): localize QR caption via I18nBot
sendClientQRLinks hardcoded English 'QRCode for client <email>:',
bypassing I18nBot, so non-English bot languages (e.g. ru-RU) still
got English. Add tgbot.answers.qrCodeForClient key with Email param
in all 13 locales and route the caption through I18nBot.
FixesMHSanaei/3x-ui#6562
* fix(tgbot): repair locale JSON syntax, harden QR i18n test
- Add missing separators so all 13 locale files parse again.
- Rewrite the regression test to read the real shipped files
(fails on malformed JSON or missing key).
- Add TestTgbotLocalesQrKeyValid covering every locale file.
* chore(tgbot): drop QR caption tests that cannot catch the bug
TestQRCodeForClientLocalizes never calls sendClientQRLinks: it registers
two messages in a synthetic bundle and asserts on I18nBot, a passthrough
to go-i18n. With the tgbot_client.go line reverted to the hardcoded
English caption, both it and TestTgbotLocalesQrKeyValid still pass, so
neither certifies the fix.
The malformed-locale class they were added for is already pinned twice:
the discord package's TestMain loads every translation file through
locale.InitLocalizer and panics on invalid JSON, and
frontend/src/test/i18n-dead-keys.test.ts parses all 13 locales and
checks each carries the en-US key set. Both go red on the #6564 syntax
error this PR first shipped.
---------
Co-authored-by: sdhfsl <sdhfsl@users.noreply.github.com>
Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
* fix(panel): accept 2FA codes from adjacent TOTP windows
CheckUser compared only gotp.Now(), so a code submitted at the end of
its 30s window (or with slight client/server clock drift) failed with
'invalid 2fa code', while the immediate retry in the next window
succeeded. Accept current +/-1 window, the standard TOTP skew
tolerance.
Fixes MHSanaei/3x-ui#6535
* fix(panel): share TOTP skew tolerance with VerifyTwoFactorCode
Move the +/-1 window helper to internal/util/totp so both 2FA
acceptance points use it: login (CheckUser) and disable/rebind plus
username/password changes (VerifyTwoFactorCode). Also shrink comments
to the 2-line house rule and anchor the unit test mid-window to avoid
a step-boundary flake.
Addresses review on #6546 (MEDIUM + 2 LOWs).
* fix(sub): send stable X-HWID on external subscription fetch
A Master panel fetching a donor subscription sent no X-HWID, so an
HWID-limited donor rejected it with 404. Identify this panel with a
stable per-installation id (persisted in settings), occupying exactly
one donor device slot.
FixesMHSanaei/3x-ui#6559
* fix(sub): address review on external X-HWID
- Serialize first-time id creation with a mutex so concurrent
first fetches cannot mint two UUIDs.
- Fix goimports grouping for the new third-party import.
- Add externalSubSendHwid opt-out (default send); document it.
- Cover header send/omit with httptest in TestFetchSendsStableHwid.
* fix(sub): drop the SQL-only X-HWID opt-out
The externalSubSendHwid opt-out added in 227ed818 had no settings
field, CLI flag or docs, so an operator could only reach it by editing
the settings table by hand, while every cache-miss fetch paid a query
for it. CLAUDE.md rules out config knobs on a one-header fix.
Also drop the test assertions that only restated the 3x-ui-server-
prefix constant; TestFetchSendsStableHwid still goes red without the
header.
---------
Co-authored-by: sdhfsl <sdhfsl@users.noreply.github.com>
Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
The inbound TLS form offered cipherSuites as a closed single-choice list,
but xray reads the value as a colon-separated list and accepts any name Go
knows, so several suites or one missing from the list could not be set.
Both the inbound and the new host field now use a tag picker that keeps
the stored value as the colon-joined string xray expects; old single
values open unchanged.
A host's cipher suites replace the inbound's in the JSON subscription
stream, and a blank field inherits them. Share links and Clash carry no
cipher suite parameter, so their output is unchanged.
EnforceHwidForSubID returned before recording anything when a sub had no
limit, so the panel's HWID Devices list stayed empty for every unlimited
client. Devices are now upserted on the (sub_id, hwid_hash) index without
enforcement or X-Hwid-* headers; the write is best-effort and only logs on
failure, so tracking can never deny a subscription nothing restricts.
The node history panel passed its Net Up / Net Down series to Sparkline
without valueMax or yFormatter, so they inherited the percentage defaults:
a fixed 0-100 scale and a "%" label. Any node above 100 KB/s drew off the
top of the chart and every axis tick and tooltip read as a percentage.
A Sparkline fed non-percentage data has to declare its own scale and unit;
every other call site already did, only the two node net series did not.
Replace the package logger variable with an atomic.Pointer so InitLogger swapping the handle no longer races with concurrent Debug/Info/Warning/Error calls from other goroutines. Also guard fileRotate with a mutex, and add a regression test that reproduces the race under concurrent logging.
The info page was a long key/value table followed by every link and two
app dropdowns, and it rendered left-to-right even for Persian and Arabic.
It now leads with a usage ring, the remaining quota and a stats grid, and
splits the rest into Subscription / Apps / Configs tabs.
- Status tells expired, data-used-up and disabled apart instead of one
"Inactive", replacing the hard-coded English expiry chip.
- The Apps tab keeps every Android and iOS app with its existing deep
link, preselects the visitor's platform and adds Windows: Hiddify and
Clash Verge Rev import directly, v2rayN copies the link.
- fa-IR and ar-EG render right-to-left; URLs, IDs and sizes stay LTR.
- The footer shows the support link and the client refresh interval, so
subPageContext now carries subUpdates (also in ?format=info).
- Status, days-left and app deep-link logic lives in subPageModel.ts,
with unit tests pinning the deep links the page already shipped.
The page stacked the WebSocket event cards above every Panel API operation
in one long scroll. The WebSocket events and the 3X-UI Panel API now sit in
separate tabs, and the Panel API shows one OpenAPI tag at a time through
section tabs placed between the Authorize bar and the operations.
The section tabs replace Swagger UI's FilterContainer and wrap the
taggedOperations selector, so all sections share one Swagger instance and
keep authorization and try-it-out state. Swagger's own filter matches tags
by substring ("Settings" would also show "Xray Settings") and does nothing
until set, so the wrapper matches the exact tag and defaults to the first.
Tag names come from the loaded spec rather than importing endpoints.ts,
which would have grown the page chunk from 23 kB to 119 kB.
A subscription outbound's tag must stay bound to the upstream server it
was assigned to for as long as that server stays in the subscription;
balancers and routing rules select by that tag.
The identity used to recognise a server across refreshes included every
query parameter. A 3x-ui upstream picks a random shortId and SNI of a
reality inbound on every request (older releases a random spiderX too),
so no reality link was ever recognised, the stable-tag reservation never
engaged, and every tag was handed out by list position. Removing or
inserting a server then re-pointed existing tags at other servers:
sub-germany carried France, sub-sweden Germany, and Sweden became
sub-sweden-1. The identity now ignores sid, sni and spx when
security=reality, since none of them selects the server. TLS sni still
counts: it can pick the backend behind a shared front.
Two more paths broke the same rule:
- A link repeated in one body (same identity, different remark) shared a
single link_identities key, so both tags gained a -N suffix on every
refresh. Repeats are now numbered.
- Links the core rejects were dropped after tagging, so the stored list
that drives positional reuse was shorter than the parsed one and a
rotated server behind a dropped link took its neighbour's tag. The
filter now runs first; a dropped link's warning names its remark
instead of a tag it never used.
A mapping an older build already swapped stays swapped: its stored
identities no longer match, so positional reuse reproduces it. Deleting
and re-adding the subscription reallocates the tags from the remarks.
Closes#6556
Each card on the Clients page now toggles its status bucket as the sole
filter, and the Clients card clears it. The bucket filters used to be
wider than the card counts: "active" still included clients near
depletion and "deactive" included disabled clients that had run out, so
a filtered list could disagree with the number on the card. Both filters
now reuse the summary expressions, and a test pins each card's count to
the size of its filtered list.
The client form body is capped at the viewport and scrolls internally
(49ef1449). Every tab ends with a Form.Item that keeps antd's 24px bottom
margin, so when the fields themselves fit, that empty margin alone pushed
the body past the cap: 752px of content in 740px at a 900px window. The
last item of each tab now drops the margin, so the body scrolls only when
real content overflows.
When it does scroll, the bar was painted light inside the dark modal: the
dark themes set body.dark and data-theme but never color-scheme, which is
what native scrollbars read. applyDom (panel, login and subscription
bundles) and the Storybook decorator now set it on the root element.
A node's "update available" tag compares its reported panel version with the
master's latest, and any non-semver side fell back to string inequality. A
dev build reports dev+<sha> (config.GetPanelVersion), so a node moved to the
dev channel from a master on the stable channel kept the tag forever; the
reverse, a stable node under a master on the dev channel, was flagged too and
the tag's default stable update installed nothing new.
A dev label and a release tag carry no order, so the comparison now only
decides within one channel; dev-to-dev still compares commits, which keeps a
node on the current dev-latest commit untagged as config.go intends.
rc-table re-runs every cell renderer whenever the Table re-renders, and
NodeList rebuilt its columns and table props on every render (the relative
time formatter was a fresh function each time). Any re-render of the Nodes
page therefore re-rendered all rows even when no node had changed: about
390ms per re-render for 150 nodes in jsdom.
The formatter is now stable and the table element is memoized on its
inputs, so a re-render that leaves the nodes untouched costs 0.5ms. A
heartbeat push that does change the nodes still re-renders every row.
Every client_stats push carries the totals of all inbounds, and
applyClientStatsEvent rebuilt each row it listed, so every push replaced
all rows, re-ran the client rollup (a JSON parse of every inbound's
settings) and re-rendered the whole table even when no number moved. Every
traffic push also built new online and active maps, re-running the same
rollup.
Rows are now rebuilt only when their totals or a client's numbers change,
and the previous maps are kept when a push repeats the same sets. Measured
in jsdom with 450 inbounds of 50 clients each: an unchanged client_stats
push went from 7.9ms to 0.6ms with no row rebuilt, and a repeated traffic
push from 13.5ms to 8.3ms without the rollup.
The LDAP sync enabled, disabled and detached clients one at a time. Each
per-client call locked the inbound and pushed to its node under that lock
with a 4s timeout, so users sharing an inbound on a node that answers its
status probe but hangs on client writes queued one push timeout apiece:
five users took 20s in the test, and hundreds of directory users behind a
hung node stretched one run over hours. Each changed email was also queued
once per configured tag, repeating a no-op lookup for every extra tag.
Enable and disable now go through BulkSetEnable, and the cleanup through
one BulkDetach per inbound: each inbound is locked, written and pushed
once, and its push stops at the first failure for the reconcile to finish.
The same five users now cost a single push timeout.
The traffic sync's call that drops online sets of nodes it no longer
fetches had no test: a job-package test cannot install an xray process,
so online state was invisible there and removing the call passed.
SetXrayProcessForTest installs a test process for tests in other packages,
the same kind of seam as Manager.SetRuntimeOverride. The new job test runs
a real tick with a disabled node and a deleted one and fails without the
call.
Deleting a node must free what the master keeps per node in memory.
Delete dropped the node's cpu and mem series but not netUp and netDown,
which the heartbeat records too, so each deleted node leaked two tiered
histories. It now drops every NodeMetricKeys entry.
InvalidateNode, called on node edit, disable and delete, cleared only the
cached Remote. The pooled HTTP client and its transport stayed cached until
a later call for the same node pruned them, which a deleted node never
makes. InvalidateNode now drops those too, outside the manager lock; an
edited node pays one fresh handshake on its next call.
The traffic sync is scheduled every 5s but synced only 8 nodes at a time,
each needing four to seven sequential requests. Past about 125 nodes 80ms
away a tick outlasted its interval, so dashboard traffic, online clients
and quota enforcement moved at a fraction of the intended cadence.
Measured with 150-300 fake nodes over real HTTP, 80ms latency, a dashboard
connected and client-IP sync on:
SQLite, 300 nodes 8: 25-30s 16: 14-16s 32: 6-9.5s
SQLite, 150 (20% slow) 8: 24-27s 16: 13-14s 32: 6-7.5s
Postgres, 150 nodes 8: 13-18s 16: 7.5-10s 32: 6.5-8.3s
No database-locked, pool or writer-queue errors at any setting, and the
merged inbound and client traffic counts matched. Postgres's one-off
adoption tick is slower at 32 than at 16 (16.5s vs 9.7s) as goroutines
wait on its 25-connection pool; steady ticks are fastest at 32.
The periodic reset job reset every due inbound, then every due client, one
at a time, and each waited on its node: up to 10s per node inbound, and 4s
per attached node inbound for a client. A few hanging nodes stretched a
single run over hours.
Both loops now run eight at a time. With the per-client fan-out of four
that stays within the 32 concurrent node calls the other node fan-outs use.
A master-side network blip flips every node in one heartbeat tick, and
each node published its own node.down, then node.up. A notifier queue holds
64 events and the rate limiter keys on the node name, so with 150 nodes most
alerts were dropped and the rest ran into Telegram and Discord limits.
Past five same-direction transitions in one tick the heartbeat publishes a
single event per direction naming the nodes (the first ten, sorted, then
+N). Smaller ticks keep per-node events with their health data, and the
notifiers already read the node name from Source, so no formatter changed.
An operation that calls every node has to finish inside the panel's 30s
write timeout. Reset all traffic, UpdatePanels and bulk inbound delete
walked the nodes one at a time, up to 10s per hanging node, so 15 hanging
nodes out of 150 kept each request running for 2m41s while the browser
had already been told it failed.
All three now fan out through fanoutInboundResults, bounded by
nodeFanoutConcurrency (32, the heartbeat's bound), and UpdatePanels keeps
its results in request order. Bulk delete still removes the rows one at a
time, since each rewrites shared routing references, and only fans out the
node pushes that delInbound now hands back.
What the master derives from a node's reports (online clients, active
inbounds, learned sub-nodes) must live only while that node is still
synced; ClearNodeOnlineClients states it: a downed node must not keep its
clients listed as online.
Only a failed snapshot fetch cleared the online set, and only a failed
probe cleared sub-nodes. A disabled node (both jobs skip it), a node marked
offline before the sync tick reached it, a deleted node, and a node whose
snapshot fetched but failed to merge all kept their clients online in
onlineClients, onlineByGuid and activeInbounds, which the dashboard and a
parent master's /clients/onlines read. Disabled and deleted nodes also kept
their sub-nodes on the Nodes page until the panel restarted.
The traffic sync now keeps online sets only for enabled, online nodes in
its list, the heartbeat keeps sub-nodes only for enabled listed nodes, both
before the empty-list return, and a failed merge clears like a failed
fetch. The sync job's one-line call has no job-level test: that package
cannot install the xray process, so RetainSyncedNodeOnlineClients carries
the tested rule.
Node I/O on the traffic-accounting path must never stall accounting; the
serial writer states it ("Keep network I/O (node pushes) OUT of fn").
AddTraffic still applied the depletion UpdateInbound for every node
inbound inside the writer closure, one at a time with context.Background.
One hanging node held the single writer for each push, freezing traffic
polls, node snapshot merges and every client edit for the whole wave; a
client shared by 150 nodes expiring could hold it for tens of minutes.
The opt-in restart on client disable then ran node by node on the same
traffic job.
Remote plans now leave the writer and go through nodePushPlan and the 4s
nodePushContext, fanned out like client pushes: an offline or slow node
defers to the reconcile its dirty flag already schedules. The node restart
runs in its own goroutine, since nothing replays or waits on it.
TestTrafficDisableImmediatelyUpdatesNodeRuntime called addTrafficLocked
directly, which pinned the push inside the writer; it now calls AddTraffic
and still requires the push to have landed on return.
* fix(clients): keep a vless reverse client's handler across a re-add
RemoveUser also drops the client's reverse outbound handler, and the account
every live remove/re-add path rebuilt carried no reverse at all: buildUserAccount
read id/flow/testseed/testpre and nothing else. Editing, bulk re-enabling, quota
renewal and adding a client to an existing inbound therefore left a reverse
client able to connect but not to open its tunnel until Xray restarted, with
nothing logged. A traffic reset is the route operators hit most, since a
depleted client is removed and re-added on every renewal.
buildUserAccount now carries the tag (it accepts either the settings JSON object
or a typed client value), and the five account maps those paths build include
the client's reverse. Core chain, read from the pinned xray-core:
AddUserOperation -> User.ToMemoryUser -> vless.Account.AsAccount copies Reverse
(proxy/vless/account.go:24), and GetReverse rebuilds the handler from the stored
account's tag (proxy/vless/inbound/inbound.go:193-205).
Each path has a test that fails without its fix; the account-level test fails on
both input shapes.
* refactor(clients): drop an account map helper nothing calls
Local.AddClient and Local.UpdateUser are only reachable through runtime.Runtime,
and all four call sites of those two methods sit in a node branch, where the
runtime is a *Remote -- Remote.AddUser ignores the map and pushes the inbound
snapshot instead. So the extraction and its test covered a path no deployment
takes, the reverse key it added could never reach a core, and the previous
commit's claim that the node-push paths go through it was wrong.
The four account maps that do reach buildUserAccount are untouched. Reported by
the PR review.
A master's per-node sync must scope what it sends to the clients that node
serves, so its cost tracks the node and not the fleet. The global-usage
push already did (node_client_traffics by node_id); the 10s client-IP push
sent GetAllInboundClientIps, the whole table, to every node.
Each node's MergeInboundClientIps then created a row for every foreign
email, and its next GET clientIps echoed the whole fleet back. Its IP-limit
job only ever reads rows for its own clients, so none of it was used. With
150 nodes x 150 clients, one IP tick pushed 299 MB and pulled 264 MB, every
node held 22,500 rows instead of 150, and sync ticks grew 3.8s -> 10.2s
even at 1ms latency; the cost grows with the square of the fleet.
Both pushes now share nodeHostedEmails. After the change the same fleet
moves 2.0 MB / 1.8 MB per tick and ticks stay near 3.2s. Nodes upgraded
with foreign rows shed them within 30 minutes via pruneStaleIpRows.
Updates the docs site's dependencies, including the Fumadocs packages,
Next 16.3.5, React 19.3 and three majors: mermaid 12, vitest 5 and
pnpm 12. Two code changes follow from the bump:
- fumadocs-core 16.15.11 makes `llms().index()` return a Promise, so
the llms.txt route now awaits it; tsc rejected the old synchronous
call
- lucide-react 1.46 renamed the BookMarked icon to BookBookmark. The
old name is still exported, but lucideIconsPlugin looks names up in
lucide's `icons` map, which only has the new one, so the Reference
section lost its sidebar icon in all four locales. The build only
printed a warning.
minimumReleaseAgeExclude gains entries for the newly installed
versions.
Checked with typecheck, lint, vitest (106 tests) and a full build: no
plugin warnings, and each locale's rendered /docs page contains the
book-bookmark icon.
The core reads a dns rule's qType as a PortList, which drops a bare numeric
0 (infra/conf/common.go: `if number != 0`), and a rule with no qTypes
matches every query. A stored `"qType": 0` therefore does not target query
type 0: it drops, refuses or hijacks all DNS through that outbound.
A qType the panel writes has to be read by the core as exactly the query
types it names. Four writers broke that:
- DNSOutboundLegacyKeysFix rewrote a lone blockTypes [0] into "qType": 0,
so "block type 0" became "block everything" on upgrade.
- That seeder shipped in v3.8.0 and is recorded as done, so fixing it does
not reach installs that already ran it. DNSOutboundQTypeZeroFix spells
any stored numeric qType 0 as "0" once, protocol id matched like the core.
- The outbound form adapter turned a typed "0" into the number 0.
- The Xray template editor saves raw JSON past that adapter; the save now
applies the same rewrite.
Each writer is pinned by a test that fails without its part. The rewrite
and the repair compare policies as the pinned core builds them, and the
repair runs through runSeeders over a database whose legacy-keys seeder
already ran, on SQLite and PostgreSQL 16.
* fix(ports): refuse an inbound on a port an AmneziaWG peer forwards
checkForwardedPortsConflict only ever ran from the AmneziaWG save path, and only
in one direction: an AmneziaWG client's forwardedPorts were checked against the
ports other inbounds already hold, while the reverse -- an ordinary inbound
saved onto a port some peer forwards -- had no guard at all. The forward
listener binds that port on every interface in both directions
(amneziawgnet/portfwd.go's attachTCP/attachUDP), so the two listeners want the
same socket: the loser either leaves the peer's forward silently dead or fails
the inbound's listen.
checkPortConflictTx now resolves that owner the same way the relay-slot checks
do -- same host, peers derived from the stored settings with the shared
InstanceFromInbound -- and names the peer in the refusal. Sitting inside
checkPortConflictTx covers both the save and the enable path added in #6549.
TestAddInboundRefusesAPortAnAmneziaWGPeerForwards fails without this -- watched
red, the create is allowed -- and its node-row companion pins the scoping that
keeps a node row legal on a locally forwarded port.
* fix(ports): name only a peer that binds as the owner of a forwarded port
The owner lookup read instance.Peers and ForwardedPortsInclude directly, so a
peer the forward supervisor skips (no email, or no address the tunnel routes
to) was reported as holding a port nothing binds -- refusing a create that is
legal with a message naming a row whose own port is its WireGuard one. It also
repeated the candidate's listen address as the forward's location, though the
forward binds :port on every interface.
Share the supervisor's own gate through amneziawgnet.ForwardedPortOwner, report
the wildcard bind, and propagate a failed owner query instead of reading it as
"no conflict", matching the sibling checks in the same file.
* style(ports): keep the forwarded-key doc block within the 2-line cap
The reworded desiredPortForwardKeys doc ran to three lines, against the rule
this repo sets for committed Go comments.
* fix(limit-ip): leave a reverse client out of the temporary disconnect
The LIMIT_IP cycle removes the client and adds it back 100 ms later. For a vless
client carrying a reverse config that is not reversible: RemoveUser calls
RemoveReverse and deletes the client's outbound handler, while the account added
back is built without the reverse field, so the tunnel stays down until Xray
restarts and the core's forward-proxy guard for that client no longer fires
(proxy/vless/inbound/inbound.go:245 and :542-544 at the pinned core). The cycle
now skips such a client and says so, instead of trading a limit violation for a
tunnel that needs a restart to come back.
TestDisconnectClientTemporarilySkipsReverseClient fails without this -- watched
red, the client is removed and re-added -- and asserts the skip is logged rather
than silent.
* style(limit-ip): keep the reverse-client comment within the 2-line cap
The block explaining why a reverse client is skipped was three lines, against
the rule this repo sets for committed Go comments; the same why fits in two.
* fix(panel): accept 2FA codes from adjacent TOTP windows
CheckUser compared only gotp.Now(), so a code submitted at the end of
its 30s window (or with slight client/server clock drift) failed with
'invalid 2fa code', while the immediate retry in the next window
succeeded. Accept current +/-1 window, the standard TOTP skew
tolerance.
Fixes MHSanaei/3x-ui#6535
* fix(panel): share TOTP skew tolerance with VerifyTwoFactorCode
Move the +/-1 window helper to internal/util/totp so both 2FA
acceptance points use it: login (CheckUser) and disable/rebind plus
username/password changes (VerifyTwoFactorCode). Also shrink comments
to the 2-line house rule and anchor the unit test mid-window to avoid
a step-boundary flake.
Addresses review on #6546 (MEDIUM + 2 LOWs).
---------
Co-authored-by: sdhfsl <sdhfsl@users.noreply.github.com>
* fix(xray): restart when a diff strands a client's live session
Disabling or deleting a client took it out of the generated config and the
hot path applied that with AlterInbound/RemoveUser, which only drops the
credential (vless, vmess, trojan and shadowsocks all keep the established
session running) -- so the panel showed a disabled client whose connection
kept passing traffic, and the core offers no API to close one session.
A diff that removes a user without re-adding the same email under the same
tag is that case: honour the operator's restart-on-client-disable setting and
let the caller replace the process, which is already how an auto-disabled
client loses its session. An edit re-adds the email and keeps the hot path.
* chore(i18n): cover manual disable and delete in the restart-setting description
The setting now also decides what happens when a client is disabled or deleted
by hand, so the description cannot keep naming only the automatic path. All 13
locales updated in the same commit to keep the wording consistent.
* fix(xray): reach the guard from the manual switch and from every protocol
Round-1 findings on this PR. The guard sat in tryHotApply, but a manual disable
or delete applies through runtime.Runtime and finishes with needRestart false,
so none of the three RestartXray schedulers fired and the predicate was never
reached: the session in #6533 kept flowing. The apply layer now asks for the
restart the setting promises when the client actually leaves the config, on the
single-client update and delete paths and on bulk disable, and only for local
inbounds so a node row cannot make the master restart its own core.
The predicate itself could not fire for shadowsocks or hysteria either, because
RemovedUsers is only produced for the protocols diffInboundUsers will diff. The
diff now also compares settings.clients of an inbound present in both configs,
which is the one shape every account list shares, so those protocols reach the
guard through the inbound instead of through nothing.
TestManualClientDisableHonoursRestartSetting fails without the apply-layer fix
("needRestart = false, want true" with the setting on) and
TestHotDiffDropsUsersOnProtocolsItCannotDiff fails without the diff fix -- both
watched red. The two three-line comments this PR added are back inside the cap.
* docs(i18n): stop scoping restartXrayOnClientDisable to auto-disable
The setting now covers a client disabled or deleted by hand as well, so its
title no longer says "Auto" in all 13 locales, and the docs callouts in en, ru,
zh and fa describe the same behaviour instead of the auto-only one.
* docs(limit-ip): correct what the temporary disconnect can actually do
The comment claimed removing and re-adding a user "disconnect[s] all
connections". RemoveUser only clears the core's credential validator in vless,
vmess, trojan, shadowsocks and hysteria alike, so a session already up keeps
running and the fail2ban ban on the logged IP is what ends the traffic. Comment
only: the protocol gate and its test are untouched.
* docs(limit-ip): say what the disconnect cycle really does per protocol
* perf(nodes): reuse one pooled client per node instead of rebuilding it
The heartbeat probe asks for a client every 5s per node, and for skip, pin and
mtls modes HTTPClientForNode built a client with its own transport each time:
every tick paid a full TCP+TLS handshake per node, which is the CPU a 100-node
fleet reports. Cache the client per node identity, close the previous one when
that identity changes, and raise the idle pool caps above any real fleet size
so a node's connection survives to its next tick.
* perf(nodes): keep one client per node in the pooled cache
Round-1 findings on this PR. The eviction dropped only entries whose key did not
start with the current identity, so every proxy variant of that identity stayed
for the life of the process. That variant is often a fresh loopback port:
withOutboundBridge mints one per call and tears the bridge down on return, so
each operator "test node" or remote-inbounds action added a client whose key can
never be hit again, and a node switched to verify mode orphaned its old entry by
returning before the loop. Replacing that filter with one entry per node bounds
the cache at the fleet size, and the verify-mode return now clears the node too.
TestHTTPClientForNodeKeepsOneClientPerNode fails without this -- watched red,
"2, want 1" -- and pins the verify-mode cleanup on the same cache.
* style(nodes): keep the eviction comment inside the two-line cap
* fix(inbounds): check ports when an inbound is enabled, not only when it is saved
The save-time guards compare enabled rows, so a row could be created while
another disabled row held its port and only collide once the disabled one was
switched on. Run the same checks before the flag moves: the refusal names the
row that owns the port, the flag is left alone, and tcp/udp coexistence and
node rows keep working.
* docs(inbounds): state the real reason the enable path needs its own check
* fix(xray): refuse a config the running core cannot bind
RestartXray stopped a working core before handing it a config whose listens
collide, so the failed bind exited the whole process (main/run.go:94) and the
one-second watchdog retried it in a loop: every protocol down, cause only in
the logs. The save-time port guards cannot cover this -- SetInboundEnable, the
AmneziaWG relay created on the first peer, template and bridge edits all reach
a colliding config with no guard on that path.
Probe the generated config at the single restart funnel instead. Collisions the
running core already serves are excused, so an established setup is never
refused by a static read being wrong about it, and the port-bucketed pass costs
nothing on a clean config.
* fix(xray): surface a refused config and re-key the bind excuse set
Round-1 findings on this PR. Refusing the swap left the running core on its
previous config with nothing but a log line to show for it, so the status
response now carries the reason while the core runs and the overview marks it;
the node list picks the same field up through that response. The excuse set is
keyed on the two listens, the port and the shared transports instead of the tag
pair, so a pair whose listen moves onto the other's address is refused again,
while the same two sockets stay excused however the generator orders them.
TestBindConflicts/excused_pair_whose_listen_changed_into_a_real_collision fails
without the key change -- watched red first.
* fix(amneziawg): refuse a WireGuard port that is the row's own relay port
All three relay checks filter themselves out of the candidates with id !=
ignoreId, so nothing ever compared an AmneziaWG row's own WireGuard listen port
with the relay port its own id derives. Saving a row on that exact port left the
embedded device (UDP on the inbound's listen address, amneziawgnet/device.go:137)
and its injected relay (TCP and UDP on 127.0.0.1, amneziawgnet/relay.go:47-61)
bound to the same UDP port, so whichever loses the race dies -- and when the
relay loses it, Xray refuses the whole config and takes every other protocol on
the host with it. The first AmneziaWG inbound on port 65101 was enough to reach
it: id 1 derives exactly that port.
The row now states the rule its three siblings do: it owns the slot its id
derives. A node-hosted row still keeps its own port, since it binds no relay on
this host.
TestAddInbound_AmneziawgRefusesItsOwnRelayPort and
TestUpdateInbound_AmneziawgRefusesItsOwnRelayPort fail without this -- both were
watched red first -- and pin the two separate call sites, AddInbound's post-Save
block and checkPortConflictTx's ignoreId > 0 block.
* fix(amneziawg): keep a disabled row's relay port reserved for port forwards
loadPortConflictContext filtered its query with enable = true, so a client's
ForwardedPorts spec could claim the relay port a disabled AmneziaWG row's id
derives. That row's relay appears with its first client -- a path that runs no
port check -- and when the relay then loses the loopback bind race to the
forward listener, Xray refuses the whole config instead of losing one forward
(#6542 review, arrived with #6540).
The context now loads every local row and gates only the ordinary-port compare on
enable, which is what a disabled row's own port is worth: free. Its relay slot is
not free, which is the rule #6540 already states for the other two guards.
TestCheckForwardedPortsConflict_DisabledAmneziawgRelayPortIsReserved fails
without this -- watched red first -- and passes with it, while
TestCheckForwardedPortsConflict_IgnoresDisabledInboundPort keeps proving that a
disabled inbound's own port stays available.
* fix(amneziawg): re-run the forward guard once a new row has its own ports
normalizeAmneziaWGSettings validates every client's ForwardedPorts before the row
is saved, and loadPortConflictContext then reads the database -- so the new
AmneziaWG row is never a candidate for itself. A client could forward exactly the
relay port the row's own id derives, or its own WireGuard listen port, and the
create was accepted: at runtime the panel's wildcard forward listener and Xray's
127.0.0.1 relay race for the same port, and a lost relay bind makes Xray refuse
the whole generated config (#6544 review, pre-existing).
The post-Save block is the only place the id is known, so it re-runs the guard
there. Both callers now share amneziaWGForwardedPortsConflict, so the collision
message lives in one place instead of two.
TestAddInbound_AmneziawgRefusesAClientForwardingItsOwnRelayPort fails without
this -- watched red first -- and passes with it.
* fix(amneziawg): stop blocking stored forward specs on a disabled row's slot
Round 2 flagged this PR's widening as the one MEDIUM it introduced, and the code
confirms it: UpdateInboundClient carries a stored ForwardedPorts spec forward for
a partial edit (client_inbound_apply.go:763-765) and re-validates it (:772 and
:909), so after an in-place upgrade an edit that never submitted the field -- a
bot enable/expiry toggle -- is refused over a slot the operator did not touch,
for a relay injectAmneziawgnetSocks does not emit while the row is disabled. The
inbound-save path re-validates every stored spec the same way.
The trade does not pay for itself: the slot this reserves is claimable only by a
spec an operator authors onto 65101-65535, while the cost lands on unrelated
operations. The precise fix -- refuse a newly claimed spec rather than a stored
one, and check the enable transition in SetInboundEnable, where the conflict is
actually created -- is larger than the hole, so the slot goes back to a
documented pre-existing item with its own follow-up.
The create-path re-run added in 80eb5712 is unaffected: it reads the settings
submitted in the same request, so it never refuses a stored value, and its test
still passes.
* test(amneziawg): pin that a peerless inbound still owns its relay port
checkAmneziawgnetSocksConflict skips a candidate whose settings yield no
qualifying peer, and normalizeAmneziaWGSettings writes Clients: [] for a fresh
AmneziaWG inbound -- so a newly created row reserves nothing, an ordinary
inbound can take its derived port, and adding that row's first client then puts
two inbounds on 127.0.0.1:65101. The client paths run no port check.
Expected red on this head; the fix follows.
* fix(amneziawg): reserve the relay port before the first peer is added
checkAmneziawgnetSocksConflict skipped a candidate whose settings yield no
qualifying peer (amneziawg.InstanceFromInbound), and normalizeAmneziaWGSettings
writes Clients: [] for a fresh AmneziaWG inbound. A newly created row therefore
reserved nothing, an ordinary inbound could be saved onto the port that row
derives, and adding its first client generated the relay next to it: two inbounds
on 127.0.0.1:65101, which makes Xray refuse the whole config and take every other
protocol on the host down with it. Nothing re-checked it later either -- only
AddInbound and UpdateInbound run checkPortConflictTx, and the client paths that
create the first peer run no port check at all.
Ownership now follows the row, so the check states the same rule as its two
siblings, which key on protocol and node_id IS NULL alone. The amneziawg import
goes with the guard.
TestCheckPortConflict_AmneziawgnetSocksRelayReservedBeforeTheFirstPeer fails
without this, on a test-only head whose go-test run failed on exactly that test,
and passes with it.
* docs(amneziawg): stop the forward check's doc block claiming every row gets a relay
Round-1 LOW: the block's justification clause read "every one of them gets a
relay inbound", which is false for exactly the rows this change newly reserves
for -- injectAmneziawgnetSocks skips a row with no peer email, and that is the
row whose port must stay reserved. A reader following the cross-reference landed
on the guard this branch removes and read it as the rule.
Replaced by the two facts that are true, which also brings the block under
CLAUDE.md's two-line cap instead of twelve lines over it. The peerless reason
stays where it is load-bearing, in the two-line comment above the candidate loop.
* test(amneziawg): pin that a disabled row still owns its relay slot
checkAmneziawgnetSocksConflict filters enable = true, so a disabled AmneziaWG
row is not a candidate when an ordinary inbound's configured port is validated.
SetInboundEnable then flips the column with no port check, so enabling that row
later puts a second inbound on 127.0.0.1:65101 and Xray refuses the whole config.
Expected red on this head; the fix follows.
* fix(amneziawg): count a disabled inbound as owning its relay slot
The forward port check filtered its candidates with enable = true, so a disabled
AmneziaWG row was invisible when an ordinary inbound's configured port was
validated. Nothing else covered the gap: the relay is not a database row, and
SetInboundEnable flips the column with no port check, so re-enabling that row put
a second inbound on 127.0.0.1:65101 and made Xray refuse its whole config,
taking every other protocol on the host down with it.
A row owns the slot its id derives for as long as the row exists, which is the
rule the reverse-direction check already follows. TestCheckPortConflict_
DisabledAmneziawgStillOwnsItsRelaySlot fails without this, on a test-only head
whose go-test run failed on exactly that test, and passes with it.
* test(amneziawg): drop the disabled-row case that asserts the reversed rule
TestCheckPortConflict_AmneziawgnetSocksRelayIgnoredWhenDisabled stated, in its
name and its doc comment, that a disabled AmneziaWG inbound's port must not
block anything -- the rule the parent commit reverses. It also never reached the
predicate it named: its fixture seeds Settings: {}, which
amneziawg.InstanceFromInbound rejects on parsed.Server == nil one statement
before the enable column is read, so it passed with or without the filter.
Leaving it would document both rules for the same operator state with nothing
failing to flag the contradiction. The rule this PR pins is covered for real by
TestCheckPortConflict_DisabledAmneziawgStillOwnsItsRelaySlot, whose fixture
carries a qualifying server block and an enabled peer.
* fix(amneziawg): wrap the relay port window instead of refusing ids past it
An AmneziaWG inbound's loopback relay port is SOCKSBasePort + row id, and
AddInbound refused any id that pushed it past 65535. The inbounds table is
AUTOINCREMENT, so an id is never reused and the counter is only reset when the
table empties: the 435-port window was a lifetime budget, and a database that
had ever created more inbounds could never create another AmneziaWG one --
the reporter's counter sits at 70350, so the protocol never worked there at all
(#6537).
Ids now wrap into the same 435 ports, which leaves every id up to 435 with the
exact port it had, so no existing row, relay or generated config moves.
Wrapping makes the id -> port map non-injective, and nothing compared two
derived relay ports before -- two relays on one port would leave Xray with a
duplicate listen and refuse to start, taking the whole panel's proxy down.
checkAmneziawgnetSocksRelayCollision now refuses a create or an edit whose
derived port another local AmneziaWG row already owns, disabled rows included:
a row owns its slot for good, and enabling it later re-runs no port check.
* test(amneziawg): give each relay-window fixture its own client email
Every fixture built the same client email, and an email is unique across the
whole panel, so AddInbound refused the second create with "Duplicate email"
before either new guard ran -- CI exercised neither the wrap nor the collision
refusal. Each fixture now derives its email from its own tag, which is what the
tag already exists for.
* fix(amneziawg): say relay port in the relay conflict message
A refusal that named the port of the automatic loopback relay read as if the
named inbound listened on an unrelated port -- its own port is the WireGuard
one. portConflictDetail now carries Relay, and both messages that report a
derived relay port say "relay port N"; messages that report a configured port
render byte-for-byte as before.
* test(amneziawg): pin that a node-assigned inbound owns no relay slot
A row adopted from a node carries a NodeID and the protocol it arrived with
(inbound_node.go:737), yet injectAmneziawgnetSocks skips it, so it binds no
loopback relay. The gate this PR added to checkPortConflictTx never looked at
NodeID, so editing such a row can be refused for a slot it does not own.
Expected red on this head; the fix follows.
* fix(amneziawg): skip the relay guards for node-assigned inbounds
Round-2 review finding: the gate this PR added to checkPortConflictTx keyed on
inbound.Protocol alone, so it also ran for a row adopted from a node. Such a row
carries a NodeID and gets no loopback relay -- injectAmneziawgnetSocks skips it
and the desired-instance query is node_id IS NULL -- so it owns no slot and can
collide with nothing, yet editing it was refused with "relay port N ... already
used by inbound '<local>'", naming a port the edited row never binds.
Wrapping made this visible: before it, an adopted id above 435 derived a port
above 65535 that no row could hold, so the pre-existing reverse check under the
same gate could not fire.
Both call sites now require NodeID == nil, matching the local-only predicate the
forward check already used. TestCheckPortConflict_NodeAssignedAmneziawgOwnsNoRelaySlot
fails without this, with the exact false refusal, and passes with it.
* fix(amneziawg): read the outbound pseudo-protocol id like the core
IsAmneziaWGOutbound compared the id exactly while every reader around it does
not: the probe lane already reads the same id with strings.EqualFold
(outbound/probe_http.go, pinned by TestBuildBatchTestConfigReadsTheProtocolIDLikeTheCore),
and the core lowercases a protocol id before it resolves the handler.
A template entry spelled "AmneziaWG" therefore stayed unbridged in two paths.
transformAmneziaWGOutbounds skipped it and handed the raw pseudo-protocol to
the core, which answers "unknown config id: amneziawg" -- Xray then fails to
start, since bridging is what makes that entry a socks outbound. The amneziawg
job skipped it too, so the reconcile loop never created the instance and the
outbound silently carried no tunnel.
The exact comparison also made the save path answer two ways for one spelling:
CheckXrayConfig routed the exact match to the panel's own validator and the
case variant to the core's, so the operator was told the core does not know a
protocol the panel implements (probe output, before: `xray core rejects
outbound "t1": infra/conf: unknown config id: amneziawg` for "AmneziaWG" and
`amneziawg outbound "t1": privateKey is required` for "amneziawg"; after: the
panel's own message for both).
Reachable only from a template that did not come through the panel's save,
which rejects the case variant today -- a restored backup, a direct DB edit, a
scripted template, or a legacy DB. That is the same class of data the
UppercaseFreedomFinalRulesFix seeder exists to repair, so the panel already
treats non-lowercase protocol ids as real operator input.
strings.EqualFold is the whole change; the package already imports strings.
* style(service): trim the amneziawg outbound test comment to two lines
The review flagged the three-line block: CLAUDE.md caps a committed Go
comment block at two lines and the test name already carries the what. The
remaining two lines keep the why — the core folds the id's case before
resolving it, so a mixed-case spelling must bridge here too.
The core lowercases an outbound's protocol id before it resolves the handler,
so an outbound spelled "Loopback" still is the loopback outbound. Both
readers that keep a loopback outbound's inboundTag in step with the inbound
it names compared the id exactly, so such an outbound was skipped: renaming
or deleting that inbound left settings.inboundTag pointing at a tag that no
longer exists, and traffic returning through the loopback outbound arrives
under a tag no routing rule can match (infra/conf/loopback.go:15 carries the
tag, proxy/loopback/loopback.go:43 uses it as the inbound identity).
The probe lane's "nothing to test here" gate had the same exact comparison,
so a "Freedom"/"Blackhole" outbound reported the vaguer "No testable
endpoint" where the canonical spelling reports "Outbound has no testable
endpoint" — the two spellings took different paths to the same rejection.
Both readers now compare case-insensitively; the outbound package reuses its
existing equalsAnyFold helper rather than adding a second one. The service
reads the config template an operator edits, so a case variant is reachable
there; server.go's GetDefaultLogOutboundTags scans the embedded config.json
instead, whose protocols are canonical by construction, so it is left as is
and no test can tell a case-insensitive read there from an exact one.
Update Ant Design, React i18n, Zod, testing utilities, Oxc tooling, GORM Postgres, Pion transport, and sing dependencies to their latest specified versions.
* fix(panel): read the outbound protocol id in the address column like the core
outboundAddresses switched on the raw id, so a row the core runs normally but
spelled "VMess", "Trojan" or "WireGuard" fell through to default and rendered
an empty Address column in the outbounds table, the card view and the
subscription table -- a populated server that looks absent, which is what
sends an operator to recreate a correct outbound.
The id is folded once before the switch, the way isUdpOutbound already folds
the transport name.
* fix(panel): fill the outbound address column from one protocol-id rule
outboundAddresses folded the id inline while isUntestable, two functions
below it in the same file, reads it through isOutboundProtocol — so the
"the core lowercases the id" rule lived in two places and two tests. It
now routes through the shared helper, which keeps the rule with the
module that owns it.
Two further gaps in the same switch, reported in the same review:
hysteria and amneziawg are both selectable in the outbound form but had
no case, so a canonically spelled row rendered a blank Address cell that
case folding could not reach; and the VLESS branch returned a bare ":"
for a row whose servers sit in vnext, which this change newly reached
for a "VLESS" spelling.
Tests: the hysteria/amneziawg cases and the bare-separator case are red
on the pre-fix switch.
* fix(panel): read the vnext shape of a vless outbound in the address column
The vless branch read only the flat settings.address/port, so a row whose
servers sit in vnext — the shape the probe's extractor reads first
(internal/web/service/outbound/outbound.go:259-269) — rendered a bare ":"
separator, or nothing at all before this branch folded the id. It now
reads vnext first and falls back to the flat pair, the order the
extractor uses, which also makes it agree with what a probe of that row
would say.
Test: "reads the vnext server of a vless row" is red on the pre-fix
branch.
* fix(panel): read the protocol id of the outbound stream tags like the core
The identity cell gated the network and security tags on an exact-match
includes() over four ids, so the same "VMess" row whose address this
branch now shows still rendered without its ws/tls tags — the row was
half-readable. It now asks the shared isOutboundProtocol, the rule every
other reader on the page uses.
Test: "renders the stream tags and the address of a VMess row" is red
without this change (['VMess'] vs ['VMess','ws','tls']).
A direct, DNS, loopback or blackhole outbound is not a proxy, so the probe
must reject it instead of measuring the panel host's own reachability. The
gate compared the protocol id exactly while the core lowercases it in
LoadWithID before resolving the handler, so "Freedom" and "DNS" were not
recognised: the HTTP probe ran through the direct outbound and returned
Success=true with a full egress block, and the row's Test button stayed
enabled because isUntestable compared exactly as well. The operator reads the
panel host's own country and delay as a working tunnel.
The batch gate now folds the id once before its switch, and isUntestable goes
through the shared isOutboundProtocol helper.
