jbonofre commented on code in PR #2839:
URL: https://github.com/apache/karaf/pull/2839#discussion_r3996919333
##########
tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/DockerfileMojo.java:
##########
@@ -31,29 +31,46 @@
@Mojo(name = "dockerfile", defaultPhase = LifecyclePhase.PACKAGE)
public class DockerfileMojo extends MojoSupport {
+ private static final String DEFAULT_IMAGE = "eclipse-temurin:11-jre";
+ private static final String DEFAULT_COMMAND = "[\"karaf\", \"run\"]";
+
@Parameter(defaultValue = "${project.build.directory}")
private File destDir;
@Parameter(defaultValue = "${project.build.directory}/assembly")
private File assembly;
- @Parameter(defaultValue = "[\"karaf\", \"run\"]")
+ @Parameter(defaultValue = DEFAULT_COMMAND, property = "command")
private String command;
+ @Parameter(defaultValue = DEFAULT_IMAGE, property = "image")
+ private String image;
+
@Override
public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("Creating Dockerfile");
+
+ String baseImage = (image == null || image.trim().isEmpty()) ?
DEFAULT_IMAGE : image.trim();
+ String cmd = (command == null || command.trim().isEmpty()) ?
DEFAULT_COMMAND : command.trim();
+
+ if (baseImage.contains("\n") || baseImage.contains("\r")) {
Review Comment:
This only blocks `\n`/`\r`. A same line value like `--platform=linux/386
alpine AS build` has no newline but still changes what `FROM` does (adds a
platform override + build stage) once written out.
The newline-only blacklist doesn't actually close the gap this PR claims to
close.
##########
tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/DockerfileMojo.java:
##########
@@ -31,29 +31,46 @@
@Mojo(name = "dockerfile", defaultPhase = LifecyclePhase.PACKAGE)
public class DockerfileMojo extends MojoSupport {
+ private static final String DEFAULT_IMAGE = "eclipse-temurin:11-jre";
+ private static final String DEFAULT_COMMAND = "[\"karaf\", \"run\"]";
+
@Parameter(defaultValue = "${project.build.directory}")
private File destDir;
@Parameter(defaultValue = "${project.build.directory}/assembly")
private File assembly;
- @Parameter(defaultValue = "[\"karaf\", \"run\"]")
+ @Parameter(defaultValue = DEFAULT_COMMAND, property = "command")
private String command;
+ @Parameter(defaultValue = DEFAULT_IMAGE, property = "image")
+ private String image;
+
@Override
public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("Creating Dockerfile");
+
+ String baseImage = (image == null || image.trim().isEmpty()) ?
DEFAULT_IMAGE : image.trim();
+ String cmd = (command == null || command.trim().isEmpty()) ?
DEFAULT_COMMAND : command.trim();
+
+ if (baseImage.contains("\n") || baseImage.contains("\r")) {
+ throw new MojoExecutionException("Invalid image: base image cannot
contain newline characters");
+ }
+ if (cmd.contains("\n") || cmd.contains("\r")) {
+ throw new MojoExecutionException("Invalid command: command cannot
contain newline characters");
+ }
+
File dockerFile = new File(destDir, "Dockerfile");
try {
StringBuilder buffer = new StringBuilder();
- buffer.append("FROM eclipse-temurin:11-jre").append("\n");
+ buffer.append("FROM ").append(baseImage).append("\n");
buffer.append("ENV KARAF_INSTALL_PATH /opt").append("\n");
buffer.append("ENV KARAF_HOME
$KARAF_INSTALL_PATH/apache-karaf").append("\n");
buffer.append("ENV KARAF_EXEC exec").append("\n");
buffer.append("ENV PATH $PATH:$KARAF_HOME/bin").append("\n");
buffer.append("COPY ").append(assembly.getName()).append("
$KARAF_HOME").append("\n");
Review Comment:
`assembly.getName()` is spliced into this `COPY` line with no validation,
unlike `image`/`command` below.
Since this PR's whole point is to stop unvalidated strings reaching raw
Dockerfile lines, a filename containing a literal newline (legal on Unix) would
let this inject an arbitrary extra Dockerfile instruction.
I propose the same guard applied here. It's your call if you are happy to do
it, else I will merge as is.
##########
tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/DockerfileMojo.java:
##########
@@ -31,29 +31,46 @@
@Mojo(name = "dockerfile", defaultPhase = LifecyclePhase.PACKAGE)
public class DockerfileMojo extends MojoSupport {
+ private static final String DEFAULT_IMAGE = "eclipse-temurin:11-jre";
+ private static final String DEFAULT_COMMAND = "[\"karaf\", \"run\"]";
+
@Parameter(defaultValue = "${project.build.directory}")
private File destDir;
@Parameter(defaultValue = "${project.build.directory}/assembly")
private File assembly;
- @Parameter(defaultValue = "[\"karaf\", \"run\"]")
+ @Parameter(defaultValue = DEFAULT_COMMAND, property = "command")
private String command;
+ @Parameter(defaultValue = DEFAULT_IMAGE, property = "image")
+ private String image;
+
@Override
public void execute() throws MojoExecutionException, MojoFailureException {
getLog().info("Creating Dockerfile");
+
+ String baseImage = (image == null || image.trim().isEmpty()) ?
DEFAULT_IMAGE : image.trim();
+ String cmd = (command == null || command.trim().isEmpty()) ?
DEFAULT_COMMAND : command.trim();
+
+ if (baseImage.contains("\n") || baseImage.contains("\r")) {
+ throw new MojoExecutionException("Invalid image: base image cannot
contain newline characters");
+ }
+ if (cmd.contains("\n") || cmd.contains("\r")) {
Review Comment:
Same gap as `image` above: `cmd` is only checked for `\n`/`\r`, not
validated as well-formed JSON exec-form. A typo like `karaf run` (missing
brackets/quotes) passes this check and silently becomes shell-form `CMD karaf
run`, changing PID 1/signal-forwarding behavior with no warning.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
