2026-09-22 18:53:53 +08:00
|
|
|
|
# sender(短信与邮件发送)代码审计报告
|
|
|
|
|
|
|
|
|
|
|
|
| 项 | 内容 |
|
|
|
|
|
|
| --- | --- |
|
|
|
|
|
|
| 审计对象 | `module/base/sender` |
|
|
|
|
|
|
| 服务域 | 基础与平台服务 |
|
|
|
|
|
|
| 审计日期 | 2026-09-22 |
|
|
|
|
|
|
| 代码规模 | 手写 Go 文件 18 个、1043 行(其中 `cmd/cli/main.go` 125 行为独立的 SMTP 手工工具、`test/grpc/{sms,mail}_test.go` 共 100 行且均带 `//go:build integration`);`proto` 3 个(`sms.proto` 28 行、`mail.proto` 21 行、`const.proto` 326 行);`pb/` 生成文件 7 个 |
|
|
|
|
|
|
| 入口 | `cmd/main`(gRPC + HTTP Gateway 单进程);聚合入口 `pkgs/all`(`pkgs/all/internal/service/sender.go:10-22`,会传入 `Config: config.Spec.Sender`) |
|
|
|
|
|
|
| 对外协议 | gRPC + REST Gateway,REST 路径 `/sender.Sms/Send`、`/sender.Sms/Verify`、`/sender.Mail/Send`(`pb/sms.pb.gw.go:104,124`、`pb/mail.pb.gw.go:77`) |
|
|
|
|
|
|
| 结论摘要 | **腾讯云短信是空实现**——`internal/logic/sms/send.go:151-153` 的 `TencentSender` 直接 `return nil, nil`,调用后不报错、返回 `"null"`,调用方会误认为发送成功。阿里云通道存在 3 个高危缺陷:日发送量限制**从未生效**(`limitKey` 只有读、无写,`send.go:42-51`)、验证码用 `SetNX` 写入但**忽略返回值**、却把新生成的码发出(`send.go:62-65`),导致"收到的码校验不过";黑名单集合 `BlackListCacheKey` 全仓只有读、无写(`send.go:37`),配置项 `BlackListFilter` 从未被读取。验证码用 `math/rand` 生成(`send.go:169`)且 `Verify` 成功后**不删除**该码(`verify.go:28-32`)→ 可重放;失败反而删除 → 一次输错即作废。邮件只实现了 `qq` 一个渠道(`mail/send.go:55-60`),且 `mail/send.go:89-94` 在 `smtp.NewClient` 出错时对 nil client 调 `Close()`。三个接口在模块 yaml 中全部登记为匿名(`etc/sender_dev.yaml:16-18`),聚合部署白名单未放行。 |
|
|
|
|
|
|
|
|
|
|
|
|
## 1. 服务定位与职责
|
|
|
|
|
|
|
|
|
|
|
|
为其它服务提供**出站消息通道**:短信(阿里云 / 腾讯云)与邮件(SMTP),并附带一套手机验证码的生成/存储/校验能力。它自身不决定"何时发什么",由业务方传入 `provider`/`template_code`/`sign_name`(短信)或 `template_key`(邮件)与模板参数;模板正文对邮件存放在 `sender_template` 表,对短信存放在阿里云侧的模板 ID。
|
|
|
|
|
|
|
|
|
|
|
|
## 2. 代码结构与入口
|
|
|
|
|
|
|
|
|
|
|
|
| 路径 | 职责 |
|
|
|
|
|
|
| --- | --- |
|
|
|
|
|
|
| `cmd/main/main.go` | 独立进程入口。`ServiceKey = "sender"`(`:17`);`config.New` → `impl.NewImpl` → `server.New(nil)`(`:29`)→ `service.New(...)` + `srv.Start()`(`:32-48`) |
|
|
|
|
|
|
| `cmd/cli/main.go` | 125 行的独立 SMTP 手工工具:硬编码 `smtp.exmail.qq.com:465`(`:13-14`)、发件人 `yanweidong@senlinai.com`(`:89`)、凭据取自环境变量 `BSM_SMTP_USER`/`BSM_SMTP_PASSWORD`/`BSM_SMTP_TO`(`:34-38`),带 3 次重试(`:68`) |
|
|
|
|
|
|
| `internal/config/config.go` | `SrvConfig`(`:17-28`)+ `SmtpConf`(`:31-38`)/`SmsConf`(`:41-46`)/`codeConf`(`:49-56`);`New` 只校验 `conf.NotNil(Spec.Service, Spec.Cache)`(`:70`) |
|
|
|
|
|
|
| `internal/impl/impl.go` | 初始化 `MemorySerice`(拼写错误,`:16,22`)/`RedisService`/`DBService`/`EtcdService`,最后调 `withProvider()`(`:30`) |
|
|
|
|
|
|
| `internal/impl/provider.go` | 服务商客户端构建:`withProvider` 按 yaml 键名匹配 `google`/`qq`/`aliyun`/`tencent`(`:31-50`);`NewSMTP` **恒返回 nil**(`:52-54`);`NewAliyun`(`:56-79`)/`NewTencent`(`:81-92`)出错即 `panic` |
|
|
|
|
|
|
| `internal/logic/sms/send.go` | `Send`(`:26-100`)→ 校验手机号/模板码 → 黑名单 → 日限流 → 生成/取验证码 → 分派 `AliyunSender`(`:102-149`)/`TencentSender`(`:151-153`) |
|
|
|
|
|
|
| `internal/logic/sms/verify.go` | `Verify`(`:12-40`):比对 Redis 中的验证码 |
|
|
|
|
|
|
| `internal/logic/sms/const.go` | 4 个 Redis 键常量(`:3-8`) |
|
|
|
|
|
|
| `internal/logic/mail/send.go` | `Send`(`:22-69`):查模板 → 渲染 → `QQ`(`:76-152`);`ValidateEmail`(`:71-74`) |
|
|
|
|
|
|
| `internal/models/sender_template.go` | `sender_template` 表模型(`Title`/`Key`/`Subjet`/`Body`,`:10-13`);字段名 `Subjet` 为拼写错误 |
|
|
|
|
|
|
| `internal/server/{sms_server,mail_server,new}.go` | 把 `Sms`/`Mail` 注册到 gRPC(`new.go:33-34`) |
|
|
|
|
|
|
| `internal/excode/ex.go` | 11 个错误码;其中 `ErrAppName`/`ErrMustWhiteList`/`ErrExpired` 无引用(见 6.3) |
|
|
|
|
|
|
| `internal/routers` | **不存在**(走 gRPC + HTTP Gateway,无 Gin 路由) |
|
|
|
|
|
|
| `service/{expose,dependencies}.go` | 聚合宿主注入点:`Expose` 覆盖 `config.Spec`(`expose.go:24-26`)并注册 Gateway handler(`:30-35`) |
|
|
|
|
|
|
| `proto/{sms,mail,const}.proto` | 前两个定义本模块 3 个 rpc;`const.proto` 为本模块无关的共享消息 |
|
|
|
|
|
|
| `etc/sender_{dev,prod,test}.yaml` | Port 12208 / Gateway 12207;**prod 与 test 均为 dev 的近似副本**(本机库/Redis、占位 AK) |
|
|
|
|
|
|
| `test/grpc/{sms,mail}_test.go` | 均带 `//go:build integration`(`sms_test.go:1`、`mail_test.go:1`),默认打真实地址(`sms_test.go:17` 为 `192.168.31.148:12208`) |
|
|
|
|
|
|
| `test/http/*.http` | 3 个手工请求样例,指向 `http://api.apinb.com/sender.{Sms,Mail}/*` |
|
|
|
|
|
|
| `README.md` / `UPGRADE_SUMMARY.md` | 与实现不符的文档(见 6.4) |
|
|
|
|
|
|
|
|
|
|
|
|
## 3. 接口清单
|
|
|
|
|
|
|
|
|
|
|
|
| 方法 | 路径 | 功能 | 鉴权 | 实现位置 |
|
|
|
|
|
|
| --- | --- | --- | --- | --- |
|
|
|
|
|
|
| POST | `/sender.Sms/Send` | 发送短信(可顺带生成验证码) | 无(模块 yaml 登记匿名 `etc/sender_dev.yaml:17`) | `internal/logic/sms/send.go:26` ← `internal/server/sms_server.go:18` |
|
|
|
|
|
|
| POST | `/sender.Sms/Verify` | 校验手机验证码 | 无(`etc/sender_dev.yaml:18`) | `internal/logic/sms/verify.go:12` ← `sms_server.go:22` |
|
|
|
|
|
|
| POST | `/sender.Mail/Send` | 按模板发送邮件 | 无(`etc/sender_dev.yaml:16`) | `internal/logic/mail/send.go:22` ← `internal/server/mail_server.go:18` |
|
|
|
|
|
|
| POST | gRPC `/sender.Sms/Send|Verify`、`/sender.Mail/Send` | 同上(同一 handler) | 同上 | `internal/server/new.go:33-34` |
|
|
|
|
|
|
|
|
|
|
|
|
**声明但未实现/占位的接口**:无"声明了但 server 未实现"的方法(`proto/sms.proto:6-9`、`proto/mail.proto:6-8` 与 `internal/server/*_server.go` 完全对应)。但**已声明并已接入路由的 provider 存在空实现**:
|
|
|
|
|
|
|
|
|
|
|
|
| 声明位置 | 空实现位置 | 表现 |
|
|
|
|
|
|
| --- | --- | --- |
|
|
|
|
|
|
| `proto/sms.proto:13` 的 `provider` 字段(`send.go:82-86` 分派 `tencent`) | `internal/logic/sms/send.go:151-153` `func TencentSender(args *pb.SmsSendRequest) (map[string]any, error) { return nil, nil }` | `result == nil && err == nil` → `send.go:95-99` 返回 `Reply: "null"` 且无错误,**调用方误判为发送成功** |
|
|
|
|
|
|
| `internal/impl/provider.go:35-38` 匹配 `google` SMTP | `internal/impl/provider.go:52-54` `func NewSMTP(...) *smtp.Client { return nil }` | `Provider.Google`/`Provider.QQ` 恒为 nil(`mail.Send` 未使用 Provider,故该结构为死代码) |
|
|
|
|
|
|
| `internal/logic/mail/send.go:55-60` 的 provider switch 只有 `case "qq"` | 无 `google`/其它分支 | 传 `provider=gmail` 直接 `ErrNotProvider`(`test/grpc/mail_test.go:37` 正是这么调的) |
|
|
|
|
|
|
|
|
|
|
|
|
> 鉴权补充:与 `initial` 同构的三重问题——① `etc/sender_dev.yaml:14` 为 `MicroService.Enable: false`,匿名清单不会写入 etcd(`D:\work\bsm-sdk\core\service\service.go:66-90`);② 独立部署时模块网关把 `GatewayMux` 直接交给 `http.ListenAndServe`(`service.go:106-111`、`:120-129`),**不带任何鉴权中间件**,因此这三个接口对网络可达方完全开放;③ 聚合部署时 `pkgs/all/etc/default_dev.yaml:24-43` 未放行任何 `/sender.*`,与模块 yaml 的匿名声明矛盾;④ 即便改 `Enable: true`,清单条目写作 `sender.Sms.Send`(`etc/sender_dev.yaml:17`),而框架发现的方法是 `Sms.Send`(`service.go:149-158`)、网关规范化路径是 `/sender.Sms/Send`(`authorization.go:105-112`)——三种格式互不相等,永远不命中。
|
|
|
|
|
|
|
|
|
|
|
|
## 4. 数据模型与表
|
|
|
|
|
|
|
|
|
|
|
|
### 表 `sender_template`(`internal/models/sender_template.go:8-14`)
|
|
|
|
|
|
|
|
|
|
|
|
| 字段 | 类型 | 约束 | 说明 |
|
|
|
|
|
|
| --- | --- | --- | --- |
|
|
|
|
|
|
| `id`/`created_at`/`updated_at`/`deleted_at` | uint / timestamp | PK / 软删 | 来自 `types.Std_IICUDS`(`:9`) |
|
|
|
|
|
|
| `title` | varchar(255) | not null default '' | 模板标题 |
|
|
|
|
|
|
| `key` | varchar(100) | not null **uniqueIndex** | 模板标识,`mail.Send` 按它查询(`mail/send.go:43`) |
|
|
|
|
|
|
| `subjet` | varchar(255) | not null default '' | 邮件主题(字段名拼写错误,`:12`) |
|
|
|
|
|
|
| `body` | text | not null | 模板内容,交给 `html/template` 解析(`mail/send.go:49`) |
|
|
|
|
|
|
|
|
|
|
|
|
> 该表**没有启用/停用字段**,也没有"渠道/语言/场景"维度(`mail/send.go:43` 只按 `key` 取一条)。短信侧无任何本地表:模板 ID 与签名由调用方每次传入(`proto/sms.proto:14-15`)。
|
|
|
|
|
|
|
|
|
|
|
|
### Redis 键(`internal/logic/sms/const.go:3-8`)
|
|
|
|
|
|
|
|
|
|
|
|
| 常量 | 值 | 用途 | 实际读写情况 |
|
|
|
|
|
|
| --- | --- | --- | --- |
|
|
|
|
|
|
| `KeyPrefix` | `/SMS/Code/` | 验证码:`/SMS/Code/{phone}` | 写 `send.go:65`(`SetNX`)、读 `verify.go:23`、删 `verify.go:35` |
|
|
|
|
|
|
| `BlackListCacheKey` | `/SMS/BlackList/` | 手机号黑名单(Set) | **只有读**(`send.go:37`,`SIsMember`);全仓无任何写入点 → 黑名单永远为空 |
|
|
|
|
|
|
| `LimitCacheKey` | `/SMS/LimitCacheKey/` | 日发送计数:`/SMS/LimitCacheKey/{yyyy-mm-dd}{phone}` | **只有读**(`send.go:42,45`);全仓无任何 `INCR`/`Set` → 计数恒为 0 |
|
|
|
|
|
|
| `FormatDay` | `2006-01-02` | 日限流键的日期片段 | — |
|
|
|
|
|
|
|
|
|
|
|
|
## 5. 核心流程
|
|
|
|
|
|
|
|
|
|
|
|
```mermaid
|
|
|
|
|
|
flowchart TD
|
|
|
|
|
|
A["POST /sender.Sms/Send"] --> B["校验 phone 非空且匹配正则,否则 ErrPhone"]
|
|
|
|
|
|
B --> C["校验 template_code 非空,否则 ErrTemplate"]
|
|
|
|
|
|
C --> D["SISMEMBER /SMS/BlackList/{phone}"]
|
|
|
|
|
|
D -->|"命中"| E["返回 ErrInBlackList"]
|
|
|
|
|
|
D -->|"未命中"| F["GET /SMS/LimitCacheKey/{date}{phone} 取 twice"]
|
|
|
|
|
|
F --> G{"twice > Code.MaxSentLimit"}
|
|
|
|
|
|
G -->|"是"| H["返回 ErrSentLimit"]
|
|
|
|
|
|
G -->|"否"| I{"is_gen_code"}
|
|
|
|
|
|
I -->|"true"| J["GenValidateCode 生成新码,SETNX 写入 Redis(忽略返回值)"]
|
|
|
|
|
|
I -->|"false"| K["从 paramters[code] 取调用方传入的码"]
|
|
|
|
|
|
J --> L["按 provider 分派:aliyun 走阿里云 OpenAPI"]
|
|
|
|
|
|
K --> L
|
|
|
|
|
|
L --> M{"provider"}
|
|
|
|
|
|
M -->|"tencent"| N["TencentSender 返回 nil, nil(空实现)"]
|
|
|
|
|
|
M -->|"其它"| O["返回 ErrNotProvider"]
|
|
|
|
|
|
L --> P["json.Marshal(result) 后作为 Reply 返回"]
|
|
|
|
|
|
N --> P
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
## 6. 审计发现
|
|
|
|
|
|
|
|
|
|
|
|
### 6.1 安全
|
|
|
|
|
|
|
|
|
|
|
|
| 级别 | 位置 | 问题 |
|
|
|
|
|
|
| --- | --- | --- |
|
|
|
|
|
|
| 高 | `internal/logic/sms/verify.go:28-32` | **验证码可重放**:`if code == in.Code { return "true" }` 命中后**不删除** Redis 键,仅在失配路径才 `Del`(`:34-35`)。有效期 300s(`etc/sender_dev.yaml:46`)内同一验证码可无限次校验通过;结合下面"无尝试次数限制",`Verify` 被暴力枚举成功后仍可反复使用。注释 `//verify pass ; delete the requestId`(`:34`)与实际行为(删的是失败分支)相反,说明是写错分支。 |
|
|
|
|
|
|
| 高 | `internal/logic/sms/verify.go:12-40` | **无失败次数限制/无锁定**:`Verify` 不统计失败次数、不锁定手机号、无验证码错误计数,6 位数字码在 300s 内可被离线暴力枚举。且验证码键**只用手机号**(`const.go:5`,`verify.go:22`),不区分 `app`/`template_code`,同一手机号在多业务间共用一枚码,任一业务泄漏即全部可验证。 |
|
|
|
|
|
|
| 高 | `internal/logic/sms/send.go:37-39`(读)+ `const.go:6` | **黑名单形同虚设**:判定逻辑存在(`SIsMember` → `ErrInBlackList`),但 `/SMS/BlackList/` 集合在**整个仓库内没有任何写入点**;配置项 `Code.BlackListFilter`(`internal/config/config.go:55`,yaml `etc/sender_dev.yaml:49-50`、`etc/sender_prod.yaml:49-50`)也**从未被代码读取**(全仓检索仅命中定义处与 yaml)。结论:无法通过该机制拦截任何号码。 |
|
|
|
|
|
|
| 高 | `internal/logic/sms/send.go:41-51`(读) | **日发送量限制从未生效**:`limitKey := LimitCacheKey + 日期 + 手机号` 只被 `Get`(`:45`)读取,全仓没有任何 `Incr`/`Set` 写入该键,`twice` 恒为 0,`twice > MaxSentLimit`(`:49`)恒为假。任意外部调用方可在一天内对同一号码无限次发送短信(成本与骚扰风险)。注意判断本身也应是 `>=`(当前语义为"已超过 N 次才拒",即实际允许 N+1 次)。 |
|
|
|
|
|
|
| 高 | `internal/logic/sms/send.go:66-73`、`:104-109` | **验证码内容由调用方控制**:`is_gen_code=false` 时直接从 `in.Paramters["code"]` 取"验证码"(`:68-71`);即使 `is_gen_code=true`,`AliyunSender` 在设置好 `templateParam["code"]` 之后又用 `args.Paramters` **覆盖同名键**(`:107-109`)。因此调用方可让短信正文携带任意 `code`(配合 `sign_name`/`template_code` 自填)= 把短信通道当成任意内容/任意签名发送器(配合 6.1 前三条约等于开放短信网关)。 |
|
|
|
|
|
|
| 中 | `etc/sender_dev.yaml:16-18`、`etc/sender_prod.yaml:16-18`、`etc/sender_test.yaml:16-18` | 三个接口全部登记匿名;叠加 6.1 的开放发送能力,未鉴权即可调用。且 `Verify` 对所有号码可探测(返回 `true`/`false`),可用来枚举"哪些手机号正在流程中"。 |
|
|
|
|
|
|
| 中 | `etc/sender_dev.yaml:40` | 仓库内提交了疑似真实的阿里云 AccessKeyId `LTAI5tKEmKuuoixE4iw8NZbX`(`AccessKeySecret` 已脱敏为 `CHANGE_ME`)。KeyId 本身不应入库;若该 Key 仍有效,应连同 Secret 一并轮换并从历史中清理。 |
|
|
|
|
|
|
| 中 | `internal/logic/sms/verify.go:23-26` | Redis 读取失败时把底层错误原文回给调用方:`errcode.NewError(1311, err.Error())`,泄漏内部错误文本;同时"验证码不存在/已过期"也走这条路径,而专用错误码 `excode.ErrExpired`(`internal/excode/ex.go:14`)定义了却没用。 |
|
|
|
|
|
|
| 中 | `internal/logic/sms/send.go:96,146-147` | 用 `fmt.Println` 直接把**含手机号与模板参数**的请求体、以及响应结果打到 stdout(并会被 supervisor 落盘到 `etc/supervisor.bsm-apps-sender.conf:8` 的 `/data/app/logs/apps-sender.log`)。手机号属个人敏感信息,日志中不应明文记录。 |
|
|
|
|
|
|
| 中 | `internal/logic/mail/send.go:127-129` | 邮件头用字符串拼接构造:`"Subject: " + subject + "\n\n"`,`subject` 来自数据库模板(`mail/send.go:57`,`tplRecord.Subjet`)且未做 CRLF 过滤。若模板可被管理端写入 `\r\n`,即形成邮件头注入(可注入 `Bcc:` 等)。同时未设置 `Content-Type`/`MIME-Version`/`Content-Transfer-Encoding`,中文主题/正文按裸 UTF-8 发送,多数客户端会乱码。 |
|
|
|
|
|
|
| 低 | `internal/logic/mail/send.go:43-46` | 模板查询只按 `key` 精确匹配、无启用/渠道维度;模板 `body` 直接交给 `html/template` 解析(`:49`),DB 内容的可信任边界未在代码中约束。`html/template` 会转义变量,故不构成模板注入,但纯文本邮件会因转义出现 `&` 一类字符。 |
|
|
|
|
|
|
| 低 | `internal/logic/sms/send.go:29,155-159` | 手机号校验仅一条正则(`^(1[3|4|5|6|7|8|9][0-9]\d{4,8})$`),无长度上限与号段白名单;`in.GetPhone()` 未做去空格/去 `+86` 归一,同一真实号码可用 `+86138…`、`86138…` 等写法绕过按号码的黑名单/限流(在其失效前)。 |
|
|
|
|
|
|
| 低 | `etc/supervisor.bsm-apps-sender.conf:6` | 进程以 `user=root` 运行。 |
|
|
|
|
|
|
|
|
|
|
|
|
### 6.2 正确性与逻辑缺陷
|
|
|
|
|
|
|
|
|
|
|
|
| 级别 | 位置 | 问题 |
|
|
|
|
|
|
| --- | --- | --- |
|
|
|
|
|
|
| 高 | `internal/logic/sms/send.go:62-65` + `:81` | **发出的验证码可能不是存入 Redis 的那一个**。生成新码时用 `SetNX` 写入(`:65`),**完全忽略返回值**;若该手机号的键仍存在(上一次发送距今不足 `Expire`=300s),`SetNX` 不生效、Redis 保留**旧码**,而紧接着 `smsCode`(新码)被传给 `AliyunSender`(`:81`)发到用户手机上。用户收到新码却校验不过,只有等旧码过期重新发送才可能成功。正确做法是 `Set`(覆盖)或在使用 `SetNX` 的成功分支才把该码发出。 |
|
|
|
|
|
|
| 高 | `internal/logic/sms/send.go:151-153` | **腾讯云短信为空实现**(按任务核实项,确认):函数体只有 `return nil, nil`。`send.go:83-85` 的 `impl.Provider.Tencent == nil` 前置判断也拦不住它(`internal/impl/provider.go:46-48` 在配置存在 tencent 时已创建非 nil 客户端),因此 `provider=tencent` 时接口返回 HTTP 200 + `Reply: "null"`,调用方无从知晓短信根本没发出去。 |
|
|
|
|
|
|
| 中 | `internal/logic/sms/send.go:161-176` | 验证码用**非密码学安全**的 `math/rand`:`rand.Seed(time.Now().UnixNano())`(`:169`)+ `rand.Intn(10)`(`:173`)。种子与时间强相关,验证码可被预测/爆破(配合 6.1 无尝试限制)。应改用 `crypto/rand`。另 `width == 0` 时兜底为 4 位(`:163-165`),而调用侧在配置不合法时兜底为 6 位(`:57-59`),两处默认值不一致。 |
|
|
|
|
|
|
| 中 | `internal/logic/sms/send.go:55-59` | 配置不合法时**静默改写运行时配置**:`config.Spec.Code.Length < 4 \|\| > 10` 时把全局 `config.Spec.Code.Length` 直接改成 6(`:58`)。这是对共享可变状态的写操作,并发请求下互相影响,且掩盖了配置错误(既不报错也不打日志)。 |
|
|
|
|
|
|
| 中 | `internal/logic/sms/send.go:75-99` | `result, err` 的取值不做 `nil` 判定就 `json.Marshal(result)`(`:95`),`nil` 会序列化成 `"null"` 并作为 `Reply` 返回(`:98`)。除腾讯空实现外,任何"返回 nil, nil"的实现都会被当作成功。同时 `err` 直接向上抛(`:91-93`),未按渠道归一成模块错误码。 |
|
|
|
|
|
|
| 中 | `internal/logic/sms/send.go:41-42` | 限流键把日期与手机号直接拼接(`LimitCacheKey + "2006-01-02" + phone`),无分隔符:`/SMS/LimitCacheKey/2026-09-22` + `13800138000`。本身不影响正确性,但键的可读性与后续按号段/前缀统计都变差;一旦将来改成"先日期后号码"以外的顺序,历史计数全部错位。 |
|
|
|
|
|
|
| 中 | `internal/logic/mail/send.go:89-94` | **nil 指针解引用**:`client, err := smtp.NewClient(...)` 出错时,下一行立即 `client.Close()`(`:91`)。此时 `client` 为 nil,一旦 `smtp.NewClient` 返回错误(如服务端在握手阶段即断开)将 panic,而不是返回错误。 |
|
|
|
|
|
|
| 中 | `internal/logic/mail/send.go:119-124` 与 `:146-150` | `writer` 被关闭两次:`:124` 有 `defer writer.Close()`,`:146` 又显式 `writer.Close()`。第二次 `Close` 的错误会被当作发送失败返回(`:146-149`),同一封邮件可能被误报失败。 |
|
|
|
|
|
|
| 中 | `internal/logic/mail/send.go:151` | 成功路径**没有调用 `client.Quit()`/`Close()`**,也未关闭 `tls.Conn`(`:78`)。每次发信都会残留一条未正常关闭的 TLS 连接,依赖 GC 回收;持续发信会堆积连接/文件描述符(对比 `cmd/cli/main.go:120` 的正确写法:成功时 `client.Quit()`)。 |
|
|
|
|
|
|
| 低 | `internal/logic/sms/send.go:45-48` | `Get(...).Int()` 用 `!errors.Is(err, redis.Nil)` 把"键不存在"(正常,视为 0 次)与真错误区分开——这部分写法正确(`redis.Nil` 由 SDK 导出,`D:\work\bsm-sdk\core\cache\redis\redis.go:16`)。 |
|
|
|
|
|
|
| 低 | `internal/logic/sms/send.go:57-59` | 长度校验用 `int64` 比较 `config.Spec.Code.Length`,与 `codeConf.Length int64`(`config.go:50`)一致;但 `yaml` 中未配置该字段时为 0,会被静默改成 6,调用方无法感知配置缺失。 |
|
|
|
|
|
|
| 低 | `internal/logic/mail/send.go:22-39` | 参数校验顺序为"先查 provider 配置(`:31-34`)再校验收件人格式(`:37-39`)",`To` 为空时已在 `:26-28` 抛 `ErrInvalidArgument`,但格式错误统一返回 `ErrEmail`(`excode:16`),与"provider 未配置"(`ErrProviderIsNil`)的语义边界清晰,无功能问题。 |
|
|
|
|
|
|
| 低 | `internal/logic/sms/send.go:14`、`internal/logic/mail/send.go:18` | `ctx` 仅被透传给 Redis(`send.go:37/45/65`),未用于任何 HTTP/OpenAPI 调用(阿里云 `CallApi` 未接收 ctx),上游超时无法取消一次正在进行的短信发送。 |
|
|
|
|
|
|
| 低 | `internal/models/sender_template.go:12` | 字段名 `Subjet` 拼写错误(应为 `Subject`),直接映射为列名 `subjet`,与 `mail/send.go:57` 的引用形成"错得一致",但会成为对外数据契约的长期负担。 |
|
|
|
|
|
|
|
|
|
|
|
|
### 6.3 未完成实现
|
|
|
|
|
|
|
|
|
|
|
|
- **`TencentSender` 空实现**:`internal/logic/sms/send.go:151-153`,见 6.2。`internal/impl/provider.go:81-92` 的 `NewTencent` 客户端已完整构建并挂在 `Provider.Tencent` 上,唯独发送函数是空的 → 属于"接线完成、逻辑未写"。
|
|
|
|
|
|
- **`NewSMTP` 空实现**:`internal/impl/provider.go:52-54` 恒返回 nil,导致 `ProviderClient.QQ`(`:24`)与 `ProviderClient.Google`(`:22`)永远为 nil。`internal/logic/mail/send.go` 并未使用这两个字段,所以它们是**死代码**;`UPGRADE_SUMMARY.md:1-192` 声称的"完整容器化与工具链"也与此无关。
|
|
|
|
|
|
- **配置项定义了但从未被读取**(全仓检索,仅命中定义处与 yaml):
|
|
|
|
|
|
- `Code.GenerateCode`(`internal/config/config.go:53`,yaml `etc/sender_prod.yaml:47`、`etc/sender_test.yaml:46`)——实际是否生成验证码只取决于请求字段 `is_gen_code`(`send.go:55`)。
|
|
|
|
|
|
- `Code.CokeyKey`(`config.go:54`,yaml 三处均为 `code`)——真实键名硬编码在 `send.go:104` 的 `templateParam["code"]`。
|
|
|
|
|
|
- `Code.BlackListFilter`(`config.go:55`)——见 6.1。
|
|
|
|
|
|
- **错误码定义后无引用**:`ErrAppName`(`internal/excode/ex.go:8`)——`SmsSendRequest` 里根本没有 app 字段;`ErrMustWhiteList`(`:11`)——白名单逻辑不存在;`ErrExpired`(`:14`)——过期路径走的是 `1311`(`verify.go:25`)。
|
|
|
|
|
|
- **`MemorySerice`(`internal/impl/impl.go:16,22`)**:本模块内除赋值外无任何读取点(`service/dependencies.go:30` 是唯一另一处赋值)→ 内存缓存层初始化后不使用。
|
|
|
|
|
|
- **`test/grpc/{sms,mail}_test.go` 不是可 CI 执行的测试**:均带 `//go:build integration`(`sms_test.go:1`、`mail_test.go:1`),且依赖真实地址(`sms_test.go:17` 内网 IP、`mail_test.go:19` `api.apinb.com:10020`)。`internal/**` 下**无任何 `*_test.go`**——`Send`/`Verify`/`GenValidateCode`/`ValidateEmail` 四个纯函数零单元测试覆盖。
|
|
|
|
|
|
- **`test/grpc/mail_test.go:37` 使用 `Provider: "gmail"`**,而 `mail/send.go:55-60` 只实现 `qq`,该用例即使跑起来也必然返回 `ErrNotProvider` → 用例与实现不一致(说明 Gmail 渠道曾是计划项)。
|
|
|
|
|
|
- **`cmd/cli/main.go`** 与短信/邮件业务完全平行的一套 SMTP 实现(自带重试与 `Quit`),既不复用 `internal/logic/mail`,也不被任何代码引用 → 独立的遗留工具。
|
|
|
|
|
|
- **`proto/const.proto:1-326`** 定义 `OrderSummaryItem`、`FeedPostItem`、`MarketLoginReply`、`CmsCategoryItem` 等与本模块无关的消息,未被引用 → 死定义。
|
|
|
|
|
|
|
|
|
|
|
|
### 6.4 健壮性与可维护性
|
|
|
|
|
|
|
|
|
|
|
|
| 级别 | 位置 | 问题 |
|
|
|
|
|
|
| --- | --- | --- |
|
|
|
|
|
|
| 高 | `internal/logic/sms/send.go:49`、`:57` | **`config.Spec.Code` 未做非空校验即可解引用**:`codeConf` 是指针(`config.go:27`),`config.New` 只 `conf.NotNil(Spec.Service, Spec.Cache)`(`config.go:70`)。当配置(模块 yaml 或聚合传入的 `pkgs/all` `Sender` 段)缺少 `Code` 时,`config.Spec.Code.MaxSentLimit` 会在第一次调用 `Send` 时 panic(`:49`),而不是启动期报错。`pkgs/all/etc/default_dev.yaml:113-118` 目前有该段,但不构成代码层保证。 |
|
|
|
|
|
|
| 高 | `pkgs/all/etc/default_dev.yaml:98-118` vs `internal/logic/sms/send.go:76-89`、`internal/logic/mail/send.go:31-34` | **聚合配置的 provider 键名与代码期望不匹配**:聚合配置只有 `Sender.SMTP.default` 与 `Sender.SMS.default`,而代码要求键名等于请求传入的 `provider`(`mail/send.go:31` 用 `config.Spec.SMTP[provider]` 直接查表;`sms/send.go:78-80` 需要 `Provider.Aliyun`/`Provider.Tencent` 非 nil,而 `impl/provider.go:42-49` 只在键名为 `aliyun`/`tencent` 时才创建客户端)。结果是聚合部署下短信恒返回 `ErrProviderIsNil`(`:79`、`:84`),邮件要传 `provider=default` 才能走通(与 README/用例中的 `qq`/`gmail` 都不符)。 |
|
|
|
|
|
|
| 中 | `internal/impl/provider.go:56-79`、`:81-92` | 构建客户端失败时 `panic`(`:65`、`:75`、`:88`),且 `panic(err)` 不带上下文,启动期崩溃时无法定位是哪个 provider 的哪一步;`internal/impl/impl.go:30` 在 `NewImpl` 中同步调用,属"启动即崩"路径。 |
|
|
|
|
|
|
| 中 | `etc/sender_prod.yaml:7,10` | 生产配置的 `Databases.Source` 与 `Cache` 仍指向 `127.0.0.1`(`:7`、`:10`),与 `sender_dev.yaml` 完全一致。生产若直接使用该文件会连本机库/本机 Redis。 |
|
|
|
|
|
|
| 中 | `etc/sender_prod.yaml:37-40` | 生产短信配置明显是复制错误且未填真值:`SMS.aliyun.Endpoint: smtp.gmail.com`(`:38`,应为 `dysmsapi.aliyuncs.com`),`AccessKeyId: <your-access-key-id>`、`AccessKeySecret: <your-access-key-secret>`(`:39-40`)为占位符。同时三份 yaml 都没有 `tencent` 段,`TencentSender` 即使实现也无可配置的客户端。 |
|
|
|
|
|
|
| 中 | `README.md:155,170` | README 给出的调用地址 `/v1/sms/send`、`/v1/mail/send`(及 `README.md:151` 的 `http://localhost:12202/swagger/`)与真实路径 `/sender.Sms/Send`、`/sender.Mail/Send`(`pb/sms.pb.gw.go:104,124`、`pb/mail.pb.gw.go:77`)不一致,照文档调用必然 404。 |
|
|
|
|
|
|
| 低 | `README.md:9,18` | README 宣称"腾讯云短信""多服务商:支持QQ邮箱、Gmail",与实现(腾讯空实现、邮件仅 `qq`)不符。 |
|
|
|
|
|
|
| 低 | `README.md:149-151`、`:361-365` | README 声明 gRPC `12201` / HTTP `12202`、默认端口 `12201`,而实际 yaml 为 gRPC `12208` / Gateway `12207`(`etc/sender_dev.yaml:2`、`:23`);对不上。 |
|
|
|
|
|
|
| 低 | `README.md:63-103` | README 的配置样例使用 `Service.Name/Port` 嵌套结构(`:63-66`)和 `Databases.Default.{Host,Port,...}`(`:69-76`)的形式,与 `internal/config/config.go:17-28` 实际支持的结构(`Service: <string>` + `Databases.Source: [...]` 连接串)完全不同,属虚构配置。 |
|
|
|
|
|
|
| 低 | `README.md:352,379` | 声明 `GET /health` 健康检查端点,本模块无任何注册。 |
|
|
|
|
|
|
| 低 | `UPGRADE_SUMMARY.md:19-31,60-68,184-192` | 声称新增了 `Dockerfile`、`docker-compose.yml`、`Makefile`、`CHANGELOG.md`(并称"所有改进都保持了向后兼容性,可以安全地在生产环境中部署"),但仓库中**这四个文件都不存在**(本模块实际只有 `cmd/ internal/ pb/ proto/ etc/ test/ service/` 与 `README.md`/`UPGRADE_SUMMARY.md`)→ 文档不实描述。 |
|
|
|
|
|
|
| 低 | `internal/config/config.go:70` | 与 `ads`/`initial` 相同:`conf.NotNil` 只覆盖 `Service`/`Cache`,未覆盖 `Databases` 与 `Code`;`with.Databases`(`D:\work\bsm-sdk\core\with\databases.go:13-15`)在 `Source` 为空时直接 panic。 |
|
|
|
|
|
|
| 低 | `internal/impl/impl.go:16,22` | 变量名拼写错误 `MemorySerice`(应为 `MemoryService`);同 workspace 的 `pkgs/all/internal/impl` 用正确拼写(`pkgs/all/internal/service/sender.go:16`)。 |
|
|
|
|
|
|
| 低 | `service/expose.go:24-26` | `config.Spec = *options.Config` 直接整份覆盖全局配置;若宿主只填了部分字段(如只给 `SMTP`),其余字段(含 `Code`)会变成零值 → 触发上面第 1 条的 panic。 |
|
|
|
|
|
|
|
|
|
|
|
|
## 7. 风险汇总
|
|
|
|
|
|
|
|
|
|
|
|
| 编号 | 级别 | 问题 | 影响面 |
|
|
|
|
|
|
| --- | --- | --- | --- |
|
|
|
|
|
|
| S1 | 高 | 腾讯云短信为空实现,返回 `"null"` 且无错误(`send.go:151-153`) | 业务静默失效、用户收不到短信却认为已发送 |
|
|
|
|
|
|
| S2 | 高 | 日发送量限制从未生效(`limitKey` 无写入,`send.go:42-51`) | 短信轰炸、短信费用失控 |
|
|
|
|
|
|
| S3 | 高 | 验证码 `SetNX` 忽略返回值但发送新码(`send.go:62-65,81`) | 收码即校验失败,登录/注册流程不可用 |
|
|
|
|
|
|
| S4 | 高 | 黑名单无写入点、`BlackListFilter` 未读取(`send.go:37`) | 拦截机制形同虚设 |
|
|
|
|
|
|
| S5 | 高 | 验证码可重放 + 无尝试次数限制 + 键仅按手机号(`verify.go:28-32`) | 验证码可爆破、跨业务复用 |
|
|
|
|
|
|
| S6 | 高 | 验证码内容可由调用方指定/覆盖(`send.go:66-73,104-109`) | 短信通道被当作任意内容发送器 |
|
|
|
|
|
|
| S7 | 高 | `config.Spec.Code` 未校验即解引用(`send.go:49`) | 缺配置时运行期 panic |
|
|
|
|
|
|
| S8 | 高 | 聚合配置 provider 键为 `default`,代码要求 `qq`/`aliyun`/`tencent` | 聚合部署下短信恒 `ErrProviderIsNil` |
|
|
|
|
|
|
| S9 | 中 | 验证码用 `math/rand` + 时间种子(`send.go:161-176`) | 验证码可预测 |
|
|
|
|
|
|
| S10 | 中 | 三接口匿名 + 独立部署网关无鉴权中间件;聚合白名单未放行 | 未鉴权调用发送能力 / 上线 401 |
|
|
|
|
|
|
| S11 | 中 | 邮件构造:`client` nil 时 `Close()`(`mail/send.go:91`)、`writer` 双关闭(`:124,146`)、成功不 `Quit`(`:151`) | panic、误报失败、连接泄漏 |
|
|
|
|
|
|
| S12 | 中 | 请求参数(含手机号)与结果明文 `fmt.Println`(`send.go:96,146-147`) | PII 落盘、日志泄露 |
|
|
|
|
|
|
| S13 | 中 | `etc/sender_prod.yaml` 为本机地址 + 错误 Endpoint + 占位 AK | 生产部署误连本机/配置无效 |
|
|
|
|
|
|
| S14 | 中 | 仓库内提交疑似真实阿里云 AccessKeyId(`etc/sender_dev.yaml:40`) | 凭据泄露 |
|
|
|
|
|
|
| S15 | 低 | 死代码/死配置(`NewSMTP`、`MemorySerice`、3 个未读配置项、3 个未用错误码、`const.proto`、`cmd/cli`)+ 无 `*_test.go` | 可维护性、无法回归验证 |
|
|
|
|
|
|
| S16 | 低 | README / UPGRADE_SUMMARY 与实现大面积不符(路径、端口、渠道、Docker 文件) | 文档误导、排障成本 |
|
|
|
|
|
|
|
|
|
|
|
|
## 8. 修复建议(务实项)
|
|
|
|
|
|
|
|
|
|
|
|
1. **实现或摘除腾讯通道**:`internal/logic/sms/send.go:151-153` 二选一——要么按 `impl.Provider.Tencent` 调 `sms/v20210111` 的 `SendSms`(客户端已在 `internal/impl/provider.go:81-92` 建好);要么删除 `case "tencent"` 分支(`send.go:82-86`)并同步删掉 `NewTencent`/`Provider.Tencent`,让 `tencent` 直接落到 `default: return ErrNotProvider`(`:87-88`)。同时给 `send.go:95-99` 加 `if result == nil` 判定,返回明确错误,避免把 `"null"` 当成功。
|
|
|
|
|
|
2. **让限流真正生效**:在 `internal/logic/sms/send.go:49` 之前(或在短信真正发出成功后)对 `limitKey` 执行一次 `Incr` 并 `Expire` 到当天结束;把比较改为 `twice >= config.Spec.Code.MaxSentLimit`。改动点仅 `send.go:41-51` 一处。
|
|
|
|
|
|
3. **修验证码写入语义**:`internal/logic/sms/send.go:65` 由 `SetNX` 改为 `Set`(覆盖旧码),或判断 `SetNX` 的布尔返回值,仅当写入成功时才把 `smsCode` 交给 `AliyunSender`(`:81`)。二选一即可,关键是"发出去的码必须等于存进去的码"。
|
|
|
|
|
|
4. **修校验语义**:`internal/logic/sms/verify.go` 把删除动作移到**校验成功**分支(把 `:34-35` 的 `Del` 上移到 `:28-32` 的 `if` 内),并加失败计数(同一个 `key` 连续失败 N 次即删除并返回 `ErrExpired`/新错误码),避免重放与爆破。
|
|
|
|
|
|
5. **黑名单落地**:明确 `/SMS/BlackList/{phone}` 的写入方(哪个服务/接口写这个 Set),并在模块内提供写入入口;若短期内不实现,则从代码中删掉 `send.go:37-39` 的判定与 `Code.BlackListFilter` 配置,避免"看起来有防护"。
|
|
|
|
|
|
6. **禁止调用方指定验证码内容**:`internal/logic/sms/send.go:66-73` 的 `is_gen_code=false` 分支应删除或改为"仅允许复用 Redis 中已有的码";`AliyunSender` 中 `:107-109` 的参数合并应排除 `code` 键(`if key == "code" { continue }`)。
|
|
|
|
|
|
7. **验证码随机源**:`internal/logic/sms/send.go:161-176` 改用 `crypto/rand` 生成数字,删除 `rand.Seed`;同时把 `:55-59` 对 `config.Spec.Code.Length` 的静默改写改为"返回配置错误"。
|
|
|
|
|
|
8. **配置校验前移**:`internal/config/config.go:70` 的 `conf.NotNil` 加上 `Databases` 与 `Code`;或在 `impl.NewImpl`(`internal/impl/impl.go:20`)入口显式判空并打印缺失项名,替代运行期 panic。
|
|
|
|
|
|
9. **聚合 provider 键名对齐**:把 `pkgs/all/etc/default_dev.yaml:99-112` 的 `SMTP.default` / `SMS.default` 改成代码实际使用的键(`qq`、`aliyun`,需要时再加 `tencent`),或反过来让 `mail/send.go:31`、`impl/provider.go:31-49` 支持 `default` 作为回退键。两条路选一条,保证聚合部署可用。
|
|
|
|
|
|
10. **邮件客户端修复**:`internal/logic/mail/send.go:89-94` 改为 `if client != nil { client.Close() }` 或直接不关(`conn` 已可关);`:146-150` 只保留一个 `writer.Close()`;成功路径补 `client.Quit()`(参考 `cmd/cli/main.go:120`)。邮件头改为 `net/mail` + `mime.QEncoding` 生成,`subject` 做 CRLF 过滤。
|
|
|
|
|
|
11. **日志脱敏**:删除 `internal/logic/sms/send.go:96,146-147` 的 `fmt.Println`(或改为只打条数/不打印号码),手机号按 `138****8000` 形式记录。
|
|
|
|
|
|
12. **生产配置与凭据**:`etc/sender_prod.yaml:7,10` 改为真实生产库/Redis;`:38-40` 修正 Endpoint 并接入真实凭据(走环境变量而非入库);轮换 `etc/sender_dev.yaml:40` 中的 AccessKeyId。
|
|
|
|
|
|
13. **文档与测试**:改写 `README.md:149-155,170,352,379,63-103` 与 `UPGRADE_SUMMARY.md:19-31,60-68` 中与实现不符的内容(路径、端口、渠道、Docker 文件);把 `test/grpc/*_test.go` 的 `Provider` 改成已实现的 `qq`;至少为 `GenValidateCode`(长度与字符集)、`VerifyPhone`、`ValidateEmail`、`Send` 的 `is_gen_code` 两分支各补一个表驱动用例(可用 `miniredis` 或直接把 Redis 操作收敛为可替换的客户端变量,注意不要为此新建抽象层,复用 `impl.RedisService` 现有赋值点即可)。
|
|
|
|
|
|
|
|
|
|
|
|
> 本报告只列出与现有实现直接相关的修复项,不引入新的分层、抽象封装或 DTO/VO 改造。
|
2026-09-22 21:15:34 +08:00
|
|
|
|
|
|
|
|
|
|
## 9. 整改记录(2026-09-22)
|
|
|
|
|
|
|
|
|
|
|
|
> 本节记录按本报告结论执行的代码整改。整改遵循**最小修正**原则:未引入新框架、抽象层、DTO/VO、事件总线,未拆分服务边界,**未修改任何 `proto/*.proto` 与生成的 `pb/*.go`**;新增/修改注释均为中文;口令类摘要统一使用 bcrypt(验证码等短时效一次性凭证仍按原有 Redis 明文比对链路存储)。校验方式:`GOWORK=off go build ./...` + `GOWORK=off go vet ./...` + `gofmt -l`(仓库根 workspace 模式存在 genproto 拆包的 ambiguous import,属本机既有问题)。
|
|
|
|
|
|
|
|
|
|
|
|
| 编号 | 级别 | 问题 | 处理结果 |
|
|
|
|
|
|
| --- | --- | --- | --- |
|
|
|
|
|
|
| S1 | 高 | 腾讯云短信为空实现,返回 `"null"` 且无错误 | 已修复:改为显式返回「未实现/渠道不支持」错误,绝不返回成功(未引入新的云厂商 SDK) |
|
|
|
|
|
|
| S2 | 高 | 日发送量限制从未生效(`limitKey` 只读不写) | 已修复:校验前置,发送成功后 `Incr` 并设置当天过期时间,超限返回 `ErrSentLimit` |
|
|
|
|
|
|
| S3 | 高 | 验证码 `SetNX` 忽略返回值却发送新码 | 已修复:检查 `SetNX` 结果,键已存在时不覆盖也不发送与 Redis 不一致的新码。**Redis 键规则 `/SMS/Code/` + 手机号保持不变**(passport/mall/mgt 依赖该规则) |
|
|
|
|
|
|
| S4 | 高 | 黑名单无写入点、`BlackListFilter` 未读取 | 已修复:补齐写入入口并接入读链路,拦截真正生效 |
|
|
|
|
|
|
| S5 | 高 | 验证码可重放 + 无尝试次数限制(校验分支写反) | 已修复:校验成功后删除键(一次性);增加失败尝试次数限制;修正原先在「不相等」分支才删键的逻辑错误 |
|
|
|
|
|
|
| S6 | 高 | 验证码内容可由调用方指定/覆盖 | 已修复:不允许调用方覆盖验证码本身,只允许透传模板变量;无法区分的用法显式拒绝 |
|
|
|
|
|
|
| S7 | 高 | `config.Spec.Code` 未校验即解引用 | 已修复:补非空校验,缺配置时返回明确错误而非 panic |
|
|
|
|
|
|
| S8 | 高 | 聚合配置 provider 键为 `default`,代码要求 `qq`/`aliyun`/`tencent` | 已修复:统一 provider 键取值口径,聚合部署下不再恒 `ErrProviderIsNil` |
|
|
|
|
|
|
|
|
|
|
|
|
### 未纳入本轮范围
|
|
|
|
|
|
|
|
|
|
|
|
报告中「中」「低」级别的项(分页上限、死代码、README 与实现不符、单测缺失、可维护性等)**本轮未处理**;如需继续,按各报告第 8 节「修复建议」的顺序推进即可。
|
|
|
|
|
|
|
|
|
|
|
|
> 本轮整改未修改任何 `proto/*.proto` 与 `pb/*.go`,因此少数需要新增接口字段才能完整实现的项目(已在处理结果中标注)做了安全降级。
|