WS auth issue

app/ui_layer/adapters/browser_adapter.py

@@ -129,6 +129,7 @@
 from app.ui_layer.diagnostics import LoopStallMonitor
 from app.ui_layer.adapters.ws_channel import ClientChannel
 from app.ui_layer.adapters.ws_lanes import message_lane
+from app.ui_layer.adapters.ws_auth import WS_PROTOCOL, WsAuth
 from app.ui_layer.adapters.session_buffer import (
     SESSION_BUFFER_LIMIT,
     SESSION_BUFFER_SLACK,
@@ -838,6 +839,8 @@ def __init__(
         super().__init__(controller, "browser")
         self._host = host
         self._port = int(os.environ.get("BROWSER_PORT", port))
+        # Origin allowlist + per-process session token for /ws (ws_auth.py).
+        self._ws_auth = WsAuth(self._port)
         self._theme_adapter = BrowserThemeAdapter(BaseTheme())
         self._chat = BrowserChatComponent(self)
         self._action_panel = BrowserActionPanelComponent(self)
@@ -1037,10 +1040,11 @@ async def _on_start(self) -> None:
             )
             self._chat._messages.insert(0, welcome_message)
 
-        self._app = web.Application()
+        self._app = web.Application(middlewares=[self._api_guard()])
 
         # API and WebSocket routes (must be registered first)
         self._app.router.add_get("/ws", self._websocket_handler)
+        self._app.router.add_get("/api/session-token", self._session_token_handler)
         self._app.router.add_get("/api/state", self._state_handler)
         self._app.router.add_get("/api/debug/loop", self._debug_loop_handler)
         if os.getenv("CRAFTBOT_DEBUG_ENDPOINTS") == "1":
@@ -1204,9 +1208,20 @@ async def _websocket_handler(
         from aiohttp import web, WSMsgType
         import asyncio
 
+        # Reject foreign pages before upgrading (browsers apply no CORS here).
+        rejected = self._ws_auth.check_ws_handshake(request.headers)
+        if rejected:
+            logger.warning(
+                f"[BROWSER ADAPTER] Rejected /ws handshake ({rejected}): "
+                f"origin={request.headers.get('Origin')!r} "
+                f"host={request.headers.get('Host')!r}"
+            )
+            raise web.HTTPForbidden(reason="WebSocket handshake not allowed")
+
         ws = web.WebSocketResponse(
             max_msg_size=100 * 1024 * 1024,
             heartbeat=30.0,  # Send ping every 30s to keep connection alive
+            protocols=(WS_PROTOCOL,),
         )
 
         try:
@@ -3687,9 +3702,14 @@ async def _chat_attachment_upload_handler(
         from aiohttp import web
 
         try:
-            name = (
-                request.rel_url.query.get("name", "attachment").strip() or "attachment"
-            )
+            # A display name, never a path: keep only the final component, or
+            # "../../x" walks out of download/ (Windows collapses ".." without
+            # the intermediate directory existing).
+            name = Path(
+                request.rel_url.query.get("name", "").strip().replace("\\", "/")
+            ).name
+            if name in ("", ".", ".."):
+                name = "attachment"
             file_type = (
                 request.rel_url.query.get("type", "application/octet-stream").strip()
                 or "application/octet-stream"
@@ -8561,8 +8581,10 @@ def _validate_path(self, file_path: str) -> Path:
             target = workspace / file_path
         target = target.resolve()
 
-        # Security check - ensure path is within workspace
-        if not str(target).startswith(str(workspace)):
+        # Security check - ensure path is within workspace. Containment by
+        # path components, never by string prefix: "../workspace_x/f" resolves
+        # to a SIBLING whose string starts with the workspace's.
+        if not target.is_relative_to(workspace):
             raise ValueError(f"Path '{file_path}' is outside workspace")
 
         return target
@@ -9739,6 +9761,53 @@ async def _index_handler(self, request: "web.Request") -> "web.Response":
         html = self._get_index_html()
         return web.Response(text=html, content_type="text/html")
 
+    def _api_guard(self):
+        """Middleware: /api/* refuses rebinding Hosts and foreign-origin writes
+        (ws_auth.check_api_request). /ws guards its own handshake."""
+        from aiohttp import web
+
+        @web.middleware
+        async def api_guard(request, handler):
+            if request.path == "/api" or request.path.startswith("/api/"):
+                rejected = self._ws_auth.check_api_request(
+                    request.method, request.headers
+                )
+                if rejected:
+                    logger.warning(
+                        f"[BROWSER ADAPTER] Refused {request.method} {request.path} "
+                        f"({rejected}): origin={request.headers.get('Origin')!r} "
+                        f"host={request.headers.get('Host')!r}"
+                    )
+                    raise web.HTTPForbidden(reason="Request not allowed")
+            return await handler(request)
+
+        return api_guard
+
+    async def _session_token_handler(self, request: "web.Request") -> "web.Response":
+        """Hand the /ws session token to CraftBot's own UI.
+
+        No CORS headers are ever set here, so a cross-origin page can't read
+        the response; ws_auth also refuses rebinding Hosts and cross-site
+        fetches outright.
+        """
+        from aiohttp import web
+
+        rejected = self._ws_auth.check_token_request(request.headers)
+        if rejected:
+            logger.warning(
+                f"[BROWSER ADAPTER] Refused session token ({rejected}): "
+                f"origin={request.headers.get('Origin')!r} "
+                f"host={request.headers.get('Host')!r}"
+            )
+            raise web.HTTPForbidden()
+        return web.json_response(
+            {"token": self._ws_auth.token},
+            headers={
+                "Cache-Control": "no-store",
+                "X-Content-Type-Options": "nosniff",
+            },
+        )
+
     async def _state_handler(self, request: "web.Request") -> "web.Response":
         """API endpoint for current state."""
         from aiohttp import web
@@ -10057,9 +10126,17 @@ def _get_index_html(self) -> str:
         let ws;
         let state = { messages: [], actions: [], status: 'Connecting...' };
 
-        function connect() {
+        async function connect() {
             const protocol = location.protocol === 'https:' ? 'wss:' : 'ws:';
-            ws = new WebSocket(`${protocol}//${location.host}/ws`);
+            let token;
+            try {
+                const resp = await fetch('/api/session-token', { cache: 'no-store' });
+                token = (await resp.json()).token;
+            } catch (err) {
+                setTimeout(connect, 2000);
+                return;
+            }
+            ws = new WebSocket(`${protocol}//${location.host}/ws`, ['craftbot', `craftbot-auth.${token}`]);
 
             ws.onopen = () => {
                 console.log('Connected to CraftBot');

app/ui_layer/adapters/ws_auth.py

@@ -0,0 +1,137 @@
+"""Request guards for the browser UI backend: the /ws handshake and /api/*.
+
+Browsers apply no CORS to WebSocket handshakes, so without a check any page
+the user opens could drive the agent through /ws. Two independent checks:
+
+1. Origin (and Host) — the handshake must come from CraftBot's own UI origin,
+   addressed by a loopback name. The Host check defeats DNS rebinding.
+2. Session token — a random secret generated at startup. The UI reads it from
+   GET /api/session-token (same-origin only: no CORS headers, so a foreign
+   page can't read the response) and sends it in Sec-WebSocket-Protocol, which
+   keeps it out of URLs and access logs.
+
+/api/* gets the Host check plus an Origin check on writes (check_api_request):
+the same cross-site threat over plain HTTP, where multipart/text-plain POSTs
+need no preflight.
+
+Legitimate UI origins: the frontend port (Vite dev server, or run.py's static
+server) and the backend port (aiohttp serving the built UI itself), each as
+localhost / 127.0.0.1 / [::1]. Tunnels and LAN URLs only ever expose Agent App
+ports, never this UI, so they are not allowed here.
+"""
+
+from __future__ import annotations
+
+import hmac
+import os
+import secrets
+from typing import Mapping, Optional, Set
+
+# Subprotocol the server echoes back; the token rides alongside it as
+# "craftbot-auth.<token>" in the client's protocol list.
+WS_PROTOCOL = "craftbot"
+WS_TOKEN_PROTOCOL_PREFIX = "craftbot-auth."
+
+_LOOPBACK_HOSTS = ("localhost", "127.0.0.1", "[::1]")
+_DEFAULT_FRONTEND_PORT = 7925
+_MUTATING = frozenset({"POST", "PUT", "PATCH", "DELETE"})
+
+
+def _ui_ports(backend_port: int) -> Set[int]:
+    ports = {int(backend_port)}
+    try:
+        ports.add(int(os.environ.get("VITE_PORT", _DEFAULT_FRONTEND_PORT)))
+    except ValueError:
+        ports.add(_DEFAULT_FRONTEND_PORT)
+    return ports
+
+
+class WsAuth:
+    """Origin/Host allowlist plus the per-process session token."""
+
+    def __init__(self, backend_port: int, token: Optional[str] = None) -> None:
+        ports = _ui_ports(backend_port)
+        self.allowed_hosts: Set[str] = {
+            f"{h}:{p}" for h in _LOOPBACK_HOSTS for p in ports
+        }
+        self.allowed_origins: Set[str] = {
+            f"http://{hp}" for hp in self.allowed_hosts
+        }
+        self.token: str = token or secrets.token_hex(32)
+
+    # -- individual checks -------------------------------------------------
+
+    def host_ok(self, host: Optional[str]) -> bool:
+        return bool(host) and host.lower() in self.allowed_hosts
+
+    def origin_ok(self, origin: Optional[str]) -> bool:
+        return bool(origin) and origin.lower().rstrip("/") in self.allowed_origins
+
+    def token_ok(self, presented: Optional[str]) -> bool:
+        if not presented:
+            return False
+        return hmac.compare_digest(presented.encode(), self.token.encode())
+
+    @staticmethod
+    def token_from_protocols(header: Optional[str]) -> Optional[str]:
+        """Pull the token out of a Sec-WebSocket-Protocol header value."""
+        for proto in (header or "").split(","):
+            proto = proto.strip()
+            if proto.startswith(WS_TOKEN_PROTOCOL_PREFIX):
+                return proto[len(WS_TOKEN_PROTOCOL_PREFIX):]
+        return None
+
+    # -- request-level decisions ------------------------------------------
+
+    def check_ws_handshake(self, headers: Mapping[str, str]) -> Optional[str]:
+        """Return None if the handshake may proceed, else the rejection reason."""
+        if not self.host_ok(headers.get("Host")):
+            return "host"
+        if not self.origin_ok(headers.get("Origin")):
+            return "origin"
+        if not self.token_ok(
+            self.token_from_protocols(headers.get("Sec-WebSocket-Protocol"))
+        ):
+            return "token"
+        return None
+
+    def check_api_request(
+        self, method: str, headers: Mapping[str, str]
+    ) -> Optional[str]:
+        """Return None if an /api/* request may proceed, else the reason.
+
+        The same threat as /ws, over plain HTTP: a multipart or text/plain
+        POST is a CORS "simple" request, so any page can SEND one with no
+        preflight — the side effect happens even though the response is
+        unreadable. So: every request must be addressed by a loopback Host
+        (defeats DNS rebinding, which would also make responses readable),
+        and a write carrying a foreign Origin is refused. Browsers always
+        send Origin on a cross-origin write; no Origin means a non-browser
+        caller (the agent-app bridge, scripts), which CSRF cannot forge.
+        """
+        if not self.host_ok(headers.get("Host")):
+            return "host"
+        if method.upper() in _MUTATING:
+            origin = headers.get("Origin")
+            if origin is not None and not self.origin_ok(origin):
+                return "origin"
+        return None
+
+    def check_token_request(self, headers: Mapping[str, str]) -> Optional[str]:
+        """Return None if GET /api/session-token may be served, else the reason.
+
+        The main protection is that the response carries no CORS headers, so a
+        cross-origin page can't read it. These checks add defense in depth:
+        a rebinding Host, a foreign Origin, or a browser-labelled cross-site
+        fetch are refused outright.
+        """
+        if not self.host_ok(headers.get("Host")):
+            return "host"
+        origin = headers.get("Origin")
+        if origin is not None and not self.origin_ok(origin):
+            return "origin"
+        site = headers.get("Sec-Fetch-Site")
+        if site is not None and site not in ("same-origin", "none"):
+            return "fetch-site"
+        return None
+

app/ui_layer/browser/frontend/src/store/socket/SocketClient.ts

@@ -48,6 +48,7 @@ export class SocketClient {
   private readonly outboxTtlMs: number
   private readonly livenessIntervalMs: number
   private readonly livenessTimeoutMs: number
+  private readonly getProtocols?: () => Promise<string[]>
 
   private ws: WebSocket | null = null
   private connecting = false
@@ -78,6 +79,7 @@ export class SocketClient {
     this.outboxTtlMs = opts.outboxTtlMs ?? 60000
     this.livenessIntervalMs = opts.livenessIntervalMs ?? 5000
     this.livenessTimeoutMs = opts.livenessTimeoutMs ?? 3000
+    this.getProtocols = opts.getProtocols
 
     if (typeof window !== 'undefined') {
       window.addEventListener('online', () => this.reconnectNow())
@@ -108,12 +110,28 @@ export class SocketClient {
       this.ws = null
     }
 
+    if (!this.getProtocols) {
+      this.open([])
+      return
+    }
+    this.getProtocols().then(
+      protocols => this.open(protocols),
+      err => {
+        // Backend down or restarting: retry with the usual backoff.
+        console.warn('[SocketClient] could not get session token:', err)
+        this.connecting = false
+        this.scheduleReconnect()
+      },
+    )
+  }
+
+  private open(protocols: string[]): void {
     const attemptId = newClientId()
     const url = `${this.url}${this.url.includes('?') ? '&' : '?'}attempt=${attemptId}`
 
     let ws: WebSocket
     try {
-      ws = new WebSocket(url)
+      ws = new WebSocket(url, protocols)
     } catch (err) {
       console.error('[SocketClient] failed to construct WebSocket:', err)
       this.connecting = false

app/ui_layer/browser/frontend/src/store/socket/socketInstance.ts

@@ -1,5 +1,5 @@
 import { SocketClient } from './SocketClient'
-import { getWsUrl } from '../../utils/connection'
+import { getWsUrl, getWsProtocols } from '../../utils/connection'
 
 // Module-level singleton. Created lazily so tests / SSR-ish environments
 // don't pay for a WebSocket at import time. The middleware calls connect()
@@ -9,7 +9,7 @@ let instance: SocketClient | null = null
 
 export function getSocketClient(): SocketClient {
   if (!instance) {
-    instance = new SocketClient({ url: getWsUrl() })
+    instance = new SocketClient({ url: getWsUrl(), getProtocols: getWsProtocols })
   }
   return instance
 }

app/ui_layer/browser/frontend/src/store/socket/types.ts

@@ -27,4 +27,10 @@ export interface SocketClientOptions {
   livenessIntervalMs?: number
   /** How long a ping may go unanswered before the backend counts as busy. */
   livenessTimeoutMs?: number
+  /**
+   * Subprotocols for each connection attempt, resolved fresh every time (the
+   * backend's session token rotates on restart). A rejection counts as a
+   * failed attempt and is retried with backoff.
+   */
+  getProtocols?: () => Promise<string[]>
 }

app/ui_layer/browser/frontend/src/utils/connection.ts

@@ -19,6 +19,21 @@ export function getWsUrl(): string {
   return `${protocol}//${window.location.host}/ws`
 }
 
+/**
+ * WebSocket subprotocols carrying the backend's per-process session token.
+ *
+ * The token comes from a same-origin fetch (proxied to the backend in dev and
+ * static-server modes); the backend never sends CORS headers for it, so other
+ * sites can't read it. Fetched on every connect: a restarted backend has a
+ * new token.
+ */
+export async function getWsProtocols(): Promise<string[]> {
+  const resp = await fetch('/api/session-token', { cache: 'no-store' })
+  if (!resp.ok) throw new Error(`session token request failed: ${resp.status}`)
+  const { token } = (await resp.json()) as { token: string }
+  return ['craftbot', `craftbot-auth.${token}`]
+}
+
 /**
  * Resolve the base URL for API requests to the backend.
  */