Merge pull request #17851 from Microsoft/10662-ArgumentEscaping

Prevent double escaping of Dockerfile commands on Windows

api/client/run.go

@@ -104,6 +104,8 @@ func (cli *DockerCli) CmdRun(args ...string) error {
 		return nil
 	}
 
+	config.ArgsEscaped = false
+
 	if !*flDetach {
 		if err := cli.CheckTtyInput(config.AttachStdin, config.Tty); err != nil {
 			return err

builder/dockerfile/dispatchers.go

@@ -389,6 +389,8 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 	b.runConfig.Cmd = config.Cmd
 	// set build-time environment for 'run'.
 	b.runConfig.Env = append(b.runConfig.Env, cmdBuildEnv...)
+	// set config as already being escaped, this prevents double escaping on windows
+	b.runConfig.ArgsEscaped = true
 
 	logrus.Debugf("[BUILDER] Command to be executed: %v", b.runConfig.Cmd)
 
@@ -479,7 +481,7 @@ func entrypoint(b *Builder, args []string, attributes map[string]bool, original
 		if runtime.GOOS != "windows" {
 			b.runConfig.Entrypoint = stringutils.NewStrSlice("/bin/sh", "-c", parsed[0])
 		} else {
-			b.runConfig.Entrypoint = stringutils.NewStrSlice("cmd", "/S /C", parsed[0])
+			b.runConfig.Entrypoint = stringutils.NewStrSlice("cmd", "/S", "/C", parsed[0])
 		}
 	}
 

builder/dockerfile/internals.go

@@ -181,7 +181,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD
 	if runtime.GOOS != "windows" {
 		b.runConfig.Cmd = stringutils.NewStrSlice("/bin/sh", "-c", fmt.Sprintf("#(nop) %s %s in %s", cmdName, srcHash, dest))
 	} else {
-		b.runConfig.Cmd = stringutils.NewStrSlice("cmd", "/S /C", fmt.Sprintf("REM (nop) %s %s in %s", cmdName, srcHash, dest))
+		b.runConfig.Cmd = stringutils.NewStrSlice("cmd", "/S", "/C", fmt.Sprintf("REM (nop) %s %s in %s", cmdName, srcHash, dest))
 	}
 	defer func(cmd *stringutils.StrSlice) { b.runConfig.Cmd = cmd }(cmd)
 

daemon/container_windows.go

@@ -136,6 +136,7 @@ func (daemon *Daemon) populateCommand(c *Container, env []string) error {
 		LayerPaths:  layerPaths,
 		Hostname:    c.Config.Hostname,
 		Isolation:   c.hostConfig.Isolation,
+		ArgsEscaped: c.Config.ArgsEscaped,
 	}
 
 	return nil

daemon/execdriver/driver_windows.go

@@ -57,6 +57,7 @@ type Command struct {
 	LayerFolder string                   `json:"layer_folder"` // Layer folder for a command
 	LayerPaths  []string                 `json:"layer_paths"`  // Layer paths for a command
 	Isolation   runconfig.IsolationLevel `json:"isolation"`    // Isolation level for the container
+	ArgsEscaped bool                     `json:"args_escaped"` // True if args are already escaped
 }
 
 // ExitStatus provides exit reasons for a container.

daemon/execdriver/windows/commandlinebuilder.go

@@ -0,0 +1,36 @@
+//+build windows
+
+package windows
+
+import (
+	"errors"
+	"syscall"
+
+	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker/daemon/execdriver"
+)
+
+// createCommandLine creates a command line from the Entrypoint and args
+// of the ProcessConfig. It escapes the arguments if they are not already
+// escaped
+func createCommandLine(processConfig *execdriver.ProcessConfig, alreadyEscaped bool) (commandLine string, err error) {
+	// While this should get caught earlier, just in case, validate that we
+	// have something to run.
+	if processConfig.Entrypoint == "" {
+		return "", errors.New("No entrypoint specified")
+	}
+
+	// Build the command line of the process
+	commandLine = processConfig.Entrypoint
+	logrus.Debugf("Entrypoint: %s", processConfig.Entrypoint)
+	for _, arg := range processConfig.Arguments {
+		logrus.Debugf("appending %s", arg)
+		if !alreadyEscaped {
+			arg = syscall.EscapeArg(arg)
+		}
+		commandLine += " " + arg
+	}
+
+	logrus.Debugf("commandLine: %s", commandLine)
+	return commandLine, nil
+}

daemon/execdriver/windows/exec.go

@@ -3,7 +3,6 @@
 package windows
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/Sirupsen/logrus"
@@ -34,21 +33,11 @@ func (d *Driver) Exec(c *execdriver.Command, processConfig *execdriver.ProcessCo
 	// Configure the environment for the process // Note NOT c.ProcessConfig.Tty
 	createProcessParms.Environment = setupEnvironmentVariables(processConfig.Env)
 
-	// While this should get caught earlier, just in case, validate that we
-	// have something to run.
-	if processConfig.Entrypoint == "" {
-		err = errors.New("No entrypoint specified")
-		logrus.Error(err)
-		return -1, err
-	}
+	createProcessParms.CommandLine, err = createCommandLine(&c.ProcessConfig, c.ArgsEscaped)
 
-	// Build the command line of the process
-	createProcessParms.CommandLine = processConfig.Entrypoint
-	for _, arg := range processConfig.Arguments {
-		logrus.Debugln("appending ", arg)
-		createProcessParms.CommandLine += " " + arg
+	if err != nil {
+		return -1, err
 	}
-	logrus.Debugln("commandLine: ", createProcessParms.CommandLine)
 
 	// Start the command running in the container.
 	pid, stdin, stdout, stderr, rc, err := hcsshim.CreateProcessInComputeSystem(c.ID, pipes.Stdin != nil, true, !processConfig.Tty, createProcessParms)

daemon/execdriver/windows/run.go

@@ -4,13 +4,11 @@ package windows
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
-	"syscall"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/daemon/execdriver"
@@ -273,21 +271,11 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd
 	// Configure the environment for the process
 	createProcessParms.Environment = setupEnvironmentVariables(c.ProcessConfig.Env)
 
-	// This should get caught earlier, but just in case - validate that we
-	// have something to run
-	if c.ProcessConfig.Entrypoint == "" {
-		err = errors.New("No entrypoint specified")
-		logrus.Error(err)
-		return execdriver.ExitStatus{ExitCode: -1}, err
-	}
+	createProcessParms.CommandLine, err = createCommandLine(&c.ProcessConfig, c.ArgsEscaped)
 
-	// Build the command line of the process
-	createProcessParms.CommandLine = c.ProcessConfig.Entrypoint
-	for _, arg := range c.ProcessConfig.Arguments {
-		logrus.Debugln("appending ", arg)
-		createProcessParms.CommandLine += " " + syscall.EscapeArg(arg)
+	if err != nil {
+		return execdriver.ExitStatus{ExitCode: -1}, err
 	}
-	logrus.Debugf("CommandLine: %s", createProcessParms.CommandLine)
 
 	// Start the command running in the container.
 	pid, stdin, stdout, stderr, _, err := hcsshim.CreateProcessInComputeSystem(c.ID, pipes.Stdin != nil, true, !c.ProcessConfig.Tty, createProcessParms)

runconfig/config.go

@@ -30,6 +30,7 @@ type Config struct {
 	StdinOnce       bool                  // If true, close stdin after the 1 attached client disconnects.
 	Env             []string              // List of environment variable to set in the container
 	Cmd             *stringutils.StrSlice // Command to run when starting the container
+	ArgsEscaped     bool                  `json:",omitempty"` // True if command is already escaped (Windows specific)
 	Image           string                // Name of the image as it was passed by the operator (eg. could be symbolic)
 	Volumes         map[string]struct{}   // List of volumes (mounts) used for the container
 	WorkingDir      string                // Current directory (PWD) in the command will be launched