CHANGELOG.md

 # Gitaly changelog
 
+## v1.17.0
+
+#### Other
+- Add glProjectPath to logs
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/1049
+- Switch from commitsSender to chunker
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/1060
+
+#### Security
+- Disable git protocol v2 temporarily
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/
+
 ## v1.16.0
 
 
@@ -33,6 +45,12 @@
 - Rewrite CommitStats in Go
   https://gitlab.com/gitlab-org/gitaly/merge_requests/1048
 
+## v1.14.1
+
+#### Security
+- Disable git protocol v2 temporarily
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/
+
 ## v1.14.0
 
 #### Fixed
@@ -89,6 +107,20 @@
 - Remove unused Ruby rebase/squash code
   https://gitlab.com/gitlab-org/gitaly/merge_requests/1033
 
+## v1.12.2
+
+#### Security
+- Disable git protocol v2 temporarily
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/
+
+## v1.12.1
+
+#### Fixed
+- Fix flaky rebase test
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/1028
+- Fix regression for https_proxy and unix socket connections
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/1032
+
 ## v1.12.0
 
 #### Fixed
@@ -137,6 +169,16 @@
 - README cleanup
   https://gitlab.com/gitlab-org/gitaly/merge_requests/996
 
+## v1.7.2
+
+#### Other
+- Fix tests failing due to test-repo change
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/1004
+
+#### Security
+- Disable git protocol v2 temporarily
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/
+
 ## v1.7.1
 
 #### Other
@@ -255,6 +297,16 @@
 - Support SSH credentials for push mirroring
   https://gitlab.com/gitlab-org/gitaly/merge_requests/959
 
+## v0.129.1
+
+#### Other
+- Fix tests failing due to test-repo change
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/1004
+
+#### Security
+- Disable git protocol v2 temporarily
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/
+
 ## v0.129.0
 
 #### Added

VERSION

-1.16.0
+1.17.0

changelogs/unreleased/1474-search-files-bytes.yml

+---
+title: Make clear there is no []byte reuse bug in SearchFilesByContent
+merge_request: 1055
+author:
+type: other

changelogs/unreleased/commits-sender-to-chunker.yml

----
-title: Switch from commitsSender to chunker
-merge_request: 1060
-author:
-type: other

changelogs/unreleased/sh-add-gl-project-name-logs.yml

----
-title: Add glProjectPath to logs
-merge_request: 1049
-author:
-type: other

internal/config/config.go

@@ -49,6 +49,17 @@ type GitlabShell struct {
 // Git contains the settings for the Git executable
 type Git struct {
 	BinPath string `toml:"bin_path"`
+
+	// ProtocolV2Enabled can be set to true to enable the newer Git protocol
+	// version. This should not be enabled until GitLab *either* stops
+	// using transfer.hideRefs for security purposes, *or* Git protocol v2
+	// respects this setting:
+	//
+	// https://public-inbox.org/git/20181213155817.27666-1-avarab@gmail.com/T/
+	//
+	// This is not user-configurable. Once a new Git version has been released,
+	// we can add code to enable it if the detected git binary is new enough
+	ProtocolV2Enabled bool
 }
 
 // Storage contains a single storage-shard

internal/git/protocol.go

@@ -7,6 +7,8 @@ import (
 
 	grpc_ctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags"
 	"github.com/prometheus/client_golang/prometheus"
+
+	"gitlab.com/gitlab-org/gitaly/internal/config"
 )
 
 const (
@@ -39,9 +41,13 @@ func AddGitProtocolEnv(ctx context.Context, req RequestWithGitProtocol, env []st
 	service, method := methodFromContext(ctx)
 
 	if req.GetGitProtocol() == ProtocolV2 {
-		env = append(env, fmt.Sprintf("GIT_PROTOCOL=%s", ProtocolV2))
+		if config.Config.Git.ProtocolV2Enabled {
+			env = append(env, fmt.Sprintf("GIT_PROTOCOL=%s", ProtocolV2))
 
-		gitProtocolRequests.WithLabelValues(service, method, "v2").Inc()
+			gitProtocolRequests.WithLabelValues(service, method, "v2").Inc()
+		} else {
+			gitProtocolRequests.WithLabelValues(service, method, "v2-rejected").Inc()
+		}
 	} else {
 		gitProtocolRequests.WithLabelValues(service, method, "v0").Inc()
 	}

internal/git/protocol_test.go

+package git
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"gitlab.com/gitlab-org/gitaly/internal/config"
+)
+
+type fakeProtocolMessage struct {
+	protocol string
+}
+
+func (f fakeProtocolMessage) GetGitProtocol() string {
+	return f.protocol
+}
+
+func setGitProtocolV2Support(value bool) func() {
+	orig := config.Config.Git.ProtocolV2Enabled
+	config.Config.Git.ProtocolV2Enabled = value
+
+	return func() {
+		config.Config.Git.ProtocolV2Enabled = orig
+	}
+}
+
+func TestAddGitProtocolEnvRespectsConfigEnabled(t *testing.T) {
+	restore := setGitProtocolV2Support(true)
+	defer restore()
+
+	env := []string{"fake=value"}
+	msg := fakeProtocolMessage{protocol: "version=2"}
+	value := AddGitProtocolEnv(context.Background(), msg, env)
+
+	require.Equal(t, value, append(env, "GIT_PROTOCOL=version=2"))
+}
+
+func TestAddGitProtocolEnvWhenV2NotRequested(t *testing.T) {
+	restore := setGitProtocolV2Support(true)
+	defer restore()
+
+	env := []string{"fake=value"}
+	msg := fakeProtocolMessage{protocol: ""}
+	value := AddGitProtocolEnv(context.Background(), msg, env)
+	require.Equal(t, value, env)
+}
+
+func TestAddGitProtocolEnvRespectsConfigDisabled(t *testing.T) {
+	restore := setGitProtocolV2Support(false)
+	defer restore()
+
+	env := []string{"fake=value"}
+	msg := fakeProtocolMessage{protocol: "GIT_PROTOCOL=version=2"}
+	value := AddGitProtocolEnv(context.Background(), msg, env)
+	require.Equal(t, value, env)
+}

internal/service/repository/search_files.go

@@ -122,8 +122,13 @@ func sendSearchFilesResultChunked(cmd *command.Command, stream gitalypb.Reposito
 	scanner := bufio.NewScanner(cmd)
 
 	for scanner.Scan() {
-		line := append(scanner.Bytes(), '\n')
-		if bytes.Equal(line, contentDelimiter) {
+		// Intentionally avoid scanner.Bytes() because that returns a []byte that
+		// becomes invalid on the next loop iteration, and we want to hold on to
+		// the contents of the current line for a while. Scanner.Text() is a
+		// string and hence immutable.
+		line := scanner.Text() + "\n"
+
+		if line == string(contentDelimiter) {
 			if err := sendMatchInChunks(buf, stream); err != nil {
 				return err
 			}

internal/service/smarthttp/.gitignore

+/testdata/scratch

internal/service/smarthttp/inforefs_test.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
-	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/streamio"
@@ -54,10 +53,8 @@ func TestSuccessfulInfoRefsUploadPackWithGitConfigOptions(t *testing.T) {
 }
 
 func TestSuccessfulInfoRefsUploadPackWithGitProtocol(t *testing.T) {
-	defer func(old string) {
-		config.Config.Git.BinPath = old
-	}(config.Config.Git.BinPath)
-	config.Config.Git.BinPath = "../../testhelper/env_git"
+	restore := testhelper.EnableGitProtocolV2Support()
+	defer restore()
 
 	server, serverSocketPath := runSmartHTTPServer(t)
 	defer server.Stop()
@@ -148,7 +145,7 @@ func TestFailureRepoNotFoundInfoRefsReceivePack(t *testing.T) {
 
 	client, conn := newSmartHTTPClient(t, serverSocketPath)
 	defer conn.Close()
-	repo := &gitalypb.Repository{StorageName: "default", RelativePath: "testdata/data/another_repo"}
+	repo := &gitalypb.Repository{StorageName: "default", RelativePath: "testdata/scratch/another_repo"}
 	rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo}
 
 	ctx, cancel := context.WithCancel(context.Background())

internal/service/smarthttp/receive_pack_test.go

@@ -68,10 +68,8 @@ func TestSuccessfulReceivePackRequest(t *testing.T) {
 }
 
 func TestSuccessfulReceivePackRequestWithGitProtocol(t *testing.T) {
-	defer func(old string) {
-		config.Config.Git.BinPath = old
-	}(config.Config.Git.BinPath)
-	config.Config.Git.BinPath = "../../testhelper/env_git"
+	restore := testhelper.EnableGitProtocolV2Support()
+	defer restore()
 
 	server, serverSocketPath := runSmartHTTPServer(t)
 	defer server.Stop()

internal/service/smarthttp/testdata/env_git

+../../../testhelper/env_git
\ No newline at end of file

internal/service/smarthttp/upload_pack_test.go

@@ -15,7 +15,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
-	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/pktline"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
@@ -149,10 +148,8 @@ func TestUploadPackRequestWithGitConfigOptions(t *testing.T) {
 }
 
 func TestUploadPackRequestWithGitProtocol(t *testing.T) {
-	defer func(old string) {
-		config.Config.Git.BinPath = old
-	}(config.Config.Git.BinPath)
-	config.Config.Git.BinPath = "../../testhelper/env_git"
+	restore := testhelper.EnableGitProtocolV2Support()
+	defer restore()
 
 	server, serverSocketPath := runSmartHTTPServer(t)
 	defer server.Stop()

internal/service/ssh/.gitignore

-/testdata
+/testdata/scratch

internal/service/ssh/receive_pack_test.go

@@ -106,10 +106,8 @@ func TestReceivePackPushSuccess(t *testing.T) {
 }
 
 func TestReceivePackPushSuccessWithGitProtocol(t *testing.T) {
-	defer func(old string) {
-		config.Config.Git.BinPath = old
-	}(config.Config.Git.BinPath)
-	config.Config.Git.BinPath = "../../testhelper/env_git"
+	restore := testhelper.EnableGitProtocolV2Support()
+	defer restore()
 
 	server, serverSocketPath := runSSHServer(t)
 	defer server.Stop()

internal/service/ssh/testdata/env_git

+../../../testhelper/env_git
\ No newline at end of file

internal/service/ssh/testhelper_test.go

@@ -14,7 +14,7 @@ import (
 )
 
 const (
-	testPath     = "testdata"
+	testPath     = "testdata/scratch"
 	testRepoRoot = testPath + "/data"
 )
 

internal/service/ssh/upload_pack_test.go

@@ -13,7 +13,6 @@ import (
 	"github.com/golang/protobuf/jsonpb"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
-	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"google.golang.org/grpc/codes"
@@ -98,10 +97,8 @@ func TestUploadPackCloneSuccess(t *testing.T) {
 }
 
 func TestUploadPackCloneSuccessWithGitProtocol(t *testing.T) {
-	defer func(old string) {
-		config.Config.Git.BinPath = old
-	}(config.Config.Git.BinPath)
-	config.Config.Git.BinPath = "../../testhelper/env_git"
+	restore := testhelper.EnableGitProtocolV2Support()
+	defer restore()
 
 	server, serverSocketPath := runSSHServer(t)
 	defer server.Stop()

internal/testhelper/git_protocol.go

+package testhelper
+
+import (
+	"gitlab.com/gitlab-org/gitaly/internal/config"
+)
+
+// EnableGitProtocolV2Support ensures that Git protocol v2 support is enabled,
+// and replaces the git binary in config with an `env_git` wrapper that allows
+// the protocol to be tested. It returns a function that restores the given
+// settings.
+//
+// Because we don't know how to get to that wrapper script from the current
+// working directory, callers must create a symbolic link to the `env_git` file
+// in their own `testdata` directories.
+func EnableGitProtocolV2Support() func() {
+	oldGitBinPath := config.Config.Git.BinPath
+	oldGitProtocolV2Enabled := config.Config.Git.ProtocolV2Enabled
+
+	config.Config.Git.BinPath = "testdata/env_git"
+	config.Config.Git.ProtocolV2Enabled = true
+
+	return func() {
+		config.Config.Git.BinPath = oldGitBinPath
+		config.Config.Git.ProtocolV2Enabled = oldGitProtocolV2Enabled
+	}
+}