From bcbc5ebd6dec639c5ce2d651d89d3ffe1b6e9bf8 Mon Sep 17 00:00:00 2001
From: Stefan Haller <stefan@haller-berlin.de>
Date: Sat, 7 Nov 2026 21:53:08 +0100
Subject: [PATCH] Refuse to remove a submodule whose git dir isn't below
 .git/modules

Git accepts "." as a submodule name, and so does GetConfigs. Removing
such a submodule in lazygit deletes .git/modules, together with the git
dirs of all other submodules. Any unpushed commits in those submodules
are lost.

Check the path of the git dir before running any git command, and
refuse to remove the submodule if the path isn't below .git/modules.
This also protects against names with ".." components, in case one ever
gets past GetConfigs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 pkg/commands/git_commands/submodule.go      | 16 ++++++++++++++--
 pkg/commands/git_commands/submodule_test.go |  7 -------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/pkg/commands/git_commands/submodule.go b/pkg/commands/git_commands/submodule.go
index addff67cb..dacab426e 100644
--- a/pkg/commands/git_commands/submodule.go
+++ b/pkg/commands/git_commands/submodule.go
@@ -2,6 +2,7 @@ package git_commands
 
 import (
 	"bufio"
+	"fmt"
 	"io"
 	"os"
 	"path/filepath"
@@ -275,6 +276,18 @@ func (self *SubmoduleCommands) runInParentModule(submodule *models.SubmoduleConf
 func (self *SubmoduleCommands) Delete(submodule *models.SubmoduleConfig) error {
 	// based on https://gist.github.com/myusuf3/7f645819ded92bda6677
 
+	// Git keeps the git dirs of submodules separately for each worktree
+	gitDirPath := submodule.GitDirPath(self.repoPaths.WorktreeGitDirPath())
+
+	// The name of the submodule comes from .gitmodules, so it is under the
+	// control of whoever committed that file. Make sure that we never delete
+	// anything other than a directory below the "modules" directory of the
+	// worktree's git dir.
+	modulesDirPath := filepath.Join(self.repoPaths.WorktreeGitDirPath(), "modules")
+	if relPath, err := filepath.Rel(modulesDirPath, gitDirPath); err != nil || relPath == "." || !filepath.IsLocal(relPath) {
+		return fmt.Errorf("refusing to delete %s because it is not inside %s", gitDirPath, modulesDirPath)
+	}
+
 	if err := self.runInParentModule(submodule, self.cmd.New(
 		NewGitCmd("submodule").
 			Arg("deinit", "--force", "--", submodule.Path).ToArgv(),
@@ -307,8 +320,7 @@ func (self *SubmoduleCommands) Delete(submodule *models.SubmoduleConfig) error {
 		self.Log.Error(err)
 	}
 
-	// Git keeps the git dirs of submodules separately for each worktree
-	return os.RemoveAll(submodule.GitDirPath(self.repoPaths.WorktreeGitDirPath()))
+	return os.RemoveAll(gitDirPath)
 }
 
 func (self *SubmoduleCommands) Add(name string, path string, url string) error {
diff --git a/pkg/commands/git_commands/submodule_test.go b/pkg/commands/git_commands/submodule_test.go
index d431dfda0..ad7ec53b8 100644
--- a/pkg/commands/git_commands/submodule_test.go
+++ b/pkg/commands/git_commands/submodule_test.go
@@ -72,21 +72,14 @@ func TestSubmoduleDeleteOutsideModulesDir(t *testing.T) {
 			assert.NoError(t, os.WriteFile(victimFile, []byte("content"), 0o644))
 
 			runner := oscommands.NewFakeRunner(t)
-			runner.
-				ExpectGitArgs([]string{"submodule", "deinit", "--force", "--", "sub"}, "", nil).
-				ExpectGitArgs([]string{"rm", "--force", "-r", "sub"}, "", nil)
 			instance := buildSubmoduleCommands(commonDeps{
 				runner:    runner,
 				repoPaths: MockRepoPaths(filepath.Join(dir, "repo")),
 			})
 
 			err := instance.Delete(&models.SubmoduleConfig{Name: s.name, Path: "sub"})
-			/* EXPECTED:
 			assert.Error(t, err)
 			assert.FileExists(t, victimFile)
-			ACTUAL: */
-			assert.NoError(t, err)
-			assert.NoFileExists(t, victimFile)
 			runner.CheckForMissingCalls()
 		})
 	}