alireza0 确认 3x-ui 关闭 fail2ban 后仍应记录客户端 IP

alireza0 在 3x-ui PR #6830 中确认,XUI_ENABLE_FAIL2BAN 只应控制限制执行;仅采集模式也应经过合并、30 分钟窗口及 CAS 写入,解决关闭 fail2ban 后不记录 IP,以及采集覆盖近期记录的问题。他称已通过变异检查验证三组测试能捕获相关回退,internal/web/job 的竞态与随机顺序测试通过,lint 无问题。合并前仍要求删除只保护生产中不会出现的零值任务的防御分支和对应测试,并调整注释位置。核对时 PR 仍未合并。

作者原文

Thanks, the root cause is right.

Gating only enforcement on XUI_ENABLE_FAIL2BAN, and routing collection-only runs through the merge + 30-minute window + CAS path, is the correct fix for #6722 and #6587. I mutation-checked the tests:

  • Restoring the early return turns TestRun_RecordsIpsWhenFail2banDisabled red.
  • Dropping the f2bEnabled guard turns TestResolveEnforce red.
  • Restoring the overwrite turns TestProcessObserved_CollectionOnlyKeepsRecentWindow red.

internal/web/job passes with -race -shuffle=on and lint is clean.

Two small things before merge:

1. Please drop the nil-source guard and TestRun_ZeroValueJobIsSafeNoOp.
Production never runs a zero-value job: NewCheckClientIpJob always sets onlineUsers. No existing test calls Run() on &CheckClientIpJob{} either. The zero-value sites only call getInboundByEmail, checkFail2BanInstalled, hasLimitIp and disconnectClientTemporarily, which never reach collectFromOnlineAPI. The only Run() on a zero value is the new test itself. So the guard protects an input that never happens, and the test only checks that guard. Per CLAUDE.md ("tests of … an input the function can never receive"), both should go.

2. Comment block in Run() (check_client_ip_job.go, above needsBinary).
It is now three lines, and our limit is two per block. It also mixes two topics. "Read only when the limit is actually applied …" describes the loadAllowlist() call under if enforce, but the new Windows line now sits between it and that call. Please keep the Windows line next to needsBinary and move the two allowlist lines back above if enforce { j.allowlist = j.loadAllowlist() }.