From 680a5d1b9433cacf3015cbc3dc8ffe82b5cfbc2a Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 3 Sep 2026 17:55:08 +0800 Subject: [PATCH 1/6] feat(security): retry failed scans Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../portal/SecurityAuditController.java | 28 ++- .../service/SecurityScanRetryAppService.java | 123 +++++++++++++ .../src/main/resources/messages.properties | 4 + .../src/main/resources/messages_ru.properties | 4 + .../src/main/resources/messages_zh.properties | 4 + .../portal/SecurityAuditControllerTest.java | 35 ++++ .../SecurityScanRetryAppServiceTest.java | 172 ++++++++++++++++++ .../domain/security/SecurityScanService.java | 29 ++- .../security/SecurityScanServiceTest.java | 44 +++++ .../security-audit-summary.test.tsx | 16 ++ .../security-audit/security-audit-summary.tsx | 23 ++- .../security-audit/use-security-audit.ts | 15 +- web/src/i18n/locales/en.json | 4 + web/src/i18n/locales/ru.json | 4 + web/src/i18n/locales/zh.json | 4 + web/src/pages/skill-detail.tsx | 12 +- 16 files changed, 513 insertions(+), 8 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryAppServiceTest.java diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SecurityAuditController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SecurityAuditController.java index 3666c46c..9ea74d6d 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SecurityAuditController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SecurityAuditController.java @@ -19,9 +19,14 @@ import com.iflytek.skillhub.domain.skill.VisibilityChecker; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.SecurityAuditResponse; +import com.iflytek.skillhub.dto.SkillLifecycleMutationResponse; +import com.iflytek.skillhub.service.AuditRequestContext; +import com.iflytek.skillhub.service.SecurityScanRetryAppService; +import jakarta.servlet.http.HttpServletRequest; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestAttribute; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; @@ -41,19 +46,40 @@ public class SecurityAuditController extends BaseApiController { private final SkillVersionRepository skillVersionRepository; private final VisibilityChecker visibilityChecker; private final ObjectMapper objectMapper; + private final SecurityScanRetryAppService securityScanRetryAppService; public SecurityAuditController(SecurityAuditRepository securityAuditRepository, SkillRepository skillRepository, SkillVersionRepository skillVersionRepository, VisibilityChecker visibilityChecker, ApiResponseFactory responseFactory, - ObjectMapper objectMapper) { + ObjectMapper objectMapper, + SecurityScanRetryAppService securityScanRetryAppService) { super(responseFactory); this.securityAuditRepository = securityAuditRepository; this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; this.visibilityChecker = visibilityChecker; this.objectMapper = objectMapper; + this.securityScanRetryAppService = securityScanRetryAppService; + } + + @PostMapping("/retry") + public ApiResponse retrySecurityScan( + @PathVariable Long skillId, + @PathVariable Long versionId, + @AuthenticationPrincipal PlatformPrincipal principal, + @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles, + HttpServletRequest request) { + SkillLifecycleMutationResponse result = securityScanRetryAppService.retry( + skillId, + versionId, + principal.userId(), + principal.platformRoles(), + userNsRoles, + AuditRequestContext.from(request) + ); + return ok("security_audit.retry.started", result); } @GetMapping diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java new file mode 100644 index 00000000..7c766bdb --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java @@ -0,0 +1,123 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.domain.audit.AuditDetail; +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.security.ScanTask; +import com.iflytek.skillhub.domain.security.ScannerType; +import com.iflytek.skillhub.domain.security.SecurityAuditRepository; +import com.iflytek.skillhub.domain.security.SecurityScanService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.dto.SkillLifecycleMutationResponse; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.util.Map; +import java.util.Set; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +@Service +public class SecurityScanRetryAppService { + + private final SkillRepository skillRepository; + private final SkillVersionRepository skillVersionRepository; + private final SecurityAuditRepository securityAuditRepository; + private final SecurityScanService securityScanService; + private final ObjectStorageService objectStorageService; + private final AuditLogService auditLogService; + + public SecurityScanRetryAppService(SkillRepository skillRepository, + SkillVersionRepository skillVersionRepository, + SecurityAuditRepository securityAuditRepository, + SecurityScanService securityScanService, + ObjectStorageService objectStorageService, + AuditLogService auditLogService) { + this.skillRepository = skillRepository; + this.skillVersionRepository = skillVersionRepository; + this.securityAuditRepository = securityAuditRepository; + this.securityScanService = securityScanService; + this.objectStorageService = objectStorageService; + this.auditLogService = auditLogService; + } + + @Transactional + public SkillLifecycleMutationResponse retry(Long skillId, + Long versionId, + String userId, + Set platformRoles, + Map namespaceRoles, + AuditRequestContext auditContext) { + Skill skill = skillRepository.findById(skillId) + .orElseThrow(() -> new DomainBadRequestException("error.skill.notFound", skillId)); + authorize(skill, userId, platformRoles, namespaceRoles); + SkillVersion version = skillVersionRepository.findBySkillIdForUpdate(skillId).stream() + .filter(candidate -> candidate.getId().equals(versionId)) + .findFirst() + .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", versionId)); + + if (version.getStatus() == SkillVersionStatus.SCANNING && hasActiveAttempt(versionId)) { + return response(skillId, versionId); + } + if (version.getStatus() != SkillVersionStatus.SCAN_FAILED) { + throw new DomainBadRequestException("error.security.scan.retry.status", version.getStatus()); + } + if (!securityScanService.isEnabled()) { + throw new DomainBadRequestException("error.security.scan.retry.disabled"); + } + + String bundleKey = bundleKey(skillId, versionId); + if (!objectStorageService.exists(bundleKey)) { + throw new DomainBadRequestException("error.security.scan.retry.bundleMissing"); + } + + ScanTask task = securityScanService.retryStoredBundleScan(version, bundleKey, userId); + auditLogService.record( + userId, + "RETRY_SECURITY_SCAN", + "SKILL_VERSION", + versionId, + null, + auditContext.clientIp(), + auditContext.userAgent(), + AuditDetail.of("taskId", task.taskId(), "version", version.getVersion()) + ); + return response(skillId, versionId); + } + + private void authorize(Skill skill, + String userId, + Set platformRoles, + Map namespaceRoles) { + Set roles = platformRoles != null ? platformRoles : Set.of(); + Map memberships = namespaceRoles != null ? namespaceRoles : Map.of(); + NamespaceRole namespaceRole = memberships.get(skill.getNamespaceId()); + boolean allowed = skill.getOwnerId().equals(userId) + || namespaceRole == NamespaceRole.OWNER + || namespaceRole == NamespaceRole.ADMIN + || roles.contains("SUPER_ADMIN") + || roles.contains("SKILL_ADMIN"); + if (!allowed) { + throw new DomainForbiddenException("error.forbidden"); + } + } + + private boolean hasActiveAttempt(Long versionId) { + return securityAuditRepository + .findLatestActiveByVersionIdAndScannerType(versionId, ScannerType.SKILL_SCANNER) + .filter(audit -> audit.getScannedAt() == null) + .isPresent(); + } + + private String bundleKey(Long skillId, Long versionId) { + return String.format("packages/%d/%d/bundle.zip", skillId, versionId); + } + + private SkillLifecycleMutationResponse response(Long skillId, Long versionId) { + return new SkillLifecycleMutationResponse(skillId, versionId, "RETRY_SECURITY_SCAN", "SCANNING"); + } +} diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index c6d80a2a..0f807c91 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -102,6 +102,10 @@ error.skill.publish.skillMd.notFound=SKILL.md not found error.skill.publish.precheck.confirmRequired=Pre-publish warnings require confirmation before publishing:\n{0} error.skill.publish.precheck.failed=Pre-publish validation failed: {0} error.security.scanner.required=Security scanner must be enabled before publishing public or namespace-visible skills +error.security.scan.retry.status=Only a failed security scan can be retried (current status: {0}) +error.security.scan.retry.disabled=Security scanning is disabled; enable it before retrying +error.security.scan.retry.bundleMissing=The stored package is unavailable; upload the skill again to retry scanning +security_audit.retry.started=Security scan retry started error.skill.publish.archived=Archived skill must be restored before publishing: {0} review.withdraw.not_pending=Only pending review submissions can be withdrawn: {0} review.withdraw.not_submitter=Only the submitter can withdraw this review diff --git a/server/skillhub-app/src/main/resources/messages_ru.properties b/server/skillhub-app/src/main/resources/messages_ru.properties index 0302c2f5..f2416492 100644 --- a/server/skillhub-app/src/main/resources/messages_ru.properties +++ b/server/skillhub-app/src/main/resources/messages_ru.properties @@ -99,6 +99,10 @@ error.skill.publish.skillMd.notFound=SKILL.md не найден error.skill.publish.precheck.confirmRequired=Предупреждения перед публикацией требуют подтверждения:\n{0} error.skill.publish.precheck.failed=Проверка перед публикацией не пройдена: {0} error.security.scanner.required=Перед публикацией публичных или видимых в пространстве имён скиллов необходимо включить сканер безопасности +error.security.scan.retry.status=Повторить можно только неудачное сканирование безопасности (текущий статус: {0}) +error.security.scan.retry.disabled=Сканер безопасности отключён; включите его перед повторной попыткой +error.security.scan.retry.bundleMissing=Сохранённый пакет недоступен; загрузите скилл заново для повторного сканирования +security_audit.retry.started=Повторное сканирование безопасности запущено error.skill.publish.archived=Архивный скилл нужно восстановить перед публикацией: {0} review.withdraw.not_pending=Отозвать можно только заявки на ревью со статусом pending: {0} review.withdraw.not_submitter=Отозвать это ревью может только отправитель diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 8b3e7a66..5fb0f83f 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -102,6 +102,10 @@ error.skill.publish.skillMd.notFound=未找到 SKILL.md error.skill.publish.precheck.confirmRequired=预发布发现以下风险提醒，确认后仍可继续发布：\n{0} error.skill.publish.precheck.failed=预发布校验失败：{0} error.security.scanner.required=发布公开或命名空间可见技能前必须启用安全扫描器 +error.security.scan.retry.status=只有安全扫描失败的版本才能重试（当前状态：{0}） +error.security.scan.retry.disabled=安全扫描器未启用，请启用后再重试 +error.security.scan.retry.bundleMissing=原技能包已不存在，请重新上传技能后再扫描 +security_audit.retry.started=已重新发起安全扫描 error.skill.publish.archived=该技能已归档，请先恢复后再发布：{0} review.withdraw.not_pending=只有待审核版本才能撤销审核：{0} review.withdraw.not_submitter=只有提交人本人可以撤销此次审核 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SecurityAuditControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SecurityAuditControllerTest.java index eb5f87d6..e19be679 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SecurityAuditControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SecurityAuditControllerTest.java @@ -15,6 +15,8 @@ import com.iflytek.skillhub.domain.skill.SkillStatus; import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.dto.SkillLifecycleMutationResponse; +import com.iflytek.skillhub.service.SecurityScanRetryAppService; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; @@ -34,7 +36,9 @@ import java.util.Set; import static org.mockito.BDDMockito.given; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -61,6 +65,37 @@ class SecurityAuditControllerTest { @MockBean private NamespaceMemberRepository namespaceMemberRepository; + @MockBean + private SecurityScanRetryAppService securityScanRetryAppService; + + @Test + void retrySecurityScan_returnsScanningState() throws Exception { + given(securityScanRetryAppService.retry( + org.mockito.ArgumentMatchers.eq(8L), + org.mockito.ArgumentMatchers.eq(42L), + org.mockito.ArgumentMatchers.eq("owner-1"), + org.mockito.ArgumentMatchers.eq(Set.of()), + org.mockito.ArgumentMatchers.anyMap(), + org.mockito.ArgumentMatchers.any())) + .willReturn(new SkillLifecycleMutationResponse(8L, 42L, "RETRY_SECURITY_SCAN", "SCANNING")); + + mockMvc.perform(post("/api/v1/skills/8/versions/42/security-audit/retry") + .with(auth("owner-1")) + .with(csrf()) + .requestAttr("userNsRoles", Map.of())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.action").value("RETRY_SECURITY_SCAN")) + .andExpect(jsonPath("$.data.status").value("SCANNING")); + } + + @Test + void retrySecurityScan_requiresAuthentication() throws Exception { + mockMvc.perform(post("/api/v1/skills/8/versions/42/security-audit/retry").with(csrf())) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + } + @Test void getSecurityAudit_returnsAuditPayload() throws Exception { SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryAppServiceTest.java new file mode 100644 index 00000000..87693db6 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryAppServiceTest.java @@ -0,0 +1,172 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.security.ScanTask; +import com.iflytek.skillhub.domain.security.ScannerType; +import com.iflytek.skillhub.domain.security.SecurityAudit; +import com.iflytek.skillhub.domain.security.SecurityAuditRepository; +import com.iflytek.skillhub.domain.security.SecurityScanService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.lang.reflect.Field; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +@ExtendWith(MockitoExtension.class) +class SecurityScanRetryAppServiceTest { + + @Mock private SkillRepository skillRepository; + @Mock private SkillVersionRepository skillVersionRepository; + @Mock private SecurityAuditRepository securityAuditRepository; + @Mock private SecurityScanService securityScanService; + @Mock private ObjectStorageService objectStorageService; + @Mock private AuditLogService auditLogService; + + private SecurityScanRetryAppService service; + private Skill skill; + private SkillVersion version; + + @BeforeEach + void setUp() { + service = new SecurityScanRetryAppService( + skillRepository, + skillVersionRepository, + securityAuditRepository, + securityScanService, + objectStorageService, + auditLogService + ); + skill = skill(8L, "owner-1"); + version = version(42L, SkillVersionStatus.SCAN_FAILED); + given(skillRepository.findById(8L)).willReturn(Optional.of(skill)); + } + + @Test + void retry_asOwnerCreatesNewAttemptAndAuditLog() { + given(skillVersionRepository.findBySkillIdForUpdate(8L)).willReturn(List.of(version)); + given(securityScanService.isEnabled()).willReturn(true); + given(objectStorageService.exists("packages/8/42/bundle.zip")).willReturn(true); + given(securityScanService.retryStoredBundleScan(version, "packages/8/42/bundle.zip", "owner-1")) + .willReturn(new ScanTask("task-new", 42L, null, "packages/8/42/bundle.zip", + "owner-1", 1L, Map.of())); + + var result = service.retry( + 8L, 42L, "owner-1", Set.of(), Map.of(), new AuditRequestContext("127.0.0.1", "test")); + + assertThat(result.status()).isEqualTo("SCANNING"); + verify(securityScanService).retryStoredBundleScan(version, "packages/8/42/bundle.zip", "owner-1"); + verify(auditLogService).record( + "owner-1", "RETRY_SECURITY_SCAN", "SKILL_VERSION", 42L, + null, "127.0.0.1", "test", "{\"taskId\":\"task-new\",\"version\":\"1.0.0\"}"); + } + + @Test + void retry_allowsNamespaceAdminAndPlatformSecurityAdmin() { + given(skillVersionRepository.findBySkillIdForUpdate(8L)).willReturn(List.of(version)); + given(securityScanService.isEnabled()).willReturn(true); + given(objectStorageService.exists("packages/8/42/bundle.zip")).willReturn(true); + given(securityScanService.retryStoredBundleScan(any(), any(), any())) + .willReturn(new ScanTask("task-new", 42L, null, "bundle", "admin", 1L, Map.of())); + + service.retry(8L, 42L, "namespace-admin", Set.of(), Map.of(5L, NamespaceRole.ADMIN), + new AuditRequestContext(null, null)); + version.setStatus(SkillVersionStatus.SCAN_FAILED); + service.retry(8L, 42L, "security-admin", Set.of("SKILL_ADMIN"), Map.of(), + new AuditRequestContext(null, null)); + + verify(securityScanService, org.mockito.Mockito.times(2)).retryStoredBundleScan(any(), any(), any()); + } + + @Test + void retry_rejectsUnauthorizedUserBeforeReadingVersionState() { + assertThatThrownBy(() -> service.retry( + 8L, 42L, "viewer", Set.of(), Map.of(), new AuditRequestContext(null, null))) + .isInstanceOf(DomainForbiddenException.class); + + verify(skillVersionRepository, never()).findBySkillIdForUpdate(any()); + } + + @Test + void retry_rejectsNonFailedVersion() { + version.setStatus(SkillVersionStatus.PENDING_REVIEW); + given(skillVersionRepository.findBySkillIdForUpdate(8L)).willReturn(List.of(version)); + + assertThatThrownBy(() -> service.retry( + 8L, 42L, "owner-1", Set.of(), Map.of(), new AuditRequestContext(null, null))) + .isInstanceOf(DomainBadRequestException.class); + + verify(securityScanService, never()).retryStoredBundleScan(any(), any(), any()); + } + + @Test + void retry_rejectsMissingStoredBundle() { + given(skillVersionRepository.findBySkillIdForUpdate(8L)).willReturn(List.of(version)); + given(securityScanService.isEnabled()).willReturn(true); + + assertThatThrownBy(() -> service.retry( + 8L, 42L, "owner-1", Set.of(), Map.of(), new AuditRequestContext(null, null))) + .isInstanceOf(DomainBadRequestException.class); + + verify(securityScanService, never()).retryStoredBundleScan(any(), any(), any()); + } + + @Test + void retry_whenAttemptAlreadyStartedReturnsCurrentStateWithoutDuplicateTask() { + version.setStatus(SkillVersionStatus.SCANNING); + given(skillVersionRepository.findBySkillIdForUpdate(8L)).willReturn(List.of(version)); + given(securityAuditRepository.findLatestActiveByVersionIdAndScannerType(42L, ScannerType.SKILL_SCANNER)) + .willReturn(Optional.of(new SecurityAudit(42L, ScannerType.SKILL_SCANNER, "task-existing"))); + + var result = service.retry( + 8L, 42L, "owner-1", Set.of(), Map.of(), new AuditRequestContext(null, null)); + + assertThat(result.status()).isEqualTo("SCANNING"); + verify(securityScanService, never()).retryStoredBundleScan(any(), any(), any()); + verify(auditLogService, never()).record(any(), any(), any(), any(), any(), any(), any(), any()); + } + + private Skill skill(Long id, String ownerId) { + Skill value = new Skill(5L, "demo", ownerId, SkillVisibility.PRIVATE); + setField(value, "id", id); + return value; + } + + private SkillVersion version(Long id, SkillVersionStatus status) { + SkillVersion value = new SkillVersion(8L, "1.0.0", "owner-1"); + setField(value, "id", id); + value.setStatus(status); + return value; + } + + private void setField(Object target, String name, Object value) { + try { + Field field = target.getClass().getDeclaredField(name); + field.setAccessible(true); + field.set(target, value); + } catch (ReflectiveOperationException e) { + throw new AssertionError(e); + } + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java index 61a8578a..59e9c274 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java @@ -69,6 +69,27 @@ public class SecurityScanService { return enabled; } + @Transactional + public ScanTask retryStoredBundleScan(SkillVersion version, String bundleKey, String publisherId) { + if (!enabled) { + throw new IllegalStateException("Security scanner is disabled"); + } + if (version.getStatus() != SkillVersionStatus.SCAN_FAILED) { + throw new IllegalStateException("Only SCAN_FAILED versions can be retried"); + } + ScanTask scanTask = new ScanTask( + UUID.randomUUID().toString(), + version.getId(), + null, + bundleKey, + publisherId, + System.currentTimeMillis(), + Map.of("scannerType", ScannerType.SKILL_SCANNER.getValue()) + ); + persistScanAttempt(version, scanTask); + return scanTask; + } + @Transactional public void triggerScan(Long versionId, List entries, String publisherId) { if (!enabled) { @@ -87,7 +108,6 @@ public class SecurityScanService { } else { packagePath = saveTempDirectory(versionId, entries).toString(); } - // Always create a new audit record — supports multiple rounds per version final ScanTask scanTask = new ScanTask( UUID.randomUUID().toString(), versionId, @@ -97,7 +117,12 @@ public class SecurityScanService { System.currentTimeMillis(), Map.of("scannerType", ScannerType.SKILL_SCANNER.getValue()) ); - auditRepository.save(new SecurityAudit(versionId, ScannerType.SKILL_SCANNER, scanTask.taskId())); + persistScanAttempt(version, scanTask); + } + + private void persistScanAttempt(SkillVersion version, ScanTask scanTask) { + // A new record preserves prior scan history while identifying this attempt independently. + auditRepository.save(new SecurityAudit(version.getId(), ScannerType.SKILL_SCANNER, scanTask.taskId())); if (scanTaskOutboxRepository != null) { scanTaskOutboxRepository.save(new ScanTaskOutbox(scanTask)); } else { diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java index b5e90381..cd98b43f 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java @@ -136,6 +136,50 @@ class SecurityScanServiceTest { assertThat(task.bundleKey()).isEqualTo("packages/8/42/bundle.zip"); } + @Test + void retryStoredBundleScan_createsFreshAuditAndDurableOutbox() throws Exception { + ScanTaskOutboxRepository outboxRepository = org.mockito.Mockito.mock(ScanTaskOutboxRepository.class); + service = new SecurityScanService( + auditRepository, + skillVersionRepository, + scanTaskProducer, + new ObjectMapper(), + "local", + true, + outboxRepository + ); + SkillVersion version = new SkillVersion(8L, "1.0.0", "owner-1"); + setId(version, 42L); + version.setStatus(SkillVersionStatus.SCAN_FAILED); + + ScanTask task = service.retryStoredBundleScan(version, "packages/8/42/bundle.zip", "owner-1"); + + ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(SecurityAudit.class); + ArgumentCaptor outboxCaptor = ArgumentCaptor.forClass(ScanTaskOutbox.class); + verify(auditRepository).save(auditCaptor.capture()); + verify(outboxRepository).save(outboxCaptor.capture()); + verify(scanTaskProducer, never()).publishScanTask(any()); + verify(skillVersionRepository).save(version); + assertThat(auditCaptor.getValue().getTaskId()).isEqualTo(task.taskId()); + assertThat(outboxCaptor.getValue().toScanTask()).isEqualTo(task); + assertThat(task.bundleKey()).isEqualTo("packages/8/42/bundle.zip"); + assertThat(task.metadata()).containsEntry("scannerType", "skill-scanner"); + assertThat(version.getStatus()).isEqualTo(SkillVersionStatus.SCANNING); + } + + @Test + void retryStoredBundleScan_rejectsNonFailedVersion() throws Exception { + SkillVersion version = new SkillVersion(8L, "1.0.0", "owner-1"); + setId(version, 42L); + version.setStatus(SkillVersionStatus.SCANNING); + + assertThatThrownBy(() -> service.retryStoredBundleScan(version, "bundle.zip", "owner-1")) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("SCAN_FAILED"); + + verify(auditRepository, never()).save(any()); + } + @Test void triggerScan_defersTaskPublishingUntilTransactionCommit() throws Exception { SkillVersion version = new SkillVersion(8L, "1.0.0", "publisher-1"); diff --git a/web/src/features/security-audit/security-audit-summary.test.tsx b/web/src/features/security-audit/security-audit-summary.test.tsx index b75f7ab3..a20a51fe 100644 --- a/web/src/features/security-audit/security-audit-summary.test.tsx +++ b/web/src/features/security-audit/security-audit-summary.test.tsx @@ -34,9 +34,11 @@ function createAudit(overrides: Partial = {}): SecurityAudi } let mockAudits: SecurityAuditRecord[] | undefined = undefined +const retryMutation = { mutate: vi.fn(), isPending: false } vi.mock('./use-security-audit', () => ({ useSecurityAudits: () => ({ data: mockAudits }), + useRetrySecurityScan: () => retryMutation, })) // Mock the Dialog components to avoid Radix UI portal / context issues in static render @@ -116,6 +118,20 @@ describe('SecurityAuditSummary', () => { expect(html).not.toContain('securityAudit.statusScanning') }) + it('renders retry only for an authorized failed version', () => { + mockAudits = [createAudit({ scannedAt: null })] + + const failedHtml = renderToStaticMarkup( + + ) + const unauthorizedHtml = renderToStaticMarkup( + + ) + + expect(failedHtml).toContain('securityAudit.retry') + expect(unauthorizedHtml).not.toContain('securityAudit.retry') + }) + it('renders the total findings count across all audits', () => { mockAudits = [ createAudit({ id: 1, findingsCount: 3 }), diff --git a/web/src/features/security-audit/security-audit-summary.tsx b/web/src/features/security-audit/security-audit-summary.tsx index c9f306d4..9830751a 100644 --- a/web/src/features/security-audit/security-audit-summary.tsx +++ b/web/src/features/security-audit/security-audit-summary.tsx @@ -4,7 +4,8 @@ import { Shield } from 'lucide-react' import { Card } from '@/shared/ui/card' import { Button } from '@/shared/ui/button' import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription } from '@/shared/ui/dialog' -import { useSecurityAudits } from './use-security-audit' +import { toast } from '@/shared/lib/toast' +import { useRetrySecurityScan, useSecurityAudits } from './use-security-audit' import { getSecurityAuditDisplayState } from './display-state' import { VerdictBadge } from './verdict-badge' import { SecurityAuditSection } from './security-audit-section' @@ -13,12 +14,14 @@ interface SecurityAuditSummaryProps { skillId: number versionId: number versionStatus?: string + canRetry?: boolean } -export function SecurityAuditSummary({ skillId, versionId, versionStatus }: SecurityAuditSummaryProps) { +export function SecurityAuditSummary({ skillId, versionId, versionStatus, canRetry = false }: SecurityAuditSummaryProps) { const { t } = useTranslation() const { data: audits } = useSecurityAudits(skillId, versionId) const [dialogOpen, setDialogOpen] = useState(false) + const retryMutation = useRetrySecurityScan(skillId, versionId) if (!audits || audits.length === 0) { return null @@ -49,6 +52,22 @@ export function SecurityAuditSummary({ skillId, versionId, versionStatus }: Secu

{t('securityAudit.totalFindings', { count: totalFindings })}

+ {canRetry && versionStatus === 'SCAN_FAILED' && ( + + )} diff --git a/web/src/features/security-audit/use-security-audit.ts b/web/src/features/security-audit/use-security-audit.ts index 4098f3e6..88bc3278 100644 --- a/web/src/features/security-audit/use-security-audit.ts +++ b/web/src/features/security-audit/use-security-audit.ts @@ -1,4 +1,4 @@ -import { useQuery } from '@tanstack/react-query' +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import { ApiError, fetchJson } from '@/api/client' import type { SecurityAuditRecord } from './types' @@ -31,3 +31,16 @@ export function useSecurityAudits( retry: false, }) } + +export function useRetrySecurityScan(skillId: number, versionId: number) { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: () => fetchJson(`/api/v1/skills/${skillId}/versions/${versionId}/security-audit/retry`, { + method: 'POST', + }), + onSuccess: () => { + void queryClient.invalidateQueries({ queryKey: ['security-audits', skillId, versionId] }) + void queryClient.invalidateQueries({ queryKey: ['skills'] }) + }, + }) +} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index e8b12806..3f0006ad 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -1589,6 +1589,10 @@ "statusScanning": "Scanning", "statusScanFailed": "Scan Failed", "failureReason": "Reason: {{reason}}", + "retry": "Retry scan", + "retrying": "Retrying...", + "retrySuccess": "Security scan restarted", + "retryError": "Could not retry security scan", "remediation": "Remediation", "viewDetails": "View Details", "verdict": { diff --git a/web/src/i18n/locales/ru.json b/web/src/i18n/locales/ru.json index c40a39c6..899e94b4 100644 --- a/web/src/i18n/locales/ru.json +++ b/web/src/i18n/locales/ru.json @@ -1620,6 +1620,10 @@ "statusScanning": "Сканирование", "statusScanFailed": "Сканирование не удалось", "failureReason": "Причина: {{reason}}", + "retry": "Повторить сканирование", + "retrying": "Повторное сканирование...", + "retrySuccess": "Сканирование запущено повторно", + "retryError": "Не удалось повторить сканирование", "remediation": "Рекомендации", "viewDetails": "Подробности", "verdict": { diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index b34b0c07..24dd2c9e 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -1588,6 +1588,10 @@ "statusScanning": "扫描中", "statusScanFailed": "扫描失败", "failureReason": "失败原因：{{reason}}", + "retry": "重新扫描", + "retrying": "正在重新扫描...", + "retrySuccess": "已重新发起安全扫描", + "retryError": "重新扫描失败", "remediation": "修复建议", "viewDetails": "查看详情", "verdict": { diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index db43c094..47842d82 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -199,6 +199,9 @@ export function SkillDetailPage() { const canReport = skill?.canReport ?? true const canHardDeleteSkill = Boolean(skill && user && (skill.ownerId === user.userId || hasRole('SUPER_ADMIN'))) const canManageLabels = Boolean(skill && user && (skill.canManageLifecycle || hasRole('SUPER_ADMIN'))) + const canManageSecurityScan = Boolean(skill && user && ( + skill.canManageLifecycle || hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN') + )) const isVersionDownloadable = selectedVersionEntry?.status === 'PUBLISHED' && (selectedVersionEntry?.downloadAvailable ?? false) useEffect(() => { @@ -1256,8 +1259,13 @@ export function SkillDetailPage() { description={skill.summary} /> - {skill.canManageLifecycle && selectedVersionEntry && ( - + {canManageSecurityScan && selectedVersionEntry && ( + )} Date: Thu, 3 Sep 2026 17:58:13 +0800 Subject: [PATCH 2/6] chore(api): refresh security scan retry schema Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- web/src/api/generated/schema.d.ts | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index b7f92c98..5fc821ed 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -1268,6 +1268,22 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/skills/{skillId}/versions/{versionId}/security-audit/retry": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["retrySecurityScan"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/skills/{canonicalSlug}/undelete": { parameters: { query?: never; @@ -8517,6 +8533,29 @@ export interface operations { }; }; }; + retrySecurityScan: { + parameters: { + query?: never; + header?: never; + path: { + skillId: number; + versionId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillLifecycleMutationResponse"]; + }; + }; + }; + }; undeleteSkill: { parameters: { query?: never; From 697bb952a4ac4d5b598eb3aa6bfcf626acf2e87a Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:25:02 +0800 Subject: [PATCH 3/6] fix(security): harden scan retry lifecycle Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../service/SecurityScanRetryAppService.java | 31 +++++++++++------ .../skillhub/stream/ScanTaskConsumer.java | 3 +- .../SecurityScanRetryAppServiceTest.java | 20 +++++++---- .../stream/ScanTaskConsumerLoggingTest.java | 5 ++- .../skillhub/stream/ScanTaskConsumerTest.java | 5 ++- .../policy/RouteSecurityPolicyRegistry.java | 1 + .../RouteSecurityPolicyRegistryTest.java | 12 +++++++ .../domain/security/SecurityScanService.java | 23 +++++++++---- .../domain/skill/SkillVersionRepository.java | 3 ++ .../security/SecurityScanServiceTest.java | 34 ++++++++++++++++--- .../infra/jpa/SkillVersionJpaRepository.java | 4 +++ .../security-audit-summary.test.tsx | 30 ++++++++++++++-- web/src/pages/skill-detail.test.tsx | 32 +++++++++++++++++ web/src/pages/skill-detail.tsx | 9 +++-- 14 files changed, 176 insertions(+), 36 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java index 7c766bdb..d68cf460 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java @@ -55,9 +55,26 @@ public class SecurityScanRetryAppService { Skill skill = skillRepository.findById(skillId) .orElseThrow(() -> new DomainBadRequestException("error.skill.notFound", skillId)); authorize(skill, userId, platformRoles, namespaceRoles); - SkillVersion version = skillVersionRepository.findBySkillIdForUpdate(skillId).stream() - .filter(candidate -> candidate.getId().equals(versionId)) - .findFirst() + + SkillVersion observedVersion = skillVersionRepository.findById(versionId) + .filter(candidate -> candidate.getSkillId().equals(skillId)) + .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", versionId)); + if (observedVersion.getStatus() != SkillVersionStatus.SCAN_FAILED + && observedVersion.getStatus() != SkillVersionStatus.SCANNING) { + throw new DomainBadRequestException("error.security.scan.retry.status", observedVersion.getStatus()); + } + if (!securityScanService.isEnabled()) { + throw new DomainBadRequestException("error.security.scan.retry.disabled"); + } + + String bundleKey = bundleKey(skillId, versionId); + if (observedVersion.getStatus() == SkillVersionStatus.SCAN_FAILED + && !objectStorageService.exists(bundleKey)) { + throw new DomainBadRequestException("error.security.scan.retry.bundleMissing"); + } + + SkillVersion version = skillVersionRepository.findByIdForUpdate(versionId) + .filter(candidate -> candidate.getSkillId().equals(skillId)) .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", versionId)); if (version.getStatus() == SkillVersionStatus.SCANNING && hasActiveAttempt(versionId)) { @@ -66,14 +83,6 @@ public class SecurityScanRetryAppService { if (version.getStatus() != SkillVersionStatus.SCAN_FAILED) { throw new DomainBadRequestException("error.security.scan.retry.status", version.getStatus()); } - if (!securityScanService.isEnabled()) { - throw new DomainBadRequestException("error.security.scan.retry.disabled"); - } - - String bundleKey = bundleKey(skillId, versionId); - if (!objectStorageService.exists(bundleKey)) { - throw new DomainBadRequestException("error.security.scan.retry.bundleMissing"); - } ScanTask task = securityScanService.retryStoredBundleScan(version, bundleKey, userId); auditLogService.record( diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/ScanTaskConsumer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/ScanTaskConsumer.java index 548b28be..490ce90b 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/ScanTaskConsumer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/ScanTaskConsumer.java @@ -232,7 +232,8 @@ public class ScanTaskConsumer extends AbstractStreamConsumer service.retry( 8L, 42L, "owner-1", Set.of(), Map.of(), new AuditRequestContext(null, null))) .isInstanceOf(DomainBadRequestException.class); verify(securityScanService, never()).retryStoredBundleScan(any(), any(), any()); + verify(skillVersionRepository, never()).findByIdForUpdate(any()); } @Test void retry_rejectsMissingStoredBundle() { - given(skillVersionRepository.findBySkillIdForUpdate(8L)).willReturn(List.of(version)); + given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); given(securityScanService.isEnabled()).willReturn(true); assertThatThrownBy(() -> service.retry( @@ -130,12 +133,15 @@ class SecurityScanRetryAppServiceTest { .isInstanceOf(DomainBadRequestException.class); verify(securityScanService, never()).retryStoredBundleScan(any(), any(), any()); + verify(skillVersionRepository, never()).findByIdForUpdate(any()); } @Test void retry_whenAttemptAlreadyStartedReturnsCurrentStateWithoutDuplicateTask() { version.setStatus(SkillVersionStatus.SCANNING); - given(skillVersionRepository.findBySkillIdForUpdate(8L)).willReturn(List.of(version)); + given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); + given(skillVersionRepository.findByIdForUpdate(42L)).willReturn(Optional.of(version)); + given(securityScanService.isEnabled()).willReturn(true); given(securityAuditRepository.findLatestActiveByVersionIdAndScannerType(42L, ScannerType.SKILL_SCANNER)) .willReturn(Optional.of(new SecurityAudit(42L, ScannerType.SKILL_SCANNER, "task-existing"))); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java index 3f295be1..ccdab04d 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java @@ -220,7 +220,10 @@ class ScanTaskConsumerLoggingTest { } @Override - public void processScanResult(Long versionId, ScannerType scannerType, SecurityScanResponse response) { + public void processScanResult(String taskId, + Long versionId, + ScannerType scannerType, + SecurityScanResponse response) { } @Override diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java index cbaf65af..eacdef88 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java @@ -716,7 +716,10 @@ class ScanTaskConsumerTest { } @Override - public void processScanResult(Long versionId, ScannerType scannerType, SecurityScanResponse response) { + public void processScanResult(String taskId, + Long versionId, + ScannerType scannerType, + SecurityScanResponse response) { this.lastVersionId = versionId; this.lastScannerType = scannerType; this.lastResponse = response; 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 58b47ae0..23375ed9 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 @@ -144,6 +144,7 @@ public class RouteSecurityPolicyRegistry { ApiTokenPolicy.require(HttpMethod.DELETE, "/api/v1/skills/*/*", "skill:delete"), ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/skills", "skill:publish"), ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/skills/*/publish", "skill:publish"), + ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/skills/*/versions/*/security-audit/retry", "skill:publish"), ApiTokenPolicy.require(HttpMethod.POST, "/api/web/skills/*/publish", "skill:publish"), ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/publish", "skill:publish"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/cli/v1/auth/whoami"), 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 4cde9090..11697808 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 @@ -64,6 +64,18 @@ class RouteSecurityPolicyRegistryTest { assertTrue(allowed.allowed()); } + @Test + void authorizeApiToken_requiresPublishScopeForSecurityScanRetry() { + var denied = registry.authorizeApiToken( + "POST", "/api/v1/skills/8/versions/42/security-audit/retry", Set.of("skill:read")); + var allowed = registry.authorizeApiToken( + "POST", "/api/v1/skills/8/versions/42/security-audit/retry", Set.of("skill:publish")); + + assertFalse(denied.allowed()); + assertEquals("skill:publish", denied.requiredScope()); + assertTrue(allowed.allowed()); + } + @Test void authorizeApiToken_requiresDeleteScopeForHardDeleteEndpoint() { var denied = registry.authorizeApiToken("DELETE", "/api/v1/skills/global/demo-skill", Set.of("skill:publish")); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java index 59e9c274..6c96c3a7 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java @@ -168,10 +168,15 @@ public class SecurityScanService { } @Transactional - public void processScanResult(Long versionId, ScannerType scannerType, SecurityScanResponse response) { - SecurityAudit audit = auditRepository.findLatestActiveByVersionIdAndScannerType(versionId, scannerType) + public void processScanResult(String taskId, + Long versionId, + ScannerType scannerType, + SecurityScanResponse response) { + SecurityAudit audit = auditRepository.findByTaskId(taskId) + .filter(candidate -> candidate.getSkillVersionId().equals(versionId)) + .filter(candidate -> candidate.getScannerType() == scannerType) .orElseThrow(() -> new IllegalStateException( - "SecurityAudit not found for versionId=" + versionId + ", scannerType=" + scannerType)); + "SecurityAudit not found for taskId=" + taskId)); SkillVersion version = skillVersionRepository.findById(versionId) .orElseThrow(() -> new IllegalStateException("SkillVersion not found: " + versionId)); @@ -185,15 +190,21 @@ public class SecurityScanService { audit.setScannedAt(Instant.now(Clock.systemUTC())); auditRepository.save(audit); - // Only transition from SCANNING — leave PUBLISHED/REJECTED/YANKED untouched - if (version.getStatus() == SkillVersionStatus.SCANNING) { + boolean currentAttempt = auditRepository + .findLatestActiveByVersionIdAndScannerType(versionId, scannerType) + .map(latest -> taskId.equals(latest.getTaskId())) + .orElse(false); + // A late result is retained on its own audit round but cannot complete a newer attempt. + if (currentAttempt && version.getStatus() == SkillVersionStatus.SCANNING) { if (version.getRequestedVisibility() == SkillVisibility.PRIVATE) { version.setStatus(SkillVersionStatus.UPLOADED); } else { version.setStatus(SkillVersionStatus.PENDING_REVIEW); } } - skillVersionRepository.save(version); + if (currentAttempt) { + skillVersionRepository.save(version); + } } private Path saveTempDirectory(Long versionId, List entries) { diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java index b99cc50f..257ef03c 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java @@ -8,6 +8,9 @@ import java.util.Optional; */ public interface SkillVersionRepository { Optional findById(Long id); + default Optional findByIdForUpdate(Long id) { + return findById(id); + } List findByIdIn(List ids); List findBySkillIdIn(List skillIds); List findBySkillIdInAndStatus(List skillIds, SkillVersionStatus status); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java index cd98b43f..3292e499 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java @@ -274,10 +274,11 @@ class SecurityScanServiceTest { @Test void processScanResult_updatesAuditAndMovesVersionToPendingReview() { - SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER); + SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER, "task-current"); SkillVersion version = new SkillVersion(8L, "1.0.0", "publisher-1"); version.setStatus(SkillVersionStatus.SCANNING); + given(auditRepository.findByTaskId("task-current")).willReturn(Optional.of(audit)); given(auditRepository.findLatestActiveByVersionIdAndScannerType(42L, ScannerType.SKILL_SCANNER)) .willReturn(Optional.of(audit)); given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); @@ -300,7 +301,7 @@ class SecurityScanServiceTest { 1.25 ); - service.processScanResult(42L, ScannerType.SKILL_SCANNER, response); + service.processScanResult("task-current", 42L, ScannerType.SKILL_SCANNER, response); assertThat(audit.getScanId()).isEqualTo("scan-123"); assertThat(audit.getVerdict()).isEqualTo(SecurityVerdict.DANGEROUS); @@ -377,10 +378,11 @@ class SecurityScanServiceTest { @Test void processScanResult_shouldNotChangeStatusWhenVersionAlreadyPublished() { - SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER); + SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER, "task-published"); SkillVersion version = new SkillVersion(8L, "1.0.0", "publisher-1"); version.setStatus(SkillVersionStatus.PUBLISHED); + given(auditRepository.findByTaskId("task-published")).willReturn(Optional.of(audit)); given(auditRepository.findLatestActiveByVersionIdAndScannerType(42L, ScannerType.SKILL_SCANNER)) .willReturn(Optional.of(audit)); given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); @@ -394,7 +396,7 @@ class SecurityScanServiceTest { 0.5 ); - service.processScanResult(42L, ScannerType.SKILL_SCANNER, response); + service.processScanResult("task-published", 42L, ScannerType.SKILL_SCANNER, response); assertThat(audit.getVerdict()).isEqualTo(SecurityVerdict.SAFE); assertThat(audit.getIsSafe()).isTrue(); @@ -402,6 +404,30 @@ class SecurityScanServiceTest { verify(skillVersionRepository).save(version); } + @Test + void processScanResult_forStaleAttemptDoesNotCompleteCurrentAttempt() throws Exception { + SecurityAudit stale = new SecurityAudit(42L, ScannerType.SKILL_SCANNER, "task-stale"); + SecurityAudit current = new SecurityAudit(42L, ScannerType.SKILL_SCANNER, "task-current"); + SkillVersion version = new SkillVersion(8L, "1.0.0", "publisher-1"); + setId(version, 42L); + version.setStatus(SkillVersionStatus.SCANNING); + given(auditRepository.findByTaskId("task-stale")).willReturn(Optional.of(stale)); + given(auditRepository.findLatestActiveByVersionIdAndScannerType(42L, ScannerType.SKILL_SCANNER)) + .willReturn(Optional.of(current)); + given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); + + service.processScanResult( + "task-stale", + 42L, + ScannerType.SKILL_SCANNER, + new SecurityScanResponse("scan-stale", SecurityVerdict.SAFE, 0, null, List.of(), 0.1) + ); + + assertThat(stale.getScanId()).isEqualTo("scan-stale"); + assertThat(version.getStatus()).isEqualTo(SkillVersionStatus.SCANNING); + verify(skillVersionRepository, never()).save(version); + } + private void setId(Object target, Long id) throws Exception { Field field = target.getClass().getDeclaredField("id"); field.setAccessible(true); diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java index e7c85e1c..4aba319c 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java @@ -22,6 +22,10 @@ import org.springframework.stereotype.Repository; */ @Repository public interface SkillVersionJpaRepository extends JpaRepository, SkillVersionRepository { + + @Override + @Query(value = "SELECT * FROM skill_version WHERE id = :id FOR UPDATE", nativeQuery = true) + Optional findByIdForUpdate(@Param("id") Long id); List findByIdIn(List ids); List findBySkillId(Long skillId); List findBySkillIdIn(List skillIds); diff --git a/web/src/features/security-audit/security-audit-summary.test.tsx b/web/src/features/security-audit/security-audit-summary.test.tsx index a20a51fe..1ae6c18e 100644 --- a/web/src/features/security-audit/security-audit-summary.test.tsx +++ b/web/src/features/security-audit/security-audit-summary.test.tsx @@ -1,5 +1,8 @@ +/** @vitest-environment jsdom */ + import { renderToStaticMarkup } from 'react-dom/server' -import { describe, expect, it, vi } from 'vitest' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { SecurityAuditRecord } from './types' import { SecurityAuditSummary } from './security-audit-summary' @@ -34,13 +37,18 @@ function createAudit(overrides: Partial = {}): SecurityAudi } let mockAudits: SecurityAuditRecord[] | undefined = undefined -const retryMutation = { mutate: vi.fn(), isPending: false } +const { retryMutation, toastMocks } = vi.hoisted(() => ({ + retryMutation: { mutate: vi.fn(), isPending: false }, + toastMocks: { success: vi.fn(), error: vi.fn() }, +})) vi.mock('./use-security-audit', () => ({ useSecurityAudits: () => ({ data: mockAudits }), useRetrySecurityScan: () => retryMutation, })) +vi.mock('@/shared/lib/toast', () => ({ toast: toastMocks })) + // Mock the Dialog components to avoid Radix UI portal / context issues in static render vi.mock('@/shared/ui/dialog', () => ({ Dialog: ({ children }: { children: React.ReactNode }) => <>{children}, @@ -56,6 +64,10 @@ vi.mock('./security-audit-section', () => ({ })) describe('SecurityAuditSummary', () => { + afterEach(() => { + cleanup() + vi.clearAllMocks() + }) it('returns null when audits is undefined', () => { mockAudits = undefined @@ -132,6 +144,20 @@ describe('SecurityAuditSummary', () => { expect(unauthorizedHtml).not.toContain('securityAudit.retry') }) + it('starts retry and exposes success and failure feedback callbacks', () => { + mockAudits = [createAudit({ scannedAt: null })] + render() + + fireEvent.click(screen.getByRole('button', { name: 'securityAudit.retry' })) + + expect(retryMutation.mutate).toHaveBeenCalledOnce() + const options = retryMutation.mutate.mock.calls[0]?.[1] + options.onSuccess() + expect(toastMocks.success).toHaveBeenCalledWith('securityAudit.retrySuccess') + options.onError(new Error('scanner unavailable')) + expect(toastMocks.error).toHaveBeenCalledWith('securityAudit.retryError', 'scanner unavailable') + }) + it('renders the total findings count across all audits', () => { mockAudits = [ createAudit({ id: 1, findingsCount: 3 }), diff --git a/web/src/pages/skill-detail.test.tsx b/web/src/pages/skill-detail.test.tsx index b9ba9a71..c65648d9 100644 --- a/web/src/pages/skill-detail.test.tsx +++ b/web/src/pages/skill-detail.test.tsx @@ -74,6 +74,12 @@ vi.mock('@/features/report/use-skill-reports', () => ({ useSubmitSkillReport: () => ({ mutateAsync: vi.fn(), isPending: false }), })) +vi.mock('@/features/security-audit/security-audit-summary', () => ({ + SecurityAuditSummary: ({ versionId, versionStatus }: { versionId: number; versionStatus?: string }) => ( +
audit:{versionId}:{versionStatus}
+ ), +})) + vi.mock('@/shared/lib/toast', () => ({ toast: { success: toastMocks.success, error: toastMocks.error }, })) @@ -499,6 +505,32 @@ describe('SkillDetailPage', () => { expect(html).not.toContain('skillDetail.versionStatusScanFailed') }) + it('binds scan retry to the failed owner preview when a published version remains visible', () => { + useSkillDetailMock.mockReturnValue({ + data: createSkill({ + canManageLifecycle: true, + headlineVersion: { id: 10, version: '1.0.0', status: 'PUBLISHED' }, + publishedVersion: { id: 10, version: '1.0.0', status: 'PUBLISHED' }, + ownerPreviewVersion: { id: 12, version: '1.2.0', status: 'SCAN_FAILED' }, + resolutionMode: 'PUBLISHED', + }), + isLoading: false, + isFetching: false, + error: null, + }) + useSkillVersionsMock.mockReturnValue({ + data: [ + { id: 10, version: '1.0.0', status: 'PUBLISHED', downloadAvailable: true }, + { id: 12, version: '1.2.0', status: 'SCAN_FAILED', downloadAvailable: false }, + ], + }) + + const html = renderToStaticMarkup() + + expect(html).toContain('audit:12:SCAN_FAILED') + expect(html).not.toContain('audit:10:PUBLISHED') + }) + it('allows long pending review versions to wrap inside the review card', () => { useSkillDetailMock.mockReturnValue({ data: createSkill({ diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index 47842d82..3122e98a 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -202,6 +202,9 @@ export function SkillDetailPage() { const canManageSecurityScan = Boolean(skill && user && ( skill.canManageLifecycle || hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN') )) + const securityAuditVersion = ownerPreviewVersion?.status === 'SCAN_FAILED' + ? ownerPreviewVersion + : selectedVersionEntry const isVersionDownloadable = selectedVersionEntry?.status === 'PUBLISHED' && (selectedVersionEntry?.downloadAvailable ?? false) useEffect(() => { @@ -1259,11 +1262,11 @@ export function SkillDetailPage() { description={skill.summary} /> - {canManageSecurityScan && selectedVersionEntry && ( + {canManageSecurityScan && securityAuditVersion && ( )} From 6770be22c5c9a3a4ae6e887b0bb7a30e052d24b0 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:32:22 +0800 Subject: [PATCH 4/6] test(security): verify retry row locking Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../service/SecurityScanRetryAppService.java | 11 +- .../SecurityScanRetryAppServiceTest.java | 17 ++- .../service/SecurityScanRetryLockingTest.java | 125 ++++++++++++++++++ .../domain/skill/SkillVersionRepository.java | 5 + .../infra/jpa/SkillVersionJpaRepository.java | 4 + 5 files changed, 150 insertions(+), 12 deletions(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryLockingTest.java diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java index d68cf460..19667b7e 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java @@ -56,19 +56,18 @@ public class SecurityScanRetryAppService { .orElseThrow(() -> new DomainBadRequestException("error.skill.notFound", skillId)); authorize(skill, userId, platformRoles, namespaceRoles); - SkillVersion observedVersion = skillVersionRepository.findById(versionId) - .filter(candidate -> candidate.getSkillId().equals(skillId)) + SkillVersionStatus observedStatus = skillVersionRepository.findStatusByIdAndSkillId(versionId, skillId) .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", versionId)); - if (observedVersion.getStatus() != SkillVersionStatus.SCAN_FAILED - && observedVersion.getStatus() != SkillVersionStatus.SCANNING) { - throw new DomainBadRequestException("error.security.scan.retry.status", observedVersion.getStatus()); + if (observedStatus != SkillVersionStatus.SCAN_FAILED + && observedStatus != SkillVersionStatus.SCANNING) { + throw new DomainBadRequestException("error.security.scan.retry.status", observedStatus); } if (!securityScanService.isEnabled()) { throw new DomainBadRequestException("error.security.scan.retry.disabled"); } String bundleKey = bundleKey(skillId, versionId); - if (observedVersion.getStatus() == SkillVersionStatus.SCAN_FAILED + if (observedStatus == SkillVersionStatus.SCAN_FAILED && !objectStorageService.exists(bundleKey)) { throw new DomainBadRequestException("error.security.scan.retry.bundleMissing"); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryAppServiceTest.java index b5b11eb1..b0b2e60a 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryAppServiceTest.java @@ -64,7 +64,8 @@ class SecurityScanRetryAppServiceTest { @Test void retry_asOwnerCreatesNewAttemptAndAuditLog() { - given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); + given(skillVersionRepository.findStatusByIdAndSkillId(42L, 8L)) + .willReturn(Optional.of(SkillVersionStatus.SCAN_FAILED)); given(skillVersionRepository.findByIdForUpdate(42L)).willReturn(Optional.of(version)); given(securityScanService.isEnabled()).willReturn(true); given(objectStorageService.exists("packages/8/42/bundle.zip")).willReturn(true); @@ -84,7 +85,8 @@ class SecurityScanRetryAppServiceTest { @Test void retry_allowsNamespaceAdminAndPlatformSecurityAdmin() { - given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); + given(skillVersionRepository.findStatusByIdAndSkillId(42L, 8L)) + .willReturn(Optional.of(SkillVersionStatus.SCAN_FAILED)); given(skillVersionRepository.findByIdForUpdate(42L)).willReturn(Optional.of(version)); given(securityScanService.isEnabled()).willReturn(true); given(objectStorageService.exists("packages/8/42/bundle.zip")).willReturn(true); @@ -107,13 +109,14 @@ class SecurityScanRetryAppServiceTest { .isInstanceOf(DomainForbiddenException.class); verify(skillVersionRepository, never()).findByIdForUpdate(any()); - verify(skillVersionRepository, never()).findById(any()); + verify(skillVersionRepository, never()).findStatusByIdAndSkillId(any(), any()); } @Test void retry_rejectsNonFailedVersion() { version.setStatus(SkillVersionStatus.PENDING_REVIEW); - given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); + given(skillVersionRepository.findStatusByIdAndSkillId(42L, 8L)) + .willReturn(Optional.of(SkillVersionStatus.PENDING_REVIEW)); assertThatThrownBy(() -> service.retry( 8L, 42L, "owner-1", Set.of(), Map.of(), new AuditRequestContext(null, null))) @@ -125,7 +128,8 @@ class SecurityScanRetryAppServiceTest { @Test void retry_rejectsMissingStoredBundle() { - given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); + given(skillVersionRepository.findStatusByIdAndSkillId(42L, 8L)) + .willReturn(Optional.of(SkillVersionStatus.SCAN_FAILED)); given(securityScanService.isEnabled()).willReturn(true); assertThatThrownBy(() -> service.retry( @@ -139,7 +143,8 @@ class SecurityScanRetryAppServiceTest { @Test void retry_whenAttemptAlreadyStartedReturnsCurrentStateWithoutDuplicateTask() { version.setStatus(SkillVersionStatus.SCANNING); - given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version)); + given(skillVersionRepository.findStatusByIdAndSkillId(42L, 8L)) + .willReturn(Optional.of(SkillVersionStatus.SCANNING)); given(skillVersionRepository.findByIdForUpdate(42L)).willReturn(Optional.of(version)); given(securityScanService.isEnabled()).willReturn(true); given(securityAuditRepository.findLatestActiveByVersionIdAndScannerType(42L, ScannerType.SKILL_SCANNER)) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryLockingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryLockingTest.java new file mode 100644 index 00000000..986a07ff --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryLockingTest.java @@ -0,0 +1,125 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.infra.jpa.SkillVersionJpaRepository; +import jakarta.persistence.EntityManager; +import jakarta.persistence.PersistenceContext; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionTemplate; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +import static org.assertj.core.api.Assertions.assertThat; + +@DataJpaTest +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@ActiveProfiles("test") +@Testcontainers +class SecurityScanRetryLockingTest { + + @Container + private static final PostgreSQLContainer POSTGRES = + new PostgreSQLContainer<>("postgres:16-alpine"); + + @DynamicPropertySource + static void configurePostgres(DynamicPropertyRegistry registry) { + registry.add("spring.datasource.url", POSTGRES::getJdbcUrl); + registry.add("spring.datasource.username", POSTGRES::getUsername); + registry.add("spring.datasource.password", POSTGRES::getPassword); + registry.add("spring.datasource.driver-class-name", () -> "org.postgresql.Driver"); + registry.add("spring.jpa.database-platform", () -> "org.hibernate.dialect.PostgreSQLDialect"); + } + + @Autowired + private SkillVersionJpaRepository skillVersionRepository; + + @Autowired + private PlatformTransactionManager transactionManager; + + @PersistenceContext + private EntityManager entityManager; + + @Test + @Transactional(propagation = Propagation.NOT_SUPPORTED) + void lockReadSeesStateCommittedWhileWaitingInsteadOfCachedPreflightEntity() throws Exception { + Fixture fixture = persistFailedVersion(); + Long versionId = fixture.versionId(); + CountDownLatch firstLocked = new CountDownLatch(1); + CountDownLatch secondAboutToLock = new CountDownLatch(1); + CountDownLatch releaseFirst = new CountDownLatch(1); + TransactionTemplate transactions = new TransactionTemplate(transactionManager); + + try (var executor = Executors.newVirtualThreadPerTaskExecutor()) { + var first = executor.submit(() -> transactions.executeWithoutResult(status -> { + SkillVersion version = skillVersionRepository.findByIdForUpdate(versionId).orElseThrow(); + version.setStatus(SkillVersionStatus.SCANNING); + firstLocked.countDown(); + await(releaseFirst); + })); + + assertThat(firstLocked.await(10, TimeUnit.SECONDS)).isTrue(); + var second = executor.submit(() -> transactions.execute(status -> { + assertThat(skillVersionRepository.findStatusByIdAndSkillId(versionId, fixture.skillId())) + .contains(SkillVersionStatus.SCAN_FAILED); + secondAboutToLock.countDown(); + return skillVersionRepository.findByIdForUpdate(versionId).orElseThrow().getStatus(); + })); + + assertThat(secondAboutToLock.await(10, TimeUnit.SECONDS)).isTrue(); + releaseFirst.countDown(); + first.get(); + assertThat(second.get()).isEqualTo(SkillVersionStatus.SCANNING); + } + } + + private Fixture persistFailedVersion() { + TransactionTemplate transaction = new TransactionTemplate(transactionManager); + return transaction.execute(status -> { + UserAccount user = new UserAccount("retry-lock-user", "Retry Lock User", null, null); + entityManager.persist(user); + Namespace namespace = new Namespace("retry-lock", "Retry Lock", user.getId()); + entityManager.persist(namespace); + entityManager.flush(); + Skill skill = new Skill(namespace.getId(), "retry-lock", user.getId(), SkillVisibility.PRIVATE); + entityManager.persist(skill); + entityManager.flush(); + SkillVersion version = new SkillVersion(skill.getId(), "1.0.0", user.getId()); + version.setStatus(SkillVersionStatus.SCAN_FAILED); + entityManager.persist(version); + entityManager.flush(); + return new Fixture(version.getId(), skill.getId()); + }); + } + + private void await(CountDownLatch latch) { + try { + if (!latch.await(10, TimeUnit.SECONDS)) { + throw new IllegalStateException("Timed out waiting for concurrent retry test"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("Concurrent retry test interrupted", e); + } + } + + private record Fixture(Long versionId, Long skillId) { + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java index 257ef03c..90018dfc 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java @@ -11,6 +11,11 @@ public interface SkillVersionRepository { default Optional findByIdForUpdate(Long id) { return findById(id); } + default Optional findStatusByIdAndSkillId(Long id, Long skillId) { + return findById(id) + .filter(version -> version.getSkillId().equals(skillId)) + .map(SkillVersion::getStatus); + } List findByIdIn(List ids); List findBySkillIdIn(List skillIds); List findBySkillIdInAndStatus(List skillIds, SkillVersionStatus status); diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java index 4aba319c..481d6127 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java @@ -26,6 +26,10 @@ public interface SkillVersionJpaRepository extends JpaRepository findByIdForUpdate(@Param("id") Long id); + + @Override + @Query("SELECT version.status FROM SkillVersion version WHERE version.id = :id AND version.skillId = :skillId") + Optional findStatusByIdAndSkillId(@Param("id") Long id, @Param("skillId") Long skillId); List findByIdIn(List ids); List findBySkillId(Long skillId); List findBySkillIdIn(List skillIds); From fd932cc1602372007ff803d8c9947bdcfc36fdec Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:35:56 +0800 Subject: [PATCH 5/6] fix(security): require explicit retry locking Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../iflytek/skillhub/domain/skill/SkillVersionRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java index 90018dfc..91f103c7 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java @@ -9,7 +9,7 @@ import java.util.Optional; public interface SkillVersionRepository { Optional findById(Long id); default Optional findByIdForUpdate(Long id) { - return findById(id); + throw new UnsupportedOperationException("This repository does not provide row locking"); } default Optional findStatusByIdAndSkillId(Long id, Long skillId) { return findById(id) From 1b7e679d4596d8e4975d9073cbc20145338dbd8c Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:59:17 +0800 Subject: [PATCH 6/6] fix(security): include csrf token in scan retry Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../stream/ScanTaskConsumerLoggingTest.java | 1 + .../security-audit/use-security-audit.test.ts | 28 +++++++++++++++++-- .../security-audit/use-security-audit.ts | 3 +- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java index ccdab04d..c388fcbf 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java @@ -232,6 +232,7 @@ class ScanTaskConsumerLoggingTest { ScannerType scannerType, String reason) { } + } private static final class TestProducer implements ScanTaskProducer { diff --git a/web/src/features/security-audit/use-security-audit.test.ts b/web/src/features/security-audit/use-security-audit.test.ts index 9f0195ec..84e8566c 100644 --- a/web/src/features/security-audit/use-security-audit.test.ts +++ b/web/src/features/security-audit/use-security-audit.test.ts @@ -12,12 +12,22 @@ import { describe, expect, it, vi } from 'vitest' // Capture the options passed to useQuery so we can assert on them. let capturedOptions: Record | undefined +let capturedMutationOptions: Record | undefined +const apiMocks = vi.hoisted(() => ({ + fetchJson: vi.fn(), + getCsrfHeaders: vi.fn(() => ({ 'X-XSRF-TOKEN': 'csrf-token' })), +})) vi.mock('@tanstack/react-query', () => ({ useQuery: (options: Record) => { capturedOptions = options return { data: undefined, isLoading: false } }, + useMutation: (options: Record) => { + capturedMutationOptions = options + return { mutate: vi.fn(), isPending: false } + }, + useQueryClient: () => ({ invalidateQueries: vi.fn() }), })) // Mock fetchJson to avoid actual network calls. The hook's queryFn @@ -30,11 +40,12 @@ vi.mock('@/api/client', () => ({ this.status = status } }, - fetchJson: vi.fn(), + fetchJson: apiMocks.fetchJson, + getCsrfHeaders: apiMocks.getCsrfHeaders, })) // Dynamic import to ensure mocks are established first. -const { useSecurityAudits } = await import('./use-security-audit') +const { useRetrySecurityScan, useSecurityAudits } = await import('./use-security-audit') describe('useSecurityAudits', () => { it('uses the correct query key structure', () => { @@ -78,4 +89,17 @@ describe('useSecurityAudits', () => { expect(capturedOptions?.retry).toBe(false) }) + + it('sends the CSRF header when retrying a security scan', async () => { + apiMocks.fetchJson.mockResolvedValueOnce({ status: 'SCANNING' }) + useRetrySecurityScan(42, 100) + + await (capturedMutationOptions?.mutationFn as () => Promise)() + + expect(apiMocks.getCsrfHeaders).toHaveBeenCalledOnce() + expect(apiMocks.fetchJson).toHaveBeenCalledWith( + '/api/v1/skills/42/versions/100/security-audit/retry', + { method: 'POST', headers: { 'X-XSRF-TOKEN': 'csrf-token' } }, + ) + }) }) diff --git a/web/src/features/security-audit/use-security-audit.ts b/web/src/features/security-audit/use-security-audit.ts index 88bc3278..e1383442 100644 --- a/web/src/features/security-audit/use-security-audit.ts +++ b/web/src/features/security-audit/use-security-audit.ts @@ -1,5 +1,5 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' -import { ApiError, fetchJson } from '@/api/client' +import { ApiError, fetchJson, getCsrfHeaders } from '@/api/client' import type { SecurityAuditRecord } from './types' async function fetchSecurityAudits( @@ -37,6 +37,7 @@ export function useRetrySecurityScan(skillId: number, versionId: number) { return useMutation({ mutationFn: () => fetchJson(`/api/v1/skills/${skillId}/versions/${versionId}/security-audit/retry`, { method: 'POST', + headers: getCsrfHeaders(), }), onSuccess: () => { void queryClient.invalidateQueries({ queryKey: ['security-audits', skillId, versionId] })