fix(api): route package preview through authenticated policy chain

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
2026-10-16 04:28:39 +00:00 · 2026-10-15 16:47:29 +08:00 · 2026-10-15 16:47:29 +08:00 · e50d54deb5
commit e50d54deb5
parent 793a519bdb
12 changed files with 167 additions and 17 deletions
--- 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

--- 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))
--- 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()))
--- 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
@ -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());
+        }
+    }
+}
--- 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);
    }

--- 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",
--- 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());
        }
    }

--- 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",
--- 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": "Например: Помощник для протоколов встреч",
--- 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": "例如：会议纪要助手",
--- 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('重试后的名称'))
  })
 })
--- a/web/src/pages/dashboard/publish.tsx
+++ b/web/src/pages/dashboard/publish.tsx
@ -328,7 +328,7 @@ export function PublishPage() {
          {selectedFile && packagePreview.isPending && <p role="status" className="text-sm text-muted-foreground">{t('publish.readingPackage')}</p>}
          {selectedFile && packagePreview.isError && <div role="alert" className="text-sm text-destructive">
            <p>{t('publish.packagePreviewFailed')}</p>
-            <Button type="button" size="sm" variant="outline" onClick={() => packagePreview.refetch()}>{t('common.retry')}</Button>
+            <Button type="button" size="sm" variant="outline" onClick={() => packagePreview.refetch()}>{t('publish.retryPackagePreview')}</Button>
          </div>}
          {selectedFile && packagePreview.data && <p className="break-all text-xs text-muted-foreground">{t('publish.packageName')}: <code>{packagePreview.data.name}</code></p>}
        </div>