sub2api-cn-relay-manager/docs/2026-06-08-SYSTEMATIC_REVIEW_REPORT.md

sub2api-cn-relay-manager 系统化全面 Review 报告

日期：2026-06-08
审查范围：cmd/、internal/、tests/integration/、scripts/、deploy/、Dockerfile*、.github/workflows/ci.yml、README.md、docs/ 中与运行/验收/真相直接相关文档
审查方式：静态源码审查 + 文档/脚本一致性核对 + 本地质量门禁执行

---

一、执行结论

结论：不建议按“严格生产级通过”评价当前代码库。
原因不是基础质量差，而是存在几类会直接影响认证边界、网关语义和 key 治理正确性的系统性问题。

本次审查同时确认：

• 当前项目的 本地质量门禁是通过的：bash ./scripts/test/verify_quality_gates.sh 全部 PASS。
• 代码库在测试覆盖、SQLite repo 测试密度、真实验收 artifact、前端回归脚本方面，已经具备较强工程化基础。
• 但以下问题仍然属于实现语义级缺陷，不会被现有门禁完全拦住。

---

二、已执行验证

已实际执行：

bash ./scripts/test/verify_quality_gates.sh

观察结果：

• test_tksea_portal_assets.sh：PASS
• verify_frontend_smoke.sh：PASS
• verify_vnext_slo_release_gate.sh：PASS
• gofmt -l .：PASS
• go vet ./...：PASS
• go test -cover ./internal/...：PASS
• go test ./tests/integration/... -count=1：PASS
• 核心覆盖率：
  • internal/access 84.0%
  • internal/app 70.1%
  • internal/provision 80.8%
  • internal/store/sqlite 77.6%
  • internal/pack 75.7%

说明：门禁通过 ≠ 业务语义无缺陷。以下问题均是在门禁通过前提下仍然成立。

---

三、关键问题清单

Critical-1：/api/keys 公共接口存在 subject 伪造风险，属于认证边界缺陷

• 文件：
  • internal/app/http_api.go — NewAPIHandlerWithAuth
  • internal/app/key_self_service.go — (*UserKeyHandler).extractSubjectID
  • deploy/tksea-portal/nginx.sub.tksea.top.conf.example
• 问题：用户 key 自助接口未绑定可信用户身份源，而是直接信任来路请求头中的 subject。
• 证据：
  1. NewAPIHandlerWithAuth 将以下接口直接暴露为公共接口，没有 requireAdminAccess 包裹：
     • POST /api/keys
     • GET /api/keys
     • GET /api/keys/{key_id}
     • POST /api/keys/{key_id}/reset
     • POST /api/keys/{key_id}/pause
     • POST /api/keys/{key_id}/resume
     • DELETE /api/keys/{key_id}
  2. extractSubjectID() 直接接受 X-Portal-Subject / X-User-Subject / X-Forwarded-User，任一非空即作为真实 subject 使用；否则甚至会退化为用 Authorization: Bearer ... 前 8 位拼出 skeleton_user_*。
  3. nginx.sub.tksea.top.conf.example 中 /portal-admin-api/ 只是普通反代，没有清洗或重建这些 header。
  4. 前端 deploy/tksea-portal/index.html 也是在浏览器侧自行构造 X-Portal-Subject 再发给 /portal-admin-api/api/keys。
• 影响：
  • 任意能访问 /portal-admin-api/api/keys* 的调用方，只要伪造 X-Portal-Subject，就可能代替其他用户创建、列出、重置、暂停、恢复或退役 key。
  • 这是直接的对象级授权绕过，不是低优先级硬化项。
• 建议：
  • /api/keys* 必须只接受服务端可信身份，不能信任浏览器自填 header。
  • 最小修复方案：由反向代理/认证层注入不可伪造身份，并在 CRM 侧校验来源；更稳妥的是 CRM 自己校验宿主登录态/JWT，再从服务端导出 subject。
  • 删除 skeleton_user_* 这种回退逻辑；它适合 demo，不适合生产认证边界。

Critical-2：公网 /v1/chat/completions 会把上游失败伪装成本地成功 HTTP 200

• 文件：
  • internal/app/http_api.go — handlePublicV1ChatCompletions
  • internal/app/route_proxy_api.go — proxyChatCompletionToShadowHost
• 问题：当 shadow host 返回 4xx/5xx 时，CRM 仍然向客户端返回 200 OK，并把结果记成 ok 指标。
• 证据：
  1. proxyChatCompletionToShadowHost() 在上游非 2xx 时，只设置：
     • info.OK = false
     • info.UpstreamStatus = <4xx/5xx>
     • info.ErrorClass = ... 但不返回 error。
  2. handlePublicV1ChatCompletions() 只有 proxyChat(...) 返回 err 才走错误分支；否则无论 result.Forward.OK 是否为 false，最终都：
     • metrics.RecordUserKeyChatRequest("ok")
     • w.WriteHeader(http.StatusOK)
  3. 代码甚至在 !result.Forward.OK 时，只是往 JSON 里附加 upstream_http_code 字段，而不是透传失败状态码。
• 影响：
  • 客户端会把真实失败误判为成功，协议语义被破坏。
  • SLO/告警指标会把失败流量记成 ok，观测真相被污染。
  • 与执行板中强调的“真实闭环/失败路径可观测”目标相冲突。
• 建议：
  • 当 result.Forward.OK == false 时，必须返回对应 HTTP 状态码，并统一错误体。
  • user_key_chat_requests_total 必须按真实 outcome 记录，而不是只要本地代理函数没抛错就记 ok。
  • 为 4xx/5xx、上游非 JSON、超时等情形补回归测试。

High-1：同一 subject + logical group 下的多个 key 实际会坍缩成同一把宿主 key，破坏 key 级治理语义

• 文件：
  • internal/host/sub2api/subscription_access.go — buildManagedSubscriptionIdentity, EnsureSubscriptionAccess
  • internal/app/key_self_service_svc.go — createFn, resetFn
  • internal/store/sqlite/user_keys_repo.go — ListByFingerprint
  • internal/app/http_api.go — handlePublicV1ChatCompletions
• 问题：项目对同一 subject + group 生成的是确定性宿主 key，而不是每条 KeyRecord 独立的真实 key；这会让“创建多把 key / reset 某一把 key / pause 某一把 key”都失去 key 级隔离。
• 证据：
  1. buildManagedSubscriptionIdentity(selector, groupID) 使用 selector|groupID 的 SHA256 生成固定：
     • CustomKey = "sk-relay-" + keyHash
  2. EnsureSubscriptionAccess() 在非 real-subscription 模式下最终返回 SubscriptionAccessRef{APIKey: identity.CustomKey}。
  3. createFn / resetFn 都调用 ensureSubjectHasAccess()，因此同一 subject + group 会反复拿到同一个明文 key。
  4. ListByFingerprint() 允许返回多条同 fingerprint 记录，handlePublicV1ChatCompletions() 只取 keys[0]。
  5. 文档 docs/2026-06-04-KEY_SELF_SERVICE_API.md 明确要求：reset 后旧 key 失效，新 key 唯一可用。
• 影响：
  • 一个用户在同 logical group 下创建两条记录，本质上可能只是同一把宿主 key 的多个投影。
  • pause/delete 某条记录不一定能准确影响该明文 key 的可用性；效果取决于哪条重复 fingerprint 记录排在最前。
  • reset 不一定产生新 key，更谈不上“旧 key 立即失效”。
• 建议：
  • 把“KeyRecord 标识”和“宿主侧实际 key 材料”一一对应，禁止同一有效明文 key 被多个活跃记录共享。
  • 若业务上故意做“subject 级共享 key”，就必须删除当前 key-level pause/reset/delete 语义，避免伪装成独立 key。
  • 至少补唯一约束或冲突处理：活跃记录不能共享同一 key_fingerprint。

High-2：allowed_models 已进入 API/DB/UI，但运行时完全未执行授权

• 文件：
  • internal/app/key_self_service.go
  • internal/app/key_self_service_svc.go
  • internal/app/http_api.go — handlePublicV1ChatCompletions
  • internal/store/migrations/0015_user_keys.sql
  • docs/2026-06-04-KEY_SELF_SERVICE_API.md
• 问题：key 创建时接收并持久化 allowed_models，UI 也展示它，但实际调用 /v1/chat/completions 时没有任何模型级校验。
• 证据：
  1. CreateUserKeyRequest、UserKeyMeta、UserKeyRecord 均含 AllowedModels。
  2. Portal 页面创建 key 时会提交 allowed_models。
  3. 代码搜索结果显示，allowed_models 仅出现在 CRUD / 文档 / 测试数据中；在网关调用路径上没有任何“模型是否允许”的判断。
  4. handlePublicV1ChatCompletions() 仅校验：admin_status、quota_status、model 非空，随后直接把 openAIReq.Model 转发。
• 影响：
  • 对外宣称的 key 粒度模型授权只是展示字段，不是实际控制。
  • 用户可绕过 UI 选择，直接调用同 logical group 下任意路由可达模型。
• 建议：
  • 在 handlePublicV1ChatCompletions() 入站阶段强制校验 model ∈ allowed_models。
  • 若 allowed_models 只是提示字段，应从 API/文档/UI 中降级为 advisory，避免误导。

High-3：expires_at 生命周期契约未落地，过期 key 仍可继续使用

• 文件：
  • internal/store/migrations/0015_user_keys.sql
  • internal/app/key_self_service.go
  • internal/app/http_api.go — handlePublicV1ChatCompletions
  • docs/2026-06-04-KEY_SELF_SERVICE_API.md
  • deploy/tksea-portal/index.html
• 问题：系统保存并展示 expires_at，但网关与自助接口都不执行过期校验。
• 证据：
  1. migration 已定义 expires_at 字段。
  2. API 文档把它定义为 KeyRecord 字段，前端也展示“到期时间”。
  3. 代码搜索中，expires_at 只出现在 repo 读写、页面展示和文档；handlePublicV1ChatCompletions() 未检查它。
• 影响：
  • 过期只是 UI 文案，不是授权边界。
  • 运维、用户和文档会误以为 key 过期后自动失效，实际不会。
• 建议：
  • 在公共网关路径显式拒绝已过期 key。
  • 为列表/详情接口补一个派生状态，避免前后端各自解释过期语义。

High-4：CI 与仓库声明的质量门禁不一致，且 Docker 验证基本失效

• 文件：
  • AGENTS.md
  • scripts/test/verify_quality_gates.sh
  • .github/workflows/ci.yml
  • Dockerfile
• 问题：CI 没有执行仓库声明的完整质量门禁；Docker job 的“镜像测试”命令还指向错误路径并被 || true 吞掉。
• 证据：
  1. 项目 AGENTS.md 明确要求：前端资产检查、frontend smoke、go vet、go test -cover ./internal/... 阈值、go test ./tests/integration/...、执行板同步等。
  2. verify_quality_gates.sh 已把这些门禁收口成一条脚本。
  3. .github/workflows/ci.yml 并未调用 verify_quality_gates.sh；只跑了：
     • go build
     • go test -race ./internal/...
     • 全局 coverage 60%
     • golangci-lint / gosec / govulncheck
  4. Docker job 中：
     • docker run --rm sub2api-cn-relay-manager:test /app/server --version || true
     • docker run --rm sub2api-cn-relay-manager:test /app/cli --help || true 但镜像实际入口和二进制路径在 Dockerfile 中是 /usr/local/bin/sub2api-cn-relay-manager，并不存在 /app/server、/app/cli。
• 影响：
  • CI 绿并不代表仓库门禁绿。
  • Docker job 即使完全失效也会继续通过，无法证明镜像可运行。
• 建议：
  • CI 直接收口到 bash ./scripts/test/verify_quality_gates.sh。
  • Docker 验证改成真实入口探活，例如启动容器后访问 /healthz。
  • 删除 || true 这种吞错写法。

Medium-1：pause API 丢弃请求里的 reason，与文档承诺不一致

• 文件：
  • internal/app/key_self_service.go — handlePauseUserKey
  • internal/app/key_self_service_svc.go — pauseFn
  • docs/2026-06-04-KEY_SELF_SERVICE_API.md
• 问题：文档声明 POST /api/keys/:id/pause 请求体可选 reason，且“暂停原因应对用户可见”；实际 handler 完全不解析请求体，直接把空字符串传给服务层。
• 证据：
  1. handlePauseUserKey() 直接调用 pauseFn(..., "")。
  2. pauseFn() 虽然接收 reason string，也会写入审计事件，但现在永远拿不到请求值。
  3. 文档明确写了“请求体可选 reason”“暂停原因应对用户可见”。
• 影响：
  • 审计记录缺失关键上下文。
  • 文档、API 和实际行为不一致。
• 建议：
  • 明确 pause request schema，解析并持久化 reason。
  • 若短期不支持，删除文档承诺和“对用户可见”的表述。

Medium-2：last_used_at 只定义不更新，运营可观测字段失真

• 文件：
  • internal/store/sqlite/user_keys_repo.go — TouchLastUsed
  • internal/app/http_api.go — handlePublicV1ChatCompletions
  • docs/2026-06-04-KEY_SELF_SERVICE_API.md
  • deploy/tksea-portal/index.html
• 问题：仓库提供了 TouchLastUsed()，API/UI 也展示 last_used_at，但实际调用链没有地方更新它。
• 证据：
  1. TouchLastUsed() 已实现。
  2. 搜索结果显示没有任何调用方。
  3. 页面展示“最近使用”，文档也把它定义为标准字段。
• 影响：
  • 门户与运营人员会看到长期为空或过期的数据。
  • 配额治理、冷 key 清理、审计回溯都会缺失基本事实源。
• 建议：
  • 在成功代理调用后更新 last_used_at。
  • 如果担心同步写放大，可异步写或批量聚合，但不能一直只定义不落地。

Medium-3：部署脚本默认值过于危险，容易误打生产环境

• 文件：scripts/deploy/deploy_tksea_portal.sh
• 问题：部署脚本内置了具体生产 IP、默认 SSH key 本地路径和生产端口。
• 证据：
  • KEY=/home/long/下载/zjsea.pem
  • REMOTE=ubuntu@43.155.133.187
  • REMOTE_CRM_PORT=18190
• 影响：
  • 新环境复用困难。
  • 在错误上下文直接执行时，有误操作生产的风险。
• 建议：
  • 把这些值移到显式 env / .env.deploy.example。
  • 缺省值应偏向安全失败，而不是默认命中生产。

三点五、第二轮异构方法补充发现

第二轮没有复用上一轮的主阅读路径，改用以下方法补查：

• 异常模式扫描：search 检查 localStorage 持久化、忽略错误、固定凭证生成、鉴权回退等模式
• 重点源码回读：对命中的 handler / portal 页面 / host adapter / repo 解码逻辑做定点复核
• LSP 诊断尝试：当前会话未在项目根激活 Go LSP，因此未产出额外语言服务器诊断；结论以下述源码证据为准

四、2026-06-08 当前整改状态（追加）

• 已本地修复并验证：
  • Critical-2：公网 /v1/chat/completions 对上游失败已返回真实失败状态，不再包装成 200/ok
  • High-1：同 subject + logical_group 的多条 key record 已改为独立 managed_identity_selector，create/reset/pause/resume 不再复用同一宿主 key
  • High-2：allowed_models 已在公网 chat 入口强制执行
  • High-3：expires_at 已在公网 chat 入口强制执行
  • Medium-1：pause 已解析请求体 reason
  • Medium-2：成功 chat 后会更新 last_used_at
• 本地验证：
  • gofmt -w 目标文件通过
  • go vet ./... 通过
  • go test ./internal/app ./internal/store/sqlite ./tests/integration/... -count=1 通过
• 尚未完成的最终闭环：
  • remote43 当前 nginx /portal-admin-api/ 仍未注入 trusted subject / proxy secret
  • remote43 当前 .env.crm 仍缺 SUB2API_CRM_TRUSTED_*
  • 因此新的线上 user-key 真验尚未完成；需先补生产 trusted-subject 链，随后再跑真实 POST /api/keys + POST /v1/chat/completions = 200 验收

High-5：Portal 管理页把 Bearer token、probe key、provider keys 持久化到 localStorage，且与页面文案相矛盾

• 文件：
  • deploy/tksea-portal/admin-common.js — readStoredConfig, writeStoredConfig
  • deploy/tksea-portal/admin/accounts.html — writeConfig
  • deploy/tksea-portal/admin/providers.html — saveConfig
  • deploy/tksea-portal/admin/logical-groups.html — saveConfig
  • deploy/tksea-portal/admin/route-health.html — saveConfig
  • deploy/tksea-portal/admin-batch-import.html — saveConfig
  • deploy/tksea-portal/admin/index.html
• 问题：多个管理页把高敏感凭证长期写入浏览器 localStorage，包括 adminToken、probeAPIKey、accessAPIKey、providerKeys、batch import entries；但首页文案明确写着 Bearer Token “不落盘，仅当前会话”。
• 证据：
  1. admin-common.js 的 writeStoredConfig() 直接执行 global.localStorage.setItem(storageKey, JSON.stringify(payload))。
  2. accounts.html / logical-groups.html / route-health.html / providers.html / admin-batch-import.html 都把 adminToken 写入存储配置。
  3. providers.html 还会持久化：
     • accessAPIKey
     • providerKeys
  4. admin-batch-import.html 还会持久化：
     • probeAPIKey
     • entries
  5. admin/index.html 第 273-275 行的 UI 提示仍写着：Bearer Token（可选） / 不落盘，仅当前会话。
• 影响：
  • 任何 XSS、浏览器扩展、共享机器、浏览器 profile 泄漏，都会直接暴露管理员 Bearer token 与第三方供应商 key 材料。
  • 这不是单纯 UX 漂移，而是前端凭证驻留策略错误。
• 建议：
  • 默认禁止把任何 token / key / entries 写入 localStorage。
  • 如确有调试需求，改成显式“记住敏感信息”开关，默认关闭，并单独标红提示风险。
  • 首页与各页面文案必须与真实持久化行为保持一致。

High-6：managed subscription 的宿主账号密码与 API key 完全由 selector + groupID 确定性推导，凭证可预测

• 文件：
  • internal/host/sub2api/subscription_access.go — buildManagedSubscriptionIdentity, createManagedSubscriptionUser, loginAsManagedSubscriptionUser, ensureManagedSubscriptionAPIKey
• 问题：系统不是生成随机宿主侧托管用户密码/托管 API key，而是直接用 selector|groupID 的哈希构造固定邮箱、固定密码、固定 custom key。
• 证据：
  1. buildManagedSubscriptionIdentity() 中：
     • Email = fmt.Sprintf("%s-%s@sub2api.local", prefix, shortHash)
     • Password = "RelayPwd!" + hash[:12]
     • CustomKey = "sk-relay-" + keyHash
  2. createManagedSubscriptionUser() 用这个固定密码创建宿主用户。
  3. loginAsManagedSubscriptionUser() 随后用同一个固定密码去 /api/v1/auth/login。
  4. ensureManagedSubscriptionAPIKey() 把 identity.CustomKey 作为 custom_key 提交给宿主。
• 影响：
  • 只要 selector 和宿主 groupID 可推断，同一套宿主凭证就可被离线重建；这与“reset 后获得新 key”目标天然冲突。
  • 它也让托管用户/托管 key 的秘密性依赖于业务标识不可猜，而不是依赖随机熵。
• 建议：
  • 宿主侧用户密码、custom key 必须改成高熵随机值，并由 CRM 服务端持久化管理。
  • selector/groupID 可以作为索引键，但不能直接当作凭证种子。
  • 若需要可重建映射，应只重建“查找键”，不要重建“登录秘密”。

Medium-4：user_keys.allowed_models 的 JSON 解码错误被静默吞掉，数据损坏会被伪装成“空模型列表”

• 文件：internal/store/sqlite/user_keys_repo.go — scanUserKeys, scanOneUserKey
• 问题：repo 在读取 allowed_models 时调用 json.Unmarshal(...)，但不检查返回错误。
• 证据：
  1. scanUserKeys()：
     • json.Unmarshal([]byte(modelsJSON.String), &k.AllowedModels)
  2. scanOneUserKey()：
     • json.Unmarshal([]byte(modelsJSON.String), &k.AllowedModels)
  3. 现有 repo 测试只覆盖正常 JSON，未覆盖损坏数据分支。
• 影响：
  • 一旦库中 allowed_models 被历史脚本、手工修复、坏迁移写坏，API 不会报错，只会悄悄返回空列表。
  • 今天它首先表现为“事实源失真”；[INFERENCE] 如果后续补上模型授权强校验，这类静默降级会进一步变成授权行为不可预测。
• 建议：
  • 解码失败时直接返回错误，而不是吞掉。
  • 为 ListByOwner / GetByID 增加 malformed JSON 测试用例。

第二轮补充结论

• 第一轮的主结论 没有被推翻：认证边界与网关错误语义仍是最需要优先修复的问题。
• 第二轮额外确认：前端管理面的凭证驻留策略 和 宿主托管身份的凭证生成策略 也存在实质性安全问题，不能只当成文档偏差处理。

---

---

四、正向评价

以下设计/工程实践值得肯定：

1. SQLite repo 层组织清晰
   internal/store/sqlite 基本遵循 repo pattern，迁移、查询、边界测试较完整，便于后续维护。

2. 质量门禁脚本收口做得好
   scripts/test/verify_quality_gates.sh 已把前端资产、浏览器 smoke、SLO 门禁、gofmt、vet、coverage、integration test 串成一条可执行基线。

3. 导入/访问闭环核心覆盖率较高
   internal/access、internal/provision、internal/store/sqlite、internal/reconcile 的测试密度明显高于普通原型仓库。

4. 真实宿主 artifact 沉淀充分
   artifacts/ 与执行板/真相文档联动，减少了“以为通过”和“真实通过”混淆。

5. 路由/治理/观测的概念边界基本清楚
   route resolve、sticky、failover、governance、SLO 等概念在命名和文档上已逐步收口，不是无结构堆叠。

6. HTTP Server 基础超时配置合理
   ReadTimeout、ReadHeaderTimeout、WriteTimeout、IdleTimeout、MaxHeaderBytes 都已设置，优于默认裸奔。

7. SQLite 单写连接限制是有意识设计
   SetMaxOpenConns(1) 针对 SQLite writer 约束有明确注释说明，避免了部分自锁型 SQLITE_BUSY 问题。

---

五、整改优先级建议

P0（必须先改）

1. 修复 /api/keys* 的 subject 信任模型，消除 header 伪造。
2. 修复 /v1/chat/completions 的错误码透传与指标统计，禁止把上游失败记成 200/ok。
3. 修复“同 subject + group 复用同一明文 key”导致的 key 级治理语义坍缩。

P1（紧随其后）

4. 落地 allowed_models 强制校验。
5. 落地 expires_at 生效逻辑。
6. 让 CI 与 verify_quality_gates.sh 对齐，并修复 Docker job 假验证。

P2（补齐契约与运营真相）

7. 实现 pause reason 请求/持久化/展示闭环。
8. 在成功调用后更新 last_used_at。
9. 收敛部署脚本默认值，避免隐式命中生产。

---

六、最终判断

如果评价标准是：

• 代码能跑、测试能过、已有较强工程基础 —— 结论是 是。
• 认证、key 治理、网关错误语义已经达到严格生产级 —— 结论是 否。

当前最值得警惕的不是普通 bug，而是两类“表面通过、语义失真”的问题：

1. 认证边界靠客户端自报身份。
2. 上游失败被本地包装成成功。

这两类问题都足以让线上行为与控制面/指标/审计出现系统性偏差，建议优先按 P0 处理后，再谈更高等级放行。