Re: [PATCH] Introduce a filter-path argument to git-daemon, for doing custom path transformations

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 14, 2009, 06:58 UTC
Message-ID: <7vvdqcd1zh.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <9e0f31700903121206m3adbabacra655c5d340365f43@mail.gmail.com>

Johan Sørensen <johan@johansorensen.com> writes:

Show 9 quoted lines

>> More importantly, you might want to point out the security concerns of
>> running a script with the full permissions of git-daemon.  (AFAICT from
>> your patch you are not dropping any privileges at any point.)
>
> Do you really think this is needed? It doesn't seem like running the
> hook scripts does anything more than trusting the script author and
> permissions of the hook scripts (?). I see the path-filter script
> exactly the same way, with the exception of having to double-check the
> user supplied path the script receives.

If I am not misreading the patch (I only skimmed it), the script is what is given to the git-daemon process from its command line, so it is under total control of the site owner. It is much much much less problematic than the security worry of allowing random hook scripts to be installed in the repositories hosted at a hosting site. I think Dscho is being a bit too paranoid in this particular case.

However, being paranoid is a good thing when we talk about instructions we give to the end users. The site owner who uses this facility needs to be aware that the script is run as the same user that runs git-daemon, and that more than one instances of the script can be run at the same time. The script writer needs to be careful about using the same scratchpad location for the temporary files the script uses and not letting multiple instances of scripts stomping on each other's toes. These things need to be documented.

Do you run git-daemon from inetd, or standalone, by the way? I am wondering how well it would scale if you spawn an external "filter path" script (by the way, "filter path" sounds as if it checks and conditionally denies access to, or something like that, which is not what you are using it for. It is more about rewriting paths, a la mod_rewrite, and I think the option is misnamed) every time you get a request.