fix(cmd): read git top-level via stdout-only helper

Resolve the repo top level through runGitCmdStdout (cmd.Output) instead of
CombinedOutput so git stderr notices (permission warnings, deprecations,
config hints) cannot pollute the resolved path. Add a regression test that a
bare repo (no work tree) fails loudly on the review path rather than silently
falling back to the invocation dir.
This commit is contained in:
kite 2026-07-06 19:19:30 +08:00
parent 82fe45a110
commit 13f81f24f6
4 changed files with 34 additions and 2 deletions

View file

@ -12,6 +12,16 @@ func runGitCmd(repoDir string, args ...string) ([]byte, error) {
return cmd.CombinedOutput()
}
// runGitCmdStdout runs git and returns stdout only. Unlike runGitCmd it keeps
// stderr separate, so git notices (permission warnings, deprecations, config
// hints) cannot pollute output that is consumed as data — e.g. the path from
// `git rev-parse --show-toplevel`.
func runGitCmdStdout(repoDir string, args ...string) ([]byte, error) {
fullArgs := append([]string{"-C", repoDir}, args...)
cmd := exec.Command("git", fullArgs...)
return cmd.Output()
}
func getCommitMessage(repoDir, commit string) (string, error) {
out, err := runGitCmd(repoDir, "log", "-1", "--format=%B", "--end-of-options", commit)
if err != nil {

View file

@ -129,7 +129,7 @@ func resolveRepoDir(input string) (string, error) {
}
// Resolve to the repository top level so paths stay repo-root relative
// when invoked from a monorepo subdirectory (#287).
out, err := runGitCmd(absPath, "rev-parse", "--show-toplevel")
out, err := runGitCmdStdout(absPath, "rev-parse", "--show-toplevel")
toplevel := strings.TrimSpace(string(out))
if err != nil || toplevel == "" {
return "", fmt.Errorf("%s is not a git repository", absPath)

View file

@ -104,7 +104,7 @@ func resolveWorkingDir(input string, requireGit bool) (string, bool, error) {
if _, statErr := os.Stat(absPath); statErr != nil {
return "", false, fmt.Errorf("stat %s: %w", absPath, statErr)
}
out, err := runGitCmd(absPath, "rev-parse", "--show-toplevel")
out, err := runGitCmdStdout(absPath, "rev-parse", "--show-toplevel")
toplevel := strings.TrimSpace(string(out))
isGit := err == nil && toplevel != ""
if !isGit {

View file

@ -2,7 +2,9 @@ package main
import (
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"github.com/open-code-review/open-code-review/internal/config/rules"
@ -158,6 +160,26 @@ func TestResolveWorkingDir_ScanSubdirKeepsScope(t *testing.T) {
}
}
// TestResolveWorkingDir_BareRepoFailsLoudly ensures a bare repository (where
// `git rev-parse --show-toplevel` yields no work tree) errors on the review
// path instead of silently falling back to the invocation dir and reproducing
// the #287 path mismatch.
func TestResolveWorkingDir_BareRepoFailsLoudly(t *testing.T) {
dir := t.TempDir()
cmd := exec.Command("git", "init", "--bare", dir)
if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("git init --bare: %v: %s", err, out)
}
_, _, err := resolveWorkingDir(dir, true)
if err == nil {
t.Fatal("expected error for bare repo with requireGit=true")
}
if !strings.Contains(err.Error(), "not a git repository") {
t.Errorf("unexpected error: %v", err)
}
}
func TestResolveWorkingDir_GitRepo(t *testing.T) {
dir := t.TempDir()
gitDir := filepath.Join(dir, ".git")