Skip to content

Reject an invalid host in auth login and auth token - #7195

Open
maksim-zuev-db wants to merge 22 commits into
mainfrom
login-reject-empty-host
Open

maksim-zuev-db wants to merge 22 commits into
mainfrom
login-reject-empty-host

Conversation

@maksim-zuev-db

@maksim-zuev-db maksim-zuev-db commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Changes

databricks auth login and databricks auth token reject a host that is not a URL with a hostname,
e.g. " ", /, ?o=123, https:// or my workspace.test, with invalid host "<input>".

  • The check is in normalizeLoginHost, used by setHost and the --skip-workspace block, so it covers
    --host, the positional argument, profile hosts and the prompt. A host without an http(s) scheme is
    checked as an https:// host, like the SDK does. It replaces errDeviceCodeNeedsHost from Prompt for the host instead of failing when device code login has none #7179.
  • auth login checks --host and the positional argument before the keyring probe and the profile name prompt.
  • The host prompt runs the same check, so it shows the error under the prompt and asks again.

Behavior changes:

  • An empty host after "Enter a host" in the login profile picker no longer starts discovery login.
  • auth login --skip-workspace --host '?a=123' fails. On main this started discovery login.
  • auth login --host https:// fails. On main login tried to connect to a host named https.
  • Spaces around a host are trimmed, so " https://x.test/?o=1 " works and keeps the workspace ID.

Why

Follow-up to a review comment on #7179. An empty host made login and token resolve the host from
DATABRICKS_HOST, save a profile without a host key, or fail later with host must start with 'https://': .

Tests

  • TestSetHost, TestPromptForHostRejectsEmptyHost, TestPromptForHostRejectsHostEmptyAfterNormalization,
    TestLoginDeviceCodeHostEmptyAfterNormalization, TestLoginSkipWorkspaceHostEmptyAfterNormalization,
    TestLoginRejectsInvalidHostBeforeProfileNamePrompt.
  • Acceptance tests cmd/auth/login/invalid-host and cmd/auth/token/invalid-host.

This pull request and its description were written by Isaac.

@maksim-zuev-db
maksim-zuev-db marked this pull request as ready for review October 27, 2026 14:00
@maksim-zuev-db
maksim-zuev-db requested review from a team as code owners October 27, 2026 14:00
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 1ba63e7

Run: 41424808692

Env ❌​FAIL 🔄​flaky ✅​pass 🙈​skip Time
✅​ aws linux-2core-8gb 281 17 3:27
✅​ aws-windows-latest-4core-16gb 283 15 3:37
❌​ azure linux-2core-8gb 5 275 17 3:22
✅​ azure-windows-latest-4core-16gb 282 15 3:38
🔄​ gcp linux-2core-8gb 2 279 17 3:45
✅​ gcp-windows-latest-4core-16gb 283 15 3:53
7 interesting tests: 5 FAIL, 2 flaky
Test Name azure linux-2core-8gb gcp linux-2core-8gb
🔄​ TestFsCpDirToDirFileNotOverwritten ✅​p 🔄​f
🔄​ TestFsCpDirToDirFileNotOverwritten/local_to_uc-volumes ✅​p 🔄​f
❌​ TestFilerWorkspaceNotebook ❌​F ✅​p
❌​ TestFilerWorkspaceNotebook/rJupyterNb.ipynb ❌​F ✅​p
❌​ TestFilerWorkspaceNotebook/rNb.r ❌​F ✅​p
❌​ TestFilerWorkspaceNotebook/scalaNb.scala ❌​F ✅​p
❌​ TestFilerWorkspaceNotebook/sqlNb.sql ❌​F ✅​p
Top 3 slowest tests (at least 2 minutes):
duration env testname
3:47 gcp-windows-latest-4core-16gb TestAccept
3:31 aws-windows-latest-4core-16gb TestAccept
3:31 azure-windows-latest-4core-16gb TestAccept

@simonfaltum simonfaltum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments inline. The main one is in login.go: removing the post-setHost empty check lets device code login continue with an empty host when the input normalizes to "" (e.g. / or ?o=123).

One more outside the diff: promptForAccountID (cmd/auth/auth.go:67) has the same problem for the account ID. An empty answer leaves AccountID == "", ToOAuthArgument then picks up DATABRICKS_ACCOUNT_ID through ConfigAttributes (or OAuth fails later), and the profile can be saved without account_id. Probably fine as a follow-up.

Comment thread cmd/auth/login.go Resolved
Comment thread cmd/auth/auth.go Resolved
Comment thread cmd/auth/auth.go Resolved
Comment thread cmd/auth/auth.go Resolved
Comment thread cmd/auth/auth_test.go Resolved
Comment thread cmd/auth/login_device_code_test.go Resolved

@simonfaltum simonfaltum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes, the first-round points look addressed. A few more comments inline, nothing blocking.

The glyph change was my suggestion, my bad. It changes every prompt that uses Validate, so I think it should be rolled back here (see the comment on prompt.go).

[nit] In the description, "auth login --skip-workspace --host '?a=123' fails instead of starting discovery login (already the case on main)" reads like a contradiction. On main the host is empty after the query params are extracted, so shouldUseDiscovery returns true and login goes on to discovery. "On main this started discovery login" would be clearer.

Comment thread cmd/auth/auth.go Resolved
Comment thread cmd/auth/login.go Resolved
Comment thread cmd/auth/login.go Resolved
Comment thread cmd/auth/login.go Resolved
Comment thread libs/cmdio/prompt.go Resolved
Comment thread cmd/auth/login_device_code_test.go Resolved
maksim-zuev-db and others added 15 commits October 30, 2026 16:52
Pressing Enter at the "Databricks host" prompt left the host empty, so
login fell back to DATABRICKS_HOST or failed later with an unrelated
error. The prompt now refuses an empty answer, which replaces the
device code specific empty host checks.

Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
A host pasted with surrounding spaces failed later with an unrelated URL
error, and a host of only spaces got past the empty host check.

Co-authored-by: Isaac <no-reply@databricks.com>
setHost strips a trailing "/" and the query params, so "/" or "?o=123"
became an empty host after the prompt check. Login and auth token then
resolved the host from DATABRICKS_HOST or failed later. Checking in
setHost covers the prompt, --host, the positional argument, and profile
hosts, and keeps the hint from the removed errDeviceCodeNeedsHost.

Co-authored-by: Isaac <no-reply@databricks.com>
WaitFor matched the error printed for the first empty answer, so the
test passed even if a whitespace-only host was accepted. Golden steps
compare the rendered screen after each Enter.

Co-authored-by: Isaac <no-reply@databricks.com>
The prompt glyph ran Validate on every render, so a prompt that rejects
empty input, such as the login host prompt, opened with a red cross
before anything was typed. The glyph is now blank until the first edit
or Enter, then shows a cross while the input is invalid and a checkmark
once it is valid.

Co-authored-by: Isaac <no-reply@databricks.com>
Mention auth token, and that an empty host after "Enter a host" no
longer starts discovery login.

Co-authored-by: Isaac <no-reply@databricks.com>
With --skip-workspace, login stripped the query params from the host
before setHost ran, so a host like "?a=123" became empty and login went
to discovery instead of failing. Both places now share normalizeHost,
which does the empty host check.

The error is now `invalid host "<input>"`. The previous hint to pass
the host as an argument or with --host did not help, because the error
only happens when a host was given.

Co-authored-by: Isaac <no-reply@databricks.com>
cmd/configure and libs/databrickscfg already have a normalizeHost.

Co-authored-by: Isaac <no-reply@databricks.com>
The glyph change affects every prompt that uses Validate, including the
databricks configure host prompt, so it does not belong in this PR. The
baseline tests added with it pass with the old glyph and stay.

Co-authored-by: Isaac <no-reply@databricks.com>
ExtractHostQueryParams also handles ?w=, ?workspace_id= and
?account_id=, not just ?o= and ?a=.

Co-authored-by: Isaac <no-reply@databricks.com>
A "/" host made the subtest name end in "//", which is awkward to
target from the command line. TestLoginSkipWorkspaceHostEmptyAfterNormalization
had landed between the two positional argument tests.

Co-authored-by: Isaac <no-reply@databricks.com>
Only the host prompt trimmed spaces, so --host "  ", a positional
argument or a profile host of only spaces passed the empty host check and
failed later with "host must start with 'https://'". A leading space also
made url.Parse fail, so ?o= never ended up in WorkspaceID.

promptForHost still trims its result because the profile picker passes
the host to getProfileName before it is normalized.

Co-authored-by: Isaac <no-reply@databricks.com>
The prompt only rejected blank input, so "/" or "?o=123" passed and
then failed in setHost. In auth login, the "Enter a host" picker path
first asked for a profile name and then failed, and in auth token the
command exited instead of asking again. Running normalizeLoginHost in
Validate keeps the prompt open with the error under it.

Co-authored-by: Isaac <no-reply@databricks.com>
The invalid host error is user-visible output of both commands, and the
auth token path was not exercised end to end.

Co-authored-by: Isaac <no-reply@databricks.com>
@maksim-zuev-db
maksim-zuev-db force-pushed the login-reject-empty-host branch from 50e1992 to 02bfdf4 Compare October 30, 2026 17:09
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Commit: 1ba63e7de | Workflow run

Regression test report

Selected 2 changed tests (limit=20, 0 not selected)

2 tests: regression=2 unreleased=0 coverage=0 failing=0

test branch main (3aa5c96) latest (v1.23.0)
TestAccept/cmd/auth/login/invalid-host ✅ ❌ ❌
TestAccept/cmd/auth/token/invalid-host ✅ ❌ ❌
TestAccept/cmd/auth/login/invalid-host: branch ✅ | main (3aa5c96) ❌ | latest (v1.23.0) ❌

main (3aa5c96):

acceptance_test.go:1341: 
        	Error Trace:	/home/runner/work/cli/cli/libs/testdiff/testdiff.go:22
        	            				/home/runner/work/cli/cli/acceptance/acceptance_test.go:1341
        	            				/home/runner/work/cli/cli/acceptance/acceptance_test.go:1082
        	            				/home/runner/work/cli/cli/acceptance/acceptance_test.go:641
        	Error:      	Not equal: 
        	            	expected: "\n>>> musterr [CLI] auth login --host ?o=123 --profile test\nError: invalid host \"?o=123\"\n\n>>> musterr [CLI] auth login --host https:// --profile test\nError: invalid host \"https://\"\n"
        	            	actual  : "\n>>> musterr [CLI] auth login --host ?o=123 --profile test\nPlease continue the authentication process in your browser:\n[DATABRICKS_URL]/oidc/v1/authorize?client_id=databricks-cli&code_challenge=PE2HZGCOwDTUJactLBKYMwyW8iGsp3o7mw0QcBPrvdQ&code_challenge_method=S256&redirect_uri=http%3A%2F%2Flocalhost%3A8020&response_type=code&scope=offline_access+all-apis&state=VZezQvt74SVfE369soDCrw\n\nError: Test script killed due to a timeout (2m0s)\n"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -2,6 +2,6 @@
        	            	 >>> musterr [CLI] auth login --host ?o=123 --profile test
        	            	-Error: invalid host "?o=123"
        	            	+Please continue the authentication process in your browser:
        	            	+[DATABRICKS_URL]/oidc/v1/authorize?client_id=databricks-cli&code_challenge=PE2HZGCOwDTUJactLBKYMwyW8iGsp3o7mw0QcBPrvdQ&code_challenge_method=S256&redirect_uri=http%3A%2F%2Flocalhost%3A8020&response_type=code&scope=offline_access+all-apis&state=VZezQvt74SVfE369soDCrw
        	            	 
        	            	->>> musterr [CLI] auth login --host https:// --profile test
        	            	-Error: invalid host "https://"
        	            	+Error: Test script killed due to a timeout (2m0s)
        	            	 
        	Test:       	TestAccept/cmd/auth/login/invalid-host
        	Messages:   	cmd/auth/login/invalid-host/output.txt vs /tmp/TestAcceptcmdauthlogininvalid-host3050142661/002/output.txt
FAIL

latest (v1.23.0):

acceptance_test.go:1341: 
        	Error Trace:	/home/runner/work/cli/cli/libs/testdiff/testdiff.go:22
        	            				/home/runner/work/cli/cli/acceptance/acceptance_test.go:1341
        	            				/home/runner/work/cli/cli/acceptance/acceptance_test.go:1082
        	            				/home/runner/work/cli/cli/acceptance/acceptance_test.go:641
        	Error:      	Not equal: 
        	            	expected: "\n>>> musterr [CLI] auth login --host ?o=123 --profile test\nError: invalid host \"?o=123\"\n\n>>> musterr [CLI] auth login --host https:// --profile test\nError: invalid host \"https://\"\n"
        	            	actual  : "\n>>> musterr [CLI] auth login --host ?o=123 --profile test\nPlease continue the authentication process in your browser:\n[DATABRICKS_URL]/oidc/v1/authorize?client_id=databricks-cli&code_challenge=-zOiVubQpTnsqs_iPEZIKIVXFAdHgJpVxuYe-uT_QgA&code_challenge_method=S256&redirect_uri=http%3A%2F%2Flocalhost%3A8021&response_type=code&scope=offline_access+all-apis&state=UAGERUAKC1VEmdFuDV2G1w\n\nError: Test script killed due to a timeout (2m0s)\n"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -2,6 +2,6 @@
        	            	 >>> musterr [CLI] auth login --host ?o=123 --profile test
        	            	-Error: invalid host "?o=123"
        	            	+Please continue the authentication process in your browser:
        	            	+[DATABRICKS_URL]/oidc/v1/authorize?client_id=databricks-cli&code_challenge=-zOiVubQpTnsqs_iPEZIKIVXFAdHgJpVxuYe-uT_QgA&code_challenge_method=S256&redirect_uri=http%3A%2F%2Flocalhost%3A8021&response_type=code&scope=offline_access+all-apis&state=UAGERUAKC1VEmdFuDV2G1w
        	            	 
        	            	->>> musterr [CLI] auth login --host https:// --profile test
        	            	-Error: invalid host "https://"
        	            	+Error: Test script killed due to a timeout (2m0s)
        	            	 
        	Test:       	TestAccept/cmd/auth/login/invalid-host
        	Messages:   	cmd/auth/login/invalid-host/output.txt vs /tmp/TestAcceptcmdauthlogininvalid-host3544017196/002/output.txt
FAIL
TestAccept/cmd/auth/token/invalid-host: branch ✅ | main (3aa5c96) ❌ | latest (v1.23.0) ❌

main (3aa5c96):

acceptance_test.go:1341: 
        	Error Trace:	/home/runner/work/cli/cli/libs/testdiff/testdiff.go:22
        	            				/home/runner/work/cli/cli/acceptance/acceptance_test.go:1341
        	            				/home/runner/work/cli/cli/acceptance/acceptance_test.go:1082
        	            				/home/runner/work/cli/cli/acceptance/acceptance_test.go:641
        	Error:      	Not equal: 
        	            	expected: "\n>>> musterr [CLI] auth token --host ?o=123\nError: invalid host \"?o=123\"\n\n>>> musterr [CLI] auth token --host https://\nError: invalid host \"https://\"\n"
        	            	actual  : "\n>>> musterr [CLI] auth token --host ?o=123\nError: host must start with 'https://': \n\n>>> musterr [CLI] auth token --host https://\nWarn: [hostmetadata] failed to fetch host metadata for https://https:, will skip for 1m0s\nError: cache: databricks OAuth is not configured for this host. no cached credentials; run `databricks auth login` to sign in\n"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -2,6 +2,7 @@
        	            	 >>> musterr [CLI] auth token --host ?o=123
        	            	-Error: invalid host "?o=123"
        	            	+Error: host must start with 'https://': 
        	            	 
        	            	 >>> musterr [CLI] auth token --host https://
        	            	-Error: invalid host "https://"
        	            	+Warn: [hostmetadata] failed to fetch host metadata for https://https:, will skip for 1m0s
        	            	+Error: cache: databricks OAuth is not configured for this host. no cached credentials; run `databricks auth login` to sign in
        	            	 
        	Test:       	TestAccept/cmd/auth/token/invalid-host
        	Messages:   	cmd/auth/token/invalid-host/output.txt vs /tmp/TestAcceptcmdauthtokeninvalid-host1804967678/002/output.txt

latest (v1.23.0): same output as main (3aa5c96)

maksim-zuev-db and others added 3 commits November 2, 2026 09:01
TestPromptBaseline_ValidateEnterBeforeEdit and
TestPromptBaseline_ValidateValidBeforeEdit were written for the blank
prompt glyph, which was reverted. They only pin unchanged libs/cmdio
behavior, and the cmd/auth goldens cover the host prompt.

Co-authored-by: Isaac <no-reply@databricks.com>
normalizeLoginHost only rejected a host that was empty after removing
the trailing slash and the query params, so "https://" became "https:/"
and "https://?o=123" became "https:". The SDK reads those as a host named
"https", so login tried to connect to it and auth token reported missing
credentials. The check now parses the host, adding https:// to a host
without an http(s) scheme as the SDK does, and fails if there is no
hostname.

A host that does not parse, e.g. with spaces, still fails later as before.

Co-authored-by: Isaac <no-reply@databricks.com>
setHost normalizes the host after the keyring probe and the profile
name prompt. With --host / and no --profile, login first asked for a
profile name and only then failed with invalid host "/". Checking a host
from --host or the positional argument right after the positional
argument is resolved fails before both.

Co-authored-by: Isaac <no-reply@databricks.com>
A host that url.Parse rejects, e.g. "my workspace.test", passed the
hostname check and failed later with an SDK parse error. It now fails
with invalid host, like a host without a hostname.

TestSetHost used "val from --host" and "val from [HOST]" as hosts,
which are not URLs, so they are now real test hosts.

Co-authored-by: Isaac <no-reply@databricks.com>
@maksim-zuev-db maksim-zuev-db changed the title Reject an empty host at the login host prompt Reject an invalid host in auth login and auth token Nov 2, 2026
maksim-zuev-db and others added 3 commits November 2, 2026 09:42
hasHostname retried any host that failed to parse with "https://" in
front, so "https://my workspace.test" or "https://x.test:abc" was read
as a host named "https" and passed the check. A host with "://" is now
not retried.

Co-authored-by: Isaac <no-reply@databricks.com>
The early host check in auth login rejects the host before the
--skip-workspace block normalizes it, so the test checks that an
invalid host is rejected, not that it is empty after normalization.

Co-authored-by: Isaac <no-reply@databricks.com>
Check for an http(s) scheme prefix instead of parsing the host twice.

Co-authored-by: Isaac <no-reply@databricks.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants