diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a42a5cac..827c72f4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,31 @@ from published versions since it shows up in the VS Code extension changelog tab and is confusing to users. Add it back between releases if needed. --> +## Unreleased + +### Added + +- Add a **Coder: Open Generated SSH Configuration File** command that opens + this editor's generated workspace hosts file in a read-only editor. + +### Changed + +- Write workspace SSH hosts to an editor-owned file in the extension's global + storage and include it from the top of your SSH config, instead of writing + the block into your config directly. Each VS Code-based editor gets its own + include and SSH host prefix, so VS Code, Cursor, Windsurf, and other clones + no longer overwrite one another's proxy settings, and legacy `coder-vscode` + authorities are migrated in other editors by automatically reopening the + window once. SSH uses the first value it finds for each option, so a + catch-all `Host *` can no longer override the connection's `ProxyCommand`. + +### Fixed + +- Ignore the SSH config file setting on Antigravity and Windsurf/Devin. They + launch ssh without pointing it at a config file, so it always reads + `~/.ssh/config`, and honoring the setting wrote the workspace host where the + connection never looked. + ## [v1.16.0](https://github.com/coder/vscode-coder/releases/tag/v1.16.0) 2026-08-06 ### Added diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 66a9ef5ec..77de98169 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,33 +14,58 @@ The `ssh-remote` scheme is registered by Microsoft's Remote - SSH extension and indicates that it should connect to the provided host name using SSH. The host name takes the format -`coder-vscode.----`. This is parsed by the CLI -(which is invoked via SSH's `ProxyCommand`) to route SSH to the right workspace. +`coder-.----`, where `` comes from +that product's URI scheme, such as `vscode`, `cursor`, or `windsurf`. The CLI is +invoked through SSH's `ProxyCommand` with this prefix so it can route SSH to the +right workspace. A legacy `coder-vscode` authority opened in another editor is +reopened once with that editor's prefix; legacy recent-folder entries remain +compatible when opening the same workspace. The Coder Remote extension also registers for the `onResolveRemoteAuthority:ssh-remote` [extension activation event](https://code.visualstudio.com/api/references/activation-events) to hook into this process, running before the Remote - SSH extension actually connects. -On activation of this event, we check if `vscode.workspace.workspaceFolders` -contains the `coder-vscode` prefix, and if so we delay activation to: +On activation of this event, we check whether the remote authority belongs to +the current editor, and if so we delay activation to: 1. Parse the host name to get the domain, username, and workspace. 2. Ensure the workspace is running. 3. Download the matching server binary to the client. 4. Configure the binary with the URL and token, asking the user for them if they are missing. Each domain gets its own config directory. -5. Add an entry to the user's SSH config for `coder-vscode.--*`. +5. Write an entry for `coder-.--*` to `ssh-config` in the + extension's global storage directory. +6. Add a per-editor `Include` block at the top of the user's SSH config. ```text -Host coder-vscode.dev.coder.com--* - ProxyCommand "/tmp/coder" --global-config "/home/kyle/.config/Code/User/globalStorage/coder.coder-remote/dev.coder.com" ssh --stdio --network-info-dir "/home/kyle/.config/Code/User/globalStorage/coder.coder-remote/net" --ssh-host-prefix coder-vscode.dev.coder.com-- %h - ConnectTimeout 0 - StrictHostKeyChecking no - UserKnownHostsFile /dev/null - LogLevel ERROR +# --- START CODER cursor --- +Include "~/.config/Cursor/User/globalStorage/coder.coder-remote/ssh-config" +# --- END CODER cursor --- + +# --- START CODER vscode --- +Include "~/.config/Code/User/globalStorage/coder.coder-remote/ssh-config" +# --- END CODER vscode --- +``` + +Each included file contains only that editor's host entries: + +```text +Host coder-cursor.dev.coder.com--* + ProxyCommand "/tmp/coder" --global-config "/home/kyle/.config/Cursor/User/globalStorage/coder.coder-remote/dev.coder.com" ssh --stdio --network-info-dir "/home/kyle/.config/Cursor/User/globalStorage/coder.coder-remote/net" --ssh-host-prefix coder-cursor.dev.coder.com-- %h + ConnectTimeout 0 + StrictHostKeyChecking no + UserKnownHostsFile /dev/null + LogLevel ERROR ``` +Which main file gains the include depends on the Remote - SSH extension. +Microsoft's and Cursor's pass `remote.SSH.configFile` to ssh with `-F`, and +VSCodium's parses the file itself instead of running ssh, so all three connect +through it. Antigravity and Windsurf/Devin renamed the setting but spawn ssh +without `-F`, so ssh reads `~/.ssh/config` regardless; we ignore the renamed +setting there rather than add the include where the connection never looks. + If any step fails, we show an error message. Once the error message is closed we close the remote so the Remote - SSH connection does not continue to connection. Otherwise, we yield, which lets the Remote - SSH continue. diff --git a/eslint.config.mjs b/eslint.config.mjs index 5ea6d07ab..062201e43 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -123,6 +123,12 @@ export default defineConfig( message: "Do not use registerCommand('coder.*', ...) directly. Use the CommandManager class instead.", }, + { + selector: + "MemberExpression[property.name='remoteAuthority'][object.property.name='env'][object.object.name='vscode']", + message: + "env.remoteAuthority is a proposed API (resolvers) and throws through our own vscode module. Read it via vscodeProposed.env.remoteAuthority.", + }, ], }, }, diff --git a/package.json b/package.json index e534270e7..c6b1f4537 100644 --- a/package.json +++ b/package.json @@ -478,12 +478,20 @@ }, { "command": "coder.viewLogs", - "title": "Coder: View Logs", + "title": "View Logs", + "category": "Coder", "icon": "$(list-unordered)" }, + { + "command": "coder.openSshConfig", + "title": "Open Generated SSH Configuration File", + "category": "Coder", + "icon": "$(file-code)" + }, { "command": "coder.exportTelemetry", - "title": "Coder: Export Telemetry", + "title": "Export Telemetry", + "category": "Coder", "icon": "$(save)" }, { @@ -616,6 +624,10 @@ "command": "coder.viewLogs", "when": "true" }, + { + "command": "coder.openSshConfig", + "when": "true" + }, { "command": "coder.exportTelemetry", "when": "true" diff --git a/src/commands.ts b/src/commands.ts index 8c38b4231..4bab9e9fa 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -50,7 +50,11 @@ import { toRemoteLogGlobs, } from "./supportBundle/remoteServerDataPath"; import { runExportTelemetryCommand } from "./telemetry/export/command"; -import { toRemoteAuthority } from "./util/authority"; +import { + isRemoteAuthorityCompatible, + parseRemoteAuthority, + toRemoteAuthority, +} from "./util/authority"; import { openInBrowser, toSafeHost } from "./util/uri"; import { vscodeProposed } from "./vscodeProposed"; import { parseNetcheckReport } from "./webviews/netcheck/types"; @@ -83,6 +87,9 @@ import type { PongMessage, } from "./workspace/duplicateWorkspaceIpc"; +const NO_SSH_CONFIG_MESSAGE = + "No SSH config has been generated yet. It is written when you connect to a workspace."; + interface OpenOptions { workspaceOwner?: string; workspaceName?: string; @@ -565,6 +572,56 @@ export class Commands { ); } + /** Open this editor's generated SSH config, picking a deployment when several exist. */ + public async openSshConfig(): Promise { + const hostname = await this.pickSshHostname(); + if (!hostname) { + return; + } + try { + await openFile(this.pathResolver.getSshConfigPath(hostname)); + } catch { + vscode.window.showInformationMessage(NO_SSH_CONFIG_MESSAGE); + return; + } + // The file is rewritten on every connection, so edits would be lost. + await vscode.commands.executeCommand( + "workbench.action.files.setActiveEditorReadonlyInSession", + ); + } + + /** A connected window resolves to its own deployment; otherwise ask. */ + private async pickSshHostname(): Promise { + try { + // remoteAuthority is a proposed API; our own vscode module may not read it. + const remoteAuthority = vscodeProposed.env.remoteAuthority; + if (remoteAuthority) { + const parts = parseRemoteAuthority(remoteAuthority); + if (parts) { + return parts.safeHostname; + } + } + } catch { + // Malformed Coder authority or unavailable API; fall through to the picker. + } + const hostnames = ( + await readdirOrEmpty(this.pathResolver.getSshConfigDir()) + ) + .map((file) => this.pathResolver.parseSshConfigFile(file)) + .filter((name) => name !== undefined); + if (hostnames.length === 0) { + vscode.window.showInformationMessage(NO_SSH_CONFIG_MESSAGE); + return undefined; + } + if (hostnames.length === 1) { + return hostnames[0]; + } + return vscode.window.showQuickPick(hostnames, { + title: "Open generated SSH configuration", + placeHolder: "Select a deployment", + }); + } + /** * View the logs for the currently connected workspace. */ @@ -1475,12 +1532,11 @@ export class Commands { const output: { workspaces: Array<{ folderUri: vscode.Uri; remoteAuthority: string }>; } = await vscode.commands.executeCommand("_workbench.getRecentlyOpened"); - const opened = output.workspaces.filter( - // Remove recents that do not belong to this connection. The remote - // authority maps to a workspace/agent combination (using the SSH host - // name). There may also be some legacy connections that still may - // reference a workspace without an agent name, which will be missed. - (opened) => opened.folderUri?.authority === remoteAuthority, + const opened = output.workspaces.filter((opened) => + isRemoteAuthorityCompatible( + opened.folderUri?.authority, + remoteAuthority, + ), ); // openRecent will always use the most recent. Otherwise, if there are // multiple we ask the user which to use. diff --git a/src/core/commandManager.ts b/src/core/commandManager.ts index c9f721c3c..6868b3121 100644 --- a/src/core/commandManager.ts +++ b/src/core/commandManager.ts @@ -20,6 +20,7 @@ export const CODER_COMMAND_IDS = [ "coder.navigateToWorkspaceSettings", "coder.refreshWorkspaces", "coder.viewLogs", + "coder.openSshConfig", "coder.exportTelemetry", "coder.viewAnnouncements", "coder.searchMyWorkspaces", diff --git a/src/core/pathResolver.ts b/src/core/pathResolver.ts index 191534ba6..ddd862f41 100644 --- a/src/core/pathResolver.ts +++ b/src/core/pathResolver.ts @@ -1,7 +1,28 @@ +import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { expandPath } from "../util"; +import { currentEditorId } from "../util/authority"; + +/** Extension of generated SSH config files; the include glob matches on it. */ +export const SSH_CONFIG_EXT = ".conf"; + +/** The per-user data dir of the platform, shared by every editor. */ +function platformDataDir(): string { + switch (process.platform) { + case "win32": + return ( + process.env.APPDATA || path.join(os.homedir(), "AppData", "Roaming") + ); + case "darwin": + return path.join(os.homedir(), "Library", "Application Support"); + default: + return ( + process.env.XDG_DATA_HOME || path.join(os.homedir(), ".local", "share") + ); + } +} export class PathResolver { constructor( @@ -42,6 +63,30 @@ export class PathResolver { return path.join(this.basePath, "net"); } + /** + * Directory of generated SSH configs, glob-included from the user's config. + * Lives in the platform data dir so every editor emits the same include. + */ + public getSshConfigDir(): string { + return path.join(platformDataDir(), "coder.coder-remote", "ssh"); + } + + /** This editor's generated SSH config for one deployment. */ + public getSshConfigPath(safeHostname: string): string { + return path.join( + this.getSshConfigDir(), + `${currentEditorId()}--${safeHostname}${SSH_CONFIG_EXT}`, + ); + } + + /** The deployment hostname if this editor generated the file, else undefined. */ + public parseSshConfigFile(fileName: string): string | undefined { + const prefix = `${currentEditorId()}--`; + return fileName.startsWith(prefix) && fileName.endsWith(SSH_CONFIG_EXT) + ? fileName.slice(prefix.length, -SSH_CONFIG_EXT.length) + : undefined; + } + /** * Return the directory where telemetry files are written. */ diff --git a/src/extension.ts b/src/extension.ts index 6dc82209e..67dd15672 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -350,6 +350,10 @@ async function doActivate( void allWorkspacesProvider.fetchAndRefresh(); }); commandManager.register("coder.viewLogs", commands.viewLogs.bind(commands)); + commandManager.register( + "coder.openSshConfig", + commands.openSshConfig.bind(commands), + ); commandManager.register( "coder.exportTelemetry", commands.exportTelemetry.bind(commands), diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 0c29a0f5d..534fa64ae 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -39,9 +39,11 @@ import { import { getHeaderCommand } from "../settings/headers"; import { escapeCommandArg, expandPath } from "../util"; import { - AuthorityPrefix, type AuthorityParts, + classifyRemoteAuthority, parseRemoteAuthority, + retargetRemoteAuthority, + toCurrentAuthorityHostPrefix, } from "../util/authority"; import { createStatusBarItem } from "../util/statusBar"; import { vscodeProposed } from "../vscodeProposed"; @@ -52,17 +54,17 @@ import { migrateAuthToSecretsStorage } from "./migration"; import { SshConfig, type SshValues, + cleanupStaleSshConfigs, mergeSshConfigValues, parseCoderSshOptions, parseSshConfig, validateDeploymentSshOptions, } from "./sshConfig"; -import { getRemoteSshSetting } from "./sshExtension"; +import { getRemoteSshConfigFile } from "./sshExtension"; import { applySettingOverrides, buildSshOverrides } from "./sshOverrides"; import { SshProcessMonitor } from "./sshProcess"; import { computeSshProperties, - findSshPropertyProblems, sshSupportsSetEnv, type SshProperties, } from "./sshSupport"; @@ -160,6 +162,13 @@ export class Remote { return; } + // parseRemoteAuthority returned null for foreign hosts, so this is + // either the current editor's authority or a migratable legacy one. + if (classifyRemoteAuthority(parts) === "legacy") { + await this.migrateLegacyAuthority(remoteAuthority, startupMode); + return; + } + this.logger.info("Setting up remote connection", { remoteAuthority, hostname: parts.safeHostname, @@ -718,6 +727,66 @@ export class Remote { return undefined; } + private async migrateLegacyAuthority( + remoteAuthority: string, + startupMode: StartupMode, + ): Promise { + const migratedAuthority = retargetRemoteAuthority(remoteAuthority); + const workspaceFile = vscode.workspace.workspaceFile; + const workspaceFolders = vscode.workspace.workspaceFolders ?? []; + const savedWorkspaceFile = + workspaceFile?.scheme === "untitled" ? undefined : workspaceFile; + if (!savedWorkspaceFile && workspaceFolders.length > 1) { + this.logger.warn( + "Cannot migrate an unsaved multi-root workspace", + remoteAuthority, + ); + const choice = await vscodeProposed.window.showWarningMessage( + "Opening the remote over the old coder-vscode SSH host", + { + modal: true, + useCustom: true, + detail: + "This editor now uses its own SSH hosts, but switching an unsaved multi-root workspace would drop its folders. " + + "To switch, save the workspace, then reload the window.", + }, + "Learn More", + ); + if (choice === "Learn More") { + await vscode.env.openExternal( + vscode.Uri.parse( + "https://code.visualstudio.com/docs/editing/workspaces/multi-root-workspaces", + ), + ); + } + return; + } + + await this.serviceContainer + .getMementoManager() + .setStartupMode(startupMode === "none" ? "start" : startupMode); + this.logger.info("Migrating legacy remote authority", { + from: remoteAuthority, + to: migratedAuthority, + }); + + const currentUri = savedWorkspaceFile ?? workspaceFolders[0]?.uri; + if (currentUri) { + await vscode.commands.executeCommand( + "vscode.openFolder", + currentUri.with({ + authority: retargetRemoteAuthority(currentUri.authority), + }), + false, + ); + return; + } + await vscode.commands.executeCommand("vscode.newWindow", { + remoteAuthority: migratedAuthority, + reuseWindow: true, + }); + } + private async resolveRemoteBinary(workspaceClient: Api): Promise { if ( this.extensionContext.extensionMode === vscode.ExtensionMode.Production @@ -881,8 +950,8 @@ export class Remote { return ["--log-dir", escapeCommandArg(logDir), "-v"]; } - private getSshConfigPath(): string { - const configured = getRemoteSshSetting("configFile"); + private getMainSshConfigPath(): string { + const configured = getRemoteSshConfigFile(); return expandPath(configured || path.join("~", ".ssh", "config")); } @@ -897,19 +966,23 @@ export class Remote { featureSet: FeatureSet, cliAuth: CliAuth, ): Promise { - const sshConfigFile = this.getSshConfigPath(); - - const sshConfig = new SshConfig(sshConfigFile, this.logger); + // One file per (editor, deployment); the user's config gains one shared include. + const sshConfig = new SshConfig(this.getMainSshConfigPath(), this.logger); await sshConfig.load(); + // Never loaded: update() regenerates it without reading the old content. + const coderConfig = new SshConfig( + this.pathResolver.getSshConfigPath(safeHostname), + this.logger, + ); // Options the user set themselves win the merge below, so they are exempt // from the deny list. Both sources are local and already trusted: whoever - // can write the SSH config could write any Host block directly, and the - // post-write check below still pins the critical options. + // can write the SSH config could write any Host block directly. const userConfigSsh = vscode.workspace .getConfiguration("coder") .get("sshConfig", []); const userConfig = parseSshConfig(userConfigSsh); + // The CLI writes its block to the user's config, so read it from there. const configSshOptions = parseCoderSshOptions(sshConfig.getRaw()); let deploymentSshConfig = {}; @@ -944,9 +1017,7 @@ export class Remote { userConfig, ); - const hostPrefix = safeHostname - ? `${AuthorityPrefix}.${safeHostname}--` - : `${AuthorityPrefix}--`; + const hostPrefix = toCurrentAuthorityHostPrefix(safeHostname); const proxyCommand = await this.buildProxyCommand( binaryPath, @@ -973,51 +1044,19 @@ export class Remote { sshValues.SetEnv = "CODER_SSH_SESSION_TYPE=vscode"; } - await sshConfig.update(safeHostname, sshValues, sshConfigOverrides); + // Write our file before including it, so the include never dangles. + await coderConfig.update(sshValues, sshConfigOverrides); + const sharedSshConfigDir = this.pathResolver.getSshConfigDir(); + await sshConfig.updateInclude(sharedSshConfigDir, safeHostname); + // Our file was just written, so only other unused deployments are swept. + // Never throws, and the connection does not depend on it. + void cleanupStaleSshConfigs(sharedSshConfigDir, this.logger); - // A user can provide a "Host *" entry in their SSH config to add options - // to all hosts. We need to ensure that the options we set are not - // overridden by the user's config. - const computedProperties = computeSshProperties( + // Mirror SSH's parse order; RemoteCommand can come from the user's config. + return computeSshProperties( hostName, - sshConfig.getRaw(), + `${coderConfig.getRaw()}\n${sshConfig.getRaw()}`, ); - const problems = findSshPropertyProblems(computedProperties, { - ProxyCommand: sshValues.ProxyCommand, - UserKnownHostsFile: sshValues.UserKnownHostsFile, - StrictHostKeyChecking: sshValues.StrictHostKeyChecking, - }); - if (problems.length > 0) { - await this.failSshConfigCheck(hostName, problems); - } - - return computedProperties; - } - - /** Show every unexpected option at once, close the remote, and abort. */ - private async failSshConfigCheck( - hostName: string, - problems: string[], - ): Promise { - const title = `Unexpected SSH Config Option${problems.length > 1 ? "s" : ""}`; - const detail = - `Your SSH config sets unexpected values for the "${hostName}" host. ` + - `Please fix the following and try again:\n\n` + - problems.map((problem) => `- ${problem}.`).join("\n"); - const result = await vscodeProposed.window.showErrorMessage( - title, - { - useCustom: true, - modal: true, - detail, - }, - "Reload Window", - ); - if (result === "Reload Window") { - await this.reloadWindow(); - } - await this.closeRemote(); - throw new Error("SSH config mismatch, closing remote"); } private watchSettings( diff --git a/src/remote/sshConfig.ts b/src/remote/sshConfig.ts index a1dfc095a..e162f06bf 100644 --- a/src/remote/sshConfig.ts +++ b/src/remote/sshConfig.ts @@ -6,13 +6,23 @@ import { unlink, writeFile, } from "node:fs/promises"; +import * as os from "node:os"; import path from "node:path"; +import { SSH_CONFIG_EXT } from "../core/pathResolver"; import { countSubstring, lowercase } from "../util"; +import { cleanupFiles } from "../util/fileCleanup"; import { renameWithRetry, tempFilePath } from "../util/fs"; import type { Logger } from "../logging/logger"; +class SshConfigBadFormat extends Error {} + +interface Block { + start: number; + end: number; +} + export interface SshValues { Host: string; ProxyCommand: string; @@ -25,7 +35,7 @@ export interface SshValues { SetEnv?: string; } -/** Interface for the file system to make it easier to test. */ +/** Injectable for tests. */ export interface FileSystem { mkdir: typeof mkdir; readFile: typeof readFile; @@ -44,16 +54,10 @@ const defaultFileSystem: FileSystem = { writeFile, }; -class SshConfigBadFormat extends Error {} - -interface Block { - raw: string; -} - -/** Matches an SSH config key at the start of a line (e.g. "ConnectTimeout", "LogLevel"). */ +/** An SSH config key at the start of a line, e.g. "ConnectTimeout". */ const SSH_KEY_REGEX = /^[a-zA-Z0-9-]+/; -/** Matches the Coder CLI's START-CODER / END-CODER block, flexible on dash count. */ +/** The Coder CLI's START-CODER block, flexible on dash count. */ const CODER_BLOCK_REGEX = /^# -+START-CODER-+$(.*?)^# -+END-CODER-+$/ms; /** Matches a string that is only an SSH config key. */ @@ -62,6 +66,45 @@ const KEY_ONLY_REGEX = /^[a-zA-Z0-9-]+$/; /** Characters that would break a value out of its config line. */ const UNSAFE_CHARS_REGEX = /[\r\n\0]/; +interface BlockMarkers { + start: string; + end: string; +} + +/** Released versions wrote deployment blocks with these markers into the user's config. */ +function legacyDeploymentMarkers(safeHostname: string): BlockMarkers { + return { + start: `# --- START CODER VSCODE ${safeHostname} ---`, + end: `# --- END CODER VSCODE ${safeHostname} ---`, + }; +} + +/** Shared include block; identical bytes from every editor, so writers converge. */ +const INCLUDE_MARKERS: BlockMarkers = { + start: "# --- START CODER ---", + end: "# --- END CODER ---", +}; + +/** Header of the generated per-deployment file. */ +const CODER_SSH_CONFIG_HEADER = `# Coder workspace hosts. Do not edit; the Coder extension rewrites this file +# on every connection. Override options with the "coder.sshConfig" setting.`; + +/** Connects rewrite the file, so anything older is unused and safe to sweep. */ +const STALE_CONFIG_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; + +/** Delete generated configs (from any editor) not connected to recently. */ +export async function cleanupStaleSshConfigs( + dir: string, + logger: Logger, +): Promise { + await cleanupFiles(dir, logger, { + label: "generated SSH config", + filter: (name) => name.endsWith(SSH_CONFIG_EXT), + select: (files, now) => + files.filter((file) => now - file.mtime > STALE_CONFIG_MAX_AGE_MS), + }); +} + /** * SSH options a deployment may not set, mirroring the server's validation of * --ssh-config-options (codersdk.ValidateSSHConfigOption). @@ -88,8 +131,8 @@ const DENIED_DEPLOYMENT_KEYS: ReadonlySet> = new Set([ ]); /** - * Options the post-write check in remote.ts pins to Coder's own value. Setting - * these in coder.sshConfig fails that check, so the error must not suggest it. + * Connection-critical options Coder always writes itself, so the error must + * not suggest overriding them in coder.sshConfig. */ const PINNED_KEYS: ReadonlySet> = new Set([ "proxycommand", @@ -178,30 +221,24 @@ export function parseCoderSshOptions(raw: string): Record { return parseSshConfig(sshOptionLines); } -/** - * Parse an array of SSH config lines into a Record. - * Handles both "Key value" and "Key=value" formats. - * Accumulates SetEnv values since SSH allows multiple environment variables. - */ +/** Parse "Key value" or "Key=value" lines, accumulating SetEnv values. */ export function parseSshConfig(lines: string[]): Record { return lines.reduce( (acc, line) => { const keyMatch = SSH_KEY_REGEX.exec(line); if (!keyMatch) { - return acc; // Malformed line + return acc; } const key = keyMatch[0]; const separator = line.at(key.length); if (separator !== "=" && separator !== " ") { - return acc; // Malformed line + return acc; } const value = line.slice(key.length + 1); - // Accumulate SetEnv values since there can be multiple. if (key.toLowerCase() === "setenv") { - // Ignore empty SetEnv values if (value !== "") { const existing = acc["SetEnv"]; acc["SetEnv"] = existing ? `${existing} ${value}` : value; @@ -215,20 +252,13 @@ export function parseSshConfig(lines: string[]): Record { ); } -/** - * Merge the given SSH config with the provided overrides. The merge handles - * key case insensitivity, so casing in the key does not matter. - */ +/** Merge overrides into config; keys match case-insensitively. */ export function mergeSshConfigValues( config: Record, overrides: Record, ): Record { const merged: Record = {}; - // We need to do a case insensitive match for the overrides as ssh config keys are case insensitive. - // To get the correct key:value, use: - // key = caseInsensitiveOverrides[key.toLowerCase()] - // value = overrides[key] const caseInsensitiveOverrides: Record = {}; Object.keys(overrides).forEach((key) => { caseInsensitiveOverrides[key.toLowerCase()] = key; @@ -236,13 +266,12 @@ export function mergeSshConfigValues( Object.keys(config).forEach((key) => { const lower = key.toLowerCase(); - // If the key is in overrides, use the override value. if (caseInsensitiveOverrides[lower]) { const correctCaseKey = caseInsensitiveOverrides[lower]; const value = overrides[correctCaseKey]; delete caseInsensitiveOverrides[lower]; - // Special handling for SetEnv - concatenate values instead of replacing. + // SetEnv concatenates instead of replacing. if (lower === "setenv") { if (value === "") { merged["SetEnv"] = config[key]; @@ -252,25 +281,22 @@ export function mergeSshConfigValues( return; } - // If the value is empty, do not add the key. It is being removed. + // An empty override removes the key. if (value !== "") { merged[correctCaseKey] = value; } return; } - // If no override, take the original value. if (config[key] !== "") { merged[key] = config[key]; } }); - // Add remaining overrides. Object.keys(caseInsensitiveOverrides).forEach((lower) => { const correctCaseKey = caseInsensitiveOverrides[lower]; const value = overrides[correctCaseKey]; - // Special handling for SetEnv - concatenate if already exists if (lower === "setenv" && merged["SetEnv"]) { merged["SetEnv"] = `${merged["SetEnv"]} ${value}`; } else { @@ -287,13 +313,6 @@ export class SshConfig { private readonly logger: Logger; private raw: string | undefined; - private startBlockComment(safeHostname: string): string { - return `# --- START CODER VSCODE ${safeHostname} ---`; - } - private endBlockComment(safeHostname: string): string { - return `# --- END CODER VSCODE ${safeHostname} ---`; - } - constructor( filePath: string, logger: Logger, @@ -305,170 +324,151 @@ export class SshConfig { } async load() { - try { - this.raw = await this.fileSystem.readFile(this.filePath, "utf-8"); - this.logger.debug("Loaded SSH config", this.filePath); - } catch { - this.logger.debug( - "SSH config file not found, starting fresh", - this.filePath, - ); - this.raw = ""; - } + this.raw = await this.read(); + this.logger.debug("Loaded SSH config", this.filePath); } /** - * Update the block for the deployment with the provided hostname. - * @throws {Error} when the hostname, values, or overrides fail validation. + * Regenerate the whole per-deployment file; last-writer-wins. Always + * writes, so the file's mtime marks the last connect for the stale sweep. */ - async update( - safeHostname: string, - values: SshValues, - overrides?: Record, - ) { - const newBlock = this.buildBlock(safeHostname, values, overrides); - const block = this.getBlock(safeHostname); - if (block) { - this.logger.debug("Replacing SSH config block", safeHostname); - this.replaceBlock(block, newBlock); - } else { - this.logger.debug("Appending new SSH config block", safeHostname); - this.appendBlock(newBlock); - } + async update(values: SshValues, overrides?: Record) { + const block = this.renderDeploymentBlock(values, overrides); + this.raw = `${CODER_SSH_CONFIG_HEADER}\n\n${block}`; await this.save(); } /** - * Get the block for the deployment with the provided hostname. + * Keep the shared include first so our options win, removing superseded + * blocks. Read-modify-write with no locking, like the CLI's config-ssh; + * when the include is already in place nothing is written. */ - private getBlock(safeHostname: string): Block | undefined { - const raw = this.getRaw(); - const startBlock = this.startBlockComment(safeHostname); - const endBlock = this.endBlockComment(safeHostname); - - const startBlockCount = countSubstring(startBlock, raw); - const endBlockCount = countSubstring(endBlock, raw); - if (startBlockCount !== endBlockCount) { - throw new SshConfigBadFormat( - `Malformed config: ${this.filePath} has an unterminated START CODER VSCODE ${safeHostname} block. Each START block must have an END block.`, - ); - } - - if (startBlockCount > 1 || endBlockCount > 1) { - throw new SshConfigBadFormat( - `Malformed config: ${this.filePath} has ${startBlockCount} START CODER VSCODE ${safeHostname} sections. Please remove all but one.`, - ); - } - - const startBlockIndex = raw.indexOf(startBlock); - const endBlockIndex = raw.indexOf(endBlock); - const hasBlock = startBlockIndex > -1 && endBlockIndex > -1; - if (!hasBlock) { - return; - } - - if (startBlockIndex === -1) { - throw new SshConfigBadFormat("Start block not found"); - } - - if (startBlockIndex === -1) { - throw new SshConfigBadFormat("End block not found"); + async updateInclude(includeDir: string, safeHostname: string) { + const block = this.renderIncludeBlock(includeDir); + const raw = await this.read(); + this.raw = this.mergeInclude(raw, block, safeHostname); + if (this.raw !== raw) { + await this.save(); + this.logger.debug("Including SSH config dir", includeDir); } + } - if (endBlockIndex < startBlockIndex) { - throw new SshConfigBadFormat( - "Malformed config, end block is before start block", - ); + public getRaw() { + if (this.raw === undefined) { + throw new Error("SshConfig is not loaded. Try sshConfig.load()"); } - return { - raw: raw.substring(startBlockIndex, endBlockIndex + endBlock.length), - }; + return this.raw; } /** - * buildBlock builds the ssh config block for the provided URL. The order of - * the keys is determinstic based on the input. Expected values are always in - * a consistent order followed by any additional overrides in sorted order. - * - * Validates everything written here, including the hostname, which lands in - * the block marker comments. - * - * @param safeHostname - The hostname for the deployment. - * @param values - The expected SSH values for using ssh with Coder. - * @param overrides - Overrides typically come from the deployment api and are - * used to override the default values. The overrides are - * given as key:value pairs where the key is the ssh config - * file key. If the key matches an expected value, the - * expected value is overridden. If it does not match an - * expected value, it is appended to the end of the block. + * Render the deployment's block, validating everything written into it. + * @throws {Error} when the values or overrides fail validation. */ - private buildBlock( - safeHostname: string, + private renderDeploymentBlock( values: SshValues, overrides?: Record, - ) { - validateSshValue("deployment hostname", safeHostname); + ): string { validateSshConfigOptions({ ...values }); validateSshConfigOptions(overrides ?? {}); - const { Host, ...otherValues } = values; - const lines = [ - this.startBlockComment(safeHostname), - "# This section is managed by the Coder VS Code extension.", - "# Changes will be overwritten on the next workspace connection.", - `Host ${Host}`, - ]; - - // configValues is the merged values of the defaults and the overrides. - const configValues = mergeSshConfigValues(otherValues, overrides ?? {}); - - // keys is the sorted keys of the merged values. - const keys = Object.keys(configValues).sort(); - keys.forEach((key) => { - const value = configValues[key]; - if (value !== "") { - lines.push(this.withIndentation(`${key} ${value}`)); - } - }); + const { Host, ...defaults } = values; + const config = mergeSshConfigValues(defaults, overrides ?? {}); + const options = Object.keys(config) + .sort() + .filter((key) => config[key] !== "") + .map((key) => ` ${key} ${config[key]}`); + return [`Host ${Host}`, ...options].join("\n"); + } + + private findBlock(raw: string, markers: BlockMarkers): Block | undefined { + const startCount = countSubstring(markers.start, raw); + const endCount = countSubstring(markers.end, raw); + if (startCount !== endCount) { + throw new SshConfigBadFormat( + `Malformed config: ${this.filePath} has ${startCount} "${markers.start}" and ${endCount} "${markers.end}" markers. Each START marker must have exactly one END marker.`, + ); + } + if (startCount > 1) { + throw new SshConfigBadFormat( + `Malformed config: ${this.filePath} has ${startCount} "${markers.start}" blocks. Please remove all but one.`, + ); + } + if (startCount === 0) { + return undefined; + } - lines.push(this.endBlockComment(safeHostname)); - return { - raw: lines.join("\n"), - }; + const start = raw.indexOf(markers.start); + const endMarkerStart = raw.indexOf(markers.end); + if (endMarkerStart < start) { + throw new SshConfigBadFormat( + `Malformed config: ${this.filePath} has an "${markers.end}" marker before its "${markers.start}" marker.`, + ); + } + return { start, end: endMarkerStart + markers.end.length }; } - private replaceBlock(oldBlock: Block, newBlock: Block) { - // A replacer function inserts $ sequences literally. - this.raw = this.getRaw().replace(oldBlock.raw, () => newBlock.raw); + private renderIncludeBlock(includeDir: string): string { + return [ + INCLUDE_MARKERS.start, + "# Moves back to the top on connect; override options via coder.sshConfig.", + `Include "${this.escapeIncludePath(includeDir)}/*${SSH_CONFIG_EXT}"`, + INCLUDE_MARKERS.end, + ].join("\n"); } - private appendBlock(block: Block) { - const raw = this.getRaw(); + private escapeIncludePath(includePath: string): string { + // Emit ~/... so home-path quirks (spaces, %, glob chars) never reach ssh. + const relative = path.relative(os.homedir(), includePath); + const argument = + relative && !relative.startsWith("..") && !path.isAbsolute(relative) + ? `~/${relative}` + : includePath; + // No escape exists for '"' in quoted arguments; OpenSSH 9.9+ fatals on + // unknown %-tokens in Include arguments. + if (/[\r\n\0"%]/.test(argument)) { + throw new Error( + "SSH include path must not contain CR, LF, NUL, %, or double-quote characters.", + ); + } + return argument.replaceAll("\\", "/").replace(/[*?[\]]/g, "\\$&"); + } - if (this.raw === "") { - this.raw = block.raw; - } else { - this.raw = `${raw.trimEnd()}\n\n${block.raw}`; + private mergeInclude( + raw: string, + includeBlock: string, + safeHostname: string, + ): string { + let rest = raw; + const superseded = [INCLUDE_MARKERS, legacyDeploymentMarkers(safeHostname)]; + for (const markers of superseded) { + const block = this.findBlock(rest, markers); + if (block) { + rest = this.removeRange(rest, block); + } } + return [includeBlock, rest].filter(Boolean).join("\n\n"); } - private withIndentation(text: string) { - return ` ${text}`; + private removeRange(raw: string, range: Block): string { + const before = raw.slice(0, range.start).trimEnd(); + const after = raw.slice(range.end).trimStart(); + return [before, after].filter(Boolean).join("\n\n"); } - private async save() { - // We want to preserve the original file mode. + /** Atomically write raw via a temp file. */ + private async save(): Promise { + // Preserve the existing file mode. const existingMode = await this.fileSystem .stat(this.filePath) .then((stat) => stat.mode) .catch((ex: NodeJS.ErrnoException) => { if (ex.code === "ENOENT") { - return 0o600; // default to 0600 if file does not exist + return 0o600; } - throw ex; // Any other error is unexpected + throw ex; }); await this.fileSystem.mkdir(path.dirname(this.filePath), { - mode: 0o700, // only owner has rwx permission, not group or everyone. + mode: 0o700, recursive: true, }); const fileName = path.basename(this.filePath); @@ -498,13 +498,7 @@ export class SshConfig { ); this.logger.debug("Saved SSH config", this.filePath); } catch (err) { - await this.fileSystem.unlink(tempPath).catch((unlinkErr: unknown) => { - this.logger.warn( - "Failed to clean up temp SSH config file", - tempPath, - unlinkErr, - ); - }); + await this.discardTemp(tempPath); throw new Error( `Failed to rename temporary SSH config file at ${tempPath} to ${this.filePath}: ${ err instanceof Error ? err.message : String(err) @@ -514,11 +508,24 @@ export class SshConfig { } } - public getRaw() { - if (this.raw === undefined) { - throw new Error("SshConfig is not loaded. Try sshConfig.load()"); - } + private async discardTemp(tempPath: string): Promise { + await this.fileSystem.unlink(tempPath).catch((unlinkErr: unknown) => { + this.logger.warn( + "Failed to clean up temp SSH config file", + tempPath, + unlinkErr, + ); + }); + } - return this.raw; + private async read(): Promise { + try { + return await this.fileSystem.readFile(this.filePath, "utf-8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + return ""; + } + throw error; + } } } diff --git a/src/remote/sshExtension.ts b/src/remote/sshExtension.ts index a56e98060..5aca747b8 100644 --- a/src/remote/sshExtension.ts +++ b/src/remote/sshExtension.ts @@ -11,31 +11,24 @@ export const REMOTE_SSH_EXTENSION_IDS = [ export type RemoteSshExtensionId = (typeof REMOTE_SSH_EXTENSION_IDS)[number]; /** - * Sections each extension reads, in order. The rebranded forks renamed the - * whole `remote.SSH` section, so reading it directly misses them. + * Extensions that spawn ssh without `-F`, so it always reads ~/.ssh/config + * and their renamed configFile setting never applies. */ -const SETTING_SECTIONS: Readonly< - Record -> = { - "jeanp413.open-remote-ssh": ["remote.SSH"], - // Windsurf became Devin and reads both, preferring the new name. - "codeium.windsurf-remote-openssh": ["remote.devinSSH", "remote.windsurfSSH"], - "anysphere.remote-ssh": ["remote.SSH"], - "ms-vscode-remote.remote-ssh": ["remote.SSH"], - "google.antigravity-remote-openssh": ["remote.antigravitySSH"], -}; +const IGNORED_CONFIG_FILE: readonly RemoteSshExtensionId[] = [ + "google.antigravity-remote-openssh", + "codeium.windsurf-remote-openssh", +]; -/** First non-empty value for a string setting, e.g. `configFile`. */ -export function getRemoteSshSetting(key: string): string | undefined { +/** The SSH config file the active extension connects through, if configured. */ +export function getRemoteSshConfigFile(): string | undefined { const id = getRemoteSshExtension()?.id; - const sections = id ? SETTING_SECTIONS[id] : ["remote.SSH"]; - for (const section of sections) { - const value = vscode.workspace.getConfiguration(section).get(key); - if (value) { - return value; - } + if (id && IGNORED_CONFIG_FILE.includes(id)) { + return undefined; } - return undefined; + return ( + vscode.workspace.getConfiguration("remote.SSH").get("configFile") || + undefined + ); } /** diff --git a/src/remote/sshSupport.ts b/src/remote/sshSupport.ts index 08f0b7fcb..b0e6f7d26 100644 --- a/src/remote/sshSupport.ts +++ b/src/remote/sshSupport.ts @@ -122,25 +122,3 @@ export function computeSshProperties( }); return merged; } - -/** - * Compare effective SSH properties against what Coder wrote, one problem per - * unexpected value. - */ -export function findSshPropertyProblems( - computed: SshProperties, - expected: Record, -): string[] { - const problems: string[] = []; - for (const [key, value] of Object.entries(expected)) { - const computedValue = computed[lowercase(key)]; - if (computedValue !== value) { - const actual = - computedValue === undefined - ? "is not set" - : `is set to "${computedValue}"`; - problems.push(`"${key}" ${actual}, but Coder expects "${value}"`); - } - } - return problems; -} diff --git a/src/util/authority.ts b/src/util/authority.ts index b99200636..bcccabaf1 100644 --- a/src/util/authority.ts +++ b/src/util/authority.ts @@ -1,5 +1,9 @@ +import * as vscode from "vscode"; + import { toSafeHost } from "./uri"; +export const LegacyAuthorityPrefix = "coder-vscode"; + export interface AuthorityParts { agent: string | undefined; sshHost: string; @@ -8,44 +12,94 @@ export interface AuthorityParts { workspace: string; } -// Prefix is a magic string that is prepended to SSH hosts to indicate that -// they should be handled by this extension. -export const AuthorityPrefix = "coder-vscode"; +export type AuthorityClassification = "current" | "legacy" | "foreign"; -const authorityHostPrefix = `${AuthorityPrefix}.`; +const sshRemotePrefix = "ssh-remote+"; const invalidAuthorityMessage = "Invalid Coder SSH authority. Must be: ----(.)"; +/** This editor's identity, keeping its SSH hosts and files apart from other editors'. */ +export function currentEditorId(): string { + const uriScheme = vscode.env.uriScheme; + if (!uriScheme) { + throw new Error("Editor URI scheme must not be empty."); + } + return uriScheme; +} + +function currentAuthorityPrefix(): string { + return `coder-${currentEditorId()}`; +} + +function getSshHostStart(authority: string): number | undefined { + if (authority.startsWith(sshRemotePrefix)) { + return sshRemotePrefix.length; + } + + for (const wrapper of [`@${sshRemotePrefix}`, `://${sshRemotePrefix}`]) { + const index = authority.lastIndexOf(wrapper); + if (index !== -1) { + return index + wrapper.length; + } + } + + return undefined; +} + +function classifySshHost(sshHost: string): AuthorityClassification { + const currentPrefix = currentAuthorityPrefix(); + if (sshHost.startsWith(`${currentPrefix}.`)) { + return "current"; + } + if ( + currentPrefix !== LegacyAuthorityPrefix && + sshHost.startsWith(`${LegacyAuthorityPrefix}.`) + ) { + return "legacy"; + } + // Anything else is foreign, including deployment-unaware hosts like + // coder-vscode--ws; their preserved config block still routes them. + return "foreign"; +} + +function authorityPrefix(classification: AuthorityClassification): string { + return classification === "legacy" + ? LegacyAuthorityPrefix + : currentAuthorityPrefix(); +} + /** * Given an authority, parse into the expected parts. * * The authority looks like `://ssh-remote+`, where the * SSH host names created by this extension match the format: - * coder-vscode.----(.) + * coder-.----(.) * * If this is not a Coder authority, return null. * * Throw an error if a Coder authority is invalid. */ export function parseRemoteAuthority(authority: string): AuthorityParts | null { - const authorityParts = authority.split("+"); - const sshHost = authorityParts[1]; - if (!sshHost) { + const sshHostStart = getSshHostStart(authority); + if (sshHostStart === undefined) { return null; } - const parts = sshHost.split("--"); - if (!parts[0].startsWith(authorityHostPrefix)) { + const sshHost = authority.slice(sshHostStart); + const classification = classifySshHost(sshHost); + if (classification === "foreign") { return null; } + // The classification guarantees the host starts with ".". + const prefix = `${authorityPrefix(classification)}.`; + const parts = sshHost.slice(prefix.length).split("--"); if (parts.length < 3) { throw new Error(invalidAuthorityMessage); } // Parse from the right because safe hostnames can contain "--". - const hostPrefix = parts.slice(0, -2).join("--"); - const safeHostname = hostPrefix.slice(authorityHostPrefix.length); + const safeHostname = parts.slice(0, -2).join("--"); const username = parts[parts.length - 2]; const workspaceAndAgent = parts[parts.length - 1]; if (!safeHostname || !username || !workspaceAndAgent) { @@ -73,15 +127,51 @@ export function parseRemoteAuthority(authority: string): AuthorityParts | null { }; } +export function classifyRemoteAuthority( + parts: AuthorityParts, +): AuthorityClassification { + return classifySshHost(parts.sshHost); +} + export function toRemoteAuthority( baseUrl: string, workspaceOwner: string, workspaceName: string, workspaceAgent: string | undefined, ): string { - let remoteAuthority = `ssh-remote+${AuthorityPrefix}.${toSafeHost(baseUrl)}--${workspaceOwner}--${workspaceName}`; + let remoteAuthority = `ssh-remote+${currentAuthorityPrefix()}.${toSafeHost(baseUrl)}--${workspaceOwner}--${workspaceName}`; if (workspaceAgent) { remoteAuthority += `.${workspaceAgent}`; } return remoteAuthority; } + +export function toCurrentAuthorityHostPrefix(safeHostname: string): string { + return `${currentAuthorityPrefix()}.${safeHostname}--`; +} + +export function retargetRemoteAuthority(authority: string): string { + const sshHostStart = getSshHostStart(authority); + if (sshHostStart === undefined) { + return authority; + } + + const sshHost = authority.slice(sshHostStart); + if (classifySshHost(sshHost) !== "legacy") { + return authority; + } + return `${authority.slice(0, sshHostStart)}${currentAuthorityPrefix()}${sshHost.slice(LegacyAuthorityPrefix.length)}`; +} + +export function isRemoteAuthorityCompatible( + authority: string | undefined, + targetAuthority: string, +): boolean { + if (!authority) { + return false; + } + return ( + authority === targetAuthority || + retargetRemoteAuthority(authority) === targetAuthority + ); +} diff --git a/test/mocks/testHelpers.ts b/test/mocks/testHelpers.ts index f89a0a316..e886d0a12 100644 --- a/test/mocks/testHelpers.ts +++ b/test/mocks/testHelpers.ts @@ -25,12 +25,15 @@ import type { IncomingMessage } from "node:http"; import type { AgentMetadataEvent } from "@/api/api-helper"; import type { CoderApi } from "@/api/coderApi"; import type { CliCredentialManager } from "@/core/cliCredentialManager"; +import type { CliManager } from "@/core/cliManager"; import type { ServiceContainer } from "@/core/container"; import type { ContextManager } from "@/core/contextManager"; import type { MementoManager } from "@/core/mementoManager"; +import type { PathResolver } from "@/core/pathResolver"; import type { SecretsManager } from "@/core/secretsManager"; import type { Deployment } from "@/deployment/types"; import type { Logger } from "@/logging/logger"; +import type { LoginCoordinator } from "@/login/loginCoordinator"; import type { NetworkInfo } from "@/remote/sshProcess"; import type { TelemetryService } from "@/telemetry/service"; import type { @@ -50,6 +53,10 @@ interface ContextManagerLike { dispose(): void; } +interface LoginCoordinatorLike { + ensureLoggedInWithDialog: LoginCoordinator["ensureLoggedInWithDialog"]; +} + export function makeNetworkInfo( overrides: Partial = {}, ): NetworkInfo { @@ -249,6 +256,7 @@ export interface MessageCall { */ export class MockUserInteraction { private readonly responses = new Map(); + private readonly patternResponses: Array<[RegExp, string | undefined]> = []; private readonly _messageCalls: MessageCall[] = []; private inputBoxValue: string | undefined; private inputBoxValidateInput: ((value: string) => Promise) | undefined; @@ -259,10 +267,14 @@ export class MockUserInteraction { } /** - * Set a response for a specific message dialog + * Set a response for a message dialog, matched exactly or by pattern. */ - setResponse(message: string, response: string | undefined): void { - this.responses.set(message, response); + setResponse(message: string | RegExp, response: string | undefined): void { + if (typeof message === "string") { + this.responses.set(message, response); + } else { + this.patternResponses.push([message, response]); + } } /** @@ -307,6 +319,7 @@ export class MockUserInteraction { */ clear(): void { this.responses.clear(); + this.patternResponses.length = 0; this._messageCalls.length = 0; this.inputBoxValue = undefined; this.inputBoxValidateInput = undefined; @@ -318,7 +331,12 @@ export class MockUserInteraction { */ private setupVSCodeMock(): void { const getResponse = (message: string): string | undefined => { - return this.responses.get(message); + if (this.responses.has(message)) { + return this.responses.get(message); + } + return this.patternResponses.find(([pattern]) => + pattern.test(message), + )?.[1]; }; const handleMessage = @@ -581,7 +599,10 @@ export function createMockServiceContainer( secretsManager?: SecretsManager; mementoManager?: MementoManager; cliCredentialManager?: CliCredentialManager; + cliManager?: CliManager; + pathResolver?: PathResolver; contextManager?: ContextManagerLike; + loginCoordinator?: LoginCoordinatorLike; } = {}, ): ServiceContainer { const telemetry = overrides.telemetry ?? createTestTelemetryService(); @@ -601,8 +622,12 @@ export function createMockServiceContainer( require("mementoManager", overrides.mementoManager), getCliCredentialManager: () => require("cliCredentialManager", overrides.cliCredentialManager), + getCliManager: () => require("cliManager", overrides.cliManager), + getPathResolver: () => require("pathResolver", overrides.pathResolver), getContextManager: () => require("contextManager", overrides.contextManager) as ContextManager, + getLoginCoordinator: () => + require("loginCoordinator", overrides.loginCoordinator) as LoginCoordinator, } as ServiceContainer; } diff --git a/test/mocks/vscode.runtime.ts b/test/mocks/vscode.runtime.ts index 55cd482e7..252ae68af 100644 --- a/test/mocks/vscode.runtime.ts +++ b/test/mocks/vscode.runtime.ts @@ -86,6 +86,9 @@ export class Uri { constructor( public scheme: string, public path: string, + public authority = "", + public query = "", + public fragment = "", ) {} get fsPath(): string { return this.path; @@ -93,27 +96,61 @@ export class Uri { static file(p: string) { return new Uri("file", p); } - static parse(v: string) { - if (v.startsWith("file://")) { - return Uri.file(v.slice("file://".length)); + static from(components: { + scheme: string; + path?: string; + authority?: string; + query?: string; + fragment?: string; + }) { + return new Uri( + components.scheme, + components.path ?? "", + components.authority ?? "", + components.query ?? "", + components.fragment ?? "", + ); + } + with(change: { + scheme?: string; + path?: string; + authority?: string; + query?: string; + fragment?: string; + }) { + return new Uri( + change.scheme ?? this.scheme, + change.path ?? this.path, + change.authority ?? this.authority, + change.query ?? this.query, + change.fragment ?? this.fragment, + ); + } + static parse(value: string) { + if (value.startsWith("file://")) { + return Uri.file(value.slice("file://".length)); } - const [scheme, ...rest] = v.split(":"); + const [scheme, ...rest] = value.split(":"); return new Uri(scheme, rest.join(":")); } toString() { - return this.scheme === "file" - ? `file://${this.path}` - : `${this.scheme}:${this.path}`; + if (!this.authority && !this.query && !this.fragment) { + return this.scheme === "file" + ? `file://${this.path}` + : `${this.scheme}:${this.path}`; + } + const authority = this.authority ? `//${this.authority}` : ""; + const query = this.query ? `?${this.query}` : ""; + const fragment = this.fragment ? `#${this.fragment}` : ""; + return `${this.scheme}:${authority}${this.path}${query}${fragment}`; } static joinPath(base: Uri, ...paths: string[]) { - // Mirror vscode-uri: collapse slashes at the seams while preserving the - // leading "//" that separates the authority from the path. const head = base.path.replace(/\/+$/, ""); const tail = paths .map((p) => p.replace(/^\/+|\/+$/g, "")) .filter(Boolean) .join("/"); - return new Uri(base.scheme, tail ? `${head}/${tail}` : head); + return base.with({ path: tail ? `${head}/${tail}` : head }); } } @@ -181,6 +218,7 @@ export const commands = { export const workspace = { getConfiguration: vi.fn(), // your helpers override this workspaceFolders: [] as unknown[], + workspaceFile: undefined as Uri | undefined, fs: { readFile: vi.fn(), writeFile: vi.fn(), diff --git a/test/unit/core/pathResolver.test.ts b/test/unit/core/pathResolver.test.ts index b3191e6d3..a6117dc56 100644 --- a/test/unit/core/pathResolver.test.ts +++ b/test/unit/core/pathResolver.test.ts @@ -1,5 +1,6 @@ +import * as os from "node:os"; import * as path from "path"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { PathResolver } from "@/core/pathResolver"; @@ -38,6 +39,107 @@ describe("PathResolver", () => { }); }); + describe("getSshConfigDir", () => { + const realPlatform = process.platform; + afterEach(() => { + Object.defineProperty(process, "platform", { value: realPlatform }); + }); + + interface SharedDirCase { + name: string; + platform: NodeJS.Platform; + env: Record; + expected: string; + } + it.each([ + { + name: "XDG_DATA_HOME on Linux", + platform: "linux", + env: { XDG_DATA_HOME: "/xdg/data" }, + expected: path.join("/xdg/data", "coder.coder-remote", "ssh"), + }, + { + name: "the XDG default on Linux", + platform: "linux", + env: { XDG_DATA_HOME: "" }, + expected: path.join( + os.homedir(), + ".local", + "share", + "coder.coder-remote", + "ssh", + ), + }, + { + name: "Application Support on macOS", + platform: "darwin", + env: {}, + expected: path.join( + os.homedir(), + "Library", + "Application Support", + "coder.coder-remote", + "ssh", + ), + }, + { + name: "APPDATA on Windows", + platform: "win32", + env: { + APPDATA: path.join("C:", "Users", "jane", "AppData", "Roaming"), + }, + expected: path.join( + "C:", + "Users", + "jane", + "AppData", + "Roaming", + "coder.coder-remote", + "ssh", + ), + }, + { + name: "the profile default on Windows", + platform: "win32", + env: { APPDATA: "" }, + expected: path.join( + os.homedir(), + "AppData", + "Roaming", + "coder.coder-remote", + "ssh", + ), + }, + ])("uses $name", ({ platform, env, expected }) => { + Object.defineProperty(process, "platform", { value: platform }); + for (const [key, value] of Object.entries(env)) { + vi.stubEnv(key, value); + } + expectPathsEqual(pathResolver.getSshConfigDir(), expected); + }); + }); + + describe("getSshConfigPath", () => { + it("names the file after the editor and deployment", () => { + expectPathsEqual( + pathResolver.getSshConfigPath("dev.coder.com"), + path.join(pathResolver.getSshConfigDir(), "vscode--dev.coder.com.conf"), + ); + }); + + it("parses the hostname only from this editor's generated files", () => { + expect( + pathResolver.parseSshConfigFile("vscode--dev.coder.com.conf"), + ).toBe("dev.coder.com"); + expect( + pathResolver.parseSshConfigFile("cursor--dev.coder.com.conf"), + ).toBeUndefined(); + expect( + pathResolver.parseSshConfigFile("vscode--notes.txt"), + ).toBeUndefined(); + }); + }); + describe("getProxyLogPath", () => { const defaultLogPath = path.join(basePath, "log"); diff --git a/test/unit/remote/remote.test.ts b/test/unit/remote/remote.test.ts index 40bc7e0af..8b3b18a97 100644 --- a/test/unit/remote/remote.test.ts +++ b/test/unit/remote/remote.test.ts @@ -1,5 +1,6 @@ import { vol } from "memfs"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; import { MementoManager } from "@/core/mementoManager"; import { PathResolver } from "@/core/pathResolver"; @@ -9,31 +10,41 @@ import { Remote } from "@/remote/remote"; import { createTestTelemetryService } from "../../mocks/telemetry"; import { createMockLogger, + createMockServiceContainer, InMemoryMemento, InMemorySecretStorage, LogCollector, MockConfigurationProvider, + MockUserInteraction, } from "../../mocks/testHelpers"; -import type * as vscode from "vscode"; - import type { Commands } from "@/commands"; import type { CliManager } from "@/core/cliManager"; -import type { ServiceContainer } from "@/core/container"; -import type { ContextManager } from "@/core/contextManager"; import type { Logger } from "@/logging/logger"; -import type { LoginCoordinator } from "@/login/loginCoordinator"; + +const mockWorkspace = vscode.workspace as typeof vscode.workspace & { + workspaceFile: vscode.Uri | undefined; + workspaceFolders: vscode.WorkspaceFolder[]; +}; +const mockEnv = vscode.env as typeof vscode.env & { uriScheme: string }; vi.mock("node:fs/promises", async () => (await import("memfs")).fs.promises); const SAFE_HOSTNAME = "coder.example.com"; const REMOTE_AUTHORITY = "ssh-remote+coder-vscode.coder.example.com--testuser--test-workspace.main"; +const CURSOR_REMOTE_AUTHORITY = + "ssh-remote+coder-cursor.coder.example.com--testuser--test-workspace.main"; +const WINDSURF_REMOTE_AUTHORITY = + "ssh-remote+coder-windsurf.coder.example.com--testuser--test-workspace.main"; +const REMOTE_SSH_EXTENSION_ID = "anysphere.remote-ssh"; const MISMATCHED_URL = "https://cursor.example.com/private?token=sensitive-url-token"; const SESSION_TOKEN = "sensitive-session-token"; function createRemote(logger: Logger = createMockLogger()) { + new MockConfigurationProvider(); + const userInteraction = new MockUserInteraction(); const pathResolver = new PathResolver("/mock/global", "/mock/log"); vol.fromJSON({ [pathResolver.getUrlPath(SAFE_HOSTNAME)]: MISMATCHED_URL, @@ -47,16 +58,21 @@ function createRemote(logger: Logger = createMockLogger()) { const ensureLoggedInWithDialog = vi .fn() .mockResolvedValue({ success: false, reason: "user_dismissed" }); - const serviceContainer = { - getLogger: () => logger, - getPathResolver: () => pathResolver, - getCliManager: () => ({}) as CliManager, - getContextManager: () => ({}) as ContextManager, - getSecretsManager: () => secretsManager, - getLoginCoordinator: () => - ({ ensureLoggedInWithDialog }) as unknown as LoginCoordinator, - getTelemetryService: () => createTestTelemetryService(), - } as ServiceContainer; + const mementoManager = new MementoManager(new InMemoryMemento()); + const serviceContainer = createMockServiceContainer({ + logger, + pathResolver, + mementoManager, + cliManager: {} as CliManager, + contextManager: { + set: vi.fn(), + get: vi.fn(() => false), + dispose: vi.fn(), + }, + secretsManager, + loginCoordinator: { ensureLoggedInWithDialog }, + telemetry: createTestTelemetryService(), + }); return { remote: new Remote( @@ -64,15 +80,194 @@ function createRemote(logger: Logger = createMockLogger()) { {} as Commands, {} as vscode.ExtensionContext, ), + ensureLoggedInWithDialog, + mementoManager, secretsManager, + userInteraction, }; } describe("Remote", () => { beforeEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); vol.reset(); - new MockConfigurationProvider(); + mockWorkspace.workspaceFile = undefined; + mockWorkspace.workspaceFolders = []; + mockEnv.uriScheme = "vscode"; + }); + + type UriOptions = Partial< + Pick + >; + const createUri = (path: string, options: UriOptions = {}) => + vscode.Uri.from({ + scheme: options.scheme ?? "cursor-remote", + authority: options.authority ?? REMOTE_AUTHORITY, + path, + query: options.query, + fragment: options.fragment, + }); + const setWorkspace = ( + folders: vscode.Uri[] = [], + workspaceFile?: vscode.Uri, + ) => { + mockWorkspace.workspaceFile = workspaceFile; + mockWorkspace.workspaceFolders = folders.map( + (uri) => ({ uri }) as vscode.WorkspaceFolder, + ); + return mockWorkspace.workspaceFolders; + }; + + it("migrates a legacy folder with its full URI", async () => { + mockEnv.uriScheme = "cursor"; + const { remote, mementoManager } = createRemote(); + setWorkspace([ + createUri("/workspace", { + query: "window=active", + fragment: "selection", + }), + ]); + + await expect( + remote.setup(REMOTE_AUTHORITY, "none", REMOTE_SSH_EXTENSION_ID), + ).resolves.toBeUndefined(); + + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + "vscode.openFolder", + createUri("/workspace", { + authority: CURSOR_REMOTE_AUTHORITY, + query: "window=active", + fragment: "selection", + }), + false, + ); + expect(await mementoManager.getAndClearStartupMode()).toBe("start"); + }); + + it("migrates a saved multi-root workspace file", async () => { + mockEnv.uriScheme = "cursor"; + const { remote, mementoManager } = createRemote(); + setWorkspace( + [createUri("/first-folder"), createUri("/second-folder")], + createUri("/project.code-workspace", { + query: "window=active", + fragment: "selection", + }), + ); + + await expect( + remote.setup(REMOTE_AUTHORITY, "none", REMOTE_SSH_EXTENSION_ID), + ).resolves.toBeUndefined(); + + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + "vscode.openFolder", + createUri("/project.code-workspace", { + authority: CURSOR_REMOTE_AUTHORITY, + query: "window=active", + fragment: "selection", + }), + false, + ); + expect(await mementoManager.getAndClearStartupMode()).toBe("start"); + }); + + interface EmptyWindowMigrationCase { + startupMode: "none" | "start" | "update"; + expectedStartupMode: "start" | "update"; + } + it.each([ + { startupMode: "none", expectedStartupMode: "start" }, + { startupMode: "start", expectedStartupMode: "start" }, + { startupMode: "update", expectedStartupMode: "update" }, + ])( + "migrates an empty window and preserves $startupMode startup mode", + async ({ startupMode, expectedStartupMode }) => { + mockEnv.uriScheme = "cursor"; + const { remote, mementoManager } = createRemote(); + + await expect( + remote.setup(REMOTE_AUTHORITY, startupMode, REMOTE_SSH_EXTENSION_ID), + ).resolves.toBeUndefined(); + + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + "vscode.newWindow", + { + remoteAuthority: CURSOR_REMOTE_AUTHORITY, + reuseWindow: true, + }, + ); + expect(await mementoManager.getAndClearStartupMode()).toBe( + expectedStartupMode, + ); + }, + ); + + it.each([ + { choice: undefined, docsUrls: [] }, + { + choice: "Learn More", + docsUrls: [expect.stringContaining("multi-root-workspaces")], + }, + ])( + "keeps an untitled multi-root workspace on the old host (choice: $choice)", + async ({ choice, docsUrls }) => { + mockEnv.uriScheme = "cursor"; + const { remote, mementoManager, userInteraction } = createRemote(); + setWorkspace( + [createUri("/first-folder"), createUri("/second-folder")], + createUri("/Untitled-1.code-workspace", { + scheme: "untitled", + authority: "", + }), + ); + userInteraction.setResponse(/coder-vscode SSH host/, choice); + + await expect( + remote.setup(REMOTE_AUTHORITY, "update", REMOTE_SSH_EXTENSION_ID), + ).resolves.toBeUndefined(); + + const warning = userInteraction + .getMessageCalls() + .find((call) => call.level === "warning"); + expect(warning?.message).toContain("coder-vscode SSH host"); + expect(warning?.items).toEqual(["Learn More"]); + expect(userInteraction.getExternalUrls()).toEqual(docsUrls); + expect(vscode.commands.executeCommand).not.toHaveBeenCalled(); + expect(await mementoManager.getAndClearStartupMode()).toBe("none"); + }, + ); + + it("continues setup for the current authority without reopening", async () => { + mockEnv.uriScheme = "cursor"; + const { remote, ensureLoggedInWithDialog } = createRemote(); + + await expect( + remote.setup(CURSOR_REMOTE_AUTHORITY, "none", REMOTE_SSH_EXTENSION_ID), + ).resolves.toBeUndefined(); + + expect(ensureLoggedInWithDialog).toHaveBeenCalledOnce(); + expect(vscode.commands.executeCommand).not.toHaveBeenCalledWith( + "vscode.openFolder", + expect.anything(), + expect.anything(), + ); + expect(vscode.commands.executeCommand).not.toHaveBeenCalledWith( + "vscode.newWindow", + expect.anything(), + ); + }); + + it("ignores a foreign authority", async () => { + mockEnv.uriScheme = "cursor"; + const { remote, ensureLoggedInWithDialog, mementoManager } = createRemote(); + + await expect( + remote.setup(WINDSURF_REMOTE_AUTHORITY, "none", REMOTE_SSH_EXTENSION_ID), + ).resolves.toBeUndefined(); + + expect(ensureLoggedInWithDialog).not.toHaveBeenCalled(); + expect(vscode.commands.executeCommand).not.toHaveBeenCalled(); + expect(await mementoManager.getAndClearStartupMode()).toBe("none"); }); it("ignores mismatched file auth and logs why", async () => { @@ -80,7 +275,7 @@ describe("Remote", () => { const { remote, secretsManager } = createRemote(logs); await expect( - remote.setup(REMOTE_AUTHORITY, "none", "anysphere.remote-ssh"), + remote.setup(REMOTE_AUTHORITY, "none", REMOTE_SSH_EXTENSION_ID), ).resolves.toBeUndefined(); expect(await secretsManager.getSessionAuth(SAFE_HOSTNAME)).toBeUndefined(); diff --git a/test/unit/remote/sshConfig.openssh.test.ts b/test/unit/remote/sshConfig.openssh.test.ts new file mode 100644 index 000000000..b8ffaf7d1 --- /dev/null +++ b/test/unit/remote/sshConfig.openssh.test.ts @@ -0,0 +1,144 @@ +import { execFile, spawnSync } from "node:child_process"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { promisify } from "node:util"; +import { afterEach, describe, expect, it } from "vitest"; + +import { SshConfig, type SshValues } from "@/remote/sshConfig"; + +import { createMockLogger } from "../../mocks/testHelpers"; + +const run = promisify(execFile); +const sshAvailable = !spawnSync("ssh", ["-V"]).error; +// Generous for slow CI runners; each ssh -G call is local and fast. +const TEST_TIMEOUT_MS = 30_000; + +const sshValues = ( + hostname: string, + proxyCommand: string, + editor = "vscode", +): SshValues => ({ + Host: `coder-${editor}.${hostname}--*`, + ProxyCommand: proxyCommand, + ConnectTimeout: "0", + StrictHostKeyChecking: "no", + UserKnownHostsFile: "/dev/null", + LogLevel: "ERROR", + ServerAliveInterval: "10", + ServerAliveCountMax: "3", +}); + +let tempDir: string | undefined; + +afterEach(async () => { + if (tempDir) { + await fs.rm(tempDir, { recursive: true, force: true }); + tempDir = undefined; + } +}); + +/** Real files and the real ssh binary; only the editor plumbing is absent. */ +async function createFixture(includeDirName: string) { + // Windows tmpdir paths use 8.3 short names ssh cannot glob; the product + // writes under the home directory anyway, emitted as a ~/... include. + const root = process.platform === "win32" ? os.homedir() : os.tmpdir(); + tempDir = await fs.mkdtemp(path.join(root, "coder-ssh-test-")); + const logger = createMockLogger(); + const includeDir = path.join(tempDir, includeDirName); + const userConfigPath = path.join(tempDir, "config"); + + return { + includeDir, + /** What the extension does on connect: write our file, then include it. */ + async connect(hostname: string, proxyCommand: string, editor = "vscode") { + const coderConfig = new SshConfig( + path.join(includeDir, `${editor}--${hostname}.conf`), + logger, + ); + await coderConfig.update(sshValues(hostname, proxyCommand, editor)); + const userConfig = new SshConfig(userConfigPath, logger); + await userConfig.load(); + await userConfig.updateInclude(includeDir, hostname); + }, + async seedUserConfig(contents: string) { + await fs.writeFile(userConfigPath, contents); + }, + async appendUserConfig(contents: string) { + await fs.appendFile(userConfigPath, contents); + }, + /** The host's effective options according to the real ssh. */ + async resolve(host: string) { + const { stdout } = await run("ssh", ["-G", "-F", userConfigPath, host], { + timeout: 10_000, + }); + return stdout; + }, + }; +} + +describe.skipIf(!sshAvailable)("include resolution by real OpenSSH", () => { + it( + "prefers the included file over a stale block and a later Host *", + async () => { + const ssh = await createFixture("Code Dir"); + await ssh.seedUserConfig( + [ + "# --- START CODER VSCODE dev.coder.com ---", + "Host coder-vscode.dev.coder.com--*", + " ProxyCommand echo stale-wins", + "# --- END CODER VSCODE dev.coder.com ---", + "", + ].join("\n"), + ); + await ssh.connect("dev.coder.com", "echo dev-wins"); + await ssh.connect("eu.coder.com", "echo eu-wins"); + await ssh.connect("dev.coder.com", "echo cursor-wins", "cursor"); + await ssh.appendUserConfig( + "\nHost *\n ProxyCommand echo user-wins\n ConnectTimeout 9\n", + ); + + const resolved = await ssh.resolve( + "coder-vscode.dev.coder.com--user--ws", + ); + expect(resolved).toContain("proxycommand echo dev-wins"); + expect(resolved).toContain("connecttimeout 0"); + expect(resolved).not.toContain("stale-wins"); + expect( + await ssh.resolve("coder-vscode.eu.coder.com--user--ws"), + ).toContain("proxycommand echo eu-wins"); + expect( + await ssh.resolve("coder-cursor.dev.coder.com--user--ws"), + ).toContain("proxycommand echo cursor-wins"); + }, + TEST_TIMEOUT_MS, + ); + + // Brackets are legal in file names on every platform, unlike * and ?. + it( + "honors escaped glob characters in the include path", + async () => { + const ssh = await createFixture("we[i]rd [dir]"); + await ssh.connect("dev.coder.com", "echo dev-wins"); + expect( + await ssh.resolve("coder-vscode.dev.coder.com--user--ws"), + ).toContain("proxycommand echo dev-wins"); + }, + TEST_TIMEOUT_MS, + ); + + it( + "keeps ssh working when the included directory is deleted", + async () => { + const ssh = await createFixture("Code Dir"); + await ssh.connect("dev.coder.com", "echo dev-wins"); + await ssh.appendUserConfig("\nHost *\n ProxyCommand echo user-wins\n"); + await fs.rm(ssh.includeDir, { recursive: true }); + + expect( + await ssh.resolve("coder-vscode.dev.coder.com--user--ws"), + ).toContain("proxycommand echo user-wins"); + }, + TEST_TIMEOUT_MS, + ); +}); diff --git a/test/unit/remote/sshConfig.test.ts b/test/unit/remote/sshConfig.test.ts index 8f875fbad..797a7fbd6 100644 --- a/test/unit/remote/sshConfig.test.ts +++ b/test/unit/remote/sshConfig.test.ts @@ -1,35 +1,31 @@ -import { it, afterEach, vi, expect, describe, beforeEach } from "vitest"; +import { vol } from "memfs"; +import * as fsPromises from "node:fs/promises"; +import * as os from "node:os"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { - SshConfig, + cleanupStaleSshConfigs, + mergeSshConfigValues, parseCoderSshOptions, parseSshConfig, - mergeSshConfigValues, - validateDeploymentSshOptions, + SshConfig, type SshValues, + validateDeploymentSshOptions, } from "@/remote/sshConfig"; import { createMockLogger } from "../../mocks/testHelpers"; -// This is not the usual path to ~/.ssh/config, but -// setting it to a different path makes it easier to test -// and makes mistakes abundantly clear. -const sshFilePath = "/Path/To/UserHomeDir/.sshConfigDir/sshConfigFile"; -const sshTempFilePrefix = - "/Path/To/UserHomeDir/.sshConfigDir/.sshConfigFile.vscode-coder-tmp-"; -const managedHeader = `# This section is managed by the Coder VS Code extension. -# Changes will be overwritten on the next workspace connection.`; - -const mockFileSystem = { - mkdir: vi.fn(), - readFile: vi.fn(), - rename: vi.fn(), - stat: vi.fn(), - unlink: vi.fn().mockResolvedValue(undefined), - writeFile: vi.fn(), -}; +vi.mock("node:fs/promises", async () => (await import("memfs")).fs.promises); +vi.mock("node:os", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, homedir: vi.fn(() => "/Path/To/UserHomeDir") }; +}); -const mockLogger = createMockLogger(); +const homeDir = "/Path/To/UserHomeDir"; +const sshFilePath = "/Path/To/UserHomeDir/.sshConfigDir/sshConfigFile"; +const hostname = "dev.coder.com"; +const fileHeader = `# Coder workspace hosts. Do not edit; the Coder extension rewrites this file +# on every connection. Override options with the "coder.sshConfig" setting.`; const BASE_SSH_VALUES = { Host: "coder-vscode.dev.coder.com--*", @@ -54,528 +50,138 @@ const BENIGN_DEPLOYMENT_OPTIONS = { serveraliveinterval: "5", } as const; -afterEach(() => { - vi.clearAllMocks(); -}); - -it("creates a new file and adds the config", async () => { - mockFileSystem.readFile.mockRejectedValueOnce("No file found"); - mockFileSystem.stat.mockRejectedValueOnce({ code: "ENOENT" }); - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); - await sshConfig.update("dev.coder.com", BASE_SSH_VALUES); - - const expectedOutput = `# --- START CODER VSCODE dev.coder.com --- -${managedHeader} -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - ServerAliveCountMax 3 - ServerAliveInterval 10 - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com ---`; - - expect(mockFileSystem.readFile).toHaveBeenCalledWith( - sshFilePath, - expect.anything(), - ); - expect(mockFileSystem.writeFile).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - expectedOutput, - expect.objectContaining({ - encoding: "utf-8", - mode: 0o600, // Default mode for new files. - }), - ); - expect(mockFileSystem.rename).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - sshFilePath, - ); -}); - -it("adds a new coder config in an existent SSH configuration", async () => { - const existentSSHConfig = `Host coder.something - ConnectTimeout=0 - LogLevel ERROR - HostName coder.something - ProxyCommand command - StrictHostKeyChecking=no - UserKnownHostsFile=/dev/null`; - mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig); - mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o644 }); - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); - await sshConfig.update("dev.coder.com", BASE_SSH_VALUES); - - const expectedOutput = `${existentSSHConfig} - -# --- START CODER VSCODE dev.coder.com --- -${managedHeader} -Host coder-vscode.dev.coder.com--* +const deploymentBlock = `Host coder-vscode.dev.coder.com--* ConnectTimeout 0 LogLevel ERROR ProxyCommand some-command-here ServerAliveCountMax 3 ServerAliveInterval 10 StrictHostKeyChecking no - UserKnownHostsFile /dev/null + UserKnownHostsFile /dev/null`; +// Released versions wrote deployment blocks with the VSCODE label. +const legacyDeploymentBlock = `# --- START CODER VSCODE dev.coder.com --- +Host stale # --- END CODER VSCODE dev.coder.com ---`; - - expect(mockFileSystem.writeFile).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - expectedOutput, - { - encoding: "utf-8", - mode: 0o644, - }, - ); - expect(mockFileSystem.rename).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - sshFilePath, - ); -}); - -it("updates an existent coder config", async () => { - const keepSSHConfig = `Host coder.something - HostName coder.something - ConnectTimeout=0 - StrictHostKeyChecking=no - UserKnownHostsFile=/dev/null - LogLevel ERROR - ProxyCommand command - -# --- START CODER VSCODE dev2.coder.com --- -Host coder-vscode.dev2.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev2.coder.com ---`; - - const existentSSHConfig = `${keepSSHConfig} - -# --- START CODER VSCODE dev.coder.com --- -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com --- - -Host * - SetEnv TEST=1`; - mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig); - mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o644 }); - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); - await sshConfig.update("dev.coder.com", { - ...BASE_SSH_VALUES, - Host: "coder-vscode.dev-updated.coder.com--*", - ProxyCommand: "some-updated-command-here", - ConnectTimeout: "1", - StrictHostKeyChecking: "yes", - }); - - const expectedOutput = `${keepSSHConfig} - -# --- START CODER VSCODE dev.coder.com --- -${managedHeader} -Host coder-vscode.dev-updated.coder.com--* - ConnectTimeout 1 - LogLevel ERROR - ProxyCommand some-updated-command-here - ServerAliveCountMax 3 - ServerAliveInterval 10 - StrictHostKeyChecking yes - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com --- - -Host * - SetEnv TEST=1`; - - expect(mockFileSystem.writeFile).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - expectedOutput, - { - encoding: "utf-8", - mode: 0o644, - }, - ); - expect(mockFileSystem.rename).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - sshFilePath, - ); -}); - -it("does not remove deployment-unaware SSH config and adds the new one", async () => { - // Before the plugin supported multiple deployments, it would only write and - // overwrite this one block. We need to leave it alone so existing - // connections keep working. Only replace blocks specific to the deployment - // that we are targeting. Going forward, all new connections will use the new - // deployment-specific block. - const existentSSHConfig = `# --- START CODER VSCODE --- +const legacyOtherDeploymentBlock = `# --- START CODER VSCODE other.coder.com --- +Host coder-vscode.other.coder.com--* +# --- END CODER VSCODE other.coder.com ---`; +const deploymentUnawareBlock = `# --- START CODER VSCODE --- Host coder-vscode--* - ConnectTimeout=0 - HostName coder.something - LogLevel ERROR - ProxyCommand command - StrictHostKeyChecking=no - UserKnownHostsFile=/dev/null # --- END CODER VSCODE ---`; - mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig); - mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o644 }); - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); - await sshConfig.update("dev.coder.com", BASE_SSH_VALUES); - - const expectedOutput = `${existentSSHConfig} +const includeDir = "~/.ssh/coder"; -# --- START CODER VSCODE dev.coder.com --- -${managedHeader} -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - ServerAliveCountMax 3 - ServerAliveInterval 10 - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com ---`; +function renderIncludeBlock(dir: string): string { + return `# --- START CODER --- +# Moves back to the top on connect; override options via coder.sshConfig. +Include "${dir}/*.conf" +# --- END CODER ---`; +} - expect(mockFileSystem.writeFile).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - expectedOutput, - { - encoding: "utf-8", - mode: 0o644, - }, - ); - expect(mockFileSystem.rename).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - sshFilePath, - ); -}); +const includeBlock = renderIncludeBlock(includeDir); -it("it does not remove a user-added block that only matches the host of an old coder SSH config", async () => { - const existentSSHConfig = `Host coder-vscode--* - ForwardAgent=yes`; - mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig); - mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o644 }); - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); - await sshConfig.update("dev.coder.com", BASE_SSH_VALUES); - - const expectedOutput = `Host coder-vscode--* - ForwardAgent=yes - -# --- START CODER VSCODE dev.coder.com --- -${managedHeader} -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - ServerAliveCountMax 3 - ServerAliveInterval 10 - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com ---`; - - expect(mockFileSystem.writeFile).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - expectedOutput, - { - encoding: "utf-8", - mode: 0o644, - }, - ); - expect(mockFileSystem.rename).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - sshFilePath, - ); -}); - -it("throws an error if there is a missing end block", async () => { - // The below config is missing an end block. - // This is a malformed config and should throw an error. - const existentSSHConfig = `Host beforeconfig - HostName before.config.tld - User before - -# --- START CODER VSCODE dev.coder.com --- -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null +const mockLogger = createMockLogger(); -Host afterconfig - HostName after.config.tld - User after`; +const readConfig = () => fsPromises.readFile(sshFilePath, "utf-8"); - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig); +async function loadSshConfig( + contents?: string, + mode = 0o644, +): Promise { + if (contents !== undefined) { + vol.fromJSON({ [sshFilePath]: contents }); + vol.chmodSync(sshFilePath, mode); + } + const sshConfig = new SshConfig(sshFilePath, mockLogger, fsPromises); await sshConfig.load(); - - // When we try to update the config, it should throw an error. - await expect( - sshConfig.update("dev.coder.com", BASE_SSH_VALUES), - ).rejects.toThrow( - `Malformed config: ${sshFilePath} has an unterminated START CODER VSCODE dev.coder.com block. Each START block must have an END block.`, - ); + return sshConfig; +} + +async function updateDeployment( + contents?: string, + values: SshValues = BASE_SSH_VALUES, + overrides?: Record, +): Promise { + const sshConfig = await loadSshConfig(contents); + await sshConfig.update(values, overrides); +} + +async function updateInclude( + contents: string, + dir: string = includeDir, +): Promise { + const sshConfig = await loadSshConfig(contents); + await sshConfig.updateInclude(dir, hostname); +} + +beforeEach(() => { + vol.reset(); + vi.mocked(os.homedir).mockReturnValue(homeDir); }); -it("throws an error if there is a mismatched start and end block count", async () => { - // The below config contains two start blocks and one end block. - // This is a malformed config and should throw an error. - // Previously were were simply taking the first occurrences of the start and - // end blocks, which would potentially lead to loss of any content between the - // missing end block and the next start block. - const existentSSHConfig = `Host beforeconfig - HostName before.config.tld - User before - -# --- START CODER VSCODE dev.coder.com --- -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# missing END CODER VSCODE dev.coder.com --- - -Host donotdelete - HostName dont.delete.me - User please - -# --- START CODER VSCODE dev.coder.com --- -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com --- - -Host afterconfig - HostName after.config.tld - User after`; - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig); - await sshConfig.load(); - - // When we try to update the config, it should throw an error. - await expect( - sshConfig.update("dev.coder.com", BASE_SSH_VALUES), - ).rejects.toThrow( - `Malformed config: ${sshFilePath} has an unterminated START CODER VSCODE dev.coder.com block. Each START block must have an END block.`, - ); +afterEach(() => { + vi.restoreAllMocks(); }); -it("throws an error if there are more than one sections with the same label", async () => { - const existentSSHConfig = `Host beforeconfig - HostName before.config.tld - User before - -# --- START CODER VSCODE dev.coder.com --- -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com --- - -Host donotdelete - HostName dont.delete.me - User please - -# --- START CODER VSCODE dev.coder.com --- -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com --- - -Host afterconfig - HostName after.config.tld - User after`; - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig); - await sshConfig.load(); - - // When we try to update the config, it should throw an error. - await expect( - sshConfig.update("dev.coder.com", BASE_SSH_VALUES), - ).rejects.toThrow( - `Malformed config: ${sshFilePath} has 2 START CODER VSCODE dev.coder.com sections. Please remove all but one.`, - ); +describe("SshConfig.getRaw", () => { + it("throws before load", () => { + const sshConfig = new SshConfig(sshFilePath, mockLogger, fsPromises); + expect(() => sshConfig.getRaw()).toThrow("SshConfig is not loaded"); + }); }); -it("correctly handles interspersed blocks with and without label", async () => { - const existentSSHConfig = `Host beforeconfig - HostName before.config.tld - User before - -# --- START CODER VSCODE --- -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE --- - -Host donotdelete - HostName dont.delete.me - User please - -# --- START CODER VSCODE dev.coder.com --- -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com --- - -Host afterconfig - HostName after.config.tld - User after`; - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig); - mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o644 }); - await sshConfig.load(); - - const expectedOutput = `Host beforeconfig - HostName before.config.tld - User before +describe("SshConfig.update", () => { + it("renders the exact header and deployment config", async () => { + await updateDeployment(); -# --- START CODER VSCODE --- -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE --- - -Host donotdelete - HostName dont.delete.me - User please - -# --- START CODER VSCODE dev.coder.com --- -${managedHeader} -Host coder-vscode.dev.coder.com--* - ConnectTimeout 0 - LogLevel ERROR - ProxyCommand some-command-here - ServerAliveCountMax 3 - ServerAliveInterval 10 - StrictHostKeyChecking no - UserKnownHostsFile /dev/null -# --- END CODER VSCODE dev.coder.com --- - -Host afterconfig - HostName after.config.tld - User after`; + expect(await readConfig()).toBe(`${fileHeader}\n\n${deploymentBlock}`); + const configDir = vol.statSync("/Path/To/UserHomeDir/.sshConfigDir"); + expect(configDir.mode & 0o777).toBe(0o700); + }); - await sshConfig.update("dev.coder.com", BASE_SSH_VALUES); + it("regenerates the whole file over existing content", async () => { + await updateDeployment("Host personal\n HostName example.com\n\n"); + expect(await readConfig()).toBe(`${fileHeader}\n\n${deploymentBlock}`); + }); - expect(mockFileSystem.writeFile).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - expectedOutput, - { - encoding: "utf-8", - mode: 0o644, - }, - ); - expect(mockFileSystem.rename).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - sshFilePath, - ); -}); + it("rewrites an unchanged file so its mtime marks the last connect", async () => { + const renameSpy = vi.spyOn(fsPromises, "rename"); + await updateDeployment(`${fileHeader}\n\n${deploymentBlock}`); + expect(renameSpy).toHaveBeenCalledTimes(1); + }); -it("override values", async () => { - mockFileSystem.readFile.mockRejectedValueOnce("No file found"); - mockFileSystem.stat.mockRejectedValueOnce({ code: "ENOENT" }); + it("applies sorted case-insensitive overrides, additions, and removals", async () => { + await updateDeployment(undefined, BASE_SSH_VALUES, { + loglevel: "DEBUG", + ConnectTimeout: "500", + ExtraKey: "ExtraValue", + StrictHostKeyChecking: "", + ExtraRemove: "", + }); - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); - await sshConfig.update("dev.coder.com", BASE_SSH_VALUES, { - loglevel: "DEBUG", // This tests case insensitive - ConnectTimeout: "500", - ExtraKey: "ExtraValue", - Foo: "bar", - Buzz: "baz", - // Remove this key - StrictHostKeyChecking: "", - ExtraRemove: "", - }); + expect(await readConfig()).toBe(`${fileHeader} - const expectedOutput = `# --- START CODER VSCODE dev.coder.com --- -${managedHeader} Host coder-vscode.dev.coder.com--* - Buzz baz ConnectTimeout 500 ExtraKey ExtraValue - Foo bar ProxyCommand some-command-here ServerAliveCountMax 3 ServerAliveInterval 10 UserKnownHostsFile /dev/null - loglevel DEBUG -# --- END CODER VSCODE dev.coder.com ---`; - - expect(mockFileSystem.readFile).toHaveBeenCalledWith( - sshFilePath, - expect.anything(), - ); - expect(mockFileSystem.writeFile).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - expectedOutput, - expect.objectContaining({ - encoding: "utf-8", - mode: 0o600, // Default mode for new files. - }), - ); - expect(mockFileSystem.rename).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - sshFilePath, - ); -}); + loglevel DEBUG`); + }); -describe("SSH config serialization", () => { /** * One case per input surface; the full character matrix is covered by the * validateDeploymentSshOptions tests below. */ interface RejectCase { name: string; - safeHostname?: string; values?: SshValues; overrides?: Record; } it.each([ - { - name: "deployment hostname newline", - safeHostname: "dev.coder.com\nHost *", - }, { name: "Host value carriage return", values: { ...BASE_SSH_VALUES, Host: "coder-vscode--*\rMatch all" }, @@ -596,69 +202,335 @@ describe("SSH config serialization", () => { overrides: { ForwardAgent: "yes\nRemoteCommand calc" }, }, ])( - "rejects $name", - async ({ - safeHostname = "dev.coder.com", - values = BASE_SSH_VALUES, - overrides, - }) => { - mockFileSystem.readFile.mockRejectedValueOnce("No file found"); - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); + "rejects unsafe serialization: $name", + async ({ values = BASE_SSH_VALUES, overrides }) => { + const sshConfig = await loadSshConfig(); - await expect( - sshConfig.update(safeHostname, values, overrides), - ).rejects.toThrow(); - expect(mockFileSystem.writeFile).not.toHaveBeenCalled(); + await expect(sshConfig.update(values, overrides)).rejects.toThrow(); + expect(vol.existsSync(sshFilePath)).toBe(false); }, ); it("accepts benign override options", async () => { - mockFileSystem.readFile.mockRejectedValueOnce("No file found"); - mockFileSystem.stat.mockRejectedValueOnce({ code: "ENOENT" }); - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); + await updateDeployment(undefined, BASE_SSH_VALUES, USER_OVERRIDES); - await sshConfig.update("dev.coder.com", BASE_SSH_VALUES, USER_OVERRIDES); - - const writtenConfig = mockFileSystem.writeFile.mock.calls[0]?.[1]; + const writtenConfig = await readConfig(); expect(writtenConfig).toContain(" ForwardAgent yes"); expect(writtenConfig).toContain(" IdentityFile ~/.ssh/coder identity"); }); +}); - it("uses literal replacement text and preserves surrounding config", async () => { - const existentSshConfig = `Host before - IdentityFile ~/.ssh/before +describe("SshConfig.updateInclude", () => { + interface IncludePositionCase { + name: string; + existing: string; + expected: string; + } + it.each([ + { name: "empty config", existing: "", expected: includeBlock }, + { + name: "prepends to user config", + existing: "Host *\n ConnectTimeout 5", + expected: `${includeBlock}\n\nHost *\n ConnectTimeout 5`, + }, + { + name: "already first", + existing: `${includeBlock}\n\nHost *`, + expected: `${includeBlock}\n\nHost *`, + }, + { + name: "moves a stale block to first", + existing: `Host *\n\n${includeBlock.replace("coder/*.conf", "old/*.conf")}`, + expected: `${includeBlock}\n\nHost *`, + }, + ])("handles $name", async ({ existing, expected }) => { + await updateInclude(existing); + expect(await readConfig()).toBe(expected); + }); -# --- START CODER VSCODE dev.coder.com --- -Host coder-vscode.dev.coder.com--* -# --- END CODER VSCODE dev.coder.com --- + it("removes the current deployment and preserves other and deployment-unaware blocks", async () => { + await updateInclude( + `${legacyOtherDeploymentBlock}\n\n${legacyDeploymentBlock}\n\n${deploymentUnawareBlock}`, + ); + expect(await readConfig()).toBe( + `${includeBlock}\n\n${legacyOtherDeploymentBlock}\n\n${deploymentUnawareBlock}`, + ); + }); + + interface MalformedEditorCase { + name: string; + existing: string; + error: string; + } + it.each([ + { + name: "extra end marker", + existing: `${includeBlock}\n# --- END CODER ---`, + error: + 'has 1 "# --- START CODER ---" and 2 "# --- END CODER ---" markers', + }, + { + name: "duplicate blocks", + existing: `${includeBlock}\n${includeBlock}`, + error: 'has 2 "# --- START CODER ---" blocks', + }, + { + name: "end before start", + existing: "# --- END CODER ---\n# --- START CODER ---", + error: + '"# --- END CODER ---" marker before its "# --- START CODER ---" marker', + }, + ])("rejects $name", async ({ existing, error }) => { + await expect(updateInclude(existing)).rejects.toThrow(error); + expect(await readConfig()).toBe(existing); + }); + + interface IncludePathEscapeCase { + dir: string; + escaped: string; + } + it.each([ + { + dir: "~/.ssh/we[i]rd/*?[dir]", + escaped: "~/.ssh/we\\[i\\]rd/\\*\\?\\[dir\\]", + }, + { + dir: "C:\\Users\\Jane Doe\\ssh", + escaped: "C:/Users/Jane Doe/ssh", + }, + ])("escapes $dir", async ({ dir, escaped }) => { + await updateInclude("", dir); + expect(await readConfig()).toContain(`Include "${escaped}/*.conf"`); + }); + + // A tilde swallows home-path quirks that ssh could not read back otherwise. + it("writes a home-relative include path with a tilde", async () => { + const home = "/home/we[i]rd %user"; + vi.mocked(os.homedir).mockReturnValue(home); + await updateInclude("", `${home}/.local/share/coder.coder-remote/ssh`); + expect(await readConfig()).toContain( + 'Include "~/.local/share/coder.coder-remote/ssh/*.conf"', + ); + }); + + type InvalidIncludeDir = string; + it.each([ + "path\rname", + "path\nname", + "path\0name", + 'path"name', + "path%name", + ])("rejects unrepresentable include paths", async (dir) => { + await expect(updateInclude("", dir)).rejects.toThrow( + "must not contain CR, LF, NUL", + ); + }); +}); + +describe("persistence", () => { + interface FileModeCase { + name: string; + existing: string | undefined; + mode: number; + } + it.each([ + { name: "new file", existing: undefined, mode: 0o600 }, + { name: "existing file", existing: "Host *", mode: 0o640 }, + ])("uses the correct mode for a $name", async ({ existing, mode }) => { + const sshConfig = await loadSshConfig(existing, mode); + await sshConfig.update(BASE_SSH_VALUES); + expect(vol.statSync(sshFilePath).mode & 0o777).toBe(mode); + }); + + type FileSystemErrorStage = "load" | "include read" | "stat"; + it.each(["load", "include read", "stat"])( + "propagates non-ENOENT %s errors", + async (stage) => { + const denied = Object.assign(new Error("denied"), { code: "EACCES" }); + if (stage === "load") { + vol.fromJSON({ [sshFilePath]: "Host initial" }); + vi.spyOn(fsPromises, "readFile").mockRejectedValueOnce(denied); + const sshConfig = new SshConfig(sshFilePath, mockLogger, fsPromises); + await expect(sshConfig.load()).rejects.toBe(denied); + return; + } + const sshConfig = await loadSshConfig("Host initial"); + if (stage === "include read") { + vi.spyOn(fsPromises, "readFile").mockRejectedValueOnce(denied); + await expect( + sshConfig.updateInclude(includeDir, hostname), + ).rejects.toThrow("denied"); + return; + } + vi.spyOn(fsPromises, "stat").mockRejectedValueOnce(denied); + await expect(sshConfig.update(BASE_SSH_VALUES)).rejects.toThrow("denied"); + }, + ); + + it("wraps write failures", async () => { + const sshConfig = await loadSshConfig("Host initial"); + vi.spyOn(fsPromises, "writeFile").mockRejectedValueOnce( + new Error("EACCES"), + ); + await expect(sshConfig.update(BASE_SSH_VALUES)).rejects.toThrow( + /Failed to write temporary SSH config file.*EACCES/, + ); + }); + + it("wraps rename failures and removes the temporary file", async () => { + const sshConfig = await loadSshConfig("Host initial"); + const error = Object.assign(new Error("EXDEV"), { code: "EXDEV" }); + vi.spyOn(fsPromises, "rename").mockRejectedValueOnce(error); + await expect(sshConfig.update(BASE_SSH_VALUES)).rejects.toThrow( + "Failed to rename temporary SSH config file", + ); + const leftoverTempFiles = Object.keys(vol.toJSON()).filter((filePath) => + filePath.includes("vscode-coder-tmp"), + ); + expect(leftoverTempFiles).toEqual([]); + }); + + it("writes over a concurrent change using the freshly read content", async () => { + const sshConfig = await loadSshConfig("Host initial"); + // The include update reads right before merging, so it picks up content + // written after load(). + vol.writeFileSync(sshFilePath, "Host concurrent"); + await sshConfig.updateInclude(includeDir, hostname); + expect(await readConfig()).toBe(`${includeBlock}\n\nHost concurrent`); + }); -Host after - IdentityFile ~/.ssh/after`; - mockFileSystem.readFile.mockResolvedValueOnce(existentSshConfig); - mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o600 }); + it("does not rewrite the file when the include is already in place", async () => { + const sshConfig = await loadSshConfig(`${includeBlock}\n\nHost *`); + const writeFileSpy = vi.spyOn(fsPromises, "writeFile"); + const renameSpy = vi.spyOn(fsPromises, "rename"); + await sshConfig.updateInclude(includeDir, hostname); + expect(writeFileSpy).not.toHaveBeenCalled(); + expect(renameSpy).not.toHaveBeenCalled(); + }); +}); - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); - await sshConfig.update("dev.coder.com", BASE_SSH_VALUES, { - IdentityFile: "$& $` $' $ $$", +describe("cleanupStaleSshConfigs", () => { + it("removes only generated configs not written for a week", async () => { + const dir = "/Path/To/UserHomeDir/ssh"; + vol.fromJSON({ + [`${dir}/vscode--old.coder.com.conf`]: "stale", + [`${dir}/cursor--fresh.coder.com.conf`]: "fresh", + [`${dir}/unrelated.txt`]: "keep", }); + const eightDaysAgo = new Date(Date.now() - 8 * 24 * 60 * 60 * 1000); + vol.utimesSync( + `${dir}/vscode--old.coder.com.conf`, + eightDaysAgo, + eightDaysAgo, + ); - const writtenConfig = String(mockFileSystem.writeFile.mock.calls[0]?.[1]); - expect(writtenConfig).toContain(" IdentityFile $& $` $' $ $$"); - expect( - writtenConfig.startsWith(`Host before - IdentityFile ~/.ssh/before + await cleanupStaleSshConfigs(dir, mockLogger); -`), - ).toBe(true); - expect( - writtenConfig.endsWith(` + expect(Object.keys(vol.toJSON()).sort()).toEqual([ + `${dir}/cursor--fresh.coder.com.conf`, + `${dir}/unrelated.txt`, + ]); + }); +}); -Host after - IdentityFile ~/.ssh/after`), - ).toBe(true); +describe("parseSshConfig", () => { + interface ParseSshConfigCase { + name: string; + input: string[]; + expected: Record; + } + it.each([ + { + name: "parses space and equals separators", + input: ["ConnectTimeout 10", "LogLevel=DEBUG"], + expected: { ConnectTimeout: "10", LogLevel: "DEBUG" }, + }, + { + name: "accumulates non-empty SetEnv values", + input: ["SetEnv A=1", "setenv=B=2 C=3", "SetEnv="], + expected: { SetEnv: "A=1 B=2 C=3" }, + }, + { + name: "skips malformed lines", + input: ["malformed", "Key:value", "# comment", "key=value"], + expected: { key: "value" }, + }, + ])("$name", ({ input, expected }) => { + expect(parseSshConfig(input)).toEqual(expected); + }); +}); + +describe("mergeSshConfigValues", () => { + interface MergeSshConfigCase { + name: string; + config: Record; + overrides: Record; + expected: Record; + } + it.each([ + { + name: "overrides case-insensitively and preserves other values", + config: { LogLevel: "ERROR", Keep: "yes" }, + overrides: { loglevel: "DEBUG" }, + expected: { loglevel: "DEBUG", Keep: "yes" }, + }, + { + name: "adds and removes keys", + config: { Remove: "value" }, + overrides: { Remove: "", Add: "value" }, + expected: { Add: "value" }, + }, + { + name: "combines SetEnv and ignores an empty override", + config: { SetEnv: "A=1" }, + overrides: { setenv: "B=2" }, + expected: { SetEnv: "A=1 B=2" }, + }, + { + name: "keeps SetEnv for an empty override", + config: { SetEnv: "A=1" }, + overrides: { SetEnv: "" }, + expected: { SetEnv: "A=1" }, + }, + { + name: "adds SetEnv from overrides", + config: {}, + overrides: { SetEnv: "A=1" }, + expected: { SetEnv: "A=1" }, + }, + ])("$name", ({ config, overrides, expected }) => { + expect(mergeSshConfigValues(config, overrides)).toEqual(expected); + }); +}); + +describe("parseCoderSshOptions", () => { + const coderBlock = (...lines: string[]) => + `# ------------START-CODER-----------\n${lines.join("\n")}\n# ------------END-CODER------------`; + + interface ParseCoderOptionsCase { + name: string; + raw: string; + expected: Record; + } + it.each([ + { name: "no block", raw: "Host personal", expected: {} }, + { + name: "options only", + raw: coderBlock( + "# :wait=yes", + "# :ssh-option=ForwardX11=yes", + "# :ssh-option=SetEnv=FOO=1", + "# :ssh-option=SetEnv=BAR=2", + ), + expected: { ForwardX11: "yes", SetEnv: "FOO=1 BAR=2" }, + }, + { + name: "flexible marker dashes", + raw: "# ---START-CODER---\n# :ssh-option=ForwardX11=yes\n# ---END-CODER---", + expected: { ForwardX11: "yes" }, + }, + ])("$name", ({ raw, expected }) => { + expect(parseCoderSshOptions(raw)).toEqual(expected); }); }); @@ -750,7 +622,7 @@ describe("validateDeploymentSshOptions", () => { ).toThrow( 'To allow "LocalCommand", set the option yourself in the "coder.sshConfig" setting', ); - // Overriding a pinned option fails the post-write check instead. + // Coder always writes the pinned options itself. expect(() => validateDeploymentSshOptions({ ProxyCommand: "evil" }, {}), ).toThrow('Coder manages "ProxyCommand", which cannot be overridden.'); @@ -784,316 +656,3 @@ describe("validateDeploymentSshOptions", () => { ).toThrow('"LocalCommand"'); }); }); - -it("fails if we are unable to write the temporary file", async () => { - const existentSSHConfig = `Host beforeconfig - HostName before.config.tld - User before`; - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig); - mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o600 }); - mockFileSystem.writeFile.mockRejectedValueOnce(new Error("EACCES")); - - await sshConfig.load(); - - expect(mockFileSystem.readFile).toHaveBeenCalledWith( - sshFilePath, - expect.anything(), - ); - await expect( - sshConfig.update("dev.coder.com", BASE_SSH_VALUES), - ).rejects.toThrow(/Failed to write temporary SSH config file.*EACCES/); -}); - -it("cleans up temp file when rename fails", async () => { - mockFileSystem.readFile.mockResolvedValueOnce("Host existing\n HostName x"); - mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o600 }); - mockFileSystem.writeFile.mockResolvedValueOnce(""); - const err = new Error("EXDEV"); - (err as NodeJS.ErrnoException).code = "EXDEV"; - mockFileSystem.rename.mockRejectedValueOnce(err); - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); - await expect( - sshConfig.update("dev.coder.com", { - ...BASE_SSH_VALUES, - ProxyCommand: "cmd", - }), - ).rejects.toThrow(/Failed to rename temporary SSH config file/); - expect(mockFileSystem.unlink).toHaveBeenCalledWith( - expect.stringContaining(sshTempFilePrefix), - ); -}); - -describe("rename retry on Windows", () => { - const realPlatform = process.platform; - - beforeEach(() => { - Object.defineProperty(process, "platform", { value: "win32" }); - vi.useFakeTimers(); - }); - afterEach(() => { - vi.useRealTimers(); - Object.defineProperty(process, "platform", { value: realPlatform }); - }); - - it("retries on transient EPERM and succeeds", async () => { - mockFileSystem.readFile.mockResolvedValueOnce( - "Host existing\n HostName x", - ); - mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o600 }); - mockFileSystem.writeFile.mockResolvedValueOnce(""); - const err = new Error("EPERM"); - (err as NodeJS.ErrnoException).code = "EPERM"; - mockFileSystem.rename - .mockRejectedValueOnce(err) - .mockResolvedValueOnce(undefined); - - const sshConfig = new SshConfig(sshFilePath, mockLogger, mockFileSystem); - await sshConfig.load(); - const promise = sshConfig.update("dev.coder.com", { - ...BASE_SSH_VALUES, - ProxyCommand: "cmd", - }); - - await vi.advanceTimersByTimeAsync(100); - await promise; - - expect(mockFileSystem.rename).toHaveBeenCalledTimes(2); - expect(mockFileSystem.unlink).not.toHaveBeenCalled(); - }); -}); - -describe("parseSshConfig", () => { - interface ParseTest { - name: string; - input: string[]; - expected: Record; - } - - it.each([ - { - name: "space separator", - input: ["Key value"], - expected: { Key: "value" }, - }, - { - name: "equals separator", - input: ["Key=value"], - expected: { Key: "value" }, - }, - { - name: "SetEnv with space", - input: ["SetEnv MY_VAR=value OTHER_VAR=othervalue"], - expected: { SetEnv: "MY_VAR=value OTHER_VAR=othervalue" }, - }, - { - name: "SetEnv with equals", - input: ["SetEnv=MY_VAR=value OTHER_VAR=othervalue"], - expected: { SetEnv: "MY_VAR=value OTHER_VAR=othervalue" }, - }, - { - name: "accumulates SetEnv entries", - input: ["SetEnv A=1", "setenv B=2 C=3"], - expected: { SetEnv: "A=1 B=2 C=3" }, - }, - { - name: "skips malformed lines", - input: ["malformed", "# comment", "key=value", " indented"], - expected: { key: "value" }, - }, - { - name: "value with spaces", - input: ["ProxyCommand ssh -W %h:%p proxy"], - expected: { ProxyCommand: "ssh -W %h:%p proxy" }, - }, - { - name: "quoted value with spaces", - input: ['SetEnv key="Hello world"'], - expected: { SetEnv: 'key="Hello world"' }, - }, - { - name: "multiple keys", - input: ["ConnectTimeout 10", "LogLevel=DEBUG", "SetEnv VAR=1"], - expected: { ConnectTimeout: "10", LogLevel: "DEBUG", SetEnv: "VAR=1" }, - }, - { - name: "ignores empty SetEnv", - input: ["SetEnv=", "SetEnv "], - expected: {}, - }, - ])("$name", ({ input, expected }) => { - expect(parseSshConfig(input)).toEqual(expected); - }); -}); - -describe("mergeSshConfigValues", () => { - interface MergeTest { - name: string; - config: Record; - overrides: Record; - expected: Record; - } - - it.each([ - { - name: "overrides case-insensitively", - config: { LogLevel: "ERROR" }, - overrides: { loglevel: "DEBUG" }, - expected: { loglevel: "DEBUG" }, - }, - { - name: "removes keys with empty string", - config: { LogLevel: "ERROR", Foo: "bar" }, - overrides: { LogLevel: "" }, - expected: { Foo: "bar" }, - }, - { - name: "adds new keys from overrides", - config: { LogLevel: "ERROR" }, - overrides: { NewKey: "value" }, - expected: { LogLevel: "ERROR", NewKey: "value" }, - }, - { - name: "preserves keys not in overrides", - config: { A: "1", B: "2" }, - overrides: { B: "3" }, - expected: { A: "1", B: "3" }, - }, - { - name: "concatenates SetEnv values", - config: { SetEnv: "A=1" }, - overrides: { SetEnv: "B=2" }, - expected: { SetEnv: "A=1 B=2" }, - }, - { - name: "concatenates SetEnv case-insensitively", - config: { SetEnv: "A=1" }, - overrides: { setenv: "B=2" }, - expected: { SetEnv: "A=1 B=2" }, - }, - { - name: "SetEnv only in override", - config: {}, - overrides: { SetEnv: "B=2" }, - expected: { SetEnv: "B=2" }, - }, - { - name: "SetEnv only in config", - config: { SetEnv: "A=1" }, - overrides: {}, - expected: { SetEnv: "A=1" }, - }, - { - name: "SetEnv with other values", - config: { SetEnv: "A=1", LogLevel: "ERROR" }, - overrides: { SetEnv: "B=2", Timeout: "10" }, - expected: { SetEnv: "A=1 B=2", LogLevel: "ERROR", Timeout: "10" }, - }, - { - name: "ignores empty SetEnv override", - config: { SetEnv: "A=1 B=2" }, - overrides: { SetEnv: "" }, - expected: { SetEnv: "A=1 B=2" }, - }, - ])("$name", ({ config, overrides, expected }) => { - expect(mergeSshConfigValues(config, overrides)).toEqual(expected); - }); -}); - -describe("parseCoderSshOptions", () => { - const coderBlock = (...lines: string[]) => - `# ------------START-CODER-----------\n${lines.join("\n")}\n# ------------END-CODER------------`; - - interface SshOptionTestCase { - name: string; - raw: string; - expected: Record; - } - it.each([ - { - name: "empty string", - raw: "", - expected: {}, - }, - { - name: "no CLI block", - raw: "Host myhost\n HostName example.com", - expected: {}, - }, - { - name: "single option", - raw: coderBlock("# :ssh-option=ForwardX11=yes"), - expected: { ForwardX11: "yes" }, - }, - { - name: "multiple options", - raw: coderBlock( - "# :ssh-option=ForwardX11=yes", - "# :ssh-option=ForwardX11Trusted=yes", - ), - expected: { ForwardX11: "yes", ForwardX11Trusted: "yes" }, - }, - { - name: "ignores non-ssh-option keys", - raw: coderBlock( - "# :wait=yes", - "# :disable-autostart=true", - "# :ssh-option=ForwardX11=yes", - ), - expected: { ForwardX11: "yes" }, - }, - { - name: "accumulates SetEnv across lines", - raw: coderBlock( - "# :ssh-option=SetEnv=FOO=1", - "# :ssh-option=SetEnv=BAR=2", - ), - expected: { SetEnv: "FOO=1 BAR=2" }, - }, - { - name: "tolerates different dash counts in markers", - raw: `# ---START-CODER---\n# :ssh-option=ForwardX11=yes\n# ---END-CODER---`, - expected: { ForwardX11: "yes" }, - }, - ])("$name", ({ raw, expected }) => { - expect(parseCoderSshOptions(raw)).toEqual(expected); - }); - - it("extracts only ssh-options from a full config", () => { - const raw = `Host personal-server - HostName 10.0.0.1 - User admin - -# ------------START-CODER----------- -# This file is managed by coder. DO NOT EDIT. -# -# You should not hand-edit this file, changes may be overwritten. -# For more information, see https://coder.com/docs -# -# :wait=yes -# :disable-autostart=true -# :ssh-option=ForwardX11=yes -# :ssh-option=ForwardX11Trusted=yes - -Host coder.mydeployment--* - ConnectTimeout 0 - ForwardX11 yes - ForwardX11Trusted yes - StrictHostKeyChecking no - UserKnownHostsFile /dev/null - LogLevel ERROR - ProxyCommand /usr/bin/coder ssh --stdio --ssh-host-prefix coder.mydeployment-- %h -# ------------END-CODER------------ - -Host work-server - HostName 10.0.0.2 - User work`; - expect(parseCoderSshOptions(raw)).toEqual({ - ForwardX11: "yes", - ForwardX11Trusted: "yes", - }); - }); -}); diff --git a/test/unit/remote/sshExtension.test.ts b/test/unit/remote/sshExtension.test.ts index 6aa4a7f4e..a59fc2211 100644 --- a/test/unit/remote/sshExtension.test.ts +++ b/test/unit/remote/sshExtension.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import * as vscode from "vscode"; -import { getRemoteSshSetting } from "@/remote/sshExtension"; +import { getRemoteSshConfigFile } from "@/remote/sshExtension"; import { config, type Settings } from "../../mocks/testHelpers"; @@ -13,58 +13,41 @@ function setup(extensionId: string, settings: Settings = {}): void { ); } -describe("getRemoteSshSetting", () => { - interface SectionCase { - id: string; - key: string; - } +describe("getRemoteSshConfigFile", () => { + it.each([ + "ms-vscode-remote.remote-ssh", + "anysphere.remote-ssh", + "jeanp413.open-remote-ssh", + ])("reads the configured file for %s", (extensionId) => { + setup(extensionId, { "remote.SSH.configFile": "/custom/config" }); - it.each([ - { id: "ms-vscode-remote.remote-ssh", key: "remote.SSH.configFile" }, - { id: "anysphere.remote-ssh", key: "remote.SSH.configFile" }, - { id: "jeanp413.open-remote-ssh", key: "remote.SSH.configFile" }, - { - id: "google.antigravity-remote-openssh", - key: "remote.antigravitySSH.configFile", - }, - { - id: "codeium.windsurf-remote-openssh", - key: "remote.devinSSH.configFile", - }, - ])("reads the section $id uses", ({ id, key }) => { - setup(id, { [key]: "/custom/config" }); - - expect(getRemoteSshSetting("configFile")).toBe("/custom/config"); - }); - - it("falls back to the legacy Windsurf section", () => { - setup("codeium.windsurf-remote-openssh", { - "remote.windsurfSSH.configFile": "/legacy/config", - }); - - expect(getRemoteSshSetting("configFile")).toBe("/legacy/config"); + expect(getRemoteSshConfigFile()).toBe("/custom/config"); }); - it("prefers the Devin section over the legacy Windsurf one", () => { - setup("codeium.windsurf-remote-openssh", { - "remote.devinSSH.configFile": "/devin/config", - "remote.windsurfSSH.configFile": "/legacy/config", - }); - - expect(getRemoteSshSetting("configFile")).toBe("/devin/config"); - }); + it.each([ + ["google.antigravity-remote-openssh", "remote.antigravitySSH.configFile"], + ["codeium.windsurf-remote-openssh", "remote.devinSSH.configFile"], + ])( + "ignores the file %s never connects through", + (extensionId, settingKey) => { + setup(extensionId, { + [settingKey]: "/custom/config", + "remote.SSH.configFile": "/stale/config", + }); + + expect(getRemoteSshConfigFile()).toBeUndefined(); + }, + ); - it("ignores another extension's section", () => { - setup("google.antigravity-remote-openssh", { - "remote.SSH.configFile": "/custom/config", - }); + it("reads remote.SSH when no extension is installed", () => { + setup("", { "remote.SSH.configFile": "/custom/config" }); - expect(getRemoteSshSetting("configFile")).toBeUndefined(); + expect(getRemoteSshConfigFile()).toBe("/custom/config"); }); - it("defaults to remote.SSH when no extension is installed", () => { - setup("", { "remote.SSH.configFile": "/custom/config" }); + it("returns undefined when nothing is configured", () => { + setup("ms-vscode-remote.remote-ssh"); - expect(getRemoteSshSetting("configFile")).toBe("/custom/config"); + expect(getRemoteSshConfigFile()).toBeUndefined(); }); }); diff --git a/test/unit/remote/sshSupport.test.ts b/test/unit/remote/sshSupport.test.ts index bfebb56e7..124fa608e 100644 --- a/test/unit/remote/sshSupport.test.ts +++ b/test/unit/remote/sshSupport.test.ts @@ -2,7 +2,6 @@ import { describe, it, expect } from "vitest"; import { computeSshProperties, - findSshPropertyProblems, sshSupportsSetEnv, sshVersionSupportsSetEnv, } from "@/remote/sshSupport"; @@ -193,32 +192,3 @@ Host coder-vscode.dev.coder.com--* }); }); }); - -describe("findSshPropertyProblems", () => { - const EXPECTED = { - ProxyCommand: "coder ssh --stdio %h", - StrictHostKeyChecking: "no", - }; - const MATCHING = { - proxycommand: "coder ssh --stdio %h", - stricthostkeychecking: "no", - }; - - it("passes when values match, ignoring options Coder does not pin", () => { - expect( - findSshPropertyProblems( - { ...MATCHING, localcommand: "echo hello" }, - EXPECTED, - ), - ).toEqual([]); - }); - - it("reports every mismatch at once", () => { - expect(findSshPropertyProblems({ proxycommand: "evil" }, EXPECTED)).toEqual( - [ - '"ProxyCommand" is set to "evil", but Coder expects "coder ssh --stdio %h"', - '"StrictHostKeyChecking" is not set, but Coder expects "no"', - ], - ); - }); -}); diff --git a/test/unit/util/authority.test.ts b/test/unit/util/authority.test.ts index 8b0dd5f2d..f4d7120c7 100644 --- a/test/unit/util/authority.test.ts +++ b/test/unit/util/authority.test.ts @@ -1,234 +1,235 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; +import * as vscode from "vscode"; import { + type AuthorityClassification, type AuthorityParts, + classifyRemoteAuthority, + isRemoteAuthorityCompatible, parseRemoteAuthority, + retargetRemoteAuthority, + toCurrentAuthorityHostPrefix, toRemoteAuthority, } from "@/util/authority"; -describe("parseRemoteAuthority", () => { - const remoteAuthority = (sshHost: string) => `vscode://ssh-remote+${sshHost}`; +const env = vscode.env as typeof vscode.env & { uriScheme: string }; +const CURSOR_AUTHORITY = "ssh-remote+coder-cursor.dev.coder.com--foo--bar.main"; +const LEGACY_AUTHORITY = "ssh-remote+coder-vscode.dev.coder.com--foo--bar.main"; +const WINDSURF_AUTHORITY = + "ssh-remote+coder-windsurf.dev.coder.com--foo--bar.main"; - it.each([ - { label: "missing SSH host", input: "vscode://ssh-remote" }, - { label: "empty SSH host", input: "vscode://ssh-remote+" }, - { - label: "non-Coder host", - input: remoteAuthority("some-unrelated-host.com"), - }, - { - label: "prefix without safeHostname separator", - input: remoteAuthority("coder-vscode--foo--bar"), - }, - { - label: "similar prefix", - input: remoteAuthority("coder-vscode-test--foo--bar"), - }, - { label: "wrong prefix", input: remoteAuthority("coder--foo--bar") }, - ])("ignores unrelated authority: $label", ({ input }) => { - expect(parseRemoteAuthority(input)).toBe(null); - }); +const parts = (prefix: string): AuthorityParts => ({ + agent: "main", + sshHost: `${prefix}.dev.coder.com--foo--bar.main`, + safeHostname: "dev.coder.com", + username: "foo", + workspace: "bar", +}); - it.each([ - { - label: "missing username and workspace", - sshHost: "coder-vscode.dev.coder.com", - }, - { - label: "missing workspace", - sshHost: "coder-vscode.dev.coder.com--foo", - }, - { - label: "manual host using Coder prefix", - sshHost: "coder-vscode.personal-host", - }, - { - label: "empty username", - sshHost: "coder-vscode.dev.coder.com----bar", - }, - { - label: "empty workspace", - sshHost: "coder-vscode.dev.coder.com--foo--", - }, - { - label: "empty hostname", - sshHost: "coder-vscode.--foo--bar", - }, +afterEach(() => { + env.uriScheme = "vscode"; +}); + +describe("parseRemoteAuthority", () => { + interface ClassificationCase { + editor: string; + prefix: string; + expected: AuthorityClassification; + } + it.each([ + { editor: "vscode", prefix: "coder-vscode", expected: "current" }, + { editor: "vscode", prefix: "coder-vscode-insiders", expected: "foreign" }, + { editor: "cursor", prefix: "coder-vscode", expected: "legacy" }, { - label: "empty trailing segment", - sshHost: "coder-vscode.dev.coder.com--foo--bar--", + editor: "vscode-insiders", + prefix: "coder-vscode-insiders", + expected: "current", }, { - label: "empty workspace before agent separator", - sshHost: "coder-vscode.dev.coder.com--foo--.agent", + editor: "vscode-insiders", + prefix: "coder-vscode", + expected: "legacy", }, - { - label: "empty agent after separator", - sshHost: "coder-vscode.dev.coder.com--foo--bar.", + ])( + "classifies $prefix as $expected in $editor", + ({ editor, prefix, expected }) => { + env.uriScheme = editor; + expect(classifyRemoteAuthority(parts(prefix))).toBe(expected); }, - ])("rejects invalid authority: $label", ({ sshHost }) => { - expect(() => parseRemoteAuthority(remoteAuthority(sshHost))).toThrow( + ); + + interface MalformedAuthorityCase { + editor: string; + sshHost: string; + } + it.each([ + { editor: "vscode", sshHost: "coder-vscode.dev.coder.com--foo" }, + { editor: "cursor", sshHost: "coder-vscode.dev.coder.com--foo" }, + { editor: "vscode", sshHost: "coder-vscode.--foo--bar" }, + { editor: "vscode", sshHost: "coder-vscode.dev.coder.com----bar" }, + { editor: "vscode", sshHost: "coder-vscode.dev.coder.com--foo--" }, + { editor: "vscode", sshHost: "coder-vscode.dev.coder.com--foo--.main" }, + { editor: "vscode", sshHost: "coder-vscode.dev.coder.com--foo--bar." }, + ])("rejects malformed current or legacy authority", ({ editor, sshHost }) => { + env.uriScheme = editor; + expect(() => parseRemoteAuthority(`ssh-remote+${sshHost}`)).toThrow( "Invalid Coder SSH authority", ); }); + it("ignores unrelated and malformed foreign authorities", () => { + expect(parseRemoteAuthority("github.com")).toBeNull(); + expect(parseRemoteAuthority("ssh-remote+coder-vscode")).toBeNull(); + env.uriScheme = "cursor"; + expect( + parseRemoteAuthority("ssh-remote+coder-windsurf.dev.coder.com--foo"), + ).toBeNull(); + }); + + it.each(["vscode", "cursor"])( + "ignores deployment-unaware historical hosts in %s", + (editor) => { + env.uriScheme = editor; + // Old versions created hosts matching the preserved `Host coder-vscode--*` block. + expect( + parseRemoteAuthority("ssh-remote+coder-vscode--user--workspace.main"), + ).toBeNull(); + }, + ); + interface ParseCase { - label: string; sshHost: string; safeHostname: string; workspace: string; - agent?: string; - username?: string; + agent: string; } - - it("round trips generated remote authorities", () => { - const authority = toRemoteAuthority( - "https://ほげ", - "alice", - "workspace", - "main", - ); - - expect(authority).toBe( - "ssh-remote+coder-vscode.xn--18j4d--alice--workspace.main", - ); - expect(parseRemoteAuthority(authority)).toStrictEqual({ - agent: "main", - sshHost: "coder-vscode.xn--18j4d--alice--workspace.main", - safeHostname: "xn--18j4d", - username: "alice", - workspace: "workspace", - } satisfies AuthorityParts); - }); - it.each([ { - label: "hostname without agent", sshHost: "coder-vscode.dev.coder.com--foo--bar", safeHostname: "dev.coder.com", workspace: "bar", + agent: "", }, { - label: "hostname with agent", - sshHost: "coder-vscode.dev.coder.com--foo--bar.baz", - safeHostname: "dev.coder.com", - workspace: "bar", - agent: "baz", - }, - { - label: "hostname containing delimiter", - sshHost: "coder-vscode.test--domain.com--foo--bar", - safeHostname: "test--domain.com", - workspace: "bar", - }, - { - label: "Punycode hostname containing delimiter", - sshHost: "coder-vscode.xn--test---8o4.example--foo--bar", - safeHostname: "xn--test---8o4.example", - workspace: "bar", - }, - { - label: "hostname with repeated delimiters and agent", - sshHost: "coder-vscode.first--middle--last.example--foo--bar.baz", + sshHost: "coder-vscode.first--middle--last.example--foo--bar.main", safeHostname: "first--middle--last.example", workspace: "bar", - agent: "baz", - }, - { - label: "hostname with many consecutive dashes", - sshHost: "coder-vscode.foo---------------bar.com--foo--bar", - safeHostname: "foo---------------bar.com", - workspace: "bar", - }, - { - label: "ambiguous workspace/agent separator", - sshHost: "coder-vscode.dev.coder.com--foo--bar.baz.qux", - safeHostname: "dev.coder.com", - workspace: "bar.baz.qux", + agent: "main", }, ])( - "parses $label", - ({ sshHost, safeHostname, workspace, agent, username }) => { - expect(parseRemoteAuthority(remoteAuthority(sshHost))).toStrictEqual({ - agent: agent ?? "", + "parses $sshHost from the right", + ({ sshHost, safeHostname, workspace, agent }) => { + expect(parseRemoteAuthority(`ssh-remote+${sshHost}`)).toStrictEqual({ + agent, sshHost, safeHostname, - username: username ?? "foo", + username: "foo", workspace, } satisfies AuthorityParts); }, ); -}); -describe("toRemoteAuthority", () => { - interface ToRemoteAuthorityCase { - url: string; - owner: string; - workspace: string; - agent: string | undefined; - expected: string; + interface WrappedAuthorityCase { + label: string; + authority: string; } - it.each([ + it.each([ + { label: "plain", authority: CURSOR_AUTHORITY }, + { label: "URI", authority: `vscode://${CURSOR_AUTHORITY}` }, { - url: "https://dev.coder.com", - owner: "foo", - workspace: "bar", - agent: undefined, - expected: "ssh-remote+coder-vscode.dev.coder.com--foo--bar", - }, - { - url: "http://dev.coder.com:3000", - owner: "foo", - workspace: "bar", - agent: "baz", - expected: "ssh-remote+coder-vscode.dev.coder.com--foo--bar.baz", - }, - { - url: "https://coder.example.com/some/path?q=1", - owner: "alice", - workspace: "web", - agent: "", - expected: "ssh-remote+coder-vscode.coder.example.com--alice--web", + label: "multiply nested", + authority: `attached-container+def@dev-container+abc@${CURSOR_AUTHORITY}`, }, + ])("parses $label wrapper", ({ authority }) => { + env.uriScheme = "cursor"; + expect(parseRemoteAuthority(authority)).toStrictEqual( + parts("coder-cursor"), + ); + }); +}); + +describe("authority construction", () => { + it("preserves the editor URI scheme and integrates with toSafeHost", () => { + env.uriScheme = "cursor--dev"; + expect( + toRemoteAuthority("https://ほげ", "alice", "workspace", "main"), + ).toBe("ssh-remote+coder-cursor--dev.xn--18j4d--alice--workspace.main"); + }); + + it("omits an absent agent", () => { + expect( + toRemoteAuthority("https://dev.coder.com", "foo", "bar", undefined), + ).toBe("ssh-remote+coder-vscode.dev.coder.com--foo--bar"); + }); + + it("formats the current host prefix", () => { + env.uriScheme = "vscode-insiders"; + expect(toCurrentAuthorityHostPrefix("dev.coder.com")).toBe( + "coder-vscode-insiders.dev.coder.com--", + ); + }); + + it("rejects an empty editor URI scheme at prefix construction", () => { + env.uriScheme = ""; + expect(() => toCurrentAuthorityHostPrefix("dev.coder.com")).toThrow( + "must not be empty", + ); + }); +}); + +describe("authority migration", () => { + it("leaves unrelated authorities unchanged", () => { + expect(retargetRemoteAuthority("github.com")).toBe("github.com"); + }); + + interface RetargetCase { + label: string; + authority: string; + expected: string; + } + it.each([ { - url: "http://192.168.1.5:8080", - owner: "foo", - workspace: "bar", - agent: undefined, - expected: "ssh-remote+coder-vscode.192.168.1.5--foo--bar", + label: "plain", + authority: LEGACY_AUTHORITY, + expected: CURSOR_AUTHORITY, }, { - url: "http://localhost:3000", - owner: "dev", - workspace: "ws", - agent: "main", - expected: "ssh-remote+coder-vscode.localhost--dev--ws.main", + label: "multiply nested", + authority: `attached-container+def@dev-container+abc@${LEGACY_AUTHORITY}`, + expected: `attached-container+def@dev-container+abc@${CURSOR_AUTHORITY}`, }, - { - url: "https://sub.DOMAIN.Example.COM", - owner: "foo", - workspace: "bar", - agent: undefined, - expected: "ssh-remote+coder-vscode.sub.domain.example.com--foo--bar", + ])( + "preserves the $label wrapper while retargeting", + ({ authority, expected }) => { + env.uriScheme = "cursor"; + expect(retargetRemoteAuthority(authority)).toBe(expected); }, + ); + + interface CompatibilityCase { + label: string; + authority: string | undefined; + expected: boolean; + } + it.each([ + { label: "exact current", authority: CURSOR_AUTHORITY, expected: true }, + { label: "retargeted legacy", authority: LEGACY_AUTHORITY, expected: true }, + { label: "missing", authority: undefined, expected: false }, { - url: "https://ほげ:8080", - owner: "foo", - workspace: "bar", - agent: undefined, - expected: "ssh-remote+coder-vscode.xn--18j4d--foo--bar", + label: "malformed legacy", + authority: "ssh-remote+coder-vscode", + expected: false, }, + { label: "foreign", authority: WINDSURF_AUTHORITY, expected: false }, { - url: "https://عربي", - owner: "foo", - workspace: "bar", - agent: undefined, - expected: "ssh-remote+coder-vscode.xn--ngbrx4e--foo--bar", - }, - ])( - "builds authority for $url", - ({ url, owner, workspace, agent, expected }) => { - expect(toRemoteAuthority(url, owner, workspace, agent)).toBe(expected); + label: "different wrapper", + authority: `dev-container+abc@${LEGACY_AUTHORITY}`, + expected: false, }, - ); + ])("requires exact compatibility for $label", ({ authority, expected }) => { + env.uriScheme = "cursor"; + expect(isRemoteAuthorityCompatible(authority, CURSOR_AUTHORITY)).toBe( + expected, + ); + }); });