diff --git a/mateclaw-server/src/main/java/vip/mate/skill/installer/ZipSkillFetcher.java b/mateclaw-server/src/main/java/vip/mate/skill/installer/ZipSkillFetcher.java index 0f4ac2ac..ef483df5 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/installer/ZipSkillFetcher.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/installer/ZipSkillFetcher.java @@ -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 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(); diff --git a/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillSecurityService.java b/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillSecurityService.java index b6fc9793..94922ac6 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillSecurityService.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillSecurityService.java @@ -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") ); /** diff --git a/mateclaw-server/src/test/java/vip/mate/skill/installer/ZipSkillFetcherTest.java b/mateclaw-server/src/test/java/vip/mate/skill/installer/ZipSkillFetcherTest.java index 9f720692..0037c325 100644 --- a/mateclaw-server/src/test/java/vip/mate/skill/installer/ZipSkillFetcherTest.java +++ b/mateclaw-server/src/test/java/vip/mate/skill/installer/ZipSkillFetcherTest.java @@ -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")); + } } diff --git a/mateclaw-server/src/test/java/vip/mate/skill/runtime/SkillSecurityServiceTest.java b/mateclaw-server/src/test/java/vip/mate/skill/runtime/SkillSecurityServiceTest.java index a20027b8..3e44052d 100644 --- a/mateclaw-server/src/test/java/vip/mate/skill/runtime/SkillSecurityServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/skill/runtime/SkillSecurityServiceTest.java @@ -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"))); + } }