On Mon, Nov 07, 2016 at 05:03:42PM +0700, Duy Nguyen wrote:
> On Wed, Nov 2, 2016 at 8:08 PM, Jeff King <peff@xxxxxxxx> wrote:
> > The attributes system may sometimes read in-tree files from
> > the filesystem, and sometimes from the index. In the latter
> > case, we do not resolve symbolic links (and are not likely
> > to ever start doing so). Let's open filesystem links with
> > O_NOFOLLOW so that the two cases behave consistently.
>
> This sounds backward to me. The major use case is reading
> .gitattributes on worktree, which follows symlinks so far. Only
> git-archive has a special need to read index-only versions. The
> worktree behavior should influence the in-index one, not the other way
> around. If we could die("BUG" when git-archive is used on symlinks
> (without --worktree-attributes). If people are annoyed by it, they can
> implement symlink folllowing (to another version in index, not on
> worktree).
I agree it feels a little backwards, as we are choosing the
lowest-common denominator of the two (so it would be reasonable to have
the in-index version follow symbolic links, or at least do so on
platforms where core.symlinks is true).
And I'll admit my main motivation is not that index/filesystem parity,
but rather just that:
git clone git://host.com/malicious-repo.git
git log
might create and read symlinks to arbitrary files on the cloner's box.
I'm not sure to what degree to be worried about that. It's not like you
can't make other arbitrary symlinks which are likely to be read if the
user actually starts looking at checked-out files. It's just that we
usually try to make a clone+log of a malicious repository safe.
That being said, I'm not convinced that reading the index version of
.gitattributes and .gitignore is just an optimization. Don't we read the
destination attributes when checking out a new tree? And doesn't merge
need to use the in-index version when we see conflicts?
So I was hoping that this was a practice that is unlikely to be in wide
use, and that we could simply ban in order to keep the attribute and
ignore code simpler and safer, both now and if we change them to do more
in-index lookups.
-Peff