Repository navigation
[Server] Bind requestState to the request it was minted for - #625
BrocksiNet wants to merge 3 commits into
Conversation
The 2026-07-28 specification recommends binding a requestState to the authenticated principal and the originating request. Add Builder::setRequestStateBinder() and RequestStateBinderInterface: the binder's value is covered by the state's MAC on mint and recomputed on verify, so a state echoed on another call or under another principal is refused like a forged one. OriginatingRequestBinder binds the principal, the method, the tool, prompt or resource and a digest of the arguments. Opt-in: without a binder, the wire format and verification are unchanged.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66dc92a2a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'principal' => null === $accessToken ? null : [ | ||
| 'iss' => \is_string($issuer = $accessToken->getClaim('iss')) ? $issuer : null, | ||
| 'client' => $accessToken->getClientId(), | ||
| 'sub' => $accessToken->getSubject(), |
There was a problem hiding this comment.
When two authorized tokens come from the same issuer/client but identify users through a claim other than sub (or carry no standard identity claims), this serializes the same principal for both. AccessToken permits arbitrary claims and JwtTokenValidator::validate() does not require sub or client_id, so for identical method/arguments one user's state can verify under the other's token; fail closed when the standard identity is unavailable or make the stable principal claim/extractor configurable.
Useful? React with 👍 / 👎.
| if (!array_is_list($value)) { | ||
| ksort($value, \SORT_STRING); | ||
| } | ||
|
|
||
| return array_map(self::canonical(...), $value); |
There was a problem hiding this comment.
When a nested tool argument is a JSON object with numeric keys out of list order, such as {"1":"one","0":"zero"}, associative decoding makes it a non-list, but sorting it to keys 0,1 and returning an array makes json_encode() serialize it exactly like ["zero","one"]. SchemaValidator::convertDataForValidator() and mixed handlers can distinguish the object and array requests, so a state can verify after a semantically different argument change; retain the JSON container type during canonicalization or fail closed on ambiguous maps.
Useful? React with 👍 / 👎.
chr-hertel
added
Server
There was a problem hiding this comment.
🟡 Changes recommended
Argument canonicalization conflates numeric-key objects with lists, allowing argument-binding bypasses, and inconsistently handles object member order, causing valid retries to fail.
2 open issues
What changed in this PR
This PR adds opt-in request-state binding to the PHP SDK’s modern protocol, tying multi-round-trip state to the principal and originating call while preserving existing behavior when no binder is configured.
Changes:
- Adds a binder interface and a recommended principal-and-request implementation.
- Applies binding to state verification, handler-minted state, and elicitation replay.
- Documents configuration and adds codec, binder, and protocol tests.
| File | Description |
|---|---|
| tests/Unit/Server/Stateless/StatelessProtocolTest.php | Tests bound retries, replay rejection, and unbound compatibility. |
| tests/Unit/Server/Stateless/RequestStateCodecTest.php | Tests binding verification and unchanged wire format. |
| tests/Unit/Server/Stateless/OriginatingRequestBinderTest.php | Tests request and principal identity binding. |
| src/Server/Stateless/StatelessProtocol.php | Uses a request-bound codec for verification and minting. |
| src/Server/Stateless/RequestStateCodec.php | Includes optional binding in the state MAC. |
| src/Server/Stateless/RequestStateBinderInterface.php | Defines the custom binding contract. |
| src/Server/Stateless/OriginatingRequestBinder.php | Implements principal, method, target, and argument binding. |
| src/Server/Builder.php | Exposes optional binder configuration. |
| docs/run/server-builder.md | Updates the builder example and option reference. |
| docs/handlers/input-required.md | Explains binding configuration and migration effects. |
| CHANGELOG.md | Records the new binding feature. |
Review effort: Balanced
| if (!array_is_list($value)) { | ||
| ksort($value, \SORT_STRING); | ||
| } | ||
|
|
||
| return array_map(self::canonical(...), $value); |
| if (!array_is_list($value)) { | ||
| ksort($value, \SORT_STRING); |
Summary
Builder::setRequestState($key, $ttl, binder: $binder)takes an optionalRequestStateBinderInterface, which binds everyrequestStatethe modern era mints to the request it was minted for.The binder is called once per request with the
Requestas the client sent it (before anyRequestEventlistener) and its?AccessToken.Its value is covered by the MAC on mint and recomputed on verify, so a state echoed on a request that binds to another value fails with the same
-32602"The supplied requestState failed verification." (HTTP 400) as a forged one.The value never goes on the wire.
OriginatingRequestBinderimplements the binding the specification recommends (MRTR, Server Requirements): the principal (iss,client_id/azp,sub, which survive a token refresh between rounds), the method, and the salient parameters from a per-method allowlist (tool or prompt name plus a SHA-256 digest of the canonicalized arguments, or the resource URI).An allowlist, as in the Python SDK, so a parameter a later revision adds cannot silently change the binding.
RequestStateCodec::withBinding(string)returns a codec with the same key and TTL whose MAC input isbody.'.'.bindinginstead ofbody.The body is base64url and never contains
., so bound and unbound inputs cannot collide.StatelessProtocolbinds the codec once per request and uses it for verifying the echoed state, forRequestContext::mintRequestState()and forElicitationReplay.docs/handlers/input-required.md, and the builder method indocs/run/server-builder.md.Design decisions
Binding by default would reject every state in flight on upgrade and change behaviour for servers that rely on a state verifying across calls.
Shipping
OriginatingRequestBinderkeeps opting in to one line, and keeps servers from hand-rolling a stable arguments digest.Matches the other builder extension points (
SessionLockInterface,NotificationBusInterface); a closure-based binder is a one-class adapter if maintainers prefercallable.The binder runs before
RequestEvent, so a listener rewriting the request cannot make a retry bind differently from its first round.The typed
Requestcarries neitherrequestStatenorinputResponses, which change between rounds; the interface docs warn that_meta(progress token, trace context, capabilities) does too.On the handshake era the
InputRequiredShimmints and verifies within one request and the state never reaches the client, so there is nothing to replay.A state minted without a binder does not verify on a server with one, and the other way round; otherwise an unbound state would stay replayable across calls after the binder is turned on.
Turning a binder on therefore fails the calls in flight for at most one TTL, which the builder docblock and the guide state.
A distinct reason would tell a probing client which part of its replay was wrong; it is also indistinguishable internally, since the binding is part of the MAC.
Alternatives considered
The TypeScript SDK also stores only a keyed tag; MAC-ing the binding directly needs no tag at all.
setRequestStateBinder(): a binder has no effect without the key, so it belongs to the call that configures the key and TTL. The key-ring change to the same method (string or list of keys) composes with the new optional argument.Backward compatibility
RequestStateCodec::__construct()third,StatelessProtocol::__construct()last).Precedents
bind(ctx)callback HMAC'd into a tag:packages/server/src/server/requestStateCodec.tssrc/mcp/server/request_state.py, python-sdk#3032Why in the SDK
A handler that mints its own state can already bind it by hand: put the principal and an arguments digest into
mintRequestState()and compare them on the retry.It cannot do that for the state the SDK mints itself:
ElicitationReplaycarries the answers of a handler that callselicit()more than once, and that state holds only the answers.A client can echo it on another call of the same tool with other arguments, or under another token, and the handler gets answers that were given for a different request.
Binding in the codec covers both kinds of state in one place, so servers do not each hand-roll it.
Motivation: Shopware (https://github.com/shopware/shopware) plans tools that ask more than once per call on the 2026-07-28 era and needs their state bound to the user and the call (shopware/shopware#22081).
Fixes #623
Test plan
RequestStateCodecTest: a bound state verifies under the same binding; is refused (mac) under another; an unbound state is refused by a bound codec and the other way round; the binding stays off the wire, which keeps its two-part shapeOriginatingRequestBinderTest: a retry with another id,_meta, argument order and refreshed token binds to the same value; another tool, arguments, nested argument, subject, client, no token, a prompt of the same name or another method binds elsewhere; a resource read binds to its URIStatelessProtocolTest(end to end throughBuilder): a bound state verifies on a retry of the same call; is refused with-32602/400 on other arguments, another principal or no principal; the binder receives the request and token; a state minted before a binder was configured is refused once one is; without a binder a state still verifies on any call, as beforemake tests(2972 tests, 8 skipped)make phpstanmake cscomposer validate --strictRelation to #626
This PR and #626 (key ring for
requestState) change the samesetRequestState()signature and codec and compose:withBinding()keeps the whole ring, and the binding is covered by the MAC under every key.Whichever merges second needs a mechanical conflict resolution, which I have ready and tested.