Skip to content

Commit 0e764d8

Browse files
yun-zhi-ztlyun-zhi-ztl
authored
feat(publish): relax skill upload constraints (#80)
* fix: keep download counts consistent across skill pages * fix: stabilize empty search ordering across sorts * fix: show disabled-account reason on login redirect * fix: mute report input placeholder text * fix: return skill detail to my skills page * test: stabilize auth context filter coverage * feat(publish): increase single file limit to 10MB * feat(publish): expand allowed file extensions * feat(publish): extend secret scanning to new text file types * feat(publish): add content validation for new file types * refactor(publish): inject configurable limits into SkillPackageArchiveExtractor * feat(publish): support zip with single root directory wrapper * feat(publish): expand determineContentType for new file types * test(publish): update tests for new upload constraints
1 parent 04e588d commit 0e764d8

File tree

8 files changed

+322
-103
lines changed

8 files changed

+322
-103
lines changed

‎server/skillhub-app/src/main/java/com/iflytek/skillhub/config/SkillPublishProperties.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@
1111
public class SkillPublishProperties {
1212

1313
private int maxFileCount = 100;
14-
private long maxSingleFileSize = 1024 * 1024;
14+
private long maxSingleFileSize = 10 * 1024 * 1024; // 10MB
1515
private long maxPackageSize = 100 * 1024 * 1024;
1616
private Set<String> allowedFileExtensions = new LinkedHashSet<>(Set.of(
17-
".md", ".txt", ".json", ".yaml", ".yml",
18-
".js", ".ts", ".py", ".sh",
19-
".png", ".jpg", ".svg"
17+
".md", ".txt", ".json", ".yaml", ".yml", ".html", ".css", ".csv", ".pdf",
18+
".toml", ".xml", ".ini", ".cfg", ".env",
19+
".js", ".ts", ".py", ".sh", ".rb", ".go", ".rs", ".java", ".kt",
20+
".lua", ".sql", ".r", ".bat", ".ps1", ".zsh", ".bash",
21+
".png", ".jpg", ".jpeg", ".svg", ".gif", ".webp", ".ico"
2022
));
2123

2224
public int getMaxFileCount() {

‎server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java‎

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.iflytek.skillhub.controller.support;
22

3+
import com.iflytek.skillhub.config.SkillPublishProperties;
34
import com.iflytek.skillhub.domain.skill.validation.PackageEntry;
45
import com.iflytek.skillhub.domain.skill.validation.SkillPackagePolicy;
56
import org.springframework.stereotype.Component;
@@ -8,18 +9,30 @@
89
import java.io.ByteArrayOutputStream;
910
import java.io.IOException;
1011
import java.util.ArrayList;
12+
import java.util.HashSet;
1113
import java.util.List;
14+
import java.util.Set;
1215
import java.util.zip.ZipEntry;
1316
import java.util.zip.ZipInputStream;
1417

1518
@Component
1619
public class SkillPackageArchiveExtractor {
1720

21+
private final long maxTotalPackageSize;
22+
private final long maxSingleFileSize;
23+
private final int maxFileCount;
24+
25+
public SkillPackageArchiveExtractor(SkillPublishProperties properties) {
26+
this.maxTotalPackageSize = properties.getMaxPackageSize();
27+
this.maxSingleFileSize = properties.getMaxSingleFileSize();
28+
this.maxFileCount = properties.getMaxFileCount();
29+
}
30+
1831
public List<PackageEntry> extract(MultipartFile file) throws IOException {
19-
if (file.getSize() > SkillPackagePolicy.MAX_TOTAL_PACKAGE_SIZE) {
32+
if (file.getSize() > maxTotalPackageSize) {
2033
throw new IllegalArgumentException(
2134
"Package too large: " + file.getSize() + " bytes (max: "
22-
+ SkillPackagePolicy.MAX_TOTAL_PACKAGE_SIZE + ")"
35+
+ maxTotalPackageSize + ")"
2336
);
2437
}
2538

@@ -34,19 +47,19 @@ public List<PackageEntry> extract(MultipartFile file) throws IOException {
3447
continue;
3548
}
3649

37-
if (entries.size() >= SkillPackagePolicy.MAX_FILE_COUNT) {
50+
if (entries.size() >= maxFileCount) {
3851
throw new IllegalArgumentException(
39-
"Too many files: more than " + SkillPackagePolicy.MAX_FILE_COUNT
52+
"Too many files: more than " + maxFileCount
4053
);
4154
}
4255

4356
String normalizedPath = SkillPackagePolicy.normalizeEntryPath(zipEntry.getName());
4457
byte[] content = readEntry(zis, normalizedPath);
4558
totalSize += content.length;
46-
if (totalSize > SkillPackagePolicy.MAX_TOTAL_PACKAGE_SIZE) {
59+
if (totalSize > maxTotalPackageSize) {
4760
throw new IllegalArgumentException(
4861
"Package too large: " + totalSize + " bytes (max: "
49-
+ SkillPackagePolicy.MAX_TOTAL_PACKAGE_SIZE + ")"
62+
+ maxTotalPackageSize + ")"
5063
);
5164
}
5265

@@ -60,7 +73,38 @@ public List<PackageEntry> extract(MultipartFile file) throws IOException {
6073
}
6174
}
6275

63-
return entries;
76+
return stripSingleRootDirectory(entries);
77+
}
78+
79+
/**
80+
* If all file paths share a single root directory prefix (e.g., "my-skill/xxx"),
81+
* strip that prefix. Otherwise return entries unchanged.
82+
*/
83+
static List<PackageEntry> stripSingleRootDirectory(List<PackageEntry> entries) {
84+
if (entries.isEmpty()) return entries;
85+
86+
Set<String> rootSegments = new HashSet<>();
87+
for (PackageEntry entry : entries) {
88+
int slashIndex = entry.path().indexOf('/');
89+
if (slashIndex < 0) {
90+
// File at root level, no stripping
91+
return entries;
92+
}
93+
rootSegments.add(entry.path().substring(0, slashIndex));
94+
}
95+
96+
if (rootSegments.size() != 1) {
97+
return entries;
98+
}
99+
100+
String prefix = rootSegments.iterator().next() + "/";
101+
return entries.stream()
102+
.map(e -> new PackageEntry(
103+
e.path().substring(prefix.length()),
104+
e.content(),
105+
e.size(),
106+
e.contentType()))
107+
.toList();
64108
}
65109

66110
private byte[] readEntry(ZipInputStream zis, String path) throws IOException {
@@ -70,10 +114,10 @@ private byte[] readEntry(ZipInputStream zis, String path) throws IOException {
70114
int read;
71115
while ((read = zis.read(buffer)) != -1) {
72116
totalRead += read;
73-
if (totalRead > SkillPackagePolicy.MAX_SINGLE_FILE_SIZE) {
117+
if (totalRead > maxSingleFileSize) {
74118
throw new IllegalArgumentException(
75119
"File too large: " + path + " (" + totalRead + " bytes, max: "
76-
+ SkillPackagePolicy.MAX_SINGLE_FILE_SIZE + ")"
120+
+ maxSingleFileSize + ")"
77121
);
78122
}
79123
outputStream.write(buffer, 0, read);
@@ -82,11 +126,27 @@ private byte[] readEntry(ZipInputStream zis, String path) throws IOException {
82126
}
83127

84128
private String determineContentType(String filename) {
85-
if (filename.endsWith(".py")) return "text/x-python";
86-
if (filename.endsWith(".json")) return "application/json";
87-
if (filename.endsWith(".yaml") || filename.endsWith(".yml")) return "application/x-yaml";
88-
if (filename.endsWith(".txt")) return "text/plain";
89-
if (filename.endsWith(".md")) return "text/markdown";
129+
String lower = filename.toLowerCase();
130+
if (lower.endsWith(".py")) return "text/x-python";
131+
if (lower.endsWith(".json")) return "application/json";
132+
if (lower.endsWith(".yaml") || lower.endsWith(".yml")) return "application/x-yaml";
133+
if (lower.endsWith(".txt")) return "text/plain";
134+
if (lower.endsWith(".md")) return "text/markdown";
135+
if (lower.endsWith(".html")) return "text/html";
136+
if (lower.endsWith(".css")) return "text/css";
137+
if (lower.endsWith(".csv")) return "text/csv";
138+
if (lower.endsWith(".xml")) return "application/xml";
139+
if (lower.endsWith(".js")) return "text/javascript";
140+
if (lower.endsWith(".ts")) return "text/typescript";
141+
if (lower.endsWith(".sh") || lower.endsWith(".bash") || lower.endsWith(".zsh")) return "text/x-shellscript";
142+
if (lower.endsWith(".png")) return "image/png";
143+
if (lower.endsWith(".jpg") || lower.endsWith(".jpeg")) return "image/jpeg";
144+
if (lower.endsWith(".gif")) return "image/gif";
145+
if (lower.endsWith(".svg")) return "image/svg+xml";
146+
if (lower.endsWith(".webp")) return "image/webp";
147+
if (lower.endsWith(".ico")) return "image/x-icon";
148+
if (lower.endsWith(".pdf")) return "application/pdf";
149+
if (lower.endsWith(".toml")) return "application/toml";
90150
return "application/octet-stream";
91151
}
92152
}

‎server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/ZipPackageExtractor.java‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public List<PackageEntry> extract(MultipartFile file) throws IOException {
6969
}
7070
}
7171

72-
return entries;
72+
return SkillPackageArchiveExtractor.stripSingleRootDirectory(entries);
7373
}
7474

7575
private byte[] readEntry(ZipInputStream zis, String path) throws IOException {
@@ -117,11 +117,27 @@ private String normalizeEntryPath(String path) {
117117
}
118118

119119
private String determineContentType(String filename) {
120-
if (filename.endsWith(".py")) return "text/x-python";
121-
if (filename.endsWith(".json")) return "application/json";
122-
if (filename.endsWith(".yaml") || filename.endsWith(".yml")) return "application/x-yaml";
123-
if (filename.endsWith(".txt")) return "text/plain";
124-
if (filename.endsWith(".md")) return "text/markdown";
120+
String lower = filename.toLowerCase();
121+
if (lower.endsWith(".py")) return "text/x-python";
122+
if (lower.endsWith(".json")) return "application/json";
123+
if (lower.endsWith(".yaml") || lower.endsWith(".yml")) return "application/x-yaml";
124+
if (lower.endsWith(".txt")) return "text/plain";
125+
if (lower.endsWith(".md")) return "text/markdown";
126+
if (lower.endsWith(".html")) return "text/html";
127+
if (lower.endsWith(".css")) return "text/css";
128+
if (lower.endsWith(".csv")) return "text/csv";
129+
if (lower.endsWith(".xml")) return "application/xml";
130+
if (lower.endsWith(".js")) return "text/javascript";
131+
if (lower.endsWith(".ts")) return "text/typescript";
132+
if (lower.endsWith(".sh") || lower.endsWith(".bash") || lower.endsWith(".zsh")) return "text/x-shellscript";
133+
if (lower.endsWith(".png")) return "image/png";
134+
if (lower.endsWith(".jpg") || lower.endsWith(".jpeg")) return "image/jpeg";
135+
if (lower.endsWith(".gif")) return "image/gif";
136+
if (lower.endsWith(".svg")) return "image/svg+xml";
137+
if (lower.endsWith(".webp")) return "image/webp";
138+
if (lower.endsWith(".ico")) return "image/x-icon";
139+
if (lower.endsWith(".pdf")) return "application/pdf";
140+
if (lower.endsWith(".toml")) return "application/toml";
125141
return "application/octet-stream";
126142
}
127143
}

‎server/skillhub-app/src/main/resources/application.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,9 @@ skillhub:
103103
anonymous-cookie-secret: ${SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET:change-me-in-production}
104104
publish:
105105
max-file-count: 100
106-
max-single-file-size: 1048576 # 1MB
106+
max-single-file-size: 10485760 # 10MB
107107
max-package-size: 104857600 # 100MB
108-
allowed-file-extensions: .md,.txt,.json,.yaml,.yml,.js,.ts,.py,.sh,.png,.jpg,.svg
108+
allowed-file-extensions: .md,.txt,.json,.yaml,.yml,.html,.css,.csv,.pdf,.toml,.xml,.ini,.cfg,.env,.js,.ts,.py,.sh,.rb,.go,.rs,.java,.kt,.lua,.sql,.r,.bat,.ps1,.zsh,.bash,.png,.jpg,.jpeg,.svg,.gif,.webp,.ico
109109
device-auth:
110110
verification-uri: ${DEVICE_AUTH_VERIFICATION_URI:${skillhub.public.base-url:}/cli/auth}
111111
bootstrap:

‎server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java‎

Lines changed: 104 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,31 @@
11
package com.iflytek.skillhub.controller.support;
22

3+
import com.iflytek.skillhub.config.SkillPublishProperties;
4+
import com.iflytek.skillhub.domain.skill.validation.PackageEntry;
5+
import org.junit.jupiter.api.BeforeEach;
36
import org.junit.jupiter.api.Test;
47
import org.springframework.mock.web.MockMultipartFile;
58

69
import java.io.ByteArrayOutputStream;
710
import java.nio.charset.StandardCharsets;
11+
import java.util.List;
12+
import java.util.Map;
813
import java.util.zip.ZipEntry;
914
import java.util.zip.ZipOutputStream;
1015

16+
import static org.junit.jupiter.api.Assertions.assertEquals;
1117
import static org.junit.jupiter.api.Assertions.assertThrows;
1218
import static org.junit.jupiter.api.Assertions.assertTrue;
1319

1420
class SkillPackageArchiveExtractorTest {
1521

16-
private final SkillPackageArchiveExtractor extractor = new SkillPackageArchiveExtractor();
22+
private SkillPackageArchiveExtractor extractor;
23+
24+
@BeforeEach
25+
void setUp() {
26+
SkillPublishProperties props = new SkillPublishProperties();
27+
extractor = new SkillPackageArchiveExtractor(props);
28+
}
1729

1830
@Test
1931
void shouldRejectPathTraversalEntry() throws Exception {
@@ -31,19 +43,89 @@ void shouldRejectPathTraversalEntry() throws Exception {
3143

3244
@Test
3345
void shouldRejectOversizedZipEntry() throws Exception {
34-
byte[] content = new byte[1024 * 1024 + 1];
35-
MockMultipartFile file = new MockMultipartFile(
36-
"file",
37-
"skill.zip",
38-
"application/zip",
39-
createZip("large.txt", content)
40-
);
46+
SkillPublishProperties props = new SkillPublishProperties();
47+
props.setMaxSingleFileSize(1024); // 1KB limit
48+
SkillPackageArchiveExtractor smallExtractor = new SkillPackageArchiveExtractor(props);
4149

42-
IllegalArgumentException error = assertThrows(IllegalArgumentException.class, () -> extractor.extract(file));
50+
byte[] content = new byte[1025]; // >1KB
51+
byte[] zip = createZip(Map.of("large.txt", content));
52+
MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", zip);
53+
54+
IllegalArgumentException error = assertThrows(IllegalArgumentException.class,
55+
() -> smallExtractor.extract(file));
4356

4457
assertTrue(error.getMessage().contains("File too large: large.txt"));
4558
}
4659

60+
@Test
61+
void respectsConfiguredSingleFileLimit() throws Exception {
62+
SkillPublishProperties props = new SkillPublishProperties();
63+
props.setMaxSingleFileSize(5 * 1024 * 1024); // 5MB
64+
SkillPackageArchiveExtractor customExtractor = new SkillPackageArchiveExtractor(props);
65+
66+
byte[] content = new byte[3 * 1024 * 1024]; // 3MB — under 5MB limit
67+
byte[] zip = createZip(Map.of("data.md", content));
68+
MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", zip);
69+
70+
List<PackageEntry> entries = customExtractor.extract(file);
71+
assertEquals(1, entries.size());
72+
}
73+
74+
@Test
75+
void stripsRootDirectoryWhenSingleFolder() throws Exception {
76+
byte[] zipBytes = createZip(Map.of(
77+
"my-skill/SKILL.md", "---\nname: test\n---\n".getBytes(),
78+
"my-skill/config.json", "{}".getBytes()
79+
));
80+
MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", zipBytes);
81+
List<PackageEntry> entries = extractor.extract(file);
82+
83+
assertTrue(entries.stream().anyMatch(e -> e.path().equals("SKILL.md")));
84+
assertTrue(entries.stream().anyMatch(e -> e.path().equals("config.json")));
85+
}
86+
87+
@Test
88+
void doesNotStripWhenMultipleRootEntries() throws Exception {
89+
byte[] zipBytes = createZip(Map.of(
90+
"SKILL.md", "---\nname: test\n---\n".getBytes(),
91+
"config.json", "{}".getBytes()
92+
));
93+
MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", zipBytes);
94+
List<PackageEntry> entries = extractor.extract(file);
95+
96+
assertTrue(entries.stream().anyMatch(e -> e.path().equals("SKILL.md")));
97+
}
98+
99+
@Test
100+
void stripsRootDirectoryWhenZipHasExplicitDirEntry() throws Exception {
101+
ByteArrayOutputStream baos = new ByteArrayOutputStream();
102+
try (ZipOutputStream zos = new ZipOutputStream(baos)) {
103+
zos.putNextEntry(new ZipEntry("my-skill/"));
104+
zos.closeEntry();
105+
zos.putNextEntry(new ZipEntry("my-skill/SKILL.md"));
106+
zos.write("---\nname: test\n---".getBytes());
107+
zos.closeEntry();
108+
}
109+
MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", baos.toByteArray());
110+
List<PackageEntry> entries = extractor.extract(file);
111+
112+
assertEquals(1, entries.size());
113+
assertEquals("SKILL.md", entries.get(0).path());
114+
}
115+
116+
@Test
117+
void doesNotStripWhenMultipleRootDirectories() throws Exception {
118+
byte[] zipBytes = createZip(Map.of(
119+
"dir-a/SKILL.md", "---\nname: test\n---\n".getBytes(),
120+
"dir-b/other.md", "# other".getBytes()
121+
));
122+
MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", zipBytes);
123+
List<PackageEntry> entries = extractor.extract(file);
124+
125+
assertTrue(entries.stream().anyMatch(e -> e.path().equals("dir-a/SKILL.md")));
126+
assertTrue(entries.stream().anyMatch(e -> e.path().equals("dir-b/other.md")));
127+
}
128+
47129
private byte[] createZip(String entryName, String content) throws Exception {
48130
return createZip(entryName, content.getBytes(StandardCharsets.UTF_8));
49131
}
@@ -58,4 +140,17 @@ private byte[] createZip(String entryName, byte[] content) throws Exception {
58140
}
59141
return baos.toByteArray();
60142
}
143+
144+
private byte[] createZip(Map<String, byte[]> entries) throws Exception {
145+
ByteArrayOutputStream baos = new ByteArrayOutputStream();
146+
try (ZipOutputStream zos = new ZipOutputStream(baos)) {
147+
for (Map.Entry<String, byte[]> e : entries.entrySet()) {
148+
ZipEntry entry = new ZipEntry(e.getKey());
149+
zos.putNextEntry(entry);
150+
zos.write(e.getValue());
151+
zos.closeEntry();
152+
}
153+
}
154+
return baos.toByteArray();
155+
}
61156
}

0 commit comments

Comments
 (0)