From 38a0892051bb21d151d6fbdccbbfed217fc2edd0 Mon Sep 17 00:00:00 2001 From: neil Date: Sun, 21 Jun 2026 15:08:51 +0800 Subject: [PATCH 1/2] fix(push): defer branch protection to remote --- cmd/push.go | 8 ++--- internal/daemon/git_handler.go | 12 -------- internal/daemon/git_handler_test.go | 47 +++++------------------------ 3 files changed, 11 insertions(+), 56 deletions(-) diff --git a/cmd/push.go b/cmd/push.go index 41f64e69..e0ac7229 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -21,15 +21,13 @@ The daemon injects credentials — workers don't need tokens in their environmen Equivalent to "git push -u origin " but credential-safe for worker sessions. -Pushes to main or master are blocked unconditionally — all changes must go through -a feature branch and PR. +Branch protection is enforced by the remote repository, not by ttal. With --force, performs a --force-with-lease push (raw --force is never used). -Force-push is also blocked on main/master. Examples: ttal push - ttal push --force # force-with-lease; blocked on main/master`, + ttal push --force # force-with-lease`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { ctx, err := pr.ResolveContextWithoutProvider() @@ -86,6 +84,6 @@ func currentBranch(workDir string) (string, error) { } func init() { - pushCmd.Flags().Bool("force", false, "Force push with --force-with-lease (blocked on main/master)") + pushCmd.Flags().Bool("force", false, "Force push with --force-with-lease") rootCmd.AddCommand(pushCmd) } diff --git a/internal/daemon/git_handler.go b/internal/daemon/git_handler.go index a51148bf..84f3c2d2 100644 --- a/internal/daemon/git_handler.go +++ b/internal/daemon/git_handler.go @@ -18,25 +18,13 @@ import ( var gitCommandContext = exec.CommandContext -// isProtectedBranch returns true if the given branch name is protected by policy. -// The list is intentionally small — extending it requires a code change. -func isProtectedBranch(branch string) bool { - return branch == "main" || branch == "master" -} - // handleGitPush executes a git push using daemon-held credentials. // WorkDir may be a ttal worktree or any registered project directory. // Credentials are injected via GIT_CONFIG env vars — never via URL embedding or keychain. func handleGitPush(req GitPushRequest) GitPushResponse { - // Validation order: empty branch → protected-branch policy → credentials if req.Branch == "" { return GitPushResponse{Error: "branch must not be empty"} } - if isProtectedBranch(req.Branch) { - return GitPushResponse{ - Error: fmt.Sprintf("push to %s blocked — use a feature branch and PR", req.Branch), - } - } // Detect remote URL to pick the right token. remoteURL, err := gitutil.RemoteURL(req.WorkDir) diff --git a/internal/daemon/git_handler_test.go b/internal/daemon/git_handler_test.go index 3d8d5390..5837f824 100644 --- a/internal/daemon/git_handler_test.go +++ b/internal/daemon/git_handler_test.go @@ -488,56 +488,25 @@ func runGitTestCmd(t *testing.T, workDir string, args ...string) { } } -func TestHandleGitPush_ForceOnProtectedBranchBlocked(t *testing.T) { +func TestHandleGitPush_AllowsMainAndMaster(t *testing.T) { for _, branch := range []string{"main", "master"} { t.Run(branch, func(t *testing.T) { + repo := initRepoWithRemote(t) + runGitTestCmd(t, repo, "switch", "-C", branch) + t.Setenv("FORGEJO_TOKEN", "test-token") + resp := handleGitPush(GitPushRequest{ - WorkDir: "/tmp/whatever", // never reached + WorkDir: repo, Branch: branch, - Force: true, }) if resp.OK { - t.Fatalf("expected OK=false for force push to %s", branch) - } - wantErr := fmt.Sprintf("push to %s blocked — use a feature branch and PR", branch) - if resp.Error != wantErr { - t.Errorf("error = %q, want %q", resp.Error, wantErr) + return } + t.Fatalf("expected push to %s to reach git push, got error: %s", branch, resp.Error) }) } } -func TestIsProtectedBranch(t *testing.T) { - protected := []string{"main", "master"} - allowed := []string{"develop", "feature/x", "release/v1", ""} - - for _, b := range protected { - if !isProtectedBranch(b) { - t.Errorf("isProtectedBranch(%q) = false, want true", b) - } - } - for _, b := range allowed { - if isProtectedBranch(b) { - t.Errorf("isProtectedBranch(%q) = true, want false", b) - } - } -} - -func TestHandleGitPush_NormalPushToMainBlockedByPolicy(t *testing.T) { - resp := handleGitPush(GitPushRequest{ - WorkDir: "/tmp/not-a-real-repo", // will fail at RemoteURL if policy guard is bypassed - Branch: "main", - Force: false, - }) - if resp.OK { - t.Fatal("expected push to main to be blocked regardless of force flag") - } - policyErr := "push to main blocked — use a feature branch and PR" - if resp.Error != policyErr { - t.Errorf("expected policy error, got: %q", resp.Error) - } -} - func TestHandleGitPush_ForceWithEmptyBranchReturnsEmptyError(t *testing.T) { resp := handleGitPush(GitPushRequest{ WorkDir: "/tmp/whatever", From b01cded1579ceb401c2a27f0c1f5fceee57dbe2a Mon Sep 17 00:00:00 2001 From: neil Date: Sun, 21 Jun 2026 15:17:15 +0800 Subject: [PATCH 2/2] test(push): assert main push bypasses local guard --- internal/daemon/git_handler_test.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/internal/daemon/git_handler_test.go b/internal/daemon/git_handler_test.go index 5837f824..17207bb3 100644 --- a/internal/daemon/git_handler_test.go +++ b/internal/daemon/git_handler_test.go @@ -491,18 +491,17 @@ func runGitTestCmd(t *testing.T, workDir string, args ...string) { func TestHandleGitPush_AllowsMainAndMaster(t *testing.T) { for _, branch := range []string{"main", "master"} { t.Run(branch, func(t *testing.T) { - repo := initRepoWithRemote(t) - runGitTestCmd(t, repo, "switch", "-C", branch) - t.Setenv("FORGEJO_TOKEN", "test-token") - resp := handleGitPush(GitPushRequest{ - WorkDir: repo, + WorkDir: "/tmp/not-a-real-repo", Branch: branch, }) - if resp.OK { - return + oldPolicyErr := fmt.Sprintf("push to %s blocked — use a feature branch and PR", branch) + if resp.Error == oldPolicyErr { + t.Fatalf("expected push to %s to bypass local policy guard, got old policy error", branch) + } + if !strings.Contains(resp.Error, "get remote URL") { + t.Fatalf("expected push to %s to reach remote resolution, got error: %s", branch, resp.Error) } - t.Fatalf("expected push to %s to reach git push, got error: %s", branch, resp.Error) }) } }