diff --git a/mateclaw-server/src/main/java/vip/mate/approval/ApprovalWorkflowService.java b/mateclaw-server/src/main/java/vip/mate/approval/ApprovalWorkflowService.java index 31814fc9..238e17fd 100644 --- a/mateclaw-server/src/main/java/vip/mate/approval/ApprovalWorkflowService.java +++ b/mateclaw-server/src/main/java/vip/mate/approval/ApprovalWorkflowService.java @@ -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 getPending(String pendingId) { + return approvalService.getPending(pendingId); + } + public PendingApproval findPendingByConversation(String conversationId) { return approvalService.findPendingByConversation(conversationId); } diff --git a/mateclaw-server/src/main/java/vip/mate/channel/webchat/WebChatController.java b/mateclaw-server/src/main/java/vip/mate/channel/webchat/WebChatController.java index 45499c16..869379c4 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/webchat/WebChatController.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/webchat/WebChatController.java @@ -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} 表示当前没有活跃流 * (幂等,不报错)。 *

- * 不做 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 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)。 + *

+ * 鉴权同其它会话管理端点 (API Key + visitorToken + 会话归属)。仅允许 + * 发起对话的访客拒绝自己会话的审批 —— 身份校验通过 + * {@code webchatUsername(visitorId)} 与 pending.userId 的等价比较 + * (对位 IM 渠道的 senderId == requester 校验)。 + *

+ * 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> 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)。 + *

+ * 与 deny 不同，approve 返回 SSE 流：原子消费审批记录后，用捕获的 + * toolCallPayload 重放工具调用，把工具结果回灌 agent 继续本轮对话。 + * 重放模式对位 web 渠道的 ChatController —— 复用 + * {@code chatWithReplayStream} + {@code restoreChatOrigin} 恢复原始 + * ChatOrigin (webchat origin 在 createPending 时已通过 ChatOriginHolder + * 持久化到 approval 行)。 + *

+ * 重放期间可能再次触发审批 (一个工具批准后 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 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()); + } + } + /** * 重新生成最后一条助手回复。 *

diff --git a/mateclaw-server/src/test/java/vip/mate/channel/webchat/WebChatApprovalInteractionTest.java b/mateclaw-server/src/test/java/vip/mate/channel/webchat/WebChatApprovalInteractionTest.java new file mode 100644 index 00000000..f2cb57d4 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/channel/webchat/WebChatApprovalInteractionTest.java @@ -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. + * + *

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> 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> 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> 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> 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> 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> 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> 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> 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> r = controller.stopSession( + API_KEY, tokenFor("visitorG"), "visitorG", "s1"); + assertThat(r.getCode()).isEqualTo(200); + assertThat(r.getData().get("stopped")).isEqualTo(Boolean.FALSE); + } +}