krz/gitbay

web: mark-read dispatches notifications read; #261 rows in step !522

merged merged by cmc on 2026-09-29 04:46 UTC · krz/gitbay:dispatch-markread-261 into main

6 files changed, +55 −13

Layout: unified · split

.gitbay/wiki/Architecture/04-Trust-Boundaries.org +2 −2
@@ -93,8 +93,8 @@ no HTTP write path (=internal/httpd/smart.go=,
93 decode the JSON result into the template (=internal/httpd/control.go=). 93 decode the JSON result into the template (=internal/httpd/control.go=).
943. Form posts pass =checkOrigin= (=accounts.go=), dispatch the 943. Form posts pass =checkOrigin= (=accounts.go=), dispatch the
95 matching command, and map the exit code to a redirect or an error on 95 matching command, and map the exit code to a redirect or an error on
96 the page. Three toggles (pin, watch, mark read) write the store 96 the page. The pin, watch and mark-read toggles dispatch =repo pin=,
97 directly instead (#261). 97 =repo watch=/=mute=/=unwatch= and =notifications read= the same way.
98 98
99** E. JSON API 99** E. JSON API
100 100
.gitbay/wiki/Architecture/09-Controls.org +2 −2
@@ -9,7 +9,7 @@ chapter names of OWASP ASVS 4.0 where one fits.
9 9
10| Control | Status | Evidence | 10| Control | Status | Evidence |
11|---------------------------------------------+----------+------------------------------------------------------------------| 11|---------------------------------------------+----------+------------------------------------------------------------------|
12| One authorization path for every surface | partial | all surfaces call =control.Dispatch= (=internal/control/control.go=); three web toggles write the store directly (#261) | 12| One authorization path for every surface | in place | all surfaces call =control.Dispatch= (=internal/control/control.go=); web form handlers, including the pin, watch and mark-read toggles, dispatch commands; login and session bookkeeping are not commands |
13| No server-side signing key | in place | =internal/sig= verifies only | 13| No server-side signing key | in place | =internal/sig= verifies only |
14| Least functionality by default | in place | API, web accounts, git://, push and registration default off (=internal/config/config.go=) | 14| Least functionality by default | in place | API, web accounts, git://, push and registration default off (=internal/config/config.go=) |
15| No git library; git runs as a subprocess with built argv | in place | =internal/gitutil= | 15| No git library; git runs as a subprocess with built argv | in place | =internal/gitutil= |
@@ -100,5 +100,5 @@ chapter names of OWASP ASVS 4.0 where one fits.
100| Service hardening | in place | systemd sandboxing ([[file:03-Deployment.org][3]]) | 100| Service hardening | in place | systemd sandboxing ([[file:03-Deployment.org][3]]) |
101| Backups offsite and append-only | in place | restic with append-only credentials (documented) | 101| Backups offsite and append-only | in place | restic with append-only credentials (documented) |
102| Restore tested | gap | tooling in place (=admin restore-drill=, Admin wiki "Restore drill"); clean-host drill pending (#259) | 102| Restore tested | gap | tooling in place (=admin restore-drill=, Admin wiki "Restore drill"); clean-host drill pending (#259) |
103| Migrations validated before commit | gap | foreign-key check runs after commit (#261) | 103| Migrations validated before commit | in place | =PRAGMA foreign_key_check= runs inside the migration transaction, before commit (=internal/store/store.go=) |
104| Signed, reviewed changes to production | in place | signed commits, =require-mr=, ff-only merges, clean-tree deploys | 104| Signed, reviewed changes to production | in place | signed commits, =require-mr=, ff-only merges, clean-tree deploys |
.gitbay/wiki/Architecture/10-Known-Gaps.org −1
@@ -11,7 +11,6 @@ what the 2026-09-27 review found; remove a row when its issue closes.
11| Issue | Area | Gap | Severity | 11| Issue | Area | Gap | Severity |
12|-------+------------------+-----------------------------------------------------------------------+----------| 12|-------+------------------+-----------------------------------------------------------------------+----------|
13| #259 | Recovery | No restore has been exercised; the procedure and tooling (=admin restore-drill=, =backup --verify=) are in place, the clean-host drill is pending | high | 13| #259 | Recovery | No restore has been exercised; the procedure and tooling (=admin restore-drill=, =backup --verify=) are in place, the clean-host drill is pending | high |
14| #261 | Various | Migration foreign-key check after commit; three web writes bypass dispatch; documentation drift | medium |
15 14
16* Not filed 15* Not filed
17 16
CHANGELOG.org +2
@@ -6,6 +6,8 @@ anything beyond "replace the binary and restart" is needed.
6 6
7* Unreleased 7* Unreleased
8 8
9- Marking notifications read from the web dispatches =notifications
10 read=, so it counts against the write budget and is audited (#261).
9- =gitbayd admin backup --verify= also checks every release asset the 11- =gitbayd admin backup --verify= also checks every release asset the
10 database names against its recorded size and sha256, and every 12 database names against its recorded size and sha256, and every
11 archived LFS object against its name (#259). 13 archived LFS object against its name (#259).
internal/httpd/account_test.go +43
@@ -498,3 +498,46 @@ func TestAccountTokenCreateAuditOmitsToken(t *testing.T) {
498 t.Fatal("no cmd token create audit row") 498 t.Fatal("no cmd token create audit row")
499 } 499 }
500} 500}
501
502// Marking notices read goes through notifications read, so it carries the
503// audit trail and write budget of the CLI command (#261).
504func TestNotificationsReadDispatches(t *testing.T) {
505 st, err := store.Open(":memory:")
506 if err != nil {
507 t.Fatal(err)
508 }
509 defer st.Close()
510 if err := st.MigrateUp(); err != nil {
511 t.Fatal(err)
512 }
513 uid, err := st.CreateUser("alice", false)
514 if err != nil {
515 t.Fatal(err)
516 }
517 u := store.User{ID: uid, Username: "alice"}
518 repoID, err := st.CreateRepo("user", uid, "app", "public")
519 if err != nil {
520 t.Fatal(err)
521 }
522 for i := 0; i < 2; i++ {
523 if err := st.AddNotice(uid, repoID, "issue", "bob", "s", "alice/app/issues/1"); err != nil {
524 t.Fatal(err)
525 }
526 }
527 s := New(config.Default(), st, nil)
528
529 notices, _ := st.Inbox(uid, true, 10, 0)
530 req := httptest.NewRequest("POST", "/notifications/read", strings.NewReader("id="+strconv.FormatInt(notices[0].ID, 10)))
531 req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
532 s.notificationsRead(httptest.NewRecorder(), req, u)
533 if n := st.UnreadNotices(uid); n != 1 {
534 t.Fatalf("unread after one id = %d, want 1", n)
535 }
536 assertAudited(t, st, "cmd notifications read")
537
538 req = httptest.NewRequest("POST", "/notifications/read", nil)
539 s.notificationsRead(httptest.NewRecorder(), req, u)
540 if n := st.UnreadNotices(uid); n != 0 {
541 t.Fatalf("unread after all = %d, want 0", n)
542 }
543}
internal/httpd/notifyweb.go +6 −8
@@ -38,20 +38,18 @@ func (s *Server) notifications(w http.ResponseWriter, r *http.Request, u store.U
38} 38}
39 39
40// notificationsRead marks one notice read, or the whole inbox when no id 40// notificationsRead marks one notice read, or the whole inbox when no id
41// is given, then returns to the list. 41// is given, through notifications read, then returns to the list (#261).
42func (s *Server) notificationsRead(w http.ResponseWriter, r *http.Request, u store.User) { 42func (s *Server) notificationsRead(w http.ResponseWriter, r *http.Request, u store.User) {
43 var ids []int64 43 argv := []string{"notifications", "read", "--all"}
44 if v := r.FormValue("id"); v != "" { 44 if v := r.FormValue("id"); v != "" {
45 n, err := strconv.ParseInt(v, 10, 64) 45 if _, err := strconv.ParseInt(v, 10, 64); err != nil {
46 if err != nil {
47 http.Error(w, "bad id", http.StatusBadRequest) 46 http.Error(w, "bad id", http.StatusBadRequest)
48 return 47 return
49 } 48 }
50 ids = append(ids, n) 49 argv = []string{"notifications", "read", v}
51 } 50 }
52 if _, err := s.st.MarkNoticesRead(u.ID, ids); err != nil { 51 if _, msg, ok := s.runControl(u, argv); !ok {
53 http.Error(w, "internal error", http.StatusInternalServerError) 52 s.setFlash(w, msg)
54 return
55 } 53 }
56 http.Redirect(w, r, "/notifications", http.StatusSeeOther) 54 http.Redirect(w, r, "/notifications", http.StatusSeeOther)
57} 55}