diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/PluginRegistryService.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/PluginRegistryService.java index 7e24f69..7613c51 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/PluginRegistryService.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/PluginRegistryService.java @@ -5,7 +5,6 @@ import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobDefinitionRepository; import jakarta.annotation.PostConstruct; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -136,14 +135,24 @@ public void registerAll() { pluginsByJobName.keySet()); } - /** Returns all registered plugins in registration order. */ + /** + * Returns an immutable snapshot of all registered plugins in registration order. + * + *

Takes the {@code pluginsByJobName} monitor and copies the values so the + * returned collection is safe to read while concurrent register/unregister calls + * mutate the map. + */ public Collection getPlugins() { - return Collections.unmodifiableCollection(pluginsByJobName.values()); + synchronized (pluginsByJobName) { + return List.copyOf(pluginsByJobName.values()); + } } /** Returns the plugin registered under the given job name, or empty if not found. */ public Optional getPlugin(String jobName) { - return Optional.ofNullable(pluginsByJobName.get(jobName)); + synchronized (pluginsByJobName) { + return Optional.ofNullable(pluginsByJobName.get(jobName)); + } } /** @@ -284,8 +293,10 @@ public List getRegisteredJobDefinitions() { * or {@code null} if not found in either source */ public String getPluginBySource(String jobName) { - if (pluginsByJobName.containsKey(jobName)) { - return "CLASSPATH"; + synchronized (pluginsByJobName) { + if (pluginsByJobName.containsKey(jobName)) { + return "CLASSPATH"; + } } Optional def = jobDefinitionRepository.findByJobName(jobName); if (def.isPresent() && Boolean.TRUE.equals(def.get().getEnabled())) { diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java index bacd12d..7a07c43 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java @@ -100,12 +100,14 @@ public JarUploadResponse uploadJar( validateJar(file); validateMainClassName(mainClassName); - String checksum = ChecksumUtil.computeSha256(file); - checkDuplicate(jobName); Path storedPath = storeFile(file, jobName, version); + // Compute the checksum from the persisted file (not the multipart stream) so the + // stored digest matches the bytes that load-time integrity verification re-checks. + String checksum = ChecksumUtil.computeSha256(storedPath); + // ── signature verification ─────────────────────────────────────────────── SignatureResult sigResult = jarSignatureVerifier.verify(storedPath); if (!sigResult.valid()) { diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java new file mode 100644 index 0000000..4e2d9c0 --- /dev/null +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadServiceTest.java @@ -0,0 +1,131 @@ +/* 2024-2026 */ +package com.fronzec.frbatchservice.batchjobs.plugins.service; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.fronzec.frbatchservice.batchjobs.plugins.audit.AuditService; +import com.fronzec.frbatchservice.batchjobs.plugins.entity.JobDefinitionEntity; +import com.fronzec.frbatchservice.batchjobs.plugins.metrics.PluginMetrics; +import com.fronzec.frbatchservice.batchjobs.plugins.repository.JobDefinitionRepository; +import com.fronzec.frbatchservice.batchjobs.plugins.util.ChecksumUtil; +import com.fronzec.frbatchservice.config.AutoApproveConfig; +import com.fronzec.frbatchservice.web.dto.JarUploadResponse; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Optional; +import java.util.jar.Attributes; +import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; +import java.util.zip.ZipEntry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.mock.web.MockMultipartFile; +import org.springframework.web.multipart.MultipartFile; + +/** Verifies the upload pipeline of {@link JarUploadService}. */ +@ExtendWith(MockitoExtension.class) +@DisplayName("JarUploadService") +class JarUploadServiceTest { + + @Mock private JobDefinitionRepository jobDefinitionRepository; + @Mock private JarSignatureVerifier jarSignatureVerifier; + @Mock private AuditService auditService; + @Mock private PluginMetrics pluginMetrics; + + @TempDir private Path jarDir; + + private JarUploadService service; + + @BeforeEach + void setUp() { + service = + new JarUploadService( + jobDefinitionRepository, + Optional.empty(), + jarSignatureVerifier, + auditService, + pluginMetrics, + "permissive", + jarDir.toString()); + } + + @Test + @DisplayName("computes the persisted checksum from the stored file, never from the multipart stream") + void uploadJar_checksumIsComputedFromStoredFile_notMultipartStream() throws IOException { + byte[] jarBytes = minimalJar(); + MockMultipartFile file = + new MockMultipartFile( + "file", "test-job-1.0.0.jar", "application/java-archive", jarBytes); + Path storedPath = jarDir.resolve("test-job-1.0.0.jar"); + + when(jobDefinitionRepository.findByJobName("test-job")).thenReturn(Optional.empty()); + when(jarSignatureVerifier.verify(any())) + .thenReturn(new SignatureResult(true, null, List.of())); + when(jobDefinitionRepository.save(any(JobDefinitionEntity.class))) + .thenAnswer( + invocation -> { + JobDefinitionEntity saved = invocation.getArgument(0); + saved.setId(1L); + return saved; + }); + + JarUploadResponse response; + try (MockedStatic checksum = mockStatic(ChecksumUtil.class)) { + // Stub only the Path overload: the digest the service stores must come from + // the persisted file. The multipart overload is left unstubbed so that a + // regression to hashing the stream would surface a null checksum AND trip + // the never() verification below. + checksum.when(() -> ChecksumUtil.computeSha256(storedPath)).thenReturn("disk-digest"); + + response = service.uploadJar(file, "test-job", "1.0.0", "com.example.MyPlugin"); + + // The checksum was read from the persisted file, not from the upload stream. + checksum.verify(() -> ChecksumUtil.computeSha256(storedPath)); + checksum.verify(() -> ChecksumUtil.computeSha256(any(MultipartFile.class)), never()); + } + + ArgumentCaptor captor = + ArgumentCaptor.forClass(JobDefinitionEntity.class); + verify(jobDefinitionRepository).save(captor.capture()); + + assertTrue(Files.exists(storedPath), "JAR should be written to disk before hashing"); + assertArrayEquals( + jarBytes, Files.readAllBytes(storedPath), "stored bytes should match the upload"); + assertEquals( + "disk-digest", + captor.getValue().getJarChecksum(), + "persisted checksum must be the digest of the stored file"); + assertEquals("disk-digest", response.jarChecksum(), "response must expose the same checksum"); + } + + /** Builds a minimal valid (unsigned) JAR with a manifest and one entry. */ + private static byte[] minimalJar() throws IOException { + Manifest manifest = new Manifest(); + manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (JarOutputStream jar = new JarOutputStream(out, manifest)) { + jar.putNextEntry(new ZipEntry("com/example/MyPlugin.class")); + jar.write("stub".getBytes()); + jar.closeEntry(); + } + return out.toByteArray(); + } +}