diff --git a/pom.xml b/pom.xml index 43065752..866f13c7 100644 --- a/pom.xml +++ b/pom.xml @@ -21,6 +21,7 @@ jenkinsci/${project.artifactId}-plugin true groovy-sandbox + false @@ -87,5 +88,11 @@ test-harness test + + org.awaitility + awaitility + 4.3.0 + test + diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java index 344c7153..27468af6 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java @@ -43,20 +43,24 @@ import org.codehaus.groovy.runtime.GStringImpl; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelistTest; -import static org.junit.Assert.*; -import org.junit.Test; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + import org.jvnet.hudson.test.Issue; -public class GroovyCallSiteSelectorTest { +class GroovyCallSiteSelectorTest { - @Test public void arrays() throws Exception { + @Test + void arrays() throws Exception { Method m = EnumeratingWhitelistTest.C.class.getDeclaredMethod("m", Object[].class); - assertEquals("literal call", m, GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[] {new Object[] {"a", "b"}})); - assertNull("we assume the interceptor has dealt with varargs", GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[]{"a", "b"})); - assertEquals("array cast", m, GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[] {new String[] {"a", "b"}})); + assertEquals(m, GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[] {new Object[] {"a", "b"}}), "literal call"); + assertNull(GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[]{"a", "b"}), "we assume the interceptor has dealt with varargs"); + assertEquals(m, GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[] {new String[] {"a", "b"}}), "array cast"); } - @Test public void overloads() throws Exception { + @Test + void overloads() throws Exception { PrintWriter receiver = new PrintWriter(new OutputStream() { @Override public void write(int b) throws IOException { @@ -69,7 +73,8 @@ public void write(int b) throws IOException { } @Issue("JENKINS-29541") - @Test public void methodsOnGString() throws Exception { + @Test + void methodsOnGString() throws Exception { GStringImpl gString = new GStringImpl(new Object[0], new String[] {"x"}); assertEquals(String.class.getMethod("substring", int.class), GroovyCallSiteSelector.method(gString, "substring", new Object[] {99})); assertEquals(GString.class.getMethod("getValues"), GroovyCallSiteSelector.method(gString, "getValues", new Object[0])); @@ -77,25 +82,29 @@ public void write(int b) throws IOException { } @Issue("JENKINS-31701") - @Test public void primitives() throws Exception { + @Test + void primitives() throws Exception { assertEquals(Primitives.class.getMethod("m1", long.class), GroovyCallSiteSelector.staticMethod(Primitives.class, "m1", new Object[] {Long.MAX_VALUE})); assertEquals(Primitives.class.getMethod("m1", long.class), GroovyCallSiteSelector.staticMethod(Primitives.class, "m1", new Object[] {99})); assertEquals(Primitives.class.getMethod("m2", long.class), GroovyCallSiteSelector.staticMethod(Primitives.class, "m2", new Object[] {Long.MAX_VALUE})); assertEquals(Primitives.class.getMethod("m2", int.class), GroovyCallSiteSelector.staticMethod(Primitives.class, "m2", new Object[] {99})); } + public static class Primitives { public static void m1(long x) {} public static void m2(int x) {} public static void m2(long x) {} } - @Test public void staticMethodsCannotBeOverridden() throws Exception { + @Test + void staticMethodsCannotBeOverridden() throws Exception { assertEquals(Jenkins.class.getMethod("getInstance"), GroovyCallSiteSelector.staticMethod(Jenkins.class, "getInstance", new Object[0])); assertEquals(Hudson.class.getMethod("getInstance"), GroovyCallSiteSelector.staticMethod(Hudson.class, "getInstance", new Object[0])); } @Issue("JENKINS-45117") - @Test public void constructorVarargs() throws Exception { + @Test + void constructorVarargs() throws Exception { assertEquals(EnvVars.class.getConstructor(), GroovyCallSiteSelector.constructor(EnvVars.class, new Object[0])); assertEquals(EnvVars.class.getConstructor(String[].class), GroovyCallSiteSelector.constructor(EnvVars.class, new Object[]{"x"})); List params = new ArrayList<>(); @@ -115,7 +124,7 @@ public static void m2(long x) {} @Issue("JENKINS-47159") @Test - public void varargsFailureCases() throws Exception { + void varargsFailureCases() { // If there's a partial match, we should get a ClassCastException final ClassCastException e = assertThrows(ClassCastException.class, () -> assertNull(GroovyCallSiteSelector.constructor(ParametersAction.class, @@ -128,7 +137,7 @@ public void varargsFailureCases() throws Exception { @Issue("JENKINS-37257") @Test - public void varargsArrayElementTypeMismatch() throws Exception { + void varargsArrayElementTypeMismatch() throws Exception { List l = Arrays.asList("a", "b", "c"); assertEquals(String.class.getMethod("join", CharSequence.class, Iterable.class), GroovyCallSiteSelector.staticMethod(String.class, "join", new Object[]{",", l})); diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyLanguageCoverageTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyLanguageCoverageTest.java index b4104b99..ae5d1d04 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyLanguageCoverageTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyLanguageCoverageTest.java @@ -2,151 +2,142 @@ import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.GenericWhitelist; -import org.junit.Ignore; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ErrorCollector; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; import org.jvnet.hudson.test.Issue; -public class GroovyLanguageCoverageTest { +class GroovyLanguageCoverageTest { - @Rule - public ErrorCollector errors = new ErrorCollector(); - - private void assertEvaluate(Whitelist whitelist, Object expected, String script) { - SandboxInterceptorTest.assertEvaluate(whitelist, expected, script, errors); - } - - @Ignore("This fails on m.\"fruit${bKey}\" returning null due to JENKINS-46327") + @Disabled("This fails on m.\"fruit${bKey}\" returning null due to JENKINS-46327") @Issue("JENKINS-46327") @Test - public void quotedIdentifiers() throws Exception { + void quotedIdentifiers() throws Exception { // See http://groovy-lang.org/syntax.html#_quoted_identifiers - assertEvaluate(new GenericWhitelist(), - true, - "def m = [fruitA: 'apple', fruitB: 'banana']\n" + - "assert m.fruitA == 'apple'\n" + - "assert m.'fruitA' == 'apple'\n" + - "assert m.\"fruitA\" == 'apple'\n" + - "assert m.'''fruitA''' == 'apple'\n" + - "assert m.\"\"\"fruitA\"\"\" == 'apple'\n" + - "assert m./fruitA/ == 'apple'\n" + - "assert m.$/fruitA/$ == 'apple'\n" + - "def bKey = 'B'\n" + - "assert m.\"fruit${bKey}\" == 'banana'\n" + - "assert m.\"\"\"fruit${bKey}\"\"\" == 'banana'" + - "assert m./fruit${bKey}/ == 'banana'\n" + - "assert m.$/fruit${bKey}/$ == 'banana'\n" + - "return true\n" - ); + Whitelist whitelist = new GenericWhitelist(); + SandboxInterceptorTest.assertEvaluate(whitelist, true, """ + def m = [fruitA: 'apple', fruitB: 'banana'] + assert m.fruitA == 'apple' + assert m.'fruitA' == 'apple' + assert m."fruitA" == 'apple' + assert m.'''fruitA''' == 'apple' + assert m.""\"fruitA""\" == 'apple' + assert m./fruitA/ == 'apple' + assert m.$/fruitA/$ == 'apple' + def bKey = 'B' + assert m."fruit${bKey}" == 'banana' + assert m.""\"fruit${bKey}""\" == 'banana'\ + assert m./fruit${bKey}/ == 'banana' + assert m.$/fruit${bKey}/$ == 'banana' + return true + """); } @Test - public void gStringWithClosure() throws Exception { + void gStringWithClosure() throws Exception { // see http://groovy-lang.org/syntax.html#_special_case_of_interpolating_closure_expressions - assertEvaluate(new GenericWhitelist(), - true, - "def number = 1\n" + - "def eagerGString = \"value == ${number}\"\n" + - "def lazyGString = \"value == ${ -> number }\"\n" + - "assert eagerGString == 'value == 1'\n" + - "assert lazyGString.toString() == 'value == 1'\n" + - "number = 2\n" + - "assert eagerGString == 'value == 1'\n" + - "assert lazyGString == 'value == 2'\n" + - "return true\n" - ); + Whitelist whitelist = new GenericWhitelist(); + SandboxInterceptorTest.assertEvaluate(whitelist, true, """ + def number = 1 + def eagerGString = "value == ${number}" + def lazyGString = "value == ${ -> number }" + assert eagerGString == 'value == 1' + assert lazyGString.toString() == 'value == 1' + number = 2 + assert eagerGString == 'value == 1' + assert lazyGString == 'value == 2' + return true + """); } @Test - public void arithmeticOperators() throws Exception { + void arithmeticOperators() throws Exception { // see http://groovy-lang.org/operators.html#_arithmetic_operators - assertEvaluate(new GenericWhitelist(), - true, - "assert 1 + 2 == 3\n" + - "assert 4 - 3 == 1\n" + - "assert 3 * 5 == 15\n" + - "assert 3 / 2 == 1.5\n" + - "assert 10 % 3 == 1\n" + - "assert 2 ** 3 == 8\n" + - "assert +3 == 3\n" + - "assert -4 == 0 - 4\n" + - "assert -(-1) == 1 \n" + - "def a = 2\n" + - "def b = a++ * 3\n" + - "assert a == 3 && b == 6\n" + - "def c = 3\n" + - "def d = c-- * 2\n" + - "assert c == 2 && d == 6\n" + - "def e = 1\n" + - "def f = ++e + 3\n" + - "assert e == 2 && f == 5\n" + - "def g = 4\n" + - "def h = --g + 1\n" + - "assert g == 3 && h == 4\n" + - "def a2 = 4\n" + - "a2 += 3\n" + - "assert a2 == 7\n" + - "def b2 = 5\n" + - "b2 -= 3\n" + - "assert b2 == 2\n" + - "def c2 = 5\n" + - "c2 *= 3\n" + - "assert c2 == 15\n" + - "def d2 = 10\n" + - "d2 /= 2\n" + - "assert d2 == 5\n" + - "def e2 = 10\n" + - "e2 %= 3\n" + - "assert e2 == 1\n" + - "def f2 = 3\n" + - "f2 **= 2\n" + - "assert f2 == 9\n" + - "return true\n" - ); + Whitelist whitelist = new GenericWhitelist(); + SandboxInterceptorTest.assertEvaluate(whitelist, true, """ + assert 1 + 2 == 3 + assert 4 - 3 == 1 + assert 3 * 5 == 15 + assert 3 / 2 == 1.5 + assert 10 % 3 == 1 + assert 2 ** 3 == 8 + assert +3 == 3 + assert -4 == 0 - 4 + assert -(-1) == 1 \s + def a = 2 + def b = a++ * 3 + assert a == 3 && b == 6 + def c = 3 + def d = c-- * 2 + assert c == 2 && d == 6 + def e = 1 + def f = ++e + 3 + assert e == 2 && f == 5 + def g = 4 + def h = --g + 1 + assert g == 3 && h == 4 + def a2 = 4 + a2 += 3 + assert a2 == 7 + def b2 = 5 + b2 -= 3 + assert b2 == 2 + def c2 = 5 + c2 *= 3 + assert c2 == 15 + def d2 = 10 + d2 /= 2 + assert d2 == 5 + def e2 = 10 + e2 %= 3 + assert e2 == 1 + def f2 = 3 + f2 **= 2 + assert f2 == 9 + return true + """); } @Test - public void bigDecimalOperators() throws Exception { + void bigDecimalOperators() throws Exception { // see http://groovy-lang.org/operators.html#_arithmetic_operators - assertEvaluate(new GenericWhitelist(), - true, - "assert 1.0 + 2.0 == 3.0\n" + - "assert 4.0 - 3.0 == 1.0\n" + - "assert 3.0 * 5.0 == 15.0\n" + - "assert 3.0 / 2.0 == 1.5\n" + - "assert 2.0 ** 3.0 == 8.0\n" + - "assert +3.0 == 3.0\n" + - "assert -4.0 == 0 - 4.0\n" + - "assert -(-1.0) == 1.0\n" + - "def a = 2.0\n" + - "def b = a++ * 3.0\n" + - "assert a == 3.0 && b == 6.0\n" + - "def c = 3.0\n" + - "def d = c-- * 2.0\n" + - "assert c == 2.0 && d == 6.0\n" + - "def e = 1.0\n" + - "def f = ++e + 3.0\n" + - "assert e == 2.0 && f == 5.0\n" + - "def g = 4.0\n" + - "def h = --g + 1.0\n" + - "assert g == 3.0 && h == 4.0\n" + - "def a2 = 4.0\n" + - "a2 += 3.0\n" + - "assert a2 == 7.0\n" + - "def b2 = 5.0\n" + - "b2 -= 3.0\n" + - "assert b2 == 2.0\n" + - "def c2 = 5.0\n" + - "c2 *= 3.0\n" + - "assert c2 == 15.0\n" + - "def d2 = 10.0\n" + - "d2 /= 2.0\n" + - "assert d2 == 5.0\n" + - "def f2 = 3.0\n" + - "f2 **= 2.0\n" + - "assert f2 == 9.0\n" + - "return true\n" - ); + Whitelist whitelist = new GenericWhitelist(); + SandboxInterceptorTest.assertEvaluate(whitelist, true, """ + assert 1.0 + 2.0 == 3.0 + assert 4.0 - 3.0 == 1.0 + assert 3.0 * 5.0 == 15.0 + assert 3.0 / 2.0 == 1.5 + assert 2.0 ** 3.0 == 8.0 + assert +3.0 == 3.0 + assert -4.0 == 0 - 4.0 + assert -(-1.0) == 1.0 + def a = 2.0 + def b = a++ * 3.0 + assert a == 3.0 && b == 6.0 + def c = 3.0 + def d = c-- * 2.0 + assert c == 2.0 && d == 6.0 + def e = 1.0 + def f = ++e + 3.0 + assert e == 2.0 && f == 5.0 + def g = 4.0 + def h = --g + 1.0 + assert g == 3.0 && h == 4.0 + def a2 = 4.0 + a2 += 3.0 + assert a2 == 7.0 + def b2 = 5.0 + b2 -= 3.0 + assert b2 == 2.0 + def c2 = 5.0 + c2 *= 3.0 + assert c2 == 15.0 + def d2 = 10.0 + d2 /= 2.0 + assert d2 == 5.0 + def f2 = 3.0 + f2 **= 2.0 + assert f2 == 9.0 + return true + """); } } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyMemoryLeakTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyMemoryLeakTest.java index 8534ca5b..c04ea503 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyMemoryLeakTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyMemoryLeakTest.java @@ -11,33 +11,48 @@ import java.util.logging.Level; import org.codehaus.groovy.reflection.ClassInfo; import org.jenkinsci.plugins.scriptsecurity.scripts.ClasspathEntry; -import org.junit.After; -import static org.junit.Assert.*; -import org.junit.ClassRule; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.BuildWatcher; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.junit.jupiter.api.extension.RegisterExtension; import org.jvnet.hudson.test.JenkinsRule; -import org.jvnet.hudson.test.LoggerRule; +import org.jvnet.hudson.test.LogRecorder; import org.jvnet.hudson.test.MemoryAssert; +import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; /** * Tests for memory leak cleanup successfully purging the most common memory leak. */ -public class GroovyMemoryLeakTest { - @ClassRule - public static BuildWatcher buildWatcher = new BuildWatcher(); - @Rule - public JenkinsRule r = new JenkinsRule(); - @Rule public LoggerRule logger = new LoggerRule().record(SecureGroovyScript.class, Level.FINER); +@WithJenkins +class GroovyMemoryLeakTest { + + @SuppressWarnings("unused") + @RegisterExtension + private static final BuildWatcherExtension BUILD_WATCHER = new BuildWatcherExtension(); + + private static final List> LOADERS = new ArrayList<>(); - @After - public void clearLoaders() { + @SuppressWarnings("unused") + private final LogRecorder logger = new LogRecorder().record(SecureGroovyScript.class, Level.FINER); + + private JenkinsRule r; + + @BeforeEach + void beforeEach(JenkinsRule rule) { + r = rule; + } + + @AfterEach + void afterEach() { LOADERS.clear(); } - private static final List> LOADERS = new ArrayList<>(); - public static void register(Object o) { + @SuppressWarnings("unused") + private static void register(Object o) { System.err.println("registering " + o); for (ClassLoader loader = o.getClass().getClassLoader(); !(loader instanceof PluginManager.UberClassLoader); loader = loader.getParent()) { System.err.println("…from " + loader); @@ -46,7 +61,7 @@ public static void register(Object o) { } @Test - public void loaderReleased() throws Exception { + void loaderReleased() throws Exception { FreeStyleProject p = r.jenkins.createProject(FreeStyleProject.class, "p"); String cp = GroovyMemoryLeakTest.class.getResource("somejar.jar").toString(); p.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript( diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java index 2fb00d9b..66d3fb3b 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java @@ -41,7 +41,7 @@ import groovy.text.Template; import groovy.transform.ASTTest; import hudson.Functions; -import java.io.File; + import java.io.IOException; import java.lang.reflect.Constructor; import java.lang.reflect.Field; @@ -58,7 +58,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Objects; import java.util.Properties; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -72,11 +71,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; @@ -87,25 +82,24 @@ import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.ProxyWhitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.StaticWhitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted; -import org.junit.Ignore; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ErrorCollector; + +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; import org.jvnet.hudson.test.Issue; import org.kohsuke.groovy.sandbox.impl.Checker.SuperConstructorWrapper; import org.kohsuke.groovy.sandbox.impl.Checker.ThisConstructorWrapper; public class SandboxInterceptorTest { - @Rule public ErrorCollector errors = new ErrorCollector(); - - @Test public void genericWhitelist() throws Exception { + @Test + void genericWhitelist() throws Exception { assertEvaluate(new GenericWhitelist(), 3, "'foo bar baz'.split(' ').length"); assertEvaluate(new GenericWhitelist(), false, "def x = null; x != null"); } /** Checks that {@link GString} is handled sanely. */ - @Test public void testGString() throws Exception { + @Test + void testGString() throws Exception { String clazz = Clazz.class.getName(); String script = "def x = 1; new " + clazz + "().method(\"foo${x}\")"; String expected = "-foo1"; @@ -114,7 +108,8 @@ public class SandboxInterceptorTest { } /** Checks that methods specifically expecting {@link GString} also work. */ - @Test public void testGString2() throws Exception { + @Test + void testGString2() throws Exception { String clazz = Clazz.class.getName(); String script = "def x = 1; def c = new " + clazz + "(); c.quote(\"-${c.specialize(x)}-${x}-\")"; String expected = "-1-'1'-"; @@ -123,7 +118,8 @@ public class SandboxInterceptorTest { } @Issue("JENKINS-29541") - @Test public void substringGString() throws Exception { + @Test + void substringGString() throws Exception { assertEvaluate(new GenericWhitelist(), "hell", "'hello world'.substring(0, 4)"); assertEvaluate(new GenericWhitelist(), "hell", "def place = 'world'; \"hello ${place}\".substring(0, 4)"); } @@ -131,7 +127,8 @@ public class SandboxInterceptorTest { /** * Tests the proper interception of builder-like method. */ - @Test public void invokeMethod() throws Exception { + @Test + void invokeMethod() throws Exception { String script = "def builder = new groovy.json.JsonBuilder(); builder.point { x 5; y 3; }; builder.toString()"; String expected = "{\"point\":{\"x\":5,\"y\":3}}"; assertEvaluate(new BlanketWhitelist(), expected, script); @@ -159,12 +156,13 @@ public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @ final RejectedAccessException x = assertThrows(RejectedAccessException.class, () -> evaluate(new ProxyWhitelist(), "class Real {}; def real = new Real(); real.nonexistent(42)")); final String message = x.getMessage(); - assertEquals(message, "method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object", x.getSignature()); - assertTrue(message, message.contains("Real nonexistent java.lang.Integer")); + assertEquals("method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object", x.getSignature(), message); + assertTrue(message.contains("Real nonexistent java.lang.Integer"), message); } - @Ignore("TODO there are various unhandled cases, such as Closure → SAM, or numeric conversions, or number → String, or boxing/unboxing.") - @Test public void testNumbers() throws Exception { + @Disabled("TODO there are various unhandled cases, such as Closure → SAM, or numeric conversions, or number → String, or boxing/unboxing.") + @Test + void testNumbers() throws Exception { String clazz = Clazz.class.getName(); String script = "int x = 1; " + clazz + ".incr(x)"; Long expected = 2L; @@ -174,14 +172,16 @@ public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @ assertEvaluate(new StaticWhitelist("staticMethod " + clazz + " incr java.lang.Long"), expected, script); } - @Test public void staticFields() throws Exception { + @Test + void staticFields() throws Exception { String clazz = Clazz.class.getName(); assertEvaluate(new StaticWhitelist("staticField " + clazz + " flag"), true, clazz + ".flag=true"); assertTrue(Clazz.flag); } @Issue("JENKINS-34599") - @Test public void finalFields() throws Exception { + @Test + void finalFields() { // Control cases: non-final fields. assertEvaluate(new ProxyWhitelist(), 99, "class X {int x = 99}; new X().x"); assertEvaluate(new ProxyWhitelist(), 99, "class X {int x; X(int x) {this.x = x}}; new X(99).x"); @@ -201,26 +201,21 @@ public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @ // Control case: initialization expressions themselves are checked. assertRejected(new ProxyWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", "class X {Object x = jenkins.model.Jenkins.instance}; new X().x"); assertRejected(new ProxyWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", "class X {Object x; {x = jenkins.model.Jenkins.instance}}; new X().x"); - try { - errors.checkThat(evaluate(new ProxyWhitelist(), "class X {static Object x = jenkins.model.Jenkins.instance}; X.x"), is("should be rejected")); - } catch (ExceptionInInitializerError x) { - errors.checkThat(x.getMessage(), ((RejectedAccessException) x.getCause()).getSignature(), is("staticMethod jenkins.model.Jenkins getInstance")); - } catch (Throwable t) { - errors.addError(t); - } - try { - errors.checkThat(evaluate(new ProxyWhitelist(), "class X {static Object x; static {x = jenkins.model.Jenkins.instance}}; X.x"), is("should be rejected")); - } catch (ExceptionInInitializerError x) { - errors.checkThat(x.getMessage(), ((RejectedAccessException) x.getCause()).getSignature(), is("staticMethod jenkins.model.Jenkins getInstance")); - } catch (Throwable t) { - errors.addError(t); - } + + ExceptionInInitializerError x = assertThrows(ExceptionInInitializerError.class, + () -> evaluate(new ProxyWhitelist(), "class X {static Object x = jenkins.model.Jenkins.instance}; X.x")); + assertThat(x.getMessage(), ((RejectedAccessException) x.getCause()).getSignature(), is("staticMethod jenkins.model.Jenkins getInstance")); + + assertThrows(ExceptionInInitializerError.class, () -> evaluate(new ProxyWhitelist(), "class X {static Object x; static {x = jenkins.model.Jenkins.instance}}; X.x")); + assertThat(x.getMessage(), ((RejectedAccessException) x.getCause()).getSignature(), is("staticMethod jenkins.model.Jenkins getInstance")); + // Control case: when there is no backing field, we should not allow setters to be called. String sps = SafePerSe.class.getName(); assertRejected(new AnnotatedWhitelist(), "method " + sps + " setSecure boolean", "class X extends " + sps + " {X() {this.secure = false}}; new X()"); } - @Test public void propertiesAndGettersAndSetters() throws Exception { + @Test + void propertiesAndGettersAndSetters() throws Exception { String clazz = Clazz.class.getName(); assertEvaluate(new StaticWhitelist("new " + clazz, "field " + clazz + " prop"), "default", "new " + clazz + "().prop"); assertEvaluate(new StaticWhitelist("new " + clazz, "method " + clazz + " getProp"), "default", "new " + clazz + "().prop"); @@ -258,14 +253,17 @@ public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @ public static final class Clazz { static boolean flag; - @Whitelisted public Clazz() {} - @Whitelisted public String method(String x) {return "-" + x;} - @Whitelisted Special specialize(Object o) { + @Whitelisted + public Clazz() {} + @Whitelisted + public String method(String x) {return "-" + x;} + @Whitelisted + Special specialize(Object o) { return new Special(o); } - @Whitelisted String quote(Object o) { - if (o instanceof GString) { - GString gs = (GString) o; + @Whitelisted + String quote(Object o) { + if (o instanceof GString gs) { Object[] values = gs.getValues(); for (int i = 0; i < values.length; i++) { if (values[i] instanceof Special) { @@ -282,7 +280,8 @@ public static final class Clazz { private String quoteSingle(Object o) { return "'" + o + "'"; } - @Whitelisted static long incr(long x) { + @Whitelisted + static long incr(long x) { return x + 1; } private String prop = "default"; @@ -321,7 +320,8 @@ public String rawProp5() { } } - @Test public void dynamicProperties() throws Exception { + @Test + void dynamicProperties() throws Exception { String dynamic = Dynamic.class.getName(); String ctor = "new " + dynamic; String getProperty = "method groovy.lang.GroovyObject getProperty java.lang.String"; @@ -334,27 +334,27 @@ public String rawProp5() { public static final class Dynamic extends GroovyObjectSupport { private final Map values = new HashMap<>(); - @Override public Object getProperty(String n) { + @Override + public Object getProperty(String n) { return values.get(n); } - @Override public void setProperty(String n, Object v) { + @Override + public void setProperty(String n, Object v) { values.put(n, v); } } - @Test public void mapProperties() throws Exception { + @Test + void mapProperties() throws Exception { assertEvaluate(new GenericWhitelist(), 42, "def m = [:]; m.answer = 42; m.answer"); } - public static final class Special { - final Object o; - Special(Object o) { - this.o = o; - } + public record Special(Object o) { } @Issue({"JENKINS-25119", "JENKINS-27725", "JENKINS-57299"}) - @Test public void defaultGroovyMethods() throws Exception { + @Test + void defaultGroovyMethods() throws Exception { assertRejected(new ProxyWhitelist(), "staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toInteger java.lang.String", "'123'.toInteger();"); assertEvaluate(new GenericWhitelist(), 123, "'123'.toInteger();"); assertEvaluate(new GenericWhitelist(), Arrays.asList(1, 4, 9), "([1, 2, 3] as int[]).collect({x -> x * x})"); @@ -375,7 +375,8 @@ public static final class Special { assertEvaluate(new GenericWhitelist(), Arrays.asList(3, 4), "[1, 2, 3, 4].takeRight(2)"); } - @Test public void whitelistedIrrelevantInsideScript() throws Exception { + @Test + void whitelistedIrrelevantInsideScript() { String clazz = Unsafe.class.getName(); String wl = Whitelisted.class.getName(); // @Whitelisted does not grant us access to anything new: @@ -387,7 +388,8 @@ public static final class Special { } @Issue("JENKINS-34741") - @Test public void structConstructor() throws Exception { + @Test + void structConstructor() throws Exception { assertEvaluate(new StaticWhitelist(), "ok", "class C {String f}; new C(f: 'ok').f"); // Map literals are equivalent to the named argument syntax. assertEvaluate(new StaticWhitelist(), "ok", "class C {String f}; new C([f: 'ok']).f"); @@ -395,17 +397,20 @@ public static final class Special { assertEvaluate(new StaticWhitelist(), "ok", "class C {String f}; def map = [f: 'ok']; new C(map).f"); // Make sure that we do not assign properties more than once. assertEvaluate(new StaticWhitelist(), 2, - "class Global { static int x = 0 }\n" + - "class C { def y; def setY(def y) { Global.x += y } }\n" + - "new C(y: 2); Global.x"); + """ + class Global { static int x = 0 } + class C { def y; def setY(def y) { Global.x += y } } + new C(y: 2); Global.x"""); // Make sure that we do not instantiate the object more than once. assertEvaluate(new StaticWhitelist(), 1, - "class Global { static int x = 0 }\n" + - "class C { def y; C() { Global.x += 1 } }\n" + - "new C(y: 2); Global.x"); + """ + class Global { static int x = 0 } + class C { def y; C() { Global.x += 1 } } + new C(y: 2); Global.x"""); } - @Test public void defSyntax() throws Exception { + @Test + void defSyntax() throws Exception { String clazz = Unsafe.class.getName(); Whitelist w = new ProxyWhitelist(new AnnotatedWhitelist(), /* for some reason def syntax triggers this */new StaticWhitelist("method java.util.Collection toArray")); assertEvaluate(w, "ok", "m(); def m() {" + clazz + ".ok()}"); @@ -421,18 +426,19 @@ private Unsafe() {} /** Expect errors from {@link org.codehaus.groovy.runtime.NullObject}. */ @Issue("kohsuke/groovy-sandbox #15") - @Test public void nullPointerException() throws Exception { + @Test + void nullPointerException() { final NullPointerException e = assertThrows(NullPointerException.class, () -> evaluate(new ProxyWhitelist(), "def x = null; x.member")); - assertEquals(Functions.printThrowable(e), "Cannot get property 'member' on null object", e.getMessage()); + assertEquals("Cannot get property 'member' on null object", e.getMessage(), Functions.printThrowable(e)); final NullPointerException e2 = assertThrows(NullPointerException.class, () -> evaluate(new ProxyWhitelist(), "def x = null; x.member = 42")); - assertEquals(Functions.printThrowable(e2), "Cannot set property 'member' on null object", e2.getMessage()); + assertEquals("Cannot set property 'member' on null object", e2.getMessage(), Functions.printThrowable(e2)); final NullPointerException e3 = assertThrows(NullPointerException.class, () -> evaluate(new ProxyWhitelist(), "def x = null; x.member()")); - assertEquals(Functions.printThrowable(e3), "Cannot invoke method member() on null object", e3.getMessage()); + assertEquals("Cannot invoke method member() on null object", e3.getMessage(), Functions.printThrowable(e3)); } /** @@ -444,7 +450,8 @@ private Unsafe() {} * script-security understands this logic and checks access at the actual target of the proxy, so that Closures * can be used safely. */ - @Test public void closureDelegate() throws Exception { + @Test + void closureDelegate() throws Exception { ProxyWhitelist rules = new ProxyWhitelist(new StaticWhitelist( "new java.lang.Exception java.lang.String", "method java.util.concurrent.Callable call", @@ -496,7 +503,8 @@ private Unsafe() {} } } - @Test public void metaClassDelegate() throws Exception { + @Test + void metaClassDelegate() throws Exception { new GroovyShell().evaluate("String.metaClass.getAnswer = {-> return 42}"); // privileged operation assertEvaluate(new StaticWhitelist(), 42, "'existence'.getAnswer()"); assertEvaluate(new StaticWhitelist(), 42, "'existence'.answer"); @@ -504,14 +512,16 @@ private Unsafe() {} } @Issue("JENKINS-28277") - @Test public void curry() throws Exception { + @Test + void curry() throws Exception { assertEvaluate(new GenericWhitelist(), 'h', "def charAt = {idx, str -> str.charAt(idx)}; def firstChar = charAt.curry(0); firstChar 'hello'"); assertEvaluate(new GenericWhitelist(), 'h', "def charOfHello = 'hello'.&charAt; def firstCharOfHello = charOfHello.curry(0); firstCharOfHello()"); assertEvaluate(new GenericWhitelist(), 'h', "def charAt = {str, idx -> str.charAt(idx)}; def firstChar = charAt.ncurry(1, 0); firstChar 'hello'"); } @Issue("JENKINS-34739") - @Test public void varargs() throws Exception { + @Test + void varargs() throws Exception { // Control cases: ProxyWhitelist wl = new ProxyWhitelist(new GenericWhitelist(), new AnnotatedWhitelist()); assertEvaluate(wl, 0, "class UsesVarargs {static int len(String... vals) {vals.length}}; UsesVarargs.len(new String[0])"); @@ -539,6 +549,7 @@ private Unsafe() {} assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode()"); assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode('one', 'two', 'three')"); } + public static class UsesVarargs { @Whitelisted public static int len(String... vals) { @@ -558,7 +569,7 @@ public static int xlen(int x, String... vals) { } @Whitelisted public static String join(String sep, String... vals) { - return Stream.of(vals).map(v -> v == null ? "" : v.toString()).collect(Collectors.joining(sep)); + return Stream.of(vals).map(v -> v == null ? "" : v).collect(Collectors.joining(sep)); } public static void explode(String... vals) {} @Whitelisted @@ -568,12 +579,9 @@ public static String varargsMethod(Integer i, Boolean b, StringContainer... s) { } } - public static final class StringContainer { - final String o; - + public record StringContainer(String o) { @Whitelisted - public StringContainer(String o) { - this.o = o; + public StringContainer { } @Whitelisted @@ -583,7 +591,8 @@ public String toString() { } } - @Test public void templates() throws Exception { + @Test + void templates() throws Exception { final GroovyShell shell = new GroovyShell(GroovySandbox.createSecureCompilerConfiguration()); final Template t; try (GroovySandbox.Scope scope = new GroovySandbox().withWhitelist(new GenericWhitelist()).enter()) { @@ -594,17 +603,20 @@ public String toString() { new ProxyWhitelist(new StaticWhitelist("method java.lang.String toLowerCase"), new GenericWhitelist()))); } - @Test public void selfProperties() throws Exception { + @Test + void selfProperties() { assertEvaluate(new ProxyWhitelist(), true, "BOOL=true; BOOL"); } - @Test public void missingPropertyException() throws Exception { + @Test + void missingPropertyException() { final MissingPropertyException x = assertThrows(MissingPropertyException.class, () -> evaluate(new ProxyWhitelist(), "GOOP")); assertEquals("GOOP", x.getProperty()); } - @Test public void specialScript() throws Exception { + @Test + void specialScript() throws Exception { CompilerConfiguration cc = GroovySandbox.createSecureCompilerConfiguration(); cc.setScriptBaseClass(SpecialScript.class.getName()); GroovyShell shell = new GroovyShell(cc); @@ -632,7 +644,8 @@ public static abstract class SpecialScript extends Script { } @Issue("JENKINS-46757") - @Test public void properties() throws Exception { + @Test + void properties() throws Exception { String script = "def properties = new Properties()"; assertRejected(new StaticWhitelist(), "new java.util.Properties", script); assertEvaluate(new StaticWhitelist("new java.util.Properties"), new Properties(), script); @@ -642,7 +655,8 @@ public static abstract class SpecialScript extends Script { } @Issue({"SECURITY-566", "SECURITY-1353"}) - @Test public void typeCoercion() throws Exception { + @Test + void typeCoercion() throws Exception { assertRejected(new StaticWhitelist("staticMethod java.util.Locale getDefault"), "method java.util.Locale getCountry", "interface I {String getCountry()}; (Locale.getDefault() as I).getCountry()"); assertRejected(new StaticWhitelist("staticMethod java.util.Locale getDefault"), "method java.util.Locale getCountry", "interface I {String getCountry()}; (Locale.getDefault() as I).country"); assertRejected(new ProxyWhitelist(), "staticMethod java.util.Locale getAvailableLocales", "interface I {Locale[] getAvailableLocales()}; (Locale as I).getAvailableLocales()"); @@ -659,7 +673,8 @@ public static abstract class SpecialScript extends Script { } @Issue("SECURITY-580") - @Test public void positionalConstructors() throws Exception { + @Test + void positionalConstructors() throws Exception { assertRejected(new ProxyWhitelist(), "new java.lang.Exception java.lang.String", "['true'] as Exception"); assertEvaluate(new StaticWhitelist("new java.lang.Exception java.lang.String", "method java.lang.Throwable getMessage"), "true", "(['true'] as Exception).getMessage()"); @@ -691,12 +706,14 @@ public static abstract class SpecialScript extends Script { } @Issue("kohsuke/groovy-sandbox #16") - @Test public void infiniteLoop() throws Exception { + @Test + void infiniteLoop() { assertEvaluate(new BlanketWhitelist(), "abc", "def split = 'a b c'.split(' '); def b = new StringBuilder(); for (i = 0; i < split.length; i++) {b.append(split[i])}; b.toString()"); } @Issue("JENKINS-25118") - @Test public void primitiveTypes() throws Exception { + @Test + void primitiveTypes() throws Exception { // Some String operations: assertRejected(new ProxyWhitelist(), "method java.lang.CharSequence charAt int", "'123'.charAt(1);"); assertEvaluate(new StaticWhitelist("method java.lang.CharSequence charAt int"), '2', "'123'.charAt(1);"); @@ -711,9 +728,11 @@ public static abstract class SpecialScript extends Script { assertEvaluate(new GenericWhitelist(), "23", "'2' + 3"); } - @Test public void ambiguousOverloads() { - assertThrows("Ambiguous overload is an error in Groovy 2", GroovyRuntimeException.class, - () -> evaluate(new AnnotatedWhitelist(), Ambiguity.class.getName() + ".m(null)")); + @Test + void ambiguousOverloads() { + assertThrows(GroovyRuntimeException.class, + () -> evaluate(new AnnotatedWhitelist(), Ambiguity.class.getName() + ".m(null)"), + "Ambiguous overload is an error in Groovy 2"); } public static final class Ambiguity { @@ -721,11 +740,13 @@ public static final class Ambiguity { @Whitelisted public static boolean m(URL x) {return true;} } - @Test public void regexps() throws Exception { + @Test + void regexps() throws Exception { assertEvaluate(new GenericWhitelist(), "goodbye world", "def text = 'hello world'; def matcher = text =~ 'hello (.+)'; matcher ? \"goodbye ${matcher[0][1]}\" : 'fail'"); } - @Test public void splitAndJoin() throws Exception { + @Test + void splitAndJoin() throws Exception { assertEvaluate(new GenericWhitelist(), Collections.singletonMap("part0", "one\ntwo"), "def list = [['one', 'two']]; def map = [:]; for (int i = 0; i < list.size(); i++) {map[\"part${i}\"] = list.get(i).join(\"\\n\")}; map"); } @@ -736,7 +757,8 @@ public Object invokeMethod(String name, Object args) { } } - @Test public void invokeMethod_vs_DefaultGroovyMethods() throws Exception { + @Test + void invokeMethod_vs_DefaultGroovyMethods() throws Exception { // Closure defines the invokeMethod method, and asBoolean is defined on DefaultGroovyMethods. // the method dispatching in this case is that c.asBoolean() resolves to DefaultGroovyMethods.asBoolean() // and not invokeMethod("asBoolean") @@ -758,7 +780,8 @@ public Object invokeMethod(String name, Object args) { } @Issue({"JENKINS-42563", "SECURITY-582"}) - @Test public void superCalls() throws Exception { + @Test + void superCalls() throws Exception { String sps = SafePerSe.class.getName(); assertRejected(new AnnotatedWhitelist(), "method " + sps + " dangerous", "class C extends " + sps + " {void dangerous() {super.dangerous()}}; new C().dangerous()"); assertRejected(new AnnotatedWhitelist(), "method " + sps + " dangerous", "class C extends " + sps + " {void x() {super.dangerous()}}; new C().x()"); @@ -774,6 +797,7 @@ public Object invokeMethod(String name, Object args) { assertRejected(new StaticWhitelist("method java.lang.Object toString", "new java.lang.Exception java.lang.String"), "method java.lang.String toUpperCase", "class X6 extends Exception {X6(String x) {super(x.toUpperCase())}}; new X6('x')"); assertRejected(new StaticWhitelist("method java.lang.Object toString", "new java.lang.Exception"), "new java.lang.Object", "class X7 extends Exception {X7(String x) {new Object()}}; new X7('x')"); } + public static class SafePerSe { @Whitelisted public SafePerSe() {} @@ -781,42 +805,51 @@ public void dangerous() {} public void setSecure(boolean x) {} } - @Test public void keywordsAndOperators() throws Exception { + @Test + void keywordsAndOperators() throws Exception { String script = IOUtils.toString(this.getClass().getResourceAsStream("SandboxInterceptorTest/all.groovy"), StandardCharsets.UTF_8); assertEvaluate(new GenericWhitelist(), null, script); } @Issue("JENKINS-31234") - @Test public void calendarGetInstance() throws Exception { + @Test + void calendarGetInstance() throws Exception { assertEvaluate(new GenericWhitelist(), true, "Calendar.getInstance().get(Calendar.DAY_OF_MONTH) < 32"); assertEvaluate(new GenericWhitelist(), true, "Calendar.instance.get(Calendar.DAY_OF_MONTH) < 32"); } @Issue("JENKINS-31701") - @Test public void primitiveWidening() throws Exception { + @Test + void primitiveWidening() { assertEvaluate(new AnnotatedWhitelist(), 4L, SandboxInterceptorTest.class.getName() + ".usePrimitive(2)"); } - @Whitelisted public static long usePrimitive(long x) { + + @Whitelisted + public static long usePrimitive(long x) { return x + 2; } @Issue("JENKINS-32211") - @Test public void tokenize() throws Exception { + @Test + void tokenize() throws Exception { assertEvaluate(new GenericWhitelist(), 3, "'foo bar baz'.tokenize().size()"); assertEvaluate(new GenericWhitelist(), 3, "'foo bar baz'.tokenize(' ').size()"); assertEvaluate(new GenericWhitelist(), 3, "'foo bar baz'.tokenize('ba').size()"); } @Issue("JENKINS-33023") - @Test public void enums() throws Exception { - String script = "enum Thing {\n" - + " FIRST(\"The first thing\");\n" - + " String description;\n" - + " public Thing(String description) {\n" - + " this.description = description;\n" - + " }\n" - + "}\n" - + "Thing.values()[0].description\n"; + @Test + void enums() throws Exception { + String script = """ + enum Thing { + FIRST("The first thing"); + String description; + public Thing(String description) { + this.description = description; + } + } + Thing.values()[0].description + """; String expected = "The first thing"; assertEvaluate(new GenericWhitelist(), expected, script); String e = E.class.getName(); @@ -826,15 +859,16 @@ public void setSecure(boolean x) {} assertEvaluate(wl, "TWO", e + ".TWO.name()"); assertRejected(wl, "staticField " + e + " ONE", e + ".ONE.name()"); // Seems undesirable, but this is the current behavior. Requires new java.util.LinkedHashMap and staticMethod ImmutableASTTransformation checkPropNames. - errors.checkThrows(ExceptionInInitializerError.class, () -> evaluate(new GenericWhitelist(), + assertThrows(ExceptionInInitializerError.class, () -> evaluate(new GenericWhitelist(), "enum Thing { ONE, TWO }; Thing.ONE.toString()")); } + public enum E { ONE(1), @Whitelisted TWO(2); private final int n; - private E(int n) { + E(int n) { this.n = n; } @Whitelisted @@ -844,7 +878,8 @@ public int getN() { public void explode() {} } - @Test public void staticMethodsCannotBeOverridden() throws Exception { + @Test + void staticMethodsCannotBeOverridden() throws Exception { assertRejected(new StaticWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", "jenkins.model.Jenkins.getInstance()"); assertRejected(new StaticWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", "jenkins.model.Jenkins.instance"); assertRejected(new StaticWhitelist(), "staticMethod hudson.model.Hudson getInstance", "hudson.model.Hudson.getInstance()"); @@ -853,18 +888,22 @@ public void explode() {} @Issue("SECURITY-1266") @Test - public void blockedASTTransformsASTTest() throws Exception { + void blockedASTTransformsASTTest() { assertAnnotationBlocked(ASTTest.class, - "@groovy.transform.ASTTest(value={ throw new Exception('ASTTest should not have been executed!') })\n" + - "@Field int x\n"); + """ + @groovy.transform.ASTTest(value={ throw new Exception('ASTTest should not have been executed!') }) + @Field int x + """); } @Issue("SECURITY-1266") @Test - public void blockedASTTransformsGrab() throws Exception { + void blockedASTTransformsGrab() { assertAnnotationBlocked(Grab.class, - "@groovy.lang.Grab(group='foo', module='bar', version='1.0')\n" + - "def foo\n"); + """ + @groovy.lang.Grab(group='foo', module='bar', version='1.0') + def foo + """); } private static Object evaluate(Whitelist whitelist, String script) { @@ -902,58 +941,41 @@ private static Object evaluate(Whitelist whitelist, String script) { return actual; } - private void assertEvaluate(Whitelist whitelist, Object expected, String script) { - assertEvaluate(whitelist, expected, script, errors); - } + public static void assertEvaluate(Whitelist whitelist, Object expected, String script) { + Object actual = evaluate(whitelist, script); + assertThat(actual, is(expected)); - public static void assertEvaluate(Whitelist whitelist, Object expected, String script, ErrorCollector errors) { - try { - Object actual = evaluate(whitelist, script); - errors.checkThat(actual, is(expected)); - } catch (Throwable t) { - errors.addError(t); + Object value = new GroovyShell().evaluate(script); + if (value instanceof GString) { + value = value.toString(); } - try { - Object actual = new GroovyShell().evaluate(script); - if (actual instanceof GString) { - actual = actual.toString(); - } - errors.checkThat("control case", actual, is(expected)); - } catch (Throwable t) { - errors.addError(t); - } - } - - private void assertRejected(Whitelist whitelist, String expectedSignature, String script) { - assertRejected(whitelist, expectedSignature, script, errors); + assertThat("control case", value, is(expected)); } - public static void assertRejected(Whitelist whitelist, String expectedSignature, String script, ErrorCollector errors) { + public static void assertRejected(Whitelist whitelist, String expectedSignature, String script) { try { - Object actual = evaluate(whitelist, script); - errors.checkThat(actual, is((Object) "should be rejected")); + evaluate(whitelist, script); } catch (GroovyRuntimeException x) { // Exceptions during script parsing and instantiation are typically wrapped in GroovyRuntimeException, and // we cannot modify our code to directly throw the cause without breaking other plugins that only catch // GroovyRuntimeException. - if (x.getCause() instanceof RejectedAccessException) { - errors.checkThat(x.getMessage(), ((RejectedAccessException)x.getCause()).getSignature(), is(expectedSignature)); - } else { - errors.addError(x); - } + RejectedAccessException rae = assertInstanceOf(RejectedAccessException.class, x.getCause()); + assertThat(x.getMessage(), rae.getSignature(), is(expectedSignature)); + return; } catch (RejectedAccessException x) { - errors.checkThat(x.getMessage(), x.getSignature(), is(expectedSignature)); - } catch (Throwable t) { - errors.addError(t); + assertThat(x.getMessage(), x.getSignature(), is(expectedSignature)); + return; } + fail("should be rejected"); } @Issue("JENKINS-37129") - @Test public void methodMissingException() throws Exception { + @Test + void methodMissingException() throws Exception { // test: trying to call a nonexistent method final MissingMethodException e = assertThrows(MissingMethodException.class, () -> evaluate(new GenericWhitelist(), "[].noSuchMethod()")); - assertEquals(e.getType(),ArrayList.class); + assertEquals(ArrayList.class, e.getType()); assertThat(e.getMethod(),is("noSuchMethod")); // control: trying to call an existing method that's not safe @@ -962,74 +984,78 @@ public static void assertRejected(Whitelist whitelist, String expectedSignature, @Issue("JENKINS-46088") @Test - public void matcherTypeAssignment() throws Exception { + void matcherTypeAssignment() throws Exception { assertEvaluate(new GenericWhitelist(), "goodbye world", "def text = 'hello world'; java.util.regex.Matcher matcher = text =~ 'hello (.+)'; matcher ? \"goodbye ${matcher[0][1]}\" : 'fail'"); } @Issue("JENKINS-46088") @Test - public void rhsOfDeclarationTransformed() throws Exception { + void rhsOfDeclarationTransformed() throws Exception { assertRejected(new StaticWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", "jenkins.model.Jenkins x = jenkins.model.Jenkins.getInstance()"); } @Issue("JENKINS-46191") @Test - public void emptyDeclaration() throws Exception { + void emptyDeclaration() throws Exception { assertEvaluate(new GenericWhitelist(), "abc", "String a; a = 'abc'; return a"); } @Issue("JENKINS-46358") @Test - public void validFromAnyDGMClass() throws Exception { + void validFromAnyDGMClass() throws Exception { // This verifies that we pick up a valid DGM-style method from a class other than DefaultGroovyMethods assertEvaluate(new GenericWhitelist(), "alppe", "String a = 'apple'; return a.replaceFirst('ppl') { it.reverse() }"); } @Issue("JENKINS-46391") @Test - public void newPattern() throws Exception { + void newPattern() throws Exception { assertEvaluate(new GenericWhitelist(), true, "def f = java.util.regex.Pattern.compile('f.*'); return f.matcher('foo').matches()"); } @Issue("JENKINS-46391") @Test - public void tildePattern() throws Exception { + void tildePattern() throws Exception { assertEvaluate(new GenericWhitelist(), Pattern.class, "def f = ~/f.*/; return f.class"); } @Issue("JENKINS-35294") @Test - public void enumWithVarargs() throws Exception { - String script = "enum Thing {\n" - + " FIRST(\"The first thing\")\n" - + " String[] descriptions;\n" - + " public Thing(String... descriptions) {\n" - + " this.descriptions = descriptions;\n" - + " }\n" - + "}\n" - + "Thing.values()[0].descriptions[0]\n"; + void enumWithVarargs() throws Exception { + String script = """ + enum Thing { + FIRST("The first thing") + String[] descriptions; + public Thing(String... descriptions) { + this.descriptions = descriptions; + } + } + Thing.values()[0].descriptions[0] + """; String expected = "The first thing"; assertEvaluate(new GenericWhitelist(), expected, script); } @Issue("JENKINS-35294") @Test - public void enumWithStringAndVarargs() throws Exception { - String script = "enum Thing {\n" - + " FIRST(\"The first thing\")\n" - + " String description;\n" - + " public Thing(String description, int... unused) {\n" - + " this.description = description;\n" - + " }\n" - + "}\n" - + "Thing.values()[0].description\n"; + void enumWithStringAndVarargs() throws Exception { + String script = """ + enum Thing { + FIRST("The first thing") + String description; + public Thing(String description, int... unused) { + this.description = description; + } + } + Thing.values()[0].description + """; String expected = "The first thing"; assertEvaluate(new GenericWhitelist(), expected, script); } @Issue("JENKINS-44557") @Test - public void varArgsWithGString() throws Exception { + void varArgsWithGString() throws Exception { ProxyWhitelist wl = new ProxyWhitelist(new GenericWhitelist(), new AnnotatedWhitelist()); String uv = UsesVarargs.class.getName(); @@ -1038,7 +1064,7 @@ public void varArgsWithGString() throws Exception { @Issue("JENKINS-47893") @Test - public void varArgsWithOtherArgs() throws Exception { + void varArgsWithOtherArgs() throws Exception { ProxyWhitelist wl = new ProxyWhitelist(new GenericWhitelist(), new AnnotatedWhitelist()); String uv = UsesVarargs.class.getName(); String sc = StringContainer.class.getName(); @@ -1053,7 +1079,7 @@ public void varArgsWithOtherArgs() throws Exception { @Issue("JENKINS-48364") @Test - public void nullFirstVarArg() throws Exception { + void nullFirstVarArg() throws Exception { ProxyWhitelist wl = new ProxyWhitelist(new GenericWhitelist(), new AnnotatedWhitelist()); String uv = UsesVarargs.class.getName(); @@ -1064,22 +1090,24 @@ public void nullFirstVarArg() throws Exception { @Issue("JENKINS-46213") @Test - public void varArgsOnStaticDeclaration() throws Exception { - String script = "class Explode {\n" + - " static TEST_FMT = 'a:%s b:%s'\n" + - " static STATIC_TEST = sprintf(\n" + - " TEST_FMT,\n" + - " '1',\n" + - " '2',\n" + - " )\n" + - " String fieldTest = sprintf(\n" + - " TEST_FMT,\n" + - " '3',\n" + - " '4',\n" + - " )\n" + - "}\n" + - "def ex = new Explode()\n" + - "return \"${Explode.STATIC_TEST} ${ex.fieldTest}\"\n"; + void varArgsOnStaticDeclaration() throws Exception { + String script = """ + class Explode { + static TEST_FMT = 'a:%s b:%s' + static STATIC_TEST = sprintf( + TEST_FMT, + '1', + '2', + ) + String fieldTest = sprintf( + TEST_FMT, + '3', + '4', + ) + } + def ex = new Explode() + return "${Explode.STATIC_TEST} ${ex.fieldTest}" + """; assertEvaluate(new StaticWhitelist("staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods sprintf java.lang.Object java.lang.String java.lang.Object[]"), "a:1 b:2 a:3 b:4", @@ -1088,17 +1116,19 @@ public void varArgsOnStaticDeclaration() throws Exception { @Issue("SECURITY-663") @Test - public void castAsFile() throws Exception { + void castAsFile() throws Exception { assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", "def s = []; ('/tmp/foo' as File).each { s << it }\n"); } @Issue("JENKINS-48501") @Test - public void nullInVarArgsAsArray() throws Exception { - String script = "def TEST_FMT = 'a:%s b:%s c:%s d:%s'\n" + - "String s = sprintf(TEST_FMT, null, '2', '3', '4')\n" + - "return s\n"; + void nullInVarArgsAsArray() throws Exception { + String script = """ + def TEST_FMT = 'a:%s b:%s c:%s d:%s' + String s = sprintf(TEST_FMT, null, '2', '3', '4') + return s + """; assertEvaluate(new StaticWhitelist("staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods sprintf java.lang.Object java.lang.String java.lang.Object[]"), "a:null b:2 c:3 d:4", script); @@ -1117,7 +1147,7 @@ public NonArrayConstructorList(boolean choiceOne, boolean choiceTwo) { @Issue("JENKINS-50380") @Test - public void checkedCastWhenAssignable() throws Exception { + void checkedCastWhenAssignable() throws Exception { String nacl = NonArrayConstructorList.class.getName(); // pre groovy-sandbox-1.18, results in unclassified new org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptorTest$NonArrayConstructorList java.lang.String assertEvaluate(new StaticWhitelist("new org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptorTest$NonArrayConstructorList boolean boolean", @@ -1148,7 +1178,7 @@ public String getOther() { @Issue("JENKINS-50470") @Test - public void checkedGetPropertyOnCollection() throws Exception { + void checkedGetPropertyOnCollection() { String snb = SimpleNamedBean.class.getName(); // Before JENKINS-50470 fix, this would error out on "unclassified field java.util.ArrayList name" @@ -1164,7 +1194,7 @@ public void checkedGetPropertyOnCollection() throws Exception { @Issue("JENKINS-50843") @Test - public void callClosureElementOfMapAsMethod() throws Exception { + void callClosureElementOfMapAsMethod() throws Exception { assertEvaluate(new GenericWhitelist(), "hello", "def m = [ f: {return 'hello'} ]; m.f()"); assertEvaluate(new GenericWhitelist(), 15, "def m = [ f: {a -> return a*3} ]; m.f(5)"); assertEvaluate(new GenericWhitelist(), "a=hello,b=10", "def m = [ f: {a,b -> return \"a=${a},b=${b}\"} ]; m.f('hello',10)"); @@ -1173,7 +1203,7 @@ public void callClosureElementOfMapAsMethod() throws Exception { @Issue("JENKINS-50906") @Test - public void scriptBindingClosureVariableCall() throws Exception { + void scriptBindingClosureVariableCall() throws Exception { assertEvaluate(new GenericWhitelist(), true, "def func = { 1 }; this.func2 = { 1 }; return func() == func2();\n"); assertEvaluate(new GenericWhitelist(), true, "def func = { x -> x }; this.func2 = { x -> x }; return func(5) == func2(5);\n"); assertEvaluate(new GenericWhitelist(), true, "def func = { x, y -> x * y }; this.func2 = { x, y -> x * y }; return func(4, 5) == func2(4, 5);\n"); @@ -1181,7 +1211,7 @@ public void scriptBindingClosureVariableCall() throws Exception { } @Test - public void dateTimeApi() throws Exception { + void dateTimeApi() throws Exception { assertEvaluate(new GenericWhitelist(), 8, "def tomorrow = java.time.LocalDate.now().plusDays(1).format(java.time.format.DateTimeFormatter.BASIC_ISO_DATE).length()"); assertEvaluate(new GenericWhitelist(), "2017-01-06", "def yesterday = java.time.LocalDate.parse('2017-01-07').minusDays(1).format(java.time.format.DateTimeFormatter.ISO_LOCAL_DATE)"); assertEvaluate(new GenericWhitelist(), 15, "java.time.LocalTime.now().withHour(15).getHour()"); @@ -1192,7 +1222,7 @@ public void dateTimeApi() throws Exception { @Issue("SECURITY-1186") @Test - public void finalizer() throws Exception { + void finalizer() { final MultipleCompilationErrorsException e = assertThrows(MultipleCompilationErrorsException.class, () -> evaluate(new GenericWhitelist(), "class Test { public void finalize() { } }; null")); assertThat(e.getErrorCollector().getErrorCount(), equalTo(1)); @@ -1202,7 +1232,7 @@ public void finalizer() throws Exception { } @Test - public void alwaysRejectPermanentlyBlacklisted() throws Exception { + void alwaysRejectPermanentlyBlacklisted() throws Exception { assertRejected(new StaticWhitelist("staticMethod java.lang.System exit int"), "staticMethod java.lang.System exit int", "System.exit(1)"); @@ -1218,18 +1248,19 @@ public void alwaysRejectPermanentlyBlacklisted() throws Exception { @Issue("JENKINS-56682") @Test - public void scriptInitializersAtFieldSyntax() throws Exception { + void scriptInitializersAtFieldSyntax() throws Exception { assertEvaluate(new GenericWhitelist(), 3, - "import groovy.transform.Field\n" + - "@Field static int foo = 1\n" + - "@Field int bar = foo + 1\n" + - "@Field int baz = bar + 1\n" + - "baz"); + """ + import groovy.transform.Field + @Field static int foo = 1 + @Field int bar = foo + 1 + @Field int baz = bar + 1 + baz"""); } @Issue("JENKINS-56682") @Test - public void scriptInitializersClassSyntax() throws Exception { + void scriptInitializersClassSyntax() throws Exception { assertEvaluate(new GenericWhitelist(), 2, "class MyScript extends Script {\n" + " { MyScript.foo++ }\n" + // The instance initializer seems to be context sensitive, if placed below the field it is treated as a closure... @@ -1240,130 +1271,158 @@ public void scriptInitializersClassSyntax() throws Exception { } @Issue("SECURITY-1538") - @Test public void blockMethodNameInMethodCalls() throws Exception { + @Test + void blockMethodNameInMethodCalls() throws Exception { assertRejected(new GenericWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", "import jenkins.model.Jenkins\n" + "1.({ Jenkins.getInstance(); 'toString' }())()"); } @Issue("SECURITY-1538") - @Test public void blockPropertyNameInAssignment() throws Exception { + @Test + void blockPropertyNameInAssignment() throws Exception { assertRejected(new GenericWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", - "import jenkins.model.Jenkins\n" + - "class Test { def x = 0 }\n" + - "def t = new Test()\n" + - "t.({ Jenkins.getInstance(); 'x' }()) = 1\n"); + """ + import jenkins.model.Jenkins + class Test { def x = 0 } + def t = new Test() + t.({ Jenkins.getInstance(); 'x' }()) = 1 + """); } @Issue("SECURITY-1538") - @Test public void blockPropertyNameInPrefixPostfixExpressions() throws Exception { + @Test + void blockPropertyNameInPrefixPostfixExpressions() throws Exception { assertRejected(new GenericWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", - "import jenkins.model.Jenkins\n" + - "class Test { def x = 0 }\n" + - "def t = new Test()\n" + - "t.({ Jenkins.getInstance(); 'x' }())++\n"); + """ + import jenkins.model.Jenkins + class Test { def x = 0 } + def t = new Test() + t.({ Jenkins.getInstance(); 'x' }())++ + """); } @Issue("SECURITY-1538") - @Test public void blockSubexpressionsInPrefixPostfixExpressions() throws Exception { + @Test + void blockSubexpressionsInPrefixPostfixExpressions() throws Exception { assertRejected(new GenericWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", - "import jenkins.model.Jenkins\n" + - "++({ Jenkins.getInstance(); 1 }())\n"); + """ + import jenkins.model.Jenkins + ++({ Jenkins.getInstance(); 1 }()) + """); assertRejected(new GenericWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", - "import jenkins.model.Jenkins\n" + - "({ Jenkins.getInstance(); 1 }())++\n"); + """ + import jenkins.model.Jenkins + ({ Jenkins.getInstance(); 1 }())++ + """); } @Issue("SECURITY-1579") - @Test public void blockInitialExpressionsInConstructorsCallingSuper() throws Exception { + @Test + void blockInitialExpressionsInConstructorsCallingSuper() throws Exception { assertRejected(new GenericWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", - "import jenkins.model.Jenkins\n" + - "class B {}\n" + - "class A extends B {\n" + - " A(x = Jenkins.getInstance()) {\n" + - " super()\n" + - " }\n" + - "}\n" + - "new A()\n"); + """ + import jenkins.model.Jenkins + class B {} + class A extends B { + A(x = Jenkins.getInstance()) { + super() + } + } + new A() + """); } @Issue("SECURITY-1658") - @Test public void blockInitialExpressionsInClosures() throws Exception { + @Test + void blockInitialExpressionsInClosures() throws Exception { assertRejected(new GenericWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", - "import jenkins.model.Jenkins\n" + - "({ j = Jenkins.getInstance() -> true })()\n"); + """ + import jenkins.model.Jenkins + ({ j = Jenkins.getInstance() -> true })() + """); } @Issue("SECURITY-1713") @Test - public void blockIllegalAnnotationsOnImports() throws Exception { + void blockIllegalAnnotationsOnImports() { assertAnnotationBlocked(ASTTest.class, - "@groovy.transform.ASTTest(value={\n" + - " throw new Exception('ASTTest should not have been executed!')\n" + - "})\n" + - "import java.lang.Object\n"); + """ + @groovy.transform.ASTTest(value={ + throw new Exception('ASTTest should not have been executed!') + }) + import java.lang.Object + """); } @Issue("SECURITY-1713") @Test - public void blockIllegalAnnotationsInAnnotations() throws Exception { + void blockIllegalAnnotationsInAnnotations() { assertAnnotationBlocked(ASTTest.class, - "@groovy.lang.Category(value = {\n" + - " @groovy.transform.ASTTest(value = {\n" + - " throw new Exception('ASTTest should not have been executed!')\n" + - " })\n" + - " Object\n" + - "})\n" + - "class Foo { }\n"); + """ + @groovy.lang.Category(value = { + @groovy.transform.ASTTest(value = { + throw new Exception('ASTTest should not have been executed!') + }) + Object + }) + class Foo { } + """); } @Issue("SECURITY-1754") - @Test public void blockDirectCallsToSyntheticConstructors() throws Exception { + @Test + void blockDirectCallsToSyntheticConstructors() { // Not ok, the call to super() in the synthetic constructor for Subclass cannot be intercepted. SecurityException e = assertThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), - "class Superclass { }\n" + - "class Subclass extends Superclass { }\n" + - "new Subclass(null)")); + """ + class Superclass { } + class Subclass extends Superclass { } + new Subclass(null)""")); assertThat(e.getMessage(), equalTo( "Rejecting illegal call to synthetic constructor: private Subclass(org.kohsuke.groovy.sandbox.impl.Checker$SuperConstructorWrapper). " + "Perhaps you meant to use one of these constructors instead: public Subclass()")); e = assertThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), - "class Superclass { Superclass(String x) { } }\n" + - "class Subclass extends Superclass {\n" + - " def wrapper\n" + - " Subclass() { super('secret.key'); def $cw = $cw; wrapper = $cw }\n" + - "}\n" + - "def wrapper = new Subclass().wrapper\n" + - "class MyFile extends File {\n" + - " MyFile(String path) {\n" + - " super(path)\n" + - " }\n" + - "}\n" + - "new MyFile(wrapper, 'unused')")); + """ + class Superclass { Superclass(String x) { } } + class Subclass extends Superclass { + def wrapper + Subclass() { super('secret.key'); def $cw = $cw; wrapper = $cw } + } + def wrapper = new Subclass().wrapper + class MyFile extends File { + MyFile(String path) { + super(path) + } + } + new MyFile(wrapper, 'unused')""")); assertThat(e.getMessage(), equalTo( "Rejecting illegal call to synthetic constructor: private MyFile(org.kohsuke.groovy.sandbox.impl.Checker$SuperConstructorWrapper,java.lang.String). " + "Perhaps you meant to use one of these constructors instead: public MyFile(java.lang.String)")); } @Issue("SECURITY-1754") - @Test public void blockMisinterceptedCallsToSyntheticConstructors() throws Exception { + @Test + void blockMisinterceptedCallsToSyntheticConstructors() { // Not ok, the call to super() in the synthetic constructor for Subclass cannot be intercepted. final SecurityException e = assertThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), - "class Superclass { }\n" + - "class Subclass extends Superclass {\n" + - " Subclass() { def x = 1 }\n" + - " Subclass(Subclass s) { def x = 1 }\n" + - "}\n" + - "new Subclass(null)")); // Intercepted as a call to the second constructor before SECURITY-1754, but actually calls synthetic constructor. + """ + class Superclass { } + class Subclass extends Superclass { + Subclass() { def x = 1 } + Subclass(Subclass s) { def x = 1 } + } + new Subclass(null)""")); // Intercepted as a call to the second constructor before SECURITY-1754, but actually calls synthetic constructor. assertThat(e.getMessage(), equalTo( "Rejecting illegal call to synthetic constructor: private Subclass(org.kohsuke.groovy.sandbox.impl.Checker$SuperConstructorWrapper). " + "Perhaps you meant to use one of these constructors instead: public Subclass(), public Subclass(Subclass)")); } @Issue("SECURITY-1754") - @Test public void blockCallsToSyntheticConstructorsViaOtherConstructors() throws Exception { + @Test + void blockCallsToSyntheticConstructorsViaOtherConstructors() { // Not ok, the call to super() in the synthetic constructor for Subclass cannot be intercepted. final SecurityException e = assertThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), @@ -1379,7 +1438,8 @@ public void blockIllegalAnnotationsInAnnotations() throws Exception { } @Issue("SECURITY-1754") - @Test public void blockConstructorWrappersFromBeingUsedDirectly() throws Exception { + @Test + void blockConstructorWrappersFromBeingUsedDirectly() throws Exception { for (Class syntheticParamType : new Class[] { SuperConstructorWrapper.class, ThisConstructorWrapper.class }) { // Not ok, instantiating any of the wrappers would allow attackers to bypass the fix. assertRejected(new GenericWhitelist(), "new " + syntheticParamType.getName() + " java.lang.Object[]", @@ -1391,61 +1451,73 @@ public void blockIllegalAnnotationsInAnnotations() throws Exception { } @Issue("SECURITY-1754") - @Test public void allowCheckedCallsToSyntheticConstructors() throws Exception { + @Test + void allowCheckedCallsToSyntheticConstructors() throws Exception { // Ok, super call is intercepted via Checker.checkedSuperConstructor. assertEvaluate(new GenericWhitelist(), "Subclass", - "class Superclass { }\n" + - "class Subclass extends Superclass { }\n" + - "new Subclass().class.simpleName"); + """ + class Superclass { } + class Subclass extends Superclass { } + new Subclass().class.simpleName"""); // Ok, this call is intercepted via Checker.checkedThisConstructor. assertEvaluate(new GenericWhitelist(), "Subclass", - "class Subclass {\n" + - " Subclass() { this(1) }\n" + - " Subclass(int x) { }\n" + - "}\n" + - "new Subclass().class.simpleName"); + """ + class Subclass { + Subclass() { this(1) } + Subclass(int x) { } + } + new Subclass().class.simpleName"""); } - @Test public void blockCallsToSyntheticConstructorsViaCasts() throws Exception { + @Test + void blockCallsToSyntheticConstructorsViaCasts() throws Exception { // Not ok, the call to super() in the synthetic constructor for Subclass cannot be intercepted. assertRejected(new GenericWhitelist(), "new Subclass org.kohsuke.groovy.sandbox.impl.Checker$SuperConstructorWrapper", - "class Superclass { }\n" + - "class Subclass extends Superclass {\n" + - " Subclass() { }\n" + - "}\n" + - "Subclass x = [null]"); + """ + class Superclass { } + class Subclass extends Superclass { + Subclass() { } + } + Subclass x = [null]"""); } @Issue("SECURITY-1754") - @Test public void groovyInterceptable() throws Throwable { + @Test + void groovyInterceptable() throws Throwable { assertRejected(new GenericWhitelist(), "method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object", - "class Test implements GroovyInterceptable {\n" + - " def hello() { 'world' }\n" + - " def invokeMethod(String name, Object args) { 'goodbye' }\n" + - "}\n" + - "new Test().hello()\n"); + """ + class Test implements GroovyInterceptable { + def hello() { 'world' } + def invokeMethod(String name, Object args) { 'goodbye' } + } + new Test().hello() + """); // Property access is not affected by GroovyInterceptable. assertEvaluate(new GenericWhitelist(), "world", - "class Test implements GroovyInterceptable {\n" + - " def hello = 'world'\n" + - " def invokeMethod(String name, Object args) { 'goodbye' }\n" + - "}\n" + - "new Test().hello\n"); + """ + class Test implements GroovyInterceptable { + def hello = 'world' + def invokeMethod(String name, Object args) { 'goodbye' } + } + new Test().hello + """); } @Issue("SECURITY-2020") - @Test public void unsafeReturnValue() throws Throwable { + @Test + void unsafeReturnValue() { final SecurityException e = assertThrows(SecurityException.class, () -> { Object result = evaluate(new GenericWhitelist(), - "class Test {\n" + - " @Override public String toString() {\n" + - " jenkins.model.Jenkins.get().setSystemMessage('Hello, world!')\n" + - " 'test'\n" + - " }\n" + - "}\n" + - "new Test()"); + """ + class Test { + @Override public String toString() { + jenkins.model.Jenkins.get().setSystemMessage('Hello, world!') + 'test' + } + } + new Test()"""); // Test.equals and Test.getClass are inherited and not sandbox-transformed, so they can be called outside of the sandbox. - assertNotEquals(result, new Object()); + assertNotEquals(new Object(), result); assertThat(result.getClass().getSimpleName(), equalTo("Test")); // Test.toString is defined in the sandbox, so it cannot be called outside of the sandbox. result.toString(); @@ -1454,202 +1526,233 @@ public void blockIllegalAnnotationsInAnnotations() throws Exception { } @Issue("SECURITY-2824") - @Test public void blockUnsafeImplicitCastsPropertiesAndAttributes() throws Throwable { + @Test + void blockUnsafeImplicitCastsPropertiesAndAttributes() throws Throwable { // Instance properties assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", - "class Test { File file }\n" + - "def t = new Test()\n" + - "t.file = ['secret.key']\n"); + """ + class Test { File file } + def t = new Test() + t.file = ['secret.key'] + """); // Static properties assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", - "class Test { static File file }\n" + - "Test.file = ['secret.key']\n"); + """ + class Test { static File file } + Test.file = ['secret.key'] + """); // Instance attributes assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", - "class Test { File file }\n" + - "def t = new Test()\n" + - "t.@file = ['secret.key']\n"); + """ + class Test { File file } + def t = new Test() + t.@file = ['secret.key'] + """); // Static attributes assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", - "class Test { static File file }\n" + - "Test.@file = ['secret.key']\n"); + """ + class Test { static File file } + Test.@file = ['secret.key'] + """); } @Issue("SECURITY-2824") - @Test public void blockUnsafeImplicitCastsPropertySetterParameters() throws Throwable { + @Test + void blockUnsafeImplicitCastsPropertySetterParameters() throws Throwable { assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", - "class Test {\n" + - " Object file\n" + - " def setFile(File file) {\n" + - " this.file = file\n" + - " }\n" + - "}\n" + - "def t = new Test()\n" + - "t.file = ['secret.key']\n " + - "t.file.class"); + """ + class Test { + Object file + def setFile(File file) { + this.file = file + } + } + def t = new Test() + t.file = ['secret.key'] + \ + t.file.class"""); } @Issue("SECURITY-2824") - @Test public void blockUnsafeImplicitCastsPropertySettersWithUnusualCapitalization() throws Throwable { + @Test + void blockUnsafeImplicitCastsPropertySettersWithUnusualCapitalization() throws Throwable { assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", - "class Test {\n" + - " String aProp\n" + - " def setAProp(List prop) {\n" + - " this.aProp = 'unused'\n" + - " }\n" + - " def setaProp(File prop) {\n" + - " this.aProp = /got a ${prop.class}/\n" + - " }\n" + - "}\n" + - "def t = new Test()\n" + - "t.aProp = ['secret.key']\n " + - "t.aProp"); + """ + class Test { + String aProp + def setAProp(List prop) { + this.aProp = 'unused' + } + def setaProp(File prop) { + this.aProp = /got a ${prop.class}/ + } + } + def t = new Test() + t.aProp = ['secret.key'] + \ + t.aProp"""); } @Issue("SECURITY-2824") - @Test public void propertiesWithMultipleSetters() throws Throwable { + @Test + void propertiesWithMultipleSetters() throws Throwable { // If you have multiple setters but one of them matches the argument type exactly, that is the one that gets used. assertEvaluate(new GenericWhitelist(), "list overload", - "class Test {\n" + - " Object prop\n" + - " def setProp(File prop) {\n" + - " this.prop = 'file overload'\n" + - " }\n" + - " def setProp(String prop) {\n" + - " this.prop = 'string overload'\n" + - " }\n" + - " def setProp(List prop) {\n" + - " this.prop = 'list overload'\n" + - " }\n" + - "}\n" + - "def t = new Test()\n" + - "t.prop = ['secret.key']\n " + - "t.prop"); + """ + class Test { + Object prop + def setProp(File prop) { + this.prop = 'file overload' + } + def setProp(String prop) { + this.prop = 'string overload' + } + def setProp(List prop) { + this.prop = 'list overload' + } + } + def t = new Test() + t.prop = ['secret.key'] + \ + t.prop"""); // In this case, somewhere in the bowels of ScriptBytecodeAdapter.setProperty / MetaClass.invokeMethod // the list is converted to an array which is then used directly as the arguments array when invoking the setter, // so the String overload is chosen. SandboxInterceptor doesn't understand this case and intercepts the field... assertEvaluate(new GenericWhitelist(), "string overload", - "class Test {\n" + - " Object prop\n" + - " def setProp(File prop) {\n" + - " this.prop = 'file overload'\n" + - " }\n" + - " def setProp(String prop) {\n" + - " this.prop = 'string overload'\n" + - " }\n" + - "}\n" + - "def t = new Test()\n" + - "t.prop = ['secret.key']\n" + - "t.prop"); + """ + class Test { + Object prop + def setProp(File prop) { + this.prop = 'file overload' + } + def setProp(String prop) { + this.prop = 'string overload' + } + } + def t = new Test() + t.prop = ['secret.key'] + t.prop"""); // ... so if the field name does not match the setter names, SandboxInterceptor will reject it. - errors.checkThrows(MissingPropertyException.class, () -> evaluate(new GenericWhitelist(), - "class Test {\n" + - " Object prop2\n" + - " def setProp(File prop) {\n" + - " this.prop2 = prop\n" + - " }\n" + - " def setProp(String prop) {\n" + - " this.prop2 = prop\n" + - " }\n" + - " def getProp() { this.prop2 }\n" + - "}\n" + - "def t = new Test()\n" + - "t.prop = ['secret.key']\n" + - "t.prop")); + assertThrows(MissingPropertyException.class, () -> evaluate(new GenericWhitelist(), + """ + class Test { + Object prop2 + def setProp(File prop) { + this.prop2 = prop + } + def setProp(String prop) { + this.prop2 = prop + } + def getProp() { this.prop2 } + } + def t = new Test() + t.prop = ['secret.key'] + t.prop""")); // If none of the overloads match directly, and there is no overload that matches the list if it is converted // to an array, then a MissingPropertyException gets thrown. - errors.checkThrows(MissingMethodException.class, () -> evaluate(new GenericWhitelist(), - "class Test {\n" + - " Object prop\n" + - " def setProp(File prop) {\n" + - " this.prop = prop\n" + - " }\n" + - " def setProp(Integer prop) {\n" + - " this.prop = prop\n" + - " }\n" + - "}\n" + - "def t = new Test()\n" + - "t.prop = ['secret.key']\n " + - "t.prop")); + assertThrows(MissingMethodException.class, () -> evaluate(new GenericWhitelist(), + """ + class Test { + Object prop + def setProp(File prop) { + this.prop = prop + } + def setProp(Integer prop) { + this.prop = prop + } + } + def t = new Test() + t.prop = ['secret.key'] + \ + t.prop""")); // Methods with more than one parameter are not setters and must be ignored when intercepting property assignment. assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String int", - "class Test {\n" + - " Object prop\n" + - " def setProp(File prop) {\n" + - " this.prop = 'file overload'\n" + - " }\n" + - " def setProp(String prop1, Integer prop2) {\n" + - " this.prop = 'multi-param overload'\n" + - " }\n" + - "}\n" + - "def t = new Test()\n" + - "t.prop = ['secret.key', 1]\n" + - "t.prop"); + """ + class Test { + Object prop + def setProp(File prop) { + this.prop = 'file overload' + } + def setProp(String prop1, Integer prop2) { + this.prop = 'multi-param overload' + } + } + def t = new Test() + t.prop = ['secret.key', 1] + t.prop"""); // ... but if you have multiple setters as well as a method with the same name that has multiple parameters, // then Groovy will invoke the multi-parameter method if it is the best match for the arguments. assertEvaluate(new GenericWhitelist(), "multi-param overload", - "class Test {\n" + - " Object prop\n" + - " def setProp(File prop) {\n" + - " this.prop = 'file overload'\n" + - " }\n" + - " def setProp(Boolean prop) {\n" + - " this.prop = 'boolean overload'\n" + - " }\n" + - " def setProp(String prop1, Integer prop2) {\n" + - " this.prop = 'multi-param overload'\n" + - " }\n" + - "}\n" + - "def t = new Test()\n" + - "t.prop = ['secret.key', 1]\n" + - "t.prop"); + """ + class Test { + Object prop + def setProp(File prop) { + this.prop = 'file overload' + } + def setProp(Boolean prop) { + this.prop = 'boolean overload' + } + def setProp(String prop1, Integer prop2) { + this.prop = 'multi-param overload' + } + } + def t = new Test() + t.prop = ['secret.key', 1] + t.prop"""); } @Issue("SECURITY-2824") - @Test public void blockUnsafeImplicitCastsMetaClassModification() throws Throwable { + @Test + void blockUnsafeImplicitCastsMetaClassModification() throws Throwable { assertRejected(new GenericWhitelist(), "method groovy.lang.GroovyObject getMetaClass", - "class Parent {\n" + - " public File prop\n" + - "}\n" + - "class Child extends Parent {\n" + - " Object prop // property\n" + - "}\n" + - "def p = new Parent()\n" + - "def c = new Child()\n" + - "c.metaClass = p.metaClass\n" + - "c.prop = ['secret.key']\n" + - "c.prop"); + """ + class Parent { + public File prop + } + class Child extends Parent { + Object prop // property + } + def p = new Parent() + def c = new Child() + c.metaClass = p.metaClass + c.prop = ['secret.key'] + c.prop"""); } @Issue("SECURITY-2824") - @Test public void blockUnsafeImplicitCastsInitialParameterExpressions() throws Throwable { + @Test + void blockUnsafeImplicitCastsInitialParameterExpressions() throws Throwable { assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", "def method(File file = ['secret.key']) { file }; method()"); assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", "({ File file = ['secret.key'] -> file })()"); assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", - "class Test {\n" + - " def x\n" + - " Test(File file = ['secret.key']) {\n" + - " x = file\n" + - " }\n" + - "}\n" + - "new Test().x"); + """ + class Test { + def x + Test(File file = ['secret.key']) { + x = file + } + } + new Test().x"""); } @Issue("SECURITY-2824") - @Test public void blockUnsafeImplicitCastsFields() throws Throwable { + @Test + void blockUnsafeImplicitCastsFields() throws Throwable { assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", - "class Test {\n" + - " File file = ['secret.key']\n" + - "}\n" + - "new Test().file"); + """ + class Test { + File file = ['secret.key'] + } + new Test().file"""); assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", "@groovy.transform.Field File file = ['secret.key']\n" + "file"); } - @Test public void blockSyntheticConstructorsFieldsAndMethods() throws Throwable { + @Test + void blockSyntheticConstructorsFieldsAndMethods() throws Throwable { assertRejected(new GenericWhitelist(), "method groovy.lang.GroovyObject getMetaClass", "class Test {}; new Test().metaClass"); assertRejected(new GenericWhitelist(), "field Test metaClass", @@ -1658,26 +1761,27 @@ public void blockIllegalAnnotationsInAnnotations() throws Exception { "getClass().$getCallSiteArray()"); assertRejected(new GenericWhitelist(), "method Script1 $getStaticMetaClass", "$getStaticMetaClass()"); - errors.checkThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), - "class Parent {" + - " Parent(String value) { }" + - "}\n" + - "class Child extends Parent {\n" + - " def wrapper\n" + - " Child(String value) {\n" + - " super(value)\n" + - " def $cw = $cw\n" + - " wrapper = $cw\n" + - " }\n" + - "}\n" + - "def cw = new Child('secret.key').wrapper\n" + - "class MyFile extends File { public MyFile(String path) { super(path) } }\n" + - "def f = new MyFile(cw, 'unused')\n" + - "[f, f.class]")); - } - - @Test - public void booleanCasts() throws Throwable { + assertThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), + """ + class Parent {\ + Parent(String value) { }\ + } + class Child extends Parent { + def wrapper + Child(String value) { + super(value) + def $cw = $cw + wrapper = $cw + } + } + def cw = new Child('secret.key').wrapper + class MyFile extends File { public MyFile(String path) { super(path) } } + def f = new MyFile(cw, 'unused') + [f, f.class]""")); + } + + @Test + void booleanCasts() throws Throwable { assertEvaluate(new GenericWhitelist(), null, "null as Boolean"); assertEvaluate(new GenericWhitelist(), true, "true as Boolean"); assertEvaluate(new GenericWhitelist(), false, "[:] as Boolean"); @@ -1690,14 +1794,14 @@ public void booleanCasts() throws Throwable { } @Test - public void staticAttributesAreNotShadowedByClassFields() throws Throwable { + void staticAttributesAreNotShadowedByClassFields() throws Throwable { assertEvaluate(new GenericWhitelist(), "foo", "class MyClass { static String name = 'foo' }; MyClass.@name"); assertEvaluate(new GenericWhitelist(), "foo", "class MyClass { static String name }; MyClass.@name = 'foo'"); } @Issue("JENKINS-42214") @Test - public void accessStaticMembersViaInstance() throws Throwable { + void accessStaticMembersViaInstance() throws Throwable { String fqcn = HasStaticMembers.class.getName(); // Make sure that we report the correct signature in RejectedAccessException. assertRejected(new AnnotatedWhitelist(), "staticField " + fqcn + " FOO", "def o = new " + fqcn + "(); o.FOO"); @@ -1744,23 +1848,24 @@ public static Whitelist allowlist(String... signatures) throws Exception { } @Issue("SECURITY-3016") - @Test public void blockUnsafeCastsPropertyAssignmentViaImplicitMapConstructor() throws Throwable { + @Test + void blockUnsafeCastsPropertyAssignmentViaImplicitMapConstructor() throws Throwable { // Map constructors are supported when using new, but these property assignments are unsafe. assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", "class Test { File f }; new Test(f: ['secret.key'])"); assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", "class Test { File f; int x }; new Test(x: 1, f: ['secret.key'])"); // Map constructors are not supported when casting, regardless of whether the property assignments are safe or not. - errors.checkThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), + assertThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), "class Test { File f }; Test t = [f: ['secret.key']]")); - errors.checkThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), + assertThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), "class Test { File f; int x }; Test t = [x: 1, f: ['secret.key']]")); - errors.checkThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), + assertThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), "class Test { File f }; [f: ['secret.key']] as Test")); - errors.checkThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), + assertThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), "class Test { File f }; (Test)[f: ['secret.key']]")); // Map constructors are not currently supported when constructing inner classes. - errors.checkThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), + assertThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), "class Outer { class Inner { File f }; def makeInner() { new Inner(f: ['secret.key']) } }; new Outer().makeInner().f")); } @@ -1770,14 +1875,14 @@ public static Whitelist allowlist(String... signatures) throws Exception { * @param annotation The annotation that will be checked. * @param script The script to check. It should use the annotation via a fully-qualified class name. */ - private void assertAnnotationBlocked(Class annotation, String script) { + private void assertAnnotationBlocked(Class annotation, String script) { assertAnnotationBlockedInternal(annotation, script); assertAnnotationBlockedInternal(annotation, "import " + annotation.getCanonicalName() + "\n" + script.replaceAll(annotation.getName(), annotation.getSimpleName())); } - private void assertAnnotationBlockedInternal(Class annotation, String script) { + private void assertAnnotationBlockedInternal(Class annotation, String script) { GroovyShell shell = new GroovyShell(GroovySandbox.createSecureCompilerConfiguration()); final MultipleCompilationErrorsException e = assertThrows(MultipleCompilationErrorsException.class, () -> shell.parse(script)); assertThat(e.getMessage(), anyOf( diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoaderTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoaderTest.java index 695d2a9b..1e950f89 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoaderTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoaderTest.java @@ -25,23 +25,26 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy; import com.github.benmanes.caffeine.cache.stats.CacheStats; -import org.junit.Test; import org.jvnet.hudson.test.Issue; +import org.junit.jupiter.api.Test; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxResolvingClassLoader.CLASS_NOT_FOUND; import static org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxResolvingClassLoader.parentClassCache; -import static org.junit.Assert.assertThrows; -public class SandboxResolvingClassLoaderTest { +import static org.junit.jupiter.api.Assertions.assertThrows; + +class SandboxResolvingClassLoaderTest { private final ClassLoader parentLoader = SandboxResolvingClassLoaderTest.class.getClassLoader(); private final SandboxResolvingClassLoader loader = new SandboxResolvingClassLoader(parentLoader); @Issue("JENKINS-59587") - @Test public void classCacheDoesNotHoldClassValuesTooWeakly() throws Exception { + @Test + void classCacheDoesNotHoldClassValuesTooWeakly() throws Exception { // Load a class that does exist. assertThat(loader.loadClass("java.lang.String", false), equalTo(String.class)); // Load a class that does not exist. diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java index 465f6fe2..e64deb09 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java @@ -57,8 +57,7 @@ import java.util.Collections; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; + import jenkins.model.Jenkins; import org.apache.commons.io.FileUtils; import org.apache.tools.ant.DirectoryScanner; @@ -75,33 +74,37 @@ import static org.hamcrest.Matchers.is; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted; import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage; -import org.junit.ClassRule; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.jvnet.hudson.test.BuildWatcher; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.api.io.TempDir; + import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.MockAuthorizationStrategy; +import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import org.kohsuke.groovy.sandbox.impl.Checker; -import static org.junit.Assert.assertEquals; -public class SecureGroovyScriptTest { +@WithJenkins +class SecureGroovyScriptTest { - @ClassRule public static BuildWatcher WATCHER = new BuildWatcher(); + @SuppressWarnings("unused") + @RegisterExtension + private static final BuildWatcherExtension BUILD_WATCHER = new BuildWatcherExtension(); - @Rule public JenkinsRule r = new JenkinsRule(); + @TempDir + private File tmpFolderRule; - @Rule public TemporaryFolder tmpFolderRule = new TemporaryFolder(); + private JenkinsRule r; + + @BeforeEach + void beforeEach(JenkinsRule rule) { + r = rule; + } private void addPostBuildAction(HtmlPage page) throws IOException { String displayName = r.jenkins.getExtensionList(BuildStepDescriptor.class).get(TestGroovyRecorder.DescriptorImpl.class).getDisplayName(); @@ -111,14 +114,14 @@ private void addPostBuildAction(HtmlPage page) throws IOException { HtmlForm config = page.getFormByName("config"); r.getButtonByCaption(config, displayName).click(); } - } /** * Basic approval test where the user doesn't have ADMINISTER privs but has unchecked * the sandbox checkbox. Should result in script going to pending approval. */ - @Test public void basicApproval() throws Exception { + @Test + void basicApproval() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); @@ -189,7 +192,8 @@ private void addPostBuildAction(HtmlPage page) throws IOException { * Basic approval test where the user doesn't have ADMINISTER privs and forceSandbox is enabled * Sandbox checkbox should not be visible, but set to true by default */ - @Test public void basicApproval_ForceSandbox() throws Exception { + @Test + void basicApproval_ForceSandbox() throws Exception { ScriptApproval.get().setForceSandbox(true); r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); @@ -234,7 +238,8 @@ private void addPostBuildAction(HtmlPage page) throws IOException { /** * Test where the user has ADMINISTER privs, default to non sandbox mode, but require approval */ - @Test public void testSandboxDefault_with_ADMINISTER_privs() throws Exception { + @Test + void testSandboxDefault_with_ADMINISTER_privs() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); @@ -290,7 +295,8 @@ private void addPostBuildAction(HtmlPage page) throws IOException { * logic should not change to the default admin behavior * Except for the messages */ - @Test public void testSandboxDefault_with_ADMINISTER_privs_ForceSandbox() throws Exception { + @Test + void testSandboxDefault_with_ADMINISTER_privs_ForceSandbox() throws Exception { ScriptApproval.get().setForceSandbox(true); testSandboxDefault_with_ADMINISTER_privs(); } @@ -298,7 +304,8 @@ private void addPostBuildAction(HtmlPage page) throws IOException { /** * Test where the user doesn't have ADMINISTER privs, default to sandbox mode. */ - @Test public void testSandboxDefault_without_ADMINISTER_privs() throws Exception { + @Test + void testSandboxDefault_without_ADMINISTER_privs() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); @@ -342,7 +349,7 @@ private void addPostBuildAction(HtmlPage page) throws IOException { } private List getAllJarFiles() throws URISyntaxException { - String testClassPath = Stream.of(getClass().getName().split("\\.")).collect(Collectors.joining("/")); + String testClassPath = String.join("/", getClass().getName().split("\\.")); File testClassDir = new File(ClassLoader.getSystemResource(testClassPath).toURI()).getAbsoluteFile(); DirectoryScanner ds = new DirectoryScanner(); @@ -360,7 +367,7 @@ private List getAllJarFiles() throws URISyntaxException { } private List copy2TempDir(Iterable files) throws IOException { - final File tempDir = tmpFolderRule.newFolder(); + final File tempDir = newFolder(tmpFolderRule, "junit"); final List copies = new ArrayList<>(); for (File f: files) { final File copy = new File(tempDir, f.getName()); @@ -379,7 +386,7 @@ private List files2entries(Iterable files) throws IOExcept } private List getAllUpdatedJarFiles() throws URISyntaxException { - String testClassPath = Stream.of(getClass().getName().split("\\.")).collect(Collectors.joining("/")); + String testClassPath = String.join("/", getClass().getName().split("\\.")); File testClassDir = new File(ClassLoader.getSystemResource(testClassPath).toURI()).getAbsoluteFile(); File updatedDir = new File(testClassDir, "updated"); @@ -398,7 +405,8 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { return ret; } - @Test public void testClasspathConfiguration() throws Exception { + @Test + void testClasspathConfiguration() throws Exception { List classpath = new ArrayList<>(); for (File jarfile: getAllJarFiles()) { classpath.add(new ClasspathEntry(jarfile.getAbsolutePath())); @@ -415,7 +423,8 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { r.assertEqualBeans(recorder.getScript(), recorder2.getScript(), "script,sandbox,classpath"); } - @Test public void testClasspathInSandbox() throws Exception { + @Test + void testClasspathInSandbox() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); @@ -491,8 +500,9 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { assertEquals(testingDisplayName, b.getDisplayName()); } } - - @Test public void testNonapprovedClasspathInSandbox() throws Exception { + + @Test + void testNonapprovedClasspathInSandbox() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); @@ -577,8 +587,9 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { r.assertBuildStatusSuccess(p.scheduleBuild2(0)); } } - - @Test public void testUpdatedClasspath() throws Exception { + + @Test + void testUpdatedClasspath() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); @@ -589,7 +600,7 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { r.jenkins.setAuthorizationStrategy(mockStrategy); // Copy jar files to temporary directory, then overwrite them with updated jar files. - File tmpDir = tmpFolderRule.newFolder(); + File tmpDir = newFolder(tmpFolderRule, "junit"); for (File jarfile: getAllJarFiles()) { FileUtils.copyFileToDirectory(jarfile, tmpDir); @@ -660,8 +671,9 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { // Success as approved. r.assertBuildStatusSuccess(p.scheduleBuild2(0)); } - - @Test public void testClasspathWithClassDirectory() throws Exception { + + @Test + void testClasspathWithClassDirectory() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); @@ -672,7 +684,7 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { r.jenkins.setAuthorizationStrategy(mockStrategy); // Copy jar files to temporary directory, then overwrite them with updated jar files. - File tmpDir = tmpFolderRule.newFolder(); + File tmpDir = newFolder(tmpFolderRule, "junit"); for (File jarfile: getAllJarFiles()) { Expand e = new Expand(); @@ -711,8 +723,9 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { assertEquals(0, pcps.size()); } } - - @Test public void testDifferentClasspathButSameContent() throws Exception { + + @Test + void testDifferentClasspathButSameContent() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); @@ -777,8 +790,9 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { assertEquals(testingDisplayName, b.getDisplayName()); } } - - @Test public void testClasspathApproval() throws Exception { + + @Test + void testClasspathApproval() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); @@ -956,7 +970,7 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { * @throws Exception */ @Test - public void forceSandbox_NonAdminSaveNonSandbox() throws Exception { + void forceSandbox_NonAdminSaveNonSandbox() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); @@ -983,8 +997,9 @@ public void forceSandbox_NonAdminSaveNonSandbox() throws Exception { } } - @Test @Issue("SECURITY-2450") - public void testScriptApproval() throws Exception { + @Test + @Issue("SECURITY-2450") + void testScriptApproval() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); @@ -1101,8 +1116,9 @@ public void testScriptApproval() throws Exception { } } - @Test @Issue("JENKINS-25348") - public void testSandboxClassResolution() throws Exception { + @Test + @Issue("JENKINS-25348") + void testSandboxClassResolution() throws Exception { File jar = Which.jarFile(Checker.class); // this child-first classloader creates an environment in which another groovy-sandbox exists @@ -1117,9 +1133,10 @@ public void testSandboxClassResolution() throws Exception { () -> sgs.configuringWithKeyItem().evaluate(a, new Binding())); assertTrue(e.getMessage().contains("staticMethod java.lang.System gc")); } - + @Issue("SECURITY-1186") - @Test public void testFinalizersForbiddenInSandbox() throws Exception { + @Test + void testFinalizersForbiddenInSandbox() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); p.getPublishersList().add(new TestGroovyRecorder( new SecureGroovyScript("class Test { public void finalize() { } }; null", true, null))); @@ -1128,7 +1145,8 @@ public void testSandboxClassResolution() throws Exception { } @Issue("SECURITY-1186") - @Test public void testFinalizersAllowedWithWholeScriptApproval() throws Exception { + @Test + void testFinalizersAllowedWithWholeScriptApproval() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); mockStrategy.grant(Jenkins.READ).everywhere().to("dev"); @@ -1157,21 +1175,23 @@ public void testSandboxClassResolution() throws Exception { @Issue("SECURITY-1292") @Test - public void blockASTTest() throws Exception { + void blockASTTest() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); - assertThat(d.doCheckScript("import groovy.transform.*\n" + - "import jenkins.model.Jenkins\n" + - "import hudson.model.FreeStyleProject\n" + - "@ASTTest(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, \"should-not-exist\") })\n" + - "@Field int x\n" + - "echo 'hello'\n", false, null).toString(), containsString("Annotation ASTTest cannot be used in the sandbox")); + assertThat(d.doCheckScript(""" + import groovy.transform.* + import jenkins.model.Jenkins + import hudson.model.FreeStyleProject + @ASTTest(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, "should-not-exist") }) + @Field int x + echo 'hello' + """, false, null).toString(), containsString("Annotation ASTTest cannot be used in the sandbox")); assertNull(r.jenkins.getItem("should-not-exist")); } @Issue("SECURITY-1292") @Test - public void blockGrab() throws Exception { + void blockGrab() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); assertThat(d.doCheckScript("@Grab(group='foo', module='bar', version='1.0')\ndef foo\n", false, null).toString(), containsString("Annotation Grab cannot be used in the sandbox")); @@ -1179,7 +1199,7 @@ public void blockGrab() throws Exception { @Issue("SECURITY-1318") @Test - public void blockGrapes() throws Exception { + void blockGrapes() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); assertThat(d.doCheckScript("@Grapes([@Grab(group='foo', module='bar', version='1.0')])\ndef foo\n", false, null).toString(), containsString("Annotation Grapes cannot be used in the sandbox")); @@ -1187,7 +1207,7 @@ public void blockGrapes() throws Exception { @Issue("SECURITY-1318") @Test - public void blockGrabConfig() throws Exception { + void blockGrabConfig() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); assertThat(d.doCheckScript("@GrabConfig(autoDownload=false)\ndef foo\n", false, null).toString(), containsString("Annotation GrabConfig cannot be used in the sandbox")); @@ -1195,7 +1215,7 @@ public void blockGrabConfig() throws Exception { @Issue("SECURITY-1318") @Test - public void blockGrabExclude() throws Exception { + void blockGrabExclude() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); assertThat(d.doCheckScript("@GrabExclude(group='org.mortbay.jetty', module='jetty-util')\ndef foo\n", false, null).toString(), containsString("Annotation GrabExclude cannot be used in the sandbox")); @@ -1203,7 +1223,7 @@ public void blockGrabExclude() throws Exception { @Issue("SECURITY-1319") @Test - public void blockGrabResolver() throws Exception { + void blockGrabResolver() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); assertThat(d.doCheckScript("@GrabResolver(name='restlet.org', root='http://maven.restlet.org')\ndef foo\n", false, null).toString(), containsString("Annotation GrabResolver cannot be used in the sandbox")); @@ -1211,7 +1231,7 @@ public void blockGrabResolver() throws Exception { @Issue("SECURITY-1318") @Test - public void blockArbitraryAnnotation() throws Exception { + void blockArbitraryAnnotation() { try { System.setProperty(RejectASTTransformsCustomizer.class.getName() + ".ADDITIONAL_BLOCKED_TRANSFORMS", "groovy.transform.Field,groovy.transform.Immutable"); SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); @@ -1224,73 +1244,83 @@ public void blockArbitraryAnnotation() throws Exception { @Issue("SECURITY-1321") @Test - public void blockAnnotationCollector() throws Exception { + void blockAnnotationCollector() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); - assertThat(d.doCheckScript("import groovy.transform.*\n" + - "import jenkins.model.Jenkins\n" + - "import hudson.model.FreeStyleProject\n" + - "@AnnotationCollector([ASTTest]) @interface Lol {}\n" + - "@Lol(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, \"should-not-exist\") })\n" + - "@Field int x\n" + - "echo 'hello'\n", false, null).toString(), containsString("Annotation AnnotationCollector cannot be used in the sandbox")); + assertThat(d.doCheckScript(""" + import groovy.transform.* + import jenkins.model.Jenkins + import hudson.model.FreeStyleProject + @AnnotationCollector([ASTTest]) @interface Lol {} + @Lol(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, "should-not-exist") }) + @Field int x + echo 'hello' + """, false, null).toString(), containsString("Annotation AnnotationCollector cannot be used in the sandbox")); assertNull(r.jenkins.getItem("should-not-exist")); } @Issue("SECURITY-1320") @Test - public void blockFQCN() throws Exception { + void blockFQCN() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); - assertThat(d.doCheckScript("import groovy.transform.*\n" + - "import jenkins.model.Jenkins\n" + - "import hudson.model.FreeStyleProject\n" + - "@groovy.transform.ASTTest(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, \"should-not-exist\") })\n" + - "@Field int x\n" + - "echo 'hello'\n", false, null).toString(), containsString("Annotation groovy.transform.ASTTest cannot be used in the sandbox")); + assertThat(d.doCheckScript(""" + import groovy.transform.* + import jenkins.model.Jenkins + import hudson.model.FreeStyleProject + @groovy.transform.ASTTest(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, "should-not-exist") }) + @Field int x + echo 'hello' + """, false, null).toString(), containsString("Annotation groovy.transform.ASTTest cannot be used in the sandbox")); assertNull(r.jenkins.getItem("should-not-exist")); } @Issue("SECURITY-1320") @Test - public void blockImportAsBlockedAnnotation() throws Exception { + void blockImportAsBlockedAnnotation() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); - assertThat(d.doCheckScript("import groovy.transform.ASTTest as lolwut\n" + - "import jenkins.model.Jenkins\n" + - "import hudson.model.FreeStyleProject\n" + - "@lolwut(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, \"should-not-exist\") })\n" + - "int x\n" + - "echo 'hello'\n", false, null).toString(), containsString("Annotation groovy.transform.ASTTest cannot be used in the sandbox")); + assertThat(d.doCheckScript(""" + import groovy.transform.ASTTest as lolwut + import jenkins.model.Jenkins + import hudson.model.FreeStyleProject + @lolwut(value={ assert Jenkins.getInstance().createProject(FreeStyleProject.class, "should-not-exist") }) + int x + echo 'hello' + """, false, null).toString(), containsString("Annotation groovy.transform.ASTTest cannot be used in the sandbox")); assertNull(r.jenkins.getItem("should-not-exist")); } @Issue("SECURITY-1336") @Test - public void blockConstructorInvocationInCheck() throws Exception { + void blockConstructorInvocationInCheck() { SecureGroovyScript.DescriptorImpl d = r.jenkins.getDescriptorByType(SecureGroovyScript.DescriptorImpl.class); - assertThat(d.doCheckScript("import jenkins.model.Jenkins\n" + - "import hudson.model.FreeStyleProject\n" + - "public class DoNotRunConstructor {\n" + - " public DoNotRunConstructor() {\n" + - " assert Jenkins.getInstance().createProject(FreeStyleProject.class, \"should-not-exist\")\n" + - " }\n" + - "}\n", false, null).toString(), containsString("OK")); + assertThat(d.doCheckScript(""" + import jenkins.model.Jenkins + import hudson.model.FreeStyleProject + public class DoNotRunConstructor { + public DoNotRunConstructor() { + assert Jenkins.getInstance().createProject(FreeStyleProject.class, "should-not-exist") + } + } + """, false, null).toString(), containsString("OK")); assertNull(r.jenkins.getItem("should-not-exist")); } @Issue("SECURITY-1336") @Test - public void blockConstructorInvocationAtRuntime() throws Exception { + void blockConstructorInvocationAtRuntime() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); p.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript( - "class DoNotRunConstructor {\n" + - " static void main(String[] args) {}\n" + - " DoNotRunConstructor() {\n" + - " assert jenkins.model.Jenkins.instance.createProject(hudson.model.FreeStyleProject, 'should-not-exist')\n" + - " }\n" + - "}\n", true, null))); + """ + class DoNotRunConstructor { + static void main(String[] args) {} + DoNotRunConstructor() { + assert jenkins.model.Jenkins.instance.createProject(hudson.model.FreeStyleProject, 'should-not-exist') + } + } + """, true, null))); FreeStyleBuild b = p.scheduleBuild2(0).get(); assertNull(r.jenkins.getItem("should-not-exist")); r.assertBuildStatus(Result.FAILURE, b); @@ -1299,7 +1329,7 @@ public void blockConstructorInvocationAtRuntime() throws Exception { @Issue("JENKINS-56682") @Test - public void testScriptAtFieldInitializers() throws Exception { + void testScriptAtFieldInitializers() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); p.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript( "import groovy.transform.Field\n" + @@ -1312,7 +1342,8 @@ public void testScriptAtFieldInitializers() throws Exception { } @Issue("SECURITY-1465") - @Test public void blockLhsInMethodPointerExpression() throws Exception { + @Test + void blockLhsInMethodPointerExpression() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); p.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript( "({" + @@ -1324,7 +1355,8 @@ public void testScriptAtFieldInitializers() throws Exception { } @Issue("SECURITY-1465") - @Test public void blockRhsInMethodPointerExpression() throws Exception { + @Test + void blockRhsInMethodPointerExpression() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); p.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript( "1.&(System.getProperty('sandboxTransformsMethodPointerRhs'))()", true, null))); @@ -1333,25 +1365,28 @@ public void testScriptAtFieldInitializers() throws Exception { } @Issue("SECURITY-1465") - @Test public void blockCastingUnsafeUserDefinedImplementationsOfCollection() throws Exception { + @Test + void blockCastingUnsafeUserDefinedImplementationsOfCollection() throws Exception { // See additional info on this test case in `SandboxTransformerTest.sandboxWillNotCastNonStandardCollections()` over in groovy-sandbox. FreeStyleProject p = r.createFreeStyleProject(); p.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript( - "def i = 0\n" + - "(({-> if(i) {\n" + - " return ['secret.txt'] as Object[]\n" + - " } else {\n" + - " i = 1\n" + - " return null\n" + - " }\n" + - "} as Collection) as File) as Object[]", true, null))); + """ + def i = 0 + (({-> if(i) { + return ['secret.txt'] as Object[] + } else { + i = 1 + return null + } + } as Collection) as File) as Object[]""", true, null))); FreeStyleBuild b = r.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0)); // Before the security fix, fails with FileNotFoundException, bypassing the sandbox! r.assertLogContains("Casting non-standard Collections to a type via constructor is not supported", b); } @Issue("SECURITY-1465") - @Test public void blockCastingSafeUserDefinedImplementationsOfCollection() throws Exception { + @Test + void blockCastingSafeUserDefinedImplementationsOfCollection() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); p.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript( "({-> return ['secret.txt'] as Object[]} as Collection) as File", true, null))); @@ -1362,7 +1397,8 @@ public void testScriptAtFieldInitializers() throws Exception { } @Issue("SECURITY-1465") - @Test public void blockEnumConstants() throws Exception { + @Test + void blockEnumConstants() throws Exception { FreeStyleProject p1 = r.createFreeStyleProject(); p1.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript( "jenkins.YesNoMaybe.MAYBE", true, null))); @@ -1377,7 +1413,8 @@ public void testScriptAtFieldInitializers() throws Exception { } @Issue("SECURITY-2848") - @Test public void blockScriptClassesWithMainMethods() throws Exception { + @Test + void blockScriptClassesWithMainMethods() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); SecureGroovyScript s = new SecureGroovyScript( "class Test extends org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScriptTest$HasMainMethod { }", true, null); @@ -1388,7 +1425,8 @@ public void testScriptAtFieldInitializers() throws Exception { } @Issue("SECURITY-2824") - @Test public void blockCastingBindingValues() throws Exception { + @Test + void blockCastingBindingValues() throws Exception { FreeStyleProject p = r.createFreeStyleProject(); SecureGroovyScript s = new SecureGroovyScript( "class Test { File list }", true, null); @@ -1401,7 +1439,8 @@ public void testScriptAtFieldInitializers() throws Exception { r.assertLogContains("new java.io.File java.lang.String", b); } - @Test public void testApprovalFromFormValidation() throws Exception { + @Test + void testApprovalFromFormValidation() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); @@ -1459,9 +1498,19 @@ public static void main(String[] args) throws IOException { } @Issue("JENKINS-38908") - @Test public void groovyCallSiteSelectorMain() throws Exception { + @Test + void groovyCallSiteSelectorMain() throws Exception { Script receiver = (Script) new SecureGroovyScript("def main() {}; this", true, null).configuring(ApprovalContext.create()).evaluate(GroovyCallSiteSelectorTest.class.getClassLoader(), new Binding(), null); assertEquals(receiver.getClass().getMethod("main"), GroovyCallSiteSelector.method(receiver, "main", new Object[0])); assertEquals(receiver.getClass().getMethod("main", String[].class), GroovyCallSiteSelector.method(receiver, "main", new Object[] {"somearg"})); } + + private static File newFolder(File root, String... subDirs) throws IOException { + String subFolder = String.join("/", subDirs); + File result = new File(root, subFolder); + if (!result.mkdirs()) { + throw new IOException("Couldn't create folders " + root); + } + return result; + } } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder.java index 6b0e8b64..6e5ce3d6 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder.java @@ -43,13 +43,13 @@ * Sample of integrating {@link SecureGroovyScript}. * The result of the configured Groovy script is set as the build description. */ -@SuppressWarnings({"unchecked", "rawtypes"}) public final class TestGroovyRecorder extends Recorder { private final SecureGroovyScript script; private transient Binding binding; - @DataBoundConstructor public TestGroovyRecorder(SecureGroovyScript script) { + @DataBoundConstructor + public TestGroovyRecorder(SecureGroovyScript script) { this.script = script.configuringWithKeyItem(); } @@ -61,7 +61,8 @@ public void setBinding(Binding binding) { this.binding = binding; } - @Override public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { + @Override + public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { try { if (binding == null) { binding = new Binding(); @@ -74,18 +75,22 @@ public void setBinding(Binding binding) { return true; } - @Override public BuildStepMonitor getRequiredMonitorService() { + @Override + public BuildStepMonitor getRequiredMonitorService() { return BuildStepMonitor.NONE; } - @Extension public static final class DescriptorImpl extends BuildStepDescriptor { + @Extension + public static final class DescriptorImpl extends BuildStepDescriptor { @NonNull - @Override public String getDisplayName() { + @Override + public String getDisplayName() { return "Test Groovy Recorder"; } - @Override public boolean isApplicable(Class jobType) { + @Override + public boolean isApplicable(Class jobType) { return true; } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelistTest.java index 534c78eb..63f3c8ac 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelistTest.java @@ -31,10 +31,10 @@ import java.util.LinkedHashMap; import java.util.Map; -import org.junit.Assert; -import org.junit.Test; +import org.junit.jupiter.api.Test; + import org.jvnet.hudson.test.Issue; -import static org.junit.Assert.*; +import static org.junit.jupiter.api.Assertions.*; public class EnumeratingWhitelistTest { @@ -44,7 +44,8 @@ public static class C { public void m(Object[] args) {} } - @Test public void matches() throws Exception { + @Test + void matches() throws Exception { Method m = C.class.getMethod("m", Object[].class); assertTrue(new EnumeratingWhitelist.MethodSignature(C.class, "m", Object[].class).matches(m)); assertTrue(new EnumeratingWhitelist.MethodSignature(C.class, "*", Object[].class).matches(m)); @@ -57,7 +58,8 @@ public void m(Object[] args) {} assertFalse(new EnumeratingWhitelist.FieldSignature(C.class, "other").matches(f)); } - @Test public void getName() throws Exception { + @Test + void getName() throws Exception { assertEquals("java.lang.Object", EnumeratingWhitelist.getName(Object.class)); assertEquals("java.lang.Object[]", EnumeratingWhitelist.getName(Object[].class)); assertEquals("java.lang.Object[][]", EnumeratingWhitelist.getName(Object[][].class)); @@ -67,7 +69,8 @@ public void m(Object[] args) {} } } - @Test public void methodExists() throws Exception { + @Test + void methodExists() throws Exception { assertTrue(new EnumeratingWhitelist.MethodSignature(Object.class, "equals", Object.class).exists()); assertFalse(new EnumeratingWhitelist.MethodSignature(String.class, "equals", Object.class).exists()); assertFalse(new EnumeratingWhitelist.MethodSignature(String.class, "compareTo", Object.class).exists()); @@ -84,7 +87,7 @@ public void m(Object[] args) {} } @Test - public void isWildcard() throws Exception { + void isWildcard() { assertTrue(new EnumeratingWhitelist.MethodSignature(C.class, "*", Object[].class).isWildcard()); assertFalse(new EnumeratingWhitelist.MethodSignature(C.class, "m", Object[].class).isWildcard()); @@ -106,7 +109,7 @@ public Fancy(int ob){} /** Verifies for caching that canonical names match method signature toString, for cache keying. */ @Test - public void canonicalNaming() throws Exception { + void canonicalNaming() throws Exception { Method m = Fancy.class.getMethod("m", Object[].class); Method staticM = Fancy.class.getMethod("staticM", Object.class); Constructor con = Fancy.class.getConstructor(null); @@ -120,15 +123,15 @@ public void canonicalNaming() throws Exception { EnumeratingWhitelist.FieldSignature fSig = new EnumeratingWhitelist.FieldSignature(Fancy.class, "myF"); EnumeratingWhitelist.FieldSignature staticFSig = new EnumeratingWhitelist.StaticFieldSignature(Fancy.class.getName(), "myStaticF"); - Assert.assertEquals(mSig.toString(), EnumeratingWhitelist.canonicalMethodSig(m)); - Assert.assertEquals(staticMSig.toString(), EnumeratingWhitelist.canonicalStaticMethodSig(staticM)); - Assert.assertEquals(conSig.toString(), EnumeratingWhitelist.canonicalConstructorSig(con)); - Assert.assertEquals(fSig.toString(), EnumeratingWhitelist.canonicalFieldSig(f)); - Assert.assertEquals(staticFSig.toString(), EnumeratingWhitelist.canonicalStaticFieldSig(staticF)); + assertEquals(mSig.toString(), EnumeratingWhitelist.canonicalMethodSig(m)); + assertEquals(staticMSig.toString(), EnumeratingWhitelist.canonicalStaticMethodSig(staticM)); + assertEquals(conSig.toString(), EnumeratingWhitelist.canonicalConstructorSig(con)); + assertEquals(fSig.toString(), EnumeratingWhitelist.canonicalFieldSig(f)); + assertEquals(staticFSig.toString(), EnumeratingWhitelist.canonicalStaticFieldSig(staticF)); } @Test - public void caching() throws Exception { + void caching() throws Exception { StaticWhitelist myList = new StaticWhitelist(); Method m = Fancy.class.getMethod("m", Object[].class); @@ -156,7 +159,7 @@ public void caching() throws Exception { } @Test - public void testCachingWithWildcards() throws Exception { + void testCachingWithWildcards() throws Exception { StaticWhitelist myList = new StaticWhitelist(); Field f = C.class.getField("myField"); myList.fieldSignatures.add(new EnumeratingWhitelist.FieldSignature(C.class, "*")); @@ -166,7 +169,8 @@ public void testCachingWithWildcards() throws Exception { } @Issue("JENKINS-42214") - @Test public void fieldExists() throws Exception { + @Test + void fieldExists() throws Exception { assertTrue(new EnumeratingWhitelist.FieldSignature("hudson.model.Result", "color").exists()); assertTrue(new EnumeratingWhitelist.StaticFieldSignature("hudson.model.Result", "ABORTED").exists()); diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelistTest.java index 77a63a3a..d3016516 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelistTest.java @@ -26,44 +26,43 @@ import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptorTest; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ErrorCollector; +import org.junit.jupiter.api.Test; import org.jvnet.hudson.test.Issue; -public class GenericWhitelistTest { - - @Rule public ErrorCollector errors = new ErrorCollector(); +class GenericWhitelistTest { - @Test public void sanity() throws Exception { + @Test + void sanity() throws Exception { StaticWhitelistTest.sanity(StaticWhitelist.class.getResource("generic-whitelist")); } @Issue("SECURITY-538") - @Test public void dynamicSubscript() throws Exception { + @Test + void dynamicSubscript() throws Exception { String dangerous = Dangerous.class.getName(); Whitelist wl = new ProxyWhitelist(new GenericWhitelist(), new AnnotatedWhitelist()); // Control cases—explicit method call: - SandboxInterceptorTest.assertRejected(wl, "staticMethod " + dangerous + " isSecured", dangerous + ".isSecured()", errors); - SandboxInterceptorTest.assertRejected(wl, "staticMethod " + dangerous + " setSecured boolean", dangerous + ".setSecured(false)", errors); - SandboxInterceptorTest.assertRejected(wl, "method " + dangerous + " getSecret", "new " + dangerous + "().getSecret()", errors); - SandboxInterceptorTest.assertRejected(wl, "method " + dangerous + " setSecret java.lang.String", "new " + dangerous + "().setSecret('')", errors); + SandboxInterceptorTest.assertRejected(wl, "staticMethod " + dangerous + " isSecured", dangerous + ".isSecured()"); + SandboxInterceptorTest.assertRejected(wl, "staticMethod " + dangerous + " setSecured boolean", dangerous + ".setSecured(false)"); + SandboxInterceptorTest.assertRejected(wl, "method " + dangerous + " getSecret", "new " + dangerous + "().getSecret()"); + SandboxInterceptorTest.assertRejected(wl, "method " + dangerous + " setSecret java.lang.String", "new " + dangerous + "().setSecret('')"); // Control cases—statically resolvable property accesses: - SandboxInterceptorTest.assertRejected(wl, "staticMethod " + dangerous + " isSecured", dangerous + ".secured", errors); - SandboxInterceptorTest.assertRejected(wl, "staticMethod " + dangerous + " setSecured boolean", dangerous + ".secured = false", errors); - SandboxInterceptorTest.assertRejected(wl, "method " + dangerous + " getSecret", "new " + dangerous + "().secret", errors); - SandboxInterceptorTest.assertRejected(wl, "method " + dangerous + " setSecret java.lang.String", "new " + dangerous + "().secret = ''", errors); + SandboxInterceptorTest.assertRejected(wl, "staticMethod " + dangerous + " isSecured", dangerous + ".secured"); + SandboxInterceptorTest.assertRejected(wl, "staticMethod " + dangerous + " setSecured boolean", dangerous + ".secured = false"); + SandboxInterceptorTest.assertRejected(wl, "method " + dangerous + " getSecret", "new " + dangerous + "().secret"); + SandboxInterceptorTest.assertRejected(wl, "method " + dangerous + " setSecret java.lang.String", "new " + dangerous + "().secret = ''"); // Test cases—dynamically resolved property accesses: String getAt = "staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods getAt java.lang.Object java.lang.String"; String putAt = "staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods putAt java.lang.Object java.lang.String java.lang.Object"; - SandboxInterceptorTest.assertRejected(wl, getAt, dangerous + "['secured']", errors); - SandboxInterceptorTest.assertRejected(wl, putAt, dangerous + "['secured'] = false", errors); - SandboxInterceptorTest.assertRejected(wl, getAt, "new " + dangerous + "()['secret']", errors); - SandboxInterceptorTest.assertRejected(wl, putAt, "new " + dangerous + "()['secret'] = ''", errors); + SandboxInterceptorTest.assertRejected(wl, getAt, dangerous + "['secured']"); + SandboxInterceptorTest.assertRejected(wl, putAt, dangerous + "['secured'] = false"); + SandboxInterceptorTest.assertRejected(wl, getAt, "new " + dangerous + "()['secret']"); + SandboxInterceptorTest.assertRejected(wl, putAt, "new " + dangerous + "()['secret'] = ''"); // Test cases via JsonOutput. - SandboxInterceptorTest.assertRejected(wl, "staticMethod groovy.json.JsonOutput toJson java.lang.Object", "groovy.json.JsonOutput.toJson(new " + dangerous + "())", errors); + SandboxInterceptorTest.assertRejected(wl, "staticMethod groovy.json.JsonOutput toJson java.lang.Object", "groovy.json.JsonOutput.toJson(new " + dangerous + "())"); // toJson(Closure) seems blocked anyway by lack of access to JsonDelegate.content, directly or via GroovyObject.setProperty } + public static class Dangerous { @Whitelisted public Dangerous() {} diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelistTest.java index 5f5bfacd..680da124 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelistTest.java @@ -24,12 +24,12 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists; -import org.junit.Test; +import org.junit.jupiter.api.Test; -public class JenkinsWhitelistTest { - - @Test public void sanity() throws Exception { +class JenkinsWhitelistTest { + + @Test + void sanity() throws Exception { StaticWhitelistTest.sanity(StaticWhitelist.class.getResource("jenkins-whitelist")); } - } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelistTest.java index 680a7958..415841c1 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelistTest.java @@ -25,9 +25,11 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; -import org.junit.Test; import org.jvnet.hudson.test.Issue; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + import java.io.IOException; import java.io.StringReader; import java.util.Arrays; @@ -37,13 +39,13 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; -public class ProxyWhitelistTest { +class ProxyWhitelistTest { - @Test public void reset() throws Exception { + @Test + void reset() throws Exception { ProxyWhitelist pw1 = new ProxyWhitelist(new StaticWhitelist(new StringReader("method java.lang.String length"))); ProxyWhitelist pw2 = new ProxyWhitelist(pw1); assertTrue(pw2.permitsMethod(String.class.getMethod("length"), "x", new Object[0])); @@ -56,7 +58,8 @@ public class ProxyWhitelistTest { assertFalse(pw2.permitsMethod(Object.class.getMethod("hashCode"), "x", new Object[0])); } - @Test public void resetStaticField() throws Exception { + @Test + void resetStaticField() throws Exception { ProxyWhitelist pw1 = new ProxyWhitelist(new StaticWhitelist(new StringReader("staticField java.util.Collections EMPTY_LIST"))); ProxyWhitelist pw2 = new ProxyWhitelist(pw1); assertTrue(pw2.permitsStaticFieldGet(Collections.class.getField("EMPTY_LIST"))); @@ -73,8 +76,9 @@ public class ProxyWhitelistTest { * 2 delegates. If the tasks have not completed by then, we can assume that there is a problem. */ @Issue("JENKINS-41797") - @Test(timeout = 30000) - public void testConcurrent() throws InterruptedException, IOException { + @Test + @Timeout(value = 30000, unit = TimeUnit.MILLISECONDS) + void testConcurrent() throws InterruptedException, IOException { int threadPoolSize = 2; List delegates = Arrays.asList( new ProxyWhitelist(new StaticWhitelist(new StringReader("staticField java.util.Collections EMPTY_LIST"))), @@ -91,7 +95,7 @@ public void testConcurrent() throws InterruptedException, IOException { es.shutdown(); // If interrupted after the timeout, something went wrong - assert es.awaitTermination(15000, TimeUnit.MILLISECONDS); + assertTrue(es.awaitTermination(15000, TimeUnit.MILLISECONDS)); } } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelistTest.java index bcc3a23d..13a1c184 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelistTest.java @@ -47,13 +47,16 @@ import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist.NewSignature; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist.Signature; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist.StaticMethodSignature; -import org.junit.Assert; -import org.junit.Test; -import static org.junit.Assert.*; -public class StaticWhitelistTest { - - @Test public void dangerous() throws Exception { +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; + +class StaticWhitelistTest { + + @Test + void dangerous() throws Exception { assertFalse(StaticWhitelist.rejectMethod(Collection.class.getMethod("clear")).isDangerous()); assertTrue(StaticWhitelist.rejectNew(File.class.getConstructor(String.class)).isDangerous()); assertTrue(StaticWhitelist.rejectMethod(GroovyObject.class.getMethod("invokeMethod", String.class, Object.class)).isDangerous()); @@ -81,7 +84,7 @@ static void sanity(URL definition) throws Exception { hasDupes = true; } } - Assert.assertFalse("Whitelist contains duplicate entries, and this is not allowed! Please see list above.", hasDupes); + assertFalse(hasDupes, "Whitelist contains duplicate entries, and this is not allowed! Please see list above."); ArrayList sorted = new ArrayList<>(sigs); Collections.sort(sorted); @@ -95,7 +98,7 @@ static void sanity(URL definition) throws Exception { isUnsorted = true; } } - Assert.assertFalse("Whitelist is out of order! Please see issues above.", isUnsorted); + assertFalse(isUnsorted, "Whitelist is out of order! Please see issues above."); for (EnumeratingWhitelist.Signature sig : sigs) { @@ -103,7 +106,7 @@ static void sanity(URL definition) throws Exception { continue; } try { - assertTrue(sig + " does not exist (or is an override)", sig.exists()); + assertTrue(sig.exists(), sig + " does not exist (or is an override)"); } catch (ClassNotFoundException x) { // Wrapping exception to include the full signature in the error message. throw new Exception("Unable to verify existence of " + sig, x); @@ -169,7 +172,8 @@ static void sanity(URL definition) throws Exception { new MethodSignature(String.class, "getChars", int.class, int.class, char[].class, int.class) )); - @Test public void sanity() throws Exception { + @Test + void sanity() throws Exception { sanity(StaticWhitelist.class.getResource("blacklist")); } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/AbstractApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/AbstractApprovalTest.java index 07cd57d9..2b93a767 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/AbstractApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/AbstractApprovalTest.java @@ -24,22 +24,32 @@ package org.jenkinsci.plugins.scriptsecurity.scripts; -import org.junit.Rule; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; + +@WithJenkins +abstract class AbstractApprovalTest> { -public abstract class AbstractApprovalTest> { + protected JenkinsRule r; - @Rule public JenkinsRule r = new JenkinsRule(); + @BeforeEach + void beforeEach(JenkinsRule rule) { + r = rule; + } /** Creates a new approvable to test. */ abstract T create() throws Exception; - @Test public void noSecurity() throws Exception { + @Test + void noSecurity() throws Exception { create().use(); } - @Test public void withSecurity() throws Exception { + @Test + void withSecurity() throws Exception { configureSecurity(); // Cannot use until approved create().assertCannotUse().approve().use(); @@ -64,7 +74,8 @@ private Approvable[] createFiveEntries() throws Exception { return entries; } - @Test public void approveInternal() throws Exception { + @Test + void approveInternal() throws Exception { configureSecurity(); final Approvable[] entries = createFiveEntries(); entries[0].approve().assertApproved(); @@ -82,7 +93,8 @@ private Approvable[] createFiveEntries() throws Exception { } - @Test public void approveExternal() throws Exception { + @Test + void approveExternal() throws Exception { configureSecurity(); final Approvable[] entries = createFiveEntries(); diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Approvable.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Approvable.java index 167a3349..062b6aab 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Approvable.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Approvable.java @@ -24,9 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.scripts; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.*; /** * Base class for approvable entities. @@ -56,20 +54,20 @@ final T assertCannotUse() throws Exception { } final T assertPending() { - assertTrue(this + " should be pending", findPending()); - assertFalse(this + " shouldn't be approved", findApproved()); + assertTrue(findPending(), this + " should be pending"); + assertFalse(findApproved(), this + " shouldn't be approved"); return self(); } final T assertApproved() { - assertFalse(this + " shouldn't be pending", findPending()); - assertTrue(this + " should be approved", findApproved()); + assertFalse(findPending(), this + " shouldn't be pending"); + assertTrue(findApproved(), this + " should be approved"); return self(); } final T assertDeleted() { - assertFalse(this + " shouldn't be pending", findPending()); - assertFalse(this + " shouldn't be approved", findApproved()); + assertFalse(findPending(), this + " shouldn't be pending"); + assertFalse(findApproved(), this + " shouldn't be approved"); return self(); } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java index e926b7dd..f7b4d960 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java @@ -27,6 +27,7 @@ import hudson.Functions; import java.io.File; +import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; import java.nio.file.Files; @@ -34,35 +35,46 @@ import jenkins.model.Jenkins; import org.htmlunit.html.HtmlPage; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.emptyString; -import static org.junit.Assert.*; +import static org.junit.jupiter.api.Assertions.*; import org.jvnet.hudson.test.*; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; -public class ClasspathEntryTest { - @Rule public TemporaryFolder rule = new TemporaryFolder(); - @Rule public JenkinsRule jr = new JenkinsRule(); +@WithJenkins +class ClasspathEntryTest { + + @TempDir + private File tmp; + + private JenkinsRule r; + + @BeforeEach + void beforeEach(JenkinsRule rule) { + r = rule; + } @Issue("SECURITY-3447") @Test - public void testDoCheckPath() throws Exception { - jr.jenkins.setSecurityRealm(jr.createDummySecurityRealm()); - jr.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy(). + void testDoCheckPath() throws Exception { + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy(). grant(Jenkins.ADMINISTER).everywhere().to("admin") .grant(Jenkins.READ).everywhere().to("dev")); Path path = Files.createTempDirectory("temp dir"); - try(JenkinsRule.WebClient webClient = jr.createWebClient()) { + try(JenkinsRule.WebClient webClient = r.createWebClient()) { webClient.login("admin"); final HtmlPage adminPage = webClient.goTo("descriptor/org.jenkinsci.plugins.scriptsecurity.scripts.ClasspathEntry/checkPath?value=" + path.toUri()); final String adminContent = adminPage.asXml(); assertThat(adminContent, containsString("Class directories are not allowed as classpath entries.")); } - try (JenkinsRule.WebClient devWebClient = jr.createWebClient()) { + try (JenkinsRule.WebClient devWebClient = r.createWebClient()) { devWebClient.login("dev"); final HtmlPage devPage = devWebClient.goTo("descriptor/org.jenkinsci.plugins.scriptsecurity.scripts.ClasspathEntry/checkPath?value=" + path.toUri()); final String devContent = devPage.asNormalizedText(); @@ -73,7 +85,8 @@ public void testDoCheckPath() throws Exception { } @WithoutJenkins - @Test public void pathURLConversion() throws Exception { + @Test + void pathURLConversion() throws Exception { if (!Functions.isWindows()) { assertRoundTrip("/tmp/x.jar", "file:/tmp/x.jar"); } else { @@ -88,29 +101,46 @@ private static void assertRoundTrip(String path, String url) throws Exception { } @WithoutJenkins - @Test public void classDirDetected() throws Exception { - final File tmpDir = rule.newFolder(); - assertTrue("Existing directory must be detected", ClasspathEntry.isClassDirectoryURL(tmpDir.toURI().toURL())); + @Test + void classDirDetected() throws Exception { + final File tmpDir = newFolder(tmp, "junit"); + assertTrue(ClasspathEntry.isClassDirectoryURL(tmpDir.toURI().toURL()), "Existing directory must be detected"); tmpDir.delete(); final File notExisting = new File(tmpDir, "missing"); final URL missing = tmpDir.toURI().toURL(); - assertFalse("Non-existing file is not considered class directory", ClasspathEntry.isClassDirectoryURL(missing)); + assertFalse(ClasspathEntry.isClassDirectoryURL(missing), "Non-existing file is not considered class directory"); final URL oneDir = new URL(missing.toExternalForm() + "/"); - assertTrue("Non-existing file is considered class directory if ending in /", ClasspathEntry.isClassDirectoryURL(oneDir)); - assertTrue("Generic URLs ending in / are considered class directories", ClasspathEntry.isClassDirectoryURL(new URL("http://example.com/folder/"))); - assertFalse("Generic URLs ending in / are not considered class directories", ClasspathEntry.isClassDirectoryURL(new URL("http://example.com/file"))); + assertTrue(ClasspathEntry.isClassDirectoryURL(oneDir), "Non-existing file is considered class directory if ending in /"); + assertTrue(ClasspathEntry.isClassDirectoryURL(new URL("http://example.com/folder/")), "Generic URLs ending in / are considered class directories"); + assertFalse(ClasspathEntry.isClassDirectoryURL(new URL("http://example.com/file")), "Generic URLs ending in / are not considered class directories"); } @WithoutJenkins @Issue("JENKINS-37599") - @Test public void pathToURL() throws Exception { + @Test + void pathToURL() throws Exception { ClasspathEntry ignore = new ClasspathEntry("http://nowhere.net/"); - ignore = new ClasspathEntry(rule.newFile("x.jar").getAbsolutePath()); - ignore = new ClasspathEntry(rule.newFolder().getAbsolutePath()); + ignore = new ClasspathEntry(newFile(tmp, "x.jar").getAbsolutePath()); + ignore = new ClasspathEntry(newFolder(tmp, "junit").getAbsolutePath()); assertThrows(MalformedURLException.class, () -> new ClasspathEntry("")); assertThrows(MalformedURLException.class, () -> new ClasspathEntry(" ")); assertThrows(MalformedURLException.class, () -> new ClasspathEntry("relative")); } + private static File newFolder(File root, String... subDirs) throws IOException { + String subFolder = String.join("/", subDirs); + File result = new File(root, subFolder); + if (!result.mkdirs()) { + throw new IOException("Couldn't create folders " + root); + } + return result; + } + + private static File newFile(File parent, String child) throws IOException { + File result = new File(parent, child); + result.createNewFile(); + return result; + } + } \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/EntryApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/EntryApprovalTest.java index bf9ce0fe..c71d758f 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/EntryApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/EntryApprovalTest.java @@ -26,9 +26,9 @@ import hudson.Util; import org.apache.commons.io.FileUtils; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + import org.jvnet.hudson.test.WithoutJenkins; import java.io.File; @@ -37,18 +37,19 @@ import java.security.SecureRandom; import java.util.TreeSet; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; -public final class EntryApprovalTest extends AbstractApprovalTest { +class EntryApprovalTest extends AbstractApprovalTest { - @Rule public TemporaryFolder tmpFolderRule = new TemporaryFolder(); + @TempDir + private File tmp; private final SecureRandom random = new SecureRandom(); @Override Entry create() throws Exception { - final File file = tmpFolderRule.newFile(); + final File file = File.createTempFile("junit", null, tmp); final byte[] bytes = new byte[1024]; random.nextBytes(bytes); FileUtils.writeByteArrayToFile(file, bytes); @@ -66,15 +67,17 @@ String getClearAllApprovedId() { } - @Test public void classDirRejectedEvenWithNoSecurity() throws Exception { - entry(tmpFolderRule.newFolder()); - assertTrue("Class directory shouldn't be pending", ScriptApproval.get().getPendingClasspathEntries().isEmpty()); - assertTrue("Class directory shouldn't be accepted", ScriptApproval.get().getApprovedClasspathEntries().isEmpty()); + @Test + void classDirRejectedEvenWithNoSecurity() throws Exception { + entry(newFolder(tmp, "junit")); + assertTrue(ScriptApproval.get().getPendingClasspathEntries().isEmpty(), "Class directory shouldn't be pending"); + assertTrue(ScriptApproval.get().getApprovedClasspathEntries().isEmpty(), "Class directory shouldn't be accepted"); } // http://stackoverflow.com/a/25393190/12916 @WithoutJenkins - @Test public void getPendingClasspathEntry() throws Exception { + @Test + void getPendingClasspathEntry() throws Exception { TreeSet pendingClasspathEntries = new TreeSet<>(); for (int i = 1; i < 100; i++) { pendingClasspathEntries.add(new ScriptApproval.PendingClasspathEntry(hashOf(i), new URL("file:/x" + i + ".jar"), ApprovalContext.create())); @@ -84,6 +87,7 @@ String getClearAllApprovedId() { assertEquals(real, dummy); assertEquals("file:/x77.jar", real.getURL().toString()); } + private static String hashOf(int i) { return Util.getDigestOf("hash #" + i); } @@ -197,4 +201,13 @@ public String toString() { return String.format("ClasspathEntry[%s]", entry.getURL()); } } + + private static File newFolder(File root, String... subDirs) throws IOException { + String subFolder = String.join("/", subDirs); + File result = new File(root, subFolder); + if (!result.mkdirs()) { + throw new IOException("Couldn't create folders " + root); + } + return result; + } } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/HasherScriptApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/HasherScriptApprovalTest.java index ad10842e..3941732a 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/HasherScriptApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/HasherScriptApprovalTest.java @@ -3,34 +3,38 @@ import org.hamcrest.Matcher; import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage; import edu.umd.cs.findbugs.annotations.NonNull; -import org.junit.Rule; -import org.junit.Test; +import org.junit.jupiter.api.Test; + +import org.junit.jupiter.api.extension.RegisterExtension; import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.JenkinsSessionRule; -import org.jvnet.hudson.test.LoggerRule; +import org.jvnet.hudson.test.LogRecorder; +import org.jvnet.hudson.test.junit.jupiter.JenkinsSessionExtension; import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; import java.util.logging.Level; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInRelativeOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -public class HasherScriptApprovalTest { - @Rule - public JenkinsSessionRule session = new JenkinsSessionRule(); - @Rule - public LoggerRule log = new LoggerRule(); +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class HasherScriptApprovalTest { + + @RegisterExtension + private final JenkinsSessionExtension session = new JenkinsSessionExtension(); + + private final LogRecorder log = new LogRecorder(); @Test @Issue("SECURITY-2564") - public void hasherMatchesItsOwnHashes() throws Throwable { + void hasherMatchesItsOwnHashes() throws Throwable { session.then(r -> { for (ScriptApproval.Hasher hasher : ScriptApproval.Hasher.values()) { assertTrue(hasher.pattern().matcher(hasher.hash("Hello World", "Text")).matches()); @@ -40,7 +44,7 @@ public void hasherMatchesItsOwnHashes() throws Throwable { @Test @Issue("SECURITY-2564") - public void warnsAndClearsDeprecatedScriptHashes() throws Throwable { + void warnsAndClearsDeprecatedScriptHashes() throws Throwable { session.then(r -> { final ScriptApproval approval = ScriptApproval.get(); approval.approveScript(ScriptApproval.Hasher.SHA1.hash("Hello World", "Text")); @@ -61,7 +65,7 @@ public void warnsAndClearsDeprecatedScriptHashes() throws Throwable { @Test @Issue("SECURITY-2564") - public void convertsScriptApprovalsOnUse() throws Throwable { + void convertsScriptApprovalsOnUse() throws Throwable { final String script = "node { echo 'Hello World' }"; final Matcher> logMatcher = containsInRelativeOrder( containsString("A script is approved with an old hash algorithm. Converting now, ")); @@ -91,7 +95,7 @@ public void convertsScriptApprovalsOnUse() throws Throwable { @Test @Issue("SECURITY-2564") - public void testConvertApprovedClasspathEntries() throws Throwable { + void testConvertApprovedClasspathEntries() throws Throwable { session.then(r -> { final ScriptApproval approval = ScriptApproval.get(); addApprovedClasspathEntries(approval); @@ -110,19 +114,15 @@ public void testConvertApprovedClasspathEntries() throws Throwable { assertThat(log.getMessages(), containsInRelativeOrder( containsString("Scheduling conversion of 2 deprecated approved classpathentry hashes."), containsString("Background conversion task scheduled."))); - try { - while (approval.isConvertingDeprecatedApprovedClasspathEntries()) { - Thread.sleep(500); - } - } catch (InterruptedException ignored) { - } + + await().until(() -> !approval.isConvertingDeprecatedApprovedClasspathEntries()); assertEquals(0, approval.countDeprecatedApprovedClasspathHashes()); }); } @Test @Issue("SECURITY-2564") - public void testClasspathEntriesConvertedOnUse() throws Throwable { + void testClasspathEntriesConvertedOnUse() throws Throwable { session.then(r -> { final ScriptApproval approval = ScriptApproval.get(); addApprovedClasspathEntries(approval); diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/JcascTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/JcascTest.java index 59b9fd11..f6d144ad 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/JcascTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/JcascTest.java @@ -5,10 +5,11 @@ import io.jenkins.plugins.casc.misc.ConfiguredWithCode; import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule; +import io.jenkins.plugins.casc.misc.junit.jupiter.WithJenkinsConfiguredWithCode; import io.jenkins.plugins.casc.model.CNode; -import org.junit.ClassRule; -import org.junit.Test; -import org.jvnet.hudson.test.LoggerRule; +import org.junit.jupiter.api.Test; + +import org.jvnet.hudson.test.LogRecorder; import java.util.logging.Level; @@ -18,29 +19,24 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder; import static org.hamcrest.core.StringContains.containsString; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; -public class JcascTest { +@WithJenkinsConfiguredWithCode +class JcascTest { - @ClassRule(order = 1) - public static LoggerRule logger = new LoggerRule().record(ScriptApproval.class.getName(), Level.WARNING) + private final LogRecorder logger = new LogRecorder().record(ScriptApproval.class.getName(), Level.WARNING) .capture(100); - @ClassRule(order = 2) - @ConfiguredWithCode("smoke_test.yaml") - public static JenkinsConfiguredWithCodeRule j = new JenkinsConfiguredWithCodeRule(); - - - @Test - public void smokeTestEntry() throws Exception { + @ConfiguredWithCode("smoke_test.yaml") + void smokeTestEntry(JenkinsConfiguredWithCodeRule j) { String[] approved = ScriptApproval.get().getApprovedSignatures(); assertEquals(1, approved.length); - assertEquals(approved[0], "method java.net.URI getHost"); + assertEquals("method java.net.URI getHost", approved[0]); String[] approvedScriptHashes = ScriptApproval.get().getApprovedScriptHashes(); assertEquals(1, approvedScriptHashes.length); - assertEquals(approvedScriptHashes[0], "fccae58c5762bdd15daca97318e9d74333203106"); + assertEquals("fccae58c5762bdd15daca97318e9d74333203106", approvedScriptHashes[0]); assertThat(logger.getMessages(), containsInAnyOrder( containsString("Adding deprecated script hash " + "that will be converted on next use: fccae58c5762bdd15daca97318e9d74333203106"))); @@ -48,7 +44,8 @@ public void smokeTestEntry() throws Exception { } @Test - public void smokeTestExport() throws Exception { + @ConfiguredWithCode("smoke_test.yaml") + void smokeTestExport(JenkinsConfiguredWithCodeRule j) throws Exception { ConfiguratorRegistry registry = ConfiguratorRegistry.get(); ConfigurationContext context = new ConfigurationContext(registry); CNode yourAttribute = getSecurityRoot(context).get("scriptApproval"); diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Manager.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Manager.java index cffabb6f..3aeeb5a7 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Manager.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Manager.java @@ -32,9 +32,7 @@ import java.io.IOException; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.*; /** * Object representing the script approval page. @@ -61,13 +59,13 @@ Manager click(String id) throws IOException { } Manager notFound(Approvable a, String id) { - assertNull(String.format("%s : Element %s should not exist", a, id), page.getElementById(id)); + assertNull(page.getElementById(id), String.format("%s : Element %s should not exist", a, id)); return this; } private DomElement assertFound(Approvable a, String id) { DomElement dom = page.getElementById(id); - assertNotNull(String.format("%s : Element %s should exist", this, id), dom); + assertNotNull(dom, String.format("%s : Element %s should exist", this, id)); return dom; } @@ -105,7 +103,7 @@ T deny() throws IOException { } T delete() throws IOException { - assertTrue(approvable + "must support deletion", approvable.canDelete()); + assertTrue(approvable.canDelete(), approvable + "must support deletion"); approvable.assertApproved(); return click("delete"); } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLoadingTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLoadingTest.java index 4fc7c14e..14c76db9 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLoadingTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLoadingTest.java @@ -29,19 +29,23 @@ import org.apache.commons.io.FileUtils; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.arrayContaining; -import org.junit.AssumptionViolatedException; -import org.junit.Rule; -import org.junit.Test; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import org.junit.jupiter.api.Test; + +import org.junit.jupiter.api.extension.RegisterExtension; import org.jvnet.hudson.test.JenkinsRule; -import org.jvnet.hudson.test.RealJenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.RealJenkinsExtension; -public final class ScriptApprovalLoadingTest { +class ScriptApprovalLoadingTest { - @Rule public final RealJenkinsRule rr = new RealJenkinsRule(); + @RegisterExtension + private final RealJenkinsExtension extension = new RealJenkinsExtension(); - @Test public void dynamicLoading() throws Throwable { - rr.then(ScriptApprovalLoadingTest::_dynamicLoading1); - rr.then(ScriptApprovalLoadingTest::_dynamicLoading2); + @Test + void dynamicLoading() throws Throwable { + extension.then(ScriptApprovalLoadingTest::_dynamicLoading1); + extension.then(ScriptApprovalLoadingTest::_dynamicLoading2); } private static void _dynamicLoading1(JenkinsRule r) throws Throwable { @@ -56,11 +60,10 @@ private static void _dynamicLoading2(JenkinsRule r) throws Throwable { try { r.jenkins.pluginManager.dynamicLoad(plugin); } catch (RestartRequiredException x) { - throw new AssumptionViolatedException("perhaps running in PCT, where this cannot be tested", x); + assumeTrue(false, "perhaps running in PCT, where this cannot be tested: " + x); } ScriptApproval sa = ScriptApproval.get(); sa.approveSignature("method java.lang.Object wait"); assertThat(sa.getApprovedSignatures(), arrayContaining("method java.lang.Object wait")); } - } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalNoteTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalNoteTest.java index c46a0171..a5194271 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalNoteTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalNoteTest.java @@ -35,26 +35,38 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertEquals; import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript; + +import static org.junit.jupiter.api.Assertions.assertEquals; import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.TestGroovyRecorder; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.Rule; -import org.jvnet.hudson.test.BuildWatcher; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.junit.jupiter.api.extension.RegisterExtension; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.MockAuthorizationStrategy; +import org.jvnet.hudson.test.junit.jupiter.BuildWatcherExtension; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; -public class ScriptApprovalNoteTest { +@WithJenkins +class ScriptApprovalNoteTest { - @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); + @SuppressWarnings("unused") + @RegisterExtension + private static final BuildWatcherExtension BUILD_WATCHER = new BuildWatcherExtension(); - @Rule public JenkinsRule r = new JenkinsRule(); + private JenkinsRule r; + + @BeforeEach + void beforeEach(JenkinsRule rule) { + r = rule; + } @Issue("JENKINS-34973") - @Test public void smokes() throws Exception { + @Test + void smokes() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy(). grant(Jenkins.ADMINISTER, Jenkins.READ, Item.READ).everywhere().to("adminUser"). @@ -85,5 +97,4 @@ public class ScriptApprovalNoteTest { TextPage raw2 = (TextPage)wc.goTo(b.getUrl()+"consoleText","text/plain"); assertThat(raw2.getContent(), containsString(" getInstance. " + Messages.ScriptApprovalNote_message())); } - } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java index e25bd110..b893a4d4 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java @@ -45,11 +45,11 @@ import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript; import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.TestGroovyRecorder; import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage; -import org.junit.Rule; -import org.junit.Test; +import org.junit.jupiter.api.Test; + import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; -import org.jvnet.hudson.test.LoggerRule; +import org.jvnet.hudson.test.LogRecorder; import org.jvnet.hudson.test.MockAuthorizationStrategy; import org.jvnet.hudson.test.recipes.LocalData; import org.xml.sax.SAXException; @@ -59,20 +59,17 @@ import java.util.List; import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; +import java.util.logging.LogRecord; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasItemInArray; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.*; + +class ScriptApprovalTest extends AbstractApprovalTest { -public class ScriptApprovalTest extends AbstractApprovalTest { - @Rule - public LoggerRule logging = new LoggerRule().record(ScriptApproval.class, Level.FINER).capture(100); + private final LogRecorder logging = new LogRecorder().record(ScriptApproval.class, Level.FINER).capture(100); private static final String CLEAR_ALL_ID = "approvedScripts-clear"; @@ -81,7 +78,8 @@ public class ScriptApprovalTest extends AbstractApprovalTest r.getMessage()).toArray(String[]::new), + assertThat(logging.getRecords().stream().map(LogRecord::getMessage).toArray(String[]::new), hasItemInArray("Malformed signature entry in scriptApproval.xml: ' new java.lang.Exception java.lang.String'")); } - @Test @LocalData("dangerousApproved") public void dangerousApprovedSignatures() { + @Test + @LocalData("dangerousApproved") + void dangerousApprovedSignatures() { String[] dangerousSignatures = ScriptApproval.get().getDangerousApprovedSignatures(); assertEquals(1, dangerousSignatures.length); } - @Test @LocalData("dangerousApproved") public void dangerousApprovedWarnings() throws IOException, SAXException { + @Test + @LocalData("dangerousApproved") + void dangerousApprovedWarnings() throws IOException, SAXException { JenkinsRule.WebClient wc = r.createWebClient(); HtmlPage managePage = wc.goTo("manage"); @@ -122,12 +124,14 @@ public void malformedScriptApproval() throws Exception { assertThat(dangerousTextArea.getTextContent(), Matchers.containsString(DANGEROUS_SIGNATURE)); } - @Test public void nothingHappening() throws Exception { + @Test + void nothingHappening() throws Exception { assertThat(r.createWebClient().goTo("manage").getByXPath("//a[@href='scriptApproval']"), Matchers.empty()); } @Issue("SECURITY-1866") - @Test public void classpathEntriesEscaped() throws Exception { + @Test + void classpathEntriesEscaped() throws Exception { // Add pending classpath entry. final UnapprovedClasspathException e = assertThrows(UnapprovedClasspathException.class, () -> ScriptApproval.get().using(new ClasspathEntry("https://www.example.com/#value=HackHack"))); @@ -143,7 +147,8 @@ public void malformedScriptApproval() throws Exception { assertThat(approvedPage.getElementById("xss"), nullValue()); } - @Test public void clearMethodsLifeCycle() throws Exception { + @Test + void clearMethodsLifeCycle() throws Exception { ScriptApproval sa = ScriptApproval.get(); assertEquals(0, sa.getApprovedSignatures().length); @@ -172,7 +177,7 @@ public void malformedScriptApproval() throws Exception { @Issue({"JENKINS-57563", "JENKINS-62708"}) @LocalData // Just a scriptApproval.xml that whitelists 'staticMethod jenkins.model.Jenkins getInstance' and a script printing all labels @Test - public void upgradeSmokes() throws Exception { + void upgradeSmokes() throws Exception { configureSecurity(); FreeStyleProject p = r.createFreeStyleProject(); p.getPublishersList().add(new TestGroovyRecorder( @@ -188,7 +193,7 @@ public void upgradeSmokes() throws Exception { @LocalData // Some scriptApproval.xml with existing signatures approved @Test - public void reload() throws Exception { + void reload() throws Exception { configureSecurity(); ScriptApproval sa = ScriptApproval.get(); @@ -210,7 +215,7 @@ public void reload() throws Exception { } @Test - public void forceSandboxTests() throws Exception { + void forceSandboxTests() throws Exception { setBasicSecurity(); try (ACLContext ctx = ACL.as(User.getById("devel", true))) { @@ -275,7 +280,7 @@ public void forceSandboxTests() throws Exception { } @Test - public void forceSandboxScriptSignatureException() throws Exception { + void forceSandboxScriptSignatureException() throws Exception { ScriptApproval.get().setForceSandbox(true); FreeStyleProject p = r.createFreeStyleProject("p"); p.getPublishersList().add(new TestGroovyRecorder(new SecureGroovyScript("jenkins.model.Jenkins.instance", true, null))); @@ -284,7 +289,7 @@ public void forceSandboxScriptSignatureException() throws Exception { } @Test - public void forceSandboxFormValidation() throws Exception { + void forceSandboxFormValidation() { setBasicSecurity(); try (ACLContext ctx = ACL.as(User.getById("devel", true))) { @@ -330,7 +335,7 @@ public void forceSandboxFormValidation() throws Exception { } @Test - public void shouldHideSandboxTest() throws Exception { + void shouldHideSandboxTest() throws Exception { setBasicSecurity(); ScriptApproval.get().setForceSandbox(true); @@ -366,7 +371,7 @@ public void shouldHideSandboxTest() throws Exception { } @Test - public void validateSandboxTest() throws Exception { + void validateSandboxTest() throws Exception { setBasicSecurity(); ScriptApproval.get().setForceSandbox(true); @@ -400,8 +405,7 @@ public void validateSandboxTest() throws Exception { * Devel: overall Read and write without admin permission * admin: System administrator */ - private void setBasicSecurity() - { + private void setBasicSecurity() { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); ScriptApproval.get().setForceSandbox(true); @@ -530,5 +534,4 @@ public String toString() { return String.format("Script[%s]", groovy); } } - }