mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <>
To: Derrick Stolee via GitGitGadget <>
	Derrick Stolee <>,
	Derrick Stolee <>
Subject: Re: [PATCH] urlmatch: do not allow passwords in URLs by default
Date: Fri, 30 Apr 2021 14:50:20 -0400	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Fri, Apr 30, 2021 at 06:37:24PM +0000, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <>
> Git allows URLs of the following pattern:
>   https://username:password@domain/route
> These URLs are then parsed to pull out the username and password for use
> when authenticating with the URL. Git is careful to anonymize the URL in
> status messages with transport_anonymize_url(), but it stores the URL as
> plaintext in the .git/config file. The password may leak in other ways.

I'm not really opposed to disallowing this entirely (with an escape
hatch, as you have here), because it really is an awful practice for a
lot of reasons. But another option we discussed previously was to allow
the initial clone, but not store the password, which would result in the
user being prompted for subsequent fetches:

I think that third patch there is just too gross. But with the first
two, if you do have a credential helper configured, then:

  git clone

would do what you want: clone with that user/pass, and then store the
result in the credential helper.

> @@ -191,6 +204,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
>  			}
>  			colon_ptr = strchr(norm.buf + scheme_len + 3, ':');
>  			if (colon_ptr) {
> +				die_if_username_password_not_allowed();
>  				passwd_off = (colon_ptr + 1) - norm.buf;
>  				passwd_len = norm.len - passwd_off;
>  				user_len = (passwd_off - 1) - (scheme_len + 3);

It's probably a bit nicer to just ignore the password, which will prompt
the user. But then, it is nicer still to use it just the one time but
not store it in the .git/config file. :)


  reply	other threads:[~2021-04-30 18:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-30 18:37 [PATCH] urlmatch: do not allow passwords in URLs by default Derrick Stolee via GitGitGadget
2021-04-30 18:50 ` Jeff King [this message]
2021-05-03 11:54   ` Derrick Stolee
2021-05-03 14:53     ` Jeff King
2021-05-01  2:00 ` brian m. carlson
2021-05-01  6:39 ` Christian Couder
2021-05-03  3:38   ` Junio C Hamano
2021-05-01  8:44 ` Ævar Arnfjörð Bjarmason
2021-05-01  8:52 ` Ævar Arnfjörð Bjarmason
2021-05-03  8:40   ` Robert Coup

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:

  List information:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \

* 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.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).