John Tapsell wrote: > Jonathan Nieder wrote: >> If anyone relies on "git log -p" or "git log -p --cc" output to make >> sure that the untrusted code they use doesn't introduce unwanted >> behavior, they are making a serious mistake. [...] > You can't just push all the blame on the user for bad defaults. The thing is, I'm not convinced this is a bad default. "Shows no diff at all for merges" is easy for a person to understand. It is much easier to understand its limitations than -c and --cc. For that reason, it is a much *better* default for security than --cc or -c (even though I believe one of the latter would be a better default for convenience). I agree that this is an important documentation bug, since introductory documentation does not explain clearly enough how "git log -p" will act for merges. >> A merge can completely >> undo important changes made in a side branch and "-c" and "--cc" will >> not show it. > > Wait, what? This is getting even worse then! Can you expand on this please? If a given file matches one of its parents, there is nothing to show in the combined diff format. Otherwise every merge would have a very long diff. If what you really want is the diff against the first parent, you can use -m --first-parent with -p. If you want the diffs against each parent, you can use -m -p. [...] >> Unfortunately that doesn't protect you from >> maliciously written commits that will be encountered when bisecting. >> At some point you have to be able to trust people. > > Seriously? Your reasoning for awful defaults is that you should just > trust people? I didn't set the defaults. I'm explaining how the tool currently behaves in response to your question. A person can do many unfortunate things if you blindly trust them and merge from them. For example, whenever git adds (or plans) support for a new header line in commit objects, before you've upgraded, a prankster can provide a bad value for that header line in objects they hand-craft. "git fsck" in your older version of git will accept the resulting objects on the assumption that they came from a newer version of git, so you won't notice. Later you upgrade Git and "git fsck" considers the objects malformed. Clients with "[transfer] fsckobjects" enabled start to reject your history. That is, this person has made your repository corrupt in the eyes of "git fsck". The usual excellent integrity checking will let you pinpoint the problem to the merge from that untrusted person so you can avoid trusting them again, and all the data will be there to recover without them. So it is auditable later. But this does mean that with the current design, there is some level of trust required to let someone commit into your history unless you inspect their work with a fine-toothed comb. All that said, if someone has ideas for improving git's support for such inspection, that would be great. "-c" just isn't it. "-c" can be a good tool for finding honest mistakes, but it doesn't protect well against an adversary. In the meantime, if you didn't intend to trust those people this much, this might mean your procedures (and git's documentation, for the sake of others in the same boat) need some changes. Sorry to be the bearer of bad news. Hope that helps, Jonathan -- 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