Fix security and quality issues from code review

Critical: Add company scoping to line item update/delete and note
delete via ownership verification through ticket join. Add companyId
validation to signed URL file serving. High: Paginate notes list
endpoint with search and sort support. Fix blob URL memory leaks in
AuthImage components with proper cleanup on unmount. Improve photo
upload error handling — count failures and show specific error count
instead of silently clearing form.

packages/admin/src/api/repairs.ts

@@ -98,7 +98,7 @@ export const repairNoteKeys = {
 export function repairNoteListOptions(ticketId: string) {
   return queryOptions({
     queryKey: repairNoteKeys.all(ticketId),
-    queryFn: () => api.get<{ data: RepairNote[] }>(`/v1/repair-tickets/${ticketId}/notes`),
+    queryFn: () => api.get<PaginatedResponse<RepairNote>>(`/v1/repair-tickets/${ticketId}/notes`, { page: 1, limit: 100, order: 'asc' }),
     enabled: !!ticketId,
   })
 }

packages/admin/src/components/repairs/ticket-notes.tsx

@@ -56,6 +56,7 @@ function AuthImage({ path, alt, className, onClick }: { path: string; alt: strin
 
   useEffect(() => {
     let cancelled = false
+    let blobUrl: string | null = null
     async function load() {
       try {
         const res = await fetch(`/v1/files/serve/${path}`, {
@@ -63,11 +64,17 @@ function AuthImage({ path, alt, className, onClick }: { path: string; alt: strin
         })
         if (!res.ok || cancelled) return
         const blob = await res.blob()
-        if (!cancelled) setSrc(URL.createObjectURL(blob))
+        if (!cancelled) {
+          blobUrl = URL.createObjectURL(blob)
+          setSrc(blobUrl)
+        }
       } catch { /* ignore */ }
     }
     load()
-    return () => { cancelled = true }
+    return () => {
+      cancelled = true
+      if (blobUrl) URL.revokeObjectURL(blobUrl)
+    }
   }, [path, token])
 
   if (!src) return <div className={`${className} bg-muted animate-pulse`} />
@@ -110,6 +117,7 @@ export function TicketNotes({ ticketId }: TicketNotesProps) {
       const note = await repairNoteMutations.create(ticketId, { content: content.trim(), visibility })
 
       // Upload attached photos to the note
+      let uploadFailures = 0
       for (const photo of photos) {
         const formData = new FormData()
         formData.append('entityType', 'repair_note')
@@ -122,16 +130,20 @@ export function TicketNotes({ ticketId }: TicketNotesProps) {
           body: formData,
         })
         if (!uploadRes.ok) {
+          uploadFailures++
           const err = await uploadRes.json().catch(() => ({}))
           console.error('Photo upload failed:', err)
-          toast.error(`Photo upload failed: ${(err as any).error?.message ?? 'Unknown error'}`)
         }
       }
 
       queryClient.invalidateQueries({ queryKey: repairNoteKeys.all(ticketId) })
       setContent('')
       setPhotos([])
-      toast.success('Note added')
+      if (uploadFailures > 0) {
+        toast.error(`Note added but ${uploadFailures} photo(s) failed to upload`)
+      } else {
+        toast.success(photos.length > 0 ? `Note added with ${photos.length} photo(s)` : 'Note added')
+      }
     } catch (err) {
       toast.error(err instanceof Error ? err.message : 'Failed to post note')
     } finally {

packages/admin/src/components/repairs/ticket-photos.tsx

@@ -13,6 +13,7 @@ function AuthImage({ path, alt, className, onClick }: { path: string; alt: strin
 
   useEffect(() => {
     let cancelled = false
+    let blobUrl: string | null = null
     async function load() {
       try {
         const res = await fetch(`/v1/files/serve/${path}`, {
@@ -20,11 +21,17 @@ function AuthImage({ path, alt, className, onClick }: { path: string; alt: strin
         })
         if (!res.ok || cancelled) return
         const blob = await res.blob()
-        if (!cancelled) setSrc(URL.createObjectURL(blob))
+        if (!cancelled) {
+          blobUrl = URL.createObjectURL(blob)
+          setSrc(blobUrl)
+        }
       } catch { /* ignore */ }
     }
     load()
-    return () => { cancelled = true }
+    return () => {
+      cancelled = true
+      if (blobUrl) URL.revokeObjectURL(blobUrl)
+    }
   }, [path, token])
 
   if (!src) return <div className={`${className} bg-muted animate-pulse`} />

packages/backend/api-tests/suites/repairs.ts

@@ -296,14 +296,16 @@ suite('Repairs', { tags: ['repairs'] }, (t) => {
     t.assert.equal(res.data.visibility, 'customer')
   })
 
-  t.test('lists notes for a ticket in chronological order', { tags: ['notes', 'read'] }, async () => {
+  t.test('lists notes for a ticket with pagination', { tags: ['notes', 'read'] }, async () => {
     const ticket = await t.api.post('/v1/repair-tickets', { customerName: 'List Notes', problemDescription: 'Test' })
     await t.api.post(`/v1/repair-tickets/${ticket.data.id}/notes`, { content: 'First note' })
     await t.api.post(`/v1/repair-tickets/${ticket.data.id}/notes`, { content: 'Second note' })
 
-    const res = await t.api.get(`/v1/repair-tickets/${ticket.data.id}/notes`)
+    const res = await t.api.get(`/v1/repair-tickets/${ticket.data.id}/notes`, { limit: 100 })
     t.assert.status(res, 200)
     t.assert.equal(res.data.data.length, 2)
+    t.assert.ok(res.data.pagination)
+    t.assert.equal(res.data.pagination.total, 2)
     t.assert.equal(res.data.data[0].content, 'First note')
     t.assert.equal(res.data.data[1].content, 'Second note')
   })
@@ -323,7 +325,7 @@ suite('Repairs', { tags: ['repairs'] }, (t) => {
     const res = await t.api.del(`/v1/repair-notes/${note.data.id}`)
     t.assert.status(res, 200)
 
-    const list = await t.api.get(`/v1/repair-tickets/${ticket.data.id}/notes`)
+    const list = await t.api.get(`/v1/repair-tickets/${ticket.data.id}/notes`, { limit: 100 })
     t.assert.equal(list.data.data.length, 0)
   })
 

packages/backend/src/routes/v1/files.ts

@@ -143,6 +143,10 @@ export const fileRoutes: FastifyPluginAsync = async (app) => {
       if (payload.purpose !== 'file-access' || payload.path !== filePath) {
         return reply.status(403).send({ error: { message: 'Invalid token', statusCode: 403 } })
       }
+      // Validate company isolation — file path must start with the token's companyId
+      if (payload.companyId && !filePath.startsWith(payload.companyId)) {
+        return reply.status(403).send({ error: { message: 'Access denied', statusCode: 403 } })
+      }
     } catch {
       return reply.status(403).send({ error: { message: 'Token expired or invalid', statusCode: 403 } })
     }

packages/backend/src/routes/v1/repairs.ts

@@ -109,14 +109,14 @@ export const repairRoutes: FastifyPluginAsync = async (app) => {
     if (!parsed.success) {
       return reply.status(400).send({ error: { message: 'Validation failed', details: parsed.error.flatten(), statusCode: 400 } })
     }
-    const item = await RepairLineItemService.update(app.db, id, parsed.data)
+    const item = await RepairLineItemService.update(app.db, request.companyId, id, parsed.data)
     if (!item) return reply.status(404).send({ error: { message: 'Line item not found', statusCode: 404 } })
     return reply.send(item)
   })
 
   app.delete('/repair-line-items/:id', { preHandler: [app.authenticate, app.requirePermission('repairs.admin')] }, async (request, reply) => {
     const { id } = request.params as { id: string }
-    const item = await RepairLineItemService.delete(app.db, id)
+    const item = await RepairLineItemService.delete(app.db, request.companyId, id)
     if (!item) return reply.status(404).send({ error: { message: 'Line item not found', statusCode: 404 } })
     return reply.send(item)
   })
@@ -210,13 +210,14 @@ export const repairRoutes: FastifyPluginAsync = async (app) => {
 
   app.get('/repair-tickets/:ticketId/notes', { preHandler: [app.authenticate, app.requirePermission('repairs.view')] }, async (request, reply) => {
     const { ticketId } = request.params as { ticketId: string }
-    const notes = await RepairNoteService.listByTicket(app.db, ticketId)
+    const params = PaginationSchema.parse(request.query)
-    return reply.send({ data: notes })
+    const result = await RepairNoteService.listByTicket(app.db, ticketId, params)
+    return reply.send(result)
   })
 
   app.delete('/repair-notes/:id', { preHandler: [app.authenticate, app.requirePermission('repairs.admin')] }, async (request, reply) => {
     const { id } = request.params as { id: string }
-    const note = await RepairNoteService.delete(app.db, id)
+    const note = await RepairNoteService.delete(app.db, request.companyId, id)
     if (!note) return reply.status(404).send({ error: { message: 'Note not found', statusCode: 404 } })
     return reply.send(note)
   })

packages/backend/src/services/repair.service.ts

@@ -254,7 +254,19 @@ export const RepairLineItemService = {
     return paginatedResponse(data, total, params.page, params.limit)
   },
 
-  async update(db: PostgresJsDatabase<any>, id: string, input: RepairLineItemUpdateInput) {
+  async verifyOwnership(db: PostgresJsDatabase<any>, companyId: string, lineItemId: string): Promise<boolean> {
+    const [item] = await db
+      .select({ ticketCompanyId: repairTickets.companyId })
+      .from(repairLineItems)
+      .innerJoin(repairTickets, eq(repairLineItems.repairTicketId, repairTickets.id))
+      .where(and(eq(repairLineItems.id, lineItemId), eq(repairTickets.companyId, companyId)))
+      .limit(1)
+    return !!item
+  },
+
+  async update(db: PostgresJsDatabase<any>, companyId: string, id: string, input: RepairLineItemUpdateInput) {
+    if (!(await this.verifyOwnership(db, companyId, id))) return null
+
     const values: Record<string, unknown> = { ...input }
     if (input.qty !== undefined) values.qty = input.qty.toString()
     if (input.unitPrice !== undefined) values.unitPrice = input.unitPrice.toString()
@@ -269,7 +281,9 @@ export const RepairLineItemService = {
     return item ?? null
   },
 
-  async delete(db: PostgresJsDatabase<any>, id: string) {
+  async delete(db: PostgresJsDatabase<any>, companyId: string, id: string) {
+    if (!(await this.verifyOwnership(db, companyId, id))) return null
+
     const [item] = await db
       .delete(repairLineItems)
       .where(eq(repairLineItems.id, id))
@@ -476,15 +490,40 @@ export const RepairNoteService = {
     return note
   },
 
-  async listByTicket(db: PostgresJsDatabase<any>, ticketId: string) {
+  async listByTicket(db: PostgresJsDatabase<any>, ticketId: string, params: PaginationInput) {
-    return db
+    const baseWhere = eq(repairNotes.repairTicketId, ticketId)
-      .select()
+    const searchCondition = params.q
-      .from(repairNotes)
+      ? buildSearchCondition(params.q, [repairNotes.content, repairNotes.authorName])
-      .where(eq(repairNotes.repairTicketId, ticketId))
+      : undefined
-      .orderBy(repairNotes.createdAt)
+    const where = searchCondition ? and(baseWhere, searchCondition) : baseWhere
+
+    const sortableColumns: Record<string, Column> = {
+      created_at: repairNotes.createdAt,
+      author_name: repairNotes.authorName,
+    }
+
+    let query = db.select().from(repairNotes).where(where).$dynamic()
+    query = withSort(query, params.sort, params.order, sortableColumns, repairNotes.createdAt)
+    query = withPagination(query, params.page, params.limit)
+
+    const [data, [{ total }]] = await Promise.all([
+      query,
+      db.select({ total: count() }).from(repairNotes).where(where),
+    ])
+
+    return paginatedResponse(data, total, params.page, params.limit)
   },
 
-  async delete(db: PostgresJsDatabase<any>, id: string) {
+  async delete(db: PostgresJsDatabase<any>, companyId: string, id: string) {
+    // Verify note belongs to a ticket owned by this company
+    const [owned] = await db
+      .select({ id: repairNotes.id })
+      .from(repairNotes)
+      .innerJoin(repairTickets, eq(repairNotes.repairTicketId, repairTickets.id))
+      .where(and(eq(repairNotes.id, id), eq(repairTickets.companyId, companyId)))
+      .limit(1)
+    if (!owned) return null
+
     const [note] = await db
       .delete(repairNotes)
       .where(eq(repairNotes.id, id))