From c47d87a461bacf20565e77f08c4d8f7235130cbf Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Mon, 10 Aug 2026 11:58:22 +0200 Subject: [PATCH 01/12] feat(diagnostics): add the managed-transport failure contract First slice of the correlated outbound diagnostics facility: a shared managed_transport_failure event schema with seven connection-ordered phases, an allowlist-only builder, a safe error-cause-chain walker that drops free-text messages, credential and query stripping for endpoint references, a bounded content-type-aware error-body snippet, a strict response-header allowlist, deterministic phase classification over the undici and socket vocabulary, and failure-only key=value emission with a generated correlation id. A non-MCP webhook consumer test proves the contract is reusable beyond the first integration. Negative tests cover every forbidden field class. Refs #7957 Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- src/lib/diagnostics/managed-transport.test.ts | 234 +++++++++++++++ src/lib/diagnostics/managed-transport.ts | 270 ++++++++++++++++++ 2 files changed, 504 insertions(+) create mode 100644 src/lib/diagnostics/managed-transport.test.ts create mode 100644 src/lib/diagnostics/managed-transport.ts diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts new file mode 100644 index 00000000000..879a0d94dff --- /dev/null +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -0,0 +1,234 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + boundedErrorBodySnippet, + buildManagedTransportFailure, + classifyTransportPhase, + emitManagedTransportFailure, + generateTransportTraceId, + MANAGED_TRANSPORT_FAILURE_EVENT, + pickSafeResponseHeaders, + safeCauseChain, + safeTargetRef, +} from "./managed-transport"; + +describe("safeTargetRef", () => { + it("strips credentials, query string, and fragment from URLs", () => { + expect(safeTargetRef("https://user:secret@api.example.com/v1/chat?api_key=k#frag")).toBe( + "api.example.com:443/v1/chat", + ); + }); + + it("keeps host and explicit port", () => { + expect(safeTargetRef("http://proxy.internal:3128/")).toBe("proxy.internal:3128/"); + }); + + it("drops query and userinfo from non-URL values", () => { + expect(safeTargetRef("user:secret@host:8080?token=x")).toBe("host:8080"); + }); +}); + +describe("safeCauseChain", () => { + it("keeps only safe fields across nested causes", () => { + const inner = Object.assign(new Error("connect ECONNREFUSED 10.0.0.9:443 key=abc"), { + code: "ECONNREFUSED", + errno: -111, + syscall: "connect", + port: 443, + address: "10.0.0.9", + }); + const outer = new Error("fetch failed https://api.example.com?token=zzz", { cause: inner }); + const chain = safeCauseChain(outer); + + expect(chain).toEqual([ + { name: "Error" }, + { name: "Error", code: "ECONNREFUSED", errno: -111, syscall: "connect", port: 443 }, + ]); + const serialized = JSON.stringify(chain); + expect(serialized).not.toContain("token"); + expect(serialized).not.toContain("10.0.0.9"); + expect(serialized).not.toContain("key=abc"); + }); + + it("guards against cause cycles and unbounded depth", () => { + const a: Record = { name: "A" }; + const b: Record = { name: "B", cause: a }; + a.cause = b; + expect(safeCauseChain(a)).toEqual([{ name: "A" }, { name: "B" }]); + + let deep: Record = { name: "leaf" }; + for (let index = 0; index < 20; index += 1) deep = { name: `n${index}`, cause: deep }; + expect(safeCauseChain(deep).length).toBeLessThanOrEqual(8); + }); +}); + +describe("pickSafeResponseHeaders", () => { + it("keeps the diagnostic allowlist and drops content-bearing headers", () => { + const picked = pickSafeResponseHeaders([ + ["Server", "envoy"], + ["Via", "1.1 proxy"], + ["X-Request-Id", "req-1"], + ["X-Envoy-Response-Flags", "UF"], + ["Set-Cookie", "session=secret"], + ["Authorization", "Bearer token"], + ["Location", "https://example.com?code=abc"], + ]); + expect(picked).toEqual({ + responseServer: "envoy", + responseVia: "1.1 proxy", + xRequestId: "req-1", + xEnvoyResponseFlags: "UF", + }); + expect(JSON.stringify(picked)).not.toContain("secret"); + expect(JSON.stringify(picked)).not.toContain("Bearer"); + }); +}); + +describe("boundedErrorBodySnippet", () => { + it("bounds textual bodies and refuses non-textual content types", () => { + const long = "x".repeat(2000); + const snippet = boundedErrorBodySnippet(long, "application/json"); + expect(snippet).toBeDefined(); + expect((snippet as string).length).toBeLessThanOrEqual(512); + expect(boundedErrorBodySnippet(long, "application/octet-stream")).toBeUndefined(); + expect(boundedErrorBodySnippet(long, undefined)).toBeUndefined(); + }); +}); + +describe("classifyTransportPhase", () => { + it("is deterministic across the failure vocabulary", () => { + expect(classifyTransportPhase({ policyDenied: true })).toBe("policy"); + expect(classifyTransportPhase({ tlsFailure: true })).toBe("tls"); + expect(classifyTransportPhase({ causeCode: "CERT_HAS_EXPIRED" })).toBe("tls"); + expect(classifyTransportPhase({ causeCode: "ECONNREFUSED" })).toBe("app_connect"); + expect(classifyTransportPhase({ causeCode: "UND_ERR_CONNECT_TIMEOUT" })).toBe("app_connect"); + expect(classifyTransportPhase({ httpStatus: 503 })).toBe("response_headers"); + expect(classifyTransportPhase({ causeCode: "UND_ERR_SOCKET" })).toBe("response_headers"); + expect(classifyTransportPhase({ causeCode: "UND_ERR_BODY_TIMEOUT" })).toBe("response_stream"); + expect(classifyTransportPhase({ streamInterrupted: true })).toBe("response_stream"); + expect(classifyTransportPhase({})).toBe("request"); + }); +}); + +describe("buildManagedTransportFailure", () => { + it("copies only allowlisted fields and sanitizes endpoints", () => { + const event = buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "trusted_env_proxy", + phase: "response_headers", + elapsedMs: 1512.6, + traceId: "trace-1", + proxy: "http://user:pw@proxy.internal:3128/?debug=1", + target: "https://mcp.example.com/stream?session=abc", + httpStatus: 503, + error: Object.assign(new Error("boom"), { code: "UND_ERR_SOCKET" }), + responseHeaders: [ + ["server", "envoy"], + ["set-cookie", "sid=secret"], + ], + sessionIdPresent: true, + errorBody: { body: '{"error":"upstream"}', contentType: "application/json" }, + }); + + expect(event.proxy).toBe("proxy.internal:3128/"); + expect(event.target).toBe("mcp.example.com:443/stream"); + expect(event.causeCode).toBe("UND_ERR_SOCKET"); + expect(event.elapsedMs).toBe(1513); + expect(event.sessionIdPresent).toBe(true); + const serialized = JSON.stringify(event); + expect(serialized).not.toContain("pw"); + expect(serialized).not.toContain("session=abc"); + expect(serialized).not.toContain("sid=secret"); + expect(serialized).not.toContain("debug=1"); + }); + + it("generates a trace id when none is supplied", () => { + const event = buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "direct", + phase: "request", + elapsedMs: 10, + }); + expect(event.traceId).toMatch(/^[0-9a-f]{32}$/); + expect(generateTransportTraceId()).not.toBe(event.traceId); + }); +}); + +describe("emitManagedTransportFailure", () => { + it("writes stable key=value lines under the shared event name", () => { + const lines: string[] = []; + emitManagedTransportFailure( + buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "trusted_env_proxy", + phase: "response_headers", + elapsedMs: 1512, + traceId: "0123456789abcdef0123456789abcdef", + target: "https://mcp.example.com/stream", + httpStatus: 503, + error: Object.assign(new Error("reset"), { code: "UND_ERR_SOCKET", syscall: "read" }), + }), + (line) => lines.push(line), + ); + + expect(lines[0]).toBe( + [ + MANAGED_TRANSPORT_FAILURE_EVENT, + "consumer=mcp", + "operation=tools/list", + "route=trusted_env_proxy", + "target=mcp.example.com:443/stream", + "phase=response_headers", + "http_status=503", + "elapsed_ms=1512", + "cause_code=UND_ERR_SOCKET", + "trace_id=0123456789abcdef0123456789abcdef", + ].join(" "), + ); + expect(lines[1]).toContain("cause_chain=Error/UND_ERR_SOCKET/read"); + }); +}); + +describe("example non-MCP consumer", () => { + it("reuses the contract for a webhook delivery failure without any MCP coupling", () => { + const lines: string[] = []; + const deliverWebhook = (url: string): void => { + const started = 4200; + const failedAt = 4907; + try { + throw Object.assign(new Error("fetch failed"), { + cause: Object.assign(new Error("connect timed out"), { + code: "UND_ERR_CONNECT_TIMEOUT", + }), + }); + } catch (error) { + const causeCode = safeCauseChain(error)[1]?.code; + emitManagedTransportFailure( + buildManagedTransportFailure({ + consumer: "webhook", + operation: "deliver", + route: "direct", + phase: classifyTransportPhase({ causeCode }), + elapsedMs: failedAt - started, + target: url, + error, + }), + (line) => lines.push(line), + ); + } + }; + + deliverWebhook("https://hooks.example.com/pay?signature=secret"); + + expect(lines[0]).toContain("consumer=webhook"); + expect(lines[0]).toContain("phase=app_connect"); + expect(lines[0]).toContain("target=hooks.example.com:443/pay"); + expect(lines.join("\n")).not.toContain("signature"); + }); +}); diff --git a/src/lib/diagnostics/managed-transport.ts b/src/lib/diagnostics/managed-transport.ts new file mode 100644 index 00000000000..3a523ebd2c2 --- /dev/null +++ b/src/lib/diagnostics/managed-transport.ts @@ -0,0 +1,270 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { randomBytes } from "node:crypto"; + +import { sanitizeTraceAttributes } from "../trace"; + +/** + * Failure-only diagnostic contract for managed outbound transports. + * + * A consumer that owns a managed HTTP or socket boundary builds one + * ManagedTransportFailure per failed operation and emits it through + * emitManagedTransportFailure. Every field is copied through an allowlist: + * authorization material, cookies, tokens, query strings, request bodies, + * and application session identifiers never reach the event. Successful + * traffic emits nothing. + */ + +/** The event name shared with OpenShell audit correlation. */ +export const MANAGED_TRANSPORT_FAILURE_EVENT = "managed_transport_failure"; + +/** The transport phase that failed, in connection order. */ +export type ManagedTransportPhase = + | "policy" + | "proxy_connect" + | "tls" + | "app_connect" + | "request" + | "response_headers" + | "response_stream"; + +/** One sanitized entry of a transport error-cause chain. */ +export interface SafeTransportCause { + name?: string; + code?: string; + errno?: number; + syscall?: string; + port?: number; +} + +/** The redacted, structured record of one failed managed-transport operation. */ +export interface ManagedTransportFailure { + consumer: string; + operation: string; + route: string; + phase: ManagedTransportPhase; + traceId: string; + elapsedMs: number; + proxy?: string; + target?: string; + httpStatus?: number; + causeCode?: string; + causeChain: SafeTransportCause[]; + responseServer?: string; + responseVia?: string; + xRequestId?: string; + xEnvoyResponseFlags?: string; + /** Whether an opaque application session identifier was present; never its value. */ + sessionIdPresent?: boolean; + errorBodySnippet?: string; +} + +/** Generates a correlation identifier safe to share across process boundaries. */ +export function generateTransportTraceId(): string { + return randomBytes(16).toString("hex"); +} + +/** Strips credentials, query string, and fragment from a URL-shaped value, keeping host:port/path. */ +export function safeTargetRef(value: string): string { + if (/^https?:\/\//i.test(value)) { + try { + const url = new URL(value); + const port = url.port || (url.protocol === "https:" ? "443" : "80"); + return `${url.hostname}:${port}${url.pathname}`; + } catch { + // fall through to the plain-string stripping below + } + } + const withoutQuery = value.split(/[?#]/, 1)[0] ?? ""; + return withoutQuery.replace(/^[^@]*@/, ""); +} + +const MAX_CAUSE_DEPTH = 8; + +/** + * Walks an error's cause chain and keeps only fields that cannot carry + * request content or credentials. Free-text messages are deliberately + * dropped: undici and Node network errors embed URLs and header values. + */ +export function safeCauseChain(error: unknown): SafeTransportCause[] { + const chain: SafeTransportCause[] = []; + const seen = new Set(); + let current = error; + while (current && typeof current === "object" && !seen.has(current)) { + seen.add(current); + if (chain.length >= MAX_CAUSE_DEPTH) break; + const record = current as Record; + const entry: SafeTransportCause = {}; + if (typeof record.name === "string") entry.name = record.name; + if (typeof record.code === "string") entry.code = record.code; + if (typeof record.errno === "number") entry.errno = record.errno; + if (typeof record.syscall === "string") entry.syscall = record.syscall; + if (typeof record.port === "number") entry.port = record.port; + if (Object.keys(entry).length > 0) chain.push(entry); + current = record.cause; + } + return chain; +} + +/** Picks the diagnostic response headers the contract allows, dropping everything else. */ +export function pickSafeResponseHeaders( + headers: Iterable<[string, string]>, +): Pick< + ManagedTransportFailure, + "responseServer" | "responseVia" | "xRequestId" | "xEnvoyResponseFlags" +> { + const picked: ReturnType = {}; + for (const [rawName, value] of headers) { + const name = rawName.toLowerCase(); + if (name === "server") picked.responseServer = value; + else if (name === "via") picked.responseVia = value; + else if (name === "x-request-id") picked.xRequestId = value; + else if (name === "x-envoy-response-flags") picked.xEnvoyResponseFlags = value; + } + return picked; +} + +const MAX_ERROR_BODY_SNIPPET = 512; +const TEXTUAL_BODY_TYPES = /^(?:text\/|application\/(?:json|problem\+json)\b)/; + +/** + * Bounds a non-2xx error body to a short redacted snippet. Non-textual + * content types yield nothing, and the caller must pass an already-consumed + * copy so streaming consumption stays untouched. + */ +export function boundedErrorBodySnippet( + body: string, + contentType: string | undefined, +): string | undefined { + if (!contentType || !TEXTUAL_BODY_TYPES.test(contentType)) return undefined; + const bounded = body.slice(0, MAX_ERROR_BODY_SNIPPET); + const sanitized = sanitizeTraceAttributes({ body: bounded }).body; + return typeof sanitized === "string" ? sanitized : undefined; +} + +/** Maps a sanitized cause code and HTTP outcome onto the failing phase. */ +export function classifyTransportPhase(input: { + policyDenied?: boolean; + causeCode?: string; + tlsFailure?: boolean; + httpStatus?: number; + streamInterrupted?: boolean; +}): ManagedTransportPhase { + if (input.policyDenied) return "policy"; + if (input.tlsFailure) return "tls"; + const code = input.causeCode ?? ""; + if ( + /^(?:UND_ERR_CONNECT_TIMEOUT|ECONNREFUSED|EHOSTUNREACH|ENETUNREACH|ENOTFOUND|EAI_AGAIN)$/.test( + code, + ) + ) { + return "app_connect"; + } + if (/CERT|TLS|SSL/.test(code)) return "tls"; + if (input.streamInterrupted) return "response_stream"; + if (input.httpStatus !== undefined) return "response_headers"; + if (/^(?:UND_ERR_SOCKET|ECONNRESET|EPIPE|UND_ERR_HEADERS_TIMEOUT)$/.test(code)) { + return "response_headers"; + } + if (/^(?:UND_ERR_BODY_TIMEOUT|UND_ERR_ABORTED)$/.test(code)) return "response_stream"; + return "request"; +} + +/** Builder input: unsanitized operational context plus the raw error. */ +export interface ManagedTransportFailureInput { + consumer: string; + operation: string; + route: string; + phase: ManagedTransportPhase; + elapsedMs: number; + traceId?: string; + proxy?: string; + target?: string; + httpStatus?: number; + error?: unknown; + responseHeaders?: Iterable<[string, string]>; + sessionIdPresent?: boolean; + errorBody?: { body: string; contentType?: string }; +} + +/** + * Builds the redacted event from operational context. Only allowlisted + * fields are copied; proxy and target lose credentials and query strings, + * and the error contributes nothing beyond its safe cause chain. + */ +export function buildManagedTransportFailure( + input: ManagedTransportFailureInput, +): ManagedTransportFailure { + const causeChain = safeCauseChain(input.error); + return { + consumer: input.consumer, + operation: input.operation, + route: input.route, + phase: input.phase, + traceId: input.traceId ?? generateTransportTraceId(), + elapsedMs: Math.max(0, Math.round(input.elapsedMs)), + ...(input.proxy === undefined ? {} : { proxy: safeTargetRef(input.proxy) }), + ...(input.target === undefined ? {} : { target: safeTargetRef(input.target) }), + ...(input.httpStatus === undefined ? {} : { httpStatus: input.httpStatus }), + ...(causeChain[0]?.code === undefined ? {} : { causeCode: causeChain[0].code }), + causeChain, + ...(input.responseHeaders === undefined ? {} : pickSafeResponseHeaders(input.responseHeaders)), + ...(input.sessionIdPresent === undefined ? {} : { sessionIdPresent: input.sessionIdPresent }), + ...(input.errorBody === undefined + ? {} + : (() => { + const snippet = boundedErrorBodySnippet( + input.errorBody.body, + input.errorBody.contentType, + ); + return snippet === undefined ? {} : { errorBodySnippet: snippet }; + })()), + }; +} + +/** + * Formats the event as stable key=value lines under the shared event name + * and hands it to write; stderr by default. Consumers call this on failure + * only. + */ +export function emitManagedTransportFailure( + event: ManagedTransportFailure, + write: (line: string) => void = (line) => process.stderr.write(`${line}\n`), +): void { + const pairs: string[] = [MANAGED_TRANSPORT_FAILURE_EVENT]; + const push = (key: string, value: string | number | boolean | undefined) => { + if (value !== undefined) pairs.push(`${key}=${value}`); + }; + push("consumer", event.consumer); + push("operation", event.operation); + push("route", event.route); + push("proxy", event.proxy); + push("target", event.target); + push("phase", event.phase); + push("http_status", event.httpStatus); + push("elapsed_ms", event.elapsedMs); + push("cause_code", event.causeCode); + push("response_server", event.responseServer); + push("response_via", event.responseVia); + push("x_request_id", event.xRequestId); + push("x_envoy_response_flags", event.xEnvoyResponseFlags); + push("session_id_present", event.sessionIdPresent); + push("trace_id", event.traceId); + write(pairs.join(" ")); + if (event.causeChain.length > 0) { + const chain = event.causeChain + .map((cause) => + [cause.name, cause.code, cause.syscall, cause.errno, cause.port] + .filter((part) => part !== undefined) + .join("/"), + ) + .join(" -> "); + write(`${MANAGED_TRANSPORT_FAILURE_EVENT} trace_id=${event.traceId} cause_chain=${chain}`); + } + if (event.errorBodySnippet !== undefined) { + write( + `${MANAGED_TRANSPORT_FAILURE_EVENT} trace_id=${event.traceId} error_body=${JSON.stringify(event.errorBodySnippet)}`, + ); + } +} From 0daaa57ab16d5f6ae71af98ace35040e26c4831c Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:01:51 +0200 Subject: [PATCH 02/12] fix(diagnostics): encode every emitted log field against injection The advisor's PRA-1 blocker is right: the formatter wrote allowlisted header values and required string fields as raw key=value text, so a delimiter- or credential-bearing upstream value could forge a second record or leak. Route every emitted string through encodeLogField, which redacts through the shared trace sanitizer, bounds length, and JSON-quotes any value carrying a control character, whitespace, quote, or separator. Add a formatter test with newline, CRLF, forged-field, and credential-shaped values proving one-record, no-leak output. Refs #7957 Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- src/lib/diagnostics/managed-transport.test.ts | 45 ++++++++++++++++++ src/lib/diagnostics/managed-transport.ts | 47 ++++++++++++++----- 2 files changed, 80 insertions(+), 12 deletions(-) diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts index 879a0d94dff..9d3aa5aae7c 100644 --- a/src/lib/diagnostics/managed-transport.test.ts +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -8,6 +8,7 @@ import { buildManagedTransportFailure, classifyTransportPhase, emitManagedTransportFailure, + encodeLogField, generateTransportTraceId, MANAGED_TRANSPORT_FAILURE_EVENT, pickSafeResponseHeaders, @@ -193,6 +194,50 @@ describe("emitManagedTransportFailure", () => { ); expect(lines[1]).toContain("cause_chain=Error/UND_ERR_SOCKET/read"); }); + + it("neutralizes delimiter- and credential-bearing values in every emitted field", () => { + const lines: string[] = []; + emitManagedTransportFailure( + buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list\nmanaged_transport_failure phase=policy", + route: "trusted_env_proxy", + phase: "response_headers", + elapsedMs: 10, + traceId: "trace-1", + httpStatus: 503, + responseHeaders: [ + ["server", "envoy value=forged"], + ["x-request-id", "id\r\ninjected=1"], + ["via", "Bearer sk-abcdef0123456789abcdef0123456789"], + ], + }), + (line) => lines.push(line), + ); + + // One record: no injected line break reached the physical output. + expect(lines).toHaveLength(1); + const line = lines[0]; + expect(line).not.toContain("\n"); + expect(line).not.toContain("\r"); + // The genuine phase field is intact and appears once as a real top-level field. + expect(line).toContain("phase=response_headers"); + // Delimiter-bearing values are JSON-quoted, so their `key=value` fragments + // live inside a quoted token rather than as forged fields. + expect(line).toContain('operation="tools/list\\nmanaged_transport_failure phase=policy"'); + expect(line).toContain('x_request_id="id\\r\\ninjected=1"'); + // A credential-shaped header value is redacted, not disclosed. + expect(line).not.toContain("sk-abcdef0123456789abcdef0123456789"); + }); +}); + +describe("encodeLogField", () => { + it("quotes delimiter-bearing values and passes plain tokens through", () => { + expect(encodeLogField("tools/list")).toBe("tools/list"); + expect(encodeLogField("1.1 proxy")).toBe('"1.1 proxy"'); + expect(encodeLogField("a\nb")).toBe('"a\\nb"'); + expect(encodeLogField(503)).toBe("503"); + }); }); describe("example non-MCP consumer", () => { diff --git a/src/lib/diagnostics/managed-transport.ts b/src/lib/diagnostics/managed-transport.ts index 3a523ebd2c2..618157531dc 100644 --- a/src/lib/diagnostics/managed-transport.ts +++ b/src/lib/diagnostics/managed-transport.ts @@ -223,9 +223,28 @@ export function buildManagedTransportFailure( }; } +const MAX_LOG_FIELD = 256; + +/** + * Encodes one value for a single-record key=value log line. Every string is + * redacted through the shared trace sanitizer, then length-bounded, and any + * value carrying a control character, whitespace, quote, or `=` is JSON + * quoted so it cannot inject a line break or forge a second field. Even an + * allowlisted upstream header reaches the line only through this policy. + */ +export function encodeLogField(value: string | number | boolean): string { + if (typeof value !== "string") return String(value); + const redacted = sanitizeTraceAttributes({ value }).value; + const text = (typeof redacted === "string" ? redacted : value).slice(0, MAX_LOG_FIELD); + if (/[\s="\\]|[\u0000-\u001f\u007f]/.test(text)) return JSON.stringify(text); + return text; +} + /** - * Formats the event as stable key=value lines under the shared event name - * and hands it to write; stderr by default. Consumers call this on failure + * Formats the event as stable single-record key=value lines under the shared + * event name and hands each to write; stderr by default. Every emitted string + * passes through encodeLogField, so a delimiter- or credential-bearing + * upstream value cannot forge records or leak. Consumers call this on failure * only. */ export function emitManagedTransportFailure( @@ -234,7 +253,7 @@ export function emitManagedTransportFailure( ): void { const pairs: string[] = [MANAGED_TRANSPORT_FAILURE_EVENT]; const push = (key: string, value: string | number | boolean | undefined) => { - if (value !== undefined) pairs.push(`${key}=${value}`); + if (value !== undefined) pairs.push(`${key}=${encodeLogField(value)}`); }; push("consumer", event.consumer); push("operation", event.operation); @@ -253,18 +272,22 @@ export function emitManagedTransportFailure( push("trace_id", event.traceId); write(pairs.join(" ")); if (event.causeChain.length > 0) { - const chain = event.causeChain - .map((cause) => - [cause.name, cause.code, cause.syscall, cause.errno, cause.port] - .filter((part) => part !== undefined) - .join("/"), - ) - .join(" -> "); - write(`${MANAGED_TRANSPORT_FAILURE_EVENT} trace_id=${event.traceId} cause_chain=${chain}`); + const chain = encodeLogField( + event.causeChain + .map((cause) => + [cause.name, cause.code, cause.syscall, cause.errno, cause.port] + .filter((part) => part !== undefined) + .join("/"), + ) + .join(" -> "), + ); + write( + `${MANAGED_TRANSPORT_FAILURE_EVENT} trace_id=${encodeLogField(event.traceId)} cause_chain=${chain}`, + ); } if (event.errorBodySnippet !== undefined) { write( - `${MANAGED_TRANSPORT_FAILURE_EVENT} trace_id=${event.traceId} error_body=${JSON.stringify(event.errorBodySnippet)}`, + `${MANAGED_TRANSPORT_FAILURE_EVENT} trace_id=${encodeLogField(event.traceId)} error_body=${JSON.stringify(event.errorBodySnippet)}`, ); } } From 007583c1825f7a809c3c814d449e68dc7f2f2a80 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:13:49 +0200 Subject: [PATCH 03/12] fix(diagnostics): redact untrusted fields at build time Addresses the advisor's PRA-2 blocker: redaction happened only in the line formatter, so an alternate consumer serializing the built event could disclose a credential embedded in an operation, route, consumer name, or allowlisted header. Redact every untrusted string field in buildManagedTransportFailure so the returned object is safe by construction; emission encoding stays as defense in depth. Also select the first cause-chain code for the top-level cause_code so a wrapped transport error surfaces its network code (advisor PRA-1 correctness). Refs #7957 Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- src/lib/diagnostics/managed-transport.test.ts | 33 +++++++++++ src/lib/diagnostics/managed-transport.ts | 55 ++++++++++++------- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts index 9d3aa5aae7c..698b6cb80c3 100644 --- a/src/lib/diagnostics/managed-transport.test.ts +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -147,6 +147,39 @@ describe("buildManagedTransportFailure", () => { expect(serialized).not.toContain("debug=1"); }); + it("redacts untrusted fields in the built object before any formatting", () => { + const token = "sk-abcdef0123456789abcdef0123456789"; + const event = buildManagedTransportFailure({ + consumer: "mcp", + operation: `tools/list ${token}`, + route: "trusted_env_proxy", + phase: "response_headers", + elapsedMs: 10, + responseHeaders: [["x-request-id", `req ${token}`]], + }); + + // The event object itself is safe to serialize without the line formatter. + const serialized = JSON.stringify(event); + expect(serialized).not.toContain(token); + expect(event.operation).not.toContain(token); + expect(event.xRequestId).not.toContain(token); + }); + + it("surfaces a nested transport cause code at the top level", () => { + const event = buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "direct", + phase: "app_connect", + elapsedMs: 10, + error: new Error("fetch failed", { + cause: Object.assign(new Error("connect refused"), { code: "ECONNREFUSED" }), + }), + }); + + expect(event.causeCode).toBe("ECONNREFUSED"); + }); + it("generates a trace id when none is supplied", () => { const event = buildManagedTransportFailure({ consumer: "mcp", diff --git a/src/lib/diagnostics/managed-transport.ts b/src/lib/diagnostics/managed-transport.ts index 618157531dc..1652b182eb0 100644 --- a/src/lib/diagnostics/managed-transport.ts +++ b/src/lib/diagnostics/managed-transport.ts @@ -107,7 +107,18 @@ export function safeCauseChain(error: unknown): SafeTransportCause[] { return chain; } -/** Picks the diagnostic response headers the contract allows, dropping everything else. */ +/** + * Redacts one untrusted string field through the shared trace sanitizer so a + * credential embedded by an upstream cannot survive into the built event. + * This runs at build time; the event object is safe to serialize or forward + * before it ever reaches the line formatter. + */ +export function redactField(value: string): string { + const redacted = sanitizeTraceAttributes({ value }).value; + return typeof redacted === "string" ? redacted : value; +} + +/** Picks the diagnostic response headers the contract allows, redacting each kept value. */ export function pickSafeResponseHeaders( headers: Iterable<[string, string]>, ): Pick< @@ -117,10 +128,10 @@ export function pickSafeResponseHeaders( const picked: ReturnType = {}; for (const [rawName, value] of headers) { const name = rawName.toLowerCase(); - if (name === "server") picked.responseServer = value; - else if (name === "via") picked.responseVia = value; - else if (name === "x-request-id") picked.xRequestId = value; - else if (name === "x-envoy-response-flags") picked.xEnvoyResponseFlags = value; + if (name === "server") picked.responseServer = redactField(value); + else if (name === "via") picked.responseVia = redactField(value); + else if (name === "x-request-id") picked.xRequestId = redactField(value); + else if (name === "x-envoy-response-flags") picked.xEnvoyResponseFlags = redactField(value); } return picked; } @@ -189,25 +200,31 @@ export interface ManagedTransportFailureInput { } /** - * Builds the redacted event from operational context. Only allowlisted - * fields are copied; proxy and target lose credentials and query strings, - * and the error contributes nothing beyond its safe cause chain. + * Builds the redacted event from operational context. Every untrusted string + * field is redacted here so the returned object is safe to serialize or + * forward without the line formatter: consumer, operation, route, endpoints, + * and allowlisted headers cannot carry a credential. proxy and target also + * lose credentials and query strings, and the error contributes nothing + * beyond its safe cause chain. The top-level causeCode is the first cause in + * the chain that carries a code, since wrapped errors keep the network code + * in a nested cause. */ export function buildManagedTransportFailure( input: ManagedTransportFailureInput, ): ManagedTransportFailure { const causeChain = safeCauseChain(input.error); + const causeCode = causeChain.find((cause) => cause.code !== undefined)?.code; return { - consumer: input.consumer, - operation: input.operation, - route: input.route, + consumer: redactField(input.consumer), + operation: redactField(input.operation), + route: redactField(input.route), phase: input.phase, traceId: input.traceId ?? generateTransportTraceId(), elapsedMs: Math.max(0, Math.round(input.elapsedMs)), - ...(input.proxy === undefined ? {} : { proxy: safeTargetRef(input.proxy) }), - ...(input.target === undefined ? {} : { target: safeTargetRef(input.target) }), + ...(input.proxy === undefined ? {} : { proxy: redactField(safeTargetRef(input.proxy)) }), + ...(input.target === undefined ? {} : { target: redactField(safeTargetRef(input.target)) }), ...(input.httpStatus === undefined ? {} : { httpStatus: input.httpStatus }), - ...(causeChain[0]?.code === undefined ? {} : { causeCode: causeChain[0].code }), + ...(causeCode === undefined ? {} : { causeCode }), causeChain, ...(input.responseHeaders === undefined ? {} : pickSafeResponseHeaders(input.responseHeaders)), ...(input.sessionIdPresent === undefined ? {} : { sessionIdPresent: input.sessionIdPresent }), @@ -226,11 +243,11 @@ export function buildManagedTransportFailure( const MAX_LOG_FIELD = 256; /** - * Encodes one value for a single-record key=value log line. Every string is - * redacted through the shared trace sanitizer, then length-bounded, and any - * value carrying a control character, whitespace, quote, or `=` is JSON - * quoted so it cannot inject a line break or forge a second field. Even an - * allowlisted upstream header reaches the line only through this policy. + * Encodes one value for a single-record key=value log line. The value is + * redacted again (defense in depth on top of the build-time redaction), + * length-bounded, and any value carrying a control character, whitespace, + * quote, or `=` is JSON quoted so it cannot inject a line break or forge a + * second field. */ export function encodeLogField(value: string | number | boolean): string { if (typeof value !== "string") return String(value); From 3106eba7a4514054ae6167041f1139bca1b127f4 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:07:33 +0200 Subject: [PATCH 04/12] fix(diagnostics): constrain cause metadata and supplied trace ids Addresses the advisor's PRA-3 blocker and trace-id warning: copied error names, codes, and syscalls are now bounded identifier tokens (anything longer or carrying other characters becomes ), and a supplied trace id is kept only when it matches the safe id shape, falling back to a generated one otherwise. The built event stays safe to serialize regardless of what an upstream library or caller puts in error metadata. Builder tests cover credential-shaped and delimiter-bearing cause fields and trace ids. Refs #7957 Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- src/lib/diagnostics/managed-transport.test.ts | 40 +++++++++++++++++++ src/lib/diagnostics/managed-transport.ts | 28 +++++++++++-- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts index 698b6cb80c3..c15e34c44a9 100644 --- a/src/lib/diagnostics/managed-transport.test.ts +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -165,6 +165,46 @@ describe("buildManagedTransportFailure", () => { expect(event.xRequestId).not.toContain(token); }); + it("constrains cause metadata and supplied trace ids in the built object", () => { + const token = "sk-abcdef0123456789abcdef0123456789"; + const event = buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "direct", + phase: "request", + elapsedMs: 10, + traceId: `Bearer ${token}\nphase=policy`, + error: Object.assign(new Error("x"), { + name: `Leaky ${token}`, + code: "ECONNRESET extra=1", + syscall: "read\nwrite", + }), + }); + + const serialized = JSON.stringify(event); + expect(serialized).not.toContain(token); + expect(serialized).not.toContain("extra=1"); + expect(serialized).not.toContain("\\n"); + expect(event.traceId).toMatch(/^[0-9a-f]{32}$/); + expect(event.causeChain[0]).toEqual({ + name: "", + code: "", + syscall: "", + }); + }); + + it("keeps a well-formed supplied trace id", () => { + const event = buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "direct", + phase: "request", + elapsedMs: 10, + traceId: "trace-0123456789", + }); + expect(event.traceId).toBe("trace-0123456789"); + }); + it("surfaces a nested transport cause code at the top level", () => { const event = buildManagedTransportFailure({ consumer: "mcp", diff --git a/src/lib/diagnostics/managed-transport.ts b/src/lib/diagnostics/managed-transport.ts index 1652b182eb0..969f8d92a26 100644 --- a/src/lib/diagnostics/managed-transport.ts +++ b/src/lib/diagnostics/managed-transport.ts @@ -65,6 +65,14 @@ export function generateTransportTraceId(): string { return randomBytes(16).toString("hex"); } +const TRACE_ID_SHAPE = /^[A-Za-z0-9-]{8,64}$/; + +/** Keeps a well-formed supplied trace id and replaces anything else with a generated one. */ +function safeSuppliedTraceId(supplied: string | undefined): string { + if (supplied !== undefined && TRACE_ID_SHAPE.test(supplied)) return supplied; + return generateTransportTraceId(); +} + /** Strips credentials, query string, and fragment from a URL-shaped value, keeping host:port/path. */ export function safeTargetRef(value: string): string { if (/^https?:\/\//i.test(value)) { @@ -81,6 +89,18 @@ export function safeTargetRef(value: string): string { } const MAX_CAUSE_DEPTH = 8; +const MAX_CAUSE_TOKEN = 64; +const CAUSE_TOKEN_SHAPE = /^[A-Za-z0-9_.-]+$/; + +/** + * Constrains one copied error identifier: error names, codes, and syscalls + * are short identifier tokens, so anything longer or carrying other + * characters is replaced rather than propagated into a serializable event. + */ +function safeCauseToken(value: string): string { + const bounded = value.slice(0, MAX_CAUSE_TOKEN); + return CAUSE_TOKEN_SHAPE.test(bounded) ? bounded : ""; +} /** * Walks an error's cause chain and keeps only fields that cannot carry @@ -96,10 +116,10 @@ export function safeCauseChain(error: unknown): SafeTransportCause[] { if (chain.length >= MAX_CAUSE_DEPTH) break; const record = current as Record; const entry: SafeTransportCause = {}; - if (typeof record.name === "string") entry.name = record.name; - if (typeof record.code === "string") entry.code = record.code; + if (typeof record.name === "string") entry.name = safeCauseToken(record.name); + if (typeof record.code === "string") entry.code = safeCauseToken(record.code); if (typeof record.errno === "number") entry.errno = record.errno; - if (typeof record.syscall === "string") entry.syscall = record.syscall; + if (typeof record.syscall === "string") entry.syscall = safeCauseToken(record.syscall); if (typeof record.port === "number") entry.port = record.port; if (Object.keys(entry).length > 0) chain.push(entry); current = record.cause; @@ -219,7 +239,7 @@ export function buildManagedTransportFailure( operation: redactField(input.operation), route: redactField(input.route), phase: input.phase, - traceId: input.traceId ?? generateTransportTraceId(), + traceId: safeSuppliedTraceId(input.traceId), elapsedMs: Math.max(0, Math.round(input.elapsedMs)), ...(input.proxy === undefined ? {} : { proxy: redactField(safeTargetRef(input.proxy)) }), ...(input.target === undefined ? {} : { target: redactField(safeTargetRef(input.target)) }), From e14b0c4f6775f8864f07d8dbe4a164de5ca86165 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Tue, 11 Aug 2026 10:19:04 +0200 Subject: [PATCH 05/12] fix(diagnostics): restrict body capture to failures and encode the snippet Addresses both advisor warnings on the managed-transport failure contract. PRA-1: buildManagedTransportFailure accepted an error body with no condition on the HTTP status, while boundedErrorBodySnippet documents non-2xx capture only. A caller could therefore attach the body of a successful response to failure diagnostics and retain response content the module promises not to keep. The snippet is now carried only when httpStatus is present and outside 200-299. An absent status is not treated as a failure status either: a transport error that never produced a response has no body to capture. PRA-2: the emitter serialized errorBodySnippet with JSON.stringify while every other field went through encodeLogField. JSON.stringify quotes and escapes, so it prevented record forging, but it neither redacts nor bounds. An event object constructed directly rather than through the builder could therefore carry a credential-bearing snippet straight into diagnostic output, bypassing the build-time redaction boundary. The snippet now goes through encodeLogField like everything else. encodeLogField takes an optional bound so the snippet keeps its documented 512-character limit rather than being truncated to the 256-character field limit. Two regression tests, both verified to fail without their fix: a builder test asserting a 200 response with a textual body yields no snippet and does not serialize its content, and an emitter test passing a hand-constructed event whose snippet carries a bearer token and a forged event line, asserting the credential is absent, the record count is unchanged, and no newline escapes. 20 tests pass in normal and shuffled order. Biome check and the src typecheck are clean; the two pre-existing banner.ts errors about an unbuilt nemoclaw/dist artifact reproduce identically on an unmodified tree. Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- src/lib/diagnostics/managed-transport.test.ts | 60 +++++++++++++++++++ src/lib/diagnostics/managed-transport.ts | 30 +++++++--- 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts index c15e34c44a9..eb9e6cb9393 100644 --- a/src/lib/diagnostics/managed-transport.test.ts +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -147,6 +147,41 @@ describe("buildManagedTransportFailure", () => { expect(serialized).not.toContain("debug=1"); }); + it("refuses an error body when the response succeeded", () => { + const withSuccess = buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "trusted_env_proxy", + phase: "response_headers", + elapsedMs: 12, + httpStatus: 200, + errorBody: { body: '{"ok":"payload"}', contentType: "application/json" }, + }); + expect(withSuccess.errorBodySnippet).toBeUndefined(); + expect(JSON.stringify(withSuccess)).not.toContain("payload"); + + const withoutStatus = buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "trusted_env_proxy", + phase: "connect", + elapsedMs: 12, + errorBody: { body: '{"ok":"payload"}', contentType: "application/json" }, + }); + expect(withoutStatus.errorBodySnippet).toBeUndefined(); + + const withFailure = buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "trusted_env_proxy", + phase: "response_headers", + elapsedMs: 12, + httpStatus: 502, + errorBody: { body: '{"error":"upstream"}', contentType: "application/json" }, + }); + expect(withFailure.errorBodySnippet).toContain("upstream"); + }); + it("redacts untrusted fields in the built object before any formatting", () => { const token = "sk-abcdef0123456789abcdef0123456789"; const event = buildManagedTransportFailure({ @@ -268,6 +303,31 @@ describe("emitManagedTransportFailure", () => { expect(lines[1]).toContain("cause_chain=Error/UND_ERR_SOCKET/read"); }); + it("encodes an error-body snippet on a directly constructed event", () => { + const lines: string[] = []; + emitManagedTransportFailure( + { + consumer: "mcp", + operation: "tools/list", + route: "trusted_env_proxy", + phase: "response_body", + traceId: "trace-9", + elapsedMs: 5, + causeChain: [], + errorBodySnippet: + "authorization: Bearer sk-live-not-a-real-key\nmanaged_transport_failure phase=forged", + }, + (line) => lines.push(line), + ); + + const emitted = lines.join("\n"); + expect(emitted).not.toContain("sk-live-not-a-real-key"); + expect(lines).toHaveLength(2); + expect(lines[1]).toContain("error_body="); + expect(lines[1]).not.toMatch(/\n/); + expect(lines.filter((line) => line.startsWith("managed_transport_failure "))).toHaveLength(2); + }); + it("neutralizes delimiter- and credential-bearing values in every emitted field", () => { const lines: string[] = []; emitManagedTransportFailure( diff --git a/src/lib/diagnostics/managed-transport.ts b/src/lib/diagnostics/managed-transport.ts index 969f8d92a26..b4cb6d05d2f 100644 --- a/src/lib/diagnostics/managed-transport.ts +++ b/src/lib/diagnostics/managed-transport.ts @@ -164,6 +164,16 @@ const TEXTUAL_BODY_TYPES = /^(?:text\/|application\/(?:json|problem\+json)\b)/; * content types yield nothing, and the caller must pass an already-consumed * copy so streaming consumption stays untouched. */ +/** + * Whether a status is a captured failure status. Body capture is restricted + * to non-2xx responses, so a caller cannot attach a successful response body + * to failure diagnostics. An absent status is not a failure status: a + * transport error that never produced a response has no body to capture. + */ +function isErrorStatus(httpStatus: number | undefined): boolean { + return httpStatus !== undefined && (httpStatus < 200 || httpStatus >= 300); +} + export function boundedErrorBodySnippet( body: string, contentType: string | undefined, @@ -248,7 +258,7 @@ export function buildManagedTransportFailure( causeChain, ...(input.responseHeaders === undefined ? {} : pickSafeResponseHeaders(input.responseHeaders)), ...(input.sessionIdPresent === undefined ? {} : { sessionIdPresent: input.sessionIdPresent }), - ...(input.errorBody === undefined + ...(input.errorBody === undefined || !isErrorStatus(input.httpStatus) ? {} : (() => { const snippet = boundedErrorBodySnippet( @@ -265,14 +275,17 @@ const MAX_LOG_FIELD = 256; /** * Encodes one value for a single-record key=value log line. The value is * redacted again (defense in depth on top of the build-time redaction), - * length-bounded, and any value carrying a control character, whitespace, + * length-bounded to maxLength, and any value carrying a control character, whitespace, * quote, or `=` is JSON quoted so it cannot inject a line break or forge a * second field. */ -export function encodeLogField(value: string | number | boolean): string { +export function encodeLogField( + value: string | number | boolean, + maxLength: number = MAX_LOG_FIELD, +): string { if (typeof value !== "string") return String(value); const redacted = sanitizeTraceAttributes({ value }).value; - const text = (typeof redacted === "string" ? redacted : value).slice(0, MAX_LOG_FIELD); + const text = (typeof redacted === "string" ? redacted : value).slice(0, maxLength); if (/[\s="\\]|[\u0000-\u001f\u007f]/.test(text)) return JSON.stringify(text); return text; } @@ -280,9 +293,10 @@ export function encodeLogField(value: string | number | boolean): string { /** * Formats the event as stable single-record key=value lines under the shared * event name and hands each to write; stderr by default. Every emitted string - * passes through encodeLogField, so a delimiter- or credential-bearing - * upstream value cannot forge records or leak. Consumers call this on failure - * only. + * passes through encodeLogField, including the error-body snippet, so a + * delimiter- or credential-bearing upstream value cannot forge records or + * leak even when the event object was constructed directly rather than by + * buildManagedTransportFailure. Consumers call this on failure only. */ export function emitManagedTransportFailure( event: ManagedTransportFailure, @@ -324,7 +338,7 @@ export function emitManagedTransportFailure( } if (event.errorBodySnippet !== undefined) { write( - `${MANAGED_TRANSPORT_FAILURE_EVENT} trace_id=${encodeLogField(event.traceId)} error_body=${JSON.stringify(event.errorBodySnippet)}`, + `${MANAGED_TRANSPORT_FAILURE_EVENT} trace_id=${encodeLogField(event.traceId)} error_body=${encodeLogField(event.errorBodySnippet, MAX_ERROR_BODY_SNIPPET)}`, ); } } From 4783135b50d088c01a5b043179683575cc8a758c Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Tue, 11 Aug 2026 10:23:39 -0700 Subject: [PATCH 06/12] fix(diagnostics): harden managed transport failures Signed-off-by: Apurv Kumaria --- src/lib/diagnostics/managed-transport.test.ts | 22 +++++++++++++++++-- src/lib/diagnostics/managed-transport.ts | 9 ++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts index eb9e6cb9393..560ba6e67e3 100644 --- a/src/lib/diagnostics/managed-transport.test.ts +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -104,6 +104,9 @@ describe("classifyTransportPhase", () => { expect(classifyTransportPhase({ policyDenied: true })).toBe("policy"); expect(classifyTransportPhase({ tlsFailure: true })).toBe("tls"); expect(classifyTransportPhase({ causeCode: "CERT_HAS_EXPIRED" })).toBe("tls"); + expect(classifyTransportPhase({ proxyConnectFailure: true, causeCode: "ECONNREFUSED" })).toBe( + "proxy_connect", + ); expect(classifyTransportPhase({ causeCode: "ECONNREFUSED" })).toBe("app_connect"); expect(classifyTransportPhase({ causeCode: "UND_ERR_CONNECT_TIMEOUT" })).toBe("app_connect"); expect(classifyTransportPhase({ httpStatus: 503 })).toBe("response_headers"); @@ -164,7 +167,7 @@ describe("buildManagedTransportFailure", () => { consumer: "mcp", operation: "tools/list", route: "trusted_env_proxy", - phase: "connect", + phase: "app_connect", elapsedMs: 12, errorBody: { body: '{"ok":"payload"}', contentType: "application/json" }, }); @@ -240,6 +243,21 @@ describe("buildManagedTransportFailure", () => { expect(event.traceId).toBe("trace-0123456789"); }); + it("replaces a credential-shaped supplied trace id before serialization", () => { + const supplied = "sk-abcdef0123456789abcdef0123456789"; + const event = buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "direct", + phase: "request", + elapsedMs: 10, + traceId: supplied, + }); + + expect(event.traceId).toMatch(/^[0-9a-f]{32}$/); + expect(JSON.stringify(event)).not.toContain(supplied); + }); + it("surfaces a nested transport cause code at the top level", () => { const event = buildManagedTransportFailure({ consumer: "mcp", @@ -310,7 +328,7 @@ describe("emitManagedTransportFailure", () => { consumer: "mcp", operation: "tools/list", route: "trusted_env_proxy", - phase: "response_body", + phase: "response_stream", traceId: "trace-9", elapsedMs: 5, causeChain: [], diff --git a/src/lib/diagnostics/managed-transport.ts b/src/lib/diagnostics/managed-transport.ts index b4cb6d05d2f..caa58fd3237 100644 --- a/src/lib/diagnostics/managed-transport.ts +++ b/src/lib/diagnostics/managed-transport.ts @@ -67,9 +67,12 @@ export function generateTransportTraceId(): string { const TRACE_ID_SHAPE = /^[A-Za-z0-9-]{8,64}$/; -/** Keeps a well-formed supplied trace id and replaces anything else with a generated one. */ +/** Keeps a supplied trace id only when its shape is allowed and sanitization leaves it unchanged. */ function safeSuppliedTraceId(supplied: string | undefined): string { - if (supplied !== undefined && TRACE_ID_SHAPE.test(supplied)) return supplied; + if (supplied !== undefined && TRACE_ID_SHAPE.test(supplied)) { + const redacted = redactField(supplied); + if (redacted === supplied) return supplied; + } return generateTransportTraceId(); } @@ -187,6 +190,7 @@ export function boundedErrorBodySnippet( /** Maps a sanitized cause code and HTTP outcome onto the failing phase. */ export function classifyTransportPhase(input: { policyDenied?: boolean; + proxyConnectFailure?: boolean; causeCode?: string; tlsFailure?: boolean; httpStatus?: number; @@ -194,6 +198,7 @@ export function classifyTransportPhase(input: { }): ManagedTransportPhase { if (input.policyDenied) return "policy"; if (input.tlsFailure) return "tls"; + if (input.proxyConnectFailure) return "proxy_connect"; const code = input.causeCode ?? ""; if ( /^(?:UND_ERR_CONNECT_TIMEOUT|ECONNREFUSED|EHOSTUNREACH|ENETUNREACH|ENOTFOUND|EAI_AGAIN)$/.test( From 7df15ef132e67c3037a898db8257e03cb30dc762 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:13:56 +0200 Subject: [PATCH 07/12] fix(diagnostics): adopt the documented transport_phase vocabulary Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- src/lib/diagnostics/managed-transport.test.ts | 61 +++++++++++++++++-- src/lib/diagnostics/managed-transport.ts | 25 +++++--- 2 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts index 560ba6e67e3..3e7dcb71530 100644 --- a/src/lib/diagnostics/managed-transport.test.ts +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -1,9 +1,13 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + import { describe, expect, it } from "vitest"; import { + type ManagedTransportPhase, boundedErrorBodySnippet, buildManagedTransportFailure, classifyTransportPhase, @@ -105,7 +109,7 @@ describe("classifyTransportPhase", () => { expect(classifyTransportPhase({ tlsFailure: true })).toBe("tls"); expect(classifyTransportPhase({ causeCode: "CERT_HAS_EXPIRED" })).toBe("tls"); expect(classifyTransportPhase({ proxyConnectFailure: true, causeCode: "ECONNREFUSED" })).toBe( - "proxy_connect", + "connect", ); expect(classifyTransportPhase({ causeCode: "ECONNREFUSED" })).toBe("app_connect"); expect(classifyTransportPhase({ causeCode: "UND_ERR_CONNECT_TIMEOUT" })).toBe("app_connect"); @@ -134,7 +138,7 @@ describe("buildManagedTransportFailure", () => { ["server", "envoy"], ["set-cookie", "sid=secret"], ], - sessionIdPresent: true, + sessionPresent: true, errorBody: { body: '{"error":"upstream"}', contentType: "application/json" }, }); @@ -142,7 +146,7 @@ describe("buildManagedTransportFailure", () => { expect(event.target).toBe("mcp.example.com:443/stream"); expect(event.causeCode).toBe("UND_ERR_SOCKET"); expect(event.elapsedMs).toBe(1513); - expect(event.sessionIdPresent).toBe(true); + expect(event.sessionPresent).toBe(true); const serialized = JSON.stringify(event); expect(serialized).not.toContain("pw"); expect(serialized).not.toContain("session=abc"); @@ -311,7 +315,7 @@ describe("emitManagedTransportFailure", () => { "operation=tools/list", "route=trusted_env_proxy", "target=mcp.example.com:443/stream", - "phase=response_headers", + "transport_phase=response_headers", "http_status=503", "elapsed_ms=1512", "cause_code=UND_ERR_SOCKET", @@ -372,7 +376,7 @@ describe("emitManagedTransportFailure", () => { expect(line).not.toContain("\n"); expect(line).not.toContain("\r"); // The genuine phase field is intact and appears once as a real top-level field. - expect(line).toContain("phase=response_headers"); + expect(line).toContain("transport_phase=response_headers"); // Delimiter-bearing values are JSON-quoted, so their `key=value` fragments // live inside a quoted token rather than as forged fields. expect(line).toContain('operation="tools/list\\nmanaged_transport_failure phase=policy"'); @@ -423,8 +427,53 @@ describe("example non-MCP consumer", () => { deliverWebhook("https://hooks.example.com/pay?signature=secret"); expect(lines[0]).toContain("consumer=webhook"); - expect(lines[0]).toContain("phase=app_connect"); + expect(lines[0]).toContain("transport_phase=app_connect"); expect(lines[0]).toContain("target=hooks.example.com:443/pay"); expect(lines.join("\n")).not.toContain("signature"); }); }); + +describe("shared vocabulary with the OpenClaw managed transport dist patch", () => { + const patchSource = readFileSync( + join(process.cwd(), "scripts/patch-openclaw-managed-transport-diagnostics.mts"), + "utf8", + ); + + it("emits the key names the patch documents for shared fields", () => { + const lines: string[] = []; + emitManagedTransportFailure( + buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "trusted_env_proxy", + phase: "response_headers", + elapsedMs: 100, + httpStatus: 503, + sessionPresent: true, + }), + (line) => lines.push(line), + ); + + for (const sharedKey of ["transport_phase", "session_present"]) { + expect(lines[0]).toContain(`${sharedKey}=`); + expect(patchSource).toContain(sharedKey); + } + // trace_id stays distinct from the patch's diagnostic_id, which is + // documented as a local identifier that does not correlate across + // process boundaries. + expect(lines[0]).toContain("trace_id="); + expect(lines[0]).not.toContain("diagnostic_id="); + }); + + it("covers every phase the patch classifier can return", () => { + const patchPhases = ["policy", "connect", "tls", "app_connect", "request", "response_headers"]; + const contractPhases: ManagedTransportPhase[] = [ + ...(patchPhases as ManagedTransportPhase[]), + "response_stream", + ]; + for (const phase of patchPhases) { + expect(patchSource).toContain(`return "${phase}"`); + } + expect(contractPhases).toHaveLength(7); + }); +}); diff --git a/src/lib/diagnostics/managed-transport.ts b/src/lib/diagnostics/managed-transport.ts index caa58fd3237..4ddf0edd710 100644 --- a/src/lib/diagnostics/managed-transport.ts +++ b/src/lib/diagnostics/managed-transport.ts @@ -19,10 +19,15 @@ import { sanitizeTraceAttributes } from "../trace"; /** The event name shared with OpenShell audit correlation. */ export const MANAGED_TRANSPORT_FAILURE_EVENT = "managed_transport_failure"; -/** The transport phase that failed, in connection order. */ +/** + * The transport phase that failed, in connection order. The first six values + * match the documented `transport_phase` vocabulary of the OpenClaw managed + * transport dist patch; `response_stream` extends it for consumers that + * classify failures after response headers arrive. + */ export type ManagedTransportPhase = | "policy" - | "proxy_connect" + | "connect" | "tls" | "app_connect" | "request" @@ -56,7 +61,7 @@ export interface ManagedTransportFailure { xRequestId?: string; xEnvoyResponseFlags?: string; /** Whether an opaque application session identifier was present; never its value. */ - sessionIdPresent?: boolean; + sessionPresent?: boolean; errorBodySnippet?: string; } @@ -198,7 +203,7 @@ export function classifyTransportPhase(input: { }): ManagedTransportPhase { if (input.policyDenied) return "policy"; if (input.tlsFailure) return "tls"; - if (input.proxyConnectFailure) return "proxy_connect"; + if (input.proxyConnectFailure) return "connect"; const code = input.causeCode ?? ""; if ( /^(?:UND_ERR_CONNECT_TIMEOUT|ECONNREFUSED|EHOSTUNREACH|ENETUNREACH|ENOTFOUND|EAI_AGAIN)$/.test( @@ -230,7 +235,7 @@ export interface ManagedTransportFailureInput { httpStatus?: number; error?: unknown; responseHeaders?: Iterable<[string, string]>; - sessionIdPresent?: boolean; + sessionPresent?: boolean; errorBody?: { body: string; contentType?: string }; } @@ -262,7 +267,7 @@ export function buildManagedTransportFailure( ...(causeCode === undefined ? {} : { causeCode }), causeChain, ...(input.responseHeaders === undefined ? {} : pickSafeResponseHeaders(input.responseHeaders)), - ...(input.sessionIdPresent === undefined ? {} : { sessionIdPresent: input.sessionIdPresent }), + ...(input.sessionPresent === undefined ? {} : { sessionPresent: input.sessionPresent }), ...(input.errorBody === undefined || !isErrorStatus(input.httpStatus) ? {} : (() => { @@ -311,12 +316,16 @@ export function emitManagedTransportFailure( const push = (key: string, value: string | number | boolean | undefined) => { if (value !== undefined) pairs.push(`${key}=${encodeLogField(value)}`); }; + // Key names shared with the OpenClaw managed transport dist patch use its + // documented vocabulary: transport_phase and session_present, as read by + // the MCP troubleshooting guide. trace_id stays distinct from the patch's + // diagnostic_id, which is documented as a local, non-correlating identifier. push("consumer", event.consumer); push("operation", event.operation); push("route", event.route); push("proxy", event.proxy); push("target", event.target); - push("phase", event.phase); + push("transport_phase", event.phase); push("http_status", event.httpStatus); push("elapsed_ms", event.elapsedMs); push("cause_code", event.causeCode); @@ -324,7 +333,7 @@ export function emitManagedTransportFailure( push("response_via", event.responseVia); push("x_request_id", event.xRequestId); push("x_envoy_response_flags", event.xEnvoyResponseFlags); - push("session_id_present", event.sessionIdPresent); + push("session_present", event.sessionPresent); push("trace_id", event.traceId); write(pairs.join(" ")); if (event.causeChain.length > 0) { From b9facda2b8b6e0db954f16e04eeae75a65f7836b Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 13 Aug 2026 02:06:34 -0700 Subject: [PATCH 08/12] fix(diagnostics): harden transport contract alignment Signed-off-by: Apurv Kumaria --- ...openclaw-managed-transport-diagnostics.mts | 27 ++++++++--- src/lib/diagnostics/managed-transport.test.ts | 45 ++++++++++++------- src/lib/diagnostics/managed-transport.ts | 19 +++++--- 3 files changed, 61 insertions(+), 30 deletions(-) diff --git a/scripts/patch-openclaw-managed-transport-diagnostics.mts b/scripts/patch-openclaw-managed-transport-diagnostics.mts index e0963bdbaab..b56e8bbe26c 100755 --- a/scripts/patch-openclaw-managed-transport-diagnostics.mts +++ b/scripts/patch-openclaw-managed-transport-diagnostics.mts @@ -10,6 +10,19 @@ const SCRIPT_PATH = fileURLToPath(import.meta.url); export const MARKER = "/* nemoclaw managed transport diagnostics (#7957) */"; +/** Transport phases emitted by the active OpenClaw managed-transport patch. */ +export const OPENCLAW_MANAGED_TRANSPORT_PHASES = [ + "policy", + "connect", + "tls", + "app_connect", + "request", + "response_headers", +] as const; + +const [POLICY_PHASE, CONNECT_PHASE, TLS_PHASE, APP_CONNECT_PHASE, REQUEST_PHASE, RESPONSE_PHASE] = + OPENCLAW_MANAGED_TRANSPORT_PHASES; + /** Client identity that only the compiled bundle-mcp session runtime carries. */ const TARGET_SIGNATURE = '"openclaw-bundle-mcp"'; @@ -121,12 +134,12 @@ export const INJECTED_DIAGNOSTIC_HELPER = [ "}", "function nemoClawMtdPhase(chain) {", '\tconst text = chain.map((cause) => (cause.code || "") + " " + (cause.message || "")).join(" ");', - '\tif (NEMOCLAW_MTD_POLICY_RE.test(text) || NEMOCLAW_MTD_CONNECT_DENIED_RE.test(text)) return "policy";', - '\tif (NEMOCLAW_MTD_CONNECT_RE.test(text)) return "connect";', - '\tif (chain.some((cause) => NEMOCLAW_MTD_TLS_CODES.includes(cause.code))) return "tls";', - '\tif (chain.some((cause) => NEMOCLAW_MTD_CONNECT_CODES.includes(cause.code))) return "app_connect";', - '\tif (chain.some((cause) => cause.code === "UND_ERR_HEADERS_TIMEOUT")) return "response_headers";', - '\treturn "request";', + `\tif (NEMOCLAW_MTD_POLICY_RE.test(text) || NEMOCLAW_MTD_CONNECT_DENIED_RE.test(text)) return ${JSON.stringify(POLICY_PHASE)};`, + `\tif (NEMOCLAW_MTD_CONNECT_RE.test(text)) return ${JSON.stringify(CONNECT_PHASE)};`, + `\tif (chain.some((cause) => NEMOCLAW_MTD_TLS_CODES.includes(cause.code))) return ${JSON.stringify(TLS_PHASE)};`, + `\tif (chain.some((cause) => NEMOCLAW_MTD_CONNECT_CODES.includes(cause.code))) return ${JSON.stringify(APP_CONNECT_PHASE)};`, + `\tif (chain.some((cause) => cause.code === "UND_ERR_HEADERS_TIMEOUT")) return ${JSON.stringify(RESPONSE_PHASE)};`, + `\treturn ${JSON.stringify(REQUEST_PHASE)};`, "}", "function nemoClawMtdHeaders(response) {", "\tconst safe = {};", @@ -305,7 +318,7 @@ export const INJECTED_DIAGNOSTIC_HELPER = [ "\t\t\t}", "\t\t\tvoid nemoClawMtdEmitResponseFailure(response, {", "\t\t\t\t...common,", - '\t\t\t\ttransport_phase: "response_headers",', + `\t\t\t\ttransport_phase: ${JSON.stringify(RESPONSE_PHASE)},`, "\t\t\t\thttp_status: response ? response.status : undefined,", "\t\t\t\telapsed_ms: elapsedMs,", "\t\t\t\tsession_present: sessionPresent", diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts index 3e7dcb71530..0ed6186d1aa 100644 --- a/src/lib/diagnostics/managed-transport.test.ts +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -1,13 +1,11 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { readFileSync } from "node:fs"; -import { join } from "node:path"; - import { describe, expect, it } from "vitest"; +import { OPENCLAW_MANAGED_TRANSPORT_PHASES } from "../../../scripts/patch-openclaw-managed-transport-diagnostics.mts"; + import { - type ManagedTransportPhase, boundedErrorBodySnippet, buildManagedTransportFailure, classifyTransportPhase, @@ -15,6 +13,7 @@ import { encodeLogField, generateTransportTraceId, MANAGED_TRANSPORT_FAILURE_EVENT, + type ManagedTransportPhase, pickSafeResponseHeaders, safeCauseChain, safeTargetRef, @@ -66,7 +65,14 @@ describe("safeCauseChain", () => { let deep: Record = { name: "leaf" }; for (let index = 0; index < 20; index += 1) deep = { name: `n${index}`, cause: deep }; - expect(safeCauseChain(deep).length).toBeLessThanOrEqual(8); + const chain = safeCauseChain(deep); + expect(chain).toHaveLength(8); + expect(chain[0]).toEqual({ name: "n19" }); + expect(chain[7]).toEqual({ name: "n12" }); + + let empty: Record = {}; + for (let index = 0; index < 20; index += 1) empty = { cause: empty }; + expect(safeCauseChain(empty)).toEqual([]); }); }); @@ -116,6 +122,9 @@ describe("classifyTransportPhase", () => { expect(classifyTransportPhase({ httpStatus: 503 })).toBe("response_headers"); expect(classifyTransportPhase({ causeCode: "UND_ERR_SOCKET" })).toBe("response_headers"); expect(classifyTransportPhase({ causeCode: "UND_ERR_BODY_TIMEOUT" })).toBe("response_stream"); + expect(classifyTransportPhase({ causeCode: "UND_ERR_BODY_TIMEOUT", httpStatus: 200 })).toBe( + "response_stream", + ); expect(classifyTransportPhase({ streamInterrupted: true })).toBe("response_stream"); expect(classifyTransportPhase({})).toBe("request"); }); @@ -377,6 +386,7 @@ describe("emitManagedTransportFailure", () => { expect(line).not.toContain("\r"); // The genuine phase field is intact and appears once as a real top-level field. expect(line).toContain("transport_phase=response_headers"); + expect(line.match(/(?:^| )transport_phase=/g)).toHaveLength(1); // Delimiter-bearing values are JSON-quoted, so their `key=value` fragments // live inside a quoted token rather than as forged fields. expect(line).toContain('operation="tools/list\\nmanaged_transport_failure phase=policy"'); @@ -434,11 +444,6 @@ describe("example non-MCP consumer", () => { }); describe("shared vocabulary with the OpenClaw managed transport dist patch", () => { - const patchSource = readFileSync( - join(process.cwd(), "scripts/patch-openclaw-managed-transport-diagnostics.mts"), - "utf8", - ); - it("emits the key names the patch documents for shared fields", () => { const lines: string[] = []; emitManagedTransportFailure( @@ -456,7 +461,6 @@ describe("shared vocabulary with the OpenClaw managed transport dist patch", () for (const sharedKey of ["transport_phase", "session_present"]) { expect(lines[0]).toContain(`${sharedKey}=`); - expect(patchSource).toContain(sharedKey); } // trace_id stays distinct from the patch's diagnostic_id, which is // documented as a local identifier that does not correlate across @@ -466,14 +470,23 @@ describe("shared vocabulary with the OpenClaw managed transport dist patch", () }); it("covers every phase the patch classifier can return", () => { - const patchPhases = ["policy", "connect", "tls", "app_connect", "request", "response_headers"]; const contractPhases: ManagedTransportPhase[] = [ - ...(patchPhases as ManagedTransportPhase[]), + ...OPENCLAW_MANAGED_TRANSPORT_PHASES, "response_stream", ]; - for (const phase of patchPhases) { - expect(patchSource).toContain(`return "${phase}"`); + for (const phase of contractPhases) { + const lines: string[] = []; + emitManagedTransportFailure( + buildManagedTransportFailure({ + consumer: "mcp", + operation: "tools/list", + route: "direct", + phase, + elapsedMs: 1, + }), + (line) => lines.push(line), + ); + expect(lines[0]).toContain(`transport_phase=${phase}`); } - expect(contractPhases).toHaveLength(7); }); }); diff --git a/src/lib/diagnostics/managed-transport.ts b/src/lib/diagnostics/managed-transport.ts index 4ddf0edd710..dbda646727a 100644 --- a/src/lib/diagnostics/managed-transport.ts +++ b/src/lib/diagnostics/managed-transport.ts @@ -16,7 +16,7 @@ import { sanitizeTraceAttributes } from "../trace"; * traffic emits nothing. */ -/** The event name shared with OpenShell audit correlation. */ +/** The event name shared with the OpenClaw managed-transport diagnostics. */ export const MANAGED_TRANSPORT_FAILURE_EVENT = "managed_transport_failure"; /** @@ -119,9 +119,15 @@ export function safeCauseChain(error: unknown): SafeTransportCause[] { const chain: SafeTransportCause[] = []; const seen = new Set(); let current = error; - while (current && typeof current === "object" && !seen.has(current)) { + let visited = 0; + while ( + current && + typeof current === "object" && + !seen.has(current) && + visited < MAX_CAUSE_DEPTH + ) { + visited += 1; seen.add(current); - if (chain.length >= MAX_CAUSE_DEPTH) break; const record = current as Record; const entry: SafeTransportCause = {}; if (typeof record.name === "string") entry.name = safeCauseToken(record.name); @@ -143,7 +149,7 @@ export function safeCauseChain(error: unknown): SafeTransportCause[] { */ export function redactField(value: string): string { const redacted = sanitizeTraceAttributes({ value }).value; - return typeof redacted === "string" ? redacted : value; + return typeof redacted === "string" ? redacted : ""; } /** Picks the diagnostic response headers the contract allows, redacting each kept value. */ @@ -214,11 +220,11 @@ export function classifyTransportPhase(input: { } if (/CERT|TLS|SSL/.test(code)) return "tls"; if (input.streamInterrupted) return "response_stream"; + if (/^(?:UND_ERR_BODY_TIMEOUT|UND_ERR_ABORTED)$/.test(code)) return "response_stream"; if (input.httpStatus !== undefined) return "response_headers"; if (/^(?:UND_ERR_SOCKET|ECONNRESET|EPIPE|UND_ERR_HEADERS_TIMEOUT)$/.test(code)) { return "response_headers"; } - if (/^(?:UND_ERR_BODY_TIMEOUT|UND_ERR_ABORTED)$/.test(code)) return "response_stream"; return "request"; } @@ -294,8 +300,7 @@ export function encodeLogField( maxLength: number = MAX_LOG_FIELD, ): string { if (typeof value !== "string") return String(value); - const redacted = sanitizeTraceAttributes({ value }).value; - const text = (typeof redacted === "string" ? redacted : value).slice(0, maxLength); + const text = redactField(value).slice(0, maxLength); if (/[\s="\\]|[\u0000-\u001f\u007f]/.test(text)) return JSON.stringify(text); return text; } From f75064936b23d9a5650291c3a2aeb10e0bf4e021 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:53:51 +0200 Subject: [PATCH 09/12] test(diagnostics): assert superseded key names stay absent Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- src/lib/diagnostics/managed-transport.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts index 0ed6186d1aa..83cdbb4d8d7 100644 --- a/src/lib/diagnostics/managed-transport.test.ts +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -462,6 +462,11 @@ describe("shared vocabulary with the OpenClaw managed transport dist patch", () for (const sharedKey of ["transport_phase", "session_present"]) { expect(lines[0]).toContain(`${sharedKey}=`); } + // The superseded key names must be absent, not merely accompanied by the + // new ones: an emitter writing both `phase=` and `transport_phase=` would + // reintroduce the second vocabulary this change removes. + expect(lines[0]).not.toMatch(/(^|\s)phase=/); + expect(lines[0]).not.toContain("session_id_present"); // trace_id stays distinct from the patch's diagnostic_id, which is // documented as a local identifier that does not correlate across // process boundaries. From 3b2dbc1a0a6111c72306cf3aa601d8d9e834c876 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Thu, 13 Aug 2026 06:30:56 -0700 Subject: [PATCH 10/12] ci(diagnostics): incorporate current main checks Signed-off-by: Carlos Villela --- ci/source-architecture-budget.json | 12 ++++++------ .../onboard/inference-selection-validation.test.ts | 3 +++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index 09950a7365b..b74bc96cdd7 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -8,14 +8,14 @@ "src/lib/adapters/docker/index.ts": 43, "src/lib/adapters/openshell/client.ts": 23, "src/lib/adapters/openshell/resolve.ts": 27, - "src/lib/adapters/openshell/runtime.ts": 52, + "src/lib/adapters/openshell/runtime.ts": 53, "src/lib/adapters/openshell/timeouts.ts": 37, "src/lib/agent/defs.ts": 32, "src/lib/cli/branding.ts": 86, "src/lib/cli/nemoclaw-oclif-command.ts": 106, "src/lib/cli/terminal-style.ts": 43, "src/lib/core/json-types.ts": 37, - "src/lib/core/ports.ts": 88, + "src/lib/core/ports.ts": 89, "src/lib/core/shell-quote.ts": 28, "src/lib/core/url-utils.ts": 27, "src/lib/core/wait.ts": 35, @@ -23,11 +23,11 @@ "src/lib/inference/config.ts": 29, "src/lib/inference/web-search.ts": 21, "src/lib/messaging/channels/index.ts": 25, - "src/lib/onboard/gateway-binding.ts": 49, + "src/lib/onboard/gateway-binding.ts": 50, "src/lib/runner.ts": 88, "src/lib/security/redact.ts": 52, "src/lib/state/onboard-session.ts": 36, - "src/lib/state/registry.ts": 99, + "src/lib/state/registry.ts": 100, "src/lib/state/state-root.ts": 20, "src/lib/subprocess-env.ts": 24, "src/lib/validation.ts": 25 @@ -46,7 +46,7 @@ "src/lib/actions/sandbox/rebuild-pipeline.ts": 28, "src/lib/actions/sandbox/snapshot.ts": 40, "src/lib/actions/uninstall/run-plan.ts": 26, - "src/lib/inference/onboard-probes.ts": 20, + "src/lib/inference/onboard-probes.ts": 21, "src/lib/inference/vllm.ts": 21, "src/lib/onboard.ts": 210, "src/lib/onboard/machine/handlers/sandbox.ts": 21, @@ -56,7 +56,7 @@ }, "allowedCycles": [], "maxRootFiles": { - "src/lib/onboard": 308, + "src/lib/onboard": 309, "src/lib/actions": 19, "src/lib/actions/sandbox": 182, "src/lib/state": 37, diff --git a/src/lib/onboard/inference-selection-validation.test.ts b/src/lib/onboard/inference-selection-validation.test.ts index df5127e1fbe..5f5fdb384fd 100644 --- a/src/lib/onboard/inference-selection-validation.test.ts +++ b/src/lib/onboard/inference-selection-validation.test.ts @@ -100,6 +100,7 @@ describe("inference selection validation", () => { ok: false, failures: [{ name: "Chat Completions API", httpStatus: 403 }], }), + teardownOrphanManagedGatewayOnAbort: vi.fn(), promptValidationRecovery, }); @@ -452,6 +453,7 @@ describe("inference selection validation", () => { agentProductName: () => "OpenClaw", getCredential: () => "test-key", probeAnthropicEndpoint, + teardownOrphanManagedGatewayOnAbort: vi.fn(), promptValidationRecovery: vi.fn(async () => "selection" as const), resolveEndpointHost: async () => [{ address: "169.254.169.254", family: 4 }], }); @@ -869,6 +871,7 @@ exit 0 agentProductName: () => "Deep Agents", getCredential: () => "test-key", probeOpenAiLikeEndpoint, + teardownOrphanManagedGatewayOnAbort: vi.fn(), promptValidationRecovery, resolveEndpointHost: async () => [{ address: "93.184.216.34", family: 4 }], }); From ab1eca2d7fef45d6025c5e466d06371374bb768c Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Thu, 13 Aug 2026 10:18:45 -0700 Subject: [PATCH 11/12] chore(ci): lower onboarding source budget --- ci/source-architecture-budget.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index c6d2f0527af..7394b419845 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -56,7 +56,7 @@ }, "allowedCycles": [], "maxRootFiles": { - "src/lib/onboard": 309, + "src/lib/onboard": 308, "src/lib/actions": 19, "src/lib/actions/sandbox": 183, "src/lib/state": 38, From debc957d9a5d96db73fb4bbed69ce3b741c6c83b Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Thu, 13 Aug 2026 10:26:43 -0700 Subject: [PATCH 12/12] fix(diagnostics): redact target userinfo --- src/lib/diagnostics/managed-transport.test.ts | 7 +++++++ src/lib/diagnostics/managed-transport.ts | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/lib/diagnostics/managed-transport.test.ts b/src/lib/diagnostics/managed-transport.test.ts index 83cdbb4d8d7..7540f509ddc 100644 --- a/src/lib/diagnostics/managed-transport.test.ts +++ b/src/lib/diagnostics/managed-transport.test.ts @@ -33,6 +33,13 @@ describe("safeTargetRef", () => { it("drops query and userinfo from non-URL values", () => { expect(safeTargetRef("user:secret@host:8080?token=x")).toBe("host:8080"); }); + + it("drops every userinfo segment before the final at sign", () => { + const target = "user:secret@second:credential@host:8080?token=x"; + + expect(safeTargetRef(target)).toBe("host:8080"); + expect(safeTargetRef(target)).not.toContain("credential"); + }); }); describe("safeCauseChain", () => { diff --git a/src/lib/diagnostics/managed-transport.ts b/src/lib/diagnostics/managed-transport.ts index dbda646727a..c34500b6b3b 100644 --- a/src/lib/diagnostics/managed-transport.ts +++ b/src/lib/diagnostics/managed-transport.ts @@ -93,7 +93,8 @@ export function safeTargetRef(value: string): string { } } const withoutQuery = value.split(/[?#]/, 1)[0] ?? ""; - return withoutQuery.replace(/^[^@]*@/, ""); + const finalAt = withoutQuery.lastIndexOf("@"); + return finalAt === -1 ? withoutQuery : withoutQuery.slice(finalAt + 1); } const MAX_CAUSE_DEPTH = 8;