fix(tools): resolve ssh paths against the remote home, keep guards intact
Follow-up to the ssh path fix salvaged from #121686. - Read the ssh anchor raw (session record, override, TERMINAL_CWD): the shared workspace-root helper expands ~ on the Hermes host, so TERMINAL_CWD='~/proj' still resolved into the container home. - Resolve ~ to the remote home the SSH environment detects at connect, bringing the environment up through the file tools' own creator (_get_file_ops, same cwd and cache) when none is live. A failed bring-up is remembered per container for 30s so one call's several resolutions don't each retry; a live environment is always used first. SSHEnvironment now records whether the home was detected, and a guessed /home/<user> (echo $HOME failed) is not used. Results are absolute and stable from the first call (read tracking and staleness checks key on them), and '..' normalizes to the real target: relative traversal like ../../../etc/x from ~ was refused on main and slipped past the sensitive-path guard on the PR head. - If the remote home cannot be detected, an ssh ~-path that climbs above ~ cannot be classified; the write guard refuses it. - ~user passes through for the remote shell instead of becoming ~/~user. - coerce_ssh_remote_cwd maps paths under the host subprocess home onto ~/, except when that home is the OS user's real home. - The outside-workspace warning compares in the remote namespace (it fired on every correct relative write when the anchor was ~). - The backend type is looked up once per resolution again (the PR head did three per local path).
This commit is contained in:
@@ -76,6 +76,7 @@ class SSHEnvironment(BaseEnvironment):
|
||||
if probe_only:
|
||||
self._sync_manager = None
|
||||
return
|
||||
self._remote_home_detected = False
|
||||
self._remote_home = self._detect_remote_home()
|
||||
self._ensure_remote_dirs()
|
||||
self._sync_manager = FileSyncManager(
|
||||
@@ -153,6 +154,7 @@ class SSHEnvironment(BaseEnvironment):
|
||||
result = self._run_ssh("echo $HOME", timeout=10)
|
||||
if result.returncode == 0 and result.stdout.strip():
|
||||
logger.debug("SSH: remote home = %s", result.stdout.strip())
|
||||
self._remote_home_detected = True
|
||||
return result.stdout.strip()
|
||||
return "/root" if self.user == "root" else f"/home/{self.user}"
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ edits to the agent process cwd, e.g. the main repo during a worktree session).
|
||||
import os
|
||||
import posixpath
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path, PurePosixPath
|
||||
|
||||
# ``TERMINAL_CWD`` values that mean "not configured" ("." from a stale config;
|
||||
@@ -17,6 +18,9 @@ _TERMINAL_CWD_SENTINELS = frozenset({"", ".", "./", "auto", "cwd"})
|
||||
_CONTAINER_PATH_BACKENDS_FALLBACK = frozenset({"docker", "singularity", "modal", "daytona", "vercel_sandbox"})
|
||||
# Backend name inferred from the live environment's class name (first match wins).
|
||||
_ENV_CLASS_NAME_HINTS = ("local", "ssh", "docker", "singularity", "modal", "daytona")
|
||||
# Container task id -> monotonic time of the last failed SSH bring-up in path resolution.
|
||||
_SSH_HOME_RETRY_AFTER = 30.0
|
||||
_ssh_home_failed_at: dict[str, float] = {}
|
||||
|
||||
|
||||
def _expand_tilde(path: str) -> str:
|
||||
@@ -164,40 +168,81 @@ def _anchor(text: str, base, container_paths: bool) -> Path | PurePosixPath:
|
||||
|
||||
|
||||
def _ssh_remote_anchor(task_id: str) -> str:
|
||||
"""Working directory on the SSH target, never the Hermes host's home.
|
||||
"""Working directory on the SSH target, read RAW: ``_authoritative_workspace_root``
|
||||
expands ``~`` on the Hermes host, which names a directory the remote does not have.
|
||||
|
||||
A recorded ``~`` stays ``~``. A recorded copy of the container subprocess
|
||||
home (the host path ``get_subprocess_home()`` returns) is the same idea
|
||||
written out by mistake, so it collapses back to ``~`` for the remote shell.
|
||||
Same precedence (session record, registered override, ``$TERMINAL_CWD``); a
|
||||
value that is neither ``~``-prefixed nor POSIX-absolute (a Windows or relative
|
||||
host path) is skipped. ``coerce_ssh_remote_cwd`` maps the host subprocess home back to ``~``.
|
||||
"""
|
||||
from agent.runtime_cwd import scope_terminal_cwd
|
||||
from tools.terminal_tool import get_session_cwd, resolve_task_overrides
|
||||
from tools.terminal_tool_config import coerce_ssh_remote_cwd
|
||||
|
||||
raw = _authoritative_workspace_root(task_id) or "~"
|
||||
return coerce_ssh_remote_cwd(raw, "ssh") or "~"
|
||||
for raw in (get_session_cwd(task_id), resolve_task_overrides(task_id).get("cwd"), scope_terminal_cwd()):
|
||||
text = str(raw or "").strip()
|
||||
if text.lower() not in _TERMINAL_CWD_SENTINELS and (text.startswith("~") or posixpath.isabs(text)):
|
||||
return coerce_ssh_remote_cwd(text, "ssh")
|
||||
return "~"
|
||||
|
||||
|
||||
def _ssh_remote_home(task_id: str) -> str | None:
|
||||
"""The SSH user's home as detected at connect (``echo $HOME``), else None.
|
||||
|
||||
Brings the environment up through the file tools' own creator (same cwd
|
||||
and cache as the call that follows) when none is live: they resolve before
|
||||
touching the backend, and a first call keyed ``~/x`` while later ones key
|
||||
``/home/u/x`` splits read tracking and staleness checks for one file. A
|
||||
failed bring-up is remembered briefly so one tool call's several
|
||||
resolutions don't each wait out the SSH connect timeout; the tool's own
|
||||
backend call reports the error.
|
||||
"""
|
||||
from tools.file_tools import _get_file_ops
|
||||
from tools.terminal_tool import _resolve_container_task_id
|
||||
from tools.terminal_tool_lifecycle import get_active_env
|
||||
|
||||
key = _resolve_container_task_id(task_id)
|
||||
env = get_active_env(task_id)
|
||||
if env is None:
|
||||
if time.monotonic() - _ssh_home_failed_at.get(key, float("-inf")) < _SSH_HOME_RETRY_AFTER:
|
||||
return None
|
||||
try:
|
||||
env = _get_file_ops(task_id).env
|
||||
except Exception: # noqa: BLE001 — connect failure
|
||||
_ssh_home_failed_at[key] = time.monotonic()
|
||||
return None
|
||||
_ssh_home_failed_at.pop(key, None)
|
||||
# A guessed /home/<user> (``echo $HOME`` failed) must not stand in for the real home.
|
||||
home = getattr(env, "_remote_home", None) if getattr(env, "_remote_home_detected", False) else None
|
||||
return home if isinstance(home, str) and posixpath.isabs(home) else None
|
||||
|
||||
|
||||
def _resolve_ssh_path(filepath: str, task_id: str) -> PurePosixPath:
|
||||
"""Resolve *filepath* in the SSH target's namespace.
|
||||
"""Resolve *filepath* in the SSH target's namespace, never via the Hermes host
|
||||
(``Path.resolve()`` and ``get_subprocess_home()`` both name host directories).
|
||||
|
||||
Do not ``Path.resolve()`` or expand ``~`` with ``get_subprocess_home()``:
|
||||
both name directories on the Hermes host (``/opt/data/home`` in Docker),
|
||||
which the remote ``cd`` then rejects.
|
||||
``~`` becomes the remote home once the live environment has detected it, so the
|
||||
result is absolute and the write guards see the real target. Before that it stays
|
||||
``~``-prefixed for the remote shell; ``~user`` always does.
|
||||
"""
|
||||
text = str(filepath or "").strip()
|
||||
if not text.startswith("~") and not posixpath.isabs(text):
|
||||
text = posixpath.join(_ssh_remote_anchor(task_id), text)
|
||||
if text == "~" or text.startswith("~/"):
|
||||
return PurePosixPath(text)
|
||||
if posixpath.isabs(text):
|
||||
home = _ssh_remote_home(task_id)
|
||||
text = home + text[1:] if home else text
|
||||
if not text.startswith("~"):
|
||||
return _normalize_without_host_deref(text)
|
||||
anchor = _ssh_remote_anchor(task_id)
|
||||
if anchor == "~":
|
||||
joined = f"~/{text}"
|
||||
elif anchor.startswith("~/"):
|
||||
joined = f"{anchor.rstrip('/')}/{text}"
|
||||
else:
|
||||
joined = posixpath.normpath(posixpath.join(anchor, text))
|
||||
if joined == "~" or joined.startswith("~/"):
|
||||
return PurePosixPath(joined)
|
||||
return _normalize_without_host_deref(joined)
|
||||
head, _, tail = text.partition("/")
|
||||
tail = posixpath.normpath(tail) if tail else "."
|
||||
return PurePosixPath(head if tail == "." else f"{head}/{tail}")
|
||||
|
||||
|
||||
def _ssh_path_escapes_home(resolved: str) -> bool:
|
||||
"""True for a still-``~``-relative SSH path that climbs above ``~``: its absolute
|
||||
target is unknown until the remote home is, so no path guard can classify it."""
|
||||
tail = resolved.partition("/")[2] if resolved.startswith("~") else ""
|
||||
return tail == ".." or tail.startswith("../")
|
||||
|
||||
|
||||
def _resolve_base_dir(
|
||||
@@ -205,17 +250,14 @@ def _resolve_base_dir(
|
||||
"""Return the ABSOLUTE base directory for resolving relative paths:
|
||||
``_authoritative_workspace_root``, else the process cwd as a last resort.
|
||||
|
||||
SSH is the exception: the anchor is a remote path (often ``~``), and the
|
||||
process cwd is the Hermes host.
|
||||
SSH resolves in the remote namespace (callers passing *container_paths* have
|
||||
already ruled SSH out).
|
||||
"""
|
||||
if _terminal_env_type_for_task(task_id) == "ssh":
|
||||
anchor = _ssh_remote_anchor(task_id)
|
||||
if anchor == "~" or anchor.startswith("~/"):
|
||||
return PurePosixPath(anchor)
|
||||
return _normalize_without_host_deref(anchor)
|
||||
root = _authoritative_workspace_root(task_id)
|
||||
if container_paths is None:
|
||||
if _terminal_env_type_for_task(task_id) == "ssh":
|
||||
return _resolve_ssh_path(".", task_id)
|
||||
container_paths = _uses_container_paths(task_id)
|
||||
root = _authoritative_workspace_root(task_id)
|
||||
# A backend's relative cwd is anchored to the process cwd once, here.
|
||||
return _anchor(_host_text(root or os.getcwd(), container_paths), os.getcwd, container_paths)
|
||||
|
||||
@@ -231,20 +273,26 @@ def _resolve_path_for_task(filepath: str, task_id: str = "default") -> Path | Pu
|
||||
|
||||
|
||||
|
||||
def _path_resolution_warning(filepath: str, resolved: Path, task_id: str = "default") -> str | None:
|
||||
def _path_resolution_warning(filepath: str, resolved: Path | PurePosixPath, task_id: str = "default") -> str | None:
|
||||
"""Warn when a RELATIVE path resolved OUTSIDE the task's workspace root (the
|
||||
edit is about to land in a different checkout than the terminal's cwd).
|
||||
``None`` for absolute paths, an unknown root, or a path under the root."""
|
||||
``None`` for absolute paths, an unknown root, or a path under the root.
|
||||
SSH compares in the remote namespace, as ``_resolve_path_for_task`` resolved it."""
|
||||
try:
|
||||
if Path(_expand_tilde(filepath)).is_absolute():
|
||||
return None
|
||||
workspace_root = _authoritative_workspace_root(task_id)
|
||||
if not workspace_root:
|
||||
return None
|
||||
if _uses_container_paths(task_id):
|
||||
root = _normalize_without_host_deref(Path(_expand_tilde(workspace_root)))
|
||||
if _terminal_env_type_for_task(task_id) == "ssh":
|
||||
if filepath.startswith("~") or posixpath.isabs(filepath):
|
||||
return None
|
||||
root = _resolve_ssh_path(".", task_id)
|
||||
else:
|
||||
root = Path(_expand_tilde(workspace_root)).resolve()
|
||||
if Path(_expand_tilde(filepath)).is_absolute():
|
||||
return None
|
||||
workspace_root = _authoritative_workspace_root(task_id)
|
||||
if not workspace_root:
|
||||
return None
|
||||
if _uses_container_paths(task_id):
|
||||
root = _normalize_without_host_deref(Path(_expand_tilde(workspace_root)))
|
||||
else:
|
||||
root = Path(_expand_tilde(workspace_root)).resolve()
|
||||
if resolved.is_relative_to(root):
|
||||
return None
|
||||
return (
|
||||
|
||||
@@ -22,7 +22,8 @@ from tools.binary_extensions import (
|
||||
is_pdf_path,
|
||||
is_sqlite_sidecar,
|
||||
)
|
||||
from tools.file_tools_paths import _expand_tilde, _resolve_path_for_task
|
||||
from tools.file_tools_paths import (
|
||||
_expand_tilde, _resolve_path_for_task, _ssh_path_escapes_home, _terminal_env_type_for_task)
|
||||
from tools.file_tools_read_tracking import _has_full_write_baseline, _read_mtime_drifted
|
||||
|
||||
# Prefixes matched after realpath. macOS: /private/var mirrors /var — block the
|
||||
@@ -153,6 +154,10 @@ def _check_sensitive_path(filepath: str, task_id: str = "default") -> str | None
|
||||
if nt_err:
|
||||
return nt_err
|
||||
candidates = (_resolved_or_raw(filepath, task_id), os.path.normpath(_expand_tilde(filepath)))
|
||||
if _ssh_path_escapes_home(candidates[0]) and _terminal_env_type_for_task(task_id) == "ssh":
|
||||
return (
|
||||
f"Refusing to write to {filepath}: it climbs above the SSH home and the remote "
|
||||
"home could not be detected, so its target cannot be checked. Pass an absolute path.")
|
||||
if any(c.startswith(_SENSITIVE_PATH_PREFIXES) or c in _SENSITIVE_EXACT_PATHS for c in candidates):
|
||||
return (
|
||||
f"Refusing to write to sensitive system path: {filepath}\n"
|
||||
|
||||
@@ -9,9 +9,10 @@ so ``tools.terminal_tool.<name>`` keeps resolving (and monkeypatching) as before
|
||||
import logging
|
||||
import json
|
||||
import os
|
||||
import posixpath
|
||||
import re
|
||||
from contextlib import contextmanager
|
||||
from typing import Any
|
||||
from typing import Any, overload
|
||||
|
||||
# Log-record parity with the origin module.
|
||||
logger = logging.getLogger("tools.terminal_tool")
|
||||
@@ -95,30 +96,35 @@ def _get_plugin_env_provider(env_type: str):
|
||||
return _plugin_registry_lookup(env_type, "get_provider", None)
|
||||
|
||||
|
||||
@overload
|
||||
def coerce_ssh_remote_cwd(cwd: str, env_type: str | None) -> str: ...
|
||||
@overload
|
||||
def coerce_ssh_remote_cwd(cwd: None, env_type: str | None) -> None: ...
|
||||
def coerce_ssh_remote_cwd(cwd: str | None, env_type: str | None) -> str | None:
|
||||
"""Cwd to send to an SSH backend.
|
||||
|
||||
``~`` and ``~/...`` stay literal so the remote shell expands them to the
|
||||
``~``-prefixed paths stay literal so the remote shell expands them to the
|
||||
SSH user's home. The Hermes process's subprocess home (``/opt/data/home``
|
||||
in the official Docker image) is a directory on the machine running
|
||||
Hermes. Using it as the remote working directory makes ``cd`` exit 126
|
||||
and anchors relative writes on a path that does not exist on the target.
|
||||
Other backends are unchanged.
|
||||
Hermes: it and anything under it are rewritten onto the remote ``~``, since
|
||||
``cd`` into the host path exits 126 on the target. A subprocess home that is
|
||||
the OS user's real home is left alone: a remote path may legitimately match
|
||||
it. Other backends are unchanged.
|
||||
"""
|
||||
if not isinstance(cwd, str) or (env_type or "").strip().lower() != "ssh":
|
||||
return cwd
|
||||
text = cwd.strip()
|
||||
if not text or text == "~" or text.startswith("~/"):
|
||||
if not text or text.startswith("~"):
|
||||
return text or "~"
|
||||
try:
|
||||
from hermes_constants import get_subprocess_home
|
||||
from hermes_constants import get_real_home, get_subprocess_home
|
||||
|
||||
home = get_subprocess_home()
|
||||
except Exception:
|
||||
home = None
|
||||
if home and os.path.normpath(text) == os.path.normpath(home):
|
||||
home = get_subprocess_home()
|
||||
if not home or not posixpath.isabs(text) or posixpath.normpath(home) == posixpath.normpath(get_real_home()):
|
||||
return text
|
||||
rel = posixpath.relpath(posixpath.normpath(text), posixpath.normpath(home))
|
||||
if rel == ".":
|
||||
return "~"
|
||||
return text
|
||||
return text if rel == ".." or rel.startswith("../") else f"~/{rel}"
|
||||
|
||||
|
||||
def _is_unusable_container_cwd(cwd: str) -> bool:
|
||||
|
||||
Reference in New Issue
Block a user