Skip to content

Commit f837953

Browse files
authored
fix: bind tunnel registry server to localhost only (#296)
1 parent 9dac5b4 commit f837953

2 files changed

Lines changed: 57 additions & 17 deletions

File tree

src/lib/tunnel/tunnel-registry-server.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {BaseItem, strongbox} from '@appium/strongbox';
55
import {TUNNEL_CONTAINER_NAME} from '../../constants.js';
66
import {getLogger} from '../logger.js';
77
import type {TunnelRegistry, TunnelRegistryEntry} from '../types.js';
8-
import {MAX_TUNNEL_REGISTRY_WAIT_MS, TUNNEL_REGISTRY_API_BASE_PATH} from './constants.js';
8+
import {MAX_TUNNEL_REGISTRY_WAIT_MS, TUNNEL_REGISTRY_API_BASE_PATH, TUNNEL_REGISTRY_HOST} from './constants.js';
99
import {isTunnelEntryReady} from './tunnel-availability.js';
1010
import {TunnelReadinessCoordinator} from './tunnel-readiness.js';
1111
import {type RouteRecord, createRouteDispatcher, getRequestPathname} from './tunnel-registry-routes.js';
@@ -127,10 +127,12 @@ export class TunnelRegistryServer {
127127
await this.handleRequest(req, res);
128128
});
129129

130-
// Start listening
130+
// Start listening on localhost only: the API exposes unauthenticated
131+
// writes (PUT /:udid), so binding all interfaces would let any LAN host
132+
// overwrite tunnel address/port entries.
131133
await new Promise<void>((resolve, reject) => {
132-
this.server?.listen(this.port, () => {
133-
log.info(`Tunnel Registry Server started on port ${this.port}`);
134+
this.server?.listen(this.port, TUNNEL_REGISTRY_HOST, () => {
135+
log.info(`Tunnel Registry Server started on ${TUNNEL_REGISTRY_HOST}:${this.port}`);
134136
log.info(`API available at http://localhost:${this.port}${TUNNEL_REGISTRY_API_BASE_PATH}`);
135137
resolve();
136138
});

test/unit/tunnel/tunnel-registry-server.spec.ts

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import assert from 'node:assert/strict';
2+
import * as net from 'node:net';
3+
import * as os from 'node:os';
24
import {afterEach, beforeEach, describe, it} from 'node:test';
35

46
import {type TunnelRegistryServer, startTunnelRegistryServer} from '../../../src/lib/tunnel/tunnel-registry-server.js';
@@ -45,7 +47,7 @@ describe('TunnelRegistryServer', function () {
4547

4648
describe('GET /remotexpc/tunnels', function () {
4749
it('should return all tunnels', async function () {
48-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels`);
50+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels`);
4951
const data = (await response.json()) as TunnelRegistry;
5052

5153
assert.strictEqual(response.status, 200);
@@ -57,7 +59,7 @@ describe('TunnelRegistryServer', function () {
5759

5860
describe('GET /remotexpc/tunnels/metadata', function () {
5961
it('should return registry metadata', async function () {
60-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels/metadata`);
62+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels/metadata`);
6163
const data = (await response.json()) as TunnelRegistry['metadata'] & {
6264
status: string;
6365
};
@@ -73,7 +75,7 @@ describe('TunnelRegistryServer', function () {
7375

7476
describe('GET /remotexpc/tunnels/:udid', function () {
7577
it('should return tunnel by UDID', async function () {
76-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels/test-udid-123`);
78+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels/test-udid-123`);
7779
const data = (await response.json()) as TunnelRegistryEntry;
7880

7981
assert.strictEqual(response.status, 200);
@@ -82,7 +84,7 @@ describe('TunnelRegistryServer', function () {
8284
});
8385

8486
it('should return 404 for non-existent UDID', async function () {
85-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels/non-existent`);
87+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels/non-existent`);
8688
const data = (await response.json()) as {error: string};
8789

8890
assert.strictEqual(response.status, 404);
@@ -111,7 +113,7 @@ describe('TunnelRegistryServer', function () {
111113
const pendingServer = await startTunnelRegistryServer(pendingRegistry, pendingPort);
112114

113115
try {
114-
const response = await fetch(`http://localhost:${pendingPort}/remotexpc/tunnels/pending-udid?waitMs=0`);
116+
const response = await fetch(`http://127.0.0.1:${pendingPort}/remotexpc/tunnels/pending-udid?waitMs=0`);
115117
const data = (await response.json()) as {error: string};
116118

117119
assert.strictEqual(response.status, 404);
@@ -147,7 +149,7 @@ describe('TunnelRegistryServer', function () {
147149

148150
try {
149151
const response = await fetch(
150-
`http://localhost:${refreshPort}/remotexpc/tunnels/refresh-udid/refresh-services`,
152+
`http://127.0.0.1:${refreshPort}/remotexpc/tunnels/refresh-udid/refresh-services`,
151153
{method: 'POST'},
152154
);
153155
const data = (await response.json()) as TunnelRegistryEntry;
@@ -162,7 +164,7 @@ describe('TunnelRegistryServer', function () {
162164

163165
describe('GET /remotexpc/tunnels/device/:deviceId', function () {
164166
it('should return tunnel by device ID', async function () {
165-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels/device/1`);
167+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels/device/1`);
166168
const data = (await response.json()) as TunnelRegistryEntry;
167169

168170
assert.strictEqual(response.status, 200);
@@ -171,7 +173,7 @@ describe('TunnelRegistryServer', function () {
171173
});
172174

173175
it('should return 404 for non-existent device ID', async function () {
174-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels/device/999`);
176+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels/device/999`);
175177
const data = (await response.json()) as {error: string};
176178

177179
assert.strictEqual(response.status, 404);
@@ -180,7 +182,7 @@ describe('TunnelRegistryServer', function () {
180182
});
181183

182184
it('should return 400 for invalid device ID', async function () {
183-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels/device/invalid`);
185+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels/device/invalid`);
184186
const data = (await response.json()) as {error: string};
185187

186188
assert.strictEqual(response.status, 400);
@@ -196,7 +198,7 @@ describe('TunnelRegistryServer', function () {
196198
rsdPort: 58784,
197199
};
198200

199-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels/test-udid-123`, {
201+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels/test-udid-123`, {
200202
method: 'PUT',
201203
headers: {'Content-Type': 'application/json'},
202204
body: JSON.stringify(updateData),
@@ -217,7 +219,7 @@ describe('TunnelRegistryServer', function () {
217219
udid: 'different-udid',
218220
};
219221

220-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels/test-udid-123`, {
222+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels/test-udid-123`, {
221223
method: 'PUT',
222224
headers: {'Content-Type': 'application/json'},
223225
body: JSON.stringify(updateData),
@@ -230,7 +232,7 @@ describe('TunnelRegistryServer', function () {
230232
});
231233

232234
it('should return 400 for invalid JSON', async function () {
233-
const response = await fetch(`http://localhost:${testPort}/remotexpc/tunnels/test-udid-123`, {
235+
const response = await fetch(`http://127.0.0.1:${testPort}/remotexpc/tunnels/test-udid-123`, {
234236
method: 'PUT',
235237
headers: {'Content-Type': 'application/json'},
236238
body: 'invalid json',
@@ -244,11 +246,47 @@ describe('TunnelRegistryServer', function () {
244246

245247
describe('Unknown routes', function () {
246248
it('should return 404 for unknown routes', async function () {
247-
const response = await fetch(`http://localhost:${testPort}/unknown/route`);
249+
const response = await fetch(`http://127.0.0.1:${testPort}/unknown/route`);
248250
const data = (await response.json()) as {error: string};
249251

250252
assert.strictEqual(response.status, 404);
251253
assert.strictEqual(data.error, 'Not found');
252254
});
253255
});
256+
257+
describe('bind address', function () {
258+
it('should only listen on localhost, not on all interfaces', async function () {
259+
// The API exposes unauthenticated writes (PUT /:udid); the server must not
260+
// accept connections arriving on non-loopback interfaces.
261+
// Node reports family as 'IPv4' on some versions and 4 on others
262+
const lanAddress = Object.values(os.networkInterfaces())
263+
.flat()
264+
.find((info) => {
265+
const family = info?.family as string | number | undefined;
266+
return family === 'IPv4' || family === 4 ? !info?.internal : false;
267+
})?.address;
268+
if (!lanAddress) {
269+
// No non-loopback interface on this machine; nothing to probe
270+
return;
271+
}
272+
273+
await new Promise<void>((resolve, reject) => {
274+
const socket = net.connect({host: lanAddress, port: testPort, timeout: 500});
275+
const cleanup = (): void => {
276+
socket.removeAllListeners();
277+
socket.destroy();
278+
};
279+
socket.once('connect', () => {
280+
cleanup();
281+
reject(new Error(`Server unexpectedly accepted a connection on non-loopback address ${lanAddress}`));
282+
});
283+
const onRefusal = (): void => {
284+
cleanup();
285+
resolve();
286+
};
287+
socket.once('error', onRefusal);
288+
socket.once('timeout', onRefusal);
289+
});
290+
});
291+
});
254292
});

0 commit comments

Comments
 (0)