Ignore submodules with a suspicious name in .gitmodules

If .gitmodules contains a submodule name like "../../../victim",
removing that submodule in lazygit deletes the directory "victim"
outside the repository, along with everything in it. With enough "../"
components the path becomes "/", and lazygit then deletes all files
that the user has permission to delete. An attacker only needs to
commit such a .gitmodules file to a repository. If a user of that
repository removes the submodule in lazygit, their files are gone.

This happens because lazygit deletes the submodule's git dir with
os.RemoveAll, and builds its path by joining .git/modules with the name
from .gitmodules. filepath.Join resolves the ".." components.

Git refuses to use such names. It prints "ignoring suspicious submodule
name" and treats the entry as if it didn't exist. Do the same in
GetConfigs, using the same rule as git's check_submodule_name().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

pkg/commands/git_commands/submodule.go

@@ -54,6 +54,11 @@ func (self *SubmoduleCommands) GetConfigs(parentModule *models.SubmoduleConfig)
 
 	configs := []*models.SubmoduleConfig{}
 	for _, config := range parsedConfigs {
+		if isSuspiciousSubmoduleName(config.Name) {
+			self.Log.Warnf("Ignoring suspicious submodule name: %s", config.Name)
+			continue
+		}
+
 		configs = append(configs, config)
 
 		if config.Path != "" {
@@ -109,6 +114,22 @@ func parseGitmodules(reader io.Reader, parentModule *models.SubmoduleConfig) ([]
 	return configs, nil
 }
 
+// isSuspiciousSubmoduleName applies the same rule as git's
+// check_submodule_name(); git ignores a submodule whose name fails it. The
+// git dir of a submodule is .git/modules/<name>, so a ".." component in the
+// name could put it outside the repository. Like git, we treat both slashes
+// and backslashes as separators on every platform.
+func isSuspiciousSubmoduleName(name string) bool {
+	if name == "" {
+		return true
+	}
+
+	components := strings.FieldsFunc(name, func(r rune) bool {
+		return r == '/' || r == '\\'
+	})
+	return lo.Contains(components, "..")
+}
+
 // 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

pkg/commands/git_commands/submodule_test.go

@@ -11,6 +11,37 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestIsSuspiciousSubmoduleName(t *testing.T) {
+	scenarios := []struct {
+		name       string
+		suspicious bool
+	}{
+		{name: "sub", suspicious: false},
+		{name: "path/to/sub", suspicious: false},
+		{name: "path\\to\\sub", suspicious: false},
+		{name: ".", suspicious: false},
+		{name: "...", suspicious: false},
+		{name: "..sub", suspicious: false},
+		{name: "sub..", suspicious: false},
+		{name: "a..b/c", suspicious: false},
+		{name: "", suspicious: true},
+		{name: "..", suspicious: true},
+		{name: "../victim", suspicious: true},
+		{name: "../../../victim", suspicious: true},
+		{name: "sub/..", suspicious: true},
+		{name: "sub/../../victim", suspicious: true},
+		{name: "..\\victim", suspicious: true},
+		{name: "sub\\..\\..\\victim", suspicious: true},
+		{name: "sub/..\\..\\victim", suspicious: true},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			assert.Equal(t, s.suspicious, isSuspiciousSubmoduleName(s.name))
+		})
+	}
+}
+
 func TestSubmoduleGetConflictCommits(t *testing.T) {
 	type scenario struct {
 		testName       string

pkg/integration/tests/submodule/ignore_suspicious_name.go

@@ -28,27 +28,9 @@ var IgnoreSuspiciousName = NewIntegrationTest(NewIntegrationTestArgs{
 	},
 	Run: func(t *TestDriver, keys config.KeybindingConfig) {
 		t.Views().Submodules().Focus().
-			/* EXPECTED:
 			IsEmpty()
-			ACTUAL: */
-			Lines(
-				Contains("../../../victim").IsSelected(),
-			).
-			Press(keys.Universal.Remove).
-			Tap(func() {
-				t.ExpectPopup().Confirmation().
-					Title(Equals("Remove submodule")).
-					Content(Equals("Are you sure you want to remove submodule '../../../victim' and its corresponding directory? This is irreversible.")).
-					Confirm()
-			}).
-			Lines(
-				Contains("../../../victim").IsSelected(),
-			)
 
 		t.FileSystem().
-			/* EXPECTED:
 			PathPresent("../victim/file")
-			ACTUAL: */
-			PathNotPresent("../victim/file")
 	},
 })