Re: Dropping '+' from fetch = +refs/heads/*:refs/remotes/origin/*?

> On Thu, Sep 01, 2011 at 11:25:31AM -0700, Junio C Hamano wrote:
> 
>> Suggested reading:
>>
>>   http://git-blame.blogspot.com/2011/08/how-to-inject-malicious-commit-to-git.html
>>
>> I am wondering if we are better off applying something along the lines of
>> this patch, so that with the default configuration, users can notice if
>> their upstream unexpectedly rewound their branches.
> 
> Hmm. This feels like it's subtly changing the meaning of
> refs/remotes/$remote/*.
> 
> Right now, I think of it as a local cache for whatever the remote side
> has. In other words, a way of separating the network-fetching parts of
> the workflow from the local parts. And in that sense, it is perfectly
> reasonable to overwrite with what the other side has, whether they
> rewind or not, because we are just representing what they have. And
> since we keep a reflog, it's not as if the previous state is lost to us
> locally.
> 
> But with this change, we are making a policy judgement about what to
> fetch. And as you noticed, it means that users need to start telling git
> about their policy (e.g., mentioning in the refspecs that pu can rewind)
> in order to keep fetch working.
> 
> So I consider that a downside, because it's extra work for the user[1].
> What are the upsides?
> 
> Is this about preventing workflow-related mistakes where people
> accidentally merge in rebased commits, creating annoying shadow
> histories?
> 
> Is it about preventing malicious rewinds from infecting downstream
> repositories?
> 
> If the former, then I suspect we need to give more guidance to the user
> than saying "reject, non-fast-forward". What then? Should they "fetch
> -f"?  Or "pull --rebase" (actually, how do they even fetch the branch
> now for such a pull --rebase)? Or talk out-of-band to the repo owner?
> 
> If the latter, then I think we should be specific about the attack
> scenarios, and what happens with and without this config. And if it's a
> security precaution, what cases doesn't it cover (e.g., initial clone is
> still vulnerable, as is a one-off pull. As are are malicious insertion
> attacks that don't involve rewinding).
> 
> And then we can weigh the upsides and the downsides.
> 
> -Peff
> 
> [1] What I really don't like is that cloning git.git is no longer:
> 
>       git clone git://git.kernel.org/pub/scm/git/git.git
> 
>     which is a minimal as it can be, but becomes:
> 
>       git clone git://git.kernel.org/pub/scm/git/git.git
>       cd git
>       git config --add remote.origin.fetch +refs/heads/pu:refs/remotes/origin/pu
> 
>     It's not that my fingers are too tired to do all that typing, but
>     rather that the first set of instructions is very easy to explain,
>     and the second one is full of magic and head-scratching about why
>     git isn't handling this magic itself.
> 
>     It would be considerably nicer if the server had some way of saying
>     "I expect this branch to be rewound". Which has been discussed off
>     and on over the years, as I recall.