fix(submodule): validate destinations before mutation

[Byron]

Tasks

This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.

• refackiew

---

Everything below this line was generated by Codex.

Created by Codex on behalf of Byron. Byron will review before this is ready to merge.

Submodule destinations could fail index validation after cloning had already changed the filesystem. Validate checkout paths before mutation across creation, initialization, direct cloning, and moves, and validate metadata destinations before creation, reuse, and renaming.

• Reuse the existing portable path validator for .git aliases, including NTFS alternate streams and short names, HFS ignored characters, invalid components, and NULs. Apply Git for Windows' filename restrictions before path normalization or directory creation, preserving evidence that Windows would otherwise strip.
• Compare checkout components with the parent's Git and common directories by filesystem identity, protecting separately named metadata directories and their filesystem aliases.
• Reject metadata nested inside another submodule's Git directory. Repeat the check after cloning and disable a clone that became nested, following Git's behavior. Preserve supported metadata symlinks and safe relocation of a submodule's own metadata.

Advisory summary

GHSA-83vg-56qc-22m7: high severity; pip package Gitpython; affected range <= 3.2.0. No patched version or CVE is currently assigned. The advisory is a draft; this description omits its exploit details.

Git reference

Inspected Git at d38352cd43ab9745686d697872408bc3249a153f, including read-cache.c::verify_path_internal, NTFS/HFS path recognition, compat/mingw.c::is_valid_win32_path, and submodule.c::validate_submodule_git_dir. Git's t7450-bad-git-dotfiles.sh covers nested metadata; t7406-submodule-update.sh covers checkout symlinks. Regression tests also compare metadata aliases with native Git index validation using both filesystem protections.

Validation

Regression tests first reproduced writes before rejection and acceptance of nested metadata. Coverage includes all 16 HFS ignored characters, NTFS aliases, Windows reserved names and invalid characters, separate Git directories, relative and absolute destinations, reuse after deinitialization, moves and renames, and metadata nesting introduced during cloning. The fixtures use harmless file content.

• Final CI on 69d4702d: 50/50 checks passed across Linux, macOS, Windows (including free-threaded Python builds), Alpine, Cygwin, dependencies, and lint.
• Initial fix, pytest test/test_submodule.py test/test_submodule_no_fetch.py: 445 passed, 3 skipped, 1 expected failure on macOS/Python 3.14, with a temporary Git configuration using the suite's expected master default branch.
• The Windows CI follow-up adds two regressions that simulate native path normalization on every platform; both failed before the fix. Focused Windows destination, relative-path, and move tests then passed: 24 passed, 2 skipped. Windows-platform Mypy, Ruff lint and formatting, and whitespace checks also passed.
• Repository-wide Ruff lint and formatting passed.
• Mypy passed for both the native and win32 platforms (46 source files); basedpyright passed without errors or warnings.

The required Codex commit review was attempted once for faf32c2b, but the CLI rejected its configured gpt-6.1-sol model with this ChatGPT account before reviewing code. Automated commit review is therefore unavailable for this commit.

The follow-up commit 69d4702d passed its single Codex review using the supported gpt-5.5 model, with no findings.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 2 · 1

Open (4)

• LPT[0-9] will incorrectly reject LPT0, which is not a Win32 reserved device name (reserved are… · New
• os.path.samefile(path, directory) can raise OSError/FileNotFoundError/PermissionError… · New
• This parametrized test does two full rglob("*") scans per case (and the parametrization is fairly… · New
• The new changelog entry mixes prose and bare URLs without consistent reStructuredText structure,… · New

What changed in this PR

This PR hardens submodule handling by validating checkout and metadata destinations before any filesystem mutation, aligning behavior more closely with Git’s path/index validation to prevent unsafe paths and nested metadata issues.

Changes:

• Add comprehensive regression tests ensuring unsafe paths are rejected without side effects (no clone attempts / no filesystem writes).
• Enforce additional path validation in submodule name/path handling, including Windows-specific restrictions and Git metadata alias detection.
• Document the security fix in the changelog for the 3.2.1 release.

File	Description
test/​test_submodule.py	Adds regression coverage for rejecting unsafe checkout/metadata destinations before mutation across multiple operations.
git/​objects/​submodule/​base.py	Adds Windows path validation, .git-alias checks via filesystem identity, and nested-metadata rejection logic in clone/move/rename/module flows.
doc/​source/​changes.rst	Adds a 3.2.1 changelog entry referencing the security advisory and release notes.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Windows LPT0 validation and release version metadata are inconsistent with the added tests and changelog.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Align VERSION with the 3.2.1 changelog heading · New
• Reject LPT0 as a reserved Windows path · New

Resolved since last review (4)

• LPT[0-9] will incorrectly reject LPT0, which is not a Win32 reserved device name (reserved are…
• This parametrized test does two full rglob("*") scans per case (and the parametrization is fairly…
• os.path.samefile(path, directory) can raise OSError/FileNotFoundError/PermissionError…
• The new changelog entry mixes prose and bare URLs without consistent reStructuredText structure,…

[Copilot]

Copilot review overview

🟡 Changes recommended

Metadata-alias and post-clone exception paths remain unsafe, and the release version is inconsistent.

Review effort: Balanced
Findings: 2 · 2

Open (4)

• Check metadata identity before allowing final symlink · New
• Align VERSION with the 3.2.1 changelog heading
• Catch InvalidGitRepositoryError for linked-worktree race · New
• Reject LPT0 as a reserved Windows path

[Copilot]

Copilot review overview

🔵 Needs a closer look

Windows validation still permits LPT0, contrary to the referenced Git implementation.

Review effort: Balanced
Findings: None

Resolved since last review (4)

• Check metadata identity before allowing final symlink
• Align VERSION with the 3.2.1 changelog heading
• Catch InvalidGitRepositoryError for linked-worktree race
• Reject LPT0 as a reserved Windows path