Skip to content

fix(ws): take WebSocket identity from the session cookie - #607

Open
prince-shakyaa wants to merge 1 commit into
GenAI-Security-Project:mainfrom
prince-shakyaa:fix/ws-connect-session-auth
Open

prince-shakyaa wants to merge 1 commit into
GenAI-Security-Project:mainfrom
prince-shakyaa:fix/ws-connect-session-auth

Conversation

@prince-shakyaa

Copy link
Copy Markdown

Fixes #606

What was wrong

/ws/connect used whatever namespace and user_id the client put in the URL, and subscribe accepted any topic. Anyone with another player's user_id could open a socket as them and receive their activity feed and challenge-completion events, including the snippet of the prompt they used to solve the challenge.

What changed

  • finbot/core/websocket/routes.py: the handshake reads the finbot_session cookie and gets namespace/user_id from the session. The query params are gone. If there's no valid session, the socket is closed before it's accepted (1008), and clients see that as an HTTP 403 on the handshake. subscribe only accepts the connection's own activity:{ns}:{user_id} topic and sends an error event for anything else.
  • finbot/core/auth/session.py: added SessionManager.get_session_identity(). It checks that the session exists, isn't expired or too old, and has a valid signature, then returns (namespace, user_id).
    • I didn't reuse get_session() because it has side effects. If the session is due for rotation, it creates a new session ID and expires the old one in 60s, but a WebSocket handshake has no response to send the new cookie on, so the player would get logged out a minute later. It can also delete a temporary session on a fingerprint mismatch.
    • The new method only reads. It skips the fingerprint check: that check runs on every normal page load anyway, and the socket only opens from a page that just passed it.
  • finbot/static/js/vendor/ctf_sidecar.js: connects to /ws/connect with no params and drops the explicit subscribe, since the server already auto-subscribes the connection to its own topic.
    • Side effect: the sidecar on the admin, darklab and labs pages now gets live events. Those templates never set window.CTF_USER_ID, so before this they connected as anonymous.

Tests

New file tests/unit/auth/test_websocket_auth.py (9 tests):

  • no cookie, unknown cookie, expired session and tampered session are all rejected, even when victim IDs are passed in the URL
  • with a valid cookie, the connection is registered under the session's user and not the user in the query string
  • subscribing to your own activity topic works
  • subscribing to another user's topic returns an error and doesn't add the subscription
  • get_session_identity() doesn't rotate or modify a session that is due for rotation
  • get_session_identity() returns None for a missing or empty cookie

Against the old route, 7 of the 9 fail (the other 2 only test the new helper). All 9 pass with the fix.

The rest of the suite:

  • unit: 316 passed, 3 failed. The 3 failures are in tests/unit/agents/test_specialized_agents.py and fail the same way on main without this change.
  • integration: 6 passed

Manual testing

I ran the app with uvicorn and Redis and used a real WebSocket client (the websockets library). The cookie came from loading /ctf/, just like in a browser.

Check Result
connect with the session cookie connected; ping/pong works
connect with no cookie, victim IDs in the URL 403 on the handshake
connect with a garbage cookie 403 on the handshake
subscribe to another user's topic error event
load a page with the same cookie after the socket 200, no new cookie (session not rotated or logged out)
two players connected, event emitted for player A through the event bus A receives it through Redis → processor → socket; B receives nothing

/ws/connect trusted namespace and user_id from the query string, and
subscribe accepted any topic, so anyone could listen to another
player's live events. The handshake now resolves the user from the
session cookie (read-only, no rotation) and only allows subscribing
to the user's own activity topic.

Fixes GenAI-Security-Project#606
@prince-shakyaa

prince-shakyaa commented Oct 20, 2026 •

Copy link
Copy Markdown
Author

Hi @saikishu , @e2hln

This PR resolves #606 where /ws/connect trusted namespace and user_id from the URL and let any socket subscribe to any topic, so anyone could listen to another player's live events. Fixed it by reading the user from the session cookie on the handshake and only allowing a socket to subscribe to its own activity topic. Added unit tests in tests/unit/auth/test_websocket_auth.py and also tested it on a local server with two players.

Let me know if you need any changes.
Thank You.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] WebSocket endpoint skips auth, so anyone can read another player's live CTF events

1 participant