name: code-review description: 执行全面的代码审查,专注于正确性、可维护性、性能、安全性和最佳实践。触发关键词:review, code review, PR review, pull request, check code, audit code, feedback, approve, request changes, comment, suggestion, LGTM, nit, blocker, code quality, best practice, architecture review, design review, security review, infra review. allowed-tools: Read, Grep, Glob, Bash
代码审查
概述
本技能提供跨多个领域的详细代码审查能力,分析代码中的错误、设计问题、性能问题、安全漏洞和最佳实践遵守情况。它帮助在代码进入生产前识别潜在问题。
代理分配
进行代码审查时,考虑使用专业技能进行特定领域的专业知识:
- 高级软件工程师(Opus)- 架构、设计模式、复杂逻辑审查、安全审查(使用
/security-audit技能)、基础设施审查 - 软件工程师(Sonnet)- 回应审查反馈,实施修复
对于专业审查,使用相关技能,如/security-audit用于安全关注点,/concurrency用于线程安全,/data-validation用于输入验证等。
指令
1. 收集上下文
- 使用 Glob 模式识别要审查的文件
- 理解项目结构和约定
- 检查现有的代码检查/格式化规则
2. 分析代码结构
- 审查文件组织和模块结构
- 检查关注点分离是否恰当
- 验证命名约定是否一致
3. 检查常见问题
- 逻辑错误和边界情况
- 错误处理的完整性
- 资源管理(内存泄漏、未关闭的句柄)
- 并发代码中的线程安全问题
- 输入验证缺口
4. 评估代码质量
- 可读性和清晰度
- DRY 原则的遵守情况
- SOLID 原则的符合性
- 适当的抽象级别
- 测试覆盖的充分性
5. 性能审查
- 算法复杂性分析
- 数据库查询效率
- 内存使用模式
- 缓存机会
审查严重级别
- 阻碍者:必须在合并前修复 - 安全问题、数据丢失、崩溃
- 关键:应在合并前修复 - 逻辑错误、主要错误、性能问题
- 主要:尽快修复 - 代码质量、可维护性、技术债务
- 次要:最好有 - 风格偏好、建议、细微问题
特定领域检查清单
安全审查检查清单
-
认证/授权
- 凭据存储得当(哈希、加盐密码)
- 会话管理(过期、安全cookie、CSRF令牌)
- 所有敏感操作的访问控制检查
- OAuth/JWT令牌验证和过期
-
输入验证
- SQL注入预防(参数化查询)
- XSS预防(输出编码)
- 命令注入预防(不使用用户输入执行shell)
- 路径遍历预防(清理文件路径)
- SSRF预防(验证URLs)
-
数据保护
- 敏感数据在传输和存储时加密
- PII处理合规(GDPR, CCPA)
- 不在代码或日志中存储密钥
- API的速率限制
- 安全随机数生成(加密级别)
-
依赖项
- 没有已知易受攻击的依赖项
- 供应链安全(验证校验和)
- 最小攻击面
基础设施代码审查检查清单
-
Terraform/IaC
- 没有硬编码的凭据或密钥
- 状态文件后端安全配置
- 资源标记用于成本追踪
- 适当的IAM角色(最小权限原则)
- 网络安全(VPC、安全组、防火墙规则)
- 灾难恢复配置(备份、多区域)
-
Kubernetes
- 定义资源限制和请求
- Pod安全策略/准入控制器
- 网络策略用于隔离
- 密钥管理(外部密钥操作符)
- 健康检查(活跃度、就绪度探针)
- RBAC配置得当
-
Docker
- 最小基础镜像(无发行版、Alpine)
- 层中没有密钥
- 用于大小的多阶段构建
- 非root用户执行
- 启用漏洞扫描
-
CI/CD
- 管道安全(密钥注入、不在日志中)
- 构建可重复性
- 部署回滚能力
- 在生产前在测试环境测试
数据管道审查检查清单
-
数据质量
- 模式验证
- 空值处理策略
- 重复检测
- 数据类型强制
-
可靠性
- 幂等操作
- 一次精确处理保证
- 失败的死信队列
- 监控和告警
-
性能
- 适当的地方批量处理
- 分区策略
- 压缩使用
- 查询优化
-
成本
- 存储生命周期策略
- 计算资源规模调整
- 数据转移最小化
ML代码审查检查清单
-
模型开发
- 可重复实验(设置种子)
- 数据版本控制
- 模型版本控制
- 特征工程文档
-
训练
- 训练/验证/测试划分正确性
- 过拟合检查
- 超参数追踪
- 梯度爆炸/消失检查
-
生产
- 模型服务延迟要求
- 模型监控(漂移检测)
- A/B测试能力
- 回滚策略
-
伦理
- 训练数据中的偏差检测
- 公平性指标
- 可解释性/可理解性
- 隐私保护(差分隐私)
最佳实践
- 具体:指向具体行并提供具体建议
- 优先处理问题:使用严重级别(阻碍者、关键、主要、次要)
- 解释原因:不仅仅说错在哪里,解释理由
- 建议解决方案:尽可能提供替代实现
- 认可好代码:识别编写良好的部分
- 考虑上下文:理解约束和权衡
- 建设性:以积极和专业的方式提出反馈
- 关注影响:按用户/业务影响优先处理问题
- 参考标准:链接到风格指南、安全基准、RFCs
示例
示例1:审查Python函数
# 审查前
def process(data):
result = []
for item in data:
if item['status'] == 'active':
result.append(item['value'] * 2)
return result
# 审查评论:
# 1. 函数名称太通用 - 考虑 'double_active_values'
# 2. 没有类型提示 - 添加类型以提高可维护性
# 3. 没有文档字符串说明目的和参数
# 4. 没有对输入数据做空/空检查
# 5. 可以使用列表推导式使代码更清晰
# 审查后
def double_active_values(data: list[dict]) -> list[int]:
"""
将数据集中所有活动项目的值加倍。
参数:
data:包含'status'和'value'键的字典列表
返回:
活动项目加倍值的列表
"""
if not data:
return []
return [item['value'] * 2 for item in data if item.get('status') == 'active']
示例2:安全审查标志
// 关键:SQL注入漏洞
const query = `SELECT * FROM users WHERE id = ${userId}`;
// 推荐:使用参数化查询
const query = "SELECT * FROM users WHERE id = ?";
db.query(query, [userId]);
示例3:性能审查
# 问题:由于嵌套循环与列表成员检查,O(n*m)复杂性
for user in users:
if user.id in active_ids: # 每次O(n)查找
process(user)
# 推荐:转换为集合以O(1)查找
active_ids_set = set(active_ids)
for user in users:
if user.id in active_ids_set:
process(user)
示例4:结构化审查与严重级别
# 代码审查:auth/login.py
## 阻碍者问题
### 第45行:SQL注入漏洞
sql = f"SELECT \* FROM users WHERE email = '{email}'"
cursor.execute(sql)
**问题**:用户输入直接内插到SQL查询中。
**影响**:攻击者可以执行任意SQL命令。
**修复**:使用参数化查询:
cursor.execute("SELECT \* FROM users WHERE email = ?", (email,))
## 关键问题
### 第78行:密码以纯文本存储
user.password = request.form['password']
**问题**:密码未哈希存储。
**影响**:数据库泄露暴露所有用户密码。
**修复**:使用bcrypt或argon2:
from bcrypt import hashpw, gensalt
user.password = hashpw(password.encode('utf-8'), gensalt())
## 主要问题
### 第112行:缺少错误处理
user = User.query.filter_by(email=email).first()
send_email(user.email, token)
**问题**:访问属性前未检查用户是否存在。
**影响**:如果用户未找到,AttributeError崩溃。
**修复**:添加空检查:
if user is None:
return {"error": "User not found"}, 404
## 次要问题
### 第23行:命名不一致
def GetUser(id): # PascalCase函数
**问题**:Python约定是函数使用snake_case。
**修复**:重命名为 `get_user(id)` 以保持一致性。
## 积极点
- 验证逻辑分离良好(第30-40行)
- 全面的单元测试覆盖
- 认证流程文档清晰
示例5:基础设施审查(Terraform)
# 阻碍者:第12行 - 硬编码AWS凭据
provider "aws" {
access_key = "AKIAIOSFODNN7EXAMPLE" # 永远不要提交凭据
secret_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
}
# 修复:使用AWS凭据配置文件或环境变量
provider "aws" {
profile = var.aws_profile
region = var.aws_region
}
# 关键:第45行 - S3桶公开可访问
resource "aws_s3_bucket" "data" {
bucket = "company-data"
acl = "public-read" # 公开所有数据
}
# 修复:设为私有并使用桶策略进行受控访问
resource "aws_s3_bucket" "data" {
bucket = "company-data"
acl = "private"
}
# 主要:第78行 - 缺少备份配置
resource "aws_db_instance" "main" {
# ... 其他配置 ...
backup_retention_period = 0 # 无备份
}
# 修复:启用自动备份
backup_retention_period = 7
backup_window = "03:00-04:00"