mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-09 22:31:14 +00:00
feature : add review doc
This commit is contained in:
parent
f47377cff1
commit
f952a6e274
2 changed files with 768 additions and 0 deletions
363
docs/review/代码审查报告.md
Normal file
363
docs/review/代码审查报告.md
Normal file
|
|
@ -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<String, Object>`,测试环境没有对应 bean。
|
||||
- 代码里也没有看到 `verificationUri` 的配置注入实现,当前构造器还要求额外的 `String` 参数,后续即使补齐 Redis bean,也大概率还会继续失败。
|
||||
|
||||
影响:
|
||||
|
||||
- 目前 controller 层测试的 19 个错误都被同一个装配问题掩盖,真实回归无法被发现。
|
||||
- CI 无法提供有效的回归保护。
|
||||
|
||||
修改建议:
|
||||
|
||||
1. 为设备码服务补全正式配置类:
|
||||
- `RedisTemplate<String, DeviceCodeData>`
|
||||
- `@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` 全绿。
|
||||
|
||||
在这五项完成之前,后续功能开发会持续叠加在一个不稳定基线上,返工概率较高。
|
||||
405
docs/review/后期优化报告.md
Normal file
405
docs/review/后期优化报告.md
Normal file
|
|
@ -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 的边界意识
|
||||
|
||||
最需要马上收口的是:
|
||||
|
||||
- 权限模型
|
||||
- 发布状态机
|
||||
- 上传安全
|
||||
- 配置一致性
|
||||
- 测试基线
|
||||
|
||||
只要先把这五件事修稳,后面的架构优化和产品创新都能顺着推进;如果不先修,越往后叠功能,返工就越贵。
|
||||
Loading…
Add table
Reference in a new issue