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)

2026-10-26 18:23:42 +01:00
parent f0fbe193fa
commit c85472285d
2 changed files with 21 additions and 2 deletions
@@ -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 };
  }

@@ -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"]);