Add remote MCP server on Cloudflare Workers (mcp-worker) - #415
Closed
MH4GF wants to merge 4 commits into
Closed
Conversation
Expose tq data over MCP Streamable HTTP so Claude.ai (Web), Claude
Desktop, and Claude Code can read the tq Turso database. v1 ships a
single read-only tool: list_projects.
Architecture (modeled on sentry-mcp's packages/mcp-cloudflare and the
Cloudflare remote-mcp-server guide):
- @cloudflare/workers-oauth-provider wraps the worker; /mcp is the
protected API route, everything else falls through to a Hono app
- GitHub OAuth upstream: /authorize redirects to GitHub carrying the
parsed AuthRequest in the state param; /callback exchanges the code,
fetches the login, and completes the grant
- Authorization is a single-user allowlist (login === "MH4GF"),
enforced both at the OAuth callback (no grant is created for other
users) and per-request in the MCP handler
- agents/mcp createMcpHandler serves MCP statelessly: no Durable
Objects, no KV beyond OAUTH_KV (grant storage for the provider)
- Tools are declared with a defineTool helper (zod input schema +
handler receiving { db, props }) and registered via a generic
registerTool wrapper; the ToolCallback cast is needed because the
SDK's conditional callback type stays deferred for generic shapes
- libsql client (@libsql/client/web) reads the same Turso DB the
local tq CLI uses; the dispatch loop and all writes stay local
Tests run in plain Node (agents/mcp imports cloudflare:* modules and
cannot load outside workerd): list_projects against in-memory libsql,
allowlist, and GitHub OAuth helpers with stubbed fetch. CI gets an
mcp-worker job (bun install / typecheck / vitest). The setup-bun and
checkout pins in that job have no version comment annotations because
the repo write-guard blocks new comments; the SHAs are v2.2.0 and
v6.0.2 respectively.
Package manager is bun (bun.lock committed). COOKIE_ENCRYPTION_KEY
from the original plan is not needed: that secret belongs to the
demo's approval-dialog cookie flow, which v1 skips by redirecting
straight to GitHub.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- docs/mcp.md described a nonexistent tq migrate subcommand; migrations run automatically via PersistentPreRunE on any tq command - wrangler.jsonc secrets comment listed COOKIE_ENCRYPTION_KEY, which the worker never reads (that secret belongs to the approval-dialog cookie flow this v1 does not implement) - Pin bun-version 1.3.5 in CI instead of latest for reproducible runs - Document known v1 security limitations in mcp-worker/README.md: no consent screen on /authorize combined with open dynamic client registration, and unsanitized tool errors; both bounded by the single-user allowlist and read-only tool surface Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces ChangesMCP Worker Implementation
Documentation and CI Integration
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
48-55: ⚡ Quick winConsider adding
persist-credentials: falseto the checkout action.The static analysis tool zizmor flags that the checkout action does not set
persist-credentials: false. While this job does not publish artifacts or require git write access, explicitly disabling credential persistence is a defense-in-depth best practice that prevents potential credential leakage.🔒 Proposed fix
- - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd + with: + persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 48 - 55, Update the actions/checkout step to explicitly disable credential persistence: in the checkout action invocation (the line using actions/checkout@de0fac2e...), add the input persist-credentials: false so the checkout step does not leave Git credentials available to subsequent steps; ensure the key is placed alongside any existing inputs for that step in the workflow file.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mcp-worker/src/app.ts`:
- Around line 53-60: The grant props are persisting the GitHub accessToken
unnecessarily; change the call to c.env.OAUTH_PROVIDER.completeAuthorization to
pass only non-secret data (e.g., { login } or an empty props object) instead of
{ login, accessToken }, keep authRequest, userId, metadata and scope as-is, and
then update the Props type definition in types.ts to remove the accessToken
field so the compiler enforces no token is stored in props; ensure any code that
previously read Props.accessToken is updated to obtain tokens from a secure,
ephemeral store or the OAuth provider when needed.
In `@mcp-worker/src/index.ts`:
- Line 12: The configuration currently exposes clientRegistrationEndpoint:
"/register" which enables dynamic client registration; remove or disable this
property in the config (the clientRegistrationEndpoint entry in mcp-worker's
exported config/object) so the /register route is no longer exposed for v1
single-user allowlisted deployments; alternatively set
clientRegistrationEndpoint to null/undefined or remove any registration route
wiring that references clientRegistrationEndpoint to prevent dynamic client
onboarding until explicitly required.
In `@mcp-worker/src/mcp/server.ts`:
- Around line 21-24: The callback currently calls tool.handler(params, ctx)
directly and lets exceptions bubble to clients; wrap the call inside a try/catch
in the callback function (the one cast to ToolCallback<Input>) so any thrown
error is caught, log the original error to your internal logger (use ctx.logger
if available or console.error) and return the same response shape with a generic
failure message (e.g., { content: [{ type: "text", text: "Tool execution failed"
}] }) instead of propagating internal details from tool.handler.
In `@mcp-worker/src/mcp/tools/list-projects.ts`:
- Around line 28-31: The template is interpolating raw DB fields (row.name,
row.work_dir, and similar) into markdown; create and call a small escape
function (e.g., escapeMarkdown) that backslash-escapes markdown-significant
characters like *, _, `, [, and ] and apply it to workDir and row.name (and any
other user-controlled fields used in the template) before composing the template
literal in the map that builds the bullet lines so that the returned string uses
escaped values.
In `@mcp-worker/src/oauth/github.ts`:
- Around line 23-34: Add explicit request timeouts to the outbound fetch calls
to GITHUB_TOKEN_URL and the subsequent user info fetch (GITHUB_USER_URL) by
using an AbortController with a configurable timeout (e.g., 5–10s): create an
AbortController, pass controller.signal into fetch, start a setTimeout that
calls controller.abort() after the timeout, and clear that timer on successful
completion to avoid leaks; apply this pattern to the fetch that posts the token
exchange (the fetch using GITHUB_TOKEN_URL and body with
client_id/client_secret/code) and to the follow-up fetch for user/profile, and
ensure errors from abort are handled/translated into a clear timeout error path.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 48-55: Update the actions/checkout step to explicitly disable
credential persistence: in the checkout action invocation (the line using
actions/checkout@de0fac2e...), add the input persist-credentials: false so the
checkout step does not leave Git credentials available to subsequent steps;
ensure the key is placed alongside any existing inputs for that step in the
workflow file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1bcd332a-7f96-4b1e-9ae4-2af20e805063
⛔ Files ignored due to path filters (1)
mcp-worker/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.github/workflows/ci.ymldocs/mcp.mdmcp-worker/.dev.vars.examplemcp-worker/.gitignoremcp-worker/README.mdmcp-worker/package.jsonmcp-worker/src/app.tsmcp-worker/src/db/client.tsmcp-worker/src/index.tsmcp-worker/src/lib/define-tool.tsmcp-worker/src/mcp/allowlist.tsmcp-worker/src/mcp/handler.tsmcp-worker/src/mcp/server.tsmcp-worker/src/mcp/tools/list-projects.tsmcp-worker/src/oauth/github.tsmcp-worker/src/types.tsmcp-worker/test/allowlist.test.tsmcp-worker/test/list-projects.test.tsmcp-worker/test/oauth.test.tsmcp-worker/tsconfig.jsonmcp-worker/vitest.config.tsmcp-worker/wrangler.jsonc
- Props now carries only the GitHub login. The access token was stored encrypted in the OAuth grant but no tool reads it, so persisting it only enlarged the secret surface in KV. Tools that need the GitHub API later can re-add it alongside a re-auth. - GitHub token-exchange and user fetches now abort after 10s via AbortSignal.timeout, so a stalled upstream fails the auth request fast instead of hanging until the platform limit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Addressed in f5116b9:
Skipped with reasons:
|
The previous flow redirected straight to GitHub and auto-completed the grant in /callback. Combined with open dynamic client registration, an attacker could register a client with their own redirect URI, lure the allowlisted user through the GitHub flow, and receive an authorization code at their redirect — a confused-deputy grant. The allowlist did not bound this, since the victim is the allowlisted user. Now /authorize GET renders a consent dialog naming the requesting client and its redirect URI; the grant is only created after the user submits /authorize POST. The OAuth state is HMAC-signed with OAUTH_STATE_SECRET and verified on the POST and again at /callback, so the AuthRequest cannot be tampered with and the consent POST cannot be forged (the signature doubles as the CSRF token). The signed-state encoding is UTF-8-safe, replacing the raw btoa/atob that would throw on non-Latin1 input. Adds OAUTH_STATE_SECRET to Env, .dev.vars.example, wrangler.jsonc, and the deploy steps. README/docs security sections rewritten from "known limitation" to the implemented model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
mcp-worker/src/oauth/state.ts (1)
53-76: 💤 Low valueConsider wrapping
base64urlToBytesin try-catch for malformed input.If
state.slice(dot + 1)contains characters invalid for base64url decoding (e.g., non-ASCII),atobat line 16 will throw. This error would propagate up unhandled, whereas other validation failures return a clear "Malformed state" or "Invalid state signature" message.The fix ensures consistent error messaging for all malformed input variants.
♻️ Suggested improvement
export async function verifyState( state: string, secret: string, ): Promise<AuthRequest> { const dot = state.indexOf("."); if (dot === -1) { throw new Error("Malformed state"); } const payload = state.slice(0, dot); - const signature = base64urlToBytes(state.slice(dot + 1)); + let signature: Uint8Array; + try { + signature = base64urlToBytes(state.slice(dot + 1)); + } catch { + throw new Error("Malformed state"); + } const key = await importKey(secret);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp-worker/src/oauth/state.ts` around lines 53 - 76, The verifyState function needs to handle malformed base64url input consistently: wrap calls to base64urlToBytes (both for signature: state.slice(dot + 1) and for payload decoding before JSON.parse) in try-catch and rethrow a clear Error("Malformed state") on any decoding/atob failure; keep the HMAC verification flow (importKey, crypto.subtle.verify) unchanged and only convert decoding errors to the same "Malformed state" error so all invalid-state decoding errors are handled uniformly.mcp-worker/src/app.ts (1)
40-73: 💤 Low valueUnhandled errors from GitHub calls expose stack traces to the client.
exchangeCodeForToken(line 54) andfetchGithubLogin(line 59) can throw on network failures, timeouts, or unexpected responses. These exceptions propagate as 500 errors with stack traces.Per PR discussion, this is accepted for v1 since only the allowlisted owner can reach this flow. Noting for awareness when multi-user support is added.
♻️ Optional: wrap external calls for cleaner error responses
+ let accessToken: string; + let login: string; + try { - const accessToken = await exchangeCodeForToken({ + accessToken = await exchangeCodeForToken({ clientId: c.env.GITHUB_CLIENT_ID, clientSecret: c.env.GITHUB_CLIENT_SECRET, code, }); - const login = await fetchGithubLogin(accessToken); + login = await fetchGithubLogin(accessToken); + } catch { + return c.text("GitHub authentication failed", 502); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp-worker/src/app.ts` around lines 40 - 73, The GitHub network calls in the /callback handler (exchangeCodeForToken and fetchGithubLogin) can throw and currently propagate stack traces to clients; wrap each external call (or both together) in a try/catch inside the app.get("/callback" handler, log the caught error details to the server logger (so debugging info is preserved) and return a generic error response to the client (e.g., c.text("Failed to contact GitHub", 502) or c.text("Authentication service error", 502)) instead of letting the exception propagate; ensure you reference the existing functions exchangeCodeForToken and fetchGithubLogin and keep the existing control flow (isAllowedLogin check and OAUTH_PROVIDER.completeAuthorization) only after successful, caught calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@mcp-worker/src/app.ts`:
- Around line 40-73: The GitHub network calls in the /callback handler
(exchangeCodeForToken and fetchGithubLogin) can throw and currently propagate
stack traces to clients; wrap each external call (or both together) in a
try/catch inside the app.get("/callback" handler, log the caught error details
to the server logger (so debugging info is preserved) and return a generic error
response to the client (e.g., c.text("Failed to contact GitHub", 502) or
c.text("Authentication service error", 502)) instead of letting the exception
propagate; ensure you reference the existing functions exchangeCodeForToken and
fetchGithubLogin and keep the existing control flow (isAllowedLogin check and
OAUTH_PROVIDER.completeAuthorization) only after successful, caught calls.
In `@mcp-worker/src/oauth/state.ts`:
- Around line 53-76: The verifyState function needs to handle malformed
base64url input consistently: wrap calls to base64urlToBytes (both for
signature: state.slice(dot + 1) and for payload decoding before JSON.parse) in
try-catch and rethrow a clear Error("Malformed state") on any decoding/atob
failure; keep the HMAC verification flow (importKey, crypto.subtle.verify)
unchanged and only convert decoding errors to the same "Malformed state" error
so all invalid-state decoding errors are handled uniformly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f880057a-3244-4af4-be06-f0800247ce8d
📒 Files selected for processing (11)
docs/mcp.mdmcp-worker/.dev.vars.examplemcp-worker/README.mdmcp-worker/src/app.tsmcp-worker/src/oauth/approval.tsmcp-worker/src/oauth/github.tsmcp-worker/src/oauth/state.tsmcp-worker/src/types.tsmcp-worker/test/list-projects.test.tsmcp-worker/test/state.test.tsmcp-worker/wrangler.jsonc
✅ Files skipped from review due to trivial changes (3)
- mcp-worker/wrangler.jsonc
- mcp-worker/README.md
- docs/mcp.md
🚧 Files skipped from review as they are similar to previous changes (2)
- mcp-worker/test/list-projects.test.ts
- mcp-worker/.dev.vars.example
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a remote MCP server (
mcp-worker/) that runs on Cloudflare Workers and exposes tq data over MCP Streamable HTTP, so Claude.ai (Web), Claude Desktop, and Claude Code can read the tq Turso database.v1 scope is deliberately minimal: a single read-only tool (
list_projects) to establish the server architecture before adding more tools.Architecture
Modeled on sentry-mcp (
packages/mcp-cloudflare) and the Cloudflare remote-mcp-server guide:@cloudflare/workers-oauth-providerwraps the worker:/mcpis the protected API route, everything else falls through to a Hono app (/authorize,/callback,/health)/authorizeredirects to GitHub carrying the parsedAuthRequestin the state param;/callbackexchanges the code, fetches the login, and completes the grantlogin === "MH4GF"), enforced both at the OAuth callback (no grant is created for other users) and per-request in the MCP handleragents/mcpcreateMcpHandlerserves MCP statelessly: no Durable Objects; onlyOAUTH_KVfor the provider's grant storagedefineToolhelper (zod input schema + handler receiving{ db, props }); the MCP SDK validates inputs through the schema (defaults included) before invoking the handler@libsql/client/webreads the same Turso DB the local tq CLI uses; dispatch and all writes stay localGo side is untouched.
Tests
bun run test— 16 tests:list_projectsagainst in-memory libsql (empty/single/rendering/order+limit), allowlist, GitHub OAuth helpers with stubbed fetchbun run typecheck— cleanwrangler deploy --dry-run— bundles cleanly (no DO binding errors)agents/mcpimportscloudflare:*modules and cannot load outside workerdmcp-workerjob (bun install / typecheck / vitest), bun pinned to 1.3.5Known v1 limitations (documented in mcp-worker/README.md)
/authorize+ open dynamic client registration: a crafted authorize link could route a grant to an attacker-registered client if the allowlisted user completes the GitHub flow. Bounded by the allowlist (owner-only) and the read-only tool surface. Planned fix: approval dialog (Cloudflare demo pattern) before write tools land.Deploy & E2E
Deploy steps in
mcp-worker/README.md(GitHub OAuth App,wrangler kv namespace create OAUTH_KV, 4 secrets,bun run deploy). Client setup indocs/mcp.md. Manual E2E (MCP Inspector / Claude Desktop / Claude.ai againstwrangler dev) is post-merge verification since it needs GitHub OAuth App credentials.🤖 Generated with Claude Code
Summary by CodeRabbit