The add-MCP form had no way to supply request headers, so an endpoint behind an edge authenticator could not be added at all. It now carries a name/value headers editor whose values ride along on the connection check and on every later request, through the existing config field. A 403 from such a gate also no longer reads as an unreachable server. It classifies exactly as a 401 does, so the flow continues to the auth step instead of stopping on "Couldn't reach this URL".
Unified · Split
| @@ -0,0 +1,15 @@ | ||
| 1 | +--- | |
| 2 | +"executor": patch | |
| 3 | +--- | |
| 4 | + | |
| 5 | +**Fix: add remote MCP servers that sit behind an authenticating proxy** | |
| 6 | + | |
| 7 | +The add-MCP form now carries an optional request headers editor. The name/value | |
| 8 | +pairs are sent on the connection check and on every later request, so an | |
| 9 | +endpoint gated by an edge authenticator — a Cloudflare Access service token, | |
| 10 | +for example — can be discovered and added. | |
| 11 | + | |
| 12 | +A `403` from such a gate is also no longer read as an unreachable server. It is | |
| 13 | +classified the same way a `401` is: the endpoint needs credentials, so the add | |
| 14 | +flow continues to the auth step instead of stopping on "Couldn't reach this | |
| 15 | +URL". | |
| @@ -0,0 +1,133 @@ | ||
| 1 | +// Regression guard for adding a remote MCP server that sits behind an edge | |
| 2 | +// authenticator. Cloudflare Access answers an unauthenticated request with a | |
| 3 | +// `403` HTML sign-in page, so the MCP server itself is never reached: no | |
| 4 | +// Bearer challenge, no RFC 9728 metadata, no JSON-RPC body. That used to read | |
| 5 | +// as "Couldn't reach this URL", which is wrong and offered no way forward. | |
| 6 | +// | |
| 7 | +// Now the 403 classifies as auth-required, and the add flow carries a request | |
| 8 | +// headers editor whose values ride along on the connection check. Both halves | |
| 9 | +// are asserted here: the flow reaches the auth editor, and the service-token | |
| 10 | +// headers actually reach the server. | |
| 11 | +// | |
| 12 | +// Selfhost-only because the probe must reach a loopback server: the selfhost | |
| 13 | +// instance runs with EXECUTOR_ALLOW_LOCAL_NETWORK. Video is the artifact. | |
| 14 | +import { randomBytes } from "node:crypto"; | |
| 15 | + | |
| 16 | +import { expect } from "@effect/vitest"; | |
| 17 | +import { Effect, Ref } from "effect"; | |
| 18 | +import { HttpServerResponse } from "effect/unstable/http"; | |
| 19 | +import { composePluginApi } from "@executor-js/api/server"; | |
| 20 | +import { deriveMcpNamespace } from "@executor-js/plugin-mcp"; | |
| 21 | +import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; | |
| 22 | +import { IntegrationSlug } from "@executor-js/sdk/shared"; | |
| 23 | +import { serveTestHttpApp } from "@executor-js/sdk/testing"; | |
| 24 | + | |
| 25 | +import { scenario } from "../src/scenario"; | |
| 26 | +import { Api, Browser, Target } from "../src/services"; | |
| 27 | +import { visit } from "../src/surfaces/browser"; | |
| 28 | + | |
| 29 | +const api = composePluginApi([mcpHttpPlugin()] as const); | |
| 30 | + | |
| 31 | +const CLIENT_ID_HEADER = "CF-Access-Client-Id"; | |
| 32 | +const CLIENT_SECRET_HEADER = "CF-Access-Client-Secret"; | |
| 33 | +const CLIENT_ID = "e2e-service-token-id"; | |
| 34 | +const CLIENT_SECRET = "e2e-service-token-secret"; | |
| 35 | + | |
| 36 | +scenario( | |
| 37 | + "MCP headers · a 403 edge gate is addable with service-token headers", | |
| 38 | + {}, | |
| 39 | + Effect.scoped( | |
| 40 | + Effect.gen(function* () { | |
| 41 | + const target = yield* Target; | |
| 42 | + const browser = yield* Browser; | |
| 43 | + const { client: makeApiClient } = yield* Api; | |
| 44 | + | |
| 45 | + // Cloudflare Access shape: every request is answered with a 403 HTML | |
| 46 | + // sign-in page. The headers each request carried are recorded so the | |
| 47 | + // scenario can prove the connection check sent the configured pair. | |
| 48 | + const seen = yield* Ref.make<readonly Readonly<Record<string, string>>[]>([]); | |
| 49 | + const server = yield* serveTestHttpApp((request) => | |
| 50 | + Effect.gen(function* () { | |
| 51 | + yield* Ref.update(seen, (all) => [...all, request.headers]); | |
| 52 | + if ((request.url ?? "").includes("/.well-known/")) { | |
| 53 | + return HttpServerResponse.text("missing", { status: 404 }); | |
| 54 | + } | |
| 55 | + return HttpServerResponse.text("<html><body>Sign in</body></html>", { | |
| 56 | + status: 403, | |
| 57 | + contentType: "text/html", | |
| 58 | + }); | |
| 59 | + }), | |
| 60 | + ); | |
| 61 | + | |
| 62 | + const endpoint = server.url("/mcp"); | |
| 63 | + // The gate reports no server name, so the probe cannot seed a unique | |
| 64 | + // identity. Selfhost identities share one tenant, so name the | |
| 65 | + // integration uniquely to keep the derived slug stable across runs. | |
| 66 | + const name = `edge-gated-403-${randomBytes(3).toString("hex")}`; | |
| 67 | + const slug = IntegrationSlug.make(deriveMcpNamespace({ name })); | |
| 68 | + const identity = yield* target.newIdentity(); | |
| 69 | + const client = yield* makeApiClient(api, identity); | |
| 70 | + | |
| 71 | + yield* Effect.gen(function* () { | |
| 72 | + yield* browser.session(identity, async ({ page, step }) => { | |
| 73 | + await step("Open the add-MCP flow pointed at the 403-gated server", async () => { | |
| 74 | + await visit(page, `/integrations/add/mcp?url=${encodeURIComponent(endpoint)}`); | |
| 75 | + // Before the fix the 403 read as unreachable and the flow stopped | |
| 76 | + // on "Couldn't reach this URL". Now it continues. | |
| 77 | + await page.getByText("How does this server authenticate?").waitFor(); | |
| 78 | + await page.getByText("Auth required").first().waitFor(); | |
| 79 | + }); | |
| 80 | + | |
| 81 | + await step("Configure the Cloudflare Access service-token headers", async () => { | |
| 82 | + await page.getByRole("button", { name: "Add header" }).click(); | |
| 83 | + await page.getByLabel("Header name").nth(0).fill(CLIENT_ID_HEADER); | |
| 84 | + await page.getByLabel("Header value").nth(0).fill(CLIENT_ID); | |
| 85 | + await page.getByRole("button", { name: "Add header" }).click(); | |
| 86 | + await page.getByLabel("Header name").nth(1).fill(CLIENT_SECRET_HEADER); | |
| 87 | + await page.getByLabel("Header value").nth(1).fill(CLIENT_SECRET); | |
| 88 | + }); | |
| 89 | + | |
| 90 | + await step("Test connection sends the headers to the server", async () => { | |
| 91 | + await page.getByRole("button", { name: "Test connection" }).click(); | |
| 92 | + // The button reports the in-flight probe with `data-loading`. | |
| 93 | + // Wait for it to clear so the re-probe has landed before we add. | |
| 94 | + await page.locator("button[data-loading]").waitFor({ state: "detached" }); | |
| 95 | + await page.getByText("Auth required").first().waitFor(); | |
| 96 | + }); | |
| 97 | + | |
| 98 | + await step("Add the integration", async () => { | |
| 99 | + await page.getByPlaceholder("e.g. Linear").fill(name); | |
| 100 | + await page.getByRole("button", { name: "Add integration" }).click(); | |
| 101 | + await page.waitForURL(/\/integrations\/(?!add\b)[^/?]+$/, { timeout: 30_000 }); | |
| 102 | + const landedSlug = new URL(page.url()).pathname.split("/").filter(Boolean).at(-1); | |
| 103 | + expect(landedSlug, "the add flow lands on the created integration").toBe(String(slug)); | |
| 104 | + }); | |
| 105 | + }); | |
| 106 | + | |
| 107 | + const requests = yield* Ref.get(seen); | |
| 108 | + const authenticated = requests.filter( | |
| 109 | + (headers) => headers[CLIENT_ID_HEADER.toLowerCase()] === CLIENT_ID, | |
| 110 | + ); | |
| 111 | + expect( | |
| 112 | + authenticated.length, | |
| 113 | + "the connection check reaches the server with the configured headers", | |
| 114 | + ).toBeGreaterThan(0); | |
| 115 | + expect( | |
| 116 | + authenticated.some( | |
| 117 | + (headers) => headers[CLIENT_SECRET_HEADER.toLowerCase()] === CLIENT_SECRET, | |
| 118 | + ), | |
| 119 | + "both halves of the service token are sent together", | |
| 120 | + ).toBe(true); | |
| 121 | + | |
| 122 | + const stored = yield* client.mcp.getServer({ params: { slug } }); | |
| 123 | + expect(stored?.config, "the headers persist on the integration").toMatchObject({ | |
| 124 | + transport: "remote", | |
| 125 | + headers: { | |
| 126 | + [CLIENT_ID_HEADER]: CLIENT_ID, | |
| 127 | + [CLIENT_SECRET_HEADER]: CLIENT_SECRET, | |
| 128 | + }, | |
| 129 | + }); | |
| 130 | + }).pipe(Effect.ensuring(client.mcp.removeServer({ params: { slug } }).pipe(Effect.ignore))); | |
| 131 | + }), | |
| 132 | + ), | |
| 133 | +); | |
| @@ -38,6 +38,8 @@ import { integrationWriteKeys } from "@executor-js/react/api/reactivity-keys"; · 37 unmodified lines | ||
| 38 | 38 | import type { McpAuthMethodInput } from "../sdk/types"; |
| 39 | 39 | import { probeMcpEndpoint, addMcpServer } from "./atoms"; |
| 40 | 40 | import { McpRemoteIntegrationFields } from "./McpRemoteIntegrationFields"; |
| 41 | +import { McpRequestHeadersEditor } from "./McpRequestHeadersEditor"; | |
| 42 | +import { mcpHeadersFromRows, type McpHeaderRow } from "./request-headers"; | |
| 41 | 43 | import { mcpAuthMethodInputFromEditorValue, mcpWireAuthInput } from "./auth-method-config"; |
| 42 | 44 | import { cloudflareNeedsCodemodeOptOut } from "../sdk/cloudflare-codemode"; |
| 43 | 45 | import { mcpPresets, type McpPreset } from "../sdk/presets"; |
| @@ -208,6 +210,12 @@ export default function AddMcpIntegration(props: { · 164 unmodified lines | ||
| 208 | 210 | remoteUrl ? { step: "url" as const, url: remoteUrl } : init, |
| 209 | 211 | ); |
| 210 | 212 | |
| 213 | + // Static request headers for the endpoint (e.g. a Cloudflare Access service | |
| 214 | + // token). They gate the probe as much as the live traffic, so the same | |
| 215 | + // values feed both. | |
| 216 | + const [headerRows, setHeaderRows] = useState<readonly McpHeaderRow[]>([]); | |
| 217 | + const headers = useMemo(() => mcpHeadersFromRows(headerRows), [headerRows]); | |
| 218 | + | |
| 211 | 219 | const doProbe = useAtomSet(probeMcpEndpoint, { mode: "promiseExit" }); |
| 212 | 220 | const doAddServer = useAtomSet(addMcpServer, { mode: "promiseExit" }); |
| 213 | 221 | |
| @@ -273,7 +281,7 @@ export default function AddMcpIntegration(props: { · 59 unmodified lines | ||
| 273 | 281 | const handleProbe = useCallback(async () => { |
| 274 | 282 | dispatch({ type: "probe-start" }); |
| 275 | 283 | const exit = await doProbe({ |
| 276 | − payload: { endpoint: state.url.trim() }, | |
| 284 | + payload: { endpoint: state.url.trim(), ...(headers ? { headers } : {}) }, | |
| 277 | 285 | }); |
| 278 | 286 | if (Exit.isFailure(exit)) { |
| 279 | 287 | dispatch({ |
| @@ -283,7 +291,7 @@ export default function AddMcpIntegration(props: { · 3 unmodified lines | ||
| 283 | 291 | return; |
| 284 | 292 | } |
| 285 | 293 | dispatch({ type: "probe-ok", probe: exit.value }); |
| 286 | − }, [state.url, doProbe]); | |
| 294 | + }, [state.url, headers, doProbe]); | |
| 287 | 295 | |
| 288 | 296 | // Keep the latest handleProbe in a ref so the debounced effect can call it |
| 289 | 297 | // without depending on its identity (which changes every render). |
| @@ -318,6 +326,7 @@ export default function AddMcpIntegration(props: { · 28 unmodified lines | ||
| 318 | 326 | : {}), |
| 319 | 327 | endpoint: state.url.trim(), |
| 320 | 328 | ...(slug ? { slug } : {}), |
| 329 | + ...(headers ? { headers } : {}), | |
| 321 | 330 | authenticationTemplate, |
| 322 | 331 | }, |
| 323 | 332 | reactivityKeys: integrationWriteKeys, |
| @@ -331,7 +340,7 @@ export default function AddMcpIntegration(props: { · 7 unmodified lines | ||
| 331 | 340 | } |
| 332 | 341 | return exit.value.slug; |
| 333 | 342 | }, |
| 334 | − [doAddServer, probe, remoteIdentity, resolvedDescription, state.url], | |
| 343 | + [doAddServer, headers, probe, remoteIdentity, resolvedDescription, state.url], | |
| 335 | 344 | ); |
| 336 | 345 | |
| 337 | 346 | const handleAddRemote = useCallback(async () => { |
| @@ -446,6 +455,16 @@ export default function AddMcpIntegration(props: { · 108 unmodified lines | ||
| 446 | 455 | </Info> |
| 447 | 456 | )} |
| 448 | 457 | |
| 458 | + {/* Static request headers. Shown in every remote state, because an | |
| 459 | + endpoint behind an edge authenticator (Cloudflare Access) fails | |
| 460 | + the very first probe until its service-token headers are set. */} | |
| 461 | + <McpRequestHeadersEditor | |
| 462 | + rows={headerRows} | |
| 463 | + onChange={setHeaderRows} | |
| 464 | + onTest={handleProbe} | |
| 465 | + testing={isProbing} | |
| 466 | + /> | |
| 467 | + | |
| 449 | 468 | {/* Authentication — declares the auth methods to register through the |
| 450 | 469 | shared list editor. The credentials themselves (API key value / |
| 451 | 470 | OAuth sign-in) are added from the integration's detail hub after |
| @@ -0,0 +1,118 @@ | ||
| 1 | +import { PlusIcon, XIcon } from "lucide-react"; | |
| 2 | + | |
| 3 | +import { Button } from "@executor-js/react/components/button"; | |
| 4 | +import { | |
| 5 | + CardStack, | |
| 6 | + CardStackContent, | |
| 7 | + CardStackEntryField, | |
| 8 | +} from "@executor-js/react/components/card-stack"; | |
| 9 | +import { FieldError } from "@executor-js/react/components/field"; | |
| 10 | +import { Input } from "@executor-js/react/components/input"; | |
| 11 | + | |
| 12 | +import { emptyHeaderRow, isValidHeaderName, type McpHeaderRow } from "./request-headers"; | |
| 13 | + | |
| 14 | +// --------------------------------------------------------------------------- | |
| 15 | +// Request headers editor — the name/value pairs sent on every request to a | |
| 16 | +// remote MCP server, including the connection check. | |
| 17 | +// | |
| 18 | +// Deliberately plain: two mono fields and a remove control per row, matching | |
| 19 | +// the metadata voice the rest of the add flow uses. Rows are keyed by index | |
| 20 | +// because the values are fully controlled, exactly as the shared placement | |
| 21 | +// editor does it. | |
| 22 | +// --------------------------------------------------------------------------- | |
| 23 | + | |
| 24 | +export function McpRequestHeadersEditor(props: { | |
| 25 | + readonly rows: readonly McpHeaderRow[]; | |
| 26 | + readonly onChange: (rows: McpHeaderRow[]) => void; | |
| 27 | + /** Re-run the connection check with the headers as typed. */ | |
| 28 | + readonly onTest?: () => void; | |
| 29 | + readonly testing?: boolean; | |
| 30 | +}) { | |
| 31 | + const { rows, onChange } = props; | |
| 32 | + | |
| 33 | + const set = (index: number, patch: Partial<McpHeaderRow>): void => | |
| 34 | + onChange(rows.map((row, j) => (j === index ? { ...row, ...patch } : row))); | |
| 35 | + | |
| 36 | + const remove = (index: number): void => onChange(rows.filter((_row, j) => j !== index)); | |
| 37 | + | |
| 38 | + const hasInvalidName = rows.some((row) => !isValidHeaderName(row.name)); | |
| 39 | + | |
| 40 | + return ( | |
| 41 | + <CardStack> | |
| 42 | + <CardStackContent className="border-t-0"> | |
| 43 | + <CardStackEntryField | |
| 44 | + label="Request headers" | |
| 45 | + description="- Optional. Sent on every request, including the connection check." | |
| 46 | + > | |
| 47 | + {rows.length > 0 && ( | |
| 48 | + <div className="flex flex-col gap-2"> | |
| 49 | + {rows.map((row, index) => ( | |
| 50 | + <div key={index} className="flex items-center gap-2"> | |
| 51 | + <Input | |
| 52 | + aria-label="Header name" | |
| 53 | + value={row.name} | |
| 54 | + onChange={(e) => set(index, { name: (e.target as HTMLInputElement).value })} | |
| 55 | + placeholder="CF-Access-Client-Id" | |
| 56 | + className="h-8 min-w-0 flex-1 font-mono text-xs" | |
| 57 | + aria-invalid={isValidHeaderName(row.name) ? undefined : true} | |
| 58 | + /> | |
| 59 | + <Input | |
| 60 | + aria-label="Header value" | |
| 61 | + value={row.value} | |
| 62 | + onChange={(e) => set(index, { value: (e.target as HTMLInputElement).value })} | |
| 63 | + placeholder="Value" | |
| 64 | + className="h-8 min-w-0 flex-1 font-mono text-xs" | |
| 65 | + /> | |
| 66 | + <Button | |
| 67 | + type="button" | |
| 68 | + variant="ghost" | |
| 69 | + size="icon-sm" | |
| 70 | + aria-label="Remove header" | |
| 71 | + className="shrink-0 text-muted-foreground hover:text-foreground" | |
| 72 | + onClick={() => remove(index)} | |
| 73 | + > | |
| 74 | + <XIcon /> | |
| 75 | + </Button> | |
| 76 | + </div> | |
| 77 | + ))} | |
| 78 | + </div> | |
| 79 | + )} | |
| 80 | + | |
| 81 | + {hasInvalidName && ( | |
| 82 | + <FieldError>A header name cannot contain spaces or a colon.</FieldError> | |
| 83 | + )} | |
| 84 | + | |
| 85 | + <div className="flex flex-wrap items-center gap-2"> | |
| 86 | + <Button | |
| 87 | + type="button" | |
| 88 | + variant="outline" | |
| 89 | + size="sm" | |
| 90 | + className="w-fit border-dashed" | |
| 91 | + onClick={() => onChange([...rows, emptyHeaderRow()])} | |
| 92 | + > | |
| 93 | + <PlusIcon /> | |
| 94 | + Add header | |
| 95 | + </Button> | |
| 96 | + {props.onTest && rows.length > 0 && ( | |
| 97 | + <Button | |
| 98 | + type="button" | |
| 99 | + variant="ghost" | |
| 100 | + size="sm" | |
| 101 | + onClick={props.onTest} | |
| 102 | + loading={props.testing} | |
| 103 | + > | |
| 104 | + Test connection | |
| 105 | + </Button> | |
| 106 | + )} | |
| 107 | + </div> | |
| 108 | + | |
| 109 | + <p className="text-[11px] text-muted-foreground"> | |
| 110 | + Stored with the integration and sent verbatim. Use these for endpoint-level access | |
| 111 | + tokens, such as a Cloudflare Access service token. A per-account credential belongs in | |
| 112 | + an auth method instead. | |
| 113 | + </p> | |
| 114 | + </CardStackEntryField> | |
| 115 | + </CardStackContent> | |
| 116 | + </CardStack> | |
| 117 | + ); | |
| 118 | +} | |
| @@ -0,0 +1,83 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// The row -> wire conversion behind the request-headers editor. What matters | |
| 3 | +// is that a half-typed row never reaches the transport and that a pasted | |
| 4 | +// token still works, so both are pinned here. | |
| 5 | +// --------------------------------------------------------------------------- | |
| 6 | + | |
| 7 | +import { describe, expect, it } from "@effect/vitest"; | |
| 8 | + | |
| 9 | +import { emptyHeaderRow, isValidHeaderName, mcpHeadersFromRows } from "./request-headers"; | |
| 10 | + | |
| 11 | +describe("mcpHeadersFromRows", () => { | |
| 12 | + it("returns undefined when nothing is configured", () => { | |
| 13 | + expect(mcpHeadersFromRows([])).toBeUndefined(); | |
| 14 | + expect(mcpHeadersFromRows([emptyHeaderRow()])).toBeUndefined(); | |
| 15 | + }); | |
| 16 | + | |
| 17 | + it("builds the wire map from completed rows", () => { | |
| 18 | + expect( | |
| 19 | + mcpHeadersFromRows([ | |
| 20 | + { name: "CF-Access-Client-Id", value: "client-id" }, | |
| 21 | + { name: "CF-Access-Client-Secret", value: "client-secret" }, | |
| 22 | + ]), | |
| 23 | + ).toEqual({ | |
| 24 | + "CF-Access-Client-Id": "client-id", | |
| 25 | + "CF-Access-Client-Secret": "client-secret", | |
| 26 | + }); | |
| 27 | + }); | |
| 28 | + | |
| 29 | + it("drops a row the user has not finished naming", () => { | |
| 30 | + expect( | |
| 31 | + mcpHeadersFromRows([ | |
| 32 | + { name: "X-Api-Key", value: "k" }, | |
| 33 | + { name: " ", value: "orphaned" }, | |
| 34 | + ]), | |
| 35 | + ).toEqual({ "X-Api-Key": "k" }); | |
| 36 | + }); | |
| 37 | + | |
| 38 | + it("trims surrounding whitespace off a pasted token", () => { | |
| 39 | + expect(mcpHeadersFromRows([{ name: " X-Token ", value: " abc123\n" }])).toEqual({ | |
| 40 | + "X-Token": "abc123", | |
| 41 | + }); | |
| 42 | + }); | |
| 43 | + | |
| 44 | + it("drops a name that cannot be put on the wire", () => { | |
| 45 | + expect( | |
| 46 | + mcpHeadersFromRows([ | |
| 47 | + { name: "Bad Header", value: "v" }, | |
| 48 | + { name: "Also:Bad", value: "v" }, | |
| 49 | + { name: "Good-Header", value: "v" }, | |
| 50 | + ]), | |
| 51 | + ).toEqual({ "Good-Header": "v" }); | |
| 52 | + }); | |
| 53 | + | |
| 54 | + it("keeps a header whose value is deliberately empty", () => { | |
| 55 | + expect(mcpHeadersFromRows([{ name: "X-Empty", value: "" }])).toEqual({ "X-Empty": "" }); | |
| 56 | + }); | |
| 57 | + | |
| 58 | + it("lets a later row win a duplicated name", () => { | |
| 59 | + expect( | |
| 60 | + mcpHeadersFromRows([ | |
| 61 | + { name: "X-Token", value: "old" }, | |
| 62 | + { name: "X-Token", value: "new" }, | |
| 63 | + ]), | |
| 64 | + ).toEqual({ "X-Token": "new" }); | |
| 65 | + }); | |
| 66 | +}); | |
| 67 | + | |
| 68 | +describe("isValidHeaderName", () => { | |
| 69 | + it("accepts an RFC 7230 token", () => { | |
| 70 | + expect(isValidHeaderName("CF-Access-Client-Id")).toBe(true); | |
| 71 | + expect(isValidHeaderName("X_Api_Key")).toBe(true); | |
| 72 | + }); | |
| 73 | + | |
| 74 | + it("treats a blank name as unfinished rather than invalid", () => { | |
| 75 | + expect(isValidHeaderName("")).toBe(true); | |
| 76 | + expect(isValidHeaderName(" ")).toBe(true); | |
| 77 | + }); | |
| 78 | + | |
| 79 | + it("rejects a name with a space or a colon", () => { | |
| 80 | + expect(isValidHeaderName("Bad Header")).toBe(false); | |
| 81 | + expect(isValidHeaderName("Also:Bad")).toBe(false); | |
| 82 | + }); | |
| 83 | +}); | |
| @@ -0,0 +1,58 @@ | ||
| 1 | +// --------------------------------------------------------------------------- | |
| 2 | +// Static request headers for a remote MCP server. | |
| 3 | +// | |
| 4 | +// Some MCP endpoints sit behind an edge authenticator rather than the MCP | |
| 5 | +// authorization spec — Cloudflare Access, for example, wants a | |
| 6 | +// `CF-Access-Client-Id` / `CF-Access-Client-Secret` pair on every request and | |
| 7 | +// answers 403 without them. Those are properties of the ENDPOINT, not of an | |
| 8 | +// account: every caller sends the same pair, and the server behind them still | |
| 9 | +// does its own MCP-level auth. They therefore belong in the integration's | |
| 10 | +// static `headers` config, which the probe and the live transport already | |
| 11 | +// read, and NOT in an auth method (an auth method carries one per-account | |
| 12 | +// credential value, so it cannot express a two-part token at all). | |
| 13 | +// | |
| 14 | +// The editor keeps rows rather than a map so a half-typed row survives a | |
| 15 | +// re-render. This module owns the row -> wire conversion. | |
| 16 | +// --------------------------------------------------------------------------- | |
| 17 | + | |
| 18 | +export type McpHeaderRow = { | |
| 19 | + readonly name: string; | |
| 20 | + readonly value: string; | |
| 21 | +}; | |
| 22 | + | |
| 23 | +export const emptyHeaderRow = (): McpHeaderRow => ({ name: "", value: "" }); | |
| 24 | + | |
| 25 | +/** RFC 7230 `token`: the only characters a header field name may contain. | |
| 26 | + * A name outside this set makes the whole request unsendable, so it is | |
| 27 | + * rejected in the editor instead of being discovered at connect time. */ | |
| 28 | +const HEADER_NAME_PATTERN = /^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/; | |
| 29 | + | |
| 30 | +/** Whether a row's name can be put on the wire. A blank name is not | |
| 31 | + * "invalid" — it is an unfinished row, which the editor leaves alone. */ | |
| 32 | +export const isValidHeaderName = (name: string): boolean => { | |
| 33 | + const trimmed = name.trim(); | |
| 34 | + return trimmed.length === 0 || HEADER_NAME_PATTERN.test(trimmed); | |
| 35 | +}; | |
| 36 | + | |
| 37 | +/** | |
| 38 | + * Collapse editor rows into the wire `headers` map. | |
| 39 | + * | |
| 40 | + * Blank and malformed names are dropped: the first is a row the user has not | |
| 41 | + * finished, the second cannot be sent at all. Names and values are trimmed — | |
| 42 | + * RFC 7230 treats surrounding whitespace as no part of a field value, so a | |
| 43 | + * token pasted with a trailing newline still works. | |
| 44 | + * | |
| 45 | + * Returns `undefined` when nothing is configured, so callers omit the field | |
| 46 | + * rather than sending an empty object. | |
| 47 | + */ | |
| 48 | +export const mcpHeadersFromRows = ( | |
| 49 | + rows: readonly McpHeaderRow[], | |
| 50 | +): Record<string, string> | undefined => { | |
| 51 | + const headers: Record<string, string> = {}; | |
| 52 | + for (const row of rows) { | |
| 53 | + const name = row.name.trim(); | |
| 54 | + if (!name || !HEADER_NAME_PATTERN.test(name)) continue; | |
| 55 | + headers[name] = row.value.trim(); | |
| 56 | + } | |
| 57 | + return Object.keys(headers).length > 0 ? headers : undefined; | |
| 58 | +}; | |
| @@ -267,6 +267,96 @@ describe("probeMcpEndpointShape", () => { · 266 unmodified lines | ||
| 267 | 267 | ), |
| 268 | 268 | ); |
| 269 | 269 | |
| 270 | + // Cloudflare Access shape: an edge authenticator in front of the MCP | |
| 271 | + // server answers an unauthenticated request with `403` and an HTML | |
| 272 | + // login page. The MCP server is never reached, so there is no Bearer | |
| 273 | + // challenge and no JSON-RPC body. This must read as "supply | |
| 274 | + // credentials", not as "this URL is not MCP" — the latter told users | |
| 275 | + // the endpoint was unreachable when it was merely protected. | |
| 276 | + it.effect("classifies a 403 HTML edge challenge as auth-required", () => | |
| 277 | + withServer( | |
| 278 | + () => | |
| 279 | + HttpServerResponse.text("<html><body>Sign in</body></html>", { | |
| 280 | + status: 403, | |
| 281 | + contentType: "text/html", | |
| 282 | + }), | |
| 283 | + (endpoint) => | |
| 284 | + Effect.gen(function* () { | |
| 285 | + const result = yield* probeMcpEndpointShape(endpoint); | |
| 286 | + expect(result).toMatchObject({ kind: "not-mcp", category: "auth-required" }); | |
| 287 | + }), | |
| 288 | + ), | |
| 289 | + ); | |
| 290 | + | |
| 291 | + it.effect("classifies 403 with Bearer + JSON-RPC error envelope as MCP+auth", () => | |
| 292 | + withServer( | |
| 293 | + () => | |
| 294 | + HttpServerResponse.jsonUnsafe( | |
| 295 | + { | |
| 296 | + jsonrpc: "2.0", | |
| 297 | + id: null, | |
| 298 | + error: { code: -32000, message: "Forbidden" }, | |
| 299 | + }, | |
| 300 | + { status: 403, headers: { "www-authenticate": "Bearer" } }, | |
| 301 | + ), | |
| 302 | + (endpoint) => | |
| 303 | + Effect.gen(function* () { | |
| 304 | + const result = yield* probeMcpEndpointShape(endpoint); | |
| 305 | + expect(result).toEqual({ kind: "mcp", requiresAuth: true }); | |
| 306 | + }), | |
| 307 | + ), | |
| 308 | + ); | |
| 309 | + | |
| 310 | + it.effect("rejects a 403 whose Bearer challenge carries a GraphQL body", () => | |
| 311 | + withServer( | |
| 312 | + () => | |
| 313 | + HttpServerResponse.jsonUnsafe( | |
| 314 | + { errors: [{ message: "Forbidden" }] }, | |
| 315 | + { status: 403, headers: { "www-authenticate": "Bearer" } }, | |
| 316 | + ), | |
| 317 | + (endpoint) => | |
| 318 | + Effect.gen(function* () { | |
| 319 | + const result = yield* probeMcpEndpointShape(endpoint); | |
| 320 | + expect(result).toMatchObject({ kind: "not-mcp", category: "auth-required" }); | |
| 321 | + }), | |
| 322 | + ), | |
| 323 | + ); | |
| 324 | + | |
| 325 | + // The other half of the Cloudflare Access story: once the service-token | |
| 326 | + // headers are configured, the same endpoint answers normally. Proves the | |
| 327 | + // probe actually puts `options.headers` on the wire. | |
| 328 | + it.effect("sends configured request headers and clears an edge challenge", () => | |
| 329 | + Effect.scoped( | |
| 330 | + Effect.gen(function* () { | |
| 331 | + const server = yield* serveProbeEndpoint((request) => { | |
| 332 | + if (request.headers["cf-access-client-id"] !== "client-id") { | |
| 333 | + return HttpServerResponse.text("<html>Sign in</html>", { | |
| 334 | + status: 403, | |
| 335 | + contentType: "text/html", | |
| 336 | + }); | |
| 337 | + } | |
| 338 | + return HttpServerResponse.jsonUnsafe({ | |
| 339 | + jsonrpc: "2.0", | |
| 340 | + id: 1, | |
| 341 | + result: { | |
| 342 | + protocolVersion: "2025-06-18", | |
| 343 | + capabilities: {}, | |
| 344 | + serverInfo: { name: "t", version: "0" }, | |
| 345 | + }, | |
| 346 | + }); | |
| 347 | + }); | |
| 348 | + | |
| 349 | + const blocked = yield* probeMcpEndpointShape(server.endpoint); | |
| 350 | + expect(blocked).toMatchObject({ kind: "not-mcp", category: "auth-required" }); | |
| 351 | + | |
| 352 | + const allowed = yield* probeMcpEndpointShape(server.endpoint, { | |
| 353 | + headers: { "CF-Access-Client-Id": "client-id" }, | |
| 354 | + }); | |
| 355 | + expect(allowed).toEqual({ kind: "mcp", requiresAuth: false }); | |
| 356 | + }), | |
| 357 | + ), | |
| 358 | + ); | |
| 359 | + | |
| 270 | 360 | it.effect("falls back to GET for OAuth-protected SSE endpoints", () => |
| 271 | 361 | withServer( |
| 272 | 362 | (request) => { |
| @@ -20,10 +20,12 @@ · 19 unmodified lines | ||
| 20 | 20 | // transport, body is an SSE stream we don't consume. |
| 21 | 21 | // - 2xx with `Content-Type: application/json` whose body parses as a |
| 22 | 22 | // JSON-RPC 2.0 envelope (`{jsonrpc:"2.0", result|error|method,...}`). |
| 23 | −// - 401 with `WWW-Authenticate: Bearer` AND a JSON-RPC error envelope | |
| 24 | −// in the body. The body shape is what separates a real MCP server | |
| 25 | −// from an unrelated OAuth-protected API: GraphQL/REST/HTML 401s | |
| 26 | −// don't shape themselves as JSON-RPC. | |
| 23 | +// - 401 (or 403) with `WWW-Authenticate: Bearer` AND a JSON-RPC error | |
| 24 | +// envelope in the body. The body shape is what separates a real MCP | |
| 25 | +// server from an unrelated OAuth-protected API: GraphQL/REST/HTML | |
| 26 | +// 401s don't shape themselves as JSON-RPC. 403 travels the same path | |
| 27 | +// because an edge authenticator (Cloudflare Access) answers with 403 | |
| 28 | +// and the endpoint is not therefore unreachable. | |
| 27 | 29 | // |
| 28 | 30 | // When POST returns 404/405/406/415 we retry with GET + `Accept: |
| 29 | 31 | // text/event-stream` to support legacy SSE-only servers; that path |
| @@ -82,6 +84,16 @@ const readHeader = (headers: Readonly<Record<string, string>>, name: string): st · 52 unmodified lines | ||
| 82 | 84 | return null; |
| 83 | 85 | }; |
| 84 | 86 | |
| 87 | +/** Statuses that mean "you are not authenticated for this resource". | |
| 88 | + * | |
| 89 | + * 401 is what the MCP authorization spec mandates. 403 is what an edge | |
| 90 | + * authenticator in front of the server returns instead — Cloudflare | |
| 91 | + * Access, for example, answers an unauthenticated request with 403 and | |
| 92 | + * never reaches the MCP server at all. Both say the same thing to the | |
| 93 | + * user: supply credentials and retry. Treating 403 as a wrong shape | |
| 94 | + * told them the URL was wrong, which it was not. */ | |
| 95 | +const isAuthStatus = (status: number): boolean => status === 401 || status === 403; | |
| 96 | + | |
| 85 | 97 | class ProbeTransportError extends Data.TaggedError("ProbeTransportError")<{ |
| 86 | 98 | readonly reason: string; |
| 87 | 99 | readonly cause: unknown; |
| @@ -213,9 +225,9 @@ const reasonFromBoundaryCause = (cause: unknown): string => { · 125 unmodified lines | ||
| 213 | 225 | |
| 214 | 226 | /** Why the probe rejected an endpoint as not-MCP. |
| 215 | 227 | * |
| 216 | − * - `auth-required` — server returned 401. We don't know for sure it's | |
| 217 | − * an MCP server (no spec-compliant Bearer challenge or the body | |
| 218 | − * isn't JSON-RPC), but the right next step for the user is the same | |
| 228 | + * - `auth-required` — server returned 401 or 403. We don't know for | |
| 229 | + * sure it's an MCP server (no spec-compliant Bearer challenge or the | |
| 230 | + * body isn't JSON-RPC), but the right next step for the user is the same | |
| 219 | 231 | * either way: provide credentials and retry. This is what |
| 220 | 232 | * misclassifies real MCP servers like cubic.dev (no |
| 221 | 233 | * resource_metadata) or ref.tools (no WWW-Authenticate at all) |
| @@ -227,7 +239,7 @@ export type McpProbeRejectCategory = "auth-required" | "wrong-shape"; · 5 unmodified lines | ||
| 227 | 239 | |
| 228 | 240 | export type McpShapeProbeResult = |
| 229 | 241 | /** Server answered initialize successfully — either a 2xx with a |
| 230 | − * JSON-RPC payload, or a 401 + WWW-Authenticate: Bearer (RFC 6750 | |
| 242 | + * JSON-RPC payload, or a 401/403 + WWW-Authenticate: Bearer (RFC 6750 | |
| 231 | 243 | * challenge) that the MCP auth spec requires. */ |
| 232 | 244 | | { readonly kind: "mcp"; readonly requiresAuth: boolean } |
| 233 | 245 | /** Endpoint is reachable but the response does not look like MCP. */ |
| @@ -253,7 +265,7 @@ export interface ProbeOptions { · 19 unmodified lines | ||
| 253 | 265 | * |
| 254 | 266 | * Returns `{kind: "mcp"}` only when the endpoint either: |
| 255 | 267 | * - answers with 2xx (unauth-OK MCP server), or |
| 256 | − * - responds 401 with a `Bearer` WWW-Authenticate challenge. | |
| 268 | + * - responds 401 or 403 with a `Bearer` WWW-Authenticate challenge. | |
| 257 | 269 | * |
| 258 | 270 | * Anything else (400, 404, 200-with-HTML, 200-with-GraphQL-errors, ...) |
| 259 | 271 | * is classified `not-mcp`. Transport errors surface as `unreachable`. |
| @@ -287,20 +299,20 @@ export const probeMcpEndpointShape = ( · 27 unmodified lines | ||
| 287 | 299 | const contentType = readHeader(response.headers, "content-type") ?? ""; |
| 288 | 300 | const isSse = /^\s*text\/event-stream\b/i.test(contentType); |
| 289 | 301 | |
| 290 | − if (response.status === 401) { | |
| 302 | + if (isAuthStatus(response.status)) { | |
| 291 | 303 | const wwwAuth = readHeader(response.headers, "www-authenticate"); |
| 292 | 304 | if (!wwwAuth || !/^\s*bearer\b/i.test(wwwAuth)) { |
| 293 | − // Spec-non-compliant 401 (no `Bearer` challenge). Before | |
| 294 | − // giving up, check whether the server still publishes | |
| 295 | − // RFC 9728 protected-resource metadata for this path — | |
| 296 | − // some real MCP servers (Datadog) do exactly this. | |
| 305 | + // Spec-non-compliant challenge (no `Bearer`). Before giving | |
| 306 | + // up, check whether the server still publishes RFC 9728 | |
| 307 | + // protected-resource metadata for this path — some real MCP | |
| 308 | + // servers (Datadog) do exactly this. | |
| 297 | 309 | if (yield* probeProtectedResourceMetadata(client, url, timeoutMs)) { |
| 298 | 310 | return { kind: "mcp", requiresAuth: true } as const; |
| 299 | 311 | } |
| 300 | 312 | return { |
| 301 | 313 | kind: "not-mcp", |
| 302 | 314 | category: "auth-required", |
| 303 | − reason: "401 without Bearer WWW-Authenticate — not an MCP auth challenge", | |
| 315 | + reason: `${response.status} without Bearer WWW-Authenticate — not an MCP auth challenge`, | |
| 304 | 316 | } as const; |
| 305 | 317 | } |
| 306 | 318 | // Spec-compliant MCP signal: the auth spec mandates a |
| @@ -347,8 +359,7 @@ export const probeMcpEndpointShape = ( · 40 unmodified lines | ||
| 347 | 359 | return { |
| 348 | 360 | kind: "not-mcp", |
| 349 | 361 | category: "auth-required", |
| 350 | − reason: | |
| 351 | − "401 + Bearer without resource_metadata, JSON-RPC body, or OAuth error body", | |
| 362 | + reason: `${response.status} + Bearer without resource_metadata, JSON-RPC body, or OAuth error body`, | |
| 352 | 363 | } as const; |
| 353 | 364 | } |
| 354 | 365 | return { kind: "mcp", requiresAuth: true } as const; |