fix: harden tenant auth and quality gates
This commit is contained in:
hectorzhao
@@ -0,0 +1,238 @@
|
||||
# CMPP 平台代码质量审查报告
|
||||
|
||||
- 报告日期：2026-08-28
|
||||
- 审查类型：当前工作区只读基线审计
|
||||
- 审查仓库基线：`171c7d38e8f17de0dd83570603da316d47c0d015`；对应业务代码基线为其父提交 `a70e9e2c078a6109dd87cf8e45aeff956faf24e8`
|
||||
- 分支状态：`main`，相对 `origin/main` 超前 12 个提交
|
||||
- 审查范围：React/Vite 前端、NestJS API、Prisma/PostgreSQL、Redis/BullMQ 接口、Go Gateway、自动化测试、构建与工程配置
|
||||
- 明确未执行：提交、推送、部署、数据库写入、短信发送、远程环境变更、生产/预生产利用验证
|
||||
|
||||
## 1. 执行摘要
|
||||
|
||||
本次综合判定为：**有条件不通过，当前版本不建议继续发布**。
|
||||
|
||||
代码已经具备较好的业务复杂度承载能力：API 45 个测试套件共 532 项全部通过，Gateway 全包测试与 `go vet` 通过，前后端正式构建、Prisma Schema、Gateway 队列契约和已有安全部署校验均通过；PostgreSQL 连接池、事务级 advisory lock、`FOR UPDATE SKIP LOCKED` 和核心业务复合索引也已实际存在。
|
||||
|
||||
但审查发现 2 个 P0 发布阻断问题：
|
||||
|
||||
1. 客户端租户范围来自浏览器可修改的 `x-tenant-id` 请求头，后端多个客户端接口直接信任该值，部分写接口还直接信任请求体中的 `tenantId`，存在跨租户读取、创建、修改或删除业务数据的风险。
|
||||
2. 用户密码使用无盐单次 SHA-256 保存和比对。数据库一旦泄漏，相同密码可直接关联，并可使用 GPU/字典进行高速离线破解，不符合生产账号系统的密码存储要求。
|
||||
|
||||
因此，本报告不把“532 项测试通过”解释为生产可发布，也不把现有 mock 单测解释为真实 PostgreSQL、Redis、MinIO、Gateway 和 CMPP 全链验收完成。
|
||||
|
||||
## 2. 质量评级
|
||||
|
||||
| 维度 | 评级 | 结论 |
|
||||
|---|---:|---|
|
||||
| 业务正确性与并发设计 | B | 关键发送链测试较多，存在 advisory lock、幂等和队列 claim 机制；未做本轮真实全链复验。 |
|
||||
| 安全性 | D | 存在租户越权和弱密码哈希两个 P0。 |
|
||||
| 自动化测试 | C | API/Gateway 测试数量较好；API 语句覆盖率 59.61%、分支覆盖率 50.99%，前端测试文件为 0，且没有覆盖率门槛。 |
|
||||
| 前端性能 | C- | 主 JavaScript 包 2.12 MB，gzip 626 KB，所有页面同步进入主包。 |
|
||||
| 可维护性 | C | 存在超大 Service/Page、无 lint/format 门禁、生产类型仍依赖 `src/mock`。 |
|
||||
| 数据库工程 | B | Schema 有效，核心复合索引、连接池和锁策略较完整；未对真实数据执行 `EXPLAIN (ANALYZE, BUFFERS)`。 |
|
||||
| 依赖与仓库卫生 | C- | 依赖审计仍有 2 个 high advisory；构建缓存被跟踪，临时文件和产物缺少统一忽略策略。 |
|
||||
|
||||
综合参考分：**58/100**。该分数用于排序治理工作，不等价于功能验收通过率。
|
||||
|
||||
## 3. 发布阻断问题
|
||||
|
||||
### CQ-SEC-001 / P0：客户端租户身份可由请求方伪造
|
||||
|
||||
**证据链**
|
||||
|
||||
- `src/api/session.ts:175-177` 从浏览器 `localStorage` 中的展示会话读取 `tenantId`。
|
||||
- `src/api/core/httpClient.ts:90-92`、`117-119` 将该值写入 `x-tenant-id`。
|
||||
- `api/src/common/tenant-id.decorator.ts:3-6` 直接返回客户端请求头中的 `x-tenant-id`，没有绑定已认证用户。
|
||||
- `api/src/auth/session-validation.middleware.ts:25-55` 验证会话用户、状态和 portal，但没有把用户所属租户写成服务端可信租户上下文，也没有校验请求头租户等于用户租户。
|
||||
- `api/src/certification/certification.controller.ts:13-19` 的客户端认证查询信任请求头，提交接口直接接受请求体。
|
||||
- `api/src/certification/certification.service.ts:49-82` 的提交逻辑直接使用 `data.tenantId` 创建认证记录、更新目标企业并写操作日志。
|
||||
- `api/src/sms-config/client-sms-config.controller.ts:26-163` 大量客户端查询和写接口依赖同一 `@TenantId()`；其中应用密钥重置、应用状态变更、材料创建等接口甚至没有传入租户范围。
|
||||
|
||||
**影响**
|
||||
|
||||
已登录企业管理员可以修改浏览器存储、请求头、请求体或资源 ID，尝试访问其他企业的认证、应用、签名、模板、账务、日志等数据。文件服务已有“从当前会话用户反查租户”的正确实现，但该保护没有成为客户端 API 的统一规则。
|
||||
|
||||
**最小修复要求**
|
||||
|
||||
1. 在服务端认证中间件中根据 `sessionUserId` 获取并写入可信 `request.tenantId`；客户端控制器只能读取该字段。
|
||||
2. 客户端路由禁止把 `x-tenant-id` 或请求体 `tenantId` 作为授权依据；即使保留请求头，也只能用于一致性检查，不得决定数据范围。
|
||||
3. 所有按资源 ID 的客户端读写在数据库查询中同时带上可信 `tenantId`。
|
||||
4. 增加 tenant-a/tenant-b 的真实 API 越权回归，覆盖查询、创建、更新、删除、密钥重置、上传下载和导出。
|
||||
|
||||
### CQ-SEC-002 / P0：用户密码采用无盐 SHA-256
|
||||
|
||||
**证据链**
|
||||
|
||||
- `api/src/users/users.service.ts:444-445`：`createHash('sha256').update(password).digest('hex')`。
|
||||
- `api/src/users/users.service.ts:141`、`213`、`267` 使用该函数创建或修改密码。
|
||||
- `api/src/auth/auth.service.ts:62` 使用字符串相等比较验证密码。
|
||||
|
||||
**影响**
|
||||
|
||||
密码哈希没有用户级 salt，也没有故意增加计算和内存成本。数据库泄漏后，弱密码可被高速离线破解；相同密码生成相同哈希，还会泄露账号之间的密码复用关系。
|
||||
|
||||
**最小修复要求**
|
||||
|
||||
1. 改用 Argon2id；如运行环境暂不支持，可使用带独立 salt 和合理成本参数的 scrypt/bcrypt。
|
||||
2. 新哈希保存算法版本与参数；验证旧 SHA-256 成功后立即透明升级，避免一次性强制所有账号重置。
|
||||
3. 使用库提供的恒定时间验证接口，不直接比较哈希字符串。
|
||||
4. 增加旧哈希迁移、错误密码、参数升级、密码修改和 sessionVersion 失效测试。
|
||||
|
||||
## 4. 高优先级问题
|
||||
|
||||
### CQ-API-001 / P1：API 缺少统一运行时输入校验
|
||||
|
||||
- `api/src/main.ts:23-27` 创建应用并配置 body parser，但没有注册全局 `ValidationPipe`。
|
||||
- 搜索未发现 `@IsString`、`@IsInt` 等 `class-validator` 规则。
|
||||
- 统计到约 166 个控制器 `@Body()` 入口；多数 DTO 是 TypeScript interface 或内联类型，运行时会被擦除。
|
||||
- `api/src/open-api/open-api.dto.ts:3-14` 仅包含 Swagger 装饰器，没有输入校验装饰器。
|
||||
|
||||
风险包括错误类型进入服务层、超长字符串、额外字段、枚举外状态和不一致的 400/500 响应。建议建立 DTO class、`transform: true`、`whitelist: true`、`forbidNonWhitelisted: true` 的统一策略，并分批为高风险写接口补齐字段长度、枚举、数组大小和格式限制。
|
||||
|
||||
### CQ-AUTH-001 / P1：验证码和匿名失败计数为进程内无界 Map
|
||||
|
||||
- `api/src/auth/auth.service.ts:20-21` 使用两个模块级 `Map`。
|
||||
- `createCaptcha()` 会持续写入记录；过期记录只有在同一 captchaId 被再次提交时才删除。
|
||||
- 匿名失败以任意登录名为 key 写入，没有容量限制或周期清理。
|
||||
|
||||
这会造成多实例登录状态不一致，并可通过大量验证码请求或随机登录名制造内存增长。建议迁移到 Redis，使用 TTL、原子计数、IP+账号双维度限流和固定容量保护。
|
||||
|
||||
### CQ-FE-001 / P1：前端主包过大且没有路由级代码分割
|
||||
|
||||
当前生产构建结果：
|
||||
|
||||
- JavaScript：2,123.63 KB，gzip 626.18 KB。
|
||||
- CSS：270.61 KB，gzip 39.84 KB。
|
||||
- Vite 明确产生“chunk 大于 500 KB”警告。
|
||||
- `src/routes/AppRoutes.tsx:2-59` 同步导入全部运营端和客户端页面；代码中未发现 `React.lazy()` 或动态 `import()`。
|
||||
- `src/components/ui/Chart.tsx:3` 使用 `import * as echarts from 'echarts'`，进一步扩大主包。
|
||||
|
||||
建议先按 admin/client 及页面路由使用 `React.lazy` + `Suspense` 分包，再按需引入 ECharts 图表模块。应在 CI 增加 bundle budget，例如初始 JS gzip 不高于 250 KB、单异步 chunk gzip 不高于 180 KB；实际阈值可在首轮拆包后校准。
|
||||
|
||||
### CQ-TEST-001 / P1：测试覆盖和验收层级不足
|
||||
|
||||
- API：45 套、532 项通过；全源覆盖率为 statements 59.61%、branches 50.99%、functions 60.33%、lines 62.35%。
|
||||
- `api/jest.config.cjs` 没有 `coverageThreshold`。
|
||||
- `src/` 下前端 `*.spec.*` / `*.test.*` 文件数量为 0。
|
||||
- Gateway 内部包覆盖率约 49.7% 至 100%，但 `cmd/gateway` 只有 0.7%，另两个命令包为 0%。
|
||||
- 本轮测试没有启动真实 PostgreSQL、Redis、MinIO 或 CMPP 模拟器，不能证明真实后端闭环。
|
||||
|
||||
建议先为租户隔离、认证、账务、发送幂等、回执关联建立真实 PostgreSQL/Redis 集成测试；前端至少覆盖登录、权限、加载/空/错误、筛选分页和高风险确认操作；随后逐步设置覆盖率门槛，避免一次性追求无意义的高百分比。
|
||||
|
||||
## 5. 中优先级问题
|
||||
|
||||
### CQ-DEP-001 / P2：生产依赖仍有 2 个 high advisory
|
||||
|
||||
`pnpm audit --prod --json` 返回：
|
||||
|
||||
1. `react-router 7.18.1`：GHSA-qwww-vcr4-c8h2，修复版本 `>=7.18.2`。
|
||||
2. `nanoid 3.3.16`：GHSA-2v37-7h3g-55p8，修复版本 `>=3.3.18`。
|
||||
|
||||
现有 `security:verify` 已证明项目没有使用 React Router RSC 模式，并验证了既有 PostCSS 缓解，因此当前可利用面低于审计工具的原始 high 评级；但版本仍处于公告范围，不能长期依赖“功能未使用”作为供应链治理。建议升级后重新执行构建、API 测试、前端浏览器回归和安全校验。
|
||||
|
||||
### CQ-MAINT-001 / P2：超大文件与缺失静态风格门禁
|
||||
|
||||
- `api/src/send-chain/send-inbound-entry.service.ts` 约 84 KB。
|
||||
- `api/src/send-chain/send-gateway-submit.service.ts` 约 52 KB。
|
||||
- `src/apps/admin/AdminDownstreamDeliveriesPage.tsx` 约 44 KB。
|
||||
- 根项目和 API 均没有 lint/format 脚本，也未发现 ESLint/Prettier 配置。
|
||||
|
||||
建议先按“持久化、状态机、队列 claim、路由、计费”拆分 send-chain 服务；页面按筛选、表格、详情和恢复操作拆分。拆分时要求行为不变，并依靠当前测试防回归。
|
||||
|
||||
### CQ-CONFIG-001 / P2：数据库连接存在硬编码开发默认凭据
|
||||
|
||||
- `api/prisma.config.ts:8-11`。
|
||||
- `api/src/prisma/prisma.service.ts:39-41`。
|
||||
|
||||
当 `DATABASE_URL` 缺失时，进程会尝试使用 `cmpp/cmpp_password` 连接本机数据库。建议生产角色 fail closed；仅在显式 `NODE_ENV=development` 或专用本地配置下允许开发默认值。
|
||||
|
||||
### CQ-ARCH-001 / P2：真实 API 代码仍与 mock 目录耦合
|
||||
|
||||
- `src/mock/` 保留完整 localStorage mock service。
|
||||
- `src/apps/admin/auditColumns.tsx:3` 的生产组件仍从 `@/mock` 导入业务类型。
|
||||
|
||||
当前没有证据表明 mock service 仍被页面运行时调用，但该目录和类型依赖会误导后续开发，并增加重新接入静态数据的风险。建议把共享类型迁移到 `src/api/types` 或 domain 模块，并在构建/检查中禁止 `src/apps/**` 导入 `src/mock/**`。
|
||||
|
||||
### CQ-REPO-001 / P2：仓库产物和审计基线不稳定
|
||||
|
||||
- `api/tsconfig.build.tsbuildinfo` 已被 Git 跟踪，每次构建产生无业务意义的修改。
|
||||
- `.gitignore` 没有统一忽略 `*.tsbuildinfo`、`outputs/` 和任务临时文件。
|
||||
- 最终工作区仍存在 `=`, `outputs/`, `pnpm-lock.yaml`, `tmp_generate_ui_drafts.py` 等既有未跟踪内容；审计窗口内还曾出现随后被并发流程处理的临时部署脚本。
|
||||
- 审计开始时 HEAD 为 `4d4c1f3`，期间其他会话先后提交了业务代码 `a70e9e2` 和部署记录 `171c7d3`；本报告已对新业务代码重新运行前端类型检查和生产构建，但完整 API 覆盖率运行发生在该纯前端提交之前。
|
||||
|
||||
建议统一包管理器与唯一锁文件，忽略纯构建缓存，并在正式审查/发布时使用固定 commit 或独立 worktree，避免审计结论对应移动目标。
|
||||
|
||||
## 6. 已通过门禁
|
||||
|
||||
| 检查项 | 结果 |
|
||||
|---|---|
|
||||
| 前端 TypeScript `tsc --noEmit` | 通过；并发提交后已重跑 |
|
||||
| Vite 生产构建 | 通过；存在包体警告 |
|
||||
| API TypeScript 正式构建 | 通过 |
|
||||
| API Jest | 45/45 套、532/532 项通过 |
|
||||
| API 覆盖率 | statements 59.61%，branches 50.99%，functions 60.33%，lines 62.35% |
|
||||
| Gateway `go test ./... -count=1` | 通过 |
|
||||
| Gateway `go vet ./...` | 通过 |
|
||||
| Gateway `go test ./... -cover` | 通过；各包覆盖率差异较大 |
|
||||
| Prisma Schema | 95 个迁移目录；`prisma validate` 通过 |
|
||||
| Gateway 队列契约 | 5/5 样例通过 |
|
||||
| 依赖缓解/安全部署脚本 | 通过 |
|
||||
| `pnpm audit --prod` | 不通过；2 个 high advisory |
|
||||
| `git diff --check` | 通过 |
|
||||
|
||||
## 7. 积极发现
|
||||
|
||||
1. PostgreSQL 连接按 API、Worker、Outbox、Callback 和 Protocol Log 角色设置独立连接池上限。
|
||||
2. 账务、日配额、频控等竞争资源使用事务级 advisory lock。
|
||||
3. 多个队列 claim 使用 `FOR UPDATE SKIP LOCKED`，适合并发消费者。
|
||||
4. `SmsMessageRecord`、`SmsBatchTask`、`CmppDownstreamDelivery` 等高频表具备 tenant/status/time 复合索引。
|
||||
5. Webhook 主链对协议、内网地址、环回地址和 DNS 解析做了 SSRF 防护，并在 HTTP 客户回调中固定解析后的目标地址。
|
||||
6. 会话 cookie 使用 HttpOnly、SameSite=Lax，并按环境控制 Secure；前端 localStorage 保存的是会话展示元数据，不是 session token。
|
||||
7. API、Gateway 和队列契约测试已经覆盖大量发送、补发、回执去重和异常分支。
|
||||
|
||||
## 8. 未验证边界
|
||||
|
||||
本报告不能替代以下验证：
|
||||
|
||||
- 真实 PostgreSQL 数据量下的 `EXPLAIN (ANALYZE, BUFFERS)` 和慢查询分析。
|
||||
- Redis Stream/BullMQ pending、lag、重试、宕机恢复和重复消费验证。
|
||||
- MinIO 上传、下载、租户隔离和大文件边界。
|
||||
- 登录后的真实浏览器 UI、控制台、网络请求、加载/空/错误/刷新状态。
|
||||
- Gateway 与 CMPP 模拟器或真实供应商的 Submit、长短信、回执、上行和断线恢复闭环。
|
||||
- 测试、预生产、生产当前部署 commit、迁移数、配置和服务状态。
|
||||
|
||||
在完成 P0 修复前，不建议通过真实环境攻击性测试证明越权；应先补自动化隔离回归，再在隔离测试环境验证。
|
||||
|
||||
## 9. 建议整改顺序
|
||||
|
||||
### 第一批：发布阻断
|
||||
|
||||
1. 服务端统一可信租户上下文，关闭请求头/请求体决定客户端租户的能力。
|
||||
2. 迁移密码哈希到 Argon2id/scrypt，并提供旧哈希透明升级。
|
||||
3. 补 tenant-a/tenant-b 越权测试和密码迁移测试。
|
||||
|
||||
### 第二批：安全与质量门禁
|
||||
|
||||
1. 建立全局运行时 DTO 校验。
|
||||
2. 将验证码、失败计数和限流迁移到 Redis TTL/原子计数。
|
||||
3. 升级两个公告依赖并保持安全校验通过。
|
||||
4. 在 CI 增加 lint、format check、覆盖率门槛、依赖审计和 bundle budget。
|
||||
|
||||
### 第三批：性能与可维护性
|
||||
|
||||
1. 前端路由级分包并按需加载 ECharts。
|
||||
2. 拆分 send-chain 超大服务和超大页面。
|
||||
3. 清理 mock 类型耦合、构建缓存和临时产物治理。
|
||||
4. 在隔离真实后端完成 API/DB/Redis/MinIO/Gateway/CMPP 全链回归。
|
||||
|
||||
## 10. 验收出口标准
|
||||
|
||||
整改完成至少应满足：
|
||||
|
||||
- P0 为 0，P1 有明确关闭证据或书面风险接受。
|
||||
- tenant-a 用户无法通过 header、body、query 或资源 ID 访问 tenant-b 数据。
|
||||
- 新密码使用强哈希；旧 SHA-256 账号登录后自动升级，数据库不再新增 SHA-256 密码。
|
||||
- 前端初始 JS 包达到约定预算，核心路由按需加载。
|
||||
- API/Gateway/前端测试与构建全部通过；覆盖率不低于本报告基线且建立门槛。
|
||||
- 依赖审计不再包含本报告两项 high advisory。
|
||||
- 真实 PostgreSQL、Redis、MinIO、Gateway 和 CMPP 测试证据与 `docs/testing-progress.md` 同步。
|
||||
Reference in New Issue
Block a user