From 64c893e2c2edccead1b13a68a36470feed30c452 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:15:41 +0100 Subject: [PATCH] F-catalog-B: the gitea module removes the npm pair's teams packages and readers and their accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0605 §5 revokes the npm write token, and the 14:04 deferral is withdrawn (security review F5): the accounts the removed pair provisioner made (mesh__) still logged in with write on the organisation's packages. cmd/agent-teams, run at each start of the module and change of its teams value, removes both teams and each account of that form whose only teams they are, never purging: one the forge refuses to delete because it owns something keeps it and has its login prohibited. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- modules/gitea/README.md | 9 +- modules/gitea/cmd/agent-teams/main.go | 4 +- modules/gitea/cmd/agent-teams/retire.go | 107 ++++++++++++++++++++ modules/gitea/cmd/agent-teams/teams.go | 4 + modules/gitea/cmd/agent-teams/teams_test.go | 64 ++++++++++++ 5 files changed, 184 insertions(+), 4 deletions(-) create mode 100644 modules/gitea/cmd/agent-teams/retire.go diff --git a/modules/gitea/README.md b/modules/gitea/README.md index 75221eb7..de98b90e 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -13,8 +13,10 @@ trusts that root. On a machine with no public domain the key is left out, and th **Both package provisions carry no credential** (`credential.form` none, design 51 *Publishing*): reads of the organisation's packages are anonymous, and every write goes through the seats' verbs, so no consumer is given an -account of the forge. The registry accounts the TypeScript pair provisioner made before stay on the forge until -removed by hand; nothing hands their passwords out any more. +account of the forge. The registry accounts the TypeScript pair provisioner made before (`mesh__`) +and its two teams `packages` and `readers` are removed by `cmd/agent-teams` at each of its runs (ADR 0605 §5 revokes the +npm write token): both teams go, and each account of that form whose only teams they are; one the forge refuses to +delete because it owns something keeps its data and has its login prohibited. ## Announcing pull requests to the merge check (novox/hq to-be 45 §9, issue 347) @@ -142,7 +144,8 @@ teams exist and who is in them is the operator's, and it is one trusted config v whenever that file changes. For each team it makes `agent-read-` in the organisation of the first repository's owner, with permission read and no other right, its repositories exactly the listed ones and its members exactly the listed accounts (an account that does not exist yet is skipped and said). A team named `agent-read-*` that has left -the value is emptied, never deleted. A team without that prefix is never touched. There are no verbs: a change is a +the value is emptied, never deleted. A team without that prefix is never touched, but the two the module made once +and declares no more, `packages` and `readers` (above). There are no verbs: a change is a change of the value. It says each change in one line on stdout, and never a secret. `go test ./cmd/agent-teams` runs it against a fake forge. diff --git a/modules/gitea/cmd/agent-teams/main.go b/modules/gitea/cmd/agent-teams/main.go index eb92207c..a42d5c8a 100644 --- a/modules/gitea/cmd/agent-teams/main.go +++ b/modules/gitea/cmd/agent-teams/main.go @@ -2,7 +2,9 @@ // runtime runs once when the module starts and again whenever the module's config value `agent-read-teams` changes: // it reads the file the mesh renders from that value, and makes the forge's teams named agent-read-* match it. Each // team reads only; its repositories and its members (agent accounts, agent-) are exactly the value's. It -// touches nothing outside the value: no team without the prefix, no repository or account of another kind. +// touches nothing else of the value's kind: no team without the prefix, no repository or account of another kind. Beside +// the value it removes what the module made once and declares no more (retire.go): the npm pair's teams packages and +// readers and the accounts in them alone. // // It says each change on stdout in one line, and never a secret. package main diff --git a/modules/gitea/cmd/agent-teams/retire.go b/modules/gitea/cmd/agent-teams/retire.go new file mode 100644 index 00000000..8da05e44 --- /dev/null +++ b/modules/gitea/cmd/agent-teams/retire.go @@ -0,0 +1,107 @@ +package main + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/url" + "regexp" + "slices" + "sort" + "strings" + + "gitea/internal/forgehttp" +) + +// The forge accounts and teams the module made once and declares no more (novox/hq ADR 0605 §5: no build module holds +// a registry's write credential; the npm write token is revoked). The npm pair provisioner, removed when the package +// seats came, made each consumer of npm-package-registry an account under the mesh's derived login +// (mesh__) and put it in the organisation's team packages (read and write) or readers (read only). +// Reads of the packages are anonymous and every write goes through the seats, so both teams go, and each account of +// that form in no team but them goes with them. Nothing else is touched: a team of another name, an account of +// another form or one still in a team the module does not retire stays as it is. +var RetiredTeams = []string{"packages", "readers"} + +// retiredAccount is the login form the npm pair provisioner made its accounts under. +var retiredAccount = regexp.MustCompile(`^mesh_[a-z0-9-]+_[a-z0-9_]+$`) + +// Retire removes the retired teams in every organisation, and each account of the pair's form whose only teams they +// are. An account the forge refuses to delete (it owns a repository or an organisation, so a delete would take what +// it owns) has its login prohibited instead, said, so its password no longer reaches anything. Each removal is said +// in one line; a run with nothing left to retire says nothing. +func (s *Syncer) Retire(ctx context.Context, orgs []string) error { + teamsOf := map[string][]string{} // login -> every team it is in, as org/team + type retired struct { + org string + team forgeTeam + } + var gone []retired + for _, org := range orgs { + teams, err := s.teams(ctx, org) + if err != nil { + return err + } + for _, t := range teams { + var members []forgeMember + if err := s.list(ctx, fmt.Sprintf("/api/v1/teams/%d/members", t.ID), &members); err != nil { + return fmt.Errorf("the members of %s in %s could not be read, so nothing is retired: %w", t.Name, org, err) + } + for _, m := range members { + teamsOf[m.Login] = append(teamsOf[m.Login], org+"/"+t.Name) + } + if slices.Contains(RetiredTeams, t.Name) { + gone = append(gone, retired{org, t}) + } + } + } + isRetired := func(orgTeam string) bool { + _, team, _ := strings.Cut(orgTeam, "/") + return slices.Contains(RetiredTeams, team) + } + var accounts []string + for login, in := range teamsOf { + if retiredAccount.MatchString(login) && slices.ContainsFunc(in, isRetired) && !slices.ContainsFunc(in, func(t string) bool { return !isRetired(t) }) { + accounts = append(accounts, login) + } + } + sort.Strings(accounts) + var errs []string + for _, a := range accounts { + if err := s.removeAccount(ctx, a); err != nil { + errs = append(errs, err.Error()) + } + } + for _, r := range gone { + if err := s.Forge.SendJSON(ctx, http.MethodDelete, fmt.Sprintf("/api/v1/teams/%d", r.team.ID), nil, nil); err != nil && !errors.Is(err, forgehttp.ErrNotFound) { + errs = append(errs, fmt.Sprintf("the retired team %s in %s could not be removed: %v", r.team.Name, r.org, err)) + continue + } + s.say("removed the retired team %s in %s (the npm pair's, ADR 0605 §5)", r.team.Name, r.org) + } + if len(errs) > 0 { + return errors.New(strings.Join(errs, "; ")) + } + return nil +} + +// removeAccount deletes a retired account, never purging what it owns; one the forge refuses to delete has its login +// prohibited instead. +func (s *Syncer) removeAccount(ctx context.Context, login string) error { + err := s.Forge.SendJSON(ctx, http.MethodDelete, "/api/v1/admin/users/"+url.PathEscape(login), nil, nil) + if err == nil || errors.Is(err, forgehttp.ErrNotFound) { + s.say("removed the retired account %s (the npm pair's, ADR 0605 §5)", login) + return nil + } + var se *forgehttp.StatusError + if !errors.As(err, &se) || se.Status != http.StatusUnprocessableEntity { + return fmt.Errorf("the retired account %s could not be removed: %w", login, err) + } + if perr := s.Forge.SendJSON(ctx, http.MethodPatch, "/api/v1/admin/users/"+url.PathEscape(login), map[string]any{ + "login_name": login, "source_id": 0, "prohibit_login": true, + }, nil); perr != nil { + return fmt.Errorf("the retired account %s could not be removed (%v) nor its login prohibited: %w", login, err, perr) + } + s.say("the retired account %s owns something, so the forge keeps it (%v): its login is prohibited instead", login, err) + return nil +} diff --git a/modules/gitea/cmd/agent-teams/teams.go b/modules/gitea/cmd/agent-teams/teams.go index 5f2a0b6e..2e7685c2 100644 --- a/modules/gitea/cmd/agent-teams/teams.go +++ b/modules/gitea/cmd/agent-teams/teams.go @@ -135,6 +135,10 @@ func (s *Syncer) Sync(ctx context.Context, v Value) error { } } } + if err := s.Retire(ctx, orgs); err != nil { + s.say("not retired: %v", err) + failed = append(failed, "the retired teams and accounts") + } if len(failed) > 0 { return fmt.Errorf("not applied: %s", strings.Join(failed, ", ")) } diff --git a/modules/gitea/cmd/agent-teams/teams_test.go b/modules/gitea/cmd/agent-teams/teams_test.go index 0dfe617c..a6800757 100644 --- a/modules/gitea/cmd/agent-teams/teams_test.go +++ b/modules/gitea/cmd/agent-teams/teams_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "sort" "strconv" "strings" @@ -34,6 +35,9 @@ type fakeForge struct { users map[string]bool teams []*fTeam next int64 + // owners are accounts that own a repository: the forge refuses to delete them (422); prohibited, whose login + // was prohibited. + owners, prohibited map[string]bool } func newFake(orgs []string, users ...string) *fakeForge { @@ -122,6 +126,26 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { out = nil } reply(200, out) + case strings.HasPrefix(p, "/api/v1/admin/users/"): + u := strings.TrimPrefix(p, "/api/v1/admin/users/") + switch { + case !f.users[u]: + reply(404, nil) + case r.Method == http.MethodDelete && f.owners[u]: + reply(422, map[string]string{"message": "user still owns one or more repositories"}) + case r.Method == http.MethodDelete: + delete(f.users, u) + for _, t := range f.teams { + delete(t.Members, u) + } + reply(204, nil) + case r.Method == http.MethodPatch && in["prohibit_login"] == true: + if f.prohibited == nil { + f.prohibited = map[string]bool{} + } + f.prohibited[u] = true + reply(200, map[string]string{}) + } case strings.HasPrefix(p, "/api/v1/users/"): if !f.users[strings.TrimPrefix(p, "/api/v1/users/")] { reply(404, nil) @@ -155,6 +179,10 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } reply(200, out) + case teamPath.MatchString(p) && r.Method == http.MethodDelete: + t := f.find(teamPath.FindStringSubmatch(p)[1]) + f.teams = slices.DeleteFunc(f.teams, func(x *fTeam) bool { return x == t }) + reply(204, nil) case teamPath.MatchString(p): t := f.find(teamPath.FindStringSubmatch(p)[1]) t.Perm = in["permission"].(string) @@ -331,3 +359,39 @@ func TestReadValue(t *testing.T) { t.Fatal("an unknown field was accepted") } } + +// Security review F5 (ADR 0605 §5): the npm pair's teams packages and readers go, and each account of the pair's form +// whose only teams they are; one that owns a repository is kept with its login prohibited; an account of the pair's +// form still in another team, an account of another form and every other team stay; a second run changes nothing. +func TestTheNpmPairsTeamsAndAccountsAreRetired(t *testing.T) { + f := newFake([]string{"novox"}, "mesh_ace_mesh_build_node", "mesh_shanks_lab", "mesh_g14_agent", "mesh_novox_owner", + "mesh-packages", "agent-ace", "jschoubben") + f.owners = map[string]bool{"mesh_novox_owner": true} + f.add("novox", "packages", "none", nil, []string{"mesh_ace_mesh_build_node", "mesh_g14_agent", "mesh_novox_owner", "jschoubben"}) + f.add("novox", "readers", "none", nil, []string{"mesh_ace_mesh_build_node", "mesh_shanks_lab"}) + f.add("novox", "agents", "none", nil, []string{"mesh_g14_agent", "agent-ace"}) + f.add("novox", "mesh-packages", "none", nil, []string{"mesh-packages"}) + s, said := syncer(t, f) + if err := s.Sync(context.Background(), Value{}); err != nil { + t.Fatal(err) + } + if f.byName("novox", "packages") != nil || f.byName("novox", "readers") != nil { + t.Errorf("the retired teams stay: %+v", f.teams) + } + if f.byName("novox", "agents") == nil || f.byName("novox", "mesh-packages") == nil { + t.Errorf("a team the module did not retire was removed: %+v", f.teams) + } + for u, want := range map[string]bool{"mesh_ace_mesh_build_node": false, "mesh_shanks_lab": false, "mesh_g14_agent": true, + "mesh_novox_owner": true, "mesh-packages": true, "agent-ace": true, "jschoubben": true} { + if f.users[u] != want { + t.Errorf("account %s kept %v, want %v", u, f.users[u], want) + } + } + if !f.prohibited["mesh_novox_owner"] || len(f.prohibited) != 1 { + t.Errorf("prohibited %v, want mesh_novox_owner alone", f.prohibited) + } + before := len(*said) + if err := s.Sync(context.Background(), Value{}); err != nil || len(*said) != before { + t.Errorf("a second run: %v, said %v; want nothing", err, (*said)[before:]) + } +}