From 1c0d242250619a0642522eae26cc2b310765a4b3 Mon Sep 17 00:00:00 2001 From: Carter Brainerd Date: Thu, 3 Sep 2026 14:16:57 -0400 Subject: [PATCH] fix/batch: prevent host execution through Git config --- internal/batches/workspace/bind_workspace.go | 21 +++- .../batches/workspace/bind_workspace_test.go | 49 +++++++++ .../batches/workspace/executor_workspace.go | 10 +- internal/batches/workspace/git.go | 104 ++++++++++++++++++ 4 files changed, 180 insertions(+), 4 deletions(-) diff --git a/internal/batches/workspace/bind_workspace.go b/internal/batches/workspace/bind_workspace.go index 3068ec2240..8297c148c9 100644 --- a/internal/batches/workspace/bind_workspace.go +++ b/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 { diff --git a/internal/batches/workspace/bind_workspace_test.go b/internal/batches/workspace/bind_workspace_test.go index 80ae000c03..781cff8261 100644 --- a/internal/batches/workspace/bind_workspace_test.go +++ b/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", diff --git a/internal/batches/workspace/executor_workspace.go b/internal/batches/workspace/executor_workspace.go index d682d46578..f6c4366755 100644 --- a/internal/batches/workspace/executor_workspace.go +++ b/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 } diff --git a/internal/batches/workspace/git.go b/internal/batches/workspace/git.go index 8f59a282b6..43c1c8ba70 100644 --- a/internal/batches/workspace/git.go +++ b/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.