Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions sdks/typescript/client/src/components/AppRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import {
AppBridge,
RESOURCE_MIME_TYPE,
type AppResult,
type McpUiMessageRequest,
type McpUiMessageResult,
type McpUiOpenLinkRequest,
Expand Down Expand Up @@ -163,7 +164,7 @@ export interface AppRendererProps {
* or standard MCP methods not yet in the Apps spec like "sampling/createMessage").
*
* This is wired to AppBridge's `fallbackRequestHandler` from the MCP SDK Protocol class.
* It receives the full JSON-RPC request and should return a result object or throw
* It receives the full JSON-RPC request and should return a result value or throw
* an McpError for unsupported methods.
*
* @example
Expand All @@ -186,7 +187,7 @@ export interface AppRendererProps {
onFallbackRequest?: (
request: JSONRPCRequest,
extra: RequestHandlerExtra,
) => Promise<Record<string, unknown>>;
) => Promise<unknown>;
}

/**
Expand Down Expand Up @@ -395,7 +396,7 @@ export const AppRenderer = forwardRef<AppRendererHandle, AppRendererProps>((prop
// not yet in the Apps spec like "sampling/createMessage")
bridge.fallbackRequestHandler = async (request, extra) => {
if (onFallbackRequestRef.current) {
return onFallbackRequestRef.current(request, extra);
return onFallbackRequestRef.current(request, extra) as Promise<AppResult>;
}
Comment on lines 396 to 400
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onFallbackRequest is typed as Promise<unknown>, but the result is cast to Promise<AppResult> when wiring bridge.fallbackRequestHandler. This cast bypasses type safety and makes it easy to return a value that AppBridge/transport can’t handle. Consider changing onFallbackRequest to return Promise<AppResult> (or narrowing/validating the returned value) and removing the cast.

Copilot uses AI. Check for mistakes.
throw new McpError(
ErrorCode.MethodNotFound,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ const mockClient = {
}),
};

// Helper to create mock RequestHandlerExtra
function createMockExtra(): import('../AppRenderer').RequestHandlerExtra {
return {
signal: new AbortController().signal,
requestId: 1,
sendNotification: vi.fn(),
sendRequest: vi.fn(),
} as import('../AppRenderer').RequestHandlerExtra;
}

describe('<AppRenderer />', () => {
const defaultProps: AppRendererProps = {
client: mockClient as unknown as Client,
Expand Down Expand Up @@ -503,7 +513,7 @@ describe('<AppRenderer />', () => {
method: 'x/clipboard/write',
params: { text: 'hello' },
};
const mockExtra = { signal: new AbortController().signal, requestId: 1, sendNotification: vi.fn(), sendRequest: vi.fn() };
const mockExtra = createMockExtra();

const result = await mockBridgeInstance?.fallbackRequestHandler?.(mockRequest, mockExtra as never);

Expand All @@ -524,7 +534,7 @@ describe('<AppRenderer />', () => {
method: 'x/unknown/method',
params: {},
};
const mockExtra = { signal: new AbortController().signal, requestId: 1, sendNotification: vi.fn(), sendRequest: vi.fn() };
const mockExtra = createMockExtra();

await expect(
mockBridgeInstance?.fallbackRequestHandler?.(mockRequest, mockExtra as never),
Expand Down Expand Up @@ -552,7 +562,7 @@ describe('<AppRenderer />', () => {
method: 'x/test/method',
params: {},
};
const mockExtra = { signal: new AbortController().signal, requestId: 1, sendNotification: vi.fn(), sendRequest: vi.fn() };
const mockExtra = createMockExtra();

const result = await mockBridgeInstance?.fallbackRequestHandler?.(mockRequest, mockExtra as never);

Expand All @@ -577,7 +587,7 @@ describe('<AppRenderer />', () => {
method: 'x/restricted/action',
params: {},
};
const mockExtra = { signal: new AbortController().signal, requestId: 1, sendNotification: vi.fn(), sendRequest: vi.fn() };
const mockExtra = createMockExtra();

await expect(
mockBridgeInstance?.fallbackRequestHandler?.(mockRequest, mockExtra as never),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ describe('UIResourceRendererWC', () => {
const el = document.createElement('ui-resource-renderer');

Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s a stray whitespace-only line here (line 144) after removing the eslint-disable comment. Please delete it to avoid trailing-whitespace noise and keep the test formatted consistently.

Suggested change

Copilot uses AI. Check for mistakes.
// Verify the element has the connectedMoveCallback method
// eslint-disable-next-line @typescript-eslint/no-explicit-any
expect('connectedMoveCallback' in el).toBe(true);

// The element should be an HTMLElement
Expand Down
59 changes: 57 additions & 2 deletions sdks/typescript/server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,10 @@ let _experimentalRequestId = 0;
* experimental methods (e.g., "x/clipboard/write"). Standard MCP methods not yet
* in the Apps spec (e.g., "sampling/createMessage") can use their canonical names.
* @param params - Request parameters
* @param options - Optional configuration for the request
* @param options.signal - Optional AbortSignal to cancel the request
* @param options.timeout - Optional timeout in milliseconds (default: 30000)
* @param options.targetOrigin - Optional target origin for postMessage (default: '*')
* @returns Promise that resolves with the host's JSON-RPC response result, or rejects
* with the JSON-RPC error
*
Expand All @@ -228,20 +232,71 @@ let _experimentalRequestId = 0;
export function sendExperimentalRequest(
method: string,
params?: Record<string, unknown>,
options?: {
signal?: AbortSignal;
timeout?: number;
targetOrigin?: string;
},
): Promise<unknown> {
const id = ++_experimentalRequestId;
const timeout = options?.timeout ?? 30000;
const targetOrigin = options?.targetOrigin ?? '*';

Comment on lines 220 to +244
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

targetOrigin currently defaults to '*' (and the JSDoc documents that default). For postMessage-based RPC this is an unsafe default because it allows sending requests to any origin. Prefer requiring an explicit targetOrigin (or deriving it from a trusted source like the embedding origin) and update the docs accordingly.

Copilot uses AI. Check for mistakes.
// Check if window.parent is available before setting up anything
if (!window.parent || window.parent === window) {
return Promise.reject(new Error('No parent window available'));
}

return new Promise((resolve, reject) => {
let isSettled = false;

const handler = (event: MessageEvent) => {
// Validate that the message comes from the parent window
if (event.source !== window.parent) {
return;
}

const data = event.data;
if (data?.jsonrpc === '2.0' && data?.id === id) {
window.removeEventListener('message', handler);
cleanup();
if (data.error) {
Comment on lines +257 to 262
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message handler validates event.source but does not validate event.origin. Without checking the expected origin, a compromised/untrusted embedding page (or an unexpected parent origin) could spoof responses for in-flight request IDs. Add an origin check tied to a configured/derived expected origin before accepting the response.

Copilot uses AI. Check for mistakes.
reject(data.error);
} else {
resolve(data.result);
}
}
};

const abortHandler = () => {
cleanup();
reject(new Error('Request aborted'));
};

const cleanup = () => {
if (!isSettled) {
isSettled = true;
window.removeEventListener('message', handler);
clearTimeout(timeoutId);
options?.signal?.removeEventListener('abort', abortHandler);
}
};

// Set up timeout
const timeoutId = setTimeout(() => {
cleanup();
reject(new Error(`Request timeout after ${timeout}ms`));
}, timeout);

// Set up abort signal
if (options?.signal) {
if (options.signal.aborted) {
cleanup();
reject(new Error('Request aborted'));
return;
}
options.signal.addEventListener('abort', abortHandler);
}

window.addEventListener('message', handler);

window.parent.postMessage(
Expand All @@ -251,7 +306,7 @@ export function sendExperimentalRequest(
method,
params: params ?? {},
},
'*',
targetOrigin,
);
});
}