This is an automated email from the ASF dual-hosted git repository. jayblanc pushed a commit to branch UNOMI-973-file-endpoint-containment in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 86b625641aac178a84ef209e2aea03c3a63bdb23 Author: Jérôme Blanchard <[email protected]> AuthorDate: Tue Aug 11 15:52:19 2026 +0200 UNOMI-973: Refuse a configuration whose endpoint cannot be honoured when it is saved The route carrying an import or export configuration is built asynchronously, long after the REST call has answered. A configuration whose endpoint is refused there was still stored and still answered 200, leaving one log line as the only trace -- the caller could not tell it from one that works. Both configuration endpoints now validate the endpoint URI of a recurrent configuration before storing it, and answer 400 Bad Request carrying the reason, so the caller can correct it. A oneshot import names no endpoint -- its file is uploaded separately -- and is unaffected. RouterCamelContext publishes the scheme allow-list and the permitted base directories through ConfigSharingService, the way it already publishes the oneshot upload directory, since router-rest cannot see the configuration router-core is wired with. --- .../apache/unomi/router/api/RouterConstants.java | 4 + .../router/core/context/RouterCamelContext.java | 4 + extensions/router/router-rest/pom.xml | 7 + .../rest/AbstractConfigurationServiceEndpoint.java | 28 +++ .../rest/ExportConfigurationServiceEndPoint.java | 14 +- .../rest/ImportConfigurationServiceEndPoint.java | 10 +- .../rest/ConfigurationEndpointValidationTest.java | 265 +++++++++++++++++++++ 7 files changed, 325 insertions(+), 7 deletions(-) diff --git a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/RouterConstants.java b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/RouterConstants.java index 5ef19fe44..03d84bcb2 100644 --- a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/RouterConstants.java +++ b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/RouterConstants.java @@ -51,6 +51,10 @@ public interface RouterConstants { String IMPORT_ONESHOT_ROUTE_ID = "ONE_SHOT_ROUTE"; String IMPORT_ONESHOT_UPLOAD_DIR = "oneshotImportUploadDir"; + String CONFIG_ALLOWED_ENDPOINTS = "routerAllowedEndpoints"; + String CONFIG_IMPORT_BASE_DIRS = "routerImportBaseDirs"; + String CONFIG_EXPORT_BASE_DIRS = "routerExportBaseDirs"; + String DEFAULT_FILE_COLUMN_SEPARATOR = ","; String DEFAULT_FILE_LINE_SEPARATOR = "\n"; String KEY_HISTORY_SIZE = "historySize"; diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java index 397e50cc1..15f401880 100644 --- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java +++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java @@ -110,6 +110,10 @@ public class RouterCamelContext implements IRouterCamelContext { scheduler = Executors.newSingleThreadScheduledExecutor(); configSharingService.setProperty(RouterConstants.IMPORT_ONESHOT_UPLOAD_DIR, uploadDir); + // shared with router-rest, which validates a configuration's endpoint before it is stored + configSharingService.setProperty(RouterConstants.CONFIG_ALLOWED_ENDPOINTS, allowedEndpoints); + configSharingService.setProperty(RouterConstants.CONFIG_IMPORT_BASE_DIRS, permittedImportBaseDirs); + configSharingService.setProperty(RouterConstants.CONFIG_EXPORT_BASE_DIRS, permittedExportBaseDirs); configSharingService.setProperty(RouterConstants.KEY_HISTORY_SIZE, execHistorySize); initCamel(); diff --git a/extensions/router/router-rest/pom.xml b/extensions/router/router-rest/pom.xml index 246deccc7..331716869 100644 --- a/extensions/router/router-rest/pom.xml +++ b/extensions/router/router-rest/pom.xml @@ -104,6 +104,13 @@ <artifactId>slf4j-api</artifactId> <scope>provided</scope> </dependency> + + <!-- tests --> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <scope>test</scope> + </dependency> </dependencies> <build> diff --git a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java index 7d180ee49..207aa4d7c 100644 --- a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java +++ b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java @@ -16,10 +16,14 @@ */ package org.apache.unomi.router.rest; +import org.apache.unomi.api.services.ConfigSharingService; +import org.apache.unomi.router.api.EndpointValidator; +import org.apache.unomi.router.api.RouterConstants; import org.apache.unomi.router.api.services.ImportExportConfigurationService; import javax.ws.rs.*; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; import java.util.List; /** @@ -29,6 +33,30 @@ public abstract class AbstractConfigurationServiceEndpoint<T> { protected ImportExportConfigurationService<T> configurationService; + protected ConfigSharingService configSharingService; + + /** + * Refuses the configuration when the endpoint it names cannot be honoured -- an unsupported scheme, + * or a file path outside the directories the deployment permits. + * + * <p>The route that would carry the configuration is built asynchronously, long after this call has + * answered, so a configuration refused there would be stored and answered {@code 200} with nothing + * but a log line to show for it. Refusing here gives the caller the reason while it can still act + * on it, and keeps the configuration out of the store. + * + * @param endpointUri the endpoint URI the configuration names + * @param permittedBaseDirsProperty the shared property holding the base directories for this direction + */ + protected void refuseIfEndpointCannotBeHonoured(String endpointUri, String permittedBaseDirsProperty) { + String refusal = EndpointValidator.validate(endpointUri, + (String) configSharingService.getProperty(RouterConstants.CONFIG_ALLOWED_ENDPOINTS), + (String) configSharingService.getProperty(permittedBaseDirsProperty)); + if (refusal != null) { + throw new BadRequestException(refusal, Response.status(Response.Status.BAD_REQUEST) + .type(MediaType.TEXT_PLAIN).entity(refusal).build()); + } + } + /** * Retrieves all the configurations. * diff --git a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ExportConfigurationServiceEndPoint.java b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ExportConfigurationServiceEndPoint.java index 317345209..a3e83b4fd 100644 --- a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ExportConfigurationServiceEndPoint.java +++ b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ExportConfigurationServiceEndPoint.java @@ -17,8 +17,10 @@ package org.apache.unomi.router.rest; import org.apache.cxf.rs.security.cors.CrossOriginResourceSharing; +import org.apache.unomi.api.services.ConfigSharingService; import org.apache.unomi.api.services.ProfileService; import org.apache.unomi.router.api.ExportConfiguration; +import org.apache.unomi.router.api.RouterConstants; import org.apache.unomi.router.api.services.ImportExportConfigurationService; import org.apache.unomi.router.api.services.ProfileExportService; import org.osgi.service.component.annotations.Component; @@ -66,6 +68,11 @@ public class ExportConfigurationServiceEndPoint extends AbstractConfigurationSer configurationService = exportConfigurationService; } + @Reference + public void setConfigSharingService(ConfigSharingService configSharingService) { + this.configSharingService = configSharingService; + } + public void setProfileExportService(ProfileExportService profileExportService) { this.profileExportService = profileExportService; } @@ -81,9 +88,12 @@ public class ExportConfigurationServiceEndPoint extends AbstractConfigurationSer */ @Override public ExportConfiguration saveConfiguration(ExportConfiguration exportConfiguration) { - ExportConfiguration exportConfigSaved = configurationService.save(exportConfiguration, true); + if (RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT.equals(exportConfiguration.getConfigType())) { + refuseIfEndpointCannotBeHonoured((String) exportConfiguration.getProperties().get("destination"), + RouterConstants.CONFIG_EXPORT_BASE_DIRS); + } - return exportConfigSaved; + return configurationService.save(exportConfiguration, true); } @Override diff --git a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ImportConfigurationServiceEndPoint.java b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ImportConfigurationServiceEndPoint.java index fa87f47ad..898f68688 100644 --- a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ImportConfigurationServiceEndPoint.java +++ b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ImportConfigurationServiceEndPoint.java @@ -58,8 +58,6 @@ public class ImportConfigurationServiceEndPoint extends AbstractConfigurationSer private static final Logger LOGGER = LoggerFactory.getLogger(ImportConfigurationServiceEndPoint.class.getName()); @Reference - protected ConfigSharingService configSharingService; - public void setConfigSharingService(ConfigSharingService configSharingService) { this.configSharingService = configSharingService; } @@ -80,10 +78,12 @@ public class ImportConfigurationServiceEndPoint extends AbstractConfigurationSer */ @Override public ImportConfiguration saveConfiguration(ImportConfiguration importConfiguration) { + if (RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT.equals(importConfiguration.getConfigType())) { + refuseIfEndpointCannotBeHonoured((String) importConfiguration.getProperties().get("source"), + RouterConstants.CONFIG_IMPORT_BASE_DIRS); + } - ImportConfiguration importConfigSaved = configurationService.save(importConfiguration, true); - - return importConfigSaved; + return configurationService.save(importConfiguration, true); } @Override diff --git a/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java b/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java new file mode 100644 index 000000000..3058e9c30 --- /dev/null +++ b/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java @@ -0,0 +1,265 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.router.rest; + +import org.apache.unomi.api.services.ConfigSharingService; +import org.apache.unomi.router.api.ExportConfiguration; +import org.apache.unomi.router.api.ImportConfiguration; +import org.apache.unomi.router.api.RouterConstants; +import org.apache.unomi.router.api.services.ImportExportConfigurationService; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import javax.ws.rs.WebApplicationException; +import java.io.File; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * A configuration whose endpoint cannot be honoured must be refused when it is saved, not silently + * accepted and then dropped when its route fails to build. + * + * <p>Route construction happens asynchronously, well after the REST call has answered, so a + * configuration that only fails there is stored, answered {@code 200}, and leaves nothing but a log + * line behind — the caller cannot tell it apart from a configuration that works. Validating at save + * time gives the caller a synchronous, actionable answer, and keeps the rejected configuration out + * of the store. + * + * <p>Only configurations that name an endpoint are concerned: a oneshot import carries no source, its + * file being uploaded separately, and must keep being saved. + */ +public class ConfigurationEndpointValidationTest { + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + private File permittedImportDir; + private File permittedExportDir; + private File arbitraryDir; + + private InMemoryConfigurationService<ImportConfiguration> importConfigurations; + private InMemoryConfigurationService<ExportConfiguration> exportConfigurations; + + private ImportConfigurationServiceEndPoint importEndpoint; + private ExportConfigurationServiceEndPoint exportEndpoint; + + @Before + public void setUp() throws Exception { + permittedImportDir = tmp.newFolder("permitted-import"); + permittedExportDir = tmp.newFolder("permitted-export"); + arbitraryDir = tmp.newFolder("arbitrary"); + + InMemoryConfigSharingService configSharingService = new InMemoryConfigSharingService(); + configSharingService.setProperty(RouterConstants.CONFIG_ALLOWED_ENDPOINTS, "file,ftp,sftp,ftps"); + configSharingService.setProperty(RouterConstants.CONFIG_IMPORT_BASE_DIRS, permittedImportDir.getAbsolutePath()); + configSharingService.setProperty(RouterConstants.CONFIG_EXPORT_BASE_DIRS, permittedExportDir.getAbsolutePath()); + + importConfigurations = new InMemoryConfigurationService<>(); + importEndpoint = new ImportConfigurationServiceEndPoint(); + importEndpoint.setImportConfigurationService(importConfigurations); + importEndpoint.setConfigSharingService(configSharingService); + + exportConfigurations = new InMemoryConfigurationService<>(); + exportEndpoint = new ExportConfigurationServiceEndPoint(); + exportEndpoint.setExportConfigurationService(exportConfigurations); + exportEndpoint.setConfigSharingService(configSharingService); + } + + @Test + public void savingARecurrentImportWhoseSourceIsInsideThePermittedBaseDirsStoresIt() { + ImportConfiguration saved = importEndpoint.saveConfiguration( + recurrentImport(fileUri(permittedImportDir, "?fileName=profiles.csv"))); + + assertEquals("in-bounds", saved.getItemId()); + assertTrue("the configuration should have been stored", importConfigurations.contains("in-bounds")); + } + + @Test + public void savingARecurrentImportWhoseSourceIsOutsideThePermittedBaseDirsIsRefused() { + ImportConfiguration configuration = recurrentImport(fileUri(arbitraryDir, "?fileName=profiles.csv")); + + assertRefused(() -> importEndpoint.saveConfiguration(configuration)); + assertFalse("a refused configuration must not be stored", importConfigurations.contains("in-bounds")); + } + + @Test + public void savingARecurrentImportWhoseFileNameOptionEscapesThePermittedBaseDirsIsRefused() { + ImportConfiguration configuration = recurrentImport( + fileUri(permittedImportDir, "?fileName=../" + arbitraryDir.getName() + "/profiles.csv")); + + assertRefused(() -> importEndpoint.saveConfiguration(configuration)); + } + + @Test + public void savingAOneshotImportThatCarriesNoSourceStoresIt() { + ImportConfiguration configuration = new ImportConfiguration(); + configuration.setItemId("oneshot"); + configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_ONESHOT); + configuration.getProperties().put("mapping", Collections.singletonMap("email", 0)); + + importEndpoint.saveConfiguration(configuration); + + assertTrue("a oneshot import names no endpoint and must keep being stored", + importConfigurations.contains("oneshot")); + } + + @Test + public void savingARecurrentExportWhoseDestinationIsInsideThePermittedBaseDirsStoresIt() { + ExportConfiguration saved = exportEndpoint.saveConfiguration( + recurrentExport(fileUri(permittedExportDir, "?fileName=profiles.csv"))); + + assertEquals("in-bounds", saved.getItemId()); + assertTrue("the configuration should have been stored", exportConfigurations.contains("in-bounds")); + } + + @Test + public void savingARecurrentExportWhoseDestinationIsOutsideThePermittedBaseDirsIsRefused() { + ExportConfiguration configuration = recurrentExport(fileUri(arbitraryDir, "?fileName=profiles.csv")); + + assertRefused(() -> exportEndpoint.saveConfiguration(configuration)); + assertFalse("a refused configuration must not be stored", exportConfigurations.contains("in-bounds")); + } + + // --------------------------------------------------------------------------------------------- + // Fixtures + // --------------------------------------------------------------------------------------------- + + /** + * A refused configuration answers {@code 400 Bad Request}, and says why: the caller has to be able + * to correct the endpoint from the answer alone. + */ + private void assertRefused(Runnable save) { + try { + save.run(); + fail("saving the configuration should have been refused"); + } catch (WebApplicationException e) { + assertEquals("a refused configuration is a bad request", 400, e.getResponse().getStatus()); + assertTrue("the refusal must say why", e.getMessage() != null && !e.getMessage().trim().isEmpty()); + } + } + + private String fileUri(File directory, String suffix) { + return "file://" + directory.getAbsolutePath() + suffix; + } + + private ImportConfiguration recurrentImport(String source) { + ImportConfiguration configuration = new ImportConfiguration(); + configuration.setItemId("in-bounds"); + configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT); + configuration.getProperties().put("source", source); + configuration.getProperties().put("mapping", Collections.singletonMap("email", 0)); + return configuration; + } + + private ExportConfiguration recurrentExport(String destination) { + ExportConfiguration configuration = new ExportConfiguration(); + configuration.setItemId("in-bounds"); + configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT); + configuration.getProperties().put("destination", destination); + configuration.getProperties().put("mapping", Collections.singletonMap("0", "firstName")); + configuration.getProperties().put("segment", "exportSegment"); + configuration.getProperties().put("period", "1m"); + return configuration; + } + + /** + * Stores what it is given, so that a test can tell a configuration that was persisted from one that + * was refused before reaching the store. + */ + private static final class InMemoryConfigurationService<T> implements ImportExportConfigurationService<T> { + + private final Map<String, T> stored = new LinkedHashMap<>(); + + boolean contains(String configId) { + return stored.containsKey(configId); + } + + @Override + public List<T> getAll() { + return new ArrayList<>(stored.values()); + } + + @Override + public T load(String configId) { + return stored.get(configId); + } + + @Override + public T save(T configuration, boolean updateRunningRoute) { + stored.put(itemIdOf(configuration), configuration); + return configuration; + } + + @Override + public void delete(String configId) { + stored.remove(configId); + } + + @Override + public Map<String, RouterConstants.CONFIG_CAMEL_REFRESH> consumeConfigsToBeRefresh() { + return Collections.emptyMap(); + } + + private String itemIdOf(T configuration) { + return configuration instanceof ImportConfiguration + ? ((ImportConfiguration) configuration).getItemId() + : ((ExportConfiguration) configuration).getItemId(); + } + } + + private static final class InMemoryConfigSharingService implements ConfigSharingService { + + private final Map<String, Object> properties = new HashMap<>(); + + @Override + public Object getProperty(String name) { + return properties.get(name); + } + + @Override + public Object setProperty(String name, Object value) { + return properties.put(name, value); + } + + @Override + public boolean hasProperty(String name) { + return properties.containsKey(name); + } + + @Override + public Object removeProperty(String name) { + return properties.remove(name); + } + + @Override + public Set<String> getPropertyNames() { + return properties.keySet(); + } + } +}
