diff --git a/server/src/__tests__/task-connector-read-routes.test.ts b/server/src/__tests__/task-connector-read-routes.test.ts new file mode 100644 index 0000000000..f903e4693c --- /dev/null +++ b/server/src/__tests__/task-connector-read-routes.test.ts @@ -0,0 +1,94 @@ +import express from "express"; +import request from "supertest"; +import type { Db } from "@paperclipai/db"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { errorHandler } from "../middleware/error-handler.js"; +import { chatChannelRoutes } from "../routes/chat-channels.js"; +import { emailRoutes } from "../routes/email.js"; +import type { ChatChannelService } from "../services/chat-channels.js"; +import type { EmailChannelService } from "../services/email-channels.js"; + +const companyId = "11111111-1111-4111-8111-111111111111"; +const issueId = "22222222-2222-4222-8222-222222222222"; +const boardActor: Express.Request["actor"] = { + type: "board", + source: "session", + userId: "test-board-user", + companyIds: [companyId], +}; +const chat = { getIssueBinding: vi.fn(), get: vi.fn() }; +const email = { authorizeRead: vi.fn(), thread: vi.fn() }; + +function app(actor = boardActor) { + const instance = express(); + instance.use((req, _res, next) => { + req.actor = actor; + next(); + }); + instance.use("/api", chatChannelRoutes({} as Db, { + service: chat as unknown as ChatChannelService, + heartbeat: { wakeup: vi.fn() }, + })); + instance.use("/api", emailRoutes({} as Db, email as unknown as EmailChannelService)); + instance.use(errorHandler); + return instance; +} + +beforeEach(() => { + vi.resetAllMocks(); + chat.getIssueBinding.mockResolvedValue(null); + email.authorizeRead.mockResolvedValue(undefined); + email.thread.mockResolvedValue(null); +}); + +describe("task connector read routes", () => { + it.each([`chat:${issueId}`, "undefined", "not-a-uuid", ` ${issueId} `])( + "rejects invalid task ID %s before reading either connector", + async (invalidId) => { + const server = app(); + const encodedId = encodeURIComponent(invalidId); + await request(server).get(`/api/issues/${encodedId}/chat-binding`).expect(400); + await request(server).get(`/api/companies/${companyId}/email/tasks/${encodedId}`).expect(400); + expect(chat.getIssueBinding).not.toHaveBeenCalled(); + expect(email.authorizeRead).not.toHaveBeenCalled(); + expect(email.thread).not.toHaveBeenCalled(); + }, + ); + + it("retains normal task reads and email authorization", async () => { + const server = app(); + await request(server).get(`/api/issues/${issueId}/chat-binding`).expect(200, "null"); + await request(server).get(`/api/companies/${companyId}/email/tasks/${issueId}`).expect(200, "null"); + expect(chat.getIssueBinding).toHaveBeenCalledWith(issueId); + expect(email.authorizeRead).toHaveBeenCalledWith(companyId, issueId, { + userId: boardActor.userId, + localImplicit: false, + }); + expect(email.thread).toHaveBeenCalledWith(companyId, issueId); + }); + + it.each([issueId, `chat:${issueId}`])("retains authentication checks for %s", async (id) => { + const server = app({ type: "none", source: "none" }); + await request(server).get(`/api/issues/${id}/chat-binding`).expect(403); + await request(server).get(`/api/companies/${companyId}/email/tasks/${id}`).expect(401); + expect(chat.getIssueBinding).not.toHaveBeenCalled(); + expect(email.authorizeRead).not.toHaveBeenCalled(); + expect(email.thread).not.toHaveBeenCalled(); + }); + + it("retains company isolation for task bindings", async () => { + const binding = { endpointId: "test-endpoint" }; + chat.getIssueBinding.mockResolvedValue(binding); + chat.get.mockResolvedValue({ companyId }); + await request(app()).get(`/api/issues/${issueId}/chat-binding`).expect(200, binding); + chat.get.mockResolvedValue({ companyId: "other-company" }); + await request(app()).get(`/api/issues/${issueId}/chat-binding`).expect(404); + }); + + it.each([issueId, `chat:${issueId}`])("retains company isolation for email task %s", async (id) => { + await request(app({ ...boardActor, companyIds: [] })) + .get(`/api/companies/${companyId}/email/tasks/${id}`).expect(403); + expect(email.authorizeRead).not.toHaveBeenCalled(); + expect(email.thread).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/routes/chat-channels.ts b/server/src/routes/chat-channels.ts index 260caa4c56..8883be250e 100644 --- a/server/src/routes/chat-channels.ts +++ b/server/src/routes/chat-channels.ts @@ -436,7 +436,11 @@ export function chatChannelRoutes(db: Db, options: ChatChannelRouteOptions) { router.get("/issues/:issueId/chat-binding", async (req, res) => { assertBoard(req); - const binding = await service.getIssueBinding(req.params.issueId as string); + const issueId = req.params.issueId as string; + if (issueId !== issueId.trim() || !isUuidLike(issueId)) { + throw badRequest("Task ID must be a UUID"); + } + const binding = await service.getIssueBinding(issueId); if (binding) { const endpoint = await getAccessibleResource( req, diff --git a/server/src/routes/email.ts b/server/src/routes/email.ts index efbe970f43..937f3bc07e 100644 --- a/server/src/routes/email.ts +++ b/server/src/routes/email.ts @@ -4,13 +4,14 @@ import { emailConnectionSchema, emailEndpointSetupSchema, emailSendSchema, + isUuidLike, } from "@paperclipai/shared"; import type { Db } from "@paperclipai/db"; import { validate } from "../middleware/validate.js"; import { assertBoard, assertCompanyAccess, hasCompanyAccess } from "./authz.js"; import { emailConnectionService } from "../services/email-connections.js"; import { accessService } from "../services/access.js"; -import { forbidden, notFound } from "../errors.js"; +import { badRequest, forbidden, notFound } from "../errors.js"; import type { EmailChannelService, EmailActor, @@ -188,14 +189,18 @@ export function emailRoutes(db: Db, service: EmailChannelService) { router.get("/companies/:companyId/email/tasks/:issueId", async (req, res) => { const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); + const issueId = req.params.issueId as string; + if (issueId !== issueId.trim() || !isUuidLike(issueId)) { + throw badRequest("Task ID must be a UUID"); + } await service.authorizeRead( companyId, - req.params.issueId as string, + issueId, actor(req), ); const thread = await service.thread( companyId, - req.params.issueId as string, + issueId, ); if ( thread && diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 2b1a21d03a..5eac1bc5e3 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -2561,10 +2561,11 @@ registry.registerPath({ tags: ["chat-channels", "issues"], summary: "Get a task's external chat binding", description: - "Returns the task's current external conversation binding, or `null` when it has none. A binding in another company is reported as not found.", + "Returns the task's current external conversation binding, or `null` when it has none. Requires a task UUID; synthetic agent-chat view IDs are invalid. A binding in another company is reported as not found.", request: { params: z.object({ issueId: z.string().uuid() }) }, responses: { 200: r.ok(externalChannelBindingResponseSchema.nullable()), + 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden, 404: r.notFound, diff --git a/ui/src/components/EmailTaskActivity.tsx b/ui/src/components/EmailTaskActivity.tsx index cef0c38a90..428a953085 100644 --- a/ui/src/components/EmailTaskActivity.tsx +++ b/ui/src/components/EmailTaskActivity.tsx @@ -17,11 +17,14 @@ export function EmailTaskActivity({ }) { const cache = useQueryClient(); const threadKey = ["email-thread", companyId, issueId]; + const queryEnabled = Boolean(companyId && issueId) && !issueId.startsWith("chat:"); const thread = useQuery({ queryKey: threadKey, queryFn: () => emailApi.thread(companyId, issueId), + enabled: queryEnabled, refetchInterval: 3000, }); + if (!queryEnabled) return null; const data = thread.data; const messages = data?.messages.filter((m) => !m.commentId) ?? []; const publications = data?.publications.filter( diff --git a/ui/src/components/chat/ExternallyConnectedTaskBanner.tsx b/ui/src/components/chat/ExternallyConnectedTaskBanner.tsx index 9a5eee6b8d..67b2fe2218 100644 --- a/ui/src/components/chat/ExternallyConnectedTaskBanner.tsx +++ b/ui/src/components/chat/ExternallyConnectedTaskBanner.tsx @@ -110,14 +110,15 @@ const filePhaseLabels: Record = { export function useIssueChatBinding(companyId: string, issueId: string) { const { enabled } = useChatConnectorsEnabled(); + const queryEnabled = enabled && Boolean(companyId && issueId) && !issueId.startsWith("chat:"); const query = useQuery({ queryKey: ["issue-chat-binding", companyId, issueId], queryFn: () => chatEndpointsApi.getIssueBinding(issueId), - enabled: enabled && Boolean(companyId && issueId), + enabled: queryEnabled, }); return { - binding: enabled ? (query.data ?? null) : null, - isLoading: enabled && query.isLoading, + binding: queryEnabled ? (query.data ?? null) : null, + isLoading: queryEnabled && query.isLoading, }; } diff --git a/ui/src/components/task-only-connector-queries.test.tsx b/ui/src/components/task-only-connector-queries.test.tsx new file mode 100644 index 0000000000..ec9a021f6c --- /dev/null +++ b/ui/src/components/task-only-connector-queries.test.tsx @@ -0,0 +1,98 @@ +// @vitest-environment jsdom + +import { flushSync } from "react-dom"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { EmailTaskActivity } from "./EmailTaskActivity"; +import { useIssueChatBinding } from "./chat/ExternallyConnectedTaskBanner"; + +const api = vi.hoisted(() => ({ getIssueBinding: vi.fn(), thread: vi.fn() })); +vi.mock("@/api/chatEndpoints", () => ({ chatEndpointsApi: api })); +vi.mock("@/api/email", () => ({ emailApi: api })); +vi.mock("@/hooks/useChatConnectorsEnabled", () => ({ + useChatConnectorsEnabled: () => ({ enabled: true, loaded: true }), +})); + +const companyId = "test-company"; +const taskId = "22222222-2222-4222-8222-222222222222"; +const chatId = `chat:${taskId}`; + +function ConnectorQueries({ issueId }: { issueId: string }) { + const { binding, isLoading } = useIssueChatBinding(companyId, issueId); + return <> + {binding ? "bound" : isLoading ? "loading" : "unbound"} + + ; +} + +describe("task-only connector queries", () => { + let container: HTMLDivElement; + let root: Root; + let client: QueryClient; + + async function render(issueId: string) { + flushSync(() => root.render( + + + , + )); + for (let i = 0; i < 5; i += 1) { + await new Promise((resolve) => window.setTimeout(resolve, 0)); + } + flushSync(() => {}); + } + + beforeEach(() => { + vi.resetAllMocks(); + api.getIssueBinding.mockResolvedValue({ endpointId: "test-endpoint" }); + api.thread.mockResolvedValue(null); + client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + flushSync(() => root.unmount()); + client.clear(); + container.remove(); + }); + + it.each([chatId, ""])("does not request task connectors for %s", async (id) => { + await render(id); + expect(api.getIssueBinding).not.toHaveBeenCalled(); + expect(api.thread).not.toHaveBeenCalled(); + expect(container.textContent).toBe("unbound"); + }); + + it("resumes task queries when navigating from an agent chat to a task", async () => { + await render(taskId); + expect(api.getIssueBinding).toHaveBeenCalledWith(taskId); + expect(api.thread).toHaveBeenCalledWith(companyId, taskId); + expect(container.textContent).toBe("bound"); + + await render(chatId); + expect(api.getIssueBinding).toHaveBeenCalledTimes(1); + expect(api.thread).toHaveBeenCalledTimes(1); + expect(container.textContent).toBe("unbound"); + + const nextTaskId = "33333333-3333-4333-8333-333333333333"; + await render(nextTaskId); + expect(api.getIssueBinding).toHaveBeenLastCalledWith(nextTaskId); + expect(api.thread).toHaveBeenLastCalledWith(companyId, nextTaskId); + expect(container.textContent).toBe("bound"); + }); + + it("does not display cached task connector data for an agent chat", async () => { + client.setQueryData(["issue-chat-binding", companyId, chatId], { endpointId: "stale-endpoint" }); + client.setQueryData(["email-thread", companyId, chatId], { + messages: [], + publications: [{ id: "stale-publication", outcome: "failed", error: "Stale task delivery" }], + }); + await render(chatId); + expect(container.textContent).toBe("unbound"); + expect(api.getIssueBinding).not.toHaveBeenCalled(); + expect(api.thread).not.toHaveBeenCalled(); + }); +});