Re: [PATCH v2] fast-import: disallow "." and ".." path components

From: Jeff King <peff@peff.net>
Date: Nov 27, 2024, 14:24 UTC
Message-ID: <20241127142458.GA1712@coredump.intra.peff.net>
In-Reply-To: <Z0Vxb4OVJYeAdgds@pks.im>

On Tue, Nov 26, 2024 at 07:57:57AM +0100, Patrick Steinhardt wrote:

Show 20 quoted lines

> On Mon, Nov 25, 2024 at 07:00:48PM +0000, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> > 
> > If a user specified e.g.
> >    M 100644 :1 ../some-file
> > then fast-import previously would happily create a git history where
> > there is a tree in the top-level directory named "..", and with a file
> > inside that directory named "some-file".  The top-level ".." directory
> > causes problems.  While git checkout will die with errors and fsck will
> > report hasDotdot problems, the user is going to have problems trying to
> > remove the problematic file.  Simply avoid creating this bad history in
> > the first place.
> 
> Makes sense.
> 
> More generally this made me wonder whether we should maybe extract some
> bits out of "fsck.c" so that we don't have to duplicate the checks done
> there in git-fast-import(1). This would for example include checks for
> ".git" and its HFS/NTFS variants as well as tree entry length checks for
> names longer than 4096 characters.

I had the same thought, but I think the right code to be using is verify_path(). That's what ultimately is used to let names into the index from trees, from update-index, or from other tools like git-apply.

So I'd consider that authoritative, and fsck is mostly trying to follow those rules while looking at only a single tree at a time. But fast-import should have the whole path as a string, just like the index code does).

-Peff