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.
662 lines
56 KiB
Markdown
662 lines
56 KiB
Markdown
# 审计报告:module/ec/address
|
||
|
||
## 1. 模块概览
|
||
|
||
`module/ec/address` 是 BSM 电商域的「收货地址库」微服务,绝对路径 `D:\work\bsm-infra\full\module\ec\address`,gRPC(`Port: 12460`)+ gRPC-gateway(`Gateway: Enable: true, Port: 12459`)双协议对外暴露。
|
||
|
||
代码规模:18 个非 `pb/` 生成物 Go 文件(约 720 行,其中 `cmd/cli/main.go` 为 7 行空壳、`test/rpc/rpc.go` 为 23 行全注释),真正的业务逻辑只有 `internal/logic/library/` 下 5 个文件共约 260 行;`pb/` 目录 137 KB,其中与本模块业务无关的 `const.pb.go` 独占 94 KB。
|
||
|
||
| 目录 | 内容 |
|
||
| --- | --- |
|
||
| `cmd/main`、`cmd/cli` | 服务入口(`cmd/main/main.go:18`);`cli` 是 `Hello World!` 空壳 |
|
||
| `etc/` | `address_dev.yaml` / `address_test.yaml` / `address_prod.yaml`(三份内容完全相同)+ supervisord 配置 |
|
||
| `internal/config` | `SrvConfig`(`Base`/`Databases`/`MicroService`/`Rpc`/`Gateway`/`APM`/`Etcd`)与 `New()` |
|
||
| `internal/impl` | 依赖注入全局变量(`RedisCache`/`Etcd`/`DBService`) |
|
||
| `internal/logic/library` | 全部业务:`Create` / `Modify` / `Get` / `Fetch` / `Delete` |
|
||
| `internal/models` | 单表模型 `AddressLibrary`(表 `address_library`)+ MySQL/Postgres 连接与 AutoMigrate |
|
||
| `internal/server` | protoc-gen-slc 生成的薄封装(直接转调 logic) |
|
||
| `service/` | 供聚合模块 `Expose()` 的嵌入注册入口 |
|
||
| `proto/` | `address.proto`(1 个 service、5 个 RPC)+ `const.proto`(311 行与本模块无关的消息) |
|
||
| `test/` | `library/*.http`(3 个)、`rpc/rpc.go`(全注释)、空的 `lint/` 目录 |
|
||
|
||
对外 5 个 RPC,全部未做资源归属校验、未做字段/状态校验、未做分页:`Create(AddressCreateRequest)`、`Modify(AddressItem)`、`Get(IdentRequest)`、`Fetch(IdentRequest)`、`Delete(AddressDeleteRequest)`(`proto/address.proto:7-22`)。
|
||
|
||
**核心结论:本模块是「生成器脚手架 + 5 个 CRUD 盲写」的状态——`Get`/`Modify`/`Delete` 三个接口完全不做归属校验(可越权读写删任意用户的收货地址),`Fetch` 因命名返回值未赋值而必然返回空列表,且默认配置下 gorm Debug 会把收件人姓名/手机号/详细地址明文写进日志文件。** 其它为无事务/无约束的默认地址切换、无任何拦截器(无 recover/超时)、三份环境配置完全一致等工程缺陷。
|
||
|
||
## 2. 审计范围与方法
|
||
|
||
**范围**:`module/ec/address` 全部非生成物(`cmd/`、`etc/`、`internal/{config,impl,logic,models,server}`、`service/`、`proto/`、`test/`、`sdk/typescript/`、`go.mod`),并阅读 `pb/address.pb.gw.go` 的路由注册与 `pb/address.pb.go` 的消息定义以判定鉴权与序列化边界。为判定「谁在调用、谁在鉴权」,交叉阅读了:
|
||
|
||
- 外部依赖 `git.apinb.com/bsm-sdk/core`(`go.mod:64` 的 `replace` 指向工作区外的 `D:\work\bsm-sdk\core`,实际阅读该本地源码):`service/meta.go`(`ParseMetaCtx`)、`service/service.go`(`Start`/`Gateway`/`Stop`)、`conf/new.go`、`crypto/token/jwt.go`、`types/db.go`(`Std_IICUDS` 含 `gorm.DeletedAt`)、`database/sql/postgresql.go`(`SetOptions` 默认 `Debug: true`)、`cache/redis`、`env/env.go`。
|
||
- 聚合层 `pkgs/all/internal/{server,service,config}`(`server.go:32` 注册鉴权拦截器、`service/address.go` 注入依赖、`config.go:63-73` 校验并注入 JWT 密钥),确认独立部署与聚合部署两条运行路径的差异。
|
||
- 跨模块消费方 `module/ec/order/internal/logic/{summary,mgt}`:确认订单侧对 `address_library` 的引用方式。
|
||
- gorm `v1.31.0` 源码(`logger/logger.go`、`gorm.go`)确认 `Debug()` 的日志脱敏行为。
|
||
|
||
**方法**:全文 `read` 18 个非生成 Go 文件 + 3 个 yaml + supervisord 配置 + 2 个 proto + 2 个 ts + go.mod;用 `grep` 交叉验证死代码、`TODO`、错误丢弃、缓存调用点、`context.WithTimeout`、`Limit`;静态检查 `gofmt -l .`(无输出,通过)与 `go vet ./...`(exit 0,无输出,通过)。
|
||
|
||
**未能覆盖/无法验证的部分**(不臆断):
|
||
|
||
1. 运行时行为未验证(无可用 DB/Redis/etcd 环境),全部结论为静态代码分析;HTTP 网关「永远 404」的结论由代码路径推得(`Mux` 恒为 nil),未实际启动验证。
|
||
2. 生产环境是否设置 `BSM_JwtSecretKey`、是否真的部署 `/data/app/bsm-ec-address`(supervisord 配置所指的独立二进制)无法确认,涉及 P1-9 的可利用性。
|
||
3. 表 `address_library` 的线上真实 DDL/索引是否存在外部迁移脚本补齐(模型侧无 `(owner_id,status)` 唯一约束,见 P1-3),未找到迁移脚本目录。
|
||
4. 网关/接入层(`D:\work\bsm-infra\gateway`、`proxy`)是否对 `/address.Library/*` 追加额外鉴权/限流未展开。
|
||
5. `utils.UUID()`(`create.go:36`)在审计时可读到的 SDK 源码中为占位实现(`encipher.New` 在 `config.go:38` 被调用,但其内部实现未展开),identity 的熵强度未做定量评估。
|
||
|
||
## 3. 问题清单
|
||
|
||
### P0
|
||
|
||
#### 1. `Get` / `Modify` / `Delete` 完全不做归属校验,任意登录用户可读、改、删他人收货地址(IDOR,且 Delete 支持批量 ID)
|
||
|
||
- **位置**:`module/ec/address/internal/logic/library/get.go:29`、`module/ec/address/internal/logic/library/modify.go:39`、`module/ec/address/internal/logic/library/delete.go:24`、`module/ec/address/internal/logic/library/create.go:37`、`module/ec/address/proto/address.proto:74`
|
||
- **证据**:
|
||
|
||
```go
|
||
// get.go:28-29 —— 登录后仅校验 JWT 签名,随后只按主键查,无 owner 过滤
|
||
address := new(models.AddressLibrary)
|
||
err = impl.DBService.Where("id=?", in.Id).First(&address).Error
|
||
```
|
||
|
||
```go
|
||
// modify.go:39 —— 无 owner 过滤,任意 id 可被覆盖写入
|
||
err = impl.DBService.Where("id=?", in.Id).Updates(&address).Error
|
||
```
|
||
|
||
```go
|
||
// delete.go:24 —— 无 owner 过滤,且 id 是数组,可一次批量删除
|
||
err = impl.DBService.Delete(&models.AddressLibrary{}, "id in ?", in.Id).Error
|
||
```
|
||
|
||
```go
|
||
// create.go:37-38 —— 说明归属数据是存在的,只是读/改/删三个接口不使用它
|
||
address.OwnerID = auth.ID
|
||
address.OwnerIdentity = auth.Identity
|
||
```
|
||
|
||
```proto
|
||
// proto/address.proto:74-76
|
||
message AddressDeleteRequest {
|
||
repeated int64 id = 1; // ID
|
||
}
|
||
```
|
||
|
||
调用方身份只做「JWT 签名有效」判断、不做角色/范围校验,三个接口都传 `nil` 选项(`get.go:19`、`delete.go:19`、`modify.go:19` 的 `service.ParseMetaCtx(ctx, nil)`;SDK 侧 `checkRole` 仅在 `opts != nil` 时生效,`bsm-sdk/core/service/meta.go:36-45`)。ID 为自增整数(`types.Std_IICUDS.ID uint primarykey`),测试样例直接用 `{"id":1}`(`test/library/get.http:6`),可枚举。
|
||
|
||
- **影响**:地址属个人隐私数据(收件人姓名、手机号、门牌号)。任何持有一个有效 token 的用户(包括其它业务线/角色的 token,因为无 role 校验)即可:① `Get` 遍历 id 批量脱库(`1..N` 全量收货人姓名+手机号+详细地址);② `Modify` 篡改他人地址(配合默认地址逻辑可改变他人默认地址),可用于「改地址、劫持发货」类欺诈;③ `Delete` 传 `{"id":[1,2,3,...]}` 批量删除,可对全站用户做数据破坏。属于可被直接利用的越权漏洞。
|
||
- **建议**:把归属条件下沉到一个仓储/模型层方法(如 `FindByIDForOwner(id, ownerIdentity)`),禁止 logic 直接拼 `Where("id=?", ...)`;`Get/Modify/Delete` 一律 `Where("id = ? AND owner_identity = ?", in.Id, auth.Identity)`,并校验 `RowsAffected == 1` 才返回成功;`Delete` 的 `id` 列表先去重、限长(如 ≤50)、并对「不属于当前用户的 id」返回 `ErrPermissionDenied` 而不是静默忽略;`ParseMetaCtx` 传入角色/范围约束(至少区分 C 端消费者 token 与内部服务 token);删除接口保留而非逐条自增 ID 的对外标识(`Identity`)亦可降低枚举面。
|
||
|
||
#### 2. `Fetch` 命名返回值 `reply` 从未赋值,地址列表接口必然返回空结果
|
||
|
||
- **位置**:`module/ec/address/internal/logic/library/fetch.go:16,24-25,34-40`
|
||
- **证据**:
|
||
|
||
```go
|
||
// fetch.go:16-40
|
||
func Fetch(ctx context.Context, in *pb.IdentRequest) (reply *pb.AddressListReply, err error) {
|
||
...
|
||
var (
|
||
address = make([]*models.AddressLibrary, 0)
|
||
result = make([]*pb.AddressItem, 0) // ← 局部变量,最终被丢弃
|
||
)
|
||
err = impl.DBService.Where("owner_identity=?", auth.Identity).Find(&address).Error
|
||
...
|
||
for _, item := range address {
|
||
result = append(result, ReflectProtoAddress(item)) // ← 写的是 result,不是 reply
|
||
}
|
||
return // ← 裸 return,返回未赋值的 reply(nil)
|
||
}
|
||
```
|
||
|
||
- **影响**:`Fetch` 恒返回 `(nil, nil)`。gRPC 层会把 typed-nil 消息序列化为空消息(protobuf-go 对 nil 消息返回空字节、不报错),客户端拿到的是 `AddressListReply{}`(`data` 为空);HTTP 网关同样得不到任何数据。即「地址列表」功能 100% 失效,前端永远显示无地址,且 `ReflectProtoAddress`(`fetch.go:43-62`)与其循环成为无效计算。这是必然发生的功能/数据错误,不是概率问题。
|
||
- **建议**:`reply = &pb.AddressListReply{Data: result}` 后返回,或改为显式返回 `return &pb.AddressListReply{Data: result}, nil`;同时增加针对 `Fetch` 的断言型单测(见 P2-15),避免命名返回值再次踩坑。顺带把 `ReflectProtoAddress` 的调用改为直接构造切片,去掉中间 `address`→`result` 的双份内存。
|
||
|
||
### P1
|
||
|
||
#### 3. 默认地址切换:无事务、无唯一约束、错误被丢弃,并发可产生多个默认地址
|
||
|
||
- **位置**:`module/ec/address/internal/logic/library/create.go:41-45`、`module/ec/address/internal/logic/library/modify.go:35-39`、`module/ec/address/internal/models/address_library.go:16-17`、`module/ec/address/internal/models/impl.go:52`
|
||
- **证据**:
|
||
|
||
```go
|
||
// create.go:41-50
|
||
if address.Status == 2 {
|
||
impl.DBService.Model(&models.AddressLibrary{}).Where("owner_id = ? and status=2", auth.ID).UpdateColumn("status", "1")
|
||
} // ← 返回的 *gorm.DB 未接收、Error 未检查;且与下面的 Create 不在同一事务
|
||
err = impl.DBService.Create(address).Error
|
||
```
|
||
|
||
```go
|
||
// modify.go:35-39 —— 同一模式,且重置发生在「归属校验缺失」的 Updates 之前
|
||
if address.Status == 2 {
|
||
impl.DBService.Model(&models.AddressLibrary{}).Where("owner_id = ? and status=2", auth.ID).UpdateColumn("status", "1")
|
||
}
|
||
err = impl.DBService.Where("id=?", in.Id).Updates(&address).Error
|
||
```
|
||
|
||
```go
|
||
// models/address_library.go:16-17 —— 只有普通索引,没有 (owner_id, status=2) 的部分唯一约束
|
||
OwnerID uint `gorm:"column:owner_id;Index;" json:"owner_id"`
|
||
OwnerIdentity string `gorm:"column:owner_identity;type:varchar(36);Index;" json:"owner_identity"`
|
||
```
|
||
|
||
```go
|
||
// models/impl.go:52 —— 单条写入也不在隐式事务里
|
||
gorm.Open(mysql.Open(dsn[0]), &gorm.Config{ SkipDefaultTransaction: true })
|
||
```
|
||
|
||
- **影响**:① 「先降级旧默认、再写入新默认」是两条独立语句、无事务、无唯一约束,两个并发请求都会执行「重置 → 插入/更新 status=2」,最终用户的 `status=2` 记录可能 >1 条,之后所有「取默认地址」的消费方(前端/订单)行为不确定;② 若第二步 `Create/Updates` 失败,旧默认已降级、新默认未建立,用户进入「无默认地址」状态且接口仍可能返回 DB 错误后前端无补偿;③ `UpdateColumn("status","1")`(字符串字面量写 int8 列)绕过 hook、不更新 `updated_at`,只能靠 DB 隐式转换;④ 重置条件用 `owner_id`、列表查询用 `owner_identity`,同一归属存在两个口径,若 token 中 `id` 缺失(`auth.ID == 0`)则重置语句会命中所有 `owner_id = 0` 的记录(跨用户越权改写默认状态,可利用性取决于是否存在 id=0 的 token,标注「推测」)。
|
||
- **建议**:把「降级旧默认 + 写入新默认」放进同一个 `DBService.Transaction`(或 `Clauses(clause.OnConflict...)` 单条 upsert);数据库增加部分唯一索引 `CREATE UNIQUE INDEX uk_addr_default ON address_library(owner_id) WHERE status = 2 AND deleted_at IS NULL`,用约束兜住并发;检查 `UpdateColumn` 的 `.Error`;统一归属口径为 `owner_identity`;对 `auth.ID == 0`/`auth.Identity == ""` 直接判为非法调用方。
|
||
|
||
#### 4. 收货人姓名/手机号/详细地址以明文入库,并在默认配置下被 gorm Debug 明文写入日志文件
|
||
|
||
- **位置**:`module/ec/address/internal/models/impl.go:82`、`module/ec/address/internal/models/address_library.go:18-26`、`module/ec/address/etc/supervisor.bsm-ec-address.conf:8`、`module/ec/address/etc/address_prod.yaml:5-7`
|
||
- **证据**:
|
||
|
||
```go
|
||
// models/impl.go:74-83 —— Postgres 分支无条件开启 Debug,Debug() 内部是 LogMode(Info)
|
||
func NewPostgres(dsn []string, options *types.SqlOptions) (gormDb *gorm.DB, err error) {
|
||
...
|
||
db, err := sql.NewPostgreSql(dsn[0], options)
|
||
...
|
||
db = db.Debug()
|
||
```
|
||
|
||
```go
|
||
// bsm-sdk/core/database/sql/postgresql.go:11-21 —— 即使不显式调用,SDK 默认也是 Debug: true
|
||
options = &types.SqlOptions{ ..., Debug: true }
|
||
```
|
||
|
||
```go
|
||
// bsm-sdk/gorm.io/gorm@v1.31.0/logger/logger.go:75-78(Default 日志器 ParameterizedQueries 默认 false)
|
||
Default = New(log.New(os.Stdout, "\r\n", log.LstdFlags), Config{ SlowThreshold: 200*time.Millisecond, LogLevel: Warn, IgnoreRecordNotFoundError: false, ...})
|
||
// logger.go:194-197:ParameterizedQueries 为 false 时 explainSQL 返回插值后的 params
|
||
```
|
||
|
||
```ini
|
||
# etc/supervisor.bsm-ec-address.conf:7-8
|
||
redirect_stderr=true
|
||
stdout_logfile=/data/app/logs/ec-address.log
|
||
```
|
||
|
||
```go
|
||
// models/address_library.go:18-26 —— 列定义里没有任何加密/脱敏,落库即明文
|
||
Phone string `gorm:"column:phone;type:varchar(20);default:'';"` // 地址电话
|
||
Detail string `gorm:"column:detail;type:varchar(255);default:'';"` // 详细地址
|
||
Contact string `gorm:"column:contact;type:varchar(20);default:'';"` // 收件人
|
||
```
|
||
|
||
- **影响**:所有三套环境配置都是 `postgres`(`address_*.yaml:5`),因此 `db = db.Debug()` 必然执行:`Debug()` 等价于 `Logger.LogMode(logger.Info)`,而 gorm 默认日志器 `ParameterizedQueries=false`,会把 INSERT/UPDATE/SELECT 的**参数插值**后打印,典型形态为 `INSERT INTO "address_library" ("owner_id","phone","contact","detail") VALUES (12,'13800138000','张三','XX市XX区XX路1号101')`。该输出经 supervisord 落盘到 `/data/app/logs/ec-address.log`,并被任何日志采集/转发链路(如 yaml 里的 `OpLog`)二次扩散。个人隐私数据(姓名、手机号、精确地址)明文长期留存于日志文件,超出「最小必要」范围,属合规级隐私泄露;同时任何能读日志文件/日志平台的人即可批量获取全站收货信息。此外,`varchar(255)` 的详细地址、`varchar(20)` 的手机号在数据库、备份、Binlog/WAL 中均为明文,无字段级加密。
|
||
- **建议**:移除 `NewPostgres` 中无条件的 `db.Debug()`,改由配置项控制且生产默认关闭;即使需要 SQL 日志,也必须使用 `logger.New(..., logger.Config{ParameterizedQueries: true})` 或自定义 `ParamsFilter` 对 `phone/contact/detail` 做掩码;日志文件权限收敛(supervisord `user=root` + 0640)与轮转/保留期策略;对 `phone`/`contact`/`detail` 增加字段级加密或至少手机号掩码存储(对外返回时按权限决定是否脱敏);明确禁止把地址明细写入 `printer.*`/`OpLog`。
|
||
|
||
#### 5. 删除语义与引用完整性缺陷:软删除且不落 `status=-1`、不检查引用、跨模块按 identity 裸读且不校验归属
|
||
|
||
- **位置**:`module/ec/address/internal/logic/library/delete.go:24`、`module/ec/address/proto/address.proto:36,49`、`module/ec/address/internal/models/address_library.go:15`、`module/ec/order/internal/logic/summary/submit.go:68`
|
||
- **证据**:
|
||
|
||
```go
|
||
// delete.go:24 —— gorm 软删除(模型内嵌 Std_IICUDS 的 gorm.DeletedAt),status 原样不动
|
||
err = impl.DBService.Delete(&models.AddressLibrary{}, "id in ?", in.Id).Error
|
||
```
|
||
|
||
```go
|
||
// bsm-sdk/core/types/db.go:33
|
||
DeletedAt gorm.DeletedAt `gorm:"column:deleted_at;type:TIMESTAMP;index;" json:"deleted_at"`
|
||
```
|
||
|
||
```proto
|
||
// proto/address.proto:36 与 49 —— 协议声明「-1为删除」,但全模块没有任何一处写入 status = -1
|
||
int32 status = 12; // 状态 -1为删除,1为正常,2为设置成默认
|
||
```
|
||
|
||
```go
|
||
// module/ec/order/internal/logic/summary/submit.go:68-75 —— 消费方按 identity 直连地址表,既不校验 owner,
|
||
// 也用 Find(无记录时不返回 ErrRecordNotFound),失败分支形同虚设
|
||
err = impl.DBService.Table("address_library").Where("identity = ?", in.AddressIdentity).Find(&address).Error
|
||
if err != nil {
|
||
if errors.Is(err, models.ErrRecordNotFound) { return nil, errcode.ErrRecordNotFound }
|
||
return nil, errcode.ErrDB
|
||
}
|
||
```
|
||
|
||
(同类消费点还有 `module/ec/order/internal/logic/summary/quick_create_by_product.go:79` 与 `module/ec/order/internal/logic/mgt/order_create.go:101` 的 `Take(&address, "identity=?", ...)`。)
|
||
|
||
- **影响**:① 协议与实现的删除语义不一致:客户端以为 `status=-1` 才是「已删除」,实际是 `deleted_at` 软删,`status` 仍为 1/2,任何只按 `status` 过滤的消费方(或将来的报表/导出)会把已删地址当作有效地址;`status=-1` 成为永不出现的死状态。② 删除不检查 `address_library.identity` 是否已被进行中的订单引用,删除后订单侧按 identity 反查会查不到(若软删除作用域在 `Table()`+自定义结构体下未生效,则会查到「已删除」的地址,两种结果都有问题);③ 跨模块直接读地址表、且不校验 owner(订单可以指定任意 `address_identity` 作为收货地址,`identity` 是 UUID 难以枚举,故无法直接利用,但一旦 identity 通过任何渠道泄露——例如 P0-1 的 `Get` 会把它返回——即可把商品寄到他人地址),并且用 `Find` 导致「地址不存在」时不报错、以零值地址继续下单(该点属订单模块,标注为跨模块集成风险)。
|
||
- **建议**:明确一种删除语义并与 proto 对齐——若保留软删,则同步把 `status` 置 -1,并在 `Fetch`/`Get` 显式过滤;`Delete` 前置校验是否被未完成订单引用(或由订单侧快照地址后不再反查);地址对外只暴露 `identity`,订单侧增加 `owner_identity = 当前用户` 的过滤与 `Take`/`ErrRecordNotFound` 处理;把地址表访问收敛为通过本模块的 RPC(`Get` by identity + owner 校验),禁止 `Table("address_library")` 直连。
|
||
|
||
#### 6. 输入校验缺失:`status` 未校验且 int32→int8 截断,字段无必填/长度/行政区划编码校验
|
||
|
||
- **位置**:`module/ec/address/internal/logic/library/create.go:39`、`module/ec/address/internal/logic/library/modify.go:33`、`module/ec/address/internal/models/address_library.go:18-26`、`module/ec/address/internal/logic/library/get.go:24`
|
||
- **证据**:
|
||
|
||
```go
|
||
// create.go:39 —— int32 直接窄化为 int8,非法值/溢出静默截断(如 259 → 3),且不校验合法集合 {-1,1,2}
|
||
address.Status = int8(in.Status)
|
||
```
|
||
|
||
```go
|
||
// modify.go:33 —— 同样不做值域校验
|
||
address.Status = int8(in.Status)
|
||
```
|
||
|
||
```go
|
||
// models/address_library.go:18-26 —— 列长度约束在 DB,代码侧无长度/格式校验
|
||
Phone string `...type:varchar(20)...`; Contact string `...type:varchar(20)...`; Detail string `...type:varchar(255)...`
|
||
Province/City/Area string `...type:varchar(255)...` // 省/市/区:纯自由文本,无行政区划编码
|
||
```
|
||
|
||
`Get` 有 `if in.GetId() == 0 { return nil, errcode.ErrInvalidArgument }`(`get.go:24-26`),而 `Modify` 完全没有同类校验(见 P2-10),`Create` 也不校验 `phone/contact/detail` 是否为空、手机号格式、国家与省市一致性。
|
||
|
||
- **影响**:① 任意 `status` 值可入库,`status=0`(客户端不传时的默认值)产生既非正常也非默认的「僵尸地址」,前端列表/默认地址逻辑都会异常;② `int8` 截断使「非法输入」变成合法值(259→3、-257→-1),审计/风控难以追溯真实取值;③ 超长 `detail/contact/phone` 直接撞 DB 的 `varchar` 上限,返回 `errcode.ErrDB`(500 语义)而非参数错误,错误归因误导且暴露 SQL 细节到日志;④ 省市县为自由文本、无行政区划编码(GB/T 2260)校验,重复/错别字/错误层级(区与市不匹配)无法识别,后续运费与配送范围计算会出错。
|
||
- **建议**:新增 `validateAddress(in)`:`status ∈ {-1,1,2}`(且禁止客户端直接设置 -1)、`phone` 用正则/`len ≤ 20`、`contact` 必填且 `len ≤ 60`、`detail` 必填且 `len ≤ 200`、`province/city/area` 与行政区划码表校验(建议把 proto 的 `province/city/area` 改为 `province_code/city_code/area_code` 并存快照名);在 handler/logic 入口完成校验,DB 约束只作最后防线;用 `int32` 承载 `status`,去掉 `int8` 窄化。
|
||
|
||
#### 7. 无任何 gRPC 拦截器:无 panic recover、无超时/重试、无调用链上下文治理
|
||
|
||
- **位置**:`module/ec/address/internal/server/new.go:23`、`module/ec/address/cmd/main/main.go:24-34`、`module/ec/address/internal/impl/with.go:35,44,53,72,80`
|
||
- **证据**:
|
||
|
||
```go
|
||
// server/new.go:20-33 —— 独立模式下 grpc.NewServer() 不带任何 option/interceptor
|
||
if standalone {
|
||
grpcServ = grpc.NewServer()
|
||
}
|
||
...
|
||
pb.RegisterLibraryServer(srv.Grpc, NewLibraryServer())
|
||
```
|
||
|
||
```go
|
||
// main.go:24-34 —— 未传任何拦截器;pkgs/all 也只注册了鉴权拦截器(无 recovery)
|
||
srv := service.New(s.Grpc, &service.Options{ Addr: config.Spec.Addr, ... })
|
||
```
|
||
|
||
```go
|
||
// internal/impl/with.go —— 启动期大量 panic(含 DB/配置路径)
|
||
if config.Spec.Databases == nil || len(config.Spec.Databases.Source) == 0 { panic("No Database Source Found !") }
|
||
```
|
||
|
||
模块内检索 `context.WithTimeout`/`WithDeadline` 仅在 `pb/*.gw.go` 生成的 `context.WithCancel` 中出现,业务代码零超时(见 `grep` 结果:`internal/**` 无一处超时);`impl.DBService` 的查询一律使用无超时的 `ctx` 无关调用(gorm 的 `WithContext` 从未被使用)。
|
||
|
||
- **影响**:① grpc-go 默认不 recover handler panic——本模块 `Insert`/`Update` 参数拼接、`ReflectProtoAddress`、任何未来引入的空指针都会让整个进程崩溃,supervisord 只能重启(`autorestart=true`),一次坏输入即可打满重启循环、影响该端口上所有服务;② 无超时:慢 SQL/锁等待会耗尽连接与 goroutine,且 `Fetch` 无分页(P2-13)时单请求内存与耗时随地址量线性增长;③ 无 `WithContext(ctx)`,客户端取消/超时不会传导到数据库,取消的请求继续占用连接;④ 六个启动路径用 `panic`/`log.Fatalln`(`with.go:35,44,53,72,80`、`models/impl.go:28,33,40`)中断进程,无降级与可读错误码。
|
||
- **建议**:为 `grpc.NewServer` 加统一拦截器链:`recovery`(含堆栈与 `request_id` 日志,返回 `codes.Internal`)、`timeout`(如 3s,可用 `grpc.UnaryInterceptor` + `context.WithTimeout`)、`logging/metrics`、`auth`;DB 调用统一 `impl.DBService.WithContext(ctx)`,由 ctx 承载 deadline;把启动期 `panic`/`Fatalln` 换成返回错误 + 明确退出码,并保留 `defer` 清理(`etcd`/`sqlDB` 的 `Close`)。
|
||
|
||
#### 8. 独立部署路径下 HTTP 网关完全不可用(`Server.Mux` 从未初始化,`ListenAndServe(nil)` 落到 DefaultServeMux)
|
||
|
||
- **位置**:`module/ec/address/internal/server/new.go:13-30`、`module/ec/address/cmd/main/main.go:23-33`、`module/ec/address/etc/address_prod.yaml:21-24`
|
||
- **证据**:
|
||
|
||
```go
|
||
// server/new.go:13-30 —— 构造 Server 时只填 Ctx/Grpc/grpcConns,Mux 保持 nil
|
||
type Server struct {
|
||
Grpc *grpc.Server
|
||
Ctx context.Context
|
||
Mux *gwRuntime.ServeMux
|
||
grpcConns map[string]*grpc.ClientConn // 连接池
|
||
}
|
||
srv := &Server{ Ctx: context.Background(), Grpc: grpcServ, grpcConns: make(map[string]*grpc.ClientConn) }
|
||
```
|
||
|
||
```go
|
||
// cmd/main/main.go:30-33 —— 把 nil 传给 SDK
|
||
GatewayCtx: s.Ctx,
|
||
GatewayConf: config.Spec.Gateway,
|
||
GatewayMux: s.Mux, // == nil,且本文件从未调用 pb.RegisterLibraryHandlerServer
|
||
```
|
||
|
||
```go
|
||
// bsm-sdk/core/service/service.go:106-111 与 120-129
|
||
if s.Opts.GatewayConf != nil && s.Opts.GatewayConf.Enable {
|
||
addr := Addr("0.0.0.0", s.Opts.GatewayConf.Port)
|
||
go s.Gateway(s.Opts.Addr, addr)
|
||
}
|
||
...
|
||
if err := http.ListenAndServe(httpAddr, s.Opts.GatewayMux); err != nil { ... } // handler 为 nil → DefaultServeMux
|
||
```
|
||
|
||
配置三套环境均为 `Gateway: Enable: true` / `Port: 12459`(`address_dev.yaml:21-24`),且 `test/library/*.http` 全部指向 `http://127.0.0.1:12459/address.Library/...`。
|
||
|
||
- **影响**:独立二进制(`etc/supervisor.bsm-ec-address.conf:2` 的 `/data/app/bsm-ec-address`)会监听 12459 但 `DefaultServeMux` 上没有任何 `/address.Library/*` 路由,所有 HTTP 请求 404;且 gRPC 网关所需的 `RegisterLibraryHandlerServer` 在此路径下从未调用,因此不是「部分接口」而是「整条 HTTP 通道」失效(该写法的根因在 protoc-gen-slc 生成的 `server.New` 模板,同仓 `module/ec/order/internal/server/new.go:26-30` 同样不设置 `Mux`;可用路径只有 `pkgs/all`、`pkgs/ecmall` 里 `gwRuntime.NewServeMux()` 的聚合模式)。这也解释了 `test/library/*.http` 事实上无法对独立服务生效。
|
||
- **建议**:在 `server.New` 中初始化 `Mux: gwRuntime.NewServeMux()`,并在启动时调用 `pb.RegisterLibraryHandlerServer(ctx, srv.Mux, NewLibraryServer())`(或由 `cmd/main` 显式调用 `service.Expose`);同时修 protoc-gen-slc 模板,使所有模块一致;给 `/healthz` 增加探针,避免「端口在监听但全 404」这类静默故障。
|
||
|
||
#### 9. JWT 密钥未做校验,独立部署回落到 SDK 内置默认密钥(可伪造任意用户身份)
|
||
|
||
- **位置**:`module/ec/address/internal/config/config.go:35-38`、`module/ec/address/etc/address_prod.yaml:27`、`bsm-sdk/core/env/env.go:19`、`bsm-sdk/core/service/meta.go:31`
|
||
- **证据**:
|
||
|
||
```go
|
||
// internal/config/config.go:34-38 —— 只校验 Service/Cache,密钥完全未校验
|
||
conf.NotNil(Spec.Service, Spec.Cache)
|
||
encipher.New(env.Runtime.JwtSecretKey)
|
||
```
|
||
|
||
```go
|
||
// bsm-sdk/core/env/env.go:19 —— 环境变量缺失时使用硬编码默认值
|
||
JwtSecretKey: GetEnvDefault("BSM_JwtSecretKey", "Cblocksmesh2022C"),
|
||
```
|
||
|
||
```go
|
||
// bsm-sdk/core/service/meta.go:31 —— 身份完全来自该密钥验签的 JWT
|
||
claims, err := token.New(env.Runtime.JwtSecretKey).ParseJwt(Authorizations[0])
|
||
```
|
||
|
||
`etc/address_*.yaml` 全文(38 行)没有 `Authorization` 段,本模块也不像 `pkgs/all/internal/config/config.go:73` 那样把密钥注入 `env.NewEnv().JwtSecretKey`。
|
||
|
||
- **影响**:独立部署时,只要运行环境未显式设置 `BSM_JwtSecretKey`,本服务的验签密钥就是公开可读源码里的 `Cblocksmesh2022C`;任何攻击者可用该密钥自行签发 `{"id":<victim>,"identity":"<victim>"}` 的 HS256 token,从而在 `Create` 时把地址挂到他人名下、并绕过所有基于 `auth.ID/auth.Identity` 的逻辑(与 P0-1 叠加后危害进一步放大)。是否真的未设置环境变量无法从仓库确认(标注「推测」),但「配置层不校验、静默回落默认值」本身即是必须修复的不安全默认。
|
||
- **建议**:`config.New` 中断言 `env.Runtime.JwtSecretKey` 非空且长度 ∈ {16,24,32}(与 `pkgs/all/internal/config/config.go:63-69` 一致),或在模块 yaml 中引入 `Authorization.Key` 并显式 `env.NewEnv().JwtSecretKey = Spec.Authorization.Key`;部署清单强制注入密钥;移除/限定 SDK 的默认密钥回落;`ParseMetaCtx` 失败时按 `errcode` 区分 401/403 并纳入告警。
|
||
|
||
### P2
|
||
|
||
#### 10. `Create`/`Modify` 字段与幂等语义缺陷:`name`/`pics` 改不动、零值无法清空、`id=0` 静默成功、重复提交产生重复地址
|
||
|
||
- **位置**:`module/ec/address/internal/logic/library/modify.go:24-39`、`module/ec/address/internal/logic/library/create.go:25-35`、`module/ec/address/internal/logic/library/get.go:24-26`
|
||
- **证据**:
|
||
|
||
```go
|
||
// modify.go:24-32 —— 构造的模型缺少 Name/Pics(Create 里有,见 create.go:33-34)
|
||
address := &models.AddressLibrary{
|
||
Country: in.Country, Phone: in.Phone, Province: in.Province,
|
||
City: in.City, Area: in.Area, Detail: in.Detail, Contact: in.Contact,
|
||
}
|
||
```
|
||
|
||
```go
|
||
// modify.go:39 —— struct 作为 Updates 参数时 gorm 只写非零字段:
|
||
// 因此「清空某个字段」「把 status 设为 0」都做不到;且没有 RowsAffected 判定
|
||
err = impl.DBService.Where("id=?", in.Id).Updates(&address).Error
|
||
```
|
||
|
||
- **影响**:`AddressItem.name`(地址备注名)与 `pics` 永远无法通过 `Modify` 更新,调用方传了也被静默丢弃;用户无法清空字段;`Modify(id=0)`(`test/library/modify.http:5-7` 就是空 body)匹配不到任何行却返回 `Code 0 / "OK"`,调用方会误以为修改成功;`Create` 无幂等键,前端重试/双击会插入两条完全相同的地址。这些都会污染地址簿并影响默认地址判定。
|
||
- **建议**:改用 `map[string]any` 或 `Select("country","phone",...,"name","pics","status").Updates(...)` 显式声明可写字段,并对必填字段做「空串即拒绝」而非「空串即忽略」的语义区分;补 `in.Id == 0` 校验(与 `get.go:24` 对齐)并检查 `RowsAffected`;为 `Create` 引入幂等键(客户端 `request_id`)或 `(owner_identity, phone, detail)` 去重窗口。
|
||
|
||
#### 11. 错误处理缺陷:`UpdateColumn` 错误被丢弃、`sqlDB` 忽略错误后解引用、删除/修改不以 `RowsAffected` 判定
|
||
|
||
- **位置**:`module/ec/address/internal/models/impl.go:63-69`、`module/ec/address/internal/logic/library/create.go:42`、`module/ec/address/internal/logic/library/modify.go:36`、`module/ec/address/internal/logic/library/delete.go:24-34`
|
||
- **证据**:
|
||
|
||
```go
|
||
// models/impl.go:63-69 —— 忽略 err 后立即解引用 sqlDB
|
||
sqlDB, _ := gormDb.DB()
|
||
sqlDB.SetMaxIdleConns(options.MaxIdleConns)
|
||
sqlDB.SetMaxOpenConns(options.MaxOpenConns)
|
||
sqlDB.SetConnMaxLifetime(options.ConnMaxLifetime)
|
||
```
|
||
|
||
```go
|
||
// create.go:42 与 modify.go:36 —— 整条 gorm 链的返回值被直接丢弃,.Error 无人查看
|
||
impl.DBService.Model(&models.AddressLibrary{}).Where("owner_id = ? and status=2", auth.ID).UpdateColumn("status", "1")
|
||
```
|
||
|
||
```go
|
||
// delete.go:24-34 —— 不检查 RowsAffected,删 0 行也返回 OK
|
||
err = impl.DBService.Delete(&models.AddressLibrary{}, "id in ?", in.Id).Error
|
||
if err != nil { ... }
|
||
return &pb.StatusReply{Code: 0, Message: "OK", ...}, nil
|
||
```
|
||
|
||
- **影响**:① `gormDb.DB()` 失败时 `sqlDB` 为 nil,`SetMaxIdleConns` 直接空指针 panic(启动期崩溃,且错误信息无上下文);② 默认地址降级失败被静默吞掉,用户会看到「创建/修改成功」但默认地址状态错乱,且日志里毫无痕迹(可观测性缺口);③ 删除不存在的 id、删除他人 id、id 列表为空(`id in (NULL)` 命中 0 行)都返回成功,攻击者/调用方无法从响应区分「已删除」与「无权限/不存在」,也让 P0-1 的批量删除更隐蔽、运维难以从返回码发现异常。
|
||
- **建议**:统一错误处理模板:`tx := ...; if tx.Error != nil {...}; if tx.RowsAffected == 0 { return errcode.ErrRecordNotFound / ErrPermissionDenied }`;`sqlDB, err := gormDb.DB(); if err != nil { return nil, err }`;`in.Id` 为空时直接 `ErrInvalidArgument`;`printer.Error` 附带方法名与关键参数(不含 PII)。
|
||
|
||
#### 12. 迁移与启动耦合:每次启动 `AutoMigrate`,且失败即 `log.Fatalln`/`os.Exit`
|
||
|
||
- **位置**:`module/ec/address/internal/models/impl.go:19-45`、`module/ec/address/internal/impl/impl.go:7-11`
|
||
- **证据**:
|
||
|
||
```go
|
||
// models/impl.go:37-42
|
||
// auto migrate table.
|
||
err = DBService.AutoMigrate(migrateTables...)
|
||
if err != nil {
|
||
log.Fatalln(err)
|
||
return
|
||
}
|
||
```
|
||
|
||
```go
|
||
// models/impl.go:25-35 —— 不支持驱动/连接失败同样 Fatalln;New 由启动路径同步调用
|
||
switch driver {
|
||
case "mysql": DBService, err = NewMysql(dsn, options)
|
||
case "postgres": DBService, err = NewPostgres(dsn, options)
|
||
default: log.Fatalln("Unsupported database driver:", driver)
|
||
}
|
||
```
|
||
|
||
- **影响**:建表/改表与进程启动强耦合——多实例同时启动会并发执行 DDL(Postgres 上 `ALTER TABLE` 需排他锁,可能出现互相等待/锁超时导致启动失败);`AutoMigrate` 只加不删、无法表达回滚,模型与线上表结构可能长期漂移(例如 P1-3 需要的部分唯一索引无法由 AutoMigrate 表达);`log.Fatalln` 直接 `os.Exit(1)`,跳过所有 `defer`(etcd/redis/sqlDB 关闭),supervisord 只能看到退出码。
|
||
- **建议**:迁移独立成版本化脚本/工具(或至少由环境变量开关 `IsAutoMigrate` 控制,生产默认关闭);启动时只做「连通性 + 版本表校验」,失败返回错误并由部署层决定重启;`log.Fatalln` 改 `printer.Error` + `return err`,把退出决策留在 `main`。
|
||
|
||
#### 13. 无缓存、依赖空转、查询无分页(`FetchRequest` 为死代码)
|
||
|
||
- **位置**:`module/ec/address/internal/logic/library/fetch.go:28`、`module/ec/address/internal/impl/impl.go:9`、`module/ec/address/internal/impl/with.go:24-31`、`module/ec/address/proto/address.proto:52-56`
|
||
- **证据**:
|
||
|
||
```go
|
||
// fetch.go:28 —— 无 Limit/Offset;proto 虽定义分页字段但 RPC 用的是 IdentRequest
|
||
err = impl.DBService.Where("owner_identity=?", auth.Identity).Find(&address).Error
|
||
```
|
||
|
||
```proto
|
||
// proto/address.proto:52-56 —— FetchRequest(含 page_no/page_size/params)没有任何 RPC 引用
|
||
message FetchRequest { int64 page_no=1; int64 page_size=2; map<string,string> params=3; }
|
||
```
|
||
|
||
```go
|
||
// impl.go:9 与 with.go:24-31 —— 初始化了 Redis 客户端,但全模块(除赋值外)无任何缓存读写
|
||
withRedisCache(vars.ServiceKey) // redis cache
|
||
```
|
||
|
||
- **影响**:`Fetch` 一次返回该用户全部地址(`grep` 确认模块内无 `Limit`),地址数量无上限时是慢查询+大响应;`RedisCache` 只被初始化(并打印 `DBIndex`),业务侧从不使用,属于「有依赖无收益」的空转(也意味着订单侧每次下单都要直查地址表,无缓存缓冲);`FetchRequest` 与其 `page_no/page_size/params` 是明显的半成品设计,与 `IdentRequest` 混用会让调用方误以为支持分页/条件过滤。
|
||
- **建议**:`Fetch` 接受 `FetchRequest` 并强制分页(默认 20、上限 100)+ `total`;或删除 `FetchRequest` 以免误导;地址列表读多写少,可对 `owner_identity` 维度加短 TTL 缓存(Redis 已有连接,或在 `service.Dependencies.Cache` 用进程内 cache),写路径失效;`owner_identity` 已有索引(`models/address_library.go:17`),但 `Fetch` 的排序未定义,建议固定 `ORDER BY status DESC, id DESC` 保证分页稳定。
|
||
|
||
#### 14. 可观测性与优雅退出缺失:DB 配置(含密码)入日志、无健康检查/request_id/APM、`select{}` 使 `defer` 永不执行
|
||
|
||
- **位置**:`module/ec/address/internal/impl/with.go:39`、`module/ec/address/internal/server/new.go:33-37`、`module/ec/address/cmd/main/main.go:36-40`、`bsm-sdk/core/service/service.go:113-114`
|
||
- **证据**:
|
||
|
||
```go
|
||
// impl/with.go:39 —— %v 打印整个 DB 配置,DSN 内含账号密码(address_*.yaml:7 的 user/password)
|
||
printer.Info("[BSM - %s] Databases: %v", vars.ServiceKey, config.Spec.Databases)
|
||
```
|
||
|
||
```go
|
||
// server/new.go:32-37 —— 只注册业务服务,无 grpc health / metrics
|
||
pb.RegisterLibraryServer(srv.Grpc, NewLibraryServer())
|
||
if standalone { reflection.Register(srv.Grpc) }
|
||
```
|
||
|
||
```go
|
||
// cmd/main/main.go:36-40 与 bsm-sdk/core/service/service.go:113-114
|
||
defer srv.Stop()
|
||
srv.Start() // Start 内部以 select{} 永久阻塞 → 上面的 defer 永不执行,且无 signal 处理
|
||
```
|
||
|
||
- **影响**:① 启动日志即输出数据库账号密码(明文,落 `/data/app/logs/ec-address.log`),与 P1-4 叠加构成凭据泄露;② 无健康检查端点/`grpc.health.v1`,K8s/LB 无法区分「进程活着但 DB 不可用」,也解释了 P1-8 这类静默 404 无法被发现;③ 无结构化日志与 `request_id` 贯通,`printer.Error(err.Error())`(如 `get.go:35`)只留下孤立字符串,无法关联到具体请求/用户;④ `APM` 段在三套 yaml 中被注释掉(`address_prod.yaml:35-38`),无链路追踪;⑤ 独立二进制没有 signal 处理,`defer srv.Stop()` 因 `select{}` 永不执行,`GrpcSrv.GracefulStop()` 形同虚设,SIGTERM 直接杀进程(对比 `pkgs/all/cmd/main/main.go:46-53` 有完整的 signal + 超时 Shutdown)。
|
||
- **建议**:启动日志改为打印驱动/主机/库名(脱敏 DSN);注册 `grpc_health_v1` 并把 DB/Redis 探活纳入;日志接入 SDK logger 并注入 `request_id`/`method`/`user_identity`;`Start` 改为接收 `context.Context` 或返回 error,`main` 里 `signal.NotifyContext` + `Stop` 超时(复用 `pkgs/all` 的写法);补充 Prometheus 指标(QPS/错误率/DB 耗时)与 APM。
|
||
|
||
#### 15. 测试缺失:无任何 Go 单测,`test/lint` 空目录,`test/rpc/rpc.go` 全注释,HTTP 用例不覆盖删除/列表与非法入参
|
||
|
||
- **位置**:`module/ec/address/test/rpc/rpc.go:3-23`、`module/ec/address/test/library/{create,get,modify}.http`、`module/ec/address/test/lint/`(空)
|
||
- **证据**:
|
||
|
||
```go
|
||
// test/rpc/rpc.go:3-23 —— 整个 main 被注释掉,文件不提供任何可执行测试
|
||
/*
|
||
func main() { ... c := pb.NewCheckClient(conn) ... }
|
||
*/
|
||
```
|
||
|
||
```
|
||
test/library/ 仅有 create.http / get.http / modify.http;无 delete.http、无 fetch.http
|
||
test/library/modify.http:5-7 请求体为空({"id":...} 都没写),无法构成有效 Modify 调用
|
||
```
|
||
|
||
模块内 `Get-ChildItem -Filter *_test.go` 无结果;`test/lint/` 目录存在但为空(同仓 `module/ec/order/test/lint` 亦为空)。
|
||
|
||
- **影响**:P0-1(越权)、P0-2(`Fetch` 恒空)、P1-3(默认地址并发)这三类问题在现有测试下不可能被发现;`modify.http` 的空 body 会「成功返回 OK」(见 P2-10),进一步掩盖问题。
|
||
- **建议**:补齐关键缺失用例清单(优先级从高到低):
|
||
1) **越权**:用户 A 的 token 调 `Get/Modify/Delete` 用户 B 的地址 id,断言返回 `ErrPermissionDenied/ErrRecordNotFound` 且 B 的数据未变(对应 P0-1);
|
||
2) **列表**:`Fetch` 造 3 条地址,断言 `data` 长度为 3 且只含本人地址(对应 P0-2);
|
||
3) **默认地址并发**:并发 N 次带 `status=2` 的 `Create`,断言最终 `status=2` 的记录恰为 1 条(对应 P1-3);
|
||
4) **非法入参**:`status` ∈ {0, 3, 259, -257}、超长 `detail/contact/phone`、空 `id` 列表删除、`Modify(id=0)` 均须返回参数错误且不落库(对应 P1-6/P2-10);
|
||
5) **鉴权**:无 `authorization`、过期 token、错误角色 token(对应 P1-9);
|
||
6) **PII**:断言日志中不出现手机号/详细地址明文(可用自定义 logger 捕获,对应 P1-4)。
|
||
|
||
### P3
|
||
|
||
#### 16. `etc/` 三套环境配置完全一致,`Service` 名错误,凭据为字面占位符
|
||
|
||
- **位置**:`module/ec/address/etc/address_dev.yaml:1-3`、`address_test.yaml`、`address_prod.yaml`、`module/ec/address/cmd/main/main.go:15`
|
||
- **证据**:
|
||
|
||
```
|
||
address_dev.yaml / address_test.yaml / address_prod.yaml 三个文件 SHA256 完全一致
|
||
53DB271D97405673CE4EB5199ADAB30B6D68362892547457BFA77D5F8A0B7936(三份同哈希)
|
||
|
||
address_prod.yaml:1-2 Service: order / Port: 12460 ← 本模块服务名应为 address(main.go:15 ServiceKey = "Address")
|
||
address_prod.yaml:7 host=127.0.0.1 user=postgres password=CHANGE_ME dbname=ec_mall ...
|
||
address_prod.yaml:10 Cache: redis://null:CHANGE_ME@127.0.0.1:6379/
|
||
```
|
||
|
||
(对比:同仓 `module/ec/order/etc/order_dev.yaml:1-2` 为 `Service: order / Port: 12442`,`module/ec/mall/etc/mall_dev.yaml` 为 `mall`;`Service` 字段是照抄 order 的。SDK 只会做 `strings.Contains(yamlString, "Service:")` 存在性检查,`bsm-sdk/core/conf/new.go:57-60`,不会校验取值,因此该错误不会启动报错,但会误导运维与后续把 `Service` 用于注册/日志的改造。)
|
||
|
||
- **影响**:生产与测试环境无差异(同库、同端口、同占位密码),无法防止误连生产/开发库;`Service: order` 与实际服务不一致,一旦有人在 SDK 侧开始消费 `Spec.Service`(如注册、鉴权白名单、APM 服务名)就会产生串服务故障;`CHANGE_ME` 是字面量(`os.ExpandEnv` 只在 `$VAR` 形式下生效),若部署时忘记改就是真实生效的弱口令。
|
||
- **建议**:三套配置按环境拆分(生产库地址、只读账号、日志级别、APM 开关),敏感项改为 `${BSM_DB_PASSWORD}` 形式的环境变量占位;`Service` 改为 `address`;增加 CI 校验(同一模块多环境 `Service` 必须等于模块名、密码不得为 `CHANGE_ME`)。
|
||
|
||
#### 17. 死代码与生成器遗留
|
||
|
||
- **位置**:`module/ec/address/cmd/cli/main.go:5-7`、`module/ec/address/test/rpc/rpc.go:3-23`、`module/ec/address/internal/logic/library/fetch.go:38`、`module/ec/address/internal/server/new.go:17,29`、`module/ec/address/internal/logic/library/{create,modify,delete}.go` 的 `StatusReply`、`module/ec/address/internal/models/query.go:5`
|
||
- **证据**:
|
||
|
||
```go
|
||
// cmd/cli/main.go:5-7 —— 与地址业务无关的脚手架残留
|
||
func main() { log.Println("Hello World!") }
|
||
```
|
||
|
||
```go
|
||
// fetch.go:38 —— 生成器提示未清理
|
||
// TODO: add your logic code & delete this line.
|
||
```
|
||
|
||
```go
|
||
// server/new.go:17,29 —— grpcConns 声明并初始化后从未被读写
|
||
grpcConns map[string]*grpc.ClientConn // 连接池
|
||
... grpcConns: make(map[string]*grpc.ClientConn),
|
||
```
|
||
|
||
`StatusReply.Identity`(`pb/address.pb.go:387`)在 `create.go:52-56`、`modify.go:45-49`、`delete.go:30-34` 从未赋值,恒为空串;`models/query.go:5` 只是 `gorm.ErrRecordNotFound` 的别名;`proto/const.proto` 311 行消息(订单/购物车/CMS/社交)与本模块 RPC 无任何引用,却生成 94 KB 的 `pb/const.pb.go`。
|
||
|
||
- **影响**:误导后续维护者(`cli` 与 `rpc.go` 看起来像入口/测试但不可用)、编译与镜像体积无谓膨胀、协议字段 `StatusReply.identity` 长期为空会让调用方写出依赖空值的错误逻辑;`TODO` 注释表明 `Fetch` 从未被真正实现过(与 P0-2 一致)。
|
||
- **建议**:删除 `cmd/cli`、`test/rpc/rpc.go`(或改写为真实可运行的 gRPC 冒烟测试)与 `grpcConns`;`StatusReply.Identity` 在 `Create` 时回填 `address.Identity` 或从 proto 移除;`const.proto` 按模块裁剪(只保留被引用的消息);清理生成器 TODO。
|
||
|
||
#### 18. TypeScript SDK 不可用且与 proto/README 不一致
|
||
|
||
- **位置**:`module/ec/address/sdk/typescript/address/index.ts:88-104`、`module/ec/address/sdk/typescript/address/const.ts:5-29`、`module/ec/address/README.md:1-2`
|
||
- **证据**:
|
||
|
||
```ts
|
||
// sdk/typescript/address/index.ts:88-104 —— 接口为空、工厂函数返回空对象,没有任何 RPC 方法
|
||
export interface Library {
|
||
}
|
||
export function createLibraryClient(handler: RequestHandler): Library {
|
||
return {
|
||
};
|
||
}
|
||
```
|
||
|
||
```ts
|
||
// sdk/typescript/address/const.ts:5-29 —— 同一目录却列出了 5 个 URL 常量,与方法缺失矛盾
|
||
export const URL_Library_Create = "/address.Library/Create"
|
||
...
|
||
export const URL_Library_Delete = "/address.Library/Delete"
|
||
```
|
||
|
||
```md
|
||
<!-- README.md:1-2 —— 全文仅一行标题 -->
|
||
# address
|
||
```
|
||
|
||
(对比同仓 `module/base/cms/README.md` 17 KB、`module/ec/mall/README.md` 15 KB。)
|
||
|
||
- **影响**:前端按 `createLibraryClient` 拿到的是空对象,无法调用任何接口(只能自己拼 `const.ts` 里的 URL),生成的 SDK 实际不可用;README 无接口说明、无鉴权说明、无数据字典,新成员只能读代码判断字段含义与状态语义(`status` 的 -1/1/2 就因此频繁被误用)。
|
||
- **建议**:修 protoc-gen-ts 模板生成真实方法(或在 `make sdk` 中失败即报错),并把 `address.proto` 的注释补全(`status` 取值、必填字段、长度约束);README 至少包含:鉴权方式(`Authorization` 头 + `ParseMetaCtx`)、5 个 RPC 的请求/响应示例、`status` 语义、错误码表、`test/library/*.http` 的使用说明。
|
||
|
||
#### 19. HTTP 语义与接口设计:全部 POST、proto 无 `google.api.http` 注解、`IdentRequest.identity` 被忽略、Create 无幂等键
|
||
|
||
- **位置**:`module/ec/address/proto/address.proto:1-22`、`module/ec/address/pb/address.pb.gw.go:407-413`、`module/ec/address/internal/logic/library/fetch.go:16-28`、`module/ec/address/internal/logic/library/get.go:17-29`
|
||
- **证据**:
|
||
|
||
```go
|
||
// pb/address.pb.gw.go:407-413 —— 5 个方法一律 POST + 固定路径,无路径参数、无 GET
|
||
pattern_Library_Create_0 = runtime.MustPattern(runtime.NewPattern(1, []int{2, 0, 2, 1}, []string{"address.Library", "Create"}, ""))
|
||
pattern_Library_Get_0 = runtime.MustPattern(runtime.NewPattern(1, []int{2, 0, 2, 1}, []string{"address.Library", "Get"}, ""))
|
||
```
|
||
|
||
```go
|
||
// get.go:24-29 —— IdentRequest 同时带 id 与 identity,但只使用 id;Fetch 则两者都不用
|
||
if in.GetId() == 0 { return nil, errcode.ErrInvalidArgument }
|
||
err = impl.DBService.Where("id=?", in.Id).First(&address).Error
|
||
```
|
||
|
||
- **影响**:查询类接口(`Get`/`Fetch`)用 POST + JSON body 表达,天然不可缓存、不符合幂等语义,也难以被网关/浏览器/CDN 正确处理;协议里 `IdentRequest.identity`(`proto/address.proto:60`)形同虚设,调用方传 identity 会被静默忽略(返回 0 或空列表),排查成本高;`Create` 无幂等键 + 无去重(见 P2-10)使「POST 重试」变成重复地址。
|
||
- **建议**:为 `Get` 增加 `GET /v1/address/{identity}`、`Fetch` 增加 `GET /v1/address?page_no=&page_size=`、`Delete` 用 `DELETE`(或明确保留 POST 语义并在文档中说明),并在 proto 中补齐 `google.api.http` 注解;`IdentRequest` 拆分为 `GetByIdentity(identity)` 与 `GetById(id)` 两个语义明确的请求(或明确只支持一种);`Create` 增加 `request_id` 幂等字段。
|
||
|
||
## 4. 推荐优化方案
|
||
|
||
按「先堵越权与隐私、再修功能、后补工程」排序,建议分三批落地:
|
||
|
||
**第一批(安全/正确性,1~2 天,对应 P0-1、P0-2、P1-3、P1-4、P1-9)**
|
||
|
||
1. 在 `logic/library` 引入统一的「归属上下文」:`auth, err := service.ParseMetaCtx(ctx, &service.ParseOptions{...})` 后统一构造 `scope := impl.DBService.Where("owner_identity = ?", auth.Identity)`,`Get/Modify/Delete` 一律 `scope.Where("id = ?", in.Id)`,并以 `RowsAffected == 1` 判定成功;`Delete` 的 id 数组去重 + 限长 + 逐条归属校验。删除 `UpdateColumn` 的无条件调用与 `sqlDB, _ :=`。
|
||
2. 修 `Fetch` 返回值(`reply = &pb.AddressListReply{Data: result}`),并补 `Fetch` 单测断言非空。
|
||
3. 默认地址改为事务 + 数据库约束:`Transaction` 内「降级旧默认 → 写入新默认」,并新增部分唯一索引 `uk_addr_default(owner_id) WHERE status = 2 AND deleted_at IS NULL`;`status` 只允许 {-1,1,2} 且客户端不得直接置 -1。
|
||
4. 关闭 SQL 明文日志:删除 `models/impl.go:82` 的 `db.Debug()`,生产配置显式 `Debug: false`;若必须保留,使用 `ParameterizedQueries: true` 的自定义 logger,并对 `phone/contact/detail` 掩码;`printer.Info` 不再打印整个 DB 配置。
|
||
5. 校验 JWT 密钥(非空、长度 16/24/32),移除对 SDK 默认密钥的静默回落;`ParseMetaCtx` 增加角色约束,区分 C 端与内部服务调用。
|
||
|
||
**第二批(健壮性/可观测性,2~4 天,对应 P1-5、P1-6、P1-7、P1-8、P2-11、P2-12、P2-14)**
|
||
|
||
6. 统一拦截器链:`recovery`(堆栈 + request_id)、`timeout`(3s)、`logging`、`metrics`;业务 DB 调用全部 `.WithContext(ctx)`。
|
||
7. 修 `server.New` 的 `Mux` 初始化与 `RegisterLibraryHandlerServer` 注册(同时推动 protoc-gen-slc 模板修复),加 `/healthz` 与 `grpc.health.v1`。
|
||
8. 引入 `validateAddress`(必填/长度/手机号/`status` 值域/行政区划编码),去掉 `int32→int8` 窄化;`Modify` 改为显式可写字段列表,补 `id == 0` 校验。
|
||
9. 明确删除语义(软删 + `status=-1` 同步),清理 `status=-1` 死状态;与订单模块约定「下单时快照地址、之后不再按 identity 反查」,或由地址服务提供带 owner 校验的 `GetByIdentity`。
|
||
10. 迁移与启动解耦(迁移脚本化,`AutoMigrate` 由开关控制),`main` 增加 signal + 超时 `Stop`。
|
||
|
||
**第三批(工程质量,1~3 天,对应 P2-10、P2-13、P2-15、P3-16~19)**
|
||
|
||
11. 补测试矩阵(越权、列表、默认地址并发、非法入参、鉴权、PII 日志断言);补齐 `test/library/delete.http`、`fetch.http` 与真实 `test/rpc/*_test.go`。
|
||
12. `Fetch` 强制分页并删除/启用 `FetchRequest`;对地址列表引入短 TTL 缓存(写路径失效)。
|
||
13. 配置分环境 + 环境变量占位;清理死代码(`cmd/cli`、`rpc.go`、`grpcConns`、`StatusReply.Identity`、`const.proto` 裁剪);修 TS SDK 生成;重写 README(鉴权、接口、错误码、`status` 语义、数据字典)。
|
||
|
||
**建议的目标结构**(分层与归属收敛):
|
||
|
||
```text
|
||
internal/logic/library/ 仅编排:解析 auth → 校验入参 → 调 models 的带归属仓储方法 → 映射 proto
|
||
internal/models/address.go FindByIDForOwner / ListByOwner(分页) / SetDefault(tx) / SoftDeleteForOwner
|
||
internal/models/validate.go validateAddress(长度/格式/值域/行政区划)
|
||
internal/impl/interceptor.go recovery + timeout + logging + auth(scope)
|
||
```
|
||
|
||
## 5. TODO 清单
|
||
|
||
- [ ] **P0-1** `Get`/`Modify`/`Delete` 全部改为按 `owner_identity` 归属查询,并以 `RowsAffected` 判定结果|验收:用 A 的 token 操作 B 的地址 id,三个接口均返回权限/不存在错误且 B 的数据零变更(含单测)|涉及:`internal/logic/library/get.go:29`、`internal/logic/library/modify.go:39`、`internal/logic/library/delete.go:24`
|
||
- [ ] **P0-2** 修 `Fetch` 命名返回值:`reply = &pb.AddressListReply{Data: result}`|验收:造 3 条本人地址后 `Fetch` 返回 3 条,他人地址不出现|涉及:`internal/logic/library/fetch.go:34-40`
|
||
- [ ] **P1-3** 默认地址切换放入事务 + 增加 `(owner_id) WHERE status=2 AND deleted_at IS NULL` 部分唯一索引,并检查降级语句的 `.Error`|验收:并发 20 次 `status=2` 的 `Create` 后仅 1 条默认;`Create` 失败时旧默认仍保留|涉及:`internal/logic/library/create.go:41-45`、`internal/logic/library/modify.go:35-39`、`internal/models/address_library.go:16-17`
|
||
- [ ] **P1-4** 关闭生产 gorm Debug 并对地址字段做日志脱敏|验收:`grep` 日志文件不出现手机号/详细地址明文;`address_prod.yaml` 下 `Debug=false`|涉及:`internal/models/impl.go:82`、`internal/impl/with.go:39`、`etc/address_prod.yaml:5-7`
|
||
- [ ] **P1-5** 统一删除语义并消除跨模块裸读地址表|验收:删除后 `status=-1` 与 `deleted_at` 同时生效、`Fetch/Get` 均不可见;订单侧不再 `Table("address_library")` 直查|涉及:`internal/logic/library/delete.go:24`、`module/ec/order/internal/logic/summary/submit.go:68`
|
||
- [ ] **P1-6** 新增 `validateAddress`(`status` 值域、必填、长度、手机号、行政区划编码),去掉 `int8` 窄化|验收:`status` ∈ {0,3,259,-257} 与超长字段均返回参数错误且不落库|涉及:`internal/logic/library/create.go:39`、`internal/logic/library/modify.go:33`
|
||
- [ ] **P1-7** 为 gRPC 增加 recovery/timeout/logging 拦截器,业务 DB 调用改用 `WithContext(ctx)`|验收:handler 内人为 panic 返回 `codes.Internal` 且进程存活;慢查询在 3s 被取消|涉及:`internal/server/new.go:23`、`cmd/main/main.go:24-34`
|
||
- [ ] **P1-8** 初始化 `Server.Mux` 并注册 `RegisterLibraryHandlerServer`,补 `/healthz`|验收:独立二进制启动后 `POST http://127.0.0.1:12459/address.Library/Fetch` 返回 200 且非 404|涉及:`internal/server/new.go:13-30`、`cmd/main/main.go:30-33`
|
||
- [ ] **P1-9** 校验 JWT 密钥非空且长度合规,禁止回落到 SDK 默认密钥|验收:未配置密钥时启动即失败并给出明确错误;用默认密钥 `Cblocksmesh2022C` 签发的 token 被拒绝|涉及:`internal/config/config.go:35-38`、`etc/address_prod.yaml:27`
|
||
- [ ] **P2-10** `Modify` 支持 `name/pics` 并可显式清空字段,补 `id == 0` 校验与 `Create` 幂等键|验收:`Modify` 能改备注名/图片;空 body 返回参数错误|涉及:`internal/logic/library/modify.go:24-39`
|
||
- [ ] **P2-11** 消除被丢弃的错误与 nil 解引用(`sqlDB, _ :=`、`UpdateColumn` 返回值),删除/修改以 `RowsAffected` 判定|验收:删除不存在/无权限的 id 返回明确错误;`gormDb.DB()` 失败时返回错误而非 panic|涉及:`internal/models/impl.go:63-69`、`internal/logic/library/create.go:42`
|
||
- [ ] **P2-12** 迁移脚本化,`AutoMigrate` 由开关控制,启动失败改为返回错误而非 `Fatalln`|验收:生产配置下启动不执行 DDL;DB 不可用时进程以可读错误退出并完成 `defer` 清理|涉及:`internal/models/impl.go:19-45`
|
||
- [ ] **P2-13** `Fetch` 强制分页并启用/删除 `FetchRequest`,地址列表引入短 TTL 缓存|验收:默认页 20 条、`page_size>100` 被拒绝;写后读缓存一致|涉及:`internal/logic/library/fetch.go:28`、`proto/address.proto:52-56`
|
||
- [ ] **P2-14** 补齐可观测性与优雅退出:健康检查、结构化日志 + `request_id`、signal + 超时 `Stop`|验收:SIGTERM 后 5s 内优雅退出且日志含 request_id;`/healthz` 反映 DB 健康|涉及:`cmd/main/main.go:36-40`、`internal/server/new.go:33-37`
|
||
- [ ] **P2-15** 补测试:越权、`Fetch` 非空、默认地址并发、非法入参、鉴权、PII 日志断言|验收:CI 中 6 组用例全绿,覆盖 `test/library/delete.http`、`fetch.http` 与 `test/rpc/*_test.go`|涉及:`test/rpc/rpc.go:3-23`、`test/library/modify.http:5-7`
|
||
- [ ] **P3-16** 三套环境配置按环境拆分(库地址/日志级别/APM),`Service` 改为 `address`,敏感项改环境变量占位|验收:三份 yaml 哈希不同;CI 校验 `Service == 模块名` 且无 `CHANGE_ME`|涉及:`etc/address_prod.yaml:1-10`
|
||
- [ ] **P3-17** 清理死代码与生成器遗留|验收:删除 `cmd/cli`、`grpcConns`、生成器 TODO;`StatusReply.identity` 回填或移除;`const.pb.go` 体积显著下降|涉及:`cmd/cli/main.go:5-7`、`internal/server/new.go:17`、`internal/logic/library/fetch.go:38`
|
||
- [ ] **P3-18** 修复 TS SDK 生成并重写 README|验收:`createLibraryClient` 暴露 5 个方法且类型与 `address.proto` 一致;README 含鉴权、接口示例、`status` 语义、错误码|涉及:`sdk/typescript/address/index.ts:88-104`、`README.md:1-2`
|
||
- [ ] **P3-19** 规范 HTTP 语义:补 `google.api.http` 注解(GET/DELETE)、拆分 `IdentRequest`、`Create` 增幂等键|验收:`GET /v1/address/{identity}`、`GET /v1/address?page_no=&page_size=` 在网关上可用|涉及:`proto/address.proto:14-21`、`internal/logic/library/get.go:24-29`
|
||
|
||
## 6. 审计摘要(供汇总使用)
|
||
|
||
- 问题数:P0=2 P1=7 P2=6 P3=4(共 19 项,同类已合并:归属校验缺失合并 3 处接口、错误丢弃合并 4 处、PII 日志与明文存储合并为 1 项)
|
||
- 最高风险(一句话):`Get`/`Modify`/`Delete` 三个接口只验 JWT 签名、不校验地址归属,任何登录用户都能按自增 id 枚举读取、篡改、批量删除全站收货人姓名与手机号,而默认配置下这些明文 PII 还会被 gorm Debug 写进 `/data/app/logs/ec-address.log`。
|
||
- 最优先 3 个动作:1) 为 `Get/Modify/Delete` 增加 `owner_identity` 归属过滤并校验 `RowsAffected`(P0-1);2) 修 `Fetch` 的命名返回值使其真正返回数据(P0-2);3) 关闭生产 gorm Debug/对地址字段脱敏,并把默认地址切换改成「事务 + 唯一约束」(P1-4、P1-3)。
|
||
- 未能覆盖/无法验证的部分:运行时与真实 DB 未验证(结论均为静态分析,`Mux=nil` 导致的网关 404 由代码路径推得);生产是否设置 `BSM_JwtSecretKey`、是否部署独立二进制影响 P1-9 的实际可利用性;`address_library` 线上 DDL 是否有外部迁移脚本补齐 `(owner_id,status)` 唯一约束未知;接入层(`gateway`/`proxy`)是否追加鉴权/限流未展开;`utils.UUID()` 的实际实现未展开,identity 熵强度未定量评估。
|