审查 Frappe 应用代码质量与安全时的检查清单

在审查 Frappe/ERPNext 应用的 diff、PR 或代码片段时，按正确性、安全、性能、并发、可读性、API 设计与测试逐项排查。

适用场景

解决思路

1. 按后果严重程度排序审查：先正确性与稳定性（§1），再安全（§2），其余依次；优先根因修复而非绕过，并说明问题为何重要（会破坏什么、影响谁）。
2. 把有状态及法律/会计/合规代码视为“失败不可接受”的代码，重点防范无报错的静默状态损坏（错误记账/库存）。
3. 对内部不变量使用断言尽早且响亮地失败，例如 `assert total_credit == total_debit`；断言只用于代码保证的不变量，不用于面向用户的校验。
4. 对每处改动都设想它会在哪里出错、会被如何误用，写出防呆代码；扩展与覆写尤其要考虑各种失败模式。
5. 禁止事务中途的部分提交：标记任何游离的 `frappe.db.commit()` / `db.rollback()`；绕过校验提交/保存（校验失败却无 SL/GL 分录）是严重 bug。
6. 不要为便利牺牲原子性（例如为修复引导流程加 `autocommit` 会让事务非原子）；autocommit 只用于建表。
7. 优先保证不变量而非 UX：“宁可牺牲 UX，也要保证正确性”；不要为迁就坏代码而降低可用代码质量。
8. 修复前先验证问题本身，有时正确的做法是“不修”；先定位根因。
9. 警惕空/`None` 过滤条件的破坏性 DB API：`set_value("Site", None, ...)` 或无过滤的 `db.delete` 会更新/删除整表；这类调用必须报错。标记任何 name/filter 可能为 `None`、空或攻击者可控的 `set_value`/`delete`/`get_value`。
10. 检查条件中类型是否真正匹配：类型不匹配的比较（字符串 vs `datetime`、字符串 vs int）会静默永不匹配或恒为真；在边界处显式转换（`cint`/`flt`）。
11. 质疑返回结构：索引结果前先问它是否可能为 `None` / `[None, None]`；注意 `as_dict=1`（字典列表）与标量的混淆。
12. 不要静默改变长期语义：调用方长期依赖的行为是契约，即使签名未变，改动也是隐性破坏性变更。
13. 安全关键代码（认证、授权、权限、用户管理）要加倍严格审查；避免漏洞远比安全修复容易。
14. 绝不拼接字符串/f-string 构造 SQL，使用 ORM 或查询构建器；必须用原生 SQL 时用参数替换（`frappe.db.sql("... where name = %s", (user,))`），绝不自行插值；更好是避免新增原生 SQL（除注入风险外还绑定单一数据库，框架追求 DB 无关，优先 `frappe.qb`）。
15. 即使使用 ORM，类型混淆也是注入途径：Frappe 接受复杂类型，预期为字符串的参数可能传入过滤列表（如 `{"key": ["!=", ""]}` 传给 `db.get_value` 可绕过密钥检查）；在信任边界校验输入类型（显式 `isinstance(key, str)`），审计每个 `@frappe.whitelist` 方法。
16. 绝不自行 `eval`/`exec`；只用 `safe_eval`/`safe_exec` 且限量，并记住“safe_exec 不是魔法”；绝不接受客户端提供的方法路径去执行。
17. 沙箱执行（RestrictedPython/`safe_exec`）不可靠，假设存在逃逸；安全开关必须放在正确的信任边界：server-script 启用是 bench 级配置，绝不放 site 级（租户可借此接管整个服务器）。
18. 优先允许列表而非阻止列表（阻止列表可被绕过）；不要默认暴露一切。
19. “`allow_guest=True` 前想十遍”——它不是绕过真正认证/授权的捷径；网页必须在读取/共享数据前应用权限；优先 `get_list`/`get_all` 而非手写查询。
20. 精确限定放宽范围：确认限流/权限例外只针对预期主体，而非例如所有非 guest 用户。
21. 路径遍历/文件系统：优先 File doctype API；用户输入进入路径时确保不能穿越（`/../../`）站点目录。
22. 加密/密钥：绝不自行发明加密，复用现有实现；校验 guest/webhook 请求真实性（HMAC）。
23. 签名/一次性 URL：使用真正保密的签名值；同时按时间和首次使用过期；仅用 URL 本身校验，不要合并表单数据（Frappe 会合并 URL 与表单数据，导致一次有效签名可重放）。
24. 密钥存入 password 字段，绝不存明文，绝不在日志或错误信息中泄露。
25. XSS 与 OWASP 其余项：不要把用户输入注入 DOM；即使看似琐碎也按严重处理（HTML/JS 注入通常导致账号劫持）；不要靠清洗并丢弃特殊字符来修 XSS，应在注入 DOM 前转义。
26. 性能即正确性：在审查阶段修复性能最便宜；性能是功能（Doherty 阈值，人类感知约 100ms）。
27. 性能预算：常见读取 < 100ms；读取 < 1s；多数写入 < 5s；绝不超过约 10s；频繁只读请求 P99 约 1s；慢同步请求会阻塞 worker（队头阻塞）。
28. 复杂度规则：高频端点必须做 O(1) 或 O(log N) 工作，最差大常数因子，绝不 O(N)；计数行是 O(N) 而非 O(1)，大表/带过滤的 `COUNT(*)` 很贵；限制无界扫描（如最近 3 个月、"1000+" 哨兵值）而非全表扫描。
29. 索引即代码：标记任何在未索引列上的 `WHERE`/join/filter；索引（含自定义索引）必须在代码中提交，不能临时添加（迁移会丢失）；表单加载拉取 comments/versions/assignments 时所有这些查询都要有索引，一个未索引查询就会拖慢整体。
30. 循环中禁止 DB 调用：“不要写循环里调 db 的校验”；标记 N+1 模式；缓存稳定值（UOM、docstatus、status）而非重复查询；后台任务可接受，请求中绝不允许。
31. 对无法修复的慢代码用 Remove → Reduce → Reuse 阶梯：删除、减少调用、记忆化；选对缓存作用域（`@redis_cache`、`@request_cache`、`@site_cache`，后者滥用会膨胀内存）；不要在 `frappe.local` 或 `frappe.flags` 手写缓存（“只会制造全新的缓存失效 bug”）；不要缓存极其廉价的工作。
32. 内存：不要把垃圾塞进共享模块级文件 / `__init__.py` / 类级状态（会永久驻留）；移除未使用的模块级 import（移入使用它的函数）；警惕泄漏。
33. 重排条件让 DB 调用最后：布尔表达式中把廉价内存检查放前面，靠短路跳过查询。
34. 在 SQL 中聚合而非 Python：用查询里的 `SUM()`/`COUNT()`，不要取全部行再在内存归约；把过滤下推到子查询，使其在 join 前生效。
35. 不要为取一个值而取整个文档：单列用 `get_value`/`get_single_value`/`set_value` 而非 `get_doc().save()`；删除用 `frappe.delete_doc` 而非 `get_doc().delete()`（后者只为删除而取文档）。
36. 热路径中不要用 MyISAM 表：读 MyISAM 表会加隐式表级锁，请求路径中绝不触碰。
37. 长任务移到后台队列：用 `enqueue(..., queue="long")`，不要放在阻塞 worker 的同步请求中。
38. 并发：check-then-act 是竞态（`if not frappe.db.exists(...): insert()` 两个 worker 都看到不存在并都插入）；优先 DB 级唯一约束，“把完整性外包给数据库”。
39. 锁的陷阱：对未索引查询做 `SELECT ... FOR UPDATE` 会锁住所有扫描行（及间隙），务必确保过滤走索引，否则锁整表；只锁父不锁子会产生“变异”文档。
40. 全局可变状态/类属性在 Python 中是全局的：共享引擎/类属性会在并发请求间泄漏查询状态产生垃圾；查询构建必须无状态；不要“对 `frappe.local` 做奇怪的事”——`local` 用于变量而非静态状态。
41. 可读性与可维护性：约 50% 开发时间用于读代码，腐烂代码最终迫使重写。
42. 函数能纯则纯，便于阅读与测试。
43. 不要传递可变对象去填充（“装配”式代码），应返回新值；传可变对象去修改会迫使读者打开两个文件才能理解一件事。
44. 会修改输入的函数必须命名得当。
45. 优先无聊的写法：函数式 map-reduce 一行虽美，请直接写 4 行 for 循环；偏好可调试代码而非聪明代码。
46. 一致性高于个人风格：代码库不应是 10 种风格的大杂烩；匹配周边格式/命名/import 约定，标记破坏约定的改动。
47. 好品味：重构使边缘情况变成常见情况，去掉特例分支；问：能否更简单？代码更少？是否是过度缩进的 if-else 汤？
48. 优先扩展共享组件而非复制粘贴分化：同一事物 3–4 个分叉实现会拖慢长期速度；避免模块间紧耦合，通过清晰、有文档的公共 API 集成。
49. 为公共模块/类/函数写 docstring；优先类型注解而非用文字描述类型（“类型提示好 10 倍”），类型检查器能发现非显而易见的 bug。
50. docstring 只提重要事项，简短切题；不要解释函数名已显而易见的琐事，聚焦“为什么”。
51. 把不相关改动拆到独立 commit/PR，保持审查聚焦、`git blame`/回滚干净。
52. API 设计：最小惊讶原则——API 名称+签名应传达约 90% 意图，用户不应被行为惊讶。
53. 拒绝接受多种不相干类型的松散/重载参数（string/dict/list/None）；优先独立的单一用途函数；警惕隐式回退，使用显式变体；“正确用法依赖部落知识的 API 是负债”。
54. 为扩展而构建，而非覆写：提供钩子；绝不在运行时 monkey-patch 核心（“不可原谅地糟糕”，会破坏未来修复），绝不复制整个核心文件改几行（修复无法传播）。
55. 成熟/公共 API 的向后兼容是义务：遵循 semver，次版本零破坏性变更；破坏性变更包括删除公共函数/字段、重排参数、新增必填参数、改变业务逻辑、移动/重命名文件（import 断裂）、升级共享依赖；重命名不保留旧名别名是不必要的破坏；每个破坏性变更都要带弃用警告+文档。
56. 警惕 schema 破坏性变更陷阱：给现有站点加必填字段、把长期字段设为唯一（需数据补丁）、无补丁改字段类型、删除字段。
57. 会静默跳过现有站点的 schema 变更需要数据补丁：Single doctype 不会把新字段默认值同步到现有站点，字段类型变更（如 text→int）不会转换现有值，两者都需显式补丁并在有数据的站点上测试。
58. 新参数放最后作为关键字参数并带安全默认值（`None` 而非 `""`），避免破坏现有位置调用；重命名时保留旧名作为 shim：`def old_name(...): return new_name(...)`。
59. 补丁卫生：数据补丁必须幂等（可安全重跑）、顺序正确（在其读取的字段/doctype 存在后运行）、放在正确的 app（框架改动在框架打补丁，而非下游 app）。
60. 修改现有测试是危险信号：若让改动通过需要编辑现有测试断言，很可能破坏了真实工作流，应显式说明理由而非迁就测试。
61. 测试：每个 PR 需有像样的测试覆盖——针对 diff 的补丁覆盖率，而非仅项目覆盖率（Frappe 目标：diff 中 85% 行覆盖）；测试应覆盖最常用的业务场景。
62. 每个修复都要回归测试：没有能捕获该 bug 的测试的修复会招致回归；对极端后果（有状态/合规）代码，超越示例——用基于属性的测试（Hypothesis）。
63. 标记缺失的迁移/数据补丁覆盖：schema 变更与数据补丁是最高风险、最少测试的区域；改变字段或迁移数据的改动需要针对真实有数据站点测试的补丁（空表总是“迁移成功”，即使改动无效）。
64. 测试必须确定且独立：不用 `random`（易抖动）；不依赖其他测试留下的状态（顺序依赖）；时间相关逻辑用 `freeze_time`。
65. 错误信息、日志与可观测性：错误信息质量是正当审查项；标题必须具体且可 Google（绝不用 "Message"/"Error"）；字段名按字段引用；说明“改了什么”：行、字段、前后值（`1 → 2`）；用户必须知道“qty 从多少变到多少？”。
66. 把失败暴露给受影响方——“邮件配置坏了，只有用户知道坏了才是用户的问题”。
67. 记录日志：保留 traceback/异常上下文（调试容易几个数量级）；记录破坏性/管理操作并带可归属身份（谁、何时、从哪），持久化在临时容器之外。

适用边界 · 注意事项

• 不适用于非 Frappe/ERPNext 项目的代码审查（文中大量建议依赖 Frappe 框架 API 与约定）。
• 不适用于纯文档、纯样式或与业务逻辑无关的琐碎改动。
• 不适用于没有代码审查流程、只做个人一次性脚本的场景。