Skip to content

Commit 64e1185

Browse files
authored
Pin cfg.Profile to the resolved name in resolveDefaultProfile (#5280)
## Why `databricks auth login --profile DEFAULT --host ...` followed by a no-flag `databricks auth describe` (or any other command that needs the U2M token) fails when secure storage is in use: ``` Unable to authenticate: A new access token could not be retrieved because the refresh token is invalid. ``` `databricks auth describe --profile DEFAULT` works. Running the same flow under `DATABRICKS_AUTH_STORAGE=plaintext` also works. So the bug is specific to secure storage + the implicit DEFAULT fallback. Root cause is a cache-key mismatch between login and read: - `cmd/auth/login.go:222` hardcodes `profileName = "DEFAULT"` when no `--profile` is given, so the OAuthArgument's cache key is the literal string `"DEFAULT"`. The token lands in the keyring under account `"DEFAULT"`. - On the read path, `cfg.Profile` starts empty, `resolveDefaultProfile` only consults `[__settings__].default_profile` (so it stays empty), and the SDK's `configFileLoader.Configure` (`config_file.go:103-105`) loads `[DEFAULT]`'s values but **deliberately leaves `cfg.Profile` empty** when it falls back (`isFallback=true`). `CLICredentials.Configure` then builds an OAuthArgument with `profile=""`, so `GetCacheKey()` falls back to `GetHostCacheKey()` and the lookup goes to the host URL, not `"DEFAULT"`. Miss. plaintext mode masks the same mismatch with `DualWritingTokenCache`, which mirrors every write under the host key — so reads via host URL still find the token. secure mode does not dual-write, so the bug surfaces. This is a pre-existing bug independent of toggling secure-storage by default, but doing so turns a corner case into the default experience. The fix here is targeted enough to land standalone. A defense-in-depth followup in `databricks-sdk-go` will drop the SDK-side `if !isFallback` gate so all SDK consumers benefit from the same self-consistency. The CLI fix lands first so secure-storage users are unblocked without waiting on an SDK release cycle. ## Changes - `cmd/root/auth.go`: `resolveDefaultProfile` swaps `databrickscfg.ResolveDefaultProfile` (settings-only) for `databrickscfg.GetDefaultProfile`, which already does the full 4-step resolution: `[__settings__].default_profile` → the only profile in the file → `[DEFAULT]` → empty. The SDK then sees a non-empty `cfg.Profile`, takes the `isFallback=false` branch, and the name flows through to `CLICredentials.Configure`. OAuthArgument's cache key now matches what login wrote. - `cmd/root/bundle.go` is intentionally NOT touched: bundles deliberately limit their fallback to `[__settings__].default_profile` so a hostless bundle does not get silently routed at a `[DEFAULT]` profile pointing at the wrong workspace. That comment in `bundle.go:74-80` stays load-bearing. - `cmd/root/auth_test.go`: - `TestMustWorkspaceClientWithoutConfiguredDefaultFallsBackToDefaultSection` now asserts `cfg.Profile == "DEFAULT"` (was `""`). The previous assertion documented the bug; the new one documents the contract. - New table-driven `TestResolveDefaultProfile` covers the full resolution order: preset `cfg.Profile`, `DATABRICKS_CONFIG_PROFILE` env, `[__settings__].default_profile`, single profile, `[DEFAULT]` section among many, no fallback, missing file. - `NEXT_CHANGELOG.md`: one-line entry describing the fix and the mismatch it removes. ## Test plan - [x] `task checks` clean - [x] `task lint-q` clean - [x] `go test ./cmd/root/... ./cmd/auth/... ./libs/databrickscfg/...` passes - [x] `go test ./acceptance -run 'TestAccept/cmd/auth'` passes - [ ] Manual repro of Pieter's case (`auth login --profile DEFAULT --host ...` then `auth describe` with no flag under secure storage) succeeds after this PR; the same flow on `main` fails. - [ ] Verify bundle resolution is unaffected: a bundle without `workspace.host` and no `--profile` still uses `[__settings__].default_profile` only (no silent DEFAULT routing). This pull request and its description were written by Isaac.
1 parent 5388341 commit 64e1185

5 files changed

Lines changed: 52 additions & 17 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
* Added `databricks aitools` command group for installing Databricks skills into your coding agents (Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity). Skills are fetched from [github.com/databricks/databricks-agent-skills](https://github.com/databricks/databricks-agent-skills) and either symlinked into each agent's skills directory or copied into the current project. Use `databricks aitools install` to set up, `update` to pull newer versions, `list` to see what's available, and `uninstall` to remove them. Pick where they go with `--scope=project|global` (`--scope=both` is accepted on `update` and `list`).
1515
* `[__settings__].default_profile` is now consulted as a fallback by `databricks api`, `databricks auth token`, and bundle commands when neither `--profile` nor `DATABRICKS_CONFIG_PROFILE` is set. `databricks auth token` continues to give precedence to `DATABRICKS_HOST` over `default_profile`. For bundle commands, `default_profile` only applies when the bundle does not pin its own `workspace.host`.
16+
* Fixed bug where auth commands did not load the DEFAULT profile properly during auth where type is `databricks-cli`.
1617
* `databricks workspace import-dir` now skips `.git`, `.databricks`, and `node_modules` directories during recursive imports. To import one of these directories deliberately, pass it as `SOURCE_PATH` ([#5118](https://github.com/databricks/cli/pull/5118)).
1718
* `databricks postgres create-role --help` now documents the `--json` body shape and rejects the common mistake of wrapping the body in `{"role": ...}` client-side with a hint pointing at the correct shape ([#5111](https://github.com/databricks/cli/pull/5111)).
1819
* `databricks aitools list` honors `--output json`, emitting a structured `{release, skills[...], summary{}}` document so coding agents and CI can consume the skill/version/installation matrix without scraping the tabular text output ([#5233](https://github.com/databricks/cli/pull/5233)).

cmd/root/auth_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,7 @@ token = named-token
514514

515515
w := cmdctx.WorkspaceClient(cmd.Context())
516516
require.NotNil(t, w)
517-
assert.Equal(t, "", w.Config.Profile)
517+
// Pinned so the OAuth cache key matches what `databricks auth login` writes.
518+
assert.Equal(t, "DEFAULT", w.Config.Profile)
518519
assert.Equal(t, "https://default.cloud.databricks.com", w.Config.Host)
519520
}

cmd/root/bundle.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,14 @@ func configureProfile(cmd *cobra.Command, b *bundle.Bundle) {
7272
profile := getProfile(cmd)
7373

7474
// Fall back to [__settings__].default_profile only when the bundle does
75-
// not pin its own host. If the bundle declares workspace.host, applying
76-
// default_profile here could route the user to a profile that points at
77-
// a different host than the bundle expects — let the SDK resolve auth
78-
// from the host instead.
75+
// not pin its own host. The legacy [DEFAULT] section is intentionally
76+
// NOT considered here: a hostless bundle silently routing to whatever
77+
// [DEFAULT] points at could deploy to the wrong workspace and mask a
78+
// missing workspace.host. Auth-only paths use the broader
79+
// databrickscfg.ResolveDefaultProfile, which also accepts [DEFAULT].
7980
if profile == "" && b.Config.Workspace.Host == "" && b.Config.Workspace.Profile == "" {
80-
profile = databrickscfg.ResolveDefaultProfile(cmd.Context())
81+
configFilePath := envlib.Get(cmd.Context(), "DATABRICKS_CONFIG_FILE")
82+
profile, _ = databrickscfg.GetConfiguredDefaultProfile(cmd.Context(), configFilePath)
8183
}
8284

8385
if profile == "" {

libs/databrickscfg/ops.go

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,26 +38,42 @@ func GetConfiguredDefaultProfile(ctx context.Context, configFilePath string) (st
3838
return GetConfiguredDefaultProfileFrom(configFile), nil
3939
}
4040

41-
// ResolveDefaultProfile returns the [__settings__].default_profile value from
42-
// the config file pointed to by DATABRICKS_CONFIG_FILE (or ~/.databrickscfg
43-
// when unset). Returns "" with no error when the file is missing, the setting
44-
// is absent, or parsing fails (a warning is logged on parse error).
41+
// ResolveDefaultProfile returns the default profile from the config file
42+
// pointed to by DATABRICKS_CONFIG_FILE (or ~/.databrickscfg when unset):
43+
// [__settings__].default_profile, else [DEFAULT] if it has a host key,
44+
// else "". Returns "" with no error when the file is missing or parsing
45+
// fails (a warning is logged on parse error).
4546
//
4647
// Callers must respect their own higher-priority sources (an explicit
4748
// --profile flag or DATABRICKS_CONFIG_PROFILE env var) before consulting
48-
// this helper. default_profile is a CLI-level fallback; the SDK does not
49-
// know about it.
49+
// this helper. default_profile and [DEFAULT] are CLI-level fallbacks; the
50+
// SDK loader silently falls back to [DEFAULT] but leaves cfg.Profile empty,
51+
// which breaks the per-profile OAuth cache key. Pinning the name here keeps
52+
// cfg.Profile in sync with what the SDK would read.
53+
//
54+
// Single-profile fallback (using "the only profile in the file" as the
55+
// default) is intentionally NOT applied: that is a prompt-seeding convenience
56+
// (see GetDefaultProfile), not an auth rule, and it would silently route a
57+
// single account-only profile through the workspace-client path.
5058
func ResolveDefaultProfile(ctx context.Context) string {
5159
configFilePath := env.Get(ctx, "DATABRICKS_CONFIG_FILE")
52-
resolved, err := GetConfiguredDefaultProfile(ctx, configFilePath)
60+
configFile, err := loadConfigFile(ctx, configFilePath)
5361
if err != nil {
5462
log.Warnf(ctx, "Failed to load default profile: %v", err)
5563
return ""
5664
}
57-
if resolved != "" {
58-
log.Debugf(ctx, "profile %q resolved from [__settings__].default_profile", resolved)
65+
if configFile == nil {
66+
return ""
67+
}
68+
if profile := GetConfiguredDefaultProfileFrom(configFile); profile != "" {
69+
log.Debugf(ctx, "profile %q resolved from [__settings__].default_profile", profile)
70+
return profile
5971
}
60-
return resolved
72+
if section := configFile.Section(ini.DefaultSection); section.HasKey("host") {
73+
log.Debugf(ctx, "profile %q resolved from the [DEFAULT] section", ini.DefaultSection)
74+
return ini.DefaultSection
75+
}
76+
return ""
6177
}
6278

6379
// GetConfiguredDefaultProfileFrom returns the explicit default profile from

libs/databrickscfg/ops_test.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ func TestResolveDefaultProfile(t *testing.T) {
327327
want: "my-workspace",
328328
},
329329
{
330-
name: "settings without default_profile",
330+
name: "settings without default_profile, no DEFAULT section",
331331
contents: "[__settings__]\nauth_storage = secure\n\n[my-workspace]\nhost = https://abc\n",
332332
want: "",
333333
},
@@ -341,6 +341,21 @@ func TestResolveDefaultProfile(t *testing.T) {
341341
contents: "[__settings__]\ndefault_profile = __settings__\n\n[profile1]\nhost = https://abc\n",
342342
want: "",
343343
},
344+
{
345+
name: "DEFAULT section with host is used when settings is empty",
346+
contents: "[DEFAULT]\nhost = https://default.abc\n\n[profile1]\nhost = https://abc\n",
347+
want: "DEFAULT",
348+
},
349+
{
350+
name: "DEFAULT section without host is ignored",
351+
contents: "[DEFAULT]\naccount_id = 1234\n\n[profile1]\nhost = https://abc\n",
352+
want: "",
353+
},
354+
{
355+
name: "settings takes precedence over DEFAULT section",
356+
contents: "[__settings__]\ndefault_profile = override\n\n[DEFAULT]\nhost = https://default.abc\n\n[override]\nhost = https://override.abc\n",
357+
want: "override",
358+
},
344359
}
345360

346361
for _, tc := range cases {

0 commit comments

Comments
 (0)