From b6cca3edd026e89a4da790f204f10b661cd212cc Mon Sep 17 00:00:00 2001 From: matevip Date: Fri, 3 Jul 2026 14:58:48 +0800 Subject: [PATCH] fix(skill): make skill ZIP size caps configurable (#467) --- .../controller/SkillInstallController.java | 13 +++- .../mate/skill/installer/SkillHubClient.java | 12 +++- .../mate/skill/installer/ZipSkillFetcher.java | 66 +++++++++++++++---- .../src/main/resources/application.yml | 7 ++ .../src/main/resources/docs/en/skills.md | 4 +- .../src/main/resources/docs/zh/skills.md | 4 +- .../skill/installer/ZipSkillFetcherTest.java | 34 ++++++++++ 7 files changed, 120 insertions(+), 20 deletions(-) diff --git a/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillInstallController.java b/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillInstallController.java index ac81767c..0f6e7b7d 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillInstallController.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillInstallController.java @@ -3,6 +3,7 @@ package vip.mate.skill.controller; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.tags.Tag; import lombok.RequiredArgsConstructor; +import org.springframework.beans.factory.annotation.Value; import org.springframework.http.MediaType; import org.springframework.web.bind.annotation.*; import org.springframework.web.multipart.MultipartFile; @@ -33,6 +34,15 @@ public class SkillInstallController { private final SkillInstaller skillInstaller; private final SkillFrontmatterParser frontmatterParser; + /** Per-entry cap inside an uploaded skill ZIP (MB). */ + @Value("${mateclaw.skill.upload.max-entry-size-mb:1}") + private long maxEntrySizeMb = 1; + + /** Total size cap for an uploaded skill ZIP (MB). The archive is buffered + * in memory during extraction, so this also bounds peak heap usage. */ + @Value("${mateclaw.skill.upload.max-total-size-mb:50}") + private long maxTotalSizeMb = 50; + @Operation(summary = "搜索 ClawHub 市场") @GetMapping("/hub/search") @RequireWorkspaceRole("admin") @@ -90,7 +100,8 @@ public class SkillInstallController { return R.fail(400, "Only .zip files are accepted"); } try { - SkillBundle bundle = ZipSkillFetcher.parse(zipFile, frontmatterParser); + SkillBundle bundle = ZipSkillFetcher.parse(zipFile, frontmatterParser, + ZipSkillFetcher.Limits.ofMb(maxEntrySizeMb, maxTotalSizeMb)); Map result = skillInstaller.installFromBundle( bundle, enable, overwrite, targetName, workspaceId); return R.ok(result); diff --git a/mateclaw-server/src/main/java/vip/mate/skill/installer/SkillHubClient.java b/mateclaw-server/src/main/java/vip/mate/skill/installer/SkillHubClient.java index baf1d102..487305f6 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/installer/SkillHubClient.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/installer/SkillHubClient.java @@ -3,6 +3,7 @@ package vip.mate.skill.installer; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; import vip.mate.skill.installer.model.HubSkillInfo; import vip.mate.skill.installer.model.SkillBundle; @@ -37,6 +38,13 @@ import java.util.Map; @Service public class SkillHubClient { + /** Per-entry / total caps for marketplace bundle ZIPs — same knobs as upload. */ + @Value("${mateclaw.skill.upload.max-entry-size-mb:1}") + private long maxEntrySizeMb = 1; + + @Value("${mateclaw.skill.upload.max-total-size-mb:50}") + private long maxTotalSizeMb = 50; + private final SkillHubProperties properties; private final ObjectMapper objectMapper; private final SkillFrontmatterParser frontmatterParser; @@ -131,7 +139,9 @@ public class SkillHubClient { // Step 3: extract + assemble SkillBundle. try { - ZipSkillFetcher.ExtractedSkill extracted = ZipSkillFetcher.extract(new ByteArrayInputStream(zipBytes)); + ZipSkillFetcher.ExtractedSkill extracted = ZipSkillFetcher.extract( + new ByteArrayInputStream(zipBytes), + ZipSkillFetcher.Limits.ofMb(maxEntrySizeMb, maxTotalSizeMb)); var parsed = frontmatterParser.parse(extracted.skillMdContent()); Map fm = parsed.getFrontmatter(); 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 e1e8221f..d176f0ed 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 @@ -27,7 +27,9 @@ import java.util.zip.ZipInputStream; * path (downloaded ZIP bytes). Hardened against: *
    *
  • Zip Slip path traversal
    • - *
  • Per-file ≤1MB, total ≤50MB
    • + *
  • Per-file / total size caps (defaults 1MB / 50MB, configurable via + * {@code mateclaw.skill.upload.max-entry-size-mb} / + * {@code mateclaw.skill.upload.max-total-size-mb})
    • *
  • Only SKILL.md / references/ / scripts/ entries are kept
    • *
  • Binary entries are skipped with a WARN — bundle storage is text-only, * so decoding them as text would persist corrupted content
    • @@ -44,8 +46,23 @@ import java.util.zip.ZipInputStream; @Slf4j public class ZipSkillFetcher { - private static final long MAX_FILE_SIZE = 1_000_000; // 1MB per file - private static final long MAX_TOTAL_SIZE = 50_000_000; // 50MB total + /** + * Size caps applied while decompressing a bundle. Callers wire these from + * {@code mateclaw.skill.upload.max-entry-size-mb} / + * {@code mateclaw.skill.upload.max-total-size-mb}; the defaults preserve + * the historical 1MB-per-entry / 50MB-total behaviour. The whole archive + * is buffered in memory during extraction, so raising the total cap + * raises peak heap usage accordingly. + */ + public record Limits(long maxEntryBytes, long maxTotalBytes) { + public static final Limits DEFAULT = ofMb(1, 50); + + public static Limits ofMb(long entryMb, long totalMb) { + return new Limits(entryMb * 1_000_000L, totalMb * 1_000_000L); + } + + long totalMb() { return maxTotalBytes / 1_000_000L; } + } private static final String SKILL_MD = "SKILL.md"; private static final String SKILL_MD_LOWER = "skill.md"; @@ -87,16 +104,26 @@ public class ZipSkillFetcher { * and source URL is the original filename. */ public static SkillBundle parse(MultipartFile zipFile, SkillFrontmatterParser parser) throws IOException { + return parse(zipFile, parser, Limits.DEFAULT); + } + + /** + * Parse an uploaded ZIP file into a SkillBundle with explicit size caps + * (see {@link Limits}). + */ + public static SkillBundle parse(MultipartFile zipFile, SkillFrontmatterParser parser, + Limits limits) throws IOException { if (zipFile == null || zipFile.isEmpty()) { throw new IllegalArgumentException("ZIP file is empty"); } - if (zipFile.getSize() > MAX_TOTAL_SIZE) { - throw new IllegalArgumentException("ZIP file too large (max 50MB)"); + if (zipFile.getSize() > limits.maxTotalBytes()) { + throw new IllegalArgumentException("ZIP file too large (max " + limits.totalMb() + + "MB; adjust mateclaw.skill.upload.max-total-size-mb)"); } ExtractedSkill extracted; try (InputStream is = zipFile.getInputStream()) { - extracted = extract(is); + extracted = extract(is, limits); } var parsed = parser.parse(extracted.skillMdContent()); @@ -140,7 +167,12 @@ public class ZipSkillFetcher { * instead of being silently dropped. */ public static ExtractedSkill extract(InputStream zipStream) throws IOException { - return extract(zipStream.readAllBytes()); + return extract(zipStream.readAllBytes(), Limits.DEFAULT); + } + + /** Variant of {@link #extract(InputStream)} with explicit size caps. */ + public static ExtractedSkill extract(InputStream zipStream, Limits limits) throws IOException { + return extract(zipStream.readAllBytes(), limits); } /** Fallback charset for archives authored on Chinese Windows (entry names / content in GBK). */ @@ -154,12 +186,17 @@ public class ZipSkillFetcher { * a one-shot stream) is what makes the retry possible. */ public static ExtractedSkill extract(byte[] zipBytes) throws IOException { + return extract(zipBytes, Limits.DEFAULT); + } + + /** Variant of {@link #extract(byte[])} with explicit size caps. */ + public static ExtractedSkill extract(byte[] zipBytes, Limits limits) throws IOException { try { - return extract(zipBytes, StandardCharsets.UTF_8); + return extract(zipBytes, StandardCharsets.UTF_8, limits); } 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); + return extract(zipBytes, GBK, limits); } throw e; } @@ -182,7 +219,7 @@ public class ZipSkillFetcher { return false; } - private static ExtractedSkill extract(byte[] zipBytes, Charset charset) throws IOException { + private static ExtractedSkill extract(byte[] zipBytes, Charset charset, Limits limits) throws IOException { List raws = new ArrayList<>(); String skillMdContent = null; String skillMdPrefix = ""; @@ -214,21 +251,22 @@ public class ZipSkillFetcher { } long declaredSize = entry.getSize(); - if (declaredSize > MAX_FILE_SIZE) { + if (declaredSize > limits.maxEntryBytes()) { log.warn("[ZipSkillFetcher] Skipping oversized entry: {} ({}bytes)", entryName, declaredSize); zis.closeEntry(); continue; } byte[] bytes = zis.readAllBytes(); - if (bytes.length > MAX_FILE_SIZE) { + if (bytes.length > limits.maxEntryBytes()) { log.warn("[ZipSkillFetcher] Skipping oversized entry post-read: {} ({}bytes)", entryName, bytes.length); zis.closeEntry(); continue; } totalSize += bytes.length; - if (totalSize > MAX_TOTAL_SIZE) { - throw new IOException("Total extracted size exceeds 50MB limit"); + if (totalSize > limits.maxTotalBytes()) { + throw new IOException("Total extracted size exceeds " + limits.totalMb() + + "MB limit (adjust mateclaw.skill.upload.max-total-size-mb)"); } // Skill bundles persist file contents as text (mate_skill_file diff --git a/mateclaw-server/src/main/resources/application.yml b/mateclaw-server/src/main/resources/application.yml index 950cabfd..891e0eca 100644 --- a/mateclaw-server/src/main/resources/application.yml +++ b/mateclaw-server/src/main/resources/application.yml @@ -199,6 +199,13 @@ mateclaw: # remain resolvable. Defaults to the legacy location for zero-config parity. base-dir: ${MATECLAW_CHAT_UPLOAD_BASE_DIR:data/chat-uploads} skill: + upload: + # Size caps for skill bundle ZIPs (upload endpoint and marketplace + # install). The archive is buffered in memory during extraction, so + # max-total-size-mb also bounds peak heap usage per install. Uploads + # additionally pass through spring.servlet.multipart limits above. + max-entry-size-mb: ${MATECLAW_SKILL_UPLOAD_MAX_ENTRY_SIZE_MB:1} + max-total-size-mb: ${MATECLAW_SKILL_UPLOAD_MAX_TOTAL_SIZE_MB:50} workspace: # Skill workspace root. Override with MATECLAW_SKILL_WORKSPACE_ROOT to # relocate it onto a persistent volume — in Docker this is pointed at diff --git a/mateclaw-server/src/main/resources/docs/en/skills.md b/mateclaw-server/src/main/resources/docs/en/skills.md index 4d846157..83ee583f 100644 --- a/mateclaw-server/src/main/resources/docs/en/skills.md +++ b/mateclaw-server/src/main/resources/docs/en/skills.md @@ -192,7 +192,7 @@ The database is the source of truth, the filesystem is a materialized cache. Tha | `id` | Primary key | | `skill_id` | FK to `mate_skill` | | `file_path` | Relative path like `scripts/run.py` or `references/cfg.md` | -| `content` | UTF-8 text (≤1 MB per file, ≤50 MB per bundle) | +| `content` | UTF-8 text (defaults: ≤1 MB per file, ≤50 MB per bundle — configurable via `mateclaw.skill.upload.max-entry-size-mb` / `max-total-size-mb`) | | `content_size` | Byte count (so listings don't have to load the blob) | | `sha256` | Content fingerprint, drives the syncer's idempotent diff | @@ -228,7 +228,7 @@ Two sync passes run at boot, so every node has the latest bundle: Third-party packagers package weirdly — some put `setup.sh` at the zip root, some emit `scripts/` entries before `SKILL.md`. As of v1.3, `ZipSkillFetcher`: -- **Two-pass extraction** — the entire archive is buffered in memory first (cap-protected at 50 MB), `SKILL.md` is located and the wrapper-dir prefix computed, then entries are classified. **Zip entry order no longer affects the result.** +- **Two-pass extraction** — the entire archive is buffered in memory first (cap-protected, 50 MB by default via `mateclaw.skill.upload.max-total-size-mb`), `SKILL.md` is located and the wrapper-dir prefix computed, then entries are classified. **Zip entry order no longer affects the result.** - **Root-level extension fallback** — files sitting next to `SKILL.md` that aren't already under a known bucket get classified by extension: `.sh / .py / .js / .rb / ...` → `scripts/`, `.md / .json / .yaml / .csv / ...` → `references/`. Unknown extensions are dropped with a `WARN` line so packaging mistakes surface instead of vanishing. - **Write-then-prune + empty-bundle guard** — reinstalls **write new files first, then prune anything in the bucket that's not in the new bundle**. If the new bundle has zero entries for a bucket (`scripts/` or `references/`), the disk copies for that bucket are **left alone** — a malformed re-extract can no longer wipe your scripts. Pass `forcePrune=true` if you really want to clear a bucket via an intentionally empty bundle. diff --git a/mateclaw-server/src/main/resources/docs/zh/skills.md b/mateclaw-server/src/main/resources/docs/zh/skills.md index 74ec2d23..205e5312 100644 --- a/mateclaw-server/src/main/resources/docs/zh/skills.md +++ b/mateclaw-server/src/main/resources/docs/zh/skills.md @@ -192,7 +192,7 @@ scripts: | `id` | 主键 | | `skill_id` | 外键到 `mate_skill` | | `file_path` | `scripts/run.py` 或 `references/cfg.md` 这种相对路径 | -| `content` | UTF-8 文本（单文件 ≤1 MB，bundle ≤50 MB） | +| `content` | UTF-8 文本（默认单文件 ≤1 MB、bundle ≤50 MB，可通过 `mateclaw.skill.upload.max-entry-size-mb` / `max-total-size-mb` 调整） | | `content_size` | 字节数（不用拉 blob 就能列） | | `sha256` | 内容指纹，给同步器做幂等 diff | @@ -228,7 +228,7 @@ scripts: 第三方打包者千奇百怪——有人把 `setup.sh` 直接放 zip 根，有人 `scripts/` 排在 `SKILL.md` 之前。`ZipSkillFetcher` v1.3 起： -- **两遍扫描**——先把所有条目缓存（受 50 MB 上限保护），定位 `SKILL.md` 算出 wrapper 前缀，再分类。**条目顺序不再影响结果**。 +- **两遍扫描**——先把所有条目缓存（受总大小上限保护，默认 50 MB，可用 `mateclaw.skill.upload.max-total-size-mb` 调整），定位 `SKILL.md` 算出 wrapper 前缀，再分类。**条目顺序不再影响结果**。 - **根目录扩展名兜底**——SKILL.md 同级的非约定文件按扩展名归类：`.sh / .py / .js / .rb / ...` → `scripts/`，`.md / .json / .yaml / .csv / ...` → `references/`，未识别扩展名落 `WARN` 日志。 - **写后裁剪 + 空 bundle 守卫**——重装时**先写新文件再裁剪不在新 bundle 里的旧文件**。如果新 bundle 某个桶（`scripts/` 或 `references/`）一个条目都没有，**保留磁盘上的旧文件**——一个解析失败的损坏 zip 不会再把你的 skill 擦干净。要强制清空就传 `forcePrune=true`。 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 6e5606d0..73a6ed90 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 @@ -267,4 +267,38 @@ class ZipSkillFetcherTest { assertEquals(1, ex.references().size(), "GBK-named reference should survive the charset fallback"); assertEquals("# 中文内容\n", ex.references().get("中文说明.md")); } + + @Test + @DisplayName("configurable per-entry cap: entry over the default 1MB survives when the cap is raised") + void raisedEntryCapKeepsLargeEntry() throws IOException { + String bigDoc = "x".repeat(2_000_000); // 2MB, over the 1MB default + byte[] zip = zipOf(List.of( + new Entry("SKILL.md", SKILL_MD), + new Entry("references/big.md", bigDoc))); + + ZipSkillFetcher.ExtractedSkill withDefaults = ZipSkillFetcher.extract(zip); + assertTrue(withDefaults.references().isEmpty(), + "default 1MB cap should drop the 2MB entry"); + + ZipSkillFetcher.ExtractedSkill withRaisedCap = ZipSkillFetcher.extract( + zip, ZipSkillFetcher.Limits.ofMb(5, 50)); + assertEquals(bigDoc, withRaisedCap.references().get("big.md"), + "raised cap should keep the 2MB entry intact"); + } + + @Test + @DisplayName("configurable total cap: error message names the effective limit and the config knob") + void totalCapErrorNamesConfiguredLimit() throws IOException { + byte[] zip = zipOf(List.of( + new Entry("SKILL.md", SKILL_MD), + new Entry("references/a.md", "y".repeat(900_000)), + new Entry("references/b.md", "z".repeat(900_000)))); + + IOException ex = assertThrows(IOException.class, + () -> ZipSkillFetcher.extract(zip, ZipSkillFetcher.Limits.ofMb(1, 1))); + assertTrue(ex.getMessage().contains("1MB"), + "message should carry the configured total cap; got: " + ex.getMessage()); + assertTrue(ex.getMessage().contains("max-total-size-mb"), + "message should point at the config property; got: " + ex.getMessage()); + } }