Skip to content

feat(kilo-mcp): adopt @cloudflare/workers-oauth-provider and zod-validate untrusted input (part 1/1) - #6093

Open
iscekic wants to merge 1 commit into
kwf/add-posthog-analytics-to-the-dcd5-l1from
kwf/use-6066-as-the-base-implement-the-following-06e3-l1
Open

feat(kilo-mcp): adopt @cloudflare/workers-oauth-provider and zod-validate untrusted input (part 1/1)#6093
iscekic wants to merge 1 commit into
kwf/add-posthog-analytics-to-the-dcd5-l1from
kwf/use-6066-as-the-base-implement-the-following-06e3-l1

Conversation

@iscekic

@iscekic iscekic commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Changelog for users

  • MCP sign-in and tool calls keep the same consent page, organization picker, and /mcp endpoint, now served by the OAuth provider library.
  • Replaying a superseded refresh token revokes the whole grant and is rejected with invalid_grant, instead of being accepted once.
  • /mcp answers a revoked, tampered, or foreign bearer with the library 401 challenge and makes no upstream request.
  • A malformed JSON-RPC envelope or tool arguments return the JSON-RPC invalid-request/invalid-params error and dispatch no tool.
  • Dynamic client registration refuses a declared scope this server does not issue.
  • An unregistered client, or one whose redirect URI cannot be validated, gets a local error page and is never redirected.
  • Denied and expired pairings show the restart call-to-action; a pending pairing keeps waiting and polling.
  • Clients discover the protected-resource metadata from the 401 challenge instead of the removed hand-rolled documents.

Changelog for maintainers

  • Adopts @cloudflare/workers-oauth-provider pinned to 0.10.3 plus zod; the library owns the token, registration, and discovery endpoints, client/code/refresh-token/revoked-jti state, and the /mcp audience check.
  • Removes the MCP_TOKEN_SECRET secret and adds the required OAUTH_KV binding. The production KV namespace id is a 32-zero placeholder that must be replaced before deploy — review this first.
  • Migrations add oauth_pending_authorizations and drop the old client, code, refresh-token, and revoked-jti tables. Existing clients and live grants are dropped, so expect client re-registration and user reconnects.
  • Identity now comes only from the library-decrypted grant props; the caller-supplied bearer and organization header are never forwarded upstream.
  • All untrusted input is validated with zod (JSON-RPC envelope, initialize/tools-call params, tool arguments, DCR metadata, consent query and form, forwarded client headers); apps/web responses and the committed catalog remain trusted, and catalog input validation stays AJV.
  • Refresh-token reuse detection wraps the library token endpoint, stores only a per-grant SHA-256 hash, and revokes the grant on replay — deliberately stricter than the library's one-step grace.
  • The hand-rolled HS256, PKCE, DCR, discovery, authorize, and token modules and their tests are removed; the browser pages and the apps/web device-auth pairing live in the new consent modules.
  • Node unit tests alias cloudflare:workers to a stub and inline the provider; OAuth sign-in and error analytics now come from the provider hooks.

E2E proof — log excerpts

[e2] refresh rotation reuse detection -> pass :: Refresh rotated 200; replay previous rt0 -> status=400 error=invalid_grant description=Refresh token reuse detected; the grant has been revoked.; newest rt1 after replay -> status=400 error=invalid_grant; access token at /mcp after replay -> status=401 (e2-refresh-reuse.log).
[p1] consent happy path -> pass :: authorize_status 200, consent_html_has_connect true, redirect_has_code true, token_status 200, token_has_access_token true (e1-consent-happy.log).
[p2] loading pending -> pass :: status_body {"status": "pending"}, consent_html_has_waiting true, consent_has_restart_hidden true (e2-loading-pending.log).
[p3] denied pairing -> pass :: first_status and second_status both {"status": "denied"}, picker_http 400, picker_is_invalid true, no code issued (e4-denied.log).
[p4] empty orgs picker -> pass :: picker_has_personal true, picker_has_acme false, picker_radio_count 1, submit_status 302 with redirect_location containing code= (e5-empty-orgs.log).
[p5] expired pairing -> pass :: status_body {"status": "expired"} and consent_input 200 with no completeAuthorization redirect (e6-expired.log).
[p6] unauthenticated /mcp -> pass :: status 401, www_authenticate resource_metadata="http://127.0.0.1:8815/.well-known/oauth-protected-resource/mcp", hand-rolled probes /oauth/register|/oauth/token|/oauth/authorize all 404 (e7-unauth-mcp.log).
<redacted>
[p8] revoked or invalid token -> pass :: status 401 with error=\"invalid_token\" and upstream_requests_delta 0 (e9-invalid-token.log).
[p9] invalid JSON-RPC request -> pass :: batch and missing-method both return jsonrpc error code -32600 (Batch requests are not supported; Expected a JSON-RPC 2.0 request with a string \"method\") (e11-invalid-request.log).
[p10] OAuth sign-in happy path -> pass :: has_code true, has_state state-1, token_status 200, succeeded_events_for_flow 1 (e12-oauth-happy-org.log).
[p11] unregistered client -> pass :: status 400, has_authorization_failed true, location null, description Invalid client_id, never redirects (e13-unregistered-client.log).
[p12] retryable unhappy and loading -> pass :: consent_waiting_text true, consent_restart_hidden true, while_down_status pending, after restore submit_status 302 redirect_has_code true (e14-retryable-loading.log).
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e2-refresh-reuse.log
initial authorization_code -> status=200 has_refresh=true has_access=true
refresh previous rt0 -> status=200 rotated=true has_access=true
replay previous rt0 -> status=400 error=invalid_grant description=Refresh token reuse detected; the grant has been revoked.
newest rt1 after replay (grant revoked) -> status=400 error=invalid_grant
access token at1 at /mcp after replay -> status=401
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e1-consent-happy.log
{
  "client_id": "pT5uld8tN2pE5_Aa",
  "authorize_status": 200,
  "consent_html_has_connect": true,
  "consent_html_has_waiting": true,
  "picker_status": 302,
  "redirect_location": "http://localhost:9999/callback?code=user-1%3AwLg7LTDRBeSbDx3b%3AiAFr-ZZYd3dJVUzi65tUogP8TnxGW81d&state=state-1&iss=http%3A%2F%2Fmcp.kiloapps.io",
  "redirect_has_code": true,
  "redirect_has_state": "state-1",
  "token_status": 200,
  "token_has_access_token": true,
  "token_body": {
    "access_token": "user-1:wLg7LTDRBeSbDx3b:jtVbjkwjfUjIi2NkgTcp8T1iezJgnBUV",
    "token_type": "bearer",
    "expires_in": 3600,
    "scope": "mcp",
    "refresh_token": "user-1:wLg7LTDRBeSbDx3b:FpPdLwa4OUBiPH1YqBcIEu1aCVaaTCuC",
    "resource": "http://mcp.kiloapps.io/mcp"
  }
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e2-loading-pending.log
{
  "client_id": "6FEoCMJnLj0XtyGj",
  "authorize_status": 200,
  "status_http": 200,
  "status_body": {
    "status": "pending"
  },
  "consent_html_has_waiting": true,
  "consent_has_restart_hidden": true,
  "status_poll_url": "/authorize/status?id=74297ce4-8e20-4bfa-a752-6992a1dd308d"
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e4-denied.log
{
  "client_id": "COjnYG7cvjrJSjra",
  "first_status": {
    "status": "denied"
  },
  "second_status": {
    "status": "denied"
  },
  "picker_http": 400,
  "picker_is_invalid": true,
  "picker_text_snippet": " Authorization request failed :root{color-scheme:dark;--background:#151515;--card:#202020;--overlay:#333333;--hover:#3a3a3a;--foreground:#fafafa;--muted:#a3a3a3;--primary:#f7
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e5-empty-orgs.log
{
  "client_id": "xMW_UqMtZT_asd8V",
  "status_body": {
    "status": "needs_org",
    "picker_url": "/authorize/org?id=6ae8bb75-eac3-4bff-ab2a-15550c073da8"
  },
  "picker_has_personal": true,
  "picker_has_acme": false,
  "picker_radio_count": 1,
  "submit_status": 302,
  "redirect_location": "http://localhost:9999/callback?code=user-1%3AdYPoew60DNDb3Lzk%3AvWR5KKhdGTaRbhNy2H2jhGliKbgDhawg&state=state-1&iss=http%3A%2F%2Fmcp.kiloapps.io"
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e6-expired.log
{
  "client_id": "bXQt1T1ObFaobNtf",
  "status_body": {
    "status": "expired"
  },
  "consent_input": 200
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e7-unauth-mcp.log
{
  "status": 401,
  "www_authenticate": "Bearer realm=\"OAuth\", resource_metadata=\"http://127.0.0.1:8815/.well-known/oauth-protected-resource/mcp\", scope=\"mcp\"",
  "resource_metadata_points_to_library": true,
  "probes": {
    "/oauth/register": 404,
    "/oauth/token": 404,
    "/oauth/authorize": 404,
    "/.well-known/oauth-authorization-server/mcp": 404
  }
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e8-happy-tool-call.log
{
  "token_status": 200,
  "token_audience": "http://mcp.kiloapps.io/mcp",
  "search_status": 200,
  "search_result_count": 10,
  "search_first_path": "organizations.admin.list",
  "call_status": 200,
  "call_result_present": true,
  "upstream_request": {
    "url": "/api/trpc/organizations.list",
<redacted>
    "org_header": "org-1"
  },
  "expected_org": "org-1"
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e9-invalid-token.log
{
  "status": 401,
  "www_authenticate": "Bearer realm=\"OAuth\", resource_metadata=\"http://127.0.0.1:8815/.well-known/oauth-protected-resource/mcp\", error=\"invalid_token\", scope=\"mcp\"",
  "upstream_requests_delta": 0
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e11-invalid-request.log
{
  "batch_status": 200,
  "batch_body": {
    "jsonrpc": "2.0",
    "id": null,
    "error": {
      "code": -32600,
      "message": "Batch requests are not supported; send one JSON-RPC message per request."
    }
  },
  "missing_method_status": 200,
  "missing_method_body": {
    "jsonrpc": "2.0",
    "id": 7,
    "error": {
      "code": -32600,
      "message": "Expected a JSON-RPC 2.0 request with a string \"method\"."
    }
  }
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e12-oauth-happy-org.log
{
  "client_id": "oHre-huwEPrgoV4V",
  "redirect_location": "http://localhost:9999/callback?code=user-1%3AhZLT24lL9LNl2J-X%3AQl3iixceu2I_Qzpg79h7WZ9DvG_DvxS5&state=state-1&iss=http%3A%2F%2Fmcp.kiloapps.io",
  "has_code": true,
  "has_state": "state-1",
  "token_status": 200,
  "token_has_access_token": true,
  "succeeded_events_for_flow": 1
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e13-unregistered-client.log
{
  "status": 400,
  "has_authorization_failed": true,
  "has_unknown_client": false,
  "location": null,
  "reason_line": "Reason: invalid_request . Close this tab and retry from your MCP client.",
  "description": "Invalid client_id",
  "text": "Authorization request failed :root{color-scheme:dark;--background:#151515;--card:#202020;--overlay:#333333;--hover:#3a3a3a;--foreground:#fafafa;--muted:#a3a3a3;--primary:#f7f586;--primary-h
}
/home/igor_kilocode_ai/.local/share/kwf/sections/use-6066-as-the-base-implement-the-following-06e3/e2e-backend/e14-retryable-loading.log
{
  "consent_waiting_text": true,
  "consent_restart_hidden": true,
  "while_down_status": {
    "status": "pending"
  },
  "while_down_authorize_status": 503,
  "after_restore_status": {
    "status": "needs_org",
    "picker_url": "/authorize/org?id=c0502291-e57d-432f-9fff-b1b711914287"
  },
  "picker_has_org1": true,
  "submit_status": 302,
  "redirect_location": "http://localhost:9999/callback?code=user-1%3ASORxDITqMVXJcJGD%3AA7LtNyoGDCbTMAvfx_ySKfcu8cPIYyVS&state=sf1&iss=http%3A%2F%2Fmcp.kiloapps.io",
  "redirect_has_code": true
}
Owner request for the stack

Use 6066 as the base. Implement the following:

  1. replace hand rolled oauth stuff with a library (@cloudflare/workers-oauth-provider, or some other)
  2. validate all untrusted incoming data with zod - data from our own services is trusted

PR stack (merge bottom to top)

Comment thread services/kilo-mcp/src/oauth/refresh-reuse.ts Outdated
Comment thread services/kilo-mcp/src/oauth/refresh-reuse.ts Outdated
Comment thread services/kilo-mcp/src/oauth/refresh-reuse.ts Outdated
Comment thread services/kilo-mcp/src/index.ts Outdated
Comment thread services/kilo-mcp/wrangler.jsonc Outdated
Comment thread services/kilo-mcp/src/index.test.ts
Comment thread services/kilo-mcp/src/oauth/consent.test.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)
  • services/kilo-mcp/src/oauth/refresh-reuse.test.ts
Previous Review Summaries (2 snapshots, latest commit a980e20)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit a980e20)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The @cloudflare/workers-oauth-provider migration and DO-backed strict refresh-reuse rewrite look sound: prior findings (non-atomic replay check, revocation from unverified token parts, unwired onError, placeholder OAUTH_KV, over-broad response cloning, and the two weak tests) are resolved. The only new finding is a test that does not exercise the duplicate-parameter guard it claims to cover; no production bugs or memory leaks were found.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
services/kilo-mcp/src/oauth/refresh-reuse.test.ts 115 Ambiguous-parameter case sends the current token last, so it passes even if the duplicate-parameter guard regresses.
Files Reviewed (14 files)
  • services/kilo-mcp/src/oauth/refresh-reuse.ts
  • services/kilo-mcp/src/store/oauth-store.ts
  • services/kilo-mcp/src/db/sqlite-schema.ts
  • services/kilo-mcp/src/index.ts
  • services/kilo-mcp/wrangler.jsonc
  • services/kilo-mcp/src/deployment-config.test.ts
  • services/kilo-mcp/src/index.test.ts
  • services/kilo-mcp/src/oauth/consent.test.ts
  • services/kilo-mcp/src/oauth/refresh-reuse.test.ts - 1 issue
  • services/kilo-mcp/src/store/oauth-store.test.ts
  • services/kilo-mcp/drizzle/0004_refresh_token_history.sql
  • services/kilo-mcp/drizzle/migrations.js
  • services/kilo-mcp/drizzle/meta/_journal.json
  • services/kilo-mcp/drizzle/meta/0004_snapshot.json (generated)

Fix these issues in Kilo Cloud

Previous review (commit b4c7ee6)

Status: 7 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 4
SUGGESTION 3
Issue Details (click to expand)

WARNING

File Line Issue
services/kilo-mcp/src/oauth/refresh-reuse.ts 115 Non-atomic refresh-reuse check: concurrent refreshes with the same token both pass, and last-writer-wins marker storage can make a legitimately issued token look like a replay and revoke the grant.
services/kilo-mcp/src/oauth/refresh-reuse.ts 119 revokeGrant is invoked from unverified userId:grantId parts of the presented refresh token, bypassing the library's secret check.
services/kilo-mcp/src/index.ts 573 onError is wired without deps, so the documented provider OAuth error analytics never fire (the hook is a no-op in production).
services/kilo-mcp/wrangler.jsonc 49 Production OAUTH_KV id is the all-zero placeholder; the worker's OAuth state binding must be replaced before deploy.

SUGGESTION

File Line Issue
services/kilo-mcp/src/oauth/refresh-reuse.ts 161 rememberIssuedRefreshToken runs on every forwarded response, cloning/parsing unrelated JSON bodies (including /mcp results).
services/kilo-mcp/src/index.test.ts 363 Test claims to send a caller-supplied org header but never does, so the anti-spoofing guarantee is not exercised.
services/kilo-mcp/src/oauth/consent.test.ts 314 Vacuous assertion does not enforce the stated no-device-code-leak invariant.
Files Reviewed (33 files)
  • services/kilo-mcp/src/index.ts - 1 issue
  • services/kilo-mcp/src/oauth/refresh-reuse.ts - 3 issues
  • services/kilo-mcp/wrangler.jsonc - 1 issue
  • services/kilo-mcp/src/index.test.ts - 1 issue
  • services/kilo-mcp/src/oauth/consent.test.ts - 1 issue
  • services/kilo-mcp/src/auth.ts
  • services/kilo-mcp/src/auth/http.ts
  • services/kilo-mcp/src/call.ts
  • services/kilo-mcp/src/types.ts
  • services/kilo-mcp/src/schemas.ts
  • services/kilo-mcp/src/oauth/consent.ts
  • services/kilo-mcp/src/oauth/kilo-pairing.ts
  • services/kilo-mcp/src/oauth/provider-hooks.ts
  • services/kilo-mcp/src/oauth/pages.ts
  • services/kilo-mcp/src/oauth/refresh-reuse.test.ts
  • services/kilo-mcp/src/oauth/provider-hooks.test.ts
  • services/kilo-mcp/src/oauth/kilo-pairing.test.ts
  • services/kilo-mcp/src/oauth/pages.test.ts
  • services/kilo-mcp/src/store/oauth-store.ts
  • services/kilo-mcp/src/store/oauth-store.test.ts
  • services/kilo-mcp/src/db/sqlite-schema.ts
  • services/kilo-mcp/src/schemas.test.ts
  • services/kilo-mcp/src/auth.test.ts
  • services/kilo-mcp/src/call.test.ts
  • services/kilo-mcp/src/test-support/cloudflare-workers-stub.ts
  • services/kilo-mcp/drizzle/0002_square_the_spike.sql
  • services/kilo-mcp/drizzle/0003_wandering_colleen_wing.sql
  • services/kilo-mcp/drizzle/migrations.js
  • services/kilo-mcp/drizzle/meta/_journal.json
  • services/kilo-mcp/package.json
  • services/kilo-mcp/vitest.config.ts
  • services/kilo-mcp/worker-configuration.d.ts
  • pnpm-workspace.yaml

Removed hand-rolled auth modules and their tests were reviewed as deletions (intentional). pnpm-lock.yaml, Drizzle snapshots, and generated meta JSON were excluded as generated artifacts.

No memory leaks were identified: the DO alarm is a single self-rescheduling purge, the validatorCache is a WeakMap, refresh-latest: keys carry a 30-day TTL, and pending-authorization rows are purged by the alarm. No .md files are changed in this PR.

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch kwf/add-posthog-analytics-to-the-dcd5-l1

@iscekic
iscekic marked this pull request as draft September 12, 2026 01:30
@iscekic
iscekic added this pull request to stack #6067 September 12, 2026 01:47
iscekic added a commit that referenced this pull request Sep 12, 2026
@iscekic
iscekic force-pushed the kwf/use-6066-as-the-base-implement-the-following-06e3-l1 branch from b4c7ee6 to a980e20 Compare September 12, 2026 11:41
@iscekic
iscekic marked this pull request as ready for review September 12, 2026 11:57
Comment thread services/kilo-mcp/src/oauth/refresh-reuse.test.ts Outdated
@iscekic
iscekic marked this pull request as draft September 12, 2026 12:15
@iscekic
iscekic force-pushed the kwf/use-6066-as-the-base-implement-the-following-06e3-l1 branch from a980e20 to 843c375 Compare September 12, 2026 12:25
@iscekic
iscekic marked this pull request as ready for review September 12, 2026 12:38
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 12, 2026
@iscekic iscekic self-assigned this Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant