Repository navigation
fix(submodule): resolve relative submodule URLs the way git does - #2289
Merged
Byron merged 2 commits intoOct 17, 2026
Merged
Byron merged 2 commits into
Byron merged 2 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
`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
a0af02f to
0aa77e4
Compare
October 17, 2026 02:27
Byron
force-pushed
the
submodule-relative-url
branch
from
0aa77e4 to
ee89568
Compare
October 17, 2026 02:56
Copy link
Copy Markdown
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 issuesSlash-containing remote names are truncated during lookup · New
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
Sorry, something went wrong.
Comment thread
git/objects/submodule/base.py
Outdated
Comment thread
git/objects/submodule/base.py
Outdated
Comment thread
git/objects/submodule/root.py
Outdated
Comment thread
git/objects/submodule/util.py
Byron
force-pushed
the
submodule-relative-url
branch
from
ee89568 to
580fd90
Compare
October 17, 2026 03:41
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
580fd90 to
0a5938d
Compare
October 17, 2026 04:22
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.
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. The next Windows CI run confirmed the verbatim-path fix and exposed two remaining equality failures: Git expanded `RUNNER~1` to `runneradmin` in local origins while Rust retained the short name. Compare local URLs after canonicalizing their existing parent directory, retaining the destination name and all other URL fields. The destination itself may not exist before cloning. SHA-1/SHA-256 URL tests and focused Clippy pass locally again.
Copy link
Copy Markdown
Contributor
Author
|
Claude on behalf of Keerthana-64: thanks for merging this, and for the follow-up in 0a5938d covering the slash-named remote and working-dir anchoring cases. |
Sorry, something went wrong.
Byron
pushed a commit
to GitoxideLabs/gitoxide
that referenced
this pull request
Oct 18, 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. The next Windows CI run confirmed the verbatim-path fix and exposed two remaining equality failures: Git expanded `RUNNER~1` to `runneradmin` in local origins while Rust retained the short name. Compare local URLs after canonicalizing their existing parent directory, retaining the destination name and all other URL fields. The destination itself may not exist before cloning. SHA-1/SHA-256 URL tests and focused Clippy pass locally again.
Byron
pushed a commit
to GitoxideLabs/gitoxide
that referenced
this pull request
Oct 18, 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. The next Windows CI run confirmed the verbatim-path fix and exposed two remaining equality failures: Git expanded `RUNNER~1` to `runneradmin` in local origins while Rust retained the short name. Compare local URLs after canonicalizing their existing parent directory, retaining the destination name and all other URL fields. The destination itself may not exist before cloning. SHA-1/SHA-256 URL tests and focused Clippy pass locally again.
Byron
pushed a commit
to GitoxideLabs/gitoxide
that referenced
this pull request
Oct 18, 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. The next Windows CI run confirmed the verbatim-path fix and exposed two remaining equality failures: Git expanded `RUNNER~1` to `runneradmin` in local origins while Rust retained the short name. Compare local URLs after canonicalizing their existing parent directory, retaining the destination name and all other URL fields. The destination itself may not exist before cloning. SHA-1/SHA-256 URL tests and focused Clippy pass locally again.
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
Reviewers
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.update resolves a ./ or ../ submodule URL with urllib.parse.urljoin against the origin URL, and Submodule._clone_repo joins a ../ URL onto the tracking remote's URL with os.path.join. urljoin only resolves against the schemes it knows, so for an ssh:// or git:// superproject the relative URL comes back unchanged and git clone is handed ./planted, a path inside the superproject's checkout; with an scp-like host:path origin, ../../planted collapses to the bare path planted. For the same .gitmodules entry, git submodule init records ssh://host/org/super.git/planted:
The superproject's author controls both .gitmodules and 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-pack runs inside the attacker's directory, and the objects are copied without the transfer.fsckObjects checks a transport fetch applies (a commit that git clone file://... refuses as badDate is 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_url in git/objects/submodule/util.py is a port of git's relative_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 what git submodule init writes to submodule.<name>.url on 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 replaces urljoin in Submodule.update, os.path.join in _clone_repo (which now also resolves ./, previously passed to git clone verbatim), and the raw relative URL that the URL change path of RootModule.update gave create_remote and set_url. The remote each site resolves against is unchanged. The two behavioral tests in test/test_submodule.py fail before the change with ./planted reaching the clone unchanged; test_submodule.py passes, and ruff, mypy and basedpyright --warnings report nothing new.