From 6dde8ad932cc164d8c131daa80f86e4e67ea8752 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Fri, 19 Jun 2026 07:42:27 -0500 Subject: [PATCH] fix(tasks): let admins fetch any task by id alone (cross-account read) GET /api/tasks scopes every lookup to the caller's own account. A lookup by `id` alone therefore returns nothing when the caller's key doesn't own the task, which blocks the background worker (customer-prompt-task) from loading a customer's scheduled task config with a shared admin key. When an admin caller queries by `id` with no `account_id` param, drop the account scope so the single task is returned regardless of owner. Non-admin id lookups stay scoped to the authenticated account (no cross-account leak). ValidatedGetTasksQuery.account_id is now optional; selectScheduledActions already filters by account_id only when present. TDD: RED (admin id lookup not cross-account, non-admin not scoped) -> GREEN. Fixes part of recoupable/chat#1810. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../__tests__/validateGetTasksQuery.test.ts | 35 +++++++++++++++++++ lib/tasks/validateGetTasksQuery.ts | 16 +++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/lib/tasks/__tests__/validateGetTasksQuery.test.ts b/lib/tasks/__tests__/validateGetTasksQuery.test.ts index b9d0dda71..21722150a 100644 --- a/lib/tasks/__tests__/validateGetTasksQuery.test.ts +++ b/lib/tasks/__tests__/validateGetTasksQuery.test.ts @@ -119,6 +119,41 @@ describe("validateGetTasksQuery", () => { }); }); + it("lets an admin fetch any task by id alone, dropping account scope", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: "worker_admin_acc", + orgId: null, + authToken: "token", + }); + vi.mocked(checkIsAdmin).mockResolvedValue(true); + + const result = await validateGetTasksQuery( + createMockRequest("http://localhost:3000/api/tasks?id=task_owned_by_someone_else"), + ); + + expect(checkIsAdmin).toHaveBeenCalledWith("worker_admin_acc"); + expect(validateAccountIdOverride).not.toHaveBeenCalled(); + // No account_id key -> selectScheduledActions filters by id only (cross-account read). + expect(result).toEqual({ id: "task_owned_by_someone_else" }); + }); + + it("keeps a non-admin id lookup scoped to the authenticated account", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: "regular_acc", + orgId: null, + authToken: "token", + }); + vi.mocked(checkIsAdmin).mockResolvedValue(false); + + const result = await validateGetTasksQuery( + createMockRequest("http://localhost:3000/api/tasks?id=task_1"), + ); + + expect(checkIsAdmin).toHaveBeenCalledWith("regular_acc"); + expect(validateAccountIdOverride).not.toHaveBeenCalled(); + expect(result).toEqual({ id: "task_1", account_id: "regular_acc" }); + }); + it("returns 403 when non-admin override is denied", async () => { vi.mocked(validateAuthContext).mockResolvedValue({ accountId: "org_owner_acc", diff --git a/lib/tasks/validateGetTasksQuery.ts b/lib/tasks/validateGetTasksQuery.ts index 3e9d56201..f581a438a 100644 --- a/lib/tasks/validateGetTasksQuery.ts +++ b/lib/tasks/validateGetTasksQuery.ts @@ -19,7 +19,9 @@ export const getTasksQuerySchema = z.object({ }); export type GetTasksQuery = z.infer; -export type ValidatedGetTasksQuery = Omit & { account_id: string }; +// account_id is optional: admin callers fetching a single task by `id` are not +// scoped to their own account, so the resolved query may omit account_id entirely. +export type ValidatedGetTasksQuery = Omit & { account_id?: string }; /** * Validates get tasks query parameters from a NextRequest. @@ -55,7 +57,7 @@ export async function validateGetTasksQuery( ); } - let targetAccountId = authResult.accountId; + let targetAccountId: string | undefined = authResult.accountId; if ( validationResult.data.account_id && @@ -76,10 +78,18 @@ export async function validateGetTasksQuery( targetAccountId = overrideResult.accountId; } + } else if (validationResult.data.id && !validationResult.data.account_id) { + // Single-task lookup by id: admins may read any task regardless of owner + // (e.g. the background worker fetching a customer's scheduled task config). + // Non-admins stay scoped to their own account. + const isAdmin = await checkIsAdmin(authResult.accountId); + if (isAdmin) { + targetAccountId = undefined; + } } return { ...validationResult.data, - account_id: targetAccountId, + ...(targetAccountId ? { account_id: targetAccountId } : {}), }; }