Subject: Re: [PATCH 0/1] Restore the ability to clone repositories owned by another user

From: "brian m. carlson" <[email-masked]>
Date: Fri, 15 Nov 2024 02:02:13 +0000

On 2024-11-15 at 01:14:48, Junio C Hamano wrote:
> "brian m. carlson" <[email-masked]> writes:
> 
> > For a long time, we've told users that the only safe way to operate on
> > an untrusted repository is to clone or fetch from it.  We've even
> > mentioned this policy in a variety of places in our documentation.
> >
> > However, f4aa8c8bb1 ("fetch/clone: detect dubious ownership of local
> > repositories", 2024-04-10), this changed in an attempt to make things
> > more secure.  That broke a lot of user use cases, which have been
> > reported to the list.
> >
> > Because our security model hasn't changed and it's still safe to clone
> > or fetch from an untrusted repository, let's revert a portion of that
> > change to allow us to clone and fetch from repositories owned by a
> > different user.  If a malicious repository were a problem for
> > upload-pack, that would probably also be exploitable on major forges,
> > and if it were a problem on the client side, then we'd also have a
> > problem with untrusted HTTPS remotes, so we're not really adding any
> > security risk here.
> 
> Nice.  Better late than never.

Yeah, I had intended to get to this sooner, but I got busy with other
things, and nobody got to it before me.  I had some time so I thought
I'd send this out now so we can minimize the number of affected versions.

I really appreciate you writing up the original patch for this; it was
very helpful and a great start.

> > Note that I haven't signed off on this patch because it's based on one
> > from Junio and I haven't gotten his sign-off yet.  It's fine to add mine
> > once he's added his.
> 
> You can forge my sign-off on the old patch ;-)

Great.  I suspect you'll probably pick this up as-is, but I've added
both our sign-offs in case we need a v2.  Let me know if you'd prefer me
to send out the unmodified v2, and I can do that.

> The proposed commit log of the patch makes me wonder what should
> happen when neither of the two bits is set.  Not strict, but we do
> not allow ourselves to enter a random repo owned by a stranger.  It
> turns out that "strict" has nothing to do with this lifting of
> excess ownership check, but the dwimming done by suffixing .git,
> etc. to the given pathnames, so there is nothing strange going on.

Exactly.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA