From c43c1b057fc02aacc0764f482637894c477031da Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 28 Jun 2026 17:52:14 +0200 Subject: [PATCH] [F029] websocket: remove broken /api/permissions/check loopback from channel stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit channel_stream called validate_channel_access, which HTTP-loopbacked to GET /api/permissions/check — a route that does not exist. Every call 404'd -> False -> the channel stream closed with WS_1008_POLICY_VIOLATION for EVERY client, so the real-time channel stream was dead. Removed the function, its call site, and the now-unused httpx + settings imports. Post-F004 the panel-token gate is the channel-stream authorization (the CEO panel is the sole WS client and may view every channel), so the broken loopback is removed rather than replaced with an in-process check the CEO always passes. The legitimate enforcement.validate_channel_access (slugs, in-process static ACL) is a different function and is untouched. F027 is resolved-by-F004 (no code change): all three per-agent streams gate on _require_panel_token first, so only the authorized CEO panel can connect — 'any viewer subscribes to any target' is closed. TDD; ruff/mypy clean; 530 unit/api+enforcement+RBAC tests green. --- roboco/api/websocket.py | 34 ------------ tests/unit/api/test_websocket_auth.py | 53 ++++++++++++++++++- .../api/test_websocket_handler_cleanup.py | 3 -- tests/unit/api/test_websocket_idle_timeout.py | 3 -- 4 files changed, 52 insertions(+), 41 deletions(-) diff --git a/roboco/api/websocket.py b/roboco/api/websocket.py index 9a68fd62..6efacec6 100644 --- a/roboco/api/websocket.py +++ b/roboco/api/websocket.py @@ -18,13 +18,11 @@ from datetime import UTC, datetime from typing import Any from uuid import UUID -import httpx import structlog from fastapi import APIRouter, WebSocket, WebSocketDisconnect, status from roboco.agents_config import CEO_AGENT_ID, verify_agent_token from roboco.api.deps import _auth_required -from roboco.config import settings from roboco.db.base import get_db from roboco.services.repositories import resolve_agent_uuid @@ -359,32 +357,6 @@ async def validate_agent_exists(agent_id: UUID | str) -> bool: return False -async def validate_channel_access(channel_id: UUID, agent_id: UUID) -> bool: - """ - Validate that an agent has access to a channel. - - Calls the permissions API to check read access. - """ - try: - url = f"http://{settings.host}:{settings.port}/api/permissions/check" - async with httpx.AsyncClient(timeout=10.0) as client: - response = await client.get( - url, - params={ - "agent_id": str(agent_id), - "channel_id": str(channel_id), - "action": "read", - }, - ) - if response.status_code == status.HTTP_200_OK: - data = response.json() - return bool(data.get("allowed", False)) - return False - except Exception: - # On error, deny access (fail closed) - return False - - # ============================================================================= # WebSocket Routes # ============================================================================= @@ -416,12 +388,6 @@ async def channel_stream( await websocket.close(code=status.WS_1008_POLICY_VIOLATION) return - # Validate agent access to channel - has_access = await validate_channel_access(channel_id, agent_id) - if not has_access: - await websocket.close(code=status.WS_1008_POLICY_VIOLATION) - return - await manager.connect_channel(websocket, channel_id, agent_id) try: diff --git a/tests/unit/api/test_websocket_auth.py b/tests/unit/api/test_websocket_auth.py index 63b208e5..428dcea9 100644 --- a/tests/unit/api/test_websocket_auth.py +++ b/tests/unit/api/test_websocket_auth.py @@ -19,7 +19,12 @@ from uuid import uuid4 import pytest from fastapi import WebSocketDisconnect, status from roboco.agents_config import CEO_AGENT_ID, issue_agent_token -from roboco.api.websocket import agent_stream, notification_stream +from roboco.api.websocket import ( + ConnectionManager, + agent_stream, + channel_stream, + notification_stream, +) if TYPE_CHECKING: import pytest as _pytest # noqa: F401 @@ -119,3 +124,49 @@ async def test_agent_stream_rejects_missing_token_when_required( ws.close.assert_awaited_once() assert ws.close.await_args.kwargs["code"] == status.WS_1008_POLICY_VIOLATION ws.accept.assert_not_awaited() + + +# --------------------------------------------------------------------------- +# The channel stream must be usable by a panel-token holder. It previously +# called validate_channel_access, which HTTP-loopbacked to a non-existent +# /api/permissions/check endpoint — every connection 404'd → False → the stream +# closed with WS_1008_POLICY_VIOLATION for every client (the channel live-stream +# was dead). Post-F004 the panel-token gate IS the channel-stream authorization +# (the CEO panel is the sole WS client and may view every channel), so the +# broken loopback check is removed rather than replaced with theater. +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_channel_stream_accepts_panel_token_holder( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A panel-token holder supplying an agent_id query param is accepted and + registered on the channel stream — not fail-closed by a dead permission + check that 404s against a non-existent endpoint.""" + monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", _SECRET) + monkeypatch.setenv("ROBOCO_AGENT_AUTH_REQUIRED", "true") + token = issue_agent_token(CEO_AGENT_ID, "ceo", "") + channel_id = uuid4() + viewer_id = uuid4() + mgr = ConnectionManager() + ws = _mock_ws( + headers={"x-agent-token": token}, + query={"agent_id": str(viewer_id)}, + ) + monkeypatch.setattr("roboco.api.websocket.manager", mgr) + + await channel_stream(ws, channel_id) + + ws.accept.assert_awaited_once() + # Not fail-closed by a dead permission check. + ws.close.assert_not_awaited() + # The "connected" confirmation is sent immediately after connect_channel + # registers the socket, and its subscriber_count proves the socket was in + # the channel's subscription set at confirmation time (the mock then raises + # WebSocketDisconnect so the finally disconnects it — the normal clean + # exit, not a fail-close). + confirmation = ws.send_json.await_args.args[0] + assert confirmation["type"] == "connected" + assert confirmation["channel_id"] == str(channel_id) + assert confirmation["subscriber_count"] == 1 diff --git a/tests/unit/api/test_websocket_handler_cleanup.py b/tests/unit/api/test_websocket_handler_cleanup.py index 8ca4c8d7..74bf2f59 100644 --- a/tests/unit/api/test_websocket_handler_cleanup.py +++ b/tests/unit/api/test_websocket_handler_cleanup.py @@ -127,9 +127,6 @@ async def test_channel_stream_disconnects_on_non_disconnect_exception( mgr = ConnectionManager() ws = _mock_ws_for_receive(RuntimeError("anyio closed")) ws.query_params = {"agent_id": str(agent_id)} - monkeypatch.setattr( - "roboco.api.websocket.validate_channel_access", AsyncMock(return_value=True) - ) monkeypatch.setattr("roboco.api.websocket.manager", mgr) with pytest.raises(RuntimeError): diff --git a/tests/unit/api/test_websocket_idle_timeout.py b/tests/unit/api/test_websocket_idle_timeout.py index f0e25319..3c96a443 100644 --- a/tests/unit/api/test_websocket_idle_timeout.py +++ b/tests/unit/api/test_websocket_idle_timeout.py @@ -126,9 +126,6 @@ async def test_channel_stream_reaps_silent_socket_after_idle_timeout( hang_future: asyncio.Future[str] = asyncio.Future() ws = _mock_ws_for_receive(hang_future) ws.query_params = {"agent_id": str(agent_id)} - monkeypatch.setattr( - "roboco.api.websocket.validate_channel_access", AsyncMock(return_value=True) - ) monkeypatch.setattr("roboco.api.websocket.manager", mgr) monkeypatch.setattr("roboco.api.websocket.IDLE_TIMEOUT_SECONDS", 0.05)