From 3faa3ff79b00b02bbb235201047345a2db21ea34 Mon Sep 17 00:00:00 2001 From: Lorenzo Dematte Date: Mon, 17 Feb 2025 10:45:23 +0100 Subject: [PATCH 1/3] Fix policy manager/parser tests to rely on real files; removed assert for absolute/relative that won't work with mixed windows/linux policies --- .../policy/entitlements/FilesEntitlement.java | 2 - .../runtime/policy/PolicyManagerTests.java | 22 ++--- .../policy/PolicyParserFailureTests.java | 29 ------ .../runtime/policy/PolicyParserTests.java | 89 ++++++++++++------- .../runtime/policy/test-policy.yaml | 4 - muted-tests.yml | 12 --- 6 files changed, 67 insertions(+), 91 deletions(-) delete mode 100644 libs/entitlement/src/test/resources/org/elasticsearch/entitlement/runtime/policy/test-policy.yaml 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 e9079948879eb..c2e0a79061488 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 @@ -120,12 +120,10 @@ public int hashCode() { } static FileData ofPath(Path path, Mode mode) { - assert path.isAbsolute(); return new AbsolutePathFileData(path, mode); } static FileData ofRelativePath(Path relativePath, BaseDir baseDir, Mode mode) { - assert relativePath.isAbsolute() == false; return new RelativePathFileData(relativePath, baseDir, mode); } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java index 90279230dbe17..f9a3c4cd9d273 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java @@ -53,17 +53,22 @@ public class PolicyManagerTests extends ESTestCase { */ private static Module NO_ENTITLEMENTS_MODULE; - private static final PathLookup TEST_PATH_LOOKUP = new PathLookup( - Path.of("/config"), - new Path[] { Path.of("/data1/"), Path.of("/data2") }, - Path.of("/temp") - ); + private static Path TEST_BASE_DIR; + + private static PathLookup TEST_PATH_LOOKUP; @BeforeClass public static void beforeClass() { try { // Any old module will do for tests using NO_ENTITLEMENTS_MODULE NO_ENTITLEMENTS_MODULE = makeClassInItsOwnModule().getModule(); + + TEST_BASE_DIR = createTempDir().toAbsolutePath(); + TEST_PATH_LOOKUP = new PathLookup( + TEST_BASE_DIR.resolve("/config"), + new Path[] { TEST_BASE_DIR.resolve("/data1/"), TEST_BASE_DIR.resolve("/data2") }, + TEST_BASE_DIR.resolve("/temp") + ); } catch (Exception e) { throw new IllegalStateException(e); } @@ -228,8 +233,7 @@ public void testGetEntitlementsReturnsEntitlementsForPluginModule() throws IOExc var entitlements = policyManager.getEntitlements(mockPluginClass); assertThat(entitlements.hasEntitlement(CreateClassLoaderEntitlement.class), is(true)); - // TODO: this can't work on Windows, we need to have the root be unknown - // assertThat(entitlements.fileAccess().canRead("/test/path"), is(true)); + assertThat(entitlements.fileAccess().canRead(TEST_BASE_DIR), is(true)); } public void testGetEntitlementsResultIsCached() { @@ -439,9 +443,7 @@ private static Policy createPluginPolicy(String... pluginModules) { name -> new Scope( name, List.of( - new FilesEntitlement( - List.of(FilesEntitlement.FileData.ofPath(Path.of("/test/path"), FilesEntitlement.Mode.READ)) - ), + new FilesEntitlement(List.of(FilesEntitlement.FileData.ofPath(TEST_BASE_DIR, FilesEntitlement.Mode.READ))), new CreateClassLoaderEntitlement() ) ) diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java index 924864d57b1cf..f64fe33158dd8 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java @@ -64,35 +64,6 @@ public void testEntitlementMissingDependentParameter() { ); } - public void testEntitlementRelativePathWhenAbsolute() { - PolicyParserException ppe = expectThrows(PolicyParserException.class, () -> new PolicyParser(new ByteArrayInputStream(""" - entitlement-module-name: - - files: - - path: test-path - mode: read - """.getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy()); - assertEquals( - "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] " - + "for entitlement type [files]: 'path' [test-path] must be absolute", - ppe.getMessage() - ); - } - - public void testEntitlementAbsolutePathWhenRelative() { - PolicyParserException ppe = expectThrows(PolicyParserException.class, () -> new PolicyParser(new ByteArrayInputStream(""" - entitlement-module-name: - - files: - - relative_path: /test-path - relative_to: data - mode: read - """.getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy()); - assertEquals( - "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] " - + "for entitlement type [files]: 'relative_path' [/test-path] must be relative", - ppe.getMessage() - ); - } - public void testEntitlementMutuallyExclusiveParameters() { PolicyParserException ppe = expectThrows(PolicyParserException.class, () -> new PolicyParser(new ByteArrayInputStream(""" entitlement-module-name: diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java index b27a29978eec7..1886f83b767be 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.entitlement.runtime.policy; +import org.elasticsearch.core.Strings; import org.elasticsearch.entitlement.runtime.policy.entitlements.CreateClassLoaderEntitlement; import org.elasticsearch.entitlement.runtime.policy.entitlements.Entitlement; import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement; @@ -18,18 +19,29 @@ import org.elasticsearch.entitlement.runtime.policy.entitlements.SetHttpsConnectionPropertiesEntitlement; import org.elasticsearch.entitlement.runtime.policy.entitlements.WriteSystemPropertiesEntitlement; import org.elasticsearch.test.ESTestCase; +import org.junit.BeforeClass; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import java.util.List; import java.util.Map; import java.util.Set; import static org.hamcrest.Matchers.equalTo; +@ESTestCase.WithoutSecurityManager public class PolicyParserTests extends ESTestCase { + public static String TEST_ABSOLUTE_PATH_TO_FILE; + + @BeforeClass + public static void beforeClass() throws IOException { + TEST_ABSOLUTE_PATH_TO_FILE = createTempFile().toAbsolutePath().toString(); + } + private static class TestWrongEntitlementName implements Entitlement {} public static class ManyConstructorsEntitlement implements Entitlement { @@ -79,15 +91,23 @@ public void testGetEntitlementTypeName() { ); } + private static InputStream createFilesTestPolicy() { + return new ByteArrayInputStream(Strings.format(""" + entitlement-module-name: + - files: + - path: "%s" + mode: "read_write" + """, TEST_ABSOLUTE_PATH_TO_FILE).getBytes(StandardCharsets.UTF_8)); + } + public void testPolicyBuilder() throws IOException { - Policy parsedPolicy = new PolicyParser(PolicyParserTests.class.getResourceAsStream("test-policy.yaml"), "test-policy.yaml", false) - .parsePolicy(); + Policy parsedPolicy = new PolicyParser(createFilesTestPolicy(), "test-policy.yaml", false).parsePolicy(); Policy expected = new Policy( "test-policy.yaml", List.of( new Scope( "entitlement-module-name", - List.of(FilesEntitlement.build(List.of(Map.of("path", "/test/path/to/file", "mode", "read_write")))) + List.of(FilesEntitlement.build(List.of(Map.of("path", TEST_ABSOLUTE_PATH_TO_FILE, "mode", "read_write")))) ) ) ); @@ -95,14 +115,13 @@ public void testPolicyBuilder() throws IOException { } public void testPolicyBuilderOnExternalPlugin() throws IOException { - Policy parsedPolicy = new PolicyParser(PolicyParserTests.class.getResourceAsStream("test-policy.yaml"), "test-policy.yaml", true) - .parsePolicy(); + Policy parsedPolicy = new PolicyParser(createFilesTestPolicy(), "test-policy.yaml", true).parsePolicy(); Policy expected = new Policy( "test-policy.yaml", List.of( new Scope( "entitlement-module-name", - List.of(FilesEntitlement.build(List.of(Map.of("path", "/test/path/to/file", "mode", "read_write")))) + List.of(FilesEntitlement.build(List.of(Map.of("path", TEST_ABSOLUTE_PATH_TO_FILE, "mode", "read_write")))) ) ) ); @@ -110,31 +129,27 @@ public void testPolicyBuilderOnExternalPlugin() throws IOException { } public void testParseFiles() throws IOException { - Policy policyWithOnePath = new PolicyParser(new ByteArrayInputStream(""" - entitlement-module-name: - - files: - - path: "/test/path/to/file" - mode: "read_write" - """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", false).parsePolicy(); + Policy policyWithOnePath = new PolicyParser(createFilesTestPolicy(), "test-policy.yaml", false).parsePolicy(); Policy expected = new Policy( "test-policy.yaml", List.of( new Scope( "entitlement-module-name", - List.of(FilesEntitlement.build(List.of(Map.of("path", "/test/path/to/file", "mode", "read_write")))) + List.of(FilesEntitlement.build(List.of(Map.of("path", TEST_ABSOLUTE_PATH_TO_FILE, "mode", "read_write")))) ) ) ); assertEquals(expected, policyWithOnePath); - Policy policyWithTwoPaths = new PolicyParser(new ByteArrayInputStream(""" + var testPathToReadDir = createTempDir(); + Policy policyWithTwoPaths = new PolicyParser(new ByteArrayInputStream(Strings.format(""" entitlement-module-name: - files: - - path: "/test/path/to/file" + - path: "%s" mode: "read_write" - - path: "/test/path/to/read-dir/" + - path: "%s" mode: "read" - """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", false).parsePolicy(); + """, TEST_ABSOLUTE_PATH_TO_FILE, testPathToReadDir).getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", false).parsePolicy(); expected = new Policy( "test-policy.yaml", List.of( @@ -143,8 +158,8 @@ public void testParseFiles() throws IOException { List.of( FilesEntitlement.build( List.of( - Map.of("path", "/test/path/to/file", "mode", "read_write"), - Map.of("path", "/test/path/to/read-dir/", "mode", "read") + Map.of("path", TEST_ABSOLUTE_PATH_TO_FILE, "mode", "read_write"), + Map.of("path", testPathToReadDir, "mode", "read") ) ) ) @@ -153,18 +168,24 @@ public void testParseFiles() throws IOException { ); assertEquals(expected, policyWithTwoPaths); - Policy policyWithMultiplePathsAndBaseDir = new PolicyParser(new ByteArrayInputStream(""" - entitlement-module-name: - - files: - - relative_path: "test/path/to/file" - relative_to: "data" - mode: "read_write" - - relative_path: "test/path/to/read-dir/" - relative_to: "config" - mode: "read" - - path: "/path/to/file" - mode: "read_write" - """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", false).parsePolicy(); + var relativePathToFile = Path.of("test/path/to/file").normalize(); + var relativePathToDir = Path.of("test/path/to/read-dir/").normalize(); + Policy policyWithMultiplePathsAndBaseDir = new PolicyParser( + new ByteArrayInputStream(Strings.format(""" + entitlement-module-name: + - files: + - relative_path: "%s" + relative_to: "data" + mode: "read_write" + - relative_path: "%s" + relative_to: "config" + mode: "read" + - path: "%s" + mode: "read_write" + """, relativePathToFile, relativePathToDir, TEST_ABSOLUTE_PATH_TO_FILE).getBytes(StandardCharsets.UTF_8)), + "test-policy.yaml", + false + ).parsePolicy(); expected = new Policy( "test-policy.yaml", List.of( @@ -173,9 +194,9 @@ public void testParseFiles() throws IOException { List.of( FilesEntitlement.build( List.of( - Map.of("relative_path", "test/path/to/file", "mode", "read_write", "relative_to", "data"), - Map.of("relative_path", "test/path/to/read-dir/", "mode", "read", "relative_to", "config"), - Map.of("path", "/path/to/file", "mode", "read_write") + Map.of("relative_path", relativePathToFile, "mode", "read_write", "relative_to", "data"), + Map.of("relative_path", relativePathToDir, "mode", "read", "relative_to", "config"), + Map.of("path", TEST_ABSOLUTE_PATH_TO_FILE, "mode", "read_write") ) ) ) diff --git a/libs/entitlement/src/test/resources/org/elasticsearch/entitlement/runtime/policy/test-policy.yaml b/libs/entitlement/src/test/resources/org/elasticsearch/entitlement/runtime/policy/test-policy.yaml deleted file mode 100644 index 2b5a4cfa783fe..0000000000000 --- a/libs/entitlement/src/test/resources/org/elasticsearch/entitlement/runtime/policy/test-policy.yaml +++ /dev/null @@ -1,4 +0,0 @@ -entitlement-module-name: - - files: - - path: "/test/path/to/file" - mode: "read_write" diff --git a/muted-tests.yml b/muted-tests.yml index 66d6bf1d45085..6f1269edfd182 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -326,18 +326,6 @@ tests: - class: org.elasticsearch.index.mapper.ShapeGeometryFieldMapperTests method: testCartesianBoundsBlockLoader issue: https://github.com/elastic/elasticsearch/issues/122661 -- class: org.elasticsearch.entitlement.runtime.policy.PolicyParserTests - method: testPolicyBuilderOnExternalPlugin - issue: https://github.com/elastic/elasticsearch/issues/122663 -- class: org.elasticsearch.entitlement.runtime.policy.PolicyParserTests - method: testParseFiles - issue: https://github.com/elastic/elasticsearch/issues/122664 -- class: org.elasticsearch.entitlement.runtime.policy.PolicyParserTests - method: testPolicyBuilder - issue: https://github.com/elastic/elasticsearch/issues/122665 -- class: org.elasticsearch.entitlement.runtime.policy.PolicyParserFailureTests - method: testEntitlementAbsolutePathWhenRelative - issue: https://github.com/elastic/elasticsearch/issues/122666 - class: org.elasticsearch.entitlement.qa.EntitlementsAllowedNonModularIT issue: https://github.com/elastic/elasticsearch/issues/122568 - class: org.elasticsearch.entitlement.qa.EntitlementsDeniedIT From 71a1e90ba05a1b8d4717344e8c0ebb0d21706832 Mon Sep 17 00:00:00 2001 From: Lorenzo Dematte Date: Mon, 17 Feb 2025 11:16:49 +0100 Subject: [PATCH 2/3] Fix path/string --- .../entitlement/runtime/policy/PolicyParserTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java index 1886f83b767be..2a5746fc88de1 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java @@ -141,7 +141,7 @@ public void testParseFiles() throws IOException { ); assertEquals(expected, policyWithOnePath); - var testPathToReadDir = createTempDir(); + String testPathToReadDir = createTempDir().toAbsolutePath().toString(); Policy policyWithTwoPaths = new PolicyParser(new ByteArrayInputStream(Strings.format(""" entitlement-module-name: - files: @@ -168,8 +168,8 @@ public void testParseFiles() throws IOException { ); assertEquals(expected, policyWithTwoPaths); - var relativePathToFile = Path.of("test/path/to/file").normalize(); - var relativePathToDir = Path.of("test/path/to/read-dir/").normalize(); + String relativePathToFile = Path.of("test/path/to/file").normalize().toString(); + String relativePathToDir = Path.of("test/path/to/read-dir/").normalize().toString(); Policy policyWithMultiplePathsAndBaseDir = new PolicyParser( new ByteArrayInputStream(Strings.format(""" entitlement-module-name: From eb09724d77f2d63293c903fa3acab9b698c114da Mon Sep 17 00:00:00 2001 From: Lorenzo Dematte Date: Mon, 17 Feb 2025 14:17:37 +0100 Subject: [PATCH 3/3] Fix quotes and \ --- .../runtime/policy/PolicyParserTests.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java index 2a5746fc88de1..9af743f817153 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java @@ -95,7 +95,7 @@ private static InputStream createFilesTestPolicy() { return new ByteArrayInputStream(Strings.format(""" entitlement-module-name: - files: - - path: "%s" + - path: '%s' mode: "read_write" """, TEST_ABSOLUTE_PATH_TO_FILE).getBytes(StandardCharsets.UTF_8)); } @@ -145,9 +145,9 @@ public void testParseFiles() throws IOException { Policy policyWithTwoPaths = new PolicyParser(new ByteArrayInputStream(Strings.format(""" entitlement-module-name: - files: - - path: "%s" + - path: '%s' mode: "read_write" - - path: "%s" + - path: '%s' mode: "read" """, TEST_ABSOLUTE_PATH_TO_FILE, testPathToReadDir).getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", false).parsePolicy(); expected = new Policy( @@ -174,13 +174,13 @@ public void testParseFiles() throws IOException { new ByteArrayInputStream(Strings.format(""" entitlement-module-name: - files: - - relative_path: "%s" + - relative_path: '%s' relative_to: "data" mode: "read_write" - - relative_path: "%s" + - relative_path: '%s' relative_to: "config" mode: "read" - - path: "%s" + - path: '%s' mode: "read_write" """, relativePathToFile, relativePathToDir, TEST_ABSOLUTE_PATH_TO_FILE).getBytes(StandardCharsets.UTF_8)), "test-policy.yaml",