Skip to content
Merged
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
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,9 @@ All tools respond in Markdown (not raw JSON). `get_issue` and `get_pull_request`
| `delete_issue` | write | Permanently delete an issue (destructive, no restoration window; requires admin; GraphQL) |
| `develop_issue` | write | Create a branch linked to an issue (GraphQL `createLinkedBranch`; optional name / base ref) |

Both `/mcp` (Streamable HTTP) and `/sse` endpoints are exposed; Claude.ai currently uses `/sse`.
Both `/mcp` (Streamable HTTP) and `/sse` endpoints are exposed; Claude.ai currently uses `/sse`; which of the two transports it lands on depends on whether it tries `POST /sse` first.

Streamable HTTP (`POST /mcp` and `POST /sse`) is stateless: each request builds its own MCP server, issues no `Mcp-Session-Id` and opens no Durable Object, so a connection writes no Durable Object rows (the free tier caps them at 100,000 a day). `GET /mcp` answers `405`. Only the legacy SSE handshake (`GET /sse` without an `mcp-protocol-version` header, plus `POST /sse/message`) still opens a `MyMCP` Durable Object per connection. A client that starts with `POST /sse` stays on the stateless path, including the `GET /sse` stream request it sends afterwards (answered `405`); a client that only speaks legacy SSE keeps paying one Durable Object per connection. A legacy SSE client that reconnects after a dropped stream sends the same headers as that streamable follow-up `GET`, so the reconnect gets `405` and the client has to start a new session.

Each tool call logs the GitHub rate-limit headers (`[github-ratelimit] remaining/limit, resets at …`) to `wrangler tail` so quota exhaustion is observable. Every successful write also emits a structured audit line (e.g. `[github-audit] {"tool":"commit_file","owner":"o","repo":"r","branch":"main","path":"x.ts"}`) to the Workers log, giving per-call accountability for LLM-mediated mutations. Both go to the Workers log only and never appear in the tool responses returned to the model.

Expand Down Expand Up @@ -299,10 +301,10 @@ Two prefixed axes: `type:*` mirror the repo's conventional-commit types (`feat`
## Security notes

- Tokens are encrypted at rest in the `OAUTH_KV` namespace using `COOKIE_ENCRYPTION_KEY`. Rotate the key (and re-deploy) to invalidate all active grants.
- The Worker is the OAuth _server_ for Claude.ai (and any other MCP client) and the OAuth _client_ for GitHub. The GitHub access token never leaves the Worker — it sits in `this.props.accessToken` inside the Durable Object instance, used by Octokit per request.
- The Worker is the OAuth _server_ for Claude.ai (and any other MCP client) and the OAuth _client_ for GitHub. The GitHub access token never leaves the Worker — it travels in the OAuth provider's request props (`ctx.props.accessToken`, or `this.props.accessToken` inside the legacy SSE Durable Object), used by Octokit per request.
- All tool calls go through a `wrapTool()` boundary that converts thrown errors into `{ isError: true, content: [{ type: "text", text: "Error: …" }] }` so the model sees the failure mode rather than the connection dropping. The error text is forwarded verbatim; Octokit already redacts the Authorization header, so tokens do not leak, though other fields are not sanitised (defence-in-depth, not done today).
- Write-tool payloads carry input-size caps (file content, commit/PR/issue/comment text, per-commit file count, the aggregate content size of a multi-file commit — see `src/tools/common.ts` — and attachment bytes, capped to what the transport carries) as defence-in-depth, so a runaway model can't burn Worker CPU/memory with a multi-megabyte payload. Oversized input is rejected with a descriptive error (per-field caps at schema validation; the aggregate-commit cap in the `commit_files` handler before any API call).
- Request size is bounded before tools run: a body over 4 MiB gets `413` on `/mcp` and `/sse/message` (counted while reading, so a chunked or understated `Content-Length` is caught too — `src/mcp/body-limit.ts`), a tool call carrying more than 1,000 array elements and object keys is rejected before schema validation, and every array or record input has its own element cap (declared in the tool schemas under `src/tools/`; the shared caps are in `src/tools/common.ts`).
- Request size is bounded before tools run: a body over 4 MiB gets `413` on `/mcp`, `POST /sse` and `/sse/message` (counted while reading, so a chunked or understated `Content-Length` is caught too — `src/mcp/body-limit.ts`), a tool call carrying more than 1,000 array elements and object keys is rejected before schema validation, and every array or record input has its own element cap (declared in the tool schemas under `src/tools/`; the shared caps are in `src/tools/common.ts`).
- This is still a small server. Audit before exposing to untrusted users; consider tightening CORS, limiting allowed origins, or restricting `ALLOWED_USERNAMES` for sensitive write tools.

## Self-Hosting Setup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ The decision was: keep the manual harness but **also** add a CI-runnable transpo

**Chosen option: 4 — Two-tier split via factory swap.**

The CI tier (`test/mcp-e2e.test.js`) drives `/register` → `/authorize` → `/token` → `/mcp initialize` + `tools/list` end-to-end inside the Workers pool. The OAuth `defaultHandler` is swapped for the fake, and the test worker's tools get an offline Octokit so tool handlers run without reaching GitHub; it also wraps `registerTool` to count handler calls and adds two `_test_*` tools that report them. All other production wiring (`MyMCP.serve`, the MCP server and its limits, OAuth-provider endpoints, CORS, KV bindings) runs unmodified.
The CI tier (`test/mcp-e2e.test.js`) drives `/register` → `/authorize` → `/token` → `/mcp initialize` + `tools/list` end-to-end inside the Workers pool. The OAuth `defaultHandler` is swapped for the fake, and the test worker's tools get an offline Octokit so tool handlers run without reaching GitHub; it also wraps `registerTool` to count handler calls and adds `_test_*` tools that report them or reset the counters (requests are stateless, so the counters live in the isolate). The test worker instruments the server before the production tools are registered through the `instrumentServer` hook. All other production wiring (the stateless `/mcp` and `POST /sse` handlers, the `MyMCP` legacy SSE class, the MCP server and its limits, OAuth-provider endpoints, CORS, KV bindings) runs unmodified.

The manual tier (`scripts/e2e/oauth-e2e.mjs`) covers the surfaces the CI tier deliberately skips — the production `GitHubHandler` (consent UI, CSRF cookie, state binding, upstream GitHub token exchange) and real tool execution against `api.github.com`. Positioned as a pre-release acceptance check, not a routine dev step.

Expand Down
54 changes: 40 additions & 14 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
import OAuthProvider from "@cloudflare/workers-oauth-provider";
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import { McpAgent } from "agents/mcp";
import { Octokit } from "octokit";
import type { Octokit } from "octokit";

import { GitHubHandler } from "./github-handler";
import { limitRequestBody } from "./mcp/body-limit";
import {
createGitHubOctokit,
newMcpServer,
type OctokitBuilder,
type ServerInstrument,
} from "./mcp/server";
import { statelessMcpHandler } from "./mcp/stateless";
import { registerTools } from "./tools";
import { MAX_TOOL_INPUT_ELEMENTS } from "./tools/common";
import type { Props } from "./utils";

// eslint-disable-next-line @typescript-eslint/no-deprecated -- migration tracked in #252
export class MyMCP extends McpAgent<Env, Record<string, never>, Props> {
server = new McpServer(
{ name: "remote-mcp-github", version: "0.1.0" },
{ maxToolInputElements: MAX_TOOL_INPUT_ELEMENTS },
);
server = newMcpServer();

async init(): Promise<void> {
registerTools(this.server, () => {
Expand All @@ -26,7 +28,7 @@ export class MyMCP extends McpAgent<Env, Record<string, never>, Props> {
}

protected createOctokit(accessToken: string): Octokit {
return new Octokit({ auth: accessToken });
return createGitHubOctokit(accessToken);
}
}

Expand All @@ -38,19 +40,43 @@ const corsOptions = {
maxAge: 86400,
};

// Factory so tests can swap in a fake auth handler while keeping the rest of
// the OAuth/MCP wiring identical to production.
export const buildOAuthProvider = (defaultHandler: ExportedHandler<Env>) =>
new OAuthProvider({
// A legacy SSE client sends no `mcp-protocol-version` on its opening GET (it
// learns the version after `initialize`); a streamable client's GET carries it.
const isStreamableHttpOnSse = (request: Request): boolean => {
if (new URL(request.url).pathname !== "/sse") return false;
return (
request.method === "POST" ||
request.method === "DELETE" ||
(request.method === "GET" && request.headers.has("mcp-protocol-version"))
);
};

type McpHooks = { instrumentServer?: ServerInstrument; createOctokit?: OctokitBuilder };

export const buildOAuthProvider = (
defaultHandler: ExportedHandler<Env>,
{ instrumentServer, createOctokit = createGitHubOctokit }: McpHooks = {},
) => {
const stateless = (route: string) =>
limitRequestBody(statelessMcpHandler({ route, corsOptions, instrumentServer, createOctokit }));
const streamableSse = stateless("/sse");
const legacySse = limitRequestBody(MyMCP.serveSSE("/sse", { corsOptions }));
return new OAuthProvider({
apiHandlers: {
"/mcp": limitRequestBody(MyMCP.serve("/mcp", { corsOptions })),
"/sse": limitRequestBody(MyMCP.serveSSE("/sse", { corsOptions })),
"/mcp": stateless("/mcp"),
"/sse": {
fetch: (request, env, ctx) =>
isStreamableHttpOnSse(request)
? streamableSse.fetch(request, env, ctx)
: legacySse.fetch(request, env, ctx),
},
},
authorizeEndpoint: "/authorize",
clientRegistrationEndpoint: "/register",
defaultHandler,
tokenEndpoint: "/token",
});
};

export default buildOAuthProvider({
fetch: (request, env, ctx) => GitHubHandler.fetch(request, env, ctx),
Expand Down
2 changes: 1 addition & 1 deletion src/mcp/body-limit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// it checks against Content-Length only, so a chunked or understated body slips past.
export const MAX_MCP_MESSAGE_BYTES = 4 * 1024 * 1024;

type FetchHandler = {
export type FetchHandler = {
fetch: (request: Request, env: Env, ctx: ExecutionContext) => Response | Promise<Response>;
};

Expand Down
27 changes: 27 additions & 0 deletions src/mcp/server.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import { Octokit } from "octokit";

import { registerTools } from "../tools";
import { MAX_TOOL_INPUT_ELEMENTS, type OctokitFactory } from "../tools/common";

export type OctokitBuilder = (accessToken: string) => Octokit;
export type ServerInstrument = (server: McpServer) => void;

export const newMcpServer = (): McpServer =>
new McpServer(
{ name: "remote-mcp-github", version: "0.1.0" },
{ maxToolInputElements: MAX_TOOL_INPUT_ELEMENTS },
);

export const createMcpServerWithTools = (
octokit: OctokitFactory,
instrument?: ServerInstrument,
): McpServer => {
const server = newMcpServer();
instrument?.(server);
registerTools(server, octokit);
return server;
};

export const createGitHubOctokit: OctokitBuilder = (accessToken) =>
new Octokit({ auth: accessToken });
45 changes: 45 additions & 0 deletions src/mcp/stateless.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { createLegacyMcpHandler, type WorkerTransportOptions } from "agents/mcp";

import type { Props } from "../utils";
import type { FetchHandler } from "./body-limit";
import { createMcpServerWithTools, type OctokitBuilder, type ServerInstrument } from "./server";

type StatelessOptions = {
route: string;
corsOptions: NonNullable<WorkerTransportOptions["corsOptions"]>;
instrumentServer: ServerInstrument | undefined;
createOctokit: OctokitBuilder;
};

const hasAccessToken = (props: unknown): props is Pick<Props, "accessToken"> =>
typeof props === "object" &&
props !== null &&
"accessToken" in props &&
typeof props.accessToken === "string";

// Fresh server per request, no session id, no Durable Object rows.
// GET and DELETE are 405: there is no session to stream to or end.
export const statelessMcpHandler = ({
route,
corsOptions,
instrumentServer,
createOctokit,
}: StatelessOptions): FetchHandler => ({
fetch: (request, env, ctx) => {
if (request.method === "GET" || request.method === "DELETE") {
return new Response("Method Not Allowed", {
status: 405,
headers: { Allow: "POST, OPTIONS" },
});
}
const props = ctx.props;
const server = createMcpServerWithTools(() => {
if (!hasAccessToken(props)) {
throw new Error("OAuth props are not available; request is not authenticated.");
}
return createOctokit(props.accessToken);
}, instrumentServer);
// The SDK v1 server only fits the "legacy" handler; its v2 factory handler needs a migration (#252).
return createLegacyMcpHandler(server, { route, corsOptions })(request, env, ctx);
},
});
41 changes: 13 additions & 28 deletions test/_fixtures/mcp-client.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,49 +24,34 @@ export const parseMcpBody = async (resp) => {
throw new Error(`unparseable /mcp response (ct=${ct}):\n${raw}`);
};

export const makeMcpClient = ({ fetch: fetchImpl, baseUrl, bearer }) => {
export const makeMcpClient = ({ fetch: fetchImpl, baseUrl, bearer, path = "/mcp" }) => {
// Tolerate baseUrl values that include a trailing slash (e.g. MCP_BASE=…:8788/)
// so callers don't have to remember; `${base}/mcp` would otherwise emit `//mcp`,
// which some servers and intermediaries route distinctly.
const mcpUrl = `${baseUrl.replace(/\/+$/, "")}/mcp`;
let sessionId = null;
const mcpUrl = `${baseUrl.replace(/\/+$/, "")}${path}`;
let id = 0;
const call = async (method, params) => {
id += 1;
const headers = {
authorization: `Bearer ${bearer}`,
"content-type": "application/json",
accept: "application/json, text/event-stream",
};
if (sessionId != null) headers["mcp-session-id"] = sessionId;
const r = await fetchImpl(mcpUrl, {
method: "POST",
headers,
headers: {
authorization: `Bearer ${bearer}`,
"content-type": "application/json",
accept: "application/json, text/event-stream",
},
body: JSON.stringify({ jsonrpc: "2.0", id, method, params }),
});
const respSession = r.headers.get("mcp-session-id");
if (respSession != null && sessionId == null) sessionId = respSession;
if (!r.ok) throw new Error(`/mcp ${method} returned ${r.status}: ${await r.text()}`);
if (!r.ok) throw new Error(`${path} ${method} returned ${r.status}: ${await r.text()}`);
const body = await parseMcpBody(r);
if (body.error != null) throw new Error(`/mcp ${method} error: ${JSON.stringify(body.error)}`);
if (body.error != null)
throw new Error(`${path} ${method} error: ${JSON.stringify(body.error)}`);
return body.result;
};
const initialize = async ({ clientInfo }) => {
const result = await call("initialize", {
const initialize = ({ clientInfo }) =>
call("initialize", {
protocolVersion: "2024-11-05",
capabilities: {},
clientInfo,
});
if (sessionId == null) {
throw new Error("server did not return Mcp-Session-Id on initialize");
}
return result;
};
return {
call,
initialize,
get sessionId() {
return sessionId;
},
};
return { call, initialize };
};
80 changes: 51 additions & 29 deletions test/_fixtures/test-worker.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Octokit } from "octokit";

import { buildOAuthProvider, MyMCP as ProductionMCP } from "../../src/index";
import type { ServerInstrument } from "../../src/mcp/server";
import { FakeGitHubHandler } from "./fake-github-handler";

const offlineOctokit = (): Octokit =>
Expand All @@ -19,39 +20,60 @@ const offlineOctokit = (): Octokit =>
type ToolCallback = (...args: unknown[]) => unknown;
type ToolRegistrar = (name: string, config: unknown, callback: ToolCallback) => unknown;

class InstrumentedMCP extends ProductionMCP {
readonly handlerCalls: Record<string, number> = {};
readonly tokensSeen: string[] = [];
const handlerCalls = new Map<string, number>();
const tokensSeen: string[] = [];

protected override createOctokit(accessToken: string): Octokit {
this.tokensSeen.push(accessToken);
return offlineOctokit();
const createOctokit = (accessToken: string): Octokit => {
tokensSeen.push(accessToken);
return offlineOctokit();
};

const instrumentServer: ServerInstrument = (server) => {
server.registerTool("_test_handler_calls", { inputSchema: {} }, () => ({
content: [{ type: "text", text: JSON.stringify(Object.fromEntries(handlerCalls)) }],
}));
server.registerTool("_test_octokit_tokens", { inputSchema: {} }, () => ({
content: [{ type: "text", text: JSON.stringify(tokensSeen) }],
}));
server.registerTool("_test_reset", { inputSchema: {} }, () => {
handlerCalls.clear();
tokensSeen.length = 0;
return { content: [{ type: "text", text: "reset" }] };
});
// Typed `string`, so Reflect.get yields `any` rather than the generic overload.
const method: string = "registerTool";
const register: ToolRegistrar = Reflect.get(server, method).bind(server);
Reflect.set(server, method, (name: string, config: unknown, callback: ToolCallback) =>
register(name, config, (...args) => {
handlerCalls.set(name, (handlerCalls.get(name) ?? 0) + 1);
return callback(...args);
}),
);
};

let durableObjectsConstructed = 0;

// Re-exported as `MyMCP` so the wrangler.jsonc binding (class_name: MyMCP) resolves here.
class OfflineMCP extends ProductionMCP {
constructor(ctx: DurableObjectState, env: Env) {
super(ctx, env);
durableObjectsConstructed++;
}

override async init(): Promise<void> {
this.server.registerTool("_test_handler_calls", { inputSchema: {} }, () => ({
content: [{ type: "text", text: JSON.stringify(this.handlerCalls) }],
}));
this.server.registerTool("_test_octokit_tokens", { inputSchema: {} }, () => ({
content: [{ type: "text", text: JSON.stringify(this.tokensSeen) }],
}));
// Typed `string`, so Reflect.get yields `any` rather than the generic overload.
const method: string = "registerTool";
const register: ToolRegistrar = Reflect.get(this.server, method).bind(this.server);
Reflect.set(this.server, method, (name: string, config: unknown, callback: ToolCallback) =>
register(name, config, (...args) => {
this.handlerCalls[name] = (this.handlerCalls[name] ?? 0) + 1;
return callback(...args);
}),
);
await super.init();
protected override createOctokit(accessToken: string): Octokit {
return createOctokit(accessToken);
}
}
export { OfflineMCP as MyMCP };

// Re-exported under the production name so the wrangler.jsonc Durable Object
// binding (class_name: MyMCP) resolves against this entry when vitest overrides `main`.
export { InstrumentedMCP as MyMCP };
const provider = buildOAuthProvider(
{ fetch: (request, env, ctx) => FakeGitHubHandler.fetch(request, env, ctx) },
{ instrumentServer, createOctokit },
);

export default buildOAuthProvider({
fetch: (request, env, ctx) => FakeGitHubHandler.fetch(request, env, ctx),
});
export default {
fetch: (request: Request, env: Env, ctx: ExecutionContext) =>
new URL(request.url).pathname === "/_test/durable-objects-constructed"
? Response.json({ count: durableObjectsConstructed })
: provider.fetch(request, env, ctx),
} satisfies ExportedHandler<Env>;
Loading