From 35adebc0261fc456af6f75715e0f0ede92a70c2b Mon Sep 17 00:00:00 2001 From: "pieter.noordhuis" Date: Wed, 4 Nov 2026 08:36:23 +0000 Subject: [PATCH] Strip credentials from origin URL in deploy telemetry bundleGitInfo assumed load_git_details had already stripped userinfo from bundle.git.origin_url, but that only happens on the local .git route. A URL set in databricks.yml, or returned by the workspace API for a Git folder, was sent to telemetry as written, including any embedded token. Strip userinfo when building the telemetry git info so every route is covered. Extract the stripping from Repository.OriginUrl into git.StripURLCredentials and use it for telemetry too. Co-authored-by: Isaac --- .../databricks.yml | 4 ++++ .../out.test.toml | 2 ++ .../output.txt | 12 ++++++++++++ .../deploy-git-origin-url-credentials/script | 5 +++++ bundle/phases/telemetry.go | 8 +++++--- bundle/phases/telemetry_test.go | 17 +++++++++++++++++ libs/git/repository.go | 11 +++++++++-- libs/git/repository_test.go | 18 ++++++++++++++++++ 8 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 acceptance/bundle/telemetry/deploy-git-origin-url-credentials/databricks.yml create mode 100644 acceptance/bundle/telemetry/deploy-git-origin-url-credentials/out.test.toml create mode 100644 acceptance/bundle/telemetry/deploy-git-origin-url-credentials/output.txt create mode 100644 acceptance/bundle/telemetry/deploy-git-origin-url-credentials/script diff --git a/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/databricks.yml b/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/databricks.yml new file mode 100644 index 00000000000..2fd32017831 --- /dev/null +++ b/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/databricks.yml @@ -0,0 +1,4 @@ +bundle: + name: test-bundle + git: + origin_url: https://user:token@github.com/databricks/cli diff --git a/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/out.test.toml b/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/out.test.toml new file mode 100644 index 00000000000..19b5b15d11f --- /dev/null +++ b/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/output.txt b/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/output.txt new file mode 100644 index 00000000000..cf85ac9be68 --- /dev/null +++ b/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/output.txt @@ -0,0 +1,12 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Files: 1 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 0 unchanged + +>>> cat [OUTPUT_DIR]/out.requests.txt +{ + "git": { + "origin_url": "https://github.com/databricks/cli" + } +} diff --git a/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/script b/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/script new file mode 100644 index 00000000000..a5bee721b53 --- /dev/null +++ b/acceptance/bundle/telemetry/deploy-git-origin-url-credentials/script @@ -0,0 +1,5 @@ +trace $CLI bundle deploy + +trace cat "$OUT_REQUESTS" | jq 'select(has("path") and .path == "/telemetry-ext") | .body.protoLogs[] | fromjson | .entry.databricks_cli_log.bundle_deploy_event | {git}' + +rm "$OUT_REQUESTS" diff --git a/bundle/phases/telemetry.go b/bundle/phases/telemetry.go index 398a0821e08..a66f58d3753 100644 --- a/bundle/phases/telemetry.go +++ b/bundle/phases/telemetry.go @@ -18,6 +18,7 @@ import ( "github.com/databricks/cli/bundle/metrics" "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/dyn" + libgit "github.com/databricks/cli/libs/git" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/telemetry" "github.com/databricks/cli/libs/telemetry/protos" @@ -232,15 +233,16 @@ func pydabsMetrics(b *bundle.Bundle) *protos.BundleDeployPydabs { } // bundleGitInfo builds the git metadata for the deploy event, or nil when the -// bundle root is not inside a git repository. OriginURL already has credentials -// stripped by the load_git_details mutator. +// bundle root is not inside a git repository. OriginURL is stripped here because +// only the local .git route strips it upstream; a value from bundle.git.origin_url +// or from the workspace API for a Git folder is passed through as written. func bundleGitInfo(b *bundle.Bundle) *protos.BundleGitInfo { git := b.Config.Bundle.Git if git.Branch == "" && git.Commit == "" && git.OriginURL == "" { return nil } return &protos.BundleGitInfo{ - OriginURL: git.OriginURL, + OriginURL: libgit.StripURLCredentials(git.OriginURL), Branch: git.Branch, Commit: git.Commit, } diff --git a/bundle/phases/telemetry_test.go b/bundle/phases/telemetry_test.go index 1436ecb7e84..8fc7de5976a 100644 --- a/bundle/phases/telemetry_test.go +++ b/bundle/phases/telemetry_test.go @@ -204,3 +204,20 @@ func TestPydabsMetrics(t *testing.T) { b := &bundle.Bundle{Config: config.Root{Experimental: &config.Experimental{Python: pythonConfig}}, Metrics: metrics} assert.Equal(t, expected, pydabsMetrics(b)) } + +func TestBundleGitInfoStripsOriginURLCredentials(t *testing.T) { + tests := []struct { + originURL string + want string + }{ + {"https://user:token@github.com/databricks/cli", "https://github.com/databricks/cli"}, + {"https://github.com/databricks/cli", "https://github.com/databricks/cli"}, + } + for _, tt := range tests { + t.Run(tt.originURL, func(t *testing.T) { + b := &bundle.Bundle{} + b.Config.Bundle.Git.OriginURL = tt.originURL + assert.Equal(t, tt.want, bundleGitInfo(b).OriginURL) + }) + } +} diff --git a/libs/git/repository.go b/libs/git/repository.go index 8ff8629831a..fb9157d8899 100644 --- a/libs/git/repository.go +++ b/libs/git/repository.go @@ -107,9 +107,11 @@ func (r *Repository) LatestCommit() (string, error) { // return origin url if it's defined, otherwise an empty string func (r *Repository) OriginUrl() string { - rawUrl := r.config.variables["remote.origin.url"] + return StripURLCredentials(r.config.variables["remote.origin.url"]) +} - // Remove username and password from the URL. +// StripURLCredentials removes the username and password from a URL. +func StripURLCredentials(rawUrl string) string { parsedUrl, err := url.Parse(rawUrl) if err != nil { // Git supports https URLs and non standard URLs like "ssh://" or "file://". @@ -118,6 +120,11 @@ func (r *Repository) OriginUrl() string { // because passwords cannot be included in the URL. return rawUrl } + // Leave URLs without credentials as written; re-serializing can rewrite + // them (e.g. escaping spaces). + if parsedUrl.User == nil { + return rawUrl + } // Setting User to nil removes the username and password from the URL when // .String() is called. // See: https://pkg.go.dev/net/url#URL.String diff --git a/libs/git/repository_test.go b/libs/git/repository_test.go index 7c76e003133..c7217fab0a9 100644 --- a/libs/git/repository_test.go +++ b/libs/git/repository_test.go @@ -231,3 +231,21 @@ func TestRepositoryOriginUrlRemovesUserCreds(t *testing.T) { repo.assertOriginUrl(tc.expected) } } + +func TestStripURLCredentials(t *testing.T) { + cases := []struct { + in string + want string + }{ + {"https://user:pass@proxy.example/simple", "https://proxy.example/simple"}, + {"https://token@proxy.example/simple", "https://proxy.example/simple"}, + {"https://proxy.example/simple", "https://proxy.example/simple"}, + {"ssh://git@github.com/databricks/cli.git", "ssh://github.com/databricks/cli.git"}, + {"git@github.com:databricks/cli.git", "git@github.com:databricks/cli.git"}, + {"not a url", "not a url"}, + {"", ""}, + } + for _, tc := range cases { + assert.Equal(t, tc.want, StripURLCredentials(tc.in)) + } +}