krz/gitbay

ci: reserve ci/ statuses; trusted reuse; required contexts !482

merged merged by cmc on 2026-09-28 22:33 UTC · krz/gitbay:ci-status-trust into main

30 files changed, +446 −46

Layout: unified · split

.gitbay/wiki/API.org +16 −2
@@ -112,8 +112,8 @@ CI reports results through the same command surface (over SSH or the
112JSON API with a full-scope token; reporting requires write access): 112JSON API with a full-scope token; reporting requires write access):
113 113
114#+begin_src sh 114#+begin_src sh
115gitbay status set <owner/name> <sha> --context build --state pending 115gitbay status set <owner/name> <sha> --context ext/build --state pending
116gitbay status set <owner/name> <sha> --context build --state success --url https://ci.example/run/1 116gitbay status set <owner/name> <sha> --context ext/build --state success --url https://ci.example/run/1
117gitbay status list <owner/name> <sha> --json # {"combined": "...", "statuses": [...]} 117gitbay status list <owner/name> <sha> --json # {"combined": "...", "statuses": [...]}
118#+end_src 118#+end_src
119 119
@@ -130,6 +130,20 @@ from outside through =status set= looks like. A repository where
130nothing has ever reported merges. Each 130nothing has ever reported merges. Each
131report also emits a =status= event to webhooks. 131report also emits a =status= event to webhooks.
132 132
133Contexts starting with =ci/= are the instance's own: its builds queue,
134reuse, skip and finish them, and =status set= refuses them with exit 4,
135so a writer cannot mark =ci/test= green on a head the build has not
136passed. Report under another prefix, such as =ext/=.
137
138=repo settings require-contexts <repo> ext/deploy ci/test= names
139statuses the checks gate waits for whether or not they have reported:
140one that has not is =pending=, and =mr show= lists it as
141=ext/deploy=missing=. Naming any context turns =require-checks= on;
142with no contexts the command clears the list and leaves
143=require-checks= as it was. =require-checks off= keeps the list, which
144waits for nothing until the gate is on again. =repo settings show=
145prints both.
146
133* Webhooks 147* Webhooks
134 148
135Per-repository outbound POSTs for repository events. Managed by repo 149Per-repository outbound POSTs for repository events. Managed by repo
.gitbay/wiki/Architecture/07-CI-and-Supply-Chain.org +2 −2
@@ -23,7 +23,7 @@ commit instead of failing silently.
231. *Queue.* The post-receive hook calls =queueJobs= 231. *Queue.* The post-receive hook calls =queueJobs=
24 (=internal/control/build.go=). Each job gets a =ci/<job>= status: 24 (=internal/control/build.go=). Each job gets a =ci/<job>= status:
25 =pending= when queued, =skipped= when path filters exclude it, or 25 =pending= when queued, =skipped= when path filters exclude it, or
26 =success= copied from an earlier build of the same tree (#177). 26 =success= copied from an earlier trusted build of the same tree on the same image (#177, #258).
27 Merge requests from forks are queued against the target repository 27 Merge requests from forks are queued against the target repository
28 with =trusted = false=. 28 with =trusted = false=.
292. *Claim.* A runner calls =runner next= over SSH 292. *Claim.* A runner calls =runner next= over SSH
@@ -55,7 +55,7 @@ Who may do what:
55| =repo secret set/remove/list= | admin on the repository | 55| =repo secret set/remove/list= | admin on the repository |
56| =repo runner add/remove= | admin on the repository | 56| =repo runner add/remove= | admin on the repository |
57| =runner next/log/done= | =runner= key attached to the repository, or admin | 57| =runner next/log/done= | =runner= key attached to the repository, or admin |
58| =status set= | write on the repository (any context name; #258) | 58| =status set= | write on the repository; =ci/*= contexts refused (=status.go=) |
59 59
60* Runner isolation 60* Runner isolation
61 61
.gitbay/wiki/Architecture/09-Controls.org +2 −2
@@ -36,7 +36,7 @@ chapter names of OWASP ASVS 4.0 where one fits.
36| Private resources indistinguishable from missing | in place | =resolveRepo= (=internal/control/repo.go=), =runGit=, smart HTTP | 36| Private resources indistinguishable from missing | in place | =resolveRepo= (=internal/control/repo.go=), =runGit=, smart HTTP |
37| Credential scopes narrow account rights | in place | key and token scopes (=control.go=, =policy/access.go=) | 37| Credential scopes narrow account rights | in place | key and token scopes (=control.go=, =policy/access.go=) |
38| Server-side write protections | in place | pre-receive =CheckPush=, signed commits (=internal/hookd/hookd.go=) | 38| Server-side write protections | in place | pre-receive =CheckPush=, signed commits (=internal/hookd/hookd.go=) |
39| Merge gates | partial | =MergeGates=; any writer can post a =ci/*= status (#258) | 39| Merge gates | in place | =MergeGates=; =ci/*= statuses written only by the build subsystem; required contexts |
40| Admin functions isolated | in place | =admin= noun gated in =Dispatch=; =audit= admin-only | 40| Admin functions isolated | in place | =admin= noun gated in =Dispatch=; =audit= admin-only |
41| CSRF protection | in place | SameSite=Lax plus =checkOrigin= (=accounts.go=) | 41| CSRF protection | in place | SameSite=Lax plus =checkOrigin= (=accounts.go=) |
42| Typed confirmation for destructive web actions | in place | =internal/httpd/confirm.go= | 42| Typed confirmation for destructive web actions | in place | =internal/httpd/confirm.go= |
@@ -89,7 +89,7 @@ chapter names of OWASP ASVS 4.0 where one fits.
89| Runner limited to attached repositories | in place | =runnerMayBuild= (=build.go=) | 89| Runner limited to attached repositories | in place | =runnerMayBuild= (=build.go=) |
90| Build images fixed by the operator | in place | =--pull=never= | 90| Build images fixed by the operator | in place | =--pull=never= |
91| Build network egress restricted | gap | #260 | 91| Build network egress restricted | gap | #260 |
92| Build results reused only across equal trust | gap | tree reuse ignores trust and image (#258) | 92| Build results reused only across equal trust | in place | =SuccessBuildForTree=, =SuccessBuildFor= (=internal/store/builds.go=) |
93 93
94** Availability and operations 94** Availability and operations
95 95
.gitbay/wiki/Architecture/10-Known-Gaps.org −1
@@ -10,7 +10,6 @@ what the 2026-09-27 review found; remove a row when its issue closes.
10 10
11| Issue | Area | Gap | Severity | 11| Issue | Area | Gap | Severity |
12|-------+------------------+-----------------------------------------------------------------------+----------| 12|-------+------------------+-----------------------------------------------------------------------+----------|
13| #258 | CI integrity | Any writer can post a =ci/*= status; tree reuse ignores trust and image | high |
14| #259 | Recovery | No restore has been exercised; verification does not check git connectivity | high | 13| #259 | Recovery | No restore has been exercised; verification does not check git connectivity | high |
15| #260 | CI network | Builds share the runner's source address; no egress policy | medium | 14| #260 | CI network | Builds share the runner's source address; no egress policy | medium |
16| #261 | Various | Migration foreign-key check after commit; three web writes bypass dispatch; documentation drift | medium | 15| #261 | Various | Migration foreign-key check after commit; three web writes bypass dispatch; documentation drift | medium |
.gitbay/wiki/CI.org +17 −2
@@ -5,8 +5,20 @@ Three mechanisms decide what a push does to CI, and they interact:
5- *Dedupe.* A job's result is a property of the commit's tree. A commit 5- *Dedupe.* A job's result is a property of the commit's tree. A commit
6 that already has a passed, queued or running build for a job is not 6 that already has a passed, queued or running build for a job is not
7 queued again; a commit whose tree already passed a job gets that 7 queued again; a commit whose tree already passed a job gets that
8 result as its status, naming the build it came from (#177). A failed, 8 result as its status, naming the build it came from (#177). Only a
9 cancelled or abandoned build does not count: that commit runs again. 9 trusted build counts, and for tree reuse only one on the image the
10 job names: a fork's green build does not stand for the repository's
11 own, so its commit is built again when it lands on a branch (#258). A
12 job that names no =image:= is compared as naming none: its reuse does
13 not notice the runner's default image changing, because reuse is
14 decided when the push is queued, before any runner claims the build,
15 and runners can differ in their default. Name the image in =ci.yml=
16 to tie reuse to it; after an operator changes a runner's =-image=,
17 =build trigger= builds a job afresh, since a triggered build is never
18 reused. When a trusted and an untrusted build of the same commit both
19 run, the one that finishes last sets =ci/<job>= on that commit. A
20 failed, cancelled or abandoned build does not count: that commit runs
21 again.
10- *Path filters.* =paths= and =paths-ignore= on a job are evaluated 22- *Path filters.* =paths= and =paths-ignore= on a job are evaluated
11 against the files the push changed. The diff base is the old tip when 23 against the files the push changed. The diff base is the old tip when
12 it is an ancestor of the new one, and the merge base with the default 24 it is an ancestor of the new one, and the merge base with the default
@@ -127,6 +139,9 @@ Rows worth a second look:
127 branch push, no merge-base fallback: every job runs, without secrets. 139 branch push, no merge-base fallback: every job runs, without secrets.
128 Filtering a head down to no jobs would make it unmergeable under 140 Filtering a head down to no jobs would make it unmergeable under
129 =require-checks= (#172). 141 =require-checks= (#172).
142- A context named in =repo settings require-contexts= must be one that
143 reports on merge request heads. A schedule-only or tag-only job never
144 reports there, so the gate stays pending (#258).
130 145
131=TestPushShapes= in =internal/hookd= runs every row against real git 146=TestPushShapes= in =internal/hookd= runs every row against real git
132and the store, and =TestPushShapesTableOnWiki= checks that this page 147and the store, and =TestPushShapesTableOnWiki= checks that this page
.gitbay/wiki/Parity.org +1
@@ -197,6 +197,7 @@ rather than the one the web page shows.
197| merge requests only | yes | yes | yes | 197| merge requests only | yes | yes | yes |
198| protected tags | yes | yes | yes | 198| protected tags | yes | yes | yes |
199| require codeowners | yes | yes | yes | 199| require codeowners | yes | yes | yes |
200| require contexts | yes | yes | no |
200| access grants | yes | no | yes | 201| access grants | yes | no | yes |
201| effective access | yes | no | yes | 202| effective access | yes | no | yes |
202| webhooks | yes | no | yes | 203| webhooks | yes | no | yes |
.gitbay/wiki/Users.org +3 −1
@@ -553,7 +553,9 @@ queues nothing, so look there when a push builds nothing.
553 553
554Each job becomes a build (=build list=, =build log=, the builds tab on 554Each job becomes a build (=build list=, =build log=, the builds tab on
555the web) and a =ci/<job>= commit status, which =repo settings 555the web) and a =ci/<job>= commit status, which =repo settings
556require-checks= can gate merges on. =build list= takes =--ref=, 556require-checks= can gate merges on. =repo settings
557require-contexts= names statuses the gate waits for until they report,
558and turns the gate on. =build list= takes =--ref=,
557=--status= and =--job= to narrow the listing, combinable; the builds 559=--status= and =--job= to narrow the listing, combinable; the builds
558tab reads the same flags from its =?ref=, =?status= and =?job= query 560tab reads the same flags from its =?ref=, =?status= and =?job= query
559parameters and groups the result into one row per commit. Steps run 561parameters and groups the result into one row per commit. Steps run
CHANGELOG.org +12
@@ -121,6 +121,18 @@ for the eighteen commands whose CLI path differs from the registry's
121 previous" link on each revision after the first, so a reviewer whose 121 previous" link on each revision after the first, so a reviewer whose
122 approval a force-push staled can see what changed without leaving the 122 approval a force-push staled can see what changed without leaving the
123 browser (#269). 123 browser (#269).
124- Untrusted builds (merge requests from forks) get a fresh HOME removed after the build and no secrets; trusted builds keep a per-repository home under =<workdir>/trusted-home=. Deploy gitbayd before the runner; the old shared homes under the runner's workdir can be deleted. (#255)
125- =status set= refuses =ci/= contexts, which belong to the instance's builds. Build results are reused only from trusted builds on the same image. =repo settings require-contexts= names status contexts that must report green; setting any turns require-checks on, and one not yet reported counts as pending. (#258)
126- Untrusted builds (merge requests from forks) get a fresh HOME
127 removed after the build and no secrets; trusted builds keep a
128 per-repository home under =<workdir>/trusted-home=. Deploy gitbayd
129 before the runner; the old shared homes under the runner's workdir
130 can be deleted. (#255)
131- =status set= refuses =ci/= contexts, which belong to the instance's
132 builds. Build results are reused only from trusted builds on the
133 same image. =repo settings require-contexts= names status contexts
134 that must report green; setting any turns require-checks on, and one
135 not yet reported counts as pending. (#258)
124 136
125* v1.36.0 — 2026-09-23 137* v1.36.0 — 2026-09-23
126 138
cmd/gitbay/main.go +1
@@ -618,6 +618,7 @@ func repoCmd() *cobra.Command {
618 pass("require-resolved", passOpts{server: []string{"repo", "settings", "require-resolved"}, needsRepo: true}), 618 pass("require-resolved", passOpts{server: []string{"repo", "settings", "require-resolved"}, needsRepo: true}),
619 pass("require-codeowners", passOpts{server: []string{"repo", "settings", "require-codeowners"}, needsRepo: true}), 619 pass("require-codeowners", passOpts{server: []string{"repo", "settings", "require-codeowners"}, needsRepo: true}),
620 pass("require-checks", passOpts{server: []string{"repo", "settings", "require-checks"}, needsRepo: true}), 620 pass("require-checks", passOpts{server: []string{"repo", "settings", "require-checks"}, needsRepo: true}),
621 pass("require-contexts", passOpts{server: []string{"repo", "settings", "require-contexts"}, needsRepo: true}),
621 pass("visibility", passOpts{server: []string{"repo", "settings", "visibility"}, needsRepo: true}), 622 pass("visibility", passOpts{server: []string{"repo", "settings", "visibility"}, needsRepo: true}),
622 pass("require-signed", passOpts{server: []string{"repo", "settings", "require-signed"}, needsRepo: true}), 623 pass("require-signed", passOpts{server: []string{"repo", "settings", "require-signed"}, needsRepo: true}),
623 pass("require-mr", passOpts{server: []string{"repo", "settings", "require-mr"}, needsRepo: true}), 624 pass("require-mr", passOpts{server: []string{"repo", "settings", "require-mr"}, needsRepo: true}),
cmd/gitbay/summaries_gen.go +1
@@ -187,6 +187,7 @@ var summaries = map[string]string{
187 "repo settings require-approvals": "require N fresh approvals to merge", 187 "repo settings require-approvals": "require N fresh approvals to merge",
188 "repo settings require-checks": "gate merges on green statuses", 188 "repo settings require-checks": "gate merges on green statuses",
189 "repo settings require-codeowners": "require an owner's approval for every file CODEOWNERS covers", 189 "repo settings require-codeowners": "require an owner's approval for every file CODEOWNERS covers",
190 "repo settings require-contexts": "name the statuses the checks gate waits for, and turn the gate on",
190 "repo settings require-mr": "protected branches take changes through merge requests only", 191 "repo settings require-mr": "protected branches take changes through merge requests only",
191 "repo settings require-resolved": "require all review threads resolved to merge", 192 "repo settings require-resolved": "require all review threads resolved to merge",
192 "repo settings require-signed": "require verified commit signatures", 193 "repo settings require-signed": "require verified commit signatures",
e2e/mrweb_test.go +1 −1
@@ -272,7 +272,7 @@ func TestMRListRows(t *testing.T) {
272 t.Fatalf("mr create: %s", errOut) 272 t.Fatalf("mr create: %s", errOut)
273 } 273 }
274 if _, errOut, code := inst.ssh(t, aliceKey, "", "status", "set", "alice/lib", sha, 274 if _, errOut, code := inst.ssh(t, aliceKey, "", "status", "set", "alice/lib", sha,
275 "--context", "ci/test", "--state", "success"); code != 0 { 275 "--context", "ext/test", "--state", "success"); code != 0 {
276 t.Fatalf("status set: %s", errOut) 276 t.Fatalf("status set: %s", errOut)
277 } 277 }
278 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "comment", "alice/lib", "1", 278 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "comment", "alice/lib", "1",
e2e/readonly_test.go +1 −1
@@ -72,7 +72,7 @@ func TestReadOnlyCommandsWriteNothing(t *testing.T) {
72 must("", "milestone", "create", "alice/app", "m1") 72 must("", "milestone", "create", "alice/app", "m1")
73 must("", "mr", "create", "alice/app", "--source", "feat", "--target", "main", "--title", "change") 73 must("", "mr", "create", "alice/app", "--source", "feat", "--target", "main", "--title", "change")
74 must("", "mr", "diff-comment", "alice/app", "1", "--path", "f.go", "--line", "3", "--message", "why") 74 must("", "mr", "diff-comment", "alice/app", "1", "--path", "f.go", "--line", "3", "--message", "why")
75 must("", "status", "set", "alice/app", sha, "--context", "ci/x", "--state", "success") 75 must("", "status", "set", "alice/app", sha, "--context", "ext/x", "--state", "success")
76 must("", "release", "create", "alice/app", "v1", "--title", "first") 76 must("", "release", "create", "alice/app", "v1", "--title", "first")
77 must("data\n", "release", "asset", "add", "alice/app", "v1", "a.txt") 77 must("data\n", "release", "asset", "add", "alice/app", "v1", "a.txt")
78 snippetOut := must("hello\n", "snippet", "create", "a.txt", "--json") 78 snippetOut := must("hello\n", "snippet", "create", "a.txt", "--json")
e2e/settingsweb_test.go +7 −2
@@ -55,7 +55,12 @@ func TestRepoSettingsWeb(t *testing.T) {
55 55
56 post(url.Values{"field": {"description"}, "description": {"a fine tool"}}) 56 post(url.Values{"field": {"description"}, "description": {"a fine tool"}})
57 post(url.Values{"field": {"website"}, "website": {"https://tool.example"}}) 57 post(url.Values{"field": {"website"}, "website": {"https://tool.example"}})
58 post(url.Values{"field": {"require-checks"}, "require-checks": {"on"}}) 58 // Saving required contexts turns the checks gate on, and the page
59 // shows it ticked with the contexts in its hint (#258).
60 if body := post(url.Values{"field": {"require-contexts"}, "contexts": {"ext/deploy lint"}}); !strings.Contains(body, `id="require-checks" name="require-checks" value="on" checked`) ||
61 !strings.Contains(body, `Also waits for <code>ext/deploy</code>, <code>lint</code> until they report.`) {
62 t.Fatalf("required contexts did not show as turning required checks on:\n%s", body)
63 }
59 post(url.Values{"field": {"require-approvals"}, "approvals": {"2"}}) 64 post(url.Values{"field": {"require-approvals"}, "approvals": {"2"}})
60 post(url.Values{"field": {"protect"}, "branch": {"main"}}) 65 post(url.Values{"field": {"protect"}, "branch": {"main"}})
61 66
@@ -66,7 +71,7 @@ func TestRepoSettingsWeb(t *testing.T) {
66 } 71 }
67 } 72 }
68 out, _, _ = inst.ssh(t, aliceKey, "", "repo", "settings", "show", "alice/app", "--json") 73 out, _, _ = inst.ssh(t, aliceKey, "", "repo", "settings", "show", "alice/app", "--json")
69 for _, want := range []string{`"require_checks":true`, `"require_approvals":2`, `"main"`} { 74 for _, want := range []string{`"require_checks":true`, `"required_contexts":["ext/deploy","lint"]`, `"require_approvals":2`, `"main"`} {
70 if !strings.Contains(out, want) { 75 if !strings.Contains(out, want) {
71 t.Fatalf("settings show missing %q:\n%s", want, out) 76 t.Fatalf("settings show missing %q:\n%s", want, out)
72 } 77 }
e2e/status_test.go +5
@@ -48,6 +48,11 @@ func TestCommitStatuses(t *testing.T) {
48 t.Fatal("reader reported a status") 48 t.Fatal("reader reported a status")
49 } 49 }
50 50
51 // ci/ is the instance's own: a writer is refused it (#258).
52 if _, errOut, code := inst.ssh(t, bobKey, "", "status", "set", "alice/svc", head, "--context", "ci/build", "--state", "success"); code != 4 || !strings.Contains(errOut, "reserved") {
53 t.Fatalf("writer posted a ci/ status: exit %d, %s", code, errOut)
54 }
55
51 // Bob (write) reports pending, then success: upsert, not duplicate. 56 // Bob (write) reports pending, then success: upsert, not duplicate.
52 if _, errOut, code := inst.ssh(t, bobKey, "", "status", "set", "alice/svc", head, 57 if _, errOut, code := inst.ssh(t, bobKey, "", "status", "set", "alice/svc", head,
53 "--context", "build", "--state", "pending", "--description", "'compiling'"); code != 0 { 58 "--context", "build", "--state", "pending", "--description", "'compiling'"); code != 0 {
internal/control/build.go +7 −2
@@ -857,10 +857,15 @@ func queueJobs(
857 if j.Tags != "" { 857 if j.Tags != "" {
858 continue 858 continue
859 } 859 }
860 if b, ok := built[j.Name]; ok && (b.Status == "success" || b.Status == "pending" || b.Status == "running") { 860 // A build of this commit that passed, or is queued or running,
861 // stands for it — unless this queue is trusted and that build was
862 // not: a fork's head that lands on a branch is built again as the
863 // repository's own (#258).
864 if b, ok := built[j.Name]; ok && (b.Trusted || !trusted) &&
865 (b.Status == "success" || b.Status == "pending" || b.Status == "running") {
861 continue 866 continue
862 } 867 }
863 if prev, ok, _ := st.SuccessBuildForTree(repo.ID, tree, j.Name); ok && prev.SHA != sha { 868 if prev, ok, _ := st.SuccessBuildForTree(repo.ID, tree, j.Name, j.Image); ok && prev.SHA != sha {
864 url := fmt.Sprintf("%s/%s/builds/%d", siteURL, repo.Path(), prev.Number) 869 url := fmt.Sprintf("%s/%s/builds/%d", siteURL, repo.Path(), prev.Number)
865 st.SetCommitStatus(repo.ID, sha, "ci/"+j.Name, "success", 870 st.SetCommitStatus(repo.ID, sha, "ci/"+j.Name, "success",
866 fmt.Sprintf("passed in build %d as %.10s, same tree", prev.Number, prev.SHA), url, userID) 871 fmt.Sprintf("passed in build %d as %.10s, same tree", prev.Number, prev.SHA), url, userID)
internal/control/build_test.go +47
@@ -669,3 +669,50 @@ func TestBuildListFlagsFilter(t *testing.T) {
669 t.Fatalf("bad --status error does not name the valid states: %s", errOut.String()) 669 t.Fatalf("bad --status error does not name the valid states: %s", errOut.String())
670 } 670 }
671} 671}
672
673// A fork's green build of a commit does not stand for the repository's
674// own: the same commit landing on a branch, or a commit with the same
675// tree, is built again as trusted (#258).
676func TestQueueBranchBuildsRebuildsWhatOnlyAForkBuilt(t *testing.T) {
677 st, repo, uid := newQueueTestRepo(t)
678 git := gitRunner(t)
679 root := t.TempDir()
680
681 src := filepath.Join(root, "src")
682 os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755)
683 os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte(
684 "jobs:\n unit:\n steps:\n - echo hi\n"), 0o644)
685 git(root, "init", "-q", "-b", "main", "src")
686 git(src, "add", ".")
687 git(src, "commit", "-q", "-m", "base")
688 first := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
689 git(src, "commit", "-q", "--allow-empty", "-m", "same tree")
690 second := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
691
692 dir := RepoDir(root, repo.OwnerName, repo.Name)
693 os.MkdirAll(filepath.Dir(dir), 0o755)
694 git(root, "clone", "-q", "--bare", src, dir)
695
696 // A fork's merge request head, built untrusted and green.
697 QueueMRBuilds(st, root, "https://x.test", repo, uid, 1, first)
698 b, ok, err := st.ClaimBuild([]int64{repo.ID}, true)
699 if err != nil || !ok || b.Trusted {
700 t.Fatalf("claim: ok=%v trusted=%v err=%v", ok, b.Trusted, err)
701 }
702 if err := st.FinishBuild(b.ID, "success"); err != nil {
703 t.Fatal(err)
704 }
705
706 // The same commit lands on main, then a commit with the same tree.
707 QueueBranchBuilds(st, root, "https://x.test", repo, uid, "main", "", first, time.Now())
708 QueueBranchBuilds(st, root, "https://x.test", repo, uid, "main", first, second, time.Now())
709 pending, _ := st.ListBuilds(repo.ID, store.BuildFilter{Status: "pending"}, 10)
710 if len(pending) != 2 {
711 t.Fatalf("queued %d builds, want 2 (one per commit): %+v", len(pending), pending)
712 }
713 for _, p := range pending {
714 if !p.Trusted {
715 t.Errorf("build %d queued untrusted on a branch push", p.Number)
716 }
717 }
718}
internal/control/checksgate_test.go +55 −7
@@ -3,6 +3,7 @@ package control
3import ( 3import (
4 "os" 4 "os"
5 "path/filepath" 5 "path/filepath"
6 "slices"
6 "strings" 7 "strings"
7 "testing" 8 "testing"
8 9
@@ -18,9 +19,30 @@ func gatesForHead(t *testing.T, ciYML string) GatesOut {
18} 19}
19 20
20func gatesForHeadSeeded(t *testing.T, ciYML string, seed bool) GatesOut { 21func gatesForHeadSeeded(t *testing.T, ciYML string, seed bool) GatesOut {
22 return gatesFor(t, ciYML, nil, func(st *store.Store, repoID, uid int64, targetSHA, _ string) {
23 if !seed {
24 return
25 }
26 if err := st.SetCommitStatus(repoID, targetSHA, "lint", "success", "", "", uid); err != nil {
27 t.Fatal(err)
28 }
29 })
30}
31
32// gatesFor builds a repository with require_checks on and set applied to
33// its settings, a bare dir holding the given .gitbay/ci.yml (empty
34// string for none), and one MR; seed records statuses before the gates
35// are computed.
36func gatesFor(t *testing.T, ciYML string, set func(*store.RepoSettings),
37 seed func(st *store.Store, repoID, uid int64, targetSHA, headSHA string)) GatesOut {
21 t.Helper() 38 t.Helper()
22 st, repo, uid := newQueueTestRepo(t) 39 st, repo, uid := newQueueTestRepo(t)
23 if _, err := st.UpdateRepoSettings(repo.ID, func(set *store.RepoSettings) { set.RequireChecks = true }); err != nil { 40 if _, err := st.UpdateRepoSettings(repo.ID, func(s *store.RepoSettings) {
41 s.RequireChecks = true
42 if set != nil {
43 set(s)
44 }
45 }); err != nil {
24 t.Fatal(err) 46 t.Fatal(err)
25 } 47 }
26 repo, err := st.RepoByID(repo.ID) 48 repo, err := st.RepoByID(repo.ID)
@@ -58,13 +80,9 @@ func gatesForHeadSeeded(t *testing.T, ciYML string, seed bool) GatesOut {
58 if err != nil { 80 if err != nil {
59 t.Fatal(err) 81 t.Fatal(err)
60 } 82 }
61 83 if seed != nil {
62 if seed { 84 seed(st, repo.ID, uid, targetSHA, headSHA)
63 if err := st.SetCommitStatus(repo.ID, targetSHA, "lint", "success", "", "", uid); err != nil {
64 t.Fatal(err)
65 }
66 } 85 }
67
68 g, err := MergeGates(st, repo, mr, dir, targetSHA, headSHA) 86 g, err := MergeGates(st, repo, mr, dir, targetSHA, headSHA)
69 if err != nil { 87 if err != nil {
70 t.Fatal(err) 88 t.Fatal(err)
@@ -118,3 +136,33 @@ func TestRequireChecksRefusesSilentHeadInReportingRepo(t *testing.T) {
118 t.Fatalf("allowed a silent head in a repository that reports statuses: %v", g.Unmet) 136 t.Fatalf("allowed a silent head in a repository that reports statuses: %v", g.Unmet)
119 } 137 }
120} 138}
139
140// A required context that has not reported holds the merge as pending,
141// even when every status that did report is green (#258).
142func TestRequiredContextMissingIsPending(t *testing.T) {
143 g := gatesFor(t, "", func(s *store.RepoSettings) { s.RequiredContexts = []string{"ext/deploy", "lint"} },
144 func(st *store.Store, repoID, uid int64, _, headSHA string) {
145 if err := st.SetCommitStatus(repoID, headSHA, "lint", "success", "", "", uid); err != nil {
146 t.Fatal(err)
147 }
148 })
149 if g.Checks != "pending" || !slices.Equal(g.ChecksMissing, []string{"ext/deploy"}) {
150 t.Fatalf("checks %q, missing %v", g.Checks, g.ChecksMissing)
151 }
152 if !checksUnmet(g) || !strings.Contains(strings.Join(g.Unmet, "\n"), "ext/deploy=missing") {
153 t.Fatalf("unmet: %v", g.Unmet)
154 }
155}
156
157// Every required context reported green: nothing is held.
158func TestRequiredContextsReportedPass(t *testing.T) {
159 g := gatesFor(t, "", func(s *store.RepoSettings) { s.RequiredContexts = []string{"lint"} },
160 func(st *store.Store, repoID, uid int64, _, headSHA string) {
161 if err := st.SetCommitStatus(repoID, headSHA, "lint", "success", "", "", uid); err != nil {
162 t.Fatal(err)
163 }
164 })
165 if checksUnmet(g) || len(g.ChecksMissing) != 0 || g.Checks != "success" {
166 t.Fatalf("checks %q, missing %v, unmet %v", g.Checks, g.ChecksMissing, g.Unmet)
167 }
168}
internal/control/mr.go +72 −1
@@ -47,6 +47,11 @@ func init() {
47 Usage: "repo settings require-checks <owner/name> on|off", 47 Usage: "repo settings require-checks <owner/name> on|off",
48 Examples: []string{"repo settings require-checks krz/gitbay on"}, 48 Examples: []string{"repo settings require-checks krz/gitbay on"},
49 Run: runRequireChecks}) 49 Run: runRequireChecks})
50 register(Command{Path: []string{"repo", "settings", "require-contexts"},
51 Summary: "name the statuses the checks gate waits for, and turn the gate on",
52 Usage: "repo settings require-contexts <owner/name> [<context>...] (none clears the list)",
53 Examples: []string{"repo settings require-contexts krz/gitbay ci/build ci/test"},
54 Run: runRequireContexts})
50 register(Command{Path: []string{"repo", "settings", "require-mr"}, 55 register(Command{Path: []string{"repo", "settings", "require-mr"},
51 Summary: "protected branches take changes through merge requests only", 56 Summary: "protected branches take changes through merge requests only",
52 Usage: "repo settings require-mr <owner/name> on|off", 57 Usage: "repo settings require-mr <owner/name> on|off",
@@ -340,6 +345,54 @@ func runRequireChecks(c *Ctx, args []string) int {
340 }) 345 })
341} 346}
342 347
348// maxRequiredContexts bounds the list: a gate naming more checks than
349// this is a configuration mistake.
350const maxRequiredContexts = 20
351
352func runRequireContexts(c *Ctx, args []string) int {
353 if len(args) < 1 {
354 return c.usage()
355 }
356 var contexts []string
357 for _, ctx := range args[1:] {
358 if ctx == "" || len(ctx) > 100 || strings.ContainsAny(ctx, " \t\r\n") {
359 return c.fail(protocol.ExitUsage, "a context is 1 to 100 characters with no whitespace: %q", ctx)
360 }
361 if !slices.Contains(contexts, ctx) {
362 contexts = append(contexts, ctx)
363 }
364 }
365 if len(contexts) > maxRequiredContexts {
366 return c.fail(protocol.ExitUsage, "at most %d required contexts", maxRequiredContexts)
367 }
368 repo, code := resolveRepo(c, args[0], policy.CanAdmin)
369 if code >= 0 {
370 return code
371 }
372 // Naming contexts asks for the gate, so it turns require_checks on in
373 // the same update. Clearing the list leaves the gate as it was.
374 s, err := c.Store.UpdateRepoSettings(repo.ID, func(s *store.RepoSettings) {
375 s.RequiredContexts = contexts
376 if len(contexts) > 0 {
377 s.RequireChecks = true
378 }
379 })
380 if err != nil {
381 return c.fail(protocol.ExitFailure, "%v", err)
382 }
383 return c.emit(s, func(w io.Writer) {
384 if len(contexts) > 0 {
385 fmt.Fprintf(w, "required contexts on %s: %s; require_checks on\n", repo.Path(), strings.Join(contexts, ", "))
386 return
387 }
388 gate := "off"
389 if s.RequireChecks {
390 gate = "on"
391 }
392 fmt.Fprintf(w, "required contexts cleared on %s; require_checks %s\n", repo.Path(), gate)
393 })
394}
395
343func runRequireMR(c *Ctx, args []string) int { 396func runRequireMR(c *Ctx, args []string) int {
344 if len(args) != 2 || (args[1] != "on" && args[1] != "off") { 397 if len(args) != 2 || (args[1] != "on" && args[1] != "off") {
345 return c.usage() 398 return c.usage()
@@ -1577,13 +1630,28 @@ func MergeGates(st *store.Store, repo store.Repo, mr store.MR, dir, targetSHA, h
1577 } 1630 }
1578 1631
1579 // Checks: with require_checks, every status the head carries must be 1632 // Checks: with require_checks, every status the head carries must be
1580 // green, and a head something was going to report on must carry some. 1633 // green, a head something was going to report on must carry some, and
1634 // every required context must have reported: one that has not is
1635 // pending whatever the others say (#258). Setting contexts turns
1636 // require_checks on; turned off again, the list is kept and unread.
1581 statuses, err := st.ListCommitStatuses(repo.ID, headSHA) 1637 statuses, err := st.ListCommitStatuses(repo.ID, headSHA)
1582 if err != nil { 1638 if err != nil {
1583 return g, err 1639 return g, err
1584 } 1640 }
1585 g.Checks = store.CombinedStatus(statuses) 1641 g.Checks = store.CombinedStatus(statuses)
1586 if set.RequireChecks { 1642 if set.RequireChecks {
1643 reported := map[string]bool{}
1644 for _, s := range statuses {
1645 reported[s.Context] = true
1646 }
1647 for _, want := range set.RequiredContexts {
1648 if !reported[want] {
1649 g.ChecksMissing = append(g.ChecksMissing, want)
1650 }
1651 }
1652 if len(g.ChecksMissing) > 0 && (g.Checks == "" || g.Checks == "success") {
1653 g.Checks = "pending"
1654 }
1587 switch g.Checks { 1655 switch g.Checks {
1588 case "success": 1656 case "success":
1589 case "": 1657 case "":
@@ -1597,6 +1665,9 @@ func MergeGates(st *store.Store, repo store.Repo, mr store.MR, dir, targetSHA, h
1597 bad = append(bad, st.Context+"="+st.State) 1665 bad = append(bad, st.Context+"="+st.State)
1598 } 1666 }
1599 } 1667 }
1668 for _, m := range g.ChecksMissing {
1669 bad = append(bad, m+"=missing")
1670 }
1600 g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires green checks; %.10s has %s", repo.Path(), headSHA, strings.Join(bad, ", "))) 1671 g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires green checks; %.10s has %s", repo.Path(), headSHA, strings.Join(bad, ", ")))
1601 } 1672 }
1602 } 1673 }
internal/control/mr_test.go +75
@@ -5,6 +5,7 @@ import (
5 "encoding/json" 5 "encoding/json"
6 "os" 6 "os"
7 "path/filepath" 7 "path/filepath"
8 "slices"
8 "strconv" 9 "strconv"
9 "strings" 10 "strings"
10 "testing" 11 "testing"
@@ -262,3 +263,77 @@ func TestMRShowPluralizesMultiRowSections(t *testing.T) {
262 } 263 }
263 } 264 }
264} 265}
266
267// require-contexts stores a deduplicated list and turns require_checks
268// on with it; an empty list clears the contexts and leaves
269// require_checks as it was. A context with whitespace is refused (#258).
270func TestRequireContextsSetsAndClears(t *testing.T) {
271 st, repo, uid := newQueueTestRepo(t)
272 alice := store.User{ID: uid, Username: "alice"}
273 dispatch := func(args ...string) int {
274 t.Helper()
275 c, _, _ := mrTestCtx(st, alice)
276 return Dispatch(c, args)
277 }
278 contexts := func(names ...string) int {
279 t.Helper()
280 return dispatch(append([]string{"repo", "settings", "require-contexts", repo.Path()}, names...)...)
281 }
282 settings := func() store.RepoSettings {
283 t.Helper()
284 got, err := st.RepoByID(repo.ID)
285 if err != nil {
286 t.Fatal(err)
287 }
288 return got.Settings
289 }
290
291 if settings().RequireChecks {
292 t.Fatal("require_checks on in a new repository")
293 }
294 if code := contexts("lint", "ext/deploy", "lint"); code != protocol.ExitOK {
295 t.Fatalf("set: exit %d", code)
296 }
297 if s := settings(); !slices.Equal(s.RequiredContexts, []string{"lint", "ext/deploy"}) || !s.RequireChecks {
298 t.Fatalf("stored %v, require_checks %v; want [lint ext/deploy], on", s.RequiredContexts, s.RequireChecks)
299 }
300 if code := contexts("bad context"); code != protocol.ExitUsage {
301 t.Fatalf("a context with a space: exit %d", code)
302 }
303 if code := contexts(); code != protocol.ExitOK {
304 t.Fatalf("clear: exit %d", code)
305 }
306 if s := settings(); len(s.RequiredContexts) != 0 || !s.RequireChecks {
307 t.Fatalf("after clearing: contexts %v, require_checks %v; want none, still on", s.RequiredContexts, s.RequireChecks)
308 }
309 if code := dispatch("repo", "settings", "require-checks", repo.Path(), "off"); code != protocol.ExitOK {
310 t.Fatalf("require-checks off: exit %d", code)
311 }
312 if code := contexts(); code != protocol.ExitOK {
313 t.Fatalf("clear again: exit %d", code)
314 }
315 if settings().RequireChecks {
316 t.Fatal("clearing the list turned require_checks on")
317 }
318}
319
320// settings show prints the checks gate beside the contexts it waits for,
321// so a list that turned the gate on is visible where the gate is (#258).
322func TestSettingsShowRequiredContexts(t *testing.T) {
323 st, repo, uid := newQueueTestRepo(t)
324 alice := store.User{ID: uid, Username: "alice"}
325 c, _, _ := mrTestCtx(st, alice)
326 if code := Dispatch(c, []string{"repo", "settings", "require-contexts", repo.Path(), "ext/deploy", "lint"}); code != protocol.ExitOK {
327 t.Fatalf("require-contexts: exit %d", code)
328 }
329 c, out, _ := mrTestCtx(st, alice)
330 if code := Dispatch(c, []string{"repo", "settings", "show", repo.Path()}); code != protocol.ExitOK {
331 t.Fatalf("settings show: exit %d", code)
332 }
333 got := strings.Join(strings.Fields(out.String()), " ")
334 for _, want := range []string{"require checks true", "required contexts ext/deploy, lint"} {
335 if !strings.Contains(got, want) {
336 t.Errorf("settings show lacks %q:\n%s", want, out.String())
337 }
338 }
339}
internal/control/output.go +2 −1
@@ -57,7 +57,8 @@ type GatesOut struct {
57 ResolvedRequired bool `json:"resolved_required"` 57 ResolvedRequired bool `json:"resolved_required"`
58 OpenThreads int `json:"open_threads"` 58 OpenThreads int `json:"open_threads"`
59 ChecksRequired bool `json:"checks_required"` 59 ChecksRequired bool `json:"checks_required"`
60 Checks string `json:"checks,omitempty"` // combined status; "" when none reported 60 Checks string `json:"checks,omitempty"` // combined status; "" when none reported
61 ChecksMissing []string `json:"checks_missing,omitempty"` // required contexts not reported
61 FastForward bool `json:"fast_forward"` 62 FastForward bool `json:"fast_forward"`
62 Unmet []string `json:"unmet,omitempty"` 63 Unmet []string `json:"unmet,omitempty"`
63} 64}
internal/control/repo.go +2
@@ -711,6 +711,8 @@ func runSettingsShow(c *Ctx, args []string) int {
711 "protected branches", strings.Join(repo.Settings.ProtectedBranches, ", "), 711 "protected branches", strings.Join(repo.Settings.ProtectedBranches, ", "),
712 "protected tags", strings.Join(repo.Settings.ProtectedTags, ", "), 712 "protected tags", strings.Join(repo.Settings.ProtectedTags, ", "),
713 "require mr", strconv.FormatBool(repo.Settings.RequireMR), 713 "require mr", strconv.FormatBool(repo.Settings.RequireMR),
714 "require checks", strconv.FormatBool(repo.Settings.RequireChecks),
715 "required contexts", strings.Join(repo.Settings.RequiredContexts, ", "),
714 "require signed commits", strconv.FormatBool(repo.Settings.RequireSignedCommits), 716 "require signed commits", strconv.FormatBool(repo.Settings.RequireSignedCommits),
715 "git daemon", strconv.FormatBool(repo.Settings.GitDaemon), 717 "git daemon", strconv.FormatBool(repo.Settings.GitDaemon),
716 "archived", strconv.FormatBool(repo.Settings.Archived), 718 "archived", strconv.FormatBool(repo.Settings.Archived),
internal/control/status.go +10 −2
@@ -16,13 +16,13 @@ func init() {
16 Summary: "report a commit status (CI)", 16 Summary: "report a commit status (CI)",
17 Usage: "status set <owner/name> <sha> --context <c> --state pending|success|failure|error [--description <d>] [--url <u>]", 17 Usage: "status set <owner/name> <sha> --context <c> --state pending|success|failure|error [--description <d>] [--url <u>]",
18 Flags: []Flag{ 18 Flags: []Flag{
19 {"--context", "<c>", "the check this status reports for", ""}, 19 {"--context", "<c>", "the check this status reports for; ci/ is reserved for the instance's builds", ""},
20 {"--state", "pending|success|failure|error", "the check's outcome", ""}, 20 {"--state", "pending|success|failure|error", "the check's outcome", ""},
21 {"--description", "<d>", "short text shown beside the state", ""}, 21 {"--description", "<d>", "short text shown beside the state", ""},
22 {"--url", "<u>", "link to the check's own output", ""}, 22 {"--url", "<u>", "link to the check's own output", ""},
23 }, 23 },
24 Examples: []string{ 24 Examples: []string{
25 "status set krz/gitbay a1b2c3d --context ci/build --state success", 25 "status set krz/gitbay a1b2c3d --context ext/lint --state success",
26 }, 26 },
27 Run: runStatusSet}) 27 Run: runStatusSet})
28 register(Command{Path: []string{"status", "list"}, 28 register(Command{Path: []string{"status", "list"},
@@ -68,6 +68,14 @@ func runStatusSet(c *Ctx, args []string) int {
68 if path == "" || sha == "" || context == "" || !validStatusState[state] { 68 if path == "" || sha == "" || context == "" || !validStatusState[state] {
69 return c.usage() 69 return c.usage()
70 } 70 }
71 // ci/<job> statuses are the build subsystem's: queued, reused,
72 // skipped and finished by the server itself. A writer who could post
73 // one could mark ci/test green on their own head before, or instead
74 // of, the build (#258). Case-folded, so CI/test is no way around it.
75 if strings.HasPrefix(strings.ToLower(context), "ci/") {
76 return c.fail(protocol.ExitDenied, "the ci/ prefix is reserved for the instance's builds; report under another name, such as ext/%s",
77 strings.TrimPrefix(strings.ToLower(context), "ci/"))
78 }
71 if url != "" && !strings.HasPrefix(url, "https://") && !strings.HasPrefix(url, "http://") { 79 if url != "" && !strings.HasPrefix(url, "https://") && !strings.HasPrefix(url, "http://") {
72 return c.fail(protocol.ExitUsage, "--url must be http(s)") 80 return c.fail(protocol.ExitUsage, "--url must be http(s)")
73 } 81 }
internal/control/status_test.go added +26
@@ -0,0 +1,26 @@
1package control
2
3import (
4 "strings"
5 "testing"
6
7 "gitbay.org/gitbay/internal/protocol"
8 "gitbay.org/gitbay/internal/store"
9)
10
11// ci/<job> statuses are the build subsystem's. A writer who could post
12// one could mark ci/test green on their own head before, or instead of,
13// the build (#258).
14func TestStatusSetRefusesReservedContext(t *testing.T) {
15 st, repo, uid := newQueueTestRepo(t)
16 for _, ctx := range []string{"ci/test", "CI/test", "ci/"} {
17 c, errOut := pruneCtx(st, t.TempDir(), store.User{ID: uid, Username: "alice"})
18 code := Dispatch(c, []string{"status", "set", repo.Path(), "abc1234", "--context", ctx, "--state", "success"})
19 if code != protocol.ExitDenied || !strings.Contains(errOut.String(), "reserved") {
20 t.Errorf("--context %s: exit %d, %s", ctx, code, errOut.String())
21 }
22 }
23 if has, err := st.RepoHasStatuses(repo.ID); err != nil || has {
24 t.Fatalf("a refused status was stored: %v %v", has, err)
25 }
26}
internal/httpd/mrpage_test.go +3
@@ -195,6 +195,9 @@ func TestMRGatesRender(t *testing.T) {
195 if out := render(&control.GatesOut{FastForward: true}); !strings.Contains(out, "All gates met") || !strings.Contains(out, "fast-forward possible") { 195 if out := render(&control.GatesOut{FastForward: true}); !strings.Contains(out, "All gates met") || !strings.Contains(out, "fast-forward possible") {
196 t.Errorf("met gates not rendered:\n%s", out) 196 t.Errorf("met gates not rendered:\n%s", out)
197 } 197 }
198 if out := render(&control.GatesOut{Checks: "pending", ChecksMissing: []string{"ext/deploy"}}); !strings.Contains(out, "waiting on <code>ext/deploy</code>") {
199 t.Errorf("missing required context not rendered:\n%s", out)
200 }
198 if out := render(nil); strings.Contains(out, "Merge gates") { 201 if out := render(nil); strings.Contains(out, "Merge gates") {
199 t.Errorf("gates block on a merge request without gates:\n%s", out) 202 t.Errorf("gates block on a merge request without gates:\n%s", out)
200 } 203 }
internal/httpd/settings.go +4
@@ -105,6 +105,8 @@ func (s *Server) settingsSubmit(w http.ResponseWriter, r *http.Request, u store.
105 argv = []string{"repo", "settings", "git-daemon", repo, onOff(v("git-daemon"))} 105 argv = []string{"repo", "settings", "git-daemon", repo, onOff(v("git-daemon"))}
106 case "require-checks": 106 case "require-checks":
107 argv = []string{"repo", "settings", "require-checks", repo, onOff(v("require-checks"))} 107 argv = []string{"repo", "settings", "require-checks", repo, onOff(v("require-checks"))}
108 case "require-contexts":
109 argv = append([]string{"repo", "settings", "require-contexts", repo}, strings.Fields(v("contexts"))...)
108 case "require-resolved": 110 case "require-resolved":
109 argv = []string{"repo", "settings", "require-resolved", repo, onOff(v("require-resolved"))} 111 argv = []string{"repo", "settings", "require-resolved", repo, onOff(v("require-resolved"))}
110 case "require-codeowners": 112 case "require-codeowners":
@@ -227,6 +229,8 @@ func fieldLabel(field string) string {
227 return "git:// serving" 229 return "git:// serving"
228 case "require-checks": 230 case "require-checks":
229 return "required checks" 231 return "required checks"
232 case "require-contexts":
233 return "required contexts"
230 case "require-approvals": 234 case "require-approvals":
231 return "approvals" 235 return "approvals"
232 case "require-resolved": 236 case "require-resolved":
internal/store/builds.go +13 −8
@@ -450,26 +450,31 @@ func (s *Store) CancelBuild(id int64) error {
450 return nil 450 return nil
451} 451}
452 452
453// SuccessBuildFor finds a passed build of the commit for the job, on any 453// SuccessBuildForTree finds a passed build of the job for a tree rather
454// ref: what a cancelled duplicate can point back at. 454// than a commit: a rebase that changes nothing in the tree has already
455// SuccessBuildForTree is SuccessBuildFor keyed by tree rather than 455// been built (#177). Only a trusted build on the image the job names
456// commit: a rebase that changes nothing in the tree has already been 456// counts: a fork's result, or one from an image the job has left, does
457// built (#177). An empty tree never matches. 457// not stand for the repository's own (#258). A job naming no image
458func (s *Store) SuccessBuildForTree(repoID int64, tree, job string) (Build, bool, error) { 458// matches builds that named none, whichever default the runner used;
459// the CI wiki page says so. An empty tree never matches.
460func (s *Store) SuccessBuildForTree(repoID int64, tree, job, image string) (Build, bool, error) {
459 if tree == "" { 461 if tree == "" {
460 return Build{}, false, nil 462 return Build{}, false, nil
461 } 463 }
462 b, err := scanBuild(s.DB.QueryRow(buildSelect+ 464 b, err := scanBuild(s.DB.QueryRow(buildSelect+
463 " WHERE repo_id = ? AND tree = ? AND job = ? AND status = 'success' ORDER BY number DESC LIMIT 1", repoID, tree, job)) 465 " WHERE repo_id = ? AND tree = ? AND job = ? AND image = ? AND trusted = 1 AND status = 'success'"+
466 " ORDER BY number DESC LIMIT 1", repoID, tree, job, image))
464 if errors.Is(err, sql.ErrNoRows) { 467 if errors.Is(err, sql.ErrNoRows) {
465 return Build{}, false, nil 468 return Build{}, false, nil
466 } 469 }
467 return b, err == nil, err 470 return b, err == nil, err
468} 471}
469 472
473// SuccessBuildFor finds a passed trusted build of the commit for the job,
474// on any ref: what a cancelled duplicate can point back at.
470func (s *Store) SuccessBuildFor(repoID int64, sha, job string) (Build, bool, error) { 475func (s *Store) SuccessBuildFor(repoID int64, sha, job string) (Build, bool, error) {
471 b, err := scanBuild(s.DB.QueryRow(buildSelect+ 476 b, err := scanBuild(s.DB.QueryRow(buildSelect+
472 " WHERE repo_id = ? AND sha = ? AND job = ? AND status = 'success' ORDER BY number DESC LIMIT 1", repoID, sha, job)) 477 " WHERE repo_id = ? AND sha = ? AND job = ? AND trusted = 1 AND status = 'success' ORDER BY number DESC LIMIT 1", repoID, sha, job))
473 if errors.Is(err, sql.ErrNoRows) { 478 if errors.Is(err, sql.ErrNoRows) {
474 return Build{}, false, nil 479 return Build{}, false, nil
475 } 480 }
internal/store/builds_test.go +42 −3
@@ -238,17 +238,56 @@ func TestSuccessBuildForTree(t *testing.T) {
238 if err := s.FinishBuild(b["unit"].ID, "success"); err != nil { 238 if err := s.FinishBuild(b["unit"].ID, "success"); err != nil {
239 t.Fatal(err) 239 t.Fatal(err)
240 } 240 }
241 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit"); !ok || prev.SHA != "aaa" { 241 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit", ""); !ok || prev.SHA != "aaa" {
242 t.Fatalf("success not found by tree: ok=%v prev=%+v", ok, prev) 242 t.Fatalf("success not found by tree: ok=%v prev=%+v", ok, prev)
243 } 243 }
244 if _, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "other"); ok { 244 if _, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "other", ""); ok {
245 t.Error("matched a different job") 245 t.Error("matched a different job")
246 } 246 }
247 if _, ok, _ := s.SuccessBuildForTree(repoID, "", "unit"); ok { 247 if _, ok, _ := s.SuccessBuildForTree(repoID, "", "unit", ""); ok {
248 t.Error("an empty tree matched") 248 t.Error("an empty tree matched")
249 } 249 }
250} 250}
251 251
252// A result stands for another commit only when it came from a trusted
253// build on the same image: a fork's green build, or one on an image the
254// job has since left, proves nothing about the repository's own (#258).
255func TestSuccessReuseNeedsTrustAndImage(t *testing.T) {
256 s := open(t)
257 if err := s.MigrateUp(); err != nil {
258 t.Fatal(err)
259 }
260 uid, _ := s.CreateUser("cmc", true)
261 repoID, _ := s.CreateRepo("user", uid, "app", "public")
262 for _, b := range []struct {
263 sha, image string
264 trusted bool
265 }{
266 {"aaa", "", false},
267 {"bbb", "localhost/old:1", true},
268 } {
269 if _, err := s.CreateBuild(repoID, "unit", b.sha, "main", `["true"]`, b.image, "tree1", b.trusted); err != nil {
270 t.Fatal(err)
271 }
272 claimed, ok, err := s.ClaimBuild([]int64{repoID}, true)
273 if err != nil || !ok {
274 t.Fatalf("claim: ok=%v err=%v", ok, err)
275 }
276 if err := s.FinishBuild(claimed.ID, "success"); err != nil {
277 t.Fatal(err)
278 }
279 }
280 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit", ""); ok {
281 t.Fatalf("reused build %d: untrusted, or on another image", prev.Number)
282 }
283 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit", "localhost/old:1"); !ok || prev.SHA != "bbb" {
284 t.Fatalf("trusted build on the same image not found: ok=%v prev=%+v", ok, prev)
285 }
286 if _, ok, _ := s.SuccessBuildFor(repoID, "aaa", "unit"); ok {
287 t.Error("an untrusted success stood for its commit")
288 }
289}
290
252// A running build whose log stream ended is reaped after StaleLogGrace, 291// A running build whose log stream ended is reaped after StaleLogGrace,
253// well before the deadline; one whose stream is still open is not (#179). 292// well before the deadline; one whose stream is still open is not (#179).
254func TestReapStaleBuildsAfterLogClosed(t *testing.T) { 293func TestReapStaleBuildsAfterLogClosed(t *testing.T) {
internal/store/repos.go +11 −7
@@ -27,13 +27,17 @@ type RepoSettings struct {
27 ProtectedTags []string `json:"protected_tags,omitempty"` // path.Match globs 27 ProtectedTags []string `json:"protected_tags,omitempty"` // path.Match globs
28 RequireSignedCommits bool `json:"require_signed_commits,omitempty"` 28 RequireSignedCommits bool `json:"require_signed_commits,omitempty"`
29 RequireChecks bool `json:"require_checks,omitempty"` 29 RequireChecks bool `json:"require_checks,omitempty"`
30 RequireApprovals int `json:"require_approvals,omitempty"` 30 // RequiredContexts are statuses require_checks waits for whether or
31 RequireResolved bool `json:"require_resolved,omitempty"` 31 // not they have reported; one that has not is pending. Setting a
32 RequireCodeowners bool `json:"require_codeowners,omitempty"` 32 // non-empty list turns RequireChecks on (#258).
33 RequireMR bool `json:"require_mr,omitempty"` 33 RequiredContexts []string `json:"required_contexts,omitempty"`
34 GitDaemon bool `json:"git_daemon,omitempty"` 34 RequireApprovals int `json:"require_approvals,omitempty"`
35 Archived bool `json:"archived,omitempty"` 35 RequireResolved bool `json:"require_resolved,omitempty"`
36 Website string `json:"website,omitempty"` 36 RequireCodeowners bool `json:"require_codeowners,omitempty"`
37 RequireMR bool `json:"require_mr,omitempty"`
38 GitDaemon bool `json:"git_daemon,omitempty"`
39 Archived bool `json:"archived,omitempty"`
40 Website string `json:"website,omitempty"`
37} 41}
38 42
39// Path returns the canonical owner/name form. 43// Path returns the canonical owner/name form.
internal/web/templates/mr.html +1
@@ -122,6 +122,7 @@
122 {{else}}<p class="row"><span class="dot ok"></span>All gates met</p>{{end}} 122 {{else}}<p class="row"><span class="dot ok"></span>All gates met</p>{{end}}
123 {{if .ApprovalsRequired}}<p class="row none">approvals: {{len .Approvals}} of {{.ApprovalsRequired}}{{if .Approvals}} ({{range $i, $a := .Approvals}}{{if $i}}, {{end}}{{$a}}{{end}}){{end}}</p>{{end}} 123 {{if .ApprovalsRequired}}<p class="row none">approvals: {{len .Approvals}} of {{.ApprovalsRequired}}{{if .Approvals}} ({{range $i, $a := .Approvals}}{{if $i}}, {{end}}{{$a}}{{end}}){{end}}</p>{{end}}
124 {{range .OwnersOutstanding}}<p class="row none">waiting on {{range $i, $o := .Owners}}{{if $i}} or {{end}}<a href="/{{$o}}">{{$o}}</a>{{end}} for {{range $i, $f := .Files}}{{if $i}}, {{end}}<code>{{$f}}</code>{{end}}</p>{{end}} 124 {{range .OwnersOutstanding}}<p class="row none">waiting on {{range $i, $o := .Owners}}{{if $i}} or {{end}}<a href="/{{$o}}">{{$o}}</a>{{end}} for {{range $i, $f := .Files}}{{if $i}}, {{end}}<code>{{$f}}</code>{{end}}</p>{{end}}
125 {{range .ChecksMissing}}<p class="row none">waiting on <code>{{.}}</code>, not yet reported</p>{{end}}
125 <p class="row none">{{if .FastForward}}fast-forward possible{{else}}not a fast-forward: rebase, or merge with a merge commit{{end}}</p> 126 <p class="row none">{{if .FastForward}}fast-forward possible{{else}}not a fast-forward: rebase, or merge with a merge commit{{end}}</p>
126 </div>{{end}} 127 </div>{{end}}
127 <div class="grp"> 128 <div class="grp">
internal/web/templates/settings.html +7 −1
@@ -72,10 +72,16 @@
72<p class="meta">Checked before a merge, in this order: checks, approvals, resolved threads, signatures.</p> 72<p class="meta">Checked before a merge, in this order: checks, approvals, resolved threads, signatures.</p>
73<form method="post" action="{{$base}}" class="setform"> 73<form method="post" action="{{$base}}" class="setform">
74 <input type="hidden" name="field" value="require-checks"> 74 <input type="hidden" name="field" value="require-checks">
75 <div><label for="require-checks">Required checks</label><p class="hint">Requires CI to succeed.</p></div> 75 <div><label for="require-checks">Required checks</label><p class="hint">Requires CI to succeed.{{with .Repo.Settings.RequiredContexts}} Also waits for {{range $i, $c := .}}{{if $i}}, {{end}}<code>{{$c}}</code>{{end}} until they report{{if not $.Repo.Settings.RequireChecks}}, once this is on{{end}}.{{end}}</p></div>
76 <div class="check"><input type="checkbox" id="require-checks" name="require-checks" value="on"{{if .Repo.Settings.RequireChecks}} checked{{end}}></div> 76 <div class="check"><input type="checkbox" id="require-checks" name="require-checks" value="on"{{if .Repo.Settings.RequireChecks}} checked{{end}}></div>
77 <div><button type="submit" class="btn">Save</button></div> 77 <div><button type="submit" class="btn">Save</button></div>
78</form> 78</form>
79<form method="post" action="{{$base}}" class="setform">
80 <input type="hidden" name="field" value="require-contexts">
81 <div><label for="contexts">Required contexts</label><p class="hint">Statuses the checks gate waits for until they report, separated by spaces. Saving any turns required checks on; saving none leaves it as it is.</p></div>
82 <div><input type="text" id="contexts" name="contexts" value="{{range $i, $c := .Repo.Settings.RequiredContexts}}{{if $i}} {{end}}{{$c}}{{end}}" autocomplete="off"></div>
83 <div><button type="submit" class="btn">Save</button></div>
84</form>
79<form method="post" action="{{$base}}" class="setform"> 85<form method="post" action="{{$base}}" class="setform">
80 <input type="hidden" name="field" value="require-approvals"> 86 <input type="hidden" name="field" value="require-approvals">
81 <div><label for="approvals">Approvals</label><p class="hint">Approvals from anyone with write access. Zero means none required.</p></div> 87 <div><label for="approvals">Approvals</label><p class="hint">Approvals from anyone with write access. Zero means none required.</p></div>