From 4227ebdd6b15ccc7d76e0100c6d0d44fdc5220cb Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 8 Jul 2025 23:25:52 +0300 Subject: [PATCH 01/30] impl: new UI setting for running unsigned binary execution A new UI setting was introduced to allow users to run unsigned binaries without any input from the user. Defaults to false which means if a binary is unsigned we will ask the user what to do next. --- .../com/coder/toolbox/settings/ReadOnlyCoderSettings.kt | 6 ++++++ .../kotlin/com/coder/toolbox/store/CoderSettingsStore.kt | 6 ++++++ src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt | 2 ++ .../kotlin/com/coder/toolbox/views/CoderSettingsPage.kt | 7 +++++++ src/main/resources/localization/defaultMessages.po | 3 +++ 5 files changed, 24 insertions(+) diff --git a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt index 4d17c09..d3dd17d 100644 --- a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt +++ b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt @@ -27,6 +27,12 @@ interface ReadOnlyCoderSettings { */ val binaryDirectory: String? + /** + * Controls whether we run the unsigned binary or we prompt + * the user for input. + */ + val allowUnsignedBinaryWithoutPrompt: Boolean + /** * Default CLI binary name based on OS and architecture */ diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index d08e8d6..cea8d44 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -37,6 +37,8 @@ class CoderSettingsStore( override val defaultURL: String get() = store[DEFAULT_URL] ?: "https://dev.coder.com" override val binarySource: String? get() = store[BINARY_SOURCE] override val binaryDirectory: String? get() = store[BINARY_DIRECTORY] + override val allowUnsignedBinaryWithoutPrompt: Boolean = + store[ALLOW_UNSIGNED_BINARY_EXEC]?.toBooleanStrictOrNull() ?: false override val defaultCliBinaryNameByOsAndArch: String get() = getCoderCLIForOS(getOS(), getArch()) override val binaryName: String get() = store[BINARY_NAME] ?: getCoderCLIForOS(getOS(), getArch()) override val dataDirectory: String? get() = store[DATA_DIRECTORY] @@ -158,6 +160,10 @@ class CoderSettingsStore( store[ENABLE_DOWNLOADS] = shouldEnableDownloads.toString() } + fun updateAllowUnsignedBinaryExec(allowUnsignedBinaryExec: Boolean) { + store[ALLOW_UNSIGNED_BINARY_EXEC] = allowUnsignedBinaryExec.toString() + } + fun updateBinaryDirectoryFallback(shouldEnableBinDirFallback: Boolean) { store[ENABLE_BINARY_DIR_FALLBACK] = shouldEnableBinDirFallback.toString() } diff --git a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt index e34436f..637df32 100644 --- a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt +++ b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt @@ -10,6 +10,8 @@ internal const val BINARY_SOURCE = "binarySource" internal const val BINARY_DIRECTORY = "binaryDirectory" +internal const val ALLOW_UNSIGNED_BINARY_EXEC = "allowUnsignedBinaryExec" + internal const val BINARY_NAME = "binaryName" internal const val DATA_DIRECTORY = "dataDirectory" diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt index 61827be..b731b9d 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt @@ -32,6 +32,11 @@ class CoderSettingsPage(context: CoderToolboxContext, triggerSshConfig: Channel< TextField(context.i18n.ptrl("Data directory"), settings.dataDirectory ?: "", TextType.General) private val enableDownloadsField = CheckboxField(settings.enableDownloads, context.i18n.ptrl("Enable downloads")) + private val allowUnsignedBinaryExecField = + CheckboxField( + settings.allowUnsignedBinaryWithoutPrompt, + context.i18n.ptrl("Allow unsigned binary execution without prompt") + ) private val enableBinaryDirectoryFallbackField = CheckboxField( settings.enableBinaryDirectoryFallback, @@ -66,6 +71,7 @@ class CoderSettingsPage(context: CoderToolboxContext, triggerSshConfig: Channel< enableDownloadsField, binaryDirectoryField, enableBinaryDirectoryFallbackField, + allowUnsignedBinaryExecField, dataDirectoryField, headerCommandField, tlsCertPathField, @@ -87,6 +93,7 @@ class CoderSettingsPage(context: CoderToolboxContext, triggerSshConfig: Channel< context.settingsStore.updateBinaryDirectory(binaryDirectoryField.textState.value) context.settingsStore.updateDataDirectory(dataDirectoryField.textState.value) context.settingsStore.updateEnableDownloads(enableDownloadsField.checkedState.value) + context.settingsStore.updateAllowUnsignedBinaryExec(allowUnsignedBinaryExecField.checkedState.value) context.settingsStore.updateBinaryDirectoryFallback(enableBinaryDirectoryFallbackField.checkedState.value) context.settingsStore.updateHeaderCommand(headerCommandField.textState.value) context.settingsStore.updateCertPath(tlsCertPathField.textState.value) diff --git a/src/main/resources/localization/defaultMessages.po b/src/main/resources/localization/defaultMessages.po index 787424c..a1c461d 100644 --- a/src/main/resources/localization/defaultMessages.po +++ b/src/main/resources/localization/defaultMessages.po @@ -79,6 +79,9 @@ msgstr "" msgid "Enable downloads" msgstr "" +msgid "Allow unsigned binary execution without prompt" +msgstr "" + msgid "Enable binary directory fallback" msgstr "" From 021e53a2fe2afae0c46408357865a1f815e951d7 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 10 Jul 2025 01:00:10 +0300 Subject: [PATCH 02/30] chore: refactor CLI downloading logic I moved and modified the logic from CliManager.download to a separate http client based on okhttp and retrofit. The refactor will allow us to easily add new steps in the main download method, and also to easily download new resources. Long term we could also re-use the okhttp client to avoid setting twice the same boilerplate (proxy which is missing from CLIManager, hostname verification and other tls settings) between cli downloader and the rest client --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 214 ++++++------------ .../cli/downloader/CoderDownloadApi.kt | 23 ++ .../cli/downloader/CoderDownloadService.kt | 146 ++++++++++++ .../toolbox/cli/downloader/DownloadResult.kt | 10 + 4 files changed, 249 insertions(+), 144 deletions(-) create mode 100644 src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadApi.kt create mode 100644 src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt create mode 100644 src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index e4ef501..12f272f 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -1,40 +1,35 @@ package com.coder.toolbox.cli import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.cli.downloader.CoderDownloadApi +import com.coder.toolbox.cli.downloader.CoderDownloadService +import com.coder.toolbox.cli.downloader.DownloadResult import com.coder.toolbox.cli.ex.MissingVersionException -import com.coder.toolbox.cli.ex.ResponseException import com.coder.toolbox.cli.ex.SSHConfigFormatException import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent -import com.coder.toolbox.settings.ReadOnlyCoderSettings import com.coder.toolbox.util.CoderHostnameVerifier import com.coder.toolbox.util.InvalidVersionException -import com.coder.toolbox.util.OS import com.coder.toolbox.util.SemVer import com.coder.toolbox.util.coderSocketFactory +import com.coder.toolbox.util.coderTrustManagers import com.coder.toolbox.util.escape import com.coder.toolbox.util.escapeSubcommand -import com.coder.toolbox.util.getHeaders -import com.coder.toolbox.util.getOS import com.coder.toolbox.util.safeHost -import com.coder.toolbox.util.sha1 -import com.jetbrains.toolbox.api.core.diagnostics.Logger import com.squareup.moshi.Json import com.squareup.moshi.JsonClass import com.squareup.moshi.JsonDataException import com.squareup.moshi.Moshi +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.OkHttpClient import org.zeroturnaround.exec.ProcessExecutor +import retrofit2.Retrofit import java.io.EOFException -import java.io.FileInputStream import java.io.FileNotFoundException -import java.net.ConnectException -import java.net.HttpURLConnection import java.net.URL -import java.nio.file.Files import java.nio.file.Path -import java.nio.file.StandardOpenOption -import java.util.zip.GZIPInputStream -import javax.net.ssl.HttpsURLConnection +import javax.net.ssl.X509TrustManager /** * Version output from the CLI's version command. @@ -58,14 +53,14 @@ private const val DOWNLOADING_CODER_CLI = "Downloading Coder CLI..." * 6. Since the binary directory can be read-only, if downloading fails, start * from step 2 with the data directory. */ -fun ensureCLI( +suspend fun ensureCLI( context: CoderToolboxContext, deploymentURL: URL, buildVersion: String, showTextProgress: (String) -> Unit ): CoderCLIManager { val settings = context.settingsStore.readOnly() - val cli = CoderCLIManager(deploymentURL, context.logger, settings) + val cli = CoderCLIManager(context, deploymentURL) // Short-circuit if we already have the expected version. This // lets us bypass the 304 which is slower and may not be @@ -74,6 +69,7 @@ fun ensureCLI( // the 304 method. val cliMatches = cli.matchesVersion(buildVersion) if (cliMatches == true) { + context.logger.info("Local CLI version matches server version: $buildVersion") return cli } @@ -95,7 +91,7 @@ fun ensureCLI( } // Try falling back to the data directory. - val dataCLI = CoderCLIManager(deploymentURL, context.logger, settings, true) + val dataCLI = CoderCLIManager(context, deploymentURL, true) val dataCLIMatches = dataCLI.matchesVersion(buildVersion) if (dataCLIMatches == true) { return dataCLI @@ -126,122 +122,52 @@ data class Features( * Manage the CLI for a single deployment. */ class CoderCLIManager( + private val context: CoderToolboxContext, // The URL of the deployment this CLI is for. private val deploymentURL: URL, - private val logger: Logger, - // Plugin configuration. - private val settings: ReadOnlyCoderSettings, // If the binary directory is not writable, this can be used to force the // manager to download to the data directory instead. - forceDownloadToData: Boolean = false, + private val forceDownloadToData: Boolean = false, ) { - val remoteBinaryURL: URL = settings.binSource(deploymentURL) - val localBinaryPath: Path = settings.binPath(deploymentURL, forceDownloadToData) - val coderConfigPath: Path = settings.dataDir(deploymentURL).resolve("config") - - /** - * Download the CLI from the deployment if necessary. - */ - fun download(buildVersion: String, showTextProgress: (String) -> Unit): Boolean { - val eTag = getBinaryETag() - val conn = remoteBinaryURL.openConnection() as HttpURLConnection - if (!settings.headerCommand.isNullOrBlank()) { - val headersFromHeaderCommand = getHeaders(deploymentURL, settings.headerCommand) - for ((key, value) in headersFromHeaderCommand) { - conn.setRequestProperty(key, value) - } - } - if (eTag != null) { - logger.info("Found existing binary at $localBinaryPath; calculated hash as $eTag") - conn.setRequestProperty("If-None-Match", "\"$eTag\"") - } - conn.setRequestProperty("Accept-Encoding", "gzip") - if (conn is HttpsURLConnection) { - conn.sslSocketFactory = coderSocketFactory(settings.tls) - conn.hostnameVerifier = CoderHostnameVerifier(settings.tls.altHostname) - } - - try { - conn.connect() - logger.info("GET ${conn.responseCode} $remoteBinaryURL") - when (conn.responseCode) { - HttpURLConnection.HTTP_OK -> { - logger.info("Downloading binary to $localBinaryPath") - Files.deleteIfExists(localBinaryPath) - Files.createDirectories(localBinaryPath.parent) - val outputStream = Files.newOutputStream( - localBinaryPath, - StandardOpenOption.CREATE, - StandardOpenOption.TRUNCATE_EXISTING - ) - val sourceStream = if (conn.isGzip()) GZIPInputStream(conn.inputStream) else conn.inputStream - - val buffer = ByteArray(DEFAULT_BUFFER_SIZE) - var bytesRead: Int - var totalRead = 0L - - sourceStream.use { source -> - outputStream.use { sink -> - while (source.read(buffer).also { bytesRead = it } != -1) { - sink.write(buffer, 0, bytesRead) - totalRead += bytesRead - showTextProgress("${settings.defaultCliBinaryNameByOsAndArch} $buildVersion - ${totalRead.toHumanReadableSize()} downloaded") - } - } - } - if (getOS() != OS.WINDOWS) { - localBinaryPath.toFile().setExecutable(true) - } - return true - } - - HttpURLConnection.HTTP_NOT_MODIFIED -> { - logger.info("Using cached binary at $localBinaryPath") - showTextProgress("Using cached binary") - return false - } - } - } catch (e: ConnectException) { - // Add the URL so this is more easily debugged. - throw ConnectException("${e.message} to $remoteBinaryURL") - } finally { - conn.disconnect() - } - throw ResponseException("Unexpected response from $remoteBinaryURL", conn.responseCode) - } - - private fun HttpURLConnection.isGzip(): Boolean = this.contentEncoding.equals("gzip", ignoreCase = true) - - fun Long.toHumanReadableSize(): String { - if (this < 1024) return "$this B" - - val kb = this / 1024.0 - if (kb < 1024) return String.format("%.1f KB", kb) + private val downloader = createDownloadService() + val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentURL) + val localBinaryPath: Path = context.settingsStore.binPath(deploymentURL, forceDownloadToData) + val coderConfigPath: Path = context.settingsStore.dataDir(deploymentURL).resolve("config") + + private fun createDownloadService(): CoderDownloadService { + val okHttpClient = OkHttpClient.Builder() + .sslSocketFactory( + coderSocketFactory(context.settingsStore.tls), + coderTrustManagers(context.settingsStore.tls.caPath)[0] as X509TrustManager + ) + .hostnameVerifier(CoderHostnameVerifier(context.settingsStore.tls.altHostname)) + .build() - val mb = kb / 1024.0 - if (mb < 1024) return String.format("%.1f MB", mb) + val retrofit = Retrofit.Builder() + .baseUrl(deploymentURL.toString()) + .client(okHttpClient) + .build() - val gb = mb / 1024.0 - return String.format("%.1f GB", gb) + val service = retrofit.create(CoderDownloadApi::class.java) + return CoderDownloadService(context, service, deploymentURL, forceDownloadToData) } /** - * Return the entity tag for the binary on disk, if any. + * Download the CLI from the deployment if necessary. */ - private fun getBinaryETag(): String? = try { - sha1(FileInputStream(localBinaryPath.toFile())) - } catch (e: FileNotFoundException) { - null - } catch (e: Exception) { - logger.warn(e, "Unable to calculate hash for $localBinaryPath") - null + suspend fun download(buildVersion: String, showTextProgress: (String) -> Unit): Boolean { + val cliDownloadResult = withContext(Dispatchers.IO) { + downloader.downloadCli(buildVersion, showTextProgress) + } + + return cliDownloadResult == DownloadResult.Downloaded } /** * Use the provided token to initializeSession the CLI. */ fun login(token: String): String { - logger.info("Storing CLI credentials in $coderConfigPath") + context.logger.info("Storing CLI credentials in $coderConfigPath") return exec( "login", deploymentURL.toString(), @@ -261,7 +187,7 @@ class CoderCLIManager( wsWithAgents: Set>, feats: Features = features, ) { - logger.info("Configuring SSH config at ${settings.sshConfigPath}") + context.logger.info("Configuring SSH config at ${context.settingsStore.sshConfigPath}") writeSSHConfig(modifySSHConfig(readSSHConfig(), wsWithAgents, feats)) } @@ -269,8 +195,8 @@ class CoderCLIManager( * Return the contents of the SSH config or null if it does not exist. */ private fun readSSHConfig(): String? = try { - Path.of(settings.sshConfigPath).toFile().readText() - } catch (e: FileNotFoundException) { + Path.of(context.settingsStore.sshConfigPath).toFile().readText() + } catch (_: FileNotFoundException) { null } @@ -301,21 +227,21 @@ class CoderCLIManager( // always use the correct URL. "--url", escape(deploymentURL.toString()), - if (!settings.headerCommand.isNullOrBlank()) "--header-command" else null, - if (!settings.headerCommand.isNullOrBlank()) escapeSubcommand(settings.headerCommand!!) else null, + if (!context.settingsStore.headerCommand.isNullOrBlank()) "--header-command" else null, + if (!context.settingsStore.headerCommand.isNullOrBlank()) escapeSubcommand(context.settingsStore.headerCommand!!) else null, "ssh", "--stdio", - if (settings.disableAutostart && feats.disableAutostart) "--disable-autostart" else null, - "--network-info-dir ${escape(settings.networkInfoDir)}" + if (context.settingsStore.disableAutostart && feats.disableAutostart) "--disable-autostart" else null, + "--network-info-dir ${escape(context.settingsStore.networkInfoDir)}" ) val proxyArgs = baseArgs + listOfNotNull( - if (!settings.sshLogDirectory.isNullOrBlank()) "--log-dir" else null, - if (!settings.sshLogDirectory.isNullOrBlank()) escape(settings.sshLogDirectory!!) else null, + if (!context.settingsStore.sshLogDirectory.isNullOrBlank()) "--log-dir" else null, + if (!context.settingsStore.sshLogDirectory.isNullOrBlank()) escape(context.settingsStore.sshLogDirectory!!) else null, if (feats.reportWorkspaceUsage) "--usage-app=jetbrains" else null, ) val extraConfig = - if (!settings.sshConfigOptions.isNullOrBlank()) { - "\n" + settings.sshConfigOptions!!.prependIndent(" ") + if (!context.settingsStore.sshConfigOptions.isNullOrBlank()) { + "\n" + context.settingsStore.sshConfigOptions!!.prependIndent(" ") } else { "" } @@ -327,7 +253,7 @@ class CoderCLIManager( SetEnv CODER_SSH_SESSION_TYPE=JetBrains """.trimIndent() - val blockContent = if (settings.isSshWildcardConfigEnabled && feats.wildcardSsh) { + val blockContent = if (context.settingsStore.isSshWildcardConfigEnabled && feats.wildcardSsh) { startBlock + System.lineSeparator() + """ Host ${getHostnamePrefix(deploymentURL)}--* @@ -357,7 +283,7 @@ class CoderCLIManager( } if (contents == null) { - logger.info("No existing SSH config to modify") + context.logger.info("No existing SSH config to modify") return blockContent + System.lineSeparator() } @@ -365,12 +291,12 @@ class CoderCLIManager( val end = "$endBlock(\\s*)".toRegex().find(contents) if (start == null && end == null && isRemoving) { - logger.info("No workspaces and no existing config blocks to remove") + context.logger.info("No workspaces and no existing config blocks to remove") return null } if (start == null && end == null) { - logger.info("Appending config block") + context.logger.info("Appending config block") val toAppend = if (contents.isEmpty()) { blockContent @@ -394,7 +320,7 @@ class CoderCLIManager( } if (isRemoving) { - logger.info("No workspaces; removing config block") + context.logger.info("No workspaces; removing config block") return listOf( contents.substring(0, start.range.first), // Need to keep the trailing newline(s) if we are not at the @@ -405,7 +331,7 @@ class CoderCLIManager( ).joinToString("") } - logger.info("Replacing existing config block") + context.logger.info("Replacing existing config block") return listOf( contents.substring(0, start.range.first), start.groupValues[1], // Leading newline(s). @@ -420,14 +346,14 @@ class CoderCLIManager( */ private fun writeSSHConfig(contents: String?) { if (contents != null) { - if (!settings.sshConfigPath.isNullOrBlank()) { - val sshConfPath = Path.of(settings.sshConfigPath) + if (!context.settingsStore.sshConfigPath.isNullOrBlank()) { + val sshConfPath = Path.of(context.settingsStore.sshConfigPath) sshConfPath.parent.toFile().mkdirs() sshConfPath.toFile().writeText(contents) } // The Coder cli will *not* create the log directory. - if (!settings.sshLogDirectory.isNullOrBlank()) { - Path.of(settings.sshLogDirectory).toFile().mkdirs() + if (!context.settingsStore.sshLogDirectory.isNullOrBlank()) { + Path.of(context.settingsStore.sshLogDirectory).toFile().mkdirs() } } } @@ -460,14 +386,14 @@ class CoderCLIManager( } catch (e: Exception) { when (e) { is InvalidVersionException -> { - logger.info("Got invalid version from $localBinaryPath: ${e.message}") + context.logger.info("Got invalid version from $localBinaryPath: ${e.message}") } else -> { // An error here most likely means the CLI does not exist or // it executed successfully but output no version which // suggests it is not the right binary. - logger.info("Unable to determine $localBinaryPath version: ${e.message}") + context.logger.info("Unable to determine $localBinaryPath version: ${e.message}") } } null @@ -485,12 +411,12 @@ class CoderCLIManager( try { SemVer.parse(rawBuildVersion) } catch (e: InvalidVersionException) { - logger.info("Got invalid build version: $rawBuildVersion") + context.logger.info("Got invalid build version: $rawBuildVersion") return null } val matches = cliVersion == buildVersion - logger.info("$localBinaryPath version $cliVersion matches $buildVersion: $matches") + context.logger.info("$localBinaryPath version $cliVersion matches $buildVersion: $matches") return matches } @@ -498,13 +424,13 @@ class CoderCLIManager( val stdout = ProcessExecutor() .command(localBinaryPath.toString(), *args) - .environment("CODER_HEADER_COMMAND", settings.headerCommand) + .environment("CODER_HEADER_COMMAND", context.settingsStore.headerCommand) .exitValues(0) .readOutput(true) .execute() .outputUTF8() val redactedArgs = listOf(*args).joinToString(" ").replace(tokenRegex, "--token ") - logger.info("`$localBinaryPath $redactedArgs`: $stdout") + context.logger.info("`$localBinaryPath $redactedArgs`: $stdout") return stdout } @@ -523,7 +449,7 @@ class CoderCLIManager( } fun getHostname(url: URL, ws: Workspace, agent: WorkspaceAgent): String { - return if (settings.isSshWildcardConfigEnabled && features.wildcardSsh) { + return if (context.settingsStore.isSshWildcardConfigEnabled && features.wildcardSsh) { "${getHostnamePrefix(url)}--${ws.ownerName}--${ws.name}.${agent.name}" } else { "coder-jetbrains-toolbox--${ws.ownerName}--${ws.name}.${agent.name}--${url.safeHost()}" diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadApi.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadApi.kt new file mode 100644 index 0000000..5a5e5b1 --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadApi.kt @@ -0,0 +1,23 @@ +package com.coder.toolbox.cli.downloader + +import okhttp3.ResponseBody +import retrofit2.Response +import retrofit2.http.GET +import retrofit2.http.Header +import retrofit2.http.HeaderMap +import retrofit2.http.Streaming +import retrofit2.http.Url + +/** + * Retrofit API for downloading CLI + */ +interface CoderDownloadApi { + @GET + @Streaming + suspend fun downloadCli( + @Url url: String, + @Header("If-None-Match") eTag: String? = null, + @HeaderMap headers: Map = emptyMap(), + @Header("Accept-Encoding") acceptEncoding: String = "gzip", + ): Response +} \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt new file mode 100644 index 0000000..9767e1d --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt @@ -0,0 +1,146 @@ +package com.coder.toolbox.cli.downloader + +import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.cli.ex.ResponseException +import com.coder.toolbox.util.OS +import com.coder.toolbox.util.getHeaders +import com.coder.toolbox.util.getOS +import com.coder.toolbox.util.sha1 +import okhttp3.ResponseBody +import retrofit2.Response +import java.io.FileInputStream +import java.net.HttpURLConnection.HTTP_NOT_MODIFIED +import java.net.HttpURLConnection.HTTP_OK +import java.net.URL +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.StandardOpenOption +import java.util.zip.GZIPInputStream +import kotlin.io.path.notExists + +/** + * Handles the download steps of Coder CLI + */ +class CoderDownloadService( + private val context: CoderToolboxContext, + private val downloadApi: CoderDownloadApi, + private val deploymentUrl: URL, + forceDownloadToData: Boolean, +) { + val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentUrl) + val localBinaryPath: Path = context.settingsStore.binPath(deploymentUrl, forceDownloadToData) + + suspend fun downloadCli(buildVersion: String, showTextProgress: (String) -> Unit): DownloadResult { + val eTag = calculateLocalETag() + val headers = getRequestHeaders() + if (eTag != null) { + context.logger.info("Found existing binary at $localBinaryPath; calculated hash as $eTag") + } + val response = downloadApi.downloadCli( + url = remoteBinaryURL.toString(), + eTag = eTag?.let { "\"$it\"" }, + headers = headers + ) + + return when (response.code()) { + HTTP_OK -> { + context.logger.info("Downloading binary to $localBinaryPath") + response.saveBinaryToDisk(buildVersion, showTextProgress)?.makeExecutable() + DownloadResult.Downloaded + } + + HTTP_NOT_MODIFIED -> { + context.logger.info("Using cached binary at $localBinaryPath") + showTextProgress("Using cached binary") + DownloadResult.Skipped + } + + else -> { + throw ResponseException( + "Unexpected response from $remoteBinaryURL", + response.code() + ) + } + } + } + + private fun calculateLocalETag(): String? { + return try { + if (localBinaryPath.notExists()) { + return null + } + sha1(FileInputStream(localBinaryPath.toFile())) + } catch (e: Exception) { + context.logger.warn(e, "Unable to calculate hash for $localBinaryPath") + null + } + } + + private fun getRequestHeaders(): Map { + return if (context.settingsStore.headerCommand.isNullOrBlank()) { + emptyMap() + } else { + getHeaders(deploymentUrl, context.settingsStore.headerCommand) + } + } + + private fun Response.saveBinaryToDisk( + buildVersion: String, + showTextProgress: (String) -> Unit + ): Path? { + val responseBody = this.body() ?: return null + Files.deleteIfExists(localBinaryPath) + Files.createDirectories(localBinaryPath.parent) + + val outputStream = Files.newOutputStream( + localBinaryPath, + StandardOpenOption.CREATE, + StandardOpenOption.TRUNCATE_EXISTING + ) + val contentEncoding = this.headers()["Content-Encoding"] + val sourceStream = if (contentEncoding?.contains("gzip", ignoreCase = true) == true) { + GZIPInputStream(responseBody.byteStream()) + } else { + responseBody.byteStream() + } + + val buffer = ByteArray(DEFAULT_BUFFER_SIZE) + var bytesRead: Int + var totalRead = 0L + // caching this because the settings store recomputes it every time + val binaryName = context.settingsStore.defaultCliBinaryNameByOsAndArch + sourceStream.use { source -> + outputStream.use { sink -> + while (source.read(buffer).also { bytesRead = it } != -1) { + sink.write(buffer, 0, bytesRead) + totalRead += bytesRead + showTextProgress( + "$binaryName $buildVersion - ${totalRead.toHumanReadableSize()} downloaded" + ) + } + } + } + return localBinaryPath + } + + + private fun Path.makeExecutable() { + if (getOS() != OS.WINDOWS) { + context.logger.info("Making $this executable...") + this.toFile().setExecutable(true) + } + } + + private fun Long.toHumanReadableSize(): String { + if (this < 1024) return "$this B" + + val kb = this / 1024.0 + if (kb < 1024) return String.format("%.1f KB", kb) + + val mb = kb / 1024.0 + if (mb < 1024) return String.format("%.1f MB", mb) + + val gb = mb / 1024.0 + return String.format("%.1f GB", gb) + } +} \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt new file mode 100644 index 0000000..1994c6b --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt @@ -0,0 +1,10 @@ +package com.coder.toolbox.cli.downloader + +/** + * Result of a download operation + */ +sealed class DownloadResult { + object Skipped : DownloadResult() + object Downloaded : DownloadResult() + data class Failed(val error: Exception) : DownloadResult() +} \ No newline at end of file From fb6e78459d0d184aaa8b27c31b9d47bf4ff1c754 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 10 Jul 2025 01:51:10 +0300 Subject: [PATCH 03/30] impl: support for downloading the cli signature From the same source where the cli binary was downloaded. Some of the previous classes like download result were updated to incorporate details like where the file was saved or whether a file was found on the remote --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 7 ++- .../cli/downloader/CoderDownloadApi.kt | 6 ++ .../cli/downloader/CoderDownloadService.kt | 60 +++++++++++++++---- .../toolbox/cli/downloader/DownloadResult.kt | 8 ++- .../com/coder/toolbox/util/URLExtensions.kt | 14 +++++ 5 files changed, 81 insertions(+), 14 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 12f272f..2f4ce27 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -3,7 +3,6 @@ package com.coder.toolbox.cli import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.cli.downloader.CoderDownloadApi import com.coder.toolbox.cli.downloader.CoderDownloadService -import com.coder.toolbox.cli.downloader.DownloadResult import com.coder.toolbox.cli.ex.MissingVersionException import com.coder.toolbox.cli.ex.SSHConfigFormatException import com.coder.toolbox.sdk.v2.models.Workspace @@ -160,7 +159,11 @@ class CoderCLIManager( downloader.downloadCli(buildVersion, showTextProgress) } - return cliDownloadResult == DownloadResult.Downloaded + var singatureDownloadResult = withContext(Dispatchers.IO) { + downloader.downloadSignature(showTextProgress) + } + + return cliDownloadResult.isDownloaded() } /** diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadApi.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadApi.kt index 5a5e5b1..4e27569 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadApi.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadApi.kt @@ -20,4 +20,10 @@ interface CoderDownloadApi { @HeaderMap headers: Map = emptyMap(), @Header("Accept-Encoding") acceptEncoding: String = "gzip", ): Response + + @GET + suspend fun downloadSignature( + @Url url: String, + @HeaderMap headers: Map = emptyMap() + ): Response } \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt index 9767e1d..6e1bd63 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt @@ -6,9 +6,11 @@ import com.coder.toolbox.util.OS import com.coder.toolbox.util.getHeaders import com.coder.toolbox.util.getOS import com.coder.toolbox.util.sha1 +import com.coder.toolbox.util.withLastSegment import okhttp3.ResponseBody import retrofit2.Response import java.io.FileInputStream +import java.net.HttpURLConnection.HTTP_NOT_FOUND import java.net.HttpURLConnection.HTTP_NOT_MODIFIED import java.net.HttpURLConnection.HTTP_OK import java.net.URL @@ -16,6 +18,7 @@ import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardOpenOption import java.util.zip.GZIPInputStream +import kotlin.io.path.name import kotlin.io.path.notExists /** @@ -32,21 +35,20 @@ class CoderDownloadService( suspend fun downloadCli(buildVersion: String, showTextProgress: (String) -> Unit): DownloadResult { val eTag = calculateLocalETag() - val headers = getRequestHeaders() if (eTag != null) { context.logger.info("Found existing binary at $localBinaryPath; calculated hash as $eTag") } val response = downloadApi.downloadCli( url = remoteBinaryURL.toString(), eTag = eTag?.let { "\"$it\"" }, - headers = headers + headers = getRequestHeaders() ) return when (response.code()) { HTTP_OK -> { context.logger.info("Downloading binary to $localBinaryPath") - response.saveBinaryToDisk(buildVersion, showTextProgress)?.makeExecutable() - DownloadResult.Downloaded + response.saveToDisk(localBinaryPath, showTextProgress, buildVersion)?.makeExecutable() + DownloadResult.Downloaded(localBinaryPath) } HTTP_NOT_MODIFIED -> { @@ -84,16 +86,17 @@ class CoderDownloadService( } } - private fun Response.saveBinaryToDisk( - buildVersion: String, - showTextProgress: (String) -> Unit + private fun Response.saveToDisk( + localPath: Path, + showTextProgress: (String) -> Unit, + buildVersion: String? = null ): Path? { val responseBody = this.body() ?: return null - Files.deleteIfExists(localBinaryPath) - Files.createDirectories(localBinaryPath.parent) + Files.deleteIfExists(localPath) + Files.createDirectories(localPath.parent) val outputStream = Files.newOutputStream( - localBinaryPath, + localPath, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING ) @@ -108,7 +111,7 @@ class CoderDownloadService( var bytesRead: Int var totalRead = 0L // caching this because the settings store recomputes it every time - val binaryName = context.settingsStore.defaultCliBinaryNameByOsAndArch + val binaryName = localPath.name sourceStream.use { source -> outputStream.use { sink -> while (source.read(buffer).also { bytesRead = it } != -1) { @@ -143,4 +146,39 @@ class CoderDownloadService( val gb = mb / 1024.0 return String.format("%.1f GB", gb) } + + suspend fun downloadSignature(showTextProgress: (String) -> Unit): DownloadResult { + val defaultCliNameWithoutExt = context.settingsStore.defaultCliBinaryNameByOsAndArch.split('.').first() + val signatureName = "$defaultCliNameWithoutExt.asc" + + val signatureURL = remoteBinaryURL.withLastSegment(signatureName) + val localSignaturePath = localBinaryPath.parent.resolve(signatureName) + context.logger.info("Downloading signature from $signatureURL") + + val response = downloadApi.downloadSignature( + url = signatureURL.toString(), + headers = getRequestHeaders() + ) + + return when (response.code()) { + HTTP_OK -> { + response.saveToDisk(localSignaturePath, showTextProgress) + DownloadResult.Downloaded(localSignaturePath) + } + + HTTP_NOT_FOUND -> { + context.logger.warn("Signature file not found at $signatureURL") + DownloadResult.NotFound + } + + else -> { + DownloadResult.Failed( + ResponseException( + "Failed to download signature from $signatureURL", + response.code() + ) + ) + } + } + } } \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt index 1994c6b..58ed451 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt @@ -1,10 +1,16 @@ package com.coder.toolbox.cli.downloader +import java.nio.file.Path + + /** * Result of a download operation */ sealed class DownloadResult { object Skipped : DownloadResult() - object Downloaded : DownloadResult() + object NotFound : DownloadResult() + data class Downloaded(val savePath: Path) : DownloadResult() data class Failed(val error: Exception) : DownloadResult() + + fun isDownloaded(): Boolean = this is Downloaded } \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt b/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt index c1aaa81..360b62a 100644 --- a/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt +++ b/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt @@ -13,6 +13,20 @@ fun URL.withPath(path: String): URL = URL( if (path.startsWith("/")) path else "/$path", ) +fun URL.withLastSegment(segment: String): URL { + val uri = this.toURI() + val basePath = uri.path.substringBeforeLast('/') + val newPath = "$basePath/$segment" + val newUri = URI( + uri.scheme, + uri.authority, + newPath, + uri.query, + uri.fragment + ) + return newUri.toURL() +} + /** * Return the host, converting IDN to ASCII in case the file system cannot * support the necessary character set. From dbf85606c700b0313f34993c5130281cbe6654cb Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 10 Jul 2025 22:15:13 +0300 Subject: [PATCH 04/30] impl: support for downloading the releases.coder.com signature --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 7 +++++++ .../cli/downloader/CoderDownloadService.kt | 16 +++++++++++++--- .../toolbox/cli/downloader/DownloadResult.kt | 9 ++++++++- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 2f4ce27..3ee7135 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -158,11 +158,18 @@ class CoderCLIManager( val cliDownloadResult = withContext(Dispatchers.IO) { downloader.downloadCli(buildVersion, showTextProgress) } + if (cliDownloadResult.isSkipped()) return false + if (cliDownloadResult.isNotFoundOrFailed()) throw IllegalStateException("Could not find or download Coder CLI") var singatureDownloadResult = withContext(Dispatchers.IO) { downloader.downloadSignature(showTextProgress) } + if (singatureDownloadResult.isNotDownloaded()) { + context.logger.info("Trying to download signature file from releases.coder.com") + singatureDownloadResult = downloader.downloadReleasesSignature(showTextProgress) + } + return cliDownloadResult.isDownloaded() } diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt index 6e1bd63..d28c5e6 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt @@ -13,6 +13,7 @@ import java.io.FileInputStream import java.net.HttpURLConnection.HTTP_NOT_FOUND import java.net.HttpURLConnection.HTTP_NOT_MODIFIED import java.net.HttpURLConnection.HTTP_OK +import java.net.URI import java.net.URL import java.nio.file.Files import java.nio.file.Path @@ -48,7 +49,7 @@ class CoderDownloadService( HTTP_OK -> { context.logger.info("Downloading binary to $localBinaryPath") response.saveToDisk(localBinaryPath, showTextProgress, buildVersion)?.makeExecutable() - DownloadResult.Downloaded(localBinaryPath) + DownloadResult.Downloaded(remoteBinaryURL, localBinaryPath) } HTTP_NOT_MODIFIED -> { @@ -148,10 +149,14 @@ class CoderDownloadService( } suspend fun downloadSignature(showTextProgress: (String) -> Unit): DownloadResult { + return downloadSignature(remoteBinaryURL, showTextProgress) + } + + private suspend fun downloadSignature(url: URL, showTextProgress: (String) -> Unit): DownloadResult { val defaultCliNameWithoutExt = context.settingsStore.defaultCliBinaryNameByOsAndArch.split('.').first() val signatureName = "$defaultCliNameWithoutExt.asc" - val signatureURL = remoteBinaryURL.withLastSegment(signatureName) + val signatureURL = url.withLastSegment(signatureName) val localSignaturePath = localBinaryPath.parent.resolve(signatureName) context.logger.info("Downloading signature from $signatureURL") @@ -163,7 +168,7 @@ class CoderDownloadService( return when (response.code()) { HTTP_OK -> { response.saveToDisk(localSignaturePath, showTextProgress) - DownloadResult.Downloaded(localSignaturePath) + DownloadResult.Downloaded(signatureURL, localSignaturePath) } HTTP_NOT_FOUND -> { @@ -180,5 +185,10 @@ class CoderDownloadService( ) } } + + } + + suspend fun downloadReleasesSignature(showTextProgress: (String) -> Unit): DownloadResult { + return downloadSignature(URI.create("https://releases.coder.com/bin").toURL(), showTextProgress) } } \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt index 58ed451..1da4007 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt @@ -1,5 +1,6 @@ package com.coder.toolbox.cli.downloader +import java.net.URL import java.nio.file.Path @@ -9,8 +10,14 @@ import java.nio.file.Path sealed class DownloadResult { object Skipped : DownloadResult() object NotFound : DownloadResult() - data class Downloaded(val savePath: Path) : DownloadResult() + data class Downloaded(val source: URL, val dst: Path) : DownloadResult() data class Failed(val error: Exception) : DownloadResult() + fun isSkipped(): Boolean = this is Skipped + + fun isNotFoundOrFailed(): Boolean = this is NotFound || this is Failed + fun isDownloaded(): Boolean = this is Downloaded + + fun isNotDownloaded(): Boolean = this !is Downloaded } \ No newline at end of file From 675430049b6f339671bdc089adcfe44f68662b22 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 10 Jul 2025 22:49:57 +0300 Subject: [PATCH 05/30] fix: read fresh values from the config store `allowUnsignedBinaryWithoutPrompt` was caching the initial value read from the store, which required a restart of Toolbox for the real value to reflect. --- src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index cea8d44..308e762 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -37,8 +37,8 @@ class CoderSettingsStore( override val defaultURL: String get() = store[DEFAULT_URL] ?: "https://dev.coder.com" override val binarySource: String? get() = store[BINARY_SOURCE] override val binaryDirectory: String? get() = store[BINARY_DIRECTORY] - override val allowUnsignedBinaryWithoutPrompt: Boolean = - store[ALLOW_UNSIGNED_BINARY_EXEC]?.toBooleanStrictOrNull() ?: false + override val allowUnsignedBinaryWithoutPrompt: Boolean + get() = store[ALLOW_UNSIGNED_BINARY_EXEC]?.toBooleanStrictOrNull() ?: false override val defaultCliBinaryNameByOsAndArch: String get() = getCoderCLIForOS(getOS(), getArch()) override val binaryName: String get() = store[BINARY_NAME] ?: getCoderCLIForOS(getOS(), getArch()) override val dataDirectory: String? get() = store[DATA_DIRECTORY] From 8d768ee8a3760b9e824bd29589345207bc9b3d97 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 10 Jul 2025 22:53:16 +0300 Subject: [PATCH 06/30] impl: prompt user when running unsigned binaries A pop-up dialog is displayed asking the user if he wants to run an unsigned cli version. The pop-up can be skipped if the user configures the `Allow unsigned binary execution without prompt` --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 28 +++++++++++++++++++ .../com/coder/toolbox/cli/ex/Exceptions.kt | 2 ++ 2 files changed, 30 insertions(+) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 3ee7135..09a2175 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -3,8 +3,10 @@ package com.coder.toolbox.cli import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.cli.downloader.CoderDownloadApi import com.coder.toolbox.cli.downloader.CoderDownloadService +import com.coder.toolbox.cli.downloader.DownloadResult.Downloaded import com.coder.toolbox.cli.ex.MissingVersionException import com.coder.toolbox.cli.ex.SSHConfigFormatException +import com.coder.toolbox.cli.ex.UnsignedBinaryExecutionDeniedException import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent import com.coder.toolbox.util.CoderHostnameVerifier @@ -27,6 +29,7 @@ import retrofit2.Retrofit import java.io.EOFException import java.io.FileNotFoundException import java.net.URL +import java.nio.file.Files import java.nio.file.Path import javax.net.ssl.X509TrustManager @@ -170,6 +173,31 @@ class CoderCLIManager( singatureDownloadResult = downloader.downloadReleasesSignature(showTextProgress) } + // if we could not find any signature and the user wants to explicitly + // confirm whether we run an unsigned cli + if (cliDownloadResult.isNotDownloaded()) { + val cli = cliDownloadResult as Downloaded + if (context.settingsStore.allowUnsignedBinaryWithoutPrompt) { + context.logger.warn("Running unsigned CLI from ${cli.source}") + } else { + val acceptsUnsignedBinary = context.ui.showYesNoPopup( + context.i18n.ptrl("Security Warning"), + context.i18n.pnotr("Can't verify the integrity of the Coder CLI pulled from ${cli.source}"), + context.i18n.ptrl("Accept"), + context.i18n.ptrl("Abort"), + ) + + if (acceptsUnsignedBinary) { + return true + } else { + // remove the cli, otherwise next time the user tries to login the cached cli is picked up + // and we don't verify cached cli signatures + Files.delete(cli.dst) + throw UnsignedBinaryExecutionDeniedException("Running unsigned CLI from ${cli.source} was denied by the user") + } + } + } + return cliDownloadResult.isDownloaded() } diff --git a/src/main/kotlin/com/coder/toolbox/cli/ex/Exceptions.kt b/src/main/kotlin/com/coder/toolbox/cli/ex/Exceptions.kt index d3ca3a4..23ae90a 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/ex/Exceptions.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/ex/Exceptions.kt @@ -5,3 +5,5 @@ class ResponseException(message: String, val code: Int) : Exception(message) class SSHConfigFormatException(message: String) : Exception(message) class MissingVersionException(message: String) : Exception(message) + +class UnsignedBinaryExecutionDeniedException(message: String) : Exception(message) \ No newline at end of file From ea3e37975a1a610dadd44c7bb3249f2f99bae82e Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 10 Jul 2025 23:48:58 +0300 Subject: [PATCH 07/30] fix: used proper result to verify if signature is downloaded --- src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 09a2175..870f037 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -132,6 +132,7 @@ class CoderCLIManager( private val forceDownloadToData: Boolean = false, ) { private val downloader = createDownloadService() + val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentURL) val localBinaryPath: Path = context.settingsStore.binPath(deploymentURL, forceDownloadToData) val coderConfigPath: Path = context.settingsStore.dataDir(deploymentURL).resolve("config") @@ -175,7 +176,7 @@ class CoderCLIManager( // if we could not find any signature and the user wants to explicitly // confirm whether we run an unsigned cli - if (cliDownloadResult.isNotDownloaded()) { + if (singatureDownloadResult.isNotDownloaded()) { val cli = cliDownloadResult as Downloaded if (context.settingsStore.allowUnsignedBinaryWithoutPrompt) { context.logger.warn("Running unsigned CLI from ${cli.source}") From 3668d461163bebf1ada8c3fff568710feda97716 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 11 Jul 2025 00:00:05 +0300 Subject: [PATCH 08/30] chore: compact code and run signature download on the IO thread --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 870f037..525e19e 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -159,31 +159,36 @@ class CoderCLIManager( * Download the CLI from the deployment if necessary. */ suspend fun download(buildVersion: String, showTextProgress: (String) -> Unit): Boolean { - val cliDownloadResult = withContext(Dispatchers.IO) { + val cliResult = withContext(Dispatchers.IO) { downloader.downloadCli(buildVersion, showTextProgress) + }.let { result -> + when { + result.isSkipped() -> return false + result.isNotFoundOrFailed() -> throw IllegalStateException("Could not find or download Coder CLI") + else -> result as Downloaded + } } - if (cliDownloadResult.isSkipped()) return false - if (cliDownloadResult.isNotFoundOrFailed()) throw IllegalStateException("Could not find or download Coder CLI") - var singatureDownloadResult = withContext(Dispatchers.IO) { + var signatureDownloadResult = withContext(Dispatchers.IO) { downloader.downloadSignature(showTextProgress) } - if (singatureDownloadResult.isNotDownloaded()) { + if (signatureDownloadResult.isNotDownloaded()) { context.logger.info("Trying to download signature file from releases.coder.com") - singatureDownloadResult = downloader.downloadReleasesSignature(showTextProgress) + signatureDownloadResult = withContext(Dispatchers.IO) { + downloader.downloadReleasesSignature(showTextProgress) + } } // if we could not find any signature and the user wants to explicitly // confirm whether we run an unsigned cli - if (singatureDownloadResult.isNotDownloaded()) { - val cli = cliDownloadResult as Downloaded + if (signatureDownloadResult.isNotDownloaded()) { if (context.settingsStore.allowUnsignedBinaryWithoutPrompt) { - context.logger.warn("Running unsigned CLI from ${cli.source}") + context.logger.warn("Running unsigned CLI from ${cliResult.source}") } else { val acceptsUnsignedBinary = context.ui.showYesNoPopup( context.i18n.ptrl("Security Warning"), - context.i18n.pnotr("Can't verify the integrity of the Coder CLI pulled from ${cli.source}"), + context.i18n.pnotr("Can't verify the integrity of the Coder CLI pulled from ${cliResult.source}"), context.i18n.ptrl("Accept"), context.i18n.ptrl("Abort"), ) @@ -193,13 +198,13 @@ class CoderCLIManager( } else { // remove the cli, otherwise next time the user tries to login the cached cli is picked up // and we don't verify cached cli signatures - Files.delete(cli.dst) - throw UnsignedBinaryExecutionDeniedException("Running unsigned CLI from ${cli.source} was denied by the user") + Files.delete(cliResult.dst) + throw UnsignedBinaryExecutionDeniedException("Running unsigned CLI from ${cliResult.source} was denied by the user") } } } - return cliDownloadResult.isDownloaded() + return cliResult.isDownloaded() } /** From a4763649702fe0c1922a5d5c99d707ff4cbce333 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 11 Jul 2025 00:23:24 +0300 Subject: [PATCH 09/30] chore: add support for bouncycastle --- build.gradle.kts | 1 + gradle/libs.versions.toml | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index 1e8c5cc..cdfc5e8 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -63,6 +63,7 @@ dependencies { ksp(libs.moshi.codegen) implementation(libs.retrofit) implementation(libs.retrofit.moshi) + implementation(libs.bundles.bouncycastle) testImplementation(kotlin("test")) testImplementation(libs.mokk) testImplementation(libs.bundles.toolbox.plugin.api) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e2dc1b4..7617e32 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -16,6 +16,7 @@ gettext = "0.7.0" plugin-structure = "3.310" mockk = "1.14.4" detekt = "1.23.7" +bouncycastle = "1.81" [libraries] toolbox-core-api = { module = "com.jetbrains.toolbox:core-api", version.ref = "toolbox-plugin-api" } @@ -34,10 +35,13 @@ retrofit-moshi = { module = "com.squareup.retrofit2:converter-moshi", version.re plugin-structure = { module = "org.jetbrains.intellij.plugins:structure-toolbox", version.ref = "plugin-structure" } mokk = { module = "io.mockk:mockk", version.ref = "mockk" } marketplace-client = { module = "org.jetbrains.intellij:plugin-repository-rest-client", version.ref = "marketplace-client" } +bouncycastle-bcpg = { module = "org.bouncycastle:bcpg-jdk18on", version.ref = "bouncycastle" } +bouncycastle-bcprov = { module = "org.bouncycastle:bcprov-jdk18on", version.ref = "bouncycastle" } [bundles] serialization = ["serialization-core", "serialization-json", "serialization-json-okio"] toolbox-plugin-api = ["toolbox-core-api", "toolbox-ui-api", "toolbox-remote-dev-api"] +bouncycastle = ["bouncycastle-bcpg", "bouncycastle-bcprov"] [plugins] kotlin = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin" } From 45a72fb09fe18618b3e22d3e4417ebe19ce103ed Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 11 Jul 2025 00:28:12 +0300 Subject: [PATCH 10/30] chore: update i18n bundle with new strings related to signature verification --- src/main/resources/localization/defaultMessages.po | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main/resources/localization/defaultMessages.po b/src/main/resources/localization/defaultMessages.po index a1c461d..385e6b7 100644 --- a/src/main/resources/localization/defaultMessages.po +++ b/src/main/resources/localization/defaultMessages.po @@ -152,4 +152,13 @@ msgid "Setting up Coder" msgstr "" msgid "Loading workspaces..." +msgstr "" + +msgid "Security Warning" +msgstr "" + +msgid "Accept" +msgstr "" + +msgid "Abort" msgstr "" \ No newline at end of file From ad443468fb2cab74d6057b560b59e673af44c0bc Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 11 Jul 2025 00:42:10 +0300 Subject: [PATCH 11/30] impl: verify gpg signed cli binaries Adds logic to verify the CLI against a detached GPG signature with the help of bouncycastle library --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 31 ++++- .../com/coder/toolbox/cli/ex/Exceptions.kt | 2 +- .../com/coder/toolbox/cli/gpg/GPGVerifier.kt | 122 ++++++++++++++++++ .../toolbox/cli/gpg/VerificationResult.kt | 15 +++ 4 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt create mode 100644 src/main/kotlin/com/coder/toolbox/cli/gpg/VerificationResult.kt diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 525e19e..f696ead 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -7,6 +7,9 @@ import com.coder.toolbox.cli.downloader.DownloadResult.Downloaded import com.coder.toolbox.cli.ex.MissingVersionException import com.coder.toolbox.cli.ex.SSHConfigFormatException import com.coder.toolbox.cli.ex.UnsignedBinaryExecutionDeniedException +import com.coder.toolbox.cli.gpg.GPGVerifier +import com.coder.toolbox.cli.gpg.VerificationResult.Failed +import com.coder.toolbox.cli.gpg.VerificationResult.Invalid import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent import com.coder.toolbox.util.CoderHostnameVerifier @@ -132,6 +135,7 @@ class CoderCLIManager( private val forceDownloadToData: Boolean = false, ) { private val downloader = createDownloadService() + private val gpgVerifier = GPGVerifier(context) val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentURL) val localBinaryPath: Path = context.settingsStore.binPath(deploymentURL, forceDownloadToData) @@ -169,20 +173,20 @@ class CoderCLIManager( } } - var signatureDownloadResult = withContext(Dispatchers.IO) { + var signatureResult = withContext(Dispatchers.IO) { downloader.downloadSignature(showTextProgress) } - if (signatureDownloadResult.isNotDownloaded()) { + if (signatureResult.isNotDownloaded()) { context.logger.info("Trying to download signature file from releases.coder.com") - signatureDownloadResult = withContext(Dispatchers.IO) { + signatureResult = withContext(Dispatchers.IO) { downloader.downloadReleasesSignature(showTextProgress) } } // if we could not find any signature and the user wants to explicitly // confirm whether we run an unsigned cli - if (signatureDownloadResult.isNotDownloaded()) { + if (signatureResult.isNotDownloaded()) { if (context.settingsStore.allowUnsignedBinaryWithoutPrompt) { context.logger.warn("Running unsigned CLI from ${cliResult.source}") } else { @@ -196,7 +200,7 @@ class CoderCLIManager( if (acceptsUnsignedBinary) { return true } else { - // remove the cli, otherwise next time the user tries to login the cached cli is picked up + // remove the cli, otherwise next time the user tries to login the cached cli is picked up, // and we don't verify cached cli signatures Files.delete(cliResult.dst) throw UnsignedBinaryExecutionDeniedException("Running unsigned CLI from ${cliResult.source} was denied by the user") @@ -204,7 +208,22 @@ class CoderCLIManager( } } - return cliResult.isDownloaded() + // we have the cli, and signature is downloaded, let's verify the signature + signatureResult = signatureResult as Downloaded + gpgVerifier.verifySignature(cliResult.dst, signatureResult.dst).let { result -> + when { + result.isValid() -> return true + result.isInvalid() -> { + val reason = (result as Invalid).reason + throw UnsignedBinaryExecutionDeniedException( + "Signature of ${cliResult.dst} is invalid." + reason?.let { " Reason: $it" }.orEmpty() + ) + } + + result.signatureIsNotFound() -> throw UnsignedBinaryExecutionDeniedException("Can't verify signature of ${cliResult.dst} because ${signatureResult.dst} does not exist") + else -> throw UnsignedBinaryExecutionDeniedException((result as Failed).error.message) + } + } } /** diff --git a/src/main/kotlin/com/coder/toolbox/cli/ex/Exceptions.kt b/src/main/kotlin/com/coder/toolbox/cli/ex/Exceptions.kt index 23ae90a..9fdff54 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/ex/Exceptions.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/ex/Exceptions.kt @@ -6,4 +6,4 @@ class SSHConfigFormatException(message: String) : Exception(message) class MissingVersionException(message: String) : Exception(message) -class UnsignedBinaryExecutionDeniedException(message: String) : Exception(message) \ No newline at end of file +class UnsignedBinaryExecutionDeniedException(message: String?) : Exception(message) \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt new file mode 100644 index 0000000..c5944f2 --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt @@ -0,0 +1,122 @@ +package com.coder.toolbox.cli.gpg + +import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.cli.gpg.VerificationResult.Failed +import com.coder.toolbox.cli.gpg.VerificationResult.Invalid +import com.coder.toolbox.cli.gpg.VerificationResult.SignatureNotFound +import com.coder.toolbox.cli.gpg.VerificationResult.Valid +import org.bouncycastle.bcpg.ArmoredInputStream +import org.bouncycastle.openpgp.PGPException +import org.bouncycastle.openpgp.PGPPublicKeyRing +import org.bouncycastle.openpgp.PGPPublicKeyRingCollection +import org.bouncycastle.openpgp.PGPSignatureList +import org.bouncycastle.openpgp.PGPUtil +import org.bouncycastle.openpgp.jcajce.JcaPGPObjectFactory +import org.bouncycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator +import org.bouncycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider +import java.io.ByteArrayInputStream +import java.nio.file.Files +import java.nio.file.Path + +class GPGVerifier( + private val context: CoderToolboxContext, +) { + + fun verifySignature( + cli: Path, + signature: Path, + ): VerificationResult { + return try { + if (!Files.exists(signature)) { + context.logger.warn("Signature file not found, skipping verification") + return SignatureNotFound + } + + val signatureBytes = Files.readAllBytes(signature) + val cliBytes = Files.readAllBytes(cli) + + val publicKeyRing = getCoderPublicKeyRing() + return verifyDetachedSignature( + cliBytes = cliBytes, + signatureBytes = signatureBytes, + publicKeyRing = publicKeyRing + ) + } catch (e: Exception) { + context.logger.error(e, "GPG signature verification failed") + Failed(e) + } + } + + private fun getCoderPublicKeyRing(): PGPPublicKeyRing { + return try { + getDefaultCoderPublicKeyRing() + } catch (e: Exception) { + throw PGPException("Failed to load Coder public GPG key", e) + } + } + + private fun getDefaultCoderPublicKeyRing(): PGPPublicKeyRing { + val coderPublicKey = """ + -----BEGIN PGP PUBLIC KEY BLOCK----- + + # Replace this with Coder's actual public key + + -----END PGP PUBLIC KEY BLOCK----- + """.trimIndent() + + return loadPublicKeyRing(coderPublicKey.toByteArray()) + } + + /** + * Verify a detached GPG signature + */ + fun verifyDetachedSignature( + cliBytes: ByteArray, + signatureBytes: ByteArray, + publicKeyRing: PGPPublicKeyRing + ): VerificationResult { + try { + val signatureInputStream = ArmoredInputStream(ByteArrayInputStream(signatureBytes)) + val pgpObjectFactory = JcaPGPObjectFactory(signatureInputStream) + val signatureList = pgpObjectFactory.nextObject() as? PGPSignatureList + ?: throw PGPException("Invalid signature format") + + if (signatureList.isEmpty) { + return Invalid("No signatures found in signature file") + } + + val signature = signatureList[0] + val publicKey = publicKeyRing.getPublicKey(signature.keyID) + ?: throw PGPException("Public key not found for signature") + + signature.init(JcaPGPContentVerifierBuilderProvider(), publicKey) + signature.update(cliBytes) + + val isValid = signature.verify() + context.logger.info("GPG signature verification result: $isValid") + if (isValid) { + return Valid + } + return Invalid() + } catch (e: Exception) { + context.logger.error(e, "GPG signature verification failed") + return Failed(e) + } + } + + /** + * Load public key ring from bytes + */ + fun loadPublicKeyRing(publicKeyBytes: ByteArray): PGPPublicKeyRing { + return try { + val keyInputStream = ArmoredInputStream(ByteArrayInputStream(publicKeyBytes)) + val keyRingCollection = PGPPublicKeyRingCollection( + PGPUtil.getDecoderStream(keyInputStream), + JcaKeyFingerprintCalculator() + ) + keyRingCollection.keyRings.next() + } catch (e: Exception) { + throw PGPException("Failed to load public key ring", e) + } + } +} \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/cli/gpg/VerificationResult.kt b/src/main/kotlin/com/coder/toolbox/cli/gpg/VerificationResult.kt new file mode 100644 index 0000000..eafafcd --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/cli/gpg/VerificationResult.kt @@ -0,0 +1,15 @@ +package com.coder.toolbox.cli.gpg + +/** + * Result of signature verification + */ +sealed class VerificationResult { + object Valid : VerificationResult() + data class Invalid(val reason: String? = null) : VerificationResult() + data class Failed(val error: Exception) : VerificationResult() + object SignatureNotFound : VerificationResult() + + fun isValid(): Boolean = this == Valid + fun isInvalid(): Boolean = this is Invalid + fun signatureIsNotFound(): Boolean = this == SignatureNotFound +} From 4cd5148c342bc2a0a3bd28b2d9eab58a6836845d Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 11 Jul 2025 22:41:30 +0300 Subject: [PATCH 12/30] impl: embed the pgp public key as a plugin resource This is the key that validates if the gpg signature was tampered --- .../META-INF/trusted-keys/pgp-public.key | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 src/main/resources/META-INF/trusted-keys/pgp-public.key diff --git a/src/main/resources/META-INF/trusted-keys/pgp-public.key b/src/main/resources/META-INF/trusted-keys/pgp-public.key new file mode 100644 index 0000000..fb5c4c5 --- /dev/null +++ b/src/main/resources/META-INF/trusted-keys/pgp-public.key @@ -0,0 +1,99 @@ +-----BEGIN PGP PUBLIC KEY BLOCK----- + +mQINBGPGrCwBEAC7SSKQIFoQdt3jYv/1okRdoleepLDG4NfcG52S45Ex3/fUA6Z/ +ewHQrx//SN+h1FLpb0zQMyamWrSh2O3dnkWridwlskb5/y8C/6OUdk4L/ZgHeyPO +Ncbyl1hqO8oViakiWt4IxwSYo83eJHxOUiCGZlqV6EpEsaur43BRHnK8EciNeIxF +Bjle3yXH1K3EgGGHpgnSoKe1nSVxtWIwX45d06v+VqnBoI6AyK0Zp+Nn8bL0EnXC +xGYU3XOkC6EmITlhMju1AhxnbkQiy8IUxXiaj3NoPc1khapOcyBybhESjRZHlgu4 +ToLZGaypjtfQJgMeFlpua7sJK0ziFMW4wOTX+6Ix/S6XA80dVbl3VEhSMpFCcgI+ +OmEd2JuBs6maG+92fCRIzGAClzV8/ifM//JU9D7Qlq6QJpcbNClODlPNDNe7RUEO +b7Bu7dJJS3VhHO9eEen6m6vRE4DNriHT4Zvq1UkHfpJUW7njzkIYRni3eNrsr4Da +U/eeGbVipok4lzZEOQtuaZlX9ytOdGrWEGMGSosTOG6u6KAKJoz7cQGZiz4pZpjR +3N2SIYv59lgpHrIV7UodGx9nzu0EKBhkoulaP1UzH8F16psSaJXRjeyl/YP8Rd2z +SYgZVLjTzkTUXkJT8fQO8zLBEuwA0IiXX5Dl7grfEeShANVrM9LVu8KkUwARAQAB +tC5Db2RlciBSZWxlYXNlIFNpZ25pbmcgS2V5IDxzZWN1cml0eUBjb2Rlci5jb20+ +iQJUBBMBCgA+FiEEKMY4lDj2Q3PIwvSKi87Yfbu4ZEsFAmPGrCwCGwMFCQWjmoAF +CwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQi87Yfbu4ZEvrQQ//a3ySdMVhnLP+ +KneonV2zuNilTMC2J/MNG7Q0hU+8I9bxCc6DDqcnBBCQkIUwJq3wmelt3nTC8RxI +fv+ggnbdF9pz7Fc91nIJsGlWpH+bu1tSIvKF/rzZA8v6xUblFFfaC7Gsc5P4xk/+ +h0XBDAy6K+7+AafgLFpRD08Y0Kf2aMcqdM6c2Zo4IPo6FNrOa66FNkypZdQ4IByW +4kMezZSTp4Phqd9yqGC4m44U8YgzmW9LHgrvS0JyIaRPcQFM31AJ50K3iYRxL1ll +ETqJvbDR8UORNQs3Qs3CEZL588BoDMX2TYObTCG6g9Om5vJT0kgUkjDxQHwbAj6E +z9j8BoWkDT2JNzwdfTbPueuRjO+A+TXA9XZtrzbEYEzh0sD9Bdr7ozSF3JAs4GZS +nqcVlyp7q44ZdePR9L8w0ksth56tBWHfE9hi5jbRDRY2OnkV7y7JtWnBDQx9bCIo +7L7aBT8eirI1ZOnUxHJrnqY5matfWjSDBFW+YmWUkjnzBsa9F4m8jq9MSD3Q/8hN +ksJFrmLQs0/8hnM39tS7kLnAaWeGvbmjnxdeMqZsICxNpbyQrq2AhF4GhWfc+NsZ +yznVagJZ9bIlGsycSXJbsA5GbXDnm172TlodMUbLF9FU8i0vV4Y7q6jKO/VsblKU +F0bhXIRqVLrd9g88IyVyyZozmwbJKIy5Ag0EY8asLAEQAMgI9bMurq6Zic4s5W0u +W6LBDHyZhe+w2a3oT/i2YgTsh8XmIjrNasYYWO67b50JKepA3fk3ZA44w8WJqq+z +HLpslEb2fY5I1HvENUMKjYAUIsswSC21DSBau4yYiRGF0MNqv/MWy5Rjc993vIU4 +4TM3mvVhPrYfIkr0jwSbxq8+cm3sBjr0gcBQO57C3w8QkcZ6jefuI7y+1ZeM7X3L +OngmBFJDEutd9LPO/6Is4j/iQfTb8WDR6OmMX3Y04RHrP4sm7jf+3ZZKjcFCZQjr +QA4XHcQyJjnMN34Fn1U7KWopivU+mqViAnVpA643dq9SiBqsl83/R03DrpwKpP7r +6qasUHSUULuS7A4n8+CDwK5KghvrS0hOwMiYoIwZIVPITSUFHPYxrCJK7gU2OHfk +IZHX5m9L5iNwLz958GwzwHuONs5bjMxILbKknRhEBOcbhcpk0jswiPNUrEdipRZY +GR9G9fzD6q4P5heV3kQRqyUUTxdDj8w7jbrwl8sm5zk+TMnPRsu2kg0uwIN1aILm +oVkDN5CiZtg00n2Fu3do5F3YkF0Cz7indx5yySr5iUuoCY0EnpqSwourJ/ZdZA9Y +ZCHjhgjwyPCbxpTGfLj1g25jzQBYn5Wdgr2aHCQcqnU8DKPCnYL9COHJJylgj0vN +NSxyDjNXYYwSrYMqs/91f5xVABEBAAGJAjwEGAEKACYWIQQoxjiUOPZDc8jC9IqL +zth9u7hkSwUCY8asLAIbDAUJBaOagAAKCRCLzth9u7hkSyMvD/0Qal5kwiKDjgBr +i/dtMka+WNBTMb6vKoM759o33YAl22On5WgLr9Uz0cjkJPtzMHxhUo8KQmiPRtsK +dOmG9NI9NttfSeQVbeL8V/DC672fWPKM4TB8X7Kkj56/KI7ueGRokDhXG2pJlhQr +HwzZsAKoCMMnjcquAhHJClK9heIpVLBGFVlmVzJETzxo6fbEU/c7L79+hOrR4BWx +Tg6Dk7mbAGe7BuQLNtw6gcWUVWtHS4iYQtE/4khU1QppC1Z/ZbZ+AJT2TAFXzIaw +0l9tcOh7+TXqsvCLsXN0wrUh1nOdxA81sNWEMY07bG1qgvHyVc7ZYM89/ApK2HP+ +bBDIpAsRCGu2MHtrnJIlNE1J14G1mnauR5qIqI3C0R5MPLXOcDtp+gnjFe+PLU+6 +rQxJObyOkyEpOvtVtJKfFnpI5bqyl8WEPN0rDaS2A27cGXi5nynSAqoM1xT15W21 +uyY2GXY26DIwVfc59wGeclwcM29nS7prRU3KtskjonJ0iQoQebYOHLxy896cK+pK +nnhZx5AQjYiZPsPktSNZjSuOvTZ3g+IDwbCSvmBHcQpitzUOPShTUTs0QjSttzk2 +I6WxP9ivoR9yJGsxwNgCgrYdyt5+hyXXW/aUVihnQwizQRbymjJ2/z+I8NRFIeYb +xbtNFaH3WjLnhm9CB/H+Lc8fUj6HaZkCDQRjxt6QARAAsjZuCMjZBaAC1LFMeRcv +9+Ck7T5UNXTL9xQr1jUFZR95I6loWiWvFJ3Uet7gIbgNYY5Dc1gDr1Oqx9KQBjsN +TUahXov5lmjF5mYeyWTDZ5TS8H3o50zQzfZRC1eEbqjiBMLAHv74KD13P62nvzv6 +Dejwc7Nwc6aOH3cdZm74kz4EmdobJYRVdd5X9EYH/hdM928SsipKhm44oj3RDGi/ +x+ptjW9gr0bnrgCbkyCMNKhnmHSM60I8f4/viRItb+hWRpZYfLxMGTBVunicSXcX +Zh6Fq/DD/yTjzN9N83/NdDvwCyKo5U/kPgD2Ixh5PyJ38cpz6774Awnb/tstCI1g +glnlNbu8Qz84STr3NRZMOgT5h5b5qASOeruG4aVo9euaYJHlnlgcoUmpbEMnwr0L +tREUXSHGXWor7EYPjUQLskIaPl9NCZ3MEw5LhsZTgEdFBnb54dxMSEl7/MYDYhD/ +uTIWOJmtsWHmuMmvfxnw5GDEhJnAp4dxUm9BZlJhfnVR07DtTKyEk37+kl6+i0ZQ +yU4HJ2GWItpLfK54E/CH+S91y7wpepb2TMkaFR2fCK0vXTGAXWK+Y+aTD8ZcLB5y +0IYPsvA0by5AFpmXNfWZiZtYvgJ5FAQZNuB5RILg3HsuDq2U4wzp5BoohWtsOzsn +antIUf/bN0D2g+pCySkc5ssAEQEAAbQuQ29kZXIgUmVsZWFzZSBTaWduaW5nIEtl +eSA8c2VjdXJpdHlAY29kZXIuY29tPokCVAQTAQoAPhYhBCHJaxy5UHGIdPZNvWpa +ZxteQKO5BQJjxt6QAhsDBQkFo5qABQsJCAcCBhUKCQgLAgQWAgMBAh4BAheAAAoJ +EGpaZxteQKO5oysP/1rSdvbKMzozvnVZoglnPjnSGStY9Pr2ziGL7eIMk2yt+Orr +j/AwxYIDgsZPQoJEr87eX2dCYtUMM1x+CpZsWu8dDVFLxyZp8nPmhUzcUCFfutw1 +UmAVKQkOra9segZtw4HVcSctpdgLw7NHq7vIQm4knIvjWmdC15r1B6/VJJI8CeaR +Zy+ToPr9fKnYs1RNdz+DRDN2521skX1DaInhB/ALeid90rJTRujaP9XeyNb9k32K +qd3h4C0KUGIf0fNKj4mmDlNosX3V/pJZATpFiF8aVPlybHQ2W5xpn1U8FJxE4hgR +rvsZmO685Qwm6p/uRI5Eymfm8JC5OQNt9Kvs/BMhotsW0u+je8UXwnznptMILpVP ++qxNuHUe1MYLdjK21LFF+Pk5O4W1TT6mKcbisOmZuQMG5DxpzUwm1Rs5AX1omuJt +iOrmQEvmrKKWC9qbcmWW1t2scnIJsNtrsvME0UjJFz+RL6UUX3xXlLK6YOUghCr8 +gZ7ZPgFqygS6tMu8TAGURzSCfijDh+eZGwqrlvngBIaO5WiNdSXC/J9aE1KThXmX +90A3Gwry+yI2kRS7o8vmghXewPTZbnG0CVHiQIH2yqFNXnhKvhaJt0g04TcnxBte +kiFqRT4K1Bb7pUIlUANmrKo9/zRCxIOopEgRH5cVQ8ZglkT0t5d3ePmAo6h0uQIN +BGPG3pABEADghhNByVoC+qCMo+SErjxz9QYA+tKoAngbgPyxxyB4RD52Z58MwVaP ++Yk0qxJYUBat3dJwiCTlUGG+yTyMOwLl7qSDr53AD5ml0hwJqnLBJ6OUyGE4ax4D +RUVBprKlDltwr98cZDgzvwEhIO2T3tNZ4vySveITj9pLonOrLkAfGXqFOqom+S37 +6eZvjKTnEUbT+S0TTynwds70W31sxVUrL62qsUnmoKEnsKXk/7X8CLXWvtNqu9kf +eiXs5Jz4N6RZUqvS0WOaaWG9v1PHukTtb8RyeookhsBqf9fWOlw5foel+NQwGQjz +0D0dDTKxn2Taweq+gWNCRH7/FJNdWa9upZ2fUAjg9hN9Ow8Y5nE3J0YKCBAQTgNa +XNtsiGQjdEKYZslxZKFM34By3LD6IrkcAEPKu9plZthmqhQumqwYRAgB9O56jg3N +GDDRyAMS7y63nNphTSatpOZtPVVMtcBw5jPjMIPFfU2dlfsvmnCvru2dvfAij+Ng +EkwOLNS8rFQHMJSQysmHuAPSYT97Yl022mPrAtb9+hwtCXt3VI6dvIARl2qPyF0D +DMw2fW5E7ivhUr2WEFiBmXunrJvMIYldBzDkkBjamelPjoevR0wfoIn0x1CbSsQi +zbEs3PXHs7nGxb9TZnHY4+J94mYHdSXrImAuH/x97OnlfUpOKPv5lwARAQABiQI8 +BBgBCgAmFiEEIclrHLlQcYh09k29alpnG15Ao7kFAmPG3pACGwwFCQWjmoAACgkQ +alpnG15Ao7m2/g//Y/YRM+Qhf71G0MJpAfym6ZqmwsT78qQ8T9w95ZeIRD7UUE8d +tm39kqJTGP6DuHCNYEMs2M88o0SoQsS/7j/8is7H/13F5o40DWjuQphia2BWkB1B +G4QRRIXMlrPX8PS92GDCtGfvxn90Li2FhQGZWlNFwvKUB7+/yLMsZzOwo7BS6PwC +hvI3eC7DBC8sXjJUxsrgFAkxQxSx/njP8f4HdUwhNnB1YA2/5IY5bk8QrXxzrAK1 +sbIAjpJdtPYOrZByyyj4ZpRcSm3ngV2n8yd1muJ5u+oRIQoGCdEIaweCj598jNFa +k378ZA11hCyNFHjpPIKnF3tfsQ8vjDatoq4Asy+HXFuo1GA/lvNgNb3Nv4FUozuv +JYJ0KaW73FZXlFBIBkMkRQE8TspHy2v/IGyNXBwKncmkszaiiozBd+T+1NUZgtk5 +9o5uKQwLHVnHIU7r/w/oN5LvLawLg2dP/f2u/KoQXMxjwLZncSH4+5tRz4oa/GMn +k4F84AxTIjGfLJeXigyP6xIPQbvJy+8iLRaCpj+v/EPwAedbRV+u0JFeqqikca70 +aGN86JBOmwpU87sfFxLI7HdI02DkvlxYYK3vYlA6zEyWaeLZ3VNr6tHcQmOnFe8Q +26gcS0AQcxQZrcWTCZ8DJYF+RnXjSVRmHV/3YDts4JyMKcD6QX8s/3aaldk= +=dLmT +-----END PGP PUBLIC KEY BLOCK----- \ No newline at end of file From fbe68deb126f78c3a7ee6c1487ccbaf19a2a9226 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 11 Jul 2025 22:43:14 +0300 Subject: [PATCH 13/30] impl: load the public key from a resource file Initially I thought about embedding it as a const string in the code but the string is too big, best to save it as resource file. The code changes are mostly related to loading the key from the file. --- .../com/coder/toolbox/cli/gpg/GPGVerifier.kt | 47 ++++++++----------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt index c5944f2..629992c 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt @@ -18,6 +18,7 @@ import java.io.ByteArrayInputStream import java.nio.file.Files import java.nio.file.Path + class GPGVerifier( private val context: CoderToolboxContext, ) { @@ -48,23 +49,29 @@ class GPGVerifier( } private fun getCoderPublicKeyRing(): PGPPublicKeyRing { - return try { - getDefaultCoderPublicKeyRing() + try { + val coderPublicKey = javaClass.getResourceAsStream("/META-INF/trusted-keys/pgp-public.key") + ?.readAllBytes() ?: throw IllegalStateException("Trusted public key not found") + return loadPublicKeyRing(coderPublicKey) } catch (e: Exception) { throw PGPException("Failed to load Coder public GPG key", e) } } - private fun getDefaultCoderPublicKeyRing(): PGPPublicKeyRing { - val coderPublicKey = """ - -----BEGIN PGP PUBLIC KEY BLOCK----- - - # Replace this with Coder's actual public key - - -----END PGP PUBLIC KEY BLOCK----- - """.trimIndent() - - return loadPublicKeyRing(coderPublicKey.toByteArray()) + /** + * Load public key ring from bytes + */ + fun loadPublicKeyRing(publicKeyBytes: ByteArray): PGPPublicKeyRing { + return try { + val keyInputStream = ArmoredInputStream(ByteArrayInputStream(publicKeyBytes)) + val keyRingCollection = PGPPublicKeyRingCollection( + PGPUtil.getDecoderStream(keyInputStream), + JcaKeyFingerprintCalculator() + ) + keyRingCollection.keyRings.next() + } catch (e: Exception) { + throw PGPException("Failed to load public key ring", e) + } } /** @@ -103,20 +110,4 @@ class GPGVerifier( return Failed(e) } } - - /** - * Load public key ring from bytes - */ - fun loadPublicKeyRing(publicKeyBytes: ByteArray): PGPPublicKeyRing { - return try { - val keyInputStream = ArmoredInputStream(ByteArrayInputStream(publicKeyBytes)) - val keyRingCollection = PGPPublicKeyRingCollection( - PGPUtil.getDecoderStream(keyInputStream), - JcaKeyFingerprintCalculator() - ) - keyRingCollection.keyRings.next() - } catch (e: Exception) { - throw PGPException("Failed to load public key ring", e) - } - } } \ No newline at end of file From 270b94918148bfb5dede2b8668a0159f98f90608 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 11 Jul 2025 22:58:26 +0300 Subject: [PATCH 14/30] impl: run the signature verification on the IO thread Signature verification some operations that are cpu bound that are light, like decoding and decompressing signature data, some medium CPU intensive operations like the cryptographic verifications and a couple of blocking IO operations: - reading the cli file - reading the signature file - reading the public key file These last should run on the IO thread to not block the main thread from drawing the screen. The cpu bound operation should run on the default thread. --- .../kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt index 629992c..8c8df94 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt @@ -5,6 +5,8 @@ import com.coder.toolbox.cli.gpg.VerificationResult.Failed import com.coder.toolbox.cli.gpg.VerificationResult.Invalid import com.coder.toolbox.cli.gpg.VerificationResult.SignatureNotFound import com.coder.toolbox.cli.gpg.VerificationResult.Valid +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import org.bouncycastle.bcpg.ArmoredInputStream import org.bouncycastle.openpgp.PGPException import org.bouncycastle.openpgp.PGPPublicKeyRing @@ -23,7 +25,7 @@ class GPGVerifier( private val context: CoderToolboxContext, ) { - fun verifySignature( + suspend fun verifySignature( cli: Path, signature: Path, ): VerificationResult { @@ -33,10 +35,13 @@ class GPGVerifier( return SignatureNotFound } - val signatureBytes = Files.readAllBytes(signature) - val cliBytes = Files.readAllBytes(cli) + val (cliBytes, signatureBytes, publicKeyRing) = withContext(Dispatchers.IO) { + val cliBytes = Files.readAllBytes(cli) + val signatureBytes = Files.readAllBytes(signature) + val publicKeyRing = getCoderPublicKeyRing() - val publicKeyRing = getCoderPublicKeyRing() + Triple(cliBytes, signatureBytes, publicKeyRing) + } return verifyDetachedSignature( cliBytes = cliBytes, signatureBytes = signatureBytes, From d5ae2897b3d21ab6e1e12a4389c161e470f7e052 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 11 Jul 2025 23:45:15 +0300 Subject: [PATCH 15/30] fix: find the key id in multiple key rings previous implementation was selecting only the first key ring from the public key file which turns out to be wrong. Instead, we should keep all the key rings and search signature key id in all the key rings. --- .../com/coder/toolbox/cli/gpg/GPGVerifier.kt | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt index 8c8df94..534549e 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt @@ -9,6 +9,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import org.bouncycastle.bcpg.ArmoredInputStream import org.bouncycastle.openpgp.PGPException +import org.bouncycastle.openpgp.PGPPublicKey import org.bouncycastle.openpgp.PGPPublicKeyRing import org.bouncycastle.openpgp.PGPPublicKeyRingCollection import org.bouncycastle.openpgp.PGPSignatureList @@ -20,7 +21,6 @@ import java.io.ByteArrayInputStream import java.nio.file.Files import java.nio.file.Path - class GPGVerifier( private val context: CoderToolboxContext, ) { @@ -38,14 +38,14 @@ class GPGVerifier( val (cliBytes, signatureBytes, publicKeyRing) = withContext(Dispatchers.IO) { val cliBytes = Files.readAllBytes(cli) val signatureBytes = Files.readAllBytes(signature) - val publicKeyRing = getCoderPublicKeyRing() + val publicKeyRing = getCoderPublicKeyRings() Triple(cliBytes, signatureBytes, publicKeyRing) } return verifyDetachedSignature( cliBytes = cliBytes, signatureBytes = signatureBytes, - publicKeyRing = publicKeyRing + publicKeyRings = publicKeyRing ) } catch (e: Exception) { context.logger.error(e, "GPG signature verification failed") @@ -53,27 +53,27 @@ class GPGVerifier( } } - private fun getCoderPublicKeyRing(): PGPPublicKeyRing { + private fun getCoderPublicKeyRings(): List { try { val coderPublicKey = javaClass.getResourceAsStream("/META-INF/trusted-keys/pgp-public.key") ?.readAllBytes() ?: throw IllegalStateException("Trusted public key not found") - return loadPublicKeyRing(coderPublicKey) + return loadPublicKeyRings(coderPublicKey) } catch (e: Exception) { throw PGPException("Failed to load Coder public GPG key", e) } } /** - * Load public key ring from bytes + * Load public key rings from bytes */ - fun loadPublicKeyRing(publicKeyBytes: ByteArray): PGPPublicKeyRing { + fun loadPublicKeyRings(publicKeyBytes: ByteArray): List { return try { val keyInputStream = ArmoredInputStream(ByteArrayInputStream(publicKeyBytes)) val keyRingCollection = PGPPublicKeyRingCollection( PGPUtil.getDecoderStream(keyInputStream), JcaKeyFingerprintCalculator() ) - keyRingCollection.keyRings.next() + keyRingCollection.keyRings.asSequence().toList() } catch (e: Exception) { throw PGPException("Failed to load public key ring", e) } @@ -85,7 +85,7 @@ class GPGVerifier( fun verifyDetachedSignature( cliBytes: ByteArray, signatureBytes: ByteArray, - publicKeyRing: PGPPublicKeyRing + publicKeyRings: List ): VerificationResult { try { val signatureInputStream = ArmoredInputStream(ByteArrayInputStream(signatureBytes)) @@ -98,7 +98,7 @@ class GPGVerifier( } val signature = signatureList[0] - val publicKey = publicKeyRing.getPublicKey(signature.keyID) + val publicKey = findPublicKey(publicKeyRings, signature.keyID) ?: throw PGPException("Public key not found for signature") signature.init(JcaPGPContentVerifierBuilderProvider(), publicKey) @@ -115,4 +115,17 @@ class GPGVerifier( return Failed(e) } } + + /** + * Find a public key across all key rings in the collection + */ + private fun findPublicKey( + keyRings: List, + keyId: Long + ): PGPPublicKey? { + keyRings.forEach { keyRing -> + keyRing.getPublicKey(keyId)?.let { return it } + } + return null + } } \ No newline at end of file From 96663e647a070f6402d65729f42a56f87f307880 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Sat, 12 Jul 2025 00:06:33 +0300 Subject: [PATCH 16/30] fix: remove the cli if it is not properly signed Otherwise, at the next Toolbox restart the signature will no longer be verified, and we run into the risk of running unsigned binaries. --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index f696ead..53c612a 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -213,16 +213,36 @@ class CoderCLIManager( gpgVerifier.verifySignature(cliResult.dst, signatureResult.dst).let { result -> when { result.isValid() -> return true - result.isInvalid() -> { - val reason = (result as Invalid).reason - throw UnsignedBinaryExecutionDeniedException( - "Signature of ${cliResult.dst} is invalid." + reason?.let { " Reason: $it" }.orEmpty() - ) + else -> { + // remove the cli, otherwise next time the user tries to login the cached cli is picked up, + // and we don't verify cached cli signatures + runCatching { Files.delete(cliResult.dst) } + .onFailure { ex -> + context.logger.warn(ex, "Failed to delete CLI file: ${cliResult.dst}") + } + + val exception = when { + result.isInvalid() -> { + val reason = (result as Invalid).reason + UnsignedBinaryExecutionDeniedException( + "Signature of ${cliResult.dst} is invalid." + reason?.let { " Reason: $it" }.orEmpty() + ) + } + + result.signatureIsNotFound() -> { + UnsignedBinaryExecutionDeniedException( + "Can't verify signature of ${cliResult.dst} because ${signatureResult.dst} does not exist" + ) + } + + else -> { + UnsignedBinaryExecutionDeniedException((result as Failed).error.message) + } + } + throw exception } - - result.signatureIsNotFound() -> throw UnsignedBinaryExecutionDeniedException("Can't verify signature of ${cliResult.dst} because ${signatureResult.dst} does not exist") - else -> throw UnsignedBinaryExecutionDeniedException((result as Failed).error.message) } + } } From 6a799954bde172a22fc65141041943ff8ce3e65b Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Sat, 12 Jul 2025 00:28:10 +0300 Subject: [PATCH 17/30] fix: avoid out of memory when verifying signatures `Files.readAllBytes()` uses direct buffers internally which can be filled up quickly when called repeatedly in coroutines, as these buffers are not released quickly by the GC. The scenario can be reproduced by trying to login a couple of times one after the other with signature verification failing each time. Instead, we can avoid memory issues by streaming the cli and feed only blocks of bytes into the signature calculation. --- .../com/coder/toolbox/cli/gpg/GPGVerifier.kt | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt index 534549e..490b48e 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/gpg/GPGVerifier.kt @@ -20,6 +20,7 @@ import org.bouncycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProv import java.io.ByteArrayInputStream import java.nio.file.Files import java.nio.file.Path +import kotlin.io.path.inputStream class GPGVerifier( private val context: CoderToolboxContext, @@ -35,15 +36,14 @@ class GPGVerifier( return SignatureNotFound } - val (cliBytes, signatureBytes, publicKeyRing) = withContext(Dispatchers.IO) { - val cliBytes = Files.readAllBytes(cli) + val (signatureBytes, publicKeyRing) = withContext(Dispatchers.IO) { val signatureBytes = Files.readAllBytes(signature) val publicKeyRing = getCoderPublicKeyRings() - Triple(cliBytes, signatureBytes, publicKeyRing) + Pair(signatureBytes, publicKeyRing) } return verifyDetachedSignature( - cliBytes = cliBytes, + cliPath = cli, signatureBytes = signatureBytes, publicKeyRings = publicKeyRing ) @@ -83,7 +83,7 @@ class GPGVerifier( * Verify a detached GPG signature */ fun verifyDetachedSignature( - cliBytes: ByteArray, + cliPath: Path, signatureBytes: ByteArray, publicKeyRings: List ): VerificationResult { @@ -102,7 +102,13 @@ class GPGVerifier( ?: throw PGPException("Public key not found for signature") signature.init(JcaPGPContentVerifierBuilderProvider(), publicKey) - signature.update(cliBytes) + cliPath.inputStream().use { fileStream -> + val buffer = ByteArray(8192) + var bytesRead: Int + while (fileStream.read(buffer).also { bytesRead = it } != -1) { + signature.update(buffer, 0, bytesRead) + } + } val isValid = signature.verify() context.logger.info("GPG signature verification result: $isValid") From 5fcb4b9dff3f4cf41e919b33ce7aed024be6009f Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Sat, 12 Jul 2025 01:22:46 +0300 Subject: [PATCH 18/30] fix: don't run signature verification When there is no signature and the user allowed running of unsigned binaries without prompt --- src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 53c612a..de367bb 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -189,6 +189,7 @@ class CoderCLIManager( if (signatureResult.isNotDownloaded()) { if (context.settingsStore.allowUnsignedBinaryWithoutPrompt) { context.logger.warn("Running unsigned CLI from ${cliResult.source}") + return true } else { val acceptsUnsignedBinary = context.ui.showYesNoPopup( context.i18n.ptrl("Security Warning"), @@ -242,7 +243,6 @@ class CoderCLIManager( throw exception } } - } } From 35433771d269c764742e1fae774cea322fed2d1f Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Sat, 12 Jul 2025 01:23:07 +0300 Subject: [PATCH 19/30] chore: fix UTs --- .../coder/toolbox/cli/CoderCLIManagerTest.kt | 237 ++++++++++-------- 1 file changed, 128 insertions(+), 109 deletions(-) diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index 5c37c9e..65aa57b 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -7,6 +7,7 @@ import com.coder.toolbox.cli.ex.SSHConfigFormatException import com.coder.toolbox.sdk.DataGen.Companion.workspace import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.settings.Environment +import com.coder.toolbox.store.ALLOW_UNSIGNED_BINARY_EXEC import com.coder.toolbox.store.BINARY_DIRECTORY import com.coder.toolbox.store.BINARY_NAME import com.coder.toolbox.store.BINARY_SOURCE @@ -43,6 +44,7 @@ import com.squareup.moshi.JsonEncodingException import com.sun.net.httpserver.HttpServer import io.mockk.mockk import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.runBlocking import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.assertDoesNotThrow import org.zeroturnaround.exec.InvalidExitValueException @@ -67,7 +69,7 @@ private val noOpTextProgress: (String) -> Unit = { _ -> } internal class CoderCLIManagerTest { private val context = CoderToolboxContext( - mockk(), + mockk(relaxed = true), mockk(), mockk(), mockk(), @@ -75,7 +77,7 @@ internal class CoderCLIManagerTest { mockk(), mockk(), mockk(relaxed = true), - mockk(), + mockk(relaxed = true), CoderSettingsStore( pluginTestSettingsStore(), Environment(), @@ -112,6 +114,9 @@ internal class CoderCLIManagerTest { if (exchange.requestURI.path == "/bin/override") { code = HttpURLConnection.HTTP_OK response = mkbinVersion("0.0.0") + } else if (exchange.requestURI.path.contains(".asc")) { + code = HttpURLConnection.HTTP_NOT_FOUND + response = "not found" } else if (!exchange.requestURI.path.startsWith("/bin/coder-")) { code = HttpURLConnection.HTTP_NOT_FOUND response = "not found" @@ -136,19 +141,14 @@ internal class CoderCLIManagerTest { fun testServerInternalError() { val (srv, url) = mockServer(HttpURLConnection.HTTP_INTERNAL_ERROR) val ccm = CoderCLIManager( - url, - context.logger, - CoderSettingsStore( - pluginTestSettingsStore(), - Environment(), - mockk(relaxed = true) - ).readOnly() + context, + url ) val ex = assertFailsWith( exceptionClass = ResponseException::class, - block = { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }, + block = { runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) } } ) assertEquals(HttpURLConnection.HTTP_INTERNAL_ERROR, ex.code) @@ -164,16 +164,16 @@ internal class CoderCLIManagerTest { ), Environment(), context.logger - ).readOnly() + ) val url = URL("http://localhost") - val ccm1 = CoderCLIManager(url, context.logger, settings) + val ccm1 = CoderCLIManager(context.copy(settingsStore = settings), url) assertEquals(settings.binSource(url), ccm1.remoteBinaryURL) assertEquals(settings.dataDir(url), ccm1.coderConfigPath.parent) assertEquals(settings.binPath(url), ccm1.localBinaryPath) // Can force using data directory. - val ccm2 = CoderCLIManager(url, context.logger, settings, true) + val ccm2 = CoderCLIManager(context.copy(settingsStore = settings), url, true) assertEquals(settings.binSource(url), ccm2.remoteBinaryURL) assertEquals(settings.dataDir(url), ccm2.coderConfigPath.parent) assertEquals(settings.binPath(url, true), ccm2.localBinaryPath) @@ -187,15 +187,16 @@ internal class CoderCLIManagerTest { val (srv, url) = mockServer() val ccm = CoderCLIManager( - url, - context.logger, - CoderSettingsStore( + context.copy( + settingsStore = CoderSettingsStore( pluginTestSettingsStore( DATA_DIRECTORY to tmpdir.resolve("cli-dir-fail-to-write").toString(), ), Environment(), context.logger - ).readOnly(), + ) + ), + url ) ccm.localBinaryPath.parent.toFile().mkdirs() @@ -203,7 +204,7 @@ internal class CoderCLIManagerTest { assertFailsWith( exceptionClass = AccessDeniedException::class, - block = { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }, + block = { runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) } }, ) srv.stop(0) @@ -221,22 +222,23 @@ internal class CoderCLIManagerTest { } val ccm = CoderCLIManager( + context.copy( + settingsStore = CoderSettingsStore( + pluginTestSettingsStore( + DATA_DIRECTORY to tmpdir.resolve("real-cli").toString(), + ), + Environment(), + context.logger + ) + ), url.toURL(), - context.logger, - CoderSettingsStore( - pluginTestSettingsStore( - DATA_DIRECTORY to tmpdir.resolve("real-cli").toString(), - ), - Environment(), - context.logger - ).readOnly(), ) - assertTrue(ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress)) + assertTrue(runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertDoesNotThrow { ccm.version() } // It should skip the second attempt. - assertFalse(ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress)) + assertFalse(runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) // Make sure login failures propagate. assertFailsWith( @@ -249,39 +251,43 @@ internal class CoderCLIManagerTest { fun testDownloadMockCLI() { val (srv, url) = mockServer() var ccm = CoderCLIManager( + context.copy( + settingsStore = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_NAME to "coder.bat", + DATA_DIRECTORY to tmpdir.resolve("mock-cli").toString(), + ALLOW_UNSIGNED_BINARY_EXEC to "true", + ), + Environment(), + context.logger, + ) + ), url, - context.logger, - CoderSettingsStore( - pluginTestSettingsStore( - BINARY_NAME to "coder.bat", - DATA_DIRECTORY to tmpdir.resolve("mock-cli").toString(), - ), - Environment(), - context.logger, - ).readOnly(), ) - assertEquals(true, ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress)) + assertEquals(true, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) // It should skip the second attempt. - assertEquals(false, ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress)) + assertEquals(false, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) // Should use the source override. ccm = CoderCLIManager( + context.copy( + settingsStore = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_SOURCE to "/bin/override", + DATA_DIRECTORY to tmpdir.resolve("mock-cli").toString(), + ALLOW_UNSIGNED_BINARY_EXEC to "true", + ), + Environment(), + context.logger + ) + ), url, - context.logger, - CoderSettingsStore( - pluginTestSettingsStore( - BINARY_SOURCE to "/bin/override", - DATA_DIRECTORY to tmpdir.resolve("mock-cli").toString(), - ), - Environment(), - context.logger - ).readOnly(), ) - assertEquals(true, ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress)) + assertEquals(true, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertContains(ccm.localBinaryPath.toFile().readText(), "0.0.0") srv.stop(0) @@ -290,15 +296,16 @@ internal class CoderCLIManagerTest { @Test fun testRunNonExistentBinary() { val ccm = CoderCLIManager( - URL("https://foo"), - context.logger, - CoderSettingsStore( + context.copy( + settingsStore = CoderSettingsStore( pluginTestSettingsStore( DATA_DIRECTORY to tmpdir.resolve("does-not-exist").toString(), ), Environment(), context.logger - ).readOnly(), + ) + ), + URL("https://foo") ) assertFailsWith( @@ -311,15 +318,17 @@ internal class CoderCLIManagerTest { fun testOverwritesWrongVersion() { val (srv, url) = mockServer() val ccm = CoderCLIManager( - url, - context.logger, - CoderSettingsStore( + context.copy( + settingsStore = CoderSettingsStore( pluginTestSettingsStore( + ALLOW_UNSIGNED_BINARY_EXEC to "true", DATA_DIRECTORY to tmpdir.resolve("overwrite-cli").toString(), ), Environment(), context.logger - ).readOnly(), + ) + ), + url ) ccm.localBinaryPath.parent.toFile().mkdirs() @@ -329,7 +338,7 @@ internal class CoderCLIManagerTest { assertEquals("cli", ccm.localBinaryPath.toFile().readText()) assertEquals(0, ccm.localBinaryPath.toFile().lastModified()) - assertTrue(ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress)) + assertTrue(runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertNotEquals("cli", ccm.localBinaryPath.toFile().readText()) assertNotEquals(0, ccm.localBinaryPath.toFile().lastModified()) @@ -346,16 +355,17 @@ internal class CoderCLIManagerTest { val settings = CoderSettingsStore( pluginTestSettingsStore( DATA_DIRECTORY to tmpdir.resolve("clobber-cli").toString(), + ALLOW_UNSIGNED_BINARY_EXEC to "true" ), Environment(), context.logger - ).readOnly() + ) - val ccm1 = CoderCLIManager(url1, context.logger, settings) - val ccm2 = CoderCLIManager(url2, context.logger, settings) + val ccm1 = CoderCLIManager(context.copy(settingsStore = settings), url1) + val ccm2 = CoderCLIManager(context.copy(settingsStore = settings), url2) - assertTrue(ccm1.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress)) - assertTrue(ccm2.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress)) + assertTrue(runBlocking { ccm1.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) + assertTrue(runBlocking { ccm2.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) srv1.stop(0) srv2.stop(0) @@ -525,10 +535,13 @@ internal class CoderCLIManagerTest { ), env = it.env, context.logger, - ).readOnly() + ) val ccm = - CoderCLIManager(it.url ?: URI.create("https://test.coder.invalid").toURL(), context.logger, settings) + CoderCLIManager( + context.copy(settingsStore = settings), + it.url ?: URI.create("https://test.coder.invalid").toURL() + ) val sshConfigPath = Path.of(settings.sshConfigPath) // Input is the configuration that we start with, if any. @@ -609,7 +622,7 @@ internal class CoderCLIManagerTest { ), Environment(), context.logger - ).readOnly() + ) val sshConfigPath = Path.of(settings.sshConfigPath) sshConfigPath.parent.toFile().mkdirs() Path.of("src/test/resources/fixtures/inputs").resolve("$it.conf").toFile().copyTo( @@ -617,7 +630,7 @@ internal class CoderCLIManagerTest { true, ) - val ccm = CoderCLIManager(URL("https://test.coder.invalid"), context.logger, settings) + val ccm = CoderCLIManager(context.copy(settingsStore = settings), URL("https://test.coder.invalid")) assertFailsWith( exceptionClass = SSHConfigFormatException::class, @@ -644,15 +657,16 @@ internal class CoderCLIManagerTest { tests.forEach { val ccm = CoderCLIManager( + context.copy( + settingsStore = CoderSettingsStore( + pluginTestSettingsStore( + HEADER_COMMAND to it, + ), + Environment(), + context.logger + ) + ), URI.create("https://test.coder.invalid").toURL(), - context.logger, - CoderSettingsStore( - pluginTestSettingsStore( - HEADER_COMMAND to it, - ), - Environment(), - context.logger - ).readOnly(), ) assertFailsWith( @@ -695,16 +709,17 @@ internal class CoderCLIManagerTest { ) val ccm = CoderCLIManager( + context.copy( + settingsStore = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_NAME to "coder.bat", + BINARY_DIRECTORY to tmpdir.resolve("bad-version").toString(), + ), + Environment(), + context.logger, + ) + ), URL("https://test.coder.parse-fail.invalid"), - context.logger, - CoderSettingsStore( - pluginTestSettingsStore( - BINARY_NAME to "coder.bat", - BINARY_DIRECTORY to tmpdir.resolve("bad-version").toString(), - ), - Environment(), - context.logger, - ).readOnly(), ) ccm.localBinaryPath.parent.toFile().mkdirs() @@ -748,16 +763,17 @@ internal class CoderCLIManagerTest { ) val ccm = CoderCLIManager( + context.copy( + settingsStore = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_NAME to "coder.bat", + BINARY_DIRECTORY to tmpdir.resolve("matches-version").toString(), + ), + Environment(), + context.logger, + ) + ), URL("https://test.coder.matches-version.invalid"), - context.logger, - CoderSettingsStore( - pluginTestSettingsStore( - BINARY_NAME to "coder.bat", - BINARY_DIRECTORY to tmpdir.resolve("matches-version").toString(), - ), - Environment(), - context.logger, - ).readOnly(), ) ccm.localBinaryPath.parent.toFile().mkdirs() @@ -852,6 +868,7 @@ internal class CoderCLIManagerTest { ENABLE_BINARY_DIR_FALLBACK to it.enableFallback.toString(), DATA_DIRECTORY to tmpdir.resolve("ensure-data-dir").toString(), BINARY_DIRECTORY to tmpdir.resolve("ensure-bin-dir").toString(), + ALLOW_UNSIGNED_BINARY_EXEC to "true" ), Environment(), context.logger @@ -886,12 +903,12 @@ internal class CoderCLIManagerTest { Result.ERROR -> { assertFailsWith( exceptionClass = AccessDeniedException::class, - block = { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) }, + block = { runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } } ) } Result.NONE -> { - val ccm = ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) + val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } assertEquals(settings.binPath(url), ccm.localBinaryPath) assertFailsWith( exceptionClass = ProcessInitException::class, @@ -900,25 +917,25 @@ internal class CoderCLIManagerTest { } Result.DL_BIN -> { - val ccm = ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) + val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } assertEquals(settings.binPath(url), ccm.localBinaryPath) assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) } Result.DL_DATA -> { - val ccm = ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) + val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } assertEquals(settings.binPath(url, true), ccm.localBinaryPath) assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) } Result.USE_BIN -> { - val ccm = ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) + val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } assertEquals(settings.binPath(url), ccm.localBinaryPath) assertEquals(SemVer.parse(it.version ?: ""), ccm.version()) } Result.USE_DATA -> { - val ccm = ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) + val ccm = runBlocking { ensureCLI(localContext, url, it.buildVersion, noOpTextProgress) } assertEquals(settings.binPath(url, true), ccm.localBinaryPath) assertEquals(SemVer.parse(it.fallbackVersion ?: ""), ccm.version()) } @@ -947,18 +964,20 @@ internal class CoderCLIManagerTest { tests.forEach { val (srv, url) = mockServer(version = it.first) val ccm = CoderCLIManager( + context.copy( + settingsStore = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_NAME to "coder.bat", + DATA_DIRECTORY to tmpdir.resolve("features").toString(), + ALLOW_UNSIGNED_BINARY_EXEC to "true" + ), + Environment(), + context.logger, + ) + ), url, - context.logger, - CoderSettingsStore( - pluginTestSettingsStore( - BINARY_NAME to "coder.bat", - DATA_DIRECTORY to tmpdir.resolve("features").toString(), - ), - Environment(), - context.logger, - ).readOnly(), ) - assertEquals(true, ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress)) + assertEquals(true, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertEquals(it.second, ccm.features, "version: ${it.first}") srv.stop(0) From 97dbc8d2b69150330842e6d27a83460e345c5961 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Sat, 12 Jul 2025 01:25:45 +0300 Subject: [PATCH 20/30] chore: next version is 0.5.0 A major feature was added --- CHANGELOG.md | 1 + gradle.properties | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a1b072..69ccd25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Added - support for matching workspace agent in the URI via the agent name +- support for checking if CLI is signed ### Removed diff --git a/gradle.properties b/gradle.properties index efbc54f..9513b30 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ -version=0.4.0 +version=0.5.0 group=com.coder.toolbox name=coder-toolbox From 27066d81e628ff9b175d054157f7e8850adb496b Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Sat, 12 Jul 2025 01:36:42 +0300 Subject: [PATCH 21/30] fix: more UTs --- src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index 65aa57b..0ec4945 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -226,6 +226,7 @@ internal class CoderCLIManagerTest { settingsStore = CoderSettingsStore( pluginTestSettingsStore( DATA_DIRECTORY to tmpdir.resolve("real-cli").toString(), + ALLOW_UNSIGNED_BINARY_EXEC to "true", ), Environment(), context.logger From 811fc850401333eed8989e5145dc35661ae9fe58 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Mon, 14 Jul 2025 23:28:03 +0300 Subject: [PATCH 22/30] fix: display errors that happened while handling URIs And also disable the work in progress animations after a fatal error occurred. Most exceptions, especially the ones related to cli signature verification were suppressed and only displayed in the logs with no visual feedback. --- .../com/coder/toolbox/CoderRemoteProvider.kt | 59 +++++++++++++------ 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt index be4c40a..3e3172a 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt @@ -35,6 +35,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.selects.onTimeout import kotlinx.coroutines.selects.select import java.net.URI +import java.util.UUID import kotlin.coroutines.cancellation.CancellationException import kotlin.time.Duration.Companion.seconds import kotlin.time.TimeSource @@ -302,31 +303,51 @@ class CoderRemoteProvider( * Handle incoming links (like from the dashboard). */ override suspend fun handleUri(uri: URI) { - linkHandler.handle( - uri, shouldDoAutoSetup(), - { - coderHeaderPage.isBusyCreatingNewEnvironment.update { - true + try { + linkHandler.handle( + uri, shouldDoAutoSetup(), + { + coderHeaderPage.isBusyCreatingNewEnvironment.update { + true + } + }, + { + coderHeaderPage.isBusyCreatingNewEnvironment.update { + false + } } - }, - { - coderHeaderPage.isBusyCreatingNewEnvironment.update { + ) { restClient, cli -> + // stop polling and de-initialize resources + close() + isInitialized.update { false } + // start initialization with the new settings + this@CoderRemoteProvider.client = restClient + coderHeaderPage.setTitle(context.i18n.pnotr(restClient.url.toString())) + + environments.showLoadingMessage() + pollJob = poll(restClient, cli) + isInitialized.waitForTrue() } - ) { restClient, cli -> - // stop polling and de-initialize resources - close() - isInitialized.update { + } catch (ex: Exception) { + context.logger.error(ex, "") + val textError = if (ex is APIResponseException) { + if (!ex.reason.isNullOrBlank()) { + ex.reason + } else ex.message + } else ex.message + + context.ui.showSnackbar( + UUID.randomUUID().toString(), + context.i18n.ptrl("Error encountered while handling Coder URI"), + context.i18n.pnotr(textError ?: ""), + context.i18n.ptrl("Dismiss") + ) + } finally { + coderHeaderPage.isBusyCreatingNewEnvironment.update { false } - // start initialization with the new settings - this@CoderRemoteProvider.client = restClient - coderHeaderPage.setTitle(context.i18n.pnotr(restClient.url.toString())) - - environments.showLoadingMessage() - pollJob = poll(restClient, cli) - isInitialized.waitForTrue() } } From 9851decab01925d90b268cdeed5b5e7b77a46786 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Mon, 14 Jul 2025 23:58:38 +0300 Subject: [PATCH 23/30] impl: check if the cli exists before running it to spill out the version The logic for matching the local CLI version with the deployment previously attempted to run the CLI with --version without first verifying that the binary existed. This commit improves that by first checking if the file exists, avoiding the unnecessary overhead of spawning a process for a non-existent binary. --- src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index de367bb..789105c 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -489,6 +489,7 @@ class CoderCLIManager( * version could not be parsed. */ fun matchesVersion(rawBuildVersion: String): Boolean? { + if (Files.notExists(localBinaryPath)) return null val cliVersion = tryVersion() ?: return null val buildVersion = try { From 306848f873c1bddcf72b2dd96aa541b83e258127 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 15 Jul 2025 00:39:42 +0300 Subject: [PATCH 24/30] impl: download retroactive cli signatures from releases.coder.com/coder-cli Retroactive cli signatures are now published at releases.coder.com/coder-cli/x.y.z/ where x.y.z is the major, minor and patch version of the deployment. --- src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt | 2 +- .../coder/toolbox/cli/downloader/CoderDownloadService.kt | 9 +++++++-- src/main/kotlin/com/coder/toolbox/util/SemVer.kt | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 789105c..06d748c 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -180,7 +180,7 @@ class CoderCLIManager( if (signatureResult.isNotDownloaded()) { context.logger.info("Trying to download signature file from releases.coder.com") signatureResult = withContext(Dispatchers.IO) { - downloader.downloadReleasesSignature(showTextProgress) + downloader.downloadReleasesSignature(buildVersion, showTextProgress) } } diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt index d28c5e6..63ca9aa 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt @@ -3,6 +3,7 @@ package com.coder.toolbox.cli.downloader import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.cli.ex.ResponseException import com.coder.toolbox.util.OS +import com.coder.toolbox.util.SemVer import com.coder.toolbox.util.getHeaders import com.coder.toolbox.util.getOS import com.coder.toolbox.util.sha1 @@ -188,7 +189,11 @@ class CoderDownloadService( } - suspend fun downloadReleasesSignature(showTextProgress: (String) -> Unit): DownloadResult { - return downloadSignature(URI.create("https://releases.coder.com/bin").toURL(), showTextProgress) + suspend fun downloadReleasesSignature(buildVersion: String, showTextProgress: (String) -> Unit): DownloadResult { + val semVer = SemVer.parse(buildVersion) + return downloadSignature( + URI.create("https://releases.coder.com/coder-cli/${semVer.major}.${semVer.minor}.${semVer.patch}/").toURL(), + showTextProgress + ) } } \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/util/SemVer.kt b/src/main/kotlin/com/coder/toolbox/util/SemVer.kt index 238ce81..a40a9a9 100644 --- a/src/main/kotlin/com/coder/toolbox/util/SemVer.kt +++ b/src/main/kotlin/com/coder/toolbox/util/SemVer.kt @@ -1,6 +1,6 @@ package com.coder.toolbox.util -class SemVer(private val major: Long = 0, private val minor: Long = 0, private val patch: Long = 0) : Comparable { +class SemVer(val major: Long = 0, val minor: Long = 0, val patch: Long = 0) : Comparable { init { require(major >= 0) { "Coder major version must be a positive number" } require(minor >= 0) { "Coder minor version must be a positive number" } From 5dcdff05e9e88f196ccd950838efe6b740aa096d Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 15 Jul 2025 01:10:15 +0300 Subject: [PATCH 25/30] fix: UTs after fallback to signatures from releases.coder.com were published We now have a lot of signatures published for a lot of older cli version which means some of the tests that were not expecting the fallback to activate are now in trouble. The simple fix is to "download" a very old version for which signatures will not be generated. --- src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index 0ec4945..518bdde 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -64,7 +64,7 @@ import kotlin.test.assertFalse import kotlin.test.assertNotEquals import kotlin.test.assertTrue -private const val VERSION_FOR_PROGRESS_REPORTING = "v2.23.1-devel+de07351b8" +private const val VERSION_FOR_PROGRESS_REPORTING = "v2.13.1-devel+de07351b8" private val noOpTextProgress: (String) -> Unit = { _ -> } internal class CoderCLIManagerTest { From 5bf07926f78a23be5dded32d2fd9904ad7a236fd Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 15 Jul 2025 01:44:58 +0300 Subject: [PATCH 26/30] chore: refactor code around signature name Instead of deriving the signature name from the cli name it is now coded into settings store just like the default cli name --- .../cli/downloader/CoderDownloadService.kt | 7 +- .../toolbox/settings/ReadOnlyCoderSettings.kt | 5 + .../coder/toolbox/store/CoderSettingsStore.kt | 67 ++++++++----- src/main/kotlin/com/coder/toolbox/util/OS.kt | 22 ++--- .../toolbox/store/CoderSettingsStoreTest.kt | 93 +++++++++++++++++++ 5 files changed, 149 insertions(+), 45 deletions(-) create mode 100644 src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt index 63ca9aa..d3b49da 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt @@ -154,11 +154,8 @@ class CoderDownloadService( } private suspend fun downloadSignature(url: URL, showTextProgress: (String) -> Unit): DownloadResult { - val defaultCliNameWithoutExt = context.settingsStore.defaultCliBinaryNameByOsAndArch.split('.').first() - val signatureName = "$defaultCliNameWithoutExt.asc" - - val signatureURL = url.withLastSegment(signatureName) - val localSignaturePath = localBinaryPath.parent.resolve(signatureName) + val signatureURL = url.withLastSegment(context.settingsStore.defaultSignatureNameByOsAndArch) + val localSignaturePath = localBinaryPath.parent.resolve(context.settingsStore.defaultSignatureNameByOsAndArch) context.logger.info("Downloading signature from $signatureURL") val response = downloadApi.downloadSignature( diff --git a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt index d3dd17d..35b56bf 100644 --- a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt +++ b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt @@ -43,6 +43,11 @@ interface ReadOnlyCoderSettings { */ val binaryName: String + /** + * Default CLI signature name based on OS and architecture + */ + val defaultSignatureNameByOsAndArch: String + /** * Where to save plugin data like the Coder binary (if not configured with * binaryDirectory) and the deployment URL and session token. diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index 308e762..313480f 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -41,6 +41,7 @@ class CoderSettingsStore( get() = store[ALLOW_UNSIGNED_BINARY_EXEC]?.toBooleanStrictOrNull() ?: false override val defaultCliBinaryNameByOsAndArch: String get() = getCoderCLIForOS(getOS(), getArch()) override val binaryName: String get() = store[BINARY_NAME] ?: getCoderCLIForOS(getOS(), getArch()) + override val defaultSignatureNameByOsAndArch: String get() = getCoderSignatureForOS(getOS(), getArch()) override val dataDirectory: String? get() = store[DATA_DIRECTORY] override val globalDataDirectory: String get() = getDefaultGlobalDataDir().normalize().toString() override val globalConfigDir: String get() = getDefaultGlobalConfigDir().normalize().toString() @@ -243,41 +244,59 @@ class CoderSettingsStore( } /** - * Return the name of the binary (with extension) for the provided OS and - * architecture. + * Return the name of the binary (with extension) for the provided OS and architecture. */ private fun getCoderCLIForOS( os: OS?, arch: Arch?, ): String { - logger.info("Resolving binary for $os $arch") + logger.debug("Resolving binary for $os $arch") + return buildCoderFileName(os, arch) + } + + /** + * Return the name of the signature file (.asc) for the provided OS and architecture. + */ + private fun getCoderSignatureForOS( + os: OS?, + arch: Arch?, + ): String { + logger.debug("Resolving signature for $os $arch") + return buildCoderFileName(os, arch, true) + } + + /** + * Build the coder file name based on OS, architecture, and whether it's a signature file. + */ + private fun buildCoderFileName( + os: OS?, + arch: Arch?, + isSignature: Boolean = false + ): String { if (os == null) { logger.error("Could not resolve client OS and architecture, defaulting to WINDOWS AMD64") - return "coder-windows-amd64.exe" + return if (isSignature) "coder-windows-amd64.asc" else "coder-windows-amd64.exe" } - return when (os) { - OS.WINDOWS -> - when (arch) { - Arch.AMD64 -> "coder-windows-amd64.exe" - Arch.ARM64 -> "coder-windows-arm64.exe" - else -> "coder-windows-amd64.exe" - } - OS.LINUX -> - when (arch) { - Arch.AMD64 -> "coder-linux-amd64" - Arch.ARM64 -> "coder-linux-arm64" - Arch.ARMV7 -> "coder-linux-armv7" - else -> "coder-linux-amd64" - } + val osName = when (os) { + OS.WINDOWS -> "windows" + OS.LINUX -> "linux" + OS.MAC -> "darwin" + } - OS.MAC -> - when (arch) { - Arch.AMD64 -> "coder-darwin-amd64" - Arch.ARM64 -> "coder-darwin-arm64" - else -> "coder-darwin-amd64" - } + val archName = when (arch) { + Arch.AMD64 -> "amd64" + Arch.ARM64 -> "arm64" + Arch.ARMV7 -> "armv7" + else -> "amd64" // default fallback } + + val extension = if (isSignature) ".asc" else when (os) { + OS.WINDOWS -> ".exe" + OS.LINUX, OS.MAC -> "" + } + + return "coder-$osName-$archName$extension" } /** diff --git a/src/main/kotlin/com/coder/toolbox/util/OS.kt b/src/main/kotlin/com/coder/toolbox/util/OS.kt index 32abd5e..ba39204 100644 --- a/src/main/kotlin/com/coder/toolbox/util/OS.kt +++ b/src/main/kotlin/com/coder/toolbox/util/OS.kt @@ -1,30 +1,19 @@ package com.coder.toolbox.util -import java.util.* +import java.util.Locale fun getOS(): OS? = OS.from(System.getProperty("os.name")) -fun getArch(): Arch? = Arch.from(System.getProperty("os.arch").lowercase(Locale.getDefault())) +fun getArch(): Arch? = Arch.from(System.getProperty("os.arch")?.lowercase(Locale.getDefault())) enum class OS { WINDOWS, LINUX, MAC; - /** - * The name of the current desktop environment. - * For Linux systems it can be GNOME, KDE, XFCE, LXDE, and so on, - * while for macOS it will be Aqua and Windows Shell for Windows. - */ - fun getDesktopEnvironment(): String? = - when (this) { - WINDOWS -> "Windows Shell" - MAC -> "Aqua" - LINUX -> System.getenv("XDG_CURRENT_DESKTOP") - } - companion object { - fun from(os: String): OS? = when { + fun from(os: String?): OS? = when { + os.isNullOrBlank() -> null os.contains("win", true) -> { WINDOWS } @@ -49,7 +38,8 @@ enum class Arch { ; companion object { - fun from(arch: String): Arch? = when { + fun from(arch: String?): Arch? = when { + arch.isNullOrBlank() -> null arch.contains("amd64", true) || arch.contains("x86_64", true) -> AMD64 arch.contains("arm64", true) || arch.contains("aarch64", true) -> ARM64 arch.contains("armv7", true) -> ARMV7 diff --git a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt new file mode 100644 index 0000000..5798524 --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt @@ -0,0 +1,93 @@ +package com.coder.toolbox.store + +import com.coder.toolbox.settings.Environment +import com.coder.toolbox.util.pluginTestSettingsStore +import com.jetbrains.toolbox.api.core.diagnostics.Logger +import io.mockk.mockk +import org.junit.jupiter.api.Assertions.assertEquals +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test + +class CoderSettingsStoreTest { + private var originalOsName: String? = null + private var originalOsArch: String? = null + + private lateinit var store: CoderSettingsStore + + @BeforeTest + fun setUp() { + originalOsName = System.getProperty("os.name") + originalOsArch = System.getProperty("os.arch") + + store = CoderSettingsStore( + pluginTestSettingsStore(), + Environment(), + mockk(relaxed = true) + ) + } + + @AfterTest + fun tearDown() { + System.setProperty("os.name", originalOsName) + System.setProperty("os.arch", originalOsArch) + } + + @Test + fun `Default CLI and signature for Windows AMD64`() = + assertBinaryAndSignature("Windows 10", "amd64", "coder-windows-amd64.exe", "coder-windows-amd64.asc") + + @Test + fun `Default CLI and signature for Windows ARM64`() = + assertBinaryAndSignature("Windows 10", "aarch64", "coder-windows-arm64.exe", "coder-windows-arm64.asc") + + @Test + fun `Default CLI and signature for Windows ARMV7`() = + assertBinaryAndSignature("Windows 10", "armv7l", "coder-windows-armv7.exe", "coder-windows-armv7.asc") + + @Test + fun `Default CLI and signature for Linux AMD64`() = + assertBinaryAndSignature("Linux", "x86_64", "coder-linux-amd64", "coder-linux-amd64.asc") + + @Test + fun `Default CLI and signature for Linux ARM64`() = + assertBinaryAndSignature("Linux", "aarch64", "coder-linux-arm64", "coder-linux-arm64.asc") + + @Test + fun `Default CLI and signature for Linux ARMV7`() = + assertBinaryAndSignature("Linux", "armv7l", "coder-linux-armv7", "coder-linux-armv7.asc") + + @Test + fun `Default CLI and signature for Mac AMD64`() = + assertBinaryAndSignature("Mac OS X", "x86_64", "coder-darwin-amd64", "coder-darwin-amd64.asc") + + @Test + fun `Default CLI and signature for Mac ARM64`() = + assertBinaryAndSignature("Mac OS X", "aarch64", "coder-darwin-arm64", "coder-darwin-arm64.asc") + + @Test + fun `Default CLI and signature for Mac ARMV7`() = + assertBinaryAndSignature("Mac OS X", "armv7l", "coder-darwin-armv7", "coder-darwin-armv7.asc") + + @Test + fun `Default CLI and signature for unknown OS and Arch`() = + assertBinaryAndSignature(null, null, "coder-windows-amd64.exe", "coder-windows-amd64.asc") + + @Test + fun `Default CLI and signature for unknown Arch fallback on Linux`() = + assertBinaryAndSignature("Linux", "mips64", "coder-linux-amd64", "coder-linux-amd64.asc") + + private fun assertBinaryAndSignature( + osName: String?, + arch: String?, + expectedBinary: String, + expectedSignature: String + ) { + if (osName == null) System.clearProperty("os.name") else System.setProperty("os.name", osName) + if (arch == null) System.clearProperty("os.arch") else System.setProperty("os.arch", arch) + + assertEquals(expectedBinary, store.defaultCliBinaryNameByOsAndArch) + assertEquals(expectedSignature, store.defaultSignatureNameByOsAndArch) + } + +} \ No newline at end of file From bce103bf20f1557c384a015c099b4cc801100d07 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 15 Jul 2025 01:52:04 +0300 Subject: [PATCH 27/30] chore: remove code around URL building It is already supported by java.net.URI --- .../toolbox/cli/downloader/CoderDownloadService.kt | 3 +-- .../kotlin/com/coder/toolbox/util/URLExtensions.kt | 14 -------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt index d3b49da..d59d476 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt @@ -7,7 +7,6 @@ import com.coder.toolbox.util.SemVer import com.coder.toolbox.util.getHeaders import com.coder.toolbox.util.getOS import com.coder.toolbox.util.sha1 -import com.coder.toolbox.util.withLastSegment import okhttp3.ResponseBody import retrofit2.Response import java.io.FileInputStream @@ -154,7 +153,7 @@ class CoderDownloadService( } private suspend fun downloadSignature(url: URL, showTextProgress: (String) -> Unit): DownloadResult { - val signatureURL = url.withLastSegment(context.settingsStore.defaultSignatureNameByOsAndArch) + val signatureURL = url.toURI().resolve(context.settingsStore.defaultSignatureNameByOsAndArch).toURL() val localSignaturePath = localBinaryPath.parent.resolve(context.settingsStore.defaultSignatureNameByOsAndArch) context.logger.info("Downloading signature from $signatureURL") diff --git a/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt b/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt index 360b62a..c1aaa81 100644 --- a/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt +++ b/src/main/kotlin/com/coder/toolbox/util/URLExtensions.kt @@ -13,20 +13,6 @@ fun URL.withPath(path: String): URL = URL( if (path.startsWith("/")) path else "/$path", ) -fun URL.withLastSegment(segment: String): URL { - val uri = this.toURI() - val basePath = uri.path.substringBeforeLast('/') - val newPath = "$basePath/$segment" - val newUri = URI( - uri.scheme, - uri.authority, - newPath, - uri.query, - uri.fragment - ) - return newUri.toURL() -} - /** * Return the host, converting IDN to ASCII in case the file system cannot * support the necessary character set. From 8342a2118bebcf9f9772ead9da49431bcbfa957d Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 15 Jul 2025 22:14:41 +0300 Subject: [PATCH 28/30] fix: raise the original error when cli can't be downloaded --- src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt | 4 +++- .../kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 06d748c..3876584 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -3,6 +3,7 @@ package com.coder.toolbox.cli import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.cli.downloader.CoderDownloadApi import com.coder.toolbox.cli.downloader.CoderDownloadService +import com.coder.toolbox.cli.downloader.DownloadResult import com.coder.toolbox.cli.downloader.DownloadResult.Downloaded import com.coder.toolbox.cli.ex.MissingVersionException import com.coder.toolbox.cli.ex.SSHConfigFormatException @@ -168,7 +169,8 @@ class CoderCLIManager( }.let { result -> when { result.isSkipped() -> return false - result.isNotFoundOrFailed() -> throw IllegalStateException("Could not find or download Coder CLI") + result.isNotFound() -> throw IllegalStateException("Could not find Coder CLI") + result.isFailed() -> throw (result as DownloadResult.Failed).error else -> result as Downloaded } } diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt index 1da4007..871b6fe 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt @@ -15,7 +15,9 @@ sealed class DownloadResult { fun isSkipped(): Boolean = this is Skipped - fun isNotFoundOrFailed(): Boolean = this is NotFound || this is Failed + fun isNotFound(): Boolean = this is NotFound + + fun isFailed(): Boolean = this is Failed fun isDownloaded(): Boolean = this is Downloaded From 881a662592796a591cf5c63d944f403c4a834f8d Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 15 Jul 2025 23:35:53 +0300 Subject: [PATCH 29/30] impl: download the cli to a temporary location The download and signature verification steps are now slightly altered to first download the cli to a temporary location and only after the signature verification is successful or the user accepted the risk of running an unsigned binary - then and only then the temp cli is moved to its final location (actually it is just a rename). If the delete fails, this prevents the unsigned binary from being picked up as the cached binary on the next run. --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 135 +++++++++--------- .../cli/downloader/CoderDownloadService.kt | 54 +++++-- 2 files changed, 110 insertions(+), 79 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 3876584..f1952f6 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -164,87 +164,88 @@ class CoderCLIManager( * Download the CLI from the deployment if necessary. */ suspend fun download(buildVersion: String, showTextProgress: (String) -> Unit): Boolean { - val cliResult = withContext(Dispatchers.IO) { - downloader.downloadCli(buildVersion, showTextProgress) - }.let { result -> - when { - result.isSkipped() -> return false - result.isNotFound() -> throw IllegalStateException("Could not find Coder CLI") - result.isFailed() -> throw (result as DownloadResult.Failed).error - else -> result as Downloaded + try { + val cliResult = withContext(Dispatchers.IO) { + downloader.downloadCli(buildVersion, showTextProgress) + }.let { result -> + when { + result.isSkipped() -> return false + result.isNotFound() -> throw IllegalStateException("Could not find Coder CLI") + result.isFailed() -> throw (result as DownloadResult.Failed).error + else -> result as Downloaded + } } - } - - var signatureResult = withContext(Dispatchers.IO) { - downloader.downloadSignature(showTextProgress) - } - if (signatureResult.isNotDownloaded()) { - context.logger.info("Trying to download signature file from releases.coder.com") - signatureResult = withContext(Dispatchers.IO) { - downloader.downloadReleasesSignature(buildVersion, showTextProgress) + var signatureResult = withContext(Dispatchers.IO) { + downloader.downloadSignature(showTextProgress) } - } - // if we could not find any signature and the user wants to explicitly - // confirm whether we run an unsigned cli - if (signatureResult.isNotDownloaded()) { - if (context.settingsStore.allowUnsignedBinaryWithoutPrompt) { - context.logger.warn("Running unsigned CLI from ${cliResult.source}") - return true - } else { - val acceptsUnsignedBinary = context.ui.showYesNoPopup( - context.i18n.ptrl("Security Warning"), - context.i18n.pnotr("Can't verify the integrity of the Coder CLI pulled from ${cliResult.source}"), - context.i18n.ptrl("Accept"), - context.i18n.ptrl("Abort"), - ) + if (signatureResult.isNotDownloaded()) { + context.logger.info("Trying to download signature file from releases.coder.com") + signatureResult = withContext(Dispatchers.IO) { + downloader.downloadReleasesSignature(buildVersion, showTextProgress) + } + } - if (acceptsUnsignedBinary) { + // if we could not find any signature and the user wants to explicitly + // confirm whether we run an unsigned cli + if (signatureResult.isNotDownloaded()) { + if (context.settingsStore.allowUnsignedBinaryWithoutPrompt) { + context.logger.warn("Running unsigned CLI from ${cliResult.source}") + downloader.commit() return true } else { - // remove the cli, otherwise next time the user tries to login the cached cli is picked up, - // and we don't verify cached cli signatures - Files.delete(cliResult.dst) - throw UnsignedBinaryExecutionDeniedException("Running unsigned CLI from ${cliResult.source} was denied by the user") + val acceptsUnsignedBinary = context.ui.showYesNoPopup( + context.i18n.ptrl("Security Warning"), + context.i18n.pnotr("Can't verify the integrity of the Coder CLI pulled from ${cliResult.source}"), + context.i18n.ptrl("Accept"), + context.i18n.ptrl("Abort"), + ) + + if (acceptsUnsignedBinary) { + downloader.commit() + return true + } else { + throw UnsignedBinaryExecutionDeniedException("Running unsigned CLI from ${cliResult.source} was denied by the user") + } } } - } - - // we have the cli, and signature is downloaded, let's verify the signature - signatureResult = signatureResult as Downloaded - gpgVerifier.verifySignature(cliResult.dst, signatureResult.dst).let { result -> - when { - result.isValid() -> return true - else -> { - // remove the cli, otherwise next time the user tries to login the cached cli is picked up, - // and we don't verify cached cli signatures - runCatching { Files.delete(cliResult.dst) } - .onFailure { ex -> - context.logger.warn(ex, "Failed to delete CLI file: ${cliResult.dst}") - } - val exception = when { - result.isInvalid() -> { - val reason = (result as Invalid).reason - UnsignedBinaryExecutionDeniedException( - "Signature of ${cliResult.dst} is invalid." + reason?.let { " Reason: $it" }.orEmpty() - ) - } - - result.signatureIsNotFound() -> { - UnsignedBinaryExecutionDeniedException( - "Can't verify signature of ${cliResult.dst} because ${signatureResult.dst} does not exist" - ) - } + // we have the cli, and signature is downloaded, let's verify the signature + signatureResult = signatureResult as Downloaded + gpgVerifier.verifySignature(cliResult.dst, signatureResult.dst).let { result -> + when { + result.isValid() -> { + downloader.commit() + return true + } - else -> { - UnsignedBinaryExecutionDeniedException((result as Failed).error.message) + else -> { + val exception = when { + result.isInvalid() -> { + val reason = (result as Invalid).reason + UnsignedBinaryExecutionDeniedException( + "Signature of ${cliResult.dst} is invalid." + reason?.let { " Reason: $it" } + .orEmpty() + ) + } + + result.signatureIsNotFound() -> { + UnsignedBinaryExecutionDeniedException( + "Can't verify signature of ${cliResult.dst} because ${signatureResult.dst} does not exist" + ) + } + + else -> { + UnsignedBinaryExecutionDeniedException((result as Failed).error.message) + } } + throw exception } - throw exception } } + } finally { + downloader.cleanup() } } @@ -475,7 +476,7 @@ class CoderCLIManager( } else -> { - // An error here most likely means the CLI does not exist or + // An error here most likely means the CLI does not exist, or // it executed successfully but output no version which // suggests it is not the right binary. context.logger.info("Unable to determine $localBinaryPath version: ${e.message}") diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt index d59d476..b414c93 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt @@ -7,6 +7,8 @@ import com.coder.toolbox.util.SemVer import com.coder.toolbox.util.getHeaders import com.coder.toolbox.util.getOS import com.coder.toolbox.util.sha1 +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import okhttp3.ResponseBody import retrofit2.Response import java.io.FileInputStream @@ -17,6 +19,7 @@ import java.net.URI import java.net.URL import java.nio.file.Files import java.nio.file.Path +import java.nio.file.StandardCopyOption import java.nio.file.StandardOpenOption import java.util.zip.GZIPInputStream import kotlin.io.path.name @@ -31,13 +34,14 @@ class CoderDownloadService( private val deploymentUrl: URL, forceDownloadToData: Boolean, ) { - val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentUrl) - val localBinaryPath: Path = context.settingsStore.binPath(deploymentUrl, forceDownloadToData) + private val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentUrl) + private val cliFinalDst: Path = context.settingsStore.binPath(deploymentUrl, forceDownloadToData) + private val cliTempDst: Path = cliFinalDst.resolveSibling("${cliFinalDst.name}.tmp") suspend fun downloadCli(buildVersion: String, showTextProgress: (String) -> Unit): DownloadResult { val eTag = calculateLocalETag() if (eTag != null) { - context.logger.info("Found existing binary at $localBinaryPath; calculated hash as $eTag") + context.logger.info("Found existing binary at $cliFinalDst; calculated hash as $eTag") } val response = downloadApi.downloadCli( url = remoteBinaryURL.toString(), @@ -47,13 +51,13 @@ class CoderDownloadService( return when (response.code()) { HTTP_OK -> { - context.logger.info("Downloading binary to $localBinaryPath") - response.saveToDisk(localBinaryPath, showTextProgress, buildVersion)?.makeExecutable() - DownloadResult.Downloaded(remoteBinaryURL, localBinaryPath) + context.logger.info("Downloading binary to temporary $cliTempDst") + response.saveToDisk(cliTempDst, showTextProgress, buildVersion)?.makeExecutable() + DownloadResult.Downloaded(remoteBinaryURL, cliTempDst) } HTTP_NOT_MODIFIED -> { - context.logger.info("Using cached binary at $localBinaryPath") + context.logger.info("Using cached binary at $cliFinalDst") showTextProgress("Using cached binary") DownloadResult.Skipped } @@ -67,14 +71,40 @@ class CoderDownloadService( } } + /** + * Renames the temporary binary file to its original destination name. + * The implementation will override sibling file that has the original + * destination name. + */ + suspend fun commit(): Path { + return withContext(Dispatchers.IO) { + context.logger.info("Renaming binary from $cliTempDst to $cliFinalDst") + Files.move(cliTempDst, cliFinalDst, StandardCopyOption.REPLACE_EXISTING) + cliFinalDst.makeExecutable() + cliFinalDst + } + } + + /** + * Cleans up the temporary binary file if it exists. + */ + suspend fun cleanup() { + withContext(Dispatchers.IO) { + runCatching { Files.deleteIfExists(cliTempDst) } + .onFailure { ex -> + context.logger.warn(ex, "Failed to delete temporary CLI file: $cliTempDst") + } + } + } + private fun calculateLocalETag(): String? { return try { - if (localBinaryPath.notExists()) { + if (cliFinalDst.notExists()) { return null } - sha1(FileInputStream(localBinaryPath.toFile())) + sha1(FileInputStream(cliFinalDst.toFile())) } catch (e: Exception) { - context.logger.warn(e, "Unable to calculate hash for $localBinaryPath") + context.logger.warn(e, "Unable to calculate hash for $cliFinalDst") null } } @@ -124,7 +154,7 @@ class CoderDownloadService( } } } - return localBinaryPath + return cliFinalDst } @@ -154,7 +184,7 @@ class CoderDownloadService( private suspend fun downloadSignature(url: URL, showTextProgress: (String) -> Unit): DownloadResult { val signatureURL = url.toURI().resolve(context.settingsStore.defaultSignatureNameByOsAndArch).toURL() - val localSignaturePath = localBinaryPath.parent.resolve(context.settingsStore.defaultSignatureNameByOsAndArch) + val localSignaturePath = cliFinalDst.parent.resolve(context.settingsStore.defaultSignatureNameByOsAndArch) context.logger.info("Downloading signature from $signatureURL") val response = downloadApi.downloadSignature( From aeb79e582ee90854315a13ee89ea438c1b85a249 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 16 Jul 2025 00:39:13 +0300 Subject: [PATCH 30/30] impl: prompt the user if when signature verification fails Ask the user if he wants to accept the risk of running a potentially tampered CLI when signature verification failed (i.e. we downloaded the signatures but either it doesn't match or there were some error while computing the signature. --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 58 +++++++++++++------ .../toolbox/cli/downloader/DownloadResult.kt | 2 - .../resources/localization/defaultMessages.po | 3 + 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index f1952f6..0a6ce8d 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -9,6 +9,7 @@ import com.coder.toolbox.cli.ex.MissingVersionException import com.coder.toolbox.cli.ex.SSHConfigFormatException import com.coder.toolbox.cli.ex.UnsignedBinaryExecutionDeniedException import com.coder.toolbox.cli.gpg.GPGVerifier +import com.coder.toolbox.cli.gpg.VerificationResult import com.coder.toolbox.cli.gpg.VerificationResult.Failed import com.coder.toolbox.cli.gpg.VerificationResult.Invalid import com.coder.toolbox.sdk.v2.models.Workspace @@ -221,26 +222,21 @@ class CoderCLIManager( } else -> { - val exception = when { - result.isInvalid() -> { - val reason = (result as Invalid).reason - UnsignedBinaryExecutionDeniedException( - "Signature of ${cliResult.dst} is invalid." + reason?.let { " Reason: $it" } - .orEmpty() - ) - } - - result.signatureIsNotFound() -> { - UnsignedBinaryExecutionDeniedException( - "Can't verify signature of ${cliResult.dst} because ${signatureResult.dst} does not exist" - ) - } - - else -> { - UnsignedBinaryExecutionDeniedException((result as Failed).error.message) - } + logFailure(result, cliResult, signatureResult) + // prompt the user if he wants to accept the risk + val shouldRunAnyway = context.ui.showYesNoPopup( + context.i18n.ptrl("Security Warning"), + context.i18n.pnotr("Could not verify the authenticity of the ${cliResult.source}, it may be tampered with. Would you like to run it anyway?"), + context.i18n.ptrl("Run anyway"), + context.i18n.ptrl("Abort"), + ) + + if (shouldRunAnyway) { + downloader.commit() + return true + } else { + throw UnsignedBinaryExecutionDeniedException("Running unverified CLI from ${cliResult.source} was denied by the user") } - throw exception } } } @@ -249,6 +245,30 @@ class CoderCLIManager( } } + private fun logFailure( + result: VerificationResult, + cliResult: Downloaded, + signatureResult: Downloaded + ) { + when { + result.isInvalid() -> { + val reason = (result as Invalid).reason + context.logger.error("Signature of ${cliResult.dst} is invalid." + reason?.let { " Reason: $it" } + .orEmpty()) + } + + result.signatureIsNotFound() -> { + context.logger.error("Can't verify signature of ${cliResult.dst} because ${signatureResult.dst} does not exist") + } + + else -> { + UnsignedBinaryExecutionDeniedException((result as Failed).error.message) + val failure = result as DownloadResult.Failed + context.logger.error(failure.error, "Failed to verify signature for ${cliResult.dst}") + } + } + } + /** * Use the provided token to initializeSession the CLI. */ diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt index 871b6fe..29d4fda 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/DownloadResult.kt @@ -19,7 +19,5 @@ sealed class DownloadResult { fun isFailed(): Boolean = this is Failed - fun isDownloaded(): Boolean = this is Downloaded - fun isNotDownloaded(): Boolean = this !is Downloaded } \ No newline at end of file diff --git a/src/main/resources/localization/defaultMessages.po b/src/main/resources/localization/defaultMessages.po index 385e6b7..41dbeca 100644 --- a/src/main/resources/localization/defaultMessages.po +++ b/src/main/resources/localization/defaultMessages.po @@ -161,4 +161,7 @@ msgid "Accept" msgstr "" msgid "Abort" +msgstr "" + +msgid "Run anyway" msgstr "" \ No newline at end of file