mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 03:13:41 +08:00
feat(skill): broaden script security scan + GBK zip-import fallback
This commit is contained in:
matevip
parent
4e01c98fc1
commit
5cd6e841a4
@ -5,8 +5,11 @@ import org.springframework.web.multipart.MultipartFile;
|
||||
import vip.mate.skill.installer.model.SkillBundle;
|
||||
import vip.mate.skill.runtime.SkillFrontmatterParser;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.nio.charset.CharacterCodingException;
|
||||
import java.nio.charset.Charset;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.nio.file.Path;
|
||||
import java.util.ArrayList;
|
||||
@ -135,12 +138,55 @@ public class ZipSkillFetcher {
|
||||
* instead of being silently dropped.
|
||||
*/
|
||||
public static ExtractedSkill extract(InputStream zipStream) throws IOException {
|
||||
return extract(zipStream.readAllBytes());
|
||||
}
|
||||
|
||||
/** Fallback charset for archives authored on Chinese Windows (entry names / content in GBK). */
|
||||
private static final Charset GBK = Charset.isSupported("GBK") ? Charset.forName("GBK") : null;
|
||||
|
||||
/**
|
||||
* Decompress raw ZIP bytes, trying UTF-8 first and falling back to GBK when
|
||||
* an entry name fails to decode as UTF-8 — the common failure mode for zips
|
||||
* packaged on Chinese Windows, where filenames are GBK and UTF-8 decoding
|
||||
* throws a {@link CharacterCodingException}. Buffering the bytes (rather than
|
||||
* a one-shot stream) is what makes the retry possible.
|
||||
*/
|
||||
public static ExtractedSkill extract(byte[] zipBytes) throws IOException {
|
||||
try {
|
||||
return extract(zipBytes, StandardCharsets.UTF_8);
|
||||
} catch (IOException | RuntimeException e) {
|
||||
if (GBK != null && isCharsetError(e)) {
|
||||
log.warn("[ZipSkillFetcher] UTF-8 entry decode failed, retrying with GBK (Windows-authored archive?)");
|
||||
return extract(zipBytes, GBK);
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
/** True if {@code t} (or any cause) is a charset-decode failure, vs a genuine "no SKILL.md" error. */
|
||||
private static boolean isCharsetError(Throwable t) {
|
||||
for (Throwable c = t; c != null; c = c.getCause()) {
|
||||
if (c instanceof CharacterCodingException) {
|
||||
return true;
|
||||
}
|
||||
String m = c.getMessage();
|
||||
if (m != null && m.toLowerCase().contains("malformed")) {
|
||||
return true;
|
||||
}
|
||||
if (c.getCause() == c) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private static ExtractedSkill extract(byte[] zipBytes, Charset charset) throws IOException {
|
||||
List<RawEntry> raws = new ArrayList<>();
|
||||
String skillMdContent = null;
|
||||
String skillMdPrefix = "";
|
||||
long totalSize = 0;
|
||||
|
||||
try (ZipInputStream zis = new ZipInputStream(zipStream, StandardCharsets.UTF_8)) {
|
||||
try (ZipInputStream zis = new ZipInputStream(new ByteArrayInputStream(zipBytes), charset)) {
|
||||
ZipEntry entry;
|
||||
while ((entry = zis.getNextEntry()) != null) {
|
||||
if (entry.isDirectory()) {
|
||||
@ -150,6 +196,13 @@ public class ZipSkillFetcher {
|
||||
|
||||
String entryName = entry.getName();
|
||||
|
||||
// Skip macOS archive cruft so it doesn't surface as "ignored" noise.
|
||||
if (entryName.startsWith("__MACOSX/") || entryName.equals(".DS_Store")
|
||||
|| entryName.endsWith("/.DS_Store")) {
|
||||
zis.closeEntry();
|
||||
continue;
|
||||
}
|
||||
|
||||
// Zip Slip guard: normalize and reject absolute / traversal entries.
|
||||
Path entryPath = Path.of(entryName).normalize();
|
||||
if (entryPath.isAbsolute() || entryName.contains("..")) {
|
||||
@ -176,7 +229,7 @@ public class ZipSkillFetcher {
|
||||
throw new IOException("Total extracted size exceeds 50MB limit");
|
||||
}
|
||||
|
||||
String content = new String(bytes, StandardCharsets.UTF_8);
|
||||
String content = new String(bytes, charset);
|
||||
String normalizedName = entryPath.toString().replace('\\', '/');
|
||||
String fileName = entryPath.getFileName().toString();
|
||||
|
||||
|
||||
@ -121,7 +121,87 @@ public class SkillSecurityService {
|
||||
"(?i)import\\s+(os|subprocess|shutil)",
|
||||
"Python system module import",
|
||||
"Importing os/subprocess/shutil enables system-level operations",
|
||||
"Ensure system operations are necessary and scoped appropriately")
|
||||
"Ensure system operations are necessary and scoped appropriately"),
|
||||
|
||||
// ===== Python：不可信反序列化 / 沙箱逃逸 =====
|
||||
rule("PICKLE_DESERIALIZE", "DESERIALIZATION", SkillValidationResult.Severity.HIGH,
|
||||
"(?i)\\b(c?pickle|dill)\\.loads?\\s*\\(",
|
||||
"Untrusted deserialization (pickle)",
|
||||
"pickle/dill load executes arbitrary code embedded in the payload",
|
||||
"Use json or a vetted serializer; never unpickle untrusted data"),
|
||||
rule("MARSHAL_LOADS", "DESERIALIZATION", SkillValidationResult.Severity.HIGH,
|
||||
"(?i)\\bmarshal\\.loads?\\s*\\(",
|
||||
"Untrusted deserialization (marshal)",
|
||||
"marshal can execute crafted bytecode",
|
||||
"Avoid marshal for external data"),
|
||||
// MEDIUM (warn, not block): yaml.load is only unsafe WITHOUT SafeLoader,
|
||||
// and the line-by-line scan can't see a SafeLoader argument that wraps
|
||||
// onto the next line — so blocking here would false-positive legit
|
||||
// multi-line safe calls. Surface it for review instead of blocking.
|
||||
rule("YAML_UNSAFE_LOAD", "DESERIALIZATION", SkillValidationResult.Severity.MEDIUM,
|
||||
"(?i)\\byaml\\.load\\s*\\((?![^)]*(?i:safe))",
|
||||
"Possibly unsafe yaml.load",
|
||||
"yaml.load without SafeLoader can instantiate arbitrary Python objects",
|
||||
"Use yaml.safe_load or Loader=yaml.SafeLoader"),
|
||||
rule("PY_SANDBOX_ESCAPE", "SANDBOX_ESCAPE", SkillValidationResult.Severity.HIGH,
|
||||
"(__subclasses__|__mro__|__builtins__|__globals__)",
|
||||
"Python sandbox-escape primitive",
|
||||
"Introspection attributes commonly used to break out of restricted execution",
|
||||
"Remove reflection into builtins / class hierarchies"),
|
||||
rule("PY_CTYPES", "CODE_EXECUTION", SkillValidationResult.Severity.HIGH,
|
||||
"(?i)\\bctypes\\.(cdll|windll)\\b",
|
||||
"Native library loading via ctypes",
|
||||
"ctypes can load and call arbitrary native code",
|
||||
"Avoid ctypes; use safe Python APIs"),
|
||||
rule("PY_DYNAMIC_IMPORT", "CODE_EXECUTION", SkillValidationResult.Severity.LOW,
|
||||
"(?i)(\\b__import__\\s*\\(|\\bimportlib\\.import_module\\s*\\()",
|
||||
"Dynamic module import",
|
||||
"Dynamic imports can load attacker-controlled modules",
|
||||
"Import modules statically by name where possible"),
|
||||
|
||||
// ===== Node.js：危险 API =====
|
||||
rule("NODE_CHILD_PROCESS", "CODE_EXECUTION", SkillValidationResult.Severity.MEDIUM,
|
||||
"(?i)(require\\s*\\(\\s*['\"]child_process['\"]\\s*\\)|child_process\\.(exec|execSync|spawn|spawnSync|fork))",
|
||||
"Node child_process execution",
|
||||
"child_process can run arbitrary system commands",
|
||||
"Confirm subprocess use is necessary and arguments are not attacker-controlled"),
|
||||
rule("NODE_NEW_FUNCTION", "CODE_EXECUTION", SkillValidationResult.Severity.HIGH,
|
||||
"(?i)\\bnew\\s+Function\\s*\\(",
|
||||
"Dynamic code execution (new Function)",
|
||||
"new Function compiles strings into executable code, like eval",
|
||||
"Use structured logic instead of constructing functions from strings"),
|
||||
rule("NODE_VM_MODULE", "CODE_EXECUTION", SkillValidationResult.Severity.MEDIUM,
|
||||
"(?i)require\\s*\\(\\s*['\"](vm|vm2)['\"]\\s*\\)",
|
||||
"Node vm/vm2 module",
|
||||
"vm/vm2 are frequently used (and escaped) for sandboxed eval",
|
||||
"Avoid the vm module for untrusted code"),
|
||||
rule("PROTOTYPE_POLLUTION", "SANDBOX_ESCAPE", SkillValidationResult.Severity.MEDIUM,
|
||||
"(\\[\\s*['\"]__proto__['\"]\\s*\\]|\\.__proto__\\s*=)",
|
||||
"Prototype pollution pattern",
|
||||
"Writing __proto__ can corrupt object prototypes platform-wide",
|
||||
"Validate keys before dynamic property assignment"),
|
||||
|
||||
// ===== 混淆 / 资源耗尽 / 持久化 / 凭据读取 =====
|
||||
rule("BASE64_PIPE_EXEC", "OBFUSCATION", SkillValidationResult.Severity.HIGH,
|
||||
"(?i)base64\\s+(-d|--decode)\\b[^\\n|]*\\|\\s*(sh|bash|zsh|python|perl|node)\\b",
|
||||
"Obfuscated execution (base64 decode piped to interpreter)",
|
||||
"Decoding then piping to a shell hides the real command from review",
|
||||
"Ship the command in clear text"),
|
||||
rule("FORK_BOMB", "RESOURCE_EXHAUSTION", SkillValidationResult.Severity.CRITICAL,
|
||||
":\\s*\\(\\s*\\)\\s*\\{\\s*:\\s*\\|\\s*:\\s*&\\s*\\}\\s*;\\s*:",
|
||||
"Fork bomb",
|
||||
"Self-replicating process that exhausts system resources",
|
||||
"Remove the fork bomb"),
|
||||
rule("PERSISTENCE", "PERSISTENCE", SkillValidationResult.Severity.MEDIUM,
|
||||
"(?i)(crontab\\s+-|/etc/cron|authorized_keys|/etc/rc\\.local|/etc/profile\\.d/|>>\\s*~?/?\\.?(bashrc|zshrc|profile))",
|
||||
"Persistence mechanism",
|
||||
"Modifies startup files / cron / SSH keys to persist across sessions",
|
||||
"Skills should not install persistence hooks"),
|
||||
rule("SECRET_FILE_READ", "DATA_EXFILTRATION", SkillValidationResult.Severity.HIGH,
|
||||
"(?i)(/etc/shadow|\\.ssh/id_(rsa|ed25519|ecdsa)|\\.aws/credentials|\\.kube/config)",
|
||||
"Sensitive credential file access",
|
||||
"References well-known secret files (private keys, cloud credentials)",
|
||||
"Skills must not read system or user credential files")
|
||||
);
|
||||
|
||||
/**
|
||||
|
||||
@ -204,4 +204,29 @@ class ZipSkillFetcherTest {
|
||||
assertEquals(1, ex.scripts().size());
|
||||
assertEquals("#!/bin/sh\n", ex.scripts().get("setup.sh"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("GBK-encoded entry names (Windows-authored zip) fall back from UTF-8 to GBK")
|
||||
void extractsGbkEncodedNames() throws IOException {
|
||||
org.junit.jupiter.api.Assumptions.assumeTrue(
|
||||
java.nio.charset.Charset.isSupported("GBK"), "GBK charset not available on this JVM");
|
||||
java.nio.charset.Charset gbk = java.nio.charset.Charset.forName("GBK");
|
||||
|
||||
ByteArrayOutputStream baos = new ByteArrayOutputStream();
|
||||
try (ZipOutputStream zos = new ZipOutputStream(baos, gbk)) {
|
||||
zos.putNextEntry(new ZipEntry("SKILL.md"));
|
||||
zos.write(SKILL_MD.getBytes(gbk));
|
||||
zos.closeEntry();
|
||||
// Chinese filename whose GBK bytes are invalid UTF-8 → forces the fallback.
|
||||
zos.putNextEntry(new ZipEntry("references/中文说明.md"));
|
||||
zos.write("# 中文内容\n".getBytes(gbk));
|
||||
zos.closeEntry();
|
||||
}
|
||||
|
||||
ZipSkillFetcher.ExtractedSkill ex = ZipSkillFetcher.extract(baos.toByteArray());
|
||||
|
||||
assertNotNull(ex.skillMdContent());
|
||||
assertEquals(1, ex.references().size(), "GBK-named reference should survive the charset fallback");
|
||||
assertEquals("# 中文内容\n", ex.references().get("中文说明.md"));
|
||||
}
|
||||
}
|
||||
|
||||
@ -214,4 +214,65 @@ class SkillSecurityServiceTest {
|
||||
assertEquals(3, sudoFinding.getLineNumber());
|
||||
assertNotNull(sudoFinding.getSnippet());
|
||||
}
|
||||
|
||||
// ===== 扩充规则：反序列化 / 沙箱逃逸 / 混淆 / 资源耗尽 / 凭据 =====
|
||||
|
||||
@Test
|
||||
@DisplayName("检测 pickle.loads 不可信反序列化 → HIGH → blocked")
|
||||
void shouldBlockPickleDeserialize(@TempDir Path tempDir) throws IOException {
|
||||
Files.writeString(tempDir.resolve("SKILL.md"), "---\nname: test\n---\n# Test");
|
||||
Path scripts = Files.createDirectory(tempDir.resolve("scripts"));
|
||||
Files.writeString(scripts.resolve("load.py"), "import pickle\nobj = pickle.loads(payload)\n");
|
||||
|
||||
SkillValidationResult result = securityService.scanDirectory(tempDir, "test_skill");
|
||||
|
||||
assertTrue(result.isBlocked());
|
||||
assertTrue(result.getFindings().stream()
|
||||
.anyMatch(f -> f.getRuleId().equals("PICKLE_DESERIALIZE")));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("检测 fork bomb → CRITICAL → blocked")
|
||||
void shouldBlockForkBomb(@TempDir Path tempDir) throws IOException {
|
||||
Files.writeString(tempDir.resolve("SKILL.md"), "---\nname: test\n---\n# Test");
|
||||
Path scripts = Files.createDirectory(tempDir.resolve("scripts"));
|
||||
Files.writeString(scripts.resolve("bomb.sh"), "#!/bin/bash\n:(){ :|:& };:\n");
|
||||
|
||||
SkillValidationResult result = securityService.scanDirectory(tempDir, "test_skill");
|
||||
|
||||
assertTrue(result.isBlocked());
|
||||
assertEquals(SkillValidationResult.Severity.CRITICAL, result.getMaxSeverity());
|
||||
assertTrue(result.getFindings().stream()
|
||||
.anyMatch(f -> f.getRuleId().equals("FORK_BOMB")));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("检测读取 SSH 私钥 → HIGH → blocked")
|
||||
void shouldBlockSecretFileRead(@TempDir Path tempDir) throws IOException {
|
||||
Files.writeString(tempDir.resolve("SKILL.md"), "---\nname: test\n---\n# Test");
|
||||
Path scripts = Files.createDirectory(tempDir.resolve("scripts"));
|
||||
Files.writeString(scripts.resolve("steal.sh"), "cat ~/.ssh/id_rsa\n");
|
||||
|
||||
SkillValidationResult result = securityService.scanDirectory(tempDir, "test_skill");
|
||||
|
||||
assertTrue(result.isBlocked());
|
||||
assertTrue(result.getFindings().stream()
|
||||
.anyMatch(f -> f.getRuleId().equals("SECRET_FILE_READ")));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Node child_process → MEDIUM → 警告但不阻断")
|
||||
void shouldWarnNodeChildProcess(@TempDir Path tempDir) throws IOException {
|
||||
Files.writeString(tempDir.resolve("SKILL.md"), "---\nname: test\n---\n# Test");
|
||||
Path scripts = Files.createDirectory(tempDir.resolve("scripts"));
|
||||
// spawn (not exec) so the existing EVAL_EXEC HIGH rule doesn't also fire
|
||||
Files.writeString(scripts.resolve("run.js"), "const cp = require('child_process');\ncp.spawn('ls');\n");
|
||||
|
||||
SkillValidationResult result = securityService.scanDirectory(tempDir, "test_skill");
|
||||
|
||||
assertTrue(result.isPassed(), "MEDIUM should pass");
|
||||
assertFalse(result.isBlocked(), "MEDIUM should not block");
|
||||
assertTrue(result.getFindings().stream()
|
||||
.anyMatch(f -> f.getRuleId().equals("NODE_CHILD_PROCESS")));
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user