fix(runner): safe hardening from the stack review pass

services/runner/AGENTS.md

@@ -13,7 +13,8 @@ Not part of the `web/` pnpm workspace. It has its OWN `pnpm-lock.yaml`, builds i
 image, and pins Node 24 / pnpm 10.30 / ESM (`"type": "module"`). It runs through `tsx` (no
 app compile step); the only build is `pnpm run build:extension` (esbuild-bundles the Pi
 extension into `dist/`). Keep it standalone so the sidecar image stays decoupled from the web
-dependency graph.
+dependency graph. `pnpm run build:extension` bundles both the Pi extension and the in-sandbox
+tool MCP shim into `dist/`.
 
 ## Commands
 

services/runner/README.md

@@ -99,10 +99,10 @@ servers. See `docs/design/agent-workflows/projects/sidecar-trust-and-sandbox-enf
 
 ## The extension bundle
 
-`scripts/build-extension.mjs` esbuild-bundles `src/extensions/agenta.ts` into one
-self-contained `dist/extensions/agenta.js` that Pi can load anywhere (host, the sidecar, a
-Daytona snapshot). The dev image bakes it; rebuild after editing the extension or the
-tracer:
+`scripts/build-extension.mjs` esbuild-bundles the Pi extension into
+`dist/extensions/agenta.js` and the in-sandbox tool MCP shim into
+`dist/tools/tool-mcp-stdio.js`. The dev image bakes both bundles; rebuild after editing the
+extension, tracer, or shim:
 
 ```bash
 pnpm run build:extension

services/runner/src/engines/sandbox_agent/tool-mcp-assets.ts

@@ -28,6 +28,16 @@ import { PKG_ROOT } from "./daemon.ts";
 
 type Log = (message: string) => void;
 
+const bundleContentsByPath = new Map<string, string>();
+
+function toolMcpBundleContents(path: string): string {
+  const cached = bundleContentsByPath.get(path);
+  if (cached !== undefined) return cached;
+  const contents = readFileSync(path, "utf-8");
+  bundleContentsByPath.set(path, contents);
+  return contents;
+}
+
 /**
  * The bundled shim path. Built by `pnpm run build:extension` alongside the Pi extension.
  * Resolved lazily (a function, not a module-level const) so `SANDBOX_AGENT_RELAY_MCP_BUNDLE`
@@ -77,11 +87,11 @@ export async function uploadToolMcpAssets(
   const specsPath = `${destDir}/tool-mcp-specs.json`;
   try {
     await sandbox.mkdirFs({ path: destDir });
-    await sandbox.writeFsFile(
-      { path: bundlePath },
-      readFileSync(bundle, "utf-8"),
-    );
-    await sandbox.writeFsFile({ path: specsPath }, JSON.stringify(specs));
+    const bundleContents = toolMcpBundleContents(bundle);
+    await Promise.all([
+      sandbox.writeFsFile({ path: bundlePath }, bundleContents),
+      sandbox.writeFsFile({ path: specsPath }, JSON.stringify(specs)),
+    ]);
     return { bundlePath, specsPath };
   } catch (err) {
     log(`tool MCP shim upload failed: ${(err as Error).message}`);

services/runner/src/tools/relay-watch.ts

@@ -74,8 +74,8 @@ export function resolveRemoteWatchWindowMs(
   const raw = process.env.AGENTA_AGENT_TOOLS_RELAY_REMOTE_WATCH_WINDOW_MS;
   if (raw === undefined || raw === "")
     return RELAY_REMOTE_WATCH_WINDOW_DEFAULT_MS;
-  const parsed = Number.parseInt(raw, 10);
-  if (!Number.isFinite(parsed)) {
+  const parsed = Number(raw);
+  if (!Number.isInteger(parsed)) {
     log(
       `[relay] unparseable AGENTA_AGENT_TOOLS_RELAY_REMOTE_WATCH_WINDOW_MS '${raw}'; using default ${RELAY_REMOTE_WATCH_WINDOW_DEFAULT_MS}`,
     );
@@ -277,6 +277,7 @@ export function daytonaRelayActivitySource(
   // entirely (no wake, no counter reset, no in-flight mutation).
   let armGeneration = 0;
   let liveGeneration = 0; // 0 = no live window
+  let missedGeneration = 0; // active window already charged for a safety-poll miss
   let outerBoundTimer: ReturnType<typeof setTimeout> | undefined;
   let deferredArmTimer: ReturnType<typeof setTimeout> | undefined;
   let waiter:
@@ -406,6 +407,7 @@ export function daytonaRelayActivitySource(
         outerBoundTimer = undefined;
         if (closed || gen !== liveGeneration) return;
         liveGeneration = 0;
+        if (missedGeneration === gen) missedGeneration = 0;
         countFailure("window outer bound expired (exec never settled)");
       },
       jitteredWindow + graceMs + outerBoundMarginMs,
@@ -415,6 +417,8 @@ export function daytonaRelayActivitySource(
         if (gen !== liveGeneration) return; // abandoned by the outer bound: dead gen
         liveGeneration = 0;
         clearOuterBound();
+        const generationHadMiss = missedGeneration === gen;
+        if (generationHadMiss) missedGeneration = 0;
         if (closed) return; // abandoned window (see close()); its wake is meaningless now
         // Completion classification (fix 2 of the slice-3 review). A nullish result
         // is a broken daemon path, not a wake: an insta-resolving runProcess that
@@ -430,7 +434,9 @@ export function daytonaRelayActivitySource(
           return;
         }
         if (result.exitCode === 0) {
-          consecutiveFailures = 0;
+          // A safety poll already charged this generation as a miss. Its ordinary zero exit
+          // must not erase that failure from the consecutive demotion counter.
+          if (!generationHadMiss) consecutiveFailures = 0;
         } else {
           // Nonzero, null (signal-killed / OOM), or MISSING exit is still a wake (the
           // list pass is harmless) but counts as a failure so a script that keeps
@@ -460,6 +466,7 @@ export function daytonaRelayActivitySource(
     noteMiss: () => {
       if (closed || demoted) return;
       // The safety poll found a request while a window claimed healthy: the watch lied.
+      if (liveGeneration !== 0) missedGeneration = liveGeneration;
       countFailure("safety poll found a request the watch missed");
     },
     wait: ({ timeoutMs, signal }) => {

services/runner/src/tools/relay.ts

@@ -418,6 +418,29 @@ function isRelayFileName(name: string): boolean {
   );
 }
 
+/** Bound one daemon removal so relay startup, polling, and shutdown cannot wedge on it. */
+const RELAY_REMOVE_TIMEOUT_MS = 10_000;
+
+async function removeRelayFile(host: RelayHost, path: string): Promise<boolean> {
+  let timer: ReturnType<typeof setTimeout> | undefined;
+  const timeout = new Promise<false>((resolve) => {
+    timer = setTimeout(() => resolve(false), RELAY_REMOVE_TIMEOUT_MS);
+  });
+  try {
+    return await Promise.race([
+      Promise.resolve(host.remove(path)).then(
+        () => true,
+        () => false,
+      ),
+      timeout,
+    ]);
+  } catch {
+    return false;
+  } finally {
+    clearTimeout(timer);
+  }
+}
+
 /**
  * Clear pre-turn relay residue from a reused relay dir (a warm-continued turn skips the
  * cold build's rm -rf, so a crashed prior turn can leave files behind). Every relay
@@ -457,7 +480,7 @@ export async function sweepStaleRelayFiles(
       // is only re-armed watch noise once the loop's seen set has it; a leftover
       // response is inert for fresh toolCallIds).
       await Promise.allSettled(
-        stale.map((name) => host.remove(`${relayDir}/${name}`)),
+        stale.map((name) => removeRelayFile(host, `${relayDir}/${name}`)),
       );
       log(
         `[relay] cleared ${stale.length} stale relay file(s) predating the turn`,
@@ -588,18 +611,9 @@ export function startToolRelay(
     // publication, consistent with the stale-sweep decision (a restarted turn must
     // not re-execute stale requests; the writer times out and surfaces a tool error
     // instead). A failed removal is recorded for retry on later list passes.
-    let removeDone: Promise<void>;
-    try {
-      removeDone = host.remove(reqPath).then(
-        () => undefined,
-        () => {
-          removeFailed.add(reqName);
-        },
-      );
-    } catch {
-      removeFailed.add(reqName);
-      removeDone = Promise.resolve();
-    }
+    const removeDone = removeRelayFile(host, reqPath).then((removed) => {
+      if (!removed) removeFailed.add(reqName);
+    });
     // The execute phase starts NOW (as soon as the read returned) and runs in the
     // background; the pickup itself only awaits stat + remove.
     inflight.push(execute(id, raw));
@@ -667,18 +681,11 @@ export function startToolRelay(
               removeFailed.delete(name);
               continue;
             }
-            try {
-              pickups.push(
-                host.remove(`${relayDir}/${name}`).then(
-                  () => {
-                    removeFailed.delete(name);
-                  },
-                  () => undefined,
-                ),
-              );
-            } catch {
-              // Still failing synchronously; keep it in the retry set.
-            }
+            pickups.push(
+              removeRelayFile(host, `${relayDir}/${name}`).then((removed) => {
+                if (removed) removeFailed.delete(name);
+              }),
+            );
           }
           for (const name of names) {
             if (!name.endsWith(RELAY_REQ_SUFFIX) || seen.has(name)) continue;

services/runner/src/tools/tool-mcp-http.ts

@@ -311,15 +311,7 @@ export function startInternalToolMcpServer(
   const requestListener = (req: IncomingMessage, res: ServerResponse): void => {
     active.add(res);
     res.on("close", () => active.delete(res));
-    // The MCP Streamable-HTTP client opens a GET SSE stream and sends a DELETE on close; this
-    // stateless server offers neither, so it returns 405 (the client tolerates 405 for both).
-    if (req.method !== "POST") {
-      res.writeHead(405, { "content-type": "application/json", allow: "POST" });
-      res.end(JSON.stringify({ error: "method not allowed" }));
-      return;
-    }
-
-    // Gate the POST path before reading or parsing attacker-controlled input.
+    // Authenticate every MCP transport verb before dispatching or revealing method support.
     if (!hasValidAuthorization(req.headers.authorization, authorizationToken)) {
       res.writeHead(401, { "content-type": "application/json" });
       res.end(
@@ -332,6 +324,14 @@ export function startInternalToolMcpServer(
       return;
     }
 
+    // The MCP Streamable-HTTP client opens a GET SSE stream and sends a DELETE on close; this
+    // stateless server offers neither, so it returns 405 (the client tolerates 405 for both).
+    if (req.method !== "POST") {
+      res.writeHead(405, { "content-type": "application/json", allow: "POST" });
+      res.end(JSON.stringify({ error: "method not allowed" }));
+      return;
+    }
+
     readBody(req)
       .then(async (raw) => {
         let parsed: any;

services/runner/src/tools/tool-mcp-stdio.ts

@@ -148,6 +148,9 @@ export function loadShimConfig(
 }
 
 /**
+ * Delivery is at least once: after pickup, the runner still executes if this writer dies, and
+ * a writer retry publishes a new request because the relay does not deduplicate retries.
+ *
  * Handle one MCP JSON-RPC message. Returns the response object, or `undefined` for a
  * notification (no `id`). Mirrors `tools/tool-mcp-http.ts` `handle` (initialize / tools-list /
  * tools-call shapes), but `tools/call` writes a relay request file (`relayToolCall`) rather
@@ -160,6 +163,14 @@ export async function handleToolMcpMessage(
 ): Promise<unknown | undefined> {
   const { id, method, params } = message ?? {};
 
+  if (typeof method !== "string") {
+    return {
+      jsonrpc: "2.0",
+      id: id ?? null,
+      error: { code: -32600, message: "invalid request" },
+    };
+  }
+
   // Notifications (no id, e.g. notifications/initialized) need no response.
   if (id === undefined || id === null) return undefined;
 
@@ -175,6 +186,10 @@ export async function handleToolMcpMessage(
     };
   }
 
+  if (method === "ping") {
+    return { jsonrpc: "2.0", id, result: {} };
+  }
+
   if (method === "tools/list") {
     return {
       jsonrpc: "2.0",
@@ -271,6 +286,13 @@ export async function runToolMcpStdio(
       message = JSON.parse(trimmed);
     } catch (err) {
       log(`parse error: ${(err as Error).message}`);
+      output.write(
+        `${JSON.stringify({
+          jsonrpc: "2.0",
+          id: null,
+          error: { code: -32700, message: "parse error" },
+        })}\n`,
+      );
       continue;
     }
     const pending = handleToolMcpMessage(message, specs, relayDir).then(

services/runner/tests/unit/relay-watch.test.ts

@@ -373,6 +373,31 @@ describe("daytonaRelayActivitySource invariants", () => {
     source.close();
   });
 
+  it("a zero exit from the same missed generation does not reset the demotion counter", async () => {
+    const { sandbox, calls } = fakeSandbox();
+    const source = daytonaRelayActivitySource(sandbox, "/relay", {
+      windowMs: 60_000,
+      backoffBaseMs: 1,
+      backoffCapMs: 2,
+    });
+
+    for (let generation = 0; generation < 3; generation += 1) {
+      const wait = source.wait({ timeoutMs: 5_000 });
+      assert.equal(calls.length, generation + 1);
+      source.noteMiss?.();
+      calls[generation].resolve({ exitCode: 0, timedOut: false });
+      assert.equal(await wait, "activity");
+      await sleep(10);
+    }
+
+    assert.equal(
+      source.isHealthy(),
+      false,
+      "three missed generations demote even though each later exits zero",
+    );
+    source.close();
+  });
+
   it("outer bound: a never-settling exec counts as a failure, re-arms inside the SAME wait, demotes after three, and late settles change nothing", async () => {
     const settlers: Array<{
       resolve: (r: { exitCode?: number | null; timedOut?: boolean }) => void;
@@ -541,6 +566,17 @@ describe("remote watch window config", () => {
     assert.match(logs[0], /unparseable/);
   });
 
+  it("partial numeric garbage -> default with one warning", () => {
+    const logs: string[] = [];
+    process.env[WINDOW_ENV] = "30000junk";
+    assert.equal(
+      resolveRemoteWatchWindowMs((m) => logs.push(m)),
+      RELAY_REMOTE_WATCH_WINDOW_DEFAULT_MS,
+    );
+    assert.equal(logs.length, 1);
+    assert.match(logs[0], /unparseable/);
+  });
+
   it("1000 -> clamped to the 5000 floor with one warning", () => {
     const logs: string[] = [];
     process.env[WINDOW_ENV] = "1000";

services/runner/tests/unit/sandbox-agent-orchestration.test.ts

@@ -720,6 +720,55 @@ describe("runSandboxAgent orchestration", () => {
     );
   });
 
+  it("wires Pi ask grants into the captured relay guard while non-Pi ask passes", async () => {
+    const request = {
+      messages: [{ role: "user", content: "use the tool" }],
+      permissions: { default: "ask" },
+      customTools: [
+        { name: "server_tool", kind: "callback", permission: "ask" },
+      ],
+    } as AgentRunRequest;
+    const relayRequest = {
+      toolName: "server_tool",
+      toolCallId: "forged-call",
+      args: {},
+    };
+
+    const pi = fakeHarness();
+    const piResult = await runSandboxAgent(
+      { ...request, harness: "pi_core" },
+      undefined,
+      undefined,
+      pi.deps,
+    );
+    assert.equal(piResult.ok, true);
+    const piGuard = pi.calls.toolRelayArgs?.[6] as Function;
+    assert.deepEqual(
+      piGuard(request.customTools?.[0], relayRequest),
+      {
+        allow: false,
+        reason:
+          "Tool 'server_tool' was not approved via the permission dialog.",
+      },
+      "Pi ask without a grant fails closed",
+    );
+
+    const claude = fakeHarness();
+    const claudeResult = await runSandboxAgent(
+      { ...request, harness: "claude" },
+      undefined,
+      undefined,
+      claude.deps,
+    );
+    assert.equal(claudeResult.ok, true);
+    const claudeGuard = claude.calls.toolRelayArgs?.[6] as Function;
+    assert.deepEqual(
+      claudeGuard(request.customTools?.[0], relayRequest),
+      { allow: true },
+      "non-Pi ask remains gated by the harness and passes the relay guard",
+    );
+  });
+
   it("delivers a non-Pi run's gateway tools over the internal HTTP MCP channel", async () => {
     // Claude takes tools only over MCP. The INTERNAL gateway-tool channel (distinct from the
     // disabled USER stdio MCP path) is restored over a loopback HTTP MCP server the runner