From ba7e9cd1a0691201b282da6f6c703d5a00daafc3 Mon Sep 17 00:00:00 2001 From: Chris Estreich Date: Tue, 10 Jun 2025 13:54:41 -0700 Subject: [PATCH] Fix getTasks result validation + co-locate analytics types (#94) --- src/actions/analytics/events.ts | 80 +++++-------------- src/actions/analytics/messages.ts | 19 ++++- src/actions/auth.ts | 55 +++++++++++++ src/actions/taskSharing.ts | 8 +- src/app/(authenticated)/usage/Messages.tsx | 2 +- .../task-sharing/SharedTaskView.tsx | 3 +- src/types/analytics/index.ts | 2 - src/types/analytics/message.ts | 19 ----- src/types/analytics/task.ts | 15 ---- 9 files changed, 99 insertions(+), 104 deletions(-) delete mode 100644 src/types/analytics/index.ts delete mode 100644 src/types/analytics/message.ts delete mode 100644 src/types/analytics/task.ts diff --git a/src/actions/analytics/events.ts b/src/actions/analytics/events.ts index daa6e26bcf..3eb85321f8 100644 --- a/src/actions/analytics/events.ts +++ b/src/actions/analytics/events.ts @@ -1,7 +1,6 @@ 'use server'; import { z } from 'zod'; -import { auth } from '@clerk/nextjs/server'; import { type RooCodeTelemetryEvent, @@ -9,67 +8,12 @@ import { } from '@roo-code/types'; import type { AnyTimePeriod } from '@/types'; -import { taskSchema } from '@/types/analytics'; import { analytics } from '@/lib/server'; import { type User, getUsersById } from '@/db/server'; +import { validateAnalyticsAccess } from '@/actions/auth'; type Table = 'events' | 'messages'; -/** - * Validates authentication and authorization for analytics functions - */ -async function validateAnalyticsAccess({ - requestedOrgId, - requestedUserId, - requireAdmin = false, - allowCrossUserAccess = false, -}: { - requestedOrgId?: string | null; - requestedUserId?: string | null; - requireAdmin?: boolean; - allowCrossUserAccess?: boolean; -}): Promise<{ - authOrgId: string; - authUserId: string; - orgRole: string; - effectiveUserId: string | null; -}> { - const { orgId: authOrgId, orgRole, userId: authUserId } = await auth(); - - // Ensure user is authenticated and belongs to the organization - if (!authOrgId || !authUserId || authOrgId !== requestedOrgId) { - throw new Error('Unauthorized: Invalid organization access'); - } - - // Check if admin access is required - if (requireAdmin && orgRole !== 'org:admin') { - throw new Error('Unauthorized: Administrator access required'); - } - - // If user is not an admin and trying to access data other than their own - if ( - orgRole !== 'org:admin' && - requestedUserId && - requestedUserId !== authUserId - ) { - throw new Error('Unauthorized: Members can only access their own data'); - } - - // For non-admin users, force userId filter to their own ID - // Unless allowCrossUserAccess is true and we're checking task sharing permissions - const effectiveUserId = - orgRole !== 'org:admin' && !allowCrossUserAccess - ? authUserId - : requestedUserId || null; - - return { - authOrgId, - authUserId, - orgRole: orgRole || 'unknown', - effectiveUserId, - }; -} - /** * captureEvent */ @@ -344,13 +288,20 @@ export const getModelUsage = async ({ * getTasks */ -const taskWithTitleSchema = taskSchema.extend({ +const taskSchema = z.object({ + taskId: z.string(), + userId: z.string(), + provider: z.string(), title: z.string().nullable(), + mode: z.string().nullable(), + model: z.string(), + completed: z.coerce.boolean(), + tokens: z.coerce.number(), + cost: z.coerce.number(), + timestamp: z.coerce.number(), }); -export type TaskWithTitle = z.infer; - -export type TaskWithUser = TaskWithTitle & { user: User }; +export type TaskWithUser = z.infer & { user: User }; export const getTasks = async ({ orgId, @@ -375,9 +326,11 @@ export const getTasks = async ({ const userFilter = effectiveUserId ? 'AND e.userId = {userId: String}' : ''; const taskFilter = taskId ? 'AND e.taskId = {taskId: String}' : ''; + const messageUserFilter = effectiveUserId ? 'AND userId = {userId: String}' : ''; + const messageTaskFilter = taskId ? 'AND taskId = {taskId: String}' : ''; const queryParams: Record = { @@ -388,9 +341,11 @@ export const getTasks = async ({ TelemetryEventName.LLM_COMPLETION, ], }; + if (effectiveUserId) { queryParams.userId = effectiveUserId; } + if (taskId) { queryParams.taskId = taskId; } @@ -424,6 +379,7 @@ export const getTasks = async ({ WHERE e.orgId = {orgId: String} AND e.type IN ({types: Array(String)}) + AND e.modelId IS NOT NULL ${userFilter} ${taskFilter} GROUP BY 1, 2 @@ -433,7 +389,7 @@ export const getTasks = async ({ query_params: queryParams, }); - const tasks = z.array(taskWithTitleSchema).parse(await results.json()); + const tasks = z.array(taskSchema).parse(await results.json()); const users = await getUsersById(tasks.map(({ userId }) => userId)); diff --git a/src/actions/analytics/messages.ts b/src/actions/analytics/messages.ts index a7c0afa1ff..7122c1641b 100644 --- a/src/actions/analytics/messages.ts +++ b/src/actions/analytics/messages.ts @@ -2,13 +2,30 @@ import { z } from 'zod'; -import { messageSchema, type Message } from '@/types/analytics'; import { analytics } from '@/lib/server'; /** * getMessages */ +const messageSchema = z.object({ + id: z.string(), + orgId: z.string(), + userId: z.string(), + taskId: z.string(), + mode: z.string().nullable(), + ts: z.number(), + type: z.enum(['ask', 'say']), + ask: z.string().nullable(), + say: z.string().nullable(), + text: z.string().nullable(), + reasoning: z.string().nullable(), + partial: z.boolean().nullable(), + timestamp: z.number(), +}); + +export type Message = z.infer; + export const getMessages = async (taskId: string): Promise => { const results = await analytics.query({ query: ` diff --git a/src/actions/auth.ts b/src/actions/auth.ts index 5efeb15676..9785272069 100644 --- a/src/actions/auth.ts +++ b/src/actions/auth.ts @@ -21,6 +21,61 @@ export async function validateAuth(): Promise< return { userId, orgId, orgRole: orgRole || 'unknown' }; } +/** + * Validates authentication and authorization for analytics functions. + */ +export async function validateAnalyticsAccess({ + requestedOrgId, + requestedUserId, + requireAdmin = false, + allowCrossUserAccess = false, +}: { + requestedOrgId?: string | null; + requestedUserId?: string | null; + requireAdmin?: boolean; + allowCrossUserAccess?: boolean; +}): Promise<{ + authOrgId: string; + authUserId: string; + orgRole: string; + effectiveUserId: string | null; +}> { + const { orgId: authOrgId, orgRole, userId: authUserId } = await auth(); + + // Ensure user is authenticated and belongs to the organization + if (!authOrgId || !authUserId || authOrgId !== requestedOrgId) { + throw new Error('Unauthorized: Invalid organization access'); + } + + // Check if admin access is required + if (requireAdmin && orgRole !== 'org:admin') { + throw new Error('Unauthorized: Administrator access required'); + } + + // If user is not an admin and trying to access data other than their own + if ( + orgRole !== 'org:admin' && + requestedUserId && + requestedUserId !== authUserId + ) { + throw new Error('Unauthorized: Members can only access their own data'); + } + + // For non-admin users, force userId filter to their own ID + // Unless allowCrossUserAccess is true and we're checking task sharing permissions + const effectiveUserId = + orgRole !== 'org:admin' && !allowCrossUserAccess + ? authUserId + : requestedUserId || null; + + return { + authOrgId, + authUserId, + orgRole: orgRole || 'unknown', + effectiveUserId, + }; +} + // Default expiration is 30 days (2592000 seconds). export async function getSignInToken( userId: string, diff --git a/src/actions/taskSharing.ts b/src/actions/taskSharing.ts index 1eadd75a16..ed3bed9f5e 100644 --- a/src/actions/taskSharing.ts +++ b/src/actions/taskSharing.ts @@ -10,7 +10,6 @@ import { shareIdSchema, } from '@/types'; import type { SharedByUser } from '@/types/task-sharing'; -import type { Message } from '@/types/analytics'; import { type TaskShare, AuditLogTargetType } from '@/db'; import { client as db, taskShares, users } from '@/db/server'; import { handleError, isAuthSuccess, generateShareToken } from '@/lib/server'; @@ -21,7 +20,12 @@ import { createShareUrl, DEFAULT_SHARE_EXPIRATION_DAYS, } from '@/lib/task-sharing'; -import { type TaskWithUser, getTasks, getMessages } from '@/actions/analytics'; +import { + type TaskWithUser, + getTasks, + type Message, + getMessages, +} from '@/actions/analytics'; import { validateAuth } from './auth'; import { insertAuditLog } from './auditLogs'; diff --git a/src/app/(authenticated)/usage/Messages.tsx b/src/app/(authenticated)/usage/Messages.tsx index 3881377443..2bee83dd82 100644 --- a/src/app/(authenticated)/usage/Messages.tsx +++ b/src/app/(authenticated)/usage/Messages.tsx @@ -1,7 +1,7 @@ import { useMemo } from 'react'; import ReactMarkdown from 'react-markdown'; -import type { Message } from '@/types/analytics'; +import type { Message } from '@/actions/analytics'; import { cn } from '@/lib/utils'; import { formatTimestamp } from '@/lib/formatters'; diff --git a/src/components/task-sharing/SharedTaskView.tsx b/src/components/task-sharing/SharedTaskView.tsx index ce93eac8ec..aa2d363362 100644 --- a/src/components/task-sharing/SharedTaskView.tsx +++ b/src/components/task-sharing/SharedTaskView.tsx @@ -1,5 +1,4 @@ -import type { TaskWithUser } from '@/actions/analytics'; -import type { Message } from '@/types/analytics'; +import type { TaskWithUser, Message } from '@/actions/analytics'; import type { SharedByUser } from '@/types/task-sharing'; import { formatCurrency, formatNumber } from '@/lib/formatters'; import { generateFallbackTitle } from '@/lib/task-utils'; diff --git a/src/types/analytics/index.ts b/src/types/analytics/index.ts deleted file mode 100644 index 60a0e684e4..0000000000 --- a/src/types/analytics/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -export * from './message'; -export * from './task'; diff --git a/src/types/analytics/message.ts b/src/types/analytics/message.ts deleted file mode 100644 index cc538ea714..0000000000 --- a/src/types/analytics/message.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { z } from 'zod'; - -export const messageSchema = z.object({ - id: z.string(), - orgId: z.string(), - userId: z.string(), - taskId: z.string(), - mode: z.string().nullable(), - ts: z.number(), - type: z.enum(['ask', 'say']), - ask: z.string().nullable(), - say: z.string().nullable(), - text: z.string().nullable(), - reasoning: z.string().nullable(), - partial: z.boolean().nullable(), - timestamp: z.number(), -}); - -export type Message = z.infer; diff --git a/src/types/analytics/task.ts b/src/types/analytics/task.ts deleted file mode 100644 index 4e18bca2d9..0000000000 --- a/src/types/analytics/task.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { z } from 'zod'; - -export const taskSchema = z.object({ - taskId: z.string(), - userId: z.string(), - provider: z.string(), - model: z.string(), - mode: z.string().nullable(), - completed: z.coerce.boolean(), - tokens: z.coerce.number(), - cost: z.coerce.number(), - timestamp: z.coerce.number(), -}); - -export type Task = z.infer;