Fix commandline double quoting for job containers

internal/jobcontainers/jobcontainer.go

@@ -253,12 +253,12 @@ func (c *JobContainer) CreateProcess(ctx context.Context, config interface{}) (_
 		Env:  env,
 		Dir:  workDir,
 		Path: absPath,
-		Args: splitArgs(commandLine),
 		SysProcAttr: &syscall.SysProcAttr{
 			// CREATE_BREAKAWAY_FROM_JOB to make sure that we're not inheriting the job object (and by extension its limits)
 			// from whatever process is going to launch the container.
 			CreationFlags: windows.CREATE_NEW_PROCESS_GROUP | windows.CREATE_BREAKAWAY_FROM_JOB,
 			Token:         syscall.Token(token),
+			CmdLine:       commandLine,
 		},
 	}
 	process := newProcess(cmd)

test/cri-containerd/jobcontainer_test.go

@@ -1,3 +1,4 @@
+//go:build functional
 // +build functional
 
 package cri_containerd
@@ -524,3 +525,42 @@ func Test_RunContainer_WorkingDirectory_JobContainer_WCOW(t *testing.T) {
 		})
 	}
 }
+
+// Test of the fix for the behavior detailed here https://github.com/microsoft/hcsshim/issues/1199
+// The underlying issue was that we would escape the args passed to us to form a commandline, and then split the commandline back into args
+// to pass to exec.Cmd in the stdlib. exec.Cmd internally does escaping of its own and thus would result in double quoting for certain
+// commandlines.
+func Test_DoubleQuoting_JobContainer_WCOW(t *testing.T) {
+	requireFeatures(t, featureWCOWProcess, featureHostProcess)
+
+	pullRequiredImages(t, []string{imageJobContainerCmdline})
+	client := newTestRuntimeClient(t)
+
+	podctx := context.Background()
+	sandboxRequest := getJobContainerPodRequestWCOW(t)
+
+	podID := runPodSandbox(t, client, podctx, sandboxRequest)
+	defer removePodSandbox(t, client, podctx, podID)
+	defer stopPodSandbox(t, client, podctx, podID)
+
+	containerRequest := getJobContainerRequestWCOW(t, podID, sandboxRequest.Config, imageJobContainerCmdline, nil)
+	ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
+	defer cancel()
+
+	containerID := createContainer(t, client, ctx, containerRequest)
+	defer removeContainer(t, client, ctx, containerID)
+	startContainer(t, client, ctx, containerID)
+	defer stopContainer(t, client, ctx, containerID)
+
+	execResponse := execSync(t, client, ctx, &runtime.ExecSyncRequest{
+		ContainerId: containerID,
+		Cmd:         []string{"cmdline.exe", `"quote test"`},
+	})
+
+	expected := `cmdline.exe "quote test"`
+	// Check that there's no double quoting going on.
+	// e.g. `cmdline.exe ""quote test"" `
+	if string(execResponse.Stdout) != expected {
+		t.Fatalf("expected cmdline for exec to be %q but got %q", expected, string(execResponse.Stdout))
+	}
+}

test/cri-containerd/main_test.go

@@ -1,3 +1,4 @@
+//go:build functional
 // +build functional
 
 package cri_containerd
@@ -55,6 +56,7 @@ const (
 	imageJobContainerHNS     = "cplatpublic.azurecr.io/jobcontainer_hns:latest"
 	imageJobContainerETW     = "cplatpublic.azurecr.io/jobcontainer_etw:latest"
 	imageJobContainerVHD     = "cplatpublic.azurecr.io/jobcontainer_vhd:latest"
+	imageJobContainerCmdline = "cplatpublic.azurecr.io/jobcontainer_cmdline:latest"
 	imageJobContainerWorkDir = "cplatpublic.azurecr.io/jobcontainer_workdir:latest"
 	alpineAspNet             = "mcr.microsoft.com/dotnet/core/aspnet:3.1-alpine3.11"
 	alpineAspnetUpgrade      = "mcr.microsoft.com/dotnet/core/aspnet:3.1.2-alpine3.11"

test/cri-containerd/test-images/jobcontainer_cmdline/dockerfile

@@ -0,0 +1,11 @@
+# Irrelevant what image version we use for job containers as there's no container <-> host OS version restraint.
+FROM golang:1.15.10-nanoserver-1809
+
+# Get administrator privileges
+USER containeradministrator
+
+WORKDIR C:\\go\\src\\cmdline
+COPY main.go .
+
+RUN go get -d -v ./...
+RUN go build -mod=mod

test/cri-containerd/test-images/jobcontainer_cmdline/main.go

@@ -0,0 +1,11 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"strings"
+)
+
+func main() {
+	fmt.Print(strings.Join(os.Args, " "))
+}