Merge pull request #5473 from libgit2/ethomson/v0.28.5v0.28.5maint/v0.28

fetchhead: strip credentials from remote URL

6 files changed, 108 insertions, 4 deletions

diff --git a/docs/changelog.md b/docs/changelog.md
index e882875eb..b842e3191 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -43,6 +43,11 @@ This is a bugfix release with the following changes:
 
 - Fixes for several memory leaks.
 
+- When fetching from an anonymous remote using a URL with authentication
+  information provided in the URL (eg `https://foo:bar@example.com/repo`),
+  we would erroneously include the literal URL in the FETCH_HEAD file.
+  We now remove that to match git's behavior.
+
 v0.28.4
 --------
 
diff --git a/src/fetchhead.c b/src/fetchhead.c
index bdded029a..5dd82fc76 100644
--- a/src/fetchhead.c
+++ b/src/fetchhead.c
@@ -13,6 +13,7 @@
 #include "buffer.h"
 #include "fileops.h"
 #include "filebuf.h"
+#include "netops.h"
 #include "refs.h"
 #include "repository.h"
 
@@ -36,6 +37,33 @@ int git_fetchhead_ref_cmp(const void *a, const void *b)
 	return 0;
 }
 
+static char *sanitized_remote_url(const char *remote_url)
+{
+	gitno_connection_data url = {0};
+	char *sanitized = NULL;
+	int error;
+
+	if (gitno_connection_data_from_url(&url, remote_url, NULL) == 0) {
+		git_buf buf = GIT_BUF_INIT;
+
+		git__free(url.user);
+		git__free(url.pass);
+		url.user = url.pass = NULL;
+
+		if ((error = gitno_connection_data_fmt(&buf, &url)) < 0)
+			goto fallback;
+
+		sanitized = git_buf_detach(&buf);
+	}
+
+fallback:
+	if (!sanitized)
+		sanitized = git__strdup(remote_url);
+
+	gitno_connection_data_free_ptrs(&url);
+	return sanitized;
+}
+
 int git_fetchhead_ref_create(
 	git_fetchhead_ref **out,
 	git_oid *oid,
@@ -57,11 +85,15 @@ int git_fetchhead_ref_create(
 	git_oid_cpy(&fetchhead_ref->oid, oid);
 	fetchhead_ref->is_merge = is_merge;
 
-	if (ref_name)
+	if (ref_name) {
 		fetchhead_ref->ref_name = git__strdup(ref_name);
+		GIT_ERROR_CHECK_ALLOC(fetchhead_ref->ref_name);
+	}
 
-	if (remote_url)
-		fetchhead_ref->remote_url = git__strdup(remote_url);
+	if (remote_url) {
+		fetchhead_ref->remote_url = sanitized_remote_url(remote_url);
+		GIT_ERROR_CHECK_ALLOC(fetchhead_ref->remote_url);
+	}
 
 	*out = fetchhead_ref;
 
diff --git a/src/netops.c b/src/netops.c
index ecbc2aebe..f63ff380a 100644
--- a/src/netops.c
+++ b/src/netops.c
@@ -206,6 +206,35 @@ cleanup:
 	return error;
 }
 
+int gitno_connection_data_fmt(git_buf *buf, gitno_connection_data *d)
+{
+	if (d->host) {
+		git_buf_puts(buf, d->use_ssl ? prefix_https : prefix_http);
+
+		if (d->user) {
+			git_buf_puts(buf, d->user);
+
+			if (d->pass) {
+				git_buf_puts(buf, ":");
+				git_buf_puts(buf, d->pass);
+			}
+
+			git_buf_putc(buf, '@');
+		}
+
+		git_buf_puts(buf, d->host);
+
+		if (d->port && strcmp(d->port, gitno__default_port(d))) {
+			git_buf_putc(buf, ':');
+			git_buf_puts(buf, d->port);
+		}
+	}
+
+	git_buf_puts(buf, d->path ? d->path : "/");
+
+	return git_buf_oom(buf) ? -1 : 0;
+}
+
 void gitno_connection_data_free_ptrs(gitno_connection_data *d)
 {
 	git__free(d->host); d->host = NULL;
diff --git a/src/netops.h b/src/netops.h
index f376bd911..fcdbccf0d 100644
--- a/src/netops.h
+++ b/src/netops.h
@@ -84,6 +84,9 @@ int gitno_connection_data_from_url(
 		const char *url,
 		const char *service_suffix);
 
+/* Format a URL into a buffer */
+int gitno_connection_data_fmt(git_buf *buf, gitno_connection_data *data);
+
 /* This frees all the pointers IN the struct, but not the struct itself. */
 void gitno_connection_data_free_ptrs(gitno_connection_data *data);
 
diff --git a/tests/fetchhead/nonetwork.c b/tests/fetchhead/nonetwork.c
index 658943273..ccfb4f773 100644
--- a/tests/fetchhead/nonetwork.c
+++ b/tests/fetchhead/nonetwork.c
@@ -108,7 +108,7 @@ void test_fetchhead_nonetwork__write(void)
 typedef struct {
 	git_vector *fetchhead_vector;
 	size_t idx;
-} fetchhead_ref_cb_data; 
+} fetchhead_ref_cb_data;
 
 static int fetchhead_ref_cb(const char *name, const char *url,
 	const git_oid *oid, unsigned int is_merge, void *payload)
@@ -493,3 +493,21 @@ void test_fetchhead_nonetwork__create_with_multiple_refspecs(void)
 	git_remote_free(remote);
 	git_buf_dispose(&path);
 }
+
+void test_fetchhead_nonetwork__credentials_are_stripped(void)
+{
+	git_fetchhead_ref *ref;
+	git_oid oid;
+
+	cl_git_pass(git_oid_fromstr(&oid, "49322bb17d3acc9146f98c97d078513228bbf3c0"));
+	cl_git_pass(git_fetchhead_ref_create(&ref, &oid, 0,
+		"refs/tags/commit_tree", "http://foo:bar@github.com/libgit2/TestGitRepository"));
+	cl_assert_equal_s(ref->remote_url, "http://github.com/libgit2/TestGitRepository");
+	git_fetchhead_ref_free(ref);
+
+	cl_git_pass(git_oid_fromstr(&oid, "49322bb17d3acc9146f98c97d078513228bbf3c0"));
+	cl_git_pass(git_fetchhead_ref_create(&ref, &oid, 0,
+		"refs/tags/commit_tree", "https://foo:bar@github.com/libgit2/TestGitRepository"));
+	cl_assert_equal_s(ref->remote_url, "https://github.com/libgit2/TestGitRepository");
+	git_fetchhead_ref_free(ref);
+}
diff --git a/tests/online/fetchhead.c b/tests/online/fetchhead.c
index ae72dde57..5e3586654 100644
--- a/tests/online/fetchhead.c
+++ b/tests/online/fetchhead.c
@@ -154,3 +154,20 @@ void test_online_fetchhead__colon_only_dst_refspec_creates_no_branch(void)
 
 	cl_assert_equal_i(refs, count_references());
 }
+
+void test_online_fetchhead__creds_get_stripped(void)
+{
+	git_buf buf = GIT_BUF_INIT;
+	git_remote *remote;
+
+	cl_git_pass(git_repository_init(&g_repo, "./foo", 0));
+	cl_git_pass(git_remote_create_anonymous(&remote, g_repo, "https://libgit3:libgit3@bitbucket.org/libgit2/testgitrepository.git"));
+	cl_git_pass(git_remote_fetch(remote, NULL, NULL, NULL));
+
+	cl_git_pass(git_futils_readbuffer(&buf, "./foo/.git/FETCH_HEAD"));
+	cl_assert_equal_s(buf.ptr,
+		"49322bb17d3acc9146f98c97d078513228bbf3c0\t\thttps://bitbucket.org/libgit2/testgitrepository.git\n");
+
+	git_remote_free(remote);
+	git_buf_dispose(&buf);
+}