Skip to content

Commit b3460e2

Browse files
thymikeethymikee
committed
feat(client): keep every client off a runtime a host owns
What the registration now records lets a client tell whether it may end the process a state directory names, so nothing outside a host touches one it does not own. A managed client that finds an embedded runtime refuses with `daemon-owner-embedded` instead of replacing it, and a request timeout retires nothing for it or for a client pinned to one. `daemon stop` reaches such a runtime over its socket, waits for the completion record its own close writes, and reports what that close could not finish as warnings; a stop with no route to that socket is refused rather than falling back to a signal. `--clean` is refused for it, because that cleanup walks the owner's whole process tree and a host process runs more than the daemon. `daemonBinding` pins a client to one instance: its token, pid and process start time. The pin is compared before dispatch and never after, so a replaced or absent instance is a typed refusal and never a reason to start, replace, or redirect anything.
1 parent 8a1b28e commit b3460e2

23 files changed

Lines changed: 1995 additions & 124 deletions

‎src/__tests__/agent-device-client-daemon-binding.test.ts‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import type {
4+
AgentDeviceDaemonTransport,
5+
AgentDeviceDaemonTransportContext,
6+
DaemonBinding,
7+
} from '@agent-device/contracts/client';
8+
import { AppError } from '@agent-device/kernel/errors';
9+
import { createAgentDeviceClient } from '../agent-device-client.ts';
10+
11+
// A pin on one daemon instance is a credential and an identity, so the client's job is to read it once
12+
// and hand it to every request of that client, and to refuse one that names no instance.
13+
14+
function recordingTransport() {
15+
const contexts: Array<AgentDeviceDaemonTransportContext | undefined> = [];
16+
const transport: AgentDeviceDaemonTransport = async (_request, context) => {
17+
contexts.push(context);
18+
return { ok: true, data: { devices: [] } };
19+
};
20+
return { contexts, transport };
21+
}
22+
23+
const PIN: DaemonBinding = {
24+
token: 'daemon-token',
25+
pid: 4242,
26+
processStartTime: 'Sun Oct 18 09:00:00 2026',
27+
};
28+
29+
test('a pinned client sends the pin with every request', async () => {
30+
const { contexts, transport } = recordingTransport();
31+
const client = createAgentDeviceClient({ daemonBinding: PIN }, { transport });
32+
33+
await client.devices.list();
34+
await client.devices.list();
35+
36+
assert.deepEqual(
37+
contexts.map((context) => context?.daemonBinding),
38+
[PIN, PIN],
39+
);
40+
});
41+
42+
test('a client that names no pin sends none', async () => {
43+
const { contexts, transport } = recordingTransport();
44+
const client = createAgentDeviceClient({}, { transport });
45+
46+
await client.devices.list();
47+
48+
assert.equal(contexts[0]?.daemonBinding, undefined);
49+
});
50+
51+
test('a pin that does not name one instance is refused where the config is read', () => {
52+
const { transport } = recordingTransport();
53+
54+
for (const malformed of [
55+
// A pid alone cannot tell a process lifetime from the process that recycled its number.
56+
{ token: 't', pid: 1 },
57+
{ pid: 1, processStartTime: 'Sun Oct 18 09:00:00 2026' },
58+
{ token: 't', processStartTime: 'Sun Oct 18 09:00:00 2026' },
59+
{ token: '', pid: 1, processStartTime: 'Sun Oct 18 09:00:00 2026' },
60+
{ token: 't', pid: 1, processStartTime: ' ' },
61+
{ token: 't', pid: 1.5, processStartTime: 'Sun Oct 18 09:00:00 2026' },
62+
{ token: 't', pid: 0, processStartTime: 'Sun Oct 18 09:00:00 2026' },
63+
'daemon-token',
64+
4242,
65+
]) {
66+
assert.throws(
67+
() => createAgentDeviceClient({ daemonBinding: malformed as DaemonBinding }, { transport }),
68+
(error: { code?: string; details?: unknown }) => {
69+
assert.equal(error.code, 'INVALID_ARGS');
70+
return true;
71+
},
72+
JSON.stringify(malformed),
73+
);
74+
}
75+
});
76+
77+
test('a refused pin names the field that failed and never its credential', () => {
78+
const { transport } = recordingTransport();
79+
const secret = 'a-token-nobody-must-see';
80+
81+
let refusal: AppError | undefined;
82+
try {
83+
// A partial pin: the credential itself is valid, so this is the shape that could leak it.
84+
createAgentDeviceClient(
85+
{ daemonBinding: { token: secret, pid: 4242 } as DaemonBinding },
86+
{ transport },
87+
);
88+
assert.fail('expected the config to be refused');
89+
} catch (error) {
90+
assert.ok(error instanceof AppError, String(error));
91+
refusal = error;
92+
}
93+
94+
assert.equal(refusal.details?.daemonBindingField, 'processStartTime');
95+
assert.doesNotMatch(JSON.stringify(refusal.details ?? {}), new RegExp(secret));
96+
});

‎src/__tests__/daemon-registration-embedded-owner.test.ts‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
import assert from 'node:assert/strict';
2+
import fs from 'node:fs';
3+
import { afterEach, test, vi } from 'vitest';
4+
import { readCurrentOwnerIdentity, isProcessAlive } from '@agent-device/host-kit/process';
5+
import {
6+
tryAcquireDaemonRegistration,
7+
stopAndRetireDaemon,
8+
} from '@agent-device/daemon-contracts/daemon-registration-owner';
9+
import { resolveDaemonPaths } from '@agent-device/daemon-contracts/daemon-resolution';
10+
import { readDaemonShutdownCompletion } from '@agent-device/daemon-contracts/daemon-shutdown-report';
11+
import { mkdtempForTestSync } from '@agent-device/test-kit';
12+
13+
// What the registration owner may do to a record a host process owns: prove the lifetime is gone
14+
// before removing anything, and never signal the process the record names.
15+
16+
const fields = {
17+
socketPort: 4210,
18+
token: 'token',
19+
version: '0.0.0-test',
20+
codeOrigin: 'checkout' as const,
21+
codeSignature: 'signature',
22+
};
23+
const ownIdentity = readCurrentOwnerIdentity();
24+
const deadIdentity = { pid: 999_999_999, startTime: 'dead-start' };
25+
26+
afterEach(() => vi.restoreAllMocks());
27+
28+
/** Writes a record that claims `ownership` for a live process, unreadable variants included. */
29+
function writeRecord(
30+
paths: ReturnType<typeof resolveDaemonPaths>,
31+
identity: unknown,
32+
ownership: unknown = 'embedded',
33+
) {
34+
const { pid, startTime } = identity as { pid: number; startTime: string };
35+
fs.writeFileSync(
36+
paths.infoPath,
37+
JSON.stringify({
38+
pid,
39+
processStartTime: startTime,
40+
token: 'token',
41+
port: 4210,
42+
...(ownership === 'corrupt' ? {} : { ownership }),
43+
}),
44+
);
45+
if (ownership === 'corrupt') fs.writeFileSync(paths.infoPath, '{not json');
46+
}
47+
48+
function writeEmbeddedRecord(paths: ReturnType<typeof resolveDaemonPaths>, identity: unknown) {
49+
writeRecord(paths, identity);
50+
}
51+
52+
/** A live acquisition of a state directory, so the caller holds the claim it judges. */
53+
async function acquire(
54+
paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-embedded-claim-')),
55+
) {
56+
const attempt = await tryAcquireDaemonRegistration(paths);
57+
assert.equal(attempt.status, 'acquired');
58+
if (attempt.status !== 'acquired') throw new Error('registration refused');
59+
return { paths, owner: attempt.owner };
60+
}
61+
62+
// A host owns an embedded runtime and ends it with its own close, so retiring that record from outside
63+
// would take the state directory out from under a process that is still serving.
64+
test('retirement of an embedded record proves the lifetime is gone and signals nothing', async () => {
65+
const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-embedded-retirement-'));
66+
writeEmbeddedRecord(paths, ownIdentity);
67+
const before = fs.readFileSync(paths.infoPath, 'utf8');
68+
const signals = vi.spyOn(process, 'kill');
69+
70+
for (const mode of ['graceful', 'force'] as const) {
71+
const result = await stopAndRetireDaemon({
72+
paths,
73+
observed: ownIdentity,
74+
mode,
75+
lockTimeoutMs: 0,
76+
});
77+
assert.equal(result.status, 'retained', JSON.stringify(result));
78+
if (result.status !== 'retained') assert.fail('an embedded owner was retired');
79+
assert.equal(result.reason, 'embedded-owner');
80+
assert.equal(result.termination?.status, 'retained');
81+
}
82+
83+
assert.deepEqual(
84+
signals.mock.calls.filter(([, requested]) => requested !== 0),
85+
[],
86+
'retirement must not reach into a host process',
87+
);
88+
assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), before);
89+
assert.equal(isProcessAlive(ownIdentity.pid), true);
90+
});
91+
92+
test('an embedded record whose lifetime is provably gone retires like any other abandoned one', async () => {
93+
const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-embedded-dead-'));
94+
writeEmbeddedRecord(paths, deadIdentity);
95+
96+
// A host that died without closing left the record behind; keeping it would wedge the state directory
97+
// for every later client with nothing left to close.
98+
const result = await stopAndRetireDaemon({
99+
paths,
100+
observed: deadIdentity,
101+
mode: 'graceful',
102+
lockTimeoutMs: 0,
103+
});
104+
assert.equal(result.status, 'retired', JSON.stringify(result));
105+
assert.equal(result.removedInfo, true);
106+
assert.equal(fs.existsSync(paths.infoPath), false);
107+
});
108+
109+
// A record this build cannot read is not a licence to signal what it names: `detached` is the one
110+
// ownership that authorizes a signal, and an unreadable or unrecognized record proves nothing about
111+
// who owns the process. This is the fail-closed side of the same rule the embedded row above proves.
112+
test('retirement refuses to signal a record whose owner it cannot recognize', async () => {
113+
for (const ownership of ['sideways', 'corrupt'] as const) {
114+
const paths = resolveDaemonPaths(mkdtempForTestSync('agent-device-owner-unknown-'));
115+
writeRecord(paths, ownIdentity, ownership);
116+
const before = fs.readFileSync(paths.infoPath, 'utf8');
117+
const signals = vi.spyOn(process, 'kill');
118+
119+
const result = await stopAndRetireDaemon({
120+
paths,
121+
observed: ownIdentity,
122+
mode: 'graceful',
123+
lockTimeoutMs: 0,
124+
});
125+
assert.equal(result.status, 'retained', ownership);
126+
if (result.status !== 'retained')
127+
assert.fail(`${ownership}: an unrecognized owner was retired`);
128+
// The reason differs from the embedded one: nothing here proves the pid is a daemon of its own, so
129+
// an operator fixes this by inspecting or updating the record, not by closing a host.
130+
assert.equal(result.reason, 'owner-unproven', ownership);
131+
assert.deepEqual(
132+
signals.mock.calls.filter(([, requested]) => requested !== 0),
133+
[],
134+
`${ownership}: an unrecognized owner is never signaled`,
135+
);
136+
assert.equal(fs.readFileSync(paths.infoPath, 'utf8'), before, ownership);
137+
vi.mocked(process.kill).mockClear();
138+
}
139+
});
140+
141+
test('a close answers its stop through the release, and an acquisition clears the answer', async () => {
142+
const { paths, owner } = await acquire();
143+
owner.publish(fields);
144+
145+
await owner.finish(undefined, { unfinished: ['1 provider release left pending'] });
146+
const record = readDaemonShutdownCompletion(paths.baseDir);
147+
// The disposition is the removal this call performed, not the caller's guess about it.
148+
assert.equal(record?.outcome, 'released');
149+
assert.deepEqual(record?.unfinished, ['1 provider release left pending']);
150+
assert.equal(record?.pid, process.pid, 'the record names the instance a stop is fenced to');
151+
152+
// Releasing the claim ends this process's authority to speak about the state directory.
153+
assert.equal(
154+
owner.recordShutdownRetention(['late']),
155+
false,
156+
'a process that no longer holds the claim writes nothing',
157+
);
158+
159+
const next = await acquire(paths);
160+
assert.equal(
161+
readDaemonShutdownCompletion(paths.baseDir),
162+
null,
163+
"a new instance must not inherit an answer to somebody else's stop",
164+
);
165+
await next.owner.finish();
166+
});

‎src/__tests__/daemon-registration-owner.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,10 @@ test('abandoned recovery never signals a live owner and absence is inspected und
362362
});
363363
});
364364

365+
// A host owns an embedded runtime and ends it with its own close, so retiring that record from outside
366+
// would take the state directory out from under a process that is still serving. The rows that prove it
367+
// are in daemon-registration-embedded-owner.test.ts, next to the owner rule they exercise.
368+
365369
test('retirement preserves a winner holding the startup lock instead of stopping or unlinking it', async () => {
366370
const { paths, owner } = await acquire();
367371
owner.publish({ ...fields, token: 'winning-token' });

‎src/agent-device-client.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type {
1616
CaptureScreenshotResult,
1717
CaptureSnapshotOptions,
1818
CaptureSnapshotResult,
19+
DaemonBinding,
1920
DaemonLifecycle,
2021
DragOptions,
2122
FlingOptions,
@@ -83,6 +84,9 @@ export function createAgentDeviceClient(
8384
const transport = deps.transport ?? createDaemonTransport(host);
8485
// A typo in a JSON or JS config must not fall through to the default and manage a daemon.
8586
readDaemonLifecycle(config.daemonLifecycle);
87+
// A pin on one daemon instance is read once, here: every request of this client authenticates with
88+
// it, so a value that never validated must never reach a request.
89+
const daemonBinding = readDaemonBinding(config.daemonBinding);
8690
// The dev-server helpers take the client's host explicitly.
8791
const prepareMetroRuntime = (input: Parameters<typeof devServer.prepareMetroRuntime>[0]) =>
8892
devServer.prepareMetroRuntime(input, host);
@@ -143,6 +147,7 @@ export function createAgentDeviceClient(
143147
authToken: merged.daemonAuthToken,
144148
signal: merged.signal,
145149
daemonLifecycle: merged.daemonLifecycle,
150+
...(daemonBinding ? { daemonBinding } : {}),
146151
}),
147152
);
148153
if (!response.ok) {
@@ -618,6 +623,40 @@ function readDaemonLifecycle(value: unknown): DaemonLifecycle | undefined {
618623
});
619624
}
620625

626+
/**
627+
* Validates a pin on one daemon instance where a client config is read, so a malformed pin cannot
628+
* become a request that silently addresses a different instance.
629+
*
630+
* `processStartTime` is required rather than optional: a pin that names only a pid cannot tell a
631+
* process lifetime from the process that recycled its number, which is the mistake the pin exists to
632+
* refuse.
633+
*/
634+
function readDaemonBinding(value: unknown): DaemonBinding | undefined {
635+
if (value === undefined) return undefined;
636+
const record =
637+
typeof value === 'object' && value !== null ? (value as Partial<DaemonBinding>) : undefined;
638+
const token = record?.token;
639+
if (typeof token !== 'string' || token.length === 0) throw daemonBindingFieldError('token');
640+
const pid = record?.pid;
641+
if (typeof pid !== 'number' || !Number.isSafeInteger(pid) || pid <= 0) {
642+
throw daemonBindingFieldError('pid');
643+
}
644+
const processStartTime = record?.processStartTime;
645+
if (typeof processStartTime !== 'string' || processStartTime.trim().length === 0) {
646+
throw daemonBindingFieldError('processStartTime');
647+
}
648+
return { token, pid, processStartTime };
649+
}
650+
651+
function daemonBindingFieldError(field: keyof DaemonBinding): AppError {
652+
return new AppError('INVALID_ARGS', `daemonBinding is missing or invalid: ${field}.`, {
653+
// A pin is a bearer credential, and this detail is echoed by `--json` and by any diagnostics an
654+
// embedder writes, so the refusal names the field that failed and never a value.
655+
daemonBindingField: field,
656+
hint: 'Use the `clientConfig.daemonBinding` a host returned; a pin built by hand needs the daemon token and the pid and process start time its record published.',
657+
});
658+
}
659+
621660
function mergeClientOptions(
622661
config: AgentDeviceClientConfig,
623662
options: InternalRequestOptions,

0 commit comments

Comments
 (0)