Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 5 additions & 19 deletions src/brew/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/brew/terminal/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 28 additions & 21 deletions src/brew/terminal/router.py
Original file line number Diff line number Diff line change
@@ -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)
Loading