diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/BasicPrePublishValidator.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/BasicPrePublishValidator.java index dbd93c5d..ff27ff4c 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/BasicPrePublishValidator.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/validation/BasicPrePublishValidator.java @@ -17,6 +17,12 @@ import java.util.regex.Pattern; @Component public class BasicPrePublishValidator implements PrePublishValidator { + private static final int MIN_GENERIC_SECRET_LENGTH = 12; + private static final Pattern ASSIGNMENT_WITH_SENSITIVE_KEY = Pattern.compile( + "(?i)(api[_-]?key|access[_-]?key|secret|password|token)\\s*[:=]\\s*" + ); + private static final Pattern IDENTIFIER = Pattern.compile("[A-Za-z_][A-Za-z0-9_]*"); + private static final Pattern BARE_LITERAL = Pattern.compile("[A-Za-z0-9_\\-]{12,}"); private static final Set PLACEHOLDER_MARKERS = Set.of( "your", "example", "sample", "placeholder", "changeme", "replace", "dummy", "mock", "test", "fake", "todo", "xxx", "redacted"); @@ -24,10 +30,7 @@ public class BasicPrePublishValidator implements PrePublishValidator { new SecretRule(Pattern.compile("(AKIA[0-9A-Z]{16})"), 1, "cloud access key"), new SecretRule(Pattern.compile("(ghp_[A-Za-z0-9]{20,})"), 1, "GitHub token"), new SecretRule(Pattern.compile("(sk-[A-Za-z0-9]{20,})"), 1, "API key"), - new SecretRule( - Pattern.compile("(?i)(api[_-]?key|access[_-]?key|secret|password|token)\\s*[:=]\\s*['\\\"]?([A-Za-z0-9_\\-]{12,})"), - 2, - "secret or token") + new SecretRule(ASSIGNMENT_WITH_SENSITIVE_KEY, 0, "secret or token") ); @Override @@ -47,7 +50,11 @@ public class BasicPrePublishValidator implements PrePublishValidator { if (!matcher.find()) { continue; } - String matchedValue = matcher.group(rule.valueGroup()); + String matchedValue = extractMatchedValue( + line, matcher, rule, isBareSecretConfiguration(entry.path())); + if (matchedValue == null) { + continue; + } if (isPlaceholderValue(matchedValue)) { continue; } @@ -80,6 +87,13 @@ public class BasicPrePublishValidator implements PrePublishValidator { || lowerPath.endsWith(".zsh") || lowerPath.endsWith(".bash"); } + private boolean isBareSecretConfiguration(String path) { + String lowerPath = path.toLowerCase(Locale.ROOT); + return lowerPath.endsWith(".yaml") || lowerPath.endsWith(".yml") + || lowerPath.endsWith(".toml") || lowerPath.endsWith(".ini") + || lowerPath.endsWith(".cfg") || lowerPath.endsWith(".env"); + } + private boolean isPlaceholderValue(String value) { if (value == null || value.isBlank()) { return false; @@ -89,5 +103,151 @@ public class BasicPrePublishValidator implements PrePublishValidator { || value.chars().allMatch(ch -> ch == 'x' || ch == 'X' || ch == '*' || ch == '-'); } + private String extractMatchedValue( + String line, Matcher matcher, SecretRule rule, boolean allowBareLiteral) { + if (rule.valueGroup() > 0) { + return matcher.group(rule.valueGroup()); + } + + do { + GenericValueScan scan = scanGenericValue(line, matcher.end(), allowBareLiteral); + if (scan.literal() != null) { + return scan.literal(); + } + if (scan.nextSearchIndex() >= line.length()) { + return null; + } + matcher.region(scan.nextSearchIndex(), line.length()); + } while (matcher.find()); + return null; + } + + private GenericValueScan scanGenericValue(String line, int valueStart, boolean allowBareLiteral) { + int start = valueStart; + while (start < line.length() && Character.isWhitespace(line.charAt(start))) { + start++; + } + if (start == line.length()) { + return new GenericValueScan(null, line.length()); + } + + QuotedLiteralStart quotedStart = findQuotedLiteralStart(line, start); + char first = line.charAt(quotedStart.index()); + if (first == '\'' || first == '"') { + return scanQuotedLiteral(line, quotedStart.index(), first, quotedStart.wrapperDepth()); + } + + int end = start; + while (end < line.length() && !isBareValueTerminator(line, end)) { + end++; + } + String bareValue = line.substring(start, end); + if (!allowBareLiteral && IDENTIFIER.matcher(bareValue).matches()) { + return new GenericValueScan(null, end); + } + String literal = BARE_LITERAL.matcher(bareValue).matches() ? bareValue : null; + return new GenericValueScan(literal, end); + } + + private QuotedLiteralStart findQuotedLiteralStart(String line, int start) { + int index = start; + int wrapperDepth = 0; + while (index < line.length() && line.charAt(index) == '(') { + wrapperDepth++; + index++; + while (index < line.length() && Character.isWhitespace(line.charAt(index))) { + index++; + } + } + return index < line.length() && (line.charAt(index) == '\'' || line.charAt(index) == '"') + ? new QuotedLiteralStart(index, wrapperDepth) + : new QuotedLiteralStart(start, 0); + } + + private GenericValueScan scanQuotedLiteral( + String line, int start, char quote, int wrapperDepth) { + boolean escaped = false; + for (int i = start + 1; i < line.length(); i++) { + char current = line.charAt(i); + if (escaped) { + escaped = false; + continue; + } + if (current == '\\') { + escaped = true; + continue; + } + if (current == quote) { + String value = line.substring(start + 1, i); + String literal = hasLiteralTerminator(line, i + 1, wrapperDepth) + && value.length() >= MIN_GENERIC_SECRET_LENGTH + ? value + : null; + return new GenericValueScan(literal, i + 1); + } + } + return new GenericValueScan(null, line.length()); + } + + private boolean hasLiteralTerminator(String line, int startIndex, int wrapperDepth) { + int index = skipWhitespace(line, startIndex); + for (int i = 0; i < wrapperDepth; i++) { + if (index == line.length() || line.charAt(index) != ')') { + return false; + } + index = skipWhitespace(line, index + 1); + } + if (isLiteralTerminatorAt(line, index)) { + return true; + } + + if (!line.startsWith("as", index) + || index + 2 >= line.length() + || !Character.isWhitespace(line.charAt(index + 2))) { + return false; + } + index = skipWhitespace(line, index + 2); + if (!line.startsWith("const", index) + || (index + 5 < line.length() + && Character.isJavaIdentifierPart(line.charAt(index + 5)))) { + return false; + } + return isLiteralTerminatorAt(line, skipWhitespace(line, index + 5)); + } + + private int skipWhitespace(String line, int startIndex) { + int index = startIndex; + while (index < line.length() && Character.isWhitespace(line.charAt(index))) { + index++; + } + return index; + } + + private boolean isLiteralTerminatorAt(String line, int index) { + if (index == line.length()) { + return true; + } + char current = line.charAt(index); + return isTrailingDelimiter(current) + || current == '#' + || (current == '/' && index + 1 < line.length() && line.charAt(index + 1) == '/'); + } + + private boolean isTrailingDelimiter(char value) { + return value == ',' || value == ';' || value == ')' || value == '}' || value == ']'; + } + + private boolean isBareValueTerminator(String line, int index) { + char current = line.charAt(index); + return Character.isWhitespace(current) + || isTrailingDelimiter(current) + || current == '#' + || (current == '/' && index + 1 < line.length() && line.charAt(index + 1) == '/'); + } + + private record GenericValueScan(String literal, int nextSearchIndex) {} + + private record QuotedLiteralStart(int index, int wrapperDepth) {} + private record SecretRule(Pattern pattern, int valueGroup, String label) {} } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/BasicPrePublishValidatorTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/BasicPrePublishValidatorTest.java index f428559c..fbad8464 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/BasicPrePublishValidatorTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/validation/BasicPrePublishValidatorTest.java @@ -7,6 +7,7 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -98,4 +99,134 @@ class BasicPrePublishValidatorTest { assertTrue(result.passed()); } + + @Test + void shouldNotWarnOnRuntimeExpressionsAssignedToSensitiveVariables() { + PackageEntry script = new PackageEntry( + "scripts/oauth.py", + """ + refresh_token = token_response.get("refresh_token") + client_secret = configured_secret + self._client_secret = credentials.client_secret + access_token = ensure_valid_access_token(session) + headers = build_headers(access_token=access_token) + client_secret = "prefix-" + configured_secret + access_token = token_v2 + access_token = configuredToken123 + refresh_token = foo123bar456 + token = ("static-prefix-") + configuredToken + access_token = ("static_prefix_") + configured_token + """.getBytes(StandardCharsets.UTF_8), + 476, + "text/x-python" + ); + + ValidationResult result = validator.validate(new PrePublishValidator.SkillPackageContext( + List.of(script), + new SkillMetadata("OAuth Skill", "desc", "1.0.0", "body", Map.of()), + "user-1", + 1L + )); + + assertTrue(result.passed()); + assertTrue(result.warnings().isEmpty()); + } + + @Test + void shouldKeepWarningOnHardcodedAndProviderSpecificCredentials() { + PackageEntry script = new PackageEntry( + "scripts/leaked.js", + """ + client_secret = "literalcredential123" + github_token = "ghp_abcdefghijklmnopqrstuvwxyz1234" + const token = "javascriptcredential123"; + const config = { token: "objectcredential123", }; + const options = { token: "multipropertycredential123", endpoint: "/api" }; + const escaped = { token: "credential\\\"value123" }; + const emptyFirst = { token: "", password: "passwordafterempty123" }; + const dynamicFirst = { token: configuredToken, password: "passwordafterdynamic123" }; + token=("wrappedcredential123"); + token="assertedcredential123" as const; + """.getBytes(StandardCharsets.UTF_8), + 568, + "text/javascript" + ); + PackageEntry configuration = new PackageEntry( + "config/settings.env", + "token=barecredential123 // leaked\n".getBytes(StandardCharsets.UTF_8), + 35, + "text/plain" + ); + + ValidationResult result = validator.validate(new PrePublishValidator.SkillPackageContext( + List.of(script, configuration), + new SkillMetadata("Unsafe Skill", "desc", "1.0.0", "body", Map.of()), + "user-1", + 1L + )); + + assertTrue(result.passed()); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 1"))); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 2"))); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 3"))); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 4"))); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 5"))); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 6"))); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 7"))); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 8"))); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 9"))); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 10"))); + assertTrue(result.warnings().stream().anyMatch(warning -> + warning.contains("config/settings.env line 1"))); + } + + @Test + void shouldNotWarnOnEmptyOrShortSensitiveLiterals() { + PackageEntry script = new PackageEntry( + "scripts/defaults.py", + """ + token = "" + client_secret = "short" + password = 'unset' + """.getBytes(StandardCharsets.UTF_8), + 58, + "text/x-python" + ); + + ValidationResult result = validator.validate(new PrePublishValidator.SkillPackageContext( + List.of(script), + new SkillMetadata("Defaults Skill", "desc", "1.0.0", "body", Map.of()), + "user-1", + 1L + )); + + assertTrue(result.passed()); + assertTrue(result.warnings().isEmpty()); + } + + @Test + void shouldScanDeeplyNestedSingleLineObjectWithoutOverflowingRegexStack() { + String content = "const config = " + + "{ nested: ".repeat(5_000) + + "{ token: \"literalcredential123\"" + + " }".repeat(5_001) + + ";"; + PackageEntry script = new PackageEntry( + "scripts/deeply-nested.js", + content.getBytes(StandardCharsets.UTF_8), + content.length(), + "text/javascript" + ); + PrePublishValidator.SkillPackageContext context = new PrePublishValidator.SkillPackageContext( + List.of(script), + new SkillMetadata("Deeply Nested Skill", "desc", "1.0.0", "body", Map.of()), + "user-1", + 1L + ); + + ValidationResult result = assertDoesNotThrow(() -> validator.validate(context)); + + assertTrue(result.passed()); + assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 1"))); + } }