From 94b49a296fde1144809a7f5e68e615b461a31414 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 16:45:49 +0200 Subject: [PATCH] Test that the gitea module makes the organisation's repositories from its config, protected before any team reaches them, and that no tool creates in the organisation (issue 1124, ADR 0428) --- modules/gitea/cmd/repositories/fake_test.go | 299 ++++++++++++ .../gitea/cmd/repositories/manifest_test.go | 77 ++++ .../cmd/repositories/repositories_test.go | 424 ++++++++++++++++++ modules/gitea/test/create-repo.test.ts | 117 ----- modules/gitea/test/no-org-create.test.ts | 56 +++ 5 files changed, 856 insertions(+), 117 deletions(-) create mode 100644 modules/gitea/cmd/repositories/fake_test.go create mode 100644 modules/gitea/cmd/repositories/manifest_test.go create mode 100644 modules/gitea/cmd/repositories/repositories_test.go delete mode 100644 modules/gitea/test/create-repo.test.ts create mode 100644 modules/gitea/test/no-org-create.test.ts diff --git a/modules/gitea/cmd/repositories/fake_test.go b/modules/gitea/cmd/repositories/fake_test.go new file mode 100644 index 00000000..43f50b11 --- /dev/null +++ b/modules/gitea/cmd/repositories/fake_test.go @@ -0,0 +1,299 @@ +package main + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "regexp" + "sort" + "strings" + "sync" + "testing" + + "gitea/internal/forgehttp" +) + +// The fake forge of issue 1124 / ADR 0428: it keeps repositories by owner/name with their branch protection rules, +// their branches and their files, records every call in order, and refuses a call a test names. Like the forge, it +// refuses a commit onto a branch whose rule does not let the caller push, and it answers only the setup login. + +const ( + admin = "mesh-admin" + tools = "mesh-tools" + org = "novox" +) + +type fRepo struct { + Private bool + Description string + Rules map[string]map[string]any // by rule_name + Branches map[string]bool + Files map[string]string + Commits int +} + +type fakeForge struct { + mu sync.Mutex + repos map[string]*fRepo // "owner/name" + calls []string // "METHOD /path" + // refuse answers a call with a status and a message when it returns a status above zero. + refuse func(method, path string) (int, string) +} + +func newFake() *fakeForge { return &fakeForge{repos: map[string]*fRepo{}} } + +func (f *fakeForge) add(full string, rules map[string]map[string]any, branches ...string) *fRepo { + r := &fRepo{Rules: map[string]map[string]any{}, Branches: map[string]bool{}, Files: map[string]string{}} + for k, v := range rules { + r.Rules[k] = v + } + for _, b := range branches { + r.Branches[b] = true + } + f.repos[full] = r + return r +} + +// writes are the calls that change something, in order. +func (f *fakeForge) writes() []string { + f.mu.Lock() + defer f.mu.Unlock() + var out []string + for _, c := range f.calls { + if !strings.HasPrefix(c, "GET ") { + out = append(out, c) + } + } + return out +} + +func (f *fakeForge) all() []string { + f.mu.Lock() + defer f.mu.Unlock() + return append([]string(nil), f.calls...) +} + +var ( + repoPath = regexp.MustCompile(`^/api/v1/repos/([^/]+)/([^/]+)$`) + rulesPath = regexp.MustCompile(`^/api/v1/repos/([^/]+)/([^/]+)/branch_protections(?:/([^/]+))?$`) + branchPath = regexp.MustCompile(`^/api/v1/repos/([^/]+)/([^/]+)/branches/([^/]+)$`) + filesPath = regexp.MustCompile(`^/api/v1/repos/([^/]+)/([^/]+)/contents$`) + movePath = regexp.MustCompile(`^/api/v1/repos/([^/]+)/([^/]+)/transfer$`) +) + +func strs(v any) []string { + var out []string + if l, ok := v.([]any); ok { + for _, s := range l { + out = append(out, fmt.Sprint(s)) + } + } + if l, ok := v.([]string); ok { + out = append(out, l...) + } + return out +} + +// lets says whether a rule lets the login push. +func lets(rule map[string]any, login string) bool { + if rule == nil { + return true + } + if rule["enable_push"] != true { + return false + } + if rule["enable_push_whitelist"] != true { + return true + } + for _, u := range strs(rule["push_whitelist_usernames"]) { + if u == login { + return true + } + } + return false +} + +func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + if u, p, ok := r.BasicAuth(); !ok || u != admin || p != "pw" { + http.Error(w, "no", http.StatusUnauthorized) + return + } + reply := func(status int, v any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + if v != nil { + _ = json.NewEncoder(w).Encode(v) + } + } + p := r.URL.Path + f.calls = append(f.calls, r.Method+" "+p) + if f.refuse != nil { + if status, said := f.refuse(r.Method, p); status > 0 { + reply(status, map[string]string{"message": said}) + return + } + } + var in map[string]any + _ = json.NewDecoder(r.Body).Decode(&in) + switch { + case p == "/api/v1/version": + reply(200, map[string]string{"version": "1.27"}) + case p == "/api/v1/user/repos" && r.Method == http.MethodPost: + name := fmt.Sprint(in["name"]) + if f.repos[admin+"/"+name] != nil { + reply(409, map[string]string{"message": "The repository with the same name already exists."}) + return + } + if in["auto_init"] == true { + reply(400, map[string]string{"message": "this fake takes only an empty repository"}) + return + } + repo := f.add(admin+"/"+name, nil) + repo.Private, _ = in["private"].(bool) + repo.Description, _ = in["description"].(string) + reply(201, map[string]any{"full_name": admin + "/" + name, "empty": true}) + case repoPath.MatchString(p) && r.Method == http.MethodGet: + m := repoPath.FindStringSubmatch(p) + repo := f.repos[m[1]+"/"+m[2]] + if repo == nil { + reply(404, map[string]string{"message": "not found"}) + return + } + reply(200, map[string]any{"full_name": m[1] + "/" + m[2], "empty": len(repo.Branches) == 0}) + case rulesPath.MatchString(p): + m := rulesPath.FindStringSubmatch(p) + repo := f.repos[m[1]+"/"+m[2]] + if repo == nil { + reply(404, nil) + return + } + switch { + case r.Method == http.MethodGet && m[3] == "": + names := make([]string, 0, len(repo.Rules)) + for n := range repo.Rules { + names = append(names, n) + } + sort.Strings(names) + out := []map[string]any{} + for _, n := range names { + rule := map[string]any{"rule_name": n} + for k, v := range repo.Rules[n] { + rule[k] = v + } + out = append(out, rule) + } + reply(200, out) + case r.Method == http.MethodPost && m[3] == "": + name := fmt.Sprint(in["rule_name"]) + if repo.Rules[name] != nil { + reply(403, map[string]string{"message": "branch protection already exist"}) + return + } + delete(in, "rule_name") + repo.Rules[name] = in + reply(201, in) + case r.Method == http.MethodPatch && m[3] != "": + rule := repo.Rules[m[3]] + if rule == nil { + reply(404, nil) + return + } + for k, v := range in { + rule[k] = v + } + reply(200, rule) + default: + reply(405, nil) + } + case branchPath.MatchString(p) && r.Method == http.MethodGet: + m := branchPath.FindStringSubmatch(p) + repo := f.repos[m[1]+"/"+m[2]] + if repo == nil || !repo.Branches[m[3]] { + reply(404, nil) + return + } + reply(200, map[string]any{"name": m[3]}) + case filesPath.MatchString(p) && r.Method == http.MethodPost: + m := filesPath.FindStringSubmatch(p) + repo := f.repos[m[1]+"/"+m[2]] + branch := fmt.Sprint(in["branch"]) + if repo == nil { + reply(404, nil) + return + } + if !lets(repo.Rules[branch], admin) { + reply(403, map[string]string{"message": "branch " + branch + " is protected"}) + return + } + files, _ := in["files"].([]any) + for _, x := range files { + op, _ := x.(map[string]any) + raw, _ := base64.StdEncoding.DecodeString(fmt.Sprint(op["content"])) + repo.Files[fmt.Sprint(op["path"])] = string(raw) + } + repo.Branches[branch] = true + repo.Commits++ + reply(201, map[string]any{"commit": map[string]any{"sha": fmt.Sprintf("%040d", repo.Commits)}}) + case movePath.MatchString(p) && r.Method == http.MethodPost: + m := movePath.FindStringSubmatch(p) + from := m[1] + "/" + m[2] + repo := f.repos[from] + to := fmt.Sprint(in["new_owner"]) + "/" + m[2] + if repo == nil || f.repos[to] != nil { + reply(422, map[string]string{"message": "cannot transfer"}) + return + } + delete(f.repos, from) + f.repos[to] = repo + reply(202, map[string]any{"full_name": to}) + default: + reply(404, map[string]string{"message": "not found"}) + } +} + +func syncer(t *testing.T, f *fakeForge) (*Syncer, *[]string) { + t.Helper() + srv := httptest.NewServer(f) + t.Cleanup(srv.Close) + pw := filepath.Join(t.TempDir(), "pw") + if err := os.WriteFile(pw, []byte("pw\n"), 0o600); err != nil { + t.Fatal(err) + } + var said []string + return &Syncer{Forge: forgehttp.Client{URL: srv.URL, User: admin, PasswordFile: pw}, Org: org, Admin: admin, Tools: tools, + Log: func(format string, a ...any) { said = append(said, fmt.Sprintf(format, a...)) }}, &said +} + +func run(t *testing.T, s *Syncer, v Value) Outcome { + t.Helper() + o, err := s.Sync(context.Background(), v) + if err != nil { + t.Fatal(err) + } + return o +} + +func result(o Outcome, name string) Result { + for _, r := range o.Repositories { + if r.Name == name { + return r + } + } + return Result{} +} + +func has(list []string, want string) bool { + for _, s := range list { + if s == want { + return true + } + } + return false +} diff --git a/modules/gitea/cmd/repositories/manifest_test.go b/modules/gitea/cmd/repositories/manifest_test.go new file mode 100644 index 00000000..bca2d014 --- /dev/null +++ b/modules/gitea/cmd/repositories/manifest_test.go @@ -0,0 +1,77 @@ +package main + +import ( + "encoding/json" + "os" + "strings" + "testing" +) + +// 1124/AC1: the manifest declares `repositories` under config as a trusted object value with an empty default, as +// it declares agent-read-teams (ADR 0365 §3, ADR 0348), renders it into a file of its own, and runs this program +// at the module's start and whenever that file changes. +func TestManifestDeclaresTheValueAndItsTrigger(t *testing.T) { + raw, err := os.ReadFile("../../module.json") + if err != nil { + t.Fatal(err) + } + var m struct { + Config map[string]map[string]any `json:"config"` + Resources []map[string]any `json:"resources"` + Build struct { + Artifacts []map[string]any `json:"artifacts"` + } `json:"build"` + } + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatal(err) + } + v := m.Config[ConfigKey] + if v == nil || v["type"] != "object" || v["trusted"] != true { + t.Fatalf("config %s = %v, want a trusted object", ConfigKey, v) + } + if d, ok := v["default"].(map[string]any); !ok || len(d) != 0 { + t.Fatalf("its default is %v, want {}", v["default"]) + } + var file, process map[string]any + for _, r := range m.Resources { + for _, k := range strs(r["takes-config"]) { + if k == ConfigKey { + file = r + } + } + } + if file == nil || file["type"] != "file" { + t.Fatalf("no file takes the value %s", ConfigKey) + } + for _, r := range m.Resources { + if r["type"] == "process" && has(strs(r["restart-on"]), file["id"].(string)) { + process = r + } + } + if process == nil { + t.Fatalf("no process runs again when %s changes", file["id"]) + } + if strings.Join(strs(process["run"]), " ") != "./repositories sync" || process["artifact"] != "repositories" { + t.Errorf("the process runs %v from %v", process["run"], process["artifact"]) + } + env, _ := process["env"].(map[string]any) + for _, k := range []string{"MESH_GITEA_URL", "MESH_GITEA_ADMIN_USER", "MESH_GITEA_ADMIN_PASSWORD_FILE", "MESH_REPOSITORIES_FILE", "MESH_REPOSITORIES_OUTCOME"} { + if env[k] == nil { + t.Errorf("the process is not given %s", k) + } + } + found := false + for _, a := range m.Build.Artifacts { + if a["name"] == "repositories" && a["from"] == "cmd/repositories" && a["binary"] == "repositories" { + found = true + } + } + if !found { + t.Error("no bundle builds cmd/repositories") + } + // Like agent-read-teams: the same kind of value, the same way of applying it. + teams := m.Config["agent-read-teams"] + if teams["trusted"] != v["trusted"] || teams["type"] != v["type"] { + t.Errorf("repositories is declared unlike agent-read-teams: %v vs %v", v, teams) + } +} diff --git a/modules/gitea/cmd/repositories/repositories_test.go b/modules/gitea/cmd/repositories/repositories_test.go new file mode 100644 index 00000000..773b0873 --- /dev/null +++ b/modules/gitea/cmd/repositories/repositories_test.go @@ -0,0 +1,424 @@ +package main + +import ( + "encoding/json" + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// The tests of issue 1124 / ADR 0428: the organisation's repositories are the gitea module's config value +// `repositories`, made and protected by the setup login, never by a tool. + +const mainRule = "main" + +// 1124/AC2: a listed repository that does not exist is made under the setup account, given main's rule letting that +// login alone push, given its first commit, narrowed to no push and the tools' merge requiring mesh/repo-check (and +// mesh/merge-gate, 1124/AC5), and only then moved into the organisation. Nothing reaches the organisation before the +// narrowed rule, and nothing else is pushed to main. +func TestNewRepositoryIsMadeInOrder(t *testing.T) { + f := newFake() + s, _ := syncer(t, f) + o := run(t, s, Value{"mesh-delivery": {Private: false, Description: "the mesh's delivery"}}) + + want := []string{ + "POST /api/v1/user/repos", + "POST /api/v1/repos/mesh-admin/mesh-delivery/branch_protections", + "POST /api/v1/repos/mesh-admin/mesh-delivery/contents", + "PATCH /api/v1/repos/mesh-admin/mesh-delivery/branch_protections/main", + "POST /api/v1/repos/mesh-admin/mesh-delivery/transfer", + } + if got := f.writes(); strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Fatalf("writes, in order:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n")) + } + // Nothing names the organisation's copy before the narrowed rule but the read that found it missing. + narrowed := false + for _, c := range f.all() { + if strings.HasPrefix(c, "PATCH ") { + narrowed = true + } + if strings.Contains(c, "/repos/novox/mesh-delivery") && !narrowed && c != "GET /api/v1/repos/novox/mesh-delivery" { + t.Errorf("%s before the narrowed rule", c) + } + } + repo := f.repos["novox/mesh-delivery"] + if repo == nil || f.repos["mesh-admin/mesh-delivery"] != nil { + t.Fatalf("not moved into the organisation: %v", f.repos) + } + if repo.Private || repo.Description != "the mesh's delivery" { + t.Errorf("made as %+v, want public with its description", repo) + } + if repo.Commits != 1 || len(repo.Files) != 2 || repo.Files["README.md"] == "" || repo.Files["merge-check.sh"] == "" { + t.Errorf("first commit: %d commits, files %v", repo.Commits, keysOf(repo.Files)) + } + if !strings.Contains(repo.Files["README.md"], "the mesh's delivery") { + t.Errorf("the README does not name its purpose: %q", repo.Files["README.md"]) + } + rule := repo.Rules[mainRule] + if rule["enable_push"] != false || rule["enable_push_whitelist"] == true || len(strs(rule["push_whitelist_usernames"])) != 0 { + t.Errorf("main still takes a push: %v", rule) + } + if rule["enable_merge_whitelist"] != true || strings.Join(strs(rule["merge_whitelist_usernames"]), ",") != tools { + t.Errorf("main's merge whitelist is not the tools' account alone: %v", rule) + } + checks := strs(rule["status_check_contexts"]) + if rule["enable_status_check"] != true || !has(checks, "mesh/repo-check") || !has(checks, "mesh/merge-gate") { + t.Errorf("main does not require mesh/repo-check and mesh/merge-gate (1124/AC5): %v", rule) + } + if r := result(o, "mesh-delivery"); r.Did != DidMade || r.Step != "" { + t.Errorf("outcome %+v, want made", r) + } + + // A second pass changes nothing. + before := len(f.writes()) + o = run(t, s, Value{"mesh-delivery": {Private: false, Description: "the mesh's delivery"}}) + if got := f.writes(); len(got) != before { + t.Errorf("a second pass wrote %v", got[before:]) + } + if r := result(o, "mesh-delivery"); r.Did != DidUnchanged { + t.Errorf("second outcome %+v, want unchanged", r) + } +} + +// 1124/AC2: the setup login's rule lets that login alone push, and it is what the first commit lands through. +func TestAdminRuleLetsTheSetupLoginAlonePush(t *testing.T) { + f := newFake() + var seen map[string]any + f.refuse = func(method, path string) (int, string) { + if method == "POST" && strings.HasSuffix(path, "/contents") { + seen = map[string]any{} + for k, v := range f.repos["mesh-admin/x"].Rules[mainRule] { + seen[k] = v + } + } + return 0, "" + } + s, _ := syncer(t, f) + run(t, s, Value{"x": {Private: true}}) + if seen["enable_push"] != true || seen["enable_push_whitelist"] != true || strings.Join(strs(seen["push_whitelist_usernames"]), ",") != admin { + t.Fatalf("the rule at the first commit was %v, want a push by %s alone", seen, admin) + } + if !f.repos["novox/x"].Private { + t.Error("made public, want private") + } +} + +// 1124/AC3: a listed repository that exists in the organisation is not made again, only its main rule put right; +// a rule still letting the setup login push is narrowed, its statuses kept (1124/AC5). +func TestExistingOrganisationRepositoryIsOnlyPutRight(t *testing.T) { + f := newFake() + hq := f.add("novox/hq", map[string]map[string]any{mainRule: { + "enable_push": true, "enable_push_whitelist": true, "push_whitelist_usernames": []any{admin}, + "enable_status_check": true, "status_check_contexts": []any{"other/x", "mesh/merge-gate"}, + }}, "main") + bare := f.add("novox/bare", nil, "main") + s, _ := syncer(t, f) + o := run(t, s, Value{"hq": {}, "bare": {}}) + + for _, c := range f.writes() { + if strings.HasSuffix(c, "/user/repos") || strings.HasSuffix(c, "/contents") || strings.HasSuffix(c, "/transfer") { + t.Errorf("an existing repository got %s", c) + } + } + rule := hq.Rules[mainRule] + if rule["enable_push"] != false || strings.Join(strs(rule["merge_whitelist_usernames"]), ",") != tools { + t.Errorf("hq's rule not narrowed: %v", rule) + } + checks := strs(rule["status_check_contexts"]) + for _, c := range []string{"other/x", "mesh/merge-gate", "mesh/repo-check"} { + if !has(checks, c) { + t.Errorf("hq's rule lacks %s: %v", c, checks) + } + } + made := bare.Rules[mainRule] + if made == nil || made["enable_push"] != false || !has(strs(made["status_check_contexts"]), "mesh/repo-check") || + !has(strs(made["status_check_contexts"]), "mesh/merge-gate") { + t.Errorf("bare's rule: %v", made) + } + if hq.Commits != 0 || bare.Commits != 0 { + t.Error("something was pushed to an existing main") + } + if r := result(o, "hq"); r.Did != DidPutRight { + t.Errorf("hq outcome %+v", r) + } +} + +// 1124/AC3: a repository found still under the setup account is finished: with main already written it is not +// written again; without a rule it gets the steps it lacks. +func TestRepositoryUnderTheSetupAccountIsFinished(t *testing.T) { + f := newFake() + f.add("mesh-admin/half", map[string]map[string]any{mainRule: { + "enable_push": true, "enable_push_whitelist": true, "push_whitelist_usernames": []any{admin}, + }}, "main") + f.add("mesh-admin/empty", nil) + s, _ := syncer(t, f) + o := run(t, s, Value{"half": {}, "empty": {}}) + + got := strings.Join(f.writes(), "\n") + want := strings.Join([]string{ + "POST /api/v1/repos/mesh-admin/empty/branch_protections", + "POST /api/v1/repos/mesh-admin/empty/contents", + "PATCH /api/v1/repos/mesh-admin/empty/branch_protections/main", + "POST /api/v1/repos/mesh-admin/empty/transfer", + "PATCH /api/v1/repos/mesh-admin/half/branch_protections/main", + "POST /api/v1/repos/mesh-admin/half/transfer", + }, "\n") + if got != want { + t.Fatalf("writes:\n%s\nwant:\n%s", got, want) + } + for _, n := range []string{"half", "empty"} { + repo := f.repos["novox/"+n] + if repo == nil || repo.Rules[mainRule]["enable_push"] != false { + t.Errorf("%s not finished: %+v", n, repo) + } + if r := result(o, n); r.Did != DidMade { + t.Errorf("%s outcome %+v", n, r) + } + } + if f.repos["novox/half"].Commits != 0 { + t.Error("main of half was written again") + } +} + +// 1124/AC5: putting a rule right adds a missing one of mesh/repo-check and mesh/merge-gate and keeps every status +// already there; no pass drops one. +func TestStatusesAreAddedAndNeverDropped(t *testing.T) { + f := newFake() + only := f.add("novox/only", map[string]map[string]any{mainRule: { + "enable_push": false, "enable_merge_whitelist": true, "merge_whitelist_usernames": []any{tools}, + "enable_status_check": true, "status_check_contexts": []any{"mesh/repo-check"}, + }}, "main") + more := f.add("novox/more", map[string]map[string]any{mainRule: { + "enable_push": false, "enable_merge_whitelist": true, "merge_whitelist_usernames": []any{tools}, + "enable_status_check": true, "status_check_contexts": []any{"mesh/merge-gate", "mesh/repo-check", "mesh/delivery"}, + }}, "main") + s, _ := syncer(t, f) + o := run(t, s, Value{"only": {}, "more": {}}) + if c := strs(only.Rules[mainRule]["status_check_contexts"]); !has(c, "mesh/merge-gate") || !has(c, "mesh/repo-check") { + t.Errorf("only: %v", c) + } + if c := strs(more.Rules[mainRule]["status_check_contexts"]); len(c) != 3 || !has(c, "mesh/delivery") { + t.Errorf("more lost a status: %v", c) + } + for _, w := range f.writes() { + if strings.Contains(w, "/more/") { + t.Errorf("a right rule was written: %s", w) + } + } + if result(o, "more").Did != DidUnchanged || result(o, "only").Did != DidPutRight { + t.Errorf("outcome %+v", o) + } +} + +// 1124/AC6: a refused step is said with the repository, the step and the forge's answer, nothing further of that +// repository is sent, and the next listed repository is still made; the health check then is unhealthy naming them. +func TestRefusedStepStopsThatRepositoryOnly(t *testing.T) { + steps := map[string]func(method, path string) bool{ + StepCreate: func(m, p string) bool { return m == "POST" && p == "/api/v1/user/repos" }, + StepAdminRule: func(m, p string) bool { return m == "POST" && strings.HasSuffix(p, "/a/branch_protections") }, + StepFirstCommit: func(m, p string) bool { return m == "POST" && strings.HasSuffix(p, "/a/contents") }, + StepNarrow: func(m, p string) bool { return m == "PATCH" && strings.Contains(p, "/a/branch_protections/") }, + StepMove: func(m, p string) bool { return m == "POST" && strings.HasSuffix(p, "/a/transfer") }, + } + for step, match := range steps { + t.Run(step, func(t *testing.T) { + f := newFake() + refused := false + f.refuse = func(method, path string) (int, string) { + if refused { + return 0, "" + } + // The create of a is refused, not b's. + if step == StepCreate && match(method, path) && !strings.Contains(strings.Join(f.calls, "\n"), "/b") { + refused = true + return 403, "the forge says no to " + step + } + if step != StepCreate && match(method, path) { + refused = true + return 403, "the forge says no to " + step + } + return 0, "" + } + s, _ := syncer(t, f) + o := run(t, s, Value{"a": {}, "b": {}}) + r := result(o, "a") + if r.Did != DidRefused || r.Step != step || !strings.Contains(r.Said, "the forge says no to "+step) { + t.Fatalf("a: %+v, want refused at %s with the forge's answer", r, step) + } + // Nothing of a after the refused call. + calls := f.all() + at := -1 + for i, c := range calls { + m := strings.SplitN(c, " ", 2) + if match(m[0], m[1]) { + at = i + break + } + } + for _, c := range calls[at+1:] { + if strings.Contains(c, "/a/") || strings.HasSuffix(c, "/a") { + t.Errorf("after the refused %s: %s", step, c) + } + } + if f.repos["novox/b"] == nil || result(o, "b").Did != DidMade { + t.Errorf("b was not made after a's refusal: %+v", result(o, "b")) + } + h := Judge(&o, nil, time.Now()) + if h.Healthy || !strings.Contains(h.Why, "novox/a") || !strings.Contains(h.Why, step) || + !strings.Contains(h.Why, "the forge says no to "+step) { + t.Errorf("health %+v, want unhealthy naming novox/a, %s and the answer", h, step) + } + }) + } +} + +// 1124/AC7: a repository taken out of the value gets no call at all. +func TestRepositoryOutOfTheValueIsNotTouched(t *testing.T) { + f := newFake() + f.add("novox/old", map[string]map[string]any{mainRule: {"enable_push": true}}, "main") + f.add("mesh-admin/older", nil) + s, _ := syncer(t, f) + run(t, s, Value{}) + run(t, s, Value{"new": {}}) + for _, c := range f.all() { + if strings.Contains(c, "old") { + t.Errorf("a repository out of the value was asked: %s", c) + } + } + if f.repos["novox/old"].Rules[mainRule]["enable_push"] != true || f.repos["mesh-admin/older"] == nil { + t.Error("a repository out of the value was changed") + } +} + +// 1124/AC6: the outcome the pass leaves is what the health check judges: healthy when every repository was made or +// put right, unhealthy naming each refusal, and unhealthy when no pass has left one. +func TestJudge(t *testing.T) { + now := time.Now() + ok := Outcome{At: now, Repositories: []Result{{Name: "hq", Did: DidUnchanged}, {Name: "x", Did: DidMade}}} + if h := Judge(&ok, nil, now); !h.Healthy || h.State != "healthy" { + t.Errorf("%+v", h) + } + if h := Judge(nil, os.ErrNotExist, now); h.Healthy || !strings.Contains(h.Why, "no pass") { + t.Errorf("no outcome: %+v", h) + } + if h := Judge(nil, errors.New("broken"), now); h.Healthy { + t.Errorf("unreadable outcome: %+v", h) + } + if h := Judge(&Outcome{At: now, Failed: "the forge is not reachable"}, nil, now); h.Healthy || + !strings.Contains(h.Why, "the forge is not reachable") { + t.Errorf("a pass that did not run: %+v", h) + } +} + +// 1124/AC6: the outcome is written where the health check reads it, and read back whole. +func TestOutcomeRoundTrip(t *testing.T) { + p := filepath.Join(t.TempDir(), "outcome.json") + o := Outcome{At: time.Now().UTC().Truncate(time.Second), Repositories: []Result{{Name: "a", Did: DidRefused, Step: StepMove, Said: "no"}}} + if err := WriteOutcome(p, o); err != nil { + t.Fatal(err) + } + got, err := ReadOutcome(p) + if err != nil || got.Repositories[0] != o.Repositories[0] || !got.At.Equal(o.At) { + t.Fatalf("%+v %v", got, err) + } +} + +// 1124/AC1: the value is read from the file the mesh renders; a missing key is the empty value; an unknown field +// or a name that is no repository name is refused. +func TestReadValue(t *testing.T) { + p := filepath.Join(t.TempDir(), "r.json") + _ = os.WriteFile(p, []byte(`{"repositories":{"mesh-delivery":{"private":true,"description":"d"}}}`), 0o600) + v, err := ReadValue(p) + if err != nil || !v["mesh-delivery"].Private || v["mesh-delivery"].Description != "d" { + t.Fatalf("%v %v", v, err) + } + _ = os.WriteFile(p, []byte(`{}`), 0o600) + if v, err := ReadValue(p); err != nil || len(v) != 0 { + t.Fatalf("%v %v", v, err) + } + _ = os.WriteFile(p, []byte(`{"repositories":{"a":{"owner":"novox"}}}`), 0o600) + if _, err := ReadValue(p); err == nil { + t.Fatal("an unknown field was accepted") + } + for _, bad := range []string{"..", "a/b", "", "a b", "-x"} { + raw, _ := json.Marshal(map[string]any{"repositories": map[string]any{bad: map[string]any{}}}) + _ = os.WriteFile(p, raw, 0o600) + if _, err := ReadValue(p); err == nil { + t.Errorf("%q was accepted as a repository name", bad) + } + } +} + +// 1124/AC4: the first commit's merge-check.sh declares its toolchain in its first lines, runs the tests of a Go +// module in the tree, and on a tree with none says so and passes. +func TestFirstCommitMergeCheck(t *testing.T) { + files := FirstCommit("mesh-delivery", Repo{Description: "the mesh's delivery"}) + script := files["merge-check.sh"] + lines := strings.SplitN(script, "\n", 4) + if lines[0] != "#!/bin/sh" || lines[1] != "# mesh-check-toolchain: go" { + t.Fatalf("first lines %q", lines[:2]) + } + if !strings.Contains(files["README.md"], "mesh-delivery") || !strings.Contains(files["README.md"], "the mesh's delivery") { + t.Errorf("README %q", files["README.md"]) + } + goBin, err := exec.LookPath("go") + if err != nil { + t.Skip("no go on PATH to run the script with") + } + + tree := t.TempDir() + mod := filepath.Join(tree, "sub") + _ = os.MkdirAll(mod, 0o755) + _ = os.WriteFile(filepath.Join(tree, "merge-check.sh"), []byte(script), 0o755) + _ = os.WriteFile(filepath.Join(mod, "go.mod"), []byte("module tiny\n\ngo 1.21\n"), 0o644) + _ = os.WriteFile(filepath.Join(mod, "tiny_test.go"), []byte(`package tiny + +import ( + "os" + "testing" +) + +func TestRan(t *testing.T) { _ = os.WriteFile("ran", []byte("yes"), 0o644) } +`), 0o644) + cmd := exec.Command("sh", "merge-check.sh") + cmd.Dir = tree + cmd.Env = append(os.Environ(), "GOFLAGS=", "GOTOOLCHAIN=local", "PATH="+filepath.Dir(goBin)+":"+os.Getenv("PATH")) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("on a tree with a Go module: %v\n%s", err, out) + } + if _, err := os.Stat(filepath.Join(mod, "ran")); err != nil || !strings.Contains(string(out), "integration") { + t.Fatalf("its tests did not run, or not with -tags integration:\n%s", out) + } + + // A failing test fails the check. + _ = os.WriteFile(filepath.Join(mod, "fail_test.go"), []byte("package tiny\n\nimport \"testing\"\n\nfunc TestFails(t *testing.T) { t.Fatal(\"no\") }\n"), 0o644) + if out, err := cmd2(tree, cmd.Env).CombinedOutput(); err == nil { + t.Fatalf("a failing test passed the check:\n%s", out) + } + + empty := t.TempDir() + _ = os.WriteFile(filepath.Join(empty, "merge-check.sh"), []byte(script), 0o755) + out, err = cmd2(empty, cmd.Env).CombinedOutput() + if err != nil || !strings.Contains(string(out), "no Go module") { + t.Fatalf("on an empty tree: %v\n%s", err, out) + } +} + +func cmd2(dir string, env []string) *exec.Cmd { + c := exec.Command("sh", "merge-check.sh") + c.Dir, c.Env = dir, env + return c +} + +func keysOf(m map[string]string) []string { + out := []string{} + for k := range m { + out = append(out, k) + } + return out +} diff --git a/modules/gitea/test/create-repo.test.ts b/modules/gitea/test/create-repo.test.ts deleted file mode 100644 index f7a464c5..00000000 --- a/modules/gitea/test/create-repo.test.ts +++ /dev/null @@ -1,117 +0,0 @@ -// A repository is created where it is asked for (novox/hq issue 1124): gitea_create_repo created under the tools' -// own user alone, so a repository of the novox organisation could not be made through it. With `owner` it is created -// in that organisation (POST /orgs/{org}/repos), the forge's refusal said with its status and its reason; without -// it, under the tools' own user as before. - -import { test, after, beforeEach } from "node:test"; -import assert from "node:assert/strict"; -import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; - -import { GiteaClient } from "../dist/client.js"; -import { ConfiguredToken } from "../dist/token.js"; -import { getGiteaTools } from "../dist/tools/index.js"; - -interface Sent { method: string; path: string; body: any } - -const state = { - sent: [] as Sent[], - // When set, what the forge answers a create in an organisation: its status and its message. - refuse: null as { status: number; message: string } | null, -}; - -function json(res: ServerResponse, status: number, body: unknown) { - res.statusCode = status; - res.setHeader("Content-Type", "application/json"); - res.end(JSON.stringify(body)); -} - -const server = createServer(async (req: IncomingMessage, res: ServerResponse) => { - const chunks: Buffer[] = []; - for await (const c of req) chunks.push(c as Buffer); - const raw = Buffer.concat(chunks).toString(); - const path = new URL(req.url ?? "", "http://x").pathname.replace(/^\/api\/v1/, ""); - const method = req.method ?? "GET"; - state.sent.push({ method, path, body: raw ? JSON.parse(raw) : undefined }); - const created = (owner: string) => { - const body = raw ? JSON.parse(raw) : {}; - return json(res, 201, { full_name: `${owner}/${body.name}`, name: body.name, owner: { login: owner }, - private: body.private, description: body.description, html_url: `http://forge/${owner}/${body.name}`, - clone_url: `http://forge/${owner}/${body.name}.git`, default_branch: "main" }); - }; - if (method === "POST" && path === "/user/repos") return created("agent"); - const org = /^\/orgs\/([^/]+)\/repos$/.exec(path); - if (method === "POST" && org) { - if (state.refuse) return json(res, state.refuse.status, { message: state.refuse.message, url: "http://forge/api/swagger" }); - return created(decodeURIComponent(org[1])); - } - return json(res, 404, { message: "not found" }); -}); -await new Promise((r) => server.listen(0, "127.0.0.1", () => r())); -after(() => new Promise((r) => server.close(() => r()))); -const { port } = server.address() as { port: number }; -const client = new GiteaClient(`http://127.0.0.1:${port}`, new ConfiguredToken("t")); -const tool = (name: string) => getGiteaTools(client).find((t) => t.name === name)!; -const body = { name: "mesh-delivery", description: "the delivery's own repository", private: true, auto_init: true }; - -beforeEach(() => { - state.sent = []; - state.refuse = null; -}); - -test("with an organisation the repository is created there, with the same body, and answered (1124/AC1)", async () => { - // Fails before the fix: createRepo takes no owner and posts to /user/repos, so the repository is the tools' user's. - const repo = await client.createRepo({ ...body }, "novox"); - assert.deepEqual(state.sent, [{ method: "POST", path: "/orgs/novox/repos", body }]); - assert.equal(repo.full_name, "novox/mesh-delivery"); - assert.equal(repo.owner, "novox"); - - // The tool passes its owner through; before the fix it has no owner argument and posts to /user/repos. - state.sent = []; - const answered = await tool("gitea_create_repo").run({ owner: "novox", ...body }) as { repo: { full_name: string } }; - assert.deepEqual(state.sent, [{ method: "POST", path: "/orgs/novox/repos", body }]); - assert.equal(answered.repo.full_name, "novox/mesh-delivery"); - assert.match(tool("gitea_create_repo").description, /organisation/, "the description says where it creates"); -}); - -test("without an owner the repository is created under the tools' own user, as before (1124/AC2)", async () => { - // Holds before the fix: it is the behaviour the fix must leave unchanged. - const repo = await client.createRepo({ ...body }); - assert.deepEqual(state.sent, [{ method: "POST", path: "/user/repos", body }]); - assert.equal(repo.full_name, "agent/mesh-delivery"); - - state.sent = []; - await tool("gitea_create_repo").run({ name: "mesh-delivery" }); - assert.deepEqual(state.sent, [{ method: "POST", path: "/user/repos", body: { name: "mesh-delivery", private: true, auto_init: true } }]); -}); - -test("the forge's refusal to create in an organisation is said with its status, its reason and the organisation (1124/AC3)", async () => { - // Fails before the fix: the create goes to /user/repos, which the fake forge answers 201, so nothing is refused. - for (const [status, message] of [ - [403, "user should be an owner or a member of the organisation"], - [404, "GetOrgByName: org does not exist [name: novox]"], - [409, "The repository with the same name already exists."], - ] as const) { - state.refuse = { status, message }; - for (const run of [ - () => client.createRepo({ ...body }, "novox"), - () => tool("gitea_create_repo").run({ owner: "novox", ...body }), - ]) { - await assert.rejects(run(), (e: Error) => { - assert.match(e.message, new RegExp(`\\b${status}\\b`), `the status ${status} is said`); - assert.ok(e.message.includes(message), `the forge's reason is said: ${e.message}`); - assert.match(e.message, /organisation novox/, "the organisation is named"); - return true; - }); - } - } -}); - -test("an owner that is not a name is refused before anything is sent (1124/AC4)", async () => { - // Holds before the fix too: namesChecked (tools/index.ts) checks args.owner for every tool. It stays so the guard - // keeps covering this tool's new argument. - for (const owner of ["..", "novox/../agent", "", "no vox", "%2e%2e", "-x"]) { - await assert.rejects(tool("gitea_create_repo").run({ owner, name: "mesh-delivery" }), /not a repository owner/, - JSON.stringify(owner)); - } - assert.deepEqual(state.sent, [], "nothing reached the forge"); -}); diff --git a/modules/gitea/test/no-org-create.test.ts b/modules/gitea/test/no-org-create.test.ts new file mode 100644 index 00000000..c5d9f54a --- /dev/null +++ b/modules/gitea/test/no-org-create.test.ts @@ -0,0 +1,56 @@ +// No tool and no agent creates a repository in an organisation (novox/hq issue 1124, ADR 0428 §8): the forge makes +// whoever creates an organisation's repository its administrator, and an administrator can take its protection off. +// So gitea_create_repo creates only under the tools' own user, with no owner argument; the tools' team keeps +// can_create_org_repo false; and the tools' token has no write:organization. The organisation's repositories are +// made by the module's setup login from its config value `repositories` (cmd/repositories). + +import { test, after } from "node:test"; +import assert from "node:assert/strict"; +import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; + +import { GiteaClient } from "../dist/client.js"; +import { ConfiguredToken, TOKEN_SCOPES } from "../dist/token.js"; +import { getGiteaTools } from "../dist/tools/index.js"; +import { TOOLS_TEAM_UNITS } from "../dist/tools-account.js"; + +const sent: { method: string; path: string; body: any }[] = []; + +const server = createServer(async (req: IncomingMessage, res: ServerResponse) => { + const chunks: Buffer[] = []; + for await (const c of req) chunks.push(c as Buffer); + const raw = Buffer.concat(chunks).toString(); + const path = new URL(req.url ?? "", "http://x").pathname.replace(/^\/api\/v1/, ""); + const body = raw ? JSON.parse(raw) : undefined; + sent.push({ method: req.method ?? "GET", path, body }); + res.setHeader("Content-Type", "application/json"); + if (req.method === "POST" && path === "/user/repos") { + res.statusCode = 201; + res.end(JSON.stringify({ full_name: `mesh-tools/${body.name}`, name: body.name, owner: { login: "mesh-tools" }, + private: body.private, html_url: "", clone_url: "", default_branch: "main" })); + return; + } + res.statusCode = 404; + res.end(JSON.stringify({ message: "not found" })); +}); +await new Promise((r) => server.listen(0, "127.0.0.1", () => r())); +after(() => new Promise((r) => server.close(() => r()))); +const { port } = server.address() as { port: number }; +const client = new GiteaClient(`http://127.0.0.1:${port}`, new ConfiguredToken("t")); +const tool = getGiteaTools(client).find((t) => t.name === "gitea_create_repo")!; + +test("gitea_create_repo has no owner argument and posts only to the tools' own user's repositories (1124/AC8)", async () => { + assert.equal("owner" in tool.input, false, "the tool takes no owner"); + assert.equal(GiteaClient.prototype.createRepo.length, 1, "createRepo takes the repository alone, no owner"); + // An owner given anyway is not an argument of the tool: the repository is the tools' user's all the same. + await tool.run({ owner: "novox", name: "mesh-delivery" }); + await client.createRepo({ name: "x" }); + assert.deepEqual(sent.map((s) => `${s.method} ${s.path}`), ["POST /user/repos", "POST /user/repos"]); + assert.ok(sent.every((s) => !s.path.startsWith("/orgs/")), "nothing is sent to an organisation"); + assert.doesNotMatch(tool.description, /organisation/i, "the description offers no organisation"); +}); + +test("the tools' team cannot create in the organisation and its token holds no write:organization (1124/AC8)", () => { + assert.equal(TOOLS_TEAM_UNITS.can_create_org_repo, false); + assert.ok(!TOKEN_SCOPES.includes("write:organization"), `scopes ${TOKEN_SCOPES.join(", ")}`); + assert.ok(TOKEN_SCOPES.every((s) => !/organization|admin/.test(s)), `scopes ${TOKEN_SCOPES.join(", ")}`); +});