diff --git a/apps/desktop/src/app/artifacts/index.tsx b/apps/desktop/src/app/artifacts/index.tsx index 8e98dd9d40d..b4dfd994e9f 100644 --- a/apps/desktop/src/app/artifacts/index.tsx +++ b/apps/desktop/src/app/artifacts/index.tsx @@ -23,6 +23,7 @@ import { type Translations, useI18n } from '@/i18n' import { sessionTitle } from '@/lib/chat-runtime' import { ExternalLink, ExternalLinkIcon, hostPathLabel, urlSlugTitleLabel, useLinkTitle } from '@/lib/external-link' import { FileImage, FileText, FolderOpen, Link2 } from '@/lib/icons' +import { mediaExternalUrl } from '@/lib/media' import { cn } from '@/lib/utils' import { notifyError } from '@/store/notifications' import type { SessionInfo, SessionMessage } from '@/types/hermes' @@ -124,17 +125,12 @@ function artifactKind(value: string): ArtifactKind { } function artifactHref(value: string): string { - if ( - value.startsWith('http://') || - value.startsWith('https://') || - value.startsWith('file://') || - value.startsWith('data:') - ) { + if (value.startsWith('http://') || value.startsWith('https://') || value.startsWith('data:')) { return value } - if (value.startsWith('/')) { - return `file://${encodeURI(value)}` + if (value.startsWith('file://') || value.startsWith('/')) { + return mediaExternalUrl(value) } return value diff --git a/apps/desktop/src/lib/media.remote.test.ts b/apps/desktop/src/lib/media.remote.test.ts index 9de4885a517..53e5c2212c3 100644 --- a/apps/desktop/src/lib/media.remote.test.ts +++ b/apps/desktop/src/lib/media.remote.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { $connection } from '@/store/session' -import { filePathFromMediaPath, gatewayMediaDataUrl, isRemoteGateway } from './media' +import { filePathFromMediaPath, gatewayMediaDataUrl, isRemoteGateway, mediaExternalUrl } from './media' describe('isRemoteGateway', () => { afterEach(() => { @@ -35,6 +35,38 @@ describe('filePathFromMediaPath', () => { }) }) +describe('mediaExternalUrl', () => { + afterEach(() => { + $connection.set(null) + }) + + it('passes through http(s) URLs untouched', () => { + $connection.set({ mode: 'remote', baseUrl: 'https://gw', token: 't' } as never) + expect(mediaExternalUrl('https://example.com/a.png')).toBe('https://example.com/a.png') + }) + + it('keeps file:// form in local mode', () => { + $connection.set({ mode: 'local' } as never) + expect(mediaExternalUrl('/tmp/a.png')).toBe('file:///tmp/a.png') + expect(mediaExternalUrl('file:///tmp/a.png')).toBe('file:///tmp/a.png') + }) + + it('rewrites gateway-local paths to an authenticated download URL', () => { + $connection.set({ mode: 'remote', baseUrl: 'https://gw', token: 's e/cret' } as never) + expect(mediaExternalUrl('file:///tmp/a b.png')).toBe( + 'https://gw/api/files/download?path=%2Ftmp%2Fa%20b.png&token=s%20e%2Fcret' + ) + expect(mediaExternalUrl('/tmp/a b.png')).toBe( + 'https://gw/api/files/download?path=%2Ftmp%2Fa%20b.png&token=s%20e%2Fcret' + ) + }) + + it('falls back to file:// when remote connection lacks a token', () => { + $connection.set({ mode: 'remote', baseUrl: 'https://gw' } as never) + expect(mediaExternalUrl('/tmp/a.png')).toBe('file:///tmp/a.png') + }) +}) + describe('gatewayMediaDataUrl', () => { const api = vi.fn(async () => ({ data_url: 'data:image/png;base64,ZHVtbXk=' })) diff --git a/apps/desktop/src/lib/media.ts b/apps/desktop/src/lib/media.ts index 145558b42aa..9c50ce6c757 100644 --- a/apps/desktop/src/lib/media.ts +++ b/apps/desktop/src/lib/media.ts @@ -56,8 +56,25 @@ export function mediaMarkdownHref(path: string): string { return `#media:${encodeURIComponent(path)}` } +// Resolve a media path to a URL the shell can open. Remote mode rewrites +// gateway-local paths to an authenticated /api/files/download URL (the file +// lives on the gateway, not this disk); local mode keeps the file:// form. export function mediaExternalUrl(path: string): string { - return /^(?:https?|file):/i.test(path) ? path : `file://${path}` + if (/^https?:/i.test(path)) { + return path + } + + if (isRemoteGateway()) { + const conn = $connection.get() + + if (conn?.baseUrl && conn.token) { + const file = encodeURIComponent(filePathFromMediaPath(path)) + + return `${conn.baseUrl}/api/files/download?path=${file}&token=${encodeURIComponent(conn.token)}` + } + } + + return /^file:/i.test(path) ? path : `file://${path}` } // Custom Electron scheme (registered in electron/main.cjs) that streams a local diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index c434fb6751d..a75a6468352 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -247,6 +247,19 @@ def _has_valid_session_token(request: Request) -> bool: return hmac.compare_digest(auth.encode(), expected.encode()) +# Routes that may also authenticate via a ``?token=`` query param, for download +# links opened by the OS shell or a new browser tab where the session header +# can't be set. Kept narrow — same query-token tradeoff as the /api/pty WS. +_QUERY_TOKEN_API_PATHS: frozenset[str] = frozenset({"/api/files/download"}) + + +def _has_valid_query_token(request: Request, path: str) -> bool: + if path not in _QUERY_TOKEN_API_PATHS: + return False + token = request.query_params.get("token", "") + return bool(token) and hmac.compare_digest(token.encode(), _SESSION_TOKEN.encode()) + + def _require_token(request: Request) -> None: """Authorize a sensitive endpoint, raising 401 if the caller isn't allowed. @@ -403,7 +416,7 @@ async def auth_middleware(request: Request, call_next): return await call_next(request) path = request.url.path if path.startswith("/api/") and path not in _PUBLIC_API_PATHS: - if not _has_valid_session_token(request): + if not _has_valid_session_token(request) and not _has_valid_query_token(request, path): return JSONResponse( status_code=401, content={"detail": "Unauthorized"}, @@ -1409,6 +1422,40 @@ async def read_managed_file(request: Request, path: str): } +@app.get("/api/files/download") +async def download_managed_file(request: Request, path: str): + """Stream a managed file as an attachment download. + + Remote clients (desktop app, browser dashboard) open agent-written files + that live on *this* gateway's disk, not theirs. Auth-gated like every other + managed-files route — ``auth_middleware`` additionally accepts the session + token as a ``?token=`` query param here so a shell/browser-opened download + (which can't set the session header) still authenticates. See ``/api/pty`` + for the same query-token precedent. + """ + policy, target, _display_path = _resolve_managed_path(path, request) + if not target.exists(): + raise HTTPException(status_code=404, detail="File not found") + if not target.is_file(): + raise HTTPException(status_code=400, detail="Path is not a file") + + try: + size = target.stat().st_size + except OSError as exc: + raise HTTPException(status_code=500, detail=f"Could not stat file: {exc}") + if size > _MANAGED_FILE_MAX_BYTES: + raise HTTPException(status_code=413, detail="File is too large") + + mime_type = mimetypes.guess_type(target.name)[0] or "application/octet-stream" + + return FileResponse( + path=str(target), + media_type=mime_type, + filename=target.name, + content_disposition_type="attachment", + ) + + @app.post("/api/files/upload") async def upload_managed_file(payload: ManagedFileUpload, request: Request): policy, target, display_path = _resolve_managed_path(payload.path, request, for_write=True) diff --git a/tests/hermes_cli/test_web_server_files.py b/tests/hermes_cli/test_web_server_files.py index 6f4b8633174..02096a616ae 100644 --- a/tests/hermes_cli/test_web_server_files.py +++ b/tests/hermes_cli/test_web_server_files.py @@ -254,6 +254,66 @@ def test_local_mode_upload_read_mkdir_delete_roundtrip(local_files_client): assert not folder.exists() +def _seed_file(client, root, name="out/hello.txt"): + file_path = root / name + created = client.post( + "/api/files/upload", + json={"path": str(file_path), "data_url": "data:text/plain;base64,aGVsbG8="}, + ) + assert created.status_code == 200 + return file_path + + +def test_download_returns_file_as_attachment(forced_files_client): + client, root = forced_files_client + file_path = _seed_file(client, root) + + resp = client.get("/api/files/download", params={"path": str(file_path)}) + assert resp.status_code == 200 + assert resp.content == b"hello" + disposition = resp.headers["content-disposition"] + assert "attachment" in disposition + assert "hello.txt" in disposition + + +def test_download_authenticates_via_query_token(forced_files_client): + client, root = forced_files_client + file_path = _seed_file(client, root) + + # Drop the session header so only the ?token= query param authenticates — + # mirrors a browser/shell-opened download that can't set the session header. + del client.headers[web_server._SESSION_HEADER_NAME] + + ok = client.get( + "/api/files/download", + params={"path": str(file_path), "token": web_server._SESSION_TOKEN}, + ) + assert ok.status_code == 200 + assert ok.content == b"hello" + + assert client.get( + "/api/files/download", params={"path": str(file_path), "token": "nope"} + ).status_code == 401 + assert client.get( + "/api/files/download", params={"path": str(file_path)} + ).status_code == 401 + + +def test_query_token_does_not_authenticate_other_endpoints(forced_files_client): + client, root = forced_files_client + file_path = _seed_file(client, root) + + del client.headers[web_server._SESSION_HEADER_NAME] + + # The query-token escape hatch is scoped to /api/files/download only; it must + # not unlock the rest of the API surface. + leaked = client.get( + "/api/files/read", + params={"path": str(file_path), "token": web_server._SESSION_TOKEN}, + ) + assert leaked.status_code == 401 + + def test_hosted_policy_locks_to_opt_data(monkeypatch): monkeypatch.delenv("HERMES_DASHBOARD_FILES_ROOT", raising=False) monkeypatch.setenv("HERMES_HOME", "/opt/data")