From 478d772f2cc6421fda1d115137b8da92cd4f7d1a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 7 Sep 2026 03:44:49 -0700 Subject: [PATCH] fix(desktop): resolve artifact downloads in their originating session --- .../electron/gateway-file-download.test.ts | 4 +- .../desktop/electron/gateway-file-download.ts | 26 ++---- apps/desktop/electron/main.ts | 7 +- .../src/app/artifacts/artifact-utils.ts | 22 ++--- apps/desktop/src/app/artifacts/index.tsx | 14 +-- .../src/app/artifacts/remote-open.test.tsx | 86 +++++++++++++++---- apps/desktop/src/global.d.ts | 1 + apps/desktop/src/lib/media.remote.test.ts | 2 +- apps/desktop/src/lib/media.ts | 21 ++++- hermes_cli/web_routers/files.py | 30 ++++++- hermes_cli/web_server_files.py | 16 ++-- tests/hermes_cli/test_web_server_files.py | 44 ++++++++++ website/docs/user-guide/desktop.md | 2 + 13 files changed, 200 insertions(+), 75 deletions(-) diff --git a/apps/desktop/electron/gateway-file-download.test.ts b/apps/desktop/electron/gateway-file-download.test.ts index a04265dbd1..7683d0192f 100644 --- a/apps/desktop/electron/gateway-file-download.test.ts +++ b/apps/desktop/electron/gateway-file-download.test.ts @@ -397,9 +397,9 @@ test('filenameFromContentDisposition prefers filename* and reduces to a basename assert.equal(filenameFromContentDisposition(undefined), '') }) -test('gatewayFilePath normalizes bare paths and file:// URLs', () => { +test('gatewayFilePath preserves bare paths and file:// URLs for gateway-native conversion', () => { assert.equal(gatewayFilePath('/Users/me/report.md'), '/Users/me/report.md') - assert.equal(gatewayFilePath('file:///Users/me/a%20b.md'), '/Users/me/a b.md') + assert.equal(gatewayFilePath('file:///Users/me/a%20b.md'), 'file:///Users/me/a%20b.md') assert.equal(gatewayFilePath(''), '') assert.equal(gatewayFilePath(null), '') }) diff --git a/apps/desktop/electron/gateway-file-download.ts b/apps/desktop/electron/gateway-file-download.ts index 15d09964bf..ac1ea430cd 100644 --- a/apps/desktop/electron/gateway-file-download.ts +++ b/apps/desktop/electron/gateway-file-download.ts @@ -100,13 +100,15 @@ export interface GatewayFileRequestPaths { export function gatewayFileRequestPaths( filePath: string, - scopePath: (requestPath: string) => string + scopePath: (requestPath: string) => string, + sessionId?: string ): GatewayFileRequestPaths { const encodedPath = encodeURIComponent(filePath) + const session = sessionId === undefined ? '' : `&session_id=${encodeURIComponent(sessionId)}` return { - dataUrl: scopePath(`/api/fs/read-data-url?path=${encodedPath}`), - download: scopePath(`/api/fs/download?path=${encodedPath}`) + dataUrl: scopePath(`/api/fs/read-data-url?path=${encodedPath}${session}`), + download: scopePath(`/api/fs/download?path=${encodedPath}${session}`) } } @@ -329,23 +331,9 @@ export function filenameFromContentDisposition(value: unknown): string { } } -// Normalize a gateway file path that may arrive as a bare path or a file:// URL. +// Preserve file URIs: only the gateway knows its native drive/UNC semantics. export function gatewayFilePath(rawPath: unknown): string { - const value = String(rawPath || '').trim() - - if (!value) { - return '' - } - - if (!/^file:/i.test(value)) { - return value - } - - try { - return decodeURIComponent(new URL(value).pathname) - } catch { - return value.replace(/^file:\/\//i, '') - } + return String(rawPath || '').trim() } // True when an error thrown by a transport wrapper represents an HTTP 404, used diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index d13ea8b736..dc669cb15c 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -8158,6 +8158,7 @@ interface GatewayFileSaveContext { } interface GatewayFileSavePayload { + sessionId?: string connectionId?: unknown path?: unknown profile?: unknown @@ -8198,8 +8199,10 @@ async function saveGatewayFile(payload: GatewayFileSavePayload = {}) { const fallbackName = path.basename(filePath) || suggested || 'download' const ctx = { suggested, fallbackName } - const requestPaths = gatewayFileRequestPaths(filePath, requestPath => - gatewayFileRequestPath(connection, connectionId, profile, requestPath) + const requestPaths = gatewayFileRequestPaths( + filePath, + requestPath => gatewayFileRequestPath(connection, connectionId, profile, requestPath), + payload.sessionId ) const url = `${connection.baseUrl}${requestPaths.download}` diff --git a/apps/desktop/src/app/artifacts/artifact-utils.ts b/apps/desktop/src/app/artifacts/artifact-utils.ts index 0d679936ad..ca9c070d58 100644 --- a/apps/desktop/src/app/artifacts/artifact-utils.ts +++ b/apps/desktop/src/app/artifacts/artifact-utils.ts @@ -1,4 +1,4 @@ -import { mediaExternalUrl, resolveMediaDisplaySrc } from '@/lib/media' +import { isArtifactFilePath, mediaExternalUrl, resolveMediaDisplaySrc } from '@/lib/media' import type { SessionInfo, SessionMessage } from '@/types/hermes' export type ArtifactKind = 'image' | 'file' | 'link' @@ -12,6 +12,7 @@ export interface ArtifactRecord { href: string label: string sessionId: string + profile?: string sessionTitle: string timestamp: number } @@ -30,7 +31,7 @@ const MARKDOWN_IMAGE_RE = /!\[([^\]]*)\]\(([^)\s]+)\)/g const MARKDOWN_LINK_RE = /\[([^\]]+)\]\(([^)\s]+)\)/g const MEDIA_RE = /[`"']?MEDIA:\s*(`[^`\n]+`|"[^"\n]+"|'[^'\n]+'|\S+)[`"']?/g const URL_RE = /https?:\/\/[^\s<>"')]+/g -const PATH_RE = /(^|[\s("'`])((?:\/|~\/|\.\.?\/)[^\s"'`<>]+(?:\.[a-z0-9]{1,8})?)/gi +const PATH_RE = /(^|[\s("'`])((?:\/|~[\\/]|\.\.?[\\/]|\\\\)[^\s"'`<>]+(?:\.[a-z0-9]{1,8})?)/gi const WINDOWS_PATH_RE = /(^|[\s("'`])([A-Za-z]:[\\/][^\s"'`<>]+(?:\.[a-z0-9]{1,8})?)/gi const IMAGE_EXT_RE = /\.(?:png|jpe?g|gif|webp|svg|bmp)(?:\?.*)?$/i @@ -135,13 +136,8 @@ function looksLikePathOrUrl(value: string): boolean { return ( value.startsWith('http://') || value.startsWith('https://') || - value.startsWith('file://') || value.startsWith('data:image/') || - value.startsWith('/') || - value.startsWith('./') || - value.startsWith('../') || - value.startsWith('~/') || - isWindowsPath(value) + isArtifactFilePath(value) ) } @@ -158,14 +154,7 @@ function artifactKind(value: string): ArtifactKind { return 'image' } - if ( - value.startsWith('/') || - value.startsWith('./') || - value.startsWith('../') || - value.startsWith('~/') || - value.startsWith('file://') || - isWindowsPath(value) - ) { + if (isArtifactFilePath(value)) { return 'file' } @@ -420,6 +409,7 @@ export function collectArtifactsForSession(session: SessionInfo, messages: Sessi href: artifactHref(value), label: artifactLabel(value), sessionId: session.id, + profile: session.profile, sessionTitle: title, timestamp: artifactTimestamp(message, session) }) diff --git a/apps/desktop/src/app/artifacts/index.tsx b/apps/desktop/src/app/artifacts/index.tsx index 3501369d77..e7e4de02e4 100644 --- a/apps/desktop/src/app/artifacts/index.tsx +++ b/apps/desktop/src/app/artifacts/index.tsx @@ -30,7 +30,7 @@ import { useLinkTitle } from '@/lib/external-link' import { FileImage, FileText, FolderOpen, Link2 } from '@/lib/icons' -import { downloadGatewayMediaFile, isRemoteGateway } from '@/lib/media' +import { downloadGatewayMediaFile, isArtifactFilePath, isRemoteGateway } from '@/lib/media' import { normalize } from '@/lib/text' import { fmtDayTime } from '@/lib/time' import { cn } from '@/lib/utils' @@ -92,7 +92,7 @@ function paginationItems(page: number, pageCount: number): Array void | Promise + onOpen: (artifact: ArtifactRecord) => void | Promise onOpenChat: (sessionId: string) => void } @@ -270,7 +270,9 @@ export function ArtifactsView({ setStatusbarItemGroup: _setStatusbarItemGroup, . }, [artifacts]) const openArtifact = useCallback( - async (href: string) => { + async (artifact: ArtifactRecord) => { + const { href } = artifact + try { // A gateway-local file resolves to file:// in remote mode (the file // lives on the gateway, not this disk). Opening that locally fails — @@ -278,8 +280,8 @@ export function ArtifactsView({ setStatusbarItemGroup: _setStatusbarItemGroup, . // URL. Fetch the bytes over the authenticated fs bridge instead. // Tilde/relative hrefs have no file URL form. Keep them gateway-owned: // expanding them on the client would target the wrong home or cwd. - if (isRemoteGateway() && /^(?:file:|~\/|\.{1,2}\/)/i.test(href)) { - await downloadGatewayMediaFile(href) + if (isRemoteGateway() && isArtifactFilePath(artifact.value)) { + await downloadGatewayMediaFile(artifact.value, { sessionId: artifact.sessionId, profile: artifact.profile }) return } @@ -592,7 +594,7 @@ const PrimaryCell = memo(function PrimaryCell({ artifact, ctx }: { artifact: Art return ( void ctx.onOpen(artifact.href)} + onClick={isLink ? undefined : () => void ctx.onOpen(artifact)} title={label} > diff --git a/apps/desktop/src/app/artifacts/remote-open.test.tsx b/apps/desktop/src/app/artifacts/remote-open.test.tsx index eac02ed5de..5d1cd495b9 100644 --- a/apps/desktop/src/app/artifacts/remote-open.test.tsx +++ b/apps/desktop/src/app/artifacts/remote-open.test.tsx @@ -6,31 +6,85 @@ import { $connection } from '@/store/session' import { ArtifactsView } from './index' +const paths = vi.hoisted(() => [ + '~/.hermes/memories/USER.md', + './report.md', + '../parent.md', + String.raw`~\home.txt`, + String.raw`.\child.txt`, + String.raw`..\ancestor.txt`, + 'file:///C:/output/drive.txt', + 'file://server/share/unc.txt', + '/srv/absolute.txt' +]) + vi.mock('@/hermes', async () => ({ ...(await vi.importActual('@/hermes')), - listAllProfileSessions: async () => ({ sessions: [{ id: 'artifact-session', title: 'Fixture' }] }), - getAllSessionMessages: async () => ({ messages: [{ role: 'assistant', timestamp: 1000, - content: '~/.hermes/memories/USER.md ./report.md ../parent.md' }] }) + listAllProfileSessions: async () => ({ + sessions: [{ id: 'artifact-session', title: 'Fixture', profile: 'origin-profile' }] + }), + getAllSessionMessages: async () => ({ + messages: [ + { + role: 'assistant', + timestamp: 1000, + content: paths.map(path => `MEDIA:${path}`).join(' ') + ' https://example.com/report.txt' + } + ] + }) })) +afterEach(() => { + cleanup() + $connection.set(null) + vi.unstubAllGlobals() +}) -afterEach(() => { cleanup(); $connection.set(null); vi.unstubAllGlobals() }) - -it('opens remote tilde and relative artifacts through their gateway without client path expansion', async () => { +it('keeps discovered file paths and originating session scope intact through remote opening', async () => { const saveGatewayFile = vi.fn().mockResolvedValue({ saved: true }) - const openExternal = vi.fn().mockRejectedValue(new Error('Invalid external URL')) + const openExternal = vi.fn() vi.stubGlobal('hermesDesktop', { saveGatewayFile, openExternal }) - $connection.set({ isFullscreen: false, nativeOverlayWidth: 0, logs: [], windowButtonPosition: null, mode: 'remote', connectionId: 'remote-fixture', profile: 'writer', baseUrl: 'http://localhost', token: '', wsUrl: '' }) - render() + $connection.set({ + isFullscreen: false, + nativeOverlayWidth: 0, + logs: [], + windowButtonPosition: null, + mode: 'remote', + connectionId: 'remote-fixture', + profile: 'writer', + baseUrl: 'http://localhost', + token: '', + wsUrl: '' + }) + render( + + + + ) - for (const name of ['USER.md', 'report.md', 'parent.md']) { + for (const name of [ + 'USER.md', + 'report.md', + 'parent.md', + 'home.txt', + 'child.txt', + 'ancestor.txt', + 'drive.txt', + 'unc.txt', + 'absolute.txt' + ]) { fireEvent.click(await screen.findByRole('button', { name })) } - await waitFor(() => expect(saveGatewayFile).toHaveBeenCalledTimes(3)) - expect(saveGatewayFile.mock.calls.map(([request]) => request)).toEqual([ - { connectionId: 'remote-fixture', profile: 'writer', path: '~/.hermes/memories/USER.md', suggestedName: 'USER.md' }, - { connectionId: 'remote-fixture', profile: 'writer', path: './report.md', suggestedName: 'report.md' }, - { connectionId: 'remote-fixture', profile: 'writer', path: '../parent.md', suggestedName: 'parent.md' } - ]) + await waitFor(() => expect(saveGatewayFile).toHaveBeenCalledTimes(paths.length)) + expect(saveGatewayFile.mock.calls.map(([request]) => request)).toEqual( + paths.map(path => ({ + connectionId: 'remote-fixture', + profile: 'origin-profile', + sessionId: 'artifact-session', + path, + suggestedName: path.split(/[\\/]/).pop() + })) + ) + expect(screen.getByRole('link').getAttribute('href')).toBe('https://example.com/report.txt') expect(openExternal).not.toHaveBeenCalled() }) diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index 43ff09810e..bd02af438c 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -272,6 +272,7 @@ declare global { connectionId?: null | string path: string profile?: null | string + sessionId?: string suggestedName?: string }) => Promise<{ canceled?: boolean diff --git a/apps/desktop/src/lib/media.remote.test.ts b/apps/desktop/src/lib/media.remote.test.ts index fbfb5b1353..6092db90f4 100644 --- a/apps/desktop/src/lib/media.remote.test.ts +++ b/apps/desktop/src/lib/media.remote.test.ts @@ -254,7 +254,7 @@ describe('downloadGatewayMediaFile', () => { expect(saveGatewayFile).toHaveBeenCalledWith({ connectionId: 'work-ssh', - path: '/Users/me/project/a b.md', + path: 'file:///Users/me/project/a%20b.md', profile: 'docker-gw', suggestedName: 'a b.md' }) diff --git a/apps/desktop/src/lib/media.ts b/apps/desktop/src/lib/media.ts index c5d7824262..cd5d6b3c46 100644 --- a/apps/desktop/src/lib/media.ts +++ b/apps/desktop/src/lib/media.ts @@ -73,6 +73,10 @@ export function isInlineMediaSrc(path: string): boolean { return /^(?:https?|data):/i.test(path) } +export function isArtifactFilePath(path: string): boolean { + return /^(?:file:|\/|[~.][\\/]|\.\.[\\/]|[a-z]:[\\/]|\\\\)/i.test(path) +} + export function isFileMediaPath(path: string): boolean { return /^(?:file:|\/|~\/|[a-z]:[\\/]|\\\\)/i.test(path) } @@ -204,9 +208,11 @@ export async function gatewayMediaDataUrl(path: string): Promise { // avoids browser/OS downloads losing OAuth cookies and avoids the data-URL cap // used by preview endpoints. export async function downloadGatewayMediaFile( - path: string + path: string, + origin?: { sessionId: string; profile?: string } ): Promise<{ canceled?: boolean; path?: string; saved: boolean }> { - const file = filePathFromMediaPath(path) + // URI conversion belongs to the gateway OS, not the renderer's URL parser. + const file = path const conn = $connection.get() if (!window.hermesDesktop?.saveGatewayFile) { @@ -216,8 +222,15 @@ export async function downloadGatewayMediaFile( return window.hermesDesktop.saveGatewayFile({ connectionId: conn?.connectionId, path: file, - profile: conn?.profile, - suggestedName: mediaName(file) + profile: origin?.profile ?? conn?.profile, + ...(origin ? { sessionId: origin.sessionId } : {}), + suggestedName: mediaName(file).replace(/(?:%[0-9a-f]{2})+/gi, encoded => { + try { + return decodeURIComponent(encoded) + } catch { + return encoded + } + }) }) } diff --git a/hermes_cli/web_routers/files.py b/hermes_cli/web_routers/files.py index 5b5de6adb6..68c7e1ed38 100644 --- a/hermes_cli/web_routers/files.py +++ b/hermes_cli/web_routers/files.py @@ -688,10 +688,30 @@ async def fs_write_text(payload: FsWriteText): return {"ok": True, "path": str(target), "byteSize": len(text.encode("utf-8"))} +async def _fs_download_path(path: str, profile: Optional[str], session_id: Optional[str]) -> Path: + if session_id is not None: + from hermes_cli.web_routers.sessions import get_session_detail + + if not session_id.strip(): + raise HTTPException(status_code=404, detail="Session not found") + session = await get_session_detail(session_id, profile) + # Validate ownership even for absolute paths; never trust a client cwd. + return _fs_path(path, cwd=session.get("cwd") or "") + if profile is not None: + from hermes_cli.web_server_cron import _cron_profile_home + + _cron_profile_home(profile) + return _fs_path(path) + + @router.get("/api/fs/read-data-url") -async def fs_read_data_url(path: str): +async def fs_read_data_url( + path: str, profile: Optional[str] = None, session_id: Optional[str] = None, +): from hermes_cli.web_server import _FS_DATA_URL_MAX_BYTES - target, st = _fs_regular_file(_fs_path(path)) + target, st = _fs_regular_file(await _fs_download_path(path, profile, session_id)) + if _is_sensitive_path(target): + raise HTTPException(status_code=403, detail="Access to sensitive files is not allowed") if st.st_size > _FS_DATA_URL_MAX_BYTES: raise HTTPException(status_code=413, detail="File too large") encoded = base64.b64encode(_fs_read_bytes(target)).decode("ascii") @@ -699,8 +719,10 @@ async def fs_read_data_url(path: str): @router.get("/api/fs/download") -async def fs_download(path: str): - target, _st = _fs_regular_file(_fs_path(path)) +async def fs_download( + path: str, profile: Optional[str] = None, session_id: Optional[str] = None, +): + target, _st = _fs_regular_file(await _fs_download_path(path, profile, session_id)) if _is_sensitive_path(target): raise HTTPException(status_code=403, detail="Access to sensitive files is not allowed") return FileResponse( diff --git a/hermes_cli/web_server_files.py b/hermes_cli/web_server_files.py index 4efc47144f..dce34644b6 100644 --- a/hermes_cli/web_server_files.py +++ b/hermes_cli/web_server_files.py @@ -21,7 +21,7 @@ class ManagedFilesPolicy: can_change_path: bool -def _fs_path(raw_path: str) -> Path: +def _fs_path(raw_path: str, *, cwd: str | None = None) -> Path: raw = str(raw_path or "").strip() if not raw: raise HTTPException(status_code=400, detail="Path is required") @@ -30,12 +30,18 @@ def _fs_path(raw_path: str) -> Path: try: if raw.lower().startswith("file:"): parsed = urllib.parse.urlparse(raw) - if parsed.netloc and parsed.netloc not in {"", "localhost"}: - raise ValueError - raw = urllib.request.url2pathname(parsed.path) + uri_path = parsed.path + if parsed.netloc and parsed.netloc.lower() != "localhost": + if os.name != "nt": + raise ValueError + uri_path = f"//{parsed.netloc}{uri_path}" + raw = urllib.request.url2pathname(uri_path) candidate = Path(raw).expanduser() if not candidate.is_absolute(): - candidate = Path.cwd() / candidate + base = Path(cwd).expanduser() if cwd is not None else Path.cwd() + if not base.is_absolute(): + raise HTTPException(status_code=400, detail="Session working directory is unavailable") + candidate = base / candidate return candidate.resolve(strict=False) except (OSError, RuntimeError, ValueError): raise HTTPException(status_code=400, detail="Invalid path") diff --git a/tests/hermes_cli/test_web_server_files.py b/tests/hermes_cli/test_web_server_files.py index 5157da814d..efa7fb3015 100644 --- a/tests/hermes_cli/test_web_server_files.py +++ b/tests/hermes_cli/test_web_server_files.py @@ -1,5 +1,6 @@ """Tests for the dashboard-managed file browser API.""" +import base64 from types import SimpleNamespace import pytest @@ -133,6 +134,49 @@ def test_download_authenticates_via_query_token(forced_files_client): ).status_code == 401 +def test_download_resolves_paths_in_the_originating_profile_session(local_files_client, monkeypatch): + from pathlib import Path + from hermes_state import SessionDB + + client, home = local_files_client + monkeypatch.setattr(Path, "home", lambda: home) + hermes_home = home / "isolated-hermes" + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + session_cwd = home / "project" + session_cwd.mkdir() + gateway_cwd = home / "gateway" + gateway_cwd.mkdir() + monkeypatch.chdir(gateway_cwd) + artifact = session_cwd / "report.txt" + artifact.write_bytes(b"session artifact") + (gateway_cwd / artifact.name).write_bytes(b"wrong gateway artifact") + for profile, sid, cwd in [("default", "origin-session", str(session_cwd)), + ("other", "other-session", str(gateway_cwd))]: + db_home = hermes_home if profile == "default" else hermes_home / "profiles" / profile + db_home.mkdir(parents=True, exist_ok=True) + (db_home / "config.yaml").write_text("{}", encoding="utf-8") + db = SessionDB(db_path=db_home / "state.db") + try: + db.create_session(sid, source="gui", cwd=cwd) + finally: + db.close() + for route in ("/api/fs/download", "/api/fs/read-data-url"): + for path in ("./report.txt", "../project/report.txt", str(artifact), artifact.as_uri()): + response = client.get(route, params={ + "path": path, "profile": "default", "session_id": "origin-session", + }) + assert response.status_code == 200, response.text + data = (base64.b64decode(response.json()["dataUrl"].split(",", 1)[1]) + if route.endswith("read-data-url") else response.content) + assert data == artifact.read_bytes() + for profile, session_id in (("other", "origin-session"), ("missing", "origin-session"), + ("default", "missing-session"), ("default", "")): + response = client.get(route, params={ + "path": str(artifact), "profile": profile, "session_id": session_id, + }) + assert response.status_code == 404, response.text + + def test_stream_requires_header_auth_and_supports_ranges(forced_files_client): client, root = forced_files_client file_path = _seed_file(client, root, name="out/demo.mp4") diff --git a/website/docs/user-guide/desktop.md b/website/docs/user-guide/desktop.md index a7ce3af333..6200403c43 100644 --- a/website/docs/user-guide/desktop.md +++ b/website/docs/user-guide/desktop.md @@ -101,6 +101,8 @@ Explore and preview the working directory without leaving the app — useful for ### Artifacts +When connected to a remote gateway, opening a file artifact downloads it through that gateway, using the artifact’s originating profile and session. Relative paths resolve against the session’s saved working directory; home-relative paths use the gateway’s home, never the Desktop machine’s home. Windows-style relative paths are recognized alongside forward-slash paths, and file URIs retain drive and network-share information for the gateway to interpret. Missing sessions or working directories produce an error rather than selecting a different local file. + The **Artifacts** view collects what your sessions generate — **images, files, and links** — into one searchable, browsable gallery. Open it from the sidebar, the command palette (**Artifacts — Browse generated outputs**), or a `nav.artifacts` shortcut you bind yourself. It indexes recent session outputs automatically; every artifact shows which session produced it with a jump back to that chat, and images and files open in a preview with download / open-in-browser / copy actions. ### Windows, tabs & panes