commit: Fix verification of arguments in `CheckObjectsExist()`

The `CheckObjectsExist()` RPC doesn't verify it got a repository in its
initial request. On the other hand, it only verifies that revisions are
valid for the first request.

Fix both issues.

Changelog: fixed

2 files changed, 17 insertions, 21 deletions

diff --git a/internal/gitaly/service/commit/check_objects_exist.go b/internal/gitaly/service/commit/check_objects_exist.go
index 83512b340..a4224f206 100644
--- a/internal/gitaly/service/commit/check_objects_exist.go
+++ b/internal/gitaly/service/commit/check_objects_exist.go
@@ -22,8 +22,8 @@ func (s *server) CheckObjectsExist(
 		return err
 	}
 
-	if err := validateCheckObjectsExistRequest(request); err != nil {
-		return err
+	if request.GetRepository() == nil {
+		return helper.ErrInvalidArgumentf("empty Repository")
 	}
 
 	objectInfoReader, cancel, err := s.catfileCache.ObjectInfoReader(
@@ -45,6 +45,12 @@ func (s *server) CheckObjectsExist(
 			return err
 		}
 
+		for _, revision := range request.GetRevisions() {
+			if err := git.ValidateRevision(revision); err != nil {
+				return helper.ErrInvalidArgumentf("invalid revision %q: %w", revision, err)
+			}
+		}
+
 		if err = checkObjectsExist(ctx, request, objectInfoReader, chunker); err != nil {
 			return err
 		}
@@ -99,13 +105,3 @@ func checkObjectsExist(
 
 	return nil
 }
-
-func validateCheckObjectsExistRequest(in *gitalypb.CheckObjectsExistRequest) error {
-	for _, revision := range in.GetRevisions() {
-		if err := git.ValidateRevision(revision); err != nil {
-			return helper.ErrInvalidArgument(err)
-		}
-	}
-
-	return nil
-}
diff --git a/internal/gitaly/service/commit/check_objects_exist_test.go b/internal/gitaly/service/commit/check_objects_exist_test.go
index 1be585c02..99b1328a0 100644
--- a/internal/gitaly/service/commit/check_objects_exist_test.go
+++ b/internal/gitaly/service/commit/check_objects_exist_test.go
@@ -51,8 +51,12 @@ func TestCheckObjectsExist(t *testing.T) {
 					Revisions: [][]byte{},
 				},
 			},
-			// This is a bug: we don't verify there's a repository.
-			expectedErr:     helper.ErrInvalidArgumentf(`GetStorageByName: no such storage: ""`),
+			expectedErr: func() error {
+				if testhelper.IsPraefectEnabled() {
+					return helper.ErrInvalidArgumentf("repo scoped: empty Repository")
+				}
+				return helper.ErrInvalidArgumentf("empty Repository")
+			}(),
 			expectedResults: map[string]bool{},
 		},
 		{
@@ -141,7 +145,7 @@ func TestCheckObjectsExist(t *testing.T) {
 					},
 				},
 			},
-			expectedErr:     helper.ErrInvalidArgumentf("revision can't start with '-'"),
+			// This is a bug: revisions of the first message are not checked.
 			expectedResults: map[string]bool{},
 		},
 		{
@@ -173,12 +177,8 @@ func TestCheckObjectsExist(t *testing.T) {
 					},
 				},
 			},
-			// This is a bug: we only check that revisions of the first message are
-			// valid, but don't care about any of the latter ones.
-			expectedErr: nil,
-			expectedResults: map[string]bool{
-				"-not-a-rev": false,
-			},
+			expectedErr:     helper.ErrInvalidArgumentf("invalid revision %q: revision can't start with '-'", "-not-a-rev"),
+			expectedResults: map[string]bool{},
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {