Add uploaded file validation

multiapps-controller-core/pom.xml

@@ -1,5 +1,5 @@
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-    xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <artifactId>multiapps-controller-core</artifactId>
@@ -13,6 +13,10 @@
     </parent>
 
     <dependencies>
+        <dependency>
+            <groupId>org.apache.tika</groupId>
+            <artifactId>tika-core</artifactId>
+        </dependency>
         <dependency>
             <groupId>jakarta.xml.bind</groupId>
             <artifactId>jakarta.xml.bind-api</artifactId>

multiapps-controller-core/src/main/java/module-info.java

@@ -59,8 +59,11 @@
     requires org.apache.commons.collections4;
     requires org.apache.commons.io;
     requires org.apache.commons.lang3;
+    requires org.apache.httpcomponents.httpclient;
+    requires org.apache.httpcomponents.httpcore;
     requires org.apache.httpcomponents.client5.httpclient5;
     requires org.apache.httpcomponents.core5.httpcore5;
+    requires org.apache.tika.core;
     requires org.cloudfoundry.multiapps.common;
     requires org.cloudfoundry.multiapps.controller.api;
     requires org.slf4j;

multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/Messages.java

@@ -91,6 +91,8 @@ public final class Messages {
     public static final String BUILDPACKS_REQUIRED_FOR_CNB = "Buildpacks must be provided when lifecycle is set to 'cnb'.";
     public static final String DOCKER_INFO_REQUIRED = "Docker information must be provided when lifecycle is set to 'docker'.";
     public static final String BUILDPACKS_NOT_ALLOWED_WITH_DOCKER = "Buildpacks must not be provided when lifecycle is set to 'docker'.";
+    public static final String EXTENSION_DESCRIPTORS_COULD_NOT_BE_PARSED_TO_VALID_YAML = "Extension descriptor(s) could not be parsed as a valid YAML file. These descriptors may fail future deployments once stricter validation is enforced. Please review and correct them now to avoid future issues. Use at your own risk";
+    public static final String UNSUPPORTED_FILE_FORMAT = "Unsupported file format! \"{0}\" detected";
 
     // Warning messages
     public static final String ENVIRONMENT_VARIABLE_IS_NOT_SET_USING_DEFAULT = "Environment variable \"{0}\" is not set. Using default \"{1}\"...";

multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/helpers/DescriptorParserFacadeFactory.java

@@ -2,7 +2,6 @@
 
 import jakarta.inject.Inject;
 import jakarta.inject.Named;
-
 import org.cloudfoundry.multiapps.common.util.YamlParser;
 import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration;
 import org.cloudfoundry.multiapps.mta.handlers.DescriptorParserFacade;
@@ -19,9 +18,21 @@ public DescriptorParserFacadeFactory(ApplicationConfiguration applicationConfigu
     }
 
     public DescriptorParserFacade getInstance() {
-        LoaderOptions loaderOptions = new LoaderOptions();
-        loaderOptions.setMaxAliasesForCollections(applicationConfiguration.getSnakeyamlMaxAliasesForCollections());
+        LoaderOptions loaderOptions = createLoaderOptions(true);
         YamlParser yamlParser = new YamlParser(loaderOptions);
         return new DescriptorParserFacade(yamlParser);
     }
+
+    public DescriptorParserFacade getInstanceWithDisabledDuplicateKeys() {
+        LoaderOptions loaderOptions = createLoaderOptions(false);
+        YamlParser yamlParser = new YamlParser(loaderOptions);
+        return new DescriptorParserFacade(yamlParser);
+    }
+
+    private LoaderOptions createLoaderOptions(boolean shouldAllowDuplicateKeys) {
+        LoaderOptions loaderOptions = new LoaderOptions();
+        loaderOptions.setMaxAliasesForCollections(applicationConfiguration.getSnakeyamlMaxAliasesForCollections());
+        loaderOptions.setAllowDuplicateKeys(shouldAllowDuplicateKeys);
+        return loaderOptions;
+    }
 }

multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/validators/parameters/FileMimeTypeValidator.java

@@ -0,0 +1,69 @@
+package org.cloudfoundry.multiapps.controller.core.validators.parameters;
+
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.text.MessageFormat;
+import java.util.function.Consumer;
+
+import jakarta.inject.Inject;
+import jakarta.inject.Named;
+import org.apache.tika.Tika;
+import org.cloudfoundry.multiapps.common.ParsingException;
+import org.cloudfoundry.multiapps.common.SLException;
+import org.cloudfoundry.multiapps.controller.core.Messages;
+import org.cloudfoundry.multiapps.controller.core.helpers.DescriptorParserFacadeFactory;
+import org.cloudfoundry.multiapps.controller.persistence.services.FileService;
+import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException;
+
+@Named
+public class FileMimeTypeValidator {
+
+    private static final String APPLICATION_OCTET_STREAM_MIME_TYPE = "application/octet-stream";
+    private static final String APPLICATION_ZIP_MIME_TYPE = "application/zip";
+    private static final String TEXT_PLAIN_MIME_TYPE = "text/plain";
+    private static final Tika mimeTypeDetector = new Tika();
+    private final DescriptorParserFacadeFactory descriptorParserFactory;
+    private final FileService fileService;
+
+    @Inject
+    public FileMimeTypeValidator(FileService fileService, DescriptorParserFacadeFactory descriptorParserFactory) {
+        this.fileService = fileService;
+        this.descriptorParserFactory = descriptorParserFactory;
+    }
+
+    public void validateFileType(String spaceGuid, String appArchiveId, Consumer<String> stepLogger) {
+        try (InputStream fileInputStream = fileService.openInputStream(spaceGuid, appArchiveId);
+            InputStream bufferedFileInputStream = new BufferedInputStream(fileInputStream)) {
+            validateInputStreamMimeType(bufferedFileInputStream, stepLogger);
+        } catch (FileStorageException | IOException e) {
+            throw new SLException(e);
+        }
+    }
+
+    private void validateInputStreamMimeType(InputStream uploadedFileInputStream, Consumer<String> stepLogger) throws IOException {
+        String detectedType = getFileMimeType(uploadedFileInputStream);
+
+        switch (detectedType) {
+            case TEXT_PLAIN_MIME_TYPE -> validateYamlFile(uploadedFileInputStream, stepLogger);
+            case APPLICATION_ZIP_MIME_TYPE, APPLICATION_OCTET_STREAM_MIME_TYPE -> {
+            }
+            default -> stepLogger.accept(MessageFormat.format(Messages.UNSUPPORTED_FILE_FORMAT, detectedType));
+        }
+
+    }
+
+    private String getFileMimeType(InputStream uploadedFileInputStream) throws IOException {
+        return mimeTypeDetector.detect(uploadedFileInputStream);
+    }
+
+    private void validateYamlFile(InputStream uploadedFileInputStream, Consumer<String> stepLogger) {
+        try {
+            descriptorParserFactory.getInstanceWithDisabledDuplicateKeys()
+                                   .parseExtensionDescriptor(uploadedFileInputStream);
+        } catch (ParsingException e) {
+            stepLogger.accept(Messages.EXTENSION_DESCRIPTORS_COULD_NOT_BE_PARSED_TO_VALID_YAML);
+            stepLogger.accept(e.getMessage());
+        }
+    }
+}

multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/helpers/DescriptorParserFacadeFactoryTest.java

@@ -23,4 +23,16 @@ void testGetInstance() {
         Assertions.assertThrows(ParsingException.class, () -> instance.parseDeploymentDescriptor(mtadYaml));
     }
 
+    @Test
+    void testParseWithDuplicateValues() {
+        final int maxAliases = 5;
+        ApplicationConfiguration applicationConfiguration = Mockito.mock(ApplicationConfiguration.class);
+        Mockito.when(applicationConfiguration.getSnakeyamlMaxAliasesForCollections())
+               .thenReturn(maxAliases);
+        DescriptorParserFacadeFactory descriptorParserFacadeFactory = new DescriptorParserFacadeFactory(applicationConfiguration);
+        DescriptorParserFacade instance = descriptorParserFacadeFactory.getInstanceWithDisabledDuplicateKeys();
+        InputStream mtadYaml = getClass().getResourceAsStream("duplicate-keys.yaml");
+        Assertions.assertThrows(ParsingException.class, () -> instance.parseDeploymentDescriptor(mtadYaml));
+    }
+
 }

multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/validators/parameters/FileMimeTypeValidatorTest.java

@@ -0,0 +1,66 @@
+package org.cloudfoundry.multiapps.controller.core.validators.parameters;
+
+import java.io.InputStream;
+import java.util.UUID;
+
+import org.cloudfoundry.multiapps.controller.core.Messages;
+import org.cloudfoundry.multiapps.controller.core.helpers.DescriptorParserFacadeFactory;
+import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration;
+import org.cloudfoundry.multiapps.controller.persistence.services.FileService;
+import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.when;
+
+class FileMimeTypeValidatorTest {
+
+    @Mock
+    private FileService fileService;
+
+    private FileMimeTypeValidator fileMimeTypeValidator;
+
+    private final String TEST_SPACE_ID = UUID.randomUUID()
+                                             .toString();
+    private final String TEST_FILE_ID = UUID.randomUUID()
+                                            .toString();
+
+    private DescriptorParserFacadeFactory descriptorParserFacadeFactory;
+
+    @BeforeEach
+    void setUp() throws Exception {
+        MockitoAnnotations.openMocks(this)
+                          .close();
+        int maxAliases = 5;
+        ApplicationConfiguration applicationConfiguration = Mockito.mock(ApplicationConfiguration.class);
+        when(applicationConfiguration.getSnakeyamlMaxAliasesForCollections())
+            .thenReturn(maxAliases);
+        descriptorParserFacadeFactory = new DescriptorParserFacadeFactory(applicationConfiguration);
+        fileMimeTypeValidator = new FileMimeTypeValidator(fileService, descriptorParserFacadeFactory);
+    }
+
+    @Test
+    void testValidationWithCorrectYaml() throws FileStorageException {
+        InputStream mtadYaml = getClass().getResourceAsStream("valid-format.yaml");
+        when(fileService.openInputStream(TEST_SPACE_ID, TEST_FILE_ID)).thenReturn(mtadYaml);
+        fileMimeTypeValidator = new FileMimeTypeValidator(fileService, descriptorParserFacadeFactory);
+        fileMimeTypeValidator.validateFileType(TEST_SPACE_ID, TEST_FILE_ID, message -> {
+        });
+    }
+
+    @Test
+    void testValidationWithDuplicateKeys() throws FileStorageException {
+        InputStream mtadYaml = getClass().getResourceAsStream("duplicate-key.yaml");
+        when(fileService.openInputStream(TEST_SPACE_ID, TEST_FILE_ID)).thenReturn(mtadYaml);
+        fileMimeTypeValidator = new FileMimeTypeValidator(fileService, descriptorParserFacadeFactory);
+        fileMimeTypeValidator.validateFileType(TEST_SPACE_ID, TEST_FILE_ID, message -> {
+            boolean doesMessageContainWarningMessage = message.contains(Messages.EXTENSION_DESCRIPTORS_COULD_NOT_BE_PARSED_TO_VALID_YAML);
+            boolean doesMessageContainException = message.contains("Error while parsing YAML string");
+            assertTrue(doesMessageContainException || doesMessageContainWarningMessage);
+        });
+    }
+}

multiapps-controller-core/src/test/resources/org/cloudfoundry/multiapps/controller/core/helpers/duplicate-keys.yaml

@@ -0,0 +1,10 @@
+_schema-version: "3.1"
+ID: com.test.1
+version: 1.0.0
+
+modules:
+  - name: spark-service-admin
+    type: abc
+    properties:
+      MAX_SIZE: а
+      MAX_SIZE: а

multiapps-controller-core/src/test/resources/org/cloudfoundry/multiapps/controller/core/validators/parameters/duplicate-key.yaml

@@ -0,0 +1,9 @@
+_schema-version: "3.3"
+ID: com.test
+extends: com.test
+
+modules:
+  - name: spark-service-admin
+    properties:
+      MAX_SIZE: а
+      MAX_SIZE: а

multiapps-controller-core/src/test/resources/org/cloudfoundry/multiapps/controller/core/validators/parameters/valid-format.yaml

@@ -0,0 +1,8 @@
+_schema-version: "3.3"
+ID: com.test
+extends: com.test
+
+modules:
+  - name: spark-service-admin
+    properties:
+      MAX_SIZE: а

multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/SyncFlowableStep.java

@@ -1,10 +1,14 @@
 package org.cloudfoundry.multiapps.controller.process.steps;
 
 import java.util.function.BiFunction;
+import java.util.function.Consumer;
 
+import com.sap.cloudfoundry.client.facade.CloudControllerException;
+import com.sap.cloudfoundry.client.facade.CloudOperationException;
+import com.sap.cloudfoundry.client.facade.CloudServiceBrokerException;
+import io.netty.handler.timeout.TimeoutException;
 import jakarta.inject.Inject;
 import jakarta.inject.Named;
-
 import org.apache.commons.lang3.StringUtils;
 import org.cloudfoundry.multiapps.common.ContentException;
 import org.cloudfoundry.multiapps.common.SLException;
@@ -28,12 +32,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.sap.cloudfoundry.client.facade.CloudControllerException;
-import com.sap.cloudfoundry.client.facade.CloudOperationException;
-import com.sap.cloudfoundry.client.facade.CloudServiceBrokerException;
-
-import io.netty.handler.timeout.TimeoutException;
-
 public abstract class SyncFlowableStep implements JavaDelegate {
 
     protected final Logger logger = LoggerFactory.getLogger(getClass());
@@ -120,7 +118,7 @@ private void handleException(ProcessContext context, Exception e) {
      *
      * @param context flowable context of the step
      * @param e thrown exception from {@link #executeStep(ProcessContext) executeStep} and pre-processed by
-     *        {@link #handleException(ProcessContext, Exception) handleException}
+     * {@link #handleException(ProcessContext, Exception) handleException}
      * @throws Exception in case derivative methods throw exception
      */
     protected void onStepError(ProcessContext context, Exception e) throws Exception {
@@ -221,4 +219,8 @@ protected ProcessLoggerPersister getProcessLogsPersister() {
         return processLoggerPersister;
     }
 
+    protected Consumer<String> getStepWarningLoggerConsumer() {
+        return message -> getStepLogger().warn(message);
+    }
+
 }

multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/ValidateDeployParametersStep.java

@@ -8,10 +8,13 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 
+import jakarta.inject.Inject;
+import jakarta.inject.Named;
 import org.apache.commons.io.IOUtils;
 import org.cloudfoundry.multiapps.common.ContentException;
 import org.cloudfoundry.multiapps.common.SLException;
 import org.cloudfoundry.multiapps.controller.client.util.ResilientOperationExecutor;
+import org.cloudfoundry.multiapps.controller.core.validators.parameters.FileMimeTypeValidator;
 import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry;
 import org.cloudfoundry.multiapps.controller.persistence.model.ImmutableFileEntry;
 import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException;
@@ -25,19 +28,18 @@
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.context.annotation.Scope;
 
-import jakarta.inject.Inject;
-import jakarta.inject.Named;
-
 @Named("validateDeployParametersStep")
 @Scope(BeanDefinition.SCOPE_PROTOTYPE)
 public class ValidateDeployParametersStep extends SyncFlowableStep {
 
     private final ResilientOperationExecutor resilientOperationExecutor = new ResilientOperationExecutor();
     private final ExecutorService fileStorageThreadPool;
+    private final FileMimeTypeValidator fileMimeTypeValidator;
 
     @Inject
-    public ValidateDeployParametersStep(ExecutorService fileStorageThreadPool) {
+    public ValidateDeployParametersStep(ExecutorService fileStorageThreadPool, FileMimeTypeValidator fileMimeTypeValidator) {
         this.fileStorageThreadPool = fileStorageThreadPool;
+        this.fileMimeTypeValidator = fileMimeTypeValidator;
     }
 
     @Override
@@ -62,8 +64,10 @@ private void validateParameters(ProcessContext context) {
         List<FileEntry> extensionDescriptors = validateExtensionDescriptorFileIds(context);
         List<FileEntry> archivePartEntries = getArchivePartEntries(context);
         validateFilesSizeLimit(context, archivePartEntries, extensionDescriptors);
+        extensionDescriptors.forEach(this::validateFileEntryType);
 
         if (archivePartEntries.size() == 1) {
+            validateFileEntryType(archivePartEntries.get(0));
             getStepLogger().infoWithoutProgressMessage(Messages.ARCHIVE_WAS_NOT_SPLIT_TOTAL_SIZE_IN_BYTES_0, archivePartEntries.get(0)
                                                                                                                                .getSize());
         } else {
@@ -181,6 +185,7 @@ private void mergeArchive(ProcessContext context, List<FileEntry> archivePartEnt
                                                        archivePartEntries.size(), archiveSize);
             FileEntry uploadedArchive = persistArchive(archiveStreamWithName, context, archiveSize);
             context.setVariable(Variables.APP_ARCHIVE_ID, uploadedArchive.getId());
+            validateFileEntryType(uploadedArchive);
             getStepLogger().infoWithoutProgressMessage(MessageFormat.format(Messages.ARCHIVE_WITH_ID_0_AND_NAME_1_WAS_STORED,
                                                                             uploadedArchive.getId(),
                                                                             archiveStreamWithName.getArchiveName()));
@@ -189,13 +194,17 @@ private void mergeArchive(ProcessContext context, List<FileEntry> archivePartEnt
         }
     }
 
+    private void validateFileEntryType(FileEntry fileEntry) {
+        fileMimeTypeValidator.validateFileType(fileEntry.getSpace(), fileEntry.getId(), getStepWarningLoggerConsumer());
+    }
+
     private String[] getArchivePartIds(ProcessContext context) {
         String archiveId = context.getRequiredVariable(Variables.APP_ARCHIVE_ID);
         return archiveId.split(",");
     }
 
     private List<FileEntry> getArchivePartEntries(ProcessContext context, String[] appArchivePartsId) {
-        return Arrays.stream(appArchivePartsId) 
+        return Arrays.stream(appArchivePartsId)
                      .map(appArchivePartId -> findFile(context, appArchivePartId))
                      .toList();
     }