Re: [PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Nov 17, 2014, 08:26 UTC
Message-ID: <5469B134.1050306@alum.mit.edu>
In-Reply-To: <xmqqvbmfyo8w.fsf@gitster.dls.corp.google.com>

On 11/16/2014 07:49 PM, Junio C Hamano wrote:

Show 25 quoted lines

> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> There is no reason for $GIT_DIR/config to be executable, plus this
>> change will help clean up repositories affected by the bug that was
>> fixed by the previous commit.
> 
> I do not think we want to do this.
> 
> It is a welcome bugfix to create $GIT_DIR/config without executable
> bit when and only when we create it.  It is very much in line with
> "There is no reason for $GIT_DIR/config to be executable"---we do
> not need to make it executable ourselves, so we shouldn't, but we
> did which was a bug we want to fix in patch 1/2 you posted.
> 
> But with the "preserve existing permissions" fix we did earlier, the
> end users are now allowed to flip the executable bit on for their
> own purpose, and dropping it without knowing why they are doing so
> is simply rude.  And honestly, Git do *not* even want to know why
> the users want to flip the bit.
> 
> So I would suggest not to spend any cycle or any code complexity to
> "repair" existing repositories.  Having that bit on does not hurt
> anybody.  Those who found it curious can flip that bit off and then
> Git with "preserve existing permissions" fix will keep that bit off
> from then on.

I disagree. The point of "preserve existing permissions" was to allow people to make their config files more readable/writable than the default, with the main use case being to help users who want to hide secret information in their config files.

I think it is really far-fetched to imagine that anybody made his config file executable on purpose. Whereas we are *sure* that we have created lots of repositories with config files that were set executable by accident.

Let's redefine the "feature" that was added in 2.1 from "preserve existing permissions" to "preserve existing read/write permissions" and thereby help people clean up the mess we made.

That being said, I still believe that executable config files are not a significant risk in practice, so I'm not going to lose sleep about it either way.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu