From 1de7c78a5e0966bee32c06b7d359b72b23db229e Mon Sep 17 00:00:00 2001 From: Alix-007 Date: Sat, 8 Aug 2026 11:44:57 +0800 Subject: [PATCH] fix(proxy): bound debug CA generation (#109106) --- src/proxy-capture/ca.test.ts | 196 ++++++++++++++++++ src/proxy-capture/ca.ts | 133 +++++++++--- .../proxy-server.managed-proxy.test.ts | 11 +- src/proxy-capture/proxy-server.test.ts | 9 +- 4 files changed, 313 insertions(+), 36 deletions(-) create mode 100644 src/proxy-capture/ca.test.ts diff --git a/src/proxy-capture/ca.test.ts b/src/proxy-capture/ca.test.ts new file mode 100644 index 000000000000..2b4e1d3683a6 --- /dev/null +++ b/src/proxy-capture/ca.test.ts @@ -0,0 +1,196 @@ +// Proxy capture CA tests cover bounded certificate generation. +import fs from "node:fs"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js"; + +const { resolveSystemBinMock, runExecMock } = vi.hoisted(() => ({ + resolveSystemBinMock: vi.fn(() => "/usr/bin/openssl"), + runExecMock: vi.fn(), +})); + +vi.mock("node:crypto", async (importOriginal) => { + const actual = await importOriginal(); + const parseMarker = (value: Buffer, prefix: string): string => { + const text = value.toString("utf8"); + if (!text.startsWith(prefix)) { + throw new Error("invalid certificate material"); + } + return text.slice(prefix.length); + }; + const cryptoMock = { ...actual } as typeof actual; + Object.defineProperties(cryptoMock, { + createPrivateKey: { + value: (value: Buffer) => ({ marker: parseMarker(value, "ca-material-marker:") }), + }, + X509Certificate: { + value: class { + readonly ca: boolean; + readonly marker: string; + + constructor(value: Buffer) { + this.marker = parseMarker(value, "ca-cert-marker:"); + this.ca = this.marker !== "not-ca"; + } + + checkPrivateKey(key: { marker?: string }): boolean { + return key.marker === this.marker; + } + }, + }, + }); + return cryptoMock; +}); +vi.mock("../infra/resolve-system-bin.js", () => ({ resolveSystemBin: resolveSystemBinMock })); +vi.mock("../process/exec.js", async (importOriginal) => ({ + ...(await importOriginal()), + runExec: runExecMock, +})); + +import { ensureDebugProxyCa } from "./ca.js"; + +const tempDirs = createTrackedTempDirs(); + +function outputPath(args: string[], flag: "-config" | "-keyout" | "-out"): string { + const index = args.indexOf(flag); + const value = args[index + 1]; + if (!value) { + throw new Error(`missing ${flag} output path`); + } + return value; +} + +function writeGeneratedPair(args: string[], marker: string): void { + fs.writeFileSync(outputPath(args, "-out"), `ca-cert-marker:${marker}`); + fs.writeFileSync(outputPath(args, "-keyout"), `ca-material-marker:${marker}`); +} + +async function makeCertPaths() { + const certDir = await tempDirs.make("openclaw-proxy-ca-"); + return { + certDir, + certPath: path.join(certDir, "root-ca.pem"), + keyPath: path.join(certDir, "root-ca-key.pem"), + }; +} + +function generatePair(marker: string): void { + runExecMock.mockImplementationOnce(async (_command: string, args: string[]) => { + writeGeneratedPair(args, marker); + }); +} + +afterEach(async () => { + vi.restoreAllMocks(); + runExecMock.mockReset(); + await tempDirs.cleanup(); +}); + +describe("ensureDebugProxyCa", () => { + it("regenerates after partial output timeout without reusing stale final files", async () => { + const { certDir, certPath, keyPath } = await makeCertPaths(); + fs.writeFileSync(certPath, "stale partial cert"); + fs.writeFileSync(keyPath, "stale partial key"); + let generatedConfig = ""; + runExecMock + .mockImplementationOnce(async (_command: string, args: string[]) => { + generatedConfig = fs.readFileSync(outputPath(args, "-config"), "utf8"); + fs.writeFileSync(outputPath(args, "-out"), "partial cert"); + fs.writeFileSync(outputPath(args, "-keyout"), "partial key"); + throw new Error("openssl timed out"); + }) + .mockImplementationOnce(async (_command: string, args: string[]) => { + writeGeneratedPair(args, "retry"); + }); + + await expect(ensureDebugProxyCa(certDir)).rejects.toThrow("openssl timed out"); + expect(fs.readFileSync(certPath, "utf8")).toBe("stale partial cert"); + expect(fs.readFileSync(keyPath, "utf8")).toBe("stale partial key"); + expect(fs.readdirSync(certDir).toSorted()).toEqual(["root-ca-key.pem", "root-ca.pem"]); + + await expect( + Promise.all([ensureDebugProxyCa(certDir), ensureDebugProxyCa(certDir)]), + ).resolves.toEqual([ + { certPath, keyPath }, + { certPath, keyPath }, + ]); + expect(runExecMock).toHaveBeenCalledTimes(2); + const [command, args, options] = runExecMock.mock.calls[0] as [string, string[], object]; + expect(command).toBe("/usr/bin/openssl"); + expect(args).toEqual(expect.arrayContaining(["req", "-extensions", "v3_ca", "-x509"])); + expect(path.dirname(outputPath(args, "-config"))).toBe(path.dirname(outputPath(args, "-out"))); + expect(path.dirname(outputPath(args, "-out")).startsWith(`${certDir}${path.sep}`)).toBe(true); + expect(options).toEqual({ logOutput: false, timeoutMs: 30_000 }); + expect(generatedConfig).toContain("CN = OpenClaw Debug Proxy"); + expect(generatedConfig).toContain("basicConstraints = critical, CA:TRUE"); + expect(generatedConfig).toContain("keyUsage = critical, keyCertSign, cRLSign"); + }); + + it("rejects matching certificate material that is not a CA", async () => { + const { certDir } = await makeCertPaths(); + generatePair("not-ca"); + + await expect(ensureDebugProxyCa(certDir)).rejects.toThrow( + "openssl generated invalid debug proxy certificate material", + ); + + expect(fs.readdirSync(certDir)).toEqual([]); + }); + + it("waits for a live lock owner before generating", async () => { + const { certDir, certPath, keyPath } = await makeCertPaths(); + const lockPath = `${keyPath}.lock`; + fs.writeFileSync(lockPath, JSON.stringify({ pid: process.pid, createdAt: new Date() })); + generatePair("live-owner-retry"); + const releaseOwner = setTimeout(() => fs.rmSync(lockPath, { force: true }), 150); + + try { + await expect(ensureDebugProxyCa(certDir)).resolves.toEqual({ certPath, keyPath }); + } finally { + clearTimeout(releaseOwner); + } + + expect(runExecMock).toHaveBeenCalledTimes(1); + }); + + it("recovers when publication is interrupted between the two renames", async () => { + const { certDir, certPath, keyPath } = await makeCertPaths(); + fs.writeFileSync(certPath, "stale cert"); + fs.writeFileSync(keyPath, "stale key"); + runExecMock + .mockImplementationOnce(async (_command: string, args: string[]) => { + writeGeneratedPair(args, "failed-publication"); + }) + .mockImplementationOnce(async (_command: string, args: string[]) => { + writeGeneratedPair(args, "retry-after-publication"); + }); + + const renameSync = fs.renameSync.bind(fs); + let rejectedPublishedCert = false; + const renameSpy = vi.spyOn(fs, "renameSync").mockImplementation((source, destination) => { + const sourcePath = source.toString(); + if ( + !rejectedPublishedCert && + destination.toString() === certPath && + sourcePath.includes(`${path.sep}.root-ca-`) + ) { + rejectedPublishedCert = true; + throw Object.assign(new Error("certificate publication failed"), { code: "EACCES" }); + } + renameSync(source, destination); + }); + + await expect(ensureDebugProxyCa(certDir)).rejects.toThrow("certificate publication failed"); + + expect(fs.readFileSync(certPath, "utf8")).toBe("stale cert"); + expect(fs.readFileSync(keyPath, "utf8")).toBe("ca-material-marker:failed-publication"); + expect(fs.readdirSync(certDir).toSorted()).toEqual(["root-ca-key.pem", "root-ca.pem"]); + + renameSpy.mockRestore(); + await expect(ensureDebugProxyCa(certDir)).resolves.toEqual({ certPath, keyPath }); + + expect(fs.readFileSync(certPath, "utf8")).toBe("ca-cert-marker:retry-after-publication"); + expect(fs.readFileSync(keyPath, "utf8")).toBe("ca-material-marker:retry-after-publication"); + expect(runExecMock).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/proxy-capture/ca.ts b/src/proxy-capture/ca.ts index 0f3aed9d8056..2ad1293ff495 100644 --- a/src/proxy-capture/ca.ts +++ b/src/proxy-capture/ca.ts @@ -1,9 +1,63 @@ // Proxy capture CA helpers create and inspect local capture CA certificates. +import { createPrivateKey, X509Certificate } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; +import { type FileLockOptions, withFileLock } from "../infra/file-lock.js"; import { resolveSystemBin } from "../infra/resolve-system-bin.js"; +import { KeyedAsyncQueue } from "../plugin-sdk/keyed-async-queue.js"; import { runExec } from "../process/exec.js"; +const DEBUG_PROXY_CA_GENERATION_TIMEOUT_MS = 30_000; +const DEBUG_PROXY_CA_OPENSSL_CONFIG = [ + "[req]", + "distinguished_name = subject", + "prompt = no", + "", + "[subject]", + "CN = OpenClaw Debug Proxy", + "", + "[v3_ca]", + "basicConstraints = critical, CA:TRUE", + "keyUsage = critical, keyCertSign, cRLSign", + "", +].join("\n"); +const DEBUG_PROXY_CA_LOCK_OPTIONS: FileLockOptions = { + retries: { + // About 36s of minimum backoff covers one full 30s OpenSSL deadline. + retries: 80, + factor: 1.3, + minTimeout: 25, + maxTimeout: 500, + randomize: true, + }, + stale: 60_000, + staleRecovery: "remove-if-unchanged", +}; +const debugProxyCaGenerationQueue = new KeyedAsyncQueue(); + +function isValidDebugProxyCaPair(certPath: string, keyPath: string): boolean { + try { + const certStat = fs.lstatSync(certPath); + const keyStat = fs.lstatSync(keyPath); + if (!certStat.isFile() || !keyStat.isFile() || certStat.size === 0 || keyStat.size === 0) { + return false; + } + const cert = new X509Certificate(fs.readFileSync(certPath)); + const key = createPrivateKey(fs.readFileSync(keyPath)); + return cert.ca && cert.checkPrivateKey(key); + } catch { + return false; + } +} + +function removeStagingDirBestEffort(stagingDir: string): void { + try { + fs.rmSync(stagingDir, { recursive: true, force: true }); + } catch { + // Cleanup failure must not replace a successful publication result. + } +} + // Ensure a short-lived root CA for local MITM debug proxy runs. Existing certs // are reused within the cert dir so repeated starts do not prompt regeneration. export async function ensureDebugProxyCa(certDir: string): Promise<{ @@ -13,32 +67,57 @@ export async function ensureDebugProxyCa(certDir: string): Promise<{ fs.mkdirSync(certDir, { recursive: true }); const certPath = path.join(certDir, "root-ca.pem"); const keyPath = path.join(certDir, "root-ca-key.pem"); - if (fs.existsSync(certPath) && fs.existsSync(keyPath)) { - return { certPath, keyPath }; - } - const openssl = resolveSystemBin("openssl"); - if (!openssl) { - throw new Error("openssl is required to generate debug proxy certificates"); - } - await runExec( - openssl, - [ - "req", - "-x509", - "-newkey", - "rsa:2048", - "-sha256", - "-days", - "7", - "-nodes", - "-keyout", - keyPath, - "-out", - certPath, - "-subj", - "/CN=OpenClaw Debug Proxy", - ], - { logOutput: false }, + const canonicalKeyPath = path.join(fs.realpathSync(certDir), "root-ca-key.pem"); + return await debugProxyCaGenerationQueue.enqueue(canonicalKeyPath, async () => + withFileLock(canonicalKeyPath, DEBUG_PROXY_CA_LOCK_OPTIONS, async () => { + if (isValidDebugProxyCaPair(certPath, keyPath)) { + return { certPath, keyPath }; + } + const openssl = resolveSystemBin("openssl"); + if (!openssl) { + throw new Error("openssl is required to generate debug proxy certificates"); + } + const stagingDir = fs.mkdtempSync(path.join(certDir, ".root-ca-")); + const stagedConfigPath = path.join(stagingDir, "openssl.cnf"); + const stagedCertPath = path.join(stagingDir, "root-ca.pem"); + const stagedKeyPath = path.join(stagingDir, "root-ca-key.pem"); + try { + fs.writeFileSync(stagedConfigPath, DEBUG_PROXY_CA_OPENSSL_CONFIG, { mode: 0o600 }); + await runExec( + openssl, + [ + "req", + "-config", + stagedConfigPath, + "-extensions", + "v3_ca", + "-x509", + "-newkey", + "rsa:2048", + "-sha256", + "-days", + "7", + "-nodes", + "-keyout", + stagedKeyPath, + "-out", + stagedCertPath, + ], + { logOutput: false, timeoutMs: DEBUG_PROXY_CA_GENERATION_TIMEOUT_MS }, + ); + if (!isValidDebugProxyCaPair(stagedCertPath, stagedKeyPath)) { + throw new Error("openssl generated invalid debug proxy certificate material"); + } + fs.chmodSync(stagedKeyPath, 0o600); + fs.chmodSync(stagedCertPath, 0o644); + // All OpenClaw writers hold this lock. Same-directory renames replace each + // file atomically; validation repairs a pair interrupted between renames. + fs.renameSync(stagedKeyPath, keyPath); + fs.renameSync(stagedCertPath, certPath); + return { certPath, keyPath }; + } finally { + removeStagingDirBestEffort(stagingDir); + } + }), ); - return { certPath, keyPath }; } diff --git a/src/proxy-capture/proxy-server.managed-proxy.test.ts b/src/proxy-capture/proxy-server.managed-proxy.test.ts index 359e1f037cab..bd95a425b14d 100644 --- a/src/proxy-capture/proxy-server.managed-proxy.test.ts +++ b/src/proxy-capture/proxy-server.managed-proxy.test.ts @@ -1,15 +1,19 @@ // Managed proxy tests cover proxy server lifecycle with managed capture files. -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdtemp, rm } from "node:fs/promises"; import { createServer as createHttpServer } from "node:http"; import { Socket, type AddressInfo } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; import type { DebugProxySettings } from "./env.js"; import { startDebugProxyServer } from "./proxy-server.js"; import { closeDebugProxyCaptureStore } from "./store.sqlite.js"; +vi.mock("./ca.js", () => ({ + ensureDebugProxyCa: async () => ({ certPath: "test", keyPath: "test" }), +})); + let testRoot: string | undefined; const originalStateDir = process.env.OPENCLAW_STATE_DIR; @@ -32,9 +36,6 @@ async function cleanupTestDirs(): Promise { async function makeSettings(): Promise { testRoot = await mkdtemp(join(tmpdir(), "openclaw-debug-proxy-managed-proxy-")); const certDir = join(testRoot, "certs"); - await mkdir(certDir, { recursive: true }); - await writeFile(join(certDir, "root-ca.pem"), "test root cert\n", "utf8"); - await writeFile(join(certDir, "root-ca-key.pem"), "test root key\n", "utf8"); process.env.OPENCLAW_STATE_DIR = testRoot; return { enabled: true, diff --git a/src/proxy-capture/proxy-server.test.ts b/src/proxy-capture/proxy-server.test.ts index 67d8dd267ca9..22421300ad99 100644 --- a/src/proxy-capture/proxy-server.test.ts +++ b/src/proxy-capture/proxy-server.test.ts @@ -1,5 +1,5 @@ // Proxy capture server tests cover request recording and response handling. -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdtemp, rm } from "node:fs/promises"; import { request as httpRequest, createServer as createHttpServer, @@ -14,6 +14,10 @@ import type { DebugProxySettings } from "./env.js"; import { startDebugProxyServer } from "./proxy-server.js"; import { closeDebugProxyCaptureStore, getDebugProxyCaptureStore } from "./store.sqlite.js"; +vi.mock("./ca.js", () => ({ + ensureDebugProxyCa: async () => ({ certPath: "test", keyPath: "test" }), +})); + let testRoot: string | undefined; const originalStateDir = process.env.OPENCLAW_STATE_DIR; @@ -36,9 +40,6 @@ async function cleanupTestRoot(): Promise { async function makeSettings(): Promise { testRoot = await mkdtemp(join(tmpdir(), "openclaw-debug-proxy-server-")); const certDir = join(testRoot, "certs"); - await mkdir(certDir, { recursive: true }); - await writeFile(join(certDir, "root-ca.pem"), "test root cert\n", "utf8"); - await writeFile(join(certDir, "root-ca-key.pem"), "test root key\n", "utf8"); process.env.OPENCLAW_STATE_DIR = testRoot; return { enabled: true,