Put no caller's text in a forge path unjudged: every segment checked, every sent path checked again
mesh/merge-gate pass: builds gitea → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery superseded: a newer head of the same pull request

The confirmation review of 2026-10-09 found the dedicated tools building paths
from caller text: a branch named .. became DELETE /repos/o/r/ (the repository),
a file path ../../../../version reached /api/v1/version, and a sha with
?sudo= set a status as any user. Each caller-supplied segment now goes through
one helper refusing empty, . and .. and / ? # %; a sha must be a commit; and
the client judges every path once more as it sends it: no dot or empty
segment, encoded or not, and only the allow-listed query keys.
This commit is contained in:
jochen
2026-10-09 13:35:15 +02:00
parent 213950e334
commit 5eab586116
5 changed files with 102 additions and 12 deletions
+18 -8
View File
@@ -6,6 +6,15 @@
import { readFileSync } from "node:fs";
import { ConfiguredToken, MintedToken, type TokenSource } from "./token.js";
import { protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js";
import { Refused, checkSentPath, pathSegment, pathSegments } from "./guard.js";
import { isCommit } from "./checks.js";
/** A commit, by its sha or the start of one, as it goes in a path: anything else is refused before it is sent. */
function commitSegment(sha: unknown): string {
const v = String(sha ?? "");
if (!isCommit(v)) throw new Refused(`${JSON.stringify(sha)} is not a commit. Nothing was sent`);
return v;
}
export type { BranchProtection, ProtectionWanted } from "./protection.js";
/** A repository, trimmed to what the mesh cares about. */
@@ -135,6 +144,7 @@ export class GiteaClient {
}
private send(path: string, options: RequestInit, token: string): Promise<Response> {
checkSentPath(path);
return fetch(`${this.baseUrl}/api/v1${path}`, {
...options,
headers: {
@@ -337,12 +347,12 @@ export class GiteaClient {
/** Set a commit's status — what a pull request whose head it is shows beside it (novox/hq to-be 45 §9).
* The forge keeps one per context, the newest, so setting it again replaces it. */
async setCommitStatus(owner: string, repo: string, sha: string, status: CommitStatus): Promise<void> {
await this.request(`/repos/${owner}/${repo}/statuses/${sha}`, { method: "POST", body: JSON.stringify(status) });
await this.request(`/repos/${owner}/${repo}/statuses/${commitSegment(sha)}`, { method: "POST", body: JSON.stringify(status) });
}
/** A commit's statuses by context, the newest of each: what a merge's head was checked as (novox/hq ADR 0239). */
async commitStatuses(owner: string, repo: string, sha: string): Promise<Record<string, string>> {
const raw = await this.request<any>(`/repos/${owner}/${repo}/commits/${sha}/status`);
const raw = await this.request<any>(`/repos/${owner}/${repo}/commits/${commitSegment(sha)}/status`);
const out: Record<string, string> = {};
for (const s of raw?.statuses ?? []) if (s?.context && !out[s.context]) out[s.context] = String(s.status ?? s.state ?? "");
return out;
@@ -351,14 +361,14 @@ export class GiteaClient {
/** A commit's statuses whole, as the forge combines them for a ref (a sha, or the start of one): the commit
* it resolved to and every status with its description, link, times and setter (novox/hq ADR 0239). */
async combinedStatus(owner: string, repo: string, ref: string): Promise<{ sha: string; state: string; statuses: unknown[] }> {
const raw = await this.request<any>(`/repos/${owner}/${repo}/commits/${encodeURIComponent(ref)}/status?limit=50`);
const raw = await this.request<any>(`/repos/${owner}/${repo}/commits/${pathSegment("commit or branch", ref)}/status?limit=50`);
return { sha: String(raw?.sha ?? ""), state: String(raw?.state ?? ""), statuses: Array.isArray(raw?.statuses) ? raw.statuses : [] };
}
/** The forge's compare of two commits, `base...head`: the commits on head that base lacks, and how many
* (novox/hq ADR 0270). Reads only. */
async compare(owner: string, repo: string, base: string, head: string): Promise<unknown> {
return this.request<unknown>(`/repos/${owner}/${repo}/compare/${encodeURIComponent(base)}...${encodeURIComponent(head)}`);
return this.request<unknown>(`/repos/${owner}/${repo}/compare/${commitSegment(base)}...${commitSegment(head)}`);
}
/** Replace a comment's body: the delivery's view, kept current in place (novox/hq ADR 0239). */
@@ -386,7 +396,7 @@ export class GiteaClient {
const had = await this.branchProtection(owner, repo, branch);
const body = protectionBody(want, !had);
if (had) {
const rule = await this.request<BranchProtection>(`/repos/${owner}/${repo}/branch_protections/${encodeURIComponent(branch)}`,
const rule = await this.request<BranchProtection>(`/repos/${owner}/${repo}/branch_protections/${pathSegments("branch", branch)}`,
{ method: "PATCH", body: JSON.stringify(body) });
return { created: false, rule };
}
@@ -440,7 +450,7 @@ export class GiteaClient {
/** One file's contents at a ref (default the repository's default branch), decoded. */
async getFile(owner: string, repo: string, path: string, ref?: string): Promise<{ path: string; ref?: string; sha: string; size: number; content: string }> {
const qs = ref ? `?ref=${encodeURIComponent(ref)}` : "";
const f = await this.request<any>(`/repos/${owner}/${repo}/contents/${path.split("/").map(encodeURIComponent).join("/")}${qs}`);
const f = await this.request<any>(`/repos/${owner}/${repo}/contents/${pathSegments("file path", path.replace(/^\/+|\/+$/g, ""))}${qs}`);
if (!f || f.type !== "file") throw new Error(`Gitea API: ${path} is not a file`);
const content = f.encoding === "base64" ? Buffer.from(String(f.content ?? ""), "base64").toString("utf8") : String(f.content ?? "");
return { path, ref, sha: String(f.sha ?? ""), size: Number(f.size ?? content.length), content };
@@ -452,7 +462,7 @@ export class GiteaClient {
}
async deleteBranch(owner: string, repo: string, branch: string): Promise<void> {
await this.request(`/repos/${owner}/${repo}/branches/${encodeURIComponent(branch)}`, { method: "DELETE" });
await this.request(`/repos/${owner}/${repo}/branches/${pathSegments("branch", branch)}`, { method: "DELETE" });
}
/** A request whose answer is text, not JSON — a diff. Same token handling as request(). */
@@ -756,5 +766,5 @@ export function directoriesAbove(paths: string[]): string[] {
/** A repository path for the forge's URL: each segment escaped, the slashes kept. */
function encodePath(path: string): string {
return path.split("/").map(encodeURIComponent).join("/");
return pathSegments("file path", path);
}
+43
View File
@@ -271,3 +271,46 @@ export function checkName(what: string, value: unknown): string {
}
return v;
}
/**
* One caller-supplied path segment, encoded: refused when empty, `.` or `..`, or holding `/`, `?`, `#` or `%` — any
* of which, encoded or not, could make the request another path or carry another query (the confirmation review of
* 2026-10-09: a branch named `..` became DELETE /repos/o/r/, the repository itself). Every value a caller gives that
* lands in a path goes through this, or through pathSegments.
*/
export function pathSegment(what: string, value: unknown): string {
const v = String(value ?? "");
if (v === "" || v === "." || v === ".." || /[/?#%\\]/.test(v) || /[\u0000-\u001f]/.test(v)) {
throw new Refused(`${JSON.stringify(value)} is not a ${what} this tool puts in a path. Nothing was sent`);
}
return encodeURIComponent(v);
}
/** A caller-supplied value that is several segments — a branch like feat/x, a file's path — each judged as one. */
export function pathSegments(what: string, value: unknown): string {
return String(value ?? "").split("/").map((part) => pathSegment(what, part)).join("/");
}
/**
* Every path the client sends, judged once more where it is sent: no empty, `.` or `..` segment, encoded or not, and
* only the query keys the tools send (sentQuery). What a caller's text could still make of a path built elsewhere
* stops here, before the forge.
*/
export function checkSentPath(path: string): void {
const at = path.search(/[?#]/);
const route = at < 0 ? path : path.slice(0, at);
if (path.includes("#")) throw new Refused(`${path}: a path with a fragment is not sent`);
const segments = route.replace(/^\/+/, "").split("/");
for (const s of segments) {
let decoded: string;
try {
decoded = decodeURIComponent(s);
} catch {
throw new Refused(`${path}: a segment that does not decode is not sent`);
}
if (decoded === "" || decoded === "." || decoded === ".." || decoded.includes("?")) {
throw new Refused(`${path}: an empty or dot segment is not sent. Nothing was sent`);
}
}
if (at >= 0) sentQuery(path.slice(at));
}
+32
View File
@@ -233,3 +233,35 @@ test("gitea_api never approves a pull request nor requests changes: those are th
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"]);
});
test("no tool's caller text becomes another path or another user: `..` and `?sudo=` are refused before the forge", async () => {
const repo = { owner: "novox", repo: "mesh-catalog" };
const cases: Array<[string, Record<string, unknown>]> = [
["gitea_delete_branch", { ...repo, branch: ".." }],
["gitea_delete_branch", { ...repo, branch: "feat/../.." }],
["gitea_delete_branch", { ...repo, branch: "x?sudo=jschoubben" }],
["gitea_delete_branch", { ...repo, branch: "%2e%2e" }],
["gitea_get_file", { ...repo, path: "../../../../version" }],
["gitea_get_file", { ...repo, path: "README.md?sudo=jschoubben" }],
["gitea_commit_status", { ...repo, sha: "abc1234?sudo=jschoubben", context: "mesh/delivery", state: "success" }],
["gitea_commit_status", { ...repo, sha: "../../../admin/users", context: "mesh/delivery", state: "success" }],
["gitea_commit_statuses", { ...repo, sha: ".." }],
["gitea_commit_statuses", { ...repo, sha: "main?sudo=jschoubben" }],
["gitea_branch_protection_set", { ...repo, branch: "..", status_checks: "mesh/merge-gate" }],
["gitea_branch_protection_set", { ...repo, branch: "main?sudo=jschoubben", status_checks: "mesh/merge-gate" }],
["gitea_contains", { ...repo, commit: "..", in: "abc1234" }],
];
for (const [name, args] of cases) {
state.asked = [];
await assert.rejects(tool(name).run(args), (err: unknown) => err instanceof Error, `${name} ${JSON.stringify(args)}`);
// Nothing that is not a read of the repository's own rules reached the forge.
assert.deepEqual(state.asked.filter((a) => !a.startsWith("GET /repos/novox/mesh-catalog/branch_protections")), [],
`${name} ${JSON.stringify(args)} reached the forge`);
}
// What is ordinary still goes: a branch with a slash, a file in a directory.
state.asked = [];
await tool("gitea_delete_branch").run({ ...repo, branch: "feat/x" });
await tool("gitea_get_file").run({ ...repo, path: "modules/gitea/module.json" }).catch(() => undefined);
assert.deepEqual(state.asked, ["DELETE /repos/novox/mesh-catalog/branches/feat/x",
"GET /repos/novox/mesh-catalog/contents/modules/gitea/module.json"]);
});
+6 -3
View File
@@ -119,9 +119,12 @@ test("a commit status is set on the commit, under the merge check's context", as
await new Promise<void>((r) => server.listen(0, r));
const port = (server.address() as any).port;
const client = new GiteaClient(`http://127.0.0.1:${port}`, "t");
await client.setCommitStatus("novox", "mesh-catalog", "abc123", { state: "failure", context: "mesh/merge-gate", description: "x" });
server.close();
assert.equal(seen!.path, "POST /api/v1/repos/novox/mesh-catalog/statuses/abc123");
try {
await client.setCommitStatus("novox", "mesh-catalog", "abc1234", { state: "failure", context: "mesh/merge-gate", description: "x" });
} finally {
server.close();
}
assert.equal(seen!.path, "POST /api/v1/repos/novox/mesh-catalog/statuses/abc1234");
assert.equal(seen!.body.state, "failure");
assert.equal(seen!.body.context, "mesh/merge-gate");
});
+3 -1
View File
@@ -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, sentQuery,
MERGE_METHODS, Refused, checkName, judgeApiCall, judgeContentsWrite, judgeDelete, judgeProtectionChange, pathSegments, routedPath, sentQuery,
ruleCovers,
} from "../guard.js";
@@ -461,6 +461,8 @@ function allGiteaTools(gitea: GiteaClient): ToolDefinition[] {
run: async (args) => {
const owner = String(args.owner), repo = String(args.repo), branch = String(args.branch ?? "").trim();
if (!branch) throw new Error("name the branch to protect");
pathSegments("branch", branch); // a rule's name is a branch's, never a path's
const before = await gitea.branchProtection(owner, repo, branch);
const want = {
statusChecks: String(args.status_checks ?? "").split(","),