Repository navigation
fix(submodule): resolve relative submodule URLs the way git does - #2289
Merged
Byron merged 2 commits intoOct 17, 2026
Merged
Conversation
`Submodule.update` resolved a `.gitmodules` URL starting with `./` or `../` with `urllib.parse.urljoin` against the superproject's `origin` URL, and `Submodule._clone_repo` appended a `../` URL to the tracking remote's URL with `os.path.join`. Neither follows git's `relative_url`, and `urljoin` only resolves against the schemes it knows: for an `ssh://` or `git://` origin it returns the relative URL unchanged, and for an scp-like `host:path` origin more than one `../` collapses to a bare path. `git clone` then received `./planted`, a path inside the superproject's checkout, where git resolves the same entry to `ssh://host/org/super.git/planted`. A submodule was therefore cloned from a directory committed by whoever authored the superproject, through git's local clone path: `git upload-pack` ran inside that directory and its objects were copied without the `transfer.fsckObjects` checks a transport fetch applies (an object that `git clone file://...` rejects as `badDate` is accepted this way). Relative submodule URLs are common in multi-repository setups, and the superproject is untrusted input to any service that updates submodules of repositories it is given. Add `resolve_relative_url`, a port of git's `relative_url`: each leading `../` strips one component of the remote URL, a path component first, then the scp-like host before the colon, and running out of components is an error; `./` prefixes are dropped and the rest is appended. It agrees with what `git submodule init` records over 272 remote and URL pairs, including the error cases. Use it in `Submodule.update`, `Submodule._clone_repo` (which now also resolves `./` URLs, previously passed to `git clone` verbatim), and the URL change path of `RootModule.update`, which created a remote from the raw relative URL and fetched it as a path relative to the module's working tree. The remote used for resolution is unchanged: `origin` in `update`, the tracking branch's remote in `_clone_repo`. Adds regression tests in `test/test_submodule.py`; the two behavioral ones fail before the change with `./planted` reaching the clone unchanged.
Byron
pushed a commit
to GitoxideLabs/gitoxide
that referenced
this pull request
Oct 16, 2026
Relative submodule URLs in `.gitmodules` refer to the superproject's upstream, even when that upstream uses `ssh://`, `git://`, or SCP syntax. Passing the existing `Submodule::url()` result directly to a clone treats `./` and `../` entries as local paths instead. GitPython's corresponding fix prompted this investigation: gitpython-developers/GitPython#2289 Document `url()` as the raw configured accessor and add `resolved_url()` for cloning or fetching. Port Git's byte-oriented `relative_url()` rules from `remote.c`, select the current branch's remote or the default remote, and fall back to the superproject itself when the selected URL is missing. Make local paths absolute so the result does not depend on the caller's working directory. Honor configuration overrides as initialized URLs, avoiding a second remote-relative resolution of local URLs written by `git submodule init`. Validate transport and local URLs, traversal errors, default-remote selection, initialized overrides, unborn and bare repositories, and non-UTF-8 local paths in disposable repositories. Compare resolution with `git submodule init`; include native-separator coverage for Windows. Validation: 34 submodule tests pass, the four new tests pass with SHA-256 fixtures, focused Clippy passes with warnings denied, and the `sha256,attributes` build passes. Documentation builds; strict rustdoc checking encounters the existing broken `Self::with_fetch_options` link in `gix/src/clone/access.rs`.
Byron
force-pushed
the
submodule-relative-url
branch
from
October 17, 2026 02:27
a0af02f to
0aa77e4
Compare
Byron
force-pushed
the
submodule-relative-url
branch
from
October 17, 2026 02:56
0aa77e4 to
ee89568
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved URL and remote-selection errors can select unintended local repositories or block valid submodule operations.
4 open issues
What changed in this PR
GitHub Copilot: This PR centralizes relative submodule URL resolution to match Git and prevent unintended checkout-local cloning.
Changes:
- Adds Git-style URL resolution.
- Applies resolution to cloning and submodule remote changes.
- Adds regression tests for remote selection and relative URLs.
| File | Description |
|---|---|
| test/test_submodule.py | Tests URL resolution and submodule updates. |
| git/objects/submodule/util.py | Adds relative URL detection and resolution. |
| git/objects/submodule/root.py | Resolves URLs when changing submodule remotes. |
| git/objects/submodule/base.py | Centralizes URL resolution and remote selection. |
Review effort: Balanced
Byron
force-pushed
the
submodule-relative-url
branch
from
October 17, 2026 03:41
ee89568 to
580fd90
Compare
It's basically a rubber-stamp of the review comments, hoping this code will be nothing more than interesting tests for the `gix-backend` branch, so the implementation here won't matter. <!-- agent --> - [P2] Fall back when a relative URL has no tracking remote — git/objects/submodule/base.py:432-435 For a locally initialized superproject without a tracking branch, `create_submodule(..., url='./source')` now enters this block and dereferences `None`, raising `AttributeError` even when `./source` is a valid repository. This addition succeeded before the patch; detached checkouts also fail at `active_branch`. Handle missing tracking branches by falling back to `origin` or the superproject directory, as Git does. - [P2] Resolve relative submodule URLs only once — git/objects/submodule/base.py:986-988 When `origin` is a local relative URL such as `../repos/super.git` and the branch tracks that remote, `_resolved_url()` turns `../child.git` into `../repos/child.git`. Because that result still starts with `../`, `_clone_repo()` resolves it again, producing `../repos/repos/child.git`. This can fail cloning or select the wrong repository. Ensure URLs already resolved by `update()` bypass the second resolution. - [P2] Defer old-URL resolution until a remote change is needed — git/objects/submodule/root.py:269-270 For an initialized superproject without `origin`, changing `.gitmodules` from `../source` to the absolute URL already used by the submodule's remote now raises `AttributeError` in `psm._resolved_url()`. Previously, the existing-remote check skipped URL handling and the update completed. Resolve the previous URL only after checking whether the new URL already matches a remote, so updates requiring no remote adjustment do not fail.
Byron
force-pushed
the
submodule-relative-url
branch
from
October 17, 2026 04:22
580fd90 to
0a5938d
Compare
Byron
pushed a commit
to GitoxideLabs/gitoxide
that referenced
this pull request
Oct 17, 2026
Relative submodule URLs in `.gitmodules` refer to the superproject's upstream, even when that upstream uses `ssh://`, `git://`, or SCP syntax. Passing the existing `Submodule::url()` result directly to a clone treats `./` and `../` entries as local paths instead. GitPython's corresponding fix prompted this investigation: gitpython-developers/GitPython#2289 Document `url()` as the raw configured accessor and add `resolved_url()` for cloning or fetching. Port Git's byte-oriented `relative_url()` rules from `remote.c`, select the current branch's remote or the default remote, and fall back to the superproject itself when the selected URL is missing. Make local paths absolute so the result does not depend on the caller's working directory. Honor configuration overrides as initialized URLs, avoiding a second remote-relative resolution of local URLs written by `git submodule init`. Validate transport and local URLs, traversal errors, default-remote selection, initialized overrides, unborn and bare repositories, and non-UTF-8 local paths in disposable repositories. Compare resolution with `git submodule init`; include native-separator coverage for Windows. Validation: 34 submodule tests pass, the four new tests pass with SHA-256 fixtures, focused Clippy passes with warnings denied, and the `sha256,attributes` build passes. Documentation builds; strict rustdoc checking encounters the existing broken `Self::with_fetch_options` link in `gix/src/clone/access.rs`. CI follow-up: `test-fast` and `test-fixtures-windows` on Windows rejected an own-upstream fallback containing a verbatim drive prefix as SCP syntax, and compared Git's short temporary path with a different canonical form. Keep this known fallback as a local path instead of parsing it as a URL; normalize native separators before filesystem resolution. Use the fixture's resolved path consistently in comparisons, and add Windows regression coverage opening the superproject through its verbatim canonical path. `test-32bit (arm32v7)` uses a Git version older than 2.51, before `submodule--helper` learned to fall back to the only remote when detached or without a branch remote. Assert the intended result directly and compare these cases with Git only on compatible versions. Remove the fixture's inherited `branch.main` settings before configuring each case so its old `origin` cannot silently override the remote under test. The URL tests pass locally with SHA-1 and SHA-256 fixtures, and focused Clippy and formatting pass. Windows and ARM execution are delegated to CI.
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.
Submodule.updateresolves a./or../submodule URL withurllib.parse.urljoinagainst theoriginURL, andSubmodule._clone_repojoins a../URL onto the tracking remote's URL withos.path.join.urljoinonly resolves against the schemes it knows, so for anssh://orgit://superproject the relative URL comes back unchanged andgit cloneis handed./planted, a path inside the superproject's checkout; with an scp-likehost:pathorigin,../../plantedcollapses to the bare pathplanted. For the same.gitmodulesentry,git submodule initrecordsssh://host/org/super.git/planted:The superproject's author controls both
.gitmodulesand the tree, so a committed directory laid out as a bare repository becomes the submodule's source. That is git's local clone path:git upload-packruns inside the attacker's directory, and the objects are copied without thetransfer.fsckObjectschecks a transport fetch applies (a commit thatgit clone file://...refuses asbadDateis accepted). Relative URLs are common in multi-repository setups, and anything that updates submodules of repositories it is given treats the superproject as untrusted.resolve_relative_urlingit/objects/submodule/util.pyis a port of git'srelative_url: each../strips one component of the remote URL, a path component first and then the scp-like host before the colon, running out of components is an error,./is dropped and the remainder appended. Checked against whatgit submodule initwrites tosubmodule.<name>.urlon git 2.54 for 272 remote and URL pairs (ssh://,host:path,https://,file://,//share, absolute and relative paths, trailing slashes,.sub,..//sub), with no differences including the error cases. It replacesurljoininSubmodule.update,os.path.joinin_clone_repo(which now also resolves./, previously passed togit cloneverbatim), and the raw relative URL that the URL change path ofRootModule.updategavecreate_remoteandset_url. The remote each site resolves against is unchanged. The two behavioral tests intest/test_submodule.pyfail before the change with./plantedreaching the clone unchanged;test_submodule.pypasses, andruff,mypyandbasedpyright --warningsreport nothing new.