Skip to content

Commit 69c10f9

Browse files
committed
fix(job): handle access-log errors and resolve IP-limit clients relationally
The IP-limit job swallowed access-log open/copy/truncate errors and matched a client's inbound with a fragile settings LIKE '%email%' substring query. Handle those errors with early returns, verify the access log is readable, and resolve the client to its inbound via the clients/client_inbounds join, keeping the substring scan only as a fallback.
1 parent 20094c8 commit 69c10f9

2 files changed

Lines changed: 108 additions & 13 deletions

File tree

internal/web/job/check_client_ip_job.go

Lines changed: 63 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -134,21 +134,34 @@ func (j *CheckClientIpJob) collectFromOnlineAPI() (map[string]map[string]int64,
134134

135135
func (j *CheckClientIpJob) clearAccessLog() {
136136
logAccessP, err := os.OpenFile(xray.GetAccessPersistentLogPath(), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644)
137-
j.checkError(err)
137+
if err != nil {
138+
j.checkError(err)
139+
return
140+
}
138141
defer logAccessP.Close()
139142

140143
accessLogPath, err := xray.GetAccessLogPath()
141-
j.checkError(err)
144+
if err != nil {
145+
j.checkError(err)
146+
return
147+
}
142148

143149
file, err := os.Open(accessLogPath)
144-
j.checkError(err)
150+
if err != nil {
151+
j.checkError(err)
152+
return
153+
}
145154
defer file.Close()
146155

147-
_, err = io.Copy(logAccessP, file)
148-
j.checkError(err)
156+
if _, err = io.Copy(logAccessP, file); err != nil {
157+
j.checkError(err)
158+
return
159+
}
149160

150-
err = os.Truncate(accessLogPath, 0)
151-
j.checkError(err)
161+
if err = os.Truncate(accessLogPath, 0); err != nil {
162+
j.checkError(err)
163+
return
164+
}
152165

153166
j.lastClear = time.Now().Unix()
154167
}
@@ -188,8 +201,16 @@ func (j *CheckClientIpJob) processLogFile(enforce bool) bool {
188201
emailRegex := regexp.MustCompile(`email: (.+)$`)
189202
timestampRegex := regexp.MustCompile(`^(\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2})`)
190203

191-
accessLogPath, _ := xray.GetAccessLogPath()
192-
file, _ := os.Open(accessLogPath)
204+
accessLogPath, err := xray.GetAccessLogPath()
205+
if err != nil {
206+
j.checkError(err)
207+
return false
208+
}
209+
file, err := os.Open(accessLogPath)
210+
if err != nil {
211+
j.checkError(err)
212+
return false
213+
}
193214
defer file.Close()
194215

195216
// Track IPs with their last seen timestamp
@@ -403,6 +424,15 @@ func (j *CheckClientIpJob) checkAccessLogAvailable(iplimitActive bool) bool {
403424
return false
404425
}
405426

427+
file, err := os.Open(accessLogPath)
428+
if err != nil {
429+
if iplimitActive {
430+
logger.Warning("[LimitIP] Access log is not readable:", err)
431+
}
432+
return false
433+
}
434+
_ = file.Close()
435+
406436
return true
407437
}
408438

@@ -685,10 +715,30 @@ func (j *CheckClientIpJob) getInboundByEmail(clientEmail string) (*model.Inbound
685715
db := database.GetDB()
686716
inbound := &model.Inbound{}
687717

688-
err := db.Model(&model.Inbound{}).Where("settings LIKE ?", "%"+clientEmail+"%").First(inbound).Error
689-
if err != nil {
690-
return nil, err
718+
err := db.Model(&model.Inbound{}).
719+
Joins("JOIN client_inbounds ON client_inbounds.inbound_id = inbounds.id").
720+
Joins("JOIN clients ON clients.id = client_inbounds.client_id").
721+
Where("clients.email = ?", clientEmail).
722+
First(inbound).Error
723+
if err == nil {
724+
return inbound, nil
725+
}
726+
727+
var candidates []model.Inbound
728+
if listErr := db.Model(&model.Inbound{}).Where("settings LIKE ?", "%"+clientEmail+"%").Find(&candidates).Error; listErr != nil {
729+
return nil, listErr
730+
}
731+
for i := range candidates {
732+
settings := map[string][]model.Client{}
733+
if jsonErr := json.Unmarshal([]byte(candidates[i].Settings), &settings); jsonErr != nil {
734+
continue
735+
}
736+
for _, client := range settings["clients"] {
737+
if client.Email == clientEmail {
738+
return &candidates[i], nil
739+
}
740+
}
691741
}
692742

693-
return inbound, nil
743+
return nil, err
694744
}

internal/web/job/check_client_ip_job_integration_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ func setupIntegrationDB(t *testing.T) {
5959
// seed an inbound whose settings json has a single client with the
6060
// given email and ip limit.
6161
func seedInboundWithClient(t *testing.T, tag, email string, limitIp int) {
62+
t.Helper()
63+
seedInboundOnlyWithClient(t, tag, email, limitIp)
64+
}
65+
66+
func seedInboundOnlyWithClient(t *testing.T, tag, email string, limitIp int) *model.Inbound {
6267
t.Helper()
6368
settings := map[string]any{
6469
"clients": []map[string]any{
@@ -83,6 +88,21 @@ func seedInboundWithClient(t *testing.T, tag, email string, limitIp int) {
8388
if err := database.GetDB().Create(inbound).Error; err != nil {
8489
t.Fatalf("seed inbound: %v", err)
8590
}
91+
return inbound
92+
}
93+
94+
func seedLinkedInboundWithClient(t *testing.T, tag, email string, limitIp int) *model.Inbound {
95+
t.Helper()
96+
inbound := seedInboundOnlyWithClient(t, tag, email, limitIp)
97+
client := &model.ClientRecord{Email: email}
98+
if err := database.GetDB().Create(client).Error; err != nil {
99+
t.Fatalf("seed client record: %v", err)
100+
}
101+
link := &model.ClientInbound{ClientId: client.Id, InboundId: inbound.Id}
102+
if err := database.GetDB().Create(link).Error; err != nil {
103+
t.Fatalf("seed client inbound link: %v", err)
104+
}
105+
return inbound
86106
}
87107

88108
// seed an InboundClientIps row with the given blob.
@@ -171,6 +191,31 @@ func TestRun_DisabledFail2BanSkipsProbeAndBanLog(t *testing.T) {
171191
}
172192
}
173193

194+
func TestGetInboundByEmailUsesClientInboundLink(t *testing.T) {
195+
setupIntegrationDB(t)
196+
197+
want := seedLinkedInboundWithClient(t, "linked-inbound", "exact@example.com", 1)
198+
seedInboundOnlyWithClient(t, "other-inbound", "not-exact@example.com", 1)
199+
200+
got, err := (&CheckClientIpJob{}).getInboundByEmail("exact@example.com")
201+
if err != nil {
202+
t.Fatalf("getInboundByEmail returned error: %v", err)
203+
}
204+
if got.Id != want.Id {
205+
t.Fatalf("getInboundByEmail returned inbound %d, want %d", got.Id, want.Id)
206+
}
207+
}
208+
209+
func TestGetInboundByEmailRejectsSubstringFallbackMatch(t *testing.T) {
210+
setupIntegrationDB(t)
211+
212+
seedInboundOnlyWithClient(t, "substring-only", "joann@example.com", 1)
213+
214+
if got, err := (&CheckClientIpJob{}).getInboundByEmail("ann@example.com"); err == nil {
215+
t.Fatalf("substring email matched inbound %d; want no exact match", got.Id)
216+
}
217+
}
218+
174219
// #4091 repro: client has limit=3, db still holds 3 idle ips from a
175220
// few minutes ago, only one live ip is actually connecting. pre-fix:
176221
// live ip got banned every tick and never appeared in the panel.

0 commit comments

Comments
 (0)