Skip to content

Commit bdc9943

Browse files
brookscclaude
brooksc
and
claude
committed
fix(security): mobile token, bind-address, route scoping, validator hardening
P1-1 StartMCPServer binds 127.0.0.1 by default; 0.0.0.0 only in Docker mode. StartRemoteServer keeps 0.0.0.0 (explicit user action for WiFi/mobile). P1-2 Coordinator token never returned to renderer. Removed 'token' field from StartRemoteServer, GetRemoteStatus, and StartMCPServer IPC returns. GetMCPStatus.serverUrl now uses getMCPRemoteServerUrl (no token). RemoteAccess store type and initial state updated accordingly. P1-3 Third mobileToken generated at startup. wifiUrl/tailscaleUrl/url all embed mobileToken instead of coordinator token. Mobile callers are classified as 'mobile' and restricted to GET /api/agents, GET /api/tasks (read-only). Only coordinator token grants WebSocket + write routes. P2-1 redactServerUrl() exported from server.ts; used in register.ts console.warn to strip token query param before logging. P2-2 validateBranchName rejects path traversal (/../) and shell metacharacters (`$(){}[]<>\\'*?!#;|&") in addition to existing control-char checks. P2-3 MCP_HydrateCoordinatedTask now calls validateBranchName on branchName and baseBranch (if present) — matches validation enforced by createTask. P2-4 PARALLEL_CODE_MCP_TOKEN added to SpawnAgent ENV_BLOCK_LIST so a crafted IPC call cannot override the MCP auth token in spawned agents. P2-5 X-Coordinator-Id header is now verified against registered coordinators (isRegisteredCoordinator) before being used for task scoping. Unregistered IDs are silently ignored — caller sees unscoped view. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a55c8fa commit bdc9943

9 files changed

Lines changed: 66 additions & 24 deletions

File tree

‎electron/ipc/pty.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,8 @@ export function spawnAgent(
232232
'DYLD_INSERT_LIBRARIES',
233233
'NODE_OPTIONS',
234234
'ELECTRON_RUN_AS_NODE',
235+
// Prevent renderer from injecting or overriding MCP auth tokens.
236+
'PARALLEL_CODE_MCP_TOKEN',
235237
]);
236238
const safeEnvOverrides: Record<string, string> = {};
237239
for (const [k, v] of Object.entries(args.env ?? {})) {

‎electron/ipc/register.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ import {
7979
import { warn as logWarn } from '../log.js';
8080
import { getMCPRemoteServerUrl, detectStaleDockerMCPUrl } from '../mcp/config.js';
8181
import { validateBranchName as sharedValidateBranchName } from '../mcp/validation.js';
82+
import { redactServerUrl } from '../remote/server.js';
8283

8384
export function selectMcpJsonDir(worktreePath: string | undefined, projectRoot: string): string {
8485
return worktreePath ?? projectRoot;
@@ -1021,12 +1022,13 @@ export function registerAllHandlers(win: BrowserWindow): void {
10211022
url: remoteServer.url,
10221023
wifiUrl: remoteServer.wifiUrl,
10231024
tailscaleUrl: remoteServer.tailscaleUrl,
1024-
token: remoteServer.token,
10251025
port: remoteServer.port,
10261026
};
10271027

10281028
const thisDir = path.dirname(fileURLToPath(import.meta.url));
10291029
const distRemote = path.join(thisDir, '..', '..', 'dist-remote');
1030+
// Remote access is an explicit user action — bind to all interfaces so WiFi/Tailscale clients
1031+
// can reach the SPA. Coordinator MCP-only mode uses 127.0.0.1 by default.
10301032
remoteServer = await startRemoteServer({
10311033
port: args.port ?? 7777,
10321034
host: '0.0.0.0',
@@ -1046,7 +1048,6 @@ export function registerAllHandlers(win: BrowserWindow): void {
10461048
url: remoteServer.url,
10471049
wifiUrl: remoteServer.wifiUrl,
10481050
tailscaleUrl: remoteServer.tailscaleUrl,
1049-
token: remoteServer.token,
10501051
port: remoteServer.port,
10511052
};
10521053
});
@@ -1077,7 +1078,6 @@ export function registerAllHandlers(win: BrowserWindow): void {
10771078
url: remoteServer.url,
10781079
wifiUrl: remoteServer.wifiUrl,
10791080
tailscaleUrl: remoteServer.tailscaleUrl,
1080-
token: remoteServer.token,
10811081
port: remoteServer.port,
10821082
};
10831083
});
@@ -1215,6 +1215,8 @@ export function registerAllHandlers(win: BrowserWindow): void {
12151215
assertString(args.projectId, 'projectId');
12161216
validatePath(args.projectRoot, 'projectRoot');
12171217
assertString(args.branchName, 'branchName');
1218+
sharedValidateBranchName(args.branchName, 'branchName');
1219+
if (args.baseBranch !== undefined) sharedValidateBranchName(args.baseBranch, 'baseBranch');
12181220
validatePath(args.worktreePath, 'worktreePath');
12191221
assertString(args.coordinatorTaskId, 'coordinatorTaskId');
12201222
coordinator?.hydrateTask({
@@ -1334,8 +1336,10 @@ export function registerAllHandlers(win: BrowserWindow): void {
13341336
if (!remoteServer) {
13351337
const thisDir = path.dirname(fileURLToPath(import.meta.url));
13361338
const distRemote = path.join(thisDir, '..', '..', 'dist-remote');
1339+
// Docker mode requires 0.0.0.0 so the agent inside the container can reach the host.
1340+
// Without Docker, bind to loopback — sub-agents are local processes.
13371341
remoteServer = await startRemoteServerOnFreePort(7777, 7800, {
1338-
host: '0.0.0.0',
1342+
host: args.dockerContainerName ? '0.0.0.0' : '127.0.0.1',
13391343
staticDir: distRemote,
13401344
getTaskName: (taskId: string) => taskNames.get(taskId) ?? taskId,
13411345
getAgentStatus: (agentId: string) => {
@@ -1510,12 +1514,11 @@ export function registerAllHandlers(win: BrowserWindow): void {
15101514
console.warn('[MCP] Config written to:', configPath);
15111515
}
15121516
console.warn('[MCP] Server path:', mcpServerPath);
1513-
console.warn('[MCP] Remote URL:', serverUrl);
1517+
console.warn('[MCP] Remote URL:', redactServerUrl(serverUrl));
15141518

15151519
return {
15161520
configPath,
15171521
serverUrl,
1518-
token: remoteServer.token,
15191522
port: remoteServer.port,
15201523
};
15211524
},
@@ -1536,7 +1539,7 @@ export function registerAllHandlers(win: BrowserWindow): void {
15361539
remoteRunning,
15371540
coordinatorRoutesAttached: coordinator !== null,
15381541
coordinatorRegistered: coordinator?.hasActiveCoordinator() ?? false,
1539-
serverUrl: remoteServer?.url ?? null,
1542+
serverUrl: remoteServer ? getMCPRemoteServerUrl(remoteServer.port) : null,
15401543
mcpConfigPath: lastMcpConfigPath,
15411544
};
15421545
});

‎electron/mcp/coordinator.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,6 +1387,10 @@ export class Coordinator {
13871387
}
13881388
}
13891389

1390+
isRegisteredCoordinator(coordinatorTaskId: string): boolean {
1391+
return this.coordinators.has(coordinatorTaskId);
1392+
}
1393+
13901394
registerCoordinator(
13911395
coordinatorTaskId: string,
13921396
projectId: string,

‎electron/mcp/validation.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@ export function validateBranchName(value: unknown, field = 'baseBranch'): string
55
if (typeof value !== 'string') throw new Error(`${field} must be a string`);
66
if (!value.trim()) throw new Error(`${field} must be a non-empty string`);
77
if (value.startsWith('-')) throw new Error(`${field} must not start with "-"`);
8-
// Reject ASCII control characters (0x00–0x1f, 0x7f) and spaces.
98
// eslint-disable-next-line no-control-regex
109
if (/[\x00-\x1f\x7f ]/.test(value)) throw new Error(`${field} contains invalid characters`);
10+
// Reject path traversal sequences.
11+
if (/(?:^|\/)\.\.(?:\/|$)/.test(value)) throw new Error(`${field} contains path traversal`);
12+
// Reject shell metacharacters that could be dangerous in command arguments.
13+
if (/[`$(){}[\]<>\\'*?!#;|&"]/.test(value))
14+
throw new Error(`${field} contains invalid characters`);
1115
return value;
1216
}

‎electron/remote/coordinator-scoping.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ function makeMockCoordinator(): Coordinator {
7777
]);
7878

7979
return {
80+
isRegisteredCoordinator: (id: string) => id === COORD_A || id === COORD_B,
8081
listTasks: () => [summaryA, summaryB],
8182
getTaskStatus: (id: string) => tasks.get(id) ?? null,
8283
sendPrompt: vi.fn().mockResolvedValue(undefined),

‎electron/remote/server.ts‎

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,17 @@ export function getMCPLogs(): MCPLogEntry[] {
4444
return mcpLogs.slice();
4545
}
4646

47+
/** Strip the token query param before logging or displaying a server URL. */
48+
export function redactServerUrl(rawUrl: string): string {
49+
try {
50+
const u = new URL(rawUrl);
51+
u.searchParams.delete('token');
52+
return u.toString();
53+
} catch {
54+
return rawUrl;
55+
}
56+
}
57+
4758
const MIME: Record<string, string> = {
4859
'.html': 'text/html',
4960
'.js': 'application/javascript',
@@ -58,7 +69,9 @@ interface RemoteServer {
5869
stop: () => Promise<void>;
5970
token: string;
6071
subtaskToken: string;
72+
mobileToken: string;
6173
port: number;
74+
/** Mobile-scoped URL (embedded mobileToken). Safe to send to the renderer. */
6275
url: string;
6376
tailscaleUrl: string | null;
6477
wifiUrl: string | null;
@@ -132,23 +145,27 @@ export function startRemoteServer(opts: {
132145
}): Promise<RemoteServer> {
133146
const token = randomBytes(24).toString('base64url');
134147
const subtaskToken = randomBytes(24).toString('base64url');
148+
const mobileToken = randomBytes(24).toString('base64url');
135149
const ips = getNetworkIps();
136150

137151
const tokenBuf = Buffer.from(token);
138152
const subtaskTokenBuf = Buffer.from(subtaskToken);
153+
const mobileTokenBuf = Buffer.from(mobileToken);
139154

140155
function classifyCandidate(
141156
candidate: string | null | undefined,
142-
): 'coordinator' | 'subtask' | null {
157+
): 'coordinator' | 'subtask' | 'mobile' | null {
143158
if (!candidate) return null;
144159
const buf = Buffer.from(candidate);
145160
if (buf.length === tokenBuf.length && timingSafeEqual(buf, tokenBuf)) return 'coordinator';
146161
if (buf.length === subtaskTokenBuf.length && timingSafeEqual(buf, subtaskTokenBuf))
147162
return 'subtask';
163+
if (buf.length === mobileTokenBuf.length && timingSafeEqual(buf, mobileTokenBuf))
164+
return 'mobile';
148165
return null;
149166
}
150167

151-
function classifyToken(req: IncomingMessage): 'coordinator' | 'subtask' | null {
168+
function classifyToken(req: IncomingMessage): 'coordinator' | 'subtask' | 'mobile' | null {
152169
const auth = req.headers.authorization;
153170
if (auth?.startsWith('Bearer ')) return classifyCandidate(auth.slice(7));
154171
const url = new URL(req.url ?? '/', `http://${req.headers.host ?? 'localhost'}`);
@@ -180,6 +197,20 @@ export function startRemoteServer(opts: {
180197
return;
181198
}
182199
}
200+
// Mobile token: read-only access to agent and task status.
201+
if (tokenClass === 'mobile') {
202+
const allowed =
203+
req.method === 'GET' &&
204+
(url.pathname === '/api/agents' ||
205+
/^\/api\/agents\/[^/]+$/.test(url.pathname) ||
206+
url.pathname === '/api/tasks' ||
207+
/^\/api\/tasks\/[^/]+$/.test(url.pathname));
208+
if (!allowed) {
209+
res.writeHead(403, { ...SECURITY_HEADERS, 'Content-Type': 'application/json' });
210+
res.end(JSON.stringify({ error: 'forbidden' }));
211+
return;
212+
}
213+
}
183214

184215
if (url.pathname === '/api/agents' && req.method === 'GET') {
185216
const list = buildAgentList(opts.getTaskName, opts.getAgentStatus);
@@ -328,10 +359,12 @@ export function startRemoteServer(opts: {
328359
}
329360

330361
// Extract the coordinator ID from the header (set by MCP coordinator clients).
331-
// When absent (REST/mobile callers), no scoping is applied.
362+
// Only honor it if it is a registered coordinator — prevents a caller from
363+
// injecting an arbitrary ID to scope against another coordinator's tasks.
332364
const callerCoordinatorId = (() => {
333365
const h = req.headers['x-coordinator-id'];
334-
return typeof h === 'string' && h ? h : undefined;
366+
if (typeof h !== 'string' || !h) return undefined;
367+
return orch.isRegisteredCoordinator(h) ? h : undefined;
335368
})();
336369

337370
const ownedByCallerOrUnscoped = (taskCoordinatorId: string): boolean =>
@@ -652,7 +685,7 @@ export function startRemoteServer(opts: {
652685
clientSubs.set(ws, new Map());
653686

654687
// Support legacy URL-based auth (verifyClient accepted all connections).
655-
// Subtask tokens are denied WS access — they only need REST signal_done.
688+
// Only coordinator token grants WS access; subtask and mobile tokens are denied.
656689
if (classifyToken(req) === 'coordinator') {
657690
authenticatedClients.add(ws);
658691
const list = buildAgentList(opts.getTaskName, opts.getAgentStatus);
@@ -671,7 +704,7 @@ export function startRemoteServer(opts: {
671704
const msg = parseClientMessage(String(raw));
672705
if (!msg) return;
673706

674-
// Handle first-message auth. Subtask tokens are denied WS access.
707+
// Handle first-message auth. Only coordinator token grants WS access.
675708
if (msg.type === 'auth') {
676709
if (classifyCandidate(msg.token) === 'coordinator') {
677710
authenticatedClients.add(ws);
@@ -779,21 +812,23 @@ export function startRemoteServer(opts: {
779812
});
780813

781814
const primaryIp = ips.wifi ?? ips.tailscale ?? '127.0.0.1';
782-
const url = `http://${primaryIp}:${opts.port}?token=${token}`;
815+
// url embeds the mobileToken — safe to surface in UI. Coordinator token never leaves the main process.
816+
const url = `http://${primaryIp}:${opts.port}?token=${mobileToken}`;
783817

784818
const result: RemoteServer = {
785819
token,
786820
subtaskToken,
821+
mobileToken,
787822
port: opts.port,
788823
url,
789824
/** Re-detect network IPs so newly connected interfaces (e.g. Tailscale) are picked up. */
790825
get wifiUrl() {
791826
const cur = getNetworkIps();
792-
return cur.wifi ? `http://${cur.wifi}:${opts.port}?token=${token}` : null;
827+
return cur.wifi ? `http://${cur.wifi}:${opts.port}?token=${mobileToken}` : null;
793828
},
794829
get tailscaleUrl() {
795830
const cur = getNetworkIps();
796-
return cur.tailscale ? `http://${cur.tailscale}:${opts.port}?token=${token}` : null;
831+
return cur.tailscale ? `http://${cur.tailscale}:${opts.port}?token=${mobileToken}` : null;
797832
},
798833
connectedClients: () => authenticatedClients.size,
799834
stop: () =>

‎src/store/core.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ export const [store, setStore] = createStore<AppStore>({
5858
missingProjectIds: {},
5959
remoteAccess: {
6060
enabled: false,
61-
token: null,
6261
port: 7777,
6362
url: null,
6463
wifiUrl: null,

‎src/store/remote.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ interface ServerResult {
66
url: string;
77
wifiUrl: string | null;
88
tailscaleUrl: string | null;
9-
token: string;
109
port: number;
1110
}
1211

@@ -18,7 +17,6 @@ export async function startRemoteAccess(port?: number): Promise<ServerResult> {
1817
const result = await invoke<ServerResult>(IPC.StartRemoteServer, port ? { port } : {});
1918
setStore('remoteAccess', {
2019
enabled: true,
21-
token: result.token,
2220
port: result.port,
2321
url: result.url,
2422
wifiUrl: result.wifiUrl,
@@ -34,7 +32,6 @@ export async function stopRemoteAccess(): Promise<{ stopped: boolean; reason?: s
3432
if (result.stopped) {
3533
setStore('remoteAccess', {
3634
enabled: false,
37-
token: null,
3835
port: 7777,
3936
url: null,
4037
wifiUrl: null,
@@ -53,7 +50,6 @@ export async function refreshRemoteStatus(): Promise<void> {
5350
url?: string;
5451
wifiUrl?: string;
5552
tailscaleUrl?: string;
56-
token?: string;
5753
port?: number;
5854
}>(IPC.GetRemoteStatus);
5955

@@ -67,7 +63,6 @@ export async function refreshRemoteStatus(): Promise<void> {
6763
url: result.url ?? null,
6864
wifiUrl: result.wifiUrl ?? null,
6965
tailscaleUrl: result.tailscaleUrl ?? null,
70-
token: result.token ?? null,
7166
port: result.port ?? 7777,
7267
});
7368
} else {

‎src/store/types.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ export interface PendingAction {
216216

217217
export interface RemoteAccess {
218218
enabled: boolean;
219-
token: string | null;
220219
port: number;
221220
url: string | null;
222221
wifiUrl: string | null;

0 commit comments

Comments
 (0)