From eb2db152fd56f695ee82924edba70ccea01c5bdb Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Mon, 29 Jun 2026 09:49:27 +0800 Subject: [PATCH] fix(ISSUE-64): restrict compliance evidenceUrl schemes Signed-off-by: dongmucat <1127093059@qq.com> --- .../SkillComplianceMetadataService.java | 15 ++++++---- .../SkillComplianceMetadataServiceTest.java | 29 +++++++++++++++++++ .../validation/SkillPackageValidatorTest.java | 28 ++++++++++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/metadata/SkillComplianceMetadataService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/metadata/SkillComplianceMetadataService.java index f15573d0..bbfa0177 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/metadata/SkillComplianceMetadataService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/metadata/SkillComplianceMetadataService.java @@ -5,12 +5,12 @@ import com.fasterxml.jackson.databind.ObjectMapper; import java.net.URI; import java.util.ArrayList; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.HashSet; import java.util.regex.Pattern; /** @@ -23,6 +23,8 @@ public class SkillComplianceMetadataService { private static final int MAX_STANDARD_VERSION_LENGTH = 32; private static final int MAX_CONTROL_TITLE_LENGTH = 200; private static final Pattern CONTROL_ID_PATTERN = Pattern.compile("^[A-Za-z0-9][A-Za-z0-9._:/-]{0,127}$"); + private static final Set ALLOWED_EVIDENCE_URL_SCHEMES = Set.of("http", "https"); + private static final String EVIDENCE_URL_ERROR = "evidenceUrl must use an http or https URI"; private static final Set ALLOWED_KEYS = Set.of( "standard", "standardVersion", @@ -221,18 +223,21 @@ public class SkillComplianceMetadataService { return null; } if (!(rawValue instanceof String value) || value.isBlank()) { - errors.add("x-astron-compliance[" + index + "].evidenceUrl must be an absolute URI"); + errors.add("x-astron-compliance[" + index + "]." + EVIDENCE_URL_ERROR); return null; } try { URI uri = URI.create(value.trim()); - if (!uri.isAbsolute()) { - errors.add("x-astron-compliance[" + index + "].evidenceUrl must be an absolute URI"); + String scheme = uri.getScheme(); + if (!uri.isAbsolute() + || scheme == null + || !ALLOWED_EVIDENCE_URL_SCHEMES.contains(scheme.toLowerCase(Locale.ROOT))) { + errors.add("x-astron-compliance[" + index + "]." + EVIDENCE_URL_ERROR); return null; } return uri.toString(); } catch (IllegalArgumentException ex) { - errors.add("x-astron-compliance[" + index + "].evidenceUrl must be an absolute URI"); + errors.add("x-astron-compliance[" + index + "]." + EVIDENCE_URL_ERROR); return null; } } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/metadata/SkillComplianceMetadataServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/metadata/SkillComplianceMetadataServiceTest.java index 4c76578f..bcec9bac 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/metadata/SkillComplianceMetadataServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/metadata/SkillComplianceMetadataServiceTest.java @@ -84,6 +84,35 @@ class SkillComplianceMetadataServiceTest { assertThat(result.errors()).contains("x-astron-compliance must be a non-empty array"); } + @Test + void parseFrontmatter_rejectsUnsafeEvidenceUrlSchemes() { + Map frontmatter = Map.of( + "x-astron-compliance", + List.of( + Map.of( + "standard", "gdpr", + "standardVersion", "2024", + "controlId", "Article-17", + "evidenceUrl", "javascript:alert(1)" + ), + Map.of( + "standard", "soc2", + "standardVersion", "2017", + "controlId", "CC6.1", + "evidenceUrl", "data:text/html;base64,SGk=" + ) + ) + ); + + SkillComplianceMetadataService.ParseResult result = service.parseFrontmatter(frontmatter); + + assertThat(result.mappings()).isEmpty(); + assertThat(result.errors()).containsExactlyInAnyOrder( + "x-astron-compliance[0].evidenceUrl must use an http or https URI", + "x-astron-compliance[1].evidenceUrl must use an http or https URI" + ); + } + @Test void readFromParsedMetadataJson_extractsMappingsFromStoredSkillMetadata() { String parsedMetadataJson = """ diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java index a97787b5..eda0246b 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/SkillPackageValidatorTest.java @@ -213,6 +213,34 @@ class SkillPackageValidatorTest { && error.contains("x-astron-compliance must be a non-empty array"))); } + @Test + void testUnsafeComplianceEvidenceUrlRejected() { + String skillMdContent = """ + --- + name: compliance-skill + description: Skill with unsafe evidence url + version: 1.0.0 + x-astron-compliance: + - standard: gdpr + standardVersion: "2024" + controlId: Article-17 + evidenceUrl: "javascript:alert(1)" + --- + Body + """; + + List entries = List.of( + new PackageEntry("SKILL.md", skillMdContent.getBytes(), skillMdContent.length(), "text/markdown") + ); + + ValidationResult result = validator.validate(entries); + + assertFalse(result.passed()); + assertTrue(result.errors().stream().anyMatch(error -> + error.contains("Invalid SKILL.md frontmatter") + && error.contains("evidenceUrl must use an http or https URI"))); + } + @Test void testPackageTooLarge() { // Use a custom validator with 2KB total limit to test the logic