git/log: Read last commit ID into buffer

The `log.LastCommitForPath()` function determines the last commit for a
specific path. This is done by first spawning git-log(1) with the path
as revision, where the returned commit ID is then passed used to read
the actual commit object.

The way we read the last commit ID is not exactly great though. We
manually read into a bytes buffer and don't perform any error checking
whatsoever -- we don't even call `Wait()` on the git-log(1) command, and
silently go ahead with the potentially-empty bytes buffer. Furthermore,
we don't verify whether the returned object ID matches our expectations
but just blindly pass it to git-cat-file(1).

Refactor the code to spawn git-log(1) with its stdout connected to a
`strings.Builder` and implementing both proper error checking and output
validation.

1 files changed, 18 insertions, 7 deletions

diff --git a/internal/git/log/last_commit.go b/internal/git/log/last_commit.go
index 87d567c9f..d92251b32 100644
--- a/internal/git/log/last_commit.go
+++ b/internal/git/log/last_commit.go
@@ -2,13 +2,13 @@ package log
 
 import (
 	"context"
-	"io"
+	"fmt"
+	"strings"
 
 	"gitlab.com/gitlab-org/gitaly/v16/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 )
 
@@ -22,22 +22,33 @@ func LastCommitForPath(
 	path string,
 	options *gitalypb.GlobalOptions,
 ) (*gitalypb.GitCommit, error) {
+	var stdout strings.Builder
 	cmd, err := gitCmdFactory.New(ctx, repo, git.Command{
 		Name:        "log",
 		Flags:       []git.Option{git.Flag{Name: "--format=%H"}, git.Flag{Name: "--max-count=1"}},
 		Args:        []string{revision.String()},
 		PostSepArgs: []string{path},
-	}, append(git.ConvertGlobalOptions(options), git.WithSetupStdout())...)
+	}, append(git.ConvertGlobalOptions(options), git.WithStdout(&stdout))...)
 	if err != nil {
 		return nil, err
 	}
 
-	commitID, err := io.ReadAll(cmd)
-	if err != nil {
-		return nil, err
+	if err := cmd.Wait(); err != nil {
+		return nil, fmt.Errorf("logging last commit for path: %w", err)
+	}
+
+	if stdout.Len() == 0 {
+		return nil, catfile.NotFoundError{Revision: fmt.Sprintf("%s:%s", revision, path)}
+	}
+
+	commitID, trailer, ok := strings.Cut(stdout.String(), "\n")
+	if !ok {
+		return nil, fmt.Errorf("expected object ID terminated by newline")
+	} else if len(trailer) > 0 {
+		return nil, fmt.Errorf("object ID has trailing data")
 	}
 
-	return catfile.GetCommit(ctx, objectReader, git.Revision(text.ChompBytes(commitID)))
+	return catfile.GetCommit(ctx, objectReader, git.Revision(commitID))
 }
 
 // GitLogCommand returns a Command that executes git log with the given the arguments