diff --git a/backend/src/__tests__/remote-ws-forward-gate.test.ts b/backend/src/__tests__/remote-ws-forward-gate.test.ts new file mode 100644 index 00000000..1b752ce8 --- /dev/null +++ b/backend/src/__tests__/remote-ws-forward-gate.test.ts @@ -0,0 +1,69 @@ +/** + * Unit coverage for the remote-WebSocket forward gate. The hub must enforce the + * originating user's role before forwarding a remote upgrade, because the + * forwarder authenticates the forwarded connection to the remote as an + * admin-gated console_session. Logs and notifications stay open to any + * authenticated user; every other path is an interactive terminal (container + * exec / host console) and is admin-only on the hub. + */ +import { describe, it, expect } from 'vitest'; +import { remoteWsForwardAllowed } from '../websocket/upgradeHandler'; + +const viewer = { role: 'viewer' as const }; +const admin = { role: 'admin' as const }; +const base = { wsResolvedUser: undefined, wsApiTokenScope: null, isProxyToken: false, decoded: {} }; + +const LOGS = '/api/stacks/web/logs'; +const NOTIF = '/ws/notifications'; +const EXEC = '/ws'; +const CONSOLE = '/api/system/host-console'; + +describe('remoteWsForwardAllowed', () => { + it('allows logs and notifications for any authenticated user', () => { + for (const p of [LOGS, NOTIF]) { + expect(remoteWsForwardAllowed(p, { ...base, wsResolvedUser: viewer })).toBe(true); + } + }); + + it('denies interactive exec/console to every non-admin role (none holds system:console)', () => { + for (const role of ['viewer', 'deployer', 'node-admin', 'auditor'] as const) { + for (const p of [EXEC, CONSOLE]) { + expect(remoteWsForwardAllowed(p, { ...base, wsResolvedUser: { role } })).toBe(false); + } + } + }); + + it('allows interactive exec/console to an admin user session', () => { + for (const p of [EXEC, CONSOLE]) { + expect(remoteWsForwardAllowed(p, { ...base, wsResolvedUser: admin })).toBe(true); + } + }); + + it('gates host console on system:console, not merely role==admin (parity with handleHostConsoleWs)', async () => { + // node-admin is the meaningful case: broad stack/node permissions but no + // system:console, so it must be denied the remote host console. + const { ROLE_PERMISSIONS } = await import('../middleware/permissions'); + for (const role of ['admin', 'viewer', 'deployer', 'node-admin', 'auditor'] as const) { + const expected = ROLE_PERMISSIONS[role].includes('system:console'); + expect(remoteWsForwardAllowed(CONSOLE, { ...base, wsResolvedUser: { role } })).toBe(expected); + } + }); + + it('denies an interactive path to a node_proxy (machine) token', () => { + expect(remoteWsForwardAllowed(EXEC, { ...base, isProxyToken: true, decoded: { scope: 'node_proxy' } })).toBe(false); + }); + + it('allows an interactive path for a full-admin api token but denies a restricted scope', () => { + expect(remoteWsForwardAllowed(EXEC, { ...base, wsApiTokenScope: 'full-admin', decoded: { scope: 'api_token' } })).toBe(true); + expect(remoteWsForwardAllowed(EXEC, { ...base, wsApiTokenScope: 'deploy-only', decoded: { scope: 'api_token' } })).toBe(false); + }); + + it('allows an interactive path for a pre-gated console_session token', () => { + expect(remoteWsForwardAllowed(CONSOLE, { ...base, decoded: { scope: 'console_session' } })).toBe(true); + }); + + it('treats an unknown path as interactive (admin-only)', () => { + expect(remoteWsForwardAllowed('/ws/unknown', { ...base, wsResolvedUser: viewer })).toBe(false); + expect(remoteWsForwardAllowed('/ws/unknown', { ...base, wsResolvedUser: admin })).toBe(true); + }); +}); diff --git a/backend/src/__tests__/upgrade-order.test.ts b/backend/src/__tests__/upgrade-order.test.ts index 82825014..dc5a6056 100644 --- a/backend/src/__tests__/upgrade-order.test.ts +++ b/backend/src/__tests__/upgrade-order.test.ts @@ -11,6 +11,7 @@ import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; import WebSocket from 'ws'; import jwt from 'jsonwebtoken'; +import bcrypt from 'bcrypt'; import crypto from 'crypto'; import type { AddressInfo } from 'net'; import { setupTestDb, cleanupTestDb, TEST_USERNAME, TEST_JWT_SECRET } from './helpers/setupTestDb'; @@ -354,6 +355,73 @@ describe('WebSocket upgrade dispatch order', () => { }); }); + describe('remote WebSocket actor authorization', () => { + // The hub must enforce the originating user's role before forwarding a + // remote upgrade: the forwarder authenticates the connection to the remote + // as an admin-gated console_session, so a non-admin's remote container-exec + // or host-console would otherwise open with admin rights on the remote. A + // denied request is a clean 403 from the dispatcher; an allowed one is + // forwarded and then fails against the unreachable test remote (not a 403). + let viewerCookie: string; + + beforeAll(async () => { + const { DatabaseService } = await import('../services/DatabaseService'); + const db = DatabaseService.getInstance(); + const hash = await bcrypt.hash('viewerpass', 1); + db.addUser({ username: 'ws-authz-viewer', password_hash: hash, role: 'viewer' }); + const viewer = db.getUserByUsername('ws-authz-viewer')!; + const viewerToken = jwt.sign( + { username: 'ws-authz-viewer', role: 'viewer', tv: viewer.token_version }, + TEST_JWT_SECRET, + { expiresIn: '1m' }, + ); + viewerCookie = `sencho_token=${viewerToken}`; + }); + + it('rejects a non-admin remote container-exec upgrade with 403 before forwarding', async () => { + const ws = connect(`/ws?nodeId=${remoteNodeId}`, { cookie: viewerCookie }); + const outcome = await waitForOutcome(ws); + expect(outcome.kind).toBe('unexpected'); + if (outcome.kind === 'unexpected') expect(outcome.status).toBe(403); + }); + + it('rejects a non-admin remote host-console upgrade with 403 before forwarding', async () => { + const ws = connect(`/api/system/host-console?nodeId=${remoteNodeId}`, { cookie: viewerCookie }); + const outcome = await waitForOutcome(ws); + expect(outcome.kind).toBe('unexpected'); + if (outcome.kind === 'unexpected') expect(outcome.status).toBe(403); + }); + + it('lets a non-admin remote logs upgrade past the gate (forwarding fails on the dead remote, not a 403)', async () => { + const ws = connect(`/api/stacks/web/logs?nodeId=${remoteNodeId}`, { cookie: viewerCookie }); + const outcome = await waitForOutcome(ws); + if (outcome.kind === 'unexpected') expect(outcome.status).not.toBe(403); + try { ws.terminate(); } catch { /* ignore */ } + }); + + it('lets an admin remote container-exec upgrade past the gate (not a 403)', async () => { + const ws = connect(`/ws?nodeId=${remoteNodeId}`, { cookie: sessionCookie }); + const outcome = await waitForOutcome(ws); + if (outcome.kind === 'unexpected') expect(outcome.status).not.toBe(403); + try { ws.terminate(); } catch { /* ignore */ } + }); + + it('rejects a non-admin remote exec to a pilot node with 403 before the missing-tunnel 503', async () => { + // The gate runs before proxy-target resolution, so a viewer is denied with + // 403 even when the pilot tunnel is down (which would otherwise be 503). + // The forward path is shared with proxy mode, so this covers pilot agents. + const { DatabaseService } = await import('../services/DatabaseService'); + const pilotId = DatabaseService.getInstance().addNode({ + name: `ws-authz-pilot-${Date.now()}`, type: 'remote', mode: 'pilot_agent', + compose_dir: '/tmp/x', is_default: false, api_url: '', api_token: '', + }); + const ws = connect(`/ws?nodeId=${pilotId}`, { cookie: viewerCookie }); + const outcome = await waitForOutcome(ws); + expect(outcome.kind).toBe('unexpected'); + if (outcome.kind === 'unexpected') expect(outcome.status).toBe(403); + }); + }); + it('dispatches /api/pilot/tunnel to the pilot handler (rejects non-pilot bearer before path-based dispatch)', async () => { // A plain session cookie is a valid *user* JWT but not a pilot JWT. The // pilot handler runs first (before the shared cookie/Bearer auth) and diff --git a/backend/src/websocket/upgradeHandler.ts b/backend/src/websocket/upgradeHandler.ts index 7245770a..e23a7b55 100644 --- a/backend/src/websocket/upgradeHandler.ts +++ b/backend/src/websocket/upgradeHandler.ts @@ -13,6 +13,7 @@ import { handleRemoteForwarder } from './remoteForwarder'; import { handleLogsWs } from './logs'; import { handleHostConsoleWs } from './hostConsole'; import { handleGenericWs, attachGenericConnectionHandlers } from './generic'; +import { ROLE_PERMISSIONS } from '../middleware/permissions'; import { rejectUpgrade as reject } from './reject'; import { looksLikeApiToken } from '../utils/apiTokenFormat'; import { validateApiToken, touchApiTokenLastUsed } from '../utils/apiTokenAuth'; @@ -30,6 +31,59 @@ function parseCookies(req: IncomingMessage): Record { ); } +// The two WebSocket paths open to any authenticated user (read-only/deploy-only +// API tokens and non-admin sessions on a remote node). Defined once so the +// scope gate and the remote-forward gate cannot drift apart. +function isLogsPath(pathname: string): boolean { + return /^\/api\/stacks\/[^/]+\/logs$/.test(pathname); +} +function isNotificationsPath(pathname: string): boolean { + return pathname === '/ws/notifications'; +} + +/** + * Decide whether a principal may open a WebSocket to a REMOTE node, applying + * the same gate the local handlers apply before the request is forwarded. + * + * Logs and notifications are the only remote paths a non-admin may open (they + * mirror handleLogsWs and the notifications channel, neither of which gates on + * admin). Every other path is treated as an interactive terminal (container + * exec via `/ws` or host console) that handleGenericWs / handleHostConsoleWs + * gate on admin / system:console; the remote authenticates the forwarded + * upgrade as an admin-gated console_session, so the hub must apply that gate + * itself. Machine (node_proxy) tokens are rejected on interactive paths, + * matching local. + */ +export function remoteWsForwardAllowed( + pathname: string, + ctx: { + wsResolvedUser?: { role: UserRole }; + wsApiTokenScope: string | null; + isProxyToken: boolean; + decoded: { scope?: string }; + }, +): boolean { + if (isLogsPath(pathname) || isNotificationsPath(pathname)) return true; + // Interactive terminal path (or any unrecognized path): admin-gated on the hub. + if (ctx.isProxyToken) return false; + // console_session is pre-gated as admin at issuance (routes/console.ts). + if (ctx.decoded.scope === 'console_session') return true; + // A read-only/deploy-only api_token is already rejected for these paths by + // the scope gate; only full-admin survives to here. + if (ctx.wsApiTokenScope) return ctx.wsApiTokenScope === 'full-admin'; + const role = ctx.wsResolvedUser?.role; + if (!role) return false; + // Mirror the exact local authority: host console requires system:console + // (handleHostConsoleWs); container exec and everything else require admin + // (handleGenericWs). These coincide today (only admin holds system:console), + // but keying off the permission keeps the remote gate in lockstep if the + // role table changes. + if (pathname.startsWith('/api/system/host-console')) { + return ROLE_PERMISSIONS[role]?.includes('system:console') ?? false; + } + return role === 'admin'; +} + /** * Attach the upgrade dispatcher to the HTTP server and wire the generic * `connection` handler on the main wss. @@ -128,10 +182,8 @@ export function attachUpgrade( // Gate WebSocket paths by API token scope if (wsApiTokenScope) { - const isLogPath = /^\/api\/stacks\/[^/]+\/logs$/.test(pathname); - const isNotifPath = pathname === '/ws/notifications'; if (wsApiTokenScope === 'read-only' || wsApiTokenScope === 'deploy-only') { - if (!isLogPath && !isNotifPath) return reject(socket, 403, 'Forbidden'); + if (!isLogsPath(pathname) && !isNotificationsPath(pathname)) return reject(socket, 403, 'Forbidden'); } } @@ -180,6 +232,17 @@ export function attachUpgrade( } if (node && node.type === 'remote') { + // Enforce the originating user's role on the hub BEFORE forwarding. + // handleRemoteForwarder exchanges the node's api_token for an + // admin-gated console_session on interactive paths, so the remote + // accepts the upgrade without re-checking the user. Without this gate a + // non-admin could open a remote container-exec or host-console socket + // that the local handlers (handleGenericWs / handleHostConsoleWs) would + // have refused. Logs and notifications stay open to any authenticated + // user, mirroring their local handlers. + if (!remoteWsForwardAllowed(pathname, { wsResolvedUser, wsApiTokenScope, isProxyToken, decoded })) { + return reject(socket, 403, 'Forbidden'); + } // Resolve the proxy target through NodeRegistry so pilot-mode nodes // (empty api_url + api_token, loopback bridge instead) and proxy-mode // nodes share one dispatch path. Mirrors proxy/remoteNodeProxy.ts.