All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH] clone/fetch: anonymize URLs in the reflog
Date: Mon, 01 Jun 2020 19:20:02 +0000	[thread overview]
Message-ID: <pull.797.git.git.1591039202561.gitgitgadget@gmail.com> (raw)

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Even if we strongly discourage putting credentials into the URLs passed
via the command-line, there _is_ support for that, and users _do_ do
that.

Let's scrub them before writing them to the reflog.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    Anonymize URLs in the reflog
    
    This came up in an internal audit, but we do not consider this to be a
    big deal: the reflog is local and not really shared with anybody.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-797%2Fdscho%2Fanonymize-clone-reflog-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-797/dscho/anonymize-clone-reflog-v1
Pull-Request: https://github.com/git/git/pull/797

 builtin/clone.c            | 10 ++++++----
 builtin/fetch.c            |  9 +++++++--
 t/t5541-http-push-smart.sh | 15 +++++++++++++++
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 1ad26f4d8c8..5fe637a6702 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -938,7 +938,7 @@ static int path_exists(const char *path)
 int cmd_clone(int argc, const char **argv, const char *prefix)
 {
 	int is_bundle = 0, is_local;
-	const char *repo_name, *repo, *work_tree, *git_dir;
+	const char *repo_name, *repo, *display_repo, *work_tree, *git_dir;
 	char *path, *dir;
 	int dest_exists;
 	const struct ref *refs, *remote_head;
@@ -993,11 +993,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	path = get_repo_path(repo_name, &is_bundle);
 	if (path)
-		repo = absolute_pathdup(repo_name);
+		display_repo = repo = absolute_pathdup(repo_name);
 	else if (!strchr(repo_name, ':'))
 		die(_("repository '%s' does not exist"), repo_name);
-	else
+	else {
 		repo = repo_name;
+		display_repo = transport_anonymize_url(repo);
+	}
 
 	/* no need to be strict, transport_set_option() will validate it again */
 	if (option_depth && atoi(option_depth) < 1)
@@ -1014,7 +1016,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		die(_("destination path '%s' already exists and is not "
 			"an empty directory."), dir);
 
-	strbuf_addf(&reflog_msg, "clone: from %s", repo);
+	strbuf_addf(&reflog_msg, "clone: from %s", display_repo);
 
 	if (option_bare)
 		work_tree = NULL;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bf6bab80fab..d58b7572114 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1765,8 +1765,13 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 
 	/* Record the command line for the reflog */
 	strbuf_addstr(&default_rla, "fetch");
-	for (i = 1; i < argc; i++)
-		strbuf_addf(&default_rla, " %s", argv[i]);
+	for (i = 1; i < argc; i++) {
+		/* This handles non-URLs gracefully */
+		char *anon = transport_anonymize_url(argv[i]);
+
+		strbuf_addf(&default_rla, " %s", anon);
+		free(anon);
+	}
 
 	fetch_config_from_gitmodules(&submodule_fetch_jobs_config,
 				     &recurse_submodules);
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 23be8ce92d6..2d60381a5e7 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -456,6 +456,21 @@ test_expect_success 'push status output scrubs password' '
 	grep "^To $HTTPD_URL/smart/test_repo.git" status
 '
 
+test_expect_success 'clone/fetch scrubs password from reflogs' '
+	cd "$ROOT_PATH" &&
+	git clone "$HTTPD_URL_USER_PASS/smart/test_repo.git" \
+		reflog-test &&
+	cd reflog-test &&
+	test_commit prepare-for-force-fetch &&
+	git switch -c away &&
+	git fetch "$HTTPD_URL_USER_PASS/smart/test_repo.git" \
+		+master:master &&
+	# should have been scrubbed down to vanilla URL
+	git log -g master >reflog &&
+	grep "$HTTPD_URL" reflog &&
+	! grep "$HTTPD_URL_USER_PASS" reflog
+'
+
 test_expect_success 'colorize errors/hints' '
 	cd "$ROOT_PATH"/test_repo_clone &&
 	test_must_fail git -c color.transport=always -c color.advice=always \

base-commit: af6b65d45ef179ed52087e80cb089f6b2349f4ec
-- 
gitgitgadget

             reply	other threads:[~2020-06-01 19:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-01 19:20 Johannes Schindelin via GitGitGadget [this message]
2020-06-01 21:47 ` [PATCH] clone/fetch: anonymize URLs in the reflog Jeff King
2020-06-02 16:55   ` Junio C Hamano
2020-06-04 20:08 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2020-06-04 20:30   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.797.git.git.1591039202561.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.