Re: persistent-https, url insteadof, and `git submodule`

From: Elliott Cable <me@ell.io>
Date: May 26, 2017, 16:22 UTC
Message-ID: <CAPZ477PoSXqahxaQVpO+m==vng==o4vQahrg_WA8Oeh7wmoW0w@mail.gmail.com>
In-Reply-To: <20170520070757.jekykxagzze3t2wy@sigill.intra.peff.net>

Hi! Thanks for the responses (I hope reply-all isn't bad mailing-list etiquette? Feel free to yell at with a direct reply!). For whatever it's worth, as a random user, here's my thoughts:

On Sat, May 20, 2017 at 2:07 AM, Jeff King <peff@peff.net> wrote:

Show 25 quoted lines

> On Fri, May 19, 2017 at 11:55:34PM +0200, Dennis Kaarsemaker wrote:
>> > On Fri, 2017-05-19 at 14:57 -0500, Elliott Cable wrote:
>> > > Presumably this isn't intended behaviour?
>> >
>> > It actually is. git-submodule sets GIT_PROTOCOL_FROM_USER to 0, which
>> > makes git not trust any urls except http(s), git, ssh and file urls
>> > unless you explicitely configure git to allow it. See the
>> > GIT_ALLOW_PROTOCOL section in man git and the git-config section it
>> > links to.
>>
>> 33cfccbbf3 (submodule: allow only certain protocols for submodule
>> fetches, 2015-09-16) says:
>> [...]
>>     But doing it this way is
>>     simpler, and makes it much less likely that we would miss a
>>     case. And since such protocols should be an exception
>>     (especially because nobody who clones from them will be able
>>     to update the submodules!), it's not likely to inconvenience
>>     anyone in practice.
>
> The other approach is to declare that a url rewrite resets the
> protocol-from-user flag to 1. IOW, since the "persistent-https" protocol
> comes from our local config, it's not dangerous and we should behave as
> if the user themselves gave it to us. That makes Elliott's case work out
> of the box.

Well, now that I'm aware of security concerns, `GIT_PROTOCOL_FROM_USER` and `GIT_ALLOW_PROTOCOL`, and so on, I wouldn't *at all* expect `insteadOf` to disable that behaviour. Instead, one of two things seems like a more ideal solution:

1. Most simply, better documentation: mention `GIT_PROTOCOL_FROM_USER`
   explicitly in the documentation of/near `insteadOf`, most
   particularly in the README for `contrib/persistent-https`.

2. Possibly, special-case “higher-security” porcelain (like
   `git-submodule`, as described in 33cfccbbf3) to ignore `insteadOf`
   rewrite-rules without additional, special configuration. This way,
   `git-submodule` works for ignorant users (like me) out of the box,
   just as it previously did, and there's no possible security
   compramise.

Just my 2¢ — thanks for your tireless contributions, loves. <3

⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt