add security fix closure prd for remaining test failure
This commit is contained in:
226
2026-05-16-ephron-security-fix-closure-prd.md
Normal file
226
2026-05-16-ephron-security-fix-closure-prd.md
Normal file
@@ -0,0 +1,226 @@
|
|||||||
|
# ephron.ren 安全修复收尾 PRD:回归测试失败问题修复
|
||||||
|
|
||||||
|
## 背景
|
||||||
|
|
||||||
|
在对 `ephron.ren` 安全问题的第三轮复核中,已确认前述 6 项修复大多已经合入仓库最新代码(`origin/main`)。但在执行回归验证时,仍存在一个未完成项:
|
||||||
|
|
||||||
|
- `tests/test_security_hardening.py` 运行结果:`9 passed, 1 failed`
|
||||||
|
- 失败用例:`test_ssrf_url_validation_blocks_private_and_localhost`
|
||||||
|
|
||||||
|
因此当前状态不能定义为“全部修复并完成验收”,而应定义为:
|
||||||
|
|
||||||
|
> **安全修复代码已基本落地,但回归测试链尚未完全打通。**
|
||||||
|
|
||||||
|
本 PRD 仅聚焦这个**剩余问题**的修复与验收收口。
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 问题定义
|
||||||
|
|
||||||
|
## 现象
|
||||||
|
运行:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
python3 -m pytest /home/ubuntu/ephron.ren/tests/test_security_hardening.py -q
|
||||||
|
```
|
||||||
|
|
||||||
|
结果:
|
||||||
|
|
||||||
|
- 9 个测试通过
|
||||||
|
- 1 个测试失败
|
||||||
|
|
||||||
|
失败用例:
|
||||||
|
- `test_ssrf_url_validation_blocks_private_and_localhost`
|
||||||
|
|
||||||
|
报错核心:
|
||||||
|
|
||||||
|
```python
|
||||||
|
ModuleNotFoundError: No module named 'itsdangerous'
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 根因分析
|
||||||
|
|
||||||
|
该失败不是 SSRF 逻辑本身有误,而是**测试导入方式导致依赖链过重**。
|
||||||
|
|
||||||
|
当前测试做法:
|
||||||
|
- 通过导入 `from src.routes.api import _is_safe_url`
|
||||||
|
- 间接触发:
|
||||||
|
- `prompt/src/routes/__init__.py`
|
||||||
|
- `prompt/src/routes/pages.py`
|
||||||
|
- `prompt/src/services/__init__.py`
|
||||||
|
- `prompt/src/services/auth.py`
|
||||||
|
- 最终进入 `itsdangerous` 依赖
|
||||||
|
|
||||||
|
因此问题本质是:
|
||||||
|
|
||||||
|
### 根因
|
||||||
|
`_is_safe_url` 的测试目标只是一个**纯函数**,但测试却走了整个 Prompt 服务的路由/服务导入链,导致:
|
||||||
|
- 单元测试不再是纯单元测试
|
||||||
|
- 测试对环境依赖过重
|
||||||
|
- 运行环境中如果未安装完整 Prompt 依赖,就会失败
|
||||||
|
|
||||||
|
这属于:
|
||||||
|
|
||||||
|
> **测试隔离性不足 / 单元测试边界设计不合理**
|
||||||
|
|
||||||
|
而不是 SSRF 修复逻辑本身失效。
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 修复目标
|
||||||
|
|
||||||
|
需要把这个剩余问题修到以下状态:
|
||||||
|
|
||||||
|
1. `test_ssrf_url_validation_blocks_private_and_localhost` 可稳定运行
|
||||||
|
2. 测试不依赖 Prompt 服务完整导入链
|
||||||
|
3. 测试不依赖 `itsdangerous` 等无关运行时组件
|
||||||
|
4. `tests/test_security_hardening.py` 全绿
|
||||||
|
5. 最终可把安全修复状态更新为“已修复并通过验证”
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 推荐修复方案
|
||||||
|
|
||||||
|
## 方案 A(推荐):把 `_is_safe_url` 下沉到更轻的共享模块
|
||||||
|
|
||||||
|
### 做法
|
||||||
|
将 `_is_safe_url` 从:
|
||||||
|
- `prompt/src/routes/api.py`
|
||||||
|
|
||||||
|
迁移到更轻量、纯函数化的模块,例如:
|
||||||
|
- `shared/url_safety.py`
|
||||||
|
- 或 `shared/network_safety.py`
|
||||||
|
|
||||||
|
然后:
|
||||||
|
- `prompt/src/routes/api.py` 改为引用共享函数
|
||||||
|
- `tests/test_security_hardening.py` 直接测试共享函数
|
||||||
|
|
||||||
|
### 优点
|
||||||
|
- 测试边界最清晰
|
||||||
|
- 不再耦合 Prompt 路由层
|
||||||
|
- 后续其他服务若也需要 SSRF URL 校验,可复用
|
||||||
|
- 更符合“安全校验逻辑应集中在 shared 层”的架构
|
||||||
|
|
||||||
|
### 风险
|
||||||
|
- 需要小幅改动导入路径
|
||||||
|
- 需要确认不会引入循环依赖
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 方案 B(次优):保留函数位置,但避免整包导入
|
||||||
|
|
||||||
|
### 做法
|
||||||
|
不改 `_is_safe_url` 的实际定义位置,但修改测试方式,例如:
|
||||||
|
- 用更轻量的 import 方式只加载目标文件
|
||||||
|
- 或抽出测试辅助加载器,避免触发 `src.routes.__init__`
|
||||||
|
|
||||||
|
### 优点
|
||||||
|
- 改动面更小
|
||||||
|
|
||||||
|
### 缺点
|
||||||
|
- 测试技巧性强,可维护性差
|
||||||
|
- 仍然保留“安全函数放在路由层”的结构问题
|
||||||
|
- 后续别人很容易再次踩坑
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 最终建议
|
||||||
|
|
||||||
|
优先采用:
|
||||||
|
|
||||||
|
> **方案 A:把 `_is_safe_url` 抽到 shared 轻模块,再让测试直接测 shared 函数。**
|
||||||
|
|
||||||
|
这是更干净、可维护、可复用的长期解法。
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 具体改动建议
|
||||||
|
|
||||||
|
## 1. 新建共享模块
|
||||||
|
例如:
|
||||||
|
- `shared/network_safety.py`
|
||||||
|
|
||||||
|
包含:
|
||||||
|
- `is_safe_external_url(url: str) -> bool`
|
||||||
|
|
||||||
|
逻辑保留当前 SSRF 防护规则:
|
||||||
|
- 仅允许 `http/https`
|
||||||
|
- 禁止 `localhost`
|
||||||
|
- 禁止 `.local`
|
||||||
|
- 禁止 loopback
|
||||||
|
- 禁止 private IP
|
||||||
|
- 禁止 link-local
|
||||||
|
- 禁止 reserved
|
||||||
|
- 禁止 multicast
|
||||||
|
- 处理 IPv6 / IPv4-mapped
|
||||||
|
|
||||||
|
## 2. 修改 Prompt 路由引用
|
||||||
|
`prompt/src/routes/api.py`
|
||||||
|
- 删除本地 `_is_safe_url`
|
||||||
|
- 改用 shared 中的 `is_safe_external_url`
|
||||||
|
|
||||||
|
## 3. 修改测试
|
||||||
|
`tests/test_security_hardening.py`
|
||||||
|
- 不再 import `src.routes.api`
|
||||||
|
- 改为直接 import shared 函数,例如:
|
||||||
|
- `from shared.network_safety import is_safe_external_url`
|
||||||
|
|
||||||
|
## 4. 重新运行测试
|
||||||
|
至少执行:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
python3 -m pytest /home/ubuntu/ephron.ren/tests/test_security_hardening.py -q
|
||||||
|
```
|
||||||
|
|
||||||
|
目标:
|
||||||
|
- 全部通过
|
||||||
|
|
||||||
|
如有条件,再补一轮相关定向测试:
|
||||||
|
- Prompt API 相关测试
|
||||||
|
- 安全回归相关测试
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 验收标准
|
||||||
|
|
||||||
|
修复完成的标准:
|
||||||
|
|
||||||
|
### 必须满足
|
||||||
|
1. `tests/test_security_hardening.py` 全绿
|
||||||
|
2. SSRF URL 校验规则保持不变或更严格
|
||||||
|
3. `prompt /api/test-connection` 仍保留:
|
||||||
|
- 管理员权限校验
|
||||||
|
- CSRF 校验
|
||||||
|
- URL 安全校验
|
||||||
|
- 泛化错误回显
|
||||||
|
|
||||||
|
### 最好满足
|
||||||
|
4. 安全 URL 校验逻辑已沉淀到 shared 层
|
||||||
|
5. 测试无需依赖 Prompt 服务完整运行时依赖
|
||||||
|
6. PRD 主文档可更新为“修复完成并通过回归验证”
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 优先级
|
||||||
|
|
||||||
|
### 当前优先级
|
||||||
|
**P1(立即收口项)**
|
||||||
|
|
||||||
|
原因:
|
||||||
|
- 业务漏洞本身已大体修复
|
||||||
|
- 剩下的是验收链最后一个阻塞点
|
||||||
|
- 不收掉这个失败测试,就不能严谨地宣布“全部完成”
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 最终目标
|
||||||
|
|
||||||
|
该问题修完后,整个安全修复工作才能从:
|
||||||
|
|
||||||
|
> “代码已改,但验证未完全通过”
|
||||||
|
|
||||||
|
升级为:
|
||||||
|
|
||||||
|
> “代码已改,测试通过,安全修复完成闭环。”
|
||||||
Reference in New Issue
Block a user