Re: git log -p unexpected behaviour - security risk?

2013-04-30 Thread John Szakmeister
On Sun, Apr 21, 2013 at 2:42 PM, Junio C Hamano gits...@pobox.com wrote: Jonathan Nieder jrnie...@gmail.com writes: 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

Re: git log -p unexpected behaviour - security risk?

2013-04-30 Thread shawn wilson
Sorta OT, but I'm curious, On Sun, Apr 21, 2013 at 12:09 PM, Jonathan Nieder jrnie...@gmail.com wrote: 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

Re: git log -p unexpected behaviour - security risk?

2013-04-30 Thread Junio C Hamano
John Szakmeister j...@szakmeister.net writes: When I added -c/--cc, I contemplated making -p imply --cc, but decided against it primarily because it is a change in traditional behaviour, and it is easy for users to say --cc instead of -p from the command line. FWIW, security aside, I

Re: git log -p unexpected behaviour - security risk?

2013-04-30 Thread John Szakmeister
On Tue, Apr 30, 2013 at 12:37 PM, Junio C Hamano gits...@pobox.com wrote: John Szakmeister j...@szakmeister.net writes: When I added -c/--cc, I contemplated making -p imply --cc, but decided against it primarily because it is a change in traditional behaviour, and it is easy for users to say

Re: git log -p unexpected behaviour - security risk?

2013-04-30 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes: By the way, these options are _not_ about showing merge commits that introduce code, and they do not help your kind of security. As I repeatedly said, you would need -p -m for that. Actually, while defaulting to --cc may be convenient, it would indeed

Re: git log -p unexpected behaviour - security risk?

2013-04-30 Thread John Szakmeister
On Tue, Apr 30, 2013 at 1:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Junio C Hamano gits...@pobox.com writes: By the way, these options are _not_ about showing merge commits that introduce code, and they do not help your kind of security. As I repeatedly said, you would need -p

Re: git log -p unexpected behaviour - security risk?

2013-04-30 Thread John Tapsell
On 30 April 2013 18:58, John Szakmeister j...@szakmeister.net wrote: On Tue, Apr 30, 2013 at 1:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Junio C Hamano gits...@pobox.com writes: By the way, these options are _not_ about showing merge commits that introduce code, and they do not

Re: git log -p unexpected behaviour - security risk?

2013-04-21 Thread Junio C Hamano
Simon Ruderich si...@ruderich.org writes: diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 104579d..cd35ec7 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -24,6 +24,10 @@ ifndef::git-format-patch[] --patch::

Re: git log -p unexpected behaviour - security risk?

2013-04-21 Thread John Tapsell
On 21 April 2013 08:26, Junio C Hamano gits...@pobox.com wrote: Simon Ruderich si...@ruderich.org writes: diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 104579d..cd35ec7 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@

Re: git log -p unexpected behaviour - security risk?

2013-04-21 Thread Jonathan Nieder
John Tapsell wrote: I'm concerned that noone is taking this security risk seriously. 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. A merge can completely undo

Re: git log -p unexpected behaviour - security risk?

2013-04-21 Thread John Tapsell
On 21 April 2013 11:21, Jonathan Nieder jrnie...@gmail.com wrote: John Tapsell wrote: I'm concerned that noone is taking this security risk seriously. 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,

Re: git log -p unexpected behaviour - security risk?

2013-04-21 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes: That's why if you want to review the code you are pulling in as a whole, it is worthwhile to do git diff HEAD...FETCH_HEAD That is how you ask What code changes does FETCH_HEAD introduce? before putting your stamp of approval on them by

Re: git log -p unexpected behaviour - security risk?

2013-04-21 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes: 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. Making log -p -m a default before -c/--cc was introduced

Re: git log -p unexpected behaviour - security risk?

2013-04-20 Thread Simon Ruderich
On Thu, Apr 11, 2013 at 11:36:26AM +0100, John Tapsell wrote: Is there a way to make --cc default? If you use aliases, something like this is easy: git config --global --add alias.lp 'log --patch --cc' I use aliases heavily, so that's my fix for now. But I think the current behaviour is

git log -p unexpected behaviour - security risk?

2013-04-11 Thread John Tapsell
Hi, I noticed that code that you put in merge will not be visible by default. This seems like a pretty horrible security problem, no? I made the following test tree, with just 3 commits: https://github.com/johnflux/ExampleEvilness.git Doing git log -p shows all very innocent commits.

Re: git log -p unexpected behaviour - security risk?

2013-04-11 Thread Tay Ray Chuan
On Thu, Apr 11, 2013 at 6:36 PM, John Tapsell johnf...@gmail.com wrote: I noticed that code that you put in merge will not be visible by default. This seems like a pretty horrible security problem, no? I made the following test tree, with just 3 commits: