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"))); + } }