diff --git a/server/internal/handler/chat.go b/server/internal/handler/chat.go index 9e1d842502..ccd22b8215 100644 --- a/server/internal/handler/chat.go +++ b/server/internal/handler/chat.go @@ -348,6 +348,14 @@ func (h *Handler) DeleteChatSession(w http.ResponseWriter, r *http.Request) { return } + // channel_chat_session_binding used to carry a chat_session FK with + // ON DELETE CASCADE; MUL-3515 §4 dropped every channel_* foreign key, so + // prune the binding here in the same tx that deletes its chat_session. + if err := qtx.DeleteChannelChatSessionBindingBySession(r.Context(), session.ID); err != nil { + writeError(w, http.StatusInternalServerError, "failed to delete chat session binding") + return + } + if err := qtx.DeleteChatSession(r.Context(), db.DeleteChatSessionParams{ ID: session.ID, WorkspaceID: session.WorkspaceID, diff --git a/server/internal/handler/chat_test.go b/server/internal/handler/chat_test.go index df1bdfe440..cc0c557029 100644 --- a/server/internal/handler/chat_test.go +++ b/server/internal/handler/chat_test.go @@ -444,3 +444,64 @@ func TestListChatMessagesPage_RejectsInvalidLimit(t *testing.T) { t.Fatalf("ListChatMessagesPage invalid limit: expected 400, got %d: %s", w.Code, w.Body.String()) } } + +// TestDeleteChatSession_PrunesChannelChatSessionBinding verifies the +// application-layer replacement for the channel_chat_session_binding +// chat_session-FK cascade (MUL-3515 §4): deleting a chat session prunes its +// channel binding in the same tx that deletes the session row. +func TestDeleteChatSession_PrunesChannelChatSessionBinding(t *testing.T) { + agentID := createHandlerTestAgent(t, "ChatDeleteBindingAgent", []byte("[]")) + sessionID := createHandlerTestChatSession(t, agentID) + ctx := context.Background() + + const appID = "cli_chat_delete_binding" + const channelChatID = "oc_chat_delete_binding" + + // channel_* rows have no FK to chat_session/workspace (MUL-3515 §4), so + // they outlive the helper's chat_session cleanup; clear by deterministic + // key before and after. + cleanChannel := func() { + _, _ = testPool.Exec(context.Background(), + `DELETE FROM channel_chat_session_binding WHERE channel_chat_id = $1`, channelChatID) + _, _ = testPool.Exec(context.Background(), + `DELETE FROM channel_installation WHERE channel_type = 'feishu' AND config->>'app_id' = $1`, appID) + } + cleanChannel() + t.Cleanup(cleanChannel) + + var installID string + if err := testPool.QueryRow(ctx, ` +INSERT INTO channel_installation (workspace_id, agent_id, channel_type, config, installer_user_id) +VALUES ($1, $2, 'feishu', jsonb_build_object('app_id', $3::text), $4) +RETURNING id +`, testWorkspaceID, agentID, appID, testUserID).Scan(&installID); err != nil { + t.Fatalf("insert channel_installation: %v", err) + } + + if _, err := testPool.Exec(ctx, ` +INSERT INTO channel_chat_session_binding (chat_session_id, installation_id, channel_type, channel_chat_id, chat_type) +VALUES ($1, $2, 'feishu', $3, 'p2p') +`, sessionID, installID, channelChatID); err != nil { + t.Fatalf("insert channel_chat_session_binding: %v", err) + } + + req := httptest.NewRequest(http.MethodDelete, "/api/chat/sessions/"+sessionID, nil) + req.Header.Set("X-User-ID", testUserID) + req = withURLParam(req, "sessionId", sessionID) + req = withChatTestWorkspaceCtx(t, req) + w := httptest.NewRecorder() + testHandler.DeleteChatSession(w, req) + + if w.Code != http.StatusNoContent { + t.Fatalf("DeleteChatSession: expected 204, got %d: %s", w.Code, w.Body.String()) + } + + var bindingExists bool + if err := testPool.QueryRow(ctx, + `SELECT EXISTS (SELECT 1 FROM channel_chat_session_binding WHERE channel_chat_id = $1)`, channelChatID).Scan(&bindingExists); err != nil { + t.Fatalf("query chat session binding: %v", err) + } + if bindingExists { + t.Fatal("deleted chat session's channel_chat_session_binding was not pruned") + } +} diff --git a/server/internal/handler/workspace_revoke.go b/server/internal/handler/workspace_revoke.go index 0d0ce1dcf5..e58dfc6052 100644 --- a/server/internal/handler/workspace_revoke.go +++ b/server/internal/handler/workspace_revoke.go @@ -112,6 +112,19 @@ func (h *Handler) revokeAndRemoveMember(ctx context.Context, workspaceID, userID } } + // channel_user_binding used to carry a member FK with ON DELETE CASCADE, so + // a removed member's IM bindings vanished automatically. MUL-3515 §4 dropped + // every channel_* foreign key, moving that integrity rule to the application + // layer: prune the bindings here, in the same tx as the member-row delete. + // The inbound path also re-checks membership (see ChannelStore.IsWorkspaceMember), + // but pruning stops a stale binding from lingering across a remove/re-add. + if err := qtx.DeleteChannelUserBindingsByWorkspaceMember(ctx, db.DeleteChannelUserBindingsByWorkspaceMemberParams{ + WorkspaceID: workspaceID, + MulticaUserID: userID, + }); err != nil { + return empty, err + } + // Member row deletion lives inside the same tx so a successful revoke is // never followed by a failed member-delete (which would leave the user // still a member with a dead runtime), and a failed revoke never leaves diff --git a/server/internal/handler/workspace_test.go b/server/internal/handler/workspace_test.go index 14d3d4b6ce..bc87fe2b37 100644 --- a/server/internal/handler/workspace_test.go +++ b/server/internal/handler/workspace_test.go @@ -596,6 +596,86 @@ func TestDeleteMember_RevokesTargetRuntimes(t *testing.T) { assertRevoked(t, fx) } +// TestDeleteMember_PrunesChannelUserBindings verifies the application-layer +// replacement for the channel_user_binding member-FK cascade (MUL-3515 §4): +// removing a member prunes that member's channel bindings, in the same tx as +// the member-row delete, while leaving a remaining member's binding intact. +func TestDeleteMember_PrunesChannelUserBindings(t *testing.T) { + fx := setupRevocationFixture(t, "handler-tests-revoke-binding", "daemon-revoke-binding") + ctx := context.Background() + + const appID = "cli_revoke_binding" + const removedOpenID = "ou_revoke_binding_removed" + const keepOpenID = "ou_revoke_binding_keep" + + // channel_* rows have no FK to workspace (MUL-3515 §4), so the fixture's + // workspace-delete cleanup never reaches them; clear by deterministic key + // both before (in case a prior run was killed mid-test) and after. + cleanChannel := func() { + _, _ = testPool.Exec(context.Background(), + `DELETE FROM channel_user_binding WHERE channel_user_id = ANY($1)`, + []string{removedOpenID, keepOpenID}) + _, _ = testPool.Exec(context.Background(), + `DELETE FROM channel_installation WHERE channel_type = 'feishu' AND config->>'app_id' = $1`, appID) + } + cleanChannel() + t.Cleanup(cleanChannel) + + var installID string + if err := testPool.QueryRow(ctx, ` +INSERT INTO channel_installation (workspace_id, agent_id, channel_type, config, installer_user_id) +VALUES ($1, $2, 'feishu', jsonb_build_object('app_id', $3::text), $4) +RETURNING id +`, fx.WorkspaceID, fx.AgentID, appID, testUserID).Scan(&installID); err != nil { + t.Fatalf("insert channel_installation: %v", err) + } + + // Binding for the member being removed — must be pruned. + if _, err := testPool.Exec(ctx, ` +INSERT INTO channel_user_binding (workspace_id, multica_user_id, installation_id, channel_type, channel_user_id) +VALUES ($1, $2, $3, 'feishu', $4) +`, fx.WorkspaceID, fx.TargetUserID, installID, removedOpenID); err != nil { + t.Fatalf("insert removed-member binding: %v", err) + } + + // Binding for the requester (an owner who stays) — must survive, proving + // the prune is scoped to the removed user, not the whole workspace. + if _, err := testPool.Exec(ctx, ` +INSERT INTO channel_user_binding (workspace_id, multica_user_id, installation_id, channel_type, channel_user_id) +VALUES ($1, $2, $3, 'feishu', $4) +`, fx.WorkspaceID, testUserID, installID, keepOpenID); err != nil { + t.Fatalf("insert remaining-member binding: %v", err) + } + + w := httptest.NewRecorder() + req := newRequest("DELETE", "/api/workspaces/"+fx.WorkspaceID+"/members/"+fx.MemberID, nil) + req.Header.Set("X-Workspace-ID", fx.WorkspaceID) + req = withURLParams(req, "id", fx.WorkspaceID, "memberId", fx.MemberID) + testHandler.DeleteMember(w, req) + + if w.Code != http.StatusNoContent { + t.Fatalf("DeleteMember: expected 204, got %d: %s", w.Code, w.Body.String()) + } + + var removedExists bool + if err := testPool.QueryRow(ctx, + `SELECT EXISTS (SELECT 1 FROM channel_user_binding WHERE channel_user_id = $1)`, removedOpenID).Scan(&removedExists); err != nil { + t.Fatalf("query removed-member binding: %v", err) + } + if removedExists { + t.Fatal("removed member's channel_user_binding was not pruned") + } + + var keepExists bool + if err := testPool.QueryRow(ctx, + `SELECT EXISTS (SELECT 1 FROM channel_user_binding WHERE channel_user_id = $1)`, keepOpenID).Scan(&keepExists); err != nil { + t.Fatalf("query remaining-member binding: %v", err) + } + if !keepExists { + t.Fatal("remaining member's channel_user_binding was wrongly pruned") + } +} + // TestLeaveWorkspace_RevokesOwnRuntimes is the self-removal counterpart: when // a member leaves a workspace voluntarily, their own runtimes are revoked // with the same atomic write set as DeleteMember.