From 403af4ceb31597874b8d9cb8517934c86be082a0 Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Wed, 24 Jun 2026 11:56:44 +0000 Subject: [PATCH 1/8] Make --profile take precedence over auth environment variables When --profile is set explicitly, host and auth credentials from the profile now win over DATABRICKS_HOST/DATABRICKS_TOKEN and other auth env vars. Previously the SDK's env-first loader order silently shadowed the selected profile (#5096). --- NEXT_CHANGELOG.md | 2 + .../account-host-with-workspace-id/output.txt | 4 +- cmd/root/auth.go | 20 ++++-- cmd/root/auth_test.go | 65 +++++++++++++++++++ libs/databrickscfg/loader.go | 47 ++++++++++++++ libs/databrickscfg/loader_test.go | 16 +++++ 6 files changed, 148 insertions(+), 6 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 30a64c54100..4f074566ddd 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -6,6 +6,8 @@ ### CLI +* An explicit `--profile` now takes precedence over authentication environment variables (`DATABRICKS_HOST`, `DATABRICKS_TOKEN`, etc.). Previously these env vars silently shadowed the selected profile's host and credentials ([#5096](https://github.com/databricks/cli/issues/5096)). + ### Bundles ### Dependency updates diff --git a/acceptance/cmd/auth/describe/account-host-with-workspace-id/output.txt b/acceptance/cmd/auth/describe/account-host-with-workspace-id/output.txt index 0cbc7f6a68c..505db5b55c1 100644 --- a/acceptance/cmd/auth/describe/account-host-with-workspace-id/output.txt +++ b/acceptance/cmd/auth/describe/account-host-with-workspace-id/output.txt @@ -7,10 +7,10 @@ Account ID: acct-123 Authenticated with: pat ----- Current configuration: - ✓ host: [DATABRICKS_URL] (from DATABRICKS_HOST environment variable) + ✓ host: [DATABRICKS_URL] (from [TEST_TMP_DIR]/home/.databrickscfg config file) ✓ account_id: acct-123 (from [TEST_TMP_DIR]/home/.databrickscfg config file) ✓ workspace_id: [NUMID] (from [TEST_TMP_DIR]/home/.databrickscfg config file) - ✓ token: ******** (from DATABRICKS_TOKEN environment variable) + ✓ token: ******** (from [TEST_TMP_DIR]/home/.databrickscfg config file) ✓ profile: acct-with-ws (from --profile flag) ✓ databricks_cli_path: [CLI] ✓ auth_type: pat diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 84c09e8a7ec..8bc99bb08e7 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -195,15 +195,20 @@ func MustAnyClient(cmd *cobra.Command, args []string) (bool, error) { func MustAccountClient(cmd *cobra.Command, args []string) error { cfg := &config.Config{} + ctx := cmd.Context() // The command-line profile flag takes precedence over DATABRICKS_CONFIG_PROFILE. pr, hasProfileFlag := profileFlagValue(cmd) if hasProfileFlag { cfg.Profile = pr + // An explicit --profile must take precedence over authentication + // environment variables; see the matching comment in MustWorkspaceClient + // and issue #5096. + cfg.Loaders = []config.Loader{databrickscfg.ResolveNonAuthFromEnv, config.ConfigFile} + } else { + auth.NormalizeDatabricksConfigFromEnv(ctx, cfg) } - ctx := cmd.Context() - auth.NormalizeDatabricksConfigFromEnv(ctx, cfg) ctx = cmdctx.SetConfigUsed(ctx, cfg) cmd.SetContext(ctx) @@ -324,9 +329,16 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { profile, hasProfileFlag := profileFlagValue(cmd) if hasProfileFlag { cfg.Profile = profile + // An explicit --profile must take precedence over authentication + // environment variables (DATABRICKS_HOST, DATABRICKS_TOKEN, ...). + // The SDK's default loader reads the environment before the config + // file and never overwrites an already-set field, so without this the + // env vars would shadow the selected profile (issue #5096). Load only + // non-auth attributes from the environment, then the profile. + cfg.Loaders = []config.Loader{databrickscfg.ResolveNonAuthFromEnv, config.ConfigFile} + } else { + auth.NormalizeDatabricksConfigFromEnv(ctx, cfg) } - - auth.NormalizeDatabricksConfigFromEnv(ctx, cfg) resolveDefaultProfile(ctx, cfg) _, isTargetFlagSet := targetFlagValue(cmd) diff --git a/cmd/root/auth_test.go b/cmd/root/auth_test.go index b2a85174098..989872f5fb4 100644 --- a/cmd/root/auth_test.go +++ b/cmd/root/auth_test.go @@ -442,6 +442,71 @@ token = flag-token } } +func TestMustWorkspaceClientProfileFlagOverridesAuthEnv(t *testing.T) { + testutil.CleanupEnvironment(t) + + configFile := filepath.Join(t.TempDir(), ".databrickscfg") + err := os.WriteFile(configFile, []byte(` +[tst-svc] +host = https://tst.cloud.databricks.com +token = tst-token +`), 0o600) + require.NoError(t, err) + + t.Setenv("DATABRICKS_CONFIG_FILE", configFile) + // direnv-style auth env vars pointing at a different (dev) workspace. Before + // the fix for #5096 these shadowed the profile selected with --profile. + t.Setenv("DATABRICKS_HOST", "https://dev.cloud.databricks.com") + t.Setenv("DATABRICKS_TOKEN", "dev-token") + + ctx := cmdio.MockDiscard(t.Context()) + ctx = SkipLoadBundle(ctx) + cmd := New(ctx) + + err = cmd.Flag("profile").Value.Set("tst-svc") + require.NoError(t, err) + + err = MustWorkspaceClient(cmd, []string{}) + require.NoError(t, err) + + w := cmdctx.WorkspaceClient(cmd.Context()) + require.NotNil(t, w) + // The explicitly selected profile must win over the auth env vars. + assert.Equal(t, "tst-svc", w.Config.Profile) + assert.Equal(t, "https://tst.cloud.databricks.com", w.Config.Host) + assert.Equal(t, "tst-token", w.Config.Token) +} + +func TestMustAccountClientProfileFlagOverridesAuthEnv(t *testing.T) { + testutil.CleanupEnvironment(t) + + configFile := filepath.Join(t.TempDir(), ".databrickscfg") + err := os.WriteFile(configFile, []byte(` +[acc-tst] +host = https://accounts.azuredatabricks.net/ +account_id = 1111 +token = tst-token +`), 0o600) + require.NoError(t, err) + + t.Setenv("DATABRICKS_CONFIG_FILE", configFile) + t.Setenv("DATABRICKS_HOST", "https://accounts.azuredatabricks.net/") + t.Setenv("DATABRICKS_TOKEN", "dev-token") + + cmd := New(t.Context()) + err = cmd.Flag("profile").Value.Set("acc-tst") + require.NoError(t, err) + + err = MustAccountClient(cmd, []string{}) + require.NoError(t, err) + + a := cmdctx.AccountClient(cmd.Context()) + require.NotNil(t, a) + // The explicitly selected profile must win over the auth env vars. + assert.Equal(t, "acc-tst", a.Config.Profile) + assert.Equal(t, "tst-token", a.Config.Token) +} + func TestAccountClientOrPromptReturnsErrorForWrongHostType(t *testing.T) { testutil.CleanupEnvironment(t) t.Setenv("PATH", "") diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index 5d942b0b7c0..8dd63c387e5 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -14,6 +14,20 @@ import ( var ResolveProfileFromHost = profileFromHostLoader{} +// ResolveNonAuthFromEnv reads configuration from environment variables, except +// for the host and any authentication credential. It is meant to replace the +// SDK's default environment loader when the user has explicitly selected a +// profile (via the --profile flag), so that the profile fully determines the +// host and authentication. +// +// The SDK's default loader order is environment first, then config file, and a +// loader never overwrites a field that is already set. As a result auth env +// vars (DATABRICKS_HOST, DATABRICKS_TOKEN, ...) shadow the selected profile. +// Skipping them here lets the subsequent config-file loader populate host and +// auth from the profile instead. See +// https://github.com/databricks/cli/issues/5096. +var ResolveNonAuthFromEnv = nonAuthEnvLoader{} + var errNoMatchingProfiles = errors.New("no matching config profiles found") type errMultipleProfiles []string @@ -60,6 +74,39 @@ func findMatchingProfile(configFile *config.File, matcher func(*ini.Section) boo return matching[0], nil } +// hostAttrName is the SDK config attribute name for the Databricks host. The +// host has no `auth` struct tag, so it is excluded from auth-only checks by +// name rather than via HasAuthAttribute. +const hostAttrName = "host" + +type nonAuthEnvLoader struct{} + +func (nonAuthEnvLoader) Name() string { + return "environment (excluding auth)" +} + +func (nonAuthEnvLoader) Configure(cfg *config.Config) error { + for _, attr := range config.ConfigAttributes { + // Leave the host and authentication credentials for the config file + // (i.e. the selected profile) to provide. + if attr.Name == hostAttrName || attr.HasAuthAttribute() { + continue + } + // Match the SDK loader semantics: don't overwrite a value previously set. + if !attr.IsZero(cfg) { + continue + } + v, _ := attr.ReadEnv() + if v == "" { + continue + } + if err := attr.SetS(cfg, v); err != nil { + return err + } + } + return nil +} + type profileFromHostLoader struct{} func (l profileFromHostLoader) Name() string { diff --git a/libs/databrickscfg/loader_test.go b/libs/databrickscfg/loader_test.go index 29d5ebfd1cc..0216fe1b924 100644 --- a/libs/databrickscfg/loader_test.go +++ b/libs/databrickscfg/loader_test.go @@ -34,6 +34,22 @@ func TestLoaderSkipsExistingAuth(t *testing.T) { assert.NoError(t, err) } +func TestResolveNonAuthFromEnvSkipsHostAndAuth(t *testing.T) { + t.Setenv("DATABRICKS_HOST", "https://env.test") + t.Setenv("DATABRICKS_TOKEN", "env-token") + t.Setenv("DATABRICKS_CLUSTER_ID", "env-cluster") + + cfg := &config.Config{} + err := ResolveNonAuthFromEnv.Configure(cfg) + require.NoError(t, err) + + // Host and auth credentials are left for the profile (config file) to set. + assert.Empty(t, cfg.Host) + assert.Empty(t, cfg.Token) + // Non-auth attributes are still populated from the environment. + assert.Equal(t, "env-cluster", cfg.ClusterID) +} + func TestLoaderSkipsExplicitAuthType(t *testing.T) { cfg := config.Config{ Loaders: []config.Loader{ From b244522bbbd715a0f9f2bda8a6d5ba72ac03c4fe Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Wed, 24 Jun 2026 12:25:58 +0000 Subject: [PATCH 2/8] Cover bundle profile and auth_type/discovery_url env precedence Extend the --profile precedence fix (#5096): - ResolveNonAuthFromEnv now also skips auth_type and discovery_url, which are tagged auth:"-" in the SDK and so are invisible to HasAuthAttribute, letting DATABRICKS_AUTH_TYPE/DATABRICKS_DISCOVERY_URL shadow the profile. It also records the env source so `auth describe` and debug output match the SDK loader. - Workspace.Client uses ResolveNonAuthFromEnv when a profile is set (from --profile or workspace.profile) so env auth vars no longer shadow the profile for bundle commands. - Use the reserved .test TLD for new test fixture hosts so the SDK's well-known host metadata resolver fast-fails instead of stalling on a live network lookup. --- bundle/config/workspace.go | 16 +++++++++++++--- bundle/config/workspace_test.go | 25 +++++++++++++++++++++++++ cmd/root/auth_test.go | 12 +++++++----- libs/databrickscfg/loader.go | 27 ++++++++++++++++++++------- libs/databrickscfg/loader_test.go | 8 +++++++- 5 files changed, 72 insertions(+), 16 deletions(-) diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index 8b869213435..844a5a813e5 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -182,9 +182,19 @@ func (w *Workspace) Client(ctx context.Context) (*databricks.WorkspaceClient, er cfg := w.Config(ctx) - // If only the host is configured, we try and unambiguously match it to - // a profile in the user's databrickscfg file. Override the default loaders. - if w.Host != "" && w.Profile == "" { + switch { + case w.Profile != "": + // An explicit profile (from --profile or workspace.profile) must take + // precedence over authentication environment variables, mirroring + // MustWorkspaceClient. The SDK's default loader reads the environment + // before the config file and never overwrites an already-set field, so + // without this DATABRICKS_HOST/DATABRICKS_TOKEN would shadow the + // selected profile (issue #5096). Load only non-auth attributes from + // the environment, then the profile. + cfg.Loaders = []config.Loader{databrickscfg.ResolveNonAuthFromEnv, config.ConfigFile} + case w.Host != "": + // If only the host is configured, we try and unambiguously match it to + // a profile in the user's databrickscfg file. Override the default loaders. cfg.Loaders = []config.Loader{ // Load auth creds from env vars config.ConfigAttributes, diff --git a/bundle/config/workspace_test.go b/bundle/config/workspace_test.go index b1898db77c8..83c15f3ac2b 100644 --- a/bundle/config/workspace_test.go +++ b/bundle/config/workspace_test.go @@ -155,6 +155,31 @@ func TestWorkspaceClientNormalizesHostBeforeProfileResolution(t *testing.T) { assert.Equal(t, "ws2", client.Config.Profile) } +func TestWorkspaceClientProfileOverridesAuthEnv(t *testing.T) { + // An explicit profile (from --profile or workspace.profile) must win over + // authentication environment variables, mirroring MustWorkspaceClient. + // See https://github.com/databricks/cli/issues/5096. + setupWorkspaceTest(t) + + err := databrickscfg.SaveToProfile(t.Context(), &config.Config{ + Profile: "tst", + Host: "https://tst.cloud.databricks.test", + Token: "tst-token", + }) + require.NoError(t, err) + + // direnv-style auth env vars pointing at a different (dev) workspace. + t.Setenv("DATABRICKS_HOST", "https://dev.cloud.databricks.test") + t.Setenv("DATABRICKS_TOKEN", "dev-token") + + w := Workspace{Profile: "tst"} + client, err := w.Client(t.Context()) + require.NoError(t, err) + assert.Equal(t, "tst", client.Config.Profile) + assert.Equal(t, "https://tst.cloud.databricks.test", client.Config.Host) + assert.Equal(t, "tst-token", client.Config.Token) +} + func TestWorkspaceConfigHTTPTimeout(t *testing.T) { for _, tc := range []struct { envVal string diff --git a/cmd/root/auth_test.go b/cmd/root/auth_test.go index 989872f5fb4..d1dc7126261 100644 --- a/cmd/root/auth_test.go +++ b/cmd/root/auth_test.go @@ -448,7 +448,7 @@ func TestMustWorkspaceClientProfileFlagOverridesAuthEnv(t *testing.T) { configFile := filepath.Join(t.TempDir(), ".databrickscfg") err := os.WriteFile(configFile, []byte(` [tst-svc] -host = https://tst.cloud.databricks.com +host = https://tst.cloud.databricks.test token = tst-token `), 0o600) require.NoError(t, err) @@ -456,7 +456,7 @@ token = tst-token t.Setenv("DATABRICKS_CONFIG_FILE", configFile) // direnv-style auth env vars pointing at a different (dev) workspace. Before // the fix for #5096 these shadowed the profile selected with --profile. - t.Setenv("DATABRICKS_HOST", "https://dev.cloud.databricks.com") + t.Setenv("DATABRICKS_HOST", "https://dev.cloud.databricks.test") t.Setenv("DATABRICKS_TOKEN", "dev-token") ctx := cmdio.MockDiscard(t.Context()) @@ -473,7 +473,7 @@ token = tst-token require.NotNil(t, w) // The explicitly selected profile must win over the auth env vars. assert.Equal(t, "tst-svc", w.Config.Profile) - assert.Equal(t, "https://tst.cloud.databricks.com", w.Config.Host) + assert.Equal(t, "https://tst.cloud.databricks.test", w.Config.Host) assert.Equal(t, "tst-token", w.Config.Token) } @@ -483,14 +483,15 @@ func TestMustAccountClientProfileFlagOverridesAuthEnv(t *testing.T) { configFile := filepath.Join(t.TempDir(), ".databrickscfg") err := os.WriteFile(configFile, []byte(` [acc-tst] -host = https://accounts.azuredatabricks.net/ +host = https://accounts.cloud.databricks.test account_id = 1111 token = tst-token `), 0o600) require.NoError(t, err) t.Setenv("DATABRICKS_CONFIG_FILE", configFile) - t.Setenv("DATABRICKS_HOST", "https://accounts.azuredatabricks.net/") + // Auth env vars pointing at a different account host. The profile must win. + t.Setenv("DATABRICKS_HOST", "https://accounts.dev.databricks.test") t.Setenv("DATABRICKS_TOKEN", "dev-token") cmd := New(t.Context()) @@ -504,6 +505,7 @@ token = tst-token require.NotNil(t, a) // The explicitly selected profile must win over the auth env vars. assert.Equal(t, "acc-tst", a.Config.Profile) + assert.Equal(t, "https://accounts.cloud.databricks.test", a.Config.Host) assert.Equal(t, "tst-token", a.Config.Token) } diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index 8dd63c387e5..13a3d7b7da2 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -74,10 +74,20 @@ func findMatchingProfile(configFile *config.File, matcher func(*ini.Section) boo return matching[0], nil } -// hostAttrName is the SDK config attribute name for the Databricks host. The -// host has no `auth` struct tag, so it is excluded from auth-only checks by -// name rather than via HasAuthAttribute. -const hostAttrName = "host" +// nonAuthEnvSkipAttrs lists SDK config attribute names that nonAuthEnvLoader +// must not read from the environment, beyond those caught by HasAuthAttribute. +// +// - host: has no `auth` struct tag, so HasAuthAttribute can't see it. +// - auth_type, discovery_url: tagged `auth:"-"` in the SDK, which the SDK +// normalizes to an empty auth tag (internal), so HasAuthAttribute reports +// false even though both select/steer the authentication method. Leaving +// them to the env would let DATABRICKS_AUTH_TYPE / DATABRICKS_DISCOVERY_URL +// shadow the selected profile, the same bug as #5096. +var nonAuthEnvSkipAttrs = map[string]bool{ + "host": true, + "auth_type": true, + "discovery_url": true, +} type nonAuthEnvLoader struct{} @@ -87,22 +97,25 @@ func (nonAuthEnvLoader) Name() string { func (nonAuthEnvLoader) Configure(cfg *config.Config) error { for _, attr := range config.ConfigAttributes { - // Leave the host and authentication credentials for the config file + // Leave the host and authentication settings for the config file // (i.e. the selected profile) to provide. - if attr.Name == hostAttrName || attr.HasAuthAttribute() { + if nonAuthEnvSkipAttrs[attr.Name] || attr.HasAuthAttribute() { continue } // Match the SDK loader semantics: don't overwrite a value previously set. if !attr.IsZero(cfg) { continue } - v, _ := attr.ReadEnv() + v, envName := attr.ReadEnv() if v == "" { continue } if err := attr.SetS(cfg, v); err != nil { return err } + // Record the source so `databricks auth describe` and debug output + // attribute the value to the environment, matching the SDK loader. + cfg.SetAttrSource(&attr, config.Source{Type: config.SourceEnv, Name: envName}) } return nil } diff --git a/libs/databrickscfg/loader_test.go b/libs/databrickscfg/loader_test.go index 0216fe1b924..a8a03864939 100644 --- a/libs/databrickscfg/loader_test.go +++ b/libs/databrickscfg/loader_test.go @@ -37,15 +37,21 @@ func TestLoaderSkipsExistingAuth(t *testing.T) { func TestResolveNonAuthFromEnvSkipsHostAndAuth(t *testing.T) { t.Setenv("DATABRICKS_HOST", "https://env.test") t.Setenv("DATABRICKS_TOKEN", "env-token") + // auth_type and discovery_url are tagged auth:"-" in the SDK, so + // HasAuthAttribute can't catch them; they must still be skipped (#5096). + t.Setenv("DATABRICKS_AUTH_TYPE", "oauth-m2m") + t.Setenv("DATABRICKS_DISCOVERY_URL", "https://discovery.env.test") t.Setenv("DATABRICKS_CLUSTER_ID", "env-cluster") cfg := &config.Config{} err := ResolveNonAuthFromEnv.Configure(cfg) require.NoError(t, err) - // Host and auth credentials are left for the profile (config file) to set. + // Host and auth settings are left for the profile (config file) to set. assert.Empty(t, cfg.Host) assert.Empty(t, cfg.Token) + assert.Empty(t, cfg.AuthType) + assert.Empty(t, cfg.DiscoveryURL) // Non-auth attributes are still populated from the environment. assert.Equal(t, "env-cluster", cfg.ClusterID) } From 68a07f709d324d45885dde65d3977b5506b08cf6 Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Wed, 24 Jun 2026 13:04:33 +0000 Subject: [PATCH 3/8] Let env fill auth fields a selected profile leaves empty A host-only profile combined with DATABRICKS_TOKEN previously failed because the profile loader chain stopped at the config file. Append config.ConfigAttributes after the profile so the environment can fill auth fields the profile does not provide, while the profile still wins for any field it sets (#5096). --- bundle/config/workspace.go | 12 ++++++++--- bundle/config/workspace_test.go | 23 ++++++++++++++++++++++ cmd/root/auth.go | 33 ++++++++++++++++++++++++------- cmd/root/auth_test.go | 35 +++++++++++++++++++++++++++++++++ libs/databrickscfg/loader.go | 12 ++++++----- 5 files changed, 100 insertions(+), 15 deletions(-) diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index 844a5a813e5..2915ba17d47 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -189,9 +189,15 @@ func (w *Workspace) Client(ctx context.Context) (*databricks.WorkspaceClient, er // MustWorkspaceClient. The SDK's default loader reads the environment // before the config file and never overwrites an already-set field, so // without this DATABRICKS_HOST/DATABRICKS_TOKEN would shadow the - // selected profile (issue #5096). Load only non-auth attributes from - // the environment, then the profile. - cfg.Loaders = []config.Loader{databrickscfg.ResolveNonAuthFromEnv, config.ConfigFile} + // selected profile (issue #5096). Load non-auth attributes from the + // environment, then the profile, then let the environment fill any auth + // fields the profile did not provide (e.g. a host-only profile combined + // with DATABRICKS_TOKEN). + cfg.Loaders = []config.Loader{ + databrickscfg.ResolveNonAuthFromEnv, + config.ConfigFile, + config.ConfigAttributes, + } case w.Host != "": // If only the host is configured, we try and unambiguously match it to // a profile in the user's databrickscfg file. Override the default loaders. diff --git a/bundle/config/workspace_test.go b/bundle/config/workspace_test.go index 83c15f3ac2b..724137f33f8 100644 --- a/bundle/config/workspace_test.go +++ b/bundle/config/workspace_test.go @@ -180,6 +180,29 @@ func TestWorkspaceClientProfileOverridesAuthEnv(t *testing.T) { assert.Equal(t, "tst-token", client.Config.Token) } +func TestWorkspaceClientProfileFillsAuthFromEnv(t *testing.T) { + // A host-only profile relies on the environment for credentials. The profile + // must take precedence for the host, but the env must still fill the token + // the profile does not provide (#5096). + setupWorkspaceTest(t) + + err := databrickscfg.SaveToProfile(t.Context(), &config.Config{ + Profile: "host-only", + Host: "https://tst.cloud.databricks.test", + }) + require.NoError(t, err) + + t.Setenv("DATABRICKS_TOKEN", "env-token") + + w := Workspace{Profile: "host-only"} + client, err := w.Client(t.Context()) + require.NoError(t, err) + assert.Equal(t, "host-only", client.Config.Profile) + assert.Equal(t, "https://tst.cloud.databricks.test", client.Config.Host) + // The token is not in the profile, so it is filled from the environment. + assert.Equal(t, "env-token", client.Config.Token) +} + func TestWorkspaceConfigHTTPTimeout(t *testing.T) { for _, tc := range []struct { envVal string diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 8bc99bb08e7..32884237d9a 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -31,6 +31,28 @@ import ( // SDK removed the variable entirely in v0.127.0, so we now own it here. var errNotWorkspaceClient = errors.New("invalid Databricks Workspace configuration - host is not a workspace host") +// profileAuthLoaders is the SDK loader chain to use when the user explicitly +// selects a profile (via --profile or workspace.profile). The selected profile +// must determine the host and authentication, taking precedence over auth +// environment variables (DATABRICKS_HOST, DATABRICKS_TOKEN, ...). The SDK's +// default chain reads the environment before the config file and never +// overwrites an already-set field, so the env vars would otherwise shadow the +// profile (issue #5096). +// +// The order matters: +// 1. ResolveNonAuthFromEnv loads non-auth attributes from the environment +// (e.g. cluster_id), preserving the env-wins precedence for those. +// 2. ConfigFile loads the selected profile, populating host and auth. +// 3. ConfigAttributes loads any remaining attributes from the environment, +// filling auth fields the profile did not provide (e.g. a host-only +// profile combined with DATABRICKS_TOKEN). It never overwrites a value the +// profile already set, so the profile still wins for #5096. +var profileAuthLoaders = []config.Loader{ + databrickscfg.ResolveNonAuthFromEnv, + config.ConfigFile, + config.ConfigAttributes, +} + type ErrNoWorkspaceProfiles struct { path string } @@ -204,7 +226,7 @@ func MustAccountClient(cmd *cobra.Command, args []string) error { // An explicit --profile must take precedence over authentication // environment variables; see the matching comment in MustWorkspaceClient // and issue #5096. - cfg.Loaders = []config.Loader{databrickscfg.ResolveNonAuthFromEnv, config.ConfigFile} + cfg.Loaders = profileAuthLoaders } else { auth.NormalizeDatabricksConfigFromEnv(ctx, cfg) } @@ -330,12 +352,9 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { if hasProfileFlag { cfg.Profile = profile // An explicit --profile must take precedence over authentication - // environment variables (DATABRICKS_HOST, DATABRICKS_TOKEN, ...). - // The SDK's default loader reads the environment before the config - // file and never overwrites an already-set field, so without this the - // env vars would shadow the selected profile (issue #5096). Load only - // non-auth attributes from the environment, then the profile. - cfg.Loaders = []config.Loader{databrickscfg.ResolveNonAuthFromEnv, config.ConfigFile} + // environment variables (DATABRICKS_HOST, DATABRICKS_TOKEN, ...); + // see profileAuthLoaders and issue #5096. + cfg.Loaders = profileAuthLoaders } else { auth.NormalizeDatabricksConfigFromEnv(ctx, cfg) } diff --git a/cmd/root/auth_test.go b/cmd/root/auth_test.go index d1dc7126261..8f4274521ea 100644 --- a/cmd/root/auth_test.go +++ b/cmd/root/auth_test.go @@ -477,6 +477,41 @@ token = tst-token assert.Equal(t, "tst-token", w.Config.Token) } +func TestMustWorkspaceClientProfileFlagFillsAuthFromEnv(t *testing.T) { + testutil.CleanupEnvironment(t) + + // A host-only profile relies on the environment for credentials. This is a + // common CI pattern: the host lives in .databrickscfg while DATABRICKS_TOKEN + // is injected by the runner. The profile must take precedence for the host, + // but the env must still fill the token the profile does not provide (#5096). + configFile := filepath.Join(t.TempDir(), ".databrickscfg") + err := os.WriteFile(configFile, []byte(` +[host-only] +host = https://tst.cloud.databricks.test +`), 0o600) + require.NoError(t, err) + + t.Setenv("DATABRICKS_CONFIG_FILE", configFile) + t.Setenv("DATABRICKS_TOKEN", "env-token") + + ctx := cmdio.MockDiscard(t.Context()) + ctx = SkipLoadBundle(ctx) + cmd := New(ctx) + + err = cmd.Flag("profile").Value.Set("host-only") + require.NoError(t, err) + + err = MustWorkspaceClient(cmd, []string{}) + require.NoError(t, err) + + w := cmdctx.WorkspaceClient(cmd.Context()) + require.NotNil(t, w) + assert.Equal(t, "host-only", w.Config.Profile) + assert.Equal(t, "https://tst.cloud.databricks.test", w.Config.Host) + // The token is not in the profile, so it is filled from the environment. + assert.Equal(t, "env-token", w.Config.Token) +} + func TestMustAccountClientProfileFlagOverridesAuthEnv(t *testing.T) { testutil.CleanupEnvironment(t) diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index 13a3d7b7da2..a5e6c91844e 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -15,16 +15,18 @@ import ( var ResolveProfileFromHost = profileFromHostLoader{} // ResolveNonAuthFromEnv reads configuration from environment variables, except -// for the host and any authentication credential. It is meant to replace the -// SDK's default environment loader when the user has explicitly selected a -// profile (via the --profile flag), so that the profile fully determines the -// host and authentication. +// for the host and any authentication credential. It runs before the config +// file loader when the user has explicitly selected a profile (via the +// --profile flag or workspace.profile), so that the profile takes precedence +// over auth environment variables. // // The SDK's default loader order is environment first, then config file, and a // loader never overwrites a field that is already set. As a result auth env // vars (DATABRICKS_HOST, DATABRICKS_TOKEN, ...) shadow the selected profile. // Skipping them here lets the subsequent config-file loader populate host and -// auth from the profile instead. See +// auth from the profile instead. A trailing config.ConfigAttributes loader can +// still fill auth fields the profile leaves empty (e.g. a host-only profile +// combined with DATABRICKS_TOKEN). See // https://github.com/databricks/cli/issues/5096. var ResolveNonAuthFromEnv = nonAuthEnvLoader{} From 72e217eef992d03c542d1ec191c2e669d2796895 Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Fri, 26 Jun 2026 08:49:31 +0000 Subject: [PATCH 4/8] Refine profile precedence handling and extend coverage - Centralize the explicit-profile loader chain in databrickscfg.ProfileAuthLoaders and extract applyProfileAuthPrecedence so all call sites share one rule. - Skip host, routing IDs (workspace_id/account_id) and SDK-internal auth-steering env attrs; guard the classification with a test that fails on SDK drift. - Apply profile precedence to `databricks api --profile`. - Let env gap-fill auth fields a host-only profile leaves empty. - Add bundle host+profile coverage and acceptance tests; clarify rationale comments. --- NEXT_CHANGELOG.md | 2 +- .../api/profile-overrides-env/out.test.toml | 3 + .../cmd/api/profile-overrides-env/output.txt | 44 +++++++++++ .../cmd/api/profile-overrides-env/script | 29 ++++++++ .../cmd/api/profile-overrides-env/test.toml | 3 + .../profile-overrides-env/out.test.toml | 3 + .../describe/profile-overrides-env/output.txt | 36 +++++++++ .../describe/profile-overrides-env/script | 26 +++++++ .../describe/profile-overrides-env/test.toml | 3 + bundle/config/workspace.go | 19 ++--- bundle/config/workspace_test.go | 38 ++++++++-- cmd/api/api.go | 14 +++- cmd/root/auth.go | 55 ++++++-------- cmd/root/auth_test.go | 50 ++++++++++--- libs/databrickscfg/loader.go | 73 +++++++++++++------ libs/databrickscfg/loader_test.go | 62 +++++++++++++++- 16 files changed, 369 insertions(+), 91 deletions(-) create mode 100644 acceptance/cmd/api/profile-overrides-env/out.test.toml create mode 100644 acceptance/cmd/api/profile-overrides-env/output.txt create mode 100644 acceptance/cmd/api/profile-overrides-env/script create mode 100644 acceptance/cmd/api/profile-overrides-env/test.toml create mode 100644 acceptance/cmd/auth/describe/profile-overrides-env/out.test.toml create mode 100644 acceptance/cmd/auth/describe/profile-overrides-env/output.txt create mode 100644 acceptance/cmd/auth/describe/profile-overrides-env/script create mode 100644 acceptance/cmd/auth/describe/profile-overrides-env/test.toml diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 4f074566ddd..6e6119c4c34 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -6,7 +6,7 @@ ### CLI -* An explicit `--profile` now takes precedence over authentication environment variables (`DATABRICKS_HOST`, `DATABRICKS_TOKEN`, etc.). Previously these env vars silently shadowed the selected profile's host and credentials ([#5096](https://github.com/databricks/cli/issues/5096)). +* An explicitly selected profile (`--profile` or a bundle's `workspace.profile`) now takes precedence over auth environment variables (`DATABRICKS_HOST`, `DATABRICKS_TOKEN`, etc.) instead of being silently shadowed by them; env vars still fill auth fields the profile leaves empty ([#5096](https://github.com/databricks/cli/issues/5096)). ### Bundles diff --git a/acceptance/cmd/api/profile-overrides-env/out.test.toml b/acceptance/cmd/api/profile-overrides-env/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/cmd/api/profile-overrides-env/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/profile-overrides-env/output.txt b/acceptance/cmd/api/profile-overrides-env/output.txt new file mode 100644 index 00000000000..f56150d9b88 --- /dev/null +++ b/acceptance/cmd/api/profile-overrides-env/output.txt @@ -0,0 +1,44 @@ + +=== api --profile overrides auth env vars (#5096) + +>>> [CLI] api get /api/2.0/clusters/list --profile my-workspace +{} + +>>> print_requests.py --get //api/2.0/clusters/list +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ], + "X-Databricks-Workspace-Id": [ + "[NUMID]" + ] + }, + "method": "GET", + "path": "/api/2.0/clusters/list" +} + +=== api host-only --profile fills the token from the environment (#5096) + +>>> [CLI] api get /api/2.0/clusters/list --profile host-only +{} + +>>> print_requests.py --get //api/2.0/clusters/list +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ], + "X-Databricks-Workspace-Id": [ + "[NUMID]" + ] + }, + "method": "GET", + "path": "/api/2.0/clusters/list" +} diff --git a/acceptance/cmd/api/profile-overrides-env/script b/acceptance/cmd/api/profile-overrides-env/script new file mode 100644 index 00000000000..a1ec0315dce --- /dev/null +++ b/acceptance/cmd/api/profile-overrides-env/script @@ -0,0 +1,29 @@ +sethome "./home" + +# One profile with full credentials, one host-only; both point at the test +# server while the auth env vars below point elsewhere. +cat > "./home/.databrickscfg" <>> [CLI] auth describe --profile my-workspace +Host: [DATABRICKS_URL] +User: [USERNAME] +Authenticated with: pat +----- +Current configuration: + ✓ host: [DATABRICKS_URL] (from [TEST_TMP_DIR]/home/.databrickscfg config file) + ✓ workspace_id: [NUMID] + ✓ token: ******** (from [TEST_TMP_DIR]/home/.databrickscfg config file) + ✓ profile: my-workspace (from --profile flag) + ✓ databricks_cli_path: [CLI] + ✓ auth_type: pat + ✓ rate_limit: [NUMID] (from DATABRICKS_RATE_LIMIT environment variable) + ✓ cloud: AWS + ✓ discovery_url: [DATABRICKS_URL]/oidc/.well-known/oauth-authorization-server + +=== Describe with a host-only --profile fills the token from the environment (#5096) + +>>> [CLI] auth describe --profile host-only +Host: [DATABRICKS_URL] +User: [USERNAME] +Authenticated with: pat +----- +Current configuration: + ✓ host: [DATABRICKS_URL] (from [TEST_TMP_DIR]/home/.databrickscfg config file) + ✓ workspace_id: [NUMID] + ✓ token: ******** (from DATABRICKS_TOKEN environment variable) + ✓ profile: host-only (from --profile flag) + ✓ databricks_cli_path: [CLI] + ✓ auth_type: pat + ✓ rate_limit: [NUMID] (from DATABRICKS_RATE_LIMIT environment variable) + ✓ cloud: AWS + ✓ discovery_url: [DATABRICKS_URL]/oidc/.well-known/oauth-authorization-server diff --git a/acceptance/cmd/auth/describe/profile-overrides-env/script b/acceptance/cmd/auth/describe/profile-overrides-env/script new file mode 100644 index 00000000000..b3846922c8f --- /dev/null +++ b/acceptance/cmd/auth/describe/profile-overrides-env/script @@ -0,0 +1,26 @@ +sethome "./home" + +# A profile carries full credentials; a second profile carries only a host. +cat > "./home/.databrickscfg" < Date: Fri, 26 Jun 2026 11:03:25 +0000 Subject: [PATCH 5/8] Trim verbose comments to keep only necessary rationale --- bundle/config/workspace.go | 6 +-- bundle/config/workspace_test.go | 8 ++-- cmd/api/api.go | 6 +-- cmd/root/auth.go | 14 ++----- cmd/root/auth_test.go | 7 ++-- libs/databrickscfg/loader.go | 64 +++++++++---------------------- libs/databrickscfg/loader_test.go | 16 +++----- 7 files changed, 38 insertions(+), 83 deletions(-) diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index 28134421c10..f68670b7e86 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -184,10 +184,8 @@ func (w *Workspace) Client(ctx context.Context) (*databricks.WorkspaceClient, er switch { case w.Profile != "": - // An explicit profile (--profile or workspace.profile) takes precedence - // over auth env vars; see databrickscfg.ProfileAuthLoaders (#5096). The - // ValidateConfigAndProfileHost check below still enforces that the bundle - // host matches the profile host. + // An explicit profile wins over auth env vars (#5096). + // ValidateConfigAndProfileHost below still checks host agreement. cfg.Loaders = databrickscfg.ProfileAuthLoaders case w.Host != "": // If only the host is configured, we try and unambiguously match it to diff --git a/bundle/config/workspace_test.go b/bundle/config/workspace_test.go index 542d8e0d232..3fb86da2b64 100644 --- a/bundle/config/workspace_test.go +++ b/bundle/config/workspace_test.go @@ -156,8 +156,7 @@ func TestWorkspaceClientNormalizesHostBeforeProfileResolution(t *testing.T) { } func TestWorkspaceClientProfileOverridesAuthEnv(t *testing.T) { - // An explicit profile (--profile or workspace.profile) must win over auth - // env vars, mirroring MustWorkspaceClient (#5096). + // An explicit profile must win over auth env vars (#5096). setupWorkspaceTest(t) err := databrickscfg.SaveToProfile(t.Context(), &config.Config{ @@ -202,9 +201,8 @@ func TestWorkspaceClientProfileFillsAuthFromEnv(t *testing.T) { } func TestWorkspaceClientHostAndProfileOverridesAuthEnv(t *testing.T) { - // A bundle that pins both workspace.host and workspace.profile: the profile - // still wins for auth over the env vars, the bundle host is honored, and - // ValidateConfigAndProfileHost passes because they agree (#5096). + // Bundle pins both workspace.host and workspace.profile: the profile wins + // for auth and the host check passes because they agree (#5096). setupWorkspaceTest(t) err := databrickscfg.SaveToProfile(t.Context(), &config.Config{ diff --git a/cmd/api/api.go b/cmd/api/api.go index 5271dd9f4dc..7526f635ffb 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -98,10 +98,8 @@ func makeCommand(method string) *cobra.Command { } if hasProfileFlag { - // An explicit --profile takes precedence over auth env vars; see - // databrickscfg.ProfileAuthLoaders (#5096). NormalizeDatabricksConfigFromEnv - // is skipped too: the host comes from the profile, not DATABRICKS_HOST, - // so promoting its ?o=/?a= query params would be wrong. + // An explicit --profile wins over auth env vars (#5096); the host + // comes from the profile, so skip env host normalization. cfg.Loaders = databrickscfg.ProfileAuthLoaders } else { auth.NormalizeDatabricksConfigFromEnv(cmd.Context(), cfg) diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 62818fbbcfc..1f7e2f7984f 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -110,17 +110,9 @@ func profileFlagValue(cmd *cobra.Command) (string, bool) { return value, value != "" } -// applyProfileAuthPrecedence makes an explicitly selected --profile take -// precedence over auth environment variables instead of being shadowed by them. -// -// With a profile flag we install databrickscfg.ProfileAuthLoaders (the single -// source of truth for this precedence rule; see its doc and #5096) and skip -// NormalizeDatabricksConfigFromEnv: the host comes from the profile, not -// DATABRICKS_HOST, so promoting that env host's ?o=/?a= query params would be -// wrong. Trade-off: a host-less profile combined with a SPOG-style -// DATABRICKS_HOST (?o=123) no longer extracts the workspace_id from the query. -// -// Without a profile flag we keep the SDK's env-first behavior. +// applyProfileAuthPrecedence makes an explicit --profile win over auth env vars +// via ProfileAuthLoaders (#5096), skipping env host normalization since the host +// comes from the profile. Without a profile flag, env-first behavior is kept. func applyProfileAuthPrecedence(ctx context.Context, cfg *config.Config, hasProfileFlag bool) { if hasProfileFlag { cfg.Loaders = databrickscfg.ProfileAuthLoaders diff --git a/cmd/root/auth_test.go b/cmd/root/auth_test.go index 59d74dc0996..29ddc89c728 100644 --- a/cmd/root/auth_test.go +++ b/cmd/root/auth_test.go @@ -454,8 +454,7 @@ token = tst-token require.NoError(t, err) t.Setenv("DATABRICKS_CONFIG_FILE", configFile) - // direnv-style auth env vars pointing at a different workspace. Before #5096 - // these shadowed the profile selected with --profile. + // Auth env vars for a different workspace; before #5096 these shadowed --profile. t.Setenv("DATABRICKS_HOST", "https://dev.cloud.databricks.test") t.Setenv("DATABRICKS_TOKEN", "dev-token") @@ -522,8 +521,8 @@ token = tst-token require.NoError(t, err) t.Setenv("DATABRICKS_CONFIG_FILE", configFile) - // Selected via DATABRICKS_CONFIG_PROFILE, not --profile: this keeps the SDK's - // env-first precedence, so the auth env vars below must still win (#5096). + // Selected via DATABRICKS_CONFIG_PROFILE, not --profile, so env-first + // precedence is kept and the auth env vars below still win (#5096). t.Setenv("DATABRICKS_CONFIG_PROFILE", "tst-svc") t.Setenv("DATABRICKS_HOST", "https://dev.cloud.databricks.test") t.Setenv("DATABRICKS_TOKEN", "dev-token") diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index 77d950a23e2..afb100db6ad 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -14,31 +14,20 @@ import ( var ResolveProfileFromHost = profileFromHostLoader{} -// ResolveNonAuthFromEnv reads config from environment variables, except for the -// host and any auth credential. It runs before the config file loader when a -// profile is explicitly selected, so the profile wins over auth env vars: the -// SDK's default chain reads env before the config file and never overwrites a -// set field, which would otherwise let env vars shadow the profile (#5096). +// ResolveNonAuthFromEnv reads non-auth, non-host config from environment +// variables. See ProfileAuthLoaders for how it fits into the chain. var ResolveNonAuthFromEnv = nonAuthEnvLoader{} -// ProfileAuthLoaders is the loader chain to use when a profile is explicitly -// selected (--profile or a bundle's workspace.profile), and the single source -// of truth for that precedence rule. The profile must win for host, routing and -// auth over the matching env vars (DATABRICKS_HOST, DATABRICKS_TOKEN, ...), -// which the SDK's default env-first chain would otherwise let shadow it (#5096). +// ProfileAuthLoaders is the loader chain for an explicitly selected profile +// (--profile or a bundle's workspace.profile). Unlike the SDK's default +// env-first chain, the profile wins over auth env vars (#5096): // -// This only governs an explicitly selected profile. One picked up from -// DATABRICKS_CONFIG_PROFILE keeps env-first precedence: reordering two -// environment signals (DATABRICKS_CONFIG_PROFILE vs DATABRICKS_HOST) is the -// SDK's domain; we only override when the profile is named out-of-band. -// -// Order: -// 1. ResolveNonAuthFromEnv: non-auth, non-routing env attrs (e.g. cluster_id), -// keeping env-wins precedence for those. +// 1. ResolveNonAuthFromEnv: non-auth env attrs (e.g. cluster_id), env-wins. // 2. ConfigFile: the selected profile (host, routing, auth). -// 3. ConfigAttributes: gap-fills only fields the profile left empty (e.g. a -// host-only profile + DATABRICKS_TOKEN, a common CI pattern); it never -// overwrites a profile value, so the profile still wins. +// 3. ConfigAttributes: gap-fills only fields the profile left empty. +// +// A profile from DATABRICKS_CONFIG_PROFILE keeps env-first precedence; only an +// out-of-band profile name triggers this chain. var ProfileAuthLoaders = []config.Loader{ ResolveNonAuthFromEnv, config.ConfigFile, @@ -91,25 +80,12 @@ func findMatchingProfile(configFile *config.File, matcher func(*ini.Section) boo return matching[0], nil } -// nonAuthEnvSkipAttrs lists SDK config attributes nonAuthEnvLoader must not read -// from the environment, beyond those caught by HasAuthAttribute. They identify -// the target workspace/account (host, routing IDs) or steer the auth method but -// are tagged auth:"-" (collapsed to Internal), so HasAuthAttribute misses them; -// leaving them to env would let an env var shadow the selected profile (#5096). -// Skipping only changes precedence: the trailing ConfigAttributes loader still -// gap-fills any the profile leaves empty. -// -// - host: no `auth` tag at all. -// - workspace_id / account_id: routing identifiers; an env var must not route -// the profile's credentials elsewhere. -// - auth_type: forces a specific auth method. -// - discovery_url: redirects OIDC discovery. -// - audience: selects the OIDC/workload-identity token audience. -// - cloud: steers cloud-specific auth (Azure/GCP/AWS). -// -// Non-auth attrs tagged auth:"-" (oauth_callback_port, debug_headers, ...) are -// intentionally not skipped; TestNonAuthEnvSkipAttrsCoverSDKInternalEnvAttrs -// guards that every auth-steering internal attribute stays classified. +// nonAuthEnvSkipAttrs lists env attributes nonAuthEnvLoader must skip on top of +// those HasAuthAttribute catches: host/routing (host, workspace_id, account_id) +// and auth-steering fields tagged auth:"-" (auth_type, discovery_url, audience, +// cloud), which HasAuthAttribute misses. Reading these from env would shadow the +// selected profile (#5096). TestNonAuthEnvSkipAttrsCoverSDKInternalEnvAttrs +// guards against SDK drift. var nonAuthEnvSkipAttrs = map[string]bool{ "host": true, "workspace_id": true, @@ -128,12 +104,11 @@ func (nonAuthEnvLoader) Name() string { func (nonAuthEnvLoader) Configure(cfg *config.Config) error { for _, attr := range config.ConfigAttributes { - // Leave the host and authentication settings for the config file - // (i.e. the selected profile) to provide. + // Host and auth come from the profile (config file), not env. if nonAuthEnvSkipAttrs[attr.Name] || attr.HasAuthAttribute() { continue } - // Match the SDK loader semantics: don't overwrite a value previously set. + // Don't overwrite an already-set value (SDK loader semantics). if !attr.IsZero(cfg) { continue } @@ -144,8 +119,7 @@ func (nonAuthEnvLoader) Configure(cfg *config.Config) error { if err := attr.SetS(cfg, v); err != nil { return err } - // Record the source so `databricks auth describe` and debug output - // attribute the value to the environment, matching the SDK loader. + // Record the source so `auth describe` attributes the value to env. cfg.SetAttrSource(&attr, config.Source{Type: config.SourceEnv, Name: envName}) } return nil diff --git a/libs/databrickscfg/loader_test.go b/libs/databrickscfg/loader_test.go index d170d60fce1..cbd1cbff2d2 100644 --- a/libs/databrickscfg/loader_test.go +++ b/libs/databrickscfg/loader_test.go @@ -66,11 +66,9 @@ func TestResolveNonAuthFromEnvSkipsHostAndAuth(t *testing.T) { } func TestProfileAuthLoadersConflictingEnvAuthMethodErrors(t *testing.T) { - // The profile provides a complete PAT, while the env provides a different - // complete method (OAuth M2M). The gap-fill loads the empty client_id/secret - // from env, so the resolved config carries two auth methods and the SDK - // rejects it: the profile wins for the fields it sets, but a conflicting - // complete method in env still errors rather than being dropped (#5096). + // Profile has a PAT; env gap-fills a different complete method (OAuth M2M). + // The config then carries two auth methods, which the SDK rejects rather + // than silently dropping the env one (#5096). t.Setenv("DATABRICKS_CLIENT_ID", "env-client-id") t.Setenv("DATABRICKS_CLIENT_SECRET", "env-client-secret") @@ -84,11 +82,9 @@ func TestProfileAuthLoadersConflictingEnvAuthMethodErrors(t *testing.T) { require.ErrorContains(t, err, "more than one authorization method configured") } -// TestNonAuthEnvSkipAttrsCoverSDKInternalEnvAttrs guards against an SDK bump -// silently re-introducing #5096. Every Internal (auth:"-") env-backed attribute -// that HasAuthAttribute misses must be either skipped (auth-steering) or listed -// below as a reviewed env-first attribute; a new SDK attribute fails this test -// until a human classifies it. +// TestNonAuthEnvSkipAttrsCoverSDKInternalEnvAttrs fails when an SDK bump adds an +// Internal (auth:"-") env-backed attribute that is neither skipped nor listed as +// a reviewed env-first attribute, forcing a human to classify it (#5096). func TestNonAuthEnvSkipAttrsCoverSDKInternalEnvAttrs(t *testing.T) { knownEnvFirstInternal := map[string]bool{ "oauth_callback_port": true, From a8e961c72b5a62e67df4e8e95e76d8682fdfbd6a Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Thu, 2 Jul 2026 13:35:58 +0000 Subject: [PATCH 6/8] Guard default_profile against env auth in databricks api The no---profile branch in `databricks api` pinned [__settings__].default_profile unconditionally, so a conflicting default profile could be merged with env auth (DATABRICKS_HOST/DATABRICKS_TOKEN), failing with "more than one authorization method configured" (the #5616 class already fixed for MustWorkspaceClient). Share the guard: export resolveDefaultProfile as ResolveDefaultProfile and call it from cmd/api, so the default profile is skipped when DATABRICKS_HOST is set. Adds an acceptance case with env PAT + conflicting default_profile. --- .../api/default-profile-vs-env/out.test.toml | 3 +++ .../cmd/api/default-profile-vs-env/output.txt | 22 +++++++++++++++++++ .../cmd/api/default-profile-vs-env/script | 19 ++++++++++++++++ .../cmd/api/default-profile-vs-env/test.toml | 3 +++ cmd/api/api.go | 17 ++++++-------- cmd/root/auth.go | 20 +++++++---------- cmd/root/auth_test.go | 2 +- 7 files changed, 63 insertions(+), 23 deletions(-) create mode 100644 acceptance/cmd/api/default-profile-vs-env/out.test.toml create mode 100644 acceptance/cmd/api/default-profile-vs-env/output.txt create mode 100644 acceptance/cmd/api/default-profile-vs-env/script create mode 100644 acceptance/cmd/api/default-profile-vs-env/test.toml diff --git a/acceptance/cmd/api/default-profile-vs-env/out.test.toml b/acceptance/cmd/api/default-profile-vs-env/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/cmd/api/default-profile-vs-env/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/default-profile-vs-env/output.txt b/acceptance/cmd/api/default-profile-vs-env/output.txt new file mode 100644 index 00000000000..9839d897b5c --- /dev/null +++ b/acceptance/cmd/api/default-profile-vs-env/output.txt @@ -0,0 +1,22 @@ + +=== api without --profile uses env auth, ignoring default_profile (#5616) + +>>> [CLI] api get /api/2.0/clusters/list +{} + +>>> print_requests.py --get //api/2.0/clusters/list +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ], + "X-Databricks-Workspace-Id": [ + "[NUMID]" + ] + }, + "method": "GET", + "path": "/api/2.0/clusters/list" +} diff --git a/acceptance/cmd/api/default-profile-vs-env/script b/acceptance/cmd/api/default-profile-vs-env/script new file mode 100644 index 00000000000..4ec3bc29997 --- /dev/null +++ b/acceptance/cmd/api/default-profile-vs-env/script @@ -0,0 +1,19 @@ +sethome "./home" + +# A default profile with conflicting (basic) auth on a different host. The env +# below still points a PAT at the test server. Without the #5616 guard, the +# default profile would be pinned and merged with the env PAT, failing with +# "more than one authorization method configured". +cat > "./home/.databrickscfg" < Date: Thu, 2 Jul 2026 14:41:32 +0000 Subject: [PATCH 7/8] Skip auth-steering env vars gap-filling a selected profile The final loader in ProfileAuthLoaders was the SDK's config.ConfigAttributes, which re-reads every still-empty attribute from the environment after the profile is loaded. That let host/routing/auth-steering env vars (auth_type, discovery_url, audience, cloud, workspace_id, account_id) shadow an explicitly selected profile, e.g. DATABRICKS_AUTH_TYPE=basic forcing basic auth over a profile's PAT (#5096). Replace it with resolveAuthGapFromEnv, a filtered env loader that skips nonAuthEnvSkipAttrs but still gap-fills real auth attrs, so a complete conflicting env auth method surfaces as an error rather than being silently dropped. Add an end-to-end ProfileAuthLoaders test covering the steering-env repro. --- libs/databrickscfg/loader.go | 39 ++++++++++++++++++++++++------- libs/databrickscfg/loader_test.go | 30 ++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index afb100db6ad..10d686dd51e 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -16,7 +16,11 @@ var ResolveProfileFromHost = profileFromHostLoader{} // ResolveNonAuthFromEnv reads non-auth, non-host config from environment // variables. See ProfileAuthLoaders for how it fits into the chain. -var ResolveNonAuthFromEnv = nonAuthEnvLoader{} +var ResolveNonAuthFromEnv = envLoader{name: "environment (excluding auth)", skipAuth: true} + +// resolveAuthGapFromEnv gap-fills auth attributes the profile left empty from +// environment variables. See ProfileAuthLoaders for how it fits into the chain. +var resolveAuthGapFromEnv = envLoader{name: "environment (auth gap-fill)", skipAuth: false} // ProfileAuthLoaders is the loader chain for an explicitly selected profile // (--profile or a bundle's workspace.profile). Unlike the SDK's default @@ -24,14 +28,19 @@ var ResolveNonAuthFromEnv = nonAuthEnvLoader{} // // 1. ResolveNonAuthFromEnv: non-auth env attrs (e.g. cluster_id), env-wins. // 2. ConfigFile: the selected profile (host, routing, auth). -// 3. ConfigAttributes: gap-fills only fields the profile left empty. +// 3. resolveAuthGapFromEnv: gap-fills auth attrs the profile left empty from +// env, so a complete conflicting env auth method still surfaces as an error +// rather than being silently dropped. It skips nonAuthEnvSkipAttrs (like the +// env-first pass) so host/routing/auth-steering attrs come from the profile +// only; using the SDK's ConfigAttributes here would let those env vars shadow +// the profile (e.g. DATABRICKS_AUTH_TYPE=basic over a PAT profile). // // A profile from DATABRICKS_CONFIG_PROFILE keeps env-first precedence; only an // out-of-band profile name triggers this chain. var ProfileAuthLoaders = []config.Loader{ ResolveNonAuthFromEnv, config.ConfigFile, - config.ConfigAttributes, + resolveAuthGapFromEnv, } var errNoMatchingProfiles = errors.New("no matching config profiles found") @@ -96,16 +105,28 @@ var nonAuthEnvSkipAttrs = map[string]bool{ "cloud": true, } -type nonAuthEnvLoader struct{} +// envLoader reads config attributes from environment variables. It always skips +// nonAuthEnvSkipAttrs so host/routing/auth-steering attrs come from the selected +// profile only (#5096). When skipAuth is true it additionally skips the SDK's +// auth attributes, for the env-first pass that runs before the profile. +type envLoader struct { + name string + skipAuth bool +} -func (nonAuthEnvLoader) Name() string { - return "environment (excluding auth)" +func (l envLoader) Name() string { + return l.name } -func (nonAuthEnvLoader) Configure(cfg *config.Config) error { +func (l envLoader) Configure(cfg *config.Config) error { for _, attr := range config.ConfigAttributes { - // Host and auth come from the profile (config file), not env. - if nonAuthEnvSkipAttrs[attr.Name] || attr.HasAuthAttribute() { + // Host/routing/auth-steering come from the profile (config file), not env. + if nonAuthEnvSkipAttrs[attr.Name] { + continue + } + // The env-first pass leaves auth for the profile; the gap-fill pass runs + // after the profile and fills only auth attrs the profile didn't set. + if l.skipAuth && attr.HasAuthAttribute() { continue } // Don't overwrite an already-set value (SDK loader semantics). diff --git a/libs/databrickscfg/loader_test.go b/libs/databrickscfg/loader_test.go index cbd1cbff2d2..3b3ad73c692 100644 --- a/libs/databrickscfg/loader_test.go +++ b/libs/databrickscfg/loader_test.go @@ -65,6 +65,36 @@ func TestResolveNonAuthFromEnvSkipsHostAndAuth(t *testing.T) { assert.Equal(t, "env-cluster", cfg.ClusterID) } +func TestProfileAuthLoadersProfileWinsOverSteeringEnv(t *testing.T) { + // A PAT profile plus auth-steering/routing env vars. The profile owns + // host/routing/auth-steering, so these env vars must not shadow it (#5096). + // In particular DATABRICKS_AUTH_TYPE=basic must not force basic auth over the + // profile's PAT. + t.Setenv("DATABRICKS_AUTH_TYPE", "basic") + t.Setenv("DATABRICKS_DISCOVERY_URL", "https://discovery.env.test") + t.Setenv("DATABRICKS_CLOUD", "azure") + t.Setenv("DATABRICKS_WORKSPACE_ID", "env-workspace") + t.Setenv("DATABRICKS_ACCOUNT_ID", "env-account") + + cfg := config.Config{ + Loaders: ProfileAuthLoaders, + ConfigFile: "profile/testdata/databrickscfg", + Profile: "DEFAULT", + } + + err := cfg.EnsureResolved() + require.NoError(t, err) + + assert.Equal(t, "https://default.test", cfg.Host) + assert.Equal(t, "default", cfg.Token) + // Steering/routing env vars did not shadow the profile. + assert.Empty(t, cfg.AuthType) + assert.Empty(t, cfg.DiscoveryURL) + assert.Empty(t, cfg.Cloud) + assert.Empty(t, cfg.WorkspaceID) + assert.Empty(t, cfg.AccountID) +} + func TestProfileAuthLoadersConflictingEnvAuthMethodErrors(t *testing.T) { // Profile has a PAT; env gap-fills a different complete method (OAuth M2M). // The config then carries two auth methods, which the SDK rejects rather From 29d2b243eddd794443af044c35d57b8eb17368fb Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Thu, 2 Jul 2026 18:53:01 +0000 Subject: [PATCH 8/8] Gap-fill the host from env for a host-less selected profile A selected profile skips host/routing/auth-steering env vars so they can't shadow it (#5096), but the host was skipped in both the env-first and the gap-fill passes. A profile that leaves the host empty (or can't be resolved) then never fell back to DATABRICKS_HOST and failed with "cannot configure default credentials". Treat the host like an auth attribute: skip it only in the env-first pass so the profile still owns it, then gap-fill it from env after the profile, mirroring the existing token gap-fill. Routing/auth-steering attrs stay skipped in both passes. --- libs/databrickscfg/loader.go | 54 ++++++++++++++++++------------- libs/databrickscfg/loader_test.go | 36 ++++++++++++++++++--- 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index 10d686dd51e..732dcf9024f 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -18,8 +18,9 @@ var ResolveProfileFromHost = profileFromHostLoader{} // variables. See ProfileAuthLoaders for how it fits into the chain. var ResolveNonAuthFromEnv = envLoader{name: "environment (excluding auth)", skipAuth: true} -// resolveAuthGapFromEnv gap-fills auth attributes the profile left empty from -// environment variables. See ProfileAuthLoaders for how it fits into the chain. +// resolveAuthGapFromEnv gap-fills the auth attributes and host the profile left +// empty from environment variables. See ProfileAuthLoaders for how it fits into +// the chain. var resolveAuthGapFromEnv = envLoader{name: "environment (auth gap-fill)", skipAuth: false} // ProfileAuthLoaders is the loader chain for an explicitly selected profile @@ -28,12 +29,13 @@ var resolveAuthGapFromEnv = envLoader{name: "environment (auth gap-fill)", skipA // // 1. ResolveNonAuthFromEnv: non-auth env attrs (e.g. cluster_id), env-wins. // 2. ConfigFile: the selected profile (host, routing, auth). -// 3. resolveAuthGapFromEnv: gap-fills auth attrs the profile left empty from -// env, so a complete conflicting env auth method still surfaces as an error -// rather than being silently dropped. It skips nonAuthEnvSkipAttrs (like the -// env-first pass) so host/routing/auth-steering attrs come from the profile -// only; using the SDK's ConfigAttributes here would let those env vars shadow -// the profile (e.g. DATABRICKS_AUTH_TYPE=basic over a PAT profile). +// 3. resolveAuthGapFromEnv: gap-fills the auth attrs and host the profile left +// empty from env, so a host-less profile still reaches the env host and a +// complete conflicting env auth method surfaces as an error rather than being +// silently dropped. It skips envAlwaysSkipAttrs so routing/auth-steering +// attrs come from the profile only; using the SDK's ConfigAttributes here +// would let those env vars shadow the profile (e.g. DATABRICKS_AUTH_TYPE=basic +// over a PAT profile). // // A profile from DATABRICKS_CONFIG_PROFILE keeps env-first precedence; only an // out-of-band profile name triggers this chain. @@ -89,14 +91,19 @@ func findMatchingProfile(configFile *config.File, matcher func(*ini.Section) boo return matching[0], nil } -// nonAuthEnvSkipAttrs lists env attributes nonAuthEnvLoader must skip on top of -// those HasAuthAttribute catches: host/routing (host, workspace_id, account_id) -// and auth-steering fields tagged auth:"-" (auth_type, discovery_url, audience, -// cloud), which HasAuthAttribute misses. Reading these from env would shadow the -// selected profile (#5096). TestNonAuthEnvSkipAttrsCoverSDKInternalEnvAttrs -// guards against SDK drift. -var nonAuthEnvSkipAttrs = map[string]bool{ - "host": true, +// hostAttr is the SDK config attribute name for the workspace/account host. +const hostAttr = "host" + +// envAlwaysSkipAttrs lists env attributes envLoader must never read when a +// profile is explicitly selected, in either pass: routing (workspace_id, +// account_id) and auth-steering fields tagged auth:"-" (auth_type, discovery_url, +// audience, cloud), which HasAuthAttribute misses. Reading these from env — even +// as a gap-fill — would re-steer or shadow the selected profile's auth (#5096). +// The host is deliberately absent: like the auth attrs it is skipped only in the +// env-first pass (see envLoader), then gap-filled after the profile so a +// host-less profile falls back to the env host. +// TestEnvAlwaysSkipAttrsCoverSDKInternalEnvAttrs guards against SDK drift. +var envAlwaysSkipAttrs = map[string]bool{ "workspace_id": true, "account_id": true, "auth_type": true, @@ -106,9 +113,10 @@ var nonAuthEnvSkipAttrs = map[string]bool{ } // envLoader reads config attributes from environment variables. It always skips -// nonAuthEnvSkipAttrs so host/routing/auth-steering attrs come from the selected +// envAlwaysSkipAttrs so routing/auth-steering attrs come from the selected // profile only (#5096). When skipAuth is true it additionally skips the SDK's -// auth attributes, for the env-first pass that runs before the profile. +// auth attributes and the host, for the env-first pass that runs before the +// profile; the gap-fill pass then fills the ones the profile left empty. type envLoader struct { name string skipAuth bool @@ -120,13 +128,13 @@ func (l envLoader) Name() string { func (l envLoader) Configure(cfg *config.Config) error { for _, attr := range config.ConfigAttributes { - // Host/routing/auth-steering come from the profile (config file), not env. - if nonAuthEnvSkipAttrs[attr.Name] { + // Routing/auth-steering come from the profile (config file), not env. + if envAlwaysSkipAttrs[attr.Name] { continue } - // The env-first pass leaves auth for the profile; the gap-fill pass runs - // after the profile and fills only auth attrs the profile didn't set. - if l.skipAuth && attr.HasAuthAttribute() { + // The env-first pass leaves the auth attrs and host for the profile; the + // gap-fill pass runs after the profile and fills only those it left empty. + if l.skipAuth && (attr.HasAuthAttribute() || attr.Name == hostAttr) { continue } // Don't overwrite an already-set value (SDK loader semantics). diff --git a/libs/databrickscfg/loader_test.go b/libs/databrickscfg/loader_test.go index 3b3ad73c692..b1ab137d789 100644 --- a/libs/databrickscfg/loader_test.go +++ b/libs/databrickscfg/loader_test.go @@ -2,6 +2,8 @@ package databrickscfg import ( "errors" + "os" + "path/filepath" "testing" "github.com/databricks/databricks-sdk-go/config" @@ -95,6 +97,32 @@ func TestProfileAuthLoadersProfileWinsOverSteeringEnv(t *testing.T) { assert.Empty(t, cfg.AccountID) } +func TestProfileAuthLoadersGapFillsHostFromEnv(t *testing.T) { + // A selected profile that leaves the host empty falls back to the env host, + // mirroring the token gap-fill (#5096). Auth-steering env vars stay ignored. + dir := t.TempDir() + cfgFile := filepath.Join(dir, ".databrickscfg") + require.NoError(t, os.WriteFile(cfgFile, []byte("[hostless]\ncluster_id = env-cluster\n"), 0o600)) + + t.Setenv("DATABRICKS_HOST", "https://env.test") + t.Setenv("DATABRICKS_TOKEN", "env-token") + t.Setenv("DATABRICKS_AUTH_TYPE", "basic") + + cfg := config.Config{ + Loaders: ProfileAuthLoaders, + ConfigFile: cfgFile, + Profile: "hostless", + } + + err := cfg.EnsureResolved() + require.NoError(t, err) + + assert.Equal(t, "https://env.test", cfg.Host) + assert.Equal(t, "env-token", cfg.Token) + // Auth-steering env var did not leak in via the gap-fill. + assert.Empty(t, cfg.AuthType) +} + func TestProfileAuthLoadersConflictingEnvAuthMethodErrors(t *testing.T) { // Profile has a PAT; env gap-fills a different complete method (OAuth M2M). // The config then carries two auth methods, which the SDK rejects rather @@ -112,10 +140,10 @@ func TestProfileAuthLoadersConflictingEnvAuthMethodErrors(t *testing.T) { require.ErrorContains(t, err, "more than one authorization method configured") } -// TestNonAuthEnvSkipAttrsCoverSDKInternalEnvAttrs fails when an SDK bump adds an +// TestEnvAlwaysSkipAttrsCoverSDKInternalEnvAttrs fails when an SDK bump adds an // Internal (auth:"-") env-backed attribute that is neither skipped nor listed as // a reviewed env-first attribute, forcing a human to classify it (#5096). -func TestNonAuthEnvSkipAttrsCoverSDKInternalEnvAttrs(t *testing.T) { +func TestEnvAlwaysSkipAttrsCoverSDKInternalEnvAttrs(t *testing.T) { knownEnvFirstInternal := map[string]bool{ "oauth_callback_port": true, "disable_oauth_refresh_token": true, @@ -128,11 +156,11 @@ func TestNonAuthEnvSkipAttrsCoverSDKInternalEnvAttrs(t *testing.T) { if !attr.Internal || len(attr.EnvVars) == 0 { continue } - if nonAuthEnvSkipAttrs[attr.Name] || knownEnvFirstInternal[attr.Name] { + if envAlwaysSkipAttrs[attr.Name] || knownEnvFirstInternal[attr.Name] { continue } t.Errorf("SDK config attribute %q (env %v) is internal (auth:\"-\") but unclassified: "+ - "add it to nonAuthEnvSkipAttrs if it steers auth/routing, or to "+ + "add it to envAlwaysSkipAttrs if it steers auth/routing, or to "+ "knownEnvFirstInternal if env-first precedence is safe (#5096)", attr.Name, attr.EnvVars) }