DEV: patchbot: update: report the exact git error to the user

The "git commit failed" warning said nothing about the cause, leaving
the admin to guess between a missing identity, an ownership refusal, a
git binary absent from the restricted PATH the web server gives to its
CGIs, etc. run_git() now captures the command's stdout and stderr
through a pipe and the warning line carries git's own message (first
255 bytes, control chars flattened), so the admin directly sees the
cause; as the command runs through /bin/sh, an unfindable git yields
status 127 and the shell's message, reworded as "cannot execute git:
..." to directly point at the typical PATH issue. Capturing also
guarantees that git output can never corrupt the CGI response nor leak
to the client on servers which wire the CGI's stderr to the socket.

diff --git a/dev/patchbot/cgi/update.awk b/dev/patchbot/cgi/update.awk
index 260eaa2576cef18648589916fd519cbdfd27948a..edb24f86d750df980090243e890591e9f0cd3d3a 100755 (executable)
--- a/dev/patchbot/cgi/update.awk
+++ b/dev/patchbot/cgi/update.awk
@@ -600,6 +600,28 @@ function lock_acquire(   i, pid, stale, mt, priv, cmd, p2)
        return -1
 }
 
+# Runs "git -C <repo> <args>". Both its stdout and stderr are captured (into
+# GITMSG, up to 255 bytes, control chars turned into spaces), so that git
+# can neither corrupt the CGI response nor leak into a server which sends
+# the CGI's stderr to the client, and above all so that the exact git error
+# can be reported to the user. Since the command goes through /bin/sh, an
+# unfindable git yields status 127 and the shell's own message, reworded to
+# directly point at the typical PATH issue.
+function run_git(args,   cmd, line, out, st)
+{
+       cmd = "git -C " q(repo) " " args " 2>&1"
+       out = ""
+       while ((cmd | getline line) > 0)
+               out = out (out == "" ? "" : " ") line
+       st = close(cmd)
+       gsub(/[\x01-\x1f\x7f]/, " ", out)
+       if (st == 127)
+               out = "cannot execute git: " out
+       GITMSG = substr(out, 1, 255)
+       sub(/ +$/, "", GITMSG)
+       return st
+}
+
 # Releases the lock, but only after checking that it is still ours: after a
 # takeover interleaving gone wrong, the path may carry someone else's live
 # lock, which must not be dismantled; ours is then a private stale dir that
@@ -705,6 +727,7 @@ function handle_post(   body, i, fname, nb_confl, git_failed, attempt, renamed)
 
                nb_new = 0; nb_confl = 0
                git_failed = 0
+               GITMSG = ""
                for (i = 1; i <= nb_dirs; i++) {
                        if (apply_directive(i)) {
                                nb_confl++
@@ -749,15 +772,13 @@ function handle_post(   body, i, fname, nb_confl, git_failed, attempt, renamed)
                        # re-pushed identical states) is not a failure: the commit is
                        # simply skipped when nothing is staged. Never checkout or
                        # reset here, it would eat an admin's uncommitted hand-edit.
-                       # Git's output is redirected to stderr so that it cannot
-                       # corrupt the CGI response.
-                       if (system("git -C " q(repo) " add -- " q(branch) " 1>&2") != 0)
+                       if (run_git("add -- " q(branch)) != 0)
                                git_failed = 1
-                       else if (system("git -C " q(repo) " diff --cached --quiet 1>&2") != 0 && \
-                                system("git -C " q(repo) " commit -q -m " q("update " branch) " 1>&2") != 0)
+                       else if (run_git("diff --cached --quiet") != 0 && \
+                                run_git("commit -q -m " q("update " branch)) != 0)
                                git_failed = 1
                        if (git_failed)
-                               print "update.awk: git commit failed in " repo " (will retry on next write)" > "/dev/stderr"
+                               print "update.awk: git commit failed in " repo ": " GITMSG > "/dev/stderr"
                }
                # else: everything conflicted, nothing changed, nothing to write
 
@@ -775,7 +796,9 @@ function handle_post(   body, i, fname, nb_confl, git_failed, attempt, renamed)
        for (i = 1; i <= nb_confl; i++)
                print "conflict " CONFL[i]
        if (git_failed)
-               print "warning: git commit failed, history not recorded (check the committer identity and permissions in the storage repository)"
+               print "warning: git commit failed, history not recorded (" \
+                     (GITMSG != "" ? GITMSG : \
+                      "check the committer identity and permissions in the storage repository") ")"
        for (i = 1; i <= nb_lines; i++) {
                if (!L_touched[i])
                        continue