From 6814ca34af03f762b7bde9d16a63d2b4a78017f4 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Thu, 26 Mar 2026 12:00:17 +0100 Subject: [PATCH 1/3] change: make malicious zip file checks a synchronous PublishCheck if scanning is enabled and ensure that the flag potentially_malicious is correctly persisted otherwise --- server/src/dev/resources/application.yml | 2 +- .../org/eclipse/openvsx/ExtensionService.java | 2 +- .../PublishExtensionVersionHandler.java | 53 +++------------- .../PublishExtensionVersionService.java | 6 ++ .../scanning/ExtensionScanService.java | 7 +-- .../scanning/MaliciousZipCheckService.java | 63 +++++++++++++++++++ .../openvsx/scanning/PublishCheck.java | 2 + .../openvsx/scanning/PublishCheckRunner.java | 4 +- .../org/eclipse/openvsx/RegistryAPITest.java | 6 +- .../PublishExtensionVersionHandlerTest.java | 8 +-- .../scanning/BlocklistCheckServiceTest.java | 3 +- .../ExtensionScanServiceEnforcementTest.java | 49 ++++++++------- .../scanning/PublishCheckRunnerTest.java | 19 +++--- .../search/SimilarityCheckServiceTest.java | 6 +- 14 files changed, 133 insertions(+), 97 deletions(-) create mode 100644 server/src/main/java/org/eclipse/openvsx/scanning/MaliciousZipCheckService.java diff --git a/server/src/dev/resources/application.yml b/server/src/dev/resources/application.yml index 1108173c3..530003da2 100644 --- a/server/src/dev/resources/application.yml +++ b/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 diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java index 641ef0b43..e09fda575 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java @@ -126,7 +126,7 @@ private ExtensionVersion publishVersionWithScan(InputStream content, PersonalAcc try (var processor = new ExtensionProcessor(extensionFile)) { scan = scanService.initializeScan(processor, token.getUser()); - scanService.runValidation(scan, extensionFile, token.getUser()); + scanService.runValidation(scan, processor, extensionFile, token.getUser()); doPublish(extensionFile, null, token, TimeUtil.getCurrentUTC(), true); diff --git a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java index ed2a2280f..484cd37fd 100644 --- a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java +++ b/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)) { diff --git a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java b/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java index ce74beb9c..7f062c061 100644 --- a/server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java +++ b/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) { diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanService.java b/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanService.java index dc207ef6f..9b0d7d70d 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanService.java +++ b/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,18 +109,19 @@ private ExtensionScan initializeScan( /** * Run validation checks and record results. - * + *

* Delegates the actual check execution to ExtensionScanner, * then records findings and manages state transitions. */ public void runValidation( @Nonnull ExtensionScan scan, + @Nonnull ExtensionProcessor processor, @Nonnull TempFile extensionFile, @Nonnull UserData user ) { transitionTo(scan, ScanStatus.VALIDATING); - var checkResult = checkRunner.runChecks(scan, extensionFile, user); + var checkResult = checkRunner.runChecks(scan, processor, extensionFile, user); // Record ALL check executions for audit trail (pass, fail, skip, error). // This gives admins visibility into what checks were run. diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/MaliciousZipCheckService.java b/server/src/main/java/org/eclipse/openvsx/scanning/MaliciousZipCheckService.java new file mode 100644 index 000000000..f996d94a7 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/scanning/MaliciousZipCheckService.java @@ -0,0 +1,63 @@ +/******************************************************************************** + * 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.springframework.core.annotation.Order; +import org.springframework.stereotype.Service; + +import java.util.List; + +/** + * Service for checking extension files for potentially malicious zip extra fields. + *

+ * Implements PublishCheck to be auto-discovered by PublishCheckRunner. + * Always enabled and enforced. + */ +@Service +@Order(4) +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 = "VSIX extension file contains zip entries with potentially harmful 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 failures) { + return MESSAGE; + } + + @Override + public PublishCheck.Result check(Context context) { + var potentiallyMalicious = context.processor().isPotentiallyMalicious(); + if (potentiallyMalicious) { + return PublishCheck.Result.fail(RULE_NAME, MESSAGE); + } else { + return PublishCheck.Result.pass(); + } + } +} diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheck.java b/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheck.java index 7907cd1d3..04ecad9f0 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheck.java +++ b/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheck.java @@ -12,6 +12,7 @@ ********************************************************************************/ package org.eclipse.openvsx.scanning; +import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.ExtensionScan; import org.eclipse.openvsx.entities.UserData; import org.eclipse.openvsx.util.TempFile; @@ -97,6 +98,7 @@ default String getUserFacingMessage(List failures) { */ record Context( @Nonnull ExtensionScan scan, + @Nonnull ExtensionProcessor processor, @Nonnull TempFile extensionFile, @Nonnull UserData user ) {} diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheckRunner.java b/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheckRunner.java index 74796f2e0..140ec1647 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheckRunner.java +++ b/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheckRunner.java @@ -12,6 +12,7 @@ ********************************************************************************/ package org.eclipse.openvsx.scanning; +import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.ExtensionScan; import org.eclipse.openvsx.entities.ScanCheckResult; import org.eclipse.openvsx.entities.UserData; @@ -70,10 +71,11 @@ public int getCheckCount() { @Nonnull public Result runChecks( @Nonnull ExtensionScan scan, + @Nonnull ExtensionProcessor processor, @Nonnull TempFile extensionFile, @Nonnull UserData user ) { - var context = new PublishCheck.Context(scan, extensionFile, user); + var context = new PublishCheck.Context(scan, processor, extensionFile, user); var allFindings = new ArrayList(); var checkExecutions = new ArrayList(); var allErrors = new ArrayList(); diff --git a/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java b/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java index 40bf514e8..61f6b5574 100644 --- a/server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java +++ b/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 ); } diff --git a/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandlerTest.java b/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandlerTest.java index fffc8900b..b1fe8341c 100644 --- a/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandlerTest.java +++ b/server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandlerTest.java @@ -22,10 +22,8 @@ import org.eclipse.openvsx.entities.PersonalAccessToken; 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.ErrorResultException; -import org.eclipse.openvsx.util.TempFile; import org.jobrunr.scheduling.JobRequestScheduler; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -74,9 +72,6 @@ class PublishExtensionVersionHandlerTest { @Mock ExtensionScanService scanService; - @Mock - ExtensionScanPersistenceService scanPersistenceService; - private PublishExtensionVersionHandler handler; @BeforeEach @@ -90,8 +85,7 @@ void setUp() throws Exception { users, validator, extensionControl, - scanService, - scanPersistenceService + scanService ); // Lenient: not all tests need this mock diff --git a/server/src/test/java/org/eclipse/openvsx/scanning/BlocklistCheckServiceTest.java b/server/src/test/java/org/eclipse/openvsx/scanning/BlocklistCheckServiceTest.java index a40c1391c..be6b6bdc6 100644 --- a/server/src/test/java/org/eclipse/openvsx/scanning/BlocklistCheckServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/scanning/BlocklistCheckServiceTest.java @@ -13,6 +13,7 @@ package org.eclipse.openvsx.scanning; import org.apache.commons.codec.digest.DigestUtils; +import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.ExtensionScan; import org.eclipse.openvsx.entities.FileDecision; import org.eclipse.openvsx.entities.UserData; @@ -293,7 +294,7 @@ private PublishCheck.Context createContext(TempFile extensionFile) { UserData user = new UserData(); user.setLoginName("testuser"); - return new PublishCheck.Context(scan, extensionFile, user); + return new PublishCheck.Context(scan, new ExtensionProcessor(extensionFile), extensionFile, user); } private FileDecision createBlockedDecision(String fileHash, String fileName) { diff --git a/server/src/test/java/org/eclipse/openvsx/scanning/ExtensionScanServiceEnforcementTest.java b/server/src/test/java/org/eclipse/openvsx/scanning/ExtensionScanServiceEnforcementTest.java index e86f308a0..e40873c92 100644 --- a/server/src/test/java/org/eclipse/openvsx/scanning/ExtensionScanServiceEnforcementTest.java +++ b/server/src/test/java/org/eclipse/openvsx/scanning/ExtensionScanServiceEnforcementTest.java @@ -12,6 +12,7 @@ ********************************************************************************/ package org.eclipse.openvsx.scanning; +import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.*; import org.eclipse.openvsx.repositories.ScannerJobRepository; import org.eclipse.openvsx.util.ErrorResultException; @@ -49,6 +50,8 @@ class ExtensionScanServiceEnforcementTest { @Mock ScannerRegistry scannerRegistry; @Mock JobRequestScheduler jobScheduler; @Mock ScannerJobRepository scanJobRepository; + @Mock + ExtensionProcessor extensionProcessor; @Mock TempFile extensionFile; private ExtensionScanService svc; @@ -120,7 +123,7 @@ private PublishCheckRunner.CheckExecution checkExecution(String checkType, ScanC @Test void runValidation_passes_whenAllChecksPass() { // Scanner returns no findings - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( List.of(), List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -130,7 +133,7 @@ void runValidation_passes_whenAllChecksPass() { )); // Act - should not throw - svc.runValidation(scan, extensionFile, user); + svc.runValidation(scan, extensionProcessor, extensionFile, user); // Assert: no failures recorded, validation lifecycle methods called verify(persistenceService).updateStatus(scan, ScanStatus.VALIDATING); @@ -140,7 +143,7 @@ void runValidation_passes_whenAllChecksPass() { @Test void runValidation_delegatesToCheckRunner() { // Scanner returns pass - when(checkRunner.runChecks(eq(scan), eq(extensionFile), eq(user))) + when(checkRunner.runChecks(eq(scan), eq(extensionProcessor), eq(extensionFile), eq(user))) .thenReturn(new PublishCheckRunner.Result( List.of(), List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -150,10 +153,10 @@ void runValidation_delegatesToCheckRunner() { )); // Act - svc.runValidation(scan, extensionFile, user); + svc.runValidation(scan, extensionProcessor, extensionFile, user); // Assert: scanner was called with correct arguments - verify(checkRunner).runChecks(scan, extensionFile, user); + verify(checkRunner).runChecks(scan, extensionProcessor, extensionFile, user); } // ========== ENFORCEMENT TESTS ========== @@ -165,7 +168,7 @@ void runValidation_doesNotThrow_whenFailuresNotEnforced_butPersistsFailures() { new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", false, null), new PublishCheckRunner.Finding("CHECK_2", "rule2", "reason2", false, null) ); - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -175,7 +178,7 @@ void runValidation_doesNotThrow_whenFailuresNotEnforced_butPersistsFailures() { )); // Act: should NOT throw because nothing is enforced - svc.runValidation(scan, extensionFile, user); + svc.runValidation(scan, extensionProcessor, extensionFile, user); // Assert: failures persisted even in monitor-only mode verify(persistenceService).recordValidationFailure(eq(scan), eq("CHECK_1"), eq("rule1"), eq("reason1"), eq(false)); @@ -188,7 +191,7 @@ void runValidation_throwsWhenEnforcedCheckFails() { var findings = List.of( new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", true, null) ); - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.REJECT)), @@ -198,7 +201,7 @@ void runValidation_throwsWhenEnforcedCheckFails() { )); // Act & Assert: should throw ErrorResultException - assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) .isInstanceOf(ErrorResultException.class); verify(persistenceService).recordValidationFailure(eq(scan), eq("CHECK_1"), any(), any(), eq(true)); @@ -211,7 +214,7 @@ void runValidation_throwsWhenBothEnforced_andBothFail() { new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", true, null), new PublishCheckRunner.Finding("CHECK_2", "rule2", "reason2", true, null) ); - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.REJECT)), @@ -221,7 +224,7 @@ void runValidation_throwsWhenBothEnforced_andBothFail() { )); // Act & Assert - assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) .isInstanceOf(ErrorResultException.class); verify(persistenceService).recordValidationFailure(eq(scan), eq("CHECK_1"), any(), any(), eq(true)); @@ -235,7 +238,7 @@ void runValidation_throwsWhenOneEnforcedFails_andOneNotEnforcedFails() { new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", true, null), new PublishCheckRunner.Finding("CHECK_2", "rule2", "reason2", false, null) ); - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.REJECT)), @@ -245,7 +248,7 @@ void runValidation_throwsWhenOneEnforcedFails_andOneNotEnforcedFails() { )); // Act & Assert: blocked because at least one enforced check failed - assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) .isInstanceOf(ErrorResultException.class); verify(persistenceService).recordValidationFailure(eq(scan), eq("CHECK_1"), any(), any(), eq(true)); @@ -257,7 +260,7 @@ void runValidation_throwsWhenOneEnforcedFails_andOneNotEnforcedFails() { @Test void runValidation_marksErrorAndRethrows_whenScannerReturnsError() { var exception = new RuntimeException("Check failed unexpectedly"); - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( List.of(), List.of(new PublishCheckRunner.CheckExecution( @@ -276,7 +279,7 @@ void runValidation_marksErrorAndRethrows_whenScannerReturnsError() { )); // Act & Assert - assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) .isInstanceOf(ErrorResultException.class) .hasMessageContaining("Check failed unexpectedly"); @@ -287,7 +290,7 @@ void runValidation_marksErrorAndRethrows_whenScannerReturnsError() { @Test void runValidation_transitionsToValidating() { - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( List.of(), List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -297,7 +300,7 @@ void runValidation_transitionsToValidating() { )); // Act - svc.runValidation(scan, extensionFile, user); + svc.runValidation(scan, extensionProcessor, extensionFile, user); // Assert: transitions to VALIDATING verify(persistenceService).updateStatus(scan, ScanStatus.VALIDATING); @@ -310,7 +313,7 @@ void runValidation_completesNormally_whenFailuresNotEnforced() { new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", false, null), new PublishCheckRunner.Finding("CHECK_2", "rule2", "reason2", false, null) ); - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -320,7 +323,7 @@ void runValidation_completesNormally_whenFailuresNotEnforced() { )); // Act - should not throw - svc.runValidation(scan, extensionFile, user); + svc.runValidation(scan, extensionProcessor, extensionFile, user); // Assert: completes normally (no exception) verify(persistenceService).updateStatus(scan, ScanStatus.VALIDATING); @@ -331,7 +334,7 @@ void runValidation_transitionsToRejected_whenEnforcedFailure() { var findings = List.of( new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", true, null) ); - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.REJECT)), @@ -341,7 +344,7 @@ void runValidation_transitionsToRejected_whenEnforcedFailure() { )); // Act & Assert: throws and transitions to REJECTED - assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) .isInstanceOf(ErrorResultException.class); verify(persistenceService).completeWithStatus(scan, ScanStatus.REJECTED); @@ -357,7 +360,7 @@ void runValidation_recordsAllFindings() { new PublishCheckRunner.Finding("CHECK_1", "rule2", "reason2", false, null), new PublishCheckRunner.Finding("CHECK_1", "rule3", "reason3", false, null) ); - when(checkRunner.runChecks(any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -367,7 +370,7 @@ void runValidation_recordsAllFindings() { )); // Act - svc.runValidation(scan, extensionFile, user); + svc.runValidation(scan, extensionProcessor, extensionFile, user); // Assert: all 3 findings recorded verify(persistenceService, times(3)).recordValidationFailure(eq(scan), eq("CHECK_1"), any(), any(), eq(false)); diff --git a/server/src/test/java/org/eclipse/openvsx/scanning/PublishCheckRunnerTest.java b/server/src/test/java/org/eclipse/openvsx/scanning/PublishCheckRunnerTest.java index d53e8c09a..cf70ac516 100644 --- a/server/src/test/java/org/eclipse/openvsx/scanning/PublishCheckRunnerTest.java +++ b/server/src/test/java/org/eclipse/openvsx/scanning/PublishCheckRunnerTest.java @@ -12,6 +12,7 @@ ********************************************************************************/ package org.eclipse.openvsx.scanning; +import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.ExtensionScan; import org.eclipse.openvsx.entities.ScanCheckResult; import org.eclipse.openvsx.entities.UserData; @@ -36,6 +37,8 @@ class PublishCheckRunnerTest { @Mock ExtensionScan scan; + @Mock + ExtensionProcessor processor; @Mock TempFile extensionFile; @Mock UserData user; @@ -45,7 +48,7 @@ void runChecks_passesWhenAllChecksPass() { var check2 = mockCheck("CHECK2", true, true, PublishCheck.Result.pass()); var runner = new PublishCheckRunner(List.of(check1, check2)); - var result = runner.runChecks(scan, extensionFile, user); + var result = runner.runChecks(scan, processor, extensionFile, user); assertTrue(result.passed()); assertFalse(result.hasEnforcedFailure()); @@ -61,7 +64,7 @@ void runChecks_failsWhenEnforcedCheckFails() { var failingCheck = mockCheck("FAIL", true, true, PublishCheck.Result.fail(List.of(failure))); var runner = new PublishCheckRunner(List.of(passingCheck, failingCheck)); - var result = runner.runChecks(scan, extensionFile, user); + var result = runner.runChecks(scan, processor, extensionFile, user); assertFalse(result.passed()); assertTrue(result.hasEnforcedFailure()); @@ -75,7 +78,7 @@ void runChecks_passesWhenNonEnforcedCheckFails() { var failingCheck = mockCheck("CHECK", true, false, PublishCheck.Result.fail(List.of(failure))); var runner = new PublishCheckRunner(List.of(failingCheck)); - var result = runner.runChecks(scan, extensionFile, user); + var result = runner.runChecks(scan, processor, extensionFile, user); assertTrue(result.passed()); assertFalse(result.hasEnforcedFailure()); @@ -89,7 +92,7 @@ void runChecks_skipsDisabledChecks() { var enabledCheck = mockCheck("ENABLED", true, true, PublishCheck.Result.pass()); var runner = new PublishCheckRunner(List.of(disabledCheck, enabledCheck)); - var result = runner.runChecks(scan, extensionFile, user); + var result = runner.runChecks(scan, processor, extensionFile, user); assertTrue(result.passed()); // Only enabled check should have an execution record @@ -106,11 +109,11 @@ void runChecks_capturesCheckError() { when(errorCheck.check(any())).thenThrow(new RuntimeException("Check failed")); var runner = new PublishCheckRunner(List.of(errorCheck)); - var result = runner.runChecks(scan, extensionFile, user); + var result = runner.runChecks(scan, processor, extensionFile, user); assertFalse(result.passed()); assertTrue(result.hasError()); - assertEquals("ERROR_CHECK", result.getRequiredErrors().get(0).checkType()); + assertEquals("ERROR_CHECK", result.getRequiredErrors().getFirst().checkType()); assertNotNull(result.getErrorMessage()); } @@ -128,7 +131,7 @@ void runChecks_stopsAfterError() { var runner = new PublishCheckRunner(List.of(errorCheck, afterErrorCheck)); - var result = runner.runChecks(scan, extensionFile, user); + var result = runner.runChecks(scan, processor, extensionFile, user); // Should have only one execution (the erroring check) assertEquals(1, result.checkExecutions().size()); @@ -182,7 +185,7 @@ void checkExecution_recordsCorrectResult() { var failingCheck = mockCheck("TEST", true, true, PublishCheck.Result.fail(List.of(failure))); var runner = new PublishCheckRunner(List.of(failingCheck)); - var result = runner.runChecks(scan, extensionFile, user); + var result = runner.runChecks(scan, processor, extensionFile, user); var execution = result.checkExecutions().getFirst(); assertEquals("TEST", execution.checkType()); diff --git a/server/src/test/java/org/eclipse/openvsx/search/SimilarityCheckServiceTest.java b/server/src/test/java/org/eclipse/openvsx/search/SimilarityCheckServiceTest.java index 863c2c207..f42ef217c 100644 --- a/server/src/test/java/org/eclipse/openvsx/search/SimilarityCheckServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/search/SimilarityCheckServiceTest.java @@ -12,6 +12,7 @@ ********************************************************************************/ package org.eclipse.openvsx.search; +import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.*; import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.scanning.PublishCheck; @@ -47,6 +48,9 @@ class SimilarityCheckServiceTest { @InjectMocks SimilarityCheckService similarityCheckService; + @Mock + ExtensionProcessor processor; + @Mock TempFile extensionFile; @@ -64,7 +68,7 @@ private PublishCheck.Context createContext(String namespaceName, String extensio scan.setNamespaceName(namespaceName); scan.setExtensionName(extensionName); scan.setExtensionDisplayName(displayName); - return new PublishCheck.Context(scan, extensionFile, user); + return new PublishCheck.Context(scan, processor, extensionFile, user); } @Test From b1102d234ab144739d009577e48908cfc0961d75 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Fri, 27 Mar 2026 13:16:22 +0100 Subject: [PATCH 2/3] rework PR to not pass an existing ExtensionProcessor for PublishChecks but rather create it on demand as its cheap, also add a unit test --- .../eclipse/openvsx/ExtensionProcessor.java | 1 + .../org/eclipse/openvsx/ExtensionService.java | 2 +- .../scanning/BlocklistCheckService.java | 4 - .../ExtensionScanCompletionService.java | 5 +- .../scanning/ExtensionScanService.java | 3 +- .../scanning/GitleaksRulesService.java | 36 +++--- .../openvsx/scanning/HttpClientExecutor.java | 4 +- .../openvsx/scanning/HttpTemplateEngine.java | 2 +- .../scanning/MaliciousZipCheckService.java | 20 ++-- .../openvsx/scanning/PublishCheck.java | 2 - .../openvsx/scanning/PublishCheckRunner.java | 4 +- .../openvsx/scanning/ScannerFileProvider.java | 14 +-- .../openvsx/scanning/SecretCheckService.java | 8 -- .../scanning/BlocklistCheckServiceTest.java | 3 +- .../ExtensionScanServiceEnforcementTest.java | 49 ++++---- .../MaliciousZipCheckServiceTest.java | 112 ++++++++++++++++++ .../scanning/PublishCheckRunnerTest.java | 17 ++- .../scanning/ScannerInvocationTest.java | 2 +- .../openvsx/scanning/ScannerRegistryTest.java | 6 +- .../search/SimilarityCheckServiceTest.java | 6 +- 20 files changed, 190 insertions(+), 110 deletions(-) create mode 100644 server/src/test/java/org/eclipse/openvsx/scanning/MaliciousZipCheckServiceTest.java diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java b/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java index 35bfb52c8..cab84e994 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java @@ -80,6 +80,7 @@ private void readInputStream() { try { zipFile = new ZipFile(extensionFile.getPath().toFile()); } catch (ZipException exc) { + exc.printStackTrace(); throw new ErrorResultException("Could not read zip file: " + exc.getMessage()); } catch (IOException exc) { throw new ErrorResultException("Could not read from input stream: " + exc.getMessage()); diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java index e09fda575..641ef0b43 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionService.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionService.java @@ -126,7 +126,7 @@ private ExtensionVersion publishVersionWithScan(InputStream content, PersonalAcc try (var processor = new ExtensionProcessor(extensionFile)) { scan = scanService.initializeScan(processor, token.getUser()); - scanService.runValidation(scan, processor, extensionFile, token.getUser()); + scanService.runValidation(scan, extensionFile, token.getUser()); doPublish(extensionFile, null, token, TimeUtil.getCurrentUTC(), true); diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/BlocklistCheckService.java b/server/src/main/java/org/eclipse/openvsx/scanning/BlocklistCheckService.java index 08089d6dd..44c17da23 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/BlocklistCheckService.java +++ b/server/src/main/java/org/eclipse/openvsx/scanning/BlocklistCheckService.java @@ -97,10 +97,6 @@ public String getUserFacingMessage(List 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(); diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanCompletionService.java b/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanCompletionService.java index bf53a4bb0..cb7fd4989 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanCompletionService.java +++ b/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 jobs) { if (!failedJobs.isEmpty()) { // Check if any REQUIRED scanners failed // Optional scanners (typically external) can fail without blocking activation - List requiredFailedJobs = new java.util.ArrayList<>(); - List optionalFailedJobs = new java.util.ArrayList<>(); + List requiredFailedJobs = new ArrayList<>(); + List optionalFailedJobs = new ArrayList<>(); for (ScannerJob failedJob : failedJobs) { // Record ERROR result for audit trail (if not already recorded) diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanService.java b/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanService.java index 9b0d7d70d..5f6513078 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanService.java +++ b/server/src/main/java/org/eclipse/openvsx/scanning/ExtensionScanService.java @@ -115,13 +115,12 @@ private ExtensionScan initializeScan( */ public void runValidation( @Nonnull ExtensionScan scan, - @Nonnull ExtensionProcessor processor, @Nonnull TempFile extensionFile, @Nonnull UserData user ) { transitionTo(scan, ScanStatus.VALIDATING); - var checkResult = checkRunner.runChecks(scan, processor, extensionFile, user); + var checkResult = checkRunner.runChecks(scan, extensionFile, user); // Record ALL check executions for audit trail (pass, fail, skip, error). // This gives admins visibility into what checks were run. diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/GitleaksRulesService.java b/server/src/main/java/org/eclipse/openvsx/scanning/GitleaksRulesService.java index 1c6bbc13b..9c3eee43f 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/GitleaksRulesService.java +++ b/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 diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/HttpClientExecutor.java b/server/src/main/java/org/eclipse/openvsx/scanning/HttpClientExecutor.java index d93ecdb4d..b37d2a4a0 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/HttpClientExecutor.java +++ b/server/src/main/java/org/eclipse/openvsx/scanning/HttpClientExecutor.java @@ -36,10 +36,10 @@ /** * Executes HTTP requests based on configuration. - * + *

* Handles different HTTP methods, body types (JSON, multipart, form-urlencoded), * headers, query parameters, file uploads, and authentication. - * + *

* Use static factory methods to create instances with scanner-specific configs. */ public class HttpClientExecutor { diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/HttpTemplateEngine.java b/server/src/main/java/org/eclipse/openvsx/scanning/HttpTemplateEngine.java index 74bb78c76..9c9987c05 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/HttpTemplateEngine.java +++ b/server/src/main/java/org/eclipse/openvsx/scanning/HttpTemplateEngine.java @@ -48,7 +48,7 @@ public String process(String template, Map placeholders) { } Matcher matcher = PLACEHOLDER_PATTERN.matcher(template); - StringBuffer result = new StringBuffer(); + StringBuilder result = new StringBuilder(); while (matcher.find()) { String placeholderName = matcher.group(1); diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/MaliciousZipCheckService.java b/server/src/main/java/org/eclipse/openvsx/scanning/MaliciousZipCheckService.java index f996d94a7..24b69fe7f 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/MaliciousZipCheckService.java +++ b/server/src/main/java/org/eclipse/openvsx/scanning/MaliciousZipCheckService.java @@ -12,6 +12,7 @@ ********************************************************************************/ package org.eclipse.openvsx.scanning; +import org.eclipse.openvsx.ExtensionProcessor; import org.springframework.core.annotation.Order; import org.springframework.stereotype.Service; @@ -24,12 +25,13 @@ * Always enabled and enforced. */ @Service -@Order(4) +@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 = "VSIX extension file contains zip entries with potentially harmful extra fields"; + 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() { @@ -48,16 +50,18 @@ public boolean isEnforced() { @Override public String getUserFacingMessage(List failures) { - return MESSAGE; + return USER_MESSAGE; } @Override public PublishCheck.Result check(Context context) { - var potentiallyMalicious = context.processor().isPotentiallyMalicious(); - if (potentiallyMalicious) { - return PublishCheck.Result.fail(RULE_NAME, MESSAGE); - } else { - return PublishCheck.Result.pass(); + try (var processor = new ExtensionProcessor(context.extensionFile())) { + var potentiallyMalicious = processor.isPotentiallyMalicious(); + if (potentiallyMalicious) { + return PublishCheck.Result.fail(RULE_NAME, MESSAGE); + } } + + return PublishCheck.Result.pass(); } } diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheck.java b/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheck.java index 04ecad9f0..7907cd1d3 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheck.java +++ b/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheck.java @@ -12,7 +12,6 @@ ********************************************************************************/ package org.eclipse.openvsx.scanning; -import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.ExtensionScan; import org.eclipse.openvsx.entities.UserData; import org.eclipse.openvsx.util.TempFile; @@ -98,7 +97,6 @@ default String getUserFacingMessage(List failures) { */ record Context( @Nonnull ExtensionScan scan, - @Nonnull ExtensionProcessor processor, @Nonnull TempFile extensionFile, @Nonnull UserData user ) {} diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheckRunner.java b/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheckRunner.java index 140ec1647..74796f2e0 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheckRunner.java +++ b/server/src/main/java/org/eclipse/openvsx/scanning/PublishCheckRunner.java @@ -12,7 +12,6 @@ ********************************************************************************/ package org.eclipse.openvsx.scanning; -import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.ExtensionScan; import org.eclipse.openvsx.entities.ScanCheckResult; import org.eclipse.openvsx.entities.UserData; @@ -71,11 +70,10 @@ public int getCheckCount() { @Nonnull public Result runChecks( @Nonnull ExtensionScan scan, - @Nonnull ExtensionProcessor processor, @Nonnull TempFile extensionFile, @Nonnull UserData user ) { - var context = new PublishCheck.Context(scan, processor, extensionFile, user); + var context = new PublishCheck.Context(scan, extensionFile, user); var allFindings = new ArrayList(); var checkExecutions = new ArrayList(); var allErrors = new ArrayList(); diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/ScannerFileProvider.java b/server/src/main/java/org/eclipse/openvsx/scanning/ScannerFileProvider.java index ea1a67db9..15851a9d5 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/ScannerFileProvider.java +++ b/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); } } } diff --git a/server/src/main/java/org/eclipse/openvsx/scanning/SecretCheckService.java b/server/src/main/java/org/eclipse/openvsx/scanning/SecretCheckService.java index fa3deab86..5a69c48d0 100644 --- a/server/src/main/java/org/eclipse/openvsx/scanning/SecretCheckService.java +++ b/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) { diff --git a/server/src/test/java/org/eclipse/openvsx/scanning/BlocklistCheckServiceTest.java b/server/src/test/java/org/eclipse/openvsx/scanning/BlocklistCheckServiceTest.java index be6b6bdc6..a40c1391c 100644 --- a/server/src/test/java/org/eclipse/openvsx/scanning/BlocklistCheckServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/scanning/BlocklistCheckServiceTest.java @@ -13,7 +13,6 @@ package org.eclipse.openvsx.scanning; import org.apache.commons.codec.digest.DigestUtils; -import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.ExtensionScan; import org.eclipse.openvsx.entities.FileDecision; import org.eclipse.openvsx.entities.UserData; @@ -294,7 +293,7 @@ private PublishCheck.Context createContext(TempFile extensionFile) { UserData user = new UserData(); user.setLoginName("testuser"); - return new PublishCheck.Context(scan, new ExtensionProcessor(extensionFile), extensionFile, user); + return new PublishCheck.Context(scan, extensionFile, user); } private FileDecision createBlockedDecision(String fileHash, String fileName) { diff --git a/server/src/test/java/org/eclipse/openvsx/scanning/ExtensionScanServiceEnforcementTest.java b/server/src/test/java/org/eclipse/openvsx/scanning/ExtensionScanServiceEnforcementTest.java index e40873c92..e86f308a0 100644 --- a/server/src/test/java/org/eclipse/openvsx/scanning/ExtensionScanServiceEnforcementTest.java +++ b/server/src/test/java/org/eclipse/openvsx/scanning/ExtensionScanServiceEnforcementTest.java @@ -12,7 +12,6 @@ ********************************************************************************/ package org.eclipse.openvsx.scanning; -import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.*; import org.eclipse.openvsx.repositories.ScannerJobRepository; import org.eclipse.openvsx.util.ErrorResultException; @@ -50,8 +49,6 @@ class ExtensionScanServiceEnforcementTest { @Mock ScannerRegistry scannerRegistry; @Mock JobRequestScheduler jobScheduler; @Mock ScannerJobRepository scanJobRepository; - @Mock - ExtensionProcessor extensionProcessor; @Mock TempFile extensionFile; private ExtensionScanService svc; @@ -123,7 +120,7 @@ private PublishCheckRunner.CheckExecution checkExecution(String checkType, ScanC @Test void runValidation_passes_whenAllChecksPass() { // Scanner returns no findings - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( List.of(), List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -133,7 +130,7 @@ void runValidation_passes_whenAllChecksPass() { )); // Act - should not throw - svc.runValidation(scan, extensionProcessor, extensionFile, user); + svc.runValidation(scan, extensionFile, user); // Assert: no failures recorded, validation lifecycle methods called verify(persistenceService).updateStatus(scan, ScanStatus.VALIDATING); @@ -143,7 +140,7 @@ void runValidation_passes_whenAllChecksPass() { @Test void runValidation_delegatesToCheckRunner() { // Scanner returns pass - when(checkRunner.runChecks(eq(scan), eq(extensionProcessor), eq(extensionFile), eq(user))) + when(checkRunner.runChecks(eq(scan), eq(extensionFile), eq(user))) .thenReturn(new PublishCheckRunner.Result( List.of(), List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -153,10 +150,10 @@ void runValidation_delegatesToCheckRunner() { )); // Act - svc.runValidation(scan, extensionProcessor, extensionFile, user); + svc.runValidation(scan, extensionFile, user); // Assert: scanner was called with correct arguments - verify(checkRunner).runChecks(scan, extensionProcessor, extensionFile, user); + verify(checkRunner).runChecks(scan, extensionFile, user); } // ========== ENFORCEMENT TESTS ========== @@ -168,7 +165,7 @@ void runValidation_doesNotThrow_whenFailuresNotEnforced_butPersistsFailures() { new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", false, null), new PublishCheckRunner.Finding("CHECK_2", "rule2", "reason2", false, null) ); - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -178,7 +175,7 @@ void runValidation_doesNotThrow_whenFailuresNotEnforced_butPersistsFailures() { )); // Act: should NOT throw because nothing is enforced - svc.runValidation(scan, extensionProcessor, extensionFile, user); + svc.runValidation(scan, extensionFile, user); // Assert: failures persisted even in monitor-only mode verify(persistenceService).recordValidationFailure(eq(scan), eq("CHECK_1"), eq("rule1"), eq("reason1"), eq(false)); @@ -191,7 +188,7 @@ void runValidation_throwsWhenEnforcedCheckFails() { var findings = List.of( new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", true, null) ); - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.REJECT)), @@ -201,7 +198,7 @@ void runValidation_throwsWhenEnforcedCheckFails() { )); // Act & Assert: should throw ErrorResultException - assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) .isInstanceOf(ErrorResultException.class); verify(persistenceService).recordValidationFailure(eq(scan), eq("CHECK_1"), any(), any(), eq(true)); @@ -214,7 +211,7 @@ void runValidation_throwsWhenBothEnforced_andBothFail() { new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", true, null), new PublishCheckRunner.Finding("CHECK_2", "rule2", "reason2", true, null) ); - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.REJECT)), @@ -224,7 +221,7 @@ void runValidation_throwsWhenBothEnforced_andBothFail() { )); // Act & Assert - assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) .isInstanceOf(ErrorResultException.class); verify(persistenceService).recordValidationFailure(eq(scan), eq("CHECK_1"), any(), any(), eq(true)); @@ -238,7 +235,7 @@ void runValidation_throwsWhenOneEnforcedFails_andOneNotEnforcedFails() { new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", true, null), new PublishCheckRunner.Finding("CHECK_2", "rule2", "reason2", false, null) ); - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.REJECT)), @@ -248,7 +245,7 @@ void runValidation_throwsWhenOneEnforcedFails_andOneNotEnforcedFails() { )); // Act & Assert: blocked because at least one enforced check failed - assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) .isInstanceOf(ErrorResultException.class); verify(persistenceService).recordValidationFailure(eq(scan), eq("CHECK_1"), any(), any(), eq(true)); @@ -260,7 +257,7 @@ void runValidation_throwsWhenOneEnforcedFails_andOneNotEnforcedFails() { @Test void runValidation_marksErrorAndRethrows_whenScannerReturnsError() { var exception = new RuntimeException("Check failed unexpectedly"); - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( List.of(), List.of(new PublishCheckRunner.CheckExecution( @@ -279,7 +276,7 @@ void runValidation_marksErrorAndRethrows_whenScannerReturnsError() { )); // Act & Assert - assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) .isInstanceOf(ErrorResultException.class) .hasMessageContaining("Check failed unexpectedly"); @@ -290,7 +287,7 @@ void runValidation_marksErrorAndRethrows_whenScannerReturnsError() { @Test void runValidation_transitionsToValidating() { - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( List.of(), List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -300,7 +297,7 @@ void runValidation_transitionsToValidating() { )); // Act - svc.runValidation(scan, extensionProcessor, extensionFile, user); + svc.runValidation(scan, extensionFile, user); // Assert: transitions to VALIDATING verify(persistenceService).updateStatus(scan, ScanStatus.VALIDATING); @@ -313,7 +310,7 @@ void runValidation_completesNormally_whenFailuresNotEnforced() { new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", false, null), new PublishCheckRunner.Finding("CHECK_2", "rule2", "reason2", false, null) ); - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -323,7 +320,7 @@ void runValidation_completesNormally_whenFailuresNotEnforced() { )); // Act - should not throw - svc.runValidation(scan, extensionProcessor, extensionFile, user); + svc.runValidation(scan, extensionFile, user); // Assert: completes normally (no exception) verify(persistenceService).updateStatus(scan, ScanStatus.VALIDATING); @@ -334,7 +331,7 @@ void runValidation_transitionsToRejected_whenEnforcedFailure() { var findings = List.of( new PublishCheckRunner.Finding("CHECK_1", "rule1", "reason1", true, null) ); - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.REJECT)), @@ -344,7 +341,7 @@ void runValidation_transitionsToRejected_whenEnforcedFailure() { )); // Act & Assert: throws and transitions to REJECTED - assertThatThrownBy(() -> svc.runValidation(scan, extensionProcessor, extensionFile, user)) + assertThatThrownBy(() -> svc.runValidation(scan, extensionFile, user)) .isInstanceOf(ErrorResultException.class); verify(persistenceService).completeWithStatus(scan, ScanStatus.REJECTED); @@ -360,7 +357,7 @@ void runValidation_recordsAllFindings() { new PublishCheckRunner.Finding("CHECK_1", "rule2", "reason2", false, null), new PublishCheckRunner.Finding("CHECK_1", "rule3", "reason3", false, null) ); - when(checkRunner.runChecks(any(), any(), any(), any())) + when(checkRunner.runChecks(any(), any(), any())) .thenReturn(new PublishCheckRunner.Result( findings, List.of(checkExecution("CHECK_1", ScanCheckResult.CheckResult.PASSED)), @@ -370,7 +367,7 @@ void runValidation_recordsAllFindings() { )); // Act - svc.runValidation(scan, extensionProcessor, extensionFile, user); + svc.runValidation(scan, extensionFile, user); // Assert: all 3 findings recorded verify(persistenceService, times(3)).recordValidationFailure(eq(scan), eq("CHECK_1"), any(), any(), eq(false)); diff --git a/server/src/test/java/org/eclipse/openvsx/scanning/MaliciousZipCheckServiceTest.java b/server/src/test/java/org/eclipse/openvsx/scanning/MaliciousZipCheckServiceTest.java new file mode 100644 index 000000000..31cfcc547 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/scanning/MaliciousZipCheckServiceTest.java @@ -0,0 +1,112 @@ +/******************************************************************************** + * 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.apache.commons.compress.archivers.zip.ExtraFieldUtils; +import org.apache.commons.compress.archivers.zip.UnicodeCommentExtraField; +import org.apache.commons.compress.archivers.zip.ZipExtraField; +import org.eclipse.openvsx.entities.ExtensionScan; +import org.eclipse.openvsx.entities.UserData; +import org.eclipse.openvsx.util.TempFile; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.FileOutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests for MaliciousZipCheckService. + */ +class MaliciousZipCheckServiceTest { + + @TempDir + Path tempDir; + + private MaliciousZipCheckService service; + + @BeforeEach + void setUp() { + service = new MaliciousZipCheckService(); + } + + @Test + void check_passesWhenNoExtraFields() throws Exception { + // Create a test zip with clean files + TempFile extensionFile = createTestZip("clean.txt", "This is clean content"); + + var context = createContext(extensionFile); + var result = service.check(context); + + assertTrue(result.passed()); + assertTrue(result.failures().isEmpty()); + } + + @Test + void check_failsWhenExtraFieldsAreFound() throws Exception { + // Create a test zip with a file that contains extra fields + TempFile extensionFile = createTestZipWithExtraField("extra.txt", "The content is clean"); + + var context = createContext(extensionFile); + var result = service.check(context); + + assertFalse(result.passed()); + assertEquals(1, result.failures().size()); + assertEquals("EXTRA_FIELDS_DETECTED", result.failures().getFirst().ruleName()); + assertTrue(result.failures().getFirst().reason().contains("extension file contains zip entries")); + } + + // --- Helper methods --- + + private TempFile createTestZip(String fileName, String content) throws Exception { + Path zipPath = tempDir.resolve("test-extension.vsix"); + try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(zipPath.toFile()))) { + ZipEntry entry = new ZipEntry(fileName); + zos.putNextEntry(entry); + zos.write(content.getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + return new TempFile(zipPath); + } + + private TempFile createTestZipWithExtraField(String fileName, String content) throws Exception { + Path zipPath = tempDir.resolve("test-extension-extra.vsix"); + try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(zipPath.toFile()))) { + ZipEntry entry = new ZipEntry(fileName); + var field = new UnicodeCommentExtraField("sample data", "sample data".getBytes()); + var data = ExtraFieldUtils.mergeLocalFileDataData(new ZipExtraField[] { field }); + entry.setExtra(data); + zos.putNextEntry(entry); + zos.write(content.getBytes(StandardCharsets.UTF_8)); + zos.closeEntry(); + } + return new TempFile(zipPath); + } + + private PublishCheck.Context createContext(TempFile extensionFile) { + ExtensionScan scan = new ExtensionScan(); + scan.setNamespaceName("test-namespace"); + scan.setExtensionName("test-extension"); + scan.setExtensionVersion("1.0.0"); + + UserData user = new UserData(); + user.setLoginName("testuser"); + + return new PublishCheck.Context(scan, extensionFile, user); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/scanning/PublishCheckRunnerTest.java b/server/src/test/java/org/eclipse/openvsx/scanning/PublishCheckRunnerTest.java index cf70ac516..bd252cd16 100644 --- a/server/src/test/java/org/eclipse/openvsx/scanning/PublishCheckRunnerTest.java +++ b/server/src/test/java/org/eclipse/openvsx/scanning/PublishCheckRunnerTest.java @@ -12,7 +12,6 @@ ********************************************************************************/ package org.eclipse.openvsx.scanning; -import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.ExtensionScan; import org.eclipse.openvsx.entities.ScanCheckResult; import org.eclipse.openvsx.entities.UserData; @@ -37,8 +36,6 @@ class PublishCheckRunnerTest { @Mock ExtensionScan scan; - @Mock - ExtensionProcessor processor; @Mock TempFile extensionFile; @Mock UserData user; @@ -48,7 +45,7 @@ void runChecks_passesWhenAllChecksPass() { var check2 = mockCheck("CHECK2", true, true, PublishCheck.Result.pass()); var runner = new PublishCheckRunner(List.of(check1, check2)); - var result = runner.runChecks(scan, processor, extensionFile, user); + var result = runner.runChecks(scan, extensionFile, user); assertTrue(result.passed()); assertFalse(result.hasEnforcedFailure()); @@ -64,7 +61,7 @@ void runChecks_failsWhenEnforcedCheckFails() { var failingCheck = mockCheck("FAIL", true, true, PublishCheck.Result.fail(List.of(failure))); var runner = new PublishCheckRunner(List.of(passingCheck, failingCheck)); - var result = runner.runChecks(scan, processor, extensionFile, user); + var result = runner.runChecks(scan, extensionFile, user); assertFalse(result.passed()); assertTrue(result.hasEnforcedFailure()); @@ -78,7 +75,7 @@ void runChecks_passesWhenNonEnforcedCheckFails() { var failingCheck = mockCheck("CHECK", true, false, PublishCheck.Result.fail(List.of(failure))); var runner = new PublishCheckRunner(List.of(failingCheck)); - var result = runner.runChecks(scan, processor, extensionFile, user); + var result = runner.runChecks(scan, extensionFile, user); assertTrue(result.passed()); assertFalse(result.hasEnforcedFailure()); @@ -92,7 +89,7 @@ void runChecks_skipsDisabledChecks() { var enabledCheck = mockCheck("ENABLED", true, true, PublishCheck.Result.pass()); var runner = new PublishCheckRunner(List.of(disabledCheck, enabledCheck)); - var result = runner.runChecks(scan, processor, extensionFile, user); + var result = runner.runChecks(scan, extensionFile, user); assertTrue(result.passed()); // Only enabled check should have an execution record @@ -109,7 +106,7 @@ void runChecks_capturesCheckError() { when(errorCheck.check(any())).thenThrow(new RuntimeException("Check failed")); var runner = new PublishCheckRunner(List.of(errorCheck)); - var result = runner.runChecks(scan, processor, extensionFile, user); + var result = runner.runChecks(scan, extensionFile, user); assertFalse(result.passed()); assertTrue(result.hasError()); @@ -131,7 +128,7 @@ void runChecks_stopsAfterError() { var runner = new PublishCheckRunner(List.of(errorCheck, afterErrorCheck)); - var result = runner.runChecks(scan, processor, extensionFile, user); + var result = runner.runChecks(scan, extensionFile, user); // Should have only one execution (the erroring check) assertEquals(1, result.checkExecutions().size()); @@ -185,7 +182,7 @@ void checkExecution_recordsCorrectResult() { var failingCheck = mockCheck("TEST", true, true, PublishCheck.Result.fail(List.of(failure))); var runner = new PublishCheckRunner(List.of(failingCheck)); - var result = runner.runChecks(scan, processor, extensionFile, user); + var result = runner.runChecks(scan, extensionFile, user); var execution = result.checkExecutions().getFirst(); assertEquals("TEST", execution.checkType()); diff --git a/server/src/test/java/org/eclipse/openvsx/scanning/ScannerInvocationTest.java b/server/src/test/java/org/eclipse/openvsx/scanning/ScannerInvocationTest.java index 81e1185ab..9fe2ed55d 100644 --- a/server/src/test/java/org/eclipse/openvsx/scanning/ScannerInvocationTest.java +++ b/server/src/test/java/org/eclipse/openvsx/scanning/ScannerInvocationTest.java @@ -47,6 +47,6 @@ void completed_withThreats() { assertFalse(invocation.result().isClean()); assertEquals(1, invocation.result().getThreats().size()); - assertEquals("virus", invocation.result().getThreats().get(0).getName()); + assertEquals("virus", invocation.result().getThreats().getFirst().getName()); } } diff --git a/server/src/test/java/org/eclipse/openvsx/scanning/ScannerRegistryTest.java b/server/src/test/java/org/eclipse/openvsx/scanning/ScannerRegistryTest.java index 80e274513..e3b1f2916 100644 --- a/server/src/test/java/org/eclipse/openvsx/scanning/ScannerRegistryTest.java +++ b/server/src/test/java/org/eclipse/openvsx/scanning/ScannerRegistryTest.java @@ -47,8 +47,7 @@ void registerScanner_throwsOnDuplicate() { registry.registerScanner(scanner1); - assertThrows(IllegalArgumentException.class, () -> - registry.registerScanner(scanner2)); + assertThrows(IllegalArgumentException.class, () -> registry.registerScanner(scanner2)); } @Test @@ -120,8 +119,7 @@ void getRegisteredTypes_returnsImmutableSet() { var types = registry.getRegisteredTypes(); - assertThrows(UnsupportedOperationException.class, () -> - types.add("TYPE_B")); + assertThrows(UnsupportedOperationException.class, () -> types.add("TYPE_B")); } @Test diff --git a/server/src/test/java/org/eclipse/openvsx/search/SimilarityCheckServiceTest.java b/server/src/test/java/org/eclipse/openvsx/search/SimilarityCheckServiceTest.java index f42ef217c..863c2c207 100644 --- a/server/src/test/java/org/eclipse/openvsx/search/SimilarityCheckServiceTest.java +++ b/server/src/test/java/org/eclipse/openvsx/search/SimilarityCheckServiceTest.java @@ -12,7 +12,6 @@ ********************************************************************************/ package org.eclipse.openvsx.search; -import org.eclipse.openvsx.ExtensionProcessor; import org.eclipse.openvsx.entities.*; import org.eclipse.openvsx.repositories.RepositoryService; import org.eclipse.openvsx.scanning.PublishCheck; @@ -48,9 +47,6 @@ class SimilarityCheckServiceTest { @InjectMocks SimilarityCheckService similarityCheckService; - @Mock - ExtensionProcessor processor; - @Mock TempFile extensionFile; @@ -68,7 +64,7 @@ private PublishCheck.Context createContext(String namespaceName, String extensio scan.setNamespaceName(namespaceName); scan.setExtensionName(extensionName); scan.setExtensionDisplayName(displayName); - return new PublishCheck.Context(scan, processor, extensionFile, user); + return new PublishCheck.Context(scan, extensionFile, user); } @Test From c7ea7cd683e31f0ae2925477cc5e5af8447139d4 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Fri, 27 Mar 2026 13:17:44 +0100 Subject: [PATCH 3/3] remove spurious debug statement --- server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java b/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java index cab84e994..35bfb52c8 100644 --- a/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java +++ b/server/src/main/java/org/eclipse/openvsx/ExtensionProcessor.java @@ -80,7 +80,6 @@ private void readInputStream() { try { zipFile = new ZipFile(extensionFile.getPath().toFile()); } catch (ZipException exc) { - exc.printStackTrace(); throw new ErrorResultException("Could not read zip file: " + exc.getMessage()); } catch (IOException exc) { throw new ErrorResultException("Could not read from input stream: " + exc.getMessage());