Skip to content

Commit 5349a5a

Browse files
committed
fix: constrain proxy HTTP network policy
1 parent 2dd7fb6 commit 5349a5a

6 files changed

Lines changed: 152 additions & 5 deletions

File tree

src/__tests__/daemon-proxy.test.ts

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,28 @@
1-
import { test } from 'vitest';
1+
import { test, vi } from 'vitest';
22
import assert from 'node:assert/strict';
33
import crypto from 'node:crypto';
44
import http from 'node:http';
5+
import { Readable } from 'node:stream';
56
import { createDaemonProxyServer } from '../remote/daemon-proxy.ts';
7+
import { createDaemonHttpServer } from '../daemon/server/http-server.ts';
8+
import { executeRunScriptHttpRequest } from '../daemon/adapters/maestro/run-script-http-child.ts';
9+
import {
10+
DAEMON_HTTP_NETWORK_ACCESS_HEADER,
11+
DAEMON_HTTP_PUBLIC_NETWORK_ACCESS,
12+
} from '../daemon/http-contract.ts';
613
import { DAEMON_RPC_PROTOCOL_VERSION } from '../daemon/http-health.ts';
714
import {
815
closeLoopbackServer,
916
listenOnLoopback,
1017
skipWhenLoopbackUnavailable,
1118
} from './test-utils/loopback.ts';
1219

20+
const requestApprovedUrlMock = vi.hoisted(() => vi.fn());
21+
22+
vi.mock('@agent-device/provision-kit/install-source-network-transport', () => ({
23+
requestApprovedUrl: requestApprovedUrlMock,
24+
}));
25+
1326
const PROXY_ARTIFACT_INVENTORY_ENTRY = {
1427
id: 'shot-1',
1528
filename: 'shot.png',
@@ -24,6 +37,7 @@ test('daemon proxy forwards rpc requests with upstream daemon token', async (t)
2437

2538
let upstreamAuth = '';
2639
let upstreamTokenHeader = '';
40+
let upstreamNetworkAccess = '';
2741
let upstreamBody: Record<string, any> | undefined;
2842
const upstream = http.createServer((req, res) => {
2943
if (req.url === '/health') {
@@ -34,6 +48,7 @@ test('daemon proxy forwards rpc requests with upstream daemon token', async (t)
3448
assert.equal(req.url, '/rpc');
3549
upstreamAuth = String(req.headers.authorization ?? '');
3650
upstreamTokenHeader = String(req.headers['x-agent-device-token'] ?? '');
51+
upstreamNetworkAccess = String(req.headers[DAEMON_HTTP_NETWORK_ACCESS_HEADER] ?? '');
3752
let body = '';
3853
req.setEncoding('utf8');
3954
req.on('data', (chunk) => {
@@ -88,6 +103,7 @@ test('daemon proxy forwards rpc requests with upstream daemon token', async (t)
88103
});
89104
assert.equal(upstreamAuth, 'Bearer daemon-secret');
90105
assert.equal(upstreamTokenHeader, 'daemon-secret');
106+
assert.equal(upstreamNetworkAccess, DAEMON_HTTP_PUBLIC_NETWORK_ACCESS);
91107
assert.equal(upstreamBody?.params?.token, 'daemon-secret');
92108
assert.equal(upstreamBody?.params?.command, 'devices');
93109
} finally {
@@ -96,6 +112,95 @@ test('daemon proxy forwards rpc requests with upstream daemon token', async (t)
96112
}
97113
});
98114

115+
test('proxy enforces public-only Maestro HTTP policy on a local daemon', async (t) => {
116+
if (await skipWhenLoopbackUnavailable(t)) return;
117+
118+
let loopbackRequests = 0;
119+
const loopbackTarget = http.createServer((_req, res) => {
120+
loopbackRequests += 1;
121+
res.end('loopback-secret');
122+
});
123+
const env = { ...process.env };
124+
delete env.AGENT_DEVICE_HTTP_AUTH_HOOK;
125+
delete env.AGENT_DEVICE_HTTP_AUTH_EXPORT;
126+
const daemon = await createDaemonHttpServer({
127+
token: 'daemon-secret',
128+
env,
129+
handleRequest: async (request) => {
130+
const url = request.positionals[0] ?? '';
131+
return {
132+
ok: true,
133+
data: await executeRunScriptHttpRequest({
134+
method: 'GET',
135+
url,
136+
headers: {},
137+
networkAccess: request.internal?.networkAccess ?? 'unrestricted',
138+
}),
139+
};
140+
},
141+
});
142+
const targetPort = await listenOnLoopback(loopbackTarget);
143+
const daemonPort = await listenOnLoopback(daemon);
144+
const proxy = createDaemonProxyServer({
145+
upstreamBaseUrl: `http://127.0.0.1:${daemonPort}`,
146+
upstreamToken: 'daemon-secret',
147+
clientToken: 'proxy-secret',
148+
});
149+
150+
try {
151+
const proxyPort = await listenOnLoopback(proxy);
152+
const post = async (url: string) => {
153+
const response = await fetch(`http://127.0.0.1:${proxyPort}/agent-device/rpc`, {
154+
method: 'POST',
155+
headers: { 'content-type': 'application/json', authorization: 'Bearer proxy-secret' },
156+
body: JSON.stringify({
157+
jsonrpc: '2.0',
158+
id: 'proxy-trust',
159+
method: 'agent_device.command',
160+
params: {
161+
token: 'proxy-secret',
162+
command: 'run_script_http',
163+
positionals: [url],
164+
flags: {},
165+
},
166+
}),
167+
});
168+
return { status: response.status, body: (await response.json()) as Record<string, any> };
169+
};
170+
171+
const loopbackResponse = await post(`http://127.0.0.1:${targetPort}/secret`);
172+
assert.equal(loopbackResponse.status, 400);
173+
assert.equal(loopbackResponse.body.error?.data?.code, 'INVALID_ARGS');
174+
assert.match(loopbackResponse.body.error?.message ?? '', /non-public address/);
175+
assert.equal(loopbackRequests, 0, 'the proxy path must never reach a loopback target');
176+
assert.equal(requestApprovedUrlMock.mock.calls.length, 0);
177+
178+
requestApprovedUrlMock.mockResolvedValue({
179+
statusCode: 200,
180+
headers: {},
181+
body: Readable.from(['public-response']),
182+
close: async () => {},
183+
});
184+
const publicUrl = 'https://93.184.216.34/public';
185+
const publicResponse = await post(publicUrl);
186+
assert.equal(publicResponse.status, 200);
187+
assert.deepEqual(publicResponse.body.result?.data, {
188+
status: 200,
189+
body: 'public-response',
190+
headers: {},
191+
});
192+
assert.equal(requestApprovedUrlMock.mock.calls.length, 1);
193+
assert.equal(requestApprovedUrlMock.mock.calls[0]?.[0].url.href, publicUrl);
194+
assert.equal(requestApprovedUrlMock.mock.calls[0]?.[0].approvedAddress, '93.184.216.34');
195+
assert.equal(requestApprovedUrlMock.mock.calls[0]?.[0].family, 4);
196+
} finally {
197+
requestApprovedUrlMock.mockReset();
198+
await closeLoopbackServer(proxy);
199+
await closeLoopbackServer(daemon);
200+
await closeLoopbackServer(loopbackTarget);
201+
}
202+
});
203+
99204
test('daemon proxy rejects unauthenticated rpc requests', async (t) => {
100205
if (await skipWhenLoopbackUnavailable(t)) return;
101206

src/daemon/http-contract.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
export const DAEMON_HTTP_BASE_PATH = '/agent-device';
22
export const DAEMON_HTTP_TENANT_HEADER = 'x-agent-device-tenant';
3+
export const DAEMON_HTTP_NETWORK_ACCESS_HEADER = 'x-agent-device-network-access';
4+
export const DAEMON_HTTP_PUBLIC_NETWORK_ACCESS = 'public-only';
35

46
export function buildDaemonHttpBaseUrl(baseUrl: string): string {
57
return buildDaemonHttpUrl(baseUrl, DAEMON_HTTP_BASE_PATH);

src/daemon/server/http-server.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import {
3333
shouldStreamRequestProgress,
3434
} from '../request-progress-protocol.ts';
3535
import { buildDaemonHealthPayload } from '../http-health.ts';
36-
import { DAEMON_HTTP_TENANT_HEADER } from '../http-contract.ts';
36+
import { DAEMON_HTTP_NETWORK_ACCESS_HEADER, DAEMON_HTTP_TENANT_HEADER } from '../http-contract.ts';
3737
import { sendRestJsonError, statusCodeForNormalizedError } from '../http-errors.ts';
3838
import { tryHandleUploadHttpRoute } from '../upload-http.ts';
3939
import { tryHandleDownloadableArtifactHttpRoute } from '../downloadable-artifact-http.ts';
@@ -534,7 +534,6 @@ export async function createDaemonHttpServer(options: {
534534
}): Promise<http.Server> {
535535
const environment = options.env ?? process.env;
536536
const authHook = await loadHttpAuthHook(environment);
537-
const trustPolicy = resolveHttpTrustPolicy({ authHookConfigured: authHook !== null });
538537
const { handleRequest, token, retainArtifacts = false, resolveRequestDiagnosticsPath } = options;
539538
return http.createServer((req, res) => {
540539
if (req.method === 'GET' && req.url === '/health') {
@@ -710,7 +709,13 @@ export async function createDaemonHttpServer(options: {
710709
if (daemonRequest.flags?.tenant !== undefined) {
711710
daemonRequest.flags = { ...daemonRequest.flags, tenant: tenantTrust.tenantId };
712711
}
713-
daemonRequest = applyHttpTrustPolicy(daemonRequest, trustPolicy);
712+
daemonRequest = applyHttpTrustPolicy(
713+
daemonRequest,
714+
resolveHttpTrustPolicy({
715+
authHookConfigured: authHook !== null,
716+
networkAccessMarker: req.headers[DAEMON_HTTP_NETWORK_ACCESS_HEADER],
717+
}),
718+
);
714719

715720
let canceledInFlight = false;
716721
// Request-scoped cancellation: mark this request canceled whenever its client

src/daemon/server/http-trust-policy.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import assert from 'node:assert/strict';
22
import { test } from 'vitest';
3+
import { DAEMON_HTTP_PUBLIC_NETWORK_ACCESS } from '../http-contract.ts';
34
import type { DaemonRequest } from '../types.ts';
45
import { applyHttpTrustPolicy, resolveHttpTrustPolicy } from './http-trust-policy.ts';
56

@@ -15,6 +16,25 @@ test('an HTTP server without an auth hook keeps local unrestricted behavior', ()
1516
});
1617
});
1718

19+
test('a proxy network marker selects public-only behavior without an auth hook', () => {
20+
assert.deepEqual(
21+
resolveHttpTrustPolicy({
22+
authHookConfigured: false,
23+
networkAccessMarker: DAEMON_HTTP_PUBLIC_NETWORK_ACCESS,
24+
}),
25+
{ networkAccess: 'public-only' },
26+
);
27+
});
28+
29+
test('an invalid or ambiguous proxy network marker fails closed', () => {
30+
for (const networkAccessMarker of ['unrestricted', ['public-only', 'public-only']]) {
31+
assert.throws(
32+
() => resolveHttpTrustPolicy({ authHookConfigured: false, networkAccessMarker }),
33+
/Invalid daemon HTTP network access marker/,
34+
);
35+
}
36+
});
37+
1838
test('the public-only policy rejects every unbacked host path source', () => {
1939
assert.throws(
2040
() =>

src/daemon/server/http-trust-policy.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,21 @@
11
import { AppError } from '@agent-device/kernel/errors';
22
import type { DaemonNetworkAccessPolicy, DaemonRequest } from '../types.ts';
3+
import { DAEMON_HTTP_PUBLIC_NETWORK_ACCESS } from '../http-contract.ts';
34

45
export type HttpTrustPolicy = {
56
networkAccess: DaemonNetworkAccessPolicy;
67
};
78

8-
export function resolveHttpTrustPolicy(params: { authHookConfigured: boolean }): HttpTrustPolicy {
9+
export function resolveHttpTrustPolicy(params: {
10+
authHookConfigured: boolean;
11+
networkAccessMarker?: string | string[];
12+
}): HttpTrustPolicy {
13+
if (params.networkAccessMarker !== undefined) {
14+
if (params.networkAccessMarker !== DAEMON_HTTP_PUBLIC_NETWORK_ACCESS) {
15+
throw new AppError('INVALID_ARGS', 'Invalid daemon HTTP network access marker');
16+
}
17+
return { networkAccess: DAEMON_HTTP_PUBLIC_NETWORK_ACCESS };
18+
}
919
return { networkAccess: params.authHookConfigured ? 'public-only' : 'unrestricted' };
1020
}
1121

src/remote/daemon-proxy.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import { readNodeHttpRequestBody } from '../utils/node-http.ts';
77
import { timingSafeStringEqual } from '../utils/timing-safe-equal.ts';
88
import {
99
DAEMON_HTTP_BASE_PATH,
10+
DAEMON_HTTP_NETWORK_ACCESS_HEADER,
11+
DAEMON_HTTP_PUBLIC_NETWORK_ACCESS,
1012
DAEMON_HTTP_TENANT_HEADER,
1113
buildDaemonHttpAuthHeaders,
1214
buildDaemonHttpUrl,
@@ -335,6 +337,9 @@ function buildUpstreamHeaders(
335337
if (route === '/rpc' && !headers.has('content-type')) {
336338
headers.set('content-type', 'application/json');
337339
}
340+
if (route === '/rpc') {
341+
headers.set(DAEMON_HTTP_NETWORK_ACCESS_HEADER, DAEMON_HTTP_PUBLIC_NETWORK_ACCESS);
342+
}
338343
for (const [name, value] of Object.entries(buildDaemonHttpAuthHeaders(upstreamToken))) {
339344
headers.set(name, value);
340345
}

0 commit comments

Comments
 (0)