Repository navigation
[v5] git: worktree, make the filesystem wrapper a symlink-safe boundary - #2277
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens worktree filesystem operations against symlink-based path escapes by enforcing a “no leading symlink traversal” invariant at the worktreeFilesystem boundary, and by explicitly removing blocking symlinks at checkout materialisation time.
Changes:
- Add leading-component symlink checks for both read and write operations in
worktreeFilesystem(plus an extra final-component check forChroot). - Ensure checkout materialisation removes any blocking symlinks before writing tracked entries.
- Add regression tests covering leading-symlink traversal rejection and forced-checkout symlink replacement behavior.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| worktree.go | Adds clearBlockingSymlinks and uses it to ensure checkout writes don’t follow/overwrite through planted symlinks. |
| worktree_fs.go | Introduces validWritePath / validNoLeadingSymlink and applies read/write boundary validation; tightens Chroot semantics. |
| worktree_fs_test.go | Adds tests to pin both boundary behavior (reject traversal) and checkout behavior (replace blocking symlinks). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
worktreeFilesystem validated path strings but did not account for symlinks already present on disk. As a result a worktree operation on an otherwise valid path could resolve through a symlinked component and act outside the worktree. Enforce the invariant at the boundary. validNoLeadingSymlink rejects paths whose leading directories resolve through an existing symlink, and it runs on every operation. Mutating operations go through validWritePath (Create, OpenFile, MkdirAll, Symlink, Rename, Remove) and reads through validReadPath (Open, Stat, ReadDir, Lstat, Readlink, Chroot). This also covers Worktree.Remove, RemoveGlob and rmFileAndDirsIfEmpty, which delete through the wrapper. Chroot additionally refuses a symlink as the final component so a sub-filesystem such as a submodule worktree cannot be scoped to a redirected target. The wrapper stops at leading-component traversal. Final-component no-follow during materialisation is enforced at the call site. checkoutFile removes a blocking symlink, whether in a leading component or at the final path, before writing. A forced checkout therefore replaces it with a real entry and succeeds, matching upstream create_directories and its unlink-before-write behaviour. Callers that do not remove it first get a safe error at the boundary. Mirrors upstream Git's has_symlink_leading_path in symlinks.c and the check_leading_path and unlink-before-write guards in entry.c. Assisted-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io>
hiddeco
approved these changes
Jul 29, 2026
rjosephwright
added a commit
to cloudboss/unobin
that referenced
this pull request
Aug 10, 2026
Includes fix go-git/go-git#2277.
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.
worktreeFilesystemvalidated path strings but did not account for symlinks already present on disk. As a result a worktree operation on an otherwise valid path could resolve through a symlinked component and act outside the worktree.Enforce the invariant at the boundary.
validNoLeadingSymlinkrejects paths whose leading directories resolve through an existing symlink, and it runs on every operation. Mutating operations go throughvalidWritePath(Create, OpenFile, MkdirAll, Symlink, Rename, Remove) and reads throughvalidReadPath(Open, Stat, ReadDir, Lstat, Readlink, Chroot). This also coversWorktree.Remove,RemoveGlobandrmFileAndDirsIfEmpty, which delete through the wrapper. Chroot additionally refuses a symlink as the final component so a sub-filesystem such as a submodule worktree cannot be scoped to a redirected target.The wrapper stops at leading-component traversal. Final-component no-follow during materialisation is enforced at the call site.
checkoutFileremoves a blocking symlink, whether in a leading component or at the final path, before writing. A forced checkout therefore replaces it with a real entry and succeeds, matching upstream create_directories and its unlink-before-write behaviour. Callers that do not remove it first get a safe error at the boundary.Mirrors upstream Git's
has_symlink_leading_pathinsymlinks.cand thecheck_leading_pathandunlink-before-writeguards inentry.c.Back-ports #2276.