fix(desktop): open remote-gateway artifacts via authenticated download (#46895)
Some checks failed
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Docker Build and Publish / build-amd64 (push) Waiting to run
Docker Build and Publish / build-arm64 (push) Waiting to run
Docker Build and Publish / merge (push) Blocked by required conditions
Lint (ruff + ty) / ruff + ty diff (push) Waiting to run
Lint (ruff + ty) / ruff enforcement (blocking) (push) Waiting to run
Lint (ruff + ty) / Windows footguns (blocking) (push) Waiting to run
Tests / test (1) (push) Waiting to run
Tests / test (2) (push) Waiting to run
Tests / test (3) (push) Waiting to run
Tests / test (4) (push) Waiting to run
Tests / test (5) (push) Waiting to run
Tests / test (6) (push) Waiting to run
Tests / save-durations (push) Blocked by required conditions
Tests / e2e (push) Waiting to run
Typecheck / typecheck (apps/bootstrap-installer) (push) Waiting to run
Typecheck / typecheck (apps/desktop) (push) Waiting to run
Typecheck / typecheck (apps/shared) (push) Waiting to run
Typecheck / typecheck (ui-tui) (push) Waiting to run
Typecheck / typecheck (web) (push) Waiting to run
Typecheck / desktop-build (push) Waiting to run
Docker / shell lint / Lint Dockerfile (hadolint) (push) Has been cancelled
Docker / shell lint / Lint docker/ shell scripts (shellcheck) (push) Has been cancelled
OSV-Scanner / Scan lockfiles (push) Has been cancelled
uv.lock check / uv lock --check (push) Has been cancelled

On a remote gateway connection, agent-written files live on the gateway
host, not the desktop's disk, so the Artifacts view's file:// hrefs failed
("Invalid external URL") and image thumbnails broke.

Make mediaExternalUrl() remote-aware in one place: in remote mode it
rewrites gateway-local paths to GET /api/files/download (a new endpoint
that streams the file as a Content-Disposition: attachment). The artifacts
view now resolves through it, and so do the existing chat-media and
generated-image callers, for free.

The download endpoint stays auth-gated; auth_middleware additionally
accepts the session token as a ?token= query param for this one path so a
shell/browser-opened download (which can't set the session header) still
authenticates — the same query-token tradeoff as the /api/pty WebSocket.
It is NOT added to PUBLIC_API_PATHS.

Salvages #46663 (which carried ~19k lines of CRLF noise and made the
endpoint public). Reimplemented on a clean LF base with the security hole
closed and tests added.

Co-authored-by: qingshan89 <qs2816661685@gmail.com>
This commit is contained in:
brooklyn! 2026-06-15 23:50:19 -05:00 committed by GitHub
parent 0441b7f19f
commit c6b0eb4de0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 163 additions and 11 deletions

View file

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

View file

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

View file

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

View file

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

View file

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