package api
import (
"os"
"reflect"
"regexp"
"strings"
"testing"
)
// repo.update is a PATCH: a field the body did not mention is left
// alone.
//
// Every field was a plain string or bool, so "absent" and "the zero
// value" were indistinguishable and every call wrote every field. The
// expensive one was mirrorable: false makes the sync endpoint answer
// 404, so editing a description silently stopped the repository
// syncing. jva hit exactly that at 22:37:21Z, one minute after the caps
// gate stopped refusing repository admins — the partial-update footgun
// had been reachable only by site admins until then, and the gate fix
// unmasked it (dc748de1).
//
// Asserted on the type rather than through a live registry because the
// property is about what the DECODER can distinguish: a plain bool
// cannot say "not sent", and no amount of Run-side care recovers that.
func TestRepoUpdateFieldsAreATriState(t *testing.T) {
for _, field := range []string{"Description", "Private", "Homepage", "Topics", "Chat", "Mirrorable"} {
f, ok := fieldOf(repoUpdateIn{}, field)
if !ok {
t.Errorf("repoUpdateIn has no %s — if it was renamed, move this guard with it", field)
continue
}
if !strings.HasPrefix(f, "*") {
t.Errorf("repoUpdateIn.%s is %s, not a pointer: absent and the zero value are the "+
"same value, so a partial update overwrites this field with nothing", field, f)
}
}
// Name is the one field a body must always carry.
if f, _ := fieldOf(repoUpdateIn{}, "Name"); f != "string" {
t.Errorf("repoUpdateIn.Name is %s, want a plain string: it identifies the repository "+
"and is never optional", f)
}
}
// The lengths must still be enforced. ops.Validate's applyRules falls
// through to `default: return nil` for a pointer, so the `max=` tags
// these fields used to carry became decoration the moment they became
// pointers. Moving a field to a tri-state and keeping its tag would
// silently drop its validation, which is exactly the class of mistake
// this ticket is about.
func TestRepoUpdateStillBoundsItsStrings(t *testing.T) {
body := opBody(t, "repo.update", "repo.delete")
for _, want := range []string{
"description is at most 300",
"homepage is at most 300",
"topics are at most 600",
} {
if !strings.Contains(body, want) {
t.Errorf("repo.update no longer checks %q by hand, and a validate tag on a pointer "+
"field does nothing: the limit is unenforced", want)
}
}
}
// A field the body did not name is not written AT ALL.
//
// The first fix for dc748de1 made the fields pointers and then merged
// them over a read of the whole settings row, so an unnamed field was
// written back as it stood. That closed the original hole and opened
// two more, because a read-modify-write has a before-image and a
// before-image can be wrong or can go stale:
//
// - Wrong: the read returned the EFFECTIVE mirrorable (false for any
// private repository) and wrote it back as the stored flag, so a
// repository taken private and restored to public came back
// unclonable. jva found that on dantest-alias (G-115).
// - Stale: two admins saving different panels in the same moment each
// wrote back their own copy of the other's field, so the later save
// reverted the earlier one silently.
//
// Writing only the named fields removes the before-image, so neither
// has anywhere to happen. This guard is stated over the patch type and
// the write path together: the handler must build a RepoEdit and must
// not reconstitute the whole row from a read.
func TestRepoUpdateWritesOnlyWhatItWasAsked(t *testing.T) {
body := opBody(t, "repo.update", "repo.delete")
if !strings.Contains(body, "edit := RepoEdit{") {
t.Error("repo.update does not build a RepoEdit: if it went back to positional " +
"arguments it cannot express \"leave this field alone\"")
}
// The exact shape of the old bug: the merged row being handed to the
// registry.
for _, gone := range []string{
"cur.description, visibilityOf(cur.private)",
"d.Registry.SetChat(in.Name, cur.chat)",
} {
if strings.Contains(body, gone) {
t.Errorf("repo.update still writes the whole settings row (%q): an unnamed "+
"field is being written back from a read", gone)
}
}
// Chat is its own write path and must be just as conditional.
if !strings.Contains(body, "if in.Chat != nil {") {
t.Error("repo.update calls SetChat unconditionally, so a body that never mentioned " +
"chat rewrites it from whatever was read")
}
}
// fieldOf reports a struct field's type as written, for a guard that is
// about the TYPE rather than about any particular value.
func fieldOf(v any, name string) (string, bool) {
t := reflect.TypeOf(v)
f, ok := t.FieldByName(name)
if !ok {
return "", false
}
return f.Type.String(), true
}
func readSource(t *testing.T, name string) string {
t.Helper()
b, err := os.ReadFile(name)
if err != nil {
t.Fatal(err)
}
return string(b)
}
// opBody returns the source of one op's registration, from its Name up
// to the next op's. Matched with a regexp rather than an exact string
// because gofmt realigns `Name:` when a neighbouring field changes
// width — which broke these guards the first time the Doc string grew,
// and a guard that fails on formatting is a guard nobody trusts.
func opBody(t *testing.T, name, next string) string {
t.Helper()
src := readSource(t, "register.go")
at := func(n string) int {
loc := regexp.MustCompile(`Name:\s*"` + regexp.QuoteMeta(n) + `"`).FindStringIndex(src)
if loc == nil {
return -1
}
return loc[0]
}
i := at(name)
if i < 0 {
t.Fatalf("%s is gone from register.go", name)
}
body := src[i:]
if j := at(next); j > i {
body = src[i:j]
}
return body
}
// A private detour must not cost a repository its mirrorable flag.
//
// Deps.Mirrorable answers the EFFECTIVE question — MayMirror, false for
// every private repository whatever the stored flag says — and the
// partial-update merge read it. So taking a repository private and then
// restoring it to public wrote "false because private" back as "false
// because asked": the repository returned reading Public everywhere,
// with /xfer answering 404 and no one able to clone it. Found by jva on
// dantest-alias within an hour of the merge landing (G-115).
//
// The merge must read the STORED flag, which is what RepoMeta now
// carries.
func TestAPrivateDetourKeepsTheMirrorableFlag(t *testing.T) {
src := readSource(t, "register.go")
i := strings.Index(src, "func (d Deps) repoSettings(repo string) repoSettingsNow")
if i < 0 {
t.Fatal("repoSettings is gone")
}
body := src[i:]
if end := strings.Index(body, "\ntype repoUpdateOut"); end > 0 {
body = body[:end]
}
if strings.Contains(body, "d.Mirrorable(repo)") {
t.Error("repoSettings reads the EFFECTIVE mirrorable and writes it back: " +
"a repository taken private and restored to public comes out unclonable")
}
if !strings.Contains(body, "meta.Mirrorable") {
t.Error("repoSettings does not read the stored mirrorable flag from RepoMeta")
}
}