mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[F029] websocket: remove broken /api/permissions/check loopback from channel stream
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.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user