Skip to content

Commit d49e7e0

Browse files
committed
fix(daemon): fail closed when the auth hook is silent about tenant
An auth hook that ran but returned no tenantId opted the deployment into tenant attestation; falling back to the client's own claim (RPC body meta.tenantId, aux-route x-agent-device-tenant header) let a holder of one valid shared token impersonate any tenant on /rpc and on the diagnostics/ upload/download routes. resolveTrustedTenant() in the new src/daemon/server/tenant-trust.ts is now the single seam both surfaces go through and the only place that computes the resulting identity: hook attests -> use it; no hook configured -> keep today's client-declared behavior (loopback/dev unchanged); hook configured but silent with a client-declared tenant -> refuse (401) instead of trusting the claim, and no raw client-declared metadata survives into the dispatched request in that case either. Fixes #2095
1 parent 2e87347 commit d49e7e0

8 files changed

Lines changed: 419 additions & 17 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@
22

33
## Unreleased
44

5+
- Security (daemon, remote/proxy HTTP only): when `AGENT_DEVICE_HTTP_AUTH_HOOK` is configured and a
6+
request's hook result does not attest a `tenantId`, the daemon no longer falls back to trusting a
7+
client-declared tenant — RPC body `meta.tenantId`, or the `x-agent-device-tenant` header on the
8+
upload/artifact-download/diagnostics routes. A request that also declares a tenant is now refused
9+
(401) instead of running as whichever tenant the caller claimed; a request that declares none
10+
proceeds unscoped, as before. This closes a shared-token impersonation path in multi-tenant
11+
deployments. Deployments with no hook configured (the local loopback CLI) are unaffected. A hook
12+
that needs per-request tenant scoping must attest `tenantId` in its own return value.
513
- Breaking (0.21): removed aggregate performance compatibility (`perf`, `perf sample`, `perf metrics`, the `metrics` alias, optionless `client.observability.perf()`, and SDK `area: 'metrics'`). Use `perf frames`, `perf memory sample`, `perf cpu profile start|stop|report`, or `perf trace start|stop`; removed CLI and raw daemon forms fail with this migration guidance.
614
- Breaking (0.21): removed legacy batch JSON steps with `positionals`/`flags`. Use `{"command":"...","input":{...}}`; rejected steps now include a concrete structured example.
715
- Breaking (0.21): removed the deprecated Node client `command.rotate` wrapper and its `RotateCommandOptions` / `RotateCommandResult` exports. Use `command.orientation`; the already-removed CLI `rotate` form keeps its targeted migration error.

src/__tests__/test-utils/env.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export function restoreEnv(key: string, previous: string | undefined): void {
2+
if (previous === undefined) delete process.env[key];
3+
else process.env[key] = previous;
4+
}
Lines changed: 339 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,339 @@
1+
import { test } from 'vitest';
2+
import assert from 'node:assert/strict';
3+
import fs from 'node:fs';
4+
import path from 'node:path';
5+
import { createDaemonHttpServer } from '../server/http-server.ts';
6+
import { resolveSessionRequestLogPath } from '../session-store.ts';
7+
import { safeSessionName } from '../session-paths.ts';
8+
import { DAEMON_HTTP_TENANT_HEADER } from '../http-contract.ts';
9+
import type { DaemonRequest, DaemonResponse } from '../types.ts';
10+
import {
11+
closeLoopbackServer,
12+
listenOnLoopback,
13+
skipWhenLoopbackUnavailable,
14+
} from '../../__tests__/test-utils/loopback.ts';
15+
import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
16+
import { restoreEnv } from '../../__tests__/test-utils/env.ts';
17+
18+
const DAEMON_TOKEN = 'daemon-secret';
19+
const DIAGNOSTICS_RECORD = '{"phase":"request_start"}\n{"phase":"request_failed"}\n';
20+
21+
function writeSilentAuthHook(root: string): string {
22+
const hookPath = path.join(root, 'silent-auth-hook.mjs');
23+
fs.writeFileSync(hookPath, 'export default function authHook() { return { ok: true }; }\n');
24+
return hookPath;
25+
}
26+
27+
const ATTESTED_TENANT_ID = 'tenant-real';
28+
29+
function writeAttestingAuthHook(root: string): string {
30+
const hookPath = path.join(root, 'attesting-auth-hook.mjs');
31+
fs.writeFileSync(
32+
hookPath,
33+
"export default function authHook() { return { tenantId: 'tenant-real' }; }\n",
34+
);
35+
return hookPath;
36+
}
37+
38+
async function withRpcServer(
39+
hookPath: string | undefined,
40+
run: (ctx: { baseUrl: string; observedRequests: DaemonRequest[] }) => Promise<void>,
41+
): Promise<void> {
42+
const previousHook = process.env.AGENT_DEVICE_HTTP_AUTH_HOOK;
43+
if (hookPath) process.env.AGENT_DEVICE_HTTP_AUTH_HOOK = hookPath;
44+
else delete process.env.AGENT_DEVICE_HTTP_AUTH_HOOK;
45+
46+
const observedRequests: DaemonRequest[] = [];
47+
const server = await createDaemonHttpServer({
48+
token: DAEMON_TOKEN,
49+
handleRequest: async (req): Promise<DaemonResponse> => {
50+
observedRequests.push(req);
51+
return { ok: true, data: { meta: req.meta } };
52+
},
53+
});
54+
try {
55+
const port = await listenOnLoopback(server);
56+
await run({ baseUrl: `http://127.0.0.1:${port}`, observedRequests });
57+
} finally {
58+
await closeLoopbackServer(server);
59+
restoreEnv('AGENT_DEVICE_HTTP_AUTH_HOOK', previousHook);
60+
}
61+
}
62+
63+
async function callRpc(
64+
baseUrl: string,
65+
payload: Record<string, unknown>,
66+
): Promise<{ status: number; body: Record<string, any> }> {
67+
const response = await fetch(`${baseUrl}/rpc`, {
68+
method: 'POST',
69+
headers: {
70+
authorization: `Bearer ${DAEMON_TOKEN}`,
71+
'content-type': 'application/json',
72+
},
73+
body: JSON.stringify(payload),
74+
});
75+
return { status: response.status, body: (await response.json()) as Record<string, any> };
76+
}
77+
78+
async function withDiagnosticsHookServer(
79+
hookPath: string | undefined,
80+
run: (ctx: { baseUrl: string; sessionsDir: string }) => Promise<void>,
81+
): Promise<void> {
82+
const previousHook = process.env.AGENT_DEVICE_HTTP_AUTH_HOOK;
83+
if (hookPath) process.env.AGENT_DEVICE_HTTP_AUTH_HOOK = hookPath;
84+
else delete process.env.AGENT_DEVICE_HTTP_AUTH_HOOK;
85+
86+
const stateDir = mkdtempForTestSync('agent-device-tenant-trust-diagnostics-');
87+
const sessionsDir = path.join(stateDir, 'sessions');
88+
const server = await createDaemonHttpServer({
89+
token: DAEMON_TOKEN,
90+
handleRequest: async (): Promise<DaemonResponse> => ({ ok: true, data: {} }),
91+
resolveRequestDiagnosticsPath: (ref) =>
92+
resolveSessionRequestLogPath(
93+
path.join(sessionsDir, safeSessionName(ref.session)),
94+
ref.requestId,
95+
),
96+
});
97+
try {
98+
const port = await listenOnLoopback(server);
99+
await run({ baseUrl: `http://127.0.0.1:${port}`, sessionsDir });
100+
} finally {
101+
await closeLoopbackServer(server);
102+
restoreEnv('AGENT_DEVICE_HTTP_AUTH_HOOK', previousHook);
103+
fs.rmSync(stateDir, { recursive: true, force: true });
104+
}
105+
}
106+
107+
function writeDiagnosticsRecord(sessionsDir: string, session: string, requestId: string): void {
108+
const recordPath = resolveSessionRequestLogPath(
109+
path.join(sessionsDir, safeSessionName(session)),
110+
requestId,
111+
);
112+
fs.mkdirSync(path.dirname(recordPath), { recursive: true });
113+
fs.writeFileSync(recordPath, DIAGNOSTICS_RECORD);
114+
}
115+
116+
function diagnosticsUrl(baseUrl: string, session: string, requestId: string): string {
117+
return `${baseUrl}/sessions/${encodeURIComponent(session)}/requests/${encodeURIComponent(requestId)}/diagnostics`;
118+
}
119+
120+
test('RPC: a hook configured but silent on tenant refuses a client-declared meta.tenantId', async (t) => {
121+
if (await skipWhenLoopbackUnavailable(t)) return;
122+
const root = mkdtempForTestSync('agent-device-tenant-trust-rpc-');
123+
try {
124+
const hookPath = writeSilentAuthHook(root);
125+
await withRpcServer(hookPath, async ({ baseUrl, observedRequests }) => {
126+
const response = await callRpc(baseUrl, {
127+
jsonrpc: '2.0',
128+
id: 'rpc-impersonate',
129+
method: 'agent_device.command',
130+
params: {
131+
command: 'session_list',
132+
positionals: [],
133+
meta: { tenantId: 'victim' },
134+
},
135+
});
136+
assert.equal(response.status, 401);
137+
assert.equal(response.body.error?.code, -32001);
138+
assert.equal(
139+
observedRequests.length,
140+
0,
141+
'the handler must never see the impersonated request',
142+
);
143+
});
144+
} finally {
145+
fs.rmSync(root, { recursive: true, force: true });
146+
}
147+
});
148+
149+
test('RPC: a hook configured but silent on tenant refuses a client-declared lease tenantId', async (t) => {
150+
if (await skipWhenLoopbackUnavailable(t)) return;
151+
const root = mkdtempForTestSync('agent-device-tenant-trust-rpc-lease-');
152+
try {
153+
const hookPath = writeSilentAuthHook(root);
154+
await withRpcServer(hookPath, async ({ baseUrl, observedRequests }) => {
155+
const response = await callRpc(baseUrl, {
156+
jsonrpc: '2.0',
157+
id: 'rpc-impersonate-lease',
158+
method: 'agent_device.lease.allocate',
159+
params: {
160+
tenantId: 'victim',
161+
runId: 'run-1',
162+
ttlMs: 60000,
163+
backend: 'android-instance',
164+
},
165+
});
166+
assert.equal(response.status, 401);
167+
assert.equal(response.body.error?.code, -32001);
168+
assert.equal(
169+
observedRequests.length,
170+
0,
171+
'the handler must never see the impersonated request',
172+
);
173+
});
174+
} finally {
175+
fs.rmSync(root, { recursive: true, force: true });
176+
}
177+
});
178+
179+
test('RPC: a hook that attests a tenant wins over a mismatched client-declared meta.tenantId', async (t) => {
180+
if (await skipWhenLoopbackUnavailable(t)) return;
181+
const root = mkdtempForTestSync('agent-device-tenant-trust-rpc-attest-');
182+
try {
183+
const hookPath = writeAttestingAuthHook(root);
184+
await withRpcServer(hookPath, async ({ baseUrl, observedRequests }) => {
185+
const response = await callRpc(baseUrl, {
186+
jsonrpc: '2.0',
187+
id: 'rpc-attested',
188+
method: 'agent_device.command',
189+
params: {
190+
command: 'session_list',
191+
positionals: [],
192+
meta: { tenantId: 'victim' },
193+
},
194+
});
195+
assert.equal(response.status, 200);
196+
assert.equal(observedRequests[0]?.meta?.tenantId, ATTESTED_TENANT_ID);
197+
});
198+
} finally {
199+
fs.rmSync(root, { recursive: true, force: true });
200+
}
201+
});
202+
203+
test('RPC: no hook configured keeps a client-declared meta.tenantId unchanged (regression)', async (t) => {
204+
if (await skipWhenLoopbackUnavailable(t)) return;
205+
await withRpcServer(undefined, async ({ baseUrl, observedRequests }) => {
206+
const response = await callRpc(baseUrl, {
207+
jsonrpc: '2.0',
208+
id: 'rpc-loopback',
209+
method: 'agent_device.command',
210+
params: {
211+
command: 'session_list',
212+
positionals: [],
213+
meta: { tenantId: 'tenant-x' },
214+
},
215+
});
216+
assert.equal(response.status, 200);
217+
assert.equal(observedRequests[0]?.meta?.tenantId, 'tenant-x');
218+
});
219+
});
220+
221+
test('aux route: a hook configured but silent on tenant refuses a client-declared header claiming another tenant', async (t) => {
222+
if (await skipWhenLoopbackUnavailable(t)) return;
223+
const root = mkdtempForTestSync('agent-device-tenant-trust-aux-');
224+
try {
225+
const hookPath = writeSilentAuthHook(root);
226+
await withDiagnosticsHookServer(hookPath, async ({ baseUrl, sessionsDir }) => {
227+
writeDiagnosticsRecord(sessionsDir, 'victim-tenant:default', 'abc123');
228+
const response = await fetch(diagnosticsUrl(baseUrl, 'victim-tenant:default', 'abc123'), {
229+
headers: {
230+
authorization: `Bearer ${DAEMON_TOKEN}`,
231+
[DAEMON_HTTP_TENANT_HEADER]: 'victim-tenant',
232+
},
233+
});
234+
assert.equal(response.status, 401);
235+
assert.equal((await response.text()).includes('request_start'), false);
236+
});
237+
} finally {
238+
fs.rmSync(root, { recursive: true, force: true });
239+
}
240+
});
241+
242+
test('aux route: a hook that attests a tenant wins over a mismatched client-declared header', async (t) => {
243+
if (await skipWhenLoopbackUnavailable(t)) return;
244+
const root = mkdtempForTestSync('agent-device-tenant-trust-aux-attest-');
245+
try {
246+
const hookPath = writeAttestingAuthHook(root);
247+
await withDiagnosticsHookServer(hookPath, async ({ baseUrl, sessionsDir }) => {
248+
writeDiagnosticsRecord(sessionsDir, `${ATTESTED_TENANT_ID}:default`, 'abc123');
249+
const owner = await fetch(
250+
diagnosticsUrl(baseUrl, `${ATTESTED_TENANT_ID}:default`, 'abc123'),
251+
{
252+
headers: {
253+
authorization: `Bearer ${DAEMON_TOKEN}`,
254+
[DAEMON_HTTP_TENANT_HEADER]: 'victim-tenant',
255+
},
256+
},
257+
);
258+
assert.equal(owner.status, 200);
259+
assert.equal(await owner.text(), DIAGNOSTICS_RECORD);
260+
});
261+
} finally {
262+
fs.rmSync(root, { recursive: true, force: true });
263+
}
264+
});
265+
266+
test('aux route: no hook configured keeps the header-declared tenant unchanged (regression)', async (t) => {
267+
if (await skipWhenLoopbackUnavailable(t)) return;
268+
await withDiagnosticsHookServer(undefined, async ({ baseUrl, sessionsDir }) => {
269+
writeDiagnosticsRecord(sessionsDir, 'tenant-a:default', 'abc123');
270+
const owner = await fetch(diagnosticsUrl(baseUrl, 'tenant-a:default', 'abc123'), {
271+
headers: {
272+
authorization: `Bearer ${DAEMON_TOKEN}`,
273+
[DAEMON_HTTP_TENANT_HEADER]: 'tenant-a',
274+
},
275+
});
276+
assert.equal(owner.status, 200);
277+
const otherTenant = await fetch(diagnosticsUrl(baseUrl, 'tenant-a:default', 'abc123'), {
278+
headers: {
279+
authorization: `Bearer ${DAEMON_TOKEN}`,
280+
[DAEMON_HTTP_TENANT_HEADER]: 'tenant-b',
281+
},
282+
});
283+
assert.equal(otherTenant.status, 401);
284+
});
285+
});
286+
287+
test('aux route (upload): a hook configured but silent on tenant refuses a client-declared header', async (t) => {
288+
if (await skipWhenLoopbackUnavailable(t)) return;
289+
const root = mkdtempForTestSync('agent-device-tenant-trust-upload-');
290+
const previousHook = process.env.AGENT_DEVICE_HTTP_AUTH_HOOK;
291+
process.env.AGENT_DEVICE_HTTP_AUTH_HOOK = writeSilentAuthHook(root);
292+
const server = await createDaemonHttpServer({
293+
token: DAEMON_TOKEN,
294+
handleRequest: async (): Promise<DaemonResponse> => ({ ok: true, data: {} }),
295+
});
296+
try {
297+
const port = await listenOnLoopback(server);
298+
const response = await fetch(`http://127.0.0.1:${port}/upload`, {
299+
method: 'POST',
300+
headers: {
301+
authorization: `Bearer ${DAEMON_TOKEN}`,
302+
[DAEMON_HTTP_TENANT_HEADER]: 'victim-tenant',
303+
'x-artifact-type': 'file',
304+
'x-artifact-filename': 'demo.apk',
305+
'content-type': 'application/octet-stream',
306+
},
307+
body: Buffer.from('fake-apk'),
308+
});
309+
assert.equal(response.status, 401);
310+
} finally {
311+
await closeLoopbackServer(server);
312+
restoreEnv('AGENT_DEVICE_HTTP_AUTH_HOOK', previousHook);
313+
fs.rmSync(root, { recursive: true, force: true });
314+
}
315+
});
316+
317+
test('RPC: a whitespace-only meta.tenantId is admitted untenanted, not forwarded raw, under a silent hook', async (t) => {
318+
if (await skipWhenLoopbackUnavailable(t)) return;
319+
const root = mkdtempForTestSync('agent-device-tenant-trust-rpc-blank-');
320+
try {
321+
const hookPath = writeSilentAuthHook(root);
322+
await withRpcServer(hookPath, async ({ baseUrl, observedRequests }) => {
323+
const response = await callRpc(baseUrl, {
324+
jsonrpc: '2.0',
325+
id: 'rpc-blank-tenant',
326+
method: 'agent_device.command',
327+
params: {
328+
command: 'session_list',
329+
positionals: [],
330+
meta: { tenantId: ' ' },
331+
},
332+
});
333+
assert.equal(response.status, 200);
334+
assert.equal(observedRequests[0]?.meta?.tenantId, undefined);
335+
});
336+
} finally {
337+
fs.rmSync(root, { recursive: true, force: true });
338+
}
339+
});

0 commit comments

Comments
 (0)