Simon
246 lines
9.0 KiB
Markdown
246 lines
9.0 KiB
Markdown
# 评审报告: workbuddy P0 安全止血推送
|
||
|
||
**推送日期**: 2026-06-14
|
||
**评审日期**: 2026-06-14
|
||
**评审人**: Claude
|
||
**关联 commit**: `3735dc0` — feat(security): P0 安全止血 - WS token 改 header + 坐席本地密码
|
||
**任务**: #10 P0 安全止血
|
||
**workbuddy 自报**: 完成
|
||
**本地验证结果**: 🟡 **部分完成,5 项遗留**
|
||
|
||
---
|
||
|
||
## ⭐ 一句话结论
|
||
|
||
workbuddy 推了 5 文件 + 2 新文件 / +263 -24 行,**2/5 P0 任务真正修好,3 项有遗留**(其中 1 项服务端代码 + 1 项前端代码 + 1 项 nginx),**需 workbuddy 下一轮修完才能算 P0 闭环**。
|
||
|
||
---
|
||
|
||
## 📊 任务清单 vs 完成度
|
||
|
||
| P0 # | 任务 | workbuddy 改动 | 真实状态 |
|
||
|---|---|---|---|
|
||
| P0-#4 | WS token 不在 URL/日志 | ws.py header 优先 + ws.ts 加 Authorization header + 漏 nginx | 🟡 **半成品** |
|
||
| P0-#5 | 坐席登录加 password | model + schema + agents.py + alembic 008 + 漏 requirements.txt + 漏降级放行 | 🟡 **半成品** |
|
||
| P0-#1 | WECOM_SECRET 集中化 | docs/安全/secret-管理.md(规划) | 🟡 **只规划未实改** |
|
||
| P0-#2 | SSL 私钥在仓 | (无) | 🟢 **8-A 阶段已修** |
|
||
| P0-#3 | Mock login | (无) | 🟢 **之前已修** |
|
||
|
||
---
|
||
|
||
## ✅ 已正确完成
|
||
|
||
### P0-#4 (服务端): `backend/app/api/ws.py`
|
||
|
||
- 优先从 `Authorization: Bearer {token}` header 取 token
|
||
- 降级从 `?token=` query param 取(向后兼容)
|
||
- 同步 `websocket_endpoint`(坐席端)+ `h5_websocket_endpoint`(H5 员工端)
|
||
- **服务端验收通过** ✅
|
||
|
||
### P0-#5 (模型层): `backend/app/models/agent.py` 字段定义
|
||
|
||
- `password_hash: Mapped[str] = mapped_column(String(128), nullable=True, default=None)`
|
||
- 字段长度 / 注释 / nullable 合理
|
||
- alembic 008 迁移脚本正确,依赖 007_role_system 存在
|
||
|
||
### P0-#5 (Schema): `backend/app/schemas/agent.py`
|
||
|
||
- `AgentLogin` 加 `password: Optional[str]` 字段
|
||
- `AgentPasswordUpdate` 单独定义(旧密码 + 新密码,6-128 位)
|
||
|
||
### P0-#5 (改密端点): `backend/app/api/agents.py` `/agents/password`
|
||
|
||
- 走 `Depends(get_current_agent)` 鉴权 ✅
|
||
- 旧密码校验 + bcrypt 哈希 + 错误码 1011-1014 区分
|
||
- 结构 OK
|
||
|
||
### docs/安全/secret-管理.md (1.9 KB)
|
||
|
||
- WECOM_SECRET / WECOM_ENCODING_AES_KEY / DIFY_API_KEY / POSTGRES_PASSWORD / REDIS_PASSWORD 风险列表
|
||
- 4 种方案对比(NAS Vault / Server Keyring / Docker Secrets / HashiCorp Vault)
|
||
- 短期止血 + 长期迁移路径
|
||
|
||
---
|
||
|
||
## 🔴 遗留 5 项(严重度按序)
|
||
|
||
### 遗留 1: [P0-#4] ws.ts 浏览器 WebSocket API **不支持自定义 header** 🔴
|
||
|
||
**文件**: `frontend-agent/src/composables/useWebSocket.ts:106-110`
|
||
|
||
```ts
|
||
ws = new WebSocket(wsUrl, [], {
|
||
headers: {
|
||
Authorization: `Bearer ${agentStore.token}`,
|
||
},
|
||
})
|
||
```
|
||
|
||
**问题**: 浏览器原生 WebSocket 构造函数第 3 参数 options **没有 `headers` 字段**(只有 `protocols`)。**Chromium / Firefox / Safari 全部忽略 options.headers**,token 实际**未发送**。
|
||
|
||
**workbuddy 误用了 Node.js `ws` 库的 API**,浏览器侧完全无效。
|
||
|
||
**修复方向**(任选一种):
|
||
|
||
| 方案 | 服务端 | 前端 | 兼容性 |
|
||
|---|---|---|---|
|
||
| A. Sec-WebSocket-Protocol 携带 | 从 `request.headers['sec-websocket-protocol']` 取 | `new WebSocket(url, [\`bearer.${token}\`])` | 🟢 标准,全浏览器 |
|
||
| B. httpOnly cookie 携带 | 登录时 set-cookie,WS 握手带 cookie | 不变(浏览器自动带) | 🟢 需 HTTPS |
|
||
| C. 短 ticket 换 token(URL) | 服务端 token 换 ticket(短 TTL),WS 用 ticket | 先 POST /ws-ticket 拿 ticket | 🟢 实用,URL 带 ticket 非 token |
|
||
|
||
**推荐方案 A**(标准,无 cookie 复杂度,前端改动最小)。
|
||
|
||
### 遗留 2: [P0-#4] nginx access_log **没关闭** 🔴
|
||
|
||
**应改文件**:
|
||
- `nginx.conf`(根目录)
|
||
- `deploy-server/nginx.conf`
|
||
|
||
**计划文件阶段 10.1.1 明说要加**:
|
||
```nginx
|
||
location /ws/ {
|
||
access_log off;
|
||
}
|
||
```
|
||
|
||
**workbuddy 漏了**。**即便前端改造好,token 经过 nginx 时仍会写 access_log**(默认 `/var/log/nginx/access.log`),任何人能 tail 这个文件拿到历史 token。
|
||
|
||
### 遗留 3: [P0-#5] model `Mapped[str]` 类型 bug 🟡
|
||
|
||
**文件**: `backend/app/models/agent.py:142-148`
|
||
|
||
```python
|
||
password_hash: Mapped[str] = mapped_column(
|
||
String(128),
|
||
nullable=True,
|
||
default=None, # ← None 实际不能赋值给 str
|
||
comment="本地密码哈希（bcrypt）",
|
||
)
|
||
```
|
||
|
||
**问题**: SQLAlchemy 2.0 strict 模式下 `Mapped[str]` + `nullable=True` + `default=None` 会**发出警告甚至报错**(`InvalidRequestError: Class does not support None`)。**实际跑起来可能挂**(取决于 strict 配置)。
|
||
|
||
**修复**: `Mapped[Optional[str]]` + 引用 `from typing import Optional`。
|
||
|
||
### 遗留 4: [P0-#5] 企微降级放行不强制 password 验证 🟡
|
||
|
||
**文件**: `backend/app/api/agents.py:236-243`
|
||
|
||
```python
|
||
local_password_verified = False
|
||
if body.password and agent and agent.password_hash:
|
||
if bcrypt.verify(body.password, agent.password_hash):
|
||
local_password_verified = True
|
||
else:
|
||
raise AppException(1011, "本地密码错误")
|
||
```
|
||
|
||
**问题**: 走 `local_password_verified` 后,**没有阻断企微 API 失败时的"降级放行"路径**(`agent_login` 之前在 `企微API不可达` 时会"已注册坐席降级放行",**不验 password**)。
|
||
|
||
**结果**: P0-#5 加了 password 字段,但**降级放行逻辑仍能绕过 password 验证** → **P0-#5 被反削弱**。
|
||
|
||
**修复**: 降级放行路径需检测 `agent.password_hash` 是否存在 → 存在则强制走 password 验证。
|
||
|
||
### 遗留 5: [P0-#5] requirements.txt 缺 passlib 依赖 🟡
|
||
|
||
**文件**: `backend/requirements.txt`
|
||
|
||
**问题**: workbuddy 改的 `agents.py` 用 `from passlib.hash import bcrypt`,但 `requirements.txt` **没加 passlib**。**生产部署会 ImportError**。
|
||
|
||
**修复**: 加 `passlib[bcrypt]==1.7.4`(或直接 `bcrypt==4.1.2` 不用 passlib,因 passlib 2024 已停维护)。
|
||
|
||
**建议**: 改用原生 `bcrypt` 库更稳:
|
||
```python
|
||
import bcrypt
|
||
bcrypt.hashpw(password.encode('utf-8'), bcrypt.gensalt())
|
||
bcrypt.checkpw(password.encode('utf-8'), agent.password_hash.encode('utf-8'))
|
||
```
|
||
|
||
---
|
||
|
||
## 📁 变更清单(commit 3735dc0)
|
||
|
||
```
|
||
backend/app/api/agents.py | +67 -0
|
||
backend/app/api/ws.py | +30 -0
|
||
backend/app/models/agent.py | +10 -0
|
||
backend/app/schemas/agent.py | +7 -0
|
||
frontend-agent/src/composables/useWebSocket.ts | +5 -0
|
||
backend/alembic/versions/008_add_agent_password.py | +37(新)
|
||
docs/安全/secret-管理.md | +67(新)
|
||
7 files changed, 263 insertions(+), 24 deletions(-)
|
||
```
|
||
|
||
---
|
||
|
||
## 🎯 workbuddy 下一轮任务清单(高优先级)
|
||
|
||
按 5 项遗留严重度排:
|
||
|
||
### ▶▶▶ workbuddy 下一轮任务清单起
|
||
|
||
#### [P0] 1. 修 ws.ts:用 Sec-WebSocket-Protocol 方案
|
||
|
||
- **文件**: `frontend-agent/src/composables/useWebSocket.ts:103-112`
|
||
- **改**: `ws = new WebSocket(wsUrl, [\`bearer.${agentStore.token}\`])`
|
||
- **配套**: ws.py 服务端从 `request.headers.get('sec-websocket-protocol', '')` 取(取 `bearer.xxxx` 部分)
|
||
|
||
#### [P0] 2. 加 nginx access_log 关闭
|
||
|
||
- **文件**:
|
||
- `nginx.conf` (根)
|
||
- `deploy-server/nginx.conf`
|
||
- **改**: 找到 `location /api/`,加 `location /ws/ { access_log off; }` 在其前/后
|
||
|
||
#### [P1] 3. 修 model 类型注解
|
||
|
||
- **文件**: `backend/app/models/agent.py:142-148`
|
||
- **改**: `Mapped[str]` → `Mapped[Optional[str]]` + `from typing import Optional`
|
||
|
||
#### [P1] 4. 修降级放行必须 password 验证
|
||
|
||
- **文件**: `backend/app/api/agents.py` agent_login 流程
|
||
- **改**: 企微 API 不可达分支检测 `agent.password_hash` 存在 → 强制走 password 验证
|
||
|
||
#### [P1] 5. 加 passlib 依赖到 requirements.txt
|
||
|
||
- **文件**: `backend/requirements.txt`
|
||
- **改**: 加 `passlib[bcrypt]==1.7.4` 或 `bcrypt==4.1.2`
|
||
- **配套(可选)**: 改 `agents.py` 用原生 `bcrypt` 库
|
||
|
||
### ▼▼▼ workbuddy 下一轮任务清单止
|
||
|
||
---
|
||
|
||
## ⚠️ 评审流程教训
|
||
|
||
1. **WebSocket API 边界知识**: 浏览器侧 vs Node.js 侧 ws 库 API 差异,workbuddy 误用
|
||
2. **依赖检查漏**: 改代码必须同时改 requirements.txt(防止 ImportError)
|
||
3. **配置改动漏**: nginx/conf 改动 plan 写了但 workbuddy 没做(规划 vs 实施脱节)
|
||
4. **类型注解一致性**: Mapped[T] + nullable=True 必须用 Optional
|
||
5. **逻辑回归**: 加新鉴权时必须 review"已有降级路径是否被绕过"
|
||
|
||
---
|
||
|
||
## 📊 风险跟踪表更新建议
|
||
|
||
| 项 | 旧状态 | 新状态 |
|
||
|---|---|---|
|
||
| P0-#4 WS token URL 泄露 | 待修 | 🟡 半成品,前端 ws.ts 改造 + nginx access_log 待关 |
|
||
| P0-#5 坐席本地密码 | 待修 | 🟡 半成品,类型 bug + 降级放行 + 缺依赖 |
|
||
| P0-#1 WECOM_SECRET 集中化 | 待修 | 🟡 仅规划,无代码改动 |
|
||
| P0-#2 SSL 私钥 | 待修 | 🟢 已完成(8-A) |
|
||
| P0-#3 Mock login | 待修 | 🟢 已完成(之前) |
|
||
|
||
---
|
||
|
||
## 🔗 推 Gitea 状态
|
||
|
||
- **本地 commit**: 3735dc0 已存 ✅
|
||
- **推 Gitea**: 🔴 卡 #8 (MariaDB 套件未装)
|
||
- **下次**: Gitea 起来后 `git push -u origin main` 一次推送,workbuddy 拿 Gitea URL 二次评审
|
||
|
||
---
|
||
|
||
**下次评审窗口**: 等 workbuddy 修完 5 项遗留后,触发新一轮评审(本任务 #18)。
|