Skip to content

MCP sign-in: the popup's Allow survives a second Approve load; Deny is final; refresh hardening - #192

Merged
maxfain merged 41 commits into
mainfrom
claude/confident-babbage-8ac6bq
Oct 13, 2026
Merged

maxfain merged 41 commits into
mainfrom
claude/confident-babbage-8ac6bq

Conversation

@maxfain

@maxfain maxfain commented Oct 13, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes a sign-in bug in the hosted MCP's OAuth flow that could strand OpenAI's plugin reviewers. It also tightens two edge cases found while reviewing that fix. This is a server-only change: tool definitions and server instructions are untouched, so it doesn't affect the plugin submission currently in review.

The bug

The sign-in popup (where ChatGPT waits) and the email-link tab share the mcp_authreq cookie, and every load of /oauth/approve minted a new CSRF token into it.

If the Approve page loaded a second time, the Allow button already showing in the popup failed with csrf validation failed. That happened when someone clicked "Approve here instead" in the email tab, or reloaded.

The only Allow still working was then the one in the email tab. That one redirects to ChatGPT without its pending state, so the code is never exchanged and the connection never finishes.

Changes (packages/api/src/mcp/)

  • One CSRF token per request. approvePage reuses the token the request's signed cookie already carries, and mints one only if the cookie has none. The token stays bound to the signed, HttpOnly, host-only cookie and its authreq_id.
  • Deny is final. Deny now consumes the authorization request before redirecting with access_denied. With a shared token, an Allow form still open in another window could otherwise mint a code after the user denied. Polling windows now see done.
  • A wrong client_id no longer burns a refresh token. rotateRefreshToken takes the presented client_id and checks it before consuming the token. A mismatch returns invalid_client and leaves the token usable by its own client.
    • Before, the token was consumed and its successor never returned.
    • The owner's next refresh then looked like reuse and revoked the whole chain.
  • /oauth/token responses are never cached. Every response, success or error, carries Cache-Control: no-store and Pragma: no-cache (RFC 6749 §5.1).
  • Failed refreshes are logged. [oauth] refresh failed is logged with its status (reused, invalid or client_mismatch) and an 8-character client-id prefix, so a connection that drops after its 1-hour token is visible in wrangler tail.
  • ping returns an empty result, as MCP 2025-06-18 requires. It used to return -32601.
  • The email tab's fallback link now reads "Approve in this tab instead". It warns that the app may not finish connecting from there.

MCP_CONNECTOR_SPEC.md §3/§4 and the CHANGELOG describe the new behaviour.

How this was found

I traced the full popup flow while diagnosing why the plugin portal's Connect seemed stuck. That symptom turned out to be on OpenAI's side: the portal never requested /oauth/authorize. The CSRF bug was found in that trace. The Deny and client_id findings came from an adversarial review of the first fix. Each was confirmed by reproducing it in a scratch copy of the tests.

Test plan

  • New tests, each confirmed to fail on the previous code and pass now:
    • A second Approve load keeps the popup's Allow valid.
    • Deny then a late Allow returns 400 and mints no code.
    • A wrong-client_id refresh is refused, then the right client's refresh succeeds (both the store and the endpoint).
    • Token responses carry no-store.
    • ping returns {}.
  • packages/api: 1288 tests pass (75 files) on the tree merged with current main.
  • npm run typecheck, npm run lint (0 errors; the one warning is in untouched routes/probe.ts), npm run lint:words, sync-positioning --check.
  • After deploy:
    • curl -s -X POST https://mcp.basedagents.ai/mcp -H 'content-type: application/json' -d '{"jsonrpc":"2.0","id":1,"method":"ping"}' returns {"jsonrpc":"2.0","id":1,"result":{}}.
    • A ChatGPT connect still completes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa


Generated by Claude Code

claude added 30 commits October 3, 2026 20:23
The hosted MCP server (the agent-registry-mcp Worker) had no
documentation outside docs/chatgpt-plugin/, and the worker code cited a
"SPEC §N" that did not exist in the repo.

- MCP_CONNECTOR_SPEC.md (new, repo root), sectioned to match the code's
  §0–§10 citations: isolation, CORS, the OAuth 2.1 authorization server
  (RFC 9728/8414 metadata, public DCR, PKCE S256, RFC 8707), magic-link
  sign-in with the same-browser binding, tokens and refresh rotation,
  the optional-bearer /mcp transport, the 11 tools with annotations,
  draft_task_link bounds, rate limits, the variable table, tests and
  local dev. The three worker headers now point to it.
- Hosted vs local wherever MCP is described: README (MCP section,
  architecture, discovery), packages/mcp README, a new section in
  packages/api README, agent.json for_agents.mcp_server.hosted, llms.txt
  via the positioning sync template, /docs/agents, GettingStarted and a
  ChatGPT & claude.ai card on /integrations.
- Stale facts fixed: the stdio server has 26 tools (was 23 in README,
  GettingStarted, agent.json); packages/mcp README version 0.7.2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
Merge origin/main (#163 MCP acquisition attribution): README keeps the
hosted/local split and carries #163's --source github tags and analytics
note; CHANGELOG is the union.

Licensing: five src/mcp files said "PROPRIETARY control-plane", but
LICENSING.md lists the MCP OAuth worker under the Apache-2.0 registry API
(everything outside src/control/), a deliberate entry. The headers now
match LICENSING.md and note that the worker imports owner lookup and email
from the proprietary src/control/ subtree. The spec states the same.

Go-live: mcp.basedagents.ai is NXDOMAIN. CI's Deploy MCP Worker attaches
the zone route but creates no DNS, so the hosted server has never been
reachable. Documented the one-time proxied `mcp` record in spec §1, the
ChatGPT runbook (step 0), wrangler.mcp.toml and the API README. The spec
also covers #163: OAuth-only hosted attribution (hosted:<client_id>,
source/campaign/acquisition_id on the connector URL, owner identity never
written) and the ACQUISITION_ANALYTICS switch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
#164)

- The hosted browse_tasks described "default: open" but sent no status,
  so the API's own default (everything but cancelled/expired) leaked
  claimed, submitted and verified tasks into "find paid work". It now
  sends status=open unless the caller picks one. Test added.
- Local dev in the spec (§10), the API README and the ChatGPT runbook:
  apply the D1 migrations locally first (the MCP config declares none;
  both configs share the database id), and override MCP_ISSUER and
  MCP_RESOURCE_URL to localhost so OAuth discovery doesn't advertise
  the production endpoints.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
With mcp.basedagents.ai now resolving, a production smoke test showed
every read tool failing with "BasedAgents API returned 522". The API
answers 200 directly, but a Worker's fetch() to a hostname on its own
zone (api.basedagents.ai on basedagents.ai) skips that zone's Worker
routes and goes to the placeholder origin behind the DNS record, which
Cloudflare answers with 522. Reproduced twice on get_chain_status and
browse_tasks.

- wrangler.mcp.toml: [[services]] binding API -> agent-registry-api.
  `wrangler deploy --dry-run` confirms env.API resolves.
- handler.ts: apiFetch goes through env.API when bound, else the
  existing public fetch (tests, local dev). The client's token still
  never crosses. New test: with the binding present, reads use it and
  never call global fetch.
- Docs (spec §5/§8/§9, API README, ChatGPT runbook, CHANGELOG): the
  binding is required, not a scaling nicety.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
ChatGPT mixed auth ("OAuth or no authentication"), per OpenAI's custom
MCP and Apps SDK auth docs:
- tools/list: every tool declares a top-level securitySchemes, noauth
  for the reads and draft_task_link, and oauth2 with scope board:post
  for post_to_board.
- An unlinked or under-scoped post_to_board answers an isError tool
  result with _meta["mcp/www_authenticate"] (insufficient_scope,
  scope="board:post"). That is ChatGPT's account-link trigger. It
  replaces the transport 401, which ChatGPT reads as the connection
  failing. A dead presented token stays a 401, for refresh.
- The server instructions open with a self-contained tool map in the
  first 512 chars.

Greptile on #164:
- P1: binding requests carried no client IP, so the API's per-IP
  limits collapsed into one 'unknown' bucket. The caller's edge-set IP
  is now forwarded as CF-Connecting-IP and X-Forwarded-For. Only this
  Worker reaches the API over the binding, and the edge overwrites any
  client-supplied CF-Connecting-IP. Test added.
- P2: local dev now runs the API Worker beside the MCP Worker (8787 and
  8788), which wrangler's dev registry connects, with issuer and
  resource on 8788.

The runbook carries the exact ChatGPT form settings. Tests 99/99.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
CHANGELOG union: main's CLI entry and this branch's entries both kept.
No other overlap (#168 touches only packages/sdk). MCP tests 99/99,
typecheck, sync checks and lint clean on the merged tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
OpenAI's plugin portal imports a ZIP (plugin.json + mcp.json + assets) and
shows the imported listing read-only, so the copy-paste metadata.json is
replaced by a generated package:

- sync-positioning §10 now writes docs/chatgpt-plugin/package/plugin.json
  (full Agent Plugins manifest: interface, review test cases, release
  notes), package/mcp.json, and test-cases.md from the positioning block.
- scripts/build-chatgpt-plugin.mjs zips the package with the 512px icon as
  logo and composerIcon into docs/chatgpt-plugin/dist/ (gitignored),
  refusing a stale package or an out-of-spec icon.
- New listing copy ("Hire AI agents, find paid work"), prompts and test
  cases; draft_task_link and the instructions now name QA/test-my-MCP asks.
- check-positioning covers the new surfaces; runbook and CHANGELOG updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa

# Conflicts:
#	CHANGELOG.md
#	docs/chatgpt-plugin/README.md
#	packages/api/src/mcp/chatgpt.json
#	packages/web/src/content/positioning.ts
…site

The "verify delivered work" golden prompt asked for the newest task, which
is usually open and has no receipt. It now asks for the most recent verified
task, and the expected behaviour names the status filter. The runbook says
the build needs the zip CLI. (Greptile on #171)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
The plugin portal's challenge token goes in OPENAI_APPS_CHALLENGE in
wrangler.mcp.toml, so the MCP worker serves it at
mcp.basedagents.ai/.well-known/openai-apps-challenge. The same token is
also a static file on the apex (text/plain via _headers), in case the
portal verifies basedagents.ai itself. It's public plain text, not a secret.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
agent-registry-mcp shipped with no secrets. Without MCP_SIGNING_SECRET,
/oauth/authorize fails closed with "authorization server misconfigured"; without
RESEND_API_KEY, magic links only reach the log. The runbook now lists both,
with the wrangler commands and the dashboard path, before domain verification.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
deferSend swallowed every error, so a wrong RESEND_API_KEY failed with no trace.
Sign-in links were minted but never mailed. The response still doesn't change
(low-enumeration), but the failure is now console.error'd for wrangler tail.
The runbook says the Resend secret is a real re_ key, not the random signing
string, and how to see the failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
The portal's scan flagged post_to_board as openWorldHint: false while it writes
to a public board. It now sets openWorldHint: true, and the test pins the hint:
only post_to_board is open-world. The spec and the annotation comment match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
A rejecting sender now covers the catch in deferSend. The response is still
the same "Check your email" page. waitUntil gets one promise that resolves
rather than rejecting, and console.error carries the provider's message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
Auditing an MCP server is the plugin's best use case, and the connector had
no tool for it.

- scan_mcp_server: security-scans an MCP server or agent tool from its npm or
  PyPI package or GitHub repo through /v1/scan. It reuses the stored report
  unless asked to rescan, and relays the scanner's own errors. Annotated
  open-world and not read-only, because a scan downloads third-party code and
  stores a public report.
- draft_audit_request: prefilled link to the Agent Compatibility Audit intake,
  with the price from the live catalog. Nothing is submitted from MCP. The
  console intake now prefills its descriptive fields from the query string,
  and a signed-in visitor's redirect keeps the query.
- The ChatGPT listing, review test cases, release notes and server
  instructions lead with "audit my MCP server". Spec, READMEs, agent.json
  and the runbook list the 13 tools.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa

# Conflicts:
#	MCP_CONNECTOR_SPEC.md
#	packages/api/src/mcp/handler.test.ts
… matches

Greptile on #175:
- An unprefixed GET /v1/scan/:name falls back to any source's row with that
  name, so npm "requests" could come back as pypi:requests. Lookups are now
  always source-prefixed, and a report from another source is ignored and
  triggers a scan. A test covers the pypi-vs-npm case.
- draft_audit_request validates URLs the way IntakeSchema does (new URL,
  https, no embedded credentials) instead of a regex. It accepts @ in paths
  and rejects https://? and user:pw@ hosts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
new URL() accepts HTTPS://Host, but the console's prefill keeps only
lowercase https:// values, so such a URL vanished from the form. The tool
now passes on u.href (lowercase scheme and host), and the test covers it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
OpenAI's plugin review "couldn't complete an automated assessment" of the
privacy policy. /privacy and /terms were SPA routes, so a crawler that
doesn't run JS got the app shell (the homepage's agent block) and no policy.
Both are now prerendered with their own title, description and canonical,
and they're in the sitemap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
The policy only described the agent registry, so ChatGPT's review had
nothing to assess for the plugin. A new section covers, as the code does:
- tool calls: arguments aren't stored, except a confirmed board post and a
  requested public scan report;
- account connection: the email is used for lookup and the Resend link and
  isn't stored by the flow; hashed tokens (1 h access, 30 d refresh); one
  10-minute cookie;
- pseudonymous usage attribution for connected apps (daily counts deleted
  after 400 days);
- no sale, ads or training, and how to disconnect or delete.

The IP section no longer says addresses are "not stored": rate-limit counters
keep them (SHA-256 on the MCP server, as-is on the API), plus an
app-registration hash. Data Sharing lists Cloudflare, Resend and Google
Analytics, and that scans fetch from npm, PyPI or GitHub.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
…reptile on #178)

oauth_clients.reg_ip_hash sits beside the client_id, and tokens tie that
client_id to the owner, so "not linked to accounts" overstated it. The text
now says the IP-based rate-limit counters aren't linked, and that the
registration hash can be linked to you once you connect that app.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
The portal blocks submission until plugin.json carries
extensions.com.openai.review.demo_recording_url. positioning gains
chatgpt.demoRecordingUrl (https only, checked at sync). plugin.json includes
it once set, and the ZIP build warns while it's empty. The runbook says where
it goes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
ChatGPT reported "Missing OAuth callback data" after a successful sign-in.
It opens /oauth/authorize in a popup and expects the redirect back there,
but the emailed link opens a new tab. Approving in that tab redirected
into a window without ChatGPT's pending state, so the minted code was
never collected.

- The "check your email" page polls the new GET /oauth/status (2.5 s, up to
  15 min) and moves to the new GET /oauth/approve on "ready". A no-JS
  "continue" link goes there too.
- /oauth/continue (the link tab) still consumes the challenge and enforces
  the same-browser cookie, then says to return to the original window, with
  an "Approve here instead" fallback. It no longer renders the Allow form.
- Both endpoints answer only for the request in the signed cookie. Approve
  still needs an owner bound by a same-browser link click, so the
  login-fixation binding is unchanged.
- The tests walk continue → approve → decision. New tests cover the handoff
  (pending → ready → done, with state preserved), cookieless and forged
  requests, and a link clicked in another browser.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
claude added 11 commits October 7, 2026 22:11
The portal blocks submission without review.demo_recording_url, so it now
points at the Loom walkthrough. The plugin.json is re-synced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
OpenAI's review couldn't confirm the category ("Productivity") and asked
for the listing to explain one main purpose. It had mixed hiring, paid work
and audits.

- Short description: "Audit MCP servers and agents". The long description
  leads with the security scan and the agent compatibility audit, then
  agent trust checks. The task marketplace follows as a secondary use.
- Category: Developer Tools. Prompts, capabilities, keywords, test-case
  order, release notes and server instructions follow the same order.
- draft_task_link drops "test my MCP server" and points audits to
  scan_mcp_server or draft_audit_request, so the tools don't overlap.
- The initialize test expects the new opening line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
The plugin review asked to remove pricing, subscription offers and
promotions from the description. These are gone from every listing field:
- bounty, USDC, escrow and "price" in the long description;
- "payouts" in the capabilities;
- "usdc bounty" and "paid tasks" in the keywords;
- "paid" in the release notes.

plugin.json's top-level description had reused the site subhead, which
mentions USDC and "paid". It now has its own text,
chatgpt.packageDescription.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
review.demo_recording_url now points at the re-recorded Loom walkthrough,
which follows the audit-first listing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa

# Conflicts:
#	packages/web/src/content/positioning.ts
…s a second Approve load

The sign-in popup and the email-link tab share the mcp_authreq cookie, and
every /oauth/approve load minted a new CSRF token into it. Loading the page a
second time (the link tab's fallback link, a reload) failed the Allow form
already showing in the popup, leaving only the email tab's Allow, which
returns to ChatGPT without its pending state.

- approvePage reuses the cookie's CSRF token for the request it names and
  mints one only when the cookie has none.
- The email tab's fallback reads "Approve in this tab instead" and warns the
  app may not finish connecting from there.
- /oauth/token sends Cache-Control: no-store and Pragma: no-cache on every
  response (RFC 6749 5.1) and logs refused refreshes with their status and a
  client-id prefix.
- The MCP handler answers ping with an empty result.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa
…sh token

Two findings from an adversarial review of the shared-CSRF change:

- Deny now consumes the authorization request before redirecting with
  access_denied. With one CSRF token per request, an Allow form still open
  in another window could otherwise mint a code after the user denied;
  polling windows now see 'done'.
- rotateRefreshToken takes the presented client_id and checks it before
  consuming the token. A mismatch returns invalid_client and leaves the token
  usable by its own client, instead of consuming it unreturned and turning
  the owner's next refresh into a 'reused' chain revocation. The refusal is
  logged like every other failed refresh.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K5VPBCqJw8VrBa7GrLboYa

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review was skipped because it would exceed your organization's monthly flex usage limit. Raise the limit in billing settings or wait until the next billing period resets limits.

@github-actions

Copy link
Copy Markdown

🔎 Console preview: https://76eb7c01.basedagents-console.pages.dev
🌐 Site preview: https://a68f6f38.auth-ai-web.pages.dev

(Passkey ceremonies only work on app.basedagents.ai — previews are for UI review.)

@maxfain
maxfain merged commit 3319207 into main Oct 13, 2026
6 checks passed
maxfain pushed a commit that referenced this pull request Oct 14, 2026
Brings in the MCP sign-in popup fix (#192). CHANGELOG.md conflicted only
because both sides added an entry at the top of Unreleased; both are kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HUEV4Uf69wpKrB7GQqEQiX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants