From 0bc801cfee35206f59c22502de8d89524a13c45d Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Mon, 18 May 2026 22:46:54 +0300 Subject: [PATCH 01/11] Pass CLI login tokens via CODER_SESSION_TOKEN instead of --token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch the CLI manager to provide session tokens through the CODER_SESSION_TOKEN environment variable when running `coder login` instead of appending `--token ` to the process arguments. We’re making this change for security reasons. Command-line arguments are more likely to be exposed through process listings, shell history, CI/job logs, and command auditing, while environment variables have a smaller exposure surface on typical systems. This aligns the plugin with the Coder CLI guidance that prefers CODER_SESSION_TOKEN over `--token`. --- CHANGELOG.md | 4 +++ README.md | 4 +++ .../com/coder/toolbox/cli/CoderCLIManager.kt | 33 +++++++++++-------- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62cf35c..3159d04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Changed + +- reduce token exposure in process arguments and command logs + ## 0.9.0 - 2026-05-14 ### Added diff --git a/README.md b/README.md index cd598a5..9bfde3e 100644 --- a/README.md +++ b/README.md @@ -539,6 +539,10 @@ support, may trigger regeneration of SSH configurations. > [!IMPORTANT] > Token authentication is required when TLS certificates are not configured. +When the plugin logs the Coder CLI in with a session token, it passes that token through the +`CODER_SESSION_TOKEN` environment variable instead of `--token`. This reduces the chances of the token showing up in +process listings, shell history, or command-line audit logs. + ## Releasing 1. Check that the changelog lists all the important changes. diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index eba6b23..fb260b7 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -265,10 +265,9 @@ class CoderCLIManager( fun login(token: String): String { context.logger.info("Storing CLI credentials in $coderConfigPath") return exec( + env = mapOf(CODER_SESSION_TOKEN_ENV_VAR to token), "login", deploymentURL.toString(), - "--token", - token, "--global-config", coderConfigPath.toString(), ) @@ -534,17 +533,23 @@ class CoderCLIManager( return matches } - private fun exec(vararg args: String): String { - val stdout = - ProcessExecutor() - .command(localBinaryPath.toString(), *args) - .environment("CODER_HEADER_COMMAND", context.settingsStore.headerCommand) - .exitValues(0) - .readOutput(true) - .execute() - .outputUTF8() - val redactedArgs = listOf(*args).joinToString(" ").replace(tokenRegex, "--token ") - context.logger.info("`$localBinaryPath $redactedArgs`: $stdout") + private fun exec(vararg args: String): String = exec(env = emptyMap(), *args) + + private fun exec(env: Map, vararg args: String): String { + val executor = ProcessExecutor() + .command(localBinaryPath.toString(), *args) + .exitValues(0) + .readOutput(true) + + context.settingsStore.headerCommand?.let { + executor.environment("CODER_HEADER_COMMAND", it) + } + env.forEach { (key, value) -> + executor.environment(key, value) + } + + val stdout = executor.execute().outputUTF8() + context.logger.info("`$localBinaryPath ${listOf(*args).joinToString(" ")}`: $stdout") return stdout } @@ -572,7 +577,7 @@ class CoderCLIManager( } companion object { - private val tokenRegex = "--token [^ ]+".toRegex() + private const val CODER_SESSION_TOKEN_ENV_VAR = "CODER_SESSION_TOKEN" private fun getHostnamePrefix(url: URL): String = "coder-jetbrains-toolbox-${url.safeHost()}" From a5488c89f2422399ccca4e4677b650ead35fab8e Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 19 May 2026 00:02:13 +0300 Subject: [PATCH 02/11] chore: add UTs for the token env --- .../coder/toolbox/cli/CoderCLIManagerTest.kt | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index bcd321f..c04350e 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -74,6 +74,7 @@ private val noOpTextProgress: (String) -> Unit = { _ -> } internal class CoderCLIManagerTest { private val ui = mockk(relaxed = true) + private val logger = mockk(relaxed = true) private val context = CoderToolboxContext( ui, mockk(), @@ -82,12 +83,12 @@ internal class CoderCLIManagerTest { mockk(), mockk(), mockk(), - mockk(relaxed = true), + logger, mockk(relaxed = true), CoderSettingsStore( pluginTestSettingsStore(), Environment(), - mockk(relaxed = true) + logger ), mockk(), object : ToolboxProxySettings { @@ -119,6 +120,14 @@ internal class CoderCLIManagerTest { listOf("#!/bin/sh", str) }.joinToString(System.lineSeparator()) + private fun writeExecutable(path: Path, contents: String) { + path.parent.toFile().mkdirs() + path.toFile().writeText(contents) + if (getOS() != OS.WINDOWS) { + path.toFile().setExecutable(true) + } + } + /** * Return the contents of a script that outputs JSON containing the version. */ @@ -424,6 +433,49 @@ internal class CoderCLIManagerTest { ) } + @Test + fun `login passes token via environment instead of argv`() { + val binaryFile = tmpdir.resolve("login-env").resolve(if (getOS() == OS.WINDOWS) "coder.bat" else "coder") + val argsFile = tmpdir.resolve("login-env").resolve("argv.txt") + val envFile = tmpdir.resolve("login-env").resolve("env.txt") + val token = "super-secret-token" + val stdout = "login ok" + val script = if (getOS() == OS.WINDOWS) { + mkbin( + """ + echo %* > "${argsFile.toAbsolutePath()}" + echo %CODER_SESSION_TOKEN% > "${envFile.toAbsolutePath()}" + echo $stdout + """.trimIndent() + ) + } else { + mkbin( + """ + printf '%s\n' "$*" > '${argsFile.toAbsolutePath()}' + printf '%s\n' "${'$'}CODER_SESSION_TOKEN" > '${envFile.toAbsolutePath()}' + printf '$stdout\n' + """.trimIndent() + ) + } + writeExecutable(binaryFile, script) + + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binaryFile.toString(), + ENABLE_DOWNLOADS to "false", + ), + Environment(), + logger + ) + val ccm = CoderCLIManager(context.copy(settingsStore = settings), URL("https://test.coder.com")) + + assertEquals("$stdout\n", ccm.login(token)) + assertContains(argsFile.toFile().readText(), "login https://test.coder.com --global-config") + assertFalse(argsFile.toFile().readText().contains("--token")) + assertFalse(argsFile.toFile().readText().contains(token)) + assertEquals(token, envFile.toFile().readText().trim()) + } + @Test fun testOverwritesWrongVersion() { val (srv, url) = mockServer() From bba2fbc2c87c2dc182a64dd5d9f411ccf5846aa6 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 19 May 2026 22:51:34 +0300 Subject: [PATCH 03/11] Replace zt-exec with internal process runner Replace zt-exec with a small internal ProcessBuilder-based runner so subprocess failures stay under our control. We needed to remove ProcessExecutor because its exception messages include the spawned process environment. Now that CLI auth is passed through CODER_SESSION_TOKEN, failures could spill tokens into logs before our own sanitization had a chance to run. That made reliable redaction impossible/ugly at the logging layer. The new runner uses Java 21 ProcessBuilder from Kotlin, captures stdout and stderr, supports expected exit codes and stderr discard-on-success behavior, and throws project-owned process exceptions with centralized secret sanitization. This keeps process execution behavior explicit while avoiding a dependency that could format sensitive environment values into exceptions. --- CHANGELOG.md | 2 + build.gradle.kts | 1 - gradle/libs.versions.toml | 2 - .../com/coder/toolbox/cli/CoderCLIManager.kt | 22 ++-- .../com/coder/toolbox/sdk/CoderRestClient.kt | 15 ++- .../kotlin/com/coder/toolbox/util/Error.kt | 3 +- .../kotlin/com/coder/toolbox/util/Headers.kt | 17 +-- .../com/coder/toolbox/util/ProcessRunner.kt | 113 ++++++++++++++++++ .../com/coder/toolbox/util/SecretSanitizer.kt | 14 +++ src/main/resources/dependencies.json | 7 -- 10 files changed, 151 insertions(+), 45 deletions(-) create mode 100644 src/main/kotlin/com/coder/toolbox/util/ProcessRunner.kt create mode 100644 src/main/kotlin/com/coder/toolbox/util/SecretSanitizer.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 3159d04..14ef6de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Changed - reduce token exposure in process arguments and command logs +- replaced the external process execution dependency with a lightweight internal runner, reducing plugin dependencies + and improving error sanitization. ## 0.9.0 - 2026-05-14 diff --git a/build.gradle.kts b/build.gradle.kts index 83b11cf..caf20fe 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -36,7 +36,6 @@ dependencies { compileOnly(libs.bundles.serialization) compileOnly(libs.coroutines.core) implementation(libs.okhttp) - implementation(libs.exec) implementation(libs.moshi) ksp(libs.moshi.codegen) implementation(libs.retrofit) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 48977d5..181b0c1 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -7,7 +7,6 @@ okhttp = "4.12.0" dependency-license-report = "3.1.2" marketplace-client = "2.0.51" gradle-wrapper = "0.16.0" -exec = "1.12" moshi = "1.15.2" ksp = "2.3.6" retrofit = "3.0.0" @@ -28,7 +27,6 @@ serialization-core = { module = "org.jetbrains.kotlinx:kotlinx-serialization-cor serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "serialization" } serialization-json-okio = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json-okio", version.ref = "serialization" } okhttp = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttp" } -exec = { module = "org.zeroturnaround:zt-exec", version.ref = "exec" } moshi = { module = "com.squareup.moshi:moshi", version.ref = "moshi" } moshi-codegen = { module = "com.squareup.moshi:moshi-kotlin-codegen", version.ref = "moshi" } retrofit = { module = "com.squareup.retrofit2:retrofit", version.ref = "retrofit" } diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index fb260b7..e04dd5a 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -20,14 +20,15 @@ import com.coder.toolbox.util.InvalidVersionException import com.coder.toolbox.util.SemVer import com.coder.toolbox.util.escape import com.coder.toolbox.util.escapeSubcommand +import com.coder.toolbox.util.runProcess import com.coder.toolbox.util.safeHost +import com.coder.toolbox.util.sanitizeSecrets 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 org.zeroturnaround.exec.ProcessExecutor import retrofit2.Retrofit import java.io.EOFException import java.io.FileNotFoundException @@ -536,20 +537,15 @@ class CoderCLIManager( private fun exec(vararg args: String): String = exec(env = emptyMap(), *args) private fun exec(env: Map, vararg args: String): String { - val executor = ProcessExecutor() - .command(localBinaryPath.toString(), *args) - .exitValues(0) - .readOutput(true) - - context.settingsStore.headerCommand?.let { - executor.environment("CODER_HEADER_COMMAND", it) - } - env.forEach { (key, value) -> - executor.environment(key, value) + val command = listOf(localBinaryPath.toString(), *args) + val processEnv = buildMap { + context.settingsStore.headerCommand?.let { put("CODER_HEADER_COMMAND", it) } + putAll(env) } - val stdout = executor.execute().outputUTF8() - context.logger.info("`$localBinaryPath ${listOf(*args).joinToString(" ")}`: $stdout") + val stdout = runProcess(command, environment = processEnv).stdout + val sanitizedStdout = stdout.sanitizeSecrets() + context.logger.info("`$localBinaryPath ${listOf(*args).joinToString(" ")}`: $sanitizedStdout") return stdout } diff --git a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt index 1d0f39c..7e114ec 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt @@ -24,6 +24,7 @@ import com.coder.toolbox.sdk.v2.models.WorkspaceBuildReason import com.coder.toolbox.sdk.v2.models.WorkspaceResource import com.coder.toolbox.sdk.v2.models.WorkspaceTransition import com.coder.toolbox.util.ReloadableTlsContext +import com.coder.toolbox.util.runProcess import com.coder.toolbox.views.state.CoderOAuthSessionContext import com.coder.toolbox.views.state.hasRefreshToken import com.squareup.moshi.Moshi @@ -32,7 +33,6 @@ import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import okhttp3.OkHttpClient -import org.zeroturnaround.exec.ProcessExecutor import retrofit2.Response import retrofit2.Retrofit import retrofit2.converter.moshi.MoshiConverterFactory @@ -411,18 +411,17 @@ open class CoderRestClient( if (command.isNullOrBlank()) return@withContext false return@withContext try { - val result = ProcessExecutor() - .command(command.split(" ").toList()) - .exitValueAny() - .readOutput(true) - .execute() + val result = runProcess( + command.split(" ").filter { it.isNotBlank() }, + expectedExitCodes = Int.MIN_VALUE..Int.MAX_VALUE, + ) if (tlsContext.reload()) { context.logger.info("Certificate refresh successful. Reloading TLS and evicting pool.") // forces OkHttp to close the broken HTTP/2 connection. httpClient.connectionPool.evictAll() return@withContext true } else { - context.logger.error("Refresh command failed with code ${result.exitValue}") + context.logger.error("Refresh command failed with code ${result.exitCode}") false } } catch (ex: Exception) { @@ -448,4 +447,4 @@ private fun Response<*>.parseErrorBody(moshi: Moshi): ApiErrorResponse? { } catch (e: Exception) { null } -} \ No newline at end of file +} diff --git a/src/main/kotlin/com/coder/toolbox/util/Error.kt b/src/main/kotlin/com/coder/toolbox/util/Error.kt index 752aa07..029b086 100644 --- a/src/main/kotlin/com/coder/toolbox/util/Error.kt +++ b/src/main/kotlin/com/coder/toolbox/util/Error.kt @@ -2,7 +2,6 @@ package com.coder.toolbox.util import com.coder.toolbox.cli.ex.ResponseException import com.coder.toolbox.sdk.ex.APIResponseException -import org.zeroturnaround.exec.InvalidExitValueException import java.net.ConnectException import java.net.UnknownHostException @@ -17,7 +16,7 @@ fun Throwable.prettify(): String { is FileSystemException -> fileSystemFailed(this.file.toString()) is java.nio.file.FileSystemException -> fileSystemFailed(this.file) is UnknownHostException -> "Unknown host $reason" - is InvalidExitValueException -> "CLI exited unexpectedly with ${this.exitValue}." + is ProcessExitException -> "CLI exited unexpectedly with ${this.result.exitCode}." is APIResponseException -> { if (this.isUnauthorized) { "Authorization failed" diff --git a/src/main/kotlin/com/coder/toolbox/util/Headers.kt b/src/main/kotlin/com/coder/toolbox/util/Headers.kt index 524c363..e3280f9 100644 --- a/src/main/kotlin/com/coder/toolbox/util/Headers.kt +++ b/src/main/kotlin/com/coder/toolbox/util/Headers.kt @@ -1,7 +1,5 @@ package com.coder.toolbox.util -import org.zeroturnaround.exec.ProcessExecutor -import java.io.OutputStream import java.net.URL private val newlineRegex = "\r?\n".toRegex() @@ -20,16 +18,11 @@ fun getHeaders( else -> Pair("sh", "-c") } val output = - ProcessExecutor() - .command(shell, caller, headerCommand) - .environment("CODER_URL", url.toString()) - // By default stderr is in the output, but we want to ignore it. stderr - // will still be included in the exception if something goes wrong. - .redirectError(OutputStream.nullOutputStream()) - .exitValues(0) - .readOutput(true) - .execute() - .outputUTF8() + runProcess( + listOf(shell, caller, headerCommand), + environment = mapOf("CODER_URL" to url.toString()), + stderrMode = ProcessStderrMode.DISCARD_ON_SUCCESS, + ).stdout // The Coder CLI will allow no output, but not blank lines. Possibly we // should skip blank lines, but it is better to have parity so commands will diff --git a/src/main/kotlin/com/coder/toolbox/util/ProcessRunner.kt b/src/main/kotlin/com/coder/toolbox/util/ProcessRunner.kt new file mode 100644 index 0000000..8750f65 --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/util/ProcessRunner.kt @@ -0,0 +1,113 @@ +package com.coder.toolbox.util + +import java.io.IOException +import java.nio.charset.Charset +import kotlin.concurrent.thread + +data class ProcessResult( + val command: List, + val exitCode: Int, + val stdout: String, + val stderr: String, +) + +sealed class ProcessRunnerException( + message: String, + cause: Throwable? = null, +) : RuntimeException(message.sanitizeSecrets(), cause) + +enum class ProcessStderrMode { + CAPTURE, + DISCARD_ON_SUCCESS, +} + +class ProcessExecutionException( + message: String, + cause: Throwable? = null +) : ProcessRunnerException(message, cause) + +class ProcessExitException( + val result: ProcessResult, + private val expectedExitCodes: IntRange, +) : ProcessRunnerException( + buildString { + append("Unexpected exit value: ${result.exitCode}, allowed exit values: $expectedExitCodes") + append(", executed command ${result.command}") + if (result.stdout.isNotBlank()) { + append(", stdout was ${result.stdout.length} bytes:\n${result.stdout}") + } + if (result.stderr.isNotBlank()) { + append(", stderr was ${result.stderr.length} bytes:\n${result.stderr}") + } + }.sanitizeSecrets() +) + +/** + * Runs a process and waits for it to finish. + * + * The wait is intentionally unbounded. Only exit code 0 is accepted by default. + * Pass [expectedExitCodes] when a command has additional valid exit codes. + * + * Standard output is always captured and returned in [ProcessResult.stdout] while + * standard error is captured by default and returned in [ProcessResult.stderr]. Use + * [ProcessStderrMode.DISCARD_ON_SUCCESS] in order to ignore it. + * Stderr is ignored for successful results, but preserved in [ProcessExitException] when the process fails. + */ +fun runProcess( + command: List, + environment: Map = emptyMap(), + expectedExitCodes: IntRange = 0..0, + stderrMode: ProcessStderrMode = ProcessStderrMode.CAPTURE, + charset: Charset = Charsets.UTF_8, +): ProcessResult { + val process = + try { + ProcessBuilder(command) + .apply { environment().putAll(environment) } + .start() + } catch (ex: IOException) { + throw ProcessExecutionException("Failed to start process $command: ${ex.message}", ex) + } + + val stdout = StringBuilder() + val stderr = StringBuilder() + val stdoutReader = thread(start = true, name = "process-stdout-reader") { + process.inputStream.bufferedReader(charset).use { stdout.append(it.readText()) } + } + val stderrReader = thread(start = true, name = "process-stderr-reader") { + process.errorStream.bufferedReader(charset).use { stderr.append(it.readText()) } + } + + val exitCode = + try { + process.waitFor() + } catch (ex: InterruptedException) { + process.destroyForcibly() + Thread.currentThread().interrupt() + throw ProcessExecutionException("Interrupted while waiting for process $command", ex) + } + + try { + stdoutReader.join() + stderrReader.join() + } catch (ex: InterruptedException) { + Thread.currentThread().interrupt() + throw ProcessExecutionException("Interrupted while reading process output for $command", ex) + } + + val stderrText = stderr.toString() + val result = ProcessResult( + command = command, + exitCode = exitCode, + stdout = stdout.toString(), + stderr = if (exitCode in expectedExitCodes && stderrMode == ProcessStderrMode.DISCARD_ON_SUCCESS) { + "" + } else { + stderrText + }, + ) + if (exitCode !in expectedExitCodes) { + throw ProcessExitException(result, expectedExitCodes) + } + return result +} diff --git a/src/main/kotlin/com/coder/toolbox/util/SecretSanitizer.kt b/src/main/kotlin/com/coder/toolbox/util/SecretSanitizer.kt new file mode 100644 index 0000000..1ee438b --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/util/SecretSanitizer.kt @@ -0,0 +1,14 @@ +package com.coder.toolbox.util + +private val sensitivePatterns = listOf( + Regex("""(CODER_SESSION_TOKEN=)([^,\s}]+)"""), + Regex("""(Coder-Session-Token:\s*)([^\s,]+)""", RegexOption.IGNORE_CASE), + Regex("""(--token\s+)(\S+)"""), + Regex("""([?&]token=)([^&\s]+)""", RegexOption.IGNORE_CASE), +) + +fun String.sanitizeSecrets(): String { + return sensitivePatterns.fold(this) { acc, regex -> + acc.replace(regex, "$1") + } +} diff --git a/src/main/resources/dependencies.json b/src/main/resources/dependencies.json index 750f871..c38ab8e 100644 --- a/src/main/resources/dependencies.json +++ b/src/main/resources/dependencies.json @@ -61,12 +61,5 @@ "url": "http://www.slf4j.org", "license": "MIT License", "licenseUrl": "http://www.opensource.org/licenses/mit-license.php" - }, - { - "name": "org.zeroturnaround:zt-exec", - "version": "1.12", - "url": "https://github.com/zeroturnaround/zt-exec", - "license": "The Apache Software License, Version 2.0", - "licenseUrl": "http://www.apache.org/licenses/LICENSE-2.0.txt" } ] From ad4376222a669ec0111565dc782b058928ffb34b Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 19 May 2026 23:19:48 +0300 Subject: [PATCH 04/11] chore: fix broken UTs --- .../com/coder/toolbox/cli/CoderCLIManagerTest.kt | 12 ++++++------ .../kotlin/com/coder/toolbox/util/HeadersTest.kt | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index c04350e..95eac7b 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -25,6 +25,8 @@ import com.coder.toolbox.store.SSH_LOG_DIR import com.coder.toolbox.util.ConnectionMonitoringService import com.coder.toolbox.util.InvalidVersionException import com.coder.toolbox.util.OS +import com.coder.toolbox.util.ProcessExecutionException +import com.coder.toolbox.util.ProcessExitException import com.coder.toolbox.util.SemVer import com.coder.toolbox.util.escape import com.coder.toolbox.util.getOS @@ -49,8 +51,6 @@ 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 -import org.zeroturnaround.exec.ProcessInitException import java.net.HttpURLConnection import java.net.InetSocketAddress import java.net.Proxy @@ -358,7 +358,7 @@ internal class CoderCLIManagerTest { // Make sure login failures propagate. assertFailsWith( - exceptionClass = InvalidExitValueException::class, + exceptionClass = ProcessExitException::class, block = { ccm.login("jetbrains-ci-test") }, ) } @@ -428,7 +428,7 @@ internal class CoderCLIManagerTest { ) assertFailsWith( - exceptionClass = ProcessInitException::class, + exceptionClass = ProcessExecutionException::class, block = { ccm.login("fake-token") }, ) } @@ -859,14 +859,14 @@ internal class CoderCLIManagerTest { fun testFailVersionParse() { val tests = mapOf( - null to ProcessInitException::class, + null to ProcessExecutionException::class, echo("""{"foo": true, "baz": 1}""") to MissingVersionException::class, echo("""{"version": ""}""") to MissingVersionException::class, echo("""v0.0.1""") to JsonEncodingException::class, echo("""{"version: """) to JsonEncodingException::class, echo("""{"version": "invalid"}""") to InvalidVersionException::class, exit(0) to MissingVersionException::class, - exit(1) to InvalidExitValueException::class, + exit(1) to ProcessExitException::class, ) val ccm = CoderCLIManager( diff --git a/src/test/kotlin/com/coder/toolbox/util/HeadersTest.kt b/src/test/kotlin/com/coder/toolbox/util/HeadersTest.kt index c3b75d2..65b9319 100644 --- a/src/test/kotlin/com/coder/toolbox/util/HeadersTest.kt +++ b/src/test/kotlin/com/coder/toolbox/util/HeadersTest.kt @@ -104,7 +104,7 @@ internal class HeadersTest { "echo foo foo=bar" to "Header name cannot contain spaces, got \"foo foo\"", "echo foo=bar " to "Header name cannot contain spaces, got \" foo\"", "exit /b 1" to "Unexpected exit value: 1", - // "foobar" appears in the InvalidExitValueException message as part of the command string. + // "foobar" appears in the process error message as part of the command string. "echo foobar>&2&&exit /b 1" to "foobar", // echo. outputs a bare CRLF; a blank line anywhere in the output is an error. "echo foo=bar&&echo." to "Blank lines are not allowed", From c9008c026f3a1a6a6e44bae020f95906f78974b8 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 19 May 2026 23:40:47 +0300 Subject: [PATCH 05/11] chore: fix broken UT on Windows --- 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 95eac7b..edddf10 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -469,7 +469,7 @@ internal class CoderCLIManagerTest { ) val ccm = CoderCLIManager(context.copy(settingsStore = settings), URL("https://test.coder.com")) - assertEquals("$stdout\n", ccm.login(token)) + assertEquals(stdout, ccm.login(token).trim()) assertContains(argsFile.toFile().readText(), "login https://test.coder.com --global-config") assertFalse(argsFile.toFile().readText().contains("--token")) assertFalse(argsFile.toFile().readText().contains(token)) From 6d264b189263091018bca3204f2c4ccc22dcb9ad Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 20 May 2026 00:03:45 +0300 Subject: [PATCH 06/11] chore: more UTs for the new process runner --- .../com/coder/toolbox/util/ErrorTest.kt | 42 +++++ .../coder/toolbox/util/ProcessRunnerTest.kt | 163 ++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 src/test/kotlin/com/coder/toolbox/util/ErrorTest.kt create mode 100644 src/test/kotlin/com/coder/toolbox/util/ProcessRunnerTest.kt diff --git a/src/test/kotlin/com/coder/toolbox/util/ErrorTest.kt b/src/test/kotlin/com/coder/toolbox/util/ErrorTest.kt new file mode 100644 index 0000000..cf64869 --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/util/ErrorTest.kt @@ -0,0 +1,42 @@ +package com.coder.toolbox.util + +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertFalse + +internal class ErrorTest { + @Test + fun `sanitizeSecrets redacts common token forms`() { + val raw = """ + env={CODER_SESSION_TOKEN=super-secret-token} + header=Coder-Session-Token: super-secret-token + argv=--token super-secret-token + uri=https://coder.example.com?token=super-secret-token + """.trimIndent() + + val sanitized = raw.sanitizeSecrets() + + assertContains(sanitized, "CODER_SESSION_TOKEN=") + assertContains(sanitized, "Coder-Session-Token: ") + assertContains(sanitized, "--token ") + assertContains(sanitized, "?token=") + assertFalse(sanitized.contains("super-secret-token")) + } + + @Test + fun `process exit exception redacts output in message`() { + val ex = ProcessExitException( + ProcessResult( + command = listOf("coder", "login"), + exitCode = 1, + stdout = "CODER_SESSION_TOKEN=super-secret-token", + stderr = "Coder-Session-Token: super-secret-token", + ), + expectedExitCodes = 0..0, + ) + + assertContains(ex.message.orEmpty(), "CODER_SESSION_TOKEN=") + assertContains(ex.message.orEmpty(), "Coder-Session-Token: ") + assertFalse(ex.message.orEmpty().contains("super-secret-token")) + } +} diff --git a/src/test/kotlin/com/coder/toolbox/util/ProcessRunnerTest.kt b/src/test/kotlin/com/coder/toolbox/util/ProcessRunnerTest.kt new file mode 100644 index 0000000..5771aec --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/util/ProcessRunnerTest.kt @@ -0,0 +1,163 @@ +package com.coder.toolbox.util + +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse + +internal class ProcessRunnerTest { + @Test + @IgnoreOnWindows + fun `runProcess captures stdout and stderr`() { + val result = runProcess(listOf("sh", "-c", "printf hello && printf problem >&2")) + + assertEquals(0, result.exitCode) + assertEquals("hello", result.stdout) + assertEquals("problem", result.stderr) + } + + @Test + @IgnoreOnWindows + fun `runProcess can discard stderr on success`() { + val result = runProcess( + listOf("sh", "-c", "printf hello && printf problem >&2"), + stderrMode = ProcessStderrMode.DISCARD_ON_SUCCESS, + ) + + assertEquals(0, result.exitCode) + assertEquals("hello", result.stdout) + assertEquals("", result.stderr) + } + + @Test + @IgnoreOnWindows + fun `runProcess includes stderr on failure when discarding stderr on success`() { + val ex = assertFailsWith { + runProcess( + listOf("sh", "-c", "printf problem >&2; exit 5"), + stderrMode = ProcessStderrMode.DISCARD_ON_SUCCESS, + ) + } + + assertEquals(5, ex.result.exitCode) + assertEquals("problem", ex.result.stderr) + assertContains(ex.message.orEmpty(), "problem") + } + + @Test + @IgnoreOnUnix + fun `runProcess captures stdout and stderr on windows`() { + val result = runProcess(listOf("cmd.exe", "/c", "echo hello&&echo problem 1>&2")) + + assertEquals(0, result.exitCode) + assertContains(result.stdout, "hello") + assertContains(result.stderr, "problem") + } + + @Test + @IgnoreOnUnix + fun `runProcess can discard stderr on success on windows`() { + val result = runProcess( + listOf("cmd.exe", "/c", "echo hello&&echo problem 1>&2"), + stderrMode = ProcessStderrMode.DISCARD_ON_SUCCESS, + ) + + assertEquals(0, result.exitCode) + assertContains(result.stdout, "hello") + assertEquals("", result.stderr) + } + + @Test + @IgnoreOnUnix + fun `runProcess includes stderr on failure when discarding stderr on success on windows`() { + val ex = assertFailsWith { + runProcess( + listOf("cmd.exe", "/c", "echo problem 1>&2&&exit /b 5"), + stderrMode = ProcessStderrMode.DISCARD_ON_SUCCESS, + ) + } + + assertEquals(5, ex.result.exitCode) + assertContains(ex.result.stderr, "problem") + assertContains(ex.message.orEmpty(), "problem") + } + + @Test + @IgnoreOnUnix + fun `runProcess passes environment to child process on windows`() { + val result = runProcess( + listOf("cmd.exe", "/c", "echo token=%CODER_SESSION_TOKEN%"), + mapOf("CODER_SESSION_TOKEN" to "token"), + ) + + assertEquals("token=token", result.stdout.trim()) + } + + @Test + @IgnoreOnUnix + fun `runProcess accepts configured non-zero exit code on windows`() { + val result = runProcess( + listOf("cmd.exe", "/c", "echo expected failure&&exit /b 7"), + expectedExitCodes = 0..7, + ) + + assertEquals(7, result.exitCode) + assertContains(result.stdout, "expected failure") + } + + @Test + @IgnoreOnUnix + fun `runProcess redacts labeled token values in failure messages on windows`() { + val ex = assertFailsWith { + runProcess( + listOf("cmd.exe", "/c", "echo CODER_SESSION_TOKEN=%CODER_SESSION_TOKEN% 1>&2&&exit /b 7"), + mapOf("CODER_SESSION_TOKEN" to "super-secret-token"), + ) + } + + assertContains(ex.message.orEmpty(), "CODER_SESSION_TOKEN=") + assertFalse(ex.message.orEmpty().contains("super-secret-token")) + } + + @Test + fun `runProcess passes environment to child process`() { + val result = + if (getOS() == OS.WINDOWS) { + runProcess( + listOf("cmd.exe", "/c", "echo %CODER_SESSION_TOKEN%"), + mapOf("CODER_SESSION_TOKEN" to "token"), + ) + } else { + runProcess( + listOf("sh", "-c", "printf %s \"${'$'}CODER_SESSION_TOKEN\""), + mapOf("CODER_SESSION_TOKEN" to "token"), + ) + } + + assertEquals("token", result.stdout.trim()) + } + + @Test + fun `runProcess throws sanitized exception on unexpected exit`() { + val ex = + if (getOS() == OS.WINDOWS) { + assertFailsWith { + runProcess( + listOf("cmd.exe", "/c", "echo CODER_SESSION_TOKEN=%CODER_SESSION_TOKEN% 1>&2&&exit /b 7"), + mapOf("CODER_SESSION_TOKEN" to "super-secret-token"), + ) + } + } else { + assertFailsWith { + runProcess( + listOf("sh", "-c", "printf 'CODER_SESSION_TOKEN=%s' \"${'$'}CODER_SESSION_TOKEN\" >&2; exit 7"), + mapOf("CODER_SESSION_TOKEN" to "super-secret-token"), + ) + } + } + + assertEquals(7, ex.result.exitCode) + assertFalse(ex.message.orEmpty().contains("super-secret-token")) + } +} From be793d95bdf1ebf30c0309d3276778e965485086 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 20 May 2026 23:12:53 +0300 Subject: [PATCH 07/11] Keep CLI and API auth on the same persisted token Pass --use-token-as-session when logging the CLI in so the CLI persists the same token the plugin already uses for REST API calls. Without this flag, a supplied token is only used to bootstrap login and the CLI mints and stores a different session token of its own. That leaves the plugin and CLI using different credentials, which makes auth state harder to reason about and prevents Toolbox from managing a single shared token consistently. Using --use-token-as-session keeps both sides on the same credential, improves consistency between REST and CLI behavior, and opens the door to revoking that shared token cleanly from Toolbox later on. Tests were updated to reflect the new login command shape. --- CHANGELOG.md | 1 + src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt | 1 + src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt | 5 ++++- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14ef6de..386fdf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - reduce token exposure in process arguments and command logs - replaced the external process execution dependency with a lightweight internal runner, reducing plugin dependencies and improving error sanitization. +- Updated CLI login to keep REST client and CLI auth on the same persisted token, reducing credential drift. ## 0.9.0 - 2026-05-14 diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index e04dd5a..ec42de3 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -268,6 +268,7 @@ class CoderCLIManager( return exec( env = mapOf(CODER_SESSION_TOKEN_ENV_VAR to token), "login", + "--use-token-as-session", deploymentURL.toString(), "--global-config", coderConfigPath.toString(), diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index edddf10..835da3d 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -470,7 +470,10 @@ internal class CoderCLIManagerTest { val ccm = CoderCLIManager(context.copy(settingsStore = settings), URL("https://test.coder.com")) assertEquals(stdout, ccm.login(token).trim()) - assertContains(argsFile.toFile().readText(), "login https://test.coder.com --global-config") + assertContains( + argsFile.toFile().readText(), + "login --use-token-as-session https://test.coder.com --global-config" + ) assertFalse(argsFile.toFile().readText().contains("--token")) assertFalse(argsFile.toFile().readText().contains(token)) assertEquals(token, envFile.toFile().readText().trim()) From d164f81638909e71761aa4da75b8a1b633257db7 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 21 May 2026 00:30:20 +0300 Subject: [PATCH 08/11] Add opt-in keyring-backed CLI auth for supported platforms Add a new useKeyring setting so Toolbox can let the Coder CLI persist the shared session token in the OS keyring when that storage mode is actually supported. When enabled, Toolbox stops forcing --global-config for authenticated CLI commands only if the CLI supports keyring-backed auth and the platform is macOS or Windows. That keeps REST and CLI auth on the same persisted token without changing Linux behavior, where the plugin continues to use its deployment-specific config directory. --- CHANGELOG.md | 3 +- README.md | 28 +- .../com/coder/toolbox/cli/CoderCLIManager.kt | 49 ++- .../toolbox/settings/ReadOnlyCoderSettings.kt | 12 +- .../coder/toolbox/store/CoderSettingsStore.kt | 7 +- .../com/coder/toolbox/store/StoreKeys.kt | 2 + .../coder/toolbox/views/CoderSettingsPage.kt | 10 + .../resources/localization/defaultMessages.po | 5 +- .../coder/toolbox/cli/CoderCLIManagerTest.kt | 335 +++++++++++++++++- .../toolbox/settings/CoderSettingsTest.kt | 4 + 10 files changed, 421 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 386fdf4..c44227e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,8 @@ - reduce token exposure in process arguments and command logs - replaced the external process execution dependency with a lightweight internal runner, reducing plugin dependencies and improving error sanitization. -- Updated CLI login to keep REST client and CLI auth on the same persisted token, reducing credential drift. +- updated CLI login to keep REST client and CLI auth on the same persisted token, reducing credential drift. +- added an opt-in keyring-backed CLI login mode to securely store the session token on supported platforms. ## 0.9.0 - 2026-05-14 diff --git a/README.md b/README.md index 9bfde3e..a17f551 100644 --- a/README.md +++ b/README.md @@ -456,11 +456,16 @@ storage paths. The options can be configured from the plugin's main Workspaces p - `Data directory` directory where deployment-specific data such as session tokens and CLI binaries are stored. Each deployment gets a host-specific subdirectory (e.g. `coder.example.com`). Supports `~` and `$HOME` - expansion. + expansion. When keyring-backed CLI storage is enabled, the session token is no longer persisted in this directory. - `Header command` command that outputs additional HTTP headers. Each line of output must be in the format key=value. The environment variable CODER_URL will be available to the command process. +- `Store CLI session in OS keyring when supported (CLI >= 2.29.0)` is an opt-in setting to force the CLI to use its + default credential backend on supported platforms instead of forcing a custom `--global-config` login path. + On macOS and Windows, that allows newer CLIs to persist the token in the OS keyring. On Linux, we continue to use + the plugin-managed `--global-config` flow even if this setting is enabled. Disabled by default. + - `lastDeploymentURL` the last Coder deployment URL that Coder Toolbox successfully authenticated to. - `workspaceViewUrl` specifies the dashboard page full URL where users can view details about a workspace. @@ -490,6 +495,27 @@ Once the binary location is resolved: 3. If **downloads are disabled** and the CLI exists but its version does not match, the stale CLI is used with a warning. If no CLI exists at all, an error is raised. +#### How keyring-backed CLI login works + +When **Store CLI session in OS keyring when supported (CLI >= 2.29.0)** is disabled, Toolbox logs the CLI in with a +deployment-specific `--global-config` directory under the configured data directory. That keeps the CLI session +isolated inside the plugin-managed config tree. + +When the setting is enabled, Toolbox still logs in with `CODER_SESSION_TOKEN` and `--use-token-as-session`, but it +stops forcing `--global-config` for authenticated CLI commands and relies on deployment URL-based auth resolution +instead. On supported platforms, this lets newer Coder CLIs use their default OS-backed storage instead of the +plugin-managed config directory. + +This matters for two reasons: + +- the CLI persists the same token that Toolbox already uses for REST API calls, instead of minting a separate token +- Toolbox can move toward managing one shared credential consistently across REST and CLI flows, including future + revoke/logout behavior + +Keyring-backed storage requires Coder CLI `2.29.0` or newer and is only supported on macOS and Windows. The setting is +opt-in and defaults to disabled. On Linux, enabling the setting does not switch the +CLI to keyring-backed auth; Toolbox keeps using the deployment-specific `--global-config` directory. + ### TLS settings The following options control the secure communication behavior of the plugin with Coder deployment and its available diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index ec42de3..3bfb457 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -17,9 +17,11 @@ import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent import com.coder.toolbox.settings.SignatureFallbackStrategy.ALLOW import com.coder.toolbox.util.InvalidVersionException +import com.coder.toolbox.util.OS import com.coder.toolbox.util.SemVer import com.coder.toolbox.util.escape import com.coder.toolbox.util.escapeSubcommand +import com.coder.toolbox.util.getOS import com.coder.toolbox.util.runProcess import com.coder.toolbox.util.safeHost import com.coder.toolbox.util.sanitizeSecrets @@ -105,6 +107,7 @@ data class Features( val reportWorkspaceUsage: Boolean = false, val wildcardSsh: Boolean = false, val buildReason: Boolean = false, + val keyringAuth: Boolean = false, ) /** @@ -113,7 +116,8 @@ data class Features( class CoderCLIManager( private val context: CoderToolboxContext, // The URL of the deployment this CLI is for. - private val deploymentURL: URL + private val deploymentURL: URL, + private val currentOs: OS? = getOS(), ) { private val downloader = createDownloadService() private val gpgVerifier = GPGVerifier(context) @@ -261,17 +265,24 @@ class CoderCLIManager( } /** - * Use the provided token to initializeSession the CLI. + * Use the provided token to initialize the CLI. + * + * When keyring storage is enabled and supported, omit --global-config so supported CLIs + * can select their default OS-backed credential storage. This only applies + * on macOS and Windows. */ - fun login(token: String): String { - context.logger.info("Storing CLI credentials in $coderConfigPath") - return exec( - env = mapOf(CODER_SESSION_TOKEN_ENV_VAR to token), + fun login(token: String, feats: Features = features): String { + val args = mutableListOf( "login", "--use-token-as-session", deploymentURL.toString(), - "--global-config", - coderConfigPath.toString(), + ) + if (!shouldUseKeyringAuth(feats)) { + args.addAll(globalConfigArgs()) + } + return exec( + env = mapOf(CODER_SESSION_TOKEN_ENV_VAR to token), + *args.toTypedArray(), ) } @@ -280,8 +291,7 @@ class CoderCLIManager( */ fun startWorkspace(workspaceOwner: String, workspaceName: String, feats: Features = features): String { val args = mutableListOf( - "--global-config", - coderConfigPath.toString(), + *workspaceAuthArgs(feats).toTypedArray(), "start", "--yes", "$workspaceOwner/$workspaceName" @@ -337,8 +347,8 @@ class CoderCLIManager( val baseArgs = listOfNotNull( escape(localBinaryPath.toString()), - "--global-config", - escape(coderConfigPath.toString()), + if (!shouldUseKeyringAuth(feats)) "--global-config" else null, + if (!shouldUseKeyringAuth(feats)) escape(coderConfigPath.toString()) else null, // CODER_URL might be set, and it will override the URL file in // the config directory, so override that here to make sure we // always use the correct URL. @@ -561,6 +571,7 @@ class CoderCLIManager( reportWorkspaceUsage = version >= SemVer(2, 13, 0), wildcardSsh = version >= SemVer(2, 19, 0), buildReason = version >= SemVer(2, 25, 0), + keyringAuth = version >= SemVer(2, 29, 0), ) } } @@ -576,6 +587,8 @@ class CoderCLIManager( companion object { private const val CODER_SESSION_TOKEN_ENV_VAR = "CODER_SESSION_TOKEN" + internal fun supportsKeyringStorage(os: OS?): Boolean = os == OS.MAC || os == OS.WINDOWS + private fun getHostnamePrefix(url: URL): String = "coder-jetbrains-toolbox-${url.safeHost()}" private fun getWsByOwner(ws: Workspace, agent: WorkspaceAgent): String = @@ -585,4 +598,16 @@ class CoderCLIManager( private fun Pair.agent() = this.second } + + private fun globalConfigArgs(): List = listOf("--global-config", coderConfigPath.toString()) + + private fun workspaceAuthArgs(feats: Features): List = + if (shouldUseKeyringAuth(feats)) { + listOf("--url", deploymentURL.toString()) + } else { + globalConfigArgs() + } + + private fun shouldUseKeyringAuth(feats: Features): Boolean = + context.settingsStore.useKeyring && feats.keyringAuth && supportsKeyringStorage(currentOs) } diff --git a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt index 2f8d82a..e58b6a4 100644 --- a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt +++ b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt @@ -70,7 +70,8 @@ interface ReadOnlyCoderSettings { /** * Where to save plugin data like the Coder binary (if not configured with - * binaryDestination) and the deployment URL and session token. + * binaryDestination) and the deployment URL. When keyring storage is not + * enabled, this also stores the CLI session token. */ val dataDirectory: String? @@ -98,6 +99,13 @@ interface ReadOnlyCoderSettings { */ val headerCommand: String? + /** + * Whether CLI login should allow the Coder CLI to persist the session in + * the operating system keyring when supported. This only takes effect on + * macOS and Windows. + */ + val useKeyring: Boolean + /** * Optional TLS settings */ @@ -286,4 +294,4 @@ enum class HttpLoggingVerbosity { else -> NONE } } -} \ No newline at end of file +} diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index ab6ea94..7ad0355 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -55,6 +55,7 @@ class CoderSettingsStore( override val globalConfigDir: String get() = getDefaultGlobalConfigDir().normalize().toString() override val enableDownloads: Boolean get() = store[ENABLE_DOWNLOADS]?.toBooleanStrictOrNull() ?: true override val headerCommand: String? get() = store[HEADER_COMMAND] + override val useKeyring: Boolean get() = store[USE_KEYRING]?.toBooleanStrictOrNull() ?: false override val tls: ReadOnlyTLSSettings get() = TLSSettings( certPath = store[TLS_CERT_PATH], @@ -214,6 +215,10 @@ class CoderSettingsStore( store[HEADER_COMMAND] = cmd } + fun updateUseKeyring(useKeyring: Boolean) { + store[USE_KEYRING] = useKeyring.toString() + } + fun updateCertPath(path: String) { store[TLS_CERT_PATH] = path } @@ -365,4 +370,4 @@ class CoderSettingsStore( val host = if (url.port > 0) "${url.safeHost()}-${url.port}" else url.safeHost() return path.resolve(host) } -} \ No newline at end of file +} diff --git a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt index 69fea40..4718cd1 100644 --- a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt +++ b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt @@ -27,6 +27,8 @@ internal const val ENABLE_DOWNLOADS = "enableDownloads" internal const val HEADER_COMMAND = "headerCommand" +internal const val USE_KEYRING = "useKeyring" + internal const val TLS_CERT_PATH = "tlsCertPath" internal const val TLS_KEY_PATH = "tlsKeyPath" diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt index 934c5ec..c359a40 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt @@ -77,6 +77,10 @@ class CoderSettingsPage( settings.headerCommand ?: "", TextType.General ) + private val useKeyringField = CheckboxField( + settings.useKeyring, + context.i18n.ptrl("Store CLI session in OS keyring when supported (CLI >= 2.29.0)") + ) private val tlsCertPathField = TextField( context.i18n.ptrl("TLS cert path"), @@ -142,6 +146,7 @@ class CoderSettingsPage( false, listOf( preferOAuth2IfAvailableField, + useKeyringField, headerCommandField, tlsCertPathField, tlsKeyPathField, @@ -188,6 +193,7 @@ class CoderSettingsPage( updateSignatureFallbackStrategy(signatureFallbackStrategyField.checkedState.value) updateHttpClientLogLevel(httpLoggingField.selectedValueState.value) updateHeaderCommand(headerCommandField.contentState.value) + updateUseKeyring(useKeyringField.checkedState.value) updatePreferAuthViaOAuth2(preferOAuth2IfAvailableField.checkedState.value) updateCertPath(tlsCertPathField.contentState.value) updateKeyPath(tlsKeyPathField.contentState.value) @@ -244,6 +250,10 @@ class CoderSettingsPage( settings.headerCommand ?: "" } + useKeyringField.checkedState.update { + settings.useKeyring + } + preferOAuth2IfAvailableField.checkedState.update { settings.preferOAuth2IfAvailable } diff --git a/src/main/resources/localization/defaultMessages.po b/src/main/resources/localization/defaultMessages.po index cfc3b0a..038a39c 100644 --- a/src/main/resources/localization/defaultMessages.po +++ b/src/main/resources/localization/defaultMessages.po @@ -85,6 +85,9 @@ msgstr "" msgid "Header command" msgstr "" +msgid "Store CLI session in OS keyring when supported (CLI >= 2.29.0)" +msgstr "" + msgid "TLS cert path" msgstr "" @@ -197,4 +200,4 @@ msgid "Unstable connection between Coder server and workspace detected. Your act msgstr "" msgid "Prefer OAuth2 if available over authentication via API Key" -msgstr "" \ No newline at end of file +msgstr "" diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index 835da3d..b4be8fd 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -22,6 +22,7 @@ import com.coder.toolbox.store.NETWORK_INFO_DIR import com.coder.toolbox.store.SSH_CONFIG_OPTIONS import com.coder.toolbox.store.SSH_CONFIG_PATH import com.coder.toolbox.store.SSH_LOG_DIR +import com.coder.toolbox.store.USE_KEYRING import com.coder.toolbox.util.ConnectionMonitoringService import com.coder.toolbox.util.InvalidVersionException import com.coder.toolbox.util.OS @@ -433,11 +434,17 @@ internal class CoderCLIManagerTest { ) } - @Test - fun `login passes token via environment instead of argv`() { - val binaryFile = tmpdir.resolve("login-env").resolve(if (getOS() == OS.WINDOWS) "coder.bat" else "coder") - val argsFile = tmpdir.resolve("login-env").resolve("argv.txt") - val envFile = tmpdir.resolve("login-env").resolve("env.txt") + private fun assertLoginUsesExpectedArgs( + extraSettings: Array>, + expectedArgsSubstring: String, + expectGlobalConfig: Boolean, + feats: Features = Features(), + osOverride: OS? = getOS(), + ) { + val binaryFile = tmpdir.resolve("login-env-${UUID.randomUUID()}") + .resolve(if (getOS() == OS.WINDOWS) "coder.bat" else "coder") + val argsFile = binaryFile.parent.resolve("argv.txt") + val envFile = binaryFile.parent.resolve("env.txt") val token = "super-secret-token" val stdout = "login ok" val script = if (getOS() == OS.WINDOWS) { @@ -463,20 +470,202 @@ internal class CoderCLIManagerTest { pluginTestSettingsStore( BINARY_DESTINATION to binaryFile.toString(), ENABLE_DOWNLOADS to "false", + *extraSettings, + ), + Environment(), + logger + ) + val ccm = CoderCLIManager( + context.copy(settingsStore = settings), + URL("https://test.coder.com"), + osOverride, + ) + + assertEquals(stdout, ccm.login(token, feats).trim()) + val args = argsFile.toFile().readText() + assertContains(args, expectedArgsSubstring) + assertEquals(expectGlobalConfig, args.contains("--global-config")) + assertFalse(args.contains("--token")) + assertFalse(args.contains(token)) + assertEquals(token, envFile.toFile().readText().trim()) + } + + @Test + fun `login passes token via environment and uses global config by default`() { + assertLoginUsesExpectedArgs( + extraSettings = emptyArray(), + expectedArgsSubstring = "login --use-token-as-session https://test.coder.com --global-config", + expectGlobalConfig = true, + ) + } + + @Test + fun `login passes token via environment and uses global config when keyring is disabled`() { + assertLoginUsesExpectedArgs( + extraSettings = arrayOf(USE_KEYRING to "false"), + expectedArgsSubstring = "login --use-token-as-session https://test.coder.com --global-config", + expectGlobalConfig = true, + ) + } + + @Test + fun `login only skips global config when keyring is enabled and runtime supports it`() { + val keyringSupported = CoderCLIManager.supportsKeyringStorage(getOS()) + assertLoginUsesExpectedArgs( + extraSettings = arrayOf(USE_KEYRING to "true"), + expectedArgsSubstring = "login --use-token-as-session https://test.coder.com", + expectGlobalConfig = !keyringSupported, + feats = Features(keyringAuth = true), + ) + } + + @Test + fun `login keeps global config when keyring is enabled but CLI does not support it`() { + assertLoginUsesExpectedArgs( + extraSettings = arrayOf(USE_KEYRING to "true"), + expectedArgsSubstring = "login --use-token-as-session https://test.coder.com --global-config", + expectGlobalConfig = true, + ) + } + + @Test + fun `login keeps global config on linux even when keyring is enabled and CLI supports it`() { + assertLoginUsesExpectedArgs( + extraSettings = arrayOf(USE_KEYRING to "true"), + expectedArgsSubstring = "login --use-token-as-session https://test.coder.com --global-config", + expectGlobalConfig = true, + feats = Features(keyringAuth = true), + osOverride = OS.LINUX, + ) + } + + @Test + fun `start workspace only uses url auth when keyring is enabled and runtime supports it`() { + val binaryFile = tmpdir.resolve("start-workspace-${UUID.randomUUID()}") + .resolve(if (getOS() == OS.WINDOWS) "coder.bat" else "coder") + val argsFile = binaryFile.parent.resolve("argv.txt") + val stdout = "start ok" + val script = if (getOS() == OS.WINDOWS) { + mkbin( + """ + echo %* > "${argsFile.toAbsolutePath()}" + echo $stdout + """.trimIndent() + ) + } else { + mkbin( + """ + printf '%s\n' "$*" > '${argsFile.toAbsolutePath()}' + printf '$stdout\n' + """.trimIndent() + ) + } + writeExecutable(binaryFile, script) + + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binaryFile.toString(), + ENABLE_DOWNLOADS to "false", + USE_KEYRING to "true", ), Environment(), logger ) val ccm = CoderCLIManager(context.copy(settingsStore = settings), URL("https://test.coder.com")) - assertEquals(stdout, ccm.login(token).trim()) - assertContains( - argsFile.toFile().readText(), - "login --use-token-as-session https://test.coder.com --global-config" + val keyringSupported = CoderCLIManager.supportsKeyringStorage(getOS()) + assertEquals(stdout, ccm.startWorkspace("alice", "dev", Features(keyringAuth = true)).trim()) + val args = argsFile.toFile().readText() + if (keyringSupported) { + assertContains(args, "--url https://test.coder.com start --yes alice/dev") + assertFalse(args.contains("--global-config")) + } else { + assertContains(args, "--global-config") + } + } + + @Test + fun `start workspace keeps global config when keyring is enabled but CLI does not support it`() { + val binaryFile = tmpdir.resolve("start-workspace-${UUID.randomUUID()}") + .resolve(if (getOS() == OS.WINDOWS) "coder.bat" else "coder") + val argsFile = binaryFile.parent.resolve("argv.txt") + val stdout = "start ok" + val script = if (getOS() == OS.WINDOWS) { + mkbin( + """ + echo %* > "${argsFile.toAbsolutePath()}" + echo $stdout + """.trimIndent() + ) + } else { + mkbin( + """ + printf '%s\n' "$*" > '${argsFile.toAbsolutePath()}' + printf '$stdout\n' + """.trimIndent() + ) + } + writeExecutable(binaryFile, script) + + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binaryFile.toString(), + ENABLE_DOWNLOADS to "false", + USE_KEYRING to "true", + ), + Environment(), + logger ) - assertFalse(argsFile.toFile().readText().contains("--token")) - assertFalse(argsFile.toFile().readText().contains(token)) - assertEquals(token, envFile.toFile().readText().trim()) + val ccm = CoderCLIManager(context.copy(settingsStore = settings), URL("https://test.coder.com")) + + assertEquals(stdout, ccm.startWorkspace("alice", "dev", Features()).trim()) + val args = argsFile.toFile().readText() + assertContains(args, "--global-config") + assertFalse(args.contains("--url https://test.coder.com start --yes alice/dev")) + } + + @Test + fun `start workspace keeps global config on linux even when keyring is enabled and CLI supports it`() { + val binaryFile = tmpdir.resolve("start-workspace-${UUID.randomUUID()}") + .resolve(if (getOS() == OS.WINDOWS) "coder.bat" else "coder") + val argsFile = binaryFile.parent.resolve("argv.txt") + val stdout = "start ok" + val script = if (getOS() == OS.WINDOWS) { + mkbin( + """ + echo %* > "${argsFile.toAbsolutePath()}" + echo $stdout + """.trimIndent() + ) + } else { + mkbin( + """ + printf '%s\n' "$*" > '${argsFile.toAbsolutePath()}' + printf '$stdout\n' + """.trimIndent() + ) + } + writeExecutable(binaryFile, script) + + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binaryFile.toString(), + ENABLE_DOWNLOADS to "false", + USE_KEYRING to "true", + ), + Environment(), + logger + ) + val ccm = CoderCLIManager( + context.copy(settingsStore = settings), + URL("https://test.coder.com"), + OS.LINUX, + ) + + assertEquals(stdout, ccm.startWorkspace("alice", "dev", Features(keyringAuth = true)).trim()) + val args = argsFile.toFile().readText() + assertContains(args, "--global-config") + assertFalse(args.contains("--url https://test.coder.com start --yes alice/dev")) } @Test @@ -803,6 +992,101 @@ internal class CoderCLIManagerTest { } } + @Test + fun `ssh config only uses url auth when keyring is enabled and runtime supports it`() { + val workspace = workspace("foo", agents = mapOf("agent1" to UUID.randomUUID().toString())) + val settings = CoderSettingsStore( + pluginTestSettingsStore( + USE_KEYRING to "true", + SSH_CONFIG_PATH to tmpdir.resolve("keyring-ssh.conf").toString(), + DATA_DIRECTORY to tmpdir.resolve("keyring-ssh-data").toString(), + NETWORK_INFO_DIR to tmpdir.resolve("keyring-network-info").toString(), + ), + Environment(), + context.logger + ) + val ccm = CoderCLIManager( + context.copy(settingsStore = settings), + URI.create("https://test.coder.invalid").toURL() + ) + + ccm.configSsh( + workspace.latestBuild.resources + .filter { it.agents != null } + .flatMap { resource -> resource.agents!!.map { agent -> workspace to agent } } + .toSet(), + Features(reportWorkspaceUsage = true, keyringAuth = true), + ) + + val keyringSupported = CoderCLIManager.supportsKeyringStorage(getOS()) + val sshConfig = Path.of(settings.sshConfigPath).toFile().readText() + assertContains(sshConfig, "--url https://test.coder.invalid") + assertEquals(!keyringSupported, sshConfig.contains("--global-config")) + } + + @Test + fun `ssh config keeps global config when keyring is enabled but CLI does not support it`() { + val workspace = workspace("foo", agents = mapOf("agent1" to UUID.randomUUID().toString())) + val settings = CoderSettingsStore( + pluginTestSettingsStore( + USE_KEYRING to "true", + SSH_CONFIG_PATH to tmpdir.resolve("keyring-ssh-unsupported.conf").toString(), + DATA_DIRECTORY to tmpdir.resolve("keyring-ssh-unsupported-data").toString(), + NETWORK_INFO_DIR to tmpdir.resolve("keyring-ssh-unsupported-network-info").toString(), + ), + Environment(), + context.logger + ) + val ccm = CoderCLIManager( + context.copy(settingsStore = settings), + URI.create("https://test.coder.invalid").toURL() + ) + + ccm.configSsh( + workspace.latestBuild.resources + .filter { it.agents != null } + .flatMap { resource -> resource.agents!!.map { agent -> workspace to agent } } + .toSet(), + Features(reportWorkspaceUsage = true), + ) + + val sshConfig = Path.of(settings.sshConfigPath).toFile().readText() + assertContains(sshConfig, "--global-config") + assertContains(sshConfig, "--url https://test.coder.invalid") + } + + @Test + fun `ssh config keeps global config on linux even when keyring is enabled and CLI supports it`() { + val workspace = workspace("foo", agents = mapOf("agent1" to UUID.randomUUID().toString())) + val settings = CoderSettingsStore( + pluginTestSettingsStore( + USE_KEYRING to "true", + SSH_CONFIG_PATH to tmpdir.resolve("keyring-linux-ssh.conf").toString(), + DATA_DIRECTORY to tmpdir.resolve("keyring-linux-ssh-data").toString(), + NETWORK_INFO_DIR to tmpdir.resolve("keyring-linux-network-info").toString(), + ), + Environment(), + context.logger + ) + val ccm = CoderCLIManager( + context.copy(settingsStore = settings), + URI.create("https://test.coder.invalid").toURL(), + OS.LINUX, + ) + + ccm.configSsh( + workspace.latestBuild.resources + .filter { it.agents != null } + .flatMap { resource -> resource.agents!!.map { agent -> workspace to agent } } + .toSet(), + Features(reportWorkspaceUsage = true, keyringAuth = true), + ) + + val sshConfig = Path.of(settings.sshConfigPath).toFile().readText() + assertContains(sshConfig, "--global-config") + assertContains(sshConfig, "--url https://test.coder.invalid") + } + @Test fun testMalformedHeader() { val tests = @@ -983,7 +1267,7 @@ internal class CoderCLIManagerTest { fun testFeatures() { val tests = listOf( - Pair("2.5.0", Features(true)), + Pair("2.5.0", Features(disableAutostart = true)), Pair("2.13.0", Features(disableAutostart = true, reportWorkspaceUsage = true)), Pair( "2.25.0", @@ -999,11 +1283,22 @@ internal class CoderCLIManagerTest { disableAutostart = true, reportWorkspaceUsage = true, wildcardSsh = true, - buildReason = true + buildReason = true, + keyringAuth = true, ) ), - Pair("2.4.9", Features(false)), - Pair("1.0.1", Features(false)), + Pair( + "2.29.0", + Features( + disableAutostart = true, + reportWorkspaceUsage = true, + wildcardSsh = true, + buildReason = true, + keyringAuth = true, + ) + ), + Pair("2.4.9", Features()), + Pair("1.0.1", Features()), ) tests.forEach { @@ -1032,6 +1327,14 @@ internal class CoderCLIManagerTest { } } + @Test + fun `keyring storage is only supported on mac and windows`() { + assertTrue(CoderCLIManager.supportsKeyringStorage(OS.MAC)) + assertTrue(CoderCLIManager.supportsKeyringStorage(OS.WINDOWS)) + assertFalse(CoderCLIManager.supportsKeyringStorage(OS.LINUX)) + assertFalse(CoderCLIManager.supportsKeyringStorage(null)) + } + companion object { private val tmpdir: Path = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-toolbox-test/cli-manager") diff --git a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt index fa4da2d..4a89a58 100644 --- a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt +++ b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt @@ -11,6 +11,7 @@ import com.coder.toolbox.store.TLS_ALTERNATE_HOSTNAME import com.coder.toolbox.store.TLS_CA_PATH import com.coder.toolbox.store.TLS_CERT_PATH import com.coder.toolbox.store.TLS_KEY_PATH +import com.coder.toolbox.store.USE_KEYRING import com.coder.toolbox.util.OS import com.coder.toolbox.util.getOS import com.coder.toolbox.util.pluginTestSettingsStore @@ -275,6 +276,7 @@ internal class CoderSettingsTest { val settings = CoderSettingsStore(pluginTestSettingsStore(), Environment(), logger) assertEquals(true, settings.readOnly().enableDownloads) assertEquals(null, settings.readOnly().headerCommand) + assertEquals(false, settings.readOnly().useKeyring) assertEquals(null, settings.readOnly().tls.certPath) assertEquals(null, settings.readOnly().tls.keyPath) assertEquals(null, settings.readOnly().tls.caPath) @@ -290,6 +292,7 @@ internal class CoderSettingsTest { pluginTestSettingsStore( ENABLE_DOWNLOADS to false.toString(), HEADER_COMMAND to "test header", + USE_KEYRING to true.toString(), TLS_CERT_PATH to "tls cert path", TLS_KEY_PATH to "tls key path", TLS_CA_PATH to "tls ca path", @@ -303,6 +306,7 @@ internal class CoderSettingsTest { assertEquals(false, settings.readOnly().enableDownloads) assertEquals("test header", settings.readOnly().headerCommand) + assertEquals(true, settings.readOnly().useKeyring) assertEquals("tls cert path", settings.readOnly().tls.certPath) assertEquals("tls key path", settings.readOnly().tls.keyPath) assertEquals("tls ca path", settings.readOnly().tls.caPath) From a715f68f848d73ec20bd973923f8cec5e6ff5b22 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 21 May 2026 21:36:36 +0300 Subject: [PATCH 09/11] warn the user when keyring is enabled but not supported --- .../com/coder/toolbox/cli/CoderCLIManager.kt | 23 +++++ .../coder/toolbox/cli/CoderCLIManagerTest.kt | 91 +++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 3bfb457..57dae16 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -30,6 +30,7 @@ import com.squareup.moshi.JsonClass import com.squareup.moshi.JsonDataException import com.squareup.moshi.Moshi import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext import retrofit2.Retrofit import java.io.EOFException @@ -272,6 +273,7 @@ class CoderCLIManager( * on macOS and Windows. */ fun login(token: String, feats: Features = features): String { + maybeWarnAboutKeyringFallback(feats) val args = mutableListOf( "login", "--use-token-as-session", @@ -290,6 +292,7 @@ class CoderCLIManager( * Start a workspace. Throws if the command execution fails. */ fun startWorkspace(workspaceOwner: String, workspaceName: String, feats: Features = features): String { + maybeWarnAboutKeyringFallback(feats) val args = mutableListOf( *workspaceAuthArgs(feats).toTypedArray(), "start", @@ -610,4 +613,24 @@ class CoderCLIManager( private fun shouldUseKeyringAuth(feats: Features): Boolean = context.settingsStore.useKeyring && feats.keyringAuth && supportsKeyringStorage(currentOs) + + private fun maybeWarnAboutKeyringFallback(feats: Features) { + if (!context.settingsStore.useKeyring || shouldUseKeyringAuth(feats)) { + return + } + + val warning = when { + !supportsKeyringStorage(currentOs) -> + "OS keyring storage is enabled, but keyring-backed CLI auth is only supported on macOS and Windows. Falling back to file-based CLI storage." + + !feats.keyringAuth -> + "OS keyring storage is enabled, but the installed Coder CLI does not support keyring-backed auth. Coder CLI 2.29.0 or newer is required. Falling back to file-based CLI storage." + + else -> null + } ?: return + + runBlocking { + context.logAndShowWarning("Keyring storage unavailable", warning) + } + } } diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index b4be8fd..b7a476c 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -47,6 +47,7 @@ import com.jetbrains.toolbox.api.ui.ToolboxUi import com.squareup.moshi.JsonEncodingException import com.sun.net.httpserver.HttpServer import io.mockk.coEvery +import io.mockk.coVerify import io.mockk.mockk import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.runBlocking @@ -539,6 +540,96 @@ internal class CoderCLIManagerTest { ) } + @Test + fun `login warns when keyring is enabled but CLI does not support it`() { + val binaryFile = tmpdir.resolve("login-warning-cli-${UUID.randomUUID()}") + .resolve(if (getOS() == OS.WINDOWS) "coder.bat" else "coder") + writeExecutable(binaryFile, mkbin(echo("ok"))) + + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binaryFile.toString(), + ENABLE_DOWNLOADS to "false", + USE_KEYRING to "true", + ), + Environment(), + logger + ) + val ccm = CoderCLIManager( + context.copy(settingsStore = settings), + URL("https://test.coder.com"), + ) + + ccm.login("super-secret-token", Features()) + + coVerify(exactly = 1) { ui.showSnackbar(any(), any(), any(), any()) } + } + + @Test + fun `login warns when keyring is enabled but OS does not support it`() { + val binaryFile = tmpdir.resolve("login-warning-os-${UUID.randomUUID()}") + .resolve(if (getOS() == OS.WINDOWS) "coder.bat" else "coder") + writeExecutable(binaryFile, mkbin(echo("ok"))) + + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binaryFile.toString(), + ENABLE_DOWNLOADS to "false", + USE_KEYRING to "true", + ), + Environment(), + logger + ) + val ccm = CoderCLIManager( + context.copy(settingsStore = settings), + URL("https://test.coder.com"), + OS.LINUX, + ) + + ccm.login("super-secret-token", Features(keyringAuth = true)) + + coVerify(exactly = 1) { ui.showSnackbar(any(), any(), any(), any()) } + } + + @Test + fun `start workspace warns when keyring is enabled but CLI does not support it`() { + val binaryFile = tmpdir.resolve("start-warning-cli-${UUID.randomUUID()}") + .resolve(if (getOS() == OS.WINDOWS) "coder.bat" else "coder") + val stdout = "start ok" + val script = if (getOS() == OS.WINDOWS) { + mkbin( + """ + echo $stdout + """.trimIndent() + ) + } else { + mkbin( + """ + printf '$stdout\n' + """.trimIndent() + ) + } + writeExecutable(binaryFile, script) + + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binaryFile.toString(), + ENABLE_DOWNLOADS to "false", + USE_KEYRING to "true", + ), + Environment(), + logger + ) + val ccm = CoderCLIManager( + context.copy(settingsStore = settings), + URL("https://test.coder.com"), + ) + + assertEquals(stdout, ccm.startWorkspace("alice", "dev", Features()).trim()) + + coVerify(exactly = 1) { ui.showSnackbar(any(), any(), any(), any()) } + } + @Test fun `start workspace only uses url auth when keyring is enabled and runtime supports it`() { val binaryFile = tmpdir.resolve("start-workspace-${UUID.randomUUID()}") From 6cd4e6d2deb984390563ec05935b15d04b6faa61 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 21 May 2026 21:40:15 +0300 Subject: [PATCH 10/11] Hide the keyring setting on unsupported operating systems --- .../kotlin/com/coder/toolbox/views/CoderSettingsPage.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt index c359a40..80834ae 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt @@ -5,6 +5,8 @@ import com.coder.toolbox.settings.HttpLoggingVerbosity.BASIC import com.coder.toolbox.settings.HttpLoggingVerbosity.BODY import com.coder.toolbox.settings.HttpLoggingVerbosity.HEADERS import com.coder.toolbox.settings.HttpLoggingVerbosity.NONE +import com.coder.toolbox.util.OS +import com.coder.toolbox.util.getOS import com.jetbrains.toolbox.api.ui.actions.RunnableActionDescription import com.jetbrains.toolbox.api.ui.components.CheckboxField import com.jetbrains.toolbox.api.ui.components.ComboBoxField @@ -35,6 +37,7 @@ class CoderSettingsPage( ) : CoderPage(MutableStateFlow(context.i18n.ptrl("Coder Settings")), false) { private val settings = context.settingsStore.readOnly() + private val shouldShowKeyringField = getOS() == OS.MAC || getOS() == OS.WINDOWS private val preferOAuth2IfAvailableField = CheckboxField( context.settingsStore.preferOAuth2IfAvailable, @@ -144,9 +147,9 @@ class CoderSettingsPage( SectionField( "Security & Authentication", false, - listOf( + listOfNotNull( preferOAuth2IfAvailableField, - useKeyringField, + useKeyringField.takeIf { shouldShowKeyringField }, headerCommandField, tlsCertPathField, tlsKeyPathField, From 7ae1d31228911ce3e09040330a6bfb74fc2defac Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 21 May 2026 23:52:28 +0300 Subject: [PATCH 11/11] Update the ssh config as soon as useKeyring setting changed --- src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt index 80834ae..8a409e7 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt @@ -206,9 +206,11 @@ class CoderSettingsPage( val sshWildcardEnabled = enableSshWildCardConfig.checkedState.value val sshTimeout = sshConnectionTimeoutField.contentState.value.toInt() + val useKeyring = useKeyringField.checkedState.value val sshSettingsChanged = sshWildcardEnabled != settings.isSshWildcardConfigEnabled || - sshTimeout != settings.sshConnectionTimeoutInSeconds + sshTimeout != settings.sshConnectionTimeoutInSeconds || + useKeyring != settings.useKeyring updateEnableSshWildcardConfig(sshWildcardEnabled) updateSshConnectionTimeoutInSeconds(sshTimeout)