From d5c61f7ad19dea6ae6938dcb5dea7daa0b54676b 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 eee5ece..e4dd4ee 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 5134b38..d979e40 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
 ##############################################