Skip to content

Commit 04758c9

Browse files
authored
fix(daemon): refuse host path install sources on the HTTP surface (#2102)
* fix(daemon): refuse host path install sources on the HTTP surface `install_from_source` accepted `source.kind: "path"` over `/rpc` behind nothing but a non-empty check, so any caller who could reach the daemon with a valid token could name a file the daemon user can read and have its bytes flow back through the install pipeline. The `url` kind already declares its trust boundary (credential rejection, blocked hostnames, non-public-address rejection, redirect caps). The `path` kind has no host-side equivalent, and it does not need one: it is a local affordance for callers that already carry the daemon's own authority, so the HTTP boundary refuses it outright instead of confining it. `HttpInstallSource` carries the narrowed contract, so a path source cannot be returned from the boundary again without failing to compile. In-process callers are untouched — `admin.ts` and `diff-screenshot.ts` build path sources without crossing `parseInstallSource`. Refs #2097 * fix(daemon): refuse host path install sources through the remote proxy The `parseInstallSource` gate covered one of the two RPC methods that can carry an install source. The generic `agent_device.command` method copies `params.meta` wholesale and `commandRpcParamsSchema` types `meta` as an opaque object, so `meta.installSource` reached the install handler unparsed — and that is the method the client actually uses. Gating it at the daemon's HTTP boundary would take local callers with it: that server binds loopback, and `agent-device proxy` puts the local daemon in HTTP mode, so a local `install <path>` on a proxy host crosses the same boundary with the same wire shape. The proxy is the seam that separates the daemon's host from callers who are not on it, so the refusal belongs there — where it also covers both RPC methods at once, because the proxy reads the body before the method. An uploaded artifact still backs a path source across the proxy: the daemon substitutes the uploaded file and never reads the wire path, and an upload id the caller does not own throws rather than falling back to it. That fallback is what would turn the carve-out into the hole, so it is now pinned by a test. This refuses `install remote:<path>` through a proxy. Naming a file on the daemon's host from a remote client is the reported vulnerability, so the affordance cannot survive the fix; the client-side prefix is left in place. Refs #2097 * refactor(proxy): give the host-path refusal its own module Review: the narration I added carried what names, a module boundary, and test names should carry, and `daemon-proxy.ts` had held no such comments before. `proxy-install-source-admission.ts` now owns the rule, so the boundary states it instead of a comment; `isBackedByUploadedArtifact` names what "unbacked" means in `carriesUnbackedHostPathInstallSource`. The proxy owner returns to 443 lines from 495. Refs #2097
1 parent 77b5be8 commit 04758c9

6 files changed

Lines changed: 290 additions & 11 deletions

File tree

src/__tests__/daemon-proxy.test.ts

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,3 +513,129 @@ function sendUploadProxyFinalize(
513513
res.setHeader('content-type', 'application/json');
514514
res.end(JSON.stringify({ ok: true, uploadId: 'tracked-upload-1' }));
515515
}
516+
517+
type ProxyRpcOutcome = {
518+
status: number;
519+
body: { error?: { code?: number; message?: string; data?: { code?: string } } };
520+
upstreamCalls: number;
521+
};
522+
523+
async function postInstallRpcThroughProxy(
524+
params: Record<string, unknown>,
525+
): Promise<ProxyRpcOutcome> {
526+
let upstreamCalls = 0;
527+
const upstream = http.createServer((req, res) => {
528+
upstreamCalls += 1;
529+
let body = '';
530+
req.setEncoding('utf8');
531+
req.on('data', (chunk) => {
532+
body += chunk;
533+
});
534+
req.on('end', () => {
535+
res.setHeader('content-type', 'application/json');
536+
res.end(
537+
JSON.stringify({
538+
jsonrpc: '2.0',
539+
id: (JSON.parse(body) as { id?: unknown }).id,
540+
result: { ok: true, data: { reached: 'upstream' } },
541+
}),
542+
);
543+
});
544+
});
545+
const proxy = createDaemonProxyServer({
546+
upstreamBaseUrl: `http://127.0.0.1:${await listenOnLoopback(upstream)}`,
547+
upstreamToken: 'daemon-secret',
548+
clientToken: 'proxy-secret',
549+
});
550+
551+
try {
552+
const proxyPort = await listenOnLoopback(proxy);
553+
const response = await fetch(`http://127.0.0.1:${proxyPort}/agent-device/rpc`, {
554+
method: 'POST',
555+
headers: { 'content-type': 'application/json', authorization: 'Bearer proxy-secret' },
556+
body: JSON.stringify({ jsonrpc: '2.0', id: 'req-1', ...params }),
557+
});
558+
return {
559+
status: response.status,
560+
body: (await response.json()) as ProxyRpcOutcome['body'],
561+
upstreamCalls,
562+
};
563+
} finally {
564+
await closeLoopbackServer(proxy);
565+
await closeLoopbackServer(upstream);
566+
}
567+
}
568+
569+
test('proxy refuses a host path install source on the generic command method', async (t) => {
570+
if (await skipWhenLoopbackUnavailable(t)) return;
571+
572+
const { status, body, upstreamCalls } = await postInstallRpcThroughProxy({
573+
method: 'agent_device.command',
574+
params: {
575+
token: 'proxy-secret',
576+
command: 'install_source',
577+
positionals: [],
578+
flags: { platform: 'android' },
579+
meta: { installSource: { kind: 'path', path: '/etc/passwd' } },
580+
},
581+
});
582+
583+
assert.equal(status, 400);
584+
assert.equal(body.error?.code, -32602);
585+
assert.equal(body.error?.data?.code, 'INVALID_ARGS');
586+
assert.equal(upstreamCalls, 0, 'the daemon must never see a proxied host path source');
587+
});
588+
589+
test('proxy refuses a host path install source on the install_from_source method', async (t) => {
590+
if (await skipWhenLoopbackUnavailable(t)) return;
591+
592+
const { status, body, upstreamCalls } = await postInstallRpcThroughProxy({
593+
method: 'agent_device.install_from_source',
594+
params: {
595+
token: 'proxy-secret',
596+
platform: 'android',
597+
source: { kind: 'path', path: '/etc/passwd' },
598+
},
599+
});
600+
601+
assert.equal(status, 400);
602+
assert.equal(body.error?.data?.code, 'INVALID_ARGS');
603+
assert.equal(upstreamCalls, 0);
604+
});
605+
606+
test('proxy forwards a path source backed by an uploaded artifact', async (t) => {
607+
if (await skipWhenLoopbackUnavailable(t)) return;
608+
609+
const { status, upstreamCalls } = await postInstallRpcThroughProxy({
610+
method: 'agent_device.command',
611+
params: {
612+
token: 'proxy-secret',
613+
command: 'install_source',
614+
positionals: [],
615+
flags: { platform: 'android' },
616+
meta: {
617+
installSource: { kind: 'path', path: '/Users/dev/Downloads/Sample.apk' },
618+
uploadedArtifactId: 'upload-1',
619+
},
620+
},
621+
});
622+
623+
assert.equal(status, 200);
624+
assert.equal(upstreamCalls, 1);
625+
});
626+
627+
test('proxy forwards url install sources unchanged', async (t) => {
628+
if (await skipWhenLoopbackUnavailable(t)) return;
629+
630+
const { status, upstreamCalls } = await postInstallRpcThroughProxy({
631+
method: 'agent_device.install_from_source',
632+
params: {
633+
token: 'proxy-secret',
634+
platform: 'android',
635+
source: { kind: 'url', url: 'https://example.com/app.apk' },
636+
},
637+
});
638+
639+
assert.equal(status, 200);
640+
assert.equal(upstreamCalls, 1);
641+
});

src/daemon/__tests__/http-server-rpc-validation.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,3 +127,86 @@ test('the rpc boundary never accepts internal request fields from the wire', asy
127127
await closeLoopbackServer(server);
128128
}
129129
});
130+
131+
async function withInstallFromSourceRpcServer(
132+
run: (
133+
post: (source: unknown) => Promise<{
134+
status: number;
135+
body: RpcErrorResponse;
136+
dispatched: DaemonRequest[];
137+
}>,
138+
) => Promise<void>,
139+
t: { skip(reason?: string): void },
140+
): Promise<void> {
141+
if (await skipWhenLoopbackUnavailable(t)) return;
142+
143+
const dispatched: DaemonRequest[] = [];
144+
const handleRequest = async (req: DaemonRequest): Promise<DaemonResponse> => {
145+
dispatched.push(req);
146+
return { ok: true, data: { ok: true } };
147+
};
148+
const server = await createDaemonHttpServer({ handleRequest });
149+
150+
try {
151+
const port = await listenOnLoopback(server);
152+
const post = async (source: unknown) => {
153+
const response = await fetch(`http://127.0.0.1:${port}/rpc`, {
154+
method: 'POST',
155+
headers: { 'content-type': 'application/json' },
156+
body: JSON.stringify({
157+
jsonrpc: '2.0',
158+
id: 'req-1',
159+
method: 'agent_device.install_from_source',
160+
params: { platform: 'android', source },
161+
}),
162+
});
163+
return {
164+
status: response.status,
165+
body: (await response.json()) as RpcErrorResponse,
166+
dispatched,
167+
};
168+
};
169+
await run(post);
170+
} finally {
171+
await closeLoopbackServer(server);
172+
}
173+
}
174+
175+
test('install_from_source rejects a host path source at the rpc boundary', async (t) => {
176+
await withInstallFromSourceRpcServer(async (post) => {
177+
const { status, body, dispatched } = await post({ kind: 'path', path: '/etc/passwd' });
178+
179+
assert.equal(status, 400);
180+
assert.equal(body.error?.code, -32602);
181+
assert.equal(body.error?.data?.code, 'INVALID_ARGS');
182+
assert.equal(dispatched.length, 0, 'a host path source must never reach the handler');
183+
}, t);
184+
});
185+
186+
test('install_from_source still admits url sources', async (t) => {
187+
await withInstallFromSourceRpcServer(async (post) => {
188+
const { status, dispatched } = await post({ kind: 'url', url: 'https://example.com/app.apk' });
189+
190+
assert.equal(status, 200);
191+
assert.equal(dispatched.length, 1);
192+
assert.deepEqual(dispatched[0]?.meta?.installSource, {
193+
kind: 'url',
194+
url: 'https://example.com/app.apk',
195+
});
196+
}, t);
197+
});
198+
199+
test('install_from_source still admits github-actions-artifact sources', async (t) => {
200+
await withInstallFromSourceRpcServer(async (post) => {
201+
const { status, dispatched } = await post({
202+
kind: 'github-actions-artifact',
203+
owner: 'callstack',
204+
repo: 'agent-device',
205+
artifactId: 42,
206+
});
207+
208+
assert.equal(status, 200);
209+
assert.equal(dispatched.length, 1);
210+
assert.equal(dispatched[0]?.meta?.installSource?.kind, 'github-actions-artifact');
211+
}, t);
212+
});

src/daemon/__tests__/install-source-resolution.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,14 @@ test('resolveInstallSource rejects GitHub Actions artifact sources on the local
7676
),
7777
).toThrow(/compatible remote daemon/i);
7878
});
79+
80+
test('resolveInstallSource refuses an unknown upload id rather than reading the wire path', () => {
81+
expect(() =>
82+
resolveInstallSource(
83+
makeRequest({
84+
uploadedArtifactId: 'not-a-real-upload',
85+
installSource: { kind: 'path', path: '/etc/passwd' },
86+
}),
87+
),
88+
).toThrow();
89+
});

src/daemon/server/http-server.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ type HttpAuthDecision =
7676
| { ok: true; tenantId?: string }
7777
| { ok: false; statusCode: number; response: JsonRpcResponse };
7878

79+
type HttpInstallSource = Exclude<DaemonInstallSource, { kind: 'path' }>;
80+
7981
const MAX_HTTP_RPC_BODY_BYTES = 1024 * 1024;
8082
const COMMAND_RPC_METHODS = new Set(['agent_device.command', 'agent-device.command']);
8183
const INSTALL_FROM_SOURCE_RPC_METHODS = new Set([
@@ -219,7 +221,7 @@ function readGitHubArtifactInteger(record: Record<string, unknown>, key: 'artifa
219221
return parsed;
220222
}
221223

222-
function parseGitHubActionsArtifactSource(record: Record<string, unknown>): DaemonInstallSource {
224+
function parseGitHubActionsArtifactSource(record: Record<string, unknown>): HttpInstallSource {
223225
const owner = readRequiredGitHubArtifactText(record, 'owner');
224226
const repo = readRequiredGitHubArtifactText(record, 'repo');
225227
const hasArtifactId = record.artifactId !== undefined;
@@ -288,7 +290,7 @@ function toLeaseDaemonRequest(
288290
};
289291
}
290292

291-
function parseInstallSource(params: Record<string, unknown>): DaemonInstallSource {
293+
function parseInstallSource(params: Record<string, unknown>): HttpInstallSource {
292294
const source = params.source;
293295
if (!source || typeof source !== 'object') {
294296
throw new AppError('INVALID_ARGS', 'Invalid params: source is required');
@@ -318,21 +320,18 @@ function parseInstallSource(params: Record<string, unknown>): DaemonInstallSourc
318320
return Object.keys(headers).length > 0 ? { kind: 'url', url, headers } : { kind: 'url', url };
319321
}
320322
if (record.kind === 'path') {
321-
const artifactPath = typeof record.path === 'string' ? record.path.trim() : '';
322-
if (!artifactPath) {
323-
throw new AppError(
324-
'INVALID_ARGS',
325-
'Invalid params: source.path is required for path sources',
326-
);
327-
}
328-
return { kind: 'path', path: artifactPath };
323+
throw new AppError(
324+
'INVALID_ARGS',
325+
'Invalid params: source.kind "path" names a file on the daemon host and is not accepted over HTTP',
326+
{ hint: 'Use a "url" or "github-actions-artifact" source.' },
327+
);
329328
}
330329
if (record.kind === 'github-actions-artifact') {
331330
return parseGitHubActionsArtifactSource(record);
332331
}
333332
throw new AppError(
334333
'INVALID_ARGS',
335-
'Invalid params: source.kind must be "url", "path", or "github-actions-artifact"',
334+
'Invalid params: source.kind must be "url" or "github-actions-artifact"',
336335
);
337336
}
338337

src/remote/daemon-proxy.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ import {
1212
buildDaemonHttpUrl,
1313
} from '../daemon/http-contract.ts';
1414
import { buildDaemonHealthPayload } from '../daemon/http-health.ts';
15+
import {
16+
carriesUnbackedHostPathInstallSource,
17+
sendHostPathInstallSourceRefused,
18+
} from './proxy-install-source-admission.ts';
1519

1620
export type DaemonProxyOptions = {
1721
upstreamBaseUrl: string;
@@ -80,6 +84,11 @@ async function handleProxyRequest(
8084
return;
8185
}
8286

87+
if (carriesUnbackedHostPathInstallSource(rpcBody)) {
88+
sendHostPathInstallSourceRefused(res, readJsonRpcId(rpcBody));
89+
return;
90+
}
91+
8392
await forwardProxyRequest({ req, res, route, options, rpcBody });
8493
}
8594

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import type { ServerResponse } from 'node:http';
2+
import { AppError, normalizeError } from '@agent-device/kernel/errors';
3+
4+
export function carriesUnbackedHostPathInstallSource(rpcBody: string | undefined): boolean {
5+
const params = readRpcParams(rpcBody);
6+
if (!params) return false;
7+
const meta = readRecord(params.meta);
8+
if (isBackedByUploadedArtifact(meta)) return false;
9+
return isHostPathInstallSource(params.source) || isHostPathInstallSource(meta?.installSource);
10+
}
11+
12+
export function sendHostPathInstallSourceRefused(res: ServerResponse, rpcId: unknown): void {
13+
const error = new AppError(
14+
'INVALID_ARGS',
15+
'Invalid params: an install source of kind "path" names a file on the daemon host and is not accepted through the proxy',
16+
{ hint: 'Upload the artifact, or use a "url" or "github-actions-artifact" source.' },
17+
);
18+
res.statusCode = 400;
19+
res.setHeader('content-type', 'application/json');
20+
res.end(
21+
JSON.stringify({
22+
jsonrpc: '2.0',
23+
id: rpcId,
24+
error: { code: -32602, message: error.message, data: normalizeError(error) },
25+
}),
26+
);
27+
}
28+
29+
function readRpcParams(rpcBody: string | undefined): Record<string, unknown> | undefined {
30+
if (!rpcBody) return undefined;
31+
try {
32+
return readRecord(readRecord(JSON.parse(rpcBody))?.params);
33+
} catch {
34+
return undefined;
35+
}
36+
}
37+
38+
function isBackedByUploadedArtifact(meta: Record<string, unknown> | undefined): boolean {
39+
const uploadedArtifactId = meta?.uploadedArtifactId;
40+
return typeof uploadedArtifactId === 'string' && uploadedArtifactId.trim().length > 0;
41+
}
42+
43+
function isHostPathInstallSource(source: unknown): boolean {
44+
return readRecord(source)?.kind === 'path';
45+
}
46+
47+
function readRecord(value: unknown): Record<string, unknown> | undefined {
48+
return value && typeof value === 'object' && !Array.isArray(value)
49+
? (value as Record<string, unknown>)
50+
: undefined;
51+
}

0 commit comments

Comments
 (0)