Delegate keyring operations to CLI and simplify deployment identity by EhabY · Pull Request #816 · coder/vscode-coder

EhabY · 2026-03-03T16:47:53Z

Summary

Replace @napi-rs/keyring (4 native .node binaries) with CLI subprocess calls (coder login --use-token-as-session to store, coder login token --url to read, coder logout --url --yes to delete), keeping the credential format in sync with the CLI automatically
Make url the single source of truth for deployment identity — fetchBinary, configure, and clearCredentials now derive safeHostname internally via toSafeHost(), eliminating redundant parameters
Download the CLI if it does not exist (or out of date) when reading/deleting the session token on login/logout
Remove KeyringStore, vendor script, supportedArchitectures config, and @napi-rs/keyring dependency

Closes #825

src/core/cliCredentialManager.ts

src/core/container.ts

mtojek · 2026-03-05T20:48:38Z

@EhabY Feel free to request a review when this PR is ready 👍

ethanndickson

I'm a little confused about why we wait for the binary to finish downloading before prompting the user for the token. We already temporarily store the token in VS Code SecretStorage in the old implementation, so couldn't we just wait until connection time when we already download the binary to transfer it from SecretStorage and into the keychain via coder login?

Of course we'd need to validate the token on entry into VS Code the old way, just hitting /api/v2/user/me, but this seems like general improvement?

Also, I think trying a coder login token before we've downloaded anything, as I mentioned a few days ago, is still worth adding. Perhaps in a follow-up PR?

EhabY · 2026-03-06T08:15:45Z

We wait only after checking the passed token or the token stored in the secret storage. So essentially it's the last fallback incase there's a token in the OS keyring.

I suppose we could attempt to use coder login token globally before attempting the download, this would work for reading only, for writing or deleting (login/logout) we have to use the downloaded CLI since it might be using a different schema/key in the keychain (if we ever change the format, essentially, writing should use the same version)

On macOS and Windows with CLI >= 2.29.0, write session tokens to the OS keyring (Keychain / Credential Manager) instead of plaintext files. The CLI reads from the keyring when invoked with --url instead of --global-config. Falls back to file storage on Linux, or older CLIs. Key changes: - Harden file fallback with mode 0o600 - Add shouldUseKeyring() as single source of truth gating on CLI version, platform, and coder.useKeyring setting - Restructure remote.ts setup() to call configure() after featureSet is known, so the keyring decision can be made - Add keyring read fallback in LoginCoordinator for tokens written by `coder login` from the terminal - Add vendor-keyring.mjs build script to copy native binaries into dist/node_modules/ for VSIX packaging (vsce can't follow pnpm symlinks) - Add KeyringStore wrapping @napi-rs/keyring with the CLI's credential format (JSON map keyed by host, base64 on macOS, raw bytes on Windows)

Replace @napi-rs/keyring (4 native .node binaries) with CLI subprocess calls. The extension now runs `coder login --use-token-as-session` to store tokens and `coder login token --url` to read them, keeping the credential format in sync with the CLI automatically. - Add CliCredentialManager that shells out to the Coder CLI - Update CliManager.configure() to accept binPath and delegate to CLI - Update LoginCoordinator to fetch CLI binary for keyring reads - Remove clearCredentials keyring cleanup (stale entries are harmless) - Remove @napi-rs/keyring dep, vendor script, supportedArchitectures - Delete KeyringStore and its tests

…both Make url the single source of truth for deployment identity. fetchBinary, configure, and clearCredentials now derive safeHostname via toSafeHost() internally, eliminating redundant parameters and preventing inconsistency. BinaryResolver and CliCredentialManager methods take url string instead of Deployment object.

…er logging Make storeToken resolve its own binary internally via the injected BinaryResolver, matching readToken and deleteToken. Remove the binPath parameter from both storeToken and configure since callers no longer need to supply it. Add locateBinary to CliManager as a cheap stat-only lookup that the container resolver tries before falling back to fetchBinary. Downgrade implementation-detail log messages from info/warn to debug, keeping info for significant events (server version, download start, stored/deleted token).

These flags are managed by the extension and conflict with the auth mode it chooses. Passing --use-keyring=false with --url breaks keyring auth, and --global-config with --url causes the CLI to ignore the keyring entirely.

src/core/cliCredentialManager.ts

mtojek · 2026-03-06T12:48:47Z

src/core/cliCredentialManager.ts

+			url,
+		];
+		try {
+			await execFileAsync(binPath, args, {


should we define timeout for execFileAsync operations, just in case it hangs?

Add a timeout of 60s with logging every 5 seconds (also added a progress monitor)

src/core/cliCredentialManager.ts

src/core/cliManager.ts

src/core/container.ts

src/login/loginCoordinator.ts

src/cliConfig.ts

ethanndickson · 2026-03-06T13:42:05Z

I suppose we could attempt to use coder login token globally before attempting the download, this would work for reading only, for writing or deleting (login/logout) we have to use the downloaded CLI since it might be using a different schema/key in the keychain (if we ever change the format, essentially, writing should use the same version)

Yeah, I agree w.r.t to writing, but there's no harm in running it, worst case it returns nothing and we fall through.

We wait only after checking the passed token or the token stored in the secret storage. So essentially it's the last fallback incase there's a token in the OS keyring.

I'm curious why we don't just prompt straight away though - we could handle it such that there would be no change in UX from the existing flow. i.e:

Given: That the token is not found anywhere on system already (coder login token, secretstorage)
Then:

User is prompted for token
They enter it
We validate it against the user/me endpoint
We store it in SecretStorage
The user goes to connect to a workspace
We download the coder binary
Once downloaded, now we write to the keyring, since we have it in secretstorage
We finish connecting to the workspace

EhabY · 2026-03-06T14:13:44Z

Given: That the token is not found anywhere on system already (coder login token, secretstorage)

Yes but how would we call coder login token without having the CLI? I'm confused 🤔

Potentially we can call coder login token (globally) if it exists, this might help delay the download of the CLI till the connection to the workspace

ethanndickson · 2026-03-06T14:17:35Z

Given: That the token is not found anywhere on system already (coder login token, secretstorage)

Yes but how would we call coder login token without having the CLI? I'm confused 🤔

Yeah whatever coder is on the PATH, if any, it's worth checking coder login token for it before we ask the user to input one.

Potentially we can call coder login token (globally) if it exists, this might help delay the download of the CLI till the connection to the workspace

My question is more about why don't we just always delay the download of the CLI to make the UX a bit less awkward / retain the current UX.

EhabY · 2026-03-06T14:20:14Z

My question is more about why don't we just always delay the download of the CLI to make the UX a bit less awkward / retain the current UX.

Ah yes, this needs to be decided: Do we want to sync between CLI and VS Code ALWAYS or change the UX so that we always check beforehand?

ethanndickson · 2026-03-06T14:37:03Z

I'm not saying we shouldn't sync between CLI and VS Code, I'm just saying we can sync at connection time, by storing it in secretstorage in the interim until the CLI is downloaded, which was what we were doing in the now reverted PR.

EhabY · 2026-03-06T14:48:32Z

Yes but we are not syncing FROM the CLI in this case, that's what I meant. VS Code -> CLI will happen on connection time which is fine since the binary is downloaded anyway by that point.

mtojek

LGTM!

Show VS Code progress notifications with Cancel support during storeToken/deleteToken CLI calls. Thread AbortSignal through the credential manager and add a reusable withCancellableProgress util.

EhabY self-assigned this Mar 3, 2026

EhabY force-pushed the add-keyring-support-cli branch 3 times, most recently from 115f6cc to b852660 Compare March 4, 2026 13:42

EhabY changed the title ~~Add keyring support through the CLI~~ Delegate keyring operations to CLI and simplify deployment identity Mar 4, 2026

EhabY commented Mar 4, 2026

View reviewed changes

src/core/cliCredentialManager.ts Show resolved Hide resolved

src/core/cliCredentialManager.ts Outdated Show resolved Hide resolved

EhabY force-pushed the add-keyring-support-cli branch 3 times, most recently from 5fb0fed to 45e0f0b Compare March 5, 2026 12:03

EhabY commented Mar 5, 2026

View reviewed changes

src/core/container.ts Show resolved Hide resolved

ethanndickson reviewed Mar 6, 2026

View reviewed changes

EhabY added 5 commits March 6, 2026 11:53

EhabY force-pushed the add-keyring-support-cli branch from e38a36e to ac64e3e Compare March 6, 2026 08:54

EhabY requested a review from ethanndickson March 6, 2026 08:56

mtojek reviewed Mar 6, 2026

View reviewed changes

EhabY force-pushed the add-keyring-support-cli branch 2 times, most recently from b3235f5 to 102ee0e Compare March 6, 2026 15:36

EhabY requested a review from mtojek March 6, 2026 15:36

EhabY force-pushed the add-keyring-support-cli branch from 102ee0e to bfa58ff Compare March 6, 2026 15:39

mtojek approved these changes Mar 6, 2026

View reviewed changes

feat: add cancellable progress monitors for credential operations

37266b8

Show VS Code progress notifications with Cancel support during storeToken/deleteToken CLI calls. Thread AbortSignal through the credential manager and add a reusable withCancellableProgress util.

EhabY force-pushed the add-keyring-support-cli branch from bfa58ff to 37266b8 Compare March 6, 2026 18:12

EhabY merged commit 7b59373 into main Mar 6, 2026
6 checks passed

EhabY deleted the add-keyring-support-cli branch March 6, 2026 18:18

Conversation

EhabY commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mtojek commented Mar 5, 2026

Uh oh!

ethanndickson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EhabY commented Mar 6, 2026

Uh oh!

Uh oh!

mtojek Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

EhabY Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ethanndickson commented Mar 6, 2026

Uh oh!

EhabY commented Mar 6, 2026

Uh oh!

ethanndickson commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EhabY commented Mar 6, 2026

Uh oh!

ethanndickson commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EhabY commented Mar 6, 2026

Uh oh!

mtojek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

EhabY commented Mar 3, 2026 •

edited

Loading

ethanndickson left a comment •

edited

Loading

ethanndickson commented Mar 6, 2026 •

edited

Loading

ethanndickson commented Mar 6, 2026 •

edited

Loading