fix(M2-005,M2-006): enforce user ownership at repo level for conversations and agents (#293)

Co-authored-by: Jason Woltje <jason@diversecanvas.com>
Co-committed-by: Jason Woltje <jason@diversecanvas.com>

apps/gateway/src/__tests__/resource-ownership.test.ts

@@ -57,11 +57,13 @@ function createBrain() {
 describe('Resource ownership checks', () => {
   it('forbids access to another user conversation', async () => {
     const brain = createBrain();
-    brain.conversations.findById.mockResolvedValue({ id: 'conv-1', userId: 'user-2' });
+    // The repo enforces ownership via the WHERE clause; it returns undefined when the
+    // conversation does not belong to the requesting user.
+    brain.conversations.findById.mockResolvedValue(undefined);
     const controller = new ConversationsController(brain as never);
 
     await expect(controller.findOne('conv-1', { id: 'user-1' })).rejects.toBeInstanceOf(
-      ForbiddenException,
+      NotFoundException,
     );
   });
 

apps/gateway/src/agent/agent-configs.controller.ts

@@ -62,7 +62,11 @@ export class AgentConfigsController {
     if (!agent.isSystem && agent.ownerId !== user.id) {
       throw new ForbiddenException('Agent does not belong to the current user');
     }
-    const updated = await this.brain.agents.update(id, dto);
+
+    // Pass ownerId for user agents so the repo WHERE clause enforces ownership.
+    // For system agents (admin path) pass undefined so the WHERE matches only on id.
+    const ownerId = agent.isSystem ? undefined : user.id;
+    const updated = await this.brain.agents.update(id, dto, ownerId);
     if (!updated) throw new NotFoundException('Agent not found');
     return updated;
   }
@@ -78,7 +82,8 @@ export class AgentConfigsController {
     if (agent.ownerId !== user.id) {
       throw new ForbiddenException('Agent does not belong to the current user');
     }
-    const deleted = await this.brain.agents.remove(id);
+    // Pass ownerId so the repo WHERE clause enforces ownership at the DB level.
+    const deleted = await this.brain.agents.remove(id, user.id);
     if (!deleted) throw new NotFoundException('Agent not found');
   }
 }

apps/gateway/src/conversations/conversations.controller.ts

@@ -2,6 +2,7 @@ import {
   Body,
   Controller,
   Delete,
+  ForbiddenException,
   Get,
   HttpCode,
   HttpStatus,
@@ -16,7 +17,6 @@ import type { Brain } from '@mosaic/brain';
 import { BRAIN } from '../brain/brain.tokens.js';
 import { AuthGuard } from '../auth/auth.guard.js';
 import { CurrentUser } from '../auth/current-user.decorator.js';
-import { assertOwner } from '../auth/resource-ownership.js';
 import {
   CreateConversationDto,
   UpdateConversationDto,
@@ -35,7 +35,9 @@ export class ConversationsController {
 
   @Get(':id')
   async findOne(@Param('id') id: string, @CurrentUser() user: { id: string }) {
-    return this.getOwnedConversation(id, user.id);
+    const conversation = await this.brain.conversations.findById(id, user.id);
+    if (!conversation) throw new NotFoundException('Conversation not found');
+    return conversation;
   }
 
   @Post()
@@ -53,8 +55,7 @@ export class ConversationsController {
     @Body() dto: UpdateConversationDto,
     @CurrentUser() user: { id: string },
   ) {
-    await this.getOwnedConversation(id, user.id);
-    const conversation = await this.brain.conversations.update(id, dto);
+    const conversation = await this.brain.conversations.update(id, user.id, dto);
     if (!conversation) throw new NotFoundException('Conversation not found');
     return conversation;
   }
@@ -62,15 +63,16 @@ export class ConversationsController {
   @Delete(':id')
   @HttpCode(HttpStatus.NO_CONTENT)
   async remove(@Param('id') id: string, @CurrentUser() user: { id: string }) {
-    await this.getOwnedConversation(id, user.id);
-    const deleted = await this.brain.conversations.remove(id);
+    const deleted = await this.brain.conversations.remove(id, user.id);
     if (!deleted) throw new NotFoundException('Conversation not found');
   }
 
   @Get(':id/messages')
   async listMessages(@Param('id') id: string, @CurrentUser() user: { id: string }) {
-    await this.getOwnedConversation(id, user.id);
-    return this.brain.conversations.findMessages(id);
+    // Verify ownership explicitly to return a clear 404 rather than an empty list.
+    const conversation = await this.brain.conversations.findById(id, user.id);
+    if (!conversation) throw new NotFoundException('Conversation not found');
+    return this.brain.conversations.findMessages(id, user.id);
   }
 
   @Post(':id/messages')
@@ -79,19 +81,16 @@ export class ConversationsController {
     @Body() dto: SendMessageDto,
     @CurrentUser() user: { id: string },
   ) {
-    await this.getOwnedConversation(id, user.id);
-    return this.brain.conversations.addMessage({
-      conversationId: id,
-      role: dto.role,
-      content: dto.content,
-      metadata: dto.metadata,
-    });
-  }
-
-  private async getOwnedConversation(id: string, userId: string) {
-    const conversation = await this.brain.conversations.findById(id);
-    if (!conversation) throw new NotFoundException('Conversation not found');
-    assertOwner(conversation.userId, userId, 'Conversation');
-    return conversation;
+    const message = await this.brain.conversations.addMessage(
+      {
+        conversationId: id,
+        role: dto.role,
+        content: dto.content,
+        metadata: dto.metadata,
+      },
+      user.id,
+    );
+    if (!message) throw new ForbiddenException('Conversation not found or access denied');
+    return message;
   }
 }

packages/brain/src/agents.ts

@@ -1,4 +1,4 @@
-import { eq, or, type Db, agents } from '@mosaic/db';
+import { eq, and, or, type Db, agents } from '@mosaic/db';
 
 export type Agent = typeof agents.$inferSelect;
 export type NewAgent = typeof agents.$inferInsert;
@@ -27,6 +27,10 @@ export function createAgentsRepo(db: Db) {
       return db.select().from(agents).where(eq(agents.isSystem, true));
     },
 
+    /**
+     * Return only agents the user may access: their own agents plus all system agents.
+     * Never returns other users' private agents.
+     */
     async findAccessible(ownerId: string): Promise<Agent[]> {
       return db
         .select()
@@ -39,17 +43,44 @@ export function createAgentsRepo(db: Db) {
       return rows[0]!;
     },
 
-    async update(id: string, data: Partial<NewAgent>): Promise<Agent | undefined> {
+    /**
+     * Update an agent.
+     *
+     * For user-owned agents pass `ownerId` — the WHERE clause will enforce ownership so that
+     * one user cannot overwrite another user's agent.  For system agents the caller must
+     * omit `ownerId` (admin-only path) and the WHERE clause only matches on `id`.
+     *
+     * Returns undefined when no row was matched (not found or ownership mismatch).
+     */
+    async update(
+      id: string,
+      data: Partial<NewAgent>,
+      ownerId?: string,
+    ): Promise<Agent | undefined> {
+      const condition =
+        ownerId !== undefined
+          ? and(eq(agents.id, id), eq(agents.ownerId, ownerId))
+          : eq(agents.id, id);
+
       const rows = await db
         .update(agents)
         .set({ ...data, updatedAt: new Date() })
-        .where(eq(agents.id, id))
+        .where(condition)
         .returning();
       return rows[0];
     },
 
-    async remove(id: string): Promise<boolean> {
-      const rows = await db.delete(agents).where(eq(agents.id, id)).returning();
+    /**
+     * Delete a user-owned agent, scoped to the given owner.
+     * Will not match system agents even if the id is correct, because system agents have
+     * `ownerId = null` which cannot equal a real user id.
+     * Returns false when no row was matched (not found, wrong owner, or system agent).
+     */
+    async remove(id: string, ownerId: string): Promise<boolean> {
+      const rows = await db
+        .delete(agents)
+        .where(and(eq(agents.id, id), eq(agents.ownerId, ownerId)))
+        .returning();
       return rows.length > 0;
     },
   };

packages/brain/src/conversations.ts

@@ -1,4 +1,4 @@
-import { eq, asc, desc, type Db, conversations, messages } from '@mosaic/db';
+import { eq, and, asc, desc, type Db, conversations, messages } from '@mosaic/db';
 
 /** Maximum number of conversations returned per list query. */
 const MAX_CONVERSATIONS = 200;
@@ -21,8 +21,15 @@ export function createConversationsRepo(db: Db) {
         .limit(MAX_CONVERSATIONS);
     },
 
-    async findById(id: string): Promise<Conversation | undefined> {
-      const rows = await db.select().from(conversations).where(eq(conversations.id, id));
+    /**
+     * Find a conversation by ID, scoped to the given user.
+     * Returns undefined if the conversation does not exist or belongs to a different user.
+     */
+    async findById(id: string, userId: string): Promise<Conversation | undefined> {
+      const rows = await db
+        .select()
+        .from(conversations)
+        .where(and(eq(conversations.id, id), eq(conversations.userId, userId)));
       return rows[0];
     },
 
@@ -31,21 +38,47 @@ export function createConversationsRepo(db: Db) {
       return rows[0]!;
     },
 
-    async update(id: string, data: Partial<NewConversation>): Promise<Conversation | undefined> {
+    /**
+     * Update a conversation, scoped to the given user.
+     * Returns undefined if the conversation does not exist or belongs to a different user.
+     */
+    async update(
+      id: string,
+      userId: string,
+      data: Partial<NewConversation>,
+    ): Promise<Conversation | undefined> {
       const rows = await db
         .update(conversations)
         .set({ ...data, updatedAt: new Date() })
-        .where(eq(conversations.id, id))
+        .where(and(eq(conversations.id, id), eq(conversations.userId, userId)))
         .returning();
       return rows[0];
     },
 
-    async remove(id: string): Promise<boolean> {
-      const rows = await db.delete(conversations).where(eq(conversations.id, id)).returning();
+    /**
+     * Delete a conversation, scoped to the given user.
+     * Returns false if the conversation does not exist or belongs to a different user.
+     */
+    async remove(id: string, userId: string): Promise<boolean> {
+      const rows = await db
+        .delete(conversations)
+        .where(and(eq(conversations.id, id), eq(conversations.userId, userId)))
+        .returning();
       return rows.length > 0;
     },
 
-    async findMessages(conversationId: string): Promise<Message[]> {
+    /**
+     * Find messages for a conversation, scoped to the given user.
+     * Returns an empty array if the conversation does not exist or belongs to a different user.
+     */
+    async findMessages(conversationId: string, userId: string): Promise<Message[]> {
+      // Verify ownership of the parent conversation before returning messages.
+      const conv = await db
+        .select()
+        .from(conversations)
+        .where(and(eq(conversations.id, conversationId), eq(conversations.userId, userId)));
+      if (conv.length === 0) return [];
+
       return db
         .select()
         .from(messages)
@@ -54,7 +87,19 @@ export function createConversationsRepo(db: Db) {
         .limit(MAX_MESSAGES);
     },
 
-    async addMessage(data: NewMessage): Promise<Message> {
+    /**
+     * Add a message to a conversation, scoped to the given user.
+     * Verifies the parent conversation belongs to the user before inserting.
+     * Returns undefined if the conversation does not exist or belongs to a different user.
+     */
+    async addMessage(data: NewMessage, userId: string): Promise<Message | undefined> {
+      // Verify ownership of the parent conversation before inserting the message.
+      const conv = await db
+        .select()
+        .from(conversations)
+        .where(and(eq(conversations.id, data.conversationId), eq(conversations.userId, userId)));
+      if (conv.length === 0) return undefined;
+
       const rows = await db.insert(messages).values(data).returning();
       return rows[0]!;
     },