Skip to content

Prevent early variable expansion in header command #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Prevent early variable expansion in header command #364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
38 changes: 34 additions & 4 deletions src/main/kotlin/com/coder/gateway/util/Escape.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
*/
Expand All @@ -15,4 +16,33 @@ fun escape(s: String): String {
return "\"" + s.replace("\"", "\\\"") + "\""
}
return s.replace("\"", "\\\"")
}
}

/**
* 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("'", "'\\''") + "'"
}
}
2 changes: 1 addition & 1 deletion src/test/fixtures/outputs/header-command-windows.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/test/fixtures/outputs/header-command.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down
30 changes: 16 additions & 14 deletions src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down
18 changes: 18 additions & 0 deletions src/test/kotlin/com/coder/gateway/util/EscapeTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
}