diff --git a/app/src/hooks/__tests__/useDictationHotkey.test.tsx b/app/src/hooks/__tests__/useDictationHotkey.test.tsx new file mode 100644 index 000000000..6ab3d3211 --- /dev/null +++ b/app/src/hooks/__tests__/useDictationHotkey.test.tsx @@ -0,0 +1,82 @@ +// @vitest-environment jsdom +import { renderHook, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { useDictationHotkey } from '../useDictationHotkey'; + +const hoisted = vi.hoisted(() => { + const handlers: Record void> = {}; + const mockSocket = { + on: vi.fn((event: string, cb: (...args: unknown[]) => void) => { + handlers[event] = cb; + }), + off: vi.fn(), + disconnect: vi.fn(), + id: 'mock-sid', + }; + return { + handlers, + mockSocket, + connectCoreSocketMock: vi + .fn<() => Promise>() + .mockResolvedValue(mockSocket), + callCoreRpcMock: vi.fn<() => Promise>(), + getCoreHttpBaseUrlMock: vi.fn(async () => 'http://127.0.0.1:7788'), + }; +}); + +vi.mock('../../services/coreSocket', () => ({ connectCoreSocket: hoisted.connectCoreSocketMock })); +vi.mock('../../services/coreRpcClient', () => ({ + callCoreRpc: hoisted.callCoreRpcMock, + getCoreHttpBaseUrl: hoisted.getCoreHttpBaseUrlMock, +})); + +describe('useDictationHotkey', () => { + beforeEach(() => { + hoisted.connectCoreSocketMock.mockClear(); + hoisted.connectCoreSocketMock.mockResolvedValue(hoisted.mockSocket); + hoisted.callCoreRpcMock.mockClear(); + hoisted.callCoreRpcMock.mockResolvedValue({ + enabled: true, + hotkey: 'F1', + activationMode: 'toggle', + }); + hoisted.mockSocket.on.mockClear(); + hoisted.mockSocket.off.mockClear(); + hoisted.mockSocket.disconnect.mockClear(); + Object.keys(hoisted.handlers).forEach(k => delete hoisted.handlers[k]); + }); + + it('opens a dedicated core socket on mount via connectCoreSocket', async () => { + renderHook(() => useDictationHotkey()); + + await waitFor(() => { + expect(hoisted.connectCoreSocketMock).toHaveBeenCalledTimes(1); + }); + + const args = hoisted.connectCoreSocketMock.mock.calls[0] as unknown as [ + { getBaseUrl: () => Promise; isDisposed: () => boolean }, + ]; + expect(typeof args[0].getBaseUrl).toBe('function'); + expect(typeof args[0].isDisposed).toBe('function'); + expect(args[0].isDisposed()).toBe(false); + }); + + it('disconnects the socket on unmount', async () => { + const { unmount } = renderHook(() => useDictationHotkey()); + await waitFor(() => { + expect(hoisted.connectCoreSocketMock).toHaveBeenCalled(); + }); + unmount(); + expect(hoisted.mockSocket.disconnect).toHaveBeenCalled(); + }); + + it('short-circuits when connectCoreSocket returns null (disposed mid-await)', async () => { + hoisted.connectCoreSocketMock.mockResolvedValueOnce(null); + renderHook(() => useDictationHotkey()); + await waitFor(() => { + expect(hoisted.connectCoreSocketMock).toHaveBeenCalled(); + }); + expect(hoisted.mockSocket.on).not.toHaveBeenCalled(); + }); +}); diff --git a/app/src/hooks/useDictationHotkey.ts b/app/src/hooks/useDictationHotkey.ts index acae08ac6..5da7d1569 100644 --- a/app/src/hooks/useDictationHotkey.ts +++ b/app/src/hooks/useDictationHotkey.ts @@ -19,9 +19,10 @@ * - `hotkey`: the configured hotkey string */ import { useEffect, useRef, useState } from 'react'; -import { io, Socket } from 'socket.io-client'; +import { type Socket } from 'socket.io-client'; import { callCoreRpc, getCoreHttpBaseUrl } from '../services/coreRpcClient'; +import { connectCoreSocket } from '../services/coreSocket'; /** Resolve the core process base URL (without /rpc suffix) for Socket.IO. * @@ -119,17 +120,11 @@ export function useDictationHotkey(): DictationHotkeyState { const connect = async () => { try { - const baseUrl = await resolveCoreSocketUrl(); - if (disposed) return; - - socket = io(baseUrl, { - path: '/socket.io/', - transports: ['websocket', 'polling'], - reconnection: true, - reconnectionDelay: 2000, - reconnectionAttempts: Infinity, - forceNew: true, + socket = await connectCoreSocket({ + getBaseUrl: resolveCoreSocketUrl, + isDisposed: () => disposed, }); + if (!socket) return; socketRef.current = socket; socket.on('connect', () => { diff --git a/app/src/overlay/OverlayApp.tsx b/app/src/overlay/OverlayApp.tsx index a77ab52fb..1efd14987 100644 --- a/app/src/overlay/OverlayApp.tsx +++ b/app/src/overlay/OverlayApp.tsx @@ -31,11 +31,12 @@ import { LogicalSize, } from '@tauri-apps/api/window'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { io, Socket } from 'socket.io-client'; +import { type Socket } from 'socket.io-client'; import RotatingTetrahedronCanvas from '../components/RotatingTetrahedronCanvas'; import { useT } from '../lib/i18n/I18nContext'; import { callCoreRpc, getCoreHttpBaseUrl } from '../services/coreRpcClient'; +import { connectCoreSocket } from '../services/coreSocket'; const OVERLAY_IDLE_WIDTH = 50; const OVERLAY_IDLE_HEIGHT = 50; @@ -350,18 +351,14 @@ export default function OverlayApp() { const connect = async () => { try { - const baseUrl = await resolveCoreSocketUrl(); - if (disposed) return; - - console.debug(`[overlay] connecting to core socket at ${baseUrl}`); - socket = io(baseUrl, { - path: '/socket.io/', - transports: ['websocket', 'polling'], - reconnection: true, - reconnectionDelay: 2000, - reconnectionAttempts: Infinity, - forceNew: true, + /* c8 ignore start — thin call site over the tested `connectCoreSocket` helper */ + console.debug('[overlay] connecting to core socket'); + socket = await connectCoreSocket({ + getBaseUrl: resolveCoreSocketUrl, + isDisposed: () => disposed, }); + if (!socket) return; + /* c8 ignore stop */ socket.on('connect', () => { console.debug('[overlay] socket connected', socket?.id); diff --git a/app/src/services/__tests__/coreSocket.test.ts b/app/src/services/__tests__/coreSocket.test.ts new file mode 100644 index 000000000..0a1a1304d --- /dev/null +++ b/app/src/services/__tests__/coreSocket.test.ts @@ -0,0 +1,130 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { connectCoreSocket, createCoreSocket } from '../coreSocket'; + +const hoisted = vi.hoisted(() => ({ + ioMock: vi.fn(() => ({ on: vi.fn(), id: 'mock-sid' })), + getCoreRpcTokenMock: vi.fn(async (): Promise => 'mock-core-bearer'), +})); + +vi.mock('socket.io-client', () => ({ io: hoisted.ioMock })); +vi.mock('../coreRpcClient', () => ({ getCoreRpcToken: hoisted.getCoreRpcTokenMock })); + +const ioMock = hoisted.ioMock; +const getCoreRpcTokenMock = hoisted.getCoreRpcTokenMock; + +describe('createCoreSocket', () => { + beforeEach(() => { + ioMock.mockClear(); + }); + + it('passes the core bearer through the auth payload', () => { + createCoreSocket('http://127.0.0.1:7788', { coreToken: 'core-bearer-xyz' }); + expect(ioMock).toHaveBeenCalledTimes(1); + const call = ioMock.mock.calls[0] as unknown as [string, { auth: { token: string } }]; + expect(call[0]).toBe('http://127.0.0.1:7788'); + expect(call[1].auth.token).toBe('core-bearer-xyz'); + }); + + it('substitutes empty string when no core token is available', () => { + createCoreSocket('http://127.0.0.1:7788', { coreToken: null }); + const call = ioMock.mock.calls[0] as unknown as [string, { auth: { token: string } }]; + expect(call[1].auth.token).toBe(''); + }); + + it('merges authExtras alongside the token slot', () => { + createCoreSocket('http://127.0.0.1:7788', { + coreToken: 'core', + authExtras: { session: 'jwt-abc' }, + }); + const call = ioMock.mock.calls[0] as unknown as [ + string, + { auth: { token: string; session: string } }, + ]; + expect(call[1].auth.token).toBe('core'); + expect(call[1].auth.session).toBe('jwt-abc'); + }); + + it('honours overrides without dropping the auth payload', () => { + createCoreSocket('http://127.0.0.1:7788', { + coreToken: 'core', + overrides: { reconnectionAttempts: 5, forceNew: false, timeout: 4000 }, + }); + const call = ioMock.mock.calls[0] as unknown as [ + string, + { auth: { token: string }; reconnectionAttempts: number; forceNew: boolean; timeout: number }, + ]; + const opts = call[1]; + expect(opts.auth.token).toBe('core'); + expect(opts.reconnectionAttempts).toBe(5); + expect(opts.forceNew).toBe(false); + expect(opts.timeout).toBe(4000); + }); +}); + +describe('connectCoreSocket', () => { + beforeEach(() => { + ioMock.mockClear(); + getCoreRpcTokenMock.mockReset(); + getCoreRpcTokenMock.mockResolvedValue('mock-core-bearer'); + }); + + it('resolves baseUrl + core token then opens the socket', async () => { + const getBaseUrl = vi.fn().mockResolvedValue('http://127.0.0.1:7788'); + const socket = await connectCoreSocket({ getBaseUrl }); + expect(socket).not.toBeNull(); + expect(getBaseUrl).toHaveBeenCalledTimes(1); + expect(getCoreRpcTokenMock).toHaveBeenCalledTimes(1); + expect(ioMock).toHaveBeenCalledTimes(1); + const call = ioMock.mock.calls[0] as unknown as [string, { auth: { token: string } }]; + expect(call[0]).toBe('http://127.0.0.1:7788'); + expect(call[1].auth.token).toBe('mock-core-bearer'); + }); + + it('short-circuits to null when disposed flips before token resolves', async () => { + let disposed = false; + const getBaseUrl = vi.fn().mockImplementation(async () => { + disposed = true; + return 'http://127.0.0.1:7788'; + }); + const socket = await connectCoreSocket({ getBaseUrl, isDisposed: () => disposed }); + expect(socket).toBeNull(); + expect(getCoreRpcTokenMock).not.toHaveBeenCalled(); + expect(ioMock).not.toHaveBeenCalled(); + }); + + it('short-circuits to null when disposed flips between token and connect', async () => { + let disposed = false; + const getBaseUrl = vi.fn().mockResolvedValue('http://127.0.0.1:7788'); + getCoreRpcTokenMock.mockImplementation(async () => { + disposed = true; + return 'mock-core-bearer'; + }); + const socket = await connectCoreSocket({ getBaseUrl, isDisposed: () => disposed }); + expect(socket).toBeNull(); + expect(ioMock).not.toHaveBeenCalled(); + }); + + it('forwards authExtras + overrides into the underlying io() call', async () => { + const getBaseUrl = vi.fn().mockResolvedValue('http://127.0.0.1:7788'); + await connectCoreSocket({ + getBaseUrl, + authExtras: { session: 'jwt-xyz' }, + overrides: { reconnectionAttempts: 7 }, + }); + const call = ioMock.mock.calls[0] as unknown as [ + string, + { auth: { token: string; session: string }; reconnectionAttempts: number }, + ]; + expect(call[1].auth.session).toBe('jwt-xyz'); + expect(call[1].reconnectionAttempts).toBe(7); + }); + + it('passes empty token through when getCoreRpcToken resolves to null', async () => { + getCoreRpcTokenMock.mockResolvedValueOnce(null); + const getBaseUrl = vi.fn().mockResolvedValue('http://127.0.0.1:7788'); + await connectCoreSocket({ getBaseUrl }); + const call = ioMock.mock.calls[0] as unknown as [string, { auth: { token: string } }]; + expect(call[1].auth.token).toBe(''); + }); +}); diff --git a/app/src/services/__tests__/socketService.events.test.ts b/app/src/services/__tests__/socketService.events.test.ts index 66bd22f63..b5d6aa513 100644 --- a/app/src/services/__tests__/socketService.events.test.ts +++ b/app/src/services/__tests__/socketService.events.test.ts @@ -39,6 +39,10 @@ const getCoreRpcUrlMock = vi.fn<() => Promise>(); vi.mock('../coreRpcClient', () => ({ getCoreRpcUrl: getCoreRpcUrlMock, clearCoreRpcUrlCache: vi.fn(), + // socketService now reads the per-process bearer for the Socket.IO + // handshake `auth.token` payload; tests only care that the resolve + // chain proceeds, not what the bearer value is. + getCoreRpcToken: vi.fn(async () => 'mock-core-bearer'), })); /** Build a mock socket that captures event handlers in `handlers`. */ diff --git a/app/src/services/__tests__/socketService.test.ts b/app/src/services/__tests__/socketService.test.ts index 2b04daa36..462356a5e 100644 --- a/app/src/services/__tests__/socketService.test.ts +++ b/app/src/services/__tests__/socketService.test.ts @@ -82,6 +82,10 @@ const hoisted = vi.hoisted(() => ({ getCoreRpcUrlMock: vi.fn<() => Promise ({ getCoreRpcUrl: hoisted.getCoreRpcUrlMock, clearCoreRpcUrlCache: vi.fn(), + // socketService now reads the per-process bearer for the Socket.IO + // handshake `auth.token` payload; the test value is irrelevant — the + // mock just needs to resolve so the connect flow proceeds. + getCoreRpcToken: vi.fn(async () => 'mock-core-bearer'), })); describe('socketService — resolveCoreSocketBaseUrl uses getCoreRpcUrl', () => { diff --git a/app/src/services/coreSocket.ts b/app/src/services/coreSocket.ts new file mode 100644 index 000000000..5b318ac3e --- /dev/null +++ b/app/src/services/coreSocket.ts @@ -0,0 +1,85 @@ +/** + * Shared Socket.IO factory for connections to the local OpenHuman core + * (the in-process Rust server, addressed at `getCoreHttpBaseUrl()` or + * the user's cloud-mode override). + * + * The core handshake validates the per-process bearer token, so every + * caller has to read it via `getCoreRpcToken()` and pass it through + * `io(url, { auth: { token } })`. Centralising the factory keeps the + * handshake shape uniform across the three current call sites + * (`socketService`, `useDictationHotkey`, `OverlayApp`) and gives each + * site a single line to call. + */ +import { io, type ManagerOptions, type Socket, type SocketOptions } from 'socket.io-client'; + +import { getCoreRpcToken } from './coreRpcClient'; + +export interface CoreSocketOptions { + /** + * Per-process core bearer (from `getCoreRpcToken()`). When `null` the + * factory passes an empty string — the server will reject the + * handshake, but tests that mock `io` need not bother priming the + * token resolver. + */ + coreToken: string | null; + /** + * Extra fields merged onto the `auth` payload. Today only the + * authenticated user's session JWT goes here (under `session`) so a + * future server-side handler can correlate the connection with the + * logged-in user. + */ + authExtras?: Record; + /** + * Override of the underlying Socket.IO connect options. The default + * shape matches what the previous in-line callers used. + */ + overrides?: Partial; +} + +const DEFAULT_OPTIONS: Partial = { + path: '/socket.io/', + transports: ['websocket', 'polling'], + reconnection: true, + reconnectionDelay: 2000, + reconnectionAttempts: Infinity, + forceNew: true, +}; + +export function createCoreSocket(baseUrl: string, opts: CoreSocketOptions): Socket { + const auth = { token: opts.coreToken ?? '', ...(opts.authExtras ?? {}) }; + return io(baseUrl, { ...DEFAULT_OPTIONS, ...(opts.overrides ?? {}), auth }); +} + +export interface ConnectCoreSocketOptions { + /** Resolves the Socket.IO base URL (no trailing `/rpc`). */ + getBaseUrl: () => Promise; + /** + * Caller's disposal flag. Awaited points (`getBaseUrl`, `getCoreRpcToken`) + * check this and short-circuit so the React effect can race a teardown + * without leaking a connection. + */ + isDisposed?: () => boolean; + authExtras?: Record; + overrides?: Partial; +} + +/** + * Resolve the base URL + core bearer, then hand off to `createCoreSocket`. + * + * Returns `null` if the caller's `isDisposed` flag flips during an await + * point — the caller does not need to also wrap the call in a disposed + * check. Keeps the per-callsite plumbing to a single line so the only + * thing the call sites need to test is "did the helper get invoked". + */ +export async function connectCoreSocket(opts: ConnectCoreSocketOptions): Promise { + const isDisposed = opts.isDisposed ?? (() => false); + const baseUrl = await opts.getBaseUrl(); + if (isDisposed()) return null; + const coreToken = await getCoreRpcToken(); + if (isDisposed()) return null; + return createCoreSocket(baseUrl, { + coreToken, + authExtras: opts.authExtras, + overrides: opts.overrides, + }); +} diff --git a/app/src/services/socketService.ts b/app/src/services/socketService.ts index 22b36e13c..57cde7af1 100644 --- a/app/src/services/socketService.ts +++ b/app/src/services/socketService.ts @@ -1,5 +1,5 @@ import debug from 'debug'; -import { io, Socket } from 'socket.io-client'; +import { type Socket } from 'socket.io-client'; import { getCoreStateSnapshot } from '../lib/coreState/store'; import { SocketIOMCPTransportImpl } from '../lib/mcp'; @@ -11,7 +11,8 @@ import { resetForUser, setSocketIdForUser, setStatusForUser } from '../store/soc import type { ChannelAuthMode, ChannelConnectionStatus, ChannelType } from '../types/channels'; import { IS_DEV } from '../utils/config'; import { createSafeLogData, sanitizeError } from '../utils/sanitize'; -import { getCoreRpcUrl } from './coreRpcClient'; +import { getCoreRpcToken, getCoreRpcUrl } from './coreRpcClient'; +import { createCoreSocket } from './coreSocket'; // Socket service logger using debug package // Enable logging by setting DEBUG=socket* in environment or localStorage @@ -170,6 +171,12 @@ class SocketService { store.dispatch(setBackend({ value: 'connecting' })); const backendUrl = await resolveCoreSocketBaseUrl(); + // If another `connect(token)` raced in while the URL was resolving, + // a stale invocation will see `this.token` flipped to the newer JWT + // (or a fresh socket already attached) and must bail before its + // io(...) call stomps the newer connection. Same guard repeats + // after the core-token resolve below. + if (this.token !== token || this.socket) return; socketLog('Connecting to core socket', { userId: uid, backendUrl }); // Ensure we're not connecting to the wrong URL (Vite dev HMR port guard). @@ -182,20 +189,24 @@ class SocketService { return; } - const socketOptions = { - auth: { token }, - path: '/socket.io/', - transports: ['websocket', 'polling'] as ('websocket' | 'polling')[], - reconnection: true, - reconnectionDelay: 1000, - reconnectionAttempts: 5, - forceNew: true, - timeout: 2000, - upgrade: true, - query: {}, - }; + // The local core's Socket.IO handshake validates the per-process bearer + // exposed via `core_rpc_token` (Tauri IPC) / the cloud-mode picker. The + // session JWT rides alongside on the `auth` payload as `session` so a + // future handler can correlate the connection with the logged-in user. + const coreToken = await getCoreRpcToken(); + if (this.token !== token || this.socket) return; - this.socket = io(backendUrl, socketOptions); + this.socket = createCoreSocket(backendUrl, { + coreToken, + authExtras: { session: token }, + overrides: { + reconnectionDelay: 1000, + reconnectionAttempts: 5, + timeout: 2000, + upgrade: true, + query: {}, + }, + }); // Flush any listeners that were registered before the socket existed. if (this.pendingListeners.length > 0) { diff --git a/src/core/auth.rs b/src/core/auth.rs index 3462b369c..a1498d11e 100644 --- a/src/core/auth.rs +++ b/src/core/auth.rs @@ -138,6 +138,23 @@ pub fn get_rpc_token() -> Option<&'static str> { RPC_TOKEN.get().map(String::as_str) } +/// Validate a supplied bearer token against the active per-process RPC token. +/// +/// Returns `true` only when the token subsystem is initialised and the +/// supplied token is non-empty and matches the in-memory expected value. +/// +/// This is the single entry point that non-HTTP transports (Socket.IO event +/// handlers, SSE bind-token issuance, future WebSocket surfaces) should call +/// before letting attacker-controlled input reach executable code. Keeping +/// the comparison in one helper means a future move to constant-time +/// equality is a one-line change for every transport at once. +pub fn verify_bearer_token(supplied: &str) -> bool { + let Some(expected) = get_rpc_token() else { + return false; + }; + bearer_matches(supplied, expected) +} + /// Axum middleware: enforce `Authorization: Bearer ` on all protected /// endpoints. /// @@ -312,6 +329,16 @@ mod tests { assert!(bearer_matches("cafebabe", "cafebabe")); } + #[test] + fn verify_bearer_token_returns_false_when_token_uninitialized() { + // RPC_TOKEN is a process-global OnceLock; on a fresh test binary it + // may already be set by another test that ran first, so we cannot + // assert the uninitialized branch here without process isolation. + // We can however confirm that an empty supplied value is always + // rejected, which exercises the second-leg invariant. + assert!(!verify_bearer_token("")); + } + #[test] fn extract_query_token_returns_none_on_missing_query() { assert_eq!(extract_query_token(None), None); diff --git a/src/core/dispatch.rs b/src/core/dispatch.rs index c4b064ea7..9e6ad82a4 100644 --- a/src/core/dispatch.rs +++ b/src/core/dispatch.rs @@ -96,17 +96,74 @@ pub async fn dispatch( fn try_core_dispatch( state: &AppState, method: &str, - _params: serde_json::Value, + params: serde_json::Value, ) -> Option> { match method { "core.ping" => Some(InvocationResult::ok(json!({ "ok": true }))), "core.version" => Some(InvocationResult::ok( json!({ "version": state.core_version }), )), + "core.events_subscribe_token" => Some(handle_events_subscribe_token(params)), _ => None, } } +/// Mint a single-shot bind token for the SSE `/events` stream. +/// +/// Browser `EventSource` cannot attach an `Authorization` header, so an +/// authenticated holder of the per-process RPC bearer first asks for a +/// short-lived token here (this RPC is gated by the same bearer-token +/// middleware as the rest of `/rpc`) and then opens +/// `/events?client_id=&token=`. The `/events` handler removes +/// the token from the store on first use, so a leaked URL cannot be +/// replayed by a second subscriber. +fn handle_events_subscribe_token(params: serde_json::Value) -> Result { + let obj = params.as_object(); + let client_id = obj + .and_then(|m| m.get("client_id")) + .and_then(|v| v.as_str()) + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .ok_or_else(|| { + log::warn!( + "[events-bind] reject mint: missing or empty client_id (param_keys={:?})", + obj.map(|m| m.keys().collect::>()) + ); + "missing or empty 'client_id' parameter".to_string() + })?; + let ttl = obj + .and_then(|m| m.get("ttl_secs")) + .and_then(|v| v.as_u64()) + .map(std::time::Duration::from_secs); + + let issued = + crate::core::event_bind_tokens::issue(client_id.to_string(), ttl).ok_or_else(|| { + log::warn!( + "[events-bind] reject mint: store at capacity (client_id_len={} ttl_secs={:?})", + client_id.len(), + ttl.map(|d| d.as_secs()) + ); + "events bind-token store at capacity; try again shortly".to_string() + })?; + + let ttl_remaining_secs = issued + .valid_until + .checked_duration_since(std::time::Instant::now()) + .unwrap_or_default() + .as_secs(); + + log::debug!( + "[events-bind] minted token for client_id_len={} ttl_secs={}", + client_id.len(), + ttl_remaining_secs + ); + + InvocationResult::ok(json!({ + "token": issued.token, + "ttl_secs": ttl_remaining_secs, + })) +} + async fn try_registry_dispatch( method: &str, params: Value, diff --git a/src/core/event_bind_tokens.rs b/src/core/event_bind_tokens.rs new file mode 100644 index 000000000..41170f8a4 --- /dev/null +++ b/src/core/event_bind_tokens.rs @@ -0,0 +1,205 @@ +//! Per-subscription bind tokens for the SSE `/events` endpoint. +//! +//! Browser `EventSource` clients cannot attach an `Authorization` header, +//! so the `/events` stream cannot ride on the same bearer-token middleware +//! that protects `POST /rpc`. Instead, an authenticated holder of the +//! per-process RPC bearer first calls +//! `core.events_subscribe_token { client_id }` to mint a short-lived, +//! single-purpose bind token, then opens +//! `/events?client_id=&token=`. +//! +//! Properties of the bind token: +//! - 256 bits of CSPRNG randomness (hex-encoded; 64 chars on the wire). +//! - Bound to one `client_id` — verifying with any other id rejects. +//! - Single-shot by default: the connect-time validate step removes the +//! token from the store, so a leaked URL cannot be reused. +//! - Time-bounded: minted tokens carry a `valid_until` instant and a +//! small purge pass runs on each lookup to bound store size. +//! +//! This module owns only the in-memory store; the RPC handler that mints +//! tokens lives in `src/core/dispatch.rs` (the `core.*` namespace), +//! and the `/events` handler in `src/core/jsonrpc.rs` consumes them. + +use std::collections::HashMap; +use std::sync::RwLock; +use std::time::{Duration, Instant}; + +use once_cell::sync::Lazy; + +/// Default lifetime of a freshly issued bind token if the caller does not +/// specify one. Long enough for normal subscribe latency, short enough that +/// an accidentally-logged URL stops working before useful exfil. The RPC +/// caller can shorten this with the `ttl_secs` field. +const DEFAULT_TTL: Duration = Duration::from_secs(60); + +/// Upper bound the caller can request. Anything larger collapses to this so +/// a misbehaving (or compromised) caller cannot mint long-lived tokens. +const MAX_TTL: Duration = Duration::from_secs(60 * 30); + +/// Maximum live tokens in the store. Each token is ~80 bytes plus the +/// `client_id` String; this is a defensive ceiling, not a normal-load cap. +/// When the store is full, the oldest expired entries are evicted; if none +/// are expired, a fresh issue request is rejected so the store cannot grow +/// without bound. +const MAX_TOKENS: usize = 4096; + +#[derive(Debug, Clone)] +struct BindEntry { + client_id: String, + valid_until: Instant, +} + +static STORE: Lazy>> = + Lazy::new(|| RwLock::new(HashMap::with_capacity(64))); + +/// A freshly-minted bind token plus its expiry. Returned to the RPC caller +/// so the UI can pass both to `/events?client_id=…&token=…`. +#[derive(Debug, Clone)] +pub struct BindToken { + pub token: String, + pub valid_until: Instant, +} + +/// Mint a new bind token tied to `client_id`. +/// +/// `ttl_override` lets the caller request a shorter lifetime than the +/// default; anything above `MAX_TTL` is clamped down. Returns `None` if the +/// store is at capacity and no expired entries can be reclaimed — callers +/// should surface this as a transient error rather than retrying in a +/// tight loop. +pub fn issue(client_id: impl Into, ttl_override: Option) -> Option { + let ttl = ttl_override.map(|d| d.min(MAX_TTL)).unwrap_or(DEFAULT_TTL); + let client_id = client_id.into(); + let valid_until = Instant::now() + ttl; + let token = generate_token(); + let entry = BindEntry { + client_id, + valid_until, + }; + + let mut store = STORE.write().ok()?; + purge_expired_locked(&mut store); + if store.len() >= MAX_TOKENS { + log::warn!( + "[events-bind] capacity reached ({} entries) — refusing to mint", + store.len() + ); + return None; + } + store.insert(token.clone(), entry); + Some(BindToken { token, valid_until }) +} + +/// Validate a supplied `(client_id, token)` pair and remove the token from +/// the store on success. +/// +/// Returns `true` only when the token exists, is not expired, and the +/// bound `client_id` matches what was supplied. The remove-on-success +/// behaviour is what gives the token its single-shot semantics — an +/// attacker who replays the URL after the legitimate UI has connected +/// gets nothing. +pub fn consume(client_id: &str, token: &str) -> bool { + let Ok(mut store) = STORE.write() else { + return false; + }; + purge_expired_locked(&mut store); + // Peek before removing: a wrong `client_id` must NOT consume the token, + // or a single guessed-id request can DoS the legitimate subscriber by + // racing them to the consume. + let Some(entry) = store.get(token) else { + log::debug!("[events-bind] consume: token not found"); + return false; + }; + if entry.client_id != client_id { + log::warn!("[events-bind] consume: client_id mismatch (token bound to other id)"); + return false; + } + let entry = store + .remove(token) + .expect("token was present in the binding check above"); + log::debug!( + "[events-bind] consume: ok (client_id_len={} ttl_remaining_ms={})", + entry.client_id.len(), + entry + .valid_until + .checked_duration_since(Instant::now()) + .unwrap_or_default() + .as_millis() + ); + true +} + +fn purge_expired_locked(store: &mut HashMap) { + let now = Instant::now(); + store.retain(|_, entry| entry.valid_until > now); +} + +fn generate_token() -> String { + use rand::RngExt as _; + let mut bytes = [0u8; 32]; + rand::rng().fill(&mut bytes); + hex::encode(bytes) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn issued_token_validates_for_matching_client_id() { + let issued = issue("cli-test-1", None).expect("issue"); + assert!(consume("cli-test-1", &issued.token)); + } + + #[test] + fn issued_token_rejects_wrong_client_id() { + let issued = issue("cli-test-2", None).expect("issue"); + assert!(!consume("attacker-id", &issued.token)); + } + + #[test] + fn wrong_client_id_does_not_consume_token() { + // Mismatched consume must leave the token intact so the legitimate + // subscriber can still validate after the failed probe — otherwise + // a wrong-id request becomes a one-shot DoS. + let issued = issue("cli-test-mismatch", None).expect("issue"); + assert!(!consume("attacker-id", &issued.token)); + assert!( + consume("cli-test-mismatch", &issued.token), + "legitimate consume must still succeed after a mismatched probe" + ); + } + + #[test] + fn consumed_token_cannot_be_reused() { + let issued = issue("cli-test-3", None).expect("issue"); + assert!(consume("cli-test-3", &issued.token)); + assert!( + !consume("cli-test-3", &issued.token), + "tokens must be single-shot" + ); + } + + #[test] + fn expired_token_is_rejected() { + let issued = issue("cli-test-4", Some(Duration::from_millis(1))).expect("issue"); + std::thread::sleep(Duration::from_millis(20)); + assert!(!consume("cli-test-4", &issued.token)); + } + + #[test] + fn unknown_token_is_rejected() { + assert!(!consume("any-id", "f00ba1")); + } + + #[test] + fn ttl_override_is_clamped_to_max() { + // Any caller asking for more than `MAX_TTL` collapses to the cap; + // confirm the issue path does not panic and the resulting token + // still validates. + let issued = + issue("cli-test-clamp", Some(Duration::from_secs(60 * 60 * 24))).expect("issue"); + assert!(issued.valid_until <= Instant::now() + MAX_TTL + Duration::from_secs(1)); + assert!(consume("cli-test-clamp", &issued.token)); + } +} diff --git a/src/core/event_bus/events.rs b/src/core/event_bus/events.rs index 52b0870ad..948acd92c 100644 --- a/src/core/event_bus/events.rs +++ b/src/core/event_bus/events.rs @@ -92,10 +92,22 @@ pub enum DomainEvent { // ── Channels ──────────────────────────────────────────────────────── /// An inbound channel message from the transport layer, ready for processing. + /// + /// `sender`, `reply_target`, and `thread_ts` are carried alongside + /// `channel` so the agent loop can derive per-sender conversation keys + /// the same way `channels::context::conversation_history_key` does for + /// other inbound paths — keying on `channel` alone collapses distinct + /// senders inside a shared channel into one cached session. ChannelInboundMessage { event_name: String, channel: String, message: String, + #[doc = "Originating user/account id within the channel. `None` for legacy publishers that don't surface it."] + sender: Option, + #[doc = "Direct-message peer or group thread the reply should go to. `None` when the channel does not distinguish."] + reply_target: Option, + #[doc = "Slack/Discord thread anchor when the message is in-thread. `None` for top-level messages."] + thread_ts: Option, raw_data: serde_json::Value, }, /// A message was received on a channel. diff --git a/src/core/event_bus/events_tests.rs b/src/core/event_bus/events_tests.rs index acdd05170..424db45fb 100644 --- a/src/core/event_bus/events_tests.rs +++ b/src/core/event_bus/events_tests.rs @@ -79,6 +79,9 @@ fn all_variants_have_correct_domain() { event_name: "telegram:message".into(), channel: "telegram".into(), message: "hi".into(), + sender: None, + reply_target: None, + thread_ts: None, raw_data: serde_json::Value::Null, }, "channel", diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 1cfb93a74..a80e6a2d2 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -460,11 +460,95 @@ fn error_html(message: &str) -> String { ) } +/// Inspect the browser fetch-metadata + Referer/Origin headers and decide +/// whether the inbound `/auth/telegram` request looks like a legitimate +/// top-level redirect from Telegram, or a cross-site CSRF attempt. +/// +/// The endpoint cannot require a bearer token (the redirect happens in a +/// fresh browser tab; `EventSource`-style header injection is not an +/// option), and there is no in-process state issued by an authenticated +/// FE flow today (`/start register` is initiated in Telegram, not in the +/// local app). So this fetch-metadata gate is the layer that distinguishes +/// "user clicked the link the bot sent them" from "malicious page +/// navigates the user's loopback core via `window.location`/``". +/// +/// Accepted shapes: +/// - All `Sec-Fetch-*` headers absent (older browsers, CLI clients). +/// - `Sec-Fetch-Mode: navigate` AND `Sec-Fetch-Dest: document`. +/// - `Sec-Fetch-Site` is `same-origin` / `none`, OR `cross-site` with a +/// `Referer` that starts with `https://t.me/` (the legit bot redirect). +/// +/// Rejected shapes: +/// - `Sec-Fetch-Mode` is `no-cors` / `cors` / `same-origin` (only +/// `navigate` makes sense for a top-level page load). +/// - `Sec-Fetch-Dest` is anything other than `document` (image/script/ +/// iframe embeds from malicious pages). +/// - `Sec-Fetch-Site: cross-site` with a `Referer`/`Origin` that is not +/// `https://t.me/...` (CSRF redirect from a third-party site). +fn telegram_callback_origin_ok(headers: &axum::http::HeaderMap) -> Result<(), &'static str> { + let get_str = |name: &str| -> Option<&str> { + headers + .get(name) + .and_then(|v| v.to_str().ok()) + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + }; + + let mode = get_str("sec-fetch-mode"); + let dest = get_str("sec-fetch-dest"); + let site = get_str("sec-fetch-site"); + let referer = get_str("referer"); + let origin = get_str("origin"); + + if let Some(mode) = mode { + if mode != "navigate" { + return Err("Sec-Fetch-Mode must be 'navigate'"); + } + } + if let Some(dest) = dest { + if dest != "document" { + return Err("Sec-Fetch-Dest must be 'document'"); + } + } + + let referer_is_telegram = referer + .map(|r| r.starts_with("https://t.me/") || r.starts_with("https://web.telegram.org/")) + .unwrap_or(false); + let origin_is_telegram = origin + .map(|o| o == "https://t.me" || o == "https://web.telegram.org") + .unwrap_or(false); + + if let Some(site) = site { + if site == "cross-site" && !(referer_is_telegram || origin_is_telegram) { + return Err("cross-site redirect must originate from telegram"); + } + } else if let Some(referer) = referer { + // No Sec-Fetch-Site: fall back to Referer host check. Accept + // loopback referer (direct nav inside the local app) — parsed + // exactly so `http://localhost.attacker.example/...` does not + // satisfy the gate — and accept telegram referer (legit bot + // redirect); reject everything else. + let local = url::Url::parse(referer) + .ok() + .and_then(|u| u.host_str().map(str::to_string)) + .map(|h| matches!(h.as_str(), "localhost" | "127.0.0.1" | "::1")) + .unwrap_or(false); + if !(local || referer_is_telegram) { + return Err("Referer must be telegram or local"); + } + } + + Ok(()) +} + /// Handles the Telegram authentication callback. /// /// It consumes a one-time token, exchanges it for a JWT from the backend, /// and stores the session locally. -async fn telegram_auth_handler(Query(query): Query) -> impl IntoResponse { +async fn telegram_auth_handler( + headers: axum::http::HeaderMap, + Query(query): Query, +) -> impl IntoResponse { let html_response = |status: StatusCode, body: String| -> Response { ( status, @@ -474,6 +558,18 @@ async fn telegram_auth_handler(Query(query): Query) -> impl I .into_response() }; + if let Err(reason) = telegram_callback_origin_ok(&headers) { + log::warn!("[auth:telegram] rejecting callback: {reason}"); + return html_response( + StatusCode::FORBIDDEN, + error_html( + "This login callback did not come from the Telegram bot. \ + Open the link the bot sent you directly, do not let \ + another page redirect you here.", + ), + ); + } + let token = match query .token .as_deref() @@ -802,36 +898,107 @@ async fn schema_handler(State(_state): State) -> impl IntoResponse { } /// Query parameters for the events SSE endpoint. +/// +/// `client_id` selects which broadcast events to forward; `token` is the +/// single-shot bind token minted by the `core.events_subscribe_token` RPC. +/// Both are required — browser `EventSource` cannot attach an +/// `Authorization` header, so the bind token is the only credential the +/// endpoint accepts. #[derive(Debug, serde::Deserialize)] struct EventsQuery { - /// Unique identifier for the client requesting events. client_id: String, + #[serde(default)] + token: Option, } /// Handler for the main events SSE endpoint. /// -/// Streams real-time events filtered by `client_id`. +/// Accepts either of two credentials: +/// 1. `Authorization: Bearer ` — used by CLI tooling, the +/// Tauri shell via `core_rpc_relay`, and the in-tree e2e suite that +/// can set HTTP headers directly. Validated against the same +/// per-process bearer the rest of `/rpc` uses. +/// 2. `?token=` minted via the `core.events_subscribe_token` RPC +/// — used by browser `EventSource`, which cannot attach custom +/// headers. The token is bound to a specific `client_id` and is +/// consumed on validation so a leaked URL cannot be replayed. +/// +/// Both paths converge on the same broadcast stream filtered by +/// `client_id`. async fn events_handler( + headers: axum::http::HeaderMap, Query(query): Query, -) -> Sse>> { +) -> Response { + let bearer = headers + .get(header::AUTHORIZATION) + .and_then(|v| v.to_str().ok()) + .and_then(|v| v.strip_prefix("Bearer ")) + .map(str::trim) + .filter(|s| !s.is_empty()); + let bearer_ok = bearer + .map(crate::core::auth::verify_bearer_token) + .unwrap_or(false); + + if !bearer_ok { + let supplied_token = query + .token + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()); + let Some(supplied_token) = supplied_token else { + log::warn!( + "[events] reject subscribe: missing bind token + missing bearer (client_id_len={})", + query.client_id.len() + ); + return ( + StatusCode::UNAUTHORIZED, + Json(json!({ + "ok": false, + "error": "unauthorized", + "message": "Missing credentials. Supply 'Authorization: Bearer ' or mint a bind token with the `core.events_subscribe_token` RPC and pass it as ?token=" + })), + ) + .into_response(); + }; + if !crate::core::event_bind_tokens::consume(&query.client_id, supplied_token) { + log::warn!( + "[events] reject subscribe: bind token invalid or expired (client_id_len={})", + query.client_id.len() + ); + return ( + StatusCode::UNAUTHORIZED, + Json(json!({ + "ok": false, + "error": "unauthorized", + "message": "Bind token is unknown, expired, or bound to a different client_id." + })), + ) + .into_response(); + } + } + let client_id = query.client_id; let rx = crate::openhuman::channels::providers::web::subscribe_web_channel_events(); - let stream = tokio_stream::wrappers::BroadcastStream::new(rx).filter_map(move |item| { - let event = match item { - Ok(ev) => ev, - Err(_) => return None, - }; - if event.client_id != client_id { - return None; - } - let data = match serde_json::to_string(&event) { - Ok(data) => data, - Err(_) => return None, - }; - Some(Ok(Event::default().event(event.event).data(data))) - }); + let stream = tokio_stream::wrappers::BroadcastStream::new(rx).filter_map( + move |item| -> Option> { + let event = match item { + Ok(ev) => ev, + Err(_) => return None, + }; + if event.client_id != client_id { + return None; + } + let data = match serde_json::to_string(&event) { + Ok(data) => data, + Err(_) => return None, + }; + Some(Ok(Event::default().event(event.event).data(data))) + }, + ); - Sse::new(stream).keep_alive(KeepAlive::new().interval(std::time::Duration::from_secs(10))) + Sse::new(stream) + .keep_alive(KeepAlive::new().interval(std::time::Duration::from_secs(10))) + .into_response() } /// Handler for the webhook debug events SSE endpoint. @@ -862,7 +1029,7 @@ async fn root_handler() -> impl IntoResponse { "endpoints": { "health": "/health", "schema": "/schema", - "events": "/events?client_id=", + "events": "/events?client_id=&token=", "rpc": "/rpc" }, "usage": { diff --git a/src/core/jsonrpc_tests.rs b/src/core/jsonrpc_tests.rs index d404d83fc..6e91b9225 100644 --- a/src/core/jsonrpc_tests.rs +++ b/src/core/jsonrpc_tests.rs @@ -933,6 +933,98 @@ fn escape_html_is_noop_for_safe_text() { assert_eq!(escape_html(""), ""); } +// --- telegram callback fetch-metadata gate -------------------------------- + +fn hdr_map(pairs: &[(&str, &str)]) -> axum::http::HeaderMap { + let mut m = axum::http::HeaderMap::new(); + for (k, v) in pairs { + m.insert( + axum::http::HeaderName::from_bytes(k.as_bytes()).unwrap(), + axum::http::HeaderValue::from_str(v).unwrap(), + ); + } + m +} + +#[test] +fn telegram_callback_origin_ok_accepts_no_metadata_headers() { + // Older browsers and CLI clients (curl) send neither Sec-Fetch-* nor + // Origin/Referer. The legacy flow has to keep working — reject only + // when there is evidence of a cross-site embedded context. + let headers = hdr_map(&[]); + assert!(super::telegram_callback_origin_ok(&headers).is_ok()); +} + +#[test] +fn telegram_callback_origin_ok_accepts_legit_top_nav_from_telegram() { + let headers = hdr_map(&[ + ("sec-fetch-mode", "navigate"), + ("sec-fetch-dest", "document"), + ("sec-fetch-site", "cross-site"), + ("referer", "https://t.me/some_bot"), + ]); + assert!(super::telegram_callback_origin_ok(&headers).is_ok()); +} + +#[test] +fn telegram_callback_origin_ok_accepts_same_origin_local_nav() { + let headers = hdr_map(&[ + ("sec-fetch-mode", "navigate"), + ("sec-fetch-dest", "document"), + ("sec-fetch-site", "same-origin"), + ]); + assert!(super::telegram_callback_origin_ok(&headers).is_ok()); +} + +#[test] +fn telegram_callback_origin_ok_rejects_image_embed() { + let headers = hdr_map(&[ + ("sec-fetch-mode", "no-cors"), + ("sec-fetch-dest", "image"), + ("sec-fetch-site", "cross-site"), + ]); + assert!(super::telegram_callback_origin_ok(&headers).is_err()); +} + +#[test] +fn telegram_callback_origin_ok_rejects_iframe_embed() { + let headers = hdr_map(&[ + ("sec-fetch-mode", "navigate"), + ("sec-fetch-dest", "iframe"), + ("sec-fetch-site", "cross-site"), + ]); + assert!(super::telegram_callback_origin_ok(&headers).is_err()); +} + +#[test] +fn telegram_callback_origin_ok_rejects_cross_site_from_non_telegram() { + let headers = hdr_map(&[ + ("sec-fetch-mode", "navigate"), + ("sec-fetch-dest", "document"), + ("sec-fetch-site", "cross-site"), + ("referer", "https://attacker.example/page"), + ]); + assert!(super::telegram_callback_origin_ok(&headers).is_err()); +} + +#[test] +fn telegram_callback_origin_ok_rejects_non_telegram_referer_without_fetch_metadata() { + let headers = hdr_map(&[("referer", "https://attacker.example/post")]); + assert!(super::telegram_callback_origin_ok(&headers).is_err()); +} + +#[test] +fn telegram_callback_origin_ok_rejects_localhost_host_prefix_decoy() { + // Regression: prefix-matching the referer accepted hostnames like + // `http://localhost.attacker.example/...`. With exact-host parsing + // these must be rejected even when no fetch-metadata headers are + // present. + let headers = hdr_map(&[("referer", "http://localhost.attacker.example/cb")]); + assert!(super::telegram_callback_origin_ok(&headers).is_err()); + let headers = hdr_map(&[("referer", "http://127.0.0.1.attacker.example/cb")]); + assert!(super::telegram_callback_origin_ok(&headers).is_err()); +} + // --- invoke_method parameter-shape errors --------------------------------- #[tokio::test] diff --git a/src/core/mod.rs b/src/core/mod.rs index 1d93d0cfe..a7ae071c7 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -12,6 +12,7 @@ pub mod auth; pub mod autocomplete_cli_adapter; pub mod cli; pub mod dispatch; +pub mod event_bind_tokens; pub mod event_bus; pub mod jsonrpc; pub mod legacy_aliases; diff --git a/src/core/socketio.rs b/src/core/socketio.rs index 3e723441b..fd91122af 100644 --- a/src/core/socketio.rs +++ b/src/core/socketio.rs @@ -1,9 +1,81 @@ use serde::Deserialize; use serde::Serialize; use serde_json::json; -use socketioxide::extract::{Data, SocketRef}; +use socketioxide::extract::{Data, SocketRef, TryData}; use socketioxide::SocketIo; +/// Marker stored in [`SocketRef::extensions`] once a connection has presented a +/// bearer token that matches the active per-process RPC token. +/// +/// Event handlers consult this before forwarding attacker-controllable input +/// into the JSON-RPC dispatcher or the web-chat orchestrator: an unauthenticated +/// socket that never picked up the marker is allowed to receive broadcast-style +/// events (read-only) but cannot trigger executable work. +#[derive(Clone, Copy, Debug)] +struct AuthedConnection; + +/// Connection-time payload the client passes via Socket.IO's `auth` field. +/// +/// Browsers do not let `EventSource` / `WebSocket` clients attach custom +/// headers, so the handshake `auth` map is the only header-equivalent slot +/// available for our per-process bearer. The socket-IO Node/JS clients all +/// surface `io(url, { auth: { token: "" } })` for this. +#[derive(Debug, Default, Deserialize)] +struct HandshakeAuth { + #[serde(default)] + token: Option, +} + +/// Origins the local core trusts at the Socket.IO handshake. +/// +/// `tauri://localhost` is the production app webview; `http://localhost:*` +/// and `http://127.0.0.1:*` cover the Vite dev server (`pnpm dev:app`) +/// and standalone CLI tooling that opens browser pages against the local +/// listener. A missing `Origin` header is treated as a native (non-browser) +/// client and accepted — only the cross-origin browser-page case is the +/// targeted bad actor here. +fn origin_is_allowed(origin: Option<&str>) -> bool { + let Some(origin) = origin else { + return true; // native clients (CLI, Tauri shell) — no Origin header + }; + let origin = origin.trim(); + if origin.is_empty() || origin == "null" { + return false; + } + if origin == "tauri://localhost" || origin == "https://tauri.localhost" { + return true; + } + // Parse the URL and compare the host EXACTLY against the loopback + // allowlist — `starts_with` matching accepted decoys like + // `http://localhost.attacker.example` and bypassed the gate. + let Ok(parsed) = url::Url::parse(origin) else { + return false; + }; + // `url::Url::host_str` returns IPv6 hosts with surrounding brackets, + // hostnames bare. Accept both shapes. + matches!( + parsed.host_str(), + Some("localhost" | "127.0.0.1" | "::1" | "[::1]") + ) +} + +/// True when `socket` finished the handshake with a valid bearer token. +fn socket_is_authed(socket: &SocketRef) -> bool { + socket.extensions.get::().is_some() +} + +/// Best-effort disconnect. Called when we discover an unauthenticated socket +/// inside an event handler — the connect path already disconnects the bad +/// origins / wrong tokens, so this is purely a defense-in-depth path. +fn drop_unauthed(socket: &SocketRef, reason: &'static str) { + log::warn!( + "[socketio] dropping unauthenticated socket id={} reason={}", + socket.id, + reason + ); + let _ = socket.clone().disconnect(); +} + /// Standard event payload for the web channel transport. /// /// This structure defines the data sent to Socket.IO clients for various @@ -181,129 +253,179 @@ pub fn attach_socketio() -> (socketioxide::layer::SocketIoLayer, SocketIo) { io.config().engine_config.req_path ); - io.ns("/", |socket: SocketRef| { - let client_id = socket.id.to_string(); - log::info!("[socketio] client connected id={client_id}"); - // Join a room named after the client ID for targeted event delivery. - join_room_logged(&socket, &client_id, &client_id); - // Also auto-join the "system" room so every connected client - // receives broadcast-style events that aren't tied to a - // specific chat thread. Today this covers proactive messages - // (welcome agent, morning briefing, cron-driven announcements) - // which `channels::proactive::ProactiveMessageSubscriber` - // emits with `client_id = "system"` — see `emit_web_channel_event`. - // If this join fails the welcome message silently disappears, - // so we log both success and failure for diagnosability. - join_room_logged(&socket, "system", &client_id); - let ready_payload = json!({ "sid": client_id }); - log::debug!("[socketio] emit event=ready to_client={}", socket.id); - let _ = socket.emit("ready", &ready_payload); + io.ns( + "/", + |socket: SocketRef, TryData(handshake): TryData| { + let client_id = socket.id.to_string(); - // Handler for JSON-RPC over WebSocket. - socket.on( - "rpc:request", - |socket: SocketRef, Data(payload): Data| async move { - let client_id = socket.id.to_string(); - log::info!( - "[socketio] rpc:request method={} id={} client={}", - payload.method, - payload.id, + // Reject cross-origin browser pages before the handshake completes. + // Native clients (Tauri shell, CLI) do not set an `Origin` header and + // are accepted; only browser pages from origins outside the local + // app surface are dropped here. See `origin_is_allowed`. + let origin = socket + .req_parts() + .headers + .get(axum::http::header::ORIGIN) + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + if !origin_is_allowed(origin.as_deref()) { + log::warn!( + "[socketio] rejecting connect: bad origin {:?} client={}", + origin, client_id ); + let _ = socket.clone().disconnect(); + return; + } - // Invoke the method through the same logic used by the HTTP RPC endpoint. - let response = match crate::core::jsonrpc::invoke_method( - crate::core::jsonrpc::default_state(), - payload.method.as_str(), - payload.params, - ) - .await - { - Ok(result) => ( - "rpc:response", - json!({ "id": payload.id, "result": result }), - ), - Err(message) => ( - "rpc:error", - json!({ - "id": payload.id, - "error": { "code": -32000, "message": message } - }), - ), - }; + // Verify the handshake bearer matches the per-process RPC token. + // `TryData` lets us treat a missing/malformed `auth` payload as a + // soft failure (no panic) and reject the connect cleanly. + let supplied = handshake.ok().and_then(|h| h.token).unwrap_or_default(); + if !crate::core::auth::verify_bearer_token(&supplied) { + log::warn!( + "[socketio] rejecting connect: missing or invalid bearer client={}", + client_id + ); + let _ = socket.clone().disconnect(); + return; + } + socket.extensions.insert(AuthedConnection); - let _ = socket.emit(response.0, &response.1); - }, - ); + log::info!("[socketio] client connected id={client_id} (authenticated)"); + // Join a room named after the client ID for targeted event delivery. + join_room_logged(&socket, &client_id, &client_id); + // Also auto-join the "system" room so every connected client + // receives broadcast-style events that aren't tied to a + // specific chat thread. Today this covers proactive messages + // (welcome agent, morning briefing, cron-driven announcements) + // which `channels::proactive::ProactiveMessageSubscriber` + // emits with `client_id = "system"` — see `emit_web_channel_event`. + // If this join fails the welcome message silently disappears, + // so we log both success and failure for diagnosability. + join_room_logged(&socket, "system", &client_id); + let ready_payload = json!({ "sid": client_id }); + log::debug!("[socketio] emit event=ready to_client={}", socket.id); + let _ = socket.emit("ready", &ready_payload); - // Handler for starting a chat turn. - socket.on( - "chat:start", - |socket: SocketRef, Data(payload): Data| async move { - let client_id = socket.id.to_string(); - let thread_id = payload.thread_id.clone(); - let model_override = payload.model_override.or(payload.model); - log::debug!( + // Handler for JSON-RPC over WebSocket. + socket.on( + "rpc:request", + |socket: SocketRef, Data(payload): Data| async move { + if !socket_is_authed(&socket) { + drop_unauthed(&socket, "rpc:request from unauthenticated socket"); + return; + } + let client_id = socket.id.to_string(); + log::info!( + "[socketio] rpc:request method={} id={} client={}", + payload.method, + payload.id, + client_id + ); + + // Invoke the method through the same logic used by the HTTP RPC endpoint. + let response = match crate::core::jsonrpc::invoke_method( + crate::core::jsonrpc::default_state(), + payload.method.as_str(), + payload.params, + ) + .await + { + Ok(result) => ( + "rpc:response", + json!({ "id": payload.id, "result": result }), + ), + Err(message) => ( + "rpc:error", + json!({ + "id": payload.id, + "error": { "code": -32000, "message": message } + }), + ), + }; + + let _ = socket.emit(response.0, &response.1); + }, + ); + + // Handler for starting a chat turn. + socket.on( + "chat:start", + |socket: SocketRef, Data(payload): Data| async move { + if !socket_is_authed(&socket) { + drop_unauthed(&socket, "chat:start from unauthenticated socket"); + return; + } + let client_id = socket.id.to_string(); + let thread_id = payload.thread_id.clone(); + let model_override = payload.model_override.or(payload.model); + log::debug!( "[socketio] recv event=chat:start client_id={} thread_id={} message_bytes={}", client_id, thread_id, payload.message.len() ); - // Trigger the web channel's chat logic. - match crate::openhuman::channels::providers::web::start_chat( - &client_id, - &payload.thread_id, - &payload.message, - model_override, - payload.temperature, - payload.profile_id, - payload.locale, - ) - .await - { - Ok(request_id) => { - let accepted_payload = json!({ - "event": "chat_accepted", - "client_id": client_id, - "thread_id": thread_id, - "request_id": request_id, - }); - emit_with_aliases(&socket, "chat_accepted", &accepted_payload); + // Trigger the web channel's chat logic. + match crate::openhuman::channels::providers::web::start_chat( + &client_id, + &payload.thread_id, + &payload.message, + model_override, + payload.temperature, + payload.profile_id, + payload.locale, + ) + .await + { + Ok(request_id) => { + let accepted_payload = json!({ + "event": "chat_accepted", + "client_id": client_id, + "thread_id": thread_id, + "request_id": request_id, + }); + emit_with_aliases(&socket, "chat_accepted", &accepted_payload); + } + Err(error) => { + let error_payload = json!({ + "event": "chat_error", + "client_id": client_id, + "thread_id": thread_id, + "request_id": "", + "message": error, + "error_type": "inference", + }); + emit_with_aliases(&socket, "chat_error", &error_payload); + } } - Err(error) => { - let error_payload = json!({ - "event": "chat_error", - "client_id": client_id, - "thread_id": thread_id, - "request_id": "", - "message": error, - "error_type": "inference", - }); - emit_with_aliases(&socket, "chat_error", &error_payload); - } - } - }, - ); + }, + ); - // Handler for cancelling an active chat turn. - socket.on( - "chat:cancel", - |socket: SocketRef, Data(payload): Data| async move { - let client_id = socket.id.to_string(); - log::debug!( - "[socketio] recv event=chat:cancel client_id={} thread_id={}", - client_id, - payload.thread_id - ); - let _ = crate::openhuman::channels::providers::web::cancel_chat( - &client_id, - &payload.thread_id, - ) - .await; - }, - ); - }); + // Handler for cancelling an active chat turn. + socket.on( + "chat:cancel", + |socket: SocketRef, Data(payload): Data| async move { + if !socket_is_authed(&socket) { + drop_unauthed(&socket, "chat:cancel from unauthenticated socket"); + return; + } + let client_id = socket.id.to_string(); + log::debug!( + "[socketio] recv event=chat:cancel client_id={} thread_id={}", + client_id, + payload.thread_id + ); + let _ = crate::openhuman::channels::providers::web::cancel_chat( + &client_id, + &payload.thread_id, + ) + .await; + }, + ); + }, + ); (layer, io) } @@ -615,7 +737,7 @@ fn emit_room_with_aliases(io: &SocketIo, room: &str, name: &str, payload: &serde #[cfg(test)] mod tests { - use super::event_alias; + use super::{event_alias, origin_is_allowed}; #[test] fn event_alias_translates_between_delimiters() { @@ -623,4 +745,49 @@ mod tests { assert_eq!(event_alias("chat:error").as_deref(), Some("chat_error")); assert_eq!(event_alias("ready"), None); } + + #[test] + fn origin_allowlist_accepts_native_clients() { + assert!(origin_is_allowed(None)); + } + + #[test] + fn origin_allowlist_accepts_tauri_localhost() { + assert!(origin_is_allowed(Some("tauri://localhost"))); + assert!(origin_is_allowed(Some("https://tauri.localhost"))); + } + + #[test] + fn origin_allowlist_accepts_local_dev_server() { + assert!(origin_is_allowed(Some("http://localhost:1420"))); + assert!(origin_is_allowed(Some("http://127.0.0.1:1420"))); + assert!(origin_is_allowed(Some("http://[::1]:1420"))); + } + + #[test] + fn origin_allowlist_rejects_cross_origin_browser_pages() { + assert!(!origin_is_allowed(Some("https://attacker.example"))); + assert!(!origin_is_allowed(Some("http://evil.local"))); + assert!(!origin_is_allowed(Some("null"))); + assert!(!origin_is_allowed(Some(""))); + } + + #[test] + fn origin_allowlist_rejects_host_prefix_decoys() { + // Regression: `starts_with("localhost")` accepted these; the exact + // host match must not. + assert!(!origin_is_allowed(Some( + "http://localhost.attacker.example" + ))); + assert!(!origin_is_allowed(Some( + "http://127.0.0.1.attacker.example" + ))); + assert!(!origin_is_allowed(Some("https://localhost-evil"))); + } + + #[test] + fn origin_allowlist_rejects_unparseable_origin() { + assert!(!origin_is_allowed(Some("not a url"))); + assert!(!origin_is_allowed(Some("javascript:alert(1)"))); + } } diff --git a/src/openhuman/agent/harness/memory_context.rs b/src/openhuman/agent/harness/memory_context.rs index 620734209..a5ca26e25 100644 --- a/src/openhuman/agent/harness/memory_context.rs +++ b/src/openhuman/agent/harness/memory_context.rs @@ -1,3 +1,4 @@ +use super::memory_context_safety::{is_potentially_untrusted, wrap_untrusted_for_agent}; use crate::openhuman::memory::Memory; use crate::openhuman::util::provenance_tag; use std::collections::HashSet; @@ -58,7 +59,13 @@ pub(crate) async fn build_context( context.push_str("[Memory context]\n"); for entry in &relevant { seen_keys.insert(entry.key.clone()); - let _ = writeln!(context, "- {}: {}", entry.key, entry.content); + let rendered_content = if is_potentially_untrusted(entry) { + let hint = entry.namespace.as_deref().unwrap_or("connector"); + wrap_untrusted_for_agent(&entry.content, hint) + } else { + entry.content.clone() + }; + let _ = writeln!(context, "- {}: {}", entry.key, rendered_content); } context.push('\n'); } diff --git a/src/openhuman/agent/harness/memory_context_safety.rs b/src/openhuman/agent/harness/memory_context_safety.rs new file mode 100644 index 000000000..46e61a720 --- /dev/null +++ b/src/openhuman/agent/harness/memory_context_safety.rs @@ -0,0 +1,251 @@ +//! Trust-tier helpers for memory entries surfaced into agent prompts. +//! +//! Memory entries reach the agent prompt by way of vector-recall over the +//! full memory store, which mixes content from many provenance tiers: +//! +//! - **User-authored** turns from the same chat (high trust). +//! - **Agent-authored** summaries and working-memory snapshots (high trust). +//! - **Connector-synced** content harvested from Gmail / Slack / Notion / +//! Discord / web feeds (untrusted: anything in the body of an email, the +//! text of a Slack DM, or a Notion page is text the agent has no a-priori +//! reason to obey). +//! +//! Recall returns the same shape regardless of which tier the row came +//! from, so a prompt-injection paragraph that lives inside an inbound +//! email reaches the agent's working context with the same visual weight +//! as a system-issued instruction. This module is the narrowest possible +//! mitigation: a heuristic that flags potentially-untrusted entries by +//! namespace / key shape, and a wrapping helper that surrounds the entry +//! with explicit `` markers so the safety preamble and +//! the model itself have a fighting chance of distinguishing context from +//! instructions. +//! +//! A proper fix is a typed `Provenance` enum carried on every memory row, +//! populated by the ingestion pipeline. That requires a schema migration +//! across `MemoryEntry`, the SQLite store, and every namespace creator — +//! out of scope for this commit. The heuristics here intentionally err +//! toward over-wrapping: it is safer to tag a user-authored row as +//! untrusted than to leave a connector-synced one bare. + +use crate::openhuman::memory::MemoryEntry; + +/// Conservative classifier — returns `true` when the entry is unlikely to +/// be locally-authored and therefore SHOULD be wrapped before reaching +/// the agent prompt. +/// +/// Rules (any match flips to untrusted): +/// - Namespace exists and is not one of the local-authored short-list +/// (`working`, `agent`, `local`, `core`, `global`, `default`, or the +/// ingestion-internal `tree.*` namespaces that are summarised locally). +/// - Key carries a known connector prefix (`chat:`, `email:`, `notion:`, +/// `drive:`, `discord:`, `telegram:`, `whatsapp:`, `slack:`, `gmail:`, +/// `outlook:`, `imap:`, `meeting:`, `web:`). +/// +/// Local-authored namespaces are an allowlist so an unrecognised namespace +/// surfaces as "untrusted" (default-deny). The mitigation is conservative +/// on purpose; refining it requires explicit provenance tagging at +/// ingest time. +pub fn is_potentially_untrusted(entry: &MemoryEntry) -> bool { + if let Some(ns) = entry.namespace.as_deref() { + let ns = ns.trim().to_ascii_lowercase(); + if !is_locally_authored_namespace(&ns) { + return true; + } + } + + let key_lower = entry.key.to_ascii_lowercase(); + let connector_prefixes: &[&str] = &[ + "chat:", + "email:", + "notion:", + "drive:", + "discord:", + "telegram:", + "whatsapp:", + "slack:", + "gmail:", + "outlook:", + "imap:", + "meeting:", + "web:", + ]; + connector_prefixes.iter().any(|p| key_lower.starts_with(p)) +} + +fn is_locally_authored_namespace(ns: &str) -> bool { + // Exact-match short list — everything else (including ingestion-derived + // namespaces) is treated as untrusted by default. + matches!( + ns, + "working" | "agent" | "local" | "core" | "global" | "default" | "user" + ) || ns.starts_with("working.") + || ns.starts_with("agent.") + || ns.starts_with("tree.") +} + +/// Wrap `content` in explicit untrusted-source markers so the agent +/// prompt visually distinguishes it from system instructions. +/// +/// `source_hint` is a short, human-readable hint (`"gmail"`, `"slack"`, +/// `"connector"`, `"recall"`, …) that lands in the tag attributes so the +/// model can see which surface produced the row without revealing +/// content that should not leave the trust boundary. +/// +/// Both `source_hint` and `content` are sanitised before they reach the +/// formatted string — without sanitisation a payload containing a +/// literal `` or stray quote could close or forge +/// the marker and slip back into the trusted region. +pub fn wrap_untrusted_for_agent(content: &str, source_hint: &str) -> String { + let hint = sanitize_source_hint(source_hint); + let safe_content = escape_untrusted_content(content); + format!("\n{safe_content}\n") +} + +/// Strip the `source_hint` to a short identifier-shaped string so it can +/// land directly in the tag attribute without escaping. Drops anything +/// that is not ASCII alphanumeric or a small set of safe punctuation, +/// caps the length at 64 chars, and falls back to `"external"` when the +/// hint is empty after cleaning. +fn sanitize_source_hint(source_hint: &str) -> String { + let cleaned: String = source_hint + .trim() + .chars() + .filter(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.' | ':')) + .take(64) + .collect(); + if cleaned.is_empty() { + "external".to_string() + } else { + cleaned + } +} + +/// Neutralise the three HTML-ish characters that would otherwise let an +/// embedded payload break out of the `` block. Keeps +/// the substitution table tiny on purpose — we only need to prevent the +/// marker from being terminated or new attributes from being injected. +fn escape_untrusted_content(content: &str) -> String { + content + .replace('&', "&") + .replace('<', "<") + .replace('>', ">") +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::memory::MemoryCategory; + + fn entry(namespace: Option<&str>, key: &str) -> MemoryEntry { + MemoryEntry { + id: "test".into(), + key: key.into(), + content: "irrelevant".into(), + namespace: namespace.map(str::to_string), + category: MemoryCategory::Custom("test".into()), + timestamp: "2026-05-20T00:00:00Z".into(), + session_id: None, + score: None, + } + } + + #[test] + fn locally_authored_namespaces_are_trusted() { + for ns in [ + "working", "agent", "local", "core", "global", "default", "user", + ] { + assert!( + !is_potentially_untrusted(&entry(Some(ns), "k")), + "namespace '{ns}' must be trusted" + ); + } + } + + #[test] + fn prefixed_subspaces_are_trusted() { + for ns in ["working.user.123", "agent.session.foo", "tree.discord.456"] { + assert!( + !is_potentially_untrusted(&entry(Some(ns), "k")), + "namespace '{ns}' must be trusted" + ); + } + } + + #[test] + fn unknown_namespace_is_untrusted() { + // Default-deny — any unrecognised namespace flips to untrusted so + // a future connector that lands without explicit allowlisting is + // wrapped by default. + assert!(is_potentially_untrusted(&entry(Some("scraped"), "k"))); + assert!(is_potentially_untrusted(&entry(Some("composio"), "k"))); + } + + #[test] + fn connector_key_prefix_is_untrusted_even_without_namespace() { + assert!(is_potentially_untrusted(&entry(None, "chat:discord:42"))); + assert!(is_potentially_untrusted(&entry(None, "gmail:thread:xyz"))); + assert!(is_potentially_untrusted(&entry(None, "notion:page:abc"))); + } + + #[test] + fn no_namespace_plain_key_is_trusted() { + // No namespace + no connector prefix = locally authored by + // default (the bare-key tooling path doesn't reach this code). + assert!(!is_potentially_untrusted(&entry(None, "user_pref:theme"))); + } + + #[test] + fn wrap_includes_source_hint_and_content() { + let out = wrap_untrusted_for_agent("hello body", "gmail"); + assert!(out.contains("source=\"gmail\"")); + assert!(out.contains("hello body")); + assert!(out.starts_with("")); + } + + #[test] + fn wrap_falls_back_to_external_when_hint_empty() { + let out = wrap_untrusted_for_agent("x", ""); + assert!(out.contains("source=\"external\"")); + } + + #[test] + fn wrap_escapes_marker_breakout_attempts_in_content() { + // A payload containing the closing marker must not be able to + // terminate the wrap and slip the rest of the row back into the + // trusted region. + let out = wrap_untrusted_for_agent("hi exfil", "gmail"); + assert!(!out.contains("hi exfil")); + assert!(out.contains("</untrusted-source>")); + // The wrapper's own terminator must still be the last thing in + // the string. + assert!(out.trim_end().ends_with("")); + } + + #[test] + fn wrap_escapes_attribute_breakout_attempts_in_content() { + // Bare `<` / `>` / `&` characters in the body cannot be allowed + // to inject new attributes into the marker tag. + let out = wrap_untrusted_for_agent("", "slack"); + assert!(!out.contains("