Skip to content

Commit fdacffa

Browse files
committed
fix(daemon): decode legacy device claims kindless instead of guessing a kind
r4295744998: released v1/v2 recorded a command-scoped claim and an open session literally named `transient:<command>` as the same record, so the label is not a kind proof, and stamping a guessed kind made the schema version alone decide whether a colon-named session's claim covered its commands. Legacy records now decode kindless, which is safe by construction rather than by guess: every consumer of a claim's kind first requires `isClaimOwnedByThisDaemon` (same stateDir, pid, and process start time), a live process writes only v3 records whose kind is stored, and the pid+start-time-reuse window settlement already classifies as `owner-process-reused` and refuses to release. No migration guesses, no recovery path beyond the principal-based stale release legacy records already had, and no public-name reservation. The record test now pins the kind assertions directly (ordinary legacy and current claims decode `transientCommand` undefined; the v3 kind comes only from its stored field; malformed kinds fail closed) and the android-lifecycle provenance comment states what the no-cwd client calls actually record: the daemon-side fallback workspace at the routed `default` address, joined explicitly by the pushes whose checkout-child cwd exists for relative payload resolution, not as a workspace-authority test (r4295745008, r4295745014).
1 parent 1c8f3e5 commit fdacffa

4 files changed

Lines changed: 72 additions & 34 deletions

File tree

‎packages/daemon-contracts/src/device-claim-record.test.ts‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,28 +42,38 @@ test('v1, v2, and v3 records decode to the process-owned claim with its process
4242
expect(record.session).toBe('work');
4343
expect(record.ownerPid).toBe(4242);
4444
expect(record.ownerToken).toBe('token');
45+
expect(record.transientCommand).toBeUndefined();
4546
}
4647
});
4748

48-
test('a legacy claim claim-kind comes from the label that encoded it, once at the boundary', () => {
49-
// The pre-kind schemas recorded a command-scoped claim only as a `transient:<command>` session.
49+
// Released v1/v2 recorded a command-scoped claim and an open session *named* `transient:<command>`
50+
// as the same record — no stored field separates them, so the kind of a legacy claim is not a
51+
// fact the decoder can read. Both decode kindless, which is safe by construction, not by guess:
52+
// the kind is consulted only for claims `isClaimOwnedByThisDaemon` accepts (same stateDir, pid,
53+
// and process start time), and a live process reads and writes only v3 records, whose kind is a
54+
// stored field. No in-flight command's claim is ever a legacy record; the original #3345 bypass
55+
// needs a current-process claim, which carries its kind.
56+
test('a legacy claim with a command-label session decodes kindless, not as a guessed command claim', () => {
5057
for (const raw of [
5158
legacyClaim({ session: 'transient:install' }),
52-
v2Claim({ session: 'transient:reinstall' }),
59+
v2Claim({ session: 'transient:install' }),
5360
]) {
54-
expect(decodeStoredDeviceClaim(raw)?.transientCommand).toBe(
55-
raw.session === 'transient:install' ? 'install' : 'reinstall',
56-
);
61+
const record = decodeStoredDeviceClaim(raw);
62+
expect(record?.transientCommand).toBeUndefined();
63+
expect(record?.session).toBe('transient:install');
5764
}
58-
// From v3 the kind is a stored field, not a label pattern: a v3 open session may be named
59-
// after a command, and a v3 record without the field is that session's claim.
65+
});
66+
67+
// The kind is a stored field from v3 on. A v3 open session may be named after a command, and a
68+
// v3 record without the field is exactly that session's claim; a malformed kind fails the record
69+
// closed instead of guessing.
70+
test('a v3 claim kind comes only from its stored field', () => {
6071
expect(
6172
decodeStoredDeviceClaim(currentClaim({ session: 'transient:install' }))?.transientCommand,
6273
).toBeUndefined();
6374
expect(
6475
decodeStoredDeviceClaim(currentClaim({ transientCommand: 'install' }))?.transientCommand,
6576
).toBe('install');
66-
// A malformed kind fails the record closed instead of guessing a kind.
6777
expect(decodeStoredDeviceClaim(currentClaim({ transientCommand: '' }))).toBeNull();
6878
expect(decodeStoredDeviceClaim(currentClaim({ transientCommand: 7 }))).toBeNull();
6979
});

‎packages/daemon-contracts/src/device-claim-record.ts‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ export type DeviceClaim = {
3131
* Set when a `transient-exclusive` command took the claim: the command it is scoped to. This is
3232
* the claim-kind discriminant (#3345) — a command-scoped claim is never an open session's claim,
3333
* even when its `session` label reads `transient:<command>` and a caller copies that label into
34-
* `--session`. Absent means an open session owns it. Legacy records carry no field and get their
35-
* kind from the label that encoded it, once, in `decodeStoredDeviceClaim`.
34+
* `--session`. Absent means an open session owns it, including every legacy record: a v1/v2
35+
* record cannot prove which it was, and no kind decision is ever taken on one (see
36+
* {@link decodeStoredDeviceClaim}).
3637
*/
3738
transientCommand?: string;
3839
/** Set by `abandonDeviceClaim`; absent while the claim still holds the device for its owner. */
@@ -60,11 +61,22 @@ export function ownershipFromClaim(claim: DeviceClaim): DeviceClaimSessionOwners
6061
};
6162
}
6263

64+
/**
65+
* Legacy (v1/v2) records decode kindless — as open-session claims — because they cannot state a
66+
* kind: released v1/v2 recorded a command-scoped claim and an open session *named*
67+
* `transient:<command>` identically, so the label is no proof of kind. Decoding kindless is safe
68+
* by construction, not by guess: every consumer of a claim's kind first requires
69+
* `isClaimOwnedByThisDaemon` (same stateDir, pid, and process start time), and a current process
70+
* writes only v3 records whose kind is stored — a legacy record can only be one this process
71+
* could not have written, so its owner identity is proven not to match. Recovery for a legacy
72+
* record needs nothing special: the recording process still holds its token, and any other owner
73+
* is settled or conflicted by the principal-based paths, none of which reads a kind.
74+
*/
6375
export function decodeStoredDeviceClaim(value: unknown): DeviceClaim | null {
6476
if (!isRecord(value)) return null;
6577
if (value.schemaVersion === DEVICE_CLAIM_SCHEMA_VERSION) return decodeCurrentClaim(value);
66-
if (value.schemaVersion === 2) return stampLegacyClaimKind(decodeClaimBody(value));
67-
if (value.schemaVersion === 1) return stampLegacyClaimKind(migrateLegacyClaim(value));
78+
if (value.schemaVersion === 2) return decodeClaimBody(value);
79+
if (value.schemaVersion === 1) return migrateLegacyClaim(value);
6880
return null;
6981
}
7082

@@ -147,22 +159,6 @@ export function transientDeviceClaimSession(command: string): string {
147159

148160
const TRANSIENT_CLAIM_LABEL_PREFIX = 'transient:';
149161

150-
/**
151-
* The command a legacy claim's `session` label encodes a command-scoped claim for, or undefined
152-
* for an open-session label. Records written before the kind field existed encoded it only in
153-
* that label, so translating it here — once, at the legacy boundary — is what lets every current
154-
* claim's kind be a stored field: reading the label at decision points is exactly the #3345 bug
155-
* where a caller could copy `transient:<command>` into `--session` and read as that claim's owner.
156-
*/
157-
function stampLegacyClaimKind(claim: DecodedClaimBody | null): DeviceClaim | null {
158-
if (!claim) return null;
159-
if (!claim.session.startsWith(TRANSIENT_CLAIM_LABEL_PREFIX)) return claim;
160-
const transientCommand = claim.session.slice(TRANSIENT_CLAIM_LABEL_PREFIX.length);
161-
// A bare `transient:` label was never a command claim: command names are non-empty, so such a
162-
// claim could only have been an open session with that literal name.
163-
return transientCommand === '' ? claim : { ...claim, transientCommand };
164-
}
165-
166162
function decodeClaimLocation(
167163
raw: Record<string, unknown>,
168164
): Pick<DeviceClaim, 'deviceKey' | 'session' | 'workspace' | 'stateDir'> | null {

‎packages/daemon/src/resources/device/__tests__/device-claim-admission.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,37 @@ test('an in-flight transient claim refuses a request named after its label, whil
208208
expect(claimedSessions()).toEqual(['transient:install']);
209209
});
210210

211+
// The label-guard branch in acquireTransientDeviceClaim: the open session's claim below is
212+
// named exactly this command's label, and a DIFFERENT session's `install` builds the same
213+
// label as its own claim session. Without the guard, resolveExistingClaim's raw string match
214+
// would hand that request the open session's release token (a renew-adopt), and disposing it
215+
// would delete a claim it never took. Mutation: drop the guard — the foreign session acquires,
216+
// and its dispose unlinks the open session's claim.
217+
test("an open session named after a command's label is not renew-adopted by that command's requests", async () => {
218+
const { stateDir } = setup();
219+
const open = await acquireDeviceClaim({
220+
device: ANDROID_EMULATOR,
221+
session: 'transient:install',
222+
workspace: '/worktrees/current',
223+
stateDir,
224+
reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims,
225+
});
226+
expect(open.status).toBe('acquired');
227+
if (open.status !== 'acquired') return;
228+
229+
const foreign = makeAdmission('transient-exclusive', stateDir, 'install', 'other-session');
230+
const error = asAppError(
231+
await foreign.admit(ANDROID_EMULATOR, localAndroid).catch((error: unknown) => error),
232+
);
233+
expect(error.code).toBe('DEVICE_IN_USE');
234+
await foreign[Symbol.asyncDispose]();
235+
236+
// The open session's claim survived, with its original token: nothing was adopted.
237+
const held = inspectDeviceClaims({})[0]?.claim;
238+
expect(held?.ownerToken).toBe(open.ownership.ownerToken);
239+
expect(await abandonDeviceClaim(open.ownership)).toBe('abandoned');
240+
});
241+
211242
test("another request in flight's transient claim of the same command refuses, and holds nothing", async () => {
212243
const { stateDir } = setup();
213244
const held = await acquireDeviceClaim({

‎test/integration/provider-scenarios/android-lifecycle.test.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -433,11 +433,12 @@ async function runAndroidSetupAndInstallWorkflow(
433433
assert.equal(push.action, 'com.example.demo.PUSH');
434434
assert.equal(push.extrasCount, 3);
435435

436-
// #3345: a claim covers a raw request only from its own session AND its own workspace. The
437-
// client calls above carried no cwd, so the open claimed this checkout at the `default`
438-
// address; these pushes join that address the way `--session` does and run from a directory
439-
// inside the checkout, so their relative payloads resolve against `cwd` as a real call's
440-
// does. A foreign workspace would refuse even under the same name — the copyable-address rule.
436+
// #3345: coverage pairs the routed session with the workspace a claim records. The client
437+
// calls above sent no cwd, so their open claim records the daemon-side fallback workspace —
438+
// this checkout — at the routed `default` address. These pushes state their provenance
439+
// explicitly: sessionExplicit joins that address, and their cwd — a checkout child directory,
440+
// chosen so the relative payloads resolve against it — canonicalizes to that recorded
441+
// workspace. A foreign cwd refuses even under the same session name: the copyable-address rule.
441442
fs.mkdirSync(path.join(process.cwd(), '.tmp'), { recursive: true });
442443
const callerWorkspace = fs.mkdtempSync(path.join(process.cwd(), '.tmp', 'push-'));
443444
try {

0 commit comments

Comments
 (0)