Merge pull request #3368 from deepseek-harness/fix/directory-picker-win32-overread

fix(directory-picker-native): decode the Win32 picker path without a fixed 32KiB view

.agents/notes/implemented/bug-fix/2026-08-31-win32-picker-path-string-read.i18n.yaml

@@ -0,0 +1,6 @@
+# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each
+# side as of the last confirmed-consistent state. Both languages carry equal authority;
+# after editing either side, bring the other along and re-record with:
+#   pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-31-win32-picker-path-string-read.md
+2026-08-31-win32-picker-path-string-read.md: 63b2a2ea5ee88b5fa3ac79aca98350f58067fd1e
+2026-08-31-win32-picker-path-string-read.zh.md: 8afc6610a0cf1c7c8649f9e1c62c26b3d89f5a03

.agents/notes/implemented/bug-fix/2026-08-31-win32-picker-path-string-read.md

@@ -0,0 +1,25 @@
+# Agent Note: Read the Win32 picker path without a fixed-size unmanaged view
+
+Status: implemented
+
+English | [中文](2026-08-31-win32-picker-path-string-read.zh.md)
+
+## Problem
+
+The Win32 picker needs to decode a NUL-terminated UTF-16 string allocated by `IShellItem::GetDisplayName` and release it through `CoTaskMemFree`. A fixed-length external ArrayBuffer adds a runtime requirement and manual terminator scanning without providing the allocation size.
+
+## Decision
+
+`readUtf16` stores the native address in a pointer-width buffer and passes it to generic `koffi.decode(buffer, 'str16')`. Generic decoding expects a pointer variable, not the string address directly. The slice follows `koffi.sizeof('void *')`; Koffi 3 represents native addresses as BigInt. The allocation must remain valid and NUL-terminated during decoding. Successful conversion leaves the original address available for `CoTaskMemFree`; if decoding throws, the string is not freed.
+
+## Alternatives considered
+
+**External view and manual scan.** This requires external-buffer support and duplicates Koffi's string conversion. Neither a fixed view nor growing chunks establish the native allocation size.
+
+**String-typed out-param.** `_Out_ str16 *` returns text but discards the pointer needed for explicit COM release. Built-in `str16!` disposal uses the CRT allocator rather than the COM allocator.
+
+**Custom disposable type.** A Koffi disposable can call `CoTaskMemFree`, but explicit conversion keeps the address and release at one call site without registering a native type.
+
+## Consequences
+
+Real-Koffi tests exercise the production result-path conversion over live UTF-16 buffers, including U+5F00, surrogate pairs, NUL termination and strings exceeding 32 KiB. Separate four- and eight-byte BigInt cases verify pointer preservation and release of the original address. Test-owned buffers stay live through the synchronous read; pointer bytes are checked before native dereferencing. The earlier scanning decision remains in the [archived note](../../archived/bug-fix/2026-08-23-win32-utf16-nul-truncation.md).

.agents/notes/implemented/bug-fix/2026-08-31-win32-picker-path-string-read.zh.md

@@ -0,0 +1,25 @@
+# Agent Note: 不用固定长度的非托管视图读取 Win32 选择器路径
+
+Status: implemented
+
+[English](2026-08-31-win32-picker-path-string-read.md) | 中文
+
+## Problem
+
+Win32 选择器需要解码 `IShellItem::GetDisplayName` 分配的 NUL 结尾 UTF-16 字符串，并通过 `CoTaskMemFree` 释放它。固定长度的外部 ArrayBuffer 增加了运行时要求和手工终止符扫描，却不能提供实际分配大小。
+
+## Decision
+
+`readUtf16` 将原生地址存入指针宽度的缓冲区，再交给通用 `koffi.decode(buffer, 'str16')`。通用解码需要指针变量，而非直接传入字符串地址。切片长度取自 `koffi.sizeof('void *')`；Koffi 3 用 BigInt 表示原生地址。解码期间分配必须保持有效且以 NUL 结尾。转换成功后，原始地址仍可交给 `CoTaskMemFree`；若解码抛错，字符串不会被释放。
+
+## Alternatives considered
+
+**外部视图加手工扫描。** 这要求运行时支持外部缓冲区，并重复实现 Koffi 的字符串转换。固定视图和递增分块都无法确定原生分配大小。
+
+**字符串类型出参。** `_Out_ str16 *` 返回文本，却丢失显式 COM 释放所需的指针。内置 `str16!` 使用 CRT 分配器释放，而非 COM 分配器。
+
+**自定义可释放类型。** Koffi 可释放类型可以调用 `CoTaskMemFree`，但显式转换无需注册原生类型，就能将地址和释放保留在同一调用点。
+
+## Consequences
+
+真实 Koffi 测试使用存活的 UTF-16 缓冲区执行生产结果路径转换，涵盖 U+5F00、代理对、NUL 终止和超过 32 KiB 的字符串。独立的四字节与八字节 BigInt 用例验证指针保持完整，并释放原始地址。测试持有的缓冲区在同步读取期间保持存活；原生解引用之前会检查指针字节。此前的扫描决策保留在[归档记录](../../archived/bug-fix/2026-08-23-win32-utf16-nul-truncation.md)中。

packages/host/directory-picker-native/src/win32-dialog-bindings.ts

@@ -25,22 +25,22 @@ interface Koffi {
   register(fn: (...args: unknown[]) => unknown, type: unknown): unknown
   unregister(callback: unknown): void
   sizeof(type: string): number
-  view(ref: unknown, len: number): ArrayBuffer
 }
 
 /**
- * Read a NUL-terminated UTF-16 string at a native address. koffi's
- * `_Out_ void **` out-params surface a raw address, and
- * `koffi.decode(addr, 'str16')` would dereference it as a pointer — crash
- * on real Windows — so view the memory directly instead.
+ * Read a valid NUL-terminated UTF-16 allocation without an external buffer.
+ * Generic `koffi.decode(..., 'str16')` expects a pointer variable, so the
+ * buffer holds the string address rather than the string bytes.
+ * @param koffi - the loaded koffi binding.
+ * @param address - the string address surfaced by the `_Out_ void **` param.
+ * @param pointerSize - the process's pointer width (`koffi.sizeof('void *')`).
+ * @returns the decoded UTF-16 path.
  */
-function readUtf16(koffi: Koffi, address: unknown): string {
-  const bytes = Buffer.from(koffi.view(address, 32768))
-  let end = 0
-  // UTF-16LE NUL is two zero bytes. A single zero low byte is a valid BMP
-  // code unit (U+XX00, e.g. 开 = U+5F00) and must not terminate the scan.
-  while (end + 1 < bytes.length && !(bytes[end] === 0 && bytes[end + 1] === 0)) end += 2
-  return bytes.toString('utf16le', 0, end)
+function readUtf16(koffi: Koffi, address: unknown, pointerSize: number): string {
+  const pointer = Buffer.alloc(8)
+  // koffi 3 surfaces `_Out_ void **` values as BigInt native addresses.
+  pointer.writeBigUInt64LE(BigInt(address as bigint | number))
+  return koffi.decode(pointer.subarray(0, pointerSize), 'str16') as string
 }
 
 const COINIT_APARTMENTTHREADED = 0x2
@@ -158,7 +158,7 @@ export async function loadWin32DialogBindings(): Promise<Win32DialogBindings> {
             const nameOut: unknown[] = [null]
             const gotName = method(item, SLOT_GET_DISPLAY_NAME, protoGetDisplayName)(SIGDN_FILESYSPATH, nameOut)
             if (gotName < 0) return { hr: gotName }
-            const path = readUtf16(koffi, nameOut[0])
+            const path = readUtf16(koffi, nameOut[0], pointerSize)
             coTaskMemFree(nameOut[0])
             return { hr: gotName, path }
           } finally {

packages/host/directory-picker-native/tests/win32-dialog-bindings.spec.ts

@@ -4,8 +4,8 @@
  * COM world stands in for ole32/user32/kernel32, keeping the vtable dispatch,
  * result extraction, memory hygiene, and the WM_CLOSE poster covered on every
  * host. The worker entry is exercised the same way with a mocked process
- * boundary (env title + `process.send`). Real-COM behavior is pinned by the
- * win32-only smoke in win32-dialog.spec.ts.
+ * boundary (env title + `process.send`). String conversion also runs through
+ * real Koffi over test-owned buffers; only the Windows libraries are faked.
  */
 
 import { afterEach, describe, expect, it, vi } from 'vitest'
@@ -13,11 +13,7 @@ import { HRESULT_CANCELLED, runFolderDialog } from '../src/win32-dialog-logic.ts
 
 const E_FAIL = 0x80004005 | 0
 const WM_CLOSE = 0x10
-/**
- * Deliberately NOT 8: the bindings must derive vtable offsets and out-buffer
- * sizes from koffi.sizeof('void *'), and a hardcoded 8 anywhere fails against
- * this width (the win32-ia32 bug class).
- */
+/** Four-byte defaults catch hardcoded x64 vtable offsets on every host. */
 const FAKE_POINTER_SIZE = 4
 
 interface ComWorld {
@@ -37,6 +33,8 @@ interface ComWorld {
   freed: unknown[]
   released: string[]
   posted: { hwnd: unknown; message: number }[]
+  /** Buffer sizes handed to the fake `decode(..., 'str16')`. */
+  str16PointerSizes: number[]
   registered: number
   unregistered: number
   uninitialized: number
@@ -48,19 +46,31 @@ function comWorld(overrides: Partial<ComWorld> = {}): ComWorld {
     hasThreadDpi: true, supportedDpiContexts: [-4], enumThrows: false,
     path: 'C:\\选中\\directory',
     titles: [], options: [], dpiContexts: [], freed: [], released: [], posted: [],
+    str16PointerSizes: [],
     registered: 0, unregistered: 0, uninitialized: 0,
     ...overrides,
   }
 }
 
-/** Sentinel pointer objects standing in for native addresses. */
+/** Sentinel pointer addresses standing in for native pointers. */
 interface FakePtr { kind: string; [key: string]: unknown }
 
-function installFakeKoffi(world: ComWorld): void {
-  const dialogPtr: FakePtr = { kind: 'dialog' }
-  const itemPtr: FakePtr = { kind: 'item' }
-  const namePtr: FakePtr = { kind: 'name', text: world.path }
-  const outBuffers = new Map<unknown, FakePtr>()
+function installFakeKoffi(world: ComWorld, options: {
+  pointerSize?: number
+  nameAddress?: bigint
+  decodeString?: (pointer: Buffer) => string
+} = {}): void {
+  const pointerSize = options.pointerSize ?? FAKE_POINTER_SIZE
+  const dialogPtr = 0x1100n
+  const itemPtr = 0x2200n
+  const namePtr = options.nameAddress ?? 0x3300n
+  const pointers = new Map<bigint, FakePtr>([
+    [dialogPtr, { kind: 'dialog' }],
+    [itemPtr, { kind: 'item' }],
+    [namePtr, { kind: 'name', text: world.path }],
+  ])
+  const outBuffers = new Map<unknown, bigint>()
+  world.str16PointerSizes = []
 
   const dispatch = (self: FakePtr, slot: number, args: unknown[]): number => {
     if (self.kind === 'dialog') {
@@ -98,8 +108,8 @@ function installFakeKoffi(world: ComWorld): void {
             case 'CoCreateInstance': return (...args: unknown[]) => {
               if (world.coCreateHr < 0) return world.coCreateHr
               // The out-pointer must be allocated at the fake's pointer width.
-              if ((args[4] as Buffer).length !== FAKE_POINTER_SIZE) {
-                throw new Error(`CoCreateInstance out buffer must be ${FAKE_POINTER_SIZE} bytes`)
+              if ((args[4] as Buffer).length !== pointerSize) {
+                throw new Error(`CoCreateInstance out buffer must be ${pointerSize} bytes`)
               }
               outBuffers.set(args[4], dialogPtr)
               return 0
@@ -126,24 +136,27 @@ function installFakeKoffi(world: ComWorld): void {
       }),
       proto: (declaration: string) => ({ declaration }),
       pointer: (type: unknown) => type,
-      sizeof: (type: string) => { void type; return FAKE_POINTER_SIZE },
-      view: (value: unknown, len: number): ArrayBuffer => {
-        const bytes = Buffer.alloc(len)
-        bytes.write((value as FakePtr).text as string, 'utf16le')
-        return bytes.buffer
-      },
+      sizeof: (type: string) => { void type; return pointerSize },
       register: (fn: (hwnd: unknown, lparam: unknown) => number) => { world.registered += 1; return { fn } },
       unregister: () => { world.unregistered += 1 },
       decode: (value: unknown, offsetOrType: unknown): unknown => {
-        if (offsetOrType === 'str16') return (value as FakePtr).text
+        if (offsetOrType === 'str16') {
+          const buffer = value as Buffer
+          if (buffer.length !== pointerSize) throw new Error(`str16 pointer buffer must be ${pointerSize} bytes, got ${buffer.length}`)
+          world.str16PointerSizes.push(buffer.length)
+          if (options.decodeString) return options.decodeString(buffer)
+          const address = pointerSize === 8 ? buffer.readBigUInt64LE(0) : BigInt(buffer.readUInt32LE(0))
+          return pointers.get(address)?.text
+        }
         if (typeof offsetOrType === 'number') {
           // Vtable slot read: offsets must be multiples of the fake width.
-          if (offsetOrType % FAKE_POINTER_SIZE !== 0) throw new Error(`vtable offset ${offsetOrType} is not pointer-aligned`)
+          if (offsetOrType % pointerSize !== 0) throw new Error(`vtable offset ${offsetOrType} is not pointer-aligned`)
           const owner = (value as { owner: FakePtr }).owner
-          return { call: (args: unknown[]) => dispatch(owner, offsetOrType / FAKE_POINTER_SIZE, args) }
+          return { call: (args: unknown[]) => dispatch(owner, offsetOrType / pointerSize, args) }
         }
-        // decode(x, 'void *'): out-buffer read or vtable read.
+        // decode(x, 'void *'): out-buffer read or object-pointer read.
         if (outBuffers.has(value)) return outBuffers.get(value)
+        if (typeof value === 'bigint') return { owner: pointers.get(value) }
         return { owner: value as FakePtr }
       },
       call: (fn: { call: (args: unknown[]) => number }, _proto: unknown, _self: unknown, ...args: unknown[]) => fn.call(args),
@@ -176,17 +189,53 @@ describe('loadWin32DialogBindings over the fake COM world', () => {
     expect(world.options).toHaveLength(1)
     expect(showing).toHaveBeenCalledWith(31337)
     expect(world.freed).toHaveLength(1)
+    expect(world.str16PointerSizes).toEqual([FAKE_POINTER_SIZE])
     expect(world.released).toEqual(['item', 'dialog'])
     expect(world.uninitialized).toBe(1)
   })
 
-  it('reads a UTF-16 path whose BMP code unit has a zero low byte (U+5F00 开)', async () => {
-    // 开 = U+5F00 → UTF-16LE bytes 00 5F. A scan that treats any zero low
-    // byte as NUL truncates here and returns the nonexistent ...\安卓.
-    const world = comWorld({ path: 'C:\\fixture\\安卓开发' })
-    installFakeKoffi(world)
+  it.each([
+    { pointerSize: 4, nameAddress: 0xf1233300n },
+    { pointerSize: 8, nameAddress: 0x123456783300n },
+  ])('preserves and frees a $pointerSize-byte BigInt address', async ({ pointerSize, nameAddress }) => {
+    const world = comWorld()
+    installFakeKoffi(world, { pointerSize, nameAddress })
     const bindings = await (await loadBindingsModule()).loadWin32DialogBindings()
-    expect(runFolderDialog(bindings, 'Pick', vi.fn())).toBe('C:\\fixture\\安卓开发')
+    expect(runFolderDialog(bindings, 'Pick', vi.fn())).toBe(world.path)
+    expect(world.str16PointerSizes).toEqual([pointerSize])
+    expect(world.freed).toEqual([nameAddress])
+    expect(world.released).toEqual(['item', 'dialog'])
+  })
+
+  it.each([
+    { label: 'zero-low-byte BMP code units', text: 'C:/fixture/安卓开发' },
+    { label: 'surrogate pairs', text: 'C:/fixture/😀' },
+    { label: 'more than 32 KiB', text: 'C:/' + '开'.repeat(17_000) },
+    { label: 'NUL termination', text: 'C:/开' + String.fromCharCode(0) + 'ignored' },
+  ])('decodes $label with real Koffi through resultPath', async ({ text }) => {
+    const { default: koffi } = await vi.importActual<typeof import('koffi')>('koffi')
+    const nul = String.fromCharCode(0)
+    const storage = Buffer.from(text + nul, 'utf16le')
+    const nameAddress = koffi.address(storage)
+    const pointerSize = koffi.sizeof('void *')
+    const expectedPointer = Buffer.alloc(pointerSize)
+    koffi.encode(expectedPointer, 'void *', nameAddress)
+    const world = comWorld()
+    const decodeString = vi.fn((pointer: Buffer): string => {
+      // Reject bad serialization before native code could dereference it.
+      expect(pointer).toEqual(expectedPointer)
+      return koffi.decode(pointer, 'str16') as string
+    })
+    installFakeKoffi(world, { pointerSize, nameAddress, decodeString })
+    const bindings = await (await loadBindingsModule()).loadWin32DialogBindings()
+
+    expect(runFolderDialog(bindings, 'Pick', vi.fn())).toBe(text.split(nul)[0])
+    expect(decodeString).toHaveBeenCalledOnce()
+    expect(world.freed).toEqual([nameAddress])
+    expect(world.released).toEqual(['item', 'dialog'])
+    expect(world.uninitialized).toBe(1)
+    // Keep the JS-owned allocation live until the synchronous native read ends.
+    expect(storage.toString('utf16le')).toBe(text + nul)
   })
 
   it('maps dismissal and the S_FALSE CoInitializeEx', async () => {