From c85472285d7156ee0f90d8613a0409ad507fd7ba Mon Sep 17 00:00:00 2001
From: jochen <jschoubben@gmail.com>
Date: Mon, 26 Oct 2026 18:23:42 +0100
Subject: [PATCH] gitea: main's rule read back must say what was written, since
 the forge silently drops a whitelist login that cannot write (issue 1945,
 review round 1)

---
 modules/gitea/client.ts                  |  7 +++++++
 modules/gitea/test/tools-account.test.ts | 16 ++++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts
index f63a4456..5e9f102e 100644
--- a/modules/gitea/client.ts
+++ b/modules/gitea/client.ts
@@ -1102,6 +1102,13 @@ export class GiteaAdmin {
     }
     const after = (await this.branchProtections(owner, repo)).find((r) => (r.rule_name ?? r.branch_name) === MAIN_BRANCH);
     if (!after) throw new Error(`main's rule of ${owner}/${repo} was written, but the forge does not list it`);
+    // The forge answers a write it did not keep: it drops a whitelist login that cannot write the repository, silently.
+    const listed = after.merge_whitelist_usernames ?? [];
+    const wrong: string[] = [];
+    if (after.enable_push !== false) wrong.push(`push is ${after.enable_push === true ? "allowed" : "not refused"}`);
+    if (after.enable_merge_whitelist !== true || !listed.includes(mergers[0])) wrong.push(`the merge whitelist [${listed.join(", ")}] leaves out ${mergers[0]}`);
+    if (after.enable_status_check !== false) wrong.push("a status is still required");
+    if (wrong.length > 0) throw new Error(`main's rule of ${owner}/${repo} was written, but the forge holds otherwise: ${wrong.join("; ")}`);
     return { created: !own, rule: after };
   }
 
diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts
index 23a30d67..2f1cd6aa 100644
--- a/modules/gitea/test/tools-account.test.ts
+++ b/modules/gitea/test/tools-account.test.ts
@@ -176,12 +176,15 @@ function fakeForge() {
       const repo = forgeRepos.find((r) => r.full === key);
       if (!repo || !["admin", "owner"].includes(permissionOf(me, repo))) return json(res, 403, { message: "user should be an owner or a collaborator with admin write of a repository" });
       if (req.method === "GET" && m[3] === undefined) return json(res, 200, userRules.has(key) ? [userRules.get(key)] : []);
+      // As gitea does: a whitelist login that cannot write the repository is dropped, and the write still answered.
+      const keep = (b: any) => (Array.isArray(b.merge_whitelist_usernames)
+        ? { ...b, merge_whitelist_usernames: b.merge_whitelist_usernames.filter((u: string) => ["write", "admin", "owner"].includes(permissionOf(u, repo))) } : b);
       if (req.method === "PATCH" && m[3] === "main") {
         if (!userRules.has(key)) return json(res, 404, { message: "branch protection does not exist" });
-        Object.assign(userRules.get(key), await read(req));
+        Object.assign(userRules.get(key), keep(await read(req)));
         return json(res, 200, userRules.get(key));
       }
-      if (req.method === "POST") { const b = await read(req); userRules.set(key, b); return json(res, 201, b); }
+      if (req.method === "POST") { const b = keep(await read(req)); userRules.set(key, b); return json(res, 201, b); }
     }
     // gitea's reqAdmin on the whole group: reading the rules is administration too.
     m = p.match(/^\/repos\/novox\/([^/]+)\/branch_protections(?:\/(.*))?$/);
@@ -664,6 +667,15 @@ test("the merging account is given write on each repository the indexer's module
     [["jschoubben/snake", false, false, 2, true]]);
   assert.equal(repo("jschoubben/snake").collaborators.get(TOOLS_ACCOUNT), "admin");
   assert.deepEqual(((await unwritableTool.run({})) as any).unwritable, []);
+  // The forge drops a whitelist login that cannot write and still answers 200: main's rule is read back and that fails loud.
+  const admin = GiteaAdmin.fromEnv(env);
+  await assert.rejects(admin.ensureMainRule("jschoubben", "dotfiles"),
+    /main's rule of jschoubben\/dotfiles was written, but the forge holds otherwise: the merge whitelist \[\] leaves out mesh-tools/);
+  userRules.get("jschoubben/snake").enable_status_check = true;
+  userRules.get("jschoubben/snake").enable_push = true;
+  const held = await admin.ensureMainRule("jschoubben", "snake");
+  assert.deepEqual([held.rule.enable_push, held.rule.enable_status_check, held.rule.merge_whitelist_usernames], [false, false, [TOOLS_ACCOUNT]]);
+  userRules.delete("jschoubben/dotfiles");
   // A module gone from the indexer leaves its repository out of the delivered list.
   await writes.take({ key: "snake", op: "delete" });
   assert.deepEqual(writes.delivered(), ["jschoubben/car-hunter", "novox/hq"]);