fix/batches: prevent Git commondir config bypass

Author: cbrnrd

internal/batches/workspace/bind_workspace_test.go

@@ -195,7 +195,8 @@ func TestDockerBindWorkspace_DiffRestoresTrustedGitConfig(t *testing.T) {
 	}
 
 	dir := *workspace.WorkDir()
-	configPath := filepath.Join(dir, ".git", "config")
+	dotGit := filepath.Join(dir, ".git")
+	configPath := filepath.Join(dotGit, "config")
 	trustedConfig, err := os.ReadFile(configPath)
 	if err != nil {
 		t.Fatal(err)
@@ -210,6 +211,26 @@ func TestDockerBindWorkspace_DiffRestoresTrustedGitConfig(t *testing.T) {
 	if err := config.Close(); err != nil {
 		t.Fatal(err)
 	}
+
+	// A commondir file redirects Git to the config in another directory. That
+	// config must not survive metadata restoration and reach host-side Git.
+	attackerCommon := filepath.Join(dir, "attacker-common")
+	if err := os.CopyFS(attackerCommon, os.DirFS(dotGit)); err != nil {
+		t.Fatal(err)
+	}
+	attackerConfig, err := os.OpenFile(filepath.Join(attackerCommon, "config"), os.O_APPEND|os.O_WRONLY, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := attackerConfig.WriteString("[filter \"attack\"]\n\tclean = command-that-must-not-run\n\trequired = true\n"); err != nil {
+		t.Fatal(err)
+	}
+	if err := attackerConfig.Close(); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dotGit, "commondir"), []byte("../attacker-common\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
 	if err := os.WriteFile(filepath.Join(dir, ".gitattributes"), []byte("*.txt filter=attack\n"), 0644); err != nil {
 		t.Fatal(err)
 	}
@@ -231,6 +252,9 @@ func TestDockerBindWorkspace_DiffRestoresTrustedGitConfig(t *testing.T) {
 	if !cmp.Equal(restoredConfig, trustedConfig) {
 		t.Fatalf("Git config was not restored:\n%s", cmp.Diff(string(trustedConfig), string(restoredConfig)))
 	}
+	if _, err := os.Stat(filepath.Join(dotGit, "commondir")); !os.IsNotExist(err) {
+		t.Fatalf("untrusted commondir was not removed: %v", err)
+	}
 }
 
 func TestUnzipRejectsGitMetadata(t *testing.T) {

internal/batches/workspace/git.go

@@ -13,6 +13,7 @@ import (
 
 type gitMetadataSnapshot struct {
 	dotGit         *gitControlFile
+	commonDir      *gitControlFile
 	config         *gitControlFile
 	configWorktree *gitControlFile
 }
@@ -34,7 +35,10 @@ func snapshotGitMetadata(dir string) (*gitMetadataSnapshot, error) {
 	case info.Mode().IsRegular():
 		snapshot.dotGit, err = snapshotGitControlFile(dotGit)
 	case info.IsDir():
-		snapshot.config, err = snapshotGitControlFile(filepath.Join(dotGit, "config"))
+		snapshot.commonDir, err = snapshotOptionalGitControlFile(filepath.Join(dotGit, "commondir"))
+		if err == nil {
+			snapshot.config, err = snapshotGitControlFile(filepath.Join(dotGit, "config"))
+		}
 		if err == nil {
 			snapshot.configWorktree, err = snapshotOptionalGitControlFile(filepath.Join(dotGit, "config.worktree"))
 		}
@@ -87,6 +91,11 @@ func (s *gitMetadataSnapshot) restore(dir string) error {
 	if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 {
 		return fmt.Errorf("%s is no longer a directory", dotGit)
 	}
+	// commondir changes which repository config Git reads. Restore it before
+	// any host-side Git command can follow an attacker-controlled redirect.
+	if err := restoreGitControlFile(filepath.Join(dotGit, "commondir"), s.commonDir); err != nil {
+		return err
+	}
 	if err := restoreGitControlFile(filepath.Join(dotGit, "config"), s.config); err != nil {
 		return err
 	}