Skip to content

Commit 7d85c05

Browse files
authored
feat: make keyring opt-in and store token at login (#835)
- Default `coder.useKeyring` to false (opt-in) due to shared logout side effects between CLI and IDE - Update setting description to document sync behavior and version requirements - Store token to OS keyring at login time so the CLI picks it up immediately - Show cancellable progress notification for all CLI keyring invocations during login - Migrate `CliCredentialManager` public API to options bag pattern with signal and keyringOnly support Closes #836
1 parent 11911fc commit 7d85c05

File tree

13 files changed

+392
-68
lines changed

13 files changed

+392
-68
lines changed

CHANGELOG.md

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22

33
## Unreleased
44

5-
### Fixed
6-
7-
- Fixed SSH config writes failing on Windows when antivirus, cloud sync software,
8-
or another process briefly locks the file.
9-
105
### Added
116

127
- Automatically set `reconnectionGraceTime`, `serverShutdownTimeout`, and `maxReconnectionAttempts`
@@ -18,6 +13,18 @@
1813
- SSH options from `coder config-ssh --ssh-option` are now applied to VS Code connections,
1914
with priority order: VS Code setting > `coder config-ssh` options > deployment config.
2015

16+
### Fixed
17+
18+
- Fixed SSH config writes failing on Windows when antivirus, cloud sync software,
19+
or another process briefly locks the file.
20+
21+
### Changed
22+
23+
- `coder.useKeyring` is now opt-in (default: false). Keyring storage requires CLI >= 2.29.0 for
24+
storage and logout sync, and >= 2.31.0 for syncing login from CLI to VS Code.
25+
- Session tokens are now saved to the OS keyring at login time (when enabled and CLI >= 2.29.0),
26+
not only when connecting to a workspace.
27+
2128
## [v1.14.0-pre](https://github.com/coder/vscode-coder/releases/tag/v1.14.0-pre) 2026-03-06
2229

2330
### Added

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,9 @@
157157
}
158158
},
159159
"coder.useKeyring": {
160-
"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.",
160+
"markdownDescription": "Store session tokens in the OS keyring (macOS Keychain, Windows Credential Manager) instead of plaintext files. Requires CLI >= 2.29.0 (>= 2.31.0 to sync login from CLI to VS Code). This will attempt to sync between the CLI and VS Code since they share the same keyring entry. It will log you out of the CLI if you log out of the IDE, and vice versa. Has no effect on Linux.",
161161
"type": "boolean",
162-
"default": true
162+
"default": false
163163
},
164164
"coder.httpClientLogLevel": {
165165
"markdownDescription": "Controls the verbosity of HTTP client logging. This affects what details are logged for each HTTP request and response.",

src/cliConfig.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ function isFlag(item: string, name: string): boolean {
6868
export function isKeyringEnabled(
6969
configs: Pick<WorkspaceConfiguration, "get">,
7070
): boolean {
71-
return isKeyringSupported() && configs.get<boolean>("coder.useKeyring", true);
71+
return (
72+
isKeyringSupported() && configs.get<boolean>("coder.useKeyring", false)
73+
);
7274
}
7375

7476
/**

src/core/cliCredentialManager.ts

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import { execFile } from "node:child_process";
22
import fs from "node:fs/promises";
3+
import os from "node:os";
34
import path from "node:path";
45
import { promisify } from "node:util";
56
import * as semver from "semver";
67

78
import { isKeyringEnabled } from "../cliConfig";
9+
import { isAbortError } from "../error/errorUtils";
810
import { featureSetForVersion } from "../featureSet";
911
import { getHeaderArgs } from "../headers";
1012
import { renameWithRetry, tempFilePath, toSafeHost } from "../util";
@@ -34,7 +36,8 @@ export type BinaryResolver = (deploymentUrl: string) => Promise<string>;
3436
* Returns true on platforms where the OS keyring is supported (macOS, Windows).
3537
*/
3638
export function isKeyringSupported(): boolean {
37-
return process.platform === "darwin" || process.platform === "win32";
39+
const platform = os.platform();
40+
return platform === "darwin" || platform === "win32";
3841
}
3942

4043
/**
@@ -54,19 +57,25 @@ export class CliCredentialManager {
5457
* files under --global-config.
5558
*
5659
* Keyring and files are mutually exclusive — never both.
60+
*
61+
* When `keyringOnly` is set, silently returns if the keyring is unavailable
62+
* instead of falling back to file storage.
5763
*/
5864
public async storeToken(
5965
url: string,
6066
token: string,
6167
configs: Pick<WorkspaceConfiguration, "get">,
62-
signal?: AbortSignal,
68+
options?: { signal?: AbortSignal; keyringOnly?: boolean },
6369
): Promise<void> {
6470
const binPath = await this.resolveKeyringBinary(
6571
url,
6672
configs,
6773
"keyringAuth",
6874
);
6975
if (!binPath) {
76+
if (options?.keyringOnly) {
77+
return;
78+
}
7079
await this.writeCredentialFiles(url, token);
7180
return;
7281
}
@@ -80,7 +89,7 @@ export class CliCredentialManager {
8089
try {
8190
await this.execWithTimeout(binPath, args, {
8291
env: { ...process.env, CODER_SESSION_TOKEN: token },
83-
signal,
92+
signal: options?.signal,
8493
});
8594
this.logger.info("Stored token via CLI for", url);
8695
} catch (error) {
@@ -92,10 +101,12 @@ export class CliCredentialManager {
92101
/**
93102
* Read a token via `coder login token --url`. Returns trimmed stdout,
94103
* or undefined on any failure (resolver, CLI, empty output).
104+
* Throws AbortError when the signal is aborted.
95105
*/
96106
public async readToken(
97107
url: string,
98108
configs: Pick<WorkspaceConfiguration, "get">,
109+
options?: { signal?: AbortSignal },
99110
): Promise<string | undefined> {
100111
let binPath: string | undefined;
101112
try {
@@ -114,27 +125,33 @@ export class CliCredentialManager {
114125

115126
const args = [...getHeaderArgs(configs), "login", "token", "--url", url];
116127
try {
117-
const { stdout } = await this.execWithTimeout(binPath, args);
128+
const { stdout } = await this.execWithTimeout(binPath, args, {
129+
signal: options?.signal,
130+
});
118131
const token = stdout.trim();
119132
return token || undefined;
120133
} catch (error) {
134+
if (isAbortError(error)) {
135+
throw error;
136+
}
121137
this.logger.warn("Failed to read token via CLI:", error);
122138
return undefined;
123139
}
124140
}
125141

126142
/**
127143
* Delete credentials for a deployment. Runs file deletion and keyring
128-
* deletion in parallel, both best-effort (never throws).
144+
* deletion in parallel, both best-effort. Throws AbortError when the
145+
* signal is aborted.
129146
*/
130147
public async deleteToken(
131148
url: string,
132149
configs: Pick<WorkspaceConfiguration, "get">,
133-
signal?: AbortSignal,
150+
options?: { signal?: AbortSignal },
134151
): Promise<void> {
135152
await Promise.all([
136153
this.deleteCredentialFiles(url),
137-
this.deleteKeyringToken(url, configs, signal),
154+
this.deleteKeyringToken(url, configs, options?.signal),
138155
]);
139156
}
140157

@@ -241,6 +258,9 @@ export class CliCredentialManager {
241258
await this.execWithTimeout(binPath, args, { signal });
242259
this.logger.info("Deleted token via CLI for", url);
243260
} catch (error) {
261+
if (isAbortError(error)) {
262+
throw error;
263+
}
244264
this.logger.warn("Failed to delete token via CLI:", error);
245265
}
246266
}

src/core/cliManager.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ import * as semver from "semver";
1010
import * as vscode from "vscode";
1111

1212
import { errToStr } from "../api/api-helper";
13+
import { isKeyringEnabled } from "../cliConfig";
1314
import * as pgp from "../pgp";
14-
import { withCancellableProgress } from "../progress";
15+
import { withCancellableProgress, withOptionalProgress } from "../progress";
1516
import { tempFilePath, toSafeHost } from "../util";
1617
import { vscodeProposed } from "../vscodeProposed";
1718

@@ -759,13 +760,13 @@ export class CliManager {
759760
}
760761

761762
const result = await withCancellableProgress(
763+
({ signal }) =>
764+
this.cliCredentialManager.storeToken(url, token, configs, { signal }),
762765
{
763766
location: vscode.ProgressLocation.Notification,
764767
title: `Storing credentials for ${url}`,
765768
cancellable: true,
766769
},
767-
({ signal }) =>
768-
this.cliCredentialManager.storeToken(url, token, configs, signal),
769770
);
770771
if (result.ok) {
771772
return;
@@ -783,14 +784,15 @@ export class CliManager {
783784
*/
784785
public async clearCredentials(url: string): Promise<void> {
785786
const configs = vscode.workspace.getConfiguration();
786-
const result = await withCancellableProgress(
787+
const result = await withOptionalProgress(
788+
({ signal }) =>
789+
this.cliCredentialManager.deleteToken(url, configs, { signal }),
787790
{
791+
enabled: isKeyringEnabled(configs),
788792
location: vscode.ProgressLocation.Notification,
789793
title: `Removing credentials for ${url}`,
790794
cancellable: true,
791795
},
792-
({ signal }) =>
793-
this.cliCredentialManager.deleteToken(url, configs, signal),
794796
);
795797
if (result.ok) {
796798
return;

src/error/errorUtils.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
import { isApiError, isApiErrorResponse } from "coder/site/src/api/errors";
22
import util from "node:util";
33

4+
/**
5+
* Check whether an unknown thrown value is an AbortError (signal cancellation).
6+
*/
7+
export function isAbortError(error: unknown): boolean {
8+
return error instanceof Error && error.name === "AbortError";
9+
}
10+
411
// getErrorDetail is copied from coder/site, but changes the default return.
512
export const getErrorDetail = (error: unknown): string | undefined | null => {
613
if (isApiError(error)) {

src/login/loginCoordinator.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import * as vscode from "vscode";
44

55
import { CoderApi } from "../api/coderApi";
66
import { needToken } from "../api/utils";
7+
import { isKeyringEnabled } from "../cliConfig";
78
import { CertificateError } from "../error/certificateError";
89
import { OAuthAuthorizer } from "../oauth/authorizer";
910
import { buildOAuthTokenData } from "../oauth/utils";
11+
import { withOptionalProgress } from "../progress";
1012
import { maybeAskAuthMethod, maybeAskUrl } from "../promptUtils";
1113
import { vscodeProposed } from "../vscodeProposed";
1214

@@ -147,6 +149,20 @@ export class LoginCoordinator implements vscode.Disposable {
147149
oauth: result.oauth, // undefined for non-OAuth logins
148150
});
149151
await this.mementoManager.addToUrlHistory(url);
152+
153+
// Fire-and-forget: sync token to OS keyring for the CLI.
154+
if (result.token) {
155+
this.cliCredentialManager
156+
.storeToken(url, result.token, vscode.workspace.getConfiguration(), {
157+
keyringOnly: true,
158+
})
159+
.catch((error) => {
160+
this.logger.warn(
161+
"Failed to store token in keyring at login:",
162+
error,
163+
);
164+
});
165+
}
150166
}
151167
}
152168

@@ -243,10 +259,20 @@ export class LoginCoordinator implements vscode.Disposable {
243259
}
244260

245261
// Try keyring token (picks up tokens written by `coder login` in the terminal)
246-
const keyringToken = await this.cliCredentialManager.readToken(
247-
deployment.url,
248-
vscode.workspace.getConfiguration(),
262+
const configs = vscode.workspace.getConfiguration();
263+
const keyringResult = await withOptionalProgress(
264+
({ signal }) =>
265+
this.cliCredentialManager.readToken(deployment.url, configs, {
266+
signal,
267+
}),
268+
{
269+
enabled: isKeyringEnabled(configs),
270+
location: vscode.ProgressLocation.Notification,
271+
title: "Reading token from OS keyring...",
272+
cancellable: true,
273+
},
249274
);
275+
const keyringToken = keyringResult.ok ? keyringResult.value : undefined;
250276
if (
251277
keyringToken &&
252278
keyringToken !== providedToken &&

src/progress.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import * as vscode from "vscode";
22

3+
import { isAbortError } from "./error/errorUtils";
4+
35
export type ProgressResult<T> =
46
| { ok: true; value: T }
57
| { ok: false; cancelled: true }
@@ -17,8 +19,8 @@ export interface ProgressContext {
1719
* `{ cancelled: true }`.
1820
*/
1921
export function withCancellableProgress<T>(
20-
options: vscode.ProgressOptions & { cancellable: true },
2122
fn: (ctx: ProgressContext) => Promise<T>,
23+
options: vscode.ProgressOptions & { cancellable: true },
2224
): Thenable<ProgressResult<T>> {
2325
return vscode.window.withProgress(
2426
options,
@@ -29,7 +31,7 @@ export function withCancellableProgress<T>(
2931
const value = await fn({ progress, signal: ac.signal });
3032
return { ok: true, value };
3133
} catch (error) {
32-
if ((error as Error).name === "AbortError") {
34+
if (isAbortError(error)) {
3335
return { ok: false, cancelled: true };
3436
}
3537
return { ok: false, cancelled: false, error };
@@ -40,6 +42,35 @@ export function withCancellableProgress<T>(
4042
);
4143
}
4244

45+
/**
46+
* Like withCancellableProgress, but only shows the progress notification when
47+
* `enabled` is true. When false, runs the function directly without UI.
48+
* Returns ProgressResult<T> in both cases for uniform call-site handling.
49+
*/
50+
export async function withOptionalProgress<T>(
51+
fn: (ctx: ProgressContext) => Promise<T>,
52+
options: vscode.ProgressOptions & { cancellable: true; enabled: boolean },
53+
): Promise<ProgressResult<T>> {
54+
if (options.enabled) {
55+
return withCancellableProgress(fn, options);
56+
}
57+
try {
58+
const noop = () => {
59+
// No-op: progress reporting is disabled.
60+
};
61+
const value = await fn({
62+
progress: { report: noop },
63+
signal: new AbortController().signal,
64+
});
65+
return { ok: true, value };
66+
} catch (error) {
67+
if (isAbortError(error)) {
68+
return { ok: false, cancelled: true };
69+
}
70+
return { ok: false, cancelled: false, error };
71+
}
72+
}
73+
4374
/**
4475
* Run a task inside a VS Code progress notification (no cancellation).
4576
* A thin wrapper over `vscode.window.withProgress` that passes only the

0 commit comments

Comments
 (0)