From 4001f7d22f254df047fd9322fcdfc5262c58f08e Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 22 Jul 2025 22:52:30 +0300 Subject: [PATCH 01/10] impl: support for downloading and verifying cli signatures --- CHANGELOG.md | 5 + .../gateway/CoderRemoteConnectionHandle.kt | 2 +- .../gateway/CoderSettingsConfigurable.kt | 8 + .../com/coder/gateway/cli/CoderCLIManager.kt | 239 ++++++++++++------ .../cli/downloader/CoderDownloadApi.kt | 29 +++ .../cli/downloader/CoderDownloadService.kt | 238 +++++++++++++++++ .../gateway/cli/downloader/DownloadResult.kt | 23 ++ .../com/coder/gateway/cli/ex/Exceptions.kt | 2 + .../com/coder/gateway/cli/gpg/GPGVerifier.kt | 142 +++++++++++ .../gateway/cli/gpg/VerificationResult.kt | 15 ++ .../coder/gateway/settings/CoderSettings.kt | 99 +++++--- .../com/coder/gateway/util/LinkHandler.kt | 2 +- .../kotlin/com/coder/gateway/util/SemVer.kt | 2 +- .../views/steps/CoderWorkspacesStepView.kt | 17 ++ .../messages/CoderGatewayBundle.properties | 4 + 15 files changed, 713 insertions(+), 114 deletions(-) create mode 100644 src/main/kotlin/com/coder/gateway/cli/downloader/CoderDownloadApi.kt create mode 100644 src/main/kotlin/com/coder/gateway/cli/downloader/CoderDownloadService.kt create mode 100644 src/main/kotlin/com/coder/gateway/cli/downloader/DownloadResult.kt create mode 100644 src/main/kotlin/com/coder/gateway/cli/gpg/GPGVerifier.kt create mode 100644 src/main/kotlin/com/coder/gateway/cli/gpg/VerificationResult.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c954387a..064765325 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ ## Unreleased +### Added + +- support for checking if CLI is signed +- improved progress reporting while downloading the CLI + ## 2.21.1 - 2025-06-26 ### Fixed diff --git a/src/main/kotlin/com/coder/gateway/CoderRemoteConnectionHandle.kt b/src/main/kotlin/com/coder/gateway/CoderRemoteConnectionHandle.kt index 790a2cd3a..481e5aa78 100644 --- a/src/main/kotlin/com/coder/gateway/CoderRemoteConnectionHandle.kt +++ b/src/main/kotlin/com/coder/gateway/CoderRemoteConnectionHandle.kt @@ -66,7 +66,7 @@ class CoderRemoteConnectionHandle { private val localTimeFormatter = DateTimeFormatter.ofPattern("yyyy-MMM-dd HH:mm") private val dialogUi = DialogUi(settings) - fun connect(getParameters: (indicator: ProgressIndicator) -> WorkspaceProjectIDE) { + fun connect(getParameters: suspend (indicator: ProgressIndicator) -> WorkspaceProjectIDE) { val clientLifetime = LifetimeDefinition() clientLifetime.launchUnderBackgroundProgress(CoderGatewayBundle.message("gateway.connector.coder.connection.provider.title")) { try { diff --git a/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt b/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt index 18373983e..2032dc699 100644 --- a/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt +++ b/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt @@ -68,6 +68,14 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") { CoderGatewayBundle.message("gateway.connector.settings.enable-binary-directory-fallback.comment"), ) }.layout(RowLayout.PARENT_GRID) + row { + cell() // For alignment. + checkBox(CoderGatewayBundle.message("gateway.connector.settings.fallback-on-coder-for-signatures.title")) + .bindSelected(state::fallbackOnCoderForSignatures) + .comment( + CoderGatewayBundle.message("gateway.connector.settings.fallback-on-coder-for-signatures.comment"), + ) + }.layout(RowLayout.PARENT_GRID) row(CoderGatewayBundle.message("gateway.connector.settings.header-command.title")) { textField().resizableColumn().align(AlignX.FILL) .bindText(state::headerCommand) diff --git a/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt index cc883a3bc..283844856 100644 --- a/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt @@ -1,41 +1,42 @@ package com.coder.gateway.cli +import com.coder.gateway.cli.downloader.CoderDownloadApi +import com.coder.gateway.cli.downloader.CoderDownloadService +import com.coder.gateway.cli.downloader.DownloadResult import com.coder.gateway.cli.ex.MissingVersionException -import com.coder.gateway.cli.ex.ResponseException import com.coder.gateway.cli.ex.SSHConfigFormatException +import com.coder.gateway.cli.ex.UnsignedBinaryExecutionDeniedException +import com.coder.gateway.cli.gpg.GPGVerifier +import com.coder.gateway.cli.gpg.VerificationResult import com.coder.gateway.sdk.v2.models.User import com.coder.gateway.sdk.v2.models.Workspace import com.coder.gateway.sdk.v2.models.WorkspaceAgent import com.coder.gateway.settings.CoderSettings import com.coder.gateway.settings.CoderSettingsState import com.coder.gateway.util.CoderHostnameVerifier +import com.coder.gateway.util.DialogUi import com.coder.gateway.util.InvalidVersionException -import com.coder.gateway.util.OS import com.coder.gateway.util.SemVer import com.coder.gateway.util.coderSocketFactory +import com.coder.gateway.util.coderTrustManagers import com.coder.gateway.util.escape import com.coder.gateway.util.escapeSubcommand -import com.coder.gateway.util.getHeaders -import com.coder.gateway.util.getOS import com.coder.gateway.util.safeHost -import com.coder.gateway.util.sha1 import com.intellij.openapi.diagnostic.Logger import com.squareup.moshi.Json import com.squareup.moshi.JsonClass import com.squareup.moshi.JsonDataException import com.squareup.moshi.Moshi +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.OkHttpClient import org.zeroturnaround.exec.ProcessExecutor +import retrofit2.Retrofit import java.io.EOFException -import java.io.FileInputStream import java.io.FileNotFoundException -import java.net.ConnectException -import java.net.HttpURLConnection import java.net.URL -import java.nio.file.Files import java.nio.file.Path -import java.nio.file.StandardCopyOption -import java.util.zip.GZIPInputStream -import javax.net.ssl.HttpsURLConnection +import javax.net.ssl.X509TrustManager /** * Version output from the CLI's version command. @@ -57,7 +58,7 @@ internal data class Version( * 6. Since the binary directory can be read-only, if downloading fails, start * from step 2 with the data directory. */ -fun ensureCLI( +suspend fun ensureCLI( deploymentURL: URL, buildVersion: String, settings: CoderSettings, @@ -72,6 +73,7 @@ fun ensureCLI( // the 304 method. val cliMatches = cli.matchesVersion(buildVersion) if (cliMatches == true) { + indicator?.invoke("Local CLI version matches server version: $buildVersion") return cli } @@ -79,7 +81,7 @@ fun ensureCLI( if (settings.enableDownloads) { indicator?.invoke("Downloading Coder CLI...") try { - cli.download() + cli.download(buildVersion, indicator) return cli } catch (e: java.nio.file.AccessDeniedException) { // Might be able to fall back to the data directory. @@ -95,12 +97,13 @@ fun ensureCLI( val dataCLI = CoderCLIManager(deploymentURL, settings, true) val dataCLIMatches = dataCLI.matchesVersion(buildVersion) if (dataCLIMatches == true) { + indicator?.invoke("Local CLI version from data directory matches server version: $buildVersion") return dataCLI } if (settings.enableDownloads) { - indicator?.invoke("Downloading Coder CLI...") - dataCLI.download() + indicator?.invoke("Downloading Coder CLI to the data directory...") + dataCLI.download(buildVersion, indicator) return dataCLI } @@ -128,78 +131,147 @@ class CoderCLIManager( private val settings: CoderSettings = CoderSettings(CoderSettingsState()), // If the binary directory is not writable, this can be used to force the // manager to download to the data directory instead. - forceDownloadToData: Boolean = false, + private val forceDownloadToData: Boolean = false, ) { + private val downloader = createDownloadService() + private val gpgVerifier = GPGVerifier(settings) + val remoteBinaryURL: URL = settings.binSource(deploymentURL) val localBinaryPath: Path = settings.binPath(deploymentURL, forceDownloadToData) val coderConfigPath: Path = settings.dataDir(deploymentURL).resolve("config") + private fun createDownloadService(): CoderDownloadService { + val okHttpClient = OkHttpClient.Builder() + .sslSocketFactory( + coderSocketFactory(settings.tls), + coderTrustManagers(settings.tls.caPath)[0] as X509TrustManager + ) + .hostnameVerifier(CoderHostnameVerifier(settings.tls.altHostname)) + .build() + + val retrofit = Retrofit.Builder() + .baseUrl(deploymentURL.toString()) + .client(okHttpClient) + .build() + + val service = retrofit.create(CoderDownloadApi::class.java) + return CoderDownloadService(settings, service, deploymentURL, forceDownloadToData) + } + /** * Download the CLI from the deployment if necessary. */ - fun download(): Boolean { - val eTag = getBinaryETag() - val conn = remoteBinaryURL.openConnection() as HttpURLConnection - if (settings.headerCommand.isNotBlank()) { - val headersFromHeaderCommand = getHeaders(deploymentURL, settings.headerCommand) - for ((key, value) in headersFromHeaderCommand) { - conn.setRequestProperty(key, value) + suspend fun download(buildVersion: String, showTextProgress: ((t: String) -> Unit)? = null): Boolean { + try { + val cliResult = withContext(Dispatchers.IO) { + downloader.downloadCli(buildVersion, showTextProgress) + }.let { result -> + when { + result.isSkipped() -> return false + result.isNotFound() -> throw IllegalStateException("Could not find Coder CLI") + result.isFailed() -> throw (result as DownloadResult.Failed).error + else -> result as DownloadResult.Downloaded + } } - } - if (eTag != null) { - logger.info("Found existing binary at $localBinaryPath; calculated hash as $eTag") - conn.setRequestProperty("If-None-Match", "\"$eTag\"") - } - conn.setRequestProperty("Accept-Encoding", "gzip") - if (conn is HttpsURLConnection) { - conn.sslSocketFactory = coderSocketFactory(settings.tls) - conn.hostnameVerifier = CoderHostnameVerifier(settings.tls.altHostname) - } - try { - conn.connect() - logger.info("GET ${conn.responseCode} $remoteBinaryURL") - when (conn.responseCode) { - HttpURLConnection.HTTP_OK -> { - logger.info("Downloading binary to $localBinaryPath") - Files.createDirectories(localBinaryPath.parent) - conn.inputStream.use { - Files.copy( - if (conn.contentEncoding == "gzip") GZIPInputStream(it) else it, - localBinaryPath, - StandardCopyOption.REPLACE_EXISTING, - ) + var signatureResult = withContext(Dispatchers.IO) { + downloader.downloadSignature(showTextProgress) + } + + if (signatureResult.isNotDownloaded()) { + if (settings.fallbackOnCoderForSignatures) { + logger.info("Trying to download signature file from releases.coder.com") + signatureResult = withContext(Dispatchers.IO) { + downloader.downloadReleasesSignature(buildVersion, showTextProgress) + } + + // if we could still not download it, ask the user if he accepts the risk + if (signatureResult.isNotDownloaded()) { + val acceptsUnsignedBinary = DialogUi(settings) + .confirm( + "Security Warning", + "Could not fetch any signatures for ${cliResult.source} from releases.coder.com. Would you like to run it anyway?" + ) + + if (acceptsUnsignedBinary) { + downloader.commit() + return true + } else { + throw UnsignedBinaryExecutionDeniedException("Running unsigned CLI from ${cliResult.source} was denied by the user") + } } - if (getOS() != OS.WINDOWS) { - localBinaryPath.toFile().setExecutable(true) + } else { + // we are not allowed to fetch signatures from releases.coder.com + // so we will ask the user if he wants to continue + val acceptsUnsignedBinary = DialogUi(settings) + .confirm( + "Security Warning", + "No signatures were found for ${cliResult.source} and fallback to releases.coder.com is not allowed. Would you like to run it anyway?" + ) + + if (acceptsUnsignedBinary) { + downloader.commit() + return true + } else { + throw UnsignedBinaryExecutionDeniedException("Running unsigned CLI from ${cliResult.source} was denied by the user") } - return true } + } + + // we have the cli, and signature is downloaded, let's verify the signature + signatureResult = signatureResult as DownloadResult.Downloaded + gpgVerifier.verifySignature(cliResult.dst, signatureResult.dst).let { result -> + when { + result.isValid() -> { + downloader.commit() + return true + } - HttpURLConnection.HTTP_NOT_MODIFIED -> { - logger.info("Using cached binary at $localBinaryPath") - return false + else -> { + logFailure(result, cliResult, signatureResult) + // prompt the user if he wants to accept the risk + val shouldRunAnyway = DialogUi(settings) + .confirm( + "Security Warning", + "Could not verify the authenticity of the ${cliResult.source}, it may be tampered with. Would you like to run it anyway?" + ) + + if (shouldRunAnyway) { + downloader.commit() + return true + } else { + throw UnsignedBinaryExecutionDeniedException("Running unverified CLI from ${cliResult.source} was denied by the user") + } + } } } - } catch (e: ConnectException) { - // Add the URL so this is more easily debugged. - throw ConnectException("${e.message} to $remoteBinaryURL") } finally { - conn.disconnect() + downloader.cleanup() } - throw ResponseException("Unexpected response from $remoteBinaryURL", conn.responseCode) } - /** - * Return the entity tag for the binary on disk, if any. - */ - private fun getBinaryETag(): String? = try { - sha1(FileInputStream(localBinaryPath.toFile())) - } catch (e: FileNotFoundException) { - null - } catch (e: Exception) { - logger.warn("Unable to calculate hash for $localBinaryPath", e) - null + private fun logFailure( + result: VerificationResult, + cliResult: DownloadResult.Downloaded, + signatureResult: DownloadResult.Downloaded + ) { + when { + result.isInvalid() -> { + val reason = (result as VerificationResult.Invalid).reason + logger.error("Signature of ${cliResult.dst} is invalid." + reason?.let { " Reason: $it" } + .orEmpty()) + } + + result.signatureIsNotFound() -> { + logger.error("Can't verify signature of ${cliResult.dst} because ${signatureResult.dst} does not exist") + } + + else -> { + UnsignedBinaryExecutionDeniedException((result as DownloadResult.Failed).error.message) + val failure = result as DownloadResult.Failed + logger.error("Failed to verify signature for ${cliResult.dst}", failure.error) + } + } } /** @@ -278,7 +350,8 @@ class CoderCLIManager( if (settings.sshLogDirectory.isNotBlank()) escape(settings.sshLogDirectory) else null, if (feats.reportWorkspaceUsage) "--usage-app=jetbrains" else null, ) - val backgroundProxyArgs = baseArgs + listOfNotNull(if (feats.reportWorkspaceUsage) "--usage-app=disable" else null) + val backgroundProxyArgs = + baseArgs + listOfNotNull(if (feats.reportWorkspaceUsage) "--usage-app=disable" else null) val extraConfig = if (settings.sshConfigOptions.isNotBlank()) { "\n" + settings.sshConfigOptions.prependIndent(" ") @@ -295,22 +368,22 @@ class CoderCLIManager( val blockContent = if (feats.wildcardSSH) { startBlock + System.lineSeparator() + - """ + """ Host ${getHostPrefix()}--* ProxyCommand ${proxyArgs.joinToString(" ")} --ssh-host-prefix ${getHostPrefix()}-- %h """.trimIndent() - .plus("\n" + sshOpts.prependIndent(" ")) - .plus(extraConfig) - .plus("\n\n") - .plus( - """ + .plus("\n" + sshOpts.prependIndent(" ")) + .plus(extraConfig) + .plus("\n\n") + .plus( + """ Host ${getHostPrefix()}-bg--* ProxyCommand ${backgroundProxyArgs.joinToString(" ")} --ssh-host-prefix ${getHostPrefix()}-bg-- %h """.trimIndent() - .plus("\n" + sshOpts.prependIndent(" ")) - .plus(extraConfig), - ).replace("\n", System.lineSeparator()) + - System.lineSeparator() + endBlock + .plus("\n" + sshOpts.prependIndent(" ")) + .plus(extraConfig), + ).replace("\n", System.lineSeparator()) + + System.lineSeparator() + endBlock } else if (workspaceNames.isEmpty()) { "" } else { @@ -329,7 +402,12 @@ class CoderCLIManager( .plus( """ Host ${getBackgroundHostName(it.first, currentUser, it.second)} - ProxyCommand ${backgroundProxyArgs.joinToString(" ")} ${getWorkspaceParts(it.first, it.second)} + ProxyCommand ${backgroundProxyArgs.joinToString(" ")} ${ + getWorkspaceParts( + it.first, + it.second + ) + } """.trimIndent() .plus("\n" + sshOpts.prependIndent(" ")) .plus(extraConfig), @@ -443,6 +521,7 @@ class CoderCLIManager( is InvalidVersionException -> { logger.info("Got invalid version from $localBinaryPath: ${e.message}") } + else -> { // An error here most likely means the CLI does not exist or // it executed successfully but output no version which diff --git a/src/main/kotlin/com/coder/gateway/cli/downloader/CoderDownloadApi.kt b/src/main/kotlin/com/coder/gateway/cli/downloader/CoderDownloadApi.kt new file mode 100644 index 000000000..fa27fdc7d --- /dev/null +++ b/src/main/kotlin/com/coder/gateway/cli/downloader/CoderDownloadApi.kt @@ -0,0 +1,29 @@ +package com.coder.gateway.cli.downloader + +import okhttp3.ResponseBody +import retrofit2.Response +import retrofit2.http.GET +import retrofit2.http.Header +import retrofit2.http.HeaderMap +import retrofit2.http.Streaming +import retrofit2.http.Url + +/** + * Retrofit API for downloading CLI + */ +interface CoderDownloadApi { + @GET + @Streaming + suspend fun downloadCli( + @Url url: String, + @Header("If-None-Match") eTag: String? = null, + @HeaderMap headers: Map = emptyMap(), + @Header("Accept-Encoding") acceptEncoding: String = "gzip", + ): Response + + @GET + suspend fun downloadSignature( + @Url url: String, + @HeaderMap headers: Map = emptyMap() + ): Response +} \ No newline at end of file diff --git a/src/main/kotlin/com/coder/gateway/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/gateway/cli/downloader/CoderDownloadService.kt new file mode 100644 index 000000000..3c315dd0e --- /dev/null +++ b/src/main/kotlin/com/coder/gateway/cli/downloader/CoderDownloadService.kt @@ -0,0 +1,238 @@ +package com.coder.gateway.cli.downloader + +import com.coder.gateway.cli.ex.ResponseException +import com.coder.gateway.settings.CoderSettings +import com.coder.gateway.util.OS +import com.coder.gateway.util.SemVer +import com.coder.gateway.util.getHeaders +import com.coder.gateway.util.getOS +import com.coder.gateway.util.sha1 +import com.intellij.openapi.diagnostic.Logger +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.ResponseBody +import retrofit2.Response +import java.io.FileInputStream +import java.net.HttpURLConnection.HTTP_NOT_FOUND +import java.net.HttpURLConnection.HTTP_NOT_MODIFIED +import java.net.HttpURLConnection.HTTP_OK +import java.net.URI +import java.net.URL +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.StandardCopyOption +import java.nio.file.StandardOpenOption +import java.util.zip.GZIPInputStream +import kotlin.io.path.name +import kotlin.io.path.notExists + +/** + * Handles the download steps of Coder CLI + */ +class CoderDownloadService( + private val settings: CoderSettings, + private val downloadApi: CoderDownloadApi, + private val deploymentUrl: URL, + forceDownloadToData: Boolean, +) { + private val remoteBinaryURL: URL = settings.binSource(deploymentUrl) + private val cliFinalDst: Path = settings.binPath(deploymentUrl, forceDownloadToData) + private val cliTempDst: Path = cliFinalDst.resolveSibling("${cliFinalDst.name}.tmp") + + suspend fun downloadCli(buildVersion: String, showTextProgress: ((t: String) -> Unit)? = null): DownloadResult { + val eTag = calculateLocalETag() + if (eTag != null) { + logger.info("Found existing binary at $cliFinalDst; calculated hash as $eTag") + } + val response = downloadApi.downloadCli( + url = remoteBinaryURL.toString(), + eTag = eTag?.let { "\"$it\"" }, + headers = getRequestHeaders() + ) + + return when (response.code()) { + HTTP_OK -> { + logger.info("Downloading binary to temporary $cliTempDst") + response.saveToDisk(cliTempDst, showTextProgress, buildVersion)?.makeExecutable() + DownloadResult.Downloaded(remoteBinaryURL, cliTempDst) + } + + HTTP_NOT_MODIFIED -> { + logger.info("Using cached binary at $cliFinalDst") + showTextProgress?.invoke("Using cached binary") + DownloadResult.Skipped + } + + else -> { + throw ResponseException( + "Unexpected response from $remoteBinaryURL", + response.code() + ) + } + } + } + + /** + * Renames the temporary binary file to its original destination name. + * The implementation will override sibling file that has the original + * destination name. + */ + suspend fun commit(): Path { + return withContext(Dispatchers.IO) { + logger.info("Renaming binary from $cliTempDst to $cliFinalDst") + Files.move(cliTempDst, cliFinalDst, StandardCopyOption.REPLACE_EXISTING) + cliFinalDst.makeExecutable() + cliFinalDst + } + } + + /** + * Cleans up the temporary binary file if it exists. + */ + suspend fun cleanup() { + withContext(Dispatchers.IO) { + runCatching { Files.deleteIfExists(cliTempDst) } + .onFailure { ex -> + logger.warn("Failed to delete temporary CLI file: $cliTempDst", ex) + } + } + } + + private fun calculateLocalETag(): String? { + return try { + if (cliFinalDst.notExists()) { + return null + } + sha1(FileInputStream(cliFinalDst.toFile())) + } catch (e: Exception) { + logger.warn("Unable to calculate hash for $cliFinalDst", e) + null + } + } + + private fun getRequestHeaders(): Map { + return if (settings.headerCommand.isBlank()) { + emptyMap() + } else { + getHeaders(deploymentUrl, settings.headerCommand) + } + } + + private fun Response.saveToDisk( + localPath: Path, + showTextProgress: ((t: String) -> Unit)? = null, + buildVersion: String? = null + ): Path? { + val responseBody = this.body() ?: return null + Files.deleteIfExists(localPath) + Files.createDirectories(localPath.parent) + + val outputStream = Files.newOutputStream( + localPath, + StandardOpenOption.CREATE, + StandardOpenOption.TRUNCATE_EXISTING + ) + val contentEncoding = this.headers()["Content-Encoding"] + val sourceStream = if (contentEncoding?.contains("gzip", ignoreCase = true) == true) { + GZIPInputStream(responseBody.byteStream()) + } else { + responseBody.byteStream() + } + + val buffer = ByteArray(DEFAULT_BUFFER_SIZE) + var bytesRead: Int + var totalRead = 0L + // local path is a temporary filename, reporting the progress with the real name + val binaryName = localPath.name.removeSuffix(".tmp") + sourceStream.use { source -> + outputStream.use { sink -> + while (source.read(buffer).also { bytesRead = it } != -1) { + sink.write(buffer, 0, bytesRead) + totalRead += bytesRead + val prettyBuildVersion = buildVersion ?: "" + showTextProgress?.invoke( + "$binaryName $prettyBuildVersion - ${totalRead.toHumanReadableSize()} downloaded" + ) + } + } + } + return cliFinalDst + } + + + private fun Path.makeExecutable() { + if (getOS() != OS.WINDOWS) { + logger.info("Making $this executable...") + this.toFile().setExecutable(true) + } + } + + private fun Long.toHumanReadableSize(): String { + if (this < 1024) return "$this B" + + val kb = this / 1024.0 + if (kb < 1024) return String.format("%.1f KB", kb) + + val mb = kb / 1024.0 + if (mb < 1024) return String.format("%.1f MB", mb) + + val gb = mb / 1024.0 + return String.format("%.1f GB", gb) + } + + suspend fun downloadSignature(showTextProgress: ((t: String) -> Unit)? = null): DownloadResult { + return downloadSignature(remoteBinaryURL, showTextProgress, getRequestHeaders()) + } + + private suspend fun downloadSignature( + url: URL, + showTextProgress: ((t: String) -> Unit)? = null, + headers: Map = emptyMap() + ): DownloadResult { + val signatureURL = url.toURI().resolve(settings.defaultSignatureNameByOsAndArch).toURL() + val localSignaturePath = cliFinalDst.parent.resolve(settings.defaultSignatureNameByOsAndArch) + logger.info("Downloading signature from $signatureURL") + + val response = downloadApi.downloadSignature( + url = signatureURL.toString(), + headers = headers + ) + + return when (response.code()) { + HTTP_OK -> { + response.saveToDisk(localSignaturePath, showTextProgress) + DownloadResult.Downloaded(signatureURL, localSignaturePath) + } + + HTTP_NOT_FOUND -> { + logger.warn("Signature file not found at $signatureURL") + DownloadResult.NotFound + } + + else -> { + DownloadResult.Failed( + ResponseException( + "Failed to download signature from $signatureURL", + response.code() + ) + ) + } + } + + } + + suspend fun downloadReleasesSignature( + buildVersion: String, + showTextProgress: ((t: String) -> Unit)? = null + ): DownloadResult { + val semVer = SemVer.parse(buildVersion) + return downloadSignature( + URI.create("https://releases.coder.com/coder-cli/${semVer.major}.${semVer.minor}.${semVer.patch}/").toURL(), + showTextProgress + ) + } + + companion object { + val logger = Logger.getInstance(CoderDownloadService::class.java.simpleName) + } +} \ No newline at end of file diff --git a/src/main/kotlin/com/coder/gateway/cli/downloader/DownloadResult.kt b/src/main/kotlin/com/coder/gateway/cli/downloader/DownloadResult.kt new file mode 100644 index 000000000..a0fcfc933 --- /dev/null +++ b/src/main/kotlin/com/coder/gateway/cli/downloader/DownloadResult.kt @@ -0,0 +1,23 @@ +package com.coder.gateway.cli.downloader + +import java.net.URL +import java.nio.file.Path + + +/** + * Result of a download operation + */ +sealed class DownloadResult { + object Skipped : DownloadResult() + object NotFound : DownloadResult() + data class Downloaded(val source: URL, val dst: Path) : DownloadResult() + data class Failed(val error: Exception) : DownloadResult() + + fun isSkipped(): Boolean = this is Skipped + + fun isNotFound(): Boolean = this is NotFound + + fun isFailed(): Boolean = this is Failed + + fun isNotDownloaded(): Boolean = this !is Downloaded +} \ No newline at end of file diff --git a/src/main/kotlin/com/coder/gateway/cli/ex/Exceptions.kt b/src/main/kotlin/com/coder/gateway/cli/ex/Exceptions.kt index 752ffaeda..448847bed 100644 --- a/src/main/kotlin/com/coder/gateway/cli/ex/Exceptions.kt +++ b/src/main/kotlin/com/coder/gateway/cli/ex/Exceptions.kt @@ -5,3 +5,5 @@ class ResponseException(message: String, val code: Int) : Exception(message) class SSHConfigFormatException(message: String) : Exception(message) class MissingVersionException(message: String) : Exception(message) + +class UnsignedBinaryExecutionDeniedException(message: String?) : Exception(message) diff --git a/src/main/kotlin/com/coder/gateway/cli/gpg/GPGVerifier.kt b/src/main/kotlin/com/coder/gateway/cli/gpg/GPGVerifier.kt new file mode 100644 index 000000000..ec1040ded --- /dev/null +++ b/src/main/kotlin/com/coder/gateway/cli/gpg/GPGVerifier.kt @@ -0,0 +1,142 @@ +package com.coder.gateway.cli.gpg + +import com.coder.gateway.settings.CoderSettings +import com.coder.gateway.cli.gpg.VerificationResult.Failed +import com.coder.gateway.cli.gpg.VerificationResult.Invalid +import com.coder.gateway.cli.gpg.VerificationResult.SignatureNotFound +import com.coder.gateway.cli.gpg.VerificationResult.Valid +import com.intellij.openapi.diagnostic.Logger +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import org.bouncycastle.bcpg.ArmoredInputStream +import org.bouncycastle.openpgp.PGPException +import org.bouncycastle.openpgp.PGPPublicKey +import org.bouncycastle.openpgp.PGPPublicKeyRing +import org.bouncycastle.openpgp.PGPPublicKeyRingCollection +import org.bouncycastle.openpgp.PGPSignatureList +import org.bouncycastle.openpgp.PGPUtil +import org.bouncycastle.openpgp.jcajce.JcaPGPObjectFactory +import org.bouncycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator +import org.bouncycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider +import java.io.ByteArrayInputStream +import java.nio.file.Files +import java.nio.file.Path +import kotlin.io.path.inputStream + +class GPGVerifier( + private val settings: CoderSettings +) { + + suspend fun verifySignature( + cli: Path, + signature: Path, + ): VerificationResult { + return try { + if (!Files.exists(signature)) { + logger.warn("Signature file not found, skipping verification") + return SignatureNotFound + } + + val (signatureBytes, publicKeyRing) = withContext(Dispatchers.IO) { + val signatureBytes = Files.readAllBytes(signature) + val publicKeyRing = getCoderPublicKeyRings() + + Pair(signatureBytes, publicKeyRing) + } + return verifyDetachedSignature( + cliPath = cli, + signatureBytes = signatureBytes, + publicKeyRings = publicKeyRing + ) + } catch (e: Exception) { + logger.error("GPG signature verification failed", e) + Failed(e) + } + } + + private fun getCoderPublicKeyRings(): List { + try { + val coderPublicKey = javaClass.getResourceAsStream("/META-INF/trusted-keys/pgp-public.key") + ?.readAllBytes() ?: throw IllegalStateException("Trusted public key not found") + return loadPublicKeyRings(coderPublicKey) + } catch (e: Exception) { + throw PGPException("Failed to load Coder public GPG key", e) + } + } + + /** + * Load public key rings from bytes + */ + fun loadPublicKeyRings(publicKeyBytes: ByteArray): List { + return try { + val keyInputStream = ArmoredInputStream(ByteArrayInputStream(publicKeyBytes)) + val keyRingCollection = PGPPublicKeyRingCollection( + PGPUtil.getDecoderStream(keyInputStream), + JcaKeyFingerprintCalculator() + ) + keyRingCollection.keyRings.asSequence().toList() + } catch (e: Exception) { + throw PGPException("Failed to load public key ring", e) + } + } + + /** + * Verify a detached GPG signature + */ + fun verifyDetachedSignature( + cliPath: Path, + signatureBytes: ByteArray, + publicKeyRings: List + ): VerificationResult { + try { + val signatureInputStream = ArmoredInputStream(ByteArrayInputStream(signatureBytes)) + val pgpObjectFactory = JcaPGPObjectFactory(signatureInputStream) + val signatureList = pgpObjectFactory.nextObject() as? PGPSignatureList + ?: throw PGPException("Invalid signature format") + + if (signatureList.isEmpty) { + return Invalid("No signatures found in signature file") + } + + val signature = signatureList[0] + val publicKey = findPublicKey(publicKeyRings, signature.keyID) + ?: throw PGPException("Public key not found for signature") + + signature.init(JcaPGPContentVerifierBuilderProvider(), publicKey) + cliPath.inputStream().use { fileStream -> + val buffer = ByteArray(8192) + var bytesRead: Int + while (fileStream.read(buffer).also { bytesRead = it } != -1) { + signature.update(buffer, 0, bytesRead) + } + } + + val isValid = signature.verify() + logger.info("GPG signature verification result: $isValid") + if (isValid) { + return Valid + } + return Invalid() + } catch (e: Exception) { + logger.error("GPG signature verification failed", e) + return Failed(e) + } + } + + /** + * Find a public key across all key rings in the collection + */ + private fun findPublicKey( + keyRings: List, + keyId: Long + ): PGPPublicKey? { + keyRings.forEach { keyRing -> + keyRing.getPublicKey(keyId)?.let { return it } + } + return null + } + + companion object { + val logger = Logger.getInstance(GPGVerifier::class.java.simpleName) + } +} \ No newline at end of file diff --git a/src/main/kotlin/com/coder/gateway/cli/gpg/VerificationResult.kt b/src/main/kotlin/com/coder/gateway/cli/gpg/VerificationResult.kt new file mode 100644 index 000000000..5e7a94ff4 --- /dev/null +++ b/src/main/kotlin/com/coder/gateway/cli/gpg/VerificationResult.kt @@ -0,0 +1,15 @@ +package com.coder.gateway.cli.gpg + +/** + * Result of signature verification + */ +sealed class VerificationResult { + object Valid : VerificationResult() + data class Invalid(val reason: String? = null) : VerificationResult() + data class Failed(val error: Exception) : VerificationResult() + object SignatureNotFound : VerificationResult() + + fun isValid(): Boolean = this == Valid + fun isInvalid(): Boolean = this is Invalid + fun signatureIsNotFound(): Boolean = this == SignatureNotFound +} diff --git a/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt b/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt index aa46ba574..197dc14cf 100644 --- a/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt +++ b/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt @@ -1,5 +1,6 @@ package com.coder.gateway.settings +import com.coder.gateway.cli.gpg.VerificationResult import com.coder.gateway.util.Arch import com.coder.gateway.util.OS import com.coder.gateway.util.expand @@ -8,6 +9,7 @@ import com.coder.gateway.util.getOS import com.coder.gateway.util.safeHost import com.coder.gateway.util.toURL import com.coder.gateway.util.withPath +import com.github.weisj.jsvg.B import com.intellij.openapi.diagnostic.Logger import java.net.URL import java.nio.file.Files @@ -34,7 +36,7 @@ enum class Source { * Return a description of the source. */ fun description(name: String): String = when (this) { - CONFIG -> "This $name was pulled from your global CLI config." + CONFIG -> "This $name was pulled from your global CLI config." DEPLOYMENT_CONFIG -> "This $name was pulled from your deployment's CLI config." LAST_USED -> "This was the last used $name." QUERY -> "This $name was pulled from the Gateway link." @@ -63,6 +65,12 @@ open class CoderSettingsState( // Whether to allow the plugin to fall back to the data directory when the // CLI directory is not writable. open var enableBinaryDirectoryFallback: Boolean = false, + + /** + * Controls whether we fall back release.coder.com + */ + open var fallbackOnCoderForSignatures: Boolean = false, + // An external command that outputs additional HTTP headers added to all // requests. The command must output each header as `key=value` on its own // line. The following environment variables will be available to the @@ -154,6 +162,17 @@ open class CoderSettings( val enableBinaryDirectoryFallback: Boolean get() = state.enableBinaryDirectoryFallback + /** + * Controls whether we fall back release.coder.com + */ + val fallbackOnCoderForSignatures: Boolean + get() = state.fallbackOnCoderForSignatures + + /** + * Default CLI signature name based on OS and architecture + */ + val defaultSignatureNameByOsAndArch: String get() = getCoderSignatureForOS(getOS(), getArch()) + /** * A command to run to set headers for API calls. */ @@ -306,12 +325,12 @@ open class CoderSettings( // SSH has not been configured yet, or using some other authorization mechanism. null } to - try { - Files.readString(dir.resolve("session")) - } catch (e: Exception) { - // SSH has not been configured yet, or using some other authorization mechanism. - null - } + try { + Files.readString(dir.resolve("session")) + } catch (e: Exception) { + // SSH has not been configured yet, or using some other authorization mechanism. + null + } } /** @@ -374,41 +393,59 @@ open class CoderSettings( } /** - * Return the name of the binary (with extension) for the provided OS and - * architecture. + * Return the name of the binary (with extension) for the provided OS and architecture. */ private fun getCoderCLIForOS( os: OS?, arch: Arch?, ): String { - logger.info("Resolving binary for $os $arch") + logger.debug("Resolving binary for $os $arch") + return buildCoderFileName(os, arch) + } + + /** + * Return the name of the signature file (.asc) for the provided OS and architecture. + */ + private fun getCoderSignatureForOS( + os: OS?, + arch: Arch?, + ): String { + logger.debug("Resolving signature for $os $arch") + return buildCoderFileName(os, arch, true) + } + + /** + * Build the coder file name based on OS, architecture, and whether it's a signature file. + */ + private fun buildCoderFileName( + os: OS?, + arch: Arch?, + isSignature: Boolean = false + ): String { if (os == null) { logger.error("Could not resolve client OS and architecture, defaulting to WINDOWS AMD64") - return "coder-windows-amd64.exe" + return if (isSignature) "coder-windows-amd64.asc" else "coder-windows-amd64.exe" } - return when (os) { - OS.WINDOWS -> - when (arch) { - Arch.AMD64 -> "coder-windows-amd64.exe" - Arch.ARM64 -> "coder-windows-arm64.exe" - else -> "coder-windows-amd64.exe" - } - OS.LINUX -> - when (arch) { - Arch.AMD64 -> "coder-linux-amd64" - Arch.ARM64 -> "coder-linux-arm64" - Arch.ARMV7 -> "coder-linux-armv7" - else -> "coder-linux-amd64" - } + val osName = when (os) { + OS.WINDOWS -> "windows" + OS.LINUX -> "linux" + OS.MAC -> "darwin" + } - OS.MAC -> - when (arch) { - Arch.AMD64 -> "coder-darwin-amd64" - Arch.ARM64 -> "coder-darwin-arm64" - else -> "coder-darwin-amd64" - } + val archName = when (arch) { + Arch.AMD64 -> "amd64" + Arch.ARM64 -> "arm64" + Arch.ARMV7 -> "armv7" + else -> "amd64" // default fallback + } + + val extension = if (isSignature) ".asc" else when (os) { + OS.WINDOWS -> ".exe" + OS.LINUX, OS.MAC -> "" } + + return "coder-$osName-$archName$extension" } companion object { diff --git a/src/main/kotlin/com/coder/gateway/util/LinkHandler.kt b/src/main/kotlin/com/coder/gateway/util/LinkHandler.kt index f802109cc..8ee600e58 100644 --- a/src/main/kotlin/com/coder/gateway/util/LinkHandler.kt +++ b/src/main/kotlin/com/coder/gateway/util/LinkHandler.kt @@ -28,7 +28,7 @@ open class LinkHandler( * Throw if required arguments are not supplied or the workspace is not in a * connectable state. */ - fun handle( + suspend fun handle( parameters: Map, indicator: ((t: String) -> Unit)? = null, ): WorkspaceProjectIDE { diff --git a/src/main/kotlin/com/coder/gateway/util/SemVer.kt b/src/main/kotlin/com/coder/gateway/util/SemVer.kt index eaf0034d4..435bdb1bf 100644 --- a/src/main/kotlin/com/coder/gateway/util/SemVer.kt +++ b/src/main/kotlin/com/coder/gateway/util/SemVer.kt @@ -1,6 +1,6 @@ package com.coder.gateway.util -class SemVer(private val major: Long = 0, private val minor: Long = 0, private val patch: Long = 0) : Comparable { +class SemVer(val major: Long = 0, val minor: Long = 0, val patch: Long = 0) : Comparable { init { require(major >= 0) { "Coder major version must be a positive number" } require(minor >= 0) { "Coder minor version must be a positive number" } diff --git a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt index 1928a4c46..f29d9a591 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -14,6 +14,7 @@ import com.coder.gateway.sdk.v2.models.WorkspaceStatus import com.coder.gateway.sdk.v2.models.toAgentList import com.coder.gateway.services.CoderRestClientService import com.coder.gateway.services.CoderSettingsService +import com.coder.gateway.services.CoderSettingsStateService import com.coder.gateway.settings.Source import com.coder.gateway.util.DialogUi import com.coder.gateway.util.InvalidVersionException @@ -40,6 +41,7 @@ import com.intellij.openapi.wm.impl.welcomeScreen.WelcomeScreenUIManager import com.intellij.ui.AnActionButton import com.intellij.ui.RelativeFont import com.intellij.ui.ToolbarDecorator +import com.intellij.ui.components.JBCheckBox import com.intellij.ui.dsl.builder.AlignX import com.intellij.ui.dsl.builder.AlignY import com.intellij.ui.dsl.builder.BottomGap @@ -116,6 +118,7 @@ class CoderWorkspacesStepView : CoderWizardStep( CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.next.text"), ) { + private val state: CoderSettingsStateService = service() private val settings: CoderSettingsService = service() private val dialogUi = DialogUi(settings) private val cs = CoroutineScope(Dispatchers.Main) @@ -130,6 +133,7 @@ class CoderWorkspacesStepView : private var tfUrl: JTextField? = null private var tfUrlComment: JLabel? = null private var cbExistingToken: JCheckBox? = null + private var cbFallbackOnSignature: JCheckBox? = null private val notificationBanner = NotificationBanner() private var tableOfWorkspaces = @@ -262,6 +266,19 @@ class CoderWorkspacesStepView : ) }.layout(RowLayout.PARENT_GRID) } + row { + cell() // For alignment. + checkBox(CoderGatewayBundle.message("gateway.connector.settings.fallback-on-coder-for-signatures.title")) + .bindSelected(state::fallbackOnCoderForSignatures).applyToComponent { + addActionListener { event -> + state.fallbackOnCoderForSignatures = (event.source as JBCheckBox).isSelected + } + } + .comment( + CoderGatewayBundle.message("gateway.connector.settings.fallback-on-coder-for-signatures.comment"), + ) + + }.layout(RowLayout.PARENT_GRID) row { scrollCell( toolbar.createPanel().apply { diff --git a/src/main/resources/messages/CoderGatewayBundle.properties b/src/main/resources/messages/CoderGatewayBundle.properties index f318012e0..3364e6f34 100644 --- a/src/main/resources/messages/CoderGatewayBundle.properties +++ b/src/main/resources/messages/CoderGatewayBundle.properties @@ -75,6 +75,10 @@ gateway.connector.settings.enable-binary-directory-fallback.title=Fall back to d gateway.connector.settings.enable-binary-directory-fallback.comment=Checking this \ box will allow the plugin to fall back to the data directory when the CLI \ directory is not writable. + +gateway.connector.settings.fallback-on-coder-for-signatures.title=Fall back on releases.coder.com for signatures +gateway.connector.settings.fallback-on-coder-for-signatures.comment=Verify binary signature using releases.coder.com when CLI signatures are not available from the deployment + gateway.connector.settings.header-command.title=Header command gateway.connector.settings.header-command.comment=An external command that \ outputs additional HTTP headers added to all requests. The command must \ From 94e74f5f65a51b7dea732405db316e90d4e79220 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 22 Jul 2025 23:00:18 +0300 Subject: [PATCH 02/10] fix: class cast exception --- src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt index 283844856..207d65a1b 100644 --- a/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt @@ -267,8 +267,8 @@ class CoderCLIManager( } else -> { - UnsignedBinaryExecutionDeniedException((result as DownloadResult.Failed).error.message) - val failure = result as DownloadResult.Failed + val failure = result as VerificationResult.Failed + UnsignedBinaryExecutionDeniedException(result.error.message) logger.error("Failed to verify signature for ${cliResult.dst}", failure.error) } } From 345371ff30de67cceedb39b1d681e85478c3b2cb Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Tue, 22 Jul 2025 23:21:27 +0300 Subject: [PATCH 03/10] impl: embed the pgp public key as a plugin resource This is the key that validates if the gpg signature was tampered --- .../META-INF/trusted-keys/pgp-public.key | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 src/main/resources/META-INF/trusted-keys/pgp-public.key diff --git a/src/main/resources/META-INF/trusted-keys/pgp-public.key b/src/main/resources/META-INF/trusted-keys/pgp-public.key new file mode 100644 index 000000000..fb5c4c502 --- /dev/null +++ b/src/main/resources/META-INF/trusted-keys/pgp-public.key @@ -0,0 +1,99 @@ +-----BEGIN PGP PUBLIC KEY BLOCK----- + +mQINBGPGrCwBEAC7SSKQIFoQdt3jYv/1okRdoleepLDG4NfcG52S45Ex3/fUA6Z/ +ewHQrx//SN+h1FLpb0zQMyamWrSh2O3dnkWridwlskb5/y8C/6OUdk4L/ZgHeyPO +Ncbyl1hqO8oViakiWt4IxwSYo83eJHxOUiCGZlqV6EpEsaur43BRHnK8EciNeIxF +Bjle3yXH1K3EgGGHpgnSoKe1nSVxtWIwX45d06v+VqnBoI6AyK0Zp+Nn8bL0EnXC +xGYU3XOkC6EmITlhMju1AhxnbkQiy8IUxXiaj3NoPc1khapOcyBybhESjRZHlgu4 +ToLZGaypjtfQJgMeFlpua7sJK0ziFMW4wOTX+6Ix/S6XA80dVbl3VEhSMpFCcgI+ +OmEd2JuBs6maG+92fCRIzGAClzV8/ifM//JU9D7Qlq6QJpcbNClODlPNDNe7RUEO +b7Bu7dJJS3VhHO9eEen6m6vRE4DNriHT4Zvq1UkHfpJUW7njzkIYRni3eNrsr4Da +U/eeGbVipok4lzZEOQtuaZlX9ytOdGrWEGMGSosTOG6u6KAKJoz7cQGZiz4pZpjR +3N2SIYv59lgpHrIV7UodGx9nzu0EKBhkoulaP1UzH8F16psSaJXRjeyl/YP8Rd2z +SYgZVLjTzkTUXkJT8fQO8zLBEuwA0IiXX5Dl7grfEeShANVrM9LVu8KkUwARAQAB +tC5Db2RlciBSZWxlYXNlIFNpZ25pbmcgS2V5IDxzZWN1cml0eUBjb2Rlci5jb20+ +iQJUBBMBCgA+FiEEKMY4lDj2Q3PIwvSKi87Yfbu4ZEsFAmPGrCwCGwMFCQWjmoAF +CwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQi87Yfbu4ZEvrQQ//a3ySdMVhnLP+ +KneonV2zuNilTMC2J/MNG7Q0hU+8I9bxCc6DDqcnBBCQkIUwJq3wmelt3nTC8RxI +fv+ggnbdF9pz7Fc91nIJsGlWpH+bu1tSIvKF/rzZA8v6xUblFFfaC7Gsc5P4xk/+ +h0XBDAy6K+7+AafgLFpRD08Y0Kf2aMcqdM6c2Zo4IPo6FNrOa66FNkypZdQ4IByW +4kMezZSTp4Phqd9yqGC4m44U8YgzmW9LHgrvS0JyIaRPcQFM31AJ50K3iYRxL1ll +ETqJvbDR8UORNQs3Qs3CEZL588BoDMX2TYObTCG6g9Om5vJT0kgUkjDxQHwbAj6E +z9j8BoWkDT2JNzwdfTbPueuRjO+A+TXA9XZtrzbEYEzh0sD9Bdr7ozSF3JAs4GZS +nqcVlyp7q44ZdePR9L8w0ksth56tBWHfE9hi5jbRDRY2OnkV7y7JtWnBDQx9bCIo +7L7aBT8eirI1ZOnUxHJrnqY5matfWjSDBFW+YmWUkjnzBsa9F4m8jq9MSD3Q/8hN +ksJFrmLQs0/8hnM39tS7kLnAaWeGvbmjnxdeMqZsICxNpbyQrq2AhF4GhWfc+NsZ +yznVagJZ9bIlGsycSXJbsA5GbXDnm172TlodMUbLF9FU8i0vV4Y7q6jKO/VsblKU +F0bhXIRqVLrd9g88IyVyyZozmwbJKIy5Ag0EY8asLAEQAMgI9bMurq6Zic4s5W0u +W6LBDHyZhe+w2a3oT/i2YgTsh8XmIjrNasYYWO67b50JKepA3fk3ZA44w8WJqq+z +HLpslEb2fY5I1HvENUMKjYAUIsswSC21DSBau4yYiRGF0MNqv/MWy5Rjc993vIU4 +4TM3mvVhPrYfIkr0jwSbxq8+cm3sBjr0gcBQO57C3w8QkcZ6jefuI7y+1ZeM7X3L +OngmBFJDEutd9LPO/6Is4j/iQfTb8WDR6OmMX3Y04RHrP4sm7jf+3ZZKjcFCZQjr +QA4XHcQyJjnMN34Fn1U7KWopivU+mqViAnVpA643dq9SiBqsl83/R03DrpwKpP7r +6qasUHSUULuS7A4n8+CDwK5KghvrS0hOwMiYoIwZIVPITSUFHPYxrCJK7gU2OHfk +IZHX5m9L5iNwLz958GwzwHuONs5bjMxILbKknRhEBOcbhcpk0jswiPNUrEdipRZY +GR9G9fzD6q4P5heV3kQRqyUUTxdDj8w7jbrwl8sm5zk+TMnPRsu2kg0uwIN1aILm +oVkDN5CiZtg00n2Fu3do5F3YkF0Cz7indx5yySr5iUuoCY0EnpqSwourJ/ZdZA9Y +ZCHjhgjwyPCbxpTGfLj1g25jzQBYn5Wdgr2aHCQcqnU8DKPCnYL9COHJJylgj0vN +NSxyDjNXYYwSrYMqs/91f5xVABEBAAGJAjwEGAEKACYWIQQoxjiUOPZDc8jC9IqL +zth9u7hkSwUCY8asLAIbDAUJBaOagAAKCRCLzth9u7hkSyMvD/0Qal5kwiKDjgBr +i/dtMka+WNBTMb6vKoM759o33YAl22On5WgLr9Uz0cjkJPtzMHxhUo8KQmiPRtsK +dOmG9NI9NttfSeQVbeL8V/DC672fWPKM4TB8X7Kkj56/KI7ueGRokDhXG2pJlhQr +HwzZsAKoCMMnjcquAhHJClK9heIpVLBGFVlmVzJETzxo6fbEU/c7L79+hOrR4BWx +Tg6Dk7mbAGe7BuQLNtw6gcWUVWtHS4iYQtE/4khU1QppC1Z/ZbZ+AJT2TAFXzIaw +0l9tcOh7+TXqsvCLsXN0wrUh1nOdxA81sNWEMY07bG1qgvHyVc7ZYM89/ApK2HP+ +bBDIpAsRCGu2MHtrnJIlNE1J14G1mnauR5qIqI3C0R5MPLXOcDtp+gnjFe+PLU+6 +rQxJObyOkyEpOvtVtJKfFnpI5bqyl8WEPN0rDaS2A27cGXi5nynSAqoM1xT15W21 +uyY2GXY26DIwVfc59wGeclwcM29nS7prRU3KtskjonJ0iQoQebYOHLxy896cK+pK +nnhZx5AQjYiZPsPktSNZjSuOvTZ3g+IDwbCSvmBHcQpitzUOPShTUTs0QjSttzk2 +I6WxP9ivoR9yJGsxwNgCgrYdyt5+hyXXW/aUVihnQwizQRbymjJ2/z+I8NRFIeYb +xbtNFaH3WjLnhm9CB/H+Lc8fUj6HaZkCDQRjxt6QARAAsjZuCMjZBaAC1LFMeRcv +9+Ck7T5UNXTL9xQr1jUFZR95I6loWiWvFJ3Uet7gIbgNYY5Dc1gDr1Oqx9KQBjsN +TUahXov5lmjF5mYeyWTDZ5TS8H3o50zQzfZRC1eEbqjiBMLAHv74KD13P62nvzv6 +Dejwc7Nwc6aOH3cdZm74kz4EmdobJYRVdd5X9EYH/hdM928SsipKhm44oj3RDGi/ +x+ptjW9gr0bnrgCbkyCMNKhnmHSM60I8f4/viRItb+hWRpZYfLxMGTBVunicSXcX +Zh6Fq/DD/yTjzN9N83/NdDvwCyKo5U/kPgD2Ixh5PyJ38cpz6774Awnb/tstCI1g +glnlNbu8Qz84STr3NRZMOgT5h5b5qASOeruG4aVo9euaYJHlnlgcoUmpbEMnwr0L +tREUXSHGXWor7EYPjUQLskIaPl9NCZ3MEw5LhsZTgEdFBnb54dxMSEl7/MYDYhD/ +uTIWOJmtsWHmuMmvfxnw5GDEhJnAp4dxUm9BZlJhfnVR07DtTKyEk37+kl6+i0ZQ +yU4HJ2GWItpLfK54E/CH+S91y7wpepb2TMkaFR2fCK0vXTGAXWK+Y+aTD8ZcLB5y +0IYPsvA0by5AFpmXNfWZiZtYvgJ5FAQZNuB5RILg3HsuDq2U4wzp5BoohWtsOzsn +antIUf/bN0D2g+pCySkc5ssAEQEAAbQuQ29kZXIgUmVsZWFzZSBTaWduaW5nIEtl +eSA8c2VjdXJpdHlAY29kZXIuY29tPokCVAQTAQoAPhYhBCHJaxy5UHGIdPZNvWpa +ZxteQKO5BQJjxt6QAhsDBQkFo5qABQsJCAcCBhUKCQgLAgQWAgMBAh4BAheAAAoJ +EGpaZxteQKO5oysP/1rSdvbKMzozvnVZoglnPjnSGStY9Pr2ziGL7eIMk2yt+Orr +j/AwxYIDgsZPQoJEr87eX2dCYtUMM1x+CpZsWu8dDVFLxyZp8nPmhUzcUCFfutw1 +UmAVKQkOra9segZtw4HVcSctpdgLw7NHq7vIQm4knIvjWmdC15r1B6/VJJI8CeaR +Zy+ToPr9fKnYs1RNdz+DRDN2521skX1DaInhB/ALeid90rJTRujaP9XeyNb9k32K +qd3h4C0KUGIf0fNKj4mmDlNosX3V/pJZATpFiF8aVPlybHQ2W5xpn1U8FJxE4hgR +rvsZmO685Qwm6p/uRI5Eymfm8JC5OQNt9Kvs/BMhotsW0u+je8UXwnznptMILpVP ++qxNuHUe1MYLdjK21LFF+Pk5O4W1TT6mKcbisOmZuQMG5DxpzUwm1Rs5AX1omuJt +iOrmQEvmrKKWC9qbcmWW1t2scnIJsNtrsvME0UjJFz+RL6UUX3xXlLK6YOUghCr8 +gZ7ZPgFqygS6tMu8TAGURzSCfijDh+eZGwqrlvngBIaO5WiNdSXC/J9aE1KThXmX +90A3Gwry+yI2kRS7o8vmghXewPTZbnG0CVHiQIH2yqFNXnhKvhaJt0g04TcnxBte +kiFqRT4K1Bb7pUIlUANmrKo9/zRCxIOopEgRH5cVQ8ZglkT0t5d3ePmAo6h0uQIN +BGPG3pABEADghhNByVoC+qCMo+SErjxz9QYA+tKoAngbgPyxxyB4RD52Z58MwVaP ++Yk0qxJYUBat3dJwiCTlUGG+yTyMOwLl7qSDr53AD5ml0hwJqnLBJ6OUyGE4ax4D +RUVBprKlDltwr98cZDgzvwEhIO2T3tNZ4vySveITj9pLonOrLkAfGXqFOqom+S37 +6eZvjKTnEUbT+S0TTynwds70W31sxVUrL62qsUnmoKEnsKXk/7X8CLXWvtNqu9kf +eiXs5Jz4N6RZUqvS0WOaaWG9v1PHukTtb8RyeookhsBqf9fWOlw5foel+NQwGQjz +0D0dDTKxn2Taweq+gWNCRH7/FJNdWa9upZ2fUAjg9hN9Ow8Y5nE3J0YKCBAQTgNa +XNtsiGQjdEKYZslxZKFM34By3LD6IrkcAEPKu9plZthmqhQumqwYRAgB9O56jg3N +GDDRyAMS7y63nNphTSatpOZtPVVMtcBw5jPjMIPFfU2dlfsvmnCvru2dvfAij+Ng +EkwOLNS8rFQHMJSQysmHuAPSYT97Yl022mPrAtb9+hwtCXt3VI6dvIARl2qPyF0D +DMw2fW5E7ivhUr2WEFiBmXunrJvMIYldBzDkkBjamelPjoevR0wfoIn0x1CbSsQi +zbEs3PXHs7nGxb9TZnHY4+J94mYHdSXrImAuH/x97OnlfUpOKPv5lwARAQABiQI8 +BBgBCgAmFiEEIclrHLlQcYh09k29alpnG15Ao7kFAmPG3pACGwwFCQWjmoAACgkQ +alpnG15Ao7m2/g//Y/YRM+Qhf71G0MJpAfym6ZqmwsT78qQ8T9w95ZeIRD7UUE8d +tm39kqJTGP6DuHCNYEMs2M88o0SoQsS/7j/8is7H/13F5o40DWjuQphia2BWkB1B +G4QRRIXMlrPX8PS92GDCtGfvxn90Li2FhQGZWlNFwvKUB7+/yLMsZzOwo7BS6PwC +hvI3eC7DBC8sXjJUxsrgFAkxQxSx/njP8f4HdUwhNnB1YA2/5IY5bk8QrXxzrAK1 +sbIAjpJdtPYOrZByyyj4ZpRcSm3ngV2n8yd1muJ5u+oRIQoGCdEIaweCj598jNFa +k378ZA11hCyNFHjpPIKnF3tfsQ8vjDatoq4Asy+HXFuo1GA/lvNgNb3Nv4FUozuv +JYJ0KaW73FZXlFBIBkMkRQE8TspHy2v/IGyNXBwKncmkszaiiozBd+T+1NUZgtk5 +9o5uKQwLHVnHIU7r/w/oN5LvLawLg2dP/f2u/KoQXMxjwLZncSH4+5tRz4oa/GMn +k4F84AxTIjGfLJeXigyP6xIPQbvJy+8iLRaCpj+v/EPwAedbRV+u0JFeqqikca70 +aGN86JBOmwpU87sfFxLI7HdI02DkvlxYYK3vYlA6zEyWaeLZ3VNr6tHcQmOnFe8Q +26gcS0AQcxQZrcWTCZ8DJYF+RnXjSVRmHV/3YDts4JyMKcD6QX8s/3aaldk= +=dLmT +-----END PGP PUBLIC KEY BLOCK----- \ No newline at end of file From c7af60352b936c4255b49f8def43cb2a4ca9fd92 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 23 Jul 2025 00:26:35 +0300 Subject: [PATCH 04/10] chore: fix UTs related to CLI downloading For one thing some method signature changed, some methods are now suspending functions that will have to run in a coroutine in the tests. The second big issue is that now the download function requests user's input via a dialog --- build.gradle.kts | 1 + .../coder/gateway/cli/CoderCLIManagerTest.kt | 121 ++++++++++++++---- 2 files changed, 94 insertions(+), 28 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 9ea24a063..63965ed1b 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -56,6 +56,7 @@ dependencies { testImplementation(kotlin("test")) // required by the unit tests testImplementation(kotlin("test-junit5")) + testImplementation("io.mockk:mockk:1.13.12") // required by IntelliJ test framework testImplementation("junit:junit:4.13.2") diff --git a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt index 5ae754ecf..90c6a90f6 100644 --- a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt @@ -10,6 +10,7 @@ import com.coder.gateway.settings.CODER_SSH_CONFIG_OPTIONS import com.coder.gateway.settings.CoderSettings import com.coder.gateway.settings.CoderSettingsState import com.coder.gateway.settings.Environment +import com.coder.gateway.util.DialogUi import com.coder.gateway.util.InvalidVersionException import com.coder.gateway.util.OS import com.coder.gateway.util.SemVer @@ -19,6 +20,9 @@ import com.coder.gateway.util.sha1 import com.coder.gateway.util.toURL import com.squareup.moshi.JsonEncodingException import com.sun.net.httpserver.HttpServer +import io.mockk.every +import io.mockk.mockkConstructor +import kotlinx.coroutines.runBlocking import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.assertDoesNotThrow import org.zeroturnaround.exec.InvalidExitValueException @@ -28,7 +32,8 @@ import java.net.InetSocketAddress import java.net.URL import java.nio.file.AccessDeniedException import java.nio.file.Path -import java.util.* +import java.util.UUID +import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertContains import kotlin.test.assertEquals @@ -37,6 +42,9 @@ import kotlin.test.assertFalse import kotlin.test.assertNotEquals import kotlin.test.assertTrue +private const val VERSION_FOR_PROGRESS_REPORTING = "v2.13.1-devel+de07351b8" +private val noOpTextProgress: (String) -> Unit = { _ -> } + internal class CoderCLIManagerTest { /** * Return the contents of a script that contains the string. @@ -65,6 +73,9 @@ internal class CoderCLIManagerTest { if (exchange.requestURI.path == "/bin/override") { code = HttpURLConnection.HTTP_OK response = mkbinVersion("0.0.0") + } else if (exchange.requestURI.path.contains(".asc")) { + code = HttpURLConnection.HTTP_NOT_FOUND + response = "not found" } else if (!exchange.requestURI.path.startsWith("/bin/coder-")) { code = HttpURLConnection.HTTP_NOT_FOUND response = "not found" @@ -85,6 +96,13 @@ internal class CoderCLIManagerTest { return Pair(srv, URL("http://localhost:" + srv.address.port)) } + @BeforeTest + fun setup() { + // Mock the DialogUi constructor + mockkConstructor(DialogUi::class) + every { anyConstructed().confirm(any(), any()) } returns true + } + @Test fun testServerInternalError() { val (srv, url) = mockServer(HttpURLConnection.HTTP_INTERNAL_ERROR) @@ -93,7 +111,7 @@ internal class CoderCLIManagerTest { val ex = assertFailsWith( exceptionClass = ResponseException::class, - block = { ccm.download() }, + block = { runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) } } ) assertEquals(HttpURLConnection.HTTP_INTERNAL_ERROR, ex.code) @@ -145,7 +163,7 @@ internal class CoderCLIManagerTest { assertFailsWith( exceptionClass = AccessDeniedException::class, - block = { ccm.download() }, + block = { runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) } }, ) srv.stop(0) @@ -168,15 +186,16 @@ internal class CoderCLIManagerTest { CoderSettings( CoderSettingsState( dataDirectory = tmpdir.resolve("real-cli").toString(), + fallbackOnCoderForSignatures = true ), ), ) - assertTrue(ccm.download()) + assertTrue(runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertDoesNotThrow { ccm.version() } // It should skip the second attempt. - assertFalse(ccm.download()) + assertFalse(runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) // Make sure login failures propagate. assertFailsWith( @@ -194,16 +213,17 @@ internal class CoderCLIManagerTest { CoderSettings( CoderSettingsState( dataDirectory = tmpdir.resolve("mock-cli").toString(), + fallbackOnCoderForSignatures = true ), binaryName = "coder.bat", ), ) - assertEquals(true, ccm.download()) + assertEquals(true, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) // It should skip the second attempt. - assertEquals(false, ccm.download()) + assertEquals(false, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) // Should use the source override. ccm = @@ -213,11 +233,12 @@ internal class CoderCLIManagerTest { CoderSettingsState( binarySource = "/bin/override", dataDirectory = tmpdir.resolve("mock-cli").toString(), + fallbackOnCoderForSignatures = true ), ), ) - assertEquals(true, ccm.download()) + assertEquals(true, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertContains(ccm.localBinaryPath.toFile().readText(), "0.0.0") srv.stop(0) @@ -242,7 +263,7 @@ internal class CoderCLIManagerTest { } @Test - fun testOverwitesWrongVersion() { + fun testOverwritesWrongVersion() { val (srv, url) = mockServer() val ccm = CoderCLIManager( @@ -250,6 +271,7 @@ internal class CoderCLIManagerTest { CoderSettings( CoderSettingsState( dataDirectory = tmpdir.resolve("overwrite-cli").toString(), + fallbackOnCoderForSignatures = true ), ), ) @@ -261,7 +283,7 @@ internal class CoderCLIManagerTest { assertEquals("cli", ccm.localBinaryPath.toFile().readText()) assertEquals(0, ccm.localBinaryPath.toFile().lastModified()) - assertTrue(ccm.download()) + assertTrue(runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertNotEquals("cli", ccm.localBinaryPath.toFile().readText()) assertNotEquals(0, ccm.localBinaryPath.toFile().lastModified()) @@ -279,14 +301,15 @@ internal class CoderCLIManagerTest { CoderSettings( CoderSettingsState( dataDirectory = tmpdir.resolve("clobber-cli").toString(), + fallbackOnCoderForSignatures = true ), ) val ccm1 = CoderCLIManager(url1, settings) val ccm2 = CoderCLIManager(url2, settings) - assertTrue(ccm1.download()) - assertTrue(ccm2.download()) + assertTrue(runBlocking { ccm1.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) + assertTrue(runBlocking { ccm2.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) srv1.stop(0) srv2.stop(0) @@ -314,8 +337,12 @@ internal class CoderCLIManagerTest { fun testConfigureSSH() { val workspace = workspace("foo", agents = mapOf("agent1" to UUID.randomUUID().toString())) val workspace2 = workspace("bar", agents = mapOf("agent1" to UUID.randomUUID().toString())) - val betterWorkspace = workspace("foo", agents = mapOf("agent1" to UUID.randomUUID().toString()), ownerName = "bettertester") - val workspaceWithMultipleAgents = workspace("foo", agents = mapOf("agent1" to UUID.randomUUID().toString(), "agent2" to UUID.randomUUID().toString())) + val betterWorkspace = + workspace("foo", agents = mapOf("agent1" to UUID.randomUUID().toString()), ownerName = "bettertester") + val workspaceWithMultipleAgents = workspace( + "foo", + agents = mapOf("agent1" to UUID.randomUUID().toString(), "agent2" to UUID.randomUUID().toString()) + ) val extraConfig = listOf( @@ -331,7 +358,12 @@ internal class CoderCLIManagerTest { SSHTest(listOf(workspace), "existing-end", "replace-end", "no-blocks"), SSHTest(listOf(workspace), "existing-end-no-newline", "replace-end-no-newline", "no-blocks"), SSHTest(listOf(workspace), "existing-middle", "replace-middle", "no-blocks"), - SSHTest(listOf(workspace), "existing-middle-and-unrelated", "replace-middle-ignore-unrelated", "no-related-blocks"), + SSHTest( + listOf(workspace), + "existing-middle-and-unrelated", + "replace-middle-ignore-unrelated", + "no-related-blocks" + ), SSHTest(listOf(workspace), "existing-only", "replace-only", "blank"), SSHTest(listOf(workspace), "existing-start", "replace-start", "no-blocks"), SSHTest(listOf(workspace), "no-blocks", "append-no-blocks", "no-blocks"), @@ -463,7 +495,10 @@ internal class CoderCLIManagerTest { Path.of("src/test/fixtures/outputs/").resolve(it.output + ".conf").toFile().readText() .replace(newlineRe, System.lineSeparator()) .replace("/tmp/coder-gateway/test.coder.invalid/config", escape(coderConfigPath.toString())) - .replace("/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", escape(ccm.localBinaryPath.toString())) + .replace( + "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", + escape(ccm.localBinaryPath.toString()) + ) .let { conf -> if (it.sshLogDirectory != null) { conf.replace("/tmp/coder-gateway/test.coder.invalid/logs", it.sshLogDirectory.toString()) @@ -476,7 +511,10 @@ internal class CoderCLIManagerTest { Path.of("src/test/fixtures/inputs/").resolve(it.remove + ".conf").toFile().readText() .replace(newlineRe, System.lineSeparator()) .replace("/tmp/coder-gateway/test.coder.invalid/config", escape(coderConfigPath.toString())) - .replace("/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", escape(ccm.localBinaryPath.toString())) + .replace( + "/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", + escape(ccm.localBinaryPath.toString()) + ) .let { conf -> if (it.sshLogDirectory != null) { conf.replace("/tmp/coder-gateway/test.coder.invalid/logs", it.sshLogDirectory.toString()) @@ -552,7 +590,10 @@ internal class CoderCLIManagerTest { "new\nline", ) - val workspace = workspace("foo", agents = mapOf("agentid1" to UUID.randomUUID().toString(), "agentid2" to UUID.randomUUID().toString())) + val workspace = workspace( + "foo", + agents = mapOf("agentid1" to UUID.randomUUID().toString(), "agentid2" to UUID.randomUUID().toString()) + ) val withAgents = workspace.latestBuild.resources.filter { it.agents != null }.flatMap { it.agents!! }.map { workspace to it } @@ -730,8 +771,24 @@ internal class CoderCLIManagerTest { EnsureCLITest(null, null, "1.0.0", false, true, true, Result.DL_DATA), // Download to fallback. EnsureCLITest(null, null, "1.0.0", false, false, true, Result.NONE), // No download, error when used. EnsureCLITest("1.0.1", "1.0.1", "1.0.0", false, true, true, Result.DL_DATA), // Update fallback. - EnsureCLITest("1.0.1", "1.0.2", "1.0.0", false, false, true, Result.USE_BIN), // No update, use outdated. - EnsureCLITest(null, "1.0.2", "1.0.0", false, false, true, Result.USE_DATA), // No update, use outdated fallback. + EnsureCLITest( + "1.0.1", + "1.0.2", + "1.0.0", + false, + false, + true, + Result.USE_BIN + ), // No update, use outdated. + EnsureCLITest( + null, + "1.0.2", + "1.0.0", + false, + false, + true, + Result.USE_DATA + ), // No update, use outdated fallback. EnsureCLITest("1.0.0", null, "1.0.0", false, false, true, Result.USE_BIN), // Use existing. EnsureCLITest("1.0.1", "1.0.0", "1.0.0", false, false, true, Result.USE_DATA), // Use existing fallback. ) @@ -746,6 +803,7 @@ internal class CoderCLIManagerTest { enableBinaryDirectoryFallback = it.enableFallback, dataDirectory = tmpdir.resolve("ensure-data-dir").toString(), binaryDirectory = tmpdir.resolve("ensure-bin-dir").toString(), + fallbackOnCoderForSignatures = true ), ) @@ -777,34 +835,39 @@ internal class CoderCLIManagerTest { Result.ERROR -> { assertFailsWith( exceptionClass = AccessDeniedException::class, - block = { ensureCLI(url, it.buildVersion, settings) }, + block = { runBlocking { ensureCLI(url, it.buildVersion, settings, noOpTextProgress) } } ) } + Result.NONE -> { - val ccm = ensureCLI(url, it.buildVersion, settings) + val ccm = runBlocking { ensureCLI(url, it.buildVersion, settings, noOpTextProgress) } assertEquals(settings.binPath(url), ccm.localBinaryPath) assertFailsWith( exceptionClass = ProcessInitException::class, block = { ccm.version() }, ) } + Result.DL_BIN -> { - val ccm = ensureCLI(url, it.buildVersion, settings) + val ccm = runBlocking { ensureCLI(url, it.buildVersion, settings, noOpTextProgress) } assertEquals(settings.binPath(url), ccm.localBinaryPath) assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) } + Result.DL_DATA -> { - val ccm = ensureCLI(url, it.buildVersion, settings) + val ccm = runBlocking { ensureCLI(url, it.buildVersion, settings, noOpTextProgress) } assertEquals(settings.binPath(url, true), ccm.localBinaryPath) assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) } + Result.USE_BIN -> { - val ccm = ensureCLI(url, it.buildVersion, settings) + val ccm = runBlocking { ensureCLI(url, it.buildVersion, settings, noOpTextProgress) } assertEquals(settings.binPath(url), ccm.localBinaryPath) assertEquals(SemVer.parse(it.version ?: ""), ccm.version()) } + Result.USE_DATA -> { - val ccm = ensureCLI(url, it.buildVersion, settings) + val ccm = runBlocking { ensureCLI(url, it.buildVersion, settings, noOpTextProgress) } assertEquals(settings.binPath(url, true), ccm.localBinaryPath) assertEquals(SemVer.parse(it.fallbackVersion ?: ""), ccm.version()) } @@ -838,11 +901,12 @@ internal class CoderCLIManagerTest { CoderSettings( CoderSettingsState( dataDirectory = tmpdir.resolve("features").toString(), + fallbackOnCoderForSignatures = true ), binaryName = "coder.bat", ), ) - assertEquals(true, ccm.download()) + assertEquals(true, runBlocking { ccm.download(VERSION_FOR_PROGRESS_REPORTING, noOpTextProgress) }) assertEquals(it.second, ccm.features, "version: ${it.first}") srv.stop(0) @@ -850,7 +914,8 @@ internal class CoderCLIManagerTest { } companion object { - private val tmpdir: Path = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway-test/cli-manager") + private val tmpdir: Path = + Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway-test/cli-manager") @JvmStatic @BeforeAll From 8f5e5596b8737e7d2e1a8866131fbca7a8c58e88 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 23 Jul 2025 01:39:04 +0300 Subject: [PATCH 05/10] fix: download the correct CLI signature for Windows The signature for windows CLI follows the format: coder-windows-amd64.exe.asc Currently it is coded to coder-windows-amd64.asc which means the plugin always fail to find any signature for windows cli --- build.gradle.kts | 1 + .../coder/gateway/settings/CoderSettings.kt | 68 +++------ src/main/kotlin/com/coder/gateway/util/OS.kt | 11 +- .../gateway/settings/CoderSettingsTest.kt | 142 +++++++++++++----- 4 files changed, 138 insertions(+), 84 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 63965ed1b..a126d0010 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -57,6 +57,7 @@ dependencies { // required by the unit tests testImplementation(kotlin("test-junit5")) testImplementation("io.mockk:mockk:1.13.12") + testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.9.0") // required by IntelliJ test framework testImplementation("junit:junit:4.13.2") diff --git a/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt b/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt index 197dc14cf..31d64d9cc 100644 --- a/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt +++ b/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt @@ -1,6 +1,5 @@ package com.coder.gateway.settings -import com.coder.gateway.cli.gpg.VerificationResult import com.coder.gateway.util.Arch import com.coder.gateway.util.OS import com.coder.gateway.util.expand @@ -9,7 +8,6 @@ import com.coder.gateway.util.getOS import com.coder.gateway.util.safeHost import com.coder.gateway.util.toURL import com.coder.gateway.util.withPath -import com.github.weisj.jsvg.B import com.intellij.openapi.diagnostic.Logger import java.net.URL import java.nio.file.Files @@ -168,6 +166,11 @@ open class CoderSettings( val fallbackOnCoderForSignatures: Boolean get() = state.fallbackOnCoderForSignatures + /** + * Default CLI binary name based on OS and architecture + */ + val defaultCliBinaryNameByOsAndArch: String get() = getCoderCLIForOS(getOS(), getArch()) + /** * Default CLI signature name based on OS and architecture */ @@ -281,9 +284,8 @@ open class CoderSettings( */ fun binSource(url: URL): URL { state.binarySource.let { - val binaryName = getCoderCLIForOS(getOS(), getArch()) return if (it.isBlank()) { - url.withPath("/bin/$binaryName") + url.withPath("/bin/$defaultCliBinaryNameByOsAndArch") } else { logger.info("Using binary source override $it") try { @@ -393,44 +395,19 @@ open class CoderSettings( } /** - * Return the name of the binary (with extension) for the provided OS and architecture. + * Returns the name of the binary (with extension) for the provided OS and architecture. */ - private fun getCoderCLIForOS( - os: OS?, - arch: Arch?, - ): String { + private fun getCoderCLIForOS(os: OS?, arch: Arch?): String { logger.debug("Resolving binary for $os $arch") - return buildCoderFileName(os, arch) - } - /** - * Return the name of the signature file (.asc) for the provided OS and architecture. - */ - private fun getCoderSignatureForOS( - os: OS?, - arch: Arch?, - ): String { - logger.debug("Resolving signature for $os $arch") - return buildCoderFileName(os, arch, true) - } - - /** - * Build the coder file name based on OS, architecture, and whether it's a signature file. - */ - private fun buildCoderFileName( - os: OS?, - arch: Arch?, - isSignature: Boolean = false - ): String { - if (os == null) { - logger.error("Could not resolve client OS and architecture, defaulting to WINDOWS AMD64") - return if (isSignature) "coder-windows-amd64.asc" else "coder-windows-amd64.exe" - } - - val osName = when (os) { - OS.WINDOWS -> "windows" - OS.LINUX -> "linux" - OS.MAC -> "darwin" + val (osName, extension) = when (os) { + OS.WINDOWS -> "windows" to ".exe" + OS.LINUX -> "linux" to "" + OS.MAC -> "darwin" to "" + null -> { + logger.error("Could not resolve client OS and architecture, defaulting to WINDOWS AMD64") + return "coder-windows-amd64.exe" + } } val archName = when (arch) { @@ -440,14 +417,17 @@ open class CoderSettings( else -> "amd64" // default fallback } - val extension = if (isSignature) ".asc" else when (os) { - OS.WINDOWS -> ".exe" - OS.LINUX, OS.MAC -> "" - } - return "coder-$osName-$archName$extension" } + /** + * Returns the name of the signature file (.asc) for the provided OS and architecture. + */ + private fun getCoderSignatureForOS(os: OS?, arch: Arch?): String { + logger.debug("Resolving signature for $os $arch") + return "${getCoderCLIForOS(os, arch)}.asc" + } + companion object { val logger = Logger.getInstance(CoderSettings::class.java.simpleName) } diff --git a/src/main/kotlin/com/coder/gateway/util/OS.kt b/src/main/kotlin/com/coder/gateway/util/OS.kt index eecd13fbe..8a3a364aa 100644 --- a/src/main/kotlin/com/coder/gateway/util/OS.kt +++ b/src/main/kotlin/com/coder/gateway/util/OS.kt @@ -4,16 +4,16 @@ import java.util.Locale fun getOS(): OS? = OS.from(System.getProperty("os.name")) -fun getArch(): Arch? = Arch.from(System.getProperty("os.arch").lowercase(Locale.getDefault())) +fun getArch(): Arch? = Arch.from(System.getProperty("os.arch")?.lowercase(Locale.getDefault())) enum class OS { WINDOWS, LINUX, - MAC, - ; + MAC; companion object { - fun from(os: String): OS? = when { + fun from(os: String?): OS? = when { + os.isNullOrBlank() -> null os.contains("win", true) -> { WINDOWS } @@ -38,7 +38,8 @@ enum class Arch { ; companion object { - fun from(arch: String): Arch? = when { + fun from(arch: String?): Arch? = when { + arch.isNullOrBlank() -> null arch.contains("amd64", true) || arch.contains("x86_64", true) -> AMD64 arch.contains("arm64", true) || arch.contains("aarch64", true) -> ARM64 arch.contains("armv7", true) -> ARMV7 diff --git a/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt b/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt index c3f69bd41..e98c1e78b 100644 --- a/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt +++ b/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt @@ -3,14 +3,36 @@ package com.coder.gateway.settings import com.coder.gateway.util.OS import com.coder.gateway.util.getOS import com.coder.gateway.util.withPath +import org.junit.jupiter.api.Assertions import java.net.URL import java.nio.file.Path +import kotlin.test.AfterTest +import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertContains import kotlin.test.assertEquals import kotlin.test.assertNotEquals internal class CoderSettingsTest { + private var originalOsName: String? = null + private var originalOsArch: String? = null + + private lateinit var store: CoderSettings + + @BeforeTest + fun setUp() { + originalOsName = System.getProperty("os.name") + originalOsArch = System.getProperty("os.arch") + store = CoderSettings(CoderSettingsState()) + System.setProperty("intellij.testFramework.rethrow.logged.errors", "false") + } + + @AfterTest + fun tearDown() { + System.setProperty("os.name", originalOsName) + System.setProperty("os.arch", originalOsArch) + } + @Test fun testExpands() { val state = CoderSettingsState() @@ -35,13 +57,13 @@ internal class CoderSettingsTest { CoderSettings( state, env = - Environment( - mapOf( - "LOCALAPPDATA" to "/tmp/coder-gateway-test/localappdata", - "HOME" to "/tmp/coder-gateway-test/home", - "XDG_DATA_HOME" to "/tmp/coder-gateway-test/xdg-data", + Environment( + mapOf( + "LOCALAPPDATA" to "/tmp/coder-gateway-test/localappdata", + "HOME" to "/tmp/coder-gateway-test/home", + "XDG_DATA_HOME" to "/tmp/coder-gateway-test/xdg-data", + ), ), - ), ) var expected = when (getOS()) { @@ -59,12 +81,12 @@ internal class CoderSettingsTest { CoderSettings( state, env = - Environment( - mapOf( - "XDG_DATA_HOME" to "", - "HOME" to "/tmp/coder-gateway-test/home", + Environment( + mapOf( + "XDG_DATA_HOME" to "", + "HOME" to "/tmp/coder-gateway-test/home", + ), ), - ), ) expected = "/tmp/coder-gateway-test/home/.local/share/coder-gateway/localhost" @@ -78,13 +100,13 @@ internal class CoderSettingsTest { CoderSettings( state, env = - Environment( - mapOf( - "LOCALAPPDATA" to "/ignore", - "HOME" to "/ignore", - "XDG_DATA_HOME" to "/ignore", + Environment( + mapOf( + "LOCALAPPDATA" to "/ignore", + "HOME" to "/ignore", + "XDG_DATA_HOME" to "/ignore", + ), ), - ), ) expected = "/tmp/coder-gateway-test/data-dir/localhost" assertEquals(Path.of(expected).toAbsolutePath(), settings.dataDir(url)) @@ -131,13 +153,13 @@ internal class CoderSettingsTest { CoderSettings( state, env = - Environment( - mapOf( - "APPDATA" to "/tmp/coder-gateway-test/cli-appdata", - "HOME" to "/tmp/coder-gateway-test/cli-home", - "XDG_CONFIG_HOME" to "/tmp/coder-gateway-test/cli-xdg-config", + Environment( + mapOf( + "APPDATA" to "/tmp/coder-gateway-test/cli-appdata", + "HOME" to "/tmp/coder-gateway-test/cli-home", + "XDG_CONFIG_HOME" to "/tmp/coder-gateway-test/cli-xdg-config", + ), ), - ), ) var expected = when (getOS()) { @@ -153,12 +175,12 @@ internal class CoderSettingsTest { CoderSettings( state, env = - Environment( - mapOf( - "XDG_CONFIG_HOME" to "", - "HOME" to "/tmp/coder-gateway-test/cli-home", + Environment( + mapOf( + "XDG_CONFIG_HOME" to "", + "HOME" to "/tmp/coder-gateway-test/cli-home", + ), ), - ), ) expected = "/tmp/coder-gateway-test/cli-home/.config/coderv2" assertEquals(Path.of(expected), settings.coderConfigDir) @@ -169,14 +191,14 @@ internal class CoderSettingsTest { CoderSettings( state, env = - Environment( - mapOf( - "CODER_CONFIG_DIR" to "/tmp/coder-gateway-test/coder-config-dir", - "APPDATA" to "/ignore", - "HOME" to "/ignore", - "XDG_CONFIG_HOME" to "/ignore", + Environment( + mapOf( + "CODER_CONFIG_DIR" to "/tmp/coder-gateway-test/coder-config-dir", + "APPDATA" to "/ignore", + "HOME" to "/ignore", + "XDG_CONFIG_HOME" to "/ignore", + ), ), - ), ) expected = "/tmp/coder-gateway-test/coder-config-dir" assertEquals(Path.of(expected), settings.coderConfigDir) @@ -402,4 +424,54 @@ internal class CoderSettingsTest { assertEquals(true, settings.ignoreSetupFailure) assertEquals("test ssh log directory", settings.sshLogDirectory) } + + + @Test + fun `Default CLI and signature for Windows AMD64`() = + assertBinaryAndSignature("Windows 10", "amd64", "coder-windows-amd64.exe", "coder-windows-amd64.exe.asc") + + @Test + fun `Default CLI and signature for Windows ARM64`() = + assertBinaryAndSignature("Windows 10", "aarch64", "coder-windows-arm64.exe", "coder-windows-arm64.exe.asc") + + @Test + fun `Default CLI and signature for Linux AMD64`() = + assertBinaryAndSignature("Linux", "x86_64", "coder-linux-amd64", "coder-linux-amd64.asc") + + @Test + fun `Default CLI and signature for Linux ARM64`() = + assertBinaryAndSignature("Linux", "aarch64", "coder-linux-arm64", "coder-linux-arm64.asc") + + @Test + fun `Default CLI and signature for Linux ARMV7`() = + assertBinaryAndSignature("Linux", "armv7l", "coder-linux-armv7", "coder-linux-armv7.asc") + + @Test + fun `Default CLI and signature for Mac AMD64`() = + assertBinaryAndSignature("Mac OS X", "x86_64", "coder-darwin-amd64", "coder-darwin-amd64.asc") + + @Test + fun `Default CLI and signature for Mac ARM64`() = + assertBinaryAndSignature("Mac OS X", "aarch64", "coder-darwin-arm64", "coder-darwin-arm64.asc") + + @Test + fun `Default CLI and signature for unknown OS and Arch`() = + assertBinaryAndSignature(null, null, "coder-windows-amd64.exe", "coder-windows-amd64.exe.asc") + + @Test + fun `Default CLI and signature for unknown Arch fallback on Linux`() = + assertBinaryAndSignature("Linux", "mips64", "coder-linux-amd64", "coder-linux-amd64.asc") + + private fun assertBinaryAndSignature( + osName: String?, + arch: String?, + expectedBinary: String, + expectedSignature: String + ) { + if (osName == null) System.clearProperty("os.name") else System.setProperty("os.name", osName) + if (arch == null) System.clearProperty("os.arch") else System.setProperty("os.arch", arch) + + Assertions.assertEquals(expectedBinary, store.defaultCliBinaryNameByOsAndArch) + Assertions.assertEquals(expectedSignature, store.defaultSignatureNameByOsAndArch) + } } From 5945b6ce73301ab2830521fc386673eac2d7e027 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Wed, 23 Jul 2025 01:45:28 +0300 Subject: [PATCH 06/10] chore: next version is 2.22.0 --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 4c6ce49bd..b3085324c 100644 --- a/gradle.properties +++ b/gradle.properties @@ -5,7 +5,7 @@ pluginGroup=com.coder.gateway artifactName=coder-gateway pluginName=Coder # SemVer format -> https://semver.org -pluginVersion=2.21.1 +pluginVersion=2.22.0 # See https://plugins.jetbrains.com/docs/intellij/build-number-ranges.html # for insight into build numbers and IntelliJ Platform versions. pluginSinceBuild=243.26574 From 06f0feb390f4e4088bb85fb3842d04af059d116f Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 25 Jul 2025 18:01:54 +0300 Subject: [PATCH 07/10] impl: strict URL validation for the connection screen This commit rejects any URL that is opaque, not hierarchical, not using http or https protocol, or it misses the hostname. --- CHANGELOG.md | 1 + .../com/coder/gateway/util/URLExtensions.kt | 21 ++++++- .../views/steps/CoderWorkspacesStepView.kt | 55 ++++++++++++++++--- .../coder/gateway/util/URLExtensionsTest.kt | 54 ++++++++++++++++++ 4 files changed, 121 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 064765325..bb987fcf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - support for checking if CLI is signed - improved progress reporting while downloading the CLI +- URL validation is stricter in the connection screen ## 2.21.1 - 2025-06-26 diff --git a/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt b/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt index 1fdeeca4c..39efbcca3 100644 --- a/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt +++ b/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt @@ -1,10 +1,12 @@ package com.coder.gateway.util +import com.coder.gateway.util.WebUrlValidationResult.Invalid import java.net.IDN import java.net.URI import java.net.URL -fun String.toURL(): URL = URL(this) + +fun String.toURL(): URL = URI.create(this).toURL() fun URL.withPath(path: String): URL = URL( this.protocol, @@ -13,6 +15,23 @@ fun URL.withPath(path: String): URL = URL( if (path.startsWith("/")) path else "/$path", ) +fun URI.validateStrictWebUrl(): WebUrlValidationResult = try { + when { + isOpaque -> Invalid("$this is opaque, instead of hierarchical") + !isAbsolute -> Invalid("$this is relative, it must be absolute") + scheme?.lowercase() !in setOf("http", "https") -> Invalid("Scheme for $this must be either http or https") + authority.isNullOrBlank() -> Invalid("$this does not have a hostname") + else -> WebUrlValidationResult.Valid + } +} catch (e: Exception) { + Invalid(e.message ?: "$this could not be parsed as a URI reference") +} + +sealed class WebUrlValidationResult { + object Valid : WebUrlValidationResult() + data class Invalid(val reason: String) : WebUrlValidationResult() +} + /** * Return the host, converting IDN to ASCII in case the file system cannot * support the necessary character set. diff --git a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt index f29d9a591..919bcef09 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -20,9 +20,11 @@ import com.coder.gateway.util.DialogUi import com.coder.gateway.util.InvalidVersionException import com.coder.gateway.util.OS import com.coder.gateway.util.SemVer +import com.coder.gateway.util.WebUrlValidationResult import com.coder.gateway.util.humanizeConnectionError import com.coder.gateway.util.isCancellation import com.coder.gateway.util.toURL +import com.coder.gateway.util.validateStrictWebUrl import com.coder.gateway.util.withoutNull import com.intellij.icons.AllIcons import com.intellij.ide.ActivityTracker @@ -78,6 +80,8 @@ import javax.swing.JLabel import javax.swing.JTable import javax.swing.JTextField import javax.swing.ListSelectionModel +import javax.swing.event.DocumentEvent +import javax.swing.event.DocumentListener import javax.swing.table.DefaultTableCellRenderer import javax.swing.table.TableCellRenderer @@ -133,7 +137,6 @@ class CoderWorkspacesStepView : private var tfUrl: JTextField? = null private var tfUrlComment: JLabel? = null private var cbExistingToken: JCheckBox? = null - private var cbFallbackOnSignature: JCheckBox? = null private val notificationBanner = NotificationBanner() private var tableOfWorkspaces = @@ -219,6 +222,31 @@ class CoderWorkspacesStepView : // Reconnect when the enter key is pressed. maybeAskTokenThenConnect() } + // Add document listener to clear error when user types + document.addDocumentListener(object : DocumentListener { + override fun insertUpdate(e: DocumentEvent?) = clearErrorState() + override fun removeUpdate(e: DocumentEvent?) = clearErrorState() + override fun changedUpdate(e: DocumentEvent?) = clearErrorState() + + private fun clearErrorState() { + tfUrlComment?.apply { + foreground = UIUtil.getContextHelpForeground() + if (tfUrl?.text.equals(client?.url?.toString())) { + text = + CoderGatewayBundle.message( + "gateway.connector.view.coder.workspaces.connect.text.connected", + client!!.url.host, + ) + } else { + text = CoderGatewayBundle.message( + "gateway.connector.view.coder.workspaces.connect.text.comment", + CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.connect.text"), + ) + } + icon = null + } + } + }) }.component button(CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.connect.text")) { // Reconnect when the connect button is pressed. @@ -268,15 +296,15 @@ class CoderWorkspacesStepView : } row { cell() // For alignment. - checkBox(CoderGatewayBundle.message("gateway.connector.settings.fallback-on-coder-for-signatures.title")) - .bindSelected(state::fallbackOnCoderForSignatures).applyToComponent { - addActionListener { event -> - state.fallbackOnCoderForSignatures = (event.source as JBCheckBox).isSelected - } + checkBox(CoderGatewayBundle.message("gateway.connector.settings.fallback-on-coder-for-signatures.title")) + .bindSelected(state::fallbackOnCoderForSignatures).applyToComponent { + addActionListener { event -> + state.fallbackOnCoderForSignatures = (event.source as JBCheckBox).isSelected } - .comment( - CoderGatewayBundle.message("gateway.connector.settings.fallback-on-coder-for-signatures.comment"), - ) + } + .comment( + CoderGatewayBundle.message("gateway.connector.settings.fallback-on-coder-for-signatures.comment"), + ) }.layout(RowLayout.PARENT_GRID) row { @@ -539,6 +567,15 @@ class CoderWorkspacesStepView : component.apply() // Force bindings to be filled. val newURL = fields.coderURL.toURL() if (settings.requireTokenAuth) { + val result = newURL.toURI().validateStrictWebUrl() + if (result is WebUrlValidationResult.Invalid) { + tfUrlComment.apply { + this?.foreground = UIUtil.getErrorForeground() + this?.text = result.reason + this?.icon = UIUtil.getBalloonErrorIcon() + } + return + } val pastedToken = dialogUi.askToken( newURL, diff --git a/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt b/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt index 2feea3404..fdf18114d 100644 --- a/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt +++ b/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt @@ -60,4 +60,58 @@ internal class URLExtensionsTest { ) } } + + @Test + fun `valid http URL should return Valid`() { + val uri = URI("http://coder.com") + val result = uri.validateStrictWebUrl() + assertEquals(WebUrlValidationResult.Valid, result) + } + + @Test + fun `valid https URL with path and query should return Valid`() { + val uri = URI("https://coder.com/bin/coder-linux-amd64?query=1") + val result = uri.validateStrictWebUrl() + assertEquals(WebUrlValidationResult.Valid, result) + } + + @Test + fun `relative URL should return Invalid with appropriate message`() { + val uri = URI("/bin/coder-linux-amd64") + val result = uri.validateStrictWebUrl() + assertEquals( + WebUrlValidationResult.Invalid("$uri is relative, it must be absolute"), + result + ) + } + + @Test + fun `opaque URI like mailto should return Invalid`() { + val uri = URI("mailto:user@coder.com") + val result = uri.validateStrictWebUrl() + assertEquals( + WebUrlValidationResult.Invalid("$uri is opaque, instead of hierarchical"), + result + ) + } + + @Test + fun `unsupported scheme like ftp should return Invalid`() { + val uri = URI("ftp://coder.com") + val result = uri.validateStrictWebUrl() + assertEquals( + WebUrlValidationResult.Invalid("Scheme for $uri must be either http or https"), + result + ) + } + + @Test + fun `http URL with missing authority should return Invalid`() { + val uri = URI("http:///bin/coder-linux-amd64") + val result = uri.validateStrictWebUrl() + assertEquals( + WebUrlValidationResult.Invalid("$uri does not have a hostname"), + result + ) + } } From f8414f99f93a16bd72b003363a5f4dcdc2d73885 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 25 Jul 2025 18:17:21 +0300 Subject: [PATCH 08/10] impl: strict URL validation for the URI handling This commit rejects any URL that is opaque, not hierarchical, not using http or https protocol, or it misses the hostname. --- .../com/coder/gateway/util/LinkHandler.kt | 4 ++ .../com/coder/gateway/util/URLExtensions.kt | 16 +++++--- .../views/steps/CoderWorkspacesStepView.kt | 6 +-- .../coder/gateway/util/URLExtensionsTest.kt | 41 +++++++++++-------- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/util/LinkHandler.kt b/src/main/kotlin/com/coder/gateway/util/LinkHandler.kt index 8ee600e58..6ac93efa3 100644 --- a/src/main/kotlin/com/coder/gateway/util/LinkHandler.kt +++ b/src/main/kotlin/com/coder/gateway/util/LinkHandler.kt @@ -37,6 +37,10 @@ open class LinkHandler( if (deploymentURL.isNullOrBlank()) { throw MissingArgumentException("Query parameter \"$URL\" is missing") } + val result = deploymentURL.validateStrictWebUrl() + if (result is WebUrlValidationResult.Invalid) { + throw IllegalArgumentException(result.reason) + } val queryTokenRaw = parameters.token() val queryToken = if (!queryTokenRaw.isNullOrBlank()) { diff --git a/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt b/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt index 39efbcca3..cd4114668 100644 --- a/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt +++ b/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt @@ -15,12 +15,18 @@ fun URL.withPath(path: String): URL = URL( if (path.startsWith("/")) path else "/$path", ) -fun URI.validateStrictWebUrl(): WebUrlValidationResult = try { + +fun String.validateStrictWebUrl(): WebUrlValidationResult = try { + val uri = URI(this) + when { - isOpaque -> Invalid("$this is opaque, instead of hierarchical") - !isAbsolute -> Invalid("$this is relative, it must be absolute") - scheme?.lowercase() !in setOf("http", "https") -> Invalid("Scheme for $this must be either http or https") - authority.isNullOrBlank() -> Invalid("$this does not have a hostname") + uri.isOpaque -> Invalid("$this is opaque, instead of hierarchical") + !uri.isAbsolute -> Invalid("$this is relative, it must be absolute") + uri.scheme?.lowercase() !in setOf("http", "https") -> + Invalid("Scheme for $this must be either http or https") + + uri.authority.isNullOrBlank() -> + Invalid("$this does not have a hostname") else -> WebUrlValidationResult.Valid } } catch (e: Exception) { diff --git a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt index 919bcef09..65417984d 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -565,9 +565,9 @@ class CoderWorkspacesStepView : private fun maybeAskTokenThenConnect(error: String? = null) { val oldURL = fields.coderURL component.apply() // Force bindings to be filled. - val newURL = fields.coderURL.toURL() if (settings.requireTokenAuth) { - val result = newURL.toURI().validateStrictWebUrl() + val result = fields.coderURL.validateStrictWebUrl() + val newURL = fields.coderURL.toURL() if (result is WebUrlValidationResult.Invalid) { tfUrlComment.apply { this?.foreground = UIUtil.getErrorForeground() @@ -590,7 +590,7 @@ class CoderWorkspacesStepView : maybeAskTokenThenConnect(it) } } else { - connect(newURL, null) + connect(fields.coderURL.toURL(), null) } } diff --git a/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt b/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt index fdf18114d..6a0c5fdaa 100644 --- a/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt +++ b/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt @@ -60,57 +60,64 @@ internal class URLExtensionsTest { ) } } - @Test fun `valid http URL should return Valid`() { - val uri = URI("http://coder.com") - val result = uri.validateStrictWebUrl() + val result = "http://coder.com".validateStrictWebUrl() assertEquals(WebUrlValidationResult.Valid, result) } @Test fun `valid https URL with path and query should return Valid`() { - val uri = URI("https://coder.com/bin/coder-linux-amd64?query=1") - val result = uri.validateStrictWebUrl() + val result = "https://coder.com/bin/coder-linux-amd64?query=1".validateStrictWebUrl() assertEquals(WebUrlValidationResult.Valid, result) } @Test fun `relative URL should return Invalid with appropriate message`() { - val uri = URI("/bin/coder-linux-amd64") - val result = uri.validateStrictWebUrl() + val url = "/bin/coder-linux-amd64" + val result = url.validateStrictWebUrl() assertEquals( - WebUrlValidationResult.Invalid("$uri is relative, it must be absolute"), + WebUrlValidationResult.Invalid("$url is relative, it must be absolute"), result ) } @Test fun `opaque URI like mailto should return Invalid`() { - val uri = URI("mailto:user@coder.com") - val result = uri.validateStrictWebUrl() + val url = "mailto:user@coder.com" + val result = url.validateStrictWebUrl() assertEquals( - WebUrlValidationResult.Invalid("$uri is opaque, instead of hierarchical"), + WebUrlValidationResult.Invalid("$url is opaque, instead of hierarchical"), result ) } @Test fun `unsupported scheme like ftp should return Invalid`() { - val uri = URI("ftp://coder.com") - val result = uri.validateStrictWebUrl() + val url = "ftp://coder.com" + val result = url.validateStrictWebUrl() assertEquals( - WebUrlValidationResult.Invalid("Scheme for $uri must be either http or https"), + WebUrlValidationResult.Invalid("Scheme for $url must be either http or https"), result ) } @Test fun `http URL with missing authority should return Invalid`() { - val uri = URI("http:///bin/coder-linux-amd64") - val result = uri.validateStrictWebUrl() + val url = "http:///bin/coder-linux-amd64" + val result = url.validateStrictWebUrl() + assertEquals( + WebUrlValidationResult.Invalid("$url does not have a hostname"), + result + ) + } + + @Test + fun `malformed URI should return Invalid with parsing error message`() { + val url = "http://[invalid-uri]" + val result = url.validateStrictWebUrl() assertEquals( - WebUrlValidationResult.Invalid("$uri does not have a hostname"), + WebUrlValidationResult.Invalid("$url could not be parsed as a URI reference"), result ) } From 8623572270fe1f9dc7a6c9573cf7bda3b066aac2 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 25 Jul 2025 18:28:36 +0300 Subject: [PATCH 09/10] fix: transform to url only after we checked the validation result --- CHANGELOG.md | 2 +- src/main/kotlin/com/coder/gateway/util/URLExtensions.kt | 1 - .../com/coder/gateway/views/steps/CoderWorkspacesStepView.kt | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb987fcf9..b2dbab4b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ - support for checking if CLI is signed - improved progress reporting while downloading the CLI -- URL validation is stricter in the connection screen +- URL validation is stricter in the connection screen and URI protocol handler ## 2.21.1 - 2025-06-26 diff --git a/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt b/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt index cd4114668..1fec66178 100644 --- a/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt +++ b/src/main/kotlin/com/coder/gateway/util/URLExtensions.kt @@ -15,7 +15,6 @@ fun URL.withPath(path: String): URL = URL( if (path.startsWith("/")) path else "/$path", ) - fun String.validateStrictWebUrl(): WebUrlValidationResult = try { val uri = URI(this) diff --git a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt index 65417984d..51a7df4b5 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -567,7 +567,6 @@ class CoderWorkspacesStepView : component.apply() // Force bindings to be filled. if (settings.requireTokenAuth) { val result = fields.coderURL.validateStrictWebUrl() - val newURL = fields.coderURL.toURL() if (result is WebUrlValidationResult.Invalid) { tfUrlComment.apply { this?.foreground = UIUtil.getErrorForeground() @@ -576,6 +575,7 @@ class CoderWorkspacesStepView : } return } + val newURL = fields.coderURL.toURL() val pastedToken = dialogUi.askToken( newURL, From 8838a8e70bae9dc7f27cec349362e0dc0b1aa081 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 25 Jul 2025 18:32:13 +0300 Subject: [PATCH 10/10] chore: update UT expected result --- src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt b/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt index 6a0c5fdaa..4c286da04 100644 --- a/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt +++ b/src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt @@ -117,7 +117,7 @@ internal class URLExtensionsTest { val url = "http://[invalid-uri]" val result = url.validateStrictWebUrl() assertEquals( - WebUrlValidationResult.Invalid("$url could not be parsed as a URI reference"), + WebUrlValidationResult.Invalid("Malformed IPv6 address at index 8: $url"), result ) }