diff --git a/src/brew/main.py b/src/brew/main.py index 6732dc3..b661b61 100644 --- a/src/brew/main.py +++ b/src/brew/main.py @@ -5,7 +5,7 @@ from collections.abc import AsyncGenerator from contextlib import asynccontextmanager, suppress -from fastapi import Depends, FastAPI, HTTPException, Request +from fastapi import Depends, FastAPI, Request from fastapi.responses import JSONResponse from fastapi.staticfiles import StaticFiles @@ -208,25 +208,11 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None]: # os.getenv (not Settings) because mount must happen at module level, before lifespan. # Settings requires fellow_email/password which aren't available at import time in tests. _mcp_enabled = os.getenv("FELLOW_MCP_ENABLED", "false").lower() == "true" -_chat_enabled = os.getenv("FELLOW_CHAT_ENABLED", "false").lower() == "true" - -def _require_terminal_enabled() -> None: - """404 the terminal endpoint when chat/MCP isn't wired. - - Read at request time so tests can monkeypatch the flags after import. - """ - if not (_chat_enabled and _mcp_enabled): - raise HTTPException(status_code=404) - - -app.include_router( - terminal_router, - prefix="/api", - # require_api_key is handled inline in the WS handler — FastAPI's WS dep - # resolver doesn't inject Header/Query the same way HTTP does. - dependencies=[Depends(_require_terminal_enabled)], -) +# NO `dependencies=[...]` here — FastAPI silently 403s WebSocket upgrades when +# deps propagate through include_router (regardless of whether the dep raises). +# Auth + the FELLOW_CHAT_ENABLED gate are enforced inline in the WS handler. +app.include_router(terminal_router, prefix="/api") if _mcp_enabled: from fastmcp import FastMCP as _FastMCP diff --git a/src/brew/terminal/process.py b/src/brew/terminal/process.py index fb51d58..2f62fec 100644 --- a/src/brew/terminal/process.py +++ b/src/brew/terminal/process.py @@ -44,6 +44,10 @@ def __init__( async def start(self) -> None: pid, fd = pty.fork() if pid == 0: + # tmux refuses to attach with TERM=dumb / unset ("open terminal + # failed: terminal does not support clear"). xterm-256color is + # what xterm.js emulates and ships in ncurses-base. + os.environ["TERM"] = "xterm-256color" os.execvp(self._argv[0], self._argv) # noqa: S606 PTY child exec self._pid = pid self._fd = fd diff --git a/src/brew/terminal/router.py b/src/brew/terminal/router.py index 018d3ec..8c5e803 100644 --- a/src/brew/terminal/router.py +++ b/src/brew/terminal/router.py @@ -1,41 +1,48 @@ -"""WebSocket endpoint at /api/terminal/ws.""" +"""WebSocket endpoint at /api/terminal/ws. + +Auth and feature-gating are enforced INLINE rather than via +include_router(dependencies=[...]) — FastAPI silently 403s WS upgrades +when deps are propagated through include_router, regardless of whether +the deps actually raise. See PR commit history for the investigation. +""" from __future__ import annotations -from typing import TYPE_CHECKING, Annotated +import os -from fastapi import APIRouter, Depends, WebSocket, status +from fastapi import APIRouter, WebSocket, status from brew.dependencies import get_settings from brew.terminal.dependencies import get_terminal_service -if TYPE_CHECKING: - from brew.config import Settings - from brew.terminal.service import TerminalService - router = APIRouter() +def _terminal_enabled() -> bool: + chat = os.getenv("FELLOW_CHAT_ENABLED", "false").lower() == "true" + mcp = os.getenv("FELLOW_MCP_ENABLED", "false").lower() == "true" + return chat and mcp + + def _extract_api_key(ws: WebSocket) -> str | None: return ws.headers.get("x-api-key") or ws.query_params.get("api_key") +def _api_key_valid(ws: WebSocket) -> bool: + expected = get_settings().api_key + if expected is None: + return True + provided = _extract_api_key(ws) + return provided is not None and provided == expected.get_secret_value() + + @router.websocket("/terminal/ws") -async def terminal_ws( - ws: WebSocket, - service: Annotated[TerminalService, Depends(get_terminal_service)], - settings: Annotated[Settings, Depends(get_settings)], -) -> None: - # Auth inline: include_router(dependencies=[...]) propagates deps to WS - # routes, but FastAPI's WS dependency resolver doesn't inject Header/Query - # parameters the same way HTTP does, so the shared `require_api_key` dep - # can't read the credential. Read directly from the WS scope instead. - if settings.api_key is not None: - provided = _extract_api_key(ws) - if provided is None or provided != settings.api_key.get_secret_value(): - await ws.close(code=status.WS_1008_POLICY_VIOLATION) - return +async def terminal_ws(ws: WebSocket) -> None: + if not _terminal_enabled() or not _api_key_valid(ws): + await ws.close(code=status.WS_1008_POLICY_VIOLATION) + return + service = get_terminal_service() await ws.accept() async with service.attached() as session: await session.run(ws)