From 82de59b3ffc11a3324958b5ff61c85081d06e0a6 Mon Sep 17 00:00:00 2001
From: Stefan Haller <stefan@haller-berlin.de>
Date: Sat, 7 Nov 2026 21:50:29 +0100
Subject: [PATCH] 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        | 21 +++++++++++++
 pkg/commands/git_commands/submodule_test.go   | 31 +++++++++++++++++++
 .../tests/submodule/ignore_suspicious_name.go | 18 -----------
 3 files changed, 52 insertions(+), 18 deletions(-)

diff --git a/pkg/commands/git_commands/submodule.go b/pkg/commands/git_commands/submodule.go
index 80ccbfcdd..addff67cb 100644
--- a/pkg/commands/git_commands/submodule.go
+++ b/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
diff --git a/pkg/commands/git_commands/submodule_test.go b/pkg/commands/git_commands/submodule_test.go
index d449d81de..ab2c10474 100644
--- a/pkg/commands/git_commands/submodule_test.go
+++ b/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
diff --git a/pkg/integration/tests/submodule/ignore_suspicious_name.go b/pkg/integration/tests/submodule/ignore_suspicious_name.go
index 497b12134..fdca074cd 100644
--- a/pkg/integration/tests/submodule/ignore_suspicious_name.go
+++ b/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")
 	},
 })