feat(mcp): enforced header precedence and origin policy on remote transports
- Client-generated HTTP/MCP/authorization headers win over configured
headers case-insensitively (Agent Plugins §7.2.1) via the new
header-policy fetch wrapper used by the HTTP and legacy SSE transports.
- headerPolicy: "origin-locked" pins configured headers to the configured
URL's origin: never forwarded across cross-origin redirects, and
method-changing redirects of JSON-RPC POSTs are refused.
- envPolicy: "literal" exempts stdio env values (and origin-locked
headers) from config-value resolution: no ambient env-name lookup, no
__omp_shell("command execution, empty values preserved.")
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Remote MCP transports now enforce header precedence and origin policy: client-generated HTTP/MCP/authorization headers win over configured headers case-insensitively, and Agent Plugins servers never forward configured headers across a redirect to a different origin (method-changing redirects of JSON-RPC POSTs are refused). Agent Plugins stdio `env` values and remote `headers` are likewise exempt from config-value resolution (no ambient env-name lookup, no `!command` execution, empty values preserved).
|
||||
|
||||
## [17.2.10] - 2026-08-06
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
@@ -27,12 +27,23 @@ export interface MCPServer {
|
||||
args?: string[];
|
||||
/** Environment variables */
|
||||
env?: Record<string, string>;
|
||||
/**
|
||||
* `literal`: env values are opaque plugin package data (Agent Plugins
|
||||
* §§4.1/9.2) — exempt from env-name lookup and `!command` resolution.
|
||||
*/
|
||||
envPolicy?: "literal";
|
||||
/** Working directory for stdio transport */
|
||||
cwd?: string;
|
||||
/** URL (for HTTP/SSE transport) */
|
||||
url?: string;
|
||||
/** HTTP headers (for HTTP transport) */
|
||||
headers?: Record<string, string>;
|
||||
/**
|
||||
* `origin-locked`: configured headers are literal package data pinned to the
|
||||
* configured URL's origin (Agent Plugins §7.2.1) — never expanded, never
|
||||
* forwarded cross-origin, and client-generated headers win case-insensitively.
|
||||
*/
|
||||
headerPolicy?: "origin-locked";
|
||||
/** Authentication configuration */
|
||||
auth?: {
|
||||
type: "oauth" | "apikey";
|
||||
|
||||
@@ -54,6 +54,7 @@ function convertToLegacyConfig(server: MCPServer): MCPServerConfig {
|
||||
};
|
||||
if (server.args) config.args = server.args;
|
||||
if (server.env) config.env = server.env;
|
||||
if (server.envPolicy) config.envPolicy = server.envPolicy;
|
||||
if (server.cwd) config.cwd = server.cwd;
|
||||
return config;
|
||||
}
|
||||
@@ -65,6 +66,7 @@ function convertToLegacyConfig(server: MCPServer): MCPServerConfig {
|
||||
url: server.url ?? "",
|
||||
};
|
||||
if (server.headers) config.headers = server.headers;
|
||||
if (server.headerPolicy) config.headerPolicy = server.headerPolicy;
|
||||
return config;
|
||||
}
|
||||
|
||||
@@ -75,6 +77,7 @@ function convertToLegacyConfig(server: MCPServer): MCPServerConfig {
|
||||
url: server.url ?? "",
|
||||
};
|
||||
if (server.headers) config.headers = server.headers;
|
||||
if (server.headerPolicy) config.headerPolicy = server.headerPolicy;
|
||||
return config;
|
||||
}
|
||||
|
||||
|
||||
@@ -37,6 +37,7 @@ import type { McpConnectionStatusEvent } from "./startup-events";
|
||||
import type { MCPToolDetails } from "./tool-bridge";
|
||||
import { DeferredMCPTool, MCPTool } from "./tool-bridge";
|
||||
import type { MCPToolCache } from "./tool-cache";
|
||||
import { setGeneratedHeader } from "./transports/header-policy";
|
||||
import type {
|
||||
MCPAuthChallenge,
|
||||
MCPGetPromptResult,
|
||||
@@ -1458,13 +1459,11 @@ export class MCPManager {
|
||||
|
||||
if (credential) {
|
||||
if (resolved.type === "http" || resolved.type === "sse") {
|
||||
resolved = {
|
||||
...resolved,
|
||||
headers: {
|
||||
...resolved.headers,
|
||||
Authorization: `Bearer ${credential.access}`,
|
||||
},
|
||||
};
|
||||
// Client-generated authorization wins over any configured header
|
||||
// with the same case-insensitive name (Agent Plugins §7.2.1).
|
||||
const headers = { ...resolved.headers };
|
||||
setGeneratedHeader(headers, "Authorization", `Bearer ${credential.access}`);
|
||||
resolved = { ...resolved, headers };
|
||||
} else {
|
||||
resolved = {
|
||||
...resolved,
|
||||
@@ -1481,7 +1480,9 @@ export class MCPManager {
|
||||
}
|
||||
|
||||
if (resolved.type !== "http" && resolved.type !== "sse") {
|
||||
if (resolved.env) {
|
||||
// Literal env values (Agent Plugins §§4.1/9.2) are opaque package data:
|
||||
// no env-name lookup, no `!command` execution, no dropping empty values.
|
||||
if (resolved.env && resolved.envPolicy !== "literal") {
|
||||
const nextEnv: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(resolved.env)) {
|
||||
const resolvedValue = await resolveConfigValue(value);
|
||||
@@ -1490,7 +1491,9 @@ export class MCPManager {
|
||||
resolved = { ...resolved, env: nextEnv };
|
||||
}
|
||||
} else {
|
||||
if (resolved.headers) {
|
||||
// Origin-locked servers (Agent Plugins §9.2) carry literal header
|
||||
// values: no placeholder or environment-variable expansion.
|
||||
if (resolved.headers && resolved.headerPolicy !== "origin-locked") {
|
||||
const nextHeaders: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(resolved.headers)) {
|
||||
const resolvedValue = await resolveConfigValue(value);
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
/**
|
||||
* HTTP header precedence and redirect-origin policy for remote MCP transports.
|
||||
*
|
||||
* Two invariants, applied at every transport fetch:
|
||||
*
|
||||
* 1. Client-generated headers (protocol headers like `Content-Type`, `Accept`,
|
||||
* `Mcp-Session-Id`, and authorization) take precedence over configured
|
||||
* headers with the same case-insensitive name. Configured headers can never
|
||||
* corrupt the MCP wire protocol via casing tricks.
|
||||
* 2. Origin-locked servers (Agent Plugins §7.2.1) never forward configured
|
||||
* headers to a different origin: redirects are followed manually and
|
||||
* configured headers are attached only when the hop targets the configured
|
||||
* origin. Method-changing redirects of non-GET requests are refused.
|
||||
*/
|
||||
|
||||
/** Header buckets for one MCP HTTP request. */
|
||||
export interface MCPHeaderSources {
|
||||
/** Client-generated HTTP/MCP/authorization headers; win case-insensitively. */
|
||||
generated: Record<string, string>;
|
||||
/** Configured headers from the server entry (package or user config). */
|
||||
configured?: Record<string, string>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge configured headers under client-generated ones: a configured entry is
|
||||
* dropped when a generated header with the same case-insensitive name exists.
|
||||
*/
|
||||
export function mergeMCPHeaders({ generated, configured }: MCPHeaderSources): Record<string, string> {
|
||||
if (!configured) return { ...generated };
|
||||
const generatedNames = new Set<string>();
|
||||
for (const name in generated) generatedNames.add(name.toLowerCase());
|
||||
const merged: Record<string, string> = {};
|
||||
for (const name in configured) {
|
||||
if (!generatedNames.has(name.toLowerCase())) merged[name] = configured[name];
|
||||
}
|
||||
return { ...merged, ...generated };
|
||||
}
|
||||
|
||||
/**
|
||||
* Set a client-generated header, removing any existing entry with the same
|
||||
* case-insensitive name so the generated value is the only one sent.
|
||||
*/
|
||||
export function setGeneratedHeader(headers: Record<string, string>, name: string, value: string): void {
|
||||
const lower = name.toLowerCase();
|
||||
for (const existing in headers) {
|
||||
if (existing.toLowerCase() === lower) delete headers[existing];
|
||||
}
|
||||
headers[name] = value;
|
||||
}
|
||||
|
||||
const REDIRECT_STATUSES: Record<number, true> = { 301: true, 302: true, 303: true, 307: true, 308: true };
|
||||
const MAX_REDIRECT_HOPS = 5;
|
||||
|
||||
export interface MCPFetchInit {
|
||||
method: "GET" | "POST" | "DELETE";
|
||||
body?: string;
|
||||
signal?: AbortSignal;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch an MCP endpoint with header precedence and, for origin-locked servers,
|
||||
* manual redirect handling that strips configured headers on cross-origin hops.
|
||||
*
|
||||
* Non-locked servers keep the platform default redirect behavior. Locked
|
||||
* non-GET requests only follow 307/308 (method-preserving); a 301/302/303
|
||||
* redirect of a JSON-RPC POST is a connection error, never a silent GET.
|
||||
*/
|
||||
export async function mcpFetch(
|
||||
url: string,
|
||||
init: MCPFetchInit,
|
||||
sources: MCPHeaderSources,
|
||||
originLocked: boolean,
|
||||
): Promise<Response> {
|
||||
if (!originLocked) {
|
||||
return fetch(url, { ...init, headers: mergeMCPHeaders(sources) });
|
||||
}
|
||||
|
||||
const configuredOrigin = new URL(url).origin;
|
||||
let currentUrl = url;
|
||||
for (let hop = 0; hop <= MAX_REDIRECT_HOPS; hop++) {
|
||||
const attachConfigured = new URL(currentUrl).origin === configuredOrigin;
|
||||
const headers = mergeMCPHeaders(attachConfigured ? sources : { generated: sources.generated });
|
||||
const response = await fetch(currentUrl, { ...init, headers, redirect: "manual" });
|
||||
if (!REDIRECT_STATUSES[response.status]) return response;
|
||||
|
||||
const location = response.headers.get("Location");
|
||||
if (!location) return response;
|
||||
await response.body?.cancel();
|
||||
if (init.method !== "GET" && response.status !== 307 && response.status !== 308) {
|
||||
throw new Error(`HTTP ${response.status}: server redirected a ${init.method} request; refusing to follow`);
|
||||
}
|
||||
currentUrl = new URL(location, currentUrl).href;
|
||||
}
|
||||
throw new Error(`Too many redirects (> ${MAX_REDIRECT_HOPS}) fetching ${url}`);
|
||||
}
|
||||
@@ -19,6 +19,7 @@ import type {
|
||||
import { toJsonRpcError } from "../../mcp/types";
|
||||
import { RequestIdAllocator } from "../request-id";
|
||||
import { createMCPTimeout, getNeverAbortSignal, isMCPTimeoutEnabled, resolveMCPTimeoutMs } from "../timeout";
|
||||
import { type MCPFetchInit, mcpFetch } from "./header-policy";
|
||||
|
||||
const HTTP_SSE_CONNECT_TIMEOUT_MS = 1_000;
|
||||
/**
|
||||
@@ -54,6 +55,16 @@ export class HttpTransport implements MCPTransport {
|
||||
|
||||
constructor(private config: MCPHttpServerConfig | MCPSseServerConfig) {}
|
||||
|
||||
/** Fetch the configured endpoint with header precedence and origin policy. */
|
||||
#fetch(init: MCPFetchInit, generated: Record<string, string>): Promise<Response> {
|
||||
return mcpFetch(
|
||||
this.config.url,
|
||||
init,
|
||||
{ generated, configured: this.config.headers },
|
||||
this.config.headerPolicy === "origin-locked",
|
||||
);
|
||||
}
|
||||
|
||||
get connected(): boolean {
|
||||
return this.#connected;
|
||||
}
|
||||
@@ -81,13 +92,12 @@ export class HttpTransport implements MCPTransport {
|
||||
if (this.#sseConnection) return;
|
||||
|
||||
this.#sseConnection = new AbortController();
|
||||
const headers: Record<string, string> = {
|
||||
const generated: Record<string, string> = {
|
||||
Accept: "text/event-stream",
|
||||
...this.config.headers,
|
||||
};
|
||||
|
||||
if (this.#sessionId) {
|
||||
headers["Mcp-Session-Id"] = this.#sessionId;
|
||||
generated["Mcp-Session-Id"] = this.#sessionId;
|
||||
}
|
||||
|
||||
let response: Response | null;
|
||||
@@ -95,11 +105,7 @@ export class HttpTransport implements MCPTransport {
|
||||
let startupFinished = false;
|
||||
const connection = this.#sseConnection;
|
||||
const startupTimeoutMs = resolveSSEConnectTimeoutMs(this.config.timeout);
|
||||
const fetchPromise = fetch(this.config.url, {
|
||||
method: "GET",
|
||||
headers,
|
||||
signal: connection.signal,
|
||||
});
|
||||
const fetchPromise = this.#fetch({ method: "GET", signal: connection.signal }, generated);
|
||||
const timeoutPromise =
|
||||
startupTimeoutMs > 0
|
||||
? new Promise<null>(resolve => {
|
||||
@@ -219,26 +225,23 @@ export class HttpTransport implements MCPTransport {
|
||||
params: params ?? {},
|
||||
};
|
||||
|
||||
const headers: Record<string, string> = {
|
||||
const generated: Record<string, string> = {
|
||||
"Content-Type": "application/json",
|
||||
Accept: "application/json, text/event-stream",
|
||||
...this.config.headers,
|
||||
};
|
||||
|
||||
if (this.#sessionId) {
|
||||
headers["Mcp-Session-Id"] = this.#sessionId;
|
||||
generated["Mcp-Session-Id"] = this.#sessionId;
|
||||
}
|
||||
|
||||
const timeout = resolveMCPTimeoutMs(this.config.timeout);
|
||||
const operation = createMCPTimeout(timeout, options?.signal);
|
||||
|
||||
try {
|
||||
const response = await fetch(this.config.url, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body: JSON.stringify(body),
|
||||
signal: operation.signal,
|
||||
});
|
||||
const response = await this.#fetch(
|
||||
{ method: "POST", body: JSON.stringify(body), signal: operation.signal },
|
||||
generated,
|
||||
);
|
||||
|
||||
// Check for session ID in response
|
||||
const newSessionId = response.headers.get("Mcp-Session-Id");
|
||||
@@ -365,23 +368,18 @@ export class HttpTransport implements MCPTransport {
|
||||
const body = error
|
||||
? { jsonrpc: "2.0" as const, id, error }
|
||||
: { jsonrpc: "2.0" as const, id, result: result ?? {} };
|
||||
const headers: Record<string, string> = {
|
||||
const generated: Record<string, string> = {
|
||||
"Content-Type": "application/json",
|
||||
Accept: "application/json, text/event-stream",
|
||||
...this.config.headers,
|
||||
};
|
||||
if (this.#sessionId) {
|
||||
headers["Mcp-Session-Id"] = this.#sessionId;
|
||||
generated["Mcp-Session-Id"] = this.#sessionId;
|
||||
}
|
||||
const payload = JSON.stringify(body);
|
||||
const timeout = resolveMCPTimeoutMs(this.config.timeout);
|
||||
const operation = createMCPTimeout(timeout);
|
||||
try {
|
||||
const resp = await fetch(this.config.url, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body: JSON.stringify(body),
|
||||
signal: operation.signal,
|
||||
});
|
||||
const resp = await this.#fetch({ method: "POST", body: payload, signal: operation.signal }, generated);
|
||||
// Retry once on auth failure if onAuthError is wired
|
||||
if (this.onAuthError && (resp.status === 401 || resp.status === 403)) {
|
||||
await resp.body?.cancel();
|
||||
@@ -389,16 +387,13 @@ export class HttpTransport implements MCPTransport {
|
||||
if (newHeaders) {
|
||||
this.config.headers ??= {};
|
||||
Object.assign(this.config.headers, newHeaders);
|
||||
Object.assign(headers, newHeaders);
|
||||
operation.clear();
|
||||
const retryOperation = createMCPTimeout(timeout);
|
||||
try {
|
||||
const retry = await fetch(this.config.url, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body: JSON.stringify(body),
|
||||
signal: retryOperation.signal,
|
||||
});
|
||||
const retry = await this.#fetch(
|
||||
{ method: "POST", body: payload, signal: retryOperation.signal },
|
||||
generated,
|
||||
);
|
||||
await retry.body?.cancel();
|
||||
} finally {
|
||||
retryOperation.clear();
|
||||
@@ -425,26 +420,23 @@ export class HttpTransport implements MCPTransport {
|
||||
params: params ?? {},
|
||||
};
|
||||
|
||||
const headers: Record<string, string> = {
|
||||
const generated: Record<string, string> = {
|
||||
"Content-Type": "application/json",
|
||||
Accept: "application/json, text/event-stream",
|
||||
...this.config.headers,
|
||||
};
|
||||
|
||||
if (this.#sessionId) {
|
||||
headers["Mcp-Session-Id"] = this.#sessionId;
|
||||
generated["Mcp-Session-Id"] = this.#sessionId;
|
||||
}
|
||||
|
||||
const timeout = resolveMCPTimeoutMs(this.config.timeout);
|
||||
const operation = createMCPTimeout(timeout);
|
||||
|
||||
try {
|
||||
const response = await fetch(this.config.url, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body: JSON.stringify(body),
|
||||
signal: operation.signal,
|
||||
});
|
||||
const response = await this.#fetch(
|
||||
{ method: "POST", body: JSON.stringify(body), signal: operation.signal },
|
||||
generated,
|
||||
);
|
||||
|
||||
// 202 Accepted is success for notifications
|
||||
if (!response.ok && response.status !== 202) {
|
||||
@@ -492,16 +484,7 @@ export class HttpTransport implements MCPTransport {
|
||||
const timeout = resolveMCPTimeoutMs(this.config.timeout);
|
||||
const operation = createMCPTimeout(timeout);
|
||||
try {
|
||||
const headers: Record<string, string> = {
|
||||
...this.config.headers,
|
||||
"Mcp-Session-Id": this.#sessionId,
|
||||
};
|
||||
|
||||
await fetch(this.config.url, {
|
||||
method: "DELETE",
|
||||
headers,
|
||||
signal: operation.signal,
|
||||
});
|
||||
await this.#fetch({ method: "DELETE", signal: operation.signal }, { "Mcp-Session-Id": this.#sessionId });
|
||||
operation.clear();
|
||||
} catch {
|
||||
operation.clear();
|
||||
|
||||
@@ -12,6 +12,7 @@ import type {
|
||||
import { toJsonRpcError } from "../../mcp/types";
|
||||
import { RequestIdAllocator } from "../request-id";
|
||||
import { createMCPTimeout, getNeverAbortSignal, resolveMCPTimeoutMs } from "../timeout";
|
||||
import { type MCPFetchInit, mcpFetch } from "./header-policy";
|
||||
|
||||
interface MCPTimeoutOperation {
|
||||
signal?: AbortSignal;
|
||||
@@ -46,6 +47,16 @@ export class LegacySseTransport implements MCPTransport {
|
||||
this.#config = config;
|
||||
}
|
||||
|
||||
/** Fetch an endpoint with header precedence and origin policy. */
|
||||
#fetch(url: string, init: MCPFetchInit, generated: Record<string, string>): Promise<Response> {
|
||||
return mcpFetch(
|
||||
url,
|
||||
init,
|
||||
{ generated, configured: this.#config.headers },
|
||||
this.#config.headerPolicy === "origin-locked",
|
||||
);
|
||||
}
|
||||
|
||||
get connected(): boolean {
|
||||
return this.#connected;
|
||||
}
|
||||
@@ -65,14 +76,11 @@ export class LegacySseTransport implements MCPTransport {
|
||||
this.#sseConnection = connection;
|
||||
|
||||
try {
|
||||
const response = await fetch(this.#config.url, {
|
||||
method: "GET",
|
||||
headers: {
|
||||
Accept: "text/event-stream",
|
||||
...this.#config.headers,
|
||||
},
|
||||
signal: operation.signal,
|
||||
});
|
||||
const response = await this.#fetch(
|
||||
this.#config.url,
|
||||
{ method: "GET", signal: operation.signal },
|
||||
{ Accept: "text/event-stream" },
|
||||
);
|
||||
|
||||
if (!response.ok) {
|
||||
const text = await response.text();
|
||||
@@ -286,17 +294,12 @@ export class LegacySseTransport implements MCPTransport {
|
||||
): Promise<Response> {
|
||||
const endpointUrl = this.#endpointUrl;
|
||||
if (!endpointUrl) throw new Error("Transport not connected");
|
||||
let headers: Record<string, string> = {
|
||||
const generated: Record<string, string> = {
|
||||
"Content-Type": "application/json",
|
||||
Accept: "application/json, text/event-stream",
|
||||
...this.#config.headers,
|
||||
};
|
||||
let response = await fetch(endpointUrl, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body: JSON.stringify(body),
|
||||
signal,
|
||||
});
|
||||
const payload = JSON.stringify(body);
|
||||
let response = await this.#fetch(endpointUrl, { method: "POST", body: payload, signal }, generated);
|
||||
const status = AIError.status(response);
|
||||
if (!this.onAuthError || (status !== 401 && status !== 403)) return response;
|
||||
|
||||
@@ -304,17 +307,7 @@ export class LegacySseTransport implements MCPTransport {
|
||||
if (!refreshedHeaders) return response;
|
||||
await response.body?.cancel();
|
||||
this.#config.headers = refreshedHeaders;
|
||||
headers = {
|
||||
"Content-Type": "application/json",
|
||||
Accept: "application/json, text/event-stream",
|
||||
...this.#config.headers,
|
||||
};
|
||||
response = await fetch(endpointUrl, {
|
||||
method: "POST",
|
||||
headers,
|
||||
body: JSON.stringify(body),
|
||||
signal,
|
||||
});
|
||||
response = await this.#fetch(endpointUrl, { method: "POST", body: payload, signal }, generated);
|
||||
return response;
|
||||
}
|
||||
|
||||
|
||||
@@ -100,6 +100,13 @@ export interface MCPStdioServerConfig extends MCPServerConfigBase {
|
||||
command: string;
|
||||
args?: string[];
|
||||
env?: Record<string, string>;
|
||||
/**
|
||||
* `literal`: env values are opaque plugin package data (Agent Plugins
|
||||
* §§4.1/9.2) — no env-name lookup, no `!command` execution, no dropping of
|
||||
* empty values. The provider already applied the only permitted expansion
|
||||
* (`${PLUGIN_ROOT}`/`${PLUGIN_DATA}`).
|
||||
*/
|
||||
envPolicy?: "literal";
|
||||
cwd?: string;
|
||||
}
|
||||
|
||||
@@ -108,6 +115,12 @@ export interface MCPHttpServerConfig extends MCPServerConfigBase {
|
||||
type: "http";
|
||||
url: string;
|
||||
headers?: Record<string, string>;
|
||||
/**
|
||||
* `origin-locked`: configured headers are literal package data pinned to the
|
||||
* configured URL's origin (Agent Plugins §7.2.1) — never expanded, never
|
||||
* forwarded cross-origin, and client-generated headers win case-insensitively.
|
||||
*/
|
||||
headerPolicy?: "origin-locked";
|
||||
}
|
||||
|
||||
/** SSE server configuration (deprecated, use HTTP) */
|
||||
@@ -115,6 +128,8 @@ export interface MCPSseServerConfig extends MCPServerConfigBase {
|
||||
type: "sse";
|
||||
url: string;
|
||||
headers?: Record<string, string>;
|
||||
/** See {@link MCPHttpServerConfig.headerPolicy}. */
|
||||
headerPolicy?: "origin-locked";
|
||||
}
|
||||
|
||||
export type MCPServerConfig = MCPStdioServerConfig | MCPHttpServerConfig | MCPSseServerConfig;
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { MCPManager } from "@oh-my-pi/pi-coding-agent/mcp/manager";
|
||||
import type { MCPServerConfig, MCPStdioServerConfig } from "@oh-my-pi/pi-coding-agent/mcp/types";
|
||||
|
||||
function expectStdio(config: MCPServerConfig): MCPStdioServerConfig {
|
||||
if (config.type === "http" || config.type === "sse") throw new Error("expected a stdio config");
|
||||
return config;
|
||||
}
|
||||
|
||||
describe("stdio env value resolution policy", () => {
|
||||
it("keeps literal-policy env values byte-for-byte and never executes commands", async () => {
|
||||
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-env-policy-"));
|
||||
const sentinel = path.join(tempDir, "pwned");
|
||||
try {
|
||||
const manager = new MCPManager(process.cwd());
|
||||
const env = {
|
||||
NAME_OF_AMBIENT: "HOME",
|
||||
EMPTY: "",
|
||||
BANG: `!touch ${sentinel}`,
|
||||
};
|
||||
const resolved = expectStdio(
|
||||
await manager.prepareConfig({
|
||||
type: "stdio",
|
||||
command: "server",
|
||||
envPolicy: "literal",
|
||||
env: { ...env },
|
||||
}),
|
||||
);
|
||||
// Agent Plugins §§4.1/9.2: values are opaque package data — no ambient
|
||||
// env lookup, no empty-value dropping, no `!command` resolution.
|
||||
expect(resolved.env).toEqual(env);
|
||||
// prepareConfig awaits any (buggy) resolution, so a shell `touch` would
|
||||
// have completed by now — the sentinel must not exist.
|
||||
expect(fs.existsSync(sentinel)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps legacy env-name expansion for servers without the literal policy", async () => {
|
||||
const manager = new MCPManager(process.cwd());
|
||||
const resolved = expectStdio(
|
||||
await manager.prepareConfig({
|
||||
type: "stdio",
|
||||
command: "server",
|
||||
env: { LOOKUP: "HOME", EMPTY: "" },
|
||||
}),
|
||||
);
|
||||
// Non-plugin servers keep the existing contract: a value naming an
|
||||
// ambient variable expands, and empty values are dropped.
|
||||
expect(resolved.env?.LOOKUP).toBe(process.env.HOME ?? "HOME");
|
||||
expect(resolved.env?.EMPTY).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,146 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test";
|
||||
import { mergeMCPHeaders, setGeneratedHeader } from "@oh-my-pi/pi-coding-agent/mcp/transports/header-policy";
|
||||
import { HttpTransport } from "@oh-my-pi/pi-coding-agent/mcp/transports/http";
|
||||
|
||||
const REQUEST_TIMEOUT_MS = 1_000;
|
||||
|
||||
let servers: Bun.Server<undefined>[] = [];
|
||||
|
||||
function serve(
|
||||
fetchHandler: (req: Request, server: Bun.Server<undefined>) => Response | Promise<Response>,
|
||||
): Bun.Server<undefined> {
|
||||
const server = Bun.serve({ port: 0, fetch: fetchHandler });
|
||||
servers.push(server);
|
||||
return server;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const server of servers) server.stop(true);
|
||||
servers = [];
|
||||
});
|
||||
|
||||
async function rpcResult(req: Request): Promise<Response> {
|
||||
const body: unknown = await req.json();
|
||||
const id = body && typeof body === "object" && "id" in body ? body.id : null;
|
||||
return new Response(JSON.stringify({ jsonrpc: "2.0", id, result: { ok: true } }), {
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
describe("mergeMCPHeaders", () => {
|
||||
it("gives generated headers case-insensitive precedence over configured ones", () => {
|
||||
const merged = mergeMCPHeaders({
|
||||
generated: { "Content-Type": "application/json", Accept: "application/json, text/event-stream" },
|
||||
configured: { "content-type": "text/plain", accept: "text/html", "X-Tenant": "t" },
|
||||
});
|
||||
expect(merged).toEqual({
|
||||
"Content-Type": "application/json",
|
||||
Accept: "application/json, text/event-stream",
|
||||
"X-Tenant": "t",
|
||||
});
|
||||
});
|
||||
|
||||
it("setGeneratedHeader replaces case-variant entries", () => {
|
||||
const headers: Record<string, string> = { authorization: "Bearer configured", "X-A": "1" };
|
||||
setGeneratedHeader(headers, "Authorization", "Bearer generated");
|
||||
expect(headers).toEqual({ Authorization: "Bearer generated", "X-A": "1" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCP transport header policy", () => {
|
||||
it("keeps generated protocol headers when configured headers collide by casing", async () => {
|
||||
let received: Headers | null = null;
|
||||
const server = serve(async req => {
|
||||
received = req.headers;
|
||||
return rpcResult(req);
|
||||
});
|
||||
const transport = new HttpTransport({
|
||||
type: "http",
|
||||
url: `http://127.0.0.1:${server.port}/mcp`,
|
||||
timeout: REQUEST_TIMEOUT_MS,
|
||||
headers: { "content-type": "text/plain", "x-tenant": "t" },
|
||||
});
|
||||
await transport.connect();
|
||||
|
||||
await transport.request("tools/list");
|
||||
expect(received!.get("content-type")).toBe("application/json");
|
||||
expect(received!.get("x-tenant")).toBe("t");
|
||||
});
|
||||
|
||||
it("follows same-origin redirects with configured headers when origin-locked", async () => {
|
||||
let received: Headers | null = null;
|
||||
const server = serve(async (req, self) => {
|
||||
const url = new URL(req.url);
|
||||
if (url.pathname === "/mcp") {
|
||||
return new Response(null, {
|
||||
status: 307,
|
||||
headers: { Location: `http://127.0.0.1:${self.port}/mcp2` },
|
||||
});
|
||||
}
|
||||
received = req.headers;
|
||||
return rpcResult(req);
|
||||
});
|
||||
const transport = new HttpTransport({
|
||||
type: "http",
|
||||
url: `http://127.0.0.1:${server.port}/mcp`,
|
||||
timeout: REQUEST_TIMEOUT_MS,
|
||||
headers: { "X-Tenant": "t" },
|
||||
headerPolicy: "origin-locked",
|
||||
});
|
||||
await transport.connect();
|
||||
|
||||
const result = await transport.request<{ ok: boolean }>("tools/list");
|
||||
expect(result.ok).toBe(true);
|
||||
// Same origin: configured headers stay attached (§7.2.1).
|
||||
expect(received!.get("x-tenant")).toBe("t");
|
||||
});
|
||||
|
||||
it("strips configured headers on cross-origin redirects when origin-locked", async () => {
|
||||
let received: Headers | null = null;
|
||||
const target = serve(async req => {
|
||||
received = req.headers;
|
||||
return rpcResult(req);
|
||||
});
|
||||
const origin = serve(
|
||||
async () =>
|
||||
new Response(null, {
|
||||
status: 307,
|
||||
headers: { Location: `http://127.0.0.1:${target.port}/mcp` },
|
||||
}),
|
||||
);
|
||||
const transport = new HttpTransport({
|
||||
type: "http",
|
||||
url: `http://127.0.0.1:${origin.port}/mcp`,
|
||||
timeout: REQUEST_TIMEOUT_MS,
|
||||
headers: { "X-Tenant": "secret" },
|
||||
headerPolicy: "origin-locked",
|
||||
});
|
||||
await transport.connect();
|
||||
|
||||
const result = await transport.request<{ ok: boolean }>("tools/list");
|
||||
expect(result.ok).toBe(true);
|
||||
// Different origin: configured headers must not be forwarded (§7.2.1)…
|
||||
expect(received!.get("x-tenant")).toBeNull();
|
||||
// …while client-generated protocol headers still are.
|
||||
expect(received!.get("content-type")).toBe("application/json");
|
||||
});
|
||||
|
||||
it("refuses method-changing redirects of POST requests when origin-locked", async () => {
|
||||
const server = serve(
|
||||
async (_req, self) =>
|
||||
new Response(null, {
|
||||
status: 302,
|
||||
headers: { Location: `http://127.0.0.1:${self.port}/elsewhere` },
|
||||
}),
|
||||
);
|
||||
const transport = new HttpTransport({
|
||||
type: "http",
|
||||
url: `http://127.0.0.1:${server.port}/mcp`,
|
||||
timeout: REQUEST_TIMEOUT_MS,
|
||||
headerPolicy: "origin-locked",
|
||||
});
|
||||
await transport.connect();
|
||||
|
||||
await expect(transport.request("tools/list")).rejects.toThrow("refusing to follow");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user