docs(25): 风控模块代码评审报告
评审方式：4 个并行评审（架构接入 / 数据层与数据库基线 / 业务逻辑正确性 / API 规范）， 关键结论由本人逐条核对代码或实测数据库复核。报告用三级标记区分可信度： ✅ 已亲自核对、🔁 两位评审独立发现同一问题、⚠️ 评审提出但未逐条复核。 总体结论：骨架合规（继承 BaseAgent、只实现 handle、工具统一走 call_tool、ORM 与数据库基线 逐列吻合且未改动任何已有表、全仓无字符串拼 SQL、权限默认拒绝、repository 严格只读）， 问题集中在三条断线和一批业务正确性缺陷。 P0：意图配置与工具白名单一条未发布（实测 DB 确认）。白名单失败关闭 ⇒ 风控当前跑不起来。 P1：模型调用绕过基座的 generate_with_model；risk chat 的 task_type 未注册 capability， 当前能用只因 deepseek-flash 恰好排在端点表第一行；邮件端点无权限校验（默认关闭， 但开启 SMTP 后即为未授权邮件发送器）；Agent 在 handle 里直写 ai_analysis 且读-改-写 无并发控制；时区口径不统一——定时规则把 UTC 0-6 点当"凌晨"，实际判的是北京时间 08:00-14:00，会持续误报。 P2：日报误报原因恒为"未填写"、研判漏阈值条件、通知失败被静默吞掉、定时扫描重启后不再 执行、一条脏数据中断整批等 10 项。 P3：分页元数据层级、游标未绑定用户与查询条件、无 limit 全量查询、索引失效、写操作无幂等、 错误码超表、SSE 未协商 Accept、接口未登记 docs/05 §19。 另记录一项需要业务方裁定的制度冲突：适当性指南第十二条矩阵允许 C1 购买 R2，第十四条却 要求投资者等级 >= 产品等级，两者对同一情形结论相反（已核对原文）。
This commit is contained in:
@@ -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:<intent>`。
|
||||
注意 `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 按需排期。
|
||||
Reference in New Issue
Block a user