Re: [PATCH v4 2/2] interpret-trailers: add option for in-place editing

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 19, 2016, 18:10 UTC
Message-ID: <CAPig+cRozqCKdC2+nyG-UM6xFo_sSqa7OhGgcycyyDQujZHtHA@mail.gmail.com>
In-Reply-To: <CAPig+cRi2knygjeaMtojAr65BE71B-z7q+s8V5rcGrV9Qja6jw@mail.gmail.com>

On Tue, Jan 19, 2016 at 12:56 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 15 quoted lines

> On Tue, Jan 19, 2016 at 12:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>>> I think POSIXPERM is all you need for this case; SANITY doesn't buy
>>> you anything, if I understand correctly.
>>>
>>>>> +     cat basic_message >message &&
>>>>> +     chmod -r message &&
>>>>> +     test_must_fail git interpret-trailers --trailer "Reviewed-by: Alice" --in-place message &&
>>
>> The purpose of "chmod -r message" is to force interpret-trailers to
>> fail due to its input being unreadable; without SANITY, i.e. running
>> this test as root, the command would happily read from message that
>> is marked as unreadable by anybody, and test_must_fail will not pass.
>
> Makes sense. I never run tests as root, thus wasn't thinking along those lines.

On reflection, this doesn't make sense to me. Perhaps I'm missing something obvious.

My understanding is that SANITY is an expectation that directory permissions work in an expected POSIXy way: that is, a file can't be deleted when its containing directory lacks 'write', and a file can't be read/accessed when the directory has neither 'read' nor 'execute'. This doesn't say anything about root not being allowed to read a file when the file itself lacks 'read'.

As far as I can tell, as coded, this test will *always* fail as root since root will always be able to read 'message'.