krz/gitbay

A CLI-first git forge. cli forge git self-hosted https://gitbay.org

Commit 1f8076c655

1f8076c65598ca4a46c345c01507d460dd87ee15

parent: 251a71053c

Verified · cmc ci/build: success ci/test: success ci/vuln: success

cmc <hello@cleberg.net> · 2026-09-04 14:06 UTC 

merge gates: CODEOWNERS is a setting, not a file that happens to exist

Every other merge requirement is something an admin turned on.
CODEOWNERS gated as soon as the file existed, so a repository that added
one to record who to ask about what acquired a merge gate it never asked
for, with no way to keep the file and drop the gate.

`require_codeowners` joins the other four in RepoSettings, with
`repo settings require-codeowners <owner/name> on|off` and a checkbox on
the settings page. Off by default. It still does not wait on
require_approvals (#99).

With the toggle on and no CODEOWNERS file on the target branch, the
merge is refused and says which branch is missing it — the alternative
is a gate that quietly enforces nothing.

Existing repositories with a CODEOWNERS file lose the gate until the
toggle is set: the server cannot know from the database alone which
branches carry the file, so nothing back-fills it.

Closes #142

Layout: unified · split

cmd/gitbay/main.go +1
@@ -434,6 +434,7 @@ func repoCmd() *cobra.Command {
434 pass("unprotect", "unprotect a branch", passOpts{server: []string{"repo", "settings", "unprotect"}, needsRepo: true}), 434 pass("unprotect", "unprotect a branch", passOpts{server: []string{"repo", "settings", "unprotect"}, needsRepo: true}),
435 pass("require-approvals", "require N fresh approvals to merge: <n>", passOpts{server: []string{"repo", "settings", "require-approvals"}, needsRepo: true}), 435 pass("require-approvals", "require N fresh approvals to merge: <n>", passOpts{server: []string{"repo", "settings", "require-approvals"}, needsRepo: true}),
436 pass("require-resolved", "require threads resolved to merge: on|off", passOpts{server: []string{"repo", "settings", "require-resolved"}, needsRepo: true}), 436 pass("require-resolved", "require threads resolved to merge: on|off", passOpts{server: []string{"repo", "settings", "require-resolved"}, needsRepo: true}),
437 pass("require-codeowners", "require an owner's approval per covered file: on|off", passOpts{server: []string{"repo", "settings", "require-codeowners"}, needsRepo: true}),
437 pass("require-checks", "gate merges on green statuses: ... on|off", passOpts{server: []string{"repo", "settings", "require-checks"}, needsRepo: true}), 438 pass("require-checks", "gate merges on green statuses: ... on|off", passOpts{server: []string{"repo", "settings", "require-checks"}, needsRepo: true}),
438 pass("visibility", "set repository visibility: public|private", passOpts{server: []string{"repo", "settings", "visibility"}, needsRepo: true}), 439 pass("visibility", "set repository visibility: public|private", passOpts{server: []string{"repo", "settings", "visibility"}, needsRepo: true}),
439 pass("require-signed", "require verified commit signatures: ... on|off", passOpts{server: []string{"repo", "settings", "require-signed"}, needsRepo: true}), 440 pass("require-signed", "require verified commit signatures: ... on|off", passOpts{server: []string{"repo", "settings", "require-signed"}, needsRepo: true}),
e2e/approvals_test.go +50 −7
@@ -52,6 +52,9 @@ func TestMergeRequirements(t *testing.T) {
52 if _, _, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-approvals", "alice/svc", "1"); code != 0 { 52 if _, _, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-approvals", "alice/svc", "1"); code != 0 {
53 t.Fatal("require-approvals failed") 53 t.Fatal("require-approvals failed")
54 } 54 }
55 if _, _, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-codeowners", "alice/svc", "on"); code != 0 {
56 t.Fatal("require-codeowners failed")
57 }
55 58
56 // No approvals: refused. The author's own approval does not count. 59 // No approvals: refused. The author's own approval does not count.
57 _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1") 60 _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1")
@@ -137,10 +140,11 @@ func TestMergeRequirements(t *testing.T) {
137 } 140 }
138} 141}
139 142
140// A CODEOWNERS file gates on its own. It used to be read only inside the 143// require_codeowners is the opt-in, not the file's presence: a repository
141// require-approvals branch, so a repository with owners and the default 144// can carry CODEOWNERS as documentation of who to ask without it gating
142// settings had no owner gating at all (#99). 145// merges. When it is on, it gates independently of require_approvals —
143func TestCodeownersWithoutRequiredApprovals(t *testing.T) { 146// the coupling that left owners unenforced under default settings (#99).
147func TestCodeownersToggle(t *testing.T) {
144 inst := startInstance(t) 148 inst := startInstance(t)
145 aliceKey := inst.newKey(t, "alice") 149 aliceKey := inst.newKey(t, "alice")
146 carolKey := inst.newKey(t, "carol") 150 carolKey := inst.newKey(t, "carol")
@@ -158,14 +162,15 @@ func TestCodeownersWithoutRequiredApprovals(t *testing.T) {
158 dir := filepath.Join(work, "w") 162 dir := filepath.Join(work, "w")
159 os.WriteFile(filepath.Join(dir, "CODEOWNERS"), []byte("*.go @carol\n"), 0o644) 163 os.WriteFile(filepath.Join(dir, "CODEOWNERS"), []byte("*.go @carol\n"), 0o644)
160 os.WriteFile(filepath.Join(dir, "svc.go"), []byte("package svc\n"), 0o644) 164 os.WriteFile(filepath.Join(dir, "svc.go"), []byte("package svc\n"), 0o644)
165 os.WriteFile(filepath.Join(dir, "lib.go"), []byte("package svc\n"), 0o644)
161 os.WriteFile(filepath.Join(dir, "README"), []byte("svc\n"), 0o644) 166 os.WriteFile(filepath.Join(dir, "README"), []byte("svc\n"), 0o644)
162 mustGit(t, dir, env, "checkout", "-q", "-b", "main") 167 mustGit(t, dir, env, "checkout", "-q", "-b", "main")
163 mustGit(t, dir, env, "add", ".") 168 mustGit(t, dir, env, "add", ".")
164 mustGit(t, dir, env, "commit", "-q", "-m", "base") 169 mustGit(t, dir, env, "commit", "-q", "-m", "base")
165 mustGit(t, dir, env, "push", "-q", "origin", "main") 170 mustGit(t, dir, env, "push", "-q", "origin", "main")
166 171
167 // One MR touches an owned file, one does not. 172 // !1 and !3 touch owned files, !2 does not.
168 for i, f := range []string{"svc.go", "README"} { 173 for i, f := range []string{"svc.go", "README", "lib.go"} {
169 mustGit(t, dir, env, "checkout", "-q", "-b", fmt.Sprintf("feat%d", i), "main") 174 mustGit(t, dir, env, "checkout", "-q", "-b", fmt.Sprintf("feat%d", i), "main")
170 os.WriteFile(filepath.Join(dir, f), []byte("changed\n"), 0o644) 175 os.WriteFile(filepath.Join(dir, f), []byte("changed\n"), 0o644)
171 mustGit(t, dir, env, "add", ".") 176 mustGit(t, dir, env, "add", ".")
@@ -177,7 +182,17 @@ func TestCodeownersWithoutRequiredApprovals(t *testing.T) {
177 } 182 }
178 } 183 }
179 184
180 // Default settings, no approvals: the owned file is gated, the other is not. 185 // Toggle off, which is the default: the file is present and gates
186 // nothing, so an owned file merges without its owner.
187 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "3"); code != 0 {
188 t.Fatalf("owned file gated with the toggle off: %s", errOut)
189 }
190
191 // Toggle on with require_approvals still 0: the owned file is gated,
192 // the unowned one is not.
193 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-codeowners", "alice/svc", "on"); code != 0 {
194 t.Fatalf("require-codeowners: %s", errOut)
195 }
181 _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1") 196 _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1")
182 if code != 4 || !strings.Contains(errOut, "CODEOWNERS") || !strings.Contains(errOut, "carol") { 197 if code != 4 || !strings.Contains(errOut, "CODEOWNERS") || !strings.Contains(errOut, "carol") {
183 t.Fatalf("codeowners gate with require-approvals off: exit %d, %s", code, errOut) 198 t.Fatalf("codeowners gate with require-approvals off: exit %d, %s", code, errOut)
@@ -191,4 +206,32 @@ func TestCodeownersWithoutRequiredApprovals(t *testing.T) {
191 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1"); code != 0 { 206 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1"); code != 0 {
192 t.Fatalf("owner-approved merge refused: %s", errOut) 207 t.Fatalf("owner-approved merge refused: %s", errOut)
193 } 208 }
209
210 // The toggle on a repository with no CODEOWNERS file says so rather
211 // than silently gating nothing.
212 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/bare"); code != 0 {
213 t.Fatalf("repo create: %s", errOut)
214 }
215 if _, _, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-codeowners", "alice/bare", "on"); code != 0 {
216 t.Fatal("require-codeowners on alice/bare failed")
217 }
218 bare := t.TempDir()
219 mustGit(t, bare, env, "clone", inst.sshURL("alice/bare"), "b")
220 bdir := filepath.Join(bare, "b")
221 os.WriteFile(filepath.Join(bdir, "a.txt"), []byte("a\n"), 0o644)
222 mustGit(t, bdir, env, "checkout", "-q", "-b", "main")
223 mustGit(t, bdir, env, "add", ".")
224 mustGit(t, bdir, env, "commit", "-q", "-m", "base")
225 mustGit(t, bdir, env, "push", "-q", "origin", "main")
226 mustGit(t, bdir, env, "checkout", "-q", "-b", "feat")
227 mustGit(t, bdir, env, "commit", "-q", "--allow-empty", "-m", "work")
228 mustGit(t, bdir, env, "push", "-q", "origin", "feat")
229 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/bare",
230 "--source", "feat", "--target", "main", "--title", "'work'"); code != 0 {
231 t.Fatalf("mr create: %s", errOut)
232 }
233 _, errOut, code = inst.ssh(t, aliceKey, "", "mr", "merge", "alice/bare", "1")
234 if code != 4 || !strings.Contains(errOut, "no CODEOWNERS file") {
235 t.Fatalf("missing CODEOWNERS file: exit %d, %s", code, errOut)
236 }
194} 237}
internal/control/mr.go +35 −9
@@ -26,6 +26,9 @@ func init() {
26 register(Command{Path: []string{"repo", "settings", "require-resolved"}, 26 register(Command{Path: []string{"repo", "settings", "require-resolved"},
27 Summary: "require all review threads resolved to merge", 27 Summary: "require all review threads resolved to merge",
28 Usage: "repo settings require-resolved <owner/name> on|off", Run: runRequireResolved}) 28 Usage: "repo settings require-resolved <owner/name> on|off", Run: runRequireResolved})
29 register(Command{Path: []string{"repo", "settings", "require-codeowners"},
30 Summary: "require an owner's approval for every file CODEOWNERS covers",
31 Usage: "repo settings require-codeowners <owner/name> on|off", Run: runRequireCodeowners})
29 register(Command{Path: []string{"repo", "settings", "require-checks"}, 32 register(Command{Path: []string{"repo", "settings", "require-checks"},
30 Summary: "gate merges on green statuses", 33 Summary: "gate merges on green statuses",
31 Usage: "repo settings require-checks <owner/name> on|off", Run: runRequireChecks}) 34 Usage: "repo settings require-checks <owner/name> on|off", Run: runRequireChecks})
@@ -158,6 +161,24 @@ func runRequireResolved(c *Ctx, args []string) int {
158 }) 161 })
159} 162}
160 163
164func runRequireCodeowners(c *Ctx, args []string) int {
165 if len(args) != 2 || (args[1] != "on" && args[1] != "off") {
166 return c.fail(protocol.ExitUsage, "usage: repo settings require-codeowners <owner/name> on|off")
167 }
168 repo, code := resolveRepo(c, args[0], policy.CanAdmin)
169 if code >= 0 {
170 return code
171 }
172 s := repo.Settings
173 s.RequireCodeowners = args[1] == "on"
174 if err := c.Store.SetRepoSettings(repo.ID, s); err != nil {
175 return c.fail(protocol.ExitFailure, "%v", err)
176 }
177 return c.emit(s, func(w io.Writer) {
178 fmt.Fprintf(w, "require_codeowners %s on %s\n", args[1], repo.Path())
179 })
180}
181
161func runRequireChecks(c *Ctx, args []string) int { 182func runRequireChecks(c *Ctx, args []string) int {
162 if len(args) != 2 || (args[1] != "on" && args[1] != "off") { 183 if len(args) != 2 || (args[1] != "on" && args[1] != "off") {
163 return c.fail(protocol.ExitUsage, "usage: repo settings require-checks <owner/name> on|off") 184 return c.fail(protocol.ExitUsage, "usage: repo settings require-checks <owner/name> on|off")
@@ -1019,8 +1040,7 @@ func runMRMerge(c *Ctx, args []string) int {
1019} 1040}
1020 1041
1021// reviewGates enforces require_approvals (fresh, non-author, latest review 1042// reviewGates enforces require_approvals (fresh, non-author, latest review
1022// per reviewer; a fresh request-changes blocks), CODEOWNERS coverage 1043// per reviewer; a fresh request-changes blocks), require_codeowners, and
1023// whenever the target branch carries a CODEOWNERS file, and
1024// require_resolved. Returns -1 to proceed. 1044// require_resolved. Returns -1 to proceed.
1025func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) int { 1045func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) int {
1026 set := repo.Settings 1046 set := repo.Settings
@@ -1060,13 +1080,19 @@ func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA
1060 } 1080 }
1061 1081
1062 // CODEOWNERS: every owned changed file needs an approval from one of 1082 // CODEOWNERS: every owned changed file needs an approval from one of
1063 // its owners. The file's presence is the opt-in; it does not wait on 1083 // its owners. require_codeowners is the opt-in — a repository can
1064 // require_approvals (#99). 1084 // carry the file as documentation of who to ask without it gating
1065 content, err := gitutil.ReadBlob(dir, "refs/heads/"+mr.TargetRef, "CODEOWNERS", 1<<20) 1085 // merges — and it does not wait on require_approvals (#99).
1066 if err != nil { 1086 if set.RequireCodeowners {
1067 content, err = gitutil.ReadBlob(dir, "refs/heads/"+mr.TargetRef, ".gitbay/CODEOWNERS", 1<<20) 1087 content, err := gitutil.ReadBlob(dir, "refs/heads/"+mr.TargetRef, "CODEOWNERS", 1<<20)
1068 } 1088 if err != nil {
1069 if err == nil && len(content) > 0 { 1089 content, err = gitutil.ReadBlob(dir, "refs/heads/"+mr.TargetRef, ".gitbay/CODEOWNERS", 1<<20)
1090 }
1091 if err != nil || len(content) == 0 {
1092 return c.fail(protocol.ExitDenied,
1093 "%s requires CODEOWNERS approval but %s carries no CODEOWNERS file",
1094 repo.Path(), mr.TargetRef)
1095 }
1070 rules := policy.ParseCodeowners(string(content)) 1096 rules := policy.ParseCodeowners(string(content))
1071 base, err := gitutil.MergeBase(dir, targetSHA, headSHA) 1097 base, err := gitutil.MergeBase(dir, targetSHA, headSHA)
1072 if err != nil { 1098 if err != nil {
internal/httpd/settings.go +2
@@ -68,6 +68,8 @@ func (s *Server) settingsSubmit(w http.ResponseWriter, r *http.Request, u store.
68 argv = []string{"repo", "settings", "require-checks", repo, onOff(v("require-checks"))} 68 argv = []string{"repo", "settings", "require-checks", repo, onOff(v("require-checks"))}
69 case "require-resolved": 69 case "require-resolved":
70 argv = []string{"repo", "settings", "require-resolved", repo, onOff(v("require-resolved"))} 70 argv = []string{"repo", "settings", "require-resolved", repo, onOff(v("require-resolved"))}
71 case "require-codeowners":
72 argv = []string{"repo", "settings", "require-codeowners", repo, onOff(v("require-codeowners"))}
71 case "require-signed": 73 case "require-signed":
72 argv = []string{"repo", "settings", "require-signed", repo, onOff(v("require-signed"))} 74 argv = []string{"repo", "settings", "require-signed", repo, onOff(v("require-signed"))}
73 case "require-approvals": 75 case "require-approvals":
internal/store/repos.go +1
@@ -26,6 +26,7 @@ type RepoSettings struct {
26 RequireChecks bool `json:"require_checks,omitempty"` 26 RequireChecks bool `json:"require_checks,omitempty"`
27 RequireApprovals int `json:"require_approvals,omitempty"` 27 RequireApprovals int `json:"require_approvals,omitempty"`
28 RequireResolved bool `json:"require_resolved,omitempty"` 28 RequireResolved bool `json:"require_resolved,omitempty"`
29 RequireCodeowners bool `json:"require_codeowners,omitempty"`
29 GitDaemon bool `json:"git_daemon,omitempty"` 30 GitDaemon bool `json:"git_daemon,omitempty"`
30 Archived bool `json:"archived,omitempty"` 31 Archived bool `json:"archived,omitempty"`
31 Website string `json:"website,omitempty"` 32 Website string `json:"website,omitempty"`
internal/web/templates/settings.html +6
@@ -63,6 +63,12 @@
63 <input type="checkbox" id="require-resolved" name="require-resolved" value="on"{{if .Repo.Settings.RequireResolved}} checked{{end}}> 63 <input type="checkbox" id="require-resolved" name="require-resolved" value="on"{{if .Repo.Settings.RequireResolved}} checked{{end}}>
64 <button type="submit">Save</button> 64 <button type="submit">Save</button>
65</form> 65</form>
66<form method="post" action="{{$base}}" class="setform">
67 <input type="hidden" name="field" value="require-codeowners">
68 <label for="require-codeowners">Require CODEOWNERS approval</label>
69 <input type="checkbox" id="require-codeowners" name="require-codeowners" value="on"{{if .Repo.Settings.RequireCodeowners}} checked{{end}}>
70 <button type="submit">Save</button>
71</form>
66<form method="post" action="{{$base}}" class="setform"> 72<form method="post" action="{{$base}}" class="setform">
67 <input type="hidden" name="field" value="require-signed"> 73 <input type="hidden" name="field" value="require-signed">
68 <label for="require-signed">Require signed commits</label> 74 <label for="require-signed">Require signed commits</label>