mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-11-13 08:13:32 -04:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
a159e24919
commit
93556c1588
@@ -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 <path>` would actually
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"))
|
||||
},
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user