Coordinate file-backed OAuth refreshes and guard uncertain outcomes

.nextchanges/cli/file-backed-oauth-cache-transactions.md

@@ -0,0 +1 @@
+* Fix concurrent file-backed OAuth cache updates and prevent automatic reuse of uncertain refresh credentials. ([#7396](https://github.com/databricks/cli/pull/7396))

acceptance/bundle/deploy/mlops-stacks/script

@@ -36,7 +36,7 @@ if [ -z "$CLOUD_ENV" ]; then
 else
   trace $CLI bundle init mlops-stacks --config-file config.json
 fi
-trace cat test_repo_mlops_stacks/README.md | head -n 4
+trace cat test_repo_mlops_stacks/README.md | sed -n '1,4p'
 
 cd "test_repo_mlops_stacks/project_name_${UNIQUE_NAME}" || exit 1
 

acceptance/cmd/auth/docker/invalid-refresh-token/output.txt

@@ -6,21 +6,21 @@ Error: A new access token could not be retrieved because the refresh token is in
   $ databricks auth login --profile docker-test
 
 >>> musterr [CLI] auth docker configure docker-test
-Error: A new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run the following command:
+Error: A previous OAuth refresh has an uncertain outcome; automatic refresh is blocked. To reauthenticate, run the following command:
   $ databricks auth login --profile docker-test
 
 === Stored workspace ID
 
 >>> musterr [CLI] auth docker host --profile docker-test
-Error: A new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run the following command:
+Error: A previous OAuth refresh has an uncertain outcome; automatic refresh is blocked. To reauthenticate, run the following command:
   $ databricks auth login --profile docker-test
 
 >>> musterr [CLI] auth docker configure docker-test
-Error: A new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run the following command:
+Error: A previous OAuth refresh has an uncertain outcome; automatic refresh is blocked. To reauthenticate, run the following command:
   $ databricks auth login --profile docker-test
 
 >>> musterr [CLI] auth docker token
-Error: A new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run the following command:
+Error: A previous OAuth refresh has an uncertain outcome; automatic refresh is blocked. To reauthenticate, run the following command:
   $ databricks auth login --profile docker-test
 
 === Docker config not created

acceptance/cmd/auth/login/device-code/non-interactive/nominal/output.txt

@@ -16,6 +16,7 @@ provided in your browser.
 
 >>> find.py home
 home/.databricks/token-cache.json
+home/.databricks/token-cache.json.lock
 
 >>> cache_keys
 [

acceptance/cmd/auth/token/force-refresh-invalid-refresh-token/output.txt

@@ -2,5 +2,5 @@ Error: A new access token could not be retrieved because the refresh token is in
   $ databricks auth login --profile test-profile
 {
   "error_code": "UNAUTHENTICATED",
-  "message": "A new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run the following command:\n  $ databricks auth login --profile test-profile"
+  "message": "A previous OAuth refresh has an uncertain outcome; automatic refresh is blocked. To reauthenticate, run the following command:\n  $ databricks auth login --profile test-profile"
 }

acceptance/cmd/completion/script

@@ -22,10 +22,10 @@ trace $CLI completion uninstall --shell zsh --auto-approve
 trace $CLI completion status --shell zsh
 
 # Test shell subcommands produce output
-$CLI completion bash 2>&1 | head -1
-$CLI completion zsh 2>&1 | head -1
-$CLI completion fish 2>&1 | head -1
-$CLI completion powershell 2>&1 | head -1
+$CLI completion bash 2>&1 | sed -n '1p'
+$CLI completion zsh 2>&1 | sed -n '1p'
+$CLI completion fish 2>&1 | sed -n '1p'
+$CLI completion powershell 2>&1 | sed -n '1p'
 
 # Test bash --no-descriptions
-$CLI completion bash --no-descriptions 2>&1 | head -1
+$CLI completion bash --no-descriptions 2>&1 | sed -n '1p'

bundle/bundle_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/bundle/env"
+	"github.com/databricks/cli/internal/testutil"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/logdiag"
@@ -178,6 +179,10 @@ func TestBundleGetResourceConfigJobsPointer(t *testing.T) {
 }
 
 func TestClearWorkspaceClient(t *testing.T) {
+	configPath := filepath.Join(t.TempDir(), "databrickscfg")
+	testutil.WriteFile(t, configPath, "")
+	t.Setenv("DATABRICKS_CONFIG_FILE", configPath)
+
 	ctx := t.Context()
 
 	// First attempt: profile "profile-A" doesn't exist → error mentions "profile-A".

bundle/config/workspace_test.go

@@ -1,9 +1,12 @@
 package config
 
 import (
+	"context"
+	"errors"
 	"io/fs"
 	"path/filepath"
 	"runtime"
+	"slices"
 	"testing"
 
 	"github.com/databricks/cli/bundle/env"
@@ -14,6 +17,21 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func stubUnavailableWorkspaceMetadata(t *testing.T, hosts ...string) {
+	t.Helper()
+	previous := config.DefaultHostMetadataResolverFactory
+	config.DefaultHostMetadataResolverFactory = func(*config.Config) config.HostMetadataResolver {
+		return func(_ context.Context, host string) (*config.HostMetadata, error) {
+			if !slices.Contains(hosts, host) {
+				t.Error("unexpected fixture metadata host")
+				return nil, errors.New("unexpected fixture metadata host")
+			}
+			return nil, nil
+		}
+	}
+	t.Cleanup(func() { config.DefaultHostMetadataResolverFactory = previous })
+}
+
 func setupWorkspaceTest(t *testing.T) string {
 	testutil.CleanupEnvironment(t)
 
@@ -27,6 +45,7 @@ func setupWorkspaceTest(t *testing.T) string {
 }
 
 func TestWorkspaceResolveProfileFromHost(t *testing.T) {
+	stubUnavailableWorkspaceMetadata(t, "https://abc.cloud.databricks.com")
 	// If only a workspace host is specified, try to find a profile that uses
 	// the same workspace host (unambiguously).
 	w := Workspace{
@@ -125,6 +144,7 @@ func TestWorkspaceNormalizeHostURL(t *testing.T) {
 }
 
 func TestWorkspaceClientNormalizesHostBeforeProfileResolution(t *testing.T) {
+	stubUnavailableWorkspaceMetadata(t, "https://spog.databricks.com")
 	// Regression test: Client() must normalize the host URL (strip ?o= and
 	// populate WorkspaceID) before building the SDK config and resolving
 	// profiles. This ensures workspace_id is available for disambiguation.
@@ -156,6 +176,7 @@ func TestWorkspaceClientNormalizesHostBeforeProfileResolution(t *testing.T) {
 }
 
 func TestWorkspaceClientProfileOverridesAuthEnv(t *testing.T) {
+	stubUnavailableWorkspaceMetadata(t, "https://tst.cloud.databricks.test")
 	// An explicit profile must win over auth env vars (#5096).
 	setupWorkspaceTest(t)
 
@@ -179,6 +200,7 @@ func TestWorkspaceClientProfileOverridesAuthEnv(t *testing.T) {
 }
 
 func TestWorkspaceClientProfileFillsAuthFromEnv(t *testing.T) {
+	stubUnavailableWorkspaceMetadata(t, "https://tst.cloud.databricks.test")
 	// Host-only profile: the profile wins for the host, but env fills the token
 	// it omits (#5096).
 	setupWorkspaceTest(t)
@@ -201,6 +223,7 @@ func TestWorkspaceClientProfileFillsAuthFromEnv(t *testing.T) {
 }
 
 func TestWorkspaceClientHostAndProfileOverridesAuthEnv(t *testing.T) {
+	stubUnavailableWorkspaceMetadata(t, "https://tst.cloud.databricks.test")
 	// Bundle pins both workspace.host and workspace.profile: the profile wins
 	// for auth and the host check passes because they agree (#5096).
 	setupWorkspaceTest(t)
@@ -247,6 +270,7 @@ func TestWorkspaceConfigHTTPTimeout(t *testing.T) {
 }
 
 func TestWorkspaceVerifyProfileForHost(t *testing.T) {
+	stubUnavailableWorkspaceMetadata(t, "https://abc.cloud.databricks.com", "https://def.cloud.databricks.com")
 	// If both a workspace host and a profile are specified,
 	// verify that the host configured in the profile matches
 	// the host configured in the bundle configuration.

cmd/auth/auth_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/databricks/cli/libs/cmdctx"
 	"github.com/databricks/cli/libs/cmdio/cmdiotest/termtest"
 	"github.com/databricks/cli/libs/databrickscfg/profile"
+	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -113,13 +114,9 @@ func TestProfileHostConflictTokenViaCobra(t *testing.T) {
 	assert.Contains(t, err.Error(), `--profile "profile-1" has host "https://www.host1.test", which conflicts with --host "https://other.host.test"`)
 }
 
-// TestProfileHostCompatibleViaCobra verifies that matching --profile and --host
-// pass the conflict check (the command will fail later for other reasons, but
-// NOT with a conflict error).
+// Matching flags reach the stubbed run boundary after the real conflict check.
 func TestProfileHostCompatibleViaCobra(t *testing.T) {
-	// Copy the fixture into a temp directory so the auth login flow's writes
-	// (e.g. silent plaintext-fallback persistence on CI runners without a
-	// usable keyring) cannot dirty the checked-in fixture.
+	// Keep Cobra's profile fixture in the owned test directory.
 	configPath := filepath.Join(t.TempDir(), ".databrickscfg")
 	fixture, err := os.ReadFile("./testdata/.databrickscfg")
 	require.NoError(t, err)
@@ -129,6 +126,10 @@ func TestProfileHostCompatibleViaCobra(t *testing.T) {
 	ctx := cmdctx.GenerateExecId(t.Context())
 	cli := root.New(ctx)
 	cli.AddCommand(New())
+	login, _, err := cli.Find([]string{"auth", "login"})
+	require.NoError(t, err)
+	reachedRun := false
+	login.RunE = func(*cobra.Command, []string) error { reachedRun = true; return nil }
 
 	cli.SetArgs([]string{
 		"auth", "login",
@@ -137,8 +138,7 @@ func TestProfileHostCompatibleViaCobra(t *testing.T) {
 	})
 
 	_, err = cli.ExecuteContextC(ctx)
-	// The command may fail for other reasons (no browser, non-interactive, etc.)
-	// but it should NOT fail with a conflict error.
+	assert.True(t, reachedRun)
 	if err != nil {
 		assert.NotContains(t, err.Error(), "conflicts with --host")
 	}

cmd/auth/login.go

@@ -126,6 +126,23 @@ func (d *defaultDiscoveryClient) IntrospectToken(ctx context.Context, host, acce
 func storeLoginToken(ctx context.Context, tokenStore storage.Store, mode storage.StorageMode, arg u2m.OAuthArgument, token *oauth2.Token) error {
 	tokenStore = storage.WrapForOAuthArgument(ctx, tokenStore, mode, arg)
 	key := arg.GetCacheKey()
+	handled, err := storage.WithFileTransaction(ctx, tokenStore,
+		func(view storage.Store) error {
+			entry := storage.Entry{Token: token}
+			if err := storage.RecordFileLoginResult(view, key, entry); err != nil {
+				return err
+			}
+			return view.Put(key, entry)
+		},
+		func(view storage.Store) error { return view.Delete(key) },
+	)
+	if handled {
+		if err != nil {
+			return fmt.Errorf("store token: %w", err)
+		}
+		storage.PinSecureMode(ctx, mode, storage.StorageModeUnknown)
+		return nil
+	}
 	if err := tokenStore.Put(key, storage.Entry{Token: token}); err != nil {
 		storeErr := fmt.Errorf("store token: %w", err)
 		// The profile is already saved, but the old token may still be present.

cmd/auth/login_device_code.go

@@ -194,7 +194,7 @@ func (l nonInteractiveDeviceCodeLogin) start(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	err = l.tokenStore.Put(l.key, storage.Entry{PendingLogin: &storage.PendingLogin{
+	err = storage.PutWithContext(ctx, l.tokenStore, l.key, storage.Entry{PendingLogin: &storage.PendingLogin{
 		DeviceCode: da.DeviceCode,
 		UserCode:   da.UserCode,
 		Host:       l.host,
@@ -274,7 +274,7 @@ func (l nonInteractiveDeviceCodeLogin) complete(ctx context.Context, userCode, c
 // deletePendingLogin deletes the pending login. A pending login that is left
 // over only fails a rerun of the second step, so an error is only logged.
 func (l nonInteractiveDeviceCodeLogin) deletePendingLogin(ctx context.Context) {
-	if err := l.tokenStore.Delete(l.key); err != nil {
+	if err := storage.DeleteWithContext(ctx, l.tokenStore, l.key); err != nil {
 		log.Debugf(ctx, "Failed to delete the pending device code login: %v", err)
 	}
 }

cmd/auth/login_device_code_test.go

@@ -241,6 +241,9 @@ func TestIsDeviceCodeSupportedForHost(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			if tt.args.Host == "https://accounts.cloud.databricks.test" {
+				stubUnavailableFixtureMetadata(t, []string{tt.args.Host})
+			}
 			args := tt.args
 			require.NoError(t, setHost(t.Context(), nil, &args, nil))
 			assert.Equal(t, tt.want, isDeviceCodeSupportedForHost(args.Host, args.DiscoveryURL))

cmd/auth/login_intent_test.go

@@ -0,0 +1,70 @@
+package auth
+
+import (
+	"crypto/sha256"
+	"errors"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/databricks/cli/libs/auth/storage"
+	"github.com/databricks/cli/libs/auth/u2m"
+	"golang.org/x/oauth2"
+)
+
+func TestStoreLoginAccessOnlyReplacesJournalBoundGeneration(t *testing.T) {
+	for _, fail := range []bool{false, true} {
+		t.Run(map[bool]string{false: "publish", true: "failed-commit-cleanup"}[fail], func(t *testing.T) {
+			path := filepath.Join(t.TempDir(), "tokens.json")
+			store, err := storage.NewFileStore(t.Context(), storage.WithFileLocation(path))
+			if err != nil {
+				t.Fatal("store setup failed")
+			}
+			const host = "https://workspace.test"
+			old := storage.Entry{Token: &oauth2.Token{AccessToken: "old", RefreshToken: "synthetic-refresh", Expiry: time.Now().Add(time.Hour)}}
+			for _, key := range []string{"profile", host, "copy"} {
+				if err := store.Put(key, old); err != nil {
+					t.Fatal("seed failed")
+				}
+			}
+			_, err = storage.WithFileTransaction(t.Context(), store, func(view storage.Store) error {
+				if _, err := storage.BeginFileRefresh(t.Context(), view, "profile", sha256.Sum256([]byte("route"))); err != nil {
+					return err
+				}
+				return errors.New("closed unknown outcome")
+			}, nil)
+			if err == nil {
+				t.Fatal("prepared setup failed")
+			}
+			arg, err := u2m.NewProfileWorkspaceOAuthArgument(host, "profile")
+			if err != nil {
+				t.Fatal("argument setup failed")
+			}
+			subject := store
+			if fail {
+				subject = obstructFileCommitStore{Store: store, path: path}
+			}
+			err = storeLoginToken(t.Context(), subject, storage.StorageModePlaintext, arg, &oauth2.Token{AccessToken: "access-only"})
+			if (err != nil) != fail {
+				t.Fatal("access-only publication outcome changed")
+			}
+			for _, key := range []string{"profile", host} {
+				entry, lookupErr := store.Lookup(key)
+				if fail {
+					if !errors.Is(lookupErr, storage.ErrNotFound) {
+						t.Fatal("failed publication left stale entry")
+					}
+				} else if lookupErr != nil || entry.Token == nil || entry.Token.AccessToken != "access-only" || entry.Token.RefreshToken != "" {
+					t.Fatal("access-only result changed")
+				}
+			}
+			_, err = storage.WithFileTransaction(t.Context(), store, func(view storage.Store) error {
+				_, err := storage.BeginFileRefresh(t.Context(), view, "copy", sha256.Sum256([]byte("route")))
+				return err
+			}, nil)
+			if !errors.Is(err, storage.ErrRefreshRecoveryRequired) {
+				t.Fatal("replacement reopened copied old generation")
+			}
+		})
+	}
+}