diff --git a/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt index 5328038f..cbd3c88e 100644 --- a/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt @@ -3,14 +3,15 @@ package com.coder.gateway.cli import com.coder.gateway.cli.ex.MissingVersionException import com.coder.gateway.cli.ex.ResponseException import com.coder.gateway.cli.ex.SSHConfigFormatException -import com.coder.gateway.settings.CoderSettings import com.coder.gateway.services.CoderSettingsState +import com.coder.gateway.settings.CoderSettings import com.coder.gateway.util.CoderHostnameVerifier import com.coder.gateway.util.InvalidVersionException -import com.coder.gateway.util.SemVer import com.coder.gateway.util.OS +import com.coder.gateway.util.SemVer import com.coder.gateway.util.coderSocketFactory import com.coder.gateway.util.escape +import com.coder.gateway.util.escapeSubcommand import com.coder.gateway.util.getHeaders import com.coder.gateway.util.getOS import com.coder.gateway.util.safeHost @@ -228,7 +229,7 @@ class CoderCLIManager( escape(localBinaryPath.toString()), "--global-config", escape(coderConfigPath.toString()), if (settings.headerCommand.isNotBlank()) "--header-command" else null, - if (settings.headerCommand.isNotBlank()) escape(settings.headerCommand) else null, + if (settings.headerCommand.isNotBlank()) escapeSubcommand(settings.headerCommand) else null, "ssh", "--stdio") val blockContent = workspaceNames.joinToString( System.lineSeparator(), diff --git a/src/main/kotlin/com/coder/gateway/util/Escape.kt b/src/main/kotlin/com/coder/gateway/util/Escape.kt index eb927658..8cb71a28 100644 --- a/src/main/kotlin/com/coder/gateway/util/Escape.kt +++ b/src/main/kotlin/com/coder/gateway/util/Escape.kt @@ -1,9 +1,10 @@ package com.coder.gateway.util /** - * 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. + * 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. * * Throws if the argument is invalid. */ @@ -15,4 +16,33 @@ fun escape(s: String): String { return "\"" + s.replace("\"", "\\\"") + "\"" } return s.replace("\"", "\\\"") -} \ No newline at end of file +} + +/** + * Escape an argument to be executed by the Coder binary such that expansions + * happen in the binary and not in SSH. + * + * Escaping happens by wrapping in single quotes on Linux and escaping % on + * Windows. + * + * Throws if the argument is invalid. + */ +fun escapeSubcommand(s: String): String { + if (s.contains("\n")) { + throw Exception("argument cannot contain newlines") + } + return if (getOS() == OS.WINDOWS) { + // On Windows variables are in the format %VAR%. % is interpreted by + // SSH as a special sequence and can be escaped with %%. Do not use + // single quotes on Windows; they appear to only be used literally. + return escape(s).replace("%", "%%") + } else { + // On *nix and similar systems variables are in the format $VAR. SSH + // will expand these before executing the proxy command; we can prevent + // this by using single quotes. You cannot escape single quotes inside + // single quotes, so if there are existing quotes you end the current + // quoted string, output an escaped quote, then start the quoted string + // again. + "'" + s.replace("'", "'\\''") + "'" + } +} diff --git a/src/test/fixtures/outputs/header-command-windows.conf b/src/test/fixtures/outputs/header-command-windows.conf index 4c2223b5..4711c2ab 100644 --- a/src/test/fixtures/outputs/header-command-windows.conf +++ b/src/test/fixtures/outputs/header-command-windows.conf @@ -1,6 +1,6 @@ # --- START CODER JETBRAINS test.coder.invalid Host coder-jetbrains--header--test.coder.invalid - 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\HeaderCommand.exe\" --url=\"%%CODER_URL%%\" --test=\"foo bar\"" 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 index 90fa7704..04e422fb 100644 --- a/src/test/fixtures/outputs/header-command.conf +++ b/src/test/fixtures/outputs/header-command.conf @@ -1,6 +1,6 @@ # --- START CODER JETBRAINS test.coder.invalid Host coder-jetbrains--header--test.coder.invalid - 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 --url="$CODER_URL" --test="foo bar" --literal='\''$CODER_URL'\''' ssh --stdio header ConnectTimeout 0 StrictHostKeyChecking no UserKnownHostsFile /dev/null diff --git a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt index 223f9a86..3c7383b4 100644 --- a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt @@ -3,16 +3,6 @@ package com.coder.gateway.cli import com.coder.gateway.cli.ex.MissingVersionException import com.coder.gateway.cli.ex.ResponseException import com.coder.gateway.cli.ex.SSHConfigFormatException -import kotlin.test.Test -import kotlin.test.assertContains -import kotlin.test.assertEquals -import kotlin.test.assertFailsWith -import kotlin.test.assertFalse -import kotlin.test.assertNotEquals -import kotlin.test.assertTrue -import org.junit.jupiter.api.assertDoesNotThrow -import org.junit.jupiter.api.BeforeAll - import com.coder.gateway.services.CoderSettingsState import com.coder.gateway.settings.CoderSettings import com.coder.gateway.util.InvalidVersionException @@ -24,13 +14,22 @@ import com.coder.gateway.util.sha1 import com.coder.gateway.util.toURL import com.google.gson.JsonSyntaxException import com.sun.net.httpserver.HttpServer +import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.assertDoesNotThrow +import org.zeroturnaround.exec.InvalidExitValueException +import org.zeroturnaround.exec.ProcessInitException import java.net.HttpURLConnection import java.net.InetSocketAddress import java.net.URL import java.nio.file.AccessDeniedException import java.nio.file.Path -import org.zeroturnaround.exec.InvalidExitValueException -import org.zeroturnaround.exec.ProcessInitException +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals +import kotlin.test.assertTrue internal class CoderCLIManagerTest { private fun mkbin(version: String): String { @@ -253,8 +252,11 @@ internal class CoderCLIManagerTest { SSHTest(listOf("foo-bar"), "no-blocks", "append-no-blocks", "no-blocks", null), SSHTest(listOf("foo-bar"), "no-related-blocks", "append-no-related-blocks", "no-related-blocks", null), SSHTest(listOf("foo-bar"), "no-newline", "append-no-newline", "no-blocks", null), - SSHTest(listOf("header"), null, "header-command", "blank", "my-header-command \"test\""), - SSHTest(listOf("header"), null, "header-command-windows", "blank", """C:\Program Files\My Header Command\"also has quotes"\HeaderCommand.exe"""), + if (getOS() == OS.WINDOWS) { + SSHTest(listOf("header"), null, "header-command-windows", "blank", """"C:\Program Files\My Header Command\HeaderCommand.exe" --url="%CODER_URL%" --test="foo bar"""") + } else { + SSHTest(listOf("header"), null, "header-command", "blank", "my-header-command --url=\"\$CODER_URL\" --test=\"foo bar\" --literal='\$CODER_URL'") + } ) val newlineRe = "\r?\n".toRegex() diff --git a/src/test/kotlin/com/coder/gateway/util/EscapeTest.kt b/src/test/kotlin/com/coder/gateway/util/EscapeTest.kt index 0428673d..12eba1c2 100644 --- a/src/test/kotlin/com/coder/gateway/util/EscapeTest.kt +++ b/src/test/kotlin/com/coder/gateway/util/EscapeTest.kt @@ -19,4 +19,22 @@ internal class EscapeTest { assertEquals(it.value, escape(it.key)) } } + + @Test + fun testEscapeSubcommand() { + val tests = if (getOS() == OS.WINDOWS) { + mapOf( + "auth.exe --url=%CODER_URL%" to "\"auth.exe --url=%%CODER_URL%%\"", + "\"my auth.exe\" --url=%CODER_URL%" to "\"\\\"my auth.exe\\\" --url=%%CODER_URL%%\"", + ) + } else { + mapOf( + "auth --url=\$CODER_URL" to "'auth --url=\$CODER_URL'", + "'my auth program' --url=\$CODER_URL" to "''\\''my auth program'\\'' --url=\$CODER_URL'", + ) + } + tests.forEach { + assertEquals(it.value, escapeSubcommand(it.key)) + } + } }