Repository navigation
fix(submodule): bind writes to the validated checkout - #2335
Merged
Merged
Conversation
Rubber-stamp, it's certainly good to have this test as it seems to be a real issue compared to Git, trusting everything Astra discovered. In any case, it seems like GIX-14 should be fixed sooner than later. <!-- agent --> The requested audit against Git's submodule path validation found that `Submodule.module()` checked the checkout path, then let `Repo` select a different working tree from `core.worktree`. A symlink in that configured path could redirect `update()` writes outside the checkout. Unforced `remove(configuration=False)` could delete the external working tree. Pass the checked checkout as `GIT_WORK_TREE` when opening the shared module handle. This keeps updates, removal and recursive traversal on the validated path without making repository queries rewrite configuration. Existing Git metadata symlinks remain supported. Git reference `6de20f6092dcf9bdb1c8efe03db4b70c82b423dd` validates checkout components in `submodule.c:validate_submodule_path()` and repairs configured worktrees before update in `builtin/submodule--helper.c:ensure_core_worktree()`. Git 2.54.0 preserves external targets in the new update/deinit comparisons. GitPython previously overwrote or deleted them. The regressions cover leaf and intermediate symlinks, and pass on both CLI and Gix backends after the fix. Record the independently observed GixPython 0.3.0 configured-worktree canonicalization difference under the still-open `GIX-14` ledger entry. The scoped submodule guard does not claim to fix native discovery upstream. Assisted-by: GPT 6.0 Astra Co-authored-by: GPT 6.0 Astra <codex@openai.com>
<!-- agent --> `SECURITY.md` described supported versions and vulnerability reporting without defining which repository inputs are trusted. This left the distinction between untrusted repository content and locally supplied administration data unclear. Document that both GitPython backends inherit Git's threat model directly. Summarize the trust placed in local configuration and hooks, the limits of ownership checks and filesystem confinement, and Git's guidance for obtaining a clean copy of an untrusted repository. Link to Git's documentation on GitHub and keep violations of its intended security boundaries reportable, including bugs that also affect Git. Checked the summary against the local Git reference at `6de20f6092dcf9bdb1c8efe03db4b70c82b423dd`. `git diff --check` passes. No runtime tests were run because this change only updates documentation. Assisted-by: GPT 6.0 Astra Co-authored-by: GPT 6.0 Astra <codex@openai.com>
Byron
force-pushed
the
validate-submodule-path
branch
from
November 3, 2026 07:36
9d74656 to
8439407
Compare
Byron
marked this pull request as ready for review
November 3, 2026 07:40
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the change that confines submodule operations to the validated checkout.
No open issues 🎉
What changed in this PR
This PR binds submodule repository handles to the validated checkout using GIT_WORK_TREE, preventing configured core.worktree paths from redirecting writes through symlinks without rewriting repository configuration.
Review by GitHub Copilot.
Changes:
- Confine submodule operations to the validated checkout.
- Add eight regression cases comparing Git and GitPython update/removal behavior for leaf and intermediate symlinks.
- Document Git-aligned trust boundaries and the remaining Gix worktree-resolution difference.
| File | Description |
|---|---|
tests/fixtures/submodule-symlinks.sh |
Set up redirected worktrees for update and removal tests. |
test/test_submodule.py |
Verify operations preserve external targets and affect the intended checkout. |
SECURITY.md |
Explain repository trust boundaries and security reporting scope. |
git/objects/submodule/base.py |
Bind module handles to the validated checkout with GIT_WORK_TREE. |
doc/gix-backend.md |
Record configured-worktree symlink resolution under open issue GIX-14. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by Codex (exact runtime model unavailable).
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Submodule.module()validated the configured checkout path, then opened aRepowhosecore.worktreecould select a different directory through a symlink. Updating could overwrite files there; unforcedremove(configuration=False)could delete that directory. This bug was discovered during the requested audit of GitPython's submodule writes against Git's existing symlink protection.Bind the shared module handle to the validated checkout with
GIT_WORK_TREE. Updates, removal, recursive traversal, and callers ofmodule()then use that checked path. Opening the handle remains read-only. Git's own update repairs persistedcore.worktree; this fix confines the GitPython handle without rewriting configuration during a query.The audit traced add, clone, reconnect, update, move, remove, deinit, and root/recursive update paths. Their existing checkout checks or Git delegation remain in place. Git-supported metadata symlinks remain supported. A separate native configured-worktree canonicalization difference is recorded under the still-open GIX-14 ledger entry.
Validation
test/test_submodule.pyandtest/test_submodule_no_fetch.py: CLI had 484 passes, Gix had 485 passes, with their existing skips/xfail. Each initially had one failure because the sandbox could not access the user's GPG agent for an existing test's commit. That case passes on both backends withcommit.gpgSign=falseinjected only into the test process; no persistent signing configuration changed.mypy,basedpyright --warnings, Ruff check/format, ShellCheck with sourced fixtures, andgit diff --checkpass.codex review --commit 9d74656found no actionable regression.6de20f6092dcf9bdb1c8efe03db4b70c82b423ddsuppliessubmodule.c:validate_submodule_path()andbuiltin/submodule--helper.c:ensure_core_worktree().Path validation remains a point-in-time check, as in Git; this change does not provide atomic protection against concurrent filesystem replacement.
Reported issue (user-authored, verbatim)
Follow-up constraints (user-authored, verbatim)