feat(publish): filter macOS metadata and add integration tests

Skip __MACOSX/, .DS_Store, and ._ resource fork entries during zip
extraction. Add integration tests for nested SKILL.md warning flow,
session invalidation 401 response, and macOS metadata filtering.
This commit is contained in:
xiose 2026-04-30 10:50:36 +08:00
parent efdcd1ce0d
commit 08e1a3ad30
4 changed files with 129 additions and 0 deletions

View file

@ -49,6 +49,11 @@ public class SkillPackageArchiveExtractor {
continue;
}
if (isOsMetadataEntry(zipEntry.getName())) {
zis.closeEntry();
continue;
}
if (entries.size() >= maxFileCount) {
throw new IllegalArgumentException(
"Too many files: more than " + maxFileCount
@ -159,6 +164,13 @@ public class SkillPackageArchiveExtractor {
return new ExtractionResult(promoted, warnings);
}
private static boolean isOsMetadataEntry(String name) {
String normalized = name.replace('\\', '/');
if (normalized.startsWith("__MACOSX/") || normalized.equals("__MACOSX")) return true;
String fileName = normalized.contains("/") ? normalized.substring(normalized.lastIndexOf('/') + 1) : normalized;
return fileName.equals(".DS_Store") || fileName.startsWith("._");
}
private byte[] readEntry(ZipInputStream zis, String path) throws IOException {
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
byte[] buffer = new byte[8192];

View file

@ -3,6 +3,7 @@ package com.iflytek.skillhub.controller.portal;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import com.iflytek.skillhub.domain.skill.validation.PackageEntry;
@ -161,6 +162,64 @@ class SkillPublishControllerTest {
.andExpect(jsonPath("$.code").value(0));
}
@Test
void publish_nestedSkillMdReturnsWarningForIgnoredFiles() throws Exception {
PlatformPrincipal principal = new PlatformPrincipal(
"usr_1", "publisher", "publisher@example.com", "", "local", Set.of("SUPER_ADMIN"));
var auth = new UsernamePasswordAuthenticationToken(
principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")));
MockMultipartFile file = new MockMultipartFile(
"file", "skill.zip", "application/zip",
buildZipWithNestedSkillMd());
mockMvc.perform(multipart("/api/v1/skills/global/publish")
.file(file)
.param("visibility", "PUBLIC")
.with(authentication(auth))
.with(csrf()))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.msg").value(
org.hamcrest.Matchers.containsString("stray.txt")));
verify(skillPublishService, never()).publishFromEntries(
eq("global"), anyList(), eq("usr_1"),
eq(SkillVisibility.PUBLIC), eq(Set.of("SUPER_ADMIN")), eq(false));
}
@Test
void publish_nestedSkillMdSucceedsWithConfirmWarnings() throws Exception {
SkillVersion version = new SkillVersion(12L, "1.0.0", "usr_1");
version.setStatus(SkillVersionStatus.PENDING_REVIEW);
version.setFileCount(1);
version.setTotalSize(128L);
ReflectionTestUtils.setField(version, "id", 34L);
given(skillPublishService.publishFromEntries(
eq("global"), ArgumentMatchers.<List<PackageEntry>>any(),
eq("usr_1"), eq(SkillVisibility.PUBLIC),
eq(Set.of("SUPER_ADMIN")), eq(true)))
.willReturn(new SkillPublishService.PublishResult(12L, "demo-skill", version));
PlatformPrincipal principal = new PlatformPrincipal(
"usr_1", "publisher", "publisher@example.com", "", "local", Set.of("SUPER_ADMIN"));
var auth = new UsernamePasswordAuthenticationToken(
principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")));
MockMultipartFile file = new MockMultipartFile(
"file", "skill.zip", "application/zip",
buildZipWithNestedSkillMd());
mockMvc.perform(multipart("/api/v1/skills/global/publish")
.file(file)
.param("visibility", "PUBLIC")
.param("confirmWarnings", "true")
.with(authentication(auth))
.with(csrf()))
.andExpect(status().isOk())
.andExpect(jsonPath("$.code").value(0));
}
private byte[] buildZipBytes() throws Exception {
try (ByteArrayOutputStream output = new ByteArrayOutputStream();
ZipOutputStream zip = new ZipOutputStream(output, StandardCharsets.UTF_8)) {
@ -176,4 +235,23 @@ class SkillPublishControllerTest {
return output.toByteArray();
}
}
private byte[] buildZipWithNestedSkillMd() throws Exception {
try (ByteArrayOutputStream output = new ByteArrayOutputStream();
ZipOutputStream zip = new ZipOutputStream(output, StandardCharsets.UTF_8)) {
zip.putNextEntry(new ZipEntry("my-skill/SKILL.md"));
zip.write("""
---
name: Demo Skill
version: 1.0.0
---
""".getBytes(StandardCharsets.UTF_8));
zip.closeEntry();
zip.putNextEntry(new ZipEntry("stray.txt"));
zip.write("ignored file".getBytes(StandardCharsets.UTF_8));
zip.closeEntry();
zip.finish();
return output.toByteArray();
}
}
}

View file

@ -187,6 +187,23 @@ class SkillPackageArchiveExtractorTest {
assertTrue(result.warnings().stream().anyMatch(w -> w.contains("stray.txt")));
}
@Test
void filtersMacOsMetadataEntries() throws Exception {
byte[] zipBytes = createZip(Map.of(
"my-skill/SKILL.md", "---\nname: test\n---\n".getBytes(),
"my-skill/README.md", "# readme".getBytes(),
"__MACOSX/my-skill/._SKILL.md", "resource fork".getBytes(),
"my-skill/.DS_Store", "binary".getBytes()
));
MockMultipartFile file = new MockMultipartFile("file", "test.zip", "application/zip", zipBytes);
List<PackageEntry> entries = extractor.extract(file);
assertEquals(2, entries.size());
assertTrue(entries.stream().noneMatch(e -> e.path().contains("MACOSX")));
assertTrue(entries.stream().noneMatch(e -> e.path().contains(".DS_Store")));
}
private byte[] createZip(String entryName, String content) throws Exception {
return createZip(entryName, content.getBytes(StandardCharsets.UTF_8));
}

View file

@ -1,6 +1,7 @@
package com.iflytek.skillhub.exception;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.when;
import com.iflytek.skillhub.dto.ApiResponse;
@ -70,4 +71,25 @@ class GlobalExceptionHandlerTest {
assertThat(body.code()).isEqualTo(408);
assertThat(body.msg()).isEqualTo("Request timed out");
}
@Test
void handleSessionInvalidated_shouldReturn401ForSessionException() {
when(request.getMethod()).thenReturn("GET");
when(sensitiveLogSanitizer.sanitizeRequestTarget(request)).thenReturn("/api/v1/skills");
IllegalStateException ex = new IllegalStateException("Session was invalidated");
ResponseEntity<ApiResponse<Void>> response = handler.handleSessionInvalidated(ex, request);
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
assertThat(response.getBody()).isNotNull();
assertThat(response.getBody().code()).isEqualTo(401);
}
@Test
void handleSessionInvalidated_shouldRethrowNonSessionException() {
IllegalStateException ex = new IllegalStateException("Some other error");
assertThatThrownBy(() -> handler.handleSessionInvalidated(ex, request))
.isSameAs(ex);
}
}