Re: how does "clone --filter=sparse:path" work?

From: Christian Couder <christian.couder@gmail.com>
Date: May 24, 2019, 09:27 UTC
Message-ID: <CAP8UFD3bVH5w=DeMi8R=_O=oyatWj8coh_MxT4kTyJyp1+pOMw@mail.gmail.com>
In-Reply-To: <20190524083142.GC9082@sigill.intra.peff.net>

On Fri, May 24, 2019 at 10:31 AM Jeff King <peff@peff.net> wrote:

>
> On Fri, May 24, 2019 at 10:05:45AM +0200, Christian Couder wrote:

Show 19 quoted lines

> > The way I see it could be restricted is by adding a config option on
> > the server, maybe called "uploadpack.sparsePathFilter", to tell which
> > filenames can be accessed using "--filter=sparse:path=".
> >
> > For example with uploadpack.sparsePathFilter set to
> > "/home/user/git/sparse/*" and "--filter=sparse:path=foo" then
> > "/home/user/git/sparse/foo" on the server would be used if it exists.
> > (Of course care should be taken that things like
> > "--filter=sparse:path=bar/../../foo" are rejected.)
> >
> > If uploadpack.sparsePathFilter is unset or set to "false", then
> > "--filter=sparse:path=<stuff>" would always error out.
> >
> > Is this what you had in mind?
>
> My plan had been to disallow it entirely, and allow some mechanism by
> which the client could specify the actual set of sparse paths itself
> (which it might get from a local file, or communicated in some
> out-of-band way to the user cloning, etc).

I think that indeed disallowing "sparse:path" is the simplest and safest way to go. And I agree that a "mechanism by which the client could specify the actual set of sparse paths itself" would be really nice. I think it might be more complex and take a significant amount of time to implement though.

Show 5 quoted lines

> If we just want a mechanism for the server to provide a pre-made sparse
> list, then I think pointing people at sparse:oid=<blob> is simpler
> there. I.e., your "foo" becomes "refs/sparse/foo" or even "HEAD:.sparse"
> or similar, and the server admin just sticks the content into the repo
> instead of dealing with exposing filesystem paths to the client.

I agree that it is simpler to just use "sparse:oid" which already works. I just thought that some servers might want to provide pre-made sparse lists that aren't in the repo so that client cannot possibly change them (by pushing into the repo), and that "sparse:path" could be used for that purpose.

Now if no one is currently interested in providing pre-made sparse lists that aren't in the repo, then I am ok to just disable "sparse:path" for now, and I might send a patch to do it soon. It will at least fix the security issue with "sparse:path" and thus enable people interested in using "sparse:oid" to start doing so (without opening a big security hole).