fix/batch: prevent host execution through Git config

internal/batches/workspace/bind_workspace.go

@@ -34,7 +34,12 @@ func (wc *dockerBindWorkspaceCreator) Create(ctx context.Context, repo *graphql.
 		return nil, errors.Wrap(err, "copying additional files into workspace")
 	}
 
-	return w, errors.Wrap(wc.prepareGitRepo(ctx, w), "preparing local git repo")
+	if err := wc.prepareGitRepo(ctx, w); err != nil {
+		return nil, errors.Wrap(err, "preparing local git repo")
+	}
+
+	w.gitMetadata, err = snapshotGitMetadata(w.dir)
+	return w, errors.Wrap(err, "snapshotting local git metadata")
 }
 
 func (*dockerBindWorkspaceCreator) prepareGitRepo(ctx context.Context, w *dockerBindWorkspace) error {
@@ -111,6 +116,10 @@ type dockerBindWorkspace struct {
 	// This is also the path that is directly mounted into the docker
 	// containers.
 	dir string
+	// gitMetadata contains the trusted Git control files from before repository
+	// code runs. The bind-mounted repository can modify its Git metadata, so the
+	// snapshot must be restored before Git is run on the host.
+	gitMetadata *gitMetadataSnapshot
 }
 
 var _ Workspace = &dockerBindWorkspace{}
@@ -129,6 +138,10 @@ func (w *dockerBindWorkspace) DockerRunOpts(ctx context.Context, target string)
 func (w *dockerBindWorkspace) WorkDir() *string { return &w.dir }
 
 func (w *dockerBindWorkspace) Diff(ctx context.Context) ([]byte, error) {
+	if err := w.gitMetadata.restore(w.dir); err != nil {
+		return nil, errors.Wrap(err, "restoring trusted git metadata")
+	}
+
 	if _, err := runGitCmd(ctx, w.dir, "add", "--all"); err != nil {
 		return nil, errors.Wrap(err, "git add failed")
 	}
@@ -141,10 +154,14 @@ func (w *dockerBindWorkspace) Diff(ctx context.Context) ([]byte, error) {
 	//
 	// ATTENTION: When you change the options here, be sure to also update the
 	// ApplyDiff method accordingly.
-	return runGitCmd(ctx, w.dir, "diff", "--cached", "--no-prefix", "--binary")
+	return runGitCmd(ctx, w.dir, "diff", "--cached", "--no-ext-diff", "--no-prefix", "--binary")
 }
 
 func (w *dockerBindWorkspace) ApplyDiff(ctx context.Context, diff []byte) error {
+	if err := w.gitMetadata.restore(w.dir); err != nil {
+		return errors.Wrap(err, "restoring trusted git metadata")
+	}
+
 	// Write the diff to a temp file so we can pass it to `git apply`
 	tmp, err := os.CreateTemp(w.tempDir, "bind-workspace-test-*")
 	if err != nil {

internal/batches/workspace/bind_workspace_test.go

@@ -143,6 +143,55 @@ func TestDockerBindWorkspaceCreator_Create(t *testing.T) {
 	})
 }
 
+func TestDockerBindWorkspace_DiffRestoresTrustedGitConfig(t *testing.T) {
+	archivePath := zipUpFiles(t, t.TempDir(), map[string]string{
+		"tracked.txt": "before\n",
+	})
+	creator := &dockerBindWorkspaceCreator{Dir: t.TempDir()}
+	workspace, err := creator.Create(context.Background(), repo, nil, &fakeRepoArchive{mockPath: archivePath})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dir := *workspace.WorkDir()
+	configPath := filepath.Join(dir, ".git", "config")
+	trustedConfig, err := os.ReadFile(configPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	config, err := os.OpenFile(configPath, os.O_APPEND|os.O_WRONLY, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := config.WriteString("[diff]\n\texternal = command-that-must-not-run\n[filter \"attack\"]\n\tclean = command-that-must-not-run\n"); err != nil {
+		t.Fatal(err)
+	}
+	if err := config.Close(); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, ".gitattributes"), []byte("*.txt filter=attack\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "tracked.txt"), []byte("after\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	diff, err := workspace.Diff(context.Background())
+	if err != nil {
+		t.Fatalf("Diff executed untrusted Git configuration: %s", err)
+	}
+	if !strings.Contains(string(diff), "+after") {
+		t.Fatalf("diff does not contain tracked change:\n%s", diff)
+	}
+	restoredConfig, err := os.ReadFile(configPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !cmp.Equal(restoredConfig, trustedConfig) {
+		t.Fatalf("Git config was not restored:\n%s", cmp.Diff(string(trustedConfig), string(restoredConfig)))
+	}
+}
+
 func TestUnzipRejectsGitMetadata(t *testing.T) {
 	for _, name := range []string{
 		".git/config",

internal/batches/workspace/executor_workspace.go

@@ -24,10 +24,16 @@ type executorWorkspaceCreator struct {
 var _ Creator = &executorWorkspaceCreator{}
 
 func (wc *executorWorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository, steps []batcheslib.Step, archive repozip.Archive) (Workspace, error) {
+	gitMetadata, err := snapshotGitMetadata(wc.RepoDir)
+	if err != nil {
+		return nil, err
+	}
+
 	return &dockerBindExecutorWorkspace{
 		dockerBindWorkspace: dockerBindWorkspace{
-			tempDir: wc.TempDir,
-			dir:     wc.RepoDir,
+			tempDir:     wc.TempDir,
+			dir:         wc.RepoDir,
+			gitMetadata: gitMetadata,
 		},
 	}, nil
 }

internal/batches/workspace/git.go

@@ -2,12 +2,116 @@ package workspace
 
 import (
 	"context"
+	"fmt"
+	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
 
+type gitMetadataSnapshot struct {
+	dotGit         *gitControlFile
+	config         *gitControlFile
+	configWorktree *gitControlFile
+}
+
+type gitControlFile struct {
+	contents []byte
+	mode     os.FileMode
+}
+
+func snapshotGitMetadata(dir string) (*gitMetadataSnapshot, error) {
+	dotGit := filepath.Join(dir, ".git")
+	info, err := os.Lstat(dotGit)
+	if err != nil {
+		return nil, err
+	}
+
+	snapshot := &gitMetadataSnapshot{}
+	switch {
+	case info.Mode().IsRegular():
+		snapshot.dotGit, err = snapshotGitControlFile(dotGit)
+	case info.IsDir():
+		snapshot.config, err = snapshotGitControlFile(filepath.Join(dotGit, "config"))
+		if err == nil {
+			snapshot.configWorktree, err = snapshotOptionalGitControlFile(filepath.Join(dotGit, "config.worktree"))
+		}
+	default:
+		return nil, fmt.Errorf("%s is not a regular file or directory", dotGit)
+	}
+	if err != nil {
+		return nil, err
+	}
+	return snapshot, nil
+}
+
+func snapshotOptionalGitControlFile(path string) (*gitControlFile, error) {
+	file, err := snapshotGitControlFile(path)
+	if os.IsNotExist(err) {
+		return nil, nil
+	}
+	return file, err
+}
+
+func snapshotGitControlFile(path string) (*gitControlFile, error) {
+	info, err := os.Lstat(path)
+	if err != nil {
+		return nil, err
+	}
+	if !info.Mode().IsRegular() {
+		return nil, fmt.Errorf("%s is not a regular file", path)
+	}
+	contents, err := os.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+	return &gitControlFile{contents: contents, mode: info.Mode()}, nil
+}
+
+func (s *gitMetadataSnapshot) restore(dir string) error {
+	if s == nil {
+		return nil
+	}
+
+	dotGit := filepath.Join(dir, ".git")
+	if s.dotGit != nil {
+		return restoreGitControlFile(dotGit, s.dotGit)
+	}
+
+	info, err := os.Lstat(dotGit)
+	if err != nil {
+		return err
+	}
+	if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 {
+		return fmt.Errorf("%s is no longer a directory", dotGit)
+	}
+	if err := restoreGitControlFile(filepath.Join(dotGit, "config"), s.config); err != nil {
+		return err
+	}
+	return restoreGitControlFile(filepath.Join(dotGit, "config.worktree"), s.configWorktree)
+}
+
+func restoreGitControlFile(path string, snapshot *gitControlFile) error {
+	if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	if snapshot == nil {
+		return nil
+	}
+
+	file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, snapshot.mode.Perm())
+	if err != nil {
+		return err
+	}
+	if _, err := file.Write(snapshot.contents); err != nil {
+		file.Close()
+		return err
+	}
+	return file.Close()
+}
+
 func runGitCmd(ctx context.Context, dir string, args ...string) ([]byte, error) {
 	// Repository contents are untrusted. Keep hooks disabled even if a command
 	// encounters an attacker-controlled local Git configuration.