mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 03:13:41 +08:00
* feat(webchat): add approval resolve + replay for API-Key channel (ISSUE #413 P1) Before this, a WebChat (API-Key) channel that hit a ToolGuard-protected tool parked the turn in a pending approval the visitor could never clear — it hung for 30 min until the GC timeout and the turn was wasted. This PR closes the loop, mirroring the web ChatController. A1 — no code change. tool_approval_requested already reaches the SDK via ToolExecutionGuardHelper's streamTracker.broadcastObject (direct SSE push, bypassing the StreamDelta path). Adding it to forwardVisitorEvent would double-deliver; the default-drop is correct. A2 — new /sessions/approve and /sessions/deny REST endpoints. Auth is the existing visitorToken + conversationId ownership guard; the actor is webchatUsername(visitorId), which resolves the 'no MateClaw username' blocker noted in the old stopSession javadoc. Both broadcast tool_approval_resolved so the SDK clears its banner in real time. A3 — approve returns an SSE stream: resolveAndConsume (atomic DB + metadata + memory), restoreChatOrigin (recovers the webchat origin captured at createPending), then chatWithReplayStream replays the tool call and continues the turn. Replay may re-trigger approvals, which the existing tool_approval_requested direct push handles. A4 — stopSession now sweeps pending approvals (denyAllByConversation) and broadcasts each resolution, so stopping a stream no longer leaves approvals lingering for the GC. Tests: WebChatApprovalInteractionTest (7) — deny resolves + broadcasts, deny auth/ownership guards, idempotent unknown-pending, stop sweep clears pending, stop no-op when nothing pending. Regression: WebChatStopStreamTest (5), WebChatArchivePinTest (6), WebChatSchemaFieldsTest (5), WebChatWikiPageListTest (8), ApprovalWorkflowServiceResolveTest (13), GcTest (7), RecoveryTest (7). * fix(webchat): IDOR guard + SSE hang fix (PR #415 review) Addresses all review feedback from mateaix: P0 IDOR (security): /sessions/approve and /sessions/deny accepted a client-supplied pendingId without cross-checking it belonged to the caller's conversation. A visitor could resolve / replay another visitor's guarded tool call. Fix: getPending(pendingId) then assert conversationId matches before resolving. Added getPending delegate on ApprovalWorkflowService so the webchat controller (which holds the workflow facade) can do the precise lookup. SSE hang: approveSession's already-resolved / error branches broadcast 'done' before streamTracker.register/attach, so the event had no subscriber and the SSE hung to the 10-min timeout. Fix: register+attach first, then resolveAndConsume. Removed the now-duplicate register/attach in the replay branch. Tests: +2 IDOR cases (cross-visitor pendingId rejected 404; mismatched pendingId rejected 404). denyResolvesPending now asserts via getPending (findPendingByConversation returns the earliest pending, polluted by cross-test map state). denyUnknownPendingIsSafe updated to expect 404 (no longer leaks pendingId existence). Isolated IDOR victim/attacker visitor IDs to avoid cross-test conversationId collisions. Regression: WebChatStopStreamTest (5), WebChatArchivePinTest (6), ApprovalWorkflowServiceResolveTest (13), GcTest (7), RecoveryTest (7). * style(webchat): use simple ChatOrigin name in approveSession (PR #415 review) Reviewer flagged fully-qualified inline types (ResolveOutcome was fixed in the prior commit; ChatOrigin was missed). Add the import and switch the 3 FQN references in approveSession to the simple name, matching the ResolveOutcome cleanup. chatStream's pre-existing FQN usages are out of this PR's scope and left untouched.
This commit is contained in:
倪程伟
GitHub
parent
b478eef78c
commit
2f12c269f4
@ -798,6 +798,17 @@ public class ApprovalWorkflowService implements ApplicationRunner {
|
||||
/**
|
||||
* 代理查询方法
|
||||
*/
|
||||
/**
|
||||
* Look up a pending approval by its exact id. Delegates to the underlying
|
||||
* {@link ApprovalService#getPending} so callers that only hold the workflow
|
||||
* facade (e.g. WebChatController) can fetch the precise record for an IDOR
|
||||
* cross-check without falling back to {@code findPendingByConversation}
|
||||
* (which returns the earliest pending, wrong when several coexist).
|
||||
*/
|
||||
public java.util.Optional<PendingApproval> getPending(String pendingId) {
|
||||
return approvalService.getPending(pendingId);
|
||||
}
|
||||
|
||||
public PendingApproval findPendingByConversation(String conversationId) {
|
||||
return approvalService.findPendingByConversation(conversationId);
|
||||
}
|
||||
|
||||
@ -47,6 +47,10 @@ import java.util.concurrent.ExecutorService;
|
||||
import java.util.concurrent.Executors;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.stream.Collectors;
|
||||
import reactor.core.Disposable;
|
||||
import vip.mate.approval.PendingApproval;
|
||||
import vip.mate.approval.ResolveOutcome;
|
||||
import vip.mate.agent.context.ChatOrigin;
|
||||
|
||||
/**
|
||||
* WebChat 嵌入式对话接口
|
||||
@ -79,6 +83,14 @@ public class WebChatController {
|
||||
private final vip.mate.skill.repository.SkillMapper skillMapper;
|
||||
private final vip.mate.wiki.repository.WikiPageMapper wikiPageMapper;
|
||||
private final vip.mate.wiki.repository.WikiKnowledgeBaseMapper wikiKbMapper;
|
||||
/**
|
||||
* ISSUE #413 P1-A2/A3/A4: drives the approval lifecycle for WebChat
|
||||
* (API-Key) channels. Before this, a tool guarded by ToolGuard would
|
||||
* create a pending approval and park the turn, but the visitor had no
|
||||
* way to resolve it -- the approval hung until the 30-min GC timeout
|
||||
* and the turn was wasted.
|
||||
*/
|
||||
private final vip.mate.approval.ApprovalWorkflowService approvalService;
|
||||
|
||||
/** Visitor-token TTL in seconds (7 days). Mirrors GeneratedFileCache's TTL. */
|
||||
static final long VISITOR_TOKEN_TTL_SECONDS = 7 * 24 * 3600L;
|
||||
@ -1013,9 +1025,12 @@ public class WebChatController {
|
||||
* Disposable 实际中断 Flux;返回 {@code stopped=false} 表示当前没有活跃流
|
||||
* (幂等,不报错)。
|
||||
* <p>
|
||||
* 不做 approval sweep:webchat 渠道目前不暴露 approval UI,且无 MateClaw
|
||||
* username 可传给 {@code denyAllByConversation}。若未来 webchat 接入审批流,
|
||||
* 再单独评估是否补这层。
|
||||
* Approval sweep (ISSUE #413 P1-A4): deny any pending approvals on this
|
||||
* conversation so they do not hang for 30 minutes until the GC timeout.
|
||||
* The visitor username derived from visitorId is the actor -- it resolves
|
||||
* the "no MateClaw username" blocker noted in the old javadoc.
|
||||
* Each denied approval broadcasts a { tool_approval_resolved} SSE
|
||||
* event so the SDK clears its banner immediately.
|
||||
*/
|
||||
@Operation(summary = "停止访客会话线程的进行中流")
|
||||
@PostMapping("/sessions/stop")
|
||||
@ -1049,9 +1064,316 @@ public class WebChatController {
|
||||
conversationId, visitorId, stopped);
|
||||
audit(channel, visitorId, "webchat.stop-session", conversationId,
|
||||
"{\"sessionId\":\"" + sid + "\",\"stopped\":" + stopped + "}");
|
||||
// ISSUE #413 P1-A4: deny pending approvals so they do not linger for
|
||||
// 30 min waiting on a GC timeout. The visitor username is the actor,
|
||||
// mirroring how the web ChatController uses the logged-in username.
|
||||
int deniedCount = 0;
|
||||
try {
|
||||
java.util.List<ResolveOutcome> denied =
|
||||
approvalService.denyAllByConversation(conversationId, webchatUsername(visitorId));
|
||||
deniedCount = denied.size();
|
||||
for (ResolveOutcome o : denied) {
|
||||
try {
|
||||
streamTracker.broadcast(conversationId, "tool_approval_resolved",
|
||||
objectMapper.writeValueAsString(java.util.Map.of(
|
||||
"pendingId", o.pendingId(),
|
||||
"decision", "denied",
|
||||
"toolName", o.toolName() != null ? o.toolName() : "")));
|
||||
} catch (Exception broadcastErr) {
|
||||
log.debug("[WebChat] approval_resolved broadcast failed for {}: {}",
|
||||
o.pendingId(), broadcastErr.getMessage());
|
||||
}
|
||||
}
|
||||
} catch (Exception sweepErr) {
|
||||
log.warn("[WebChat] approval sweep failed for {}: {}", conversationId, sweepErr.getMessage());
|
||||
}
|
||||
if (deniedCount > 0) {
|
||||
log.info("[WebChat] Denied {} pending approval(s) on stop for {}", deniedCount, conversationId);
|
||||
}
|
||||
return R.ok(Map.of("stopped", stopped));
|
||||
}
|
||||
|
||||
/**
|
||||
* 拒绝一个待审批的工具调用 (ISSUE #413 P1-A2)。
|
||||
* <p>
|
||||
* 鉴权同其它会话管理端点 (API Key + visitorToken + 会话归属)。仅允许
|
||||
* 发起对话的访客拒绝自己会话的审批 —— 身份校验通过
|
||||
* {@code webchatUsername(visitorId)} 与 pending.userId 的等价比较
|
||||
* (对位 IM 渠道的 senderId == requester 校验)。
|
||||
* <p>
|
||||
* resolve 后立即广播 {@code tool_approval_resolved} SSE 事件，让 SDK
|
||||
* 实时清理审批 banner。返回同步 JSON (非 SSE)，因为 deny 不需要重放工具。
|
||||
*
|
||||
* @param pendingId the approval pendingId returned in the
|
||||
* {@code tool_approval_requested} event
|
||||
*/
|
||||
@Operation(summary = "拒绝访客会话中的待审批工具调用")
|
||||
@PostMapping("/sessions/deny")
|
||||
public R<Map<String, Object>> denySession(
|
||||
@RequestHeader("X-MC-Key") String apiKey,
|
||||
@RequestHeader(value = "X-MC-Visitor-Token", required = false) String visitorToken,
|
||||
@RequestParam String visitorId,
|
||||
@RequestParam(required = false) String sessionId,
|
||||
@RequestParam String pendingId) {
|
||||
ChannelEntity channel = resolveChannel(apiKey);
|
||||
if (channel == null) {
|
||||
return R.fail(401, "Invalid API Key");
|
||||
}
|
||||
if (!verifyVisitorToken(visitorTokenSecret, channel.getId(), visitorId, visitorToken)) {
|
||||
return R.fail(401, "Invalid or missing visitor token");
|
||||
}
|
||||
String sid;
|
||||
try {
|
||||
sid = normalizeSessionId(sessionId);
|
||||
} catch (IllegalArgumentException ex) {
|
||||
return R.fail(400, ex.getMessage());
|
||||
}
|
||||
String conversationId = deriveConversationId(apiKey, visitorId, sid);
|
||||
if (!ownsConversation(conversationId, visitorId)) {
|
||||
return R.fail(404, "Session not found");
|
||||
}
|
||||
// IDOR guard (review #415): the caller owns the conversation, but the
|
||||
// pendingId is client-supplied — cross-check that the pending actually
|
||||
// belongs to this conversation before resolving, otherwise a visitor
|
||||
// could resolve / replay another visitor's guarded tool call.
|
||||
// getPending(pendingId) gives the exact record (vs findPendingByConversation
|
||||
// which returns the earliest, wrong when several pendings coexist).
|
||||
var ownedOpt = approvalService.getPending(pendingId);
|
||||
if (ownedOpt.isEmpty()
|
||||
|| !conversationId.equals(ownedOpt.get().getConversationId())) {
|
||||
return R.fail(404, "Pending approval not found for this session");
|
||||
}
|
||||
// Resolve and broadcast outside the persistence transaction: SSE is
|
||||
// not rollback-capable, so the broadcast must follow a committed DB write.
|
||||
String actor = webchatUsername(visitorId);
|
||||
ResolveOutcome outcome = approvalService.resolve(pendingId, actor, "denied");
|
||||
broadcastApprovalResolved(conversationId, outcome);
|
||||
audit(channel, visitorId, "webchat.deny-approval", conversationId,
|
||||
"{\"pendingId\":\"" + escapeJson(pendingId) + "\",\"resolved\":"
|
||||
+ outcome.dbSynced() + "}");
|
||||
return R.ok(Map.of("resolved", outcome.dbSynced(), "decision", outcome.decision()));
|
||||
}
|
||||
|
||||
/**
|
||||
* 批准一个待审批的工具调用并重放 (ISSUE #413 P1-A2 + P1-A3)。
|
||||
* <p>
|
||||
* 与 deny 不同，approve 返回 SSE 流：原子消费审批记录后，用捕获的
|
||||
* toolCallPayload 重放工具调用，把工具结果回灌 agent 继续本轮对话。
|
||||
* 重放模式对位 web 渠道的 ChatController —— 复用
|
||||
* {@code chatWithReplayStream} + {@code restoreChatOrigin} 恢复原始
|
||||
* ChatOrigin (webchat origin 在 createPending 时已通过 ChatOriginHolder
|
||||
* 持久化到 approval 行)。
|
||||
* <p>
|
||||
* 重放期间可能再次触发审批 (一个工具批准后 agent 可能调用下一个受保护
|
||||
* 工具) —— 该场景由 {@code tool_approval_requested} 直推事件自然覆盖，
|
||||
* 无需特殊处理。事件投递走与 {@link #chatStream} 相同的 broadcast 路径。
|
||||
*
|
||||
* @param pendingId the approval pendingId returned in the
|
||||
* {@code tool_approval_requested} event
|
||||
*/
|
||||
@Operation(summary = "批准访客会话中的待审批工具调用并重放")
|
||||
@PostMapping(value = "/sessions/approve", produces = MediaType.TEXT_EVENT_STREAM_VALUE)
|
||||
public SseEmitter approveSession(
|
||||
@RequestHeader("X-MC-Key") String apiKey,
|
||||
@RequestHeader(value = "X-MC-Visitor-Token", required = false) String visitorToken,
|
||||
@RequestParam String visitorId,
|
||||
@RequestParam(required = false) String sessionId,
|
||||
@RequestParam String pendingId) {
|
||||
SseEmitter emitter = new Utf8SseEmitter(10 * 60 * 1000L);
|
||||
ChannelEntity channel = resolveChannel(apiKey);
|
||||
if (channel == null) {
|
||||
sendErrorAndComplete(emitter, "Invalid API Key");
|
||||
return emitter;
|
||||
}
|
||||
if (!verifyVisitorToken(visitorTokenSecret, channel.getId(), visitorId, visitorToken)) {
|
||||
sendErrorAndComplete(emitter, "Invalid or missing visitor token");
|
||||
return emitter;
|
||||
}
|
||||
String sid;
|
||||
try {
|
||||
sid = normalizeSessionId(sessionId);
|
||||
} catch (IllegalArgumentException ex) {
|
||||
sendErrorAndComplete(emitter, ex.getMessage());
|
||||
return emitter;
|
||||
}
|
||||
String conversationId = deriveConversationId(apiKey, visitorId, sid);
|
||||
if (!ownsConversation(conversationId, visitorId)) {
|
||||
sendErrorAndComplete(emitter, "Session not found");
|
||||
return emitter;
|
||||
}
|
||||
// IDOR guard (review #415): cross-check the client-supplied pendingId
|
||||
// actually belongs to this conversation before resolving, otherwise a
|
||||
// visitor could approve + replay another visitor's guarded tool call.
|
||||
var ownedApprovalOpt = approvalService.getPending(pendingId);
|
||||
if (ownedApprovalOpt.isEmpty()
|
||||
|| !conversationId.equals(ownedApprovalOpt.get().getConversationId())) {
|
||||
sendErrorAndComplete(emitter, "Pending approval not found for this session");
|
||||
return emitter;
|
||||
}
|
||||
|
||||
emitter.onCompletion(() -> log.debug("[WebChat] approve SSE completed: {}", conversationId));
|
||||
emitter.onTimeout(() -> {
|
||||
log.debug("[WebChat] approve SSE timeout: {}", conversationId);
|
||||
streamTracker.complete(conversationId);
|
||||
});
|
||||
emitter.onError(e -> {
|
||||
log.debug("[WebChat] approve SSE error: {} - {}", conversationId, e.getMessage());
|
||||
streamTracker.complete(conversationId);
|
||||
});
|
||||
|
||||
String actor = webchatUsername(visitorId);
|
||||
sseExecutor.execute(() -> {
|
||||
// Register + attach the emitter FIRST so every downstream branch
|
||||
// (already-resolved, no-agent, error, replay) can broadcast a
|
||||
// terminal event the SDK actually receives. Doing this after
|
||||
// resolveAndConsume left the already-resolved / error paths
|
||||
// broadcasting into a subscriber-less tracker, so the SSE hung
|
||||
// to the 10-min timeout (review #415).
|
||||
streamTracker.register(conversationId);
|
||||
streamTracker.attach(conversationId, emitter);
|
||||
try {
|
||||
// Atomically consume the approval (DB + metadata + memory, single tx).
|
||||
ResolveOutcome consumed = approvalService.resolveAndConsume(pendingId, actor);
|
||||
if (consumed.consumedSnapshot() == null) {
|
||||
// already resolved / not found — emit a terminal done so the
|
||||
// SDK's stream listener closes cleanly instead of hanging.
|
||||
broadcastApprovalResolved(conversationId, consumed);
|
||||
streamTracker.broadcast(conversationId, "done",
|
||||
"{\"status\":\"already_resolved\"}");
|
||||
return;
|
||||
}
|
||||
|
||||
// Notify the SDK the approval flipped (clears the banner) before
|
||||
// replay output starts streaming.
|
||||
broadcastApprovalResolved(conversationId, consumed);
|
||||
|
||||
PendingApproval snapshot = consumed.consumedSnapshot();
|
||||
Long replayAgentId = snapshot.getAgentId() != null
|
||||
? parseLongOrNull(snapshot.getAgentId()) : null;
|
||||
if (replayAgentId == null) {
|
||||
log.warn("[WebChat] approve: no agentId on consumed approval {}, cannot replay",
|
||||
pendingId);
|
||||
streamTracker.broadcast(conversationId, "done",
|
||||
"{\"status\":\"error\",\"message\":\"No agent bound to approval\"}");
|
||||
return;
|
||||
}
|
||||
|
||||
// Restore the original ChatOrigin captured at createPending time.
|
||||
// Falls back to a fresh webchat origin when none was persisted
|
||||
// (defensive — mirrors ChatController:304-306).
|
||||
ChatOrigin replayOrigin =
|
||||
approvalService.restoreChatOrigin(snapshot.getChatOrigin());
|
||||
if (replayOrigin == ChatOrigin.EMPTY) {
|
||||
var agent = agentService.getAgent(replayAgentId);
|
||||
Long wsId = agent != null ? agent.getWorkspaceId() : 1L;
|
||||
replayOrigin = ChatOrigin.web(
|
||||
conversationId, actor, wsId, null).withSender(null, "api", null);
|
||||
}
|
||||
|
||||
// Neutral replay prompt (aligned with IM + web channels — naming a
|
||||
// tool here can mislead the LLM on fallthrough).
|
||||
String replayPrompt = "继续执行已批准的工具调用。";
|
||||
StringBuilder assistantReply = new StringBuilder();
|
||||
final int[] usage = {0, 0};
|
||||
final String[] modelInfo = {null, null};
|
||||
|
||||
streamTracker.broadcast(conversationId, "message_start",
|
||||
"{\"role\":\"assistant\"}");
|
||||
|
||||
Disposable disposable = agentService.chatWithReplayStream(
|
||||
replayAgentId, replayPrompt, conversationId,
|
||||
snapshot.getToolCallPayload(), actor, replayOrigin)
|
||||
.doOnNext(delta -> {
|
||||
if (delta.isEvent() && "_usage_final".equals(delta.eventType())) {
|
||||
Map<String, Object> data = delta.eventData();
|
||||
usage[0] = ((Number) data.getOrDefault("promptTokens", 0)).intValue();
|
||||
usage[1] = ((Number) data.getOrDefault("completionTokens", 0)).intValue();
|
||||
Object model = data.get("runtimeModelName");
|
||||
Object provider = data.get("runtimeProviderId");
|
||||
if (model != null) modelInfo[0] = model.toString();
|
||||
if (provider != null) modelInfo[1] = provider.toString();
|
||||
}
|
||||
if (delta.isEvent()) {
|
||||
forwardVisitorEvent(conversationId, delta.eventType(), delta.eventData());
|
||||
}
|
||||
if (delta.content() != null && !delta.content().isEmpty()) {
|
||||
assistantReply.append(delta.content());
|
||||
if (!delta.persistenceOnly()) {
|
||||
streamTracker.broadcast(conversationId, "content_delta",
|
||||
"{\"text\":" + escapeJson(delta.content()) + "}");
|
||||
}
|
||||
}
|
||||
if (delta.thinking() != null && !delta.thinking().isEmpty()
|
||||
&& !delta.persistenceOnly()) {
|
||||
streamTracker.broadcast(conversationId, "thinking_delta",
|
||||
"{\"text\":" + escapeJson(delta.thinking()) + "}");
|
||||
}
|
||||
})
|
||||
.doOnComplete(() -> {
|
||||
String reply = assistantReply.toString();
|
||||
try {
|
||||
if (!reply.isBlank()) {
|
||||
conversationService.saveMessage(
|
||||
conversationId, "assistant", reply, List.of(),
|
||||
"completed", usage[0], usage[1], modelInfo[0], modelInfo[1]);
|
||||
}
|
||||
} catch (Exception persistErr) {
|
||||
log.warn("[WebChat] approve replay persist failed: {}", persistErr.getMessage());
|
||||
}
|
||||
streamTracker.broadcast(conversationId, "done",
|
||||
"{\"status\":\"completed\"}");
|
||||
streamTracker.complete(conversationId);
|
||||
})
|
||||
.doOnError(e -> {
|
||||
log.error("[WebChat] approve replay stream error: {}", e.getMessage());
|
||||
streamTracker.broadcast(conversationId, "error",
|
||||
"{\"message\":" + escapeJson(e.getMessage()) + "}");
|
||||
streamTracker.complete(conversationId);
|
||||
})
|
||||
.subscribe();
|
||||
streamTracker.setDisposable(conversationId, disposable);
|
||||
} catch (Exception e) {
|
||||
log.error("[WebChat] approve failed for {}: {}", conversationId, e.getMessage());
|
||||
try {
|
||||
streamTracker.broadcast(conversationId, "error",
|
||||
"{\"message\":" + escapeJson(e.getMessage()) + "}");
|
||||
} catch (Exception ignored) {}
|
||||
streamTracker.complete(conversationId);
|
||||
}
|
||||
});
|
||||
audit(channel, visitorId, "webchat.approve-approval", conversationId,
|
||||
"{\"pendingId\":\"" + escapeJson(pendingId) + "\",\"replay\":true}");
|
||||
return emitter;
|
||||
}
|
||||
|
||||
/** Parse a Long leniently; null/blank/non-numeric return null. */
|
||||
private static Long parseLongOrNull(String s) {
|
||||
if (s == null || s.isBlank()) return null;
|
||||
try {
|
||||
return Long.parseLong(s.trim());
|
||||
} catch (NumberFormatException e) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Broadcast a {@code tool_approval_resolved} event so the SDK clears its
|
||||
* approval banner in real time. Shared by approve / deny / stop-sweep.
|
||||
* (ISSUE #413 P1)
|
||||
*/
|
||||
private void broadcastApprovalResolved(String conversationId, ResolveOutcome outcome) {
|
||||
try {
|
||||
streamTracker.broadcast(conversationId, "tool_approval_resolved",
|
||||
objectMapper.writeValueAsString(Map.of(
|
||||
"pendingId", outcome.pendingId(),
|
||||
"decision", outcome.decision() != null ? outcome.decision() : "",
|
||||
"toolName", outcome.toolName() != null ? outcome.toolName() : "")));
|
||||
} catch (Exception e) {
|
||||
log.debug("[WebChat] approval_resolved broadcast failed for {}: {}",
|
||||
outcome.pendingId(), e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 重新生成最后一条助手回复。
|
||||
* <p>
|
||||
|
||||
@ -0,0 +1,223 @@
|
||||
package vip.mate.channel.webchat;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.test.context.SpringBootTest;
|
||||
import org.springframework.jdbc.core.JdbcTemplate;
|
||||
import org.springframework.test.context.TestPropertySource;
|
||||
import vip.mate.MateClawApplication;
|
||||
import vip.mate.approval.ApprovalWorkflowService;
|
||||
import vip.mate.approval.PendingApproval;
|
||||
import vip.mate.channel.web.ChatStreamTracker;
|
||||
import vip.mate.channel.webchat.WebChatController.WebChatCreateSessionRequest;
|
||||
import vip.mate.common.result.R;
|
||||
|
||||
import java.util.Map;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
/**
|
||||
* Verifies ISSUE #413 P1: the WebChat (API-Key) channel can now resolve tool
|
||||
* approvals. Before the fix a ToolGuard-protected tool would park the turn in
|
||||
* a pending approval the visitor could never clear — it hung for 30 min until
|
||||
* the GC timeout and the turn was wasted.
|
||||
*
|
||||
* <p>Covers the synchronous paths (deny + stop-sweep). The approve path drives
|
||||
* a live agent replay stream and is exercised separately; the auth + ownership
|
||||
* guards it shares with deny are validated here.
|
||||
*/
|
||||
@SpringBootTest(
|
||||
classes = MateClawApplication.class,
|
||||
webEnvironment = SpringBootTest.WebEnvironment.NONE
|
||||
)
|
||||
@TestPropertySource(properties = {
|
||||
"spring.datasource.url=jdbc:h2:mem:webchat_approve_${random.uuid};MODE=MySQL;DATABASE_TO_LOWER=TRUE;CASE_INSENSITIVE_IDENTIFIERS=TRUE;DB_CLOSE_DELAY=-1",
|
||||
"spring.ai.dashscope.api-key=test-key",
|
||||
"spring.main.web-application-type=none",
|
||||
"mateclaw.jwt.secret=webchat-it-secret-0123456789"
|
||||
})
|
||||
class WebChatApprovalInteractionTest {
|
||||
|
||||
private static final String SECRET = "webchat-it-secret-0123456789";
|
||||
private static final String API_KEY = "testkey1abcdefgh"; // key8 = "testkey1"
|
||||
private static final long CHANNEL_ID = 9_147_310L;
|
||||
private static final long AGENT_ID = 9_147_3101L;
|
||||
|
||||
@Autowired private WebChatController controller;
|
||||
@Autowired private ApprovalWorkflowService approvalService;
|
||||
@Autowired private ChatStreamTracker streamTracker;
|
||||
@Autowired private JdbcTemplate jdbc;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
jdbc.update("DELETE FROM mate_channel WHERE id = ?", CHANNEL_ID);
|
||||
jdbc.update("DELETE FROM mate_agent WHERE id = ?", AGENT_ID);
|
||||
jdbc.update(
|
||||
"MERGE INTO mate_agent (id, name, agent_type, system_prompt, max_iterations, enabled, " +
|
||||
"workspace_id, create_time, update_time, deleted) " +
|
||||
"KEY(id) VALUES (?, 'wc-approve-agent', 'react', '', 10, TRUE, 1, " +
|
||||
"CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 0)",
|
||||
AGENT_ID);
|
||||
jdbc.update("INSERT INTO mate_channel (id, name, channel_type, agent_id, config_json, enabled, " +
|
||||
"workspace_id, create_time, update_time, deleted) " +
|
||||
"VALUES (?, 'wc', 'webchat', ?, ?, TRUE, 1, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 0)",
|
||||
CHANNEL_ID, AGENT_ID, "{\"api_key\":\"" + API_KEY + "\"}");
|
||||
}
|
||||
|
||||
private WebChatCreateSessionRequest req(String visitorId, String sessionId) {
|
||||
WebChatCreateSessionRequest r = new WebChatCreateSessionRequest();
|
||||
r.setVisitorId(visitorId);
|
||||
r.setSessionId(sessionId);
|
||||
return r;
|
||||
}
|
||||
|
||||
private String tokenFor(String visitorId) {
|
||||
return WebChatController.computeVisitorToken(SECRET, CHANNEL_ID, visitorId);
|
||||
}
|
||||
|
||||
private String seedPending(String visitorId, String sessionId) {
|
||||
controller.createSession(API_KEY, req(visitorId, sessionId));
|
||||
String cid = WebChatController.deriveConversationId(API_KEY, visitorId, sessionId);
|
||||
// The actor stored on the approval is the webchat username, mirroring
|
||||
// how chatStream sets it via webchatUsername(visitorId).
|
||||
String actor = "webchat:" + API_KEY.substring(0, 8) + ":" + visitorId;
|
||||
return approvalService.createPending(
|
||||
cid, actor, "write_file", "{}", "high-severity edit",
|
||||
"{}", "[]", String.valueOf(AGENT_ID));
|
||||
}
|
||||
|
||||
// ---------------- deny ----------------
|
||||
|
||||
@Test
|
||||
@DisplayName("deny resolves a pending approval and broadcasts tool_approval_resolved")
|
||||
void denyResolvesPending() {
|
||||
String pendingId = seedPending("visitorA", "s1");
|
||||
String cid = WebChatController.deriveConversationId(API_KEY, "visitorA", "s1");
|
||||
|
||||
// Register the stream so the broadcast has a live subscriber state.
|
||||
streamTracker.register(cid);
|
||||
|
||||
R<Map<String, Object>> r = controller.denySession(
|
||||
API_KEY, tokenFor("visitorA"), "visitorA", "s1", pendingId);
|
||||
|
||||
assertThat(r.getCode()).isEqualTo(200);
|
||||
assertThat(r.getData().get("resolved")).isEqualTo(Boolean.TRUE);
|
||||
assertThat(r.getData().get("decision")).isEqualTo("denied");
|
||||
|
||||
// The approval is no longer pending. Query by the exact pendingId (not
|
||||
// findPendingByConversation, which returns the earliest pending and
|
||||
// would be polluted by cross-test map state when several pendings
|
||||
// coexist for the same conversation).
|
||||
var after = approvalService.getPending(pendingId);
|
||||
assertThat(after.isEmpty() || !"pending".equals(after.get().getStatus()))
|
||||
.as("approval should be resolved, not pending").isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("deny rejects a bad visitor token → 401")
|
||||
void denyRejectsBadToken() {
|
||||
String pendingId = seedPending("visitorB", "s1");
|
||||
R<Map<String, Object>> r = controller.denySession(
|
||||
API_KEY, "bogus-token", "visitorB", "s1", pendingId);
|
||||
assertThat(r.getCode()).isEqualTo(401);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("deny rejects an unknown session → 404 (no namespace probing)")
|
||||
void denyRejectsUnknownSession() {
|
||||
String pendingId = seedPending("visitorC", "s1");
|
||||
R<Map<String, Object>> r = controller.denySession(
|
||||
API_KEY, tokenFor("visitorC"), "visitorC", "never-created", pendingId);
|
||||
assertThat(r.getCode()).isEqualTo(404);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("deny on a bad API Key → 401")
|
||||
void denyRejectsBadApiKey() {
|
||||
R<Map<String, Object>> r = controller.denySession(
|
||||
"bogus-key", "any-token", "visitorD", "s1", "any-pending");
|
||||
assertThat(r.getCode()).isEqualTo(401);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("deny of an unknown pendingId returns 404 (does not leak existence)")
|
||||
void denyUnknownPendingIsSafe() {
|
||||
controller.createSession(API_KEY, req("visitorE", "s1"));
|
||||
R<Map<String, Object>> r = controller.denySession(
|
||||
API_KEY, tokenFor("visitorE"), "visitorE", "s1", "wf-ghostthatdoesnotexist");
|
||||
// After the IDOR guard (review #415) an unknown / mismatched pendingId
|
||||
// is rejected with 404 rather than an idempotent 200 — this also avoids
|
||||
// leaking whether a given pendingId exists.
|
||||
assertThat(r.getCode()).isEqualTo(404);
|
||||
}
|
||||
|
||||
// ---------------- IDOR guard (review #415) ----------------
|
||||
|
||||
@Test
|
||||
@DisplayName("deny rejects a pendingId belonging to ANOTHER visitor's session → 404")
|
||||
void denyRejectsCrossVisitorPendingId() {
|
||||
// Victim owns session victimX and its pending approval.
|
||||
String pendingIdVictim = seedPending("victimX", "s1");
|
||||
// Attacker also has a valid token + own session (ownsConversation passes).
|
||||
controller.createSession(API_KEY, req("attackerY", "s1"));
|
||||
|
||||
// Attacker tries to deny the victim's pendingId while authenticated as
|
||||
// the attacker against the attacker's own session. Before the IDOR fix
|
||||
// this would resolve the victim's approval — a cross-visitor privilege
|
||||
// escalation. Now the pendingId↔conversationId cross-check returns 404.
|
||||
R<Map<String, Object>> r = controller.denySession(
|
||||
API_KEY, tokenFor("attackerY"), "attackerY", "s1", pendingIdVictim);
|
||||
|
||||
assertThat(r.getCode()).isEqualTo(404);
|
||||
|
||||
// The victim's approval is untouched.
|
||||
String cidVictim = WebChatController.deriveConversationId(API_KEY, "victimX", "s1");
|
||||
var stillPending = approvalService.getPending(pendingIdVictim);
|
||||
assertThat(stillPending).as("victim's approval must not be resolved by attacker").isPresent();
|
||||
assertThat(stillPending.get().getStatus()).isEqualTo("pending");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("deny rejects a pendingId that does not belong to the caller's session → 404")
|
||||
void denyRejectsMismatchedPendingId() {
|
||||
// Visitor owns the session, but passes a pendingId that doesn't match
|
||||
// the session's pending (e.g. a stale/guessed id).
|
||||
seedPending("visitorH", "s1");
|
||||
R<Map<String, Object>> r = controller.denySession(
|
||||
API_KEY, tokenFor("visitorH"), "visitorH", "s1", "wf-not-yours-12345");
|
||||
assertThat(r.getCode()).isEqualTo(404);
|
||||
}
|
||||
|
||||
// ---------------- stop sweep (A4) ----------------
|
||||
|
||||
@Test
|
||||
@DisplayName("stop denies pending approvals on the conversation (approval sweep)")
|
||||
void stopSweepsPendingApprovals() {
|
||||
seedPending("visitorF", "s1");
|
||||
String cid = WebChatController.deriveConversationId(API_KEY, "visitorF", "s1");
|
||||
|
||||
// A pending approval exists before stop.
|
||||
assertThat(approvalService.findPendingByConversation(cid)).isNotNull();
|
||||
|
||||
R<Map<String, Object>> r = controller.stopSession(
|
||||
API_KEY, tokenFor("visitorF"), "visitorF", "s1");
|
||||
|
||||
assertThat(r.getCode()).isEqualTo(200);
|
||||
// After the sweep the approval is gone from the pending map.
|
||||
PendingApproval after = approvalService.findPendingByConversation(cid);
|
||||
assertThat(after == null || !"pending".equals(after.getStatus()))
|
||||
.as("stop should have denied the pending approval").isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("stop with no pending approvals is unaffected (sweep is a no-op)")
|
||||
void stopNoPendingStillWorks() {
|
||||
controller.createSession(API_KEY, req("visitorG", "s1"));
|
||||
R<Map<String, Object>> r = controller.stopSession(
|
||||
API_KEY, tokenFor("visitorG"), "visitorG", "s1");
|
||||
assertThat(r.getCode()).isEqualTo(200);
|
||||
assertThat(r.getData().get("stopped")).isEqualTo(Boolean.FALSE);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user