From 173970ea3184fafa87fc90824678bed01265f515 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Blanchard?= Date: Tue, 11 Aug 2026 15:03:33 +0200 Subject: [PATCH 1/7] UNOMI-973: Add route-construction tests for file endpoint containment A recurrent import configuration names a source, and a recurrent export configuration names a destination; both are used as Camel endpoint URIs. When the scheme is file, the URI must resolve inside a base directory the deployment permits -- the directory it names, and every path-bearing option it carries (fileName, move, moveFailed, preMove, doneFileName, include). FileEndpointContainmentTest covers route construction for both directions: in-bounds sources and destinations keep building routes, at any depth under the base directory and whether or not the directory exists yet; out-of-bounds ones build none. Remote schemes carry no local path and stay unaffected. Carry the permitted base directories into the route builders, separately for each direction, so a route builder cannot be wired with the wrong list. They are not read yet, so the containment tests fail. Two more fail on their own ground: a source without a scheme, and a blank destination, each raise StringIndexOutOfBoundsException out of configure() and cost the deployment the other routes of the same batch. --- extensions/router/router-core/pom.xml | 7 + .../ProfileExportCollectRouteBuilder.java | 9 + .../ProfileImportFromSourceRouteBuilder.java | 9 + .../route/RouterAbstractRouteBuilder.java | 1 + .../route/FileEndpointContainmentTest.java | 373 ++++++++++++++++++ 5 files changed, 399 insertions(+) create mode 100644 extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java diff --git a/extensions/router/router-core/pom.xml b/extensions/router/router-core/pom.xml index abaecd0387..efb1f5fed8 100644 --- a/extensions/router/router-core/pom.xml +++ b/extensions/router/router-core/pom.xml @@ -148,6 +148,13 @@ slf4j-api provided + + + + junit + junit + test + diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java index 5529c109b8..a9b8ec5bc4 100644 --- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java +++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java @@ -93,6 +93,15 @@ public void configure() throws Exception { } } + /** + * Sets the comma-separated list of base directories an export {@code file} endpoint may resolve into. + * + * @param permittedExportBaseDirs the permitted base directories + */ + public void setPermittedExportBaseDirs(String permittedExportBaseDirs) { + this.permittedBaseDirs = permittedExportBaseDirs; + } + public void setExportConfigurationList(List exportConfigurationList) { this.exportConfigurationList = exportConfigurationList; } diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java index 1ebc5c3884..d9e41ce3a2 100644 --- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java +++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java @@ -132,6 +132,15 @@ public void process(Exchange exchange) throws Exception { } } + /** + * Sets the comma-separated list of base directories an import {@code file} endpoint may resolve into. + * + * @param permittedImportBaseDirs the permitted base directories + */ + public void setPermittedImportBaseDirs(String permittedImportBaseDirs) { + this.permittedBaseDirs = permittedImportBaseDirs; + } + public void setImportConfigurationList(List importConfigurationList) { this.importConfigurationList = importConfigurationList; } diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java index ad06a00ecb..00ae702044 100644 --- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java +++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java @@ -45,6 +45,7 @@ public abstract class RouterAbstractRouteBuilder extends RouteBuilder { protected String configType; protected String allowedEndpoints; + protected String permittedBaseDirs; protected ProfileService profileService; diff --git a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java new file mode 100644 index 0000000000..a771e64398 --- /dev/null +++ b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java @@ -0,0 +1,373 @@ +/* + * 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.core.route; + +import org.apache.camel.component.jackson.JacksonDataFormat; +import org.apache.camel.impl.DefaultCamelContext; +import org.apache.unomi.api.services.ProfileService; +import org.apache.unomi.router.api.ExportConfiguration; +import org.apache.unomi.router.api.ImportConfiguration; +import org.apache.unomi.router.api.ProfileToImport; +import org.apache.unomi.router.api.RouterConstants; +import org.junit.After; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; +import java.lang.reflect.Proxy; +import java.nio.file.Files; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +/** + * A recurrent import configuration names a {@code source}, and a recurrent export configuration + * names a {@code destination}. Both are used as Apache Camel endpoint URIs. When the URI uses the + * {@code file} scheme, it must resolve inside one of the base directories the deployment + * permits: a {@code file} endpoint that resolves anywhere else is refused, and its route is not + * built. + * + *

Containment covers the directory named by the URI and every path-bearing option it + * carries ({@code fileName}, {@code move}, {@code moveFailed}, {@code preMove}, {@code doneFileName}, + * {@code include}, ...) — validating only the directory part would leave + * {@code file:///permitted/?fileName=../../elsewhere} open. + * + *

Remote schemes ({@code ftp}, {@code sftp}, {@code ftps}) carry no local path and are not + * subject to directory containment; the scheme allow-list keeps governing them. + * + *

These tests exercise route construction, which is where a configuration is turned into + * a live route: a configuration whose endpoint is refused must leave no route behind, whether it + * arrives through the REST API or was already persisted before the deployment was configured. + */ +public class FileEndpointContainmentTest { + + /** The shipped default. The containment rules must hold while {@code file} is an allowed scheme. */ + private static final String DEFAULT_ALLOWED_ENDPOINTS = "file,ftp,sftp,ftps"; + + private static final Map NO_KAFKA = new HashMap<>(); + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + private DefaultCamelContext camelContext; + + /** The base directory recurrent imports are confined to. */ + private File permittedImportDir; + + /** The base directory recurrent exports are confined to. */ + private File permittedExportDir; + + /** A directory the deployment never permitted — the "arbitrary directory" of the defect. */ + private File arbitraryDir; + + /** A sibling whose path shares a textual prefix with the permitted import directory. */ + private File permittedImportDirLookalike; + + @Before + public void setUp() throws Exception { + permittedImportDir = tmp.newFolder("permitted-import"); + permittedExportDir = tmp.newFolder("permitted-export"); + arbitraryDir = tmp.newFolder("arbitrary"); + permittedImportDirLookalike = tmp.newFolder("permitted-import-evil"); + camelContext = new DefaultCamelContext(); + } + + @After + public void tearDown() throws Exception { + camelContext.stop(); + } + + // --------------------------------------------------------------------------------------------- + // Import — the configured source is read by Unomi + // --------------------------------------------------------------------------------------------- + + @Test + public void importRouteIsBuiltWhenSourceIsInsidePermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("in-bounds", fileUri(permittedImportDir, "?fileName=profiles.csv"))); + + assertRouteBuilt("in-bounds", "a source inside the permitted base directory is legitimate"); + } + + @Test + public void importRouteIsBuiltWhenSourceIsInAnExistingSubdirectoryOfPermittedBaseDir() throws Exception { + File dropDir = new File(permittedImportDir, "incoming"); + assertTrue("could not prepare the test fixture", dropDir.mkdir()); + + addImportRoutes(recurrentImport("subdirectory", fileUri(dropDir, "?fileName=profiles.csv"))); + + assertRouteBuilt("subdirectory", "containment is recursive — a source at any depth under the base directory is legitimate"); + } + + @Test + public void importRouteIsRefusedWhenSourceIsASymlinkPointingOutsidePermittedBaseDir() throws Exception { + File link = new File(permittedImportDir, "elsewhere"); + try { + Files.createSymbolicLink(link.toPath(), arbitraryDir.toPath()); + } catch (IOException | UnsupportedOperationException e) { + Assume.assumeNoException("this file system does not support symbolic links", e); + } + + addImportRoutes(recurrentImport("symlink", fileUri(link, "?fileName=profiles.csv"))); + + assertRouteRefused("symlink", "the source leaves the permitted base directory once symbolic links are resolved"); + } + + @Test + public void importRouteIsRefusedWhenSourceIsOutsidePermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("arbitrary-dir", fileUri(arbitraryDir, "?fileName=profiles.csv"))); + + assertRouteRefused("arbitrary-dir", "the source directory is not one the deployment permits"); + } + + @Test + public void importRouteIsRefusedWhenSourceEscapesPermittedBaseDirWithParentSegments() throws Exception { + String escaping = fileUri(permittedImportDir, "/../" + arbitraryDir.getName() + "?fileName=profiles.csv"); + + addImportRoutes(recurrentImport("dot-dot", escaping)); + + assertRouteRefused("dot-dot", "the source resolves outside the permitted base directory once normalized"); + } + + @Test + public void importRouteIsRefusedWhenSourceEscapesPermittedBaseDirWithEncodedParentSegments() throws Exception { + String escaping = fileUri(permittedImportDir, "/%2e%2e/" + arbitraryDir.getName() + "?fileName=profiles.csv"); + + addImportRoutes(recurrentImport("encoded-dot-dot", escaping)); + + assertRouteRefused("encoded-dot-dot", "percent-encoded parent segments must be decoded before containment is decided"); + } + + @Test + public void importRouteIsRefusedWhenSourceDirectoryOnlySharesAPrefixWithPermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("lookalike", fileUri(permittedImportDirLookalike, "?fileName=profiles.csv"))); + + assertRouteRefused("lookalike", "containment compares path components, not string prefixes"); + } + + @Test + public void importRouteIsRefusedWhenFileNameOptionEscapesPermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("filename-escape", + fileUri(permittedImportDir, "?fileName=../" + arbitraryDir.getName() + "/profiles.csv"))); + + assertRouteRefused("filename-escape", "fileName carries a path and must be contained too"); + } + + @Test + public void importRouteIsRefusedWhenMoveOptionEscapesPermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("move-escape", + fileUri(permittedImportDir, "?fileName=profiles.csv&move=../" + arbitraryDir.getName()))); + + assertRouteRefused("move-escape", "move carries a path and must be contained too"); + } + + @Test + public void importRouteIsBuiltWhenMoveOptionIsRelativeAndStaysInsidePermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("relative-move", + fileUri(permittedImportDir, "?fileName=profiles.csv&move=.done"))); + + assertRouteBuilt("relative-move", "a relative move target inside the base directory is how the feature is normally used"); + } + + @Test + public void remoteImportEndpointIsNotSubjectToDirectoryContainment() throws Exception { + addImportRoutes(recurrentImport("remote", "ftp://ftp.example.com/profiles?fileName=profiles.csv")); + + assertRouteBuilt("remote", "ftp is an allowed scheme and carries no local path"); + } + + @Test + public void importRouteIsRefusedWhenSchemeIsNotAllowed() throws Exception { + addImportRoutes("ftp,sftp,ftps", recurrentImport("scheme-denied", fileUri(permittedImportDir, "?fileName=profiles.csv"))); + + assertRouteRefused("scheme-denied", "file is not in the configured scheme allow-list"); + } + + @Test + public void importRouteIsRefusedWhenSchemeIsOnlyASubstringOfAnAllowedScheme() throws Exception { + addImportRoutes(recurrentImport("substring-scheme", "fil://" + permittedImportDir.getAbsolutePath() + "?fileName=profiles.csv")); + + assertRouteRefused("substring-scheme", "the allow-list is a set of schemes, not a string to search"); + } + + @Test + public void malformedSourceIsSkippedWithoutPreventingTheOtherRoutesFromBeingBuilt() throws Exception { + addImportRoutes( + recurrentImport("no-scheme", permittedImportDir.getAbsolutePath() + "/profiles.csv"), + recurrentImport("well-formed", fileUri(permittedImportDir, "?fileName=profiles.csv"))); + + assertRouteRefused("no-scheme", "an endpoint without a scheme cannot be honoured"); + assertRouteBuilt("well-formed", "one malformed configuration must not cost the deployment its other routes"); + } + + // --------------------------------------------------------------------------------------------- + // Export — the configured destination is written by Unomi + // --------------------------------------------------------------------------------------------- + + @Test + public void exportRouteIsBuiltWhenDestinationIsInsidePermittedBaseDir() throws Exception { + addExportRoutes(recurrentExport("in-bounds", fileUri(permittedExportDir, "?fileName=profiles.csv"))); + + assertRouteBuilt("in-bounds", "a destination inside the permitted base directory is legitimate"); + } + + @Test + public void exportRouteIsBuiltWhenDestinationIsInASubdirectoryThatDoesNotExistYet() throws Exception { + File notCreatedYet = new File(permittedExportDir, "2026-08"); + + addExportRoutes(recurrentExport("subdirectory", fileUri(notCreatedYet, "?fileName=profiles.csv"))); + + assertRouteBuilt("subdirectory", "an export destination is created on first write — containment must not require the directory to exist"); + } + + @Test + public void exportRouteIsRefusedWhenDestinationIsOutsidePermittedBaseDir() throws Exception { + addExportRoutes(recurrentExport("arbitrary-dir", fileUri(arbitraryDir, "?fileName=profiles.csv"))); + + assertRouteRefused("arbitrary-dir", "the destination directory is not one the deployment permits"); + } + + @Test + public void exportRouteIsRefusedWhenDestinationEscapesPermittedBaseDirWithParentSegments() throws Exception { + String escaping = fileUri(permittedExportDir, "/../" + arbitraryDir.getName() + "?fileName=profiles.csv"); + + addExportRoutes(recurrentExport("dot-dot", escaping)); + + assertRouteRefused("dot-dot", "the destination resolves outside the permitted base directory once normalized"); + } + + @Test + public void exportRouteIsRefusedWhenFileNameOptionEscapesPermittedBaseDir() throws Exception { + addExportRoutes(recurrentExport("filename-escape", + fileUri(permittedExportDir, "?fileName=../" + arbitraryDir.getName() + "/profiles.csv"))); + + assertRouteRefused("filename-escape", "fileName carries a path and must be contained too"); + } + + @Test + public void remoteExportEndpointIsNotSubjectToDirectoryContainment() throws Exception { + addExportRoutes(recurrentExport("remote", "ftp://ftp.example.com/profiles?fileName=profiles.csv")); + + assertRouteBuilt("remote", "ftp is an allowed scheme and carries no local path"); + } + + @Test + public void malformedDestinationIsSkippedWithoutPreventingTheOtherRoutesFromBeingBuilt() throws Exception { + addExportRoutes( + recurrentExport("blank", ""), + recurrentExport("well-formed", fileUri(permittedExportDir, "?fileName=profiles.csv"))); + + assertRouteRefused("blank", "a blank destination cannot be honoured"); + assertRouteBuilt("well-formed", "one malformed configuration must not cost the deployment its other routes"); + } + + // --------------------------------------------------------------------------------------------- + // Fixtures + // --------------------------------------------------------------------------------------------- + + private void assertRouteBuilt(String routeId, String why) { + assertNotNull("no route was built for configuration '" + routeId + "', although " + why, + camelContext.getRouteDefinition(routeId)); + } + + private void assertRouteRefused(String routeId, String why) { + assertNull("a route was built for configuration '" + routeId + "', although " + why, + camelContext.getRouteDefinition(routeId)); + } + + private String fileUri(File directory, String suffix) { + return "file://" + directory.getAbsolutePath() + suffix; + } + + private ImportConfiguration recurrentImport(String itemId, String source) { + ImportConfiguration configuration = new ImportConfiguration(); + configuration.setItemId(itemId); + configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT); + configuration.setActive(true); + configuration.getProperties().put("source", source); + configuration.getProperties().put("mapping", Collections.singletonMap("0", 0)); + return configuration; + } + + private ExportConfiguration recurrentExport(String itemId, String destination) { + ExportConfiguration configuration = new ExportConfiguration(); + configuration.setItemId(itemId); + configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT); + configuration.setActive(true); + 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; + } + + private void addImportRoutes(ImportConfiguration... configurations) throws Exception { + addImportRoutes(DEFAULT_ALLOWED_ENDPOINTS, configurations); + } + + private void addImportRoutes(String allowedEndpoints, ImportConfiguration... configurations) throws Exception { + ProfileImportFromSourceRouteBuilder builder = + new ProfileImportFromSourceRouteBuilder(NO_KAFKA, RouterConstants.CONFIG_TYPE_NOBROKER); + builder.setImportConfigurationList(Arrays.asList(configurations)); + builder.setProfileService(noOpProfileService()); + builder.setJacksonDataFormat(new JacksonDataFormat(ProfileToImport.class)); + builder.setAllowedEndpoints(allowedEndpoints); + builder.setPermittedImportBaseDirs(permittedImportDir.getAbsolutePath()); + builder.setContext(camelContext); + camelContext.addRoutes(builder); + } + + private void addExportRoutes(ExportConfiguration... configurations) throws Exception { + ProfileExportCollectRouteBuilder builder = + new ProfileExportCollectRouteBuilder(NO_KAFKA, RouterConstants.CONFIG_TYPE_NOBROKER); + builder.setExportConfigurationList(Arrays.asList(configurations)); + builder.setJacksonDataFormat(new JacksonDataFormat(ProfileToImport.class)); + builder.setAllowedEndpoints(DEFAULT_ALLOWED_ENDPOINTS); + builder.setPermittedExportBaseDirs(permittedExportDir.getAbsolutePath()); + builder.setContext(camelContext); + camelContext.addRoutes(builder); + } + + /** + * The route builders ask the profile service for the profile property types while they build. + * Nothing in these tests depends on what it answers. + */ + private static ProfileService noOpProfileService() { + return (ProfileService) Proxy.newProxyInstance( + ProfileService.class.getClassLoader(), + new Class[]{ProfileService.class}, + (proxy, method, args) -> { + if (Collection.class.isAssignableFrom(method.getReturnType())) { + return Collections.emptyList(); + } + if (List.class.isAssignableFrom(method.getReturnType())) { + return Collections.emptyList(); + } + return null; + }); + } +} From c0ecaa7fc90f1fdca0902604dd1bb4f8c0c4c601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Blanchard?= Date: Tue, 11 Aug 2026 15:11:17 +0200 Subject: [PATCH 2/7] UNOMI-973: Confine file import/export endpoints to permitted base directories EndpointValidator decides whether a configured endpoint URI may be used: its scheme must belong to the allow-list, and a file endpoint must resolve inside one of the base directories the deployment permits. Containment covers the directory the URI names and every path-bearing option it carries, and is decided on canonical paths -- percent-encoding decoded, RAW() unwrapped, parent segments resolved, symbolic links followed -- compared component by component, so a sibling sharing a textual prefix is not taken for a child. It is recursive and does not require the directory to exist, since an export destination is created on first write. Two new settings carry the base directories, one per direction, defaulting under karaf.data: an export cannot then write into a directory an import route polls. Route builders receive them through direction-specific setters, so neither can be wired with the other's list. The scheme test now matches whole schemes rather than searching the raw setting, and an endpoint with no scheme, or a blank destination, is reported and skipped instead of raising StringIndexOutOfBoundsException out of configure() -- which cost the deployment every other route of the batch. The oneshot upload route builds its own endpoint and is unaffected. Remote schemes carry no local path and stay governed by the scheme allow-list alone. Recurrent file configurations resolving outside the permitted directories stop building routes, which is a behaviour change for existing deployments. --- .../unomi/router/api/EndpointValidator.java | 249 ++++++++++++++++++ .../core/context/RouterCamelContext.java | 14 + .../ProfileExportCollectRouteBuilder.java | 7 +- .../ProfileImportFromSourceRouteBuilder.java | 10 +- .../OSGI-INF/blueprint/blueprint.xml | 4 + .../resources/org.apache.unomi.router.cfg | 9 +- .../route/FileEndpointContainmentTest.java | 49 ++++ .../apache/unomi/itests/ProfileExportIT.java | 4 +- .../resources/org.apache.unomi.router.cfg | 6 +- 9 files changed, 342 insertions(+), 10 deletions(-) create mode 100644 extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java diff --git a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java new file mode 100644 index 0000000000..1a18a0e89a --- /dev/null +++ b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java @@ -0,0 +1,249 @@ +/* + * 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.api; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +/** + * Decides whether the endpoint URI carried by an import or export configuration may be used. + * + *

Two rules apply. The scheme must belong to the configured allow-list. And a {@code file} + * endpoint must resolve inside one of the base directories the deployment permits — the directory + * the URI names, and every path-bearing option it carries, since validating only the directory would + * leave {@code file:///permitted/?fileName=../../elsewhere} open. + * + *

Containment is recursive: any depth under a permitted base directory is accepted, whether or + * not the directory exists yet. It is decided on canonical paths — percent-encoding decoded, parent + * segments resolved, symbolic links followed — and compared component by component, so a sibling + * that merely shares a textual prefix with a permitted directory is not mistaken for one of its + * children. + * + *

Schemes other than {@code file} carry no local path and are left to the scheme allow-list. + */ +public final class EndpointValidator { + + public static final String FILE_SCHEME = "file"; + + /** + * The Camel file endpoint options whose value is, or contains, a path. Compared in lower case. + */ + private static final Set PATH_BEARING_OPTIONS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( + "filename", "tempfilename", "move", "movefailed", "premove", "donefilename", + "include", "antinclude", "antfilter"))); + + private EndpointValidator() { + } + + /** + * Validates the endpoint URI of an import or export configuration. + * + * @param endpointUri the endpoint URI, as configured + * @param allowedSchemes the comma-separated list of allowed schemes + * @param permittedBaseDirs the comma-separated list of base directories a {@code file} endpoint may + * resolve into + * @return {@code null} when the endpoint may be used, otherwise the reason it is refused + */ + public static String validate(String endpointUri, String allowedSchemes, String permittedBaseDirs) { + if (isBlank(endpointUri)) { + return "no endpoint is configured"; + } + + int schemeSeparator = endpointUri.indexOf(':'); + if (schemeSeparator <= 0) { + return "endpoint '" + endpointUri + "' has no scheme"; + } + + String scheme = endpointUri.substring(0, schemeSeparator); + if (!containsIgnoreCase(split(allowedSchemes), scheme)) { + return "endpoint scheme '" + scheme + "' is not allowed"; + } + + if (!FILE_SCHEME.equalsIgnoreCase(scheme)) { + return null; + } + + return validateContainment(endpointUri, permittedBaseDirs); + } + + private static String validateContainment(String endpointUri, String permittedBaseDirs) { + List baseDirs = new ArrayList<>(); + for (String baseDir : split(permittedBaseDirs)) { + baseDirs.add(canonicalize(Paths.get(baseDir))); + } + if (baseDirs.isEmpty()) { + return "no permitted base directory is configured for file endpoints"; + } + + int querySeparator = endpointUri.indexOf('?'); + String head = querySeparator < 0 ? endpointUri : endpointUri.substring(0, querySeparator); + String query = querySeparator < 0 ? "" : endpointUri.substring(querySeparator + 1); + + Path directory = Paths.get(decode(stripScheme(head))); + if (!isContained(directory, baseDirs)) { + return "directory '" + directory + "' is outside the permitted directories"; + } + + for (String[] parameter : parseQuery(query)) { + if (!PATH_BEARING_OPTIONS.contains(parameter[0].toLowerCase(Locale.ROOT))) { + continue; + } + String value = stripRaw(decode(parameter[1])); + if (value.isEmpty()) { + continue; + } + if (!isContained(directory.resolve(value), baseDirs)) { + return "option '" + parameter[0] + "' points outside the permitted directories"; + } + } + + return null; + } + + /** + * Removes the scheme, and the authority separator Camel tolerates in any of its forms + * ({@code file:dir}, {@code file://dir}, {@code file:///dir}). + */ + private static String stripScheme(String head) { + String path = head.substring(head.indexOf(':') + 1); + return path.startsWith("//") ? path.substring(2) : path; + } + + private static boolean isContained(Path path, List baseDirs) { + Path candidate = canonicalize(path); + for (Path baseDir : baseDirs) { + if (candidate.startsWith(baseDir)) { + return true; + } + } + return false; + } + + /** + * Resolves a path to the one the file system would actually use: made absolute, stripped of its + * parent segments, and with the symbolic links of its existing part followed. A path that does not + * exist yet is canonicalized through its deepest existing ancestor — an export destination is + * created on first write, and must be decided on before it exists. + */ + private static Path canonicalize(Path path) { + Path normalized = path.toAbsolutePath().normalize(); + Path existing = normalized; + while (existing != null && !Files.exists(existing, LinkOption.NOFOLLOW_LINKS)) { + existing = existing.getParent(); + } + if (existing == null) { + return normalized; + } + try { + return existing.toRealPath().resolve(existing.relativize(normalized)); + } catch (IOException e) { + return normalized; + } + } + + /** + * {@code RAW(...)} and {@code RAW{...}} tell Camel not to decode a value; the path it wraps is used + * as it stands. + */ + private static String stripRaw(String value) { + if (value.startsWith("RAW(") && value.endsWith(")")) { + return value.substring(4, value.length() - 1); + } + if (value.startsWith("RAW{") && value.endsWith("}")) { + return value.substring(4, value.length() - 1); + } + return value; + } + + private static List parseQuery(String query) { + List parameters = new ArrayList<>(); + for (String parameter : query.split("&")) { + if (parameter.isEmpty()) { + continue; + } + int separator = parameter.indexOf('='); + if (separator > 0) { + parameters.add(new String[]{parameter.substring(0, separator), parameter.substring(separator + 1)}); + } + } + return parameters; + } + + /** + * Decodes the percent-encoding of a URI, so that containment is decided on the path the file system + * will see. Unlike form decoding, {@code +} is left alone: it is a valid character in a file name. + */ + private static String decode(String value) { + if (value.indexOf('%') < 0) { + return value; + } + ByteArrayOutputStream decoded = new ByteArrayOutputStream(value.length()); + for (int i = 0; i < value.length(); i++) { + char character = value.charAt(i); + if (character == '%' && i + 2 < value.length()) { + int high = Character.digit(value.charAt(i + 1), 16); + int low = Character.digit(value.charAt(i + 2), 16); + if (high >= 0 && low >= 0) { + decoded.write((high << 4) + low); + i += 2; + continue; + } + } + decoded.write(character); + } + return new String(decoded.toByteArray(), StandardCharsets.UTF_8); + } + + private static List split(String commaSeparated) { + List values = new ArrayList<>(); + if (commaSeparated == null) { + return values; + } + for (String value : commaSeparated.split(",")) { + String trimmed = value.trim(); + if (!trimmed.isEmpty()) { + values.add(trimmed); + } + } + return values; + } + + private static boolean containsIgnoreCase(List values, String searched) { + for (String value : values) { + if (value.equalsIgnoreCase(searched)) { + return true; + } + } + return false; + } + + private static boolean isBlank(String value) { + return value == null || value.trim().isEmpty(); + } +} 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 4d329209d3..397e50cc13 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 @@ -71,6 +71,8 @@ public class RouterCamelContext implements IRouterCamelContext { private Map kafkaProps; private String configType; private String allowedEndpoints; + private String permittedImportBaseDirs; + private String permittedExportBaseDirs; private BundleContext bundleContext; private ConfigSharingService configSharingService; @@ -179,6 +181,7 @@ private void initCamel() throws Exception { builderReader.setImportConfigurationService(importConfigurationService); builderReader.setJacksonDataFormat(jacksonDataFormat); builderReader.setAllowedEndpoints(allowedEndpoints); + builderReader.setPermittedImportBaseDirs(permittedImportBaseDirs); builderReader.setContext(camelContext); camelContext.addRoutes(builderReader); @@ -206,6 +209,7 @@ private void initCamel() throws Exception { profileExportCollectRouteBuilder.setExportConfigurationList(exportConfigurationService.getAll()); profileExportCollectRouteBuilder.setPersistenceService(persistenceService); profileExportCollectRouteBuilder.setAllowedEndpoints(allowedEndpoints); + profileExportCollectRouteBuilder.setPermittedExportBaseDirs(permittedExportBaseDirs); profileExportCollectRouteBuilder.setJacksonDataFormat(jacksonDataFormat); profileExportCollectRouteBuilder.setContext(camelContext); camelContext.addRoutes(profileExportCollectRouteBuilder); @@ -249,6 +253,7 @@ public void updateProfileImportReaderRoute(String configId, boolean fireEvent) t builder.setImportConfigurationService(importConfigurationService); builder.setProfileService(profileService); builder.setAllowedEndpoints(allowedEndpoints); + builder.setPermittedImportBaseDirs(permittedImportBaseDirs); builder.setJacksonDataFormat(jacksonDataFormat); builder.setContext(camelContext); camelContext.addRoutes(builder); @@ -269,6 +274,7 @@ public void updateProfileExportReaderRoute(String configId, boolean fireEvent) t profileExportCollectRouteBuilder.setExportConfigurationList(Collections.singletonList(exportConfiguration)); profileExportCollectRouteBuilder.setPersistenceService(persistenceService); profileExportCollectRouteBuilder.setAllowedEndpoints(allowedEndpoints); + profileExportCollectRouteBuilder.setPermittedExportBaseDirs(permittedExportBaseDirs); profileExportCollectRouteBuilder.setJacksonDataFormat(jacksonDataFormat); profileExportCollectRouteBuilder.setContext(camelContext); camelContext.addRoutes(profileExportCollectRouteBuilder); @@ -334,4 +340,12 @@ public void setConfigType(String configType) { public void setAllowedEndpoints(String allowedEndpoints) { this.allowedEndpoints = allowedEndpoints; } + + public void setPermittedImportBaseDirs(String permittedImportBaseDirs) { + this.permittedImportBaseDirs = permittedImportBaseDirs; + } + + public void setPermittedExportBaseDirs(String permittedExportBaseDirs) { + this.permittedExportBaseDirs = permittedExportBaseDirs; + } } diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java index a9b8ec5bc4..d0f075f1c8 100644 --- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java +++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java @@ -19,8 +19,8 @@ import org.apache.camel.LoggingLevel; import org.apache.camel.component.kafka.KafkaEndpoint; import org.apache.camel.model.ProcessorDefinition; -import org.apache.commons.lang3.StringUtils; import org.apache.unomi.persistence.spi.PersistenceService; +import org.apache.unomi.router.api.EndpointValidator; import org.apache.unomi.router.api.ExportConfiguration; import org.apache.unomi.router.api.RouterConstants; import org.apache.unomi.router.core.bean.CollectProfileBean; @@ -62,7 +62,8 @@ public void configure() throws Exception { exportConfiguration.getProperties() != null && exportConfiguration.getProperties().size() > 0) { if ((Map) exportConfiguration.getProperties().get("mapping") != null) { String destinationEndpoint = (String) exportConfiguration.getProperties().get("destination"); - if (StringUtils.isNotBlank(destinationEndpoint) && allowedEndpoints.contains(destinationEndpoint.substring(0, destinationEndpoint.indexOf(':')))) { + String refusal = EndpointValidator.validate(destinationEndpoint, allowedEndpoints, permittedBaseDirs); + if (refusal == null) { String timerString = "timer://collectProfile?fixedRate=true&period=" + (String) exportConfiguration.getProperties().get("period"); if ((String) exportConfiguration.getProperties().get("delay") != null) { timerString += "&delay=" + (String) exportConfiguration.getProperties().get("delay"); @@ -82,7 +83,7 @@ public void configure() throws Exception { prDef.to((String) getEndpointURI(RouterConstants.DIRECTION_FROM, RouterConstants.DIRECT_EXPORT_DEPOSIT_BUFFER)); } } else { - LOGGER.error("Endpoint scheme {} is not allowed, route {} will be skipped.", destinationEndpoint.substring(0, destinationEndpoint.indexOf(':')), exportConfiguration.getItemId()); + LOGGER.error("Destination endpoint is refused ({}), route {} will be skipped.", refusal, exportConfiguration.getItemId()); } } else { LOGGER.warn("Mapping is null in export configuration, route {} will be skipped!", exportConfiguration.getItemId()); diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java index d9e41ce3a2..54b9109fc8 100644 --- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java +++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java @@ -23,6 +23,7 @@ import org.apache.camel.component.kafka.KafkaEndpoint; import org.apache.camel.model.ProcessorDefinition; import org.apache.commons.lang3.StringUtils; +import org.apache.unomi.router.api.EndpointValidator; import org.apache.unomi.router.api.ImportConfiguration; import org.apache.unomi.router.api.RouterConstants; import org.apache.unomi.router.api.services.ImportExportConfigurationService; @@ -92,9 +93,12 @@ public void configure() throws Exception { lineSplitProcessor.setProfilePropertyTypes(profileService.getTargetPropertyTypes("profiles")); String endpoint = (String) importConfiguration.getProperties().get("source"); - endpoint += "&moveFailed=.error"; + if (StringUtils.isNotBlank(endpoint)) { + endpoint += "&moveFailed=.error"; + } - if (StringUtils.isNotBlank(endpoint) && allowedEndpoints.contains(endpoint.substring(0, endpoint.indexOf(':')))) { + String refusal = EndpointValidator.validate(endpoint, allowedEndpoints, permittedBaseDirs); + if (refusal == null) { ProcessorDefinition prDef = from(endpoint) .routeId(importConfiguration.getItemId())// This allow identification of the route for manual start/stop .autoStartup(importConfiguration.isActive())// Auto-start if the import configuration is set active @@ -126,7 +130,7 @@ public void process(Exchange exchange) throws Exception { prDef.to((String) getEndpointURI(RouterConstants.DIRECTION_FROM, RouterConstants.DIRECT_IMPORT_DEPOSIT_BUFFER)); } } else { - LOGGER.error("Endpoint scheme {} is not allowed, route {} will be skipped.", endpoint.substring(0, endpoint.indexOf(':')), importConfiguration.getItemId()); + LOGGER.error("Source endpoint is refused ({}), route {} will be skipped.", refusal, importConfiguration.getItemId()); } } } diff --git a/extensions/router/router-core/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/extensions/router/router-core/src/main/resources/OSGI-INF/blueprint/blueprint.xml index aae3abbe2e..291e80e6b2 100644 --- a/extensions/router/router-core/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/extensions/router/router-core/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -26,6 +26,8 @@ + + @@ -84,6 +86,8 @@ init-method="init" destroy-method="destroy"> + + diff --git a/extensions/router/router-core/src/main/resources/org.apache.unomi.router.cfg b/extensions/router/router-core/src/main/resources/org.apache.unomi.router.cfg index 7a87050c6f..0f89e2d42c 100644 --- a/extensions/router/router-core/src/main/resources/org.apache.unomi.router.cfg +++ b/extensions/router/router-core/src/main/resources/org.apache.unomi.router.cfg @@ -38,4 +38,11 @@ executionsHistory.size=${org.apache.unomi.router.executionsHistory.size:-5} executions.error.report.size=${org.apache.unomi.router.executions.error.report.size:-200} #Allowed source endpoints -config.allowedEndpoints=${org.apache.unomi.router.config.allowedEndpoints:-file,ftp,sftp,ftps} \ No newline at end of file +config.allowedEndpoints=${org.apache.unomi.router.config.allowedEndpoints:-file,ftp,sftp,ftps} + +#Base directories a file endpoint may resolve into, comma-separated. A recurrent import source or +#export destination using the file scheme is refused unless it resolves inside one of them, at any +#depth. Import and export are kept apart so that an export cannot write into a directory an import +#route is polling; point them at the same directory only if that is what you mean. +config.import.baseDir=${org.apache.unomi.router.config.import.baseDir:-${karaf.data}/router/import/} +config.export.baseDir=${org.apache.unomi.router.config.export.baseDir:-${karaf.data}/router/export/} \ No newline at end of file diff --git a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java index a771e64398..e10d3b0c64 100644 --- a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java +++ b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java @@ -185,6 +185,39 @@ public void importRouteIsRefusedWhenMoveOptionEscapesPermittedBaseDir() throws E assertRouteRefused("move-escape", "move carries a path and must be contained too"); } + @Test + public void importRouteIsRefusedWhenMoveFailedOptionEscapesPermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("movefailed-escape", + fileUri(permittedImportDir, "?fileName=profiles.csv&moveFailed=../" + arbitraryDir.getName()))); + + assertRouteRefused("movefailed-escape", + "moveFailed carries a path, and the route builder appends one of its own — every occurrence must be contained"); + } + + @Test + public void importRouteIsRefusedWhenPreMoveOptionEscapesPermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("premove-escape", + fileUri(permittedImportDir, "?fileName=profiles.csv&preMove=../" + arbitraryDir.getName()))); + + assertRouteRefused("premove-escape", "preMove carries a path and must be contained too"); + } + + @Test + public void importRouteIsRefusedWhenDoneFileNameOptionEscapesPermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("donefilename-escape", + fileUri(permittedImportDir, "?fileName=profiles.csv&doneFileName=../" + arbitraryDir.getName() + "/done"))); + + assertRouteRefused("donefilename-escape", "doneFileName carries a path and must be contained too"); + } + + @Test + public void importRouteIsRefusedWhenPathBearingOptionIsWrappedInRaw() throws Exception { + addImportRoutes(recurrentImport("raw-escape", + fileUri(permittedImportDir, "?fileName=RAW(../" + arbitraryDir.getName() + "/profiles.csv)"))); + + assertRouteRefused("raw-escape", "RAW() only tells Camel not to decode the value — the path it carries is used as-is"); + } + @Test public void importRouteIsBuiltWhenMoveOptionIsRelativeAndStaysInsidePermittedBaseDir() throws Exception { addImportRoutes(recurrentImport("relative-move", @@ -268,6 +301,22 @@ public void exportRouteIsRefusedWhenFileNameOptionEscapesPermittedBaseDir() thro assertRouteRefused("filename-escape", "fileName carries a path and must be contained too"); } + @Test + public void exportRouteIsRefusedWhenTempFileNameOptionEscapesPermittedBaseDir() throws Exception { + addExportRoutes(recurrentExport("tempfilename-escape", + fileUri(permittedExportDir, "?fileName=profiles.csv&tempFileName=../" + arbitraryDir.getName() + "/profiles.tmp"))); + + assertRouteRefused("tempfilename-escape", "tempFileName carries a path and must be contained too"); + } + + @Test + public void exportRouteIsRefusedWhenDoneFileNameOptionEscapesPermittedBaseDir() throws Exception { + addExportRoutes(recurrentExport("donefilename-escape", + fileUri(permittedExportDir, "?fileName=profiles.csv&doneFileName=../" + arbitraryDir.getName() + "/done"))); + + assertRouteRefused("donefilename-escape", "doneFileName carries a path and must be contained too"); + } + @Test public void remoteExportEndpointIsNotSubjectToDirectoryContainment() throws Exception { addExportRoutes(recurrentExport("remote", "ftp://ftp.example.com/profiles?fileName=profiles.csv")); diff --git a/itests/src/test/java/org/apache/unomi/itests/ProfileExportIT.java b/itests/src/test/java/org/apache/unomi/itests/ProfileExportIT.java index 2b52d53d14..e767a73bae 100644 --- a/itests/src/test/java/org/apache/unomi/itests/ProfileExportIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/ProfileExportIT.java @@ -99,13 +99,13 @@ public void testExport() throws InterruptedException { exportConfiguration.getProperties().put("mapping", mapping); exportConfiguration.getProperties().put("segment", "exportItSeg"); exportConfiguration.getProperties().put("period", "1m"); - File exportDir = new File("data/tmp/"); + File exportDir = new File("data/tmp/recurrent_export/"); exportConfiguration.getProperties().put("destination", "file://" + exportDir.getAbsolutePath() + "?fileName=profiles-export.csv"); exportConfiguration.setActive(true); exportConfigurationService.save(exportConfiguration, true); - final File exportResult = new File("data/tmp/profiles-export.csv"); + final File exportResult = new File("data/tmp/recurrent_export/profiles-export.csv"); keepTrying("Failed waiting for export file to be created", () -> exportResult, File::exists, 1000, 100); logger.info("PATH : {}", exportResult.getAbsolutePath()); diff --git a/itests/src/test/resources/org.apache.unomi.router.cfg b/itests/src/test/resources/org.apache.unomi.router.cfg index 6d5f985bb9..d0e5c3753e 100644 --- a/itests/src/test/resources/org.apache.unomi.router.cfg +++ b/itests/src/test/resources/org.apache.unomi.router.cfg @@ -40,4 +40,8 @@ executionsHistory.size=5 executions.error.report.size=200 #Allowed source endpoints -config.allowedEndpoints=file,ftp,sftp,ftps \ No newline at end of file +config.allowedEndpoints=file,ftp,sftp,ftps + +#Base directories a file endpoint may resolve into +config.import.baseDir=${karaf.data}/tmp/recurrent_import +config.export.baseDir=${karaf.data}/tmp/recurrent_export From 86b625641aac178a84ef209e2aea03c3a63bdb23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Blanchard?= Date: Tue, 11 Aug 2026 15:52:19 +0200 Subject: [PATCH 3/7] 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. --- .../unomi/router/api/RouterConstants.java | 4 + .../core/context/RouterCamelContext.java | 4 + extensions/router/router-rest/pom.xml | 7 + .../AbstractConfigurationServiceEndpoint.java | 28 ++ .../ExportConfigurationServiceEndPoint.java | 14 +- .../ImportConfigurationServiceEndPoint.java | 10 +- .../ConfigurationEndpointValidationTest.java | 265 ++++++++++++++++++ 7 files changed, 325 insertions(+), 7 deletions(-) create mode 100644 extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java 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 5ef19fe447..03d84bcb2d 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 @@ enum CONFIG_CAMEL_REFRESH { 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 397e50cc13..15f401880b 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 void init() throws Exception { 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 246deccc73..331716869d 100644 --- a/extensions/router/router-rest/pom.xml +++ b/extensions/router/router-rest/pom.xml @@ -104,6 +104,13 @@ slf4j-api provided + + + + junit + junit + test + 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 7d180ee495..207aa4d7cc 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 { protected ImportExportConfigurationService 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. + * + *

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 3173452093..a3e83b4fda 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 void setExportConfigurationService(ImportExportConfigurationServiceRoute 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. + * + *

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 importConfigurations; + private InMemoryConfigurationService 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 implements ImportExportConfigurationService { + + private final Map stored = new LinkedHashMap<>(); + + boolean contains(String configId) { + return stored.containsKey(configId); + } + + @Override + public List 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 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 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 getPropertyNames() { + return properties.keySet(); + } + } +} From 3c7f42b681ce68884dc0ea8696cc6754f1179232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Blanchard?= Date: Tue, 11 Aug 2026 16:20:10 +0200 Subject: [PATCH 4/7] UNOMI-973: Record on a configuration whether its endpoint can be honoured The permitted directories are an operational setting and the configurations are user data, so the two drift apart: a configuration that was legitimate when it was created is refused once the deployment is reconfigured. Refusing it silently left its owner with a configuration that looks fine and does nothing, the only trace being a log line. A configuration whose endpoint is refused while its route is built is now marked INVALID_ENDPOINT and saved, so it can be seen and dealt with. Nothing is deleted and no restart is blocked: correcting or removing it belongs to whoever owns it. Restoring the permitted directories clears the mark on its own at the next rebuild, so operations can undo a change without anyone touching the configurations. The status is its own value rather than one of the execution statuses: those report on a run that happened, this one says no run can. Keeping them apart is what makes the mark safe to clear -- the record of a run that genuinely failed is left alone. Saving is done without asking for a route refresh, which would rebuild the route, refuse it again and save it again, without end. --- .../unomi/router/api/RouterConstants.java | 7 + .../core/context/RouterCamelContext.java | 2 + .../ProfileExportCollectRouteBuilder.java | 7 + .../ProfileImportFromSourceRouteBuilder.java | 1 + .../route/RouterAbstractRouteBuilder.java | 33 ++ .../route/FileEndpointContainmentTest.java | 15 + .../route/RefusedConfigurationStatusTest.java | 305 ++++++++++++++++++ 7 files changed, 370 insertions(+) create mode 100644 extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RefusedConfigurationStatusTest.java 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 03d84bcb2d..9df7a27529 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 @@ -32,6 +32,13 @@ enum CONFIG_CAMEL_REFRESH { String CONFIG_STATUS_COMPLETE_ERRORS = "ERRORS"; String CONFIG_STATUS_COMPLETE_SUCCESS = "SUCCESS"; String CONFIG_STATUS_COMPLETE_WITH_ERRORS = "WITH_ERRORS"; + /** + * The configuration names an endpoint that cannot be honoured, so no route carries it. Kept apart + * from the execution statuses above: those report on a run that happened, this one says no run can. + * It is set and cleared by the route builders alone, so that restoring the deployment's permitted + * directories brings the configuration back on its own. + */ + String CONFIG_STATUS_INVALID_ENDPOINT = "INVALID_ENDPOINT"; String IMPORT_EXPORT_CONFIG_TYPE_RECURRENT = "recurrent"; String IMPORT_EXPORT_CONFIG_TYPE_ONESHOT = "oneshot"; 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 15f401880b..d0e8dcdae3 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 @@ -211,6 +211,7 @@ private void initCamel() throws Exception { //Profiles collect ProfileExportCollectRouteBuilder profileExportCollectRouteBuilder = new ProfileExportCollectRouteBuilder(kafkaProps, configType); profileExportCollectRouteBuilder.setExportConfigurationList(exportConfigurationService.getAll()); + profileExportCollectRouteBuilder.setExportConfigurationService(exportConfigurationService); profileExportCollectRouteBuilder.setPersistenceService(persistenceService); profileExportCollectRouteBuilder.setAllowedEndpoints(allowedEndpoints); profileExportCollectRouteBuilder.setPermittedExportBaseDirs(permittedExportBaseDirs); @@ -276,6 +277,7 @@ public void updateProfileExportReaderRoute(String configId, boolean fireEvent) t if (RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT.equals(exportConfiguration.getConfigType())) { ProfileExportCollectRouteBuilder profileExportCollectRouteBuilder = new ProfileExportCollectRouteBuilder(kafkaProps, configType); profileExportCollectRouteBuilder.setExportConfigurationList(Collections.singletonList(exportConfiguration)); + profileExportCollectRouteBuilder.setExportConfigurationService(exportConfigurationService); profileExportCollectRouteBuilder.setPersistenceService(persistenceService); profileExportCollectRouteBuilder.setAllowedEndpoints(allowedEndpoints); profileExportCollectRouteBuilder.setPermittedExportBaseDirs(permittedExportBaseDirs); diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java index d0f075f1c8..f59b46edb5 100644 --- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java +++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java @@ -23,6 +23,7 @@ import org.apache.unomi.router.api.EndpointValidator; 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.core.bean.CollectProfileBean; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -38,6 +39,7 @@ public class ProfileExportCollectRouteBuilder extends RouterAbstractRouteBuilder private static final Logger LOGGER = LoggerFactory.getLogger(ProfileExportCollectRouteBuilder.class); private List exportConfigurationList; + private ImportExportConfigurationService exportConfigurationService; private PersistenceService persistenceService; public ProfileExportCollectRouteBuilder(Map kafkaProps, String configType) { @@ -63,6 +65,7 @@ public void configure() throws Exception { if ((Map) exportConfiguration.getProperties().get("mapping") != null) { String destinationEndpoint = (String) exportConfiguration.getProperties().get("destination"); String refusal = EndpointValidator.validate(destinationEndpoint, allowedEndpoints, permittedBaseDirs); + recordEndpointOutcome(exportConfiguration, exportConfigurationService, refusal); if (refusal == null) { String timerString = "timer://collectProfile?fixedRate=true&period=" + (String) exportConfiguration.getProperties().get("period"); if ((String) exportConfiguration.getProperties().get("delay") != null) { @@ -103,6 +106,10 @@ public void setPermittedExportBaseDirs(String permittedExportBaseDirs) { this.permittedBaseDirs = permittedExportBaseDirs; } + public void setExportConfigurationService(ImportExportConfigurationService exportConfigurationService) { + this.exportConfigurationService = exportConfigurationService; + } + public void setExportConfigurationList(List exportConfigurationList) { this.exportConfigurationList = exportConfigurationList; } diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java index 54b9109fc8..4c468a6591 100644 --- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java +++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java @@ -98,6 +98,7 @@ public void configure() throws Exception { } String refusal = EndpointValidator.validate(endpoint, allowedEndpoints, permittedBaseDirs); + recordEndpointOutcome(importConfiguration, importConfigurationService, refusal); if (refusal == null) { ProcessorDefinition prDef = from(endpoint) .routeId(importConfiguration.getItemId())// This allow identification of the route for manual start/stop diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java index 00ae702044..fa49989983 100644 --- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java +++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java @@ -23,7 +23,9 @@ import org.apache.camel.component.kafka.KafkaEndpoint; import org.apache.commons.lang3.StringUtils; import org.apache.unomi.api.services.ProfileService; +import org.apache.unomi.router.api.ImportExportConfiguration; import org.apache.unomi.router.api.RouterConstants; +import org.apache.unomi.router.api.services.ImportExportConfigurationService; import java.util.Map; @@ -61,6 +63,37 @@ public RouterAbstractRouteBuilder(Map kafkaProps, String configT this.configType = configType; } + /** + * Records, on the configuration itself, whether the endpoint it names can be honoured. + * + *

The permitted directories are an operational setting and the configurations are user data, so + * the two drift apart: a configuration that was legitimate when it was created can be refused after + * the deployment is reconfigured. Refusing it silently leaves the owner with a configuration that + * looks fine and does nothing, so the refusal is written where they will see it. It is theirs to + * correct or remove — nothing is deleted here. + * + *

The other way round matters just as much: restoring the permitted directories must bring the + * configuration back on its own, without anyone having to touch it. Only the status this method + * sets is cleared, so the record of a run that genuinely failed survives. + * + *

The configuration is saved without asking for its running route to be refreshed: the refresh + * would rebuild the route, refuse it again and save it again, without end. + * + * @param configuration the configuration whose endpoint was examined + * @param service the service holding that kind of configuration + * @param refusal the reason the endpoint was refused, or {@code null} if it can be honoured + */ + protected void recordEndpointOutcome( + T configuration, ImportExportConfigurationService service, String refusal) { + if (refusal != null) { + configuration.setStatus(RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT); + service.save(configuration, false); + } else if (RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT.equals(configuration.getStatus())) { + configuration.setStatus(null); + service.save(configuration, false); + } + } + public Object getEndpointURI(String direction, String operationDepositBuffer) { Object endpoint; if (RouterConstants.CONFIG_TYPE_KAFKA.equals(configType)) { diff --git a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java index e10d3b0c64..78287c6f8c 100644 --- a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java +++ b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java @@ -23,6 +23,7 @@ import org.apache.unomi.router.api.ImportConfiguration; import org.apache.unomi.router.api.ProfileToImport; import org.apache.unomi.router.api.RouterConstants; +import org.apache.unomi.router.api.services.ImportExportConfigurationService; import org.junit.After; import org.junit.Assume; import org.junit.Before; @@ -382,6 +383,7 @@ private void addImportRoutes(String allowedEndpoints, ImportConfiguration... con ProfileImportFromSourceRouteBuilder builder = new ProfileImportFromSourceRouteBuilder(NO_KAFKA, RouterConstants.CONFIG_TYPE_NOBROKER); builder.setImportConfigurationList(Arrays.asList(configurations)); + builder.setImportConfigurationService(discardingConfigurationService()); builder.setProfileService(noOpProfileService()); builder.setJacksonDataFormat(new JacksonDataFormat(ProfileToImport.class)); builder.setAllowedEndpoints(allowedEndpoints); @@ -394,6 +396,7 @@ private void addExportRoutes(ExportConfiguration... configurations) throws Excep ProfileExportCollectRouteBuilder builder = new ProfileExportCollectRouteBuilder(NO_KAFKA, RouterConstants.CONFIG_TYPE_NOBROKER); builder.setExportConfigurationList(Arrays.asList(configurations)); + builder.setExportConfigurationService(discardingConfigurationService()); builder.setJacksonDataFormat(new JacksonDataFormat(ProfileToImport.class)); builder.setAllowedEndpoints(DEFAULT_ALLOWED_ENDPOINTS); builder.setPermittedExportBaseDirs(permittedExportDir.getAbsolutePath()); @@ -401,6 +404,18 @@ private void addExportRoutes(ExportConfiguration... configurations) throws Excep camelContext.addRoutes(builder); } + /** + * A refused configuration is recorded, and these tests are not about that: what they observe is + * whether the route was built. + */ + @SuppressWarnings("unchecked") + private static ImportExportConfigurationService discardingConfigurationService() { + return (ImportExportConfigurationService) Proxy.newProxyInstance( + ImportExportConfigurationService.class.getClassLoader(), + new Class[]{ImportExportConfigurationService.class}, + (proxy, method, args) -> "save".equals(method.getName()) ? args[0] : null); + } + /** * The route builders ask the profile service for the profile property types while they build. * Nothing in these tests depends on what it answers. diff --git a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RefusedConfigurationStatusTest.java b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RefusedConfigurationStatusTest.java new file mode 100644 index 0000000000..d0daf23e4d --- /dev/null +++ b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RefusedConfigurationStatusTest.java @@ -0,0 +1,305 @@ +/* + * 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.core.route; + +import org.apache.camel.component.jackson.JacksonDataFormat; +import org.apache.camel.impl.DefaultCamelContext; +import org.apache.unomi.api.services.ProfileService; +import org.apache.unomi.router.api.ExportConfiguration; +import org.apache.unomi.router.api.ImportConfiguration; +import org.apache.unomi.router.api.ProfileToImport; +import org.apache.unomi.router.api.RouterConstants; +import org.apache.unomi.router.api.services.ImportExportConfigurationService; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.lang.reflect.Proxy; +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 static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +/** + * The permitted base directories are an operational setting, and the configurations are user data: + * the two drift apart. A configuration that was legitimate when it was created can find itself + * outside the permitted directories after the deployment is reconfigured, and its route then stops + * being built. + * + *

Leaving that silent is what makes it painful — the screen keeps showing the configuration as + * running, and the only trace is a log line nobody reads. So a configuration whose endpoint is + * refused while its route is being built is recorded as failed, and stays in the store: it is for + * whoever owns it to correct it or remove it, and they can only do that if they can see it. + * + *

Recording it must not schedule the configuration for a route refresh — that would have the + * refresh rebuild the route, refuse it again, and save it again, indefinitely. + */ +public class RefusedConfigurationStatusTest { + + private static final String DEFAULT_ALLOWED_ENDPOINTS = "file,ftp,sftp,ftps"; + + private static final Map NO_KAFKA = new HashMap<>(); + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + private DefaultCamelContext camelContext; + + private File permittedImportDir; + private File permittedExportDir; + private File arbitraryDir; + + private RecordingConfigurationService importConfigurations; + private RecordingConfigurationService exportConfigurations; + + @Before + public void setUp() throws Exception { + permittedImportDir = tmp.newFolder("permitted-import"); + permittedExportDir = tmp.newFolder("permitted-export"); + arbitraryDir = tmp.newFolder("arbitrary"); + camelContext = new DefaultCamelContext(); + importConfigurations = new RecordingConfigurationService<>(); + exportConfigurations = new RecordingConfigurationService<>(); + } + + @After + public void tearDown() throws Exception { + camelContext.stop(); + } + + @Test + public void aRefusedImportConfigurationIsRecordedAsFailed() throws Exception { + ImportConfiguration configuration = recurrentImport(fileUri(arbitraryDir, "?fileName=profiles.csv")); + + addImportRoutes(configuration); + + assertNull("no route should have been built", camelContext.getRouteDefinition("out-of-bounds")); + assertEquals("the configuration should be recorded as failed", + RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT, configuration.getStatus()); + assertTrue("the configuration should have been saved so the failure is visible", + importConfigurations.contains("out-of-bounds")); + } + + @Test + public void aRefusedExportConfigurationIsRecordedAsFailed() throws Exception { + ExportConfiguration configuration = recurrentExport(fileUri(arbitraryDir, "?fileName=profiles.csv")); + + addExportRoutes(configuration); + + assertNull("no route should have been built", camelContext.getRouteDefinition("out-of-bounds")); + assertEquals("the configuration should be recorded as failed", + RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT, configuration.getStatus()); + assertTrue("the configuration should have been saved so the failure is visible", + exportConfigurations.contains("out-of-bounds")); + } + + @Test + public void recordingARefusedImportConfigurationDoesNotScheduleARouteRefresh() throws Exception { + addImportRoutes(recurrentImport(fileUri(arbitraryDir, "?fileName=profiles.csv"))); + + assertFalse("refreshing the route would refuse and save it again, without end", + importConfigurations.lastSaveAskedForARouteRefresh); + } + + @Test + public void recordingARefusedExportConfigurationDoesNotScheduleARouteRefresh() throws Exception { + addExportRoutes(recurrentExport(fileUri(arbitraryDir, "?fileName=profiles.csv"))); + + assertFalse("refreshing the route would refuse and save it again, without end", + exportConfigurations.lastSaveAskedForARouteRefresh); + } + + @Test + public void anImportConfigurationRecoversWhenItsEndpointBecomesAcceptableAgain() throws Exception { + ImportConfiguration configuration = recurrentImport(fileUri(permittedImportDir, "?fileName=profiles.csv")); + configuration.setStatus(RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT); + + addImportRoutes(configuration); + + assertNull("restoring the permitted directories must bring the configuration back on its own", + configuration.getStatus()); + assertTrue("the recovery must be persisted", importConfigurations.contains("out-of-bounds")); + } + + @Test + public void anExportConfigurationRecoversWhenItsEndpointBecomesAcceptableAgain() throws Exception { + ExportConfiguration configuration = recurrentExport(fileUri(permittedExportDir, "?fileName=profiles.csv")); + configuration.setStatus(RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT); + + addExportRoutes(configuration); + + assertNull("restoring the permitted directories must bring the configuration back on its own", + configuration.getStatus()); + assertTrue("the recovery must be persisted", exportConfigurations.contains("out-of-bounds")); + } + + @Test + public void aConfigurationThatFailedItsLastRunKeepsThatStatusWhenItsRouteIsRebuilt() throws Exception { + ImportConfiguration configuration = recurrentImport(fileUri(permittedImportDir, "?fileName=profiles.csv")); + configuration.setStatus(RouterConstants.CONFIG_STATUS_COMPLETE_ERRORS); + + addImportRoutes(configuration); + + assertEquals("a failed run is a different matter, and its record must survive", + RouterConstants.CONFIG_STATUS_COMPLETE_ERRORS, configuration.getStatus()); + } + + @Test + public void anAcceptedImportConfigurationIsLeftAlone() throws Exception { + ImportConfiguration configuration = recurrentImport(fileUri(permittedImportDir, "?fileName=profiles.csv")); + configuration.setItemId("in-bounds"); + + addImportRoutes(configuration); + + assertNull("an accepted configuration keeps the status it had", configuration.getStatus()); + assertFalse("an accepted configuration is not saved while its route is built", + importConfigurations.contains("in-bounds")); + } + + @Test + public void anAcceptedExportConfigurationIsLeftAlone() throws Exception { + ExportConfiguration configuration = recurrentExport(fileUri(permittedExportDir, "?fileName=profiles.csv")); + configuration.setItemId("in-bounds"); + + addExportRoutes(configuration); + + assertNull("an accepted configuration keeps the status it had", configuration.getStatus()); + assertFalse("an accepted configuration is not saved while its route is built", + exportConfigurations.contains("in-bounds")); + } + + // --------------------------------------------------------------------------------------------- + // Fixtures + // --------------------------------------------------------------------------------------------- + + private String fileUri(File directory, String suffix) { + return "file://" + directory.getAbsolutePath() + suffix; + } + + private ImportConfiguration recurrentImport(String source) { + ImportConfiguration configuration = new ImportConfiguration(); + configuration.setItemId("out-of-bounds"); + configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT); + configuration.setActive(true); + configuration.getProperties().put("source", source); + configuration.getProperties().put("mapping", Collections.singletonMap("0", 0)); + return configuration; + } + + private ExportConfiguration recurrentExport(String destination) { + ExportConfiguration configuration = new ExportConfiguration(); + configuration.setItemId("out-of-bounds"); + configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT); + configuration.setActive(true); + 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; + } + + private void addImportRoutes(ImportConfiguration... configurations) throws Exception { + ProfileImportFromSourceRouteBuilder builder = + new ProfileImportFromSourceRouteBuilder(NO_KAFKA, RouterConstants.CONFIG_TYPE_NOBROKER); + builder.setImportConfigurationList(java.util.Arrays.asList(configurations)); + builder.setImportConfigurationService(importConfigurations); + builder.setProfileService(noOpProfileService()); + builder.setJacksonDataFormat(new JacksonDataFormat(ProfileToImport.class)); + builder.setAllowedEndpoints(DEFAULT_ALLOWED_ENDPOINTS); + builder.setPermittedImportBaseDirs(permittedImportDir.getAbsolutePath()); + builder.setContext(camelContext); + camelContext.addRoutes(builder); + } + + private void addExportRoutes(ExportConfiguration... configurations) throws Exception { + ProfileExportCollectRouteBuilder builder = + new ProfileExportCollectRouteBuilder(NO_KAFKA, RouterConstants.CONFIG_TYPE_NOBROKER); + builder.setExportConfigurationList(java.util.Arrays.asList(configurations)); + builder.setExportConfigurationService(exportConfigurations); + builder.setJacksonDataFormat(new JacksonDataFormat(ProfileToImport.class)); + builder.setAllowedEndpoints(DEFAULT_ALLOWED_ENDPOINTS); + builder.setPermittedExportBaseDirs(permittedExportDir.getAbsolutePath()); + builder.setContext(camelContext); + camelContext.addRoutes(builder); + } + + private static ProfileService noOpProfileService() { + return (ProfileService) Proxy.newProxyInstance( + ProfileService.class.getClassLoader(), + new Class[]{ProfileService.class}, + (proxy, method, args) -> java.util.Collection.class.isAssignableFrom(method.getReturnType()) + ? Collections.emptyList() : null); + } + + /** + * Stores what it is given, and remembers whether the last save asked for the running route to be + * refreshed — a refused configuration must not, or the refresh loops. + */ + private static final class RecordingConfigurationService implements ImportExportConfigurationService { + + private final Map stored = new LinkedHashMap<>(); + + private boolean lastSaveAskedForARouteRefresh; + + boolean contains(String configId) { + return stored.containsKey(configId); + } + + @Override + public List getAll() { + return new ArrayList<>(stored.values()); + } + + @Override + public T load(String configId) { + return stored.get(configId); + } + + @Override + public T save(T configuration, boolean updateRunningRoute) { + lastSaveAskedForARouteRefresh = updateRunningRoute; + stored.put(itemIdOf(configuration), configuration); + return configuration; + } + + @Override + public void delete(String configId) { + stored.remove(configId); + } + + @Override + public Map consumeConfigsToBeRefresh() { + return Collections.emptyMap(); + } + + private String itemIdOf(T configuration) { + return configuration instanceof ImportConfiguration + ? ((ImportConfiguration) configuration).getItemId() + : ((ExportConfiguration) configuration).getItemId(); + } + } +} From 8da970e731f840c3f35721fb130d9b716680f5b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Blanchard?= Date: Wed, 12 Aug 2026 16:46:43 +0200 Subject: [PATCH 5/7] UNOMI-973: Add integration tests for file endpoint containment The unit tests decide the containment rules; what only a running Unomi can show is that the settings reach the two places that read them, through paths nothing else exercises: the REST layer, which refuses a configuration as it is saved, and the route builders, which refuse one that is already stored. ProfileImportExportContainmentIT covers both. A configuration outside the permitted directories is answered 400 with a reason in the body and is not stored; one inside is accepted. One stored while bypassing the REST layer -- the configuration that was already there when the deployment was reconfigured -- is marked INVALID_ENDPOINT, consumes no file and writes none, and recovers on its own once its endpoint is acceptable again. The existing recurrent tests move to the permitted directories, which is what their configurations now have to name. ProfileImportBasicIT is left untouched on purpose: the oneshot upload builds its own endpoint, and its passing unchanged is what shows it stayed out of this. --- .../java/org/apache/unomi/itests/AllITs.java | 1 + .../ProfileImportExportContainmentIT.java | 260 ++++++++++++++++++ 2 files changed, 261 insertions(+) create mode 100644 itests/src/test/java/org/apache/unomi/itests/ProfileImportExportContainmentIT.java diff --git a/itests/src/test/java/org/apache/unomi/itests/AllITs.java b/itests/src/test/java/org/apache/unomi/itests/AllITs.java index c50e88464f..be8f596a8b 100644 --- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java +++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java @@ -43,6 +43,7 @@ ProfileImportRankingIT.class, ProfileImportActorsIT.class, ProfileExportIT.class, + ProfileImportExportContainmentIT.class, ProfileMergeIT.class, EventServiceIT.class, PropertiesUpdateActionIT.class, diff --git a/itests/src/test/java/org/apache/unomi/itests/ProfileImportExportContainmentIT.java b/itests/src/test/java/org/apache/unomi/itests/ProfileImportExportContainmentIT.java new file mode 100644 index 0000000000..1a594336a0 --- /dev/null +++ b/itests/src/test/java/org/apache/unomi/itests/ProfileImportExportContainmentIT.java @@ -0,0 +1,260 @@ +/* + * 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.itests; + +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.apache.http.util.EntityUtils; +import org.apache.unomi.router.api.ExportConfiguration; +import org.apache.unomi.router.api.ImportConfiguration; +import org.apache.unomi.router.api.RouterConstants; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.ops4j.pax.exam.junit.PaxExam; +import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; +import org.ops4j.pax.exam.spi.reactors.PerSuite; + +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.HashMap; +import java.util.Map; + +/** + * A recurrent import or export configuration using the {@code file} scheme must resolve inside the base + * directory the deployment permits — {@code config.import.baseDir} and {@code config.export.baseDir}, + * set for these tests in {@code org.apache.unomi.router.cfg}. + * + *

The unit tests decide the containment rules. What only a running Unomi can show is that the + * settings actually reach the two places that need them: the REST layer, which refuses a configuration + * as it is saved, and the route builders, which refuse one that is already stored. Those two read the + * setting through different paths, and neither is exercised by a unit test. + */ +@RunWith(PaxExam.class) +@ExamReactorStrategy(PerSuite.class) +public class ProfileImportExportContainmentIT extends BaseIT { + + private static final String IMPORT_CONFIGURATION_URL = "/cxs/importConfiguration"; + private static final String EXPORT_CONFIGURATION_URL = "/cxs/exportConfiguration"; + + /** Permitted by org.apache.unomi.router.cfg. */ + private static final String PERMITTED_IMPORT_DIR = "data/tmp/recurrent_import"; + private static final String PERMITTED_EXPORT_DIR = "data/tmp/recurrent_export"; + + /** Not permitted by anything: a sibling of the two above. */ + private static final String ARBITRARY_DIR = "data/tmp/it-containment-arbitrary"; + + private String createdImportConfigId; + private String createdExportConfigId; + + @After + public void cleanup() { + if (createdImportConfigId != null) { + importConfigurationService.delete(createdImportConfigId); + createdImportConfigId = null; + } + if (createdExportConfigId != null) { + exportConfigurationService.delete(createdExportConfigId); + createdExportConfigId = null; + } + } + + // --------------------------------------------------------------------------------------------- + // Refused as it is saved + // --------------------------------------------------------------------------------------------- + + @Test + public void savingAnImportConfigurationOutsideThePermittedDirectoryIsRefused() throws Exception { + ImportConfiguration configuration = recurrentImport("it-import-refused", + fileUri(ARBITRARY_DIR, "?fileName=containment-it.csv")); + + Response response = postJson(IMPORT_CONFIGURATION_URL, configuration); + + Assert.assertEquals("a configuration whose source cannot be honoured is a bad request", + 400, response.status); + Assert.assertFalse("the refusal must say why, so the caller can correct it", + response.body.trim().isEmpty()); + Assert.assertNull("a refused configuration must not be stored", + importConfigurationService.load("it-import-refused")); + } + + @Test + public void savingAnExportConfigurationOutsideThePermittedDirectoryIsRefused() throws Exception { + ExportConfiguration configuration = recurrentExport("it-export-refused", + fileUri(ARBITRARY_DIR, "?fileName=containment-it.csv")); + + Response response = postJson(EXPORT_CONFIGURATION_URL, configuration); + + Assert.assertEquals("a configuration whose destination cannot be honoured is a bad request", + 400, response.status); + Assert.assertFalse("the refusal must say why, so the caller can correct it", + response.body.trim().isEmpty()); + Assert.assertNull("a refused configuration must not be stored", + exportConfigurationService.load("it-export-refused")); + } + + @Test + public void savingAnImportConfigurationInsideThePermittedDirectoryIsAccepted() throws Exception { + createdImportConfigId = "it-import-accepted"; + ImportConfiguration configuration = recurrentImport(createdImportConfigId, + fileUri(PERMITTED_IMPORT_DIR, "?fileName=containment-it.csv&consumer.delay=10m")); + + Response response = postJson(IMPORT_CONFIGURATION_URL, configuration); + + Assert.assertEquals("a configuration inside the permitted directory is legitimate", + 200, response.status); + keepTrying("the accepted configuration should have been stored", + () -> importConfigurationService.load(createdImportConfigId), c -> c != null, 1000, 20); + } + + // --------------------------------------------------------------------------------------------- + // Already stored, refused when its route is built + // --------------------------------------------------------------------------------------------- + + @Test + public void aStoredImportConfigurationOutsideThePermittedDirectoryIsMarkedAndConsumesNothing() throws Exception { + File arbitraryDir = new File(ARBITRARY_DIR); + Assert.assertTrue("could not prepare the test fixture", arbitraryDir.exists() || arbitraryDir.mkdirs()); + File waiting = new File(arbitraryDir, "must-not-be-consumed.csv"); + Files.write(waiting.toPath(), "email,firstName\nnobody@example.com,Nobody\n".getBytes(StandardCharsets.UTF_8)); + + createdImportConfigId = "it-import-stored-out-of-bounds"; + ImportConfiguration configuration = recurrentImport(createdImportConfigId, + fileUri(ARBITRARY_DIR, "?fileName=must-not-be-consumed.csv")); + + // bypasses the REST layer on purpose: this is the configuration that was already there when the + // deployment's permitted directories changed + importConfigurationService.save(configuration, true); + + keepTrying("a configuration whose route cannot be built must be marked, not silently dropped", + () -> importConfigurationService.load(createdImportConfigId), + c -> c != null && RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT.equals(c.getStatus()), 1000, 30); + + Assert.assertTrue("no route may consume a source outside the permitted directory", waiting.exists()); + } + + @Test + public void aStoredExportConfigurationOutsideThePermittedDirectoryIsMarkedAndWritesNothing() throws Exception { + File arbitraryDir = new File(ARBITRARY_DIR); + Assert.assertTrue("could not prepare the test fixture", arbitraryDir.exists() || arbitraryDir.mkdirs()); + File shouldNeverAppear = new File(arbitraryDir, "export-must-not-appear.csv"); + Files.deleteIfExists(shouldNeverAppear.toPath()); + + createdExportConfigId = "it-export-stored-out-of-bounds"; + ExportConfiguration configuration = recurrentExport(createdExportConfigId, + fileUri(ARBITRARY_DIR, "?fileName=export-must-not-appear.csv")); + + exportConfigurationService.save(configuration, true); + + keepTrying("a configuration whose route cannot be built must be marked, not silently dropped", + () -> exportConfigurationService.load(createdExportConfigId), + c -> c != null && RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT.equals(c.getStatus()), 1000, 30); + + Assert.assertFalse("no route may write to a destination outside the permitted directory", + shouldNeverAppear.exists()); + } + + @Test + public void aMarkedConfigurationRecoversOnItsOwnWhenItsEndpointBecomesAcceptableAgain() throws Exception { + createdImportConfigId = "it-import-recovering"; + ImportConfiguration configuration = recurrentImport(createdImportConfigId, + fileUri(ARBITRARY_DIR, "?fileName=containment-it.csv")); + importConfigurationService.save(configuration, true); + + ImportConfiguration marked = keepTrying("the configuration should first be marked", + () -> importConfigurationService.load(createdImportConfigId), + c -> c != null && RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT.equals(c.getStatus()), 1000, 30); + + // what operations would do: put the endpoint back where it is permitted + marked.getProperties().put("source", fileUri(PERMITTED_IMPORT_DIR, "?fileName=containment-it.csv&consumer.delay=10m")); + importConfigurationService.save(marked, true); + + keepTrying("restoring the endpoint must clear the mark without anyone touching the status", + () -> importConfigurationService.load(createdImportConfigId), + c -> c != null && c.getStatus() == null, 1000, 30); + } + + // --------------------------------------------------------------------------------------------- + // Fixtures + // --------------------------------------------------------------------------------------------- + + private String fileUri(String directory, String suffix) { + return "file://" + new File(directory).getAbsolutePath() + suffix; + } + + /** + * Executes the request against the shared client rather than through {@code executeHttpRequest}, + * which consumes the response body to log it whenever the status is not {@code 200} — these tests + * need to read that body themselves. + */ + private Response postJson(String url, Object body) throws Exception { + HttpPost request = new HttpPost(getFullUrl(url)); + request.setEntity(new StringEntity(objectMapper.writeValueAsString(body), ContentType.APPLICATION_JSON)); + try (CloseableHttpResponse response = httpClient.execute(request)) { + return new Response(response.getStatusLine().getStatusCode(), + response.getEntity() == null ? "" : EntityUtils.toString(response.getEntity())); + } + } + + private static final class Response { + private final int status; + private final String body; + + private Response(int status, String body) { + this.status = status; + this.body = body; + } + } + + private ImportConfiguration recurrentImport(String itemId, String source) { + ImportConfiguration configuration = new ImportConfiguration(); + configuration.setItemId(itemId); + configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT); + configuration.setColumnSeparator(","); + configuration.setActive(true); + + Map mapping = new HashMap<>(); + mapping.put("email", 0); + mapping.put("firstName", 1); + configuration.getProperties().put("mapping", mapping); + configuration.getProperties().put("source", source); + configuration.setMergingProperty("email"); + return configuration; + } + + private ExportConfiguration recurrentExport(String itemId, String destination) { + ExportConfiguration configuration = new ExportConfiguration(); + configuration.setItemId(itemId); + configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT); + configuration.setColumnSeparator(";"); + configuration.setMultiValueDelimiter("()"); + configuration.setMultiValueSeparator(";"); + configuration.setActive(true); + + Map mapping = new HashMap<>(); + mapping.put("0", "firstName"); + configuration.getProperties().put("mapping", mapping); + configuration.getProperties().put("segment", "itContainmentSegment"); + configuration.getProperties().put("period", "1m"); + configuration.getProperties().put("destination", destination); + return configuration; + } +} From d233a9fbe1f7ec6af4bd8a958137747de59ab654 Mon Sep 17 00:00:00 2001 From: Jerome Blanchard Date: Tue, 18 Aug 2026 11:51:28 +0200 Subject: [PATCH 6/7] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../java/org/apache/unomi/router/api/EndpointValidator.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java index 1a18a0e89a..a71c99e04a 100644 --- a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java +++ b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java @@ -111,7 +111,8 @@ private static String validateContainment(String endpointUri, String permittedBa } for (String[] parameter : parseQuery(query)) { - if (!PATH_BEARING_OPTIONS.contains(parameter[0].toLowerCase(Locale.ROOT))) { + String optionName = decode(parameter[0]); + if (!PATH_BEARING_OPTIONS.contains(optionName.toLowerCase(Locale.ROOT))) { continue; } String value = stripRaw(decode(parameter[1])); @@ -119,7 +120,7 @@ private static String validateContainment(String endpointUri, String permittedBa continue; } if (!isContained(directory.resolve(value), baseDirs)) { - return "option '" + parameter[0] + "' points outside the permitted directories"; + return "option '" + optionName + "' points outside the permitted directories"; } } From 56664e9f4a170d82b3d80182673b45316610e845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Blanchard?= Date: Tue, 18 Aug 2026 14:49:15 +0200 Subject: [PATCH 7/7] UNOMI-973: Decide containment on the endpoint Camel resolves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Containment was decided on the URI as it is written, which is not always the path Camel ends up using, and three readings differed. An option name is percent-decoded by Camel before it is bound, so file%4Eame is the fileName option and went unchecked. A RAW() value ends at the marker that closes it, not at the first ampersand, so splitting the query on every ampersand left the rest of fileName=RAW(x&/../../elsewhere) unread. And a path-bearing option is a File Language expression, so move=${file:parent}/../elsewhere normalized to a child of the endpoint directory while Camel sends the consumed file to a sibling. The URI is now read the way Camel reads it. ${file:parent} stands for the endpoint directory, which is the shallowest directory it can expand to, so parent segments are measured against the worst case. A token that expands to a name stands for one path component. Every other expression is refused, since a header or the body can hold any path at all and there is nothing left to validate. Two paths were decided on although they cannot be resolved. A path the file system cannot use -- one holding a nul character, for one -- raised InvalidPathException out of validate, which answers 500 over REST and costs the batch its other routes at startup: the failure mode this change exists to remove. And a path whose existing part cannot be resolved was accepted as it stands, so a dangling symbolic link inside a permitted directory was taken for a child of it, and would have left it as soon as its target was created. Both are refusals now, and nothing is thrown out of the validator. Percent-decoding kept only the low byte of the characters it did not decode, so a path mixing an escape with a non-ASCII name -- café%20files -- was compared against a path Camel never uses. Unescaped characters now keep their own encoding. include, antInclude and antFilter are no longer held to containment. They select among the files the endpoint directory already offers and cannot direct a read or a write anywhere else, while a regular expression is not a valid path on every file system -- holding them to it only refuses legitimate patterns. moveExisting and tempPrefix are held to it instead: both carry a path Camel resolves against the endpoint directory. --- .../unomi/router/api/EndpointValidator.java | 201 ++++++++++++++++-- .../route/FileEndpointContainmentTest.java | 125 ++++++++++- .../ConfigurationEndpointValidationTest.java | 8 + 3 files changed, 315 insertions(+), 19 deletions(-) diff --git a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java index a71c99e04a..aedf0b1e40 100644 --- a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java +++ b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.Paths; @@ -45,6 +46,15 @@ * that merely shares a textual prefix with a permitted directory is not mistaken for one of its * children. * + *

What is validated is what Camel will use, so the URI is read the way Camel reads it: option + * names are percent-decoded before they are matched, a {@code RAW()} value keeps its ampersands + * instead of being cut in two, and the File Language expressions a path-bearing option may carry are + * accounted for rather than taken literally. + * + *

A path the file system cannot make sense of, and a path whose existing part cannot be resolved, + * are both refusals: nothing is thrown out of this class, because one malformed endpoint must not + * cost a deployment the routes of every other configuration. + * *

Schemes other than {@code file} carry no local path and are left to the scheme allow-list. */ public final class EndpointValidator { @@ -53,10 +63,35 @@ public final class EndpointValidator { /** * The Camel file endpoint options whose value is, or contains, a path. Compared in lower case. + * + *

Selection options ({@code include}, {@code exclude}, {@code antInclude}, {@code antExclude}) + * are deliberately absent: they are patterns matched against the files the endpoint directory + * already offers, not paths Camel resolves, so they cannot direct a read or a write anywhere else. + * Holding them to containment would only refuse legitimate patterns — a regular expression is not + * a valid path on every file system. */ private static final Set PATH_BEARING_OPTIONS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( - "filename", "tempfilename", "move", "movefailed", "premove", "donefilename", - "include", "antinclude", "antfilter"))); + "filename", "tempfilename", "tempprefix", "move", "movefailed", "moveexisting", "premove", + "donefilename"))); + + /** + * Camel evaluates a path-bearing option as a File Language expression, so the value that reaches + * the file system is not the one configured. These tokens expand to a name, or to a name and the + * subdirectories the file already sits in: whatever they hold, they cannot hold a parent segment, + * so containment can be decided with them replaced by a placeholder. + */ + private static final Set NAME_TOKENS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( + "file:name", "file:name.noext", "file:name.ext", "file:onlyname", "file:onlyname.noext", + "file:ext", "file:size", "file:modified", "exchangeid"))); + + /** Expands to a date, which cannot hold a parent segment either. A prefix, not a whole token. */ + private static final String DATE_TOKEN_PREFIX = "date:"; + + /** Expands to the directory the file sits in, which is the endpoint directory or one below it. */ + private static final String PARENT_TOKEN = "file:parent"; + + /** Stands in for a token that expands to a name: one path component, never a parent segment. */ + private static final String NAME_PLACEHOLDER = "_"; private EndpointValidator() { } @@ -89,10 +124,17 @@ public static String validate(String endpointUri, String allowedSchemes, String return null; } - return validateContainment(endpointUri, permittedBaseDirs); + try { + return validateContainment(endpointUri, permittedBaseDirs); + } catch (Refusal refusal) { + return refusal.getMessage(); + } catch (InvalidPathException e) { + return "endpoint '" + endpointUri + "' does not denote a path this file system can use: " + + e.getMessage(); + } } - private static String validateContainment(String endpointUri, String permittedBaseDirs) { + private static String validateContainment(String endpointUri, String permittedBaseDirs) throws Refusal { List baseDirs = new ArrayList<>(); for (String baseDir : split(permittedBaseDirs)) { baseDirs.add(canonicalize(Paths.get(baseDir))); @@ -111,16 +153,16 @@ private static String validateContainment(String endpointUri, String permittedBa } for (String[] parameter : parseQuery(query)) { - String optionName = decode(parameter[0]); - if (!PATH_BEARING_OPTIONS.contains(optionName.toLowerCase(Locale.ROOT))) { + String option = parameter[0]; + if (!PATH_BEARING_OPTIONS.contains(decode(option).toLowerCase(Locale.ROOT))) { continue; } String value = stripRaw(decode(parameter[1])); if (value.isEmpty()) { continue; } - if (!isContained(directory.resolve(value), baseDirs)) { - return "option '" + optionName + "' points outside the permitted directories"; + if (!isContained(directory.resolve(withoutExpressions(option, value)), baseDirs)) { + return "option '" + option + "' points outside the permitted directories"; } } @@ -136,7 +178,7 @@ private static String stripScheme(String head) { return path.startsWith("//") ? path.substring(2) : path; } - private static boolean isContained(Path path, List baseDirs) { + private static boolean isContained(Path path, List baseDirs) throws Refusal { Path candidate = canonicalize(path); for (Path baseDir : baseDirs) { if (candidate.startsWith(baseDir)) { @@ -151,8 +193,12 @@ private static boolean isContained(Path path, List baseDirs) { * parent segments, and with the symbolic links of its existing part followed. A path that does not * exist yet is canonicalized through its deepest existing ancestor — an export destination is * created on first write, and must be decided on before it exists. + * + *

A path whose existing part cannot be resolved is refused rather than accepted as it stands: a + * dangling symbolic link inside a permitted directory would otherwise be taken for a child of it, + * and would leave it as soon as its target is created. */ - private static Path canonicalize(Path path) { + private static Path canonicalize(Path path) throws Refusal { Path normalized = path.toAbsolutePath().normalize(); Path existing = normalized; while (existing != null && !Files.exists(existing, LinkOption.NOFOLLOW_LINKS)) { @@ -164,10 +210,70 @@ private static Path canonicalize(Path path) { try { return existing.toRealPath().resolve(existing.relativize(normalized)); } catch (IOException e) { - return normalized; + throw new Refusal("path '" + normalized + "' cannot be resolved on the file system: " + e); } } + /** + * Replaces the File Language expressions of a path-bearing option by what they can contribute to a + * path, so that containment is decided on the value Camel will resolve instead of on the + * placeholder as it is written: {@code move=${file:parent}/../elsewhere} normalizes to a child of + * the endpoint directory while Camel sends it to a sibling. + * + *

{@code ${file:parent}} becomes the endpoint directory itself, which is the shallowest + * directory it can expand to, so parent segments are measured against the worst case. A token that + * expands to a name becomes a placeholder. Every other expression is refused: a header, a property + * or the body can hold any path at all, and there is nothing left to validate. + */ + private static String withoutExpressions(String option, String value) throws Refusal { + if (value.indexOf('$') < 0) { + return value; + } + StringBuilder substituted = new StringBuilder(value.length()); + int position = 0; + while (position < value.length()) { + int start = expressionStart(value, position); + if (start < 0) { + substituted.append(value, position, value.length()); + break; + } + substituted.append(value, position, start); + int open = value.indexOf('{', start); + int close = value.indexOf('}', open); + if (close < 0) { + throw new Refusal("option '" + option + "' carries an expression that is never closed"); + } + substituted.append(contributionOf(option, value.substring(open + 1, close))); + position = close + 1; + } + return substituted.toString(); + } + + /** + * The start of the next expression, in either of the forms Camel accepts ({@code ${...}} and + * {@code $simple{...}}), or {@code -1}. A dollar sign that starts neither is a plain character. + */ + private static int expressionStart(String value, int from) { + for (int i = value.indexOf('$', from); i >= 0; i = value.indexOf('$', i + 1)) { + if (value.startsWith("${", i) || value.startsWith("$simple{", i)) { + return i; + } + } + return -1; + } + + private static String contributionOf(String option, String expression) throws Refusal { + String token = expression.trim().toLowerCase(Locale.ROOT); + if (PARENT_TOKEN.equals(token)) { + return "."; + } + if (NAME_TOKENS.contains(token) || token.startsWith(DATE_TOKEN_PREFIX)) { + return NAME_PLACEHOLDER; + } + throw new Refusal("option '" + option + "' uses the expression '${" + expression + + "}', whose value cannot be held inside the permitted directories"); + } + /** * {@code RAW(...)} and {@code RAW{...}} tell Camel not to decode a value; the path it wraps is used * as it stands. @@ -182,45 +288,95 @@ private static String stripRaw(String value) { return value; } + /** + * Splits the query into its parameters the way Camel does. A {@code RAW()} value ends at the marker + * that closes it, not at the first ampersand, so an ampersand it contains does not start a new + * parameter: splitting on every ampersand would leave the rest of that value unvalidated, which is + * enough to carry {@code fileName=RAW(profiles.csv&../../elsewhere)} through. + */ private static List parseQuery(String query) { List parameters = new ArrayList<>(); - for (String parameter : query.split("&")) { - if (parameter.isEmpty()) { - continue; - } + int position = 0; + while (position < query.length()) { + int end = endOfParameter(query, position); + String parameter = query.substring(position, end); int separator = parameter.indexOf('='); if (separator > 0) { parameters.add(new String[]{parameter.substring(0, separator), parameter.substring(separator + 1)}); } + position = end + 1; } return parameters; } + /** Where the parameter that starts at {@code from} ends: exclusive, on its closing ampersand. */ + private static int endOfParameter(String query, int from) { + int ampersand = query.indexOf('&', from); + int valueStart = query.indexOf('=', from); + char closing = 0; + if (valueStart >= 0 && (ampersand < 0 || valueStart < ampersand)) { + closing = rawClosingMarker(query.substring(valueStart + 1)); + } + if (closing == 0) { + return ampersand < 0 ? query.length() : ampersand; + } + for (int i = valueStart + 1; i < query.length(); i++) { + if (query.charAt(i) == closing && (i + 1 == query.length() || query.charAt(i + 1) == '&')) { + return i + 1; + } + } + return query.length(); + } + + private static char rawClosingMarker(String value) { + if (value.startsWith("RAW(")) { + return ')'; + } + if (value.startsWith("RAW{")) { + return '}'; + } + return 0; + } + /** * Decodes the percent-encoding of a URI, so that containment is decided on the path the file system * will see. Unlike form decoding, {@code +} is left alone: it is a valid character in a file name. + * Characters that are not escaped keep their own encoding, so a path that mixes an escape with a + * non-ASCII name is not corrupted into a different path. */ private static String decode(String value) { if (value.indexOf('%') < 0) { return value; } ByteArrayOutputStream decoded = new ByteArrayOutputStream(value.length()); + StringBuilder verbatim = new StringBuilder(); for (int i = 0; i < value.length(); i++) { char character = value.charAt(i); if (character == '%' && i + 2 < value.length()) { int high = Character.digit(value.charAt(i + 1), 16); int low = Character.digit(value.charAt(i + 2), 16); if (high >= 0 && low >= 0) { + writeUtf8(verbatim, decoded); decoded.write((high << 4) + low); i += 2; continue; } } - decoded.write(character); + verbatim.append(character); } + writeUtf8(verbatim, decoded); return new String(decoded.toByteArray(), StandardCharsets.UTF_8); } + private static void writeUtf8(StringBuilder verbatim, ByteArrayOutputStream decoded) { + if (verbatim.length() == 0) { + return; + } + byte[] bytes = verbatim.toString().getBytes(StandardCharsets.UTF_8); + decoded.write(bytes, 0, bytes.length); + verbatim.setLength(0); + } + private static List split(String commaSeparated) { List values = new ArrayList<>(); if (commaSeparated == null) { @@ -247,4 +403,17 @@ private static boolean containsIgnoreCase(List values, String searched) private static boolean isBlank(String value) { return value == null || value.trim().isEmpty(); } + + /** + * A reason an endpoint cannot be validated, carried back to {@link #validate} to be answered as a + * refusal. Nothing is thrown out of this class. + */ + private static final class Refusal extends Exception { + + private static final long serialVersionUID = 1L; + + Refusal(String reason) { + super(reason); + } + } } diff --git a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java index 78287c6f8c..0d065f9db1 100644 --- a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java +++ b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java @@ -55,8 +55,13 @@ * *

Containment covers the directory named by the URI and every path-bearing option it * carries ({@code fileName}, {@code move}, {@code moveFailed}, {@code preMove}, {@code doneFileName}, - * {@code include}, ...) — validating only the directory part would leave - * {@code file:///permitted/?fileName=../../elsewhere} open. + * ...) — validating only the directory part would leave + * {@code file:///permitted/?fileName=../../elsewhere} open. It is decided on what Camel will use, so + * the URI is read the way Camel reads it: percent-encoded option names, {@code RAW()} values that + * carry an ampersand, and the File Language expressions a path-bearing option may hold. + * + *

Selection options ({@code include}, {@code antInclude}) are patterns matched against the files + * the directory already offers, not paths Camel resolves, and are not held to containment. * *

Remote schemes ({@code ftp}, {@code sftp}, {@code ftps}) carry no local path and are not * subject to directory containment; the scheme allow-list keeps governing them. @@ -227,6 +232,99 @@ public void importRouteIsBuiltWhenMoveOptionIsRelativeAndStaysInsidePermittedBas assertRouteBuilt("relative-move", "a relative move target inside the base directory is how the feature is normally used"); } + @Test + public void importRouteIsRefusedWhenPathBearingOptionNameIsPercentEncoded() throws Exception { + addImportRoutes(recurrentImport("encoded-option-name", + fileUri(permittedImportDir, "?file%4Eame=../" + arbitraryDir.getName() + "/profiles.csv"))); + + assertRouteRefused("encoded-option-name", + "Camel decodes an option name before it binds it, so 'file%4Eame' is the fileName option"); + } + + @Test + public void importRouteIsRefusedWhenRawOptionValueCarriesTheEscapeBehindAnAmpersand() throws Exception { + addImportRoutes(recurrentImport("raw-ampersand", + fileUri(permittedImportDir, "?fileName=RAW(x&/../../" + arbitraryDir.getName() + "/profiles.csv)"))); + + assertRouteRefused("raw-ampersand", + "a RAW value ends at the marker that closes it, so the ampersand it carries does not start a new option"); + } + + @Test + public void importRouteIsRefusedWhenMoveOptionUsesAnExpressionThatLeavesPermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("expression-escape", + fileUri(permittedImportDir, "?fileName=profiles.csv&move=${file:parent}/../" + arbitraryDir.getName()))); + + assertRouteRefused("expression-escape", + "Camel evaluates move as an expression, and this one sends the consumed file to a sibling directory"); + } + + @Test + public void importRouteIsRefusedWhenMoveOptionUsesAnExpressionThatCannotBeValidated() throws Exception { + addImportRoutes(recurrentImport("opaque-expression", + fileUri(permittedImportDir, "?fileName=profiles.csv&move=${header.destination}"))); + + assertRouteRefused("opaque-expression", "a header can hold any path at all, so there is nothing left to validate"); + } + + @Test + public void importRouteIsBuiltWhenMoveOptionUsesTheParentExpressionAndStaysInsidePermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("parent-expression", + fileUri(permittedImportDir, "?fileName=profiles.csv&move=${file:parent}/.done/${file:onlyname}"))); + + assertRouteBuilt("parent-expression", "moving a consumed file next to itself is how the feature is normally used"); + } + + @Test + public void importRouteIsBuiltWhenSelectionPatternsAreNotPaths() throws Exception { + addImportRoutes(recurrentImport("selection-patterns", + fileUri(permittedImportDir, "?include=..&antInclude=**/*.csv&consumer.delay=10m"))); + + assertRouteBuilt("selection-patterns", + "a selection pattern is matched against the files the directory offers, so '..' asks for a two-character " + + "name — it is not a path Camel resolves, and holding it to containment only refuses patterns"); + } + + @Test + public void importRouteIsRefusedWhenSourceIsADanglingSymlinkInsidePermittedBaseDir() throws Exception { + File link = new File(permittedImportDir, "dangling"); + try { + Files.createSymbolicLink(link.toPath(), new File(tmp.getRoot(), "not-created-yet").toPath()); + } catch (IOException | UnsupportedOperationException e) { + Assume.assumeNoException("this file system does not support symbolic links", e); + } + + addImportRoutes(recurrentImport("dangling-symlink", fileUri(link, "?fileName=profiles.csv"))); + + assertRouteRefused("dangling-symlink", + "a link whose target cannot be resolved would leave the base directory as soon as its target is created"); + } + + @Test + public void unusablePathIsSkippedWithoutPreventingTheOtherRoutesFromBeingBuilt() throws Exception { + addImportRoutes( + recurrentImport("nul-character", fileUri(permittedImportDir, "?fileName=profiles%00.csv")), + recurrentImport("well-formed", fileUri(permittedImportDir, "?fileName=profiles.csv"))); + + assertRouteRefused("nul-character", "the file system cannot use a path that holds a nul character"); + assertRouteBuilt("well-formed", + "a path the file system rejects is a refusal, not an exception that costs the deployment its other routes"); + } + + @Test + public void importRouteIsBuiltWhenPermittedBaseDirIsNonAsciiAndTheUriIsPartlyEncoded() throws Exception { + File nonAsciiBaseDir = tmp.newFolder("caf\u00e9-import"); + Assume.assumeTrue("this file system does not keep non-ASCII directory names", nonAsciiBaseDir.isDirectory()); + File dropDir = new File(nonAsciiBaseDir, "drop zone"); + assertTrue("could not prepare the test fixture", dropDir.mkdir()); + + addImportRoutesInto(nonAsciiBaseDir, + recurrentImport("non-ascii", fileUri(nonAsciiBaseDir, "/drop%20zone?fileName=profiles.csv"))); + + assertRouteBuilt("non-ascii", + "decoding an escape must not corrupt the characters around it, or containment is decided on another path"); + } + @Test public void remoteImportEndpointIsNotSubjectToDirectoryContainment() throws Exception { addImportRoutes(recurrentImport("remote", "ftp://ftp.example.com/profiles?fileName=profiles.csv")); @@ -318,6 +416,15 @@ public void exportRouteIsRefusedWhenDoneFileNameOptionEscapesPermittedBaseDir() assertRouteRefused("donefilename-escape", "doneFileName carries a path and must be contained too"); } + @Test + public void exportRouteIsBuiltWhenFileNameOptionUsesADateExpression() throws Exception { + addExportRoutes(recurrentExport("date-expression", + fileUri(permittedExportDir, "?fileName=profiles-export-${date:now:yyyyMMddHHmm}.csv"))); + + assertRouteBuilt("date-expression", + "a date cannot hold a parent segment, and naming an export after it is what the documentation shows"); + } + @Test public void remoteExportEndpointIsNotSubjectToDirectoryContainment() throws Exception { addExportRoutes(recurrentExport("remote", "ftp://ftp.example.com/profiles?fileName=profiles.csv")); @@ -380,6 +487,13 @@ private void addImportRoutes(ImportConfiguration... configurations) throws Excep } private void addImportRoutes(String allowedEndpoints, ImportConfiguration... configurations) throws Exception { + ProfileImportFromSourceRouteBuilder builder = importRouteBuilder(allowedEndpoints, configurations); + builder.setPermittedImportBaseDirs(permittedImportDir.getAbsolutePath()); + builder.setContext(camelContext); + camelContext.addRoutes(builder); + } + + private ProfileImportFromSourceRouteBuilder importRouteBuilder(String allowedEndpoints, ImportConfiguration... configurations) { ProfileImportFromSourceRouteBuilder builder = new ProfileImportFromSourceRouteBuilder(NO_KAFKA, RouterConstants.CONFIG_TYPE_NOBROKER); builder.setImportConfigurationList(Arrays.asList(configurations)); @@ -387,7 +501,12 @@ private void addImportRoutes(String allowedEndpoints, ImportConfiguration... con builder.setProfileService(noOpProfileService()); builder.setJacksonDataFormat(new JacksonDataFormat(ProfileToImport.class)); builder.setAllowedEndpoints(allowedEndpoints); - builder.setPermittedImportBaseDirs(permittedImportDir.getAbsolutePath()); + return builder; + } + + private void addImportRoutesInto(File permittedBaseDir, ImportConfiguration... configurations) throws Exception { + ProfileImportFromSourceRouteBuilder builder = importRouteBuilder(DEFAULT_ALLOWED_ENDPOINTS, configurations); + builder.setPermittedImportBaseDirs(permittedBaseDir.getAbsolutePath()); builder.setContext(camelContext); camelContext.addRoutes(builder); } 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 index 3058e9c30f..b93adf8292 100644 --- 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 @@ -116,6 +116,14 @@ public void savingARecurrentImportWhoseFileNameOptionEscapesThePermittedBaseDirs assertRefused(() -> importEndpoint.saveConfiguration(configuration)); } + @Test + public void savingARecurrentImportWhoseSourceIsNotAUsablePathIsRefused() { + ImportConfiguration configuration = recurrentImport(fileUri(permittedImportDir, "?fileName=profiles%00.csv")); + + assertRefused(() -> importEndpoint.saveConfiguration(configuration)); + assertFalse("a refused configuration must not be stored", importConfigurations.contains("in-bounds")); + } + @Test public void savingAOneshotImportThatCarriesNoSourceStoresIt() { ImportConfiguration configuration = new ImportConfiguration();