From 93556c1588b69434cc817caf01cfc032af215122 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 23:19:09 +0100 Subject: [PATCH] Ignore submodules with a suspicious path in .gitmodules If .gitmodules contains a submodule path like "../other_repo", nuking the working tree in lazygit stashes all changes and untracked files of that other repository. The same happens if the path is a symlink that points to it. The changes can be recovered with "git stash pop" in that repository, but the user has no reason to look there. This happens because lazygit runs some commands in the submodule's directory, and takes that directory from .gitmodules as it is. Git itself only uses a path from .gitmodules if its index has a submodule at that path, and such a path can't lead out of the repository. Ignore a submodule unless its path is a directory below the directory that contains the .gitmodules file, without any symlinks in between. This also keeps GetConfigs from reading the same .gitmodules file again and again for a path of ".". Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/submodule.go | 43 +++++++++++-- pkg/commands/git_commands/submodule_test.go | 63 +++++++++++++++++++ .../tests/submodule/ignore_suspicious_path.go | 10 --- 3 files changed, 101 insertions(+), 15 deletions(-) diff --git a/pkg/commands/git_commands/submodule.go b/pkg/commands/git_commands/submodule.go index dacab426e..982f9416c 100644 --- a/pkg/commands/git_commands/submodule.go +++ b/pkg/commands/git_commands/submodule.go @@ -60,13 +60,16 @@ func (self *SubmoduleCommands) GetConfigs(parentModule *models.SubmoduleConfig) continue } + if isSuspiciousSubmodulePath(dir, config.Path) { + self.Log.Warnf("Ignoring suspicious submodule path: %s", config.Path) + continue + } + configs = append(configs, config) - if config.Path != "" { - nestedConfigs, err := self.GetConfigs(config) - if err == nil { - configs = append(configs, nestedConfigs...) - } + nestedConfigs, err := self.GetConfigs(config) + if err == nil { + configs = append(configs, nestedConfigs...) } } @@ -131,6 +134,36 @@ func isSuspiciousSubmoduleName(name string) bool { return lo.Contains(components, "..") } +// isSuspiciousSubmodulePath reports whether a submodule path from the +// .gitmodules file in parentDir might lead somewhere other than a directory +// below parentDir. We run commands in the submodule's directory and read the +// .gitmodules file in it, so the path must not lead anywhere else. +// +// Git never checks out a file through a symlink, so a path that goes through +// a symlink can't be a submodule in the first place. We reject such paths +// even if the symlink points to a directory below parentDir. +func isSuspiciousSubmodulePath(parentDir string, path string) bool { + if !filepath.IsLocal(path) || filepath.Clean(path) == "." { + return true + } + + dir := parentDir + for _, component := range strings.Split(filepath.Clean(path), string(filepath.Separator)) { + dir = filepath.Join(dir, component) + fileInfo, err := os.Lstat(dir) + if os.IsNotExist(err) { + // Nothing below this point exists (e.g. because the submodule + // isn't checked out), so it can't lead anywhere else either + return false + } + if err != nil || !fileInfo.IsDir() { + return true + } + } + + return false +} + // AnyHaveStageableChanges reports whether any of the given submodule paths has // a checked-out commit that differs from the one recorded in the // superproject's index, i.e. a change that `git add ` would actually diff --git a/pkg/commands/git_commands/submodule_test.go b/pkg/commands/git_commands/submodule_test.go index ad7ec53b8..2740ecf51 100644 --- a/pkg/commands/git_commands/submodule_test.go +++ b/pkg/commands/git_commands/submodule_test.go @@ -45,6 +45,69 @@ func TestIsSuspiciousSubmoduleName(t *testing.T) { } } +func TestIsSuspiciousSubmodulePath(t *testing.T) { + dir := t.TempDir() + parentDir := filepath.Join(dir, "repo") + assert.NoError(t, os.MkdirAll(filepath.Join(parentDir, "sub"), 0o755)) + assert.NoError(t, os.MkdirAll(filepath.Join(parentDir, "path", "to", "sub"), 0o755)) + assert.NoError(t, os.MkdirAll(filepath.Join(dir, "other"), 0o755)) + assert.NoError(t, os.WriteFile(filepath.Join(parentDir, "file"), []byte("content"), 0o644)) + + scenarios := []struct { + path string + suspicious bool + }{ + {path: "sub", suspicious: false}, + {path: "sub/", suspicious: false}, + {path: "path/to/sub", suspicious: false}, + {path: "not_checked_out", suspicious: false}, + {path: "path/not_checked_out/sub", suspicious: false}, + {path: "", suspicious: true}, + {path: ".", suspicious: true}, + {path: "./", suspicious: true}, + {path: "sub/..", suspicious: true}, + {path: "..", suspicious: true}, + {path: "../other", suspicious: true}, + {path: "sub/../../other", suspicious: true}, + {path: filepath.Join(dir, "other"), suspicious: true}, + {path: "file", suspicious: true}, + {path: "file/sub", suspicious: true}, + } + + for _, s := range scenarios { + t.Run(s.path, func(t *testing.T) { + assert.Equal(t, s.suspicious, isSuspiciousSubmodulePath(parentDir, s.path)) + }) + } +} + +func TestIsSuspiciousSubmodulePathWithSymlinks(t *testing.T) { + dir := t.TempDir() + parentDir := filepath.Join(dir, "repo") + assert.NoError(t, os.MkdirAll(filepath.Join(parentDir, "sub"), 0o755)) + assert.NoError(t, os.MkdirAll(filepath.Join(dir, "other", "sub"), 0o755)) + if err := os.Symlink(filepath.Join("..", "other"), filepath.Join(parentDir, "link_outside")); err != nil { + t.Skipf("Can't create symlinks: %v", err) + } + assert.NoError(t, os.Symlink("sub", filepath.Join(parentDir, "link_inside"))) + + scenarios := []struct { + path string + suspicious bool + }{ + {path: "sub", suspicious: false}, + {path: "link_outside", suspicious: true}, + {path: "link_outside/sub", suspicious: true}, + {path: "link_inside", suspicious: true}, + } + + for _, s := range scenarios { + t.Run(s.path, func(t *testing.T) { + assert.Equal(t, s.suspicious, isSuspiciousSubmodulePath(parentDir, s.path)) + }) + } +} + func TestSubmoduleDeleteOutsideModulesDir(t *testing.T) { scenarios := []struct { testName string diff --git a/pkg/integration/tests/submodule/ignore_suspicious_path.go b/pkg/integration/tests/submodule/ignore_suspicious_path.go index 0f0d0e953..d2cf3416e 100644 --- a/pkg/integration/tests/submodule/ignore_suspicious_path.go +++ b/pkg/integration/tests/submodule/ignore_suspicious_path.go @@ -38,13 +38,7 @@ var IgnoreSuspiciousPath = NewIntegrationTest(NewIntegrationTestArgs{ }, Run: func(t *TestDriver, keys config.KeybindingConfig) { t.Views().Submodules(). - /* EXPECTED: IsEmpty() - ACTUAL: */ - Lines( - Contains("other"), - Contains("linked"), - ) t.Views().Files().Focus(). Lines( @@ -65,11 +59,7 @@ var IgnoreSuspiciousPath = NewIntegrationTest(NewIntegrationTestArgs{ IsEmpty() t.FileSystem(). - /* EXPECTED: FileContent("../other_repo/file", Equals("modified")). FileContent("../linked_repo/file", Equals("modified")) - ACTUAL: */ - FileContent("../other_repo/file", Equals("original")). - FileContent("../linked_repo/file", Equals("original")) }, })