Update sandbox with new rules from upstream

This upgrades groovy from 2.4.15 to 2.5.8.

CHANGELOG.md

@@ -1,3 +1,10 @@
+# 1.0.0
+
+## Improved
+
+- Added support for groovy 2.5.8
+- Relaxed the sandbox rules to allow more of the Java API to be used in the DSL. This includes some date, calendar, regex and collection APIs. 
+
 # 0.8.0
 
 ## Improved

build.gradle

@@ -33,7 +33,7 @@ apply from: "https://raw.githubusercontent.com/gocd/gocd-plugin-gradle-task-help
 
 gocdPlugin {
   id = 'cd.go.contrib.plugins.configrepo.groovy'
-  pluginVersion = '0.8.0'
+  pluginVersion = '1.0.0'
   goCdVersion = '18.3.0'
   name = 'GoCD Groovy Configuration plugin'
   description = 'GoCD pipelines and environments configuration in Groovy'
@@ -60,5 +60,12 @@ allprojects {
 subprojects {
   repositories {
     mavenCentral()
+    maven {
+      url = 'https://repo.jenkins-ci.org/releases'
+      content {
+        includeModule "org.kohsuke", 'groovy-sandbox'
+      }
+    }
   }
+
 }

cli/build.gradle

@@ -20,7 +20,7 @@ dependencies {
   compile project(':json')
   compile project(':dsl-validation')
   compile group: 'com.beust', name: 'jcommander', version: '1.78'
-  compile group: 'org.codehaus.groovy', name: 'groovy-all', version: '2.4.15'
+  compile group: 'org.codehaus.groovy', name: 'groovy', version: '2.5.8'
   compile group: 'org.hibernate.validator', name: 'hibernate-validator', version: '6.1.0.Final'
   compile group: 'org.glassfish', name: 'javax.el', version: '3.0.0'
 }

dsl/build.gradle

@@ -30,7 +30,7 @@ dependencies {
 
   compileOnly group: 'org.projectlombok', name: 'lombok', version: '1.18.8'
 
-  compile group: 'org.codehaus.groovy', name: 'groovy-all', version: '2.4.15'
+  compile group: 'org.codehaus.groovy', name: 'groovy', version: '2.5.8'
   compile group: 'com.fasterxml.jackson.core', name: 'jackson-annotations', version: '2.10.0'
   compile group: 'org.apache.commons', name: 'commons-lang3', version: '3.9'
   compile group: 'javax.validation', name: 'validation-api', version: '2.0.1.Final'
@@ -113,7 +113,7 @@ javadoc {
   options.overview('overview.html')
   options.showFromPublic()
 
-  options.links('https://docs.oracle.com/javase/9/docs/api', "http://docs.groovy-lang.org/docs/groovy-2.4.15/html/gapi")
+  options.links('https://docs.oracle.com/javase/9/docs/api', "http://docs.groovy-lang.org/docs/groovy-2.5.8/html/gapi")
   title = "GoCD Groovy DSL API ${version}"
   options.footer = 'Copyright (c) 2019 ThoughtWorks, Inc.'
 }

dsl/src/test/java/cd/go/contrib/plugins/configrepo/groovy/dsl/TestAllSignatures.java

@@ -50,7 +50,7 @@ void testJSONIndex() {
     }
 
     private static ScanResult scanResult = new ClassGraph()
-            .verbose()
+//            .verbose()
             .enableClassInfo()
             .enableMethodInfo()
             .ignoreClassVisibility()

gocd-groovy-dsl-config-plugin/build.gradle

@@ -25,7 +25,7 @@ dependencies {
   compile project(':cli')
   compile project(':groovy-export')
 
-  compile group: 'org.codehaus.groovy', name: 'groovy-all', version: '2.4.15'
+  compile group: 'org.codehaus.groovy', name: 'groovy', version: '2.5.8'
 
   extractedAtTopLevel project(':jar-class-loader')
 

groovy-export/build.gradle

@@ -20,7 +20,7 @@ apply plugin: 'maven-publish'
 dependencies {
   compile project(':dsl')
   compile project(':json')
-  compile group: 'org.codehaus.groovy', name: 'groovy-all', version: '2.4.15'
+  compile group: 'org.codehaus.groovy', name: 'groovy', version: '2.5.8'
 
   testCompile project(path: ':json', configuration: 'testArchives')
   testCompile project(':sandbox')

groovy-export/src/main/java/cd/go/contrib/plugins/configrepo/groovy/export/IndentedStringWriter.java

@@ -28,7 +28,7 @@
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
-import static groovy.json.StringEscapeUtils.escapeJava;
+import static org.apache.commons.lang3.StringEscapeUtils.escapeJava;
 import static org.apache.commons.lang3.StringUtils.*;
 
 public class IndentedStringWriter {

sandbox/build.gradle

@@ -15,26 +15,30 @@
  */
 
 dependencies {
-  compile group: 'org.codehaus.groovy', name: 'groovy-all', version: '2.4.15'
+  compile group: 'org.codehaus.groovy', name: 'groovy', version: '2.5.8'
+  compile group: 'org.codehaus.groovy', name: 'groovy-sql', version: '2.5.8'
+  compile group: 'org.codehaus.groovy', name: 'groovy-swing', version: '2.5.8'
+  compile group: 'org.codehaus.groovy', name: 'groovy-dateutil', version: '2.5.8'
+  compile group: 'org.codehaus.groovy', name: 'groovy-xml', version: '2.5.8'
+
+  compile group: 'com.github.ben-manes.caffeine', name: 'caffeine', version: '2.8.0'
   compile group: 'org.apache.commons', name: 'commons-lang3', version: '3.9'
 
-  compile(group: 'org.kohsuke', name: 'groovy-sandbox', version: '1.19') {
+  compile(group: 'org.kohsuke', name: 'groovy-sandbox', version: '1.24') {
     exclude(group: 'org.codehaus.groovy')
   }
-  compile group: 'org.codehaus.groovy', name: 'groovy-all', version: '2.4.15'
-  compile(group: 'com.google.guava', name: 'guava', version: '28.1-jre')
+  compile group: 'com.google.guava', name: 'guava', version: '28.1-jre'
 
+  testCompile group: 'org.codehaus.groovy', name: 'groovy-all', version: '2.5.8'
   testCompile group: 'junit', name: 'junit', version: '4.12'
+  // for @Grab annotation
+  testCompile group: 'org.apache.ivy', name: 'ivy', version: '2.4.0'
   testCompile group: 'org.hamcrest', name: 'hamcrest-core', version: '2.2'
   testCompile group: 'org.hamcrest', name: 'hamcrest-library', version: '2.2'
   testRuntime group: 'org.junit.vintage', name: 'junit-vintage-engine', version: '5.5.2'
 }
 
 tasks.withType(JavaCompile) {
-  options.deprecation = true
-  options.encoding = 'utf-8'
-  options.debug = true
-  options.warnings = false
   // since this code that we copied from elsewhere, we disable all lint
   options.compilerArgs << "-Xlint:-unchecked"
 }

sandbox/src/main/java/cd/go/contrib/plugins/configrepo/groovy/sandbox/GroovyCallSiteSelector.java

@@ -29,6 +29,8 @@
 import org.apache.commons.lang3.ClassUtils;
 import org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
@@ -39,13 +41,15 @@
 
 /**
  * Assists in determination of which method or other JVM element is actually about to be called by Groovy.
- * Most of this just duplicates what {@link java.lang.invoke.MethodHandles.Lookup} and {@link java.lang.invoke.MethodHandle#asType} do,
+ * Most of this just duplicates what {@link java.lang.invoke.MethodHandles.Lookup} and {@link
+ * java.lang.invoke.MethodHandle#asType} do,
  * but {@link org.codehaus.groovy.vmplugin.v7.TypeTransformers} shows that there are Groovy-specific complications.
- * Comments in https://github.com/kohsuke/groovy-sandbox/issues/7 note that it would be great for the sandbox itself to just tell us what the call site is so we would not have to guess.
+ * Comments in https://github.com/kohsuke/groovy-sandbox/issues/7 note that it would be great for the sandbox itself to
+ * just tell us what the call site is so we would not have to guess.
  */
 class GroovyCallSiteSelector {
 
-    private static boolean matches(Class<?>[] parameterTypes, Object[] parameters, boolean varargs) {
+    private static boolean matches(@Nonnull Class<?>[] parameterTypes, @Nonnull Object[] parameters, boolean varargs) {
         if (varargs) {
             parameters = parametersForVarargs(parameterTypes, parameters);
         }
@@ -67,8 +71,8 @@ private static boolean matches(Class<?>[] parameterTypes, Object[] parameters, b
             }
             if (
                     parameterTypes[i].isPrimitive()
-                    && parameters[i] != null
-                    && isInstancePrimitive(ClassUtils.primitiveToWrapper(parameterTypes[i]), parameters[i])
+                            && parameters[i] != null
+                            && isInstancePrimitive(ClassUtils.primitiveToWrapper(parameterTypes[i]), parameters[i])
             ) {
                 // Groovy passes primitive values as objects (for example, passes 0 as Integer(0))
                 // The prior test fails as int.class.isInstance(new Integer(0)) returns false.
@@ -86,7 +90,8 @@ && isInstancePrimitive(ClassUtils.primitiveToWrapper(parameterTypes[i]), paramet
     }
 
     /**
-     * Translates a method parameter list with varargs possibly spliced into the end into the actual parameters to be passed to the JVM call.
+     * Translates a method parameter list with varargs possibly spliced into the end into the actual parameters to be
+     * passed to the JVM call.
      */
     private static Object[] parametersForVarargs(Class<?>[] parameterTypes, Object[] parameters) {
         int fixedLen = parameterTypes.length - 1;
@@ -117,7 +122,7 @@ private static Object[] parametersForVarargs(Class<?>[] parameterTypes, Object[]
     /**
      * {@link Class#isInstance} extended to handle some important cases of primitive types.
      */
-    private static boolean isInstancePrimitive(Class<?> type, Object instance) {
+    private static boolean isInstancePrimitive(@Nonnull Class<?> type, @Nonnull Object instance) {
         if (type.isInstance(instance)) {
             return true;
         }
@@ -140,11 +145,16 @@ private static boolean isInstancePrimitive(Class<?> type, Object instance) {
     /**
      * Looks up the most general possible definition of a given method call.
      * Preferentially searches for compatible definitions in supertypes.
-     * @param receiver an actual receiver object
-     * @param method the method name
-     * @param args a set of actual arguments
+     *
+     * @param receiver
+     *         an actual receiver object
+     * @param method
+     *         the method name
+     * @param args
+     *         a set of actual arguments
      */
-    public static Method method(Object receiver, String method, Object[] args) {
+    public static @CheckForNull
+    Method method(@Nonnull Object receiver, @Nonnull String method, @Nonnull Object[] args) {
         for (Class<?> c : types(receiver)) {
             Method candidate = findMatchingMethod(c, method, args);
             if (candidate != null) {
@@ -160,7 +170,8 @@ public static Method method(Object receiver, String method, Object[] args) {
         return null;
     }
 
-    public static Constructor<?> constructor(Class<?> receiver, Object[] args) {
+    public static @CheckForNull
+    Constructor<?> constructor(@Nonnull Class<?> receiver, @Nonnull Object[] args) {
         Constructor<?>[] constructors = receiver.getDeclaredConstructors();
         Constructor<?> candidate = null;
         for (Constructor<?> c : constructors) {
@@ -188,11 +199,12 @@ public static Constructor<?> constructor(Class<?> receiver, Object[] args) {
         return null;
     }
 
-    public static Method staticMethod(Class<?> receiver, String method, Object[] args) {
+    public static @CheckForNull
+    Method staticMethod(@Nonnull Class<?> receiver, @Nonnull String method, @Nonnull Object[] args) {
         return findMatchingMethod(receiver, method, args);
     }
 
-    private static Method findMatchingMethod(Class<?> receiver, String method, Object[] args) {
+    private static Method findMatchingMethod(@Nonnull Class<?> receiver, @Nonnull String method, @Nonnull Object[] args) {
         Method candidate = null;
 
         for (Method m : receiver.getDeclaredMethods()) {
@@ -212,7 +224,7 @@ private static Method findMatchingMethod(Class<?> receiver, String method, Objec
     /**
      * Emulates, with some tweaks, {@link org.codehaus.groovy.reflection.ParameterTypes#isVargsMethod(Object[])}
      */
-    private static boolean isVarArgsMethod(Method m, Object[] args) {
+    private static boolean isVarArgsMethod(@Nonnull Method m, @Nonnull Object[] args) {
         if (m.isVarArgs()) {
             return true;
         }
@@ -241,7 +253,8 @@ private static boolean isVarArgsMethod(Method m, Object[] args) {
         return false;
     }
 
-    public static Field field(Object receiver, String field) {
+    public static @CheckForNull
+    Field field(@Nonnull Object receiver, @Nonnull String field) {
         for (Class<?> c : types(receiver)) {
             for (Field f : c.getDeclaredFields()) {
                 if (f.getName().equals(field)) {
@@ -252,7 +265,8 @@ public static Field field(Object receiver, String field) {
         return null;
     }
 
-    public static Field staticField(Class<?> receiver, String field) {
+    public static @CheckForNull
+    Field staticField(@Nonnull Class<?> receiver, @Nonnull String field) {
         for (Field f : receiver.getDeclaredFields()) {
             if (f.getName().equals(field)) {
                 return f;
@@ -261,12 +275,13 @@ public static Field staticField(Class<?> receiver, String field) {
         return null;
     }
 
-    private static Iterable<Class<?>> types(Object o) {
+    private static Iterable<Class<?>> types(@Nonnull Object o) {
         Set<Class<?>> types = new LinkedHashSet<Class<?>>();
         visitTypes(types, o.getClass());
         return types;
     }
-    private static void visitTypes(Set<Class<?>> types, Class<?> c) {
+
+    private static void visitTypes(@Nonnull Set<Class<?>> types, @Nonnull Class<?> c) {
         Class<?> s = c.getSuperclass();
         if (s != null) {
             visitTypes(types, s);
@@ -307,6 +322,7 @@ private static boolean isMoreSpecific(AccessibleObject more, Class<?>[] morePara
         return more.toString().compareTo(less.toString()) > 0;
     }
 
-    private GroovyCallSiteSelector() {}
+    private GroovyCallSiteSelector() {
+    }
 
 }

sandbox/src/main/java/cd/go/contrib/plugins/configrepo/groovy/sandbox/GroovySandbox.java

@@ -27,17 +27,22 @@
 import cd.go.contrib.plugins.configrepo.groovy.sandbox.whitelists.ClassLoaderWhitelist;
 import cd.go.contrib.plugins.configrepo.groovy.sandbox.whitelists.ProxyWhitelist;
 import cd.go.contrib.plugins.configrepo.groovy.sandbox.whitelists.Whitelist;
+import groovy.grape.GrabAnnotationTransformation;
 import groovy.lang.Script;
 import org.codehaus.groovy.control.CompilerConfiguration;
 import org.kohsuke.groovy.sandbox.GroovyInterceptor;
 import org.kohsuke.groovy.sandbox.SandboxTransformer;
 
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.concurrent.Callable;
 
 public class GroovySandbox {
 
     public static CompilerConfiguration createSecureCompilerConfiguration() {
         CompilerConfiguration cc = new CompilerConfiguration();
+        cc.addCompilationCustomizers(new RejectASTTransformsCustomizer());
+        cc.setDisabledGlobalASTTransformations(new HashSet<>(Collections.singletonList(GrabAnnotationTransformation.class.getName())));
         cc.addCompilationCustomizers(new SandboxTransformer());
         return cc;
     }