From a29a5931cde78ab85eb5b40f3ef690dda3e97c31 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sun, 2 Mar 2025 11:29:45 -0800 Subject: [PATCH] Remove ignoreUrl file setting property (#123718) Urls may make the FileAccessTree invalid. This commit removes the flag for filtering urls, instead always filtering them. --- .../policy/entitlements/FilesEntitlement.java | 26 ++++++-------- .../entitlements/FilesEntitlementTests.java | 34 ++++--------------- 2 files changed, 16 insertions(+), 44 deletions(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java index c22a47764d1c3..3d3a67bd69473 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java @@ -91,8 +91,8 @@ static FileData ofRelativePath(Path relativePath, BaseDir baseDir, Mode mode) { return new RelativePathFileData(relativePath, baseDir, mode, null, false); } - static FileData ofPathSetting(String setting, BaseDir baseDir, Mode mode, boolean ignoreUrl) { - return new PathSettingFileData(setting, baseDir, mode, ignoreUrl, null, false); + static FileData ofPathSetting(String setting, BaseDir baseDir, Mode mode) { + return new PathSettingFileData(setting, baseDir, mode, null, false); } /** @@ -220,22 +220,21 @@ public FileData withPlatform(Platform platform) { } } - private record PathSettingFileData(String setting, BaseDir baseDir, Mode mode, boolean ignoreUrl, Platform platform, boolean exclusive) + private record PathSettingFileData(String setting, BaseDir baseDir, Mode mode, Platform platform, boolean exclusive) implements RelativeFileData { @Override public PathSettingFileData withExclusive(boolean exclusive) { - return new PathSettingFileData(setting, baseDir, mode, ignoreUrl, platform, exclusive); + return new PathSettingFileData(setting, baseDir, mode, platform, exclusive); } @Override public Stream resolveRelativePaths(PathLookup pathLookup) { - Stream result = pathLookup.settingResolver().apply(setting); - if (ignoreUrl) { - result = result.filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false); - } - return result.map(pathLookup.configDir()::resolve); + Stream result = pathLookup.settingResolver() + .apply(setting) + .filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false); + return result.map(Path::of); } @Override @@ -243,7 +242,7 @@ public FileData withPlatform(Platform platform) { if (platform == platform()) { return this; } - return new PathSettingFileData(setting, baseDir, mode, ignoreUrl, platform, exclusive); + return new PathSettingFileData(setting, baseDir, mode, platform, exclusive); } } @@ -331,8 +330,6 @@ public static FilesEntitlement build(List paths) { String settingBaseDirAsString = checkString.apply(file, "basedir_if_relative"); String modeAsString = checkString.apply(file, "mode"); String platformAsString = checkString.apply(file, "platform"); - Boolean ignoreUrlAsStringBoolean = checkBoolean.apply(file, "ignore_url"); - boolean ignoreUrlAsString = ignoreUrlAsStringBoolean != null && ignoreUrlAsStringBoolean; Boolean exclusiveBoolean = checkBoolean.apply(file, "exclusive"); boolean exclusive = exclusiveBoolean != null && exclusiveBoolean; @@ -359,9 +356,6 @@ public static FilesEntitlement build(List paths) { throw new PolicyValidationException("'relative_to' may only be used with 'relative_path'"); } - if (ignoreUrlAsStringBoolean != null && pathSetting == null) { - throw new PolicyValidationException("'ignore_url' may only be used with 'path_setting'"); - } if (settingBaseDirAsString != null && pathSetting == null) { throw new PolicyValidationException("'basedir_if_relative' may only be used with 'path_setting'"); } @@ -388,7 +382,7 @@ public static FilesEntitlement build(List paths) { throw new PolicyValidationException("files entitlement with a 'path_setting' must specify 'basedir_if_relative'"); } BaseDir baseDir = parseBaseDir(settingBaseDirAsString); - fileData = FileData.ofPathSetting(pathSetting, baseDir, mode, ignoreUrlAsString); + fileData = FileData.ofPathSetting(pathSetting, baseDir, mode); } else { throw new AssertionError("File entry validation error"); } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java index e051cc2479a73..7bc8e39fb1b27 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java @@ -101,25 +101,25 @@ public void testPathSettingResolve() { List.of(Map.of("path_setting", "foo.bar", "basedir_if_relative", "config", "mode", "read")) ); var filesData = entitlement.filesData(); - assertThat(filesData, contains(FileData.ofPathSetting("foo.bar", CONFIG, READ, false))); + assertThat(filesData, contains(FileData.ofPathSetting("foo.bar", CONFIG, READ))); - var fileData = FileData.ofPathSetting("foo.bar", CONFIG, READ, false); + var fileData = FileData.ofPathSetting("foo.bar", CONFIG, READ); // empty settings assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), empty()); - fileData = FileData.ofPathSetting("foo.bar", CONFIG, READ, false); + fileData = FileData.ofPathSetting("foo.bar", CONFIG, READ); settings = Settings.builder().put("foo.bar", "/setting/path").build(); assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/setting/path"))); - fileData = FileData.ofPathSetting("foo.*.bar", CONFIG, READ, false); + fileData = FileData.ofPathSetting("foo.*.bar", CONFIG, READ); settings = Settings.builder().put("foo.baz.bar", "/setting/path").build(); assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/setting/path"))); - fileData = FileData.ofPathSetting("foo.*.bar", CONFIG, READ, false); + fileData = FileData.ofPathSetting("foo.*.bar", CONFIG, READ); settings = Settings.builder().put("foo.baz.bar", "/setting/path").put("foo.baz2.bar", "/other/path").build(); assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), containsInAnyOrder(Path.of("/setting/path"), Path.of("/other/path"))); - fileData = FileData.ofPathSetting("foo.bar", CONFIG, READ, false); + fileData = FileData.ofPathSetting("foo.bar", CONFIG, READ); settings = Settings.builder().put("foo.bar", "relative_path").build(); assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/config/relative_path"))); } @@ -140,28 +140,6 @@ public void testPathSettingBasedirValidation() { assertThat(e.getMessage(), is("'basedir_if_relative' may only be used with 'path_setting'")); } - public void testPathSettingIgnoreUrl() { - var fileData = FileData.ofPathSetting("foo.*.bar", CONFIG, READ, true); - settings = Settings.builder().put("foo.nonurl.bar", "/setting/path").put("foo.url.bar", "https://mysite").build(); - assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/setting/path"))); - } - - public void testIgnoreUrlValidation() { - var e = expectThrows( - PolicyValidationException.class, - () -> FilesEntitlement.build(List.of(Map.of("path", "/foo", "mode", "read", "ignore_url", true))) - ); - assertThat(e.getMessage(), is("'ignore_url' may only be used with 'path_setting'")); - - e = expectThrows( - PolicyValidationException.class, - () -> FilesEntitlement.build( - List.of(Map.of("relative_path", "foo", "relative_to", "config", "mode", "read", "ignore_url", true)) - ) - ); - assertThat(e.getMessage(), is("'ignore_url' may only be used with 'path_setting'")); - } - public void testExclusiveParsing() throws Exception { Policy parsedPolicy = new PolicyParser(new ByteArrayInputStream(""" entitlement-module-name: