mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-07 08:26:00 +00:00
fix(security): trigger security scan for admin-published skills
Super admin auto-publish flow was skipping security scanning entirely. Now triggerScan is called regardless of autoPublish flag, while preserving the PUBLISHED status (scan runs as post-publish audit rather than blocking). Closes #415
This commit is contained in:
parent
84914c9d94
commit
ec4598efec
4 changed files with 108 additions and 9 deletions
|
|
@ -84,8 +84,11 @@ public class SecurityScanService {
|
|||
System.currentTimeMillis(),
|
||||
Map.of("scannerType", ScannerType.SKILL_SCANNER.getValue())
|
||||
));
|
||||
version.setStatus(SkillVersionStatus.SCANNING);
|
||||
skillVersionRepository.save(version);
|
||||
// Only transition to SCANNING if the version is not already published (auto-publish flow)
|
||||
if (version.getStatus() != SkillVersionStatus.PUBLISHED) {
|
||||
version.setStatus(SkillVersionStatus.SCANNING);
|
||||
skillVersionRepository.save(version);
|
||||
}
|
||||
}
|
||||
|
||||
@Transactional
|
||||
|
|
@ -106,11 +109,14 @@ public class SecurityScanService {
|
|||
audit.setScannedAt(Instant.now(Clock.systemUTC()));
|
||||
auditRepository.save(audit);
|
||||
|
||||
// Set status based on requestedVisibility
|
||||
if (version.getRequestedVisibility() == SkillVisibility.PRIVATE) {
|
||||
version.setStatus(SkillVersionStatus.UPLOADED);
|
||||
} else {
|
||||
version.setStatus(SkillVersionStatus.PENDING_REVIEW);
|
||||
// Only transition status if the version is not already published (auto-publish flow)
|
||||
if (version.getStatus() != SkillVersionStatus.PUBLISHED) {
|
||||
// Set status based on requestedVisibility
|
||||
if (version.getRequestedVisibility() == SkillVisibility.PRIVATE) {
|
||||
version.setStatus(SkillVersionStatus.UPLOADED);
|
||||
} else {
|
||||
version.setStatus(SkillVersionStatus.PENDING_REVIEW);
|
||||
}
|
||||
}
|
||||
skillVersionRepository.save(version);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -404,8 +404,8 @@ public class SkillPublishService {
|
|||
));
|
||||
}
|
||||
|
||||
// Trigger security scan for all non-autoPublish versions
|
||||
if (!autoPublish && securityScanService.isEnabled()) {
|
||||
// Trigger security scan for all versions (including auto-publish)
|
||||
if (securityScanService.isEnabled()) {
|
||||
securityScanService.triggerScan(version.getId(), entries, publisherId);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -210,6 +210,54 @@ class SecurityScanServiceTest {
|
|||
verify(skillVersionRepository).save(version);
|
||||
}
|
||||
|
||||
@Test
|
||||
void triggerScan_shouldNotChangeStatusWhenVersionAlreadyPublished() throws Exception {
|
||||
SkillVersion version = new SkillVersion(8L, "1.0.0", "publisher-1");
|
||||
setId(version, 42L);
|
||||
version.setStatus(SkillVersionStatus.PUBLISHED);
|
||||
PackageEntry entry = new PackageEntry(
|
||||
"README.md",
|
||||
"# demo".getBytes(),
|
||||
6L,
|
||||
"text/markdown"
|
||||
);
|
||||
|
||||
given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version));
|
||||
|
||||
service.triggerScan(42L, List.of(entry), "publisher-1");
|
||||
|
||||
verify(auditRepository).save(org.mockito.ArgumentMatchers.any(SecurityAudit.class));
|
||||
verify(scanTaskProducer).publishScanTask(org.mockito.ArgumentMatchers.any(ScanTask.class));
|
||||
assertThat(version.getStatus()).isEqualTo(SkillVersionStatus.PUBLISHED);
|
||||
}
|
||||
|
||||
@Test
|
||||
void processScanResult_shouldNotChangeStatusWhenVersionAlreadyPublished() {
|
||||
SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER);
|
||||
SkillVersion version = new SkillVersion(8L, "1.0.0", "publisher-1");
|
||||
version.setStatus(SkillVersionStatus.PUBLISHED);
|
||||
|
||||
given(auditRepository.findLatestActiveByVersionIdAndScannerType(42L, ScannerType.SKILL_SCANNER))
|
||||
.willReturn(Optional.of(audit));
|
||||
given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version));
|
||||
|
||||
SecurityScanResponse response = new SecurityScanResponse(
|
||||
"scan-456",
|
||||
SecurityVerdict.SAFE,
|
||||
0,
|
||||
null,
|
||||
List.of(),
|
||||
0.5
|
||||
);
|
||||
|
||||
service.processScanResult(42L, ScannerType.SKILL_SCANNER, response);
|
||||
|
||||
assertThat(audit.getVerdict()).isEqualTo(SecurityVerdict.SAFE);
|
||||
assertThat(audit.getIsSafe()).isTrue();
|
||||
assertThat(version.getStatus()).isEqualTo(SkillVersionStatus.PUBLISHED);
|
||||
verify(skillVersionRepository).save(version);
|
||||
}
|
||||
|
||||
private void setId(Object target, Long id) throws Exception {
|
||||
Field field = target.getClass().getDeclaredField("id");
|
||||
field.setAccessible(true);
|
||||
|
|
|
|||
|
|
@ -1327,6 +1327,51 @@ class SkillPublishServiceTest {
|
|||
verify(securityScanService).triggerScan(eq(10L), anyList(), eq(publisherId));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testPublishFromEntries_SuperAdmin_WhenScannerEnabled_ShouldTriggerScan() throws Exception {
|
||||
String namespaceSlug = "test-ns";
|
||||
String publisherId = "admin-user";
|
||||
String skillMdContent = "---\nname: admin-skill\ndescription: Test\nversion: 1.0.0\n---\nBody";
|
||||
|
||||
PackageEntry skillMd = new PackageEntry("SKILL.md", skillMdContent.getBytes(), skillMdContent.length(), "text/markdown");
|
||||
List<PackageEntry> entries = List.of(skillMd);
|
||||
|
||||
Namespace namespace = new Namespace(namespaceSlug, "Test NS", "user-1");
|
||||
setId(namespace, 1L);
|
||||
SkillMetadata metadata = new SkillMetadata("admin-skill", "Test", "1.0.0", "Body", Map.of());
|
||||
Skill skill = new Skill(1L, "admin-skill", publisherId, SkillVisibility.PUBLIC);
|
||||
setId(skill, 1L);
|
||||
|
||||
when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace));
|
||||
when(skillPackageValidator.validate(entries)).thenReturn(ValidationResult.pass());
|
||||
when(skillMetadataParser.parse(skillMdContent)).thenReturn(metadata);
|
||||
when(prePublishValidator.validate(any())).thenReturn(ValidationResult.pass());
|
||||
when(skillRepository.findByNamespaceIdAndSlug(any(), eq("admin-skill"))).thenReturn(List.of(skill));
|
||||
when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(any(), eq("admin-skill"), eq(publisherId))).thenReturn(Optional.of(skill));
|
||||
when(skillVersionRepository.findBySkillIdAndVersion(any(), eq("1.0.0"))).thenReturn(Optional.empty());
|
||||
when(skillVersionRepository.save(any(SkillVersion.class))).thenAnswer(invocation -> {
|
||||
SkillVersion saved = invocation.getArgument(0);
|
||||
if (saved.getId() == null) {
|
||||
setId(saved, 10L);
|
||||
}
|
||||
return saved;
|
||||
});
|
||||
when(skillRepository.save(any())).thenReturn(skill);
|
||||
when(securityScanService.isEnabled()).thenReturn(true);
|
||||
|
||||
SkillPublishService.PublishResult result = service.publishFromEntries(
|
||||
namespaceSlug,
|
||||
entries,
|
||||
publisherId,
|
||||
SkillVisibility.PUBLIC,
|
||||
Set.of("SUPER_ADMIN")
|
||||
);
|
||||
|
||||
assertEquals(SkillVersionStatus.PUBLISHED, result.version().getStatus());
|
||||
verify(securityScanService).triggerScan(eq(10L), anyList(), eq(publisherId));
|
||||
verify(reviewTaskRepository, never()).save(any(ReviewTask.class));
|
||||
}
|
||||
|
||||
private void setId(Object entity, Long id) throws Exception {
|
||||
Field idField = entity.getClass().getDeclaredField("id");
|
||||
idField.setAccessible(true);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue