diff --git a/CHANGELOG.md b/CHANGELOG.md index 929c259ef..ed3de9b02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ ## Unreleased +### Fixed + +- When a proxy command argument (such as the URL) contains `?` and `&`, escape + it in the SSH config by using double quotes, as these characters have special + meanings in shells. + ## 2.14.0 - 2024-08-30 ### Fixed diff --git a/gradle.properties b/gradle.properties index a4325041c..c6ab9e544 100644 --- a/gradle.properties +++ b/gradle.properties @@ -26,7 +26,7 @@ pluginUntilBuild=242.* # that exists, ideally the most recent one, for example # 233.15325-EAP-CANDIDATE-SNAPSHOT). platformType=GW -platformVersion=233.15325-EAP-CANDIDATE-SNAPSHOT +platformVersion=233.15619-EAP-CANDIDATE-SNAPSHOT instrumentationCompiler=242.19533-EAP-CANDIDATE-SNAPSHOT # Gateway does not have open sources. platformDownloadSources=true diff --git a/src/main/kotlin/com/coder/gateway/util/Escape.kt b/src/main/kotlin/com/coder/gateway/util/Escape.kt index 8cb71a28f..af22bfe50 100644 --- a/src/main/kotlin/com/coder/gateway/util/Escape.kt +++ b/src/main/kotlin/com/coder/gateway/util/Escape.kt @@ -3,8 +3,14 @@ package com.coder.gateway.util /** * Escape an argument to be used in the ProxyCommand of an SSH config. * - * Escaping happens by surrounding with double quotes if the argument contains - * whitespace and escaping any existing double quotes regardless of whitespace. + * Escaping happens by: + * 1. Surrounding with double quotes if the argument contains whitespace, ?, or + * & (to handle query parameters in URLs) as these characters have special + * meaning in shells. + * 2. Always escaping existing double quotes. + * + * Double quotes does not preserve the literal values of $, `, \, *, @, and ! + * (when history expansion is enabled); these are not currently handled. * * Throws if the argument is invalid. */ @@ -12,7 +18,7 @@ fun escape(s: String): String { if (s.contains("\n")) { throw Exception("argument cannot contain newlines") } - if (s.contains(" ") || s.contains("\t")) { + if (s.contains(" ") || s.contains("\t") || s.contains("&") || s.contains("?")) { return "\"" + s.replace("\"", "\\\"") + "\"" } return s.replace("\"", "\\\"") diff --git a/src/test/fixtures/outputs/url.conf b/src/test/fixtures/outputs/url.conf new file mode 100644 index 000000000..8854325c7 --- /dev/null +++ b/src/test/fixtures/outputs/url.conf @@ -0,0 +1,16 @@ +# --- START CODER JETBRAINS test.coder.invalid +Host coder-jetbrains--url--test.coder.invalid + ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config --url "https://test.coder.invalid?foo=bar&baz=qux" ssh --stdio --usage-app=jetbrains url + ConnectTimeout 0 + StrictHostKeyChecking no + UserKnownHostsFile /dev/null + LogLevel ERROR + SetEnv CODER_SSH_SESSION_TYPE=JetBrains +Host coder-jetbrains--url--test.coder.invalid--bg + ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config --url "https://test.coder.invalid?foo=bar&baz=qux" ssh --stdio --usage-app=disable url + ConnectTimeout 0 + StrictHostKeyChecking no + UserKnownHostsFile /dev/null + LogLevel ERROR + SetEnv CODER_SSH_SESSION_TYPE=JetBrains +# --- END CODER JETBRAINS test.coder.invalid diff --git a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt index 1cbe5747a..1baafe540 100644 --- a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt @@ -303,6 +303,7 @@ internal class CoderCLIManagerTest { val extraConfig: String = "", val env: Environment = Environment(), val sshLogDirectory: Path? = null, + val url: URL? = null ) @Test @@ -390,6 +391,13 @@ internal class CoderCLIManagerTest { "blank", sshLogDirectory = tmpdir.resolve("ssh-logs"), ), + SSHTest( + listOf("url"), + input = null, + output = "url", + remove = "blank", + url = URL("https://test.coder.invalid?foo=bar&baz=qux"), + ), ) val newlineRe = "\r?\n".toRegex() @@ -408,7 +416,7 @@ internal class CoderCLIManagerTest { env = it.env, ) - val ccm = CoderCLIManager(URL("https://test.coder.invalid"), settings) + val ccm = CoderCLIManager(it.url ?: URL("https://test.coder.invalid"), settings) // Input is the configuration that we start with, if any. if (it.input != null) { diff --git a/src/test/kotlin/com/coder/gateway/util/EscapeTest.kt b/src/test/kotlin/com/coder/gateway/util/EscapeTest.kt index 8da5232e5..3e8265874 100644 --- a/src/test/kotlin/com/coder/gateway/util/EscapeTest.kt +++ b/src/test/kotlin/com/coder/gateway/util/EscapeTest.kt @@ -15,6 +15,10 @@ internal class EscapeTest { """C:\echo "hello world"""" to """"C:\echo \"hello world\""""", """C:\"no"\"spaces"""" to """C:\\"no\"\\"spaces\"""", """"C:\Program Files\HeaderCommand.exe" --flag""" to """"\"C:\Program Files\HeaderCommand.exe\" --flag"""", + "https://coder.com" to """https://coder.com""", + "https://coder.com/?question" to """"https://coder.com/?question"""", + "https://coder.com/&ersand" to """"https://coder.com/&ersand"""", + "https://coder.com/?with&both" to """"https://coder.com/?with&both"""", ) tests.forEach { assertEquals(it.value, escape(it.key))