diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e1e0a06..b20fcc75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +### Added + +- support for configuring the SSH config path, letting Toolbox manage a separate SSH config file instead of always + writing to `~/.ssh/config` + ## 0.9.3 - 2026-08-11 ### Changed diff --git a/README.md b/README.md index 32b3a159..10c5b0cb 100644 --- a/README.md +++ b/README.md @@ -568,6 +568,9 @@ The following options control the SSH behavior of the Coder CLI. - `Enable SSH wildcard config` enables or disables wildcard entries in the SSH configuration, which allow generic rules for matching multiple workspaces. +- `SSH config path` is the SSH configuration file managed by the plugin and used for Toolbox connections. It defaults to + `~/.ssh/config`; choose a separate writable file to keep the primary SSH configuration read-only. + - `SSH connnection timeout (seconds)` controls how long the SSH client will wait while trying to establish a TCP connection to the remote host before giving up. Defaults to 0 seconds which means it uses the system’s TCP timeout settings instead. diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt index 82bc5bdd..056f2c20 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt @@ -55,6 +55,7 @@ import kotlinx.coroutines.selects.onTimeout import kotlinx.coroutines.selects.select import java.net.URI import java.net.URL +import java.nio.file.Path import kotlin.coroutines.cancellation.CancellationException import kotlin.time.Duration.Companion.seconds import kotlin.time.TimeSource @@ -74,7 +75,7 @@ class CoderRemoteProvider( internal val lastEnvironments = mutableListOf() private var isSshConfigurationWarningShown = false - private val sshConfigTrigger = Channel(Channel.CONFLATED) + private val sshConfigTrigger = Channel(Channel.CONFLATED) private val workspaceRefreshTrigger = Channel(Channel.CONFLATED) private val providerVisibleTrigger = Channel(Channel.CONFLATED) private val dialogUi = DialogUi(context) @@ -185,11 +186,9 @@ class CoderRemoteProvider( onTimeout(POLL_INTERVAL) { context.logger.debug("workspace poller waked up by the $POLL_INTERVAL timeout") } - sshConfigTrigger.onReceive { shouldTrigger -> - if (shouldTrigger) { - context.logger.debug("workspace poller waked up because it should reconfigure the ssh configurations") - configureSsh(cli, lastEnvironments) - } + sshConfigTrigger.onReceive { staleSshConfigPath -> + context.logger.debug("workspace poller waked up because it should reconfigure the ssh configurations") + configureSsh(cli, lastEnvironments, staleSshConfigPath) } workspaceRefreshTrigger.onReceive { shouldTrigger -> if (shouldTrigger) { @@ -211,10 +210,30 @@ class CoderRemoteProvider( * configuration is necessary to connect, but a read-only or malformed SSH * config must not prevent Toolbox from showing the workspaces it resolved. */ - private fun configureSsh(cli: CoderCLIManager, resolvedEnvironments: List) { + private fun configureSsh( + cli: CoderCLIManager, + resolvedEnvironments: List, + staleSshConfigPath: String? = null, + ) { try { - cli.configSsh(resolvedEnvironments.mapNotNull { it.toWorkspaceAddressOrNull() }.toSet()) + cli.configSsh( + resolvedEnvironments.mapNotNull { it.toWorkspaceAddressOrNull() }.toSet(), + sshConfigPath = context.settingsStore.sshConfigPath, + ) isSshConfigurationWarningShown = false + + // Only attempt cleanup if the previous file actually exists; configSsh would + // otherwise create a stray file there just to hold an empty managed block. + if (staleSshConfigPath != null && Path.of(staleSshConfigPath).toFile().exists()) { + runCatching { + cli.configSsh(emptySet(), sshConfigPath = staleSshConfigPath) + }.onFailure { ex -> + context.logger.warn( + ex, + "Failed to remove the managed SSH config block from the previous location: $staleSshConfigPath" + ) + } + } } catch (ex: Exception) { // Identifier failures are security boundary violations, not recoverable file-system errors. // Let the outer poll handler reject the response before it publishes the environments. diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 2dad5b60..ce9fcd9f 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -298,17 +298,18 @@ class CoderCLIManager( internal fun configSsh( workspaceAddresses: Set, feats: Features = features, + sshConfigPath: String = context.settingsStore.sshConfigPath, ) { - context.logger.info("Configuring SSH config at ${context.settingsStore.sshConfigPath}") - writeSSHConfig(modifySSHConfig(readSSHConfig(), workspaceAddresses, feats)) + context.logger.info("Configuring SSH config at $sshConfigPath") + writeSSHConfig(modifySSHConfig(readSSHConfig(sshConfigPath), workspaceAddresses, feats), sshConfigPath) context.logger.info("Finished configuring SSH config") } /** * Return the contents of the SSH config or null if it does not exist. */ - private fun readSSHConfig(): String? = try { - Path.of(context.settingsStore.sshConfigPath).toFile().readText() + private fun readSSHConfig(sshConfigPath: String): String? = try { + Path.of(sshConfigPath).toFile().readText() } catch (_: FileNotFoundException) { null } @@ -484,11 +485,11 @@ class CoderCLIManager( /** * Write the provided SSH config or do nothing if null. */ - private fun writeSSHConfig(contents: String?) { + private fun writeSSHConfig(contents: String?, sshConfigPath: String) { if (contents != null) { - if (context.settingsStore.sshConfigPath.isNotBlank()) { - val sshConfPath = Path.of(context.settingsStore.sshConfigPath) - sshConfPath.parent.toFile().mkdirs() + if (sshConfigPath.isNotBlank()) { + val sshConfPath = Path.of(sshConfigPath) + sshConfPath.parent?.toFile()?.mkdirs() sshConfPath.toFile().writeText(contents) } // The Coder cli will *not* create the log directory. diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index 14cc9f63..22abc0e4 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -72,7 +72,7 @@ class CoderSettingsStore( override val isSshWildcardConfigEnabled: Boolean get() = store[ENABLE_SSH_WILDCARD_CONFIG]?.toBooleanStrictOrNull() ?: true override val sshConfigPath: String - get() = store[SSH_CONFIG_PATH].takeUnless { it.isNullOrEmpty() } + get() = store[SSH_CONFIG_PATH]?.takeIf { it.isNotBlank() }?.let { expand(it) } ?: Path.of(System.getProperty("user.home")).resolve(".ssh/config").normalize().toString() override val sshLogDirectory: String? get() = store[SSH_LOG_DIR]?.takeIf { it.isNotBlank() }?.let { expand(it) } override val sshConfigOptions: String? @@ -242,6 +242,10 @@ class CoderSettingsStore( store[ENABLE_SSH_WILDCARD_CONFIG] = enable.toString() } + fun updateSshConfigPath(path: String) { + store[SSH_CONFIG_PATH] = path + } + fun updateSshLogDir(path: String) { store[SSH_LOG_DIR] = path } diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt index 29a1e15d..b8fb0455 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt @@ -60,12 +60,20 @@ class Action( closesPage: Boolean = false, highlightInRed: Boolean = false, enabled: () -> Boolean = { true }, + private val validateBlock: () -> Boolean = { true }, private val actionBlock: suspend () -> Unit, ) : RunnableActionDescription { override val label: LocalizableString = context.i18n.ptrl(description) override val shouldClosePage: Boolean = closesPage override val isEnabled: Boolean = enabled() override val isDangerous: Boolean = highlightInRed + + /** + * Toolbox calls this before [run], and only calls [run] (and closes the + * page, if [shouldClosePage]) when this returns true. + */ + override fun validate(): Boolean = validateBlock() + override fun run() { context.cs.launch(CoroutineName("$description Action")) { try { diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt index f3c64958..1d830b29 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt @@ -5,14 +5,20 @@ 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.canCreateDirectory +import com.coder.toolbox.util.expand import com.jetbrains.toolbox.api.ui.actions.RunnableActionDescription import com.jetbrains.toolbox.api.ui.components.CheckboxField import com.jetbrains.toolbox.api.ui.components.ComboBoxField import com.jetbrains.toolbox.api.ui.components.ComboBoxField.LabelledValue +import com.jetbrains.toolbox.api.ui.components.FieldModifier import com.jetbrains.toolbox.api.ui.components.SectionField import com.jetbrains.toolbox.api.ui.components.TextField import com.jetbrains.toolbox.api.ui.components.TextType import com.jetbrains.toolbox.api.ui.components.UiField +import com.jetbrains.toolbox.api.ui.components.ValidatableField +import com.jetbrains.toolbox.api.ui.components.ValidationResult +import com.jetbrains.toolbox.api.ui.components.validate import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel @@ -20,6 +26,8 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import java.nio.file.Files +import java.nio.file.Path /** * A page for modifying Coder settings. @@ -30,7 +38,7 @@ import kotlinx.coroutines.launch */ class CoderSettingsPage( private val context: CoderToolboxContext, - sshConfigTrigger: Channel, + sshConfigTrigger: Channel, private val onSettingsClosed: () -> Unit ) : CoderPage(MutableStateFlow(context.i18n.ptrl("Coder Settings")), false) { @@ -108,6 +116,13 @@ class CoderSettingsPage( TextType.Integer ) + private val sshConfigPathField = TextField( + context.i18n.ptrl("SSH config path"), + settings.sshConfigPath, + TextType.General, + validator = ::validateSshConfigPath + ) + private val sshExtraArgs = TextField( context.i18n.ptrl("Extra SSH options"), settings.sshConfigOptions ?: "", @@ -125,6 +140,47 @@ class CoderSettingsPage( TextType.General ) + /** + * Toolbox does not validate fields on its own; a page's action is + * responsible for validating and reporting whether everything came back + * clean. `UiPage.runValidations()` only looks at the top-level fields + * list, but ours are nested inside `SectionField`s, so fields have to be + * flattened out of their sections before each is validated. + */ + private fun validateFields(): Boolean = fields.value.allFieldsValid() + + private fun List.allFieldsValid(): Boolean = all { field -> + when (field) { + is SectionField -> field.contentState.value.allFieldsValid() + is ValidatableField<*> -> { + field.validate() + field.modifiers.value.none { it is FieldModifier.LocalizableError || it is FieldModifier.Error } + } + + else -> true + } + } + + private fun validateSshConfigPath(rawPath: String): ValidationResult { + val expandedPath = expand(rawPath) + if (expandedPath.isBlank()) { + return ValidationResult.Invalid(context.i18n.ptrl("SSH config path must not be empty")) + } + val configPath = Path.of(expandedPath) + if (Files.exists(configPath)) { + return if (Files.isRegularFile(configPath) && Files.isWritable(configPath)) { + ValidationResult.Valid + } else { + ValidationResult.Invalid(context.i18n.ptrl("SSH config path must point to a writable file")) + } + } + return if (configPath.parent?.canCreateDirectory() == true) { + ValidationResult.Valid + } else { + ValidationResult.Invalid(context.i18n.ptrl("SSH config path's parent directory must be writable")) + } + } + private lateinit var visibilityUpdateJob: Job override val fields: StateFlow> = MutableStateFlow( listOf( @@ -166,6 +222,7 @@ class CoderSettingsPage( false, listOf( enableSshWildCardConfig, + sshConfigPathField, sshConnectionTimeoutField, sshLogDirField, networkInfoDirField, @@ -177,7 +234,7 @@ class CoderSettingsPage( override val actionButtons: StateFlow> = MutableStateFlow( listOf( - Action(context, "Save", closesPage = true) { + Action(context, "Save", closesPage = true, validateBlock = ::validateFields) { with(context.settingsStore) { updateBinarySource(binarySourceField.contentState.value) updateBinaryDestination(binaryDestinationField.contentState.value) @@ -197,16 +254,21 @@ class CoderSettingsPage( val sshWildcardEnabled = enableSshWildCardConfig.checkedState.value val sshTimeout = sshConnectionTimeoutField.contentState.value.toInt() + val sshConfigPath = sshConfigPathField.contentState.value + val previousSshConfigPath = settings.sshConfigPath val sshSettingsChanged = sshWildcardEnabled != settings.isSshWildcardConfigEnabled || - sshTimeout != settings.sshConnectionTimeoutInSeconds + sshTimeout != settings.sshConnectionTimeoutInSeconds || + sshConfigPath != previousSshConfigPath updateEnableSshWildcardConfig(sshWildcardEnabled) updateSshConnectionTimeoutInSeconds(sshTimeout) + updateSshConfigPath(sshConfigPath) if (sshSettingsChanged) { + val staleSshConfigPath = previousSshConfigPath.takeIf { it != settings.sshConfigPath } runCatching { - sshConfigTrigger.send(true) + sshConfigTrigger.send(staleSshConfigPath) context.logger.info("Settings have been modified, ssh config is going to be regenerated...") } } @@ -276,6 +338,10 @@ class CoderSettingsPage( settings.sshConnectionTimeoutInSeconds.toString() } + sshConfigPathField.contentState.update { + settings.sshConfigPath + } + sshExtraArgs.contentState.update { settings.sshConfigOptions ?: "" } diff --git a/src/main/kotlin/com/coder/toolbox/views/EnvironmentView.kt b/src/main/kotlin/com/coder/toolbox/views/EnvironmentView.kt index 2f56e7ee..4978db0d 100644 --- a/src/main/kotlin/com/coder/toolbox/views/EnvironmentView.kt +++ b/src/main/kotlin/com/coder/toolbox/views/EnvironmentView.kt @@ -51,6 +51,12 @@ private class WorkspaceSshConnectionInfo( */ override val userName: String? = null + /** + * Use the config managed by this plugin rather than Toolbox's default SSH + * config, which may be a separate read-only user-managed file. + */ + override val sshConfigPath: String = context.settingsStore.sshConfigPath + override val connectionTimeoutMillis: Long get() = context.settingsStore.sshConnectionTimeoutInSeconds.seconds.inWholeMilliseconds @@ -64,6 +70,7 @@ private class WorkspaceSshConnectionInfo( if (workspace.name != other.workspace.name) return false if (agent.name != other.agent.name) return false if (host != other.host) return false + if (sshConfigPath != other.sshConfigPath) return false return true } @@ -73,6 +80,7 @@ private class WorkspaceSshConnectionInfo( result = 31 * result + workspace.name.hashCode() result = 31 * result + agent.name.hashCode() result = 31 * result + host.hashCode() + result = 31 * result + sshConfigPath.hashCode() return result } diff --git a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt index fcadd867..84560ace 100644 --- a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt +++ b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt @@ -83,7 +83,7 @@ class CoderRemoteProviderTest { val agent = mockAgent("agent1") val workspace = mockWorkspace("ws1", WorkspaceStatus.RUNNING, listOf(mockResource(listOf(agent)))) coEvery { mockClient.workspaces(any()) } returns listOf(workspace) - every { mockCli.configSsh(any(), any()) } throws FileNotFoundException("Permission denied") + every { mockCli.configSsh(any(), any(), any()) } throws FileNotFoundException("Permission denied") // when val pollJob = remoteProvider.poll(mockClient, mockCli) @@ -115,7 +115,7 @@ class CoderRemoteProviderTest { val agent = mockAgent("agent1") val workspace = mockWorkspace("ws1", WorkspaceStatus.RUNNING, listOf(mockResource(listOf(agent)))) coEvery { mockClient.workspaces(any()) } returns listOf(workspace) - every { mockCli.configSsh(any(), any()) } throws + every { mockCli.configSsh(any(), any(), any()) } throws InvalidCoderIdentifierException("The deployment returned an invalid workspace name") val pollJob = remoteProvider.poll(mockClient, mockCli) diff --git a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt index 4949a029..0eca47b8 100644 --- a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt +++ b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt @@ -112,6 +112,51 @@ class CoderSettingsStoreTest { } } + @Test + fun `ssh config path uses the configured value`() { + val configuredPath = "/tmp/coder-toolbox-test/config" + + store.updateSshConfigPath(configuredPath) + + assertEquals(configuredPath, store.sshConfigPath) + } + + @Test + fun `ssh config path defaults to ~-ssh-config when unset`() { + val home = Path.of(System.getProperty("user.home")) + + assertEquals(home.resolve(".ssh/config").normalize().toString(), store.sshConfigPath) + } + + @Test + fun `ssh config path falls back to the default when the configured value is blank`() { + val home = Path.of(System.getProperty("user.home")) + + store.updateSshConfigPath(" ") + + assertEquals(home.resolve(".ssh/config").normalize().toString(), store.sshConfigPath) + } + + @Test + fun `ssh config path expands tilde in the configured value`() { + // Don't override OS — tilde expansion depends on the real File.separator. + val home = Path.of(System.getProperty("user.home")) + + val settings = storeWith(SSH_CONFIG_PATH to "~/coder-ssh/config") + + assertEquals(home.resolve("coder-ssh/config").toString(), settings.sshConfigPath) + } + + @Test + fun `ssh config path expands HOME in the configured value`() { + // Don't override OS — $HOME expansion depends on the real File.separator. + val home = Path.of(System.getProperty("user.home")) + + val settings = storeWith(SSH_CONFIG_PATH to "\$HOME/coder-ssh/config") + + assertEquals(home.resolve("coder-ssh/config").toString(), settings.sshConfigPath) + } + @Test fun `binPath expands tilde in binaryDestination when it points to an existing executable`() { // Don't override OS — tilde expansion depends on the real File.separator. diff --git a/src/test/kotlin/com/coder/toolbox/views/EnvironmentViewTest.kt b/src/test/kotlin/com/coder/toolbox/views/EnvironmentViewTest.kt new file mode 100644 index 00000000..397a9c76 --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/views/EnvironmentViewTest.kt @@ -0,0 +1,40 @@ +package com.coder.toolbox.views + +import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.cli.CoderCLIManager +import com.coder.toolbox.cli.WorkspaceAddress +import com.coder.toolbox.sdk.v2.models.Workspace +import com.coder.toolbox.sdk.v2.models.WorkspaceAgent +import com.coder.toolbox.store.CoderSettingsStore +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.runBlocking +import java.net.URL +import kotlin.test.Test +import kotlin.test.assertEquals + +class EnvironmentViewTest { + @Test + fun `connection info passes the configured SSH config path to Toolbox`() = runBlocking { + val context = mockk(relaxed = true) + val settings = mockk() + val cli = mockk() + val workspace = mockk { + every { name } returns "workspace" + every { ownerName } returns "owner" + } + val agent = mockk { + every { name } returns "agent" + } + val url = URL("https://coder.example.com") + val configuredPath = "/tmp/coder-toolbox-test/config" + + every { context.settingsStore } returns settings + every { settings.sshConfigPath } returns configuredPath + every { cli.getHostname(url, any()) } returns "coder.example.com--workspace.agent" + + val connectionInfo = EnvironmentView(context, url, cli, workspace, agent).getConnectionInfo() + + assertEquals(configuredPath, connectionInfo.sshConfigPath) + } +}