Commit `314c9ca55c`

314c9ca55c3b7866b5bac4eb806a7e200e42edd4

parent: ba9cd01976

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-11 16:16 UTC

control, sshd, hookd: a cross-repo close is checked against the pushing key

The hook told the daemon which account pushed, not which key. A deploy
key's scope grants nothing outside its binding, so a close naming another
repository has to see the scope as well as the account's access: the
session's scope now travels with the push through GITBAY_KEY_SCOPE and
hookd.Request, and closeTarget requires ScopeAllowsGit on the target
beside CanWrite. An archived target is refused, and targets resolve once
per push rather than once per commit.

Ref #203

cmd/gitbayd/hook.go +1

		@@ -171,6 +171,7 @@ func hookCmd() *cobra.Command {
171	171	Hook: args[0],
172	172	RepoID: repoID,
173	173	UserID: userID,
	174	Scope: os.Getenv(hookd.EnvScope),
174	175	Updates: updates,
175	176	}, func(emit func(hookd.RawCommit) error) error {
176	177	return streamIncomingCommits(updates, emit)

internal/control/commitrefs.go +36 −11

		@@ -34,14 +34,32 @@ const maxMessageCommits = 100
34	34	// landed on the default branch (old..new): closing keywords close the
35	35	// issue, bare #N leaves a reference comment. Each (issue, sha) pair acts
36	36	// at most once, ever. actorID — the pusher or merger — authorizes and
37		// signs the resulting comments; failures are logged, never fatal, because
38		// this runs after the push or merge already succeeded.
39		func ProcessCommitMessages(st *store.Store, dir string, repo store.Repo, actorID int64, old, new string) {
	37	// signs the resulting comments, and scope is the key they used, which a
	38	// cross-repo close is checked against too; failures are logged, never
	39	// fatal, because this runs after the push or merge already succeeded.
	40	func ProcessCommitMessages(st *store.Store, dir string, repo store.Repo, actorID int64, scope, old, new string) {
40	41	msgs, err := gitutil.RevListMessages(dir, old, new, maxMessageCommits)
41	42	if err != nil {
42	43	slog.Error("commit refs: listing messages", "repo", repo.Path(), "err", err)
43	44	return
44	45	}
	46	// Commits in one push name the same repositories over and over, and
	47	// each resolution is three queries; keep the answers, refusals too.
	48	resolved := map[string]struct {
	49	repo store.Repo
	50	ok bool
	51	}{}
	52	target := func(path string) (store.Repo, bool) {
	53	if r, seen := resolved[path]; seen {
	54	return r.repo, r.ok
	55	}
	56	t, ok := closeTarget(st, repo, actorID, scope, path)
	57	resolved[path] = struct {
	58	repo store.Repo
	59	ok bool
	60	}{t, ok}
	61	return t, ok
	62	}
45	63	for _, m := range msgs {
46	64	closes := closingRefs(m.Message)
47	65	local := map[int64]bool{}
		@@ -59,11 +77,11 @@ func ProcessCommitMessages(st *store.Store, dir string, repo store.Repo, actorID
59	77	subject, _, _ := strings.Cut(m.Message, "\n")
60	78	author := authorLink(st, m.AuthorName, m.AuthorEmail)
61	79	for _, ref := range closes {
62		target, ok := closeTarget(st, repo, actorID, ref.Path)
	80	t, ok := target(ref.Path)
63	81	if !ok {
64	82	continue
65	83	}
66		actOnIssue(st, repo, target, actorID, m.SHA, ref.N, true, subject, author)
	84	actOnIssue(st, repo, t, actorID, m.SHA, ref.N, true, subject, author)
67	85	}
68	86	for n := range refs {
69	87	actOnIssue(st, repo, repo, actorID, m.SHA, n, false, subject, author)
		@@ -81,9 +99,9 @@ func ProcessCommitMessages(st *store.Store, dir string, repo store.Repo, actorID
81	99	// key is per merge request rather than the merged sha, because sharing
82	100	// the sha let a bare "#N" in a commit message claim it first and silently
83	101	// suppress the close.
84		func ProcessMRDescription(st *store.Store, repo store.Repo, mr store.MR, actorID int64) {
	102	func ProcessMRDescription(st *store.Store, repo store.Repo, mr store.MR, actorID int64, scope string) {
85	103	for _, ref := range closingRefs(mr.Title + "\n" + mr.Body) {
86		target, ok := closeTarget(st, repo, actorID, ref.Path)
	104	target, ok := closeTarget(st, repo, actorID, scope, ref.Path)
87	105	if !ok {
88	106	continue
89	107	}
		@@ -134,9 +152,10 @@ func closingRefs(text string) []closeRef {
134	152
135	153	// closeTarget resolves where a closing reference acts: the source
136	154	// repository for a bare #N, or the named repository when the actor holds
137		// write there. false means the reference stays text; nothing is logged
138		// above debug, since a refusal must not confirm the target exists.
139		func closeTarget(st *store.Store, source store.Repo, actorID int64, path string) (store.Repo, bool) {
	155	// write there with a key whose scope reaches it. false means the
	156	// reference stays text; nothing is logged above debug, since a refusal
	157	// must not confirm the target exists.
	158	func closeTarget(st *store.Store, source store.Repo, actorID int64, scope, path string) (store.Repo, bool) {
140	159	if path == "" {
141	160	return source, true
142	161	}
		@@ -152,10 +171,16 @@ func closeTarget(st *store.Store, source store.Repo, actorID int64, path string)
152	171	if err != nil {
153	172	return store.Repo{}, false
154	173	}
155		if !policy.CanWrite(actor, target, grant) {
	174	// The account's access and the key's reach both have to hold: a deploy
	175	// key is bound to one repository and inherits nothing from whoever
	176	// registered it, so its scope allows no write anywhere else.
	177	if !policy.CanWrite(actor, target, grant) \|\| !policy.ScopeAllowsGit(scope, target.Path(), true) {
156	178	slog.Debug("commit refs: cross-repo close refused", "source", source.Path(), "target", path)
157	179	return store.Repo{}, false
158	180	}
	181	if target.Settings.Archived {
	182	return store.Repo{}, false
	183	}
159	184	return target, true
160	185	}
161	186

internal/control/commitrefs_test.go +23 −4

		@@ -1,6 +1,7 @@
1	1	package control
2	2
3	3	import (
	4	"fmt"
4	5	"slices"
5	6	"strings"
6	7	"testing"
		@@ -55,22 +56,40 @@ func TestMRDescriptionClosesAcrossRepos(t *testing.T) {
55	56	}
56	57	// carol cannot write acme/priv: the issue stays open and no comment
57	58	// lands.
58		ProcessMRDescription(f.st, f.app, mr(1, "Closes acme/priv#1"), f.carol)
	59	ProcessMRDescription(f.st, f.app, mr(1, "Closes acme/priv#1"), f.carol, "full")
59	60	if iss, _ := f.st.IssueByNumber(f.priv.ID, 1); iss.State != "open" {
60	61	t.Fatal("outsider closed a private repo's issue")
61	62	}
	63	// A deploy key on alice/app is bound to alice/app: alice's own access
	64	// to acme/priv is not the key's to use.
	65	deploy := fmt.Sprintf("deploy:%d:rw", f.app.ID)
	66	ProcessMRDescription(f.st, f.app, mr(2, "Closes acme/priv#1"), f.alice, deploy)
	67	if iss, _ := f.st.IssueByNumber(f.priv.ID, 1); iss.State != "open" {
	68	t.Fatal("a deploy key closed an issue outside its binding")
	69	}
	70	// An archived target is read-only, cross-repo closes included.
	71	if _, err := f.st.UpdateRepoSettings(f.priv.ID, func(rs *store.RepoSettings) { rs.Archived = true }); err != nil {
	72	t.Fatal(err)
	73	}
	74	ProcessMRDescription(f.st, f.app, mr(3, "Closes acme/priv#1"), f.alice, "full")
	75	if iss, _ := f.st.IssueByNumber(f.priv.ID, 1); iss.State != "open" {
	76	t.Fatal("an archived repository's issue was closed")
	77	}
	78	if _, err := f.st.UpdateRepoSettings(f.priv.ID, func(rs *store.RepoSettings) { rs.Archived = false }); err != nil {
	79	t.Fatal(err)
	80	}
62	81	// alice can: it closes with a comment naming the source repository.
63		ProcessMRDescription(f.st, f.app, mr(2, "Closes acme/priv#1"), f.alice)
	82	ProcessMRDescription(f.st, f.app, mr(4, "Closes acme/priv#1"), f.alice, "full")
64	83	iss, _ := f.st.IssueByNumber(f.priv.ID, 1)
65	84	if iss.State != "closed" {
66	85	t.Fatal("writer did not close across repos")
67	86	}
68	87	comments, _ := f.st.ListIssueComments(iss.ID)
69		if len(comments) != 1 \|\| !strings.Contains(comments[0].Body, "(/alice/app/mrs/2)") {
	88	if len(comments) != 1 \|\| !strings.Contains(comments[0].Body, "(/alice/app/mrs/4)") {
70	89	t.Fatalf("close comment = %+v", comments)
71	90	}
72	91	// An unknown path is text; a bare #N still acts in the source repo.
73		ProcessMRDescription(f.st, f.app, mr(3, "Closes nobody/nothing#1 and closes #1"), f.alice)
	92	ProcessMRDescription(f.st, f.app, mr(5, "Closes nobody/nothing#1 and closes #1"), f.alice, "full")
74	93	if iss, _ := f.st.IssueByNumber(f.app.ID, 1); iss.State != "closed" {
75	94	t.Fatal("bare #N stopped working")
76	95	}

internal/control/mr.go +2 −2

		@@ -1208,8 +1208,8 @@ func runMRMerge(c *Ctx, args []string) int {
1208	1208	// description is scanned after them, so a commit wins the attribution
1209	1209	// when both name the same issue.
1210	1210	if mr.TargetRef == repo.DefaultBranch {
1211		ProcessCommitMessages(c.Store, dir, repo, c.User.ID, targetSHA, newSHA)
1212		ProcessMRDescription(c.Store, repo, mr, c.User.ID)
	1211	ProcessCommitMessages(c.Store, dir, repo, c.User.ID, c.Scope, targetSHA, newSHA)
	1212	ProcessMRDescription(c.Store, repo, mr, c.User.ID, c.Scope)
1213	1213	RecordLandedCommits(c.Store, dir, repo, targetSHA, newSHA)
1214	1214	}
1215	1215	// A merge moves the ref directly, so it never reaches post-receive and

internal/hookd/hookd.go +9 −4

		@@ -35,12 +35,17 @@ const (
35	35	EnvSocket = "GITBAY_HOOK_SOCKET"
36	36	EnvRepoID = "GITBAY_REPO_ID"
37	37	EnvUserID = "GITBAY_USER_ID"
	38	EnvScope = "GITBAY_KEY_SCOPE"
38	39	)
39	40
40	41	type Request struct {
41		Hook string `json:"hook"` // pre-receive \| post-receive
42		RepoID int64 `json:"repo_id"`
43		UserID int64 `json:"user_id"`
	42	Hook string `json:"hook"` // pre-receive \| post-receive
	43	RepoID int64 `json:"repo_id"`
	44	UserID int64 `json:"user_id"`
	45	// Scope is the pushing key's scope. The user id alone is the account
	46	// the key belongs to, and a deploy key grants nothing outside its
	47	// binding, so anything acting on another repository needs this too.
	48	Scope string `json:"scope"`
44	49	Updates []policy.RefUpdate `json:"updates"`
45	50	}
46	51
		@@ -237,7 +242,7 @@ func (s *Server) postReceive(req Request) {
237	242	// in their messages (closes #N, plain #N).
238	243	if pushedRepoErr == nil && branch == pushedRepo.DefaultBranch && !u.IsDelete {
239	244	dir := control.RepoDir(s.cfg.Server.Root, pushedRepo.OwnerName, pushedRepo.Name)
240		control.ProcessCommitMessages(s.st, dir, pushedRepo, req.UserID, u.Old, u.New)
	245	control.ProcessCommitMessages(s.st, dir, pushedRepo, req.UserID, req.Scope, u.Old, u.New)
241	246	control.RecordLandedCommits(s.st, dir, pushedRepo, u.Old, u.New)
242	247	}
243	248	// A branch push with a .gitbay/ci.yml queues one build per job.

internal/sshd/sshd.go +1

		@@ -404,6 +404,7 @@ func runGit(cfg config.Config, st *store.Store, user store.User, scope string, a
404	404	hookd.EnvSocket + "=" + hookd.SocketPath(cfg.Server.Root),
405	405	hookd.EnvRepoID + "=" + strconv.FormatInt(repo.ID, 10),
406	406	hookd.EnvUserID + "=" + strconv.FormatInt(user.ID, 10),
	407	hookd.EnvScope + "=" + scope,
407	408	}
408	409	// A storage quota on the owner rides the same mechanism as the pack
409	410	// cap: the pack may be no larger than what the owner has left.

Commit 314c9ca55c

Commit `314c9ca55c`