Don't escape customOptions or single-string command options (#3729)

src/debugging/netcore/NetCoreDebugHelper.ts

@@ -5,9 +5,9 @@
 
 import * as fse from 'fs-extra';
 import * as path from 'path';
-import { composeArgs, ContainerOS, VoidCommandResponse, withArg, withQuotedArg } from '../../runtimes/docker';
+import { CommandLineArgs, composeArgs, ContainerOS, VoidCommandResponse, withArg, withQuotedArg } from '../../runtimes/docker';
 import { DialogResponses, IActionContext, UserCancelledError } from '@microsoft/vscode-azext-utils';
-import { DebugConfiguration, MessageItem, ProgressLocation, ShellQuotedString, window } from 'vscode';
+import { DebugConfiguration, MessageItem, ProgressLocation, window } from 'vscode';
 import { ext } from '../../extensionVariables';
 import { localize } from '../../localize';
 import { NetCoreTaskHelper, NetCoreTaskOptions } from '../../tasks/netcore/NetCoreTaskHelper';
@@ -307,7 +307,7 @@ export class NetCoreDebugHelper implements DebugHelper {
 
     private async isDebuggerInstalled(containerName: string, debuggerPath: string, containerOS: ContainerOS): Promise<boolean> {
         let containerCommand: string;
-        let containerCommandArgs: ShellQuotedString[];
+        let containerCommandArgs: CommandLineArgs;
         if (containerOS === 'windows') {
             containerCommand = 'cmd';
             containerCommandArgs = composeArgs(

src/runtimes/docker/clients/DockerClientBase/DockerClientBase.ts

@@ -88,6 +88,7 @@ import {
     withFlagArg,
     withNamedArg,
     withQuotedArg,
+    withVerbatimArg,
 } from "../../utils/commandLineBuilder";
 import { CommandNotSupportedError } from '../../utils/CommandNotSupportedError';
 import { byteStreamToGenerator, stringStreamToGenerator } from '../../utils/streamToGenerator';
@@ -390,7 +391,7 @@ export abstract class DockerClientBase extends ConfigurableClient implements ICo
             withDockerLabelsArg(options.labels),
             withNamedArg('--iidfile', options.imageIdFile),
             withDockerBuildArg(options.args),
-            withArg(options.customOptions),
+            withVerbatimArg(options.customOptions),
             withQuotedArg(options.path),
         )();
     }
@@ -829,9 +830,9 @@ export abstract class DockerClientBase extends ConfigurableClient implements ICo
             withDockerEnvArg(options.environmentVariables),
             withNamedArg('--env-file', options.environmentFiles),
             withNamedArg('--entrypoint', options.entrypoint),
-            withArg(options.customOptions),
+            withVerbatimArg(options.customOptions),
             withArg(options.imageRef),
-            withArg(...(toArray(options.command || []))),
+            typeof options.command === 'string' ? withVerbatimArg(options.command) : withArg(...(toArray(options.command || []))),
         )();
     }
 
@@ -875,7 +876,7 @@ export abstract class DockerClientBase extends ConfigurableClient implements ICo
             withFlagArg('--tty', options.tty),
             withDockerEnvArg(options.environmentVariables),
             withArg(options.container),
-            withArg(...toArray(options.command)),
+            typeof options.command === 'string' ? withVerbatimArg(options.command) : withArg(...toArray(options.command)),
         )();
     }
 

src/runtimes/docker/clients/DockerComposeClient/DockerComposeClient.ts

@@ -14,15 +14,16 @@ import {
     RestartCommandOptions,
     StartCommandOptions,
     StopCommandOptions,
-    UpCommandOptions
+    UpCommandOptions,
 } from '../../contracts/ContainerOrchestratorClient';
 import {
     CommandLineArgs,
     CommandLineCurryFn,
     composeArgs,
     withArg,
     withFlagArg,
-    withNamedArg
+    withNamedArg,
+    withVerbatimArg,
 } from '../../utils/commandLineBuilder';
 import { stringStreamToGenerator } from '../../utils/streamToGenerator';
 import { ConfigurableClient } from '../ConfigurableClient';
@@ -96,7 +97,7 @@ export class DockerComposeClient extends ConfigurableClient implements IContaine
             withNamedArg('--scale', Object.entries(options.scale || {}).map(([service, scale]) => `${service}=${scale}`)),
             withNamedArg('--timeout', options.timeoutSeconds?.toString(10)),
             withFlagArg('--wait', options.wait),
-            withArg(options.customOptions),
+            withVerbatimArg(options.customOptions),
             withArg(...(options.services || [])),
         )();
     }
@@ -125,7 +126,7 @@ export class DockerComposeClient extends ConfigurableClient implements IContaine
             withNamedArg('--rmi', options.removeImages),
             withFlagArg('--volumes', options.removeVolumes),
             withNamedArg('--timeout', options.timeoutSeconds?.toString(10)),
-            withArg(options.customOptions),
+            withVerbatimArg(options.customOptions),
         )();
     }
 

src/runtimes/docker/test/DockerClient.test.ts

@@ -14,7 +14,9 @@ import { ShellQuoting } from 'vscode';
 import {
     DockerClient,
 } from '../clients/DockerClient/DockerClient';
+import { BuildImageCommandOptions, RunContainerCommandOptions } from '../contracts/ContainerClient';
 import { escaped } from '../utils/commandLineBuilder';
+import { Bash, Powershell } from '../utils/spawnStreamAsync';
 
 dayjs.extend(customParseFormat);
 dayjs.extend(utc);
@@ -91,3 +93,49 @@ describe('DockerClient', () => {
         });
     });
 });
+
+describe('DockerClient (unit)', () => {
+    const client = new DockerClient();
+
+    it('Should produce the expected lack of quoting/escaping customOptions', async () => {
+        const options: BuildImageCommandOptions = {
+            path: '.',
+            customOptions: '--no-cache --progress plain'
+        };
+
+        const commandResponse = await client.buildImage(options);
+        const pwshQuoted = new Powershell().quote(commandResponse.args);
+        const bashQuoted = new Bash().quote(commandResponse.args);
+
+        expect(pwshQuoted).to.deep.equal(['image', 'build', '--no-cache --progress plain', '\'.\'']);
+        expect(bashQuoted).to.deep.equal(['image', 'build', '--no-cache --progress plain', '\'.\'']);
+    });
+
+    it('Should produce the expected lack of quoting/escaping a single string command', async () => {
+        const options: RunContainerCommandOptions = {
+            imageRef: 'someimage',
+            command: 'sh -c "echo hello world"',
+        };
+
+        const commandResponse = await client.runContainer(options);
+        const pwshQuoted = new Powershell().quote(commandResponse.args);
+        const bashQuoted = new Bash().quote(commandResponse.args);
+
+        expect(pwshQuoted).to.deep.equal(['container', 'run', 'someimage', 'sh -c "echo hello world"']);
+        expect(bashQuoted).to.deep.equal(['container', 'run', 'someimage', 'sh -c "echo hello world"']);
+    });
+
+    it('Should produce the expected quoting/escaping of an array command', async () => {
+        const options: RunContainerCommandOptions = {
+            imageRef: 'someimage',
+            command: ['sh', '-c', 'echo hello world'],
+        };
+
+        const commandResponse = await client.runContainer(options);
+        const pwshQuoted = new Powershell().quote(commandResponse.args);
+        const bashQuoted = new Bash().quote(commandResponse.args);
+
+        expect(pwshQuoted).to.deep.equal(['container', 'run', 'someimage', 'sh', '-c', 'echo` hello` world']);
+        expect(bashQuoted).to.deep.equal(['container', 'run', 'someimage', 'sh', '-c', 'echo\\ hello\\ world']);
+    });
+});

src/runtimes/docker/test/DockerComposeClient.test.ts

@@ -18,6 +18,7 @@ import {
     UpCommandOptions
 } from '../contracts/ContainerOrchestratorClient';
 import { AccumulatorStream } from '../utils/AccumulatorStream';
+import { Bash, Powershell } from '../utils/spawnStreamAsync';
 
 const commonOptions: CommonOrchestratorCommandOptions = {
     files: ['docker-compose.yml'],
@@ -102,3 +103,24 @@ xdescribe('DockerComposeClient', () => {
         expect(result).to.contain('registry');
     });
 });
+
+describe('DockerComposeClient (unit)', () => {
+    const client = new DockerComposeClient();
+    client.composeV2 = false;
+
+    it('Should produce the expected lack of quoting/escaping customOptions', async () => {
+        const options: UpCommandOptions = {
+            ...commonOptions,
+            detached: true,
+            build: true,
+            customOptions: '--timeout 10 --wait'
+        };
+
+        const commandResponse = await client.up(options);
+        const pwshQuoted = new Powershell().quote(commandResponse.args);
+        const bashQuoted = new Bash().quote(commandResponse.args);
+
+        expect(pwshQuoted).to.deep.equal(['--file', '\'docker-compose.yml\'', 'up', '--detach', '--build', '--timeout 10 --wait']);
+        expect(bashQuoted).to.deep.equal(['--file', '\'docker-compose.yml\'', 'up', '--detach', '--build', '--timeout 10 --wait']);
+    });
+});

src/runtimes/docker/utils/commandLineBuilder.ts

@@ -6,7 +6,7 @@
 import { ShellQuotedString, ShellQuoting } from 'vscode';
 import { toArray } from './toArray';
 
-export type CommandLineArgs = Array<ShellQuotedString>;
+export type CommandLineArgs = Array<ShellQuotedString | string>;
 
 export type CommandLineCurryFn = (cmdLineArgs?: CommandLineArgs) => CommandLineArgs;
 
@@ -95,14 +95,33 @@ export function withNamedArg(
 ): CommandLineCurryFn {
     return (cmdLineArgs: CommandLineArgs = []) => {
         return toArray(args)
-        .reduce((allArgs, arg) => {
-            if (arg) {
-                const normalizedArg = shouldQuote ? quoted(arg) : escaped(arg);
-                if (assignValue) {
-                    return withArg(`${name}=${normalizedArg}`)(allArgs);
+            .reduce((allArgs, arg) => {
+                if (arg) {
+                    const normalizedArg = shouldQuote ? quoted(arg) : escaped(arg);
+                    if (assignValue) {
+                        return withArg(`${name}=${normalizedArg}`)(allArgs);
+                    }
+
+                    return withArg(name, normalizedArg)(allArgs);
                 }
 
-                return withArg(name, normalizedArg)(allArgs);
+                return allArgs;
+            }, cmdLineArgs);
+    };
+}
+
+/**
+ * Functional method for adding additional verbatim arguments to an existing list of
+ * arguments. They will not be quoted nor escaped.
+ * @param args Raw arguments to add to the CommandLineArguments records
+ * @returns A function that takes an optional array of CommandLineArguments and appends the provided arguments
+ * @deprecated {@link withArg} should be used instead in almost all cases
+ */
+export function withVerbatimArg(...args: Array<string | null | undefined>): CommandLineCurryFn {
+    return (cmdLineArgs: CommandLineArgs = []) => {
+        return args.reduce<CommandLineArgs>((allArgs, arg) => {
+            if (arg) {
+                return [...allArgs, arg];
             }
 
             return allArgs;

src/runtimes/docker/utils/spawnStreamAsync.ts

@@ -62,6 +62,13 @@ export class Powershell extends Shell {
         const escape = (value: string) => `\`${value}`;
 
         return args.map((quotedArg) => {
+            // If it's a verbatim argument, return it as-is.
+            // The overwhelming majority of arguments are `ShellQuotedString`, so
+            // verbatim arguments will only show up if `withVerbatimArg` is used.
+            if (typeof quotedArg === 'string') {
+                return quotedArg;
+            }
+
             switch (quotedArg.quoting) {
                 case ShellQuoting.Escape:
                     return quotedArg.value.replace(/[ "'()]/g, escape);
@@ -103,6 +110,13 @@ export class Bash extends Shell {
         const escape = (value: string) => `\\${value}`;
 
         return args.map((quotedArg) => {
+            // If it's a verbatim argument, return it as-is.
+            // The overwhelming majority of arguments are `ShellQuotedString`, so
+            // verbatim arguments will only show up if `withVerbatimArg` is used.
+            if (typeof quotedArg === 'string') {
+                return quotedArg;
+            }
+
             switch (quotedArg.quoting) {
                 case ShellQuoting.Escape:
                     return quotedArg.value.replace(/[ "']/g, escape);

src/runtimes/runners/TaskCommandRunnerFactory.ts

@@ -5,7 +5,7 @@
 
 import * as os from 'os';
 import * as vscode from 'vscode';
-import { CommandNotSupportedError, CommandRunner, ICommandRunnerFactory, Like, normalizeCommandResponseLike, PromiseCommandResponse, StreamingCommandRunner, VoidCommandResponse } from '../docker';
+import { CommandLineArgs, CommandNotSupportedError, CommandRunner, ICommandRunnerFactory, Like, normalizeCommandResponseLike, PromiseCommandResponse, StreamingCommandRunner, VoidCommandResponse } from '../docker';
 
 interface TaskCommandRunnerOptions {
     taskName: string;
@@ -35,7 +35,7 @@ export class TaskCommandRunnerFactory implements ICommandRunnerFactory {
     }
 }
 
-async function executeAsTask(options: TaskCommandRunnerOptions, command: string, args?: vscode.ShellQuotedString[]): Promise<void> {
+async function executeAsTask(options: TaskCommandRunnerOptions, command: string, args?: CommandLineArgs): Promise<void> {
     const shellExecutionOptions = { cwd: options.cwd || options.workspaceFolder?.uri?.fsPath || os.homedir() };
 
     const shellExecution = args ?