Skip to content

One-click authorize; bind agent identity to the OAuth grant - #7

Merged
maylonmcmurry-max merged 2 commits into
mainfrom
fix/oauth-one-click-authorize
Sep 1, 2026
Merged

maylonmcmurry-max merged 2 commits into
mainfrom
fix/oauth-one-click-authorize

Conversation

@maylonmcmurry-max

Copy link
Copy Markdown
Collaborator

Addresses #3 — "We should just authorize the account. We can't pick a name and voice unless the authorization goes through."

The problem

The consent screen asked for an agent name and voice before authorization completed, and inserted an agents row as a side effect. But nothing ever read the props that row was announced through — src/index.ts discarded the execution context, and handleToolCall declared the id as _agentId and never used it.

So the client was never told the id that had just been created for it. Agents called register_agent, got a second identity, and every consent leaked a dead row.

What changed

Consent is now a single Authorize button that writes nothing. The grant carries an opaque subject, kept stable per browser by an rb_subject cookie so re-consenting does not strand an agent that already registered. register_agent binds the new row to that subject and is idempotent.

This also closes a security hole

Making identity knowable server-side let me fix something worse that was sitting underneath: identity was a caller-supplied string. Every tool read agent_id straight from the arguments and only checked the row existed — so any token holder could act as any agent whose UUID they knew. And UUIDs are freely readable from get_leaderboard, list_battles, list_intros and get_battle.

resolveCaller now resolves the caller from the token and treats a supplied agent_id as an assertion to verify, not an identity to trust. Because the server can now tell who is calling, agent_id became optional on every tool.

Verified with two independent tokens — token A attempting to act as agent B across set_voice, submit_intro, submit_feedback, challenge_agent and get_my_engagement_status:

set_voice                  -> {"error":"agent_id does not belong to this connection."}
submit_intro               -> {"error":"agent_id does not belong to this connection."}
submit_feedback            -> {"error":"agent_id does not belong to this connection."}
challenge_agent            -> {"error":"agent_id does not belong to this connection."}
get_my_engagement_status   -> {"error":"agent_id does not belong to this connection."}

All refused, and B's data confirmed unchanged afterwards.

Two things to be aware of before merging

1. Already-connected agents keep working. Live tokens carry agentId and no subject; they adopt their existing row once under a legacy:<id> subject. Without that fallback every connected client would break on its next tool call. Marked TODO remove after 2026-11-30.

2. Legacy agents become read-only relics. Rows with a NULL owner_subject — pre-existing agents and the Rift house agent — are deliberately unclaimable. Auto-adopting them by name would reopen the same hole, since their UUIDs leak from the read-only tools. Their score, verses and intros persist and still render on /, /leaderboard and /stage, and they remain valid opponent_id/callee_id targets; their operators re-authorize and re-register. Worth a SELECT count(*) FROM agents against remote D1 before deploying to size the blast radius.

/admin/seed-rift is untouched and still works — every Rift mutation is raw SQL keyed on name = 'Rift' and never goes through resolveCaller.

Verification

Against a local wrangler dev instance:

Check Result
Consent form no name field, no voice select, one button
Rows created by consent 0 — before and after two full grants
Two grants → two agents distinct ids, distinct subjects
Re-register, different name returns original agent, already_registered: true, count unchanged
Spoofing attempts (5 tools) all refused, no state mutated
NULL-owner row (Rift stand-in) unhijackable
agent_id omitted entirely resolves to the caller, mutates the right agent
Full intro → call-to-stage → react works without passing agent_id once
Unauthenticated 401

tsc --noEmit holds at the same 4 pre-existing errors (#5) — none new.

Other changes

WorkerEntrypoint conversion for the api handler (the only way to read grant props); owner_subject column added via the existing ensureSchema pattern plus schema.sql, with a unique index; onboarding and /connect copy updated so register_agent is step one.

Note on the other open PR

#6 (the transport fix) is independent and can merge first — it is the actual blocker. Both touch src/index.ts around the apiHandler, so whichever lands second needs a small mechanical rebase.

🤖 Generated with Claude Code

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
rapbattle a830380 Sep 01 2026, 09:41 PM

The consent screen asked for an agent name and voice before authorization
completed, and inserted an agents row as a side effect. Nothing ever read
the props that row was announced through, so the client was never told its
id: agents called register_agent and got a second identity, and every
consent leaked a dead row.

Consent is now a single Authorize button that writes nothing. The grant
carries an opaque subject, kept stable per browser by an rb_subject cookie
so re-consenting does not strand an agent that already registered.
register_agent binds the new row to that subject and is idempotent.

That makes identity knowable server-side, which closes a hole: tools read
agent_id straight from the arguments and only checked the row existed, so
any token could act as any agent whose UUID it knew — and UUIDs are
readable from get_leaderboard, list_battles, list_intros and get_battle.
resolveCaller now resolves the caller from the token and treats a supplied
agent_id as an assertion to verify, not an identity to trust. agent_id is
optional everywhere as a result.

Agents predating this keep working: a grant carrying agentId and no subject
adopts its row once under legacy:<id>. Rows with a NULL owner_subject —
legacy agents and the Rift house agent — stay unclaimable and read-only,
so a leaked UUID cannot be used to seize one. The /admin seed path is
untouched.

Reading the API surface's props requires it to be a WorkerEntrypoint, so
apiHandler became one; defaultHandler and the unauthenticated consent
screen are unchanged.

Refs #3
Rebasing onto #6 merged cleanly by line, and was wrong. The transport
call site still passed `undefined` where the entrypoint now has the
grant's props in scope, so resolveCaller saw no subject and every tool
call over JSON-RPC came back "Unauthenticated. Reconnect over MCP OAuth."

Clients would have connected and then failed at everything, and it
typechecked, so nothing would have flagged it. Confirmed against a local
instance both ways: with `undefined` set_voice returns the auth error,
with `props` it resolves the caller from the token and succeeds.
@maylonmcmurry-max
maylonmcmurry-max force-pushed the fix/oauth-one-click-authorize branch from 7a2c9c9 to a830380 Compare September 1, 2026 21:41
@maylonmcmurry-max

Copy link
Copy Markdown
Collaborator Author

Rebased onto main now that #6 has landed, plus one extra commit that matters.

The rebase merged cleanly by line and was still wrong. #6 added the JSON-RPC transport call site, and this PR turns the handler into a WorkerEntrypoint that has the grant's props in scope. Git reconciled them without a conflict — and produced:

const props = (this.ctx as ...).props;      // identity, right here
...
return handleMcpPost(request, env, undefined, origin);   // and discarded

resolveCaller would have seen no subject, so every tool call over the real MCP transport would return {"error":"Unauthenticated. Reconnect over MCP OAuth."} — clients connect, then nothing works. It typechecks, so neither CI nor review would have caught it.

Confirmed both directions against a local instance:

call site set_voice over JSON-RPC
undefined (naive merge) isError: true — Unauthenticated. Reconnect over MCP OAuth.
props (fixed) isError: false — resolves the caller from the token, succeeds

Also verified end-to-end through the transport with no agent_id passed anywhere: register_agent binds the agent, and a follow-up set_voice resolves the caller from the token alone.

Worth knowing for #8, which is still queued: it does not touch src/index.ts, so it is unaffected by this class of problem.

@maylonmcmurry-max
maylonmcmurry-max merged commit 05b81aa into main Sep 1, 2026
1 check passed
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