{"thread":{"id":"11142","subject":"[PATCH] git-cvsserver runs hooks/post-update","startedAt":"2007-12-05T10:59:12Z","lastAt":"2007-12-05T11:18:26Z","messageCount":3,"participants":["Junio C Hamano","Michael Witten"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"62001","messageId":"7v63zdtnjz.fsf@gitster.siamese.dyndns.org","threadId":"11142","inReplyTo":null,"subject":"[PATCH] git-cvsserver runs hooks/post-update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-05T10:59:12Z","receivedAt":"2007-12-05T10:59:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Although we have introduced post-receive, we have not deprecated post-update\nhook.  This adds support for it to emulate receive-pack better.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * I'd like to have the updated cvsserver by 1.5.4-rc0 so I updated the\n   series myself.  Ack, improvements, fixes?\n\n git-cvsserver.perl |    6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 11f5127..ecded3b 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -1357,6 +1357,12 @@ sub req_ci\n \t\tclose $pipe || die \"bad pipe: $! $?\";\n \t}\n \n+\t### Then hooks/post-update\n+\t$hook = $ENV{GIT_DIR}.'hooks/post-update';\n+\tif (-x $hook) {\n+\t\tsystem($hook, \"refs/heads/$state->{module}\");\n+\t}\n+\n     $updater->update();\n \n     # foreach file specified on the command line ...\n-- \n1.5.3.7-2134-g53f9\n"},{"id":"62003","messageId":"7245A47D-143F-4DC7-8414-D8C99C0ED9C3@mit.edu","threadId":"11142","inReplyTo":"7v63zdtnjz.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsserver runs hooks/post-update","fromName":"Michael Witten","fromEmail":"mfwitten@mit.edu","sentAt":"2007-12-05T11:07:29Z","receivedAt":"2007-12-05T11:07:29Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"\nOn 5 Dec 2007, at 5:59:12 AM, Junio C Hamano wrote:\n\n> +\t### Then hooks/post-update\n> +\t$hook = $ENV{GIT_DIR}.'hooks/post-update';\n> +\tif (-x $hook) {\n> +\t\tsystem($hook, \"refs/heads/$state->{module}\");\n> +\t}\n> +\n\nFirstly, I apologize for not getting this small patch done myself;\nit's very hectic this time of year.\n\nSecondly, I'd like to recall my unanswered question:\n\n> Also, I explicitly decided to pipe input into post-receive\n> by hand rather than relying on a system() call that someone\n> might exploit maliciously:\n\n\t### Emulate git-receive-pack by running hooks/post-receive\n\tmy $hook = $ENV{GIT_DIR}.'hooks/post-receive';\n\tif( -x $hook ) {\n\t\topen(my $pipe, \"| $hook\") || die \"can't fork $!\";\n\n\t\tlocal $SIG{PIPE} = sub { die 'pipe broke' };\n\n\t\tprint $pipe \"$parenthash $commithash refs/heads/$state->{module}\\n\";\n\n\t\tclose $pipe || die \"bad pipe: $! $?\";\n\t}\n\n> Unfortunately, it turns out that open() with a pipe essentially\n> invokes system(); the solution is to fork a child process and\n> then to turn the child into the process with which communication\n> is desired via a call to exec().\n>\n> Because the rest of git-cvsserver.perl uses explicit system()\n> calls, I have been wondering if I am being overly cautious.\n\nAm I being overly cautious?\n"},{"id":"62006","messageId":"7vtzmxs83h.fsf@gitster.siamese.dyndns.org","threadId":"11142","inReplyTo":"7245A47D-143F-4DC7-8414-D8C99C0ED9C3@mit.edu","subject":"Re: [PATCH] git-cvsserver runs hooks/post-update","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-05T11:18:26Z","receivedAt":"2007-12-05T11:18:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Witten <mfwitten@MIT.EDU> writes:\n\n> \t### Emulate git-receive-pack by running hooks/post-receive\n> \tmy $hook = $ENV{GIT_DIR}.'hooks/post-receive';\n> \tif( -x $hook ) {\n> \t\topen(my $pipe, \"| $hook\") || die \"can't fork $!\";\n>\n> \t\tlocal $SIG{PIPE} = sub { die 'pipe broke' };\n>\n> \t\tprint $pipe \"$parenthash $commithash refs/heads/$state->{module}\\n\";\n>\n> \t\tclose $pipe || die \"bad pipe: $! $?\";\n> \t}\n>\n>> Unfortunately, it turns out that open() with a pipe essentially\n>> invokes system(); the solution is to fork a child process and\n>> then to turn the child into the process with which communication\n>> is desired via a call to exec().\n>>\n>> Because the rest of git-cvsserver.perl uses explicit system()\n>> calls, I have been wondering if I am being overly cautious.\n>\n> Am I being overly cautious?\n\nI do not think you are.  open($fh, \"| $hook\") can be confused with any\nIFS in $hook (there is no problem with 'hooks/post-receive', but\n$ENV{GIT_DIR} part can have pretty much anything other than NUL), so if\nanything, you are not being careful enough.  Other parts of cvsserver\nseem to be more careful by doing open($fh, '-|', @cmd), which does not\nhave this problem.\n\nThe execution environment of post-receive is probably wrong; I think\nreceive-pack runs the hooks with their $CWD = $GIT_DIR.\n"}]}