From b324c831cbc02c82b99c18113b622d03539d23cd Mon Sep 17 00:00:00 2001 From: Byron Date: Tue, 4 Aug 2026 11:07:02 +0200 Subject: [PATCH 1/2] Initialize submodule repository before handled failures Direct recursive updates with keep_going could swallow an invalid-name validation error and then access mrepo before assignment. Reproduce that path and initialize mrepo before validation so recursion is safely skipped. Git baseline: submodule.c at 883a47ef6496c96a5d6132ed8c87fcd44ebf8d1a validates submodule paths before operating on them. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- .basedpyright/baseline.json | 8 -------- git/objects/submodule/base.py | 4 +--- test/test_submodule.py | 2 ++ 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/.basedpyright/baseline.json b/.basedpyright/baseline.json index 013b097ae..59a5145c3 100644 --- a/.basedpyright/baseline.json +++ b/.basedpyright/baseline.json @@ -491,14 +491,6 @@ "lineCount": 1 } }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 15, - "endColumn": 20, - "lineCount": 1 - } - }, { "code": "reportArgumentType", "range": { diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 658259cd2..9b1eb0e02 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -747,9 +747,7 @@ def update( prefix = "DRY-RUN: " # END handle prefix - # To keep things plausible in dry-run mode. - if dry_run: - mrepo = None + mrepo = None # END init mrepo def fetch_remotes(module_repo: "Repo") -> None: diff --git a/test/test_submodule.py b/test/test_submodule.py index 217ee7a99..32b1c7af1 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -994,6 +994,8 @@ def test_root_update_keeps_going_after_invalid_submodule_name(self, rwdir): assert not clone.submodule("../invalid").module_exists() assert clone.submodule("valid").module_exists() + clone.submodule("../invalid").update(recursive=True, keep_going=True) + @with_rw_directory @_patch_git_config("protocol.file.allow", "always") def test_list_only_valid_submodules(self, rwdir): From 4299c990e1ca21896f9485277caf7bb0ae5b404c Mon Sep 17 00:00:00 2001 From: Byron Date: Tue, 4 Aug 2026 10:08:41 +0200 Subject: [PATCH 2/2] Validate submodule names on every module path branch _module_abspath only validated names when Git used separate gitfile submodule directories, so legacy Git behavior could accept the same invalid name. Validate before selecting the Git-version-dependent path branch and cover the legacy branch explicitly. Git baseline: submodule.c at 883a47ef6496c96a5d6132ed8c87fcd44ebf8d1a validates submodule paths before filesystem operations. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/objects/submodule/base.py | 3 ++- test/test_submodule.py | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 9b1eb0e02..56fd16669 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -316,8 +316,9 @@ def _validated_name(cls, name: str) -> str: @classmethod def _module_abspath(cls, parent_repo: "Repo", path: PathLike, name: str) -> PathLike: + name = cls._validated_name(name) if cls._need_gitfile_submodules(parent_repo.git): - return osp.join(parent_repo.git_dir, "modules", cls._validated_name(name)) + return osp.join(parent_repo.git_dir, "modules", name) if parent_repo.working_tree_dir: return osp.join(parent_repo.working_tree_dir, path) raise NotADirectoryError() diff --git a/test/test_submodule.py b/test/test_submodule.py index 32b1c7af1..8c8a53641 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -971,6 +971,10 @@ def test_update_rejects_parent_component_in_name(self, rwdir): with pytest.raises(ValueError, match="submodule name"): Submodule._module_abspath(clone, "module", name) + with mock.patch.object(Submodule, "_need_gitfile_submodules", return_value=False): + with pytest.raises(ValueError, match="submodule name"): + Submodule._module_abspath(clone, "module", "../module") + @with_rw_directory @_patch_git_config("protocol.file.allow", "always") def test_root_update_keeps_going_after_invalid_submodule_name(self, rwdir):