From 3a178ebb7ea7270181abbdef95871f98f03c61d1 Mon Sep 17 00:00:00 2001
From: Runxi Yu <me@runxiyu.org>
Date: Mon, 17 Feb 2025 23:46:32 +0800
Subject: [PATCH] hooks, etc.: Restructure concurrency and data flow

Previously we accepted handler connections at hooks_handle and used a
mess of channels and concurrent maps to let receive_pack handle the
session. This doesn't work well because there are conditions where a
push occurs but the hook is not called, e.g. when the destination branch
is up to date. There is no reliable way of checking whether the
subprocess is going to call the hook or not; it's technically possible
to parse stderr but that interface is not guaranteed to be stable and
IIRC has changed in the past). So receive_pack would be waiting on the
channel to receive a hooks connection to handle but it'll never receive
one, causing a deadlock. This entire thing was overengineered and was
very prone to error.

Here we let receive_pack put the cookie into the map, then start and
wait for the subprocess to finish. When the hook actually runs and
connects to its UNIX domain socket, the handler would check its cookie
within the map. If the hook doesn't run, then nothing happens. The
git-receive-pack subprocess blocks the execution of the SSH handler,
and when git-receive-pack exists, the SSH handler (using a defer)
deletes the cookie from the map.

There may be caveats in signal handling or other cases that cause the
cookie to be deleted from the map prematurely.

---
 git_hooks_handle.go        | 27 ++++++++++++++++-----------
 ssh_handle_receive_pack.go | 33 +++++++++++++--------------------

diff --git a/git_hooks_handle.go b/git_hooks_handle.go
index 371993389e8239def40f9107e8e3479a96cc0103..91f1894707d9e0d92aed047ec3db97266b870995 100644
--- a/git_hooks_handle.go
+++ b/git_hooks_handle.go
@@ -7,12 +7,13 @@ 	"errors"
 	"fmt"
 	"net"
 	"os"
+	"path/filepath"
 	"syscall"
 )
 
 var (
-	err_get_fd       = errors.New("Unable to get file descriptor")
-	err_get_ucred    = errors.New("Failed getsockopt")
+	err_get_fd    = errors.New("Unable to get file descriptor")
+	err_get_ucred = errors.New("Failed getsockopt")
 )
 
 func hooks_handle_connection(conn net.Conn) {
@@ -38,10 +39,10 @@ 		fmt.Fprintln(conn, "Failed to read cookie:", err.Error())
 		return
 	}
 
-	deployer_chan, ok := hooks_cookie_deployer.Load(string(cookie))
+	pack_to_hook, ok := pack_to_hook_by_cookie.Load(string(cookie))
 	if !ok {
 		conn.Write([]byte{1})
-		fmt.Fprintln(conn, "Invalid cookie")
+		fmt.Fprintln(conn, "Invalid handler cookie")
 		return
 	}
 
@@ -71,14 +72,18 @@ 		}
 		args = append(args, arg.String())
 	}
 
-	callback := make(chan struct{})
-
-	deployer_chan <- hooks_cookie_deployer_return{
-		args:     args,
-		callback: callback,
-		conn:     conn,
+	switch filepath.Base(args[0]) {
+	case "pre-receive":
+		if pack_to_hook.direct_access {
+			conn.Write([]byte{0})
+		} else {
+			conn.Write([]byte{1})
+			fmt.Fprintln(conn, "Non-maintainer push access not implemented yet")
+		}
+	default:
+		conn.Write([]byte{1})
+		fmt.Fprintln(conn, "Invalid hook:", args[0])
 	}
-	<-callback
 }
 
 func serve_git_hooks(listener net.Listener) error {
diff --git a/ssh_handle_receive_pack.go b/ssh_handle_receive_pack.go
index 85655b1ad8986eefa8562d74a8438aca01f675bf..c6800119d81cf96c88b7b73ee48fab5648d6e801 100644
--- a/ssh_handle_receive_pack.go
+++ b/ssh_handle_receive_pack.go
@@ -4,7 +4,6 @@ import (
 	"crypto/rand"
 	"errors"
 	"fmt"
-	"net"
 	"os"
 	"os/exec"
 
@@ -14,13 +13,14 @@ )
 
 var err_unauthorized_push = errors.New("You are not authorized to push to this repository")
 
-type hooks_cookie_deployer_return struct {
-	args     []string
-	callback chan struct{}
-	conn     net.Conn
+type pack_to_hook_t struct {
+	session       *glider_ssh.Session
+	pubkey        string
+	direct_access bool
+	repo_path     string
 }
 
-var hooks_cookie_deployer = cmap.ComparableMap[string, chan hooks_cookie_deployer_return]{}
+var pack_to_hook_by_cookie = cmap.Map[string, pack_to_hook_t]{}
 
 func ssh_handle_receive_pack(session glider_ssh.Session, pubkey string, repo_identifier string) (err error) {
 	repo_path, access, err := get_repo_path_perms_from_ssh_path_pubkey(session.Context(), repo_identifier, pubkey)
@@ -33,9 +33,13 @@ 	if err != nil {
 		fmt.Fprintln(session.Stderr(), "Error while generating cookie:", err)
 	}
 
-	deployer_channel := make(chan hooks_cookie_deployer_return)
-	hooks_cookie_deployer.Store(cookie, deployer_channel)
-	defer hooks_cookie_deployer.Delete(cookie)
+	pack_to_hook_by_cookie.Store(cookie, pack_to_hook_t{
+		session:       &session,
+		pubkey:        pubkey,
+		direct_access: access,
+		repo_path:     repo_path,
+	})
+	defer pack_to_hook_by_cookie.Delete(cookie)
 
 	proc := exec.CommandContext(session.Context(), "git-receive-pack", repo_path)
 	proc.Env = append(os.Environ(),
@@ -51,17 +55,6 @@ 	if err != nil {
 		fmt.Fprintln(session.Stderr(), "Error while starting process:", err)
 		return err
 	}
-
-	deployer := <-deployer_channel
-
-	if access {
-		deployer.conn.Write([]byte{0})
-	} else {
-		deployer.conn.Write([]byte{1})
-		fmt.Fprintln(deployer.conn, "Hi! We don't support pushing from non-authorized users yet. This will be implemented soon.")
-	}
-
-	deployer.callback <- struct{}{}
 
 	err = proc.Wait()
 	if exitError, ok := err.(*exec.ExitError); ok {

-- 
2.48.1