On Fri, May 24, 2019 at 10:31 AM Jeff King <peff@xxxxxxxx> wrote: > > On Fri, May 24, 2019 at 10:05:45AM +0200, Christian Couder wrote: > > 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. > 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).
