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 174a179df39cabd3f9060bb6ba9899e48c1e1173 Author: jsinovassin <[email protected]> AuthorDate: Tue Sep 1 18:48:16 2026 +0200 UNOMI-973: Cover what the follow-ups changed and the existing tests did not Each of the five behavioural commits carried a test that fails without it. These are the parts of what they changed that nothing yet observed. Resolving a path one component at a time, beyond the case that motivated it: a path-bearing option leaving through a link and a parent segment is refused the same way the endpoint directory is; a link that stays inside the base directory still builds its route; a base directory that is itself a link still matches the directory it points at, so naming one directory by two paths is one directory; and more parent segments than a path has components is a refusal rather than an exception. Confining localWorkDirectory, beyond the import route: the export direction refuses it too, the option is matched on its name rather than on how it is spelt, and the REST layer answers 400 for a remote source that stages its downloads out of bounds -- three code paths, one rule. Appending moveFailed: the endpoint the route is built on is now asserted, not just the fact that a route exists. A source that carries no query opens one, a source that carries a query gets another parameter. Answering 503 while the router is starting: the export endpoint does it too, and so does a partly published configuration -- the scheme allow-list there, the permitted directories not yet. 71 tests over the three modules. --- .../core/route/FileEndpointContainmentTest.java | 98 ++++++++++++++++++++++ .../rest/ConfigurationEndpointValidationTest.java | 36 ++++++++ 2 files changed, 134 insertions(+) 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 85e22b368..b47f85e47 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 @@ -42,6 +42,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -165,6 +166,23 @@ public class FileEndpointContainmentTest { "a parent segment applies to the target of the link that precedes it, not to the link's own parent"); } + @Test + public void moveFailedIsAppendedAsTheFirstOptionOfASourceThatCarriesNone() throws Exception { + addImportRoutes(recurrentImport("first-option", fileUri(permittedImportDir, ""))); + + assertEquals("the option must open the query, or it becomes part of the directory name", + fileUri(permittedImportDir, "?moveFailed=.error"), builtEndpointUri("first-option")); + } + + @Test + public void moveFailedIsAppendedToASourceThatAlreadyCarriesAQuery() throws Exception { + addImportRoutes(recurrentImport("next-option", fileUri(permittedImportDir, "?fileName=profiles.csv"))); + + assertEquals("an existing query takes the option as another parameter", + fileUri(permittedImportDir, "?fileName=profiles.csv&moveFailed=.error"), + builtEndpointUri("next-option")); + } + @Test public void importRouteIsBuiltWhenSourceCarriesNoOption() throws Exception { // the router appends moveFailed itself; a source with no query has no separator to append to, @@ -174,6 +192,64 @@ public class FileEndpointContainmentTest { assertRouteBuilt("no-option", "a source may name a directory and nothing else"); } + @Test + public void importRouteIsBuiltWhenSourceIsASymlinkPointingInsidePermittedBaseDir() throws Exception { + File target = new File(permittedImportDir, "incoming"); + assertTrue("could not prepare the test fixture", target.mkdir()); + File link = new File(permittedImportDir, "shortcut"); + try { + Files.createSymbolicLink(link.toPath(), target.toPath()); + } catch (IOException | UnsupportedOperationException e) { + Assume.assumeNoException("this file system does not support symbolic links", e); + } + + addImportRoutes(recurrentImport("symlink-inside", fileUri(link, "?fileName=profiles.csv"))); + + assertRouteBuilt("symlink-inside", "resolving a link must not refuse one that stays inside the base directory"); + } + + @Test + public void importRouteIsBuiltWhenThePermittedBaseDirIsItselfASymlink() throws Exception { + File link = new File(tmp.getRoot(), "permitted-import-link"); + try { + Files.createSymbolicLink(link.toPath(), permittedImportDir.toPath()); + } catch (IOException | UnsupportedOperationException e) { + Assume.assumeNoException("this file system does not support symbolic links", e); + } + + // the deployment names the link, the configuration names the directory it points at + addImportRoutesInto(link, recurrentImport("base-dir-link", fileUri(permittedImportDir, "?fileName=profiles.csv"))); + + assertRouteBuilt("base-dir-link", + "both sides are resolved, so naming the same directory by two paths is the same directory"); + } + + @Test + public void importRouteIsRefusedWhenAnOptionLeavesThroughASymlinkAndAParentSegment() throws Exception { + File outsideChild = new File(arbitraryDir, "child"); + assertTrue("could not prepare the test fixture", outsideChild.mkdir()); + File link = new File(permittedImportDir, "option-link"); + try { + Files.createSymbolicLink(link.toPath(), outsideChild.toPath()); + } catch (IOException | UnsupportedOperationException e) { + Assume.assumeNoException("this file system does not support symbolic links", e); + } + + addImportRoutes(recurrentImport("option-symlink-parent", + fileUri(permittedImportDir, "?fileName=profiles.csv&move=option-link/.."))); + + assertRouteRefused("option-symlink-parent", + "a path-bearing option is resolved the same way the endpoint directory is"); + } + + @Test + public void importRouteIsRefusedWhenSourceClimbsAboveTheRoot() throws Exception { + addImportRoutes(recurrentImport("above-root", "file:///../../../../../../../../etc?fileName=profiles.csv")); + + assertRouteRefused("above-root", + "more parent segments than the path has components is a refusal, not an exception"); + } + @Test public void importRouteIsRefusedWhenSourceIsOutsidePermittedBaseDir() throws Exception { addImportRoutes(recurrentImport("arbitrary-dir", fileUri(arbitraryDir, "?fileName=profiles.csv"))); @@ -494,6 +570,22 @@ public class FileEndpointContainmentTest { assertRouteBuilt("remote", "ftp is an allowed scheme and carries no local path"); } + @Test + public void exportRouteIsRefusedWhenDestinationStagesItsDownloadsOutsidePermittedBaseDir() throws Exception { + addExportRoutes(recurrentExport("remote-staging", "ftp://ftp.example.com/profiles"; + + "?fileName=profiles.csv&localWorkDirectory=" + arbitraryDir.getAbsolutePath())); + + assertRouteRefused("remote-staging", "the option names a local directory in either direction"); + } + + @Test + public void importRouteIsRefusedWhenTheStagingOptionIsSpeltInAnotherCase() throws Exception { + addImportRoutes(recurrentImport("staging-case", "ftp://ftp.example.com/profiles"; + + "?fileName=profiles.csv&LOCALWORKDIRECTORY=" + arbitraryDir.getAbsolutePath())); + + assertRouteRefused("staging-case", "the option is matched on its name, not on how it is spelt"); + } + @Test public void malformedDestinationIsSkippedWithoutPreventingTheOtherRoutesFromBeingBuilt() throws Exception { addExportRoutes( @@ -513,6 +605,12 @@ public class FileEndpointContainmentTest { camelContext.getRouteDefinition(routeId)); } + /** The endpoint URI the route was actually built on, which is not the one that was configured. */ + private String builtEndpointUri(String routeId) { + assertRouteBuilt(routeId, "there is no endpoint to look at otherwise"); + return camelContext.getRouteDefinition(routeId).getInputs().get(0).getUri(); + } + private void assertRouteRefused(String routeId, String why) { assertNull("a route was built for configuration '" + routeId + "', although " + why, camelContext.getRouteDefinition(routeId)); 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 b5df90d11..a5ea1de0a 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 @@ -113,6 +113,42 @@ public class ConfigurationEndpointValidationTest { assertFalse("nothing is stored while the endpoint cannot be judged", importConfigurations.contains("in-bounds")); } + @Test + public void savingARecurrentImportWhoseRemoteSourceStagesDownloadsOutsideThePermittedBaseDirsIsRefused() { + // ftp is an allowed scheme, but localWorkDirectory names a local directory all the same + ImportConfiguration configuration = recurrentImport("ftp://ftp.example.com/profiles"; + + "?fileName=profiles.csv&localWorkDirectory=" + arbitraryDir.getAbsolutePath()); + + assertRefused(() -> importEndpoint.saveConfiguration(configuration)); + assertFalse("a refused configuration must not be stored", importConfigurations.contains("in-bounds")); + } + + @Test + public void savingARecurrentImportBeforeThePermittedDirectoriesArePublishedIsNotRefused() throws Exception { + // the scheme allow-list is there, the directories are not: still nothing to judge against + InMemoryConfigSharingService partial = new InMemoryConfigSharingService(); + partial.setProperty(RouterConstants.CONFIG_ALLOWED_ENDPOINTS, "file,ftp,sftp,ftps"); + ImportConfigurationServiceEndPoint starting = new ImportConfigurationServiceEndPoint(); + starting.setImportConfigurationService(importConfigurations); + starting.setConfigSharingService(partial); + + ImportConfiguration configuration = recurrentImport(fileUri(permittedImportDir, "?fileName=profiles.csv")); + + assertUnavailable(() -> starting.saveConfiguration(configuration)); + } + + @Test + public void savingARecurrentExportBeforeTheRouterPublishedItsSettingsIsNotRefused() throws Exception { + ExportConfigurationServiceEndPoint starting = new ExportConfigurationServiceEndPoint(); + starting.setExportConfigurationService(exportConfigurations); + starting.setConfigSharingService(new InMemoryConfigSharingService()); + + ExportConfiguration configuration = recurrentExport(fileUri(permittedExportDir, "?fileName=profiles.csv")); + + assertUnavailable(() -> starting.saveConfiguration(configuration)); + assertFalse("nothing is stored while the endpoint cannot be judged", exportConfigurations.contains("in-bounds")); + } + @Test public void savingARecurrentImportWhoseSourceIsOutsideThePermittedBaseDirsIsRefused() { ImportConfiguration configuration = recurrentImport(fileUri(arbitraryDir, "?fileName=profiles.csv"));
