fix(mcp): own sandbox CSP fail-closed validation in the decoder (#110267)

* fix(mcp): reject sandbox CSP metadata that normalizes to no policy

A present ?csp= value that decodes to valid JSON but is not a usable CSP
(e.g. null or a wrong-shaped object) normalized to undefined and was
indistinguishable from an absent parameter, so the gateway served proxy
HTML under the default policy. encodeCsp omits the query param entirely
for such values, so a present-but-empty policy is never legitimate;
throw so the sandbox endpoint fails closed with 400. Found by review on
the revert of 73685b4e7c946; the gap predates that commit.

* fix(mcp): treat empty csp query value as malformed, not absent

?csp= with an empty value passed the falsy absent-guard and served proxy
HTML under the default policy. Only a truly absent parameter (null) may
skip validation; an empty string now falls through to JSON.parse and
fails closed with 400.

* refactor(gateway): drop redundant sandbox CSP handler guard

decodeMcpAppSandboxCsp now throws for every present-but-unusable value
(1ca26c508d added the same fail-closed behavior at the handler seam),
so the handler-level 'present but falsy' check is unreachable. Keep the
invariant in the decoder, which owns policy decode semantics.

* fix(test): reset diagnostic listener-presence mirror between non-isolated files

resetOpenClawGlobalDiagnosticState clears the listener sets and deletes
the diagnostic-events state key, but the listener-presence counts live
under a separate globalThis record and survived, so
hasInternalDiagnosticEventListeners() stayed true for every later file
in the worker once any file leaked a registration (e.g. the import-time
listener in src/logging/diagnostic-run-activity.ts whose stop handle is
lost to the module-registry reset). Zero the counts to match the cleared
sets. Root cause of the model-call-diagnostics flake in CI run
29624203224; #110288 added the victim-side reset, this fixes the class.

src/agents/mcp-app-sandbox.ts

@@ -91,15 +91,23 @@ export function resolveMcpAppSandboxPort(gatewayPort: number, configuredPort?: n
   return sandboxPort;
 }
 
+// Malformed input must throw: the gateway sandbox endpoint relies on it to fail
+// closed with 400 instead of serving proxy HTML under a default policy. That
+// includes valid JSON that is not a usable CSP — encodeCsp omits the query
+// param entirely in that case, so a present-but-empty value is never legitimate.
 export function decodeMcpAppSandboxCsp(value: string | null): McpAppCsp | undefined {
-  if (!value) {
+  if (value === null) {
     return undefined;
   }
   if (value.length > MCP_APP_SANDBOX_CSP_MAX_ENCODED_BYTES) {
     throw new Error("MCP App CSP metadata is too large");
   }
   const decoded = JSON.parse(Buffer.from(value, "base64url").toString("utf8")) as unknown;
-  return normalizeMcpAppCsp(decoded);
+  const normalized = normalizeMcpAppCsp(decoded);
+  if (!normalized) {
+    throw new Error("MCP App CSP metadata is not a valid policy");
+  }
+  return normalized;
 }
 
 /** Trusted outer document. The untrusted app HTML is written only into its inner iframe. */

src/gateway/mcp-app-sandbox-http.test.ts

@@ -49,6 +49,9 @@ describe("MCP App sandbox HTTP origin", () => {
     expect(request("/", "GET").res.statusCode).toBe(404);
     expect(request(buildMcpAppSandboxPath(), "POST").res.statusCode).toBe(404);
     expect(request(`${buildMcpAppSandboxPath()}?csp=not-json`).res.statusCode).toBe(400);
+    const jsonButNotCsp = Buffer.from("null", "utf8").toString("base64url");
+    expect(request(`${buildMcpAppSandboxPath()}?csp=${jsonButNotCsp}`).res.statusCode).toBe(400);
+    expect(request(`${buildMcpAppSandboxPath()}?csp=`).res.statusCode).toBe(400);
     expect(request("http://[", "GET").res.statusCode).toBe(400);
   });
 });

src/gateway/mcp-app-sandbox-http.ts

@@ -30,13 +30,9 @@ function handleMcpAppSandboxHttpRequest(req: IncomingMessage, res: ServerRespons
     return;
   }
 
-  const encodedCsp = url.searchParams.get("csp");
   let csp;
   try {
-    csp = decodeMcpAppSandboxCsp(encodedCsp);
-    if (encodedCsp !== null && !csp) {
-      throw new Error("invalid MCP App sandbox policy");
-    }
+    csp = decodeMcpAppSandboxCsp(url.searchParams.get("csp"));
   } catch {
     res.statusCode = 400;
     res.setHeader("Content-Type", "text/plain; charset=utf-8");

test/non-isolated-runner.ts

@@ -28,6 +28,9 @@ const SHARED_TEST_SETUP = Symbol.for("openclaw.sharedTestSetup");
 const EMBEDDED_RUN_STATE = Symbol.for("openclaw.embeddedRunState");
 const REPLY_RUN_REGISTRY = Symbol.for("openclaw.replyRunRegistry");
 const DIAGNOSTIC_EVENTS_STATE = Symbol.for("openclaw.diagnosticEvents.state.v1");
+const DIAGNOSTIC_EVENT_LISTENER_PRESENCE = Symbol.for(
+  "openclaw.diagnosticEventListenerPresence.v1",
+);
 const nativeTimerGlobals = {
   setTimeout: globalThis.setTimeout,
   clearTimeout: globalThis.clearTimeout,
@@ -230,6 +233,17 @@ function resetOpenClawGlobalDiagnosticState(): void {
   state?.toolExecutionListeners?.clear();
   state?.asyncQueue?.splice(0);
   Reflect.deleteProperty(globalStore, DIAGNOSTIC_EVENTS_STATE);
+  // The listener-presence mirror is a separate globalThis record; clearing the
+  // sets above without zeroing it leaves hasInternalDiagnosticEventListeners()
+  // true for the next file (e.g. an import-time registration like
+  // diagnostic-run-activity.ts whose stop handle died with the module registry).
+  const presence = globalStore[DIAGNOSTIC_EVENT_LISTENER_PRESENCE] as
+    | { internalCount?: number; trustedCount?: number }
+    | undefined;
+  if (presence) {
+    presence.internalCount = 0;
+    presence.trustedCount = 0;
+  }
 }
 
 const SERIALIZED_RESOLVE_MOCKS = Symbol.for("openclaw.serializedResolveMocks");