fix: stop sending websocket auth key in URL

[DeoJin]

Summary

• stop appending the long-lived dashboard access key to the browser WebSocket URL
• rely on the existing session cookie for browser WebSocket auth instead
• update the WebSocket auth test to validate header-based programmatic access instead of query-string auth

Validation

• npx jest --runInBand --config jest.ws.config.cjs

Fixes #29

[greptile-apps]

Greptile Summary

This PR addresses a security concern (issue #29) by removing the long-lived access key from browser WebSocket upgrade URLs, where it was visible in browser history, server access logs, and network proxies. The fix is sound: because authMiddleware runs before express.static, every user who loads dashboard.js has already been issued a session cookie, so the browser WebSocket connection can rely on that cookie exclusively without placing credentials in the URL.

Key changes:

• public/js/dashboard.js: connectWS now opens a plain WebSocket URL instead of appending the access key as a query string, relying on the session cookie the browser sends automatically on the upgrade request.
• tests/websocket.test.js: the "correct query-string" test is replaced with a "correct auth header" test, better representing the programmatic API access path.

One area worth addressing: src/websocket.js (not modified in this PR) still accepts the access key via query string for backward compatibility, but that code path is now entirely untested. The team should decide whether to keep it with a regression test, or remove it to complete the URL-exposure fix end-to-end.

Confidence Score: 4/5

• Safe to merge — the core fix is correct and the authentication logic is sound; one test coverage gap remains.
• The browser WebSocket change is logically correct because the HTTP auth middleware always establishes a session cookie before dashboard.js is served, so the cookie-only WebSocket approach is safe. The test suite was appropriately updated for the new flow. Score is 4 rather than 5 because the server-side query-string auth path in websocket.js — which still exists for backward compatibility — lost all test coverage in this PR, leaving a potential regression blind spot.
• No files in the diff require special attention, but the unmodified src/websocket.js should be reviewed to decide whether the query-string auth branch should be removed or re-tested.

Important Files Changed

Filename	Overview
public/js/dashboard.js	Removes the access key from the browser WebSocket URL; relies on session cookie sent automatically by browser. Change is safe because authMiddleware runs before express.static, so any user who executes this file already holds a valid session cookie.
tests/websocket.test.js	Test updated to cover header-based programmatic auth and session-cookie auth. However, removing the query-string auth test leaves the still-supported server-side query-string path in websocket.js completely uncovered.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant HTTP as Express (authMiddleware)
    participant Static as express.static
    participant WS as WebSocket Server

    Note over Browser,WS: Initial page load
    Browser->>HTTP: GET / (unauthenticated)
    HTTP-->>Browser: 302 redirect and sets session cookie
    Browser->>HTTP: GET / (with session cookie)
    HTTP->>Static: next() — authenticated
    Static-->>Browser: dashboard.js

    Note over Browser,WS: WebSocket — BEFORE this PR
    Browser->>WS: Upgrade request with access key in URL query string
    WS-->>Browser: 101 Switching Protocols

    Note over Browser,WS: WebSocket — AFTER this PR
    Browser->>WS: Upgrade request (session cookie sent automatically)
    WS-->>Browser: 101 Switching Protocols

    Note over Browser,WS: Programmatic access (unchanged)
    Browser->>WS: Upgrade request with auth header
    WS-->>Browser: 101 Switching Protocols

Loading

Comments Outside Diff (1)

1. tests/websocket.test.js, line 69-80 (link)

   Query-string WebSocket auth path goes untested

   The previous test validated query-string based WebSocket auth. Its replacement correctly covers header-based programmatic access, but src/websocket.js line 11 still accepts the key via url.searchParams.get('key') for backward compatibility — a branch that is now completely untested.

   There are two reasonable paths forward:

   • Preserve backward compat: add a test confirming that a WebSocket connection authenticated via query string still succeeds, so regressions in that server path are caught.
   • Remove query-string auth from the server too: drop the url.searchParams.get('key') call from websocket.js and add a test confirming query-string auth is now rejected, which would complete the security fix end-to-end.

   Without either change, a bug introduced in that server-side branch would go undetected by the test suite.

_{Last reviewed commit: 02ca5dc}