What
POST /oauth/token accepts the same authorization code repeatedly inside its
five-minute window. Each redemption returns HTTP 200 and an access token that
opens a working MCP session.
RFC 6749 §4.1.2 and RFC 9700 §2.1.1 both require an authorization code to be
single-use, and require the AS to revoke previously issued tokens if a code is
replayed.
Where
src/http/oauth-manager.ts, exchangeCode():
// No GETDEL, because there is nothing stored. Replay is bounded by PKCE
// (required at issue time) and by the five-minute expiry sealed inside.
The comment is accurate about why it is bounded — the code is a sealed,
self-describing record with no server-side row to delete, so there is nothing
to mark spent. This issue is not "someone forgot"; it is that the bound is
weaker than the spec's, and the deviation is currently invisible.
Severity: low, and I want to be precise about why
Measured, not argued (see repro): the replayed redemption returns the
identical access token, not a new one. So replay does not mint an
additional credential — it re-serves the one the legitimate client already
holds. And redemption still requires the PKCE verifier, which never leaves the
client process, so an attacker holding only the code (browser history, the
loopback URL bar, a proxy log) cannot redeem it.
The residual exposure is: for five minutes, the code is a second bearer alias
for the access token, for anyone who has both code and verifier. That is
essentially a client-compromise scenario, where the token is already lost.
What it does cost us is the spec's replay signal — a replayed code is
supposed to be treated as evidence of interception and to revoke the issued
tokens. We cannot do that, because we cannot tell a replay from a first use.
Repro
node ~/work/qa-mcp/negatives.mjs # against a local rig on master 1.2.12
PASS correct verifier got 200 want 200 baseline
PASS wrong verifier got 400 want 400
PASS no verifier got 400 want 400
PASS wrong redirect_uri got 400 want 400
PASS tampered code (sig) got 400 want 400
FAIL code replay refused got 200 want 400 second token issued
FAIL replayed token usable got yes want no distinct from first: false
Every other guard the design leans on was mutation-tested in the same run and
holds: a wrong verifier, a missing verifier, a mismatched redirect_uri and a
tampered signature are each refused, one variable changed per case.
Options
- Accept and document it. Turn the code comment into a stated deviation
with this reasoning, so the next reader does not have to re-derive whether
it is deliberate. Cheapest, and defensible given the measured impact.
- A small spent-code set in process memory. Codes live five minutes, so
the set is tiny and self-pruning. Costs the Redis-free property nothing —
this is in-process state, like sessions already are — but it is per-instance,
so it stops working the moment the slug runs more than one container. That
caveat is the whole decision: it buys real single-use only while N=1.
- Shorten the code TTL. Narrows the window without changing the property.
I have no strong preference between 1 and 2; 2's per-instance caveat is the
thing to weigh, and it argues for 1 if multi-instance is on the roadmap.
Found while completing the token -> session -> tool call span (the part of the
login nobody had run). Everything else in that span passed.
What
POST /oauth/tokenaccepts the same authorization code repeatedly inside itsfive-minute window. Each redemption returns HTTP 200 and an access token that
opens a working MCP session.
RFC 6749 §4.1.2 and RFC 9700 §2.1.1 both require an authorization code to be
single-use, and require the AS to revoke previously issued tokens if a code is
replayed.
Where
src/http/oauth-manager.ts,exchangeCode():The comment is accurate about why it is bounded — the code is a sealed,
self-describing record with no server-side row to delete, so there is nothing
to mark spent. This issue is not "someone forgot"; it is that the bound is
weaker than the spec's, and the deviation is currently invisible.
Severity: low, and I want to be precise about why
Measured, not argued (see repro): the replayed redemption returns the
identical access token, not a new one. So replay does not mint an
additional credential — it re-serves the one the legitimate client already
holds. And redemption still requires the PKCE verifier, which never leaves the
client process, so an attacker holding only the code (browser history, the
loopback URL bar, a proxy log) cannot redeem it.
The residual exposure is: for five minutes, the code is a second bearer alias
for the access token, for anyone who has both code and verifier. That is
essentially a client-compromise scenario, where the token is already lost.
What it does cost us is the spec's replay signal — a replayed code is
supposed to be treated as evidence of interception and to revoke the issued
tokens. We cannot do that, because we cannot tell a replay from a first use.
Repro
Every other guard the design leans on was mutation-tested in the same run and
holds: a wrong verifier, a missing verifier, a mismatched
redirect_uriand atampered signature are each refused, one variable changed per case.
Options
with this reasoning, so the next reader does not have to re-derive whether
it is deliberate. Cheapest, and defensible given the measured impact.
the set is tiny and self-pruning. Costs the Redis-free property nothing —
this is in-process state, like sessions already are — but it is per-instance,
so it stops working the moment the slug runs more than one container. That
caveat is the whole decision: it buys real single-use only while N=1.
I have no strong preference between 1 and 2; 2's per-instance caveat is the
thing to weigh, and it argues for 1 if multi-instance is on the roadmap.
Found while completing the token -> session -> tool call span (the part of the
login nobody had run). Everything else in that span passed.