Re: [PATCH] urlmatch: create fetch.credentialsInUrl config

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, peff@peff.net, me@ttaylorr.com,
	avarab@gmail.com, christian.couder@gmail.com, jrnieder@gmail.com,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	Robert Coup <robert.coup@koordinates.com>,
	Derrick Stolee <derrickstolee@github.com>
Subject: Re: [PATCH] urlmatch: create fetch.credentialsInUrl config
Date: Tue, 24 May 2022 13:51:40 +0200 (CEST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.2205241346590.352@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <xmqqr14kqdag.fsf@gitster.g>

Hi Junio,

On Mon, 23 May 2022, Junio C Hamano wrote:

> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> >     urlmatch: create fetch.credentialsInUrl config
> >
> >     This is a modified version of the patch I submitted a while ago [1].
> >
> >     Based on the feedback, changing the behavior to fail by default was not
> >     a good approach. Further, the idea to stop storing the credentials in
> >     config and redirect them to a credential manager was already considered
> >     by Peff [2] but not merged.
>
> I just peeked [2] and I am not sure why we didn't X-<.  The solution
> there covers "git clone" that records the origin URL but this one
> would cover URL regardless of where the URL came from---as long as
> an insecure URL is used, we warn or die, and it is even against the
> URL that came from the command line.
>
> In a sense, I think these are more or less orthogonal.  [2]'s "clone
> can strip the user:pass from the URL it writes to the config, while
> passing user:pass to the credential API", especially if it is
> extended to "git remote add", would stop two common avenues that
> such an insecure URL can go to the configuration file.  The approach
> taken by this patch would complement it to a degree, as long as the
> user cares.
>
> I am not sure if there is a legitimate case where the user does not
> care, though.  For a script, it may be handy if a URL can contain an
> ever-changing user:pass pair, where the pass is generated by
> something like s/key, for example, and for such a command line that
> knowingly have user:pass pair, having to set the configuration to
> "ignore" may be cumbersome.

To provide one data point: a few of Git for Windows' automated builds use
the `https://user@pass:host/` form to clone and push, using a Personal
Access Token as password (that is of course marked as a secret, read: it
will get redacted out of the logs).

So yes, there are scripts that rely on Git's current behavior to work.

If Git changes behavior, I will have to adjust those build definitions.

In this instance, I believe that the benefit of safeguarding Git's users
outweighs the burden of having to adjust such scripts/definitions.

Ciao,
Dscho