docs(mcp): identity typing semantics + #541 migration note (#459 review follow-up) - #556
Open
ncw1992120 wants to merge 5 commits into
Open
docs(mcp): identity typing semantics + #541 migration note (#459 review follow-up)#556ncw1992120 wants to merge 5 commits into
ncw1992120 wants to merge 5 commits into
Conversation
…follow-up) Two follow-up features for the MCP identity-forward (mateaix#459): ## Feature 1: JWKS endpoint for public key distribution REST backends can now verify identity tokens by fetching the RSA public key from a well-known JWKS URL instead of out-of-band PEM config. - New endpoint: GET /api/v1/mcp/.well-known/jwks.json (public, no auth) - McpIdentityForwardService.publicKeyJwks() derives the public key from the parsed RSAPrivateCrtKey (modulus + public exponent) — no separate publicKeyPem config needed. - Returns standard JWKS: {"keys":[{"kty":"RSA","use":"sig","alg":"RS256", "kid":"...","n":"base64url","e":"base64url"}]} - SecurityConfig: added the endpoint to permitAll (public key is public). ## Feature 2: Identity injection moved from tool args to MCP _meta Identity (plaintext + token) now travels in the MCP _meta field (the caller-controlled channel on tools/call) instead of the tool arguments JSON (the model-controlled channel). This is semantically cleaner and removes spoof risk entirely — the model cannot author _meta. ### Breaking change for MCP server implementers - OLD: read identity from args `__mateclaw_user__` / `__mateclaw_token__` - NEW: read identity from `_meta` `mateclaw_user` / `mateclaw_token` (via the MCP Context.meta in FastMCP / SDK equivalents) ### Implementation - McpIdentityForwardService: new resolveMeta() returns Map<String,String> for _meta; removed old resolve()/Injection (args-based API). - IdentityForwardingToolCallback: simplified to a type marker + identity provider; removed args JSON merge (inject/withClaim); new metaInjection() returns the _meta key-value map. - ProgressAwareMcpToolCallback: direct callTool path now triggers on progress token OR identity forwarding (both need _meta). Merges progress + identity into one _meta map. Args are model-verbatim (identity is NOT in args anymore). - McpIdentityForwardProperties: new META_USER_KEY/META_TOKEN_KEY constants; old USER_ARG/TOKEN_ARG kept @deprecated for migration reference. ## Tests - McpIdentityForwardServiceTest: JWKS verification round-trip (mint token → derive public key from JWKS → verify), JWKS empty when token disabled/bad. - IdentityForwardingToolCallbackTest: rewritten for _meta API. - ProgressAwareMcpToolCallbackTest: new _meta identity injection tests with ArgumentCaptor<CallToolRequest> asserting _meta contents; progress + identity merge; fallback drops identity (logged). - 145 MCP + context tests pass. ## Docs - Updated zh/en mcp.md: _meta migration, Python Context.meta examples, JWKS endpoint + PyJWKClient verification example.
- CRITICAL: RFC 7518 violation in JWK n/e encoding — BigInteger.toByteArray() prepends a 0x00 sign byte for positive numbers with the high bit set (every RSA modulus), producing a 256× larger modulus to strict JWT parsers (PyJWT, java-jwt). Fixed base64UrlEncode() to strip the leading zero. Test hardened to assert byte length = ceil(bitLength/8) and modulus equality without the BigInteger(1,...) signum crutch. - MEDIUM: signingKey() re-logged ERROR on every call when PEM was blank/null — lastAttemptedPem was set to null, so the fast-path guard (lastAttemptedPem != null) never short-circuited. Added a parseAttempted boolean to dedup regardless of PEM nullability. - MEDIUM: JWKS endpoint lacked Cache-Control header. Added Cache-Control: public, max-age=300 + documented kid-on-rotation requirement. - LOW: non-CRT signing key silently returned empty JWKS with no log. Added a warning when the key is non-null but not RSAPrivateCrtKey.
…teaix#541 R2) Round-2 review found the round-1 parseAttempted fix (preventing repeated ERROR logging on blank/null PEM) had no regression test. Added a ListAppender-based test that calls resolveMeta 3× with a blank PEM and asserts the "private-key-pem is empty" ERROR fires exactly once, not per call. Guards against a refactor removing the dedup thinking it's redundant.
Round-3 review found the docs' Python examples used a non-existent FastMCP API: `ctx.meta` doesn't exist on the official mcp SDK's Context class, and the meta value is a Pydantic model (not a dict), so `.get()` crashes with AttributeError. Every server implementer copying the example would hit a runtime crash on the first identity-forwarded call. Fixed all 4 code blocks (plaintext + token, en + zh) to use the correct pattern: `ctx.request_context.meta` + `getattr(raw_meta, "mateclaw_user", None)`. Verified: wrapping chain, instanceof detection, audience freshness, no-context overload safety, SecurityConfig ordering, RFC 7518 encoding — all correct per round-3 review.
…teaix#459 review follow-up) 收尾 ISSUE mateaix#459 Review(webchat/api 渠道 requesterId 身份语义错配)的剩余两项:文档明确(建议③)与前缀对齐文档化(问题②)。堆叠在 mateaix#541 分支之上。 文档(zh/en mcp.md 镜像): - 身份从"认证用户名"重构为带类型的调用方身份:<trust>:<subject> (authenticated / anonymous / external 三级语义表 + fail-closed 清单) - JWT claims 补 trust / channel_type;重写 sub 说明,写清"验签通过 ≠ 认证用户,必须检查 trust claim" - 示例更新:明文按 trust 前缀分级;验签按 trust claim 降级/拒绝 - 新增「迁移说明」:mateaix#541 硬切换(args → _meta,无兼容期)+ FastMCP 正确读取模式(ctx.request_context.meta + getattr) - 新增「身份命名空间」:四套访客标识格式对照(记忆 api:<vid> / 会话 webchat:<vid> / 审计 webchat:<chanId>:<vid> / MCP <trust>:<vid>)+ 剥前缀 join 关联规则 代码注释(防格式增殖):MemoryOwnerResolver / webchatUsername / classify() 各加交叉引用,指向文档「身份命名空间」节。 验证:McpIdentityForwardServiceTest + IdentityForwardingToolCallbackTest + ProgressAwareMcpToolCallbackTest 共 41 例全绿;纯文档+注释,无行为变更。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
收尾 ISSUE #459 Review(webchat / api 渠道下
requesterId的身份语义错配)的剩余两项:classify())与 JWTtrust/channel_typeclaims 已在 feat(mcp): JWKS 端点 + 身份注入迁移到 _meta(#459 follow-up) #541 实现,但 zh/en 文档仍停在"注入明文用户名"的旧描述。api:<vid>)、会话(webchat:<vid>)、审计(webchat:<chanId>:<vid>)、MCP 透传(<trust>:<vid>)四处标识不同。经爆炸半径调查(会话用户名长 visitorId 已不可逆哈希、ownerKey 三表带唯一索引、MCP 前缀是对外契约),决策走文档化对齐,不动持久化数据。acad25bf)。#541 合入前,diff 包含 #541 全部内容;请只 review 顶部 commit。#541 合入 dev 后,本 PR diff 会自动收窄为这 5 个文件(+178/−32)。改动(顶部 commit)
文档(
mateclaw-server/src/main/resources/docs/{zh,en}/mcp.md镜像):<trust>:<subject>:三级语义表(authenticated/anonymous/external的来源、subject、后端对待方式)+ fail-closed 清单trust/channel_type;重写陈旧的sub说明,写清 Review 要求的告诫:验签通过 ≠ 认证用户,必须检查trustclaim(否则 webchat 访客会获得不该有的信任级)trustclaim 降级/拒绝args["__mateclaw_user__"]→_meta["mateclaw_user"],args 不再携带);FastMCP 正确读取模式ctx.request_context.meta+getattr(警告 R3 前错误写法ctx.meta.get()会 AttributeError);server 迁移 checklist代码注释(防格式增殖,各一条交叉引用):
MemoryOwnerResolver类 javadoc、WebChatController.webchatUsername()、McpIdentityForwardService.classify()(注明 trust 前缀是远端后端消费的外部契约)。#541 发送侧审计结论(随 PR 说明)
针对"#541 是否改坏了身份透传"做了全链审计,结论:MateClaw 发送侧完好——包装链
PrefixedName→ProgressAware→Identity→SDK完整、ToolContext 逐层转发、SDK 0.16.0CallToolRequest.meta带@JsonProperty("_meta")、测试以 ArgumentCaptor 断言新契约。收不到身份的 server 侧原因几乎必然是:仍读 args(旧契约已停写)或按 R3 前错误文档实现(ctx.meta)——本 PR 的「迁移说明」小节正对此而写。验证
McpIdentityForwardServiceTest+IdentityForwardingToolCallbackTest+ProgressAwareMcpToolCallbackTest共 41 例全绿Follow-up(不在本 PR)
executePreApproved审批回放丢身份(plan 模式,ToolExecutionExecutor:~700)AgentController、DefaultAgentInvoker)