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..19667b7e --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java @@ -0,0 +1,131 @@ +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); + + SkillVersionStatus observedStatus = skillVersionRepository.findStatusByIdAndSkillId(versionId, skillId) + .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", versionId)); + 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 (observedStatus == 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)) { + return response(skillId, versionId); + } + if (version.getStatus() != SkillVersionStatus.SCAN_FAILED) { + throw new DomainBadRequestException("error.security.scan.retry.status", version.getStatus()); + } + + 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/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, "viewer", Set.of(), Map.of(), new AuditRequestContext(null, null))) + .isInstanceOf(DomainForbiddenException.class); + + verify(skillVersionRepository, never()).findByIdForUpdate(any()); + verify(skillVersionRepository, never()).findStatusByIdAndSkillId(any(), any()); + } + + @Test + void retry_rejectsNonFailedVersion() { + version.setStatus(SkillVersionStatus.PENDING_REVIEW); + 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))) + .isInstanceOf(DomainBadRequestException.class); + + verify(securityScanService, never()).retryStoredBundleScan(any(), any(), any()); + verify(skillVersionRepository, never()).findByIdForUpdate(any()); + } + + @Test + void retry_rejectsMissingStoredBundle() { + given(skillVersionRepository.findStatusByIdAndSkillId(42L, 8L)) + .willReturn(Optional.of(SkillVersionStatus.SCAN_FAILED)); + 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()); + verify(skillVersionRepository, never()).findByIdForUpdate(any()); + } + + @Test + void retry_whenAttemptAlreadyStartedReturnsCurrentStateWithoutDuplicateTask() { + version.setStatus(SkillVersionStatus.SCANNING); + 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)) + .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-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-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..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 @@ -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 @@ -229,6 +232,7 @@ class ScanTaskConsumerLoggingTest { ScannerType scannerType, String reason) { } + } private static final class TestProducer implements ScanTaskProducer { 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 61a8578a..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 @@ -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 { @@ -143,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)); @@ -160,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..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 @@ -8,6 +8,14 @@ import java.util.Optional; */ public interface SkillVersionRepository { Optional findById(Long id); + default Optional findByIdForUpdate(Long id) { + throw new UnsupportedOperationException("This repository does not provide row locking"); + } + 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-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..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 @@ -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"); @@ -230,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)); @@ -256,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); @@ -333,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)); @@ -350,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(); @@ -358,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..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 @@ -22,6 +22,14 @@ 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); + + @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); 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; 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..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,11 +37,18 @@ function createAudit(overrides: Partial = {}): SecurityAudi } let mockAudits: SecurityAuditRecord[] | undefined = undefined +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}, @@ -54,6 +64,10 @@ vi.mock('./security-audit-section', () => ({ })) describe('SecurityAuditSummary', () => { + afterEach(() => { + cleanup() + vi.clearAllMocks() + }) it('returns null when audits is undefined', () => { mockAudits = undefined @@ -116,6 +130,34 @@ 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('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/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.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 4098f3e6..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 { useQuery } from '@tanstack/react-query' -import { ApiError, fetchJson } from '@/api/client' +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' +import { ApiError, fetchJson, getCsrfHeaders } from '@/api/client' import type { SecurityAuditRecord } from './types' async function fetchSecurityAudits( @@ -31,3 +31,17 @@ 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', + headers: getCsrfHeaders(), + }), + 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.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 db43c094..3122e98a 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -199,6 +199,12 @@ 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 securityAuditVersion = ownerPreviewVersion?.status === 'SCAN_FAILED' + ? ownerPreviewVersion + : selectedVersionEntry const isVersionDownloadable = selectedVersionEntry?.status === 'PUBLISHED' && (selectedVersionEntry?.downloadAvailable ?? false) useEffect(() => { @@ -1256,8 +1262,13 @@ export function SkillDetailPage() { description={skill.summary} /> - {skill.canManageLifecycle && selectedVersionEntry && ( - + {canManageSecurityScan && securityAuditVersion && ( + )}