Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion internal/batches/workspace/bind_workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -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) {
Expand Down
11 changes: 10 additions & 1 deletion internal/batches/workspace/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

type gitMetadataSnapshot struct {
dotGit *gitControlFile
commonDir *gitControlFile
config *gitControlFile
configWorktree *gitControlFile
}
Expand All @@ -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"))
}
Expand Down Expand Up @@ -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
}
Expand Down
Loading