Repository navigation
Per-route WebSocket auth sweep with a route-level (not file-level) guard #14965
Description
Activity
Enumerated: 17 WebSocket routes that reach
accept()with no authenticationProduced during the review of PR #14989 (which fixes three of them: the VNC proxy,
the terminal session socket, and — after review — the SSH terminal socket #14991).
Recording the full list here because this issue is the sweep, and an enumeration
is the thing it needs.file:line route notes api/terminal.py:1071/ws/ssh/{host_id}live and reachable from chat — see #14991 api/overseer_handlers.py:435/ws/{session_id}runs commands api/advanced_control.py:572/ws/desktop/{session_id}api/advanced_control.py:530/ws/monitoringapi/logs.py:848/api/logs/tail/{filename}filename is caller-supplied api/process_management.py:172/processes/{process_id}/streamapi/presence_ws.py:24/ws/sessions/{session_id}/presenceidentity from an unverified ?user_id=— the docstring admits itapi/intelligent_agent.py:254/streamapi/voice_stream.py:316/streamapi/knowledge_research_ws.py:119/ws/knowledge/researchapi/long_running_operations.py:494/{operation_id}/progressapi/monitoring.py:1148/realtimeapi/analytics.py:675/ws/realtimeapi/analytics.py:1098/ws/analytics/liveapi/analytics_quality.py:1668/wsapi/startup.py:127/wsservices/workflow_automation/routes.py:596/workflow_ws/{session_id}Several carry an
enforce_ws_origincall. That is not authentication — its own
docstring inapi/ws_security.pysays authorisation is decided separately by each
endpoint, and it passes any client that sends noOriginheader at all, which is
every non-browser client.Why this list is the argument for #14963
Three of these were fixed one at a time, by hand, in one PR. The remaining 17 are
the same shape. Per-route attachment means the guard is opt-in, and the failure
mode is silent: a route that simply never calls it looks identical to one that
does not need it. That is howssh_terminal_websocketsat one route away from
terminal_websocketin the same file and was missed by the issue that named its
sibling.A router-level dependency (#14963) makes omission impossible rather than merely
noticed, which is why it should land before the remaining 17 are swept
individually — otherwise the sweep has to be repeated every time a route is added.Suggested acceptance criteria for this issue
- A test enumerates every
@router.websocketin the backend and asserts each
one is covered by authentication — route-level, not file-level, so a new
unguarded route in an already-guarded file still fails. - The guard derives its population by collecting routes from the app, not
by grepping decorators, so a route registered another way cannot hide. - If the population ever drops to zero the test FAILS loudly rather than
reporting a clean sweep — an empty result must never read as a clean result. - Any route that is legitimately public is listed in an explicit, commented
allowlist with the reason, not silently skipped.
Not in scope here
Two credential-layer issues found alongside, worth their own tracking:
authenticate_websocket(auth_middleware.py:1075-1099) accepts only a
?token=JWT — no cookie session, noAuthorizationheader, no internal key —
whileauthenticate_ws_adminin the same area accepts all four. fix(ws): task_workspace_ws — Origin gate requires Authorization header but admin-auth accepts cookies → same-origin cookie-admin lockout #11016 records
the same lockout being fixed forapi/task_workspace_ws.py.- Two competing rejection conventions coexist: close-
1008-before-accept(), and
accept-then-close-4001as pinned bytests/test_websockets_auth_reject_12366.py:73.
Browsers surface a pre-accept close as1006, so clients cannot distinguish
"not authorized" from "offline".
Related: #14958 (umbrella), #14959, #14960, #14961, #14991, #14963, PR #14989.
- A test enumerates every
Correction to my enumeration above — two of the 17 routes I listed are in fact guarded, and the list was missing others.
I re-derived it myself with an AST sweep over every
@router.websocketinautobot-backendandautobot-slm-backend, matching any callee whose name looks auth-related, rather than relaying a hand-built list.Wrongly listed as unguarded — both call
enforce_ws_adminbeforeaccept():api/advanced_control.py:572/ws/desktop/{session_id}api/advanced_control.py:530/ws/monitoring
Missing from my list:
services/workflow_automation/routes.py:597/workflow_ws/{session_id}— no guard at all, not even an origin checkutils/operation_timeout_integration.py:381/{operation_id}/progress— same
Corrected count on
Dev_new_gui: 18 unauthenticated routes, of which 16 have an origin check only and 2 have no guard at all. PR #14989 fixes three of them (terminal.py:903,terminal.py:1021,vnc_proxy.py:380), leaving 15.A methodology note that belongs in this issue's acceptance criteria
My first sweep returned 23 and included
api/task_workspace_ws.py:246/tasks/{task_id}/shell— a shell endpoint. That was a false positive: the route authenticates through_authenticate_ws_admin, a thin local wrapper aroundauthenticate_ws_admin, and my exact-name matching missed the leading underscore. I nearly reported a correctly-guarded shell as open.That is directly relevant to what this issue is asking for. A sweep keyed on names — of guards, of routes, of decorators — is wrong in both directions: it misses guards behind a local alias, and it misses routes registered by any path other than the literal decorator. Whatever guard lands here should:
- derive its population by collecting routes from the running app, not by grepping decorators;
- decide
guardedby whether the dependency actually runs, not by whether a name appears in the body; - fail loudly if the population is empty rather than reporting a clean sweep.
The last point is not hypothetical here. PR #14989 verified empirically that a router-level
Depends(check_admin_permission)does not run for WebSocket routes at all — FastAPI cannot resolve an HTTP-Request-typed dependency against a WebSocket scope. So a route can carry a guard in its signature, read as protected to any name-based check, and be completely unauthenticated at runtime. That is the strongest argument in this issue for verifying behaviour over appearance.Open-by-design entry for this guard's list (WebSocket auth triage, #17009):
/api/startup/ws(api/startup.py): read-only boot-progress broadcast. Inbound frames are read only to detect disconnect. It carries no user or tenant data. It stays open so a client can watch start-up before any credential exists.
Its writer,
POST /api/startup/phase, is unauthenticated and is tracked separately as #17012. That issue is about the write, not this read.
Part of #14958.
Problem
Two WebSocket routes were found unauthenticated by reading them (#14959,
#14960). A coarse file-level grep suggests there may be more, but that heuristic is
not trustworthy and the real state of the other sockets is unknown.
Evidence
A per-file heuristic — file contains
@router.websocket, and contains zero occurrences ofauthenticate_websocket|authenticate_ws_admin|enforce_ws_admin|get_current_user— flags:api/analytics_quality.pyapi/knowledge_research_ws.pyapi/logs.pyapi/long_running_operations.pyapi/monitoring.pyapi/presence_ws.pyapi/startup.pyapi/voice_stream.pyThese are candidates, not findings. The heuristic is unreliable in both directions:
api/vnc_proxy.pyas having 7 auth symbols andapi/terminal.pyas having 20, because their REST routes are authenticated — while theWebSocket route in each is not. A file-level count cannot see which route the symbols
attach to.
import, or a wrapper that this grep does not name.
So the list above is a starting set, not a verdict, and the sweep must not be run with the same
instrument that produced it.
Proposed approach
Enumerate every
@router.websocketroute in the backend and determine, per route, whetheran identity is established before
websocket.accept(). Read the routes; do not grep the files.For each route record: authenticated (how), intentionally public (why), or unauthenticated
(defect → fix under this umbrella, or its own issue if the fix is substantial).
Then make the property enforceable: a repository guard that enumerates WebSocket routes and
asserts each one either resolves an identity or appears on an explicit, justified public list.
The guard must assert on the route, not the file, or it reproduces exactly the blind spot
that hid these two defects.
Risks / constraints
auth helper and never calls it. Prefer asserting behaviour — drive each route with an
unauthenticated handshake and check the outcome.
a positive allowlist, never a default.
Acceptance criteria
@router.websocketroute inautobot-backend/is classified by reading it:authenticated / intentionally public / defect.
file:lineevidence.real routing surface, confirm the guard goes red, revert) — a guard never observed failing
is not known to work.
Implementation Order
Wave: 3
Depends on: #14963
Unblocks: none — terminal leaf