fix: Phase 3 line-walk fixes (3a-3d)
F3a-01 agent.py: _safe_float() prevents ValueError from non-numeric metric values F3b-01 schedule_runner.py: fix tz-aware minus naive DateTime TypeError F3b-02 schedule_runner.py: cron field divided by 0 returns False instead of crash F3c-01 redis/client.py: mask credentials in REDIS_URL log output F3d-01 main.py: cancel background tasks when primary worker lock is lost F3d-02 main.py: strip whitespace from CORS origins after split Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,24 @@
|
||||
# Phase 3 逐行走读修复
|
||||
|
||||
**日期**: 2026-05-23
|
||||
**批次**: 3a(安全核心)、3b(background)、3c(基础设施)、3d(main/middleware)
|
||||
|
||||
## 修复清单
|
||||
|
||||
| ID | 文件 | 级 | 说明 | 修复 |
|
||||
|----|------|----|------|------|
|
||||
| F3a-01 | `agent.py:97-102` | P2 | `float(cpu/mem/disk)` 对非数字 Agent 数据 → ValueError/500 | 提取 `_safe_float()` 包裹转换,非数字值 WARNING 级跳过 |
|
||||
| F3b-01 | `schedule_runner.py:84` | P1 | `now`(tz-aware)- `last_run_at`(MySQL naive)→ TypeError → 已跑调度永远失败 | 统一去除 tzinfo 后再减 |
|
||||
| F3b-02 | `schedule_runner.py:47` | P3 | cron `*/0` → ZeroDivisionError | step==0 时返回 False 跳过 |
|
||||
| F3c-01 | `redis/client.py:49` | P2 | Redis URL 含 auth 时密码写入日志 | 启动日志脱敏(`user:**@host`) |
|
||||
| F3d-01 | `main.py:116-118` | P2 | primary lock 丢失时续锁退出,后台任务继续双跑 | lock 丢失时 cancel 所有后台任务 |
|
||||
| F3d-02 | `main.py:344` | P2 | CORS origins 不去除空格 | `o.strip()` 过滤 |
|
||||
|
||||
## 审查结论
|
||||
|
||||
| 批次 | 覆盖 | 结论 |
|
||||
|------|------|------|
|
||||
| 3a | auth_jwt.py、webssh.py、crypto.py、agent.py、dependencies.py | 1 BUG 修复;安全核心均正确 |
|
||||
| 3b | heartbeat_flush、retry_runner、schedule_runner、self_monitor | 2 BUG 修复;整体逻辑正确 |
|
||||
| 3c | engine_compat.py、session.py、redis/client.py | 1 日志脱敏;新增文件无安全问题 |
|
||||
| 3d | main.py | 2 BUG 修复;中间件注册顺序正确 |
|
||||
+26
-14
@@ -83,6 +83,17 @@ def _threshold_for(thresholds: dict, metric: str) -> float:
|
||||
return float(thresholds.get(metric, defaults.get(metric, 80)))
|
||||
|
||||
|
||||
def _safe_float(value, name: str) -> Optional[float]:
|
||||
"""Convert agent-supplied metric value to float, discarding malformed values."""
|
||||
if value is None:
|
||||
return None
|
||||
try:
|
||||
return float(value)
|
||||
except (ValueError, TypeError):
|
||||
logger.warning(f"Agent sent non-numeric {name}: {value!r!s:.50} — skipped")
|
||||
return None
|
||||
|
||||
|
||||
def _detect_alerts(system_info: dict, thresholds: dict) -> list:
|
||||
"""Detect metrics exceeding alert thresholds
|
||||
|
||||
@@ -90,16 +101,16 @@ def _detect_alerts(system_info: dict, thresholds: dict) -> list:
|
||||
E.g. [("cpu", 85.3), ("mem", 92.1)]
|
||||
"""
|
||||
alerts = []
|
||||
cpu = system_info.get("cpu_usage") or system_info.get("cpu")
|
||||
mem = system_info.get("mem_usage") or system_info.get("mem")
|
||||
disk = system_info.get("disk_usage") or system_info.get("disk")
|
||||
cpu = _safe_float(system_info.get("cpu_usage") or system_info.get("cpu"), "cpu")
|
||||
mem = _safe_float(system_info.get("mem_usage") or system_info.get("mem"), "mem")
|
||||
disk = _safe_float(system_info.get("disk_usage") or system_info.get("disk"), "disk")
|
||||
|
||||
if cpu is not None and float(cpu) > _threshold_for(thresholds, "cpu"):
|
||||
alerts.append(("cpu", float(cpu)))
|
||||
if mem is not None and float(mem) > _threshold_for(thresholds, "mem"):
|
||||
alerts.append(("mem", float(mem)))
|
||||
if disk is not None and float(disk) > _threshold_for(thresholds, "disk"):
|
||||
alerts.append(("disk", float(disk)))
|
||||
if cpu is not None and cpu > _threshold_for(thresholds, "cpu"):
|
||||
alerts.append(("cpu", cpu))
|
||||
if mem is not None and mem > _threshold_for(thresholds, "mem"):
|
||||
alerts.append(("mem", mem))
|
||||
if disk is not None and disk > _threshold_for(thresholds, "disk"):
|
||||
alerts.append(("disk", disk))
|
||||
|
||||
return alerts
|
||||
|
||||
@@ -114,11 +125,12 @@ def _detect_recovery(system_info: dict, prev_alerts: set, thresholds: dict) -> l
|
||||
parts = prev.split(":")
|
||||
if len(parts) != 2:
|
||||
continue
|
||||
metric, prev_val = parts[0], float(parts[1])
|
||||
current = system_info.get(f"{metric}_usage") or system_info.get(metric)
|
||||
|
||||
if current is not None and float(current) < _threshold_for(thresholds, metric):
|
||||
recoveries.append((metric, float(current)))
|
||||
metric = parts[0]
|
||||
current = _safe_float(
|
||||
system_info.get(f"{metric}_usage") or system_info.get(metric), metric
|
||||
)
|
||||
if current is not None and current < _threshold_for(thresholds, metric):
|
||||
recoveries.append((metric, current))
|
||||
|
||||
return recoveries
|
||||
|
||||
|
||||
@@ -45,6 +45,8 @@ def _field_match(expr: str, value: int) -> bool:
|
||||
return True
|
||||
if part.startswith("*/"):
|
||||
step = int(part[2:])
|
||||
if step == 0:
|
||||
return False # */0 is invalid; skip rather than ZeroDivisionError
|
||||
return value % step == 0
|
||||
if "-" in part:
|
||||
low, high = part.split("-", 1)
|
||||
@@ -79,9 +81,15 @@ async def schedule_runner_loop():
|
||||
if not _cron_match(schedule.cron_expr, now):
|
||||
continue
|
||||
|
||||
# Don't re-trigger if already ran this minute
|
||||
# Don't re-trigger if already ran this minute.
|
||||
# MySQL DateTime is stored without tz info (naive), so strip tz from now
|
||||
# before subtraction to avoid TypeError.
|
||||
if schedule.last_run_at:
|
||||
seconds_since = (now - schedule.last_run_at).total_seconds()
|
||||
last_run = schedule.last_run_at
|
||||
now_naive = now.replace(tzinfo=None)
|
||||
if last_run.tzinfo is not None:
|
||||
last_run = last_run.replace(tzinfo=None)
|
||||
seconds_since = (now_naive - last_run).total_seconds()
|
||||
if seconds_since < SCHEDULE_CHECK_INTERVAL:
|
||||
continue
|
||||
|
||||
|
||||
@@ -46,7 +46,17 @@ async def init_redis(app: FastAPI) -> None:
|
||||
|
||||
# Mandatory startup validation (ADR-009)
|
||||
await _redis.ping()
|
||||
logger.info(f"Redis connected: {settings.REDIS_URL} (BlockingConnectionPool, max=50)")
|
||||
# Mask credentials in URL before logging (redis://user:pwd@host → redis://**@host)
|
||||
_safe_url = settings.REDIS_URL
|
||||
try:
|
||||
from urllib.parse import urlparse, urlunparse
|
||||
_p = urlparse(settings.REDIS_URL)
|
||||
if _p.password:
|
||||
_p = _p._replace(netloc=f"{_p.username or ''}:**@{_p.hostname}:{_p.port or 6379}")
|
||||
_safe_url = urlunparse(_p)
|
||||
except Exception:
|
||||
_safe_url = "redis://<masked>"
|
||||
logger.info(f"Redis connected: {_safe_url} (BlockingConnectionPool, max=50)")
|
||||
|
||||
except Exception as e:
|
||||
logger.error(f"Redis startup validation failed: {e}")
|
||||
|
||||
+4
-2
@@ -114,7 +114,9 @@ async def _renew_primary_lock():
|
||||
if current == worker_id:
|
||||
await redis.expire(_PRIMARY_LOCK_KEY, _PRIMARY_LOCK_TTL)
|
||||
else:
|
||||
logger.warning("Primary worker lock lost — another worker took over")
|
||||
logger.warning("Primary worker lock lost — another worker took over; cancelling background tasks")
|
||||
for task in _background_tasks:
|
||||
task.cancel()
|
||||
break
|
||||
except Exception as e:
|
||||
logger.error(f"Primary lock renewal failed: {e}")
|
||||
@@ -341,7 +343,7 @@ app.add_middleware(DbSessionMiddleware)
|
||||
# CORS — restrict to actual frontend origin (ECC security fix: was allow_origins=["*"])
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origins=settings.CORS_ORIGINS.split(",") if settings.CORS_ORIGINS else ["http://localhost:8600"],
|
||||
allow_origins=[o.strip() for o in settings.CORS_ORIGINS.split(",") if o.strip()] if settings.CORS_ORIGINS else ["http://localhost:8600"],
|
||||
allow_credentials=True,
|
||||
allow_methods=["*"],
|
||||
allow_headers=["*"],
|
||||
|
||||
Reference in New Issue
Block a user