diff --git a/2026-05-16-ephron-security-fix-closure-prd.md b/2026-05-16-ephron-security-fix-closure-prd.md new file mode 100644 index 0000000..46a54cc --- /dev/null +++ b/2026-05-16-ephron-security-fix-closure-prd.md @@ -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(立即收口项)** + +原因: +- 业务漏洞本身已大体修复 +- 剩下的是验收链最后一个阻塞点 +- 不收掉这个失败测试,就不能严谨地宣布“全部完成” + +--- + +## 最终目标 + +该问题修完后,整个安全修复工作才能从: + +> “代码已改,但验证未完全通过” + +升级为: + +> “代码已改,测试通过,安全修复完成闭环。”