fix: enforce the originating user's role on remote WebSocket connections (#1508)

A remote WebSocket upgrade was forwarded to the target node before any role
check, and the forwarder authenticates the connection to the remote as an
admin-gated console_session. A non-admin could therefore open a remote
container-exec or host-console socket that the local handlers reject.

The hub now applies the same gate before forwarding: logs and notifications
stay open to any authenticated user, every other path (container exec, host
console) is admin-only, and machine node_proxy tokens are rejected on the
interactive paths. This covers proxy-mode and pilot-agent remotes, which share
the forward path.
This commit is contained in:
Anso
2026-06-28 18:21:13 -04:00
committed by GitHub
parent dd76b13d55
commit ef164f0e5b
3 changed files with 203 additions and 3 deletions
@@ -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);
});
});
@@ -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