From bbf7b137da26062fa2045346c6d2e56c0273eb42 Mon Sep 17 00:00:00 2001
From: Zach <3724288+zedkipp@users.noreply.github.com>
Date: Wed, 26 Nov 2025 02:17:31 -0700
Subject: [PATCH] fix(cli): remove defaulting to keyring when --global-config
set (#20943)
This fixes a regression that caused the VS code extension to be unable
to authenticate after making keyring usage on by default. This is
because the VS code extension assumes the CLI will always use the
session token stored on disk, specifically in the directory specified by
--global-config.
This fix makes keyring usage enabled when the --global-config directory
is not set. This is a bit wonky but necessary to allow the extension to
continue working without modification and without backwards compat
concerns. In the future we should modify these extensions to either
access the credential in the keyring (like Coder Desktop) or some other
approach that doesn't rely on the session token being stored on disk.
Tests:
`coder login dev.coder.com` -> token stored in keyring
`coder login --global-config=/tmp/ dev.coder.com` -> token stored in
`/tmp/session`
---
cli/clitest/clitest.go | 8 +++---
cli/keyring_test.go | 2 ++
cli/root.go | 32 +++++++++++++++------
cli/testdata/coder_--help.golden | 7 +++--
docs/reference/cli/index.md | 2 +-
enterprise/cli/testdata/coder_--help.golden | 7 +++--
6 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/cli/clitest/clitest.go b/cli/clitest/clitest.go
index 20db312101..3e506a26b6 100644
--- a/cli/clitest/clitest.go
+++ b/cli/clitest/clitest.go
@@ -61,10 +61,10 @@ func NewWithCommand(
t testing.TB, cmd *serpent.Command, args ...string,
) (*serpent.Invocation, config.Root) {
configDir := config.Root(t.TempDir())
- // Keyring usage is disabled here because many existing tests expect the session token
- // to be stored on disk and is not properly instrumented for parallel testing against
- // the actual operating system keyring.
- invArgs := append([]string{"--global-config", string(configDir), "--use-keyring=false"}, args...)
+ // Keyring usage is disabled here when --global-config is set because many existing
+ // tests expect the session token to be stored on disk and is not properly instrumented
+ // for parallel testing against the actual operating system keyring.
+ invArgs := append([]string{"--global-config", string(configDir)}, args...)
return setupInvocation(t, cmd, invArgs...), configDir
}
diff --git a/cli/keyring_test.go b/cli/keyring_test.go
index 27b7c12d53..7cb190845a 100644
--- a/cli/keyring_test.go
+++ b/cli/keyring_test.go
@@ -54,6 +54,7 @@ func setupKeyringTestEnv(t *testing.T, clientURL string, args ...string) keyring
serviceName := keyringTestServiceName(t)
root.WithKeyringServiceName(serviceName)
+ root.UseKeyringWithGlobalConfig()
inv, cfg := clitest.NewWithDefaultKeyringCommand(t, cmd, args...)
@@ -169,6 +170,7 @@ func TestUseKeyring(t *testing.T) {
logoutCmd, err := logoutRoot.Command(logoutRoot.AGPL())
require.NoError(t, err)
logoutRoot.WithKeyringServiceName(env.serviceName)
+ logoutRoot.UseKeyringWithGlobalConfig()
logoutInv, _ := clitest.NewWithDefaultKeyringCommand(t, logoutCmd,
"logout",
diff --git a/cli/root.go b/cli/root.go
index 2ec035d64a..7f0561e180 100644
--- a/cli/root.go
+++ b/cli/root.go
@@ -483,9 +483,9 @@ func (r *RootCmd) Command(subcommands []*serpent.Command) (*serpent.Command, err
Flag: varUseKeyring,
Env: envUseKeyring,
Description: "Store and retrieve session tokens using the operating system " +
- "keyring. Enabled by default. If the keyring is not supported on the " +
- "current platform, file-based storage is used automatically. Set to " +
- "false to force file-based storage.",
+ "keyring. This flag is ignored and file-based storage is used when " +
+ "--global-config is set or keyring usage is not supported on the current " +
+ "platform. Set to false to force file-based storage on supported platforms.",
Default: "true",
Value: serpent.BoolOf(&r.useKeyring),
Group: globalGroup,
@@ -536,11 +536,12 @@ type RootCmd struct {
disableDirect bool
debugHTTP bool
- disableNetworkTelemetry bool
- noVersionCheck bool
- noFeatureWarning bool
- useKeyring bool
- keyringServiceName string
+ disableNetworkTelemetry bool
+ noVersionCheck bool
+ noFeatureWarning bool
+ useKeyring bool
+ keyringServiceName string
+ useKeyringWithGlobalConfig bool
}
// InitClient creates and configures a new client with authentication, telemetry,
@@ -721,8 +722,14 @@ func (r *RootCmd) createUnauthenticatedClient(ctx context.Context, serverURL *ur
// flag.
func (r *RootCmd) ensureTokenBackend() sessionstore.Backend {
if r.tokenBackend == nil {
+ // Checking for the --global-config directory being set is a bit wonky but necessary
+ // to allow extensions that invoke the CLI with this flag (e.g. VS code) to continue
+ // working without modification. In the future we should modify these extensions to
+ // either access the credential in the keyring (like Coder Desktop) or some other
+ // approach that doesn't rely on the session token being stored on disk.
+ assumeExtensionInUse := r.globalConfig != config.DefaultDir() && !r.useKeyringWithGlobalConfig
keyringSupported := runtime.GOOS == "windows" || runtime.GOOS == "darwin"
- if r.useKeyring && keyringSupported {
+ if r.useKeyring && !assumeExtensionInUse && keyringSupported {
serviceName := sessionstore.DefaultServiceName
if r.keyringServiceName != "" {
serviceName = r.keyringServiceName
@@ -742,6 +749,13 @@ func (r *RootCmd) WithKeyringServiceName(serviceName string) {
r.keyringServiceName = serviceName
}
+// UseKeyringWithGlobalConfig enables the use of the keyring storage backend
+// when the --global-config directory is set. This is only intended as an override
+// for tests, which require specifying the global config directory for test isolation.
+func (r *RootCmd) UseKeyringWithGlobalConfig() {
+ r.useKeyringWithGlobalConfig = true
+}
+
type AgentAuth struct {
// Agent Client config
agentToken string
diff --git a/cli/testdata/coder_--help.golden b/cli/testdata/coder_--help.golden
index 0057fdc15d..ab13e2af71 100644
--- a/cli/testdata/coder_--help.golden
+++ b/cli/testdata/coder_--help.golden
@@ -111,9 +111,10 @@ variables or flags.
--use-keyring bool, $CODER_USE_KEYRING (default: true)
Store and retrieve session tokens using the operating system keyring.
- Enabled by default. If the keyring is not supported on the current
- platform, file-based storage is used automatically. Set to false to
- force file-based storage.
+ This flag is ignored and file-based storage is used when
+ --global-config is set or keyring usage is not supported on the
+ current platform. Set to false to force file-based storage on
+ supported platforms.
-v, --verbose bool, $CODER_VERBOSE
Enable verbose output.
diff --git a/docs/reference/cli/index.md b/docs/reference/cli/index.md
index e22e2deeec..b26ec94a7f 100644
--- a/docs/reference/cli/index.md
+++ b/docs/reference/cli/index.md
@@ -179,7 +179,7 @@ Disable network telemetry. Network telemetry is collected when connecting to wor
| Environment | $CODER_USE_KEYRING |
| Default | true |
-Store and retrieve session tokens using the operating system keyring. Enabled by default. If the keyring is not supported on the current platform, file-based storage is used automatically. Set to false to force file-based storage.
+Store and retrieve session tokens using the operating system keyring. This flag is ignored and file-based storage is used when --global-config is set or keyring usage is not supported on the current platform. Set to false to force file-based storage on supported platforms.
### --global-config
diff --git a/enterprise/cli/testdata/coder_--help.golden b/enterprise/cli/testdata/coder_--help.golden
index 41ed320968..e199e8cc27 100644
--- a/enterprise/cli/testdata/coder_--help.golden
+++ b/enterprise/cli/testdata/coder_--help.golden
@@ -70,9 +70,10 @@ variables or flags.
--use-keyring bool, $CODER_USE_KEYRING (default: true)
Store and retrieve session tokens using the operating system keyring.
- Enabled by default. If the keyring is not supported on the current
- platform, file-based storage is used automatically. Set to false to
- force file-based storage.
+ This flag is ignored and file-based storage is used when
+ --global-config is set or keyring usage is not supported on the
+ current platform. Set to false to force file-based storage on
+ supported platforms.
-v, --verbose bool, $CODER_VERBOSE
Enable verbose output.