From 7f9705a29c29b77f9049f2d1222a2d08530fc225 Mon Sep 17 00:00:00 2001 From: Runxi Yu Date: Wed, 19 Feb 2025 17:08:14 +0800 Subject: [PATCH] hooks: Use ssh stderr directly instead of going through hook --- git_hooks_handle.go | 231 +++++++++++++++++++++++------------------------------ ssh_handle_receive_pack.go | 4 ++-- diff --git a/git_hooks_handle.go b/git_hooks_handle.go index 954cbfdbdd30caaf37f5d9ef54733fc5dec01145..b0383a2bb0ab7101f0cb268462fe7c7848860fff 100644 --- a/git_hooks_handle.go +++ b/git_hooks_handle.go @@ -63,165 +63,132 @@ fmt.Fprintln(conn, "Invalid handler cookie") return } - var argc64 uint64 - err = binary.Read(conn, binary.NativeEndian, &argc64) - if err != nil { - if _, err := conn.Write([]byte{1}); err != nil { - return + ssh_stderr := pack_to_hook.session.Stderr() + + hook_return_value := func() byte { + var argc64 uint64 + err = binary.Read(conn, binary.NativeEndian, &argc64) + if err != nil { + fmt.Fprintln(ssh_stderr, "Failed to read argc:", err.Error()) + return 1 } - fmt.Fprintln(conn, "Failed to read argc:", err.Error()) - return - } - var args []string - for i := uint64(0); i < argc64; i++ { - var arg bytes.Buffer - for { - b := make([]byte, 1) - n, err := conn.Read(b) - if err != nil || n != 1 { - if _, err := conn.Write([]byte{1}); err != nil { - return + var args []string + for i := uint64(0); i < argc64; i++ { + var arg bytes.Buffer + for { + b := make([]byte, 1) + n, err := conn.Read(b) + if err != nil || n != 1 { + fmt.Fprintln(ssh_stderr, "Failed to read arg:", err.Error()) + return 1 + } + if b[0] == 0 { + break } - fmt.Fprintln(conn, "Failed to read arg:", err.Error()) - return + arg.WriteByte(b[0]) } - if b[0] == 0 { - break - } - arg.WriteByte(b[0]) + args = append(args, arg.String()) } - args = append(args, arg.String()) - } - var stdin bytes.Buffer - _, err = io.Copy(&stdin, conn) - if err != nil { - fmt.Fprintln(conn, "Failed to read to the stdin buffer:", err.Error()) - } - - switch filepath.Base(args[0]) { - case "pre-receive": - if pack_to_hook.direct_access { - if _, err := conn.Write([]byte{0}); err != nil { - return - } - } else { - all_ok := true - messages := make([]string, 0) + var stdin bytes.Buffer + _, err = io.Copy(&stdin, conn) + if err != nil { + fmt.Fprintln(conn, "Failed to read to the stdin buffer:", err.Error()) + } - for { - line, err := stdin.ReadString('\n') - if errors.Is(err, io.EOF) { - break - } - line = line[:len(line)-1] + switch filepath.Base(args[0]) { + case "pre-receive": + if pack_to_hook.direct_access { + return 0 + } else { + all_ok := true + for { + line, err := stdin.ReadString('\n') + if errors.Is(err, io.EOF) { + break + } + line = line[:len(line)-1] - old_oid, rest, found := strings.Cut(line, " ") - if !found { - if _, err := conn.Write([]byte{1}); err != nil { - return + old_oid, rest, found := strings.Cut(line, " ") + if !found { + fmt.Fprintln(ssh_stderr, "Invalid pre-receive line:", line) + return 1 } - fmt.Fprintln(conn, "Invalid pre-receive line:", line) - break - } - new_oid, ref_name, found := strings.Cut(rest, " ") - if !found { - if _, err := conn.Write([]byte{1}); err != nil { - return + new_oid, ref_name, found := strings.Cut(rest, " ") + if !found { + fmt.Fprintln(ssh_stderr, "Invalid pre-receive line:", line) + return 1 } - fmt.Fprintln(conn, "Invalid pre-receive line:", line) - break - } - if strings.HasPrefix(ref_name, "refs/heads/contrib/") { - if all_zero_num_string(old_oid) { // New branch - messages = append(messages, "Acceptable push to new contrib branch: "+ref_name) - // TODO: Create a merge request. If that fails, - // we should just reject this entire push - // immediately. - } else { // Existing contrib branch - // TODO: Check if the current user is authorized - // to push to this contrib branch. - repo, err := git.PlainOpen(pack_to_hook.repo_path) - if err != nil { - if _, err := conn.Write([]byte{1}); err != nil { - return + if strings.HasPrefix(ref_name, "refs/heads/contrib/") { + if all_zero_num_string(old_oid) { // New branch + fmt.Fprintln(ssh_stderr, "Acceptable push to new contrib branch: "+ref_name) + // TODO: Create a merge request. If that fails, + // we should just reject this entire push + // immediately. + } else { // Existing contrib branch + // TODO: Check if the current user is authorized + // to push to this contrib branch. + repo, err := git.PlainOpen(pack_to_hook.repo_path) + if err != nil { + fmt.Fprintln(ssh_stderr, "Daemon failed to open repo:", err.Error()) + return 1 } - fmt.Fprintln(conn, "Daemon failed to open repo:", err.Error()) - return - } - old_hash := plumbing.NewHash(old_oid) - fmt.Println(old_hash) + old_hash := plumbing.NewHash(old_oid) - old_commit, err := repo.CommitObject(old_hash) - if err != nil { - if _, err := conn.Write([]byte{1}); err != nil { - return + old_commit, err := repo.CommitObject(old_hash) + if err != nil { + fmt.Fprintln(ssh_stderr, "Daemon failed to get old commit:", err.Error()) + return 1 } - fmt.Fprintln(conn, "Daemon failed to get old commit:", err.Error()) - return - } + + // Potential BUG: I'm not sure if new_commit is guaranteed to be + // detectable as they haven't been merged into the main repo's + // objects yet. But it seems to work, and I don't think there's + // any reason for this to only work intermitently. + new_hash := plumbing.NewHash(new_oid) + new_commit, err := repo.CommitObject(new_hash) + if err != nil { + fmt.Fprintln(ssh_stderr, "Daemon failed to get new commit:", err.Error()) + return 1 + } - // Potential BUG: I'm not sure if new_commit is guaranteed to be - // detectable as they haven't been merged into the main repo's - // objects yet. But it seems to work, and I don't think there's - // any reason for this to only work intermitently. - new_hash := plumbing.NewHash(new_oid) - new_commit, err := repo.CommitObject(new_hash) - if err != nil { - if _, err := conn.Write([]byte{1}); err != nil { - return + is_ancestor, err := old_commit.IsAncestor(new_commit) + if err != nil { + fmt.Fprintln(ssh_stderr, "Daemon failed to check if old commit is ancestor:", err.Error()) + return 1 } - fmt.Fprintln(conn, "Daemon failed to get new commit:", err.Error()) - return - } - is_ancestor, err := old_commit.IsAncestor(new_commit) - if err != nil { - if _, err := conn.Write([]byte{1}); err != nil { - return + if !is_ancestor { + // TODO: Create MR snapshot ref instead + all_ok = false + fmt.Fprintln(ssh_stderr, "Rejecting force push to contrib branch: "+ref_name) + continue } - fmt.Fprintln(conn, "Daemon failed to check if old commit is ancestor:", err.Error()) - return - } - if !is_ancestor { - // TODO: Create MR snapshot ref instead - all_ok = false - messages = append(messages, "Rejecting force push to contrib branch: "+ref_name) - continue + fmt.Fprintln(ssh_stderr, "Acceptable push to existing contrib branch: "+ref_name) } - - messages = append(messages, "Acceptable push to existing contrib branch: "+ref_name) + } else { // Non-contrib branch + all_ok = false + fmt.Fprintln(ssh_stderr, "Rejecting push to non-contrib branch: "+ref_name) } - } else { // Non-contrib branch - all_ok = false - messages = append(messages, "Rejecting push to non-contrib branch: "+ref_name) } - } - if all_ok { - if _, err := conn.Write([]byte{0}); err != nil { - return - } - } else { - if _, err := conn.Write([]byte{1}); err != nil { - return + if all_ok { + return 0 + } else { + return 1 } } - - for _, message := range messages { - fmt.Fprintln(conn, message) - } - } - default: - if _, err := conn.Write([]byte{1}); err != nil { - return + default: + fmt.Fprintln(ssh_stderr, "Invalid hook:", args[0]) + return 1 } - fmt.Fprintln(conn, "Invalid hook:", args[0]) - } + }() + + conn.Write([]byte{hook_return_value}) } func serve_git_hooks(listener net.Listener) error { diff --git a/ssh_handle_receive_pack.go b/ssh_handle_receive_pack.go index 2eb647d9dbf25ed9ba2395073b99d12c3e1e30d7..b76887f151db32d13e834d18c2c3b8e0200d906e 100644 --- a/ssh_handle_receive_pack.go +++ b/ssh_handle_receive_pack.go @@ -13,7 +13,7 @@ var err_unauthorized_push = errors.New("you are not authorized to push to this repository") type pack_to_hook_t struct { - session *glider_ssh.Session + session glider_ssh.Session pubkey string direct_access bool repo_path string @@ -38,7 +38,7 @@ fmt.Fprintln(session.Stderr(), "Error while generating cookie:", err) } pack_to_hook_by_cookie.Store(cookie, pack_to_hook_t{ - session: &session, + session: session, pubkey: pubkey, direct_access: access, repo_path: repo_path, -- 2.48.1