mirror of
https://github.com/iflytek/skillhub.git
synced 2026-10-11 03:37:57 +00:00
fix(bootstrap): enforce strict builtin skill skips
Signed-off-by: dongmucat <1127093059@qq.com>
This commit is contained in:
parent
dd3e511a91
commit
1b09ab88a2
5 changed files with 34 additions and 33 deletions
|
|
@ -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 仍会继续处理,应用可用状态不受影响。
|
||||
|
|
|
|||
|
|
@ -258,8 +258,8 @@ public class BuiltinSkillInitializer {
|
|||
|
||||
private boolean shouldSkipBeforeDownload(Long namespaceId, ManifestItem item) {
|
||||
List<Skill> 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<PackageEntry> entries) {
|
||||
List<Skill> 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<Skill> existingSkills) {
|
||||
private boolean hasOtherOwnerConflict(List<Skill> 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) {
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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<PackageEntry> 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
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue