From 1b09ab88a2c03497bd451b66c00f341060d7b163 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Wed, 10 Jun 2026 14:19:41 +0800 Subject: [PATCH] fix(bootstrap): enforce strict builtin skill skips Signed-off-by: dongmucat <1127093059@qq.com> --- docs/20-cloud-url-builtin-skills-setup.md | 6 ++--- .../bootstrap/BuiltinSkillInitializer.java | 15 +++++------ .../BuiltinSkillPackageExtractor.java | 4 +++ .../BuiltinSkillInitializerTest.java | 27 +++++-------------- .../BuiltinSkillPackageExtractorTest.java | 15 +++++++++++ 5 files changed, 34 insertions(+), 33 deletions(-) diff --git a/docs/20-cloud-url-builtin-skills-setup.md b/docs/20-cloud-url-builtin-skills-setup.md index 316faa8d..7de92d63 100644 --- a/docs/20-cloud-url-builtin-skills-setup.md +++ b/docs/20-cloud-url-builtin-skills-setup.md @@ -173,9 +173,9 @@ skillhub-hello-1.0.0.zip | `@global/{slug}` 已存在，owner 是 `builtin-skill-publisher`，但目标版本不存在 | 发布新版本 | | 同版本已存在且已发布 | 下载前跳过 | | 同版本已存在但不是 `PUBLISHED` | 下载前跳过并记录日志 | -| `@global/{slug}` 已被其他 owner 发布 | 下载前跳过并记录 warning | +| `@global/{slug}` 已被其他 owner 创建或发布 | 下载前跳过并记录 warning | -这意味着内置同步不会接管用户或管理员已经发布的同 slug Skill；仅有待审或未发布版本不会阻断内置同步。 +这意味着内置同步不会接管用户或管理员已经创建的同 slug Skill；即使该 Skill 仍处于待审、未发布或已拒绝状态，也会跳过对应 manifest item。 同版本已存在时，同步器不会重新下载远端 zip，也不会验证远端对象内容是否发生漂移。 如果多实例同时启动，可能出现多个实例同时尝试发布同一个内置版本。同步器会在发布失败后重新查询目标版本；如果发现同版本已经以相同内容发布成功，则视为并发场景下的正常跳过。 @@ -249,7 +249,7 @@ SKILLHUB_BUILTIN_SKILLS_ENABLED=false | package download failed | 检查云存储对象是否存在、是否返回 HTTP 200、是否超时 | | package must contain SKILL.md | 检查 zip 是否存在唯一可识别的 `SKILL.md` 入口 | | manifest version does not match package version | 检查 manifest `version` 和 `SKILL.md version` 是否一致 | -| slug is already published by another user | 说明 `@global/{slug}` 已被非内置发布者发布，内置同步不会覆盖 | +| slug already belongs to another user | 说明 `@global/{slug}` 已被非内置发布者创建或发布，内置同步不会覆盖 | | published fingerprint differs | 并发发布异常后发现同一内置版本已存在但内容不同，需要人工确认是否发生了版本冲突 | 如果某个 manifest item 失败，后续 item 仍会继续处理，应用可用状态不受影响。 diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java index 5145c404..34379db7 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java @@ -258,8 +258,8 @@ public class BuiltinSkillInitializer { private boolean shouldSkipBeforeDownload(Long namespaceId, ManifestItem item) { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); - if (hasPublishedOtherOwnerConflict(existingSkills)) { - log.warn("Skipping built-in skill slug={} before download because the slug is already published by another user", + if (hasOtherOwnerConflict(existingSkills)) { + log.warn("Skipping built-in skill slug={} before download because the slug already belongs to another user", item.slug()); return true; } @@ -290,8 +290,8 @@ public class BuiltinSkillInitializer { private boolean shouldSkipExisting(Long namespaceId, ManifestItem item, List entries) { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); - if (hasPublishedOtherOwnerConflict(existingSkills)) { - log.warn("Skipping built-in skill slug={} because the slug is already published by another user", + if (hasOtherOwnerConflict(existingSkills)) { + log.warn("Skipping built-in skill slug={} because the slug already belongs to another user", item.slug()); return true; } @@ -360,12 +360,9 @@ public class BuiltinSkillInitializer { return false; } - private boolean hasPublishedOtherOwnerConflict(List existingSkills) { + private boolean hasOtherOwnerConflict(List existingSkills) { return existingSkills.stream() - .filter(skill -> !SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())) - .anyMatch(skill -> !skillVersionRepository - .findBySkillIdAndStatus(skill.getId(), SkillVersionStatus.PUBLISHED) - .isEmpty()); + .anyMatch(skill -> !SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())); } private String computeFingerprint(SkillVersion version) { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractor.java index d023190d..973dd5b2 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractor.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractor.java @@ -21,6 +21,10 @@ public class BuiltinSkillPackageExtractor { public SkillPackageArchiveExtractor.ExtractionResult extract(byte[] zipBytes) throws IOException { SkillPackageArchiveExtractor.ExtractionResult result = archiveExtractor.extractWithWarnings(new ByteArrayMultipartFile(zipBytes)); + if (!result.warnings().isEmpty()) { + throw new IllegalArgumentException("Built-in skill package has warnings: " + + String.join("; ", result.warnings())); + } boolean hasSkillMd = result.entries().stream() .anyMatch(entry -> SkillPackagePolicy.SKILL_MD_PATH.equals(entry.path())); if (!hasSkillMd) { diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java index 01310147..c2b5a836 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java @@ -184,13 +184,10 @@ class BuiltinSkillInitializerTest { } @Test - void skipsPublishedSkillOwnedByAnotherUserBeforeDownloadingPackage() throws Exception { + void skipsSkillOwnedByAnotherUserBeforeDownloadingPackage() { Skill otherSkill = skill(100L, "skillhub-hello", "someone-else"); - SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); givenManifestAndSystemPublisher(); when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(otherSkill)); - when(skillVersionRepository.findBySkillIdAndStatus(100L, SkillVersionStatus.PUBLISHED)) - .thenReturn(List.of(published)); runInitializer(); @@ -199,27 +196,15 @@ class BuiltinSkillInitializerTest { } @Test - void publishesWhenOnlyUnpublishedSkillOwnedByAnotherUserExists() throws Exception { + void skipsUnpublishedSkillOwnedByAnotherUserBeforeDownloadingPackage() { Skill otherSkill = skill(100L, "skillhub-hello", "someone-else"); - List entries = packageEntries("skillhub-hello", "1.0.0", "same"); - givenExtractedPackage(entries); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")) - .thenReturn(List.of(otherSkill)) - .thenReturn(List.of(otherSkill)); - lenient().when(skillVersionRepository.findBySkillIdAndStatus(100L, SkillVersionStatus.PUBLISHED)) - .thenReturn(List.of()); + givenManifestAndSystemPublisher(); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(otherSkill)); runInitializer(); - verify(downloader).download(URI.create(ITEM.url())); - verify(skillPublishService).publishFromEntries( - eq(GLOBAL), - eq(entries), - eq(PUBLISHER), - eq(SkillVisibility.PUBLIC), - eq(Set.of("SUPER_ADMIN")), - eq(false) - ); + verify(downloader, never()).download(any()); + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } @Test diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractorTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractorTest.java index 8836546f..8f1a5397 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractorTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractorTest.java @@ -65,6 +65,21 @@ class BuiltinSkillPackageExtractorTest { .containsExactly("SKILL.md", "README.md"); } + @Test + void rejectsZipWhenSkillDirectoryPromotionWouldIgnoreOutsideFiles() throws Exception { + byte[] zip = zip(entry("skillhub-hello/SKILL.md", """ + --- + name: skillhub-hello + version: 1.0.0 + --- + # SkillHub Hello + """), entry("LICENSE", "Apache-2.0")); + + assertThatThrownBy(() -> extractor.extract(zip)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Ignored file outside skill directory: LICENSE"); + } + private static ZipSource entry(String path, String content) { return new ZipSource(path, content.getBytes(StandardCharsets.UTF_8)); }