auth: rename legacy storage mode to plaintext, make it the default (#5088)

simonfaltum · web-flow · commit d58d93af30b2 · 2026-04-29T09:40:29.000+02:00
## Why

The CLI's storage-mode resolver had three values: `legacy` (default,
file cache + host-key dual-write), `secure` (OS keyring), and
`plaintext` (file cache, no dual-write, intended placeholder for a
future no-mirror mode).

The `plaintext` path duplicated `legacy` minus the host-key entry that
older Go SDKs (v0.61-v0.103) still rely on, so its "no dual-write"
property bought users nothing. Two modes is the right surface:
`plaintext` for the file cache and `secure` for the OS keyring.

While in there, also fixes the host-key dual-write code path so it
actually goes through the SDK on every cache write (including refresh),
the way `DualWritingTokenCache`'s docstring already claimed.

## Changes

**Mode rename**

- `plaintext` takes today's `legacy` semantics (file cache + host-key
dual-write) and becomes the resolver default.
- `secure` is unchanged.
- `legacy` is removed from the user-visible surface.
`DATABRICKS_AUTH_STORAGE=legacy` is now rejected with the standard
"unknown storage mode" error listing `plaintext` and `secure`. The
keyword was undocumented and users on the default were unaffected.

**Wrap-once refactor**

- New `storage.WrapForOAuthArgument(cache, mode, arg)` returns
`NewDualWritingTokenCache(...)` for plaintext, the cache unchanged
otherwise. Applied at the three login `NewPersistentAuth` call sites
(login main flow, `discoveryLogin`, `runInlineLogin`).
- Deletes `dualWriteLegacyHostKey`/`mirrorTokenUnderHostKey` and the
post-Challenge call sites. The mirror now happens inside the SDK's own
Store call via the wrapper, removing one redundant Lookup and one
redundant primary-key Store per login.
- `DualWritingTokenCache.Store` now treats the host-key mirror as
best-effort: a failure on the second Store is silently dropped.
Previously the wrapper returned the error, but it was always called by a
helper that swallowed it; pulling the wrapper into the SDK Store path
made that error fatal, which would block primary login over a
non-essential backward-compat shim.

**Acceptance fixtures**

- `legacy-env-default/` -&gt; `plaintext-env-default/`. Scripts that set
`DATABRICKS_AUTH_STORAGE=legacy` now set `=plaintext`. Error-message
outputs regenerated.

## Test plan

- [x] `make checks` clean
- [x] `make test` passes (5305 unit, 2514 acceptance)
- [x] `make lint` 0 issues
- [x] Storage-mode acceptance: invalid env, invalid config,
env-overrides-config, plaintext-env-default with the new error-message
format.
- [x] `TestWrapForOAuthArgument`: end-to-end Store across plaintext /
secure / unknown — primary key always written, host-key mirror only in
plaintext.
- [x] `TestDualWritingCacheStoreHostKeyFailureIsBestEffort`: host-key
write error does not propagate; primary write persists.
diff --git a/acceptance/bundle/run_as/job_default/.gitignore b/acceptance/bundle/run_as/job_default/.gitignore
@@ -0,0 +1 @@
+out.requests.txt
diff --git a/acceptance/bundle/run_as/job_default/test.toml b/acceptance/bundle/run_as/job_default/test.toml
@@ -1,4 +1,5 @@
 Badness = "run_as is still set even though it's not in bundle and not in reset request"
+Ignore = [".databricks/.gitignore"]
 
 Local = false
 Cloud = true
diff --git a/acceptance/cmd/auth/storage-modes/env-overrides-config/script b/acceptance/cmd/auth/storage-modes/env-overrides-config/script
@@ -1,4 +1,4 @@
-export DATABRICKS_AUTH_STORAGE=legacy
+export DATABRICKS_AUTH_STORAGE=plaintext
 
 cat > "./home/.databrickscfg" <<ENDCFG
 [__settings__]
@@ -26,7 +26,7 @@ ENDCACHE
 title "Token cache keys before logout\n"
 jq -S '.tokens | keys' "./home/.databricks/token-cache.json"
 
-# Env var = legacy must beat [__settings__] auth_storage = secure.
+# Env var = plaintext must beat [__settings__] auth_storage = secure.
 # Proof: logout clears the file-backed token-cache.json. If the env override
 # were ignored, the CLI would try the keyring and leave the file cache alone.
 trace $CLI auth logout --profile dev --auto-approve
diff --git a/acceptance/cmd/auth/storage-modes/invalid-config/output.txt b/acceptance/cmd/auth/storage-modes/invalid-config/output.txt
@@ -1,5 +1,5 @@
 
 >>> [CLI] auth token --profile nonexistent
-Error: auth_storage: unknown storage mode "bogus" (want legacy, secure, or plaintext)
+Error: auth_storage: unknown storage mode "bogus" (want plaintext or secure)
 
 Exit code: 1
diff --git a/acceptance/cmd/auth/storage-modes/invalid-env/output.txt b/acceptance/cmd/auth/storage-modes/invalid-env/output.txt
@@ -1,5 +1,5 @@
 
 >>> [CLI] auth token --profile nonexistent
-Error: DATABRICKS_AUTH_STORAGE: unknown storage mode "bogus" (want legacy, secure, or plaintext)
+Error: DATABRICKS_AUTH_STORAGE: unknown storage mode "bogus" (want plaintext or secure)
 
 Exit code: 1
diff --git a/acceptance/cmd/auth/storage-modes/plaintext-env-default/out.test.toml b/acceptance/cmd/auth/storage-modes/plaintext-env-default/out.test.toml
diff --git a/acceptance/cmd/auth/storage-modes/plaintext-env-default/output.txt b/acceptance/cmd/auth/storage-modes/plaintext-env-default/output.txt
diff --git a/acceptance/cmd/auth/storage-modes/plaintext-env-default/script b/acceptance/cmd/auth/storage-modes/plaintext-env-default/script
@@ -1,4 +1,4 @@
-export DATABRICKS_AUTH_STORAGE=legacy
+export DATABRICKS_AUTH_STORAGE=plaintext
 
 cat > "./home/.databrickscfg" <<ENDCFG
 [dev]
diff --git a/cmd/auth/login.go b/cmd/auth/login.go
@@ -60,23 +60,6 @@ func discoveryErr(msg string, err error) error {
 	return fmt.Errorf("%s%s", msg, discoveryFallbackTip)
 }
 
-// dualWriteLegacyHostKey mirrors the freshly minted token under the legacy
-// host-based cache key so users alternating between CLI and SDK find it.
-// Skipped for secure mode to avoid multiplying keyring entries.
-func dualWriteLegacyHostKey(ctx context.Context, tokenCache cache.TokenCache, arg u2m.OAuthArgument, mode storage.StorageMode) {
-	if mode != storage.StorageModeLegacy {
-		return
-	}
-	t, err := tokenCache.Lookup(arg.GetCacheKey())
-	if err != nil || t == nil {
-		return
-	}
-	dual := storage.NewDualWritingTokenCache(tokenCache, arg)
-	if err := dual.Store(arg.GetCacheKey(), t); err != nil {
-		log.Debugf(ctx, "token cache dual-write failed: %v", err)
-	}
-}
-
 type discoveryPersistentAuth interface {
 	Challenge() error
 	Token() (*oauth2.Token, error)
@@ -256,7 +239,7 @@ a new profile is created.
 		persistentAuthOpts := []u2m.PersistentAuthOption{
 			u2m.WithOAuthArgument(oauthArgument),
 			u2m.WithBrowser(getBrowserFunc(cmd)),
-			u2m.WithTokenCache(tokenCache),
+			u2m.WithTokenCache(storage.WrapForOAuthArgument(tokenCache, mode, oauthArgument)),
 		}
 		if len(scopesList) > 0 {
 			persistentAuthOpts = append(persistentAuthOpts, u2m.WithScopes(scopesList))
@@ -273,7 +256,6 @@ a new profile is created.
 		if err = persistentAuth.Challenge(); err != nil {
 			return err
 		}
-		dualWriteLegacyHostKey(ctx, tokenCache, oauthArgument, mode)
 		// At this point, an OAuth token has been successfully minted and stored
 		// in the CLI cache. The rest of the command focuses on:
 		// 1. Workspace selection for SPOG hosts (best-effort);
@@ -593,7 +575,7 @@ func discoveryLogin(ctx context.Context, in discoveryLoginInputs) error {
 		u2m.WithOAuthArgument(arg),
 		u2m.WithBrowser(in.browserFunc),
 		u2m.WithDiscoveryLogin(),
-		u2m.WithTokenCache(in.tokenCache),
+		u2m.WithTokenCache(storage.WrapForOAuthArgument(in.tokenCache, in.mode, arg)),
 	}
 	if len(scopesList) > 0 {
 		opts = append(opts, u2m.WithScopes(scopesList))
@@ -613,7 +595,6 @@ func discoveryLogin(ctx context.Context, in discoveryLoginInputs) error {
 	if err := persistentAuth.Challenge(); err != nil {
 		return discoveryErr("login via login.databricks.com failed", err)
 	}
-	dualWriteLegacyHostKey(ctx, in.tokenCache, arg, in.mode)
 
 	discoveredHost := arg.GetDiscoveredHost()
 	if discoveredHost == "" {
diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go
@@ -15,7 +15,6 @@ import (
 	"time"
 
 	"github.com/databricks/cli/libs/auth"
-	"github.com/databricks/cli/libs/auth/storage"
 	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/cli/libs/databrickscfg/profile"
 	"github.com/databricks/cli/libs/env"
@@ -1104,73 +1103,3 @@ func TestLoginRejectsPositionalArgWithProfileFlag(t *testing.T) {
 	err := cmd.Execute()
 	assert.ErrorContains(t, err, `argument "https://example.com" cannot be combined with --host or --profile`)
 }
-
-func TestDualWriteLegacyHostKey(t *testing.T) {
-	const (
-		profileName = "dual-profile"
-		host        = "https://dual-host.example.com"
-	)
-	tok := &oauth2.Token{AccessToken: "abc", RefreshToken: "r"}
-
-	cacheWithToken := func() *inMemoryTokenCache {
-		return &inMemoryTokenCache{Tokens: map[string]*oauth2.Token{profileName: tok}}
-	}
-	emptyCache := func() *inMemoryTokenCache {
-		return &inMemoryTokenCache{Tokens: map[string]*oauth2.Token{}}
-	}
-	newArg := func(t *testing.T) *u2m.BasicDiscoveryOAuthArgument {
-		arg, err := u2m.NewBasicDiscoveryOAuthArgument(profileName)
-		require.NoError(t, err)
-		arg.SetDiscoveredHost(host)
-		return arg
-	}
-
-	cases := []struct {
-		name        string
-		mode        storage.StorageMode
-		cache       func() *inMemoryTokenCache
-		wantHostKey bool
-	}{
-		{
-			name:        "legacy mirrors cached token under host key",
-			mode:        storage.StorageModeLegacy,
-			cache:       cacheWithToken,
-			wantHostKey: true,
-		},
-		{
-			name:  "legacy is a no-op when cache has no entry",
-			mode:  storage.StorageModeLegacy,
-			cache: emptyCache,
-		},
-		{
-			name:  "secure skips dual-write",
-			mode:  storage.StorageModeSecure,
-			cache: cacheWithToken,
-		},
-		{
-			name:  "plaintext skips dual-write",
-			mode:  storage.StorageModePlaintext,
-			cache: cacheWithToken,
-		},
-		{
-			name:  "unknown mode skips dual-write",
-			mode:  storage.StorageModeUnknown,
-			cache: cacheWithToken,
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			c := tc.cache()
-			dualWriteLegacyHostKey(t.Context(), c, newArg(t), tc.mode)
-
-			got, err := c.Lookup(host)
-			if tc.wantHostKey {
-				require.NoError(t, err)
-				assert.Equal(t, tok, got)
-			} else {
-				assert.ErrorIs(t, err, cache.ErrNotFound)
-			}
-		})
-	}
-}
diff --git a/cmd/auth/token.go b/cmd/auth/token.go
@@ -132,9 +132,9 @@ type loadTokenArgs struct {
 	// responsible for construction so that tests can substitute an in-memory cache.
 	tokenCache cache.TokenCache
 
-	// mode is the resolved storage mode. When set to StorageModeLegacy, login
-	// paths mirror freshly minted tokens under the legacy host-based key so
-	// older SDKs that still look up by host continue to find them.
+	// mode is the resolved storage mode. When set to StorageModePlaintext,
+	// login paths mirror freshly minted tokens under the legacy host-based
+	// key so older SDKs that still look up by host continue to find them.
 	mode storage.StorageMode
 
 	// persistentAuthOpts are the options to pass to the persistent auth client.
@@ -460,7 +460,7 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler, tokenCache c
 	persistentAuthOpts := []u2m.PersistentAuthOption{
 		u2m.WithOAuthArgument(oauthArgument),
 		u2m.WithBrowser(func(url string) error { return browser.Open(ctx, url) }),
-		u2m.WithTokenCache(tokenCache),
+		u2m.WithTokenCache(storage.WrapForOAuthArgument(tokenCache, mode, oauthArgument)),
 	}
 	if len(scopesList) > 0 {
 		persistentAuthOpts = append(persistentAuthOpts, u2m.WithScopes(scopesList))
@@ -477,7 +477,6 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler, tokenCache c
 	if err = persistentAuth.Challenge(); err != nil {
 		return "", nil, err
 	}
-	dualWriteLegacyHostKey(ctx, tokenCache, oauthArgument, mode)
 
 	clearKeys := oauthLoginClearKeys()
 	clearKeys = append(clearKeys, databrickscfg.ExperimentalIsUnifiedHostKey)
diff --git a/libs/auth/storage/cache.go b/libs/auth/storage/cache.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/databricks/databricks-sdk-go/credentials/u2m"
 	"github.com/databricks/databricks-sdk-go/credentials/u2m/cache"
 )
 
@@ -37,6 +38,21 @@ func ResolveCache(ctx context.Context, override StorageMode) (cache.TokenCache,
 	return resolveCacheWith(ctx, override, defaultCacheFactories())
 }
 
+// WrapForOAuthArgument wraps tokenCache so SDK-side writes (Challenge, refresh)
+// dual-write to the legacy host-based cache key when mode is plaintext. Other
+// modes return tokenCache unchanged: secure mode never writes a host-key entry,
+// and the wrapper has nothing to do for non-file backends.
+//
+// Pass the OAuthArgument that the same NewPersistentAuth call will use. For
+// discovery arguments the discovered host is read at Store time, so it is
+// safe to wrap before Challenge populates it.
+func WrapForOAuthArgument(tokenCache cache.TokenCache, mode StorageMode, arg u2m.OAuthArgument) cache.TokenCache {
+	if mode != StorageModePlaintext {
+		return tokenCache
+	}
+	return NewDualWritingTokenCache(tokenCache, arg)
+}
+
 // resolveCacheWith is the pure form of ResolveCache. It takes the factory
 // set as a parameter so tests can inject stubs.
 func resolveCacheWith(ctx context.Context, override StorageMode, f cacheFactories) (cache.TokenCache, StorageMode, error) {
@@ -47,9 +63,7 @@ func resolveCacheWith(ctx context.Context, override StorageMode, f cacheFactorie
 	switch mode {
 	case StorageModeSecure:
 		return f.newKeyring(), mode, nil
-	case StorageModeLegacy, StorageModePlaintext:
-		// Plaintext currently maps to the file cache; a dedicated
-		// plaintext backend (no host-keyed dual-writes) is a follow-up.
+	case StorageModePlaintext:
 		c, err := f.newFile(ctx)
 		if err != nil {
 			return nil, "", fmt.Errorf("open file token cache: %w", err)
diff --git a/libs/auth/storage/cache_test.go b/libs/auth/storage/cache_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/libs/env"
+	"github.com/databricks/databricks-sdk-go/credentials/u2m"
 	"github.com/databricks/databricks-sdk-go/credentials/u2m/cache"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -36,14 +37,14 @@ func hermetic(t *testing.T) {
 	t.Setenv("DATABRICKS_CONFIG_FILE", filepath.Join(t.TempDir(), "databrickscfg"))
 }
 
-func TestResolveCache_DefaultsToLegacyFile(t *testing.T) {
+func TestResolveCache_DefaultsToPlaintextFile(t *testing.T) {
 	hermetic(t)
 	ctx := t.Context()
 
 	got, mode, err := resolveCacheWith(ctx, "", fakeFactories(t))
 
 	require.NoError(t, err)
-	assert.Equal(t, StorageModeLegacy, mode)
+	assert.Equal(t, StorageModePlaintext, mode)
 	assert.Equal(t, "file", got.(stubCache).source)
 }
 
@@ -69,7 +70,7 @@ func TestResolveCache_EnvVarSelectsSecure(t *testing.T) {
 	assert.Equal(t, "keyring", got.(stubCache).source)
 }
 
-func TestResolveCache_PlaintextFallsBackToFile(t *testing.T) {
+func TestResolveCache_PlaintextOverrideUsesFile(t *testing.T) {
 	hermetic(t)
 	ctx := t.Context()
 
@@ -109,8 +110,51 @@ func TestResolveCache_FileFactoryErrorPropagates(t *testing.T) {
 		newKeyring: func() cache.TokenCache { return stubCache{source: "keyring"} },
 	}
 
-	_, _, err := resolveCacheWith(ctx, StorageModeLegacy, factories)
+	_, _, err := resolveCacheWith(ctx, StorageModePlaintext, factories)
 
 	require.Error(t, err)
 	assert.ErrorIs(t, err, boom)
 }
+
+func TestWrapForOAuthArgument(t *testing.T) {
+	const (
+		host       = "https://example.com"
+		profileKey = "myprofile"
+	)
+	arg, err := u2m.NewProfileWorkspaceOAuthArgument(host, profileKey)
+	require.NoError(t, err)
+
+	cases := []struct {
+		name        string
+		mode        StorageMode
+		wantWrap    bool
+		wantHostKey bool
+	}{
+		{"plaintext wraps and mirrors under host key", StorageModePlaintext, true, true},
+		{"secure returns inner unchanged; no host-key mirror", StorageModeSecure, false, false},
+		{"unknown returns inner unchanged; no host-key mirror", StorageModeUnknown, false, false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			inner := newMemoryCache()
+			got := WrapForOAuthArgument(inner, tc.mode, arg)
+
+			_, wrapped := got.(*DualWritingTokenCache)
+			assert.Equal(t, tc.wantWrap, wrapped, "wrapper presence")
+
+			tok := &oauth2.Token{AccessToken: "abc"}
+			require.NoError(t, got.Store(profileKey, tok))
+
+			primary, err := inner.Lookup(profileKey)
+			require.NoError(t, err, "primary key must always be written")
+			assert.Equal(t, tok, primary)
+
+			_, err = inner.Lookup(host)
+			if tc.wantHostKey {
+				require.NoError(t, err, "host-key mirror expected in plaintext mode")
+			} else {
+				assert.ErrorIs(t, err, cache.ErrNotFound, "no host-key mirror expected outside plaintext mode")
+			}
+		})
+	}
+}
diff --git a/libs/auth/storage/dual_writing_cache.go b/libs/auth/storage/dual_writing_cache.go
@@ -32,6 +32,12 @@ func NewDualWritingTokenCache(inner u2m_cache.TokenCache, arg u2m.OAuthArgument)
 // also mirrored under the host key (when distinct); writes under any other
 // key pass through unchanged so that a Store(hostKey, t) from an older SDK
 // that still dual-writes internally does not recursively re-expand.
+//
+// The host-key mirror is best-effort: if the second Store fails, the error
+// is silently dropped. The host-key entry is a backward-compat shim for old
+// Go SDK versions (v0.61-v0.103) that still look up by host. Failing the
+// whole Store call would break primary login over a non-essential mirror,
+// so a stale host-key entry is the lesser harm.
 func (c *DualWritingTokenCache) Store(key string, t *oauth2.Token) error {
 	if err := c.inner.Store(key, t); err != nil {
 		return err
@@ -44,7 +50,8 @@ func (c *DualWritingTokenCache) Store(key string, t *oauth2.Token) error {
 	if hostKey == "" || hostKey == primaryKey {
 		return nil
 	}
-	return c.inner.Store(hostKey, t)
+	_ = c.inner.Store(hostKey, t)
+	return nil
 }
 
 // Lookup implements [u2m_cache.TokenCache]; delegates to the inner cache.
diff --git a/libs/auth/storage/dual_writing_cache_test.go b/libs/auth/storage/dual_writing_cache_test.go
diff --git a/libs/auth/storage/mode.go b/libs/auth/storage/mode.go
diff --git a/libs/auth/storage/mode_test.go b/libs/auth/storage/mode_test.go

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`Badness = "run_as is still set even though it's not in bundle and not in reset request"`
	`2`	`+Ignore = [".databricks/.gitignore"]`
`2`	`3`
`3`	`4`	`Local = false`
`4`	`5`	`Cloud = true`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-export DATABRICKS_AUTH_STORAGE=legacy`
	`1`	`+export DATABRICKS_AUTH_STORAGE=plaintext`
`2`	`2`
`3`	`3`	`cat > "./home/.databrickscfg" <<ENDCFG`
`4`	`4`	`[dev]`