Skip to content

Commit bef762f

Browse files
prdoyleprdoyle
authored
Remove duplicate exclusive paths (#124023)
* Remove duplicate exclusive paths * Normalize paths in tests to support Windows * Remove withMode
1 parent c96fcf7 commit bef762f

File tree

2 files changed

+101
-15
lines changed

2 files changed

+101
-15
lines changed

‎libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@
2626
import java.util.Objects;
2727
import java.util.function.BiConsumer;
2828

29+
import static java.util.Comparator.comparing;
2930
import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
3031
import static org.elasticsearch.entitlement.runtime.policy.FileUtils.PATH_ORDER;
32+
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE;
3133

3234
public final class FileAccessTree {
3335

@@ -59,8 +61,7 @@ static List<ExclusivePath> buildExclusivePathList(List<ExclusiveFileEntitlement>
5961
}
6062
}
6163
}
62-
exclusivePaths.sort((ep1, ep2) -> PATH_ORDER.compare(ep1.path(), ep2.path()));
63-
return exclusivePaths;
64+
return exclusivePaths.stream().sorted(comparing(ExclusivePath::path, PATH_ORDER)).distinct().toList();
6465
}
6566

6667
static void validateExclusivePaths(List<ExclusivePath> exclusivePaths) {
@@ -103,7 +104,7 @@ private FileAccessTree(
103104
List<String> writePaths = new ArrayList<>();
104105
BiConsumer<Path, Mode> addPath = (path, mode) -> {
105106
var normalized = normalizePath(path);
106-
if (mode == Mode.READ_WRITE) {
107+
if (mode == READ_WRITE) {
107108
writePaths.add(normalized);
108109
}
109110
readPaths.add(normalized);
@@ -139,7 +140,7 @@ private FileAccessTree(
139140
}
140141

141142
// everything has access to the temp dir, config dir and the jdk
142-
addPathAndMaybeLink.accept(pathLookup.tempDir(), Mode.READ_WRITE);
143+
addPathAndMaybeLink.accept(pathLookup.tempDir(), READ_WRITE);
143144
// TODO: this grants read access to the config dir for all modules until explicit read entitlements can be added
144145
addPathAndMaybeLink.accept(pathLookup.configDir(), Mode.READ);
145146

‎libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java‎

Lines changed: 96 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111

1212
import org.elasticsearch.common.settings.Settings;
1313
import org.elasticsearch.core.SuppressForbidden;
14+
import org.elasticsearch.entitlement.runtime.policy.FileAccessTree.ExclusiveFileEntitlement;
1415
import org.elasticsearch.entitlement.runtime.policy.FileAccessTree.ExclusivePath;
1516
import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement;
17+
import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.FileData;
1618
import org.elasticsearch.test.ESTestCase;
1719
import org.junit.BeforeClass;
1820

@@ -26,6 +28,11 @@
2628
import java.util.Map;
2729

2830
import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
31+
import static org.elasticsearch.entitlement.runtime.policy.FileAccessTree.buildExclusivePathList;
32+
import static org.elasticsearch.entitlement.runtime.policy.FileAccessTree.normalizePath;
33+
import static org.elasticsearch.entitlement.runtime.policy.Platform.WINDOWS;
34+
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ;
35+
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE;
2936
import static org.hamcrest.Matchers.equalTo;
3037
import static org.hamcrest.Matchers.is;
3138

@@ -195,7 +202,7 @@ public void testNormalizePath() {
195202
}
196203

197204
public void testNormalizeDirectorySeparatorWindows() {
198-
assumeTrue("normalization of windows paths", Platform.WINDOWS.isCurrent());
205+
assumeTrue("normalization of windows paths", WINDOWS.isCurrent());
199206

200207
assertThat(FileAccessTree.normalizePath(Path.of("C:\\a\\b")), equalTo("C:\\a\\b"));
201208
assertThat(FileAccessTree.normalizePath(Path.of("C:/a.xml")), equalTo("C:\\a.xml"));
@@ -254,7 +261,7 @@ public void testJdkAccess() {
254261

255262
@SuppressForbidden(reason = "don't care about the directory location in tests")
256263
public void testFollowLinks() throws IOException {
257-
assumeFalse("Windows requires admin right to create symbolic links", Platform.WINDOWS.isCurrent());
264+
assumeFalse("Windows requires admin right to create symbolic links", WINDOWS.isCurrent());
258265

259266
Path baseSourceDir = Files.createTempDirectory("fileaccess_source");
260267
Path source1Dir = baseSourceDir.resolve("source1");
@@ -348,23 +355,101 @@ public void testInvalidExclusiveAccess() {
348355
}
349356

350357
public void testDuplicatePrunedPaths() {
351-
List<String> paths = List.of("/a", "/a", "/a/b", "/a/b", "/b/c", "b/c/d", "b/c/d", "b/c/d", "e/f", "e/f");
352-
paths = FileAccessTree.pruneSortedPaths(paths);
353-
assertEquals(List.of("/a", "/b/c", "b/c/d", "e/f"), paths);
358+
List<String> inputPaths = List.of("/a", "/a", "/a/b", "/a/b", "/b/c", "b/c/d", "b/c/d", "b/c/d", "e/f", "e/f");
359+
List<String> outputPaths = List.of("/a", "/b/c", "b/c/d", "e/f");
360+
var actual = FileAccessTree.pruneSortedPaths(inputPaths.stream().map(p -> normalizePath(path(p))).toList());
361+
var expected = outputPaths.stream().map(p -> normalizePath(path(p))).toList();
362+
assertEquals(expected, actual);
363+
}
364+
365+
public void testDuplicateExclusivePaths() {
366+
// Bunch o' handy definitions
367+
var originalFileData = FileData.ofPath(path("/a/b"), READ).withExclusive(true);
368+
var fileDataWithWriteMode = FileData.ofPath(path("/a/b"), READ_WRITE).withExclusive(true);
369+
var original = new ExclusiveFileEntitlement("component1", "module1", new FilesEntitlement(List.of(originalFileData)));
370+
var differentComponent = new ExclusiveFileEntitlement("component2", original.moduleName(), original.filesEntitlement());
371+
var differentModule = new ExclusiveFileEntitlement(original.componentName(), "module2", original.filesEntitlement());
372+
var differentPath = new ExclusiveFileEntitlement(
373+
original.componentName(),
374+
original.moduleName(),
375+
new FilesEntitlement(
376+
List.of(FileData.ofPath(path("/c/d"), originalFileData.mode()).withExclusive(originalFileData.exclusive()))
377+
)
378+
);
379+
var differentMode = new ExclusiveFileEntitlement(
380+
original.componentName(),
381+
original.moduleName(),
382+
new FilesEntitlement(List.of(fileDataWithWriteMode))
383+
);
384+
var differentPlatform = new ExclusiveFileEntitlement(
385+
original.componentName(),
386+
original.moduleName(),
387+
new FilesEntitlement(List.of(originalFileData.withPlatform(WINDOWS)))
388+
);
389+
var originalExclusivePath = new ExclusivePath("component1", "module1", normalizePath(path("/a/b")));
390+
391+
// Some basic tests
392+
393+
assertEquals(
394+
"Single element should trivially work",
395+
List.of(originalExclusivePath),
396+
buildExclusivePathList(List.of(original), TEST_PATH_LOOKUP)
397+
);
398+
assertEquals(
399+
"Two identical elements should be combined",
400+
List.of(originalExclusivePath),
401+
buildExclusivePathList(List.of(original, original), TEST_PATH_LOOKUP)
402+
);
403+
404+
// Don't merge things we shouldn't
405+
406+
var distinctEntitlements = List.of(original, differentComponent, differentModule, differentPath);
407+
var distinctPaths = List.of(
408+
originalExclusivePath,
409+
new ExclusivePath("component2", original.moduleName(), originalExclusivePath.path()),
410+
new ExclusivePath(original.componentName(), "module2", originalExclusivePath.path()),
411+
new ExclusivePath(original.componentName(), original.moduleName(), normalizePath(path("/c/d")))
412+
);
413+
assertEquals(
414+
"Distinct elements should not be combined",
415+
distinctPaths,
416+
buildExclusivePathList(distinctEntitlements, TEST_PATH_LOOKUP)
417+
);
418+
419+
// Do merge things we should
420+
421+
List<ExclusiveFileEntitlement> interleavedEntitlements = new ArrayList<>();
422+
distinctEntitlements.forEach(e -> {
423+
interleavedEntitlements.add(e);
424+
interleavedEntitlements.add(original);
425+
});
426+
assertEquals(
427+
"Identical elements should be combined wherever they are in the list",
428+
distinctPaths,
429+
buildExclusivePathList(interleavedEntitlements, TEST_PATH_LOOKUP)
430+
);
431+
432+
var equivalentEntitlements = List.of(original, differentMode, differentPlatform);
433+
var equivalentPaths = List.of(originalExclusivePath);
434+
assertEquals(
435+
"Exclusive paths should be combined even if the entitlements are different",
436+
equivalentPaths,
437+
buildExclusivePathList(equivalentEntitlements, TEST_PATH_LOOKUP)
438+
);
354439
}
355440

356441
public void testWindowsAbsolutPathAccess() {
357-
assumeTrue("Specific to windows for paths with a root (DOS or UNC)", Platform.WINDOWS.isCurrent());
442+
assumeTrue("Specific to windows for paths with a root (DOS or UNC)", WINDOWS.isCurrent());
358443

359444
var fileAccessTree = FileAccessTree.of(
360445
"test",
361446
"test",
362447
new FilesEntitlement(
363448
List.of(
364-
FilesEntitlement.FileData.ofPath(Path.of("\\\\.\\pipe\\"), FilesEntitlement.Mode.READ),
365-
FilesEntitlement.FileData.ofPath(Path.of("D:\\.gradle"), FilesEntitlement.Mode.READ),
366-
FilesEntitlement.FileData.ofPath(Path.of("D:\\foo"), FilesEntitlement.Mode.READ),
367-
FilesEntitlement.FileData.ofPath(Path.of("C:\\foo"), FilesEntitlement.Mode.READ_WRITE)
449+
FileData.ofPath(Path.of("\\\\.\\pipe\\"), READ),
450+
FileData.ofPath(Path.of("D:\\.gradle"), READ),
451+
FileData.ofPath(Path.of("D:\\foo"), READ),
452+
FileData.ofPath(Path.of("C:\\foo"), FilesEntitlement.Mode.READ_WRITE)
368453
)
369454
),
370455
TEST_PATH_LOOKUP,
@@ -400,7 +485,7 @@ static FilesEntitlement entitlement(Map<String, String> value) {
400485
static List<ExclusivePath> exclusivePaths(String componentName, String moduleName, String... paths) {
401486
List<ExclusivePath> exclusivePaths = new ArrayList<>();
402487
for (String path : paths) {
403-
exclusivePaths.add(new ExclusivePath(componentName, moduleName, path(path).toString()));
488+
exclusivePaths.add(new ExclusivePath(componentName, moduleName, normalizePath(path(path))));
404489
}
405490
return exclusivePaths;
406491
}

0 commit comments

Comments
 (0)