From 9bb6b31db70daa0d4a42bf95e064e6bf60cc3bb6 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Fri, 5 Jun 2026 16:16:55 +0800 Subject: [PATCH 01/11] feat(bootstrap): sync built-in skills from cloud manifest Signed-off-by: dongmucat <1127093059@qq.com> --- .../iflytek/skillhub/SkillhubApplication.java | 7 +- .../bootstrap/BuiltinSkillInitializer.java | 333 +++++++++++++++++ .../bootstrap/BuiltinSkillManifestLoader.java | 108 ++++++ .../BuiltinSkillPackageExtractor.java | 90 +++++ .../bootstrap/BuiltinSkillProperties.java | 17 + .../BuiltinSkillRemotePackageDownloader.java | 142 +++++++ .../src/main/resources/application.yml | 2 + .../resources/builtin-skills/manifest.json | 3 + .../BuiltinSkillInitializerTest.java | 347 ++++++++++++++++++ .../BuiltinSkillManifestLoaderTest.java | 157 ++++++++ .../BuiltinSkillPackageExtractorTest.java | 84 +++++ .../BuiltinSkillPropertiesBindingTest.java | 47 +++ ...iltinSkillRemotePackageDownloaderTest.java | 219 +++++++++++ 13 files changed, 1555 insertions(+), 1 deletion(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoader.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractor.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillProperties.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloader.java create mode 100644 server/skillhub-app/src/main/resources/builtin-skills/manifest.json create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoaderTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractorTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPropertiesBindingTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloaderTest.java diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/SkillhubApplication.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/SkillhubApplication.java index bef19708..71ee7de8 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/SkillhubApplication.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/SkillhubApplication.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub; +import com.iflytek.skillhub.bootstrap.BuiltinSkillProperties; import com.iflytek.skillhub.config.ProfileFieldPolicyProperties; import com.iflytek.skillhub.config.ProfileModerationProperties; import org.springframework.boot.SpringApplication; @@ -10,7 +11,11 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties * Main Spring Boot entry point for the SkillHub backend application. */ @SpringBootApplication -@EnableConfigurationProperties({ProfileModerationProperties.class, ProfileFieldPolicyProperties.class}) +@EnableConfigurationProperties({ + BuiltinSkillProperties.class, + ProfileModerationProperties.class, + ProfileFieldPolicyProperties.class +}) public class SkillhubApplication { public static void main(String[] args) { SpringApplication.run(SkillhubApplication.class, args); 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 new file mode 100644 index 00000000..7ff69a68 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java @@ -0,0 +1,333 @@ +package com.iflytek.skillhub.bootstrap; + +import com.iflytek.skillhub.bootstrap.BuiltinSkillManifestLoader.ManifestItem; +import com.iflytek.skillhub.controller.support.SkillPackageArchiveExtractor; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.SlugValidator; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillFile; +import com.iflytek.skillhub.domain.skill.SkillFileRepository; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.metadata.SkillMetadata; +import com.iflytek.skillhub.domain.skill.metadata.SkillMetadataParser; +import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.validation.PackageEntry; +import com.iflytek.skillhub.domain.skill.validation.SkillPackagePolicy; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.ApplicationArguments; +import org.springframework.boot.ApplicationRunner; +import org.springframework.stereotype.Component; + +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.util.Comparator; +import java.util.HexFormat; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +/** + * Best-effort startup synchronizer for remotely hosted built-in skill packages. + */ +@Component +public class BuiltinSkillInitializer implements ApplicationRunner { + + static final String GLOBAL_NAMESPACE = "global"; + static final String SYSTEM_PUBLISHER_ID = "builtin-skill-publisher"; + + private static final Logger log = LoggerFactory.getLogger(BuiltinSkillInitializer.class); + private static final Set SYSTEM_PUBLISHER_ROLES = Set.of("SUPER_ADMIN"); + + private final BuiltinSkillProperties properties; + private final BuiltinSkillManifestLoader manifestLoader; + private final BuiltinSkillRemotePackageDownloader downloader; + private final BuiltinSkillPackageExtractor extractor; + private final SkillMetadataParser metadataParser; + private final NamespaceRepository namespaceRepository; + private final NamespaceMemberRepository namespaceMemberRepository; + private final UserAccountRepository userAccountRepository; + private final SkillRepository skillRepository; + private final SkillVersionRepository skillVersionRepository; + private final SkillFileRepository skillFileRepository; + private final SkillPublishService skillPublishService; + + public BuiltinSkillInitializer( + BuiltinSkillProperties properties, + BuiltinSkillManifestLoader manifestLoader, + BuiltinSkillRemotePackageDownloader downloader, + BuiltinSkillPackageExtractor extractor, + SkillMetadataParser metadataParser, + NamespaceRepository namespaceRepository, + NamespaceMemberRepository namespaceMemberRepository, + UserAccountRepository userAccountRepository, + SkillRepository skillRepository, + SkillVersionRepository skillVersionRepository, + SkillFileRepository skillFileRepository, + SkillPublishService skillPublishService) { + this.properties = properties; + this.manifestLoader = manifestLoader; + this.downloader = downloader; + this.extractor = extractor; + this.metadataParser = metadataParser; + this.namespaceRepository = namespaceRepository; + this.namespaceMemberRepository = namespaceMemberRepository; + this.userAccountRepository = userAccountRepository; + this.skillRepository = skillRepository; + this.skillVersionRepository = skillVersionRepository; + this.skillFileRepository = skillFileRepository; + this.skillPublishService = skillPublishService; + } + + @Override + public void run(ApplicationArguments args) { + if (!properties.isEnabled()) { + log.info("Built-in skill startup synchronization is disabled"); + return; + } + + Optional namespace = namespaceRepository.findBySlug(GLOBAL_NAMESPACE); + if (namespace.isEmpty()) { + log.warn("Global namespace '{}' does not exist, skipping built-in skill synchronization", + GLOBAL_NAMESPACE); + return; + } + + List items = manifestLoader.load(); + if (items.isEmpty()) { + log.info("No built-in skill manifest items to synchronize"); + return; + } + + try { + ensureSystemPublisher(namespace.get()); + } catch (RuntimeException exception) { + log.error("Failed to initialize built-in skill system publisher, skipping synchronization: {}", + exception.getMessage(), exception); + return; + } + + for (ManifestItem item : items) { + try { + syncItem(namespace.get(), item); + } catch (Exception exception) { + log.error( + "Failed to synchronize built-in skill slug={} version={}: {}", + item.slug(), + item.version(), + exception.getMessage(), + exception + ); + } + } + } + + private void ensureSystemPublisher(Namespace namespace) { + userAccountRepository.findById(SYSTEM_PUBLISHER_ID) + .orElseGet(() -> userAccountRepository.save(new UserAccount( + SYSTEM_PUBLISHER_ID, + "Built-in Skill Publisher", + null, + null + ))); + + if (namespaceMemberRepository.findByNamespaceIdAndUserId(namespace.getId(), SYSTEM_PUBLISHER_ID).isEmpty()) { + namespaceMemberRepository.save(new NamespaceMember( + namespace.getId(), + SYSTEM_PUBLISHER_ID, + NamespaceRole.OWNER + )); + } + } + + private void syncItem(Namespace namespace, ManifestItem item) throws Exception { + Optional packageBytes = downloader.download(URI.create(item.url())); + if (packageBytes.isEmpty()) { + log.warn("Skipping built-in skill slug={} version={} because package download failed", + item.slug(), item.version()); + return; + } + + SkillPackageArchiveExtractor.ExtractionResult extractionResult = extractor.extract(packageBytes.get()); + List entries = extractionResult.entries(); + SkillMetadata metadata = parseSkillMetadata(entries); + String packageSlug = SlugValidator.slugify(metadata.name()); + if (!item.slug().equals(packageSlug)) { + log.warn( + "Skipping built-in skill manifest slug={} version={} because package slug is {}", + item.slug(), + item.version(), + packageSlug + ); + return; + } + if (!item.version().equals(metadata.version())) { + log.warn( + "Skipping built-in skill slug={} because manifest version {} does not match package version {}", + item.slug(), + item.version(), + metadata.version() + ); + return; + } + + if (shouldSkipExisting(namespace.getId(), item, entries)) { + return; + } + + try { + skillPublishService.publishFromEntries( + GLOBAL_NAMESPACE, + entries, + SYSTEM_PUBLISHER_ID, + SkillVisibility.PUBLIC, + SYSTEM_PUBLISHER_ROLES, + false + ); + log.info("Published built-in skill slug={} version={} to @{}", + item.slug(), item.version(), GLOBAL_NAMESPACE); + } catch (RuntimeException exception) { + if (isAlreadyPublishedWithSameFingerprint(namespace.getId(), item, entries)) { + log.info("Built-in skill slug={} version={} was published concurrently, skipping", + item.slug(), item.version()); + return; + } + log.error("Failed to publish built-in skill slug={} version={}: {}", + item.slug(), item.version(), exception.getMessage(), exception); + } + } + + private SkillMetadata parseSkillMetadata(List entries) { + PackageEntry skillMd = entries.stream() + .filter(entry -> SkillPackagePolicy.SKILL_MD_PATH.equals(entry.path())) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException( + "Built-in skill package must contain " + SkillPackagePolicy.SKILL_MD_PATH)); + return metadataParser.parse(new String(skillMd.content(), StandardCharsets.UTF_8)); + } + + private boolean shouldSkipExisting(Long namespaceId, ManifestItem item, List entries) { + List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); + boolean hasNonBuiltinOwner = existingSkills.stream() + .anyMatch(skill -> !SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())); + if (hasNonBuiltinOwner) { + log.warn("Skipping built-in skill slug={} because the slug is already owned by another user", + item.slug()); + return true; + } + + Optional builtinSkill = existingSkills.stream() + .filter(skill -> SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())) + .findFirst(); + if (builtinSkill.isEmpty()) { + return false; + } + + Optional existingVersion = skillVersionRepository + .findBySkillIdAndVersion(builtinSkill.get().getId(), item.version()); + if (existingVersion.isEmpty()) { + return false; + } + + SkillVersion version = existingVersion.get(); + if (version.getStatus() != SkillVersionStatus.PUBLISHED) { + log.info("Skipping built-in skill slug={} version={} because existing version status is {}", + item.slug(), item.version(), version.getStatus()); + return true; + } + + String packageFingerprint = computeFingerprint(entries); + String existingFingerprint = computeFingerprint(version); + if (packageFingerprint.equals(existingFingerprint)) { + log.info("Skipping built-in skill slug={} version={} because it is already published", + item.slug(), item.version()); + } else { + log.warn( + "Skipping built-in skill slug={} version={} because published fingerprint differs: existing={}, package={}", + item.slug(), + item.version(), + existingFingerprint, + packageFingerprint + ); + } + return true; + } + + private boolean isAlreadyPublishedWithSameFingerprint(Long namespaceId, ManifestItem item, List entries) { + List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); + for (Skill skill : existingSkills) { + if (!SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())) { + continue; + } + Optional version = skillVersionRepository + .findBySkillIdAndVersion(skill.getId(), item.version()); + if (version.isPresent() && version.get().getStatus() == SkillVersionStatus.PUBLISHED) { + String packageFingerprint = computeFingerprint(entries); + String existingFingerprint = computeFingerprint(version.get()); + if (packageFingerprint.equals(existingFingerprint)) { + return true; + } + log.warn( + "Built-in skill slug={} version={} was published concurrently with different content: existing={}, package={}", + item.slug(), + item.version(), + existingFingerprint, + packageFingerprint + ); + return false; + } + } + return false; + } + + private String computeFingerprint(SkillVersion version) { + List files = skillFileRepository.findByVersionId(version.getId()).stream() + .sorted(Comparator.comparing(SkillFile::getFilePath)) + .toList(); + return computeFingerprintFromFileDigests(files.stream() + .map(file -> new FileDigest(file.getFilePath(), file.getSha256())) + .toList()); + } + + private String computeFingerprint(List entries) { + return computeFingerprintFromFileDigests(entries.stream() + .map(entry -> new FileDigest(entry.path(), sha256(entry.content()))) + .toList()); + } + + private String computeFingerprintFromFileDigests(List files) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + for (FileDigest file : files.stream().sorted(Comparator.comparing(FileDigest::path)).toList()) { + String line = file.path() + ":" + file.sha256() + "\n"; + digest.update(line.getBytes(StandardCharsets.UTF_8)); + } + return "sha256:" + HexFormat.of().formatHex(digest.digest()); + } catch (Exception exception) { + throw new IllegalStateException("Failed to compute built-in skill fingerprint", exception); + } + } + + private static String sha256(byte[] content) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + return HexFormat.of().formatHex(digest.digest(content)); + } catch (Exception exception) { + throw new IllegalStateException("Failed to compute built-in skill file digest", exception); + } + } + + private record FileDigest(String path, String sha256) { + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoader.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoader.java new file mode 100644 index 00000000..487179a4 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoader.java @@ -0,0 +1,108 @@ +package com.iflytek.skillhub.bootstrap; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.namespace.SlugValidator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.core.io.Resource; +import org.springframework.core.io.ResourceLoader; +import org.springframework.stereotype.Component; +import org.springframework.util.StringUtils; + +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +@Component +public class BuiltinSkillManifestLoader { + + static final String MANIFEST_LOCATION = "classpath:builtin-skills/manifest.json"; + static final int MAX_ITEMS = 100; + + private static final Logger log = LoggerFactory.getLogger(BuiltinSkillManifestLoader.class); + + private final ObjectMapper objectMapper; + private final ResourceLoader resourceLoader; + + public BuiltinSkillManifestLoader(ObjectMapper objectMapper, ResourceLoader resourceLoader) { + this.objectMapper = objectMapper; + this.resourceLoader = resourceLoader; + } + + public List load() { + Resource resource = resourceLoader.getResource(MANIFEST_LOCATION); + if (!resource.exists()) { + log.warn("Built-in skill manifest not found at {}", MANIFEST_LOCATION); + return List.of(); + } + + JsonNode root; + try (InputStream inputStream = resource.getInputStream()) { + root = objectMapper.readTree(inputStream); + } catch (IOException | RuntimeException ex) { + log.warn("Failed to read built-in skill manifest at {}: {}", MANIFEST_LOCATION, ex.getMessage()); + return List.of(); + } + + if (root == null || root.isNull()) { + log.warn("Built-in skill manifest at {} is empty", MANIFEST_LOCATION); + return List.of(); + } + + JsonNode skillsNode = root.path("skills"); + if (!skillsNode.isArray()) { + log.warn("Built-in skill manifest at {} does not contain an array field 'skills'", MANIFEST_LOCATION); + return List.of(); + } + + List items = new ArrayList<>(); + Set seenSlugVersions = new HashSet<>(); + int totalEntries = skillsNode.size(); + if (totalEntries > MAX_ITEMS) { + log.warn("Built-in skill manifest has {} entries, only the first {} entries will be processed", + totalEntries, MAX_ITEMS); + } + int limit = Math.min(totalEntries, MAX_ITEMS); + for (int index = 0; index < limit; index++) { + JsonNode itemNode = skillsNode.get(index); + String slug = text(itemNode, "slug"); + String version = text(itemNode, "version"); + String url = text(itemNode, "url"); + if (!StringUtils.hasText(slug) || !StringUtils.hasText(version) || !StringUtils.hasText(url)) { + log.warn("Skipping built-in skill manifest item {} because slug, version, and url are required", index); + continue; + } + try { + SlugValidator.validate(slug); + } catch (RuntimeException ex) { + log.warn("Skipping built-in skill manifest item {} because slug is invalid [slug={}]: {}", + index, slug, ex.getMessage()); + continue; + } + + String key = slug + "\n" + version; + if (!seenSlugVersions.add(key)) { + log.warn("Skipping duplicate built-in skill manifest item for slug={} version={}", slug, version); + continue; + } + + items.add(new ManifestItem(slug, version, url)); + } + return List.copyOf(items); + } + + private static String text(JsonNode node, String fieldName) { + JsonNode value = node.get(fieldName); + if (value == null || !value.isTextual()) { + return ""; + } + return value.asText().trim(); + } + + public record ManifestItem(String slug, String version, String url) { + } +} 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 new file mode 100644 index 00000000..9cd44cd6 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractor.java @@ -0,0 +1,90 @@ +package com.iflytek.skillhub.bootstrap; + +import com.iflytek.skillhub.controller.support.SkillPackageArchiveExtractor; +import com.iflytek.skillhub.domain.skill.validation.SkillPackagePolicy; +import org.springframework.stereotype.Component; +import org.springframework.web.multipart.MultipartFile; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; + +@Component +public class BuiltinSkillPackageExtractor { + + private final SkillPackageArchiveExtractor archiveExtractor; + + public BuiltinSkillPackageExtractor(SkillPackageArchiveExtractor archiveExtractor) { + this.archiveExtractor = archiveExtractor; + } + + public SkillPackageArchiveExtractor.ExtractionResult extract(byte[] zipBytes) throws IOException { + assertRootSkillMd(zipBytes); + SkillPackageArchiveExtractor.ExtractionResult result = + archiveExtractor.extractWithWarnings(new ByteArrayMultipartFile(zipBytes)); + boolean hasRootSkillMd = result.entries().stream() + .anyMatch(entry -> SkillPackagePolicy.SKILL_MD_PATH.equals(entry.path())); + if (!hasRootSkillMd) { + throw new IllegalArgumentException("Built-in skill package must contain root " + SkillPackagePolicy.SKILL_MD_PATH); + } + return result; + } + + private void assertRootSkillMd(byte[] zipBytes) throws IOException { + try (ZipInputStream zipInputStream = new ZipInputStream(new ByteArrayInputStream(zipBytes))) { + ZipEntry entry; + while ((entry = zipInputStream.getNextEntry()) != null) { + if (!entry.isDirectory() && SkillPackagePolicy.SKILL_MD_PATH.equals(entry.getName())) { + return; + } + zipInputStream.closeEntry(); + } + } + throw new IllegalArgumentException("Built-in skill package must contain root " + SkillPackagePolicy.SKILL_MD_PATH); + } + + private record ByteArrayMultipartFile(byte[] bytes) implements MultipartFile { + + @Override + public String getName() { + return "file"; + } + + @Override + public String getOriginalFilename() { + return "builtin-skill.zip"; + } + + @Override + public String getContentType() { + return "application/zip"; + } + + @Override + public boolean isEmpty() { + return bytes.length == 0; + } + + @Override + public long getSize() { + return bytes.length; + } + + @Override + public byte[] getBytes() { + return bytes.clone(); + } + + @Override + public InputStream getInputStream() { + return new ByteArrayInputStream(bytes); + } + + @Override + public void transferTo(java.io.File dest) throws IOException { + throw new UnsupportedOperationException("Built-in skill zip adapter is read-only"); + } + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillProperties.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillProperties.java new file mode 100644 index 00000000..6fd3626d --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillProperties.java @@ -0,0 +1,17 @@ +package com.iflytek.skillhub.bootstrap; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +@ConfigurationProperties(prefix = "skillhub.builtin-skills") +public class BuiltinSkillProperties { + + private boolean enabled = true; + + public boolean isEnabled() { + return enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloader.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloader.java new file mode 100644 index 00000000..c2819d3f --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloader.java @@ -0,0 +1,142 @@ +package com.iflytek.skillhub.bootstrap; + +import com.iflytek.skillhub.config.SkillPublishProperties; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.time.Duration; +import java.util.Locale; +import java.util.Optional; +import java.util.regex.Pattern; + +@Component +public class BuiltinSkillRemotePackageDownloader { + + static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(5); + static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(30); + static final String ALLOWED_HOST = "bjcdn.openstorage.cn"; + + private static final Logger log = LoggerFactory.getLogger(BuiltinSkillRemotePackageDownloader.class); + private static final Pattern IPV4_LITERAL = Pattern.compile("\\d{1,3}(\\.\\d{1,3}){3}"); + + private final long maxPackageSize; + private final HttpClient httpClient; + + @Autowired + public BuiltinSkillRemotePackageDownloader(SkillPublishProperties properties) { + this( + properties, + HttpClient.newBuilder() + .connectTimeout(CONNECT_TIMEOUT) + .followRedirects(HttpClient.Redirect.NEVER) + .build() + ); + } + + BuiltinSkillRemotePackageDownloader(SkillPublishProperties properties, HttpClient httpClient) { + this.maxPackageSize = properties.getMaxPackageSize(); + this.httpClient = httpClient; + } + + public Optional download(URI uri) { + if (!isAllowedUrl(uri)) { + log.warn("Skipping built-in skill package download because URL is not allowed: {}", safeUrl(uri)); + return Optional.empty(); + } + + HttpRequest request = HttpRequest.newBuilder(uri) + .timeout(REQUEST_TIMEOUT) + .GET() + .build(); + try { + HttpResponse response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream()); + if (response.statusCode() != 200) { + log.warn("Failed to download built-in skill package from {}: HTTP {}", + safeUrl(uri), + response.statusCode()); + return Optional.empty(); + } + try (InputStream body = response.body()) { + return readBounded(body); + } + } catch (IOException ex) { + log.warn("Failed to download built-in skill package from {}: {}", safeUrl(uri), ex.getMessage()); + return Optional.empty(); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + log.warn("Interrupted while downloading built-in skill package from {}", safeUrl(uri)); + return Optional.empty(); + } catch (RuntimeException ex) { + log.warn("Failed to download built-in skill package from {}: {}", safeUrl(uri), ex.getMessage()); + return Optional.empty(); + } + } + + HttpClient httpClient() { + return httpClient; + } + + static boolean isAllowedUrl(URI uri) { + if (uri == null || !"https".equalsIgnoreCase(uri.getScheme())) { + return false; + } + if (uri.getRawUserInfo() != null) { + return false; + } + int port = uri.getPort(); + if (port != -1 && port != 443) { + return false; + } + String host = uri.getHost(); + if (host == null) { + return false; + } + String normalizedHost = host.toLowerCase(Locale.ROOT); + if (isDisallowedHostLiteral(normalizedHost)) { + return false; + } + return normalizedHost.equals(ALLOWED_HOST) || normalizedHost.endsWith("." + ALLOWED_HOST); + } + + private Optional readBounded(InputStream inputStream) throws IOException { + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + byte[] buffer = new byte[8192]; + long totalRead = 0; + int read; + while ((read = inputStream.read(buffer)) != -1) { + totalRead += read; + if (totalRead > maxPackageSize) { + log.warn("Built-in skill package download exceeded max package size: {} bytes (max: {})", + totalRead, + maxPackageSize); + return Optional.empty(); + } + outputStream.write(buffer, 0, read); + } + return Optional.of(outputStream.toByteArray()); + } + + private static boolean isDisallowedHostLiteral(String host) { + return "localhost".equals(host) + || IPV4_LITERAL.matcher(host).matches() + || host.contains(":"); + } + + private static String safeUrl(URI uri) { + if (uri == null) { + return ""; + } + String host = uri.getHost(); + String path = uri.getRawPath(); + return (host == null ? "" : host) + (path == null ? "" : path); + } +} diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index a592b035..7d9a1cbc 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -93,6 +93,8 @@ spring: enable: ${SPRING_MAIL_SMTP_STARTTLS_ENABLE:false} skillhub: + builtin-skills: + enabled: ${SKILLHUB_BUILTIN_SKILLS_ENABLED:true} auth: mock: enabled: ${SKILLHUB_AUTH_MOCK_ENABLED:false} diff --git a/server/skillhub-app/src/main/resources/builtin-skills/manifest.json b/server/skillhub-app/src/main/resources/builtin-skills/manifest.json new file mode 100644 index 00000000..2b485e41 --- /dev/null +++ b/server/skillhub-app/src/main/resources/builtin-skills/manifest.json @@ -0,0 +1,3 @@ +{ + "skills": [] +} 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 new file mode 100644 index 00000000..4e3322bb --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java @@ -0,0 +1,347 @@ +package com.iflytek.skillhub.bootstrap; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.bootstrap.BuiltinSkillManifestLoader.ManifestItem; +import com.iflytek.skillhub.controller.support.SkillPackageArchiveExtractor; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillFile; +import com.iflytek.skillhub.domain.skill.SkillFileRepository; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.metadata.SkillMetadataParser; +import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.validation.PackageEntry; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.boot.DefaultApplicationArguments; +import org.springframework.test.util.ReflectionTestUtils; + +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.util.HexFormat; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +@ExtendWith(MockitoExtension.class) +class BuiltinSkillInitializerTest { + + private static final String GLOBAL = "global"; + private static final String PUBLISHER = "builtin-skill-publisher"; + private static final ManifestItem ITEM = new ManifestItem( + "agentguard", + "1.0.0", + "https://bjcdn.openstorage.cn/skills/agentguard.zip" + ); + + @Mock private BuiltinSkillManifestLoader manifestLoader; + @Mock private BuiltinSkillRemotePackageDownloader downloader; + @Mock private BuiltinSkillPackageExtractor extractor; + @Mock private NamespaceRepository namespaceRepository; + @Mock private NamespaceMemberRepository namespaceMemberRepository; + @Mock private UserAccountRepository userAccountRepository; + @Mock private SkillRepository skillRepository; + @Mock private SkillVersionRepository skillVersionRepository; + @Mock private SkillFileRepository skillFileRepository; + @Mock private SkillPublishService skillPublishService; + + private BuiltinSkillProperties properties; + private BuiltinSkillInitializer initializer; + private Namespace globalNamespace; + + @BeforeEach + void setUp() { + properties = new BuiltinSkillProperties(); + initializer = new BuiltinSkillInitializer( + properties, + manifestLoader, + downloader, + extractor, + new SkillMetadataParser(), + namespaceRepository, + namespaceMemberRepository, + userAccountRepository, + skillRepository, + skillVersionRepository, + skillFileRepository, + skillPublishService + ); + globalNamespace = new Namespace(GLOBAL, "Global", "system"); + ReflectionTestUtils.setField(globalNamespace, "id", 1L); + } + + @Test + void skipsWhenDisabled() { + properties.setEnabled(false); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(manifestLoader, never()).load(); + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + } + + @Test + void skipsAllItemsWhenGlobalNamespaceDoesNotExist() { + when(namespaceRepository.findBySlug(GLOBAL)).thenReturn(Optional.empty()); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(manifestLoader, never()).load(); + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + } + + @Test + void skipsExistingSkillOwnedByAnotherUser() throws Exception { + Skill otherSkill = skill(100L, "agentguard", "someone-else"); + givenExtractedPackage(); + when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of(otherSkill)); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + } + + @Test + void skipsPublishedSameVersionWhenFingerprintMatches() throws Exception { + Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); + SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); + List entries = packageEntries("agentguard", "1.0.0", "same"); + givenExtractedPackage(entries); + when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of(builtinSkill)); + when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(published)); + when(skillFileRepository.findByVersionId(200L)).thenReturn(skillFilesFor(entries, 200L)); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + } + + @Test + void skipsPublishedSameVersionWhenFingerprintDiffers() throws Exception { + Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); + SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); + givenExtractedPackage(); + when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of(builtinSkill)); + when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(published)); + when(skillFileRepository.findByVersionId(200L)).thenReturn(List.of( + new SkillFile(200L, "SKILL.md", 7L, "text/markdown", sha256("changed"), "storage-key") + )); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + } + + @Test + void skipsExistingSameVersionWhenNotPublished() throws Exception { + Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); + SkillVersion uploaded = version(200L, 100L, "1.0.0", SkillVersionStatus.UPLOADED); + givenExtractedPackage(); + when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of(builtinSkill)); + when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(uploaded)); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + } + + @Test + void skipsWhenManifestSlugDoesNotMatchPackageMetadata() throws Exception { + givenExtractedPackage(packageEntries("other-skill", "1.0.0", "same")); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + } + + @Test + void skipsWhenManifestVersionDoesNotMatchPackageMetadata() throws Exception { + givenExtractedPackage(packageEntries("agentguard", "1.0.1", "same")); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + } + + @Test + void publishesNewVersionToGlobalAsPublicWithSystemPublisher() throws Exception { + List entries = packageEntries("agentguard", "1.0.0", "same"); + givenExtractedPackage(entries); + when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of()); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + ArgumentCaptor> entriesCaptor = ArgumentCaptor.captor(); + verify(skillPublishService).publishFromEntries( + eq(GLOBAL), + entriesCaptor.capture(), + eq(PUBLISHER), + eq(SkillVisibility.PUBLIC), + eq(Set.of("SUPER_ADMIN")), + eq(false) + ); + assertThat(entriesCaptor.getValue()).isEqualTo(entries); + } + + @Test + void createsSystemPublisherAndGlobalMembershipBeforePublishing() throws Exception { + List entries = packageEntries("agentguard", "1.0.0", "same"); + givenExtractedPackage(entries); + when(userAccountRepository.findById(PUBLISHER)).thenReturn(Optional.empty()); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER)).thenReturn(Optional.empty()); + when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of()); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UserAccount.class); + verify(userAccountRepository).save(userCaptor.capture()); + assertThat(userCaptor.getValue().getId()).isEqualTo(PUBLISHER); + + ArgumentCaptor memberCaptor = ArgumentCaptor.forClass(NamespaceMember.class); + verify(namespaceMemberRepository).save(memberCaptor.capture()); + assertThat(memberCaptor.getValue().getNamespaceId()).isEqualTo(1L); + assertThat(memberCaptor.getValue().getUserId()).isEqualTo(PUBLISHER); + assertThat(memberCaptor.getValue().getRole()).isEqualTo(NamespaceRole.OWNER); + } + + @Test + void treatsConcurrentDuplicatePublishedVersionAsCompleted() throws Exception { + Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); + SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); + List entries = packageEntries("agentguard", "1.0.0", "same"); + givenExtractedPackage(entries); + when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")) + .thenReturn(List.of()) + .thenReturn(List.of(builtinSkill)); + when(skillPublishService.publishFromEntries( + eq(GLOBAL), any(), eq(PUBLISHER), eq(SkillVisibility.PUBLIC), eq(Set.of("SUPER_ADMIN")), eq(false))) + .thenThrow(new DomainBadRequestException("error.skill.version.exists", "1.0.0")); + when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(published)); + when(skillFileRepository.findByVersionId(200L)).thenReturn(skillFilesFor(entries, 200L)); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(skillPublishService).publishFromEntries( + eq(GLOBAL), any(), eq(PUBLISHER), eq(SkillVisibility.PUBLIC), eq(Set.of("SUPER_ADMIN")), eq(false)); + } + + @Test + void doesNotTreatConcurrentDuplicateWithDifferentFingerprintAsCompleted() throws Exception { + Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); + SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); + givenExtractedPackage(packageEntries("agentguard", "1.0.0", "new-content")); + when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")) + .thenReturn(List.of()) + .thenReturn(List.of(builtinSkill)); + when(skillPublishService.publishFromEntries( + eq(GLOBAL), any(), eq(PUBLISHER), eq(SkillVisibility.PUBLIC), eq(Set.of("SUPER_ADMIN")), eq(false))) + .thenThrow(new DomainBadRequestException("error.skill.version.exists", "1.0.0")); + when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(published)); + when(skillFileRepository.findByVersionId(200L)).thenReturn(List.of( + new SkillFile(200L, "SKILL.md", 7L, "text/markdown", sha256("old-content"), "storage-key") + )); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(skillFileRepository).findByVersionId(200L); + verify(skillPublishService).publishFromEntries( + eq(GLOBAL), any(), eq(PUBLISHER), eq(SkillVisibility.PUBLIC), eq(Set.of("SUPER_ADMIN")), eq(false)); + } + + private void givenExtractedPackage() throws Exception { + givenExtractedPackage(packageEntries("agentguard", "1.0.0", "same")); + } + + private void givenExtractedPackage(List entries) throws Exception { + byte[] bytes = "zip".getBytes(StandardCharsets.UTF_8); + when(namespaceRepository.findBySlug(GLOBAL)).thenReturn(Optional.of(globalNamespace)); + when(manifestLoader.load()).thenReturn(List.of(ITEM)); + lenient().when(userAccountRepository.findById(PUBLISHER)).thenReturn(Optional.of(systemPublisher())); + lenient().when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER)) + .thenReturn(Optional.of(new NamespaceMember(1L, PUBLISHER, NamespaceRole.OWNER))); + when(downloader.download(URI.create(ITEM.url()))).thenReturn(Optional.of(bytes)); + when(extractor.extract(bytes)).thenReturn(new SkillPackageArchiveExtractor.ExtractionResult(entries, List.of())); + } + + private static UserAccount systemPublisher() { + return new UserAccount(PUBLISHER, "Built-in Skill Publisher", null, null); + } + + private static Skill skill(Long id, String slug, String ownerId) { + Skill skill = new Skill(1L, slug, ownerId, SkillVisibility.PUBLIC); + ReflectionTestUtils.setField(skill, "id", id); + return skill; + } + + private static SkillVersion version(Long id, Long skillId, String version, SkillVersionStatus status) { + SkillVersion skillVersion = new SkillVersion(skillId, version, PUBLISHER); + ReflectionTestUtils.setField(skillVersion, "id", id); + skillVersion.setStatus(status); + return skillVersion; + } + + private static List packageEntries(String name, String version, String readme) { + byte[] skillMd = (""" + --- + name: %s + description: Built-in guardrails + version: %s + --- + # %s + """).formatted(name, version, name).getBytes(StandardCharsets.UTF_8); + byte[] readmeBytes = readme.getBytes(StandardCharsets.UTF_8); + return List.of( + new PackageEntry("SKILL.md", skillMd, skillMd.length, "text/markdown"), + new PackageEntry("README.md", readmeBytes, readmeBytes.length, "text/markdown") + ); + } + + private static List skillFilesFor(List entries, Long versionId) { + return entries.stream() + .map(entry -> new SkillFile( + versionId, + entry.path(), + entry.size(), + entry.contentType(), + sha256(entry.content()), + "storage-key/" + entry.path() + )) + .toList(); + } + + private static String sha256(String content) { + return sha256(content.getBytes(StandardCharsets.UTF_8)); + } + + private static String sha256(byte[] content) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + return HexFormat.of().formatHex(digest.digest(content)); + } catch (Exception exception) { + throw new IllegalStateException(exception); + } + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoaderTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoaderTest.java new file mode 100644 index 00000000..39ea4b38 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoaderTest.java @@ -0,0 +1,157 @@ +package com.iflytek.skillhub.bootstrap; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; +import org.springframework.core.io.ByteArrayResource; +import org.springframework.core.io.ResourceLoader; + +import java.nio.charset.StandardCharsets; +import java.util.List; + +class BuiltinSkillManifestLoaderTest { + + @Test + void loadsManifestItemsInOrder() { + BuiltinSkillManifestLoader loader = loaderWith(""" + { + "skills": [ + {"slug": "agentguard", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/agentguard.zip"}, + {"slug": "agentguard", "version": "1.1.0", "url": "https://cdn.bjcdn.openstorage.cn/agentguard.zip"} + ] + } + """); + + List items = loader.load(); + + assertThat(items) + .extracting(BuiltinSkillManifestLoader.ManifestItem::version) + .containsExactly("1.0.0", "1.1.0"); + } + + @Test + void returnsEmptyListWhenManifestIsMissing() { + BuiltinSkillManifestLoader loader = new BuiltinSkillManifestLoader( + new ObjectMapper(), + new ResourceLoader() { + @Override + public org.springframework.core.io.Resource getResource(String location) { + return new MissingResource(); + } + + @Override + public ClassLoader getClassLoader() { + return getClass().getClassLoader(); + } + } + ); + + assertThat(loader.load()).isEmpty(); + } + + @Test + void returnsEmptyListWhenManifestIsMalformed() { + BuiltinSkillManifestLoader loader = loaderWith("{not-json"); + + assertThat(loader.load()).isEmpty(); + } + + @Test + void returnsEmptyListWhenManifestIsEmpty() { + BuiltinSkillManifestLoader loader = loaderWith(""); + + assertThat(loader.load()).isEmpty(); + } + + @Test + void skipsItemsWithMissingHumanFieldsAndDuplicateSlugVersion() { + BuiltinSkillManifestLoader loader = loaderWith(""" + { + "skills": [ + {"slug": "agentguard", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/first.zip"}, + {"slug": "agentguard", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/second.zip"}, + {"slug": "InvalidUppercase", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/invalid.zip"}, + {"slug": "", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/blank.zip"}, + {"slug": "missing-version", "url": "https://bjcdn.openstorage.cn/missing-version.zip"}, + {"slug": "missing-url", "version": "1.0.0"}, + {"slug": "valid-after-invalid", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/valid.zip"} + ] + } + """); + + List items = loader.load(); + + assertThat(items) + .extracting(BuiltinSkillManifestLoader.ManifestItem::url) + .containsExactly( + "https://bjcdn.openstorage.cn/first.zip", + "https://bjcdn.openstorage.cn/valid.zip" + ); + } + + @Test + void capsManifestEntriesAtOneHundredRawEntries() { + StringBuilder json = new StringBuilder("{\"skills\":["); + for (int i = 0; i < 101; i++) { + if (i > 0) { + json.append(','); + } + if (i == 0) { + json.append("{\"slug\":\"\",\"version\":\"1.0.0\",\"url\":\"https://bjcdn.openstorage.cn/blank.zip\"}"); + } else { + json.append("{\"slug\":\"skill-").append(i) + .append("\",\"version\":\"1.0.0\",\"url\":\"https://bjcdn.openstorage.cn/skill-") + .append(i) + .append(".zip\"}"); + } + } + json.append("]}"); + + BuiltinSkillManifestLoader loader = loaderWith(json.toString()); + + assertThat(loader.load()).hasSize(99); + } + + private BuiltinSkillManifestLoader loaderWith(String content) { + ResourceLoader resourceLoader = new ResourceLoader() { + @Override + public org.springframework.core.io.Resource getResource(String location) { + return new ByteArrayResource(content.getBytes(StandardCharsets.UTF_8)) { + @Override + public boolean exists() { + return true; + } + + @Override + public String getDescription() { + return "test manifest"; + } + }; + } + + @Override + public ClassLoader getClassLoader() { + return getClass().getClassLoader(); + } + }; + return new BuiltinSkillManifestLoader(new ObjectMapper(), resourceLoader); + } + + static class MissingResource extends ByteArrayResource { + + MissingResource() { + super(new byte[0]); + } + + @Override + public boolean exists() { + return false; + } + + @Override + public String getDescription() { + return "missing manifest"; + } + } +} 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 new file mode 100644 index 00000000..99f710b9 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPackageExtractorTest.java @@ -0,0 +1,84 @@ +package com.iflytek.skillhub.bootstrap; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.iflytek.skillhub.config.SkillPublishProperties; +import com.iflytek.skillhub.controller.support.SkillPackageArchiveExtractor; +import com.iflytek.skillhub.domain.skill.validation.SkillPackagePolicy; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +class BuiltinSkillPackageExtractorTest { + + private final BuiltinSkillPackageExtractor extractor = new BuiltinSkillPackageExtractor( + new SkillPackageArchiveExtractor(new SkillPublishProperties()) + ); + + @Test + void extractsZipBytesThroughArchiveExtractor() throws Exception { + byte[] zip = zip( + entry("SKILL.md", """ + --- + name: agentguard + version: 1.0.0 + --- + # AgentGuard + """), + entry("README.md", "# Readme") + ); + + SkillPackageArchiveExtractor.ExtractionResult result = extractor.extract(zip); + + assertThat(result.entries()) + .extracting(entry -> entry.path()) + .containsExactly("SKILL.md", "README.md"); + } + + @Test + void rejectsZipWithoutRootSkillMd() throws Exception { + byte[] zip = zip(entry("README.md", "# Readme")); + + assertThatThrownBy(() -> extractor.extract(zip)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(SkillPackagePolicy.SKILL_MD_PATH); + } + + @Test + void rejectsZipWithOnlyNestedSkillMd() throws Exception { + byte[] zip = zip(entry("agentguard/SKILL.md", """ + --- + name: agentguard + version: 1.0.0 + --- + # AgentGuard + """)); + + assertThatThrownBy(() -> extractor.extract(zip)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(SkillPackagePolicy.SKILL_MD_PATH); + } + + private static ZipSource entry(String path, String content) { + return new ZipSource(path, content.getBytes(StandardCharsets.UTF_8)); + } + + private static byte[] zip(ZipSource... sources) throws Exception { + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + try (ZipOutputStream zipOutputStream = new ZipOutputStream(outputStream)) { + for (ZipSource source : sources) { + zipOutputStream.putNextEntry(new ZipEntry(source.path())); + zipOutputStream.write(source.content()); + zipOutputStream.closeEntry(); + } + } + return outputStream.toByteArray(); + } + + record ZipSource(String path, byte[] content) { + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPropertiesBindingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPropertiesBindingTest.java new file mode 100644 index 00000000..174b8347 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillPropertiesBindingTest.java @@ -0,0 +1,47 @@ +package com.iflytek.skillhub.bootstrap; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.SystemEnvironmentPropertySource; + +import java.util.Map; + +class BuiltinSkillPropertiesBindingTest { + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withUserConfiguration(TestConfig.class); + + @Test + void enabledDefaultsToTrue() { + contextRunner.run((context) -> { + BuiltinSkillProperties properties = context.getBean(BuiltinSkillProperties.class); + + assertThat(properties.isEnabled()).isTrue(); + }); + } + + @Test + void bindsEnabledFromEnvironmentStyleProperty() { + contextRunner + .withInitializer((context) -> context.getEnvironment().getPropertySources().addFirst( + new SystemEnvironmentPropertySource( + "test-env", + Map.of("SKILLHUB_BUILTIN_SKILLS_ENABLED", "false") + ) + )) + .run((context) -> { + BuiltinSkillProperties properties = context.getBean(BuiltinSkillProperties.class); + + assertThat(properties.isEnabled()).isFalse(); + }); + } + + @Configuration + @EnableConfigurationProperties(BuiltinSkillProperties.class) + static class TestConfig { + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloaderTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloaderTest.java new file mode 100644 index 00000000..9dbf2cdf --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloaderTest.java @@ -0,0 +1,219 @@ +package com.iflytek.skillhub.bootstrap; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.iflytek.skillhub.config.SkillPublishProperties; +import org.junit.jupiter.api.Test; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSession; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.net.Authenticator; +import java.net.CookieHandler; +import java.net.ProxySelector; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpHeaders; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.time.Duration; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; + +class BuiltinSkillRemotePackageDownloaderTest { + + @Test + void acceptsAllowedHttpsCdnHostsOnly() { + assertThat(BuiltinSkillRemotePackageDownloader.isAllowedUrl(URI.create("https://bjcdn.openstorage.cn/a.zip"))) + .isTrue(); + assertThat(BuiltinSkillRemotePackageDownloader.isAllowedUrl(URI.create("https://assets.bjcdn.openstorage.cn/a.zip"))) + .isTrue(); + assertThat(BuiltinSkillRemotePackageDownloader.isAllowedUrl(URI.create("http://bjcdn.openstorage.cn/a.zip"))) + .isFalse(); + assertThat(BuiltinSkillRemotePackageDownloader.isAllowedUrl(URI.create("https://evil.com/a.zip"))) + .isFalse(); + assertThat(BuiltinSkillRemotePackageDownloader.isAllowedUrl(URI.create("https://user:pass@bjcdn.openstorage.cn/a.zip"))) + .isFalse(); + assertThat(BuiltinSkillRemotePackageDownloader.isAllowedUrl(URI.create("https://bjcdn.openstorage.cn:8443/a.zip"))) + .isFalse(); + assertThat(BuiltinSkillRemotePackageDownloader.isAllowedUrl(URI.create("https://127.0.0.1/a.zip"))) + .isFalse(); + assertThat(BuiltinSkillRemotePackageDownloader.isAllowedUrl(URI.create("https://localhost/a.zip"))) + .isFalse(); + } + + @Test + void defaultHttpClientDoesNotFollowRedirects() { + BuiltinSkillRemotePackageDownloader downloader = new BuiltinSkillRemotePackageDownloader(new SkillPublishProperties()); + + assertThat(downloader.httpClient().followRedirects()).isEqualTo(HttpClient.Redirect.NEVER); + assertThat(downloader.httpClient().connectTimeout()).contains(Duration.ofSeconds(5)); + } + + @Test + void downloadsAllowedUrlWithThirtySecondRequestTimeout() { + FakeHttpClient client = new FakeHttpClient(200, new byte[] {1, 2, 3}); + BuiltinSkillRemotePackageDownloader downloader = new BuiltinSkillRemotePackageDownloader( + new SkillPublishProperties(), + client + ); + + Optional bytes = downloader.download(URI.create("https://bjcdn.openstorage.cn/package.zip")); + + assertThat(bytes).contains(new byte[] {1, 2, 3}); + assertThat(client.lastRequest.timeout()).contains(Duration.ofSeconds(30)); + } + + @Test + void rejectsRedirectResponsesWithoutReadingLocation() { + FakeHttpClient client = new FakeHttpClient(302, new byte[] {1}); + BuiltinSkillRemotePackageDownloader downloader = new BuiltinSkillRemotePackageDownloader( + new SkillPublishProperties(), + client + ); + + Optional bytes = downloader.download(URI.create("https://bjcdn.openstorage.cn/package.zip")); + + assertThat(bytes).isEmpty(); + assertThat(client.sendCalls).isEqualTo(1); + } + + @Test + void rejectedUrlDoesNotSendHttpRequest() { + FakeHttpClient client = new FakeHttpClient(200, new byte[] {1}); + BuiltinSkillRemotePackageDownloader downloader = new BuiltinSkillRemotePackageDownloader( + new SkillPublishProperties(), + client + ); + + Optional bytes = downloader.download(URI.create("https://example.com/package.zip")); + + assertThat(bytes).isEmpty(); + assertThat(client.sendCalls).isZero(); + } + + @Test + void stopsReadingWhenResponseExceedsMaxPackageSize() { + SkillPublishProperties properties = new SkillPublishProperties(); + properties.setMaxPackageSize(2); + FakeHttpClient client = new FakeHttpClient(200, new byte[] {1, 2, 3}); + BuiltinSkillRemotePackageDownloader downloader = new BuiltinSkillRemotePackageDownloader(properties, client); + + assertThat(downloader.download(URI.create("https://bjcdn.openstorage.cn/package.zip"))).isEmpty(); + } + + static class FakeHttpClient extends HttpClient { + + private final int statusCode; + private final byte[] body; + private HttpRequest lastRequest; + private int sendCalls; + + FakeHttpClient(int statusCode, byte[] body) { + this.statusCode = statusCode; + this.body = body; + } + + @Override + public Optional cookieHandler() { + return Optional.empty(); + } + + @Override + public Optional connectTimeout() { + return Optional.of(Duration.ofSeconds(5)); + } + + @Override + public Redirect followRedirects() { + return Redirect.NEVER; + } + + @Override + public Optional proxy() { + return Optional.empty(); + } + + @Override + public SSLContext sslContext() { + return null; + } + + @Override + public SSLParameters sslParameters() { + return null; + } + + @Override + public Optional authenticator() { + return Optional.empty(); + } + + @Override + public Version version() { + return Version.HTTP_1_1; + } + + @Override + public Optional executor() { + return Optional.empty(); + } + + @Override + public HttpResponse send(HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) + throws IOException { + lastRequest = request; + sendCalls++; + @SuppressWarnings("unchecked") + T responseBody = (T) new ByteArrayInputStream(body); + return new FakeResponse<>(request, statusCode, responseBody); + } + + @Override + public CompletableFuture> sendAsync( + HttpRequest request, + HttpResponse.BodyHandler responseBodyHandler + ) { + throw new UnsupportedOperationException(); + } + + @Override + public CompletableFuture> sendAsync( + HttpRequest request, + HttpResponse.BodyHandler responseBodyHandler, + HttpResponse.PushPromiseHandler pushPromiseHandler + ) { + throw new UnsupportedOperationException(); + } + } + + record FakeResponse(HttpRequest request, int statusCode, T body) implements HttpResponse { + @Override + public Optional> previousResponse() { + return Optional.empty(); + } + + @Override + public HttpHeaders headers() { + return HttpHeaders.of(java.util.Map.of(), (name, value) -> true); + } + + @Override + public URI uri() { + return request.uri(); + } + + @Override + public HttpClient.Version version() { + return HttpClient.Version.HTTP_1_1; + } + + @Override + public Optional sslSession() { + return Optional.empty(); + } + } +} From 0f752e23058442444e5df4f0b18876928649d948 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Fri, 5 Jun 2026 17:16:25 +0800 Subject: [PATCH 02/11] docs(bootstrap): add built-in skill cloud setup guide Signed-off-by: dongmucat <1127093059@qq.com> --- docs/20-cloud-url-builtin-skills-setup.md | 264 ++++++++++++++++++++++ 1 file changed, 264 insertions(+) create mode 100644 docs/20-cloud-url-builtin-skills-setup.md diff --git a/docs/20-cloud-url-builtin-skills-setup.md b/docs/20-cloud-url-builtin-skills-setup.md new file mode 100644 index 00000000..1ddc054a --- /dev/null +++ b/docs/20-cloud-url-builtin-skills-setup.md @@ -0,0 +1,264 @@ +# 云存储链接内置 Skills 配置指南 + +本文说明如何通过仓库内 manifest 配置 SkillHub 内置 Skills，以及应用启动时这些 Skills 如何从云存储同步到 `@global` 空间。 + +适用场景： + +- 希望 SkillHub 新部署实例默认带有一批官方内置 Skills。 +- 不希望把完整 Skill 包目录长期放在代码仓库和镜像中。 +- 内置 Skill 包已经上传到官方可控的云存储域名。 + +## 1. 方案概览 + +内置 Skills 不再以本地目录包的形式直接随仓库维护。当前方案只在仓库中维护一个 manifest 文件，应用启动时根据 manifest 中的云存储 URL 下载 zip 包，并通过 SkillHub 现有发布链路发布到 `@global`。 + +流程： + +```text +维护 manifest -> 构建/部署 SkillHub 镜像 -> 应用启动 -> 读取 manifest -> 下载云存储 zip 包 -> 校验包内容 -> 发布到 @global -> 对所有用户公开可见 +``` + +核心文件： + +```text +server/skillhub-app/src/main/resources/builtin-skills/manifest.json +``` + +首版 manifest 只需要维护三个字段： + +- `slug`：Skill 在 `@global` 下的 slug。 +- `version`：期望同步的 Skill 版本。 +- `url`：Skill zip 包的云存储 HTTPS 链接。 + +## 2. Manifest 配置 + +manifest 文件格式如下： + +```json +{ + "skills": [ + { + "slug": "skillhub-hello", + "version": "1.0.0", + "url": "https://bjcdn.openstorage.cn//skillhub-hello-1.0.0.zip" + } + ] +} +``` + +可以配置多个 Skills，也可以为同一个 `slug` 配置多个版本： + +```json +{ + "skills": [ + { + "slug": "skillhub-hello", + "version": "1.0.0", + "url": "https://bjcdn.openstorage.cn//skillhub-hello-1.0.0.zip" + }, + { + "slug": "skillhub-hello", + "version": "1.1.0", + "url": "https://bjcdn.openstorage.cn//skillhub-hello-1.1.0.zip" + }, + { + "slug": "skillhub-guide", + "version": "1.0.0", + "url": "https://bjcdn.openstorage.cn//skillhub-guide-1.0.0.zip" + } + ] +} +``` + +配置要求： + +- `skills` 必须是数组。 +- 每一项必须同时填写 `slug`、`version`、`url`。 +- `slug` 必须符合 SkillHub slug 规则。 +- 同一个 `slug + version` 重复出现时，只处理第一条，后续重复项会被跳过。 +- manifest 最多处理前 100 条 entries。 +- 同一个 `slug` 的多个版本建议按从旧到新的顺序排列；运行时按 manifest 文件顺序处理，不做自动版本排序。 + +## 3. Skill 包要求 + +manifest 中的 `url` 必须指向 zip 包。zip 包需要满足 SkillHub Skill 包协议： + +- zip 根目录必须包含 `SKILL.md`。 +- `SKILL.md` frontmatter 中必须包含合法的 `name`、`description`、`version` 等元数据。 +- `SKILL.md` 中的 `name` 经过 slug 归一化后，必须等于 manifest 中的 `slug`。 +- `SKILL.md` 中的 `version` 必须等于 manifest 中的 `version`。 +- 包内容仍会经过 SkillHub 现有发布校验，包括文件数量、文件大小、扩展名、文件类型等规则。 + +示例： + +```text +skillhub-hello-1.0.0.zip +├── SKILL.md +├── README.md +└── scripts/ + └── check.js +``` + +不推荐的结构： + +```text +skillhub-hello-1.0.0.zip +└── skillhub-hello/ + └── SKILL.md +``` + +原因是内置 Skill 同步要求根目录存在 `SKILL.md`，不会把嵌套目录中的 `SKILL.md` 当作入口。 + +## 4. URL 安全限制 + +内置 Skill 同步由后端在启动时主动下载远程文件，因此 URL 有严格限制。 + +首版只允许： + +- `https://` 协议。 +- host 为 `bjcdn.openstorage.cn`。 +- host 为 `bjcdn.openstorage.cn` 的子域名，例如 `assets.bjcdn.openstorage.cn`。 +- 默认 HTTPS 端口，或显式 `:443`。 + +以下 URL 会被跳过： + +- `http://...` +- 非 `bjcdn.openstorage.cn` 及其子域名。 +- 带 userinfo 的 URL，例如 `https://user:pass@bjcdn.openstorage.cn/file.zip`。 +- 非 443 端口，例如 `https://bjcdn.openstorage.cn:8443/file.zip`。 +- `localhost`、IP 地址、IPv6 literal 等 host。 +- 需要 HTTP redirect 才能拿到文件的链接。 + +如果某一项 URL 不符合规则，SkillHub 会记录日志并跳过该项，不会阻塞应用启动。 + +## 5. 启动同步流程 + +应用启动时同步器只执行一次。 + +详细流程： + +1. 检查 `skillhub.builtin-skills.enabled` 是否开启。 +2. 读取 `classpath:builtin-skills/manifest.json`。 +3. 查询 `@global` 命名空间是否存在；如果不存在，跳过同步。 +4. 确保系统发布者 `builtin-skill-publisher` 存在，并且是 `@global` 的 `OWNER`。 +5. 按 manifest 顺序处理每一个 item。 +6. 下载对应 zip 包。 +7. 解包并校验根目录 `SKILL.md`。 +8. 校验 manifest 中的 `slug`、`version` 与包内元数据一致。 +9. 检查是否已存在同名 Skill 或同版本。 +10. 需要发布时调用现有 `SkillPublishService.publishFromEntries(...)`。 +11. 发布完成后，该 Skill 位于 `@global/{slug}`，可见性为 `PUBLIC`。 + +同步逻辑不会直接写数据库 seed 数据。它复用现有发布服务，因此会保留现有的包校验、对象存储写入、版本记录、latest version 更新、事件和搜索索引同步。 + +## 6. 幂等与冲突处理 + +内置 Skill 同步支持重复启动和多次部署。 + +幂等键： + +```text +@global/{slug} + version +``` + +行为说明： + +| 场景 | 行为 | +|---|---| +| `@global/{slug}` 不存在 | 发布 manifest 中的 Skill | +| `@global/{slug}` 已存在，owner 是 `builtin-skill-publisher`，但目标版本不存在 | 发布新版本 | +| 同版本已存在且已发布，内容一致 | 跳过 | +| 同版本已存在且已发布，但内容不一致 | 跳过并记录 warning | +| 同版本已存在但不是 `PUBLISHED` | 跳过并记录日志 | +| `@global/{slug}` 已存在，但 owner 不是 `builtin-skill-publisher` | 跳过并记录 warning | + +这意味着内置同步不会接管用户或管理员已经创建的同 slug Skill。 + +如果多实例同时启动，可能出现多个实例同时尝试发布同一个内置版本。同步器会在发布失败后重新查询目标版本；如果发现同版本已经以相同内容发布成功，则视为并发场景下的正常跳过。 + +## 7. 开关配置 + +内置 Skill 同步默认开启。 + +Spring 配置项： + +```yaml +skillhub: + builtin-skills: + enabled: true +``` + +环境变量： + +```dotenv +SKILLHUB_BUILTIN_SKILLS_ENABLED=true +``` + +如需禁用启动同步： + +```dotenv +SKILLHUB_BUILTIN_SKILLS_ENABLED=false +``` + +禁用后，应用启动时不会读取 manifest，也不会下载或发布任何内置 Skill。 + +## 8. 维护流程 + +新增一个内置 Skill 的推荐步骤： + +1. 准备 Skill 包，并确认 zip 根目录包含 `SKILL.md`。 +2. 检查 `SKILL.md` 中的 `name` 和 `version`。 +3. 上传 zip 到 `bjcdn.openstorage.cn` 或其子域名下的官方云存储路径。 +4. 在 `server/skillhub-app/src/main/resources/builtin-skills/manifest.json` 中新增一项。 +5. 确保 manifest 中的 `slug` 等于 `SKILL.md name` 归一化后的 slug。 +6. 确保 manifest 中的 `version` 等于 `SKILL.md version`。 +7. 本地或测试环境启动 SkillHub，查看后端日志确认同步结果。 +8. 在 Web UI 或 API 中确认 `@global/{slug}` 已公开可见。 + +更新一个已有内置 Skill 的推荐步骤： + +1. 不要覆盖已经发布过的旧版本 zip 内容。 +2. 在 `SKILL.md` 中提升 `version`。 +3. 重新打包并上传新的 zip 文件。 +4. 在 manifest 中新增一条同 `slug`、新 `version` 的记录。 +5. 保留旧版本记录，除非产品明确不再需要该旧版本在新实例中预置。 + +不推荐： + +- 修改旧版本 zip 内容但保持同一个 `version`。 +- 把 URL 指向会发生内容变化的临时对象。 +- 使用需要登录、签名跳转或重定向的下载链接。 + +## 9. 日志与排查 + +启动时可以通过后端日志观察同步结果。 + +常见日志含义： + +| 日志含义 | 处理建议 | +|---|---| +| manifest not found | 确认 `builtin-skills/manifest.json` 是否被打进 classpath | +| slug, version, and url are required | 检查 manifest item 是否缺字段或字段不是字符串 | +| slug is invalid | 检查 slug 是否符合 SkillHub slug 规则 | +| URL is not allowed | 检查 URL 是否为 HTTPS、host 是否为 `bjcdn.openstorage.cn` 或其子域名 | +| 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 owned by another user | 说明 `@global/{slug}` 已被非内置发布者占用，内置同步不会覆盖 | +| published fingerprint differs | 同一内置版本已存在但内容不同，需要人工确认是否错误覆盖了远程包 | + +如果某个 manifest item 失败，后续 item 仍会继续处理，应用启动也会继续。 + +## 10. 验收检查 + +配置或新增内置 Skill 后，建议至少完成以下检查： + +- manifest JSON 格式合法。 +- 每个 item 都包含 `slug`、`version`、`url`。 +- URL 使用 `https://bjcdn.openstorage.cn/...` 或可信子域名。 +- zip 根目录包含 `SKILL.md`。 +- `SKILL.md name` 归一化后的 slug 与 manifest `slug` 一致。 +- `SKILL.md version` 与 manifest `version` 一致。 +- 启动日志没有该 item 的 warning 或 error。 +- Web UI 中可以看到 `@global/{slug}`。 +- Skill 可被匿名或登录用户按公开 Skill 规则发现。 From 4c4a888b01e0547d3675c4b157c5bac7dcc265c1 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Mon, 8 Jun 2026 14:01:19 +0800 Subject: [PATCH 03/11] fix(bootstrap): harden built-in skill startup sync Signed-off-by: dongmucat <1127093059@qq.com> --- docs/20-cloud-url-builtin-skills-setup.md | 29 ++--- .../bootstrap/BuiltinSkillInitializer.java | 70 ++++++++++-- .../V43__user_account_system_account.sql | 19 +++ .../BuiltinSkillInitializerTest.java | 108 ++++++++++-------- .../skillhub/domain/user/UserAccount.java | 10 ++ 5 files changed, 169 insertions(+), 67 deletions(-) create mode 100644 server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql diff --git a/docs/20-cloud-url-builtin-skills-setup.md b/docs/20-cloud-url-builtin-skills-setup.md index 1ddc054a..a2d537fa 100644 --- a/docs/20-cloud-url-builtin-skills-setup.md +++ b/docs/20-cloud-url-builtin-skills-setup.md @@ -140,14 +140,17 @@ skillhub-hello-1.0.0.zip 1. 检查 `skillhub.builtin-skills.enabled` 是否开启。 2. 读取 `classpath:builtin-skills/manifest.json`。 3. 查询 `@global` 命名空间是否存在；如果不存在，跳过同步。 -4. 确保系统发布者 `builtin-skill-publisher` 存在，并且是 `@global` 的 `OWNER`。 -5. 按 manifest 顺序处理每一个 item。 -6. 下载对应 zip 包。 -7. 解包并校验根目录 `SKILL.md`。 -8. 校验 manifest 中的 `slug`、`version` 与包内元数据一致。 -9. 检查是否已存在同名 Skill 或同版本。 -10. 需要发布时调用现有 `SkillPublishService.publishFromEntries(...)`。 -11. 发布完成后，该 Skill 位于 `@global/{slug}`，可见性为 `PUBLIC`。 +4. 确保系统发布者 `builtin-skill-publisher` 存在，并且该账号带有系统账号标记。 +5. 如果该用户 ID 已被非系统账号占用，直接跳过本次内置 Skill 同步，不授予 `@global` 权限。 +6. 确保系统发布者是 `@global` 的 `OWNER`。 +7. 按 manifest 顺序处理每一个 item。 +8. 下载前先检查 `@global/{slug}` 和目标版本是否已经存在；如果已经确定应跳过，则不发起远程下载。 +9. 只有需要发布新 Skill 或新版本时，才下载对应 zip 包。 +10. 解包并校验根目录 `SKILL.md`。 +11. 校验 manifest 中的 `slug`、`version` 与包内元数据一致。 +12. 发布前再次检查是否已存在同名 Skill 或同版本，处理并发启动场景。 +13. 需要发布时调用现有 `SkillPublishService.publishFromEntries(...)`。 +14. 发布完成后，该 Skill 位于 `@global/{slug}`，可见性为 `PUBLIC`。 同步逻辑不会直接写数据库 seed 数据。它复用现有发布服务，因此会保留现有的包校验、对象存储写入、版本记录、latest version 更新、事件和搜索索引同步。 @@ -167,10 +170,9 @@ skillhub-hello-1.0.0.zip |---|---| | `@global/{slug}` 不存在 | 发布 manifest 中的 Skill | | `@global/{slug}` 已存在，owner 是 `builtin-skill-publisher`，但目标版本不存在 | 发布新版本 | -| 同版本已存在且已发布，内容一致 | 跳过 | -| 同版本已存在且已发布，但内容不一致 | 跳过并记录 warning | -| 同版本已存在但不是 `PUBLISHED` | 跳过并记录日志 | -| `@global/{slug}` 已存在，但 owner 不是 `builtin-skill-publisher` | 跳过并记录 warning | +| 同版本已存在且已发布 | 下载前跳过 | +| 同版本已存在但不是 `PUBLISHED` | 下载前跳过并记录日志 | +| `@global/{slug}` 已存在，但 owner 不是 `builtin-skill-publisher` | 下载前跳过并记录 warning | 这意味着内置同步不会接管用户或管理员已经创建的同 slug Skill。 @@ -238,6 +240,7 @@ SKILLHUB_BUILTIN_SKILLS_ENABLED=false | 日志含义 | 处理建议 | |---|---| | manifest not found | 确认 `builtin-skills/manifest.json` 是否被打进 classpath | +| publisher account id already exists but is not a system account | `builtin-skill-publisher` 已被普通账号占用；需要人工处理账号冲突后再启用内置同步 | | slug, version, and url are required | 检查 manifest item 是否缺字段或字段不是字符串 | | slug is invalid | 检查 slug 是否符合 SkillHub slug 规则 | | URL is not allowed | 检查 URL 是否为 HTTPS、host 是否为 `bjcdn.openstorage.cn` 或其子域名 | @@ -245,7 +248,7 @@ SKILLHUB_BUILTIN_SKILLS_ENABLED=false | package must contain SKILL.md | 检查 zip 根目录是否存在 `SKILL.md` | | manifest version does not match package version | 检查 manifest `version` 和 `SKILL.md version` 是否一致 | | slug is already owned by another user | 说明 `@global/{slug}` 已被非内置发布者占用，内置同步不会覆盖 | -| published fingerprint differs | 同一内置版本已存在但内容不同，需要人工确认是否错误覆盖了远程包 | +| 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 7ff69a68..63ae3981 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 @@ -111,7 +111,9 @@ public class BuiltinSkillInitializer implements ApplicationRunner { } try { - ensureSystemPublisher(namespace.get()); + if (!ensureSystemPublisher(namespace.get())) { + return; + } } catch (RuntimeException exception) { log.error("Failed to initialize built-in skill system publisher, skipping synchronization: {}", exception.getMessage(), exception); @@ -133,14 +135,25 @@ public class BuiltinSkillInitializer implements ApplicationRunner { } } - private void ensureSystemPublisher(Namespace namespace) { - userAccountRepository.findById(SYSTEM_PUBLISHER_ID) - .orElseGet(() -> userAccountRepository.save(new UserAccount( - SYSTEM_PUBLISHER_ID, - "Built-in Skill Publisher", - null, - null - ))); + private boolean ensureSystemPublisher(Namespace namespace) { + Optional existingPublisher = userAccountRepository.findById(SYSTEM_PUBLISHER_ID); + UserAccount publisher; + if (existingPublisher.isPresent()) { + publisher = existingPublisher.get(); + } else { + publisher = UserAccount.systemAccount( + SYSTEM_PUBLISHER_ID, + "Built-in Skill Publisher", + null, + null + ); + userAccountRepository.save(publisher); + } + if (!publisher.isSystemAccount()) { + log.error("Built-in skill publisher account id '{}' already exists but is not a system account; " + + "skipping built-in skill synchronization", SYSTEM_PUBLISHER_ID); + return false; + } if (namespaceMemberRepository.findByNamespaceIdAndUserId(namespace.getId(), SYSTEM_PUBLISHER_ID).isEmpty()) { namespaceMemberRepository.save(new NamespaceMember( @@ -149,9 +162,14 @@ public class BuiltinSkillInitializer implements ApplicationRunner { NamespaceRole.OWNER )); } + return true; } private void syncItem(Namespace namespace, ManifestItem item) throws Exception { + if (shouldSkipBeforeDownload(namespace.getId(), item)) { + return; + } + Optional packageBytes = downloader.download(URI.create(item.url())); if (packageBytes.isEmpty()) { log.warn("Skipping built-in skill slug={} version={} because package download failed", @@ -217,6 +235,40 @@ public class BuiltinSkillInitializer implements ApplicationRunner { return metadataParser.parse(new String(skillMd.content(), StandardCharsets.UTF_8)); } + private boolean shouldSkipBeforeDownload(Long namespaceId, ManifestItem item) { + List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); + boolean hasNonBuiltinOwner = existingSkills.stream() + .anyMatch(skill -> !SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())); + if (hasNonBuiltinOwner) { + log.warn("Skipping built-in skill slug={} before download because the slug is already owned by another user", + item.slug()); + return true; + } + + Optional builtinSkill = existingSkills.stream() + .filter(skill -> SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())) + .findFirst(); + if (builtinSkill.isEmpty()) { + return false; + } + + Optional existingVersion = skillVersionRepository + .findBySkillIdAndVersion(builtinSkill.get().getId(), item.version()); + if (existingVersion.isEmpty()) { + return false; + } + + SkillVersion version = existingVersion.get(); + if (version.getStatus() == SkillVersionStatus.PUBLISHED) { + log.info("Skipping built-in skill slug={} version={} before download because it is already published", + item.slug(), item.version()); + } else { + log.info("Skipping built-in skill slug={} version={} before download because existing version status is {}", + item.slug(), item.version(), version.getStatus()); + } + return true; + } + private boolean shouldSkipExisting(Long namespaceId, ManifestItem item, List entries) { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); boolean hasNonBuiltinOwner = existingSkills.stream() diff --git a/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql b/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql new file mode 100644 index 00000000..c525bfd2 --- /dev/null +++ b/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql @@ -0,0 +1,19 @@ +ALTER TABLE user_account + ADD COLUMN system_account BOOLEAN NOT NULL DEFAULT FALSE; + +UPDATE user_account +SET system_account = TRUE +WHERE id = 'builtin-skill-publisher' + AND display_name = 'Built-in Skill Publisher' + AND email IS NULL + AND avatar_url IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM local_credential + WHERE local_credential.user_id = user_account.id + ) + AND NOT EXISTS ( + SELECT 1 + FROM identity_binding + WHERE identity_binding.user_id = user_account.id + ); 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 4e3322bb..7afa9cd9 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 @@ -52,9 +52,9 @@ class BuiltinSkillInitializerTest { private static final String GLOBAL = "global"; private static final String PUBLISHER = "builtin-skill-publisher"; private static final ManifestItem ITEM = new ManifestItem( - "agentguard", + "skillhub-hello", "1.0.0", - "https://bjcdn.openstorage.cn/skills/agentguard.zip" + "https://bjcdn.openstorage.cn/skills/skillhub-hello.zip" ); @Mock private BuiltinSkillManifestLoader manifestLoader; @@ -114,57 +114,64 @@ class BuiltinSkillInitializerTest { } @Test - void skipsExistingSkillOwnedByAnotherUser() throws Exception { - Skill otherSkill = skill(100L, "agentguard", "someone-else"); - givenExtractedPackage(); - when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of(otherSkill)); + void skipsSynchronizationWhenPublisherIdIsOccupiedByNonSystemAccount() { + when(namespaceRepository.findBySlug(GLOBAL)).thenReturn(Optional.of(globalNamespace)); + when(manifestLoader.load()).thenReturn(List.of(ITEM)); + when(userAccountRepository.findById(PUBLISHER)) + .thenReturn(Optional.of(new UserAccount(PUBLISHER, "Human User", "human@example.com", null))); initializer.run(new DefaultApplicationArguments(new String[0])); + verify(namespaceMemberRepository, never()).save(any()); + verify(downloader, never()).download(any()); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } @Test - void skipsPublishedSameVersionWhenFingerprintMatches() throws Exception { - Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); + void skipsPublishedSameVersionBeforeDownloadingPackage() { + Skill builtinSkill = skill(100L, "skillhub-hello", PUBLISHER); SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); - List entries = packageEntries("agentguard", "1.0.0", "same"); - givenExtractedPackage(entries); - when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of(builtinSkill)); + when(namespaceRepository.findBySlug(GLOBAL)).thenReturn(Optional.of(globalNamespace)); + when(manifestLoader.load()).thenReturn(List.of(ITEM)); + when(userAccountRepository.findById(PUBLISHER)).thenReturn(Optional.of(systemPublisher())); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER)) + .thenReturn(Optional.of(new NamespaceMember(1L, PUBLISHER, NamespaceRole.OWNER))); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(builtinSkill)); when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(published)); - when(skillFileRepository.findByVersionId(200L)).thenReturn(skillFilesFor(entries, 200L)); initializer.run(new DefaultApplicationArguments(new String[0])); + verify(downloader, never()).download(any()); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } @Test - void skipsPublishedSameVersionWhenFingerprintDiffers() throws Exception { - Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); - SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); - givenExtractedPackage(); - when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of(builtinSkill)); - when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(published)); - when(skillFileRepository.findByVersionId(200L)).thenReturn(List.of( - new SkillFile(200L, "SKILL.md", 7L, "text/markdown", sha256("changed"), "storage-key") - )); - - initializer.run(new DefaultApplicationArguments(new String[0])); - - verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); - } - - @Test - void skipsExistingSameVersionWhenNotPublished() throws Exception { - Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); + void skipsExistingSameVersionWhenNotPublishedBeforeDownloadingPackage() { + Skill builtinSkill = skill(100L, "skillhub-hello", PUBLISHER); SkillVersion uploaded = version(200L, 100L, "1.0.0", SkillVersionStatus.UPLOADED); - givenExtractedPackage(); - when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of(builtinSkill)); + when(namespaceRepository.findBySlug(GLOBAL)).thenReturn(Optional.of(globalNamespace)); + when(manifestLoader.load()).thenReturn(List.of(ITEM)); + when(userAccountRepository.findById(PUBLISHER)).thenReturn(Optional.of(systemPublisher())); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER)) + .thenReturn(Optional.of(new NamespaceMember(1L, PUBLISHER, NamespaceRole.OWNER))); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(builtinSkill)); when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(uploaded)); initializer.run(new DefaultApplicationArguments(new String[0])); + verify(downloader, never()).download(any()); + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + } + + @Test + void skipsExistingSkillOwnedByAnotherUserBeforeDownloadingPackage() throws Exception { + Skill otherSkill = skill(100L, "skillhub-hello", "someone-else"); + givenManifestAndSystemPublisher(); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(otherSkill)); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(downloader, never()).download(any()); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } @@ -179,7 +186,7 @@ class BuiltinSkillInitializerTest { @Test void skipsWhenManifestVersionDoesNotMatchPackageMetadata() throws Exception { - givenExtractedPackage(packageEntries("agentguard", "1.0.1", "same")); + givenExtractedPackage(packageEntries("skillhub-hello", "1.0.1", "same")); initializer.run(new DefaultApplicationArguments(new String[0])); @@ -188,9 +195,9 @@ class BuiltinSkillInitializerTest { @Test void publishesNewVersionToGlobalAsPublicWithSystemPublisher() throws Exception { - List entries = packageEntries("agentguard", "1.0.0", "same"); + List entries = packageEntries("skillhub-hello", "1.0.0", "same"); givenExtractedPackage(entries); - when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of()); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of()); initializer.run(new DefaultApplicationArguments(new String[0])); @@ -208,17 +215,18 @@ class BuiltinSkillInitializerTest { @Test void createsSystemPublisherAndGlobalMembershipBeforePublishing() throws Exception { - List entries = packageEntries("agentguard", "1.0.0", "same"); + List entries = packageEntries("skillhub-hello", "1.0.0", "same"); givenExtractedPackage(entries); when(userAccountRepository.findById(PUBLISHER)).thenReturn(Optional.empty()); when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER)).thenReturn(Optional.empty()); - when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")).thenReturn(List.of()); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of()); initializer.run(new DefaultApplicationArguments(new String[0])); ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UserAccount.class); verify(userAccountRepository).save(userCaptor.capture()); assertThat(userCaptor.getValue().getId()).isEqualTo(PUBLISHER); + assertThat(userCaptor.getValue().isSystemAccount()).isTrue(); ArgumentCaptor memberCaptor = ArgumentCaptor.forClass(NamespaceMember.class); verify(namespaceMemberRepository).save(memberCaptor.capture()); @@ -229,11 +237,12 @@ class BuiltinSkillInitializerTest { @Test void treatsConcurrentDuplicatePublishedVersionAsCompleted() throws Exception { - Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); + Skill builtinSkill = skill(100L, "skillhub-hello", PUBLISHER); SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); - List entries = packageEntries("agentguard", "1.0.0", "same"); + List entries = packageEntries("skillhub-hello", "1.0.0", "same"); givenExtractedPackage(entries); - when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")) + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")) + .thenReturn(List.of()) .thenReturn(List.of()) .thenReturn(List.of(builtinSkill)); when(skillPublishService.publishFromEntries( @@ -250,10 +259,11 @@ class BuiltinSkillInitializerTest { @Test void doesNotTreatConcurrentDuplicateWithDifferentFingerprintAsCompleted() throws Exception { - Skill builtinSkill = skill(100L, "agentguard", PUBLISHER); + Skill builtinSkill = skill(100L, "skillhub-hello", PUBLISHER); SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); - givenExtractedPackage(packageEntries("agentguard", "1.0.0", "new-content")); - when(skillRepository.findByNamespaceIdAndSlug(1L, "agentguard")) + givenExtractedPackage(packageEntries("skillhub-hello", "1.0.0", "new-content")); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")) + .thenReturn(List.of()) .thenReturn(List.of()) .thenReturn(List.of(builtinSkill)); when(skillPublishService.publishFromEntries( @@ -272,7 +282,7 @@ class BuiltinSkillInitializerTest { } private void givenExtractedPackage() throws Exception { - givenExtractedPackage(packageEntries("agentguard", "1.0.0", "same")); + givenExtractedPackage(packageEntries("skillhub-hello", "1.0.0", "same")); } private void givenExtractedPackage(List entries) throws Exception { @@ -286,8 +296,16 @@ class BuiltinSkillInitializerTest { when(extractor.extract(bytes)).thenReturn(new SkillPackageArchiveExtractor.ExtractionResult(entries, List.of())); } + private void givenManifestAndSystemPublisher() { + when(namespaceRepository.findBySlug(GLOBAL)).thenReturn(Optional.of(globalNamespace)); + when(manifestLoader.load()).thenReturn(List.of(ITEM)); + when(userAccountRepository.findById(PUBLISHER)).thenReturn(Optional.of(systemPublisher())); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER)) + .thenReturn(Optional.of(new NamespaceMember(1L, PUBLISHER, NamespaceRole.OWNER))); + } + private static UserAccount systemPublisher() { - return new UserAccount(PUBLISHER, "Built-in Skill Publisher", null, null); + return UserAccount.systemAccount(PUBLISHER, "Built-in Skill Publisher", null, null); } private static Skill skill(Long id, String slug, String ownerId) { diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/user/UserAccount.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/user/UserAccount.java index 0ef2c569..19e6961f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/user/UserAccount.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/user/UserAccount.java @@ -27,6 +27,9 @@ public class UserAccount { @Column(name = "merged_to_user_id") private String mergedToUserId; + @Column(name = "system_account", nullable = false) + private boolean systemAccount = false; + @Column(name = "created_at", nullable = false, updatable = false) private Instant createdAt; @@ -43,6 +46,12 @@ public class UserAccount { this.status = UserStatus.ACTIVE; } + public static UserAccount systemAccount(String id, String displayName, String email, String avatarUrl) { + UserAccount user = new UserAccount(id, displayName, email, avatarUrl); + user.systemAccount = true; + return user; + } + @PrePersist void prePersist() { this.createdAt = Instant.now(Clock.systemUTC()); @@ -65,6 +74,7 @@ public class UserAccount { public void setStatus(UserStatus status) { this.status = status; } public String getMergedToUserId() { return mergedToUserId; } public void setMergedToUserId(String mergedToUserId) { this.mergedToUserId = mergedToUserId; } + public boolean isSystemAccount() { return systemAccount; } public Instant getCreatedAt() { return createdAt; } public Instant getUpdatedAt() { return updatedAt; } public boolean isActive() { return this.status == UserStatus.ACTIVE; } From 5cc934a294494277585568e18c927602229b5610 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Mon, 8 Jun 2026 15:20:10 +0800 Subject: [PATCH 04/11] fix(bootstrap): harden builtin skill startup sync Signed-off-by: dongmucat <1127093059@qq.com> --- docs/20-cloud-url-builtin-skills-setup.md | 9 ++- .../bootstrap/BuiltinSkillInitializer.java | 33 ++++++-- .../skillhub/service/AdminUserAppService.java | 8 ++ .../V43__user_account_system_account.sql | 5 ++ .../src/main/resources/messages.properties | 1 + .../src/main/resources/messages_zh.properties | 1 + .../BuiltinSkillInitializerTest.java | 79 +++++++++++++++---- .../BuiltinSkillManifestLoaderTest.java | 8 +- .../BuiltinSkillPackageExtractorTest.java | 10 +-- .../db/FlywayMigrationGuardrailTest.java | 16 ++++ .../service/AdminUserAppServiceTest.java | 35 ++++++++ 11 files changed, 170 insertions(+), 35 deletions(-) diff --git a/docs/20-cloud-url-builtin-skills-setup.md b/docs/20-cloud-url-builtin-skills-setup.md index a2d537fa..7e5c5eb2 100644 --- a/docs/20-cloud-url-builtin-skills-setup.md +++ b/docs/20-cloud-url-builtin-skills-setup.md @@ -15,7 +15,7 @@ 流程： ```text -维护 manifest -> 构建/部署 SkillHub 镜像 -> 应用启动 -> 读取 manifest -> 下载云存储 zip 包 -> 校验包内容 -> 发布到 @global -> 对所有用户公开可见 +维护 manifest -> 构建/部署 SkillHub 镜像 -> 应用 ready -> 后台读取 manifest -> 下载云存储 zip 包 -> 校验包内容 -> 发布到 @global -> 对所有用户公开可见 ``` 核心文件： @@ -133,7 +133,7 @@ skillhub-hello-1.0.0.zip ## 5. 启动同步流程 -应用启动时同步器只执行一次。 +应用 ready 后同步器会在后台执行一次，不阻塞应用 ready。 详细流程： @@ -175,6 +175,7 @@ skillhub-hello-1.0.0.zip | `@global/{slug}` 已存在，但 owner 不是 `builtin-skill-publisher` | 下载前跳过并记录 warning | 这意味着内置同步不会接管用户或管理员已经创建的同 slug Skill。 +同版本已存在时，同步器不会重新下载远端 zip，也不会验证远端对象内容是否发生漂移。 如果多实例同时启动，可能出现多个实例同时尝试发布同一个内置版本。同步器会在发布失败后重新查询目标版本；如果发现同版本已经以相同内容发布成功，则视为并发场景下的正常跳过。 @@ -202,7 +203,7 @@ SKILLHUB_BUILTIN_SKILLS_ENABLED=true SKILLHUB_BUILTIN_SKILLS_ENABLED=false ``` -禁用后，应用启动时不会读取 manifest，也不会下载或发布任何内置 Skill。 +禁用后，应用 ready 后不会读取 manifest，也不会下载或发布任何内置 Skill。 ## 8. 维护流程 @@ -250,7 +251,7 @@ SKILLHUB_BUILTIN_SKILLS_ENABLED=false | slug is already owned by another user | 说明 `@global/{slug}` 已被非内置发布者占用，内置同步不会覆盖 | | published fingerprint differs | 并发发布异常后发现同一内置版本已存在但内容不同，需要人工确认是否发生了版本冲突 | -如果某个 manifest item 失败，后续 item 仍会继续处理，应用启动也会继续。 +如果某个 manifest item 失败，后续 item 仍会继续处理，应用可用状态不受影响。 ## 10. 验收检查 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 63ae3981..d6626087 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 @@ -25,8 +25,9 @@ import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.boot.ApplicationArguments; -import org.springframework.boot.ApplicationRunner; +import org.springframework.boot.context.event.ApplicationReadyEvent; +import org.springframework.context.event.EventListener; +import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Component; import java.net.URI; @@ -42,7 +43,7 @@ import java.util.Set; * Best-effort startup synchronizer for remotely hosted built-in skill packages. */ @Component -public class BuiltinSkillInitializer implements ApplicationRunner { +public class BuiltinSkillInitializer { static final String GLOBAL_NAMESPACE = "global"; static final String SYSTEM_PUBLISHER_ID = "builtin-skill-publisher"; @@ -90,8 +91,13 @@ public class BuiltinSkillInitializer implements ApplicationRunner { this.skillPublishService = skillPublishService; } - @Override - public void run(ApplicationArguments args) { + @EventListener(ApplicationReadyEvent.class) + @Async("skillhubEventExecutor") + public void synchronizeAfterApplicationReady() { + synchronize(); + } + + void synchronize() { if (!properties.isEnabled()) { log.info("Built-in skill startup synchronization is disabled"); return; @@ -170,7 +176,12 @@ public class BuiltinSkillInitializer implements ApplicationRunner { return; } - Optional packageBytes = downloader.download(URI.create(item.url())); + Optional packageUri = parsePackageUri(item); + if (packageUri.isEmpty()) { + return; + } + + Optional packageBytes = downloader.download(packageUri.get()); if (packageBytes.isEmpty()) { log.warn("Skipping built-in skill slug={} version={} because package download failed", item.slug(), item.version()); @@ -226,6 +237,16 @@ public class BuiltinSkillInitializer implements ApplicationRunner { } } + private Optional parsePackageUri(ManifestItem item) { + try { + return Optional.of(URI.create(item.url())); + } catch (IllegalArgumentException exception) { + log.warn("Skipping built-in skill slug={} version={} because URL is not allowed: {}", + item.slug(), item.version(), exception.getMessage()); + return Optional.empty(); + } + } + private SkillMetadata parseSkillMetadata(List entries) { PackageEntry skillMd = entries.stream() .filter(entry -> SkillPackagePolicy.SKILL_MD_PATH.equals(entry.path())) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java index 656cefa0..b684c744 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/AdminUserAppService.java @@ -81,6 +81,7 @@ public class AdminUserAppService { @Transactional public AdminUserMutationResponse updateUserRole(String userId, String roleCode, Set actorPlatformRoles) { UserAccount user = loadUser(userId); + rejectSystemAccountMutation(user); String normalizedRoleCode = normalizeRoleCode(roleCode); if ("SUPER_ADMIN".equals(normalizedRoleCode) @@ -102,6 +103,7 @@ public class AdminUserAppService { @Transactional public AdminUserMutationResponse updateUserStatus(String userId, String status) { UserAccount user = loadUser(userId); + rejectSystemAccountMutation(user); UserStatus nextStatus = parseManageableStatus(status); user.setStatus(nextStatus); userAccountRepository.save(user); @@ -164,4 +166,10 @@ public class AdminUserAppService { return userAccountRepository.findById(userId) .orElseThrow(() -> new DomainNotFoundException("error.admin.user.notFound", userId)); } + + private void rejectSystemAccountMutation(UserAccount user) { + if (user.isSystemAccount()) { + throw new DomainForbiddenException("error.admin.user.systemAccount.immutable"); + } + } } diff --git a/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql b/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql index c525bfd2..bc3d5caa 100644 --- a/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql +++ b/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql @@ -16,4 +16,9 @@ WHERE id = 'builtin-skill-publisher' SELECT 1 FROM identity_binding WHERE identity_binding.user_id = user_account.id + ) + AND NOT EXISTS ( + SELECT 1 + FROM api_token + WHERE api_token.user_id = user_account.id ); diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index be3e2ebe..2301e0d5 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -134,6 +134,7 @@ error.deviceAuth.deviceCode.used=Device code has already been used error.admin.user.notFound=User not found: {0} error.admin.user.role.invalid=Invalid role: {0} error.admin.user.role.superAdmin.assignDenied=Only SUPER_ADMIN can assign SUPER_ADMIN role +error.admin.user.systemAccount.immutable=System accounts cannot be modified from user management error.admin.user.status.invalid=Invalid user status: {0} error.admin.user.status.unsupported=Only ACTIVE or DISABLED status can be managed here error.skill.publish.nameConflict=A published skill with name ''{0}'' already exists in this namespace diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index cef09563..a968cc77 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -134,6 +134,7 @@ error.deviceAuth.deviceCode.used=设备验证码已被使用 error.admin.user.notFound=用户不存在：{0} error.admin.user.role.invalid=无效的角色：{0} error.admin.user.role.superAdmin.assignDenied=只有 SUPER_ADMIN 可以分配 SUPER_ADMIN 角色 +error.admin.user.systemAccount.immutable=系统账号不能在用户管理中修改 error.admin.user.status.invalid=无效的用户状态：{0} error.admin.user.status.unsupported=这里只允许管理 ACTIVE 或 DISABLED 状态的用户 error.skill.publish.nameConflict=该命名空间下已存在名为"{0}"的已发布技能，无法提交 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 7afa9cd9..1f8d938a 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 @@ -35,18 +35,24 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.boot.DefaultApplicationArguments; +import org.springframework.boot.ApplicationRunner; +import org.springframework.boot.context.event.ApplicationReadyEvent; +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; +import org.springframework.context.event.EventListener; +import org.springframework.scheduling.annotation.Async; import org.springframework.test.util.ReflectionTestUtils; import java.net.URI; import java.nio.charset.StandardCharsets; +import java.lang.reflect.Method; import java.security.MessageDigest; import java.util.HexFormat; import java.util.List; import java.util.Optional; import java.util.Set; -@ExtendWith(MockitoExtension.class) +@ExtendWith({MockitoExtension.class, OutputCaptureExtension.class}) class BuiltinSkillInitializerTest { private static final String GLOBAL = "global"; @@ -97,7 +103,7 @@ class BuiltinSkillInitializerTest { void skipsWhenDisabled() { properties.setEnabled(false); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(manifestLoader, never()).load(); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); @@ -107,12 +113,26 @@ class BuiltinSkillInitializerTest { void skipsAllItemsWhenGlobalNamespaceDoesNotExist() { when(namespaceRepository.findBySlug(GLOBAL)).thenReturn(Optional.empty()); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(manifestLoader, never()).load(); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } + @Test + void synchronizesAfterApplicationReadyWithoutBlockingApplicationRunner() throws Exception { + assertThat(ApplicationRunner.class.isAssignableFrom(BuiltinSkillInitializer.class)).isFalse(); + + Method method = BuiltinSkillInitializer.class.getDeclaredMethod("synchronizeAfterApplicationReady"); + EventListener eventListener = method.getAnnotation(EventListener.class); + Async async = method.getAnnotation(Async.class); + + assertThat(eventListener).isNotNull(); + assertThat(eventListener.value()).containsExactly(ApplicationReadyEvent.class); + assertThat(async).isNotNull(); + assertThat(async.value()).isEqualTo("skillhubEventExecutor"); + } + @Test void skipsSynchronizationWhenPublisherIdIsOccupiedByNonSystemAccount() { when(namespaceRepository.findBySlug(GLOBAL)).thenReturn(Optional.of(globalNamespace)); @@ -120,7 +140,7 @@ class BuiltinSkillInitializerTest { when(userAccountRepository.findById(PUBLISHER)) .thenReturn(Optional.of(new UserAccount(PUBLISHER, "Human User", "human@example.com", null))); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(namespaceMemberRepository, never()).save(any()); verify(downloader, never()).download(any()); @@ -139,7 +159,7 @@ class BuiltinSkillInitializerTest { when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(builtinSkill)); when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(published)); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(downloader, never()).download(any()); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); @@ -157,7 +177,7 @@ class BuiltinSkillInitializerTest { when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(builtinSkill)); when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(uploaded)); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(downloader, never()).download(any()); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); @@ -169,17 +189,34 @@ class BuiltinSkillInitializerTest { givenManifestAndSystemPublisher(); when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(otherSkill)); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(downloader, never()).download(any()); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } + @Test + void skipsMalformedUrlWithoutSynchronizationFailureLog(CapturedOutput output) { + ManifestItem malformed = new ManifestItem( + "skillhub-hello", + "1.0.0", + "https://bjcdn.openstorage.cn/skills/%zz.zip" + ); + givenManifestAndSystemPublisher(List.of(malformed)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of()); + + runInitializer(); + + verify(downloader, never()).download(any()); + verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); + assertThat(output).doesNotContain("Failed to synchronize built-in skill slug=skillhub-hello"); + } + @Test void skipsWhenManifestSlugDoesNotMatchPackageMetadata() throws Exception { givenExtractedPackage(packageEntries("other-skill", "1.0.0", "same")); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } @@ -188,7 +225,7 @@ class BuiltinSkillInitializerTest { void skipsWhenManifestVersionDoesNotMatchPackageMetadata() throws Exception { givenExtractedPackage(packageEntries("skillhub-hello", "1.0.1", "same")); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } @@ -199,7 +236,7 @@ class BuiltinSkillInitializerTest { givenExtractedPackage(entries); when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of()); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); ArgumentCaptor> entriesCaptor = ArgumentCaptor.captor(); verify(skillPublishService).publishFromEntries( @@ -221,7 +258,7 @@ class BuiltinSkillInitializerTest { when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER)).thenReturn(Optional.empty()); when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of()); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); ArgumentCaptor userCaptor = ArgumentCaptor.forClass(UserAccount.class); verify(userAccountRepository).save(userCaptor.capture()); @@ -251,14 +288,14 @@ class BuiltinSkillInitializerTest { when(skillVersionRepository.findBySkillIdAndVersion(100L, "1.0.0")).thenReturn(Optional.of(published)); when(skillFileRepository.findByVersionId(200L)).thenReturn(skillFilesFor(entries, 200L)); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(skillPublishService).publishFromEntries( eq(GLOBAL), any(), eq(PUBLISHER), eq(SkillVisibility.PUBLIC), eq(Set.of("SUPER_ADMIN")), eq(false)); } @Test - void doesNotTreatConcurrentDuplicateWithDifferentFingerprintAsCompleted() throws Exception { + void doesNotTreatConcurrentDuplicateWithDifferentFingerprintAsCompleted(CapturedOutput output) throws Exception { Skill builtinSkill = skill(100L, "skillhub-hello", PUBLISHER); SkillVersion published = version(200L, 100L, "1.0.0", SkillVersionStatus.PUBLISHED); givenExtractedPackage(packageEntries("skillhub-hello", "1.0.0", "new-content")); @@ -274,11 +311,13 @@ class BuiltinSkillInitializerTest { new SkillFile(200L, "SKILL.md", 7L, "text/markdown", sha256("old-content"), "storage-key") )); - initializer.run(new DefaultApplicationArguments(new String[0])); + runInitializer(); verify(skillFileRepository).findByVersionId(200L); verify(skillPublishService).publishFromEntries( eq(GLOBAL), any(), eq(PUBLISHER), eq(SkillVisibility.PUBLIC), eq(Set.of("SUPER_ADMIN")), eq(false)); + assertThat(output).contains("Failed to publish built-in skill slug=skillhub-hello version=1.0.0"); + assertThat(output).doesNotContain("was published concurrently, skipping"); } private void givenExtractedPackage() throws Exception { @@ -297,13 +336,21 @@ class BuiltinSkillInitializerTest { } private void givenManifestAndSystemPublisher() { + givenManifestAndSystemPublisher(List.of(ITEM)); + } + + private void givenManifestAndSystemPublisher(List items) { when(namespaceRepository.findBySlug(GLOBAL)).thenReturn(Optional.of(globalNamespace)); - when(manifestLoader.load()).thenReturn(List.of(ITEM)); + when(manifestLoader.load()).thenReturn(items); when(userAccountRepository.findById(PUBLISHER)).thenReturn(Optional.of(systemPublisher())); when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, PUBLISHER)) .thenReturn(Optional.of(new NamespaceMember(1L, PUBLISHER, NamespaceRole.OWNER))); } + private void runInitializer() { + initializer.synchronize(); + } + private static UserAccount systemPublisher() { return UserAccount.systemAccount(PUBLISHER, "Built-in Skill Publisher", null, null); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoaderTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoaderTest.java index 39ea4b38..66bc128f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoaderTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillManifestLoaderTest.java @@ -17,8 +17,8 @@ class BuiltinSkillManifestLoaderTest { BuiltinSkillManifestLoader loader = loaderWith(""" { "skills": [ - {"slug": "agentguard", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/agentguard.zip"}, - {"slug": "agentguard", "version": "1.1.0", "url": "https://cdn.bjcdn.openstorage.cn/agentguard.zip"} + {"slug": "skillhub-hello", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/skillhub-hello.zip"}, + {"slug": "skillhub-hello", "version": "1.1.0", "url": "https://cdn.bjcdn.openstorage.cn/skillhub-hello.zip"} ] } """); @@ -69,8 +69,8 @@ class BuiltinSkillManifestLoaderTest { BuiltinSkillManifestLoader loader = loaderWith(""" { "skills": [ - {"slug": "agentguard", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/first.zip"}, - {"slug": "agentguard", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/second.zip"}, + {"slug": "skillhub-hello", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/first.zip"}, + {"slug": "skillhub-hello", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/second.zip"}, {"slug": "InvalidUppercase", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/invalid.zip"}, {"slug": "", "version": "1.0.0", "url": "https://bjcdn.openstorage.cn/blank.zip"}, {"slug": "missing-version", "url": "https://bjcdn.openstorage.cn/missing-version.zip"}, 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 99f710b9..1df4203d 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 @@ -24,10 +24,10 @@ class BuiltinSkillPackageExtractorTest { byte[] zip = zip( entry("SKILL.md", """ --- - name: agentguard + name: skillhub-hello version: 1.0.0 --- - # AgentGuard + # SkillHub Hello """), entry("README.md", "# Readme") ); @@ -50,12 +50,12 @@ class BuiltinSkillPackageExtractorTest { @Test void rejectsZipWithOnlyNestedSkillMd() throws Exception { - byte[] zip = zip(entry("agentguard/SKILL.md", """ + byte[] zip = zip(entry("skillhub-hello/SKILL.md", """ --- - name: agentguard + name: skillhub-hello version: 1.0.0 --- - # AgentGuard + # SkillHub Hello """)); assertThatThrownBy(() -> extractor.extract(zip)) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java index ee23aee6..f2b87da5 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java @@ -72,6 +72,14 @@ class FlywayMigrationGuardrailTest { assertThat(invalidFiles).isEmpty(); } + @Test + void systemAccountMigration_mustNotPromoteUsersWithApiTokens() throws IOException { + String migration = Files.readString(migrationPath("V43__user_account_system_account.sql")); + + assertThat(migration).contains("FROM api_token"); + assertThat(migration).contains("api_token.user_id = user_account.id"); + } + private List migrationFiles() throws IOException { Path root = repoRoot() .resolve("server") @@ -92,4 +100,12 @@ class FlywayMigrationGuardrailTest { private String relativeToRepo(Path file) { return repoRoot().relativize(file).toString(); } + + private Path migrationPath(String fileName) { + return repoRoot() + .resolve("server") + .resolve("skillhub-app") + .resolve("src/main/resources/db/migration") + .resolve(fileName); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java index 4dd22e9c..7f2cfc22 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AdminUserAppServiceTest.java @@ -89,6 +89,18 @@ class AdminUserAppServiceTest { () -> service.updateUserRole("user-1", "SUPER_ADMIN", Set.of("USER_ADMIN"))); } + @Test + void updateUserRole_rejectsSystemAccount() { + when(userAccountRepository.findById("builtin-skill-publisher")) + .thenReturn(Optional.of(systemUser())); + + assertThrows(DomainForbiddenException.class, + () -> service.updateUserRole("builtin-skill-publisher", "AUDITOR", Set.of("SUPER_ADMIN"))); + + verify(userRoleBindingRepository, never()).deleteByUserId(any()); + verify(userRoleBindingRepository, never()).save(any(UserRoleBinding.class)); + } + @Test void updateUserRole_replacesExistingBindings() { when(userAccountRepository.findById("user-1")) @@ -137,6 +149,17 @@ class AdminUserAppServiceTest { assertThat(response.status()).isEqualTo("DISABLED"); } + @Test + void updateUserStatus_rejectsSystemAccount() { + when(userAccountRepository.findById("builtin-skill-publisher")) + .thenReturn(Optional.of(systemUser())); + + assertThrows(DomainForbiddenException.class, + () -> service.updateUserStatus("builtin-skill-publisher", "DISABLED")); + + verify(userAccountRepository, never()).save(any(UserAccount.class)); + } + @Test void updateUserStatus_withUnknownUser_throwsNotFound() { when(userAccountRepository.findById("missing")).thenReturn(Optional.empty()); @@ -152,6 +175,18 @@ class AdminUserAppServiceTest { return user; } + private UserAccount systemUser() { + UserAccount user = UserAccount.systemAccount( + "builtin-skill-publisher", + "Built-in Skill Publisher", + null, + null + ); + ReflectionTestUtils.setField(user, "createdAt", Instant.parse("2026-03-13T09:00:00Z")); + ReflectionTestUtils.setField(user, "updatedAt", Instant.parse("2026-03-13T09:00:00Z")); + return user; + } + private Role role(String code) { Role role = new Role(); ReflectionTestUtils.setField(role, "code", code); From 973c336c82ae66c507acc3019264bcd38392d608 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Mon, 8 Jun 2026 18:01:20 +0800 Subject: [PATCH 05/11] fix(auth): protect builtin system account boundaries Signed-off-by: dongmucat <1127093059@qq.com> --- docs/20-cloud-url-builtin-skills-setup.md | 2 +- .../V43__user_account_system_account.sql | 10 +++++++ .../db/FlywayMigrationGuardrailTest.java | 10 +++++++ .../auth/local/PasswordResetService.java | 3 ++ .../auth/local/PasswordResetServiceTest.java | 28 +++++++++++++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) diff --git a/docs/20-cloud-url-builtin-skills-setup.md b/docs/20-cloud-url-builtin-skills-setup.md index 7e5c5eb2..097a26d0 100644 --- a/docs/20-cloud-url-builtin-skills-setup.md +++ b/docs/20-cloud-url-builtin-skills-setup.md @@ -142,7 +142,7 @@ skillhub-hello-1.0.0.zip 3. 查询 `@global` 命名空间是否存在；如果不存在，跳过同步。 4. 确保系统发布者 `builtin-skill-publisher` 存在，并且该账号带有系统账号标记。 5. 如果该用户 ID 已被非系统账号占用，直接跳过本次内置 Skill 同步，不授予 `@global` 权限。 -6. 确保系统发布者是 `@global` 的 `OWNER`。 +6. 如果系统发布者还不是 `@global` 成员，则创建 `OWNER` 成员记录；已有成员记录不会自动改角色。 7. 按 manifest 顺序处理每一个 item。 8. 下载前先检查 `@global/{slug}` 和目标版本是否已经存在；如果已经确定应跳过，则不发起远程下载。 9. 只有需要发布新 Skill 或新版本时，才下载对应 zip 包。 diff --git a/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql b/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql index bc3d5caa..c96bb9fa 100644 --- a/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql +++ b/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql @@ -21,4 +21,14 @@ WHERE id = 'builtin-skill-publisher' SELECT 1 FROM api_token WHERE api_token.user_id = user_account.id + ) + AND NOT EXISTS ( + SELECT 1 + FROM user_role_binding + WHERE user_role_binding.user_id = user_account.id + ) + AND NOT EXISTS ( + SELECT 1 + FROM namespace_member + WHERE namespace_member.user_id = user_account.id ); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java index f2b87da5..aad2480f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java @@ -80,6 +80,16 @@ class FlywayMigrationGuardrailTest { assertThat(migration).contains("api_token.user_id = user_account.id"); } + @Test + void systemAccountMigration_mustNotPromoteUsersWithRolesOrNamespaceMemberships() throws IOException { + String migration = Files.readString(migrationPath("V43__user_account_system_account.sql")); + + assertThat(migration).contains("FROM user_role_binding"); + assertThat(migration).contains("user_role_binding.user_id = user_account.id"); + assertThat(migration).contains("FROM namespace_member"); + assertThat(migration).contains("namespace_member.user_id = user_account.id"); + } + private List migrationFiles() throws IOException { Path root = repoRoot() .resolve("server") diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetService.java index 60b52acc..74653ead 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/local/PasswordResetService.java @@ -178,6 +178,9 @@ public class PasswordResetService { } private boolean isEligibleForReset(UserAccount user) { + if (user.isSystemAccount()) { + return false; + } if (user.getStatus() != UserStatus.ACTIVE) { return false; } diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java index aa78c1fb..251b7d54 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/local/PasswordResetServiceTest.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.atLeastOnce; @@ -215,4 +216,31 @@ class PasswordResetServiceTest { .extracting("status") .isEqualTo(HttpStatus.BAD_REQUEST); } + + @Test + void adminTriggerPasswordReset_forSystemAccount_throwsBadRequest() { + UserAccount user = UserAccount.systemAccount( + "builtin-skill-publisher", + "Built-in Skill Publisher", + "builtin@example.com", + null + ); + given(userAccountRepository.findById("builtin-skill-publisher")).willReturn(Optional.of(user)); + lenient().when(credentialRepository.findByUserId("builtin-skill-publisher")).thenReturn( + Optional.of(new LocalCredential("builtin-skill-publisher", "builtin", "encoded")) + ); + lenient().when(resetRequestRepository.findByUserIdAndConsumedAtIsNullAndExpiresAtAfterOrderByCreatedAtDesc( + anyString(), any(Instant.class)) + ).thenReturn(List.of()); + lenient().when(passwordEncoder.encode(anyString())).thenReturn("encoded-value"); + + assertThatThrownBy(() -> service.adminTriggerPasswordReset("builtin-skill-publisher", "admin_1")) + .isInstanceOf(AuthFlowException.class) + .extracting("status") + .isEqualTo(HttpStatus.BAD_REQUEST); + + verify(credentialRepository, never()).findByUserId("builtin-skill-publisher"); + verify(resetRequestRepository, never()).save(any(PasswordResetRequest.class)); + verify(mailSender, never()).send(any(SimpleMailMessage.class)); + } } From 973c37613ee8429a27ce8bd23c2fb394d3c8b203 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Tue, 9 Jun 2026 14:19:25 +0800 Subject: [PATCH 06/11] fix(bootstrap): harden builtin skill sync Signed-off-by: dongmucat <1127093059@qq.com> --- docs/20-cloud-url-builtin-skills-setup.md | 6 +- .../bootstrap/BuiltinSkillInitializer.java | 20 ++-- .../BuiltinSkillRemotePackageDownloader.java | 74 ++++++++++-- .../V43__user_account_system_account.sql | 49 ++++++++ .../BuiltinSkillInitializerTest.java | 29 ++++- ...iltinSkillRemotePackageDownloaderTest.java | 106 +++++++++++++++++- .../db/FlywayMigrationGuardrailTest.java | 12 ++ 7 files changed, 273 insertions(+), 23 deletions(-) diff --git a/docs/20-cloud-url-builtin-skills-setup.md b/docs/20-cloud-url-builtin-skills-setup.md index 097a26d0..24bac647 100644 --- a/docs/20-cloud-url-builtin-skills-setup.md +++ b/docs/20-cloud-url-builtin-skills-setup.md @@ -172,9 +172,9 @@ skillhub-hello-1.0.0.zip | `@global/{slug}` 已存在，owner 是 `builtin-skill-publisher`，但目标版本不存在 | 发布新版本 | | 同版本已存在且已发布 | 下载前跳过 | | 同版本已存在但不是 `PUBLISHED` | 下载前跳过并记录日志 | -| `@global/{slug}` 已存在，但 owner 不是 `builtin-skill-publisher` | 下载前跳过并记录 warning | +| `@global/{slug}` 已被其他 owner 发布 | 下载前跳过并记录 warning | -这意味着内置同步不会接管用户或管理员已经创建的同 slug Skill。 +这意味着内置同步不会接管用户或管理员已经发布的同 slug Skill；仅有待审或未发布版本不会阻断内置同步。 同版本已存在时，同步器不会重新下载远端 zip，也不会验证远端对象内容是否发生漂移。 如果多实例同时启动，可能出现多个实例同时尝试发布同一个内置版本。同步器会在发布失败后重新查询目标版本；如果发现同版本已经以相同内容发布成功，则视为并发场景下的正常跳过。 @@ -248,7 +248,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 owned by another user | 说明 `@global/{slug}` 已被非内置发布者占用，内置同步不会覆盖 | +| slug is already published by 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 d6626087..5145c404 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,10 +258,8 @@ public class BuiltinSkillInitializer { private boolean shouldSkipBeforeDownload(Long namespaceId, ManifestItem item) { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); - boolean hasNonBuiltinOwner = existingSkills.stream() - .anyMatch(skill -> !SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())); - if (hasNonBuiltinOwner) { - log.warn("Skipping built-in skill slug={} before download because the slug is already owned by another user", + if (hasPublishedOtherOwnerConflict(existingSkills)) { + log.warn("Skipping built-in skill slug={} before download because the slug is already published by another user", item.slug()); return true; } @@ -292,10 +290,8 @@ public class BuiltinSkillInitializer { private boolean shouldSkipExisting(Long namespaceId, ManifestItem item, List entries) { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); - boolean hasNonBuiltinOwner = existingSkills.stream() - .anyMatch(skill -> !SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())); - if (hasNonBuiltinOwner) { - log.warn("Skipping built-in skill slug={} because the slug is already owned by another user", + if (hasPublishedOtherOwnerConflict(existingSkills)) { + log.warn("Skipping built-in skill slug={} because the slug is already published by another user", item.slug()); return true; } @@ -364,6 +360,14 @@ public class BuiltinSkillInitializer { return false; } + private boolean hasPublishedOtherOwnerConflict(List existingSkills) { + return existingSkills.stream() + .filter(skill -> !SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())) + .anyMatch(skill -> !skillVersionRepository + .findBySkillIdAndStatus(skill.getId(), SkillVersionStatus.PUBLISHED) + .isEmpty()); + } + private String computeFingerprint(SkillVersion version) { List files = skillFileRepository.findByVersionId(version.getId()).stream() .sorted(Comparator.comparing(SkillFile::getFilePath)) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloader.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloader.java index c2819d3f..2e507f73 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloader.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloader.java @@ -16,6 +16,12 @@ import java.net.http.HttpResponse; import java.time.Duration; import java.util.Locale; import java.util.Optional; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.regex.Pattern; @Component @@ -30,6 +36,7 @@ public class BuiltinSkillRemotePackageDownloader { private final long maxPackageSize; private final HttpClient httpClient; + private final Duration requestTimeout; @Autowired public BuiltinSkillRemotePackageDownloader(SkillPublishProperties properties) { @@ -38,13 +45,22 @@ public class BuiltinSkillRemotePackageDownloader { HttpClient.newBuilder() .connectTimeout(CONNECT_TIMEOUT) .followRedirects(HttpClient.Redirect.NEVER) - .build() + .build(), + REQUEST_TIMEOUT ); } BuiltinSkillRemotePackageDownloader(SkillPublishProperties properties, HttpClient httpClient) { + this(properties, httpClient, REQUEST_TIMEOUT); + } + + BuiltinSkillRemotePackageDownloader( + SkillPublishProperties properties, + HttpClient httpClient, + Duration requestTimeout) { this.maxPackageSize = properties.getMaxPackageSize(); this.httpClient = httpClient; + this.requestTimeout = requestTimeout; } public Optional download(URI uri) { @@ -54,19 +70,19 @@ public class BuiltinSkillRemotePackageDownloader { } HttpRequest request = HttpRequest.newBuilder(uri) - .timeout(REQUEST_TIMEOUT) + .timeout(requestTimeout) .GET() .build(); try { HttpResponse response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream()); - if (response.statusCode() != 200) { - log.warn("Failed to download built-in skill package from {}: HTTP {}", - safeUrl(uri), - response.statusCode()); - return Optional.empty(); - } try (InputStream body = response.body()) { - return readBounded(body); + if (response.statusCode() != 200) { + log.warn("Failed to download built-in skill package from {}: HTTP {}", + safeUrl(uri), + response.statusCode()); + return Optional.empty(); + } + return readBoundedWithTimeout(body, uri); } } catch (IOException ex) { log.warn("Failed to download built-in skill package from {}: {}", safeUrl(uri), ex.getMessage()); @@ -125,6 +141,46 @@ public class BuiltinSkillRemotePackageDownloader { return Optional.of(outputStream.toByteArray()); } + private Optional readBoundedWithTimeout(InputStream inputStream, URI uri) throws IOException { + ExecutorService executor = Executors.newVirtualThreadPerTaskExecutor(); + Future> future = executor.submit(() -> readBounded(inputStream)); + try { + return future.get(Math.max(1, requestTimeout.toMillis()), TimeUnit.MILLISECONDS); + } catch (TimeoutException ex) { + closeQuietly(inputStream); + future.cancel(true); + log.warn("Timed out while downloading built-in skill package body from {} after {}", + safeUrl(uri), + requestTimeout); + return Optional.empty(); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + closeQuietly(inputStream); + future.cancel(true); + log.warn("Interrupted while reading built-in skill package body from {}", safeUrl(uri)); + return Optional.empty(); + } catch (ExecutionException ex) { + Throwable cause = ex.getCause(); + if (cause instanceof IOException ioException) { + throw ioException; + } + if (cause instanceof RuntimeException runtimeException) { + throw runtimeException; + } + throw new IllegalStateException("Failed to read built-in skill package body", cause); + } finally { + executor.shutdownNow(); + } + } + + private static void closeQuietly(InputStream inputStream) { + try { + inputStream.close(); + } catch (IOException ignored) { + // Best-effort cleanup after timeout/interruption. + } + } + private static boolean isDisallowedHostLiteral(String host) { return "localhost".equals(host) || IPV4_LITERAL.matcher(host).matches() diff --git a/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql b/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql index c96bb9fa..af7456ef 100644 --- a/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql +++ b/server/skillhub-app/src/main/resources/db/migration/V43__user_account_system_account.sql @@ -32,3 +32,52 @@ WHERE id = 'builtin-skill-publisher' FROM namespace_member WHERE namespace_member.user_id = user_account.id ); + +UPDATE user_account +SET system_account = TRUE, + display_name = 'Built-in Skill Publisher', + email = NULL, + avatar_url = NULL +WHERE id = 'builtin-skill-publisher' + AND display_name = 'SkillHub Built-in Publisher' + AND email = 'builtin-skill-publisher@example.invalid' + AND avatar_url IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM local_credential + WHERE local_credential.user_id = user_account.id + ) + AND NOT EXISTS ( + SELECT 1 + FROM identity_binding + WHERE identity_binding.user_id = user_account.id + ) + AND NOT EXISTS ( + SELECT 1 + FROM api_token + WHERE api_token.user_id = user_account.id + ) + AND NOT EXISTS ( + SELECT 1 + FROM user_role_binding + WHERE user_role_binding.user_id = user_account.id + ) + AND EXISTS ( + SELECT 1 + FROM namespace_member legacy_member + JOIN namespace legacy_namespace ON legacy_namespace.id = legacy_member.namespace_id + WHERE legacy_member.user_id = user_account.id + AND legacy_namespace.slug = 'global' + AND legacy_member.role = 'OWNER' + ) + AND NOT EXISTS ( + SELECT 1 + FROM namespace_member bad_member + LEFT JOIN namespace bad_namespace ON bad_namespace.id = bad_member.namespace_id + WHERE bad_member.user_id = user_account.id + AND ( + bad_namespace.slug IS NULL + OR bad_namespace.slug <> 'global' + OR bad_member.role <> 'OWNER' + ) + ); 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 1f8d938a..01310147 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,10 +184,13 @@ class BuiltinSkillInitializerTest { } @Test - void skipsExistingSkillOwnedByAnotherUserBeforeDownloadingPackage() throws Exception { + void skipsPublishedSkillOwnedByAnotherUserBeforeDownloadingPackage() throws Exception { 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(); @@ -195,6 +198,30 @@ class BuiltinSkillInitializerTest { verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } + @Test + void publishesWhenOnlyUnpublishedSkillOwnedByAnotherUserExists() throws Exception { + 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()); + + 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) + ); + } + @Test void skipsMalformedUrlWithoutSynchronizationFailureLog(CapturedOutput output) { ManifestItem malformed = new ManifestItem( diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloaderTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloaderTest.java index 9dbf2cdf..dc935f5d 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloaderTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillRemotePackageDownloaderTest.java @@ -10,6 +10,7 @@ import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSession; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.net.Authenticator; import java.net.CookieHandler; import java.net.ProxySelector; @@ -22,6 +23,11 @@ import java.time.Duration; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; class BuiltinSkillRemotePackageDownloaderTest { @@ -81,6 +87,21 @@ class BuiltinSkillRemotePackageDownloaderTest { assertThat(client.sendCalls).isEqualTo(1); } + @Test + void closesNonSuccessResponseBody() { + CloseAwareInputStream body = new CloseAwareInputStream(new byte[] {1}); + FakeHttpClient client = new FakeHttpClient(500, body); + BuiltinSkillRemotePackageDownloader downloader = new BuiltinSkillRemotePackageDownloader( + new SkillPublishProperties(), + client + ); + + Optional bytes = downloader.download(URI.create("https://bjcdn.openstorage.cn/package.zip")); + + assertThat(bytes).isEmpty(); + assertThat(body.closed()).isTrue(); + } + @Test void rejectedUrlDoesNotSendHttpRequest() { FakeHttpClient client = new FakeHttpClient(200, new byte[] {1}); @@ -105,14 +126,40 @@ class BuiltinSkillRemotePackageDownloaderTest { assertThat(downloader.download(URI.create("https://bjcdn.openstorage.cn/package.zip"))).isEmpty(); } + @Test + void returnsEmptyWhenResponseBodyStopsBeforeCompletion() throws Exception { + BlockingInputStream body = new BlockingInputStream(); + FakeHttpClient client = new FakeHttpClient(200, body); + BuiltinSkillRemotePackageDownloader downloader = new BuiltinSkillRemotePackageDownloader( + new SkillPublishProperties(), + client, + Duration.ofMillis(50) + ); + ExecutorService executor = Executors.newSingleThreadExecutor(); + Future> result = executor.submit( + () -> downloader.download(URI.create("https://bjcdn.openstorage.cn/package.zip"))); + + try { + assertThat(result.get(1, TimeUnit.SECONDS)).isEmpty(); + assertThat(body.closed()).isTrue(); + } finally { + body.close(); + executor.shutdownNow(); + } + } + static class FakeHttpClient extends HttpClient { private final int statusCode; - private final byte[] body; + private final InputStream body; private HttpRequest lastRequest; private int sendCalls; FakeHttpClient(int statusCode, byte[] body) { + this(statusCode, new ByteArrayInputStream(body)); + } + + FakeHttpClient(int statusCode, InputStream body) { this.statusCode = statusCode; this.body = body; } @@ -168,7 +215,7 @@ class BuiltinSkillRemotePackageDownloaderTest { lastRequest = request; sendCalls++; @SuppressWarnings("unchecked") - T responseBody = (T) new ByteArrayInputStream(body); + T responseBody = (T) body; return new FakeResponse<>(request, statusCode, responseBody); } @@ -190,6 +237,61 @@ class BuiltinSkillRemotePackageDownloaderTest { } } + static final class CloseAwareInputStream extends ByteArrayInputStream { + + private boolean closed; + + private CloseAwareInputStream(byte[] bytes) { + super(bytes); + } + + @Override + public void close() throws IOException { + closed = true; + super.close(); + } + + boolean closed() { + return closed; + } + } + + static final class BlockingInputStream extends InputStream { + + private final AtomicBoolean closed = new AtomicBoolean(); + + @Override + public int read() { + waitUntilClosed(); + return -1; + } + + @Override + public int read(byte[] bytes, int offset, int length) { + waitUntilClosed(); + return -1; + } + + @Override + public void close() { + closed.set(true); + } + + boolean closed() { + return closed.get(); + } + + private void waitUntilClosed() { + while (!closed.get()) { + try { + Thread.sleep(10); + } catch (InterruptedException ignored) { + Thread.currentThread().interrupt(); + } + } + } + } + record FakeResponse(HttpRequest request, int statusCode, T body) implements HttpResponse { @Override public Optional> previousResponse() { diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java index aad2480f..21369016 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/db/FlywayMigrationGuardrailTest.java @@ -90,6 +90,18 @@ class FlywayMigrationGuardrailTest { assertThat(migration).contains("namespace_member.user_id = user_account.id"); } + @Test + void systemAccountMigration_mustPromoteLegacyBuiltinPublisherSafely() throws IOException { + String migration = Files.readString(migrationPath("V43__user_account_system_account.sql")); + + assertThat(migration).contains("SkillHub Built-in Publisher"); + assertThat(migration).contains("builtin-skill-publisher@example.invalid"); + assertThat(migration).contains("legacy_namespace.slug = 'global'"); + assertThat(migration).contains("legacy_member.role = 'OWNER'"); + assertThat(migration).contains("bad_member.user_id = user_account.id"); + assertThat(migration).contains("bad_namespace.slug <> 'global'"); + } + private List migrationFiles() throws IOException { Path root = repoRoot() .resolve("server") From dd3e511a91cf26573cbf39757808578143e1a12e Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Wed, 10 Jun 2026 10:31:40 +0800 Subject: [PATCH 07/11] fix(bootstrap): support skill directory archives Signed-off-by: dongmucat <1127093059@qq.com> --- docs/20-cloud-url-builtin-skills-setup.md | 17 +++++++------- .../BuiltinSkillPackageExtractor.java | 22 +++---------------- .../BuiltinSkillPackageExtractorTest.java | 12 +++++----- 3 files changed, 19 insertions(+), 32 deletions(-) diff --git a/docs/20-cloud-url-builtin-skills-setup.md b/docs/20-cloud-url-builtin-skills-setup.md index 24bac647..316faa8d 100644 --- a/docs/20-cloud-url-builtin-skills-setup.md +++ b/docs/20-cloud-url-builtin-skills-setup.md @@ -83,7 +83,7 @@ manifest 文件格式如下： manifest 中的 `url` 必须指向 zip 包。zip 包需要满足 SkillHub Skill 包协议： -- zip 根目录必须包含 `SKILL.md`。 +- zip 可以在根目录直接包含 `SKILL.md`，也可以包含一个单独的顶层 Skill 目录，并在该目录下包含 `SKILL.md`。 - `SKILL.md` frontmatter 中必须包含合法的 `name`、`description`、`version` 等元数据。 - `SKILL.md` 中的 `name` 经过 slug 归一化后，必须等于 manifest 中的 `slug`。 - `SKILL.md` 中的 `version` 必须等于 manifest 中的 `version`。 @@ -99,15 +99,16 @@ skillhub-hello-1.0.0.zip └── check.js ``` -不推荐的结构： +同样支持标准单目录 Skill 包： ```text skillhub-hello-1.0.0.zip └── skillhub-hello/ - └── SKILL.md + ├── SKILL.md + └── README.md ``` -原因是内置 Skill 同步要求根目录存在 `SKILL.md`，不会把嵌套目录中的 `SKILL.md` 当作入口。 +如果 zip 中存在多个顶层目录，或在多个目录中同时出现 `SKILL.md`，同步器会跳过该项并记录错误，避免误选入口。 ## 4. URL 安全限制 @@ -146,7 +147,7 @@ skillhub-hello-1.0.0.zip 7. 按 manifest 顺序处理每一个 item。 8. 下载前先检查 `@global/{slug}` 和目标版本是否已经存在；如果已经确定应跳过，则不发起远程下载。 9. 只有需要发布新 Skill 或新版本时，才下载对应 zip 包。 -10. 解包并校验根目录 `SKILL.md`。 +10. 解包并校验 Skill 入口 `SKILL.md`。 11. 校验 manifest 中的 `slug`、`version` 与包内元数据一致。 12. 发布前再次检查是否已存在同名 Skill 或同版本，处理并发启动场景。 13. 需要发布时调用现有 `SkillPublishService.publishFromEntries(...)`。 @@ -209,7 +210,7 @@ SKILLHUB_BUILTIN_SKILLS_ENABLED=false 新增一个内置 Skill 的推荐步骤： -1. 准备 Skill 包，并确认 zip 根目录包含 `SKILL.md`。 +1. 准备 Skill 包，并确认 zip 根目录直接包含 `SKILL.md`，或只有一个顶层 Skill 目录且该目录包含 `SKILL.md`。 2. 检查 `SKILL.md` 中的 `name` 和 `version`。 3. 上传 zip 到 `bjcdn.openstorage.cn` 或其子域名下的官方云存储路径。 4. 在 `server/skillhub-app/src/main/resources/builtin-skills/manifest.json` 中新增一项。 @@ -246,7 +247,7 @@ SKILLHUB_BUILTIN_SKILLS_ENABLED=false | slug is invalid | 检查 slug 是否符合 SkillHub slug 规则 | | URL is not allowed | 检查 URL 是否为 HTTPS、host 是否为 `bjcdn.openstorage.cn` 或其子域名 | | package download failed | 检查云存储对象是否存在、是否返回 HTTP 200、是否超时 | -| package must contain SKILL.md | 检查 zip 根目录是否存在 `SKILL.md` | +| 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}` 已被非内置发布者发布，内置同步不会覆盖 | | published fingerprint differs | 并发发布异常后发现同一内置版本已存在但内容不同，需要人工确认是否发生了版本冲突 | @@ -260,7 +261,7 @@ SKILLHUB_BUILTIN_SKILLS_ENABLED=false - manifest JSON 格式合法。 - 每个 item 都包含 `slug`、`version`、`url`。 - URL 使用 `https://bjcdn.openstorage.cn/...` 或可信子域名。 -- zip 根目录包含 `SKILL.md`。 +- zip 根目录直接包含 `SKILL.md`，或只有一个顶层 Skill 目录且该目录包含 `SKILL.md`。 - `SKILL.md name` 归一化后的 slug 与 manifest `slug` 一致。 - `SKILL.md version` 与 manifest `version` 一致。 - 启动日志没有该 item 的 warning 或 error。 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 9cd44cd6..d023190d 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 @@ -8,8 +8,6 @@ import org.springframework.web.multipart.MultipartFile; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; -import java.util.zip.ZipEntry; -import java.util.zip.ZipInputStream; @Component public class BuiltinSkillPackageExtractor { @@ -21,30 +19,16 @@ public class BuiltinSkillPackageExtractor { } public SkillPackageArchiveExtractor.ExtractionResult extract(byte[] zipBytes) throws IOException { - assertRootSkillMd(zipBytes); SkillPackageArchiveExtractor.ExtractionResult result = archiveExtractor.extractWithWarnings(new ByteArrayMultipartFile(zipBytes)); - boolean hasRootSkillMd = result.entries().stream() + boolean hasSkillMd = result.entries().stream() .anyMatch(entry -> SkillPackagePolicy.SKILL_MD_PATH.equals(entry.path())); - if (!hasRootSkillMd) { - throw new IllegalArgumentException("Built-in skill package must contain root " + SkillPackagePolicy.SKILL_MD_PATH); + if (!hasSkillMd) { + throw new IllegalArgumentException("Built-in skill package must contain " + SkillPackagePolicy.SKILL_MD_PATH); } return result; } - private void assertRootSkillMd(byte[] zipBytes) throws IOException { - try (ZipInputStream zipInputStream = new ZipInputStream(new ByteArrayInputStream(zipBytes))) { - ZipEntry entry; - while ((entry = zipInputStream.getNextEntry()) != null) { - if (!entry.isDirectory() && SkillPackagePolicy.SKILL_MD_PATH.equals(entry.getName())) { - return; - } - zipInputStream.closeEntry(); - } - } - throw new IllegalArgumentException("Built-in skill package must contain root " + SkillPackagePolicy.SKILL_MD_PATH); - } - private record ByteArrayMultipartFile(byte[] bytes) implements MultipartFile { @Override 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 1df4203d..8836546f 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 @@ -49,18 +49,20 @@ class BuiltinSkillPackageExtractorTest { } @Test - void rejectsZipWithOnlyNestedSkillMd() throws Exception { + void acceptsZipWithSingleTopLevelSkillDirectory() throws Exception { byte[] zip = zip(entry("skillhub-hello/SKILL.md", """ --- name: skillhub-hello version: 1.0.0 --- # SkillHub Hello - """)); + """), entry("skillhub-hello/README.md", "# Readme")); - assertThatThrownBy(() -> extractor.extract(zip)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining(SkillPackagePolicy.SKILL_MD_PATH); + SkillPackageArchiveExtractor.ExtractionResult result = extractor.extract(zip); + + assertThat(result.entries()) + .extracting(entry -> entry.path()) + .containsExactly("SKILL.md", "README.md"); } private static ZipSource entry(String path, String content) { 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 08/11] 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)); } From f35f91616a4e4f75de3aa14c0b56f980860423ff Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Wed, 10 Jun 2026 16:51:35 +0800 Subject: [PATCH 09/11] fix(publish): preserve latest version reference cleanup order Signed-off-by: dongmucat <1127093059@qq.com> --- .../bootstrap/BuiltinSkillInitializer.java | 75 +++++++++++++------ .../BuiltinSkillInitializerTest.java | 10 ++- .../skill/service/SkillPublishService.java | 11 ++- .../service/SkillPublishServiceTest.java | 7 +- 4 files changed, 72 insertions(+), 31 deletions(-) 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 34379db7..20b9a363 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 @@ -126,10 +126,21 @@ public class BuiltinSkillInitializer { return; } + int published = 0; + int idempotentSkipped = 0; + int conflictSkipped = 0; + int failed = 0; for (ManifestItem item : items) { try { - syncItem(namespace.get(), item); + SyncOutcome outcome = syncItem(namespace.get(), item); + switch (outcome) { + case PUBLISHED -> published++; + case IDEMPOTENT_SKIPPED -> idempotentSkipped++; + case CONFLICT_SKIPPED -> conflictSkipped++; + case FAILED -> failed++; + } } catch (Exception exception) { + failed++; log.error( "Failed to synchronize built-in skill slug={} version={}: {}", item.slug(), @@ -139,6 +150,14 @@ public class BuiltinSkillInitializer { ); } } + log.info( + "Built-in skill synchronization finished: total={}, published={}, idempotentSkipped={}, conflictSkipped={}, failed={}", + items.size(), + published, + idempotentSkipped, + conflictSkipped, + failed + ); } private boolean ensureSystemPublisher(Namespace namespace) { @@ -171,21 +190,22 @@ public class BuiltinSkillInitializer { return true; } - private void syncItem(Namespace namespace, ManifestItem item) throws Exception { - if (shouldSkipBeforeDownload(namespace.getId(), item)) { - return; + private SyncOutcome syncItem(Namespace namespace, ManifestItem item) throws Exception { + Optional skipBeforeDownload = shouldSkipBeforeDownload(namespace.getId(), item); + if (skipBeforeDownload.isPresent()) { + return skipBeforeDownload.get(); } Optional packageUri = parsePackageUri(item); if (packageUri.isEmpty()) { - return; + return SyncOutcome.FAILED; } Optional packageBytes = downloader.download(packageUri.get()); if (packageBytes.isEmpty()) { log.warn("Skipping built-in skill slug={} version={} because package download failed", item.slug(), item.version()); - return; + return SyncOutcome.FAILED; } SkillPackageArchiveExtractor.ExtractionResult extractionResult = extractor.extract(packageBytes.get()); @@ -199,7 +219,7 @@ public class BuiltinSkillInitializer { item.version(), packageSlug ); - return; + return SyncOutcome.FAILED; } if (!item.version().equals(metadata.version())) { log.warn( @@ -208,11 +228,12 @@ public class BuiltinSkillInitializer { item.version(), metadata.version() ); - return; + return SyncOutcome.FAILED; } - if (shouldSkipExisting(namespace.getId(), item, entries)) { - return; + Optional skipExisting = shouldSkipExisting(namespace.getId(), item, entries); + if (skipExisting.isPresent()) { + return skipExisting.get(); } try { @@ -226,14 +247,16 @@ public class BuiltinSkillInitializer { ); log.info("Published built-in skill slug={} version={} to @{}", item.slug(), item.version(), GLOBAL_NAMESPACE); + return SyncOutcome.PUBLISHED; } catch (RuntimeException exception) { if (isAlreadyPublishedWithSameFingerprint(namespace.getId(), item, entries)) { log.info("Built-in skill slug={} version={} was published concurrently, skipping", item.slug(), item.version()); - return; + return SyncOutcome.IDEMPOTENT_SKIPPED; } log.error("Failed to publish built-in skill slug={} version={}: {}", item.slug(), item.version(), exception.getMessage(), exception); + return SyncOutcome.FAILED; } } @@ -256,25 +279,25 @@ public class BuiltinSkillInitializer { return metadataParser.parse(new String(skillMd.content(), StandardCharsets.UTF_8)); } - private boolean shouldSkipBeforeDownload(Long namespaceId, ManifestItem item) { + private Optional shouldSkipBeforeDownload(Long namespaceId, ManifestItem item) { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); if (hasOtherOwnerConflict(existingSkills)) { log.warn("Skipping built-in skill slug={} before download because the slug already belongs to another user", item.slug()); - return true; + return Optional.of(SyncOutcome.CONFLICT_SKIPPED); } Optional builtinSkill = existingSkills.stream() .filter(skill -> SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())) .findFirst(); if (builtinSkill.isEmpty()) { - return false; + return Optional.empty(); } Optional existingVersion = skillVersionRepository .findBySkillIdAndVersion(builtinSkill.get().getId(), item.version()); if (existingVersion.isEmpty()) { - return false; + return Optional.empty(); } SkillVersion version = existingVersion.get(); @@ -285,35 +308,35 @@ public class BuiltinSkillInitializer { log.info("Skipping built-in skill slug={} version={} before download because existing version status is {}", item.slug(), item.version(), version.getStatus()); } - return true; + return Optional.of(SyncOutcome.IDEMPOTENT_SKIPPED); } - private boolean shouldSkipExisting(Long namespaceId, ManifestItem item, List entries) { + private Optional shouldSkipExisting(Long namespaceId, ManifestItem item, List entries) { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); if (hasOtherOwnerConflict(existingSkills)) { log.warn("Skipping built-in skill slug={} because the slug already belongs to another user", item.slug()); - return true; + return Optional.of(SyncOutcome.CONFLICT_SKIPPED); } Optional builtinSkill = existingSkills.stream() .filter(skill -> SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())) .findFirst(); if (builtinSkill.isEmpty()) { - return false; + return Optional.empty(); } Optional existingVersion = skillVersionRepository .findBySkillIdAndVersion(builtinSkill.get().getId(), item.version()); if (existingVersion.isEmpty()) { - return false; + return Optional.empty(); } SkillVersion version = existingVersion.get(); if (version.getStatus() != SkillVersionStatus.PUBLISHED) { log.info("Skipping built-in skill slug={} version={} because existing version status is {}", item.slug(), item.version(), version.getStatus()); - return true; + return Optional.of(SyncOutcome.IDEMPOTENT_SKIPPED); } String packageFingerprint = computeFingerprint(entries); @@ -321,6 +344,7 @@ public class BuiltinSkillInitializer { if (packageFingerprint.equals(existingFingerprint)) { log.info("Skipping built-in skill slug={} version={} because it is already published", item.slug(), item.version()); + return Optional.of(SyncOutcome.IDEMPOTENT_SKIPPED); } else { log.warn( "Skipping built-in skill slug={} version={} because published fingerprint differs: existing={}, package={}", @@ -329,8 +353,8 @@ public class BuiltinSkillInitializer { existingFingerprint, packageFingerprint ); + return Optional.of(SyncOutcome.CONFLICT_SKIPPED); } - return true; } private boolean isAlreadyPublishedWithSameFingerprint(Long namespaceId, ManifestItem item, List entries) { @@ -404,4 +428,11 @@ public class BuiltinSkillInitializer { private record FileDigest(String path, String sha256) { } + + private enum SyncOutcome { + PUBLISHED, + IDEMPOTENT_SKIPPED, + CONFLICT_SKIPPED, + FAILED + } } 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 c2b5a836..296f547a 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 @@ -196,14 +196,16 @@ class BuiltinSkillInitializerTest { } @Test - void skipsUnpublishedSkillOwnedByAnotherUserBeforeDownloadingPackage() { + void skipsSkillOwnedByAnotherUserAfterDownloadingPackage() throws Exception { Skill otherSkill = skill(100L, "skillhub-hello", "someone-else"); - givenManifestAndSystemPublisher(); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(otherSkill)); + givenExtractedPackage(packageEntries("skillhub-hello", "1.0.0", "same")); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")) + .thenReturn(List.of()) + .thenReturn(List.of(otherSkill)); runInitializer(); - verify(downloader, never()).download(any()); + verify(downloader).download(URI.create(ITEM.url())); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java index fed90cfc..698a0fdd 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java @@ -564,6 +564,13 @@ public class SkillPublishService { throw new DomainBadRequestException("error.skill.version.exists", version.getVersion()); } + // PostgreSQL prevents deleting a skill_version while skill.latest_version_id still references it. + if (version.getId().equals(skill.getLatestVersionId())) { + skill.setLatestVersionId(null); + skillRepository.save(skill); + skillRepository.flush(); + } + reviewTaskRepository.findBySkillVersionIdAndStatus(version.getId(), ReviewTaskStatus.PENDING) .ifPresent(reviewTaskRepository::delete); @@ -579,10 +586,6 @@ public class SkillPublishService { securityScanService.softDeleteByVersionId(version.getId()); skillVersionRepository.delete(version); skillVersionRepository.flush(); - - if (version.getId().equals(skill.getLatestVersionId())) { - skill.setLatestVersionId(null); - } } private String resolveNamespaceSlug(Long namespaceId) { diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java index 77264b23..e40fa545 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java @@ -455,6 +455,7 @@ class SkillPublishServiceTest { Skill skill = new Skill(1L, "test-skill", publisherId, SkillVisibility.PUBLIC); setId(skill, 1L); + skill.setLatestVersionId(8L); SkillVersion draftVersion = new SkillVersion(1L, "1.0.0-beta", publisherId); draftVersion.setStatus(SkillVersionStatus.DRAFT); setId(draftVersion, 8L); @@ -485,10 +486,14 @@ class SkillPublishServiceTest { Set.of() ); - InOrder inOrder = inOrder(skillVersionRepository); + assertNull(skill.getLatestVersionId()); + InOrder inOrder = inOrder(skillRepository, skillVersionRepository); + inOrder.verify(skillRepository).save(skill); + inOrder.verify(skillRepository).flush(); inOrder.verify(skillVersionRepository).delete(draftVersion); inOrder.verify(skillVersionRepository).flush(); inOrder.verify(skillVersionRepository, times(2)).save(any(SkillVersion.class)); + inOrder.verify(skillRepository).save(skill); } @Test From 738e8f8ceeeda61988541339738d538f47e6cbf9 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Wed, 10 Jun 2026 18:00:10 +0800 Subject: [PATCH 10/11] fix(web): stabilize frontend validation Signed-off-by: dongmucat <1127093059@qq.com> --- web/src/pages/dashboard/my-skills.tsx | 8 ++++---- web/vite.config.ts | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/web/src/pages/dashboard/my-skills.tsx b/web/src/pages/dashboard/my-skills.tsx index 0d90f928..4dbe4765 100644 --- a/web/src/pages/dashboard/my-skills.tsx +++ b/web/src/pages/dashboard/my-skills.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react' +import { useCallback, useEffect, useState } from 'react' import { useLocation, useNavigate, useSearch } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { useAuth } from '@/features/auth/use-auth' @@ -64,20 +64,20 @@ export function MySkillsPage() { const [withdrawTarget, setWithdrawTarget] = useState<{ namespace: string; slug: string; name: string; version: string } | null>(null) const [promotionTarget, setPromotionTarget] = useState<{ skillId: number; versionId: number; name: string; version: string } | null>(null) - const updateSearch = (next: Partial, options?: { replace?: boolean }) => { + const updateSearch = useCallback((next: Partial, options?: { replace?: boolean }) => { navigate({ to: '/dashboard/skills', search: (prev) => ({ ...prev, ...next }), replace: options?.replace, }) - } + }, [navigate]) // Push the debounced keyword to the URL (reset page to 0 when search changes) useEffect(() => { if (debouncedKeyword !== keyword) { updateSearch({ q: debouncedKeyword || undefined, page: 0 }, { replace: true }) } - }, [debouncedKeyword]) + }, [debouncedKeyword, keyword, updateSearch]) // Sync keywordInput when navigating back via returnTo useEffect(() => { diff --git a/web/vite.config.ts b/web/vite.config.ts index 631fd1c0..f49b7759 100644 --- a/web/vite.config.ts +++ b/web/vite.config.ts @@ -22,6 +22,8 @@ export default defineConfig({ }, test: { exclude: ['**/node_modules/**', '**/e2e/**'], + testTimeout: 30000, + hookTimeout: 30000, }, server: { port: 3000, From 8045e52f5ed03896f9ea4f33bea54a316c32e73f Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Thu, 11 Jun 2026 11:20:27 +0800 Subject: [PATCH 11/11] feat(bootstrap): add skillhub hello builtin skill Signed-off-by: dongmucat <1127093059@qq.com> --- .../src/main/resources/builtin-skills/manifest.json | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/skillhub-app/src/main/resources/builtin-skills/manifest.json b/server/skillhub-app/src/main/resources/builtin-skills/manifest.json index 2b485e41..8dba7bac 100644 --- a/server/skillhub-app/src/main/resources/builtin-skills/manifest.json +++ b/server/skillhub-app/src/main/resources/builtin-skills/manifest.json @@ -1,3 +1,9 @@ { - "skills": [] + "skills": [ + { + "slug": "skillhub-hello", + "version": "1.0.0", + "url": "https://bjcdn.openstorage.cn/aicontest/2026-06-11/f8a59af3-30d4-4031-80f6-ebff74b05195.zip" + } + ] }