git's start_command (run_command.c) executes a command (e.g. hook) but does not verify that it has properly set up the environment. It seems that in the unlikely case where putenv (run_command.c:117) fails, the command may have undesirable effects e.g. GIT_INDEX_FILE should have been set (interactive pre-commit hooks) but the default index will be used instead. It would be safer not to run the command but just exit in that case.
On Thu, Dec 03, 2009 at 08:19:05PM +0100, Marinescu Paul dan wrote:
git's start_command (run_command.c) executes a command (e.g. hook) but
does not verify that it has properly set up the environment. It seems
that in the unlikely case where putenv (run_command.c:117) fails, the
command may have undesirable effects e.g. GIT_INDEX_FILE should have
been set (interactive pre-commit hooks) but the default index will be
used instead. It would be safer not to run the command but just exit
in that case.--
Hmm. It is simple enough to patch the one use of putenv, but there are
34 other calls to setenv, which has the same problem. I am tempted to
ignore it, as it is extremely unlikely for this ever to happen, and
adding error checking everywhere reduces the code readability.
But the consequences, as you mention, could include data loss, which
argues for being on the safe side. In that case, we would probably want
an "xsetenv" to die() if we fail to avoid cluttering the code
everywhere.
I dunno. If we're going to do it, it is probably maint-worthy. Junio?
-Peff