Enter git-forgeHEADmaster

A small self-hosted git forge: HTTP push and pull, cgit browsing,
local and OIDC accounts, TOTP, scoped push tokens, per-repo
permissions, issues, a wiki, and outbound mail.

It does not reimplement git. git-http-backend and cgit run as CGI
behind its own listener, which authorises every request before
exec'ing either. One binary, one data directory, one listener: no
database, no JavaScript, and state is JSON in a git repo with a
working tree.

1 files changed, 692 insertions, 0 deletions

diff --git a/admin_test.go b/admin_test.go
new file mode 100644
index 0000000..94ba8c7
--- /dev/null
+++ b/admin_test.go
@@ -0,0 +1,692 @@
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+)
+
+// The name is on the header, in every mail subject, in the otpauth URI, and in
+// the realm git prompts with. All four have to move together.
+func TestSiteRenameReachesEverything(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	addRepo(t, f, "priv", Repo{Visibility: visPrivate, Issues: featDisabled, Wiki: featDisabled})
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+
+	form := url.Values{
+		"csrf":      {f.csrf(withCookies("/", session))},
+		"site_name": {"Renamed"}, "site_desc": {"a subtitle"},
+	}
+	if w := post(h, "/admin/site", form, session); w.Code != http.StatusSeeOther {
+		t.Fatalf("saving the site: status %d", w.Code)
+	}
+
+	if got := f.subject("hello"); got != "[Renamed] hello" {
+		t.Errorf("a mail subject reads %q", got)
+	}
+	if got := f.totpURI("alice", knownSecret); !strings.Contains(got, "Renamed") {
+		t.Errorf("the otpauth uri reads %q", got)
+	}
+	challenge := get(h, "/priv.git/info/refs?service=git-upload-pack", nil)
+	if got := challenge.Header().Get("WWW-Authenticate"); !strings.Contains(got, "Renamed") {
+		t.Errorf("git is prompted with %q", got)
+	}
+	if body := get(h, "/", nil).Body.String(); !strings.Contains(body, "Renamed") {
+		t.Error("the header does not carry the new name")
+	}
+
+	// Refused at the door rather than escaped everywhere the name lands.
+	form.Set("site_name", `Ren"amed`)
+	if w := post(h, "/admin/site", form, session); w.Code != http.StatusBadRequest {
+		t.Errorf("a name with a quote in it was accepted: status %d", w.Code)
+	}
+	if got := f.siteName(); got != "Renamed" {
+		t.Errorf("the refused name was stored anyway: %q", got)
+	}
+}
+
+// saveSite posts the /admin/site form. A nil image leaves that part out, which
+// is how the form arrives when no file was chosen.
+func saveSite(t *testing.T, f *Forge, h http.Handler, session []*http.Cookie, name string, logo, icon []byte) int {
+	t.Helper()
+	var body bytes.Buffer
+	mw := multipart.NewWriter(&body)
+	mw.WriteField("csrf", f.csrf(withCookies("/", session)))
+	mw.WriteField("site_name", name)
+	mw.WriteField("site_desc", "")
+	add := func(field string, image []byte) {
+		if image == nil {
+			return
+		}
+		part, err := mw.CreateFormFile(field, field)
+		if err != nil {
+			t.Fatal(err)
+		}
+		part.Write(image)
+	}
+	add("logo_file", logo)
+	add("icon_file", icon)
+	mw.Close()
+
+	r := httptest.NewRequest("POST", "/admin/site", &body)
+	r.Header.Set("Content-Type", mw.FormDataContentType())
+	for _, c := range session {
+		r.AddCookie(c)
+	}
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, r)
+	return w.Code
+}
+
+// A save that was refused must change nothing. Both images are written to the
+// config directory, so anything refused after one of them would leave the new
+// file there, serving from the next restart.
+func TestARefusedSaveKeepsTheImages(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+
+	kept := []byte("\x89PNG\r\n\x1a\n" + strings.Repeat("k", 64))
+	refused := []byte("\x89PNG\r\n\x1a\n" + strings.Repeat("r", 64))
+	svg := []byte(`<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg"></svg>`)
+
+	if code := saveSite(t, f, h, session, "Good Name", kept, nil); code != http.StatusSeeOther {
+		t.Fatalf("saving a logo: status %d", code)
+	}
+	if code := saveSite(t, f, h, session, `Bad"Name`, refused, nil); code != http.StatusBadRequest {
+		t.Fatalf("a name with a quote in it: status %d", code)
+	}
+	// And a logo this save would otherwise have taken, refused for the favicon
+	// posted beside it.
+	if code := saveSite(t, f, h, session, "Good Name", refused, svg); code != http.StatusBadRequest {
+		t.Fatalf("an svg favicon: status %d", code)
+	}
+
+	// What the running process serves, and what the next start would.
+	if got := f.site.Load().Logo.Body; !bytes.Equal(got, kept) {
+		t.Error("the refused image is already being served")
+	}
+	cfg, err := readConfig(f.store.File("forge.json"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	site, err := loadSite(f.store.configDir, cfg.SiteName, cfg.SiteDesc, cfg.Logo, cfg.Icon)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(site.Logo.Body, kept) {
+		t.Error("the refused image is what a restart would serve")
+	}
+
+	// A save that fails after the image has landed: with no config file to
+	// rewrite, the config write fails, and the image has to be put back.
+	if err := os.Remove(f.store.File("forge.json")); err != nil {
+		t.Fatal(err)
+	}
+	if code := saveSite(t, f, h, session, "Good Name", refused, nil); code != http.StatusInternalServerError {
+		t.Fatalf("a save with no config file to write: status %d", code)
+	}
+	if got, err := os.ReadFile(f.store.File("logo")); err != nil || !bytes.Equal(got, kept) {
+		t.Errorf("the failed save left %d bytes where the next start reads the logo: %v", len(got), err)
+	}
+}
+
+// A scalar override is an overlay. Reading through it to rewrite the file
+// would turn the override into the file's own value, permanently.
+func TestSavingTheSiteKeepsTheEnvironmentOut(t *testing.T) {
+	t.Setenv("GIT_FORGE_LISTEN", "127.0.0.1:9999")
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	if f.cfg.Listen != "127.0.0.1:9999" {
+		t.Fatalf("the override did not take: the process is on %q", f.cfg.Listen)
+	}
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+
+	form := url.Values{
+		"csrf":      {f.csrf(withCookies("/", session))},
+		"site_name": {"Renamed"}, "site_desc": {""},
+	}
+	if w := post(h, "/admin/site", form, session); w.Code != http.StatusSeeOther {
+		t.Fatalf("saving the site: status %d", w.Code)
+	}
+	data, err := os.ReadFile(f.store.File("forge.json"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(data), `"site_name": "Renamed"`) {
+		t.Fatalf("the name was not saved at all:\n%s", data)
+	}
+	if strings.Contains(string(data), "9999") {
+		t.Errorf("the environment was written into the file:\n%s", data)
+	}
+}
+
+// /setup hands out the global admin flag, so two people posting the form at
+// the same moment must not both get through it.
+func TestSetupClosesOnce(t *testing.T) {
+	f := testForge(t)
+	token := randomToken(16)
+	f.setupToken.Store(&token)
+	h := f.routes()
+
+	var wg sync.WaitGroup
+	for i := range 4 {
+		wg.Add(2)
+		go func() {
+			defer wg.Done()
+			post(h, "/setup", url.Values{
+				"token": {token}, "name": {fmt.Sprintf("admin%d", i)},
+				"password": {"hunter2hunter2"}, "again": {"hunter2hunter2"},
+			}, nil)
+		}()
+		// And a reader of the same field, for the race detector.
+		go func() { defer wg.Done(); get(h, "/setup", nil) }()
+	}
+	wg.Wait()
+
+	if n := len(f.store.Users()); n != 1 {
+		t.Errorf("%d accounts came out of one setup form", n)
+	}
+	if code := get(h, "/setup", nil).Code; code != http.StatusNotFound {
+		t.Errorf("/setup is still there after setup: status %d", code)
+	}
+}
+
+// An uploaded image is bytes handed back to a browser.
+func TestImageIsCheckedBeforeItIsKept(t *testing.T) {
+	png := []byte("\x89PNG\r\n\x1a\n" + strings.Repeat("x", 64))
+	if err := checkImage(png); err != nil {
+		t.Errorf("a png was refused: %v", err)
+	}
+	svg := []byte(`<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>`)
+	if err := checkImage(svg); err == nil {
+		t.Error("an svg was accepted, and svg can carry script")
+	}
+	if err := checkImage([]byte("#!/bin/sh\nrm -rf /\n")); err == nil {
+		t.Error("a shell script was accepted as an image")
+	}
+	if err := checkImage(append(png, make([]byte, maxImage)...)); err == nil {
+		t.Error("an oversized image was accepted")
+	}
+}
+
+// One shape in repos.json, not two. Driven through the route, because what
+// adopting records is the settings it was already being served with.
+func TestAdoptedRepoLooksLikeACreatedOne(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	if err := gitInit(filepath.Join(f.store.reposDir, "orphan.git"), true); err != nil {
+		t.Fatal(err)
+	}
+	if err := f.store.scanRepos(); err != nil {
+		t.Fatal(err)
+	}
+
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+	form := url.Values{"csrf": {f.csrf(withCookies("/", session))}, "path": {"orphan"}}
+	if w := post(h, "/admin/repo/adopt", form, session); w.Code != http.StatusSeeOther {
+		t.Fatalf("adopting: status %d", w.Code)
+	}
+	repo, ok := f.store.Repo("orphan")
+	if !ok || repo.Visibility != visPrivate || repo.Issues != featDisabled || repo.Wiki != featDisabled {
+		t.Errorf("adopting changed how it is served: %+v", repo)
+	}
+	data, err := os.ReadFile(f.store.File("repos.json"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(data), "null") {
+		t.Errorf("an adopted repository is written differently from a created one:\n%s", data)
+	}
+	if w := post(h, "/admin/repo/adopt", form, session); w.Code != http.StatusBadRequest {
+		t.Errorf("adopting a repository that already has an entry: status %d", w.Code)
+	}
+}
+
+// The sweep runs across every repository rather than the one an admin happens
+// to be looking at, and it takes nobody else's grant with it.
+func TestDeletingAnAccountTakesItsGrants(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addUser(t, f, "bob", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	for _, path := range []string{"one", "two"} {
+		addRepo(t, f, path, Repo{
+			Visibility: visPrivate, Issues: featDisabled, Wiki: featDisabled,
+			Grants: map[string]string{"alice": grantAdmin, "bob": grantWrite},
+		})
+	}
+
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+	form := func(name string) url.Values {
+		return url.Values{
+			"csrf": {f.csrf(withCookies("/", session))}, "name": {name},
+			"code": {freshCode(t, f, "alice", knownSecret)},
+		}
+	}
+	// Capitalised, because the shift key is forgiven wherever a name is typed.
+	if w := post(h, "/admin/user/delete", form("Bob"), session); w.Code != http.StatusSeeOther {
+		t.Fatalf("deleting bob: status %d", w.Code)
+	}
+	if _, ok := f.store.User("bob"); ok {
+		t.Error("the account survived being deleted")
+	}
+	for _, path := range []string{"one", "two"} {
+		repo, _ := f.store.Repo(path)
+		if _, ok := repo.Grants["bob"]; ok {
+			t.Errorf("%s still grants to a deleted account: %v", path, repo.Grants)
+		}
+		if repo.Grants["alice"] != grantAdmin {
+			t.Errorf("%s lost somebody else's grant: %v", path, repo.Grants)
+		}
+	}
+	// Administration is web only, so the last way in cannot be deleted either.
+	if w := post(h, "/admin/user/delete", form("alice"), session); w.Code != http.StatusBadRequest {
+		t.Errorf("deleting the last admin: status %d", w.Code)
+	}
+	if _, ok := f.store.User("alice"); !ok {
+		t.Error("the last admin was deleted anyway")
+	}
+}
+
+// A grant on an unadopted directory has to record the settings it is already
+// served with, rather than fail validation on the fields it does not have.
+func TestGrantOnAnUnadoptedRepo(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addUser(t, f, "bob", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	if err := gitInit(filepath.Join(f.store.reposDir, "orphan.git"), true); err != nil {
+		t.Fatal(err)
+	}
+	if err := f.store.scanRepos(); err != nil {
+		t.Fatal(err)
+	}
+
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+	form := url.Values{
+		"csrf": {f.csrf(withCookies("/", session))}, "account": {"bob"},
+		"level": {grantRead}, "code": {freshCode(t, f, "alice", knownSecret)},
+	}
+	if w := post(h, "/orphan/settings/grant", form, session); w.Code != http.StatusSeeOther {
+		t.Fatalf("granting on an unadopted repository: status %d", w.Code)
+	}
+	repo, ok := f.store.Repo("orphan")
+	if !ok || repo.Grants["bob"] != grantRead {
+		t.Fatalf("the grant did not land: %+v", repo)
+	}
+	if repo.Visibility != visPrivate || repo.Issues != featDisabled || repo.Wiki != featDisabled {
+		t.Errorf("adopting it by granting changed how it is served: %+v", repo)
+	}
+}
+
+// A repository admin holding no authenticator is refused rather than quietly
+// exempted, and the page says so rather than drawing a form that cannot work.
+func TestGrantNeedsAnAuthenticator(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "owner", "hunter2hunter2")
+	addUser(t, f, "bob", "hunter2hunter2")
+	addRepo(t, f, "notes", Repo{
+		Visibility: visPrivate, Issues: featDisabled, Wiki: featDisabled,
+		Grants: map[string]string{"owner": grantAdmin},
+	})
+	h := f.routes()
+	session := signIn(t, f, h, "owner")
+
+	page := get(h, "/notes/settings", session)
+	if page.Code != http.StatusOK {
+		t.Fatalf("the settings page: status %d", page.Code)
+	}
+	if body := page.Body.String(); !strings.Contains(body, "/settings/totp") {
+		t.Error("the page does not say where to enrol an authenticator")
+	}
+	if strings.Contains(page.Body.String(), `name="level"`) {
+		t.Error("the page draws a grant form this account cannot submit")
+	}
+
+	form := url.Values{
+		"csrf": {f.csrf(withCookies("/", session))}, "account": {"bob"}, "level": {grantRead},
+	}
+	if w := post(h, "/notes/settings/grant", form, session); w.Code != http.StatusForbidden {
+		t.Errorf("a grant with no authenticator behind it: status %d", w.Code)
+	}
+	if repo, _ := f.store.Repo("notes"); repo.Grants["bob"] != "" {
+		t.Error("the grant landed anyway")
+	}
+}
+
+// Creating a repository, turning a feature on, and deleting the lot are the
+// three most consequential things an admin does, and none of them was driven
+// through its route by anything but the container smoke test.
+func TestRepoLifeThroughTheRoutes(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+	form := func(v url.Values) url.Values {
+		v.Set("csrf", f.csrf(withCookies("/", session)))
+		return v
+	}
+
+	create := form(url.Values{
+		"section": {"infra"}, "name": {"dns"}, "desc": {"zones"},
+		"visibility": {visPrivate}, "issues": {featMembers}, "wiki": {featDisabled},
+	})
+	if w := post(h, "/admin/repo/create", create, session); w.Code != http.StatusSeeOther {
+		t.Fatalf("creating a repository in a section: status %d, %s", w.Code, w.Body)
+	}
+	repo, ok := f.store.Repo("infra/dns")
+	if !ok || repo.Desc != "zones" || repo.Visibility != visPrivate || repo.Issues != featMembers {
+		t.Fatalf("the record is not what the form said: %+v", repo)
+	}
+	// The wiki was off, so it has no repository yet.
+	if _, err := os.Stat(filepath.Join(f.store.reposDir, "infra/dns.wiki.git")); err == nil {
+		t.Error("a disabled wiki was given a repository anyway")
+	}
+	// A name that is not a usable path is refused rather than coerced.
+	bad := form(url.Values{"name": {"Notes"}, "visibility": {visPrivate}, "issues": {featDisabled}, "wiki": {featDisabled}})
+	if w := post(h, "/admin/repo/create", bad, session); w.Code != http.StatusBadRequest {
+		t.Errorf("an uppercase name: status %d", w.Code)
+	}
+
+	// Turning the wiki on has to bring its repository with it, or the tab
+	// leads to a page with nothing behind it.
+	save := form(url.Values{
+		"desc": {"zones"}, "visibility": {visPrivate},
+		"issues": {featMembers}, "wiki": {featMembers}, "labels": {"bug\n bug \n\nazz"},
+		"code": {freshCode(t, f, "alice", knownSecret)}, // the wiki knob moves
+	})
+	if w := post(h, "/infra/dns/settings", save, session); w.Code != http.StatusSeeOther {
+		t.Fatalf("saving settings: status %d", w.Code)
+	}
+	if _, err := os.Stat(filepath.Join(f.store.reposDir, "infra/dns.wiki.git")); err != nil {
+		t.Errorf("turning the wiki on left it with no repository: %v", err)
+	}
+	if code := get(h, "/infra/dns/wiki", session).Code; code != http.StatusOK {
+		t.Errorf("the wiki that was just turned on answered %d", code)
+	}
+	// One label once, sorted, and the blank line is not a label.
+	if repo, _ := f.store.Repo("infra/dns"); len(repo.Labels) != 2 ||
+		repo.Labels[0] != "azz" || repo.Labels[1] != "bug" {
+		t.Errorf("the labels are %q", repo.Labels)
+	}
+	// users on a private repository is refused, not coerced.
+	if w := post(h, "/infra/dns/settings", form(url.Values{
+		"visibility": {visPrivate}, "issues": {featUsers}, "wiki": {featDisabled},
+	}), session); w.Code != http.StatusBadRequest {
+		t.Errorf("issues set to users on a private repository: status %d", w.Code)
+	}
+
+	// Deletion takes all three directories and every token pointing at the
+	// path, because the path can be created again.
+	addToken(t, f, "alice", "infra/dns")
+	addToken(t, f, "alice", "elsewhere")
+	del := form(url.Values{"confirm": {"infra/dns"}, "code": {freshCode(t, f, "alice", knownSecret)}})
+	if w := post(h, "/infra/dns/settings/delete", del, session); w.Code != http.StatusSeeOther {
+		t.Fatalf("deleting: status %d", w.Code)
+	}
+	if _, ok := f.store.Repo("infra/dns"); ok {
+		t.Error("the repository survived deletion")
+	}
+	for _, suffix := range []string{".git", ".issues.git", ".wiki.git"} {
+		if _, err := os.Stat(filepath.Join(f.store.reposDir, "infra/dns"+suffix)); err == nil {
+			t.Errorf("infra/dns%s is still on disk", suffix)
+		}
+	}
+	// The section it was alone in goes with it.
+	if _, err := os.Stat(filepath.Join(f.store.reposDir, "infra")); err == nil {
+		t.Error("the empty section was left behind")
+	}
+	u, _ := f.store.User("alice")
+	if len(u.Tokens) != 1 || u.Tokens[0].Repo != "elsewhere" {
+		t.Errorf("the token sweep took the wrong ones: %+v", u.Tokens)
+	}
+}
+
+// An admin never handles a password: the account is made with none and a one
+// time link is displayed for the admin to convey.
+func TestUserCreateThroughTheRoute(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+	form := func(name, code string) url.Values {
+		return url.Values{
+			"csrf": {f.csrf(withCookies("/", session))}, "name": {name},
+			"email": {"bob@example.com"}, "code": {code},
+		}
+	}
+
+	// It hands out a way in, so it costs the acting admin's own code.
+	if w := post(h, "/admin/user/create", form("bob", "000000"), session); w.Code != http.StatusForbidden {
+		t.Errorf("creating an account with a wrong code: status %d", w.Code)
+	}
+	if _, ok := f.store.User("bob"); ok {
+		t.Fatal("the account was made anyway")
+	}
+
+	w := post(h, "/admin/user/create", form("Bob", freshCode(t, f, "alice", knownSecret)), session)
+	if w.Code != http.StatusOK {
+		t.Fatalf("creating an account: status %d", w.Code)
+	}
+	bob, ok := f.store.User("bob")
+	if !ok {
+		t.Fatalf("no account was made: %v", f.store.Users())
+	}
+	if bob.Admin || bob.Password != "" || bob.Origin != originLocal {
+		t.Errorf("a new account should hold no flag and no password: %+v", bob)
+	}
+	// Off the page as the template wrote it, so the separators are escaped.
+	link := strings.ReplaceAll(between(w.Body.String(), "<pre>", "</pre>"), "&amp;", "&")
+	if !strings.Contains(link, "/set-password?") {
+		t.Fatalf("no one time link on the page: %q", link)
+	}
+	// And it works, once.
+	if _, _, ok := f.checkSetPassword(mustQuery(t, link)); !ok {
+		t.Error("the link the admin was given does not verify")
+	}
+	if w := post(h, "/admin/user/create", form("bob", freshCode(t, f, "alice", knownSecret)), session); w.Code != http.StatusBadRequest {
+		t.Errorf("creating the same account twice: status %d", w.Code)
+	}
+}
+
+func mustQuery(t *testing.T, link string) url.Values {
+	t.Helper()
+	u, err := url.Parse(link)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return u.Query()
+}
+
+// The logo an admin uploads is served back from the config directory, and an
+// empty setting means cgit's own, which nothing links to.
+func TestUploadedLogoIsServed(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+
+	if code := get(h, "/logo", nil).Code; code != http.StatusNotFound {
+		t.Errorf("/logo with none configured answered %d", code)
+	}
+	logo := []byte("\x89PNG\r\n\x1a\n" + strings.Repeat("p", 64))
+	if code := saveSite(t, f, h, session, "Forge", logo, nil); code != http.StatusSeeOther {
+		t.Fatalf("saving a logo: status %d", code)
+	}
+
+	got := get(h, "/logo", nil)
+	if got.Code != http.StatusOK || !bytes.Equal(got.Body.Bytes(), logo) {
+		t.Errorf("/logo answered %d with %d bytes", got.Code, got.Body.Len())
+	}
+	if kind := got.Header().Get("Content-Type"); kind != "image/png" {
+		t.Errorf("/logo is served as %q", kind)
+	}
+	// And the header points at it rather than at cgit's.
+	if page := get(h, "/", nil).Body.String(); !strings.Contains(page, `src="/logo"`) {
+		t.Error("the header does not point at the uploaded logo")
+	}
+}
+
+// The favicon is its own setting, served with the type its bytes say, and
+// cgit's own until one is uploaded.
+func TestUploadedFaviconIsServed(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "alice", "hunter2hunter2")
+	addAdmin(t, f, "alice")
+	h := f.routes()
+	session := codeSession(t, f, h, "alice", "hunter2hunter2")
+
+	// Guarded, because cgit's own file is what there is to fall back to.
+	if want := f.cgit.favicon; len(want) > 0 {
+		if got := get(h, "/favicon.ico", nil); !bytes.Equal(got.Body.Bytes(), want) {
+			t.Errorf("/favicon.ico with none configured answered %d, not cgit's own", got.Code)
+		}
+	}
+
+	icon := []byte("\x89PNG\r\n\x1a\n" + strings.Repeat("i", 64))
+	if code := saveSite(t, f, h, session, "Forge", nil, icon); code != http.StatusSeeOther {
+		t.Fatalf("saving a favicon: status %d", code)
+	}
+
+	got := get(h, "/favicon.ico", nil)
+	if got.Code != http.StatusOK || !bytes.Equal(got.Body.Bytes(), icon) {
+		t.Errorf("/favicon.ico answered %d with %d bytes", got.Code, got.Body.Len())
+	}
+	if kind := got.Header().Get("Content-Type"); kind != "image/png" {
+		t.Errorf("/favicon.ico is served as %q", kind)
+	}
+	// The logo is its own setting, and posting one did not touch it.
+	if code := get(h, "/logo", nil).Code; code != http.StatusNotFound {
+		t.Errorf("uploading a favicon configured a logo too: /logo answered %d", code)
+	}
+
+	// A later save with no file chosen is not a save that clears it.
+	if code := saveSite(t, f, h, session, "Forge", nil, nil); code != http.StatusSeeOther {
+		t.Fatalf("a save with no file chosen: status %d", code)
+	}
+	if got := get(h, "/favicon.ico", nil); !bytes.Equal(got.Body.Bytes(), icon) {
+		t.Error("a save with no file chosen dropped the favicon")
+	}
+}
+
+// Who may read or write outlives the session that changes it, so moving one of
+// those knobs costs a code. A description does not.
+func TestKnobChangeNeedsACode(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "owner", "hunter2hunter2")
+	addTOTP(t, f, "owner")
+	addRepo(t, f, "notes", Repo{
+		Desc: "before", Visibility: visPrivate, Issues: featDisabled, Wiki: featDisabled,
+		Labels: []string{}, Grants: map[string]string{"owner": grantAdmin},
+	})
+	h := f.routes()
+	session := codeSession(t, f, h, "owner", "hunter2hunter2")
+	form := func(v url.Values) url.Values {
+		v.Set("csrf", f.csrf(withCookies("/", session)))
+		return v
+	}
+
+	desc := form(url.Values{
+		"desc": {"after"}, "visibility": {visPrivate},
+		"issues": {featDisabled}, "wiki": {featDisabled},
+	})
+	if w := post(h, "/notes/settings", desc, session); w.Code != http.StatusSeeOther {
+		t.Fatalf("a description with no code: status %d", w.Code)
+	}
+	if repo, _ := f.store.Repo("notes"); repo.Desc != "after" {
+		t.Errorf("the description is %q", repo.Desc)
+	}
+
+	open := url.Values{
+		"desc": {"after"}, "visibility": {visPublic},
+		"issues": {featDisabled}, "wiki": {featDisabled},
+	}
+	if w := post(h, "/notes/settings", form(open), session); w.Code != http.StatusForbidden {
+		t.Fatalf("opening the repository with no code: status %d", w.Code)
+	}
+	if repo, _ := f.store.Repo("notes"); repo.Visibility != visPrivate {
+		t.Fatal("a repository was opened without a code")
+	}
+	open.Set("code", freshCode(t, f, "owner", knownSecret))
+	if w := post(h, "/notes/settings", form(open), session); w.Code != http.StatusSeeOther {
+		t.Fatalf("opening the repository with a code: status %d", w.Code)
+	}
+	if repo, _ := f.store.Repo("notes"); repo.Visibility != visPublic {
+		t.Error("the code was right and the repository is still private")
+	}
+}
+
+// A save carries the fields the form owns and nothing else, so a grant handed
+// out while it was open is still there afterwards.
+func TestSettingsSaveKeepsGrants(t *testing.T) {
+	f := testForge(t)
+	addUser(t, f, "owner", "hunter2hunter2")
+	addTOTP(t, f, "owner")
+	addRepo(t, f, "notes", Repo{
+		Desc: "before", Visibility: visPrivate, Issues: featDisabled, Wiki: featDisabled,
+		Labels: []string{}, Grants: map[string]string{"owner": grantAdmin},
+	})
+	h := f.routes()
+	session := codeSession(t, f, h, "owner", "hunter2hunter2")
+
+	// Straight into repos.json, standing in for a grant that landed while the
+	// request that follows was in flight.
+	err := f.store.UpdateRepos("repos: grant bob", f.store.forgeAuthor(), func(m map[string]Repo) error {
+		entry := m["notes"]
+		entry.Grants = map[string]string{"owner": grantAdmin, "bob": grantWrite}
+		m["notes"] = entry
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	stale, _ := f.store.Repo("notes")
+	stale.Grants = map[string]string{"owner": grantAdmin} // what the request read
+	f.repoSettingsSave(httptest.NewRecorder(), withForm(f, session, url.Values{
+		"desc": {"after"}, "visibility": {visPrivate},
+		"issues": {featDisabled}, "wiki": {featDisabled},
+	}), "notes", stale)
+
+	repo, _ := f.store.Repo("notes")
+	if repo.Desc != "after" {
+		t.Errorf("the description is %q", repo.Desc)
+	}
+	if repo.Grants["bob"] != grantWrite {
+		t.Errorf("the save dropped a grant it never read: %v", repo.Grants)
+	}
+}
+
+// withForm is a parsed POST, for driving a handler below the router.
+func withForm(f *Forge, session []*http.Cookie, v url.Values) *http.Request {
+	v.Set("csrf", f.csrf(withCookies("/", session)))
+	r := httptest.NewRequest("POST", "/notes/settings", strings.NewReader(v.Encode()))
+	r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	for _, c := range session {
+		r.AddCookie(c)
+	}
+	r.ParseForm()
+	return r
+}