diff --git a/openspec/changes/refine-display-name-publishing/tasks.md b/openspec/changes/refine-display-name-publishing/tasks.md index 8fe4fb8f..7faae7e0 100644 --- a/openspec/changes/refine-display-name-publishing/tasks.md +++ b/openspec/changes/refine-display-name-publishing/tasks.md @@ -1,10 +1,10 @@ ## Implementation -- [ ] Add safe authenticated package preview and version-bound optional display name. -- [ ] Preserve snapshot compatibility and lifecycle metadata across approval/yank/promotion. -- [ ] Implement web prefill with user-edit and stale-response protection. -- [ ] Show one management status with pending metadata and full copyable detail coordinate. -- [ ] Update generated API types and user guides. +- [x] Add safe authenticated package preview and version-bound optional display name. +- [x] Preserve snapshot compatibility and lifecycle metadata across approval/yank/promotion. +- [x] Implement web prefill with user-edit and stale-response protection. +- [x] Show one management status with pending metadata and full copyable detail coordinate. +- [x] Update generated API types and user guides. ## Verification diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubRegistrySecurityConfig.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubRegistrySecurityConfig.java index 4f2c5969..128d838f 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubRegistrySecurityConfig.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubRegistrySecurityConfig.java @@ -9,7 +9,9 @@ import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.http.SessionCreationPolicy; import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; +import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; /** @@ -43,11 +45,14 @@ public class ClawHubRegistrySecurityConfig { HttpSecurity http, ApiTokenAuthenticationFilter apiTokenAuthenticationFilter) throws Exception { http - .securityMatcher( - "/api/v1/search", - "/api/v1/download", - "/api/v1/skills/*" - ) + .securityMatcher(new AndRequestMatcher( + new OrRequestMatcher( + new AntPathRequestMatcher("/api/v1/search"), + new AntPathRequestMatcher("/api/v1/download"), + new AntPathRequestMatcher("/api/v1/skills/*")), + new NegatedRequestMatcher(new AntPathRequestMatcher( + "/api/v1/skills/package-preview", HttpMethod.POST.name())) + )) .authorizeHttpRequests(auth -> auth.anyRequest().permitAll()) .requestCache(cache -> cache.disable()) .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerSecurityTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerSecurityTest.java index eb731e82..14d9f6f5 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerSecurityTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerSecurityTest.java @@ -39,6 +39,17 @@ class ClawHubCompatControllerSecurityTest { @MockBean private ClawHubCompatAppService clawHubCompatAppService; + @Test + void getPackagePreviewSlugStillUsesCompatibilityEndpoint() throws Exception { + when(clawHubCompatAppService.getSkill(eq("package-preview"), isNull(), isNull())) + .thenThrow(new DomainNotFoundException("error.skill.notFound", "package-preview")); + + mockMvc.perform(get("/api/v1/skills/package-preview")) + .andExpect(status().isNotFound()); + + verify(clawHubCompatAppService).getSkill(eq("package-preview"), isNull(), isNull()); + } + @Test void getSkill_returnsNotFound_whenAnonymousCannotAccessPrivateSkill() throws Exception { when(clawHubCompatAppService.getSkill(eq("priv"), isNull(), isNull())) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillPackagePreviewSecurityIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillPackagePreviewSecurityIntegrationTest.java new file mode 100644 index 00000000..ce8fdce2 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillPackagePreviewSecurityIntegrationTest.java @@ -0,0 +1,130 @@ +package com.iflytek.skillhub.controller.portal; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.token.ApiTokenService; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.dto.SkillPackagePreviewResponse; +import com.iflytek.skillhub.service.SkillPackagePreviewService; +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.mock.web.MockHttpSession; +import org.springframework.mock.web.MockMultipartFile; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** Exercises real ordered filter chains, scope parsing, and stored browser sessions. + * Token lifecycle verification is stubbed; these tests focus on routing and access control. + */ +@SpringBootTest(properties = "skillhub.ratelimit.enabled=false") +@AutoConfigureMockMvc +@ActiveProfiles("test") +class SkillPackagePreviewSecurityIntegrationTest { + @Autowired MockMvc mockMvc; + @MockBean ApiTokenService tokens; + @Autowired UserAccountRepository users; + @MockBean SkillPackagePreviewService previews; + private String userId; + + @BeforeEach + void setUp() { + userId = "preview-security-" + UUID.randomUUID(); + users.save(new UserAccount(userId, "Preview user", userId + "@example.test", "")); + } + + @ParameterizedTest + @ValueSource(strings = {"/api/v1", "/api/web"}) + void browserSessionWithCsrfCanPreview(String prefix) throws Exception { + given(previews.preview(anyList())).willReturn(new SkillPackagePreviewResponse("foo", "中文标题", "Summary", "1.0.0")); + mockMvc.perform(multipart(prefix + "/skills/package-preview").file(archive()) + .session(session()).with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.displayName").value("中文标题")); + verify(previews).preview(anyList()); + } + + @ParameterizedTest + @ValueSource(strings = {"/api/v1", "/api/web"}) + void publishScopedTokenCanPreviewWithoutSessionOrCsrf(String prefix) throws Exception { + given(previews.preview(anyList())).willReturn(new SkillPackagePreviewResponse("foo", "中文标题", "Summary", "1.0.0")); + mockMvc.perform(multipart(prefix + "/skills/package-preview").file(archive()) + .header("Authorization", "Bearer " + token("skill:publish"))) + .andExpect(status().isOk()); + verify(previews).preview(anyList()); + } + + @ParameterizedTest + @ValueSource(strings = {"/api/v1", "/api/web"}) + void missingPublishScopeIsRejectedBeforeParsing(String prefix) throws Exception { + mockMvc.perform(multipart(prefix + "/skills/package-preview").file(archive()) + .header("Authorization", "Bearer " + token("skill:read"))) + .andExpect(status().isForbidden()); + verifyNoInteractions(previews); + } + + @ParameterizedTest + @ValueSource(strings = {"/api/v1", "/api/web"}) + void anonymousIsRejectedBeforeParsing(String prefix) throws Exception { + mockMvc.perform(multipart(prefix + "/skills/package-preview").file(archive()).with(csrf())) + .andExpect(status().isUnauthorized()); + verifyNoInteractions(previews); + } + + @ParameterizedTest + @ValueSource(strings = {"/api/v1", "/api/web"}) + void browserSessionWithoutCsrfIsRejectedBeforeParsing(String prefix) throws Exception { + mockMvc.perform(multipart(prefix + "/skills/package-preview").file(archive()).session(session())) + .andExpect(status().is4xxClientError()); + verifyNoInteractions(previews); + } + + private MockHttpSession session() { + var context = SecurityContextHolder.createEmptyContext(); + var principal = new PlatformPrincipal(userId, "Preview user", userId + "@example.test", "", "local", Set.of()); + context.setAuthentication(new UsernamePasswordAuthenticationToken(principal, null, List.of())); + var session = new MockHttpSession(); + session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, context); + return session; + } + + private String token(String scope) { + String rawToken = "preview-test-" + UUID.randomUUID(); + var token = new com.iflytek.skillhub.auth.entity.ApiToken(userId, "preview", "preview", "test-hash", + "[\"" + scope + "\"]"); + given(tokens.validateToken(rawToken)).willReturn(java.util.Optional.of(token)); + return rawToken; + } + + private MockMultipartFile archive() throws Exception { + try (var output = new ByteArrayOutputStream(); var zip = new ZipOutputStream(output)) { + zip.putNextEntry(new ZipEntry("SKILL.md")); + zip.write("---\nname: foo\ndescription: Summary\n---\nBody".getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + zip.finish(); + return new MockMultipartFile("file", "skill.zip", "application/zip", output.toByteArray()); + } + } +} 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 57701679..21dbb87c 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 @@ -293,7 +293,7 @@ class SkillPublishControllerTest { mockMvc.perform(multipart("/api/v1/skills/package-preview") .file(new MockMultipartFile("file", "skill.zip", "application/zip", buildZipBytes())) .with(csrf())) - .andExpect(status().isTooManyRequests()); + .andExpect(status().isUnauthorized()); org.mockito.Mockito.verifyNoInteractions(skillPublishService); } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java index 23536a47..8e54fc51 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java @@ -178,6 +178,8 @@ public class RouteSecurityPolicyRegistry { ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/whoami"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/search"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/skills"), + ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/skills/package-preview", "skill:publish"), + ApiTokenPolicy.require(HttpMethod.POST, "/api/web/skills/package-preview", "skill:publish"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/skills/**"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/skills"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/skills/**"), @@ -302,9 +304,7 @@ public class RouteSecurityPolicyRegistry { "ANY /api/v1/auth/local/**", "ANY /api/v1/admin/**", "POST /api/v1/skills/avatar-uploads", - "POST /api/v1/skills/package-preview", "POST /api/web/skills/avatar-uploads", - "POST /api/web/skills/package-preview", "POST /api/v1/suites/reviews/*/approve", "POST /api/v1/suites/reviews/*/reject", "POST /api/web/suites/reviews/*/approve", diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java index bf3d7c3a..d7c5813b 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java @@ -19,11 +19,12 @@ class RouteSecurityPolicyRegistryTest { private final RouteSecurityPolicyRegistry registry = new RouteSecurityPolicyRegistry(); @Test - void packagePreviewIsAuthenticatedAndSessionOnly() { + void packagePreviewIsAuthenticatedAndRequiresPublishScope() { for (String prefix : java.util.List.of("/api/v1", "/api/web")) { assertEquals(RouteSecurityPolicyRegistry.AccessLevel.AUTHENTICATED, registry.accessLevel("POST", prefix + "/skills/package-preview")); - assertFalse(registry.authorizeApiToken("POST", prefix + "/skills/package-preview", Set.of("skill:read", "skill:publish", "skill:delete", "token:manage")).allowed()); + assertFalse(registry.authorizeApiToken("POST", prefix + "/skills/package-preview", Set.of("skill:read")).allowed()); + assertTrue(registry.authorizeApiToken("POST", prefix + "/skills/package-preview", Set.of("skill:publish")).allowed()); } } diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 8fec1ae6..0760ee63 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -1831,6 +1831,7 @@ "copyright": "© 2026 SkillHub. Apache-2.0." }, "publish": { + "retryPackagePreview": "Retry", "packageName": "Package name", "displayName": "Display name", "displayNamePlaceholder": "For example: Meeting notes assistant", diff --git a/web/src/i18n/locales/ru.json b/web/src/i18n/locales/ru.json index ef4b6248..da1bde32 100644 --- a/web/src/i18n/locales/ru.json +++ b/web/src/i18n/locales/ru.json @@ -1822,6 +1822,7 @@ "goToPage": "Перейти на страницу {{page}}" }, "publish": { + "retryPackagePreview": "Повторить", "packageName": "Имя в пакете", "displayName": "Отображаемое имя", "displayNamePlaceholder": "Например: Помощник для протоколов встреч", diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index dc45fd32..0f429519 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -1830,6 +1830,7 @@ "copyright": "© 2026 SkillHub. Apache-2.0." }, "publish": { + "retryPackagePreview": "重试", "packageName": "包内名称", "displayName": "显示名称", "displayNamePlaceholder": "例如:会议纪要助手", diff --git a/web/src/pages/dashboard/publish-display-name.test.tsx b/web/src/pages/dashboard/publish-display-name.test.tsx index 6b450ead..9c35e1f7 100644 --- a/web/src/pages/dashboard/publish-display-name.test.tsx +++ b/web/src/pages/dashboard/publish-display-name.test.tsx @@ -67,7 +67,7 @@ describe('web display name publishing', () => { mount(); select('invalid.zip') await screen.findByRole('alert') expect(screen.getByRole('button', { name: 'publish.confirm' }).hasAttribute('disabled')).toBe(true) - fireEvent.click(screen.getByRole('button', { name: 'common.retry' })) + fireEvent.click(screen.getByRole('button', { name: 'publish.retryPackagePreview' })) await waitFor(() => expect((screen.getByLabelText('publish.displayName') as HTMLInputElement).value).toBe('重试后的名称')) }) }) diff --git a/web/src/pages/dashboard/publish.tsx b/web/src/pages/dashboard/publish.tsx index 85cbd695..c3cdf974 100644 --- a/web/src/pages/dashboard/publish.tsx +++ b/web/src/pages/dashboard/publish.tsx @@ -328,7 +328,7 @@ export function PublishPage() { {selectedFile && packagePreview.isPending &&

{t('publish.readingPackage')}

} {selectedFile && packagePreview.isError &&

{t('publish.packagePreviewFailed')}

- +
} {selectedFile && packagePreview.data &&

{t('publish.packageName')}: {packagePreview.data.name}

}