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)) + } +}