fix(security): workspace path allowlist + session ownership (#655) (#705)

* fix(security): workspace path allowlist + session ownership (#655)

Prevent authenticated cross-tenant RCE by constraining every client-supplied
workspace path to a configurable allowlist (WORKSPACE_ROOT), and binding
interactive sessions to their creating user.

- enforce_workspace_allowlist() (ui/dependencies.py): resolved path must be
  within a WORKSPACE_ROOT entry; '..' escapes collapse via resolve(). In HOSTED
  mode the allowlist is mandatory (fail closed) and each user is confined to
  <root>/<user_id>. No-op when WORKSPACE_ROOT is unset (single-operator default).
- Applied at all client-supplied-path funnels: get_v2_workspace (REST),
  POST /api/v2/workspaces (init), and POST /api/v2/sessions (create).
- interactive_sessions gains a user_id column (+ idempotent ALTER migration);
  create_session persists the authed user. terminal_ws/session_chat_ws now fail
  closed: when auth is enabled an ownerless or mismatched session is rejected.

Closes the RCE foundation; remaining hosted multi-tenancy hardening (session
REST owner-scoping, TOCTOU symlink isolation) tracked as follow-up.

* test(security): assert WebSocketDisconnect(1008) on chat ownership mismatch (#655)

CodeRabbit: tighten the broad pytest.raises(Exception) to the specific
WebSocketDisconnect with the policy-violation close code.

* docs: document WORKSPACE_ROOT allowlist + deployment mode env vars (#655)

Author: frankbria

CLAUDE.md

@@ -267,6 +267,16 @@ ANTHROPIC_API_KEY=sk-ant-...          # Required for Anthropic provider (default
 E2B_API_KEY=e2b_...                   # Required for --engine cloud
 DATABASE_PATH=./codeframe.db          # Optional
 
+# Server deployment + workspace allowlist (#655)
+CODEFRAME_DEPLOYMENT_MODE=self_hosted # self_hosted (default) or hosted (multi-tenant)
+WORKSPACE_ROOT=/srv/workspaces        # os.pathsep-separated allowlist of permitted
+                                      # workspace roots. Any client-supplied workspace
+                                      # path (REST, workspace init, session create) must
+                                      # resolve inside a root, else 403. Unset = no
+                                      # allowlist (single-operator self-hosted default).
+                                      # In hosted mode it is MANDATORY (fail closed) and
+                                      # each user is confined to <root>/<user_id>.
+
 # LLM Provider selection (multi-provider support)
 # Priority: CLI flag > env var > .codeframe/config.yaml > default (anthropic)
 CODEFRAME_LLM_PROVIDER=anthropic      # Provider: anthropic (default), openai, ollama, vllm, compatible

codeframe/platform_store/repositories/interactive_sessions.py

@@ -23,17 +23,19 @@ def create(
         task_id: Optional[str] = None,
         agent_type: str = "claude",
         model: Optional[str] = None,
+        user_id: Optional[int] = None,
     ) -> dict:
         now = datetime.now(UTC).isoformat()
         session_id = str(uuid.uuid4())
         self._execute(
             """
             INSERT INTO interactive_sessions
                 (id, workspace_path, task_id, state, agent_type, model,
-                 cost_usd, input_tokens, output_tokens, created_at, updated_at, ended_at)
-            VALUES (?, ?, ?, 'active', ?, ?, 0.0, 0, 0, ?, ?, NULL)
+                 cost_usd, input_tokens, output_tokens, created_at, updated_at,
+                 ended_at, user_id)
+            VALUES (?, ?, ?, 'active', ?, ?, 0.0, 0, 0, ?, ?, NULL, ?)
             """,
-            (session_id, workspace_path, task_id, agent_type, model, now, now),
+            (session_id, workspace_path, task_id, agent_type, model, now, now, user_id),
         )
         self._commit()
         return self.get(session_id)

codeframe/platform_store/schema_manager.py

@@ -179,10 +179,23 @@ def _create_interactive_session_tables(self, cursor: sqlite3.Cursor) -> None:
                 output_tokens INTEGER DEFAULT 0,
                 created_at  TEXT NOT NULL,
                 updated_at  TEXT NOT NULL,
-                ended_at    TEXT
+                ended_at    TEXT,
+                -- Owning user; the terminal/chat WebSockets reject a session
+                -- whose owner != the authenticated user (issue #655). NULL in
+                -- no-auth mode, where ownership is intentionally not enforced.
+                user_id     INTEGER REFERENCES users(id) ON DELETE SET NULL
             )
             """
         )
+        # Migrate pre-#655 databases that created the table without user_id.
+        existing_cols = {
+            row[1] for row in cursor.execute("PRAGMA table_info(interactive_sessions)")
+        }
+        if "user_id" not in existing_cols:
+            cursor.execute(
+                "ALTER TABLE interactive_sessions ADD COLUMN user_id INTEGER "
+                "REFERENCES users(id) ON DELETE SET NULL"
+            )
 
         cursor.execute(
             """

codeframe/ui/dependencies.py

@@ -6,17 +6,79 @@
 v2-only: All dependencies use codeframe.core modules.
 """
 
+import os
 from pathlib import Path
-from typing import Optional
+from typing import Any, Dict, Optional
 
-from fastapi import HTTPException, Query, Request
+from fastapi import Depends, HTTPException, Query, Request
 
+from codeframe.auth.dependencies import require_auth
 from codeframe.workspace import WorkspaceManager
 
 # v2 imports
 from codeframe.core.workspace import Workspace, get_workspace, workspace_exists
 
 
+def _allowed_workspace_roots() -> list[Path]:
+    """Permitted workspace roots from ``WORKSPACE_ROOT`` (os.pathsep-separated).
+
+    Empty when unset — meaning "no allowlist" (single-operator self-hosted, the
+    default). Each root is resolved so containment checks defeat ``..`` escapes.
+    """
+    raw = os.getenv("WORKSPACE_ROOT", "").strip()
+    if not raw:
+        return []
+    return [
+        Path(p).expanduser().resolve()
+        for p in raw.split(os.pathsep)
+        if p.strip()
+    ]
+
+
+def _within_any_root(path: Path, roots: list[Path]) -> bool:
+    return any(path == r or path.is_relative_to(r) for r in roots)
+
+
+def enforce_workspace_allowlist(path: Path, user_id: Optional[int]) -> Path:
+    """Validate a resolved workspace path against the allowlist (issue #655).
+
+    Shared by every entry point that resolves a client-supplied workspace path:
+    ``get_v2_workspace`` (REST) and interactive session creation (whose stored
+    path later becomes a terminal shell's ``cwd``). Without it, an authenticated
+    user can point operations at any host directory — authenticated
+    cross-tenant RCE once the server serves >1 user.
+
+    Returns the (resolved) path on success; raises ``HTTPException`` otherwise.
+    """
+    # Local import avoids a circular import (server -> routers -> dependencies).
+    from codeframe.ui.server import is_hosted_mode
+
+    path = path.resolve()
+    roots = _allowed_workspace_roots()
+    if is_hosted_mode():
+        # Hosted/multi-tenant: the allowlist is mandatory (fail closed) and each
+        # user is confined to <root>/<user_id> so one tenant can't reach
+        # another's subtree.
+        # ponytail: path-namespace binding, not a DB ownership table. Upgrade to
+        # registry-backed owner_user_id checks if workspaces ever live outside a
+        # per-user root.
+        if not roots:
+            raise HTTPException(
+                status_code=500,
+                detail="Server misconfigured: WORKSPACE_ROOT must be set in hosted mode.",
+            )
+        if user_id is None:
+            raise HTTPException(status_code=403, detail="Authenticated user required.")
+        roots = [r / str(user_id) for r in roots]
+
+    if roots and not _within_any_root(path, roots):
+        raise HTTPException(
+            status_code=403,
+            detail="Workspace path is outside the permitted workspace roots.",
+        )
+    return path
+
+
 def get_workspace_manager(request: Request) -> WorkspaceManager:
     """Get workspace manager from application state.
 
@@ -41,6 +103,7 @@ def get_v2_workspace(
         description="Path to workspace directory (defaults to server's working directory)",
     ),
     request: Request = None,
+    auth: Dict[str, Any] = Depends(require_auth),
 ) -> Workspace:
     """Get v2 Workspace from path or server default.
 
@@ -76,6 +139,9 @@ async def endpoint(workspace: Workspace = Depends(get_v2_workspace)):
         # Fall back to current working directory
         path = Path.cwd()
 
+    # Enforce the workspace allowlist (issue #655).
+    path = enforce_workspace_allowlist(path, auth.get("user_id"))
+
     # Validate workspace exists
     # Note: Avoid exposing full filesystem paths in error messages for hosted deployments
     if not workspace_exists(path):
@@ -100,4 +166,5 @@ async def endpoint(workspace: Workspace = Depends(get_v2_workspace)):
 __all__ = [
     "get_workspace_manager",
     "get_v2_workspace",
+    "enforce_workspace_allowlist",
 ]

codeframe/ui/routers/interactive_sessions_v2.py

@@ -10,17 +10,21 @@
     POST   /api/v2/sessions/{id}/messages          - Add message to session
     GET    /api/v2/sessions/{id}/messages          - Get message history (?limit=&offset=)
 
-Auth note: auth is a project-wide gap tracked in #336. All other v2 routers also
-lack auth — adding it only here would be inconsistent with the rest of the API.
+Auth: enforced project-wide via router-level ``require_auth`` (#336). Session
+creation additionally validates ``workspace_path`` against the workspace
+allowlist (#655) since the stored path becomes a terminal shell ``cwd``.
 """
 
 import logging
-from typing import Optional
+from pathlib import Path
+from typing import Any, Dict, Optional
 
-from fastapi import APIRouter, HTTPException, Query, Request
+from fastapi import APIRouter, Depends, HTTPException, Query, Request
 from pydantic import BaseModel, field_validator
 
+from codeframe.auth.dependencies import require_auth
 from codeframe.lib.rate_limiter import rate_limit_standard
+from codeframe.ui.dependencies import enforce_workspace_allowlist
 
 logger = logging.getLogger(__name__)
 
@@ -136,14 +140,27 @@ def _session_to_response(row: dict) -> SessionResponse:
 
 @router.post("", status_code=201, response_model=SessionResponse)
 @rate_limit_standard()
-def create_session(body: SessionCreate, request: Request):
-    """Create a new interactive agent session."""
+def create_session(
+    body: SessionCreate,
+    request: Request,
+    auth: Dict[str, Any] = Depends(require_auth),
+):
+    """Create a new interactive agent session.
+
+    The stored ``workspace_path`` later becomes a terminal shell's ``cwd``
+    (terminal_ws / session_chat_ws), so it must clear the same allowlist as
+    REST workspace access (issue #655) — validated and resolved here.
+    """
     repo = _get_repo(request)
+    workspace_path = enforce_workspace_allowlist(
+        Path(body.workspace_path), auth.get("user_id")
+    )
     session = repo.create(
-        workspace_path=body.workspace_path,
+        workspace_path=str(workspace_path),
         task_id=body.task_id,
         agent_type=body.agent_type,
         model=body.model,
+        user_id=auth.get("user_id"),
     )
     return _session_to_response(session)
 

codeframe/ui/routers/session_chat_ws.py

@@ -108,7 +108,7 @@ async def _run_streaming_adapter(
 async def session_chat_ws(session_id: str, websocket: WebSocket) -> None:
     """Bidirectional WebSocket for streaming agent chat on an interactive session."""
     # --- Auth ---
-    authenticated, _user_id = await _authenticate_websocket(websocket)
+    authenticated, user_id = await _authenticate_websocket(websocket)
     if not authenticated:
         return
 
@@ -123,6 +123,19 @@ async def session_chat_ws(session_id: str, websocket: WebSocket) -> None:
         await websocket.close(code=4008, reason="Session not found or ended")
         return
 
+    # --- Ownership check (issue #655) ---
+    # The chat agent runs file-system tools scoped to the session's workspace, so
+    # one authenticated user must not drive another user's session. Skipped in
+    # no-auth mode (user_id is None), matching terminal_ws / REST behavior.
+    # Fail closed: when auth is enabled (user_id is not None), an ownerless
+    # (NULL, e.g. pre-#655 migrated) or mismatched session is rejected.
+    session_user_id = session.get("user_id")
+    if user_id is not None and (
+        session_user_id is None or int(session_user_id) != user_id
+    ):
+        await websocket.close(code=1008, reason="Forbidden: session belongs to another user")
+        return
+
     # --- Accept connection; everything after this point must run inside the
     #     try/finally so unregister() and close() always execute even if
     #     update_state, register, or get_token_queue raises. ---

codeframe/ui/routers/terminal_ws.py

@@ -76,9 +76,13 @@ async def session_terminal_ws(session_id: str, websocket: WebSocket) -> None:
 
     # --- Ownership check ---
     # Skipped in no-auth mode (user_id is None): there is no identity to enforce,
-    # matching REST behavior under CODEFRAME_AUTH_REQUIRED=false.
+    # matching REST behavior under CODEFRAME_AUTH_REQUIRED=false. When auth IS
+    # enabled, fail closed — an ownerless (NULL, e.g. pre-#655 migrated) or
+    # mismatched session is rejected.
     session_user_id = session.get("user_id")
-    if user_id is not None and session_user_id is not None and int(session_user_id) != user_id:
+    if user_id is not None and (
+        session_user_id is None or int(session_user_id) != user_id
+    ):
         await websocket.close(code=4003, reason="Forbidden: session belongs to another user")
         return
 

codeframe/ui/routers/workspace_v2.py

@@ -19,7 +19,8 @@
 
 from codeframe.core import workspace as ws
 from codeframe.lib.rate_limiter import rate_limit_standard
-from codeframe.ui.dependencies import get_v2_workspace
+from codeframe.auth.dependencies import require_auth
+from codeframe.ui.dependencies import enforce_workspace_allowlist, get_v2_workspace
 from codeframe.core.workspace import WORKSPACE_CONFIG_FILENAME, Workspace
 from codeframe.ui.response_models import api_error, ErrorCodes
 from codeframe.ui.routers._helpers import atomic_write_json
@@ -225,6 +226,7 @@ async def list_workspaces(request: Request) -> WorkspaceListResponse:
 async def init_workspace(
     request: Request,
     body: InitWorkspaceRequest,
+    auth: dict = Depends(require_auth),
 ) -> WorkspaceResponse:
     """Initialize a new workspace for a repository.
 
@@ -244,7 +246,13 @@ async def init_workspace(
             - 404: Repository path not found
     """
     try:
-        repo_path = Path(body.repo_path).resolve()
+        # Enforce the workspace allowlist before touching disk (issue #655):
+        # initializing a workspace writes a .codeframe/ marker, so an arbitrary
+        # repo_path would let an authenticated user seed workspaces anywhere on
+        # the host.
+        repo_path = enforce_workspace_allowlist(
+            Path(body.repo_path), auth.get("user_id")
+        )
 
         # Validate path exists
         if not repo_path.exists():

tests/api/test_session_chat_ws.py

@@ -9,6 +9,8 @@
 and explicit teardown within tests where needed.
 """
 
+import os
+
 import pytest
 from unittest.mock import patch
 from fastapi.testclient import TestClient
@@ -25,8 +27,14 @@
 # ---------------------------------------------------------------------------
 
 
-def _create_session(client: TestClient, workspace_path: str = "/tmp/ws-test") -> str:
-    """Create an interactive session and return its ID."""
+def _create_session(client: TestClient, workspace_path: str | None = None) -> str:
+    """Create an interactive session and return its ID.
+
+    Defaults to a path under the test ``WORKSPACE_ROOT`` (set by the api
+    conftest) so it clears the workspace allowlist (#655).
+    """
+    if workspace_path is None:
+        workspace_path = os.path.join(os.environ.get("WORKSPACE_ROOT", "/tmp"), "ws-test")
     resp = client.post(
         "/api/v2/sessions",
         json={"workspace_path": workspace_path, "agent_type": "claude"},
@@ -39,6 +47,36 @@ def _ws_url(session_id: str, token: str) -> str:
     return f"/ws/sessions/{session_id}/chat?token={token}"
 
 
+def test_chat_ws_ownership_mismatch_closes():
+    """A session owned by another user is refused (issue #655)."""
+    from unittest.mock import AsyncMock, MagicMock, patch
+
+    from fastapi import FastAPI
+    from fastapi.testclient import TestClient
+
+    from codeframe.ui.routers.session_chat_ws import router
+
+    app = FastAPI()
+    app.include_router(router)
+    fake_db = MagicMock()
+    fake_db.interactive_sessions.get.return_value = {
+        "state": "active",
+        "workspace_path": "/tmp",
+        "user_id": 999,
+    }
+    app.state.db = fake_db
+    client = TestClient(app)
+
+    with patch(
+        "codeframe.ui.routers.session_chat_ws._authenticate_websocket",
+        new=AsyncMock(return_value=(True, 1)),
+    ):
+        with pytest.raises(WebSocketDisconnect) as exc:
+            with client.websocket_connect("/ws/sessions/s1/chat?token=x"):
+                pass
+        assert exc.value.code == 1008
+
+
 # ---------------------------------------------------------------------------
 # Auth tests
 # ---------------------------------------------------------------------------