diff --git a/CHANGELOG.md b/CHANGELOG.md index 62cf35c..c44227e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +### 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. +- 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 ### Added diff --git a/README.md b/README.md index cd598a5..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 @@ -539,6 +565,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/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 eba6b23..57dae16 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -17,17 +17,21 @@ 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 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.runBlocking import kotlinx.coroutines.withContext -import org.zeroturnaround.exec.ProcessExecutor import retrofit2.Retrofit import java.io.EOFException import java.io.FileNotFoundException @@ -104,6 +108,7 @@ data class Features( val reportWorkspaceUsage: Boolean = false, val wildcardSsh: Boolean = false, val buildReason: Boolean = false, + val keyringAuth: Boolean = false, ) /** @@ -112,7 +117,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) @@ -260,17 +266,25 @@ 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( + fun login(token: String, feats: Features = features): String { + maybeWarnAboutKeyringFallback(feats) + val args = mutableListOf( "login", + "--use-token-as-session", deploymentURL.toString(), - "--token", - token, - "--global-config", - coderConfigPath.toString(), + ) + if (!shouldUseKeyringAuth(feats)) { + args.addAll(globalConfigArgs()) + } + return exec( + env = mapOf(CODER_SESSION_TOKEN_ENV_VAR to token), + *args.toTypedArray(), ) } @@ -278,9 +292,9 @@ 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( - "--global-config", - coderConfigPath.toString(), + *workspaceAuthArgs(feats).toTypedArray(), "start", "--yes", "$workspaceOwner/$workspaceName" @@ -336,8 +350,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. @@ -534,17 +548,18 @@ 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 command = listOf(localBinaryPath.toString(), *args) + val processEnv = buildMap { + context.settingsStore.headerCommand?.let { put("CODER_HEADER_COMMAND", it) } + putAll(env) + } + + val stdout = runProcess(command, environment = processEnv).stdout + val sanitizedStdout = stdout.sanitizeSecrets() + context.logger.info("`$localBinaryPath ${listOf(*args).joinToString(" ")}`: $sanitizedStdout") return stdout } @@ -559,6 +574,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), ) } } @@ -572,7 +588,9 @@ class CoderCLIManager( } companion object { - private val tokenRegex = "--token [^ ]+".toRegex() + 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()}" @@ -583,4 +601,36 @@ 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) + + 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/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/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/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/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt index 934c5ec..8a409e7 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, @@ -77,6 +80,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"), @@ -140,8 +147,9 @@ class CoderSettingsPage( SectionField( "Security & Authentication", false, - listOf( + listOfNotNull( preferOAuth2IfAvailableField, + useKeyringField.takeIf { shouldShowKeyringField }, headerCommandField, tlsCertPathField, tlsKeyPathField, @@ -188,6 +196,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) @@ -197,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) @@ -244,6 +255,10 @@ class CoderSettingsPage( settings.headerCommand ?: "" } + useKeyringField.checkedState.update { + settings.useKeyring + } + preferOAuth2IfAvailableField.checkedState.update { settings.preferOAuth2IfAvailable } 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" } ] 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 bcd321f..b7a476c 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -22,9 +22,12 @@ 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 +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 @@ -44,13 +47,12 @@ 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 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 @@ -74,6 +76,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 +85,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 +122,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. */ @@ -349,7 +360,7 @@ internal class CoderCLIManagerTest { // Make sure login failures propagate. assertFailsWith( - exceptionClass = InvalidExitValueException::class, + exceptionClass = ProcessExitException::class, block = { ccm.login("jetbrains-ci-test") }, ) } @@ -419,11 +430,335 @@ internal class CoderCLIManagerTest { ) assertFailsWith( - exceptionClass = ProcessInitException::class, + exceptionClass = ProcessExecutionException::class, block = { ccm.login("fake-token") }, ) } + 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) { + 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", + *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 `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()}") + .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")) + + 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 + ) + 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 fun testOverwritesWrongVersion() { val (srv, url) = mockServer() @@ -748,6 +1083,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 = @@ -807,14 +1237,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( @@ -928,7 +1358,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", @@ -944,11 +1374,22 @@ internal class CoderCLIManagerTest { disableAutostart = true, reportWorkspaceUsage = true, wildcardSsh = true, - buildReason = true + buildReason = true, + keyringAuth = true, + ) + ), + Pair( + "2.29.0", + Features( + disableAutostart = true, + reportWorkspaceUsage = true, + wildcardSsh = true, + buildReason = true, + keyringAuth = true, ) ), - Pair("2.4.9", Features(false)), - Pair("1.0.1", Features(false)), + Pair("2.4.9", Features()), + Pair("1.0.1", Features()), ) tests.forEach { @@ -977,6 +1418,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) 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/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", 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")) + } +}