Cache `GroovySourceFileAllowlist` responses and remove entries from the default allowlist that are unrelated to `WithScriptDescriptor.WithScriptAllowlist` #829

dwnusbaum · 2024-01-09T18:04:01Z

GroovySourceFileAllowlist can be consulted many times, but the allowed files are effectively static and just depend on which plugins are installed. Once a file has been allowed once, we don't need to check all of the allowlists again.

Testing done

Submitter checklist

Give feedback

Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Options
Successfully updated the issue's project

There was an error updating the issue's project
Ensure that the pull request title represents the desired changelog entry

Ensure that the pull request title represents the desired changelog entry
Options
Successfully updated the issue's project

There was an error updating the issue's project
Please describe what you did

Please describe what you did
Options
Successfully updated the issue's project

There was an error updating the issue's project
Link to relevant issues in GitHub or Jira

Link to relevant issues in GitHub or Jira
Options
Successfully updated the issue's project

There was an error updating the issue's project
Link to relevant pull requests, esp. upstream and downstream changes

Link to relevant pull requests, esp. upstream and downstream changes
Options
Successfully updated the issue's project

There was an error updating the issue's project
Ensure you have provided tests - that demonstrates feature works or fixes the issue

Ensure you have provided tests - that demonstrates feature works or fixes the issue
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

GroovySourceFileAllowlist can be consulted many times, but the allowed files are effectively static and just depend on which plugins are installed. Once a file has been allowed once, we don't need to check all of the allowlists again.

…tDescriptor.WithScriptAllowlist

dwnusbaum · 2024-01-09T18:13:36Z

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/GroovySourceFileAllowlist.java

@@ -152,6 +148,22 @@ private static boolean endsWithIgnoreCase(String value, String suffix) {
        }
    }

+    @Extension
+    public static class AllowedGroovyResourcesCache {
+        private final Map<String, Boolean> cache = new ConcurrentHashMap<>();


At most there would be around 50 entries in the map if you used every single plugin that has Groovy source files, so I don't think we need anything fancy.

Actually IDK if that's right, let me see if we get here with Groovy's speculative class loading as well.

Never mind, we do really only get here for the weird Groovy DSL script files in Jenkins plugins or other attempts to directly load Groovy source files on the classpath. I guess that is obvious in hindsight, otherwise we would see a ton of log spam for otherwise non-existent classes.

(SandboxResolvingClassLoader in script-security on the other hand does see a bunch of class loading attempts for random non-existent classes, but I guess that has to do with overriding loadClass, which we do not do here.)

dwnusbaum · 2024-01-09T18:14:34Z