From ee2160d00c22c6a4a74bbabe1239228ee1e5d834 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 14 Oct 2026 22:45:16 +0200 Subject: [PATCH] Drop every mesh-tools token by id when the forge holds two, and stop re-minting on every call (hq issue 739) On 2026-10-14 from 22:28 local the forge held two tokens named mesh-tools. gitea refuses a delete by such a name with 422 "multiple matches", so every gitea tool call re-entered the mint, made two basic-auth calls that hash the admin password, and failed: several times a minute, on a control-node that was already overloaded, and mesh-delivery could read neither open pulls nor views. The watcher and the tools run as separate bundles, so the in-process sharing of one token source does not cover them; two mints racing leave two tokens by the name, because the forge's check that a name is unused is not atomic. A 422 on the delete by name now lists the account's tokens and drops each one carrying the name by its id, then mints. A failed mint is not asked of the forge again for 30 s: the same error is answered without a call. --- modules/gitea/test/token.test.ts | 85 +++++++++++++++++++++++++++++--- modules/gitea/token.ts | 64 ++++++++++++++++++++++-- 2 files changed, 137 insertions(+), 12 deletions(-) diff --git a/modules/gitea/test/token.test.ts b/modules/gitea/test/token.test.ts index 750c8122..175fe1be 100644 --- a/modules/gitea/test/token.test.ts +++ b/modules/gitea/test/token.test.ts @@ -15,7 +15,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { collectTools } from "@novox/mesh-sdk/tools"; -import { AdminRefused, MintedToken, TOKEN_SCOPES } from "../dist/token.js"; +import { AdminRefused, MINT_RETRY_AFTER_MS, MintedToken, TOKEN_SCOPES } from "../dist/token.js"; import { GiteaClient } from "../dist/client.js"; import { getGiteaTools } from "../dist/tools/index.js"; @@ -29,6 +29,9 @@ interface Forge { mints: number; lastScopes: string[] | null; tokens: Map; + /** Tokens beyond the one per name `tokens` holds, by id: what two mints racing leave (novox/hq issue 739). */ + twins: Map; + tokenCalls: number; scopesOf: Map; admins: Map; pullState: string; @@ -42,6 +45,9 @@ function fakeForge(): Promise { mints: 0, lastScopes: null as string[] | null, tokens: new Map(), // name -> value + twins: new Map(), + idOf: new Map(), // value -> id + tokenCalls: 0, scopesOf: new Map(), // value -> scopes, so a route can enforce them like gitea does admins: new Map([[ADMIN, PASSWORD]]), pullState: "open", @@ -73,21 +79,41 @@ function fakeForge(): Promise { const tokens = url.pathname.match(/^\/api\/v1\/users\/([^/]+)\/tokens(?:\/([^/]+))?$/); if (tokens) { const user = basic(req); + forge.tokenCalls++; if (user === null || user !== decodeURIComponent(tokens[1])) return json(res, 401, { message: "auth required" }); + const named = (name: string): number => + (forge.tokens.has(name) ? 1 : 0) + [...forge.twins.values()].filter((t) => t.name === name).length; if (req.method === "POST") { const { name, scopes } = await body(req); - if (forge.tokens.has(name)) return json(res, 400, { message: "token name has already been used" }); + if (named(name) > 0) return json(res, 400, { message: "token name has already been used" }); forge.mints++; forge.lastScopes = scopes; const sha1 = `minted-${forge.mints}-${Math.random().toString(36).slice(2)}`; forge.tokens.set(name, sha1); + forge.idOf.set(sha1, forge.mints); forge.scopesOf.set(sha1, scopes); return json(res, 201, { id: forge.mints, name, sha1, scopes, token_last_eight: sha1.slice(-8) }); } + if (req.method === "GET" && !tokens[2]) { + const page = Number(url.searchParams.get("page") ?? "1"); + const all = [...[...forge.tokens].map(([name, v]) => ({ id: forge.idOf.get(v) ?? 0, name })), + ...[...forge.twins].map(([id, t]) => ({ id, name: t.name }))]; + return json(res, 200, page === 1 ? all : []); + } if (req.method === "DELETE" && tokens[2]) { - const name = decodeURIComponent(tokens[2]); - if (!forge.tokens.has(name)) return json(res, 404, { message: "token not found" }); - forge.tokens.delete(name); + const key = decodeURIComponent(tokens[2]); + // gitea: "identified by ID and if not available by name" + if (/^\d+$/.test(key)) { + const id = Number(key); + if (forge.twins.delete(id)) return json(res, 204, null); + const hit = [...forge.tokens].find(([, v]) => forge.idOf.get(v) === id); + if (!hit) return json(res, 404, { message: "token not found" }); + forge.tokens.delete(hit[0]); + return json(res, 204, null); + } + if (named(key) > 1) return json(res, 422, { message: `multiple matches for token name '${key}'` }); + if (!forge.tokens.has(key)) return json(res, 404, { message: "token not found" }); + forge.tokens.delete(key); return json(res, 204, null); } return json(res, 405, { message: "method not allowed" }); @@ -180,7 +206,9 @@ function fakeForge(): Promise { get pullState() { return forge.pullState; }, get pullTitle() { return forge.pullTitle; }, get branchDeleted() { return forge.branchDeleted; }, + get tokenCalls() { return forge.tokenCalls; }, tokens: forge.tokens, + twins: forge.twins, scopesOf: forge.scopesOf, admins: forge.admins, close: () => new Promise((r) => server.close(() => r())), @@ -209,8 +237,9 @@ async function delivered(forge: Forge): Promise<{ env: NodeJS.ProcessEnv; file: } /** A client as a fresh process would build it: a new source over the kept file, its log captured. */ -function minted(env: NodeJS.ProcessEnv, logs: string[]): GiteaClient { +function minted(env: NodeJS.ProcessEnv, logs: string[], now?: () => number): GiteaClient { const source = new MintedToken({ + now, url: env.MESH_GITEA_URL!, admin: env.MESH_GITEA_ADMIN_USER!, passwordFile: env.MESH_GITEA_ADMIN_PASSWORD_FILE!, @@ -325,6 +354,44 @@ test("the kept file is gone but the forge still holds a token by that name: repl assert.ok(logs.some((l) => l.includes('already holds a token named "mesh-tools"')), logs.join("\n")); }); +test("two tokens carry the name (two mints raced): each is dropped by its id and one is minted, not a 422 for good (issue 739)", async () => { + const { env, logs } = await delivered(forge); + await minted(env, logs).listRepos(); + // What two processes minting at once leave: a second token by the same name, whose value nobody kept. + // Two tokens by the name, neither of them the one kept. + forge.tokens.clear(); + forge.twins.set(9001, { name: "mesh-tools", sha1: "raced-first" }); + forge.twins.set(9002, { name: "mesh-tools", sha1: "raced-second" }); + + const repos = await minted(env, logs).listRepos(); + + assert.equal(repos.length, 1); + assert.equal(forge.twins.size, 0); + assert.equal([...forge.tokens.keys()].filter((n) => n === "mesh-tools").length, 1); + assert.ok(logs.some((l) => l.includes('holds 2 tokens named "mesh-tools"')), logs.join("\n")); +}); + +test("a mint that failed is not asked of the forge again within the retry window: the same error, no basic-auth call (issue 739)", async () => { + const { env, logs } = await delivered(forge); + forge.admins.delete(ADMIN); + try { + let clock = 5_000_000; + const client = minted(env, logs, () => clock); + await assert.rejects(client.listRepos(), AdminRefused); + const calls = forge.tokenCalls; + + await assert.rejects(client.listRepos(), AdminRefused); + await assert.rejects(client.listRepos(), AdminRefused); + assert.equal(forge.tokenCalls, calls); + + clock += MINT_RETRY_AFTER_MS; + await assert.rejects(client.listRepos(), AdminRefused); + assert.equal(forge.tokenCalls, calls + 1); + } finally { + forge.admins.set(ADMIN, PASSWORD); + } +}); + test("concurrent first calls share one mint", async () => { const { env, logs } = await delivered(forge); const client = minted(env, logs); @@ -339,7 +406,8 @@ test("the admin account is refused: said plainly, nothing kept, and the next cal const { env, file, logs } = await delivered(forge); forge.admins.delete(ADMIN); // the forge's data came from a predecessor; mesh-admin was never created there try { - const client = minted(env, logs); + let clock = 1_000_000; + const client = minted(env, logs, () => clock); const before = forge.mints; await assert.rejects(client.listRepos(), (err: unknown) => { @@ -354,8 +422,9 @@ test("the admin account is refused: said plainly, nothing kept, and the next cal assert.equal(forge.mints, before); await assert.rejects(stat(file), /ENOENT/); - // The account appears (the operator created it): the very next call mints and works. + // The account appears (the operator created it): the next call after the retry window mints and works. forge.admins.set(ADMIN, PASSWORD); + clock += MINT_RETRY_AFTER_MS; assert.equal((await client.listRepos()).length, 1); assert.equal(forge.mints, before + 1); } finally { diff --git a/modules/gitea/token.ts b/modules/gitea/token.ts index 3aac30b1..482c685b 100644 --- a/modules/gitea/token.ts +++ b/modules/gitea/token.ts @@ -97,13 +97,25 @@ export interface MintedTokenOptions { readonly scopes?: readonly string[]; readonly log?: (line: string) => void; readonly fetch?: typeof fetch; + /** The clock, in milliseconds; a test sets it. */ + readonly now?: () => number; } +/** + * **How long a failed mint is not tried again** (novox/hq issue 739). Every call that finds no token mints, and a + * mint is two or three calls over basic auth, each one hashing the admin password on the forge. A mint that keeps + * failing, once per tool call, is a loop the forge pays for in CPU: on 2026-10-14 it ran several times a minute + * on the control-node while it was already overloaded. Within this window the last failure is answered again, + * without asking the forge. + */ +export const MINT_RETRY_AFTER_MS = 30_000; + /** The token the module mints for itself, kept in its state and renewed when the forge rejects it. */ export class MintedToken implements TokenSource { private held: string | null = null; private readFile = false; private inflight: Promise | null = null; + private failed: { at: number; error: unknown } | null = null; private readonly name: string; private readonly scopes: readonly string[]; private readonly log: (line: string) => void; @@ -114,8 +126,11 @@ export class MintedToken implements TokenSource { this.scopes = opts.scopes ?? TOKEN_SCOPES; this.log = opts.log ?? ((line) => console.log(`[gitea] ${line}`)); this.fetchImpl = opts.fetch ?? fetch; + this.now = opts.now ?? Date.now; } + private readonly now: () => number; + /** * Build from the runtime's environment: the forge's URL, the admin login and password file the * manifest hands the runtime, and the module's state directory (MESH_GITEA_STATE_DIR, a directory @@ -126,7 +141,10 @@ export class MintedToken implements TokenSource { const opts = MintedToken.optionsFromEnv(url, env); // One source per kept file in a process. The watcher and the tools entrypoint both build a // client in the same runtime; two sources over one file would each renew on a 401 and drop the - // other's token by name, forever. Shared, a renewal is one renewal. + // other's token by name, forever. Shared, a renewal is one renewal. Across processes it is not shared: the + // runtime serves the watcher (index.js) and the tools (tools/index.js) as separate bundles, so two sources over + // one file do exist. renew() reads the kept file before it mints, and a race that still leaves two tokens by + // the name is undone by dropping each by its id (doMint; novox/hq issue 739). const shared = MintedToken.shared.get(opts.file); if (shared) return shared; const source = new MintedToken(opts); @@ -163,7 +181,7 @@ export class MintedToken implements TokenSource { } if (kept !== null) return this.mint("the kept token was minted with other scopes than the tools take now — minting one with them"); } - return this.mint("no token kept — minting one"); + return this.mint(this.failed ? "no token held since the last mint failed — minting one" : "no token kept — minting one"); } async renew(rejected: string): Promise { @@ -188,8 +206,19 @@ export class MintedToken implements TokenSource { /** One mint at a time: concurrent first calls share it, rather than each minting its own. */ private mint(why: string): Promise { if (this.inflight === null) { + // A mint that failed a moment ago is not asked of the forge again yet (novox/hq issue 739): its error stands. + if (this.failed !== null && this.now() - this.failed.at < MINT_RETRY_AFTER_MS) return Promise.reject(this.failed.error); this.log(why); - this.inflight = this.doMint().finally(() => { + this.inflight = this.doMint().then( + (token) => { + this.failed = null; + return token; + }, + (err) => { + this.failed = { at: this.now(), error: err }; + throw err; + }, + ).finally(() => { this.inflight = null; }); } @@ -261,7 +290,12 @@ export class MintedToken implements TokenSource { // went while the forge's data stayed. It is ours to replace: drop it by name and mint again. this.log(`the forge already holds a token named "${this.name}" — replacing it`); const dropped = await call("DELETE", `/${encodeURIComponent(this.name)}`); - if (dropped.status !== 204 && dropped.status !== 404) { + if (dropped.status === 422) { + // More than one token carries the name (novox/hq issue 739): two processes minted at once, and the forge's + // check that a name is unused is not atomic. The forge refuses a delete by such a name for good, so every + // one of them is dropped by its id — all are ours, and none of their values is kept. + await this.dropEveryNamed(call, dropped); + } else if (dropped.status !== 204 && dropped.status !== 404) { throw new Error(`Gitea DELETE /users/${this.opts.admin}/tokens/${this.name}: ${dropped.status} ${detail(dropped.body)}`); } res = await call("POST", "", { name: this.name, scopes: this.scopes }); @@ -277,6 +311,28 @@ export class MintedToken implements TokenSource { this.log(`minted a token for "${this.opts.admin}" (${this.scopes.join(", ")}), kept at ${this.opts.file}`); return token; } + + /** Drop, by id, every token of the admin account that carries this name; the forge listed them, page by page. */ + private async dropEveryNamed(call: (method: string, path?: string) => Promise<{ status: number; body: any }>, + refused: { status: number; body: any }): Promise { + const ids: number[] = []; + for (let page = 1; page <= 20; page++) { + const listed = await call("GET", `?page=${page}&limit=50`); + if (listed.status !== 200 || !Array.isArray(listed.body)) { + throw new Error(`Gitea DELETE /users/${this.opts.admin}/tokens/${this.name}: ${refused.status} ${detail(refused.body)}; ` + + `and its tokens could not be listed to drop them one by one: ${listed.status} ${detail(listed.body)}`); + } + for (const t of listed.body) if (t?.name === this.name && Number.isInteger(t?.id)) ids.push(t.id); + if (listed.body.length < 50) break; + } + this.log(`the forge holds ${ids.length} tokens named "${this.name}" — dropping each by its id`); + for (const id of ids) { + const dropped = await call("DELETE", `/${id}`); + if (dropped.status !== 204 && dropped.status !== 404) { + throw new Error(`Gitea DELETE /users/${this.opts.admin}/tokens/${id}: ${dropped.status} ${detail(dropped.body)}`); + } + } + } } function detail(body: unknown): string {