Skip to content

Commit c404ef1

Browse files
committed
Harden worktree detection against submodules and bare repos
IsLinkedWorktree now reads the .git gitfile and checks for worktree admin markers (.git/worktrees/ or .bare/worktrees/) instead of just checking whether .git is a file. This prevents submodules (which use .git/modules/) from being misidentified as linked worktrees. MainRepoRoot now supports both .git/worktrees/ and .bare/worktrees/ markers when extracting the main repo root, and handles relative gitdir paths by resolving them against the worktree root. The shared parseGitfile helper normalizes paths with filepath.ToSlash for consistent marker matching. Extract parseGitfile and hasWorktreeMarker into shared helpers used by GetWorktreeID, IsLinkedWorktree, and MainRepoRoot. Fix git worktree add argument order in tests (-b before path). Signed-off-by: Paulo Gomes <paulo@entire.io> Assisted-by: Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: f6d95094dfb9
1 parent ea0d5ff commit c404ef1

3 files changed

Lines changed: 155 additions & 44 deletions

File tree

cmd/entire/cli/paths/paths.go

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"sync"
1212

1313
"github.com/entireio/cli/cmd/entire/cli/checkpoint/id"
14-
"github.com/entireio/cli/cmd/entire/cli/osroot"
1514
)
1615

1716
// Directory constants
@@ -120,27 +119,23 @@ func ClearWorktreeRootCache() {
120119
}
121120

122121
// IsLinkedWorktree returns true if the given path is inside a linked git worktree
123-
// (as opposed to the main repository). Linked worktrees have .git as a file
124-
// pointing to the main repo, while the main repo has .git as a directory.
125-
// Uses os.Root for traversal-resistant access.
122+
// (as opposed to the main repository or a submodule). Linked worktrees have .git
123+
// as a file whose gitdir points into a worktree admin dir (.git/worktrees/ or
124+
// .bare/worktrees/). Submodules also have .git as a file but point into
125+
// .git/modules/, so they are not treated as linked worktrees.
126126
func IsLinkedWorktree(worktreeRoot string) bool {
127-
root, err := os.OpenRoot(worktreeRoot)
128-
if err != nil {
129-
return false
130-
}
131-
defer root.Close()
132-
133-
info, err := root.Stat(".git")
134-
if err != nil {
127+
gitdir, err := parseGitfile(worktreeRoot)
128+
if err != nil || gitdir == "" {
135129
return false
136130
}
137-
return !info.IsDir()
131+
return hasWorktreeMarker(gitdir)
138132
}
139133

140134
// MainRepoRoot returns the root directory of the main repository.
141135
// In the main repo, this returns the same as WorktreeRoot.
142136
// In a linked worktree, this parses the .git file to find the main repo root.
143-
// Uses os.Root for traversal-resistant reads.
137+
// Supports both standard (.git/worktrees/) and bare-repo (.bare/worktrees/) layouts,
138+
// and handles relative gitdir paths.
144139
//
145140
// Per gitrepository-layout(5), a worktree's .git file is a "gitfile" containing
146141
// "gitdir: <path>" pointing to $GIT_DIR/worktrees/<id> in the main repository.
@@ -151,31 +146,33 @@ func MainRepoRoot(ctx context.Context) (string, error) {
151146
return "", fmt.Errorf("failed to get worktree path: %w", err)
152147
}
153148

154-
if !IsLinkedWorktree(worktreeRoot) {
155-
return worktreeRoot, nil
156-
}
157-
158-
root, err := os.OpenRoot(worktreeRoot)
149+
gitdir, err := parseGitfile(worktreeRoot)
159150
if err != nil {
160-
return "", fmt.Errorf("failed to open worktree root: %w", err)
151+
return "", err
161152
}
162-
defer root.Close()
163153

164-
// Worktree .git file contains: "gitdir: /path/to/main/.git/worktrees/<id>"
165-
content, err := osroot.ReadFile(root, ".git")
166-
if err != nil {
167-
return "", fmt.Errorf("failed to read .git file: %w", err)
154+
// Main worktree or non-worktree gitfile (e.g. submodule): return as-is.
155+
if gitdir == "" || !hasWorktreeMarker(gitdir) {
156+
return worktreeRoot, nil
168157
}
169158

170-
gitdir := strings.TrimSpace(string(content))
171-
gitdir = strings.TrimPrefix(gitdir, "gitdir: ")
172-
173-
// Extract main repo root: everything before "/.git/"
174-
idx := strings.LastIndex(gitdir, "/.git/")
175-
if idx < 0 {
176-
return "", fmt.Errorf("unexpected gitdir format: %s", gitdir)
159+
// Extract main repo root: everything before the worktree marker.
160+
for _, marker := range worktreeMarkers {
161+
if idx := strings.LastIndex(gitdir, marker); idx >= 0 {
162+
// marker includes the trailing slash of the git dir (e.g. ".git/worktrees/"),
163+
// so we need to trim the leading dir separator + marker prefix.
164+
// For ".git/worktrees/", the repo root is everything before "/.git/worktrees/".
165+
// The marker without the admin-dir prefix is "/worktrees/", but we want
166+
// the path before the git-dir itself, so find the git-dir boundary.
167+
gitDirSuffix := strings.TrimSuffix(marker, "worktrees/") // ".git/" or ".bare/"
168+
gitDirIdx := strings.LastIndex(gitdir, "/"+gitDirSuffix)
169+
if gitDirIdx >= 0 {
170+
return filepath.FromSlash(gitdir[:gitDirIdx]), nil
171+
}
172+
}
177173
}
178-
return gitdir[:idx], nil
174+
175+
return "", fmt.Errorf("unexpected gitdir format: %s", gitdir)
179176
}
180177

181178
// AbsPath returns the absolute path for a relative path within the repository.

cmd/entire/cli/paths/paths_test.go

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func TestMainRepoRoot_LinkedWorktree(t *testing.T) {
151151
if err := os.MkdirAll(filepath.Dir(worktreeDir), 0o755); err != nil {
152152
t.Fatalf("MkdirAll: %v", err)
153153
}
154-
cmd := exec.Command("git", "worktree", "add", worktreeDir, "-b", "test-branch") //nolint:noctx // test code
154+
cmd := exec.Command("git", "worktree", "add", "-b", "test-branch", worktreeDir) //nolint:noctx // test code
155155
cmd.Dir = tmpDir
156156
cmd.Env = testutil.GitIsolatedEnv()
157157
if output, err := cmd.CombinedOutput(); err != nil {
@@ -170,6 +170,53 @@ func TestMainRepoRoot_LinkedWorktree(t *testing.T) {
170170
}
171171
}
172172

173+
func TestMainRepoRoot_Submodule(t *testing.T) {
174+
// Cannot use t.Parallel: uses t.Chdir
175+
// MainRepoRoot should return the submodule root (not the superproject)
176+
// when running from inside a submodule.
177+
superDir := t.TempDir()
178+
resolved, err := filepath.EvalSymlinks(superDir)
179+
if err != nil {
180+
t.Fatalf("EvalSymlinks: %v", err)
181+
}
182+
superDir = resolved
183+
184+
// Create the "library" repo that will become a submodule
185+
libDir := t.TempDir()
186+
testutil.InitRepo(t, libDir)
187+
testutil.WriteFile(t, libDir, "lib.txt", "lib")
188+
testutil.GitAdd(t, libDir, "lib.txt")
189+
testutil.GitCommit(t, libDir, "lib init")
190+
191+
// Create the superproject
192+
testutil.InitRepo(t, superDir)
193+
testutil.WriteFile(t, superDir, "main.txt", "main")
194+
testutil.GitAdd(t, superDir, "main.txt")
195+
testutil.GitCommit(t, superDir, "super init")
196+
197+
// Add submodule (allow file transport for local clone)
198+
cmd := exec.Command("git", "-c", "protocol.file.allow=always", "submodule", "add", libDir, "libs/mylib") //nolint:noctx // test code
199+
cmd.Dir = superDir
200+
cmd.Env = testutil.GitIsolatedEnv()
201+
if output, err := cmd.CombinedOutput(); err != nil {
202+
t.Fatalf("git submodule add: %v\n%s", err, output)
203+
}
204+
205+
submoduleDir := filepath.Join(superDir, "libs", "mylib")
206+
207+
ClearWorktreeRootCache()
208+
t.Chdir(submoduleDir)
209+
210+
// MainRepoRoot should return the submodule root, not the superproject
211+
root, err := MainRepoRoot(context.Background())
212+
if err != nil {
213+
t.Fatalf("MainRepoRoot() error: %v", err)
214+
}
215+
if root != submoduleDir {
216+
t.Errorf("MainRepoRoot() = %q, want %q (should stay in submodule, not escape to superproject)", root, submoduleDir)
217+
}
218+
}
219+
173220
func TestIsLinkedWorktree(t *testing.T) {
174221
t.Parallel()
175222

@@ -195,6 +242,29 @@ func TestIsLinkedWorktree(t *testing.T) {
195242
}
196243
})
197244

245+
t.Run("bare repo worktree", func(t *testing.T) {
246+
t.Parallel()
247+
dir := t.TempDir()
248+
if err := os.WriteFile(filepath.Join(dir, ".git"), []byte("gitdir: /repo/.bare/worktrees/main\n"), 0o644); err != nil {
249+
t.Fatal(err)
250+
}
251+
if !IsLinkedWorktree(dir) {
252+
t.Error("IsLinkedWorktree() = false for bare repo worktree, want true")
253+
}
254+
})
255+
256+
t.Run("submodule", func(t *testing.T) {
257+
t.Parallel()
258+
dir := t.TempDir()
259+
// Submodules have .git as a file pointing into .git/modules/, not .git/worktrees/
260+
if err := os.WriteFile(filepath.Join(dir, ".git"), []byte("gitdir: /repo/.git/modules/mylib\n"), 0o644); err != nil {
261+
t.Fatal(err)
262+
}
263+
if IsLinkedWorktree(dir) {
264+
t.Error("IsLinkedWorktree() = true for submodule, want false")
265+
}
266+
})
267+
198268
t.Run("no git", func(t *testing.T) {
199269
t.Parallel()
200270
dir := t.TempDir()

cmd/entire/cli/paths/worktree.go

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,24 @@ package paths
33
import (
44
"fmt"
55
"os"
6+
"path/filepath"
67
"strings"
78

89
"github.com/entireio/cli/cmd/entire/cli/osroot"
910
)
1011

11-
// GetWorktreeID returns the internal git worktree identifier for the given path.
12-
// For the main worktree (where .git is a directory), returns empty string.
13-
// For linked worktrees (where .git is a file), extracts the name from
14-
// .git/worktrees/<name>/ path. This name is stable across `git worktree move`.
15-
// Uses os.Root for traversal-resistant access.
16-
func GetWorktreeID(worktreePath string) (string, error) {
12+
// worktreeMarkers are the path segments that identify a git worktree admin
13+
// directory inside the gitdir value. Both standard (.git/worktrees/) and
14+
// bare-repo (.bare/worktrees/) layouts are supported.
15+
var worktreeMarkers = []string{".git/worktrees/", ".bare/worktrees/"}
16+
17+
// parseGitfile reads a .git gitfile via os.Root and returns the raw gitdir value.
18+
// Returns empty string and no error if .git is a directory (main worktree).
19+
// Returns an error if .git doesn't exist or cannot be read.
20+
func parseGitfile(worktreePath string) (string, error) {
1721
root, err := os.OpenRoot(worktreePath)
1822
if err != nil {
19-
return "", fmt.Errorf("failed to open worktree path: %w", err)
23+
return "", fmt.Errorf("failed to open path: %w", err)
2024
}
2125
defer root.Close()
2226

@@ -30,7 +34,6 @@ func GetWorktreeID(worktreePath string) (string, error) {
3034
return "", nil
3135
}
3236

33-
// Linked worktree has .git as a file with content: "gitdir: /path/to/.git/worktrees/<name>"
3437
content, err := osroot.ReadFile(root, ".git")
3538
if err != nil {
3639
return "", fmt.Errorf("failed to read .git file: %w", err)
@@ -43,12 +46,53 @@ func GetWorktreeID(worktreePath string) (string, error) {
4346

4447
gitdir := strings.TrimPrefix(line, "gitdir: ")
4548

49+
// Resolve relative gitdir paths against the worktree root.
50+
if !filepath.IsAbs(gitdir) {
51+
gitdir = filepath.Join(worktreePath, gitdir)
52+
}
53+
gitdir = filepath.Clean(gitdir)
54+
55+
// Normalize to forward slashes for consistent marker matching.
56+
gitdir = filepath.ToSlash(gitdir)
57+
58+
return gitdir, nil
59+
}
60+
61+
// hasWorktreeMarker reports whether a gitdir value contains a worktree admin
62+
// marker (e.g. ".git/worktrees/" or ".bare/worktrees/"). This distinguishes
63+
// linked worktrees from submodules and other gitfile-based layouts.
64+
func hasWorktreeMarker(gitdir string) bool {
65+
for _, marker := range worktreeMarkers {
66+
if strings.Contains(gitdir, marker) {
67+
return true
68+
}
69+
}
70+
return false
71+
}
72+
73+
// GetWorktreeID returns the internal git worktree identifier for the given path.
74+
// For the main worktree (where .git is a directory), returns empty string.
75+
// For linked worktrees (where .git is a file pointing into a worktree admin dir),
76+
// extracts the name from the .git/worktrees/<name>/ path. This name is stable
77+
// across `git worktree move`.
78+
// Returns an error for non-worktree gitfiles (e.g. submodules).
79+
// Uses os.Root for traversal-resistant access.
80+
func GetWorktreeID(worktreePath string) (string, error) {
81+
gitdir, err := parseGitfile(worktreePath)
82+
if err != nil {
83+
return "", err
84+
}
85+
86+
// Main worktree: .git is a directory, gitdir is empty.
87+
if gitdir == "" {
88+
return "", nil
89+
}
90+
4691
// Extract worktree name from path like /repo/.git/worktrees/<name>
4792
// or /repo/.bare/worktrees/<name> (bare repo + worktree layout).
48-
// The path after the marker is the worktree identifier.
4993
var worktreeID string
5094
var found bool
51-
for _, marker := range []string{".git/worktrees/", ".bare/worktrees/"} {
95+
for _, marker := range worktreeMarkers {
5296
_, worktreeID, found = strings.Cut(gitdir, marker)
5397
if found {
5498
break

0 commit comments

Comments
 (0)