From f952a6e274a70ecb80fa0c459b4c54834696d202 Mon Sep 17 00:00:00 2001 From: tlzhu3 Date: Thu, 12 Mar 2026 19:28:49 +0800 Subject: [PATCH] feature : add review doc --- docs/review/代码审查报告.md | 363 ++++++++++++++++++++++++++++++++ docs/review/后期优化报告.md | 405 ++++++++++++++++++++++++++++++++++++ 2 files changed, 768 insertions(+) create mode 100644 docs/review/代码审查报告.md create mode 100644 docs/review/后期优化报告.md diff --git a/docs/review/代码审查报告.md b/docs/review/代码审查报告.md new file mode 100644 index 00000000..0bb93718 --- /dev/null +++ b/docs/review/代码审查报告.md @@ -0,0 +1,363 @@ +# SkillHub 代码审查报告 + +## 1. 审查结论 + +当前版本不建议直接作为后续多人并行开发与生产化落地的基线。 + +结论等级:`有条件不通过` + +阻塞原因主要集中在以下几类: + +1. 认证与鉴权存在高风险缺陷,部分接口可被越权使用,设备码登录实现还存在可伪造令牌问题。 +2. 发布状态机与设计文档不一致,`latest` 语义、搜索事件、下载链路会被未审核版本污染。 +3. 上传与存储链路缺少路径安全和流式限制,存在路径穿越、内存耗尽、脏对象遗留等风险。 +4. 自动化质量门禁未闭环,`skillhub-app` 集成测试无法启动,前端 lint 也未通过。 + +## 2. 审查范围 + +本次重点审查了以下内容: + +- 设计文档: + - `docs/01-system-architecture.md` + - `docs/03-authentication-design.md` + - `docs/05-business-flows.md` + - `docs/06-api-design.md` + - `docs/07-skill-protocol.md` + - `docs/09-deployment.md` +- 后端核心代码: + - 认证与安全:`skillhub-auth` + - 发布、查询、下载、评审、提升:`skillhub-domain` + `skillhub-app` + - 搜索:`skillhub-search` + - 存储:`skillhub-storage` +- 前端核心代码: + - `web/src/features/skill/markdown-renderer.tsx` + +本次执行的关键校验: + +- `pnpm run typecheck`:通过 +- `pnpm run lint`:失败 +- `pnpm run build`:通过,但主包体积告警 +- `mvn -q -DskipTests compile`:通过 +- `mvn test`:失败,`skillhub-app` 24 个测试里 19 个错误 + +## 3. 关键发现 + +### [P0] 设备码登录当前会返回可预测的伪令牌,并且存在并发重复签发风险 + +问题位置: + +- `server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java:85-111` +- `server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java:64-75` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/DeviceAuthController.java:21-29` + +问题说明: + +- `pollToken()` 在设备码授权成功后直接返回 `token_ + deviceCode`,这不是签名令牌,也不是随机不透明令牌,而是由外部输入可推导得到的占位值。 +- `/api/v1/cli/auth/device/**` 被 `permitAll()` 放开,意味着设备流轮询端点是匿名可访问的,这本身没问题,但前提是返回的必须是正式、不可伪造、可审计、可吊销的令牌。 +- `pollToken()` 采用“先读状态,再写 USED”的非原子流程。两个并发轮询请求可以同时读到 `AUTHORIZED`,从而重复获取访问令牌。 + +影响: + +- 这是认证链路上的核心安全缺陷,风险等级为阻塞上线。 +- 设备授权一旦成功,令牌可被预测,且一次授权可能被多次兑换。 + +修改建议: + +1. 设备流成功后必须调用正式的 token 签发服务,返回随机 opaque token 或签名 JWT。 +2. 设备码消费必须改成 Redis Lua / CAS 方式,保证“仅成功兑换一次”。 +3. 为设备码申请、授权、轮询增加限流和审计。 +4. 补充成功兑换、重复兑换、并发轮询、过期轮询的集成测试。 + +### [P1] 评审与提升流程缺失关键权限校验,已形成越权入口 + +问题位置: + +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java:56-64` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java:50-68` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java:54-61` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java:47-78` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java:102-126` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java:86-105` + +问题说明: + +- `submitReview()` 只根据 `skillVersionId` 和当前用户提交,不校验提交人是否为该 skill 的 owner、namespace 管理员或被允许的发布者。 +- `submitPromotion()` 只校验源版本已发布、目标 namespace 是 GLOBAL,不校验申请人是否有权提升该 skill。 +- `listPendingReviews()`、`getReviewDetail()`、`getPromotionDetail()` 缺少明确授权判断,当前只要已登录即可读取相关数据,存在流程信息泄露。 + +与设计文档冲突: + +- `docs/05-business-flows.md:104-119` 明确要求“owner 或 namespace admin”才能发起提升。 +- `docs/06-api-design.md` 中 review / promotion 相关接口的权限边界也比当前实现更严格。 + +影响: + +- 任意登录用户理论上可以替别人的草稿发起评审。 +- 任意登录用户理论上可以为别人的 skill 发起提升申请。 +- 待审核数据与审批详情对无关用户暴露。 + +修改建议: + +1. 在 `ReviewService.submitReview()` 前补 owner / namespace ADMIN/OWNER 校验。 +2. 在 `PromotionService.submitPromotion()` 前补 source skill 所属 namespace 的提交权限校验。 +3. `listPendingReviews()`、`getReviewDetail()`、`getPromotionDetail()` 必须按 namespace 或平台角色做访问控制,未授权时返回 `403`,不能返回空列表掩盖问题。 + +### [P1] 发布流程在审核前就覆盖 `latestVersionId` 并发出 `SkillPublishedEvent`,会污染下载、解析、搜索和标签语义 + +问题位置: + +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java:143-156` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java:217-225` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java:68-75` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java:106-124` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java:344-351` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillTagService.java:44-47` +- `server/skillhub-search/src/main/java/com/iflytek/skillhub/search/event/SearchIndexEventListener.java:26-39` + +问题说明: + +- `publishFromEntries()` 创建的新版本状态是 `PENDING_REVIEW`,但随后立即: + - 更新 `skill.latestVersionId` + - 发出 `SkillPublishedEvent` +- `downloadLatest()`、`resolveLatestVersion()`、`latest` 保留标签、搜索结果里的 latest version 都依赖 `latestVersionId`。 +- 当一个已发布 skill 再上传新版本时,`latestVersionId` 会被指向一个尚未发布的版本,导致“最新版本”不可下载、不可解析或显示错误。 + +与设计文档冲突: + +- `docs/05-business-flows.md:30-45` 的 Phase 2 定义是“发布后直接 `PUBLISHED` 才更新 latest”。 +- `docs/05-business-flows.md:49-55` 的 Phase 3 定义是“`DRAFT -> PENDING_REVIEW -> PUBLISHED`”,审核通过后才进入发布态。 +- 当前代码实际做成了“创建后直接 `PENDING_REVIEW`,但又按已发布版本处理”,状态语义前后矛盾。 + +影响: + +- `latest` 下载链路会在有待审版本时报错。 +- 搜索索引会被过早重建。 +- 详情页、我的技能页、标签查询都会看到未发布版本。 + +修改建议: + +1. 冻结状态机语义: + - 要么 Phase 2:发布即 `PUBLISHED` + - 要么 Phase 3:上传只创建草稿 / 待审,审核通过后才更新 latest +2. `latestVersionId` 必须只表示“最新已发布版本”。 +3. `SkillPublishedEvent` 只能在版本真正进入 `PUBLISHED` 后发出。 +4. 若需要展示“最新草稿”,单独引入 `latestDraftVersionId` 或查询逻辑,不要复用 published 语义字段。 + +### [P1] 上传链路存在路径穿越与压缩包资源耗尽风险 + +问题位置: + +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java:64-83` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidator.java:28-81` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java:166-189` +- `server/skillhub-storage/src/main/java/com/iflytek/skillhub/storage/LocalFileStorageService.java:21-31` +- `server/skillhub-storage/src/main/java/com/iflytek/skillhub/storage/LocalFileStorageService.java:61` + +问题说明: + +- Controller 侧对 zip entry 使用 `readAllBytes()`,在验证之前就把每个解压后文件整体读入内存。 +- 包校验器没有校验路径规范化,也没有禁止 `../`、绝对路径、反斜杠路径、重复路径。 +- `LocalFileStorageService.resolve()` 直接 `basePath.resolve(key)`,没有 `normalize()` 和 `startsWith(basePath)` 校验。 +- 攻击者可以构造恶意 zip 路径,例如 `../../outside.txt`,通过 `skills/{skillId}/{versionId}/{filePath}` 最终逃逸本地存储根目录。 + +影响: + +- 本地存储模式下存在任意文件写入风险。 +- 恶意压缩包可造成内存压力甚至进程 OOM。 + +修改建议: + +1. 在解压阶段引入流式校验,限制总解压大小、单文件大小、文件数量。 +2. 对 entry path 做统一规范化,只允许: + - 相对路径 + - `/` 分隔 + - 禁止 `..`、禁止绝对路径、禁止空段 +3. `LocalFileStorageService` 必须在 `normalize()` 后校验目标路径仍位于 `basePath` 内。 +4. 对重复路径在校验阶段直接返回 `400`,不能等数据库唯一键报错。 + +### [P1] `GET /api/v1/skills/**` 过度放开,导致匿名 `500` 与私有元数据泄露 + +问题位置: + +- `server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java:75` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillStarController.java:40-45` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillRatingController.java:37-46` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java:73-94` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillTagController.java:29-40` + +问题说明: + +- `SecurityConfig` 直接放开了所有 `GET /api/v1/skills/**`。 +- 但其中至少两类 GET 并不适合匿名访问: + - `GET /api/v1/skills/{skillId}/star` + - `GET /api/v1/skills/{skillId}/rating` + 这两个接口直接解引用 `principal.userId()`,匿名访问会触发空指针,落到全局异常后返回 `500`。 +- 另外: + - `listVersions()` 没有 visibility 校验 + - `listTags()` 没有 visibility 校验 + 这会把 `PRIVATE` / `NAMESPACE_ONLY` skill 的版本信息、标签信息暴露给匿名或无关用户。 + +影响: + +- 安全上属于“鉴权边界与路由策略不一致”。 +- 行为上会出现匿名访问 500,破坏 API 契约。 + +修改建议: + +1. 不要用 `GET /api/v1/skills/**` 一刀切放开,改成白名单到具体公共接口。 +2. 对“需要当前用户上下文”的 GET 接口显式要求认证。 +3. `listVersions()`、`listTags()` 必须补充与详情接口一致的 visibility 校验。 + +### [P1] `skillhub-app` 集成测试当前无法启动,测试信号失真 + +问题位置: + +- `server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java:19-25` +- `server/skillhub-app/src/test/resources/application-test.yml:1-33` +- `server/skillhub-app/target/surefire-reports/com.iflytek.skillhub.controller.AuthControllerTest.txt` + +问题说明: + +- `mvn test` 中,`skillhub-storage`、`skillhub-domain`、`skillhub-auth` 的测试通过,但 `skillhub-app` 的 Spring 上下文无法启动。 +- 直接原因是 `DeviceAuthService` 依赖 `RedisTemplate`,测试环境没有对应 bean。 +- 代码里也没有看到 `verificationUri` 的配置注入实现,当前构造器还要求额外的 `String` 参数,后续即使补齐 Redis bean,也大概率还会继续失败。 + +影响: + +- 目前 controller 层测试的 19 个错误都被同一个装配问题掩盖,真实回归无法被发现。 +- CI 无法提供有效的回归保护。 + +修改建议: + +1. 为设备码服务补全正式配置类: + - `RedisTemplate` + - `@ConfigurationProperties` 形式的 `verificationUri` +2. 测试环境提供 stub / mock bean,确保 app context 可启动。 +3. 优先修复后重新运行 controller 集成测试,再评估剩余问题。 + +### [P2] 存储配置键名与部署文档漂移,S3/MinIO 路径当前并不能真正启用 + +问题位置: + +- `server/skillhub-storage/src/main/java/com/iflytek/skillhub/storage/StorageProperties.java:7-15` +- `server/skillhub-storage/src/main/java/com/iflytek/skillhub/storage/LocalFileStorageService.java:12-14` +- `server/skillhub-storage/src/main/java/com/iflytek/skillhub/storage/S3StorageService.java:19-21` +- `server/skillhub-app/src/main/resources/application.yml:55-58` +- `docker-compose.prod.yml:51-59` +- `docs/09-deployment.md` + +问题说明: + +- 代码使用的配置键是 `skillhub.storage.provider`。 +- `application.yml` 写的是 `skillhub.storage.type`。 +- `docker-compose.prod.yml` 也没有把 S3/MinIO 相关配置注入到后端容器。 + +影响: + +- 即使文档和运维层面按 MinIO/S3 部署,也无法通过当前配置切换到 `S3StorageService`。 +- 实际运行会默认为本地文件存储,和部署文档不一致。 + +修改建议: + +1. 统一配置键,只保留一种: + - 推荐 `skillhub.storage.provider` +2. 修正文档、`application.yml`、`docker-compose.prod.yml`、`StorageProperties` 的一致性。 +3. 增加一个启动时自检日志,打印当前启用的 storage provider。 + +### [P2] 上传大小与文件白名单配置未真正落地,当前仍是硬编码 + +问题位置: + +- `server/skillhub-app/src/main/resources/application.yml:47-64` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidator.java:12-20` + +问题说明: + +- 配置文件声明: + - `multipart` 允许到 `100MB` + - `skillhub.publish.max-package-size` 为 `100MB` + - `allowed-file-extensions` 可配置 +- 但实际校验器仍然硬编码: + - 总包 `10MB` + - 固定扩展名集合 + +影响: + +- 文档、配置、运行时行为三者不一致。 +- 运维或产品侧修改配置后不会生效,问题定位成本高。 + +修改建议: + +1. 将 `SkillPackageValidator` 改为读取 `@ConfigurationProperties`。 +2. 保持“网关上限”“multipart 上限”“业务校验上限”三层配置含义清晰并一致。 + +### [P2] 资源不存在时多处直接 `orElseThrow()`,会把正常 404 场景放大成 500 + +问题位置: + +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/PromotionController.java:101-111` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java:60-62` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java:97` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java:123-132` +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillRatingController.java:30-33` + +问题说明: + +- 多个 controller 直接使用裸 `orElseThrow()`。 +- 当资源不存在时会抛出 `NoSuchElementException`,最后被全局异常处理器转成 `500`。 +- `SkillRatingController` 还存在 `score` 缺失时的空指针风险。 + +影响: + +- API 契约不稳定,客户端会把普通业务错误当成服务故障。 + +修改建议: + +1. 统一抛出领域层 `NotFound/BadRequest` 异常。 +2. 给评分、评审、提升请求增加 `@Valid` 和显式字段校验。 + +### [P2] 前端质量门禁未闭环,lint 未通过,产物主包偏大 + +问题位置: + +- `web/src/features/skill/markdown-renderer.tsx:15-16` + +问题说明: + +- 当前 `eslint` 失败: + - `@ts-ignore` 应改为 `@ts-expect-error` + - `node` 参数未使用 +- `pnpm run build` 虽能通过,但主 JS chunk 约 `751.25 kB`,明显偏大。 + +影响: + +- CI 无法建立严格前端门禁。 +- 首屏加载与缓存更新成本偏高。 + +修改建议: + +1. 先修复 lint 报错,恢复前端静态检查门禁。 +2. 后续按页面级或功能级拆包,优先处理 markdown/highlight、管理页、上传页等非首屏模块。 + +## 4. 与设计文档的主要偏差 + +当前代码与文档的主要偏差有: + +1. `docs/05-business-flows.md` 对 Phase 2 / Phase 3 的发布状态定义,与 `SkillPublishService` 现状不一致。 +2. 文档里定义了幂等、审计、孤儿对象清理、异步事件兜底,但代码里尚未真正实现。 +3. 文档要求的权限边界比当前 review / promotion / skill GET 路由更严格,当前实现明显偏松。 +4. 部署文档强调 MinIO / S3 可切换,但配置层并未打通。 + +## 5. 现阶段是否可进入开发 + +可以继续开发,但不建议直接进入“多人并行开发 + 联调 + 准生产验证”阶段。 + +建议先完成以下最小修复集: + +1. 修复设备码认证实现与 app 测试装配。 +2. 收紧 review / promotion / skills GET 的鉴权边界。 +3. 修正发布状态机,保证 `latestVersionId` 只指向已发布版本。 +4. 修复 zip 路径校验和本地存储路径归一化。 +5. 打通前后端质量门禁:`mvn test`、`pnpm lint`、`pnpm build` 全绿。 + +在这五项完成之前,后续功能开发会持续叠加在一个不稳定基线上,返工概率较高。 diff --git a/docs/review/后期优化报告.md b/docs/review/后期优化报告.md new file mode 100644 index 00000000..b8a07694 --- /dev/null +++ b/docs/review/后期优化报告.md @@ -0,0 +1,405 @@ +# SkillHub 后期优化报告 + +## 1. 总体判断 + +SkillHub 当前已经具备“能跑起来的模块化单体”雏形,分层方向是对的,但距离“可持续迭代、可对外开放、可灰度发布”的工程化状态还有明显差距。 + +后续优化应分成两类: + +1. `阻塞型修复`:先把安全、鉴权、状态机、测试基线修好。 +2. `架构型收敛`:把文档、配置、协议、模块边界收敛成一个长期稳定的实现。 + +下面的建议默认以“后续要继续实际代码开发”为前提,而不是只做文档美化。 + +## 2. 优先级建议 + +### T0:1 周内必须完成 + +1. 修复设备码登录: + - 正式 token 签发 + - 原子消费 + - Redis bean / 配置注入补齐 +2. 修复 review / promotion / skills GET 的权限边界。 +3. 修复发布状态机: + - `latestVersionId` + - `SkillPublishedEvent` + - `latest` 标签 + - 搜索重建触发时机 +4. 修复上传路径安全和解压限制。 +5. 恢复质量门禁: + - `mvn test` + - `pnpm lint` + - `pnpm build` + +### T1:2 到 3 周内完成 + +1. 冻结配置协议与部署方式: + - storage provider + - S3 / MinIO + - publish 限制配置 +2. 完成评审、提升、下载、搜索的权限策略统一。 +3. 落地幂等与审计最小版。 +4. 收敛 API 路径与 DTO 契约。 + +### T2:1 到 2 个迭代完成 + +1. 引入更稳健的异步一致性方案。 +2. 做前端拆包、性能与体验优化。 +3. 增强发布安全能力和安装完整性校验。 +4. 做更强的产品化能力,例如可视化审核、统计分析、推荐等。 + +## 3. 架构优化建议 + +### 3.1 冻结发布状态机 + +当前最需要收敛的是“发布”和“审核”到底是什么关系。 + +建议明确采用下面其中一种,不要混合: + +方案 A:Phase 2 简化模型 + +- 上传成功即 `PUBLISHED` +- 不存在 `PENDING_REVIEW` +- `latestVersionId` 直接指向新版本 + +方案 B:Phase 3 审核模型 + +- 上传只创建 `DRAFT` +- 显式提交后进入 `PENDING_REVIEW` +- 审核通过后才进入 `PUBLISHED` +- 只有 `PUBLISHED` 版本能影响: + - `latestVersionId` + - `latest` 保留标签 + - 搜索索引 + - 下载入口 + +如果选择方案 B,建议新增下面两个边界对象: + +- `PublishedVersionPointer` + - 只负责“当前可安装版本” +- `DraftVersionView` + - 只负责“作者或审核人能看到的最新草稿/待审版本” + +不要再用一个 `latestVersionId` 同时表达“最新上传版本”和“最新已发布版本”。 + +### 3.2 建立统一授权层 + +当前权限判断分散在: + +- `SecurityConfig` +- Controller +- Domain Service +- `VisibilityChecker` +- `ReviewPermissionChecker` + +建议统一成三层: + +1. `Authentication` + - 只负责身份识别 +2. `Resource Access Policy` + - 只负责“谁能看” +3. `Action Authorization Policy` + - 只负责“谁能做什么” + +推荐拆出统一的 policy 组件,例如: + +- `SkillReadPolicy` +- `SkillPublishPolicy` +- `ReviewPolicy` +- `PromotionPolicy` +- `NamespacePolicy` + +Controller 不再自己拼权限,只把上下文交给 policy。 + +### 3.3 重构上传入口为单独的 Package Ingestion 模块 + +建议把当前“解压 + 校验 + 存储 + bundle 构建 + 元数据提取”从 `SkillPublishController` / `SkillPublishService` 中抽离,形成独立应用服务: + +- `SkillPackageIngestionService` + +职责建议如下: + +- 解析 zip stream +- 路径校验 +- 大小限制 +- 重复路径校验 +- 内容类型判断 +- 哈希计算 +- 生成标准化 manifest +- 输出领域对象 `ValidatedSkillPackage` + +这样好处是: + +- Web / CLI 共享同一套上传逻辑 +- 更容易做流式处理 +- 更容易接入病毒扫描、签名验证、内容审核 + +### 3.4 引入存储补偿与异步一致性机制 + +当前对象存储与数据库事务是“两套事务”,必须承认这一点。 + +建议至少补齐两层机制: + +1. 同步补偿 + - 上传过程中一旦数据库失败,立即尝试删除已写入对象 +2. 异步兜底 + - 定时扫描 orphan object + - 定时补建搜索索引 + +如果未来继续扩展,建议走标准化方案: + +- `transactional outbox` +- 后台 worker +- 幂等消费 + +### 3.5 配置体系收敛成强类型配置 + +当前配置散落在: + +- `application.yml` +- `docker-compose*.yml` +- `@ConditionalOnProperty` +- 硬编码常量 + +建议做一次统一收敛: + +- `SkillhubStorageProperties` +- `SkillhubPublishProperties` +- `SkillhubAuthProperties` +- `SkillhubRateLimitProperties` + +并要求: + +1. 所有运行时行为只从配置类读取。 +2. 业务代码禁止继续写死阈值。 +3. 启动时打印关键配置摘要,方便排障。 + +## 4. 设计优化建议 + +### 4.1 API 路径风格统一 + +当前同时存在两种风格: + +- 面向用户的 namespace/slug 坐标 +- 面向内部的数字 ID 路径 + +建议明确区分: + +- 外部公开 API:统一用 `namespace/slug/version/tag` +- 内部管理或后台 API:可以保留数字 ID + +对外协议越稳定,前端、CLI、第三方集成越容易维护。 + +### 4.2 文档与代码冻结同一份状态机和权限矩阵 + +建议补一份真正可执行的“冻结文档”,只包含这些内容: + +1. 版本状态流转表 +2. skill 可见性矩阵 +3. review / promotion 权限矩阵 +4. token scope 矩阵 +5. 公共 GET 白名单 + +这份文档应该成为: + +- 后端开发实现基准 +- 前端联调基准 +- 测试用例来源 + +### 4.3 Token 设计升级为“角色 + scope”双约束 + +当前 API Token 更像“换一种方式拿到完整用户权限”。 + +建议升级成: + +- `identity` +- `platformRoles` +- `scopes` +- `resourceConstraints` + +例如: + +- `skill:read` +- `skill:publish` +- `review:read` +- `review:approve` +- `namespace:team-a` + +并要求 Filter 只解析身份,具体权限由下游 policy 再次判断。 + +### 4.4 设备码登录设计应独立成完整子协议 + +建议把设备流单独文档化并独立实现,最少包括: + +1. device code 生命周期 +2. user code 生命周期 +3. 轮询频率限制 +4. 授权成功后的单次兑换 +5. access token / refresh token +6. 撤销与过期处理 +7. 审计字段 + +不要把它作为“先写个占位版本”长期保留在主干里。 + +## 5. 功能优化建议 + +### 5.1 发布体验 + +建议在当前基础上补齐: + +1. 上传预检接口 + - 只做包校验,不落库 +2. 发布结果详情页 + - 显示版本状态、校验结果、审计信息 +3. 版本差异展示 + - 新旧文件列表 diff +4. bundle 指纹展示 + - 便于 CLI 做一致性校验 + +### 5.2 审核体验 + +建议增加: + +1. review 队列按 namespace / 状态 / 时间过滤 +2. promotion 队列显示来源 skill 与目标 namespace +3. 审核意见模板 +4. 审核历史轨迹 + +### 5.3 安装与下载体验 + +建议补齐: + +1. 下载返回指纹 +2. CLI 安装后校验 hash +3. “latest” 与自定义 tag 的解析提示 +4. 私有 skill 访问失败时更清晰的错误码 + +### 5.4 搜索体验 + +建议按阶段演进: + +1. 先把权限过滤、排序、分页做稳定 +2. 再补关键词高亮、标签过滤、namespace 过滤 +3. 最后再考虑向量搜索或推荐 + +先把 correctness 做扎实,比过早上复杂搜索引擎更重要。 + +## 6. 创新优化建议 + +### 6.1 增加发布安全链 + +可以在上传后增加可插拔安全扫描链: + +- 文件白名单校验 +- 敏感内容扫描 +- 恶意脚本特征扫描 +- 许可证扫描 +- 依赖清单提取 + +这部分可以通过 `PrePublishValidator` 真正扩展,而不是继续 `NoOp`。 + +### 6.2 增加制品签名与可验证安装 + +建议为 bundle 增加: + +- 版本指纹 +- 服务端签名 +- CLI 验签 + +这样 SkillHub 才更像一个可被信任的官方分发源,而不只是文件托管站。 + +### 6.3 增加“来源关系”与“派生关系”图谱 + +当前 promotion 已经有 `sourceSkillId` 雏形,后面可以扩展成: + +- fork / promote / mirror / upstream + +这能支撑: + +- 技能来源追踪 +- 派生链可视化 +- 全局版与团队版差异说明 + +## 7. 迭代优化建议 + +### 7.1 推荐实施顺序 + +第一阶段:安全与正确性收口 + +- 设备码登录 +- review / promotion 鉴权 +- skills GET 白名单 +- 上传路径安全 +- latest 语义修复 + +第二阶段:工程化基线收口 + +- 测试全绿 +- 配置统一 +- 存储 provider 打通 +- 文档与代码对齐 +- 前端 lint 与拆包 + +第三阶段:产品化能力增强 + +- 审核后台 +- 审计日志 +- 幂等与回放 +- 更完整的 CLI 体验 + +第四阶段:平台化能力增强 + +- 签名安装 +- 安全扫描 +- 推荐与搜索增强 +- 可观测性与运营报表 + +### 7.2 每阶段验收标准 + +建议引入明确验收门槛: + +第一阶段验收: + +- 不再存在 P0 / P1 安全问题 +- `mvn test` 通过 +- `pnpm lint && pnpm build` 通过 + +第二阶段验收: + +- 配置切换本地/S3 存储可实测 +- 发布/审核/下载链路与文档一致 +- 私有 skill 权限边界有自动化测试 + +第三阶段验收: + +- review / promotion / audit 有可视化页面 +- CLI 对外可稳定联调 + +## 8. 建议立即补的文档 + +为了让后续开发真正顺利,建议马上追加 4 份冻结文档: + +1. `权限矩阵冻结文档` +2. `发布状态机冻结文档` +3. `上传与存储安全规范` +4. `API Token / Device Flow 协议冻结文档` + +这 4 份文档会比继续补泛化架构图更有实际开发价值。 + +## 9. 最终建议 + +SkillHub 目前最值得保留的是: + +- 模块化单体拆分方向 +- 领域服务初步分层 +- review / promotion / search / storage 的边界意识 + +最需要马上收口的是: + +- 权限模型 +- 发布状态机 +- 上传安全 +- 配置一致性 +- 测试基线 + +只要先把这五件事修稳,后面的架构优化和产品创新都能顺着推进;如果不先修,越往后叠功能,返工就越贵。