Merge branch 'sh-fix-last-commit-pathspec' into 'master'

Ensure literal pathspecs are used for LastCommitForPath

Closes #2857

See merge request gitlab-org/gitaly!2271

3 files changed, 51 insertions, 1 deletions

diff --git a/changelogs/unreleased/sh-fix-last-commit-pathspec.yml b/changelogs/unreleased/sh-fix-last-commit-pathspec.yml
new file mode 100644
index 000000000..1fe986254
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-last-commit-pathspec.yml
@@ -0,0 +1,5 @@
+---
+title: Enforce literal paths for LastCommitForPath
+merge_request: 2271
+author:
+type: fixed
diff --git a/internal/git/log/last_commit.go b/internal/git/log/last_commit.go
index 76b3c9915..e99914ae6 100644
--- a/internal/git/log/last_commit.go
+++ b/internal/git/log/last_commit.go
@@ -13,7 +13,7 @@ import (
 
 // LastCommitForPath returns the last commit which modified path.
 func LastCommitForPath(ctx context.Context, batch *catfile.Batch, repo *gitalypb.Repository, revision string, path string) (*gitalypb.GitCommit, error) {
-	cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{
+	cmd, err := git.SafeCmd(ctx, repo, []git.Option{git.Flag{"--literal-pathspecs"}}, git.SubCmd{
 		Name:        "log",
 		Flags:       []git.Option{git.Flag{"--format=%H"}, git.Flag{"--max-count=1"}},
 		Args:        []string{revision},
diff --git a/internal/service/commit/last_commit_for_path_test.go b/internal/service/commit/last_commit_for_path_test.go
index 2b6f2ec75..152575848 100644
--- a/internal/service/commit/last_commit_for_path_test.go
+++ b/internal/service/commit/last_commit_for_path_test.go
@@ -2,6 +2,9 @@ package commit
 
 import (
 	"context"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/golang/protobuf/ptypes/timestamp"
@@ -142,3 +145,45 @@ func TestFailedLastCommitForPathRequest(t *testing.T) {
 		})
 	}
 }
+
+func TestSuccessfulLastCommitWithGlobCharacters(t *testing.T) {
+	server, serverSocketPath := startTestServices(t)
+	defer server.Stop()
+
+	client, conn := newCommitServiceClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepoWithWorktree(t)
+	defer cleanupFn()
+
+	branch := "last-commit-test"
+	filename := filepath.Join(testRepoPath, ":wq")
+	f, err := os.Create(filename)
+	require.NoError(t, err)
+	f.WriteString("hello world\n")
+	f.Close()
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "checkout", "-b", branch, "master")
+	testhelper.MustRunCommand(t, nil, "git", "--literal-pathspecs", "-C", testRepoPath, "add", ":wq")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "commit", "-a", "-m", "add file")
+
+	branchSha := getBranchSha(t, testRepoPath, branch)
+
+	request := &gitalypb.LastCommitForPathRequest{
+		Repository: testRepo,
+		Revision:   []byte(branchSha),
+		Path:       []byte(":wq"),
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	response, err := client.LastCommitForPath(ctx, request)
+	require.NoError(t, err)
+	require.NotNil(t, response.GetCommit())
+	require.Equal(t, branchSha, response.GetCommit().Id)
+}
+
+func getBranchSha(t *testing.T, repoPath string, branchName string) string {
+	branchSha := string(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "rev-parse", branchName))
+	return strings.TrimSpace(branchSha)
+}