From 5d6bf0ee7c64326a066b6eccdeecf2da999241ae Mon Sep 17 00:00:00 2001 From: 1PoPTRoN Date: Wed, 19 Aug 2026 01:42:47 -0700 Subject: [PATCH 1/5] fix(hermes): prove broker ownership from a live pid Refuse a healthy managed-tool listener unless its recorded pid still resolves to the NemoClaw broker. This prevents a stale in-process ownership latch from adopting a foreign process after the broker exits while retaining the registered-process reuse path and operator recovery message. Signed-off-by: 1PoPTRoN --- src/lib/hermes-tool-gateway-broker.ts | 50 +- test/hermes-tool-gateway-broker.test.ts | 1300 ++++++++++++----------- 2 files changed, 707 insertions(+), 643 deletions(-) diff --git a/src/lib/hermes-tool-gateway-broker.ts b/src/lib/hermes-tool-gateway-broker.ts index 2dc48ba47a2..0d0a28cbeee 100644 --- a/src/lib/hermes-tool-gateway-broker.ts +++ b/src/lib/hermes-tool-gateway-broker.ts @@ -63,6 +63,8 @@ const HERMES_TOOL_GATEWAY_CONTROL_CONTRACT_PATH = path.join( ); const HERMES_TOOL_GATEWAY_RUNTIME_MISMATCH_RECOVERY = "Reauthorize every managed-tool Hermes sandbox, then retry."; +const HERMES_TOOL_GATEWAY_UNOWNED_LISTENER_RECOVERY = + "Stop the process holding that port, then retry."; const HERMES_TOOL_GATEWAY_CONTROL_CLIENT_SOURCE = [ 'const http = require("node:http");', "const [socketPath, route, timeoutValue] = process.argv.slice(1);", @@ -118,8 +120,6 @@ const HERMES_TOOL_GATEWAY_CONTROL_CLIENT_SOURCE = [ "});", ].join("\n"); -let brokerStartedThisRun = false; - function sleep(ms) { const lock = new Int32Array(new SharedArrayBuffer(4)); Atomics.wait(lock, 0, 0, ms); @@ -555,7 +555,7 @@ function preflightHermesToolGatewayCloneBinding(sandboxName) { } const pid = readPid(); - const currentBrokerOwned = isHermesToolGatewayBrokerProcess(pid) || brokerStartedThisRun; + const currentBrokerOwned = isHermesToolGatewayBrokerProcess(pid); const currentBrokerHealthy = isHermesToolGatewayBrokerHealthy(); if (currentBrokerHealthy && !currentBrokerOwned) { throw new Error("Hermes managed-tool broker health endpoint is not owned by NemoClaw"); @@ -738,8 +738,23 @@ function ensureHermesToolGatewayBroker(options = {}) { const desiredHash = brokerRuntimeHash(); const hashMatches = readBrokerHash() === desiredHash; const pid = readPid(); - const currentBrokerOwned = isHermesToolGatewayBrokerProcess(pid) || brokerStartedThisRun; - const currentBrokerHealthy = currentBrokerOwned && isHermesToolGatewayBrokerHealthy(); + const currentBrokerOwned = isHermesToolGatewayBrokerProcess(pid); + const brokerHealthy = isHermesToolGatewayBrokerHealthy(); + const currentBrokerHealthy = currentBrokerOwned && brokerHealthy; + // `/health` is unauthenticated on a fixed port, so reachability proves + // liveness and never identity. Ownership comes only from a recorded pid that + // still resolves to a running broker, re-proved on every call: a broker can + // exit and leave the port free for another process to bind. Refuse before any + // path can adopt, restart around, or stage credentials against a listener + // NemoClaw cannot prove it owns. + if (brokerHealthy && !currentBrokerOwned) { + console.error( + "Hermes managed-tool broker health endpoint is not owned by NemoClaw; " + + `refusing to reuse the listener on port ${HERMES_TOOL_GATEWAY_PORT}. ` + + HERMES_TOOL_GATEWAY_UNOWNED_LISTENER_RECOVERY, + ); + return false; + } if (options.startWithoutCredential) { if (currentBrokerHealthy) { return hashMatches && fs.existsSync(HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH); @@ -752,7 +767,6 @@ function ensureHermesToolGatewayBroker(options = {}) { isHermesToolGatewayBrokerHealthy() && fs.existsSync(HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH) ) { - brokerStartedThisRun = true; return true; } sleep(250); @@ -779,7 +793,6 @@ function ensureHermesToolGatewayBroker(options = {}) { refreshToken, options.sandboxName ?? null, ); - if (registered) brokerStartedThisRun = true; return registered; } if (refreshPlan === "start-or-restart") { @@ -791,7 +804,6 @@ function ensureHermesToolGatewayBroker(options = {}) { isHermesToolGatewayBrokerHealthy() && registerHermesToolGatewayRuntimeCredential(refreshToken, options.sandboxName ?? null) ) { - brokerStartedThisRun = true; return true; } sleep(250); @@ -799,25 +811,9 @@ function ensureHermesToolGatewayBroker(options = {}) { return false; } - if ( - !options.forceRestart && - hashMatches && - brokerStartedThisRun && - isHermesToolGatewayBrokerHealthy() - ) { - return true; - } - if ( - !options.forceRestart && - hashMatches && - isHermesToolGatewayBrokerProcess(pid) && - isHermesToolGatewayBrokerHealthy() - ) { - brokerStartedThisRun = true; - return true; - } - if (!options.forceRestart && hashMatches && isHermesToolGatewayBrokerHealthy()) { - brokerStartedThisRun = true; + // `currentBrokerHealthy` already requires ownership, covering both proofs the + // three former branches tested separately, so reuse is one condition. + if (!options.forceRestart && hashMatches && currentBrokerHealthy) { return true; } // Raw Nous OAuth stays out of durable ~/.nemoclaw state. If the broker is diff --git a/test/hermes-tool-gateway-broker.test.ts b/test/hermes-tool-gateway-broker.test.ts index 0c182ebb937..e2d13e453da 100644 --- a/test/hermes-tool-gateway-broker.test.ts +++ b/test/hermes-tool-gateway-broker.test.ts @@ -233,6 +233,52 @@ describe("Hermes managed-tool gateway broker", () => { ).toBe("start-or-restart"); }); + it( + "refuses a healthy listener on the managed-tool port that it does not own", + async ({ resources, skip }) => { + const { home } = resources.home("nemoclaw-broker-ownership-"); + vi.stubEnv("HOME", home); + delete require.cache[require.resolve(BROKER_WRAPPER)]; + const broker = require(BROKER_WRAPPER); + + const impostor = resources.ownServer( + http.createServer((_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ ok: true, services: [] })); + }), + ); + await new Promise((resolve, reject) => { + impostor.once("error", reject); + impostor.listen(broker.HERMES_TOOL_GATEWAY_PORT, "127.0.0.1", () => resolve()); + }); + + // The health probe shells out to curl. Where a harness blocks loopback + // HTTP for subprocesses no listener reads as healthy, so report that gap + // instead of asserting nothing. + skip( + !broker.isHermesToolGatewayBrokerHealthy(), + "curl cannot read a loopback response in this environment", + ); + + const refusal = vi.spyOn(console, "error").mockImplementation(() => {}); + // Reachability is not identity: `/health` is unauthenticated, so an + // unowned listener must never be adopted as this run's broker. + expect(broker.ensureHermesToolGatewayBroker({})).toBe(false); + const diagnostics = refusal.mock.calls.map((call) => call.join(" ")).join("\n"); + + // The refusal has to name the port it declined, and it must leave no pid + // record, which would mean a broker was spawned against the held port. + expect(diagnostics).toContain(String(broker.HERMES_TOOL_GATEWAY_PORT)); + const pidPath = path.join( + path.dirname(broker.HERMES_TOOL_GATEWAY_STATE_DIR), + "hermes-tool-gateway-broker.pid", + ); + expect(fs.existsSync(pidPath)).toBe(false); + delete require.cache[require.resolve(BROKER_WRAPPER)]; + }, + BROKER_TEST_TIMEOUT_MS, + ); + it("preserves durable state when live credential unregister fails", () => { delete require.cache[require.resolve(BROKER_WRAPPER)]; const broker = require(BROKER_WRAPPER); @@ -270,105 +316,109 @@ describe("Hermes managed-tool gateway broker", () => { } }); - it("uses the current Node runtime for private control requests without a curl dependency", { - timeout: BROKER_TEST_TIMEOUT_MS, - }, async ({ resources }) => { - const previousHome = process.env.HOME; - const previousPath = process.env.PATH; - const home = resources.ownDirectory(fs.mkdtempSync("/tmp/nc-hermes-node-control-")); - try { - process.env.HOME = home; - delete require.cache[require.resolve(BROKER_WRAPPER)]; - const broker = require(BROKER_WRAPPER); - broker.persistHermesToolGatewayProviderState( - "sandbox", - "test-only-refresh", - "test-only-broker", - "sandbox-hermes-inference", - ); - const capturePath = path.join(home, "control-request.json"); - const serverSource = [ - 'const fs = require("node:fs");', - 'const http = require("node:http");', - "const [socketPath, capturePath] = process.argv.slice(1);", - "const server = http.createServer((request, response) => {", - " const chunks = [];", - ' request.on("data", (chunk) => chunks.push(chunk));', - ' request.on("end", () => {', - ' const body = Buffer.concat(chunks).toString("utf8");', - " fs.writeFileSync(capturePath, JSON.stringify({", - " path: request.url,", - " body,", - " }));", - ' response.writeHead(body.includes("reject-refresh") ? 503 : 200, {', - ' "content-type": "application/json",', - " });", - ' response.end("{\\\"registered\\\":true}");', - " });", - "});", - "server.listen(socketPath, () => {", - " fs.chmodSync(socketPath, 0o600);", - ' process.stdout.write("ready\\n");', - "});", - 'process.once("SIGTERM", () => server.close(() => process.exit(0)));', - ].join("\n"); - const server = resources.ownChild( - spawn( - process.execPath, - [ - "--input-type=commonjs", - "--eval", - serverSource, - broker.HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH, - capturePath, - ], - { stdio: ["ignore", "pipe", "pipe"] }, - ), - ); - let output = ""; - server.stdout?.on("data", (chunk) => { - output += chunk.toString(); - }); - server.stderr?.on("data", (chunk) => { - output += chunk.toString(); - }); - await waitForBrokerCondition( - "native Node control server", - server, - () => output, - () => output.includes("ready"), - ); + it( + "uses the current Node runtime for private control requests without a curl dependency", + { + timeout: BROKER_TEST_TIMEOUT_MS, + }, + async ({ resources }) => { + const previousHome = process.env.HOME; + const previousPath = process.env.PATH; + const home = resources.ownDirectory(fs.mkdtempSync("/tmp/nc-hermes-node-control-")); + try { + process.env.HOME = home; + delete require.cache[require.resolve(BROKER_WRAPPER)]; + const broker = require(BROKER_WRAPPER); + broker.persistHermesToolGatewayProviderState( + "sandbox", + "test-only-refresh", + "test-only-broker", + "sandbox-hermes-inference", + ); + const capturePath = path.join(home, "control-request.json"); + const serverSource = [ + 'const fs = require("node:fs");', + 'const http = require("node:http");', + "const [socketPath, capturePath] = process.argv.slice(1);", + "const server = http.createServer((request, response) => {", + " const chunks = [];", + ' request.on("data", (chunk) => chunks.push(chunk));', + ' request.on("end", () => {', + ' const body = Buffer.concat(chunks).toString("utf8");', + " fs.writeFileSync(capturePath, JSON.stringify({", + " path: request.url,", + " body,", + " }));", + ' response.writeHead(body.includes("reject-refresh") ? 503 : 200, {', + ' "content-type": "application/json",', + " });", + ' response.end("{\\\"registered\\\":true}");', + " });", + "});", + "server.listen(socketPath, () => {", + " fs.chmodSync(socketPath, 0o600);", + ' process.stdout.write("ready\\n");', + "});", + 'process.once("SIGTERM", () => server.close(() => process.exit(0)));', + ].join("\n"); + const server = resources.ownChild( + spawn( + process.execPath, + [ + "--input-type=commonjs", + "--eval", + serverSource, + broker.HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH, + capturePath, + ], + { stdio: ["ignore", "pipe", "pipe"] }, + ), + ); + let output = ""; + server.stdout?.on("data", (chunk) => { + output += chunk.toString(); + }); + server.stderr?.on("data", (chunk) => { + output += chunk.toString(); + }); + await waitForBrokerCondition( + "native Node control server", + server, + () => output, + () => output.includes("ready"), + ); - process.env.PATH = "/path-with-no-curl"; - expect( - broker.registerHermesToolGatewayRuntimeCredential("test-only-refresh", "sandbox"), - ).toBe(true); - expect(JSON.parse(fs.readFileSync(capturePath, "utf8"))).toEqual({ - path: "/credentials/register", - body: JSON.stringify({ - sandbox: "sandbox", - refresh_token: "test-only-refresh", - }), - }); - broker.persistHermesToolGatewayProviderState( - "sandbox", - "reject-refresh", - "test-only-broker", - "sandbox-hermes-inference", - ); - expect(broker.registerHermesToolGatewayRuntimeCredential("reject-refresh", "sandbox")).toBe( - false, - ); - } finally { - previousHome === undefined - ? Reflect.deleteProperty(process.env, "HOME") - : Reflect.set(process.env, "HOME", previousHome); - previousPath === undefined - ? Reflect.deleteProperty(process.env, "PATH") - : Reflect.set(process.env, "PATH", previousPath); - delete require.cache[require.resolve(BROKER_WRAPPER)]; - } - }); + process.env.PATH = "/path-with-no-curl"; + expect( + broker.registerHermesToolGatewayRuntimeCredential("test-only-refresh", "sandbox"), + ).toBe(true); + expect(JSON.parse(fs.readFileSync(capturePath, "utf8"))).toEqual({ + path: "/credentials/register", + body: JSON.stringify({ + sandbox: "sandbox", + refresh_token: "test-only-refresh", + }), + }); + broker.persistHermesToolGatewayProviderState( + "sandbox", + "reject-refresh", + "test-only-broker", + "sandbox-hermes-inference", + ); + expect(broker.registerHermesToolGatewayRuntimeCredential("reject-refresh", "sandbox")).toBe( + false, + ); + } finally { + previousHome === undefined + ? Reflect.deleteProperty(process.env, "HOME") + : Reflect.set(process.env, "HOME", previousHome); + previousPath === undefined + ? Reflect.deleteProperty(process.env, "PATH") + : Reflect.set(process.env, "PATH", previousPath); + delete require.cache[require.resolve(BROKER_WRAPPER)]; + } + }, + ); it("restores a prior destination broker binding and reports cleanup failure", () => { delete require.cache[require.resolve(BROKER_WRAPPER)]; @@ -494,562 +544,580 @@ describe("Hermes managed-tool gateway broker", () => { expect(() => broker.probeHermesToolGatewayBrokerStart({ port: probePort })).not.toThrow(); }); - it("refreshes via header, replaces upstream auth, normalizes responses, and rotates OpenShell storage", { - timeout: BROKER_TEST_TIMEOUT_MS, - }, async ({ resources }) => { - const tmp = resources.temporaryDirectory("nemoclaw-hermes-tool-broker-"); - const stateDir = path.join(tmp, "state"); - const binDir = path.join(tmp, "bin"); - fs.mkdirSync(stateDir, { recursive: true, mode: 0o700 }); - fs.mkdirSync(binDir, { recursive: true }); - const openshellLog = path.join(tmp, "openshell.log"); - const openshellBin = path.join(binDir, "openshell"); - fs.writeFileSync( - openshellBin, - [ - "#!/bin/sh", - `printf '%s\\n' "$*" >> "${openshellLog}"`, - `printf 'refresh=%s\\n' "$NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN" >> "${openshellLog}"`, - `printf 'openai=%s\\n' "$OPENAI_API_KEY" >> "${openshellLog}"`, - "exit 0", - "", - ].join("\n"), - { mode: 0o755 }, - ); - const statePath = path.join(stateDir, "sandbox.json"); - fs.writeFileSync( - statePath, - JSON.stringify( - { - version: 1, - sandbox: "sandbox", - provider_name: "sandbox-hermes-tool-gateway", - credential_env: "NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN", - broker_token: "broker-1", - broker_token_sha256: sha256("broker-1"), - refresh_token_sha256: sha256("refresh-1"), - client_id: "hermes-cli", - }, - null, - 2, - ), - { mode: 0o600 }, - ); + it( + "refreshes via header, replaces upstream auth, normalizes responses, and rotates OpenShell storage", + { + timeout: BROKER_TEST_TIMEOUT_MS, + }, + async ({ resources }) => { + const tmp = resources.temporaryDirectory("nemoclaw-hermes-tool-broker-"); + const stateDir = path.join(tmp, "state"); + const binDir = path.join(tmp, "bin"); + fs.mkdirSync(stateDir, { recursive: true, mode: 0o700 }); + fs.mkdirSync(binDir, { recursive: true }); + const openshellLog = path.join(tmp, "openshell.log"); + const openshellBin = path.join(binDir, "openshell"); + fs.writeFileSync( + openshellBin, + [ + "#!/bin/sh", + `printf '%s\\n' "$*" >> "${openshellLog}"`, + `printf 'refresh=%s\\n' "$NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN" >> "${openshellLog}"`, + `printf 'openai=%s\\n' "$OPENAI_API_KEY" >> "${openshellLog}"`, + "exit 0", + "", + ].join("\n"), + { mode: 0o755 }, + ); + const statePath = path.join(stateDir, "sandbox.json"); + fs.writeFileSync( + statePath, + JSON.stringify( + { + version: 1, + sandbox: "sandbox", + provider_name: "sandbox-hermes-tool-gateway", + credential_env: "NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN", + broker_token: "broker-1", + broker_token_sha256: sha256("broker-1"), + refresh_token_sha256: sha256("refresh-1"), + client_id: "hermes-cli", + }, + null, + 2, + ), + { mode: 0o600 }, + ); - const tokenRequests: Array<{ body: string; refreshHeader?: string }> = []; - const agentKeyRequests: Array<{ body: string; authorization?: string }> = []; - const portal = resources.ownServer( - http.createServer((req, res) => { - const chunks: Buffer[] = []; - req.on("data", (chunk) => chunks.push(chunk)); - req.on("end", () => { - const body = Buffer.concat(chunks).toString("utf8"); - res.writeHead(200, { "Content-Type": "application/json" }); - if (req.url === "/api/oauth/agent-key") { - agentKeyRequests.push({ + const tokenRequests: Array<{ body: string; refreshHeader?: string }> = []; + const agentKeyRequests: Array<{ body: string; authorization?: string }> = []; + const portal = resources.ownServer( + http.createServer((req, res) => { + const chunks: Buffer[] = []; + req.on("data", (chunk) => chunks.push(chunk)); + req.on("end", () => { + const body = Buffer.concat(chunks).toString("utf8"); + res.writeHead(200, { "Content-Type": "application/json" }); + if (req.url === "/api/oauth/agent-key") { + agentKeyRequests.push({ + body, + authorization: req.headers.authorization, + }); + res.end( + JSON.stringify({ + api_key: "agent-key-2", + expires_in: 1800, + inference_base_url: "https://inference-api.nousresearch.com/v1", + }), + ); + return; + } + tokenRequests.push({ body, - authorization: req.headers.authorization, + refreshHeader: req.headers["x-nous-refresh-token"] as string | undefined, }); res.end( JSON.stringify({ - api_key: "agent-key-2", - expires_in: 1800, - inference_base_url: "https://inference-api.nousresearch.com/v1", + access_token: "access-2", + refresh_token: "refresh-2", + expires_in: 900, + token_type: "Bearer", }), ); - return; - } - tokenRequests.push({ - body, - refreshHeader: req.headers["x-nous-refresh-token"] as string | undefined, }); - res.end( - JSON.stringify({ - access_token: "access-2", - refresh_token: "refresh-2", - expires_in: 900, - token_type: "Bearer", - }), - ); - }); - }), - ); - const portalPort = await listen(portal); - - const upstreamRequests: Array<{ - url?: string; - authorization?: string; - browserUseApiKey?: string; - apiKey?: string; - acceptEncoding?: string; - }> = []; - const upstream = resources.ownServer( - http.createServer((req, res) => { - upstreamRequests.push({ - url: req.url, - authorization: req.headers.authorization, - browserUseApiKey: req.headers["x-browser-use-api-key"] as string | undefined, - apiKey: req.headers["x-api-key"] as string | undefined, - acceptEncoding: req.headers["accept-encoding"] as string | undefined, - }); - const body = zlib.gzipSync(JSON.stringify({ ok: true, path: req.url })); - res.writeHead(200, { - "Content-Type": "application/json", - "Content-Encoding": "gzip", - "Content-Length": String(body.length), - "Content-MD5": "not-a-real-digest", - "Set-Cookie": "fixture_session=1; HttpOnly; Secure; SameSite=Strict", - }); - res.end(body); - }), - ); - const upstreamPort = await listen(upstream); - const matrixPath = path.join(tmp, "matrix.json"); - const upstreamBase = `http://127.0.0.1:${upstreamPort}`; - fs.writeFileSync( - matrixPath, - JSON.stringify({ - "nous-web": { service: "firecrawl", upstream: upstreamBase }, - "nous-image": { service: "fal-queue", upstream: upstreamBase }, - "nous-audio": { service: "openai-audio", upstream: upstreamBase }, - "nous-browser": { service: "browser-use", upstream: upstreamBase }, - "nous-code": { service: "modal", upstream: upstreamBase }, - }), - ); - const brokerPort = await freePort(); - - const child = resources.ownChild( - spawn(process.execPath, ["--experimental-strip-types", SCRIPT], { - env: { - ...process.env, - HERMES_TOOL_GATEWAY_PORT: String(brokerPort), - HERMES_TOOL_GATEWAY_STATE_DIR: stateDir, - HERMES_TOOL_GATEWAY_MATRIX_PATH: matrixPath, - NOUS_PORTAL_BASE_URL: `http://127.0.0.1:${portalPort}`, - NEMOCLAW_OPENSHELL_BIN: openshellBin, - NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN: "refresh-1", - }, - stdio: ["ignore", "pipe", "pipe"], - }), - ); + }), + ); + const portalPort = await listen(portal); - let output = ""; - child.stdout.on("data", (chunk) => { - output += chunk.toString(); - }); - child.stderr.on("data", (chunk) => { - output += chunk.toString(); - }); + const upstreamRequests: Array<{ + url?: string; + authorization?: string; + browserUseApiKey?: string; + apiKey?: string; + acceptEncoding?: string; + }> = []; + const upstream = resources.ownServer( + http.createServer((req, res) => { + upstreamRequests.push({ + url: req.url, + authorization: req.headers.authorization, + browserUseApiKey: req.headers["x-browser-use-api-key"] as string | undefined, + apiKey: req.headers["x-api-key"] as string | undefined, + acceptEncoding: req.headers["accept-encoding"] as string | undefined, + }); + const body = zlib.gzipSync(JSON.stringify({ ok: true, path: req.url })); + res.writeHead(200, { + "Content-Type": "application/json", + "Content-Encoding": "gzip", + "Content-Length": String(body.length), + "Content-MD5": "not-a-real-digest", + "Set-Cookie": "fixture_session=1; HttpOnly; Secure; SameSite=Strict", + }); + res.end(body); + }), + ); + const upstreamPort = await listen(upstream); + const matrixPath = path.join(tmp, "matrix.json"); + const upstreamBase = `http://127.0.0.1:${upstreamPort}`; + fs.writeFileSync( + matrixPath, + JSON.stringify({ + "nous-web": { service: "firecrawl", upstream: upstreamBase }, + "nous-image": { service: "fal-queue", upstream: upstreamBase }, + "nous-audio": { service: "openai-audio", upstream: upstreamBase }, + "nous-browser": { service: "browser-use", upstream: upstreamBase }, + "nous-code": { service: "modal", upstream: upstreamBase }, + }), + ); + const brokerPort = await freePort(); - await waitForBrokerCondition( - "broker health", - child, - () => output, - async () => { - const response = await fetch(`http://127.0.0.1:${brokerPort}/health`, { - signal: AbortSignal.timeout(1_000), - }); - return response.status === 200; - }, - ); - await waitForBrokerCondition( - "inference provider refresh", - child, - () => output, - () => { - try { - return fs.readFileSync(openshellLog, "utf8").includes("provider update hermes-provider"); - } catch { - return false; - } - }, - ); + const child = resources.ownChild( + spawn(process.execPath, ["--experimental-strip-types", SCRIPT], { + env: { + ...process.env, + HERMES_TOOL_GATEWAY_PORT: String(brokerPort), + HERMES_TOOL_GATEWAY_STATE_DIR: stateDir, + HERMES_TOOL_GATEWAY_MATRIX_PATH: matrixPath, + NOUS_PORTAL_BASE_URL: `http://127.0.0.1:${portalPort}`, + NEMOCLAW_OPENSHELL_BIN: openshellBin, + NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN: "refresh-1", + }, + stdio: ["ignore", "pipe", "pipe"], + }), + ); - const unknown = await fetch(`http://127.0.0.1:${brokerPort}/unknown`); - expect(unknown.status).toBe(404); + let output = ""; + child.stdout.on("data", (chunk) => { + output += chunk.toString(); + }); + child.stderr.on("data", (chunk) => { + output += chunk.toString(); + }); - const denied = await fetch(`http://127.0.0.1:${brokerPort}/firecrawl/v1/scrape`, { - headers: { Authorization: "Bearer wrong-broker-token" }, - }); - expect(denied.status).toBe(401); + await waitForBrokerCondition( + "broker health", + child, + () => output, + async () => { + const response = await fetch(`http://127.0.0.1:${brokerPort}/health`, { + signal: AbortSignal.timeout(1_000), + }); + return response.status === 200; + }, + ); + await waitForBrokerCondition( + "inference provider refresh", + child, + () => output, + () => { + try { + return fs + .readFileSync(openshellLog, "utf8") + .includes("provider update hermes-provider"); + } catch { + return false; + } + }, + ); - const firecrawl = await fetch(`http://127.0.0.1:${brokerPort}/firecrawl/v1/scrape?debug=1`, { - method: "POST", - headers: { - "Content-Type": "application/json", - "x-api-key": "refresh-2", - }, - body: JSON.stringify({ url: "https://example.com" }), - }); - const firecrawlBody = await firecrawl.text(); - expect(firecrawl.status, `${firecrawlBody}\n${output}`).toBe(200); - expect(firecrawl.headers.get("content-encoding")).toBeNull(); - expect(firecrawl.headers.get("content-length")).toBeNull(); - expect(firecrawl.headers.get("content-md5")).toBeNull(); - expect(firecrawl.headers.get("set-cookie")).toBeNull(); - expect(JSON.parse(firecrawlBody)).toEqual({ ok: true, path: "/v1/scrape?debug=1" }); - expect(tokenRequests).toHaveLength(1); - expect(tokenRequests[0]?.refreshHeader).toBe("refresh-1"); - expect(new URLSearchParams(tokenRequests[0]?.body).get("refresh_token")).toBeNull(); - expect(new URLSearchParams(tokenRequests[0]?.body).get("grant_type")).toBe("refresh_token"); - expect(agentKeyRequests).toHaveLength(1); - expect(agentKeyRequests[0]?.authorization).toBe("Bearer access-2"); - expect(JSON.parse(agentKeyRequests[0]?.body || "{}")).toEqual({ - min_ttl_seconds: 1800, - }); - expect(upstreamRequests[0]).toMatchObject({ - url: "/v1/scrape?debug=1", - authorization: "Bearer access-2", - acceptEncoding: "identity", - }); - expect(upstreamRequests[0]?.apiKey).toBeUndefined(); + const unknown = await fetch(`http://127.0.0.1:${brokerPort}/unknown`); + expect(unknown.status).toBe(404); - const rotatedState = JSON.parse(fs.readFileSync(statePath, "utf8")); - expect(rotatedState.refresh_token_sha256).toBe(sha256("refresh-2")); - const openshellOutput = fs.readFileSync(openshellLog, "utf8"); - expect(openshellOutput).toContain( - "provider update sandbox-hermes-tool-gateway --credential NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN", - ); - expect(openshellOutput).toContain("refresh=broker-1"); - expect(openshellOutput).not.toContain("refresh=refresh-2"); - expect(openshellOutput).toContain( - "provider update hermes-provider --credential OPENAI_API_KEY --config OPENAI_BASE_URL=https://inference-api.nousresearch.com/v1", - ); - expect(openshellOutput).toContain("openai=agent-key-2"); - expect(rotatedState.inference_provider_name).toBe("hermes-provider"); - expect(rotatedState.inference_credential_env).toBe("OPENAI_API_KEY"); - expect(rotatedState.inference_agent_key_expires_at).toBeTruthy(); + const denied = await fetch(`http://127.0.0.1:${brokerPort}/firecrawl/v1/scrape`, { + headers: { Authorization: "Bearer wrong-broker-token" }, + }); + expect(denied.status).toBe(401); - const checks = [ - ["/browser-use/browsers", { "X-Browser-Use-API-Key": "broker-1" }, "browser"], - ["/fal-queue/fal-ai/test", { Authorization: "Key broker-1" }, "fal"], - ["/openai-audio/v1/audio/speech", { "openai-api-key": "broker-1" }, "audio"], - ["/modal/sandboxes", { Authorization: "Bearer broker-1" }, "modal"], - ] as const; - for (const [route, headers] of checks) { - const resp = await fetch(`http://127.0.0.1:${brokerPort}${route}`, { + const firecrawl = await fetch(`http://127.0.0.1:${brokerPort}/firecrawl/v1/scrape?debug=1`, { method: "POST", - headers, - body: "{}", + headers: { + "Content-Type": "application/json", + "x-api-key": "refresh-2", + }, + body: JSON.stringify({ url: "https://example.com" }), }); - expect(resp.status).toBe(200); - } - expect(upstreamRequests[1]).toMatchObject({ - url: "/browsers", - browserUseApiKey: "access-2", - }); - expect(upstreamRequests[1]?.authorization).toBeUndefined(); - expect(upstreamRequests[2]).toMatchObject({ - url: "/fal-ai/test", - authorization: "Key access-2", - }); - expect(upstreamRequests[3]).toMatchObject({ - url: "/v1/audio/speech", - authorization: "Bearer access-2", - }); - expect(upstreamRequests[4]).toMatchObject({ - url: "/sandboxes", - authorization: "Bearer access-2", - }); - expect(tokenRequests).toHaveLength(1); - expect(agentKeyRequests).toHaveLength(1); - expect(output).not.toContain("refresh-1"); - expect(output).not.toContain("refresh-2"); - expect(output).not.toContain("access-2"); - expect(output).not.toContain("sandbox-secret"); - expect(output).not.toContain("agent-key-2"); - }); + const firecrawlBody = await firecrawl.text(); + expect(firecrawl.status, `${firecrawlBody}\n${output}`).toBe(200); + expect(firecrawl.headers.get("content-encoding")).toBeNull(); + expect(firecrawl.headers.get("content-length")).toBeNull(); + expect(firecrawl.headers.get("content-md5")).toBeNull(); + expect(firecrawl.headers.get("set-cookie")).toBeNull(); + expect(JSON.parse(firecrawlBody)).toEqual({ ok: true, path: "/v1/scrape?debug=1" }); + expect(tokenRequests).toHaveLength(1); + expect(tokenRequests[0]?.refreshHeader).toBe("refresh-1"); + expect(new URLSearchParams(tokenRequests[0]?.body).get("refresh_token")).toBeNull(); + expect(new URLSearchParams(tokenRequests[0]?.body).get("grant_type")).toBe("refresh_token"); + expect(agentKeyRequests).toHaveLength(1); + expect(agentKeyRequests[0]?.authorization).toBe("Bearer access-2"); + expect(JSON.parse(agentKeyRequests[0]?.body || "{}")).toEqual({ + min_ttl_seconds: 1800, + }); + expect(upstreamRequests[0]).toMatchObject({ + url: "/v1/scrape?debug=1", + authorization: "Bearer access-2", + acceptEncoding: "identity", + }); + expect(upstreamRequests[0]?.apiKey).toBeUndefined(); - it("keeps source and destination credentials live in one broker process and unregisters only the destination", { - timeout: BROKER_TEST_TIMEOUT_MS, - }, async ({ resources }) => { - const tmp = resources.temporaryDirectory("nemoclaw-hermes-tool-broker-coexistence-"); - const stateDir = path.join(tmp, "state"); - const binDir = path.join(tmp, "bin"); - // AF_UNIX paths are short on macOS; the Vitest-owned TMPDIR itself can - // exceed that limit before the socket name is appended. - const socketDir = resources.ownDirectory(fs.mkdtempSync("/tmp/nc-hermes-broker-")); - const controlSocket = path.join(socketDir, "control.sock"); - fs.chmodSync(socketDir, 0o777); - fs.mkdirSync(stateDir, { recursive: true, mode: 0o700 }); - fs.mkdirSync(binDir, { recursive: true }); - const openshellLog = path.join(tmp, "openshell.log"); - const openshellBin = path.join(binDir, "openshell"); - fs.writeFileSync( - openshellBin, - `#!/bin/sh\nprintf '%s\\n' "$*" >> "${openshellLog}"\nexit 0\n`, - { mode: 0o755 }, - ); + const rotatedState = JSON.parse(fs.readFileSync(statePath, "utf8")); + expect(rotatedState.refresh_token_sha256).toBe(sha256("refresh-2")); + const openshellOutput = fs.readFileSync(openshellLog, "utf8"); + expect(openshellOutput).toContain( + "provider update sandbox-hermes-tool-gateway --credential NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN", + ); + expect(openshellOutput).toContain("refresh=broker-1"); + expect(openshellOutput).not.toContain("refresh=refresh-2"); + expect(openshellOutput).toContain( + "provider update hermes-provider --credential OPENAI_API_KEY --config OPENAI_BASE_URL=https://inference-api.nousresearch.com/v1", + ); + expect(openshellOutput).toContain("openai=agent-key-2"); + expect(rotatedState.inference_provider_name).toBe("hermes-provider"); + expect(rotatedState.inference_credential_env).toBe("OPENAI_API_KEY"); + expect(rotatedState.inference_agent_key_expires_at).toBeTruthy(); + + const checks = [ + ["/browser-use/browsers", { "X-Browser-Use-API-Key": "broker-1" }, "browser"], + ["/fal-queue/fal-ai/test", { Authorization: "Key broker-1" }, "fal"], + ["/openai-audio/v1/audio/speech", { "openai-api-key": "broker-1" }, "audio"], + ["/modal/sandboxes", { Authorization: "Bearer broker-1" }, "modal"], + ] as const; + for (const [route, headers] of checks) { + const resp = await fetch(`http://127.0.0.1:${brokerPort}${route}`, { + method: "POST", + headers, + body: "{}", + }); + expect(resp.status).toBe(200); + } + expect(upstreamRequests[1]).toMatchObject({ + url: "/browsers", + browserUseApiKey: "access-2", + }); + expect(upstreamRequests[1]?.authorization).toBeUndefined(); + expect(upstreamRequests[2]).toMatchObject({ + url: "/fal-ai/test", + authorization: "Key access-2", + }); + expect(upstreamRequests[3]).toMatchObject({ + url: "/v1/audio/speech", + authorization: "Bearer access-2", + }); + expect(upstreamRequests[4]).toMatchObject({ + url: "/sandboxes", + authorization: "Bearer access-2", + }); + expect(tokenRequests).toHaveLength(1); + expect(agentKeyRequests).toHaveLength(1); + expect(output).not.toContain("refresh-1"); + expect(output).not.toContain("refresh-2"); + expect(output).not.toContain("access-2"); + expect(output).not.toContain("sandbox-secret"); + expect(output).not.toContain("agent-key-2"); + }, + ); - const writeState = ( - sandbox: string, - brokerToken: string, - refreshToken: string, - inferenceProviderName: string, - ): void => { + it( + "keeps source and destination credentials live in one broker process and unregisters only the destination", + { + timeout: BROKER_TEST_TIMEOUT_MS, + }, + async ({ resources }) => { + const tmp = resources.temporaryDirectory("nemoclaw-hermes-tool-broker-coexistence-"); + const stateDir = path.join(tmp, "state"); + const binDir = path.join(tmp, "bin"); + // AF_UNIX paths are short on macOS; the Vitest-owned TMPDIR itself can + // exceed that limit before the socket name is appended. + const socketDir = resources.ownDirectory(fs.mkdtempSync("/tmp/nc-hermes-broker-")); + const controlSocket = path.join(socketDir, "control.sock"); + fs.chmodSync(socketDir, 0o777); + fs.mkdirSync(stateDir, { recursive: true, mode: 0o700 }); + fs.mkdirSync(binDir, { recursive: true }); + const openshellLog = path.join(tmp, "openshell.log"); + const openshellBin = path.join(binDir, "openshell"); fs.writeFileSync( - path.join(stateDir, `${sandbox}.json`), - JSON.stringify( - { - version: 1, - sandbox, - provider_name: `${sandbox}-hermes-tool-gateway`, - inference_provider_name: inferenceProviderName, - inference_credential_env: "OPENAI_API_KEY", - credential_env: "NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN", - broker_token: brokerToken, - broker_token_sha256: sha256(brokerToken), - refresh_token_sha256: sha256(refreshToken), - client_id: "hermes-cli", - }, - null, - 2, + openshellBin, + `#!/bin/sh\nprintf '%s\\n' "$*" >> "${openshellLog}"\nexit 0\n`, + { mode: 0o755 }, + ); + + const writeState = ( + sandbox: string, + brokerToken: string, + refreshToken: string, + inferenceProviderName: string, + ): void => { + fs.writeFileSync( + path.join(stateDir, `${sandbox}.json`), + JSON.stringify( + { + version: 1, + sandbox, + provider_name: `${sandbox}-hermes-tool-gateway`, + inference_provider_name: inferenceProviderName, + inference_credential_env: "OPENAI_API_KEY", + credential_env: "NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN", + broker_token: brokerToken, + broker_token_sha256: sha256(brokerToken), + refresh_token_sha256: sha256(refreshToken), + client_id: "hermes-cli", + }, + null, + 2, + ), + { mode: 0o600 }, + ); + }; + writeState("source", "source-broker-token", "source-refresh-token", "hermes-provider"); + writeState( + "destination", + "destination-broker-token", + "destination-refresh-token", + "destination-hermes-inference", + ); + + const refreshHeaders: string[] = []; + const portal = resources.ownServer( + http.createServer((req, res) => + handleHermesBrokerCoexistencePortal(refreshHeaders, req, res), ), - { mode: 0o600 }, ); - }; - writeState("source", "source-broker-token", "source-refresh-token", "hermes-provider"); - writeState( - "destination", - "destination-broker-token", - "destination-refresh-token", - "destination-hermes-inference", - ); + const portalPort = await listen(portal); - const refreshHeaders: string[] = []; - const portal = resources.ownServer( - http.createServer((req, res) => - handleHermesBrokerCoexistencePortal(refreshHeaders, req, res), - ), - ); - const portalPort = await listen(portal); + const upstreamAuthorizations: string[] = []; + const upstream = resources.ownServer( + http.createServer((req, res) => { + upstreamAuthorizations.push(String(req.headers.authorization || "")); + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ ok: true })); + }), + ); + const upstreamPort = await listen(upstream); + const matrixPath = path.join(tmp, "matrix.json"); + fs.writeFileSync( + matrixPath, + JSON.stringify({ + "nous-web": { + service: "firecrawl", + upstream: `http://127.0.0.1:${upstreamPort}`, + }, + }), + ); + const brokerPort = await freePort(); + const child = resources.ownChild( + spawn(process.execPath, ["--experimental-strip-types", SCRIPT], { + env: { + ...process.env, + HERMES_TOOL_GATEWAY_PORT: String(brokerPort), + HERMES_TOOL_GATEWAY_STATE_DIR: stateDir, + HERMES_TOOL_GATEWAY_MATRIX_PATH: matrixPath, + HERMES_TOOL_GATEWAY_CONTROL_SOCKET: controlSocket, + HERMES_INFERENCE_AGENT_KEY_REFRESH_INTERVAL_MS: "3600000", + NOUS_PORTAL_BASE_URL: `http://127.0.0.1:${portalPort}`, + NEMOCLAW_OPENSHELL_BIN: openshellBin, + NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN: "source-refresh-token", + }, + stdio: ["ignore", "pipe", "pipe"], + }), + ); - const upstreamAuthorizations: string[] = []; - const upstream = resources.ownServer( - http.createServer((req, res) => { - upstreamAuthorizations.push(String(req.headers.authorization || "")); - res.writeHead(200, { "Content-Type": "application/json" }); - res.end(JSON.stringify({ ok: true })); - }), - ); - const upstreamPort = await listen(upstream); - const matrixPath = path.join(tmp, "matrix.json"); - fs.writeFileSync( - matrixPath, - JSON.stringify({ - "nous-web": { - service: "firecrawl", - upstream: `http://127.0.0.1:${upstreamPort}`, - }, - }), - ); - const brokerPort = await freePort(); - const child = resources.ownChild( - spawn(process.execPath, ["--experimental-strip-types", SCRIPT], { - env: { - ...process.env, - HERMES_TOOL_GATEWAY_PORT: String(brokerPort), - HERMES_TOOL_GATEWAY_STATE_DIR: stateDir, - HERMES_TOOL_GATEWAY_MATRIX_PATH: matrixPath, - HERMES_TOOL_GATEWAY_CONTROL_SOCKET: controlSocket, - HERMES_INFERENCE_AGENT_KEY_REFRESH_INTERVAL_MS: "3600000", - NOUS_PORTAL_BASE_URL: `http://127.0.0.1:${portalPort}`, - NEMOCLAW_OPENSHELL_BIN: openshellBin, - NEMOCLAW_HERMES_TOOL_GATEWAY_REFRESH_TOKEN: "source-refresh-token", + let output = ""; + child.stdout.on("data", (chunk) => { + output += chunk.toString(); + }); + child.stderr.on("data", (chunk) => { + output += chunk.toString(); + }); + await waitForBrokerCondition( + "broker and private control socket", + child, + () => output, + async () => { + const response = await fetch(`http://127.0.0.1:${brokerPort}/health`, { + signal: AbortSignal.timeout(1_000), + }); + return ( + response.status === 200 && + fs.existsSync(controlSocket) && + (fs.statSync(controlSocket).mode & 0o777) === 0o600 && + (fs.statSync(socketDir).mode & 0o777) === 0o700 + ); }, - stdio: ["ignore", "pipe", "pipe"], - }), - ); + ); - let output = ""; - child.stdout.on("data", (chunk) => { - output += chunk.toString(); - }); - child.stderr.on("data", (chunk) => { - output += chunk.toString(); - }); - await waitForBrokerCondition( - "broker and private control socket", - child, - () => output, - async () => { - const response = await fetch(`http://127.0.0.1:${brokerPort}/health`, { - signal: AbortSignal.timeout(1_000), + const proxy = (brokerToken: string) => + fetch(`http://127.0.0.1:${brokerPort}/firecrawl/v1/scrape`, { + method: "POST", + headers: { Authorization: `Bearer ${brokerToken}` }, + body: "{}", }); - return ( - response.status === 200 && - fs.existsSync(controlSocket) && - (fs.statSync(controlSocket).mode & 0o777) === 0o600 && - (fs.statSync(socketDir).mode & 0o777) === 0o700 - ); - }, - ); - - const proxy = (brokerToken: string) => - fetch(`http://127.0.0.1:${brokerPort}/firecrawl/v1/scrape`, { - method: "POST", - headers: { Authorization: `Bearer ${brokerToken}` }, - body: "{}", - }); - expect((await proxy("source-broker-token")).status).toBe(200); - await expect( - controlRequest(controlSocket, "/credentials/register", { - sandbox: "destination", - refresh_token: "destination-refresh-token", - }), - ).resolves.toMatchObject({ status: 200 }); - expect((await proxy("destination-broker-token")).status).toBe(200); + expect((await proxy("source-broker-token")).status).toBe(200); + await expect( + controlRequest(controlSocket, "/credentials/register", { + sandbox: "destination", + refresh_token: "destination-refresh-token", + }), + ).resolves.toMatchObject({ status: 200 }); + expect((await proxy("destination-broker-token")).status).toBe(200); - const stagedPayload = { - sandbox: "staged", - refresh_token: "staged-refresh-token", - inference_provider_name: "staged-hermes-inference", - request_id: `nc_clone_${"3".repeat(32)}`, - deadline_at_ms: Date.now() + 120_000, - }; - const stagedResponse = await controlRequest(controlSocket, "/credentials/stage", stagedPayload); - expect(stagedResponse.status, `${stagedResponse.body}\n${output}`).toBe(200); - const staged = JSON.parse(stagedResponse.body) as { - activation_token: string; - broker_token: string; - }; - expect(staged.activation_token).toMatch(/^nc_activate_/u); - expect(staged.broker_token).toMatch(/^nc_broker_/u); - const repeatedStage = await controlRequest(controlSocket, "/credentials/stage", stagedPayload); - expect(repeatedStage.status).toBe(200); - expect(JSON.parse(repeatedStage.body)).toMatchObject(staged); - writeState("staged", staged.broker_token, "staged-refresh-token", "staged-hermes-inference"); - await expect( - controlRequest(controlSocket, "/credentials/activate", { + const stagedPayload = { sandbox: "staged", - activation_token: staged.activation_token, + refresh_token: "staged-refresh-token", + inference_provider_name: "staged-hermes-inference", + request_id: `nc_clone_${"3".repeat(32)}`, deadline_at_ms: Date.now() + 120_000, - }), - ).resolves.toMatchObject({ status: 200 }); - await expect( - controlRequest(controlSocket, "/credentials/activate", { - sandbox: "staged", - activation_token: staged.activation_token, - deadline_at_ms: Date.now() + 120_000, - }), - ).resolves.toMatchObject({ status: 200 }); - await expect( - controlRequest(controlSocket, "/credentials/status", { - activation_token: staged.activation_token, - }), - ).resolves.toMatchObject({ status: 200 }); + }; + const stagedResponse = await controlRequest( + controlSocket, + "/credentials/stage", + stagedPayload, + ); + expect(stagedResponse.status, `${stagedResponse.body}\n${output}`).toBe(200); + const staged = JSON.parse(stagedResponse.body) as { + activation_token: string; + broker_token: string; + }; + expect(staged.activation_token).toMatch(/^nc_activate_/u); + expect(staged.broker_token).toMatch(/^nc_broker_/u); + const repeatedStage = await controlRequest( + controlSocket, + "/credentials/stage", + stagedPayload, + ); + expect(repeatedStage.status).toBe(200); + expect(JSON.parse(repeatedStage.body)).toMatchObject(staged); + writeState("staged", staged.broker_token, "staged-refresh-token", "staged-hermes-inference"); + await expect( + controlRequest(controlSocket, "/credentials/activate", { + sandbox: "staged", + activation_token: staged.activation_token, + deadline_at_ms: Date.now() + 120_000, + }), + ).resolves.toMatchObject({ status: 200 }); + await expect( + controlRequest(controlSocket, "/credentials/activate", { + sandbox: "staged", + activation_token: staged.activation_token, + deadline_at_ms: Date.now() + 120_000, + }), + ).resolves.toMatchObject({ status: 200 }); + await expect( + controlRequest(controlSocket, "/credentials/status", { + activation_token: staged.activation_token, + }), + ).resolves.toMatchObject({ status: 200 }); - const discardedPayload = { - sandbox: "discarded", - refresh_token: "discarded-refresh-token", - inference_provider_name: "discarded-hermes-inference", - request_id: `nc_clone_${"4".repeat(32)}`, - deadline_at_ms: Date.now() + 120_000, - }; - const discardedStage = await controlRequest( - controlSocket, - "/credentials/stage", - discardedPayload, - ); - expect(discardedStage.status, `${discardedStage.body}\n${output}`).toBe(200); - const discarded = JSON.parse(discardedStage.body) as { activation_token: string }; - await expect( - controlRequest(controlSocket, "/credentials/discard", { + const discardedPayload = { sandbox: "discarded", - activation_token: discarded.activation_token, - }), - ).resolves.toMatchObject({ status: 200 }); - await expect( - controlRequest(controlSocket, "/credentials/discard", { - sandbox: "discarded", - activation_token: discarded.activation_token, - }), - ).resolves.toMatchObject({ status: 200 }); - await expect( - controlRequest(controlSocket, "/credentials/stage", discardedPayload), - ).resolves.toMatchObject({ status: 400 }); - - await expect( - controlRequest(controlSocket, "/credentials/stage", { - sandbox: "deadline", - refresh_token: "deadline-refresh-token", - inference_provider_name: "deadline-hermes-inference", - request_id: `nc_clone_${"5".repeat(32)}`, - deadline_at_ms: Date.now() + 50, - }), - ).resolves.toMatchObject({ status: 400 }); - await expect( - controlRequest(controlSocket, "/credentials/stage", { - sandbox: "Invalid_Sandbox", - refresh_token: "must-not-reach-portal", - inference_provider_name: "valid-hermes-inference", - request_id: `nc_clone_${"6".repeat(32)}`, + refresh_token: "discarded-refresh-token", + inference_provider_name: "discarded-hermes-inference", + request_id: `nc_clone_${"4".repeat(32)}`, deadline_at_ms: Date.now() + 120_000, - }), - ).resolves.toMatchObject({ status: 400 }); - expect(refreshHeaders).not.toContain("must-not-reach-portal"); - expect((await proxy(staged.broker_token)).status).toBe(200); + }; + const discardedStage = await controlRequest( + controlSocket, + "/credentials/stage", + discardedPayload, + ); + expect(discardedStage.status, `${discardedStage.body}\n${output}`).toBe(200); + const discarded = JSON.parse(discardedStage.body) as { activation_token: string }; + await expect( + controlRequest(controlSocket, "/credentials/discard", { + sandbox: "discarded", + activation_token: discarded.activation_token, + }), + ).resolves.toMatchObject({ status: 200 }); + await expect( + controlRequest(controlSocket, "/credentials/discard", { + sandbox: "discarded", + activation_token: discarded.activation_token, + }), + ).resolves.toMatchObject({ status: 200 }); + await expect( + controlRequest(controlSocket, "/credentials/stage", discardedPayload), + ).resolves.toMatchObject({ status: 400 }); - await expect( - controlRequest(controlSocket, "/credentials/unregister", { - sandbox: "destination", - }), - ).resolves.toMatchObject({ status: 200 }); - expect((await proxy("destination-broker-token")).status).toBe(401); - expect((await proxy("source-broker-token")).status).toBe(200); + await expect( + controlRequest(controlSocket, "/credentials/stage", { + sandbox: "deadline", + refresh_token: "deadline-refresh-token", + inference_provider_name: "deadline-hermes-inference", + request_id: `nc_clone_${"5".repeat(32)}`, + deadline_at_ms: Date.now() + 50, + }), + ).resolves.toMatchObject({ status: 400 }); + await expect( + controlRequest(controlSocket, "/credentials/stage", { + sandbox: "Invalid_Sandbox", + refresh_token: "must-not-reach-portal", + inference_provider_name: "valid-hermes-inference", + request_id: `nc_clone_${"6".repeat(32)}`, + deadline_at_ms: Date.now() + 120_000, + }), + ).resolves.toMatchObject({ status: 400 }); + expect(refreshHeaders).not.toContain("must-not-reach-portal"); + expect((await proxy(staged.broker_token)).status).toBe(200); - expect(upstreamAuthorizations).toEqual([ - "Bearer access-source", - "Bearer access-destination", - "Bearer access-staged", - "Bearer access-source", - ]); - expect(refreshHeaders).toEqual( - expect.arrayContaining(["source-refresh-token", "destination-refresh-token"]), - ); - const openshellUpdates = fs.readFileSync(openshellLog, "utf8"); - expect(openshellUpdates).toContain( - "provider update hermes-provider --credential OPENAI_API_KEY", - ); - expect(openshellUpdates).toContain( - "provider update destination-hermes-inference --credential OPENAI_API_KEY", - ); - expect(openshellUpdates).toContain( - "provider update staged-hermes-inference --credential OPENAI_API_KEY", - ); - expect(output).not.toContain("source-refresh-token"); - expect(output).not.toContain("destination-refresh-token"); + await expect( + controlRequest(controlSocket, "/credentials/unregister", { + sandbox: "destination", + }), + ).resolves.toMatchObject({ status: 200 }); + expect((await proxy("destination-broker-token")).status).toBe(401); + expect((await proxy("source-broker-token")).status).toBe(200); - const openIncompleteControlRequest = async (): Promise => { - const socket = net.createConnection(controlSocket); - socket.on("error", () => {}); - await once(socket, "connect"); - socket.write( - [ - "POST /credentials/register HTTP/1.1", - "Host: localhost", - "Content-Type: application/json", - "Content-Length: 200", - "Connection: keep-alive", - "", - '{"sandbox":"destination"', - ].join("\r\n"), + expect(upstreamAuthorizations).toEqual([ + "Bearer access-source", + "Bearer access-destination", + "Bearer access-staged", + "Bearer access-source", + ]); + expect(refreshHeaders).toEqual( + expect.arrayContaining(["source-refresh-token", "destination-refresh-token"]), ); - return socket; - }; + const openshellUpdates = fs.readFileSync(openshellLog, "utf8"); + expect(openshellUpdates).toContain( + "provider update hermes-provider --credential OPENAI_API_KEY", + ); + expect(openshellUpdates).toContain( + "provider update destination-hermes-inference --credential OPENAI_API_KEY", + ); + expect(openshellUpdates).toContain( + "provider update staged-hermes-inference --credential OPENAI_API_KEY", + ); + expect(output).not.toContain("source-refresh-token"); + expect(output).not.toContain("destination-refresh-token"); + + const openIncompleteControlRequest = async (): Promise => { + const socket = net.createConnection(controlSocket); + socket.on("error", () => {}); + await once(socket, "connect"); + socket.write( + [ + "POST /credentials/register HTTP/1.1", + "Host: localhost", + "Content-Type: application/json", + "Content-Length: 200", + "Connection: keep-alive", + "", + '{"sandbox":"destination"', + ].join("\r\n"), + ); + return socket; + }; - const timedOutRequest = await openIncompleteControlRequest(); - const requestTimeoutStartedAt = Date.now(); - await once(timedOutRequest, "close", { signal: AbortSignal.timeout(3_000) }); - expect(Date.now() - requestTimeoutStartedAt).toBeLessThan(3_000); + const timedOutRequest = await openIncompleteControlRequest(); + const requestTimeoutStartedAt = Date.now(); + await once(timedOutRequest, "close", { signal: AbortSignal.timeout(3_000) }); + expect(Date.now() - requestTimeoutStartedAt).toBeLessThan(3_000); - const shutdownRequest = await openIncompleteControlRequest(); - const childExit = once(child, "exit", { signal: AbortSignal.timeout(3_000) }); - const shutdownStartedAt = Date.now(); - child.kill("SIGTERM"); - await childExit; - shutdownRequest.destroy(); - expect(Date.now() - shutdownStartedAt).toBeLessThan(3_000); - }); + const shutdownRequest = await openIncompleteControlRequest(); + const childExit = once(child, "exit", { signal: AbortSignal.timeout(3_000) }); + const shutdownStartedAt = Date.now(); + child.kill("SIGTERM"); + await childExit; + shutdownRequest.destroy(); + expect(Date.now() - shutdownStartedAt).toBeLessThan(3_000); + }, + ); }); From 9e7afd28928d37e74a31ca954ea5bb7f0e53ddba Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 19 Aug 2026 15:58:44 -0700 Subject: [PATCH 2/5] fix(hermes): bind broker identity to listener Signed-off-by: Apurv Kumaria --- src/lib/hermes-tool-gateway-broker.ts | 19 +- test/hermes-tool-gateway-broker.test.ts | 257 +++++++++++++++++++++--- 2 files changed, 240 insertions(+), 36 deletions(-) diff --git a/src/lib/hermes-tool-gateway-broker.ts b/src/lib/hermes-tool-gateway-broker.ts index 0d0a28cbeee..1c92622ae76 100644 --- a/src/lib/hermes-tool-gateway-broker.ts +++ b/src/lib/hermes-tool-gateway-broker.ts @@ -555,7 +555,7 @@ function preflightHermesToolGatewayCloneBinding(sandboxName) { } const pid = readPid(); - const currentBrokerOwned = isHermesToolGatewayBrokerProcess(pid); + const currentBrokerOwned = isHermesToolGatewayBrokerPortOwner(pid); const currentBrokerHealthy = isHermesToolGatewayBrokerHealthy(); if (currentBrokerHealthy && !currentBrokerOwned) { throw new Error("Hermes managed-tool broker health endpoint is not owned by NemoClaw"); @@ -650,6 +650,16 @@ function isHermesToolGatewayBrokerProcess(pid) { return Boolean(cmdline && cmdline.includes("tool-gateway-broker.ts")); } +function isHermesToolGatewayBrokerPortOwner(pid) { + if (!isHermesToolGatewayBrokerProcess(pid)) return false; + const listenerPids = runCapture(["lsof", "-ti", `:${HERMES_TOOL_GATEWAY_PORT}`, "-sTCP:LISTEN"], { + ignoreError: true, + }) + .split(/\r?\n/u) + .map((line) => Number.parseInt(line.trim(), 10)); + return listenerPids.includes(pid); +} + function isHermesToolGatewayBrokerHealthy() { const result = run( [ @@ -738,7 +748,7 @@ function ensureHermesToolGatewayBroker(options = {}) { const desiredHash = brokerRuntimeHash(); const hashMatches = readBrokerHash() === desiredHash; const pid = readPid(); - const currentBrokerOwned = isHermesToolGatewayBrokerProcess(pid); + const currentBrokerOwned = isHermesToolGatewayBrokerPortOwner(pid); const brokerHealthy = isHermesToolGatewayBrokerHealthy(); const currentBrokerHealthy = currentBrokerOwned && brokerHealthy; // `/health` is unauthenticated on a fixed port, so reachability proves @@ -763,7 +773,7 @@ function ensureHermesToolGatewayBroker(options = {}) { const nextPid = spawnHermesToolGatewayBroker(""); for (let attempt = 0; attempt < 20; attempt++) { if ( - isHermesToolGatewayBrokerProcess(nextPid) && + isHermesToolGatewayBrokerPortOwner(nextPid) && isHermesToolGatewayBrokerHealthy() && fs.existsSync(HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH) ) { @@ -800,7 +810,7 @@ function ensureHermesToolGatewayBroker(options = {}) { const nextPid = spawnHermesToolGatewayBroker(refreshToken, options.sandboxName ?? null); for (let attempt = 0; attempt < 20; attempt++) { if ( - isHermesToolGatewayBrokerProcess(nextPid) && + isHermesToolGatewayBrokerPortOwner(nextPid) && isHermesToolGatewayBrokerHealthy() && registerHermesToolGatewayRuntimeCredential(refreshToken, options.sandboxName ?? null) ) { @@ -909,6 +919,7 @@ module.exports = { discardHermesToolGatewayCloneBinding, bindHermesToolGatewayCloneProviderState, planHermesToolGatewayBrokerRefresh, + brokerRuntimeHash, isHermesToolGatewayBrokerHealthy, killStaleHermesToolGatewayBroker, ensureHermesToolGatewayBroker, diff --git a/test/hermes-tool-gateway-broker.test.ts b/test/hermes-tool-gateway-broker.test.ts index e2d13e453da..17a267781e1 100644 --- a/test/hermes-tool-gateway-broker.test.ts +++ b/test/hermes-tool-gateway-broker.test.ts @@ -13,7 +13,12 @@ import path from "node:path"; import zlib from "node:zlib"; import { vi } from "vitest"; import { handleHermesBrokerCoexistencePortal } from "./helpers/hermes-tool-gateway-broker-fixture"; -import { describe, expect, test as it } from "./helpers/owned-test-resources"; +import { + describe, + expect, + type OwnedTestResources, + test as it, +} from "./helpers/owned-test-resources"; import { testTimeout } from "./helpers/timeouts"; const SCRIPT = path.join( @@ -73,6 +78,102 @@ function listen(server: http.Server): Promise { }); } +async function startForeignHealthListener( + resources: OwnedTestResources, + port: number, +): Promise { + const source = [ + 'const http = require("node:http");', + "const port = Number(process.argv[1]);", + "const listener = http.createServer((_request, response) => {", + ' response.writeHead(200, { "content-type": "application/json" });', + ' response.end("{\\\"ok\\\":true}");', + "});", + 'listener.once("error", (error) => {', + " process.stderr.write(`port ${port} ${error.code || error.message}\\n`);", + " process.exit(1);", + "});", + 'listener.listen(port, "127.0.0.1", () => process.stdout.write("ready\\n"));', + 'process.once("SIGTERM", () => listener.close(() => process.exit(0)));', + ].join("\n"); + const child = resources.ownChild( + spawn(process.execPath, ["--input-type=commonjs", "--eval", source, String(port)], { + stdio: ["ignore", "pipe", "pipe"], + }), + ); + let output = ""; + child.stdout?.on("data", (chunk) => { + output += chunk.toString(); + }); + child.stderr?.on("data", (chunk) => { + output += chunk.toString(); + }); + await waitForBrokerCondition( + "foreign health listener", + child, + () => output, + () => output.includes("ready"), + ); + return child; +} + +async function startBrokerLikeListener( + resources: OwnedTestResources, + port: number, + controlSocket: string, +): Promise { + const source = [ + 'const http = require("node:http");', + "const [, portValue, controlSocket] = process.argv.slice(1);", + "const listener = http.createServer((_request, response) => {", + ' response.writeHead(200, { "content-type": "application/json" });', + ' response.end("{\\\"ok\\\":true}");', + "});", + "const control = http.createServer((_request, response) => {", + " response.writeHead(200);", + ' response.end("{}");', + "});", + 'listener.once("error", (error) => {', + " process.stderr.write(`${error.code || error.message}\\n`);", + " process.exit(1);", + "});", + 'listener.listen(Number(portValue), "127.0.0.1", () => {', + ' control.listen(controlSocket, () => process.stdout.write("ready\\n"));', + "});", + 'process.once("SIGTERM", () => {', + " listener.close(() => control.close(() => process.exit(0)));", + "});", + ].join("\n"); + const child = resources.ownChild( + spawn( + process.execPath, + [ + "--input-type=commonjs", + "--eval", + source, + "tool-gateway-broker.ts", + String(port), + controlSocket, + ], + { stdio: ["ignore", "pipe", "pipe"] }, + ), + ); + let output = ""; + child.stdout?.on("data", (chunk) => { + output += chunk.toString(); + }); + child.stderr?.on("data", (chunk) => { + output += chunk.toString(); + }); + await waitForBrokerCondition( + "broker-like listener", + child, + () => output, + () => output.includes("ready"), + ); + return child; +} + function brokerDiagnostics(child: ChildProcess, output: () => string): string { const captured = output().trim() || ""; return [ @@ -234,47 +335,138 @@ describe("Hermes managed-tool gateway broker", () => { }); it( - "refuses a healthy listener on the managed-tool port that it does not own", + "reuses its listener and rechecks ownership after the process exits", async ({ resources, skip }) => { - const { home } = resources.home("nemoclaw-broker-ownership-"); + const home = resources.ownDirectory(fs.mkdtempSync("/tmp/nc-broker-ownership-")); vi.stubEnv("HOME", home); delete require.cache[require.resolve(BROKER_WRAPPER)]; const broker = require(BROKER_WRAPPER); + const credsDir = path.dirname(broker.HERMES_TOOL_GATEWAY_STATE_DIR); + const pidPath = path.join(credsDir, "hermes-tool-gateway-broker.pid"); + const hashPath = path.join(credsDir, "hermes-tool-gateway-broker.hash"); + fs.mkdirSync(credsDir, { recursive: true, mode: 0o700 }); - const impostor = resources.ownServer( - http.createServer((_req, res) => { - res.writeHead(200, { "Content-Type": "application/json" }); - res.end(JSON.stringify({ ok: true, services: [] })); - }), + let ownedBroker: ChildProcess; + try { + ownedBroker = await startBrokerLikeListener( + resources, + broker.HERMES_TOOL_GATEWAY_PORT, + broker.HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH, + ); + } catch (error) { + skip( + String(error).includes("EADDRINUSE"), + `port ${String(broker.HERMES_TOOL_GATEWAY_PORT)} is already held`, + ); + throw error; + } + + try { + fs.writeFileSync(pidPath, `${String(ownedBroker.pid)}\n`, { mode: 0o600 }); + fs.writeFileSync(hashPath, `${broker.brokerRuntimeHash()}\n`, { mode: 0o600 }); + expect(broker.ensureHermesToolGatewayBroker({ startWithoutCredential: true })).toBe(true); + + const ownedBrokerExit = once(ownedBroker, "exit"); + ownedBroker.kill("SIGTERM"); + await ownedBrokerExit; + + try { + await startForeignHealthListener(resources, broker.HERMES_TOOL_GATEWAY_PORT); + } catch (error) { + skip( + String(error).includes("EADDRINUSE"), + `port ${String(broker.HERMES_TOOL_GATEWAY_PORT)} is already held`, + ); + throw error; + } + + const refusal = vi.spyOn(console, "error").mockImplementation(() => {}); + expect(broker.ensureHermesToolGatewayBroker({ startWithoutCredential: true })).toBe(false); + expect(refusal.mock.calls.flat().join("\n")).toContain( + String(broker.HERMES_TOOL_GATEWAY_PORT), + ); + } finally { + delete require.cache[require.resolve(BROKER_WRAPPER)]; + } + }, + BROKER_TEST_TIMEOUT_MS, + ); + + it( + "refuses a live recorded process that does not own the managed-tool port", + async ({ resources, skip }) => { + const home = resources.ownDirectory(fs.mkdtempSync("/tmp/nc-broker-port-owner-")); + vi.stubEnv("HOME", home); + delete require.cache[require.resolve(BROKER_WRAPPER)]; + const broker = require(BROKER_WRAPPER); + + try { + await startForeignHealthListener(resources, broker.HERMES_TOOL_GATEWAY_PORT); + } catch (error) { + skip( + String(error).includes("EADDRINUSE"), + `port ${String(broker.HERMES_TOOL_GATEWAY_PORT)} is already held`, + ); + throw error; + } + + const brokerLikeProcess = resources.ownChild( + spawn( + process.execPath, + [ + "--input-type=commonjs", + "--eval", + 'process.stdout.write("ready\\n"); setInterval(() => {}, 1_000);', + "tool-gateway-broker.ts", + ], + { stdio: ["ignore", "pipe", "pipe"] }, + ), ); - await new Promise((resolve, reject) => { - impostor.once("error", reject); - impostor.listen(broker.HERMES_TOOL_GATEWAY_PORT, "127.0.0.1", () => resolve()); + let brokerLikeOutput = ""; + brokerLikeProcess.stdout?.on("data", (chunk) => { + brokerLikeOutput += chunk.toString(); }); - - // The health probe shells out to curl. Where a harness blocks loopback - // HTTP for subprocesses no listener reads as healthy, so report that gap - // instead of asserting nothing. - skip( - !broker.isHermesToolGatewayBrokerHealthy(), - "curl cannot read a loopback response in this environment", + await waitForBrokerCondition( + "broker-like non-listener", + brokerLikeProcess, + () => brokerLikeOutput, + () => brokerLikeOutput.includes("ready"), ); - const refusal = vi.spyOn(console, "error").mockImplementation(() => {}); - // Reachability is not identity: `/health` is unauthenticated, so an - // unowned listener must never be adopted as this run's broker. - expect(broker.ensureHermesToolGatewayBroker({})).toBe(false); - const diagnostics = refusal.mock.calls.map((call) => call.join(" ")).join("\n"); - - // The refusal has to name the port it declined, and it must leave no pid - // record, which would mean a broker was spawned against the held port. - expect(diagnostics).toContain(String(broker.HERMES_TOOL_GATEWAY_PORT)); - const pidPath = path.join( - path.dirname(broker.HERMES_TOOL_GATEWAY_STATE_DIR), - "hermes-tool-gateway-broker.pid", + const credsDir = path.dirname(broker.HERMES_TOOL_GATEWAY_STATE_DIR); + fs.mkdirSync(credsDir, { recursive: true, mode: 0o700 }); + const pidPath = path.join(credsDir, "hermes-tool-gateway-broker.pid"); + fs.writeFileSync(pidPath, `${String(brokerLikeProcess.pid)}\n`, { mode: 0o600 }); + let controlRequests = 0; + const control = resources.ownServer( + http.createServer((_request, response) => { + controlRequests += 1; + response.writeHead(200); + response.end("{}"); + }), ); - expect(fs.existsSync(pidPath)).toBe(false); - delete require.cache[require.resolve(BROKER_WRAPPER)]; + await new Promise((resolve, reject) => { + control.once("error", reject); + control.listen(broker.HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH, () => resolve()); + }); + + try { + const refusal = vi.spyOn(console, "error").mockImplementation(() => {}); + // Process identity without listener ownership cannot authorize a + // credential registration through the private control socket. + expect( + broker.ensureHermesToolGatewayBroker({ + refreshToken: "test-only-refresh", + sandboxName: "sandbox", + }), + ).toBe(false); + expect(refusal.mock.calls.flat().join("\n")).toContain( + String(broker.HERMES_TOOL_GATEWAY_PORT), + ); + expect(controlRequests).toBe(0); + } finally { + delete require.cache[require.resolve(BROKER_WRAPPER)]; + } }, BROKER_TEST_TIMEOUT_MS, ); @@ -829,6 +1021,7 @@ describe("Hermes managed-tool gateway broker", () => { // exceed that limit before the socket name is appended. const socketDir = resources.ownDirectory(fs.mkdtempSync("/tmp/nc-hermes-broker-")); const controlSocket = path.join(socketDir, "control.sock"); + // The broad starting mode proves that broker startup narrows the directory to 0o700. fs.chmodSync(socketDir, 0o777); fs.mkdirSync(stateDir, { recursive: true, mode: 0o700 }); fs.mkdirSync(binDir, { recursive: true }); From 6a6ccf3b7d7dca3c96876c6842dfd7b54b7c5278 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 19 Aug 2026 17:49:55 -0700 Subject: [PATCH 3/5] fix(hermes): diagnose broker ownership checks Signed-off-by: Apurv Kumaria --- src/lib/hermes-tool-gateway-broker.ts | 35 ++++++++++++++++++----- test/hermes-tool-gateway-broker.test.ts | 37 +++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/src/lib/hermes-tool-gateway-broker.ts b/src/lib/hermes-tool-gateway-broker.ts index 1c92622ae76..5839ebc7280 100644 --- a/src/lib/hermes-tool-gateway-broker.ts +++ b/src/lib/hermes-tool-gateway-broker.ts @@ -10,7 +10,7 @@ const os = require("os"); const path = require("path"); const { spawn, spawnSync } = require("child_process"); -const { ROOT, run, runCapture, validateName } = require("./runner"); +const { ROOT, run, runCapture, runCaptureEx, validateName } = require("./runner"); const { buildSubprocessEnv } = require("./subprocess-env"); const { getCredsDir } = require("./credentials/store"); const oauth = require("./oauth-device-code"); @@ -65,6 +65,7 @@ const HERMES_TOOL_GATEWAY_RUNTIME_MISMATCH_RECOVERY = "Reauthorize every managed-tool Hermes sandbox, then retry."; const HERMES_TOOL_GATEWAY_UNOWNED_LISTENER_RECOVERY = "Stop the process holding that port, then retry."; +let reportedMissingListenerInspector = false; const HERMES_TOOL_GATEWAY_CONTROL_CLIENT_SOURCE = [ 'const http = require("node:http");', "const [socketPath, route, timeoutValue] = process.argv.slice(1);", @@ -558,7 +559,11 @@ function preflightHermesToolGatewayCloneBinding(sandboxName) { const currentBrokerOwned = isHermesToolGatewayBrokerPortOwner(pid); const currentBrokerHealthy = isHermesToolGatewayBrokerHealthy(); if (currentBrokerHealthy && !currentBrokerOwned) { - throw new Error("Hermes managed-tool broker health endpoint is not owned by NemoClaw"); + throw new Error( + "Hermes managed-tool broker health endpoint is not owned by NemoClaw; " + + `port ${HERMES_TOOL_GATEWAY_PORT} is held by another process. ` + + HERMES_TOOL_GATEWAY_UNOWNED_LISTENER_RECOVERY, + ); } if (!currentBrokerOwned || !currentBrokerHealthy) { probeHermesToolGatewayBrokerStart(); @@ -650,11 +655,26 @@ function isHermesToolGatewayBrokerProcess(pid) { return Boolean(cmdline && cmdline.includes("tool-gateway-broker.ts")); } -function isHermesToolGatewayBrokerPortOwner(pid) { - if (!isHermesToolGatewayBrokerProcess(pid)) return false; - const listenerPids = runCapture(["lsof", "-ti", `:${HERMES_TOOL_GATEWAY_PORT}`, "-sTCP:LISTEN"], { - ignoreError: true, - }) +function isHermesToolGatewayBrokerPortOwner(pid, deps = {}) { + const isBrokerProcess = deps.isBrokerProcess ?? isHermesToolGatewayBrokerProcess; + if (!isBrokerProcess(pid)) return false; + const listener = (deps.runCaptureEx ?? runCaptureEx)([ + "lsof", + "-ti", + `:${HERMES_TOOL_GATEWAY_PORT}`, + "-sTCP:LISTEN", + ]); + if (listener.exitCode === null && !listener.timedOut) { + if (!reportedMissingListenerInspector) { + (deps.reportError ?? console.error)( + "NemoClaw cannot verify Hermes managed-tool broker port ownership because lsof is " + + "unavailable. Install lsof, then retry.", + ); + reportedMissingListenerInspector = true; + } + return false; + } + const listenerPids = listener.stdout .split(/\r?\n/u) .map((line) => Number.parseInt(line.trim(), 10)); return listenerPids.includes(pid); @@ -920,6 +940,7 @@ module.exports = { bindHermesToolGatewayCloneProviderState, planHermesToolGatewayBrokerRefresh, brokerRuntimeHash, + isHermesToolGatewayBrokerPortOwner, isHermesToolGatewayBrokerHealthy, killStaleHermesToolGatewayBroker, ensureHermesToolGatewayBroker, diff --git a/test/hermes-tool-gateway-broker.test.ts b/test/hermes-tool-gateway-broker.test.ts index 17a267781e1..90f5e797168 100644 --- a/test/hermes-tool-gateway-broker.test.ts +++ b/test/hermes-tool-gateway-broker.test.ts @@ -452,6 +452,14 @@ describe("Hermes managed-tool gateway broker", () => { try { const refusal = vi.spyOn(console, "error").mockImplementation(() => {}); + let preflightError: unknown; + try { + broker.preflightHermesToolGatewayCloneBinding("clone"); + } catch (error) { + preflightError = error; + } + expect(String(preflightError)).toContain(String(broker.HERMES_TOOL_GATEWAY_PORT)); + expect(String(preflightError)).toContain("Stop the process holding that port, then retry."); // Process identity without listener ownership cannot authorize a // credential registration through the private control socket. expect( @@ -471,6 +479,35 @@ describe("Hermes managed-tool gateway broker", () => { BROKER_TEST_TIMEOUT_MS, ); + it("reports a missing listener inspector once and fails ownership closed", () => { + delete require.cache[require.resolve(BROKER_WRAPPER)]; + const broker = require(BROKER_WRAPPER); + const diagnostic = vi.fn(); + const runCaptureEx = vi + .fn() + .mockReturnValueOnce({ stdout: "", exitCode: 1, timedOut: false }) + .mockReturnValue({ stdout: "", exitCode: null, timedOut: false }); + const deps = { + isBrokerProcess: () => true, + runCaptureEx, + reportError: diagnostic, + }; + + expect(broker.isHermesToolGatewayBrokerPortOwner(42, deps)).toBe(false); + expect(diagnostic).not.toHaveBeenCalled(); + expect(broker.isHermesToolGatewayBrokerPortOwner(42, deps)).toBe(false); + expect(broker.isHermesToolGatewayBrokerPortOwner(42, deps)).toBe(false); + expect(diagnostic).toHaveBeenCalledExactlyOnceWith( + expect.stringContaining("Install lsof, then retry."), + ); + expect(runCaptureEx).toHaveBeenCalledWith([ + "lsof", + "-ti", + `:${String(broker.HERMES_TOOL_GATEWAY_PORT)}`, + "-sTCP:LISTEN", + ]); + }); + it("preserves durable state when live credential unregister fails", () => { delete require.cache[require.resolve(BROKER_WRAPPER)]; const broker = require(BROKER_WRAPPER); From 7822aacd26f8b6826ac2649c505c893b05a40137 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Thu, 20 Aug 2026 03:29:10 -0700 Subject: [PATCH 4/5] fix(cli): verify broker port before stale cleanup Signed-off-by: Carlos Villela --- src/lib/hermes-tool-gateway-broker.ts | 2 +- test/hermes-tool-gateway-broker.test.ts | 66 +++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/lib/hermes-tool-gateway-broker.ts b/src/lib/hermes-tool-gateway-broker.ts index 5839ebc7280..36b7b7ce90e 100644 --- a/src/lib/hermes-tool-gateway-broker.ts +++ b/src/lib/hermes-tool-gateway-broker.ts @@ -698,7 +698,7 @@ function isHermesToolGatewayBrokerHealthy() { function killStaleHermesToolGatewayBroker() { const pid = readPid(); - if (isHermesToolGatewayBrokerProcess(pid)) { + if (isHermesToolGatewayBrokerPortOwner(pid)) { run(["kill", String(pid)], { ignoreError: true, suppressOutput: true }); } clearPid(); diff --git a/test/hermes-tool-gateway-broker.test.ts b/test/hermes-tool-gateway-broker.test.ts index 90f5e797168..a38cef82e61 100644 --- a/test/hermes-tool-gateway-broker.test.ts +++ b/test/hermes-tool-gateway-broker.test.ts @@ -479,6 +479,72 @@ describe("Hermes managed-tool gateway broker", () => { BROKER_TEST_TIMEOUT_MS, ); + it( + "keeps a stale broker-like non-listener alive while replacement startup clears its records", + async ({ resources, skip }) => { + const home = resources.ownDirectory(fs.mkdtempSync("/tmp/nc-broker-stale-non-listener-")); + vi.stubEnv("HOME", home); + delete require.cache[require.resolve(BROKER_WRAPPER)]; + const broker = require(BROKER_WRAPPER); + const brokerLikeProcess = resources.ownChild( + spawn( + process.execPath, + [ + "--input-type=commonjs", + "--eval", + 'process.stdout.write("ready\\n"); setInterval(() => {}, 1_000);', + "tool-gateway-broker.ts", + ], + { stdio: ["ignore", "pipe", "pipe"] }, + ), + ); + let brokerLikeOutput = ""; + brokerLikeProcess.stdout?.on("data", (chunk) => { + brokerLikeOutput += chunk.toString(); + }); + await waitForBrokerCondition( + "broker-like non-listener", + brokerLikeProcess, + () => brokerLikeOutput, + () => brokerLikeOutput.includes("ready"), + ); + + const credsDir = path.dirname(broker.HERMES_TOOL_GATEWAY_STATE_DIR); + const pidPath = path.join(credsDir, "hermes-tool-gateway-broker.pid"); + const hashPath = path.join(credsDir, "hermes-tool-gateway-broker.hash"); + fs.mkdirSync(credsDir, { recursive: true, mode: 0o700 }); + fs.writeFileSync(pidPath, `${String(brokerLikeProcess.pid)}\n`, { mode: 0o600 }); + fs.writeFileSync(hashPath, "stale-hash\n", { mode: 0o600 }); + fs.writeFileSync(broker.HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH, "stale", { mode: 0o600 }); + + let replacementPid = brokerLikeProcess.pid!; + try { + try { + expect(broker.ensureHermesToolGatewayBroker({ startWithoutCredential: true })).toBe(true); + } catch (error) { + skip( + String(error).includes("EADDRINUSE"), + `port ${String(broker.HERMES_TOOL_GATEWAY_PORT)} is already held`, + ); + throw error; + } + replacementPid = Number.parseInt(fs.readFileSync(pidPath, "utf8").trim(), 10); + expect(replacementPid).not.toBe(brokerLikeProcess.pid); + expect(fs.readFileSync(hashPath, "utf8").trim()).toBe(broker.brokerRuntimeHash()); + expect(fs.statSync(broker.HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH).isSocket()).toBe(true); + expect(() => process.kill(brokerLikeProcess.pid!, 0)).not.toThrow(); + } finally { + try { + process.kill(replacementPid, "SIGTERM"); + } catch { + // The recorded process can exit before test cleanup. + } + delete require.cache[require.resolve(BROKER_WRAPPER)]; + } + }, + BROKER_TEST_TIMEOUT_MS, + ); + it("reports a missing listener inspector once and fails ownership closed", () => { delete require.cache[require.resolve(BROKER_WRAPPER)]; const broker = require(BROKER_WRAPPER); From c3ac6b1f455979eb2340da4a11283a53d7b5d9a3 Mon Sep 17 00:00:00 2001 From: "J. Yaunches" Date: Thu, 20 Aug 2026 12:12:56 -0400 Subject: [PATCH 5/5] fix(hermes): recheck broker ownership after health Signed-off-by: J. Yaunches --- src/lib/hermes-tool-gateway-broker.ts | 36 +++++-- test/hermes-tool-gateway-broker.test.ts | 124 ++++++++++++++++++------ 2 files changed, 120 insertions(+), 40 deletions(-) diff --git a/src/lib/hermes-tool-gateway-broker.ts b/src/lib/hermes-tool-gateway-broker.ts index 36b7b7ce90e..5d4c196f8bd 100644 --- a/src/lib/hermes-tool-gateway-broker.ts +++ b/src/lib/hermes-tool-gateway-broker.ts @@ -538,7 +538,7 @@ function probeHermesToolGatewayBrokerStart(options = {}) { * disposable private runtime files and performs no durable provider, * credential, or broker-process mutation. */ -function preflightHermesToolGatewayCloneBinding(sandboxName) { +function preflightHermesToolGatewayCloneBinding(sandboxName, deps = {}) { validateName(sandboxName, "sandbox name"); const requiredRuntimeFiles = [ HERMES_TOOL_GATEWAY_SCRIPT, @@ -556,8 +556,8 @@ function preflightHermesToolGatewayCloneBinding(sandboxName) { } const pid = readPid(); - const currentBrokerOwned = isHermesToolGatewayBrokerPortOwner(pid); - const currentBrokerHealthy = isHermesToolGatewayBrokerHealthy(); + const { owned: currentBrokerOwned, healthy: currentBrokerHealthy } = + verifyHermesToolGatewayBroker(pid, deps); if (currentBrokerHealthy && !currentBrokerOwned) { throw new Error( "Hermes managed-tool broker health endpoint is not owned by NemoClaw; " + @@ -696,6 +696,20 @@ function isHermesToolGatewayBrokerHealthy() { return result.status === 0; } +function verifyHermesToolGatewayBroker(pid, deps = {}) { + const isPortOwner = deps.isPortOwner ?? isHermesToolGatewayBrokerPortOwner; + const isHealthy = deps.isHealthy ?? isHermesToolGatewayBrokerHealthy; + const ownedBeforeHealth = isPortOwner(pid); + const healthy = isHealthy(); + // The unauthenticated health request can outlive the recorded broker. Require + // the same process to own the listener after each successful health probe. + const ownedAfterHealth = healthy && isPortOwner(pid); + return { + healthy, + owned: ownedBeforeHealth && ownedAfterHealth, + }; +} + function killStaleHermesToolGatewayBroker() { const pid = readPid(); if (isHermesToolGatewayBrokerPortOwner(pid)) { @@ -760,7 +774,7 @@ function planHermesToolGatewayBrokerRefresh({ return "start-or-restart"; } -function ensureHermesToolGatewayBroker(options = {}) { +function ensureHermesToolGatewayBroker(options = {}, deps = {}) { const refreshToken = typeof options.refreshToken === "string" && options.refreshToken.trim() ? options.refreshToken.trim() @@ -768,8 +782,8 @@ function ensureHermesToolGatewayBroker(options = {}) { const desiredHash = brokerRuntimeHash(); const hashMatches = readBrokerHash() === desiredHash; const pid = readPid(); - const currentBrokerOwned = isHermesToolGatewayBrokerPortOwner(pid); - const brokerHealthy = isHermesToolGatewayBrokerHealthy(); + const { owned: currentBrokerOwned, healthy: brokerHealthy } = + verifyHermesToolGatewayBroker(pid, deps); const currentBrokerHealthy = currentBrokerOwned && brokerHealthy; // `/health` is unauthenticated on a fixed port, so reachability proves // liveness and never identity. Ownership comes only from a recorded pid that @@ -792,9 +806,10 @@ function ensureHermesToolGatewayBroker(options = {}) { killStaleHermesToolGatewayBroker(); const nextPid = spawnHermesToolGatewayBroker(""); for (let attempt = 0; attempt < 20; attempt++) { + const nextBroker = verifyHermesToolGatewayBroker(nextPid, deps); if ( - isHermesToolGatewayBrokerPortOwner(nextPid) && - isHermesToolGatewayBrokerHealthy() && + nextBroker.owned && + nextBroker.healthy && fs.existsSync(HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH) ) { return true; @@ -829,9 +844,10 @@ function ensureHermesToolGatewayBroker(options = {}) { killStaleHermesToolGatewayBroker(); const nextPid = spawnHermesToolGatewayBroker(refreshToken, options.sandboxName ?? null); for (let attempt = 0; attempt < 20; attempt++) { + const nextBroker = verifyHermesToolGatewayBroker(nextPid, deps); if ( - isHermesToolGatewayBrokerPortOwner(nextPid) && - isHermesToolGatewayBrokerHealthy() && + nextBroker.owned && + nextBroker.healthy && registerHermesToolGatewayRuntimeCredential(refreshToken, options.sandboxName ?? null) ) { return true; diff --git a/test/hermes-tool-gateway-broker.test.ts b/test/hermes-tool-gateway-broker.test.ts index a38cef82e61..a4cd58c9f64 100644 --- a/test/hermes-tool-gateway-broker.test.ts +++ b/test/hermes-tool-gateway-broker.test.ts @@ -96,25 +96,12 @@ async function startForeignHealthListener( 'listener.listen(port, "127.0.0.1", () => process.stdout.write("ready\\n"));', 'process.once("SIGTERM", () => listener.close(() => process.exit(0)));', ].join("\n"); - const child = resources.ownChild( - spawn(process.execPath, ["--input-type=commonjs", "--eval", source, String(port)], { - stdio: ["ignore", "pipe", "pipe"], - }), - ); - let output = ""; - child.stdout?.on("data", (chunk) => { - output += chunk.toString(); - }); - child.stderr?.on("data", (chunk) => { - output += chunk.toString(); - }); - await waitForBrokerCondition( + return startInlineListener( + resources, "foreign health listener", - child, - () => output, - () => output.includes("ready"), + source, + [String(port)], ); - return child; } async function startBrokerLikeListener( @@ -144,19 +131,23 @@ async function startBrokerLikeListener( " listener.close(() => control.close(() => process.exit(0)));", "});", ].join("\n"); + return startInlineListener(resources, "broker-like listener", source, [ + "tool-gateway-broker.ts", + String(port), + controlSocket, + ]); +} + +async function startInlineListener( + resources: OwnedTestResources, + description: string, + source: string, + args: readonly string[], +): Promise { const child = resources.ownChild( - spawn( - process.execPath, - [ - "--input-type=commonjs", - "--eval", - source, - "tool-gateway-broker.ts", - String(port), - controlSocket, - ], - { stdio: ["ignore", "pipe", "pipe"] }, - ), + spawn(process.execPath, ["--input-type=commonjs", "--eval", source, ...args], { + stdio: ["ignore", "pipe", "pipe"], + }), ); let output = ""; child.stdout?.on("data", (chunk) => { @@ -166,7 +157,7 @@ async function startBrokerLikeListener( output += chunk.toString(); }); await waitForBrokerCondition( - "broker-like listener", + description, child, () => output, () => output.includes("ready"), @@ -334,6 +325,79 @@ describe("Hermes managed-tool gateway broker", () => { ).toBe("start-or-restart"); }); + it("refuses a listener whose ownership changes during health verification", async ({ + resources, + }) => { + const home = resources.ownDirectory(fs.mkdtempSync("/tmp/nc-broker-ownership-race-")); + vi.stubEnv("HOME", home); + delete require.cache[require.resolve(BROKER_WRAPPER)]; + const broker = require(BROKER_WRAPPER); + const credsDir = path.dirname(broker.HERMES_TOOL_GATEWAY_STATE_DIR); + const pidPath = path.join(credsDir, "hermes-tool-gateway-broker.pid"); + fs.mkdirSync(credsDir, { recursive: true, mode: 0o700 }); + fs.writeFileSync(pidPath, String(process.pid) + "\n", { mode: 0o600 }); + broker.persistHermesToolGatewayProviderState("clone", "test-only-refresh"); + + let controlRequests = 0; + const control = resources.ownServer( + http.createServer((_request, response) => { + controlRequests += 1; + response.writeHead(200); + response.end("{}"); + }), + ); + await new Promise((resolve, reject) => { + control.once("error", reject); + control.listen(broker.HERMES_TOOL_GATEWAY_CONTROL_SOCKET_PATH, () => resolve()); + }); + + function replaceListenerDuringHealth() { + let listenerOwner = "broker"; + const observations: string[] = []; + return { + deps: { + isPortOwner: () => { + observations.push("owner:" + listenerOwner); + return listenerOwner === "broker"; + }, + isHealthy: () => { + observations.push("health"); + listenerOwner = "foreign"; + return true; + }, + }, + observations, + }; + } + + try { + const preflightRace = replaceListenerDuringHealth(); + expect(() => + broker.preflightHermesToolGatewayCloneBinding("clone", preflightRace.deps), + ).toThrow("health endpoint is not owned by NemoClaw"); + expect(preflightRace.observations).toEqual(["owner:broker", "health", "owner:foreign"]); + + const credentialRace = replaceListenerDuringHealth(); + const refusal = vi.spyOn(console, "error").mockImplementation(() => {}); + expect( + broker.ensureHermesToolGatewayBroker( + { + refreshToken: "test-only-refresh", + sandboxName: "clone", + }, + credentialRace.deps, + ), + ).toBe(false); + expect(credentialRace.observations).toEqual(["owner:broker", "health", "owner:foreign"]); + expect(refusal.mock.calls.flat().join("\n")).toContain( + String(broker.HERMES_TOOL_GATEWAY_PORT), + ); + expect(controlRequests).toBe(0); + } finally { + delete require.cache[require.resolve(BROKER_WRAPPER)]; + } + }); + it( "reuses its listener and rechecks ownership after the process exits", async ({ resources, skip }) => {