golang-code-review

name	golang-code-review
description	对 Golang 项目进行代码安全评审。触发场景：用户想评审 Go 代码改动、检查 PR/MR 安全问题、review golang 代码、分析 gorm SQL 改动、检查 panic/error/越界风险。工作流：基于 git diff（当前分支 vs master）获取改动代码，结合上下文分析安全隐患，并列出所有 GORM SQL 改动及其原始 SQL。只要用户提到"review golang"、"评审 go 代码"、"检查 go 改动"、"gorm sql 分析"等，必须触发本 skill。

Golang Code Review Skill

概述

本 skill 用于对 Golang 项目做安全评审，聚焦以下三类问题：

1. 安全风险：panic、未判断 error、数组越界、nil 指针解引用
2. GORM SQL 改动：提取所有使用 GORM 框架的 SQL 操作，还原原始 SQL 语句

---

执行步骤

Step 1：分支来源检查

在做代码评审之前，先验证当前分支的来源是否符合规范。

# 获取当前分支名
git branch --show-current

# 检查当前分支是否从 master 切出（找到与 master 的分叉点）
git merge-base HEAD master

# 查看从分叉点到现在的所有 merge commit，检查是否有 develop 分支合入
git log $(git merge-base HEAD master)..HEAD --merges --oneline

# 查看完整提交历史（含合并），找出所有合入的分支名
git log $(git merge-base HEAD master)..HEAD --oneline --graph

判断逻辑：

1. 验证是否从 master 切出

# 找分叉点
FORK_POINT=$(git merge-base HEAD master)
# 检查分叉点是否在 master 的提交历史中
git branch --contains $FORK_POINT | grep -E '^\*?\s*master$'

- ✅ 分叉点存在于 master 历史 → 分支来源正常
- ❌ 分叉点不在 master 历史 → 说明不是从 master 切出，输出警告

2. 检查是否有 develop 分支代码合入

# 查找 merge commit 中来自 develop 的记录
git log $(git merge-base HEAD master)..HEAD --merges --oneline | grep -i develop

# 或通过引用日志确认
git log $(git merge-base HEAD master)..HEAD --oneline --graph | grep -i develop

- ✅ 无 develop 相关 merge commit → 分支干净
- ❌ 发现 develop 分支合入记录 → 输出高危警告，列出具体 merge commit

输出格式（在最终报告最前面展示）：

【分支来源检查】
当前分支: feature/xxx
分叉自:   master (commit: a1b2c3d)

✅ 来源正常：当前分支从 master 切出
❌ 警告：检测到 develop 分支代码合入！
   Merge commit: f3e2d1c  Merge branch 'develop' into feature/xxx
   合入时间: 2024-01-15 14:32
   风险说明: develop 分支可能包含未经测试或未上线的功能，混入当前分支可能导致意外代码上线

如果 develop 分支有代码合入，应在评审报告顶部用 🔴 标注，提醒评审人重点关注。

Step 2：获取 Git Diff

# 获取当前分支与 master 的完整 diff（含上下文行）
git diff master...HEAD -- '*.go'

# 如果 master 不存在，尝试 main
git diff main...HEAD -- '*.go'

# 获取变更的文件列表
git diff --name-only master...HEAD -- '*.go'

如果仓库没有 master/main，改用：

git log --oneline -1  # 确认 HEAD
git diff HEAD~1 HEAD -- '*.go'

Step 3：获取上下文代码

对于 diff 中涉及的每个文件，获取完整文件内容以理解上下文：

# 获取变更文件的完整内容（当前版本）
git show HEAD:<filepath>

# 或直接读取工作区文件
cat <filepath>

重点关注：

• diff 所在函数的完整实现
• 被修改代码调用的函数签名
• 结构体定义（判断字段是否可为 nil）
• 错误处理链路

读取文件时使用带行号的命令，为后续行号定位做准备：

# 带行号读取，方便后续 grep 验证
cat -n <filepath>

# 或只看关键函数所在区域
grep -n "func \|\.Where\|\.Find\|\.Create\|\.Update\|\.Delete" <filepath>

Step 4：安全问题分析

逐一检查以下风险类别：

3.1 Panic 风险

场景	检查点
nil 指针解引用	指针/接口在使用前是否判空
类型断言	x.(T) 是否使用双返回值 v, ok := x.(T)
map 并发读写	是否有并发场景未加锁
channel 操作	向关闭的 channel 发送数据
空接口转换	interface{} 断言未处理 ok

3.2 未判断 Error

场景	检查点
函数返回 error	_, err 中 err 是否被 if err != nil 检查
GORM 操作	.Error 字段是否被检查
文件操作	os.Open/Read/Write 的 error
JSON 解析	json.Unmarshal 的 error
类型转换	strconv 系列函数的 error

3.3 数组/切片越界

场景	检查点
直接下标访问	arr[i] 前是否判断 i < len(arr)
切片操作	s[a:b] 中 b 是否可能超出 len
空切片取第一个	s[0] 前是否判断 len(s) > 0
for range 后取值	循环结束后直接取值可能越界

4.4 其他常见问题

• goroutine 泄露（context 未传递或未取消）
• 锁未释放（defer mu.Unlock() 是否正确）
• 资源未关闭（rows.Close()、file.Close()）

Step 5：提取 GORM SQL 改动

识别所有 GORM 操作并还原原始 SQL。

5.1 识别 GORM 操作模式

// 查询类
db.Where(...).Find(&result)
db.Where(...).First(&result)
db.Raw("SELECT ...").Scan(&result)
db.Table("...").Select("...").Where(...).Find(...)

// 写入类
db.Create(&obj)
db.Save(&obj)
db.Model(&obj).Update(field, value)
db.Model(&obj).Updates(map/struct)

// 删除类
db.Where(...).Delete(&obj)
db.Unscoped().Where(...).Delete(&obj)

// 执行类
db.Exec("UPDATE ...")

5.2 SQL 还原规则

GORM 代码	对应 SQL
.Where("name = ?", "tom")	WHERE name = 'tom'
.Where("age > ? AND status = ?", 18, 1)	WHERE age > 18 AND status = 1
.Where(User{Name: "tom"})	WHERE name = 'tom'
.Find(&users)	SELECT * FROM users
.Select("id, name")	SELECT id, name FROM ...
.Limit(10).Offset(20)	LIMIT 10 OFFSET 20
.Order("created_at DESC")	ORDER BY created_at DESC
.Joins("LEFT JOIN orders ON ...")	LEFT JOIN orders ON ...
.Create(&user)	INSERT INTO users (...) VALUES (...)
.Save(&user)	INSERT ... ON DUPLICATE KEY UPDATE 或 UPDATE users SET ... WHERE id=?
.Updates(map)	UPDATE users SET col1=?, col2=? WHERE ...
.Delete(&user)	DELETE FROM users WHERE id=?（软删除：UPDATE users SET deleted_at=? WHERE id=?）
.Unscoped().Delete()	DELETE FROM users WHERE id=?（硬删除）

还原步骤：

1. 确定操作表名（Model/Table 参数或结构体名转 snake_case）
2. 拼接 SELECT 子句
3. 拼接 JOIN 子句
4. 拼接 WHERE 条件，将 ? 替换为实际值（如能推断）
5. 拼接 ORDER BY / LIMIT / OFFSET
6. 对写操作，列出所有被修改的字段

---

行号确认（重要）

git diff 输出的 @@ 行号是 diff 上下文行号，不能直接用于报告，必须通过当前分支的实际文件确认真实行号。

对每一处需要标注行号的问题代码，执行以下步骤：

# 用问题代码的关键字在当前文件中定位真实行号
grep -n "关键代码片段" <filepath>

# 如果关键字不唯一，结合函数名缩小范围
grep -n "func FunctionName" <filepath>

# 多行 GORM 链式调用，用链式起始行定位
grep -n "db\.Where\|db\.Model\|db\.Table" <filepath>

规则：

• 报告中的行号必须来自 grep -n 的实际输出，不得使用 diff 中的 @@ 行号
• 对于多行链式调用（如 GORM），行号指向该调用的第一行
• 若同一代码片段出现多次，结合函数上下文取正确的那一处

---

输出格式

一、安全风险清单

按严重程度排列（🔴 高危 / 🟡 中危 / 🟢 低危）：

🔴 [高危] 文件: internal/service/user.go, 行: 42
   问题: nil 指针解引用 —— user 可能为 nil，但直接访问 user.Name
   改动前: （原始代码）
   改动后: （新代码）
   建议: 在使用 user 前添加 if user == nil { return ... }

🟡 [中危] 文件: internal/repo/order.go, 行: 88
   问题: GORM 查询结果未判断 Error
   改动后: result := db.Where(...).Find(&orders)
   建议: 添加 if result.Error != nil { return result.Error }

二、GORM SQL 改动清单

【新增】internal/repo/order.go, 行: 55
GORM 代码:
  db.Where("user_id = ? AND status IN ?", userID, []int{1, 2}).
     Order("created_at DESC").
     Limit(10).
     Find(&orders)

原始 SQL:
  SELECT * FROM orders
  WHERE user_id = <userID> AND status IN (1, 2)
  ORDER BY created_at DESC
  LIMIT 10

备注: 如 Order 有 deleted_at 字段（软删除），实际 SQL 还会附加 AND deleted_at IS NULL

---

【修改】internal/repo/product.go, 行: 120
GORM 代码（改动前）:
  db.Where("id = ?", id).First(&product)

GORM 代码（改动后）:
  db.Where("id = ? AND shop_id = ?", id, shopID).First(&product)

原始 SQL（改动后）:
  SELECT * FROM products
  WHERE id = <id> AND shop_id = <shopID>
  ORDER BY id
  LIMIT 1

---

注意事项

• 若项目使用了 GORM v1（github.com/jinzhu/gorm）和 v2（gorm.io/gorm），软删除行为略有差异，请在备注中注明版本
• 对于 db.Raw() 和 db.Exec()，直接提取 SQL 字符串，无需转换
• 链式调用跨多行时，需完整拼接所有条件
• Scopes 函数（db.Scopes(ActiveUser)）需追踪 Scope 定义来还原条件