diff --git a/esbuild.mjs b/esbuild.mjs index 54fb4a1d..9a0e89f3 100644 --- a/esbuild.mjs +++ b/esbuild.mjs @@ -32,7 +32,7 @@ const buildOptions = { // undefined when bundled to CJS, causing runtime errors. openpgp: "./node_modules/openpgp/dist/node/openpgp.min.cjs", }, - external: ["vscode"], + external: ["vscode", "@napi-rs/keyring"], sourcemap: production ? "external" : true, minify: production, plugins: watch ? [logRebuildPlugin] : [], diff --git a/eslint.config.mjs b/eslint.config.mjs index 87839fc7..09b06257 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -158,7 +158,7 @@ export default defineConfig( // Build config - ESM with Node globals { - files: ["esbuild.mjs"], + files: ["esbuild.mjs", "scripts/*.mjs"], languageOptions: { globals: { ...globals.node, diff --git a/package.json b/package.json index b8885154..16c46227 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,7 @@ "test:integration": "tsc -p test --outDir out --noCheck && node esbuild.mjs && vscode-test", "test:webview": "vitest --project webview", "typecheck": "concurrently -g \"tsc --noEmit\" \"tsc --noEmit -p test\"", - "vscode:prepublish": "pnpm build:production", + "vscode:prepublish": "pnpm build:production && node scripts/vendor-keyring.mjs", "watch": "concurrently -n extension,webviews \"pnpm watch:extension\" \"pnpm watch:webviews\"", "watch:extension": "node esbuild.mjs --watch", "watch:webviews": "pnpm -r --filter \"./packages/*\" --parallel dev" @@ -164,6 +164,11 @@ "type": "string" } }, + "coder.useKeyring": { + "markdownDescription": "Store session tokens in the OS keyring (macOS Keychain, Windows Credential Manager) instead of plaintext files. Requires CLI >= 2.29.0. Has no effect on Linux.", + "type": "boolean", + "default": true + }, "coder.httpClientLogLevel": { "markdownDescription": "Controls the verbosity of HTTP client logging. This affects what details are logged for each HTTP request and response.", "type": "string", @@ -471,6 +476,7 @@ "word-wrap": "1.2.5" }, "dependencies": { + "@napi-rs/keyring": "^1.2.0", "@peculiar/x509": "^1.14.3", "@repo/shared": "workspace:*", "axios": "1.13.5", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6d9fb826..ba00bdf5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -55,6 +55,9 @@ importers: .: dependencies: + '@napi-rs/keyring': + specifier: ^1.2.0 + version: 1.2.0 '@peculiar/x509': specifier: ^1.14.3 version: 1.14.3 @@ -178,7 +181,7 @@ importers: version: 4.1.0 coder: specifier: 'catalog:' - version: https://codeload.github.com/coder/coder/tar.gz/cfdbd5251a79ca1d88f46c20b9cc7c66fd167ae0 + version: https://codeload.github.com/coder/coder/tar.gz/4057363f781dd2b480936d9836540866ca08080d concurrently: specifier: ^9.2.1 version: 9.2.1 @@ -935,6 +938,87 @@ packages: '@lit/reactive-element@2.1.2': resolution: {integrity: sha512-pbCDiVMnne1lYUIaYNN5wrwQXDtHaYtg7YEFPeW+hws6U47WeFvISGUWekPGKWOP1ygrs0ef0o1VJMk1exos5A==} + '@napi-rs/keyring-darwin-arm64@1.2.0': + resolution: {integrity: sha512-CA83rDeyONDADO25JLZsh3eHY8yTEtm/RS6ecPsY+1v+dSawzT9GywBMu2r6uOp1IEhQs/xAfxgybGAFr17lSA==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [darwin] + + '@napi-rs/keyring-darwin-x64@1.2.0': + resolution: {integrity: sha512-dBHjtKRCj4ByfnfqIKIJLo3wueQNJhLRyuxtX/rR4K/XtcS7VLlRD01XXizjpre54vpmObj63w+ZpHG+mGM8uA==} + engines: {node: '>= 10'} + cpu: [x64] + os: [darwin] + + '@napi-rs/keyring-freebsd-x64@1.2.0': + resolution: {integrity: sha512-DPZFr11pNJSnaoh0dzSUNF+T6ORhy3CkzUT3uGixbA71cAOPJ24iG8e8QrLOkuC/StWrAku3gBnth2XMWOcR3Q==} + engines: {node: '>= 10'} + cpu: [x64] + os: [freebsd] + + '@napi-rs/keyring-linux-arm-gnueabihf@1.2.0': + resolution: {integrity: sha512-8xv6DyEMlvRdqJzp4F39RLUmmTQsLcGYYv/3eIfZNZN1O5257tHxTrFYqAsny659rJJK2EKeSa7PhrSibQqRWQ==} + engines: {node: '>= 10'} + cpu: [arm] + os: [linux] + + '@napi-rs/keyring-linux-arm64-gnu@1.2.0': + resolution: {integrity: sha512-Pu2V6Py+PBt7inryEecirl+t+ti8bhZphjP+W68iVaXHUxLdWmkgL9KI1VkbRHbx5k8K5Tew9OP218YfmVguIA==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-arm64-musl@1.2.0': + resolution: {integrity: sha512-8TDymrpC4P1a9iDEaegT7RnrkmrJN5eNZh3Im3UEV5PPYGtrb82CRxsuFohthCWQW81O483u1bu+25+XA4nKUw==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + libc: [musl] + + '@napi-rs/keyring-linux-riscv64-gnu@1.2.0': + resolution: {integrity: sha512-awsB5XI1MYL7fwfjMDGmKOWvNgJEO7mM7iVEMS0fO39f0kVJnOSjlu7RHcXAF0LOx+0VfF3oxbWqJmZbvRCRHw==} + engines: {node: '>= 10'} + cpu: [riscv64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-x64-gnu@1.2.0': + resolution: {integrity: sha512-8E+7z4tbxSJXxIBqA+vfB1CGajpCDRyTyqXkBig5NtASrv4YXcntSo96Iah2QDR5zD3dSTsmbqJudcj9rKKuHQ==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-x64-musl@1.2.0': + resolution: {integrity: sha512-8RZ8yVEnmWr/3BxKgBSzmgntI7lNEsY7xouNfOsQkuVAiCNmxzJwETspzK3PQ2FHtDxgz5vHQDEBVGMyM4hUHA==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + libc: [musl] + + '@napi-rs/keyring-win32-arm64-msvc@1.2.0': + resolution: {integrity: sha512-AoqaDZpQ6KPE19VBLpxyORcp+yWmHI9Xs9Oo0PJ4mfHma4nFSLVdhAubJCxdlNptHe5va7ghGCHj3L9Akiv4cQ==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [win32] + + '@napi-rs/keyring-win32-ia32-msvc@1.2.0': + resolution: {integrity: sha512-EYL+EEI6bCsYi3LfwcQdnX3P/R76ENKNn+3PmpGheBsUFLuh0gQuP7aMVHM4rTw6UVe+L3vCLZSptq/oeacz0A==} + engines: {node: '>= 10'} + cpu: [ia32] + os: [win32] + + '@napi-rs/keyring-win32-x64-msvc@1.2.0': + resolution: {integrity: sha512-xFlx/TsmqmCwNU9v+AVnEJgoEAlBYgzFF5Ihz1rMpPAt4qQWWkMd4sCyM1gMJ1A/GnRqRegDiQpwaxGUHFtFbA==} + engines: {node: '>= 10'} + cpu: [x64] + os: [win32] + + '@napi-rs/keyring@1.2.0': + resolution: {integrity: sha512-d0d4Oyxm+v980PEq1ZH2PmS6cvpMIRc17eYpiU47KgW+lzxklMu6+HOEOPmxrpnF/XQZ0+Q78I2mgMhbIIo/dg==} + engines: {node: '>= 10'} + '@napi-rs/wasm-runtime@0.2.12': resolution: {integrity: sha512-ZVWUcfwY4E/yPitQJl481FjFo3K22D6qF0DuFH6Y/nbnE11GY5uguDxZMGXPQ8WQ0128MXQD7TnfHyK4oWoIJQ==} @@ -1938,8 +2022,8 @@ packages: resolution: {integrity: sha512-gfrHV6ZPkquExvMh9IOkKsBzNDk6sDuZ6DdBGUBkvFnTCqCxzpuq48RySgP0AnaqQkw2zynOFj9yly6T1Q2G5Q==} engines: {node: '>=16'} - coder@https://codeload.github.com/coder/coder/tar.gz/cfdbd5251a79ca1d88f46c20b9cc7c66fd167ae0: - resolution: {tarball: https://codeload.github.com/coder/coder/tar.gz/cfdbd5251a79ca1d88f46c20b9cc7c66fd167ae0} + coder@https://codeload.github.com/coder/coder/tar.gz/4057363f781dd2b480936d9836540866ca08080d: + resolution: {tarball: https://codeload.github.com/coder/coder/tar.gz/4057363f781dd2b480936d9836540866ca08080d} version: 0.0.0 color-convert@2.0.1: @@ -5133,6 +5217,57 @@ snapshots: dependencies: '@lit-labs/ssr-dom-shim': 1.5.1 + '@napi-rs/keyring-darwin-arm64@1.2.0': + optional: true + + '@napi-rs/keyring-darwin-x64@1.2.0': + optional: true + + '@napi-rs/keyring-freebsd-x64@1.2.0': + optional: true + + '@napi-rs/keyring-linux-arm-gnueabihf@1.2.0': + optional: true + + '@napi-rs/keyring-linux-arm64-gnu@1.2.0': + optional: true + + '@napi-rs/keyring-linux-arm64-musl@1.2.0': + optional: true + + '@napi-rs/keyring-linux-riscv64-gnu@1.2.0': + optional: true + + '@napi-rs/keyring-linux-x64-gnu@1.2.0': + optional: true + + '@napi-rs/keyring-linux-x64-musl@1.2.0': + optional: true + + '@napi-rs/keyring-win32-arm64-msvc@1.2.0': + optional: true + + '@napi-rs/keyring-win32-ia32-msvc@1.2.0': + optional: true + + '@napi-rs/keyring-win32-x64-msvc@1.2.0': + optional: true + + '@napi-rs/keyring@1.2.0': + optionalDependencies: + '@napi-rs/keyring-darwin-arm64': 1.2.0 + '@napi-rs/keyring-darwin-x64': 1.2.0 + '@napi-rs/keyring-freebsd-x64': 1.2.0 + '@napi-rs/keyring-linux-arm-gnueabihf': 1.2.0 + '@napi-rs/keyring-linux-arm64-gnu': 1.2.0 + '@napi-rs/keyring-linux-arm64-musl': 1.2.0 + '@napi-rs/keyring-linux-riscv64-gnu': 1.2.0 + '@napi-rs/keyring-linux-x64-gnu': 1.2.0 + '@napi-rs/keyring-linux-x64-musl': 1.2.0 + '@napi-rs/keyring-win32-arm64-msvc': 1.2.0 + '@napi-rs/keyring-win32-ia32-msvc': 1.2.0 + '@napi-rs/keyring-win32-x64-msvc': 1.2.0 + '@napi-rs/wasm-runtime@0.2.12': dependencies: '@emnapi/core': 1.7.1 @@ -6301,7 +6436,7 @@ snapshots: cockatiel@3.2.1: {} - coder@https://codeload.github.com/coder/coder/tar.gz/cfdbd5251a79ca1d88f46c20b9cc7c66fd167ae0: {} + coder@https://codeload.github.com/coder/coder/tar.gz/4057363f781dd2b480936d9836540866ca08080d: {} color-convert@2.0.1: dependencies: diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index ff3c5fd7..95094da6 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -26,3 +26,16 @@ onlyBuiltDependencies: - keytar - unrs-resolver - utf-8-validate + +# Install @napi-rs/keyring native binaries for macOS and Windows so they're +# available when building the universal VSIX (even on Linux CI). +# Only macOS and Windows use the keyring; Linux falls back to file storage. +supportedArchitectures: + os: + - current + - darwin + - win32 + cpu: + - current + - x64 + - arm64 diff --git a/scripts/vendor-keyring.mjs b/scripts/vendor-keyring.mjs new file mode 100644 index 00000000..5cf8cd31 --- /dev/null +++ b/scripts/vendor-keyring.mjs @@ -0,0 +1,61 @@ +/** + * Vendor @napi-rs/keyring into dist/node_modules/ for VSIX packaging. + * + * pnpm uses symlinks that vsce can't follow. This script resolves them and + * copies the JS wrapper plus macOS/Windows .node binaries into dist/, where + * Node's require() resolution finds them from dist/extension.js. + */ +import { + cpSync, + existsSync, + mkdirSync, + readdirSync, + realpathSync, + rmSync, +} from "node:fs"; +import { join, resolve } from "node:path"; + +const keyringPkg = resolve("node_modules/@napi-rs/keyring"); +const outputDir = resolve("dist/node_modules/@napi-rs/keyring"); + +if (!existsSync(keyringPkg)) { + console.error("@napi-rs/keyring not found — run pnpm install first"); + process.exit(1); +} + +// Copy the JS wrapper package (resolving pnpm symlinks) +const resolvedPkg = realpathSync(keyringPkg); +rmSync(outputDir, { recursive: true, force: true }); +mkdirSync(outputDir, { recursive: true }); +cpSync(resolvedPkg, outputDir, { recursive: true }); + +// Native binary packages live as siblings of the resolved keyring package in +// pnpm's content-addressable store (they aren't hoisted to node_modules). +const siblingsDir = resolve(resolvedPkg, ".."); +const nativePackages = [ + "keyring-darwin-arm64", + "keyring-darwin-x64", + "keyring-win32-arm64-msvc", + "keyring-win32-x64-msvc", +]; + +for (const pkg of nativePackages) { + const pkgDir = join(siblingsDir, pkg); + if (!existsSync(pkgDir)) { + console.error( + `Missing native package: ${pkg}\n` + + "Ensure supportedArchitectures in pnpm-workspace.yaml includes all target platforms.", + ); + process.exit(1); + } + const nodeFile = readdirSync(pkgDir).find((f) => f.endsWith(".node")); + if (!nodeFile) { + console.error(`No .node binary found in ${pkg}`); + process.exit(1); + } + cpSync(join(pkgDir, nodeFile), join(outputDir, nodeFile)); +} + +console.log( + `Vendored @napi-rs/keyring with ${nativePackages.length} platform binaries into dist/`, +); diff --git a/src/api/workspace.ts b/src/api/workspace.ts index fdedfcb8..e7d38327 100644 --- a/src/api/workspace.ts +++ b/src/api/workspace.ts @@ -7,7 +7,7 @@ import { import { spawn } from "node:child_process"; import * as vscode from "vscode"; -import { getGlobalFlags } from "../cliConfig"; +import { type CliAuth, getGlobalFlags } from "../cliConfig"; import { type FeatureSet } from "../featureSet"; import { escapeCommandArg } from "../util"; import { type UnidirectionalStream } from "../websocket/eventStreamConnection"; @@ -50,7 +50,7 @@ export class LazyStream { */ export async function startWorkspaceIfStoppedOrFailed( restClient: Api, - globalConfigDir: string, + auth: CliAuth, binPath: string, workspace: Workspace, writeEmitter: vscode.EventEmitter, @@ -65,7 +65,7 @@ export async function startWorkspaceIfStoppedOrFailed( return new Promise((resolve, reject) => { const startArgs = [ - ...getGlobalFlags(vscode.workspace.getConfiguration(), globalConfigDir), + ...getGlobalFlags(vscode.workspace.getConfiguration(), auth), "start", "--yes", createWorkspaceIdentifier(workspace), diff --git a/src/cliConfig.ts b/src/cliConfig.ts index 1f23949d..c38fc3a1 100644 --- a/src/cliConfig.ts +++ b/src/cliConfig.ts @@ -1,39 +1,76 @@ -import { type WorkspaceConfiguration } from "vscode"; +import * as vscode from "vscode"; +import { type FeatureSet } from "./featureSet"; import { getHeaderArgs } from "./headers"; +import { isKeyringSupported } from "./keyringStore"; import { escapeCommandArg } from "./util"; +export type CliAuth = + | { mode: "global-config"; configDir: string } + | { mode: "url"; url: string }; + /** * Returns the raw global flags from user configuration. */ export function getGlobalFlagsRaw( - configs: Pick, + configs: Pick, ): string[] { return configs.get("coder.globalFlags", []); } /** * Returns global configuration flags for Coder CLI commands. - * Always includes the `--global-config` argument with the specified config directory. + * Includes either `--global-config` or `--url` depending on the auth mode. */ export function getGlobalFlags( - configs: Pick, - configDir: string, + configs: Pick, + auth: CliAuth, ): string[] { + const authFlags = + auth.mode === "url" + ? ["--url", escapeCommandArg(auth.url)] + : ["--global-config", escapeCommandArg(auth.configDir)]; + // Last takes precedence/overrides previous ones return [ ...getGlobalFlagsRaw(configs), - "--global-config", - escapeCommandArg(configDir), + ...authFlags, ...getHeaderArgs(configs), ]; } +/** + * Single source of truth: should the extension use the OS keyring for this session? + * Requires CLI >= 2.29.0, macOS or Windows, and the coder.useKeyring setting enabled. + */ +export function shouldUseKeyring(featureSet: FeatureSet): boolean { + return ( + featureSet.keyringAuth && + isKeyringSupported() && + vscode.workspace.getConfiguration().get("coder.useKeyring", true) + ); +} + +/** + * Resolves how the CLI should authenticate: via the keyring (`--url`) or via + * the global config directory (`--global-config`). + */ +export function resolveCliAuth( + featureSet: FeatureSet, + deploymentUrl: string | undefined, + configDir: string, +): CliAuth { + if (shouldUseKeyring(featureSet) && deploymentUrl) { + return { mode: "url", url: deploymentUrl }; + } + return { mode: "global-config", configDir }; +} + /** * Returns SSH flags for the `coder ssh` command from user configuration. */ export function getSshFlags( - configs: Pick, + configs: Pick, ): string[] { // Make sure to match this default with the one in the package.json return configs.get("coder.sshFlags", ["--disable-autostart"]); diff --git a/src/commands.ts b/src/commands.ts index 107f9556..50c3490e 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -4,12 +4,14 @@ import { } from "coder/site/src/api/typesGenerated"; import * as fs from "node:fs/promises"; import * as path from "node:path"; +import * as semver from "semver"; import * as vscode from "vscode"; import { createWorkspaceIdentifier, extractAgents } from "./api/api-helper"; import { type CoderApi } from "./api/coderApi"; -import { getGlobalFlags } from "./cliConfig"; +import { getGlobalFlags, resolveCliAuth } from "./cliConfig"; import { type CliManager } from "./core/cliManager"; +import * as cliUtils from "./core/cliUtils"; import { type ServiceContainer } from "./core/container"; import { type MementoManager } from "./core/mementoManager"; import { type PathResolver } from "./core/pathResolver"; @@ -17,6 +19,7 @@ import { type SecretsManager } from "./core/secretsManager"; import { type DeploymentManager } from "./deployment/deploymentManager"; import { CertificateError } from "./error/certificateError"; import { toError } from "./error/errorUtils"; +import { featureSetForVersion } from "./featureSet"; import { type Logger } from "./logging/logger"; import { type LoginCoordinator } from "./login/loginCoordinator"; import { maybeAskAgent, maybeAskUrl } from "./promptUtils"; @@ -210,12 +213,13 @@ export class Commands { this.logger.debug("Logging out"); - const safeHostname = - this.deploymentManager.getCurrentDeployment()?.safeHostname; + const deployment = this.deploymentManager.getCurrentDeployment(); + const safeHostname = deployment?.safeHostname; await this.deploymentManager.clearDeployment(); if (safeHostname) { + await this.cliManager.clearCredentials(safeHostname); await this.secretsManager.clearAllAuthData(safeHostname); } @@ -283,6 +287,7 @@ export class Commands { if (selected.hostnames.length === 1) { const selectedHostname = selected.hostnames[0]; + await this.cliManager.clearCredentials(selectedHostname); await this.secretsManager.clearAllAuthData(selectedHostname); this.logger.info("Removed credentials for", selectedHostname); vscode.window.showInformationMessage( @@ -300,9 +305,10 @@ export class Commands { ); if (confirm === "Remove All") { await Promise.all( - selected.hostnames.map((h) => - this.secretsManager.clearAllAuthData(h), - ), + selected.hostnames.map(async (h) => { + await this.cliManager.clearCredentials(h); + await this.secretsManager.clearAllAuthData(h); + }), ); this.logger.info( "Removed credentials for all deployments:", @@ -452,10 +458,13 @@ export class Commands { safeHost, ); + const version = semver.parse(await cliUtils.version(binary)); + const featureSet = featureSetForVersion(version); const configDir = this.pathResolver.getGlobalConfigDir(safeHost); + const auth = resolveCliAuth(featureSet, baseUrl, configDir); const globalFlags = getGlobalFlags( vscode.workspace.getConfiguration(), - configDir, + auth, ); terminal.sendText( `${escapeCommandArg(binary)} ${globalFlags.join(" ")} ssh ${app.workspace_name}`, diff --git a/src/core/cliManager.ts b/src/core/cliManager.ts index 0c5572e6..7a653f30 100644 --- a/src/core/cliManager.ts +++ b/src/core/cliManager.ts @@ -2,24 +2,30 @@ import globalAxios, { type AxiosInstance, type AxiosRequestConfig, } from "axios"; -import { type Api } from "coder/site/src/api/api"; import { createWriteStream, type WriteStream } from "node:fs"; import fs from "node:fs/promises"; -import { type IncomingMessage } from "node:http"; import path from "node:path"; import prettyBytes from "pretty-bytes"; import * as semver from "semver"; import * as vscode from "vscode"; import { errToStr } from "../api/api-helper"; -import { type Logger } from "../logging/logger"; +import { shouldUseKeyring } from "../cliConfig"; +import { isKeyringSupported, type KeyringStore } from "../keyringStore"; import * as pgp from "../pgp"; import { vscodeProposed } from "../vscodeProposed"; import { BinaryLock } from "./binaryLock"; import * as cliUtils from "./cliUtils"; import * as downloadProgress from "./downloadProgress"; -import { type PathResolver } from "./pathResolver"; + +import type { Api } from "coder/site/src/api/api"; +import type { IncomingMessage } from "node:http"; + +import type { FeatureSet } from "../featureSet"; +import type { Logger } from "../logging/logger"; + +import type { PathResolver } from "./pathResolver"; export class CliManager { private readonly binaryLock: BinaryLock; @@ -27,6 +33,7 @@ export class CliManager { constructor( private readonly output: Logger, private readonly pathResolver: PathResolver, + private readonly keyringStore: KeyringStore, ) { this.binaryLock = new BinaryLock(output); } @@ -708,13 +715,51 @@ export class CliManager { safeHostname: string, url: string | undefined, token: string | null, + featureSet: FeatureSet, ) { + if (shouldUseKeyring(featureSet) && url && token !== null) { + try { + this.keyringStore.setToken(url, token); + this.output.info("Stored token in OS keyring for", url); + return; + } catch (error) { + this.output.warn( + "Keyring write failed, falling back to file storage", + error, + ); + } + } await Promise.all([ this.updateUrlForCli(safeHostname, url), this.updateTokenForCli(safeHostname, token), ]); } + /** + * Remove credentials for a deployment from both keyring and file storage. + */ + public async clearCredentials(safeHostname: string): Promise { + if (isKeyringSupported()) { + try { + this.keyringStore.deleteToken(safeHostname); + this.output.info("Removed keyring token for", safeHostname); + } catch (error) { + this.output.warn("Failed to remove keyring token", error); + } + } + + const tokenPath = this.pathResolver.getSessionTokenPath(safeHostname); + const urlPath = this.pathResolver.getUrlPath(safeHostname); + await Promise.all([ + fs.rm(tokenPath, { force: true }).catch((error) => { + this.output.warn("Failed to remove token file", tokenPath, error); + }), + fs.rm(urlPath, { force: true }).catch((error) => { + this.output.warn("Failed to remove URL file", urlPath, error); + }), + ]); + } + /** * Update the URL for the deployment with the provided hostname on disk which * can be used by the CLI via --url-file. If the URL is falsey, do nothing. @@ -757,7 +802,7 @@ export class CliManager { const tempPath = filePath + ".temp-" + Math.random().toString(36).substring(8); try { - await fs.writeFile(tempPath, content); + await fs.writeFile(tempPath, content, { mode: 0o600 }); await fs.rename(tempPath, filePath); } catch (err) { await fs.rm(tempPath, { force: true }).catch((rmErr) => { diff --git a/src/core/container.ts b/src/core/container.ts index 7ea0b76e..51413c2e 100644 --- a/src/core/container.ts +++ b/src/core/container.ts @@ -1,5 +1,6 @@ import * as vscode from "vscode"; +import { KeyringStore } from "../keyringStore"; import { type Logger } from "../logging/logger"; import { LoginCoordinator } from "../login/loginCoordinator"; @@ -18,6 +19,7 @@ export class ServiceContainer implements vscode.Disposable { private readonly pathResolver: PathResolver; private readonly mementoManager: MementoManager; private readonly secretsManager: SecretsManager; + private readonly keyringStore: KeyringStore; private readonly cliManager: CliManager; private readonly contextManager: ContextManager; private readonly loginCoordinator: LoginCoordinator; @@ -34,12 +36,18 @@ export class ServiceContainer implements vscode.Disposable { context.globalState, this.logger, ); - this.cliManager = new CliManager(this.logger, this.pathResolver); + this.keyringStore = new KeyringStore(this.logger); + this.cliManager = new CliManager( + this.logger, + this.pathResolver, + this.keyringStore, + ); this.contextManager = new ContextManager(context); this.loginCoordinator = new LoginCoordinator( this.secretsManager, this.mementoManager, this.logger, + this.keyringStore, context.extension.id, ); } @@ -68,6 +76,10 @@ export class ServiceContainer implements vscode.Disposable { return this.contextManager; } + getKeyringStore(): KeyringStore { + return this.keyringStore; + } + getLoginCoordinator(): LoginCoordinator { return this.loginCoordinator; } diff --git a/src/featureSet.ts b/src/featureSet.ts index d5f54452..7f66d846 100644 --- a/src/featureSet.ts +++ b/src/featureSet.ts @@ -5,6 +5,7 @@ export interface FeatureSet { proxyLogDirectory: boolean; wildcardSSH: boolean; buildReason: boolean; + keyringAuth: boolean; } /** @@ -35,5 +36,10 @@ export function featureSetForVersion( buildReason: (version?.compare("2.25.0") ?? 0) >= 0 || version?.prerelease[0] === "devel", + + // Keyring-backed token storage was added in CLI 2.29.0 + keyringAuth: + (version?.compare("2.29.0") ?? 0) >= 0 || + version?.prerelease[0] === "devel", }; } diff --git a/src/keyringStore.ts b/src/keyringStore.ts new file mode 100644 index 00000000..3e975582 --- /dev/null +++ b/src/keyringStore.ts @@ -0,0 +1,182 @@ +import { type Logger } from "./logging/logger"; + +/** A single entry in the CLI's keyring credential map. */ +interface CredentialEntry { + coder_url: string; + api_token: string; +} + +type CredentialMap = Record; + +/** Subset of @napi-rs/keyring Entry used for credential storage. */ +export interface KeyringEntry { + getPassword(): string | null; + setPassword(password: string): void; + getSecret(): Uint8Array | null; + setSecret(secret: Uint8Array): void; + deleteCredential(): void; +} + +const KEYRING_SERVICE = "coder-v2-credentials"; +const KEYRING_ACCOUNT = "coder-login-credentials"; + +function createDefaultEntry(): KeyringEntry { + // Lazy require so Linux never loads the native binary. + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { Entry } = require("@napi-rs/keyring") as { + Entry: { + new (service: string, username: string): KeyringEntry; + withTarget( + target: string, + service: string, + username: string, + ): KeyringEntry; + }; + }; + + if (process.platform === "darwin") { + // On macOS, withTarget selects a keychain domain, not an attribute — using + // it creates a separate entry the CLI can't find. The two-arg constructor + // matches the CLI's kSecAttrService + kSecAttrAccount lookup. + return new Entry(KEYRING_SERVICE, KEYRING_ACCOUNT); + } + + // On Windows, withTarget sets the credential's lookup key, matching the CLI. + return Entry.withTarget(KEYRING_SERVICE, KEYRING_SERVICE, KEYRING_ACCOUNT); +} + +/** Extracts the host from a URL for use as credential map key (matches CLI format). */ +function toHost(deploymentUrl: string): string { + try { + return new URL(deploymentUrl).host; + } catch { + return deploymentUrl; + } +} + +/** + * Finds the map key matching a safeHostname. VS Code identifies deployments by + * safeHostname (port stripped), while the CLI stores map keys via `toHost` + * which preserves ports. The fallback strips ports from map keys so VS Code's + * port-less hostname still matches a CLI-written entry with a port. + */ +function findMapKey( + map: CredentialMap, + safeHostname: string, +): string | undefined { + if (safeHostname in map) { + return safeHostname; + } + for (const key of Object.keys(map)) { + const hostWithoutPort = key.replace(/:\d+$/, ""); + if (hostWithoutPort === safeHostname) { + return key; + } + } + return undefined; +} + +/** + * Returns true on platforms where the OS keyring is supported (macOS, Windows). + */ +export function isKeyringSupported(): boolean { + return process.platform === "darwin" || process.platform === "win32"; +} + +/** + * Wraps @napi-rs/keyring with the credential encoding the Coder CLI expects. + * The native module is loaded lazily so Linux never touches it. + * + * Encoding (must match CLI): + * macOS: base64-encoded JSON via setPassword/getPassword + * Windows: raw UTF-8 JSON bytes via setSecret/getSecret + */ +export class KeyringStore { + public constructor( + private readonly logger: Logger, + private readonly entryFactory: () => KeyringEntry = createDefaultEntry, + ) {} + + public setToken(deploymentUrl: string, token: string): void { + this.assertSupported(); + const entry = this.entryFactory(); + const map = this.readMap(entry); + const host = toHost(deploymentUrl); + map[host] = { coder_url: host, api_token: token }; + this.writeMap(entry, map); + } + + public getToken(safeHostname: string): string | undefined { + this.assertSupported(); + const entry = this.entryFactory(); + const map = this.readMap(entry); + const key = findMapKey(map, safeHostname); + return key !== undefined ? map[key].api_token : undefined; + } + + public deleteToken(safeHostname: string): void { + this.assertSupported(); + const entry = this.entryFactory(); + const map = this.readMap(entry); + const key = findMapKey(map, safeHostname); + if (key === undefined) { + return; + } + delete map[key]; + if (Object.keys(map).length === 0) { + try { + entry.deleteCredential(); + } catch { + // NoEntry is fine — nothing to delete + } + } else { + this.writeMap(entry, map); + } + } + + private assertSupported(): void { + if (!isKeyringSupported()) { + throw new Error(`Keyring is not supported on ${process.platform}`); + } + } + + private readMap(entry: KeyringEntry): CredentialMap { + try { + const raw = this.readRaw(entry); + if (!raw) { + return {}; + } + return JSON.parse(raw) as CredentialMap; + } catch (error) { + this.logger.warn("Failed to read keyring credential map", error); + return {}; + } + } + + private readRaw(entry: KeyringEntry): string | null { + if (process.platform === "darwin") { + const password = entry.getPassword(); + return password !== null + ? Buffer.from(password, "base64").toString("utf-8") + : null; + } + if (process.platform === "win32") { + const secret = entry.getSecret(); + return secret !== null ? Buffer.from(secret).toString("utf-8") : null; + } + throw new Error(`Keyring is not supported on ${process.platform}`); + } + + private writeMap(entry: KeyringEntry, map: CredentialMap): void { + const json = JSON.stringify(map); + if (process.platform === "darwin") { + entry.setPassword(Buffer.from(json).toString("base64")); + return; + } + if (process.platform === "win32") { + entry.setSecret(Buffer.from(json)); + return; + } + throw new Error(`Keyring is not supported on ${process.platform}`); + } +} diff --git a/src/login/loginCoordinator.ts b/src/login/loginCoordinator.ts index cc746df6..2c1f1b42 100644 --- a/src/login/loginCoordinator.ts +++ b/src/login/loginCoordinator.ts @@ -15,6 +15,7 @@ import type { User } from "coder/site/src/api/typesGenerated"; import type { MementoManager } from "../core/mementoManager"; import type { OAuthTokenData, SecretsManager } from "../core/secretsManager"; import type { Deployment } from "../deployment/types"; +import type { KeyringStore } from "../keyringStore"; import type { Logger } from "../logging/logger"; type LoginResult = @@ -39,6 +40,7 @@ export class LoginCoordinator implements vscode.Disposable { private readonly secretsManager: SecretsManager, private readonly mementoManager: MementoManager, private readonly logger: Logger, + private readonly keyringStore: KeyringStore, extensionId: string, ) { this.oauthAuthorizer = new OAuthAuthorizer( @@ -211,11 +213,13 @@ export class LoginCoordinator implements vscode.Disposable { // mTLS authentication (no token needed) if (!needToken(vscode.workspace.getConfiguration())) { + this.logger.debug("Attempting mTLS authentication (no token required)"); return this.tryMtlsAuth(client, isAutoLogin); } // Try provided token first if (providedToken) { + this.logger.debug("Trying provided token"); const result = await this.tryTokenAuth( client, providedToken, @@ -231,12 +235,32 @@ export class LoginCoordinator implements vscode.Disposable { deployment.safeHostname, ); if (auth?.token && auth.token !== providedToken) { + this.logger.debug("Trying stored session token"); const result = await this.tryTokenAuth(client, auth.token, isAutoLogin); if (result !== "unauthorized") { return result; } } + // Try keyring token (picks up tokens written by `coder login` in the terminal) + let keyringToken: string | undefined; + try { + keyringToken = this.keyringStore.getToken(deployment.safeHostname); + } catch (error) { + this.logger.warn("Failed to read token from keyring", error); + } + if ( + keyringToken && + keyringToken !== providedToken && + keyringToken !== auth?.token + ) { + this.logger.debug("Trying token from OS keyring"); + const result = await this.tryTokenAuth(client, keyringToken, isAutoLogin); + if (result !== "unauthorized") { + return result; + } + } + // Prompt user for token const authMethod = await maybeAskAuthMethod(client); switch (authMethod) { diff --git a/src/remote/remote.ts b/src/remote/remote.ts index c51f53db..3018e7dc 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -21,7 +21,13 @@ import { extractAgents } from "../api/api-helper"; import { AuthInterceptor } from "../api/authInterceptor"; import { CoderApi } from "../api/coderApi"; import { needToken } from "../api/utils"; -import { getGlobalFlags, getGlobalFlagsRaw, getSshFlags } from "../cliConfig"; +import { + type CliAuth, + getGlobalFlags, + getGlobalFlagsRaw, + getSshFlags, + resolveCliAuth, +} from "../cliConfig"; import { type Commands } from "../commands"; import { watchConfigurationChanges } from "../configWatcher"; import { type CliManager } from "../core/cliManager"; @@ -119,11 +125,6 @@ export class Remote { hasUrl: Boolean(baseUrlRaw), hasToken: token !== undefined, }); - // Empty token is valid for mTLS - if (baseUrlRaw && token !== undefined) { - await this.cliManager.configure(parts.safeHostname, baseUrlRaw, token); - } - const disposables: vscode.Disposable[] = []; try { @@ -175,7 +176,7 @@ export class Remote { } }; - // It could be that the cli config was deleted. If so, ask for the url. + // It could be that the cli config was deleted. If so, ask for the url. if ( !baseUrlRaw || (!token && needToken(vscode.workspace.getConfiguration())) @@ -210,26 +211,6 @@ export class Remote { // Store for use in commands. this.commands.remoteWorkspaceClient = workspaceClient; - // Listen for token changes for this deployment - disposables.push( - this.secretsManager.onDidChangeSessionAuth( - parts.safeHostname, - async (auth) => { - workspaceClient.setCredentials(auth?.url, auth?.token); - if (auth?.url) { - await this.cliManager.configure( - parts.safeHostname, - auth.url, - auth.token, - ); - this.logger.info( - "Updated CLI config with new token for remote deployment", - ); - } - }, - ), - ); - let binaryPath: string | undefined; if ( this.extensionContext.extensionMode === vscode.ExtensionMode.Production @@ -264,6 +245,42 @@ export class Remote { const featureSet = featureSetForVersion(version); + // Write token to keyring or file (after CLI version is known) + if (baseUrlRaw && token !== undefined) { + await this.cliManager.configure( + parts.safeHostname, + baseUrlRaw, + token, + featureSet, + ); + } + + // Listen for token changes for this deployment + disposables.push( + this.secretsManager.onDidChangeSessionAuth( + parts.safeHostname, + async (auth) => { + workspaceClient.setCredentials(auth?.url, auth?.token); + if (auth?.url) { + await this.cliManager.configure( + parts.safeHostname, + auth.url, + auth.token, + featureSet, + ); + this.logger.info( + "Updated CLI config with new token for remote deployment", + ); + } + }, + ), + ); + + const configDir = this.pathResolver.getGlobalConfigDir( + parts.safeHostname, + ); + const cliAuth = resolveCliAuth(featureSet, baseUrlRaw, configDir); + // Server versions before v0.14.1 don't support the vscodessh command! if (!featureSet.vscodessh) { await vscodeProposed.window.showErrorMessage( @@ -361,7 +378,7 @@ export class Remote { binaryPath, featureSet, this.logger, - this.pathResolver, + cliAuth, ); disposables.push(stateMachine); @@ -541,6 +558,7 @@ export class Remote { binaryPath, logDir, featureSet, + cliAuth, ); } catch (error) { this.logger.warn("Failed to configure SSH", error); @@ -715,14 +733,12 @@ export class Remote { hostPrefix: string, logDir: string, useWildcardSSH: boolean, + cliAuth: CliAuth, ): Promise { const vscodeConfig = vscode.workspace.getConfiguration(); const escapedBinaryPath = escapeCommandArg(binaryPath); - const globalConfig = getGlobalFlags( - vscodeConfig, - this.pathResolver.getGlobalConfigDir(label), - ); + const globalConfig = getGlobalFlags(vscodeConfig, cliAuth); const logArgs = await this.getLogArgs(logDir); if (useWildcardSSH) { @@ -789,6 +805,7 @@ export class Remote { binaryPath: string, logDir: string, featureSet: FeatureSet, + cliAuth: CliAuth, ) { let deploymentSSHConfig = {}; try { @@ -845,6 +862,7 @@ export class Remote { hostPrefix, logDir, featureSet.wildcardSSH, + cliAuth, ); const sshValues: SSHValues = { diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index e188132d..26b8ffba 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -19,7 +19,7 @@ import type { import type * as vscode from "vscode"; import type { CoderApi } from "../api/coderApi"; -import type { PathResolver } from "../core/pathResolver"; +import type { CliAuth } from "../cliConfig"; import type { FeatureSet } from "../featureSet"; import type { Logger } from "../logging/logger"; @@ -41,7 +41,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { private readonly binaryPath: string, private readonly featureSet: FeatureSet, private readonly logger: Logger, - private readonly pathResolver: PathResolver, + private readonly cliAuth: CliAuth, ) { this.terminal = new TerminalSession("Workspace Build"); } @@ -71,12 +71,9 @@ export class WorkspaceStateMachine implements vscode.Disposable { progress.report({ message: `starting ${workspaceName}...` }); this.logger.info(`Starting ${workspaceName}`); - const globalConfigDir = this.pathResolver.getGlobalConfigDir( - this.parts.safeHostname, - ); await startWorkspaceIfStoppedOrFailed( this.workspaceClient, - globalConfigDir, + this.cliAuth, this.binaryPath, workspace, this.terminal.writeEmitter, diff --git a/test/mocks/testHelpers.ts b/test/mocks/testHelpers.ts index 6d1af77a..e8f97f42 100644 --- a/test/mocks/testHelpers.ts +++ b/test/mocks/testHelpers.ts @@ -12,6 +12,7 @@ import type { User } from "coder/site/src/api/typesGenerated"; import type { IncomingMessage } from "node:http"; import type { CoderApi } from "@/api/coderApi"; +import type { KeyringStore } from "@/keyringStore"; import type { Logger } from "@/logging/logger"; /** @@ -375,6 +376,14 @@ export class InMemorySecretStorage implements vscode.SecretStorage { } } +export function createMockKeyringStore(): KeyringStore { + return { + setToken: vi.fn(), + getToken: vi.fn().mockReturnValue(undefined), + deleteToken: vi.fn(), + } as unknown as KeyringStore; +} + export function createMockLogger(): Logger { return { trace: vi.fn(), diff --git a/test/unit/cliConfig.test.ts b/test/unit/cliConfig.test.ts index e35fa687..7f0ecdf3 100644 --- a/test/unit/cliConfig.test.ts +++ b/test/unit/cliConfig.test.ts @@ -1,21 +1,48 @@ -import { it, expect, describe } from "vitest"; +import * as semver from "semver"; +import { afterEach, beforeEach, it, expect, describe, vi } from "vitest"; -import { getGlobalFlags, getGlobalFlagsRaw, getSshFlags } from "@/cliConfig"; +import { + type CliAuth, + getGlobalFlags, + getGlobalFlagsRaw, + getSshFlags, + resolveCliAuth, + shouldUseKeyring, +} from "@/cliConfig"; +import { featureSetForVersion } from "@/featureSet"; import { MockConfigurationProvider } from "../mocks/testHelpers"; import { isWindows } from "../utils/platform"; +const globalConfigAuth: CliAuth = { + mode: "global-config", + configDir: "/config/dir", +}; + describe("cliConfig", () => { describe("getGlobalFlags", () => { it("should return global-config and header args when no global flags configured", () => { const config = new MockConfigurationProvider(); - expect(getGlobalFlags(config, "/config/dir")).toStrictEqual([ + expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ "--global-config", '"/config/dir"', ]); }); + it("should return --url when auth mode is url", () => { + const config = new MockConfigurationProvider(); + const urlAuth: CliAuth = { + mode: "url", + url: "https://dev.coder.com", + }; + + expect(getGlobalFlags(config, urlAuth)).toStrictEqual([ + "--url", + '"https://dev.coder.com"', + ]); + }); + it("should return global flags from config with global-config appended", () => { const config = new MockConfigurationProvider(); config.set("coder.globalFlags", [ @@ -23,7 +50,7 @@ describe("cliConfig", () => { "--disable-direct-connections", ]); - expect(getGlobalFlags(config, "/config/dir")).toStrictEqual([ + expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ "--verbose", "--disable-direct-connections", "--global-config", @@ -39,7 +66,7 @@ describe("cliConfig", () => { "--disable-direct-connections", ]); - expect(getGlobalFlags(config, "/config/dir")).toStrictEqual([ + expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ "-v", "--global-config /path/to/ignored", "--disable-direct-connections", @@ -58,7 +85,7 @@ describe("cliConfig", () => { "--no-feature-warning", ]); - const result = getGlobalFlags(config, "/config/dir"); + const result = getGlobalFlags(config, globalConfigAuth); expect(result).toStrictEqual([ "-v", "--header-command custom", // ignored by CLI @@ -69,6 +96,24 @@ describe("cliConfig", () => { quoteCommand(headerCommand), ]); }); + + it("should include --url with header args when using url mode", () => { + const headerCommand = "echo test"; + const config = new MockConfigurationProvider(); + config.set("coder.headerCommand", headerCommand); + const urlAuth: CliAuth = { + mode: "url", + url: "https://dev.coder.com", + }; + + const result = getGlobalFlags(config, urlAuth); + expect(result).toStrictEqual([ + "--url", + '"https://dev.coder.com"', + "--header-command", + quoteCommand(headerCommand), + ]); + }); }); describe("getGlobalFlagsRaw", () => { @@ -115,6 +160,125 @@ describe("cliConfig", () => { ]); }); }); + + describe("shouldUseKeyring", () => { + let originalPlatform: NodeJS.Platform; + + beforeEach(() => { + originalPlatform = process.platform; + }); + + afterEach(() => { + vi.stubGlobal("process", { ...process, platform: originalPlatform }); + vi.unstubAllGlobals(); + }); + + it("returns true when all conditions are met (macOS, keyringAuth, setting enabled)", () => { + vi.stubGlobal("process", { ...process, platform: "darwin" }); + const config = new MockConfigurationProvider(); + config.set("coder.useKeyring", true); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + expect(shouldUseKeyring(featureSet)).toBe(true); + }); + + it("returns true when all conditions are met (Windows)", () => { + vi.stubGlobal("process", { ...process, platform: "win32" }); + const config = new MockConfigurationProvider(); + config.set("coder.useKeyring", true); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + expect(shouldUseKeyring(featureSet)).toBe(true); + }); + + it("returns false on Linux", () => { + vi.stubGlobal("process", { ...process, platform: "linux" }); + const config = new MockConfigurationProvider(); + config.set("coder.useKeyring", true); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + expect(shouldUseKeyring(featureSet)).toBe(false); + }); + + it("returns false when CLI version is too old", () => { + vi.stubGlobal("process", { ...process, platform: "darwin" }); + const config = new MockConfigurationProvider(); + config.set("coder.useKeyring", true); + const featureSet = featureSetForVersion(semver.parse("2.28.0")); + expect(shouldUseKeyring(featureSet)).toBe(false); + }); + + it("returns false when setting is disabled", () => { + vi.stubGlobal("process", { ...process, platform: "darwin" }); + const config = new MockConfigurationProvider(); + config.set("coder.useKeyring", false); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + expect(shouldUseKeyring(featureSet)).toBe(false); + }); + + it("returns true for devel prerelease on macOS", () => { + vi.stubGlobal("process", { ...process, platform: "darwin" }); + const config = new MockConfigurationProvider(); + config.set("coder.useKeyring", true); + const featureSet = featureSetForVersion( + semver.parse("0.0.0-devel+abc123"), + ); + expect(shouldUseKeyring(featureSet)).toBe(true); + }); + }); + + describe("resolveCliAuth", () => { + let originalPlatform: NodeJS.Platform; + + beforeEach(() => { + originalPlatform = process.platform; + }); + + afterEach(() => { + vi.stubGlobal("process", { ...process, platform: originalPlatform }); + vi.unstubAllGlobals(); + }); + + it("returns url mode when keyring should be used", () => { + vi.stubGlobal("process", { ...process, platform: "darwin" }); + const config = new MockConfigurationProvider(); + config.set("coder.useKeyring", true); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + const auth = resolveCliAuth( + featureSet, + "https://dev.coder.com", + "/config/dir", + ); + expect(auth).toEqual({ + mode: "url", + url: "https://dev.coder.com", + }); + }); + + it("returns global-config mode when keyring should not be used", () => { + vi.stubGlobal("process", { ...process, platform: "linux" }); + new MockConfigurationProvider(); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + const auth = resolveCliAuth( + featureSet, + "https://dev.coder.com", + "/config/dir", + ); + expect(auth).toEqual({ + mode: "global-config", + configDir: "/config/dir", + }); + }); + + it("returns global-config mode when url is undefined", () => { + vi.stubGlobal("process", { ...process, platform: "darwin" }); + const config = new MockConfigurationProvider(); + config.set("coder.useKeyring", true); + const featureSet = featureSetForVersion(semver.parse("2.29.0")); + const auth = resolveCliAuth(featureSet, undefined, "/config/dir"); + expect(auth).toEqual({ + mode: "global-config", + configDir: "/config/dir", + }); + }); + }); }); function quoteCommand(value: string): string { diff --git a/test/unit/core/cliManager.concurrent.test.ts b/test/unit/core/cliManager.concurrent.test.ts index 96ca3529..5d32f0a2 100644 --- a/test/unit/core/cliManager.concurrent.test.ts +++ b/test/unit/core/cliManager.concurrent.test.ts @@ -18,6 +18,7 @@ import { PathResolver } from "@/core/pathResolver"; import * as pgp from "@/pgp"; import { + createMockKeyringStore, createMockLogger, createMockStream, MockConfigurationProvider, @@ -82,6 +83,7 @@ function setupManager(testDir: string): CliManager { return new CliManager( createMockLogger(), new PathResolver(testDir, "/code/log"), + createMockKeyringStore(), ); } diff --git a/test/unit/core/cliManager.test.ts b/test/unit/core/cliManager.test.ts index e037a0e0..27e8f1d7 100644 --- a/test/unit/core/cliManager.test.ts +++ b/test/unit/core/cliManager.test.ts @@ -9,12 +9,15 @@ import * as path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import * as vscode from "vscode"; +import * as cliConfig from "@/cliConfig"; import { CliManager } from "@/core/cliManager"; import * as cliUtils from "@/core/cliUtils"; import { PathResolver } from "@/core/pathResolver"; +import { isKeyringSupported } from "@/keyringStore"; import * as pgp from "@/pgp"; import { + createMockKeyringStore, createMockLogger, createMockStream, MockConfigurationProvider, @@ -23,6 +26,8 @@ import { } from "../../mocks/testHelpers"; import { expectPathsEqual } from "../../utils/platform"; +import type { FeatureSet } from "@/featureSet"; + vi.mock("os"); vi.mock("axios"); @@ -48,6 +53,18 @@ vi.mock("proper-lockfile", () => ({ check: () => Promise.resolve(false), })); +vi.mock("@/cliConfig", async () => { + const actual = + await vi.importActual("@/cliConfig"); + return { ...actual, shouldUseKeyring: vi.fn() }; +}); + +vi.mock("@/keyringStore", async () => { + const actual = + await vi.importActual("@/keyringStore"); + return { ...actual, isKeyringSupported: vi.fn().mockReturnValue(true) }; +}); + vi.mock("@/pgp"); vi.mock("@/vscodeProposed", () => ({ @@ -71,6 +88,7 @@ describe("CliManager", () => { let mockUI: MockUserInteraction; let mockApi: Api; let mockAxios: AxiosInstance; + let mockKeyring: ReturnType; const TEST_VERSION = "1.2.3"; const TEST_URL = "https://test.coder.com"; @@ -80,6 +98,13 @@ describe("CliManager", () => { const ARCH = "amd64"; const BINARY_NAME = `coder-${PLATFORM}-${ARCH}`; const BINARY_PATH = `${BINARY_DIR}/${BINARY_NAME}`; + const MOCK_FEATURE_SET: FeatureSet = { + vscodessh: true, + proxyLogDirectory: true, + wildcardSSH: true, + buildReason: true, + keyringAuth: true, + }; beforeEach(() => { vi.resetAllMocks(); @@ -92,15 +117,18 @@ describe("CliManager", () => { mockConfig = new MockConfigurationProvider(); mockProgress = new MockProgressReporter(); mockUI = new MockUserInteraction(); + mockKeyring = createMockKeyringStore(); manager = new CliManager( createMockLogger(), new PathResolver(BASE_PATH, "/code/log"), + mockKeyring, ); // Mock only what's necessary vi.mocked(os.platform).mockReturnValue(PLATFORM); vi.mocked(os.arch).mockReturnValue(ARCH); vi.mocked(pgp.readPublicKeys).mockResolvedValue([]); + vi.mocked(isKeyringSupported).mockReturnValue(true); }); afterEach(async () => { @@ -117,6 +145,7 @@ describe("CliManager", () => { "deployment", "https://coder.example.com", "test-token", + MOCK_FEATURE_SET, ); expect(memfs.readFileSync("/path/base/deployment/url", "utf8")).toBe( @@ -128,7 +157,12 @@ describe("CliManager", () => { }); it("should skip URL when undefined but write token", async () => { - await manager.configure("deployment", undefined, "test-token"); + await manager.configure( + "deployment", + undefined, + "test-token", + MOCK_FEATURE_SET, + ); // No entry for the url expect(memfs.existsSync("/path/base/deployment/url")).toBe(false); @@ -138,7 +172,12 @@ describe("CliManager", () => { }); it("should skip token when null but write URL", async () => { - await manager.configure("deployment", "https://coder.example.com", null); + await manager.configure( + "deployment", + "https://coder.example.com", + null, + MOCK_FEATURE_SET, + ); // No entry for the session expect(memfs.readFileSync("/path/base/deployment/url", "utf8")).toBe( @@ -148,7 +187,12 @@ describe("CliManager", () => { }); it("should write empty string for token when provided", async () => { - await manager.configure("deployment", "https://coder.example.com", ""); + await manager.configure( + "deployment", + "https://coder.example.com", + "", + MOCK_FEATURE_SET, + ); expect(memfs.readFileSync("/path/base/deployment/url", "utf8")).toBe( "https://coder.example.com", @@ -159,13 +203,141 @@ describe("CliManager", () => { }); it("should use base path directly when label is empty", async () => { - await manager.configure("", "https://coder.example.com", "token"); + await manager.configure( + "", + "https://coder.example.com", + "token", + MOCK_FEATURE_SET, + ); expect(memfs.readFileSync("/path/base/url", "utf8")).toBe( "https://coder.example.com", ); expect(memfs.readFileSync("/path/base/session", "utf8")).toBe("token"); }); + + it("should write to keyring and skip files when featureSet enables keyring", async () => { + vi.mocked(cliConfig.shouldUseKeyring).mockReturnValue(true); + + await manager.configure( + "deployment", + "https://coder.example.com", + "test-token", + MOCK_FEATURE_SET, + ); + + expect(mockKeyring.setToken).toHaveBeenCalledWith( + "https://coder.example.com", + "test-token", + ); + expect(memfs.existsSync("/path/base/deployment/url")).toBe(false); + expect(memfs.existsSync("/path/base/deployment/session")).toBe(false); + }); + + it("should fall back to files when keyring write fails", async () => { + vi.mocked(cliConfig.shouldUseKeyring).mockReturnValue(true); + vi.mocked(mockKeyring.setToken).mockImplementation(() => { + throw new Error("keyring unavailable"); + }); + + await manager.configure( + "deployment", + "https://coder.example.com", + "test-token", + MOCK_FEATURE_SET, + ); + + expect(memfs.readFileSync("/path/base/deployment/url", "utf8")).toBe( + "https://coder.example.com", + ); + expect(memfs.readFileSync("/path/base/deployment/session", "utf8")).toBe( + "test-token", + ); + }); + + it("should fall back to files when url is undefined even with featureSet", async () => { + vi.mocked(cliConfig.shouldUseKeyring).mockReturnValue(true); + + await manager.configure( + "deployment", + undefined, + "test-token", + MOCK_FEATURE_SET, + ); + + expect(memfs.existsSync("/path/base/deployment/url")).toBe(false); + expect(memfs.readFileSync("/path/base/deployment/session", "utf8")).toBe( + "test-token", + ); + }); + + it("should fall back to files when token is null even with featureSet", async () => { + vi.mocked(cliConfig.shouldUseKeyring).mockReturnValue(true); + + await manager.configure( + "deployment", + "https://coder.example.com", + null, + MOCK_FEATURE_SET, + ); + + expect(memfs.readFileSync("/path/base/deployment/url", "utf8")).toBe( + "https://coder.example.com", + ); + expect(memfs.existsSync("/path/base/deployment/session")).toBe(false); + }); + }); + + describe("Clear Credentials", () => { + it("should delete keyring token and remove files", async () => { + // Pre-create credential files + memfs.mkdirSync("/path/base/deployment", { recursive: true }); + memfs.writeFileSync("/path/base/deployment/session", "old-token"); + memfs.writeFileSync("/path/base/deployment/url", "https://example.com"); + + await manager.clearCredentials("deployment"); + + expect(mockKeyring.deleteToken).toHaveBeenCalledWith("deployment"); + expect(memfs.existsSync("/path/base/deployment/session")).toBe(false); + expect(memfs.existsSync("/path/base/deployment/url")).toBe(false); + }); + + it("should continue file cleanup even when keyring delete fails", async () => { + memfs.mkdirSync("/path/base/deployment", { recursive: true }); + memfs.writeFileSync("/path/base/deployment/session", "old-token"); + memfs.writeFileSync("/path/base/deployment/url", "https://example.com"); + + vi.mocked(mockKeyring.deleteToken).mockImplementation(() => { + throw new Error("keyring unavailable"); + }); + + await manager.clearCredentials("deployment"); + + // Files should still be cleaned up despite keyring failure + expect(memfs.existsSync("/path/base/deployment/session")).toBe(false); + expect(memfs.existsSync("/path/base/deployment/url")).toBe(false); + }); + + it("should not throw when files don't exist", async () => { + await expect( + manager.clearCredentials("deployment"), + ).resolves.not.toThrow(); + + expect(mockKeyring.deleteToken).toHaveBeenCalledWith("deployment"); + }); + + it("should skip keyring delete on unsupported platforms", async () => { + vi.mocked(isKeyringSupported).mockReturnValue(false); + memfs.mkdirSync("/path/base/deployment", { recursive: true }); + memfs.writeFileSync("/path/base/deployment/session", "old-token"); + memfs.writeFileSync("/path/base/deployment/url", "https://example.com"); + + await manager.clearCredentials("deployment"); + + expect(mockKeyring.deleteToken).not.toHaveBeenCalled(); + expect(memfs.existsSync("/path/base/deployment/session")).toBe(false); + expect(memfs.existsSync("/path/base/deployment/url")).toBe(false); + }); }); describe("Binary Version Validation", () => { @@ -578,7 +750,11 @@ describe("CliManager", () => { it("handles binary with spaces in path", async () => { const pathWithSpaces = "/path with spaces/bin"; const resolver = new PathResolver(pathWithSpaces, "/log"); - const manager = new CliManager(createMockLogger(), resolver); + const manager = new CliManager( + createMockLogger(), + resolver, + createMockKeyringStore(), + ); withSuccessfulDownload(); const result = await manager.fetchBinary(mockApi, "test label"); diff --git a/test/unit/featureSet.test.ts b/test/unit/featureSet.test.ts index 919f7089..b8dfad08 100644 --- a/test/unit/featureSet.test.ts +++ b/test/unit/featureSet.test.ts @@ -28,4 +28,16 @@ describe("check version support", () => { }, ); }); + it("keyring auth", () => { + ["v2.28.0", "v2.28.9", "v1.0.0", "v2.3.3+e491217"].forEach((v: string) => { + expect(featureSetForVersion(semver.parse(v)).keyringAuth).toBeFalsy(); + }); + ["v2.29.0", "v2.29.1", "v2.30.0", "v3.0.0"].forEach((v: string) => { + expect(featureSetForVersion(semver.parse(v)).keyringAuth).toBeTruthy(); + }); + // devel prerelease should enable keyring + expect( + featureSetForVersion(semver.parse("0.0.0-devel+abc123")).keyringAuth, + ).toBeTruthy(); + }); }); diff --git a/test/unit/keyringStore.test.ts b/test/unit/keyringStore.test.ts new file mode 100644 index 00000000..61106451 --- /dev/null +++ b/test/unit/keyringStore.test.ts @@ -0,0 +1,207 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + type KeyringEntry, + KeyringStore, + isKeyringSupported, +} from "@/keyringStore"; + +import { createMockLogger } from "../mocks/testHelpers"; + +/** + * In-memory backing store that simulates the OS keyring. + * Each call to `factory()` returns a fresh handle pointing to the same + * shared state — matching real @napi-rs/keyring behavior where + * `Entry.withTarget()` returns a new handle to the same credential. + */ +function createMockEntryFactory() { + let password: string | null = null; + let secret: Uint8Array | null = null; + + return { + factory: (): KeyringEntry => ({ + getPassword: () => password, + setPassword: (p: string) => { + password = p; + }, + getSecret: () => secret, + setSecret: (s: Uint8Array) => { + secret = s; + }, + deleteCredential: () => { + password = null; + secret = null; + }, + }), + getRawPassword: () => password, + getRawSecret: () => secret, + hasCredential: () => password !== null || secret !== null, + }; +} + +function stubPlatform(platform: string) { + vi.stubGlobal("process", { ...process, platform }); +} + +describe("isKeyringSupported", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it.each([ + { platform: "darwin", expected: true }, + { platform: "win32", expected: true }, + { platform: "linux", expected: false }, + { platform: "freebsd", expected: false }, + ])("returns $expected for $platform", ({ platform, expected }) => { + stubPlatform(platform); + expect(isKeyringSupported()).toBe(expected); + }); +}); + +describe("KeyringStore", () => { + let store: KeyringStore; + let mockEntry: ReturnType; + + beforeEach(() => { + mockEntry = createMockEntryFactory(); + store = new KeyringStore(createMockLogger(), mockEntry.factory); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + // CRUD behavior is platform-independent; darwin is used as the test platform. + describe("token operations", () => { + beforeEach(() => { + stubPlatform("darwin"); + }); + + it("sets and gets a token", () => { + store.setToken("https://dev.coder.com", "my-token"); + expect(store.getToken("dev.coder.com")).toBe("my-token"); + }); + + it("overwrites token for same deployment", () => { + store.setToken("https://dev.coder.com", "old-token"); + store.setToken("https://dev.coder.com", "new-token"); + expect(store.getToken("dev.coder.com")).toBe("new-token"); + }); + + it("preserves other deployments on set", () => { + store.setToken("https://dev.coder.com", "token-1"); + store.setToken("https://staging.coder.com", "token-2"); + + expect(store.getToken("dev.coder.com")).toBe("token-1"); + expect(store.getToken("staging.coder.com")).toBe("token-2"); + }); + + it("returns undefined for missing deployment", () => { + expect(store.getToken("unknown.coder.com")).toBeUndefined(); + }); + + it("deletes token while preserving others", () => { + store.setToken("https://dev.coder.com", "token-1"); + store.setToken("https://staging.coder.com", "token-2"); + + store.deleteToken("dev.coder.com"); + + expect(store.getToken("dev.coder.com")).toBeUndefined(); + expect(store.getToken("staging.coder.com")).toBe("token-2"); + }); + + it("deletes entire credential when last token is removed", () => { + store.setToken("https://dev.coder.com", "token-1"); + store.deleteToken("dev.coder.com"); + expect(store.getToken("dev.coder.com")).toBeUndefined(); + // OS keyring entry itself should be cleaned up, not left as empty JSON + expect(mockEntry.hasCredential()).toBe(false); + }); + + it("handles delete of non-existent deployment gracefully", () => { + store.setToken("https://dev.coder.com", "token-1"); + store.deleteToken("unknown.coder.com"); + expect(store.getToken("dev.coder.com")).toBe("token-1"); + }); + + it("strips URL path and protocol, keeping only host", () => { + store.setToken("https://dev.coder.com/some/path", "my-token"); + expect(store.getToken("dev.coder.com")).toBe("my-token"); + }); + + it("matches safeHostname to map key with port", () => { + store.setToken("https://dev.coder.com:3000", "my-token"); + // safeHostname (port stripped) finds the entry stored with port + expect(store.getToken("dev.coder.com")).toBe("my-token"); + + store.deleteToken("dev.coder.com"); + expect(store.getToken("dev.coder.com")).toBeUndefined(); + expect(mockEntry.hasCredential()).toBe(false); + }); + }); + + describe("platform encoding", () => { + it("macOS: base64-encoded JSON via password", () => { + stubPlatform("darwin"); + store.setToken("https://dev.coder.com", "token"); + const decoded = JSON.parse( + Buffer.from(mockEntry.getRawPassword()!, "base64").toString("utf-8"), + ); + expect(decoded).toEqual({ + "dev.coder.com": { coder_url: "dev.coder.com", api_token: "token" }, + }); + }); + + it("macOS: returns undefined for corrupted credential", () => { + stubPlatform("darwin"); + store.setToken("https://dev.coder.com", "token"); + mockEntry + .factory() + .setPassword(Buffer.from("not-valid-json").toString("base64")); + expect(store.getToken("dev.coder.com")).toBeUndefined(); + }); + + it("Windows: raw UTF-8 JSON via secret", () => { + stubPlatform("win32"); + store.setToken("https://dev.coder.com", "token"); + const decoded = JSON.parse( + Buffer.from(mockEntry.getRawSecret()!).toString("utf-8"), + ); + expect(decoded).toEqual({ + "dev.coder.com": { coder_url: "dev.coder.com", api_token: "token" }, + }); + // Also verify the win32 read path round-trips + expect(store.getToken("dev.coder.com")).toBe("token"); + }); + + it("Windows: returns undefined for corrupted credential", () => { + stubPlatform("win32"); + store.setToken("https://dev.coder.com", "token"); + mockEntry.factory().setSecret(Buffer.from("not-valid-json")); + expect(store.getToken("dev.coder.com")).toBeUndefined(); + }); + + it("preserves port in map key for CLI compatibility", () => { + stubPlatform("darwin"); + store.setToken("https://dev.coder.com:8080", "my-token"); + const decoded = JSON.parse( + Buffer.from(mockEntry.getRawPassword()!, "base64").toString("utf-8"), + ); + expect(decoded).toEqual({ + "dev.coder.com:8080": { + coder_url: "dev.coder.com:8080", + api_token: "my-token", + }, + }); + }); + + it("throws on unsupported platform", () => { + stubPlatform("linux"); + const msg = "Keyring is not supported on linux"; + expect(() => store.setToken("https://dev.coder.com", "t")).toThrow(msg); + expect(() => store.getToken("dev.coder.com")).toThrow(msg); + expect(() => store.deleteToken("dev.coder.com")).toThrow(msg); + }); + }); +}); diff --git a/test/unit/login/loginCoordinator.test.ts b/test/unit/login/loginCoordinator.test.ts index 8b850a3b..7d7a345f 100644 --- a/test/unit/login/loginCoordinator.test.ts +++ b/test/unit/login/loginCoordinator.test.ts @@ -10,6 +10,7 @@ import { maybeAskAuthMethod } from "@/promptUtils"; import { createAxiosError, + createMockKeyringStore, createMockLogger, createMockUser, InMemoryMemento, @@ -121,6 +122,7 @@ function createTestContext() { secretsManager, mementoManager, logger, + createMockKeyringStore(), "coder.coder-remote", ); @@ -309,6 +311,7 @@ describe("LoginCoordinator", () => { secretsManager, mementoManager, logger, + createMockKeyringStore(), "coder.coder-remote", );