Re: [PATCH v6 0/5] config: introduce discovery.bare and protected config

From: Glen Choo <chooglen@google.com>
Date: Jul 12, 2022, 20:47 UTC
Message-ID: <kl6lv8s25a80.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <220709.86zghj8d6i.gmgdl@evledraar.gmail.com>

Thanks for following up. I'm a concerned that this thread will be unproductive if all we're doing is reiterating our own opinions. I'm ok if the conclusion is "agree to disagree", but let's not spend too much time talking circles around one another (myself included, of course:)).

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 22 quoted lines

> On Fri, Jul 01 2022, Glen Choo wrote:
>>> The "more narrow" and "more secure" go hand-in-hand, since if you work
>>> on such servers you'd turn this to "always" because you want to read
>>> such config, but then be left vulnerable to the actual (and muche rarer)
>>> exploit we're trying to prevent.
>>
>> The point that we're not defending bare repo users is fair, but maybe
>> the group we're trying to protect isn't really dedicated Git-serving
>> servers. This exploit requires you to have a bare repo inside the
>> working tree of a non-bare repo. So I think this is less of an issue for
>> a server, and more for "mixed-use" environments with both regular and
>> bare clones.
>
> Yes, but this is only something that's even a question because of an
> artificial limitation your proposal here suffers from.
>
> I.e. in trying to detect nefarious repos where you've got "looks like
> bare" content *tracked* in another repo you're conflating it with *any
> bare repo*.
>
> And the only reason we're doing that seems to me to be a premature
> optimization.

Right, I hear you. Besides performance, let me offer the perspective that I should have led with in the previous email. In this thread and the original "embedded bare repo" one [1], there is a huge diversity of opinion on what the default behavior should be, e.g.:

- How do we detect an embedded bare repo (fsck check? walk up [and check
  if it's tracked]?)
- What to do when we detect one (ignore the config? block the repo?)
- How to preserve workflows that rely on embedded bare repos (some kind
  of (global|per-repo) exception list? allow the repo but not the
  config?)

And rightfully so! There are a lot of options here, so we want to make sure we get the defaults right. But at the same time that implies a pretty slow, difficult process.

On the other hand, I haven't seen nearly as much disagreement on "just refuse to work with bare repos" because it's so restrictive that it probably won't be the default. So it'll have no effect on most users, but still confers protection for the subset of users who can benefit from it. For those who want the problem fixed _today_ (e.g. my employer), this seems like simple, low-hanging fruit that buys time for us to find good default.

FWIW, when time permits I'd be happy to work on that good default (which will probably be some variant of "walk up"), and to pay off the tech debt introduced by this implementation (I have some ideas about how we could improve the config API to achieve this [2]). Hopefully that helps allay some of your concerns?

[1] https://lore.kernel.org/git/kl6lsfqpygsj.fsf@chooglen-macbookpro.roam.corp.google.com [2] https://lore.kernel.org/git/kl6lr13fi9qn.fsf@chooglen-macbookpro.roam.corp.google.com