SmartUp Developer
234 lines
9.0 KiB
Markdown
234 lines
9.0 KiB
Markdown
# SmartUp 项目优化审查报告
|
||
|
||
> 基于代码审计整理的优化点，按严重程度分级。
|
||
> 审查范围：backend/ 全部 Python 代码、frontend/src/ 全部 TS/Vue 代码、Dockerfile、docker-compose.yml。
|
||
|
||
---
|
||
|
||
## 🔴 严重 —— 生产稳定性 & 安全风险
|
||
|
||
### 1. SQLite 并发写导致锁冲突
|
||
|
||
**位置**：`backend/app/services/scheduler.py:28-91`（`_check_upstream`）
|
||
**问题**：APScheduler `BackgroundScheduler` 使用线程池执行检测任务，每个线程独立调用 `SessionLocal()` 创建 SQLAlchemy session。SQLite 的锁粒度是整个文件，多线程并发 `commit()`（尤其是 `website_sync.sync_binding()` 内多次提交）必然出现 `database is locked` 错误。
|
||
|
||
```python
|
||
# scheduler.py:28 — 每个 job 开独立 session
|
||
db: Session = SessionLocal()
|
||
...
|
||
|
||
# website_sync.py:178-180 — 内部多次 commit
|
||
db.commit()
|
||
...
|
||
db.commit()
|
||
```
|
||
|
||
**建议**：上游检测改为单线程串行队列（`apscheduler` 设置 `max_instances=1` + 单 worker），或将 SQLite 替换为 PostgreSQL。
|
||
|
||
### 2. CORS 通配符 + credentials 违反规范
|
||
|
||
**位置**：`backend/app/main.py:48-53`
|
||
**问题**：`allow_origins=["*"]` 与 `allow_credentials=True` 同时设置，浏览器端会忽略通配符（CORS 规范要求显式 origin）。当前实际行为取决于 FastAPI 中间件实现，不可靠。
|
||
|
||
```python
|
||
app.add_middleware(
|
||
CORSMiddleware,
|
||
allow_origins=["*"], # ← 通配符
|
||
allow_credentials=True, # ← 不允许与通配符共存
|
||
...
|
||
)
|
||
```
|
||
|
||
**建议**：`allow_origins` 设为显式列表（从环境变量读取），或关闭 `allow_credentials`。
|
||
|
||
### 3. HTTP 客户端每次请求新建连接，无连接池
|
||
|
||
**位置**：
|
||
- `backend/app/services/upstream_client.py`（`UpstreamClient._request`，约 L230）
|
||
- `backend/app/services/website_client.py`（`Sub2ApiWebsiteClient._request`，约 L95）
|
||
|
||
**问题**：每次 API 调用都创建新的 `httpx.Client()`，用完即销毁。一个上游检测周期（login → groups → rates）建立 3 个独立 TCP 连接；N 个上游即 3N 个连接。无连接复用、无 DNS 缓存。
|
||
|
||
```python
|
||
# upstream_client.py
|
||
def _request(self, method, path, body=None, auth=True):
|
||
with httpx.Client(timeout=self.timeout) as client: # ← 每个请求新建
|
||
resp = client.request(...)
|
||
```
|
||
|
||
**建议**：在 `__init__` 中创建 `httpx.AsyncClient` 或复用 `Client`，析构时关闭。
|
||
|
||
### 4. 快照表无限增长
|
||
|
||
**位置**：`backend/app/models/snapshot.py`（`UpstreamRateSnapshot` 模型）
|
||
**问题**：每次检测成功都 INSERT 一行，无任何清理策略。按每 10 分钟一次、数据量约 2KB/行估算：
|
||
|
||
- 1 个上游 × 1 年 ≈ 52K 行 ≈ 100MB+
|
||
- 10 个上游 × 1 年 ≈ 0.5M 行 ≈ 1GB+
|
||
|
||
`snapshot_json` 是 `Text` 字段存全量 JSON，查询 `ORDER BY captured_at DESC` 扫描行数随时间线性增长。
|
||
|
||
**建议**：定时清理（保留最近 N 条或最近 M 天），或只保存 diff 增量。
|
||
|
||
### 5. 弱默认凭据 + 无登录限流
|
||
|
||
**位置**：
|
||
- `backend/app/config.py:7-8`
|
||
- `backend/app/routers/auth.py:12-20`
|
||
- `backend/app/utils/auth.py:41-57`
|
||
|
||
**问题**：
|
||
- `admin_password` 默认值 `"changeme"`，`jwt_secret` 默认值 `"change-me-in-production"`
|
||
- `/api/auth/login` 无任何速率限制，可暴力枚举
|
||
- `/api/auth/logout` 是空操作（JWT 无黑名单），token 过期前一直有效
|
||
- JWT 不包含 `jti` 等唯一标识，无法按需吊销
|
||
|
||
```python
|
||
# config.py
|
||
admin_password: str = "changeme"
|
||
jwt_secret: str = "change-me-in-production"
|
||
|
||
# auth.py
|
||
@router.post("/logout")
|
||
def logout():
|
||
return {"message": "logged out"} # ← 纯空操作
|
||
```
|
||
|
||
**建议**：登录端点加限流（内存令牌桶或 `slowapi`）；密码至少 8 位校验；JWT 加入 `jti` + 内存/DB 黑名单。
|
||
|
||
### 6. 密码被 bcrypt 静默截断到 72 字节
|
||
|
||
**位置**：`backend/app/utils/auth.py:12-18`
|
||
**问题**：`hash_password` 和 `verify_password` 都做 `[:72]` 截断。用户设置超长密码时无提示，登录时输入完整密码也能匹配——用户永远不会发现密码被截断了。
|
||
|
||
```python
|
||
pw = password.encode("utf-8")[:72]
|
||
return bcrypt.hashpw(pw, bcrypt.gensalt()).decode("utf-8")
|
||
```
|
||
|
||
**建议**：在设置/修改密码时检查长度并给出提示，或改用非截断的哈希方案。
|
||
|
||
---
|
||
|
||
## 🟡 中等 —— 代码质量 & 维护性
|
||
|
||
### 7. 重复工具函数
|
||
|
||
**位置**：
|
||
- `backend/app/services/upstream_client.py:69-80`（`_decimal_str`）
|
||
- `backend/app/services/website_client.py:17-28`（`decimal_string`）
|
||
|
||
**问题**：同一份 Decimal 格式化逻辑在两个文件中重复实现，行为略有差异（函数名不同但逻辑相同）。修改一个必然漏掉另一个。
|
||
|
||
**建议**：抽取到 `app/utils/number.py` 统一引用。
|
||
|
||
### 8. 事务边界模糊，部分失败导致状态不一致
|
||
|
||
**位置**：`backend/app/services/scheduler.py:55-88`
|
||
**问题**：`_check_upstream` 在同一个 DB session 内顺序执行：
|
||
|
||
1. 写 snapshot → `db.commit()`
|
||
2. 调用 `webhook_service.send_rate_changed()` → 内部 `httpx.post` + `db.commit()`
|
||
3. 调用 `website_sync.sync_affected_bindings()` → 内部多次 `httpx.put` + 多次 `db.commit()`
|
||
|
||
如果步骤 2 或 3 失败，步骤 1 的 snapshot 已经提交，但 webhook 和网站同步丢失——数据处于"检测到了变化但没通知"的不一致状态。
|
||
|
||
**建议**：检测（只读 + snapshot 写入）与通知/同步（webhook + 网站写回）分离为两个独立事务。
|
||
|
||
### 9. 硬编码上游 IP 迁移逻辑
|
||
|
||
**位置**：`backend/app/database.py:58-65`
|
||
**问题**：`_migrate_custom_pages()` 包含针对特定 IP 的数据迁移：
|
||
|
||
```python
|
||
"host": "%://170.106.100.210",
|
||
```
|
||
|
||
这是特定部署的遗留数据修正，放在通用数据库初始化代码中。其他部署者每次启动都会跑这段无用代码。
|
||
|
||
**建议**：移出到独立 migration 脚本，或在检测到实际数据时按需执行。
|
||
|
||
### 10. Scheduler 关闭不等待进行中的任务
|
||
|
||
**位置**：`backend/app/services/scheduler.py:120`
|
||
**问题**：`_scheduler.shutdown(wait=False)` 立即返回，正在执行的 job 可能被中断（正在写 DB 或发 HTTP 请求）。
|
||
|
||
```python
|
||
def stop_scheduler() -> None:
|
||
if _scheduler.running:
|
||
_scheduler.shutdown(wait=False) # ← 不等待
|
||
```
|
||
|
||
**建议**：设 `wait=True` 或给一个合理的 grace period（如 30s）。
|
||
|
||
### 11. `consecutive_failures` 未在启用/编辑时重置
|
||
|
||
**位置**：`backend/app/services/scheduler.py:60-75`
|
||
**问题**：失败计数只在检测成功时归零。如果一个上游被禁用（`enabled=False`）、修改配置、再启用，旧的失败计数会延续——可能立即标记为 unhealthy。同样，手动 `check-now` 成功也没有重置计数器。
|
||
|
||
**建议**：编辑上游或重新启用时重置计数；`check-now` 成功路径也一并重置。
|
||
|
||
---
|
||
|
||
## 🟢 轻量 —— 体验 & 运维改进
|
||
|
||
### 12. 前端 localStorage key 易冲突
|
||
|
||
**位置**：`frontend/src/api/index.ts:14-15`
|
||
**问题**：硬编码 `'smartup_token'` / `'smartup_email'`，同域名下多个实例会互相覆盖。
|
||
|
||
**建议**：用环境变量或 `import.meta.env.VITE_APP_KEY` 做 key 前缀。
|
||
|
||
### 13. 健康检查开销大
|
||
|
||
**位置**：`docker-compose.yml:23-28`
|
||
**问题**：`CMD ["python", "-c", "import urllib.request; ..."]` 每 30 秒启动一次完整 Python 解释器，每次约 50-100ms 启动耗时。
|
||
|
||
**建议**：Dockerfile 安装 `curl`，改为 `curl -f http://localhost:8000/healthz`。
|
||
|
||
### 14. 缺少 `.dockerignore`
|
||
|
||
**位置**：项目根目录
|
||
**问题**：无 `.dockerignore`，`node_modules/`、`__pycache__`、`data/`、`.venv/` 等大规模目录都会被打包进入 Docker build context，拖慢构建。
|
||
|
||
**建议**：添加 `.dockerignore`，排除 `node_modules/`、`data/`、`*.pyc`、`.git/`、`.venv/` 等。
|
||
|
||
### 15. 前端请求无网络重试
|
||
|
||
**位置**：`frontend/src/api/index.ts:6-20`
|
||
**问题**：axios 拦截器遇到 401 直接跳登录页。网络瞬断、502/503 等临时故障无重试。
|
||
|
||
**建议**：引入 `axios-retry` 实现指数退避重试（对非 401/4xx 可重试）。
|
||
|
||
### 16. 快照分页 API 实现微妙
|
||
|
||
**位置**：`backend/app/routers/upstreams.py:256-294`
|
||
**问题**：`list_snapshots` 为了计算 diff 数量多取 1 行，与 `offset` + `limit` 配合时逻辑复杂：
|
||
|
||
```python
|
||
rows = (
|
||
db.query(...)
|
||
.offset(offset)
|
||
.limit(limit + 1) # 多取 1 行用于 diff
|
||
.all()
|
||
)
|
||
```
|
||
|
||
当 `offset > 0` 时，多取的 1 行是相对于 offset 的额外行，并不一定是前一页的最后一行——diff 结果可能不准。
|
||
|
||
**建议**：放弃"附带 diff 计数"设计，改为前端单独调 diff 接口，或直接在前次快照 id 上做 diff。
|
||
|
||
---
|
||
|
||
## 📋 优先级实施建议
|
||
|
||
| 优先级 | 编号 | 估算工时 | 影响面 |
|
||
|--------|------|---------|--------|
|
||
| P0 | 1, 2, 3, 4, 5 | 1-2 天 | DB 锁死 / 安全漏洞 / 性能衰减 |
|
||
| P1 | 6, 7, 8, 10, 11 | 0.5 天 | 数据不一致 / 维护负担 |
|
||
| P2 | 9, 12, 13, 14, 15, 16 | 0.5 天 | 部署体验 / 代码整洁 |
|
||
|
||
---
|
||
|
||
*生成日期：2026-06-13 · 审查范围：commit HEAD*
|