Merge branch 'qmnguyen0711/backport-url-key-config-validation' into '15-5-stable'

Backport URL Key config validation

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5222

Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>

4 files changed, 9 insertions, 7 deletions

diff --git a/internal/git/command_options.go b/internal/git/command_options.go
index d72c8f120..c74b6c1e3 100644
--- a/internal/git/command_options.go
+++ b/internal/git/command_options.go
@@ -33,7 +33,7 @@ var (
 	// keys which git would parse just fine, but we only have a limited
 	// number of config entries anyway. Most importantly, we cannot allow
 	// `=` as part of the key as that would break parsing of `git -c`.
-	configKeyGlobalRegex = regexp.MustCompile(`^[[:alnum:]]+(\.[-/_a-zA-Z0-9]+)+$`)
+	configKeyGlobalRegex = regexp.MustCompile(`^[[:alnum:]]+(\.[-/_:@a-zA-Z0-9]+)+$`)
 
 	flagRegex = regexp.MustCompile(`^(-|--)[[:alnum:]]`)
 )
diff --git a/internal/git/command_options_test.go b/internal/git/command_options_test.go
index 30410909d..5bdf7fcaa 100644
--- a/internal/git/command_options_test.go
+++ b/internal/git/command_options_test.go
@@ -175,6 +175,12 @@ func TestGlobalOption(t *testing.T) {
 			expected: []string{"-c", "foo.bar="},
 		},
 		{
+			desc:     "config pair with URL key",
+			option:   ConfigPair{Key: "http.https://user@example.com/repo.git.user", Value: "kitty"},
+			valid:    true,
+			expected: []string{"-c", "http.https://user@example.com/repo.git.user=kitty"},
+		},
+		{
 			desc:   "config pair with invalid section format",
 			option: ConfigPair{Key: "foo", Value: "value"},
 			valid:  false,
@@ -186,7 +192,7 @@ func TestGlobalOption(t *testing.T) {
 		},
 		{
 			desc:   "config pair with disallowed character in key",
-			option: ConfigPair{Key: "http.https://weak.example.com.sslVerify", Value: "false"},
+			option: ConfigPair{Key: "foo.b=r", Value: "value"},
 			valid:  false,
 		},
 	} {
diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go
index c45441b21..245482cf5 100644
--- a/internal/gitaly/config/config.go
+++ b/internal/gitaly/config/config.go
@@ -435,9 +435,6 @@ func (cfg *Cfg) validateGit() error {
 		if err := validateGitConfigKey(configPair.Key); err != nil {
 			return fmt.Errorf("invalid configuration key %q: %w", configPair.Key, err)
 		}
-		if configPair.Value == "" {
-			return fmt.Errorf("invalid configuration value: %q", configPair.Value)
-		}
 	}
 
 	return nil
diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go
index ed56ae236..d7275c8c1 100644
--- a/internal/gitaly/config/config_test.go
+++ b/internal/gitaly/config/config_test.go
@@ -410,9 +410,8 @@ func TestValidateGitConfig(t *testing.T) {
 		{
 			desc: "missing value",
 			configPairs: []GitConfig{
-				{Key: "foo.bar"},
+				{Key: "foo.bar", Value: ""},
 			},
-			expectedErr: fmt.Errorf("invalid configuration value: \"\""),
 		},
 	}