This is an automated email from the ASF dual-hosted git repository. jsinovassin pushed a commit to branch UNOMI-973-review-followups in repository https://gitbox.apache.org/repos/asf/unomi.git
commit daa1da8f7700d32e0746eb0ce32aa0668d0ed0b5 Author: jsinovassin <[email protected]> AuthorDate: Tue Sep 1 18:34:41 2026 +0200 UNOMI-973: Confine localWorkDirectory whatever the endpoint's scheme A scheme other than file was answered valid without its options being looked at, on the grounds that a remote endpoint carries no local path. localWorkDirectory is the exception. The FTP and SFTP components stage the content they download into files there: FtpOperations .retrieveFileToFileInLocalWorkDirectory writes new File(localWorkDirectory, <the name the remote server announces>) and creates the directories on the way. So ftp://host/x?localWorkDirectory=<karaf.home>/deploy was accepted, and ftp is in the shipped allow-list. The option is now held to the permitted base directories whatever the scheme. It stays out of PATH_BEARING_OPTIONS because Camel resolves it on its own rather than against the directory the endpoint names, and it carries no File Language expression to account for. --- .../apache/unomi/router/api/EndpointValidator.java | 59 +++++++++++++++++++--- .../core/route/FileEndpointContainmentTest.java | 38 +++++++++++++- 2 files changed, 88 insertions(+), 9 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 ad18305ed..4e08f22a1 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 @@ -55,7 +55,10 @@ import java.util.Set; * 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. * - * <p>Schemes other than {@code file} carry no local path and are left to the scheme allow-list. + * <p>A scheme other than {@code file} addresses a remote server, so its directory and its + * path-bearing options are remote and none of this applies to them — with one exception. + * {@code localWorkDirectory} names a <em>local</em> directory, which the remote components stage + * their downloads in, so it is confined whatever the scheme. */ public final class EndpointValidator { @@ -90,6 +93,17 @@ public final class EndpointValidator { /** 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"; + /** + * The one option that names a local directory whatever the scheme: the remote components write the + * content they download into files there, under the name the remote server announces. + * + * <p>It is kept out of {@link #PATH_BEARING_OPTIONS} because it is not resolved the same way. Those + * options are resolved against the directory the endpoint names; this one Camel resolves on its + * own — {@code new File(FileUtil.normalizePath(value))} — so it is held to the permitted + * directories directly, and it carries no File Language expression to account for. + */ + private static final String LOCAL_WORK_DIRECTORY_OPTION = "localworkdirectory"; + /** Stands in for a token that expands to a name: one path component, never a parent segment. */ private static final String NAME_PLACEHOLDER = "_"; @@ -124,11 +138,11 @@ public final class EndpointValidator { return "endpoint scheme '" + scheme + "' is not allowed"; } - if (!FILE_SCHEME.equalsIgnoreCase(scheme)) { - return null; - } - try { + String refusal = validateLocalWorkDirectory(endpointUri, permittedBaseDirs); + if (refusal != null || !FILE_SCHEME.equalsIgnoreCase(scheme)) { + return refusal; + } return validateContainment(endpointUri, permittedBaseDirs); } catch (Refusal refusal) { return refusal.getMessage(); @@ -138,14 +152,45 @@ public final class EndpointValidator { } } - private static String validateContainment(String endpointUri, String permittedBaseDirs) throws Refusal { + /** + * Confines {@code localWorkDirectory}, the local directory a remote endpoint may be told to stage + * its downloads in. Left alone, it is a write outside the permitted directories that the scheme + * allow-list never sees: {@code ftp://host/x?localWorkDirectory=/opt/unomi/deploy} stages what the + * remote server sends, under the name the remote server chooses. + */ + private static String validateLocalWorkDirectory(String endpointUri, String permittedBaseDirs) throws Refusal { + int querySeparator = endpointUri.indexOf('?'); + if (querySeparator < 0) { + return null; + } + for (String[] parameter : parseQuery(endpointUri.substring(querySeparator + 1))) { + if (!LOCAL_WORK_DIRECTORY_OPTION.equals(decode(parameter[0]).toLowerCase(Locale.ROOT))) { + continue; + } + String value = stripRaw(decode(parameter[1])); + if (value.isEmpty()) { + continue; + } + if (!isContained(Paths.get(value), baseDirectories(permittedBaseDirs))) { + return "option '" + parameter[0] + "' points outside the permitted directories"; + } + } + return null; + } + + private static List<Path> baseDirectories(String permittedBaseDirs) throws Refusal { List<Path> 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"; + throw new Refusal("no permitted base directory is configured for file endpoints"); } + return baseDirs; + } + + private static String validateContainment(String endpointUri, String permittedBaseDirs) throws Refusal { + List<Path> baseDirs = baseDirectories(permittedBaseDirs); int querySeparator = endpointUri.indexOf('?'); String head = querySeparator < 0 ? endpointUri : endpointUri.substring(0, querySeparator); 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 2f749687a..97ad266bc 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 @@ -63,8 +63,11 @@ import static org.junit.Assert.assertTrue; * <p>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. * - * <p>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. + * <p>Remote schemes ({@code ftp}, {@code sftp}, {@code ftps}) address a remote server, so their + * directory and their path-bearing options are remote and are not subject to directory containment; + * the scheme allow-list keeps governing them. {@code localWorkDirectory} is the exception: it names a + * local directory, which the remote components stage their downloads in, and it is confined whatever + * the scheme. * * <p>These tests exercise route <em>construction</em>, which is where a configuration is turned into * a live route: a configuration whose endpoint is refused must leave no route behind, whether it @@ -351,6 +354,37 @@ public class FileEndpointContainmentTest { assertRouteBuilt("remote", "ftp is an allowed scheme and carries no local path"); } + @Test + public void importRouteIsRefusedWhenARemoteSourceStagesItsDownloadsOutsidePermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("remote-staging", "ftp://ftp.example.com/profiles"; + + "?fileName=profiles.csv&localWorkDirectory=" + arbitraryDir.getAbsolutePath())); + + assertRouteRefused("remote-staging", + "localWorkDirectory is a local directory: the remote server's content is written there, " + + "under the name the remote server chooses"); + } + + @Test + public void importRouteIsBuiltWhenARemoteSourceStagesItsDownloadsInsidePermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("remote-staging-in-bounds", "ftp://ftp.example.com/profiles"; + + "?fileName=profiles.csv&localWorkDirectory=" + + new File(permittedImportDir, "staging").getAbsolutePath())); + + assertRouteBuilt("remote-staging-in-bounds", + "staging downloads inside the permitted base directory is legitimate, and the directory " + + "need not exist yet"); + } + + @Test + public void importRouteIsRefusedWhenSourceStagesItsDownloadsOutsidePermittedBaseDir() throws Exception { + addImportRoutes(recurrentImport("local-staging", fileUri(permittedImportDir, + "?fileName=profiles.csv&localWorkDirectory=" + arbitraryDir.getAbsolutePath()))); + + assertRouteRefused("local-staging", + "the option is resolved on its own, not against the endpoint directory, so a permitted " + + "directory does not cover it"); + } + @Test public void importRouteIsRefusedWhenSchemeIsNotAllowed() throws Exception { addImportRoutes("ftp,sftp,ftps", recurrentImport("scheme-denied", fileUri(permittedImportDir, "?fileName=profiles.csv")));
