代码审查实战：提升代码质量与团队协作效率的关键技巧
Tr° 培静项目初期的技术选型
去年接了个内部管理后台的活，前端团队就我和另一个同事。需求看起来挺常规：用户权限分级、敏感操作留痕、前后端分离。但安全这块老板特别上心，说之前吃过 XSS 和 CSRF 的亏，这次必须把代码审查流程搞起来。
其实一开始我有点抵触——又不是开源项目，两个人开发还要搞 PR、Code Review？但后来想想，安全相关的逻辑一旦出错就是大事，比如删除数据、修改权限这些接口，写错一个判断条件可能就导致越权访问。所以最后决定，所有涉及权限、输入处理、API 调用的代码，必须走一轮人工审查。
工具选了 GitLab 自带的 MR（Merge Request）流程，加个简单的 checklist 模板，强制要求填写“是否涉及敏感操作”“是否做了输入校验”之类的字段。没上 SonarQube 那种重型工具，一是时间紧，二是小团队没必要。
最大的坑：误报太多，差点放弃
刚开始执行时，问题来了——同事提的 MR 里一堆“疑似 XSS”的警告，点进去一看，全是 innerHTML 用得稍微灵活一点就被标红。比如动态渲染用户头像链接：
// 错误示范（但当时觉得没问题）
const avatar = <img src="${user.avatar}" onerror="this.src='/default.png'">;
container.innerHTML = avatar;静态扫描工具直接报高危，说 user.avatar 可能被注入恶意脚本。但其实后端返回的 avatar 字段是经过白名单过滤的 CDN 地址，理论上安全。可审查的人（也就是我）每次都要手动确认，效率低到想骂人。
更烦的是，有些地方为了性能用了 dangerouslySetInnerHTML（React 项目），虽然加了注释说明“已转义”，但工具照样报警。折腾了几天，MR 积压，大家开始抱怨：“这流程是不是太重了？”
核心代码就这几行：自定义审查规则
后来我意识到，不能全靠工具，得结合业务写自己的校验逻辑。于是抽了两天时间，写了个简单的 ESLint 插件，专门针对我们项目里的高危模式做精准识别。
比如，我们约定：所有用户输入的数据，如果要插入 DOM，必须通过 sanitizeHTML 函数处理。那我就在插件里加了一条规则：只要检测到 innerHTML 赋值，且右侧表达式不包含 sanitizeHTML 调用，就报错。
// .eslintrc.js
module.exports = {
plugins: ['custom-security'],
rules: {
'custom-security/no-unsafe-innerhtml': 'error'
}
};对应的规则实现（简化版）：
// eslint-plugin-custom-security/rules/no-unsafe-innerhtml.js
module.exports = {
create(context) {
return {
AssignmentExpression(node) {
if (
node.left.type === 'MemberExpression' &&
node.left.property.name === 'innerHTML'
) {
const right = node.right;
// 简单判断右侧是否是 sanitizeHTML(...) 调用
if (
right.type !== 'CallExpression' ||
right.callee.name !== 'sanitizeHTML'
) {
context.report({
node,
message: 'innerHTML 必须使用 sanitizeHTML 处理'
});
}
}
}
};
}
};同时，我们封装了一个安全的 sanitizeHTML 函数，基于 DOMPurify，但加了项目特定的配置：
import DOMPurify from 'dompurify';
const purify = DOMPurify.sanitize;
// 允许 img 标签，但只允许特定域名
DOMPurify.addHook('afterSanitizeAttributes', function(node) {
if (node.tagName === 'IMG') {
const src = node.getAttribute('src');
if (src && !src.startsWith('https://cdn.jztheme.com/')) {
node.removeAttribute('src');
}
}
});
export const sanitizeHTML = (str) => {
return purify(str, { ALLOWED_TAGS: ['img', 'p', 'br'] });
};这样，只有明确调用 sanitizeHTML 的地方才能用 innerHTML，其他地方一提交就报错，根本过不了 lint。误报率直线下降，审查时我只需要看一眼有没有漏掉这个函数就行。
踩坑提醒：这三点一定注意
- 别过度依赖自动化：工具能拦住 80% 的低级错误，但剩下的 20% 需要人工判断。比如某个接口的参数虽然是用户输入，但只用于日志记录，不回显到页面，其实不用转义。这时候硬套规则反而麻烦。
- 审查清单要具体：一开始 checklist 写的是“检查安全性”，结果没人知道查什么。后来改成“确认所有 API 响应中的字符串字段是否可能被当作 HTML 渲染”，明确多了。
- 性能和安全的平衡：有次为了安全，给每个列表项都加了
sanitizeHTML，结果列表滚动卡顿。最后发现是重复调用导致的，改成只在数据加载时处理一次，缓存结果，性能才回来。
最终的解决方案
整套流程跑下来，效果还不错。上线半年，没再出现 XSS 问题（至少没被用户反馈）。CSRF 也通过审查确保每个 POST 请求都带了 token。最满意的是，现在新同事提 MR 时，会主动在描述里写：“已用 sanitizeHTML 处理用户内容，见第 45 行”。
不过也有没完全解决的问题：比如某些老页面还在用 jQuery 直接拼接 HTML，因为重构成本太高，暂时加了全局注释忽略 ESLint 规则。我知道这不完美，但优先级排在后面，影响不大。
另外，审查流程还是有点慢。有时候我忙，MR 要等一两天才 review。后来我们加了个 Slack 机器人，MR 一创建就 @ 我，响应速度提升不少。
回顾与反思
回头看，代码审查在安全场景下真的值。虽然前期折腾了点，但避免了后期更大的坑。而且，写自定义规则的过程让我对项目的安全边界更清楚了——哪些地方是雷区，哪些可以放松。
要说还能优化的地方，可能是引入更智能的上下文分析。比如，如果变量来自可信的 API（如 fetch('https://jztheme.com/api/user') 返回的固定结构），能不能自动标记为安全？不过目前还没找到成熟的方案，先这么用着吧。
总的来说，这套轻量级的审查机制，配合定制化的 lint 规则，对我们这种小团队够用了。不是最优解，但简单、有效、能落地。
以上是我踩坑后的总结，希望对你有帮助。如果你有更优的实现方式，比如怎么自动化处理那些“理论上安全但工具乱报”的情况，欢迎评论区交流！
暂无评论