The fork protection logic incorrectly treated all repos where origin != steveyegge/beads as forks, including user's own projects that just use beads as a tool. Changes: - Add isForkOfBeads() that scans ALL remotes for steveyegge/beads - Only apply protection when a beads-related remote exists - Add git config opt-out: `git config beads.fork-protection false` (per-clone, never tracked, matches beads.role pattern) Test coverage for 8 scenarios plus edge cases for config values.
This commit is contained in:
committed by
GitHub
parent
00d0eb0192
commit
f3f713d77a
@@ -13,21 +13,36 @@ import (
|
|||||||
// ensureForkProtection prevents contributors from accidentally committing
|
// ensureForkProtection prevents contributors from accidentally committing
|
||||||
// the upstream issue database when working in a fork.
|
// the upstream issue database when working in a fork.
|
||||||
//
|
//
|
||||||
// When we detect this is a fork (origin != steveyegge/beads), we add
|
// When we detect this is a fork (any remote points to steveyegge/beads),
|
||||||
// .beads/issues.jsonl to .git/info/exclude so it won't be staged.
|
// we add .beads/issues.jsonl to .git/info/exclude so it won't be staged.
|
||||||
// This is a per-clone setting that doesn't modify tracked files.
|
// This is a per-clone setting that doesn't modify tracked files.
|
||||||
|
//
|
||||||
|
// Users can disable this with: git config beads.fork-protection false
|
||||||
func ensureForkProtection() {
|
func ensureForkProtection() {
|
||||||
// Find git root
|
// Find git root first (needed for git config check)
|
||||||
gitRoot := git.GetRepoRoot()
|
gitRoot := git.GetRepoRoot()
|
||||||
if gitRoot == "" {
|
if gitRoot == "" {
|
||||||
return // Not in a git repo
|
return // Not in a git repo
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check if fork protection is explicitly disabled via git config (GH#823)
|
||||||
|
// Use: git config beads.fork-protection false
|
||||||
|
if isForkProtectionDisabled(gitRoot) {
|
||||||
|
debug.Printf("fork protection: disabled via git config")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Check if this is the upstream repo (maintainers)
|
// Check if this is the upstream repo (maintainers)
|
||||||
if isUpstreamRepo(gitRoot) {
|
if isUpstreamRepo(gitRoot) {
|
||||||
return // Maintainers can commit issues.jsonl
|
return // Maintainers can commit issues.jsonl
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Only protect actual forks - repos with any remote pointing to beads (GH#823)
|
||||||
|
// This prevents false positives on user's own projects that just use beads
|
||||||
|
if !isForkOfBeads(gitRoot) {
|
||||||
|
return // Not a fork of beads, user's own project
|
||||||
|
}
|
||||||
|
|
||||||
// Check if already excluded
|
// Check if already excluded
|
||||||
excludePath := filepath.Join(gitRoot, ".git", "info", "exclude")
|
excludePath := filepath.Join(gitRoot, ".git", "info", "exclude")
|
||||||
if isAlreadyExcluded(excludePath) {
|
if isAlreadyExcluded(excludePath) {
|
||||||
@@ -69,6 +84,32 @@ func isUpstreamRepo(gitRoot string) bool {
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// isForkOfBeads checks if ANY remote points to steveyegge/beads.
|
||||||
|
// This handles any remote naming convention (origin, upstream, github, etc.)
|
||||||
|
// and correctly identifies actual beads forks vs user's own projects. (GH#823)
|
||||||
|
func isForkOfBeads(gitRoot string) bool {
|
||||||
|
cmd := exec.Command("git", "-C", gitRoot, "remote", "-v")
|
||||||
|
out, err := cmd.Output()
|
||||||
|
if err != nil {
|
||||||
|
return false // No remotes or git error - not a fork
|
||||||
|
}
|
||||||
|
|
||||||
|
// If any remote URL contains steveyegge/beads, this is a beads-related repo
|
||||||
|
return strings.Contains(string(out), "steveyegge/beads")
|
||||||
|
}
|
||||||
|
|
||||||
|
// isForkProtectionDisabled checks if fork protection is disabled via git config.
|
||||||
|
// Users can opt out with: git config beads.fork-protection false
|
||||||
|
// Only exact "false" disables; any other value or unset means enabled.
|
||||||
|
func isForkProtectionDisabled(gitRoot string) bool {
|
||||||
|
cmd := exec.Command("git", "-C", gitRoot, "config", "--get", "beads.fork-protection")
|
||||||
|
out, err := cmd.Output()
|
||||||
|
if err != nil {
|
||||||
|
return false // Not set or error - default to enabled
|
||||||
|
}
|
||||||
|
return strings.TrimSpace(string(out)) == "false"
|
||||||
|
}
|
||||||
|
|
||||||
// isAlreadyExcluded checks if issues.jsonl is already in the exclude file
|
// isAlreadyExcluded checks if issues.jsonl is already in the exclude file
|
||||||
func isAlreadyExcluded(excludePath string) bool {
|
func isAlreadyExcluded(excludePath string) bool {
|
||||||
content, err := os.ReadFile(excludePath) //nolint:gosec // G304: path is constructed from git root, not user input
|
content, err := os.ReadFile(excludePath) //nolint:gosec // G304: path is constructed from git root, not user input
|
||||||
|
|||||||
@@ -2,11 +2,56 @@ package main
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"os"
|
"os"
|
||||||
|
"os/exec"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// setupGitRepoForForkTest creates a temporary git repository for testing
|
||||||
|
func setupGitRepoForForkTest(t *testing.T) string {
|
||||||
|
t.Helper()
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
// Create .beads directory
|
||||||
|
beadsDir := filepath.Join(dir, ".beads")
|
||||||
|
if err := os.MkdirAll(beadsDir, 0755); err != nil {
|
||||||
|
t.Fatalf("failed to create .beads directory: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Initialize git repo
|
||||||
|
cmd := exec.Command("git", "init", "--initial-branch=main")
|
||||||
|
cmd.Dir = dir
|
||||||
|
if err := cmd.Run(); err != nil {
|
||||||
|
t.Fatalf("failed to init git repo: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Configure git user
|
||||||
|
cmd = exec.Command("git", "config", "user.email", "test@test.com")
|
||||||
|
cmd.Dir = dir
|
||||||
|
_ = cmd.Run()
|
||||||
|
|
||||||
|
cmd = exec.Command("git", "config", "user.name", "Test User")
|
||||||
|
cmd.Dir = dir
|
||||||
|
_ = cmd.Run()
|
||||||
|
|
||||||
|
return dir
|
||||||
|
}
|
||||||
|
|
||||||
|
// addRemote adds a git remote to the test repo
|
||||||
|
func addRemote(t *testing.T, dir, name, url string) {
|
||||||
|
t.Helper()
|
||||||
|
cmd := exec.Command("git", "remote", "add", name, url)
|
||||||
|
cmd.Dir = dir
|
||||||
|
if err := cmd.Run(); err != nil {
|
||||||
|
t.Fatalf("failed to add remote %s: %v", name, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ============================================================================
|
||||||
|
// Test isUpstreamRepo (existing tests, updated)
|
||||||
|
// ============================================================================
|
||||||
|
|
||||||
func TestIsUpstreamRepo(t *testing.T) {
|
func TestIsUpstreamRepo(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
@@ -45,6 +90,118 @@ func TestIsUpstreamRepo(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Test 1: Upstream maintainer (origin = steveyegge/beads)
|
||||||
|
func TestIsUpstreamRepo_Maintainer(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
addRemote(t, dir, "origin", "https://github.com/steveyegge/beads.git")
|
||||||
|
|
||||||
|
if !isUpstreamRepo(dir) {
|
||||||
|
t.Error("expected isUpstreamRepo to return true for steveyegge/beads")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test 1b: Upstream maintainer with SSH URL
|
||||||
|
func TestIsUpstreamRepo_MaintainerSSH(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
addRemote(t, dir, "origin", "git@github.com:steveyegge/beads.git")
|
||||||
|
|
||||||
|
if !isUpstreamRepo(dir) {
|
||||||
|
t.Error("expected isUpstreamRepo to return true for SSH steveyegge/beads")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test isUpstreamRepo with non-beads origin
|
||||||
|
func TestIsUpstreamRepo_NotUpstream(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
addRemote(t, dir, "origin", "https://github.com/peterkc/beads.git")
|
||||||
|
|
||||||
|
if isUpstreamRepo(dir) {
|
||||||
|
t.Error("expected isUpstreamRepo to return false for fork origin")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test isUpstreamRepo with no origin
|
||||||
|
func TestIsUpstreamRepo_NoOrigin(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
// Don't add origin remote
|
||||||
|
|
||||||
|
if isUpstreamRepo(dir) {
|
||||||
|
t.Error("expected isUpstreamRepo to return false when no origin exists")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ============================================================================
|
||||||
|
// Test isForkOfBeads (new tests for GH#823)
|
||||||
|
// ============================================================================
|
||||||
|
|
||||||
|
// Test 2: Fork (standard) - origin=fork, upstream=beads
|
||||||
|
func TestIsForkOfBeads_StandardFork(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
addRemote(t, dir, "origin", "https://github.com/peterkc/beads.git")
|
||||||
|
addRemote(t, dir, "upstream", "https://github.com/steveyegge/beads.git")
|
||||||
|
|
||||||
|
if !isForkOfBeads(dir) {
|
||||||
|
t.Error("expected isForkOfBeads to return true for standard fork setup")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test 3: Fork (custom naming) - origin=fork, github=beads
|
||||||
|
func TestIsForkOfBeads_CustomNaming(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
addRemote(t, dir, "origin", "https://github.com/peterkc/beads.git")
|
||||||
|
addRemote(t, dir, "github", "https://github.com/steveyegge/beads.git")
|
||||||
|
|
||||||
|
if !isForkOfBeads(dir) {
|
||||||
|
t.Error("expected isForkOfBeads to return true for custom remote naming")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test 4: User's own project (no beads remote) - THE BUG CASE
|
||||||
|
func TestIsForkOfBeads_UserProject(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
addRemote(t, dir, "origin", "https://github.com/mycompany/myapp.git")
|
||||||
|
|
||||||
|
if isForkOfBeads(dir) {
|
||||||
|
t.Error("expected isForkOfBeads to return false for user's own project")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test 5: User's project with unrelated upstream - THE BUG CASE
|
||||||
|
func TestIsForkOfBeads_UserProjectWithUpstream(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
addRemote(t, dir, "origin", "https://github.com/mycompany/myapp.git")
|
||||||
|
addRemote(t, dir, "upstream", "https://github.com/other/repo.git")
|
||||||
|
|
||||||
|
if isForkOfBeads(dir) {
|
||||||
|
t.Error("expected isForkOfBeads to return false for user's project with unrelated upstream")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test 6: No remotes
|
||||||
|
func TestIsForkOfBeads_NoRemotes(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
// Don't add any remotes
|
||||||
|
|
||||||
|
if isForkOfBeads(dir) {
|
||||||
|
t.Error("expected isForkOfBeads to return false when no remotes exist")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test SSH URL detection
|
||||||
|
func TestIsForkOfBeads_SSHRemote(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
addRemote(t, dir, "origin", "git@github.com:peterkc/beads.git")
|
||||||
|
addRemote(t, dir, "upstream", "git@github.com:steveyegge/beads.git")
|
||||||
|
|
||||||
|
if !isForkOfBeads(dir) {
|
||||||
|
t.Error("expected isForkOfBeads to return true for SSH upstream")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ============================================================================
|
||||||
|
// Test isAlreadyExcluded (existing tests)
|
||||||
|
// ============================================================================
|
||||||
|
|
||||||
func TestIsAlreadyExcluded(t *testing.T) {
|
func TestIsAlreadyExcluded(t *testing.T) {
|
||||||
// Create temp file with exclusion
|
// Create temp file with exclusion
|
||||||
tmpDir := t.TempDir()
|
tmpDir := t.TempDir()
|
||||||
@@ -72,6 +229,10 @@ func TestIsAlreadyExcluded(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ============================================================================
|
||||||
|
// Test addToExclude (existing tests)
|
||||||
|
// ============================================================================
|
||||||
|
|
||||||
func TestAddToExclude(t *testing.T) {
|
func TestAddToExclude(t *testing.T) {
|
||||||
tmpDir := t.TempDir()
|
tmpDir := t.TempDir()
|
||||||
infoDir := filepath.Join(tmpDir, ".git", "info")
|
infoDir := filepath.Join(tmpDir, ".git", "info")
|
||||||
@@ -111,3 +272,64 @@ func TestAddToExclude(t *testing.T) {
|
|||||||
t.Errorf("exclude file missing .beads/issues.jsonl: %s", content)
|
t.Errorf("exclude file missing .beads/issues.jsonl: %s", content)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ============================================================================
|
||||||
|
// Test isForkProtectionDisabled (git config opt-out)
|
||||||
|
// ============================================================================
|
||||||
|
|
||||||
|
// Test isForkProtectionDisabled with various git config values
|
||||||
|
func TestIsForkProtectionDisabled(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
config string // value to set, empty = don't set
|
||||||
|
expected bool
|
||||||
|
}{
|
||||||
|
{"not set", "", false},
|
||||||
|
{"set to false", "false", true},
|
||||||
|
{"set to true", "true", false},
|
||||||
|
{"set to other", "disabled", false}, // only "false" disables
|
||||||
|
{"set to FALSE", "FALSE", false}, // case-sensitive
|
||||||
|
{"set to 0", "0", false}, // only "false" disables
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
|
||||||
|
if tt.config != "" {
|
||||||
|
cmd := exec.Command("git", "-C", dir, "config", "beads.fork-protection", tt.config)
|
||||||
|
if err := cmd.Run(); err != nil {
|
||||||
|
t.Fatalf("failed to set git config: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
result := isForkProtectionDisabled(dir)
|
||||||
|
if result != tt.expected {
|
||||||
|
t.Errorf("isForkProtectionDisabled() = %v, want %v (config=%q)", result, tt.expected, tt.config)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test 8: Config opt-out via git config (replaces YAML config)
|
||||||
|
func TestConfigOptOut_GitConfig(t *testing.T) {
|
||||||
|
dir := setupGitRepoForForkTest(t)
|
||||||
|
addRemote(t, dir, "origin", "https://github.com/peterkc/beads.git")
|
||||||
|
addRemote(t, dir, "upstream", "https://github.com/steveyegge/beads.git")
|
||||||
|
|
||||||
|
// Verify this IS a fork of beads
|
||||||
|
if !isForkOfBeads(dir) {
|
||||||
|
t.Fatal("expected isForkOfBeads to return true for test setup")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Set git config opt-out
|
||||||
|
cmd := exec.Command("git", "-C", dir, "config", "beads.fork-protection", "false")
|
||||||
|
if err := cmd.Run(); err != nil {
|
||||||
|
t.Fatalf("failed to set git config: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify opt-out is detected
|
||||||
|
if !isForkProtectionDisabled(dir) {
|
||||||
|
t.Error("expected isForkProtectionDisabled to return true after setting git config")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -38,8 +38,8 @@ var YamlOnlyKeys = map[string]bool{
|
|||||||
// Git settings
|
// Git settings
|
||||||
"git.author": true,
|
"git.author": true,
|
||||||
"git.no-gpg-sign": true,
|
"git.no-gpg-sign": true,
|
||||||
"no-push": true,
|
"no-push": true,
|
||||||
"no-git-ops": true, // Disable git ops in bd prime session close protocol (GH#593)
|
"no-git-ops": true, // Disable git ops in bd prime session close protocol (GH#593)
|
||||||
|
|
||||||
// Sync settings
|
// Sync settings
|
||||||
"sync-branch": true,
|
"sync-branch": true,
|
||||||
|
|||||||
Reference in New Issue
Block a user