diff --git a/apps/desktop/src/main/__tests__/attachment-ingest-resolve.test.ts b/apps/desktop/src/main/__tests__/attachment-ingest-resolve.test.ts index 8eb812f74c..5128176131 100644 --- a/apps/desktop/src/main/__tests__/attachment-ingest-resolve.test.ts +++ b/apps/desktop/src/main/__tests__/attachment-ingest-resolve.test.ts @@ -22,8 +22,15 @@ import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, test } from 'node:test'; -import { resolveAttachmentRefs, resolveIngestItems } from '../attachment-ingest.js'; +import { + type AttachmentSnapshotInput, + resolveAttachmentRefs, + resolveIngestItems, + resolvePickedAttachments, + sniffPickedAttachmentMimeType, +} from '../attachment-ingest.js'; import { createAttachmentApprovalRegistry } from '../attachment-approval.js'; +import { attachmentKindFromMimeType } from '@maka/core/attachments'; describe('resolveIngestItems (pre-read validation)', () => { test('rejects more than 8 items before touching approvals or stat', async () => { @@ -235,6 +242,169 @@ describe('resolveIngestItems (pre-read validation)', () => { }); describe('resolveAttachmentRefs', () => { + test('uses PDF magic bytes instead of a spoofed PNG extension', async () => { + const dir = await mkdtemp(join(tmpdir(), 'att-sniff-')); + const path = join(dir, 'report.png'); + await writeFile(path, Buffer.from('%PDF-1.4\nfixture')); + let resizeCalls = 0; + let captured: AttachmentSnapshotInput | undefined; + try { + await resolveAttachmentRefs({ + files: [{ path, size: 16 }], + resizeImage: async (bytes) => { + resizeCalls += 1; + return bytes; + }, + snapshot: async (input) => { + captured = input; + return { + kind: input.attachmentKind, + name: input.name, + mimeType: input.mimeType, + bytes: input.content.byteLength, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'attachment-1' }, + }; + }, + }); + assert.equal(resizeCalls, 0); + assert.equal(captured?.mimeType, 'application/pdf'); + assert.equal(captured?.attachmentKind, 'pdf'); + assert.equal(captured?.artifactKind, 'pdf'); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test('recognises a PDF whose header sits behind a preamble, not as an ordinary file', async () => { + // A PDF may carry bytes before %PDF-; readers scan the first ~1 KiB for it. + // Missing it here would route the file to `other` and let a downstream text + // read decode binary as UTF-8 — so it must still resolve as a PDF. + const content = Buffer.concat([Buffer.alloc(64), Buffer.from('%PDF-1.7\nfixture')]); + let captured: AttachmentSnapshotInput | undefined; + + await resolveAttachmentRefs({ + files: [{ name: 'notes.txt', size: content.byteLength, content }], + snapshot: async (input) => { + captured = input; + return { + kind: input.attachmentKind, + name: input.name, + mimeType: input.mimeType, + bytes: input.content.byteLength, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'attachment-1' }, + }; + }, + }); + + assert.equal(captured?.mimeType, 'application/pdf'); + assert.equal(captured?.attachmentKind, 'pdf'); + assert.equal(captured?.artifactKind, 'pdf'); + }); + + test('uses PNG magic bytes instead of conflicting renderer metadata', async () => { + const content = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + let resizeCalls = 0; + let captured: AttachmentSnapshotInput | undefined; + + await resolveAttachmentRefs({ + files: [{ name: 'report.pdf', mimeType: 'application/pdf', size: content.byteLength, content }], + resizeImage: async (bytes) => { + resizeCalls += 1; + return bytes; + }, + snapshot: async (input) => { + captured = input; + return { + kind: input.attachmentKind, + name: input.name, + mimeType: input.mimeType, + bytes: input.content.byteLength, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'attachment-1' }, + }; + }, + }); + + assert.equal(resizeCalls, 1); + assert.equal(captured?.mimeType, 'image/png'); + assert.equal(captured?.attachmentKind, 'image'); + assert.equal(captured?.artifactKind, 'image'); + }); + + test('recognises an image with no extension from its bytes', async () => { + const content = Buffer.from([0xff, 0xd8, 0xff, 0xe0]); + let captured: AttachmentSnapshotInput | undefined; + + await resolveAttachmentRefs({ + files: [{ name: 'clipboard-image', size: content.byteLength, content }], + snapshot: async (input) => { + captured = input; + return { + kind: input.attachmentKind, + name: input.name, + mimeType: input.mimeType, + bytes: input.content.byteLength, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'attachment-1' }, + }; + }, + }); + + assert.equal(captured?.mimeType, 'image/jpeg'); + assert.equal(captured?.attachmentKind, 'image'); + }); + + test('updates the MIME when image resizing changes the encoded format', async () => { + const jpeg = Buffer.from([0xff, 0xd8, 0xff, 0xe0]); + const resizedPng = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + let captured: AttachmentSnapshotInput | undefined; + + await resolveAttachmentRefs({ + files: [{ name: 'photo.jpg', size: jpeg.byteLength, content: jpeg }], + resizeImage: async () => resizedPng, + snapshot: async (input) => { + captured = input; + return { + kind: input.attachmentKind, + name: input.name, + mimeType: input.mimeType, + bytes: input.content.byteLength, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'attachment-1' }, + }; + }, + }); + + assert.equal(captured?.mimeType, 'image/png'); + assert.deepEqual(captured?.content, resizedPng); + }); + + test('does not decode unknown bytes merely because the renderer calls them an image', async () => { + const content = Buffer.from('not an image'); + let resizeCalls = 0; + let captured: AttachmentSnapshotInput | undefined; + + await resolveAttachmentRefs({ + files: [{ name: 'payload.svg', mimeType: 'image/svg+xml', size: content.byteLength, content }], + resizeImage: async (bytes) => { + resizeCalls += 1; + return bytes; + }, + snapshot: async (input) => { + captured = input; + return { + kind: input.attachmentKind, + name: input.name, + mimeType: input.mimeType, + bytes: input.content.byteLength, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'attachment-1' }, + }; + }, + }); + + assert.equal(resizeCalls, 0); + assert.equal(captured?.mimeType, 'application/octet-stream'); + assert.equal(captured?.attachmentKind, 'other'); + assert.equal(captured?.artifactKind, 'file'); + }); + test('rejects a path grown beyond the cap before creating a Host artifact', async () => { const dir = await mkdtemp(join(tmpdir(), 'att-cap-')); const path = join(dir, 'grew.bin'); @@ -258,3 +428,81 @@ describe('resolveAttachmentRefs', () => { } }); }); + +describe('sniffPickedAttachmentMimeType (pick-time staging kind)', () => { + test('stages a real image named .pdf as an image so it previews and notices', async () => { + const dir = await mkdtemp(join(tmpdir(), 'att-pick-')); + const path = join(dir, 'report.pdf'); + await writeFile(path, Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])); + try { + const mimeType = await sniffPickedAttachmentMimeType(path, 'report.pdf'); + assert.equal(mimeType, 'image/png'); + assert.equal(attachmentKindFromMimeType(mimeType, 'report.pdf'), 'image'); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test('stages a real PDF named .png as a pdf, and unknown bytes as an ordinary file', async () => { + const dir = await mkdtemp(join(tmpdir(), 'att-pick-')); + const pdfPath = join(dir, 'report.png'); + const junkPath = join(dir, 'payload.png'); + await writeFile(pdfPath, Buffer.from('%PDF-1.4\nfixture')); + await writeFile(junkPath, Buffer.from('not an image')); + try { + assert.equal(await sniffPickedAttachmentMimeType(pdfPath, 'report.png'), 'application/pdf'); + const junk = await sniffPickedAttachmentMimeType(junkPath, 'payload.png'); + assert.equal(junk, 'application/octet-stream'); + assert.equal(attachmentKindFromMimeType(junk, 'payload.png'), 'other'); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test('a read failure downgrades a claimed image/PDF name rather than trusting it', async () => { + // The prefix read failing must not reinstate the extension-based image/PDF + // claim resolveAttachmentMimeType exists to reject: staging stays unblocked + // but the spoofed kind is dropped (the send path re-reads and confirms). + const spoofed = await sniffPickedAttachmentMimeType('/no/such/file.pdf', 'file.pdf'); + assert.equal(spoofed, 'application/octet-stream'); + assert.equal(attachmentKindFromMimeType(spoofed, 'file.pdf'), 'other'); + // A non-image/PDF claim still resolves, so genuine document kinds still stage. + const docx = await sniffPickedAttachmentMimeType('/no/such/file.docx', 'file.docx'); + assert.equal( + docx, + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + ); + }); +}); + +describe('resolvePickedAttachments (pick dialog owner: stage by content)', () => { + test('stamps each picked path with its sniffed MIME and main-side size, not its extension', async () => { + const dir = await mkdtemp(join(tmpdir(), 'att-pick-plan-')); + const imageAsPdf = join(dir, 'report.pdf'); // real PNG bytes behind a .pdf name + const junkAsPng = join(dir, 'payload.png'); // non-image bytes behind an image name + await writeFile(imageAsPdf, Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])); + await writeFile(junkAsPng, Buffer.from('not an image')); + const statCalls: string[] = []; + try { + const plan = await resolvePickedAttachments([imageAsPdf, junkAsPng], async (path) => { + statCalls.push(path); + return { size: path.endsWith('report.pdf') ? 8 : 12 }; + }); + + // report.pdf carries image bytes → stages as an image (thumbnail + notice). + assert.equal(plan[0].name, 'report.pdf'); + assert.equal(plan[0].mimeType, 'image/png'); + assert.equal(attachmentKindFromMimeType(plan[0].mimeType, plan[0].name), 'image'); + assert.equal(plan[0].size, 8); + + // A disguised .png loses its image claim → stages as an ordinary file. + assert.equal(plan[1].mimeType, 'application/octet-stream'); + assert.equal(attachmentKindFromMimeType(plan[1].mimeType, plan[1].name), 'other'); + + // Sizes come from the injected (main-side) stat, one call per path. + assert.deepEqual(statCalls, [imageAsPdf, junkAsPng]); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/apps/desktop/src/main/__tests__/attachment-preview.test.ts b/apps/desktop/src/main/__tests__/attachment-preview.test.ts index 1fc5ecd4cf..221f5a9a55 100644 --- a/apps/desktop/src/main/__tests__/attachment-preview.test.ts +++ b/apps/desktop/src/main/__tests__/attachment-preview.test.ts @@ -26,7 +26,7 @@ import { type AttachmentPreviewResult, } from '../attachment-preview.js'; -const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); +const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); function fakeRenderPreview(bytes: Uint8Array): Promise<{ bytes: Uint8Array; mimeType: string } | null> { return Promise.resolve({ bytes, mimeType: 'image/png' }); @@ -82,7 +82,7 @@ describe('staged attachment preview (approval source)', () => { assert.equal(read, 0); }); - it('refuses non-image approvals without touching the file', async () => { + it('reads non-image approvals once to verify their content before refusing them', async () => { const approvals = createAttachmentApprovalRegistry(); const [issued] = approvals.issueApprovals(1, [{ path: '/tmp/notes.md', name: 'notes.md', size: 4 }]); let read = 0; @@ -92,12 +92,102 @@ describe('staged attachment preview (approval source)', () => { approvals, readFile: async () => { read += 1; - return PNG; + return Buffer.from('text'); }, renderPreview: fakeRenderPreview, }); assert.deepEqual(result, { ok: false, reason: 'not_image' }); - assert.equal(read, 0); + assert.equal(read, 1); + }); + + it('previews real image bytes even when the file name claims PDF', async () => { + const approvals = createAttachmentApprovalRegistry(); + const [issued] = approvals.issueApprovals(1, [ + { path: '/tmp/report.pdf', name: 'report.pdf', mimeType: 'application/pdf', size: PNG.byteLength }, + ]); + let renderCalls = 0; + const result = await loadApprovalPreview({ + senderId: 1, + approvalId: issued.approvalId, + approvals, + readFile: async () => PNG, + renderPreview: async (bytes) => { + renderCalls += 1; + return { bytes, mimeType: 'image/png' }; + }, + }); + + assert.deepEqual(result, { + ok: true, + base64: Buffer.from(PNG).toString('base64'), + mimeType: 'image/png', + }); + assert.equal(renderCalls, 1); + }); + + it('does not send PDF bytes with a spoofed PNG name to the image decoder', async () => { + const pdf = Buffer.from('%PDF-1.4\nfixture'); + const approvals = createAttachmentApprovalRegistry(); + const [issued] = approvals.issueApprovals(1, [ + { path: '/tmp/report.png', name: 'report.png', size: pdf.byteLength }, + ]); + let renderCalls = 0; + const result = await loadApprovalPreview({ + senderId: 1, + approvalId: issued.approvalId, + approvals, + readFile: async () => pdf, + renderPreview: async () => { + renderCalls += 1; + return null; + }, + }); + + assert.deepEqual(result, { ok: false, reason: 'not_image' }); + assert.equal(renderCalls, 0); + }); + + it('does not send unknown bytes with a spoofed PNG name to the image decoder', async () => { + const unknown = Buffer.from('not an image'); + const approvals = createAttachmentApprovalRegistry(); + const [issued] = approvals.issueApprovals(1, [ + { path: '/tmp/payload.png', name: 'payload.png', size: unknown.byteLength }, + ]); + let renderCalls = 0; + const result = await loadApprovalPreview({ + senderId: 1, + approvalId: issued.approvalId, + approvals, + readFile: async () => unknown, + renderPreview: async () => { + renderCalls += 1; + return null; + }, + }); + + assert.deepEqual(result, { ok: false, reason: 'not_image' }); + assert.equal(renderCalls, 0); + }); + + it('uses the sniffed MIME when returning original image bytes as the preview', async () => { + const webp = Buffer.from('RIFF0000WEBPVP8 ', 'ascii'); + const approvals = createAttachmentApprovalRegistry(); + const [issued] = approvals.issueApprovals(1, [ + { path: '/tmp/photo.png', name: 'photo.png', size: webp.byteLength }, + ]); + const result = await loadApprovalPreview({ + senderId: 1, + approvalId: issued.approvalId, + approvals, + readFile: async () => webp, + renderPreview: async () => null, + }); + + assert.deepEqual(result, { + ok: true, + base64: webp.toString('base64'), + mimeType: 'image/webp', + }); }); it('registers the client-owned attachment preview IPC boundary', async () => { diff --git a/apps/desktop/src/main/__tests__/new-task-staged-content.test.ts b/apps/desktop/src/main/__tests__/new-task-staged-content.test.ts index 0eba968b64..ee1ca35b77 100644 --- a/apps/desktop/src/main/__tests__/new-task-staged-content.test.ts +++ b/apps/desktop/src/main/__tests__/new-task-staged-content.test.ts @@ -137,8 +137,45 @@ function stubFilePicker(): { }; } +const PNG_MAGIC = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + +function fileWithBytes(name: string, type: string, bytes: ArrayLike): File { + return new File([Uint8Array.from(bytes)], name, { type }); +} + function textFile(name: string): File { - return { name, type: 'text/plain', size: 12 } as unknown as File; + // Bytes that match no signature, so content sniffing leaves the declared + // text type in place rather than downgrading it. + return fileWithBytes(name, 'text/plain', new TextEncoder().encode('plain notes')); +} + +/** A blob whose leading-byte read rejects, exercising the `sniffFileMimeType` + * catch: the declared image/PDF type must be downgraded, not reinstated. */ +function unreadableFile(name: string, type: string): File { + return { + name, + type, + size: 32, + slice: () => ({ arrayBuffer: () => Promise.reject(new Error('slice read failed')) }), + } as unknown as File; +} + +/** A blob whose leading-byte read is held open, so a test can switch the active + * draft while `fileToPending` is mid-sniff — the window this PR's async made + * real. Bytes are PNG so it stages as an image once the read resolves. */ +function deferredImageFile(name: string): { file: File; resolveRead(): void } { + let release: () => void = () => {}; + const arrayBuffer = () => + new Promise((resolve) => { + release = () => resolve(new Uint8Array(PNG_MAGIC).buffer); + }); + const file = { + name, + type: 'image/png', + size: PNG_MAGIC.length, + slice: () => ({ arrayBuffer }), + } as unknown as File; + return { file, resolveRead: () => release() }; } function modelChoice(model: string, supportsVision: boolean): ChatModelChoice { @@ -226,7 +263,7 @@ test('AppShell composition shows the localized non-vision image notice once per service: idleAttachmentService, }), ); - const image = { name: 'chart.png', type: 'image/png', size: 12 } as unknown as File; + const image = fileWithBytes('chart.png', 'image/png', PNG_MAGIC); await probe.render('session-en', 'en'); await act(() => probe.latest().attachFilePaths([image])); @@ -331,3 +368,95 @@ test('files chosen in the native dialog land in the composer now on screen', asy ['chosen.txt'], ); }); + +test('files dropped while a session switch is mid-sniff land in the composer now on screen', async () => { + const probe = await mountProbe((options) => + useComposerAttachments({ + ...options, + toastApi: { error() {} }, + service: idleAttachmentService, + }), + ); + await probe.render(NEW_TASK_PENDING_KEY); + + // fileToPending reads the leading bytes before staging; that read is async, + // so the surface can change before it resolves. The file must land where the + // user is looking now, not in the bucket bound before the read started. + const dropped = deferredImageFile('photo.png'); + const attaching = probe.latest().attachFilePaths([dropped.file]); + await probe.render('session-1'); + await act(async () => { + dropped.resolveRead(); + await attaching; + }); + + assert.deepEqual( + probe.latest().pendingAttachments.map((item) => item.displayName), + ['photo.png'], + ); +}); + +test('dropped files stage by content: a real image named .pdf notices, a disguised .png does not', async () => { + const calls: Array<{ title: string }> = []; + const probe = await mountProbe((options) => + useComposerAttachments({ + ...options, + toastApi: { error() {} }, + imageNotice: { + notify(title) { + calls.push({ title }); + }, + // No selected target that supports vision, so a staged image notices. + supportsVision: () => false, + }, + service: idleAttachmentService, + }), + ); + await probe.render('session-1'); + + // A real image behind a `.pdf` name stages as an image — thumbnail path and + // the non-vision notice both key off the sniffed kind, not the extension. + await act(() => + probe.latest().attachFilePaths([fileWithBytes('report.pdf', 'application/pdf', PNG_MAGIC)]), + ); + assert.equal(probe.latest().pendingAttachments.at(-1)?.kind, 'image'); + assert.equal(calls.length, 1); + + // A non-image behind a `.png` name and an `image/png` claim does neither: the + // unverified claim is downgraded rather than trusted. + await act(() => + probe.latest().attachFilePaths([ + fileWithBytes('photo.png', 'image/png', new TextEncoder().encode('not an image')), + ]), + ); + assert.equal(probe.latest().pendingAttachments.at(-1)?.kind, 'other'); + assert.equal(calls.length, 1); +}); + +test('a failed leading-byte read downgrades the declared image claim rather than trusting it', async () => { + const calls: Array<{ title: string }> = []; + const probe = await mountProbe((options) => + useComposerAttachments({ + ...options, + toastApi: { error() {} }, + imageNotice: { + notify(title) { + calls.push({ title }); + }, + supportsVision: () => false, + }, + service: idleAttachmentService, + }), + ); + await probe.render('session-1'); + + // `slice().arrayBuffer()` rejects: the catch must route an empty prefix + // through the downgrade, so an `image/png` claim it could not verify stages + // as an ordinary file and never fires the vision notice. + await act(() => + probe.latest().attachFilePaths([unreadableFile('screenshot.png', 'image/png')]), + ); + assert.equal(probe.latest().pendingAttachments.at(-1)?.kind, 'other'); + assert.equal(probe.latest().pendingAttachments.at(-1)?.mimeType, 'application/octet-stream'); + assert.equal(calls.length, 0); +}); diff --git a/apps/desktop/src/main/attachment-ingest.ts b/apps/desktop/src/main/attachment-ingest.ts index 96194d6560..446cb99fb6 100644 --- a/apps/desktop/src/main/attachment-ingest.ts +++ b/apps/desktop/src/main/attachment-ingest.ts @@ -22,9 +22,11 @@ import { open } from 'node:fs/promises'; import { basename } from 'node:path'; import { attachmentKindFromMimeType, - guessMimeFromName, MAX_ATTACHMENT_BYTES, MAX_ATTACHMENT_COUNT, + ATTACHMENT_MIME_SNIFF_BYTES, + resolveAttachmentMimeType, + sniffAttachmentMimeType, } from '@maka/core/attachments'; import type { ArtifactKind } from '@maka/core/artifacts'; import type { AttachmentRef } from '@maka/core/events'; @@ -57,12 +59,13 @@ export async function resolveAttachmentRefs(input: { const refs: AttachmentRef[] = []; for (const file of input.files) { const name = attachmentFileName(file); - const mimeType = file.mimeType && file.mimeType.length > 0 ? file.mimeType : guessMimeFromName(name); + let bytes: Uint8Array = isPathAttachment(file) ? await readFileCapped(file.path, maxBytes) : file.content; + let mimeType = resolveAttachmentMimeType(bytes, file.mimeType, name); const kind = attachmentKindFromMimeType(mimeType, name); - let bytes: Uint8Array = isPathAttachment(file) ? await readFileCapped(file.path, maxBytes) : file.content; if (kind === 'image' && input.resizeImage) { bytes = await input.resizeImage(bytes); + mimeType = sniffAttachmentMimeType(bytes) ?? mimeType; } const artifactKind: ArtifactKind = kind === 'image' ? 'image' : kind === 'pdf' ? 'pdf' : 'file'; @@ -79,6 +82,66 @@ export async function resolveAttachmentRefs(input: { return refs; } +/** + * Content type for a user-picked path, read cheaply from a short prefix so the + * composer can stage — and later preview — an attachment by its bytes rather + * than its extension. Mirrors the send-path precedence in + * {@link resolveAttachmentMimeType}: a real image named `report.pdf` resolves + * to its image MIME (so the composer shows a thumbnail and the vision notice), + * a disguised file loses its spoofed image/PDF claim. A read failure resolves + * an empty prefix through the same policy, so staging stays unblocked without + * reinstating the name's unverified image/PDF claim (the send path re-reads). + */ +export async function sniffPickedAttachmentMimeType(path: string, name: string): Promise { + let prefix: Uint8Array = new Uint8Array(); + try { + prefix = await readFilePrefix(path, ATTACHMENT_MIME_SNIFF_BYTES); + } catch { + // Fall through with the empty prefix: routing it through + // resolveAttachmentMimeType downgrades a claimed image/PDF name rather than + // trusting it, keeping one owner for the content-first policy. + } + return resolveAttachmentMimeType(prefix, undefined, name); +} + +/** + * Resolve the paths returned by the pick dialog into approval-plan entries, + * each staged under its content-sniffed MIME rather than its extension — the + * headline behavior of this feature, extracted from the `attachments:pickFiles` + * IPC handler so the content decision is testable without a native dialog. + * `stat` is injected (the handler passes `node:fs/promises`); sizes come from + * main, never the renderer. + */ +export async function resolvePickedAttachments( + paths: readonly string[], + stat: (path: string) => Promise<{ size: number }>, +): Promise> { + return Promise.all( + paths.map(async (path) => { + const name = basename(path); + return { + path, + name, + size: (await stat(path)).size, + mimeType: await sniffPickedAttachmentMimeType(path, name), + }; + }), + ); +} + +/** Read up to `byteCount` leading bytes without loading the whole file, for + * content sniffing at pick time (a full read waits until send). */ +async function readFilePrefix(path: string, byteCount: number): Promise { + const fh = await open(path, 'r'); + try { + const buf = Buffer.alloc(byteCount); + const { bytesRead } = await fh.read(buf, 0, byteCount, 0); + return buf.subarray(0, bytesRead); + } finally { + await fh.close(); + } +} + function isPathAttachment(file: AttachmentIngestFile): file is Extract { return 'path' in file; } diff --git a/apps/desktop/src/main/attachment-preview.ts b/apps/desktop/src/main/attachment-preview.ts index a6b3c6651a..5128a6516e 100644 --- a/apps/desktop/src/main/attachment-preview.ts +++ b/apps/desktop/src/main/attachment-preview.ts @@ -18,7 +18,7 @@ */ import { Buffer } from 'node:buffer'; -import { attachmentKindFromMimeType, guessMimeFromName, MAX_ATTACHMENT_BYTES } from '@maka/core/attachments'; +import { MAX_ATTACHMENT_BYTES, sniffAttachmentMimeType } from '@maka/core/attachments'; import type { AttachmentApprovalRegistry } from './attachment-approval.js'; export type AttachmentPreviewResult = @@ -65,17 +65,14 @@ export async function loadApprovalPreview(input: { } const approved = input.approvals.peekApproval(input.senderId, input.approvalId); if (!approved) return { ok: false, reason: 'not_found' }; - const mimeType = - approved.mimeType && approved.mimeType.length > 0 - ? approved.mimeType - : guessMimeFromName(approved.name); - if (attachmentKindFromMimeType(mimeType, approved.name) !== 'image') { - return { ok: false, reason: 'not_image' }; - } const maxBytes = input.maxBytes ?? MAX_ATTACHMENT_BYTES; if (approved.size > maxBytes) return { ok: false, reason: 'unreadable' }; try { const bytes = await input.readFile(approved.path, approved.size); + const contentMimeType = sniffAttachmentMimeType(bytes); + if (!contentMimeType?.startsWith('image/')) { + return { ok: false, reason: 'not_image' }; + } const preview = await input.renderPreview(bytes); if (preview) { return { @@ -86,7 +83,7 @@ export async function loadApprovalPreview(input: { } const fallbackCap = input.inlineFallbackMaxBytes ?? INLINE_PREVIEW_FALLBACK_MAX_BYTES; if (bytes.byteLength <= fallbackCap) { - return { ok: true, base64: Buffer.from(bytes).toString('base64'), mimeType }; + return { ok: true, base64: Buffer.from(bytes).toString('base64'), mimeType: contentMimeType }; } return { ok: false, reason: 'unreadable' }; } catch { diff --git a/apps/desktop/src/main/runtime-host-boot.ts b/apps/desktop/src/main/runtime-host-boot.ts index 3d352e8370..6575ccc5aa 100644 --- a/apps/desktop/src/main/runtime-host-boot.ts +++ b/apps/desktop/src/main/runtime-host-boot.ts @@ -30,7 +30,7 @@ import { } from "electron"; import { randomUUID } from "node:crypto"; import { readFileSync } from "node:fs"; -import { basename, join } from "node:path"; +import { join } from "node:path"; import { type ConnectionEvent } from '@maka/core/connections'; import { type SessionChangedEvent, type SessionChangedReason } from '@maka/core/session'; import { isBotDeliveryProvider } from '@maka/core/bot-chat-settings'; @@ -77,7 +77,7 @@ import { createAppUpdateService } from "./app-update-service.js"; import { createAttachmentApprovalRegistry } from "./attachment-approval.js"; import { renderAttachmentPreview, resizeImageForAttachment } from "./attachment-resize-native.js"; import { registerAttachmentPreviewIpc } from "./attachment-preview.js"; -import { readFileCapped } from "./attachment-ingest.js"; +import { readFileCapped, resolvePickedAttachments } from "./attachment-ingest.js"; import { registerBrowserIpc } from "./browser-ipc-main.js"; import { browserViewHost } from "./browser/browser-host.js"; import { releaseBrowserSession } from "./browser/session.js"; @@ -1710,13 +1710,10 @@ function registerPersistentClientIpc(): void { if (result.canceled || !result.filePaths[0]) return { ok: false, reason: "cancelled" }; const { stat } = await import("node:fs/promises"); - const chosen = await Promise.all( - result.filePaths.map(async (path) => ({ - path, - name: basename(path), - size: (await stat(path)).size, - })), - ); + // Route by content, not the extension: each picked path is staged under the + // kind its sniffed MIME implies, so a real image named `report.pdf` previews + // and triggers the vision notice, and a disguised file does neither. + const chosen = await resolvePickedAttachments(result.filePaths, (path) => stat(path)); return { ok: true, files: attachmentApprovals.issueApprovals(event.sender.id, chosen), diff --git a/apps/desktop/src/renderer/use-composer-attachments.ts b/apps/desktop/src/renderer/use-composer-attachments.ts index e02f23c3f5..4f762d8847 100644 --- a/apps/desktop/src/renderer/use-composer-attachments.ts +++ b/apps/desktop/src/renderer/use-composer-attachments.ts @@ -18,7 +18,12 @@ */ import { useEffect, useMemo, useRef, useState } from 'react'; -import { attachmentKindFromMimeType, guessMimeFromName } from '@maka/core/attachments'; +import { + ATTACHMENT_MIME_SNIFF_BYTES, + attachmentKindFromMimeType, + guessMimeFromName, + resolveAttachmentMimeType, +} from '@maka/core/attachments'; import { DIRECTORY_REFERENCE_MAX_COUNT, type AttachmentRef, @@ -108,18 +113,37 @@ function approvalToPending(file: { }; } -function fileToPending(file: File): PendingAttachment { - const mimeType = file.type || undefined; +async function fileToPending(file: File): Promise { + // Sniff the leading bytes so a spoofed extension (a real image named + // `report.pdf`, or a PDF named `photo.png`) stages under its true kind — + // matching how main resolves picked files and how the send path routes. + const mimeType = await sniffFileMimeType(file); return { stagingKey: crypto.randomUUID(), displayName: file.name, mimeType, - kind: attachmentKindFromMimeType(mimeType ?? '', file.name), + kind: attachmentKindFromMimeType(mimeType, file.name), size: file.size, source: { type: 'file', file }, }; } +/** Content type for a dropped/pasted blob, from its {@link ATTACHMENT_MIME_SNIFF_BYTES} + * prefix. A failed slice read resolves an empty prefix through the same policy + * rather than falling back to the renderer-declared type — reinstating that + * unverified image/PDF claim is exactly what this content-first path avoids. */ +async function sniffFileMimeType(file: File): Promise { + const declared = file.type || undefined; + let prefix = new Uint8Array(); + try { + prefix = new Uint8Array(await file.slice(0, ATTACHMENT_MIME_SNIFF_BYTES).arrayBuffer()); + } catch { + // Fall through with the empty prefix so the declared image/PDF claim is + // downgraded, not trusted; staging stays unblocked and the send path re-reads. + } + return resolveAttachmentMimeType(prefix, declared, file.name); +} + function retainedToPending(attachment: AttachmentRef): PendingAttachment { return { stagingKey: crypto.randomUUID(), @@ -348,8 +372,14 @@ export function useComposerAttachments(options: { async function attachFilePaths(files: File[]): Promise { if (files.length === 0) return; - const ownerKey = options.draftKey; - const staged = files.map(fileToPending); + // Bind the owner AFTER the sniff reads resolve, never before: fileToPending + // became async to read each file's leading bytes, so the surface can change + // during that I/O (a network volume or spun-down drive makes it seconds). + // The files belong in the composer the user is looking at now — not a bucket + // they have since left, where they would be invisible but still sendable. + // Same reasoning as pickAttachments above. + const staged = await Promise.all(files.map(fileToPending)); + const ownerKey = liveOptionsRef.current.draftKey; updateAttachments((map) => appendPending(map, ownerKey, staged)); for (const item of staged) lifecycleRef.current.stagedKeys.add(item.stagingKey); notifyStagedImages(ownerKey, staged); diff --git a/packages/core/src/__tests__/attachments.test.ts b/packages/core/src/__tests__/attachments.test.ts index 27ee9abf4e..c4c2ba59ca 100644 --- a/packages/core/src/__tests__/attachments.test.ts +++ b/packages/core/src/__tests__/attachments.test.ts @@ -19,7 +19,14 @@ import assert from 'node:assert/strict'; import { describe, test } from 'node:test'; -import { formatAttachmentResourceRef, parseAttachmentResourceRef } from '../attachments.js'; +import { + ATTACHMENT_MIME_SNIFF_BYTES, + formatAttachmentResourceRef, + parseAttachmentResourceRef, + PDF_HEADER_SCAN_BYTES, + resolveAttachmentMimeType, + sniffAttachmentMimeType, +} from '../attachments.js'; describe('attachment resource refs', () => { test('round-trips one canonical Session Artifact without embedding Session authority', () => { @@ -51,3 +58,86 @@ describe('attachment resource refs', () => { assert.equal(parseAttachmentResourceRef('maka://runtime/attachments/a/b'), null); }); }); + +describe('attachment content sniffing', () => { + test('recognises supported image and PDF signatures', () => { + const fixtures: Array<[Uint8Array, string]> = [ + [Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), 'image/png'], + [Buffer.from([0xff, 0xd8, 0xff, 0xe0]), 'image/jpeg'], + [Buffer.from('GIF89a', 'ascii'), 'image/gif'], + [Buffer.from('RIFF0000WEBP', 'ascii'), 'image/webp'], + [Buffer.from('%PDF-1.7', 'ascii'), 'application/pdf'], + ]; + + for (const [bytes, expected] of fixtures) { + assert.equal(sniffAttachmentMimeType(bytes), expected); + } + }); + + test('does not sniff BMP: unsupported downstream, and its 2-byte marker is too weak', () => { + // No reader (send path or Read) accepts BMP, and `BM` matches arbitrary + // files, so treating those bytes as an image would route them to a decoder + // that later fails with unsupported_mime. + assert.equal(sniffAttachmentMimeType(Buffer.from('BM harmless text')), undefined); + }); + + test('finds a PDF header behind a preamble, but keeps image sniffing at offset 0', () => { + // PDF readers tolerate junk before %PDF- and scan the first ~1 KiB, so a + // header past the sniffing prefix must still resolve as a PDF. + assert.equal( + sniffAttachmentMimeType( + Buffer.concat([Buffer.alloc(ATTACHMENT_MIME_SNIFF_BYTES), Buffer.from('%PDF-1.7')]), + ), + 'application/pdf', + ); + // Image signatures are only valid at their fixed offset: a PNG header behind + // a preamble is not a decodable image stream, so it is not sniffed. + assert.equal( + sniffAttachmentMimeType( + Buffer.concat([ + Buffer.alloc(4), + Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), + ]), + ), + undefined, + ); + // Still bounded: a PDF header past the scan window is not a match, so + // sniffing never becomes an unbounded read. + assert.equal( + sniffAttachmentMimeType( + Buffer.concat([Buffer.alloc(PDF_HEADER_SCAN_BYTES), Buffer.from('%PDF-1.7')]), + ), + undefined, + ); + // An extension-like payload is not a signature. + assert.equal(sniffAttachmentMimeType(Buffer.from('report.png')), undefined); + }); +}); + +describe('resolveAttachmentMimeType (content-first precedence)', () => { + const PNG = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + const PDF = Buffer.from('%PDF-1.7', 'ascii'); + + test('sniffed content wins over a conflicting name or supplied MIME', () => { + assert.equal(resolveAttachmentMimeType(PNG, 'application/pdf', 'report.pdf'), 'image/png'); + assert.equal(resolveAttachmentMimeType(PDF, 'image/png', 'photo.png'), 'application/pdf'); + }); + + test('downgrades an unverified image/PDF claim to octet-stream', () => { + const notAnImage = Buffer.from('not an image'); + assert.equal( + resolveAttachmentMimeType(notAnImage, 'image/png', 'payload.png'), + 'application/octet-stream', + ); + assert.equal( + resolveAttachmentMimeType(notAnImage, undefined, 'payload.png'), + 'application/octet-stream', + ); + }); + + test('keeps a non-image document claim so real document kinds still resolve', () => { + const docx = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'; + assert.equal(resolveAttachmentMimeType(Buffer.from('PK'), docx, 'notes.docx'), docx); + assert.equal(resolveAttachmentMimeType(Buffer.from('PK'), undefined, 'notes.docx'), docx); + }); +}); diff --git a/packages/core/src/attachments.ts b/packages/core/src/attachments.ts index 6fe82ed2bd..06550236c1 100644 --- a/packages/core/src/attachments.ts +++ b/packages/core/src/attachments.ts @@ -83,7 +83,6 @@ const MIME_BY_EXTENSION: Readonly> = { jpeg: 'image/jpeg', gif: 'image/gif', webp: 'image/webp', - bmp: 'image/bmp', pdf: 'application/pdf', docx: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', xlsx: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', @@ -93,6 +92,70 @@ const MIME_BY_EXTENSION: Readonly> = { ppt: 'application/vnd.ms-powerpoint', }; +export const ATTACHMENT_MIME_SNIFF_BYTES = 16; + +/** + * PDF permits arbitrary bytes before the `%PDF-` header and readers + * conventionally scan roughly the first kilobyte for it. Searching that window + * keeps a PDF with a preamble from sniffing as nothing — which downstream would + * otherwise decode as UTF-8 text — while staying bounded so sniffing never + * becomes an unbounded read. Only the PDF header needs it: image signatures are + * valid solely at their fixed offset within {@link ATTACHMENT_MIME_SNIFF_BYTES}. + */ +export const PDF_HEADER_SCAN_BYTES = 1024; + +export type SniffedAttachmentMimeType = + | 'image/png' + | 'image/jpeg' + | 'image/gif' + | 'image/webp' + | 'application/pdf'; + +/** + * Identify the attachment formats whose bytes affect how Maka processes them. + * Image signatures are read from the fixed {@link ATTACHMENT_MIME_SNIFF_BYTES} + * prefix at their required offset; the PDF header may sit behind a short + * preamble, so it is searched across the first {@link PDF_HEADER_SCAN_BYTES}. + * Both bounds are fixed, so callers can apply this before handing untrusted + * input to an image decoder without turning sniffing into an unbounded read. + */ +export function sniffAttachmentMimeType(bytes: Uint8Array): SniffedAttachmentMimeType | undefined { + const prefix = bytes.subarray(0, ATTACHMENT_MIME_SNIFF_BYTES); + if (startsWithBytes(prefix, [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])) { + return 'image/png'; + } + if (startsWithBytes(prefix, [0xff, 0xd8, 0xff])) return 'image/jpeg'; + if (startsWithAscii(prefix, 'GIF87a') || startsWithAscii(prefix, 'GIF89a')) return 'image/gif'; + if (startsWithAscii(prefix, 'RIFF') && startsWithAscii(prefix, 'WEBP', 8)) return 'image/webp'; + if (containsAscii(bytes.subarray(0, PDF_HEADER_SCAN_BYTES), '%PDF-')) return 'application/pdf'; + return undefined; +} + +function startsWithBytes(bytes: Uint8Array, signature: readonly number[], offset = 0): boolean { + return ( + bytes.length >= offset + signature.length && + signature.every((value, index) => bytes[offset + index] === value) + ); +} + +function startsWithAscii(bytes: Uint8Array, signature: string, offset = 0): boolean { + if (bytes.length < offset + signature.length) return false; + for (let index = 0; index < signature.length; index += 1) { + if (bytes[offset + index] !== signature.charCodeAt(index)) return false; + } + return true; +} + +/** True if `signature` appears anywhere in `bytes` (bounded by the caller's + * slice), for headers a format allows to sit behind a preamble. */ +function containsAscii(bytes: Uint8Array, signature: string): boolean { + const lastOffset = bytes.length - signature.length; + for (let offset = 0; offset <= lastOffset; offset += 1) { + if (startsWithAscii(bytes, signature, offset)) return true; + } + return false; +} + /** * Best-effort MIME from a file name, used when the picker gives no MIME * (Electron's openDialog only returns paths). Falls back to @@ -105,6 +168,36 @@ export function guessMimeFromName(fileName: string): string { return MIME_BY_EXTENSION[ext] ?? 'application/octet-stream'; } +/** + * Decide an attachment's MIME with content taking precedence over the name and + * any renderer-supplied MIME, so a spoofed extension cannot steer routing. + * Sniffed bytes win outright. When nothing sniffs, a *claimed* image/PDF MIME + * (from the name or the renderer) is downgraded to `application/octet-stream` + * so unverified bytes never enter the image or PDF path; any other claim is + * kept so genuine document kinds still resolve. `bytes` may be a prefix rather + * than the whole file: image signatures resolve from the + * {@link ATTACHMENT_MIME_SNIFF_BYTES} prefix, while a PDF header behind a + * preamble is only found within the first {@link PDF_HEADER_SCAN_BYTES} — the + * send path passes the full bytes, pick-time staging passes the short prefix. + */ +export function resolveAttachmentMimeType( + bytes: Uint8Array, + suppliedMimeType: string | undefined, + fileName: string, +): string { + const sniffed = sniffAttachmentMimeType(bytes); + if (sniffed) return sniffed; + + const fallback = + suppliedMimeType && suppliedMimeType.length > 0 + ? suppliedMimeType + : guessMimeFromName(fileName); + const normalized = fallback.toLowerCase(); + return normalized.startsWith('image/') || normalized === 'application/pdf' + ? 'application/octet-stream' + : fallback; +} + /** * Route a MIME type to an {@link AttachmentRef} kind. The runtime * consumption split is image vs. everything-else (images become provider diff --git a/packages/runtime/src/image-file.ts b/packages/runtime/src/image-file.ts index e87a04a4a6..48b7b89f3f 100644 --- a/packages/runtime/src/image-file.ts +++ b/packages/runtime/src/image-file.ts @@ -24,15 +24,10 @@ import { MAX_MODEL_IMAGE_EDGE, MAX_READ_IMAGE_BYTES, READ_IMAGE_TOO_LARGE_MESSAGE, + sniffAttachmentMimeType, } from '@maka/core/attachments'; const IMAGE_EXTENSIONS = new Set(['.png', '.jpg', '.jpeg', '.gif', '.webp']); -const PNG_SIGNATURE = Buffer.from('\x89PNG\r\n\x1a\n', 'latin1'); -const JPEG_SIGNATURE = Buffer.from('\xff\xd8\xff', 'latin1'); -const GIF87A_SIGNATURE = Buffer.from('GIF87a'); -const GIF89A_SIGNATURE = Buffer.from('GIF89a'); -const RIFF_SIGNATURE = Buffer.from('RIFF'); -const WEBP_SIGNATURE = Buffer.from('WEBP'); export type ImageMimeType = 'image/png' | 'image/jpeg' | 'image/gif' | 'image/webp'; export function isSupportedImagePath(path: string): boolean { @@ -85,18 +80,8 @@ function imageTooLargeError(): Error { } function sniffImageMime(bytes: Uint8Array): ImageMimeType | undefined { - if (startsWith(bytes, PNG_SIGNATURE)) return 'image/png'; - if (startsWith(bytes, JPEG_SIGNATURE)) return 'image/jpeg'; - if (startsWith(bytes, GIF87A_SIGNATURE) || startsWith(bytes, GIF89A_SIGNATURE)) - return 'image/gif'; - if (startsWith(bytes, RIFF_SIGNATURE) && startsWith(bytes, WEBP_SIGNATURE, 8)) - return 'image/webp'; - return undefined; -} - -function startsWith(bytes: Uint8Array, prefix: Uint8Array, offset = 0): boolean { - return ( - bytes.length >= offset + prefix.length && - prefix.every((value, index) => bytes[offset + index] === value) - ); + // Core owns the byte signatures (shared with the attachment and artifact + // paths); this reader decodes only images, so a sniffed PDF is not one here. + const sniffed = sniffAttachmentMimeType(bytes); + return sniffed && sniffed !== 'application/pdf' ? sniffed : undefined; } diff --git a/packages/storage/src/artifact-store.ts b/packages/storage/src/artifact-store.ts index 814470c707..0ba7d9794c 100644 --- a/packages/storage/src/artifact-store.ts +++ b/packages/storage/src/artifact-store.ts @@ -52,6 +52,7 @@ import { isDeepResearchArtifactRole, type DeepResearchArtifactRole, } from '@maka/core/deep-research-run'; +import { sniffAttachmentMimeType } from '@maka/core/attachments'; import { publishMarkerFile, readBoundedMarkerFile } from './marker-file.js'; import { ARTIFACT_PUBLICATION_STAGING_PATTERN, @@ -1928,17 +1929,11 @@ function isAlreadyExists(error: unknown): boolean { } function sniffAllowedBinaryMime(bytes: Uint8Array): string | null { - if (startsWith(bytes, [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])) return 'image/png'; - if (startsWith(bytes, [0xff, 0xd8, 0xff])) return 'image/jpeg'; - if (asciiStartsWith(bytes, 'GIF87a') || asciiStartsWith(bytes, 'GIF89a')) return 'image/gif'; - if ( - asciiStartsWith(bytes, 'RIFF') && - bytes.length >= 12 && - String.fromCharCode(...bytes.slice(8, 12)) === 'WEBP' - ) { - return 'image/webp'; - } - if (asciiStartsWith(bytes, '%PDF-')) return 'application/pdf'; + // Core owns the binary signatures, shared with the attachment and image-read + // paths so the three cannot drift. SVG needs a wider text scan than a fixed + // prefix, so it stays local to this reader. + const sniffed = sniffAttachmentMimeType(bytes); + if (sniffed) return sniffed; const leading = new TextDecoder('utf-8', { fatal: false }) .decode(bytes.slice(0, Math.min(bytes.length, 512))) .trimStart(); @@ -1946,13 +1941,3 @@ function sniffAllowedBinaryMime(bytes: Uint8Array): string | null { return 'image/svg+xml'; return null; } - -function startsWith(bytes: Uint8Array, prefix: number[]): boolean { - if (bytes.length < prefix.length) return false; - return prefix.every((value, index) => bytes[index] === value); -} - -function asciiStartsWith(bytes: Uint8Array, prefix: string): boolean { - if (bytes.length < prefix.length) return false; - return prefix.split('').every((char, index) => bytes[index] === char.charCodeAt(0)); -}