fix/batches: prevent repository archives from enabling Git hooks

Author: cbrnrd

internal/batches/workspace/bind_workspace.go

@@ -190,6 +190,15 @@ func unzip(ctx context.Context, zipFile, dest string) error {
 	}
 	defer r.Close()
 
+	for _, f := range r.File {
+		// ZIP paths use forward slashes on every platform. Reject backslashes
+		// rather than letting Windows reinterpret a filename from a Unix
+		// repository as a directory hierarchy.
+		if strings.ContainsRune(f.Name, '\\') {
+			return fmt.Errorf("%q: illegal file path", f.Name)
+		}
+	}
+
 	outputBase := filepath.Clean(dest) + string(os.PathSeparator)
 
 	for _, f := range r.File {

internal/batches/workspace/bind_workspace_test.go

@@ -143,6 +143,54 @@ func TestDockerBindWorkspaceCreator_Create(t *testing.T) {
 	})
 }
 
+func TestUnzipRejectsUnsafeArchivePaths(t *testing.T) {
+	tests := []string{
+		`.git\config`,
+		`hooks\pre-commit`,
+	}
+
+	for _, name := range tests {
+		t.Run(name, func(t *testing.T) {
+			archivePath := zipUpFiles(t, t.TempDir(), map[string]string{name: "malicious"})
+			dest := t.TempDir()
+
+			if err := unzip(context.Background(), archivePath, dest); err == nil {
+				t.Fatal("expected unsafe archive path to be rejected")
+			}
+
+			entries, err := os.ReadDir(dest)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(entries) != 0 {
+				t.Fatalf("archive was partially extracted: %v", entries)
+			}
+		})
+	}
+}
+
+func TestUnzipAllowsSafeControlPaths(t *testing.T) {
+	files := map[string]string{
+		".git_config":      "config",
+		"hooks_pre-commit": "hook",
+	}
+	archivePath := zipUpFiles(t, t.TempDir(), files)
+	dest := t.TempDir()
+
+	if err := unzip(context.Background(), archivePath, dest); err != nil {
+		t.Fatal(err)
+	}
+	for name, want := range files {
+		have, err := os.ReadFile(filepath.Join(dest, name))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if string(have) != want {
+			t.Errorf("%s: got %q, want %q", name, have, want)
+		}
+	}
+}
+
 func TestDockerBindWorkspace_ApplyDiff(t *testing.T) {
 	// Create a zip file for all the other tests to use.
 	fakeFilesTmpDir := t.TempDir()

internal/batches/workspace/git.go

@@ -9,6 +9,9 @@ import (
 )
 
 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.
+	args = append([]string{"-c", "core.hooksPath=/dev/null"}, args...)
 	cmd := exec.CommandContext(ctx, "git", args...)
 	cmd.Env = []string{
 		// Don't use the system wide git config.

internal/batches/workspace/git_test.go

@@ -0,0 +1,46 @@
+package workspace
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestRunGitCmdDisablesHooks(t *testing.T) {
+	dir := t.TempDir()
+	ctx := context.Background()
+
+	if _, err := runGitCmd(ctx, dir, "init", "--quiet"); err != nil {
+		t.Fatal(err)
+	}
+	config, err := os.OpenFile(filepath.Join(dir, ".git", "config"), os.O_APPEND|os.O_WRONLY, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := config.WriteString("[core]\n\thooksPath = hooks\n"); err != nil {
+		t.Fatal(err)
+	}
+	if err := config.Close(); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Mkdir(filepath.Join(dir, "hooks"), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "hooks", "pre-commit"), []byte("#!/bin/sh\necho hook-ran > hook-ran\n"), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("test\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := runGitCmd(ctx, dir, "add", "README.md"); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := runGitCmd(ctx, dir, "commit", "--quiet", "-m", "test"); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := os.Stat(filepath.Join(dir, "hook-ran")); !os.IsNotExist(err) {
+		t.Fatalf("pre-commit hook ran: %v", err)
+	}
+}