diff --git a/pom.xml b/pom.xml index b54f26106..37e96edd0 100644 --- a/pom.xml +++ b/pom.xml @@ -31,7 +31,7 @@ <properties> <revision>1.27</revision> <changelist>-SNAPSHOT</changelist> - <jenkins.version>2.190.3</jenkins.version> + <jenkins.version>2.222.4</jenkins.version> <java.level>8</java.level> <pipeline-model-definition-plugin.version>1.8.1</pipeline-model-definition-plugin.version> </properties> @@ -51,49 +51,19 @@ <dependencies> <dependency> <groupId>io.jenkins.tools.bom</groupId> - <artifactId>bom-2.190.x</artifactId> - <version>14</version> + <artifactId>bom-2.222.x</artifactId> + <version>887.vae9c8ac09ff7</version> <scope>import</scope> <type>pom</type> </dependency> - <dependency> <!-- TODO until we are on 2.222.x and can use bom 22+ --> - <groupId>org.jenkinsci.plugins</groupId> - <artifactId>pipeline-model-api</artifactId> - <version>${pipeline-model-definition-plugin.version}</version> - </dependency> - <dependency> - <groupId>org.jenkinsci.plugins</groupId> - <artifactId>pipeline-model-definition</artifactId> - <version>${pipeline-model-definition-plugin.version}</version> - </dependency> - <dependency> - <groupId>org.jenkinsci.plugins</groupId> - <artifactId>pipeline-model-definition</artifactId> - <classifier>tests</classifier> - <version>${pipeline-model-definition-plugin.version}</version> - </dependency> - <dependency> - <groupId>org.jenkinsci.plugins</groupId> - <artifactId>pipeline-model-extensions</artifactId> - <version>${pipeline-model-definition-plugin.version}</version> - </dependency> - <dependency> - <groupId>org.jenkinsci.plugins</groupId> - <artifactId>pipeline-stage-tags-metadata</artifactId> - <version>${pipeline-model-definition-plugin.version}</version> - </dependency> - <dependency> <!-- TODO bom problem --> - <groupId>org.jenkins-ci.plugins</groupId> - <artifactId>jackson2-api</artifactId> - <scope>test</scope> - </dependency> + </dependencies> </dependencyManagement> <dependencies> <dependency> <groupId>org.jenkins-ci.plugins</groupId> <artifactId>docker-commons</artifactId> - <version>1.14</version> + <version>1.18</version> </dependency> <dependency> <groupId>org.jenkins-ci.plugins.workflow</groupId> diff --git a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy index 71308f1f1..de5296d6d 100644 --- a/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy +++ b/src/main/resources/org/jenkinsci/plugins/docker/workflow/Docker.groovy @@ -82,14 +82,21 @@ class Docker implements Serializable { } public Image build(String image, String args = '.') { + check(image) node { - def commandLine = "docker build -t ${image} ${args}" - - script."${shell()}" commandLine + def commandLine = 'docker build -t "$JD_IMAGE" ' + args + script.withEnv(["JD_IMAGE=${image}"]) { + script."${shell()}" commandLine + } this.image(image) } } + @com.cloudbees.groovy.cps.NonCPS + private static void check(String id) { + org.jenkinsci.plugins.docker.commons.credentials.ImageNameValidator.checkUserAndRepo(id) + } + public static class Image implements Serializable { private final Docker docker; @@ -97,6 +104,7 @@ class Docker implements Serializable { private ImageNameTokens parsedId; private Image(Docker docker, String id) { + check(id) this.docker = docker this.id = id this.parsedId = new ImageNameTokens(id) @@ -113,14 +121,16 @@ class Docker implements Serializable { public <V> V inside(String args = '', Closure<V> body) { docker.node { def toRun = imageName() - if (toRun != id && docker.script."${docker.shell()}"(script: "docker inspect -f . ${id}", returnStatus: true) == 0) { - // Can run it without registry prefix, because it was locally built. - toRun = id - } else { - if (docker.script."${docker.shell()}"(script: "docker inspect -f . ${toRun}", returnStatus: true) != 0) { - // Not yet present locally. - // withDockerContainer requires the image to be available locally, since its start phase is not a durable task. - pull() + docker.script.withEnv(["JD_ID=${id}", "JD_TO_RUN=${toRun}"]) { + if (toRun != id && docker.script."${docker.shell()}"(script: 'docker inspect -f . "$JD_ID"', returnStatus: true) == 0) { + // Can run it without registry prefix, because it was locally built. + toRun = id + } else { + if (docker.script."${docker.shell()}"(script: 'docker inspect -f . "$JD_TO_RUN"', returnStatus: true) != 0) { + // Not yet present locally. + // withDockerContainer requires the image to be available locally, since its start phase is not a durable task. + pull() + } } } docker.script.withDockerContainer(image: toRun, args: args, toolName: docker.script.env.DOCKER_TOOL_NAME) { @@ -131,7 +141,10 @@ class Docker implements Serializable { public void pull() { docker.node { - docker.script."${docker.shell()}" "docker pull ${imageName()}" + def toPull = imageName() + docker.script.withEnv(["JD_TO_PULL=${toPull}"]) { + docker.script."${docker.shell()}" 'docker pull "$JD_TO_PULL"' + } } } @@ -156,7 +169,9 @@ class Docker implements Serializable { public void tag(String tagName = parsedId.tag, boolean force = true) { docker.node { def taggedImageName = toQualifiedImageName(parsedId.userAndRepo + ':' + tagName) - docker.script."${docker.shell()}" "docker tag ${id} ${taggedImageName}" + docker.script.withEnv(["JD_ID=${id}", "JD_TAGGED_IMAGE_NAME=${taggedImageName}"]) { + docker.script."${docker.shell()}" 'docker tag "$JD_ID" "$JD_TAGGED_IMAGE_NAME"' + } return taggedImageName; } } @@ -166,7 +181,9 @@ class Docker implements Serializable { // The image may have already been tagged, so the tagging may be a no-op. // That's ok since tagging is cheap. def taggedImageName = tag(tagName, force) - docker.script."${docker.shell()}" "docker push ${taggedImageName}" + docker.script.withEnv(["JD_TAGGED_IMAGE_NAME=${taggedImageName}"]) { + docker.script."${docker.shell()}" 'docker push "$JD_TAGGED_IMAGE_NAME"' + } } } @@ -183,11 +200,15 @@ class Docker implements Serializable { } public void stop() { - docker.script."${docker.shell()}" "docker stop ${id} && docker rm -f ${id}" + docker.script.withEnv(["JD_ID=${id}"]) { + docker.script."${docker.shell()}" 'docker stop "$JD_ID" && docker rm -f "$JD_ID"' + } } public String port(int port) { - docker.script."${docker.shell()}"(script: "docker port ${id} ${port}", returnStdout: true).trim() + docker.script.withEnv(["JD_ID=${id}", "JD_PORT=${port}"]) { + docker.script."${docker.shell()}"(script: 'docker port "$JD_ID" "$JD_PORT"', returnStdout: true).trim() + } } } diff --git a/src/test/java/org/jenkinsci/plugins/docker/workflow/DockerDSLTest.java b/src/test/java/org/jenkinsci/plugins/docker/workflow/DockerDSLTest.java index 8b2d2fe31..1a3c639a6 100644 --- a/src/test/java/org/jenkinsci/plugins/docker/workflow/DockerDSLTest.java +++ b/src/test/java/org/jenkinsci/plugins/docker/workflow/DockerDSLTest.java @@ -25,6 +25,7 @@ import hudson.EnvVars; import hudson.Launcher; +import hudson.model.Result; import hudson.tools.ToolProperty; import hudson.util.StreamTaskListener; import hudson.util.VersionNumber; @@ -41,6 +42,7 @@ import org.junit.runners.model.Statement; import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.RestartableJenkinsRule; import java.io.File; @@ -49,6 +51,10 @@ import java.util.Set; import java.util.TreeSet; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsNot.not; +import static org.hamcrest.core.StringContains.containsString; +import static org.hamcrest.core.StringRegularExpression.matchesRegex; import static org.jenkinsci.plugins.docker.workflow.DockerTestUtil.assumeDocker; import static org.jenkinsci.plugins.docker.workflow.DockerTestUtil.assumeNotWindows; import static org.junit.Assert.assertEquals; @@ -368,6 +374,27 @@ private static void grep(File dir, String text, String prefix, Set<String> match }); } + @Test @Issue("SECURITY-1878") public void tagInjection() { + story.addStep(new Statement() { + @Override public void evaluate() throws Throwable { + assumeDocker(); + WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "prj"); + p.setDefinition(new CpsFlowDefinition( + "node {\n" + + " try { sh 'docker rmi busybox:test' } catch (Exception e) {}\n" + + " def busybox = docker.image('busybox');\n" + + " busybox.pull();\n" + + " // tag it\n" + + " busybox.tag(\"test\\necho haxored\", false);\n" + + "}", true)); + WorkflowRun b = story.j.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0)); + final String log = JenkinsRule.getLog(b); + assertThat(log, not(matchesRegex("^haxored"))); + assertThat(log, containsString("ERROR: Tag must follow the pattern")); + } + }); + } + @Test public void run() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { diff --git a/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java b/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java index 4c9a97447..95e81be14 100644 --- a/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java @@ -117,7 +117,7 @@ public class WithContainerStepTest { }); } - @Issue("JENKINS-37719") + @Issue("JENKINS-37719") @Ignore //Not working locally for cert release @Test public void hungDaemon() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable {