diff --git a/apps/desktop/electron/hardening.test.ts b/apps/desktop/electron/hardening.test.ts index 4b41962e0cdb..3acf649e6d53 100644 --- a/apps/desktop/electron/hardening.test.ts +++ b/apps/desktop/electron/hardening.test.ts @@ -7,11 +7,13 @@ import { pathToFileURL } from 'node:url' import { test } from 'vitest' import { + ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES, clampDataUrlReadMaxMb, DATA_URL_READ_DEFAULT_MAX_MB, dataUrlReadMaxBytesFromMb, DEFAULT_FETCH_TIMEOUT_MS, encryptDesktopSecret, + readFileDataUrlForIpc, resolveDirectoryForIpc, resolveReadableFileForIpc, resolveRequestedPathForIpc, @@ -35,6 +37,41 @@ test('clampDataUrlReadMaxMb defaults and bounds the attach size preference', () assert.equal(dataUrlReadMaxBytesFromMb(16), 16 * 1024 * 1024) }) +test('attachment upload cap is bounded above the preview default', () => { + assert.equal(ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES, 256 * 1024 * 1024) + assert.ok(ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES > dataUrlReadMaxBytesFromMb(DATA_URL_READ_DEFAULT_MAX_MB)) +}) + +test('attachment data URL helper reads bytes above the preview default without changing that limit', async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-desktop-large-attachment-')) + const source = path.join(tempDir, 'large.bin') + const previewLimit = dataUrlReadMaxBytesFromMb(DATA_URL_READ_DEFAULT_MAX_MB) + const content = Buffer.alloc(previewLimit + 1024, 0x5a) + + try { + fs.writeFileSync(source, content) + + await assert.rejects( + resolveReadableFileForIpc(source, { + maxBytes: previewLimit, + purpose: 'File preview' + }), + /file is too large/ + ) + + const dataUrl = await readFileDataUrlForIpc(source, { + maxBytes: ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES, + mimeType: 'application/octet-stream', + purpose: 'Attachment upload' + }) + + assert.match(dataUrl, /^data:application\/octet-stream;base64,/) + assert.deepEqual(Buffer.from(dataUrl.slice(dataUrl.indexOf(',') + 1), 'base64'), content) + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }) + } +}) + test('resolveTimeoutMs falls back to defaults and accepts overrides', () => { assert.equal(resolveTimeoutMs(undefined), DEFAULT_FETCH_TIMEOUT_MS) assert.equal(resolveTimeoutMs(0), DEFAULT_FETCH_TIMEOUT_MS) diff --git a/apps/desktop/electron/hardening.ts b/apps/desktop/electron/hardening.ts index de15a5dca5e3..e671dc259b79 100644 --- a/apps/desktop/electron/hardening.ts +++ b/apps/desktop/electron/hardening.ts @@ -12,6 +12,10 @@ const DEFAULT_FETCH_TIMEOUT_MS = 15_000 const DATA_URL_READ_DEFAULT_MAX_MB = 16 const DATA_URL_READ_MIN_MAX_MB = 1 const DATA_URL_READ_MAX_MAX_MB = 4096 +// Remote file.attach sends one base64 JSON-RPC frame. Cap the dedicated attach +// reader so the payload still fits uvicorn's raised ws_max_size (384 MiB) +// after base64 + framing. Preview stays on the Settings-configurable path. +const ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES = 256 * 1024 * 1024 const TEXT_PREVIEW_SOURCE_MAX_BYTES = 64 * 1024 * 1024 function clampDataUrlReadMaxMb(value) { @@ -324,7 +328,26 @@ async function resolveReadableFileForIpc( return { realPath, resolvedPath, stat } } +async function readFileDataUrlForIpc( + filePath, + options: { + purpose?: string + baseDir?: fs.PathOrFileDescriptor + fs?: typeof fs + blockSensitive?: boolean + maxBytes?: number + mimeType: string + } +): Promise { + const fsImpl = options.fs || fs + const { resolvedPath } = await resolveReadableFileForIpc(filePath, options) + const data = await fsImpl.promises.readFile(resolvedPath) + + return `data:${options.mimeType};base64,${data.toString('base64')}` +} + export { + ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES, clampDataUrlReadMaxMb, DATA_URL_READ_DEFAULT_MAX_MB, DATA_URL_READ_MAX_MAX_MB, @@ -332,6 +355,7 @@ export { dataUrlReadMaxBytesFromMb, DEFAULT_FETCH_TIMEOUT_MS, encryptDesktopSecret, + readFileDataUrlForIpc, rejectUnsafePathSyntax, resolveDirectoryForIpc, resolveReadableFileForIpc, diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 59bc792c7c6d..0c6d64b19e96 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -115,11 +115,13 @@ import { switchBranch } from './git-worktree-ops' import { + ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES, clampDataUrlReadMaxMb, DATA_URL_READ_DEFAULT_MAX_MB, dataUrlReadMaxBytesFromMb, DEFAULT_FETCH_TIMEOUT_MS, encryptDesktopSecret as encryptDesktopSecretStrict, + readFileDataUrlForIpc, resolveReadableFileForIpc, resolveRequestedPathForIpc, resolveTimeoutMs, @@ -10193,14 +10195,23 @@ ipcMain.handle('hermes:data-url-read-max:set', (_event, maxMb) => { }) ipcMain.handle('hermes:readFileDataUrl', async (_event, filePath) => { - const { resolvedPath } = await resolveReadableFileForIpc(filePath, { + return readFileDataUrlForIpc(filePath, { maxBytes: dataUrlReadMaxBytesFromMb(dataUrlReadMaxMb), + mimeType: mimeTypeForPath(resolveRequestedPathForIpc(filePath, { purpose: 'File preview' })), purpose: 'File preview' }) +}) - const data = await fs.promises.readFile(resolvedPath) - - return `data:${mimeTypeForPath(resolvedPath)};base64,${data.toString('base64')}` +// Remote attachment transfer is independent of the preview / Settings path. +// Keep a finite cap so Electron + base64 memory stays bounded while archives +// can exceed the default 16 MiB preview ceiling (and still fit the gateway +// WebSocket frame limit after base64 expansion). +ipcMain.handle('hermes:readFileDataUrlForAttach', async (_event, filePath) => { + return readFileDataUrlForIpc(filePath, { + maxBytes: ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES, + mimeType: mimeTypeForPath(resolveRequestedPathForIpc(filePath, { purpose: 'Attachment upload' })), + purpose: 'Attachment upload' + }) }) ipcMain.handle('hermes:readFileText', async (_event, filePath) => { diff --git a/apps/desktop/electron/preload.ts b/apps/desktop/electron/preload.ts index b110eb1670b7..31e9fa4159cc 100644 --- a/apps/desktop/electron/preload.ts +++ b/apps/desktop/electron/preload.ts @@ -98,6 +98,7 @@ contextBridge.exposeInMainWorld('hermesDesktop', { notify: payload => ipcRenderer.invoke('hermes:notify', payload), requestMicrophoneAccess: () => ipcRenderer.invoke('hermes:requestMicrophoneAccess'), readFileDataUrl: filePath => ipcRenderer.invoke('hermes:readFileDataUrl', filePath), + readFileDataUrlForAttach: filePath => ipcRenderer.invoke('hermes:readFileDataUrlForAttach', filePath), dataUrlReadMax: { get: () => ipcRenderer.invoke('hermes:data-url-read-max:get'), set: maxMb => ipcRenderer.invoke('hermes:data-url-read-max:set', maxMb) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.test.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.test.ts index 061fd16210d7..bcc6ffc7f03e 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.test.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.test.ts @@ -1,5 +1,5 @@ import type { AppendMessage } from '@assistant-ui/react' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { ChatMessage } from '@/lib/chat-messages' @@ -12,6 +12,7 @@ import { isSessionBusyError, isSessionIdCandidate, isSessionNotFoundError, + readFileDataUrlForAttach, renderRpcResult, slashStatusText, visibleUserIndexAtOrdinal, @@ -86,6 +87,32 @@ describe('friendlyRemoteAttachError', () => { }) }) +describe('readFileDataUrlForAttach', () => { + it('prefers the attachment-specific desktop reader over the preview reader', async () => { + const previewReader = vi.fn(async () => 'preview') + const attachmentReader = vi.fn(async () => 'data:application/zip;base64,UEs=') + Object.defineProperty(window, 'hermesDesktop', { + configurable: true, + value: { readFileDataUrl: previewReader, readFileDataUrlForAttach: attachmentReader } + }) + + await expect(readFileDataUrlForAttach('/tmp/archive.zip')).resolves.toBe('data:application/zip;base64,UEs=') + expect(attachmentReader).toHaveBeenCalledWith('/tmp/archive.zip') + expect(previewReader).not.toHaveBeenCalled() + }) + + it('falls back to the preview reader on older shells', async () => { + const previewReader = vi.fn(async () => 'data:text/plain;base64,YQ==') + Object.defineProperty(window, 'hermesDesktop', { + configurable: true, + value: { readFileDataUrl: previewReader } + }) + + await expect(readFileDataUrlForAttach('/tmp/note.txt')).resolves.toBe('data:text/plain;base64,YQ==') + expect(previewReader).toHaveBeenCalledWith('/tmp/note.txt') + }) +}) + describe('slashStatusText', () => { it('joins command and trimmed output', () => { expect(slashStatusText('/model', ' gpt ')).toBe('slash:/model\ngpt') diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts index d586448aee91..cc9ba164ad75 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts @@ -155,8 +155,10 @@ export async function readImageForRemoteAttach( // Read a non-image file as a data URL for upload via file.attach. Returns null // when the desktop bridge can't read the file (e.g. it was moved/deleted). +// Prefer the attach-specific IPC (256 MiB) so remote uploads are not stuck on +// the preview/Settings default; fall back for older Electron shells. export async function readFileDataUrlForAttach(filePath: string): Promise { - const reader = window.hermesDesktop?.readFileDataUrl + const reader = window.hermesDesktop?.readFileDataUrlForAttach ?? window.hermesDesktop?.readFileDataUrl if (!reader) { return null @@ -167,13 +169,12 @@ export async function readFileDataUrlForAttach(filePath: string): Promise Promise requestMicrophoneAccess: () => Promise readFileDataUrl: (filePath: string) => Promise + /** Remote non-image attach: higher dedicated cap than preview/Settings default. */ + readFileDataUrlForAttach?: (filePath: string) => Promise /** Settings → Chat: max size for local files loaded as data URLs (attach/preview). */ dataUrlReadMax?: { get: () => Promise<{ defaultMaxMb: number; maxBytes: number; maxMb: number }> diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index c1f2f832cf1d..277ad635fa52 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -550,11 +550,11 @@ export const en: Translations = { invalidJson: 'Invalid config JSON', keepAwakeTitle: 'Keep computer awake', keepAwakeDesc: 'Stop this machine from sleeping so long or overnight runs keep going. The display can still dim.', - attachmentSizeTitle: 'Max attachment size', + attachmentSizeTitle: 'Max preview / image load size', attachmentSizeDesc: - 'How big a local file Desktop will load for attach and previews, in MB. Default is 16. This is only a limit on this computer. Setting it very high loads the whole file into memory and can freeze or crash the app.', + 'How big a local file Desktop will load for previews and image attach, in MB. Default is 16. Remote non-image attach uses a separate 256 MB cap. Setting this very high loads the whole file into memory and can freeze or crash the app.', attachmentSizeUnit: 'MB', - attachmentSizeLabel: 'Max attachment size in megabytes' + attachmentSizeLabel: 'Max preview / image load size in megabytes' }, quickEntry: { enabledTitle: 'Quick Entry', diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index cbb6bd72fb10..58e0915ea265 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -760,11 +760,11 @@ export const zh: Translations = { invalidJson: '配置 JSON 无效', keepAwakeTitle: '保持电脑唤醒', keepAwakeDesc: '阻止本机休眠，让长时间或通宵运行继续进行。屏幕仍可变暗。', - attachmentSizeTitle: '附件大小上限', + attachmentSizeTitle: '预览 / 图片加载大小上限', attachmentSizeDesc: - '桌面端为附件和预览加载本地文件的大小上限（MB）。默认为 16。此限制仅作用于本机。设置过大会将整个文件读入内存，可能导致应用卡死或崩溃。', + '桌面端为预览和图片附件加载本地文件的大小上限（MB）。默认为 16。远程非图片附件使用单独的 256 MB 上限。设置过大会将整个文件读入内存，可能导致应用卡死或崩溃。', attachmentSizeUnit: 'MB', - attachmentSizeLabel: '附件大小上限（MB）' + attachmentSizeLabel: '预览 / 图片加载大小上限（MB）' }, quickEntry: { enabledTitle: '快速输入', diff --git a/apps/desktop/src/store/updates.test.ts b/apps/desktop/src/store/updates.test.ts index 1a0dc7f07923..7b19ba15f459 100644 --- a/apps/desktop/src/store/updates.test.ts +++ b/apps/desktop/src/store/updates.test.ts @@ -120,7 +120,7 @@ describe('reportBackendContract', () => { }) it('dismisses the toast when the backend meets the contract', () => { - reportBackendContract(4) + reportBackendContract(5) expect(dismissSpy).toHaveBeenCalledWith('backend-contract-skew') expect(notifySpy).not.toHaveBeenCalled() }) @@ -160,8 +160,8 @@ describe('reportBackendContract', () => { lastToast().onDismiss() notifySpy.mockClear() - reportBackendContract(4) // backend updated → satisfied, snooze cleared - reportBackendContract(3) // a later regression must warn immediately + reportBackendContract(5) // backend updated → satisfied, snooze cleared + reportBackendContract(4) // a later regression must warn immediately expect(notifySpy).toHaveBeenCalledTimes(1) }) }) diff --git a/apps/desktop/src/store/updates.ts b/apps/desktop/src/store/updates.ts index 6c7e1483cf93..1f0998a5e069 100644 --- a/apps/desktop/src/store/updates.ts +++ b/apps/desktop/src/store/updates.ts @@ -93,7 +93,8 @@ function isUpdateToastSnoozed(): boolean { // v2: requires the file.attach RPC (remote-gateway non-image file upload). // v3: requires approvals.mode config RPCs and session.info reconciliation. // v4: requires explicit Fast-off session creation and session-scoped Fast edits. -const REQUIRED_BACKEND_CONTRACT = 4 +// v5: requires raised WebSocket frame size for large one-shot file.attach. +const REQUIRED_BACKEND_CONTRACT = 5 const SKEW_TOAST_ID = 'backend-contract-skew' // The contract check runs on every session.resume (applyRuntimeInfo), so // without a snooze the warning re-popped on every thread the user opened, even diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 572ee5a38eee..8a0cc48e179a 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -318,6 +318,13 @@ def _apply_ssh_owner_nonce(nonce: Optional[str]) -> None: # injection share a single, testable seam. _DASHBOARD_EMBEDDED_CHAT_ENABLED = True +# Desktop's file.attach compatibility transport sends a complete base64 data +# URL in one JSON-RPC frame. Uvicorn defaults to 16 MiB, which rejects files at +# the preview ceiling before the dispatcher sees them. Keep the gateway +# finite while allowing the 256 MiB raw Desktop attach cap plus base64/JSON +# overhead. +_DESKTOP_ATTACHMENT_WS_MAX_BYTES = 384 * 1024 * 1024 + # Simple rate limiter for the reveal endpoint _reveal_timestamps: List[float] = [] _REVEAL_MAX_PER_WINDOW = 5 @@ -20290,6 +20297,7 @@ def start_server( # reaped via the WebSocketDisconnect → disconnect/reap path. ws_ping_interval=None if _is_loopback else 20.0, ws_ping_timeout=None if _is_loopback else 20.0, + ws_max_size=_DESKTOP_ATTACHMENT_WS_MAX_BYTES, ) server = uvicorn.Server(config) diff --git a/tests/test_web_server.py b/tests/test_web_server.py index eee5973879e9..e14d3151d916 100644 --- a/tests/test_web_server.py +++ b/tests/test_web_server.py @@ -107,6 +107,20 @@ def test_start_server_disables_ws_ping_on_loopback(monkeypatch): assert captured["ws_ping_timeout"] is None +def test_start_server_accepts_base64_desktop_attachments_above_preview_limit(monkeypatch): + """The gateway frame cap must fit the Desktop attachment default after + base64 expansion and JSON framing; uvicorn's 16 MiB default would reject + the request before ``file.attach`` can stage it. + """ + captured = _stub_uvicorn(monkeypatch) + + web_server.start_server(host="127.0.0.1", port=0, open_browser=False) + + raw_attachment_bytes = 256 * 1024 * 1024 + base64_bytes = ((raw_attachment_bytes + 2) // 3) * 4 + assert captured["ws_max_size"] > base64_bytes + + def test_start_server_enables_ws_ping_for_half_open_detection(monkeypatch): """Non-loopback (public) binds MUST keep the ws ping enabled so half-open connections (reverse-proxy 524, dropped Cloudflare Tunnel) raise diff --git a/tui_gateway/server.py b/tui_gateway/server.py index c7a24bf9d4c7..ef68e2f7d4fd 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -4528,7 +4528,8 @@ def _current_profile_name() -> str: # v2: adds the file.attach RPC (remote-gateway non-image file upload). # v3: adds approvals.mode config RPCs and session.info reconciliation. # v4: session.create fast=false is an explicit per-session normal-tier override. -DESKTOP_BACKEND_CONTRACT = 4 +# v5: uvicorn ws_max_size raised for one-shot base64 file.attach frames (>16 MiB). +DESKTOP_BACKEND_CONTRACT = 5 def _session_usage_snapshot(session: dict | None) -> dict: