自动化代码的评审要看什么
自动化代码的特殊之处在于它的运行结果直接作用于生产系统,一次执行的影响范围可能覆盖数百台主机。因此评审的重点不是代码风格,而是「这段代码在最坏情况下能造成多大破坏」。围绕这个判断,关注点集中在几条线上:外部输入是否被拼接进命令、凭据是否硬编码、破坏性操作的作用范围是否被限定、以及环境变量与默认值是否可能把测试动作打到生产上。
基本信息
| 适用对象 | 剧本、部署脚本、运维工具与流水线配置 |
|---|---|
| 评审重点 | 影响范围与最坏后果,而非风格问题 |
| 高风险写法 | 命令拼接、通配删除、默认指向生产、凭据硬编码 |
| 验证手段 | 本地或测试环境先跑一次,观察实际作用范围 |
| 配套要求 | 评审意见需与变更记录一同留存 |
六类必看问题
- 命令拼接:外部数据直接进入命令字符串,特殊字符可被解释为额外命令
- 作用范围:删除、覆盖、重启类动作是否限定了主机与路径,是否带确认条件
- 默认指向:变量默认值是否指向生产环境,误用时缺少防呆
- 凭据处理:是否硬编码口令与密钥,是否把敏感值传给命令参数
- 权限使用:提权调用是否只覆盖必要命令,是否可被参数扩展成任意执行
- 异常处理:错误被吞掉继续执行时,破坏性动作可能在部分失败后仍继续放大
高风险与安全写法对照
高风险与安全写法对照
# 风险:外部输入直接拼进命令
rm -rf /data/$USER_INPUT/*
# 安全:白名单校验 + 限定路径 + 明确失败
case "$DATA_DIR" in /data/app1|/data/app2) ;; *) echo 'path not allowed' >&2; exit 1;; esac
rm -rf -- "/data/$DATA_DIR/"tmp/*
# 风险:凭据出现在命令行
mytool --password "$DB_PASS" deploy
# 安全:经标准输入或配置文件传递
mytool --password-stdin deploy < /run/secrets/db_pass
grep -nE 'rm -rf /|chmod 777|curl .*\| *(ba)?sh' playbook.yml # 自查高风险模式问题与后果
| 问题 | 最坏后果 | 评审动作 |
|---|---|---|
| 命令拼接 | 在受管主机上执行任意命令 | 要求改为参数化或白名单校验 |
| 通配删除 | 误删超出预期的目录 | 要求限定路径并加前置校验 |
| 默认指向生产 | 测试执行打到生产 | 要求默认指向测试并显式指定生产 |
| 凭据硬编码 | 仓库与日志同时泄漏 | 要求改用注入通道 |
| 异常被忽略 | 部分失败后继续扩大影响 | 要求失败即中止并回滚 |
评审时优先看「谁会被影响」而不是「代码是否正确」:同样的语法错误,在被管主机数为一台和一千台时代价完全不同。对涉及批量与破坏性动作的变更,可以要求提交者附上在测试环境的执行输出。
把远程脚本直接管道交给解释器执行,等于放弃了版本、校验与回滚能力,且每次执行的内容都可能不同。需要引入外部脚本时,应固定版本、校验完整性后再执行。