Files
nofx/.claude/commands/20-code-review.md
2025-11-07 22:12:53 -05:00

330 lines
16 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

---
name: code-review
description: 通用代码审查命令,基于业务需求进行白盒逻辑正确性审查和技术架构评估
---
# 代码审查命令
## 审查任务
请对当前工作区的代码修改进行全面审查,重点关注业务逻辑的正确性和技术架构的合理性。
## 审查维度
### 1. 业务层面审查BLOCKING级别
- **需求来源验证**:寻找并验证业务需求来源(不能基于代码反推需求)
- **需求实现完整性**验证代码是否100%满足业务需求
- **业务流程逻辑正确性**:检查状态转换、数据流、条件判断的逻辑
- **数据结构正确性**:验证模型定义、字段约束、业务规则映射
- **Edge Case处理**:评估边界条件和异常场景的处理合理性
### 2. 技术层面审查
- **架构合理性**:模块化、职责分离、依赖关系
- **KISS原则遵循**:避免过度工程化
- **扩展性评估**:未来功能添加的容易程度
- **非Adhoc修改验证**:是否遵循现有代码模式
- **性能问题检测**查找明显的性能问题如N+1查询等
- **单元测试完备性**核心逻辑90%覆盖率要求
### 3. 契约与连通性专项检查BLOCKING
- 端点一致性:前端端点集中配置;路径/动态段/大小写/末尾分隔符与后端路由完全一致HTTP 方法语义匹配(幂等/副作用)。
- 认证与跨域:统一的认证机制(会话/令牌前端传递方式与后端期望一致Cookie/Authorization 等);跨域与 CSRF 策略匹配。
- 请求/响应 Schema字段名、类型、必选/可选一致;时间/数值/布尔的编码一致;分页/排序参数与响应元信息对齐。
- 错误与状态码4xx/5xx 使用合理;错误负载结构稳定且可解析;前端根据错误类型提供可恢复提示。
- 异步/事件(如有):事件类型与载荷字段与文档一致;开始/结束/错误/心跳等语义完整;增量合并无丢失/重复;存在降级路径。
- 端到端透传API→Service→下游存储/第三方)参数(上下文/会话/区域/幂等键)无遗漏;写操作具备幂等/去重策略。
- 日志与隐私:日志含必要上下文(追踪/用户/会话敏感信息脱敏UI 默认不展示内部实现细节。
#### 全链路 Contract Crosswalk必做
- 选取关键用户流,建立“参数对齐矩阵”:
- 前端请求 → 后端路由/处理 → 服务层 → 下游(存储/第三方)→ 持久化字段。
- 列出每个参数:`name | type | required | default | source-of-truth(文件:行)`
- 如存在事件/流式契约,补充事件类型与载荷字段,以及必要的状态回写。
- 输出差异点与最小修复建议(谁改、改哪、如何不破坏现有行为)。
### 4. 高发问题清单(优先排查)
- 命名错位:不同层对同一概念使用了不同命名风格或名称(例如 `camelCase``snake_case`、标识符命名不一致)。
- 路径错位:端点路径或动态段命名不一致;末尾分隔符导致重定向或方法失败。
- 类型漂移:数字/字符串/布尔/时间类型在层间编码不一致或未正确转换。
- 认证混用请求在不同处使用了不同认证方式跨域请求未按需携带凭证CSRF 保护缺失。
- 事件缺口:后端新增事件类型或字段未在前端解析;增量合并逻辑导致重复/丢失。
- 可见性/状态:后端状态位或可见性字段被忽略,导致 UI 展示与真实状态不一致。
### 5. 零假设验证VERIFY-FIRST GateBLOCKING
- 禁止基于“猜测的字段/API/事件”写逻辑。每个关键元素必须给出证据:
- 模型/字段定义位置(文件:行)
- 路由/处理方法签名(文件:行)
- 前端类型/解析/调用代码(文件:行)
- 无证据的假设一律不通过。
### 6. 用户视角 E2E 可见性审计BLOCKING
⚠️ **这是最关键的检查** - "代码完美但功能不工作"的主要原因就是跳过了这一步!
**MANDATORY USER FLOW VERIFICATION (必须执行):**
- **完整点击路径追踪**:从用户点击开始,逐步追踪到最终状态
- 用户点击X → 调用函数Y → 导航到页面Z → 显示内容W
- **必须验证每个步骤都正确执行**
- **URL路由验证**:所有导航路径在路由配置中存在且正确处理参数
- **状态传递验证**点击后的状态变化是否正确反映在UI中
- **错误场景测试**:参数缺失、网络错误、权限不足等场景的处理
**具体检查项目:**
- 入口可见:对应功能的入口(按钮/导航/控件)在默认场景与目标设备上可达;不被误判条件隐藏。
- **链接落地(核心)**:页面路由/回跳/深链接一致;从入口到完成形成闭环。
- 点击通知 → 是否真的跳转到预期页面?
- 分享链接 → 是否真的加载预期内容?
- 所有导航路径都必须实际追踪验证!
- 状态完整:加载/空数据/错误/权限不足 均有清晰呈现与可恢复路径。
- 角色/开关:与权限/Feature Flag 的可见性符合预期;默认值不阻断主流程。
**⛔ 禁止行为:**
- ❌ 只看代码结构,不追踪实际执行流程
- ❌ 假设navigate()调用就等于用户到达了目标页面
- ❌ 不验证URL参数处理逻辑
- ❌ 说"看起来正确"而不验证"实际正确"
### 7. Web3 AI 交易系统安全审查BLOCKING 级别)
🔐 **资金安全是生死线** - 一个安全漏洞可能导致所有资金损失!
#### 7.1 私钥与密钥管理CRITICAL
- **零泄露原则**
- ❌ 禁止:私钥/助记词出现在日志、错误消息、前端代码、Git 历史中
- ❌ 禁止:明文存储私钥(环境变量、配置文件、数据库)
- ✅ 必需使用硬件钱包、HSM、或加密密钥管理服务AWS KMS/Vault
- ✅ 必需API Key、密钥材料必须加密存储运行时解密
- **最小权限原则**
- 交易签名密钥与只读查询密钥分离
- 每个功能使用独立的子账户/权限
- 定期轮换 API Key 和访问令牌
- **验证检查**
- [ ] grep 搜索 `private_key``mnemonic``seed` 等关键词,确保无硬编码
- [ ] 检查所有密钥存储位置的加密状态
- [ ] 验证密钥访问日志和审计追踪
#### 7.2 交易安全CRITICAL
- **签名验证**
- ✅ 必需:所有交易必须经过签名验证
- ✅ 必需:验证交易发起者身份(防止伪造)
- ✅ 必需:使用 nonce/序列号防止重放攻击
- **交易参数验证**
- ✅ 必需验证接收地址合法性checksum、白名单
- ✅ 必需:金额/价格/滑点限制(防止异常大额交易)
- ✅ 必需Gas Price/Gas Limit 上限保护(防止 Gas 耗尽攻击)
- ✅ 必需Deadline/超时保护(防止过期交易执行)
- **滑点与价格保护**
- ✅ 必需:设置合理的滑点容忍度(如 0.5%-2%
- ✅ 必需:价格预言机验证(多源对比、时间戳检查)
- ✅ 必需:异常价格波动拒绝交易
- **验证检查**
- [ ] 所有交易调用都有 nonce 或幂等键
- [ ] 金额/价格参数都有上下限验证
- [ ] Gas 费用有最大限制
- [ ] 滑点保护代码存在且正确
#### 7.3 AI 决策安全CRITICAL
- **提示注入防护**
- ❌ 禁止:直接将用户输入拼接到 AI prompt 中
- ✅ 必需:用户输入消毒/转义(防止 prompt injection
- ✅ 必需:系统提示与用户输入明确分离(使用角色隔离)
- ✅ 必需敏感操作需要用户明确确认AI 不能自主决定大额交易
- **决策审计**
- ✅ 必需:记录所有 AI 决策的完整上下文(输入、输出、时间戳、模型版本)
- ✅ 必需:决策可追溯、可回放、可审计
- ✅ 必需:异常决策告警(如突然的大额交易建议)
- **模型安全**
- ✅ 必需:使用官方 API避免第三方代理防止中间人攻击
- ✅ 必需API 响应验证(检测异常输出、格式错误)
- ✅ 必需:模型输出不直接执行,必须经过参数验证
- **验证检查**
- [ ] 搜索用户输入拼接点,确保有消毒处理
- [ ] 检查决策日志是否完整(包含所有关键参数)
- [ ] 验证大额交易需要额外确认机制
#### 7.4 智能合约交互安全CRITICAL
- **授权范围控制**
- ❌ 禁止:无限授权(`approve(spender, type(uint256).max)`
- ✅ 必需:按需授权,每次交易前计算精确授权额度
- ✅ 必需:定期清理过期授权
- ✅ 必需:监控授权事件,异常授权告警
- **合约调用验证**
- ✅ 必需:合约地址白名单(只与已审计合约交互)
- ✅ 必需:函数选择器验证(防止调用错误函数)
- ✅ 必需:调用参数类型/范围验证
- ✅ 必需模拟执行dry-run后再真实执行
- **重入与异常处理**
- ✅ 必需处理合约调用失败情况revert、out of gas
- ✅ 必需:检查返回值,不假设调用成功
- ✅ 必需:避免在外部调用后修改关键状态(防重入)
- **验证检查**
- [ ] grep `approve` 确保无无限授权
- [ ] 所有合约地址来自配置/白名单,无硬编码
- [ ] 调用失败有完整的错误处理和回退逻辑
#### 7.5 资金保护机制BLOCKING
- **限额控制**
- ✅ 必需:单笔交易金额上限(如 $1000
- ✅ 必需:日/周/月累计限额
- ✅ 必需:异常交易频率限制(防止快速耗尽资金)
- ✅ 必需:大额交易需要多重签名或延迟执行
- **紧急暂停**
- ✅ 必需全局紧急停止按钮kill switch
- ✅ 必需异常检测自动暂停如价格异常、Gas 费暴涨)
- ✅ 必需:暂停后资金安全提取机制
- **余额监控**
- ✅ 必需:实时余额监控,低于阈值告警
- ✅ 必需:异常资金流出告警(大额转出、未知接收方)
- ✅ 必需:定期对账(链上余额 vs 系统记录)
- **验证检查**
- [ ] 限额配置存在且合理
- [ ] 紧急暂停功能可测试且有权限控制
- [ ] 余额监控代码存在且接入告警系统
#### 7.6 链上数据验证CRITICAL
- **预言机安全**
- ❌ 禁止:单一数据源(可被操纵)
- ✅ 必需多预言机对比Chainlink、Band、UMA 等)
- ✅ 必需:价格偏差检测(多源价格差异超阈值拒绝)
- ✅ 必需:时间戳验证(数据新鲜度检查,拒绝过期数据)
- **区块确认**
- ✅ 必需:等待足够的区块确认(主网建议 ≥12 块L2 根据实际情况)
- ✅ 必需处理链重组可能pending → confirmed → finalized
- ✅ 必需交易回执验证status=1 成功)
- **数据完整性**
- ✅ 必需事件日志完整性检查topic、参数匹配
- ✅ 必需:合约状态一致性验证(链上 vs 本地缓存)
- ✅ 必需MEV 保护(使用私有内存池或 Flashbots
- **验证检查**
- [ ] 价格数据来自多个预言机
- [ ] 区块确认数配置合理
- [ ] 交易状态检查包含 finalized 状态
## 审查结果
请给出以下三种结果之一:
-**通过**:可以直接提交
-**不通过**存在BLOCKING问题必须修复
- ⚠️ **需要修复**:有改进空间,建议修复
## 核心原则
1. **白盒逻辑正确性是根本**:业务逻辑错误是生死线
2. **需求驱动**:必须找到真实需求来源
3. **客观分析**:基于实际代码和需求,不自我欺骗
4. **actionable建议**:提供具体的修复指导
## 评审交付物(必须包含)
- **问题清单**:逐条指出"谁与谁不一致"(路径/参数/字段/事件/状态码),附最小复现样本。
- **最小修复建议**:明确"谁改、改哪里、如何不破坏现有调用"(可附 1-3 行级 diff 建议)。
- **兼容/过渡策略**:必要时说明双解析/版本前缀/灰度开关/降级方案。
- **🚨 E2E验证报告**对每个用户交互流程的完整追踪验证MANDATORY
## 强制性E2E验证清单必须逐项检查
在给出审查结果前,必须完成以下验证:
### ✅ 用户点击验证
- [ ] 所有onClick处理器都能正确执行
- [ ] 处理器中的navigate()调用指向正确的路径
- [ ] 目标路径在路由配置中存在
- [ ] 目标页面能正确处理URL参数
### ✅ 导航流程验证
- [ ] 从点击到页面加载的完整路径畅通
- [ ] URL参数正确传递和解析
- [ ] 页面状态正确初始化
- [ ] 用户看到预期的内容和界面
### ✅ 状态一致性验证
- [ ] 点击后应用状态正确更新
- [ ] UI界面反映状态变化
- [ ] 没有状态不同步的问题
### ✅ 安全验证Web3 AI 交易系统 - MANDATORY
- [ ] **密钥安全**:无私钥泄露(日志/错误/前端/Git
- [ ] **密钥管理**:私钥加密存储,无明文环境变量
- [ ] **交易验证**所有交易有签名验证、nonce、金额限制
- [ ] **滑点保护**:价格/滑点验证存在且合理
- [ ] **AI 安全**:用户输入有消毒处理,无直接拼接到 prompt
- [ ] **决策审计**AI 决策有完整日志(输入/输出/时间戳)
- [ ] **合约安全**:无无限授权,合约地址来自白名单
- [ ] **限额保护**:存在单笔/累计交易限额
- [ ] **紧急机制**:有 kill switch 或暂停功能
- [ ] **预言机安全**:价格数据来自多源,有偏差检测
- [ ] **确认机制**:大额交易需要用户明确确认
### ⛔ 审查失败条件
如果以下任一项为真,审查必须标记为❌不通过:
**功能性问题:**
- 存在navigate()指向不存在或错误的路径
- 用户点击后无法到达预期页面
- 状态更新不完整导致UI不一致
- 关键用户流程无法完成
**安全性问题Web3 AI 系统):**
- 私钥/助记词出现在日志、错误消息、前端代码、Git 历史中
- 私钥明文存储(环境变量/配置文件/数据库)
- 交易缺少签名验证、nonce、或金额限制
- 存在无限授权(`approve(spender, type(uint256).max)`
- 用户输入直接拼接到 AI promptprompt injection 风险)
- AI 可以自主决定大额交易(无用户确认)
- 缺少紧急暂停机制
- 单一预言机数据源(可被操纵)
- 大额交易无多重签名或延迟执行
**记住:代码编译通过 ≠ 功能正确工作 ≠ 资金安全**
## 技术验证方法MANDATORY
### 🔍 导航路径验证脚本
执行以下检查来验证导航逻辑:
```bash
# 1. 找出所有navigate()调用
grep -r "navigate(" frontend/src --include="*.tsx" --include="*.ts" -n
# 2. 找出所有路由定义
grep -r "path=" frontend/src --include="*.tsx" --include="*.ts" -n
# 3. 检查URL参数处理
grep -r "useSearchParams\|URLSearchParams" frontend/src --include="*.tsx" --include="*.ts" -n
```
### 🔍 状态管理验证
```bash
# 检查状态更新逻辑
grep -r "useState\|useEffect.*navigate" frontend/src --include="*.tsx" --include="*.ts" -n
# 检查onClick处理器
grep -r "onClick.*=>" frontend/src --include="*.tsx" --include="*.ts" -n
```
### 🚨 必须回答的验证问题
对于每个用户交互,审查者必须回答:
1. **点击发生什么?**
- onClick处理器具体做了什么操作
- 调用了哪些函数?传递了什么参数?
2. **导航去哪里?**
- navigate()的目标路径是什么?
- 这个路径在路由配置中存在吗?
- 路径参数格式正确吗?
3. **目标页面做什么?**
- 目标页面/组件如何处理URL参数
- 是否正确提取和使用参数?
- 用户最终看到什么内容?
4. **状态是否一致?**
- 点击后应用状态如何变化?
- UI是否正确反映状态变化
- 有没有状态不同步问题?
**如果审查者无法回答这些问题,审查必须标记为❌不通过**
## 快速验证提示
- 端点集中来源:前端禁止硬编码 URL新增/变更端点已同步到常量/SDK。
- 认证一致:跨域/跨端口请求按需携带凭证Cookie/Token不依赖未声明的自定义头。
- 异步降级:在不支持事件/流式或网络异常时具备降级路径与用户提示。
- 可见性扫描:关键入口在默认态与目标设备上可见;空/错误/加载可复现且可恢复。
- 自动化检查:加入简单脚本/CI 规则检查硬编码端点、路径格式、必需认证头/凭证的使用一致性。