scriptoriumadmin/skillhub
1
0
Fork 0
mirror of https://github.com/iflytek/skillhub.git synced 2026-09-05 08:05:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(security): harden scan retry lifecycle

Browse source 
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS 2026-09-03 18:25:02 +08:00
parent 0fb00f01b6
commit 697bb952a4
14 changed files with 176 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SecurityScanRetryAppService.java
View file

@ -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(

3
server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/ScanTaskConsumer.java
View file

@ -232,7 +232,8 @@ public class ScanTaskConsumer extends AbstractStreamConsumer<ScanTaskConsumer.Sc
SecurityScanRequest request = new SecurityScanRequest(
payload.taskId(), payload.versionId(), skillPath, Map.of());
SecurityScanResponse response = securityScanner.scan(request);
securityScanService.processScanResult(payload.versionId(), payload.scannerType(), response);
securityScanService.processScanResult(
payload.taskId(), payload.versionId(), payload.scannerType(), response);
}
private static final class ConcurrentScanInProgressException extends RuntimeException {

20
server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SecurityScanRetryAppServiceTest.java
View file

@ -17,7 +17,6 @@ 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;
@ -65,7 +64,8 @@ class SecurityScanRetryAppServiceTest {
@Test
void retry_asOwnerCreatesNewAttemptAndAuditLog() {
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(objectStorageService.exists("packages/8/42/bundle.zip")).willReturn(true);
given(securityScanService.retryStoredBundleScan(version, "packages/8/42/bundle.zip", "owner-1"))
@ -84,7 +84,8 @@ class SecurityScanRetryAppServiceTest {
@Test
void retry_allowsNamespaceAdminAndPlatformSecurityAdmin() {
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(objectStorageService.exists("packages/8/42/bundle.zip")).willReturn(true);
given(securityScanService.retryStoredBundleScan(any(), any(), any()))
@ -105,24 +106,26 @@ class SecurityScanRetryAppServiceTest {
8L, 42L, "viewer", Set.of(), Map.of(), new AuditRequestContext(null, null)))
.isInstanceOf(DomainForbiddenException.class);
verify(skillVersionRepository, never()).findBySkillIdForUpdate(any());
verify(skillVersionRepository, never()).findByIdForUpdate(any());
verify(skillVersionRepository, never()).findById(any());
}
@Test
void retry_rejectsNonFailedVersion() {
version.setStatus(SkillVersionStatus.PENDING_REVIEW);
given(skillVersionRepository.findBySkillIdForUpdate(8L)).willReturn(List.of(version));
given(skillVersionRepository.findById(42L)).willReturn(Optional.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());
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")));

5
server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java
View file

@ -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

5
server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java
View file

@ -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;

1
server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java
View file

@ -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"),

12
server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java
View file

@ -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"));

23
server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/SecurityScanService.java
View file

@ -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<PackageEntry> entries) {

3
server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java
View file

@ -8,6 +8,9 @@ import java.util.Optional;
*/
public interface SkillVersionRepository {
Optional<SkillVersion> findById(Long id);
default Optional<SkillVersion> findByIdForUpdate(Long id) {
return findById(id);
}
List<SkillVersion> findByIdIn(List<Long> ids);
List<SkillVersion> findBySkillIdIn(List<Long> skillIds);
List<SkillVersion> findBySkillIdInAndStatus(List<Long> skillIds, SkillVersionStatus status);

34
server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java
View file

@ -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);

4
server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java
View file

@ -22,6 +22,10 @@ import org.springframework.stereotype.Repository;
*/
@Repository
public interface SkillVersionJpaRepository extends JpaRepository<SkillVersion, Long>, SkillVersionRepository {
@Override
@Query(value = "SELECT * FROM skill_version WHERE id = :id FOR UPDATE", nativeQuery = true)
Optional<SkillVersion> findByIdForUpdate(@Param("id") Long id);
List<SkillVersion> findByIdIn(List<Long> ids);
List<SkillVersion> findBySkillId(Long skillId);
List<SkillVersion> findBySkillIdIn(List<Long> skillIds);

30
web/src/features/security-audit/security-audit-summary.test.tsx
View file

@ -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<SecurityAuditRecord> = {}): 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(<SecurityAuditSummary skillId={1} versionId={10} versionStatus="SCAN_FAILED" canRetry />)
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 }),

32
web/src/pages/skill-detail.test.tsx
View file

@ -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 }) => (
<div data-testid="security-audit-summary">audit:{versionId}:{versionStatus}</div>
),
}))
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(<SkillDetailPage />)
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({

9
web/src/pages/skill-detail.tsx
View file

@ -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 && (
<SecurityAuditSummary
skillId={skill.id}
versionId={selectedVersionEntry.id}
versionStatus={selectedVersionEntry.status}
versionId={securityAuditVersion.id}
versionStatus={securityAuditVersion.status}
canRetry
/>
)}