From a1aa15e80f126af8a02a29720b7c2118d3943515 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 16 Jun 2026 12:52:52 +0000 Subject: [PATCH 1/8] feat: support configurable CLI global config --- package.json | 8 +- src/core/cliCredentialManager.ts | 59 ++++++++++++--- src/core/pathResolver.ts | 8 +- src/login/loginCoordinator.ts | 31 +++++--- src/remote/remote.ts | 6 ++ src/supportBundle/settings.ts | 1 + test/unit/cliConfig.test.ts | 57 ++++++++++++++ test/unit/core/cliCredentialManager.test.ts | 84 ++++++++++++++++++++- test/unit/core/pathResolver.test.ts | 66 ++++++++++++++++ test/unit/login/loginCoordinator.test.ts | 29 +++++++ test/unit/supportBundle/settings.test.ts | 8 ++ 11 files changed, 330 insertions(+), 27 deletions(-) diff --git a/package.json b/package.json index 5bb9524f05..dee79a19cd 100644 --- a/package.json +++ b/package.json @@ -181,8 +181,14 @@ ], "scope": "machine" }, + "coder.globalConfig": { + "markdownDescription": "Path to the global Coder CLI config directory passed with `--global-config`. Defaults to `CODER_CONFIG_DIR` if not set, otherwise the extension's per-deployment global storage directory. Set this to a shared CLI config directory such as `~/.config/coderv2` to share login/auth with the Coder CLI. Ignored when `#coder.useKeyring#` is active and supported.\n\nSupports `${env:VAR}`, `${userHome}`, and a leading `~`.", + "type": "string", + "default": "", + "scope": "machine" + }, "coder.globalFlags": { - "markdownDescription": "Global flags to pass to every Coder CLI invocation. Enter each flag as a separate array item, in order. Do **not** include the `coder` command itself. See the [CLI reference](https://coder.com/docs/reference/cli) for available global flags.\n\nSupports `${env:VAR}`, `${userHome}`, and a leading `~`. For `--flag=value` items the expansion applies to the value half, so `--cfg=~/coder` works.\n\nFor `--header-command`, precedence is: `#coder.headerCommand#` setting, then `CODER_HEADER_COMMAND` environment variable, then the value specified here. The `--global-config` and `--use-keyring` flags are silently ignored as the extension manages them via `#coder.useKeyring#`.", + "markdownDescription": "Global flags to pass to every Coder CLI invocation. Enter each flag as a separate array item, in order. Do **not** include the `coder` command itself. See the [CLI reference](https://coder.com/docs/reference/cli) for available global flags.\n\nSupports `${env:VAR}`, `${userHome}`, and a leading `~`. For `--flag=value` items the expansion applies to the value half, so `--cfg=~/coder` works.\n\nFor `--header-command`, precedence is: `#coder.headerCommand#` setting, then `CODER_HEADER_COMMAND` environment variable, then the value specified here. The `--global-config` and `--use-keyring` flags are silently ignored; use `#coder.globalConfig#` and `#coder.useKeyring#` instead.", "type": "array", "items": { "type": "string" diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index 245f1a5e1a..e46129fc06 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -122,29 +122,35 @@ export class CliCredentialManager { } /** - * Read a token via `coder login token --url`. Returns trimmed stdout, - * or undefined on any failure (resolver, CLI, empty output). - * Throws AbortError when the signal is aborted. + * Read a token from CLI-managed credentials. Uses `coder login token --url` + * when keyring auth is active, otherwise reads the file credentials under + * --global-config. Returns undefined on any failure (resolver, CLI, empty + * output). Throws AbortError when the signal is aborted. */ public async readToken( url: string, configs: Pick, options?: { signal?: AbortSignal }, ): Promise { - let binPath: string | undefined; + if (!isKeyringEnabled(configs)) { + return this.readCredentialFiles(url); + } + + let binPath: string; try { - binPath = await this.resolveKeyringBinary( - url, - configs, - "keyringTokenRead", - ); + binPath = await this.resolveBinary(url); + const cliVersion = semver.parse(await version(binPath)); + const featureSet = featureSetForVersion(cliVersion); + if (!featureSet.keyringAuth) { + return this.readCredentialFiles(url); + } + if (!featureSet.keyringTokenRead) { + return undefined; + } } catch (error) { this.logger.warn("Could not resolve CLI binary for token read:", error); return undefined; } - if (!binPath) { - return undefined; - } const args = [...getHeaderArgs(configs), "login", "token", "--url", url]; try { @@ -248,6 +254,31 @@ export class CliCredentialManager { } } + /** + * Read URL and token files under --global-config. + */ + private async readCredentialFiles(url: string): Promise { + try { + const safeHostname = toSafeHost(url); + const [storedUrl, token] = await Promise.all([ + fs.readFile(this.pathResolver.getUrlPath(safeHostname), "utf8"), + fs.readFile( + this.pathResolver.getSessionTokenPath(safeHostname), + "utf8", + ), + ]); + if (normalizeCredentialUrl(storedUrl) !== normalizeCredentialUrl(url)) { + return undefined; + } + return token.trim() || undefined; + } catch (error) { + if ((error as NodeJS.ErrnoException)?.code !== "ENOENT") { + this.logger.warn("Failed to read credential files:", error); + } + return undefined; + } + } + /** * Delete URL and token files. Best-effort: never throws. */ @@ -317,3 +348,7 @@ export class CliCredentialManager { ); } } + +function normalizeCredentialUrl(url: string): string { + return url.trim().replace(/\/+$/, ""); +} diff --git a/src/core/pathResolver.ts b/src/core/pathResolver.ts index f4b337584f..74ad6541a4 100644 --- a/src/core/pathResolver.ts +++ b/src/core/pathResolver.ts @@ -10,13 +10,15 @@ export class PathResolver { ) {} /** - * Return the directory for the deployment with the provided hostname to - * where the global Coder configs are stored. + * Return the directory where the global Coder configs are stored. * * The caller must ensure this directory exists before use. */ public getGlobalConfigDir(safeHostname: string): string { - return path.join(this.basePath, safeHostname); + return ( + PathResolver.resolveOverride("coder.globalConfig", "CODER_CONFIG_DIR") || + path.join(this.basePath, safeHostname) + ); } /** diff --git a/src/login/loginCoordinator.ts b/src/login/loginCoordinator.ts index cb02627a96..8c1d0b1e2e 100644 --- a/src/login/loginCoordinator.ts +++ b/src/login/loginCoordinator.ts @@ -314,30 +314,41 @@ export class LoginCoordinator implements vscode.Disposable { } } - // Try keyring token (picks up tokens written by `coder login` in the terminal) + // Try CLI-managed credentials. This reads from the OS keyring when + // enabled, otherwise from the resolved global config directory. const configs = vscode.workspace.getConfiguration(); - const keyringResult = await withOptionalProgress( + const keyringEnabled = isKeyringEnabled(configs); + const cliCredentialResult = await withOptionalProgress( ({ signal }) => this.cliCredentialManager.readToken(deployment.url, configs, { signal, }), { - enabled: isKeyringEnabled(configs), + enabled: keyringEnabled, location: vscode.ProgressLocation.Notification, title: "Reading token from OS keyring...", cancellable: true, }, ); - const keyringToken = keyringResult.ok ? keyringResult.value : undefined; + const cliCredentialToken = cliCredentialResult.ok + ? cliCredentialResult.value + : undefined; if ( - keyringToken && - keyringToken !== providedToken && - keyringToken !== auth?.token + cliCredentialToken && + cliCredentialToken !== providedToken && + cliCredentialToken !== auth?.token ) { - this.logger.debug("Trying token from OS keyring"); - const result = await this.tryTokenAuth(client, keyringToken, isAutoLogin); + this.logger.debug("Trying token from CLI credentials"); + const result = await this.tryTokenAuth( + client, + cliCredentialToken, + isAutoLogin, + ); if (result !== "unauthorized") { - return withLoginMethod("keyring_token", result); + return withLoginMethod( + keyringEnabled ? "keyring_token" : "cli_token", + result, + ); } } diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 61b0216ef8..c80b9417eb 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -455,6 +455,12 @@ export class Remote { title: string; getValue: () => unknown; }> = [ + { + setting: "coder.globalConfig", + title: "Global Config", + getValue: () => + this.pathResolver.getGlobalConfigDir(parts.safeHostname), + }, { setting: "coder.globalFlags", title: "Global Flags", diff --git a/src/supportBundle/settings.ts b/src/supportBundle/settings.ts index 8517d4f1f4..49b69dce7a 100644 --- a/src/supportBundle/settings.ts +++ b/src/supportBundle/settings.ts @@ -23,6 +23,7 @@ const COLLECTED_SETTINGS: readonly string[] = [ "coder.disableUpdateNotifications", "coder.enableDownloads", "coder.experimental.oauth", + "coder.globalConfig", "coder.httpClientLogLevel", "coder.insecure", "coder.networkThreshold.latencyMs", diff --git a/test/unit/cliConfig.test.ts b/test/unit/cliConfig.test.ts index 2d6be51c4b..f75675a936 100644 --- a/test/unit/cliConfig.test.ts +++ b/test/unit/cliConfig.test.ts @@ -375,5 +375,62 @@ describe("cliConfig", () => { configDir: "/config/dir", }); }); + + it("uses caller-provided config directory in global-config mode", () => { + vi.mocked(os.platform).mockReturnValue("linux"); + const config = new MockConfigurationProvider(); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + const auth = resolveCliAuth( + config, + featureSet, + "https://dev.coder.com", + "/custom/coderv2", + ); + + expect(getGlobalFlags(config, auth)).toStrictEqual([ + "--global-config", + "/custom/coderv2", + ]); + }); + + it("keeps keyring precedence over caller-provided config directory", () => { + vi.mocked(os.platform).mockReturnValue("darwin"); + const config = new MockConfigurationProvider(); + config.set("coder.useKeyring", true); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + const auth = resolveCliAuth( + config, + featureSet, + "https://dev.coder.com", + "/custom/coderv2", + ); + + expect(getGlobalFlags(config, auth)).toStrictEqual([ + "--url", + "https://dev.coder.com", + ]); + }); + + it("does not let globalFlags override caller-provided config directory", () => { + vi.mocked(os.platform).mockReturnValue("linux"); + const config = new MockConfigurationProvider(); + config.set("coder.globalFlags", [ + "--verbose", + "--global-config=/ignored/coderv2", + ]); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + const auth = resolveCliAuth( + config, + featureSet, + "https://dev.coder.com", + "/custom/coderv2", + ); + + expect(getGlobalFlags(config, auth)).toStrictEqual([ + "--verbose", + "--global-config", + "/custom/coderv2", + ]); + }); }); }); diff --git a/test/unit/core/cliCredentialManager.test.ts b/test/unit/core/cliCredentialManager.test.ts index 9fc2ccfbcb..e2cf036e2c 100644 --- a/test/unit/core/cliCredentialManager.test.ts +++ b/test/unit/core/cliCredentialManager.test.ts @@ -131,6 +131,9 @@ const TEST_PATH_RESOLVER = new PathResolver("/mock/base", "/mock/log"); const CRED_DIR = "/mock/base/dev.coder.com"; const URL_FILE = `${CRED_DIR}/url`; const SESSION_FILE = `${CRED_DIR}/session`; +const CUSTOM_CRED_DIR = "/custom/coderv2"; +const CUSTOM_URL_FILE = `${CUSTOM_CRED_DIR}/url`; +const CUSTOM_SESSION_FILE = `${CUSTOM_CRED_DIR}/session`; function writeCredentialFiles(url: string, token: string) { vol.mkdirSync(CRED_DIR, { recursive: true }); @@ -219,6 +222,21 @@ describe("CliCredentialManager", () => { }); }); + it("writes files under configured global config when keyring is disabled", async () => { + new MockConfigurationProvider().set( + "coder.globalConfig", + CUSTOM_CRED_DIR, + ); + const { manager } = setup(); + + await expect( + manager.storeToken(TEST_URL, "my-token", configs), + ).resolves.toBeUndefined(); + + expect(memfs.readFileSync(CUSTOM_URL_FILE, "utf8")).toBe(TEST_URL); + expect(memfs.readFileSync(CUSTOM_SESSION_FILE, "utf8")).toBe("my-token"); + }); + it("falls back to files when CLI version too old", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); vi.mocked(cliExec.version).mockResolvedValueOnce("2.28.0"); @@ -350,10 +368,57 @@ describe("CliCredentialManager", () => { expect(execFile).not.toHaveBeenCalled(); }); - it("skips CLI when keyring is disabled", async () => { + it("reads files when keyring is disabled", async () => { + writeCredentialFiles(TEST_URL, "file-token"); stubExecFile({ stdout: "my-token" }); const { manager } = setup(); + expect(await manager.readToken(TEST_URL, configs)).toBe("file-token"); + expect(execFile).not.toHaveBeenCalled(); + }); + + it("reads files under configured global config when keyring is disabled", async () => { + new MockConfigurationProvider().set( + "coder.globalConfig", + CUSTOM_CRED_DIR, + ); + vol.mkdirSync(CUSTOM_CRED_DIR, { recursive: true }); + memfs.writeFileSync(CUSTOM_URL_FILE, `${TEST_URL}\n`); + memfs.writeFileSync(CUSTOM_SESSION_FILE, "custom-file-token\n"); + const { manager } = setup(); + + expect(await manager.readToken(TEST_URL, configs)).toBe( + "custom-file-token", + ); + expect(execFile).not.toHaveBeenCalled(); + }); + + it("does not read files when keyring token read is unsupported", async () => { + vi.mocked(isKeyringEnabled).mockReturnValue(true); + vi.mocked(cliExec.version).mockResolvedValueOnce("2.30.0"); + writeCredentialFiles(TEST_URL, "file-token"); + const { manager, resolver } = setup(); + + expect(await manager.readToken(TEST_URL, configs)).toBeUndefined(); + expect(resolver).toHaveBeenCalledWith(TEST_URL); + expect(execFile).not.toHaveBeenCalled(); + }); + + it("reads files when keyring is enabled but unsupported by the CLI", async () => { + vi.mocked(isKeyringEnabled).mockReturnValue(true); + vi.mocked(cliExec.version).mockResolvedValueOnce("2.28.0"); + writeCredentialFiles(TEST_URL, "file-token"); + const { manager, resolver } = setup(); + + expect(await manager.readToken(TEST_URL, configs)).toBe("file-token"); + expect(resolver).toHaveBeenCalledWith(TEST_URL); + expect(execFile).not.toHaveBeenCalled(); + }); + + it("does not read files for a different URL", async () => { + writeCredentialFiles("https://other.coder.com", "file-token"); + const { manager } = setup(); + expect(await manager.readToken(TEST_URL, configs)).toBeUndefined(); expect(execFile).not.toHaveBeenCalled(); }); @@ -438,6 +503,23 @@ describe("CliCredentialManager", () => { expect(memfs.existsSync(SESSION_FILE)).toBe(false); }); + it("deletes files under configured global config", async () => { + new MockConfigurationProvider().set( + "coder.globalConfig", + CUSTOM_CRED_DIR, + ); + vol.mkdirSync(CUSTOM_CRED_DIR, { recursive: true }); + memfs.writeFileSync(CUSTOM_URL_FILE, TEST_URL); + memfs.writeFileSync(CUSTOM_SESSION_FILE, "old-token"); + const { manager } = setup(); + + await manager.deleteToken(TEST_URL, configs); + + expect(execFile).not.toHaveBeenCalled(); + expect(memfs.existsSync(CUSTOM_URL_FILE)).toBe(false); + expect(memfs.existsSync(CUSTOM_SESSION_FILE)).toBe(false); + }); + it("never throws on CLI error", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); stubExecFile({ error: "logout failed" }); diff --git a/test/unit/core/pathResolver.test.ts b/test/unit/core/pathResolver.test.ts index 4a602382fa..dc033db747 100644 --- a/test/unit/core/pathResolver.test.ts +++ b/test/unit/core/pathResolver.test.ts @@ -19,6 +19,72 @@ describe("PathResolver", () => { mockConfig = new MockConfigurationProvider(); }); + describe("getGlobalConfigDir", () => { + it("uses per-deployment global storage when no override is configured", () => { + vi.stubEnv("CODER_CONFIG_DIR", ""); + mockConfig.set("coder.globalConfig", ""); + + expectPathsEqual( + pathResolver.getGlobalConfigDir("deployment"), + path.join(basePath, "deployment"), + ); + }); + + it("uses configured global config directory directly", () => { + mockConfig.set("coder.globalConfig", "/custom/coderv2"); + + expectPathsEqual( + pathResolver.getGlobalConfigDir("deployment"), + "/custom/coderv2", + ); + }); + + it("uses CODER_CONFIG_DIR when setting is empty", () => { + vi.stubEnv("CODER_CONFIG_DIR", " /env/coderv2 "); + mockConfig.set("coder.globalConfig", ""); + + expectPathsEqual( + pathResolver.getGlobalConfigDir("deployment"), + "/env/coderv2", + ); + }); + + it("uses setting before CODER_CONFIG_DIR", () => { + vi.stubEnv("CODER_CONFIG_DIR", "/env/coderv2"); + mockConfig.set("coder.globalConfig", " /setting/coderv2 "); + + expectPathsEqual( + pathResolver.getGlobalConfigDir("deployment"), + "/setting/coderv2", + ); + }); + + it("normalizes configured global config directory", () => { + mockConfig.set("coder.globalConfig", "/custom/../coderv2/./dir"); + + expectPathsEqual( + pathResolver.getGlobalConfigDir("deployment"), + "/coderv2/dir", + ); + }); + + it("expands paths in configured global config directory", () => { + mockConfig.set("coder.globalConfig", "~/coderv2"); + const result = pathResolver.getGlobalConfigDir("deployment"); + + expect(result).not.toContain("~"); + expect(result).toContain("coderv2"); + }); + + it("expands paths in CODER_CONFIG_DIR", () => { + vi.stubEnv("CODER_CONFIG_DIR", "~/coderv2"); + const result = pathResolver.getGlobalConfigDir("deployment"); + + expect(result).not.toContain("~"); + expect(result).toContain("coderv2"); + }); + }); + describe("getProxyLogPath", () => { const defaultLogPath = path.join(basePath, "log"); diff --git a/test/unit/login/loginCoordinator.test.ts b/test/unit/login/loginCoordinator.test.ts index b5925b94d6..71e66fe751 100644 --- a/test/unit/login/loginCoordinator.test.ts +++ b/test/unit/login/loginCoordinator.test.ts @@ -202,6 +202,35 @@ describe("LoginCoordinator", () => { expect(auth?.token).toBe("stored-token"); }); + it("authenticates with CLI credential token on success", async () => { + const { + mockCredentialManager, + secretsManager, + coordinator, + mockSuccessfulAuth, + } = createTestContext(); + const user = mockSuccessfulAuth(); + vi.mocked(mockCredentialManager.readToken).mockResolvedValueOnce( + "cli-credential-token", + ); + + const result = await coordinator.ensureLoggedIn({ + url: TEST_URL, + safeHostname: TEST_HOSTNAME, + }); + + expect(result).toEqual({ + success: true, + method: "cli_token", + user, + token: "cli-credential-token", + }); + expect(vscode.window.showInputBox).not.toHaveBeenCalled(); + + const auth = await secretsManager.getSessionAuth(TEST_HOSTNAME); + expect(auth?.token).toBe("cli-credential-token"); + }); + it("prompts for token when no stored auth exists", async () => { const { userInteraction, diff --git a/test/unit/supportBundle/settings.test.ts b/test/unit/supportBundle/settings.test.ts index 43f4c9abe0..32e0180f48 100644 --- a/test/unit/supportBundle/settings.test.ts +++ b/test/unit/supportBundle/settings.test.ts @@ -46,6 +46,7 @@ describe("collectSettingsFile", () => { "coder.tlsCertFile": "/etc/ssl/cert.pem", "coder.sshFlags": ["--disable-autostart"], "coder.defaultUrl": "https://coder.example.com", + "coder.globalConfig": "/home/user/.config/coderv2", "coder.proxyLogDirectory": "/home/user/.coder/logs", "coder.insecure": true, "coder.httpClientLogLevel": "debug", @@ -72,6 +73,10 @@ describe("collectSettingsFile", () => { defaultValue: "", globalValue: "https://coder.example.com", }, + "coder.globalConfig": { + defaultValue: "", + globalValue: "/home/user/.config/coderv2", + }, "coder.proxyLogDirectory": { defaultValue: "", globalValue: "/home/user/.coder/logs", @@ -106,6 +111,9 @@ describe("collectSettingsFile", () => { expect(settings["coder.defaultUrl"]?.effective).toBe( "https://coder.example.com", ); + expect(settings["coder.globalConfig"]?.effective).toBe( + "/home/user/.config/coderv2", + ); expect(settings["coder.proxyLogDirectory"]?.effective).toBe( "/home/user/.coder/logs", ); From d73eb2be195e92522b1483817822b4e472ad1aaf Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 17 Jun 2026 13:03:02 +0000 Subject: [PATCH 2/8] refactor: address global config review feedback --- CHANGELOG.md | 3 + src/core/cliCredentialManager.ts | 106 +++++++++++++------- src/uri/uriHandler.ts | 3 +- src/uri/utils.ts | 19 ++++ src/util.ts | 24 ++--- test/unit/core/cliCredentialManager.test.ts | 94 ++++++++++------- test/unit/core/pathResolver.test.ts | 99 +++++++++--------- test/unit/uri/utils.test.ts | 30 ++++++ test/unit/util.test.ts | 14 --- 9 files changed, 235 insertions(+), 157 deletions(-) create mode 100644 src/uri/utils.ts create mode 100644 test/unit/uri/utils.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 79ab6c08ae..32d9989808 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,9 @@ ### Added +- New `coder.globalConfig` setting to override the Coder CLI `--global-config` + directory, with `CODER_CONFIG_DIR` fallback, so file-backed CLI login/auth can + be shared with the VS Code extension when keyring auth is not active. - New **Shared Workspaces** view in the Coder sidebar that lists workspaces other users have shared with you, with search and refresh actions, so you can find and open them just like your own. diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index e46129fc06..58a490a923 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -6,7 +6,7 @@ import { promisify } from "node:util"; import * as semver from "semver"; import { isAbortError } from "../error/errorUtils"; -import { featureSetForVersion } from "../featureSet"; +import { featureSetForVersion, type FeatureSet } from "../featureSet"; import { CredentialCliError, CredentialFileError, @@ -15,7 +15,7 @@ import { import { isKeyringEnabled } from "../settings/cli"; import { getHeaderArgs } from "../settings/headers"; import { type TelemetryReporter } from "../telemetry/reporter"; -import { toSafeHost } from "../util"; +import { removeTrailingSlashes, toSafeHost } from "../uri/utils"; import { writeAtomically } from "../util/fs"; import { version } from "./cliExec"; @@ -30,6 +30,10 @@ import type { PathResolver } from "./pathResolver"; const execFileAsync = promisify(execFile); type KeyringFeature = "keyringAuth" | "keyringTokenRead"; +type TokenReadSource = + | { mode: "files" } + | { mode: "keyring"; binPath: string } + | { mode: "none" }; const EXEC_TIMEOUT_MS = 60_000; const EXEC_LOG_INTERVAL_MS = 5_000; @@ -132,33 +136,28 @@ export class CliCredentialManager { configs: Pick, options?: { signal?: AbortSignal }, ): Promise { - if (!isKeyringEnabled(configs)) { + const source = await this.resolveTokenReadSource(url, configs); + if (source.mode === "files") { return this.readCredentialFiles(url); } - - let binPath: string; - try { - binPath = await this.resolveBinary(url); - const cliVersion = semver.parse(await version(binPath)); - const featureSet = featureSetForVersion(cliVersion); - if (!featureSet.keyringAuth) { - return this.readCredentialFiles(url); - } - if (!featureSet.keyringTokenRead) { - return undefined; - } - } catch (error) { - this.logger.warn("Could not resolve CLI binary for token read:", error); + if (source.mode === "none") { return undefined; } + return this.readKeyringToken(source.binPath, url, configs, options); + } + private async readKeyringToken( + binPath: string, + url: string, + configs: Pick, + options?: { signal?: AbortSignal }, + ): Promise { const args = [...getHeaderArgs(configs), "login", "token", "--url", url]; try { const { stdout } = await this.execWithTimeout(binPath, args, { signal: options?.signal, }); - const token = stdout.trim(); - return token || undefined; + return nonEmpty(stdout); } catch (error) { if (isAbortError(error)) { throw error; @@ -206,8 +205,33 @@ export class CliCredentialManager { return undefined; } const binPath = await this.resolveBinary(url); - const cliVersion = semver.parse(await version(binPath)); - return featureSetForVersion(cliVersion)[feature] ? binPath : undefined; + return (await this.getFeatureSet(binPath))[feature] ? binPath : undefined; + } + + private async resolveTokenReadSource( + url: string, + configs: Pick, + ): Promise { + if (!isKeyringEnabled(configs)) { + return { mode: "files" }; + } + try { + const binPath = await this.resolveBinary(url); + const featureSet = await this.getFeatureSet(binPath); + if (!featureSet.keyringAuth) { + return { mode: "files" }; + } + return featureSet.keyringTokenRead + ? { mode: "keyring", binPath } + : { mode: "none" }; + } catch (error) { + this.logger.warn("Could not resolve CLI binary for token read:", error); + return { mode: "none" }; + } + } + + private async getFeatureSet(binPath: string): Promise { + return featureSetForVersion(semver.parse(await version(binPath))); } /** @@ -259,18 +283,10 @@ export class CliCredentialManager { */ private async readCredentialFiles(url: string): Promise { try { - const safeHostname = toSafeHost(url); - const [storedUrl, token] = await Promise.all([ - fs.readFile(this.pathResolver.getUrlPath(safeHostname), "utf8"), - fs.readFile( - this.pathResolver.getSessionTokenPath(safeHostname), - "utf8", - ), - ]); - if (normalizeCredentialUrl(storedUrl) !== normalizeCredentialUrl(url)) { - return undefined; - } - return token.trim() || undefined; + const files = await this.readCredentialFilePair(url); + return sameCredentialUrl(files.url, url) + ? nonEmpty(files.token) + : undefined; } catch (error) { if ((error as NodeJS.ErrnoException)?.code !== "ENOENT") { this.logger.warn("Failed to read credential files:", error); @@ -279,6 +295,17 @@ export class CliCredentialManager { } } + private async readCredentialFilePair( + url: string, + ): Promise<{ url: string; token: string }> { + const safeHostname = toSafeHost(url); + const [storedUrl, token] = await Promise.all([ + fs.readFile(this.pathResolver.getUrlPath(safeHostname), "utf8"), + fs.readFile(this.pathResolver.getSessionTokenPath(safeHostname), "utf8"), + ]); + return { url: storedUrl, token }; + } + /** * Delete URL and token files. Best-effort: never throws. */ @@ -349,6 +376,15 @@ export class CliCredentialManager { } } -function normalizeCredentialUrl(url: string): string { - return url.trim().replace(/\/+$/, ""); +function sameCredentialUrl(storedUrl: string, expectedUrl: string): boolean { + return cleanCredentialUrl(storedUrl) === cleanCredentialUrl(expectedUrl); +} + +function cleanCredentialUrl(url: string): string { + return removeTrailingSlashes(url.trim()); +} + +function nonEmpty(value: string): string | undefined { + const trimmed = value.trim(); + return trimmed || undefined; } diff --git a/src/uri/uriHandler.ts b/src/uri/uriHandler.ts index 1e57652647..411626d0c5 100644 --- a/src/uri/uriHandler.ts +++ b/src/uri/uriHandler.ts @@ -4,9 +4,10 @@ import { errToStr } from "../api/api-helper"; import { AuthTelemetry } from "../instrumentation/auth"; import { CALLBACK_PATH } from "../oauth/utils"; import { maybeAskUrl } from "../promptUtils"; -import { toSafeHost } from "../util"; import { vscodeProposed } from "../vscodeProposed"; +import { toSafeHost } from "./utils"; + import type { Commands } from "../commands"; import type { ServiceContainer } from "../core/container"; import type { DeploymentManager } from "../deployment/deploymentManager"; diff --git a/src/uri/utils.ts b/src/uri/utils.ts new file mode 100644 index 0000000000..fa7f067924 --- /dev/null +++ b/src/uri/utils.ts @@ -0,0 +1,19 @@ +import url from "node:url"; + +/** + * Given a URL, return the host in a format that is safe to write. + */ +export function toSafeHost(rawUrl: string): string { + const u = new URL(rawUrl); + // If the host is invalid, an empty string is returned. Although, `new URL` + // should already have thrown in that case. + return url.domainToASCII(u.hostname) || u.hostname; +} + +export function removeTrailingSlashes(value: string): string { + let end = value.length; + while (end > 0 && value[end - 1] === "/") { + end--; + } + return end === value.length ? value : value.slice(0, end); +} diff --git a/src/util.ts b/src/util.ts index 5ea37b8c1c..a79ad31762 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,7 +1,8 @@ import os from "node:os"; -import url from "node:url"; import * as vscode from "vscode"; +import { removeTrailingSlashes, toSafeHost } from "./uri/utils"; + export interface AuthorityParts { agent: string | undefined; sshHost: string; @@ -121,15 +122,7 @@ export function toRemoteAuthority( return remoteAuthority; } -/** - * Given a URL, return the host in a format that is safe to write. - */ -export function toSafeHost(rawUrl: string): string { - const u = new URL(rawUrl); - // If the host is invalid, an empty string is returned. Although, `new URL` - // should already have thrown in that case. - return url.domainToASCII(u.hostname) || u.hostname; -} +export { removeTrailingSlashes, toSafeHost }; /** * Substitute `${env:VAR}` with `process.env.VAR` (unset → empty string), @@ -203,11 +196,12 @@ export function escapeShellArg(arg: string): string { * the connection URL unchanged. */ export function resolveUiUrl(connectionUrl: string): string { - const alt = vscode.workspace - .getConfiguration("coder") - .get("alternativeWebUrl") - ?.trim() - .replace(/\/+$/, ""); + const alt = removeTrailingSlashes( + vscode.workspace + .getConfiguration("coder") + .get("alternativeWebUrl") + ?.trim() ?? "", + ); return alt || connectionUrl; } diff --git a/test/unit/core/cliCredentialManager.test.ts b/test/unit/core/cliCredentialManager.test.ts index e2cf036e2c..e84a04fe10 100644 --- a/test/unit/core/cliCredentialManager.test.ts +++ b/test/unit/core/cliCredentialManager.test.ts @@ -129,16 +129,41 @@ const configWithHeaders = { const TEST_PATH_RESOLVER = new PathResolver("/mock/base", "/mock/log"); const CRED_DIR = "/mock/base/dev.coder.com"; -const URL_FILE = `${CRED_DIR}/url`; -const SESSION_FILE = `${CRED_DIR}/session`; const CUSTOM_CRED_DIR = "/custom/coderv2"; -const CUSTOM_URL_FILE = `${CUSTOM_CRED_DIR}/url`; -const CUSTOM_SESSION_FILE = `${CUSTOM_CRED_DIR}/session`; -function writeCredentialFiles(url: string, token: string) { - vol.mkdirSync(CRED_DIR, { recursive: true }); - memfs.writeFileSync(URL_FILE, url); - memfs.writeFileSync(SESSION_FILE, token); +function credentialPaths(dir = CRED_DIR) { + return { + url: `${dir}/url`, + session: `${dir}/session`, + }; +} + +function writeCredentialFiles( + url: string, + token: string, + dir = CRED_DIR, +): void { + const paths = credentialPaths(dir); + vol.mkdirSync(dir, { recursive: true }); + memfs.writeFileSync(paths.url, url); + memfs.writeFileSync(paths.session, token); +} + +function readCredentialFiles(dir = CRED_DIR) { + const paths = credentialPaths(dir); + return { + url: memfs.readFileSync(paths.url, "utf8"), + session: memfs.readFileSync(paths.session, "utf8"), + }; +} + +function credentialFilesExist(dir = CRED_DIR): boolean { + const paths = credentialPaths(dir); + return memfs.existsSync(paths.url) || memfs.existsSync(paths.session); +} + +function useCustomGlobalConfig(): void { + new MockConfigurationProvider().set("coder.globalConfig", CUSTOM_CRED_DIR); } function setup(resolver?: BinaryResolver) { @@ -186,8 +211,10 @@ describe("CliCredentialManager", () => { ).resolves.toBeUndefined(); expect(execFile).not.toHaveBeenCalled(); - expect(memfs.readFileSync(URL_FILE, "utf8")).toBe(TEST_URL); - expect(memfs.readFileSync(SESSION_FILE, "utf8")).toBe("my-token"); + expect(readCredentialFiles()).toStrictEqual({ + url: TEST_URL, + session: "my-token", + }); expect(sink.expectOne("auth.credential.store")).toMatchObject({ properties: { category: "file", @@ -223,18 +250,17 @@ describe("CliCredentialManager", () => { }); it("writes files under configured global config when keyring is disabled", async () => { - new MockConfigurationProvider().set( - "coder.globalConfig", - CUSTOM_CRED_DIR, - ); + useCustomGlobalConfig(); const { manager } = setup(); await expect( manager.storeToken(TEST_URL, "my-token", configs), ).resolves.toBeUndefined(); - expect(memfs.readFileSync(CUSTOM_URL_FILE, "utf8")).toBe(TEST_URL); - expect(memfs.readFileSync(CUSTOM_SESSION_FILE, "utf8")).toBe("my-token"); + expect(readCredentialFiles(CUSTOM_CRED_DIR)).toStrictEqual({ + url: TEST_URL, + session: "my-token", + }); }); it("falls back to files when CLI version too old", async () => { @@ -247,8 +273,10 @@ describe("CliCredentialManager", () => { ).resolves.toBeUndefined(); expect(execFile).not.toHaveBeenCalled(); - expect(memfs.readFileSync(URL_FILE, "utf8")).toBe(TEST_URL); - expect(memfs.readFileSync(SESSION_FILE, "utf8")).toBe("token"); + expect(readCredentialFiles()).toStrictEqual({ + url: TEST_URL, + session: "token", + }); }); it("throws when CLI exec fails", async () => { @@ -378,13 +406,12 @@ describe("CliCredentialManager", () => { }); it("reads files under configured global config when keyring is disabled", async () => { - new MockConfigurationProvider().set( - "coder.globalConfig", + useCustomGlobalConfig(); + writeCredentialFiles( + `${TEST_URL}\n`, + "custom-file-token\n", CUSTOM_CRED_DIR, ); - vol.mkdirSync(CUSTOM_CRED_DIR, { recursive: true }); - memfs.writeFileSync(CUSTOM_URL_FILE, `${TEST_URL}\n`); - memfs.writeFileSync(CUSTOM_SESSION_FILE, "custom-file-token\n"); const { manager } = setup(); expect(await manager.readToken(TEST_URL, configs)).toBe( @@ -481,8 +508,7 @@ describe("CliCredentialManager", () => { const exec = lastExecArgs(); expect(exec.bin).toBe(TEST_BIN); expect(exec.args).toEqual(["logout", "--url", TEST_URL, "--yes"]); - expect(memfs.existsSync(URL_FILE)).toBe(false); - expect(memfs.existsSync(SESSION_FILE)).toBe(false); + expect(credentialFilesExist()).toBe(false); expect(sink.expectOne("auth.credential.clear")).toMatchObject({ properties: { category: "keyring", @@ -499,25 +525,18 @@ describe("CliCredentialManager", () => { await manager.deleteToken(TEST_URL, configs); expect(execFile).not.toHaveBeenCalled(); - expect(memfs.existsSync(URL_FILE)).toBe(false); - expect(memfs.existsSync(SESSION_FILE)).toBe(false); + expect(credentialFilesExist()).toBe(false); }); it("deletes files under configured global config", async () => { - new MockConfigurationProvider().set( - "coder.globalConfig", - CUSTOM_CRED_DIR, - ); - vol.mkdirSync(CUSTOM_CRED_DIR, { recursive: true }); - memfs.writeFileSync(CUSTOM_URL_FILE, TEST_URL); - memfs.writeFileSync(CUSTOM_SESSION_FILE, "old-token"); + useCustomGlobalConfig(); + writeCredentialFiles(TEST_URL, "old-token", CUSTOM_CRED_DIR); const { manager } = setup(); await manager.deleteToken(TEST_URL, configs); expect(execFile).not.toHaveBeenCalled(); - expect(memfs.existsSync(CUSTOM_URL_FILE)).toBe(false); - expect(memfs.existsSync(CUSTOM_SESSION_FILE)).toBe(false); + expect(credentialFilesExist(CUSTOM_CRED_DIR)).toBe(false); }); it("never throws on CLI error", async () => { @@ -573,8 +592,7 @@ describe("CliCredentialManager", () => { await manager.deleteToken(TEST_URL, configs); expect(execFile).not.toHaveBeenCalled(); - expect(memfs.existsSync(URL_FILE)).toBe(false); - expect(memfs.existsSync(SESSION_FILE)).toBe(false); + expect(credentialFilesExist()).toBe(false); }); it("passes signal through to execFile", async () => { diff --git a/test/unit/core/pathResolver.test.ts b/test/unit/core/pathResolver.test.ts index dc033db747..47e6402512 100644 --- a/test/unit/core/pathResolver.test.ts +++ b/test/unit/core/pathResolver.test.ts @@ -20,64 +20,55 @@ describe("PathResolver", () => { }); describe("getGlobalConfigDir", () => { - it("uses per-deployment global storage when no override is configured", () => { - vi.stubEnv("CODER_CONFIG_DIR", ""); - mockConfig.set("coder.globalConfig", ""); - - expectPathsEqual( - pathResolver.getGlobalConfigDir("deployment"), - path.join(basePath, "deployment"), - ); - }); - - it("uses configured global config directory directly", () => { - mockConfig.set("coder.globalConfig", "/custom/coderv2"); - - expectPathsEqual( - pathResolver.getGlobalConfigDir("deployment"), - "/custom/coderv2", - ); - }); - - it("uses CODER_CONFIG_DIR when setting is empty", () => { - vi.stubEnv("CODER_CONFIG_DIR", " /env/coderv2 "); - mockConfig.set("coder.globalConfig", ""); - - expectPathsEqual( - pathResolver.getGlobalConfigDir("deployment"), - "/env/coderv2", - ); - }); - - it("uses setting before CODER_CONFIG_DIR", () => { - vi.stubEnv("CODER_CONFIG_DIR", "/env/coderv2"); - mockConfig.set("coder.globalConfig", " /setting/coderv2 "); - - expectPathsEqual( - pathResolver.getGlobalConfigDir("deployment"), - "/setting/coderv2", - ); - }); - - it("normalizes configured global config directory", () => { - mockConfig.set("coder.globalConfig", "/custom/../coderv2/./dir"); + it.each([ + { + name: "uses per-deployment global storage when no override is configured", + setting: "", + env: "", + expected: path.join(basePath, "deployment"), + }, + { + name: "uses configured global config directory directly", + setting: "/custom/coderv2", + expected: "/custom/coderv2", + }, + { + name: "uses CODER_CONFIG_DIR when setting is empty", + setting: "", + env: " /env/coderv2 ", + expected: "/env/coderv2", + }, + { + name: "uses setting before CODER_CONFIG_DIR", + setting: " /setting/coderv2 ", + env: "/env/coderv2", + expected: "/setting/coderv2", + }, + { + name: "normalizes configured global config directory", + setting: "/custom/../coderv2/./dir", + expected: "/coderv2/dir", + }, + ])("$name", ({ setting, env, expected }) => { + vi.stubEnv("CODER_CONFIG_DIR", env); + mockConfig.set("coder.globalConfig", setting); - expectPathsEqual( - pathResolver.getGlobalConfigDir("deployment"), - "/coderv2/dir", - ); + expectPathsEqual(pathResolver.getGlobalConfigDir("deployment"), expected); }); - it("expands paths in configured global config directory", () => { - mockConfig.set("coder.globalConfig", "~/coderv2"); - const result = pathResolver.getGlobalConfigDir("deployment"); - - expect(result).not.toContain("~"); - expect(result).toContain("coderv2"); - }); + it.each([ + { + name: "configured global config directory", + setting: "~/coderv2", + }, + { + name: "CODER_CONFIG_DIR", + env: "~/coderv2", + }, + ])("expands paths in $name", ({ setting, env }) => { + vi.stubEnv("CODER_CONFIG_DIR", env); + mockConfig.set("coder.globalConfig", setting ?? ""); - it("expands paths in CODER_CONFIG_DIR", () => { - vi.stubEnv("CODER_CONFIG_DIR", "~/coderv2"); const result = pathResolver.getGlobalConfigDir("deployment"); expect(result).not.toContain("~"); diff --git a/test/unit/uri/utils.test.ts b/test/unit/uri/utils.test.ts new file mode 100644 index 0000000000..74efed3715 --- /dev/null +++ b/test/unit/uri/utils.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; + +import { removeTrailingSlashes, toSafeHost } from "@/uri/utils"; + +describe("toSafeHost", () => { + it.each([ + ["https://foobar:8080", "foobar"], + ["https://ほげ", "xn--18j4d"], + ["https://test.😉.invalid", "test.xn--n28h.invalid"], + ["https://dev.😉-coder.com", "dev.xn---coder-vx74e.com"], + ["http://ignore-port.com:8080", "ignore-port.com"], + ])("returns %s for %s", (input, expected) => { + expect(toSafeHost(input)).toBe(expected); + }); + + it("throws for invalid URLs", () => { + expect(() => toSafeHost("invalid url")).toThrow("Invalid URL"); + }); +}); + +describe("removeTrailingSlashes", () => { + it.each([ + ["https://coder.example.com", "https://coder.example.com"], + ["https://coder.example.com/", "https://coder.example.com"], + ["https://coder.example.com///", "https://coder.example.com"], + ["///", ""], + ])("returns %j for %j", (input, expected) => { + expect(removeTrailingSlashes(input)).toBe(expected); + }); +}); diff --git a/test/unit/util.test.ts b/test/unit/util.test.ts index 952ee35afd..e2b301f47e 100644 --- a/test/unit/util.test.ts +++ b/test/unit/util.test.ts @@ -13,7 +13,6 @@ import { parseRemoteAuthority, resolveUiUrl, toRemoteAuthority, - toSafeHost, } from "@/util"; import { MockConfigurationProvider } from "../mocks/testHelpers"; @@ -172,19 +171,6 @@ describe("parseRemoteAuthority", () => { ); }); -describe("toSafeHost", () => { - it("escapes url host", () => { - expect(toSafeHost("https://foobar:8080")).toBe("foobar"); - expect(toSafeHost("https://ほげ")).toBe("xn--18j4d"); - expect(toSafeHost("https://test.😉.invalid")).toBe("test.xn--n28h.invalid"); - expect(toSafeHost("https://dev.😉-coder.com")).toBe( - "dev.xn---coder-vx74e.com", - ); - expect(() => toSafeHost("invalid url")).toThrow("Invalid URL"); - expect(toSafeHost("http://ignore-port.com:8080")).toBe("ignore-port.com"); - }); -}); - describe("countSubstring", () => { it("handles empty strings", () => { expect(countSubstring("", "")).toBe(0); From 82ad283f7f80e0ad6d9901ad885081449803ead2 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 18 Jun 2026 12:44:42 +0300 Subject: [PATCH 3/8] refactor: reorganize util modules and return token source Split src/util.ts into focused modules: src/util/uri.ts (toSafeHost, removeTrailingSlashes, resolveUiUrl, openInBrowser) and src/util/authority.ts (Remote SSH authority helpers), replacing src/uri/utils.ts. Migrate all importers and mirror the unit tests. Make CliCredentialManager.readToken return the token together with its source ("keyring" | "files") so LoginCoordinator labels the login method from the actual source instead of re-deriving it from whether keyring is enabled. --- src/api/authInterceptor.ts | 2 +- src/commands.ts | 3 +- src/core/cliCredentialManager.ts | 33 ++- src/core/cliManager.ts | 2 +- src/core/secretsManager.ts | 2 +- src/login/loginCoordinator.ts | 14 +- src/oauth/authorizer.ts | 2 +- src/remote/remote.ts | 5 +- src/remote/workspaceStateMachine.ts | 2 +- src/uri/uriHandler.ts | 3 +- src/uri/utils.ts | 19 -- src/util.ts | 118 -------- src/util/authority.ts | 87 ++++++ src/util/uri.ts | 43 +++ src/webviews/tasks/tasksPanelProvider.ts | 2 +- test/unit/core/cliCredentialManager.test.ts | 19 +- test/unit/login/loginCoordinator.test.ts | 7 +- .../unit/remote/workspaceStateMachine.test.ts | 2 +- test/unit/uri/utils.test.ts | 30 --- test/unit/util.test.ts | 252 ------------------ test/unit/util/authority.test.ts | 161 +++++++++++ test/unit/util/uri.test.ts | 128 +++++++++ 22 files changed, 478 insertions(+), 458 deletions(-) delete mode 100644 src/uri/utils.ts create mode 100644 src/util/authority.ts create mode 100644 src/util/uri.ts delete mode 100644 test/unit/uri/utils.test.ts create mode 100644 test/unit/util/authority.test.ts create mode 100644 test/unit/util/uri.test.ts diff --git a/src/api/authInterceptor.ts b/src/api/authInterceptor.ts index 4341c6d3b5..ea533f6e40 100644 --- a/src/api/authInterceptor.ts +++ b/src/api/authInterceptor.ts @@ -2,7 +2,7 @@ import { type AxiosError, isAxiosError } from "axios"; import { AuthTelemetry } from "../instrumentation/auth"; import { OAuthError } from "../oauth/errors"; -import { toSafeHost } from "../util"; +import { toSafeHost } from "../util/uri"; import type * as vscode from "vscode"; diff --git a/src/commands.ts b/src/commands.ts index 6431b80c8b..d85f59df03 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -46,7 +46,8 @@ import { import { resolveCliAuth } from "./settings/cli"; import { appendVsCodeLogs } from "./supportBundle/appendVsCodeLogs"; import { runExportTelemetryCommand } from "./telemetry/export/command"; -import { openInBrowser, toRemoteAuthority, toSafeHost } from "./util"; +import { toRemoteAuthority } from "./util/authority"; +import { openInBrowser, toSafeHost } from "./util/uri"; import { vscodeProposed } from "./vscodeProposed"; import { parseNetcheckReport } from "./webviews/netcheck/types"; import { parseSpeedtestResult } from "./webviews/speedtest/types"; diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index 58a490a923..c3b52f8497 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -15,8 +15,8 @@ import { import { isKeyringEnabled } from "../settings/cli"; import { getHeaderArgs } from "../settings/headers"; import { type TelemetryReporter } from "../telemetry/reporter"; -import { removeTrailingSlashes, toSafeHost } from "../uri/utils"; import { writeAtomically } from "../util/fs"; +import { removeTrailingSlashes, toSafeHost } from "../util/uri"; import { version } from "./cliExec"; @@ -35,6 +35,11 @@ type TokenReadSource = | { mode: "keyring"; binPath: string } | { mode: "none" }; +export interface CliCredential { + token: string; + source: "keyring" | "files"; +} + const EXEC_TIMEOUT_MS = 60_000; const EXEC_LOG_INTERVAL_MS = 5_000; @@ -128,22 +133,30 @@ export class CliCredentialManager { /** * Read a token from CLI-managed credentials. Uses `coder login token --url` * when keyring auth is active, otherwise reads the file credentials under - * --global-config. Returns undefined on any failure (resolver, CLI, empty - * output). Throws AbortError when the signal is aborted. + * --global-config. Returns the token and the source it came from, or + * undefined on any failure (resolver, CLI, empty output). Throws AbortError + * when the signal is aborted. */ public async readToken( url: string, configs: Pick, options?: { signal?: AbortSignal }, - ): Promise { + ): Promise { const source = await this.resolveTokenReadSource(url, configs); if (source.mode === "files") { - return this.readCredentialFiles(url); + const token = await this.readCredentialFiles(url); + return token ? { token, source: "files" } : undefined; } if (source.mode === "none") { return undefined; } - return this.readKeyringToken(source.binPath, url, configs, options); + const token = await this.readKeyringToken( + source.binPath, + url, + configs, + options, + ); + return token ? { token, source: "keyring" } : undefined; } private async readKeyringToken( @@ -284,7 +297,7 @@ export class CliCredentialManager { private async readCredentialFiles(url: string): Promise { try { const files = await this.readCredentialFilePair(url); - return sameCredentialUrl(files.url, url) + return sameNormalizedUrl(files.url, url) ? nonEmpty(files.token) : undefined; } catch (error) { @@ -376,11 +389,11 @@ export class CliCredentialManager { } } -function sameCredentialUrl(storedUrl: string, expectedUrl: string): boolean { - return cleanCredentialUrl(storedUrl) === cleanCredentialUrl(expectedUrl); +function sameNormalizedUrl(storedUrl: string, expectedUrl: string): boolean { + return normalizeUrl(storedUrl) === normalizeUrl(expectedUrl); } -function cleanCredentialUrl(url: string): string { +function normalizeUrl(url: string): string { return removeTrailingSlashes(url.trim()); } diff --git a/src/core/cliManager.ts b/src/core/cliManager.ts index 7154640b75..87de6b3acd 100644 --- a/src/core/cliManager.ts +++ b/src/core/cliManager.ts @@ -23,8 +23,8 @@ import { import * as pgp from "../pgp"; import { withCancellableProgress, withOptionalProgress } from "../progress"; import { isKeyringEnabled } from "../settings/cli"; -import { toSafeHost } from "../util"; import { tempFilePath } from "../util/fs"; +import { toSafeHost } from "../util/uri"; import { vscodeProposed } from "../vscodeProposed"; import { BinaryLock } from "./binaryLock"; diff --git a/src/core/secretsManager.ts b/src/core/secretsManager.ts index 1d9c22be9e..5255c122c8 100644 --- a/src/core/secretsManager.ts +++ b/src/core/secretsManager.ts @@ -1,7 +1,7 @@ import { z } from "zod"; import { DeploymentSchema, type Deployment } from "../deployment/types"; -import { toSafeHost } from "../util"; +import { toSafeHost } from "../util/uri"; import type { OAuth2ClientRegistrationResponse } from "coder/site/src/api/typesGenerated"; import type { Memento, SecretStorage, Disposable } from "vscode"; diff --git a/src/login/loginCoordinator.ts b/src/login/loginCoordinator.ts index 8c1d0b1e2e..0023b95dbc 100644 --- a/src/login/loginCoordinator.ts +++ b/src/login/loginCoordinator.ts @@ -10,7 +10,7 @@ import { buildOAuthTokenData } from "../oauth/utils"; import { withOptionalProgress } from "../progress"; import { maybeAskAuthMethod, maybeAskUrl } from "../promptUtils"; import { isKeyringEnabled } from "../settings/cli"; -import { openInBrowser } from "../util"; +import { openInBrowser } from "../util/uri"; import { vscodeProposed } from "../vscodeProposed"; import type { User } from "coder/site/src/api/typesGenerated"; @@ -330,23 +330,23 @@ export class LoginCoordinator implements vscode.Disposable { cancellable: true, }, ); - const cliCredentialToken = cliCredentialResult.ok + const cliCredential = cliCredentialResult.ok ? cliCredentialResult.value : undefined; if ( - cliCredentialToken && - cliCredentialToken !== providedToken && - cliCredentialToken !== auth?.token + cliCredential && + cliCredential.token !== providedToken && + cliCredential.token !== auth?.token ) { this.logger.debug("Trying token from CLI credentials"); const result = await this.tryTokenAuth( client, - cliCredentialToken, + cliCredential.token, isAutoLogin, ); if (result !== "unauthorized") { return withLoginMethod( - keyringEnabled ? "keyring_token" : "cli_token", + cliCredential.source === "keyring" ? "keyring_token" : "cli_token", result, ); } diff --git a/src/oauth/authorizer.ts b/src/oauth/authorizer.ts index 809ccfaed5..3da0d7c11f 100644 --- a/src/oauth/authorizer.ts +++ b/src/oauth/authorizer.ts @@ -1,7 +1,7 @@ import * as vscode from "vscode"; import { CoderApi } from "../api/coderApi"; -import { resolveUiUrl } from "../util"; +import { resolveUiUrl } from "../util/uri"; import { AUTH_GRANT_TYPE, diff --git a/src/remote/remote.ts b/src/remote/remote.ts index c80b9417eb..dbfec53399 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -37,13 +37,12 @@ import { resolveCliAuth, } from "../settings/cli"; import { getHeaderCommand } from "../settings/headers"; +import { escapeCommandArg, expandPath } from "../util"; import { AuthorityPrefix, type AuthorityParts, - escapeCommandArg, - expandPath, parseRemoteAuthority, -} from "../util"; +} from "../util/authority"; import { vscodeProposed } from "../vscodeProposed"; import { WorkspaceMonitor } from "../workspace/workspaceMonitor"; diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index c459aa5026..f0915da4cf 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -37,7 +37,7 @@ import type { StartupMode } from "../core/mementoManager"; import type { FeatureSet } from "../featureSet"; import type { Logger } from "../logging/logger"; import type { CliAuth } from "../settings/cli"; -import type { AuthorityParts } from "../util"; +import type { AuthorityParts } from "../util/authority"; /** * Manages workspace and agent state transitions until ready for SSH connection. diff --git a/src/uri/uriHandler.ts b/src/uri/uriHandler.ts index 411626d0c5..efd8f1b7d5 100644 --- a/src/uri/uriHandler.ts +++ b/src/uri/uriHandler.ts @@ -4,10 +4,9 @@ import { errToStr } from "../api/api-helper"; import { AuthTelemetry } from "../instrumentation/auth"; import { CALLBACK_PATH } from "../oauth/utils"; import { maybeAskUrl } from "../promptUtils"; +import { toSafeHost } from "../util/uri"; import { vscodeProposed } from "../vscodeProposed"; -import { toSafeHost } from "./utils"; - import type { Commands } from "../commands"; import type { ServiceContainer } from "../core/container"; import type { DeploymentManager } from "../deployment/deploymentManager"; diff --git a/src/uri/utils.ts b/src/uri/utils.ts deleted file mode 100644 index fa7f067924..0000000000 --- a/src/uri/utils.ts +++ /dev/null @@ -1,19 +0,0 @@ -import url from "node:url"; - -/** - * Given a URL, return the host in a format that is safe to write. - */ -export function toSafeHost(rawUrl: string): string { - const u = new URL(rawUrl); - // If the host is invalid, an empty string is returned. Although, `new URL` - // should already have thrown in that case. - return url.domainToASCII(u.hostname) || u.hostname; -} - -export function removeTrailingSlashes(value: string): string { - let end = value.length; - while (end > 0 && value[end - 1] === "/") { - end--; - } - return end === value.length ? value : value.slice(0, end); -} diff --git a/src/util.ts b/src/util.ts index a79ad31762..33510981c5 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,23 +1,4 @@ import os from "node:os"; -import * as vscode from "vscode"; - -import { removeTrailingSlashes, toSafeHost } from "./uri/utils"; - -export interface AuthorityParts { - agent: string | undefined; - sshHost: string; - safeHostname: string; - username: string; - 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"; - -const authorityHostPrefix = `${AuthorityPrefix}.`; -const invalidAuthorityMessage = - "Invalid Coder SSH authority. Must be: ----(.)"; // Regex patterns to find the SSH port from Remote SSH extension logs. // `ms-vscode-remote.remote-ssh`: `-> socksPort ->` or `between local port ` @@ -52,78 +33,6 @@ export function findPort(text: string): number | null { return Number.parseInt(portStr); } -/** - * 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.----(.) - * - * 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) { - return null; - } - - const parts = sshHost.split("--"); - if (!parts[0].startsWith(authorityHostPrefix)) { - return null; - } - - 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 username = parts[parts.length - 2]; - const workspaceAndAgent = parts[parts.length - 1]; - if (!safeHostname || !username || !workspaceAndAgent) { - throw new Error(invalidAuthorityMessage); - } - - let workspace = workspaceAndAgent; - let agent = ""; - const workspaceParts = workspaceAndAgent.split("."); - // Multiple dots are ambiguous because workspace and agent share this separator. - if (workspaceParts.length === 2) { - workspace = workspaceParts[0]; - agent = workspaceParts[1]; - if (!workspace || !agent) { - throw new Error(invalidAuthorityMessage); - } - } - - return { - agent, - sshHost, - safeHostname, - username, - workspace, - }; -} - -export function toRemoteAuthority( - baseUrl: string, - workspaceOwner: string, - workspaceName: string, - workspaceAgent: string | undefined, -): string { - let remoteAuthority = `ssh-remote+${AuthorityPrefix}.${toSafeHost(baseUrl)}--${workspaceOwner}--${workspaceName}`; - if (workspaceAgent) { - remoteAuthority += `.${workspaceAgent}`; - } - return remoteAuthority; -} - -export { removeTrailingSlashes, toSafeHost }; - /** * Substitute `${env:VAR}` with `process.env.VAR` (unset → empty string), * `${userHome}` (anywhere) with `os.homedir()`, and a leading `~` with @@ -189,30 +98,3 @@ export function escapeShellArg(arg: string): string { } return `'${arg.replace(/'/g, "'\\''")}'`; } - -/** - * Return the URL for opening Coder pages in the browser. Uses the - * `coder.alternativeWebUrl` setting when configured, otherwise returns - * the connection URL unchanged. - */ -export function resolveUiUrl(connectionUrl: string): string { - const alt = removeTrailingSlashes( - vscode.workspace - .getConfiguration("coder") - .get("alternativeWebUrl") - ?.trim() ?? "", - ); - return alt || connectionUrl; -} - -/** - * Open a path on the Coder deployment in the user's browser, applying - * `coder.alternativeWebUrl` when configured. - */ -export function openInBrowser( - connectionUrl: string, - path: string, -): Thenable { - const base = vscode.Uri.parse(resolveUiUrl(connectionUrl)); - return vscode.env.openExternal(vscode.Uri.joinPath(base, path)); -} diff --git a/src/util/authority.ts b/src/util/authority.ts new file mode 100644 index 0000000000..b992006360 --- /dev/null +++ b/src/util/authority.ts @@ -0,0 +1,87 @@ +import { toSafeHost } from "./uri"; + +export interface AuthorityParts { + agent: string | undefined; + sshHost: string; + safeHostname: string; + username: string; + 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"; + +const authorityHostPrefix = `${AuthorityPrefix}.`; +const invalidAuthorityMessage = + "Invalid Coder SSH authority. Must be: ----(.)"; + +/** + * 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.----(.) + * + * 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) { + return null; + } + + const parts = sshHost.split("--"); + if (!parts[0].startsWith(authorityHostPrefix)) { + return null; + } + + 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 username = parts[parts.length - 2]; + const workspaceAndAgent = parts[parts.length - 1]; + if (!safeHostname || !username || !workspaceAndAgent) { + throw new Error(invalidAuthorityMessage); + } + + let workspace = workspaceAndAgent; + let agent = ""; + const workspaceParts = workspaceAndAgent.split("."); + // Multiple dots are ambiguous because workspace and agent share this separator. + if (workspaceParts.length === 2) { + workspace = workspaceParts[0]; + agent = workspaceParts[1]; + if (!workspace || !agent) { + throw new Error(invalidAuthorityMessage); + } + } + + return { + agent, + sshHost, + safeHostname, + username, + workspace, + }; +} + +export function toRemoteAuthority( + baseUrl: string, + workspaceOwner: string, + workspaceName: string, + workspaceAgent: string | undefined, +): string { + let remoteAuthority = `ssh-remote+${AuthorityPrefix}.${toSafeHost(baseUrl)}--${workspaceOwner}--${workspaceName}`; + if (workspaceAgent) { + remoteAuthority += `.${workspaceAgent}`; + } + return remoteAuthority; +} diff --git a/src/util/uri.ts b/src/util/uri.ts new file mode 100644 index 0000000000..d8fcea6e3d --- /dev/null +++ b/src/util/uri.ts @@ -0,0 +1,43 @@ +import url from "node:url"; +import * as vscode from "vscode"; + +/** + * Given a URL, return the host in a format that is safe to write. + */ +export function toSafeHost(rawUrl: string): string { + const u = new URL(rawUrl); + // If the host is invalid, an empty string is returned. Although, `new URL` + // should already have thrown in that case. + return url.domainToASCII(u.hostname) || u.hostname; +} + +export function removeTrailingSlashes(value: string): string { + return value.replace(/\/+$/, ""); +} + +/** + * Return the URL for opening Coder pages in the browser. Uses the + * `coder.alternativeWebUrl` setting when configured, otherwise returns + * the connection URL unchanged. + */ +export function resolveUiUrl(connectionUrl: string): string { + const alt = removeTrailingSlashes( + vscode.workspace + .getConfiguration("coder") + .get("alternativeWebUrl") + ?.trim() ?? "", + ); + return alt || connectionUrl; +} + +/** + * Open a path on the Coder deployment in the user's browser, applying + * `coder.alternativeWebUrl` when configured. + */ +export function openInBrowser( + connectionUrl: string, + path: string, +): Thenable { + const base = vscode.Uri.parse(resolveUiUrl(connectionUrl)); + return vscode.env.openExternal(vscode.Uri.joinPath(base, path)); +} diff --git a/src/webviews/tasks/tasksPanelProvider.ts b/src/webviews/tasks/tasksPanelProvider.ts index 7aad4ef364..9c465583dd 100644 --- a/src/webviews/tasks/tasksPanelProvider.ts +++ b/src/webviews/tasks/tasksPanelProvider.ts @@ -25,7 +25,7 @@ import { streamBuildLogs, } from "../../api/workspace"; import { type Logger } from "../../logging/logger"; -import { openInBrowser } from "../../util"; +import { openInBrowser } from "../../util/uri"; import { vscodeProposed } from "../../vscodeProposed"; import { dispatchCommand, diff --git a/test/unit/core/cliCredentialManager.test.ts b/test/unit/core/cliCredentialManager.test.ts index e84a04fe10..295db14f6d 100644 --- a/test/unit/core/cliCredentialManager.test.ts +++ b/test/unit/core/cliCredentialManager.test.ts @@ -365,7 +365,7 @@ describe("CliCredentialManager", () => { const token = await manager.readToken(TEST_URL, configs); expect(resolver).toHaveBeenCalledWith(TEST_URL); - expect(token).toBe("my-token"); + expect(token).toEqual({ token: "my-token", source: "keyring" }); expect(lastExecArgs().args).toEqual([ "login", "token", @@ -401,7 +401,10 @@ describe("CliCredentialManager", () => { stubExecFile({ stdout: "my-token" }); const { manager } = setup(); - expect(await manager.readToken(TEST_URL, configs)).toBe("file-token"); + expect(await manager.readToken(TEST_URL, configs)).toEqual({ + token: "file-token", + source: "files", + }); expect(execFile).not.toHaveBeenCalled(); }); @@ -414,9 +417,10 @@ describe("CliCredentialManager", () => { ); const { manager } = setup(); - expect(await manager.readToken(TEST_URL, configs)).toBe( - "custom-file-token", - ); + expect(await manager.readToken(TEST_URL, configs)).toEqual({ + token: "custom-file-token", + source: "files", + }); expect(execFile).not.toHaveBeenCalled(); }); @@ -437,7 +441,10 @@ describe("CliCredentialManager", () => { writeCredentialFiles(TEST_URL, "file-token"); const { manager, resolver } = setup(); - expect(await manager.readToken(TEST_URL, configs)).toBe("file-token"); + expect(await manager.readToken(TEST_URL, configs)).toEqual({ + token: "file-token", + source: "files", + }); expect(resolver).toHaveBeenCalledWith(TEST_URL); expect(execFile).not.toHaveBeenCalled(); }); diff --git a/test/unit/login/loginCoordinator.test.ts b/test/unit/login/loginCoordinator.test.ts index 71e66fe751..e5d212ebdf 100644 --- a/test/unit/login/loginCoordinator.test.ts +++ b/test/unit/login/loginCoordinator.test.ts @@ -210,9 +210,10 @@ describe("LoginCoordinator", () => { mockSuccessfulAuth, } = createTestContext(); const user = mockSuccessfulAuth(); - vi.mocked(mockCredentialManager.readToken).mockResolvedValueOnce( - "cli-credential-token", - ); + vi.mocked(mockCredentialManager.readToken).mockResolvedValueOnce({ + token: "cli-credential-token", + source: "files", + }); const result = await coordinator.ensureLoggedIn({ url: TEST_URL, diff --git a/test/unit/remote/workspaceStateMachine.test.ts b/test/unit/remote/workspaceStateMachine.test.ts index fab7d14721..cf9b55538c 100644 --- a/test/unit/remote/workspaceStateMachine.test.ts +++ b/test/unit/remote/workspaceStateMachine.test.ts @@ -42,7 +42,7 @@ import type { CoderApi } from "@/api/coderApi"; import type { StartupMode } from "@/core/mementoManager"; import type { FeatureSet } from "@/featureSet"; import type { TelemetryService } from "@/telemetry/service"; -import type { AuthorityParts } from "@/util"; +import type { AuthorityParts } from "@/util/authority"; vi.mock("@/api/workspace", async (importActual) => { const { LazyStream } = await importActual(); diff --git a/test/unit/uri/utils.test.ts b/test/unit/uri/utils.test.ts deleted file mode 100644 index 74efed3715..0000000000 --- a/test/unit/uri/utils.test.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { removeTrailingSlashes, toSafeHost } from "@/uri/utils"; - -describe("toSafeHost", () => { - it.each([ - ["https://foobar:8080", "foobar"], - ["https://ほげ", "xn--18j4d"], - ["https://test.😉.invalid", "test.xn--n28h.invalid"], - ["https://dev.😉-coder.com", "dev.xn---coder-vx74e.com"], - ["http://ignore-port.com:8080", "ignore-port.com"], - ])("returns %s for %s", (input, expected) => { - expect(toSafeHost(input)).toBe(expected); - }); - - it("throws for invalid URLs", () => { - expect(() => toSafeHost("invalid url")).toThrow("Invalid URL"); - }); -}); - -describe("removeTrailingSlashes", () => { - it.each([ - ["https://coder.example.com", "https://coder.example.com"], - ["https://coder.example.com/", "https://coder.example.com"], - ["https://coder.example.com///", "https://coder.example.com"], - ["///", ""], - ])("returns %j for %j", (input, expected) => { - expect(removeTrailingSlashes(input)).toBe(expected); - }); -}); diff --git a/test/unit/util.test.ts b/test/unit/util.test.ts index e2b301f47e..d9e9128165 100644 --- a/test/unit/util.test.ts +++ b/test/unit/util.test.ts @@ -1,176 +1,14 @@ import os from "node:os"; import { afterEach, beforeEach, describe, it, expect, vi } from "vitest"; -import * as vscode from "vscode"; import { - type AuthorityParts, countSubstring, escapeCommandArg, escapeShellArg, expandPath, findPort, - openInBrowser, - parseRemoteAuthority, - resolveUiUrl, - toRemoteAuthority, } from "@/util"; -import { MockConfigurationProvider } from "../mocks/testHelpers"; - -describe("parseRemoteAuthority", () => { - const remoteAuthority = (sshHost: string) => `vscode://ssh-remote+${sshHost}`; - - 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); - }); - - 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", - }, - { - label: "empty trailing segment", - sshHost: "coder-vscode.dev.coder.com--foo--bar--", - }, - { - label: "empty workspace before agent separator", - sshHost: "coder-vscode.dev.coder.com--foo--.agent", - }, - { - label: "empty agent after separator", - sshHost: "coder-vscode.dev.coder.com--foo--bar.", - }, - ])("rejects invalid authority: $label", ({ sshHost }) => { - expect(() => parseRemoteAuthority(remoteAuthority(sshHost))).toThrow( - "Invalid Coder SSH authority", - ); - }); - - interface ParseCase { - label: string; - sshHost: string; - safeHostname: string; - workspace: string; - agent?: string; - username?: 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", - }, - { - 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", - 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", - }, - ])( - "parses $label", - ({ sshHost, safeHostname, workspace, agent, username }) => { - expect(parseRemoteAuthority(remoteAuthority(sshHost))).toStrictEqual({ - agent: agent ?? "", - sshHost, - safeHostname, - username: username ?? "foo", - workspace, - } satisfies AuthorityParts); - }, - ); -}); - describe("countSubstring", () => { it("handles empty strings", () => { expect(countSubstring("", "")).toBe(0); @@ -413,93 +251,3 @@ describe("findPort", () => { expect(findPort(log)).toBe(3333); }); }); - -describe("resolveUiUrl", () => { - let configurationProvider: MockConfigurationProvider; - - beforeEach(() => { - configurationProvider = new MockConfigurationProvider(); - }); - - it("returns the connection URL when no alternative is configured", () => { - expect(resolveUiUrl("https://coder.example.com:7004")).toBe( - "https://coder.example.com:7004", - ); - }); - - it.each([ - { name: "empty", value: "" }, - { name: "whitespace", value: " " }, - ])( - "returns the connection URL when the alternative is $name", - ({ value }) => { - configurationProvider.set("coder.alternativeWebUrl", value); - expect(resolveUiUrl("https://coder.example.com:7004")).toBe( - "https://coder.example.com:7004", - ); - }, - ); - - it.each([ - { - name: "uses the alternative URL when configured", - value: "https://coder.example.com", - }, - { name: "strips trailing slashes", value: "https://coder.example.com/" }, - { - name: "strips multiple trailing slashes", - value: "https://coder.example.com///", - }, - { name: "trims whitespace", value: " https://coder.example.com " }, - ])("$name", ({ value }) => { - configurationProvider.set("coder.alternativeWebUrl", value); - expect(resolveUiUrl("https://coder.example.com:7004")).toBe( - "https://coder.example.com", - ); - }); -}); - -describe("openInBrowser", () => { - let configurationProvider: MockConfigurationProvider; - - beforeEach(() => { - configurationProvider = new MockConfigurationProvider(); - vi.mocked(vscode.env.openExternal).mockClear(); - }); - - it("opens the connection URL joined with the path when no alt URL is set", () => { - openInBrowser("https://coder.example.com:7004", "/templates"); - expect(vscode.env.openExternal).toHaveBeenCalledWith( - vscode.Uri.parse("https://coder.example.com:7004/templates"), - ); - }); - - it("opens the alternative URL when configured", () => { - configurationProvider.set( - "coder.alternativeWebUrl", - "https://coder.example.com", - ); - openInBrowser("https://coder.example.com:7004", "/templates"); - expect(vscode.env.openExternal).toHaveBeenCalledWith( - vscode.Uri.parse("https://coder.example.com/templates"), - ); - }); - - it("preserves a path prefix on the alternative URL", () => { - configurationProvider.set( - "coder.alternativeWebUrl", - "https://proxy.example.com/coder", - ); - openInBrowser("https://coder.example.com:7004", "/templates"); - expect(vscode.env.openExternal).toHaveBeenCalledWith( - vscode.Uri.parse("https://proxy.example.com/coder/templates"), - ); - }); - - it("joins paths without a leading slash", () => { - openInBrowser("https://coder.example.com", "templates"); - expect(vscode.env.openExternal).toHaveBeenCalledWith( - vscode.Uri.parse("https://coder.example.com/templates"), - ); - }); -}); diff --git a/test/unit/util/authority.test.ts b/test/unit/util/authority.test.ts new file mode 100644 index 0000000000..9438a3cec0 --- /dev/null +++ b/test/unit/util/authority.test.ts @@ -0,0 +1,161 @@ +import { describe, expect, it } from "vitest"; + +import { + type AuthorityParts, + parseRemoteAuthority, + toRemoteAuthority, +} from "@/util/authority"; + +describe("parseRemoteAuthority", () => { + const remoteAuthority = (sshHost: string) => `vscode://ssh-remote+${sshHost}`; + + 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); + }); + + 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", + }, + { + label: "empty trailing segment", + sshHost: "coder-vscode.dev.coder.com--foo--bar--", + }, + { + label: "empty workspace before agent separator", + sshHost: "coder-vscode.dev.coder.com--foo--.agent", + }, + { + label: "empty agent after separator", + sshHost: "coder-vscode.dev.coder.com--foo--bar.", + }, + ])("rejects invalid authority: $label", ({ sshHost }) => { + expect(() => parseRemoteAuthority(remoteAuthority(sshHost))).toThrow( + "Invalid Coder SSH authority", + ); + }); + + interface ParseCase { + label: string; + sshHost: string; + safeHostname: string; + workspace: string; + agent?: string; + username?: 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", + }, + { + 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", + 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", + }, + ])( + "parses $label", + ({ sshHost, safeHostname, workspace, agent, username }) => { + expect(parseRemoteAuthority(remoteAuthority(sshHost))).toStrictEqual({ + agent: agent ?? "", + sshHost, + safeHostname, + username: username ?? "foo", + workspace, + } satisfies AuthorityParts); + }, + ); +}); diff --git a/test/unit/util/uri.test.ts b/test/unit/util/uri.test.ts new file mode 100644 index 0000000000..140764ea9a --- /dev/null +++ b/test/unit/util/uri.test.ts @@ -0,0 +1,128 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; + +import { + openInBrowser, + removeTrailingSlashes, + resolveUiUrl, + toSafeHost, +} from "@/util/uri"; + +import { MockConfigurationProvider } from "../../mocks/testHelpers"; + +describe("toSafeHost", () => { + it.each([ + ["https://foobar:8080", "foobar"], + ["https://ほげ", "xn--18j4d"], + ["https://test.😉.invalid", "test.xn--n28h.invalid"], + ["https://dev.😉-coder.com", "dev.xn---coder-vx74e.com"], + ["http://ignore-port.com:8080", "ignore-port.com"], + ])("returns %s for %s", (input, expected) => { + expect(toSafeHost(input)).toBe(expected); + }); + + it("throws for invalid URLs", () => { + expect(() => toSafeHost("invalid url")).toThrow("Invalid URL"); + }); +}); + +describe("removeTrailingSlashes", () => { + it.each([ + ["https://coder.example.com", "https://coder.example.com"], + ["https://coder.example.com/", "https://coder.example.com"], + ["https://coder.example.com///", "https://coder.example.com"], + ["///", ""], + ])("returns %j for %j", (input, expected) => { + expect(removeTrailingSlashes(input)).toBe(expected); + }); +}); + +describe("resolveUiUrl", () => { + let configurationProvider: MockConfigurationProvider; + + beforeEach(() => { + configurationProvider = new MockConfigurationProvider(); + }); + + it("returns the connection URL when no alternative is configured", () => { + expect(resolveUiUrl("https://coder.example.com:7004")).toBe( + "https://coder.example.com:7004", + ); + }); + + it.each([ + { name: "empty", value: "" }, + { name: "whitespace", value: " " }, + ])( + "returns the connection URL when the alternative is $name", + ({ value }) => { + configurationProvider.set("coder.alternativeWebUrl", value); + expect(resolveUiUrl("https://coder.example.com:7004")).toBe( + "https://coder.example.com:7004", + ); + }, + ); + + it.each([ + { + name: "uses the alternative URL when configured", + value: "https://coder.example.com", + }, + { name: "strips trailing slashes", value: "https://coder.example.com/" }, + { + name: "strips multiple trailing slashes", + value: "https://coder.example.com///", + }, + { name: "trims whitespace", value: " https://coder.example.com " }, + ])("$name", ({ value }) => { + configurationProvider.set("coder.alternativeWebUrl", value); + expect(resolveUiUrl("https://coder.example.com:7004")).toBe( + "https://coder.example.com", + ); + }); +}); + +describe("openInBrowser", () => { + let configurationProvider: MockConfigurationProvider; + + beforeEach(() => { + configurationProvider = new MockConfigurationProvider(); + vi.mocked(vscode.env.openExternal).mockClear(); + }); + + it("opens the connection URL joined with the path when no alt URL is set", () => { + openInBrowser("https://coder.example.com:7004", "/templates"); + expect(vscode.env.openExternal).toHaveBeenCalledWith( + vscode.Uri.parse("https://coder.example.com:7004/templates"), + ); + }); + + it("opens the alternative URL when configured", () => { + configurationProvider.set( + "coder.alternativeWebUrl", + "https://coder.example.com", + ); + openInBrowser("https://coder.example.com:7004", "/templates"); + expect(vscode.env.openExternal).toHaveBeenCalledWith( + vscode.Uri.parse("https://coder.example.com/templates"), + ); + }); + + it("preserves a path prefix on the alternative URL", () => { + configurationProvider.set( + "coder.alternativeWebUrl", + "https://proxy.example.com/coder", + ); + openInBrowser("https://coder.example.com:7004", "/templates"); + expect(vscode.env.openExternal).toHaveBeenCalledWith( + vscode.Uri.parse("https://proxy.example.com/coder/templates"), + ); + }); + + it("joins paths without a leading slash", () => { + openInBrowser("https://coder.example.com", "templates"); + expect(vscode.env.openExternal).toHaveBeenCalledWith( + vscode.Uri.parse("https://coder.example.com/templates"), + ); + }); +}); From 2866718a1567390e6f193f0f2c1b58340ceec278 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 23 Jun 2026 10:30:20 +0300 Subject: [PATCH 4/8] test: cover keyring login path, toRemoteAuthority, and dedupe URL normalization - Add loginCoordinator test for the keyring_token method (source: "keyring") - Add comprehensive toRemoteAuthority tests over varied URI types - Test toSafeHost/normalizeUrl with Arabic alongside Japanese IDNs - Consolidate the trim + strip-trailing-slashes logic into a shared normalizeUrl in util/uri, reused by resolveUiUrl and cliCredentialManager --- src/core/cliCredentialManager.ts | 6 +- src/util/uri.ts | 10 +++- test/unit/login/loginCoordinator.test.ts | 22 +++++++ test/unit/util/authority.test.ts | 73 ++++++++++++++++++++++++ test/unit/util/uri.test.ts | 14 +++++ 5 files changed, 117 insertions(+), 8 deletions(-) diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index c3b52f8497..f09422ec81 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -16,7 +16,7 @@ import { isKeyringEnabled } from "../settings/cli"; import { getHeaderArgs } from "../settings/headers"; import { type TelemetryReporter } from "../telemetry/reporter"; import { writeAtomically } from "../util/fs"; -import { removeTrailingSlashes, toSafeHost } from "../util/uri"; +import { normalizeUrl, toSafeHost } from "../util/uri"; import { version } from "./cliExec"; @@ -393,10 +393,6 @@ function sameNormalizedUrl(storedUrl: string, expectedUrl: string): boolean { return normalizeUrl(storedUrl) === normalizeUrl(expectedUrl); } -function normalizeUrl(url: string): string { - return removeTrailingSlashes(url.trim()); -} - function nonEmpty(value: string): string | undefined { const trimmed = value.trim(); return trimmed || undefined; diff --git a/src/util/uri.ts b/src/util/uri.ts index d8fcea6e3d..0072118e5d 100644 --- a/src/util/uri.ts +++ b/src/util/uri.ts @@ -15,17 +15,21 @@ export function removeTrailingSlashes(value: string): string { return value.replace(/\/+$/, ""); } +/** Trim surrounding whitespace and strip trailing slashes from a URL. */ +export function normalizeUrl(value: string): string { + return removeTrailingSlashes(value.trim()); +} + /** * Return the URL for opening Coder pages in the browser. Uses the * `coder.alternativeWebUrl` setting when configured, otherwise returns * the connection URL unchanged. */ export function resolveUiUrl(connectionUrl: string): string { - const alt = removeTrailingSlashes( + const alt = normalizeUrl( vscode.workspace .getConfiguration("coder") - .get("alternativeWebUrl") - ?.trim() ?? "", + .get("alternativeWebUrl") ?? "", ); return alt || connectionUrl; } diff --git a/test/unit/login/loginCoordinator.test.ts b/test/unit/login/loginCoordinator.test.ts index e5d212ebdf..0a5224142d 100644 --- a/test/unit/login/loginCoordinator.test.ts +++ b/test/unit/login/loginCoordinator.test.ts @@ -232,6 +232,28 @@ describe("LoginCoordinator", () => { expect(auth?.token).toBe("cli-credential-token"); }); + it("reports keyring_token method when the credential comes from the keyring", async () => { + const { mockCredentialManager, coordinator, mockSuccessfulAuth } = + createTestContext(); + const user = mockSuccessfulAuth(); + vi.mocked(mockCredentialManager.readToken).mockResolvedValueOnce({ + token: "keyring-token", + source: "keyring", + }); + + const result = await coordinator.ensureLoggedIn({ + url: TEST_URL, + safeHostname: TEST_HOSTNAME, + }); + + expect(result).toEqual({ + success: true, + method: "keyring_token", + user, + token: "keyring-token", + }); + }); + it("prompts for token when no stored auth exists", async () => { const { userInteraction, diff --git a/test/unit/util/authority.test.ts b/test/unit/util/authority.test.ts index 9438a3cec0..8b0dd5f2d5 100644 --- a/test/unit/util/authority.test.ts +++ b/test/unit/util/authority.test.ts @@ -159,3 +159,76 @@ describe("parseRemoteAuthority", () => { }, ); }); + +describe("toRemoteAuthority", () => { + interface ToRemoteAuthorityCase { + url: string; + owner: string; + workspace: string; + agent: string | undefined; + expected: string; + } + it.each([ + { + 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", + }, + { + url: "http://192.168.1.5:8080", + owner: "foo", + workspace: "bar", + agent: undefined, + expected: "ssh-remote+coder-vscode.192.168.1.5--foo--bar", + }, + { + url: "http://localhost:3000", + owner: "dev", + workspace: "ws", + agent: "main", + expected: "ssh-remote+coder-vscode.localhost--dev--ws.main", + }, + { + url: "https://sub.DOMAIN.Example.COM", + owner: "foo", + workspace: "bar", + agent: undefined, + expected: "ssh-remote+coder-vscode.sub.domain.example.com--foo--bar", + }, + { + url: "https://ほげ:8080", + owner: "foo", + workspace: "bar", + agent: undefined, + expected: "ssh-remote+coder-vscode.xn--18j4d--foo--bar", + }, + { + 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); + }, + ); +}); diff --git a/test/unit/util/uri.test.ts b/test/unit/util/uri.test.ts index 140764ea9a..68a90161d6 100644 --- a/test/unit/util/uri.test.ts +++ b/test/unit/util/uri.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import * as vscode from "vscode"; import { + normalizeUrl, openInBrowser, removeTrailingSlashes, resolveUiUrl, @@ -14,6 +15,7 @@ describe("toSafeHost", () => { it.each([ ["https://foobar:8080", "foobar"], ["https://ほげ", "xn--18j4d"], + ["https://عربي", "xn--ngbrx4e"], ["https://test.😉.invalid", "test.xn--n28h.invalid"], ["https://dev.😉-coder.com", "dev.xn---coder-vx74e.com"], ["http://ignore-port.com:8080", "ignore-port.com"], @@ -37,6 +39,18 @@ describe("removeTrailingSlashes", () => { }); }); +describe("normalizeUrl", () => { + it.each([ + ["https://coder.example.com", "https://coder.example.com"], + [" https://coder.example.com ", "https://coder.example.com"], + ["https://coder.example.com///", "https://coder.example.com"], + [" https://coder.example.com/ ", "https://coder.example.com"], + ["", ""], + ])("returns %j for %j", (input, expected) => { + expect(normalizeUrl(input)).toBe(expected); + }); +}); + describe("resolveUiUrl", () => { let configurationProvider: MockConfigurationProvider; From 8b483bc274cb2b837b1b066cb31046e97d8aa0cf Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 23 Jun 2026 22:24:42 +0300 Subject: [PATCH 5/8] refactor: CLI-mediate credentials and move global-config to globalFlags Replace the dedicated `coder.globalConfig` setting with a `--global-config` passthrough in `coder.globalFlags`, honored on 2.31.0+. The extension no longer reads/writes the CLI credential files itself: it writes via `coder login` (0.25.0+, `--use-token-as-session`), reads via `coder login token` (2.31.0+), and deletes via `coder logout` (keyring or file with `--global-config`). Direct file writes remain only as a pre-0.25 fallback. The binary cache no longer follows the config dir. --- package.json | 8 +- src/core/cliCredentialManager.ts | 296 ++++++++++---------- src/core/pathResolver.ts | 11 +- src/featureSet.ts | 3 + src/remote/remote.ts | 6 - src/settings/cli.ts | 39 ++- src/supportBundle/settings.ts | 1 - test/unit/api/workspace.test.ts | 1 + test/unit/cliConfig.test.ts | 88 ++++-- test/unit/core/cliCredentialManager.test.ts | 205 ++++++++------ test/unit/core/cliExec.test.ts | 16 +- test/unit/core/pathResolver.test.ts | 62 +--- test/unit/featureSet.test.ts | 7 + test/unit/supportBundle/settings.test.ts | 8 - 14 files changed, 410 insertions(+), 341 deletions(-) diff --git a/package.json b/package.json index dee79a19cd..62775a3be5 100644 --- a/package.json +++ b/package.json @@ -181,14 +181,8 @@ ], "scope": "machine" }, - "coder.globalConfig": { - "markdownDescription": "Path to the global Coder CLI config directory passed with `--global-config`. Defaults to `CODER_CONFIG_DIR` if not set, otherwise the extension's per-deployment global storage directory. Set this to a shared CLI config directory such as `~/.config/coderv2` to share login/auth with the Coder CLI. Ignored when `#coder.useKeyring#` is active and supported.\n\nSupports `${env:VAR}`, `${userHome}`, and a leading `~`.", - "type": "string", - "default": "", - "scope": "machine" - }, "coder.globalFlags": { - "markdownDescription": "Global flags to pass to every Coder CLI invocation. Enter each flag as a separate array item, in order. Do **not** include the `coder` command itself. See the [CLI reference](https://coder.com/docs/reference/cli) for available global flags.\n\nSupports `${env:VAR}`, `${userHome}`, and a leading `~`. For `--flag=value` items the expansion applies to the value half, so `--cfg=~/coder` works.\n\nFor `--header-command`, precedence is: `#coder.headerCommand#` setting, then `CODER_HEADER_COMMAND` environment variable, then the value specified here. The `--global-config` and `--use-keyring` flags are silently ignored; use `#coder.globalConfig#` and `#coder.useKeyring#` instead.", + "markdownDescription": "Global flags to pass to every Coder CLI invocation. Enter each flag as a separate array item, in order. Do **not** include the `coder` command itself. See the [CLI reference](https://coder.com/docs/reference/cli) for available global flags.\n\nSupports `${env:VAR}`, `${userHome}`, and a leading `~`. For `--flag=value` items the expansion applies to the value half, so `--cfg=~/coder` works.\n\nSet `--global-config` here to point the CLI at a shared config directory (e.g. `--global-config=~/.config/coderv2` to share login/auth with the Coder CLI); requires a deployment on 2.31.0+ and is ignored when `#coder.useKeyring#` is active. The `--use-keyring` flag is ignored; use `#coder.useKeyring#` instead.\n\nFor `--header-command`, precedence is: `#coder.headerCommand#` setting, then `CODER_HEADER_COMMAND` environment variable, then the value specified here.", "type": "array", "items": { "type": "string" diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index f09422ec81..d516959892 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -12,11 +12,11 @@ import { CredentialFileError, CredentialTelemetry, } from "../instrumentation/credentials"; -import { isKeyringEnabled } from "../settings/cli"; +import { getGlobalFlags, isKeyringEnabled } from "../settings/cli"; import { getHeaderArgs } from "../settings/headers"; import { type TelemetryReporter } from "../telemetry/reporter"; import { writeAtomically } from "../util/fs"; -import { normalizeUrl, toSafeHost } from "../util/uri"; +import { toSafeHost } from "../util/uri"; import { version } from "./cliExec"; @@ -29,11 +29,13 @@ import type { PathResolver } from "./pathResolver"; const execFileAsync = promisify(execFile); -type KeyringFeature = "keyringAuth" | "keyringTokenRead"; -type TokenReadSource = - | { mode: "files" } - | { mode: "keyring"; binPath: string } - | { mode: "none" }; +// keyring/cli-file delegate to the CLI; legacy-file writes plaintext directly. +type CliTransport = + | { kind: "keyring"; binPath: string } + | { kind: "cli-file"; binPath: string; allowOverride: boolean }; + +type WriteTransport = CliTransport | { kind: "legacy-file" }; +type ReadTransport = CliTransport | { kind: "none" }; export interface CliCredential { token: string; @@ -74,11 +76,8 @@ export class CliCredentialManager { } /** - * Store credentials for a deployment URL. Uses the OS keyring when the - * setting is enabled and the CLI supports it; otherwise writes plaintext - * files under --global-config. - * - * Keyring and files are mutually exclusive, never both. + * Store credentials via `coder login` (keyring or file-backed); falls back + * to writing plaintext files directly on deployments older than 0.25.0. */ public storeToken( url: string, @@ -87,36 +86,35 @@ export class CliCredentialManager { options?: { signal?: AbortSignal }, ): Promise { return this.credentialTelemetry.traceStore(configs, async (span) => { - const binPath = await this.resolveKeyringBinary( - url, - configs, - "keyringAuth", - ); - if (!binPath) { + const transport = await this.resolveWriteTransport(url, configs); + if (transport.kind === "legacy-file") { span.setProperty("category", "file"); await this.writeCredentialFiles(url, token); return; } - span.setProperty("category", "keyring"); - await this.storeKeyringToken(binPath, url, token, configs, options); + span.setProperty( + "category", + transport.kind === "keyring" ? "keyring" : "file", + ); + await this.cliLogin(transport, url, token, configs, options); }); } - private async storeKeyringToken( - binPath: string, + private async cliLogin( + transport: CliTransport, url: string, token: string, configs: Pick, options?: { signal?: AbortSignal }, ): Promise { const args = [ - ...getHeaderArgs(configs), + ...this.credentialGlobalFlags(transport, url, configs), "login", "--use-token-as-session", url, ]; try { - await this.execWithTimeout(binPath, args, { + await this.execWithTimeout(transport.binPath, args, { env: { ...process.env, CODER_SESSION_TOKEN: token }, signal: options?.signal, }); @@ -131,41 +129,41 @@ export class CliCredentialManager { } /** - * Read a token from CLI-managed credentials. Uses `coder login token --url` - * when keyring auth is active, otherwise reads the file credentials under - * --global-config. Returns the token and the source it came from, or - * undefined on any failure (resolver, CLI, empty output). Throws AbortError - * when the signal is aborted. + * Read a token via `coder login token` (keyring or file-backed). Requires + * 2.31.0+; older deployments return undefined. Returns the token and its + * source, or undefined on any failure. Throws AbortError on abort. */ public async readToken( url: string, configs: Pick, options?: { signal?: AbortSignal }, ): Promise { - const source = await this.resolveTokenReadSource(url, configs); - if (source.mode === "files") { - const token = await this.readCredentialFiles(url); - return token ? { token, source: "files" } : undefined; - } - if (source.mode === "none") { + const transport = await this.resolveReadTransport(url, configs); + if (transport.kind === "none") { return undefined; } - const token = await this.readKeyringToken( - source.binPath, + const args = [ + ...this.credentialGlobalFlags(transport, url, configs), + "login", + "token", + "--url", url, - configs, - options, - ); - return token ? { token, source: "keyring" } : undefined; + ]; + const token = await this.runTokenRead(transport.binPath, args, options); + if (!token) { + return undefined; + } + return { + token, + source: transport.kind === "keyring" ? "keyring" : "files", + }; } - private async readKeyringToken( + private async runTokenRead( binPath: string, - url: string, - configs: Pick, + args: string[], options?: { signal?: AbortSignal }, ): Promise { - const args = [...getHeaderArgs(configs), "login", "token", "--url", url]; try { const { stdout } = await this.execWithTimeout(binPath, args, { signal: options?.signal, @@ -181,9 +179,9 @@ export class CliCredentialManager { } /** - * Delete credentials for a deployment. Runs file deletion and keyring - * deletion in parallel, both best-effort. Throws AbortError when the - * signal is aborted. + * Delete credentials for a deployment. Removes the default-dir files and + * logs out of the active store (keyring or file via --global-config), both + * best-effort. Throws AbortError when the signal is aborted. */ public deleteToken( url: string, @@ -193,56 +191,116 @@ export class CliCredentialManager { return this.credentialTelemetry.traceClear(configs, async (span) => { await Promise.all([ this.deleteCredentialFiles(url), - this.deleteKeyringToken(url, configs, { - signal: options?.signal, - span, - }), + this.cliLogout(url, configs, { signal: options?.signal, span }), ]); }); } /** - * Resolve a CLI binary for keyring operations. Returns the binary path - * when keyring is enabled in settings and the CLI version supports the - * requested feature, or undefined to fall back to file-based storage. - * - * Throws on binary resolution or version-check failure (caller decides - * whether to catch or propagate). + * Log out via `coder logout`, keyring or file (--global-config). Records + * failures on the span instead of throwing (except on abort). */ - private async resolveKeyringBinary( + private async cliLogout( url: string, configs: Pick, - feature: KeyringFeature, - ): Promise { - if (!isKeyringEnabled(configs)) { - return undefined; + { signal, span }: { signal?: AbortSignal; span: Span }, + ): Promise { + let transport: WriteTransport; + try { + transport = await this.resolveWriteTransport(url, configs); + } catch (error) { + this.logger.warn("Could not resolve CLI binary for logout:", error); + span.setProperty("error.type", "binary"); + span.markError(); + return; + } + if (transport.kind === "legacy-file") { + return; + } + const args = [ + ...this.credentialGlobalFlags(transport, url, configs), + "logout", + "--url", + url, + "--yes", + ]; + try { + await this.execWithTimeout(transport.binPath, args, { signal }); + this.logger.info("Deleted token via CLI for", url); + } catch (error) { + if (isAbortError(error)) { + throw error; + } + this.logger.warn("Failed to delete token via CLI:", error); + span.setProperty("error.type", "cli"); + span.markError(); } - const binPath = await this.resolveBinary(url); - return (await this.getFeatureSet(binPath))[feature] ? binPath : undefined; } - private async resolveTokenReadSource( + private async resolveCli( url: string, - configs: Pick, - ): Promise { - if (!isKeyringEnabled(configs)) { - return { mode: "files" }; - } + propagateError: boolean, + ): Promise<{ binPath: string; featureSet: FeatureSet } | undefined> { try { const binPath = await this.resolveBinary(url); - const featureSet = await this.getFeatureSet(binPath); - if (!featureSet.keyringAuth) { - return { mode: "files" }; - } - return featureSet.keyringTokenRead - ? { mode: "keyring", binPath } - : { mode: "none" }; + return { binPath, featureSet: await this.getFeatureSet(binPath) }; } catch (error) { - this.logger.warn("Could not resolve CLI binary for token read:", error); - return { mode: "none" }; + if (propagateError) { + throw error; + } + this.logger.warn("Could not resolve CLI binary:", error); + return undefined; } } + private async resolveWriteTransport( + url: string, + configs: Pick, + ): Promise { + const keyringEnabled = isKeyringEnabled(configs); + const cli = await this.resolveCli(url, keyringEnabled); + if (cli && keyringEnabled && cli.featureSet.keyringAuth) { + return { kind: "keyring", binPath: cli.binPath }; + } + if (cli?.featureSet.cliLogin) { + return cliFileTransport(cli); + } + return { kind: "legacy-file" }; + } + + private async resolveReadTransport( + url: string, + configs: Pick, + ): Promise { + const keyringEnabled = isKeyringEnabled(configs); + const cli = await this.resolveCli(url, false); + if (cli && keyringEnabled && cli.featureSet.keyringAuth) { + return cli.featureSet.keyringTokenRead + ? { kind: "keyring", binPath: cli.binPath } + : { kind: "none" }; + } + if (cli?.featureSet.keyringTokenRead) { + return cliFileTransport(cli); + } + return { kind: "none" }; + } + + /** Keyring uses the default store; file mode passes --global-config. */ + private credentialGlobalFlags( + transport: CliTransport, + url: string, + configs: Pick, + ): string[] { + if (transport.kind === "keyring") { + return getHeaderArgs(configs); + } + return getGlobalFlags(configs, { + mode: "global-config", + configDir: this.pathResolver.getGlobalConfigDir(toSafeHost(url)), + allowOverride: transport.allowOverride, + }); + } + private async getFeatureSet(binPath: string): Promise { return featureSetForVersion(semver.parse(await version(binPath))); } @@ -291,34 +349,6 @@ export class CliCredentialManager { } } - /** - * Read URL and token files under --global-config. - */ - private async readCredentialFiles(url: string): Promise { - try { - const files = await this.readCredentialFilePair(url); - return sameNormalizedUrl(files.url, url) - ? nonEmpty(files.token) - : undefined; - } catch (error) { - if ((error as NodeJS.ErrnoException)?.code !== "ENOENT") { - this.logger.warn("Failed to read credential files:", error); - } - return undefined; - } - } - - private async readCredentialFilePair( - url: string, - ): Promise<{ url: string; token: string }> { - const safeHostname = toSafeHost(url); - const [storedUrl, token] = await Promise.all([ - fs.readFile(this.pathResolver.getUrlPath(safeHostname), "utf8"), - fs.readFile(this.pathResolver.getSessionTokenPath(safeHostname), "utf8"), - ]); - return { url: storedUrl, token }; - } - /** * Delete URL and token files. Best-effort: never throws. */ @@ -337,43 +367,6 @@ export class CliCredentialManager { ); } - /** - * Delete keyring token via `coder logout`. Best-effort: records the failure - * on the span instead of throwing (except on abort), so it is tagged where - * it occurs. - */ - private async deleteKeyringToken( - url: string, - configs: Pick, - { signal, span }: { signal?: AbortSignal; span: Span }, - ): Promise { - let binPath: string | undefined; - try { - binPath = await this.resolveKeyringBinary(url, configs, "keyringAuth"); - } catch (error) { - this.logger.warn("Could not resolve keyring binary for delete:", error); - span.setProperty("error.type", "binary"); - span.markError(); - return; - } - if (!binPath) { - return; - } - - const args = [...getHeaderArgs(configs), "logout", "--url", url, "--yes"]; - try { - await this.execWithTimeout(binPath, args, { signal }); - this.logger.info("Deleted token via CLI for", url); - } catch (error) { - if (isAbortError(error)) { - throw error; - } - this.logger.warn("Failed to delete token via CLI:", error); - span.setProperty("error.type", "cli"); - span.markError(); - } - } - /** Atomically write content to a file. */ private async atomicWriteFile( filePath: string, @@ -389,8 +382,17 @@ export class CliCredentialManager { } } -function sameNormalizedUrl(storedUrl: string, expectedUrl: string): boolean { - return normalizeUrl(storedUrl) === normalizeUrl(expectedUrl); +function cliFileTransport(cli: { + binPath: string; + featureSet: FeatureSet; +}): CliTransport { + // Override applies only once read+write are CLI-mediated (2.31+), matching + // resolveCliAuth. + return { + kind: "cli-file", + binPath: cli.binPath, + allowOverride: cli.featureSet.keyringTokenRead, + }; } function nonEmpty(value: string): string | undefined { diff --git a/src/core/pathResolver.ts b/src/core/pathResolver.ts index 74ad6541a4..191534ba61 100644 --- a/src/core/pathResolver.ts +++ b/src/core/pathResolver.ts @@ -10,15 +10,12 @@ export class PathResolver { ) {} /** - * Return the directory where the global Coder configs are stored. - * - * The caller must ensure this directory exists before use. + * Per-deployment directory for the extension's Coder configs. A user + * `--global-config` in `coder.globalFlags` redirects the CLI; this stays the + * default. Caller must ensure it exists. */ public getGlobalConfigDir(safeHostname: string): string { - return ( - PathResolver.resolveOverride("coder.globalConfig", "CODER_CONFIG_DIR") || - path.join(this.basePath, safeHostname) - ); + return path.join(this.basePath, safeHostname); } /** diff --git a/src/featureSet.ts b/src/featureSet.ts index 8cc17b8cb5..3b7ed64022 100644 --- a/src/featureSet.ts +++ b/src/featureSet.ts @@ -6,6 +6,7 @@ export interface FeatureSet { wildcardSSH: boolean; buildReason: boolean; cliUpdate: boolean; + cliLogin: boolean; keyringAuth: boolean; keyringTokenRead: boolean; supportBundle: boolean; @@ -47,6 +48,8 @@ export function featureSetForVersion( buildReason: versionAtLeast(version, "2.25.0"), // `coder update` with stop transition (stops before updating) cliUpdate: versionAtLeast(version, "2.24.0"), + // `coder login --use-token-as-session` to write a token (file or keyring) + cliLogin: versionAtLeast(version, "0.25.0"), // Keyring-backed token storage via `coder login` keyringAuth: versionAtLeast(version, "2.29.0"), // `coder login token` for reading tokens from the keyring diff --git a/src/remote/remote.ts b/src/remote/remote.ts index dbfec53399..0e7975a0a4 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -454,12 +454,6 @@ export class Remote { title: string; getValue: () => unknown; }> = [ - { - setting: "coder.globalConfig", - title: "Global Config", - getValue: () => - this.pathResolver.getGlobalConfigDir(parts.safeHostname), - }, { setting: "coder.globalFlags", title: "Global Flags", diff --git a/src/settings/cli.ts b/src/settings/cli.ts index 5086e6d628..94d17150b5 100644 --- a/src/settings/cli.ts +++ b/src/settings/cli.ts @@ -8,7 +8,7 @@ import type { WorkspaceConfiguration } from "vscode"; import type { FeatureSet } from "../featureSet"; export type CliAuth = - | { mode: "global-config"; configDir: string } + | { mode: "global-config"; configDir: string; allowOverride: boolean } | { mode: "url"; url: string }; /** @@ -51,27 +51,39 @@ function buildGlobalFlags( escAuth: (s: string) => string, escHeader: (s: string) => string, ): string[] { - const authFlags = - auth.mode === "url" - ? ["--url", escAuth(auth.url)] - : ["--global-config", escAuth(auth.configDir)]; + const userFlags = getExpandedUserGlobalFlags(configs); + + // Honor a user `--global-config` only when allowOverride (file mode, 2.31+); + // otherwise strip it and emit our own so it matches where we read/write. + const honorOverride = + auth.mode === "global-config" && + auth.allowOverride && + userFlags.some((flag) => isFlag(flag, "--global-config")); // Escape each user flag so expansion-introduced whitespace stays inside // one shell token. `escAuth` is `identity` on the array path. - const filtered = stripManagedFlags(getExpandedUserGlobalFlags(configs)).map( - escAuth, - ); + const filtered = stripManagedFlags(userFlags, !honorOverride).map(escAuth); + + const authFlags = + auth.mode === "url" + ? ["--url", escAuth(auth.url)] + : honorOverride + ? [] + : ["--global-config", escAuth(auth.configDir)]; return [...filtered, ...authFlags, ...getHeaderArgs(configs, escHeader)]; } -function stripManagedFlags(flags: string[]): string[] { +function stripManagedFlags( + flags: string[], + stripGlobalConfig: boolean, +): string[] { const filtered: string[] = []; for (let i = 0; i < flags.length; i++) { if (isFlag(flags[i], "--use-keyring")) { continue; } - if (isFlag(flags[i], "--global-config")) { + if (stripGlobalConfig && isFlag(flags[i], "--global-config")) { // Skip the next item too when the value is a separate entry. if (flags[i] === "--global-config") { i++; @@ -113,7 +125,12 @@ export function resolveCliAuth( if (isKeyringEnabled(configs) && featureSet.keyringAuth) { return { mode: "url", url: deploymentUrl }; } - return { mode: "global-config", configDir }; + // Honored only on 2.31.0+, where CLI-mediated read/write share the directory. + return { + mode: "global-config", + configDir, + allowOverride: featureSet.keyringTokenRead, + }; } /** diff --git a/src/supportBundle/settings.ts b/src/supportBundle/settings.ts index 49b69dce7a..8517d4f1f4 100644 --- a/src/supportBundle/settings.ts +++ b/src/supportBundle/settings.ts @@ -23,7 +23,6 @@ const COLLECTED_SETTINGS: readonly string[] = [ "coder.disableUpdateNotifications", "coder.enableDownloads", "coder.experimental.oauth", - "coder.globalConfig", "coder.httpClientLogLevel", "coder.insecure", "coder.networkThreshold.latencyMs", diff --git a/test/unit/api/workspace.test.ts b/test/unit/api/workspace.test.ts index 3e2e38c482..605a378b69 100644 --- a/test/unit/api/workspace.test.ts +++ b/test/unit/api/workspace.test.ts @@ -27,6 +27,7 @@ const featureSet: FeatureSet = { wildcardSSH: true, buildReason: true, cliUpdate: true, + cliLogin: true, keyringAuth: true, keyringTokenRead: true, supportBundle: true, diff --git a/test/unit/cliConfig.test.ts b/test/unit/cliConfig.test.ts index f75675a936..48c897a2d8 100644 --- a/test/unit/cliConfig.test.ts +++ b/test/unit/cliConfig.test.ts @@ -21,6 +21,7 @@ vi.mock("node:os"); const globalConfigAuth: CliAuth = { mode: "global-config", configDir: "/config/dir", + allowOverride: true, }; describe("cliConfig", () => { @@ -91,35 +92,41 @@ describe("cliConfig", () => { interface GlobalConfigCase { scenario: string; flags: string[]; + expected: string[]; } it.each([ - { - scenario: "space-separated in one item", - flags: ["-v", "--global-config /path/to/ignored"], - }, { scenario: "equals form", - flags: ["-v", "--global-config=/path/to/ignored"], + flags: ["-v", "--global-config=/custom/coderv2"], + expected: ["-v", "--global-config=/custom/coderv2"], }, { scenario: "separate items", - flags: ["-v", "--global-config", "/path/to/ignored"], + flags: ["-v", "--global-config", "/custom/coderv2"], + expected: ["-v", "--global-config", "/custom/coderv2"], }, ])( - "should filter --global-config ($scenario) in both auth modes", - ({ flags }) => { - const urlAuth: CliAuth = { - mode: "url", - url: "https://dev.coder.com", - }; + "passes user --global-config through in file mode and drops our default ($scenario)", + ({ flags, expected }) => { const config = new MockConfigurationProvider(); config.set("coder.globalFlags", flags); - expect(getGlobalShellFlags(config, globalConfigAuth)).toStrictEqual([ - "-v", - "--global-config", - "/config/dir", - ]); + expect(getGlobalShellFlags(config, globalConfigAuth)).toStrictEqual( + expected, + ); + }, + ); + + it.each([ + { scenario: "space-separated in one item", flag: "--global-config /x" }, + { scenario: "equals form", flag: "--global-config=/x" }, + ])( + "strips user --global-config in keyring (url) mode ($scenario)", + ({ flag }) => { + const urlAuth: CliAuth = { mode: "url", url: "https://dev.coder.com" }; + const config = new MockConfigurationProvider(); + config.set("coder.globalFlags", ["-v", flag]); + expect(getGlobalShellFlags(config, urlAuth)).toStrictEqual([ "-v", "--url", @@ -128,6 +135,18 @@ describe("cliConfig", () => { }, ); + it("strips user --global-config (separate items) in keyring (url) mode", () => { + const urlAuth: CliAuth = { mode: "url", url: "https://dev.coder.com" }; + const config = new MockConfigurationProvider(); + config.set("coder.globalFlags", ["-v", "--global-config", "/x"]); + + expect(getGlobalShellFlags(config, urlAuth)).toStrictEqual([ + "-v", + "--url", + "https://dev.coder.com", + ]); + }); + it("should not filter flags with similar prefixes", () => { const config = new MockConfigurationProvider(); config.set("coder.globalFlags", ["--global-configs", "--use-keyrings"]); @@ -373,6 +392,8 @@ describe("cliConfig", () => { expect(auth).toEqual({ mode: "global-config", configDir: "/config/dir", + // 2.29 < 2.31, so a user --global-config is not honored. + allowOverride: false, }); }); @@ -411,25 +432,48 @@ describe("cliConfig", () => { ]); }); - it("does not let globalFlags override caller-provided config directory", () => { + it("lets globalFlags --global-config override the caller-provided directory on 2.31+", () => { vi.mocked(os.platform).mockReturnValue("linux"); const config = new MockConfigurationProvider(); config.set("coder.globalFlags", [ "--verbose", - "--global-config=/ignored/coderv2", + "--global-config=/custom/coderv2", ]); - const featureSet = featureSetForVersion(semver.parse("2.29.0")); + const featureSet = featureSetForVersion(semver.parse("2.31.0")); const auth = resolveCliAuth( config, featureSet, "https://dev.coder.com", - "/custom/coderv2", + "/default/coderv2", + ); + + // User's directory passes through; our default is dropped. + expect(getGlobalFlags(config, auth)).toStrictEqual([ + "--verbose", + "--global-config=/custom/coderv2", + ]); + }); + + it("ignores globalFlags --global-config on deployments older than 2.31", () => { + vi.mocked(os.platform).mockReturnValue("linux"); + const config = new MockConfigurationProvider(); + config.set("coder.globalFlags", [ + "--verbose", + "--global-config=/custom/coderv2", + ]); + const featureSet = featureSetForVersion(semver.parse("2.30.0")); + const auth = resolveCliAuth( + config, + featureSet, + "https://dev.coder.com", + "/default/coderv2", ); + // User override stripped; our default is used so it matches where we wrote. expect(getGlobalFlags(config, auth)).toStrictEqual([ "--verbose", "--global-config", - "/custom/coderv2", + "/default/coderv2", ]); }); }); diff --git a/test/unit/core/cliCredentialManager.test.ts b/test/unit/core/cliCredentialManager.test.ts index 295db14f6d..10a086f877 100644 --- a/test/unit/core/cliCredentialManager.test.ts +++ b/test/unit/core/cliCredentialManager.test.ts @@ -1,6 +1,7 @@ import { fs as memfs, vol } from "memfs"; import { execFile } from "node:child_process"; import * as os from "node:os"; +import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -26,9 +27,11 @@ vi.mock("node:child_process", () => ({ vi.mock("node:os"); -vi.mock("@/settings/cli", () => ({ - isKeyringEnabled: vi.fn().mockReturnValue(false), -})); +vi.mock("@/settings/cli", async () => { + const actual = + await vi.importActual("@/settings/cli"); + return { ...actual, isKeyringEnabled: vi.fn().mockReturnValue(false) }; +}); vi.mock("@/core/cliExec", async () => { const actual = @@ -119,16 +122,29 @@ function failingResolver(): BinaryResolver { return vi.fn().mockRejectedValue(new Error("no binary")); } -const configs = { get: vi.fn().mockReturnValue(undefined) }; +// Honor the defaultValue arg so getExpandedUserGlobalFlags sees [] when unset. +const configs = { + get: vi.fn((_key: string, defaultValue?: unknown) => defaultValue), +}; const configWithHeaders = { - get: vi.fn((key: string) => - key === "coder.headerCommand" ? "my-header-cmd" : undefined, + get: vi.fn((key: string, defaultValue?: unknown) => + key === "coder.headerCommand" ? "my-header-cmd" : defaultValue, ), }; +// A configs that sets a user --global-config override via coder.globalFlags. +function configWithGlobalConfig(dir: string) { + return { + get: vi.fn((key: string, defaultValue?: unknown) => + key === "coder.globalFlags" ? [`--global-config=${dir}`] : defaultValue, + ), + }; +} + const TEST_PATH_RESOLVER = new PathResolver("/mock/base", "/mock/log"); -const CRED_DIR = "/mock/base/dev.coder.com"; +// Built with path.join so it matches getGlobalConfigDir on Windows too. +const CRED_DIR = path.join("/mock/base", "dev.coder.com"); const CUSTOM_CRED_DIR = "/custom/coderv2"; function credentialPaths(dir = CRED_DIR) { @@ -162,10 +178,6 @@ function credentialFilesExist(dir = CRED_DIR): boolean { return memfs.existsSync(paths.url) || memfs.existsSync(paths.session); } -function useCustomGlobalConfig(): void { - new MockConfigurationProvider().set("coder.globalConfig", CUSTOM_CRED_DIR); -} - function setup(resolver?: BinaryResolver) { const r = resolver ?? successResolver(); const sink = new TestSink(); @@ -203,18 +215,25 @@ describe("CliCredentialManager", () => { }); describe("storeToken", () => { - it("writes files when keyring is disabled", async () => { - const { manager, sink } = setup(); + it("writes via coder login (file mode) when keyring is disabled", async () => { + stubExecFile({ stdout: "" }); + const { manager, resolver, sink } = setup(); await expect( manager.storeToken(TEST_URL, "my-token", configs), ).resolves.toBeUndefined(); - expect(execFile).not.toHaveBeenCalled(); - expect(readCredentialFiles()).toStrictEqual({ - url: TEST_URL, - session: "my-token", - }); + expect(resolver).toHaveBeenCalledWith(TEST_URL); + const exec = lastExecArgs(); + expect(exec.args).toEqual([ + "--global-config", + CRED_DIR, + "login", + "--use-token-as-session", + TEST_URL, + ]); + expect(exec.env.CODER_SESSION_TOKEN).toBe("my-token"); + expect(exec.args).not.toContain("my-token"); expect(sink.expectOne("auth.credential.store")).toMatchObject({ properties: { category: "file", @@ -249,34 +268,52 @@ describe("CliCredentialManager", () => { }); }); - it("writes files under configured global config when keyring is disabled", async () => { - useCustomGlobalConfig(); + it("writes via coder login under a user --global-config override", async () => { + stubExecFile({ stdout: "" }); const { manager } = setup(); - await expect( - manager.storeToken(TEST_URL, "my-token", configs), - ).resolves.toBeUndefined(); + await manager.storeToken( + TEST_URL, + "my-token", + configWithGlobalConfig(CUSTOM_CRED_DIR), + ); + + expect(lastExecArgs().args).toEqual([ + `--global-config=${CUSTOM_CRED_DIR}`, + "login", + "--use-token-as-session", + TEST_URL, + ]); + }); + + it("writes files directly for deployments older than 0.25", async () => { + vi.mocked(cliExec.version).mockResolvedValue("0.24.0"); + const { manager } = setup(); - expect(readCredentialFiles(CUSTOM_CRED_DIR)).toStrictEqual({ + await manager.storeToken(TEST_URL, "my-token", configs); + + expect(execFile).not.toHaveBeenCalled(); + expect(readCredentialFiles()).toStrictEqual({ url: TEST_URL, session: "my-token", }); }); - it("falls back to files when CLI version too old", async () => { + it("writes via coder login (file) when keyring is enabled but unsupported", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); vi.mocked(cliExec.version).mockResolvedValueOnce("2.28.0"); + stubExecFile({ stdout: "" }); const { manager } = setup(); - await expect( - manager.storeToken(TEST_URL, "token", configs), - ).resolves.toBeUndefined(); + await manager.storeToken(TEST_URL, "token", configs); - expect(execFile).not.toHaveBeenCalled(); - expect(readCredentialFiles()).toStrictEqual({ - url: TEST_URL, - session: "token", - }); + expect(lastExecArgs().args).toEqual([ + "--global-config", + CRED_DIR, + "login", + "--use-token-as-session", + TEST_URL, + ]); }); it("throws when CLI exec fails", async () => { @@ -396,38 +433,46 @@ describe("CliCredentialManager", () => { expect(execFile).not.toHaveBeenCalled(); }); - it("reads files when keyring is disabled", async () => { - writeCredentialFiles(TEST_URL, "file-token"); - stubExecFile({ stdout: "my-token" }); - const { manager } = setup(); + it("reads via coder login token (file mode) when keyring is disabled", async () => { + stubExecFile({ stdout: "file-token\n" }); + const { manager, resolver } = setup(); expect(await manager.readToken(TEST_URL, configs)).toEqual({ token: "file-token", source: "files", }); - expect(execFile).not.toHaveBeenCalled(); + expect(resolver).toHaveBeenCalledWith(TEST_URL); + expect(lastExecArgs().args).toEqual([ + "--global-config", + CRED_DIR, + "login", + "token", + "--url", + TEST_URL, + ]); }); - it("reads files under configured global config when keyring is disabled", async () => { - useCustomGlobalConfig(); - writeCredentialFiles( - `${TEST_URL}\n`, - "custom-file-token\n", - CUSTOM_CRED_DIR, - ); + it("reads via coder login token under a user --global-config override", async () => { + stubExecFile({ stdout: "custom-file-token" }); const { manager } = setup(); - expect(await manager.readToken(TEST_URL, configs)).toEqual({ - token: "custom-file-token", - source: "files", - }); - expect(execFile).not.toHaveBeenCalled(); + expect( + await manager.readToken( + TEST_URL, + configWithGlobalConfig(CUSTOM_CRED_DIR), + ), + ).toEqual({ token: "custom-file-token", source: "files" }); + expect(lastExecArgs().args).toEqual([ + `--global-config=${CUSTOM_CRED_DIR}`, + "login", + "token", + "--url", + TEST_URL, + ]); }); - it("does not read files when keyring token read is unsupported", async () => { - vi.mocked(isKeyringEnabled).mockReturnValue(true); - vi.mocked(cliExec.version).mockResolvedValueOnce("2.30.0"); - writeCredentialFiles(TEST_URL, "file-token"); + it("returns undefined for file mode on deployments older than 2.31", async () => { + vi.mocked(cliExec.version).mockResolvedValue("2.30.0"); const { manager, resolver } = setup(); expect(await manager.readToken(TEST_URL, configs)).toBeUndefined(); @@ -435,25 +480,23 @@ describe("CliCredentialManager", () => { expect(execFile).not.toHaveBeenCalled(); }); - it("reads files when keyring is enabled but unsupported by the CLI", async () => { + it("does not read when keyring token read is unsupported", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); - vi.mocked(cliExec.version).mockResolvedValueOnce("2.28.0"); - writeCredentialFiles(TEST_URL, "file-token"); + vi.mocked(cliExec.version).mockResolvedValueOnce("2.30.0"); const { manager, resolver } = setup(); - expect(await manager.readToken(TEST_URL, configs)).toEqual({ - token: "file-token", - source: "files", - }); + expect(await manager.readToken(TEST_URL, configs)).toBeUndefined(); expect(resolver).toHaveBeenCalledWith(TEST_URL); expect(execFile).not.toHaveBeenCalled(); }); - it("does not read files for a different URL", async () => { - writeCredentialFiles("https://other.coder.com", "file-token"); - const { manager } = setup(); + it("returns undefined when keyring is enabled but unsupported by the CLI", async () => { + vi.mocked(isKeyringEnabled).mockReturnValue(true); + vi.mocked(cliExec.version).mockResolvedValueOnce("2.28.0"); + const { manager, resolver } = setup(); expect(await manager.readToken(TEST_URL, configs)).toBeUndefined(); + expect(resolver).toHaveBeenCalledWith(TEST_URL); expect(execFile).not.toHaveBeenCalled(); }); @@ -525,27 +568,24 @@ describe("CliCredentialManager", () => { }); }); - it("deletes files even when keyring is disabled", async () => { + it("deletes files and invokes coder logout (file) when keyring is disabled", async () => { + stubExecFile({ stdout: "" }); writeCredentialFiles(TEST_URL, "old-token"); const { manager } = setup(); await manager.deleteToken(TEST_URL, configs); - expect(execFile).not.toHaveBeenCalled(); + expect(lastExecArgs().args).toEqual([ + "--global-config", + CRED_DIR, + "logout", + "--url", + TEST_URL, + "--yes", + ]); expect(credentialFilesExist()).toBe(false); }); - it("deletes files under configured global config", async () => { - useCustomGlobalConfig(); - writeCredentialFiles(TEST_URL, "old-token", CUSTOM_CRED_DIR); - const { manager } = setup(); - - await manager.deleteToken(TEST_URL, configs); - - expect(execFile).not.toHaveBeenCalled(); - expect(credentialFilesExist(CUSTOM_CRED_DIR)).toBe(false); - }); - it("never throws on CLI error", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); stubExecFile({ error: "logout failed" }); @@ -589,7 +629,7 @@ describe("CliCredentialManager", () => { expect(lastExecArgs().args).toContain("--header-command"); }); - it("skips keyring when CLI version too old", async () => { + it("logs out via coder logout (file) when keyring is enabled but unsupported", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); vi.mocked(cliExec.version).mockResolvedValueOnce("2.28.0"); stubExecFile({ stdout: "" }); @@ -598,7 +638,14 @@ describe("CliCredentialManager", () => { await manager.deleteToken(TEST_URL, configs); - expect(execFile).not.toHaveBeenCalled(); + expect(lastExecArgs().args).toEqual([ + "--global-config", + CRED_DIR, + "logout", + "--url", + TEST_URL, + "--yes", + ]); expect(credentialFilesExist()).toBe(false); }); diff --git a/test/unit/core/cliExec.test.ts b/test/unit/core/cliExec.test.ts index 774f6e9de3..3c578fbbe1 100644 --- a/test/unit/core/cliExec.test.ts +++ b/test/unit/core/cliExec.test.ts @@ -182,7 +182,10 @@ describe("cliExec", () => { `process.exit(1);`, ].join("\n"); const bin = await writeExecutable(tmp, "speedtest-err", code); - const { env } = setup({ mode: "global-config", configDir: "/tmp" }, bin); + const { env } = setup( + { mode: "global-config", configDir: "/tmp", allowOverride: true }, + bin, + ); await expect( cliExec.speedtest(env, "owner/workspace", "bad"), ).rejects.toThrow("invalid argument for -t flag"); @@ -192,7 +195,10 @@ describe("cliExec", () => { // Hangs forever so the only way out is the abort signal. const code = `setInterval(() => {}, 1000);`; const bin = await writeExecutable(tmp, "speedtest-hang", code); - const { env } = setup({ mode: "global-config", configDir: "/tmp" }, bin); + const { env } = setup( + { mode: "global-config", configDir: "/tmp", allowOverride: true }, + bin, + ); const ac = new AbortController(); ac.abort(); await expect( @@ -281,7 +287,10 @@ describe("cliExec", () => { `process.exit(1);`, ].join("\n"); const bin = await writeExecutable(tmp, "sb-err", code); - const { env } = setup({ mode: "global-config", configDir: "/tmp" }, bin); + const { env } = setup( + { mode: "global-config", configDir: "/tmp", allowOverride: true }, + bin, + ); await expect( cliExec.supportBundle(env, "owner/workspace", "/tmp/bundle.zip"), ).rejects.toThrow("workspace not found"); @@ -338,6 +347,7 @@ describe("cliExec", () => { const { configs, env } = setup({ mode: "global-config", configDir: "/cfg", + allowOverride: true, }); configs.set("coder.globalFlags", ["--verbose"]); diff --git a/test/unit/core/pathResolver.test.ts b/test/unit/core/pathResolver.test.ts index 47e6402512..b3191e6d30 100644 --- a/test/unit/core/pathResolver.test.ts +++ b/test/unit/core/pathResolver.test.ts @@ -20,59 +20,21 @@ describe("PathResolver", () => { }); describe("getGlobalConfigDir", () => { - it.each([ - { - name: "uses per-deployment global storage when no override is configured", - setting: "", - env: "", - expected: path.join(basePath, "deployment"), - }, - { - name: "uses configured global config directory directly", - setting: "/custom/coderv2", - expected: "/custom/coderv2", - }, - { - name: "uses CODER_CONFIG_DIR when setting is empty", - setting: "", - env: " /env/coderv2 ", - expected: "/env/coderv2", - }, - { - name: "uses setting before CODER_CONFIG_DIR", - setting: " /setting/coderv2 ", - env: "/env/coderv2", - expected: "/setting/coderv2", - }, - { - name: "normalizes configured global config directory", - setting: "/custom/../coderv2/./dir", - expected: "/coderv2/dir", - }, - ])("$name", ({ setting, env, expected }) => { - vi.stubEnv("CODER_CONFIG_DIR", env); - mockConfig.set("coder.globalConfig", setting); - - expectPathsEqual(pathResolver.getGlobalConfigDir("deployment"), expected); + it("uses the per-deployment global storage directory", () => { + expectPathsEqual( + pathResolver.getGlobalConfigDir("deployment"), + path.join(basePath, "deployment"), + ); }); - it.each([ - { - name: "configured global config directory", - setting: "~/coderv2", - }, - { - name: "CODER_CONFIG_DIR", - env: "~/coderv2", - }, - ])("expands paths in $name", ({ setting, env }) => { - vi.stubEnv("CODER_CONFIG_DIR", env); - mockConfig.set("coder.globalConfig", setting ?? ""); - - const result = pathResolver.getGlobalConfigDir("deployment"); + it("ignores coder.globalConfig and CODER_CONFIG_DIR (override lives in globalFlags)", () => { + vi.stubEnv("CODER_CONFIG_DIR", "/env/coderv2"); + mockConfig.set("coder.globalConfig", "/custom/coderv2"); - expect(result).not.toContain("~"); - expect(result).toContain("coderv2"); + expectPathsEqual( + pathResolver.getGlobalConfigDir("deployment"), + path.join(basePath, "deployment"), + ); }); }); diff --git a/test/unit/featureSet.test.ts b/test/unit/featureSet.test.ts index c7bee26cbf..5be56d255a 100644 --- a/test/unit/featureSet.test.ts +++ b/test/unit/featureSet.test.ts @@ -34,6 +34,13 @@ describe("check version support", () => { ["v2.19.0", "v2.19.1", "v2.20.0+e491217", "v5.0.4+e491217"], ); }); + it("cli login", () => { + expectFlag( + "cliLogin", + ["v0.24.0", "v0.14.0", "v0.0.1"], + ["v0.25.0", "v0.25.1", "v2.31.0", "v3.0.0"], + ); + }); it("keyring auth", () => { expectFlag( "keyringAuth", diff --git a/test/unit/supportBundle/settings.test.ts b/test/unit/supportBundle/settings.test.ts index 32e0180f48..43f4c9abe0 100644 --- a/test/unit/supportBundle/settings.test.ts +++ b/test/unit/supportBundle/settings.test.ts @@ -46,7 +46,6 @@ describe("collectSettingsFile", () => { "coder.tlsCertFile": "/etc/ssl/cert.pem", "coder.sshFlags": ["--disable-autostart"], "coder.defaultUrl": "https://coder.example.com", - "coder.globalConfig": "/home/user/.config/coderv2", "coder.proxyLogDirectory": "/home/user/.coder/logs", "coder.insecure": true, "coder.httpClientLogLevel": "debug", @@ -73,10 +72,6 @@ describe("collectSettingsFile", () => { defaultValue: "", globalValue: "https://coder.example.com", }, - "coder.globalConfig": { - defaultValue: "", - globalValue: "/home/user/.config/coderv2", - }, "coder.proxyLogDirectory": { defaultValue: "", globalValue: "/home/user/.coder/logs", @@ -111,9 +106,6 @@ describe("collectSettingsFile", () => { expect(settings["coder.defaultUrl"]?.effective).toBe( "https://coder.example.com", ); - expect(settings["coder.globalConfig"]?.effective).toBe( - "/home/user/.config/coderv2", - ); expect(settings["coder.proxyLogDirectory"]?.effective).toBe( "/home/user/.coder/logs", ); From a04db49ee2237221676a540c8eb19e0d570a7932 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 25 Jun 2026 13:43:34 +0300 Subject: [PATCH 6/8] refactor: drop pre-0.25 support and simplify credential resolution - Bump the minimum supported deployment version to 0.25.0 (where `coder login` lives), gating on `featureSet.cliLogin`, and move the compatibility check before credential configuration so older servers get a clear message. - Remove the pre-0.25 direct-file-write fallback; credential writes now always go through `coder login`. Drop the now-unused `vscodessh` feature flag and the dead `CredentialFileError` class plus its `filesystem`/`file` telemetry categories. - Collapse the `resolveCli` overloads into one throwing resolver; the best-effort read path catches instead. - Fix the stale `coder.globalConfig` CHANGELOG entry (the setting was dropped) and two pre-existing test typecheck breaks (`toSafeHost` import path and a `CliAuth.allowOverride` literal). --- CHANGELOG.md | 12 ++- src/core/cliCredentialManager.ts | 101 +++++--------------- src/core/cliManager.ts | 4 +- src/featureSet.ts | 15 +-- src/instrumentation/EVENTS.md | 18 ++-- src/instrumentation/cli.ts | 10 +- src/instrumentation/credentials.ts | 12 +-- src/remote/remote.ts | 27 +++--- test/unit/api/workspace.test.ts | 3 +- test/unit/commands.netcheck.test.ts | 2 +- test/unit/core/cliCredentialManager.test.ts | 23 ++--- test/unit/core/cliExec.test.ts | 10 +- 12 files changed, 80 insertions(+), 157 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32d9989808..c65870ed4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,15 @@ raw output. When a slow connection is detected, the network status bar tooltip also links to the network check alongside the latency test, for a deployment-wide view of the network path. +- Share login with the terminal `coder` CLI by setting `--global-config=` + in `coder.globalFlags`; the extension reads file-based CLI credentials from + that directory (requires a deployment on 2.31.0+). + +### Changed + +- File-based credentials are now written and cleared through `coder login` and + `coder logout` rather than by the extension directly. The minimum supported + Coder version is now v0.25.0. ### Fixed @@ -30,9 +39,6 @@ ### Added -- New `coder.globalConfig` setting to override the Coder CLI `--global-config` - directory, with `CODER_CONFIG_DIR` fallback, so file-backed CLI login/auth can - be shared with the VS Code extension when keyring auth is not active. - New **Shared Workspaces** view in the Coder sidebar that lists workspaces other users have shared with you, with search and refresh actions, so you can find and open them just like your own. diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index d516959892..4002036313 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -1,7 +1,6 @@ import { execFile } from "node:child_process"; import fs from "node:fs/promises"; import os from "node:os"; -import path from "node:path"; import { promisify } from "node:util"; import * as semver from "semver"; @@ -9,13 +8,11 @@ import { isAbortError } from "../error/errorUtils"; import { featureSetForVersion, type FeatureSet } from "../featureSet"; import { CredentialCliError, - CredentialFileError, CredentialTelemetry, } from "../instrumentation/credentials"; import { getGlobalFlags, isKeyringEnabled } from "../settings/cli"; import { getHeaderArgs } from "../settings/headers"; import { type TelemetryReporter } from "../telemetry/reporter"; -import { writeAtomically } from "../util/fs"; import { toSafeHost } from "../util/uri"; import { version } from "./cliExec"; @@ -29,12 +26,11 @@ import type { PathResolver } from "./pathResolver"; const execFileAsync = promisify(execFile); -// keyring/cli-file delegate to the CLI; legacy-file writes plaintext directly. +// keyring uses the CLI's default store; cli-file passes --global-config. type CliTransport = | { kind: "keyring"; binPath: string } | { kind: "cli-file"; binPath: string; allowOverride: boolean }; -type WriteTransport = CliTransport | { kind: "legacy-file" }; type ReadTransport = CliTransport | { kind: "none" }; export interface CliCredential { @@ -60,8 +56,8 @@ export function isKeyringSupported(): boolean { } /** - * Delegates credential storage to the Coder CLI. Owns all credential - * persistence: keyring-backed (via CLI) and file-based (plaintext). + * Delegates credential storage to the Coder CLI, both keyring-backed and + * file-based, via `coder login`/`coder logout`. */ export class CliCredentialManager { private readonly credentialTelemetry: CredentialTelemetry; @@ -76,8 +72,8 @@ export class CliCredentialManager { } /** - * Store credentials via `coder login` (keyring or file-backed); falls back - * to writing plaintext files directly on deployments older than 0.25.0. + * Store credentials via `coder login` (keyring or file-backed). Throws if the + * CLI binary cannot be resolved. */ public storeToken( url: string, @@ -87,11 +83,6 @@ export class CliCredentialManager { ): Promise { return this.credentialTelemetry.traceStore(configs, async (span) => { const transport = await this.resolveWriteTransport(url, configs); - if (transport.kind === "legacy-file") { - span.setProperty("category", "file"); - await this.writeCredentialFiles(url, token); - return; - } span.setProperty( "category", transport.kind === "keyring" ? "keyring" : "file", @@ -205,7 +196,7 @@ export class CliCredentialManager { configs: Pick, { signal, span }: { signal?: AbortSignal; span: Span }, ): Promise { - let transport: WriteTransport; + let transport: CliTransport; try { transport = await this.resolveWriteTransport(url, configs); } catch (error) { @@ -214,9 +205,6 @@ export class CliCredentialManager { span.markError(); return; } - if (transport.kind === "legacy-file") { - return; - } const args = [ ...this.credentialGlobalFlags(transport, url, configs), "logout", @@ -237,49 +225,43 @@ export class CliCredentialManager { } } + /** Resolve the CLI binary and its feature set, or throw if unavailable. */ private async resolveCli( url: string, - propagateError: boolean, - ): Promise<{ binPath: string; featureSet: FeatureSet } | undefined> { - try { - const binPath = await this.resolveBinary(url); - return { binPath, featureSet: await this.getFeatureSet(binPath) }; - } catch (error) { - if (propagateError) { - throw error; - } - this.logger.warn("Could not resolve CLI binary:", error); - return undefined; - } + ): Promise<{ binPath: string; featureSet: FeatureSet }> { + const binPath = await this.resolveBinary(url); + return { binPath, featureSet: await this.getFeatureSet(binPath) }; } private async resolveWriteTransport( url: string, configs: Pick, - ): Promise { - const keyringEnabled = isKeyringEnabled(configs); - const cli = await this.resolveCli(url, keyringEnabled); - if (cli && keyringEnabled && cli.featureSet.keyringAuth) { + ): Promise { + const cli = await this.resolveCli(url); + if (isKeyringEnabled(configs) && cli.featureSet.keyringAuth) { return { kind: "keyring", binPath: cli.binPath }; } - if (cli?.featureSet.cliLogin) { - return cliFileTransport(cli); - } - return { kind: "legacy-file" }; + return cliFileTransport(cli); } private async resolveReadTransport( url: string, configs: Pick, ): Promise { - const keyringEnabled = isKeyringEnabled(configs); - const cli = await this.resolveCli(url, false); - if (cli && keyringEnabled && cli.featureSet.keyringAuth) { + // Reading is best-effort: a missing binary means no CLI credentials. + const cli = await this.resolveCli(url).catch((error) => { + this.logger.warn("Could not resolve CLI binary:", error); + return undefined; + }); + if (!cli) { + return { kind: "none" }; + } + if (isKeyringEnabled(configs) && cli.featureSet.keyringAuth) { return cli.featureSet.keyringTokenRead ? { kind: "keyring", binPath: cli.binPath } : { kind: "none" }; } - if (cli?.featureSet.keyringTokenRead) { + if (cli.featureSet.keyringTokenRead) { return cliFileTransport(cli); } return { kind: "none" }; @@ -328,27 +310,6 @@ export class CliCredentialManager { } } - /** - * Write URL and token files under --global-config. - */ - private async writeCredentialFiles( - url: string, - token: string, - ): Promise { - try { - const safeHostname = toSafeHost(url); - await Promise.all([ - this.atomicWriteFile(this.pathResolver.getUrlPath(safeHostname), url), - this.atomicWriteFile( - this.pathResolver.getSessionTokenPath(safeHostname), - token, - ), - ]); - } catch (error) { - throw new CredentialFileError(error); - } - } - /** * Delete URL and token files. Best-effort: never throws. */ @@ -366,20 +327,6 @@ export class CliCredentialManager { ), ); } - - /** Atomically write content to a file. */ - private async atomicWriteFile( - filePath: string, - content: string, - ): Promise { - await fs.mkdir(path.dirname(filePath), { recursive: true }); - await writeAtomically( - filePath, - (tempPath) => fs.writeFile(tempPath, content, { mode: 0o600 }), - (rmErr, tempPath) => - this.logger.warn("Failed to delete temp file", tempPath, rmErr), - ); - } } function cliFileTransport(cli: { diff --git a/src/core/cliManager.ts b/src/core/cliManager.ts index 87de6b3acd..465e1a9d0f 100644 --- a/src/core/cliManager.ts +++ b/src/core/cliManager.ts @@ -994,8 +994,8 @@ export class CliManager { /** * Configure the CLI for the deployment with the provided hostname. * - * Stores credentials in the OS keyring when the setting is enabled and the - * CLI supports it, otherwise writes plaintext files under --global-config. + * Stores credentials via `coder login`: in the OS keyring when the setting is + * enabled and the CLI supports it, otherwise file-based under --global-config. * * Both URL and token are required. Empty tokens are allowed (e.g. mTLS * authentication) but the URL must be a non-empty string. diff --git a/src/featureSet.ts b/src/featureSet.ts index 3b7ed64022..7c7ac41e6a 100644 --- a/src/featureSet.ts +++ b/src/featureSet.ts @@ -1,12 +1,11 @@ import type * as semver from "semver"; export interface FeatureSet { - vscodessh: boolean; + cliLogin: boolean; proxyLogDirectory: boolean; wildcardSSH: boolean; buildReason: boolean; cliUpdate: boolean; - cliLogin: boolean; keyringAuth: boolean; keyringTokenRead: boolean; supportBundle: boolean; @@ -33,13 +32,9 @@ export function featureSetForVersion( version: semver.SemVer | null, ): FeatureSet { return { - vscodessh: !( - version?.major === 0 && - version?.minor <= 14 && - version?.patch < 1 && - version?.prerelease.length === 0 - ), - + // `coder login --use-token-as-session` to write a token (file or keyring). + // The extension relies on this, so 0.25.0 is the minimum supported version. + cliLogin: versionAtLeast(version, "0.25.0"), // --log-dir flag for proxy logs; vscodessh fails if unsupported proxyLogDirectory: versionAtLeast(version, "2.4.0"), // Wildcard SSH host matching @@ -48,8 +43,6 @@ export function featureSetForVersion( buildReason: versionAtLeast(version, "2.25.0"), // `coder update` with stop transition (stops before updating) cliUpdate: versionAtLeast(version, "2.24.0"), - // `coder login --use-token-as-session` to write a token (file or keyring) - cliLogin: versionAtLeast(version, "0.25.0"), // Keyring-backed token storage via `coder login` keyringAuth: versionAtLeast(version, "2.29.0"), // `coder login token` for reading tokens from the keyring diff --git a/src/instrumentation/EVENTS.md b/src/instrumentation/EVENTS.md index b4171cf69b..a3186665da 100644 --- a/src/instrumentation/EVENTS.md +++ b/src/instrumentation/EVENTS.md @@ -210,7 +210,7 @@ Secret-storage session read during remote setup. No custom attributes. | ----------------- | ------------------------------------------------- | | `keyring_enabled` | `true`, `false` (from settings) | | `category` | `keyring`, `file` (the storage actually involved) | -| `error.type` | `binary`, `cli`, `file` | +| `error.type` | `binary`, `cli` | ### Logs @@ -265,12 +265,12 @@ Phase `cli.download.verify` covers binary signature verification: #### `cli.configure` -| Attribute | Values | -| ------------------- | ------------------------------------------- | -| `silent` | `true`, `false` | -| `credential_source` | `session_token`, `empty_token` | -| `abort_stage` | `credential_store` | -| `error.type` | `filesystem`, `credential_store`, `unknown` | +| Attribute | Values | +| ------------------- | ------------------------------ | +| `silent` | `true`, `false` | +| `credential_source` | `session_token`, `empty_token` | +| `abort_stage` | `credential_store` | +| `error.type` | `credential_store`, `unknown` | ## Commands @@ -337,8 +337,8 @@ Emitted by `RemoteSetupTelemetry` around connecting to a remote workspace. | --------- | -------------------------------------------------------------------------------------------- | | `outcome` | `workspace_not_found`, `incompatible_server` (non-throwing early exits; the span is aborted) | -Phases (`remote.setup.`): `cli_resolve`, `cli_configure`, -`compatibility_check`, `workspace_lookup`, `workspace_monitor_setup`, +Phases (`remote.setup.`): `cli_resolve`, `compatibility_check`, +`cli_configure`, `workspace_lookup`, `workspace_monitor_setup`, `workspace_ready`, `agent_resolve`, `ssh_config_write`, `ssh_monitor_setup`, `connection_handoff`. diff --git a/src/instrumentation/cli.ts b/src/instrumentation/cli.ts index d819c7e6db..2057e2a65c 100644 --- a/src/instrumentation/cli.ts +++ b/src/instrumentation/cli.ts @@ -1,4 +1,3 @@ -import { CredentialFileError } from "./credentials"; import { recordAborted, recordError } from "./outcomes"; import type { CallerPropertyValue } from "../telemetry/event"; @@ -20,10 +19,7 @@ export type CliVersionCheckOutcome = | "match" | "mismatch" | "unreadable"; -export type CliConfigureErrorType = - | "filesystem" - | "credential_store" - | "unknown"; +export type CliConfigureErrorType = "credential_store" | "unknown"; export type CliResolveErrorType = | "downloads_disabled" | "download" @@ -192,10 +188,6 @@ export class CliConfigureTrace { } function categorizeConfigureError(error: unknown): CliConfigureErrorType { - // A CredentialFileError is a file-write failure; anything else is keyring/CLI. - if (error instanceof CredentialFileError) { - return "filesystem"; - } return error instanceof Error ? "credential_store" : "unknown"; } diff --git a/src/instrumentation/credentials.ts b/src/instrumentation/credentials.ts index 5d6096d415..c921e3837a 100644 --- a/src/instrumentation/credentials.ts +++ b/src/instrumentation/credentials.ts @@ -6,7 +6,7 @@ import type { WorkspaceConfiguration } from "vscode"; import type { TelemetryReporter } from "../telemetry/reporter"; import type { Span } from "../telemetry/span"; -export type CredentialErrorCategory = "binary" | "cli" | "file"; +export type CredentialErrorCategory = "binary" | "cli"; type CredentialEvent = "auth.credential.store" | "auth.credential.clear"; @@ -68,9 +68,6 @@ export class CredentialTelemetry { } function categorizeCredentialError(error: unknown): CredentialErrorCategory { - if (error instanceof CredentialFileError) { - return "file"; - } if (error instanceof CredentialCliError) { return "cli"; } @@ -83,10 +80,3 @@ export class CredentialCliError extends Error { this.name = "CredentialCliError"; } } - -export class CredentialFileError extends Error { - public constructor(cause: unknown) { - super("Credential file operation failed", { cause }); - this.name = "CredentialFileError"; - } -} diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 0e7975a0a4..14f7b46097 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -264,16 +264,6 @@ export class Remote { this.resolveRemoteBinary(workspaceClient), ); - // Write token to keyring or file - if (baseUrl && token !== undefined) { - await tracer.phase("cli_configure", () => - this.cliManager.configure(baseUrl, token), - ); - } - - // Listen for token changes for this deployment - disposables.push(this.watchRemoteSessionAuth(context, workspaceClient)); - const { featureSet, cliAuth } = await tracer.phase( "compatibility_check", () => @@ -285,14 +275,15 @@ export class Remote { }), ); - // Server versions before v0.14.1 don't support the vscodessh command! - if (!featureSet.vscodessh) { + // Reject deployments below our minimum supported version (v0.25.0) + // before configuring credentials, so they get a clear message. + if (!featureSet.cliLogin) { tracer.markAborted("incompatible_server"); await vscodeProposed.window.showErrorMessage( "Incompatible Server", { detail: - "Your Coder server is too old to support the Coder extension! Please upgrade to v0.14.1 or newer.", + "Your Coder server is too old to support the Coder extension! Please upgrade to v0.25.0 or newer.", modal: true, useCustom: true, }, @@ -305,6 +296,16 @@ export class Remote { return; } + // Write token to keyring or file + if (baseUrl && token !== undefined) { + await tracer.phase("cli_configure", () => + this.cliManager.configure(baseUrl, token), + ); + } + + // Listen for token changes for this deployment + disposables.push(this.watchRemoteSessionAuth(context, workspaceClient)); + // Next is to find the workspace from the URI scheme provided. const foundWorkspace = await tracer.phase("workspace_lookup", () => this.lookupWorkspace(context, workspaceClient), diff --git a/test/unit/api/workspace.test.ts b/test/unit/api/workspace.test.ts index 605a378b69..4ed140413a 100644 --- a/test/unit/api/workspace.test.ts +++ b/test/unit/api/workspace.test.ts @@ -22,12 +22,11 @@ vi.mock(import("node:child_process"), async (importOriginal) => ({ const { spawn } = await import("node:child_process"); const featureSet: FeatureSet = { - vscodessh: true, + cliLogin: true, proxyLogDirectory: true, wildcardSSH: true, buildReason: true, cliUpdate: true, - cliLogin: true, keyringAuth: true, keyringTokenRead: true, supportBundle: true, diff --git a/test/unit/commands.netcheck.test.ts b/test/unit/commands.netcheck.test.ts index 901b2091f5..9704f5da00 100644 --- a/test/unit/commands.netcheck.test.ts +++ b/test/unit/commands.netcheck.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import * as vscode from "vscode"; import { Commands } from "@/commands"; -import { toSafeHost } from "@/util"; +import { toSafeHost } from "@/util/uri"; import { createTelemetryHarness } from "../mocks/telemetry"; import { createMockLogger, MockUserInteraction } from "../mocks/testHelpers"; diff --git a/test/unit/core/cliCredentialManager.test.ts b/test/unit/core/cliCredentialManager.test.ts index 10a086f877..a3fea9ed83 100644 --- a/test/unit/core/cliCredentialManager.test.ts +++ b/test/unit/core/cliCredentialManager.test.ts @@ -165,14 +165,6 @@ function writeCredentialFiles( memfs.writeFileSync(paths.session, token); } -function readCredentialFiles(dir = CRED_DIR) { - const paths = credentialPaths(dir); - return { - url: memfs.readFileSync(paths.url, "utf8"), - session: memfs.readFileSync(paths.session, "utf8"), - }; -} - function credentialFilesExist(dir = CRED_DIR): boolean { const paths = credentialPaths(dir); return memfs.existsSync(paths.url) || memfs.existsSync(paths.session); @@ -286,17 +278,14 @@ describe("CliCredentialManager", () => { ]); }); - it("writes files directly for deployments older than 0.25", async () => { - vi.mocked(cliExec.version).mockResolvedValue("0.24.0"); - const { manager } = setup(); - - await manager.storeToken(TEST_URL, "my-token", configs); + it("throws and writes no files when the binary cannot be resolved", async () => { + const { manager } = setup(failingResolver()); + await expect( + manager.storeToken(TEST_URL, "my-token", configs), + ).rejects.toThrow("no binary"); expect(execFile).not.toHaveBeenCalled(); - expect(readCredentialFiles()).toStrictEqual({ - url: TEST_URL, - session: "my-token", - }); + expect(credentialFilesExist()).toBe(false); }); it("writes via coder login (file) when keyring is enabled but unsupported", async () => { diff --git a/test/unit/core/cliExec.test.ts b/test/unit/core/cliExec.test.ts index 3c578fbbe1..27548c4172 100644 --- a/test/unit/core/cliExec.test.ts +++ b/test/unit/core/cliExec.test.ts @@ -230,7 +230,10 @@ describe("cliExec", () => { `process.exit(1);`, ].join("\n"); const bin = await writeExecutable(tmp, "netcheck-err", code); - const { env } = setup({ mode: "global-config", configDir: "/tmp" }, bin); + const { env } = setup( + { mode: "global-config", configDir: "/tmp", allowOverride: true }, + bin, + ); await expect(cliExec.netcheck(env)).rejects.toThrow( "You are not logged in", ); @@ -240,7 +243,10 @@ describe("cliExec", () => { // Hangs forever so the only way out is the abort signal. const code = `setInterval(() => {}, 1000);`; const bin = await writeExecutable(tmp, "netcheck-hang", code); - const { env } = setup({ mode: "global-config", configDir: "/tmp" }, bin); + const { env } = setup( + { mode: "global-config", configDir: "/tmp", allowOverride: true }, + bin, + ); const ac = new AbortController(); ac.abort(); await expect(cliExec.netcheck(env, ac.signal)).rejects.toMatchObject({ From c76d431ca1394dacf4358fe358b9d2cf9c373cc6 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 25 Jun 2026 13:44:38 +0300 Subject: [PATCH 7/8] fix: make credential binary resolution locate-only Routing file-mode credentials through `coder login`/`logout` means the credential binary resolver now runs in the login and logout flows, not just on connect. It previously fell back to `fetchBinary` on a cache miss, so a file-mode user's login (including auto-login on startup) or logout could trigger a surprise CLI download with a progress popup, where before it was silent file I/O. Make the resolver locate-only. The connect and CLI-command flows still fetch the binary first, so credential ops become best-effort against an already-present binary and never download on their own. If no binary exists yet, credential sharing is simply skipped until a real connection fetches one. --- src/core/container.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/core/container.ts b/src/core/container.ts index 289cf428ab..b448a1bdda 100644 --- a/src/core/container.ts +++ b/src/core/container.ts @@ -1,6 +1,5 @@ import * as vscode from "vscode"; -import { CoderApi } from "../api/coderApi"; import { AuthTelemetry } from "../instrumentation/auth"; import { LoginCoordinator } from "../login/loginCoordinator"; import { OAuthCallback } from "../oauth/oauthCallback"; @@ -83,12 +82,9 @@ export class ServiceContainer implements vscode.Disposable { "BinaryResolver called before CliManager was initialised", ); } - try { - return await this.cliManager.locateBinary(url); - } catch { - const client = CoderApi.create(url, "", this.logger); - return this.cliManager.fetchBinary(client); - } + // Locate-only: never download from credential ops; the connect and + // CLI-command flows fetch the binary first. + return this.cliManager.locateBinary(url); }, this.pathResolver, this.telemetryService, From 5ddf592fa79beb37ce5748deb0565b62b697c6c3 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Fri, 26 Jun 2026 02:19:56 +0300 Subject: [PATCH 8/8] refactor: address review feedback on global-config - rename FeatureSet.keyringTokenRead to tokenRead (token subcommand works for both stores) - rename resolveUiUrl to resolveCoderDashboardUrl - clarify openInBrowser docstring (connectionUrl is arbitrary) - split buildGlobalFlags on auth mode to drop the nested ternary --- src/core/cliCredentialManager.ts | 6 ++-- src/featureSet.ts | 6 ++-- src/oauth/authorizer.ts | 4 +-- src/settings/cli.ts | 33 ++++++++++----------- src/util/uri.ts | 8 ++--- test/unit/api/workspace.test.ts | 2 +- test/unit/core/cliCredentialManager.test.ts | 2 +- test/unit/featureSet.test.ts | 4 +-- test/unit/util/uri.test.ts | 10 +++---- 9 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index 4002036313..763d12d65f 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -257,11 +257,11 @@ export class CliCredentialManager { return { kind: "none" }; } if (isKeyringEnabled(configs) && cli.featureSet.keyringAuth) { - return cli.featureSet.keyringTokenRead + return cli.featureSet.tokenRead ? { kind: "keyring", binPath: cli.binPath } : { kind: "none" }; } - if (cli.featureSet.keyringTokenRead) { + if (cli.featureSet.tokenRead) { return cliFileTransport(cli); } return { kind: "none" }; @@ -338,7 +338,7 @@ function cliFileTransport(cli: { return { kind: "cli-file", binPath: cli.binPath, - allowOverride: cli.featureSet.keyringTokenRead, + allowOverride: cli.featureSet.tokenRead, }; } diff --git a/src/featureSet.ts b/src/featureSet.ts index 7c7ac41e6a..57a3b1d684 100644 --- a/src/featureSet.ts +++ b/src/featureSet.ts @@ -7,7 +7,7 @@ export interface FeatureSet { buildReason: boolean; cliUpdate: boolean; keyringAuth: boolean; - keyringTokenRead: boolean; + tokenRead: boolean; supportBundle: boolean; } @@ -45,8 +45,8 @@ export function featureSetForVersion( cliUpdate: versionAtLeast(version, "2.24.0"), // Keyring-backed token storage via `coder login` keyringAuth: versionAtLeast(version, "2.29.0"), - // `coder login token` for reading tokens from the keyring - keyringTokenRead: versionAtLeast(version, "2.31.0"), + // `coder login token` for reading tokens (keyring or file) + tokenRead: versionAtLeast(version, "2.31.0"), // `coder support bundle` (officially released/unhidden in 2.10.0) supportBundle: versionAtLeast(version, "2.10.0"), }; diff --git a/src/oauth/authorizer.ts b/src/oauth/authorizer.ts index 3da0d7c11f..864ca08a47 100644 --- a/src/oauth/authorizer.ts +++ b/src/oauth/authorizer.ts @@ -1,7 +1,7 @@ import * as vscode from "vscode"; import { CoderApi } from "../api/coderApi"; -import { resolveUiUrl } from "../util/uri"; +import { resolveCoderDashboardUrl } from "../util/uri"; import { AUTH_GRANT_TYPE, @@ -382,7 +382,7 @@ function toBrowserAuthorizationUrl( ): URL { const endpoint = new URL(authorizationEndpoint); const connectionBase = new URL(connectionUrl); - const browserBase = new URL(resolveUiUrl(connectionUrl)); + const browserBase = new URL(resolveCoderDashboardUrl(connectionUrl)); const connectionPrefix = connectionBase.pathname.replace(/\/$/, ""); const browserPrefix = browserBase.pathname.replace(/\/$/, ""); const underConnection = diff --git a/src/settings/cli.ts b/src/settings/cli.ts index 94d17150b5..4827ec75db 100644 --- a/src/settings/cli.ts +++ b/src/settings/cli.ts @@ -52,26 +52,25 @@ function buildGlobalFlags( escHeader: (s: string) => string, ): string[] { const userFlags = getExpandedUserGlobalFlags(configs); + const headers = getHeaderArgs(configs, escHeader); - // Honor a user `--global-config` only when allowOverride (file mode, 2.31+); - // otherwise strip it and emit our own so it matches where we read/write. + // Escape after stripping so expansion whitespace stays in one shell token. + const cleanUserFlags = (stripGlobalConfig: boolean) => + stripManagedFlags(userFlags, stripGlobalConfig).map(escAuth); + + // Keyring mode: --url auth; drop user --global-config (would force file storage). + if (auth.mode === "url") { + return [...cleanUserFlags(true), "--url", escAuth(auth.url), ...headers]; + } + + // File mode: keep the user's --global-config on 2.31+, else emit our own. const honorOverride = - auth.mode === "global-config" && auth.allowOverride && userFlags.some((flag) => isFlag(flag, "--global-config")); - - // Escape each user flag so expansion-introduced whitespace stays inside - // one shell token. `escAuth` is `identity` on the array path. - const filtered = stripManagedFlags(userFlags, !honorOverride).map(escAuth); - - const authFlags = - auth.mode === "url" - ? ["--url", escAuth(auth.url)] - : honorOverride - ? [] - : ["--global-config", escAuth(auth.configDir)]; - - return [...filtered, ...authFlags, ...getHeaderArgs(configs, escHeader)]; + const authFlags = honorOverride + ? [] + : ["--global-config", escAuth(auth.configDir)]; + return [...cleanUserFlags(!honorOverride), ...authFlags, ...headers]; } function stripManagedFlags( @@ -129,7 +128,7 @@ export function resolveCliAuth( return { mode: "global-config", configDir, - allowOverride: featureSet.keyringTokenRead, + allowOverride: featureSet.tokenRead, }; } diff --git a/src/util/uri.ts b/src/util/uri.ts index 0072118e5d..669590f5be 100644 --- a/src/util/uri.ts +++ b/src/util/uri.ts @@ -25,7 +25,7 @@ export function normalizeUrl(value: string): string { * `coder.alternativeWebUrl` setting when configured, otherwise returns * the connection URL unchanged. */ -export function resolveUiUrl(connectionUrl: string): string { +export function resolveCoderDashboardUrl(connectionUrl: string): string { const alt = normalizeUrl( vscode.workspace .getConfiguration("coder") @@ -35,13 +35,13 @@ export function resolveUiUrl(connectionUrl: string): string { } /** - * Open a path on the Coder deployment in the user's browser, applying - * `coder.alternativeWebUrl` when configured. + * Open a path in the user's browser, resolved against `coder.alternativeWebUrl` + * when set, otherwise against `connectionUrl`. */ export function openInBrowser( connectionUrl: string, path: string, ): Thenable { - const base = vscode.Uri.parse(resolveUiUrl(connectionUrl)); + const base = vscode.Uri.parse(resolveCoderDashboardUrl(connectionUrl)); return vscode.env.openExternal(vscode.Uri.joinPath(base, path)); } diff --git a/test/unit/api/workspace.test.ts b/test/unit/api/workspace.test.ts index 4ed140413a..fea30aeef3 100644 --- a/test/unit/api/workspace.test.ts +++ b/test/unit/api/workspace.test.ts @@ -28,7 +28,7 @@ const featureSet: FeatureSet = { buildReason: true, cliUpdate: true, keyringAuth: true, - keyringTokenRead: true, + tokenRead: true, supportBundle: true, }; diff --git a/test/unit/core/cliCredentialManager.test.ts b/test/unit/core/cliCredentialManager.test.ts index a3fea9ed83..15b90bc311 100644 --- a/test/unit/core/cliCredentialManager.test.ts +++ b/test/unit/core/cliCredentialManager.test.ts @@ -491,7 +491,7 @@ describe("CliCredentialManager", () => { it("returns undefined when CLI version too old for token read", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); - // 2.30 supports keyringAuth but not keyringTokenRead (requires 2.31+) + // 2.30 supports keyringAuth but not tokenRead (requires 2.31+) vi.mocked(cliExec.version).mockResolvedValueOnce("2.30.0"); stubExecFile({ stdout: "my-token" }); const { manager } = setup(); diff --git a/test/unit/featureSet.test.ts b/test/unit/featureSet.test.ts index 5be56d255a..4b6bf6b09a 100644 --- a/test/unit/featureSet.test.ts +++ b/test/unit/featureSet.test.ts @@ -48,9 +48,9 @@ describe("check version support", () => { ["v2.29.0", "v2.29.1", "v2.30.0", "v3.0.0"], ); }); - it("keyring token read", () => { + it("token read", () => { expectFlag( - "keyringTokenRead", + "tokenRead", ["v2.30.0", "v2.29.0", "v2.28.0", "v1.0.0"], ["v2.31.0", "v2.31.1", "v2.32.0", "v3.0.0"], ); diff --git a/test/unit/util/uri.test.ts b/test/unit/util/uri.test.ts index 68a90161d6..6c326b4874 100644 --- a/test/unit/util/uri.test.ts +++ b/test/unit/util/uri.test.ts @@ -5,7 +5,7 @@ import { normalizeUrl, openInBrowser, removeTrailingSlashes, - resolveUiUrl, + resolveCoderDashboardUrl, toSafeHost, } from "@/util/uri"; @@ -51,7 +51,7 @@ describe("normalizeUrl", () => { }); }); -describe("resolveUiUrl", () => { +describe("resolveCoderDashboardUrl", () => { let configurationProvider: MockConfigurationProvider; beforeEach(() => { @@ -59,7 +59,7 @@ describe("resolveUiUrl", () => { }); it("returns the connection URL when no alternative is configured", () => { - expect(resolveUiUrl("https://coder.example.com:7004")).toBe( + expect(resolveCoderDashboardUrl("https://coder.example.com:7004")).toBe( "https://coder.example.com:7004", ); }); @@ -71,7 +71,7 @@ describe("resolveUiUrl", () => { "returns the connection URL when the alternative is $name", ({ value }) => { configurationProvider.set("coder.alternativeWebUrl", value); - expect(resolveUiUrl("https://coder.example.com:7004")).toBe( + expect(resolveCoderDashboardUrl("https://coder.example.com:7004")).toBe( "https://coder.example.com:7004", ); }, @@ -90,7 +90,7 @@ describe("resolveUiUrl", () => { { name: "trims whitespace", value: " https://coder.example.com " }, ])("$name", ({ value }) => { configurationProvider.set("coder.alternativeWebUrl", value); - expect(resolveUiUrl("https://coder.example.com:7004")).toBe( + expect(resolveCoderDashboardUrl("https://coder.example.com:7004")).toBe( "https://coder.example.com", ); });