Skip to content

Commit d38936c

Browse files
committed
fix: normalize "." repo identifier to auto-detect in openGitRepo
When a user passes "." as a repo identifier (e.g., `aifr log .`, `aifr refs .`, `aifr stash-list .`), it means "this directory" — semantically identical to the no-argument auto-detect case. Previously, "." was treated as an explicit filesystem path and subjected to access control on the repo root, causing it to be denied when the auto-detect path (empty string) would succeed for the same repo. Fix by normalizing "." to "" at the top of openGitRepo, the single chokepoint for all git operations. This applies consistently to all git commands (log, refs, stash-list, rev-parse, git-config) without touching any individual command's arg parsing. ".." and other relative paths ("./sub", "../other") remain subject to access control since they may reference a different repository. https://claude.ai/code/session_016QBk2VFvP92AXspnSePeLz
1 parent 4f9e31e commit d38936c

File tree

2 files changed

+137
-0
lines changed

2 files changed

+137
-0
lines changed

internal/engine/gitops.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,15 @@ import (
1717
// openGitRepo opens a git repository and checks access control for
1818
// filesystem-path repos. Named repos and CWD auto-detect skip the
1919
// access check since they are admin-configured or implicitly allowed.
20+
//
21+
// Bare "." is normalized to "" (auto-detect) since it means "this
22+
// directory" — semantically identical to the no-argument case. Other
23+
// relative paths ("./sub", "..", "../other") remain subject to access
24+
// control because they may reference a different repository.
2025
func (e *Engine) openGitRepo(repoIdentifier string) (*git.Repository, string, error) {
26+
if repoIdentifier == "." {
27+
repoIdentifier = ""
28+
}
2129
repo, repoPath, err := e.gitProvider.OpenRepo(repoIdentifier)
2230
if err != nil {
2331
return nil, "", err

internal/engine/gitops_test.go

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// Copyright 2026 — see LICENSE file for terms.
2+
package engine
3+
4+
import (
5+
"os"
6+
"testing"
7+
8+
"go.pennock.tech/aifr/internal/accessctl"
9+
"go.pennock.tech/aifr/internal/config"
10+
)
11+
12+
// TestOpenGitRepoDotNormalization verifies that "." is treated as
13+
// auto-detect (same as "") rather than as an explicit filesystem path.
14+
// This matters because auto-detect skips access control (the user is
15+
// already in the directory), while explicit paths are checked.
16+
func TestOpenGitRepoDotNormalization(t *testing.T) {
17+
dir := t.TempDir()
18+
initTestGitRepo(t, dir, 1)
19+
20+
// Create an engine that does NOT allow the repo dir.
21+
// Use a non-existent allow path so nothing is permitted.
22+
checker, err := accessctl.NewChecker(accessctl.CheckerParams{
23+
Allow: []string{"/nonexistent-allow-path/**"},
24+
})
25+
if err != nil {
26+
t.Fatal(err)
27+
}
28+
eng, err := NewEngine(checker, config.DefaultConfig())
29+
if err != nil {
30+
t.Fatal(err)
31+
}
32+
33+
// cd into the repo so auto-detect finds it.
34+
origDir, err := os.Getwd()
35+
if err != nil {
36+
t.Fatal(err)
37+
}
38+
t.Cleanup(func() { os.Chdir(origDir) })
39+
if err := os.Chdir(dir); err != nil {
40+
t.Fatal(err)
41+
}
42+
43+
// Auto-detect (empty string) should succeed — no access check.
44+
_, err = eng.Log("", "HEAD", LogParams{MaxCount: 1})
45+
if err != nil {
46+
t.Errorf("Log with empty repo (auto-detect) failed: %v", err)
47+
}
48+
49+
// Bare "." should also succeed — normalized to auto-detect.
50+
_, err = eng.Log(".", "HEAD", LogParams{MaxCount: 1})
51+
if err != nil {
52+
t.Errorf("Log with \".\" repo failed: %v", err)
53+
}
54+
55+
// Explicit absolute path should be denied.
56+
_, err = eng.Log(dir, "HEAD", LogParams{MaxCount: 1})
57+
if err == nil {
58+
t.Error("Log with explicit absolute path should have been denied")
59+
}
60+
61+
// ".." should be denied (it's a different path, subject to access control).
62+
// Create a subdir and cd into it so ".." resolves to the repo root.
63+
subdir := dir + "/subdir"
64+
os.Mkdir(subdir, 0o755)
65+
if err := os.Chdir(subdir); err != nil {
66+
t.Fatal(err)
67+
}
68+
_, err = eng.Log("..", "HEAD", LogParams{MaxCount: 1})
69+
if err == nil {
70+
t.Error("Log with \"..\" should have been denied")
71+
}
72+
}
73+
74+
// TestDotRepoConsistentAcrossCommands verifies that "." normalization
75+
// works for all git commands, not just log.
76+
func TestDotRepoConsistentAcrossCommands(t *testing.T) {
77+
dir := t.TempDir()
78+
initTestGitRepo(t, dir, 1)
79+
80+
checker, err := accessctl.NewChecker(accessctl.CheckerParams{
81+
Allow: []string{"/nonexistent-allow-path/**"},
82+
})
83+
if err != nil {
84+
t.Fatal(err)
85+
}
86+
eng, err := NewEngine(checker, config.DefaultConfig())
87+
if err != nil {
88+
t.Fatal(err)
89+
}
90+
91+
origDir, err := os.Getwd()
92+
if err != nil {
93+
t.Fatal(err)
94+
}
95+
t.Cleanup(func() { os.Chdir(origDir) })
96+
if err := os.Chdir(dir); err != nil {
97+
t.Fatal(err)
98+
}
99+
100+
// All these should succeed with "." (normalized to auto-detect).
101+
t.Run("Refs", func(t *testing.T) {
102+
_, err := eng.Refs(".", true, true, true)
103+
if err != nil {
104+
t.Errorf("Refs with \".\" failed: %v", err)
105+
}
106+
})
107+
108+
t.Run("Log", func(t *testing.T) {
109+
_, err := eng.Log(".", "HEAD", LogParams{MaxCount: 1})
110+
if err != nil {
111+
t.Errorf("Log with \".\" failed: %v", err)
112+
}
113+
})
114+
115+
// All these should fail with explicit absolute path.
116+
t.Run("Refs_absolute_denied", func(t *testing.T) {
117+
_, err := eng.Refs(dir, true, true, true)
118+
if err == nil {
119+
t.Error("Refs with absolute path should have been denied")
120+
}
121+
})
122+
123+
t.Run("Log_absolute_denied", func(t *testing.T) {
124+
_, err := eng.Log(dir, "HEAD", LogParams{MaxCount: 1})
125+
if err == nil {
126+
t.Error("Log with absolute path should have been denied")
127+
}
128+
})
129+
}

0 commit comments

Comments
 (0)