Let gitea_api send only the query keys it needs, never sudo, and never approve a pull request
The confirmation review of 2026-10-09 measured GET /user?sudo=… answer the operator's own account: the query went to the forge unjudged, and the tools' token is the site administrator's, so any caller could act as any user. gitea_api now sends only allow-listed query keys and refuses sudo in any case, and a review that approves or requests changes is the operator's.
This commit is contained in:
jochen
@@ -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<string> = 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<string, unknown>)) {
|
||||
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<string, unknown> | 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 ` +
|
||||
|
||||
@@ -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"]);
|
||||
});
|
||||
|
||||
@@ -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) } : {}),
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user