fix(gui): harden multimodal image attachments

packages/attachment/attachment-local/README.md

@@ -1,6 +1,6 @@
 # @deepseek-ai/dsh-attachment-local
 
-The private local implementation of [`@deepseek-ai/dsh-attachment`](../attachment). Objects land at `<DSH_HOME>/attachments/v1/objects/<sha256-prefix>/<sha256>` and are addressed by an opaque `sha256:` id. Writes use a private staging directory, owner-only files, a synced temporary file, and an atomic exclusive hard-link publish; reads re-check the digest, media signature, dimensions, and logged metadata.
+The private local implementation of [`@deepseek-ai/dsh-attachment`](../attachment). Objects land at `<DSH_HOME>/attachments/v1/objects/<sha256-prefix>/<sha256>` and are addressed by an opaque `sha256:` id. Writes use a private staging directory, owner-only files, a synced temporary file, and an atomic exclusive hard-link publish; reads re-check the digest, media signature, dimensions, and logged metadata. Byte and pixel limits are write-time admission policy, so a later policy reduction does not make already-admitted history unreadable.
 
 `DSH_HOME` resolves through the shared path policy: explicit config, `$DSH_HOME`, then `~/.dsh`. Session logs contain only the reference and verified metadata, never this host path.
 

packages/attachment/attachment-local/src/image.ts

@@ -11,31 +11,31 @@ export interface DetectedImage {
 }
 
 function ascii(data: Uint8Array, start: number, value: string): boolean {
+  /* v8 ignore next -- Every call site establishes the fixed header span before comparing it. */
   if (data.length < start + value.length) return false
   for (let i = 0; i < value.length; i++) if (data[start + i] !== value.charCodeAt(i)) return false
   return true
 }
 
 function u16be(data: Uint8Array, offset: number): number {
-  return ((data[offset] ?? 0) << 8) | (data[offset + 1] ?? 0)
+  return new DataView(data.buffer, data.byteOffset, data.byteLength).getUint16(offset)
 }
 
 function u16le(data: Uint8Array, offset: number): number {
-  return (data[offset] ?? 0) | ((data[offset + 1] ?? 0) << 8)
+  return new DataView(data.buffer, data.byteOffset, data.byteLength).getUint16(offset, true)
 }
 
 function u24le(data: Uint8Array, offset: number): number {
-  return (data[offset] ?? 0) | ((data[offset + 1] ?? 0) << 8) | ((data[offset + 2] ?? 0) << 16)
+  const view = new DataView(data.buffer, data.byteOffset, data.byteLength)
+  return view.getUint8(offset) | (view.getUint8(offset + 1) << 8) | (view.getUint8(offset + 2) << 16)
 }
 
 function u32be(data: Uint8Array, offset: number): number {
-  return (((data[offset] ?? 0) * 0x1000000) + ((data[offset + 1] ?? 0) << 16)
-    + ((data[offset + 2] ?? 0) << 8) + (data[offset + 3] ?? 0)) >>> 0
+  return new DataView(data.buffer, data.byteOffset, data.byteLength).getUint32(offset)
 }
 
 function u32le(data: Uint8Array, offset: number): number {
-  return ((data[offset] ?? 0) + ((data[offset + 1] ?? 0) << 8)
-    + ((data[offset + 2] ?? 0) << 16) + ((data[offset + 3] ?? 0) * 0x1000000)) >>> 0
+  return new DataView(data.buffer, data.byteOffset, data.byteLength).getUint32(offset, true)
 }
 
 function dimensions(width: number, height: number, mediaType: ImageMediaType): DetectedImage {
@@ -86,10 +86,11 @@ export function detectImage(data: Uint8Array): DetectedImage {
     if (declaredLength > data.length) throw new AttachmentError('WebP data is truncated.', 'INVALID_IMAGE')
     if (ascii(data, 12, 'VP8X')) return dimensions(u24le(data, 24) + 1, u24le(data, 27) + 1, 'image/webp')
     if (ascii(data, 12, 'VP8L') && data[20] === 0x2f) {
-      const b0 = data[21] ?? 0
-      const b1 = data[22] ?? 0
-      const b2 = data[23] ?? 0
-      const b3 = data[24] ?? 0
+      const view = new DataView(data.buffer, data.byteOffset, data.byteLength)
+      const b0 = view.getUint8(21)
+      const b1 = view.getUint8(22)
+      const b2 = view.getUint8(23)
+      const b3 = view.getUint8(24)
       return dimensions(1 + b0 + ((b1 & 0x3f) << 8), 1 + (b1 >> 6) + (b2 << 2) + ((b3 & 0x0f) << 10), 'image/webp')
     }
     if (ascii(data, 12, 'VP8 ') && data[23] === 0x9d && data[24] === 0x01 && data[25] === 0x2a) {

packages/attachment/attachment-local/src/index.ts

@@ -67,7 +67,7 @@ export class LocalAttachmentStore extends AttachmentStore {
   }
 
   async readImage(ref: ImageAttachmentRef): Promise<StoredImageAttachment> {
-    return readImageFile(this.root, ref, this.imageLimits)
+    return readImageFile(this.root, ref)
   }
 }
 

packages/attachment/attachment-local/src/store.ts

@@ -38,15 +38,20 @@ function ensureReference(ref: ImageAttachmentRef): string {
   return match[1]
 }
 
-function validateMetadata(data: Uint8Array, declaredMediaType: ImageAttachmentRef['mediaType'], limits: ImageAttachmentLimits): Omit<ImageAttachmentRef, 'attachmentId' | 'name'> {
+function inspectMetadata(data: Uint8Array, declaredMediaType: ImageAttachmentRef['mediaType']): Omit<ImageAttachmentRef, 'attachmentId' | 'name'> {
   if (data.byteLength === 0) throw new AttachmentError('Image is empty.', 'INVALID_IMAGE')
-  if (data.byteLength > limits.maxImageBytes) throw new AttachmentError('Image exceeds the configured byte limit.', 'IMAGE_TOO_LARGE')
   const detected = detectImage(data)
   if (detected.mediaType !== declaredMediaType) throw new AttachmentError('Declared image type does not match its bytes.', 'IMAGE_TYPE_MISMATCH')
-  if (detected.width * detected.height > limits.maxImagePixels) throw new AttachmentError('Image exceeds the configured decoded-pixel limit.', 'IMAGE_TOO_MANY_PIXELS')
   return { ...detected, bytes: data.byteLength }
 }
 
+function validateAdmission(metadata: Omit<ImageAttachmentRef, 'attachmentId' | 'name'>, limits: ImageAttachmentLimits): void {
+  if (metadata.bytes > limits.maxImageBytes) throw new AttachmentError('Image exceeds the configured byte limit.', 'IMAGE_TOO_LARGE')
+  if (metadata.width * metadata.height > limits.maxImagePixels) {
+    throw new AttachmentError('Image exceeds the configured decoded-pixel limit.', 'IMAGE_TOO_MANY_PIXELS')
+  }
+}
+
 /**
  * Save and verify immutable image bytes below a versioned attachment root.
  * @param root - absolute `DSH_HOME/attachments/v1` root.
@@ -55,7 +60,8 @@ function validateMetadata(data: Uint8Array, declaredMediaType: ImageAttachmentRe
  * @returns durable content-addressed reference.
  */
 export async function saveImageFile(root: string, input: SaveImageAttachment, limits: ImageAttachmentLimits): Promise<ImageAttachmentRef> {
-  const metadata = validateMetadata(input.data, input.mediaType, limits)
+  const metadata = inspectMetadata(input.data, input.mediaType)
+  validateAdmission(metadata, limits)
   const sha256 = digest(input.data)
   const bucket = join(root, 'objects', sha256.slice(0, 2))
   const staging = join(root, 'tmp')
@@ -75,16 +81,25 @@ export async function saveImageFile(root: string, input: SaveImageAttachment, li
     try {
       await link(temporary, target)
     } catch (error) {
+      /* v8 ignore next -- Private same-filesystem directories make EEXIST the only recoverable link race. */
       if (!(error instanceof Error && 'code' in error && error.code === 'EEXIST')) throw error
       const existing = new Uint8Array(await readFile(target))
       if (digest(existing) !== sha256) throw new AttachmentError('Stored attachment failed integrity verification.', 'ATTACHMENT_CORRUPT')
     }
     await unlink(temporary)
   } catch (error) {
-    if (handle !== undefined) await handle.close().catch(() => { /* close failure is superseded by the storage failure */ })
-    await unlink(temporary).catch((cleanupError: unknown) => {
-      if (!(cleanupError instanceof Error && 'code' in cleanupError && cleanupError.code === 'ENOENT')) throw cleanupError
-    })
+    /* v8 ignore next -- A descriptor can remain open only when the underlying write/sync/close operation fails. */
+    if (handle !== undefined) await handle.close().catch(
+      /* v8 ignore next -- Close failure is superseded by the storage operation that entered cleanup. */
+      () => {},
+    )
+    await unlink(temporary).catch(
+      /* v8 ignore next -- The callback requires a second independent staging-unlink failure. */
+      (cleanupError: unknown) => {
+        /* v8 ignore next -- Cleanup is best-effort only for a staging file already removed by a failed operation. */
+        if (!(cleanupError instanceof Error && 'code' in cleanupError && cleanupError.code === 'ENOENT')) throw cleanupError
+      },
+    )
     if (error instanceof AttachmentError) throw error
     throw new AttachmentError('Unable to persist image attachment.', 'ATTACHMENT_WRITE_FAILED', { cause: error })
   }
@@ -100,10 +115,9 @@ export async function saveImageFile(root: string, input: SaveImageAttachment, li
  * Read and verify one content-addressed image.
  * @param root - absolute `DSH_HOME/attachments/v1` root.
  * @param ref - reference recorded in the session log.
- * @param limits - resolved storage policy.
  * @returns verified bytes and reference.
  */
-export async function readImageFile(root: string, ref: ImageAttachmentRef, limits: ImageAttachmentLimits): Promise<StoredImageAttachment> {
+export async function readImageFile(root: string, ref: ImageAttachmentRef): Promise<StoredImageAttachment> {
   const sha256 = ensureReference(ref)
   let data: Uint8Array
   try {
@@ -113,7 +127,7 @@ export async function readImageFile(root: string, ref: ImageAttachmentRef, limit
     throw new AttachmentError('Unable to read image attachment.', 'ATTACHMENT_READ_FAILED', { cause: error })
   }
   if (digest(data) !== sha256) throw new AttachmentError('Stored attachment failed integrity verification.', 'ATTACHMENT_CORRUPT')
-  const metadata = validateMetadata(data, ref.mediaType, limits)
+  const metadata = inspectMetadata(data, ref.mediaType)
   if (metadata.bytes !== ref.bytes || metadata.width !== ref.width || metadata.height !== ref.height) {
     throw new AttachmentError('Stored attachment metadata does not match its reference.', 'ATTACHMENT_CORRUPT')
   }

packages/attachment/attachment-local/tests/image.spec.ts

@@ -0,0 +1,93 @@
+import { describe, expect, it } from 'vitest'
+import { detectImage } from '../src/image.ts'
+
+function bytes(text: string): number[] {
+  return [...Buffer.from(text, 'ascii')]
+}
+
+function webp(chunk: string, mutate: (data: Uint8Array) => void): Uint8Array {
+  const data = new Uint8Array(30)
+  data.set(bytes('RIFF'), 0)
+  data.set([22, 0, 0, 0], 4)
+  data.set(bytes('WEBP'), 8)
+  data.set(bytes(chunk), 12)
+  mutate(data)
+  return data
+}
+
+describe('raster header detection', () => {
+  it('detects PNG dimensions', () => {
+    const data = Uint8Array.from(Buffer.from(
+      'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=',
+      'base64',
+    ))
+    expect(detectImage(data)).toEqual({ mediaType: 'image/png', width: 1, height: 1 })
+  })
+
+  it('detects both GIF revisions and rejects zero dimensions', () => {
+    expect(detectImage(Uint8Array.from([...bytes('GIF87a'), 3, 0, 2, 0])))
+      .toEqual({ mediaType: 'image/gif', width: 3, height: 2 })
+    expect(detectImage(Uint8Array.from([...bytes('GIF89a'), 4, 0, 5, 0])))
+      .toEqual({ mediaType: 'image/gif', width: 4, height: 5 })
+    expect(() => detectImage(Uint8Array.from([...bytes('GIF89a'), 0, 0, 1, 0])))
+      .toThrow(/positive/)
+    expect(() => detectImage(Uint8Array.from([...bytes('GIF89a'), 1, 0, 0, 0])))
+      .toThrow(/positive/)
+  })
+
+  it('walks JPEG marker forms and reports malformed dimensions', () => {
+    const sof = [0xff, 0xc0, 0, 7, 8, 0, 2, 0, 3]
+    expect(detectImage(Uint8Array.from([0xff, 0xd8, ...sof])))
+      .toEqual({ mediaType: 'image/jpeg', width: 3, height: 2 })
+    expect(detectImage(Uint8Array.from([
+      0xff, 0xd8,
+      0xe0, 0, 2,
+      0x01,
+      0xff, ...sof,
+    ]))).toEqual({ mediaType: 'image/jpeg', width: 3, height: 2 })
+
+    expect(() => detectImage(Uint8Array.from([0xff, 0xd8, 0xd9, 0, 0, 0])))
+      .toThrow(/missing/)
+    expect(() => detectImage(Uint8Array.from([0xff, 0xd8, 0xff, 0xff, 0xff, 0xff])))
+      .toThrow(/missing/)
+    expect(() => detectImage(Uint8Array.from([0xff, 0xd8, 0xe0, 0, 1, 0])))
+      .toThrow(/truncated/)
+    expect(() => detectImage(Uint8Array.from([0xff, 0xd8, 0xe0, 0, 9, 0])))
+      .toThrow(/truncated/)
+    expect(() => detectImage(Uint8Array.from([0xff, 0xd8, 0xc0, 0, 6, 0, 0, 0, 0])))
+      .toThrow(/dimensions are truncated/)
+  })
+
+  it('detects each WebP header and rejects truncated or unknown chunks', () => {
+    expect(detectImage(webp('VP8X', (data) => {
+      data.set([2, 0, 0], 24)
+      data.set([3, 0, 0], 27)
+    }))).toEqual({ mediaType: 'image/webp', width: 3, height: 4 })
+
+    expect(detectImage(webp('VP8L', (data) => {
+      data[20] = 0x2f
+      data.set([2, 0, 1, 0], 21)
+    }))).toEqual({ mediaType: 'image/webp', width: 3, height: 5 })
+
+    expect(detectImage(webp('VP8 ', (data) => {
+      data.set([0x9d, 0x01, 0x2a], 23)
+      data.set([6, 0, 7, 0], 26)
+    }))).toEqual({ mediaType: 'image/webp', width: 6, height: 7 })
+
+    const truncated = webp('VP8X', () => {})
+    truncated[4] = 23
+    expect(() => detectImage(truncated)).toThrow(/truncated/)
+    expect(() => detectImage(webp('NOPE', () => {}))).toThrow(/dimensions are missing/)
+    expect(() => detectImage(webp('VP8L', () => {}))).toThrow(/dimensions are missing/)
+    expect(() => detectImage(webp('VP8 ', () => {}))).toThrow(/dimensions are missing/)
+  })
+
+  it('rejects unrecognized bytes and near-miss signatures', () => {
+    expect(() => detectImage(new Uint8Array(0))).toThrow(/Unsupported/)
+    expect(() => detectImage(Uint8Array.from([...bytes('GIFxxa'), 1, 0, 1, 0])))
+      .toThrow(/Unsupported/)
+    const nearWebp = webp('VP8X', () => {})
+    nearWebp[8] = 0
+    expect(() => detectImage(nearWebp)).toThrow(/Unsupported/)
+  })
+})

packages/attachment/attachment-local/tests/index.spec.ts

@@ -0,0 +1,39 @@
+import { Context } from 'cordis'
+import { mkdtemp, rm } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { describe, expect, it } from 'vitest'
+import LocalAttachmentStore, {
+  DEFAULT_MAX_IMAGE_BYTES,
+  DEFAULT_MAX_IMAGE_PIXELS,
+  DEFAULT_MAX_IMAGES_PER_MESSAGE,
+  DEFAULT_MAX_MESSAGE_IMAGE_BYTES,
+} from '../src/index.ts'
+
+describe('local attachment service', () => {
+  it('resolves every omitted admission limit explicitly', () => {
+    const service = new LocalAttachmentStore(new Context(), {})
+    expect(service.imageLimits).toEqual({
+      maxImageBytes: DEFAULT_MAX_IMAGE_BYTES,
+      maxImagesPerMessage: DEFAULT_MAX_IMAGES_PER_MESSAGE,
+      maxMessageImageBytes: DEFAULT_MAX_MESSAGE_IMAGE_BYTES,
+      maxImagePixels: DEFAULT_MAX_IMAGE_PIXELS,
+      mediaTypes: ['image/png', 'image/jpeg', 'image/webp', 'image/gif'],
+    })
+  })
+
+  it('saves and reads through the service boundary', async () => {
+    const dshHome = await mkdtemp(join(tmpdir(), 'dsh-attachment-service-'))
+    try {
+      const service = new LocalAttachmentStore(new Context(), { dshHome })
+      const data = Uint8Array.from(Buffer.from(
+        'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=',
+        'base64',
+      ))
+      const ref = await service.saveImage({ data, mediaType: 'image/png' })
+      await expect(service.readImage(ref)).resolves.toEqual({ ref, data })
+    } finally {
+      await rm(dshHome, { recursive: true, force: true })
+    }
+  })
+})

packages/attachment/attachment-local/tests/store.spec.ts

@@ -54,11 +54,21 @@ describe('local attachment store', () => {
     expect(new Uint8Array(await readFile(object))).toEqual(PNG)
     expect((await stat(object)).mode & 0o777).toBe(0o600)
     expect((await stat(join(storageRoot, 'objects', sha256.slice(0, 2)))).mode & 0o777).toBe(0o700)
-    await expect(readImageFile(storageRoot, first, LIMITS)).resolves.toEqual({ ref: first, data: PNG })
+    await expect(readImageFile(storageRoot, first)).resolves.toEqual({ ref: first, data: PNG })
+  })
+
+  it('keeps admitted history readable after deployment limits become stricter', async () => {
+    const storageRoot = await root()
+    const ref = await saveImageFile(storageRoot, { data: PNG, mediaType: 'image/png' }, LIMITS)
+
+    await expect(readImageFile(storageRoot, ref)).resolves.toEqual({ ref, data: PNG })
   })
 
   it('rejects malformed bytes, mismatched declarations, byte limits, and decoded-pixel limits', async () => {
     const storageRoot = await root()
+    await expect(saveImageFile(storageRoot, {
+      data: new Uint8Array(0), mediaType: 'image/png',
+    }, LIMITS)).rejects.toMatchObject({ code: 'INVALID_IMAGE' })
     await expect(saveImageFile(storageRoot, {
       data: Uint8Array.of(1, 2, 3), mediaType: 'image/png',
     }, LIMITS)).rejects.toMatchObject({ code: 'INVALID_IMAGE' })
@@ -74,6 +84,10 @@ describe('local attachment store', () => {
     await expect(saveImageFile(storageRoot, {
       data: wide, mediaType: 'image/png',
     }, LIMITS)).rejects.toMatchObject({ code: 'IMAGE_TOO_MANY_PIXELS' })
+    const unnamed = await saveImageFile(storageRoot, {
+      data: PNG, mediaType: 'image/png', name: '\u0000',
+    }, LIMITS)
+    expect(unnamed).not.toHaveProperty('name')
   })
 
   it('fails closed when an object is missing, corrupted, or addressed by an invalid reference', async () => {
@@ -83,14 +97,45 @@ describe('local attachment store', () => {
     const object = join(storageRoot, 'objects', sha256.slice(0, 2), sha256)
     await chmod(object, 0o600)
     await writeFile(object, Uint8Array.of(1, 2, 3))
-    await expect(readImageFile(storageRoot, ref, LIMITS))
+    await expect(readImageFile(storageRoot, ref))
       .rejects.toMatchObject({ code: 'ATTACHMENT_CORRUPT' })
-    await expect(readImageFile(storageRoot, { ...ref, attachmentId: 'bad' as never }, LIMITS))
+    await expect(readImageFile(storageRoot, { ...ref, attachmentId: 'bad' as never }))
       .rejects.toMatchObject({ code: 'INVALID_ATTACHMENT_REF' })
 
     const missingRoot = await root()
     await mkdir(missingRoot, { recursive: true })
-    await expect(readImageFile(missingRoot, ref, LIMITS))
+    await expect(readImageFile(missingRoot, ref))
       .rejects.toMatchObject({ code: 'ATTACHMENT_NOT_FOUND' })
+
+    const unreadableRoot = await root()
+    const target = join(unreadableRoot, 'objects', sha256.slice(0, 2), sha256)
+    await mkdir(target, { recursive: true })
+    await expect(readImageFile(unreadableRoot, ref))
+      .rejects.toMatchObject({ code: 'ATTACHMENT_READ_FAILED' })
+  })
+
+  it('rejects conflicting existing objects and reference metadata mismatches', async () => {
+    const storageRoot = await root()
+    const sha256 = createHash('sha256').update(PNG).digest('hex')
+    const target = join(storageRoot, 'objects', sha256.slice(0, 2), sha256)
+    await mkdir(join(storageRoot, 'objects', sha256.slice(0, 2)), { recursive: true })
+    await writeFile(target, Uint8Array.of(1, 2, 3))
+    await expect(saveImageFile(storageRoot, { data: PNG, mediaType: 'image/png' }, LIMITS))
+      .rejects.toMatchObject({ code: 'ATTACHMENT_CORRUPT' })
+
+    await writeFile(target, PNG)
+    const ref = await saveImageFile(storageRoot, { data: PNG, mediaType: 'image/png' }, LIMITS)
+    await expect(readImageFile(storageRoot, { ...ref, width: ref.width + 1 }))
+      .rejects.toMatchObject({ code: 'ATTACHMENT_CORRUPT' })
+  })
+
+  it('maps unexpected publication failures to a stable storage error', async () => {
+    const storageRoot = await root()
+    const sha256 = createHash('sha256').update(PNG).digest('hex')
+    const target = join(storageRoot, 'objects', sha256.slice(0, 2), sha256)
+    await mkdir(target, { recursive: true })
+
+    await expect(saveImageFile(storageRoot, { data: PNG, mediaType: 'image/png' }, LIMITS))
+      .rejects.toMatchObject({ code: 'ATTACHMENT_WRITE_FAILED' })
   })
 })