fix(desktop): resolve artifact downloads in their originating session

This commit is contained in:
Teknium
2026-09-07 03:44:49 -07:00
parent f9d05081d8
commit 478d772f2c
13 changed files with 200 additions and 75 deletions

View File

@@ -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), '')
})

View File

@@ -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

View File

@@ -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}`

View File

@@ -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)
})

View File

@@ -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<number | 'ellip
}
type CellCtx = {
onOpen: (href: string) => void | Promise<void>
onOpen: (artifact: ArtifactRecord) => void | Promise<void>
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 (
<ArtifactCellAction
href={isLink ? artifact.href : undefined}
onClick={isLink ? undefined : () => void ctx.onOpen(artifact.href)}
onClick={isLink ? undefined : () => void ctx.onOpen(artifact)}
title={label}
>
<span className="mt-0.5 grid size-6 shrink-0 place-items-center self-start rounded-md bg-(--ui-bg-tertiary) text-(--ui-text-tertiary)">

View File

@@ -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(<MemoryRouter><ArtifactsView /></MemoryRouter>)
$connection.set({
isFullscreen: false,
nativeOverlayWidth: 0,
logs: [],
windowButtonPosition: null,
mode: 'remote',
connectionId: 'remote-fixture',
profile: 'writer',
baseUrl: 'http://localhost',
token: '',
wsUrl: ''
})
render(
<MemoryRouter>
<ArtifactsView />
</MemoryRouter>
)
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()
})

View File

@@ -272,6 +272,7 @@ declare global {
connectionId?: null | string
path: string
profile?: null | string
sessionId?: string
suggestedName?: string
}) => Promise<{
canceled?: boolean

View File

@@ -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'
})

View File

@@ -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<string> {
// 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
}
})
})
}

View File

@@ -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(

View File

@@ -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")

View File

@@ -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")

View File

@@ -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