mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 03:00:23 +00:00
fix(workspace): allow adding external paths not under home directory (#991)
The workspace add endpoint used resolve_trusted_workspace() which blocks any path outside the user's home directory, the saved workspace list, or BOOT_DEFAULT_WORKSPACE. This created a circular dependency: to add /mnt/d/Projects you need it in the saved list, but to get it in the list you need to add it. Fix: introduce validate_workspace_to_add() used by /api/workspaces/add, which only blocks non-existent paths, non-directories, and known system roots. The stricter resolve_trusted_workspace() is still used for actual file operations within a workspace. Fixes #953. Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
This commit is contained in:
+2
-1
@@ -328,6 +328,7 @@ from api.workspace import (
|
||||
read_file_content,
|
||||
safe_resolve_ws,
|
||||
resolve_trusted_workspace,
|
||||
validate_workspace_to_add,
|
||||
)
|
||||
from api.upload import handle_upload, handle_transcribe
|
||||
from api.streaming import _sse, _run_agent_streaming, cancel_stream
|
||||
@@ -3029,7 +3030,7 @@ def _handle_workspace_add(handler, body):
|
||||
if not path_str:
|
||||
return bad(handler, "path is required")
|
||||
try:
|
||||
p = resolve_trusted_workspace(path_str)
|
||||
p = validate_workspace_to_add(path_str)
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
wss = load_workspaces()
|
||||
|
||||
@@ -430,6 +430,36 @@ def resolve_trusted_workspace(path: str | Path | None = None) -> Path:
|
||||
)
|
||||
|
||||
|
||||
|
||||
|
||||
def validate_workspace_to_add(path: str) -> Path:
|
||||
"""Validate a path for *adding* to the workspace list (less restrictive than resolve_trusted_workspace).
|
||||
|
||||
When a user explicitly adds a new workspace path, we trust their intent — they
|
||||
have console or filesystem access to that path and are consciously registering it.
|
||||
We only block: non-existent paths, non-directories, and known system roots.
|
||||
|
||||
The stricter ``resolve_trusted_workspace`` is used when *using* an existing workspace
|
||||
(file reads/writes) to prevent path traversal after the list is built.
|
||||
"""
|
||||
candidate = Path(path).expanduser().resolve()
|
||||
|
||||
if not candidate.exists():
|
||||
raise ValueError(f"Path does not exist: {candidate}")
|
||||
if not candidate.is_dir():
|
||||
raise ValueError(f"Path is not a directory: {candidate}")
|
||||
|
||||
# Block known system roots and their immediate children
|
||||
for blocked in _workspace_blocked_roots():
|
||||
try:
|
||||
candidate.relative_to(blocked)
|
||||
raise ValueError(f"Path points to a system directory: {candidate}")
|
||||
except ValueError as e:
|
||||
if "system directory" in str(e):
|
||||
raise
|
||||
|
||||
return candidate
|
||||
|
||||
def safe_resolve_ws(root: Path, requested: str) -> Path:
|
||||
"""Resolve a relative path inside a workspace root, raising ValueError on traversal."""
|
||||
resolved = (root / requested).resolve()
|
||||
|
||||
+15
-2
@@ -166,12 +166,25 @@ def test_chat_start_rejects_workspace_outside_trusted_root(tmp_path):
|
||||
assert "outside" in result.get("error", "").lower()
|
||||
|
||||
|
||||
def test_workspace_add_rejects_path_outside_trusted_root(tmp_path):
|
||||
def test_workspace_add_allows_external_valid_paths(tmp_path):
|
||||
"""Adding a path outside home is now allowed when the user explicitly provides it.
|
||||
The strict trust check (resolve_trusted_workspace) is only applied when *using*
|
||||
an existing workspace, not when registering a new one (validate_workspace_to_add)."""
|
||||
outside = tmp_path / "outside-add"
|
||||
outside.mkdir(parents=True, exist_ok=True)
|
||||
result, status = post("/api/workspaces/add", {"path": str(outside), "name": "Outside"})
|
||||
# Explicit registration of an external path is now allowed
|
||||
assert status == 200, f"Expected 200, got {status}: {result}"
|
||||
# Verify it was actually saved
|
||||
wss_result, ws_status = get("/api/workspaces")
|
||||
paths = [w["path"] for w in wss_result.get("workspaces", [])]
|
||||
assert str(outside.resolve()) in paths
|
||||
|
||||
|
||||
def test_workspace_add_rejects_system_paths():
|
||||
"""System paths (/, /etc, /sys) are always rejected even with the relaxed add validation."""
|
||||
_, status = post("/api/workspaces/add", {"path": "/etc", "name": "System"})
|
||||
assert status == 400
|
||||
assert "outside" in result.get("error", "").lower()
|
||||
|
||||
|
||||
def test_session_new_rejects_workspace_outside_trusted_root(tmp_path):
|
||||
|
||||
Reference in New Issue
Block a user