diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java index 62817f66f530c..fd2590c114d0d 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java @@ -26,8 +26,10 @@ import java.util.Objects; import java.util.function.BiConsumer; +import static java.util.Comparator.comparing; import static org.elasticsearch.core.PathUtils.getDefaultFileSystem; import static org.elasticsearch.entitlement.runtime.policy.FileUtils.PATH_ORDER; +import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE; public final class FileAccessTree { @@ -59,8 +61,7 @@ static List buildExclusivePathList(List } } } - exclusivePaths.sort((ep1, ep2) -> PATH_ORDER.compare(ep1.path(), ep2.path())); - return exclusivePaths; + return exclusivePaths.stream().sorted(comparing(ExclusivePath::path, PATH_ORDER)).distinct().toList(); } static void validateExclusivePaths(List exclusivePaths) { @@ -103,7 +104,7 @@ private FileAccessTree( List writePaths = new ArrayList<>(); BiConsumer addPath = (path, mode) -> { var normalized = normalizePath(path); - if (mode == Mode.READ_WRITE) { + if (mode == READ_WRITE) { writePaths.add(normalized); } readPaths.add(normalized); @@ -139,7 +140,7 @@ private FileAccessTree( } // everything has access to the temp dir, config dir and the jdk - addPathAndMaybeLink.accept(pathLookup.tempDir(), Mode.READ_WRITE); + addPathAndMaybeLink.accept(pathLookup.tempDir(), READ_WRITE); // TODO: this grants read access to the config dir for all modules until explicit read entitlements can be added addPathAndMaybeLink.accept(pathLookup.configDir(), Mode.READ); @@ -156,14 +157,15 @@ private FileAccessTree( this.writePaths = pruneSortedPaths(writePaths).toArray(new String[0]); } - private static List pruneSortedPaths(List paths) { + // package private for testing + static List pruneSortedPaths(List paths) { List prunedReadPaths = new ArrayList<>(); if (paths.isEmpty() == false) { String currentPath = paths.get(0); prunedReadPaths.add(currentPath); for (int i = 1; i < paths.size(); ++i) { String nextPath = paths.get(i); - if (isParent(currentPath, nextPath) == false) { + if (currentPath.equals(nextPath) == false && isParent(currentPath, nextPath) == false) { prunedReadPaths.add(nextPath); currentPath = nextPath; } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java index 08625e02b9997..ac9430246324f 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java @@ -11,8 +11,10 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.SuppressForbidden; +import org.elasticsearch.entitlement.runtime.policy.FileAccessTree.ExclusiveFileEntitlement; import org.elasticsearch.entitlement.runtime.policy.FileAccessTree.ExclusivePath; import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement; +import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.FileData; import org.elasticsearch.test.ESTestCase; import org.junit.BeforeClass; @@ -26,6 +28,11 @@ import java.util.Map; import static org.elasticsearch.core.PathUtils.getDefaultFileSystem; +import static org.elasticsearch.entitlement.runtime.policy.FileAccessTree.buildExclusivePathList; +import static org.elasticsearch.entitlement.runtime.policy.FileAccessTree.normalizePath; +import static org.elasticsearch.entitlement.runtime.policy.Platform.WINDOWS; +import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ; +import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -195,7 +202,7 @@ public void testNormalizePath() { } public void testNormalizeDirectorySeparatorWindows() { - assumeTrue("normalization of windows paths", Platform.WINDOWS.isCurrent()); + assumeTrue("normalization of windows paths", WINDOWS.isCurrent()); assertThat(FileAccessTree.normalizePath(Path.of("C:\\a\\b")), equalTo("C:\\a\\b")); assertThat(FileAccessTree.normalizePath(Path.of("C:/a.xml")), equalTo("C:\\a.xml")); @@ -254,7 +261,7 @@ public void testJdkAccess() { @SuppressForbidden(reason = "don't care about the directory location in tests") public void testFollowLinks() throws IOException { - assumeFalse("Windows requires admin right to create symbolic links", Platform.WINDOWS.isCurrent()); + assumeFalse("Windows requires admin right to create symbolic links", WINDOWS.isCurrent()); Path baseSourceDir = Files.createTempDirectory("fileaccess_source"); Path source1Dir = baseSourceDir.resolve("source1"); @@ -347,18 +354,102 @@ public void testInvalidExclusiveAccess() { assertThat(tree.canWrite(path("a")), is(false)); } + public void testDuplicatePrunedPaths() { + List inputPaths = List.of("/a", "/a", "/a/b", "/a/b", "/b/c", "b/c/d", "b/c/d", "b/c/d", "e/f", "e/f"); + List outputPaths = List.of("/a", "/b/c", "b/c/d", "e/f"); + var actual = FileAccessTree.pruneSortedPaths(inputPaths.stream().map(p -> normalizePath(path(p))).toList()); + var expected = outputPaths.stream().map(p -> normalizePath(path(p))).toList(); + assertEquals(expected, actual); + } + + public void testDuplicateExclusivePaths() { + // Bunch o' handy definitions + var originalFileData = FileData.ofPath(path("/a/b"), READ).withExclusive(true); + var fileDataWithWriteMode = FileData.ofPath(path("/a/b"), READ_WRITE).withExclusive(true); + var original = new ExclusiveFileEntitlement("component1", "module1", new FilesEntitlement(List.of(originalFileData))); + var differentComponent = new ExclusiveFileEntitlement("component2", original.moduleName(), original.filesEntitlement()); + var differentModule = new ExclusiveFileEntitlement(original.componentName(), "module2", original.filesEntitlement()); + var differentPath = new ExclusiveFileEntitlement( + original.componentName(), + original.moduleName(), + new FilesEntitlement( + List.of(FileData.ofPath(path("/c/d"), originalFileData.mode()).withExclusive(originalFileData.exclusive())) + ) + ); + var differentMode = new ExclusiveFileEntitlement( + original.componentName(), + original.moduleName(), + new FilesEntitlement(List.of(fileDataWithWriteMode)) + ); + var differentPlatform = new ExclusiveFileEntitlement( + original.componentName(), + original.moduleName(), + new FilesEntitlement(List.of(originalFileData.withPlatform(WINDOWS))) + ); + var originalExclusivePath = new ExclusivePath("component1", "module1", normalizePath(path("/a/b"))); + + // Some basic tests + + assertEquals( + "Single element should trivially work", + List.of(originalExclusivePath), + buildExclusivePathList(List.of(original), TEST_PATH_LOOKUP) + ); + assertEquals( + "Two identical elements should be combined", + List.of(originalExclusivePath), + buildExclusivePathList(List.of(original, original), TEST_PATH_LOOKUP) + ); + + // Don't merge things we shouldn't + + var distinctEntitlements = List.of(original, differentComponent, differentModule, differentPath); + var distinctPaths = List.of( + originalExclusivePath, + new ExclusivePath("component2", original.moduleName(), originalExclusivePath.path()), + new ExclusivePath(original.componentName(), "module2", originalExclusivePath.path()), + new ExclusivePath(original.componentName(), original.moduleName(), normalizePath(path("/c/d"))) + ); + assertEquals( + "Distinct elements should not be combined", + distinctPaths, + buildExclusivePathList(distinctEntitlements, TEST_PATH_LOOKUP) + ); + + // Do merge things we should + + List interleavedEntitlements = new ArrayList<>(); + distinctEntitlements.forEach(e -> { + interleavedEntitlements.add(e); + interleavedEntitlements.add(original); + }); + assertEquals( + "Identical elements should be combined wherever they are in the list", + distinctPaths, + buildExclusivePathList(interleavedEntitlements, TEST_PATH_LOOKUP) + ); + + var equivalentEntitlements = List.of(original, differentMode, differentPlatform); + var equivalentPaths = List.of(originalExclusivePath); + assertEquals( + "Exclusive paths should be combined even if the entitlements are different", + equivalentPaths, + buildExclusivePathList(equivalentEntitlements, TEST_PATH_LOOKUP) + ); + } + public void testWindowsAbsolutPathAccess() { - assumeTrue("Specific to windows for paths with a root (DOS or UNC)", Platform.WINDOWS.isCurrent()); + assumeTrue("Specific to windows for paths with a root (DOS or UNC)", WINDOWS.isCurrent()); var fileAccessTree = FileAccessTree.of( "test", "test", new FilesEntitlement( List.of( - FilesEntitlement.FileData.ofPath(Path.of("\\\\.\\pipe\\"), FilesEntitlement.Mode.READ), - FilesEntitlement.FileData.ofPath(Path.of("D:\\.gradle"), FilesEntitlement.Mode.READ), - FilesEntitlement.FileData.ofPath(Path.of("D:\\foo"), FilesEntitlement.Mode.READ), - FilesEntitlement.FileData.ofPath(Path.of("C:\\foo"), FilesEntitlement.Mode.READ_WRITE) + FileData.ofPath(Path.of("\\\\.\\pipe\\"), READ), + FileData.ofPath(Path.of("D:\\.gradle"), READ), + FileData.ofPath(Path.of("D:\\foo"), READ), + FileData.ofPath(Path.of("C:\\foo"), FilesEntitlement.Mode.READ_WRITE) ) ), TEST_PATH_LOOKUP, @@ -394,7 +485,7 @@ static FilesEntitlement entitlement(Map value) { static List exclusivePaths(String componentName, String moduleName, String... paths) { List exclusivePaths = new ArrayList<>(); for (String path : paths) { - exclusivePaths.add(new ExclusivePath(componentName, moduleName, path(path).toString())); + exclusivePaths.add(new ExclusivePath(componentName, moduleName, normalizePath(path(path)))); } return exclusivePaths; }