threads / patch / 11142

patchgit-cvsserver runs hooks/post-update

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

## tl;dr

3 messages between Dec 5, 2007 and Dec 5, 2007. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Junio C Hamano· Dec 5, 2007, 10:59 UTC · lore

Although we have introduced post-receive, we have not deprecated post-update hook. This adds support for it to emulate receive-pack better.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * I'd like to have the updated cvsserver by 1.5.4-rc0 so I updated the
   series myself.  Ack, improvements, fixes?
 git-cvsserver.perl |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
Show changes to git-cvsserver.perl +6 −0
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 11f5127..ecded3b 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1357,6 +1357,12 @@ sub req_ci
 		close $pipe || die "bad pipe: $! $?";
 	}
 
+	### Then hooks/post-update
+	$hook = $ENV{GIT_DIR}.'hooks/post-update';
+	if (-x $hook) {
+		system($hook, "refs/heads/$state->{module}");
+	}
+
     $updater->update();
 
     # foreach file specified on the command line ...

-- 
1.5.3.7-2134-g53f9
Michael Witten· Dec 5, 2007, 11:07 UTC · re: Junio C Hamano · lore

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

On 5 Dec 2007, at 5:59:12 AM, Junio C Hamano wrote:
Show 6 quoted lines
> +	### Then hooks/post-update
> +	$hook = $ENV{GIT_DIR}.'hooks/post-update';
> +	if (-x $hook) {
> +		system($hook, "refs/heads/$state->{module}");
> +	}
> +

Firstly, I apologize for not getting this small patch done myself; it's very hectic this time of year.

Secondly, I'd like to recall my unanswered question:
> Also, I explicitly decided to pipe input into post-receive
> by hand rather than relying on a system() call that someone
> might exploit maliciously:
	### 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: $! $?";
	}
Show 7 quoted lines
> 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?
Junio C Hamano· Dec 5, 2007, 11:18 UTC · re: Michael Witten · lore

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

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.

← back to recent threads