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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html