From 824a992afcbe14c11cfbc4dc0098f62243b57946 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:17:22 +0800 Subject: [PATCH 1/6] fix(validation): ignore credential expressions Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../validation/BasicPrePublishValidator.java | 77 +++++++++++++++++-- .../BasicPrePublishValidatorTest.java | 54 +++++++++++++ 2 files changed, 126 insertions(+), 5 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 dbd93c5d..c4012791 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,14 @@ import java.util.regex.Pattern; @Component public class BasicPrePublishValidator implements PrePublishValidator { + private static final Pattern ASSIGNMENT_WITH_SENSITIVE_KEY = Pattern.compile( + "(?i)(api[_-]?key|access[_-]?key|secret|password|token)\\s*[:=]\\s*(.+)$" + ); + private static final Pattern QUOTED_LITERAL = Pattern.compile( + "^(['\"])(.*)\\1(?:\\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 +32,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 +52,10 @@ public class BasicPrePublishValidator implements PrePublishValidator { if (!matcher.find()) { continue; } - String matchedValue = matcher.group(rule.valueGroup()); + String matchedValue = extractMatchedValue(line, matcher, rule); + if (matchedValue == null) { + continue; + } if (isPlaceholderValue(matchedValue)) { continue; } @@ -89,5 +97,64 @@ public class BasicPrePublishValidator implements PrePublishValidator { || value.chars().allMatch(ch -> ch == 'x' || ch == 'X' || ch == '*' || ch == '-'); } + private String extractMatchedValue(String line, Matcher matcher, SecretRule rule) { + if (rule.valueGroup() > 0) { + return matcher.group(rule.valueGroup()); + } + + Matcher assignmentMatcher = ASSIGNMENT_WITH_SENSITIVE_KEY.matcher(line); + if (!assignmentMatcher.find()) { + return null; + } + + String rawValue = assignmentMatcher.group(2).trim(); + if (rawValue.isBlank()) { + return null; + } + + Matcher quotedLiteralMatcher = QUOTED_LITERAL.matcher(rawValue); + if (quotedLiteralMatcher.matches()) { + return quotedLiteralMatcher.group(2); + } + + rawValue = stripInlineComment(rawValue); + if (rawValue.isBlank()) { + return null; + } + + quotedLiteralMatcher = QUOTED_LITERAL.matcher(rawValue); + if (quotedLiteralMatcher.matches()) { + return quotedLiteralMatcher.group(2); + } + + if (looksLikeExpression(rawValue) || IDENTIFIER.matcher(rawValue).matches()) { + return null; + } + + return BARE_LITERAL.matcher(rawValue).matches() ? rawValue : null; + } + + private String stripInlineComment(String rawValue) { + int hashIndex = rawValue.indexOf('#'); + if (hashIndex >= 0) { + return rawValue.substring(0, hashIndex).trim(); + } + return rawValue; + } + + private boolean looksLikeExpression(String rawValue) { + return rawValue.contains("(") + || rawValue.contains(")") + || rawValue.contains(".") + || rawValue.contains("[") + || rawValue.contains("]") + || rawValue.contains("{") + || rawValue.contains("}") + || rawValue.contains(",") + || rawValue.contains(" ") + || rawValue.contains("+") + || rawValue.contains("/"); + } + 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..e7f5d456 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 @@ -98,4 +98,58 @@ 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) + """.getBytes(StandardCharsets.UTF_8), + 229, + "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", }; + """.getBytes(StandardCharsets.UTF_8), + 184, + "text/javascript" + ); + + ValidationResult result = validator.validate(new PrePublishValidator.SkillPackageContext( + List.of(script), + 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"))); + } } From 7c62aa218adf8b309435a17637f2af34a21eb172 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:34:51 +0800 Subject: [PATCH 2/6] fix(validation): avoid regex stack overflow Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../validation/BasicPrePublishValidator.java | 61 ++++++++++++++++--- .../BasicPrePublishValidatorTest.java | 30 ++++++++- 2 files changed, 81 insertions(+), 10 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 c4012791..8e8a914a 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 @@ -20,9 +20,6 @@ public class BasicPrePublishValidator implements PrePublishValidator { private static final Pattern ASSIGNMENT_WITH_SENSITIVE_KEY = Pattern.compile( "(?i)(api[_-]?key|access[_-]?key|secret|password|token)\\s*[:=]\\s*(.+)$" ); - private static final Pattern QUOTED_LITERAL = Pattern.compile( - "^(['\"])(.*)\\1(?:\\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( @@ -112,9 +109,9 @@ public class BasicPrePublishValidator implements PrePublishValidator { return null; } - Matcher quotedLiteralMatcher = QUOTED_LITERAL.matcher(rawValue); - if (quotedLiteralMatcher.matches()) { - return quotedLiteralMatcher.group(2); + String quotedLiteral = extractQuotedLiteral(rawValue); + if (quotedLiteral != null) { + return quotedLiteral; } rawValue = stripInlineComment(rawValue); @@ -122,9 +119,9 @@ public class BasicPrePublishValidator implements PrePublishValidator { return null; } - quotedLiteralMatcher = QUOTED_LITERAL.matcher(rawValue); - if (quotedLiteralMatcher.matches()) { - return quotedLiteralMatcher.group(2); + quotedLiteral = extractQuotedLiteral(rawValue); + if (quotedLiteral != null) { + return quotedLiteral; } if (looksLikeExpression(rawValue) || IDENTIFIER.matcher(rawValue).matches()) { @@ -134,6 +131,52 @@ public class BasicPrePublishValidator implements PrePublishValidator { return BARE_LITERAL.matcher(rawValue).matches() ? rawValue : null; } + private String extractQuotedLiteral(String rawValue) { + if (rawValue.length() < 2) { + return null; + } + + char quote = rawValue.charAt(0); + if (quote != '\'' && quote != '"') { + return null; + } + + boolean escaped = false; + for (int i = 1; i < rawValue.length(); i++) { + char current = rawValue.charAt(i); + if (escaped) { + escaped = false; + continue; + } + if (current == '\\') { + escaped = true; + continue; + } + if (current == quote) { + return hasOnlyTrailingSyntax(rawValue, i + 1) ? rawValue.substring(1, i) : null; + } + } + return null; + } + + private boolean hasOnlyTrailingSyntax(String rawValue, int startIndex) { + for (int i = startIndex; i < rawValue.length(); i++) { + char current = rawValue.charAt(i); + if (Character.isWhitespace(current) || isTrailingDelimiter(current)) { + continue; + } + if (current == '#') { + return true; + } + return current == '/' && i + 1 < rawValue.length() && rawValue.charAt(i + 1) == '/'; + } + return true; + } + + private boolean isTrailingDelimiter(char value) { + return value == ',' || value == ';' || value == ')' || value == '}' || value == ']'; + } + private String stripInlineComment(String rawValue) { int hashIndex = rawValue.indexOf('#'); if (hashIndex >= 0) { 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 e7f5d456..0cca83f6 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; @@ -109,8 +110,9 @@ class BasicPrePublishValidatorTest { 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 """.getBytes(StandardCharsets.UTF_8), - 229, + 275, "text/x-python" ); @@ -152,4 +154,30 @@ class BasicPrePublishValidatorTest { assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 3"))); assertTrue(result.warnings().stream().anyMatch(warning -> warning.contains("line 4"))); } + + @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"))); + } } From 9d0431f7d3625c3400d87972fcd02295886c4219 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:45:48 +0800 Subject: [PATCH 3/6] fix(validation): preserve credential literal boundaries Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../validation/BasicPrePublishValidator.java | 30 ++++++++++--------- .../BasicPrePublishValidatorTest.java | 30 ++++++++++++++++++- 2 files changed, 45 insertions(+), 15 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 8e8a914a..b31cdc07 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,7 @@ 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*(.+)$" ); @@ -111,7 +112,7 @@ public class BasicPrePublishValidator implements PrePublishValidator { String quotedLiteral = extractQuotedLiteral(rawValue); if (quotedLiteral != null) { - return quotedLiteral; + return quotedLiteral.length() >= MIN_GENERIC_SECRET_LENGTH ? quotedLiteral : null; } rawValue = stripInlineComment(rawValue); @@ -121,7 +122,7 @@ public class BasicPrePublishValidator implements PrePublishValidator { quotedLiteral = extractQuotedLiteral(rawValue); if (quotedLiteral != null) { - return quotedLiteral; + return quotedLiteral.length() >= MIN_GENERIC_SECRET_LENGTH ? quotedLiteral : null; } if (looksLikeExpression(rawValue) || IDENTIFIER.matcher(rawValue).matches()) { @@ -153,24 +154,25 @@ public class BasicPrePublishValidator implements PrePublishValidator { continue; } if (current == quote) { - return hasOnlyTrailingSyntax(rawValue, i + 1) ? rawValue.substring(1, i) : null; + return hasLiteralTerminator(rawValue, i + 1) ? rawValue.substring(1, i) : null; } } return null; } - private boolean hasOnlyTrailingSyntax(String rawValue, int startIndex) { - for (int i = startIndex; i < rawValue.length(); i++) { - char current = rawValue.charAt(i); - if (Character.isWhitespace(current) || isTrailingDelimiter(current)) { - continue; - } - if (current == '#') { - return true; - } - return current == '/' && i + 1 < rawValue.length() && rawValue.charAt(i + 1) == '/'; + private boolean hasLiteralTerminator(String rawValue, int startIndex) { + int index = startIndex; + while (index < rawValue.length() && Character.isWhitespace(rawValue.charAt(index))) { + index++; } - return true; + if (index == rawValue.length()) { + return true; + } + + char current = rawValue.charAt(index); + return isTrailingDelimiter(current) + || current == '#' + || (current == '/' && index + 1 < rawValue.length() && rawValue.charAt(index + 1) == '/'); } private boolean isTrailingDelimiter(char value) { 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 0cca83f6..5610c8a8 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 @@ -136,8 +136,10 @@ class BasicPrePublishValidatorTest { github_token = "ghp_abcdefghijklmnopqrstuvwxyz1234" const token = "javascriptcredential123"; const config = { token: "objectcredential123", }; + const options = { token: "multipropertycredential123", endpoint: "/api" }; + const escaped = { token: "credential\\\"value123" }; """.getBytes(StandardCharsets.UTF_8), - 184, + 319, "text/javascript" ); @@ -153,6 +155,32 @@ class BasicPrePublishValidatorTest { 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"))); + } + + @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 From 8f9db2ada7ca958a6c213d602cdf2bc858eee1e8 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:03:56 +0800 Subject: [PATCH 4/6] fix(validation): scan all sensitive assignments Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../validation/BasicPrePublishValidator.java | 122 ++++++++---------- .../BasicPrePublishValidatorTest.java | 11 +- 2 files changed, 66 insertions(+), 67 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 b31cdc07..863f3b7f 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 @@ -19,7 +19,7 @@ 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*(.+)$" + "(?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,}"); @@ -100,51 +100,50 @@ public class BasicPrePublishValidator implements PrePublishValidator { return matcher.group(rule.valueGroup()); } - Matcher assignmentMatcher = ASSIGNMENT_WITH_SENSITIVE_KEY.matcher(line); - if (!assignmentMatcher.find()) { - return null; - } - - String rawValue = assignmentMatcher.group(2).trim(); - if (rawValue.isBlank()) { - return null; - } - - String quotedLiteral = extractQuotedLiteral(rawValue); - if (quotedLiteral != null) { - return quotedLiteral.length() >= MIN_GENERIC_SECRET_LENGTH ? quotedLiteral : null; - } - - rawValue = stripInlineComment(rawValue); - if (rawValue.isBlank()) { - return null; - } - - quotedLiteral = extractQuotedLiteral(rawValue); - if (quotedLiteral != null) { - return quotedLiteral.length() >= MIN_GENERIC_SECRET_LENGTH ? quotedLiteral : null; - } - - if (looksLikeExpression(rawValue) || IDENTIFIER.matcher(rawValue).matches()) { - return null; - } - - return BARE_LITERAL.matcher(rawValue).matches() ? rawValue : null; + do { + GenericValueScan scan = scanGenericValue(line, matcher.end()); + 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 String extractQuotedLiteral(String rawValue) { - if (rawValue.length() < 2) { - return null; + private GenericValueScan scanGenericValue(String line, int valueStart) { + int start = valueStart; + while (start < line.length() && Character.isWhitespace(line.charAt(start))) { + start++; + } + if (start == line.length()) { + return new GenericValueScan(null, line.length()); } - char quote = rawValue.charAt(0); - if (quote != '\'' && quote != '"') { - return null; + char first = line.charAt(start); + if (first == '\'' || first == '"') { + return scanQuotedLiteral(line, start, first); } + int end = start; + while (end < line.length() && !isBareValueTerminator(line, end)) { + end++; + } + String bareValue = line.substring(start, end); + if (IDENTIFIER.matcher(bareValue).matches() + && bareValue.chars().filter(Character::isDigit).count() < 3) { + return new GenericValueScan(null, end); + } + String literal = BARE_LITERAL.matcher(bareValue).matches() ? bareValue : null; + return new GenericValueScan(literal, end); + } + + private GenericValueScan scanQuotedLiteral(String line, int start, char quote) { boolean escaped = false; - for (int i = 1; i < rawValue.length(); i++) { - char current = rawValue.charAt(i); + for (int i = start + 1; i < line.length(); i++) { + char current = line.charAt(i); if (escaped) { escaped = false; continue; @@ -154,52 +153,45 @@ public class BasicPrePublishValidator implements PrePublishValidator { continue; } if (current == quote) { - return hasLiteralTerminator(rawValue, i + 1) ? rawValue.substring(1, i) : null; + String value = line.substring(start + 1, i); + String literal = hasLiteralTerminator(line, i + 1) + && value.length() >= MIN_GENERIC_SECRET_LENGTH + ? value + : null; + return new GenericValueScan(literal, i + 1); } } - return null; + return new GenericValueScan(null, line.length()); } - private boolean hasLiteralTerminator(String rawValue, int startIndex) { + private boolean hasLiteralTerminator(String line, int startIndex) { int index = startIndex; - while (index < rawValue.length() && Character.isWhitespace(rawValue.charAt(index))) { + while (index < line.length() && Character.isWhitespace(line.charAt(index))) { index++; } - if (index == rawValue.length()) { + if (index == line.length()) { return true; } - char current = rawValue.charAt(index); + char current = line.charAt(index); return isTrailingDelimiter(current) || current == '#' - || (current == '/' && index + 1 < rawValue.length() && rawValue.charAt(index + 1) == '/'); + || (current == '/' && index + 1 < line.length() && line.charAt(index + 1) == '/'); } private boolean isTrailingDelimiter(char value) { return value == ',' || value == ';' || value == ')' || value == '}' || value == ']'; } - private String stripInlineComment(String rawValue) { - int hashIndex = rawValue.indexOf('#'); - if (hashIndex >= 0) { - return rawValue.substring(0, hashIndex).trim(); - } - return rawValue; + 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 boolean looksLikeExpression(String rawValue) { - return rawValue.contains("(") - || rawValue.contains(")") - || rawValue.contains(".") - || rawValue.contains("[") - || rawValue.contains("]") - || rawValue.contains("{") - || rawValue.contains("}") - || rawValue.contains(",") - || rawValue.contains(" ") - || rawValue.contains("+") - || rawValue.contains("/"); - } + private record GenericValueScan(String literal, int nextSearchIndex) {} 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 5610c8a8..cb3c0444 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 @@ -111,8 +111,9 @@ class BasicPrePublishValidatorTest { access_token = ensure_valid_access_token(session) headers = build_headers(access_token=access_token) client_secret = "prefix-" + configured_secret + access_token = token_v2 """.getBytes(StandardCharsets.UTF_8), - 275, + 299, "text/x-python" ); @@ -138,8 +139,11 @@ class BasicPrePublishValidatorTest { 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=barecredential123 // leaked """.getBytes(StandardCharsets.UTF_8), - 319, + 518, "text/javascript" ); @@ -157,6 +161,9 @@ class BasicPrePublishValidatorTest { 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"))); } @Test 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 5/6] 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 From a6aa073627095d22701d279c32052b002e275bea Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:25:29 +0800 Subject: [PATCH 6/6] fix(validation): preserve wrapper expression boundaries Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../validation/BasicPrePublishValidator.java | 29 +++++++++++++------ .../BasicPrePublishValidatorTest.java | 4 ++- 2 files changed, 23 insertions(+), 10 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 c65e8908..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 @@ -131,10 +131,10 @@ public class BasicPrePublishValidator implements PrePublishValidator { return new GenericValueScan(null, line.length()); } - int quotedStart = findQuotedLiteralStart(line, start); - char first = line.charAt(quotedStart); + QuotedLiteralStart quotedStart = findQuotedLiteralStart(line, start); + char first = line.charAt(quotedStart.index()); if (first == '\'' || first == '"') { - return scanQuotedLiteral(line, quotedStart, first); + return scanQuotedLiteral(line, quotedStart.index(), first, quotedStart.wrapperDepth()); } int end = start; @@ -149,20 +149,23 @@ public class BasicPrePublishValidator implements PrePublishValidator { return new GenericValueScan(literal, end); } - private int findQuotedLiteralStart(String line, int start) { + 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) == '"') - ? index - : start; + ? new QuotedLiteralStart(index, wrapperDepth) + : new QuotedLiteralStart(start, 0); } - private GenericValueScan scanQuotedLiteral(String line, int start, char quote) { + 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); @@ -176,7 +179,7 @@ public class BasicPrePublishValidator implements PrePublishValidator { } if (current == quote) { String value = line.substring(start + 1, i); - String literal = hasLiteralTerminator(line, i + 1) + String literal = hasLiteralTerminator(line, i + 1, wrapperDepth) && value.length() >= MIN_GENERIC_SECRET_LENGTH ? value : null; @@ -186,8 +189,14 @@ public class BasicPrePublishValidator implements PrePublishValidator { return new GenericValueScan(null, line.length()); } - private boolean hasLiteralTerminator(String line, int startIndex) { + 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; } @@ -238,5 +247,7 @@ public class BasicPrePublishValidator implements PrePublishValidator { 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 1e19c4c5..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 @@ -114,8 +114,10 @@ class BasicPrePublishValidatorTest { access_token = token_v2 access_token = configuredToken123 refresh_token = foo123bar456 + token = ("static-prefix-") + configuredToken + access_token = ("static_prefix_") + configured_token """.getBytes(StandardCharsets.UTF_8), - 371, + 476, "text/x-python" );