diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/RedisStreamConfig.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/RedisStreamConfig.java index c084a764..8c479cfe 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/RedisStreamConfig.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/RedisStreamConfig.java @@ -4,6 +4,7 @@ import com.iflytek.skillhub.domain.security.ScanTaskProducer; import com.iflytek.skillhub.domain.security.SecurityScanService; import com.iflytek.skillhub.domain.security.SecurityScanner; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.storage.ObjectStorageService; import com.iflytek.skillhub.stream.RedissonScanTaskProducer; import com.iflytek.skillhub.stream.ScanTaskConsumer; import org.redisson.api.RedissonClient; @@ -45,7 +46,8 @@ public class RedisStreamConfig { SecurityScanner securityScanner, SecurityScanService securityScanService, SkillVersionRepository skillVersionRepository, - ScanTaskProducer scanTaskProducer) { + ScanTaskProducer scanTaskProducer, + ObjectStorageService objectStorageService) { return new ScanTaskConsumer( redissonClient, streamKey, @@ -54,6 +56,7 @@ public class RedisStreamConfig { securityScanService, skillVersionRepository, scanTaskProducer, + objectStorageService, reclaimEnabled, reclaimMinIdle, reclaimBatchSize, diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java index 9502582d..047215d3 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java @@ -30,7 +30,12 @@ public class RedissonScanTaskProducer implements ScanTaskProducer { Map fields = new HashMap<>(); fields.put("taskId", task.taskId()); fields.put("versionId", String.valueOf(task.versionId())); - fields.put("skillPath", task.skillPath()); + if (task.skillPath() != null && !task.skillPath().isBlank()) { + fields.put("skillPath", task.skillPath()); + } + if (task.bundleKey() != null && !task.bundleKey().isBlank()) { + fields.put("bundleKey", task.bundleKey()); + } fields.put("publisherId", task.publisherId() != null ? task.publisherId() : ""); fields.put("createdAtMillis", String.valueOf(task.createdAtMillis())); if (task.metadata() != null) { @@ -39,7 +44,7 @@ public class RedissonScanTaskProducer implements ScanTaskProducer { RStream stream = redissonClient.getStream(streamKey, StringCodec.INSTANCE); StreamMessageId messageId = stream.add(StreamAddArgs.entries(fields)); - log.info("Published scan task: taskId={}, versionId={}, recordId={}", - task.taskId(), task.versionId(), messageId); + log.info("Published scan task: taskId={}, versionId={}, bundleKey={}, hasSkillPath={}, recordId={}", + task.taskId(), task.versionId(), task.bundleKey(), task.skillPath() != null && !task.skillPath().isBlank(), messageId); } } 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 4c956f63..19722d87 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 @@ -9,12 +9,15 @@ import com.iflytek.skillhub.domain.security.SecurityScanService; import com.iflytek.skillhub.domain.security.SecurityScanner; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.storage.ObjectStorageService; import org.redisson.api.RedissonClient; import java.io.IOException; +import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import java.time.Duration; import java.util.Comparator; import java.util.Map; @@ -26,6 +29,7 @@ public class ScanTaskConsumer extends AbstractStreamConsumer version.getStatus() == SkillVersionStatus.SCANNING) .ifPresent(version -> { version.setStatus(SkillVersionStatus.SCAN_FAILED); skillVersionRepository.save(version); }); } finally { - cleanupTempPath(payload.skillPath); + cleanupTempPath(payload.cleanupPath()); } } @Override protected void retryMessage(ScanTaskPayload payload, int retryCount) { + log.warn("Retrying security scan task: taskId={}, versionId={}, scanner={}, nextRetryCount={}, source={}", + payload.taskId(), + payload.versionId(), + payload.scannerType(), + retryCount, + payload.sourceDescription()); + cleanupRetryTempPath(payload); scanTaskProducer.publishScanTask(new ScanTask( - payload.taskId, - payload.versionId, - payload.skillPath, + payload.taskId(), + payload.versionId(), + payload.skillPath(), + payload.bundleKey(), null, System.currentTimeMillis(), - Map.of("retryCount", String.valueOf(retryCount)) + Map.of( + "retryCount", String.valueOf(retryCount), + "scannerType", payload.scannerType().getValue() + ) )); } + private String resolveWorkingSkillPath(ScanTaskPayload payload) { + if (payload.bundleKey() == null) { + if (payload.skillPath() == null || payload.skillPath().isBlank()) { + throw new IllegalStateException("Security scan task missing skillPath and bundleKey"); + } + payload.markWorkingSkillPath(payload.skillPath()); + return payload.skillPath(); + } + + try { + Files.createDirectories(SCAN_TEMP_DIR); + Path tempFile = Files.createTempFile(SCAN_TEMP_DIR, payload.versionId() + "-", ".zip"); + payload.markWorkingSkillPath(tempFile.toString()); + try (InputStream inputStream = objectStorageService.getObject(payload.bundleKey())) { + Files.copy(inputStream, tempFile, StandardCopyOption.REPLACE_EXISTING); + } + log.debug("Staged security scan bundle: taskId={}, versionId={}, bundleKey={}, tempPath={}", + payload.taskId(), payload.versionId(), payload.bundleKey(), tempFile); + return tempFile.toString(); + } catch (Exception e) { + log.error("Failed to stage security scan bundle: taskId={}, versionId={}, bundleKey={}", + payload.taskId(), payload.versionId(), payload.bundleKey(), e); + cleanupTempPath(payload.workingSkillPath()); + throw new IllegalStateException("Failed to stage scan bundle: " + payload.bundleKey(), e); + } + } + + private void cleanupRetryTempPath(ScanTaskPayload payload) { + if (payload.bundleKey() != null) { + cleanupTempPath(payload.workingSkillPath()); + } + } + private void cleanupTempPath(String skillPath) { + if (skillPath == null || skillPath.isBlank()) { + return; + } try { Path path = Paths.get(skillPath).toAbsolutePath().normalize(); if (!path.startsWith(SCAN_TEMP_DIR)) { @@ -165,11 +235,78 @@ public class ScanTaskConsumer extends AbstractStreamConsumer appender; + + @AfterEach + void tearDown() { + if (appender != null) { + logger.detachAppender(appender); + appender.stop(); + } + } + + @Test + void publishScanTask_logsBundleKeyPresence() { + @SuppressWarnings("unchecked") + RStream stream = mock(RStream.class); + @SuppressWarnings("unchecked") + RStream typedStream = (RStream) (RStream) stream; + RedissonClient redissonClient = mock(RedissonClient.class); + doReturn(typedStream).when(redissonClient).getStream("skillhub:scan:requests", StringCodec.INSTANCE); + when(stream.add(any())).thenReturn(new StreamMessageId(1, 0)); + RedissonScanTaskProducer producer = new RedissonScanTaskProducer(redissonClient, "skillhub:scan:requests"); + attachAppender(); + + producer.publishScanTask(new ScanTask( + "task-1", + 42L, + null, + "packages/8/42/bundle.zip", + "publisher-1", + 1711260000000L, + Map.of("scannerType", "skill-scanner") + )); + + assertThat(loggedMessages()).anyMatch(message -> message.contains( + "Published scan task: taskId=task-1, versionId=42, bundleKey=packages/8/42/bundle.zip, hasSkillPath=false" + )); + } + + private void attachAppender() { + logger.setLevel(Level.INFO); + appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + } + + private java.util.List loggedMessages() { + return appender.list.stream() + .map(ILoggingEvent::getFormattedMessage) + .toList(); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerTest.java index 18bfa24d..003c1158 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerTest.java @@ -35,6 +35,7 @@ class RedissonScanTaskProducerTest { "task-1", 42L, "/tmp/skill", + null, "publisher-1", 1711260000000L, Map.of("scannerType", "skill-scanner") 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 new file mode 100644 index 00000000..3175c83f --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java @@ -0,0 +1,347 @@ +package com.iflytek.skillhub.stream; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.security.ScanTaskProducer; +import com.iflytek.skillhub.domain.security.ScannerType; +import com.iflytek.skillhub.domain.security.SecurityScanResponse; +import com.iflytek.skillhub.domain.security.SecurityScanService; +import com.iflytek.skillhub.domain.security.SecurityScanner; +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.storage.ObjectMetadata; +import com.iflytek.skillhub.storage.ObjectStorageService; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.redisson.api.RStream; +import org.redisson.api.RedissonClient; +import org.redisson.api.StreamMessageId; +import org.slf4j.LoggerFactory; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.time.Duration; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +class ScanTaskConsumerLoggingTest { + + private final Logger logger = (Logger) LoggerFactory.getLogger(TestableLoggingConsumer.class); + private ListAppender appender; + + @AfterEach + void tearDown() { + if (appender != null) { + logger.detachAppender(appender); + appender.stop(); + } + } + + @Test + void handleMessage_logsBundleTaskStartAndRetry() { + TestProducer producer = new TestProducer(); + TestableLoggingConsumer consumer = new TestableLoggingConsumer( + new FailingScanner(new IllegalStateException("scanner unavailable")), + new NoOpSecurityScanService(), + new EmptySkillVersionRepository(), + producer, + new InMemoryObjectStorageService(Map.of("packages/8/42/bundle.zip", "zip".getBytes())) + ); + attachAppender(); + + consumer.handleMessage(new StreamMessageId(1, 0), Map.of( + "taskId", "task-1", + "versionId", "42", + "bundleKey", "packages/8/42/bundle.zip", + "scannerType", ScannerType.SKILL_SCANNER.getValue() + )); + + assertThat(loggedMessages()).anyMatch(message -> message.contains( + "Processing security scan task: taskId=task-1, versionId=42, scanner=SKILL_SCANNER, retryCount=0, source=bundleKey:packages/8/42/bundle.zip" + )); + assertThat(loggedMessages()).anyMatch(message -> message.contains( + "Retrying security scan task: taskId=task-1, versionId=42, scanner=SKILL_SCANNER, nextRetryCount=1" + )); + assertThat(producer.lastMetadata).containsEntry("retryCount", "1"); + } + + @Test + void handleMessage_logsBundleStageFailureBeforeRetry() { + TestableLoggingConsumer consumer = new TestableLoggingConsumer( + new FailingScanner(null), + new NoOpSecurityScanService(), + new EmptySkillVersionRepository(), + new TestProducer(), + new InMemoryObjectStorageService(new IllegalStateException("bundle missing")) + ); + attachAppender(); + + consumer.handleMessage(new StreamMessageId(2, 0), Map.of( + "taskId", "task-2", + "versionId", "43", + "bundleKey", "packages/8/43/bundle.zip", + "scannerType", ScannerType.SKILL_SCANNER.getValue() + )); + + assertThat(loggedMessages()).anyMatch(message -> message.contains( + "Failed to stage security scan bundle: taskId=task-2, versionId=43, bundleKey=packages/8/43/bundle.zip" + )); + } + + @Test + void handleMessage_logsFinalFailureAfterRetriesExhausted() { + SkillVersion version = new SkillVersion(8L, "1.0.0", "publisher-1"); + setVersionId(version, 44L); + version.setStatus(SkillVersionStatus.SCANNING); + TestableLoggingConsumer consumer = new TestableLoggingConsumer( + new FailingScanner(new IllegalStateException("scanner unavailable")), + new NoOpSecurityScanService(), + new SingleSkillVersionRepository(version), + new TestProducer(), + new InMemoryObjectStorageService(Map.of("packages/8/44/bundle.zip", "zip".getBytes())) + ); + attachAppender(); + + consumer.handleMessage(new StreamMessageId(3, 0), Map.of( + "taskId", "task-3", + "versionId", "44", + "bundleKey", "packages/8/44/bundle.zip", + "scannerType", ScannerType.SKILL_SCANNER.getValue(), + "retryCount", "3" + )); + + assertThat(loggedMessages()).anyMatch(message -> message.contains( + "Security scan task failed permanently: taskId=task-3, versionId=44, scanner=SKILL_SCANNER" + )); + } + + private void attachAppender() { + logger.setLevel(Level.INFO); + appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + } + + private List loggedMessages() { + return appender.list.stream() + .map(ILoggingEvent::getFormattedMessage) + .toList(); + } + + private void setVersionId(SkillVersion version, Long id) { + try { + java.lang.reflect.Field field = SkillVersion.class.getDeclaredField("id"); + field.setAccessible(true); + field.set(version, id); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + + private static final class TestableLoggingConsumer extends ScanTaskConsumer { + private final RStream stream = mock(RStream.class); + + private TestableLoggingConsumer(SecurityScanner securityScanner, + SecurityScanService securityScanService, + SkillVersionRepository skillVersionRepository, + ScanTaskProducer scanTaskProducer, + ObjectStorageService objectStorageService) { + super( + mock(RedissonClient.class), + "skillhub:scan:requests", + "skillhub-scanners", + securityScanner, + securityScanService, + skillVersionRepository, + scanTaskProducer, + objectStorageService + ); + } + + @Override + protected RStream createStream() { + return stream; + } + } + + private static final class FailingScanner implements SecurityScanner { + private final RuntimeException failure; + + private FailingScanner(RuntimeException failure) { + this.failure = failure; + } + + @Override + public SecurityScanResponse scan(com.iflytek.skillhub.domain.security.SecurityScanRequest request) { + if (failure != null) { + throw failure; + } + throw new IllegalStateException("unexpected"); + } + + @Override + public boolean isHealthy() { + return true; + } + + @Override + public String getScannerType() { + return "skill-scanner"; + } + } + + private static final class NoOpSecurityScanService extends SecurityScanService { + private NoOpSecurityScanService() { + super(null, null, task -> {}, new ObjectMapper(), "upload", true); + } + + @Override + public void processScanResult(Long versionId, ScannerType scannerType, SecurityScanResponse response) { + } + } + + private static final class TestProducer implements ScanTaskProducer { + private Map lastMetadata; + + @Override + public void publishScanTask(com.iflytek.skillhub.domain.security.ScanTask task) { + this.lastMetadata = task.metadata(); + } + } + + private static class EmptySkillVersionRepository implements SkillVersionRepository { + @Override + public Optional findById(Long id) { + return Optional.empty(); + } + + @Override + public List findByIdIn(List ids) { + throw new UnsupportedOperationException(); + } + + @Override + public List findBySkillIdIn(List skillIds) { + throw new UnsupportedOperationException(); + } + + @Override + public List findBySkillIdInAndStatus(List skillIds, SkillVersionStatus status) { + throw new UnsupportedOperationException(); + } + + @Override + public List findBySkillId(Long skillId) { + throw new UnsupportedOperationException(); + } + + @Override + public Optional findBySkillIdAndVersion(Long skillId, String version) { + throw new UnsupportedOperationException(); + } + + @Override + public List findBySkillIdAndStatus(Long skillId, SkillVersionStatus status) { + throw new UnsupportedOperationException(); + } + + @Override + public SkillVersion save(SkillVersion version) { + return version; + } + + @Override + public void delete(SkillVersion version) { + throw new UnsupportedOperationException(); + } + + @Override + public void flush() { + } + + @Override + public void deleteBySkillId(Long skillId) { + throw new UnsupportedOperationException(); + } + } + + private static final class SingleSkillVersionRepository extends EmptySkillVersionRepository { + private final SkillVersion version; + + private SingleSkillVersionRepository(SkillVersion version) { + this.version = version; + } + + @Override + public Optional findById(Long id) { + return Optional.of(version); + } + } + + private static final class InMemoryObjectStorageService implements ObjectStorageService { + private final Map objects; + private final RuntimeException failure; + + private InMemoryObjectStorageService(Map objects) { + this.objects = objects; + this.failure = null; + } + + private InMemoryObjectStorageService(RuntimeException failure) { + this.objects = Map.of(); + this.failure = failure; + } + + @Override + public void putObject(String key, InputStream data, long size, String contentType) { + throw new UnsupportedOperationException(); + } + + @Override + public InputStream getObject(String key) { + if (failure != null) { + throw failure; + } + byte[] content = objects.get(key); + if (content == null) { + throw new IllegalStateException("missing: " + key); + } + return new ByteArrayInputStream(content); + } + + @Override + public void deleteObject(String key) { + throw new UnsupportedOperationException(); + } + + @Override + public void deleteObjects(List keys) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean exists(String key) { + return objects.containsKey(key); + } + + @Override + public ObjectMetadata getMetadata(String key) { + byte[] content = objects.get(key); + return new ObjectMetadata(content.length, "application/zip", Instant.now()); + } + + @Override + public String generatePresignedUrl(String key, Duration expiry, String downloadFilename) { + throw new UnsupportedOperationException(); + } + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerPathSafetyTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerPathSafetyTest.java index 41721746..984acbd2 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerPathSafetyTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerPathSafetyTest.java @@ -6,6 +6,7 @@ import com.iflytek.skillhub.domain.security.ScanTaskProducer; import com.iflytek.skillhub.domain.security.SecurityScanService; import com.iflytek.skillhub.domain.security.SecurityScanner; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.storage.ObjectStorageService; import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.Path; @@ -23,7 +24,8 @@ class ScanTaskConsumerPathSafetyTest { org.mockito.Mockito.mock(SecurityScanner.class), org.mockito.Mockito.mock(SecurityScanService.class), org.mockito.Mockito.mock(SkillVersionRepository.class), - org.mockito.Mockito.mock(ScanTaskProducer.class) + org.mockito.Mockito.mock(ScanTaskProducer.class), + org.mockito.Mockito.mock(ObjectStorageService.class) ); Path outsideFile = Files.createTempFile("scan-cleanup-", ".txt"); Files.writeString(outsideFile, "keep"); 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 275b9403..66bd9864 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 @@ -14,18 +14,28 @@ import com.iflytek.skillhub.domain.security.SecurityVerdict; 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.storage.ObjectStorageService; +import com.iflytek.skillhub.storage.ObjectMetadata; import org.junit.jupiter.api.Test; +import org.redisson.api.RStream; +import org.redisson.api.RedissonClient; +import org.redisson.api.StreamMessageId; +import java.io.ByteArrayInputStream; +import java.io.InputStream; import java.io.IOException; import java.lang.reflect.Field; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Instant; +import java.time.Duration; import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; class ScanTaskConsumerTest { private static final Path SCAN_TEMP_DIR = Path.of("/tmp/skillhub-scans"); @@ -46,12 +56,19 @@ class ScanTaskConsumerTest { securityScanner, securityScanService, new InMemorySkillVersionRepository(), - new InMemoryScanTaskProducer() + new InMemoryScanTaskProducer(), + new InMemoryObjectStorageService() ); Files.createDirectories(SCAN_TEMP_DIR); Path tempDir = Files.createTempDirectory(SCAN_TEMP_DIR, "scan-task-consumer-success"); Files.writeString(tempDir.resolve("README.md"), "# demo"); - ScanTaskConsumer.ScanTaskPayload payload = new ScanTaskConsumer.ScanTaskPayload("task-1", 42L, tempDir.toString(), ScannerType.SKILL_SCANNER); + ScanTaskConsumer.ScanTaskPayload payload = new ScanTaskConsumer.ScanTaskPayload( + "task-1", + 42L, + tempDir.toString(), + null, + ScannerType.SKILL_SCANNER + ); consumer.invokeProcessBusiness(payload); consumer.invokeMarkCompleted(payload); @@ -80,11 +97,18 @@ class ScanTaskConsumerTest { new StubSecurityScanner(), new StubSecurityScanService(), skillVersionRepository, - new InMemoryScanTaskProducer() + new InMemoryScanTaskProducer(), + new InMemoryObjectStorageService() ); Files.createDirectories(SCAN_TEMP_DIR); Path tempFile = Files.createTempFile(SCAN_TEMP_DIR, "scan-task-consumer-failure", ".zip"); - ScanTaskConsumer.ScanTaskPayload payload = new ScanTaskConsumer.ScanTaskPayload("task-2", 42L, tempFile.toString(), ScannerType.SKILL_SCANNER); + ScanTaskConsumer.ScanTaskPayload payload = new ScanTaskConsumer.ScanTaskPayload( + "task-2", + 42L, + tempFile.toString(), + null, + ScannerType.SKILL_SCANNER + ); consumer.invokeMarkFailed(payload, "scan failed"); @@ -101,9 +125,16 @@ class ScanTaskConsumerTest { new StubSecurityScanner(), new StubSecurityScanService(), new InMemorySkillVersionRepository(), - producer + producer, + new InMemoryObjectStorageService() + ); + ScanTaskConsumer.ScanTaskPayload payload = new ScanTaskConsumer.ScanTaskPayload( + "task-3", + 77L, + "/tmp/retry", + null, + ScannerType.SKILL_SCANNER ); - ScanTaskConsumer.ScanTaskPayload payload = new ScanTaskConsumer.ScanTaskPayload("task-3", 77L, "/tmp/retry", ScannerType.SKILL_SCANNER); consumer.invokeRetryMessage(payload, 2); @@ -112,31 +143,174 @@ class ScanTaskConsumerTest { 77L, "/tmp/retry", null, + null, producer.publishedTask.createdAtMillis(), - Map.of("retryCount", "2") + Map.of( + "retryCount", "2", + "scannerType", ScannerType.SKILL_SCANNER.getValue() + ) )); } + @Test + void processBusiness_withBundleKey_downloadsPackageFromObjectStorageAndCleansTempFile() throws Exception { + byte[] packageBytes = "zip-bytes".getBytes(); + InMemoryObjectStorageService objectStorageService = new InMemoryObjectStorageService(Map.of( + "packages/8/42/bundle.zip", packageBytes + )); + StubSecurityScanner securityScanner = new StubSecurityScanner(); + securityScanner.response = new SecurityScanResponse( + "scan-4", + SecurityVerdict.SAFE, + 0, + "LOW", + List.of(), + 0.4 + ); + StubSecurityScanService securityScanService = new StubSecurityScanService(); + TestableScanTaskConsumer consumer = new TestableScanTaskConsumer( + securityScanner, + securityScanService, + new InMemorySkillVersionRepository(), + new InMemoryScanTaskProducer(), + objectStorageService + ); + ScanTaskConsumer.ScanTaskPayload payload = new ScanTaskConsumer.ScanTaskPayload( + "task-4", + 42L, + null, + "packages/8/42/bundle.zip", + ScannerType.SKILL_SCANNER + ); + + consumer.invokeProcessBusiness(payload); + + Path downloadedPackage = Path.of(securityScanner.lastRequest.skillPackagePath()); + assertThat(downloadedPackage).startsWith(SCAN_TEMP_DIR); + assertThat(Files.readAllBytes(downloadedPackage)).isEqualTo(packageBytes); + assertThat(objectStorageService.lastGetKey).isEqualTo("packages/8/42/bundle.zip"); + + consumer.invokeMarkCompleted(payload); + + assertThat(Files.exists(downloadedPackage)).isFalse(); + } + + @Test + void handleMessage_retryableScannerFailureWithBundleKey_requeuesTaskAndCleansStagedTempFile() throws Exception { + long versionId = 42424242L; + byte[] packageBytes = "zip-bytes".getBytes(); + InMemoryObjectStorageService objectStorageService = new InMemoryObjectStorageService(Map.of( + "packages/8/42424242/bundle.zip", packageBytes + )); + deleteScanTempFiles(versionId); + StubSecurityScanner securityScanner = new StubSecurityScanner(); + securityScanner.failure = new IllegalStateException("scanner unavailable"); + InMemoryScanTaskProducer producer = new InMemoryScanTaskProducer(); + InMemorySkillVersionRepository repository = new InMemorySkillVersionRepository(); + TestableScanTaskConsumer consumer = new TestableScanTaskConsumer( + securityScanner, + new StubSecurityScanService(), + repository, + producer, + objectStorageService + ); + + consumer.handleMessage(new StreamMessageId(9, 0), Map.of( + "taskId", "task-5", + "versionId", String.valueOf(versionId), + "bundleKey", "packages/8/42424242/bundle.zip", + "scannerType", ScannerType.SKILL_SCANNER.getValue() + )); + + assertThat(producer.publishedTask).isEqualTo(new ScanTask( + "task-5", + versionId, + null, + "packages/8/42424242/bundle.zip", + null, + producer.publishedTask.createdAtMillis(), + Map.of( + "retryCount", "1", + "scannerType", ScannerType.SKILL_SCANNER.getValue() + ) + )); + assertThat(repository.savedVersion).isNull(); + assertThat(listScanTempFiles(versionId)).isEmpty(); + } + + @Test + void handleMessage_retryableBundleDownloadFailure_requeuesTaskWithoutLeakingTempFile() throws Exception { + long versionId = 43434343L; + InMemoryObjectStorageService objectStorageService = new InMemoryObjectStorageService(); + objectStorageService.getFailure = new IllegalStateException("missing bundle"); + deleteScanTempFiles(versionId); + InMemoryScanTaskProducer producer = new InMemoryScanTaskProducer(); + TestableScanTaskConsumer consumer = new TestableScanTaskConsumer( + new StubSecurityScanner(), + new StubSecurityScanService(), + new InMemorySkillVersionRepository(), + producer, + objectStorageService + ); + + consumer.handleMessage(new StreamMessageId(10, 0), Map.of( + "taskId", "task-6", + "versionId", String.valueOf(versionId), + "bundleKey", "packages/8/43434343/bundle.zip", + "scannerType", ScannerType.SKILL_SCANNER.getValue() + )); + + assertThat(producer.publishedTask.bundleKey()).isEqualTo("packages/8/43434343/bundle.zip"); + assertThat(producer.publishedTask.metadata()).containsEntry("retryCount", "1"); + assertThat(listScanTempFiles(versionId)).isEmpty(); + } + private void setField(Object target, String fieldName, Object value) throws Exception { Field field = target.getClass().getDeclaredField(fieldName); field.setAccessible(true); field.set(target, value); } + private List listScanTempFiles(long versionId) throws IOException { + Files.createDirectories(SCAN_TEMP_DIR); + try (var stream = Files.list(SCAN_TEMP_DIR)) { + return stream + .filter(path -> path.getFileName().toString().startsWith(versionId + "-")) + .toList(); + } + } + + private void deleteScanTempFiles(long versionId) throws IOException { + for (Path path : listScanTempFiles(versionId)) { + Files.deleteIfExists(path); + } + } + private static final class TestableScanTaskConsumer extends ScanTaskConsumer { + private final RStream stream; + + @SuppressWarnings("unchecked") private TestableScanTaskConsumer(SecurityScanner securityScanner, SecurityScanService securityScanService, SkillVersionRepository skillVersionRepository, - ScanTaskProducer scanTaskProducer) { + ScanTaskProducer scanTaskProducer, + ObjectStorageService objectStorageService) { super( - null, + mock(RedissonClient.class), "skillhub:scan:requests", "skillhub-scanners", securityScanner, securityScanService, skillVersionRepository, - scanTaskProducer + scanTaskProducer, + objectStorageService ); + this.stream = mock(RStream.class); + } + + @Override + protected RStream createStream() { + return stream; } private void invokeProcessBusiness(ScanTaskPayload payload) { @@ -159,10 +333,14 @@ class ScanTaskConsumerTest { private static final class StubSecurityScanner implements SecurityScanner { private SecurityScanRequest lastRequest; private SecurityScanResponse response; + private RuntimeException failure; @Override public SecurityScanResponse scan(SecurityScanRequest request) { this.lastRequest = request; + if (failure != null) { + throw failure; + } return response; } @@ -329,6 +507,67 @@ class ScanTaskConsumerTest { } } + private static final class InMemoryObjectStorageService implements ObjectStorageService { + private final Map objects; + private String lastGetKey; + private RuntimeException getFailure; + + private InMemoryObjectStorageService() { + this(Map.of()); + } + + private InMemoryObjectStorageService(Map objects) { + this.objects = new java.util.HashMap<>(objects); + } + + @Override + public void putObject(String key, InputStream data, long size, String contentType) { + throw unsupported(); + } + + @Override + public InputStream getObject(String key) { + lastGetKey = key; + if (getFailure != null) { + throw getFailure; + } + byte[] content = objects.get(key); + if (content == null) { + throw new IllegalStateException("Missing object: " + key); + } + return new ByteArrayInputStream(content); + } + + @Override + public void deleteObject(String key) { + throw unsupported(); + } + + @Override + public void deleteObjects(List keys) { + throw unsupported(); + } + + @Override + public boolean exists(String key) { + return objects.containsKey(key); + } + + @Override + public ObjectMetadata getMetadata(String key) { + byte[] content = objects.get(key); + if (content == null) { + throw new IllegalStateException("Missing object: " + key); + } + return new ObjectMetadata(content.length, "application/zip", Instant.now()); + } + + @Override + public String generatePresignedUrl(String key, Duration expiry, String downloadFilename) { + throw unsupported(); + } + } + private static UnsupportedOperationException unsupported() { return new UnsupportedOperationException(); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/ScanTask.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/ScanTask.java index 53dcee4a..a04848da 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/ScanTask.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/security/ScanTask.java @@ -6,6 +6,7 @@ public record ScanTask( String taskId, Long versionId, String skillPath, + String bundleKey, String publisherId, long createdAtMillis, Map metadata 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 a290be3c..195510a5 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 @@ -12,7 +12,6 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -22,8 +21,6 @@ import java.time.Instant; import java.util.List; import java.util.Map; import java.util.UUID; -import java.util.zip.ZipEntry; -import java.util.zip.ZipOutputStream; @Service public class SecurityScanService { @@ -67,13 +64,21 @@ public class SecurityScanService { SkillVersion version = skillVersionRepository.findById(versionId) .orElseThrow(() -> new IllegalStateException("SkillVersion not found: " + versionId)); - String packagePath = resolvePackagePath(versionId, entries).toString(); + String packagePath = null; + String bundleKey = null; + if ("upload".equalsIgnoreCase(scanMode)) { + validateUploadEntries(entries); + bundleKey = buildBundleStorageKey(version.getSkillId(), versionId); + } else { + packagePath = saveTempDirectory(versionId, entries).toString(); + } // Always create a new audit record — supports multiple rounds per version auditRepository.save(new SecurityAudit(versionId, ScannerType.SKILL_SCANNER)); scanTaskProducer.publishScanTask(new ScanTask( UUID.randomUUID().toString(), versionId, packagePath, + bundleKey, publisherId, System.currentTimeMillis(), Map.of("scannerType", ScannerType.SKILL_SCANNER.getValue()) @@ -104,13 +109,6 @@ public class SecurityScanService { skillVersionRepository.save(version); } - private Path resolvePackagePath(Long versionId, List entries) { - if ("upload".equalsIgnoreCase(scanMode)) { - return saveTempZip(versionId, entries); - } - return saveTempDirectory(versionId, entries); - } - private Path saveTempDirectory(Long versionId, List entries) { try { Path skillDir = TEMP_BASE_DIR.resolve(String.valueOf(versionId)).normalize(); @@ -129,29 +127,16 @@ public class SecurityScanService { } } - private Path saveTempZip(Long versionId, List entries) { - try { - Path dir = TEMP_BASE_DIR; - Files.createDirectories(dir); - Path zipPath = dir.resolve(versionId + ".zip"); - - try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); - ZipOutputStream zos = new ZipOutputStream(baos)) { - for (PackageEntry entry : entries) { - zos.putNextEntry(new ZipEntry(safeZipEntryName(entry.path()))); - zos.write(entry.content()); - zos.closeEntry(); - } - zos.finish(); - Files.write(zipPath, baos.toByteArray()); - } - - return zipPath; - } catch (IOException e) { - throw new IllegalStateException("Failed to save temp ZIP for versionId: " + versionId, e); + private void validateUploadEntries(List entries) { + for (PackageEntry entry : entries) { + safeZipEntryName(entry.path()); } } + private String buildBundleStorageKey(Long skillId, Long versionId) { + return String.format("packages/%d/%d/bundle.zip", skillId, versionId); + } + private String serializeFindings(List findings) { try { return objectMapper.writeValueAsString(findings); 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 2b67bece..8b4a2d81 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 @@ -89,6 +89,39 @@ class SecurityScanServiceTest { assertThat(task.versionId()).isEqualTo(42L); assertThat(task.publisherId()).isEqualTo("publisher-1"); assertThat(task.skillPath()).contains("42"); + assertThat(task.bundleKey()).isNull(); + } + + @Test + void triggerScan_uploadModePublishesBundleKeyWithoutLocalTempPath() throws Exception { + service = new SecurityScanService( + auditRepository, + skillVersionRepository, + scanTaskProducer, + new ObjectMapper(), + "upload", + true + ); + SkillVersion version = new SkillVersion(8L, "1.0.0", "publisher-1"); + setId(version, 42L); + 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"); + + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(ScanTask.class); + verify(scanTaskProducer).publishScanTask(taskCaptor.capture()); + + ScanTask task = taskCaptor.getValue(); + assertThat(task.versionId()).isEqualTo(42L); + assertThat(task.skillPath()).isNull(); + assertThat(task.bundleKey()).isEqualTo("packages/8/42/bundle.zip"); } @Test diff --git a/web/src/features/skill/install-command.test.ts b/web/src/features/skill/install-command.test.ts index 6ad22188..961d46b7 100644 --- a/web/src/features/skill/install-command.test.ts +++ b/web/src/features/skill/install-command.test.ts @@ -1,5 +1,13 @@ -import { afterEach, describe, expect, it } from 'vitest' -import { buildInstallCommand, buildInstallTarget, getBaseUrl } from './install-command' +import { createElement } from 'react' +import { renderToStaticMarkup } from 'react-dom/server' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { InstallCommand, buildInstallCommand, buildInstallTarget, getBaseUrl } from './install-command' + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string) => key, + }), +})) describe('install-command', () => { const originalWindow = globalThis.window @@ -64,4 +72,14 @@ describe('install-command', () => { setMockWindow() expect(getBaseUrl()).toBe('https://fallback.example.com') }) + + it('renders the install command in a more compact code block', () => { + setMockWindow('http://localhost:3000') + + const html = renderToStaticMarkup(createElement(InstallCommand, { namespace: 'global', slug: 'meeting-minutes-generator' })) + + expect(html).toContain('px-4 py-3') + expect(html).toContain('leading-relaxed') + expect(html).toContain('break-all') + }) }) diff --git a/web/src/features/skill/install-command.tsx b/web/src/features/skill/install-command.tsx index 95369dea..47a09373 100644 --- a/web/src/features/skill/install-command.tsx +++ b/web/src/features/skill/install-command.tsx @@ -61,8 +61,8 @@ export function InstallCommand({ namespace, slug }: InstallCommandProps) { > {copied ? : } - 
-        
+      
+        
           {command}
         
       

diff --git a/web/src/pages/skill-detail.test.tsx b/web/src/pages/skill-detail.test.tsx
index aa10dea3..deed65d3 100644
--- a/web/src/pages/skill-detail.test.tsx
+++ b/web/src/pages/skill-detail.test.tsx
@@ -323,4 +323,47 @@ describe('SkillDetailPage', () => {
     expect(html).toContain('skillDetail.versionStatusPendingReview')
     expect(html).not.toContain('skillDetail.versionStatusScanFailed')
   })
+
+  it('allows long pending review versions to wrap inside the review card', () => {
+    useSkillDetailMock.mockReturnValue({
+      data: createSkill({
+        headlineVersion: { id: 13, version: '20260326.055640-build-with-very-long-suffix', status: 'PENDING_REVIEW' },
+        publishedVersion: { id: 10, version: '20260326.055538', status: 'PUBLISHED' },
+        ownerPreviewVersion: { id: 13, version: '20260326.055640-build-with-very-long-suffix', status: 'PENDING_REVIEW' },
+        resolutionMode: 'PUBLISHED',
+      }),
+      isLoading: false,
+      isFetching: false,
+      error: null,
+    })
+    useSkillVersionsMock.mockReturnValue({
+      data: [
+        {
+          id: 13,
+          version: '20260326.055640-build-with-very-long-suffix',
+          status: 'PENDING_REVIEW',
+          changelog: '',
+          fileCount: 1,
+          totalSize: 12,
+          publishedAt: null,
+          downloadAvailable: false,
+        },
+        {
+          id: 10,
+          version: '20260326.055538',
+          status: 'PUBLISHED',
+          changelog: '',
+          fileCount: 1,
+          totalSize: 12,
+          publishedAt: '2026-03-20T00:00:00Z',
+          downloadAvailable: true,
+        },
+      ],
+    })
+
+    const html = renderToStaticMarkup()
+
+    expect(html).toContain('break-all')
+    expect(html).toContain('leading-snug')
+  })
 })
diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx
index d2c4b920..b37e7ed7 100644
--- a/web/src/pages/skill-detail.tsx
+++ b/web/src/pages/skill-detail.tsx
@@ -940,7 +940,7 @@ export function SkillDetailPage() {
         
           

             
{t('skillDetail.version')}

-            

+            

               {headlineVersion ? `v${headlineVersion.version}` : '—'}
             

           

@@ -1034,7 +1034,7 @@ export function SkillDetailPage() {
                 

                   {t('skillDetail.pendingReviewVersionLabel')}
                 

-                

+                

                   v{ownerPreviewVersion.version}