From b11b0000484eb87daf1102b9413ad75a6a9ff312 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Thu, 20 Aug 2026 16:28:41 +0700 Subject: [PATCH] fix(agent-bridge): make the regenerate-cert endpoint actually mint a new cert (#10467) (#10715) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merged via merge-train (release/v3.8.50, batch1 2026-08-20) — static gates (typecheck/file-size/complexity/cognitive/changelog) green on the combined tree; test:unit reds observed in the boarded run were verified pre-existing on the pure release tip (unrelated flake), not caused by this PR. Thanks for the contribution! --- .../agent-bridge/cert/regenerate/route.ts | 9 +- src/mitm/cert/generate.ts | 9 +- ...bridge-cert-regenerate-force-10467.test.ts | 93 +++++++++++++++++++ 3 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 tests/unit/agent-bridge-cert-regenerate-force-10467.test.ts diff --git a/src/app/api/tools/agent-bridge/cert/regenerate/route.ts b/src/app/api/tools/agent-bridge/cert/regenerate/route.ts index 2459275e71..c6b66eba1b 100644 --- a/src/app/api/tools/agent-bridge/cert/regenerate/route.ts +++ b/src/app/api/tools/agent-bridge/cert/regenerate/route.ts @@ -9,11 +9,10 @@ import { createErrorResponse } from "@/lib/api/errorResponse"; export async function POST(): Promise { try { - // generateCert checks for existing files — force-regenerate by deleting first - // is not in scope; the function is idempotent (returns existing paths). If a - // caller needs a fresh cert they must delete the old one manually. We expose - // whatever generateCert decides. - const result = await generateCert(); + // #10467: generateCert() returns the existing paths untouched when a cert is already + // on disk, which made this endpoint a no-op — the download still served the old file. + // This route is the one caller that must always mint a fresh cert. + const result = await generateCert({ force: true }); return Response.json({ ok: true, certPath: result.cert, keyPath: result.key }); } catch (err) { const msg = sanitizeErrorMessage(err instanceof Error ? err.message : String(err)); diff --git a/src/mitm/cert/generate.ts b/src/mitm/cert/generate.ts index 6eb26878bd..ae9b6c69e6 100644 --- a/src/mitm/cert/generate.ts +++ b/src/mitm/cert/generate.ts @@ -16,12 +16,17 @@ const TARGET_HOST = TARGET_HOSTS[0]; /** * Generate self-signed SSL certificate using selfsigned (pure JS, no openssl needed) */ -export async function generateCert(): Promise<{ key: string; cert: string }> { +export async function generateCert(options?: { + force?: boolean; +}): Promise<{ key: string; cert: string }> { const certDir = path.join(resolveMitmDataDir(), "mitm"); const keyPath = path.join(certDir, "server.key"); const certPath = path.join(certDir, "server.crt"); - if (fs.existsSync(keyPath) && fs.existsSync(certPath)) { + // #10467: callers that only need a cert to exist keep the existing one, but the + // regenerate endpoint has to actually mint a new one — otherwise a cert missing the + // SANs added in #6494 can never be replaced from the UI. + if (!options?.force && fs.existsSync(keyPath) && fs.existsSync(certPath)) { console.log("✅ SSL certificate already exists"); return { key: keyPath, cert: certPath }; } diff --git a/tests/unit/agent-bridge-cert-regenerate-force-10467.test.ts b/tests/unit/agent-bridge-cert-regenerate-force-10467.test.ts new file mode 100644 index 0000000000..fc600982c6 --- /dev/null +++ b/tests/unit/agent-bridge-cert-regenerate-force-10467.test.ts @@ -0,0 +1,93 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { X509Certificate } from "node:crypto"; + +// #10467: POST /api/tools/agent-bridge/cert/regenerate called generateCert() with no +// arguments. generateCert() short-circuits when server.key and server.crt already exist, +// so the endpoint was a no-op on every machine that had ever started the bridge — the +// download route kept serving the old file, with the same md5 and, for anyone whose cert +// predates #6494, still missing the extra SANs. generateCert now takes { force } and the +// regenerate route is the one caller that passes it. + +const certModule = "../../src/mitm/cert/generate.ts"; + +async function withTempDataDir(fn: (dir: string) => Promise): Promise { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cert-10467-")); + const previous = process.env.DATA_DIR; + process.env.DATA_DIR = dir; + try { + return await fn(dir); + } finally { + if (previous === undefined) delete process.env.DATA_DIR; + else process.env.DATA_DIR = previous; + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +const read = (p: string) => fs.readFileSync(p, "utf8"); + +test("generateCert() keeps the existing certificate when it is already on disk", async () => { + await withTempDataDir(async () => { + const { generateCert } = await import(certModule); + + const first = await generateCert(); + const certBefore = read(first.cert); + const keyBefore = read(first.key); + + const second = await generateCert(); + + assert.equal(second.cert, first.cert, "path should be stable"); + assert.equal(read(second.cert), certBefore, "certificate should not be replaced"); + assert.equal(read(second.key), keyBefore, "key should not be replaced"); + }); +}); + +test("generateCert({ force: true }) mints a new certificate over the existing one", async () => { + await withTempDataDir(async () => { + const { generateCert } = await import(certModule); + + const first = await generateCert(); + const certBefore = read(first.cert); + const keyBefore = read(first.key); + + const forced = await generateCert({ force: true }); + + assert.equal(forced.cert, first.cert, "path should be stable"); + assert.notEqual(read(forced.cert), certBefore, "certificate should be replaced"); + assert.notEqual(read(forced.key), keyBefore, "key should be replaced"); + }); +}); + +test("the forced certificate is still valid and carries every antigravity SAN", async () => { + await withTempDataDir(async () => { + const { generateCert } = await import(certModule); + const { ANTIGRAVITY_TARGET } = await import("../../src/mitm/targets/antigravity.ts"); + + await generateCert(); + const forced = await generateCert({ force: true }); + + const x509 = new X509Certificate(fs.readFileSync(forced.cert)); + const sans = (x509.subjectAltName ?? "") + .split(",") + .map((entry) => entry.trim().replace(/^DNS:/, "")) + .filter(Boolean); + + for (const host of ANTIGRAVITY_TARGET.hosts) { + assert.ok(sans.includes(host), `forced cert is missing SAN for ${host}`); + } + }); +}); + +test("generateCert({ force: true }) creates the certificate when none exists yet", async () => { + await withTempDataDir(async () => { + const { generateCert } = await import(certModule); + + const result = await generateCert({ force: true }); + + assert.ok(fs.existsSync(result.cert), "certificate should exist"); + assert.ok(fs.existsSync(result.key), "key should exist"); + }); +});