fix(security): reject file:// URLs that name a host (1.0.2b7+ra.2)

src/octop/api/routers/workspace.py

@@ -4,6 +4,7 @@
 
 import logging
 import re
+import urllib.parse
 from pathlib import Path
 from typing import Any, Literal
 
@@ -155,6 +156,24 @@ def _workspace_io_path(path: str, *, from_workspace: bool = False) -> str:
     """
     raw = path.strip()
     if raw.startswith("file://"):
+        # A file:// URL must not carry a host. `urlparse` puts the drive letter
+        # in netloc for the two-slash form Windows clients send
+        # (`file://C:/…` from `Path.as_posix()`), and that is the only netloc
+        # we accept. Anything else is rejected rather than resolved:
+        #
+        # - `file://server/share/x` (UNC) would otherwise silently resolve to
+        #   the local path `/share/x`.
+        # - `file://C:\Users\x` (backslashes) puts the whole path in netloc
+        #   and used to resolve to `.`, the server process's cwd.
+        #
+        # Resolving either and then testing containment depends on the host
+        # filesystem layout. Denying here does not.
+        netloc = urllib.parse.urlparse(raw).netloc
+        is_drive = len(netloc) == 2 and netloc[1] == ":" and netloc[0].isalpha()
+        if netloc and not is_drive:
+            raise OctopError(
+                ErrorCode.FORBIDDEN, f"cannot access {path!r}: file:// URL must not name a host"
+            )
         resolved = backend_workspace_path(raw)
         if resolved is None:
             raise OctopError(ErrorCode.NOT_FOUND, f"cannot resolve {path!r}")
@@ -222,7 +241,13 @@ async def read_file(
     """
     ws = await require_agent_workspace(agent_id, user=user, as_user=as_user, server=server)
     io_path = _safe_io_path(ws, path, from_workspace=False)
-    content = await ws.aread_text(io_path)
+    try:
+        content = await ws.aread_text(io_path)
+    except PermissionError as exc:
+        # The backend's own containment check can still deny a path that
+        # passed _safe_io_path (e.g. ``C:/Users/...`` on Windows). Map it like
+        # download_file/read_doc instead of letting it surface as a 500.
+        raise OctopError(ErrorCode.NOT_FOUND, f"cannot read {path!r}") from exc
     if content is None:
         raise OctopError(ErrorCode.NOT_FOUND, f"cannot read {path!r}")
     return {"path": path, "content": coerce_read_content(content)}

tests/integration/test_workspace_api.py

@@ -582,3 +582,54 @@ async def test_from_workspace_param_is_rejected_as_unknown(env: Any) -> None:
     )
     # Workspace-relative 'etc/passwd' simply does not exist → 404, never 200.
     assert r.status_code == 404, r.text
+
+
+@pytest.mark.asyncio
+async def test_file_url_with_host_netloc_forbidden(env: Any) -> None:
+    """A file:// URL must not name a host - SEC-2, variant of the netloc bug.
+
+    urlparse puts everything between // and the next / into netloc. Only a bare
+    drive letter (file://C:/..., which Windows clients emit from
+    Path.as_posix()) is meaningful here. A UNC host used to resolve to the local
+    path /share/x, and a backslash path used to resolve to `.`, the process cwd.
+    Both now fail closed with 403 instead of being resolved and then tested
+    against a host-layout-dependent allowlist.
+    """
+    c, _srv, auth, aid = env
+    hostiles = [
+        "file://server/share/etc/passwd",
+        "file://attacker.example/share/x",
+        "file://C:" + chr(92) + "Users" + chr(92) + "a" + chr(92) + "x.txt",
+    ]
+    for hostile in hostiles:
+        r = await c.get(
+            f"/api/agents/{aid}/workspace/file",
+            params={"path": hostile},
+            headers=auth,
+        )
+        assert r.status_code == 403, (hostile, r.text)
+
+
+@pytest.mark.asyncio
+async def test_file_url_bare_drive_netloc_is_not_rejected_as_host(env: Any) -> None:
+    """The two-slash Windows form must keep working - SEC-2 regression guard.
+
+    file://C:/... is the shape Windows CI builds. It must resolve to a real
+    absolute path: not rejected as a host, and not collapsed to `.`.
+    """
+    c, _srv, auth, aid = env
+    # Built literally: Path.as_posix() only produces a drive letter on Windows,
+    # so on Linux CI this would silently degrade to the three-slash form and
+    # stop testing the shape we care about.
+    drive_url = "file://C:/Users/nobody/sec2-drive-shape-must-not-exist.txt"
+    r = await c.get(
+        f"/api/agents/{aid}/workspace/file",
+        params={"path": drive_url},
+        headers=auth,
+    )
+    # A bare drive letter is the one netloc we accept, so the host rejection
+    # must not fire on the shape Windows actually sends. Assert on the reason,
+    # not the status: on Windows the path is host-absolute and the containment
+    # layer still denies it (404); on Linux it is workspace-relative (404).
+    assert "must not name a host" not in r.text, (drive_url, r.text)
+    assert r.status_code in (403, 404), (drive_url, r.status_code, r.text)