Files
hms/docs/discussions/2026-04-28-comprehensive-qa-review.md
iven 9dd6095e77
Some checks failed
CI / frontend-build (push) Has been cancelled
CI / security-audit (push) Has been cancelled
CI / rust-check (push) Has been cancelled
CI / rust-test (push) Has been cancelled
fix: P0/P1 安全与质量缺陷修复 — 10 项 QA 审查问题解决
P0 安全修复:
- tenant_rls: SQL 拼接改为参数化查询防止注入
- follow_up_service: UUID SQL 拼接改为参数化原生查询
- RLS 策略: 新迁移移除空字符串绕过条件
- SSE 消息推送: token 键名 'token' → 'access_token' 修复
- rate_limit: 登录端点 Redis 不可达时 fail-close

P1 质量修复:
- 小程序缓存清理: preservedKeys 补全认证键名
- 小程序 token 刷新: 失败时清除所有认证数据
- 小程序 401: redirectTo → reLaunch 兼容 tabBar
- 集成测试: 信号量限制并行数据库创建(4个)
- change_password: 乐观锁 version 硬编码 → 动态递增

测试: 516 全部通过 (含 153 集成测试)
2026-04-28 00:57:41 +08:00

263 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# HMS 全面质量保证与代码审查报告
> 日期: 2026-04-28 | 审查范围: 后端 Rust (15 crate) + Web 前端 (133 文件) + 微信小程序 (40 页面)
## 一、构建/测试基线
| 检查项 | 结果 | 详情 |
|--------|------|------|
| `cargo check` | PASS | 12 个 warningerp-plugin 11 个 + erp-server 1 个 dead_code |
| `cargo test --workspace` | **FAIL** | 52 passed / **101 failed** / 0 ignored |
| `pnpm build` (Web) | PASS | 大型 chunk 警告 (antd 1.5MB, charts 1.4MB, editor 799KB) |
| `cargo clippy` | 未运行 | — |
| `cargo fmt --check` | 未运行 | — |
### 集成测试失败根因
PostgreSQL `max_connections = 100`,但 153 个集成测试并行执行时每个需要 2+ 连接(创建独立数据库 + 迁移),总量远超连接上限。**单独运行时所有测试通过**,这是测试并行化问题而非代码 bug。
**修复方案:**
1. 设置 `cargo test -- --test-threads=4` 限制并行度
2. 或在 `TestDb::new()` 中使用信号量控制同时创建数据库的数量
3. 或将测试改为共享数据库 + 事务回滚模式
---
## 二、问题总览
| 来源 | CRITICAL | HIGH | MEDIUM | LOW | 合计 |
|------|----------|------|--------|-----|------|
| 后端 Rust | 2 | 8 | 7 | 4 | 21 |
| Web 前端 | 3 | 6 | 7 | 6 | 22 |
| 微信小程序 | 3 | 6 | 8 | 7 | 24 |
| 安全专项 | 2 | 4 | 4 | 1 | 11 |
| **去重合并后** | **6** | **14** | **16** | **10** | **~46** |
> 后端和安全专项有交叉RLS SQL 注入等),已去重。
---
## 三、CRITICAL — 必须立即修复
### [C-01] RLS 中间件 SQL 拼接(后端+安全)
- **文件**: `crates/erp-server/src/middleware/tenant_rls.rs:26-29`
- **问题**: `format!("SET app.current_tenant_id = '{}'", tid)` 使用字符串拼接 SQL虽然当前 `tid` 是 UUID 类型限制了注入风险,但违反安全编码原则
- **修复**: 使用参数化查询 `Statement::from_sql_and_values`
### [C-02] follow_up_service SQL 拼接(后端+安全)
- **文件**: `crates/erp-health/src/service/follow_up_service.rs:74-81`
- **问题**: 直接 `format!` 拼接 UUID 到 SQL IN 子句
- **修复**: 改用 SeaORM `is_in` 过滤器
### [C-03] SSE Token 键名错误(前端)
- **文件**: `apps/web/src/stores/message.ts:75`
- **问题**: `localStorage.getItem('token')` 但 token 存储键名是 `access_token`,导致 SSE 消息推送从未连接
- **修复**: 改为 `localStorage.getItem('access_token')`
### [C-04] AES 加密密钥硬编码到小程序 bundle小程序
- **文件**: `apps/miniprogram/config/index.ts:16`
- **问题**: `defineConstants` 将加密密钥作为字符串字面量注入到 JS bundle反编译即可获取
- **修复**: 改用运行时安全配置接口获取,或评估本地加密的必要性
### [C-05] PII 数据明文传输/展示(小程序)
- **文件**: `apps/miniprogram/src/services/patient.ts`
- **问题**: 手机号、身份证号等 PII 数据从 API 明文返回,前端直接展示
- **修复**: 后端返回脱敏数据(掩码处理),前端展示时脱敏
### [C-06] 清除缓存破坏认证状态(小程序)
- **文件**: `apps/miniprogram/src/pages/profile/settings/index.tsx:16`
- **问题**: `handleClearCache` 保留 user/tenant_id 但未保留 access_token/refresh_token导致认证不一致
- **修复**: preservedKeys 包含所有认证相关键名,或直接调用 logout()
---
## 四、HIGH — 应尽快修复
### 后端
| # | 问题 | 文件 | 说明 |
|---|------|------|------|
| H-01 | points_service.rs 1805 行 | `erp-health/src/service/points_service.rs` | 超出 800 行上限 2 倍+,应按业务域拆分 |
| H-02 | patient_service.rs 1048 行 | `erp-health/src/service/patient_service.rs` | 超出 800 行上限,应拆分标签/家庭成员管理 |
| H-03 | remove_doctor 缺少乐观锁 | `erp-health/src/service/patient_service.rs:766-795` | 其他删除操作都有 version 检查,此处遗漏 |
| H-04 | change_password 硬编码 version | `erp-auth/src/service/auth_service.rs:327` | `version = Set(2)` 硬编码,并发密码修改会覆盖 |
| H-05 | HealthError→AppError 转换丢失上下文 | `erp-health/src/error.rs:124` | DbError 映射到 Internal唯一约束等语义丢失 |
| H-06 | HealthError::From\<AppError\> 语义倒置 | `erp-health/src/error.rs:135-139` | Unauthorized 被转为 Validation语义错误 |
| H-07 | send_reminders 缺少 tenant_id | `erp-health/src/service/appointment_service.rs:563` | 查询所有租户预约,内存压力无控制 |
| H-08 | RLS RESET 后台 task 隔离 | `erp-server/src/middleware/tenant_rls.rs:39-44` | fire-and-forget task 可能丢失租户上下文 |
### 安全
| # | 问题 | 文件 | 说明 |
|---|------|------|------|
| H-09 | RLS 策略空字符串绕过 | `migration/m20260427_000086_enable_rls_all_tables.rs:30-32` | 空字符串时 RLS 不过滤,可能导致跨租户数据泄漏 |
| H-10 | 文件上传扩展名无白名单 | `erp-server/src/handlers/upload.rs:89-94` | 有 Content-Type + magic bytes 校验但缺少扩展名白名单 |
| H-11 | Rate Limiting fail-open | `erp-server/src/middleware/rate_limit.rs:126-129` | Redis 不可用时登录暴力破解保护完全失效 |
| H-12 | JWT Validation 默认配置 | `erp-auth/src/service/token_service.rs:148` | 未显式指定算法,未验证 issuer |
### 前端
| # | 问题 | 文件 | 说明 |
|---|------|------|------|
| H-13 | Token 存储在 localStorage | `apps/web/src/stores/auth.ts:54-56` | XSS 攻击可窃取,应迁移到 HttpOnly cookie |
| H-14 | ArticleEditor.tsx 554 行 | `apps/web/src/pages/health/ArticleEditor.tsx` | 超出 400 行推荐上限 |
### 小程序
| # | 问题 | 文件 | 说明 |
|---|------|------|------|
| H-15 | 双存储系统混用 | stores/auth.ts + services/request.ts | secure-storage 与 Taro.setStorageSync 混用 |
| H-16 | Token 刷新不同步 store | `services/request.ts:47-52` | 刷新失败删 token 但 store 中 user 仍非 null |
| H-17 | noImplicitAny 关闭 | `tsconfig.json:9` | 20+ 处 any 使用 |
| H-18 | buildCategoryTree 修改输入 | `services/article.ts:27-42` | 违反不可变原则 |
| H-19 | 401 redirectTo 在 tabBar 页面无效 | `services/request.ts:72` | 应使用 reLaunch |
| H-20 | 生产日志可能泄漏 | `services/request.ts:58-64` | IS_DEV 守卫依赖构建配置正确性 |
---
## 五、MEDIUM — 建议修复
| # | 领域 | 问题 | 文件 |
|---|------|------|------|
| M-01 | 后端 | handler 层泛型约束重复 50+ 次 | 所有 handler 文件 |
| M-02 | 后端 | stats_service date_trunc 硬编码重复 | stats_service.rs |
| M-03 | 后端 | RLS 空字符串匹配策略 | enable_rls_all_tables.rs |
| M-04 | 后端 | audit_service fire-and-forget 无错误反馈 | 多处 audit_service::record 调用 |
| M-05 | 后端 | DataScope 默认 All 过于宽松 | erp-core/src/rbac.rs:44-49 |
| M-06 | 后端 | HealthCrypto 与 PiiCrypto 重复实现 | erp-health/src/crypto.rs |
| M-07 | 后端 | seed.rs 表名未用 sanitize_identifier | erp-health/src/service/seed.rs |
| M-08 | 安全 | 小程序 AES ECB 模式暗示 | miniprogram/src/utils/secure-storage.ts:18 |
| M-09 | 安全 | dev_default() 无运行时保护 | erp-health/src/crypto.rs:40-51 |
| M-10 | 安全 | CORS permissive 模式 + credentials | erp-server/src/main.rs:660 |
| M-11 | 小程序 | 轮询无退避8s 硬编码 | consultation/detail/index.tsx:15 |
| M-12 | 小程序 | current_patient_id 无权限校验 | services/request.ts:17-18 |
| M-13 | 小程序 | listPatients page_size:100 硬编码 | services/patient.ts:15-18 |
| M-14 | 小程序 | 咨询消息轮询竞态 | consultation/detail/index.tsx:49-71 |
| M-15 | 小程序 | listAppointments 重复定义 | doctor.ts + appointment.ts |
| M-16 | 小程序 | TrendChart canvasId 硬编码 | TrendChart/index.tsx:123 |
---
## 六、LOW — 可改进
| # | 领域 | 问题 |
|---|------|------|
| L-01 | 后端 | EventBus publish best-effort 可能丢失事件 |
| L-02 | 后端 | REDIS_AVAIL 全局静态影响测试隔离 |
| L-03 | 后端 | tag 验证 count 比对不处理重复 ID |
| L-04 | 后端 | CORS 缺少自定义头部 |
| L-05 | 安全 | 开发日志可能泄漏 API 路径 |
| L-06 | 小程序 | BLEManager 单例/类模式混用 |
| L-07 | 小程序 | Picker 使用 Web API (e.target.value) |
| L-08 | 小程序 | ErrorBoundary 使用 vh 单位兼容性 |
| L-09 | 小程序 | 50+ 处空 catch 块无日志 |
| L-10 | 小程序 | useDidShow cleanup 不生效 |
| L-11 | 小程序 | 科室列表硬编码 |
| L-12 | 小程序 | babel preset 放在 dependencies |
---
## 七、性能问题
### P-01. 前端 Bundle 体积过大
| Chunk | 大小 | 建议 |
|-------|------|------|
| vendor-antd | 1,530 KB | 按需导入 antd 组件tree-shaking 检查 |
| vendor-charts | 1,458 KB | 动态 import仅在统计页面加载 |
| vendor-editor | 799 KB | 动态 import仅在文章编辑页加载 |
### P-02. 集成测试并行度过高
153 个测试同时创建数据库PostgreSQL 连接耗尽。需限制 `--test-threads`
### P-03. send_reminders 一次性加载所有租户预约
无分页/流式处理,租户数量增长后内存压力增大。
---
## 八、编译器 Warning 清单
| Warning | 文件 | 数量 | 类型 |
|---------|------|------|------|
| unused field `check_result` | erp-plugin (query_builder.rs) | 1 | dead_code |
| unused field `timestamp` | erp-server (analytics.rs) | 1 | dead_code |
| unused methods | erp-server test_fixture.rs | 3 | dead_code |
| multiple warnings | erp-plugin (lib) | 11 | unused imports/vars |
**建议**: `cargo fix --lib -p erp-plugin` 可自动修复 6 个。
---
## 九、值得肯定的实践
| 领域 | 实践 | 文件 |
|------|------|------|
| 密码存储 | Argon2 + 随机盐 | erp-auth/src/service/password.rs |
| Token 安全 | Refresh token SHA-256 哈希 + 使用后轮换 | erp-auth/src/service/token_service.rs |
| SQL 注入防护(插件) | sanitize_identifier + 参数化查询 | erp-plugin/src/dynamic_table.rs |
| XSS 防护 | 未使用 dangerouslySetInnerHTML | 全局确认 |
| 文件上传 | Content-Type + magic bytes 双重校验 | erp-server/src/handlers/upload.rs |
| 安全启动 | 拒绝默认密钥启动 | erp-server/src/main.rs:192-215 |
| 审计日志 | 登录/登出/密码修改均记录 | erp-auth/src/service/auth_service.rs |
| 账户锁定 | 5 次失败后 15 分钟锁定 | rate_limit.rs |
| PII 加密 | KEK/DEK 分层 + HMAC 索引 + 脱敏 | erp-core/src/crypto/ |
| 乐观锁 | 所有更新操作检查 version | erp-health 全模块 |
| 软删除 | 统一 deleted_at 字段 | erp-health 全模块 |
| 测试隔离 | 每个测试独立数据库 | test_db.rs |
| 前端状态 | Zustand store + 请求去重 | message.ts |
| 小程序 BLE | 适配器模式,接口抽象良好 | BLEManager.ts |
---
## 十、修复优先级建议
### P0 — 本周内修复(安全基线 + 功能 bug
| # | 行动 | 预估工作量 |
|---|------|-----------|
| C-01 | RLS 中间件参数化查询 | 15 分钟 |
| C-02 | follow_up_service 改 SeaORM 查询 | 30 分钟 |
| C-03 | SSE token 键名修正 | 5 分钟 |
| H-09 | RLS 策略移除空字符串绕过 | 30 分钟 |
| H-11 | 登录端点 rate limit fail-close | 1 小时 |
### P1 — 两周内修复(代码质量 + 安全加固)
| # | 行动 | 预估工作量 |
|---|------|-----------|
| H-01 | points_service.rs 拆分 | 2 小时 |
| H-04 | change_password 乐观锁修复 | 30 分钟 |
| H-05/06 | 错误类型转换修复 | 2 小时 |
| H-12 | JWT Validation 显式配置 | 15 分钟 |
| H-16 | 小程序 token 刷新同步 store | 1 小时 |
| H-19 | 小程序 401 reLaunch | 30 分钟 |
| 集成测试 | 限制 --test-threads 或连接池 | 2 小时 |
### P2 — 一个月内修复(性能优化 + 体验改善)
| # | 行动 | 预估工作量 |
|---|------|-----------|
| P-01 | 前端 bundle 代码分割 | 4 小时 |
| C-04 | 小程序加密密钥方案重新评估 | 2 小时 |
| C-05/06 | PII 脱敏 + 缓存清理 | 4 小时 |
| M-01~07 | 后端 MEDIUM 问题逐步修复 | 8 小时 |
| M-08~16 | 小程序 MEDIUM 问题逐步修复 | 6 小时 |
---
## 十一、测试覆盖空白
| 领域 | 当前状态 | 优先级 |
|------|---------|--------|
| erp-health service 层集成测试 | 153 个(并行问题) | P0 |
| erp-health handler 层测试 | 无 | P1 |
| 前端健康模块组件测试 | 仅 StatusTag | P1 |
| E2E 健康模块测试 | 无 | P1 |
| 小程序单元测试 | 无 | P2 |
| 性能/负载测试 | 无 | P2 |
---
*报告生成时间: 2026-04-28*
*审查工具: cargo check/test, pnpm build, 4 个并行专项审查 agent*