From e2cdc0c6e725365873f165292f2fcae9da16c70f Mon Sep 17 00:00:00 2001 From: emozilla Date: Fri, 23 Oct 2026 13:04:42 -0400 Subject: [PATCH] fix(files): preview and send a picked image on a profile whose workspace is an SSH machine On a profile whose terminal backend is SSH, an image chosen with the Webapp's file picker showed no preview and could not be sent. The picker stages the file on the gateway host, in the profile's uploads folder, and the Webapp reads it back through /api/fs/read-data-url to preview and send it. That route sent every path except the host image cache to the SSH machine, which has no such file and answered 404. Saving the file through /api/fs/download failed the same way. A file the upload route staged for the profile is now read on the gateway host, like a pasted image. Only a regular file directly in that profile's uploads folder with the staged name prefix counts, after resolving symlinks and .., so the exemption reads no other host file for an SSH profile: a staged file of another profile, another file in the folder, a file below it, a path that leaves it through .. and a link to a file elsewhere all still go to the SSH machine. A relative path never counts as a host file. --- hermes_cli/staged_uploads.py | 13 +++- hermes_cli/web_routers/files.py | 31 +++++---- tests/hermes_cli/test_web_server_fs.py | 90 ++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 12 deletions(-) diff --git a/hermes_cli/staged_uploads.py b/hermes_cli/staged_uploads.py index dd3e3f7ad9..b07781edd0 100644 --- a/hermes_cli/staged_uploads.py +++ b/hermes_cli/staged_uploads.py @@ -3,7 +3,8 @@ ``POST /api/chat/file-upload`` publishes a picked file at ``/uploads/web--`` and returns a ``staged_upload`` descriptor: that path, the profile home and this installation's id. The gateway's ``file.attach`` trusts the descriptor only after resolving it back to exactly that -layout, so the writer and the reader of the layout live here together. +layout, so the writer and the reader of the layout live here together. The file routes read a staged +file back from this host by the same layout when the profile's workspace is an SSH machine. A named profile that is missing or deleted is refused on both sides by the rule every named-home write follows (``hermes_constants.assert_named_profile_home_live`` / ``mkdir_under_hermes_home``): @@ -212,3 +213,13 @@ def resolve_staged_upload(home: Path, raw_path: str) -> Path: if source.stat().st_size > WEBAPP_ATTACHMENT_MAX_BYTES: raise ValueError("staged upload exceeds the browser attachment size limit") return source + + +def is_staged_upload(home: Path, target: Path) -> bool: + """Whether the resolved *target* is a file the upload route staged for *home*: a regular file + directly in its uploads folder, named with the staged prefix.""" + try: + uploads = (home / _UPLOADS_DIR).resolve(strict=True) + except (OSError, RuntimeError): + return False + return target.parent == uploads and target.name.startswith(_STAGED_PREFIX) and target.is_file() diff --git a/hermes_cli/web_routers/files.py b/hermes_cli/web_routers/files.py index 06a82274d8..da9274e218 100644 --- a/hermes_cli/web_routers/files.py +++ b/hermes_cli/web_routers/files.py @@ -28,7 +28,9 @@ from fastapi.responses import FileResponse, JSONResponse, Response, StreamingRes from hermes_cli import web_server_file_tickets as file_tickets from hermes_cli._subprocess_compat import windows_hide_flags from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access -from hermes_cli.staged_uploads import ensure_profile_subdir, removed_on_failure, resolve_upload_home +from hermes_cli.staged_uploads import ( + ensure_profile_subdir, is_staged_upload, removed_on_failure, resolve_upload_home, +) from hermes_cli.web_deps import late from hermes_cli.web_upload_limit import CHAT_IMAGE_MAX_BYTES from hermes_cli.web_server_files import ( @@ -317,28 +319,35 @@ def _chat_image_dir(home: Path) -> Path: return get_hermes_dir("cache/images", "image_cache", home=home) -def _is_host_image(path: str, profile: Optional[str]) -> bool: - """Whether *path* is in the profile's image cache, which lives on this host even when the - workspace is an SSH machine: a browser paste is saved there, then read back to be sent.""" +def _is_host_file(path: str, profile: Optional[str]) -> bool: + """Whether *path* is a file the profile keeps on this host even when its workspace is an SSH + machine, read back to be previewed and sent: a browser paste in its image cache, or a file the + browser's picker staged in its uploads folder. Only a file directly in one of those two folders + counts, after resolving symlinks and ``..``, so an SSH profile reads no other host file here.""" + if not Path(path).is_absolute(): + return False try: target = Path(path).resolve(strict=True) except (OSError, RuntimeError): return False # not on this host: the workspace's to answer with _profile_scope(profile): - cache = _chat_image_dir(get_hermes_home()) + home = get_hermes_home() + cache = _chat_image_dir(home) try: - return target.parent == cache.resolve(strict=True) + if target.parent == cache.resolve(strict=True): + return True except (OSError, RuntimeError): - return False + pass + return is_staged_upload(home, target) def _remote_fs_backend(path: str, profile: Optional[str]): - """The SSH workspace reader for *path*, or None when this host owns it. The host image cache is - checked before the SSH machine is dialed, so a paste reads back while that machine is down; other - paths answer 503 until it is back.""" + """The SSH workspace reader for *path*, or None when this host owns it. The host's pastes and + staged uploads are checked before the SSH machine is dialed, so they read back while that + machine is down; other paths answer 503 until it is back.""" from hermes_cli.ssh_workspace_fs import SshWorkspaceFsError - if _is_host_image(path, profile): + if _is_host_file(path, profile): return None try: return _fs_backend(profile) diff --git a/tests/hermes_cli/test_web_server_fs.py b/tests/hermes_cli/test_web_server_fs.py index 6da379c6fb..53202645bb 100644 --- a/tests/hermes_cli/test_web_server_fs.py +++ b/tests/hermes_cli/test_web_server_fs.py @@ -1,4 +1,5 @@ import base64 +import os import urllib.parse import json from contextlib import nullcontext @@ -665,6 +666,95 @@ def test_a_browser_paste_reads_back_while_the_ssh_machine_is_unreachable(client, assert (response.status_code, response.json()) == (503, {"detail": "SSH workspace is unavailable"}) +class _NotOnTheSshMachine: + """An SSH workspace that has none of the files a test asks it for; it records each ask.""" + + def __init__(self): + self.asked = [] + + def read_bytes(self, path, *, max_bytes, read_limit=None): + self.asked.append(path) + raise ssh_workspace_fs.SshWorkspaceFsError("ENOENT", f"{path} is not on the SSH machine") + + def inspect_file(self, path): + self.asked.append(path) + raise ssh_workspace_fs.SshWorkspaceFsError("ENOENT", f"{path} is not on the SSH machine") + + +def _ssh_profiles(tmp_path, monkeypatch): + """A default home and a named profile ``remote-dev``, both with an SSH machine as workspace.""" + home = tmp_path / ".hermes" + profile = home / "profiles" / "remote-dev" + profile.mkdir(parents=True) + (profile / "config.yaml").write_text("{}", encoding="utf-8") + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.delenv("HERMES_DASHBOARD_FILES_ROOT", raising=False) + workspace = _NotOnTheSshMachine() + monkeypatch.setattr(file_routes, "_fs_backend", lambda profile=None: workspace) + return profile, workspace + + +def test_a_picked_image_reads_back_from_the_host_for_an_ssh_profile(client, monkeypatch, tmp_path): + """The browser's file picker stages a file in the profile's uploads folder on this host, even + when its workspace is an SSH machine. Previewing and sending a picked image read it back through + /api/fs/read-data-url (saving it, through /api/fs/download), so a file the upload route staged + for that profile is read on this host. Every other path still goes to the SSH machine: a staged + file of another profile, a file in the uploads folder the upload route did not name, one in a + folder below it, and a path that leaves the folder through ``..``.""" + profile, workspace = _ssh_profiles(tmp_path, monkeypatch) + image = base64.b64decode("iVBORw0KGgo=") + + def pick(query): + staged = client.post("/api/chat/file-upload", params=query, + files={"file": ("shot.png", image, "image/png")}) + assert staged.status_code == 200, staged.text + return Path(staged.json()["path"]) + + remote_dev = {"profile": "remote-dev"} + staged, default_staged = pick(remote_dev), pick({}) + unnamed = staged.parent / "shot.png" + nested = staged.parent / "below" / "web-shot.png" + nested.parent.mkdir() + (profile / "web-shot.png").write_bytes(image) + left_through_dots = f"{staged.parent}{os.sep}..{os.sep}web-shot.png" + for path in (unnamed, nested): + path.write_bytes(image) + + for query, path in ((remote_dev, staged), ({}, default_staged)): + read_back = client.get("/api/fs/read-data-url", params={"path": str(path), **query}) + saved = client.get("/api/fs/download", params={"path": str(path), **query}) + assert read_back.status_code == 200, read_back.text + assert read_back.json()["dataUrl"] == "data:image/png;base64,iVBORw0KGgo=" + assert (saved.status_code, saved.content) == (200, image) + assert workspace.asked == [] + + not_host_owned = [str(default_staged), str(unnamed), str(nested), left_through_dots] + for path in not_host_owned: + for route in ("read-data-url", "download"): + response = client.get(f"/api/fs/{route}", params={"path": path, **remote_dev}) + assert response.status_code == 404, (route, path, response.text) + assert workspace.asked == [path for path in not_host_owned for _route in range(2)] + + +@pytest.mark.platforms("posix") +def test_a_link_staged_in_the_uploads_folder_is_not_read_from_the_host(client, monkeypatch, tmp_path): + """A staged name that links to another host file resolves outside the uploads folder, so an + SSH profile's read of it goes to the SSH machine and never returns the host file.""" + profile, workspace = _ssh_profiles(tmp_path, monkeypatch) + host_file = tmp_path / "host-only.png" + host_file.write_bytes(b"host only") + uploads = profile / "uploads" + uploads.mkdir(mode=0o700) + link = uploads / "web-0123456789abcdef-shot.png" + link.symlink_to(host_file) + + response = client.get("/api/fs/read-data-url", params={"path": str(link), "profile": "remote-dev"}) + + assert response.status_code == 404, response.text + assert workspace.asked == [str(link)] + + @pytest.mark.parametrize("endpoint", ["read-text", "read-data-url", "download"]) def test_fs_reads_translate_docker_workspace_path_to_host( client, docker_volumes, monkeypatch, endpoint,