diff --git a/api/routes.py b/api/routes.py index 31bd0377..ce8d0e50 100644 --- a/api/routes.py +++ b/api/routes.py @@ -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() diff --git a/api/workspace.py b/api/workspace.py index b66b42fe..8d194f85 100644 --- a/api/workspace.py +++ b/api/workspace.py @@ -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() diff --git a/tests/test_sprint3.py b/tests/test_sprint3.py index 0184854f..4a59de20 100644 --- a/tests/test_sprint3.py +++ b/tests/test_sprint3.py @@ -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):