fix(skill): make skill ZIP size caps configurable (#467)

This commit is contained in:
matevip 2026-07-03 14:58:48 +08:00
parent bb946685b3
commit b6cca3edd0
7 changed files with 120 additions and 20 deletions

View File

@ -3,6 +3,7 @@ package vip.mate.skill.controller;
import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag; import io.swagger.v3.oas.annotations.tags.Tag;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
import org.springframework.web.bind.annotation.*; import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile; import org.springframework.web.multipart.MultipartFile;
@ -33,6 +34,15 @@ public class SkillInstallController {
private final SkillInstaller skillInstaller; private final SkillInstaller skillInstaller;
private final SkillFrontmatterParser frontmatterParser; 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 市场") @Operation(summary = "搜索 ClawHub 市场")
@GetMapping("/hub/search") @GetMapping("/hub/search")
@RequireWorkspaceRole("admin") @RequireWorkspaceRole("admin")
@ -90,7 +100,8 @@ public class SkillInstallController {
return R.fail(400, "Only .zip files are accepted"); return R.fail(400, "Only .zip files are accepted");
} }
try { try {
SkillBundle bundle = ZipSkillFetcher.parse(zipFile, frontmatterParser); SkillBundle bundle = ZipSkillFetcher.parse(zipFile, frontmatterParser,
ZipSkillFetcher.Limits.ofMb(maxEntrySizeMb, maxTotalSizeMb));
Map<String, Object> result = skillInstaller.installFromBundle( Map<String, Object> result = skillInstaller.installFromBundle(
bundle, enable, overwrite, targetName, workspaceId); bundle, enable, overwrite, targetName, workspaceId);
return R.ok(result); return R.ok(result);

View File

@ -3,6 +3,7 @@ package vip.mate.skill.installer;
import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service; import org.springframework.stereotype.Service;
import vip.mate.skill.installer.model.HubSkillInfo; import vip.mate.skill.installer.model.HubSkillInfo;
import vip.mate.skill.installer.model.SkillBundle; import vip.mate.skill.installer.model.SkillBundle;
@ -37,6 +38,13 @@ import java.util.Map;
@Service @Service
public class SkillHubClient { 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 SkillHubProperties properties;
private final ObjectMapper objectMapper; private final ObjectMapper objectMapper;
private final SkillFrontmatterParser frontmatterParser; private final SkillFrontmatterParser frontmatterParser;
@ -131,7 +139,9 @@ public class SkillHubClient {
// Step 3: extract + assemble SkillBundle. // Step 3: extract + assemble SkillBundle.
try { 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()); var parsed = frontmatterParser.parse(extracted.skillMdContent());
Map<String, Object> fm = parsed.getFrontmatter(); Map<String, Object> fm = parsed.getFrontmatter();

View File

@ -27,7 +27,9 @@ import java.util.zip.ZipInputStream;
* path (downloaded ZIP bytes). Hardened against: * path (downloaded ZIP bytes). Hardened against:
* <ul> * <ul>
* <li>Zip Slip path traversal</li> * <li>Zip Slip path traversal</li>
* <li>Per-file 1MB, total 50MB</li> * <li>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})</li>
* <li>Only SKILL.md / references/ / scripts/ entries are kept</li> * <li>Only SKILL.md / references/ / scripts/ entries are kept</li>
* <li>Binary entries are skipped with a WARN bundle storage is text-only, * <li>Binary entries are skipped with a WARN bundle storage is text-only,
* so decoding them as text would persist corrupted content</li> * so decoding them as text would persist corrupted content</li>
@ -44,8 +46,23 @@ import java.util.zip.ZipInputStream;
@Slf4j @Slf4j
public class ZipSkillFetcher { 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 = "SKILL.md";
private static final String SKILL_MD_LOWER = "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. * and source URL is the original filename.
*/ */
public static SkillBundle parse(MultipartFile zipFile, SkillFrontmatterParser parser) throws IOException { 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()) { if (zipFile == null || zipFile.isEmpty()) {
throw new IllegalArgumentException("ZIP file is empty"); throw new IllegalArgumentException("ZIP file is empty");
} }
if (zipFile.getSize() > MAX_TOTAL_SIZE) { if (zipFile.getSize() > limits.maxTotalBytes()) {
throw new IllegalArgumentException("ZIP file too large (max 50MB)"); throw new IllegalArgumentException("ZIP file too large (max " + limits.totalMb()
+ "MB; adjust mateclaw.skill.upload.max-total-size-mb)");
} }
ExtractedSkill extracted; ExtractedSkill extracted;
try (InputStream is = zipFile.getInputStream()) { try (InputStream is = zipFile.getInputStream()) {
extracted = extract(is); extracted = extract(is, limits);
} }
var parsed = parser.parse(extracted.skillMdContent()); var parsed = parser.parse(extracted.skillMdContent());
@ -140,7 +167,12 @@ public class ZipSkillFetcher {
* instead of being silently dropped. * instead of being silently dropped.
*/ */
public static ExtractedSkill extract(InputStream zipStream) throws IOException { 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). */ /** 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. * a one-shot stream) is what makes the retry possible.
*/ */
public static ExtractedSkill extract(byte[] zipBytes) throws IOException { 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 { try {
return extract(zipBytes, StandardCharsets.UTF_8); return extract(zipBytes, StandardCharsets.UTF_8, limits);
} catch (IOException | RuntimeException e) { } catch (IOException | RuntimeException e) {
if (GBK != null && isCharsetError(e)) { if (GBK != null && isCharsetError(e)) {
log.warn("[ZipSkillFetcher] UTF-8 entry decode failed, retrying with GBK (Windows-authored archive?)"); 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; throw e;
} }
@ -182,7 +219,7 @@ public class ZipSkillFetcher {
return false; 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<RawEntry> raws = new ArrayList<>(); List<RawEntry> raws = new ArrayList<>();
String skillMdContent = null; String skillMdContent = null;
String skillMdPrefix = ""; String skillMdPrefix = "";
@ -214,21 +251,22 @@ public class ZipSkillFetcher {
} }
long declaredSize = entry.getSize(); long declaredSize = entry.getSize();
if (declaredSize > MAX_FILE_SIZE) { if (declaredSize > limits.maxEntryBytes()) {
log.warn("[ZipSkillFetcher] Skipping oversized entry: {} ({}bytes)", entryName, declaredSize); log.warn("[ZipSkillFetcher] Skipping oversized entry: {} ({}bytes)", entryName, declaredSize);
zis.closeEntry(); zis.closeEntry();
continue; continue;
} }
byte[] bytes = zis.readAllBytes(); 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); log.warn("[ZipSkillFetcher] Skipping oversized entry post-read: {} ({}bytes)", entryName, bytes.length);
zis.closeEntry(); zis.closeEntry();
continue; continue;
} }
totalSize += bytes.length; totalSize += bytes.length;
if (totalSize > MAX_TOTAL_SIZE) { if (totalSize > limits.maxTotalBytes()) {
throw new IOException("Total extracted size exceeds 50MB limit"); 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 // Skill bundles persist file contents as text (mate_skill_file

View File

@ -199,6 +199,13 @@ mateclaw:
# remain resolvable. Defaults to the legacy location for zero-config parity. # remain resolvable. Defaults to the legacy location for zero-config parity.
base-dir: ${MATECLAW_CHAT_UPLOAD_BASE_DIR:data/chat-uploads} base-dir: ${MATECLAW_CHAT_UPLOAD_BASE_DIR:data/chat-uploads}
skill: 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: workspace:
# Skill workspace root. Override with MATECLAW_SKILL_WORKSPACE_ROOT to # Skill workspace root. Override with MATECLAW_SKILL_WORKSPACE_ROOT to
# relocate it onto a persistent volume — in Docker this is pointed at # relocate it onto a persistent volume — in Docker this is pointed at

View File

@ -192,7 +192,7 @@ The database is the source of truth, the filesystem is a materialized cache. Tha
| `id` | Primary key | | `id` | Primary key |
| `skill_id` | FK to `mate_skill` | | `skill_id` | FK to `mate_skill` |
| `file_path` | Relative path like `scripts/run.py` or `references/cfg.md` | | `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) | | `content_size` | Byte count (so listings don't have to load the blob) |
| `sha256` | Content fingerprint, drives the syncer's idempotent diff | | `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`: 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. - **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. - **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.

View File

@ -192,7 +192,7 @@ scripts:
| `id` | 主键 | | `id` | 主键 |
| `skill_id` | 外键到 `mate_skill` | | `skill_id` | 外键到 `mate_skill` |
| `file_path` | `scripts/run.py``references/cfg.md` 这种相对路径 | | `file_path` | `scripts/run.py``references/cfg.md` 这种相对路径 |
| `content` | UTF-8 文本(单文件 ≤1 MBbundle ≤50 MB | | `content` | UTF-8 文本(默认单文件 ≤1 MB、bundle ≤50 MB可通过 `mateclaw.skill.upload.max-entry-size-mb` / `max-total-size-mb` 调整 |
| `content_size` | 字节数(不用拉 blob 就能列) | | `content_size` | 字节数(不用拉 blob 就能列) |
| `sha256` | 内容指纹,给同步器做幂等 diff | | `sha256` | 内容指纹,给同步器做幂等 diff |
@ -228,7 +228,7 @@ scripts:
第三方打包者千奇百怪——有人把 `setup.sh` 直接放 zip 根,有人 `scripts/` 排在 `SKILL.md` 之前。`ZipSkillFetcher` v1.3 起: 第三方打包者千奇百怪——有人把 `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` 日志。 - **根目录扩展名兜底**——SKILL.md 同级的非约定文件按扩展名归类:`.sh / .py / .js / .rb / ...` → `scripts/``.md / .json / .yaml / .csv / ...` → `references/`,未识别扩展名落 `WARN` 日志。
- **写后裁剪 + 空 bundle 守卫**——重装时**先写新文件再裁剪不在新 bundle 里的旧文件**。如果新 bundle 某个桶(`scripts/` 或 `references/`)一个条目都没有,**保留磁盘上的旧文件**——一个解析失败的损坏 zip 不会再把你的 skill 擦干净。要强制清空就传 `forcePrune=true` - **写后裁剪 + 空 bundle 守卫**——重装时**先写新文件再裁剪不在新 bundle 里的旧文件**。如果新 bundle 某个桶(`scripts/` 或 `references/`)一个条目都没有,**保留磁盘上的旧文件**——一个解析失败的损坏 zip 不会再把你的 skill 擦干净。要强制清空就传 `forcePrune=true`

View File

@ -267,4 +267,38 @@ class ZipSkillFetcherTest {
assertEquals(1, ex.references().size(), "GBK-named reference should survive the charset fallback"); assertEquals(1, ex.references().size(), "GBK-named reference should survive the charset fallback");
assertEquals("# 中文内容\n", ex.references().get("中文说明.md")); 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());
}
} }