Re: git submodule: update=!command

From: Chris Packham <judge.packham@gmail.com>
Date: Mar 18, 2015, 07:38 UTC
Message-ID: <CAFOYHZA=d94swSvfrR0+RdfVrf8a5RwwnYPphtnKz44O_nFeCg@mail.gmail.com>
In-Reply-To: <20150317195030.GA18725@peff.net>

A little late to this thread

On Wed, Mar 18, 2015 at 8:50 AM, Jeff King <peff@peff.net> wrote:

Show 18 quoted lines

> On Tue, Mar 17, 2015 at 03:28:57PM -0400, Ryan Lortie wrote:
>
>> The first is a question about git's basic policy with respect to things
>> like this.  I hope that it's safe to assume that running 'git' commands
>> on repositories downloaded from potentially-hostile places will never
>> result in the authors of those repositories being able to run code on my
>> machine.
>
> Definitely, our policy is that downloading a git repository should not
> result in arbitrary code being run. If there is a case of that, it would
> be a serious security bug.
>
> I am not an expert on submodules, but I think the security module there
> is:
>
>   1. You can do whatever you like in submodule.*.update entries in
>      .git/config, including arbitrary code. Nobody but the user can
>      write to it.

Which was always the intention of the !command feature. It's for users who want to use additional git porcelains that need some help dealing with submodule updates (e.g stgit).

Show 8 quoted lines

>   2. The submodule code may migrate entries from .gitmodules into
>      .git/config, but does so with an allow-known-good whitelist (see
>      git-submodule.sh lines 622-637).
>
> So AFAICT there's no bug here, and the system is working as designed.
> It might be worth mentioning that restriction in the submodule
> documentation, if only to prevent non-malicious people from wondering
> why adding "!foo" does not work in .gitmodules.

At the time the !command feature and copying of update config from .gitmodules slid past each other on the list. But out of that I think we got a much better handling that provides security and version compatibility.

Show 18 quoted lines

>> If that is true then, the second request would be to spell this out more
>> explicitly in the relevant documentation.  I'm happy to write a patch to
>> do that, if it is deemed appropriate.
>
> Yeah, spelling out the security model more explicitly would be good.
> There is also some subtlety around hooks. Doing:
>
>   git clone user@host:/path/to/repo.git local
>
> should never run code controlled by "repo.git" as "user@host". But
> doing:
>
>   ssh user@host 'cd /path/to/repo.git && git log'
>
> will respect the .git/config in repo.git, which may include arbitrary
> commands.
>
> -Peff