From 803f01eef2bd993cec6512be4a925e4119226f2b Mon Sep 17 00:00:00 2001
From: Tim Hockin <thockin@google.com>
Date: Wed, 13 Dec 2023 16:24:06 -0800
Subject: [PATCH] Try not to log credentials in repo URL

This extracts the username and password from the URL and replaces it
with the bare URL, using the usual credential mechanism for user/pass.
---
 main.go     | 26 +++++++++++++++++++++++---
 test_e2e.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/main.go b/main.go
index eee5ece12..e4dd4ee86 100644
--- a/main.go
+++ b/main.go
@@ -481,6 +481,11 @@ func main() {
 		if *flPassword != "" && *flPasswordFile != "" {
 			handleConfigError(log, true, "ERROR: only one of --password and --password-file may be specified")
 		}
+		if u, err := url.Parse(*flRepo); err == nil { // it may not even parse as a URL, that's OK
+			if u.User != nil {
+				handleConfigError(log, true, "ERROR: credentials may not be specified in --repo when --username is specified")
+			}
+		}
 	} else {
 		if *flPassword != "" {
 			handleConfigError(log, true, "ERROR: --password may only be specified when --username is specified")
@@ -564,6 +569,21 @@ func main() {
 	absTouchFile := makeAbsPath(*flTouchFile, absRoot)
 
 	// Merge credential sources.
+	if *flUsername == "" {
+		// username and user@host URLs are validated as mutually exclusive
+		if u, err := url.Parse(*flRepo); err == nil { // it may not even parse as a URL, that's OK
+			if u.User != nil {
+				if user := u.User.Username(); user != "" {
+					*flUsername = user
+				}
+				if pass, found := u.User.Password(); found {
+					*flPassword = pass
+				}
+				u.User = nil
+				*flRepo = u.String()
+			}
+		}
+	}
 	if *flUsername != "" {
 		cred := credential{
 			URL:          *flRepo,
@@ -1535,7 +1555,7 @@ func (git *repoSync) currentWorktree() (worktree, error) {
 // and tries to clean up any detritus.  This function returns whether the
 // current hash has changed and what the new hash is.
 func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Context) error) (bool, string, error) {
-	git.log.V(3).Info("syncing", "repo", git.repo)
+	git.log.V(3).Info("syncing", "repo", redactURL(git.repo))
 
 	if err := refreshCreds(ctx); err != nil {
 		return false, "", fmt.Errorf("credential refresh failed: %w", err)
@@ -1660,7 +1680,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con
 
 // fetch retrieves the specified ref from the upstream repo.
 func (git *repoSync) fetch(ctx context.Context, ref string) error {
-	git.log.V(2).Info("fetching", "ref", ref, "repo", git.repo)
+	git.log.V(2).Info("fetching", "ref", ref, "repo", redactURL(git.repo))
 
 	// Fetch the ref and do some cleanup, setting or un-setting the repo's
 	// shallow flag as appropriate.
@@ -1711,7 +1731,7 @@ func md5sum(s string) string {
 
 // StoreCredentials stores a username and password for later use.
 func (git *repoSync) StoreCredentials(ctx context.Context, url, username, password string) error {
-	git.log.V(1).Info("storing git credential", "url", url)
+	git.log.V(1).Info("storing git credential", "url", redactURL(url))
 	git.log.V(9).Info("md5 of credential", "url", url, "username", md5sum(username), "password", md5sum(password))
 
 	creds := fmt.Sprintf("url=%v\nusername=%v\npassword=%v\n", url, username, password)
diff --git a/test_e2e.sh b/test_e2e.sh
index 5134b3881..d979e4036 100755
--- a/test_e2e.sh
+++ b/test_e2e.sh
@@ -1812,6 +1812,46 @@ function e2e::auth_http_password() {
     assert_file_eq "$ROOT/link/file" "$FUNCNAME"
 }
 
+##############################################
+# Test HTTP basicauth with a password in the URL
+##############################################
+function e2e::auth_http_password_in_url() {
+    # Run a git-over-HTTP server.
+    CTR=$(docker_run \
+        -v "$REPO":/git/repo:ro \
+        e2e/test/httpd)
+    IP=$(docker_ip "$CTR")
+
+    # Try with wrong username
+    assert_fail \
+        GIT_SYNC \
+            --one-time \
+            --repo="http://wrong:testpass@$IP/repo" \
+            --root="$ROOT" \
+            --link="link"
+    assert_file_absent "$ROOT/link/file"
+
+    # Try with wrong password
+    assert_fail \
+        GIT_SYNC \
+            --one-time \
+            --repo="http://testuser:wrong@$IP/repo" \
+            --root="$ROOT" \
+            --link="link"
+    assert_file_absent "$ROOT/link/file"
+
+    # Try with the right password
+    GIT_SYNC \
+        --one-time \
+        --repo="http://testuser:testpass@$IP/repo" \
+        --root="$ROOT" \
+        --link="link"
+
+    assert_link_exists "$ROOT/link"
+    assert_file_exists "$ROOT/link/file"
+    assert_file_eq "$ROOT/link/file" "$FUNCNAME"
+}
+
 ##############################################
 # Test HTTP basicauth with a password-file
 ##############################################