mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-24 00:55:35 +00:00
fix(validation): ignore credential expressions
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
parent
19cc56be9e
commit
824a992afc
2 changed files with 126 additions and 5 deletions
|
|
@ -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<String> 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) {}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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")));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue