Dev

[h3xxit]

Summary by cubic

Replaces the SSE and Streamable HTTP placeholder stubs with working transports, and changes CLI callToolStreaming from always throwing to yielding the completed result as one chunk. It also hardens discovery, authentication, reconnection, and error reporting, including protections for remote manuals and MCP token endpoints.

HTTP

• HTTP, SSE, and Streamable HTTP map path, header, body, and query arguments and support API-key, Basic, and OAuth2 authentication.
• SSE parses events incrementally, filters by event type, and reconnects dropped GET streams up to five times with Last-Event-ID; body-bearing calls are not retried.
• Streamable HTTP parses NDJSON and JSON, re-chunks binary responses, and applies one timeout across token fetch, request, and streaming; GET calls with body_field are rejected.
• Failed calls and discovery preserve status and bounded server response bodies on errors, while query strings stay out of logs.
• Manuals discovered from remote origins cannot point tools at loopback URLs; local discovery remains allowed.

MCP

• Tool results prefer structuredContent, retain the legacy structured_output fallback, and unwrap non-object FastMCP {result} wrappers.
• Recursive JSON Schema references no longer prevent tool discovery.
• Stdio child stderr is suppressed by default; set UTCP_MCP_CHILD_STDERR=inherit to show it.
• MCP validates OAuth2 token URLs before sending client credentials.

^{Written for commit 002ea10. Summary will update on new commits.}

[cubic-dev-ai]

2 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/http/src/sse_communication_protocol.ts">

<violation number="1" location="packages/http/src/sse_communication_protocol.ts:589">
P2: When a server sends a malformed `retry:` value such as `20ms` or `-1`, `parseInt` still changes the reconnect delay. Accept only digit-only, finite values so malformed server fields are ignored.</violation>
</file>

<file name="packages/mcp/tests/mcp_communication_protocol.test.ts">

<violation number="1" location="packages/mcp/tests/mcp_communication_protocol.test.ts:783">
P2: The assertion `expect(out.properties.root).toEqual({ $ref: "#/$defs/node" })` contradicts the actual `circular: 'ignore'` behavior of @apidevtools/json-schema-ref-parser (v15). Only the ref on the cycle itself (`next` inside node) is left as a $ref; the acyclic first ref `properties.root` → `#/$defs/node` is inlined into the node object. This also contradicts the sibling "acyclic refs are still inlined" test, which asserts the structurally identical first ref `properties.a` → `$defs.s` IS inlined. As written, `out.properties.root` deep-equals the node object, not the $ref, so the test fails. Expect the inlined node instead: root should be the object with `value` and a `next` $ref.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When a server sends a malformed retry: value such as 20ms or -1, parseInt still changes the reconnect delay. Accept only digit-only, finite values so malformed server fields are ignored.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/http/src/sse_communication_protocol.ts, line 589:

<comment>When a server sends a malformed `retry:` value such as `20ms` or `-1`, `parseInt` still changes the reconnect delay. Accept only digit-only, finite values so malformed server fields are ignored.</comment>

<file context>
@@ -201,22 +273,439 @@ export class SseCommunicationProtocol implements CommunicationProtocol {
+      } else if (field === 'id') {
+        event.id = value;
+      } else if (field === 'retry') {
+        const retry = parseInt(value, 10);
+        if (!Number.isNaN(retry)) {
+          event.retry = retry;
</file context>

Suggested change

	const retry = parseInt(value, 10);
	if (!Number.isNaN(retry)) {
	event.retry = retry;
	if (/^\d+$/.test(value)) {
	const retry = Number(value);
	if (Number.isFinite(retry)) {
	event.retry = retry;
	}
	}

[cubic-dev-ai]

P2: The assertion expect(out.properties.root).toEqual({ $ref: "#/$defs/node" }) contradicts the actual circular: 'ignore' behavior of @apidevtools/json-schema-ref-parser (v15). Only the ref on the cycle itself (next inside node) is left as a $ref; the acyclic first ref properties.root → #/$defs/node is inlined into the node object. This also contradicts the sibling "acyclic refs are still inlined" test, which asserts the structurally identical first ref properties.a → $defs.s IS inlined. As written, out.properties.root deep-equals the node object, not the $ref, so the test fails. Expect the inlined node instead: root should be the object with value and a next $ref.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp/tests/mcp_communication_protocol.test.ts, line 783:

<comment>The assertion `expect(out.properties.root).toEqual({ $ref: "#/$defs/node" })` contradicts the actual `circular: 'ignore'` behavior of @apidevtools/json-schema-ref-parser (v15). Only the ref on the cycle itself (`next` inside node) is left as a $ref; the acyclic first ref `properties.root` → `#/$defs/node` is inlined into the node object. This also contradicts the sibling "acyclic refs are still inlined" test, which asserts the structurally identical first ref `properties.a` → `$defs.s` IS inlined. As written, `out.properties.root` deep-equals the node object, not the $ref, so the test fails. Expect the inlined node instead: root should be the object with `value` and a `next` $ref.</comment>

<file context>
@@ -713,4 +713,88 @@ describe("McpCommunicationProtocol", () => {
+      // stringify throws; with `circular: 'ignore'` every $ref on the cycle
+      // stays a $ref string, which serializes fine.
+      expect(() => JSON.stringify(out)).not.toThrow();
+      expect(out.properties.root).toEqual({ $ref: "#/$defs/node" });
+      // The definitions the leftover $refs point at must survive so the
+      // schema remains resolvable by whoever consumes it.
</file context>

[h3xxit]

Disposition of the 11 findings, all handled in #45 (targets dev, so this PR picks them up once it merges):

• Empty id: / Last-Event-ID, event_type: 'message' default, streaming error body read in full, unbounded NDJSON line: fixed.
• retry: digits only: fixed, plus a test.
• Uppercase Content-Type in Streamable HTTP: fixed.
• Duplicated OAuth2 / auth-header code: extracted into _auth.ts, shared by SSE and Streamable HTTP; path-parameter substitution likewise shared with the HTTP protocol via _url.ts.
• Legacy structured_output: accepted again as a fallback after structuredContent, README wording updated.
• stderr hint wording: adopted.
• Wall-clock assertion in the stalled-token test: replaced by asserting the token endpoint saw exactly one request.
• Circular-schema test assertion: not changed. The test passes as written on this branch (bun test packages/mcp, 42 pass); with circular: 'ignore' json-schema-ref-parser 15 leaves every $ref on the cycle unresolved, including the first hop into it, which is what the assertion checks.

#45 also carries fixes from my own review: query-string secrets no longer logged, POST streams never re-issued on reconnect, bounded error-body reads everywhere, the original axios error kept reachable on the normalized error.

[cubic-dev-ai]

5 issues found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/mcp/tests/oauth_token_url_security.test.ts">

<violation number="1" location="packages/mcp/tests/oauth_token_url_security.test.ts:19">
P2: This assertion does not verify that credentials are never sent. Stub `_axiosInstance.post` and assert its call count remains zero, so a post-before-validation regression fails.</violation>

<violation number="2" location="packages/mcp/tests/oauth_token_url_security.test.ts:26">
P2: This test depends on the host’s port 1 state, making the suite environment-dependent and potentially slow or credential-posting. Use a per-test protocol with a mocked rejected `post` to exercise the loopback guard without real network I/O.</violation>
</file>

<file name="packages/http/src/http_communication_protocol.ts">

<violation number="1" location="packages/http/src/http_communication_protocol.ts:419">
P1: Remote manuals can still make OAuth2 tool calls send client credentials to a loopback `auth.token_url`. Extend the remote-origin restriction to OAuth2 token URLs, or preserve the remote trust context through token fetching.</violation>

<violation number="2" location="packages/http/src/http_communication_protocol.ts:421">
P1: A remote manual can bypass this guard with a dynamic host such as `http://{host}:8080/secret`. The guard sees the unresolved template before substitution, then the final loopback URL passes `ensureSecureUrl`, enabling SSRF. Reject placeholders in URL authorities or carry the remote-manual trust decision into the post-substitution invocation check.</violation>
</file>

<file name="packages/mcp/src/mcp_communication_protocol.ts">

<violation number="1" location="packages/mcp/src/mcp_communication_protocol.ts:771">
P1: When an allowed token endpoint redirects with 307/308, Axios can replay the OAuth request and its client secret to the redirect target because this validation covers only the initial URL. Disable redirects for token requests or validate every redirect before replaying credentials.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Remote manuals can still make OAuth2 tool calls send client credentials to a loopback auth.token_url. Extend the remote-origin restriction to OAuth2 token URLs, or preserve the remote trust context through token fetching.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/http/src/http_communication_protocol.ts, line 419:

<comment>Remote manuals can still make OAuth2 tool calls send client credentials to a loopback `auth.token_url`. Extend the remote-origin restriction to OAuth2 token URLs, or preserve the remote trust context through token fetching.</comment>

<file context>
@@ -402,6 +403,31 @@ export class HttpCommunicationProtocol implements CommunicationProtocol {
+   */
+  private _rejectRemoteLoopbackToolUrls(discoveryUrl: string, manual: { tools?: Array<any> }): void {
+    if (isLoopbackUrl(discoveryUrl)) return;
+    for (const tool of manual.tools || []) {
+      const toolUrl = tool?.tool_call_template?.url;
+      if (typeof toolUrl === 'string' && isLoopbackUrl(toolUrl)) {
</file context>

[cubic-dev-ai]

P1: A remote manual can bypass this guard with a dynamic host such as http://{host}:8080/secret. The guard sees the unresolved template before substitution, then the final loopback URL passes ensureSecureUrl, enabling SSRF. Reject placeholders in URL authorities or carry the remote-manual trust decision into the post-substitution invocation check.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/http/src/http_communication_protocol.ts, line 421:

<comment>A remote manual can bypass this guard with a dynamic host such as `http://{host}:8080/secret`. The guard sees the unresolved template before substitution, then the final loopback URL passes `ensureSecureUrl`, enabling SSRF. Reject placeholders in URL authorities or carry the remote-manual trust decision into the post-substitution invocation check.</comment>

<file context>
@@ -402,6 +403,31 @@ export class HttpCommunicationProtocol implements CommunicationProtocol {
+    if (isLoopbackUrl(discoveryUrl)) return;
+    for (const tool of manual.tools || []) {
+      const toolUrl = tool?.tool_call_template?.url;
+      if (typeof toolUrl === 'string' && isLoopbackUrl(toolUrl)) {
+        throw new Error(
+          `Security error during manual discovery: a manual fetched from ${JSON.stringify(discoveryUrl)} ` +
</file context>

[cubic-dev-ai]

P1: When an allowed token endpoint redirects with 307/308, Axios can replay the OAuth request and its client secret to the redirect target because this validation covers only the initial URL. Disable redirects for token requests or validate every redirect before replaying credentials.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp/src/mcp_communication_protocol.ts, line 771:

<comment>When an allowed token endpoint redirects with 307/308, Axios can replay the OAuth request and its client secret to the redirect target because this validation covers only the initial URL. Disable redirects for token requests or validate every redirect before replaying credentials.</comment>

<file context>
@@ -766,6 +766,9 @@ export class McpCommunicationProtocol implements CommunicationProtocol {
   private async _handleOAuth2(authDetails: OAuth2Auth): Promise<string> {
+    // Validate the token endpoint before sending credentials to it, so a
+    // manual cannot direct the operator's client secret at an arbitrary host.
+    ensureSecureMcpUrl(authDetails.token_url, 'MCP OAuth2 token URL');
     const clientId = authDetails.client_id;
     const cachedToken = this._oauthTokens.get(clientId);
</file context>

[cubic-dev-ai]

P2: This test depends on the host’s port 1 state, making the suite environment-dependent and potentially slow or credential-posting. Use a per-test protocol with a mocked rejected post to exercise the loopback guard without real network I/O.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp/tests/oauth_token_url_security.test.ts, line 26:

<comment>This test depends on the host’s port 1 state, making the suite environment-dependent and potentially slow or credential-posting. Use a per-test protocol with a mocked rejected `post` to exercise the loopback guard without real network I/O.</comment>

<file context>
@@ -0,0 +1,30 @@
+
+  test("a loopback token URL passes the guard (then fails on connection, not the guard)", async () => {
+    // Port 1 refuses immediately; the resulting error must not be the guard's.
+    await expect(protocol._handleOAuth2(auth("http://127.0.0.1:1/token"))).rejects.not.toThrow(
+      "Security error",
+    );
</file context>

[cubic-dev-ai]

P2: This assertion does not verify that credentials are never sent. Stub _axiosInstance.post and assert its call count remains zero, so a post-before-validation regression fails.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp/tests/oauth_token_url_security.test.ts, line 19:

<comment>This assertion does not verify that credentials are never sent. Stub `_axiosInstance.post` and assert its call count remains zero, so a post-before-validation regression fails.</comment>

<file context>
@@ -0,0 +1,30 @@
+  });
+
+  test("rejects a non-loopback plain-HTTP token URL before sending credentials", async () => {
+    await expect(protocol._handleOAuth2(auth("http://attacker.example/token"))).rejects.toThrow(
+      "Security error",
+    );
</file context>