From 52969c997c5dc3530d64ee1fcb79ba11653d9d04 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:17:03 +0800 Subject: [PATCH] fix(validation): classify bare secrets by file context Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../validation/BasicPrePublishValidator.java | 63 ++++++++++++++++--- .../BasicPrePublishValidatorTest.java | 20 ++++-- 2 files changed, 70 insertions(+), 13 deletions(-) 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 863f3b7f..c65e8908 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 @@ -50,7 +50,8 @@ public class BasicPrePublishValidator implements PrePublishValidator { if (!matcher.find()) { continue; } - String matchedValue = extractMatchedValue(line, matcher, rule); + String matchedValue = extractMatchedValue( + line, matcher, rule, isBareSecretConfiguration(entry.path())); if (matchedValue == null) { continue; } @@ -86,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; @@ -95,13 +103,14 @@ public class BasicPrePublishValidator implements PrePublishValidator { || value.chars().allMatch(ch -> ch == 'x' || ch == 'X' || ch == '*' || ch == '-'); } - private String extractMatchedValue(String line, Matcher matcher, SecretRule rule) { + 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()); + GenericValueScan scan = scanGenericValue(line, matcher.end(), allowBareLiteral); if (scan.literal() != null) { return scan.literal(); } @@ -113,7 +122,7 @@ public class BasicPrePublishValidator implements PrePublishValidator { return null; } - private GenericValueScan scanGenericValue(String line, int valueStart) { + private GenericValueScan scanGenericValue(String line, int valueStart, boolean allowBareLiteral) { int start = valueStart; while (start < line.length() && Character.isWhitespace(line.charAt(start))) { start++; @@ -122,9 +131,10 @@ public class BasicPrePublishValidator implements PrePublishValidator { return new GenericValueScan(null, line.length()); } - char first = line.charAt(start); + int quotedStart = findQuotedLiteralStart(line, start); + char first = line.charAt(quotedStart); if (first == '\'' || first == '"') { - return scanQuotedLiteral(line, start, first); + return scanQuotedLiteral(line, quotedStart, first); } int end = start; @@ -132,14 +142,26 @@ public class BasicPrePublishValidator implements PrePublishValidator { end++; } String bareValue = line.substring(start, end); - if (IDENTIFIER.matcher(bareValue).matches() - && bareValue.chars().filter(Character::isDigit).count() < 3) { + 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 int findQuotedLiteralStart(String line, int start) { + int index = start; + while (index < line.length() && line.charAt(index) == '(') { + index++; + while (index < line.length() && Character.isWhitespace(line.charAt(index))) { + index++; + } + } + return index < line.length() && (line.charAt(index) == '\'' || line.charAt(index) == '"') + ? index + : start; + } + private GenericValueScan scanQuotedLiteral(String line, int start, char quote) { boolean escaped = false; for (int i = start + 1; i < line.length(); i++) { @@ -165,14 +187,37 @@ public class BasicPrePublishValidator implements PrePublishValidator { } private boolean hasLiteralTerminator(String line, int startIndex) { + int index = skipWhitespace(line, startIndex); + 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 == '#' 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 cb3c0444..1e19c4c5 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 @@ -112,8 +112,10 @@ class BasicPrePublishValidatorTest { headers = build_headers(access_token=access_token) client_secret = "prefix-" + configured_secret access_token = token_v2 + access_token = configuredToken123 + refresh_token = foo123bar456 """.getBytes(StandardCharsets.UTF_8), - 299, + 371, "text/x-python" ); @@ -141,14 +143,21 @@ class BasicPrePublishValidatorTest { const escaped = { token: "credential\\\"value123" }; const emptyFirst = { token: "", password: "passwordafterempty123" }; const dynamicFirst = { token: configuredToken, password: "passwordafterdynamic123" }; - token=barecredential123 // leaked + token=("wrappedcredential123"); + token="assertedcredential123" as const; """.getBytes(StandardCharsets.UTF_8), - 518, + 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), + List.of(script, configuration), new SkillMetadata("Unsafe Skill", "desc", "1.0.0", "body", Map.of()), "user-1", 1L @@ -164,6 +173,9 @@ class BasicPrePublishValidatorTest { 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