Change: make malicious zip file checks a synchronous PublishCheck

server/src/dev/resources/application.yml

@@ -206,7 +206,7 @@ ovsx:
         "contact": "infrastructure@eclipse-foundation.org"
       }
   scanning:
-    enabled: false
+    enabled: true
     # Shared archive limits for all scanning checks (secret detection, blocklist, etc.)
     max-archive-size-bytes: 1073741824   # 1 GB total archive limit
     max-single-file-bytes: 268435456     # 256 MB per-file limit

server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java

@@ -20,11 +20,8 @@
 import org.eclipse.openvsx.UserService;
 import org.eclipse.openvsx.adapter.VSCodeIdNewExtensionJobRequest;
 import org.eclipse.openvsx.entities.*;
-import org.eclipse.openvsx.entities.ScanCheckResult.CheckCategory;
-import org.eclipse.openvsx.entities.ScanCheckResult.CheckResult;
 import org.eclipse.openvsx.extension_control.ExtensionControlService;
 import org.eclipse.openvsx.repositories.RepositoryService;
-import org.eclipse.openvsx.scanning.ExtensionScanPersistenceService;
 import org.eclipse.openvsx.scanning.ExtensionScanService;
 import org.eclipse.openvsx.util.*;
 import org.jobrunr.scheduling.JobRequestScheduler;
@@ -58,7 +55,6 @@ public class PublishExtensionVersionHandler {
     private final ExtensionValidator validator;
     private final ExtensionControlService extensionControl;
     private final ExtensionScanService scanService;
-    private final ExtensionScanPersistenceService scanPersistenceService;
 
     public PublishExtensionVersionHandler(
             PublishExtensionVersionService service,
@@ -69,8 +65,7 @@ public PublishExtensionVersionHandler(
             UserService users,
             ExtensionValidator validator,
             ExtensionControlService extensionControl,
-            ExtensionScanService scanService,
-            ExtensionScanPersistenceService scanPersistenceService
+            ExtensionScanService scanService
     ) {
         this.service = service;
         this.integrityService = integrityService;
@@ -81,7 +76,6 @@ public PublishExtensionVersionHandler(
         this.validator = validator;
         this.extensionControl = extensionControl;
         this.scanService = scanService;
-        this.scanPersistenceService = scanPersistenceService;
     }
 
     public boolean isLicenseRequired() {
@@ -276,48 +270,15 @@ private void doPublish(TempFile extensionFile, ExtensionService extensionService
 
         service.storeResource(extensionFile);
         service.persistResource(download);
-        try(var processor = new ExtensionProcessor(extensionFile)) {
-            extVersion.setPotentiallyMalicious(processor.isPotentiallyMalicious());
-            if (extVersion.isPotentiallyMalicious()) {
+        try (var processor = new ExtensionProcessor(extensionFile)) {
+            // to keep backwards compatibility, mark extension versions as potentially malicious
+            // if no scan service is enabled and the vsix file contains entries with extra fields.
+            if (!scanService.isEnabled() && processor.isPotentiallyMalicious()) {
+                service.markExtensionAsPotentiallyMalicious(extVersion);
                 logger.atWarn()
                         .setMessage("Extension version is potentially malicious: {}")
                         .addArgument(() -> NamingUtil.toLogFormat(extVersion))
                         .log();
-
-                // Record as a publish check failure and reject the extension
-                 if (scan != null) {
-                    var now = TimeUtil.getCurrentUTC();
-                    var checkType = "MALICIOUS_ZIP_CHECK";
-                    var reason = "VSIX contains zip entries with potentially harmful extra fields";
-
-                    // Record the check result for audit trail
-                    scanPersistenceService.recordCheckResult(
-                            scan,
-                            checkType,
-                            CheckCategory.PUBLISH_CHECK,
-                            CheckResult.REJECT,
-                            now,                     // startedAt
-                            now,                     // completedAt
-                            1,                       // filesScanned - the vsix file
-                            1,                       // findingsCount
-                            reason,
-                            null,                    // errorMessage
-                            null,                    // scannerJobId - not a scanner job
-                            true
-                    );
-
-                    // Also record as validation failure for the failures list
-                    scanPersistenceService.recordValidationFailure(
-                            scan,
-                            checkType,
-                            "EXTRA_FIELDS_DETECTED",  // ruleName
-                            reason,
-                            true                      // enforced
-                    );
-
-                    scanService.rejectScan(scan);
-                    logger.info("Scan {} rejected due to potentially malicious extension", scan.getId());
-                }
                 return;
             }
 
@@ -326,7 +287,7 @@ private void doPublish(TempFile extensionFile, ExtensionService extensionService
                 service.persistResource(tempFile.getResource());
             };
 
-            if(integrityService.isEnabled()) {
+            if (integrityService.isEnabled()) {
                 var keyPair = extVersion.getSignatureKeyPair();
                 if(keyPair != null) {
                     try(var signature = integrityService.generateSignature(extensionFile, keyPair)) {

server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java

@@ -66,6 +66,12 @@ public void persistResource(FileResource resource) {
         entityManager.persist(resource);
     }
 
+    @Transactional
+    public void markExtensionAsPotentiallyMalicious(ExtensionVersion extVersion) {
+        extVersion = entityManager.merge(extVersion);
+        extVersion.setPotentiallyMalicious(true);
+    }
+
     @Transactional
     @CacheEvict(value = CACHE_SITEMAP, allEntries = true)
     public void activateExtension(ExtensionVersion extVersion, ExtensionService extensions) {

server/src/main/java/org/eclipse/openvsx/scanning/BlocklistCheckService.java

@@ -97,10 +97,6 @@ public String getUserFacingMessage(List<Failure> failures) {
 
     @Override
     public Result check(Context context) {
-        if (context.extensionFile() == null) {
-            return Result.pass();
-        }
-
         var blockedFiles = checkForBlockedFiles(context.extensionFile());
         if (blockedFiles.isEmpty()) {
             return Result.pass();

server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanCompletionService.java

@@ -35,6 +35,7 @@
 import org.springframework.transaction.annotation.Transactional;
 
 import java.time.LocalDateTime;
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -457,8 +458,8 @@ private boolean completeExtensionScan(String scanId, List<ScannerJob> jobs) {
         if (!failedJobs.isEmpty()) {
             // Check if any REQUIRED scanners failed
             // Optional scanners (typically external) can fail without blocking activation
-            List<ScannerJob> requiredFailedJobs = new java.util.ArrayList<>();
-            List<ScannerJob> optionalFailedJobs = new java.util.ArrayList<>();
+            List<ScannerJob> requiredFailedJobs = new ArrayList<>();
+            List<ScannerJob> optionalFailedJobs = new ArrayList<>();
 
             for (ScannerJob failedJob : failedJobs) {
                 // Record ERROR result for audit trail (if not already recorded)

server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanService.java

@@ -22,12 +22,10 @@
 import org.jobrunr.scheduling.JobRequestScheduler;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.dao.DataIntegrityViolationException;
 import org.springframework.stereotype.Component;
 
 import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
-import java.time.LocalDateTime;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -111,7 +109,7 @@ private ExtensionScan initializeScan(
 
     /**
      * Run validation checks and record results.
-     * 
+     * <p>
      * Delegates the actual check execution to ExtensionScanner,
      * then records findings and manages state transitions.
      */

server/src/main/java/org/eclipse/openvsx/scanning/GitleaksRulesService.java

@@ -243,29 +243,29 @@ private void startRedisSubscriber() {
 
     private void subscribeLoop() {
         AtomicInteger backoffMs = new AtomicInteger(1000);
-        ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(
-            new NamedThreadFactory("gitleaks-rules-reconnect"));
 
-        while (running && !Thread.currentThread().isInterrupted()) {
-            ScheduledFuture<?> resetTask = null;
-            try {
-                resetTask = executor.schedule(() -> backoffMs.set(1000), 10, TimeUnit.SECONDS);
-                logger.debug("Subscribing to gitleaks rules update channel");
-                jedisCluster.subscribe(this, RULES_UPDATE_CHANNEL);
-            } catch (Exception e) {
-                if (!running) break;
-                logger.warn("Gitleaks rules subscriber disconnected, reconnecting in {}s: {}",
-                    backoffMs.get() / 1000, e.getMessage());
-                if (resetTask != null) resetTask.cancel(true);
+        try (var executor = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("gitleaks-rules-reconnect"))) {
+            while (running && !Thread.currentThread().isInterrupted()) {
+                ScheduledFuture<?> resetTask = null;
                 try {
-                    Thread.sleep(backoffMs.get());
-                    backoffMs.set(Math.min(backoffMs.get() * 2, 30000));
-                } catch (InterruptedException ignored) {
-                    break;
+                    resetTask = executor.schedule(() -> backoffMs.set(1000), 10, TimeUnit.SECONDS);
+                    logger.debug("Subscribing to gitleaks rules update channel");
+                    jedisCluster.subscribe(this, RULES_UPDATE_CHANNEL);
+                } catch (Exception e) {
+                    if (!running) break;
+                    logger.warn("Gitleaks rules subscriber disconnected, reconnecting in {}s: {}",
+                            backoffMs.get() / 1000, e.getMessage());
+                    if (resetTask != null) resetTask.cancel(true);
+                    try {
+                        Thread.sleep(backoffMs.get());
+                        backoffMs.set(Math.min(backoffMs.get() * 2, 30000));
+                    } catch (InterruptedException ignored) {
+                        break;
+                    }
                 }
             }
+            executor.shutdownNow();
         }
-        executor.shutdownNow();
     }
 
     @Override

server/src/main/java/org/eclipse/openvsx/scanning/HttpClientExecutor.java

@@ -36,10 +36,10 @@
 
 /**
  * Executes HTTP requests based on configuration.
- * 
+ * <p>
  * Handles different HTTP methods, body types (JSON, multipart, form-urlencoded),
  * headers, query parameters, file uploads, and authentication.
- * 
+ * <p>
  * Use static factory methods to create instances with scanner-specific configs.
  */
 public class HttpClientExecutor {

server/src/main/java/org/eclipse/openvsx/scanning/HttpTemplateEngine.java

@@ -48,7 +48,7 @@ public String process(String template, Map<String, String> placeholders) {
         }
 
         Matcher matcher = PLACEHOLDER_PATTERN.matcher(template);
-        StringBuffer result = new StringBuffer();
+        StringBuilder result = new StringBuilder();
 
         while (matcher.find()) {
             String placeholderName = matcher.group(1);

server/src/main/java/org/eclipse/openvsx/scanning/MaliciousZipCheckService.java

@@ -0,0 +1,67 @@
+/********************************************************************************
+ * Copyright (c) 2026 Contributors to the Eclipse Foundation
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * https://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ ********************************************************************************/
+package org.eclipse.openvsx.scanning;
+
+import org.eclipse.openvsx.ExtensionProcessor;
+import org.springframework.core.annotation.Order;
+import org.springframework.stereotype.Service;
+
+import java.util.List;
+
+/**
+ * Service for checking extension files for potentially malicious zip extra fields.
+ * <p>
+ * Implements PublishCheck to be auto-discovered by PublishCheckRunner.
+ * Always enabled and enforced.
+ */
+@Service
+@Order(0)
+public class MaliciousZipCheckService implements PublishCheck {
+
+    public static final String CHECK_TYPE = "MALICIOUS_ZIP_CHECK";
+    private static final String RULE_NAME = "EXTRA_FIELDS_DETECTED";
+    private static final String MESSAGE = "extension file contains zip entries with potentially harmful extra fields";
+    private static final String USER_MESSAGE = "Extension contains zip entries with unsupported extra fields";
+
+    @Override
+    public String getCheckType() {
+        return CHECK_TYPE;
+    }
+
+    @Override
+    public boolean isEnabled() {
+        return true;
+    }
+
+    @Override
+    public boolean isEnforced() {
+        return true;
+    }
+
+    @Override
+    public String getUserFacingMessage(List<Failure> failures) {
+        return USER_MESSAGE;
+    }
+
+    @Override
+    public PublishCheck.Result check(Context context) {
+        try (var processor = new ExtensionProcessor(context.extensionFile())) {
+            var potentiallyMalicious = processor.isPotentiallyMalicious();
+            if (potentiallyMalicious) {
+                return PublishCheck.Result.fail(RULE_NAME, MESSAGE);
+            }
+        }
+
+        return PublishCheck.Result.pass();
+    }
+}

server/src/main/java/org/eclipse/openvsx/scanning/ScannerFileProvider.java

@@ -83,9 +83,7 @@ public TempFile getExtensionFile(long extensionVersionId) throws ScannerExceptio
             TempFile extensionFile = storageUtil.downloadFile(download);
 
             if (extensionFile == null) {
-                throw new ScannerException(
-                    "Failed to download file for extension version: " + extensionVersionId
-                );
+                throw new ScannerException("Failed to download file for extension version: " + extensionVersionId);
             }
 
             logger.debug("Extension file ready for scanning: {} (extension version: {})",
@@ -96,15 +94,9 @@ public TempFile getExtensionFile(long extensionVersionId) throws ScannerExceptio
         } catch (ScannerException e) {
             throw e;
         } catch (IOException e) {
-            throw new ScannerException(
-                "Failed to download file for extension version " + extensionVersionId,
-                e
-            );
+            throw new ScannerException("Failed to download file for extension version " + extensionVersionId, e);
         } catch (Exception e) {
-            throw new ScannerException(
-                "Failed to retrieve file for extension version " + extensionVersionId,
-                e
-            );
+            throw new ScannerException("Failed to retrieve file for extension version " + extensionVersionId, e);
         }
     }
 }

server/src/main/java/org/eclipse/openvsx/scanning/SecretCheckService.java

@@ -23,7 +23,6 @@
 import org.springframework.stereotype.Service;
 import jakarta.validation.constraints.NotNull;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -104,10 +103,6 @@ public String getCheckType() {
 
     @Override
     public PublishCheck.Result check(PublishCheck.Context context) {
-        if (context.extensionFile() == null) {
-            return PublishCheck.Result.pass();
-        }
-
         var scanResult = scanForSecrets(context.extensionFile());
         if (!scanResult.isSecretsFound()) {
             return PublishCheck.Result.pass();
@@ -232,9 +227,6 @@ private SecretDetector.Result scanForSecrets(@NotNull TempFile extensionFile) {
         } catch (ZipException e) {
             logger.error("Failed to open extension file as zip: {}", e.getMessage());
             throw new SecretScanningException("Failed to scan extension file: invalid zip format", e);
-        } catch (IOException e) {
-            logger.error("Failed to scan extension file: {}", e.getMessage());
-            throw new SecretScanningException("Failed to scan extension file: " + e.getMessage(), e);
         } catch (SecretScanningException e) {
             throw e;
         } catch (Exception e) {

server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java

@@ -2627,8 +2627,7 @@ PublishExtensionVersionHandler publishExtensionVersionHandler(
                 UserService users,
                 ExtensionValidator validator,
                 ExtensionControlService extensionControl,
-                ExtensionScanService extensionScanService,
-                ExtensionScanPersistenceService scanPersistenceService
+                ExtensionScanService extensionScanService
         ) {
             return new PublishExtensionVersionHandler(
                     service,
@@ -2639,8 +2638,7 @@ PublishExtensionVersionHandler publishExtensionVersionHandler(
                     users,
                     validator,
                     extensionControl,
-                    extensionScanService,
-                    scanPersistenceService
+                    extensionScanService
             );
         }