diff --git a/docs/security-scanning.md b/docs/security-scanning.md index 3686d474..ffdf6356 100644 --- a/docs/security-scanning.md +++ b/docs/security-scanning.md @@ -49,6 +49,7 @@ skillhub: key: skillhub:scan:requests group: skillhub-scanners reclaim-min-idle: PT16M + max-unavailable-age: PT1H ``` Important environment variables: @@ -60,6 +61,7 @@ Important environment variables: - `SKILLHUB_SCAN_STREAM_KEY` - `SKILLHUB_SCAN_STREAM_GROUP` - `SKILLHUB_SCAN_STREAM_RECLAIM_MIN_IDLE` +- `SKILLHUB_SECURITY_STREAM_MAX_UNAVAILABLE_AGE` Scanner-side optional environment variables: @@ -134,7 +136,9 @@ Response fields include: ## Failure Semantics - scan task retries are handled by `AbstractStreamConsumer` -- final failure marks the version as `SCAN_FAILED` +- scanner connection failures, HTTP 429, and HTTP 5xx remain pending for automatic recovery +- unavailable tasks older than `max-unavailable-age` are marked `SCAN_FAILED`, acknowledged, and removed from the Redis Stream +- other final failures mark the version as `SCAN_FAILED` after retry exhaustion - even after scan failure, a review task is still created so the package does not get stuck forever This keeps the existing human review path intact while making scanner failures visible. 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 484c3242..ccb1356e 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 @@ -8,12 +8,13 @@ import com.iflytek.skillhub.observability.MessageObservationSupport; import com.iflytek.skillhub.storage.ObjectStorageService; import com.iflytek.skillhub.stream.RedissonScanTaskProducer; import com.iflytek.skillhub.stream.ScanTaskConsumer; +import java.time.Clock; +import java.time.Duration; import org.redisson.api.RedissonClient; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import java.time.Duration; @Configuration @ConditionalOnProperty(prefix = "skillhub.security.scanner", name = "enabled", havingValue = "true") @@ -40,6 +41,9 @@ public class RedisStreamConfig { @Value("${skillhub.security.scanner.retry-max-attempts:3}") private int maxRetryAttempts; + @Value("${skillhub.security.stream.max-unavailable-age:PT1H}") + private Duration maxUnavailableAge; + @Bean public RedissonScanTaskProducer redisScanTaskProducer( RedissonClient redissonClient, @@ -55,6 +59,7 @@ public class RedisStreamConfig { SkillVersionRepository skillVersionRepository, ScanTaskProducer scanTaskProducer, ObjectStorageService objectStorageService, + Clock clock, MessageObservationSupport messageObservationSupport) { return new ScanTaskConsumer( redissonClient, @@ -70,6 +75,8 @@ public class RedisStreamConfig { reclaimBatchSize, reclaimInterval, maxRetryAttempts, + maxUnavailableAge, + clock, messageObservationSupport ); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java index a90bb4a8..962db1b7 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java @@ -247,7 +247,7 @@ public abstract class AbstractStreamConsumer { } private void handleFailure(T payload, int retryCount, Exception e) { - if (retryCount < maxRetryCount()) { + if (shouldRetry(payload, e, retryCount)) { // Retry publication remains inside the current consumer scope, so the new producer // span and message carrier continue the original trace. retryMessage(payload, retryCount + 1); @@ -293,6 +293,10 @@ public abstract class AbstractStreamConsumer { return DEFAULT_MAX_RETRY_COUNT; } + protected boolean shouldRetry(T payload, Exception error, int retryCount) { + return retryCount < maxRetryCount(); + } + protected boolean shouldDeferFailure(T payload, Exception error) { return false; } 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 37579015..51fcfeac 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 @@ -21,12 +21,18 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; +import java.time.Clock; +import java.time.DateTimeException; import java.time.Duration; +import java.time.Instant; import java.util.Comparator; import java.util.Map; +import java.util.Objects; public class ScanTaskConsumer extends AbstractStreamConsumer { private static final Path SCAN_TEMP_DIR = Paths.get("/tmp/skillhub-scans").toAbsolutePath().normalize(); + private static final Duration DEFAULT_MAX_UNAVAILABLE_AGE = Duration.ofHours(1); + private static final Duration MAX_CLOCK_SKEW = Duration.ofMinutes(5); private final RedissonClient redissonClient; private final SecurityScanner securityScanner; @@ -35,6 +41,8 @@ public class ScanTaskConsumer extends AbstractStreamConsumer now.plus(MAX_CLOCK_SKEW).toEpochMilli()) { + return true; + } + try { + return !Instant.ofEpochMilli(createdAtMillis).plus(maxUnavailableAge).isAfter(now); + } catch (DateTimeException | ArithmeticException ignored) { + return true; + } + } + + private Duration taskAge(ScanTaskPayload payload) { + try { + Duration age = Duration.between(Instant.ofEpochMilli(payload.createdAtMillis()), clock.instant()); + return age.isNegative() ? Duration.ZERO : age; + } catch (DateTimeException | ArithmeticException ignored) { + return maxUnavailableAge; + } + } + + private long parseCreatedAtMillis(String messageId, String value) { + Long createdAt = parsePositiveLong(value); + if (createdAt != null) { + return createdAt; + } + int separator = messageId.indexOf('-'); + String redisTimestamp = separator >= 0 ? messageId.substring(0, separator) : messageId; + Long fallback = parsePositiveLong(redisTimestamp); + return fallback != null ? fallback : 0L; + } + + private Long parsePositiveLong(String value) { + if (value == null || value.isBlank()) { + return null; + } + try { + long parsed = Long.parseLong(value); + return parsed > 0 ? parsed : null; + } catch (NumberFormatException ignored) { + return null; + } + } + protected static final class ScanTaskPayload { private final String taskId; private final Long versionId; @@ -322,11 +401,12 @@ public class ScanTaskConsumer extends AbstractStreamConsumer