diff --git a/modules/gitea/guard.ts b/modules/gitea/guard.ts index 7fa757f3..4b451f64 100644 --- a/modules/gitea/guard.ts +++ b/modules/gitea/guard.ts @@ -83,12 +83,45 @@ export function routedPath(raw: string): string { return path; } +/** + * The query keys `gitea_api` sends: what the forge's listing and reading endpoints take. Any other key is refused, + * `sudo` above all — the forge lets a site administrator's token act as any user named by it, and the tools' token is + * one (the confirmation review of 2026-10-09 read the operator's own account through `GET /user?sudo=…`). The token + * keys are refused too: authentication is the client's, never the caller's. + */ +export const QUERY_KEYS: ReadonlySet = new Set([ + "limit", "page", "state", "type", "q", "sort", "order", "since", "before", "ref", "sha", "path", "labels", + "milestones", "keyword", "stat", "verification", "files", "not", "mode", "include_desc", "archived", "private", + "exclusive", "uid", "priority_repo_id", "created_by", "assigned_by", "mentioned_by", "whitespace", "binary", +]); + +/** The query a call sends: only allow-listed keys, each as given; refused when any other key is there. */ +export function sentQuery(raw: string): string { + const at = String(raw ?? "").indexOf("?"); + if (at < 0) return ""; + const query = String(raw).slice(at + 1).split("#")[0]; + const params = new URLSearchParams(query); + const kept = new URLSearchParams(); + for (const [key, value] of params) { + if (key.toLowerCase() === "sudo") { + throw new Refused(`the query key ${key} is refused: it would act as another user of the forge (novox/hq ADR 0266). Nothing was sent`); + } + if (!QUERY_KEYS.has(key)) { + throw new Refused(`the query key ${key} is not one gitea_api sends: it sends ${[...QUERY_KEYS].join(", ")} (novox/hq ADR 0266). Nothing was sent`); + } + kept.append(key, value); + } + const out = kept.toString(); + return out ? "?" + out : ""; +} + /** What `gitea_api` may do with this call, judged on the call alone (novox/hq ADR 0266). */ export function judgeApiCall(method: string, rawPath: string, body: unknown): ApiJudgement { const m = String(method || "GET").toUpperCase(); let path: string; try { path = routedPath(rawPath); + sentQuery(rawPath); } catch (err) { return { refused: (err as Error).message }; } @@ -110,6 +143,16 @@ export function judgeApiCall(method: string, rawPath: string, body: unknown): Ap "base" in (body as Record)) { return { refused: `${m} ${path} changing a pull request's base is refused (novox/hq ADR 0266): its checks were judged against the base it had. Open a new pull request. Nothing was sent` }; } + // An approval, or a request for changes, is a reviewer's word on a merge: the operator's, never a tool's — the + // tools' token is the forge's administrator, and its approval would count as anyone's (novox/hq ADR 0266). + if (m === "POST" && /^\/repos\/[^/]+\/[^/]+\/pulls\/\d+\/reviews(\/\d+)?$/.test(path)) { + const event = String((body as Record | null)?.event ?? "").trim().toUpperCase(); + if (event !== "" && event !== "COMMENT" && event !== "PENDING") { + return { refused: `${m} ${path} with the event ${event} is refused: approving a pull request, or requesting ` + + `changes on it, is the operator's at the forge as themselves, never a tool's (novox/hq ADR 0266). A review ` + + `that only comments is sent. Nothing was sent` }; + } + } if (AUTHOR_WRITES.some((w) => w.methods.includes(m) && w.path.test(path))) return { allowed: true }; return { refused: `${m} ${path} is not a write gitea_api makes (novox/hq ADR 0266): it writes only what an author ` + diff --git a/modules/gitea/test/guard.test.ts b/modules/gitea/test/guard.test.ts index 02da7e17..3b7d1f7d 100644 --- a/modules/gitea/test/guard.test.ts +++ b/modules/gitea/test/guard.test.ts @@ -17,6 +17,7 @@ interface Sent { method: string; path: string; body: any } const state = { sent: [] as Sent[], + asked: [] as string[], statuses: [] as { context: string; state: string }[], rules: [] as any[], lastCommit: "2026-01-01T00:00:00Z", @@ -26,6 +27,7 @@ const protectedMain = () => ({ rule_name: "main", enable_push: false, enable_sta beforeEach(() => { state.sent = []; + state.asked = []; state.statuses = [{ context: "mesh/merge-gate", state: "success" }, { context: "mesh/repo-check", state: "success" }]; state.rules = [protectedMain()]; }); @@ -46,6 +48,7 @@ const server = createServer(async (req, res) => { const path = url.pathname.replace(/^\/api\/v1/, ""); const method = req.method ?? "GET"; const body = await read(req); + state.asked.push(`${method} ${path}${url.search}`); if (method !== "GET") state.sent.push({ method, path, body }); const repo = "/repos/novox/mesh-catalog"; if (method === "GET" && path === repo) return json(res, 200, { full_name: "novox/mesh-catalog", default_branch: "main" }); @@ -159,7 +162,7 @@ test("gitea_delete_repo deletes only the tools' own user's repository, unguarded }); test("gitea_api sends the path it judged, and refuses moving a pull request's base", async () => { - await tool("gitea_api").run({ path: "/api/v1/repos/novox/mesh-catalog/issues/3/comments/?x=1", method: "POST", body: { body: "hi" } }); + await tool("gitea_api").run({ path: "/api/v1/repos/novox/mesh-catalog/issues/3/comments/?page=1", method: "POST", body: { body: "hi" } }); assert.deepEqual(writes(), ["POST /repos/novox/mesh-catalog/issues/3/comments"]); await assert.rejects(tool("gitea_api").run({ path: "/repos/novox/mesh-catalog/pulls/7", method: "PATCH", body: { base: "unguarded" } }), /changing a pull request's base/); @@ -206,3 +209,27 @@ test("the judges, without a forge", () => { assert.equal(judgeProtectionChange(null, { statusChecks: [] }), null); assert.equal(judgeProtectionChange(protectedMain(), { statusChecks: ["mesh/repo-check", "mesh/merge-gate"] }), 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"]) { + await assert.rejects(tool("gitea_api").run({ path }), /query key/, path); + } + await assert.rejects(tool("gitea_api").run({ path: "/repos/novox/mesh-catalog/issues/3/comments?sudo=jschoubben", + method: "POST", body: { body: "as the operator" } }), /query key sudo/); + assert.deepEqual(state.asked, []); + await tool("gitea_api").run({ path: "/repos/novox/mesh-catalog/issues?state=open&limit=5" }); + assert.deepEqual(state.asked, ["GET /repos/novox/mesh-catalog/issues?state=open&limit=5"]); +}); + +test("gitea_api never approves a pull request nor requests changes: those are the operator's", async () => { + for (const [path, event] of [["/repos/novox/mesh-catalog/pulls/7/reviews", "APPROVE"], + ["/repos/novox/mesh-catalog/pulls/7/reviews", "approve"], ["/repos/novox/mesh-catalog/pulls/7/reviews", "REQUEST_CHANGES"], + ["/repos/novox/mesh-catalog/pulls/7/reviews/5", "APPROVE"]]) { + await assert.rejects(tool("gitea_api").run({ path, method: "POST", body: { event, body: "lgtm" } }), + /operator's at the forge/, `${path} ${event}`); + } + assert.deepEqual(writes(), []); + await tool("gitea_api").run({ path: "/repos/novox/mesh-catalog/pulls/7/reviews", method: "POST", body: { event: "COMMENT", body: "a note" } }); + assert.deepEqual(writes(), ["POST /repos/novox/mesh-catalog/pulls/7/reviews"]); +}); diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index 91357c7f..7f96734d 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -14,7 +14,7 @@ import { GiteaClient, type BranchProtection } from "../client.js"; import { appendNote, deliveryStatus, run, viewBody, viewComment } from "../delivery.js"; import { containedFrom, isCommit, mergeableUnder, newestByContext } from "../checks.js"; import { - MERGE_METHODS, Refused, checkName, judgeApiCall, judgeContentsWrite, judgeDelete, judgeProtectionChange, routedPath, + MERGE_METHODS, Refused, checkName, judgeApiCall, judgeContentsWrite, judgeDelete, judgeProtectionChange, routedPath, sentQuery, ruleCovers, } from "../guard.js"; @@ -643,10 +643,9 @@ function allGiteaTools(gitea: GiteaClient): ToolDefinition[] { await gitea.branchProtections(owner, repo)); if (refused) throw new Refused(refused); } - // The path judged is the path sent: normalised, its query kept. + // The path judged is the path sent: normalised, with only the allow-listed query keys (guard.ts). const raw = String(args.path ?? ""); - const query = raw.includes("?") ? raw.slice(raw.indexOf("?")).split("#")[0] : ""; - const result = await gitea.api(routedPath(raw) + query, { + const result = await gitea.api(routedPath(raw) + sentQuery(raw), { method, ...(args.body ? { body: JSON.stringify(args.body) } : {}), });