fix(hermes): decide broker reuse from ownership

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);",
@@ -730,6 +732,36 @@ function planHermesToolGatewayBrokerRefresh({
   return "start-or-restart";
 }
 
+/**
+ * Decide whether the listener already on HERMES_TOOL_GATEWAY_PORT may be
+ * adopted as this run's broker.
+ *
+ * sourceOfTruth: This is the only place that converts broker reachability into
+ * a reuse decision for the no-credential path.
+ * invalidState: `/health` is unauthenticated and binds a fixed port, so a
+ * reachable endpoint proves liveness only. Ownership must come from the pid
+ * file or from a broker this process started; reachability must never stand in
+ * for identity, and must never latch `brokerStartedThisRun` for later callers.
+ * regressionTest: test/hermes-tool-gateway-broker-unowned-listener.test.ts.
+ * removalCondition: remove the "refuse-unowned-listener" outcome only when the
+ * broker authenticates `/health` with the per-sandbox broker token, so that a
+ * probe can prove identity instead of reachability.
+ */
+function planHermesToolGatewayBrokerReuse({
+  brokerHealthy,
+  currentBrokerOwned,
+  forceRestart = false,
+  hashMatches,
+}) {
+  if (brokerHealthy && !currentBrokerOwned) {
+    return "refuse-unowned-listener";
+  }
+  if (!forceRestart && hashMatches && brokerHealthy) {
+    return "reuse-current";
+  }
+  return "no-usable-broker";
+}
+
 function ensureHermesToolGatewayBroker(options = {}) {
   const refreshToken =
     typeof options.refreshToken === "string" && options.refreshToken.trim()
@@ -739,7 +771,27 @@ function ensureHermesToolGatewayBroker(options = {}) {
   const hashMatches = readBrokerHash() === desiredHash;
   const pid = readPid();
   const currentBrokerOwned = isHermesToolGatewayBrokerProcess(pid) || brokerStartedThisRun;
-  const currentBrokerHealthy = currentBrokerOwned && isHermesToolGatewayBrokerHealthy();
+  const brokerHealthy = isHermesToolGatewayBrokerHealthy();
+  const currentBrokerHealthy = currentBrokerOwned && brokerHealthy;
+  const reusePlan = planHermesToolGatewayBrokerReuse({
+    brokerHealthy,
+    currentBrokerOwned,
+    forceRestart: options.forceRestart,
+    hashMatches,
+  });
+  // Refuse before any path can adopt, restart around, or stage credentials
+  // against a listener NemoClaw cannot prove it owns. The clone preflight
+  // already rejects this state; every entry point has to agree, because a
+  // single adopting path latches `brokerStartedThisRun` and thereby satisfies
+  // the ownership test for the rest of the process.
+  if (reusePlan === "refuse-unowned-listener") {
+    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);
@@ -799,24 +851,10 @@ 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()) {
+  // `currentBrokerOwned` already covers both ownership proofs the three former
+  // branches tested separately (`brokerStartedThisRun` and a live broker pid),
+  // and the unowned case returned above, so reuse is one decision.
+  if (reusePlan === "reuse-current") {
     brokerStartedThisRun = true;
     return true;
   }
@@ -913,6 +951,7 @@ module.exports = {
   discardHermesToolGatewayCloneBinding,
   bindHermesToolGatewayCloneProviderState,
   planHermesToolGatewayBrokerRefresh,
+  planHermesToolGatewayBrokerReuse,
   isHermesToolGatewayBrokerHealthy,
   killStaleHermesToolGatewayBroker,
   ensureHermesToolGatewayBroker,

test/helpers/hermes-tool-gateway-broker-ownership-fixture.ts

@@ -0,0 +1,56 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+//
+// Support for the broker ownership regression. The branching setup lives here
+// so the test bodies stay linear, which `tools/growth-guardrails/test-conditionals.mts`
+// requires of every `*.test.ts` file.
+
+import http from "node:http";
+
+/** A listener that answers the broker health probe and serves nothing else. */
+export function createUnownedHealthListener(): http.Server {
+  return http.createServer((req, res) => {
+    if (req.url === "/health") {
+      res.writeHead(200, { "Content-Type": "application/json" });
+      res.end(JSON.stringify({ ok: true, services: [] }));
+      return;
+    }
+    res.writeHead(404).end();
+  });
+}
+
+export function listenOn(server: http.Server, port: number): Promise<void> {
+  return new Promise((resolve, reject) => {
+    server.once("error", reject);
+    server.listen(port, "127.0.0.1", () => resolve());
+  });
+}
+
+export function closeServer(server: http.Server): Promise<void> {
+  return new Promise((resolve) => server.close(() => resolve()));
+}
+
+/** Wait for a killed broker to release the fixed managed-tool gateway port. */
+export async function waitForPortFree(port: number): Promise<void> {
+  const deadline = Date.now() + 10_000;
+  while (Date.now() < deadline) {
+    const probe = http.createServer();
+    const free = await new Promise<boolean>((resolve) => {
+      probe.once("error", () => resolve(false));
+      probe.listen(port, "127.0.0.1", () => resolve(true));
+    });
+    await closeServer(probe);
+    if (free) return;
+    await new Promise((resolve) => setTimeout(resolve, 100));
+  }
+  throw new Error(`port ${port} was still held after the staged broker was killed`);
+}
+
+/** Restore a captured environment value, including the previously-unset case. */
+export function restoreEnv(key: string, previous: string | undefined): void {
+  if (previous === undefined) {
+    delete process.env[key];
+    return;
+  }
+  process.env[key] = previous;
+}

test/hermes-tool-gateway-broker-unowned-listener.test.ts

@@ -0,0 +1,222 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import fs from "node:fs";
+import { createRequire } from "node:module";
+import os from "node:os";
+import path from "node:path";
+import {
+  closeServer,
+  createUnownedHealthListener,
+  listenOn,
+  restoreEnv,
+  waitForPortFree,
+} from "./helpers/hermes-tool-gateway-broker-ownership-fixture";
+import { describe, expect, test as it } from "./helpers/owned-test-resources";
+import { testTimeout } from "./helpers/timeouts";
+
+const require = createRequire(import.meta.url);
+const BROKER_WRAPPER = path.join(
+  import.meta.dirname,
+  "..",
+  "src",
+  "lib",
+  "hermes-tool-gateway-broker.ts",
+);
+
+const BROKER_TEST_TIMEOUT_MS = testTimeout(45_000);
+
+/** Every reuse input that must still refuse a listener NemoClaw does not own. */
+const UNOWNED_LISTENER_CASES = [
+  { label: "a matching hash", forceRestart: false, hashMatches: true },
+  { label: "a mismatched hash", forceRestart: false, hashMatches: false },
+  { label: "a restart request", forceRestart: true, hashMatches: true },
+  { label: "a restart request and a mismatched hash", forceRestart: true, hashMatches: false },
+];
+
+/** Every reuse input that resolves to an ordinary restart when the port is dead. */
+const UNREACHABLE_PORT_CASES = [
+  { label: "an unowned broker and a matching hash", currentBrokerOwned: false, forceRestart: false, hashMatches: true },
+  { label: "an unowned broker and a mismatched hash", currentBrokerOwned: false, forceRestart: false, hashMatches: false },
+  { label: "an unowned broker and a restart request", currentBrokerOwned: false, forceRestart: true, hashMatches: true },
+  { label: "an unowned broker, a restart request, and a mismatched hash", currentBrokerOwned: false, forceRestart: true, hashMatches: false },
+  { label: "an owned broker and a matching hash", currentBrokerOwned: true, forceRestart: false, hashMatches: true },
+  { label: "an owned broker and a mismatched hash", currentBrokerOwned: true, forceRestart: false, hashMatches: false },
+  { label: "an owned broker and a restart request", currentBrokerOwned: true, forceRestart: true, hashMatches: true },
+  { label: "an owned broker, a restart request, and a mismatched hash", currentBrokerOwned: true, forceRestart: true, hashMatches: false },
+];
+
+function loadBrokerWithHome(home: string) {
+  process.env.HOME = home;
+  // The module resolves its credential paths at load time, and
+  // `brokerStartedThisRun` is module state, so each case needs a fresh copy.
+  delete require.cache[require.resolve(BROKER_WRAPPER)];
+  return require(BROKER_WRAPPER);
+}
+
+describe("Hermes managed-tool broker ownership", () => {
+  it("refuses to adopt a healthy listener it does not own", () => {
+    const broker = require(BROKER_WRAPPER);
+
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: false,
+        forceRestart: false,
+        hashMatches: true,
+      }),
+    ).toBe("refuse-unowned-listener");
+  });
+
+  // Ownership is the only input that may admit a listener. A mismatched hash or
+  // a restart request must not downgrade the refusal into an ordinary "no
+  // broker" outcome, because only the refusal names the held port.
+  it.each(UNOWNED_LISTENER_CASES)(
+    "keeps refusing an unowned listener with $label",
+    ({ forceRestart, hashMatches }) => {
+      const broker = require(BROKER_WRAPPER);
+
+      expect(
+        broker.planHermesToolGatewayBrokerReuse({
+          brokerHealthy: true,
+          currentBrokerOwned: false,
+          forceRestart,
+          hashMatches,
+        }),
+      ).toBe("refuse-unowned-listener");
+    },
+  );
+
+  it("treats an omitted restart request as no restart", () => {
+    const broker = require(BROKER_WRAPPER);
+
+    // `ensureHermesToolGatewayBroker` forwards `options.forceRestart`, which is
+    // undefined whenever a caller omits it, so the default is the production
+    // path rather than a convenience.
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        hashMatches: true,
+      }),
+    ).toBe("reuse-current");
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: false,
+        hashMatches: true,
+      }),
+    ).toBe("refuse-unowned-listener");
+  });
+
+  it("adopts a healthy listener it owns when the runtime hash still matches", () => {
+    const broker = require(BROKER_WRAPPER);
+
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        forceRestart: false,
+        hashMatches: true,
+      }),
+    ).toBe("reuse-current");
+  });
+
+  it("declines an owned broker when the runtime hash moved or a restart was requested", () => {
+    const broker = require(BROKER_WRAPPER);
+
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        forceRestart: false,
+        hashMatches: false,
+      }),
+    ).toBe("no-usable-broker");
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        forceRestart: true,
+        hashMatches: true,
+      }),
+    ).toBe("no-usable-broker");
+    expect(
+      broker.planHermesToolGatewayBrokerReuse({
+        brokerHealthy: true,
+        currentBrokerOwned: true,
+        forceRestart: true,
+        hashMatches: false,
+      }),
+    ).toBe("no-usable-broker");
+  });
+
+  // An unreachable port cannot be refused as unowned; it is the ordinary
+  // restart case whether or not a prior broker was ours.
+  it.each(UNREACHABLE_PORT_CASES)(
+    "reports no usable broker when nothing answers on the port with $label",
+    ({ currentBrokerOwned, forceRestart, hashMatches }) => {
+      const broker = require(BROKER_WRAPPER);
+
+      expect(
+        broker.planHermesToolGatewayBrokerReuse({
+          brokerHealthy: false,
+          currentBrokerOwned,
+          forceRestart,
+          hashMatches,
+        }),
+      ).toBe("no-usable-broker");
+    },
+  );
+
+  it(
+    "reports no broker when a foreign listener answers health after a stale hash is left behind",
+    async ({ skip }) => {
+      const previousHome = process.env.HOME;
+      const home = fs.mkdtempSync(path.join(os.tmpdir(), "nc-broker-unowned-"));
+      const impostor = createUnownedHealthListener();
+
+      try {
+        const staging = loadBrokerWithHome(home);
+        const credsDir = path.dirname(staging.HERMES_TOOL_GATEWAY_STATE_DIR);
+        const hashPath = path.join(credsDir, "hermes-tool-gateway-broker.hash");
+        const pidPath = path.join(credsDir, "hermes-tool-gateway-broker.pid");
+
+        // Let the module write its own runtime hash rather than recomputing it
+        // here. A hand-built hash would stop matching the moment the runtime
+        // inputs change, and this case would then pass for the wrong reason.
+        // The hash is written when the broker is spawned, so it exists whether
+        // or not that broker went on to become healthy.
+        staging.ensureHermesToolGatewayBroker({ startWithoutCredential: true });
+        const runtimeHash = fs.readFileSync(hashPath, "utf8");
+
+        staging.killStaleHermesToolGatewayBroker();
+        await waitForPortFree(staging.HERMES_TOOL_GATEWAY_PORT);
+
+        // A broker that crashed leaves its runtime hash behind while the pid
+        // goes away. That is the exact state this regression covers.
+        fs.writeFileSync(hashPath, runtimeHash, { mode: 0o600 });
+        expect(fs.existsSync(pidPath)).toBe(false);
+
+        const settled = loadBrokerWithHome(home);
+        await listenOn(impostor, settled.HERMES_TOOL_GATEWAY_PORT);
+        // The probe shells out to curl. Where a harness blocks loopback HTTP
+        // for subprocesses, no listener can be observed as healthy and this
+        // case would assert nothing, so report it as skipped instead of passed.
+        skip(
+          !settled.isHermesToolGatewayBrokerHealthy(),
+          "curl cannot read a loopback response in this environment",
+        );
+
+        // Reachability alone must not answer "broker ready".
+        expect(settled.ensureHermesToolGatewayBroker({})).toBe(false);
+      } finally {
+        await closeServer(impostor);
+        restoreEnv("HOME", previousHome);
+        delete require.cache[require.resolve(BROKER_WRAPPER)];
+        fs.rmSync(home, { recursive: true, force: true });
+      }
+    },
+    BROKER_TEST_TIMEOUT_MS,
+  );
+});