Skip to content

Commit fa46d9b

Browse files
committed
fix(remote): restore mobile terminal input
1 parent 5d7c22f commit fa46d9b

4 files changed

Lines changed: 356 additions & 64 deletions

File tree

electron/remote/server-ws.test.ts

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// WebSocket-level access control for the remote server.
2+
// Mobile clients may type into agent terminals (input) but must not be able
3+
// to resize the PTY or kill agents.
4+
5+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
6+
import WebSocket from 'ws';
7+
8+
vi.mock('../ipc/pty.js', () => ({
9+
writeToAgent: vi.fn(),
10+
resizeAgent: vi.fn(),
11+
killAgent: vi.fn(),
12+
subscribeToAgent: vi.fn(),
13+
unsubscribeFromAgent: vi.fn(),
14+
getAgentScrollback: vi.fn(() => null),
15+
getActiveAgentIds: vi.fn(() => []),
16+
getAgentMeta: vi.fn(() => null),
17+
getAgentCols: vi.fn(() => 80),
18+
onPtyEvent: vi.fn(() => vi.fn()), // returns an unsubscribe fn
19+
}));
20+
21+
const pty = await import('../ipc/pty.js');
22+
const { startRemoteServer } = await import('./server.js');
23+
24+
let port = 0;
25+
let coordinatorToken = '';
26+
let mobileToken = '';
27+
let stop: () => Promise<void>;
28+
29+
beforeEach(async () => {
30+
const srv = await startRemoteServer({
31+
port: 0,
32+
host: '0.0.0.0',
33+
staticDir: '/nonexistent',
34+
getTaskName: (id) => id,
35+
getAgentStatus: () => ({ status: 'exited', exitCode: null, lastLine: '' }),
36+
getCoordinator: () => null,
37+
});
38+
port = srv.port;
39+
coordinatorToken = srv.token;
40+
mobileToken = srv.mobileToken;
41+
stop = srv.stop;
42+
vi.clearAllMocks();
43+
});
44+
45+
afterEach(async () => {
46+
await stop();
47+
});
48+
49+
/** Connect and authenticate; resolves once the server replies (agents list). */
50+
function connectAndAuth(token: string): Promise<WebSocket> {
51+
return new Promise((resolve, reject) => {
52+
const ws = new WebSocket(`ws://127.0.0.1:${port}/ws`);
53+
ws.on('open', () => ws.send(JSON.stringify({ type: 'auth', token })));
54+
ws.once('message', () => resolve(ws));
55+
ws.on('close', (code) => reject(new Error(`closed before auth ack: ${code}`)));
56+
ws.on('error', reject);
57+
});
58+
}
59+
60+
function waitForClose(ws: WebSocket): Promise<number> {
61+
return new Promise((resolve) => {
62+
// Drop connectAndAuth's rejecting close/error listeners — from here on
63+
// a close is the expected outcome, not a failure.
64+
ws.removeAllListeners('close');
65+
ws.removeAllListeners('error');
66+
ws.on('close', (code) => resolve(code));
67+
});
68+
}
69+
70+
describe('mobile token over WebSocket', () => {
71+
it('forwards input to the agent PTY', async () => {
72+
const ws = await connectAndAuth(mobileToken);
73+
ws.send(JSON.stringify({ type: 'input', agentId: 'agent-1', data: 'hi' }));
74+
await vi.waitFor(() => {
75+
expect(pty.writeToAgent).toHaveBeenCalledWith('agent-1', 'hi');
76+
});
77+
expect(ws.readyState).toBe(WebSocket.OPEN);
78+
ws.close();
79+
});
80+
81+
it('silently drops oversized input (>4096 chars) without closing', async () => {
82+
const ws = await connectAndAuth(mobileToken);
83+
ws.send(JSON.stringify({ type: 'input', agentId: 'agent-1', data: 'x'.repeat(4097) }));
84+
// Probe with a valid message to ensure the oversized one was processed first
85+
ws.send(JSON.stringify({ type: 'input', agentId: 'agent-1', data: 'ok' }));
86+
await vi.waitFor(() => {
87+
expect(pty.writeToAgent).toHaveBeenCalledWith('agent-1', 'ok');
88+
});
89+
expect(pty.writeToAgent).not.toHaveBeenCalledWith('agent-1', 'x'.repeat(4097));
90+
expect(ws.readyState).toBe(WebSocket.OPEN);
91+
ws.close();
92+
});
93+
94+
it('rejects resize with 4003 and does not resize the PTY', async () => {
95+
const ws = await connectAndAuth(mobileToken);
96+
const closed = waitForClose(ws);
97+
ws.send(JSON.stringify({ type: 'resize', agentId: 'agent-1', cols: 80, rows: 24 }));
98+
expect(await closed).toBe(4003);
99+
expect(pty.resizeAgent).not.toHaveBeenCalled();
100+
});
101+
102+
it('rejects kill with 4003 and does not kill the agent', async () => {
103+
const ws = await connectAndAuth(mobileToken);
104+
const closed = waitForClose(ws);
105+
ws.send(JSON.stringify({ type: 'kill', agentId: 'agent-1' }));
106+
expect(await closed).toBe(4003);
107+
expect(pty.killAgent).not.toHaveBeenCalled();
108+
});
109+
});
110+
111+
describe('coordinator token over WebSocket', () => {
112+
it('forwards input to the agent PTY', async () => {
113+
const ws = await connectAndAuth(coordinatorToken);
114+
ws.send(JSON.stringify({ type: 'input', agentId: 'agent-1', data: 'hello' }));
115+
await vi.waitFor(() => {
116+
expect(pty.writeToAgent).toHaveBeenCalledWith('agent-1', 'hello');
117+
});
118+
ws.close();
119+
});
120+
});

electron/remote/server.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,8 @@ export function startRemoteServer(opts: {
273273
return;
274274
}
275275
}
276-
// Mobile token: read-only access to agent status only.
276+
// Mobile token: read-only REST access to agent status only (terminal
277+
// input goes over the WebSocket, not REST).
277278
// Task/coordinator routes are intentionally excluded — the mobile view shows
278279
// agent terminals, not coordinator sub-tasks, and the mobile token is embedded
279280
// in a QR-code URL reachable by anyone on the local network.
@@ -852,9 +853,11 @@ export function startRemoteServer(opts: {
852853
return;
853854
}
854855

855-
// Mobile clients are read-only — block all PTY mutation messages
856+
// Mobile clients may type into agent terminals (`input`) but cannot
857+
// resize the PTY (desktop owns the geometry) or kill agents — the
858+
// mobile token travels in a QR-code URL, so keep its blast radius small.
856859
if (clientTokenTypes.get(ws) === 'mobile') {
857-
if (msg.type === 'input' || msg.type === 'resize' || msg.type === 'kill') {
860+
if (msg.type === 'resize' || msg.type === 'kill') {
858861
ws.close(4003, 'Forbidden');
859862
return;
860863
}

0 commit comments

Comments
 (0)