diff --git a/CHANGELOG.md b/CHANGELOG.md index 025bf45..2c35ec1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,9 @@ - include a Coder support bundle for the selected workspace and agent only when using the environment's _Collect logs_ action in Toolbox -- include active and recent local CLI SSH logs in workspace log collection, using the configured or default CLI log - directory +- include active and recent local CLI SSH logs in workspace log collection. For CLI 2.4.0 and newer the plugin now + passes an explicit `--log-dir` to the SSH ProxyCommand (the configured directory, `CODER_SSH_LOG_DIR`, or + `ssh-logs/` in the deployment's plugin data directory) and collects and cleans up logs from that same directory ### Changed diff --git a/README.md b/README.md index 4ee29f7..26fecc0 100644 --- a/README.md +++ b/README.md @@ -409,33 +409,40 @@ The plugin runs `coder support bundle` using the deployment's existing CLI login and its agent, when available. It places `coder-support.zip` inside the environment's diagnostic directory in the Toolbox log archive. Collection does not start a stopped workspace. -The same diagnostic directory also includes recent local CLI SSH logs under `coder-ssh/`, even if support bundle -generation fails. The plugin uses the configured SSH log directory, then `CODER_SSH_LOG_DIR`, or otherwise the CLI's -default state directory: `$XDG_STATE_HOME/coder/logs` when set to an absolute path, falling back to -`~/.local/state/coder/logs` on Linux, `~/Library/Application Support/coder/logs` on macOS, or -`%LOCALAPPDATA%/coder/logs` on Windows. - -Only regular files directly inside the SSH log directory with these names are eligible: - -- `coder-ssh*.log`: CLI SSH session logs. -- `coder-ssh*.log.`: rotated CLI SSH logs, where `` contains only digits, such as `.log.1`. -- `.log`: legacy SSH logs with a numeric filename, such as `12345.log`. - -Collection skips subdirectories, symbolic links, and other filenames, including rotated legacy files such as -`12345.log.1`. These local logs can include sessions for other workspaces that share the log directory. +Recent local CLI SSH logs are added in place to `coder-support.zip` under `toolbox-logs/ssh/`. If the bundle could +not be generated or is not a valid archive, the logs are written to a separate `coder-ssh.zip` in the diagnostic +directory instead, using the same layout. + +For CLI versions **2.4.0 and newer**, the plugin passes `--log-dir -v` to the SSH ProxyCommand, enabling +verbose file logging even on CLIs that do not log by default. The directory is the configured SSH log directory, +then `CODER_SSH_LOG_DIR`, then `ssh-logs/` inside the deployment's plugin data directory. The plugin does not infer +the CLI's default state directory, so logs written there by earlier setups are not collected. CLI versions older than +2.4.0, or whose version is not recognized, receive neither logging flag and skip cleanup. Their SSH logs go wherever +the CLI writes them by default, so collection only finds logs if you point the configured SSH log directory or +`CODER_SSH_LOG_DIR` at that location. + +Only regular files directly inside the SSH log directory named `coder-ssh*-.log` are eligible, where `` +contains only digits. Collection skips subdirectories, symbolic links, and other filenames, including bare PID names +such as `12345.log` and rotated `.log.` files. These local logs can include sessions for other workspaces +that share the log directory. The following fixed limits apply and are not configurable: - **Age:** include files modified within the last **3 days (72 hours)**, based on the file's last-modified time. The identified active SSH session's log for the selected workspace/agent is included regardless of age. - Rotated logs remain subject to the age cutoff. If the active log cannot be identified, the cutoff applies to - every file. + If the active log cannot be identified, the cutoff applies to every file. - **Size:** copy only the last **50 MiB (52,428,800 bytes) per file**, including the active log. This is a limit on each file's uncompressed contents, not on the total archive size. -These limits apply only to the collected copies; the original log files are unchanged. Missing or unreadable SSH +These limits apply only to the archived copies; the original log files are unchanged. Missing or unreadable SSH logs do not prevent collecting the remaining diagnostics. +On each connection attempt with a supported CLI, background cleanup keeps the newest **20** `coder-ssh*-.log` +files by modification time and deletes other matching regular files only if they are older than **7 days**, so more +than 20 files can remain when the extra ones are newer. Cleanup does not recurse or delete symlinks. Collection itself +never deletes source logs. +CLI 2.38+ also applies its own 50-file pruning in this directory. + The bundle can contain deployment health and configuration, network diagnostics, workspace build logs, template source, and agent diagnostics. Available information depends on your permissions and workspace connectivity. Review the bundle before sharing, following diff --git a/build.gradle.kts b/build.gradle.kts index 80a2fdd..f7825c0 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -38,6 +38,7 @@ dependencies { compileOnly(libs.bundles.serialization) compileOnly(libs.coroutines.core) implementation(libs.okhttp) + implementation(libs.zip4j) implementation(libs.moshi) ksp(libs.moshi.codegen) implementation(libs.retrofit) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c223079..6a28804 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -17,6 +17,7 @@ mockk = "1.14.11" detekt = "1.23.8" bouncycastle = "1.86" jackson = "2.22.3" +zip4j = "2.11.5" [libraries] toolbox-core-api = { module = "com.jetbrains.toolbox:core-api", version.ref = "toolbox-plugin-api" } @@ -37,6 +38,7 @@ mokk = { module = "io.mockk:mockk", version.ref = "mockk" } marketplace-client = { module = "org.jetbrains.intellij:plugin-repository-rest-client", version.ref = "marketplace-client" } bouncycastle-bcpg = { module = "org.bouncycastle:bcpg-jdk18on", version.ref = "bouncycastle" } bouncycastle-bcprov = { module = "org.bouncycastle:bcprov-jdk18on", version.ref = "bouncycastle" } +zip4j = { module = "net.lingala.zip4j:zip4j", version.ref = "zip4j" } jackson-module-kotlin = { module = "com.fasterxml.jackson.module:jackson-module-kotlin", version.ref = "jackson" } [bundles] diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt index 6524a2c..f79a990 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt @@ -115,6 +115,7 @@ class CoderRemoteEnvironment( private val networkMetricsMarshaller = Moshi.Builder().build().adapter(NetworkMetrics::class.java) private val proxyCommandHandle = SshCommandProcessHandle(context) private var pollJob: Job? = null + @Volatile private var sshPid: Long? = null @@ -326,6 +327,7 @@ class CoderRemoteEnvironment( private fun pollNetworkMetrics(): Job = context.cs.launch(CoroutineName("Network Metrics Poller")) { + cli.cleanupSshLogs(currentSessionId()) context.logger.info(currentSessionId(), "Starting the network metrics poll job for $id") while (isActive) { val currentAgent = agent ?: break diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 420208f..c2cc71c 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -12,6 +12,7 @@ import com.coder.toolbox.cli.gpg.GPGVerifier import com.coder.toolbox.cli.gpg.VerificationResult import com.coder.toolbox.cli.gpg.VerificationResult.Failed import com.coder.toolbox.cli.gpg.VerificationResult.Invalid +import com.coder.toolbox.diagnostics.CoderSshLogCollector import com.coder.toolbox.sdk.CoderHttpClientBuilder import com.coder.toolbox.session.SessionId import com.coder.toolbox.settings.SignatureFallbackStrategy.ALLOW @@ -26,6 +27,7 @@ import com.squareup.moshi.Json import com.squareup.moshi.JsonClass import com.squareup.moshi.JsonDataException import com.squareup.moshi.Moshi +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runInterruptible import kotlinx.coroutines.withContext @@ -101,6 +103,7 @@ suspend fun ensureCLI( * The supported features of the CLI. */ data class Features( + val proxyLogDirectory: Boolean = false, val disableAutostart: Boolean = false, val reportWorkspaceUsage: Boolean = false, val wildcardSsh: Boolean = false, @@ -132,9 +135,22 @@ class CoderCLIManager( /** Returns the effective SSH log directory for this CLI's current settings. */ internal fun getSshLogDirectory(): Path = getSshLogDirectory( context.settingsStore.sshLogDirectory, - os = currentOs, + defaultDirectory = context.settingsStore.dataDir(deploymentURL).resolve("ssh-logs"), ) + @Suppress("TooGenericExceptionCaught") + internal suspend fun cleanupSshLogs(sessionId: SessionId?) = withContext(Dispatchers.IO) { + try { + if (features.proxyLogDirectory) { + CoderSshLogCollector(context.logger, getSshLogDirectory()).cleanup(sessionId) + } + } catch (ex: CancellationException) { + throw ex + } catch (ex: Exception) { + context.logger.warn(sessionId, ex, "Could not prepare SSH log cleanup") + } + } + private fun createDownloadService(): CoderDownloadService { val okHttpClient = CoderHttpClientBuilder.default(context) @@ -341,6 +357,9 @@ class CoderCLIManager( sshConfigPath: String = context.settingsStore.sshConfigPath, ) { context.logger.info(sessionIds, "Configuring SSH config at $sshConfigPath") + if (workspaceAddresses.isNotEmpty() && feats.proxyLogDirectory) { + Files.createDirectories(getSshLogDirectory()) + } writeSSHConfig( modifySSHConfig(workspaceAddresses, sessionIds, readSSHConfig(sshConfigPath), feats), sshConfigPath, @@ -389,9 +408,9 @@ class CoderCLIManager( ) val proxyArgs = buildList { addAll(baseArgs) - context.settingsStore.sshLogDirectory?.takeIf { it.isNotBlank() }?.let { + if (feats.proxyLogDirectory) { add("--log-dir") - add(it) + add(getSshLogDirectory().toString()) add("-v") } if (feats.reportWorkspaceUsage) add("--usage-app=jetbrains") @@ -530,10 +549,6 @@ class CoderCLIManager( sshConfPath.parent?.toFile()?.mkdirs() sshConfPath.toFile().writeText(contents) } - // The Coder cli will *not* create the log directory. - if (!context.settingsStore.sshLogDirectory.isNullOrBlank()) { - Path.of(context.settingsStore.sshLogDirectory).toFile().mkdirs() - } } } @@ -661,6 +676,7 @@ class CoderCLIManager( Features() } else { Features( + proxyLogDirectory = version >= SemVer(2, 4, 0), disableAutostart = version >= SemVer(2, 5, 0), reportWorkspaceUsage = version >= SemVer(2, 13, 0), wildcardSsh = version >= SemVer(2, 19, 0), @@ -686,44 +702,14 @@ class CoderCLIManager( internal fun supportsKeyringStorage(os: OS?): Boolean = os == OS.MAC || os == OS.WINDOWS - /** Resolves the log directory used by `coder ssh`, including its XDG state directory default. */ + /** Resolve the same explicit directory for ProxyCommand logging, cleanup, and support bundles. */ internal fun getSshLogDirectory( configuredDirectory: String?, + defaultDirectory: Path, env: Map = System.getenv(), - os: OS? = getOS(), - home: Path = Path.of( - env[if (os == OS.WINDOWS) "USERPROFILE" else "HOME"].orEmpty().ifEmpty { - System.getProperty("user.home") - }, - ), - ): Path { - val directoryOverride = configuredDirectory?.takeIf { it.isNotBlank() } - ?: env["CODER_SSH_LOG_DIR"]?.takeIf { it.isNotEmpty() } - if (directoryOverride != null) return Path.of(directoryOverride) - - // The CLI uses github.com/adrg/xdg: XDG_STATE_HOME must be absolute after home expansion. - val stateHome = absoluteStatePath(env["XDG_STATE_HOME"], home, os) ?: when (os) { - OS.MAC -> home.resolve("Library/Application Support") - OS.WINDOWS -> absoluteStatePath(env["LOCALAPPDATA"], home, os) ?: home.resolve("AppData/Local") - else -> home.resolve(".local/state") - } - return stateHome.resolve("coder/logs") - } - - private fun absoluteStatePath(value: String?, home: Path, os: OS?): Path? { - if (value.isNullOrEmpty()) return null - val prefix = if (os == OS.WINDOWS) { - "%USERPROFILE%".takeIf { value.startsWith(it) } - } else { - listOf("~", "\$HOME").firstOrNull { value.startsWith(it) } - } - val path = if (prefix == null) { - Path.of(value) - } else { - home.resolve(value.removePrefix(prefix).trimStart('/', '\\')) - } - return path.takeIf { it.isAbsolute } - } + ): Path = (configuredDirectory?.takeIf { it.isNotBlank() } + ?: env["CODER_SSH_LOG_DIR"]?.takeIf { it.isNotBlank() }) + ?.let { Path.of(it) } ?: defaultDirectory } private fun globalConfigArgs(): List = listOf("--global-config", coderConfigPath.toString()) diff --git a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderSshLogCollector.kt b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderSshLogCollector.kt index 801f432..5b6f5da 100644 --- a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderSshLogCollector.kt +++ b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderSshLogCollector.kt @@ -2,97 +2,119 @@ package com.coder.toolbox.diagnostics import com.coder.toolbox.session.SessionId import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ensureActive import kotlinx.coroutines.withContext +import net.lingala.zip4j.ZipFile +import net.lingala.zip4j.model.ZipParameters +import java.nio.channels.Channels import java.nio.channels.FileChannel import java.nio.file.Files import java.nio.file.LinkOption.NOFOLLOW_LINKS import java.nio.file.Path -import java.nio.file.StandardOpenOption.CREATE import java.nio.file.StandardOpenOption.READ -import java.nio.file.StandardOpenOption.TRUNCATE_EXISTING -import java.nio.file.StandardOpenOption.WRITE import java.nio.file.attribute.BasicFileAttributes +import java.nio.file.attribute.FileTime import java.time.Instant -// Include current CLI session filenames and older PID-based logs. -private val SSH_LOG_FILE = Regex("coder-ssh.*\\.log(?:\\.\\d+)?|\\d+\\.log") -private const val SSH_LOG_MAX_AGE_SECONDS = 3L * 24 * 60 * 60 -private const val SSH_LOG_MAX_BYTES = 50L * 1024 * 1024 +// Only Coder SSH logs named coder-ssh--.log. +private val SSH_LOG_FILE = Regex("coder-ssh.*-\\d+\\.log") +private const val SSH_LOG_MAX_AGE_3_DAYS_SECONDS = 3L * 24 * 60 * 60 +private const val SSH_LOG_CLEANUP_AGE_7_DAYS_SECONDS = 7L * 24 * 60 * 60 +private const val SSH_LOG_KEEP_COUNT = 20 +private const val SSH_LOG_MAX_50_MB_BYTES = 50L * 1024 * 1024 +private const val ARCHIVE_DIRECTORY = "toolbox-logs/ssh" internal class CoderSshLogCollector( private val logger: CoderLogger, private val logDirectory: Path, ) { + /** + * Adds recent SSH logs, and the log of the active SSH process, to [archive] in place. + * Each log is cut to its last 50 MB. + */ suspend fun collect( - logsRootFolder: Path, + archive: Path, sessionId: SessionId? = null, activeSshPid: Long? = null, ) = withContext(Dispatchers.IO) { - // Older CLIs may not have created any session logs. if (Files.notExists(logDirectory)) return@withContext - val cutoff = Instant.now().minusSeconds(SSH_LOG_MAX_AGE_SECONDS) - val entries = Files.newDirectoryStream(logDirectory).use { it.toList() } - // CLI proxy log names end in the parent SSH PID; timestamps distinguish reused PIDs. - val activeLog = activeSshPid?.let { pid -> - entries.filter { entry -> - val name = entry.fileName.toString() - (name == "$pid.log" || (name.startsWith("coder-ssh") && name.endsWith("-$pid.log"))) && - Files.isRegularFile(entry, NOFOLLOW_LINKS) - }.maxByOrNull { it.fileName.toString() } - } - for (entry in entries) { - ensureActive() - // An active session can remain quiet longer than the collection window. - collectFile(entry, logsRootFolder.resolve("coder-ssh"), cutoff.takeUnless { entry == activeLog }, sessionId) + val cutoff = Instant.now().minusSeconds(SSH_LOG_MAX_AGE_3_DAYS_SECONDS) + ZipFile(archive.toFile()).use { zip -> + for (entry in sshLogFiles()) { + ensureActive() + val attributes = readAttributes(entry, sessionId) ?: continue + val isActive = activeSshPid != null && entry.fileName.toString().endsWith("-$activeSshPid.log") + val isRecent = !attributes.lastModifiedTime().toInstant().isBefore(cutoff) + if (isActive || isRecent) addToArchive(zip, entry, attributes.lastModifiedTime(), sessionId) + } } } + /** Delete logs that are both outside the newest 20 and older than 7 days. */ @Suppress("TooGenericExceptionCaught") - private fun CoroutineScope.collectFile(source: Path, destination: Path, cutoff: Instant?, sessionId: SessionId?) { - if (!SSH_LOG_FILE.matches(source.fileName.toString())) return - var output: Path? = null + suspend fun cleanup(sessionId: SessionId? = null) = withContext(Dispatchers.IO) { try { - val attributes = Files.readAttributes(source, BasicFileAttributes::class.java, NOFOLLOW_LINKS) - val expired = cutoff != null && attributes.lastModifiedTime().toInstant().isBefore(cutoff) - if (!attributes.isRegularFile || expired) return + if (Files.notExists(logDirectory)) return@withContext + val cutoff = Instant.now().minusSeconds(SSH_LOG_CLEANUP_AGE_7_DAYS_SECONDS) + sshLogFiles() + .mapNotNull { entry -> + ensureActive() + readAttributes(entry, sessionId)?.let { entry to it.lastModifiedTime().toInstant() } + } + .sortedByDescending { (_, modified) -> modified } + .drop(SSH_LOG_KEEP_COUNT) + .filter { (_, modified) -> modified.isBefore(cutoff) } + .forEach { (path, _) -> + ensureActive() + runCatching { Files.deleteIfExists(path) } + .onFailure { logger.warn(sessionId, it, "Could not delete old SSH log $path") } + } + } catch (ex: CancellationException) { + throw ex + } catch (ex: Exception) { + logger.warn(sessionId, ex, "Could not clean up SSH logs in $logDirectory") + } + } + + // The log directory can be shared, so filter by name before looking at anything else. + private fun sshLogFiles(): List = Files.newDirectoryStream(logDirectory) { + SSH_LOG_FILE.matches(it.fileName.toString()) + }.use { it.toList() } + @Suppress("TooGenericExceptionCaught") + private fun readAttributes(source: Path, sessionId: SessionId?): BasicFileAttributes? { + return try { + Files.readAttributes(source, BasicFileAttributes::class.java, NOFOLLOW_LINKS).takeIf { it.isRegularFile } + } catch (ex: Exception) { + logger.warn(sessionId, ex, "Could not inspect SSH log $source") + null + } + } + + @Suppress("TooGenericExceptionCaught") + private fun addToArchive(zip: ZipFile, source: Path, modified: FileTime, sessionId: SessionId?) { + try { FileChannel.open(source, READ, NOFOLLOW_LINKS).use { input -> val size = input.size() - if (size > SSH_LOG_MAX_BYTES) { + if (size > SSH_LOG_MAX_50_MB_BYTES) { logger.warn( sessionId, - "Truncating SSH log $source ($size bytes) to its last $SSH_LOG_MAX_BYTES bytes", + "Truncating SSH log $source ($size bytes) to its last $SSH_LOG_MAX_50_MB_BYTES bytes", ) + input.position(size - SSH_LOG_MAX_50_MB_BYTES) } - Files.createDirectories(destination) - val targetPath = destination.resolve(source.fileName) - output = targetPath - FileChannel.open(targetPath, CREATE, WRITE, TRUNCATE_EXISTING).use { target -> - copyTail(input, target, size) + val parameters = ZipParameters().apply { + fileNameInZip = "$ARCHIVE_DIRECTORY/${source.fileName}" + lastModifiedFileTime = modified.toMillis() } + zip.addStream(Channels.newInputStream(input), parameters) } } catch (ex: CancellationException) { - output?.let { runCatching { Files.deleteIfExists(it) } } throw ex } catch (ex: Exception) { - output?.let { runCatching { Files.deleteIfExists(it) } } logger.warn(sessionId, ex, "Could not collect SSH log $source") } } - - private fun CoroutineScope.copyTail(input: FileChannel, target: FileChannel, size: Long) { - var remaining = minOf(size, SSH_LOG_MAX_BYTES) - var offset = size - remaining - while (remaining > 0) { - ensureActive() - val copied = input.transferTo(offset, remaining, target) - if (copied == 0L) break - offset += copied - remaining -= copied - } - } } diff --git a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderSupportBundleCollector.kt b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderSupportBundleCollector.kt index 3056ed9..66a0e8f 100644 --- a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderSupportBundleCollector.kt +++ b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderSupportBundleCollector.kt @@ -5,6 +5,7 @@ import com.jetbrains.toolbox.api.remoteDev.deploy.DiagnosticInfoCollector import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext +import net.lingala.zip4j.ZipFile import java.nio.file.Files import java.nio.file.Path @@ -25,7 +26,11 @@ internal class CoderSupportBundleCollector( try { Files.createDirectories(logsRootFolder) collectSupportBundle(logsRootFolder, sessionId) - sshLogs.collect(logsRootFolder, sessionId, sshPid) + // Without a usable support bundle the SSH logs still go into an archive of their own. + val bundle = logsRootFolder.resolve("coder-support.zip") + val hasBundle = ZipFile(bundle.toFile()).isValidZipFile + val archive = if (hasBundle) bundle else logsRootFolder.resolve("coder-ssh.zip") + sshLogs.collect(archive, sessionId, sshPid) } catch (ex: CancellationException) { throw ex } catch (ex: Exception) { diff --git a/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt b/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt index 9fbe8ca..9d78fbd 100644 --- a/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt +++ b/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt @@ -37,17 +37,19 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import java.nio.file.Files +import java.nio.file.Path import java.nio.file.attribute.FileTime import java.time.Duration import java.time.Instant import java.util.UUID import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit +import java.util.zip.ZipFile import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -59,6 +61,10 @@ import kotlin.test.assertSame import kotlin.test.assertTrue class CoderRemoteEnvironmentTest { + private fun readSshLog(archive: Path, name: String): String? = ZipFile(archive.toFile()).use { zip -> + zip.getEntry("toolbox-logs/ssh/$name")?.let { zip.getInputStream(it).reader().readText() } + } + @Test @OptIn(ExperimentalCoroutinesApi::class) fun `diagnostics reuse the SSH process resolved by network polling and clear it on disconnect`() = runTest { @@ -79,7 +85,7 @@ class CoderRemoteEnvironmentTest { root.resolve("before-connection"), ) } - assertFalse(Files.exists(root.resolve("before-connection/coder-ssh"))) + assertFalse(Files.exists(root.resolve("before-connection/coder-ssh.zip"))) verify(exactly = 0) { anyConstructed().findByWorkspaceAndAgent(any(), any()) } @@ -97,7 +103,7 @@ class CoderRemoteEnvironmentTest { } assertEquals( "active session", - Files.readString(root.resolve("diagnostics/coder-ssh/coder-ssh-active-12345.log")), + readSshLog(root.resolve("diagnostics/coder-ssh.zip"), "coder-ssh-active-12345.log"), ) fixture.environment.afterDisconnect(isManual = false) @@ -106,7 +112,7 @@ class CoderRemoteEnvironmentTest { root.resolve("after-disconnect"), ) } - assertFalse(Files.exists(root.resolve("after-disconnect/coder-ssh"))) + assertFalse(Files.exists(root.resolve("after-disconnect/coder-ssh.zip"))) verify(exactly = 1) { anyConstructed().findByWorkspaceAndAgent(any(), any()) } @@ -128,7 +134,7 @@ class CoderRemoteEnvironmentTest { val root = Files.createTempDirectory("coder-diagnostics-ended-log-test") try { val sshLogs = Files.createDirectories(root.resolve("ssh-logs")) - val activeLog = Files.writeString(sshLogs.resolve("12345.log"), "active session") + val activeLog = Files.writeString(sshLogs.resolve("coder-ssh-active-12345.log"), "active session") Files.setLastModifiedTime(activeLog, FileTime.from(Instant.now().minus(Duration.ofDays(4)))) every { fixture.cli.getSshLogDirectory() } returns sshLogs every { fixture.settingsStore.networkInfoDir } returns root.resolve("metrics").toString() @@ -138,7 +144,10 @@ class CoderRemoteEnvironmentTest { runBlocking { fixture.environment.diagnosticInfoCollector.collectAdditionalDiagnostics(root.resolve("connected")) } - assertTrue(Files.exists(root.resolve("connected/coder-ssh/12345.log"))) + assertEquals( + "active session", + readSshLog(root.resolve("connected/coder-ssh.zip"), "coder-ssh-active-12345.log"), + ) advanceTimeBy(5_000) runCurrent() @@ -146,7 +155,7 @@ class CoderRemoteEnvironmentTest { fixture.environment.diagnosticInfoCollector.collectAdditionalDiagnostics(root.resolve("ended")) } - assertFalse(Files.exists(root.resolve("ended/coder-ssh"))) + assertFalse(Files.exists(root.resolve("ended/coder-ssh.zip"))) verify(exactly = 2) { anyConstructed().findByWorkspaceAndAgent(fixture.workspace, fixture.agent) } @@ -203,7 +212,7 @@ class CoderRemoteEnvironmentTest { val root = Files.createTempDirectory("coder-diagnostics-test") try { val sshLogs = Files.createDirectories(root.resolve("ssh-logs")) - Files.writeString(sshLogs.resolve("coder-ssh-session.log"), "SSH session") + Files.writeString(sshLogs.resolve("coder-ssh-session-1.log"), "SSH session") every { fixture.cli.getSshLogDirectory() } returns sshLogs val collector = fixture.environment.diagnosticInfoCollector fixture.environment.update( @@ -228,7 +237,7 @@ class CoderRemoteEnvironmentTest { ) } verify(exactly = 1) { fixture.cli.getSshLogDirectory() } - assertEquals("SSH session", Files.readString(root.resolve("coder-ssh/coder-ssh-session.log"))) + assertEquals("SSH session", readSshLog(root.resolve("coder-ssh.zip"), "coder-ssh-session-1.log")) fixture.environment.update(fixture.workspace, null) collector.collectAdditionalDiagnostics(root) coVerify(exactly = 1) { refreshedCli.supportBundle(match { it.agentName == null }, any()) } @@ -245,8 +254,8 @@ class CoderRemoteEnvironmentTest { try { val firstLogs = Files.createDirectories(root.resolve("first-logs")) val refreshedLogs = Files.createDirectories(root.resolve("refreshed-logs")) - Files.writeString(firstLogs.resolve("coder-ssh-session.log"), "first session") - Files.writeString(refreshedLogs.resolve("coder-ssh-session.log"), "refreshed session") + Files.writeString(firstLogs.resolve("coder-ssh-session-1.log"), "first session") + Files.writeString(refreshedLogs.resolve("coder-ssh-session-1.log"), "refreshed session") every { fixture.cli.getSshLogDirectory() } returns firstLogs val firstCollector = fixture.environment.diagnosticInfoCollector verify(exactly = 1) { fixture.cli.getSshLogDirectory() } @@ -262,11 +271,11 @@ class CoderRemoteEnvironmentTest { assertEquals( "first session", - Files.readString(root.resolve("first-diagnostics/coder-ssh/coder-ssh-session.log")), + readSshLog(root.resolve("first-diagnostics/coder-ssh.zip"), "coder-ssh-session-1.log"), ) assertEquals( "refreshed session", - Files.readString(root.resolve("refreshed-diagnostics/coder-ssh/coder-ssh-session.log")), + readSshLog(root.resolve("refreshed-diagnostics/coder-ssh.zip"), "coder-ssh-session-1.log"), ) verify(exactly = 1) { fixture.cli.getSshLogDirectory() } verify(exactly = 1) { refreshedCli.getSshLogDirectory() } diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index d8fb2ea..6d28956 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -106,6 +106,28 @@ internal class CoderCLIManagerTest { assertEquals(secondDirectory, cli.getSshLogDirectory()) } + @Test + fun `proxy logging is gated and defaults to a deployment specific plugin directory`() { + val cli = supportBundleCli + val sshConfig = supportBundleProcessRoot.resolve("ssh-config") + val ws = workspace("dev", agents = mapOf("main" to UUID.randomUUID().toString())) + val address = WorkspaceAddress.from(ws, ws.latestBuild.resources.first().agents!!.first()) + context.settingsStore.updateSshLogDir("") + val expectedDirectory = context.settingsStore.dataDir(URI("https://coder.example.test").toURL()) + .resolve("ssh-logs") + cli.configSsh(setOf(address), feats = Features(proxyLogDirectory = true), sshConfigPath = sshConfig.toString()) + assertContains( + Files.readString(sshConfig), + "--log-dir ${ProxyCommandBuilder().argument(expectedDirectory.toString()).render()} -v", + ) + assertTrue(Files.isDirectory(expectedDirectory)) + + context.settingsStore.updateSshLogDir(supportBundleProcessRoot.resolve("custom").toString()) + cli.configSsh(setOf(address), feats = Features(), sshConfigPath = sshConfig.toString()) + assertFalse(Files.readString(sshConfig).contains("--log-dir")) + assertFalse(Files.exists(supportBundleProcessRoot.resolve("custom"))) + } + private fun waitingCommand(): List = listOf( "/bin/sh", "-c", "echo $$ > \"\$1\"; exec sleep 30", "test", supportBundleProcessRoot.resolve("pid").toString(), ) @@ -1034,6 +1056,7 @@ internal class CoderCLIManagerTest { "log-dir", "blank", sshLogDirectory = tmpdir.resolve("ssh-logs"), + features = Features(proxyLogDirectory = true, reportWorkspaceUsage = true), ), SSHTest( listOf(workspace), @@ -1671,11 +1694,15 @@ internal class CoderCLIManagerTest { fun testFeatures() { val tests = listOf( - Pair("2.5.0", Features(disableAutostart = true)), - Pair("2.13.0", Features(disableAutostart = true, reportWorkspaceUsage = true)), + Pair("2.5.0", Features(proxyLogDirectory = true, disableAutostart = true)), + Pair( + "2.13.0", + Features(proxyLogDirectory = true, disableAutostart = true, reportWorkspaceUsage = true) + ), Pair( "2.25.0", Features( + proxyLogDirectory = true, disableAutostart = true, reportWorkspaceUsage = true, wildcardSsh = true, @@ -1684,6 +1711,7 @@ internal class CoderCLIManagerTest { ), Pair( "4.9.0", Features( + proxyLogDirectory = true, disableAutostart = true, reportWorkspaceUsage = true, wildcardSsh = true, @@ -1694,6 +1722,7 @@ internal class CoderCLIManagerTest { Pair( "2.29.0", Features( + proxyLogDirectory = true, disableAutostart = true, reportWorkspaceUsage = true, wildcardSsh = true, @@ -1701,7 +1730,9 @@ internal class CoderCLIManagerTest { keyringAuth = true, ) ), - Pair("2.4.9", Features()), + Pair("2.4.9", Features(proxyLogDirectory = true)), + Pair("2.4.0", Features(proxyLogDirectory = true)), + Pair("2.3.9", Features()), Pair("1.0.1", Features()), ) diff --git a/src/test/kotlin/com/coder/toolbox/cli/SshLogDirectoryTest.kt b/src/test/kotlin/com/coder/toolbox/cli/SshLogDirectoryTest.kt index f45b7d5..003d911 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/SshLogDirectoryTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/SshLogDirectoryTest.kt @@ -1,7 +1,6 @@ package com.coder.toolbox.cli import com.coder.toolbox.cli.CoderCLIManager.Companion.getSshLogDirectory -import com.coder.toolbox.util.OS import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import kotlin.test.Test @@ -9,89 +8,24 @@ import kotlin.test.assertEquals class SshLogDirectoryTest { @TempDir - lateinit var home: Path + lateinit var root: Path @Test - fun `uses the CLI state directory default on each platform`() { - val defaults = mapOf( - OS.LINUX to ".local/state/coder/logs", - OS.MAC to "Library/Application Support/coder/logs", - OS.WINDOWS to "AppData/Local/coder/logs", - ) - for ((os, directory) in defaults) { - assertEquals(home.resolve(directory), getSshLogDirectory(null, emptyMap(), os, home)) - } + fun `uses plugin directory regardless of CLI state environment`() { + val default = root.resolve("ssh-logs") + assertEquals(default, getSshLogDirectory(null, default, mapOf("XDG_STATE_HOME" to "/state"))) } @Test - fun `configured directory takes precedence over environment overrides`() { - val configured = home.resolve("configured logs") - val env = mapOf( - "CODER_SSH_LOG_DIR" to home.resolve("env-logs").toString(), - "XDG_STATE_HOME" to home.resolve("state").toString(), - ) - assertEquals(configured, getSshLogDirectory(configured.toString(), env, OS.LINUX, home)) - assertEquals(home.resolve("env-logs"), getSshLogDirectory(" ", env, OS.LINUX, home)) + fun `configured directory takes precedence over environment`() { + val configured = root.resolve("configured logs") + val env = mapOf("CODER_SSH_LOG_DIR" to root.resolve("env-logs").toString()) + assertEquals(configured, getSshLogDirectory(configured.toString(), root, env)) + assertEquals(root.resolve("env-logs"), getSshLogDirectory(" ", root, env)) } @Test - fun `XDG state override takes precedence on every platform`() { - val state = home.resolve("custom-state") - val env = mapOf( - "CODER_SSH_LOG_DIR" to "", - "XDG_STATE_HOME" to state.toString(), - "LOCALAPPDATA" to home.resolve("local-app-data").toString(), - ) - for (os in OS.entries) { - assertEquals(state.resolve("coder/logs"), getSshLogDirectory(null, env, os, home)) - } - } - - @Test - fun `relative XDG state overrides fall back to the platform default`() { - for (state in listOf("", "relative/state")) { - assertEquals( - home.resolve(".local/state/coder/logs"), - getSshLogDirectory(null, mapOf("XDG_STATE_HOME" to state), OS.LINUX, home), - ) - } - } - - @Test - fun `expands home references in XDG paths using CLI platform conventions`() { - val overrides = mapOf( - OS.LINUX to listOf("~/state", "\$HOME/state"), - OS.MAC to listOf("~/state", "\$HOME/state"), - OS.WINDOWS to listOf("%USERPROFILE%/state"), - ) - for ((os, values) in overrides) { - for (value in values) { - assertEquals( - home.resolve("state/coder/logs"), - getSshLogDirectory(null, mapOf("XDG_STATE_HOME" to value), os, home), - ) - } - } - } - - @Test - fun `uses Windows local app data when set`() { - val localAppData = home.resolve("local-app-data") - assertEquals( - localAppData.resolve("coder/logs"), - getSshLogDirectory(null, mapOf("LOCALAPPDATA" to localAppData.toString()), OS.WINDOWS, home), - ) - } - - @Test - fun `uses the CLI home environment when no state override is set`() { - assertEquals( - home.resolve(".local/state/coder/logs"), - getSshLogDirectory(null, mapOf("HOME" to home.toString()), OS.LINUX), - ) - assertEquals( - home.resolve("AppData/Local/coder/logs"), - getSshLogDirectory(null, mapOf("USERPROFILE" to home.toString()), OS.WINDOWS), - ) + fun `blank overrides use the plugin directory`() { + assertEquals(root, getSshLogDirectory(" ", root, mapOf("CODER_SSH_LOG_DIR" to ""))) } } diff --git a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderSshLogCollectorTest.kt b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderSshLogCollectorTest.kt index d2254f2..2eb5cd0 100644 --- a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderSshLogCollectorTest.kt +++ b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderSshLogCollectorTest.kt @@ -1,9 +1,6 @@ package com.coder.toolbox.diagnostics -import com.coder.toolbox.session.SessionId -import com.jetbrains.toolbox.api.core.diagnostics.Logger import io.mockk.mockk -import io.mockk.verify import kotlinx.coroutines.CancellationException import kotlinx.coroutines.cancel import kotlinx.coroutines.currentCoroutineContext @@ -18,6 +15,7 @@ import java.nio.file.Path import java.nio.file.attribute.FileTime import java.time.Duration import java.time.Instant +import java.util.zip.ZipFile import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -29,18 +27,26 @@ class CoderSshLogCollectorTest { lateinit var root: Path private val logs get() = root.resolve("logs") - private val diagnostics get() = root.resolve("diagnostics") + private val archive get() = root.resolve("archive.zip") private fun collector() = CoderSshLogCollector(mockk(relaxed = true), logs) + private fun collected(): Map { + if (Files.notExists(archive)) return emptyMap() + return ZipFile(archive.toFile()).use { zip -> + zip.entries().asSequence().associate { + it.name.removePrefix("toolbox-logs/ssh/") to zip.getInputStream(it).reader().readText() + } + } + } + @Test - fun `only the newest log for the active SSH process bypasses the age cutoff`() = runTest { + fun `logs of the active SSH process bypass the age cutoff`() = runTest { Files.createDirectories(logs) val activeName = "coder-ssh-20261010-120000-abcde-12345.log" val oldNames = listOf( activeName, - "coder-ssh-20261009-120000-abcde-12345.log", - "$activeName.1", + "coder-ssh-20261010-120000-abcde-12345.log.1", "coder-ssh-20261010-120000-abcde-67890.log", "coder-ssh-20261010-120000-abcde-112345.log", "12345.log", @@ -50,63 +56,53 @@ class CoderSshLogCollectorTest { val file = Files.writeString(logs.resolve(name), name) Files.setLastModifiedTime(file, oldTime) } - Files.writeString(logs.resolve("coder-ssh-recent.log"), "recent") + Files.writeString(logs.resolve("coder-ssh-recent-1.log"), "recent") - collector().collect(diagnostics, activeSshPid = 12345) + collector().collect(archive, activeSshPid = 12345) - val collected = Files.list(diagnostics.resolve("coder-ssh")).use { files -> - files.map { it.fileName.toString() }.toList().toSet() - } - assertEquals(setOf(activeName, "coder-ssh-recent.log"), collected) - assertEquals(activeName, Files.readString(diagnostics.resolve("coder-ssh/$activeName"))) + assertEquals(mapOf(activeName to activeName, "coder-ssh-recent-1.log" to "recent"), collected()) } @Test - fun `active PID-only logs bypass the age cutoff`() = runTest { + fun `PID-only logs are ignored even for the active PID`() = runTest { Files.createDirectories(logs) val source = Files.writeString(logs.resolve("12345.log"), "active session") Files.setLastModifiedTime(source, FileTime.from(Instant.now().minus(Duration.ofDays(4)))) - collector().collect(diagnostics, activeSshPid = 12345) + collector().collect(archive, activeSshPid = 12345) - assertEquals("active session", Files.readString(diagnostics.resolve("coder-ssh/12345.log"))) + assertFalse(Files.exists(archive)) } @Test - fun `collects recent CLI logs including PID and rotated logs only from the directory itself`() = runTest { + fun `collects only recent PID-suffixed CLI logs from the directory itself`() = runTest { Files.createDirectories(logs) - val expected = mapOf( - "coder-ssh-20261015-120000-abcde-123.log" to "session", - "coder-ssh-session.log.1" to "rotated", - "12345.log" to "older CLI", - ) + val expected = mapOf("coder-ssh-20261015-120000-abcde-123.log" to "session") for ((name, contents) in expected) Files.writeString(logs.resolve(name), contents) - for (name in listOf("other.log", "secret.env", "12345.log.1", "coder-ssh.log.zip")) { + val unrelated = listOf( + "12345.log", "other.log", "secret.env", "12345.log.1", "coder-ssh.log.zip", + "coder-ssh-session.log", "coder-ssh-session-1.log.1", + ) + for (name in unrelated) { Files.writeString(logs.resolve(name), "unrelated") } - val old = Files.writeString(logs.resolve("coder-ssh-old.log"), "old") + val old = Files.writeString(logs.resolve("coder-ssh-old-1.log"), "old") Files.setLastModifiedTime(old, FileTime.from(Instant.now().minus(Duration.ofDays(4)))) Files.createDirectories(logs.resolve("nested")) Files.writeString(logs.resolve("nested/coder-ssh-nested.log"), "nested") - Files.createDirectory(logs.resolve("coder-ssh-directory.log")) + Files.createDirectory(logs.resolve("coder-ssh-directory-1.log")) - collector().collect(diagnostics) + collector().collect(archive) - val actual = Files.list(diagnostics.resolve("coder-ssh")).use { files -> - files.toList().associate { it.fileName.toString() to Files.readString(it) } - } - assertEquals(expected, actual) + assertEquals(expected, collected()) } @Test - fun `missing or empty log directory leaves other diagnostics available`() = runTest { - Files.createDirectories(diagnostics) - Files.writeString(diagnostics.resolve("coder-support.zip"), "bundle") - collector().collect(diagnostics) + fun `missing or empty log directory does not create an archive`() = runTest { + collector().collect(archive) Files.createDirectories(logs) - collector().collect(diagnostics) - assertEquals("bundle", Files.readString(diagnostics.resolve("coder-support.zip"))) - assertFalse(Files.exists(diagnostics.resolve("coder-ssh"))) + collector().collect(archive) + assertFalse(Files.exists(archive)) } @Test @@ -116,28 +112,8 @@ class CoderSshLogCollectorTest { val secret = Files.writeString(root.resolve("secret.txt"), "secret") Files.createSymbolicLink(logs.resolve("coder-ssh-secret-12345.log"), secret) Files.createSymbolicLink(logs.resolve("coder-ssh-directory-12345.log"), root) - collector().collect(diagnostics, activeSshPid = 12345) - assertFalse(Files.exists(diagnostics.resolve("coder-ssh"))) - } - - @Test - fun `one failed copy does not prevent collecting other logs`() = runTest { - Files.createDirectories(logs) - val source = Files.writeString(logs.resolve("coder-ssh-blocked.log"), "blocked") - Files.writeString(logs.resolve("coder-ssh-readable.log"), "readable") - val blocked = Files.createDirectories(diagnostics.resolve("coder-ssh/coder-ssh-blocked.log")) - Files.writeString(blocked.resolve("keep.txt"), "existing diagnostics") - val delegate = mockk(relaxed = true) - val logger = CoderLogger(delegate, mockk(), backgroundScope, mockk()) - val sessionId = SessionId.generate() - - CoderSshLogCollector(logger, logs).collect(diagnostics, sessionId) - - assertEquals("readable", Files.readString(diagnostics.resolve("coder-ssh/coder-ssh-readable.log"))) - assertEquals("existing diagnostics", Files.readString(blocked.resolve("keep.txt"))) - verify(exactly = 1) { - delegate.warn(any(), "client_session_id=$sessionId Could not collect SSH log $source") - } + collector().collect(archive, activeSshPid = 12345) + assertFalse(Files.exists(archive)) } @Test @@ -152,44 +128,123 @@ class CoderSshLogCollectorTest { } Files.setLastModifiedTime(source, FileTime.from(Instant.now().minus(Duration.ofDays(4)))) - collector().collect(diagnostics, activeSshPid = 12345) + collector().collect(archive, activeSshPid = 12345) - val output = diagnostics.resolve("coder-ssh/coder-ssh-large-12345.log") - assertEquals(limit, Files.size(output)) - RandomAccessFile(output.toFile(), "r").use { - assertEquals(0, it.read()) - it.seek(limit - 4) - assertEquals("tail", it.readLine()) + val output = ZipFile(archive.toFile()).use { zip -> + zip.getInputStream(zip.getEntry("toolbox-logs/ssh/coder-ssh-large-12345.log")).readAllBytes() } + assertEquals(limit, output.size.toLong()) + assertEquals(0, output.first().toInt()) + assertEquals("tail", String(output, output.size - 4, 4)) assertEquals(limit + 4, Files.size(source)) RandomAccessFile(source.toFile(), "r").use { assertEquals('h'.code, it.read()) } } + private fun writeLogs(prefix: String, count: Int, minAge: Duration) { + Files.createDirectories(logs) + // Higher indexes are older, so the ordering between files is deterministic. + for (index in 1..count) { + val file = Files.writeString(logs.resolve("coder-ssh-$prefix-$index.log"), "log") + Files.setLastModifiedTime(file, FileTime.from(Instant.now().minus(minAge.plusMinutes(index.toLong())))) + } + } + + private fun remainingLogs(prefix: String = ""): Int = + Files.list(logs).use { files -> files.filter { prefix in it.fileName.toString() }.count() }.toInt() + + @Test + fun `cleanup deletes nothing when there are fewer than 20 logs even if all are older than 7 days`() = runTest { + writeLogs("old", 10, Duration.ofDays(8)) + collector().cleanup() + assertEquals(10, remainingLogs()) + } + + @Test + fun `cleanup deletes nothing when there are 50 logs and all are newer than 7 days`() = runTest { + writeLogs("recent", 50, Duration.ofDays(1)) + collector().cleanup() + assertEquals(50, remainingLogs()) + } + + @Test + fun `cleanup deletes the oldest 30 of 50 logs when 30 are older than 7 days`() = runTest { + writeLogs("old", 30, Duration.ofDays(8)) + writeLogs("recent", 20, Duration.ofDays(1)) + collector().cleanup() + assertEquals(20, remainingLogs()) + assertEquals(20, remainingLogs("recent")) + } + + @Test + fun `cleanup deletes the 15 old logs of 50 and keeps the other 35`() = runTest { + writeLogs("old", 15, Duration.ofDays(8)) + writeLogs("recent", 35, Duration.ofDays(1)) + collector().cleanup() + assertEquals(35, remainingLogs()) + assertEquals(35, remainingLogs("recent")) + } + + @Test + fun `cleanup keeps the newest 20 even when all 25 logs are older than 7 days`() = runTest { + writeLogs("old", 25, Duration.ofDays(8)) + collector().cleanup() + assertEquals(20, remainingLogs()) + for (index in 21..25) assertFalse(Files.exists(logs.resolve("coder-ssh-old-$index.log")), "log $index") + } + + @Test + fun `cleanup ignores unrelated files and directories`() = runTest { + writeLogs("old", 25, Duration.ofDays(8)) + val ignored = listOf("12345.log", "other.log", "coder-ssh-old.log.1") + for (name in ignored) { + Files.setLastModifiedTime(Files.writeString(logs.resolve(name), "keep"), FileTime.from(Instant.EPOCH)) + } + val directory = Files.createDirectory(logs.resolve("coder-ssh-directory-1.log")) + Files.setLastModifiedTime(directory, FileTime.from(Instant.EPOCH)) + + collector().cleanup() + + for (name in ignored) assertTrue(Files.exists(logs.resolve(name))) + assertTrue(Files.isDirectory(directory)) + } + + @Test + @EnabledOnOs(OS.LINUX, OS.MAC) + fun `cleanup skips symlinks and their targets`() = runTest { + Files.createDirectories(logs) + val target = Files.writeString(root.resolve("target.log"), "keep") + Files.setLastModifiedTime(target, FileTime.from(Instant.EPOCH)) + val link = Files.createSymbolicLink(logs.resolve("coder-ssh-link-1.log"), target) + repeat(20) { Files.writeString(logs.resolve("coder-ssh-$it.log"), "recent") } + collector().cleanup() + assertTrue(Files.isSymbolicLink(link)) + assertEquals("keep", Files.readString(target)) + } + @Test fun `cancellation propagates without collecting SSH logs`() = runTest { Files.createDirectories(logs) - Files.writeString(logs.resolve("coder-ssh-session.log"), "SSH session") + Files.writeString(logs.resolve("coder-ssh-session-1.log"), "SSH session") launch { currentCoroutineContext().cancel() - assertFailsWith { collector().collect(diagnostics) } + assertFailsWith { collector().collect(archive) } }.join() - assertFalse(Files.exists(diagnostics)) + assertFalse(Files.exists(archive)) } @Test @EnabledOnOs(OS.LINUX, OS.MAC) fun `unreadable files do not prevent collecting other logs`() = runTest { Files.createDirectories(logs) - val unreadable = Files.writeString(logs.resolve("coder-ssh-unreadable.log"), "unreadable") - Files.writeString(logs.resolve("coder-ssh-readable.log"), "readable") + val unreadable = Files.writeString(logs.resolve("coder-ssh-unreadable-1.log"), "unreadable") + Files.writeString(logs.resolve("coder-ssh-readable-1.log"), "readable") val permissions = Files.getPosixFilePermissions(unreadable) try { Files.setPosixFilePermissions(unreadable, emptySet()) - collector().collect(diagnostics) - assertTrue(Files.exists(diagnostics.resolve("coder-ssh/coder-ssh-readable.log"))) - if (!Files.isReadable(unreadable)) { - assertFalse(Files.exists(diagnostics.resolve("coder-ssh/coder-ssh-unreadable.log"))) - } + collector().collect(archive) + val collected = collected() + assertEquals("readable", collected["coder-ssh-readable-1.log"]) + if (!Files.isReadable(unreadable)) assertFalse("coder-ssh-unreadable-1.log" in collected) } finally { Files.setPosixFilePermissions(unreadable, permissions) } diff --git a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderSupportBundleCollectorTest.kt b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderSupportBundleCollectorTest.kt index 2f6b708..5a17460 100644 --- a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderSupportBundleCollectorTest.kt +++ b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderSupportBundleCollectorTest.kt @@ -1,8 +1,6 @@ package com.coder.toolbox.diagnostics -import com.coder.toolbox.cli.CoderCLIManager.Companion.getSshLogDirectory import com.coder.toolbox.session.SessionId -import com.coder.toolbox.util.OS import com.jetbrains.toolbox.api.core.diagnostics.Logger import io.mockk.confirmVerified import io.mockk.mockk @@ -12,6 +10,9 @@ import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.io.TempDir import java.nio.file.Files import java.nio.file.Path +import java.util.zip.ZipEntry +import java.util.zip.ZipFile +import java.util.zip.ZipOutputStream import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -26,7 +27,7 @@ class CoderSupportBundleCollectorTest { fun `unavailable diagnostics directory stops collection and logs one warning`() = runTest { val destination = Files.writeString(root.resolve("diagnostics"), "existing file") val sshLogs = Files.createDirectories(root.resolve("ssh-logs")) - val sshLog = Files.writeString(sshLogs.resolve("coder-ssh-session.log"), "SSH session") + val sshLog = Files.writeString(sshLogs.resolve("coder-ssh-session-1.log"), "SSH session") val logger = mockk(relaxed = true) var bundleCollected = false val collector = CoderSupportBundleCollector(logger, sshLogs) { @@ -43,39 +44,29 @@ class CoderSupportBundleCollectorTest { } @Test - fun `writes the bundle inside Toolbox diagnostics`() = runTest { + fun `appends SSH logs inside the bundle and preserves original entries and source logs`() = runTest { val sshLogs = Files.createDirectories(root.resolve("ssh-logs")) - Files.writeString(sshLogs.resolve("coder-ssh-session.log"), "SSH session") + Files.writeString(sshLogs.resolve("coder-ssh-session-1.log"), "SSH session") val collector = CoderSupportBundleCollector(mockk(relaxed = true), sshLogs) { - Files.writeString(it, "bundle") + writeBundle(it) } collector.collectAdditionalDiagnostics(root.resolve("diagnostics")) - assertEquals("bundle", Files.readString(root.resolve("diagnostics/coder-support.zip"))) - assertEquals("SSH session", Files.readString(root.resolve("diagnostics/coder-ssh/coder-ssh-session.log"))) - } - - @Test - fun `collects SSH logs from the CLI default when no log directory is configured`() = runTest { - val sshLogs = Files.createDirectories(root.resolve(".local/state/coder/logs")) - Files.writeString(sshLogs.resolve("coder-ssh-default.log"), "default SSH session") - val collector = CoderSupportBundleCollector( - mockk(relaxed = true), - getSshLogDirectory(null, emptyMap(), OS.LINUX, root), - ) { Files.writeString(it, "bundle") } - - collector.collectAdditionalDiagnostics(root.resolve("diagnostics")) - - assertEquals( - "default SSH session", - Files.readString(root.resolve("diagnostics/coder-ssh/coder-ssh-default.log")), - ) + ZipFile(root.resolve("diagnostics/coder-support.zip").toFile()).use { zip -> + assertEquals("bundle", zip.getInputStream(zip.getEntry("agent.txt")).reader().readText()) + assertEquals( + "SSH session", + zip.getInputStream(zip.getEntry("toolbox-logs/ssh/coder-ssh-session-1.log")).reader().readText(), + ) + } + assertFalse(Files.exists(root.resolve("diagnostics/coder-ssh"))) + assertEquals("SSH session", Files.readString(sshLogs.resolve("coder-ssh-session-1.log"))) } @Test fun `failure removes partial bundle and preserves other diagnostics without leaking error details`() = runTest { Files.writeString(root.resolve("other.log"), "other diagnostics") val sshLogs = Files.createDirectories(root.resolve("ssh-logs")) - Files.writeString(sshLogs.resolve("12345.log"), "SSH session") + Files.writeString(sshLogs.resolve("coder-ssh-session-1.log"), "SSH session") val delegate = mockk(relaxed = true) val logger = CoderLogger(delegate, mockk(), backgroundScope, mockk()) val sessionId = SessionId.generate() @@ -89,7 +80,7 @@ class CoderSupportBundleCollectorTest { val report = Files.readString(root.resolve("coder-support-error.txt")) assertTrue(report.contains("IllegalStateException")) assertFalse(report.contains("sensitive command output")) - assertEquals("SSH session", Files.readString(root.resolve("coder-ssh/12345.log"))) + assertEquals("SSH session", readSshLog(root.resolve("coder-ssh.zip"))) verify(exactly = 1) { delegate.warn(any(), "client_session_id=$sessionId Could not collect the Coder support bundle") } @@ -98,7 +89,7 @@ class CoderSupportBundleCollectorTest { @Test fun `cancellation propagates and removes partial output`() = runTest { val sshLogs = Files.createDirectories(root.resolve("ssh-logs")) - Files.writeString(sshLogs.resolve("coder-ssh-session.log"), "SSH session") + Files.writeString(sshLogs.resolve("coder-ssh-session-1.log"), "SSH session") val collector = CoderSupportBundleCollector(mockk(relaxed = true), sshLogs) { Files.writeString(it, "partial") throw CancellationException("cancelled") @@ -106,7 +97,7 @@ class CoderSupportBundleCollectorTest { assertFailsWith { collector.collectAdditionalDiagnostics(root) } assertFalse(Files.exists(root.resolve("coder-support.zip"))) assertFalse(Files.exists(root.resolve("coder-support-error.txt"))) - assertFalse(Files.exists(root.resolve("coder-ssh"))) + assertFalse(Files.exists(root.resolve("coder-ssh.zip"))) } @Test @@ -128,4 +119,29 @@ class CoderSupportBundleCollectorTest { ) } } + + @Test + fun `invalid bundle is untouched and SSH logs go into their own archive`() = runTest { + val logs = Files.createDirectories(root.resolve("ssh-logs")) + Files.writeString(logs.resolve("coder-ssh-session-1.log"), "session") + val collector = CoderSupportBundleCollector(mockk(relaxed = true), logs) { + Files.writeString(it, "invalid zip") + } + collector.collectAdditionalDiagnostics(root) + assertEquals("invalid zip", Files.readString(root.resolve("coder-support.zip"))) + assertEquals("session", readSshLog(root.resolve("coder-ssh.zip"))) + } + + private fun readSshLog(archive: Path): String = ZipFile(archive.toFile()).use { zip -> + zip.getInputStream(zip.getEntry("toolbox-logs/ssh/coder-ssh-session-1.log")).reader().readText() + } + + private fun writeBundle(path: Path) { + ZipOutputStream(Files.newOutputStream(path)).use { zip -> + zip.putNextEntry(ZipEntry("agent.txt")) + zip.write("bundle".toByteArray()) + zip.closeEntry() + } + } + }