Skip to content

CXH-2349: harden account hostname resolution #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
al-conductorone wants to merge 2 commits into
base: main
Choose a base branch
from
Open

CXH-2349: harden account hostname resolution #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0fa67d6
CXH-2349: harden account hostname resolution
al-conductorone Aug 28, 2026
9d65bf7
CXH-2349: normalise account hostname case and resolve AWS account host
al-conductorone Aug 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions pkg/databricks/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,26 @@ type Client struct {
isWSAPIAvailable bool
}

// hostMatches reports whether hostname equals suffix or sits under it at a DNS
// label boundary. Plain strings.HasSuffix would let evilazuredatabricks.net
// match azuredatabricks.net; requiring the leading dot prevents that.
func hostMatches(hostname, suffix string) bool {
return hostname == suffix || strings.HasSuffix(hostname, "."+suffix)

@github-actions github-actions Bot Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: hostMatches is case-sensitive, but DNS hostnames are not — a config value like MyOrg.AzureDatabricks.net (or a trailing-dot FQDN) falls through to the default branch and yields accounts.MyOrg.AzureDatabricks.net instead of the canonical Azure account host. The failure is fail-safe rather than a spoofing bypass, but since this function's job is host classification it's worth normalising first, e.g. hostname = strings.ToLower(strings.TrimSuffix(hostname, ".")) in GetAccountHostname before matching.

@FeliLucero1 FeliLucero1 Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fyi / out of scope: this is still case-sensitive — MyOrg.AzureDatabricks.NET falls through instead of hitting azure. pre-existing under HasSuffix, not blocking this PR.

@FeliLucero1 FeliLucero1 Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fyi / out of scope: trailing FQDN dot (….azuredatabricks.net.) also misses. atypical input, not blocking.

}

func GetAccountHostname(hostname string) string {
if strings.HasSuffix(hostname, azureHost) {
hostname = strings.ToLower(strings.TrimSuffix(hostname, "."))
switch {
case hostMatches(hostname, azureHost):
return "accounts." + azureHost
} else if strings.HasSuffix(hostname, gcpHost) {
case hostMatches(hostname, gcpHost):
return "accounts." + gcpHost
case hostMatches(hostname, defaultHost):
return "accounts." + defaultHost
default:
// Unknown hosts have no canonical account suffix, so prefix as given.
return "accounts." + hostname
Comment on lines +69 to +71

@github-actions github-actions Bot Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: The comment is inaccurate for AWS — AWS does have a canonical account host (accounts.cloud.databricks.com), which is exactly the defaultHost constant this PR deletes. Real AWS workspace hostnames look like dbc-1234abcd.cloud.databricks.com, so this branch produces accounts.dbc-1234abcd.cloud.databricks.com, which is not a valid account endpoint (the added test encodes this as "aws falls through unnormalised"). This is pre-existing behaviour and users can work around it with --account-hostname, so not blocking, but consider keeping defaultHost = "cloud.databricks.com" and adding a third hostMatches(hostname, defaultHost) case for symmetry with Azure/GCP; at minimum reword the comment so it doesn't claim AWS has no canonical suffix.

}
return "accounts." + hostname
}

func NewClient(ctx context.Context, httpClient *http.Client, hostname, accountHostname, accountID, baseURL string, auth Auth, excludeWorkspaces []string) (*Client, error) {
Expand Down
26 changes: 26 additions & 0 deletions pkg/databricks/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,32 @@ func TestIsWorkspaceExcluded(t *testing.T) {
}
}

func TestGetAccountHostname(t *testing.T) {
tests := []struct {
name string
hostname string
want string
}{
{"azure subdomain", "myorg.azuredatabricks.net", "accounts.azuredatabricks.net"},
{"azure bare host", "azuredatabricks.net", "accounts.azuredatabricks.net"},
{"azure mixed case", "MyOrg.AzureDatabricks.net", "accounts.azuredatabricks.net"},
{"azure trailing dot", "myorg.azuredatabricks.net.", "accounts.azuredatabricks.net"},
{"gcp subdomain", "myorg.gcp.databricks.com", "accounts.gcp.databricks.com"},
{"aws workspace host normalises to account host", "dbc-1234abcd.cloud.databricks.com", "accounts.cloud.databricks.com"},
{"unknown host falls through unnormalised", "myorg.example.com", "accounts.myorg.example.com"},
// CXH-2349: a bare-suffix match would resolve this to the Azure account host.
{"azure lookalike does not match", "evilazuredatabricks.net", "accounts.evilazuredatabricks.net"},
{"gcp lookalike does not match", "evilgcp.databricks.com", "accounts.evilgcp.databricks.com"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := GetAccountHostname(tt.hostname); got != tt.want {
t.Errorf("GetAccountHostname(%q) = %q, want %q", tt.hostname, got, tt.want)
}
})
}
}

func TestIsWorkspaceExcludedReturnsEveryMatchedKey(t *testing.T) {
ws := Workspace{ID: 12345, Name: "prod", DeploymentName: "dbc-abc"}
c := newTestClient(t, []string{"prod", "dbc-abc", "staging"})
Expand Down
Loading