Subject: Re: [PATCH 0/6] open in-tree files with O_NOFOLLOW

From: Junio C Hamano <[email-masked]>
Date: Thu, 25 Feb 2021 11:25:19 -0800

Jeff King <[email-masked]> writes:

> On Tue, Feb 16, 2021 at 07:48:23AM -0500, Jeff King wrote:
>
>> I am beginning to wonder if just opening them all with O_NOFOLLOW (and a
>> hacky 2-syscall fallback for portability) might be less ugly than all of
>> this.
>
> So here's what that series might look like. It would replace all of this
> verify_path() stuff entirely (and fsck, though we might want to add
> detection to fsck just as an informational thing). It gives similar
> protections, and would similarly force people using an in-tree symlink
> to stop doing that. But it makes it much less of a pain to do so,
> because they can still check out, etc; the symlinks just won't be
> followed.
>
> I think we could even use the same technique to roll back the
> restrictions on .gitmodules being a symlink. That one makes me a bit
> more nervous, just because we also write it. I _think_ that might be
> safe, because we only do so using a temp file and rename(), which should
> replace the symlink.
>
>   [1/6]: add open_nofollow() helper
>   [2/6]: attr: convert "macro_ok" into a flags field
>   [3/6]: exclude: add flags parameter to add_patterns()
>   [4/6]: attr: do not respect symlinks for in-tree .gitattributes
>   [5/6]: exclude: do not respect symlinks for in-tree .gitignore
>   [6/6]: mailmap: do not respect symlinks for in-tree .mailmap
>
>  attr.c                    | 60 +++++++++++++++++++++++++--------------
>  builtin/sparse-checkout.c |  8 +++---
>  dir.c                     | 21 ++++++++++----
>  dir.h                     |  3 +-
>  git-compat-util.h         |  7 +++++
>  mailmap.c                 | 22 ++++++++++----
>  t/t0003-attributes.sh     | 36 +++++++++++++++++++++--
>  t/t0008-ignores.sh        | 34 ++++++++++++++++++++++
>  t/t4203-mailmap.sh        | 31 ++++++++++++++++++++
>  wrapper.c                 | 16 +++++++++++
>  10 files changed, 197 insertions(+), 41 deletions(-)

So, I've read these changes and they all looked quite reasonable.

Where do we want to go from here?

Merge it down and forget about the changes in verify_path() and fsck
in the jk/symlinked-dotgitx-files topic?  Do we want to also cover
the .gitmodules file with the same mechansim?

Thanks.