fix(feedback): harden image delivery

src/main/ipc/feedback-image-attachments.test.ts

@@ -2,9 +2,9 @@ import { describe, expect, it } from 'vitest'
 import {
   MAX_FEEDBACK_IMAGE_BYTES,
   MAX_FEEDBACK_IMAGE_COUNT,
+  appendFeedbackImagesToFormData,
   feedbackImageFilename,
   isSupportedFeedbackImageContentType,
-  normalizeFeedbackImageBytes,
   validateFeedbackImages
 } from './feedback-image-attachments'
 
@@ -27,9 +27,7 @@ describe('isSupportedFeedbackImageContentType', () => {
     'rejects the inherited member %s',
     (contentType) => {
       expect(isSupportedFeedbackImageContentType(contentType)).toBe(false)
-      expect(validateFeedbackImages([image(contentType)])).toBe(
-        `Unsupported image type ${contentType}.`
-      )
+      expect(validateFeedbackImages([image(contentType)])).toBe('Unsupported image type.')
     }
   )
 })
@@ -49,15 +47,17 @@ describe('feedbackImageFilename', () => {
   })
 })
 
-describe('normalizeFeedbackImageBytes', () => {
-  it('preserves exact typed-array bounds without copying the backing buffer', () => {
+describe('appendFeedbackImagesToFormData', () => {
+  it('frames only the exact typed-array view', async () => {
     const backing = new Uint8Array([0, 1, 2, 3])
     const view = backing.subarray(1, 3)
+    const formData = new FormData()
 
-    const normalized = normalizeFeedbackImageBytes(view)
+    appendFeedbackImagesToFormData(formData, [{ contentType: 'image/png', data: view }])
 
-    expect(normalized).toBe(view)
-    expect([...normalized]).toEqual([1, 2])
+    const blob = formData.get('feedbackImage')
+    expect(blob).toBeInstanceOf(Blob)
+    expect(new Uint8Array(await (blob as Blob).arrayBuffer())).toEqual(new Uint8Array([1, 2]))
   })
 })
 
@@ -78,6 +78,14 @@ describe('validateFeedbackImages', () => {
     ).toBe(`Attach ${MAX_FEEDBACK_IMAGE_COUNT} images or fewer.`)
   })
 
+  it('rejects malformed attachment collections and byte payloads', () => {
+    expect(validateFeedbackImages('image/png')).toBe('Image attachments must be a list.')
+    expect(validateFeedbackImages([null])).toBe('Invalid image attachment.')
+    expect(validateFeedbackImages([{ contentType: 'image/png', data: '8388608' }])).toBe(
+      'Invalid image attachment bytes.'
+    )
+  })
+
   it('rejects an empty attachment', () => {
     expect(validateFeedbackImages([image('image/png', 0)])).toBe('Image attachment is empty.')
   })

src/main/ipc/feedback-image-attachments.ts

@@ -1,3 +1,5 @@
+import { readFetchResponseJsonWithinLimit } from '../../shared/fetch-response-body'
+
 // Why: mirrors the server allow-list. Slack picks a renderer from the filename
 // extension, so every accepted type needs one.
 const FEEDBACK_IMAGE_EXTENSIONS: Record<string, string> = {
@@ -9,6 +11,7 @@ const FEEDBACK_IMAGE_EXTENSIONS: Record<string, string> = {
 
 export const MAX_FEEDBACK_IMAGE_COUNT = 4
 export const MAX_FEEDBACK_IMAGE_BYTES = 8 * 1024 * 1024
+export const MAX_FEEDBACK_IMAGE_RESPONSE_BYTES = 64 * 1024
 export const FEEDBACK_IMAGE_FORM_FIELD = 'feedbackImage'
 
 export type FeedbackImageAttachment = {
@@ -26,24 +29,30 @@ export function getSupportedFeedbackImageContentTypes(): string[] {
   return Object.keys(FEEDBACK_IMAGE_EXTENSIONS)
 }
 
-export function normalizeFeedbackImageBytes(data: Uint8Array): Uint8Array {
-  // Why: a Uint8Array already carries exact view bounds; copying it here adds
-  // another 32 MiB at the four-image cap before Blob framing copies it again.
-  return data instanceof Uint8Array ? data : new Uint8Array(data)
-}
-
 export function feedbackImageFilename(index: number, contentType: string): string {
   return `feedback-image-${index + 1}.${FEEDBACK_IMAGE_EXTENSIONS[contentType]}`
 }
 
 /** Defence in depth: the renderer validates first, but this channel is reachable directly. */
-export function validateFeedbackImages(images: FeedbackImageAttachment[]): string | null {
+export function validateFeedbackImages(images: unknown): string | null {
+  if (!Array.isArray(images)) {
+    return 'Image attachments must be a list.'
+  }
   if (images.length > MAX_FEEDBACK_IMAGE_COUNT) {
     return `Attach ${MAX_FEEDBACK_IMAGE_COUNT} images or fewer.`
   }
   for (const image of images) {
+    if (!image || typeof image !== 'object') {
+      return 'Invalid image attachment.'
+    }
+    if (typeof image.contentType !== 'string') {
+      return 'Invalid image attachment content type.'
+    }
     if (!isSupportedFeedbackImageContentType(image.contentType)) {
-      return `Unsupported image type ${image.contentType}.`
+      return 'Unsupported image type.'
+    }
+    if (!(image.data instanceof Uint8Array)) {
+      return 'Invalid image attachment bytes.'
     }
     if (image.data.byteLength === 0) {
       return 'Image attachment is empty.'
@@ -68,13 +77,16 @@ export function appendFeedbackImagesToFormData(
   }
 }
 
-/** Older servers omit this field; any settled 2xx still confirms the feedback text landed. */
+/** A 2xx confirms the text; only the response contract can confirm its images. */
 export async function readFeedbackImagesDelivered(response: Response): Promise<boolean> {
   try {
-    const parsed: unknown = await response.json()
+    const parsed: unknown = await readFetchResponseJsonWithinLimit(
+      response,
+      MAX_FEEDBACK_IMAGE_RESPONSE_BYTES
+    )
     if (typeof parsed === 'object' && parsed !== null && 'imagesDelivered' in parsed) {
-      return (parsed as { imagesDelivered?: unknown }).imagesDelivered !== false
+      return (parsed as { imagesDelivered?: unknown }).imagesDelivered === true
     }
   } catch {}
-  return true
+  return false
 }

src/main/ipc/feedback.test.ts

@@ -16,6 +16,7 @@ vi.mock('electron', () => ({
   net: { fetch: (...args: unknown[]) => fetchMock(...args) }
 }))
 
+import { MAX_FEEDBACK_IMAGE_RESPONSE_BYTES } from './feedback-image-attachments'
 import { registerFeedbackHandlers, submitFeedback } from './feedback'
 
 function okResponse(): Response {
@@ -446,16 +447,16 @@ describe('submitFeedback', () => {
       })
     })
 
-    it('treats a 2xx without the field as delivered so older servers still succeed', async () => {
+    it('reports unconfirmed delivery when a 2xx omits the image result', async () => {
       fetchMock.mockResolvedValue(jsonResponse({ ok: true }))
 
       await expect(submitFeedback(imageSubmitArgs([pngImage()]))).resolves.toEqual({
         ok: true,
-        imagesDelivered: true
+        imagesDelivered: false
       })
     })
 
-    it('treats a settled non-JSON 2xx as delivered for legacy servers', async () => {
+    it('reports unconfirmed delivery for a settled non-JSON 2xx', async () => {
       fetchMock.mockResolvedValue({
         ok: true,
         status: 202,
@@ -466,7 +467,34 @@ describe('submitFeedback', () => {
 
       await expect(submitFeedback(imageSubmitArgs([pngImage()]))).resolves.toEqual({
         ok: true,
-        imagesDelivered: true
+        imagesDelivered: false
+      })
+    })
+
+    it('reports unconfirmed delivery when the response body aborts before the deadline', async () => {
+      fetchMock.mockResolvedValue({
+        ok: true,
+        status: 202,
+        json: async () => {
+          throw new TypeError('terminated')
+        }
+      } as unknown as Response)
+
+      await expect(submitFeedback(imageSubmitArgs([pngImage()]))).resolves.toEqual({
+        ok: true,
+        imagesDelivered: false
+      })
+      expect(requestInit().signal).toMatchObject({ aborted: false })
+    })
+
+    it('bounds the image-delivery response body', async () => {
+      fetchMock.mockResolvedValue(
+        new Response('x'.repeat(MAX_FEEDBACK_IMAGE_RESPONSE_BYTES + 1), { status: 202 })
+      )
+
+      await expect(submitFeedback(imageSubmitArgs([pngImage()]))).resolves.toEqual({
+        ok: true,
+        imagesDelivered: false
       })
     })
 
@@ -514,7 +542,36 @@ describe('submitFeedback', () => {
         images: [{ contentType: 'constructor', data: new Uint8Array(4).fill(1) }]
       })) as { ok: boolean; error?: string }
 
-      expect(result).toMatchObject({ ok: false, error: 'Unsupported image type constructor.' })
+      expect(result).toMatchObject({ ok: false, error: 'Unsupported image type.' })
+      expect(fetchMock).not.toHaveBeenCalled()
+    })
+
+    it('rejects malformed IPC bytes before typed-array normalization', async () => {
+      registerFeedbackHandlers()
+      const result = (await handlers.get('feedback:submit')?.(null, {
+        feedback: 'images attached',
+        githubLogin: null,
+        githubEmail: null,
+        images: [{ contentType: 'image/png', data: '8388608' }]
+      })) as { ok: boolean; error?: string }
+
+      expect(result).toMatchObject({ ok: false, error: 'Invalid image attachment bytes.' })
+      expect(fetchMock).not.toHaveBeenCalled()
+    })
+
+    it('rejects oversized IPC batches before normalizing their entries', async () => {
+      registerFeedbackHandlers()
+      const result = (await handlers.get('feedback:submit')?.(null, {
+        feedback: 'images attached',
+        githubLogin: null,
+        githubEmail: null,
+        images: Array.from({ length: 5 }, () => ({
+          contentType: 'image/png',
+          data: '8388608'
+        }))
+      })) as { ok: boolean; error?: string }
+
+      expect(result).toMatchObject({ ok: false, error: 'Attach 4 images or fewer.' })
       expect(fetchMock).not.toHaveBeenCalled()
     })
 

src/main/ipc/feedback.ts

@@ -2,7 +2,6 @@ import os from 'node:os'
 import { app, ipcMain, net } from 'electron'
 import {
   appendFeedbackImagesToFormData,
-  normalizeFeedbackImageBytes,
   readFeedbackImagesDelivered,
   validateFeedbackImages,
   type FeedbackImageAttachment
@@ -285,7 +284,7 @@ export async function submitFeedback(
 ): Promise<FeedbackSubmitResult> {
   // Why: buildSubmitBody drops images on the crash lane, so validating them
   // there would abort a crash report over attachments it never meant to send.
-  if (args.submissionType !== 'crash' && args.images?.length) {
+  if (args.submissionType !== 'crash' && args.images !== undefined) {
     const imageError = validateFeedbackImages(args.images)
     if (imageError) {
       return { ok: false, status: null, error: imageError }
@@ -343,16 +342,20 @@ export async function submitFeedback(
 
 export function registerFeedbackHandlers(): void {
   ipcMain.removeHandler('feedback:submit')
-  ipcMain.handle('feedback:submit', (_event, args: FeedbackSubmitArgs) =>
+  ipcMain.handle('feedback:submit', (_event, args: FeedbackSubmitArgs) => {
+    // Why: validate the raw clone before normalization so a tiny hostile value
+    // cannot become a large main-process typed-array allocation.
+    if (args.images !== undefined) {
+      const imageError = validateFeedbackImages(args.images)
+      if (imageError) {
+        return { ok: false, status: null, error: imageError }
+      }
+    }
     // Why: crash submissions are main-only. A compromised renderer can invoke
     // this channel directly, so force the public feedback lane at the boundary.
-    submitFeedback({
+    return submitFeedback({
       ...args,
-      submissionType: 'feedback',
-      images: args.images?.map((image) => ({
-        contentType: image.contentType,
-        data: normalizeFeedbackImageBytes(image.data)
-      }))
+      submissionType: 'feedback'
     })
-  )
+  })
 }

src/renderer/src/components/sidebar/SidebarFeedbackDialog.test.tsx

@@ -74,6 +74,14 @@ afterEach(() => {
 })
 
 describe('SidebarFeedbackDialog image submission', () => {
+  it('keeps the dialog scrollable within short windows', () => {
+    const { container } = render(<SidebarFeedbackDialog open onOpenChange={vi.fn()} />)
+    const content = container.querySelector('.scrollbar-sleek')
+
+    expect(content?.className).toContain('max-h-[calc(100vh-3rem)]')
+    expect(content?.className).toContain('overflow-y-auto')
+  })
+
   it('waits for in-flight image reads before enabling Send', async () => {
     let finishRead: ((value: unknown) => void) | undefined
     mocks.readFeedbackImageFiles.mockReturnValue(
@@ -112,7 +120,9 @@ describe('SidebarFeedbackDialog image submission', () => {
     })
 
     await waitFor(() => expect((send as HTMLButtonElement).disabled).toBe(false))
-    expect(screen.getByRole('button', { name: 'Remove shot.png' })).toBeTruthy()
+    const remove = screen.getByRole('button', { name: 'Remove shot.png' })
+    expect(remove.dataset.slot).toBe('button')
+    expect(remove.dataset.size).toBe('icon-xs')
     fireEvent.click(send)
     await waitFor(() => expect(mocks.submit).toHaveBeenCalledTimes(1))
     expect(mocks.submit.mock.calls[0]?.[0].images).toEqual([
@@ -120,6 +130,42 @@ describe('SidebarFeedbackDialog image submission', () => {
     ])
   })
 
+  it('warns when the server cannot confirm image delivery', async () => {
+    mocks.readFeedbackImageFiles.mockResolvedValue({
+      images: [
+        {
+          id: 'shot',
+          name: 'shot.png',
+          contentType: 'image/png',
+          bytes: 1,
+          data: new Uint8Array([1]),
+          previewUrl: 'blob:shot'
+        }
+      ],
+      errors: []
+    })
+    mocks.submit.mockResolvedValue({ ok: true, imagesDelivered: false })
+    const onOpenChange = vi.fn()
+    const { container } = render(<SidebarFeedbackDialog open onOpenChange={onOpenChange} />)
+    fireEvent.change(screen.getByPlaceholderText('What could we improve?'), {
+      target: { value: 'Screenshot attached' }
+    })
+    const input = container.querySelector<HTMLInputElement>('input[type="file"]')
+    fireEvent.change(input!, {
+      target: { files: [new File(['x'], 'shot.png', { type: 'image/png' })] }
+    })
+    await screen.findByRole('button', { name: 'Remove shot.png' })
+
+    fireEvent.click(screen.getByRole('button', { name: 'Send' }))
+
+    await waitFor(() =>
+      expect(mocks.toastWarning).toHaveBeenCalledWith(
+        'Feedback sent, but image delivery could not be confirmed.'
+      )
+    )
+    expect(onOpenChange).toHaveBeenCalledWith(false)
+  })
+
   it('does not consume text when the pasted image cannot be attached', () => {
     mocks.readFeedbackImageFiles.mockResolvedValue({
       images: [],

src/renderer/src/components/sidebar/SidebarFeedbackDialog.tsx

@@ -229,7 +229,7 @@ export function SidebarFeedbackDialog({
           toast.warning(
             translate(
               'auto.components.sidebar.SidebarFeedbackDialog.imagesNotDelivered',
-              'Feedback sent, but the images could not be attached.'
+              'Feedback sent, but image delivery could not be confirmed.'
             )
           )
         } else {
@@ -266,7 +266,7 @@ export function SidebarFeedbackDialog({
     <Dialog open={open} onOpenChange={onOpenChange}>
       <DialogContent
         ref={contentRef}
-        className="sm:max-w-lg"
+        className="max-h-[calc(100vh-3rem)] overflow-y-auto scrollbar-sleek sm:max-w-lg"
         onOpenAutoFocus={(event) => {
           event.preventDefault()
           feedbackTextareaRef.current?.focus()

src/renderer/src/components/sidebar/SidebarFeedbackImageAttachments.tsx

@@ -80,8 +80,10 @@ export function SidebarFeedbackImageAttachments({
                 alt={image.name}
                 className="size-14 rounded border border-border object-cover"
               />
-              <button
+              <Button
                 type="button"
+                variant="outline"
+                size="icon-xs"
                 aria-label={translate(
                   'auto.components.sidebar.SidebarFeedbackImageAttachments.removeImage',
                   'Remove {{fileName}}',
@@ -89,10 +91,10 @@ export function SidebarFeedbackImageAttachments({
                 )}
                 disabled={disabled}
                 onClick={() => onRemove(image.id)}
-                className="absolute -right-1.5 -top-1.5 flex size-4 items-center justify-center rounded-full border border-border bg-background text-muted-foreground opacity-0 transition-opacity hover:text-foreground focus-visible:opacity-100 group-hover/attachment:opacity-100"
+                className="absolute -right-2 -top-2 rounded-full text-muted-foreground opacity-80 hover:opacity-100 hover:text-foreground focus-visible:opacity-100"
               >
                 <X className="size-2.5" />
-              </button>
+              </Button>
               <span className="mt-0.5 block text-center text-[10px] leading-none text-muted-foreground">
                 {formatFeedbackImageSize(image.bytes)}
               </span>

src/renderer/src/i18n/locales/en.json

@@ -4228,7 +4228,7 @@
           "69969ba364": "Sending…",
           "imageReadFailed": "Could not read the attached images. Try attaching them again.",
           "attachWhileSending": "Wait for the current feedback to finish sending before attaching more images.",
-          "imagesNotDelivered": "Feedback sent, but the images could not be attached."
+          "imagesNotDelivered": "Feedback sent, but image delivery could not be confirmed."
         },
         "SidebarFilter": {
           "e3b3898218": "Add project",

src/renderer/src/i18n/locales/es.json

@@ -4190,7 +4190,7 @@
           "69969ba364": "Envío…",
           "imageReadFailed": "Could not read the attached images. Try attaching them again.",
           "attachWhileSending": "Wait for the current feedback to finish sending before attaching more images.",
-          "imagesNotDelivered": "Feedback sent, but the images could not be attached."
+          "imagesNotDelivered": "Feedback sent, but image delivery could not be confirmed."
         },
         "SidebarFilter": {
           "e3b3898218": "Agregar proyecto",

src/renderer/src/i18n/locales/ja.json

@@ -4171,7 +4171,7 @@
           "69969ba364": "送信中…",
           "imageReadFailed": "Could not read the attached images. Try attaching them again.",
           "attachWhileSending": "Wait for the current feedback to finish sending before attaching more images.",
-          "imagesNotDelivered": "Feedback sent, but the images could not be attached."
+          "imagesNotDelivered": "Feedback sent, but image delivery could not be confirmed."
         },
         "SidebarFilter": {
           "e3b3898218": "プロジェクトの追加",

src/renderer/src/i18n/locales/ko.json

@@ -4171,7 +4171,7 @@
           "69969ba364": "전송 중…",
           "imageReadFailed": "Could not read the attached images. Try attaching them again.",
           "attachWhileSending": "Wait for the current feedback to finish sending before attaching more images.",
-          "imagesNotDelivered": "Feedback sent, but the images could not be attached."
+          "imagesNotDelivered": "Feedback sent, but image delivery could not be confirmed."
         },
         "SidebarFilter": {
           "e3b3898218": "프로젝트 추가",

src/renderer/src/i18n/locales/zh.json

@@ -4171,7 +4171,7 @@
           "69969ba364": "正在发送...",
           "imageReadFailed": "Could not read the attached images. Try attaching them again.",
           "attachWhileSending": "Wait for the current feedback to finish sending before attaching more images.",
-          "imagesNotDelivered": "Feedback sent, but the images could not be attached."
+          "imagesNotDelivered": "Feedback sent, but image delivery could not be confirmed."
         },
         "SidebarFilter": {
           "e3b3898218": "添加项目",