Fix two bugs breaking repo browse/settings on freshly pushed repos
1. gitexec: InitBareRepo left HEAD pointing at whatever this machine's git defaults new repos to, with nothing forcing it to match the branch a client actually pushes. A repo pushed as "main" against a HEAD stuck on "master" (or vice versa) has real content but HEAD never resolves, so ReadFileAtHEAD/ListTree treated it as empty — every freshly pushed repo 404'd on /repo-tree and /repo-blob. Fixed two ways: InitBareRepo now sets HEAD to refs/heads/main explicitly, and both read functions fall back to the repo's sole branch when HEAD doesn't resolve, so already-affected repos self-heal on the next read with no migration. 2. adminrpc: errors only cross the Unix socket as strings, and the client was wrapping them in a fresh errors.New — which can never compare equal to a sentinel like store.ErrNotFound even with identical text. Every `err == store.ErrNotFound` check downstream of a live (RPC-connected) call was silently broken; concretely, GetACL returning ErrNotFound for a repo with no collaborators yet 500'd the repo settings page. Fixed by reconstructing the sentinel by message in the client's call() — one fix at the transport layer instead of patching every call site. Both covered by regression tests that reproduce the exact scenario rather than just re-asserting the fixed behavior.
4 files changed
+166 −10
A
internal/adminrpc/adminrpc_test.go
+57 −0
M
internal/adminrpc/client.go
+8 −0
M
internal/gitexec/gitexec.go
+52 −10
M
internal/gitexec/gitexec_test.go
+49 −0
internal/adminrpc/adminrpc_test.go
@@ -0,0 +1,57 @@
+package adminrpc
+
+import (
+ "path/filepath"
+ "testing"
+ "time"
+
+ "gitfed/internal/admin"
+ "gitfed/internal/federation"
+ "gitfed/internal/store"
+)
+
+// TestErrNotFoundSurvivesRPC reproduces a real bug: the wire only carries
+// error strings, so a naive client just wrapping resp.Error in a fresh
+// errors.New produces a value that will never compare equal to
+// store.ErrNotFound — breaking every `err == store.ErrNotFound` check
+// downstream (this exact thing 500'd gitfed-web's repo settings page,
+// since GetACL on a repo with no collaborators yet returns ErrNotFound).
+func TestErrNotFoundSurvivesRPC(t *testing.T) {
+ dir := t.TempDir()
+ st, err := store.Open(filepath.Join(dir, "gitfed.db"))
+ if err != nil {
+ t.Fatalf("open store: %v", err)
+ }
+ defer st.Close()
+
+ a := admin.New(st, federation.NewResolver(st, "local.test", true), "local.test", dir)
+ if err := a.CreateRepo("alice/demo", "alice"); err != nil {
+ t.Fatalf("create repo: %v", err)
+ }
+ // No collaborators granted, so the ACL bucket has no entry for this
+ // repo — GetACL is expected to return store.ErrNotFound.
+
+ socketPath := filepath.Join(dir, "admin.sock")
+ srv := NewServer(a, socketPath)
+ go func() { _ = srv.ListenAndServe() }()
+
+ client := NewClient(socketPath)
+ deadline := time.Now().Add(2 * time.Second)
+ for {
+ if client.Ping() == nil {
+ break
+ }
+ if time.Now().After(deadline) {
+ t.Fatal("server never came up")
+ }
+ time.Sleep(10 * time.Millisecond)
+ }
+
+ _, err = client.GetACL("alice/demo")
+ if err == nil {
+ t.Fatal("expected store.ErrNotFound, got nil")
+ }
+ if err != store.ErrNotFound {
+ t.Fatalf("got error %q (%T), want the exact store.ErrNotFound sentinel — identity was lost crossing the RPC boundary", err, err)
+ }
+}
internal/adminrpc/client.go
@@ -50,6 +50,14 @@ func (c *Client) call(method string, args any, out any) error {
return err
}
if resp.Error != "" {
+ // Reconstruct sentinel errors by message so identity comparisons
+ // like `err == store.ErrNotFound` still work across the RPC
+ // boundary — JSON only carries the error string, and a fresh
+ // errors.New(resp.Error) would never compare equal to the
+ // original sentinel even with the same text.
+ if resp.Error == store.ErrNotFound.Error() {
+ return store.ErrNotFound
+ }
return errors.New(resp.Error)
}
if out != nil && len(resp.Result) > 0 {
internal/gitexec/gitexec.go
@@ -81,6 +81,17 @@ func InitBareRepo(path string) error {
if err != nil {
return fmt.Errorf("gitexec: git init --bare: %w: %s", err, out)
}
+
+ // Match the branch name modern git clients actually push by default,
+ // rather than whatever this machine's git happens to default new
+ // repos to — otherwise HEAD can point at a branch nobody ever pushes
+ // (resolveDefaultRef below falls back gracefully either way, but
+ // there's no reason to rely on the fallback when we can just get it
+ // right at creation time).
+ symCmd := exec.Command("git", "--git-dir="+path, "symbolic-ref", "HEAD", "refs/heads/main")
+ if out, err := symCmd.CombinedOutput(); err != nil {
+ return fmt.Errorf("gitexec: set default branch: %w: %s", err, out)
+ }
return nil
}
@@ -108,13 +119,39 @@ func Serve(verb Verb, path string, stdin io.Reader, stdout, stderr io.Writer) er
return cmd.Run()
}
+// resolveDefaultRef returns the tree-ish gitfed treats as this repo's
+// default branch. It tries HEAD first, but a bare repo's HEAD symref is set
+// once at creation time (see InitBareRepo) and doesn't move just because a
+// client pushes a differently-named branch — so a repo that's had exactly
+// one branch pushed to it, ever, is unambiguous even when HEAD is stale.
+// ok is false only for a genuinely empty or ambiguous (multiple branches,
+// none of them HEAD) repo.
+func resolveDefaultRef(repoPath string) (ref string, ok bool) {
+ if exec.Command("git", "--git-dir="+repoPath, "rev-parse", "--verify", "-q", "HEAD").Run() == nil {
+ return "HEAD", true
+ }
+ out, err := exec.Command("git", "--git-dir="+repoPath, "for-each-ref", "--format=%(refname)", "refs/heads/").Output()
+ if err != nil {
+ return "", false
+ }
+ refs := strings.Fields(string(out))
+ if len(refs) != 1 {
+ return "", false
+ }
+ return refs[0], true
+}
+
// ReadFileAtHEAD returns the content of filename as it exists in the tree at
-// HEAD of the bare repo at repoPath. found is false (with a nil error) if
-// the repo has no commits yet or the file doesn't exist at HEAD — both are
-// expected, unremarkable states for e.g. an optional README.
+// the repo's default branch (see resolveDefaultRef). found is false (with a
+// nil error) if the repo has no commits yet or the file doesn't exist there
+// — both are expected, unremarkable states for e.g. an optional README.
func ReadFileAtHEAD(repoPath, filename string) (content string, found bool, err error) {
+ ref, ok := resolveDefaultRef(repoPath)
+ if !ok {
+ return "", false, nil
+ }
filename = strings.TrimPrefix(filename, "/")
- cmd := exec.Command("git", "--git-dir="+repoPath, "show", "HEAD:"+filename)
+ cmd := exec.Command("git", "--git-dir="+repoPath, "show", ref+":"+filename)
var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = &stderr
@@ -122,7 +159,7 @@ func ReadFileAtHEAD(repoPath, filename string) (content string, found bool, err
if notFoundGitError(stderr.String()) {
return "", false, nil
}
- return "", false, fmt.Errorf("gitexec: read %s at HEAD: %w: %s", filename, err, stderr.String())
+ return "", false, fmt.Errorf("gitexec: read %s at %s: %w: %s", filename, ref, err, stderr.String())
}
return stdout.String(), true, nil
}
@@ -143,14 +180,19 @@ func notFoundGitError(msg string) bool {
}
// ListTree returns the immediate children (files and directories) at path
-// in the tree at HEAD, directories first then files, alphabetically within
-// each. path == "" lists the repo root. found is false (nil error) for an
-// empty repo or a path that doesn't exist — both unremarkable.
+// in the tree at the repo's default branch (see resolveDefaultRef),
+// directories first then files, alphabetically within each. path == "" lists
+// the repo root. found is false (nil error) for an empty repo or a path
+// that doesn't exist — both unremarkable.
func ListTree(repoPath, path string) (entries []TreeEntry, found bool, err error) {
+ ref, ok := resolveDefaultRef(repoPath)
+ if !ok {
+ return nil, false, nil
+ }
path = strings.Trim(path, "/")
- treeish := "HEAD"
+ treeish := ref
if path != "" {
- treeish = "HEAD:" + path
+ treeish = ref + ":" + path
}
cmd := exec.Command("git", "--git-dir="+repoPath, "ls-tree", treeish)
internal/gitexec/gitexec_test.go
@@ -133,3 +133,52 @@ func TestReadFileAtHEADEmptyRepo(t *testing.T) {
t.Fatal("empty repo should never report a tree as found")
}
}
+
+// TestPushToMismatchedDefaultBranch reproduces a real bug: InitBareRepo
+// points HEAD at refs/heads/main, but nothing stops a client from pushing
+// under a different branch name (an older git installation still defaults
+// to "master", or someone names their trunk something else entirely). HEAD
+// then never resolves even though the repo plainly has content — this is
+// the exact scenario that made every freshly pushed repo 404 in gitfed-web.
+func TestPushToMismatchedDefaultBranch(t *testing.T) {
+ tmp := t.TempDir()
+ barePath := filepath.Join(tmp, "repo.git")
+ if err := InitBareRepo(barePath); err != nil {
+ t.Fatalf("init bare repo: %v", err)
+ }
+
+ work := filepath.Join(tmp, "work")
+ if err := os.Mkdir(work, 0755); err != nil {
+ t.Fatal(err)
+ }
+ run(t, work, "git", "init", "-q", "-b", "trunk") // deliberately not "main"
+ if err := os.WriteFile(filepath.Join(work, "README.md"), []byte("# Hello\n"), 0644); err != nil {
+ t.Fatal(err)
+ }
+ run(t, work, "git", "add", "README.md")
+ run(t, work, "git", "commit", "-q", "-m", "initial")
+ run(t, work, "git", "remote", "add", "origin", barePath)
+ run(t, work, "git", "push", "-q", "origin", "trunk")
+
+ // Sanity check this test actually exercises the mismatch: HEAD must
+ // NOT resolve on its own, or this isn't testing the fallback at all.
+ if exec.Command("git", "--git-dir="+barePath, "rev-parse", "--verify", "-q", "HEAD").Run() == nil {
+ t.Fatal("test setup invalid: HEAD unexpectedly resolves, mismatch not reproduced")
+ }
+
+ content, found, err := ReadFileAtHEAD(barePath, "README.md")
+ if err != nil {
+ t.Fatalf("ReadFileAtHEAD: %v", err)
+ }
+ if !found || content != "# Hello\n" {
+ t.Fatalf("ReadFileAtHEAD: found=%v content=%q, want found with content", found, content)
+ }
+
+ root, found, err := ListTree(barePath, "")
+ if err != nil {
+ t.Fatalf("ListTree: %v", err)
+ }
+ if !found || len(root) != 1 || root[0].Name != "README.md" {
+ t.Fatalf("ListTree: found=%v root=%+v, want [README.md]", found, root)
+ }
+}