code-review

代码审查

适用场景

• 用户分享代码（文件、代码片段或差异），并请求反馈
• 用户粘贴拉取请求，或想了解代码是否已达到生产就绪状态
• 合并前质量把关或查找缺陷
• 审查拉取请求中的架构决策

所需上下文

从 startup-context 获取：技术栈、产品阶段、团队规模。此外还需要用户提供：

• 要审查的代码或差异
• 代码的预期用途（拉取请求的目的或功能背景）
• 任何具体关注点（性能、安全性、正确性）
• 如果无法从代码中明确判断，需提供语言和框架

工作流

遵循结构化的五步方法。每一步必须完成后才能进入下一步。

1. 上下文 — 在开始任何分析前，了解并总结拉取请求的目的。用 1-2 句话向用户复述意图。如果不清楚，先询问再继续。切勿在不了解代码要实现什么的情况下开始审查。
2. 结构 — 评估架构决策和设计模式：
   • 代码是否放在了合适的模块或层中？
   • 抽象是否恰当（不过多，也不过少）？
   • 此更改是否符合现有代码库的模式？
   • 对于不明显的设计选择，在提出替代方案之前，先认可作者的考虑。
3. 细节 — 从多个维度评估代码质量：
   • 正确性： 逻辑错误、差一错误、null/undefined 处理、竞态条件、边界情况
   • 安全性： OWASP Top 10 基线——注入、身份验证失效、数据泄露、XSS、访问控制、错误配置、不安全的反序列化、存在漏洞的组件
   • 性能： N+1 查询、不必要的重新渲染、大型数据集上的 O(n^2)、缺少缓存、内存泄漏
   • 命名和清晰度： 名称能否表达意图？函数是否专注于单一职责？
4. 测试 — 以与代码审查同等的严谨程度验证测试覆盖率：
   • 是否包含行为断言（而非仅测试实现）？
   • 是否覆盖边界情况和错误路径？
   • 测试是否脆弱，还是能适应重构？
   • 缺少哪些测试用例？
5. 反馈 — 生成按优先级和类别整理的报告，提供具体代码示例和切实可行的改进建议。明确认可优秀的模式和决策。

输出格式

# Code Review: [Feature/File Name]

## Summary
One-paragraph assessment: what the PR does, whether it is ready to merge, needs minor fixes, or needs rework.

## Findings

### Critical (must fix before merge)
- **[CRT-1] Title** — file:line — description, why it matters, suggested fix with code example

### Major (should fix before merge)
- **[MAJ-1] Title** — file:line — description, why it matters, suggested fix

### Minor (fix when convenient)
- **[MIN-1] Title** — file:line — description, suggestion

### Positive (things done well)
- **[POS-1] Title** — file:line — what was done well and why it matters

## Questions
Clarifying questions about non-obvious design choices before blocking on them.

## Suggested Tests
- Test case 1
- Test case 2

框架与最佳实践

严重程度定义

严重程度	定义	处理方式
严重	安全漏洞、数据丢失风险、生产环境崩溃、核心功能失效	阻止合并
重大	严重缺陷、性能回退、关键路径缺少错误处理、违反架构规范	合并前应修复
轻微	风格问题、命名改进、小幅优化、文档缺失	方便时修复
正面	代码编写良好、模式运用得当、错误处理周全	明确认可并予以鼓励

审查原则

• 反馈始终要有具体依据。 每项发现都必须注明文件和行号，并包含具体的改进建议，不能只说“这里可以做得更好”。
• 明确认可出色的工作。 指出优秀的模式、清晰的抽象和周全的错误处理。只指出问题的审查会打击士气，也不够全面。
• 认可作者的考虑。 对于不明显的选择，假设作者有其理由。提出替代方案前先询问。可以这样表达：“我看到你选择了 X——这是因为 Y 吗？如果是，可以考虑 Z 作为替代方案。”
• 不要因自动化工具会处理的风格问题而阻止合并。 代码检查和格式化是 CI 的职责，不是审查者的职责。应关注逻辑、架构和正确性。
• 以同等重要性审查测试。 测试不是事后才考虑的事项。关键路径缺少测试属于重大问题，而非轻微问题。

OWASP Top 10 快速检查

1. 注入 — 用户输入是否经过参数化处理？检查 SQL、NoSQL、OS 命令和 LDAP
2. 身份验证失效 — 会话是否安全？令牌是否轮换？密码是否经过哈希处理（bcrypt/argon2）？
3. 敏感数据泄露 — 密钥是否存储在环境变量中（而非代码中）？静态存储的个人身份信息是否加密？
4. XXE — 是否已禁用 XML 解析，或配置为拒绝外部实体？
5. 访问控制失效 — 每个端点是否都检查授权，而不仅仅是身份验证？
6. 安全配置错误 — 是否已关闭调试模式？CORS 限制是否严格？是否设置了安全标头？
7. XSS — 输出是否经过编码？是否未对用户数据使用 dangerouslySetInnerHTML / v-html？
8. 不安全的反序列化 — 是否已验证来自不可信来源的序列化对象？
9. 易受攻击的组件 — 依赖项是否为最新版本？是否存在已知的 CVE？
10. 日志记录不足 — 是否记录了身份验证失败、访问违规和错误？

N+1 查询检测

• 遍历集合时，为每个项目执行一次数据库调用
• 在循环中使用 ORM 延迟加载（例如，在 for user in users 循环中访问 user.posts）
• GraphQL 解析器为每个父项获取关联数据，却未使用 DataLoader
• 修复方法： 使用预加载、批量查询或 DataLoader 模式

语言无关的危险信号

• 函数超过 40 行，或嵌套层级超过 3 层
• 会改变行为的布尔参数（应使用不同的函数）
• 静默吞掉异常的 catch 块
• 应该由配置控制的硬编码值
• 公共 API 边界缺少输入验证

相关技能

• security-review — 出现严重安全问题时，衔接深入的安全审计
• architecture-design — 代码审查发现需要重新思考架构的结构性问题时，衔接此技能

示例

示例提示词：“审查这个用于创建用户的 Express 路由处理程序。”

良好输出片段：

# Code Review: POST /api/users Handler

## Summary
The handler creates users but has a critical SQL injection vulnerability and missing
input validation. Needs fixes before merge. Clean async/await usage is a positive.

## Findings
### Critical
- **[CRT-1] SQL Injection** — routes/users.js:14 — Email interpolated
  directly into SQL string. Use parameterized queries:
  `db.query('INSERT INTO users (email) VALUES ($1)', [email])`

### Major
- **[MAJ-1] No input validation** — routes/users.js:12 — Email from
  `req.body` used with no validation. Add zod/joi schema validation.
- **[MAJ-2] Missing error handling** — routes/users.js:18 — DB errors
  propagate as 500 with stack trace. Wrap in try/catch, return generic error.

### Positive
- **[POS-1] Clean async/await** — routes/users.js:11 — Easy to follow,
  no callback nesting.

## Questions
- Is there a validation middleware already in the project that should be reused here?