Re: [PATCH] git-cvsserver runs hooks/post-update

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 5, 2007, 11:18 UTC
Message-ID: <7vtzmxs83h.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <7245A47D-143F-4DC7-8414-D8C99C0ED9C3@mit.edu>

Michael Witten <mfwitten@MIT.EDU> writes:

Show 21 quoted lines

> 	### Emulate git-receive-pack by running hooks/post-receive
> 	my $hook = $ENV{GIT_DIR}.'hooks/post-receive';
> 	if( -x $hook ) {
> 		open(my $pipe, "| $hook") || die "can't fork $!";
>
> 		local $SIG{PIPE} = sub { die 'pipe broke' };
>
> 		print $pipe "$parenthash $commithash refs/heads/$state->{module}\n";
>
> 		close $pipe || die "bad pipe: $! $?";
> 	}
>
>> Unfortunately, it turns out that open() with a pipe essentially
>> invokes system(); the solution is to fork a child process and
>> then to turn the child into the process with which communication
>> is desired via a call to exec().
>>
>> Because the rest of git-cvsserver.perl uses explicit system()
>> calls, I have been wondering if I am being overly cautious.
>
> Am I being overly cautious?

I do not think you are. open($fh, "| $hook") can be confused with any IFS in $hook (there is no problem with 'hooks/post-receive', but $ENV{GIT_DIR} part can have pretty much anything other than NUL), so if anything, you are not being careful enough. Other parts of cvsserver seem to be more careful by doing open($fh, '-|', @cmd), which does not have this problem.

The execution environment of post-receive is probably wrong; I think receive-pack runs the hooks with their $CWD = $GIT_DIR.