Skip to content

Commit 4f68cd7

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 484dd05 commit 4f68cd7

23 files changed

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

‎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)