Re: [PATCH 2/2] submodule: munge paths to submodule git directories

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 14, 2018, 21:12 UTC
Message-ID: <20180814211211.GF142615@aiede.svl.corp.google.com>
In-Reply-To: <CAGZ79kZUq5jPqyb=B1ppEi1QhNGmhLXeV6vPn8ouR=YGEN32pg@mail.gmail.com>

Hi,

Stefan Beller wrote:

> On Tue, Aug 14, 2018 at 11:57 AM Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 28 quoted lines

>> Second, what if we store the pathname in config?  We already store the
>> URL there:
>>
>>         [submodule "plugins/hooks"]
>>                 url = https://gerrit.googlesource.com/plugins/hooks
>>
>> So we could (as a followup patch) do something like
>>
>>         [submodule "plugins/hooks"]
>>                 url = https://gerrit.googlesource.com/plugins/hooks
>>                 gitdirname = plugins%2fhooks
>>
>> and use that for lookups instead of regenerating the directory name.
>> What do you think?
>
> As I just looked at worktree code, this sounds intriguing for the wrong
> reason (again), as a user may want to point the gitdirname to a repository
> that they have already on disk outside the actual superproject. They
> would be reinventing worktrees in the submodule space. ;-)
>
> This would open up the security hole that we just had, again.
> So we'd have to make sure that the gitdirname (instead of the
> now meaningless subsection name) is proof to ../ attacks.
>
> I feel uneasy about this as then the user might come in
> and move submodules and repoint the gitdirname...
> to a not url encoded path. Exposing this knob just
> asks for trouble, no?

What if we forbid directory separator characters in the gitdirname?

[...]

> What would happen if gitdirname is changed as part of
> history? (The same problem we have now with changing
> the subsection name)

In this proposal, it would only be read from config, not from .gitmodules.

Thanks, Jonathan