Re: [PATCH 1/2] fsck: make symlinked .gitignore and .gitattributes a warning

From: Jeff King <peff@peff.net>
Date: Feb 16, 2021, 12:48 UTC
Message-ID: <YCu/FoLl8p15mwio@coredump.intra.peff.net>
In-Reply-To: <YCsc0OePtrotjeg5@coredump.intra.peff.net>

On Mon, Feb 15, 2021 at 08:16:00PM -0500, Jeff King wrote:

> That said, they'd probably want to checkout those old commits, too. So
> we probably do need a config override, even if it's a broad one ("trust
> me, this repo is OK, just allow symlinks for these special files").

Another option here is setting core.symlinks to false. That works more broadly than just the one symlink, though.

It might be possible to apply that same setting (perhaps automatically, even) to just these .gitattributes, etc, metafiles. That may get tricky, though, as we'd need to do it not just on checkout, but any time we're considering the file (because we wouldn't want "git add" to re-add it as a non-symlink, nor git-diff to report it, etc).

Show 13 quoted lines

> > So aren't we both making the fsck check too loose and the client too
> > strict? Would anyone care if this was an error on fsck if we did the "is
> > outside repo?" check?
> 
> An outside-the-repo check would probably be less invasive, but:
> 
>   - it still allows broken setups
> 
>   - it's significantly more complex. I know that the implementation I
>     showed errs on the side of complaining in at least some cases
>     (because it doesn't know if intermediate paths are themselves
>     symlinks). But I'd worry there are also cases where it covers too
>     little, nullifying the protection.

Adding to the "complexity" point: it's also impossible to implement via fsck, where we do not have the full path of the tree entry. We could live without the fsck support if need be, though.

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.

-Peff