diff --git a/modules/gitea/README.md b/modules/gitea/README.md index d1a3397b..cb97c96f 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -102,21 +102,23 @@ changed under the commit refused; a commit over its size limits refused; protect forge matches them; a code search stopped at its deadline; every malformed ask refused; and each read summarised. -## Each agent's read credential (novox/hq ADR 0365 §1, §3–§5, issue 737) +## Each agent's credential (novox/hq ADR 0365 §1, §3–§5, issue 737; ADR 0398 §1, issue 948) -An agent clones and fetches the repositories it may read as a forge account of its own, `agent-`. The +An agent clones, fetches and pushes branches of the repositories as a forge account of its own, `agent-`. The module offers the provision `gitea-read`, whose credential its software issues (ADR 0305) one per holder (issue 749): the Go bundle `cmd/agent-token` serves `gitea_agent_token`, which the vault alone asks (ADR 0349 clause 10; any other caller is refused and nothing is minted). For each holder the vault names that is the claude-code module on a machine, it - makes the account `agent-` with the administrator's login, when there is none: no administrator, a random - password sent once and kept nowhere, and **in no team** — it reads public repositories until the operator adds it - to a team that reads more (§3). An account found made an administrator, inactive or barred from logging in is + password sent once and kept nowhere. It is added to the team `agents` of the organisation `novox`, which the + administrator's login makes when there is none and puts back to its shape when found changed: write on code, issues + and pull requests of every repository, no right to make repositories (the shape of the forge tools' team). The + operator's read teams (`agent-read-*`, ADR 0365 §3) are `cmd/agent-teams`'s and stay. An account found made an administrator, inactive or barred from logging in is made a plain account again; -- deletes the account's token by the name `mesh-agent-read` (by its id, every one of them) and mints one whose one - scope is `read:repository`: whatever team it is put in, the forge refuses it a push, a comment and any token or - administration of its own (§5). Every ask mints anew; that is its rotation; +- deletes the account's token by the name `mesh-agent-read` (by its id, every one of them) and mints one whose + scopes are `write:repository` and `write:issue`: the forge refuses it any token or administration of its own + (§5), and the main branch of every repository takes no push from it (ADR 0398 §2). Every ask mints anew; that is its rotation; - answers git's credential-store lines for it — the forge's public origin, read from `origins` (the route's name), and on the forge's own machine its address there too — sealed to the key the vault made for the call. A holder of another module is not answered and said under `skipped`. @@ -209,3 +211,13 @@ run before anything is deleted; an unreadable range keeping its whole package; a without saying the password. And the range matcher against npm's rules — exact, `^`, `~`, `x` and `*`, comparisons with partial versions, hyphen ranges, `||`, and the pre-release rule — and each lockfile format. + +## Main takes no push, and only the tools' account merges (novox/hq ADR 0398 §2, issue 948) + +The rule for `main` is the same in every repository of the organisation `novox`: `enable_push` false, and a merge +whitelist of the tools' account (`mesh-tools`) alone. `protection.ts` composes it (`mainRuleBody`, and +`protectionBody` for the branch `main` always carries it); `setUpToolsAccount` puts it right in every repository +whenever the tools' account is set up, with the administrator's login (`GiteaAdmin.ensureMainMerges`) — the tools' +account is no repository's administrator, so no tool can. A rule found keeps the statuses it requires; a repository +with none is given one that requires none. `judgeProtectionChange` refuses, for `main`, a change that enables +push, drops the merge whitelist or widens it past that account. diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index bb8de0d2..e0b80364 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -5,7 +5,7 @@ import { readFileSync } from "node:fs"; import { ConfiguredToken, MintedToken, type TokenSource } from "./token.js"; -import { protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js"; +import { MAIN_BRANCH, mainRuleBody, protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js"; import { Refused, checkSentPath, pathSegment, pathSegments } from "./guard.js"; import { isCommit } from "./checks.js"; import { TOOLS_ACCOUNT } from "./tools-account.js"; @@ -713,6 +713,9 @@ export class GiteaClient { enable_status_check: Boolean(b.enable_status_check), status_check_contexts: Array.isArray(b.status_check_contexts) ? b.status_check_contexts.map(String) : [], required_approvals: Number(b.required_approvals ?? 0), + // Not every forge version sends these on a branch; absent they stay absent, never read as "no whitelist". + ...(b.enable_merge_whitelist === undefined ? {} : { enable_merge_whitelist: Boolean(b.enable_merge_whitelist) }), + ...(Array.isArray(b.merge_whitelist_usernames) ? { merge_whitelist_usernames: b.merge_whitelist_usernames.map(String) } : {}), }; } @@ -721,7 +724,7 @@ export class GiteaClient { * asked — or created, refusing direct pushes unless asked otherwise. Answers the rule as it is now. */ async setBranchProtection(owner: string, repo: string, branch: string, want: ProtectionWanted): Promise<{ created: boolean; rule: BranchProtection }> { const had = await this.branchProtection(owner, repo, branch); - const body = protectionBody(want, !had); + const body = protectionBody(want, !had, branch); if (had) { const rule = await this.request(`${repoPath(owner, repo)}/branch_protections/${pathSegments("branch", branch)}`, { method: "PATCH", body: JSON.stringify(body) }); @@ -988,6 +991,39 @@ export class GiteaAdmin { return Array.isArray(res.body) ? res.body : []; } + /** Put main's rule right in every repository of an organisation (novox/hq ADR 0398 §2, issue 948): no push, and + * only the tools' account merges. The rule there is edited, its statuses left as they are; a repository with no + * rule for main gets one that requires none. Answers the repositories it touched. With the administrator's login: + * the tools' account is no repository's administrator, so no tool can do this (ADR 0365 §2). */ + async ensureMainMerges(org: string): Promise { + const repos: string[] = []; + for (let page = 1; page <= 200; page++) { + const res = await this.request(`/orgs/${encodeURIComponent(org)}/repos?limit=50&page=${page}`); + if (res.status !== 200) GiteaAdmin.fail(`/orgs/${org}/repos`, res); + const list = Array.isArray(res.body) ? res.body : []; + for (const r of list) if (r?.name) repos.push(String(r.name)); + if (list.length < 50) break; + } + for (const repo of repos) { + const rule = `${repoPath(org, repo)}/branch_protections/${MAIN_BRANCH}`; + const patch = await this.request(rule, { method: "PATCH", body: JSON.stringify(mainRuleBody()) }); + if (patch.status === 200) continue; + if (patch.status !== 404) GiteaAdmin.fail(rule, patch); + const made = await this.request(`${repoPath(org, repo)}/branch_protections`, { + method: "POST", + body: JSON.stringify({ rule_name: MAIN_BRANCH, enable_status_check: false, status_check_contexts: [], ...mainRuleBody() }), + }); + if (made.status === 201) continue; + if (made.status === 403 || made.status === 409 || made.status === 422) { + // A rule made between the two calls: edit it. + const again = await this.request(rule, { method: "PATCH", body: JSON.stringify(mainRuleBody()) }); + if (again.status === 200) continue; + } + GiteaAdmin.fail(`${repoPath(org, repo)}/branch_protections`, made); + } + return repos; + } + /** Ensure the npm-owner org exists. 201 created, 2xx/404-then-created, and 422/409 (a concurrent * create won the race) are all success. */ async ensureOrg(name: string): Promise { diff --git a/modules/gitea/cmd/agent-token/main.go b/modules/gitea/cmd/agent-token/main.go index 92f5a6d8..0303b670 100644 --- a/modules/gitea/cmd/agent-token/main.go +++ b/modules/gitea/cmd/agent-token/main.go @@ -1,10 +1,10 @@ -// agent-token: the forge's read credential for each node's agent (novox/hq ADR 0365 clauses 1, 3, 4 and 5; issue -// 737). A process the node's runtime launches and speaks MCP over stdio to, beside the module's other bundles. +// agent-token: the forge's credential for each node's agent (novox/hq ADR 0365 clauses 1, 3, 4 and 5, ADR 0398 §1; issues +// 737, 948). A process the node's runtime launches and speaks MCP over stdio to, beside the module's other bundles. // // It serves one tool, gitea_agent_token, which the vault alone asks (ADR 0349 clause 10, issue 749): for each holder // of the provision `gitea-read` the vault names — the claude-code module on one machine — it makes that machine's -// agent a forge account of its own, `agent-`, no administrator and in no team, mints it a token whose one -// scope is `read:repository`, and answers the git credential lines for it, each holder's sealed to the key the vault +// agent a forge account of its own, `agent-`, no administrator and a member of the `agents` write team, mints it a token whose +// scopes are `write:repository` and `write:issue`, and answers the git credential lines for it, each holder's sealed to the key the vault // made for the call. Asked again, it mints again and deletes the account's token from before: that is its rotation. // // stdout is the protocol; it says what it says on stderr, and never a token or the admin's password. @@ -33,10 +33,10 @@ func main() { func Tools(minter func() (*Minter, error)) []stdio.Tool { return []stdio.Tool{{ Name: ToolName, - Description: "The forge's read credential for each node's agent (novox/hq ADR 0365), answered to the vault " + + Description: "The forge's credential for each node's agent (novox/hq ADR 0365, ADR 0398), answered to the vault " + "alone: for each holder named, a forge account of the node's agent's own (agent-), no administrator " + - "and in no team, and a token of scope read:repository minted for it anew, its git credential lines " + - "sealed to seal_to. Any other caller is refused and nothing is minted.", + "and a member of the agents team (write on every repository), and a token of scopes write:repository and " + + "write:issue minted for it anew, its git credential lines sealed to seal_to. Any other caller is refused and nothing is minted.", Input: map[string]any{ "type": "object", "properties": map[string]any{ diff --git a/modules/gitea/cmd/agent-token/token.go b/modules/gitea/cmd/agent-token/token.go index 33e8de9e..a827312f 100644 --- a/modules/gitea/cmd/agent-token/token.go +++ b/modules/gitea/cmd/agent-token/token.go @@ -23,7 +23,7 @@ import ( const ( // ToolName is the issuing tool the vault asks (the offer's credential tool in module.json). ToolName = "gitea_agent_token" - // Provision is what the tool issues: the forge's read credential, one per agent. + // Provision is what the tool issues: the forge's agent credential (named gitea-read from before it wrote), one per agent. Provision = "gitea-read" // Holder is the one consuming module answered: the agent's own module, which places the credential in the // agent's home (ADR 0365 §4, ADR 0266). A holder of another module is not answered and keeps nothing. @@ -38,10 +38,26 @@ const ( deadline = 45 * time.Second ) -// TokenScopes are the only scopes an agent's token carries (ADR 0365 §4, §5): it reads repositories and nothing -// else, so the forge refuses it a push, a comment, a token of its own and any administration — whatever team the -// account is in. -var TokenScopes = []string{"read:repository"} +// TokenScopes are the only scopes an agent's token carries (ADR 0398 §1, issue 948): it writes repositories and +// issues and nothing else, so the forge refuses it a token of its own and any administration. Where it may write is +// the agents team's (a write team on every repository); main takes no push whatever the token (ADR 0398 §2). +var TokenScopes = []string{"write:repository", "write:issue"} + +const ( + // TeamOrg is the organisation whose repositories are the mesh's. + TeamOrg = "novox" + // TeamName is the one team every agent's account is in (ADR 0398 §1). + TeamName = "agents" +) + +// teamShape is what the agents team grants: write on code, issues and pull requests of every repository of the +// organisation and no more, the shape of the forge tools' team (tools-account.ts). +func teamShape() map[string]any { + return map[string]any{ + "name": TeamName, "permission": "write", "includes_all_repositories": true, "can_create_org_repo": false, + "units_map": map[string]string{"repo.code": "write", "repo.issues": "write", "repo.pulls": "write"}, + } +} var nodeName = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,31}$`) @@ -193,6 +209,9 @@ func (m *Minter) Answer(ask Ask) (Answer, error) { if err := m.ensureAccount(ctx, account); err != nil { return Answer{}, err } + if err := m.ensureInTeam(ctx, account); err != nil { + return Answer{}, err + } token, err := m.mint(ctx, account) if err != nil { return Answer{}, err @@ -208,7 +227,7 @@ func (m *Minter) Answer(ask Ask) (Answer, error) { return Answer{}, fmt.Errorf("the credential for %s could not be sealed: %w", account, err) } out.Holders = append(out.Holders, Sealed{Node: h.Node, Module: h.Module, Sealed: base64.StdEncoding.EncodeToString(sealed)}) - m.say("minted a read token for %s (%s on %s); its token from before is deleted", account, h.Module, h.Node) + m.say("minted a write token for %s (%s on %s); its token from before is deleted", account, h.Module, h.Node) } return out, nil } @@ -264,8 +283,7 @@ type forgeUser struct { } // ensureAccount makes the agent's account, plain, when there is none, and puts back what was changed by hand: never -// an administrator, active, able to use its token. It adds it to no team: what it may read beyond public -// repositories is the operator's to grant (ADR 0365 §3). +// an administrator, active, able to use its token. Its team is ensureInTeam's. func (m *Minter) ensureAccount(ctx context.Context, account string) error { var u forgeUser err := m.Forge.GetJSON(ctx, "/api/v1/users/"+account, &u) @@ -280,7 +298,7 @@ func (m *Minter) ensureAccount(ctx context.Context, account string) error { if err != nil { return fmt.Errorf("the account %s could not be made: %w", account, err) } - m.say("made the forge account %s for that machine's agent, in no team", account) + m.say("made the forge account %s for that machine's agent, in no team yet", account) return nil case err != nil: return fmt.Errorf("the account %s could not be read: %w", account, err) @@ -297,6 +315,50 @@ func (m *Minter) ensureAccount(ctx context.Context, account string) error { return nil } +// ensureInTeam makes the agents team when there is none, puts a team that is there back to its shape, and adds the +// account to it; an account already in it stays (ADR 0398 §1, issue 948). +func (m *Minter) ensureInTeam(ctx context.Context, account string) error { + var id int64 + for page := 1; page <= 20 && id == 0; page++ { + var teams []struct { + ID int64 `json:"id"` + Name string `json:"name"` + } + if err := m.Forge.GetJSON(ctx, fmt.Sprintf("/api/v1/orgs/%s/teams?limit=50&page=%d", TeamOrg, page), &teams); err != nil { + return fmt.Errorf("the teams of %s could not be read: %w", TeamOrg, err) + } + for _, t := range teams { + if t.Name == TeamName { + id = t.ID + } + } + if len(teams) < 50 { + break + } + } + if id != 0 { + if err := m.Forge.SendJSON(ctx, http.MethodPatch, fmt.Sprintf("/api/v1/teams/%d", id), teamShape(), nil); err != nil { + return fmt.Errorf("the team %s could not be put back to its shape: %w", TeamName, err) + } + } else { + var made struct { + ID int64 `json:"id"` + } + if err := m.Forge.SendJSON(ctx, http.MethodPost, "/api/v1/orgs/"+TeamOrg+"/teams", teamShape(), &made); err != nil { + return fmt.Errorf("the team %s could not be made: %w", TeamName, err) + } + id = made.ID + m.say("made the forge team %s in %s: write on every repository", TeamName, TeamOrg) + } + if id == 0 { + return fmt.Errorf("the forge answered no id for the team %s", TeamName) + } + if err := m.Forge.SendJSON(ctx, http.MethodPut, fmt.Sprintf("/api/v1/teams/%d/members/%s", id, account), nil, nil); err != nil { + return fmt.Errorf("the account %s could not be added to the team %s: %w", account, TeamName, err) + } + return nil +} + type forgeToken struct { ID int64 `json:"id"` Name string `json:"name"` @@ -304,7 +366,7 @@ type forgeToken struct { } // mint deletes every token of the account by this module's name, by its id (a name held twice is cleared, not -// refused), and mints one with the read scope alone. +// refused), and mints one with the two write scopes alone. func (m *Minter) mint(ctx context.Context, account string) (string, error) { var held []forgeToken if err := m.Forge.GetJSON(ctx, "/api/v1/users/"+account+"/tokens?limit=50", &held); err != nil { diff --git a/modules/gitea/cmd/agent-token/token_test.go b/modules/gitea/cmd/agent-token/token_test.go index 3f12dc4d..9fbdfb63 100644 --- a/modules/gitea/cmd/agent-token/token_test.go +++ b/modules/gitea/cmd/agent-token/token_test.go @@ -33,13 +33,20 @@ type fakeForge struct { made map[string]map[string]any minted map[string][]string // account -> scopes of its last mint failMint bool + // teams by name; members by team name (issue 948). + teams map[string]map[string]any + teamIDs map[string]int64 + members map[string][]string } func newFakeForge() *fakeForge { return &fakeForge{users: map[string]*forgeUser{}, tokens: map[string][]forgeToken{}, made: map[string]map[string]any{}, - minted: map[string][]string{}, next: 1} + minted: map[string][]string{}, next: 1, + teams: map[string]map[string]any{}, teamIDs: map[string]int64{}, members: map[string][]string{}} } +var teamPath = regexp.MustCompile(`^/api/v1/teams/(\d+)(?:/members/([^/]+))?$`) + var tokenPath = regexp.MustCompile(`^/api/v1/users/([^/]+)/tokens(?:/(\d+))?$`) func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -80,6 +87,40 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { u.Active, _ = in["active"].(bool) u.ProhibitLogin, _ = in["prohibit_login"].(bool) reply(200, map[string]any{}) + case r.URL.Path == "/api/v1/orgs/novox/teams": + if r.Method == http.MethodGet { + var list []map[string]any + for name, id := range f.teamIDs { + list = append(list, map[string]any{"id": id, "name": name}) + } + reply(200, list) + return + } + name, _ := in["name"].(string) + f.teams[name] = in + f.teamIDs[name] = f.next + f.next++ + reply(201, map[string]any{"id": f.teamIDs[name], "name": name}) + case teamPath.MatchString(r.URL.Path): + m := teamPath.FindStringSubmatch(r.URL.Path) + for name, id := range f.teamIDs { + if fmt.Sprint(id) != m[1] { + continue + } + if m[2] == "" && r.Method == http.MethodPatch { + f.teams[name] = in + reply(200, map[string]any{"id": id}) + return + } + if m[2] != "" && r.Method == http.MethodPut { + if !slices.Contains(f.members[name], m[2]) { + f.members[name] = append(f.members[name], m[2]) + } + reply(204, nil) + return + } + } + reply(404, map[string]any{"message": "no such team"}) case tokenPath.MatchString(r.URL.Path): m := tokenPath.FindStringSubmatch(r.URL.Path) account := m[1] @@ -199,7 +240,7 @@ func TestOnlyTheVaultIsAnsweredAndNothingIsMintedForAnyoneElse(t *testing.T) { } } -func TestEachAgentGetsAnAccountOfItsOwnAndAReadTokenSealedForItAlone(t *testing.T) { +func TestEachAgentGetsAnAccountOfItsOwnAndAWriteTokenSealedForItAlone(t *testing.T) { h := newHarness(t) ask, err := readAsk(h.args(HolderAsk{"anchor", "claude-code"}, HolderAsk{"laptop", "claude-code"}, HolderAsk{"laptop", "lab"})) if err != nil { @@ -240,15 +281,23 @@ func TestEachAgentGetsAnAccountOfItsOwnAndAReadTokenSealedForItAlone(t *testing. if made := h.forge.made[account]; made["must_change_password"] != false || made["password"] == "" { t.Errorf("%s was made with %v", account, made) } - if !slices.Equal(h.forge.minted[account], []string{"read:repository"}) { - t.Errorf("%s's token carries %v, want read:repository alone", account, h.forge.minted[account]) + if !slices.Equal(h.forge.minted[account], []string{"write:repository", "write:issue"}) { + t.Errorf("%s's token carries %v, want write:repository and write:issue alone (issue 948)", account, h.forge.minted[account]) } } for _, c := range h.forge.calls { - if strings.Contains(c, "/teams") || strings.Contains(c, "/orgs/") || strings.Contains(c, "collaborators") { - t.Errorf("an agent's account was given access (%s): it starts in no team, the operator grants (ADR 0365 §3)", c) + if strings.Contains(c, "collaborators") { + t.Errorf("an agent's account was given access one repository at a time (%s) (issue 948)", c) } } + for _, account := range []string{"agent-anchor", "agent-laptop"} { + if !slices.Contains(h.forge.members["agents"], account) { + t.Errorf("%s is not in the agents team: %v (issue 948, ADR 0398 §1)", account, h.forge.members) + } + } + if len(h.forge.members) != 1 { + t.Errorf("an agent was put in a team but agents: %v", h.forge.members) + } if _, made := h.forge.users["agent-laptop-lab"]; made || len(h.forge.users) != 2 { t.Errorf("accounts made: %v", h.forge.users) } @@ -374,3 +423,40 @@ func tokenIn(t *testing.T, lines, account string) string { } return all[0][1] } + +func TestTheAgentsTeamIsMadeWithWriteOnEveryRepositoryAndReusedWhenThere(t *testing.T) { + h := newHarness(t) + ask, _ := readAsk(h.args(HolderAsk{"laptop", "claude-code"})) + if _, err := h.minter.Answer(ask); err != nil { + t.Fatal(err) + } + team := h.forge.teams["agents"] + if team == nil { + t.Fatalf("no agents team was made (issue 948): %v", h.forge.teams) + } + if team["permission"] != "write" || team["includes_all_repositories"] != true || team["can_create_org_repo"] != false { + t.Errorf("the agents team is %v, want write on all repositories and no repository creation", team) + } + units, _ := team["units_map"].(map[string]any) + for _, u := range []string{"repo.code", "repo.issues", "repo.pulls"} { + if units[u] != "write" { + t.Errorf("unit %s is %v, want write", u, units[u]) + } + } + id := h.forge.teamIDs["agents"] + // Changed by hand, then asked again for another node: reused, put back, the new account added. + h.forge.teams["agents"] = map[string]any{"name": "agents", "permission": "read"} + ask, _ = readAsk(h.args(HolderAsk{"laptop", "claude-code"}, HolderAsk{"anchor", "claude-code"})) + if _, err := h.minter.Answer(ask); err != nil { + t.Fatal(err) + } + if h.forge.teamIDs["agents"] != id || len(h.forge.teamIDs) != 1 { + t.Errorf("the team was made again: %v", h.forge.teamIDs) + } + if h.forge.teams["agents"]["permission"] != "write" { + t.Errorf("the team was not put back to write: %v", h.forge.teams["agents"]) + } + if !slices.Equal(h.forge.members["agents"], []string{"agent-laptop", "agent-anchor"}) { + t.Errorf("members %v, each once", h.forge.members["agents"]) + } +} diff --git a/modules/gitea/guard.ts b/modules/gitea/guard.ts index daee4265..1843e38c 100644 --- a/modules/gitea/guard.ts +++ b/modules/gitea/guard.ts @@ -17,7 +17,8 @@ // // Pure, so it is tested without a forge; the tools do the asking (tools/index.ts). -import type { BranchProtection, ProtectionWanted } from "./protection.js"; +import { MAIN_BRANCH, type BranchProtection, type ProtectionWanted } from "./protection.js"; +import { TOOLS_ACCOUNT } from "./tools-account.js"; /** A refusal of policy, in the words the caller sees. Never an error of the forge's. */ export class Refused extends Error { @@ -287,7 +288,10 @@ export function judgeContentsWrite(target: string, defaultBranch: string, rules: * override allowed. */ export function judgeProtectionChange(before: BranchProtection | null, want: ProtectionWanted, - covering: BranchProtection[] = []): string | null { + covering: BranchProtection[] = [], branch = ""): string | null { + const isMain = branch === MAIN_BRANCH || (before !== null && (before.rule_name ?? before.branch_name) === MAIN_BRANCH); + const main = isMain ? judgeMain(before, want) : null; + if (main) return main; if (!before) { for (const glob of covering) { const weaker = judgeProtectionChange(glob, { @@ -315,6 +319,24 @@ export function judgeProtectionChange(before: BranchProtection | null, want: Pro `operator's, at the forge as themselves. Nothing was changed`; } +/** Main's rule takes no push and lets only the tools' account merge (novox/hq ADR 0398 §2, issue 948): a change + * that enables push, drops the merge whitelist or widens it past that account is refused, whatever the rule was. */ +function judgeMain(before: BranchProtection | null, want: ProtectionWanted): string | null { + const why: string[] = []; + if (want.push === true) why.push("it lets a person push to the branch directly"); + if (want.mergeWhitelist !== undefined) { + const names = want.mergeWhitelist ?? []; + if (names.length === 0) why.push("it drops the merge whitelist of main"); + else if (names.some((n) => n !== TOOLS_ACCOUNT)) { + why.push(`it widens who may merge into main beyond ${TOOLS_ACCOUNT}`); + } + } + if (!why.length) return null; + return `the protection of ${before?.rule_name ?? before?.branch_name ?? "main"} is only made stronger through a tool: ` + + `${why.join("; ")}. Main takes no push, and only ${TOOLS_ACCOUNT} merges into it (novox/hq ADR 0398 §2) — ` + + `weakening that is the operator's, at the forge as themselves. Nothing was changed`; +} + /** The token's own user's repositories, untouched for this long, are the only ones a tool deletes. */ export const DELETABLE_AFTER_DAYS = 30; diff --git a/modules/gitea/protection.ts b/modules/gitea/protection.ts index 5751c2c7..923d10c1 100644 --- a/modules/gitea/protection.ts +++ b/modules/gitea/protection.ts @@ -2,6 +2,11 @@ // `mesh/merge-gate`, the module graph's gate, and `mesh/repo-check`, the repository's own tests — before it // may merge. Pure, so it is tested without a forge; the client does the asking (client.ts). +import { TOOLS_ACCOUNT } from "./tools-account.js"; + +/** The branch whose rule takes no push and lets only the tools' account merge (novox/hq ADR 0398 §2, issue 948). */ +export const MAIN_BRANCH = "main"; + /** A branch protection rule, as the forge keeps it — the fields the mesh reads; the forge sends more. */ export interface BranchProtection { rule_name?: string; @@ -11,6 +16,8 @@ export interface BranchProtection { status_check_contexts?: string[] | null; required_approvals?: number; block_admin_merge_override?: boolean; + enable_merge_whitelist?: boolean; + merge_whitelist_usernames?: string[] | null; [field: string]: unknown; } @@ -23,15 +30,24 @@ export interface ProtectionWanted { push?: boolean; /** Whether an administrator is stopped from merging past a status that has not succeeded. */ blockAdminOverride?: boolean; + /** Who may merge into the branch; for main only ever the tools' account (ADR 0398 §2). Not given: left as it is. + * Empty or null asks for no whitelist, which the guard refuses for main; the body for main ignores it. */ + mergeWhitelist?: string[] | null; +} + +/** What main's rule always says: no push, and only the tools' account merges (novox/hq ADR 0398 §2, issue 948). */ +export function mainRuleBody(): Record { + return { enable_push: false, enable_merge_whitelist: true, merge_whitelist_usernames: [TOOLS_ACCOUNT] }; } /** The forge's body for a wanted protection. */ -export function protectionBody(want: ProtectionWanted, creating: boolean): Record { +export function protectionBody(want: ProtectionWanted, creating: boolean, branch = ""): Record { const checks = [...new Set(want.statusChecks.map((c) => c.trim()).filter(Boolean))]; const body: Record = { enable_status_check: checks.length > 0, status_check_contexts: checks }; if (want.push !== undefined) body.enable_push = want.push; else if (creating) body.enable_push = false; if (want.blockAdminOverride !== undefined) body.block_admin_merge_override = want.blockAdminOverride; + if (branch === MAIN_BRANCH) Object.assign(body, mainRuleBody()); // whatever was asked: main takes no push (ADR 0398 §2) return body; } diff --git a/modules/gitea/test/guard.test.ts b/modules/gitea/test/guard.test.ts index 034bf238..fda32d5d 100644 --- a/modules/gitea/test/guard.test.ts +++ b/modules/gitea/test/guard.test.ts @@ -491,6 +491,24 @@ test("the judges, without a forge", () => { assert.equal(judgeProtectionChange(protectedMain(), { statusChecks: ["mesh/repo-check", "mesh/merge-gate"] }), null); }); +test("main's protection is never weakened in its push or its merge whitelist (issue 948, ADR 0398 §2)", () => { + const strong = () => ({ ...protectedMain(), enable_merge_whitelist: true, merge_whitelist_usernames: ["mesh-tools"] }); + const checks = ["mesh/merge-gate", "mesh/repo-check"]; + assert.equal(judgeProtectionChange(strong(), { statusChecks: checks }, [], "main"), null); + assert.equal(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: ["mesh-tools"] }, [], "main"), null); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: [] }, [], "main") ?? "", /merge whitelist/); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: null }, [], "main") ?? "", /merge whitelist/); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: ["mesh-tools", "agent-ace"] }, [], "main") ?? "", /widens/); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: ["agent-ace"] }, [], "main") ?? "", /widens/); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, push: true }, [], "main") ?? "", /push/); + // Even a rule that lets a person push already, or has no rule yet, is not given a push through a tool. + assert.match(judgeProtectionChange({ ...strong(), enable_push: true }, { statusChecks: checks, push: true }, [], "main") ?? "", /push/); + assert.match(judgeProtectionChange(null, { statusChecks: checks, push: true }, [], "main") ?? "", /push/); + assert.match(judgeProtectionChange(null, { statusChecks: checks, mergeWhitelist: ["agent-ace"] }, [], "main") ?? "", /widens/); + // Another branch is judged as before. + assert.equal(judgeProtectionChange(null, { statusChecks: checks, push: true }, [], "feat/x"), null); +}); + test("gitea_api never acts as another user: sudo, in any case, and any query key it does not send are refused", async () => { for (const path of ["/user?sudo=jschoubben", "/user?SUDO=jschoubben", "/repos/novox/mesh-catalog/issues?state=open&Sudo=jschoubben", "/repos/novox/mesh-catalog/issues?token=abc", "/repos/novox/mesh-catalog/issues?access_token=abc"]) { diff --git a/modules/gitea/test/protection.test.ts b/modules/gitea/test/protection.test.ts index 9dc122de..d75422d7 100644 --- a/modules/gitea/test/protection.test.ts +++ b/modules/gitea/test/protection.test.ts @@ -5,7 +5,7 @@ import { test } from "node:test"; // mesh's merge check before it merges. test("the statuses asked for replace the rule's, and an empty list requires none", async () => { - const { protectionBody } = await import("../protection.ts"); + const { protectionBody } = await import("../dist/protection.js"); assert.deepEqual(protectionBody({ statusChecks: ["mesh/merge-gate", " mesh/repo-check", "", "mesh/merge-gate"] }, false), { enable_status_check: true, status_check_contexts: ["mesh/merge-gate", "mesh/repo-check"] }, "an edited rule keeps its pushes as they are, and each status once"); @@ -13,9 +13,37 @@ test("the statuses asked for replace the rule's, and an empty list requires none }); test("a new rule refuses direct pushes unless asked; an administrator's override only when said", async () => { - const { protectionBody } = await import("../protection.ts"); + const { protectionBody } = await import("../dist/protection.js"); assert.equal(protectionBody({ statusChecks: ["mesh/merge-gate"] }, true).enable_push, false); assert.equal(protectionBody({ statusChecks: ["mesh/merge-gate"], push: true }, true).enable_push, true); assert.equal(protectionBody({ statusChecks: ["mesh/merge-gate"] }, true).block_admin_merge_override, undefined); assert.equal(protectionBody({ statusChecks: ["mesh/merge-gate"], blockAdminOverride: true }, false).block_admin_merge_override, true); }); + +// Issue 948 (novox/hq ADR 0398 §2): main takes no push, and only the tools' account merges. +test("main's body always refuses pushes and lets only the tools' account merge, whatever is asked", async () => { + const { protectionBody } = await import("../dist/protection.js"); + const { TOOLS_ACCOUNT } = await import("../dist/tools-account.js"); + for (const creating of [true, false]) { + for (const want of [{ statusChecks: ["mesh/merge-gate"] }, { statusChecks: [], push: true }, { statusChecks: [], push: false }]) { + const body = protectionBody(want, creating, "main"); + assert.equal(body.enable_push, false, "main takes no push, an edit and a push=true included"); + assert.equal(body.enable_merge_whitelist, true); + assert.deepEqual(body.merge_whitelist_usernames, [TOOLS_ACCOUNT]); + } + } + assert.deepEqual(TOOLS_ACCOUNT, "mesh-tools"); +}); + +test("another branch's body carries no merge whitelist", async () => { + const { protectionBody } = await import("../dist/protection.js"); + const body = protectionBody({ statusChecks: ["mesh/merge-gate"] }, false, "release/1"); + assert.equal(body.enable_merge_whitelist, undefined); + assert.equal(body.merge_whitelist_usernames, undefined); + assert.equal(protectionBody({ statusChecks: [] }, false).enable_merge_whitelist, undefined); +}); + +test("the rule main is put right in the forge's own words: the body a setup sends carries no status checks of its own", async () => { + const { mainRuleBody } = await import("../dist/protection.js"); + assert.deepEqual(mainRuleBody(), { enable_push: false, enable_merge_whitelist: true, merge_whitelist_usernames: ["mesh-tools"] }); +}); diff --git a/modules/gitea/test/token.test.ts b/modules/gitea/test/token.test.ts index d72202f2..1df14e36 100644 --- a/modules/gitea/test/token.test.ts +++ b/modules/gitea/test/token.test.ts @@ -204,6 +204,8 @@ function fakeForge(): Promise { if (req.method === "GET" && p === "/api/v1/orgs/novox/teams") return json(res, 200, [{ id: 7, name: "mesh-tools" }]); if (req.method === "PATCH" && p === "/api/v1/teams/7") return json(res, 200, { id: 7 }); if (req.method === "PUT" && p === "/api/v1/teams/7/members/mesh-tools") return json(res, 204, null); + // Main's rule put right in every repository (issue 948): this forge has none. + if (req.method === "GET" && p === "/api/v1/orgs/novox/repos") return json(res, 200, []); } const adminUser = url.pathname.match(/^\/api\/v1\/admin\/users\/([^/]+)$/); if (adminUser && req.method === "PATCH") { diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index 6941a13a..02c947ad 100644 --- a/modules/gitea/test/tools-account.test.ts +++ b/modules/gitea/test/tools-account.test.ts @@ -31,6 +31,10 @@ function fakeForge() { const tokens: Token[] = []; const teams: Team[] = []; const calls: string[] = []; + // The organisation's repositories, and the rule each has for main, which the administrator's setup puts right + // (issue 948, ADR 0398 §2). + const repos = ["hq", "mesh-catalog"]; + const mainRules = new Map([["hq", { rule_name: "main", enable_status_check: true, status_check_contexts: ["mesh/merge-gate"] }]]); // What the forge does wrong when a test says so: a token mint or delete it fails, a team another process made // between this one's look and its create (gitea answers the create 422). const faults = { mint: false, deleteToken: false, teamRace: false }; @@ -131,6 +135,10 @@ function fakeForge() { if (req.method === "PATCH") { t.units = await read(req); return json(res, 200, { id: t.id }); } if (req.method === "PUT" && m[2]) { t.members.add(decodeURIComponent(m[2])); return json(res, 204, null); } } + if (req.method === "GET" && p === "/orgs/novox/repos") { + if (!isAdmin(me)) return json(res, 403, { message: "must be an owner of the organisation" }); + return json(res, 200, url.searchParams.get("page") === "1" ? repos.map((name) => ({ name })) : []); + } // Repository administration: a site administrator, or a team member whose team administers the repository. m = p.match(/^\/repos\/novox\/([^/]+)\/branches\/(.+)$/); if (m && req.method === "GET") { @@ -140,10 +148,21 @@ function fakeForge() { return json(res, 404, { message: "branch does not exist" }); } // gitea's reqAdmin on the whole group: reading the rules is administration too. - m = p.match(/^\/repos\/novox\/([^/]+)\/branch_protections(?:\/.*)?$/); + m = p.match(/^\/repos\/novox\/([^/]+)\/branch_protections(?:\/(.*))?$/); if (m) { const admins = isAdmin(me) || teams.some((t) => t.members.has(me) && t.units.permission === "admin"); if (!admins) return json(res, 403, { message: "user should be an owner or a collaborator with admin write of a repository" }); + if (req.method === "PATCH" && m[2] === "main") { + const rule = mainRules.get(m[1]); + if (!rule) return json(res, 404, { message: "branch protection does not exist" }); + Object.assign(rule, await read(req)); + return json(res, 200, rule); + } + if (req.method === "POST" && p.endsWith("/branch_protections")) { + const b = await read(req); + mainRules.set(m[1], b); + return json(res, 201, b); + } return json(res, 201, { rule_name: "main" }); } m = p.match(/^\/repos\/novox\/([^/]+)\/statuses\/([0-9a-f]+)$/); @@ -155,11 +174,11 @@ function fakeForge() { if (req.method === "GET" && p === "/repos/search") return json(res, 200, { data: [] }); return json(res, 404, { message: "no such route in the fake" }); }); - return new Promise<{ url: string; users: typeof users; tokens: typeof tokens; teams: typeof teams; calls: string[]; + return new Promise<{ url: string; users: typeof users; tokens: typeof tokens; teams: typeof teams; calls: string[]; mainRules: typeof mainRules; faults: typeof faults; close(): Promise }>((resolve) => { server.listen(0, "127.0.0.1", () => { const { port } = server.address() as { port: number }; - resolve({ url: `http://127.0.0.1:${port}`, users, tokens, teams, calls, faults, + resolve({ url: `http://127.0.0.1:${port}`, users, tokens, teams, calls, faults, mainRules, close: () => new Promise((r) => server.close(() => r())) }); }); }); @@ -330,3 +349,21 @@ test("a team another process made between the look and the create is found, not const team = forge.teams.find((t) => t.name === "mesh-tools"); assert.ok(team?.members.has(TOOLS_ACCOUNT)); }); + +test("setting the tools' account up puts main's rule right in every repository: no push, only the tools' account merges (issue 948)", async () => { + const env = await environment(); + const from = forge.calls.length; + await fresh(env).listRepos(); + const hq = forge.mainRules.get("hq"); + assert.equal(hq.enable_push, false); + assert.equal(hq.enable_merge_whitelist, true); + assert.deepEqual(hq.merge_whitelist_usernames, [TOOLS_ACCOUNT]); + assert.deepEqual(hq.status_check_contexts, ["mesh/merge-gate"], "a rule found keeps the statuses it requires"); + const catalog = forge.mainRules.get("mesh-catalog"); + assert.ok(catalog, "a repository with no rule for main is given one"); + assert.equal(catalog.rule_name, "main"); + assert.equal(catalog.enable_push, false); + assert.deepEqual(catalog.merge_whitelist_usernames, [TOOLS_ACCOUNT]); + assert.ok(forge.calls.slice(from).filter((c) => /branch_protections/.test(c)).every((c) => c.endsWith(` as ${ADMIN}`)), + "only the administrator's login touches a rule: the tools' account is no repository's administrator"); +}); diff --git a/modules/gitea/tools-account.ts b/modules/gitea/tools-account.ts index 03df8e32..087e49d0 100644 --- a/modules/gitea/tools-account.ts +++ b/modules/gitea/tools-account.ts @@ -41,6 +41,8 @@ export interface ToolsForge { ensureUser(username: string, password: string, email: string, plain?: boolean): Promise; ensureTeamWith(org: string, team: string, units: Record): Promise; addUserToTeam(teamId: number, username: string): Promise; + /** Main's rule in every repository of the organisation: no push, only the tools' account merges (ADR 0398 §2). */ + ensureMainMerges?(org: string): Promise; } /** Make the tools' account, plain, a member of its team on every repository of the organisation. Its password is @@ -49,4 +51,7 @@ export async function setUpToolsAccount(forge: ToolsForge, account = TOOLS_ACCOU await forge.ensureUser(account, password(), `${account}@noreply.localhost`, true); const team = await forge.ensureTeamWith(TOOLS_ORG, TOOLS_TEAM, { name: TOOLS_TEAM, ...TOOLS_TEAM_UNITS }); await forge.addUserToTeam(team, account); + // Main takes no push and only this account merges into it (novox/hq ADR 0398 §2, issue 948): put right in every + // repository now, by the administrator's login, which the account itself is not. + await forge.ensureMainMerges?.(TOOLS_ORG); } diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index 27a8c610..6914c412 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -32,6 +32,7 @@ export function summarised(p: BranchProtection) { required_statuses: p.enable_status_check ? (p.status_check_contexts ?? []) : [], required_approvals: p.required_approvals ?? 0, admin_may_override: !(p.block_admin_merge_override ?? false), + merge_whitelist: p.enable_merge_whitelist ? (p.merge_whitelist_usernames ?? []) : null, }; } @@ -411,7 +412,7 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini }, { name: "gitea_branch_protection_set", - description: "Only ever stronger (novox/hq ADR 0266): a change that drops a required status, lets a person push directly or lets an administrator merge past a status is refused — that is the operator's, at the forge. Make a branch require commit statuses before a pull request merges into it — the mesh's merge check sets `mesh/merge-gate` (the module graph's gate) and `mesh/repo-check` (the repository's own tests). Edits the branch's rule (its required statuses replaced by these, everything else kept unless given) or creates one, which refuses direct pushes unless push=true. Answers the rule before and after.", + description: "Only ever stronger (novox/hq ADR 0266): a change that drops a required status, lets a person push directly or lets an administrator merge past a status is refused, and on main so is a change that drops the merge whitelist (only mesh-tools merges there) or widens it (novox/hq ADR 0398 §2, issue 948) — that is the operator's, at the forge. Make a branch require commit statuses before a pull request merges into it — the mesh's merge check sets `mesh/merge-gate` (the module graph's gate) and `mesh/repo-check` (the repository's own tests). Edits the branch's rule (its required statuses replaced by these, everything else kept unless given) or creates one, which refuses direct pushes unless push=true. Answers the rule before and after.", input: { owner: { type: "string", description: "the repository owner" }, repo: { type: "string", description: "the repository name" }, @@ -433,7 +434,7 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini }; const covering = before ? [] : (await gitea.branchProtections(owner, repo)) .filter((r) => ruleCovers(String(r.rule_name ?? r.branch_name ?? ""), branch)); - const weakens = judgeProtectionChange(before, want, covering); + const weakens = judgeProtectionChange(before, want, covering, branch); if (weakens) throw new Refused(weakens); const { created, rule } = await gitea.setBranchProtection(owner, repo, branch, want); return { branch, created, before: before ? summarised(before) : null, after: summarised(rule) };