fix(inbounds): close the port check-and-claim race on the serial writer (#6225)

* fix(inbounds): close the port check-and-claim race on the serial writer

AddInbound reads the port conflict outside its transaction and then commits in
a bare db.Transaction, so two overlapping creates both pass the read and both
insert. UpdateInbound already runs on the single traffic writer, and so does
the node snapshot path; AddInbound is the one inbound writer left out.

Move it onto runSerializedTx and evaluate the conflict inside the transaction,
in both AddInbound and UpdateInbound. The check and the claim then commit
together on one goroutine, which closes the window on SQLite (immediate write
lock) and PostgreSQL alike without new schema, locks or configuration.

The wildcard/specific pair is the case worth naming: those are two distinct
rows, so no unique index can reject them — only the semantic check can, and
only if nothing can interleave between it and the insert.

* fix(inbounds): restore the port check UpdateInbound lost

The previous commit deleted UpdateInbound's pre-flight conflict check and never
added the in-transaction one, so editing an inbound onto an occupied port was
accepted outright. No test covered that path, so CI stayed green.

Evaluate the conflict inside the transaction, as AddInbound already does, and
add the regression test that fails without it.

* chore: drop the accidentally committed dist build stub

internal/web/dist/.gitkeep is what make dist-stub creates locally. Committing
it changes fresh-clone behaviour for everyone: today a bare go build fails
loudly on //go:embed all:dist, which is the documented signal to run the stub
target; with the file present the build succeeds and the panel serves an empty
dist instead.

---------

Co-authored-by: n0ctal <n0ctal@users.noreply.github.com>
This commit is contained in:
n0ctal
2026-08-18 14:48:40 +05:00
committed by GitHub
parent 5c9268c431
commit 81cfd8570e
3 changed files with 124 additions and 21 deletions
+17 -19
View File
@@ -930,18 +930,11 @@ func (s *InboundService) AddInbound(inbound *model.Inbound) (*model.Inbound, boo
return inbound, false, err
}
conflict, err := s.checkPortConflict(inbound, 0)
if err != nil {
return inbound, false, err
}
if conflict != nil {
return inbound, false, common.NewError(conflict.String())
}
inbound.Tag, err = s.resolveInboundTag(inbound, 0)
tag, err := s.resolveInboundTag(inbound, 0)
if err != nil {
return inbound, false, err
}
inbound.Tag = tag
clients, err := s.GetClients(inbound)
if err != nil {
@@ -1027,10 +1020,16 @@ func (s *InboundService) AddInbound(inbound *model.Inbound) (*model.Inbound, boo
}
}
db := database.GetDB()
needRestart := false
var postCommitApply func()
err = db.Transaction(func(tx *gorm.DB) error {
err = runSerializedTx(func(tx *gorm.DB) error {
conflict, cErr := checkPortConflictTx(tx, inbound, 0)
if cErr != nil {
return cErr
}
if conflict != nil {
return common.NewError(conflict.String())
}
markDirty := false
if err := tx.Omit("ClientStats").Save(inbound).Error; err != nil {
return err
@@ -1416,14 +1415,6 @@ func (s *InboundService) UpdateInbound(inbound *model.Inbound) (*model.Inbound,
// stays scoped to its own node (the payload's nodeId is unreliable, often absent).
inbound.NodeID = oldInbound.NodeID
conflict, err := s.checkPortConflict(inbound, inbound.Id)
if err != nil {
return inbound, false, err
}
if conflict != nil {
return inbound, false, common.NewError(conflict.String())
}
// Capture the pre-edit protocol and routing state before oldInbound is
// overwritten with the new values further down, then ensure a routed
// inbound keeps a stable egress port (reusing the one already stored).
@@ -1441,6 +1432,13 @@ func (s *InboundService) UpdateInbound(inbound *model.Inbound) (*model.Inbound,
var postCommitApply func()
txErr := runSerializedTx(func(tx *gorm.DB) error {
conflict, cErr := checkPortConflictTx(tx, inbound, inbound.Id)
if cErr != nil {
return cErr
}
if conflict != nil {
return common.NewError(conflict.String())
}
if err := s.updateClientTraffics(tx, oldInbound, inbound); err != nil {
return err
}