🛡️ Sentinel: [HIGH] Fix IDOR and insecure server actions

.jules/sentinel.md

@@ -0,0 +1,11 @@
+# Sentinel Security Journal - Critical Learnings
+
+## 2025-05-14 - [IDOR in Server Actions]
+**Vulnerability:** Server actions in `lib/actions/chat.ts` were accepting `userId` as an argument from the client, allowing attackers to access or modify other users' chats by providing their IDs. Additionally, `getChatMessages` lacked any authorization check, returning all messages for any given `chatId`.
+**Learning:** Next.js server actions (exported functions in files with `'use server'`) are public endpoints. Trusting client-side parameters for user identification is a major security risk. Authorization must be enforced at the entry point of every server action.
+**Prevention:** Always retrieve user identity server-side from secure sessions/cookies using utilities like `getCurrentUserIdOnServer()`. For resource access (like messages), verify ownership or public visibility using a database-level check (e.g., `dbGetChat`) before performing the operation.
+
+## 2025-05-14 - [Database UUID Type Safety]
+**Vulnerability:** Passing a non-UUID string like `'anonymous'` to a PostgreSQL `uuid` field in a query can cause a database error.
+**Learning:** When handling unauthenticated users in database queries, ensure that either a valid UUID is used or the logic explicitly handles the absence of a user ID before hitting the database query that expects a UUID.
+**Prevention:** Refactor database utility functions to accept `string | null` and handle the `null` case early to avoid invalid type syntax errors in SQL.

app/actions.tsx

@@ -397,8 +397,7 @@ async function submit(formData?: FormData, skip?: boolean) {
     } as CoreMessage)
   }
 
-  const userId = 'anonymous'
-  const currentSystemPrompt = (await getSystemPrompt(userId)) || ''
+  const currentSystemPrompt = (await getSystemPrompt()) || ''
   const mapProvider = formData?.get('mapProvider') as 'mapbox' | 'google'
 
   async function processEvents() {
@@ -655,7 +654,7 @@ export const AI = createAI<AIState, UIState>({
       title,
       messages: updatedMessages
     }
-    await saveChat(chat, actualUserId)
+    await saveChat(chat)
   }
 })
 

app/search/[id]/page.tsx

@@ -15,10 +15,8 @@ export interface SearchPageProps {
 
 export async function generateMetadata({ params }: SearchPageProps) {
   const { id } = await params; // Keep as is for now
-  // TODO: Metadata generation might need authenticated user if chats are private
-  // For now, assuming getChat can be called or it handles anon access for metadata appropriately
-  const userId = await getCurrentUserIdOnServer(); // Attempt to get user for metadata
-  const chat = await getChat(id, userId || 'anonymous'); // Pass userId or 'anonymous' if none
+  // getChat will handle authenticated or anonymous access via session
+  const chat = await getChat(id);
   return {
     title: chat?.title?.toString().slice(0, 50) || 'Search',
   };
@@ -34,14 +32,14 @@ export default async function SearchPage({ params }: SearchPageProps) {
     redirect('/');
   }
 
-  const chat = await getChat(id, userId);
+  const chat = await getChat(id);
 
   if (!chat) {
     // If chat doesn't exist or user doesn't have access (handled by getChat)
     notFound();
   }
 
-  // Fetch messages for the chat
+  // Fetch messages for the chat (verified by getChatMessages)
   const dbMessages: DrizzleMessage[] = await getChatMessages(chat.id);
 
   // Transform DrizzleMessages to AIMessages

components/history-list.tsx

@@ -5,16 +5,16 @@ import { getChats } from '@/lib/actions/chat';
 import type { Chat as DrizzleChat } from '@/lib/actions/chat-db';
 
 type HistoryListProps = {
-  userId?: string;
+  // userId is no longer used here as getChats() retrieves it from the session
 };
 
-const loadChats = cache(async (userId?: string): Promise<DrizzleChat[] | null> => {
-  return await getChats(userId);
+const loadChats = cache(async (): Promise<DrizzleChat[] | null> => {
+  return await getChats();
 });
 
-export async function HistoryList({ userId }: HistoryListProps) {
+export async function HistoryList({}: HistoryListProps) {
   try {
-    const chats = await loadChats(userId);
+    const chats = await loadChats();
 
     if (!chats) {
       return (

components/settings/components/settings.tsx

@@ -86,7 +86,7 @@ export function Settings({ initialTab = "system-prompt" }: SettingsProps) {
       if (!userId || authLoading) return;
 
       const [existingPrompt, selectedModel] = await Promise.all([
-        getSystemPrompt(userId),
+        getSystemPrompt(),
         getSelectedModel(),
       ]);
 
@@ -119,7 +119,7 @@ export function Settings({ initialTab = "system-prompt" }: SettingsProps) {
     try {
       // Save the system prompt and selected model
       const [promptSaveResult, modelSaveResult] = await Promise.all([
-        saveSystemPrompt(userId, data.systemPrompt),
+        saveSystemPrompt(data.systemPrompt),
         saveSelectedModel(data.selectedModel),
       ]);
 

lib/actions/chat-db.ts

@@ -19,9 +19,9 @@ export type NewMessage = typeof messages.$inferInsert;
  * @param userId - The ID of the user requesting the chat.
  * @returns The chat object if found and accessible, otherwise null.
  */
-export async function getChat(id: string, userId: string): Promise<Chat | null> {
-  if (!userId) {
-    console.warn('getChat called without userId');
+export async function getChat(id: string, userId: string | null): Promise<Chat | null> {
+  if (!userId || userId === 'anonymous') {
+    console.warn('getChat called without valid userId');
     // Potentially allow fetching public chats if userId is null for anonymous users
     const result = await db.select().from(chats).where(and(eq(chats.id, id), eq(chats.visibility, 'public'))).limit(1);
     return result[0] || null;

lib/actions/chat.ts

@@ -20,9 +20,14 @@ import { users } from '@/lib/db/schema'
 import { eq } from 'drizzle-orm'
 import { getCurrentUserIdOnServer } from '@/lib/auth/get-current-user'
 
-export async function getChats(userId?: string | null): Promise<DrizzleChat[]> {
+/**
+ * Retrieves chats for the current authenticated user.
+ * Protected against IDOR by retrieving userId from session.
+ */
+export async function getChats(): Promise<DrizzleChat[]> {
+  const userId = await getCurrentUserIdOnServer()
   if (!userId) {
-    console.warn('getChats called without userId, returning empty array.')
+    console.warn('getChats: No authenticated user found.')
     return []
   }
 
@@ -35,13 +40,15 @@ export async function getChats(userId?: string | null): Promise<DrizzleChat[]> {
   }
 }
 
-export async function getChat(id: string, userId: string): Promise<DrizzleChat | null> {
-  if (!userId) {
-    console.warn('getChat called without userId.')
-    return null;
-  }
+/**
+ * Retrieves a specific chat, ensuring the current user has access.
+ * Protected against IDOR by retrieving userId from session.
+ */
+export async function getChat(id: string): Promise<DrizzleChat | null> {
+  const userId = await getCurrentUserIdOnServer()
   try {
-    const chat = await dbGetChat(id, userId)
+    // dbGetChat verifies ownership or public visibility
+    const chat = await dbGetChat(id, userId || 'anonymous')
     return chat
   } catch (error) {
     console.error(`Error fetching chat ${id} from DB:`, error)
@@ -50,32 +57,44 @@ export async function getChat(id: string, userId: string): Promise<DrizzleChat |
 }
 
 /**
- * Retrieves all messages for a specific chat.
+ * Retrieves all messages for a specific chat, ensuring the current user has access.
+ * Protected against IDOR by verifying chat access before fetching messages.
  */
 export async function getChatMessages(chatId: string): Promise<DrizzleMessage[]> {
   if (!chatId) {
     console.warn('getChatMessages called without chatId');
     return [];
   }
+
+  const userId = await getCurrentUserIdOnServer();
+
   try {
+    // Verify user has access to this chat before returning messages
+    const chat = await dbGetChat(chatId, userId || 'anonymous');
+    if (!chat) {
+      console.warn(`getChatMessages: Unauthorized access attempt to chat ${chatId}`);
+      return [];
+    }
+
     return dbGetMessagesByChatId(chatId);
   } catch (error) {
     console.error(`Error fetching messages for chat ${chatId} in getChatMessages:`, error);
     return [];
   }
 }
 
-export async function clearChats(
-  userId?: string | null
-): Promise<{ error?: string } | void> {
-  const currentUserId = userId || (await getCurrentUserIdOnServer())
-  if (!currentUserId) {
-    console.error('clearChats: No user ID provided or found.')
-    return { error: 'User ID is required to clear chats' }
+/**
+ * Clears chat history for the current authenticated user.
+ */
+export async function clearChats(): Promise<{ error?: string } | void> {
+  const userId = await getCurrentUserIdOnServer()
+  if (!userId) {
+    console.error('clearChats: No authenticated user found.')
+    return { error: 'Authentication required to clear chats' }
   }
 
   try {
-    const success = await dbClearHistory(currentUserId)
+    const success = await dbClearHistory(userId)
     if (!success) {
       return { error: 'Failed to clear chats from database.' }
     }
@@ -87,16 +106,19 @@ export async function clearChats(
   }
 }
 
-export async function saveChat(chat: OldChatType, userId: string): Promise<string | null> {
-  if (!userId && !chat.userId) {
-    console.error('saveChat: userId is required either as a parameter or in chat object.')
-    return null;
+/**
+ * Saves a chat for the current authenticated user.
+ */
+export async function saveChat(chat: OldChatType): Promise<string | null> {
+  const userId = await getCurrentUserIdOnServer()
+  if (!userId) {
+    console.error('saveChat: User not authenticated.')
+    return null
   }
-  const effectiveUserId = userId || chat.userId;
 
   const newChatData: DbNewChat = {
     id: chat.id,
-    userId: effectiveUserId,
+    userId: userId,
     title: chat.title || 'Untitled Chat',
     createdAt: chat.createdAt ? new Date(chat.createdAt) : new Date(),
     visibility: 'private',
@@ -105,7 +127,7 @@ export async function saveChat(chat: OldChatType, userId: string): Promise<strin
 
   const newMessagesData: Omit<DbNewMessage, 'chatId'>[] = chat.messages.map(msg => ({
     id: msg.id,
-    userId: effectiveUserId,
+    userId: userId,
     role: msg.role,
     content: typeof msg.content === 'object' ? JSON.stringify(msg.content) : msg.content,
     createdAt: msg.createdAt ? new Date(msg.createdAt) : new Date(),
@@ -120,6 +142,9 @@ export async function saveChat(chat: OldChatType, userId: string): Promise<strin
   }
 }
 
+/**
+ * Updates drawing context for a chat.
+ */
 export async function updateDrawingContext(chatId: string, contextData: { drawnFeatures: any[], cameraState: any }) {
   const userId = await getCurrentUserIdOnServer();
   if (!userId) {
@@ -147,11 +172,14 @@ export async function updateDrawingContext(chatId: string, contextData: { drawnF
   }
 }
 
+/**
+ * Saves the system prompt for the current authenticated user.
+ */
 export async function saveSystemPrompt(
-  userId: string,
   prompt: string
 ): Promise<{ success?: boolean; error?: string }> {
-  if (!userId) return { error: 'User ID is required' }
+  const userId = await getCurrentUserIdOnServer()
+  if (!userId) return { error: 'Authentication required' }
   if (!prompt) return { error: 'Prompt is required' }
 
   try {
@@ -166,9 +194,11 @@ export async function saveSystemPrompt(
   }
 }
 
-export async function getSystemPrompt(
-  userId: string
-): Promise<string | null> {
+/**
+ * Retrieves the system prompt for the current authenticated user.
+ */
+export async function getSystemPrompt(): Promise<string | null> {
+  const userId = await getCurrentUserIdOnServer()
   if (!userId) return null
 
   try {