classifySMTPError returned keys already carrying "pages.settings.", while the
four keys TestConnection returns directly do not, and the alert renders every
Message under that one prefix. Any classified failure therefore looked up
pages.settings.pages.settings.smtpErrorAuth, which does not exist, so the panel
printed the key instead of "Authentication failed — check username and
password". The unknown case was worse: it appended the raw error to the key, and
its own text interpolated {{ .Error }}, Go template syntax the frontend's
i18next never fills.
Return the keys unprefixed like the rest, and point the unknown case at the
panel log, which already carries the underlying error.
Xray ignores port for DoH/DoHL/DoQL, so valuesToWire deliberately stores none
for an encrypted address and a non-standard port has to go inside the URL. The
form kept offering the field anyway, pre-filled with the 53 from its own
defaults: a port typed there was dropped on save and redrawn as 53 on reopen,
which reads as the panel losing the value.
Render the port field only where it is actually stored. DoT keeps it, since
tls:// is not an encrypted-address scheme for this purpose.
Closes#6403
Two saves that the panel UI never makes, but the documented REST API does.
A client whose allowedIPs normalized to empty passed validation, then
InstanceFromInbound skipped the peer and dropped the whole instance when it was
the only one. Nothing logged it, so an enabled inbound simply never opened its
socket. Refuse an enabled peer with no address, naming the client, the way the
injection and collision checks already do.
The server keypair was regenerated whenever a payload omitted privateKey, which
invalidates every client config already distributed, and a payload carrying only
privateKey left publicKey empty so rendered configs got a blank "PublicKey =".
An omitted key now means unchanged: the stored pair is carried forward, a
half-supplied pair has its public half derived, and generation is reserved for
an inbound that has no stored keys at all. UpdateInbound loads the stored row
before normalizing so those keys are available.
Closes#6407
A Host row may set Security to tls on an inbound whose own stream is Reality.
The emitted link then carried security=tls next to pbk, sid, spx and the
Reality dest as sni: the endpoint no longer performs a Reality handshake, so
those describe a server the client will never reach, and clients that honour
them fail to connect. Only the security key was rewritten at emit time, and the
existing strip covered alpn/sni/fp/pcs for forceTls=none alone.
Clear the Reality-only parameters before the endpoint's own TLS overrides are
applied, so a host that supplies its own sni or fingerprint still wins.
Closes#6424
A client attached to several WireGuard/AmneziaWG inbounds is that many
independent peers, each with its own keypair, preshared key and tunnel address.
The client edit form can only represent one peer, so Update's per-inbound loop
stamped that single field set onto every attached inbound: every peer ended up
with identical keys and one inbound's address, and the tunnels on all the other
nodes stopped working with no way to recover the overwritten values from the
panel. The only guard covered AllowedIPs, and only for AmneziaWG.
When more than one tunnel inbound is in scope and the caller sent no per-inbound
override, clear the shared peer fields so UpdateInboundClient's existing
carry-forward preserves each inbound's own. A scoped update (?inboundIds=) still
narrows to one inbound and edits it normally.
Closes#6372
UpdateInbound restores the stored NodeID before the node-eligibility check, so
the payload can never introduce an assignment there — the check could only ever
fire on a row that already had one. A node's MTProto inbound arrives on the
master by adoption, which does not go through that check, so every later edit of
it was refused with "mtproto inbounds cannot be assigned to a node". That made
the share address of a node-managed MTProto inbound impossible to change from
the panel that generates its subscription links.
Refuse only a protocol change into an ineligible protocol, which is the one way
an update can still strand a row the master's sidecar loops would never
reconcile.
Closes#6415
Deleting a client tombstones its email for 90s so a node snapshot captured
before the deletion cannot resurrect it. Nothing withdrew that tombstone when
the operator re-created the same email, so on a master with at least one node
the next merge filtered the live client out of the snapshot and SyncInbound
pruned its inbound link. The client reappeared only once the tombstone expired,
which is the 90-120s detach window reported.
Withdraw it on a successful create, single and bulk, so a tombstone can never
outlive the identity it was meant to bury. A failed create still leaves it
standing, which is what keeps the stale-snapshot guard intact.
Closes#6370
A Clash "server" is a bare host, not a URI authority, but the custom share
address strategy stores an IPv6 literal with brackets so the address normalizer
can hand it to the raw link generators. The Clash renderer copied that value
into every proxy verbatim, so mihomo received server: "[2001:db8::1]" and
failed to parse the node. Raw links were unaffected because joinHostPort strips
the brackets and re-adds exactly one.
Strip them once where the renderer takes the resolved dest, which is the single
place all three proxy builders read the address from.
Closes#6373
REVIEW.md told the reviewer which repository rules to check but never to try
breaking the change, so the conditions this panel actually meets went
unexamined. "Try to break it" adds six, each tied to a mechanism here rather
than to a generic checklist: an upgrade over an operator's existing rows and
the rollback that reads them again, a restart that drops in-memory state under
the cron jobs, a sub-node racing the master on the same row, an operation
applied twice, an inbound or client at the empty and the thousand end, and a
dependency that is down. It closes with the gate that running a case is not
reporting it - each one still has to clear the verification bar below it, so
the section cannot become a licence for hypotheticals.
repo-context.md said nothing about which bot reads it. Only the issue analyst
does, since the review job's briefing moved inline in acf3603d, so it becomes
issue-analyst-context.md and its title names the analyst instead of "the Claude
bot". bot_context_test.go pins that path in a constant, so the rename carries
through the constant, the four test names and the two comments that named the
old file - one of which still said "the bot prompts", plural.
Backticks come off mtg-multi in the new section: the same test file reads any
hyphenated backticked token in REVIEW.md as a CI job name, and fails on one
ci.yml does not define.
The review job ran the official code-review plugin, which fans a pull
request out to five Sonnet reviewers plus a Haiku scorer per finding and
drops everything scored under 80, and the briefing file spent most of its
lines overriding that plugin. Both are gone: the job hands one Senior
Software Engineer prompt to the action inline, the way the issue analyst
does, and denies the Agent tool so the single role is mechanical rather
than a request.
REVIEW.md moves from the emoji markers to CRITICAL/HIGH/MEDIUM/LOW with a
pre-existing qualifier. The uncapped rule is scoped to findings the pull
request introduced or worsened so it cannot collide with the cap of three
pre-existing ones. "A finding is a report, not a patch" stays as it was.
Workflow housekeeping: GH_TOKEN, REPO and PR live in the job env instead
of six step copies; the skip gate is per pull request, so a head pushed
after the automatic review is reviewed only on @claude review; the comment
counters sum gh's per-page jq output, which read "0\n0" as a review on a
pull request with more than 100 comments; --max-turns rises to 300 because
every read now costs the single agent a turn instead of a subagent.
Bump frontend and Go dependencies, then modernize a few hot paths with newer Go string/range helpers. Also widen the client form quota/limit fields to improve the layout.
Bump xtls/xray-core to 37ceb8b4b6 (v26.9.8) and the three binary pins
(DockerInit.sh, release.yml Linux + Windows) in lockstep. No deleted
symbols; the impact is entirely on the JSON config surface.
Outbound "proxySettings" is now refused by the config loader (moved to
streamSettings.sockopt.dialerProxy) and a freedom outbound rejects
sockopt.addressPortStrategy. Either key in a stored template would keep
the core from starting after the upgrade, so a new OutboundRemovedKeysFix
seeder rewrites xrayTemplateConfig once: proxySettings.tag becomes
sockopt.dialerProxy (an existing dialerProxy wins) and addressPortStrategy
is dropped from freedom outbounds. Template saves and outbound
subscriptions already run through the vendored loader, so the new
refusals surface there with the core's own message.
REALITY no longer applies a built-in minClientVer (26.3.27) when the
field is empty. The form placeholder and the min/max hints in all 13
locales now say that empty means no minimum.
New upstream keys the Zod schemas would otherwise strip, with form
support where a sibling field already had it:
- blackhole response type "custom" with base64 customResponseData
- realm finalmask ipMode (dual/v4/v6) and portMapping (UPnP / NAT-PMP)
- quicParams brutalDisableLossCompensation, disableChromeParrot,
disableGSO, disableStatelessReset
- hysteria masquerade proxy xForwarded
- wireguard outbound remoteDNS
- routing rule localOS
freedom.domainStrategy is only deprecated upstream (auto-migrated to
sockopt.domainStrategy with a warning) and is left untouched.
An operator with several nodes reported that editing a client or resetting
its traffic takes more than ten seconds on the master. Measured against real
Remote HTTP (fake node servers, one client per node), the healthy case is
already fast — 3 nodes: update 51ms, delete 51ms; 5 nodes: 102ms / 103ms —
but two things were not:
- ResetTrafficByEmail still walked its inbounds one node round-trip after
another: 152ms at 3 nodes, 253ms at 5, linear in node count.
- Every per-client op blocked on the SLOWEST node's push. With one node
answering in 3s, update/delete/reset all took 3003ms regardless of node
count. A node that answers the 4s heartbeat probe but hangs on the push
stays "online", so every edit waited on it up to remoteHTTPTimeout — the
ten seconds in the report. More nodes only raise the odds one is sick.
The push is an immediacy optimisation, not the source of truth: every one of
these ops calls MarkNodeDirtyTx inside the transaction that commits the
change, before it pushes, and the node reconcile job converges a dirty node on
its next 5s tick by re-sending the inbound whose fingerprint was not advanced.
So bound the synchronous push with nodeClientPushTimeout = 4s — the budget the
heartbeat and traffic-sync jobs already treat as "responsive" — at the eight
node-branch push sites. A node that does not answer in time is left dirty and
converged a few seconds later instead of stalling the request; the tag-cache
list fetch inside resolveRemoteID shares the same budget.
Once one push in a batch has timed out, the rest of that inbound's batch now
stops pushing too, as AddInboundClient already did: the node is dirty and one
reconcile converges the whole inbound. Deleting three clients on one hung node
went from 30.08s (three remote timeouts) to 4.06s; at the 32-client push
threshold that is 128s of deadlines saved per inbound.
Fan the reset out through fanoutInboundApplies like the other client ops. Its
node propagation is still attempted whatever the node's status flag says, as
before, because nothing replays a traffic reset — the reconcile pushes inbound
config, not counters — so a node still serving after being marked offline must
receive it now or never.
Trade-offs stated plainly: a node that would have answered in 4–10s now falls
to the reconcile's full-inbound push, which on the node is a delete+add of the
inbound and drops its sessions there — the same fallback a failed 10s push
already used, now reached sooner. The reset stays best-effort with no retry
path, which predates this change. The response still reports success while a
timed-out node catches up; the pending-node badge is keyed off node status by
design, so only the warning log records it.
Tests: a barrier test that a sequential reset cannot satisfy; two tests against
a real runtime.Remote and an httptest node that hangs on the push, pinning that
an edit returns at the deadline (exactly one push reached the node, the node
is left dirty) and that a bulk delete stops after its first timed-out push.
All red without the change; the two hung-node tests pay their 4s deadline on
every run.
63b46cd6 made a multi-inbound client create apply its inbounds concurrently,
and d34ec97f did the same for the single-client update, delete and detach. The
bulk operations were never converted, so they still walked their inbounds in a
plain sequential loop with a node RPC in each iteration — and those are what the
panel actually calls for a multi-select delete or an enable/disable, which is
why editing and deleting still felt slow on a master with several nodes.
Measured with a node runtime injecting 100ms per RPC, one client per node:
nodes=1 update=101ms bulkSetEnable=101ms bulkAdjust=101ms bulkDelete=101ms
nodes=3 update=102ms bulkSetEnable=302ms bulkAdjust=304ms bulkDelete=303ms
nodes=5 update=203ms bulkSetEnable=504ms bulkAdjust=504ms bulkDelete=504ms
after, all of them track the single-client ops:
nodes=3 bulkSetEnable=103ms bulkAdjust=101ms bulkDelete=101ms
nodes=5 bulkSetEnable=202ms bulkAdjust=202ms bulkDelete=202ms
Generalize the fanout into fanoutInboundResults over an arbitrary per-inbound
result type and route six loops through it: BulkDelete, BulkSetEnable,
BulkAdjust, BulkDetach, BulkAttach, BulkCreate, plus applyClientFieldByEmail —
the field edit behind the Telegram bot's enable/limit/expiry buttons and the
LDAP job. Each keeps its preparation sequential and overlaps only the node
pushes, inheriting the same concurrency cap and per-inbound panic recovery.
Two ordering details the sequential loops got for free and the fanout must do
itself: the three loops that ranged a map now walk sortedInboundIds, so which
inbound wins a per-email skip reason is the lowest id instead of whatever the
map yielded; and BulkAttach de-duplicates a repeated inbound id up front,
because the second pass used to see the client the first pass had just added.
The allocating paths stay serial when a tunnel inbound is involved. WireGuard
and AmneziaWG pick a free peer address by reading every inbound's used-set
before they write, so two overlapping allocations hand out the same address and
the in-transaction re-check refuses the loser — a bulk create of two clients
onto two wg inbounds returned created=1. addFanoutLimit drops those batches back
to one at a time; every other protocol keeps the full cap.
Eight tests: seven barrier tests that a sequential caller cannot satisfy (peak
pushes in flight is 1 without the change, 4 with it), and one that pins the
tunnel allocation.
A client edit fans out one transaction per inbound, and each one renames the
single shared clients row but calls MarkNodeDirtyTx for only its OWN node. So
between the first and the last commit the record already carries the new email
while every other node hosting that client is still config_dirty = false.
setRemoteTrafficLocked gates the snapshot merge on that flag, so a merge landing
in the gap is accepted, sees a pre-rename snapshot, finds no record for the old
email and inserts one through syncInboundClients' CreateInBatches — the only
place in the panel that creates a client record. The ghost is never in any later
merge's perInboundOld, so markSyncOrphan never fires and ReapSyncOrphans never
collects it: the operator is left with a permanent second client under the old
name. The same stale merge reverts an expiry-only edit instead of duplicating it.
Mark every node hosting the client dirty in one serialized write before the
fanout starts, so a merge queued behind it skips the node instead of merging a
half-applied edit. The nodes were going to be marked by their own applies
anyway; doing it up front only moves it earlier, and a client on local-only
inbounds never reaches the writer at all.
The set is the client's FULL attachment list, taken before the inboundIds
filter narrows it: the rename rewrites the one shared record, so an inbound the
filter excluded goes stale too.
Two tests, both red without the change. The first pins the ordering rather than
the end state — it reads the watched node's flag from inside another inbound's
push, so moving the marking after the fanout turns it red. The second pins that
the filtered path still covers the excluded node.
This narrows the window rather than closing it everywhere. A reconcile tick can
still clear the flag mid-fanout, and on a filtered edit the excluded inbound
keeps the old email in its settings for good, so its next merge duplicates
again. The case-drift path — a node reporting another case of a known email —
is untouched and still duplicates.
63b46cd6 made a multi-inbound client op apply its inbounds concurrently and
stop aborting at the first failure, so an error can now come back together
with needRestart=true: the inbounds that succeeded committed real changes and
their Xray still needs the restart. That commit taught the two callers it
converted — create and attach — to read the flag before the error check.
d34ec97f then routed Update, Delete, Detach and DeleteByEmail's record-less
fallback through the same fanout but touched no caller, so on a master with
several nodes a partly-applied edit or delete returned (true, err) into a
handler that returned on err first. Xray was never flagged for the work that
landed and notifyClientsChanged never fired, so the running config kept
serving the pre-edit client set and every open panel showed stale rows until
something else happened to trigger a restart.
Read needRestart before the error check in update, delete and detach, and
broadcast on needRestart || err == nil — the same shape create and attach have
had since 63b46cd6. The predicate is a strict superset of the old err == nil,
and needRestart is only ever assigned after a runSerializedTx commit, so it
firing genuinely means something landed.
The three handlers are pinned by a new controller test each: one client on two
inbounds, the second one's settings JSON corrupted so the op commits on one and
fails on the other, asserting both the success:false response and the restart
flag. All three fail without the change.
The API docs for update, del and detach now describe the partial-application
contract, as add and attach already did. Detach ends at the fanout so every one
of its errors carries the inbound prefix; update and delete write the client
record afterwards, and a failure there is reported without one.
telego.NewBot validates the token against `^\d+:[\w-]{35}$` before any
option is applied, so the "test-token" literal in the two
not-modified tests failed with "telego: invalid token format" and the
go-test and race jobs went red on every run since #6340. Use a
placeholder token that matches the format; the tests now reach the
mock API server, pass with the guard in place and fail without it.
ValidateObfuscation exists, by its own doc comment, so that a bad manual
entry cannot break the embedded device's IpcSet. It was not covering enough
to do that. Auditing the panel against amneziawg-go v3.1.20260828's full
UAPI surface turned up two holes, both confirmed by driving the values
through a real IpcSet:
S1 = 70000 upstream parses s1-s4 as uint16
S2 = 70000 (device/uapi.go)
Jc = -1 jc/jmin/jmax are uint32, so no negatives
Jmin/Jmax = -5/-1
Jc = 5000000000 and nothing wider than uint32
I1 = <rand 100> newObfChain hard-fails on an unknown tag
I1 = <r 100 ... and on a missing '>'
I1 = <> ... and on an empty one
All eight passed validation and were then rejected by the device. Only S3
and S4 were bounded, which is why the asymmetry went unnoticed. The inbound
saves, the reconcile fails on every tick, and the interface never comes up
with a single log line to say so.
Bound the five numeric fields to the widths upstream actually parses, and
check the I1-I5 chain's <tag value> structure against a tag set mirroring
upstream's own obfBuilders map. Each tag's value grammar stays amneziawg-go's
to enforce -- that is eight builders across several files, and duplicating
them here would drift. So <r abc> still reaches IpcSet, now as the only
remaining class rather than one of four.
Mirror the same bounds in the Zod schema, next to the max() that s3 and s4
already carried, so the form rejects the value instead of the save doing it.
TestValidatedObfuscationAlwaysApplies pins the contract itself: whatever
ValidateObfuscation accepts, a real amneziawg-go device must accept too. It
covers the specs the new grammar check deliberately allows, not just the ones
it rejects, so the allowlist cannot quietly become stricter than upstream.
The rest of the audit found no gaps: all 17 settable device keys reach
buildUAPIConfig, ServerSettings, the Zod schema and all three .conf
emitters. fwmark and persistent_keepalive_interval remain unemitted, both
deliberately -- the panel models no fwmark anywhere, and keepAlive is carried
client-side where WireGuard puts it.
Half-close. Both TCP relays -- RelayTCP into Xray's SOCKS5 inbound and
relayTCPForward into a peer's tunnel address -- waited on a single `done`
receive and then closed both sides. A client that finished sending and shut
down its write side therefore had the connection torn down before the
response came back. pipeBothWays now runs both directions to completion and
propagates the half-close via CloseWrite (which *net.TCPConn and
*gonet.TCPConn both implement), falling back to a full Close for anything
that does not.
Waiting for both directions reintroduces the risk the old single-receive was
implicitly avoiding: a peer that vanishes mid-transfer would pin the pair
forever. guardedReader bounds that, but as an idle window rather than a
total one -- the deadline is re-armed on every read once armed -- so a slow
transfer is never cut, while a silent peer is. Two minutes matches the idle
window UDPRelay.pump and portForwardUDPIdleTimeout already use.
UDP session retirement. pump's teardown deleted the map entry by key alone,
so a session that lost a create race evicted whichever session currently
held that source, orphaning a live flow. It now retires only its own entry,
and Handle keeps the already-published session when it loses the race. The
map is keyed on netip.AddrPort rather than src.String(), matching
udpForwardListener next door and dropping one allocation per relayed
datagram.
SOCKS5 reply decoding. bytesReader had a value receiver, so each Read
restarted at the head of the slice, and receive never advanced past a
domain-form address because its switch only handled ATYP 0x01 and 0x04 -- a
0x03 reply decoded to a wrong source, port and payload. splitSocks5Addr
replaces it: all three address forms, length-checked at every step, with the
domain form accepting only a literal. Resolving there would have put a
blocking DNS lookup on the receive path, and a datagram's own source is an
address already. Unreachable against Xray's own inbound, which always
answers with an IP, so this is a latent-bug fix rather than an observed one.
Every packet crossing the embedded AmneziaWG interface allocated instead of
reusing gVisor's pools, in both directions. stackTun.Write injected each
decrypted packet and never called DecRef, so the packet buffer and its chunk
were never returned; stackTun.Read copied each view out and never released
it. gVisor's own link endpoints settle the ownership question -- loopback.go
and sharedmem.go both DecRef immediately after DeliverNetworkPacket, because
the injector owns the buffer.
AttachUDPHandler compounded it by cloning a packet buffer it then dropped on
the floor, on top of a Data().AsRange().ToSlice() that already returns an
owned copy, so the clone bought nothing and stranded a pooled buffer plus a
cloned view per datagram.
Measured with the benchmarks added here:
stackTunWrite (upload) 794ns -> 107ns 4 -> 0 allocs
stackTunRead (download) 707ns -> 129ns 3 -> 0 allocs
UDP datagram, end to end 2.69us -> 1.58us 8 -> 2 allocs
The remaining UDP allocation is the ToSlice copy itself. Through a real
handshaked tunnel -- both devices in one process over loopback, so
ChaCha20-Poly1305 and the UDP syscalls dominate -- it is worth -48% bytes/op
and -33% allocs/op, and about +4.8% throughput in each direction (n=18,
p<=0.01). On a small VPS, where the allocation pressure is not spread over
24 idle cores, the throughput share should be larger; that part is reasoning,
not something measured here.
The three regression tests assert allocations per packet rather than timing,
since the defect is the pool miss, not the nanoseconds. Thresholds leave room
for the extra allocation -race adds.
Editing, deleting or detaching a client on a master with several nodes took
one node round-trip per node, added end to end. Create and Attach already
fanned their per-inbound applies out through fanoutInboundClientAdds, but
Update, Delete, Detach and DeleteByEmail's record-less fallback still walked
their inbounds in a plain sequential loop, and each iteration blocks on a
node RPC (10s timeout, more when a node is slow or has just gone unreachable
and the heartbeat has not marked it offline yet).
Measured with a node runtime injecting 100ms per RPC, before:
nodes=1 create=101ms update=101ms delete=101ms
nodes=3 create=102ms update=303ms delete=302ms
nodes=5 create=202ms update=504ms delete=504ms
after, all three track create:
nodes=3 create=102ms update=102ms delete=101ms
nodes=5 create=203ms update=203ms delete=203ms
Generalize the existing helper into fanoutInboundApplies over an inboundApply
list and route the four remaining loops through it, so they inherit the same
concurrency cap, per-inbound panic recovery and joined errors. Each caller
still builds its payloads sequentially first: fillProtocolDefaults mints the
shared credentials on the first inbound and every later one reuses them, so
that order has to stay deterministic. Only the applies overlap; their DB work
still serializes through the single traffic writer, and the per-inbound
mutation lock is unchanged, which is exactly what Create has relied on.
Behaviour change: one failing inbound no longer aborts the remaining ones,
matching what Create already does. The error still names each failed inbound
and the record-level writes are still skipped when any inbound failed.
The snapshot merge on the same serialized writer was measured as a second
suspect and cleared: ~43ms per node at 500 clients, an order of magnitude
below the RPC serialization.
The READMEs had not moved since 2026-07-07, 341 commits ago, and had
drifted far enough to misdescribe the panel: AmneziaWG and MTProto
inbounds were missing from the protocol list entirely, the outbound
list predated PIA, and the API section still advertised Swagger rather
than scoped, optionally expiring tokens.
Add the two missing protocols plus a bullet each for what makes them
notable — AmneziaWG runs on the embedded userspace netstack, so unlike
the DKMS/awg-quick shape it originally shipped with there is nothing to
install, and MTProto client edits hot-apply through the mtg-multi
management API instead of bouncing the process. Fold the smaller
additions into the bullets they belong to (HWID device limits, IP-limit
exemptions, renewal cycles, inbound cloning, balancer-to-balancer
fallback, geosite/geoip browsing, named subscription formats) and add
one for PWA installability.
Point documentation at docs.sanaei.dev, which the panel sidebar already
links to and which supersedes the wiki, using each README's own locale
where the docs site has one (fa/ru/zh). Bump the pinned install example
to the current stable tag, note the .sha256 verification install.sh and
update.sh now perform, and document XUI_NODE_TOKEN_KEY_FILE /
XUI_NODE_TOKEN_KEY, which no markdown in the repo covered.
All seven files move together so the language picker keeps pointing at
equivalent documents.
Two follow-ups on the preceding fixes, no behaviour change:
- The sweep comment in inbound_node.go had grown to a contiguous six-line
block, over the two-line maximum. The prefix rationale it carried is
already stated by nodeSelectedTagSet itself and by 6f40a51d's message.
- The probe cap test asserted only that an error came back, which cannot
tell a size rejection from a transport failure or a success=false
envelope. It now pins LastError to the decode rejection.
Both remain red-first: neutralizing maxProbeBodyBytes still fails the probe
test on the new assertion.
The alias re-application added in 0775fcaa wrote every adoptedAliases entry
onto the rebuilt map unconditionally, so an alias could override the id the
node itself reported for that same central tag. adoptedAliases is never
pruned — cacheDel clears remoteIDByTag and pushedFP only — so the entry
outlives the pairing that created it.
That inverts the intended precedence: once a push renames a node inbound to
the central tag, the node reports it directly, and a stale alias pointing at
some other inbound reusing the old name would win. Every state-changing op on
that inbound then targets the wrong one, overwriting or deleting an inbound
the operator created separately.
The alias now only fills a gap: a central tag the node already reports is
left alone.
probe decoded the node status response with json.NewDecoder(resp.Body) and no
size limit. encoding/json buffers the whole value before decoding, so the
allocation was dictated by the peer regardless of how few fields the envelope
declares — and the heartbeat job probes up to 32 nodes concurrently on a 4s
budget with no client-level timeout.
The sibling RPC path already caps every node response at 64 MiB
(readCappedBody in internal/web/runtime), so this was the one uncapped read
of node-controlled data. A status envelope holds a handful of scalars, so the
cap here is 1 MiB rather than the RPC figure.
The peer is untrusted in the skip and pin TLS modes, and the same decode is
reachable from the nodes test and probe endpoints.
AdoptInboundAlias maps a central tag onto a node inbound that carries a
different name, recording the pairing in both remoteIDByTag and
adoptedAliases. refreshRemoteIDs then rebuilt remoteIDByTag from the tags the
node reports and nothing else, so the central-tag entry was dropped on the
next cache miss for any other tag.
After that every op on the adopted inbound failed to resolve, and UpdateInbound
falls back to AddInbound — creating a duplicate inbound on the node at the same
port. cacheGetTag only recovers an n<id>- prefix flip, never an arbitrary
alias, so the pairing could not be rediscovered until a master restart.
The rebuild now re-applies adoptedAliases onto the fresh map, which keeps the
map the single place a tag is resolved from.
In "selected" sync mode the reconcile sweep built its set of managed tags
verbatim from node.InboundTags. A panel-created node inbound is stored with
an n<id>- prefix (composeInboundTag) and pushed to the node with that prefix
stripped (wireInbound), so the tag the node reports never matched the set and
the sweep skipped it.
The effect is the case the sweep exists for: an operator deletes a node
inbound while the node is offline, and the node keeps serving it — and its
clients — indefinitely. Only unprefixed tags were unaffected, which is why
the existing selected-mode test did not catch it.
nodeSelectedTagSet already builds both tag forms for exactly this reason and
is used by the snapshot filter; the sweep now uses it too, so the two agree.
claude-bot.yml ran three jobs: the pull-request review, an @claude mention
responder, and a conflict resolver that committed and pushed to contributor
branches. Only the review is wanted, so the other two are gone and the file
is renamed to say what is left.
Consequences worth knowing:
- secrets.CLAUDE_BOT_PAT is no longer referenced by any workflow. It was the
only push credential handed to an agent in this repository and can now be
deleted from the repository settings.
- @claude goes unanswered everywhere. claude-issue-analyst.yml deliberately
excludes mentions (!contains(body, '@claude')) so the two jobs would not
both reply; with the mention job gone, only `@claude review` on a pull
request still reaches anything. Dropping that clause from the analyst would
restore mention answering on issues.
- The workflow display name changes, so a branch protection rule keyed on
"Claude Bot / review" has to become "Claude PR Review / review". The job
name, which is what statusCheckRollup reports, is unchanged.
The review job itself is byte-identical. The workflow-level permission drops
to issues: read, which is all the remaining job needs - it already declares
its own.
`grep -ci amneziawg` returned 0 in both .github/claude/repo-context.md and
REVIEW.md while CLAUDE.md has carried the protocol for releases. The issue
analyst and the review bot could not name internal/amneziawg/,
internal/amneziawgnet/ or internal/pia/, and the mention job's inline map
enumerated ten protocols with amneziawg missing from the list.
The 3.1 obfuscation parameters are generated twice - GenerateObfuscation31 in
internal/amneziawg/params.go and generateAwgObfuscation in
frontend/src/lib/xray/amneziawg-obfuscation.ts - so REVIEW.md now names that
pair as a divergence surface next to the three link implementations. Commit
bd1c27b0 was already a bug in exactly that pair.
Also corrects the CLAUDE.md CLI list, which omitted encrypt-tokens.
Creating or attaching a client across N inbounds called AddInboundClient
once per inbound, strictly one after another. When those inbounds live on
different nodes each call is a full node round-trip bounded by the 10s
remote timeout, so the request cost the SUM of every node's latency: two
nodes felt instant, three took ~13s and timed out bot callers, which is
how it surfaced as "two out of four account creations fail".
Split the per-inbound preparation from the apply. Preparation stays
ordered and single-threaded because fillProtocolDefaults mints the shared
credentials on the first inbound and every later one reuses them; the
applies then run concurrently, capped at inboundFanoutConcurrency. A
4-node create measured 1.205s -> 0.307s with peak overlap 1 -> 4.
Consequences of no longer aborting at the first failing inbound:
- Every apply error is tagged with its inbound and the failures are
joined, so all of them reach the caller instead of just the first.
- The fanout goroutines recover their own panics. Off the request
goroutine gin's Recovery no longer covers them, and an unrecovered
panic would kill the panel rather than fail one inbound.
- A partly-applied call commits clients on the inbounds that succeeded,
so the controller and the LDAP job now read needRestart before the
error check; otherwise Xray was never flagged for the work that landed.
- limitHwid is applied only when every inbound succeeded. Applying it
after a failure rewrites limit_hwid and trims the registered devices of
an email that already existed, which is silent data loss on an
operation the panel reported as failed.
Update the API docs for the new partial-application contract and the
inbound-tagged error strings.
The race job failed with "panic: test timed out after 10m0s" in
internal/web/service (FAIL at 600.106s) while every other package passed
and the non-race go-test job ran the same package in 57s.
Nothing hung. The race detector costs this repo ~8.5-10x (internal/database
8.4s -> 73s, internal/sub 17.8s -> 149s), and internal/web/service has 671
tests, ~40 of which each pay a full InitDB + AutoMigrate. That puts it right
on go test's 10-minute default per-package timeout: the last four race jobs
finished in 10m10s-10m28s before this one crossed the line.
Pass -timeout 25m in ci.yml and `make race` so the largest package has real
headroom while a genuine deadlock is still bounded. Verified locally:
ok internal/web/service 265.425s, 658 tests, no data races.
Update frontend dev tooling to Vitest 5 by bumping `vitest`, `@vitest/browser-playwright`, and `@vitest/coverage-v8`, plus `@types/react-dom`. Add an override for `@storybook/addon-vitest` to pin Vitest-related packages to compatible versions and avoid dependency mismatch issues. Also bump the Go toolchain patch version from `1.27.0` to `1.27.1` in `go.mod`.
The sync adopts each node's reported clients through SyncInbound, which resolves
a client record by email alone — and clients.email is globally unique. A node
reporting a colliding email therefore overwrote that client's UUID even when the
client is attached only to a master inbound, and the master then rebuilt its own
Xray config with the node-supplied credential: the real user locked out.
Skip a reported client whose record is attached only to inbounds of other nodes.
A record attached nowhere stays adoptable, so the soft-orphan reattach path a
flapping node depends on is unaffected.
subJsons and subClashs served the raw body and returned before enforceHwid ran,
so appending ?view=raw to a JSON or Clash subscription URL handed out a complete,
client-consumable config however many devices were already registered. The branch
exists to stop a browser's Accept: text/html from being answered with the info
page, not to skip the gate.
Gate the raw branch and leave the other gate where it was, below
maybeServeSubPage, so the HTML info page stays ungated as before.
resolveXrayLogPaths looked the log object up by the exact keys "access" and
"error", but xray-core decodes that object with encoding/json, which falls back
to a case-insensitive field match. "Access": "/tmp/pwn.log" therefore reached
AccessLog untouched and Xray — root, in a standard install — created the file
there, reopening the arbitrary write that GHSA-jm48-m3rr-9hgg closed.
Fold every case variant onto the canonical key before confining it. When both a
canonical key and a variant are present the canonical value wins, so a
"none" cannot be overridden by a smuggled "Access" path.
Ten review runs fired in under two hours on 3 September and every one after
11:25 came back rejected: the five-hour usage window was at 100 percent
(overageStatus rejected, org_level_disabled) while the seven-day window sat at
29. Two of them reviewed the same head SHA and one pull request was reviewed
four times, because a draft/ready toggle re-fires pull_request_target and the
skip decision is only reachable after a full checkout and a model boot.
Settle it in the workflow instead: a bot comment carrying "Reviewed head:" and
the pinned SHA means this head is done, so the pr-head checkout, the brief and
the action are all skipped. An explicit "@claude review" is exempt, so a
maintainer can still force one.
A refused run also failed the job twice over - the action's exit 1 plus "the
review posted nothing" - with nothing on the pull request to say why, which
reads as a broken bot rather than an exhausted budget. The job now classifies
its own transcript: a rejected rate_limit_event, or a 529 that survived every
retry, posts one line on the pull request and stays green. Anything else still
fails loudly.
Also tightens that check, which counted ANY bot comment quoting the head SHA as
a legitimate skip; the conflict-resolution job quotes SHAs too, so a dead run
could go green on one.
The `suggestion` blocks stopped once the briefing moved into its own file, but
the carve-out that survived — "one clause naming where the fix belongs" — was
being stretched from a location into an instruction. #6397 dictated what to
write in a comment and which existing test to copy; #6394 named the fix
outright. The clause now permits a file, a function, a symbol or a layer and
nothing about what happens there, and closes the stretch three ways: prose is
a patch the moment a verb describes the change, so is holding up an existing
symbol as the model to copy, and a clause the maintainer could apply as
written is the fix however it is punctuated.
Three rules the rubric was missing, none of which existed anywhere. A 🔴 or 🟡
says in one clause what the change did to the code it is about, the way a 🟣
already says it predates it — otherwise nothing in the comment shows the
marker was earned. A claim about a caller or a callee needs that file read:
the dispatch-rule violation this repo cares most about sits a frame outside
the diff, and the skill is told to avoid reading past the changes. And nothing
pads the comment.
The briefing's one named override aimed at a step that does not exist. The
plugin the job loads defines no `--comment` flag and mentions suggestions
nowhere, so `max --comment <target>` is inert trailing text. Replaced with the
six overrides that are real: the skill calls pre-existing issues and unmodified
lines false positives, drops every finding its confidence pass scores under 80
and then posts nothing at all (a nitpick scores 50, so that filter empties all
five nit slots), says to avoid emojis against a severity system that is three
of them, mandates a "Found N issues" format, and forbids reading build signal.
Refresh the Go module set in go.mod and go.sum to newer patch/minor releases, including xray-related dependencies, gRPC, WireGuard, and supporting indirect libraries. This keeps the project aligned with upstream fixes and compatibility updates without changing application code.
Update dependency versions across `docs` and `frontend`, including Next/Fumadocs packages in docs and Ant Design, React Query, Storybook, and related tooling in frontend. Also updates lint/format tool versions (`oxlint`, `oxfmt`), bumps docs `pnpm` package manager version, and refreshes workspace release-age exclusions for the newly upgraded docs packages.
The issue analyst moves verbatim from claude-bot.yml into its own
claude-issue-analyst.yml, so claude-bot.yml now holds only the pull-request
side: review, @claude mentions and conflict resolution.
The review job's briefing was a single 2,600-character quoted string inside
claude_args, unreadable and unreviewable. It now lives in
.github/claude/review-job.md, assembled at run time with a "This run"
section that hands the reviewer the pinned head SHA, the pull request and
the exact check-runs command, and reaches the CLI through
--append-system-prompt-file. The agent-mode action sets no system-prompt
append of its own, so the file flag cannot collide with one.
Findings no longer carry the fix: REVIEW.md and the brief both forbid
suggestion blocks, patches and replacement snippets, overriding the
code-review skill's --comment step, which attaches a committable suggestion
to any small fix. A finding states what is wrong, where, what triggers it
and what breaks; the maintainer decides the change.
Routine dependency refresh: telego 1.11.2, go-sqlite3 1.14.50, grpc 1.83.1,
miekg/dns 1.1.73, sing 0.8.14 and the usual indirect churn on the Go side;
react-query 5.102.2, i18next 26.4.0, react-hook-form 7.86.0, Storybook
10.5.10 and vite 8.2.2 on the frontend, which also lifts the private frontend
package to 1.0.0.
That left npm run lint:deprecated with five call sites. Zod 4 deprecates the
ZodTypeAny alias in favour of the bare z.ZodType constraint, and react-query
renamed queryClient.fetchQuery to queryClient.query ahead of removing the old
name in the next major — the two share an implementation, so the swap in the
settings test is behaviour-identical.
Also untracks internal/web/dist/.gitkeep. 1872659d dropped its gitignore
exception on the grounds that nothing under dist/ is ever meant to be
tracked, but the file was already in the index, so the rule never applied and
every frontend build that empties dist/ resurfaced it as a spurious deletion.
make dist-stub and every CI job recreate it on disk.
ipStaleAfterSeconds was only applied while a row was being rewritten, and
rows are only rewritten for clients present in the current online scan. A
client that stopped connecting therefore kept its last addresses forever
in inbound_client_ips, and node_client_ips rows (including those of
deleted clients) were never revisited at all. Sweep both tables every five
minutes, dropping entries past the cutoff and deleting rows that end up
empty. The sweep runs ahead of the fail2ban and api-mode gates so
retention holds even on panels that collect nothing.
Closes#6286
SanitizePublicHTTPURL rejected a hostname as soon as any single resolved
address was blocked, so a resolver returning a bogon AAAA for the test URL
host (e.g. 2001::1 for www.google.com, inside the Teredo range blocked
since b51f0976) failed the outbound Check button outright — including TCP
mode, which never uses the test URL. Mirror SSRFGuardedDialContext instead:
one usable address is enough, because the guarded dialer skips blocked
answers at connect time; a hostname with nothing usable is still refused.
Closes#6290
disconnectClientTemporarily still gated on the protocol list from before
XrayAPI.AddUser learned hysteria, so an over-limit Hysteria2 client kept
its QUIC session until the fail2ban ban aged out, while a VLESS client in
the same situation was dropped at once. buildUserAccount handles hysteria
and model.Client already marshals the auth field the re-add needs, so admit
the protocol. wireguard stays excluded: its keepAlive marshals as a JSON
number, which the string-only user-field parsing rejects after the user was
already removed.
Closes#6256
genHysteriaLink only looked up the UDP hop range on the no-endpoint path,
after the externalProxy fan-out had already returned. An inbound with Hosts
therefore emitted per-host links without mport, so clients pinned themselves
to the single listening port and silently lost port hopping. Set the param
before the fan-out so every endpoint inherits it, matching the frontend
link builder and the Clash emitter.
Closes#6264
tlsData rebuilds the client-side tlsSettings from a whitelist of keys
and never copied cipherSuites, so an inbound configured with e.g.
"TLS_AES_256_GCM_SHA384" handed clients a config that negotiated any
suite. Copy it through when non-empty; it is a real xray-core
tlsSettings field, unlike the non-standard "cs" share-link param.
Go 1.27.0 shipped on 2026-08-19. Raise the go directive and the builder
image so Docker and release builds pick it up; every CI job already reads
the version from go.mod, and golangci-lint v2.13.1 release binaries are
themselves built with go1.27.0, so the lint job needs no pin change.
The review job checked the pull request out through refs/pull/N/head, a
ref the author can move after a maintainer types "@claude review". Code
scanning flagged it twice on the issue_comment path: an untrusted
checkout in a privileged context (alert 111) and the time-of-check /
time-of-use race that ref creates (alert 110).
Resolve the head once, up front, and refuse the run when the fork was
pushed to after the request that vouched for it, mirroring the freshness
gate resolve-conflicts already uses; the checkout then names that
immutable SHA. pull_request_target runs take the head SHA straight from
the payload, so they skip the comparison. The trailing "posted nothing"
check no longer fires on top of a refusal, which would otherwise report
a second, misleading failure.
REVIEW.md said what blocks and what does not, but never how to mark a finding,
so every review invented its own shape and none carried a severity. It now
names the three markers the hosted Code Review service uses - Important, Nit,
Pre-existing - and keys them to what the pull request did rather than to how
alarming a defect looks alone: a defect it introduces or worsens is Important,
one it merely brought into view is Pre-existing and cannot be a reason to hold
it. Pre-existing was missing entirely, and checking what this panel emits means
reading far outside the diff, so those findings had nowhere to go except a
wrong Important or silence.
The volume cap said how many and never which. It now collapses a nit repeated
across files into one finding, prefers a nit in code the pull request wrote
over one in code it only moved, caps pre-existing findings at three, and states
that Important findings are never capped - a section listing two caps otherwise
reads as licence to trim what matters. The review opens with a tally so the
author sees the shape before the detail.
Two contradictions went with it. The file told the reviewer to skip what CI
enforces and then to check that a new i18n key reaches all 13 locales, which
i18n-dead-keys.test.ts pins in both directions - the rule moves to "Do not
report" with the reason. "Anything CI already enforces: npm audit" overstated
what runs; CI audits production dependencies at high and above, so a
dev-dependency advisory is out of scope by design.
The reviewer could not read its own CI. Only postgres-durable-first runs
against PostgreSQL, and XRAY_E2E_BINARY and XUI_SCALE_TEST are set by no job,
so a dialect or migration change can carry a wall of green while the paths it
touches never executed. That belongs to the verification bar, next to the rule
that a behaviour claim needs a file:line citation, and "CI passed" now needs a
run actually read. Also names the two house choices no linter defends: neither
golangci-lint nor oxlint rejects a testify or Tailwind import.
Both kinds of claim rot on a rename, so a test pins them the way
repo-context.md's claims are already pinned - the CI jobs REVIEW.md names must
exist in ci.yml, the skip gates it calls unset must stay unset, and the locale
count must match the directory.
The review itself moves from high to max effort, and the prompt records why it
names REVIEW.md at all: the code-review skill reads CLAUDE.md on its own but
not REVIEW.md, so dropping that clause would silently stop the file applying.
Drops a CLAUDE.md reference to tools/seedperf/, which no longer exists - the
review reads that file as project context, so a stale path there misleads it.
Three consecutive review runs (#6105, #6265, #6272) posted accurate findings
but ended with the same "nothing was verified" paragraph, and the transcripts
show why: under pull_request_target the only checkout is the base branch, so
every Read of a changed file returned the pre-merge version and the agent
fell back to fetching blobs one at a time through the API — 452 Bash calls on
#6105 alone. It tried `git fetch origin pull/N/head` in all three runs and was
denied every time.
Check the head out read-only beside the base tree and say so in the prompt, so
the reviewer greps the code actually under review. Nothing builds or executes
from pr-head/: this job carries a write-scoped token, which is exactly the
pwn-request REVIEW.md classes as blocking.
CI had already run the full gate on each head SHA, but no run ever looked —
`check-runs` appears in none of the three transcripts. Point the reviewer at
it so a red or missing required check becomes a finding instead of a
disclaimer.
Also pass an explicit review level: with none given the skill reuses the last
one typed, which in CI does not exist (ReportFindings recorded level=null on
#6272). And allow WebFetch/WebSearch — the PIA review was denied both while
trying to confirm the bundled PIA public key, then had to file that same check
as unverified.
Two failures from the same afternoon, both in the bot workflow.
The review of #6272 ran for 34 minutes across four subagents and posted
"No issues found. Checked for bugs and CLAUDE.md compliance." — three
lines for a 73-file diff. The agent had written a per-area coverage
summary in its own last turn and then dropped it on the floor, because
the code-review skill's comment template carries findings and nothing
else. A comment that cannot distinguish a thorough clean review from a
run that died early is not evidence, so REVIEW.md now states what the
posted comment must show and the system prompt points the run at it.
The same run logged 67 permission denials. Only the inline-comment MCP
tool was named in --allowedTools, so `gh api`, writing the diff to a
scratch file, and reading it back were all auto-denied: agents spent
turns hunting for a writable directory, and the openapi.json copy check
REVIEW.md calls blocking could not be run at all ("gh api was
unavailable in this sandbox"). Name the tools the review actually uses.
The conflict resolution on #6243 resolved both conflicted files
correctly and was then rejected by its own guard: "Edits outside the
conflicted set: CLAUDE.md". The agent never touched CLAUDE.md — it had
Edit rights on exactly two paths and no shell. claude-code-action
deletes and restores CLAUDE.md, .claude/, .mcp.json and friends from
the base branch before it runs, because the PR head is untrusted, and
that restore is what dirtied the tree. Name that set once, exclude it
from the stray-edit check, and hand back rather than resolve when a
conflict lands inside it — the restore would silently overwrite the
resolution and stage the base copy.
golangci-lint v2.13.0 pinned honnef.co/go/tools v0.8.0-rc.1, whose
staticcheck never terminates on internal/web/service/tgbot: the run pins
~520% CPU with RSS climbing past 700MB rather than deadlocking, so it
reads as a hang. controller/, job/ and service/... only appeared stuck
because they pull tgbot into the analysis graph. v2.13.1 ships the final
v0.8.0 and clears it — that package goes from unbounded to 0s, and a
cold full run to 22s. CI needs no pin; it already tracks latest.
The same bump reworded SA1019 from parser.ParseDir to go/parser.ParseDir,
which silently voided the openapigen exclusion, so the pattern now
matches either spelling.
fasthttp Client.RetryIf is deprecated in favour of RetryIfErr. The old
path left resetTimeout at its zero value, so returning false preserves
the existing retry timing exactly.
The rest are gofumpt redundant-paren removals from the stricter
formatter — semantic no-ops.
Three separate ways the bot went silent after the move to the official
code-review skill:
- The skill skips a PR it has already commented on without comparing the
reviewed head to the current one, so #6272 got no review of the commits
pushed after the first pass. A prior review now only justifies a skip
when its "Reviewed head:" SHA matches the current head, and never when
the run came from an explicit "@claude review".
- The review agent launched its subagents in the background and ended its
turn to wait for them. A headless run terminates on end_turn, so the
findings were discarded and the job still reported success. The prompt
now requires foreground subagents, and a new step fails the job when a
run posts nothing for the current head, instead of passing green.
- A custom prompt puts claude-code-action in agent mode, which never adds
the eyes reaction, so a mention gave no sign it had been picked up.
The four pull_request_target review jobs in claude-bot.yml (Senior
Developer / QA / Tester / Arbiter and their shared rubric) are replaced by
a single review job running the official code-review plugin - the same
skill behind Anthropic's hosted Code Review and the review workflow
/install-github-app generates. The hosted service needs a Team/Enterprise
organisation, so the plugin runs in CI on the maintainer's subscription
instead: inline findings on PR open and ready-for-review, plus manual
(re-)review when the owner or a collaborator comments "@claude review".
The official example triggers on pull_request, but GitHub withholds
secrets from fork runs and essentially every 3x-ui pull request is from a
fork, so the job keeps the lanes' pull_request_target posture: the
workspace is the base revision and nothing from the pull request is
checked out or executed.
What the lanes uniquely knew is distilled into REVIEW.md, handed to the
skill via --append-system-prompt and pinned by bot_context_test.go the way
repo-context.md is: the runtime.Runtime dispatch rule, migration and
upgrade safety, the four-step route contract chain including the unchecked
docs copy, the i18n rule, the three link implementations, and the
wire-format verification bar. The mention job now ignores "@claude
review" comments on pull requests so the review trigger does not also
wake the generic bot, and the lane-only rubric file goes with the lanes.
The remaining prompts also lose their tone micro-rules (no emoji, no
exclamation marks, no filler) and the workflow's comment banners are
removed.
Client CRUD latency scaled with the number of client-inbound edges rather
than with the size of the change. On a 5k-client / 8-inbound / ~56k-edge
PostgreSQL panel, creating one client took 60-120s (#6252).
Two independent causes, both confirmed by the reporter's pg_stat_statements
and reproduced locally at their topology.
SyncInbound deleted every client_inbounds row for an inbound and re-inserted
the whole set, so a one-client edit rewrote thousands of unrelated rows. The
dominant caller was not user CRUD: the node traffic poll re-syncs every node
inbound from its snapshot every 5s, so the panel churned the entire
membership table continuously in the background. SyncInbound now reads the
current links and writes only the difference - insert missing, update a
changed flow_override, delete departed. Callers are unchanged, so every
reconciliation path benefits, and the four hot client CRUD paths additionally
pass only the clients they touched via ApplyInboundClientDelta.
The insert needs clause.OnConflict: the unconditional delete it replaces also
serialized concurrent syncs of one inbound, and the node poll commits in its
own transaction outside the serialized writer, where a duplicate key would
abort the whole poll on PostgreSQL.
Identity and membership questions expanded every inbound's settings.clients
JSON - 5.75s per call under the reporter's load. They now read the indexed
clients and client_inbounds tables, which every read path already trusts,
over just the emails being checked. A LOWER(email) expression index keeps
the case-insensitive matching indexed; a struct tag cannot declare one.
Measured on PostgreSQL 17 at 8 inbounds x 6000 clients, rows written to
client_inbounds per operation, before -> after:
create across 8 inbounds 48008 ins / 48000 del -> 8 ins / 0 del
update the client 48008 ins / 48008 del -> 0 ins / 0 del
detach from 4 inbounds 24000 ins / 24004 del -> 0 ins / 4 del
delete the client 24000 ins / 24004 del -> 0 ins / 4 del
Two behavior changes worth naming. An email seen with two different subIds
across two inbounds' JSON used to be locked so that no add could claim it,
including the one with the correct subId; the clients row now adjudicates.
And on an install whose settings JSON holds an email with no matching link,
"is this email on another inbound" now answers no, so deleting it elsewhere
purges its traffic rows; compactOrphans and the startup heal already
converge such drift.
Every added test was verified against a hand-written mutation of this change,
so none of them pass regardless of the fix. One mutation survives on purpose:
swapping OnConflict DoUpdates for DoNothing is only observable when two
transactions race the same row, and a timing-dependent test would be flaky.
Per-node batching of remote pushes and the metadata-only inbounds list from
the same report are deliberately not in this change.
Closes#6252
The confirmation gate in updateSetting only covered the true -> false
transition, so a settings save that kept twoFactorEnable=true while
carrying a non-blank twoFactorToken silently rebound the authenticator.
preserveRedactedSecrets restores the stored secret only when the
submitted one is blank, so a non-blank value went straight through
without any branch asking for a code.
Not reachable pre-auth or cross-site (CSRFMiddleware rejects unsafe
methods without the session token), but it matters after a session
hijack or with an admin API token, which sets api_authed and
short-circuits the CSRF check: the attacker gains persistence and locks
the legitimate operator out of their own authenticator.
Now a code is required whenever 2FA is currently on and the submitted
secret differs from the stored one. Enabling from off is untouched, as
no code exists yet to verify, and a blank secret still means
"unchanged", so the panel's normal save path is unaffected.
Reported by @n0ctal (GHSA-xqqw-jqqv-99h6).
IsBlockedIP leaned entirely on Go's net.IP predicates, which judge an
address by its own range only. 6to4 (2002::/16), NAT64 (64:ff9b::/96 and
64:ff9b:1::/48) and Teredo (2001::/32) each tunnel an arbitrary IPv4
destination inside an IPv6 address, so all five predicates returned false
for e.g. 64:ff9b::7f00:1 and the SSRF guard waved it through. CGNAT
(100.64.0.0/10) and the deprecated site-local block were unclassified for
the same reason. Reported as GHSA-cfpf-wmjp-gh6c.
Reaching the embedded IPv4 needs a 6to4 tunnel, NAT64 gateway or Teredo
client on the host, none of which exist by default, so this is hardening
rather than a live path off a stock install. The guard backs outbound
subscription fetches, node sync, reality scan, the tgbot API URL and the
xray setting test URL, which is reason enough to close the gap.
The deprecated and local-use prefixes are blocked outright since nothing
public routes through them. The NAT64 well-known prefix is judged by the
IPv4 it embeds instead: on a DNS64 network every public IPv4 host resolves
into it, so blocking it wholesale would break legitimate fetches.
fumadocs-core 16.14.5 switched its search engine from Orama to zbsearch 4,
so the panel docs follow it up to the same major.
zbsearch 4 still rejects locale codes as tokenizer languages ("en" throws,
only "english" is accepted), so the custom search dialog that forces an
English index stays necessary — verified by loading the built static index
for all four locales and searching it through fumadocs' own client.
Around that:
- use `staticClient`, as `oramaStaticClient` is now a deprecated alias
- drop @orama/orama, which nothing depends on or imports any more
- correct the two comments that still described Orama and pointed at its
docs and tokenizer package, one of them suggesting a language zbsearch
does not have
- restore the corepack integrity hash on `packageManager`, which CI reads
through pnpm/action-setup
- prune minimumReleaseAgeExclude entries for versions no longer installed
The API reference MDX changes are serialization-only: fumadocs-openapi
11.2.4 emits plain scalars where it used folded ones. Parsed frontmatter
and page bodies are unchanged.
The from-scratch lockfile regeneration in b9eda09d bumped rolldown 1.2.4 ->
1.2.5 but wrote back only 14 of its 16 optional platform bindings: npm
removed the old @rolldown/binding-darwin-x64 and
@rolldown/binding-linux-arm64-gnu entries and never added the 1.2.5 ones.
Both are still listed in rolldown's optionalDependencies, so the packages
section no longer matches the dependency graph.
npm ci validates the whole ideal tree, not just the packages installable on
the current platform, so it aborted with EUSAGE everywhere and took down all
four workflows that install the frontend - CI, Release, CodeQL and Docs
Deploy - each at its first npm ci step. The Go jobs were unaffected.
Regenerated with a clean npm install --package-lock-only, which resolves from
registry metadata alone and keeps every optional binding regardless of the
host platform. The diff is purely additive - the two missing blocks, no
version changes.
npm install was failing with ERESOLVE: the lockfile pinned storybook 10.5.7
and vitest 4.1.10 as peers while package.json asked for ^10.5.9 and ^4.1.11,
and npm would not move either. Neither npm update, a targeted install, nor
--package-lock-only broke the cycle, so node_modules and package-lock.json
were regenerated from scratch (601 packages, 0 vulnerabilities).
oxlint 1.79.0 then promoted five React Compiler rules into the correctness
category, flagging 101 pre-existing sites. 1.78.0 exits 0 on the same tree,
so nothing in our code changed - the rule set grew. They are fixed rather
than suppressed:
- refs (31): latest-value ref writes moved out of render into an effect.
onlineClientsRef turned out to be write-only and is gone; expireDiffRef
and trafficDiffRef were replaced by reading the values directly.
- set-state-in-effect (55): reset-on-open modals now adjust state during
render; where an effect mixed a synchronous reset with an async fetch, the
reset moved to render and the effect kept only the request. useMediaQuery
became useSyncExternalStore.
- preserve-manual-memoization (11): optional-chained deps the compiler cannot
match, hoisted to locals or dropped where the memo wrapped a string concat.
- purity (3): Date.now() in render replaced by a state-backed clock, which
also refreshes the expiry tag every 60s instead of freezing it until the
next unrelated re-render.
- immutability (1): applyClientStatsEvent merged websocket traffic into
DBInbound rows in place; it now rebuilds only the rows it touches.
Two things fell out of that. clientCount is derived with useMemo instead of
an imperative rebuildClientCount() called from five sites, which also fixes a
staleness bug where changing the expiry or traffic threshold left the counts
alone until some later rebuild. statsVersion existed only to force a
re-render after an in-place mutation, is meaningless now that rows are
replaced, and nothing read it, so it is removed.
Also adds a lint:fix script - oxlint --fix was previously only reachable
through the lint-staged hook.
* chore(frontend,docs): move to TypeScript 7 and replace ESLint with oxlint
TypeScript 7 is the native Go port and ships no programmatic compiler
API, so typescript-eslint cannot run at all: it peer-pins
typescript >=4.8.4 <6.1.0 (canary too) and hard-crashes with
"typescript-eslint does not support TS 7.0". Upstream support is
tracked in typescript-eslint#10940 and targets TS >=7.1.
Rather than wait, or carry Microsoft's side-by-side alias (which keeps
a second TS 6 install alive purely to feed the linter), both projects
move to oxlint, which never depended on the TypeScript API.
Typecheck drops from ~9.7s to ~2.2s and 167 packages leave frontend/.
oxlint has no no-restricted-syntax, so the #6121/#6127 cleared-
InputNumber guard is reimplemented as a JS plugin in
frontend/tools/oxlint/. It was verified to still fire in
pages/settings/** and pages/xray/** and to stay exempt in *Modal.tsx.
The type-aware @deprecated sweep survives too, as
`npm run lint:deprecated`: oxlint's type-aware mode runs on
oxlint-tsgolint, which drives the TS 7 typescript-go checker, so the
TS 7 move is what makes it possible.
Behaviour is preserved rather than tightened. jsx-a11y/prefer-tag-over-role
is off in both configs because it was never part of the recommended sets
ESLint actually ran, and oxlint honours the existing eslint-disable
comments, so no source churn was needed.
Two real fixes fell out of the stricter linting:
- outbound-link-parser.test.ts used `out?.streamSettings` behind an `as`
cast, which hid the optional chain from ESLint and would throw on a
null parse; the rest of the file already used `out!`.
- InputAddon's conditional role/tabIndex/onKeyDown is genuinely
accessible but oxlint cannot evaluate it, so it gets a scoped disable.
* chore(docs): replace Prettier with oxfmt
oxfmt is the oxc project's Prettier-compatible formatter, so this pairs
with the oxlint move and drops the last JS-based tool from the docs
toolchain.
The swap is behaviour-preserving. Running Prettier and oxfmt over the
same files, with the existing .prettierrc.json settings migrated via
`oxfmt --migrate=prettier`, produces byte-identical output on every
file. (Comparing them outside the project directory is misleading:
Prettier silently falls back to its defaults when it cannot find its
config, which looks like a mismatch but is not one.)
The 18 files reformatted here were already failing `pnpm format:check`
before this change — Prettier wanted the exact same edits. The check is
not part of docs-ci.yml, which is why the drift went unnoticed.
.prettierignore becomes ignorePatterns in .oxfmtrc.json, keeping the
deliberate MDX exclusion: reflowing MDX prose merges headings into
paragraphs and collapses lists inside Steps/Callout components. Both
that and the generated fumadocs-openapi reference output were verified
untouched.
oxfmt is pinned to 0.63.0 rather than latest. pnpm 11's built-in
minimumReleaseAge policy rejects same-day releases, and 0.64.0 would
have made pnpm silently append 20 waiver lines to pnpm-workspace.yaml.
* style(frontend): adopt oxfmt and format src
frontend/ has never had a formatter, so this reformats 344 of 497 files
in src/. The change is purely whitespace, quoting and line wrapping —
no logic is touched. It is kept in its own commit so it does not bury
the TypeScript 7 / oxlint migration or the git blame for the code
itself.
Settings match docs/ and the code as it was already written: single
quotes, semicolons, trailing commas, 2-space indent, 100 columns. That
was measured rather than assumed — src/ was already uniformly
single-quoted and 2-space indented, with p90 line length at 75.
Formatting is scoped to src/ (mirroring `oxlint src`) and
.oxfmtrc.json ignores src/generated. Both matter: `make gen-check`
compares src/generated and public/openapi.json, and
`make msw-worker-check` byte-compares public/mockServiceWorker.js
against the installed MSW runtime, so reformatting any of them breaks
the gate.
Reflowing also moves `eslint-disable-next-line` comments off the line
they guard, which broke two suppressions that had been silently
correct before:
- clone-inbound-modal.test.tsx: the object literal became multi-line,
leaving `} as any;` four lines below its no-explicit-any disable.
- ClientsPage.tsx: the useMemo dependency array moved onto its own
line, out from under its exhaustive-deps disable.
Both comments were relocated onto the line they actually guard, and
verified to still suppress by removing them and watching the errors
return.
* ci: enforce formatting in CI and make verify
Adding oxfmt in the previous two commits gave both projects a formatter
but nothing that checks it, which is how docs/ had already drifted to 18
unformatted files: docs-ci.yml runs typecheck, lint, test and build, but
never format:check, so Prettier's complaints were only ever visible to
whoever ran it by hand.
Wire `format:check` into the frontend job in ci.yml and the docs job in
docs-ci.yml, and add a `format-check` target to `make verify` so the
local gate keeps mirroring CI as the Makefile header promises.
Verified the step actually bites rather than passing vacuously: adding
a badly formatted line to a source file in each project makes both
`make format-check` and `pnpm format:check` fail, and reverting it makes
them pass again.
No workflow referenced ESLint or Prettier by name — they all invoke the
package scripts — so the tooling swap needed no other CI changes.
* ci: trigger CI on Makefile changes
The path filters listed **.go, go.mod, go.sum, frontend/**, .nvmrc and
ci.yml itself, but not the Makefile — so a change to the canonical task
runner that ci.yml is meant to mirror could land without any job
running. The previous commit, which edits both, only triggers because
it happens to touch ci.yml too.
* fix(frontend): replace deprecated Ant Design 6 APIs in the geo components
`npm run lint:deprecated` reported five uses of props Ant Design 6 has
deprecated. All five are gone, and the matching runtime warnings no
longer appear in the test output.
Tag `bordered={false}` becomes `variant="filled"` and Space `direction`
becomes `orientation`; both are the one-to-one replacements named in
antd's own deprecation messages, and `direction`/`orientation` share the
same Orientation type.
Input `addonAfter` is the one that is not a rename. It becomes a
`Space.Compact block` wrapping the Input and the browse Button, which is
antd's documented migration. `block` keeps the field filling its form
row as the addon did. Note this is a deliberate visual change: the
button used to be a borderless `type="text"` icon sitting inside the
addon's grey box, and is now a regular button whose border joins the
input. The tooltip, aria-label, ref, id and onBlur wiring are unchanged,
so the react-hook-form binding in RuleFormModal and the existing tests
still address it the same way.
Only these five were deprecated. The other `bordered` props in the tree
sit on QRCode, Table, Descriptions and Alert, where the prop is not
deprecated, and these were the only two Space `direction` uses in the
codebase.
* fix(frontend): restore lint rules lost in the oxlint migration, and test the guard
Addresses the review on #6262.
The frontend config re-enabled only no-explicit-any and no-unused-vars
and left the rest of tseslint's recommended set to oxlint's correctness
category. It does not cover all of it. Confirmed by linting one probe
file against both configs: docs/ (which enumerates the rules) reports
all nine, frontend/ reported four. So ban-ts-comment,
no-empty-object-type, no-namespace, no-require-imports and
no-unsafe-function-type had silently stopped being enforced — a `//
@ts-ignore` or a `namespace` block would have landed unflagged. The ten
rules are now mirrored from docs/.oxlintrc.json, and src/ still passes.
The #6121/#6127 guard was 57 lines of hand-written AST walking with no
test. It now has one: fixtures for the three banned shapes plus an
onNumber()-wrapped control, asserting the rule fires three times and
that .oxlintrc.json still wires it to the right paths. Verified it fails
for the right reason by making walk() enumerate nothing, which is the
silent-death mode the review described — the traversal depends on
Object.keys() seeing AST children as own enumerable properties.
The fixtures deliberately violate the rule, so their oxlint config is
named guard.oxlintrc.json rather than .oxlintrc.json: oxlint discovers
nested configs by directory, which would otherwise turn the fixtures
into three lint errors. The test passes it explicitly with -c.
Also from the review:
- lint and format now cover tools/ as well as src/, so the one piece of
hand-written lint logic in the repo is no longer the least covered
file in it.
- lint-staged runs oxfmt before oxlint --fix. Formatting became a hard
CI gate in this PR while the hook only ran the linter, so a commit
could pass the hook and fail CI on formatting alone.
- .oxfmtrc.json ignores public/, so the artefacts that make gen-check
and make msw-worker-check byte-compare stay safe even if oxfmt is
invoked without a path argument.
- The MDX and generated-reference rationales that .prettierignore
carried are back as comments in docs/.oxfmtrc.json — oxlint and oxfmt
both accept JSONC, so relocating them was unnecessary.
Not applied: the review also suggested restoring ../internal/web/dist to
the ignore lists. Both tools reject `..` patterns outright ("patterns
are resolved within the config file's directory"), and being outside
frontend/ it is unreachable anyway.
The !internal/web/dist/.gitkeep exception kept the dist build stub
permanently visible as untracked noise and let it slip into commits four
separate times (each reverted with a 'drop the accidentally committed
dist build stub' commit). Nothing under dist/ is ever meant to be
tracked — make dist-stub and CI recreate the stub on disk — so drop the
whole exception block and let the plain dist/ rule cover it.
Translate the `importKeepHostSettings` and `importKeepHostSettingsDesc` keys from English placeholders into 10 locale files (ar-EG, es-ES, fa-IR, id-ID, ja-JP, pt-BR, tr-TR, vi-VN, zh-CN, zh-TW).
Also removes `main_test.go` which contained a test for a `commandHelp` function.
The three prompts still enforced the comment ban CLAUDE.md replaced with
the 2-line cap on Aug 1 (1ff90c5b), so the review bot would flag every
legitimate short comment; frontend/CLAUDE.md and CONTRIBUTING.md carried
the same stale rule. The PR reviewer's recipe for reading a post-change
file (headRefOid + pr diff) was unfulfillable with its allowlist - it now
fetches refs/pull/N/head and reads blobs via git show, object-only, no
checkout. Conventions the reviewer checks now include the unchecked docs
openapi.json copy step, the docs/lib/xray third link implementation, the
both-ways route contract, and the i18n dead-key half of the rule.
Also: drop the SUBPROCESS_ENV_SCRUB=0 override on the two untrusted-input
jobs (the mention job proves gh works scrubbed); teach the triage prompt
the issue forms (pre-applied labels, required fields, no re-asking); add
a security-report exception plus SECURITY.md so vulnerabilities are not
confirmed publicly; add a clarification follow-up job so a reporter's
reply to "clarification needed" is actually processed; review PRs again
on ready_for_review and skip drafts; stamp the reviewed head SHA so
force-pushes visibly date a review; scope gh issue/pr edit to label and
title flags; per-job concurrency; comment guards now match the actual
bot login after the run started; artifact names survive re-runs; the
mention prompt's repo map and env-var facts corrected (XUI_PORT,
XUI_TUNNEL_HEALTH_*, distro env files, memory.high, encrypt-tokens).
The bug and feature forms also referenced a "needs triage" label that
does not exist in the repo and was silently never applied - dropped.
persian-calendar-suite seeds today's date and emits it whenever it mounts
without a value. Clearing the expiry remounts the picker with a null value,
so the library immediately fired onChange(today) and the date came straight
back — and it also painted that seeded date into its read-only input.
Swallow the mount-time emit (re-armed on every clear-remount) and hide the
seeded text while the value is empty, so a cleared expiry stays empty and a
fresh client/inbound form no longer silently adopts today as its expiry.
Revert 43bc9153 and its follow-up 338822ab. The copy-only notice replaced the
themed sub page for every browser request, so mobile users got a bare "This is
a subscription link" screen instead of their traffic, expiry and links — and it
left serveSubPage plus the custom-theme renderer as dead code.
Constrain the copy-only subscription page to the dynamic viewport, wrap long localized text, and infer text direction so mobile browsers cannot render a horizontally shifted desktop-width page. Add regression coverage for the responsive layout contract.
Removes the `virtual` table option from Clients, Inbounds, and Nodes list pages. This aligns table behavior across key admin views and avoids virtualization-related rendering/interaction issues with the existing scroll and pagination setup.
Refresh the Go toolchain from 1.26.5 to 1.26.6 and update the related x/* and protobuf dependency set in go.mod/go.sum. This keeps the project aligned with the current patch releases and ensures the module graph matches the expected transitive versions.
Update fumadocs-core/mdx/ui to 16.14.3, lucide-react to 1.31.0, @types/node to 26.2.0, typescript-eslint to 8.67.0, esbuild to 0.28.2, shiki to 4.4.3, and various other transitive dependencies.
Collapse animates opacity in over motionDurationMid; the Collapsed story's
play function only waited for visibility, so the addon-a11y color-contrast
check could sample a mid-fade, lower-contrast frame and fail flakily in CI.
Wait for the panel's opacity to settle to 1 first.
Keep swagger-ui-react and its transitive dependencies in the lazy swagger chunk so the initial panel bundle stays smaller. This avoids eager loading the OpenAPI UI on first paint while keeping the API docs route unchanged.
Refresh frontend package versions and regenerate the lockfile. This updates core UI and tooling packages including Ant Design, React Hook Form, Storybook, Vite, eslint/typescript-eslint, @noble/hashes, persian-calendar-suite, and swagger-ui-react to pick up the latest fixes and minor improvements.
Move html/body shell and global css to root app layout to avoid hydration/script warnings from nested document nodes. Disable provider theme injection and add a custom script-free theme switch in shared layout slots.
Also migrate docs search static client initializer to ZBSearch (initDB), add zbsearch dependency, and align docs lint tooling with ESLint 9 compatibility so npm run lint passes.
Updates multiple frontend packages (antd, react-hook-form, storybook, vite, swagger-ui-react, playwright, typescript-eslint, etc.) and Go dependencies (gopsutil, gorm postgres driver, pion/transport, ugorji/codec, genproto, and others). Also replaces `__dirname` with `import.meta.dirname` in vite.config.js for ESM compatibility.
The "go: build" task hardcoded bin/3x-ui.exe, so building on Linux
produced a binary carrying a Windows extension. It now emits bin/3x-ui
and keeps the .exe name behind a windows override.
The Postgres launch config prepended C:\Program Files\PostgreSQL\18\bin
to PATH on every platform. Linux separates entries with ':', not ';', so
that string fused into the first real PATH entry and clobbered it. Moved
it into a windows block, which is where the pg_dump/pg_restore lookups
in ServerService need it anyway.
Saving a client walks every inbound it is attached to and calls
UpdateInboundClient, which re-keys the client's email in inbound_client_ips
to the spelling in the edited settings. The email match is EqualFold, so when
an inbound's settings JSON drifted in case from the client record the panel
issues a case-only rename of the tracking row.
inbound_client_ips.client_email is unique and case-sensitive, and the
IP-limit job keys its rows on whatever casing Xray reports, so both spellings
can already be present. The rename then aborts the whole edit with
"duplicate key value violates unique constraint
uni_inbound_client_ips_client_email" — the client could not be saved at all,
including when only adding an inbound to it.
The caller only renames onto an identity no live client holds, so a row on
the target email is stale IP tracking: delete it before renaming. The blob is
rebuilt by the next scan anyway.
Fact-checked every line of CLAUDE.md against the tree. Six claims were wrong,
and two told an agent the opposite of the truth.
The file said nothing checks endpoints.ts against the Go routes and nothing
fails the build on a missing i18n key. Both guards exist and both run in
make verify: TestRouteRegistryContract diffs the real router against the
registry in both directions, and i18n-dead-keys.test.ts rejects a locale that
misses an en-US key as well as an en-US key nothing references. An agent
trusting the old text either skips a step it thinks is unenforced or is
blindsided when a "silent" omission turns the suite red.
The rest: the Go locale returns an empty string for an unknown key, not the raw
key; mtg-multi is a prebuilt binary fetched at build time, not a Go dependency
built from source; commits are type(area): summary, not <area>: summary, and
perf is in active use; make verify is the fast gate, not a mirror of CI, which
also runs race, vulncheck, a live-Postgres job where a SKIP is a failure, and a
fuzz smoke.
Add the five facts most likely to burn an agent, all reproduced before writing
them down. A fresh clone has no internal/web/dist, so go build dies on the embed
pattern while thirty-odd packages pass — it reads as a broken repo rather than a
missing make dist-stub. Every state-changing inbound/client op must dispatch
through runtime.Runtime; a direct xray/api.go call passes all local tests and
silently breaks every multi-node install, which is exactly what a hard rule is
for. Node 24 is required because make gen imports .ts directly. Postgres, xray
e2e and scale tests skip themselves without their env vars. An endpoint change
has a fourth step nothing checks: syncing docs/public/openapi.json.
Definition of done loses its first step — verify's gen-check already runs gen
and fails on a dirty generated diff.
A client that hit its quota or expiry was disabled, then destroyed on both
panels a few seconds later. Five defects fed the same hard delete.
ReconcileNode pushed buildRuntimeInboundForAPI, which strips disabled
clients. Every other call site targets an in-memory Xray config, where
dropping a user is harmless; a node target is a peer panel's DATABASE, so
the node deleted the row, stopped reporting it, and the master mirrored that
deletion back. Split the builder in two: buildInboundForNodePush injects
fallbacks only, buildInboundForLocalRuntime adds the strip on top. The names
now say which targets they are safe for.
setRemoteTrafficLocked trusted a config_dirty the caller sampled before the
snapshot round-trip. A client added inside that window commits on the same
serialized writer and marks the node dirty, but the merge still treated the
older snapshot as authoritative and deleted it. Re-read the flag inside the
writer.
In "selected" sync mode, FilterNodeSnapshot strips a deselected tag, but the
sweep loaded every inbound with node_id set, so deselecting a tag read as
"the node deleted it" and wiped an inbound the node still serves. Skip tags
outside the node's managed set.
A failed SyncInbound was logged and swallowed; on SQLite the transaction
still commits, and the sweep then deleted the innocent clients whose links
that failure had left unbuilt. Skip the sweep for such an inbound, and close
the trigger: SyncInbound now stores the trimmed email it looks up by, and
email validation rejects every unicode space rather than only U+0020.
ClientService.Delete tombstones up front and deliberately keeps the record
when an inbound fails, so the next attempt can retry the leftovers. The
tombstone did not lift with it, so the next merge dropped the client from
the synced settings and finished the deletion this path had refused. Add
withdrawClientTombstones on every failure path, in BulkDelete too.
Finally, make the sweep itself recoverable. "Ended the merge unattached" is
true for a real remote deletion and equally true for a bad merge, so it now
stamps sync_orphaned_at instead of deleting; any later merge that sees the
client attached clears the mark, and a reaper removes only what stayed
orphaned past the grace period. The traffic row survives that window too, or
a reclaimed client would come back with its usage, quota and expiry reset.
The mark is written by this sweep alone, so orphans from any other cause
keep their existing manual-cleanup semantics.
FetchVlessFlags returns (empty map, nil) whenever the bind succeeds but the
search yields nothing usable — a renamed OU, a service account that lost read
on the user attribute, a filter that stopped matching. The only guard on the
destructive half of the sync was `err != nil`, so that answer was read as
"every user is gone" and the job detached every client from the configured
inbounds, once a minute, for as long as the directory stayed broken.
Gate auto-delete behind autoDeleteSafeForFetch: refuse an empty fetch, and
refuse one that collapsed below half of the last successful sync, which is a
misconfigured directory far more often than real churn.
Also stop splitCsv from defaulting an empty string to DefaultTruthyValues.
That default belongs to the truthy-value setting, but splitCsv is also what
parses ldapInboundTags, so an unconfigured tag list silently resolved to
["true","1","yes","on"]. It only ever bounded the blast radius by accident.
Three agent-facing rules, each written after the same mistake showed up in
review.
Comments were banned outright, which the codebase itself contradicts on
almost every file — the ban pushed real invariants out of the code entirely.
Allow them, but cap a block at 2 lines and spend those lines on the *why* a
name cannot carry.
Add a scope rule: the fix must be the smallest change that removes the root
cause. A small bug does not earn new columns, jobs, abstractions or config;
if it genuinely needs architecture, agree on that first instead of shipping
it alongside the fix.
Add two testing rules: a test must go red when its fix is reverted, and it
must cover something that can actually break. A test that passes either way
certifies nothing and is then cited as proof the fix works.
Frontend deps: @hookform/resolvers 5.4.3 -> 5.5.7, Storybook 10.5.4 -> 10.5.5
across the four packages we declare, globals 17.7.0 -> 17.8.0, and jsdom
29.1.1 -> 30.0.1. The jsdom major replaces its CSS and selector stack --
@asamuzakjp/css-color 5 -> 6, @asamuzakjp/dom-selector 7 -> 8, undici 7 -> 8,
nwsapi and generational-cache folded into their parents, whatwg-url 17 nested
underneath. Nothing in the Vitest suites reaches those directly and the whole
frontend gate (typecheck, lint, tests, build, Storybook compile) is green.
Panel frontend version to 0.6.0.
Backend deps: mattn/go-sqlite3 1.14.48 -> 1.14.49 and valyala/fasthttp
1.72.0 -> 1.73.0, plus the golang.org/x/exp and genproto/googleapis/rpc
indirect bumps that came with them.
Go tests: modernize -fix output, covering range-over-int, sync.WaitGroup.Go
in place of manual Add/Done pairs, maps.Copy, and Go 1.26 new(expr) for
pointer-to-value in the forwarded-trust table. The storedAs helper is deleted
instead of being left behind a //go:fix inline directive -- keeping it that way
fails govet on the one call site the rewrite did not reach, and every caller now
takes new(...) directly. Behaviour is unchanged.
DnsTab: the hosts-sync effect tested dns while declaring dnsEnabled in its
dependency array. Both carry the same truth value, so this is exhaustive-deps
hygiene rather than a behaviour change.
The clients page was slow on panels with many clients for two independent
reasons: the server rebuilt the whole picture on every request, and the
browser rebuilt the whole table on every poll.
Server side, ListPaged loaded every client row, every client_inbounds link
and every client_traffics row into Go memory, then filtered, sorted and
paginated in a loop -- on a request the page repeats every five seconds.
Every predicate now runs in SQL and only the requested page's ids are
hydrated, so the cost tracks the page size rather than the client count.
Measured on SQLite with a realistic status mix: the default view at 100k
clients goes from 1,072ms to 64ms. Behaviour is preserved deliberately in
the subtle places -- the cross-panel global-traffic overlay is folded into
the same used-bytes expression the predicates and sort use, LIKE wildcards
are escaped so a search for "a_b" stays literal, and the two different
tiebreak rules the in-memory comparator had are reproduced per sort key.
The summary's per-bucket email lists are capped at 200 with exact counters
beside them. They only back hover popovers, but shipping every match made
the response grow with the panel: at 100k clients it carried ~42k emails,
and the page revalidated all of them through a strict Zod parse every five
seconds. The popover now shows a "+N" chip for the remainder.
Browser side, the page fired three sequential list requests per load and
threw the first two away: the query went out before the persisted sort was
applied, and again before the configured page size was known -- 0 meaning
"one long page" is indistinguishable from "not loaded yet". The page size
is now derived rather than mirrored through an effect, and the previous
visit's value is remembered so the single request goes out at mount instead
of queueing behind /setting/defaultSettings.
Then the per-poll work. Reading isFetching made it a tracked property, so
the refetch interval notified twice per cycle and re-rendered the page even
when structural sharing left the data identical. Xray reports a traffic row
per client whether or not it moved bytes, so the speed map was mostly zeros
and was replaced wholesale every push; zero rows are now dropped and an
unchanged result returns the previous object, which lets React bail out
instead of re-rendering. The five Tooltip-wrapped buttons and the inbound
chips per row do not depend on traffic at all and are now memoised, keyed on
the email because a push replaces the row object of every client whose
counters moved. antd's hashed:false drops 3,311 :where(.css-<hash>) wrappers
and 29% of the generated stylesheet, and a pinned cssVar key stops each of
the eleven page-level ConfigProviders minting its own token scope.
Two callers that only need the mutations, GroupsPage and ClientBulkAddModal,
no longer start the list query -- the groups page had been polling the full
paged list every five seconds for data it never renders.
The resolve-conflicts job runs on issue_comment, a privileged trigger: it
holds GITHUB_TOKEN, the Claude OAuth token and the push PAT, and it checks
out fork code with `gh pr checkout`. The only gate was that the commenter
is the repository owner, which says nothing about the code that ends up in
the workspace. A contributor could force-push to the pull request head
between the owner asking for the merge and the runner fetching it, so the
owner reviews one tree and the job runs another.
Verify before anything is checked out that the head repository was last
pushed to before the triggering comment was written, and refuse the run
otherwise. Pin the head SHA reported by that check and abort if the commit
`gh pr checkout` lands on differs, which closes the remaining window
between the check and the fetch. Require author_association to be OWNER
alongside the existing login comparison.
This also clears CodeQL actions/untrusted-checkout/high, which for
issue_comment triggers demands both an actor/association check and a
comment-vs-head-date check dominating the checkout.
Replace the ten-small-cards overview with an action bar, four vitals
tiles carrying 72-sample sparklines seeded from /server/history, a
two-series throughput chart, a TCP/UDP connections chart, and a
grouped system strip (uptime xray|os, panel ram|threads, ip
addresses). StatusCard and XrayStatusCard are deleted; every modal
stays reachable from the action bar, the Xray error message moves
into a tooltip on the state pill, and the panel version text keeps
opening the update modal (the dev-channel switch lives there) even
when no update is available. Live values sit beside the
upload/download and tcp/udp legends, a health sentence appears only
when a vital crosses the shared warn/crit thresholds now exported
from models/status, and load average is left to System History.
The sidebar becomes an auto-collapsed 72px icon rail that expands as
an overlay on hover: rail width, brand-row height and menu paddings
are pinned so nothing shifts during the transition, the collapsed-menu
tooltips are disabled, hover state survives the per-page sidebar
remounts (with a matches(':hover') resync), and the manual collapse
trigger is gone.
Sparkline gains rgb()/rgba() support in its fill gradient, a
showLegend prop so pages stop reaching into its internals, and loses
a dependency-less repaint effect that doubled canvas paints. Chart
tooltips show clock time via the new TimeFormatter.formatClock;
accents come from theme tokens instead of status.cpu.color. Verified
by screenshot at 390/800/1150/1280/1400/1600px in light and dark,
en and fa-IR, plus programmatic geometry checks on the sidebar.
Locale files gain 8 keys and lose 9 dead ones across all 13
languages.
resolve-conflicts is the one job that puts a pull request's own tree in
the working directory while holding CLAUDE_CODE_OAUTH_TOKEN,
CLAUDE_BOT_PAT and a write-scoped token, which is what CodeQL alert 101
(actions/untrusted-checkout) points at. Nothing in the job executes that
tree and the trigger is gated on the repository owner, so the alert is
not reachable as written, but two of its guards were weaker than they
read.
Git hooks were neutered only after gh pr checkout had already run, so
the guard sat one step behind the checkout it exists to cover; it now
precedes it. The conflicted paths are concatenated into the
--allowedTools value handed to the model, so a path carrying a comma or
a parenthesis would widen that allowlist. Only both-modified paths reach
that code today, which means they already exist in the base repository,
but the merge is now handed back to the maintainer unless every
conflicted path is plain [A-Za-z0-9._/-].
The file's header comment block is dropped.
The Nodes page cache is overwritten wholesale by the heartbeat websocket
push, but the job broadcast a raw []*model.Node while the REST list returns
[]*service.NodeView. model.Node tags the api token json:"-" and carries no
hasApiToken field, so every push stripped the flag the edit form reads to
decide whether a token is already stored.
One 5s tick after the page loaded, editing any non-mTLS node then failed
with "Name, address, port and API token are required" — and stayed failed,
because setQueryData refreshes dataUpdatedAt, so the query never goes stale
and never refetches the intact REST payload.
Broadcast the NodeView read contract instead.
The frontend's golden fixtures are the panel's model of an xray config, but
nothing ever asked xray-core whether it would accept them: the snapshots only
prove the Zod schemas agree with themselves. Building every fixture through the
same config builders the panel hands its config to — conf.InboundDetourConfig
for the full-config and AddInbound paths, conf.RouterConfig for
ApplyRoutingConfig, conf.DNSConfig for the dns section — found seven the core
refuses, three of them reachable from the panel's own UI. A refusal is not
scoped to one inbound: the config fails to load and every inbound stays down.
Hysteria: xray-core builds version 2 only, in both the protocol settings and
the transport settings, but the inbound settings schema accepted any version
from 1 up and its comment claimed upstream still supported v1. Both fixtures
carried version 1. The schema now pins 2, GenXrayInboundConfig heals stored
rows on the way out the way it already heals shadowsocks ciphers and wireguard
peers, and the share link drops the dead hysteria:// scheme — the subscription
server already emitted hysteria2:// for the same inbound.
XHTTP uplinkDataPlacement: both transport forms offered "query", which the core
has never accepted for that field (auto and body always, cookie and header in
packet-up mode). Replaced with auto, which was missing, and the default label
now names auto rather than body.
FinalMask items: switching an item to the rand-driven array kind wrote
packet:[] next to the rand. xray-core counts an empty array as a packet and
every item kind is exclusive, so noise answers "len(item.Packet) > 0 &&
item.Rand.To > 0" and header-custom "exactly one item kind must be set". The
editor now clears the packet, and GetXrayConfig strips the residue from rows
already saved with it.
The remaining four were stale fixtures: an xmc mask still on the usernames
shape v26.7.28 replaced with profiles, a fragment mask with no length, and
header-custom and noise items passing an array to the string packet kind — all
shapes the panel's own editors cannot produce.
golden_fixtures_xray_test.go keeps this from drifting again: every fixture in
every category is built through xray-core on each run, with a self-signed pair
standing in for the deployment certificate paths, so the next core bump reports
which fixture it broke.
Exercising the whole XrayAPI surface against a real xray-core 26.7.28 (the
version go.mod pins) turned up a way for ordinary panel activity to kill the
core process, plus two smaller mismatches with what the core actually does.
buildUserAccount picked the shadowsocks account type by falling through to a
2022 account whenever the cipher was not one of six hardcoded names. xray's
legacy and 2022 inbounds cast the account they are handed without checking
(proxy/shadowsocks/validator.go, proxy/shadowsocks_2022/inbound_multi.go), so
the wrong type is not an error — it panics the core and drops every connection
on the server. The fallback was reachable without any misconfiguration:
autoRenewClients hands AddUser the client object straight out of the inbound's
settings, where the cipher lives under "method", never "cipher", so every
auto-renewed client on a legacy-cipher shadowsocks inbound took xray down. The
xray-valid aead_* aliases hit it too. The cipher is now read from either key,
matched with the same table (and case-insensitivity) the core's own conf
package uses, and an unrecognized one is an error instead of a guess.
The legacy shadowsocks validator is also the only one that accepts a second
user under an email it already holds, and RemoveUser then drops just one of
them — a disabled or expired client kept connecting. AddUser now drops the
email first on that account type so a single removal fully revokes the client.
GetTraffic skipped every stat the first time it saw it. xray creates a
counter on a user's first use, so that dropped a new client's traffic for a
whole polling interval, as did the counter reset after a core restart. Only
the first poll of a process is a baseline now; later, unseen and rewound
counters both count from zero.
Also fixes three unchecked settings["method"].(string) assertions that panic
the panel on a shadowsocks inbound whose settings carry no method, and bounds
TestRoute's port so an out-of-range value cannot wrap into the uint32 the
core is asked about.
Tests: api_users_e2e_test.go drives add/remove for every protocol against a
real core and asserts it survives each one (skipped unless XRAY_E2E_BINARY is
set); the account-type, traffic-delta and renew paths get unit coverage.
Bump xtls/xray-core to 5ca6f4b7d4dc (v26.7.28) and move the three binary
pins (DockerInit.sh, the Linux and Windows URLs in release.yml) in lockstep
so the in-process conf.Build() validation and the child binary agree.
XMC finalmask (#6487) is the breaking change. The mask's `usernames` string
list is gone, replaced by a required `profiles` array whose entries each need
a 3-16 character [A-Za-z0-9_] username, a parseable UUID and both Mojang
texture fields; the "default to Dream when empty" fallback was removed, so an
xmc mask saved by an older panel now fails to build and takes the whole
config down with it rather than degrading one inbound.
The textures are a signed blob only Mojang's session server can issue, so a
legacy username cannot be upgraded automatically. The panel now:
- rejects an incomplete xmc mask at save time (AddInbound/UpdateInbound),
pointing at the specific field that is missing;
- drops only the offending mask when generating the core config, for rows
that never went through the form (upgrade, node sync, restored backup,
direct DB edit), warning which inbound lost its obfuscation instead of
leaving every inbound offline;
- carries legacy usernames into profile stubs in the finalmask form so the
operator keeps their player names and sees exactly what still needs
filling in, and edits profiles through a list editor.
No destructive DB migration: unlike the removed shadowsocks ciphers there is
no valid replacement to rewrite to, and dropping the mask from stored rows
would discard the operator's hostname and password for config they can still
repair. The generation-time strip already prevents the startup failure.
Also track the core's xmux maxConnections fallback, lowered from 6 to 3 for
anti-TSPU, in the fresh-XMUX seed so a new panel config matches what the core
would pick on its own.
TUN gained a `desc` key and random utunN naming, but the Go validator no
longer accepts TUN inbounds and the panel only renders legacy saved rows, so
nothing there needs adapting. The remaining commits are REALITY log-warning
wording, gRPC/XHTTP localAddr accuracy and a routing tweak, none of which
change the JSON config surface.
Tests cross-check the panel's profile predicate against conf.XMCProfile.Build()
so a future core release that tightens or relaxes the rules fails loudly
rather than silently emitting configs the core refuses to start on.
This reverts commit c004c18d90.
Showing {{EMAIL}}/{{USERNAME}} on the first subscription-body link only is
intentional, not an oversight in 876d55f2. Restoring the behaviour and the
tests that pin it.
Making the identity tokens configurable is the sanctioned route for the
operators asking for them on every link (#5935), rather than flipping the
default for everyone.
CLAUDE.md rules out // line comments in committed Go. The rationale they
carried is in the commit messages for each fix; doc comments that already
existed are kept, updated where the code they describe changed.
Also replaces reflect.Ptr with reflect.Pointer and rewrites the YAML keyword
alternation as a lookup table, both flagged by golangci-lint.
Sanaei