operations: Validate tag name more thoroughly in UserCreateTag

While we validate that the tag name doesn't contain any spaces in
UserCreateTag, that's as far as our validation goes. This is missing a
lot of different cases though where the tag name is invalidly formatted
like for example when it contains newlines. As a result, we don't fail
early when the tag name is invalid but will fail when git-update-ref(1)
notices the invalid format.

While not a serious error given that we do eventually detect this, the
error message we return in that case is still one of the old errors that
we have added to stay compatible with the exact errors returned by the
old Ruby implementation. Furthermore, we retun an `Unknown` error code
in that case instead of an `InvalidArgument`.

Fix this by using `git.ValidateRevision()` instead, which catches a lot
more invalid reference names than just a contained space.

Changelog: fixed

2 files changed, 8 insertions, 14 deletions

diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go
index d73f6842c..de679caa9 100644
--- a/internal/gitaly/service/operations/tags.go
+++ b/internal/gitaly/service/operations/tags.go
@@ -58,8 +58,8 @@ func validateUserCreateTag(req *gitalypb.UserCreateTagRequest) error {
 		return fmt.Errorf("empty tag name")
 	}
 
-	if bytes.Contains(req.TagName, []byte(" ")) {
-		return fmt.Errorf("tag name contains space")
+	if err := git.ValidateRevision(req.TagName); err != nil {
+		return fmt.Errorf("invalid tag name: %w", err)
 	}
 
 	if req.User == nil {
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index 30f4b4c04..f92def19d 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -1191,8 +1191,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
 			tagName:        "a tag",
 			targetRevision: "master",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            helper.ErrInvalidArgumentf("validating request: tag name contains space"),
+			err:            helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "space in annotated tag name",
@@ -1200,16 +1199,14 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
 			targetRevision: "master",
 			message:        "a message",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            helper.ErrInvalidArgumentf("validating request: tag name contains space"),
+			err:            helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "newline in lightweight tag name",
 			tagName:        "a\ntag",
 			targetRevision: "master",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Errorf(codes.Unknown, "Gitlab::Git::CommitError: Could not update refs/tags/%s. Please refresh and try again.", "a\ntag"),
+			err:            helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "newline in annotated tag name",
@@ -1217,16 +1214,14 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
 			targetRevision: "master",
 			message:        "a message",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            status.Error(codes.Unknown, "Rugged::InvalidError: failed to parse signature - expected prefix doesn't match actual"),
+			err:            helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "injection in lightweight tag name",
 			tagName:        injectedTag,
 			targetRevision: "master",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            helper.ErrInvalidArgumentf("validating request: tag name contains space"),
+			err:            helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 		{
 			desc:           "injection in annotated tag name",
@@ -1234,8 +1229,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
 			targetRevision: "master",
 			message:        "a message",
 			user:           gittest.TestUser,
-			response:       nil,
-			err:            helper.ErrInvalidArgumentf("validating request: tag name contains space"),
+			err:            helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"),
 		},
 	}