Skip to content

Commit 46ffc0b

Browse files
committed
improve how discover detect a repo, for safety reasons
1 parent df9fb47 commit 46ffc0b

8 files changed

Lines changed: 167 additions & 15 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func main() {
5050
// Find the repository and where git reads its configuration from.
5151
env, err := gitconfig.Discover(".")
5252
if err != nil {
53-
log.Fatal(err) // ErrUntrusted, as git refuses it
53+
log.Fatal(err) // ErrNotRepository, or ErrUntrusted as git refuses it
5454
}
5555

5656
// Read the effective configuration: system, global, local, worktree

‎discover.go‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,23 +94,71 @@ func discover(dir string, env environ) (Env, error) {
9494
// it when started there, and the paths whose owner git checks. In a working
9595
// tree (dir holds .git, or is a "gitdir:" file), they are the working tree,
9696
// its .git and the git directory; elsewhere, dir alone. The first is the
97-
// path safe.directory lists.
97+
// path safe.directory lists. A git directory that checkGitDir refuses fails
98+
// with ErrNotRepository.
9899
func findRepo(dir string) (gitDir string, owned []string, err error) {
99100
if dir, err = filepath.Abs(dir); err != nil {
100101
return "", nil, err
101102
}
102103
dot := filepath.Join(dir, ".git")
103104
if fi, err := os.Stat(dir); err == nil && fi.Mode().IsRegular() {
104105
dir, dot = filepath.Dir(dir), dir
105-
} else if _, err := os.Stat(dot); errors.Is(err, fs.ErrNotExist) {
106-
// Also a directory that does not exist (yet): loading finds no
107-
// files in it.
106+
} else if _, err := os.Stat(dot); isMissing(err) {
107+
if err := checkGitDir(dir); err != nil {
108+
return "", nil, err
109+
}
108110
return dir, []string{dir}, nil
109111
} else if err != nil {
110112
return "", nil, &FileError{Path: dot, Err: err}
111113
}
112-
gitDir, err = resolveGitDir(dot)
113-
return gitDir, []string{dir, dot, gitDir}, err
114+
if gitDir, err = resolveGitDir(dot); err != nil {
115+
return "", nil, err
116+
}
117+
if err := checkGitDir(gitDir); err != nil {
118+
return "", nil, err
119+
}
120+
return gitDir, []string{dir, dot, gitDir}, nil
121+
}
122+
123+
// checkGitDir fails with ErrNotRepository unless gitDir is a git directory
124+
// as git recognizes one: it holds a HEAD file, and its common directory
125+
// (gitDir itself, or the one its commondir file names in a linked worktree)
126+
// holds objects and refs directories. Any other directory would have its
127+
// config read and written as the local scope. HEAD may also be a symbolic
128+
// link to a branch (core.preferSymlinkRefs), whose target does not exist
129+
// while the branch is unborn or packed. What HEAD holds is not checked,
130+
// where git also requires a ref or an object name.
131+
func checkGitDir(gitDir string) error {
132+
fi, err := os.Lstat(filepath.Join(gitDir, "HEAD"))
133+
if err != nil {
134+
return notRepository(gitDir, err)
135+
}
136+
if !fi.Mode().IsRegular() && fi.Mode()&fs.ModeSymlink == 0 {
137+
return &FileError{Path: gitDir, Err: ErrNotRepository}
138+
}
139+
r, err := Env{GitDir: gitDir}.repo()
140+
if err != nil {
141+
return err
142+
}
143+
for _, name := range []string{"objects", "refs"} {
144+
fi, err := os.Stat(filepath.Join(r.commonDir, name))
145+
if err != nil {
146+
return notRepository(gitDir, err)
147+
}
148+
if !fi.IsDir() {
149+
return &FileError{Path: gitDir, Err: ErrNotRepository}
150+
}
151+
}
152+
return nil
153+
}
154+
155+
// notRepository wraps the error met checking the git directory gitDir: a
156+
// missing path means it is not a repository.
157+
func notRepository(gitDir string, err error) error {
158+
if isMissing(err) {
159+
err = ErrNotRepository
160+
}
161+
return &FileError{Path: gitDir, Err: err}
114162
}
115163

116164
// checkTrust fails with ErrUntrusted unless the current user owns every

‎doc.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
// Reading the effective configuration of a repository and changing a value:
1515
//
1616
// env, err := gitconfig.Discover(".") // the working tree
17-
// if err != nil { ... } // ErrUntrusted, as git refuses it
17+
// if err != nil { ... } // ErrNotRepository, or ErrUntrusted
1818
//
1919
// cfg, err := env.Load()
2020
// if err != nil { ... }

‎docs/COMPATIBILITY.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ in [GIT-BEHAVIOUR.md](GIT-BEHAVIOUR.md).
7676
| Repository ownership check | done when git finds the repository, not for `GIT_DIR` | done by `Discover`, not for an `Env` filled by hand | 1 (X5) |
7777
| What is checked, and listed in `safe.directory` | depends on where git starts: a working tree, or a git directory | the same, from the path given to `Discover` | 1 — the caller should pass the working tree, which users list |
7878
| Untrusted repository | `git config` silently skips the local scope; other commands fail | `Discover` fails with `ErrUntrusted` | a silently missing scope surprises; reading less is not safer than refusing — *safe default* |
79+
| `Discover` given a directory that is not a repository | git searches the parent directories, and fails with "not a git repository" when there is none | `ErrNotRepository`: a git directory must hold a `HEAD` file, and its common directory `objects/` and `refs/` directories, as for git, which also requires `HEAD` to hold a ref or an object name | `Discover` does not search; reading any directory's `config` as the local scope would read and write a file that is not git's — *safe default* |
7980
| `SUDO_UID` = `' 1000'`, `+1000`, or beyond 32 bits | accepted; a value beyond 32 bits wraps around (2³² + 1000 is 1000) | ignored: digits only, at most 32 bits | the wrap-around trusts a user nobody named — *safe default* |
8081
| Administrators group on Windows | trusted for an administrator (Git for Windows commit messages, 2022 and 2025) | the same | 1 — not observed: no Windows git to run but CI's |
8182
| UNC paths on Windows | must be listed in `safe.directory` since Git for Windows 2.54 (release notes) | owner checked like any path | the release note is not precise enough to follow; not observed |

‎docs/GIT-BEHAVIOUR.md‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,3 +300,34 @@ tree; started in a git directory, it checks that directory alone, and
300300
| the same, `/r/dots` owned by `bob`, `safe.directory=/r/dots` / `=/r/home` | accepted / refused, at `/r/dots` |
301301
| `--separate-git-dir=/r/sep/x` for `/r/w`, `/r/w` owned by `bob`, `git -C /r/sep/x` | accepted |
302302
| linked worktree `/r/lw` owned by `bob`, `git -C /r/w/.git/worktrees/lw` / `git -C /r/lw` | accepted / refused, at `/r/lw` |
303+
304+
## What is a repository
305+
306+
Observed with git 2.47.3, as `git rev-parse --git-dir` run in `/r/w` with
307+
`GIT_CEILING_DIRECTORIES=/r`. "Refused" means `fatal: not a git repository`.
308+
Unless stated, `/r/w/.git` holds `HEAD` (`ref: refs/heads/main`) and the
309+
directories `objects` and `refs`.
310+
311+
| `/r/w/.git` | Result |
312+
|---|---|
313+
| as stated | accepted |
314+
| `HEAD` alone | refused |
315+
| no `objects`, or no `refs` | refused |
316+
| `objects` or `refs` a regular file | refused |
317+
| `objects` a symbolic link to a directory | accepted |
318+
| `HEAD` a directory | refused |
319+
| `HEAD` holding `garbage` | refused |
320+
| `HEAD` holding an object name | accepted |
321+
| `HEAD` a symbolic link to `refs/heads/main`, which does not exist | accepted |
322+
| `HEAD` a symbolic link to a directory | accepted |
323+
| created by `git init --ref-format=reftable`: `refs` is a directory holding a regular file `heads`, `HEAD` holds `ref: refs/heads/.invalid` | accepted |
324+
325+
A linked worktree's git directory, `/r/w/.git/worktrees/lw`, holds `HEAD`
326+
and `commondir` (`../..`) but no `objects`:
327+
328+
| Linked worktree `/r/lw` | Result |
329+
|---|---|
330+
| as created by `git worktree add` | accepted |
331+
| no `refs` in `worktrees/lw` | accepted |
332+
| no `refs`, or no `objects`, in `/r/w/.git` | refused |
333+
| no `commondir` | refused |

‎env.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,13 @@ type CommandEntry struct {
112112
// followed. Finding the repository is left to the caller: Discover does not
113113
// search parent directories and does not read GIT_DIR. Pass "" when not in
114114
// a repository: only the system, global and command-line scopes exist then.
115+
// A dir that is not a repository fails with ErrNotRepository, in a
116+
// *FileError naming the git directory expected: its .git when it exists (or
117+
// the directory a gitdir file names), dir itself otherwise. As for git, a
118+
// git directory must hold a HEAD file, and its common directory objects and
119+
// refs directories; any other would have its config read and written as
120+
// the local scope. A caller that wants the repository only when there is
121+
// one can fall back to Discover("") on ErrNotRepository.
115122
//
116123
// The repository must be trusted, as git requires: owned by the current
117124
// user (on Unix, also the SUDO_UID user for root; on Windows, the

‎env_test.go‎

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ func initRepo(t testing.TB, gitDir, config string) {
5454
t.Helper()
5555
writeFile(t, filepath.Join(gitDir, "HEAD"), "ref: refs/heads/main\n")
5656
writeFile(t, filepath.Join(gitDir, "config"), config)
57+
require.NoError(t, os.MkdirAll(filepath.Join(gitDir, "objects"), 0o755))
58+
require.NoError(t, os.MkdirAll(filepath.Join(gitDir, "refs"), 0o755))
5759
}
5860

5961
// addWorktree registers a linked worktree named name in the repository at
@@ -1309,21 +1311,76 @@ func TestDiscover(t *testing.T) {
13091311
assert.Equal(t, gitDir, env.GitDir)
13101312
})
13111313

1312-
t.Run("working tree", func(t *testing.T) {
1314+
t.Run("HEAD a link to a branch with no loose file", func(t *testing.T) {
1315+
// core.preferSymlinkRefs: the branch is unborn or packed.
13131316
home := cleanGitEnv(t)
13141317
gitDir := filepath.Join(home, "repo", ".git")
13151318
initRepo(t, gitDir, "")
1319+
require.NoError(t, os.Remove(filepath.Join(gitDir, "HEAD")))
1320+
symlink(t, filepath.Join("refs", "heads", "main"), filepath.Join(gitDir, "HEAD"))
13161321
env, err := gitconfig.Discover(filepath.Join(home, "repo"))
13171322
require.NoError(t, err)
13181323
assert.Equal(t, gitDir, env.GitDir)
13191324
})
13201325

1321-
t.Run("missing directory", func(t *testing.T) {
1322-
// Nothing to check, nothing to load.
1326+
t.Run("working tree", func(t *testing.T) {
13231327
home := cleanGitEnv(t)
1324-
env, err := gitconfig.Discover(filepath.Join(home, "missing"))
1328+
gitDir := filepath.Join(home, "repo", ".git")
1329+
initRepo(t, gitDir, "")
1330+
env, err := gitconfig.Discover(filepath.Join(home, "repo"))
13251331
require.NoError(t, err)
1326-
assert.Equal(t, filepath.Join(home, "missing"), env.GitDir)
1332+
assert.Equal(t, gitDir, env.GitDir)
1333+
})
1334+
1335+
t.Run("not a repository", func(t *testing.T) {
1336+
home := cleanGitEnv(t)
1337+
// A plain directory whose config would be read as the local scope.
1338+
writeFile(t, filepath.Join(home, "plain", "config"), "[a]\n\tb = 1\n")
1339+
// A config that is not git's, and not a file.
1340+
require.NoError(t, os.MkdirAll(filepath.Join(home, "dotdir", "config"), 0o755))
1341+
// A .git without HEAD, and a HEAD that is a directory.
1342+
initRepo(t, filepath.Join(home, "nohead", ".git"), "")
1343+
require.NoError(t, os.Remove(filepath.Join(home, "nohead", ".git", "HEAD")))
1344+
initRepo(t, filepath.Join(home, "headdir"), "")
1345+
require.NoError(t, os.Remove(filepath.Join(home, "headdir", "HEAD")))
1346+
require.NoError(t, os.MkdirAll(filepath.Join(home, "headdir", "HEAD"), 0o755))
1347+
// HEAD alone, objects missing, and refs a file.
1348+
writeFile(t, filepath.Join(home, "headonly", "HEAD"), "ref: refs/heads/main\n")
1349+
initRepo(t, filepath.Join(home, "noobjects"), "")
1350+
require.NoError(t, os.Remove(filepath.Join(home, "noobjects", "objects")))
1351+
initRepo(t, filepath.Join(home, "reffile"), "")
1352+
require.NoError(t, os.Remove(filepath.Join(home, "reffile", "refs")))
1353+
writeFile(t, filepath.Join(home, "reffile", "refs"), "")
1354+
// A linked worktree whose common directory has no objects.
1355+
initRepo(t, filepath.Join(home, "main", ".git"), "")
1356+
addWorktree(t, filepath.Join(home, "main", ".git"), "lw", "main")
1357+
require.NoError(t, os.Remove(filepath.Join(home, "main", ".git", "objects")))
1358+
writeFile(t, filepath.Join(home, "lw", ".git"), "gitdir: ../main/.git/worktrees/lw\n")
1359+
// A gitdir file naming a directory that is not a repository.
1360+
writeFile(t, filepath.Join(home, "wt", ".git"), "gitdir: ../plain\n")
1361+
// A regular file in the way.
1362+
writeFile(t, filepath.Join(home, "file"), "")
1363+
1364+
for _, tc := range []struct{ dir, gitDir string }{
1365+
{"missing", "missing"},
1366+
{"plain", "plain"},
1367+
{"dotdir", "dotdir"},
1368+
{"nohead", filepath.Join("nohead", ".git")},
1369+
{filepath.Join("nohead", ".git"), filepath.Join("nohead", ".git")},
1370+
{"headdir", "headdir"},
1371+
{"headonly", "headonly"},
1372+
{"noobjects", "noobjects"},
1373+
{"reffile", "reffile"},
1374+
{"lw", filepath.Join("main", ".git", "worktrees", "lw")},
1375+
{"wt", "plain"},
1376+
{filepath.Join("file", "sub"), filepath.Join("file", "sub")},
1377+
} {
1378+
_, err := gitconfig.Discover(filepath.Join(home, tc.dir))
1379+
require.ErrorIs(t, err, gitconfig.ErrNotRepository, tc.dir)
1380+
fe, ok := errors.AsType[*gitconfig.FileError](err)
1381+
require.True(t, ok, tc.dir)
1382+
assert.Equal(t, filepath.Join(home, tc.gitDir), filepath.Clean(fe.Path), tc.dir)
1383+
}
13271384
})
13281385

13291386
t.Run("malformed gitdir file", func(t *testing.T) {

‎errors.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ import (
2121
// unwrapped, as no file is concerned. The error returned by the
2222
// callback of Env.UpdateFile or Env.UpdateScope is returned as is.
2323
// - Discover also returns an *EnvError for an environment variable it
24-
// cannot interpret, and ErrUntrusted in a *FileError for a repository
25-
// git would refuse.
24+
// cannot interpret, ErrNotRepository in a *FileError for a directory
25+
// that is not a repository, and ErrUntrusted in a *FileError for a
26+
// repository git would refuse.
2627
//
2728
// Error messages name files, whose paths can come from a configuration
2829
// file someone else wrote. As in git, but stricter, control characters
@@ -82,6 +83,13 @@ var (
8283
// safe.directory. The *FileError's Path is the path to list.
8384
ErrUntrusted = errors.New("gitconfig: repository not owned by the current user nor listed in safe.directory")
8485

86+
// ErrNotRepository is returned by Discover for a directory that is not
87+
// a repository: the git directory expected, its .git when it exists and
88+
// the directory itself otherwise, holds no HEAD file, or its common
89+
// directory no objects and refs directories. The *FileError's Path is
90+
// that git directory.
91+
ErrNotRepository = errors.New("gitconfig: not a git repository")
92+
8593
// ErrClosed is returned when a File is edited after the callback of
8694
// Env.UpdateFile or Env.UpdateScope has returned.
8795
ErrClosed = errors.New("gitconfig: file is closed")

0 commit comments

Comments
 (0)