Merge remote-tracking branch 'dev/13-3-stable' into 13-3-stable13-3-stable

5 files changed, 138 insertions, 2 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ae999af77..bc638a8c3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # Gitaly changelog
 
+## 13.3.9 (2020-11-02)
+
+### Security (1 change)
+
+- Removal of all http.*.extraHeader config values.
+
+
 ## 13.3.8 (2020-10-21)
 
 - No changes.
diff --git a/VERSION b/VERSION
index 176df6ffe..4feaa4d3c 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-13.3.8
\ No newline at end of file
+13.3.9
\ No newline at end of file
diff --git a/internal/service/repository/optimize.go b/internal/service/repository/optimize.go
index 9d0d58056..0d8520133 100644
--- a/internal/service/repository/optimize.go
+++ b/internal/service/repository/optimize.go
@@ -1,11 +1,16 @@
 package repository
 
 import (
+	"bufio"
 	"context"
+	"errors"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"strings"
 
 	"github.com/prometheus/client_golang/prometheus"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
@@ -130,6 +135,12 @@ func (s *server) optimizeRepository(ctx context.Context, repository *gitalypb.Re
 		return fmt.Errorf("OptimizeRepository: remove empty refs: %w", err)
 	}
 
+	// TODO: https://gitlab.com/gitlab-org/gitaly/-/issues/3138
+	// This is a temporary code and needs to be removed once it will be run on all repositories at least once.
+	if err := s.unsetAllConfigsByRegexp(ctx, repository, "^http\\..+\\.extraHeader$"); err != nil {
+		return fmt.Errorf("OptimizeRepository: unset all configs by regexp: %w", err)
+	}
+
 	return nil
 }
 
@@ -157,3 +168,92 @@ func (s *server) validateOptimizeRepositoryRequest(in *gitalypb.OptimizeReposito
 
 	return nil
 }
+
+func (s *server) unsetAllConfigsByRegexp(ctx context.Context, repository *gitalypb.Repository, regexp string) error {
+	keys, err := getConfigKeys(ctx, repository, regexp)
+	if err != nil {
+		return fmt.Errorf("get config keys: %w", err)
+	}
+
+	if err := unsetConfigKeys(ctx, repository, keys); err != nil {
+		return fmt.Errorf("unset all keys: %w", err)
+	}
+
+	return nil
+}
+
+func getConfigKeys(ctx context.Context, repository *gitalypb.Repository, regexp string) ([]string, error) {
+	cmd, err := git.SafeCmd(ctx, repository, nil, git.SubCmd{
+		Name: "config",
+		Flags: []git.Option{
+			git.Flag{Name: "--name-only"},
+			git.ValueFlag{Name: "--get-regexp", Value: regexp},
+		},
+	})
+	if err != nil {
+		return nil, fmt.Errorf("creation of 'git config': %w", err)
+	}
+
+	keys, err := parseConfigKeys(cmd)
+	if err != nil {
+		return nil, fmt.Errorf("parse config keys: %w", err)
+	}
+
+	if err := cmd.Wait(); err != nil {
+		var termErr *exec.ExitError
+		if errors.As(err, &termErr) {
+			if termErr.ExitCode() == 1 {
+				// https://git-scm.com/docs/git-config#_description: The section or key is invalid (ret=1)
+				// This means no matching values were found.
+				return nil, nil
+			}
+		}
+		return nil, fmt.Errorf("wait for 'git config': %w", err)
+	}
+
+	return keys, nil
+}
+
+func parseConfigKeys(reader io.Reader) ([]string, error) {
+	var keys []string
+
+	scanner := bufio.NewScanner(reader)
+	for scanner.Scan() {
+		keys = append(keys, scanner.Text())
+	}
+	if err := scanner.Err(); err != nil {
+		return nil, err
+	}
+
+	return keys, nil
+}
+
+func unsetConfigKeys(ctx context.Context, repository *gitalypb.Repository, names []string) error {
+	for _, name := range names {
+		if err := unsetAll(ctx, repository, name); err != nil {
+			return fmt.Errorf("unset all: %w", err)
+		}
+	}
+
+	return nil
+}
+
+func unsetAll(ctx context.Context, repository *gitalypb.Repository, name string) error {
+	if strings.TrimSpace(name) == "" {
+		return nil
+	}
+
+	cmd, err := git.SafeCmd(ctx, repository, nil, git.SubCmd{
+		Name:  "config",
+		Flags: []git.Option{git.ValueFlag{Name: "--unset-all", Value: name}},
+	})
+	if err != nil {
+		return fmt.Errorf("creation of 'git config': %w", err)
+	}
+
+	if err := cmd.Wait(); err != nil {
+		return fmt.Errorf("wait for 'git config': %w", err)
+	}
+
+	return nil
+}
diff --git a/internal/service/repository/optimize_test.go b/internal/service/repository/optimize_test.go
index cba2caa29..ee5fafb6c 100644
--- a/internal/service/repository/optimize_test.go
+++ b/internal/service/repository/optimize_test.go
@@ -1,6 +1,8 @@
 package repository
 
 import (
+	"bytes"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -52,6 +54,23 @@ func TestOptimizeRepository(t *testing.T) {
 
 	testhelper.CreateCommit(t, testRepoPath, "master", nil)
 
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "http.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c1.git.extraHeader", "Authorization: Basic secret-password")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "http.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c2.git.extraHeader", "Authorization: Basic secret-password")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "randomStart-http.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c3.git.extraHeader", "Authorization: Basic secret-password")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "http.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c4.git.extraHeader-randomEnd", "Authorization: Basic secret-password")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "hTTp.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git.ExtrAheaDeR", "Authorization: Basic secret-password")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "http.http://extraHeader/extraheader/EXTRAHEADER.git.extraHeader", "Authorization: Basic secret-password")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "https.https://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git.extraHeader", "Authorization: Basic secret-password")
+	confFileData, err := ioutil.ReadFile(filepath.Join(testRepoPath, "config"))
+	require.NoError(t, err)
+	require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c1.git")))
+	require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c2.git")))
+	require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c3")))
+	require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c4.git")))
+	require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git")))
+	require.True(t, bytes.Contains(confFileData, []byte("http://extraHeader/extraheader/EXTRAHEADER.git")))
+	require.True(t, bytes.Contains(confFileData, []byte("https://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git")))
+
 	serverSocketPath, stop := runRepoServer(t)
 	defer stop()
 
@@ -61,6 +80,16 @@ func TestOptimizeRepository(t *testing.T) {
 	_, err = repoClient.OptimizeRepository(ctx, &gitalypb.OptimizeRepositoryRequest{Repository: testRepo})
 	require.NoError(t, err)
 
+	confFileData, err = ioutil.ReadFile(filepath.Join(testRepoPath, "config"))
+	require.NoError(t, err)
+	require.False(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c1.git")))
+	require.False(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c2.git")))
+	require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c3")))
+	require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c4.git")))
+	require.False(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git")))
+	require.False(t, bytes.Contains(confFileData, []byte("http://extraHeader/extraheader/EXTRAHEADER.git.extraHeader")))
+	require.True(t, bytes.Contains(confFileData, []byte("https://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git")))
+
 	require.Equal(t, getNewestPackfileModtime(t, testRepoPath), newestsPackfileTime, "there should not have been a new packfile created")
 
 	testRepo, testRepoPath, cleanupBare := testhelper.InitBareRepo(t)
diff --git a/ruby/proto/gitaly/version.rb b/ruby/proto/gitaly/version.rb
index 797d4b9c4..30f7af238 100644
--- a/ruby/proto/gitaly/version.rb
+++ b/ruby/proto/gitaly/version.rb
@@ -2,5 +2,5 @@
 # (https://gitlab.com/gitlab-org/release-tools/), and should not be
 # modified.
 module Gitaly
-  VERSION = '13.3.8'
+  VERSION = '13.3.9'
 end