From db36fd0393a825ff0cf9e4d692ca548b889c94e3 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 21 May 2026 15:17:25 +0000 Subject: [PATCH 1/5] feat: collect VS Code diagnostics in support bundles --- src/commands.ts | 4 +- src/core/supportBundleLogs.ts | 158 ----------- src/supportBundle/appendVsCodeLogs.ts | 90 +++++++ src/supportBundle/diagnostics.ts | 18 ++ src/supportBundle/files.ts | 155 +++++++++++ src/supportBundle/logFiles.ts | 214 +++++++++++++++ src/supportBundle/settings.ts | 152 +++++++++++ src/supportBundle/vscodeLogs.ts | 86 ++++++ test/unit/core/supportBundleLogs.test.ts | 254 ------------------ .../supportBundle/appendVsCodeLogs.test.ts | 178 ++++++++++++ test/unit/supportBundle/diagnostics.test.ts | 50 ++++ test/unit/supportBundle/files.test.ts | 87 ++++++ test/unit/supportBundle/logFiles.test.ts | 244 +++++++++++++++++ test/unit/supportBundle/settings.test.ts | 236 ++++++++++++++++ test/unit/supportBundle/vscodeLogs.test.ts | 87 ++++++ 15 files changed, 1599 insertions(+), 414 deletions(-) delete mode 100644 src/core/supportBundleLogs.ts create mode 100644 src/supportBundle/appendVsCodeLogs.ts create mode 100644 src/supportBundle/diagnostics.ts create mode 100644 src/supportBundle/files.ts create mode 100644 src/supportBundle/logFiles.ts create mode 100644 src/supportBundle/settings.ts create mode 100644 src/supportBundle/vscodeLogs.ts delete mode 100644 test/unit/core/supportBundleLogs.test.ts create mode 100644 test/unit/supportBundle/appendVsCodeLogs.test.ts create mode 100644 test/unit/supportBundle/diagnostics.test.ts create mode 100644 test/unit/supportBundle/files.test.ts create mode 100644 test/unit/supportBundle/logFiles.test.ts create mode 100644 test/unit/supportBundle/settings.test.ts create mode 100644 test/unit/supportBundle/vscodeLogs.test.ts diff --git a/src/commands.ts b/src/commands.ts index 1f48d36a40..745e01a684 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -11,7 +11,6 @@ import { workspaceStatusLabel, } from "./api/api-helper"; import * as cliExec from "./core/cliExec"; -import { appendVsCodeLogs } from "./core/supportBundleLogs"; import { CertificateError } from "./error/certificateError"; import { toError } from "./error/errorUtils"; import { type FeatureSet, featureSetForVersion } from "./featureSet"; @@ -26,6 +25,7 @@ import { applySettingOverrides, } from "./remote/sshOverrides"; import { resolveCliAuth } from "./settings/cli"; +import { appendVsCodeLogs } from "./supportBundle/appendVsCodeLogs"; import { toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; import { parseSpeedtestResult } from "./webviews/speedtest/types"; @@ -304,7 +304,7 @@ export class Commands { await appendVsCodeLogs( outputUri.fsPath, { - remoteSshLogPath: this.workspaceLogPath, + activeProxyLogPath: this.workspaceLogPath, proxyLogDir: this.pathResolver.getProxyLogPath(), extensionLogDir: this.pathResolver.getCodeLogDir(), }, diff --git a/src/core/supportBundleLogs.ts b/src/core/supportBundleLogs.ts deleted file mode 100644 index e21551994c..0000000000 --- a/src/core/supportBundleLogs.ts +++ /dev/null @@ -1,158 +0,0 @@ -import { unzip, zip, type Zippable } from "fflate"; -import * as fs from "node:fs/promises"; -import * as path from "node:path"; -import { promisify } from "node:util"; - -import { type Logger } from "../logging/logger"; -import { renameWithRetry } from "../util/fs"; - -export interface LogSources { - remoteSshLogPath?: string; - proxyLogDir?: string; - extensionLogDir?: string; -} - -// 3 days is enough context for recent issues; matching the 7-day -// rotation would bloat the bundle. -const LOG_MAX_AGE_MS = 3 * 24 * 60 * 60 * 1000; - -const unzipAsync = promisify(unzip); -const zipAsync = promisify(zip); - -async function collectDirFiles( - dirPath: string, - logger: Logger, -): Promise> { - const results = new Map(); - - let entries: string[]; - try { - entries = await fs.readdir(dirPath); - } catch (error) { - logger.warn(`Could not read log directory ${dirPath}`, error); - return results; - } - - const cutoff = Date.now() - LOG_MAX_AGE_MS; - - await Promise.all( - entries.map(async (entry) => { - const filePath = path.join(dirPath, entry); - try { - const stat = await fs.stat(filePath); - if (!stat.isFile() || stat.mtimeMs < cutoff) { - return; - } - results.set(entry, await fs.readFile(filePath)); - } catch (error) { - logger.warn(`Could not read log file ${filePath}`, error); - } - }), - ); - - return results; -} - -/** - * Gather log files from each source independently so a failure in one - * does not affect the others. - */ -async function collectLogFiles( - sources: LogSources, - logger: Logger, -): Promise> { - const files = new Map(); - - if (sources.remoteSshLogPath) { - try { - files.set( - `vscode-logs/remote-ssh/${path.basename(sources.remoteSshLogPath)}`, - await fs.readFile(sources.remoteSshLogPath), - ); - } catch (error) { - logger.warn("Could not read Remote SSH log", error); - } - } - - if (sources.proxyLogDir) { - for (const [name, data] of await collectDirFiles( - sources.proxyLogDir, - logger, - )) { - files.set(`vscode-logs/proxy/${name}`, data); - } - } - - if (sources.extensionLogDir) { - for (const [name, data] of await collectDirFiles( - sources.extensionLogDir, - logger, - )) { - files.set(`vscode-logs/extension/${name}`, data); - } - } - - return files; -} - -/** - * Best-effort: append VS Code logs to a support bundle zip. - * Uses atomic rename to avoid corrupting the original bundle on failure. - */ -export async function appendVsCodeLogs( - zipPath: string, - sources: LogSources, - logger: Logger, -): Promise { - try { - const logFiles = await collectLogFiles(sources, logger); - if (logFiles.size === 0) { - logger.info("No VS Code logs found to add to support bundle"); - return; - } - - logger.info( - `Adding ${logFiles.size} VS Code log file(s) to support bundle`, - ); - - // Write to a named temporary path first so a failure at the rename step - // leaves the user with a properly named file containing VS Code logs. - const parsed = path.parse(zipPath); - const vscodeBundlePath = path.join( - parsed.dir, - `${parsed.name}-vscode${parsed.ext}`, - ); - - try { - const entries: Zippable = await unzipAsync(await fs.readFile(zipPath)); - for (const [name, data] of logFiles) { - entries[name] = data; - } - const updated = await zipAsync(entries); - await fs.writeFile(vscodeBundlePath, updated); - } catch (error) { - logger.error("Failed to add VS Code logs to support bundle", error); - try { - await fs.rm(vscodeBundlePath, { force: true }); - } catch (cleanupError) { - logger.warn( - `Could not clean up partial bundle at ${vscodeBundlePath}`, - cleanupError, - ); - } - return; - } - - try { - await renameWithRetry(fs.rename, vscodeBundlePath, zipPath); - } catch (error) { - logger.warn( - `Could not replace original bundle; VS Code logs saved separately at ${vscodeBundlePath}`, - error, - ); - } - } catch (error) { - // Best-effort: never let a failure here lose the user's bundle. - logger.error("Unexpected error appending VS Code logs", error); - } -} diff --git a/src/supportBundle/appendVsCodeLogs.ts b/src/supportBundle/appendVsCodeLogs.ts new file mode 100644 index 0000000000..e6fb39ba65 --- /dev/null +++ b/src/supportBundle/appendVsCodeLogs.ts @@ -0,0 +1,90 @@ +import { unzip, zip, type Zippable } from "fflate"; +import { randomUUID } from "node:crypto"; +import * as fs from "node:fs/promises"; +import * as path from "node:path"; +import { promisify } from "node:util"; + +import { type Logger } from "../logging/logger"; +import { renameWithRetry } from "../util/fs"; + +import { collectVsCodeDiagnostics, type LogSources } from "./diagnostics"; + +export type { LogSources } from "./diagnostics"; + +const unzipAsync = promisify(unzip); +const zipAsync = promisify(zip); + +function vscodeBundlePath(zipPath: string): string { + const parsed = path.parse(zipPath); + return path.join( + parsed.dir, + `${parsed.name}-vscode-${randomUUID()}${parsed.ext}`, + ); +} + +async function writeBundleWithLogs( + zipPath: string, + outputPath: string, + logFiles: Map, +): Promise { + const sourceMode = (await fs.stat(zipPath)).mode & 0o777; + const entries: Zippable = await unzipAsync(await fs.readFile(zipPath)); + + for (const [name, data] of logFiles) { + entries[name] = data; + } + + await fs.writeFile(outputPath, await zipAsync(entries)); + await fs.chmod(outputPath, sourceMode); +} + +/** + * Best-effort: append VS Code logs to a support bundle zip. + * Uses atomic rename to avoid corrupting the original bundle on failure. + */ +export async function appendVsCodeLogs( + zipPath: string, + sources: LogSources, + logger: Logger, +): Promise { + try { + const logFiles = await collectVsCodeDiagnostics(sources, logger); + if (logFiles.size === 0) { + logger.info("No VS Code logs found to add to support bundle"); + return; + } + + logger.info( + `Adding ${logFiles.size} VS Code log file(s) to support bundle`, + ); + + const outputBundlePath = vscodeBundlePath(zipPath); + try { + await writeBundleWithLogs(zipPath, outputBundlePath, logFiles); + } catch (error) { + logger.error("Failed to add VS Code logs to support bundle", error); + + try { + await fs.rm(outputBundlePath, { force: true }); + } catch (cleanupError) { + logger.warn( + `Could not clean up partial bundle at ${outputBundlePath}`, + cleanupError, + ); + } + return; + } + + try { + await renameWithRetry(fs.rename, outputBundlePath, zipPath); + } catch (error) { + logger.warn( + `Could not replace original bundle; VS Code logs saved separately at ${outputBundlePath}`, + error, + ); + } + } catch (error) { + // Best-effort: never let a failure here lose the user's bundle. + logger.error("Unexpected error appending VS Code logs", error); + } +} diff --git a/src/supportBundle/diagnostics.ts b/src/supportBundle/diagnostics.ts new file mode 100644 index 0000000000..c76680e61c --- /dev/null +++ b/src/supportBundle/diagnostics.ts @@ -0,0 +1,18 @@ +import { type Logger } from "../logging/logger"; + +import { collectSupportLogFiles, type LogSources } from "./logFiles"; +import { collectSettingsFile } from "./settings"; + +export type { LogSources } from "./logFiles"; + +export async function collectVsCodeDiagnostics( + sources: LogSources, + logger: Logger, +): Promise> { + const files = await collectSupportLogFiles(sources, logger); + const settings = collectSettingsFile(logger); + if (settings) { + files.set("vscode-logs/settings.json", settings); + } + return files; +} diff --git a/src/supportBundle/files.ts b/src/supportBundle/files.ts new file mode 100644 index 0000000000..e8b4625168 --- /dev/null +++ b/src/supportBundle/files.ts @@ -0,0 +1,155 @@ +import { type Dirent } from "node:fs"; +import * as fs from "node:fs/promises"; +import * as path from "node:path"; + +import { type Logger } from "../logging/logger"; + +export interface CollectedFile { + data: Uint8Array; + mtimeMs: number; + relativePath: string; +} + +// 3 days is enough context for recent issues; matching the 7-day +// rotation would bloat the bundle. +const LOG_MAX_AGE_MS = 3 * 24 * 60 * 60 * 1000; +const MAX_LOG_SCAN_DEPTH = 6; + +export const isLogFile = (name: string): boolean => name.endsWith(".log"); + +export function normalizeZipPath(filePath: string): string { + return filePath.replaceAll(path.sep, "/"); +} + +export function addFiles( + target: Map, + source: Map, +): void { + for (const [name, data] of source) { + target.set(name, data); + } +} + +export function prefixFiles( + prefix: string, + files: Map, +): Map { + return new Map([...files].map(([name, data]) => [`${prefix}/${name}`, data])); +} + +function isObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function isNotFoundError(error: unknown): boolean { + return isObject(error) && error["code"] === "ENOENT"; +} + +function cutoffTime(): number { + return Date.now() - LOG_MAX_AGE_MS; +} + +async function readRecentFile( + filePath: string, + logger: Logger, +): Promise<{ data: Uint8Array; mtimeMs: number } | undefined> { + try { + const stat = await fs.stat(filePath); + if (!stat.isFile() || stat.mtimeMs < cutoffTime()) { + return undefined; + } + return { data: await fs.readFile(filePath), mtimeMs: stat.mtimeMs }; + } catch (error) { + logger.warn(`Could not read log file ${filePath}`, error); + return undefined; + } +} + +async function readDir( + dirPath: string, + logger: Logger, + options: { withFileTypes: true; warnOnMissing?: boolean }, +): Promise; +async function readDir( + dirPath: string, + logger: Logger, + options?: { warnOnMissing?: boolean }, +): Promise; +async function readDir( + dirPath: string, + logger: Logger, + options: { withFileTypes?: boolean; warnOnMissing?: boolean } = {}, +): Promise { + try { + return options.withFileTypes + ? await fs.readdir(dirPath, { withFileTypes: true }) + : await fs.readdir(dirPath); + } catch (error) { + if (options.warnOnMissing !== false || !isNotFoundError(error)) { + logger.warn(`Could not read log directory ${dirPath}`, error); + } + return []; + } +} + +export async function collectDirFiles( + dirPath: string, + logger: Logger, + filter: (name: string) => boolean = () => true, + warnOnMissing = true, +): Promise> { + const results = new Map(); + const entries = await readDir(dirPath, logger, { warnOnMissing }); + + await Promise.all( + entries.map(async (entry) => { + if (!filter(entry)) { + return; + } + + const file = await readRecentFile(path.join(dirPath, entry), logger); + if (file) { + results.set(entry, file.data); + } + }), + ); + + return results; +} + +export async function collectMatchingFiles( + rootPath: string, + logger: Logger, + matches: (relativePath: string, fileName: string) => boolean, +): Promise { + const results: CollectedFile[] = []; + + async function walk(dirPath: string, depth: number): Promise { + const entries = await readDir(dirPath, logger, { withFileTypes: true }); + await Promise.all( + entries.map(async (entry) => { + const entryPath = path.join(dirPath, entry.name); + + if (entry.isDirectory()) { + if (depth < MAX_LOG_SCAN_DEPTH) { + await walk(entryPath, depth + 1); + } + return; + } + + const relativePath = path.relative(rootPath, entryPath); + if (!entry.isFile() || !matches(relativePath, entry.name)) { + return; + } + + const file = await readRecentFile(entryPath, logger); + if (file) { + results.push({ ...file, relativePath }); + } + }), + ); + } + + await walk(rootPath, 0); + return results; +} diff --git a/src/supportBundle/logFiles.ts b/src/supportBundle/logFiles.ts new file mode 100644 index 0000000000..ad1e70fba8 --- /dev/null +++ b/src/supportBundle/logFiles.ts @@ -0,0 +1,214 @@ +import * as fs from "node:fs/promises"; +import * as path from "node:path"; + +import { type Logger } from "../logging/logger"; +import { REMOTE_SSH_EXTENSION_IDS } from "../remote/sshExtension"; + +import { + addFiles, + collectDirFiles, + collectMatchingFiles, + type CollectedFile, + isLogFile, + normalizeZipPath, + prefixFiles, +} from "./files"; +import { collectWindowLogDirs, resolveLogContext } from "./vscodeLogs"; + +export interface LogSources { + activeProxyLogPath?: string; + proxyLogDir?: string; + extensionLogDir?: string; +} + +const isProxyLogFile = (name: string): boolean => + name.startsWith("coder-ssh") && isLogFile(name); + +async function collectCurrentExtensionLogs( + extensionLogDir: string, + logger: Logger, +): Promise> { + return prefixFiles( + "vscode-logs/extension", + await collectDirFiles(extensionLogDir, logger, isLogFile), + ); +} + +async function collectWindowExtensionLogs( + windowPath: string, + windowRelativePath: string, + extensionId: string, + logger: Logger, +): Promise> { + return prefixFiles( + `vscode-logs/extension/${normalizeZipPath(windowRelativePath)}`, + await collectDirFiles( + path.join(windowPath, "exthost", extensionId), + logger, + isLogFile, + false, + ), + ); +} + +async function collectExtensionLogs( + extensionLogDir: string, + logger: Logger, +): Promise> { + const context = resolveLogContext(extensionLogDir); + if (!context) { + return collectCurrentExtensionLogs(extensionLogDir, logger); + } + + const files = new Map(); + const extensionId = path.basename(extensionLogDir); + for (const windowLogDir of await collectWindowLogDirs( + context.logsRoot, + logger, + )) { + addFiles( + files, + await collectWindowExtensionLogs( + windowLogDir.windowPath, + windowLogDir.relativePath, + extensionId, + logger, + ), + ); + } + + return files.size > 0 + ? files + : collectCurrentExtensionLogs(extensionLogDir, logger); +} + +function isRemoteSshLog(relativePath: string, fileName: string): boolean { + if (!fileName.includes("Remote - SSH") || !isLogFile(fileName)) { + return false; + } + + const parts = normalizeZipPath(relativePath).split("/"); + return parts.some( + (part) => + part.startsWith("output_logging_") || + (REMOTE_SSH_EXTENSION_IDS as readonly string[]).includes(part), + ); +} + +function newestLog(logs: CollectedFile[]): CollectedFile | undefined { + return logs.toSorted( + (a, b) => + b.mtimeMs - a.mtimeMs || b.relativePath.localeCompare(a.relativePath), + )[0]; +} + +async function collectRemoteSshLogs( + extensionLogDir: string, + logger: Logger, +): Promise> { + const context = resolveLogContext(extensionLogDir); + const files = new Map(); + if (!context) { + return files; + } + + const remoteSshLogs: CollectedFile[] = []; + const extensionId = path.basename(extensionLogDir); + for (const windowLogDir of await collectWindowLogDirs( + context.logsRoot, + logger, + )) { + if ( + ( + await collectWindowExtensionLogs( + windowLogDir.windowPath, + windowLogDir.relativePath, + extensionId, + logger, + ) + ).size === 0 + ) { + continue; + } + + for (const logFile of await collectMatchingFiles( + windowLogDir.windowPath, + logger, + isRemoteSshLog, + )) { + const relativePath = normalizeZipPath( + path.join(windowLogDir.relativePath, logFile.relativePath), + ); + remoteSshLogs.push({ ...logFile, relativePath }); + files.set(`vscode-logs/remote-ssh/${relativePath}`, logFile.data); + } + } + + const currentWindowRelativePath = normalizeZipPath( + path.relative(context.logsRoot, context.currentWindowPath), + ); + const currentWindowLogs = remoteSshLogs.filter((logFile) => + logFile.relativePath.startsWith(`${currentWindowRelativePath}/`), + ); + const activeLog = newestLog( + currentWindowLogs.length > 0 ? currentWindowLogs : remoteSshLogs, + ); + if (activeLog) { + files.set( + `vscode-logs/remote-ssh/${path.basename(activeLog.relativePath)}`, + activeLog.data, + ); + } + + return files; +} + +async function collectProxyLogs( + sources: LogSources, + logger: Logger, +): Promise> { + const files = new Map(); + + if (sources.activeProxyLogPath) { + try { + files.set( + "vscode-logs/proxy/active.log", + await fs.readFile(sources.activeProxyLogPath), + ); + } catch (error) { + logger.warn("Could not read active Coder SSH proxy log", error); + } + } + + if (sources.proxyLogDir) { + addFiles( + files, + prefixFiles( + "vscode-logs/proxy", + await collectDirFiles(sources.proxyLogDir, logger, isProxyLogFile), + ), + ); + } + + return files; +} + +export async function collectSupportLogFiles( + sources: LogSources, + logger: Logger, +): Promise> { + const files = await collectProxyLogs(sources, logger); + + if (sources.extensionLogDir) { + addFiles( + files, + await collectExtensionLogs(sources.extensionLogDir, logger), + ); + addFiles( + files, + await collectRemoteSshLogs(sources.extensionLogDir, logger), + ); + } + + return files; +} diff --git a/src/supportBundle/settings.ts b/src/supportBundle/settings.ts new file mode 100644 index 0000000000..4b8cdfacd9 --- /dev/null +++ b/src/supportBundle/settings.ts @@ -0,0 +1,152 @@ +import * as vscode from "vscode"; + +import { type Logger } from "../logging/logger"; +import { REMOTE_SSH_EXTENSION_IDS } from "../remote/sshExtension"; + +interface ConfigurationContribution { + properties?: unknown; +} + +interface ExtensionPackageJson { + contributes?: unknown; + name?: unknown; + publisher?: unknown; +} + +type SettingValue = unknown; +type SettingInspection = Record; +type SettingDiagnostics = Record; + +const REDACTED_SETTINGS = new Set([ + "coder.binarySource", + "coder.globalFlags", + "coder.headerCommand", + "coder.sshConfig", + "coder.sshFlags", + "coder.tlsCertRefreshCommand", +]); + +const REMOTE_SETTINGS = new Set([ + "remote.SSH.connectTimeout", + "remote.SSH.logLevel", + "remote.SSH.reconnectionGraceTime", + "remote.SSH.serverShutdownTimeout", + "remote.SSH.useExecServer", + "remote.SSH.useLocalServer", + "remote.autoForwardPorts", +]); + +function isObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function readPackageJson(value: unknown): ExtensionPackageJson | undefined { + return isObject(value) ? value : undefined; +} + +function configurationContributions( + packageJson: ExtensionPackageJson, +): ConfigurationContribution[] { + if (!isObject(packageJson.contributes)) { + return []; + } + + const configuration = packageJson.contributes["configuration"]; + const contributions = Array.isArray(configuration) + ? configuration + : [configuration]; + return contributions.filter(isObject); +} + +function isCoderExtension(extension: vscode.Extension): boolean { + const packageJson = readPackageJson(extension.packageJSON); + return ( + extension.id === "coder.coder-remote" || + (packageJson?.publisher === "coder" && packageJson.name === "coder-remote") + ); +} + +function isRemoteSshExtension(extension: vscode.Extension): boolean { + return (REMOTE_SSH_EXTENSION_IDS as readonly string[]).includes(extension.id); +} + +function shouldCollectKey( + extension: vscode.Extension, + key: string, +): boolean { + return ( + (isCoderExtension(extension) && key.startsWith("coder.")) || + (isRemoteSshExtension(extension) && REMOTE_SETTINGS.has(key)) + ); +} + +function configurationKeys(): string[] { + const keys = new Set(); + + for (const extension of vscode.extensions.all) { + const packageJson = readPackageJson(extension.packageJSON); + if (!packageJson) { + continue; + } + + for (const contribution of configurationContributions(packageJson)) { + if (!isObject(contribution.properties)) { + continue; + } + + for (const key of Object.keys(contribution.properties)) { + if (shouldCollectKey(extension, key)) { + keys.add(key); + } + } + } + } + + return [...keys].sort(); +} + +function redactedSettingValue(value: SettingValue): string { + const emptyArray = Array.isArray(value) && value.length === 0; + return value === undefined || value === null || value === "" || emptyArray + ? "" + : ""; +} + +function maybeRedactSetting(key: string, value: SettingValue): SettingValue { + return REDACTED_SETTINGS.has(key) ? redactedSettingValue(value) : value; +} + +function collectSettingsDiagnostics(): SettingDiagnostics { + const config = vscode.workspace.getConfiguration(); + const diagnostics: SettingDiagnostics = {}; + + for (const key of configurationKeys()) { + const inspected = config.inspect(key); + if (!inspected) { + continue; + } + + const entry: SettingInspection = { + effective: maybeRedactSetting(key, config.get(key)), + }; + for (const [name, value] of Object.entries(inspected)) { + entry[name] = name === "key" ? value : maybeRedactSetting(key, value); + } + diagnostics[key] = entry; + } + + return diagnostics; +} + +export function collectSettingsFile(logger: Logger): Uint8Array | undefined { + try { + const diagnostics = collectSettingsDiagnostics(); + if (Object.keys(diagnostics).length === 0) { + return undefined; + } + return Buffer.from(`${JSON.stringify(diagnostics, null, "\t")}\n`); + } catch (error) { + logger.warn("Could not collect VS Code settings", error); + return undefined; + } +} diff --git a/src/supportBundle/vscodeLogs.ts b/src/supportBundle/vscodeLogs.ts new file mode 100644 index 0000000000..052f79d160 --- /dev/null +++ b/src/supportBundle/vscodeLogs.ts @@ -0,0 +1,86 @@ +import { type Dirent } from "node:fs"; +import * as fs from "node:fs/promises"; +import * as path from "node:path"; + +import { type Logger } from "../logging/logger"; + +interface WindowLogDir { + relativePath: string; + windowPath: string; +} + +export interface LogContext { + currentWindowPath: string; + logsRoot: string; +} + +export function resolveLogContext( + extensionLogDir: string, +): LogContext | undefined { + const resolved = path.resolve(extensionLogDir); + const extensionId = path.basename(resolved); + const exthostDir = path.dirname(resolved); + const windowDir = path.dirname(exthostDir); + const windowName = path.basename(windowDir); + const sessionDir = path.dirname(windowDir); + + if ( + extensionId !== "coder.coder-remote" || + path.basename(exthostDir) !== "exthost" || + !/^window\d+$/i.test(windowName) + ) { + return undefined; + } + + const sessionName = path.basename(sessionDir); + return { + currentWindowPath: windowDir, + logsRoot: /^\d{8}T\d{6}/.test(sessionName) + ? path.dirname(sessionDir) + : sessionDir, + }; +} + +async function readDirents(dirPath: string, logger: Logger): Promise { + try { + return await fs.readdir(dirPath, { withFileTypes: true }); + } catch (error) { + logger.warn(`Could not read log directory ${dirPath}`, error); + return []; + } +} + +export async function collectWindowLogDirs( + logsRoot: string, + logger: Logger, +): Promise { + const windows: WindowLogDir[] = []; + const rootEntries = await readDirents(logsRoot, logger); + + await Promise.all( + rootEntries.map(async (entry) => { + if (!entry.isDirectory()) { + return; + } + + const entryPath = path.join(logsRoot, entry.name); + if (/^window\d+$/i.test(entry.name)) { + windows.push({ relativePath: entry.name, windowPath: entryPath }); + return; + } + + const sessionEntries = await readDirents(entryPath, logger); + for (const windowEntry of sessionEntries.filter( + (sessionEntry) => + sessionEntry.isDirectory() && /^window\d+$/i.test(sessionEntry.name), + )) { + windows.push({ + relativePath: `${entry.name}/${windowEntry.name}`, + windowPath: path.join(entryPath, windowEntry.name), + }); + } + }), + ); + + return windows.sort((a, b) => a.relativePath.localeCompare(b.relativePath)); +} diff --git a/test/unit/core/supportBundleLogs.test.ts b/test/unit/core/supportBundleLogs.test.ts deleted file mode 100644 index e942e40e4f..0000000000 --- a/test/unit/core/supportBundleLogs.test.ts +++ /dev/null @@ -1,254 +0,0 @@ -import { strToU8, unzipSync, zipSync } from "fflate"; -import * as fs from "node:fs/promises"; -import * as os from "node:os"; -import * as path from "node:path"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; - -import { appendVsCodeLogs } from "@/core/supportBundleLogs"; -import { renameWithRetry } from "@/util/fs"; - -import { createMockLogger } from "../../mocks/testHelpers"; - -// Wrap renameWithRetry so individual tests can override it via -// mockRejectedValueOnce; by default it calls through to the real impl. -vi.mock("@/util/fs", async () => { - const actual = await vi.importActual("@/util/fs"); - return { ...actual, renameWithRetry: vi.fn(actual.renameWithRetry) }; -}); - -// chmod to 0o000 is a no-op as root and on Windows. -const canTestUnreadable = - process.getuid?.() !== 0 && process.platform !== "win32"; - -let tmpDir: string; - -beforeEach(async () => { - tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "support-bundle-")); -}); - -afterEach(async () => { - await fs.rm(tmpDir, { recursive: true, force: true }); -}); - -const logger = createMockLogger(); - -/** Set a file's mtime to N days in the past. */ -async function setAge(filePath: string, daysAgo: number): Promise { - const past = new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000); - await fs.utimes(filePath, past, past); -} - -async function makeBundle(): Promise { - const zipPath = path.join(tmpDir, "coder-support-123.zip"); - await fs.writeFile( - zipPath, - zipSync({ "server/info.txt": strToU8("server data") }), - ); - return zipPath; -} - -async function readZip(zipPath: string): Promise> { - const entries = unzipSync(await fs.readFile(zipPath)); - return Object.fromEntries( - Object.entries(entries).map(([name, data]) => [ - name, - Buffer.from(data).toString(), - ]), - ); -} - -function vsCodeLogKeys(entries: Record): string[] { - return Object.keys(entries) - .filter((k) => k.startsWith("vscode-logs/")) - .sort(); -} - -describe("appendVsCodeLogs", () => { - it("merges logs from all three sources and skips subdirectories", async () => { - const zipPath = await makeBundle(); - - const sshLog = path.join(tmpDir, "ssh.log"); - await fs.writeFile(sshLog, "ssh"); - - const proxyDir = path.join(tmpDir, "proxy"); - await fs.mkdir(proxyDir); - await fs.writeFile(path.join(proxyDir, "coder-ssh-recent.log"), "proxy"); - await fs.mkdir(path.join(proxyDir, "subdir")); - - const extDir = path.join(tmpDir, "ext"); - await fs.mkdir(extDir); - await fs.writeFile(path.join(extDir, "Coder.log"), "ext"); - - await appendVsCodeLogs( - zipPath, - { - remoteSshLogPath: sshLog, - proxyLogDir: proxyDir, - extensionLogDir: extDir, - }, - logger, - ); - - const entries = await readZip(zipPath); - expect(Object.keys(entries).sort()).toEqual([ - "server/info.txt", - "vscode-logs/extension/Coder.log", - "vscode-logs/proxy/coder-ssh-recent.log", - "vscode-logs/remote-ssh/ssh.log", - ]); - expect(entries["server/info.txt"]).toBe("server data"); - expect(entries["vscode-logs/proxy/coder-ssh-recent.log"]).toBe("proxy"); - }); - - it("does not touch the zip when no logs are found", async () => { - const zipPath = await makeBundle(); - const beforeStat = await fs.stat(zipPath); - const beforeBytes = await fs.readFile(zipPath); - - await appendVsCodeLogs(zipPath, {}, logger); - - expect((await fs.stat(zipPath)).mtimeMs).toBe(beforeStat.mtimeMs); - expect(Buffer.compare(beforeBytes, await fs.readFile(zipPath))).toBe(0); - }); - - it("merges a large number of files without dropping any", async () => { - const zipPath = await makeBundle(); - - const proxyDir = path.join(tmpDir, "proxy"); - const extDir = path.join(tmpDir, "ext"); - await fs.mkdir(proxyDir); - await fs.mkdir(extDir); - - const fileCount = 60; - await Promise.all( - Array.from({ length: fileCount }, (_, i) => - Promise.all([ - fs.writeFile(path.join(proxyDir, `proxy-${i}.log`), `proxy-${i}`), - fs.writeFile(path.join(extDir, `ext-${i}.log`), `ext-${i}`), - ]), - ), - ); - - await appendVsCodeLogs( - zipPath, - { proxyLogDir: proxyDir, extensionLogDir: extDir }, - logger, - ); - - const entries = await readZip(zipPath); - const keys = vsCodeLogKeys(entries); - expect(keys).toHaveLength(fileCount * 2); - for (let i = 0; i < fileCount; i++) { - expect(entries[`vscode-logs/proxy/proxy-${i}.log`]).toBe(`proxy-${i}`); - expect(entries[`vscode-logs/extension/ext-${i}.log`]).toBe(`ext-${i}`); - } - }); - - it("filters proxy logs older than 3 days by mtime", async () => { - const zipPath = await makeBundle(); - - const proxyDir = path.join(tmpDir, "proxy"); - await fs.mkdir(proxyDir); - await fs.writeFile(path.join(proxyDir, "recent.log"), "recent"); - await fs.writeFile(path.join(proxyDir, "old.log"), "old"); - await setAge(path.join(proxyDir, "old.log"), 5); - - await appendVsCodeLogs(zipPath, { proxyLogDir: proxyDir }, logger); - - expect(vsCodeLogKeys(await readZip(zipPath))).toEqual([ - "vscode-logs/proxy/recent.log", - ]); - }); - - it("filters extension logs older than 3 days by mtime", async () => { - const zipPath = await makeBundle(); - - const extDir = path.join(tmpDir, "ext"); - await fs.mkdir(extDir); - await fs.writeFile(path.join(extDir, "Coder-recent.log"), "recent"); - await fs.writeFile(path.join(extDir, "Coder-old.log"), "old"); - await setAge(path.join(extDir, "Coder-old.log"), 5); - - await appendVsCodeLogs(zipPath, { extensionLogDir: extDir }, logger); - - expect(vsCodeLogKeys(await readZip(zipPath))).toEqual([ - "vscode-logs/extension/Coder-recent.log", - ]); - }); - - it.runIf(canTestUnreadable)( - "skips missing or unreadable sources and includes the rest", - async () => { - const zipPath = await makeBundle(); - - const proxyDir = path.join(tmpDir, "proxy"); - await fs.mkdir(proxyDir); - await fs.writeFile(path.join(proxyDir, "good.log"), "ok"); - const badLog = path.join(proxyDir, "bad.log"); - await fs.writeFile(badLog, "secret"); - await fs.chmod(badLog, 0o000); - - try { - await appendVsCodeLogs( - zipPath, - { - remoteSshLogPath: path.join(tmpDir, "nonexistent.log"), - proxyLogDir: proxyDir, - extensionLogDir: path.join(tmpDir, "no-such-dir"), - }, - logger, - ); - - expect(vsCodeLogKeys(await readZip(zipPath))).toEqual([ - "vscode-logs/proxy/good.log", - ]); - } finally { - await fs.chmod(badLog, 0o644); - } - }, - ); - - it("keeps the -vscode.zip sibling when rename fails", async () => { - const zipPath = await makeBundle(); - const beforeStat = await fs.stat(zipPath); - const beforeBytes = await fs.readFile(zipPath); - - const sshLog = path.join(tmpDir, "ssh.log"); - await fs.writeFile(sshLog, "ssh content"); - - vi.mocked(renameWithRetry).mockRejectedValueOnce( - new Error("simulated rename failure"), - ); - - await appendVsCodeLogs(zipPath, { remoteSshLogPath: sshLog }, logger); - - expect((await fs.stat(zipPath)).mtimeMs).toBe(beforeStat.mtimeMs); - expect(Buffer.compare(beforeBytes, await fs.readFile(zipPath))).toBe(0); - - const siblingPath = path.join(tmpDir, "coder-support-123-vscode.zip"); - const entries = await readZip(siblingPath); - expect(Object.keys(entries).sort()).toEqual([ - "server/info.txt", - "vscode-logs/remote-ssh/ssh.log", - ]); - expect(entries["vscode-logs/remote-ssh/ssh.log"]).toBe("ssh content"); - }); - - it("leaves the original zip intact and cleans up the partial sibling when corrupted", async () => { - const zipPath = path.join(tmpDir, "coder-support-123.zip"); - await fs.writeFile(zipPath, "not a zip"); - const beforeStat = await fs.stat(zipPath); - const beforeBytes = await fs.readFile(zipPath); - - const logPath = path.join(tmpDir, "ssh.log"); - await fs.writeFile(logPath, "content"); - - await appendVsCodeLogs(zipPath, { remoteSshLogPath: logPath }, logger); - - expect((await fs.stat(zipPath)).mtimeMs).toBe(beforeStat.mtimeMs); - expect(Buffer.compare(beforeBytes, await fs.readFile(zipPath))).toBe(0); - expect(await fs.readdir(tmpDir)).not.toContain( - "coder-support-123-vscode.zip", - ); - }); -}); diff --git a/test/unit/supportBundle/appendVsCodeLogs.test.ts b/test/unit/supportBundle/appendVsCodeLogs.test.ts new file mode 100644 index 0000000000..11ebb7fd4f --- /dev/null +++ b/test/unit/supportBundle/appendVsCodeLogs.test.ts @@ -0,0 +1,178 @@ +import { strToU8, unzipSync, zipSync } from "fflate"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { appendVsCodeLogs } from "@/supportBundle/appendVsCodeLogs"; +import { collectVsCodeDiagnostics } from "@/supportBundle/diagnostics"; +import { renameWithRetry } from "@/util/fs"; + +import { createMockLogger } from "../../mocks/testHelpers"; + +const collectVsCodeDiagnosticsMock = vi.hoisted(() => vi.fn()); + +vi.mock("@/supportBundle/diagnostics", () => ({ + collectVsCodeDiagnostics: collectVsCodeDiagnosticsMock, +})); + +// Wrap renameWithRetry so individual tests can override it via +// mockRejectedValueOnce; by default it calls through to the real impl. +vi.mock("@/util/fs", async () => { + const actual = await vi.importActual("@/util/fs"); + return { ...actual, renameWithRetry: vi.fn(actual.renameWithRetry) }; +}); + +const canTestFileMode = process.platform !== "win32"; +let tmpDir: string; +const logger = createMockLogger(); + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "support-bundle-")); + vi.mocked(collectVsCodeDiagnostics).mockReset(); + vi.mocked(renameWithRetry).mockClear(); +}); + +afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +async function makeBundle(): Promise { + const zipPath = path.join(tmpDir, "coder-support-123.zip"); + await fs.writeFile( + zipPath, + zipSync({ "server/info.txt": strToU8("server data") }), + ); + return zipPath; +} + +async function readZip(zipPath: string): Promise> { + const entries = unzipSync(await fs.readFile(zipPath)); + return Object.fromEntries( + Object.entries(entries).map(([name, data]) => [ + name, + Buffer.from(data).toString(), + ]), + ); +} + +async function findBundleSibling(): Promise { + const sibling = (await fs.readdir(tmpDir)).find( + (name) => + name.startsWith("coder-support-123-vscode-") && name.endsWith(".zip"), + ); + if (!sibling) { + throw new Error("support bundle sibling not found"); + } + return path.join(tmpDir, sibling); +} + +describe("appendVsCodeLogs", () => { + it("adds collected diagnostics to the support bundle", async () => { + const zipPath = await makeBundle(); + vi.mocked(collectVsCodeDiagnostics).mockResolvedValue( + new Map([ + ["vscode-logs/settings.json", Buffer.from("settings")], + ["vscode-logs/proxy/active.log", Buffer.from("proxy")], + ]), + ); + + await appendVsCodeLogs(zipPath, {}, logger); + + expect(await readZip(zipPath)).toEqual({ + "server/info.txt": "server data", + "vscode-logs/proxy/active.log": "proxy", + "vscode-logs/settings.json": "settings", + }); + }); + + it("does not rewrite the bundle when no diagnostics are found", async () => { + const zipPath = await makeBundle(); + const beforeBytes = await fs.readFile(zipPath); + vi.mocked(collectVsCodeDiagnostics).mockResolvedValue(new Map()); + + await appendVsCodeLogs(zipPath, {}, logger); + + expect(Buffer.compare(beforeBytes, await fs.readFile(zipPath))).toBe(0); + expect(vi.mocked(renameWithRetry)).not.toHaveBeenCalled(); + }); + + it("keeps a VS Code bundle sibling when replacing the original fails", async () => { + const zipPath = await makeBundle(); + const beforeBytes = await fs.readFile(zipPath); + vi.mocked(collectVsCodeDiagnostics).mockResolvedValue( + new Map([["vscode-logs/proxy/active.log", Buffer.from("proxy")]]), + ); + vi.mocked(renameWithRetry).mockRejectedValueOnce( + new Error("simulated rename failure"), + ); + + await appendVsCodeLogs(zipPath, {}, logger); + + expect(Buffer.compare(beforeBytes, await fs.readFile(zipPath))).toBe(0); + expect(await readZip(await findBundleSibling())).toEqual({ + "server/info.txt": "server data", + "vscode-logs/proxy/active.log": "proxy", + }); + }); + + it.runIf(canTestFileMode)( + "preserves bundle permissions when replacing the original fails", + async () => { + const zipPath = await makeBundle(); + await fs.chmod(zipPath, 0o600); + vi.mocked(collectVsCodeDiagnostics).mockResolvedValue( + new Map([["vscode-logs/proxy/active.log", Buffer.from("proxy")]]), + ); + vi.mocked(renameWithRetry).mockRejectedValueOnce( + new Error("simulated rename failure"), + ); + + await appendVsCodeLogs(zipPath, {}, logger); + + expect((await fs.stat(await findBundleSibling())).mode & 0o777).toBe( + 0o600, + ); + }, + ); + + it("leaves the original zip and existing sibling intact when the bundle cannot be read", async () => { + const zipPath = path.join(tmpDir, "coder-support-123.zip"); + await fs.writeFile(zipPath, "not a zip"); + const existingSiblingPath = path.join( + tmpDir, + "coder-support-123-vscode.zip", + ); + await fs.writeFile(existingSiblingPath, "existing"); + const beforeBytes = await fs.readFile(zipPath); + vi.mocked(collectVsCodeDiagnostics).mockResolvedValue( + new Map([["vscode-logs/proxy/active.log", Buffer.from("proxy")]]), + ); + + await appendVsCodeLogs(zipPath, {}, logger); + + expect(Buffer.compare(beforeBytes, await fs.readFile(zipPath))).toBe(0); + expect(await fs.readFile(existingSiblingPath, "utf8")).toBe("existing"); + expect( + (await fs.readdir(tmpDir)).filter((name) => + name.startsWith("coder-support-123-vscode-"), + ), + ).toEqual([]); + }); + + it("leaves the bundle unchanged when diagnostics collection fails", async () => { + const zipPath = await makeBundle(); + const beforeBytes = await fs.readFile(zipPath); + vi.mocked(collectVsCodeDiagnostics).mockRejectedValueOnce( + new Error("diagnostics failed"), + ); + + await appendVsCodeLogs(zipPath, {}, logger); + + expect(Buffer.compare(beforeBytes, await fs.readFile(zipPath))).toBe(0); + expect(logger.error).toHaveBeenCalledWith( + "Unexpected error appending VS Code logs", + expect.any(Error), + ); + }); +}); diff --git a/test/unit/supportBundle/diagnostics.test.ts b/test/unit/supportBundle/diagnostics.test.ts new file mode 100644 index 0000000000..fc1ea90c18 --- /dev/null +++ b/test/unit/supportBundle/diagnostics.test.ts @@ -0,0 +1,50 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { collectVsCodeDiagnostics } from "@/supportBundle/diagnostics"; +import { collectSupportLogFiles } from "@/supportBundle/logFiles"; +import { collectSettingsFile } from "@/supportBundle/settings"; + +import { createMockLogger } from "../../mocks/testHelpers"; + +const collectSupportLogFilesMock = vi.hoisted(() => vi.fn()); +const collectSettingsFileMock = vi.hoisted(() => vi.fn()); + +vi.mock("@/supportBundle/logFiles", () => ({ + collectSupportLogFiles: collectSupportLogFilesMock, +})); + +vi.mock("@/supportBundle/settings", () => ({ + collectSettingsFile: collectSettingsFileMock, +})); + +const logger = createMockLogger(); + +beforeEach(() => { + vi.mocked(collectSupportLogFiles).mockReset(); + vi.mocked(collectSettingsFile).mockReset(); +}); + +describe("collectVsCodeDiagnostics", () => { + it("combines log files and settings", async () => { + vi.mocked(collectSupportLogFiles).mockResolvedValue( + new Map([["vscode-logs/proxy/active.log", Buffer.from("proxy")]]), + ); + vi.mocked(collectSettingsFile).mockReturnValue(Buffer.from("settings")); + + await expect(collectVsCodeDiagnostics({}, logger)).resolves.toEqual( + new Map([ + ["vscode-logs/proxy/active.log", Buffer.from("proxy")], + ["vscode-logs/settings.json", Buffer.from("settings")], + ]), + ); + }); + + it("does not add settings when none are collected", async () => { + vi.mocked(collectSupportLogFiles).mockResolvedValue(new Map()); + vi.mocked(collectSettingsFile).mockReturnValue(undefined); + + await expect(collectVsCodeDiagnostics({}, logger)).resolves.toEqual( + new Map(), + ); + }); +}); diff --git a/test/unit/supportBundle/files.test.ts b/test/unit/supportBundle/files.test.ts new file mode 100644 index 0000000000..f400303d68 --- /dev/null +++ b/test/unit/supportBundle/files.test.ts @@ -0,0 +1,87 @@ +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { + addFiles, + collectDirFiles, + collectMatchingFiles, + isLogFile, + normalizeZipPath, + prefixFiles, +} from "@/supportBundle/files"; + +import { createMockLogger } from "../../mocks/testHelpers"; + +let tmpDir: string; +const logger = createMockLogger(); + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "support-bundle-files-")); +}); + +afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +async function setAge(filePath: string, daysAgo: number): Promise { + const past = new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000); + await fs.utimes(filePath, past, past); +} + +describe("support bundle file helpers", () => { + it("normalizes zip paths and identifies log files", () => { + expect(normalizeZipPath("a/b/c.log")).toBe("a/b/c.log"); + expect(isLogFile("Coder.log")).toBe(true); + expect(isLogFile("settings.json")).toBe(false); + }); + + it("prefixes and merges file maps", () => { + const target = new Map([["existing", Buffer.from("old")]]); + addFiles( + target, + prefixFiles("vscode-logs/proxy", new Map([["a.log", Buffer.from("a")]])), + ); + + expect([...target.keys()].sort()).toEqual([ + "existing", + "vscode-logs/proxy/a.log", + ]); + }); + + it("collects recent matching files and skips old files and subdirectories", async () => { + await fs.writeFile(path.join(tmpDir, "recent.log"), "recent"); + await fs.writeFile(path.join(tmpDir, "old.log"), "old"); + await fs.writeFile(path.join(tmpDir, "notes.txt"), "notes"); + await fs.mkdir(path.join(tmpDir, "subdir")); + await setAge(path.join(tmpDir, "old.log"), 5); + + const files = await collectDirFiles(tmpDir, logger, isLogFile); + + expect(files).toEqual(new Map([["recent.log", Buffer.from("recent")]])); + }); + + it("walks matching recent files recursively", async () => { + const nested = path.join(tmpDir, "window1", "output_logging_1"); + await fs.mkdir(nested, { recursive: true }); + await fs.writeFile(path.join(nested, "1-Remote - SSH.log"), "ssh"); + + const files = await collectMatchingFiles( + tmpDir, + logger, + (_relativePath, fileName) => fileName.includes("Remote - SSH"), + ); + + expect(files).toMatchObject([ + { + data: Buffer.from("ssh"), + relativePath: path.join( + "window1", + "output_logging_1", + "1-Remote - SSH.log", + ), + }, + ]); + }); +}); diff --git a/test/unit/supportBundle/logFiles.test.ts b/test/unit/supportBundle/logFiles.test.ts new file mode 100644 index 0000000000..6df22dbc0b --- /dev/null +++ b/test/unit/supportBundle/logFiles.test.ts @@ -0,0 +1,244 @@ +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { collectSupportLogFiles } from "@/supportBundle/logFiles"; + +import { createMockLogger } from "../../mocks/testHelpers"; + +// chmod to 0o000 is a no-op as root and on Windows. +const canTestUnreadable = + process.getuid?.() !== 0 && process.platform !== "win32"; + +let tmpDir: string; +const logger = createMockLogger(); + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "support-bundle-logs-")); +}); + +afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +async function setAge(filePath: string, daysAgo: number): Promise { + const past = new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000); + await fs.utimes(filePath, past, past); +} + +async function collectTextFiles( + sources: Parameters[0], +): Promise> { + const files = await collectSupportLogFiles(sources, logger); + return Object.fromEntries( + [...files].map(([name, data]) => [name, Buffer.from(data).toString()]), + ); +} + +describe("collectSupportLogFiles", () => { + it("collects active proxy log and recent Coder SSH proxy logs", async () => { + const proxyDir = path.join(tmpDir, "proxy"); + await fs.mkdir(proxyDir); + const activeProxyLog = path.join(tmpDir, "custom-active.log"); + await fs.writeFile(activeProxyLog, "active"); + await fs.writeFile(path.join(proxyDir, "coder-ssh-recent.log"), "recent"); + await fs.writeFile(path.join(proxyDir, "coder-ssh-old.log"), "old"); + await fs.writeFile(path.join(proxyDir, "other.log"), "other"); + await fs.writeFile(path.join(proxyDir, "secret.env"), "secret"); + await fs.mkdir(path.join(proxyDir, "subdir")); + await setAge(path.join(proxyDir, "coder-ssh-old.log"), 5); + + await expect( + collectTextFiles({ + activeProxyLogPath: activeProxyLog, + proxyLogDir: proxyDir, + }), + ).resolves.toEqual({ + "vscode-logs/proxy/active.log": "active", + "vscode-logs/proxy/coder-ssh-recent.log": "recent", + }); + }); + + it("collects recent extension logs from a non-canonical extension log directory", async () => { + const extDir = path.join(tmpDir, "ext"); + await fs.mkdir(extDir); + await fs.writeFile(path.join(extDir, "Coder-recent.log"), "recent"); + await fs.writeFile(path.join(extDir, "Coder-old.log"), "old"); + await fs.writeFile(path.join(extDir, "notes.txt"), "notes"); + await fs.mkdir(path.join(extDir, "subdir")); + await setAge(path.join(extDir, "Coder-old.log"), 5); + + await expect( + collectTextFiles({ extensionLogDir: extDir }), + ).resolves.toEqual({ + "vscode-logs/extension/Coder-recent.log": "recent", + }); + }); + + it("collects extension and Remote-SSH logs across recent VS Code sessions", async () => { + const logsRoot = path.join(tmpDir, "logs"); + const currentSession = "20240103T000000"; + const previousSession = "20240102T000000"; + const oldSession = "20231231T000000"; + const window = "window1"; + + const currentExtDir = path.join( + logsRoot, + currentSession, + window, + "exthost", + "coder.coder-remote", + ); + const previousExtDir = path.join( + logsRoot, + previousSession, + window, + "exthost", + "coder.coder-remote", + ); + const oldExtDir = path.join( + logsRoot, + oldSession, + window, + "exthost", + "coder.coder-remote", + ); + await fs.mkdir(currentExtDir, { recursive: true }); + await fs.mkdir(previousExtDir, { recursive: true }); + await fs.mkdir(oldExtDir, { recursive: true }); + await fs.writeFile(path.join(currentExtDir, "Coder.log"), "current"); + await fs.writeFile(path.join(previousExtDir, "Coder.log"), "previous"); + await fs.writeFile(path.join(oldExtDir, "Coder.log"), "old"); + await setAge(path.join(oldExtDir, "Coder.log"), 5); + + const currentRemoteDir = path.join( + logsRoot, + currentSession, + window, + "output_logging_current", + ); + const previousRemoteDir = path.join( + logsRoot, + previousSession, + window, + "output_logging_previous", + ); + const oldRemoteDir = path.join( + logsRoot, + oldSession, + window, + "output_logging_old", + ); + await fs.mkdir(currentRemoteDir, { recursive: true }); + await fs.mkdir(previousRemoteDir, { recursive: true }); + await fs.mkdir(oldRemoteDir, { recursive: true }); + await fs.writeFile( + path.join(currentRemoteDir, "1-Remote - SSH.log"), + "current ssh", + ); + await fs.writeFile( + path.join(previousRemoteDir, "1-Remote - SSH.log"), + "previous ssh", + ); + await fs.writeFile( + path.join(oldRemoteDir, "1-Remote - SSH.log"), + "old ssh", + ); + await setAge(path.join(oldRemoteDir, "1-Remote - SSH.log"), 5); + const future = new Date(Date.now() + 60_000); + await fs.utimes( + path.join(previousRemoteDir, "1-Remote - SSH.log"), + future, + future, + ); + + await expect( + collectTextFiles({ extensionLogDir: currentExtDir }), + ).resolves.toEqual({ + [`vscode-logs/extension/${currentSession}/${window}/Coder.log`]: + "current", + [`vscode-logs/extension/${previousSession}/${window}/Coder.log`]: + "previous", + "vscode-logs/remote-ssh/1-Remote - SSH.log": "current ssh", + [`vscode-logs/remote-ssh/${currentSession}/${window}/output_logging_current/1-Remote - SSH.log`]: + "current ssh", + [`vscode-logs/remote-ssh/${previousSession}/${window}/output_logging_previous/1-Remote - SSH.log`]: + "previous ssh", + }); + }); + + it("collects Remote-SSH logs only for windows with Coder extension logs", async () => { + const logsRoot = path.join(tmpDir, "logs"); + const currentExtDir = path.join( + logsRoot, + "20240101T000000", + "window1", + "exthost", + "coder.coder-remote", + ); + await fs.mkdir(currentExtDir, { recursive: true }); + await fs.writeFile(path.join(currentExtDir, "Coder.log"), "coder"); + const relatedRemoteDir = path.join( + logsRoot, + "20240101T000000", + "window1", + "output_logging_1", + ); + const unrelatedRemoteDir = path.join( + logsRoot, + "20240101T000000", + "window2", + "output_logging_2", + ); + await fs.mkdir(relatedRemoteDir, { recursive: true }); + await fs.mkdir(unrelatedRemoteDir, { recursive: true }); + await fs.writeFile( + path.join(relatedRemoteDir, "1-Remote - SSH.log"), + "related", + ); + await fs.writeFile( + path.join(unrelatedRemoteDir, "1-Remote - SSH.log"), + "unrelated", + ); + + const files = await collectTextFiles({ extensionLogDir: currentExtDir }); + + expect(files).toMatchObject({ + "vscode-logs/remote-ssh/1-Remote - SSH.log": "related", + "vscode-logs/remote-ssh/20240101T000000/window1/output_logging_1/1-Remote - SSH.log": + "related", + }); + expect( + files[ + "vscode-logs/remote-ssh/20240101T000000/window2/output_logging_2/1-Remote - SSH.log" + ], + ).toBeUndefined(); + }); + + it.runIf(canTestUnreadable)( + "skips missing or unreadable sources and includes readable files", + async () => { + const proxyDir = path.join(tmpDir, "proxy"); + await fs.mkdir(proxyDir); + await fs.writeFile(path.join(proxyDir, "coder-ssh-good.log"), "ok"); + const badLog = path.join(proxyDir, "coder-ssh-bad.log"); + await fs.writeFile(badLog, "secret"); + await fs.chmod(badLog, 0o000); + + try { + await expect( + collectTextFiles({ + activeProxyLogPath: path.join(tmpDir, "nonexistent.log"), + proxyLogDir: proxyDir, + extensionLogDir: path.join(tmpDir, "no-such-dir"), + }), + ).resolves.toEqual({ + "vscode-logs/proxy/coder-ssh-good.log": "ok", + }); + } finally { + await fs.chmod(badLog, 0o644); + } + }, + ); +}); diff --git a/test/unit/supportBundle/settings.test.ts b/test/unit/supportBundle/settings.test.ts new file mode 100644 index 0000000000..07a7bd288a --- /dev/null +++ b/test/unit/supportBundle/settings.test.ts @@ -0,0 +1,236 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; + +import { collectSettingsFile } from "@/supportBundle/settings"; + +import { createMockLogger } from "../../mocks/testHelpers"; + +const logger = createMockLogger(); + +beforeEach(() => { + setExtensions([]); + vi.mocked(vscode.workspace.getConfiguration).mockReset(); + vi.mocked(logger.warn).mockClear(); +}); + +function setExtensions( + extensions: Array<{ id: string; packageJSON: unknown }>, +): void { + Object.defineProperty(vscode.extensions, "all", { + configurable: true, + value: extensions, + }); +} + +function setConfiguration( + values: Record, + inspections: Record>, +): void { + const config: Partial = { + get: (key: string): T | undefined => values[key] as T | undefined, + inspect: (key: string) => { + const inspection = inspections[key]; + return inspection ? { key, ...inspection } : undefined; + }, + }; + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue( + config as vscode.WorkspaceConfiguration, + ); +} + +function readSettings(): Record> { + const data = collectSettingsFile(logger); + if (!data) { + throw new Error("settings were not collected"); + } + return JSON.parse(Buffer.from(data).toString()) as Record< + string, + Record + >; +} + +describe("collectSettingsFile", () => { + it("returns undefined when there are no supported settings", () => { + setConfiguration({}, {}); + + expect(collectSettingsFile(logger)).toBeUndefined(); + }); + + it("collects Coder settings and allowlisted Remote-SSH settings only", () => { + setExtensions([ + { + id: "coder.coder-remote", + packageJSON: { + name: "coder-remote", + publisher: "coder", + contributes: { + configuration: { + properties: { + "coder.binarySource": {}, + "coder.globalFlags": {}, + "coder.headerCommand": {}, + "coder.proxyLogDirectory": {}, + "coder.sshConfig": {}, + "coder.sshFlags": {}, + "coder.tlsCertRefreshCommand": {}, + "unrelated.setting": {}, + }, + }, + }, + }, + }, + { + id: "ms-vscode-remote.remote-ssh", + packageJSON: { + contributes: { + configuration: { + properties: { + "remote.SSH.connectTimeout": {}, + "remote.SSH.remotePlatform": {}, + "remote.autoForwardPorts": {}, + }, + }, + }, + }, + }, + { + id: "other.remote-extension", + packageJSON: { + contributes: { + configuration: { + properties: { "remote.someToken": {} }, + }, + }, + }, + }, + ]); + setConfiguration( + { + "coder.binarySource": "", + "coder.globalFlags": ["--header-command", "DO_NOT_LEAK_SECRET"], + "coder.headerCommand": "echo DO_NOT_LEAK_SECRET", + "coder.proxyLogDirectory": "/tmp/proxy", + "coder.sshConfig": ["SetEnv TOKEN=DO_NOT_LEAK_SECRET"], + "coder.sshFlags": ["--flag", "DO_NOT_LEAK_SECRET"], + "coder.tlsCertRefreshCommand": "refresh DO_NOT_LEAK_SECRET", + "remote.SSH.connectTimeout": 1800, + "remote.SSH.remotePlatform": { workspace: "linux" }, + "remote.autoForwardPorts": true, + }, + { + "coder.binarySource": { defaultValue: "", globalValue: "" }, + "coder.globalFlags": { + defaultValue: [], + globalValue: ["--header-command", "DO_NOT_LEAK_SECRET"], + }, + "coder.headerCommand": { + defaultValue: "", + globalValue: "echo DO_NOT_LEAK_SECRET", + }, + "coder.proxyLogDirectory": { + defaultValue: "", + globalValue: "/tmp/proxy", + }, + "coder.sshConfig": { + defaultValue: [], + globalValue: ["SetEnv TOKEN=DO_NOT_LEAK_SECRET"], + }, + "coder.sshFlags": { + defaultValue: [], + globalValue: ["--flag", "DO_NOT_LEAK_SECRET"], + }, + "coder.tlsCertRefreshCommand": { + defaultValue: "", + globalValue: "refresh DO_NOT_LEAK_SECRET", + }, + "remote.SSH.connectTimeout": { + defaultValue: 60, + globalValue: 1800, + }, + "remote.SSH.remotePlatform": { + globalValue: { workspace: "linux" }, + }, + "remote.autoForwardPorts": { + defaultValue: false, + workspaceValue: true, + }, + }, + ); + + const raw = Buffer.from( + collectSettingsFile(logger) ?? new Uint8Array(), + ).toString(); + const settings = JSON.parse(raw) as Record>; + + expect(raw).not.toContain("DO_NOT_LEAK_SECRET"); + expect(Object.keys(settings).sort()).toEqual([ + "coder.binarySource", + "coder.globalFlags", + "coder.headerCommand", + "coder.proxyLogDirectory", + "coder.sshConfig", + "coder.sshFlags", + "coder.tlsCertRefreshCommand", + "remote.SSH.connectTimeout", + "remote.autoForwardPorts", + ]); + expect(settings["coder.binarySource"]).toEqual({ + defaultValue: "", + effective: "", + globalValue: "", + key: "coder.binarySource", + }); + expect(settings["coder.headerCommand"]?.effective).toBe(""); + expect(settings["coder.globalFlags"]?.defaultValue).toBe(""); + expect(settings["coder.globalFlags"]?.effective).toBe(""); + expect(settings["coder.sshConfig"]?.effective).toBe(""); + expect(settings["coder.proxyLogDirectory"]?.effective).toBe("/tmp/proxy"); + expect(settings["remote.SSH.connectTimeout"]?.globalValue).toBe(1800); + expect(settings["remote.autoForwardPorts"]?.workspaceValue).toBe(true); + }); + + it("warns and returns undefined when settings collection fails", () => { + setExtensions([ + { + id: "coder.coder-remote", + packageJSON: { + contributes: { + configuration: { + properties: { "coder.proxyLogDirectory": {} }, + }, + }, + }, + }, + ]); + vi.mocked(vscode.workspace.getConfiguration).mockImplementation(() => { + throw new Error("settings failed"); + }); + + expect(collectSettingsFile(logger)).toBeUndefined(); + expect(logger.warn).toHaveBeenCalledWith( + "Could not collect VS Code settings", + expect.any(Error), + ); + }); + + it("supports array-style configuration contributions", () => { + setExtensions([ + { + id: "coder.coder-remote", + packageJSON: { + contributes: { + configuration: [{ properties: { "coder.proxyLogDirectory": {} } }], + }, + }, + }, + ]); + setConfiguration( + { "coder.proxyLogDirectory": "/tmp/proxy" }, + { "coder.proxyLogDirectory": { globalValue: "/tmp/proxy" } }, + ); + + expect(readSettings()["coder.proxyLogDirectory"]?.effective).toBe( + "/tmp/proxy", + ); + }); +}); diff --git a/test/unit/supportBundle/vscodeLogs.test.ts b/test/unit/supportBundle/vscodeLogs.test.ts new file mode 100644 index 0000000000..aeacac36b5 --- /dev/null +++ b/test/unit/supportBundle/vscodeLogs.test.ts @@ -0,0 +1,87 @@ +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { + collectWindowLogDirs, + resolveLogContext, +} from "@/supportBundle/vscodeLogs"; + +import { createMockLogger } from "../../mocks/testHelpers"; + +let tmpDir: string; +const logger = createMockLogger(); + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "support-bundle-vscode-")); +}); + +afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +describe("resolveLogContext", () => { + it("resolves VS Code session log layout", () => { + const extensionLogDir = path.join( + tmpDir, + "20240101T000000", + "window1", + "exthost", + "coder.coder-remote", + ); + + expect(resolveLogContext(extensionLogDir)).toEqual({ + currentWindowPath: path.join(tmpDir, "20240101T000000", "window1"), + logsRoot: tmpDir, + }); + }); + + it("resolves flat window log layout", () => { + const extensionLogDir = path.join( + tmpDir, + "window1", + "exthost", + "coder.coder-remote", + ); + + expect(resolveLogContext(extensionLogDir)).toEqual({ + currentWindowPath: path.join(tmpDir, "window1"), + logsRoot: tmpDir, + }); + }); + + it("ignores paths outside the Coder extension log layout", () => { + expect( + resolveLogContext(path.join(tmpDir, "window1", "other")), + ).toBeUndefined(); + }); +}); + +describe("collectWindowLogDirs", () => { + it("finds and sorts session and flat window directories", async () => { + await fs.mkdir(path.join(tmpDir, "20240102T000000", "window2"), { + recursive: true, + }); + await fs.mkdir(path.join(tmpDir, "20240101T000000", "window1"), { + recursive: true, + }); + await fs.mkdir(path.join(tmpDir, "window3"), { recursive: true }); + await fs.writeFile(path.join(tmpDir, "not-a-window.log"), "ignore"); + + await expect(collectWindowLogDirs(tmpDir, logger)).resolves.toEqual([ + { + relativePath: "20240101T000000/window1", + windowPath: path.join(tmpDir, "20240101T000000", "window1"), + }, + { + relativePath: "20240102T000000/window2", + windowPath: path.join(tmpDir, "20240102T000000", "window2"), + }, + { + relativePath: "window3", + windowPath: path.join(tmpDir, "window3"), + }, + ]); + }); +}); From f7c23b00661992fe117abc0543b962ab1dc70223 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 26 May 2026 18:19:32 +0300 Subject: [PATCH 2/5] refactor(supportBundle): harden log/settings collection and simplify Address code review findings and collapse the support-bundle module from 6 files to 4. Behavioral fixes: - redact Coder paths, URLs, and hostnames in shared bundles - lstat in readRecentFile so a planted symlink can't pull host files in - create the temp zip with the source's mode (no umask window, no chmod failure path that deletes the otherwise-valid bundle) - pick the active SSH log only from the current window, never fall back - drop the literal extension-id check so rebranded forks work; anchor the session-name regex - collect Remote-SSH logs from exthost dirs (the path-branch was dead) and proxy logs named bare .log - preserve the active proxy log's real filename and dedup it against the dir scan - stable lexicographic tie-break in newestLog - suppress ENOENT warnings during rotation-racey recursive walks Structural cleanup: replace package.json setting discovery with an explicit allowlist, merge vscodeLogs.ts and diagnostics.ts into logFiles.ts, fuse the per-window extension and Remote-SSH walks, drop the readDir overload and the dead collectExtensionLogs fallback. --- src/supportBundle/appendVsCodeLogs.ts | 11 +- src/supportBundle/diagnostics.ts | 18 - src/supportBundle/files.ts | 63 ++-- src/supportBundle/logFiles.ts | 320 +++++++++++------- src/supportBundle/settings.ts | 135 +++----- src/supportBundle/vscodeLogs.ts | 86 ----- .../supportBundle/appendVsCodeLogs.test.ts | 12 +- test/unit/supportBundle/diagnostics.test.ts | 50 --- test/unit/supportBundle/files.test.ts | 19 ++ test/unit/supportBundle/logFiles.test.ts | 249 +++++++++++++- test/unit/supportBundle/settings.test.ts | 217 ++++-------- test/unit/supportBundle/vscodeLogs.test.ts | 87 ----- 12 files changed, 600 insertions(+), 667 deletions(-) delete mode 100644 src/supportBundle/diagnostics.ts delete mode 100644 src/supportBundle/vscodeLogs.ts delete mode 100644 test/unit/supportBundle/diagnostics.test.ts delete mode 100644 test/unit/supportBundle/vscodeLogs.test.ts diff --git a/src/supportBundle/appendVsCodeLogs.ts b/src/supportBundle/appendVsCodeLogs.ts index e6fb39ba65..b977f2d0a5 100644 --- a/src/supportBundle/appendVsCodeLogs.ts +++ b/src/supportBundle/appendVsCodeLogs.ts @@ -7,9 +7,9 @@ import { promisify } from "node:util"; import { type Logger } from "../logging/logger"; import { renameWithRetry } from "../util/fs"; -import { collectVsCodeDiagnostics, type LogSources } from "./diagnostics"; +import { collectVsCodeDiagnostics, type LogSources } from "./logFiles"; -export type { LogSources } from "./diagnostics"; +export type { LogSources } from "./logFiles"; const unzipAsync = promisify(unzip); const zipAsync = promisify(zip); @@ -18,7 +18,7 @@ function vscodeBundlePath(zipPath: string): string { const parsed = path.parse(zipPath); return path.join( parsed.dir, - `${parsed.name}-vscode-${randomUUID()}${parsed.ext}`, + `${parsed.name}-vscode-${randomUUID().slice(0, 8)}${parsed.ext}`, ); } @@ -34,8 +34,9 @@ async function writeBundleWithLogs( entries[name] = data; } - await fs.writeFile(outputPath, await zipAsync(entries)); - await fs.chmod(outputPath, sourceMode); + // Set mode at create time: no umask window, no separate chmod that + // could fail on filesystems that don't honor mode bits. + await fs.writeFile(outputPath, await zipAsync(entries), { mode: sourceMode }); } /** diff --git a/src/supportBundle/diagnostics.ts b/src/supportBundle/diagnostics.ts deleted file mode 100644 index c76680e61c..0000000000 --- a/src/supportBundle/diagnostics.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { type Logger } from "../logging/logger"; - -import { collectSupportLogFiles, type LogSources } from "./logFiles"; -import { collectSettingsFile } from "./settings"; - -export type { LogSources } from "./logFiles"; - -export async function collectVsCodeDiagnostics( - sources: LogSources, - logger: Logger, -): Promise> { - const files = await collectSupportLogFiles(sources, logger); - const settings = collectSettingsFile(logger); - if (settings) { - files.set("vscode-logs/settings.json", settings); - } - return files; -} diff --git a/src/supportBundle/files.ts b/src/supportBundle/files.ts index e8b4625168..3fa8fabbbe 100644 --- a/src/supportBundle/files.ts +++ b/src/supportBundle/files.ts @@ -15,7 +15,8 @@ export interface CollectedFile { const LOG_MAX_AGE_MS = 3 * 24 * 60 * 60 * 1000; const MAX_LOG_SCAN_DEPTH = 6; -export const isLogFile = (name: string): boolean => name.endsWith(".log"); +// Accept .log and VS Code's rotated .log.N form. +export const isLogFile = (name: string): boolean => /\.log(\.\d+)?$/.test(name); export function normalizeZipPath(filePath: string): string { return filePath.replaceAll(path.sep, "/"); @@ -37,16 +38,12 @@ export function prefixFiles( return new Map([...files].map(([name, data]) => [`${prefix}/${name}`, data])); } -function isObject(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} - -function isNotFoundError(error: unknown): boolean { - return isObject(error) && error["code"] === "ENOENT"; -} - -function cutoffTime(): number { - return Date.now() - LOG_MAX_AGE_MS; +function isEnoent(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + (error as { code?: unknown }).code === "ENOENT" + ); } async function readRecentFile( @@ -54,8 +51,10 @@ async function readRecentFile( logger: Logger, ): Promise<{ data: Uint8Array; mtimeMs: number } | undefined> { try { - const stat = await fs.stat(filePath); - if (!stat.isFile() || stat.mtimeMs < cutoffTime()) { + // lstat: skip symlinks so a planted link can't pull host files in. + const stat = await fs.lstat(filePath); + const cutoff = Date.now() - LOG_MAX_AGE_MS; + if (!stat.isFile() || stat.mtimeMs < cutoff) { return undefined; } return { data: await fs.readFile(filePath), mtimeMs: stat.mtimeMs }; @@ -65,27 +64,15 @@ async function readRecentFile( } } -async function readDir( - dirPath: string, - logger: Logger, - options: { withFileTypes: true; warnOnMissing?: boolean }, -): Promise; -async function readDir( +async function readDirents( dirPath: string, logger: Logger, - options?: { warnOnMissing?: boolean }, -): Promise; -async function readDir( - dirPath: string, - logger: Logger, - options: { withFileTypes?: boolean; warnOnMissing?: boolean } = {}, -): Promise { + warnOnMissing = true, +): Promise { try { - return options.withFileTypes - ? await fs.readdir(dirPath, { withFileTypes: true }) - : await fs.readdir(dirPath); + return await fs.readdir(dirPath, { withFileTypes: true }); } catch (error) { - if (options.warnOnMissing !== false || !isNotFoundError(error)) { + if (warnOnMissing || !isEnoent(error)) { logger.warn(`Could not read log directory ${dirPath}`, error); } return []; @@ -99,21 +86,19 @@ export async function collectDirFiles( warnOnMissing = true, ): Promise> { const results = new Map(); - const entries = await readDir(dirPath, logger, { warnOnMissing }); + const entries = await readDirents(dirPath, logger, warnOnMissing); await Promise.all( entries.map(async (entry) => { - if (!filter(entry)) { + if (!entry.isFile() || !filter(entry.name)) { return; } - - const file = await readRecentFile(path.join(dirPath, entry), logger); + const file = await readRecentFile(path.join(dirPath, entry.name), logger); if (file) { - results.set(entry, file.data); + results.set(entry.name, file.data); } }), ); - return results; } @@ -125,23 +110,21 @@ export async function collectMatchingFiles( const results: CollectedFile[] = []; async function walk(dirPath: string, depth: number): Promise { - const entries = await readDir(dirPath, logger, { withFileTypes: true }); + // Silence ENOENT on descents; VS Code log rotation races are normal. + const entries = await readDirents(dirPath, logger, depth === 0); await Promise.all( entries.map(async (entry) => { const entryPath = path.join(dirPath, entry.name); - if (entry.isDirectory()) { if (depth < MAX_LOG_SCAN_DEPTH) { await walk(entryPath, depth + 1); } return; } - const relativePath = path.relative(rootPath, entryPath); if (!entry.isFile() || !matches(relativePath, entry.name)) { return; } - const file = await readRecentFile(entryPath, logger); if (file) { results.push({ ...file, relativePath }); diff --git a/src/supportBundle/logFiles.ts b/src/supportBundle/logFiles.ts index ad1e70fba8..130bd42b71 100644 --- a/src/supportBundle/logFiles.ts +++ b/src/supportBundle/logFiles.ts @@ -1,3 +1,4 @@ +import { type Dirent } from "node:fs"; import * as fs from "node:fs/promises"; import * as path from "node:path"; @@ -13,7 +14,7 @@ import { normalizeZipPath, prefixFiles, } from "./files"; -import { collectWindowLogDirs, resolveLogContext } from "./vscodeLogs"; +import { collectSettingsFile } from "./settings"; export interface LogSources { activeProxyLogPath?: string; @@ -21,146 +22,115 @@ export interface LogSources { extensionLogDir?: string; } -const isProxyLogFile = (name: string): boolean => - name.startsWith("coder-ssh") && isLogFile(name); - -async function collectCurrentExtensionLogs( - extensionLogDir: string, - logger: Logger, -): Promise> { - return prefixFiles( - "vscode-logs/extension", - await collectDirFiles(extensionLogDir, logger, isLogFile), - ); +interface WindowLogDir { + relativePath: string; + windowPath: string; } -async function collectWindowExtensionLogs( - windowPath: string, - windowRelativePath: string, - extensionId: string, - logger: Logger, -): Promise> { - return prefixFiles( - `vscode-logs/extension/${normalizeZipPath(windowRelativePath)}`, - await collectDirFiles( - path.join(windowPath, "exthost", extensionId), - logger, - isLogFile, - false, - ), - ); +interface LogContext { + currentWindowPath: string; + logsRoot: string; } -async function collectExtensionLogs( - extensionLogDir: string, - logger: Logger, -): Promise> { - const context = resolveLogContext(extensionLogDir); - if (!context) { - return collectCurrentExtensionLogs(extensionLogDir, logger); - } - - const files = new Map(); - const extensionId = path.basename(extensionLogDir); - for (const windowLogDir of await collectWindowLogDirs( - context.logsRoot, - logger, - )) { - addFiles( - files, - await collectWindowExtensionLogs( - windowLogDir.windowPath, - windowLogDir.relativePath, - extensionId, - logger, - ), - ); - } - - return files.size > 0 - ? files - : collectCurrentExtensionLogs(extensionLogDir, logger); -} +// Coder CLI writes either `coder-ssh-*.log` or bare `.log`. +const isProxyLogFile = (name: string): boolean => + isLogFile(name) && (name.startsWith("coder-ssh") || /^\d+\.log$/.test(name)); function isRemoteSshLog(relativePath: string, fileName: string): boolean { - if (!fileName.includes("Remote - SSH") || !isLogFile(fileName)) { + if (!isLogFile(fileName)) { return false; } - const parts = normalizeZipPath(relativePath).split("/"); - return parts.some( - (part) => - part.startsWith("output_logging_") || + // Whole exthost dir belongs to one extension; output_logging_* is shared. + if ( + parts.some((part) => (REMOTE_SSH_EXTENSION_IDS as readonly string[]).includes(part), + ) + ) { + return true; + } + return ( + parts.some((part) => part.startsWith("output_logging_")) && + fileName.includes("Remote - SSH") ); } function newestLog(logs: CollectedFile[]): CollectedFile | undefined { - return logs.toSorted( - (a, b) => - b.mtimeMs - a.mtimeMs || b.relativePath.localeCompare(a.relativePath), - )[0]; + // Lexicographic tie-break (not localeCompare) so the choice is locale-stable. + return logs.toSorted((a, b) => { + if (b.mtimeMs !== a.mtimeMs) { + return b.mtimeMs - a.mtimeMs; + } + if (b.relativePath === a.relativePath) { + return 0; + } + return b.relativePath > a.relativePath ? 1 : -1; + })[0]; } -async function collectRemoteSshLogs( +export function resolveLogContext( extensionLogDir: string, - logger: Logger, -): Promise> { - const context = resolveLogContext(extensionLogDir); - const files = new Map(); - if (!context) { - return files; +): LogContext | undefined { + const resolved = path.resolve(extensionLogDir); + const exthostDir = path.dirname(resolved); + const windowDir = path.dirname(exthostDir); + const windowName = path.basename(windowDir); + const sessionDir = path.dirname(windowDir); + + // Trust the layout, not the literal id: forks may rebrand the id. + if ( + path.basename(exthostDir) !== "exthost" || + !/^window\d+$/i.test(windowName) + ) { + return undefined; } - const remoteSshLogs: CollectedFile[] = []; - const extensionId = path.basename(extensionLogDir); - for (const windowLogDir of await collectWindowLogDirs( - context.logsRoot, - logger, - )) { - if ( - ( - await collectWindowExtensionLogs( - windowLogDir.windowPath, - windowLogDir.relativePath, - extensionId, - logger, - ) - ).size === 0 - ) { - continue; - } + const sessionName = path.basename(sessionDir); + return { + currentWindowPath: windowDir, + // Anchored so `20240101T000000-foo` doesn't widen logsRoot. + logsRoot: /^\d{8}T\d{6}$/.test(sessionName) + ? path.dirname(sessionDir) + : sessionDir, + }; +} - for (const logFile of await collectMatchingFiles( - windowLogDir.windowPath, - logger, - isRemoteSshLog, - )) { - const relativePath = normalizeZipPath( - path.join(windowLogDir.relativePath, logFile.relativePath), - ); - remoteSshLogs.push({ ...logFile, relativePath }); - files.set(`vscode-logs/remote-ssh/${relativePath}`, logFile.data); - } +async function readWindowDir( + dirPath: string, + logger: Logger, +): Promise { + try { + return await fs.readdir(dirPath, { withFileTypes: true }); + } catch (error) { + logger.warn(`Could not read log directory ${dirPath}`, error); + return []; } +} - const currentWindowRelativePath = normalizeZipPath( - path.relative(context.logsRoot, context.currentWindowPath), - ); - const currentWindowLogs = remoteSshLogs.filter((logFile) => - logFile.relativePath.startsWith(`${currentWindowRelativePath}/`), - ); - const activeLog = newestLog( - currentWindowLogs.length > 0 ? currentWindowLogs : remoteSshLogs, +export async function collectWindowLogDirs( + logsRoot: string, + logger: Logger, +): Promise { + const windows: WindowLogDir[] = []; + await Promise.all( + (await readWindowDir(logsRoot, logger)).map(async (entry) => { + if (!entry.isDirectory()) return; + const entryPath = path.join(logsRoot, entry.name); + if (/^window\d+$/i.test(entry.name)) { + windows.push({ relativePath: entry.name, windowPath: entryPath }); + return; + } + for (const sub of await readWindowDir(entryPath, logger)) { + if (sub.isDirectory() && /^window\d+$/i.test(sub.name)) { + windows.push({ + relativePath: `${entry.name}/${sub.name}`, + windowPath: path.join(entryPath, sub.name), + }); + } + } + }), ); - if (activeLog) { - files.set( - `vscode-logs/remote-ssh/${path.basename(activeLog.relativePath)}`, - activeLog.data, - ); - } - - return files; + return windows.sort((a, b) => (a.relativePath > b.relativePath ? 1 : -1)); } async function collectProxyLogs( @@ -168,11 +138,14 @@ async function collectProxyLogs( logger: Logger, ): Promise> { const files = new Map(); + const activeBasename = sources.activeProxyLogPath + ? path.basename(sources.activeProxyLogPath) + : undefined; - if (sources.activeProxyLogPath) { + if (sources.activeProxyLogPath && activeBasename) { try { files.set( - "vscode-logs/proxy/active.log", + `vscode-logs/proxy/${activeBasename}`, await fs.readFile(sources.activeProxyLogPath), ); } catch (error) { @@ -185,11 +158,98 @@ async function collectProxyLogs( files, prefixFiles( "vscode-logs/proxy", - await collectDirFiles(sources.proxyLogDir, logger, isProxyLogFile), + // Active log was already added above; don't double-bundle. + await collectDirFiles( + sources.proxyLogDir, + logger, + (name) => isProxyLogFile(name) && name !== activeBasename, + ), + ), + ); + } + return files; +} + +async function collectVsCodeWindowLogs( + extensionLogDir: string, + logger: Logger, +): Promise> { + const files = new Map(); + const context = resolveLogContext(extensionLogDir); + + if (!context) { + // Non-canonical layout: scan the extension dir + assumed window dir + // (one level up, or two if the parent is `exthost`). + addFiles( + files, + prefixFiles( + "vscode-logs/extension", + await collectDirFiles(extensionLogDir, logger, isLogFile), + ), + ); + const exthostDir = path.dirname(extensionLogDir); + const windowDir = + path.basename(exthostDir) === "exthost" + ? path.dirname(exthostDir) + : exthostDir; + for (const log of await collectMatchingFiles( + windowDir, + logger, + isRemoteSshLog, + )) { + files.set( + `vscode-logs/remote-ssh/${normalizeZipPath(log.relativePath)}`, + log.data, + ); + } + return files; + } + + const extensionId = path.basename(extensionLogDir); + const currentWindowSshLogs: CollectedFile[] = []; + + for (const window of await collectWindowLogDirs(context.logsRoot, logger)) { + const extLogs = await collectDirFiles( + path.join(window.windowPath, "exthost", extensionId), + logger, + isLogFile, + false, + ); + // Skip windows that never hosted Coder; their SSH logs aren't ours. + if (extLogs.size === 0) continue; + + addFiles( + files, + prefixFiles( + `vscode-logs/extension/${normalizeZipPath(window.relativePath)}`, + extLogs, ), ); + + const isCurrent = window.windowPath === context.currentWindowPath; + for (const sshLog of await collectMatchingFiles( + window.windowPath, + logger, + isRemoteSshLog, + )) { + const relativePath = normalizeZipPath( + path.join(window.relativePath, sshLog.relativePath), + ); + files.set(`vscode-logs/remote-ssh/${relativePath}`, sshLog.data); + if (isCurrent) { + currentWindowSshLogs.push({ ...sshLog, relativePath }); + } + } } + // Current window only; falling back to others would mislabel a stale log. + const activeLog = newestLog(currentWindowSshLogs); + if (activeLog) { + files.set( + `vscode-logs/remote-ssh/${path.basename(activeLog.relativePath)}`, + activeLog.data, + ); + } return files; } @@ -198,17 +258,23 @@ export async function collectSupportLogFiles( logger: Logger, ): Promise> { const files = await collectProxyLogs(sources, logger); - if (sources.extensionLogDir) { addFiles( files, - await collectExtensionLogs(sources.extensionLogDir, logger), - ); - addFiles( - files, - await collectRemoteSshLogs(sources.extensionLogDir, logger), + await collectVsCodeWindowLogs(sources.extensionLogDir, logger), ); } + return files; +} +export async function collectVsCodeDiagnostics( + sources: LogSources, + logger: Logger, +): Promise> { + const files = await collectSupportLogFiles(sources, logger); + const settings = collectSettingsFile(logger); + if (settings) { + files.set("vscode-logs/settings.json", settings); + } return files; } diff --git a/src/supportBundle/settings.ts b/src/supportBundle/settings.ts index 4b8cdfacd9..a2cfc7834e 100644 --- a/src/supportBundle/settings.ts +++ b/src/supportBundle/settings.ts @@ -1,32 +1,40 @@ import * as vscode from "vscode"; import { type Logger } from "../logging/logger"; -import { REMOTE_SSH_EXTENSION_IDS } from "../remote/sshExtension"; - -interface ConfigurationContribution { - properties?: unknown; -} - -interface ExtensionPackageJson { - contributes?: unknown; - name?: unknown; - publisher?: unknown; -} - -type SettingValue = unknown; -type SettingInspection = Record; -type SettingDiagnostics = Record; +// Paths, hostnames, URLs, and command strings: anything user-supplied that +// could identify a machine or deployment in a shared bundle. const REDACTED_SETTINGS = new Set([ + "coder.binaryDestination", "coder.binarySource", + "coder.defaultUrl", "coder.globalFlags", "coder.headerCommand", + "coder.proxyBypass", + "coder.proxyLogDirectory", "coder.sshConfig", "coder.sshFlags", + "coder.tlsAltHost", + "coder.tlsCaFile", + "coder.tlsCertFile", "coder.tlsCertRefreshCommand", + "coder.tlsKeyFile", ]); -const REMOTE_SETTINGS = new Set([ +// Explicit allowlist instead of package.json discovery: discovery requires +// the extension to be installed (it isn't for tests/headless runs) and +// silently misses settings declared via contributes.configurationDefaults. +const COLLECTED_SETTINGS = [ + ...REDACTED_SETTINGS, + "coder.autologin", + "coder.disableNotifications", + "coder.disableSignatureVerification", + "coder.disableUpdateNotifications", + "coder.enableDownloads", + "coder.httpClientLogLevel", + "coder.insecure", + "coder.networkThreshold.latencyMs", + "coder.useKeyring", "remote.SSH.connectTimeout", "remote.SSH.logLevel", "remote.SSH.reconnectionGraceTime", @@ -34,76 +42,10 @@ const REMOTE_SETTINGS = new Set([ "remote.SSH.useExecServer", "remote.SSH.useLocalServer", "remote.autoForwardPorts", -]); - -function isObject(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} - -function readPackageJson(value: unknown): ExtensionPackageJson | undefined { - return isObject(value) ? value : undefined; -} - -function configurationContributions( - packageJson: ExtensionPackageJson, -): ConfigurationContribution[] { - if (!isObject(packageJson.contributes)) { - return []; - } - - const configuration = packageJson.contributes["configuration"]; - const contributions = Array.isArray(configuration) - ? configuration - : [configuration]; - return contributions.filter(isObject); -} - -function isCoderExtension(extension: vscode.Extension): boolean { - const packageJson = readPackageJson(extension.packageJSON); - return ( - extension.id === "coder.coder-remote" || - (packageJson?.publisher === "coder" && packageJson.name === "coder-remote") - ); -} - -function isRemoteSshExtension(extension: vscode.Extension): boolean { - return (REMOTE_SSH_EXTENSION_IDS as readonly string[]).includes(extension.id); -} +].sort(); -function shouldCollectKey( - extension: vscode.Extension, - key: string, -): boolean { - return ( - (isCoderExtension(extension) && key.startsWith("coder.")) || - (isRemoteSshExtension(extension) && REMOTE_SETTINGS.has(key)) - ); -} - -function configurationKeys(): string[] { - const keys = new Set(); - - for (const extension of vscode.extensions.all) { - const packageJson = readPackageJson(extension.packageJSON); - if (!packageJson) { - continue; - } - - for (const contribution of configurationContributions(packageJson)) { - if (!isObject(contribution.properties)) { - continue; - } - - for (const key of Object.keys(contribution.properties)) { - if (shouldCollectKey(extension, key)) { - keys.add(key); - } - } - } - } - - return [...keys].sort(); -} +type SettingValue = unknown; +type SettingInspection = Record; function redactedSettingValue(value: SettingValue): string { const emptyArray = Array.isArray(value) && value.length === 0; @@ -112,29 +54,34 @@ function redactedSettingValue(value: SettingValue): string { : ""; } -function maybeRedactSetting(key: string, value: SettingValue): SettingValue { +function maybeRedact( + key: string, + name: string, + value: SettingValue, +): SettingValue { + // `key` and `languageIds` are inspect() metadata, not the secret payload. + if (name === "key" || name === "languageIds") { + return value; + } return REDACTED_SETTINGS.has(key) ? redactedSettingValue(value) : value; } -function collectSettingsDiagnostics(): SettingDiagnostics { +function collectSettingsDiagnostics(): Record { const config = vscode.workspace.getConfiguration(); - const diagnostics: SettingDiagnostics = {}; - - for (const key of configurationKeys()) { + const diagnostics: Record = {}; + for (const key of COLLECTED_SETTINGS) { const inspected = config.inspect(key); if (!inspected) { continue; } - const entry: SettingInspection = { - effective: maybeRedactSetting(key, config.get(key)), + effective: maybeRedact(key, "effective", config.get(key)), }; for (const [name, value] of Object.entries(inspected)) { - entry[name] = name === "key" ? value : maybeRedactSetting(key, value); + entry[name] = maybeRedact(key, name, value); } diagnostics[key] = entry; } - return diagnostics; } diff --git a/src/supportBundle/vscodeLogs.ts b/src/supportBundle/vscodeLogs.ts deleted file mode 100644 index 052f79d160..0000000000 --- a/src/supportBundle/vscodeLogs.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { type Dirent } from "node:fs"; -import * as fs from "node:fs/promises"; -import * as path from "node:path"; - -import { type Logger } from "../logging/logger"; - -interface WindowLogDir { - relativePath: string; - windowPath: string; -} - -export interface LogContext { - currentWindowPath: string; - logsRoot: string; -} - -export function resolveLogContext( - extensionLogDir: string, -): LogContext | undefined { - const resolved = path.resolve(extensionLogDir); - const extensionId = path.basename(resolved); - const exthostDir = path.dirname(resolved); - const windowDir = path.dirname(exthostDir); - const windowName = path.basename(windowDir); - const sessionDir = path.dirname(windowDir); - - if ( - extensionId !== "coder.coder-remote" || - path.basename(exthostDir) !== "exthost" || - !/^window\d+$/i.test(windowName) - ) { - return undefined; - } - - const sessionName = path.basename(sessionDir); - return { - currentWindowPath: windowDir, - logsRoot: /^\d{8}T\d{6}/.test(sessionName) - ? path.dirname(sessionDir) - : sessionDir, - }; -} - -async function readDirents(dirPath: string, logger: Logger): Promise { - try { - return await fs.readdir(dirPath, { withFileTypes: true }); - } catch (error) { - logger.warn(`Could not read log directory ${dirPath}`, error); - return []; - } -} - -export async function collectWindowLogDirs( - logsRoot: string, - logger: Logger, -): Promise { - const windows: WindowLogDir[] = []; - const rootEntries = await readDirents(logsRoot, logger); - - await Promise.all( - rootEntries.map(async (entry) => { - if (!entry.isDirectory()) { - return; - } - - const entryPath = path.join(logsRoot, entry.name); - if (/^window\d+$/i.test(entry.name)) { - windows.push({ relativePath: entry.name, windowPath: entryPath }); - return; - } - - const sessionEntries = await readDirents(entryPath, logger); - for (const windowEntry of sessionEntries.filter( - (sessionEntry) => - sessionEntry.isDirectory() && /^window\d+$/i.test(sessionEntry.name), - )) { - windows.push({ - relativePath: `${entry.name}/${windowEntry.name}`, - windowPath: path.join(entryPath, windowEntry.name), - }); - } - }), - ); - - return windows.sort((a, b) => a.relativePath.localeCompare(b.relativePath)); -} diff --git a/test/unit/supportBundle/appendVsCodeLogs.test.ts b/test/unit/supportBundle/appendVsCodeLogs.test.ts index 11ebb7fd4f..0faa1266b2 100644 --- a/test/unit/supportBundle/appendVsCodeLogs.test.ts +++ b/test/unit/supportBundle/appendVsCodeLogs.test.ts @@ -5,14 +5,14 @@ import * as path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { appendVsCodeLogs } from "@/supportBundle/appendVsCodeLogs"; -import { collectVsCodeDiagnostics } from "@/supportBundle/diagnostics"; +import { collectVsCodeDiagnostics } from "@/supportBundle/logFiles"; import { renameWithRetry } from "@/util/fs"; import { createMockLogger } from "../../mocks/testHelpers"; const collectVsCodeDiagnosticsMock = vi.hoisted(() => vi.fn()); -vi.mock("@/supportBundle/diagnostics", () => ({ +vi.mock("@/supportBundle/logFiles", () => ({ collectVsCodeDiagnostics: collectVsCodeDiagnosticsMock, })); @@ -31,6 +31,9 @@ beforeEach(async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "support-bundle-")); vi.mocked(collectVsCodeDiagnostics).mockReset(); vi.mocked(renameWithRetry).mockClear(); + vi.mocked(logger.error).mockClear(); + vi.mocked(logger.warn).mockClear(); + vi.mocked(logger.info).mockClear(); }); afterEach(async () => { @@ -57,9 +60,8 @@ async function readZip(zipPath: string): Promise> { } async function findBundleSibling(): Promise { - const sibling = (await fs.readdir(tmpDir)).find( - (name) => - name.startsWith("coder-support-123-vscode-") && name.endsWith(".zip"), + const sibling = (await fs.readdir(tmpDir)).find((name) => + /^coder-support-123-vscode-[a-f0-9]{8}\.zip$/.test(name), ); if (!sibling) { throw new Error("support bundle sibling not found"); diff --git a/test/unit/supportBundle/diagnostics.test.ts b/test/unit/supportBundle/diagnostics.test.ts deleted file mode 100644 index fc1ea90c18..0000000000 --- a/test/unit/supportBundle/diagnostics.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; - -import { collectVsCodeDiagnostics } from "@/supportBundle/diagnostics"; -import { collectSupportLogFiles } from "@/supportBundle/logFiles"; -import { collectSettingsFile } from "@/supportBundle/settings"; - -import { createMockLogger } from "../../mocks/testHelpers"; - -const collectSupportLogFilesMock = vi.hoisted(() => vi.fn()); -const collectSettingsFileMock = vi.hoisted(() => vi.fn()); - -vi.mock("@/supportBundle/logFiles", () => ({ - collectSupportLogFiles: collectSupportLogFilesMock, -})); - -vi.mock("@/supportBundle/settings", () => ({ - collectSettingsFile: collectSettingsFileMock, -})); - -const logger = createMockLogger(); - -beforeEach(() => { - vi.mocked(collectSupportLogFiles).mockReset(); - vi.mocked(collectSettingsFile).mockReset(); -}); - -describe("collectVsCodeDiagnostics", () => { - it("combines log files and settings", async () => { - vi.mocked(collectSupportLogFiles).mockResolvedValue( - new Map([["vscode-logs/proxy/active.log", Buffer.from("proxy")]]), - ); - vi.mocked(collectSettingsFile).mockReturnValue(Buffer.from("settings")); - - await expect(collectVsCodeDiagnostics({}, logger)).resolves.toEqual( - new Map([ - ["vscode-logs/proxy/active.log", Buffer.from("proxy")], - ["vscode-logs/settings.json", Buffer.from("settings")], - ]), - ); - }); - - it("does not add settings when none are collected", async () => { - vi.mocked(collectSupportLogFiles).mockResolvedValue(new Map()); - vi.mocked(collectSettingsFile).mockReturnValue(undefined); - - await expect(collectVsCodeDiagnostics({}, logger)).resolves.toEqual( - new Map(), - ); - }); -}); diff --git a/test/unit/supportBundle/files.test.ts b/test/unit/supportBundle/files.test.ts index f400303d68..ea167397a5 100644 --- a/test/unit/supportBundle/files.test.ts +++ b/test/unit/supportBundle/files.test.ts @@ -34,7 +34,10 @@ describe("support bundle file helpers", () => { it("normalizes zip paths and identifies log files", () => { expect(normalizeZipPath("a/b/c.log")).toBe("a/b/c.log"); expect(isLogFile("Coder.log")).toBe(true); + expect(isLogFile("Coder.log.1")).toBe(true); + expect(isLogFile("Coder.log.12")).toBe(true); expect(isLogFile("settings.json")).toBe(false); + expect(isLogFile("Coder.log.gz")).toBe(false); }); it("prefixes and merges file maps", () => { @@ -84,4 +87,20 @@ describe("support bundle file helpers", () => { }, ]); }); + + it.runIf(process.platform !== "win32")( + "does not follow symlinks when reading files", + async () => { + const outsideTarget = path.join(tmpDir, "outside.secret"); + await fs.writeFile(outsideTarget, "should-not-be-read"); + const logsDir = path.join(tmpDir, "logs"); + await fs.mkdir(logsDir); + await fs.symlink(outsideTarget, path.join(logsDir, "evil.log")); + await fs.writeFile(path.join(logsDir, "good.log"), "good"); + + const files = await collectDirFiles(logsDir, logger, isLogFile); + + expect(files).toEqual(new Map([["good.log", Buffer.from("good")]])); + }, + ); }); diff --git a/test/unit/supportBundle/logFiles.test.ts b/test/unit/supportBundle/logFiles.test.ts index 6df22dbc0b..888a91bd8a 100644 --- a/test/unit/supportBundle/logFiles.test.ts +++ b/test/unit/supportBundle/logFiles.test.ts @@ -3,7 +3,11 @@ import * as os from "node:os"; import * as path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { collectSupportLogFiles } from "@/supportBundle/logFiles"; +import { + collectSupportLogFiles, + collectWindowLogDirs, + resolveLogContext, +} from "@/supportBundle/logFiles"; import { createMockLogger } from "../../mocks/testHelpers"; @@ -37,13 +41,14 @@ async function collectTextFiles( } describe("collectSupportLogFiles", () => { - it("collects active proxy log and recent Coder SSH proxy logs", async () => { + it("collects active proxy log under its real name alongside Coder SSH proxy logs", async () => { const proxyDir = path.join(tmpDir, "proxy"); await fs.mkdir(proxyDir); const activeProxyLog = path.join(tmpDir, "custom-active.log"); await fs.writeFile(activeProxyLog, "active"); await fs.writeFile(path.join(proxyDir, "coder-ssh-recent.log"), "recent"); await fs.writeFile(path.join(proxyDir, "coder-ssh-old.log"), "old"); + await fs.writeFile(path.join(proxyDir, "12345.log"), "pid-style"); await fs.writeFile(path.join(proxyDir, "other.log"), "other"); await fs.writeFile(path.join(proxyDir, "secret.env"), "secret"); await fs.mkdir(path.join(proxyDir, "subdir")); @@ -55,15 +60,33 @@ describe("collectSupportLogFiles", () => { proxyLogDir: proxyDir, }), ).resolves.toEqual({ - "vscode-logs/proxy/active.log": "active", + "vscode-logs/proxy/custom-active.log": "active", "vscode-logs/proxy/coder-ssh-recent.log": "recent", + "vscode-logs/proxy/12345.log": "pid-style", }); }); - it("collects recent extension logs from a non-canonical extension log directory", async () => { + it("does not double-bundle the active proxy log when it lives inside proxyLogDir", async () => { + const proxyDir = path.join(tmpDir, "proxy"); + await fs.mkdir(proxyDir); + const activeProxyLog = path.join(proxyDir, "coder-ssh-active.log"); + await fs.writeFile(activeProxyLog, "active"); + + const result = await collectTextFiles({ + activeProxyLogPath: activeProxyLog, + proxyLogDir: proxyDir, + }); + + expect(result).toEqual({ + "vscode-logs/proxy/coder-ssh-active.log": "active", + }); + }); + + it("collects recent extension logs (including rotated .log.N) from a non-canonical extension log directory", async () => { const extDir = path.join(tmpDir, "ext"); await fs.mkdir(extDir); await fs.writeFile(path.join(extDir, "Coder-recent.log"), "recent"); + await fs.writeFile(path.join(extDir, "Coder-recent.log.1"), "rotated"); await fs.writeFile(path.join(extDir, "Coder-old.log"), "old"); await fs.writeFile(path.join(extDir, "notes.txt"), "notes"); await fs.mkdir(path.join(extDir, "subdir")); @@ -73,6 +96,7 @@ describe("collectSupportLogFiles", () => { collectTextFiles({ extensionLogDir: extDir }), ).resolves.toEqual({ "vscode-logs/extension/Coder-recent.log": "recent", + "vscode-logs/extension/Coder-recent.log.1": "rotated", }); }); @@ -216,6 +240,128 @@ describe("collectSupportLogFiles", () => { ).toBeUndefined(); }); + it("collects Remote-SSH logs from the host extension's exthost dir even when filenames lack 'Remote - SSH'", async () => { + const logsRoot = path.join(tmpDir, "logs"); + const currentExtDir = path.join( + logsRoot, + "20240101T000000", + "window1", + "exthost", + "coder.coder-remote", + ); + await fs.mkdir(currentExtDir, { recursive: true }); + await fs.writeFile(path.join(currentExtDir, "Coder.log"), "coder"); + + const remoteSshExtDir = path.join( + logsRoot, + "20240101T000000", + "window1", + "exthost", + "ms-vscode-remote.remote-ssh", + ); + await fs.mkdir(remoteSshExtDir, { recursive: true }); + await fs.writeFile(path.join(remoteSshExtDir, "1.log"), "ext-host-log"); + + const files = await collectTextFiles({ extensionLogDir: currentExtDir }); + + expect(files).toMatchObject({ + "vscode-logs/remote-ssh/20240101T000000/window1/exthost/ms-vscode-remote.remote-ssh/1.log": + "ext-host-log", + }); + }); + + it("does not promote a stale log from another window when the current window has no Remote-SSH logs", async () => { + const logsRoot = path.join(tmpDir, "logs"); + const currentSession = "20240103T000000"; + const otherSession = "20240102T000000"; + + const currentExtDir = path.join( + logsRoot, + currentSession, + "window1", + "exthost", + "coder.coder-remote", + ); + const otherExtDir = path.join( + logsRoot, + otherSession, + "window1", + "exthost", + "coder.coder-remote", + ); + await fs.mkdir(currentExtDir, { recursive: true }); + await fs.mkdir(otherExtDir, { recursive: true }); + await fs.writeFile(path.join(currentExtDir, "Coder.log"), "current"); + await fs.writeFile(path.join(otherExtDir, "Coder.log"), "other"); + + const otherSshDir = path.join( + logsRoot, + otherSession, + "window1", + "output_logging_1", + ); + await fs.mkdir(otherSshDir, { recursive: true }); + await fs.writeFile( + path.join(otherSshDir, "1-Remote - SSH.log"), + "stale ssh", + ); + + const files = await collectTextFiles({ extensionLogDir: currentExtDir }); + + expect(files["vscode-logs/remote-ssh/1-Remote - SSH.log"]).toBeUndefined(); + expect( + files[ + `vscode-logs/remote-ssh/${otherSession}/window1/output_logging_1/1-Remote - SSH.log` + ], + ).toBe("stale ssh"); + }); + + it("falls back to scanning the assumed window dir when the layout is non-canonical", async () => { + const extDir = path.join(tmpDir, "ext"); + const siblingSshDir = path.join(tmpDir, "output_logging_1"); + await fs.mkdir(extDir); + await fs.mkdir(siblingSshDir); + await fs.writeFile(path.join(siblingSshDir, "1-Remote - SSH.log"), "ssh"); + await fs.writeFile(path.join(extDir, "Coder.log"), "coder"); + + const files = await collectTextFiles({ extensionLogDir: extDir }); + + expect(files).toMatchObject({ + "vscode-logs/extension/Coder.log": "coder", + "vscode-logs/remote-ssh/output_logging_1/1-Remote - SSH.log": "ssh", + }); + }); + + it("resolves a forked Coder extension id via the same layout", async () => { + const logsRoot = path.join(tmpDir, "logs"); + const currentExtDir = path.join( + logsRoot, + "20240101T000000", + "window1", + "exthost", + "mycompany.coder-remote", + ); + await fs.mkdir(currentExtDir, { recursive: true }); + await fs.writeFile(path.join(currentExtDir, "Coder.log"), "coder"); + + const previousExtDir = path.join( + logsRoot, + "20240102T000000", + "window1", + "exthost", + "mycompany.coder-remote", + ); + await fs.mkdir(previousExtDir, { recursive: true }); + await fs.writeFile(path.join(previousExtDir, "Coder.log"), "previous"); + + const files = await collectTextFiles({ extensionLogDir: currentExtDir }); + + expect(files).toMatchObject({ + "vscode-logs/extension/20240101T000000/window1/Coder.log": "coder", + "vscode-logs/extension/20240102T000000/window1/Coder.log": "previous", + }); + }); + it.runIf(canTestUnreadable)( "skips missing or unreadable sources and includes readable files", async () => { @@ -242,3 +388,98 @@ describe("collectSupportLogFiles", () => { }, ); }); + +describe("resolveLogContext", () => { + it("resolves VS Code session log layout", () => { + const extensionLogDir = path.join( + tmpDir, + "20240101T000000", + "window1", + "exthost", + "coder.coder-remote", + ); + + expect(resolveLogContext(extensionLogDir)).toEqual({ + currentWindowPath: path.join(tmpDir, "20240101T000000", "window1"), + logsRoot: tmpDir, + }); + }); + + it("resolves flat window log layout", () => { + const extensionLogDir = path.join( + tmpDir, + "window1", + "exthost", + "coder.coder-remote", + ); + + expect(resolveLogContext(extensionLogDir)).toEqual({ + currentWindowPath: path.join(tmpDir, "window1"), + logsRoot: tmpDir, + }); + }); + + it("ignores paths outside the Coder extension log layout", () => { + expect( + resolveLogContext(path.join(tmpDir, "window1", "other")), + ).toBeUndefined(); + }); + + it("accepts a forked Coder extension id provided the layout matches", () => { + const extensionLogDir = path.join( + tmpDir, + "20240101T000000", + "window1", + "exthost", + "mycompany.coder-remote", + ); + + expect(resolveLogContext(extensionLogDir)).toEqual({ + currentWindowPath: path.join(tmpDir, "20240101T000000", "window1"), + logsRoot: tmpDir, + }); + }); + + it("does not treat a non-canonical session-suffix name as a session root", () => { + const extensionLogDir = path.join( + tmpDir, + "20240101T000000-foo", + "window1", + "exthost", + "coder.coder-remote", + ); + + expect(resolveLogContext(extensionLogDir)).toEqual({ + currentWindowPath: path.join(tmpDir, "20240101T000000-foo", "window1"), + logsRoot: path.join(tmpDir, "20240101T000000-foo"), + }); + }); +}); + +describe("collectWindowLogDirs", () => { + it("finds and sorts session and flat window directories", async () => { + await fs.mkdir(path.join(tmpDir, "20240102T000000", "window2"), { + recursive: true, + }); + await fs.mkdir(path.join(tmpDir, "20240101T000000", "window1"), { + recursive: true, + }); + await fs.mkdir(path.join(tmpDir, "window3"), { recursive: true }); + await fs.writeFile(path.join(tmpDir, "not-a-window.log"), "ignore"); + + await expect(collectWindowLogDirs(tmpDir, logger)).resolves.toEqual([ + { + relativePath: "20240101T000000/window1", + windowPath: path.join(tmpDir, "20240101T000000", "window1"), + }, + { + relativePath: "20240102T000000/window2", + windowPath: path.join(tmpDir, "20240102T000000", "window2"), + }, + { + relativePath: "window3", + windowPath: path.join(tmpDir, "window3"), + }, + ]); + }); +}); diff --git a/test/unit/supportBundle/settings.test.ts b/test/unit/supportBundle/settings.test.ts index 07a7bd288a..e4b5be600a 100644 --- a/test/unit/supportBundle/settings.test.ts +++ b/test/unit/supportBundle/settings.test.ts @@ -8,20 +8,10 @@ import { createMockLogger } from "../../mocks/testHelpers"; const logger = createMockLogger(); beforeEach(() => { - setExtensions([]); vi.mocked(vscode.workspace.getConfiguration).mockReset(); vi.mocked(logger.warn).mockClear(); }); -function setExtensions( - extensions: Array<{ id: string; packageJSON: unknown }>, -): void { - Object.defineProperty(vscode.extensions, "all", { - configurable: true, - value: extensions, - }); -} - function setConfiguration( values: Record, inspections: Record>, @@ -38,17 +28,6 @@ function setConfiguration( ); } -function readSettings(): Record> { - const data = collectSettingsFile(logger); - if (!data) { - throw new Error("settings were not collected"); - } - return JSON.parse(Buffer.from(data).toString()) as Record< - string, - Record - >; -} - describe("collectSettingsFile", () => { it("returns undefined when there are no supported settings", () => { setConfiguration({}, {}); @@ -56,104 +35,40 @@ describe("collectSettingsFile", () => { expect(collectSettingsFile(logger)).toBeUndefined(); }); - it("collects Coder settings and allowlisted Remote-SSH settings only", () => { - setExtensions([ - { - id: "coder.coder-remote", - packageJSON: { - name: "coder-remote", - publisher: "coder", - contributes: { - configuration: { - properties: { - "coder.binarySource": {}, - "coder.globalFlags": {}, - "coder.headerCommand": {}, - "coder.proxyLogDirectory": {}, - "coder.sshConfig": {}, - "coder.sshFlags": {}, - "coder.tlsCertRefreshCommand": {}, - "unrelated.setting": {}, - }, - }, - }, - }, - }, - { - id: "ms-vscode-remote.remote-ssh", - packageJSON: { - contributes: { - configuration: { - properties: { - "remote.SSH.connectTimeout": {}, - "remote.SSH.remotePlatform": {}, - "remote.autoForwardPorts": {}, - }, - }, - }, - }, - }, - { - id: "other.remote-extension", - packageJSON: { - contributes: { - configuration: { - properties: { "remote.someToken": {} }, - }, - }, - }, - }, - ]); + it("redacts sensitive Coder settings while preserving safe ones", () => { setConfiguration( { - "coder.binarySource": "", - "coder.globalFlags": ["--header-command", "DO_NOT_LEAK_SECRET"], "coder.headerCommand": "echo DO_NOT_LEAK_SECRET", - "coder.proxyLogDirectory": "/tmp/proxy", - "coder.sshConfig": ["SetEnv TOKEN=DO_NOT_LEAK_SECRET"], "coder.sshFlags": ["--flag", "DO_NOT_LEAK_SECRET"], - "coder.tlsCertRefreshCommand": "refresh DO_NOT_LEAK_SECRET", - "remote.SSH.connectTimeout": 1800, - "remote.SSH.remotePlatform": { workspace: "linux" }, - "remote.autoForwardPorts": true, + "coder.tlsCertFile": "/etc/ssl/DO_NOT_LEAK_SECRET.pem", + "coder.defaultUrl": "https://internal.DO_NOT_LEAK_SECRET", + "coder.insecure": true, + "coder.httpClientLogLevel": "debug", + "coder.binarySource": "", }, { - "coder.binarySource": { defaultValue: "", globalValue: "" }, - "coder.globalFlags": { - defaultValue: [], - globalValue: ["--header-command", "DO_NOT_LEAK_SECRET"], - }, "coder.headerCommand": { defaultValue: "", globalValue: "echo DO_NOT_LEAK_SECRET", }, - "coder.proxyLogDirectory": { - defaultValue: "", - globalValue: "/tmp/proxy", - }, - "coder.sshConfig": { - defaultValue: [], - globalValue: ["SetEnv TOKEN=DO_NOT_LEAK_SECRET"], - }, "coder.sshFlags": { defaultValue: [], globalValue: ["--flag", "DO_NOT_LEAK_SECRET"], }, - "coder.tlsCertRefreshCommand": { + "coder.tlsCertFile": { defaultValue: "", - globalValue: "refresh DO_NOT_LEAK_SECRET", + globalValue: "/etc/ssl/DO_NOT_LEAK_SECRET.pem", }, - "remote.SSH.connectTimeout": { - defaultValue: 60, - globalValue: 1800, - }, - "remote.SSH.remotePlatform": { - globalValue: { workspace: "linux" }, + "coder.defaultUrl": { + defaultValue: "", + globalValue: "https://internal.DO_NOT_LEAK_SECRET", }, - "remote.autoForwardPorts": { - defaultValue: false, - workspaceValue: true, + "coder.insecure": { defaultValue: false, globalValue: true }, + "coder.httpClientLogLevel": { + defaultValue: "info", + globalValue: "debug", }, + "coder.binarySource": { defaultValue: "", globalValue: "" }, }, ); @@ -163,45 +78,66 @@ describe("collectSettingsFile", () => { const settings = JSON.parse(raw) as Record>; expect(raw).not.toContain("DO_NOT_LEAK_SECRET"); - expect(Object.keys(settings).sort()).toEqual([ - "coder.binarySource", - "coder.globalFlags", - "coder.headerCommand", - "coder.proxyLogDirectory", - "coder.sshConfig", - "coder.sshFlags", - "coder.tlsCertRefreshCommand", - "remote.SSH.connectTimeout", - "remote.autoForwardPorts", - ]); - expect(settings["coder.binarySource"]).toEqual({ + expect(settings["coder.headerCommand"]).toEqual({ defaultValue: "", - effective: "", - globalValue: "", - key: "coder.binarySource", + effective: "", + globalValue: "", + key: "coder.headerCommand", }); - expect(settings["coder.headerCommand"]?.effective).toBe(""); - expect(settings["coder.globalFlags"]?.defaultValue).toBe(""); - expect(settings["coder.globalFlags"]?.effective).toBe(""); - expect(settings["coder.sshConfig"]?.effective).toBe(""); - expect(settings["coder.proxyLogDirectory"]?.effective).toBe("/tmp/proxy"); + expect(settings["coder.sshFlags"]?.effective).toBe(""); + expect(settings["coder.tlsCertFile"]?.effective).toBe(""); + expect(settings["coder.defaultUrl"]?.effective).toBe(""); + expect(settings["coder.binarySource"]?.effective).toBe(""); + // Non-sensitive settings pass through verbatim. + expect(settings["coder.insecure"]?.effective).toBe(true); + expect(settings["coder.httpClientLogLevel"]?.effective).toBe("debug"); + }); + + it("collects allowlisted Remote-SSH settings", () => { + setConfiguration( + { + "remote.SSH.connectTimeout": 1800, + "remote.autoForwardPorts": true, + }, + { + "remote.SSH.connectTimeout": { defaultValue: 60, globalValue: 1800 }, + "remote.autoForwardPorts": { + defaultValue: false, + workspaceValue: true, + }, + }, + ); + + const settings = JSON.parse( + Buffer.from(collectSettingsFile(logger) ?? new Uint8Array()).toString(), + ) as Record>; + expect(settings["remote.SSH.connectTimeout"]?.globalValue).toBe(1800); expect(settings["remote.autoForwardPorts"]?.workspaceValue).toBe(true); }); - it("warns and returns undefined when settings collection fails", () => { - setExtensions([ + it("preserves languageIds metadata on redacted settings", () => { + setConfiguration( + { "coder.headerCommand": "echo secret" }, { - id: "coder.coder-remote", - packageJSON: { - contributes: { - configuration: { - properties: { "coder.proxyLogDirectory": {} }, - }, - }, + "coder.headerCommand": { + globalValue: "echo secret", + languageIds: ["typescript"], }, }, + ); + + const settings = JSON.parse( + Buffer.from(collectSettingsFile(logger) ?? new Uint8Array()).toString(), + ) as Record>; + + expect(settings["coder.headerCommand"]?.languageIds).toEqual([ + "typescript", ]); + expect(settings["coder.headerCommand"]?.globalValue).toBe(""); + }); + + it("warns and returns undefined when settings collection fails", () => { vi.mocked(vscode.workspace.getConfiguration).mockImplementation(() => { throw new Error("settings failed"); }); @@ -212,25 +148,4 @@ describe("collectSettingsFile", () => { expect.any(Error), ); }); - - it("supports array-style configuration contributions", () => { - setExtensions([ - { - id: "coder.coder-remote", - packageJSON: { - contributes: { - configuration: [{ properties: { "coder.proxyLogDirectory": {} } }], - }, - }, - }, - ]); - setConfiguration( - { "coder.proxyLogDirectory": "/tmp/proxy" }, - { "coder.proxyLogDirectory": { globalValue: "/tmp/proxy" } }, - ); - - expect(readSettings()["coder.proxyLogDirectory"]?.effective).toBe( - "/tmp/proxy", - ); - }); }); diff --git a/test/unit/supportBundle/vscodeLogs.test.ts b/test/unit/supportBundle/vscodeLogs.test.ts deleted file mode 100644 index aeacac36b5..0000000000 --- a/test/unit/supportBundle/vscodeLogs.test.ts +++ /dev/null @@ -1,87 +0,0 @@ -import * as fs from "node:fs/promises"; -import * as os from "node:os"; -import * as path from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; - -import { - collectWindowLogDirs, - resolveLogContext, -} from "@/supportBundle/vscodeLogs"; - -import { createMockLogger } from "../../mocks/testHelpers"; - -let tmpDir: string; -const logger = createMockLogger(); - -beforeEach(async () => { - tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "support-bundle-vscode-")); -}); - -afterEach(async () => { - await fs.rm(tmpDir, { recursive: true, force: true }); -}); - -describe("resolveLogContext", () => { - it("resolves VS Code session log layout", () => { - const extensionLogDir = path.join( - tmpDir, - "20240101T000000", - "window1", - "exthost", - "coder.coder-remote", - ); - - expect(resolveLogContext(extensionLogDir)).toEqual({ - currentWindowPath: path.join(tmpDir, "20240101T000000", "window1"), - logsRoot: tmpDir, - }); - }); - - it("resolves flat window log layout", () => { - const extensionLogDir = path.join( - tmpDir, - "window1", - "exthost", - "coder.coder-remote", - ); - - expect(resolveLogContext(extensionLogDir)).toEqual({ - currentWindowPath: path.join(tmpDir, "window1"), - logsRoot: tmpDir, - }); - }); - - it("ignores paths outside the Coder extension log layout", () => { - expect( - resolveLogContext(path.join(tmpDir, "window1", "other")), - ).toBeUndefined(); - }); -}); - -describe("collectWindowLogDirs", () => { - it("finds and sorts session and flat window directories", async () => { - await fs.mkdir(path.join(tmpDir, "20240102T000000", "window2"), { - recursive: true, - }); - await fs.mkdir(path.join(tmpDir, "20240101T000000", "window1"), { - recursive: true, - }); - await fs.mkdir(path.join(tmpDir, "window3"), { recursive: true }); - await fs.writeFile(path.join(tmpDir, "not-a-window.log"), "ignore"); - - await expect(collectWindowLogDirs(tmpDir, logger)).resolves.toEqual([ - { - relativePath: "20240101T000000/window1", - windowPath: path.join(tmpDir, "20240101T000000", "window1"), - }, - { - relativePath: "20240102T000000/window2", - windowPath: path.join(tmpDir, "20240102T000000", "window2"), - }, - { - relativePath: "window3", - windowPath: path.join(tmpDir, "window3"), - }, - ]); - }); -}); From e907191cb9cab12802a8c66f89e9289444a2fe22 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 26 May 2026 19:05:04 +0300 Subject: [PATCH 3/5] refactor(supportBundle): address PR #971 review feedback - Fix sort comparator equality case in collectWindowLogDirs (CRF-1) - Drop readWindowDir; reuse exported readDirents (CRF-2) - Route active proxy log through readLogFile so it gets the lstat symlink guard and a path-bearing warn message (CRF-4, CRF-10) - Add coder.experimental.oauth, coder.telemetry.level, coder.telemetry.local to COLLECTED_SETTINGS (CRF-5) - Rename logFiles/writeBundleWithLogs to diagnosticFiles/ writeBundleWithDiagnostics and update log copy (CRF-6) - Add JSDoc on collectVsCodeDiagnostics and collectSettingsFile (CRF-7) --- src/supportBundle/appendVsCodeLogs.ts | 29 ++++++++------ src/supportBundle/files.ts | 20 +++++++--- src/supportBundle/logFiles.ts | 38 +++++++------------ src/supportBundle/settings.ts | 12 +++++- .../supportBundle/appendVsCodeLogs.test.ts | 2 +- 5 files changed, 58 insertions(+), 43 deletions(-) diff --git a/src/supportBundle/appendVsCodeLogs.ts b/src/supportBundle/appendVsCodeLogs.ts index b977f2d0a5..169739bb0d 100644 --- a/src/supportBundle/appendVsCodeLogs.ts +++ b/src/supportBundle/appendVsCodeLogs.ts @@ -22,15 +22,15 @@ function vscodeBundlePath(zipPath: string): string { ); } -async function writeBundleWithLogs( +async function writeBundleWithDiagnostics( zipPath: string, outputPath: string, - logFiles: Map, + diagnosticFiles: Map, ): Promise { const sourceMode = (await fs.stat(zipPath)).mode & 0o777; const entries: Zippable = await unzipAsync(await fs.readFile(zipPath)); - for (const [name, data] of logFiles) { + for (const [name, data] of diagnosticFiles) { entries[name] = data; } @@ -49,21 +49,28 @@ export async function appendVsCodeLogs( logger: Logger, ): Promise { try { - const logFiles = await collectVsCodeDiagnostics(sources, logger); - if (logFiles.size === 0) { - logger.info("No VS Code logs found to add to support bundle"); + const diagnosticFiles = await collectVsCodeDiagnostics(sources, logger); + if (diagnosticFiles.size === 0) { + logger.info("No VS Code diagnostics found to add to support bundle"); return; } logger.info( - `Adding ${logFiles.size} VS Code log file(s) to support bundle`, + `Adding ${diagnosticFiles.size} VS Code diagnostic file(s) to support bundle`, ); const outputBundlePath = vscodeBundlePath(zipPath); try { - await writeBundleWithLogs(zipPath, outputBundlePath, logFiles); + await writeBundleWithDiagnostics( + zipPath, + outputBundlePath, + diagnosticFiles, + ); } catch (error) { - logger.error("Failed to add VS Code logs to support bundle", error); + logger.error( + "Failed to add VS Code diagnostics to support bundle", + error, + ); try { await fs.rm(outputBundlePath, { force: true }); @@ -80,12 +87,12 @@ export async function appendVsCodeLogs( await renameWithRetry(fs.rename, outputBundlePath, zipPath); } catch (error) { logger.warn( - `Could not replace original bundle; VS Code logs saved separately at ${outputBundlePath}`, + `Could not replace original bundle; VS Code diagnostics saved separately at ${outputBundlePath}`, error, ); } } catch (error) { // Best-effort: never let a failure here lose the user's bundle. - logger.error("Unexpected error appending VS Code logs", error); + logger.error("Unexpected error appending VS Code diagnostics", error); } } diff --git a/src/supportBundle/files.ts b/src/supportBundle/files.ts index 3fa8fabbbe..4614a6fba2 100644 --- a/src/supportBundle/files.ts +++ b/src/supportBundle/files.ts @@ -46,15 +46,14 @@ function isEnoent(error: unknown): boolean { ); } -async function readRecentFile( +// lstat rejects symlinks so a planted link can't pull host files in. +export async function readLogFile( filePath: string, logger: Logger, ): Promise<{ data: Uint8Array; mtimeMs: number } | undefined> { try { - // lstat: skip symlinks so a planted link can't pull host files in. const stat = await fs.lstat(filePath); - const cutoff = Date.now() - LOG_MAX_AGE_MS; - if (!stat.isFile() || stat.mtimeMs < cutoff) { + if (!stat.isFile()) { return undefined; } return { data: await fs.readFile(filePath), mtimeMs: stat.mtimeMs }; @@ -64,7 +63,18 @@ async function readRecentFile( } } -async function readDirents( +async function readRecentFile( + filePath: string, + logger: Logger, +): Promise<{ data: Uint8Array; mtimeMs: number } | undefined> { + const file = await readLogFile(filePath, logger); + if (!file || file.mtimeMs < Date.now() - LOG_MAX_AGE_MS) { + return undefined; + } + return file; +} + +export async function readDirents( dirPath: string, logger: Logger, warnOnMissing = true, diff --git a/src/supportBundle/logFiles.ts b/src/supportBundle/logFiles.ts index 130bd42b71..2e195ba082 100644 --- a/src/supportBundle/logFiles.ts +++ b/src/supportBundle/logFiles.ts @@ -1,5 +1,3 @@ -import { type Dirent } from "node:fs"; -import * as fs from "node:fs/promises"; import * as path from "node:path"; import { type Logger } from "../logging/logger"; @@ -13,6 +11,8 @@ import { isLogFile, normalizeZipPath, prefixFiles, + readDirents, + readLogFile, } from "./files"; import { collectSettingsFile } from "./settings"; @@ -95,32 +95,20 @@ export function resolveLogContext( }; } -async function readWindowDir( - dirPath: string, - logger: Logger, -): Promise { - try { - return await fs.readdir(dirPath, { withFileTypes: true }); - } catch (error) { - logger.warn(`Could not read log directory ${dirPath}`, error); - return []; - } -} - export async function collectWindowLogDirs( logsRoot: string, logger: Logger, ): Promise { const windows: WindowLogDir[] = []; await Promise.all( - (await readWindowDir(logsRoot, logger)).map(async (entry) => { + (await readDirents(logsRoot, logger)).map(async (entry) => { if (!entry.isDirectory()) return; const entryPath = path.join(logsRoot, entry.name); if (/^window\d+$/i.test(entry.name)) { windows.push({ relativePath: entry.name, windowPath: entryPath }); return; } - for (const sub of await readWindowDir(entryPath, logger)) { + for (const sub of await readDirents(entryPath, logger)) { if (sub.isDirectory() && /^window\d+$/i.test(sub.name)) { windows.push({ relativePath: `${entry.name}/${sub.name}`, @@ -130,7 +118,7 @@ export async function collectWindowLogDirs( } }), ); - return windows.sort((a, b) => (a.relativePath > b.relativePath ? 1 : -1)); + return windows.sort((a, b) => a.relativePath.localeCompare(b.relativePath)); } async function collectProxyLogs( @@ -143,13 +131,10 @@ async function collectProxyLogs( : undefined; if (sources.activeProxyLogPath && activeBasename) { - try { - files.set( - `vscode-logs/proxy/${activeBasename}`, - await fs.readFile(sources.activeProxyLogPath), - ); - } catch (error) { - logger.warn("Could not read active Coder SSH proxy log", error); + // No age cutoff: long-lived sessions can have mtime past the window. + const file = await readLogFile(sources.activeProxyLogPath, logger); + if (file) { + files.set(`vscode-logs/proxy/${activeBasename}`, file.data); } } @@ -267,6 +252,11 @@ export async function collectSupportLogFiles( return files; } +/** + * Collects proxy logs, Remote-SSH and extension logs across recent windows, + * and a redacted `coder.*` / `remote.*` settings snapshot. Keys are + * zip-relative paths under `vscode-logs/`. + */ export async function collectVsCodeDiagnostics( sources: LogSources, logger: Logger, diff --git a/src/supportBundle/settings.ts b/src/supportBundle/settings.ts index a2cfc7834e..82aa7269ad 100644 --- a/src/supportBundle/settings.ts +++ b/src/supportBundle/settings.ts @@ -4,7 +4,7 @@ import { type Logger } from "../logging/logger"; // Paths, hostnames, URLs, and command strings: anything user-supplied that // could identify a machine or deployment in a shared bundle. -const REDACTED_SETTINGS = new Set([ +const REDACTED_SETTINGS: ReadonlySet = new Set([ "coder.binaryDestination", "coder.binarySource", "coder.defaultUrl", @@ -24,16 +24,19 @@ const REDACTED_SETTINGS = new Set([ // Explicit allowlist instead of package.json discovery: discovery requires // the extension to be installed (it isn't for tests/headless runs) and // silently misses settings declared via contributes.configurationDefaults. -const COLLECTED_SETTINGS = [ +const COLLECTED_SETTINGS: readonly string[] = [ ...REDACTED_SETTINGS, "coder.autologin", "coder.disableNotifications", "coder.disableSignatureVerification", "coder.disableUpdateNotifications", "coder.enableDownloads", + "coder.experimental.oauth", "coder.httpClientLogLevel", "coder.insecure", "coder.networkThreshold.latencyMs", + "coder.telemetry.level", + "coder.telemetry.local", "coder.useKeyring", "remote.SSH.connectTimeout", "remote.SSH.logLevel", @@ -85,6 +88,11 @@ function collectSettingsDiagnostics(): Record { return diagnostics; } +/** + * Returns a UTF-8 JSON snapshot of `inspect()` output for the allowlisted + * `coder.*` / `remote.*` settings. Sensitive values (paths, hostnames, + * URLs, commands) are replaced with `` or ``. + */ export function collectSettingsFile(logger: Logger): Uint8Array | undefined { try { const diagnostics = collectSettingsDiagnostics(); diff --git a/test/unit/supportBundle/appendVsCodeLogs.test.ts b/test/unit/supportBundle/appendVsCodeLogs.test.ts index 0faa1266b2..9d0b61dc06 100644 --- a/test/unit/supportBundle/appendVsCodeLogs.test.ts +++ b/test/unit/supportBundle/appendVsCodeLogs.test.ts @@ -173,7 +173,7 @@ describe("appendVsCodeLogs", () => { expect(Buffer.compare(beforeBytes, await fs.readFile(zipPath))).toBe(0); expect(logger.error).toHaveBeenCalledWith( - "Unexpected error appending VS Code logs", + "Unexpected error appending VS Code diagnostics", expect.any(Error), ); }); From 162b6113f147362282127aba0ad24e31ce8a06a3 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 26 May 2026 19:27:04 +0300 Subject: [PATCH 4/5] refactor(supportBundle): share Remote-SSH layout predicates Address PR #971 CRF-8: extract the Remote-SSH log layout knowledge (extension exthost dirs, `output_logging_` prefix, `Remote - SSH` filename fragment) into `sshExtension.ts` as predicates and consume them from both `sshProcess.ts` and `supportBundle/logFiles.ts`. A future VS Code log layout change now touches one place. --- src/remote/sshExtension.ts | 22 ++++++++++++++++++++++ src/remote/sshProcess.ts | 8 ++++++-- src/supportBundle/files.ts | 2 +- src/supportBundle/logFiles.ts | 15 +++++++-------- 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/remote/sshExtension.ts b/src/remote/sshExtension.ts index 70ed849daa..eae3d5ca4b 100644 --- a/src/remote/sshExtension.ts +++ b/src/remote/sshExtension.ts @@ -10,6 +10,28 @@ export const REMOTE_SSH_EXTENSION_IDS = [ export type RemoteSshExtensionId = (typeof REMOTE_SSH_EXTENSION_IDS)[number]; +/** + * VS Code Remote-SSH log layout, shared by the live SSH monitor and the + * support-bundle collector so a future layout change updates one place. + */ +const OUTPUT_LOGGING_DIR_PREFIX = "output_logging_"; +const REMOTE_SSH_LOG_NAME_FRAGMENT = "Remote - SSH"; + +/** True if `dirName` is the exthost dir of a known Remote-SSH extension. */ +export function isRemoteSshExtensionDir(dirName: string): boolean { + return (REMOTE_SSH_EXTENSION_IDS as readonly string[]).includes(dirName); +} + +/** True if `dirName` is a VS Code shared output channel dir. */ +export function isOutputLoggingDir(dirName: string): boolean { + return dirName.startsWith(OUTPUT_LOGGING_DIR_PREFIX); +} + +/** True if `fileName` is the Remote-SSH log inside a shared output channel. */ +export function isSharedChannelRemoteSshLog(fileName: string): boolean { + return fileName.includes(REMOTE_SSH_LOG_NAME_FRAGMENT); +} + type RemoteSshExtension = vscode.Extension & { id: RemoteSshExtensionId; }; diff --git a/src/remote/sshProcess.ts b/src/remote/sshProcess.ts index 77b1d68a60..58d0fee497 100644 --- a/src/remote/sshProcess.ts +++ b/src/remote/sshProcess.ts @@ -8,6 +8,10 @@ import { findPort } from "../util"; import { cleanupFiles } from "../util/fileCleanup"; import { NetworkStatusReporter } from "./networkStatus"; +import { + isOutputLoggingDir, + isSharedChannelRemoteSshLog, +} from "./sshExtension"; import type { Logger } from "../logging/logger"; import type { TelemetryReporter } from "../telemetry/reporter"; @@ -514,7 +518,7 @@ async function findRemoteSshLogPath( try { const dirs = await fs.readdir(logsParentDir); const outputDirs = dirs - .filter((d) => d.startsWith("output_logging_")) + .filter(isOutputLoggingDir) .sort((a, b) => a.localeCompare(b)) .reverse(); @@ -541,6 +545,6 @@ async function findRemoteSshLogPath( async function findSshLogInDir(dirPath: string): Promise { const files = await fs.readdir(dirPath); - const remoteSshLog = files.find((f) => f.includes("Remote - SSH")); + const remoteSshLog = files.find(isSharedChannelRemoteSshLog); return remoteSshLog ? path.join(dirPath, remoteSshLog) : undefined; } diff --git a/src/supportBundle/files.ts b/src/supportBundle/files.ts index 4614a6fba2..dcf3bf1c9c 100644 --- a/src/supportBundle/files.ts +++ b/src/supportBundle/files.ts @@ -46,7 +46,7 @@ function isEnoent(error: unknown): boolean { ); } -// lstat rejects symlinks so a planted link can't pull host files in. +/** Read a file with an lstat guard that rejects symlinks. */ export async function readLogFile( filePath: string, logger: Logger, diff --git a/src/supportBundle/logFiles.ts b/src/supportBundle/logFiles.ts index 2e195ba082..288fb30249 100644 --- a/src/supportBundle/logFiles.ts +++ b/src/supportBundle/logFiles.ts @@ -1,7 +1,11 @@ import * as path from "node:path"; import { type Logger } from "../logging/logger"; -import { REMOTE_SSH_EXTENSION_IDS } from "../remote/sshExtension"; +import { + isOutputLoggingDir, + isRemoteSshExtensionDir, + isSharedChannelRemoteSshLog, +} from "../remote/sshExtension"; import { addFiles, @@ -42,16 +46,11 @@ function isRemoteSshLog(relativePath: string, fileName: string): boolean { } const parts = normalizeZipPath(relativePath).split("/"); // Whole exthost dir belongs to one extension; output_logging_* is shared. - if ( - parts.some((part) => - (REMOTE_SSH_EXTENSION_IDS as readonly string[]).includes(part), - ) - ) { + if (parts.some(isRemoteSshExtensionDir)) { return true; } return ( - parts.some((part) => part.startsWith("output_logging_")) && - fileName.includes("Remote - SSH") + parts.some(isOutputLoggingDir) && isSharedChannelRemoteSshLog(fileName) ); } From 7e8a79d2bf2f6ecbe5fc1d03222ab34663f84277 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 26 May 2026 20:18:24 +0300 Subject: [PATCH 5/5] fix(supportBundle): cap log size and stop redacting defaults Address PR #971 review findings: - CRF-12: tail-truncate log files over 50 MB in readLogFile to prevent OOM during rezip when a long-lived session has a multi-GB debug log - CRF-13: passthrough `defaultValue` in maybeRedact so package.json defaults (public, no user data) stay visible for triage --- src/supportBundle/files.ts | 31 +++++++++++++++++++++--- src/supportBundle/settings.ts | 10 ++++++-- test/unit/supportBundle/files.test.ts | 17 +++++++++++++ test/unit/supportBundle/settings.test.ts | 2 +- 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/supportBundle/files.ts b/src/supportBundle/files.ts index dcf3bf1c9c..39f7e09788 100644 --- a/src/supportBundle/files.ts +++ b/src/supportBundle/files.ts @@ -10,10 +10,11 @@ export interface CollectedFile { relativePath: string; } -// 3 days is enough context for recent issues; matching the 7-day -// rotation would bloat the bundle. +/** 3-day window; the 7-day rotation would bloat the bundle. */ const LOG_MAX_AGE_MS = 3 * 24 * 60 * 60 * 1000; const MAX_LOG_SCAN_DEPTH = 6; +/** Per-file cap to prevent OOM on multi-GB debug logs. */ +const MAX_LOG_FILE_BYTES = 50 * 1024 * 1024; // Accept .log and VS Code's rotated .log.N form. export const isLogFile = (name: string): boolean => /\.log(\.\d+)?$/.test(name); @@ -46,7 +47,22 @@ function isEnoent(error: unknown): boolean { ); } -/** Read a file with an lstat guard that rejects symlinks. */ +/** Read the last `length` bytes of `filePath`. */ +async function readTail(filePath: string, length: number): Promise { + const handle = await fs.open(filePath, "r"); + try { + const { size } = await handle.stat(); + const offset = Math.max(0, size - length); + const readLen = Math.min(length, size); + const buf = Buffer.alloc(readLen); + const { bytesRead } = await handle.read(buf, 0, readLen, offset); + return buf.subarray(0, bytesRead); + } finally { + await handle.close(); + } +} + +/** lstat-guarded read; tail-truncates files over `MAX_LOG_FILE_BYTES`. */ export async function readLogFile( filePath: string, logger: Logger, @@ -56,6 +72,15 @@ export async function readLogFile( if (!stat.isFile()) { return undefined; } + if (stat.size > MAX_LOG_FILE_BYTES) { + logger.warn( + `Truncating log file ${filePath} (${stat.size} bytes) to last ${MAX_LOG_FILE_BYTES} bytes`, + ); + return { + data: await readTail(filePath, MAX_LOG_FILE_BYTES), + mtimeMs: stat.mtimeMs, + }; + } return { data: await fs.readFile(filePath), mtimeMs: stat.mtimeMs }; } catch (error) { logger.warn(`Could not read log file ${filePath}`, error); diff --git a/src/supportBundle/settings.ts b/src/supportBundle/settings.ts index 82aa7269ad..85b1693078 100644 --- a/src/supportBundle/settings.ts +++ b/src/supportBundle/settings.ts @@ -57,13 +57,19 @@ function redactedSettingValue(value: SettingValue): string { : ""; } +/** inspect() metadata + public package.json default; not user-supplied. */ +const REDACTION_PASSTHROUGH: ReadonlySet = new Set([ + "key", + "languageIds", + "defaultValue", +]); + function maybeRedact( key: string, name: string, value: SettingValue, ): SettingValue { - // `key` and `languageIds` are inspect() metadata, not the secret payload. - if (name === "key" || name === "languageIds") { + if (REDACTION_PASSTHROUGH.has(name)) { return value; } return REDACTED_SETTINGS.has(key) ? redactedSettingValue(value) : value; diff --git a/test/unit/supportBundle/files.test.ts b/test/unit/supportBundle/files.test.ts index ea167397a5..e8c7fac078 100644 --- a/test/unit/supportBundle/files.test.ts +++ b/test/unit/supportBundle/files.test.ts @@ -10,6 +10,7 @@ import { isLogFile, normalizeZipPath, prefixFiles, + readLogFile, } from "@/supportBundle/files"; import { createMockLogger } from "../../mocks/testHelpers"; @@ -88,6 +89,22 @@ describe("support bundle file helpers", () => { ]); }); + it("truncates oversized log files to the tail", async () => { + const filePath = path.join(tmpDir, "huge.log"); + const head = Buffer.alloc(60 * 1024 * 1024, "H"); + const tail = Buffer.from("TAIL_MARKER\n"); + await fs.writeFile(filePath, Buffer.concat([head, tail])); + + const result = await readLogFile(filePath, logger); + + expect(result?.data.byteLength).toBe(50 * 1024 * 1024); + expect( + Buffer.from(result?.data ?? new Uint8Array()) + .subarray(-tail.byteLength) + .toString(), + ).toBe("TAIL_MARKER\n"); + }); + it.runIf(process.platform !== "win32")( "does not follow symlinks when reading files", async () => { diff --git a/test/unit/supportBundle/settings.test.ts b/test/unit/supportBundle/settings.test.ts index e4b5be600a..76fd0ccacf 100644 --- a/test/unit/supportBundle/settings.test.ts +++ b/test/unit/supportBundle/settings.test.ts @@ -79,7 +79,7 @@ describe("collectSettingsFile", () => { expect(raw).not.toContain("DO_NOT_LEAK_SECRET"); expect(settings["coder.headerCommand"]).toEqual({ - defaultValue: "", + defaultValue: "", effective: "", globalValue: "", key: "coder.headerCommand",