v5: config: validate submodule names#2082
Merged
pjbgf merged 4 commits intoMay 11, 2026
Merged
Conversation
`Submodule.Validate()` validated `Path` against `dotdotPath` but left `Name` unchecked, so a `.gitmodules` declaring `[submodule ".."]` passed validation cleanly. Downstream the same name reached `storage/filesystem/dotgit.DotGit.Module(name)` where it was joined into the storage path with no containment check, letting the submodule storer escape `.git/modules/` and write into the parent repo's `.git/`. Mirror canonical Git's `check_submodule_name` from `submodule-config.c` [1], rejecting empty names and any name with a `..` path component using both `/` and `\\` as separators (the canonical loop uses both unconditionally so the rule stays platform-consistent). On top of that, reject names that are bare `.`, contain a NUL byte, start or end with a separator, or carry a Windows drive prefix — go-git-specific edge cases the canonical loop does not exercise because it treats names as opaque C strings. The path-component check is also extended for filesystems whose canonicalisation can turn an innocent-looking name into `..`: HFS+ ignores a fixed set of Unicode code points (zero-width and directional marks) so `.<U+200C>.` resolves to `..` on macOS, and NTFS strips trailing spaces, periods, and an alternate-data-stream suffix so `.. `, `....`, and `..::$INDEX_ALLOCATION` all resolve to `..` on Windows. Both variants are detected by calling `pathutil.IsHFSDot` and `pathutil.IsNTFSDot` with `.` as the needle — they already port the relevant fragments of canonical Git's `next_hfs_char` and `is_ntfs_dotgit` — and run unconditionally on every submodule name since `.gitmodules` is attacker-controlled by definition. Wrap the new `ErrModuleBadName` sentinel (whose message echoes canonical Git's `ignoring suspicious submodule name` warning) with the offending name in `Validate()` so callers can both `errors.Is` the sentinel and read the bad name in the error string. [1]: https://github.com/git/git/blob/v2.54.0/submodule-config.c#L214-L237 Backport of go-git#2079. Assisted-by: Claude Opus 4.7 Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Extend `unmarshalSubmodules`'s silent-skip to also drop entries whose `Name` fails `Submodule.Validate()`. The previous check only matched `ErrModuleBadPath`; with the new `ErrModuleBadName` validation landed in the preceding commit, a `.gitmodules` declaring `[submodule ".."]` would still be inserted into the map keyed by `..` and reach `Submodule.Repository()` callers downstream. Mirror canonical Git's "ignoring suspicious submodule name" semantics by treating bad-name entries the same way bad-path entries are already treated: drop them without failing the parse, so a malformed `.gitmodules` does not break unrelated config loading. Empty-path and empty-URL entries keep their existing lenient behaviour of being stored despite the validation error, preserving backward compatibility with `.gitmodules` files in the wild. Backport of go-git#2079. Assisted-by: Claude Opus 4.7 Signed-off-by: Hidde Beydals <hidde@hhh.computer>
`DotGit.Module(name)` joined the raw `name` into
`modulePath = "modules"` and chrooted to the result without any
containment check. A `Submodule` whose `Name` is `..` (or a sibling
traversal like `foo/../..`) caused `Chroot("modules/..")` to resolve
back to `.git/`, so once `Submodule.Repository()` opened that storer
any subsequent `Storer.SetReference` / `SetEncodedObject` call wrote
into the parent repo's `.git/`. The parser-layer fix in the
preceding commit already rejects such names from `.gitmodules`,
but `Module` is also reachable from any caller that constructs a
`Submodule` struct programmatically — including the transactional
storage wrapper — so a defence in depth is warranted.
After joining, clean the result with `path.Clean(filepath.ToSlash)`
and refuse the call unless the cleaned path is exactly `modules` or
remains under `modules/`. This catches `..`-traversal escapes
without rejecting nested but contained names like `lib/foo`, which
canonical Git permits.
Names whose joined form `path.Clean`s back into the modules/
subtree (for example `/etc`, which becomes `modules/etc`, or
`modules/../escape`, which becomes `modules/escape`) are accepted
here — the parser layer is responsible for the shape of the name.
The storage layer's only job is to ensure no caller can write
outside `modules/`.
Backport of go-git#2079.
Assisted-by: Claude Opus 4.7
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Cover the storage-layer defence: constructing a `Submodule` with `Name = ".."` programmatically (bypassing the .gitmodules parser) must not result in `Repository()` opening a storer rooted in the parent's `.git/` directory, and must leave the parent's HEAD reference untouched. Use `filesystem.NewStorage` rather than `memory.NewStorage` for the parent because only the filesystem-backed `ModuleStorer` exercises the dotgit defence; the in-memory implementation maps names to storage units directly without traversing a path. The test asserts the call returns an error wrapping `dotgit.ErrModuleNameEscape` and that the parent's HEAD target is unchanged after the failed call. Backport of go-git#2079. Assisted-by: Claude Opus 4.7 Signed-off-by: Hidde Beydals <hidde@hhh.computer>
6e1d942 to
5042dca
Compare
chhe
pushed a commit
to chhe/act_runner
that referenced
this pull request
May 19, 2026
This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [github.com/go-git/go-git/v5](https://github.com/go-git/go-git) | `v5.19.0` → `v5.19.1` |  |  | --- ### Release Notes <details> <summary>go-git/go-git (github.com/go-git/go-git/v5)</summary> ### [`v5.19.1`](https://github.com/go-git/go-git/releases/tag/v5.19.1) [Compare Source](go-git/go-git@v5.19.0...v5.19.1) #### What's Changed - v5: plumbing: transport/ssh, Shell-quote path by [@​hiddeco](https://github.com/hiddeco) in [#​2068](go-git/go-git#2068) - v5: git: submodule, Fix relative URL resolution by [@​hiddeco](https://github.com/hiddeco) in [#​2070](go-git/go-git#2070) - v5: git: submodule, canonical remote for relative URLs by [@​hiddeco](https://github.com/hiddeco) in [#​2074](go-git/go-git#2074) - v5: git: submodule, error on remote without URLs by [@​hiddeco](https://github.com/hiddeco) in [#​2078](go-git/go-git#2078) - v5: plumbing: format/idxfile, Validate offset64 indices by [@​hiddeco](https://github.com/hiddeco) in [#​2084](go-git/go-git#2084) - v5: \*: Reject malformed variable-length integers by [@​hiddeco](https://github.com/hiddeco) in [#​2092](go-git/go-git#2092) - v5: plumbing: format/packfile, Tighten delta validation by [@​hiddeco](https://github.com/hiddeco) in [#​2091](go-git/go-git#2091) - v5: Add `worktreeFilesystem` wrapper for worktree and hardening by [@​hiddeco](https://github.com/hiddeco) in [#​2100](go-git/go-git#2100) - v5: config: validate submodule names by [@​hiddeco](https://github.com/hiddeco) in [#​2082](go-git/go-git#2082) - build: Update module github.com/go-git/go-git/v5 to v5.19.0 \[SECURITY] (releases/v5.x) by [@​go-git-renovate](https://github.com/go-git-renovate)\[bot] in [#​2111](go-git/go-git#2111) - v5: git: Allow MkdirAll on worktree-root paths by [@​hiddeco](https://github.com/hiddeco) in [#​2117](go-git/go-git#2117) - v5: git: Stop validating symlink target paths by [@​pjbgf](https://github.com/pjbgf) in [#​2116](go-git/go-git#2116) - v5: plumbing: format decoder input bounds and contracts by [@​hiddeco](https://github.com/hiddeco) in [#​2125](go-git/go-git#2125) - plumbing: format/packfile, cap delta chain depth in parser by [@​pjbgf](https://github.com/pjbgf) in [#​2137](go-git/go-git#2137) **Full Changelog**: <go-git/go-git@v5.19.0...v5.19.1> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xODIuMSIsInVwZGF0ZWRJblZlciI6IjQzLjE4Mi4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://gitea.com/gitea/runner/pulls/980 Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Renovate Bot <renovate-bot@gitea.com> Co-committed-by: Renovate Bot <renovate-bot@gitea.com>
Maks1mS
pushed a commit
to stplr-dev/stplr
that referenced
this pull request
May 20, 2026
This PR contains the following updates: | Package | Type | Update | Change | OpenSSF | |---|---|---|---|---| | [github.com/go-git/go-git/v5](https://github.com/go-git/go-git) | require | patch | `v5.19.0` → `v5.19.1` | [](https://securityscorecards.dev/viewer/?uri=github.com/go-git/go-git) | --- >⚠️ **Warning** > > Some dependencies could not be looked up. Check the [Dependency Dashboard](issues/23) for more information. --- ### Release Notes <details> <summary>go-git/go-git (github.com/go-git/go-git/v5)</summary> ### [`v5.19.1`](https://github.com/go-git/go-git/releases/tag/v5.19.1) [Compare Source](go-git/go-git@v5.19.0...v5.19.1) #### What's Changed - v5: plumbing: transport/ssh, Shell-quote path by [@​hiddeco](https://github.com/hiddeco) in [#​2068](go-git/go-git#2068) - v5: git: submodule, Fix relative URL resolution by [@​hiddeco](https://github.com/hiddeco) in [#​2070](go-git/go-git#2070) - v5: git: submodule, canonical remote for relative URLs by [@​hiddeco](https://github.com/hiddeco) in [#​2074](go-git/go-git#2074) - v5: git: submodule, error on remote without URLs by [@​hiddeco](https://github.com/hiddeco) in [#​2078](go-git/go-git#2078) - v5: plumbing: format/idxfile, Validate offset64 indices by [@​hiddeco](https://github.com/hiddeco) in [#​2084](go-git/go-git#2084) - v5: \*: Reject malformed variable-length integers by [@​hiddeco](https://github.com/hiddeco) in [#​2092](go-git/go-git#2092) - v5: plumbing: format/packfile, Tighten delta validation by [@​hiddeco](https://github.com/hiddeco) in [#​2091](go-git/go-git#2091) - v5: Add `worktreeFilesystem` wrapper for worktree and hardening by [@​hiddeco](https://github.com/hiddeco) in [#​2100](go-git/go-git#2100) - v5: config: validate submodule names by [@​hiddeco](https://github.com/hiddeco) in [#​2082](go-git/go-git#2082) - build: Update module github.com/go-git/go-git/v5 to v5.19.0 \[SECURITY] (releases/v5.x) by [@​go-git-renovate](https://github.com/go-git-renovate)\[bot] in [#​2111](go-git/go-git#2111) - v5: git: Allow MkdirAll on worktree-root paths by [@​hiddeco](https://github.com/hiddeco) in [#​2117](go-git/go-git#2117) - v5: git: Stop validating symlink target paths by [@​pjbgf](https://github.com/pjbgf) in [#​2116](go-git/go-git#2116) - v5: plumbing: format decoder input bounds and contracts by [@​hiddeco](https://github.com/hiddeco) in [#​2125](go-git/go-git#2125) - plumbing: format/packfile, cap delta chain depth in parser by [@​pjbgf](https://github.com/pjbgf) in [#​2137](go-git/go-git#2137) **Full Changelog**: <go-git/go-git@v5.19.0...v5.19.1> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At 12:00 AM through 04:59 AM and 10:00 PM through 11:59 PM, Monday through Friday (`* 0-4,22-23 * * 1-5`) - Only on Sunday and Saturday (`* * * * 0,6`) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xNzAuMjIiLCJ1cGRhdGVkSW5WZXIiOiI0My4xNzAuMjIiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbIktpbmQvRGVwZW5kZW5jaWVzIl19--> Reviewed-on: https://altlinux.space/stapler/stplr/pulls/435
12 tasks
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
2 participants
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.Validate()validatedPathagainstdotdotPathbut leftNameunchecked, so submodule names with unusual shapes (empty, bare./.., names containing..components, NUL bytes, or leading or trailing separators) were accepted at parse time and propagated toDotGit.Module(name), which joined them into the on-disk modules path without verifying the result stayed undermodules/.The parser now mirrors canonical Git's
check_submodule_namefromsubmodule-config.c1, extended with a few defensive checks for go-git-specific edge cases. The storage layer adds an independent path-containment check inDotGit.Module(name), so any caller that constructs aSubmoduleprogrammatically — including the transactional storage wrapper — is also protected, and a regression test exercises the storage-layer defence end-to-end. Nested but contained names likelib/foocontinue to work; only path-traversal shapes are refused.Backport of #2079.