Matche Coder CLI behavior for escaping to SSH config

code-asher · code-asher · commit a090a3c2d57c · 2023-09-22T03:56:08.000-08:00
diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt
@@ -507,12 +507,10 @@ class CoderCLIManager @JvmOverloads constructor(
             return if (cliMatches == null && dataCLIMatches != null) dataCLI else cli
         }
 
-        var escapeRegex = """(["\\])""".toRegex()
-
         /**
-         * Escape a command argument by wrapping it in double quotes and escaping
-         * any slashes and double quotes in the argument.  For example, echo "te\st"
-         *  becomes "echo \"te\\st\"".
+         * Escape a command argument to be used in the ProxyCommand of an SSH
+         * config.  Surround with double quotes if the argument contains
+         * whitespace and escape any existing double quotes.
          *
          * Throws if the argument is invalid.
          */
@@ -521,7 +519,10 @@ class CoderCLIManager @JvmOverloads constructor(
             if (s.contains("\n")) {
                 throw Exception("argument cannot contain newlines")
             }
-            return "\"" + s.replace(escapeRegex, """\\$1""") + "\""
+            if (s.contains(" ") || s.contains("\t")) {
+                return "\"" + s.replace("\"", "\\\"") + "\""
+            }
+            return s.replace("\"", "\\\"")
         }
     }
 }
diff --git a/src/test/fixtures/outputs/append-blank-newlines.conf b/src/test/fixtures/outputs/append-blank-newlines.conf
@@ -5,7 +5,7 @@
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/append-blank.conf b/src/test/fixtures/outputs/append-blank.conf
@@ -1,7 +1,7 @@
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/append-no-blocks.conf b/src/test/fixtures/outputs/append-no-blocks.conf
@@ -6,7 +6,7 @@ Host test2
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/append-no-newline.conf b/src/test/fixtures/outputs/append-no-newline.conf
@@ -5,7 +5,7 @@ Host test2
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/append-no-related-blocks.conf b/src/test/fixtures/outputs/append-no-related-blocks.conf
@@ -12,7 +12,7 @@ some jetbrains config
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/header-command-windows.conf b/src/test/fixtures/outputs/header-command-windows.conf
@@ -1,7 +1,7 @@
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--header--test.coder.invalid
   HostName coder.header
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" --header-command "C:\\Program Files\\My Header Command\\\"also has quotes\"\\HeaderCommand.exe" ssh --stdio header
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config --header-command "C:\Program Files\My Header Command\\"also has quotes\"\HeaderCommand.exe" ssh --stdio header
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/header-command.conf b/src/test/fixtures/outputs/header-command.conf
@@ -1,7 +1,7 @@
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--header--test.coder.invalid
   HostName coder.header
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" --header-command "my-header-command \"test\"" ssh --stdio header
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config --header-command "my-header-command \"test\"" ssh --stdio header
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/multiple-workspaces.conf b/src/test/fixtures/outputs/multiple-workspaces.conf
@@ -1,15 +1,15 @@
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo--test.coder.invalid
   HostName coder.foo
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
   LogLevel ERROR
   SetEnv CODER_SSH_SESSION_TYPE=JetBrains
 Host coder-jetbrains--bar--test.coder.invalid
   HostName coder.bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/replace-end-no-newline.conf b/src/test/fixtures/outputs/replace-end-no-newline.conf
@@ -4,7 +4,7 @@ Host test2
   Port 443 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/replace-end.conf b/src/test/fixtures/outputs/replace-end.conf
@@ -5,7 +5,7 @@ Host test2
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/replace-middle-ignore-unrelated.conf b/src/test/fixtures/outputs/replace-middle-ignore-unrelated.conf
@@ -6,7 +6,7 @@ some coder config
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/replace-middle.conf b/src/test/fixtures/outputs/replace-middle.conf
@@ -3,7 +3,7 @@ Host test
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/replace-only.conf b/src/test/fixtures/outputs/replace-only.conf
@@ -1,7 +1,7 @@
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/fixtures/outputs/replace-start.conf b/src/test/fixtures/outputs/replace-start.conf
@@ -1,7 +1,7 @@
 # --- START CODER JETBRAINS test.coder.invalid
 Host coder-jetbrains--foo-bar--test.coder.invalid
   HostName coder.foo-bar
-  ProxyCommand "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64" --global-config "/tmp/coder-gateway/test.coder.invalid/config" ssh --stdio foo-bar
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo-bar
   ConnectTimeout 0
   StrictHostKeyChecking no
   UserKnownHostsFile /dev/null
diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy
@@ -386,9 +386,13 @@ class CoderCLIManagerTest extends Specification {
 
         where:
         str                                             | expected
-        $/C:\"quote after slash"/$                      | $/"C:\\\"quote after slash\""/$
-        $/C:\echo "hello world"/$                       | $/"C:\\echo \"hello world\""/$
-        $/"C:\Program Files\HeaderCommand.exe" --flag/$ | $/"\"C:\\Program Files\\HeaderCommand.exe\" --flag"/$
+        $//tmp/coder/$                                  | $//tmp/coder/$
+        $//tmp/c o d e r/$                              | $/"/tmp/c o d e r"/$
+        $/C:\no\spaces.exe/$                            | $/C:\no\spaces.exe/$
+        $/C:\"quote after slash"/$                      | $/"C:\\"quote after slash\""/$
+        $/C:\echo "hello world"/$                       | $/"C:\echo \"hello world\""/$
+        $/C:\"no"\"spaces"/$                            | $/C:\\"no\"\\"spaces\"/$
+        $/"C:\Program Files\HeaderCommand.exe" --flag/$ | $/"\"C:\Program Files\HeaderCommand.exe\" --flag"/$
     }
 
     def "configures an SSH file"() {
@@ -405,8 +409,8 @@ class CoderCLIManagerTest extends Specification {
 
         def expectedConf = Path.of("src/test/fixtures/outputs/").resolve(output + ".conf").toFile().text
                 .replaceAll("\\r?\\n", System.lineSeparator())
-                .replace("\"/tmp/coder-gateway/test.coder.invalid/config\"", CoderCLIManager.escape(coderConfigPath.toString()))
-                .replace("\"/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64\"", CoderCLIManager.escape(ccm.localBinaryPath.toString()))
+                .replace("/tmp/coder-gateway/test.coder.invalid/config", CoderCLIManager.escape(coderConfigPath.toString()))
+                .replace("/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", CoderCLIManager.escape(ccm.localBinaryPath.toString()))
 
         when:
         ccm.configSsh(workspaces.collect { DataGen.workspace(it) }, headerCommand)

Original file line number	Diff line number	Diff line change
`@@ -507,12 +507,10 @@ class CoderCLIManager @JvmOverloads constructor(`
`507`	`507`	`return if (cliMatches == null && dataCLIMatches != null) dataCLI else cli`
`508`	`508`	`}`
`509`	`509`
`510`		`- var escapeRegex = """(["\\])""".toRegex()`
`511`		`-`
`512`	`510`	`/**`
`513`		`- * Escape a command argument by wrapping it in double quotes and escaping`
`514`		`- * any slashes and double quotes in the argument. For example, echo "te\st"`
`515`		`- * becomes "echo \"te\\st\"".`
	`511`	`+ * Escape a command argument to be used in the ProxyCommand of an SSH`
	`512`	`+ * config. Surround with double quotes if the argument contains`
	`513`	`+ * whitespace and escape any existing double quotes.`
`516`	`514`	`*`
`517`	`515`	`* Throws if the argument is invalid.`
`518`	`516`	`*/`
`@@ -521,7 +519,10 @@ class CoderCLIManager @JvmOverloads constructor(`
`521`	`519`	`if (s.contains("\n")) {`
`522`	`520`	`throw Exception("argument cannot contain newlines")`
`523`	`521`	`}`
`524`		`- return "\"" + s.replace(escapeRegex, """\\$1""") + "\""`
	`522`	`+ if (s.contains(" ") \|\| s.contains("\t")) {`
	`523`	`+ return "\"" + s.replace("\"", "\\\"") + "\""`
	`524`	`+ }`
	`525`	`+ return s.replace("\"", "\\\"")`
`525`	`526`	`}`
`526`	`527`	`}`
`527`	`528`	`}`