审查代码
基于证据和项目上下文进行审查。优先考虑错误行为、实际风险和回归问题;不要仅仅为了填充清单而提出意见。
尊重范围与安全
- 将审查视为只读操作。未经明确请求,不得编辑代码、切换分支、安装或更新依赖项、应用修复、提交、发布评论或执行部署。
- 当安全的且相关的本地检查可执行时,可以执行不可变的本地检查。在执行来自不可信更改的代码之前,请评估命令及其影响。
- 未经授权,不得将代码、秘密或元数据发送到外部服务。
- 区分审查与实现:当请求仅为审查时,报告发现而不进行修正。
- 保留用户现有的更改,避免破坏性命令。
构建上下文
- 完整阅读
AGENTS.md以及适用于被审查目录的说明。 - 通过请求、问题、PR 描述、差异和相关历史记录确定变更的目标、要求和范围。
- 在断定某处存在缺陷之前,检查所有被更改的文件以及直接相关的代码。
- 验证项目的实际配置、版本、架构和约定。不要通过类比应用其他语言或框架的模式。
- 在提问之前,先调查本地可用的信息。仅当信息缺失导致无法做出负责任的结论时才提问。
- 记录分析的局限性,例如未执行的测试、不可用的依赖项或缺少需求。
执行审查
在适用的维度上评估每个更改:
功能与回归
- 确认实现满足请求的行为并保留现有流程。
- 检查边界条件、空值、部分失败、操作重复、并发和状态转换。
- 检查错误处理、回滚、幂等性和副作用。
- 验证公共接口、向后兼容性、迁移以及对消费者的影响。
安全与隐私
- 在正确的边界检查输入验证和输出编码。
- 查找授权、认证、CSRF、XSS、注入、路径遍历、SSRF、不安全上传和批量赋值漏洞。
- 检查代码、响应、URL 和日志中是否暴露秘密或个人数据。
- 根据用例和资源评估权限;孤立认证不能替代授权。
- 将依赖项警报视为需要确认的证据,考虑可利用性和修复的兼容性。
数据、性能与可靠性
- 识别 N+1 问题、重复查询、过度加载、代价高昂的循环以及与预期量级不匹配的算法。
- 当更改涉及持久化时,检查事务、锁、约束、索引和引用完整性。
- 在相关路径上检查分页、限制、超时、重试和内存消耗。
- 仅在存在合理的瓶颈时才建议优化或缓存;同时考虑失效和一致性。
- 在货币计算中,检查小数精度、四舍五入和原子性。
设计与可维护性
- 评估内聚性、关注点分离、依赖方向和领域语言。
- 优先选择简单且显式的代码。不要要求任意小的函数或过早的抽象来消除所有重复。
- 检查名称、契约、类型处理以及解释非明显决策的注释。
- 将机械风格问题留给项目的格式化工具和 linter,除非影响正确性或理解。
测试与文档
- 检查测试是否与风险成比例,覆盖主要行为、边界、错误和相关的回归。
- 评估断言的质量、隔离性以及测试在发生预期缺陷时失败的能力;不要仅使用覆盖率百分比。
- 当公共行为发生变化时,检查文档、API 契约和操作说明。
- 在执行检查之前,遵循仓库规定的命令和环境。准确报告已执行和未执行的内容。
对 PHP 和 CakePHP 应用检查
当技术栈涉及 PHP 或 CakePHP 时,还需:
- 检查与 PHP 版本的兼容性,并使用项目配置的工具,如 PHPUnit、PHPCS、PHPStan、Psalm 和
composer audit。 - 保持 Controller 专注于 HTTP 输入和编排;将业务规则放在项目采用的层中,通常是 Services。
- 检查每个操作和资源的 Policy 和授权检查。
- 评估 Validators、RulesChecker、
patchEntity()、Entity 的可访问字段以及批量赋值保护。 - 优先使用 ORM、查询构建器和绑定参数,避免拼接 SQL;检查 associations、
contain()、字段选择以及 N+1 风险。 - 在涉及多次写入的操作中检查事务,并区分持久化失败与意外异常。
- 检查模板中的转义、显式不转义的字段、CSRF 以及可能泄露内部细节的响应。
- 检查迁移的实际可逆性、现有值、锁、索引、外键以及与受支持数据库的兼容性。
- 尊重
composer.lock;不建议不加区分地更新依赖项。
对于 Lavanderias Core 团队中的 CakePHP 项目,尤其需要遵循 Controller → Service → Table/Entity 流程、AbstractService::run() 契约、按配置文件的 Policy、MariaDB、PHPUnit 以及在容器内执行命令(测试时始终不带 -u root)。
对 Web 前端应用检查
- 检查上下文转义、安全的 DOM 操作、CSRF、服务器端验证以及数据暴露。
- 检查加载、错误、并发请求、取消和乱序响应状态。
- 遵守本地技术栈和约定;在 Lavanderias Core 的新代码中,优先使用 StimulusJS 和 Axios,将 jQuery 限制在遗留维护中。
- 当更改影响界面时,评估可访问性和响应式行为。
验证发现
在报告问题之前:
- 确认该问题是由审查范围引入或影响的。
- 定位演示该问题的最小代码区域。
- 描述一个具体场景,其中该行为会发生。
- 检查其他层、配置或测试是否已经减轻了风险。
- 根据概率和影响调整严重性,而不是根据规则的抽象类型。
差异中不存在并不能证明系统中不存在。如果仓库可用,在报告缺少授权、验证、事务或同等保护之前,检查相关层。如果只有孤立的片段,则将该点作为条件性风险或疑问提出,而不是确认的缺陷。
不要将遥远的假设作为确认的缺陷提出。当存在相关的不确定性时,明确标识它,并说明哪些证据可以解决疑问。
生成报告
首先按严重性顺序呈现发现:
- 阻塞: 数据损坏、可利用漏洞、严重不可用或核心行为不可行。
- 高: 具有重大影响的可能缺陷或重要回归。
- 中: 具有有限影响的实际问题,依赖于特定条件。
- 低: 具体且局部的改进,没有立即的重大风险。
对于每个发现,包括:
### [严重性] 客观标题
- 位置:`path/to/file.ext:line`
- 证据:代码中可演示的行为。
- 影响:受影响的场景及其后果。
- 建议:与之成比例的修复方向,无需重写整个解决方案。
始终使用文件的本地行号,而不是差异文件中片段的全局位置。如果差异未提供足够的 hunk 编号,则引用符号或方法,并说明行号不可用。保持范围简短且直接与发现相关。将等效的出现分组,而不是重复评论。
在发现之后,简要记录:
- 影响结论的疑问或假设;
- 执行的检查及其结果;
- 残余风险或测试缺口。
如果没有发现,则明确声明,并仍报告局限性和执行的检查。不要用更改的通用摘要替代发现,除非用户也请求摘要。