Refactor before entitlements for testing (#129099)

* Support multiple plugin source paths

* Refactor: remove unncessary PathLookup method.

It's only called in one place, and there's no need to override it for testing.
Removing it just makes things simpler.

* Refactor: local var for pathLookup

* Fix bugs in test build info parsing

* Fix representative_class in test

* Move BridgeUtilTests.

Tests in org.elasticsearch.entitlement.bridge are going to be uniquely hard to
test once we patch the bridge into java.base, due to Java's prohibition on
split packages.

Let's just move this guy to another package.

* Upcast (?!) Java23EntitlementChecker to EntitlementChecker

* Empty TestPathLookup

* Create PolicyManager during bootstrap, allowing us to share initialization

* Use empty component path list instead of null

* Downcast to the class of the check method.

In our unit test, we have a mock checker that doesn't extend
EntitlementChecker, so downcasting to that would require us to needlessly
rework the unit test.

* Fix javadoc typos

Author: prdoyle

libs/entitlement/asm-provider/src/main/java/org/elasticsearch/entitlement/instrumentation/impl/InstrumenterImpl.java

@@ -37,10 +37,11 @@
 import static org.objectweb.asm.ClassWriter.COMPUTE_FRAMES;
 import static org.objectweb.asm.ClassWriter.COMPUTE_MAXS;
 import static org.objectweb.asm.Opcodes.ACC_STATIC;
+import static org.objectweb.asm.Opcodes.CHECKCAST;
 import static org.objectweb.asm.Opcodes.INVOKEINTERFACE;
 import static org.objectweb.asm.Opcodes.INVOKESTATIC;
 
-public class InstrumenterImpl implements Instrumenter {
+public final class InstrumenterImpl implements Instrumenter {
     private static final Logger logger = LogManager.getLogger(InstrumenterImpl.class);
 
     private final String getCheckerClassMethodDescriptor;
@@ -271,7 +272,8 @@ public void visitCode() {
         }
 
         private void pushEntitlementChecker() {
-            InstrumenterImpl.this.pushEntitlementChecker(mv);
+            mv.visitMethodInsn(INVOKESTATIC, handleClass, "instance", getCheckerClassMethodDescriptor, false);
+            mv.visitTypeInsn(CHECKCAST, checkMethod.className());
         }
 
         private void pushCallerClass() {
@@ -319,10 +321,7 @@ private void invokeInstrumentationMethod() {
                 true
             );
         }
-    }
 
-    protected void pushEntitlementChecker(MethodVisitor mv) {
-        mv.visitMethodInsn(INVOKESTATIC, handleClass, "instance", getCheckerClassMethodDescriptor, false);
     }
 
     record ClassFileInfo(String fileName, byte[] bytecodes) {}

libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java

@@ -14,9 +14,11 @@
 import com.sun.tools.attach.AttachNotSupportedException;
 import com.sun.tools.attach.VirtualMachine;
 
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.entitlement.initialization.EntitlementInitialization;
+import org.elasticsearch.entitlement.runtime.policy.PathLookup;
 import org.elasticsearch.entitlement.runtime.policy.PathLookupImpl;
 import org.elasticsearch.entitlement.runtime.policy.Policy;
 import org.elasticsearch.entitlement.runtime.policy.PolicyManager;
@@ -26,6 +28,7 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
@@ -38,20 +41,20 @@ public class EntitlementBootstrap {
      * calls to methods protected by entitlements from classes without a valid
      * policy will throw {@link org.elasticsearch.entitlement.runtime.api.NotEntitledException}.
      *
-     * @param serverPolicyPatch a policy with additional entitlements to patch the embedded server layer policy
-     * @param pluginPolicies a map holding policies for plugins (and modules), by plugin (or module) name.
-     * @param scopeResolver a functor to map a Java Class to the component and module it belongs to.
-     * @param settingResolver a functor to resolve a setting name pattern for one or more Elasticsearch settings.
-     * @param dataDirs       data directories for Elasticsearch
-     * @param sharedRepoDirs shared repository directories for Elasticsearch
-     * @param configDir      the config directory for Elasticsearch
-     * @param libDir         the lib directory for Elasticsearch
-     * @param modulesDir     the directory where Elasticsearch modules are
-     * @param pluginsDir     the directory where plugins are installed for Elasticsearch
-     * @param sourcePaths    a map holding the path to each plugin or module jars, by plugin (or module) name.
-     * @param tempDir        the temp directory for Elasticsearch
-     * @param logsDir        the log directory for Elasticsearch
-     * @param pidFile        path to a pid file for Elasticsearch, or {@code null} if one was not specified
+     * @param serverPolicyPatch            additional entitlements to patch the embedded server layer policy
+     * @param pluginPolicies               maps each plugin name to the corresponding {@link Policy}
+     * @param scopeResolver                a functor to map a Java Class to the component and module it belongs to.
+     * @param settingResolver              a functor to resolve a setting name pattern for one or more Elasticsearch settings.
+     * @param dataDirs                     data directories for Elasticsearch
+     * @param sharedRepoDirs               shared repository directories for Elasticsearch
+     * @param configDir                    the config directory for Elasticsearch
+     * @param libDir                       the lib directory for Elasticsearch
+     * @param modulesDir                   the directory where Elasticsearch modules are
+     * @param pluginsDir                   the directory where plugins are installed for Elasticsearch
+     * @param pluginSourcePaths            maps each plugin name to the location of that plugin's code
+     * @param tempDir                      the temp directory for Elasticsearch
+     * @param logsDir                      the log directory for Elasticsearch
+     * @param pidFile                      path to a pid file for Elasticsearch, or {@code null} if one was not specified
      * @param suppressFailureLogPackages   packages for which we do not need or want to log Entitlements failures
      */
     public static void bootstrap(
@@ -65,35 +68,33 @@ public static void bootstrap(
         Path libDir,
         Path modulesDir,
         Path pluginsDir,
-        Map<String, Path> sourcePaths,
+        Map<String, Collection<Path>> pluginSourcePaths,
         Path logsDir,
         Path tempDir,
-        Path pidFile,
+        @Nullable Path pidFile,
         Set<Package> suppressFailureLogPackages
     ) {
         logger.debug("Loading entitlement agent");
         if (EntitlementInitialization.initializeArgs != null) {
             throw new IllegalStateException("initialization data is already set");
         }
+        PathLookupImpl pathLookup = new PathLookupImpl(
+            getUserHome(),
+            configDir,
+            dataDirs,
+            sharedRepoDirs,
+            libDir,
+            modulesDir,
+            pluginsDir,
+            logsDir,
+            tempDir,
+            pidFile,
+            settingResolver
+        );
         EntitlementInitialization.initializeArgs = new EntitlementInitialization.InitializeArgs(
-            serverPolicyPatch,
-            pluginPolicies,
-            scopeResolver,
-            new PathLookupImpl(
-                getUserHome(),
-                configDir,
-                dataDirs,
-                sharedRepoDirs,
-                libDir,
-                modulesDir,
-                pluginsDir,
-                logsDir,
-                tempDir,
-                pidFile,
-                settingResolver
-            ),
-            sourcePaths,
-            suppressFailureLogPackages
+            pathLookup,
+            suppressFailureLogPackages,
+            createPolicyManager(pluginPolicies, pathLookup, serverPolicyPatch, scopeResolver, pluginSourcePaths)
         );
         exportInitializationToAgent();
         loadAgent(findAgentJar(), EntitlementInitialization.class.getName());
@@ -151,5 +152,24 @@ static String findAgentJar() {
         }
     }
 
+    private static PolicyManager createPolicyManager(
+        Map<String, Policy> pluginPolicies,
+        PathLookup pathLookup,
+        Policy serverPolicyPatch,
+        Function<Class<?>, PolicyManager.PolicyScope> scopeResolver,
+        Map<String, Collection<Path>> pluginSourcePaths
+    ) {
+        FilesEntitlementsValidation.validate(pluginPolicies, pathLookup);
+
+        return new PolicyManager(
+            HardcodedEntitlements.serverPolicy(pathLookup.pidFile(), serverPolicyPatch),
+            HardcodedEntitlements.agentEntitlements(),
+            pluginPolicies,
+            scopeResolver,
+            pluginSourcePaths,
+            pathLookup
+        );
+    }
+
     private static final Logger logger = LogManager.getLogger(EntitlementBootstrap.class);
 }

libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/FilesEntitlementsValidation.java renamed to libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/FilesEntitlementsValidation.java

@@ -7,7 +7,7 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
-package org.elasticsearch.entitlement.initialization;
+package org.elasticsearch.entitlement.bootstrap;
 
 import org.elasticsearch.core.Strings;
 import org.elasticsearch.entitlement.runtime.policy.FileAccessTree;
@@ -17,6 +17,7 @@
 
 import java.nio.file.Path;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -44,7 +45,7 @@ static void validate(Map<String, Policy> pluginPolicies, PathLookup pathLookup)
                     .map(x -> ((FilesEntitlement) x))
                     .findFirst();
                 if (filesEntitlement.isPresent()) {
-                    var fileAccessTree = FileAccessTree.withoutExclusivePaths(filesEntitlement.get(), pathLookup, null);
+                    var fileAccessTree = FileAccessTree.withoutExclusivePaths(filesEntitlement.get(), pathLookup, List.of());
                     validateReadFilesEntitlements(pluginPolicy.getKey(), scope.moduleName(), fileAccessTree, readAccessForbidden);
                     validateWriteFilesEntitlements(pluginPolicy.getKey(), scope.moduleName(), fileAccessTree, writeAccessForbidden);
                 }

libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/HardcodedEntitlements.java renamed to libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/HardcodedEntitlements.java

@@ -7,7 +7,7 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
-package org.elasticsearch.entitlement.initialization;
+package org.elasticsearch.entitlement.bootstrap;
 
 import org.elasticsearch.core.Booleans;
 import org.elasticsearch.entitlement.runtime.policy.Policy;

libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java

@@ -10,22 +10,17 @@
 package org.elasticsearch.entitlement.initialization;
 
 import org.elasticsearch.core.Booleans;
-import org.elasticsearch.core.Nullable;
 import org.elasticsearch.entitlement.bridge.EntitlementChecker;
 import org.elasticsearch.entitlement.runtime.policy.ElasticsearchEntitlementChecker;
 import org.elasticsearch.entitlement.runtime.policy.PathLookup;
-import org.elasticsearch.entitlement.runtime.policy.Policy;
 import org.elasticsearch.entitlement.runtime.policy.PolicyChecker;
 import org.elasticsearch.entitlement.runtime.policy.PolicyCheckerImpl;
 import org.elasticsearch.entitlement.runtime.policy.PolicyManager;
 
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
-import java.nio.file.Path;
-import java.util.Map;
 import java.util.Set;
-import java.util.function.Function;
 
 import static java.util.Objects.requireNonNull;
 
@@ -69,35 +64,23 @@ public static EntitlementChecker checker() {
      */
     public static void initialize(Instrumentation inst) throws Exception {
         // the checker _MUST_ be set before _any_ instrumentation is done
-        checker = initChecker(createPolicyManager());
+        checker = initChecker(initializeArgs.policyManager());
         initInstrumentation(inst);
     }
 
     /**
      * Arguments to {@link #initialize}. Since that's called in a static context from the agent,
      * we have no way to pass arguments directly, so we stuff them in here.
      *
-     * @param serverPolicyPatch
-     * @param pluginPolicies
-     * @param scopeResolver
      * @param pathLookup
-     * @param sourcePaths
      * @param suppressFailureLogPackages
+     * @param policyManager
      */
-    public record InitializeArgs(
-        @Nullable Policy serverPolicyPatch,
-        Map<String, Policy> pluginPolicies,
-        Function<Class<?>, PolicyManager.PolicyScope> scopeResolver,
-        PathLookup pathLookup,
-        Map<String, Path> sourcePaths,
-        Set<Package> suppressFailureLogPackages
-    ) {
+    public record InitializeArgs(PathLookup pathLookup, Set<Package> suppressFailureLogPackages, PolicyManager policyManager) {
         public InitializeArgs {
-            requireNonNull(pluginPolicies);
-            requireNonNull(scopeResolver);
             requireNonNull(pathLookup);
-            requireNonNull(sourcePaths);
             requireNonNull(suppressFailureLogPackages);
+            requireNonNull(policyManager);
         }
     }
 
@@ -110,22 +93,6 @@ private static PolicyCheckerImpl createPolicyChecker(PolicyManager policyManager
         );
     }
 
-    private static PolicyManager createPolicyManager() {
-        Map<String, Policy> pluginPolicies = initializeArgs.pluginPolicies();
-        PathLookup pathLookup = initializeArgs.pathLookup();
-
-        FilesEntitlementsValidation.validate(pluginPolicies, pathLookup);
-
-        return new PolicyManager(
-            HardcodedEntitlements.serverPolicy(pathLookup.pidFile(), initializeArgs.serverPolicyPatch()),
-            HardcodedEntitlements.agentEntitlements(),
-            pluginPolicies,
-            initializeArgs.scopeResolver(),
-            initializeArgs.sourcePaths(),
-            pathLookup
-        );
-    }
-
     /**
      * If bytecode verification is enabled, ensure these classes get loaded before transforming/retransforming them.
      * For these classes, the order in which we transform and verify them matters. Verification during class transformation is at least an

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

@@ -9,7 +9,6 @@
 
 package org.elasticsearch.entitlement.runtime.policy;
 
-import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.Strings;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement;
@@ -25,6 +24,7 @@
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -219,7 +219,7 @@ private static String[] buildUpdatedAndSortedExclusivePaths(
     FileAccessTree(
         FilesEntitlement filesEntitlement,
         PathLookup pathLookup,
-        Path componentPath,
+        Collection<Path> componentPaths,
         String[] sortedExclusivePaths,
         FileAccessTreeComparison comparison
     ) {
@@ -267,9 +267,7 @@ private static String[] buildUpdatedAndSortedExclusivePaths(
         pathLookup.getBaseDirPaths(TEMP).forEach(tempPath -> addPathAndMaybeLink.accept(tempPath, READ_WRITE));
         // TODO: this grants read access to the config dir for all modules until explicit read entitlements can be added
         pathLookup.getBaseDirPaths(CONFIG).forEach(configPath -> addPathAndMaybeLink.accept(configPath, Mode.READ));
-        if (componentPath != null) {
-            addPathAndMaybeLink.accept(componentPath, Mode.READ);
-        }
+        componentPaths.forEach(p -> addPathAndMaybeLink.accept(p, Mode.READ));
 
         // TODO: watcher uses javax.activation which looks for known mime types configuration, should this be global or explicit in watcher?
         Path jdk = Paths.get(System.getProperty("java.home"));
@@ -314,13 +312,13 @@ static FileAccessTree of(
         String moduleName,
         FilesEntitlement filesEntitlement,
         PathLookup pathLookup,
-        @Nullable Path componentPath,
+        Collection<Path> componentPaths,
         List<ExclusivePath> exclusivePaths
     ) {
         return new FileAccessTree(
             filesEntitlement,
             pathLookup,
-            componentPath,
+            componentPaths,
             buildUpdatedAndSortedExclusivePaths(componentName, moduleName, exclusivePaths, DEFAULT_COMPARISON),
             DEFAULT_COMPARISON
         );
@@ -332,9 +330,9 @@ static FileAccessTree of(
     public static FileAccessTree withoutExclusivePaths(
         FilesEntitlement filesEntitlement,
         PathLookup pathLookup,
-        @Nullable Path componentPath
+        Collection<Path> componentPaths
     ) {
-        return new FileAccessTree(filesEntitlement, pathLookup, componentPath, new String[0], DEFAULT_COMPARISON);
+        return new FileAccessTree(filesEntitlement, pathLookup, componentPaths, new String[0], DEFAULT_COMPARISON);
     }
 
     public boolean canRead(Path path) {

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PathLookup.java

@@ -32,7 +32,9 @@ enum BaseDir {
 
     Stream<Path> getBaseDirPaths(BaseDir baseDir);
 
-    Stream<Path> resolveRelativePaths(BaseDir baseDir, Path relativePath);
-
+    /**
+     * @return all paths obtained by resolving all values of the given setting under all
+     * paths of the given {@code baseDir}.
+     */
     Stream<Path> resolveSettingPaths(BaseDir baseDir, String settingName);
 }

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PathLookupImpl.java

@@ -66,11 +66,6 @@ public Stream<Path> getBaseDirPaths(BaseDir baseDir) {
         };
     }
 
-    @Override
-    public Stream<Path> resolveRelativePaths(BaseDir baseDir, Path relativePath) {
-        return getBaseDirPaths(baseDir).map(path -> path.resolve(relativePath));
-    }
-
     @Override
     public Stream<Path> resolveSettingPaths(BaseDir baseDir, String settingName) {
         List<Path> relativePaths = settingResolver.apply(settingName)