Commit `01ced7ef9e`

01ced7ef9eff83ca1b088335758fb1a7ecf6ecca

parent: b15d84acd6

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

cmc <hello@cleberg.net> · 2026-09-03 15:39 UTC

web: repo create, issue and MR create, edit and comment dispatch the command

Six handlers called the store directly, so from a browser the per-account
repository quota, the archived-repository refusal, participant
notifications, the stored body format and the audit entry were all
skipped. They now run repo create, issue create, issue edit, mr edit,
issue comment and mr comment through the registry with bodies on stdin,
and map the exit code to the HTTP status. Labels on create and edit go
through issue label, which enforces write access itself.

TestWebWritesGoThroughRegistry: a second repository past the quota is
refused on the form, and an archived repository refuses a web comment.

Closes #93

Layout: unified · split

e2e/webwrites_test.go added +66

@@ -0,0 +1,66 @@
	1	package e2e
	2
	3	import (
	4	"encoding/json"
	5	"net/url"
	6	"strings"
	7	"testing"
	8	)
	9
	10	// Web writes dispatch the command the CLI runs, so a rule the command
	11	// layer enforces holds from a browser too. Repo create used to call the
	12	// store directly and skip the per-account quota; issue comments skipped
	13	// the archived check (#93).
	14	func TestWebWritesGoThroughRegistry(t *testing.T) {
	15	inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n[limits]\nmax_repos_per_user = 1\n")
	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/first"); code != 0 {
	20	t.Fatalf("repo create: %s", errOut)
	21	}
	22	if _, errOut, code := inst.ssh(t, aliceKey, "", "issue", "create", "alice/first", "--title", "one"); code != 0 {
	23	t.Fatalf("issue create: %s", errOut)
	24	}
	25
	26	out, errOut, code := inst.ssh(t, aliceKey, "", "web", "login", "--json")
	27	if code != 0 {
	28	t.Fatalf("web login: %s", errOut)
	29	}
	30	var env struct {
	31	Data struct {
	32	URL string `json:"url"`
	33	} `json:"data"`
	34	}
	35	if err := json.Unmarshal([]byte(out), &env); err != nil {
	36	t.Fatalf("web login JSON: %v\n%s", err, out)
	37	}
	38	browser := newBrowser(t)
	39	if status, _ := browserGet(t, browser, inst.base()+env.Data.URL[strings.Index(env.Data.URL, "/login"):]); status != 200 {
	40	t.Fatalf("login: %d", status)
	41	}
	42
	43	// The quota holds on the web: the form comes back with the refusal and
	44	// no repository exists.
	45	status, body := browserPost(t, browser, inst.base()+"/new", url.Values{"name": {"second"}, "visibility": {"public"}})
	46	if status != 200 \|\| !strings.Contains(body, "role=\"alert\"") {
	47	t.Fatalf("second repo over quota was not refused on the form: %d\n%s", status, body)
	48	}
	49	if _, _, code := inst.ssh(t, aliceKey, "", "repo", "show", "alice/second"); code != 3 {
	50	t.Fatalf("repo created past the quota from the web: exit %d, want 3", code)
	51	}
	52
	53	// An archived repository refuses a comment from the web as it does
	54	// over ssh.
	55	if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "archive", "alice/first"); code != 0 {
	56	t.Fatalf("repo archive: %s", errOut)
	57	}
	58	status, _ = browserPost(t, browser, inst.base()+"/alice/first/issues/1/comment", url.Values{"body": {"late"}})
	59	if status/100 != 4 {
	60	t.Fatalf("comment on an archived repo from the web: %d, want 4xx", status)
	61	}
	62	show, _, _ := inst.ssh(t, aliceKey, "", "issue", "show", "alice/first", "1")
	63	if strings.Contains(show, "late") {
	64	t.Fatalf("archived repo took a web comment:\n%s", show)
	65	}
	66	}

internal/httpd/accounts.go +56 −147

	@@ -4,7 +4,6 @@ import (
4	"fmt"	4	"fmt"
5	"net/http"	5	"net/http"
6	"slices"	6	"slices"
7	"strconv"
8	"strings"	7	"strings"
9	"time"	8	"time"
10		9
	@@ -13,6 +12,7 @@ import (
13	"gitbay.org/gitbay/internal/control"	12	"gitbay.org/gitbay/internal/control"
14	"gitbay.org/gitbay/internal/gitutil"	13	"gitbay.org/gitbay/internal/gitutil"
15	"gitbay.org/gitbay/internal/policy"	14	"gitbay.org/gitbay/internal/policy"
		15	"gitbay.org/gitbay/internal/protocol"
16	"gitbay.org/gitbay/internal/store"	16	"gitbay.org/gitbay/internal/store"
17	)	17	)
18		18
	@@ -133,44 +133,17 @@ func (s Server) newRepoForm(w http.ResponseWriter, r http.Request, u store.Use
133	}	133	}
134		134
135	func (s Server) newRepoSubmit(w http.ResponseWriter, r http.Request, u store.User) {	135	func (s Server) newRepoSubmit(w http.ResponseWriter, r http.Request, u store.User) {
136	name := r.FormValue("name")
137	visibility := "public"
138	if r.FormValue("visibility") == "private" {
139	visibility = "private"
140	}
141	fail := func(msg string) { s.renderNewRepo(w, u, msg) }
142	if err := policy.ValidateName(name); err != nil {
143	fail(err.Error())
144	return
145	}
146	// Owner: yourself, or an org you admin — same rule as repo create.
147	owner := r.FormValue("owner")	136	owner := r.FormValue("owner")
148	ownerKind, ownerID := "user", u.ID
149	if owner == "" {	137	if owner == "" {
150	owner = u.Username	138	owner = u.Username
151	}	139	}
152	if owner != u.Username {	140	name := r.FormValue("name")
153	org, err := s.st.OrgByName(owner)	141	argv := []string{"repo", "create", owner + "/" + name}
154	if err != nil {	142	if r.FormValue("visibility") == "private" {
155	fail("no such organization")	143	argv = append(argv, "--private")
156	return
157	}
158	role, _ := s.st.OrgRole(org.ID, u.ID)
159	if role != "admin" {
160	fail("only admins of " + owner + " can create repositories there")
161	return
162	}
163	ownerKind, ownerID = "org", org.ID
164	}
165	id, err := s.st.CreateRepo(ownerKind, ownerID, name, visibility)
166	if err != nil {
167	fail(err.Error())
168	return
169	}	144	}
170	dir := control.RepoDir(s.cfg.Server.Root, owner, name)	145	if _, msg, ok := s.runControl(u, argv); !ok {
171	if err := gitutil.InitBare(dir, "main", control.HooksDir(s.cfg.Server.Root)); err != nil {	146	s.renderNewRepo(w, u, msg)
172	s.st.DeleteRepo(id)
173	fail("initializing repository failed")
174	return	147	return
175	}	148	}
176	http.Redirect(w, r, "/"+owner+"/"+name, http.StatusSeeOther)	149	http.Redirect(w, r, "/"+owner+"/"+name, http.StatusSeeOther)
	@@ -288,155 +261,91 @@ func (s Server) issueCreateForm(w http.ResponseWriter, r http.Request, u store
288	}{p, body, tplName, templates})	261	}{p, body, tplName, templates})
289	}	262	}
290		263
		264	// Issue and merge request writes run the command the CLI runs, so the
		265	// archived check, notifications, body format and the audit entry have one
		266	// implementation. Bodies travel on stdin, the way --file - does.
		267
291	func (s Server) issueCreateSubmit(w http.ResponseWriter, r http.Request, u store.User) {	268	func (s Server) issueCreateSubmit(w http.ResponseWriter, r http.Request, u store.User) {
292	repo, ok := s.repoForUser(w, r, u, policy.CanRead)	269	repoPath := r.PathValue("owner") + "/" + r.PathValue("repo")
293	if !ok {
294	return
295	}
296	title := strings.TrimSpace(r.FormValue("title"))	270	title := strings.TrimSpace(r.FormValue("title"))
297	if title == "" {	271	code, data, msg := s.dispatchJSON(u, []string{"issue", "create", repoPath, "--title", title, "--file", "-"}, r.FormValue("body"))
298	http.Error(w, "title required", http.StatusBadRequest)	272	if code != protocol.ExitOK {
		273	http.Error(w, msg, statusForExit(code))
299	return	274	return
300	}	275	}
301	n, err := s.st.CreateIssue(repo.ID, u.ID, title, r.FormValue("body"), "md")	276	n := int64(data["number"].(float64))
302	if err != nil {	277	// Labels need write access, matching the SSH rule; the command refuses
303	http.Error(w, "internal error", http.StatusInternalServerError)	278	// otherwise and the issue stands without them.
304	return	279	if args := fieldArgs("--add", r.FormValue("labels")); len(args) > 0 {
305	}	280	s.runControl(u, append([]string{"issue", "label", repoPath, fmt.Sprint(n)}, args...))
306	s.st.RecordEvent(repo.ID, u.ID, "issue.created", fmt.Sprintf(`{"number":%d}`, n))
307	// Labels need write access, matching the SSH rule; ignored otherwise.
308	if labels := strings.Fields(r.FormValue("labels")); len(labels) > 0 {
309	grant, _ := s.st.AccessRole(repo.ID, u.ID)
310	if policy.CanWrite(u, repo, grant) {
311	if iss, err := s.st.IssueByNumber(repo.ID, n); err == nil {
312	for _, l := range labels {
313	s.st.SetIssueLabel(repo.ID, iss.ID, l, true)
314	}
315	}
316	}
317	}	281	}
318	http.Redirect(w, r, fmt.Sprintf("/%s/issues/%d", repo.Path(), n), http.StatusSeeOther)	282	http.Redirect(w, r, fmt.Sprintf("/%s/issues/%d", repoPath, n), http.StatusSeeOther)
319	}	283	}
320		284
321	// issueEditSubmit edits title/body (author or write) and, with write	285	// issueEditSubmit edits title/body (author or write) and, with write
322	// access, replaces the label set.	286	// access, replaces the label set.
323	func (s Server) issueEditSubmit(w http.ResponseWriter, r http.Request, u store.User) {	287	func (s Server) issueEditSubmit(w http.ResponseWriter, r http.Request, u store.User) {
324	repo, ok := s.repoForUser(w, r, u, policy.CanRead)	288	repoPath := r.PathValue("owner") + "/" + r.PathValue("repo")
325	if !ok {	289	n := r.PathValue("n")
326	return
327	}
328	n, _ := strconv.ParseInt(r.PathValue("n"), 10, 64)
329	iss, err := s.st.IssueByNumber(repo.ID, n)
330	if err != nil {
331	http.NotFound(w, r)
332	return
333	}
334	grant, _ := s.st.AccessRole(repo.ID, u.ID)
335	canWrite := policy.CanWrite(u, repo, grant)
336	if iss.Author != u.Username && !canWrite {
337	http.Error(w, "only the author or users with write access can edit", http.StatusForbidden)
338	return
339	}
340	title := strings.TrimSpace(r.FormValue("title"))	290	title := strings.TrimSpace(r.FormValue("title"))
341	if title == "" {	291	code, _, msg := s.dispatchJSON(u, []string{"issue", "edit", repoPath, n, "--title", title, "--file", "-"}, r.FormValue("body"))
342	http.Error(w, "title required", http.StatusBadRequest)	292	if code != protocol.ExitOK {
		293	http.Error(w, msg, statusForExit(code))
343	return	294	return
344	}	295	}
345	body := r.FormValue("body")	296	var cur struct {
346	if err := s.st.UpdateIssueText(iss.ID, &title, &body, nil); err != nil {	297	Labels []string `json:"labels"`
347	http.Error(w, "internal error", http.StatusInternalServerError)
348	return
349	}	298	}
350	if canWrite {	299	if _, ok := s.runControlInto(u, []string{"issue", "show", repoPath, n}, &cur); ok {
351	want := strings.Fields(r.FormValue("labels"))	300	want := strings.Fields(r.FormValue("labels"))
352	for _, l := range iss.Labels {	301	var args []string
		302	for _, l := range cur.Labels {
353	if !slices.Contains(want, l) {	303	if !slices.Contains(want, l) {
354	s.st.SetIssueLabel(repo.ID, iss.ID, l, false)	304	args = append(args, "--remove", l)
355	}	305	}
356	}	306	}
357	for _, l := range want {	307	for _, l := range want {
358	s.st.SetIssueLabel(repo.ID, iss.ID, l, true)	308	if !slices.Contains(cur.Labels, l) {
		309	args = append(args, "--add", l)
		310	}
		311	}
		312	if len(args) > 0 {
		313	s.runControl(u, append([]string{"issue", "label", repoPath, n}, args...))
359	}	314	}
360	}	315	}
361	http.Redirect(w, r, fmt.Sprintf("/%s/issues/%d", repo.Path(), n), http.StatusSeeOther)	316	http.Redirect(w, r, fmt.Sprintf("/%s/issues/%s", repoPath, n), http.StatusSeeOther)
362	}	317	}
363		318
364	// mrEditSubmit edits an MR's title/body (author or write).	319	// mrEditSubmit edits an MR's title/body (author or write).
365	func (s Server) mrEditSubmit(w http.ResponseWriter, r http.Request, u store.User) {	320	func (s Server) mrEditSubmit(w http.ResponseWriter, r http.Request, u store.User) {
366	repo, ok := s.repoForUser(w, r, u, policy.CanRead)	321	repoPath := r.PathValue("owner") + "/" + r.PathValue("repo")
367	if !ok {	322	n := r.PathValue("n")
368	return
369	}
370	n, _ := strconv.ParseInt(r.PathValue("n"), 10, 64)
371	m, err := s.st.MRByNumber(repo.ID, n)
372	if err != nil {
373	http.NotFound(w, r)
374	return
375	}
376	grant, _ := s.st.AccessRole(repo.ID, u.ID)
377	if m.Author != u.Username && !policy.CanWrite(u, repo, grant) {
378	http.Error(w, "only the author or users with write access can edit", http.StatusForbidden)
379	return
380	}
381	title := strings.TrimSpace(r.FormValue("title"))	323	title := strings.TrimSpace(r.FormValue("title"))
382	if title == "" {	324	code, _, msg := s.dispatchJSON(u, []string{"mr", "edit", repoPath, n, "--title", title, "--file", "-"}, r.FormValue("body"))
383	http.Error(w, "title required", http.StatusBadRequest)	325	if code != protocol.ExitOK {
		326	http.Error(w, msg, statusForExit(code))
384	return	327	return
385	}	328	}
386	body := r.FormValue("body")	329	http.Redirect(w, r, fmt.Sprintf("/%s/mrs/%s", repoPath, n), http.StatusSeeOther)
387	if err := s.st.UpdateMRText(m.ID, &title, &body, nil); err != nil {
388	http.Error(w, "internal error", http.StatusInternalServerError)
389	return
390	}
391	http.Redirect(w, r, fmt.Sprintf("/%s/mrs/%d", repo.Path(), n), http.StatusSeeOther)
392	}	330	}
393		331
394	func (s Server) issueCommentSubmit(w http.ResponseWriter, r http.Request, u store.User) {	332	func (s Server) issueCommentSubmit(w http.ResponseWriter, r http.Request, u store.User) {
395	repo, ok := s.repoForUser(w, r, u, policy.CanRead)	333	s.commentSubmit(w, r, u, "issue", "issues")
396	if !ok {
397	return
398	}
399	n, _ := strconv.ParseInt(r.PathValue("n"), 10, 64)
400	iss, err := s.st.IssueByNumber(repo.ID, n)
401	if err != nil {
402	http.NotFound(w, r)
403	return
404	}
405	body := strings.TrimSpace(r.FormValue("body"))
406	if body == "" {
407	http.Error(w, "empty comment", http.StatusBadRequest)
408	return
409	}
410	if err := s.st.AddIssueComment(iss.ID, u.ID, body, "md"); err != nil {
411	http.Error(w, "internal error", http.StatusInternalServerError)
412	return
413	}
414	s.st.RecordEvent(repo.ID, u.ID, "issue.commented", fmt.Sprintf(`{"number":%d}`, n))
415	http.Redirect(w, r, fmt.Sprintf("/%s/issues/%d", repo.Path(), n), http.StatusSeeOther)
416	}	334	}
417		335
418	func (s Server) mrCommentSubmit(w http.ResponseWriter, r http.Request, u store.User) {	336	func (s Server) mrCommentSubmit(w http.ResponseWriter, r http.Request, u store.User) {
419	repo, ok := s.repoForUser(w, r, u, policy.CanRead)	337	s.commentSubmit(w, r, u, "mr", "mrs")
420	if !ok {	338	}
421	return	339
422	}	340	func (s Server) commentSubmit(w http.ResponseWriter, r http.Request, u store.User, noun, segment string) {
423	n, _ := strconv.ParseInt(r.PathValue("n"), 10, 64)	341	repoPath := r.PathValue("owner") + "/" + r.PathValue("repo")
424	m, err := s.st.MRByNumber(repo.ID, n)	342	n := r.PathValue("n")
425	if err != nil {	343	code, _, msg := s.dispatchJSON(u, []string{noun, "comment", repoPath, n, "--file", "-"}, strings.TrimSpace(r.FormValue("body")))
426	http.NotFound(w, r)	344	if code != protocol.ExitOK {
427	return	345	http.Error(w, msg, statusForExit(code))
428	}
429	body := strings.TrimSpace(r.FormValue("body"))
430	if body == "" {
431	http.Error(w, "empty comment", http.StatusBadRequest)
432	return
433	}
434	if err := s.st.AddMRComment(m.ID, u.ID, body, "md"); err != nil {
435	http.Error(w, "internal error", http.StatusInternalServerError)
436	return	346	return
437	}	347	}
438	s.st.RecordEvent(repo.ID, u.ID, "mr.commented", fmt.Sprintf(`{"number":%d}`, n))	348	http.Redirect(w, r, fmt.Sprintf("/%s/%s/%s", repoPath, segment, n), http.StatusSeeOther)
439	http.Redirect(w, r, fmt.Sprintf("/%s/mrs/%d", repo.Path(), n), http.StatusSeeOther)
440	}	349	}
441		350
442	type editPage struct {	351	type editPage struct {

internal/httpd/control.go +12 −4

 // In JSON mode a failure is an envelope carrying the message rather than
 // stderr text, so both paths are read from the same envelope.
 func (s *Server) runControlJSON(u store.User, argv []string) (data map[string]any, msg string, ok bool) {
+	code, data, msg := s.dispatchJSON(u, argv, "")
+	return data, msg, code == protocol.ExitOK
+}
+// dispatchJSON runs a command in JSON mode with stdin, and returns its
+// exit code with the decoded data or the failure message. Handlers that
+// answer a form use the code to pick an HTTP status.
+func (s *Server) dispatchJSON(u store.User, argv []string, stdin string) (code int, data map[string]any, msg string) {
 	var stdout, stderr bytes.Buffer
 	ctx := &control.Ctx{
 		User:   u,
 		Scope:  "full",
 		Store:  s.st,
 		Cfg:    s.cfg,
-		Stdin:  strings.NewReader(""),
+		Stdin:  strings.NewReader(stdin),
 		Stdout: &stdout,
 		Stderr: &stderr,
 		JSON:   true,
 		ViaAPI: true,
 	}
-	code := control.Dispatch(ctx, argv)
+	code = control.Dispatch(ctx, argv)
 	var env struct {
 		Data  map[string]any `json:"data"`
 		Error string         `json:"error"`
 		if m == "" {
 			m = "the command failed"
 		}
-		return nil, m, false
+		return code, nil, m
 	}
-	return env.Data, "", true
+	return code, env.Data, ""
 }
 // authorNames maps commit author addresses to account names for one

Commit 01ced7ef9e

Commit `01ced7ef9e`