docs: add per-module audit reports (18 modules)
Add static security/quality audit reports for all 18 Go service modules under module/, plus a consolidated index (docs/audit/README.md) with per-module statistics, top risks, cross-module systemic defects and a phased TODO list (T1-T19). No production code is modified.
This commit is contained in:
652
docs/audit/module-base-feedback.md
Normal file
652
docs/audit/module-base-feedback.md
Normal file
@@ -0,0 +1,652 @@
|
||||
# 审计报告:module/base/feedback
|
||||
|
||||
## 1. 模块概览
|
||||
|
||||
`base/feedback` 是用户反馈工单微服务,暴露一个 gRPC 服务 `feedback.Method`,共 6 个方法:`List / Get / Add / Modify / Delete / Remark`(`proto/feedback.proto:6-13`),数据落 PostgreSQL 三张表:`feedback_item`(主表)、`feedback_images`、`feedback_accessory`(`internal/models/feedback_item.go:33`、`feedback_images.go:24`、`feedback_accessory.go:25`)。
|
||||
|
||||
- 代码规模:非生成 Go 源码 12 个文件约 400 行(`cmd/main/main.go` 50 行、`internal/*` 7 个逻辑/模型/服务文件、`service/{dependencies,expose}.go`);`pb/` 生成代码 4 个文件(含 97KB 的 `pb/const.pb.go`);`test/` 仅 1 个 `.http` 样例 + 1 个整段注释的 `rpc.go`;**无任何 `*_test.go`**。
|
||||
- 对外形式:gRPC + grpc-gateway。`cmd/main/main.go:21-46` 走 SDK `service.New/Start`(独立进程);另一条路径是 `service.Expose`(`service/expose.go:18-26`),被 `pkgs/all/internal/service/feedback.go:9-20` 与 `pkgs/ecmall/internal/service/feedback.go:9-20` 注册进聚合进程。
|
||||
- 网关路由:proto 无 `google.api.http` 注解,生成的 gateway 路径是裸路径 `POST /feedback.Method/{Method}`(`pb/feedback.pb.gw.go:206-324,472-477`);聚合入口另有动态路径 `POST /rpc/feedback/Method/{Method}`(`wiki/api/04-feedback.md:9,25-30`、`pkgs/all/internal/server/server.go:61`)。
|
||||
- 依赖外部组件:PostgreSQL(gorm)、Redis、内存缓存、Etcd(`internal/impl/impl.go:22-31`),其中 Redis/内存缓存**模块内无任何读取点**(见问题 23)。
|
||||
- 配置:`etc/feedback.yaml`、`etc/feedback_{dev,test,prod}.yaml`,键名是 `Name/ListenOn/Dsn/Anonymous/Log/Prometheus/Telemetry`,与 SDK `conf.Base` 的 `Service/Port/BindIP/Databases/Cache` **完全不对应**(见问题 9)。
|
||||
- 鉴权模型(前置结论):聚合进程由宿主统一鉴权(`pkgs/all/internal/server/authorization.go:42-66` 的 unary 拦截器与 HTTP 中间件),`pkgs/all/etc/default_dev.yaml:24-43`、`pkgs/ecmall/etc/default_dev.yaml:24-43` 的匿名白名单**不含 `feedback.*`**,因此 6 个方法在聚合入口都要求 JWT;接口内部只有 `List/Add/Modify` 调用了 `service.ParseMetaCtx`(`list.go:19`、`add.go:21`、`modify.go:24`),且仅验签不校验归属。
|
||||
|
||||
## 2. 审计范围与方法
|
||||
|
||||
已读文件(全部非生成代码 + 配置 + proto + 文档):`README.md`(542 行)、`cmd/main/main.go`、`cmd/cli/main.go`、`internal/config/config.go`、`internal/impl/impl.go`、`internal/logic/method/{add,list,get,modify,delete,ref,remark}.go`、`internal/models/{feedback_item,feedback_images,feedback_accessory}.go`、`internal/server/{new,method_server}.go`、`service/{dependencies,expose}.go`、`test/{add.http,rpc/rpc.go}`、`proto/{feedback,const}.proto`、`etc/*.yaml`、`go.mod`、`wiki/api/04-feedback.md`、`.builds/etc/feedback_prod.yaml`。
|
||||
|
||||
生成代码只做接口一致性检查:`pb.MethodServer` 6 个方法与 `internal/server/method_server.go:18-39` 的转发一致;`pb.FeedbackItem`/`FeedbackImage`/`FeedbackAccessory` 字段与 `internal/logic/method/ref.go:13-45` 的转换基本对齐(发现的缺字段见问题 6);gateway 路由 `pattern_Method_*_0`(`pb/feedback.pb.gw.go:472-477`)与 README:175-180 一致。
|
||||
|
||||
为判定鉴权、GORM 语义与部署行为,另外读取了:`bsm-sdk/core/service/{meta.go,service.go,register.go}`、`conf/{new.go,types.go}`、`env/env.go`、`types/db.go`、`database/{new.go,sql/postgresql.go}`、`with/databases.go`、`crypto/token/jwt.go`、`pkgs/all/internal/{server/{server.go,authorization.go},service/feedback.go,impl/impl.go}`、`pkgs/all/etc/default_dev.yaml`、`pkgs/ecmall/internal/service/feedback.go`、`pkgs/ecmall/etc/default_dev.yaml`、`scripts/build-all-linux.sh`;以及 GORM v1.31.2 源码 `finisher_api.go`、`callbacks/update.go`、`callbacks/associations.go`、`callbacks/callbacks.go`、`statement.go`、`schema/schema.go`(用于确证 `Updates`/`Count`/关联保存的真实行为)。
|
||||
|
||||
静态检查(在本模块目录执行):`gofmt -l .` → 无输出(全部已格式化);`go vet ./...` → 退出码 0,无告警。两者作为基线门禁可保留(问题 26 的测试项另论)。
|
||||
|
||||
已核验为“无问题”的点(避免误判):全部 SQL 条件均使用 `?` 占位符,`Order("created_at desc")` 为常量,**未发现注入点**(`list.go:39-71`、`get.go:29`、`delete.go:26`、`remark.go:32`);业务代码中**未发现 `_ =` 忽略错误或未检查 err**(`add/list/get/modify/delete/remark` 每步都判 err);`Add` 的图片/附件插入依赖 GORM 关联保存,Postgres 驱动默认开启事务(`database/sql/postgresql.go:32-41` 未设 `SkipDefaultTransaction`),**Add 本身是原子的**(Modify 不是,见问题 7)。
|
||||
|
||||
未覆盖/无法验证:未运行服务、未连接真实 PostgreSQL/Redis/Etcd,未 dump 线上表结构;`feedback_images.identity` / `feedback_accessory.identity` 的唯一索引是否真实存在取决于线上 DDL(模型声明了 `uniqueIndex`,但仓库内**找不到任何建表/迁移脚本**,见问题 16),凡依赖该前提的结论均已标注「推测」;未逐行阅读 `pb/feedback.pb.go`(1310 行) 与 `pb/const.pb.go`;SDK(`bsm-sdk/core`)与聚合服务(`pkgs/all`、`pkgs/ecmall`)仅按需读取片段,其自身问题只在与本模块调用点相关处引用。
|
||||
|
||||
## 3. 问题清单
|
||||
|
||||
### P0
|
||||
|
||||
#### 1. `Get`/`Delete`/`Remark` 既无鉴权也无归属校验,任意登录用户可读取、篡改、删除他人反馈
|
||||
|
||||
- **位置**:`module/base/feedback/internal/logic/method/get.go:19-35`、`delete.go:19-29`、`remark.go:19-35`;对照 `list.go:19`、`add.go:21`、`modify.go:24`
|
||||
- **证据**:
|
||||
```go
|
||||
// get.go:19-29 —— 全程没有 ParseMetaCtx,也不比对 passport_identity
|
||||
func Get(ctx context.Context, in *pb.GetRequest) (reply *pb.GetReply, err error) {
|
||||
if in.GetIdentity() == "" { return nil, errcode.ErrInvalidArgument }
|
||||
...
|
||||
err = impl.DBService.Preload("Images").Where("identity = ?", in.GetIdentity()).First(record).Error
|
||||
// delete.go:26
|
||||
err = impl.DBService.Where("identity = ?", in.GetIdentity()).Delete(new(models.FeedbackItem)).Error
|
||||
// remark.go:32 —— 任意调用方直接指定 status,可把别人的工单标记为"已处理"
|
||||
err = impl.DBService.Where("identity = ?", in.GetIdentity()).Updates(record).Error
|
||||
```
|
||||
```go
|
||||
// 对照:list.go:19 / add.go:21 / modify.go:24 —— 只有这三个方法解析了 JWT
|
||||
auth, err := service.ParseMetaCtx(ctx, nil)
|
||||
```
|
||||
- **影响**:`identity` 是 36 位 UUID/ULID,但 `List` 会把它连同 email/phone 一起回给同机构用户,且日志/前端分享链接都可能泄露;拿到任意一个 identity 即可:①`Get` 读取他人反馈的 `email`/`phone`/`content`/`remark`(PII);②`Remark` 改写状态与备注,把他人工单标记「已处理」或写入误导性备注;③`Delete` 软删他人工单。聚合入口的宿主拦截器只保证「已登录」(`pkgs/all/internal/server/authorization.go:43-51` 只验 JWT,`default_dev.yaml:24-43` 白名单不含 feedback),**不等于授权**。在 `cmd/main` 独立部署下更严重:`internal/server/new.go:23` 用 `grpc.NewServer()` 未装配任何拦截器,`Get/Delete/Remark` 连登录都不需要(`List/Add/Modify` 仍有 `ParseMetaCtx`)。
|
||||
- **建议**:在 `Get/Modify/Delete/Remark` 统一先取 `auth`,并把查询条件写成 `Where("identity = ? AND (passport_identity = ? OR ?)", ...)`(或对管理端角色单独放行 `auth.Role`);把「当前用户只能操作自己的工单」固化成 `models` 层唯一的辅助函数(如 `OwnedQuery(ctx)`),禁止 logic 直接拼 `identity = ?`;同时给独立入口装配与聚合一致的拦截器。
|
||||
|
||||
#### 2. `Modify` 用新 UUID 覆盖 `identity`,改写业务主键,导致记录失联与子表脱钩
|
||||
|
||||
- **位置**:`internal/logic/method/modify.go:35-52,64,72,89`;GORM 语义见 `callbacks/update.go:274-304`
|
||||
- **证据**:
|
||||
```go
|
||||
// modify.go:35-38 —— 更新用的结构体里塞了一个全新 UUID
|
||||
record := &models.FeedbackItem{
|
||||
Std_IICUDS: types.Std_IICUDS{
|
||||
Identity: utils.UUID(),
|
||||
},
|
||||
...
|
||||
// modify.go:89 —— 结构化 Updates 会把所有非零、非主键字段写进 SET
|
||||
err = impl.DBService.Where("identity = ?", in.GetIdentity()).Updates(record).Error
|
||||
```
|
||||
```go
|
||||
// gorm v1.31.2 callbacks/update.go:276-295 —— 只有主键被排除在 SET 之外
|
||||
for _, dbName := range stmt.Schema.DBNames {
|
||||
if field := updatingSchema.LookUpField(dbName); field != nil {
|
||||
if !field.PrimaryKey || ... {
|
||||
...
|
||||
if (ok || !isZero) && field.Updatable {
|
||||
set = append(set, clause.Assignment{Column: clause.Column{Name: field.DBName}, Value: value})
|
||||
```
|
||||
- **影响**:`FeedbackItem` 的主键是 `id`(`bsm-sdk/core/types/db.go:29`),`identity` 只是 `uniqueIndex` 列,因此被判为可更新字段;GORM 实际生成 `UPDATE feedback_item SET identity='<新UUID>', passport_identity=..., ... WHERE identity='<旧identity>'`。本条属于**必然发生的语义性数据损坏**:一次成功的 `Modify` 之后,原 identity 再也查不到该工单(`Get`/`Delete`/`Remark`/`List` 全部按 identity 查询),而 `modify.go:64,72` 写入的 `item_identity` 仍是调用方传入的旧 identity(关联保存路径又会用 `record` 的新 identity 覆盖它,见问题 7),子表与主表就此脱钩,形成无法通过 API 追溯的孤儿数据。仓库中 `.builds/_tmp_gorm_probe/main.go:34-41` 正是为验证「struct Updates 会写入哪些列」而留的探针,说明该行为已被怀疑过但未修复。
|
||||
- **建议**:更新用结构体不要带 `Identity`(或删除该字段赋值),并把 `Updates` 改成 map/`Select` 显式列出可改字段(`title/content/category/status/remark/email/phone`);`identity` 应加 `gorm:"->"`(只读)或在业务上禁止写入;补一条单测断言 `Modify` 前后 `identity` 不变。
|
||||
|
||||
#### 3. 生产数据库账号口令与内网拓扑硬编码进仓库,并随构建产物一同发布
|
||||
|
||||
- **位置**:`module/base/feedback/etc/feedback_prod.yaml:4`、`.builds/etc/feedback_prod.yaml:4`、`scripts/build-all-linux.sh:95-102`
|
||||
- **证据**:
|
||||
```yaml
|
||||
# etc/feedback_prod.yaml:4(.builds/etc/feedback_prod.yaml:4 内容完全相同)
|
||||
Dsn: postgres://prod:MakeW2023~PROD@192.168.0.224:5432/scf?sslmode=disable&TimeZone=Asia/Shanghai
|
||||
# etc/feedback_prod.yaml:7,11,40
|
||||
Cache: redis://null:CHANGE_ME@192.168.0.43:6379/
|
||||
- 192.168.0.83:2379
|
||||
Endpoint: http://139.159.232.50:14268/api/traces
|
||||
```
|
||||
```bash
|
||||
# scripts/build-all-linux.sh:98-100 —— 构建时把该配置复制进产物目录
|
||||
config_file="${module_dir}/etc/${config_name}"
|
||||
[[ -f "${config_file}" ]] || continue
|
||||
cp -- "${config_file}" "${CONFIG_ROOT}/${config_name}"
|
||||
```
|
||||
- **影响**:一个看起来是真实凭据的 `prod:MakeW2023~PROD@192.168.0.224/scf` 明文提交在版本库中,同时暴露生产 DB 主机、Redis 主机(.43)、Etcd 主机(.83)、Jaeger 采集端(139.159.232.50,公网 IP)与库名 `scf`;`sslmode=disable` 意味着凭据与业务数据在链路上明文传输。任何能读到仓库或构建产物(`.builds/etc/`,`scripts/build-all-linux.sh:100` 自动复制)的人即可直连生产库。此外 `Cache: redis://null:CHANGE_ME@...` 的占位口令会被 `conf.NotNil` 当作合法值放行。
|
||||
- **建议**:立即轮换 `prod` 账号口令并清理 git 历史(`git filter-repo`),把 DSN/Redis/Etcd 全部改为环境变量或密钥管理注入;生产 DSN 使用 `sslmode=require` 与最小权限账号(仅 `feedback_*` 表 DML);启动时增加「拒绝 `CHANGE_ME`、拒绝 `sslmode=disable`」的 fail-fast 校验(`internal/config/config.go` 增加校验点)。
|
||||
|
||||
### P1
|
||||
|
||||
#### 4. `List` 的分页总数在第 2 页及以后恒为 0(`Find` 之后复用同一 Statement 调 `Count`)
|
||||
|
||||
- **位置**:`internal/logic/method/list.go:71`
|
||||
- **证据**:
|
||||
```go
|
||||
// list.go:71 —— Limit/Offset/Order 与 Find、Count 在同一次链式调用上
|
||||
if err := sess.Limit(int(in.GetSize())).Offset(int(offset)).Order("created_at desc").Find(&list).Count(&count).Error; err != nil {
|
||||
```
|
||||
```go
|
||||
// gorm v1.31.2 finisher_api.go:497-511 —— Count 只删除 ORDER BY,LIMIT/OFFSET 会被带进 count 语句
|
||||
if orderByClause, ok := db.Statement.Clauses["ORDER BY"]; ok {
|
||||
if _, ok := db.Statement.Clauses["GROUP BY"]; !ok {
|
||||
delete(tx.Statement.Clauses, "ORDER BY")
|
||||
...
|
||||
if _, ok := db.Statement.Clauses["GROUP BY"]; ok || tx.RowsAffected != 1 {
|
||||
*count = tx.RowsAffected
|
||||
```
|
||||
- **影响**:`Count` 生成的 SQL 是 `SELECT count(*) ... WHERE ... LIMIT 10 OFFSET 10`;聚合行只有 1 行,`OFFSET 10` 使其被跳过,`RowsAffected=0 != 1`,于是 `count` 被写成 0。**第 1 页总数正确、第 2 页起 `count=0`**,前端分页器无法翻页(或显示「共 0 条」),且 `ListReply.count` 是唯一的条目总数来源(`list.go:76-79`)。深分页本身也随 offset 增大而变慢(见问题 11)。
|
||||
- **建议**:拆成两条独立语句:先 `sess.Count(&count)`(不带 Limit/Offset),再 `sess.Limit(size).Offset(offset).Order("created_at desc").Find(&list)`;或改用 `Session(&gorm.Session{})` 派生新会话。补单测断言 `page=2` 时 `count` 等于真实总数。
|
||||
|
||||
#### 5. `List` 的 `user_name` 过滤引用了不存在的列 `username`,带该条件即 SQL 报错
|
||||
|
||||
- **位置**:`internal/logic/method/list.go:48-51`;列名定义 `internal/models/feedback_item.go:15`
|
||||
- **证据**:
|
||||
```go
|
||||
// list.go:50
|
||||
sess = sess.Where("username = ?", username)
|
||||
// feedback_item.go:15 —— 模型列名是 user_name(README:444 的建表语句同为 user_name)
|
||||
UserName string `gorm:"column:user_name;type:varchar(20);default:'';" json:"user_name"` // 用户名
|
||||
```
|
||||
- **影响**:`Where("username = ?")` 是原样 SQL 片段,PostgreSQL 会报 `column "username" does not exist`,请求以 500/`Unknown` 返回;即只要前端在列表页按用户名搜索就必然失败(除非线上表另有一个遗留 `username` 列,属**推测**,模型与 README 的 DDL 均不支持该前提)。同时 `email`/`phone`/`store_identity`/`user_identity` 四个筛选字段在 proto(`proto/feedback.proto:23-25`)与 wiki(`wiki/api/04-feedback.md:42-49`)中公开,但逻辑中完全没有实现,调用方会静默拿到未被过滤的全量结果——这比报错更危险。
|
||||
- **建议**:改为 `Where("user_name = ?", username)`;把未实现的筛选字段要么实现(email/phone 精确匹配)要么从 proto 与文档中删除;对 `/rpc` 动态接口增加「请求字段全量实现」的契约测试。
|
||||
|
||||
#### 6. `Add`/`Modify` 静默丢弃 `category`/`agency`/`store_identity`,响应又从不回传这些字段
|
||||
|
||||
- **位置**:`internal/logic/method/add.go:27-43`、`modify.go:35-52`、`ref.go:13-26`;proto 字段 `proto/feedback.proto:78,83,49-50`
|
||||
- **证据**:
|
||||
```go
|
||||
// add.go:35-42 —— AddRequest 有 category(10)/agency(15)/store_identity(14),构造时全部未赋值
|
||||
UserName: in.GetUserName(),
|
||||
Status: in.GetStatus(),
|
||||
Email: in.GetEmail(),
|
||||
Phone: in.GetPhone(),
|
||||
Title: in.GetTitle(),
|
||||
Content: in.GetContent(),
|
||||
// modify.go:43-51 同样没有 Agency / StoreIdentity
|
||||
// ref.go:14-26 —— 出参也不含 UserIdentity / Agency / StoreIdentity
|
||||
Identity: item.Identity, UserName: item.UserName, Email: item.Email, Phone: item.Phone,
|
||||
```
|
||||
- **影响**:`Add` 是全模块唯一的工单创建入口,但它写入的记录 `category`/`agency` 恒为 `''`,`store_identity` 在模型里根本没有对应列(`feedback_item.go:14-24` 无该字段)→ 提交方传的分类/机构/店铺信息**永久丢失**;连带 `List` 的 `category`(`list.go:60-63`)与 `agency`(`list.go:38-39`)过滤对新建数据恒不匹配,`wiki/api/04-feedback.md:45` 文档化的 `store_identity` 字段在整条链路上不存在。同理 `Get`/`List` 回包的 `user_identity`/`agency`/`store_identity`(proto 字段 2/16/15)永远是空串,管理端无法判断工单归谁、属于哪个机构。
|
||||
- **建议**:`Add`/`Modify` 补齐 `Category`/`Agency` 赋值并在模型中新增 `store_identity`(或从 proto 删除);`convert()` 补 `UserIdentity`/`Agency`/`StoreIdentity`;把 proto 字段与模型列的对应关系写成表驱动测试,避免再次静默丢字段。
|
||||
|
||||
#### 7. `Modify` 的关联写入重复、附件缺失 identity、且整段无事务
|
||||
|
||||
- **位置**:`internal/logic/method/modify.go:70-76,79-86,89-106`;GORM 关联回调 `callbacks/callbacks.go:72`、`callbacks/associations.go:189-259`、`statement.go:771`
|
||||
- **证据**:
|
||||
```go
|
||||
// modify.go:70-76 —— 附件循环没有生成 Identity(Add 中同位置有 utils.UUID())
|
||||
for _, v := range in.GetAccessories() {
|
||||
record.Accessories = append(record.Accessories, models.FeedbackAccessory{
|
||||
ItemIdentity: in.GetIdentity(), Title: v.Title, FilePath: v.FilePath,
|
||||
})
|
||||
}
|
||||
// modify.go:79-106 —— 先软删子表、再更新主表、最后显式 Create;三步之间无事务
|
||||
err = impl.DBService.Where("item_identity = ?", in.GetIdentity()).Delete(&models.FeedbackImage{}).Error
|
||||
...
|
||||
err = impl.DBService.Where("identity = ?", in.GetIdentity()).Updates(record).Error // 该语句自身还会保存关联
|
||||
...
|
||||
if len(record.Images) > 0 { err = impl.DBService.Create(&record.Images).Error }
|
||||
```
|
||||
```go
|
||||
// gorm v1.31.2 callbacks/callbacks.go:72 —— UPDATE 回调注册了 after-associations
|
||||
updateCallback.Register("gorm:save_after_associations", SaveAfterAssociations(false))
|
||||
// statement.go:771 —— 未显式 Select 时 restricted=false,HasMany 分支不会被跳过
|
||||
return results, !notRestricted && len(stmt.Selects) > 0
|
||||
```
|
||||
```go
|
||||
// bsm-sdk/core/types/db.go:30 —— identity 带 uniqueIndex,软删行仍占用该唯一值
|
||||
Identity string `gorm:"column:identity;type:varchar(36);uniqueIndex;" json:"identity"`
|
||||
```
|
||||
- **影响**:三处相互叠加的缺陷:①**附件 identity 为空串**,多个附件会写入同一空值(Add 中每个附件都有 `utils.UUID()`,此处遗漏,`add.go:57-65` 可对照);②`Updates(record)` 会触发 GORM 的 after-associations 回调,把 `record.Images`/`record.Accessories` 再插一遍(关联保存时还会用 `record` 的新 identity 覆盖 `item_identity`),随后 `modify.go:96,102` 又显式 `Create` 同一批 identity 的行——两条写入路径必然都发生,结果是重复行,或(若 `identity` 唯一索引存在)第二次插入报重复键(**哪种结果取决于线上 DDL,属推测;但"同一批子行被写两次"由上述回调链确定**);③**无事务**:软删子表成功后主表或重建失败,就留下「主表 identity 已被改写、图片已删、附件残缺」的中间态,且由于问题 2,事后连原记录都找不到,无法补偿。此外硬删除前的软删让旧行继续占用 identity,与 ①空 identity 共同构成唯一键冲突的高发路径。
|
||||
- **建议**:用 `impl.DBService.Transaction(func(tx *gorm.DB) error {...})` 包住「删子表 → 更新主表 → 重建子表」,所有语句改用 `tx`;附件补齐 `Identity: utils.UUID()`;更新主表时 `Omit(clause.Associations)` 或显式 `Select` 字段,避免关联被隐式再写;重建前先判断 `RowsAffected`。补测试:2 个附件的 Modify、缺省 identity 的图片、故意在第 3 步注入错误后断言回滚。
|
||||
|
||||
#### 8. `Get`/`List` 未预加载 `Accessories`,附件在响应中永远为空
|
||||
|
||||
- **位置**:`internal/logic/method/get.go:29`、`list.go:35`、`ref.go:38-45`
|
||||
- **证据**:
|
||||
```go
|
||||
// get.go:29 / list.go:35 —— 只 Preload 了 Images
|
||||
err = impl.DBService.Preload("Images").Where("identity = ?", in.GetIdentity()).First(record).Error
|
||||
sess := impl.DBService.Preload("Images")
|
||||
// ref.go:38-45 —— 却把 Accessories 一并转换后返回
|
||||
for _, accessory := range item.Accessories {
|
||||
reply.Accessories = append(reply.Accessories, &pb.FeedbackAccessory{...})
|
||||
```
|
||||
- **影响**:`FeedbackItem` 声明了 `Accessories` 关联(`models/feedback_item.go:24`),转换函数也依赖它,但两条查询路径都没有 `Preload("Accessories")`,GORM 不会自动填充 → `Get`/`List` 返回的 `accessories` 永远是空数组;用户提交的附件(`add.go:57-66` 确实写库)在详情与列表里不可见,只有直接查库才能看到。
|
||||
- **建议**:两处改为 `Preload("Images").Preload("Accessories")`;补一条集成测试,断言 `Add` 带附件后 `Get` 能取回同名附件。
|
||||
|
||||
#### 9. 独立部署入口(`cmd/main`)实际不可用:配置键与 SDK 不匹配导致启动即 fatal,且 `Mux` 恒为 nil
|
||||
|
||||
- **位置**:`module/base/feedback/etc/feedback_{dev,test,prod}.yaml:1-4`、`internal/config/config.go:29-44`、`internal/server/new.go:16,20-30`、`cmd/main/main.go:28,37`
|
||||
- **证据**:
|
||||
```yaml
|
||||
# etc/feedback_prod.yaml:1-4 —— 用的是旧键名;同目录 dev/test 文件结构相同
|
||||
Name: {ServiceKey}
|
||||
ListenOn: 0.0.0.0:12210
|
||||
Dsn: postgres://prod:...@192.168.0.224:5432/scf?...
|
||||
```
|
||||
```go
|
||||
// bsm-sdk/core/conf/types.go:7-21 —— Base 只有 Service/Port/Cache/SecretKey/BindIP/Addr/Log,没有 Name/ListenOn/Dsn
|
||||
type Base struct { Service string `yaml:"Service"`; Port string `yaml:"Port"`; ... }
|
||||
type DBConf struct { Driver string `yaml:"Driver"`; Source []string `yaml:"Source"` }
|
||||
// bsm-sdk/core/conf/new.go:35-36,58-60 —— 按 feedback_{mode}.yaml 读取,且强制要求出现 "Service:"
|
||||
cfp := fmt.Sprintf("%s_%s.yaml", strings.ToLower(srvKey), env.Runtime.Mode)
|
||||
if !strings.Contains(yamlString, "Service:") { log.Fatalln("ERROR: Service Not Nil", cfp) }
|
||||
// bsm-sdk/core/with/databases.go:13-15 —— Databases 为 nil 时直接 panic
|
||||
if cfg == nil || len(cfg.Source) == 0 { panic("No Database Source Found !") }
|
||||
```
|
||||
```go
|
||||
// internal/server/new.go:16,26-30 —— Mux 字段声明了但从未赋值
|
||||
Mux *gwRuntime.ServeMux
|
||||
srv := &Server{ Ctx: context.Background(), Grpc: grpcServ, grpcConns: map[string]*grpc.ClientConn{} }
|
||||
// cmd/main/main.go:37 —— 于是把 nil 交给 SDK 当网关 handler
|
||||
GatewayMux: s.Mux,
|
||||
```
|
||||
- **影响**:三份模块配置里的 `Name/ListenOn/Dsn/Anonymous(映射)/Log.ServiceName/Log.Mode/Prometheus/Telemetry` 都不是 `SrvConfig` 的字段(`internal/config/config.go:17-25`),`conf.New` 在读到 `feedback_prod.yaml` 后会因找不到 `"Service:"` 直接 `log.Fatalln`;即使补上 `Service:`,`Databases` 也仍是 nil,`with.Databases` 会 panic,且 `Spec.Port` 恒为空 → 走 `conf.CheckPort` 的随机端口分支(`conf/new.go:90-96`),与配置里写的 12210 完全脱节。同时网关路由只可能在 `service.Expose`(`service/expose.go:23`)里注册,`cmd/main` 从不调用它,`s.Mux` 永远是 nil,SDK `service.Gateway` 会把 nil handler 交给 `http.ListenAndServe`(`bsm-sdk/core/service/service.go:126`)→ 落到 `DefaultServeMux`,README:175-180 与 `wiki/api/04-feedback.md:11` 文档化的 HTTP 端点全部 404。其他模块(例:`module/base/ads/etc/ads_dev.yaml:1-7` 用 `Service:/Port:/Databases:`)证明这是本模块配置未随 SDK 升级的落后,而非 SDK 行为。`scripts/build-all-linux.sh:52-71,97-100` 仍在为每个模块构建独立二进制并发布这份配置,因此该路径是**随时会被启用的雷**。
|
||||
- **建议**:把 `etc/feedback_{dev,test,prod}.yaml` 重写为 `Service/Port/BindIP/Databases.Driver+Source/Cache/MicroService` 结构(对照 ads 或钱包模块),并删除 `Dsn/ListenOn/Name` 旧键;在 `internal/server.New()` 里 `Mux: gwRuntime.NewServeMux()` 并调用 `pb.RegisterMethodHandlerServer(srv.Ctx, srv.Mux, NewMethodServer())`,让 `cmd/main` 与 `service.Expose` 走同一条注册路径;补一条启动自检(配置解析 + `Databases != nil` + 网关路由探活)。
|
||||
|
||||
#### 10. `List` 的 `agency` 分支绕开用户隔离,可越权读取他人反馈
|
||||
|
||||
- **位置**:`internal/logic/method/list.go:37-45`
|
||||
- **证据**:
|
||||
```go
|
||||
// list.go:37-45 —— 传了 agency 就完全不再按 passport_identity 收窄
|
||||
if in.GetAgency() != "" {
|
||||
sess = sess.Where("agency = ?", in.GetAgency())
|
||||
} else {
|
||||
userIdentity := auth.Identity
|
||||
if userIdentity != "" {
|
||||
sess = sess.Where("passport_identity = ?", userIdentity)
|
||||
}
|
||||
}
|
||||
```
|
||||
- **影响**:`agency` 是调用方可控的普通请求字段(`proto/feedback.proto:26`),一旦传入就跳过 `passport_identity` 过滤,返回该机构下**所有用户**的工单(含 email/phone/content)。同样的 `else` 分支还有一个次生缺陷:当 `auth.Identity` 为空串时不加任何用户条件,等价于全表分页读取——`service.ParseMetaCtx` 只校验签名不校验 identity 非空(`bsm-sdk/core/service/meta.go:19-47`),服务间令牌或经特殊签发的令牌会命中该分支。需注意:本模块的 `Add`/`Modify` 都不写 `agency`(问题 6),因此「按 agency 越权」的**实际可利用性取决于库中是否已存在非空 agency 的历史数据**(属推测);但 `auth.Identity == ""` 走全表这条分支不依赖任何历史数据。
|
||||
- **建议**:把机构维度改成「先按身份收窄、再叠加机构过滤」:`sess.Where("passport_identity = ?", auth.Identity)`,管理端查询另开角色判定(`ParseMetaCtx(ctx, &service.ParseOptions{RoleValue: "admin"})`);`auth.Identity == ""` 时立即返回 `ErrPermissionDenied` 而不是放行全表;`agency` 应从客户端入参改为由服务端身份/机构配置推导。
|
||||
|
||||
#### 11. 关键索引缺失 + `OFFSET` 深分页:每次列表/详情都是一次子表全表扫描
|
||||
|
||||
- **位置**:`internal/models/feedback_images.go:13`、`feedback_accessory.go:13`、`feedback_item.go:22`、`list.go:35,71`
|
||||
- **证据**:
|
||||
```go
|
||||
// feedback_images.go:13 / feedback_accessory.go:13 —— item_identity 无 index 标签(对比 Std_Passport 的 passport_identity 有 Index)
|
||||
ItemIdentity string `gorm:"column:item_identity;type:varchar(36);default:'';" json:"item_identity"`
|
||||
// feedback_item.go:22 —— agency 无索引,却作为 List 的过滤列
|
||||
Agency string `gorm:"column:agency;type:varchar(255);default:'';" json:"agency"`
|
||||
// list.go:35,71 —— Preload 触发 `WHERE item_identity IN (...)`,排序列为 created_at(无索引)
|
||||
sess := impl.DBService.Preload("Images")
|
||||
... .Order("created_at desc").Find(&list) ...
|
||||
```
|
||||
- **影响**:`Preload("Images")` 生成的 `SELECT * FROM feedback_images WHERE item_identity IN (...) AND deleted_at IS NULL` 没有可用索引(模型未声明,且见问题 16 迁移不会执行),随子表增长退化为顺序扫描;`feedback_accessory` 同理(虽当前未被 Preload,见问题 8)。`ORDER BY created_at DESC` 也没有索引,`OFFSET` 越大越慢(问题 4 的第 N 页查询要扫描并丢弃 `(page-1)*size` 行)。叠加问题 1/10 的越权读,构成可被外部放大的资源消耗路径。README:320-325 声称「关键字段建立复合索引」「使用预加载减少 N+1 查询」,其中索引部分在模型里没有任何落点。
|
||||
- **建议**:为 `feedback_images(item_identity)`、`feedback_accessory(item_identity)`、`feedback_item(passport_identity, created_at)`、`feedback_item(agency)` 建索引并纳入可执行迁移;把 `OFFSET` 分页改为基于 `(created_at, id)` 的 keyset 分页;列表查询用 `WithContext` 加超时。
|
||||
|
||||
### P2
|
||||
|
||||
#### 12. 数据库调用完全没有 `context` 传递,也没有超时/重试
|
||||
|
||||
- **位置**:`internal/logic/method/{list,get,add,modify,delete,remark}.go`(全部数据访问)
|
||||
- **证据**:
|
||||
```go
|
||||
// get.go:29 —— ctx 只用于 ParseMetaCtx(且 get 连这步都没有),从未进入 DB 调用
|
||||
err = impl.DBService.Preload("Images").Where("identity = ?", in.GetIdentity()).First(record).Error
|
||||
// 全模块 grep 结果:无任何 WithContext 调用
|
||||
```
|
||||
- **影响**:`ctx` 参数在 `Get/Delete/Remark` 中甚至完全未使用(编译期不会报错),客户端断开或上游超时后 SQL 继续在服务端执行,慢查询会占满 `SqlOptionMaxOpenConns=64`(`bsm-sdk/core/vars/sql.go:8`)的连接池并拖垮整个聚合进程;gRPC 层也未设置服务端超时/`grpc.KeepaliveParams`(`internal/server/new.go:23` 用默认 `grpc.NewServer()`)。没有重试/熔断,也没有任何降级。
|
||||
- **建议**:统一改为 `impl.DBService.WithContext(ctx)`,在 gRPC 拦截器或 logic 入口设置 `context.WithTimeout`(如 3s);为读接口加只读重试(幂等);把连接池与超时参数显式传入 `with.Databases(cfg, &types.SqlOptions{...})`。
|
||||
|
||||
#### 13. 业务 panic 无恢复:独立入口无拦截器,聚合入口的 gRPC 路径也没有 recovery
|
||||
|
||||
- **位置**:`internal/server/new.go:20-30`、`pkgs/all/internal/server/server.go:32`、`service/dependencies.go:19-32`
|
||||
- **证据**:
|
||||
```go
|
||||
// internal/server/new.go:23 —— 没有任何 unary/stream 拦截器(鉴权/recover/日志/限流全缺)
|
||||
grpcServ = grpc.NewServer()
|
||||
// pkgs/all/internal/server/server.go:32 —— 聚合只装了鉴权拦截器,无 recovery
|
||||
grpcServer := grpc.NewServer(grpc.UnaryInterceptor(auth.unaryInterceptor))
|
||||
```
|
||||
- **影响**:`impl.DBService` 等全局依赖一旦为 nil(例如调用方使用 `service.Expose` 却未传 `Dependencies.DB`,见问题 18),`impl.DBService.Preload(...)` 会以 nil 接收者 panic;gRPC 服务端未装 `recovery` 拦截器时该 panic 会终止**整个进程**(聚合进程承载 14+ 模块)。聚合 HTTP 侧有 `gin.Recovery()`(`server.go:35`)与 `http.Server` 兜底,但 `grpc-gateway` 的 `RegisterMethodHandlerServer` 路径(`service/expose.go:23`)是进程内直调,最终仍落到未保护的 gRPC 方法实现上。
|
||||
- **建议**:为 gRPC 增加 recovery 拦截器(`grpc_recovery` 或自写,log + 返回 `codes.Internal`),并在 `Expose`/`NewImpl` 里对 DB 做非空校验(问题 18);关键逻辑入口加 `defer recover` 兜底并上报。
|
||||
|
||||
#### 14. 无结构化日志、无健康检查、无指标;数据库原始错误直接回给调用方
|
||||
|
||||
- **位置**:全部 logic(无 logger 调用)、`internal/server/new.go:33-37`、`internal/config/config.go:23`
|
||||
- **证据**:
|
||||
```go
|
||||
// modify.go:89-92 —— 原始 GORM/PG 错误未经包装直接上抛(gRPC 层会映射为 Unknown + SQL 文本)
|
||||
err = impl.DBService.Where("identity = ?", in.GetIdentity()).Updates(record).Error
|
||||
if err != nil { return nil, err }
|
||||
// internal/server/new.go:33-36 —— 只注册业务服务与 reflection,没有 health 服务
|
||||
pb.RegisterMethodServer(srv.Grpc, NewMethodServer())
|
||||
if standalone { reflection.Register(srv.Grpc) }
|
||||
```
|
||||
- **影响**:模块内没有任何日志语句,问题定位只能依赖宿主;`reflection` 在生产独立部署下无条件开启(`new.go:35-37`),可被匿名枚举服务与消息结构。唯一违反 NOT NULL 或重复键时会返回 `duplicate key value violates unique constraint "idx_feedback_images_identity"` 之类的原始 SQL 文本,向调用方泄露表名/索引名;配置里的 `APM`(`config.go:23`)与 `Prometheus/Telemetry`(prod yaml:32-42)从未被读取(问题 23),因此没有任何指标与链路数据。README:330-335 声称有 `/health` 与 `/metrics` 端点,仓库内无实现。
|
||||
- **建议**:接入 SDK logger(`printer`/`logger`),错误统一包装为 `errcode`(保留原始 err 于日志而非响应体);注册 `grpc_health_v1` 健康服务;reflection 改为配置开关且生产默认关闭;APM/Prometheus 要么接上要么从配置删除。
|
||||
|
||||
#### 15. 生产环境默认开启 GORM `Debug`,SQL 与 email/phone/正文明文写入日志(PII)
|
||||
|
||||
- **位置**:`internal/impl/impl.go:28`、`bsm-sdk/core/database/sql/postgresql.go:11-24,46-48`
|
||||
- **证据**:
|
||||
```go
|
||||
// internal/impl/impl.go:28 —— opts 传 nil
|
||||
DBService = with.Databases(config.Spec.Databases, nil)
|
||||
// bsm-sdk/core/database/sql/postgresql.go:13-20,46-48 —— 默认 Debug:true,随即 gormDb.Debug()
|
||||
options = &types.SqlOptions{ ..., IsAutoMigrate: false, LogStdout: false, Debug: true }
|
||||
if options.Debug { gormDb = gormDb.Debug() }
|
||||
```
|
||||
- **影响**:`Debug()` 会把每条 SQL 连参数一起打印(`Explain` 后的完整语句),而本模块的 SQL 参数包含 `email`、`phone`、反馈正文、附件 URL 与用户 identity;日志模式为文件且无脱敏钩子 → PII 落盘并可能被集中采集。聚合进程同样传 nil(`pkgs/all/internal/impl/impl.go:38`),影响面不止本模块。README:407-415 还指导用户 `grep ERROR logs/feedback.log`,但 `Log` 配置键(yaml 用 `Mode/Path/Stat`)与 SDK `LogConf`(`Name/Level/Dir/Endpoint/Console/File/Remote`)不匹配 → 日志级别/路径配置实际全部失效。
|
||||
- **建议**:生产显式传 `&types.SqlOptions{Debug:false, IsAutoMigrate:false}`(或按 `BSM_RuntimeMode` 切换);若必须留 SQL 日志,接入 `logger.ParamsFilter` 对 email/phone 做掩码;修正 `Log` 配置键。
|
||||
|
||||
#### 16. 迁移与建表不可执行:`MigrateTables` 登记形同虚设,仓库内无任何 DDL,README 的迁移命令不存在
|
||||
|
||||
- **位置**:`internal/models/{feedback_item.go:27-30,feedback_images.go:17-20,feedback_accessory.go:18-21}`、`internal/impl/impl.go:28`、`README.md:266-270`
|
||||
- **证据**:
|
||||
```go
|
||||
// feedback_item.go:27-30 —— 三个模型都把自己登记进全局迁移表
|
||||
func init() { database.MigrateTables = append(database.MigrateTables, &FeedbackItem{}) }
|
||||
// bsm-sdk/core/database/new.go:43-48 —— 但只有 IsAutoMigrate 为真才会执行
|
||||
if len(MigrateTables) > 0 && options.IsAutoMigrate { err = db.AutoMigrate(MigrateTables...) }
|
||||
// bsm-sdk/core/database/sql/postgresql.go:17 —— 默认 false,且 conf.DBConf 只有 Driver/Source,无法从 YAML 打开
|
||||
IsAutoMigrate: false,
|
||||
```
|
||||
- **影响**:本模块既没有 `migrations/` 或 `.sql` 文件(全仓库 grep `feedback_item` 无任何建表语句),也没有可执行迁移入口,`init()` 登记的唯一效果是让 `database.MigrateTables` 全局变量增长;README:268-269 教用户执行 `go run cmd/main/main.go migrate`,但 `cmd/main/main.go:21-49` 只有 `Run()/main()`,没有子命令解析,该命令必然是空跑。聚合模式下更甚:`pkgs/all`/`pkgs/ecmall` 只调 `applyDependencies` 注入共享 DB,从不调用本模块的 `impl.NewImpl()`,且聚合库是 `bsm_dev`(`pkgs/ecmall/etc/default_dev.yaml:14`)而本模块独立配置指向 `milu`(`etc/feedback_dev.yaml:4`)→ 上线前若无人手工建表,所有接口都会以 `relation "feedback_item" does not exist` 失败;表结构(尤其是 `deleted_at`、`identity` 唯一索引)与模型是否一致也无从校验。
|
||||
- **建议**:提交版本化 DDL(含三张表、`deleted_at`、上述索引),提供独立的 `cmd/migrate` 或启动可配的 `IsAutoMigrate` 开关,并把「聚合库必须含 feedback_* 表」写入部署检查清单;删除 README 中不存在的命令。
|
||||
|
||||
#### 17. 优雅退出不可达:`Start()` 内 `select {}` 让 `defer srv.Stop()` 永不执行,且无信号处理
|
||||
|
||||
- **位置**:`cmd/main/main.go:41-45`、`bsm-sdk/core/service/service.go:98-115,141-144`
|
||||
- **证据**:
|
||||
```go
|
||||
// cmd/main/main.go:41-45 —— 期望 defer 在退出时优雅停止
|
||||
defer srv.Stop()
|
||||
srv.Start()
|
||||
// bsm-sdk/core/service/service.go:113-115 —— Start 内部永久阻塞,defer 永远不触发
|
||||
// 阻塞主线程
|
||||
select {}
|
||||
```
|
||||
- **影响**:独立部署的进程只能被强杀(`SIGKILL`)或依赖运行时默认的 `SIGTERM` 立即终止行为,在途 gRPC 请求被截断,`GrpcSrv.GracefulStop()`(`service.go:142-144`)形同虚设;模块内也没有 `signal.Notify` + `s.http.Shutdown` 的关停编排(对比聚合侧 `pkgs/all/internal/server/server.go:92-110` 有 `Stop(ctx)`)。同时 DB/Redis/Etcd 连接没有关闭点。
|
||||
- **建议**:模块自行实现信号处理(`signal.NotifyContext`)并在收到信号后按序 `GracefulStop` → 关闭 DB/Redis/Etcd,设置关停超时;或在 SDK `Start()` 里改为阻塞在可取消的 channel 上,让 `defer Stop()` 生效。
|
||||
|
||||
#### 18. `Expose`/依赖注入不校验 nil,缺依赖时是运行期 panic 而不是启动失败
|
||||
|
||||
- **位置**:`service/dependencies.go:19-32`、`service/expose.go:18-26`
|
||||
- **证据**:
|
||||
```go
|
||||
// dependencies.go:19-31 —— 只在非 nil 时覆盖,没有任何「必填」校验
|
||||
func applyDependencies(deps Dependencies) {
|
||||
if deps.DB != nil { impl.DBService = deps.DB }
|
||||
...
|
||||
// expose.go:20-23 —— GRPC/Gateway 直接透传,nil 会在 Handle/注册阶段炸
|
||||
server.New(options.GRPC)
|
||||
if err := pb.RegisterMethodHandlerServer(ctx, options.Gateway, server.NewMethodServer()); err != nil {
|
||||
```
|
||||
- **影响**:`Expose` 是聚合侧唯一入口,若调用方忘记传 `DB`(或 `GRPC`/`Gateway`),服务照常「注册成功」,直到第一个请求才 panic(`impl.DBService` 为 nil);`server.New` 的返回值被丢弃(`expose.go:20`),`Mux` 恒为 nil 的事实也无从暴露(问题 9)。与聚合侧 `newAuthorization` 的 fail-fast 风格(`pkgs/all/internal/server/authorization.go:25-40` 缺 key 直接返回错误)不一致。
|
||||
- **建议**:`Expose` 入口做 `if options.DB == nil || options.GRPC == nil || options.Gateway == nil { return fmt.Errorf(...) }`,并把 `server.New` 的 `*Server` 用于初始化 `Mux`/校验;`applyDependencies` 返回 error 而不是静默跳过。
|
||||
|
||||
#### 19. 写接口忽略 `RowsAffected`:对不存在的记录返回「成功」,且 `Updates(struct)` 无法清空字段
|
||||
|
||||
- **位置**:`internal/logic/method/delete.go:26-34`、`remark.go:32-40`、`modify.go:89-111`
|
||||
- **证据**:
|
||||
```go
|
||||
// delete.go:26-34 —— 不检查 RowsAffected 即返回 OK
|
||||
err = impl.DBService.Where("identity = ?", in.GetIdentity()).Delete(new(models.FeedbackItem)).Error
|
||||
if err != nil { return nil, err }
|
||||
return &pb.StatusReply{Message: vars.OK, Timeseq: time.Now().UnixMilli()}, nil
|
||||
// remark.go:26-32 —— Updates(struct) 忽略零值,也无法把 remark 清空为 ""
|
||||
record := &models.FeedbackItem{
|
||||
Remark: in.GetRemark(), // 备注内容
|
||||
Status: in.GetStatus(), // 状态
|
||||
}
|
||||
err = impl.DBService.Where("identity = ?", in.GetIdentity()).Updates(record).Error
|
||||
```
|
||||
- **影响**:传入任意不存在(或无权访问,见问题 1)的 identity,`Delete`/`Remark`/`Modify` 都返回 `code=0/OK`,调用方无法区分「删除成功」与「什么也没发生」,审计与告警全部失真;`Updates(struct)` 跳过零值字段意味着「清空备注」「把状态设回 0」在 API 层不可表达(前端会显示已保存但值未变)。
|
||||
- **建议**:统一检查 `result.RowsAffected == 0 → ErrRecordNotFound`;需要可空语义的字段改用 `map[string]any` 或 `Select` 显式指定列;把 `Status`/`Remark` 的可空写法与 proto 的 optional 语义对齐。
|
||||
|
||||
#### 20. 模型 `status` 同列双重定义且语义冲突,状态流转无任何校验
|
||||
|
||||
- **位置**:`internal/models/feedback_item.go:12,18`、`bsm-sdk/core/types/db.go:28-35`、`add.go:36`、`remark.go:26-32`
|
||||
- **证据**:
|
||||
```go
|
||||
// feedback_item.go:12,18 —— 嵌入式 Std_IICUDS 已带 Status int8(-1禁止/1正常),外层又定义 int32(1未处理/2已处理)
|
||||
type FeedbackItem struct {
|
||||
types.Std_IICUDS
|
||||
Status int32 `gorm:"column:status;default:1;" json:"status"` // 状态,1未处理,2已处理
|
||||
// bsm-sdk/core/types/db.go:34
|
||||
Status int8 `gorm:"column:status;default:0;index;" json:"status"` // 状态:默认为0,-1禁止,1为正常
|
||||
```
|
||||
- **影响**:GORM 按「同名列取绑定名更短者」的规则解析(`schema/schema.go:230-258`),外层 `int32 Status` 胜出,内层 `int8 Status`(连同它的 `index` 与 `-1/1` 语义)被覆盖——同一张表同一个 `status` 列被两套互斥的语义定义,任何写入方(`Add` 直取 `in.GetStatus()`、`Remark` 直取 `in.GetStatus()`)都没有状态机约束:调用方可以创建时就写 `status=2`(伪造「已处理」),也可以把已完成的工单改回 `1`,管理端无法据此做 SLA 统计。内层字段的 `index` 是否仍被 DDL 生成取决于阴影解析(属**推测**),但语义冲突确定存在。
|
||||
- **建议**:把业务状态从 `Std_IICUDS.Status` 中剥离为独立命名(如 `HandleStatus`,列 `handle_status`)或显式 `Omit` 内层字段;定义 `StatusPending=1/StatusHandled=2` 常量并集中校验合法迁移(1→2、2→1 需权限),禁止 `Add` 直接设置处理状态。
|
||||
|
||||
#### 21. 提交路径无限流、无幂等、无长度校验
|
||||
|
||||
- **位置**:`internal/logic/method/add.go:19-74`、`internal/models/feedback_item.go:15-21`
|
||||
- **证据**:
|
||||
```go
|
||||
// add.go:69-74 —— 无任何去重/限流/长度检查,直接落库并返回新 identity
|
||||
err = impl.DBService.Create(record).Error
|
||||
if err != nil { return nil, err }
|
||||
return &pb.AddReply{Identity: record.Identity}, nil
|
||||
// feedback_item.go:15,20 —— 列宽固定,超长输入由数据库报错
|
||||
UserName string `gorm:"column:user_name;type:varchar(20);default:'';" json:"user_name"` // 用户名
|
||||
Content string `gorm:"column:content;type:varchar(500);" json:"content"` // 内容
|
||||
```
|
||||
- **影响**:`Add` 是唯一的用户侧写入口,任何已登录用户都能以脚本无限提交(无 IP/用户维度限流、无频控、无验证码校验、无内容长度与敏感词校验);客户端重试或用户双击会产生两条内容相同的工单(identity 每次新生成,`add.go:29`),没有业务幂等键。`user_name(20)/title(255)/content(500)` 列宽限制未在应用层校验,超长直接由 PG 抛 `value too long` → 500,且 `Images.URL(255)`、`Accessories.FilePath(500)` 同理。
|
||||
- **建议**:按用户 + 时间窗做限流(Redis 计数器,本模块已有未使用的 `impl.RedisService`,见问题 23);约定客户端幂等键(如 `request_id`)并唯一约束去重;用 `protoc-gen-validate` 风格的长度/必填校验(title/content 非空、email/phone 格式与长度)在入口拒绝,而不是让 DB 报错。
|
||||
|
||||
#### 22. 图片 URL 与附件 `file_path` 完全信任调用方,无协议/域名/归属校验
|
||||
|
||||
- **位置**:`internal/logic/method/add.go:52,64`、`modify.go:65,74`、`test/add.http:11-16`
|
||||
- **证据**:
|
||||
```go
|
||||
// add.go:52,64 —— 客户端给什么就存什么
|
||||
URL: v.GetUrl(), // 图片URL
|
||||
FilePath: v.FilePath, // 附件文件路径
|
||||
// test/add.http:14 —— 官方样例直接使用内网下载地址
|
||||
"file_path": "http://127.0.0.1:22210/file/download/1"
|
||||
```
|
||||
- **影响**:服务端不校验 `javascript:`/`data:` 等危险协议,也不校验域名归属(可指向内网地址或他人上传的文件 ID,样例里就是 `http://127.0.0.1:22210/file/download/1`)。当管理端把 `url`/`file_path` 直接渲染为 `<a href>`/`<img src>` 或由服务端代理下载时,会形成存储型 XSS / SSRF / 越权取文件的组合风险(最终可利用性取决于前端与文件服务实现,属**推测**)。附件与工单之间也没有绑定校验,可以引用不属于自己的文件记录。
|
||||
- **建议**:只接受 `https://`(或本服务文件网关域名)并做主机白名单;图片/附件改为「先由文件服务上传获得 token,再用 token 换取 URL」的流程,服务端校验归属;渲染侧统一做协议过滤。
|
||||
|
||||
#### 23. 缓存依赖初始化后无任何读取点,配置项静默失效(含 Redis/内存缓存/Log/Prometheus/Telemetry)
|
||||
|
||||
- **位置**:`internal/impl/impl.go:22-31`、`service/dependencies.go:19-32`、`etc/feedback_prod.yaml:25-42`、`README.md:310-335`
|
||||
- **证据**:
|
||||
```go
|
||||
// impl.go:24-26 —— 初始化了 Redis 与内存缓存
|
||||
MemorySerice = with.Memory(nil)
|
||||
RedisService = with.RedisCache(config.Spec.Cache)
|
||||
// 全模块 grep:impl.RedisService / impl.MemorySerice 只有赋值,没有任何读取点
|
||||
```
|
||||
```yaml
|
||||
# etc/feedback_prod.yaml:25-29 —— 键名与 SDK conf.LogConf(Name/Level/Dir/Console/File/Remote) 完全不同
|
||||
Log:
|
||||
ServiceName: {ServiceKey}
|
||||
Mode: file
|
||||
Path: logs/{ServiceKey}
|
||||
```
|
||||
- **影响**:README:312-318 宣称「反馈列表缓存 5 分钟、详情 10 分钟、用户信息 30 分钟」,代码里没有任何缓存读写(`List`/`Get` 每次都打 DB,见问题 11),Redis 与内存缓存对象纯属死依赖,同时给聚合侧造成「已注入缓存」的错觉(`service/dependencies.go:29-31`);`Log/Prometheus/Telemetry` 三个配置节对应的字段不在 `SrvConfig`(`config.go:17-25`)中,YAML 解析时被静默忽略——即生产配置里看起来开着的监控与日志其实没有生效。
|
||||
- **建议**:要么按 README 实现读缓存(列表按「身份+条件」、详情按 identity,写路径失效),要么删除初始化与文档承诺;把 `Log`/`Prometheus`/`Telemetry` 配置节改成 SDK 认可的结构(或新建对应字段)并加启动校验,避免「配置写了但不生效」。
|
||||
|
||||
#### 24. `Anonymous` 白名单内容指向本模块不存在的服务,与实现完全脱节
|
||||
|
||||
- **位置**:`etc/feedback_dev.yaml:16-23`、`feedback_test.yaml:16-23`、`feedback_prod.yaml:15-22`
|
||||
- **证据**:
|
||||
```yaml
|
||||
# etc/feedback_prod.yaml:15-22 —— 三份配置相同
|
||||
Anonymous:
|
||||
Key: anonymous.{Workspace}
|
||||
Urls:
|
||||
- feedback.Check.Hello
|
||||
- feedback.Check.Updates
|
||||
- feedback.Data.Configure
|
||||
- feedback.Data.Areas
|
||||
- feedback.Data.Tags
|
||||
```
|
||||
- **影响**:本模块只注册了 `feedback.Method`(`proto/feedback.proto:6`),`feedback.Check.*`/`feedback.Data.*` 在任何地方都不存在(全仓库 grep 无实现),因此这份白名单既不能命中任何真实方法,也向运维传递「本服务有匿名接口」的错误信息;同时该节是映射(`Key`/`Urls`),而 SDK 侧 `MicroServiceConf.Anonymous` 是 `[]string`(`bsm-sdk/core/conf/types.go:23-26`),格式也不兼容(若配置能解析到这一步会 unmarshal 失败)。真实生效的白名单在聚合配置里(`pkgs/all/etc/default_dev.yaml:24-43`,不含 feedback)。
|
||||
- **建议**:删除该节或改写为 SDK 结构并只列真实需要匿名的 `feedback.Method/*`(按业务确认,例如反馈提交是否允许匿名);在文档中明确「匿名策略的唯一来源是聚合 `Authorization.Anonymous`」,避免两处声明打架。
|
||||
|
||||
#### 25. 分层与可测试性缺陷:logic 直接依赖全局可变单例,无法注入假 DB
|
||||
|
||||
- **位置**:`internal/logic/method/*.go`(全部形如 `impl.DBService.`)、`internal/impl/impl.go:13-18`
|
||||
- **证据**:
|
||||
```go
|
||||
// get.go:29 / list.go:35 / delete.go:26 / remark.go:32 —— 直接使用包级全局变量
|
||||
err = impl.DBService.Preload("Images").Where("identity = ?", in.GetIdentity()).First(record).Error
|
||||
// impl.go:16
|
||||
DBService *gorm.DB // 数据库服务
|
||||
```
|
||||
- **影响**:业务逻辑没有仓储接口/依赖注入点,唯一的数据访问入口是可变全局变量,导致:①无法在单测中替换 DB(与问题 26 的零测试互为因果);②`service.Expose` 与 `impl.NewImpl` 两条初始化路径会互相覆盖同一批全局变量(`dependencies.go:19-32`),并发测试或同进程多实例场景下行为不确定;③`Get/Delete/Remark` 三个方法连 `ctx` 都不传递(问题 12),说明入口签名与依赖注入一旦缺失就难以发现。
|
||||
- **建议**:定义最小仓储接口(`type FeedbackRepo interface{ List(ctx, ...); Get(ctx, identity); ... }`)并在 `Expose` 中注入实现,logic 只依赖接口;把 `impl.DBService` 降级为组合根内部状态。
|
||||
|
||||
#### 26. 测试覆盖为零(模块内无任何 `*_test.go`,`test/lint` 目录不存在)
|
||||
|
||||
- **位置**:`module/base/feedback/test/`(仅 `add.http`、`rpc/rpc.go`);`test/rpc/rpc.go:3-23`
|
||||
- **证据**:
|
||||
```go
|
||||
// test/rpc/rpc.go:3-23 —— 整个 main 函数体被注释,无法执行;package 名也不是 main
|
||||
package rpc
|
||||
|
||||
func main() {
|
||||
/*
|
||||
... grpc.Dial / c.List() ...
|
||||
*/
|
||||
}
|
||||
```
|
||||
- **影响**:`gofmt -l .` 与 `go vet ./...` 是本模块唯一的自动化门禁,问题 2(identity 被改写)、问题 4(count=0)、问题 5(列名错)、问题 7(附件 identity 为空)这类「必然发生」的缺陷全部能在单测里被拦住,但当前没有任何测试;`test/add.http` 是手工样例(且样例里的 `type` 字段在 proto 中不存在、JSON 尾随逗号非法,`test/add.http:9,16`)。
|
||||
- **建议**:至少补齐以下用例并纳入 CI(`go test ./...` + `gofmt -l` + `go vet`):
|
||||
1. `Add` 全字段落库(含 category/agency/附件),断言附件 identity 非空、`Get` 能取回附件(覆盖问题 6/8);
|
||||
2. `Modify` 前后 `identity` 不变、`item_identity` 指向正确、子表不重复(问题 2/7);
|
||||
3. 越权矩阵:A 用户对 B 用户的 identity 执行 Get/Modify/Delete/Remark 全部拒绝(问题 1/10);
|
||||
4. `List` 分页:`page=1/2/末页/超界` 的 `count` 与 `len(list)`;带 `user_name`/`status`/`category` 过滤(问题 4/5);
|
||||
5. 写接口对不存在 identity 返回 `ErrRecordNotFound`(问题 19);
|
||||
6. 状态常量与非法流转(问题 20)、超长字段与非法 URL 的入参校验(问题 21/22);
|
||||
7. 网关契约:`POST /feedback.Method/Add` 与 `/rpc/feedback/Method/Add` 的 200/401 期望值(问题 9),以及 `Expose` 传 nil 依赖时返回错误而非 panic(问题 18)。
|
||||
|
||||
### P3
|
||||
|
||||
#### 27. README 大量内容与实现不符(端口、目录、命令、缓存、健康检查、Docker/K8s)
|
||||
|
||||
- **位置**:`README.md:36,56-59,164-169,184,217-237,262-263,268-269,310-335,432-455`
|
||||
- **证据**:
|
||||
```markdown
|
||||
<!-- README:164-169 表格标注端口 12101;README:184 swagger 在 12102 -->
|
||||
| Method | Add | 添加反馈 | 12101 |
|
||||
- **本地**: http://localhost:12102/feedback.swagger.json
|
||||
<!-- README:217-221 -->
|
||||
# 运行所有测试
|
||||
go test ./...
|
||||
# README:269
|
||||
go run cmd/main/main.go migrate
|
||||
```
|
||||
- **影响**:实际配置端口是 12210(`etc/feedback_prod.yaml:2`)、聚合入口是 12000/12001(`pkgs/all/etc/default_dev.yaml:5,8`);`swagger/`、`scripts/`、`Makefile`、`Dockerfile`、`docker-compose.yml`、`k8s-deployment.yaml`、`.githooks` 全部不存在(模块根目录仅有 `cmd etc internal pb proto service test`);`go test ./...` 无测试可跑;`migrate` 子命令不存在(问题 16);README:436-455 的建表 DDL 与模型不一致(缺 `deleted_at`,`passport_id` 类型、索引均未提及)。README:19 宣称「完善的安全: 输入验证」,与问题 1-3、21-22 直接矛盾。
|
||||
- **建议**:按代码实际情况重写 README(端口/路径/命令/目录/DDL),并删掉所有未实现能力的宣称;把 `wiki/api/04-feedback.md` 与 README 的接口表合并为单一份生成文档。
|
||||
|
||||
#### 28. `etc/feedback.yaml` 是死配置;三份环境配置雷同且与 `.builds` 重复
|
||||
|
||||
- **位置**:`etc/feedback.yaml:1-6`、`etc/feedback_{dev,test,prod}.yaml`、`.builds/etc/feedback_prod.yaml`
|
||||
- **证据**:
|
||||
```yaml
|
||||
# etc/feedback.yaml:1-6 —— 文件名不含 _<mode>,conf.New 只读 feedback_{mode}.yaml
|
||||
Name: feedback.rpc
|
||||
ListenOn: 0.0.0.0:8080
|
||||
Etcd:
|
||||
Key: feedback.rpc
|
||||
```
|
||||
```go
|
||||
// bsm-sdk/core/conf/new.go:35
|
||||
cfp := fmt.Sprintf("%s_%s.yaml", strings.ToLower(srvKey), env.Runtime.Mode)
|
||||
```
|
||||
- **影响**:`feedback.yaml` 永远不会被加载(`BSM_RuntimeMode` 只会拼出 `feedback_dev/test/prod.yaml`),且其中的 `Key: feedback.rpc` 与其它文件声明的 `service.{Workspace}.{ServiceKey}.rpc` 命名规则互相矛盾;dev/test/prod 三份文件除 DSN 外几乎逐字相同(dev 与 test 的差异仅在 DB 主机/库名),prod 与 `.builds/etc/feedback_prod.yaml` 完全重复,任何配置修改都要同步多处,极易漏改(问题 3 的密钥正是这样泄漏的)。
|
||||
- **建议**:删除 `etc/feedback.yaml`;用模板 + 环境变量(`os.ExpandEnv` 已支持)生成各环境配置,密钥全部外置;`.builds/etc` 下不发密钥文件。
|
||||
|
||||
#### 29. 时间字符串不带时区;`ListRequest` 暴露了多个未实现字段
|
||||
|
||||
- **位置**:`internal/logic/method/ref.go:24-25`、`bsm-sdk/core/vars/time.go:6`、`proto/feedback.proto:19,23-25`
|
||||
- **证据**:
|
||||
```go
|
||||
// ref.go:24-25 —— 布局串不含时区
|
||||
CreatedAt: item.CreatedAt.Format(vars.YYYY_MM_DD_HH_MM_SS),
|
||||
// bsm-sdk/core/vars/time.go:6
|
||||
YYYY_MM_DD_HH_MM_SS string = "2006-01-02 15:04:05"
|
||||
```
|
||||
- **影响**:返回的时间是 `2024-01-01 10:00:00` 形式的裸本地时间,依赖 DSN 里的 `TimeZone=Asia/Shanghai` 与进程 TZ 一致才能正确解释(聚合库 DSN 也写了 Asia/Shanghai,属**推测**其一致);跨时区客户端或夏令时场景无法还原为绝对时刻,排序与展示也容易错位。同时 `ListRequest.user_identity/email/phone/store_identity`(proto 字段 3/7/8/9)在逻辑中完全未使用(问题 5),文档却把它们列为可用筛选条件。
|
||||
- **建议**:统一返回 RFC3339(带偏移)或明确 `_at` 字段为 UTC+8 并在文档标注;未实现的入参字段从 proto 删除或补齐实现。
|
||||
|
||||
#### 30. 死代码与空壳文件
|
||||
|
||||
- **位置**:`proto/const.proto` / `pb/const.pb.go`(97KB)、`cmd/cli/main.go`、`test/rpc/rpc.go`、`internal/server/new.go:17`
|
||||
- **证据**:
|
||||
```go
|
||||
// cmd/cli/main.go:5-7
|
||||
func main() { log.Println("Hello World!") }
|
||||
// internal/server/new.go:17 —— grpcConns 声明后从未使用(既非连接池也无任何读写)
|
||||
grpcConns map[string]*grpc.ClientConn // 连接池
|
||||
```
|
||||
- **影响**:`feedback.proto` 未 `import` 任何文件(`proto/feedback.proto:1-3`),`const.proto` 里全是 cms/order/social/market 等他域消息(`proto/const.proto:7-315`),本模块无任何引用(grep `pb.Empty|pb.StdIicuds|pb.IDRequest` 无命中),却被打包进每个使用者的二进制并出现在文档生成流程中;`cmd/cli` 是 Hello World,`test/rpc/rpc.go` 整段注释,`grpcConns` 是无用字段。
|
||||
- **建议**:删除 `proto/const.proto` 与 `pb/const.pb.go`、`cmd/cli`、`grpcConns`;`test/rpc/rpc.go` 要么恢复为可运行的冒烟脚本(`package main` + 真实请求)要么删除。
|
||||
|
||||
#### 31. 魔法数字、拼写与仓库外相对依赖
|
||||
|
||||
- **位置**:`list.go:29-31,55`、`add.go:36`、`internal/impl/impl.go:17`、`go.mod:69`
|
||||
- **证据**:
|
||||
```go
|
||||
// list.go:29-31,55 —— 10/50/0 为裸字面量
|
||||
if size < 1 || size > 50 { in.Size = 10 }
|
||||
if status != 0 { sess = sess.Where("status = ?", status) }
|
||||
// internal/impl/impl.go:17 —— 拼写少了 v,且该字段无读取点
|
||||
MemorySerice *cache.Cache // 内存缓存服务
|
||||
// go.mod:69
|
||||
replace git.apinb.com/bsm-sdk/core => ../../../../../bsm-sdk/core
|
||||
```
|
||||
- **影响**:分页上限、默认页大小、状态值散落在逻辑里,与 `models` 的注释(1 未处理/2 已处理)没有单一事实来源,修改时容易漏改;`MemorySerice` 拼写错误会在重命名/检索时造成困扰;`go.mod` 的 `replace` 指向仓库外相对路径(`D:\work\bsm-sdk\core`),意味着在干净环境中仅凭本仓库无法构建(构建脚本依赖外部 checkout + `go.work`),CI/新同事环境易踩坑。
|
||||
- **建议**:把分页与状态值提为常量(`const DefaultPageSize=10; MaxPageSize=50; StatusPending=1; StatusHandled=2`);统一改名为 `MemoryService`(同时改 `internal/impl` 与所有引用);依赖统一用 `go.work` 管理,`go.mod` 不保留穿透仓库的相对 `replace`。
|
||||
|
||||
#### 32. 代码注释与实现不符(误导后续维护者)
|
||||
|
||||
- **位置**:`internal/logic/method/delete.go:25`、`list.go:34,70`、`README.md:16,19,320`
|
||||
- **证据**:
|
||||
```go
|
||||
// delete.go:25 —— 事实是只软删主表,子表没有任何级联(模型无 OnDelete/约束)
|
||||
// 删除记录(GORM会自动处理关联的图片和附件删除)
|
||||
err = impl.DBService.Where("identity = ?", in.GetIdentity()).Delete(new(models.FeedbackItem)).Error
|
||||
// list.go:34 —— 声称预加载"图片信息",实际与 ref.go:38-45 的附件需求不匹配(问题 8)
|
||||
// 构建查询会话,预加载图片信息
|
||||
```
|
||||
- **影响**:`Delete` 之后 `feedback_images`/`feedback_accessory` 中的行仍然存在(只是主表被软删),与注释和 README:16「Redis 缓存 + 数据库优化」形成误导;维护者据此认为级联已处理,会放弃补偿清理,长期累积孤儿数据(在主表记录被软删后无法通过 API 发现)。
|
||||
- **建议**:修正注释并把级联删除显式实现(事务内软删子表,或改为硬删并加 `ON DELETE CASCADE` + 定期清理任务);同步修正 README 中所有与实现不符的能力描述。
|
||||
|
||||
## 4. 推荐优化方案
|
||||
|
||||
1. **授权闭环(目标:所有写读接口都基于身份做归属判定)**:做法:在 `Get/Modify/Delete/Remark` 统一 `service.ParseMetaCtx`,抽出 `ownedQuery(ctx)` 把 `passport_identity` 条件注入所有按 identity 的查询/更新/删除;管理端另设明确角色(`ParseOptions.RoleValue`);`List` 改为「先按身份收窄,再叠加机构/分类过滤」,`auth.Identity` 为空直接拒绝。影响面:`internal/logic/method/*.go`、可选新增 `internal/logic/authz`。风险:中;需与前端/管理后台确认角色与机构口径,避免误拦管理端。
|
||||
2. **修掉两处必然损坏(目标:Modify 不再改写主键,子表一次写入)**:做法:`Modify` 用 `map[string]any`/`Select` 更新白名单字段且不带 `Identity`;`Omit(clause.Associations)` 关闭隐式关联写;附件补 `utils.UUID()`;整段包 `Transaction`。影响面:`modify.go`、`add.go`(对照补齐 category/agency)、`ref.go`(回传缺字段)。风险:低-中;历史已被改写的 identity 需要用 `feedback_images.item_identity` 反查修复(一次性数据订正脚本)。
|
||||
3. **查询与分页正确性(目标:count 正确、过滤列正确、子表可查)**:做法:拆开 `Count` 与 `Find`;`username` → `user_name`;`Preload("Accessories")`;补齐 `email/phone` 过滤或删除字段。影响面:`list.go`、`get.go`。风险:低;属于可立即上线的修复。
|
||||
4. **索引与分页性能(目标:从全表扫描到索引扫描 + 稳定上限)**:做法:提交可执行 DDL(`feedback_images(item_identity)`、`feedback_accessory(item_identity)`、`feedback_item(passport_identity, created_at)`、`feedback_item(agency)`),`List` 走 keyset 分页 + `WithContext` 超时。影响面:新增 `migrations/`、`list.go`、`models/*`。风险:中;大表建索引需用 `CONCURRENTLY` 并在低峰执行。
|
||||
5. **密钥与配置治理(目标:仓库/产物内零凭据,配置真正生效)**:做法:轮换生产口令并清理 git 历史;DSN/Redis/Etcd 走环境变量或密钥管理;prod 使用 `sslmode=require` + 最小权限账号;`etc/*.yaml` 重写为 SDK 认可的 `Service/Port/Databases/Cache/MicroService/Log` 结构并加 fail-fast 校验(拒绝 `CHANGE_ME`/`sslmode=disable`);删除 `etc/feedback.yaml`。影响面:`etc/*`、`.builds/etc/*`、`scripts/build-all-linux.sh`、`internal/config/config.go`。风险:中;需与运维约定注入方式,避免上线即失败。
|
||||
6. **入口统一(目标:独立与聚合行为一致、网关不再 404)**:做法:`internal/server.New()` 内建 `Mux` 并注册 `RegisterMethodHandlerServer`,`cmd/main` 与 `service.Expose` 走同一函数;`Expose` 校验 nil 依赖并返回 error;独立入口装配与聚合一致的鉴权/recovery 拦截器。影响面:`internal/server/new.go`、`cmd/main/main.go`、`service/{expose,dependencies}.go`。风险:中;需回归两条部署路径的启动自检。
|
||||
7. **可观测性与生命周期(目标:故障可定位、可优雅重启)**:做法:接入 SDK logger 并对 email/phone 掩码、生产关闭 GORM `Debug`;注册 gRPC health、接上或删除 APM/Prometheus;实现信号处理 + 关停编排;reflection 改为配置开关。影响面:`internal/impl/impl.go`、`internal/logic/*`、`cmd/main/main.go`、`internal/server/new.go`。风险:低。
|
||||
8. **数据治理与状态机(目标:状态语义唯一、可审计)**:做法:把处理状态从 `Std_IICUDS.Status` 剥离命名,定义状态常量与合法迁移、记录处理人(新增 `handler_identity`/`handled_at`,目前包里没有任何处理人字段,`proto/feedback.proto:34-51` 与模型均无),`Add` 不允许设置处理状态;补迁移。影响面:`internal/models/*`、`proto/feedback.proto`、`logic/*`、`wiki/api/04-feedback.md` 重新生成。风险:中;涉及 DDL 与前端契约变更。
|
||||
9. **限流与防刷(目标:提交入口不可被滥用)**:做法:用已初始化但闲置的 Redis 做「用户+IP」维度的滑动窗口限流、幂等键去重(唯一约束)、入参长度与内容校验。影响面:`add.go`、新增 `internal/logic/limit`、`internal/impl`。风险:低-中;阈值需灰度确认。
|
||||
10. **测试与 CI 门禁(目标:上述修复可回归)**:做法:按问题 26 的清单补单测与网关集成测试,CI 跑 `go test ./...`、`gofmt -l .`、`go vet ./...`(后两者当前已通过,可直接作为基线)。影响面:`test/`、模块 CI。风险:低。
|
||||
|
||||
## 5. TODO 清单
|
||||
|
||||
- [ ] **P0-1** 在 `Get/Modify/Delete/Remark` 接入 `ParseMetaCtx` 并统一按 `passport_identity` 做归属过滤(管理端走显式角色)|验收:用户 A 用 B 的 identity 调这四个方法全部返回权限/未找到错误,且 A 的 `List` 只返回自己的工单|涉及:`module/base/feedback/internal/logic/method/get.go:19`, `delete.go:19`, `remark.go:19`, `modify.go:24`, `list.go:37`
|
||||
- [ ] **P0-2** `Modify` 更新语句不再写入 `identity`,改用 `Select`/map 白名单字段并 `Omit(clause.Associations)`|验收:单测断言 `Modify` 前后 `identity` 不变、无重复图片行、`item_identity` 始终指向当前 identity|涉及:`module/base/feedback/internal/logic/method/modify.go:35`, `modify.go:89`
|
||||
- [ ] **P0-3** 轮换生产 DB/Redis 口令、清理 git 历史中凭据,并把 DSN 改为环境变量注入 + `sslmode=require`|验收:`grep -rn "MakeW2023" .` 与构建产物中均无凭据;启动时若 DSN 含 `CHANGE_ME`/`sslmode=disable` 直接失败|涉及:`module/base/feedback/etc/feedback_prod.yaml:4`, `.builds/etc/feedback_prod.yaml:4`, `scripts/build-all-linux.sh:100`
|
||||
- [ ] **P1-4** `List` 拆分 `Count` 与 `Find` 两条语句|验收:`page=2` 返回的 `count` 等于真实总数(单测覆盖 page=1/2/末页/超界)|涉及:`module/base/feedback/internal/logic/method/list.go:71`
|
||||
- [ ] **P1-5** 修正 `username` → `user_name`,并对未实现的筛选字段(email/phone/store_identity/user_identity)补实现或从 proto 删除|验收:带 `user_name` 的 `List` 正常返回 200 且结果正确;proto 中不存在「文档可见但未实现」的字段|涉及:`module/base/feedback/internal/logic/method/list.go:50`, `proto/feedback.proto:19`
|
||||
- [ ] **P1-6** `Add`/`Modify` 补齐 `category`/`agency`(及 `store_identity` 落列),`convert()` 回传 `user_identity`/`agency`/`store_identity`|验收:`Add` 后 `Get` 能读回同值 category/agency,`List` 按 category/agency 能筛到该记录|涉及:`module/base/feedback/internal/logic/method/add.go:35`, `modify.go:43`, `ref.go:14`
|
||||
- [ ] **P1-7** `Modify` 用事务包住「删子表→更新主表→重建子表」,附件补 `utils.UUID()`,并消除重复写入|验收:2 个附件的 `Modify` 成功且附件 identity 非空;注入第 3 步失败的测试断言全部回滚、无重复行/唯一键冲突|涉及:`module/base/feedback/internal/logic/method/modify.go:70`, `modify.go:79`, `modify.go:96`
|
||||
- [ ] **P1-8** `Get`/`List` 增加 `Preload("Accessories")`|验收:`Add` 带附件后 `Get`/`List` 返回同名附件|涉及:`module/base/feedback/internal/logic/method/get.go:29`, `list.go:35`
|
||||
- [ ] **P1-9** 统一入口注册路径:`internal/server.New()` 内建 `Mux` 并注册网关 handler,`cmd/main` 与 `service.Expose` 复用;重写 `etc/feedback_*.yaml` 为 SDK 结构|验收:独立启动后 `POST /feedback.Method/List`(带合法 JWT)返回 200 而非 404,且配置解析不再 fatal、端口与配置文件一致|涉及:`module/base/feedback/internal/server/new.go:16`, `cmd/main/main.go:37`, `module/base/feedback/etc/feedback_prod.yaml:1`
|
||||
- [ ] **P1-10** `List` 不再因 `agency` 跳过身份过滤;`auth.Identity` 为空时拒绝|验收:传入任意 `agency` 也只返回本人工单;空 identity 令牌返回权限错误|涉及:`module/base/feedback/internal/logic/method/list.go:38`
|
||||
- [ ] **P1-11** 提交三张表的索引 DDL 并把列表改为 keyset 分页|验收:`EXPLAIN` 显示 `feedback_images(item_identity)` 与 `feedback_item(passport_identity, created_at)` 走索引;第 100 页查询耗时与第 1 页同量级|涉及:`module/base/feedback/internal/models/feedback_images.go:13`, `feedback_accessory.go:13`, `feedback_item.go:22`
|
||||
- [ ] **P2-12** 所有 DB 调用改为 `WithContext(ctx)` 并加超时/重试|验收:客户端取消后查询在超时内中断(慢查询测试验证)|涉及:`module/base/feedback/internal/logic/method/list.go:71`, `get.go:29`, `modify.go:89`
|
||||
- [ ] **P2-13** gRPC 增加 recovery 拦截器并为 nil 依赖 fail-fast|验收:注入 panic 后进程存活且返回 `Internal`;`Expose` 传 nil DB/GRPC/Gateway 时返回明确错误|涉及:`module/base/feedback/internal/server/new.go:23`, `service/expose.go:20`, `service/dependencies.go:19`
|
||||
- [ ] **P2-14** 接入结构化日志与 gRPC health,错误包装不外泄 SQL 文本;reflection 改配置开关|验收:生产日志无 SQL 原文、`/health` 或 gRPC health 返回 SERVING、生产 `grpcurl list` 失败|涉及:`module/base/feedback/internal/server/new.go:35`, `module/base/feedback/internal/logic/method/modify.go:90`
|
||||
- [ ] **P2-15** 生产关闭 GORM `Debug`(显式传 `SqlOptions`),并对 email/phone 做日志掩码|验收:生产日志中不再出现完整 SQL 与手机号/邮箱明文|涉及:`module/base/feedback/internal/impl/impl.go:28`
|
||||
- [ ] **P2-16** 提供可执行迁移(三张表 + `deleted_at` + 索引),删除 README 中不存在的 `migrate` 命令或实现它|验收:从空库按仓库步骤执行后所有接口可用;`grep feedback_item` 能找到 DDL 文件|涉及:`module/base/feedback/internal/models/feedback_item.go:27`, `README.md:269`
|
||||
- [ ] **P2-17** 实现信号处理与真正的优雅退出(关停 gRPC/DB/Redis/Etcd)|验收:`SIGTERM` 后在途请求完成、进程 5s 内退出|涉及:`module/base/feedback/cmd/main/main.go:42`
|
||||
- [ ] **P2-18** `Expose`/`applyDependencies` 对 `DB/GRPC/Gateway/Cache` 做 nil 校验并返回错误,不再丢弃 `server.New` 返回值|验收:任一依赖为 nil 时聚合启动即失败并打印原因,而非首个请求 panic|涉及:`module/base/feedback/service/expose.go:20`, `service/expose.go:23`, `service/dependencies.go:19`
|
||||
- [ ] **P2-19** 三个写接口检查 `RowsAffected` 并在 0 行时返回 `ErrRecordNotFound`;备注/状态用显式列更新以支持清空|验收:对不存在 identity 的 Delete/Remark/Modify 返回未找到;可将 remark 置空|涉及:`module/base/feedback/internal/logic/method/delete.go:26`, `remark.go:32`, `modify.go:89`
|
||||
- [ ] **P2-20** 处理状态与 `Std_IICUDS.Status` 解耦,定义状态常量与合法迁移,`Add` 禁止直接设置处理状态|验收:`Add` 传 `status=2` 被拒或被忽略;非法流转被拒绝|涉及:`module/base/feedback/internal/models/feedback_item.go:18`, `module/base/feedback/internal/logic/method/add.go:36`
|
||||
- [ ] **P2-21** 提交入口加限流 + 幂等键 + 入参长度/格式校验|验收:超过阈值的提交被拒;重复提交同一幂等键只产生一条工单;超长 title/content 返回参数错误而非 500|涉及:`module/base/feedback/internal/logic/method/add.go:19`, `internal/models/feedback_item.go:15`
|
||||
- [ ] **P2-22** 校验图片 `url` 与附件 `file_path`(协议/主机白名单 + 归属)|验收:`javascript:`/内网地址/他人文件路径被拒绝|涉及:`module/base/feedback/internal/logic/method/add.go:52`, `add.go:64`
|
||||
- [ ] **P2-23** 缓存与配置治理:实现读缓存或在文档中删除承诺;`Log/Prometheus/Telemetry` 改 SDK 结构并加启动校验|验收:代码中明确区分为「已实现缓存」或「无缓存」,配置项被实际读取(可打印生效值)|涉及:`module/base/feedback/internal/impl/impl.go:24`, `etc/feedback_prod.yaml:25`
|
||||
- [ ] **P2-24** 删除或改写 `Anonymous` 白名单(当前指向不存在的 `feedback.Check/Data` 服务,且映射格式与 SDK `[]string` 不兼容)|验收:配置中只列真实需要匿名的方法,或该节被删除并在文档说明匿名策略唯一来源是聚合 `Authorization.Anonymous`|涉及:`module/base/feedback/etc/feedback_prod.yaml:15`, `module/base/feedback/etc/feedback_dev.yaml:16`
|
||||
- [ ] **P2-25** 引入仓储接口,logic 不再直接使用全局 `impl.DBService`|验收:单测可用假仓储替换 DB 并覆盖全部 6 个方法|涉及:`module/base/feedback/internal/logic/method/get.go:29`, `module/base/feedback/internal/impl/impl.go:16`
|
||||
- [ ] **P2-26** 按问题 26 的清单补齐测试并纳入 CI(`go test ./...` + `gofmt -l` + `go vet`)|验收:上述 7 类用例全部存在且通过;覆盖率纳入门禁|涉及:`module/base/feedback/test/rpc/rpc.go:3`, `module/base/feedback/test/add.http:9`
|
||||
- [ ] **P3-27** 按实现重写 README(端口/目录/命令/健康检查/缓存/DDL),删除不存在的 swagger/Makefile/Docker/K8s 描述|验收:README 中每条命令与路径都能在仓库中找到对应实现|涉及:`module/base/feedback/README.md:164`, `README.md:184`, `README.md:269`, `README.md:438`
|
||||
- [ ] **P3-28** 删除死配置 `etc/feedback.yaml`,并用模板 + 环境变量消除 dev/test/prod 与 `.builds/etc` 之间的重复|验收:`conf.New` 只会读取真实存在的 `feedback_{mode}.yaml`,配置变更只需改一处|涉及:`module/base/feedback/etc/feedback.yaml:1`, `module/base/feedback/etc/feedback_prod.yaml:1`, `scripts/build-all-linux.sh:97`
|
||||
- [ ] **P3-29** 时间返回带时区(RFC3339 或显式标注 UTC+8),并把 `ListRequest` 中未实现的筛选字段补实现或删除|验收:响应时间字符串含时区;proto 中不存在「文档可见但未实现」的字段|涉及:`module/base/feedback/internal/logic/method/ref.go:24`, `module/base/feedback/proto/feedback.proto:19`
|
||||
- [ ] **P3-30** 删除死代码:`proto/const.proto`/`pb/const.pb.go`(97KB)、`cmd/cli`、`internal/server/new.go:17` 的 `grpcConns`;`test/rpc/rpc.go` 恢复为可运行冒烟脚本或删除|验收:`grep -rn "const.pb\|grpcConns\|Hello World" module/base/feedback` 无残留业务引用|涉及:`module/base/feedback/proto/const.proto:7`, `module/base/feedback/internal/server/new.go:17`, `module/base/feedback/cmd/cli/main.go:6`
|
||||
- [ ] **P3-31** 提取分页/状态常量为具名常量、修正 `MemorySerice` 拼写、去掉 `go.mod` 中穿透仓库的相对 `replace`|验收:逻辑中无裸状态/分页字面量;`grep -rn "MemorySerice" module/base/feedback` 无命中;干净环境下可构建|涉及:`module/base/feedback/internal/logic/method/list.go:29`, `module/base/feedback/internal/impl/impl.go:17`, `module/base/feedback/go.mod:69`
|
||||
- [ ] **P3-32** 修正与实现不符的注释(`delete.go:25` 声称级联删除、`list.go:34` 预加载描述),并让行为与注释一致(事务内软删子表或改为级联硬删)|验收:`Delete` 后子表无孤儿行;注释与代码一致|涉及:`module/base/feedback/internal/logic/method/delete.go:25`, `module/base/feedback/internal/logic/method/list.go:34`
|
||||
|
||||
## 6. 审计摘要(供汇总使用)
|
||||
|
||||
- 问题数:P0=3 P1=8 P2=15 P3=6
|
||||
- 最高风险(一句话):`Get`/`Delete`/`Remark` 只验「已登录」不验归属(`get.go:19`、`delete.go:19`、`remark.go:19`,独立入口更是连登录都不需要),任意调用方可读取他人反馈的 email/phone、改写处理状态或删除他人工单;同时 `Modify` 用新 UUID 覆盖 `identity`(`modify.go:37,89`)会**必然**让工单失联、子表脱钩,两者叠加使数据既被越权改动又无法追溯修复。
|
||||
- 最优先 3 个动作:1) 给 `Get/Modify/Delete/Remark` 加身份与归属校验(P0-1);2) 修掉 `Modify` 覆盖 `identity` 与关联双写/无事务(P0-2、P1-7);3) 轮换并外置生产库口令、清理仓库与 `.builds` 产物中的凭据(P0-3);紧随其后是 `Count` 置 0(P1-4)与 `username` 列名错误(P1-5)这类可立即上线的小修。
|
||||
- 未能覆盖/无法验证的部分:未运行服务、未连接真实 PostgreSQL/Redis/Etcd,未 dump 线上表结构,因此「`feedback_images.identity` 唯一索引是否存在」「线上 `feedback_item` 是否有遗留 `username` 列」「`ORDER BY created_at` 是否走索引」均为静态推断(依赖该前提的结论已在正文标注「推测」);`pb/feedback.pb.go`(1310 行) 与 `pb/const.pb.go` 未逐行阅读(仅做接口与消息一致性比对);未审计 CI/发布流水线与线上真实配置是否被环境变量或 etcd 覆盖;SDK(`bsm-sdk/core`)与聚合服务(`pkgs/all`、`pkgs/ecmall`)仅按需读取相关片段,其中 SDK 侧问题(`with.Databases` 默认 `Debug:true`/`IsAutoMigrate:false`、`service.Start()` 用 `select {}` 阻塞、`env` 硬编码兜底 JWT 密钥 `Cblocksmesh2022C`)归属 SDK 仓库,本报告仅在与本模块调用点相关处引用。
|
||||
Reference in New Issue
Block a user