krz/gitbay

policy, control, web, wiki: require-mr makes protected branches merge-only !339

merged merged by cmc on 2026-09-08 02:35 UTC · krz/gitbay:require-mr into main

11 files changed, +165 −2

Layout: unified · split

.gitbay/wiki/Users.org +8
@@ -192,6 +192,7 @@ Access and settings (owner or =admin= grant):
192gitbay repo access grant you/project alice write # read | write | admin 192gitbay repo access grant you/project alice write # read | write | admin
193gitbay repo access revoke you/project alice 193gitbay repo access revoke you/project alice
194gitbay repo settings protect you/project main # no force-push, no delete 194gitbay repo settings protect you/project main # no force-push, no delete
195gitbay repo settings require-mr you/project on # protected branches: merge requests only
195gitbay repo settings default-branch you/project trunk # HEAD, and what the web shows 196gitbay repo settings default-branch you/project trunk # HEAD, and what the web shows
196gitbay repo settings require-signed you/project on # every commit must verify 197gitbay repo settings require-signed you/project on # every commit must verify
197gitbay repo settings git-daemon you/project on # expose over git:// 198gitbay repo settings git-daemon you/project on # expose over git://
@@ -397,6 +398,13 @@ to ask without it gating merges; with it on and no file on the target
397branch, the merge is refused and says so. It does not wait on 398branch, the merge is refused and says so. It does not wait on
398=require-approvals=. 399=require-approvals=.
399 400
401Those gates apply to =mr merge=. A direct push to a protected branch
402passes none of them until =require-mr on=: then an existing protected
403branch refuses every push, including =repo commit-file= and the web
404editor, and the server's merge is its only writer. Creating the branch
405is still a push, since there is nothing to open a merge request against
406yet.
407
400A merge request whose target is another open merge request's source 408A merge request whose target is another open merge request's source
401branch is stacked on it: =mr create= says so, =mr show= carries 409branch is stacked on it: =mr create= says so, =mr show= carries
402=stacked_on= and =stacked=, and merging the lower one retargets the 410=stacked_on= and =stacked=, and merging the lower one retargets the
cmd/gitbay/main.go +1
@@ -479,6 +479,7 @@ func repoCmd() *cobra.Command {
479 pass("require-checks", "gate merges on green statuses: ... on|off", passOpts{server: []string{"repo", "settings", "require-checks"}, needsRepo: true}), 479 pass("require-checks", "gate merges on green statuses: ... on|off", passOpts{server: []string{"repo", "settings", "require-checks"}, needsRepo: true}),
480 pass("visibility", "set repository visibility: public|private", passOpts{server: []string{"repo", "settings", "visibility"}, needsRepo: true}), 480 pass("visibility", "set repository visibility: public|private", passOpts{server: []string{"repo", "settings", "visibility"}, needsRepo: true}),
481 pass("require-signed", "require verified commit signatures: ... on|off", passOpts{server: []string{"repo", "settings", "require-signed"}, needsRepo: true}), 481 pass("require-signed", "require verified commit signatures: ... on|off", passOpts{server: []string{"repo", "settings", "require-signed"}, needsRepo: true}),
482 pass("require-mr", "protected branches take changes through merge requests only: on|off", passOpts{server: []string{"repo", "settings", "require-mr"}, needsRepo: true}),
482 pass("description", "set the repository description: <text>", passOpts{server: []string{"repo", "settings", "description"}, needsRepo: true}), 483 pass("description", "set the repository description: <text>", passOpts{server: []string{"repo", "settings", "description"}, needsRepo: true}),
483 pass("website", "set the repository website: <url> ('' clears)", passOpts{server: []string{"repo", "settings", "website"}, needsRepo: true}), 484 pass("website", "set the repository website: <url> ('' clears)", passOpts{server: []string{"repo", "settings", "website"}, needsRepo: true}),
484 pass("git-daemon", "expose over git://: ... on|off", passOpts{server: []string{"repo", "settings", "git-daemon"}, needsRepo: true}), 485 pass("git-daemon", "expose over git://: ... on|off", passOpts{server: []string{"repo", "settings", "git-daemon"}, needsRepo: true}),
e2e/requiremr_test.go added +83
@@ -0,0 +1,83 @@
1package e2e
2
3import (
4 "os"
5 "path/filepath"
6 "strings"
7 "testing"
8)
9
10// With require-mr on, a protected branch takes changes through mr merge
11// only: direct pushes and commit-file are refused, a branch that does
12// not exist yet can still be created, and unprotected branches are
13// unaffected (#197).
14func TestRequireMR(t *testing.T) {
15 inst := startInstance(t)
16 aliceKey := inst.newKey(t, "alice")
17 inst.admin(t, "admin", "user", "create", "alice",
18 "--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
19 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/lib"); code != 0 {
20 t.Fatalf("repo create: %s", errOut)
21 }
22 env := inst.gitEnv(aliceKey)
23 work := t.TempDir()
24 mustGit(t, work, env, "clone", "-q", inst.sshURL("alice/lib"), "lib")
25 dir := filepath.Join(work, "lib")
26 write := func(name, content string) {
27 t.Helper()
28 if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
29 t.Fatal(err)
30 }
31 mustGit(t, dir, env, "add", name)
32 mustGit(t, dir, env, "commit", "-q", "-m", name)
33 }
34 mustGit(t, dir, env, "checkout", "-q", "-b", "main")
35 write("README", "one\n")
36 mustGit(t, dir, env, "push", "-q", "origin", "main")
37
38 for _, args := range [][]string{
39 {"repo", "settings", "protect", "alice/lib", "main"},
40 {"repo", "settings", "protect", "alice/lib", "release"},
41 {"repo", "settings", "require-mr", "alice/lib", "on"},
42 } {
43 if _, errOut, code := inst.ssh(t, aliceKey, "", args...); code != 0 {
44 t.Fatalf("%v: %s", args, errOut)
45 }
46 }
47 out, _, _ := inst.ssh(t, aliceKey, "", "repo", "settings", "show", "alice/lib", "--json")
48 if !strings.Contains(out, `"require_mr":true`) {
49 t.Fatalf("settings show: %s", out)
50 }
51
52 // Direct push to main: refused, with the reason.
53 write("two", "two\n")
54 if out, code := gitRun(t, dir, env, "push", "origin", "main"); code == 0 || !strings.Contains(out, "merge requests only") {
55 t.Fatalf("direct push to protected branch: %d\n%s", code, out)
56 }
57 if _, errOut, code := inst.ssh(t, aliceKey, "x\n", "repo", "commit-file", "alice/lib", "notes.txt",
58 "--ref", "main", "--file", "-"); code != 4 || !strings.Contains(errOut, "merge requests only") {
59 t.Fatalf("commit-file to protected branch: %d %s", code, errOut)
60 }
61 // Creating a protected branch is still a push; so is an unprotected one.
62 mustGit(t, dir, env, "push", "-q", "origin", "main:release")
63 mustGit(t, dir, env, "push", "-q", "origin", "main:feature")
64
65 // The same change lands through a merge request.
66 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/lib",
67 "--source", "feature", "--target", "main", "--title", "'two'"); code != 0 {
68 t.Fatalf("mr create: %s", errOut)
69 }
70 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/lib", "1", "--strategy", "ff"); code != 0 {
71 t.Fatalf("mr merge: %s", errOut)
72 }
73 if out, _, _ := inst.ssh(t, aliceKey, "", "repo", "log", "alice/lib", "--limit", "1"); !strings.Contains(out, "two") {
74 t.Fatalf("merge did not move main: %s", out)
75 }
76
77 // Off again, and the push goes through.
78 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-mr", "alice/lib", "off"); code != 0 {
79 t.Fatalf("require-mr off: %s", errOut)
80 }
81 write("three", "three\n")
82 mustGit(t, dir, env, "push", "-q", "origin", "main")
83}
internal/control/commitfile.go +4
@@ -3,6 +3,7 @@ package control
3import ( 3import (
4 "fmt" 4 "fmt"
5 "io" 5 "io"
6 "slices"
6 "strings" 7 "strings"
7 8
8 "gitbay.org/gitbay/internal/gitutil" 9 "gitbay.org/gitbay/internal/gitutil"
@@ -55,6 +56,9 @@ func runCommitFile(c *Ctx, args []string) int {
55 if !ok || filePath == "" { 56 if !ok || filePath == "" {
56 return c.fail(protocol.ExitUsage, "path must stay inside the repository") 57 return c.fail(protocol.ExitUsage, "path must stay inside the repository")
57 } 58 }
59 if repo.Settings.RequireMR && slices.Contains(repo.Settings.ProtectedBranches, ref) {
60 return c.fail(protocol.ExitDenied, "branch %s accepts changes through merge requests only", ref)
61 }
58 // The server authors this commit, so it cannot sign it. 62 // The server authors this commit, so it cannot sign it.
59 if repo.Settings.RequireSignedCommits { 63 if repo.Settings.RequireSignedCommits {
60 return c.fail(protocol.ExitDenied, 64 return c.fail(protocol.ExitDenied,
internal/control/mr.go +20
@@ -32,6 +32,9 @@ func init() {
32 register(Command{Path: []string{"repo", "settings", "require-checks"}, 32 register(Command{Path: []string{"repo", "settings", "require-checks"},
33 Summary: "gate merges on green statuses", 33 Summary: "gate merges on green statuses",
34 Usage: "repo settings require-checks <owner/name> on|off", Run: runRequireChecks}) 34 Usage: "repo settings require-checks <owner/name> on|off", Run: runRequireChecks})
35 register(Command{Path: []string{"repo", "settings", "require-mr"},
36 Summary: "protected branches take changes through merge requests only",
37 Usage: "repo settings require-mr <owner/name> on|off", Run: runRequireMR})
35 register(Command{Path: []string{"repo", "settings", "require-signed"}, 38 register(Command{Path: []string{"repo", "settings", "require-signed"},
36 Summary: "require verified commit signatures", 39 Summary: "require verified commit signatures",
37 Usage: "repo settings require-signed <owner/name> on|off", Run: runRequireSigned}) 40 Usage: "repo settings require-signed <owner/name> on|off", Run: runRequireSigned})
@@ -217,6 +220,23 @@ func runRequireChecks(c *Ctx, args []string) int {
217 }) 220 })
218} 221}
219 222
223func runRequireMR(c *Ctx, args []string) int {
224 if len(args) != 2 || (args[1] != "on" && args[1] != "off") {
225 return c.fail(protocol.ExitUsage, "usage: repo settings require-mr <owner/name> on|off")
226 }
227 repo, code := resolveRepo(c, args[0], policy.CanAdmin)
228 if code >= 0 {
229 return code
230 }
231 s, err := c.Store.UpdateRepoSettings(repo.ID, func(s *store.RepoSettings) { s.RequireMR = args[1] == "on" })
232 if err != nil {
233 return c.fail(protocol.ExitFailure, "%v", err)
234 }
235 return c.emit(s, func(w io.Writer) {
236 fmt.Fprintf(w, "require_mr %s on %s\n", args[1], repo.Path())
237 })
238}
239
220func runRequireSigned(c *Ctx, args []string) int { 240func runRequireSigned(c *Ctx, args []string) int {
221 if len(args) != 2 || (args[1] != "on" && args[1] != "off") { 241 if len(args) != 2 || (args[1] != "on" && args[1] != "off") {
222 return c.fail(protocol.ExitUsage, "usage: repo settings require-signed <owner/name> on|off") 242 return c.fail(protocol.ExitUsage, "usage: repo settings require-signed <owner/name> on|off")
internal/control/repo.go +2 −2
@@ -606,8 +606,8 @@ func runSettingsShow(c *Ctx, args []string) int {
606 return code 606 return code
607 } 607 }
608 return c.emit(repo.Settings, func(w io.Writer) { 608 return c.emit(repo.Settings, func(w io.Writer) {
609 fmt.Fprintf(w, "protected_branches: %s\nrequire_signed_commits: %v\ngit_daemon: %v\narchived: %v\n", 609 fmt.Fprintf(w, "protected_branches: %s\nrequire_mr: %v\nrequire_signed_commits: %v\ngit_daemon: %v\narchived: %v\n",
610 strings.Join(repo.Settings.ProtectedBranches, ", "), repo.Settings.RequireSignedCommits, repo.Settings.GitDaemon, repo.Settings.Archived) 610 strings.Join(repo.Settings.ProtectedBranches, ", "), repo.Settings.RequireMR, repo.Settings.RequireSignedCommits, repo.Settings.GitDaemon, repo.Settings.Archived)
611 }) 611 })
612} 612}
613 613
internal/httpd/settings.go +2
@@ -78,6 +78,8 @@ func (s *Server) settingsSubmit(w http.ResponseWriter, r *http.Request, u store.
78 argv = []string{"repo", "settings", "require-resolved", repo, onOff(v("require-resolved"))} 78 argv = []string{"repo", "settings", "require-resolved", repo, onOff(v("require-resolved"))}
79 case "require-codeowners": 79 case "require-codeowners":
80 argv = []string{"repo", "settings", "require-codeowners", repo, onOff(v("require-codeowners"))} 80 argv = []string{"repo", "settings", "require-codeowners", repo, onOff(v("require-codeowners"))}
81 case "require-mr":
82 argv = []string{"repo", "settings", "require-mr", repo, onOff(v("require-mr"))}
81 case "require-signed": 83 case "require-signed":
82 argv = []string{"repo", "settings", "require-signed", repo, onOff(v("require-signed"))} 84 argv = []string{"repo", "settings", "require-signed", repo, onOff(v("require-signed"))}
83 case "require-approvals": 85 case "require-approvals":
internal/policy/access.go +8
@@ -106,7 +106,15 @@ func CheckPush(repo store.Repo, updates []RefUpdate) string {
106 if u.IsForce { 106 if u.IsForce {
107 return "branch " + branch + " is protected: force-push refused" 107 return "branch " + branch + " is protected: force-push refused"
108 } 108 }
109 // Under require_mr the server's merge is the only writer of an
110 // existing protected branch. Creating one is still a push:
111 // there is nothing to route a merge request into yet.
112 if repo.Settings.RequireMR && !isZeroSHA(u.Old) {
113 return "branch " + branch + " accepts changes through merge requests only"
114 }
109 } 115 }
110 } 116 }
111 return "" 117 return ""
112} 118}
119
120func isZeroSHA(sha string) bool { return sha != "" && strings.Trim(sha, "0") == "" }
internal/policy/access_test.go +29
@@ -1,6 +1,7 @@
1package policy 1package policy
2 2
3import ( 3import (
4 "strings"
4 "testing" 5 "testing"
5 6
6 "gitbay.org/gitbay/internal/store" 7 "gitbay.org/gitbay/internal/store"
@@ -110,3 +111,31 @@ func TestCheckPush(t *testing.T) {
110 }) 111 })
111 } 112 }
112} 113}
114
115// With require_mr, a protected branch takes no direct push once it
116// exists; creating it and pushing elsewhere are unaffected (#197).
117func TestCheckPushRequireMR(t *testing.T) {
118 repo := store.Repo{Settings: store.RepoSettings{ProtectedBranches: []string{"main"}, RequireMR: true}}
119 const zero = "0000000000000000000000000000000000000000"
120 cases := []struct {
121 name string
122 updates []RefUpdate
123 denied bool
124 }{
125 {"update protected", []RefUpdate{{Ref: "refs/heads/main", Old: "abc", New: "def"}}, true},
126 {"create protected", []RefUpdate{{Ref: "refs/heads/main", Old: zero, New: "def"}}, false},
127 {"delete protected", []RefUpdate{{Ref: "refs/heads/main", Old: "abc", New: zero, IsDelete: true}}, true},
128 {"update unprotected", []RefUpdate{{Ref: "refs/heads/dev", Old: "abc", New: "def"}}, false},
129 }
130 for _, tc := range cases {
131 t.Run(tc.name, func(t *testing.T) {
132 msg := CheckPush(repo, tc.updates)
133 if (msg != "") != tc.denied {
134 t.Errorf("CheckPush = %q, denied should be %v", msg, tc.denied)
135 }
136 })
137 }
138 if msg := CheckPush(repo, cases[0].updates); !strings.Contains(msg, "merge requests only") {
139 t.Errorf("message %q", msg)
140 }
141}
internal/store/repos.go +1
@@ -28,6 +28,7 @@ type RepoSettings struct {
28 RequireApprovals int `json:"require_approvals,omitempty"` 28 RequireApprovals int `json:"require_approvals,omitempty"`
29 RequireResolved bool `json:"require_resolved,omitempty"` 29 RequireResolved bool `json:"require_resolved,omitempty"`
30 RequireCodeowners bool `json:"require_codeowners,omitempty"` 30 RequireCodeowners bool `json:"require_codeowners,omitempty"`
31 RequireMR bool `json:"require_mr,omitempty"`
31 GitDaemon bool `json:"git_daemon,omitempty"` 32 GitDaemon bool `json:"git_daemon,omitempty"`
32 Archived bool `json:"archived,omitempty"` 33 Archived bool `json:"archived,omitempty"`
33 Website string `json:"website,omitempty"` 34 Website string `json:"website,omitempty"`
internal/web/templates/settings.html +7
@@ -105,6 +105,13 @@
105 </select> 105 </select>
106 <button type="submit">Protect</button> 106 <button type="submit">Protect</button>
107</form> 107</form>
108<form method="post" action="{{$base}}" class="setform">
109 <input type="hidden" name="field" value="require-mr">
110 <label for="require-mr">Merge requests only</label>
111 <input type="checkbox" id="require-mr" name="require-mr" value="on"{{if .Repo.Settings.RequireMR}} checked{{end}}>
112 <button type="submit">Save</button>
113</form>
114<p class="meta">With merge requests only, a protected branch refuses every direct push once it exists; the merge gates above are then what a change has to pass.</p>
108 115
109<h2>Dependencies</h2> 116<h2>Dependencies</h2>
110<form method="post" action="{{$base}}" class="setform"> 117<form method="post" action="{{$base}}" class="setform">