Remove ignoreUrl file setting property (#123718) (#123821)

rjernst · web-flow · commit 9875f4f1838f · 2025-03-03T07:39:18.000+11:00
Urls may make the FileAccessTree invalid. This commit removes the flag
for filtering urls, instead always filtering them.
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
@@ -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,30 +220,29 @@ 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<Path> resolveRelativePaths(PathLookup pathLookup) {
-            Stream<String> 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<String> result = pathLookup.settingResolver()
+                .apply(setting)
+                .filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false);
+            return result.map(Path::of);
         }
 
         @Override
         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<Object> 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<Object> 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<Object> 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
@@ -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:

Original file line number	Diff line number	Diff line change
`@@ -91,8 +91,8 @@ static FileData ofRelativePath(Path relativePath, BaseDir baseDir, Mode mode) {`
`91`	`91`	`return new RelativePathFileData(relativePath, baseDir, mode, null, false);`
`92`	`92`	`}`
`93`	`93`
`94`		`- static FileData ofPathSetting(String setting, BaseDir baseDir, Mode mode, boolean ignoreUrl) {`
`95`		`- return new PathSettingFileData(setting, baseDir, mode, ignoreUrl, null, false);`
	`94`	`+ static FileData ofPathSetting(String setting, BaseDir baseDir, Mode mode) {`
	`95`	`+ return new PathSettingFileData(setting, baseDir, mode, null, false);`
`96`	`96`	`}`
`97`	`97`
`98`	`98`	`/**`
`@@ -220,30 +220,29 @@ public FileData withPlatform(Platform platform) {`
`220`	`220`	`}`
`221`	`221`	`}`
`222`	`222`
`223`		`- private record PathSettingFileData(String setting, BaseDir baseDir, Mode mode, boolean ignoreUrl, Platform platform, boolean exclusive)`
	`223`	`+ private record PathSettingFileData(String setting, BaseDir baseDir, Mode mode, Platform platform, boolean exclusive)`
`224`	`224`	`implements`
`225`	`225`	`RelativeFileData {`
`226`	`226`
`227`	`227`	`@Override`
`228`	`228`	`public PathSettingFileData withExclusive(boolean exclusive) {`
`229`		`- return new PathSettingFileData(setting, baseDir, mode, ignoreUrl, platform, exclusive);`
	`229`	`+ return new PathSettingFileData(setting, baseDir, mode, platform, exclusive);`
`230`	`230`	`}`
`231`	`231`
`232`	`232`	`@Override`
`233`	`233`	`public Stream<Path> resolveRelativePaths(PathLookup pathLookup) {`
`234`		`- Stream<String> result = pathLookup.settingResolver().apply(setting);`
`235`		`- if (ignoreUrl) {`
`236`		`- result = result.filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false);`
`237`		`- }`
`238`		`- return result.map(pathLookup.configDir()::resolve);`
	`234`	`+ Stream<String> result = pathLookup.settingResolver()`
	`235`	`+ .apply(setting)`
	`236`	`+ .filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false);`
	`237`	`+ return result.map(Path::of);`
`239`	`238`	`}`
`240`	`239`
`241`	`240`	`@Override`
`242`	`241`	`public FileData withPlatform(Platform platform) {`
`243`	`242`	`if (platform == platform()) {`
`244`	`243`	`return this;`
`245`	`244`	`}`
`246`		`- return new PathSettingFileData(setting, baseDir, mode, ignoreUrl, platform, exclusive);`
	`245`	`+ return new PathSettingFileData(setting, baseDir, mode, platform, exclusive);`
`247`	`246`	`}`
`248`	`247`	`}`
`249`	`248`
`@@ -331,8 +330,6 @@ public static FilesEntitlement build(List<Object> paths) {`
`331`	`330`	`String settingBaseDirAsString = checkString.apply(file, "basedir_if_relative");`
`332`	`331`	`String modeAsString = checkString.apply(file, "mode");`
`333`	`332`	`String platformAsString = checkString.apply(file, "platform");`
`334`		`- Boolean ignoreUrlAsStringBoolean = checkBoolean.apply(file, "ignore_url");`
`335`		`- boolean ignoreUrlAsString = ignoreUrlAsStringBoolean != null && ignoreUrlAsStringBoolean;`
`336`	`333`	`Boolean exclusiveBoolean = checkBoolean.apply(file, "exclusive");`
`337`	`334`	`boolean exclusive = exclusiveBoolean != null && exclusiveBoolean;`
`338`	`335`
`@@ -359,9 +356,6 @@ public static FilesEntitlement build(List<Object> paths) {`
`359`	`356`	`throw new PolicyValidationException("'relative_to' may only be used with 'relative_path'");`
`360`	`357`	`}`
`361`	`358`
`362`		`- if (ignoreUrlAsStringBoolean != null && pathSetting == null) {`
`363`		`- throw new PolicyValidationException("'ignore_url' may only be used with 'path_setting'");`
`364`		`- }`
`365`	`359`	`if (settingBaseDirAsString != null && pathSetting == null) {`
`366`	`360`	`throw new PolicyValidationException("'basedir_if_relative' may only be used with 'path_setting'");`
`367`	`361`	`}`
`@@ -388,7 +382,7 @@ public static FilesEntitlement build(List<Object> paths) {`
`388`	`382`	`throw new PolicyValidationException("files entitlement with a 'path_setting' must specify 'basedir_if_relative'");`
`389`	`383`	`}`
`390`	`384`	`BaseDir baseDir = parseBaseDir(settingBaseDirAsString);`
`391`		`- fileData = FileData.ofPathSetting(pathSetting, baseDir, mode, ignoreUrlAsString);`
	`385`	`+ fileData = FileData.ofPathSetting(pathSetting, baseDir, mode);`
`392`	`386`	`} else {`
`393`	`387`	`throw new AssertionError("File entry validation error");`
`394`	`388`	`}`