Skip to content

fix(mcp): #459 身份透传功能 3 轮对抗式审查（修复 22 项发现） #540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ncw1992120 wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

fix(mcp): #459 身份透传功能 3 轮对抗式审查（修复 22 项发现） #540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
469ffe2
fix(mcp): round-1 adversarial review of #459 identity forwarding
ncw1992120 Jul 17, 2026
db873f2
fix(mcp): round-2 adversarial review of #459 identity forwarding
ncw1992120 Jul 17, 2026
25aa27b
fix(mcp): round-3 adversarial review of #459 — deployment + hardening
ncw1992120 Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ TAVILY_API_KEY=
# 生产部署必须设置，至少 32 位随机字符串：openssl rand -base64 48
JWT_SECRET=

# MCP on-behalf-of 身份透传（STDIO MCP server 代表登录用户调 REST 后端）的
# RS256 签名私钥（PKCS#8 PEM，明文、不带 passphrase）。仅当你在
# application.yml 里把 mateclaw.mcp.identity-forward.servers 配了允许清单、
# 且 token.enabled=true 时才需要。留空 ⇒ token 模式 fail-closed（不签发、不注入）。
# 生成：openssl genpkey -algorithm RSA -pkcs8 -out mcp-idfwd-private.pem
# 然后把整段 PEM（含 -----BEGIN/END-----）作为该变量的值。
# 公钥配到 REST 后端用于验签。详见 docs/{en,zh}/mcp.md "on-behalf-of" 章节。
MCP_IDFWD_PRIVATE_KEY_PEM=

# CORS 白名单（逗号分隔，如 https://mateclaw.example.com,https://admin.example.com）。
# 若留空，服务器会允许所有 origin 并在启动日志里 WARN。生产部署务必设置。
MATECLAW_CORS_ALLOWED_ORIGINS=
Expand Down
4 changes: 4 additions & 0 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ services:
SERPER_API_KEY: ${SERPER_API_KEY:-}
TAVILY_API_KEY: ${TAVILY_API_KEY:-}
JWT_SECRET: ${JWT_SECRET:-}
# MCP on-behalf-of identity forwarding (RS256 signing key, PKCS#8 PEM).
# Only needed when mateclaw.mcp.identity-forward has opted-in servers and
# token.enabled=true. Empty by default ⇒ token mode fails closed (safe).
MCP_IDFWD_PRIVATE_KEY_PEM: ${MCP_IDFWD_PRIVATE_KEY_PEM:-}
MATECLAW_CORS_ALLOWED_ORIGINS: ${MATECLAW_CORS_ALLOWED_ORIGINS:-}
# SearXNG: tell the app where to reach the sidecar container
SEARXNG_BASE_URL: ${SEARXNG_BASE_URL:-http://searxng:8080}
Expand Down
31 changes: 18 additions & 13 deletions mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ public SseEmitter chatStream(
// deny 是正常 turn 终结，用户可能在 awaiting_approval 阶段排了消息
ChatStreamTracker.CompletionResult denyCr = streamTracker.completeAndConsumeIfLast(conversationId);
if (denyCr.allDone() && denyCr.queuedInput() != null) {
startQueuedMessage(conversationId, emitter, approvalEmitterDone, denyCr.queuedInput(), username, requestBaseUrl);
startQueuedMessage(conversationId, emitter, approvalEmitterDone, denyCr.queuedInput(), username, requesterUserIdOf(auth), requestBaseUrl);
} else {
completeEmitterQuietly(emitter, approvalEmitterDone);
}
Expand All @@ -287,7 +287,7 @@ public SseEmitter chatStream(
// 审批记录被另一个请求消费，但用户可能在等待期间排了消息
ChatStreamTracker.CompletionResult consumedNullCr = streamTracker.completeAndConsumeIfLast(conversationId);
if (consumedNullCr.allDone() && consumedNullCr.queuedInput() != null) {
startQueuedMessage(conversationId, emitter, approvalEmitterDone, consumedNullCr.queuedInput(), username, requestBaseUrl);
startQueuedMessage(conversationId, emitter, approvalEmitterDone, consumedNullCr.queuedInput(), username, requesterUserIdOf(auth), requestBaseUrl);
} else {
completeEmitterQuietly(emitter, approvalEmitterDone);
}
Expand All @@ -310,8 +310,11 @@ public SseEmitter chatStream(
vip.mate.agent.context.ChatOrigin replayOrigin =
approvalService.restoreChatOrigin(finalConsumed.getChatOrigin());
if (replayOrigin == vip.mate.agent.context.ChatOrigin.EMPTY) {
// Carry the immutable requesterUserId so on-behalf-of
// identity forwarding (issue #459) asserts the real
// account on replayed tool calls, not the username.
replayOrigin = vip.mate.agent.context.ChatOrigin.web(
conversationId, username, workspaceId, null);
conversationId, username, workspaceId, null, null, requesterUserIdOf(auth));
}
// Carry the request-thread base URL so any file a replayed
// tool generates gets an absolute download link.
Expand Down Expand Up @@ -392,7 +395,7 @@ public SseEmitter chatStream(
ChatStreamTracker.CompletionResult cr = streamTracker.completeAndConsumeIfLast(conversationId);
if (cr.allDone()) {
if (cr.queuedInput() != null) {
startQueuedMessage(conversationId, emitter, approvalEmitterDone, cr.queuedInput(), username, requestBaseUrl);
startQueuedMessage(conversationId, emitter, approvalEmitterDone, cr.queuedInput(), username, requesterUserIdOf(auth), requestBaseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
completeEmitterQuietly(emitter, approvalEmitterDone);
Expand Down Expand Up @@ -493,7 +496,7 @@ public SseEmitter chatStream(
ChatStreamTracker.CompletionResult cr = streamTracker.completeAndConsumeIfLast(conversationId);
if (cr.allDone()) {
if (cr.queuedInput() != null) {
startQueuedMessage(conversationId, emitter, approvalEmitterDone, cr.queuedInput(), username, requestBaseUrl);
startQueuedMessage(conversationId, emitter, approvalEmitterDone, cr.queuedInput(), username, requesterUserIdOf(auth), requestBaseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
completeEmitterQuietly(emitter, approvalEmitterDone);
Expand Down Expand Up @@ -708,7 +711,7 @@ public SseEmitter chatStream(
// genuinely doesn't want continuation, no message would
// have been in messageQueue to begin with.
if (cr.queuedInput() != null) {
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), username, requestBaseUrl);
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), username, requesterUserIdOf(auth), requestBaseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
// 延迟关闭 emitter，确保最后的事件都已发送
Expand Down Expand Up @@ -802,7 +805,7 @@ public SseEmitter chatStream(
if (cr.allDone()) {
if (cr.queuedInput() != null) {
// 无论中断类型，都消费排队消息（修复 Disposable 不可用时队列被丢弃的 bug）
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), username, requestBaseUrl);
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), username, requesterUserIdOf(auth), requestBaseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
completeEmitterQuietly(emitter, emitterDone);
Expand Down Expand Up @@ -925,7 +928,7 @@ public SseEmitter chatStream(
// — just run it. Aligns with doOnComplete and the 4 other
// queue-launch sites in this controller.
if (cr.queuedInput() != null) {
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), username, requestBaseUrl);
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), username, requesterUserIdOf(auth), requestBaseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
completeEmitterQuietly(emitter, emitterDone);
Expand Down Expand Up @@ -1349,7 +1352,7 @@ public static class ChatStreamRequest {
*/
private void startQueuedMessage(String conversationId, SseEmitter emitter, AtomicBoolean emitterDone,
ChatStreamTracker.QueuedInput preConsumedInput, String requesterId,
String baseUrl) {
Long requesterUserId, String baseUrl) {
if (preConsumedInput == null) {
conversationService.updateStreamStatus(conversationId, "idle");
completeEmitterQuietly(emitter, emitterDone);
Expand Down Expand Up @@ -1408,9 +1411,11 @@ private void startQueuedMessage(String conversationId, SseEmitter emitter, Atomi
streamTracker.incrementFlux(conversationId);
// RFC-063r §2.5: queued messages land in the same conversation; carry
// a web-origin ChatOrigin so any cron job created during the queued
// turn keeps a consistent (null-channel) binding.
// turn keeps a consistent (null-channel) binding. Carry the immutable
// requesterUserId so on-behalf-of identity forwarding (issue #459) still
// asserts the authenticated account, not just the mutable username.
vip.mate.agent.context.ChatOrigin queuedOrigin =
vip.mate.agent.context.ChatOrigin.web(conversationId, requesterId, null, null)
vip.mate.agent.context.ChatOrigin.web(conversationId, requesterId, null, null, null, requesterUserId)
.withBaseUrl(baseUrl);
Disposable disposable = agentService.chatStructuredStream(agentId, queuedMessage, conversationId, requesterId, null, queuedOrigin)
.doOnNext(delta -> {
Expand Down Expand Up @@ -1474,7 +1479,7 @@ private void startQueuedMessage(String conversationId, SseEmitter emitter, Atomi
if (cr.allDone()) {
if (cr.queuedInput() != null) {
// 链式续跑：queued stream 期间又排了新消息
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), requesterId, baseUrl);
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), requesterId, requesterUserId, baseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
sseExecutor.execute(() -> {
Expand Down Expand Up @@ -1519,7 +1524,7 @@ private void startQueuedMessage(String conversationId, SseEmitter emitter, Atomi
ChatStreamTracker.CompletionResult cr = streamTracker.completeAndConsumeIfLast(conversationId);
if (cr.allDone()) {
if (cr.queuedInput() != null) {
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), requesterId, baseUrl);
startQueuedMessage(conversationId, emitter, emitterDone, cr.queuedInput(), requesterId, requesterUserId, baseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
completeEmitterQuietly(emitter, emitterDone);
Expand Down
19 changes: 18 additions & 1 deletion mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/IdentityForwardingToolCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,25 @@ static String withClaim(String toolInput, String key, String value) {
node.put(key, value);
return MAPPER.writeValueAsString(node);
} catch (Exception e) {
log.warn("[McpIdentity] failed to inject identity into tool input: {}", e.getMessage());
// Sanitize: exception messages may echo fragments of the (model/user
// controlled) toolInput, and Jackson errors embed a source snippet —
// strip CR/LF so a crafted input can't forge extra log lines under
// naive log shippers, and cap the length.
log.warn("[McpIdentity] failed to inject identity into tool input: {}",
sanitize(e.getMessage()));
return toolInput;
}
}

/**
* Make an exception message safe to interpolate into a log line: replace
* CR/LF (and other ASCII control chars) with a visible glyph and cap the
* length, so a crafted toolInput echoed inside a Jackson/parse error can't
* forge additional log records (log-injection hardening).
*/
static String sanitize(String msg) {
if (msg == null) return "";
String cleaned = msg.replaceAll("[\\r\\n\\t\\p{Cntrl}]", "?");
return cleaned.length() > 200 ? cleaned.substring(0, 200) + "…" : cleaned;
}
}
12 changes: 12 additions & 0 deletions mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/McpIdentityForwardProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,20 @@ public boolean forwardsTo(Long serverId, String serverName) {
* Audience claim for a server's minted tokens: an explicit mapping (by name
* or id) when configured, otherwise the server name (or id as string). Lets
* the backend reject a token minted for a different server.
*
* @throws IllegalArgumentException when both {@code serverId} and
* {@code serverName} are {@code null} — there is no defensible
* audience for an unidentified server, and silently returning the
* literal string {@code "null"} would let two distinct null/null
* servers share one audience (defeating per-server isolation).
* Callers reachable from the production wrap path always have at
* least one of the two.
*/
public String audienceFor(Long serverId, String serverName) {
if (serverId == null && (serverName == null || serverName.isBlank())) {
throw new IllegalArgumentException(
"audienceFor requires a serverId or serverName; both were null/blank");
}
Map<String, String> aud = token.getAudiences();
if (serverName != null && aud.containsKey(serverName)) {
return aud.get(serverName);
Expand Down
Loading