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

merged merged by cmc on 2026-09-03 15:59 UTC · krz/gitbay:web-writes-registry into main

3 files changed, +134 −151

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