[PATCH] gitolite-shell ignores git command exit codes, causing POST_GIT triggers to run on failed operations

[Rashmi]

Problem Description
When git operations fail (e.g., due to VREF checks), gitolite-shell continues execution and runs POST_GIT triggers as if the operation succeeded. This leads to incorrect behavior where post-processing occurs even when the underlying git operation failed. We have custom VREF hooks that get executed as part of git pushes and we observe that the vref check does return the exit code but gitolite-shell doesnt handle the non zero exit code.

Root Cause

In src/gitolite-shell lines 140-146, the code calls _system() to execute git commands but doesn't check the return value. 

From eb423d9ba5c6e6736d0fb019631e67547d71de74 Mon Sep 17 00:00:00 2001
From: rashmi-prithyani-1 <rashmipr...@gmail.com>
Date: Thu, 24 Jul 2025 13:07:00 -0700
Subject: [PATCH] handle exit code returned by _system call and dont run post
 git triggers if git operation fails

---
 src/gitolite-shell | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/gitolite-shell b/src/gitolite-shell
index 072e0ff..0a52bee 100755
--- a/src/gitolite-shell
+++ b/src/gitolite-shell
@@ -137,11 +137,17 @@ sub main {
     }
 
     trigger( 'PRE_GIT', $repo, $user, $aa, 'any', $verb );
+    
+    my $exit_code;
     if ( $ENV{REQUEST_URI} ) {
-        _system( "git", "http-backend" );
+        $exit_code = _system( "git", "http-backend" );
     } else {
         my $repodir = "'$rc{GL_REPO_BASE}/$repo.git'";
-        _system( "git", "shell", "-c", "$verb $repodir" );
+        $exit_code = _system( "git", "shell", "-c", "$verb $repodir" );
+    }
+    
+    if ( $exit_code != 0 ) {
+        exit $exit_code;
     }
     trigger( 'POST_GIT', $repo, $user, $aa, 'any', $verb );
 }
--
2.48.1

[Sitaram Chamarty]

On Thu, Jul 24, 2025 at 01:20:06PM -0700, Rashmi wrote:
> *Problem Description*

> When git operations fail (e.g., due to VREF checks), gitolite-shell
> continues execution and runs POST_GIT triggers as if the operation

Hmm... interesting.

https://gitolite.com/gitolite/triggers.html#trigger-specific-arguments-and-other-details
clearly says (under the POST_GIT section):

Note that the POST_GIT trigger has no way of knowing if the
push succeeded, because 'git-shell' (or maybe
'git-receive-pack', I don't know) exits cleanly even if the
update hook died.

It is possible that git-shell has changed its behaviour in the
12+ years since that was written, because I feel sure I would
not have *explicitly* said that if I had not tested it.

I also wonder if anyone is actually depending on the old
behaviour.

I'll get back to you and push at least a modified version of
this when I get a chance.

sitaram

> succeeded. This leads to incorrect behavior where post-processing occurs
> even when the underlying git operation failed. We have custom VREF hooks
> that get executed as part of git pushes and we observe that the vref check
> does return the exit code but gitolite-shell doesnt handle the non zero
> exit code.
>

> *Root Cause*

> --
> You received this message because you are subscribed to the Google Groups "gitolite" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to gitolite+u...@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/gitolite/bf220a7a-6ea9-4bee-979b-62b922c7a18cn%40googlegroups.com.

[Sitaram Chamarty]

Sorry for top posting from my phone....

I just noticed that the section just before the one I quoted (i.e the section for ACCESS_2) explicitly says that the POST_GIT triggers *will* run.

It's one thing to wonder if anybody is depending on *accidental* behavior, but it's quite another to make a change that goes against *documented* behavior.

I may end up making that change eventually but it may take some time.

Sitaram

[Rashmi]

Thanks for the detailed context — that all makes sense. To preserve backward compatibility while addressing the correctness issue, I’d be happy to update the patch to support a config flag like:

POST_GIT_ON_FAILURE = 1

This would default to current behavior (always run POST_GIT), but when set, Gitolite would skip POST_GIT if the Git command exits non-zero (e.g., due to VREF rejections).

Would you be open to this approach? If yes, I can submit the updated patch shortly.

Thanks,
Rashmi

 src/gitolite-shell | 10 ++++++++--

[Sitaram Chamarty]

On Mon, Jul 28, 2025 at 01:35:04PM -0700, Rashmi wrote:

> On Saturday, July 26, 2025 at 5:05:21 AM UTC-7 Sitaram Chamarty wrote:

> > On 26 July 2025 17:21:53 GMT+05:30, Sitaram Chamarty <sita...@gmail.com>
> > wrote:

> >>> - _system( "git", "shell", "-c", "$verb $repodir" );
> >>> + $exit_code = _system( "git", "shell", "-c", "$verb $repodir" );

I hate to ask but did you test this?

Yet I still get nothing (the new exit_code variable is always
empty, regardless of success or failure of the update hook).

In other words, this is *still* the behaviour I am seeing:

> >> Note that the POST_GIT trigger has no way of knowing if the
> >> push succeeded, because 'git-shell' (or maybe
> >> 'git-receive-pack', I don't know) exits cleanly even if the
> >> update hook died.

(I'm on git 2.43.0 -- not the absolute latest but not that old
either so I assume that is not the cause.)

[Rashmi]

Yes—this came out of a production incident where a custom VREF check was failing (exiting with code 1), but our system—relying on the gitolite-shell binary—still interpreted the entire operation as a success (exit code 0). This led to users seeing error messages during pushes while our service-level logs showed the request as successful. Since the failure didn't propagate back up cleanly, we missed alerting on it and only became aware via user reports.

Here’s the specific call stack we observed:

user     31501  31455  7 21:19 ?        00:00:00              \_ /usr/bin/perl /opt/internal/gitolite/gitolite-shell exampleuser                 (exit code: 0)
user     31520  31501  0 21:19 ?        00:00:00                  \_ git shell -c git-receive-pack '/repos/sample-project.git'
user     31521  31520  0 21:19 ?        00:00:00                      \_ git receive-pack /repos/sample-project.git
user     31573  31521  0 21:19 ?        00:00:00                          \_ /usr/bin/perl hooks/update refs/heads/exampleuser/feature-branch abc123
user     31592  31573  0 21:19 ?        00:00:00                              \_ /bin/bash /opt/internal/vref/time refs/heads/exampleuser/feature-branch
user     31594  31592  0 21:19 ?        00:00:00                                  \_ /usr/bin/custom-vref-check refs/heads/exampleuser/feature-branch   (exit code: 1)

Despite the custom-vref-check clearly failing, gitolite-shell ultimately returned 0. I’m on git 2.48.1, and when I tested this, I reproduced the same behavior. Can you share a bit more about your test setup ? Was it also a failing custom VREF check failing during a push ?

Thanks,

Rashmi 

[Sitaram Chamarty]

Rashmi,

As of git version 2.43.0 (which is what I currently have), git's
behaviour has not changed, and there is no way for gitolite to
*know* that the update hook failed, because "git-shell" always
exits with a zero, regardless of what happened with the update
hook. If a later git has changed that, I am not aware of it,
nor am I in a position to test it till at least mid-August.

More to the point, if a newer "git shell" did change that
behaviour, then you would not even need a patch to achieve what
you want. Gitolite's `_system()` function does not return if
the program that was executed returns a non-zero, so control
would not even come back to the location your patch references.

Hope this helps.

sitaram