diff --git a/docs/25-风控模块代码评审报告.md b/docs/25-风控模块代码评审报告.md new file mode 100644 index 0000000..3f2f358 --- /dev/null +++ b/docs/25-风控模块代码评审报告.md @@ -0,0 +1,210 @@ +# 25 · 风控模块代码评审报告 + +> **评审对象**:合并 `origin/qyqy_develop`(`870fd0d`)带入的风控模块(约 5000 行)。 +> +> **评审方式**:4 个并行评审(架构接入 / 数据层与数据库基线 / 业务逻辑正确性 / API 规范), +> 关键结论由本人逐条核对代码或实测数据库复核。 +> +> **验证标记**: +> - ✅ **已验证**:本人核对过代码原文或实测过数据库,证据在文中; +> - 🔁 **交叉印证**:两位评审独立发现同一问题(可信度更高); +> - ⚠️ **待复核**:评审提出、本人未逐条核对。 +> +> **本报告只陈述问题与依据,不含任何修改。** 修复请另行安排。 + +--- + +## 一、总体结论 + +**骨架是合规的**,这一点要先说清楚:风控正确继承了 `BaseAgent`、只实现 `handle()`、工具调用统一走 `self.call_tool`、ORM 与数据库基线**逐列吻合且没有改动任何已有表**、全仓无字符串拼 SQL、权限 scope 为空时默认拒绝、repository 严格只读。 + +问题集中在**三条断线**和**一批业务正确性缺陷**: + +1. **运行期配置没发布** → 风控在当前环境**跑不起来**; +2. **模型调用绕过基座的模型服务** → 治理链路断了一环; +3. **时区口径不统一** → 定时规则判错时段、日报日界错位(两位评审独立发现)。 + +--- + +## 二、P0 · 阻断级 + +### 1. 意图配置与工具白名单一条都没发布 ✅ + +实测数据库: + +``` +platform_config_item(当前生效版本 5 条):customer_service:faq / policy_explain / + product_inquiry / suitability_check / fund_query_demo:fund_quote ← 无任何 risk:* +agent_intent_config 中 agent_type='risk':0 条 +``` + +风控声明了 4 个意图(`risk_agent.py:19-22`:`risk_overview` / `risk_search` / `risk_evidence` / `general`)。 +底座的白名单是**失败关闭**的:`ToolExecutor` 取「发布配置的 `allowed_tools`」与「代码声明的 +`allowed_tools`」**交集**,缺配置时交集为空 ⇒ **任何 `call_tool` 都被拒绝**。 + +**表现**:风控只能走 `handle` 里不依赖工具的 fallback 分支,等于不可用。 + +**修复**:补 `agent_intent_config` 与发布版 `agent_tools` 的 `risk:`。 +注意 `config_release` 是**整版本替换**语义,必须继承现有 5 条配置项,否则会把客服的白名单清空。 + +--- + +## 三、P1 · 安全与治理 + +### 2. 模型调用绕过基座的模型服务 ✅ + +- 底座**有**公共入口:`base.py:79-84` 的 `generate_with_model()`,且 `base.py:86-95` 把它列入 + **禁止子类覆写**的集合 —— 底座明确视其为治理方法。 +- 风控却自己构造客户端:`risk_agent.py:133` `self._chat_model_client = model_client or RiskAgentModelClient()`, + `:225` 用它调模型;`risk_agent_model_client.py:88-96` 自己发 HTTP。 +- `bootstrap.py:262-265` 的 `lambda _context: RiskAgent(RiskAgent.definition)` 让 `model_client` + **永远是默认值**,连注入替换都做不到。 + +它**复用了 gateway 的配置解析**(`DatabaseModelEndpointResolver` + `EnvironmentSecretResolver`), +但**没走 gateway 的调用层** —— 降级、端点排序、统一错误映射,以及 gateway 将来的任何改进 +(限流、成本统计)它都享受不到。 + +**修复**:改走 `ModelGenerationService`,或删掉该文件直接用 `self.generate_with_model`。 + +### 3. 能力过滤失效,当前能工作只是巧合 🔁 ✅ + +`risk_agent_model_client.py:51-54` 传 `task_type="risk_agent_chat"`,不在 `TASK_CAPABILITY` 里 → +`model_gateway.py:196-198` **返回全部 active 端点** → 客户端只取前 `max_attempts`(默认 2)个。 + +**实测端点表**:`id=3 deepseek-flash`(caps 含 `text_generation`)恰好排在 +`id=5 qwen-embedding` 前面,所以现在能拿到文本端点。 + +**但这是巧合**:`model_gateway.py:181-184` 的注释自己警告过"取决于端点在表里的顺序"是缺陷。 + +**修复**:`TASK_CAPABILITY` 补 `"risk_agent_chat": "text_generation"`(及相关 task_type)。 + +### 4. `POST /daily-report/mail` 无权限校验 ✅ + +- `api/controllers/risk.py:207-217`:只有 `build_request_context`(管 401),**无任何权限校验**。 +- `risk_daily_report_mail_service.py:16-42`:无 `AuthorizationService`、拿不到 context, + **收件人 / 主题 / 正文全部由客户端决定**。 + +**当前不可被利用**:`:17-20` 有两层默认关闭开关 —— +`RISK_DAILY_REPORT_MAIL_ENABLED` 默认 `False`(返回 `disabled`)、`DRY_RUN` 默认 `True`。 +**但一旦运维开启 SMTP,它就是一个未授权的邮件发送器。** + +**修复**:在开启 SMTP 之前必须先加权限校验与收件人白名单。 + +### 5. Agent 在 `handle` 里直接写库 ✅ + +`risk_agent.py:141-146` 直接调 `RiskAnalysisService.generate_for_context(...)`; +`risk_analysis_service.py:145-156` 写 `alert.ai_analysis` 并 `commit()`。 + +**它不是"无审计的黑箱"** —— `:63` 有 `AuthorizationService.require`、`:147-155` 手写了 `InteractionAudit`。 +真正的问题是两条: + +1. 审计**不经基座统一链路**(只有走 `call_tool` 才有 `_tool_records` 与统一审计); +2. `ai_analysis` 是**读-改-写且无并发控制**(`dict(alert.ai_analysis or {})` → 赋值 → commit), + 并发请求会互相覆盖。 + +### 6. 时区口径不统一 🔁 ✅(最严重的业务缺陷) + +**定时规则判错时段**:`risk_scan_service.py:248` `0 <= confirmed_at.hour < 6`, +而 `:258` 生成的摘要写的是「**凌晨时段**发生 X 元交易」。库内是 UTC ⇒ +`[0,6)` UTC = **北京时间 08:00–14:00**。**规则本意是"凌晨",实际判的是整个上午**,会持续误报。 +同源代码见 `risk_judgement_service.py:238`,`:243` 还把 UTC 小时当北京时间展示。 + +**日报日界错位**:`risk_daily_report_service.py:89` 用 `generated_at.date()` 取日界, +而 `:61` 传入的是 `_utc_naive(now)`;`:119` 的 `report_date` 用 UTC 日期,`:415` 却按 +Asia/Shanghai 展示。北京 08:00 前生成时,统计窗口是"前一日 08:00 – 当日 08:00",跨了零点; +`:241` 的"当日新增"又与展示时间矛盾。 + +**REST 与 Agent 两条路径口径不一致**:`api/schemas/risk.py:32-33` 的时间参数是**裸 `datetime`**, +`risk_query_service.py:77-78` 原样透传去比 UTC 列;而 Agent 路径做了时区转换 +(`risk_natural_language.py:117` `tzinfo=SHANGHAI`)。 + +**修复**:统一约定"库里 UTC、边界处按北京时间换算",并在 REST 入口做同样的规范化。 + +--- + +## 四、P2 · 业务正确性(会导致误报 / 漏报)⚠️ + +| # | 问题 | 位置 | 后果 | +|---|---|---|---| +| 7 | 日报"误报原因"恒为"未填写" | `risk_repository.py:944-967` 的 `_alert_row` 不含 `close_reason`,`risk_daily_report_service.py:137` 读 `item.get("close_reason") or "未填写"` | 日报字段永远为空,失去分析价值 | +| 8 | 研判漏了阈值条件 | `risk_judgement_service.py:189` 只用 age/amount 判 RW-012;扫描侧 `risk_scan_service.py:209` 还有 `amount < Decimal(average) * 3` | 未达 3 倍均值也判"证据支持风险" ⇒ **误报** | +| 9 | 合并告警破坏证据结构 | `risk_scan_service.py:368` 重建 snapshot 只留 `product_id` + `merged_alerts`;`risk_judgement_service.py:103` 的 `snapshot.get("ratio")` 恒为 None | RW-003 研判降级 | +| 10 | 列表级无条件放行 | `risk_judgement_service.py:43-50`:`_assess_list_rule` 收到 `item` **却完全没用**,对 RW-018 硬编码返回「已解除」+ 硬编码理由 | 列表显示"已解除"、不会触发人工核查(详情级 `:259` 另有校验,点进去是对的) | +| 11 | 通知失败被吞 | `risk_scan_service.py:449` `except Exception: … return 0`,预警照常 commit,`:418` 只回 `notification_count=0` | **无法区分"无需通知"和"高风险通知创建失败"** —— 风控里这很危险 | +| 12 | 定时扫描重启后不再执行 | `risk_scan_scheduler.py:102` `last_run_at` 仅存内存;`:123` `if self.last_run_at is None: return config.run_immediately`(默认 False,`config.py:65`) | **重启后永不 due**;`:136` 失败后每 30s 无退避重试 | +| 13 | 一条脏数据中断整批 | `risk_scan_service.py:468` `int(level.replace(prefix, ""))` 遇非 C1-C5 抛 `ValueError`,`refresh_alerts` 无逐条隔离,`:411` 整批 rollback | 一条脏数据 ⇒ 整批扫描失败 | +| 14 | 年龄口径不一 | `risk_scan_service.py:462` 用 UTC 日期,`risk_judgement_service.py:381` 用服务器 `date.today()` | 生日边界差 1 天,65 岁阈值可能翻面 | +| 15 | 幂等仅在应用层 | `risk_scan_service.py:332` `_exists` + JSON 列无唯一索引;`:37` 的 asyncio.Lock 仅进程内 | 多 Web worker 并发存在重复告警窗口 | +| 16 | 状态跳变与静默截断 | `risk_action_service.py:65-66` 允许"待处理+已确认"直接关误报、跳过调查;`:103` `min(20, score_before)` 静默把 >20 压到 20 | 处置流程可被绕过;评分被静默改写 | + +--- + +## 五、P3 · 接口规范与性能 ⚠️ + +| # | 问题 | 位置 | +|---|---|---| +| 17 | 列表分页元数据放错层级:`next_cursor`/`has_more` 塞进 `data`,而 `docs/05` §3.3 要求放 `meta` 且明令"不得增加其他顶层字段"(`_envelope` 只输出 `trace_id`,已 ✅ 核对 `api/controllers/risk.py:220-224`) | `risk_query_service.py:158-167`,影响 5 个列表端点 | +| 18 | 游标只存 offset、**未绑定用户与查询条件**(`docs/05` §16.1 要求绑定),且 offset 无上界 | `risk_cursor.py:15` | +| 19 | offset 游标 + 可变排序键 ⇒ 并发下必然跳行或重复(排序首列是 `case(alert_level==HIGH…)`) | `risk_repository.py:155-166` | +| 20 | 无 `.limit()` 的全量查询:三处 select 无 limit(登录记录连时间窗都没有);`load()` 取全部未闭环告警(注释自认"不应用分页") | `risk_repository.py:251-274`、`306-316` | +| 21 | 索引失效:`trigger_rule_codes.contains([...])` → `JSON_CONTAINS`,基线 `fin_risk_alert` 无该列索引;多处 `like(f"%{keyword}%")` 全表扫 | `risk_repository.py:724`、`390`、`705-713` | +| 22 | 写操作无幂等:6 个 POST 端点无 `Idempotency-Key`(`docs/05` §5.1/§6.2) | `api/controllers/risk.py:60、89、100、111、167、207` | +| 23 | 错误码超表:把 `AGENT_INPUT_INVALID`(表定 422)配了 **413**,而 413 不在 `docs/05` §3.5 状态码表内 | `risk_evidence_archive_service.py:51-54` | +| 24 | SSE 未校验 `Accept`(`SseNotAcceptableError`/406 已定义但未被使用) | `api/controllers/risk.py:182-204` | +| 25 | 接口未登记 `docs/05`:§19 目录里一条风控接口都没有,而 §20 明确要求"新增接口必须同步更新 §19";且 §12 约定的前缀是 `/risk-scans/**`、`/risk-alerts/**`,实现是 `/api/v1/risk` | `docs/05-接口文档.md` §12/§19/§20 | + +--- + +## 六、做得好、建议保持 ✅ + +- **ORM 与数据库基线逐列吻合,没有改动任何已有表** —— 你们那条"不可变基线"的红线守住了; + `alembic/env.py` 的 `target_metadata = None` 也保证了 ORM 不会反向改表。 +- **全仓无字符串拼 SQL**,纯 SQLAlchemy 表达式;`f"%{keyword}%"` 只是绑定参数的值。 +- **默认拒绝**:权限 scope 为 None/denied 时返回 `false()` 而不是放行。 +- **repository 严格只读**:模块内无 `add/update/delete/commit`。 +- **三个工具 `read_only` 默认 True**,没碰"Agent 公共工具仅允许只读"这条红线。 +- **输出防护**:Agent 侧拦截越权处置话术与协议标记、内部 ID 脱敏、体积限制; + API 侧不透出审计信息、预警详情刻意规避内部主键。 +- **上传有大小与类型双重校验**:10MB 上限、魔数/zip 结构/UTF-8 校验、路径限定在项目内、防覆盖。 +- **状态机守卫扎实**:`with_for_update`、防重复确认/升级、已关闭不可更新。 +- **扫描单事务**:异常 rollback 后 raise,不是静默成功。 +- **双层并发保护**:asyncio.Lock + MySQL `GET_LOCK`。 +- **schema 严格**:请求模型普遍 `extra="forbid", frozen=True`,长度与范围校验齐全。 + +--- + +## 七、需要业务方裁定的问题 + +### 1. 政策文档自相矛盾 ✅(本人已核对原文) + +- `knowledge/policy/个人投资者适当性管理指南.md:306` 第十二条匹配矩阵:**C1 可购买 R2**; +- 同文件 `:330` 第十四条:"**正向匹配**:投资者风险等级必须**大于或等于**产品风险等级,方可购买"。 + +C1(1) 与 R2(2) 相比 `1 < 2`:**按矩阵可以买,按第十四条不能买**。 +风控扫描按后者实现(`risk_scan_service.py:169` `gap > 0 and missing_trace`)。 + +**这不是代码问题,是制度文本冲突**,需要业务方定一条为准。客服侧的适当性裁决走的是 +`check_suitability`(按档案等级与匹配规则),两边口径也需要对齐。 + +### 2. 第十五条豁免规则未落地 ⚠️ + +C3→R4(单只 ≤ 总资产 20%)、C4→R5(≤10%)的**持仓占比校验完全没有实现**; +`risk_judgement_service.py:157` 只要留痕齐全就判"疑似误报"。是暂不实现还是漏了,需确认。 + +### 3. `docs/24` 需要同步更新 + +`docs/24-客服Agent阶段性总结与下阶段计划.md` 里的「风控 Agent:需要先有规则引擎,尚未启动」 +与本报告结论已不符;"当前状态"表的数字也已被本次合并刷新。 + +--- + +## 八、建议的修复顺序 + +1. **发配置**(P0 #1)—— 这是风控能不能跑的前提;务必继承现有 5 条配置项。 +2. **时区统一**(P1 #6)—— 修完误报会明显下降,这也是"北京时间"要求的落地。 +3. **模型走 gateway + 补 capability**(P1 #2 #3)—— 消除对端点表行顺序的隐式依赖。 +4. **邮件端点加权限**(P1 #4)—— 必须在开启 SMTP 之前。 +5. **通知失败不再静默**(P2 #11)、**扫描调度持久化**(P2 #12)—— 这两条直接关系到 + "风控会不会悄悄不工作",属于风控系统的基本可信度。 +6. **政策口径裁定**(七 #1)—— 需要业务方拍板,然后代码与知识库一起对齐。 +7. 其余 P2/P3 按需排期。