fix: address review feedback on stable-identity PR

Three fixes from maintainer review:

1. Logging regression (trojan/vless/vmess): when a client has no name,
   the old code fell back to the index as a log label; the new code was
   emitting `[] inbound connection`. Align with the hysteria2 pattern:
   omit the `[name]` prefix entirely when name is empty.

2. Duplicate/empty client names: trojan's Service.UpdateUsers rejects a
   repeated identity, so two clients with the same name on the same
   inbound would make UpdateUsers fail. vmess silently overwrites the
   earlier entry. vless misattributes flow. Add server-side validation
   in ClientService.Save (new, edit, addbulk) that rejects empty names
   and names already in use on any shared inbound within the same tx.

3. Remove .github/workflows/check.yml: the workflow only triggered on
   the feature branch, so it would be permanently dead after merge.
This commit is contained in:
LiGaAmG 2026-08-26 17:49:53 +03:00
parent 6174cf6726
commit dd2b4bbcf9
5 changed files with 69 additions and 60 deletions

View file

@ -1,52 +0,0 @@
name: Check patched protocols
on:
workflow_dispatch:
push:
branches:
- fix/hysteria2-stable-user-identity
jobs:
vet:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: recursive
- uses: actions/setup-go@v5
with:
go-version-file: go.mod
cache: false
- name: go vet (patched protocols)
env:
CGO_ENABLED: 0
run: |
go vet ./core/protocol/hysteria/...
go vet ./core/protocol/hysteria2/...
go vet ./core/protocol/tuic/...
go vet ./core/protocol/trojan/...
go vet ./core/protocol/vless/...
go vet ./core/protocol/vmess/...
- name: gofmt check
run: |
files=$(gofmt -l \
core/protocol/hysteria/inbound.go \
core/protocol/hysteria/users.go \
core/protocol/hysteria2/inbound.go \
core/protocol/hysteria2/users.go \
core/protocol/tuic/inbound.go \
core/protocol/tuic/users.go \
core/protocol/trojan/inbound.go \
core/protocol/trojan/users.go \
core/protocol/vless/inbound.go \
core/protocol/vless/users.go \
core/protocol/vmess/inbound.go \
core/protocol/vmess/users.go)
if [ -n "$files" ]; then
echo "gofmt issues in: $files"
exit 1
fi
echo "gofmt OK"

View file

@ -188,8 +188,10 @@ func (h *Inbound) newConnection(ctx context.Context, conn net.Conn, metadata ada
}
if user != "" {
metadata.User = user
h.logger.InfoContext(ctx, "[", user, "] inbound connection to ", metadata.Destination)
} else {
h.logger.InfoContext(ctx, "inbound connection to ", metadata.Destination)
}
h.logger.InfoContext(ctx, "[", user, "] inbound connection to ", metadata.Destination)
h.router.RouteConnectionEx(ctx, conn, metadata, onClose)
}
@ -203,8 +205,10 @@ func (h *Inbound) newPacketConnection(ctx context.Context, conn N.PacketConn, me
}
if user != "" {
metadata.User = user
h.logger.InfoContext(ctx, "[", user, "] inbound packet connection to ", metadata.Destination)
} else {
h.logger.InfoContext(ctx, "inbound packet connection to ", metadata.Destination)
}
h.logger.InfoContext(ctx, "[", user, "] inbound packet connection to ", metadata.Destination)
h.router.RoutePacketConnectionEx(ctx, conn, metadata, onClose)
}

View file

@ -171,8 +171,10 @@ func (h *Inbound) newConnectionEx(ctx context.Context, conn net.Conn, metadata a
}
if user != "" {
metadata.User = user
h.logger.InfoContext(ctx, "[", user, "] inbound connection to ", metadata.Destination)
} else {
h.logger.InfoContext(ctx, "inbound connection to ", metadata.Destination)
}
h.logger.InfoContext(ctx, "[", user, "] inbound connection to ", metadata.Destination)
h.router.RouteConnectionEx(ctx, conn, metadata, onClose)
}
@ -190,9 +192,17 @@ func (h *Inbound) newPacketConnectionEx(ctx context.Context, conn N.PacketConn,
if metadata.Destination.Fqdn == packetaddr.SeqPacketMagicAddress {
metadata.Destination = M.Socksaddr{}
conn = packetaddr.NewConn(bufio.NewNetPacketConn(conn), metadata.Destination)
h.logger.InfoContext(ctx, "[", user, "] inbound packet addr connection")
if user != "" {
h.logger.InfoContext(ctx, "[", user, "] inbound packet addr connection")
} else {
h.logger.InfoContext(ctx, "inbound packet addr connection")
}
} else {
h.logger.InfoContext(ctx, "[", user, "] inbound packet connection to ", metadata.Destination)
if user != "" {
h.logger.InfoContext(ctx, "[", user, "] inbound packet connection to ", metadata.Destination)
} else {
h.logger.InfoContext(ctx, "inbound packet connection to ", metadata.Destination)
}
}
h.router.RoutePacketConnectionEx(ctx, conn, metadata, onClose)
}

View file

@ -177,8 +177,10 @@ func (h *Inbound) newConnectionEx(ctx context.Context, conn net.Conn, metadata a
}
if user != "" {
metadata.User = user
h.logger.InfoContext(ctx, "[", user, "] inbound connection to ", metadata.Destination)
} else {
h.logger.InfoContext(ctx, "inbound connection to ", metadata.Destination)
}
h.logger.InfoContext(ctx, "[", user, "] inbound connection to ", metadata.Destination)
h.router.RouteConnectionEx(ctx, conn, metadata, onClose)
}
@ -196,9 +198,17 @@ func (h *Inbound) newPacketConnectionEx(ctx context.Context, conn N.PacketConn,
if metadata.Destination.Fqdn == packetaddr.SeqPacketMagicAddress {
metadata.Destination = M.Socksaddr{}
conn = packetaddr.NewConn(bufio.NewNetPacketConn(conn), metadata.Destination)
h.logger.InfoContext(ctx, "[", user, "] inbound packet addr connection")
if user != "" {
h.logger.InfoContext(ctx, "[", user, "] inbound packet addr connection")
} else {
h.logger.InfoContext(ctx, "inbound packet addr connection")
}
} else {
h.logger.InfoContext(ctx, "[", user, "] inbound packet connection to ", metadata.Destination)
if user != "" {
h.logger.InfoContext(ctx, "[", user, "] inbound packet connection to ", metadata.Destination)
} else {
h.logger.InfoContext(ctx, "inbound packet connection to ", metadata.Destination)
}
}
h.router.RoutePacketConnectionEx(ctx, conn, metadata, onClose)
}

View file

@ -3,6 +3,7 @@ package service
import (
"bytes"
"encoding/json"
"fmt"
"strings"
"time"
@ -47,6 +48,36 @@ func (s *ClientService) GetAll() (*[]model.Client, error) {
return &clients, nil
}
// validateClientName rejects empty names and names that are already in use
// by another client sharing at least one inbound, within the same transaction.
func (s *ClientService) validateClientName(tx *gorm.DB, client *model.Client) error {
if strings.TrimSpace(client.Name) == "" {
return fmt.Errorf("client name must not be empty")
}
var inboundIds []uint
if err := json.Unmarshal(client.Inbounds, &inboundIds); err != nil || len(inboundIds) == 0 {
return nil
}
var count int64
query := tx.Raw(`
SELECT COUNT(*) FROM clients, json_each(clients.inbounds) AS je
WHERE clients.name = ? AND je.value IN ?`,
client.Name, inboundIds)
if client.Id != 0 {
query = tx.Raw(`
SELECT COUNT(*) FROM clients, json_each(clients.inbounds) AS je
WHERE clients.name = ? AND clients.id != ? AND je.value IN ?`,
client.Name, client.Id, inboundIds)
}
if err := query.Scan(&count).Error; err != nil {
return err
}
if count > 0 {
return fmt.Errorf("client name %q is already used by another client on the same inbound", client.Name)
}
return nil
}
func (s *ClientService) Save(tx *gorm.DB, act string, data json.RawMessage, hostname string) ([]uint, error) {
var err error
var inboundIds []uint
@ -58,6 +89,9 @@ func (s *ClientService) Save(tx *gorm.DB, act string, data json.RawMessage, host
if err != nil {
return nil, err
}
if err = s.validateClientName(tx, &client); err != nil {
return nil, err
}
if err = setConfigIdentity(&client); err != nil {
return nil, err
}
@ -92,6 +126,9 @@ func (s *ClientService) Save(tx *gorm.DB, act string, data json.RawMessage, host
}
now := time.Now().Unix()
for _, client := range clients {
if err = s.validateClientName(tx, client); err != nil {
return nil, err
}
if err = setConfigIdentity(client); err != nil {
return nil, err
}