From 08e1a3ad30b6fe3213d53e3ddb94e2bb35f46202 Mon Sep 17 00:00:00 2001 From: xiose Date: Thu, 30 Apr 2026 10:50:36 +0800 Subject: [PATCH] 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. --- .../support/SkillPackageArchiveExtractor.java | 12 +++ .../portal/SkillPublishControllerTest.java | 78 +++++++++++++++++++ .../SkillPackageArchiveExtractorTest.java | 17 ++++ .../exception/GlobalExceptionHandlerTest.java | 22 ++++++ 4 files changed, 129 insertions(+) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java index fad1eb72..e2aa41c3 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractor.java @@ -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]; diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillPublishControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillPublishControllerTest.java index ae5fd27b..8b8c714a 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillPublishControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillPublishControllerTest.java @@ -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.>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(); + } + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java index 0807773c..bf1b655f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/SkillPackageArchiveExtractorTest.java @@ -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 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)); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java index 05b7ed16..d8c4ac64 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java @@ -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> 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); + } }