mirror of
https://github.com/ruvnet/RuView
synced 2026-07-28 18:21:42 +00:00
websocket.service.js (used by the pose/event streams) opened a bare `new WebSocket(url)` with no ADR-272 ticket exchange, unlike sensing.service.js which already mints a ticket per connect. Since a browser cannot set an Authorization header on a WebSocket upgrade, and the server rejects a long-lived bearer passed as a query string (CWE-598), the pose stream 401'd whenever auth was on — visible in the Live Demo tab as "Failed to create WebSocket connection". Fix: createWebSocketWithTimeout() now strips any `token` query param a caller put on the URL (pose.service.js does this) and exchanges the stored bearer for a single-use `?ticket=` via withWsTicket(), done at this one choke point so every consumer (pose, events, training) and every reconnect attempt gets a fresh ticket. Second, smaller bug: pose-fusion/js/main.js auto-connected to a hardcoded `ws://localhost:8765/ws/sensing`, but the Docker image serves the sensing WebSocket on :3001 (the same 3000->3001 mapping sensing.service.js already encodes) — the auto-connect dialed a port nothing listens on. Reuses that port mapping and tickets both the auto-connect and the manual "Connect" button. Bumped the pose-fusion.html cache-buster (v=13 -> v=14) so browsers actually fetch the updated main.js. Adds ui/services/websocket.service.test.mjs (4 executed Node tests, stubbed WebSocket/fetch/localStorage) covering: no-auth passthrough, ticket exchange + bearer-never-in-URL, stray ?token= stripping, and the pre-ADR-272 404 fallback. Wired into the CI "Run UI unit tests" step. Reported with a verified fix in #1461 by wsc7r4zcj4-collab; this PR implements the same fix against current main with an added regression test. Closes #1461
This commit is contained in:
@@ -204,7 +204,7 @@ jobs:
|
||||
node-version: '22'
|
||||
|
||||
- name: Run UI unit tests
|
||||
run: node --test ui/sw.test.mjs ui/services/ws-ticket.test.mjs
|
||||
run: node --test ui/sw.test.mjs ui/services/ws-ticket.test.mjs ui/services/websocket.service.test.mjs
|
||||
|
||||
# Unit and Integration Tests
|
||||
# Python pytest matrix — runs against the archived v1 Python tree.
|
||||
|
||||
+1
-1
@@ -196,6 +196,6 @@
|
||||
|
||||
</div><!-- /main-grid -->
|
||||
|
||||
<script type="module" src="pose-fusion/js/main.js?v=13"></script>
|
||||
<script type="module" src="pose-fusion/js/main.js?v=14"></script>
|
||||
</body>
|
||||
</html>
|
||||
|
||||
@@ -10,6 +10,7 @@ import { CnnEmbedder } from './cnn-embedder.js?v=13';
|
||||
import { FusionEngine } from './fusion-engine.js?v=13';
|
||||
import { PoseDecoder } from './pose-decoder.js?v=13';
|
||||
import { CanvasRenderer } from './canvas-renderer.js?v=13';
|
||||
import { withWsTicket } from '../../services/ws-ticket.js';
|
||||
|
||||
// === State ===
|
||||
let mode = 'dual'; // 'dual' | 'video' | 'csi'
|
||||
@@ -114,7 +115,9 @@ function init() {
|
||||
const url = wsUrlInput.value.trim();
|
||||
if (!url) return;
|
||||
connectWsBtn.textContent = 'Connecting...';
|
||||
const ok = await csiSimulator.connectLive(url);
|
||||
// ADR-272: exchange the stored bearer for a single-use ?ticket= before the
|
||||
// upgrade — a browser cannot set an Authorization header on a WebSocket.
|
||||
const ok = await csiSimulator.connectLive(await withWsTicket(url));
|
||||
connectWsBtn.textContent = ok ? '✓ Connected' : 'Connect';
|
||||
if (ok) {
|
||||
connectWsBtn.classList.add('active');
|
||||
@@ -136,10 +139,19 @@ function init() {
|
||||
});
|
||||
csiCnn.tryLoadWasm(wasmBase);
|
||||
|
||||
// Auto-connect to local sensing server WebSocket if available
|
||||
const defaultWsUrl = 'ws://localhost:8765/ws/sensing';
|
||||
// Auto-connect to local sensing server WebSocket if available.
|
||||
// Served from the Docker image the sensing stream lives on :3001 (same
|
||||
// port mapping sensing.service.js uses); the standalone dev server stays
|
||||
// on :8765.
|
||||
const wsPortMap = { '3000': '3001' };
|
||||
const mappedPort = wsPortMap[window.location.port];
|
||||
const defaultWsUrl = mappedPort
|
||||
? `ws://${window.location.hostname}:${mappedPort}/ws/sensing`
|
||||
: 'ws://localhost:8765/ws/sensing';
|
||||
if (wsUrlInput) wsUrlInput.value = defaultWsUrl;
|
||||
csiSimulator.connectLive(defaultWsUrl).then(ok => {
|
||||
// ADR-272: exchange the stored bearer for a single-use ?ticket= before the
|
||||
// upgrade — a browser cannot set an Authorization header on a WebSocket.
|
||||
withWsTicket(defaultWsUrl).then(u => csiSimulator.connectLive(u)).then(ok => {
|
||||
if (ok && connectWsBtn) {
|
||||
connectWsBtn.textContent = '✓ Live ESP32';
|
||||
connectWsBtn.classList.add('active');
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { API_CONFIG, buildWsUrl } from '../config/api.config.js';
|
||||
import { backendDetector } from '../utils/backend-detector.js';
|
||||
import { withWsTicket } from './ws-ticket.js';
|
||||
|
||||
export class WebSocketService {
|
||||
constructor() {
|
||||
@@ -115,8 +116,19 @@ export class WebSocketService {
|
||||
}
|
||||
|
||||
async createWebSocketWithTimeout(url) {
|
||||
// ADR-272: the server gates /ws/* and /api/v1/stream/* behind bearer auth,
|
||||
// and a browser cannot set an Authorization header on an upgrade request.
|
||||
// Exchange the stored bearer for a single-use ?ticket= here — immediately
|
||||
// before the socket opens, on every attempt — so reconnects each get a
|
||||
// fresh ticket. Also strip any long-lived `token` param a caller put in
|
||||
// the URL (e.g. pose.service.js): the bearer itself must never travel in
|
||||
// a query string.
|
||||
const urlObj = new URL(url);
|
||||
urlObj.searchParams.delete('token');
|
||||
const connectUrl = await withWsTicket(urlObj.toString());
|
||||
|
||||
return new Promise((resolve, reject) => {
|
||||
const ws = new WebSocket(url);
|
||||
const ws = new WebSocket(connectUrl);
|
||||
const timeout = setTimeout(() => {
|
||||
ws.close();
|
||||
reject(new Error(`Connection timeout after ${this.config.connectionTimeout}ms`));
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
// Executed regression test for issue #1461: the pose/event WebSocket path
|
||||
// (unlike sensing.service.js) opened a bare `new WebSocket(url)` with no
|
||||
// ADR-272 ticket exchange, and never stripped the long-lived bearer that
|
||||
// pose.service.js puts on the URL as `?token=`. Both meant the pose stream
|
||||
// 401'd whenever RUVIEW_API_TOKEN was set.
|
||||
//
|
||||
// Run: node --test ui/sw.test.mjs ui/services/ws-ticket.test.mjs ui/services/websocket.service.test.mjs
|
||||
//
|
||||
// This EXECUTES createWebSocketWithTimeout in Node with a stub `WebSocket`
|
||||
// class and stubbed `fetch`/`localStorage`, so it verifies the actual ticket
|
||||
// exchange + token stripping wiring, not just that the file parses.
|
||||
|
||||
import { test, beforeEach } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
|
||||
const STORAGE_KEY = 'ruview-api-token';
|
||||
|
||||
let stored = {};
|
||||
let fetchCalls = [];
|
||||
let fetchImpl = async () => ({ ok: true, status: 200, json: async () => ({ ticket: 'T' }) });
|
||||
let lastConstructedUrl = null;
|
||||
|
||||
globalThis.localStorage = {
|
||||
getItem: (k) => (k in stored ? stored[k] : null),
|
||||
setItem: (k, v) => { stored[k] = String(v); },
|
||||
removeItem: (k) => { delete stored[k]; },
|
||||
};
|
||||
globalThis.fetch = async (...args) => { fetchCalls.push(args); return fetchImpl(...args); };
|
||||
|
||||
// Minimal WebSocket stub: records the URL it was opened with and fires
|
||||
// `onopen` on the next microtask — by then createWebSocketWithTimeout's
|
||||
// Promise executor has already assigned `ws.onopen`, so this resolves the
|
||||
// real promise the way a successful upgrade would, instead of leaving the
|
||||
// 10s connection-timeout timer dangling for the whole test run.
|
||||
class FakeWebSocket {
|
||||
constructor(url) {
|
||||
lastConstructedUrl = url;
|
||||
this.url = url;
|
||||
this.readyState = 0; // CONNECTING
|
||||
queueMicrotask(() => { if (this.onopen) this.onopen(); });
|
||||
}
|
||||
close() {}
|
||||
}
|
||||
globalThis.WebSocket = FakeWebSocket;
|
||||
|
||||
const { WebSocketService } = await import('./websocket.service.js');
|
||||
|
||||
beforeEach(() => {
|
||||
stored = {};
|
||||
fetchCalls = [];
|
||||
fetchImpl = async () => ({ ok: true, status: 200, json: async () => ({ ticket: 'T' }) });
|
||||
lastConstructedUrl = null;
|
||||
});
|
||||
|
||||
test('with no stored bearer, the socket opens with the URL unchanged', async () => {
|
||||
const svc = new WebSocketService();
|
||||
await svc.createWebSocketWithTimeout('ws://host/api/v1/stream/pose?min_confidence=0.3');
|
||||
assert.equal(lastConstructedUrl, 'ws://host/api/v1/stream/pose?min_confidence=0.3');
|
||||
assert.equal(fetchCalls.length, 0, 'no ticket should be minted when auth is off');
|
||||
});
|
||||
|
||||
test('a stored bearer is exchanged for a single-use ticket before the socket opens', async () => {
|
||||
stored[STORAGE_KEY] = 'secret-bearer';
|
||||
const svc = new WebSocketService();
|
||||
await svc.createWebSocketWithTimeout('ws://host/api/v1/stream/pose?min_confidence=0.3');
|
||||
|
||||
const url = new URL(lastConstructedUrl);
|
||||
assert.equal(url.searchParams.get('ticket'), 'T');
|
||||
assert.equal(url.searchParams.get('min_confidence'), '0.3', 'other params must survive');
|
||||
assert.ok(!lastConstructedUrl.includes('secret-bearer'), `bearer leaked into URL: ${lastConstructedUrl}`);
|
||||
|
||||
const [path, init] = fetchCalls[0];
|
||||
assert.equal(path, '/api/v1/ws-ticket');
|
||||
assert.equal(init.headers.Authorization, 'Bearer secret-bearer');
|
||||
});
|
||||
|
||||
test('a stray ?token= from a caller (pose.service.js) is stripped, not forwarded', async () => {
|
||||
// Regression for #1461: pose.service.js puts the long-lived bearer on the
|
||||
// URL as `?token=`. That must never reach the actual WebSocket upgrade —
|
||||
// the ticket exchange above is the only credential that belongs in the URL.
|
||||
stored[STORAGE_KEY] = 'secret-bearer';
|
||||
const svc = new WebSocketService();
|
||||
await svc.createWebSocketWithTimeout('ws://host/api/v1/stream/pose?token=secret-bearer&max_fps=30');
|
||||
|
||||
const url = new URL(lastConstructedUrl);
|
||||
assert.equal(url.searchParams.get('token'), null, 'token param must be stripped');
|
||||
assert.equal(url.searchParams.get('ticket'), 'T', 'a real ticket must replace it');
|
||||
assert.equal(url.searchParams.get('max_fps'), '30', 'unrelated params must survive');
|
||||
assert.ok(!lastConstructedUrl.includes('secret-bearer'));
|
||||
});
|
||||
|
||||
test('with no ADR-272 endpoint on the server (404), the socket still opens without a ticket', async () => {
|
||||
stored[STORAGE_KEY] = 'secret-bearer';
|
||||
fetchImpl = async () => ({ ok: false, status: 404, json: async () => ({}) });
|
||||
const svc = new WebSocketService();
|
||||
await svc.createWebSocketWithTimeout('ws://host/api/v1/stream/pose');
|
||||
assert.equal(lastConstructedUrl, 'ws://host/api/v1/stream/pose');
|
||||
});
|
||||
Reference in New Issue
Block a user