Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions src/contract/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,20 @@ export const leaseRequest = defineOperation({
role: "agent",
input: leaseRequestInputSchema,
output: leaseGrantSchema,
/**
* Deliberately no `authorize` hook, and not an oversight to pair with `lease.cancel`'s: ADR
* §4 is explicit that any agent session may request a lease under an arbitrary
* `requesterId` -- that is the whole mechanism behind "one connection (the host, acting as
* a proxy for many agents) holds many leases by passing one requester id per session". The
* lease's actual `ownerId` is never client-supplied (always `session.principal`, see the
* dispatcher's `#leaseRequest`), so this does not let one session take over another's
* lease -- only choose the attribution label on a lease it will itself own. `lease.cancel`
* used to look inconsistent next to this (forbidding a `requesterId` that did not equal the
* principal, on an operation that has none of `lease.request`'s ownership protection to
* begin with) -- fixed by gating `lease.cancel` on the pending request's recorded owner
* (`pendingRequestOwner`, see its own `authorize` hook below) instead, which is consistent
* with this operation's design rather than in tension with it.
*/
});

// ---- lease.cancel (new, ADR §9) --------------------------------------------------------------
Expand All @@ -135,16 +149,25 @@ export const leaseCancel = defineOperation({
input: z.object({ requesterId: z.string().optional() }),
output: z.object({ result: z.enum(["cancelled", "not-found", "not-cancellable"]) }),
/**
* ADR §9: "cancels this principal's pending request by requester id" -- not any pending
* request, keyed by a `requesterId` the caller can pass arbitrarily. Deliberately not
* `ownsLease` (that hook resolves a *lease's* recorded `ownerId` from the registry; a
* pending, not-yet-granted request has no lease yet to look up). `requesterId` defaults to
* the principal the same way the handler's own default does (see `dispatcher.ts`'s
* `#leaseCancel`), so an omitted `requesterId` always passes -- only an explicit, *different*
* `requesterId` is gated, and only admin may supply one.
* ADR §9: "cancels this principal's pending request by requester id". Gated on the pending
* request's *recorded owner* (`pendingRequestOwner`, ADR §4's `ownerId` -- always the
* creating session's principal, never the caller-suppliable `requesterId`), not on comparing
* `requesterId` to the principal directly -- that comparison would forbid exactly the case
* ADR §4 exists for: one connection (`principal: "host"`) proxying many agents, each under
* its own `requesterId` (`"agent-7"`). Deliberately not `ownsLease` (that hook resolves a
* *lease's* recorded `ownerId` from the registry; a pending, not-yet-granted request has no
* lease yet to look up -- `pendingRequestOwner` is the wait-queue equivalent). An omitted
* `requesterId` defaults to the principal the same way the handler's own default does (see
* `dispatcher.ts`'s `#leaseCancel`), so it always resolves to a request this principal owns.
* A `requesterId` with no pending request resolves `pendingRequestOwner` to `undefined`,
* which is treated as authorized (same convention as `ownsLease`) so the handler's own
* `not-found` surfaces instead of a misleading `FORBIDDEN`.
*/
authorize: (input, context) =>
context.role === "admin" || (input.requesterId ?? context.principal) === context.principal,
authorize: (input, context) => {
if (context.role === "admin") return true;
const owner = context.pendingRequestOwner(input.requesterId ?? context.principal);
return owner === undefined || owner === context.principal;
},
});

// ---- lease.renew ----------------------------------------------------------------------------
Expand Down
7 changes: 7 additions & 0 deletions src/contract/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,17 @@ export const helloRequestSchema = z
* What the daemon replies with. `role` is declared now (every field a client will eventually
* need to assert it got what it asked for, per ADR §5) but is a fixed value until PR 2 resolves
* it from a real credential -- see `DaemonServer#handleHello`.
*
* `principal` is the connection's resolved, fixed-for-its-lifetime identity (ADR §4): what a
* `hello` request supplied, or the daemon's own default (today: `defaultRequesterId`) when it
* omitted one. Without this the client cannot learn its own principal in that fallback case, and
* would otherwise have to guess -- see the abort-authorization defect this closes in
* `simlock-client/client.ts`.
*/
export const helloReplySchema = z.object({
protocolVersion: z.number().int(),
daemonProtocolRange: protocolRangeSchema,
version: z.string(),
role: z.enum(["agent", "admin"]),
principal: z.string(),
});
10 changes: 10 additions & 0 deletions src/contract/roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ export interface AuthorizeContext {
readonly principal: string;
readonly role: Role;
readonly ownerId: (id: string) => string | undefined;
/** ADR §4/§9: the session principal that created the pending request identified by
* `requesterId` -- looked up against the live wait queue, the same "lookup, not a value"
* shape as `ownerId` above and for the same reason (the only identifier available at
* `authorize` time is whatever the input schema already validated). Used by `lease.cancel`
* so a proxy connection (one principal, many `requesterId`s) can cancel what it created,
* rather than comparing the caller-suppliable `requesterId` to the principal directly.
* Returns `undefined` for a `requesterId` with no pending request, which `lease.cancel`'s
* `authorize` hook treats as authorized on purpose -- same convention as `ownsLease` -- so
* the handler's own `not-found` surfaces instead of a misleading `FORBIDDEN`. */
readonly pendingRequestOwner: (requesterId: string) => string | undefined;
}

/**
Expand Down
15 changes: 15 additions & 0 deletions src/core/lease-acquisition-coordinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,21 @@ export class LeaseAcquisitionCoordinator implements AcquisitionMaintenance {
return this.options.queue.depth;
}

/**
* The session principal a pending request was created under (ADR §4: `ownerId` on
* `LeaseRequestOptions`, always the session principal, never the caller-suppliable
* `requesterId`). `undefined` when no pending request exists for this requester id --
* `cancelPending`'s own `not-found` outcome is what surfaces that, not this lookup, so a
* caller cancelling a request that already settled (or never existed) is not authorized on
* a manufactured owner. A synchronous read like `queueDepth`, not routed through
* `decisions.run`: no state is asserted or mutated, and `#leaseCancel`'s contract-level
* `authorize` hook (ADR §2 step 3) runs before the handler, so it cannot go through the
* handler's own serialized decision anyway.
*/
pendingRequestOwner(requesterId: string): string | undefined {
return this.options.queue.findPendingWaiter(requesterId)?.options.ownerId;
}

get queueHeadSpec(): DeviceSpec | undefined {
return (this.options.queue.head as AcquisitionWaiter | undefined)?.spec;
}
Expand Down
7 changes: 7 additions & 0 deletions src/core/lease-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,13 @@ export class LeaseEngine {
return this.#acquisition.cancelPending(requesterId);
}

/** The session principal that owns a pending request, for `lease.cancel`'s owner-aware
* `authorize` hook (ADR §4). See the coordinator method's own comment. */
// fallow-ignore-next-line unused-class-member -- reached through the QueueControl port by the dispatcher's authorize context (same as the sibling cancelPending).
pendingRequestOwner(requesterId: string): string | undefined {
return this.#acquisition.pendingRequestOwner(requesterId);
}

// fallow-ignore-next-line unused-class-member -- reached through the LeaseCommands port by DaemonServer (same as the sibling heartbeat).
async renew(leaseId: string, ttlMs?: number): Promise<LeaseRecord> {
return this.#releaseCoordinator.renew(leaseId, ttlMs);
Expand Down
6 changes: 6 additions & 0 deletions src/core/lease-ports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ export interface QueueControl {
readonly queueDepth: number;
detachQueuedProgress(requesterId: string): Promise<void>;
cancelPending(requesterId: string): Promise<"cancelled" | "not-found" | "not-cancellable">;
/** ADR §4: the session principal a pending request was created under -- always the creating
* session's principal (`LeaseRequestOptions.ownerId`), never the caller-suppliable
* `requesterId`. `undefined` when no pending request exists for this requester id. Used by
* `lease.cancel`'s `authorize` hook so a proxy connection (one principal, many
* `requesterId`s) can cancel what it created, per ADR §4/§9. */
pendingRequestOwner(requesterId: string): string | undefined;
}

/** Read-only capacity view used by daemon status. */
Expand Down
75 changes: 73 additions & 2 deletions src/daemon/dispatcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,16 +282,19 @@ describe("Dispatcher: ownership", () => {
expect(admin.leases).toHaveLength(1);
});

it("lease.cancel: rejects an agent naming a different requesterId with FORBIDDEN, admits an admin", async () => {
it("lease.cancel: naming a requesterId with no pending request always passes (not-found), admin bypasses too", async () => {
const { dispatcher } = await buildDispatcher();

// A requesterId nobody has a pending request under: not gated, since there is no owner to
// compare against -- `pendingRequestOwner` resolves `undefined`, and `not-found` is what
// surfaces (the same convention `ownsLease` uses for an unknown lease id).
await expect(
dispatcher.dispatch(
"lease.cancel",
{ requesterId: "tok_other" },
session({ principal: "tok_agent", role: "agent" }),
),
).rejects.toMatchObject({ code: "FORBIDDEN" });
).resolves.toMatchObject({ result: "not-found" });

// An omitted requesterId always passes (defaults to the principal) -- see the operation's
// `authorize` hook doc in `operations.ts`.
Expand All @@ -307,6 +310,74 @@ describe("Dispatcher: ownership", () => {
),
).resolves.toMatchObject({ result: "not-found" });
});

it("lease.cancel: gated on the pending request's recorded owner (ADR §4), not the requesterId -- so a proxy connection can cancel what it created", async () => {
const { dispatcher, engine } = await buildDispatcher();

// Fill iOS capacity (maxRunning: 2, see testConfig) so a third request queues instead of
// granting immediately -- cancellability requires a still-`queued` waiter.
await dispatcher.dispatch(
"lease.request",
{
model: "iPhone 17 Pro",
mode: "detached",
requesterId: "tok_owner1",
osVersion: "26.5",
platform: "ios",
},
session({ principal: "tok_owner1" }),
);
await dispatcher.dispatch(
"lease.request",
{
model: "iPhone 17 Pro",
mode: "detached",
requesterId: "tok_owner2",
osVersion: "26.5",
platform: "ios",
},
session({ principal: "tok_owner2" }),
);

// ADR §4's proxy case: one connection, principal "host", requesting under a different
// requesterId ("agent-7") than its own principal.
const queuedRequest = dispatcher.dispatch(
"lease.request",
{
model: "iPhone 17 Pro",
mode: "held",
requesterId: "agent-7",
osVersion: "26.5",
platform: "ios",
},
session({ principal: "host" }),
);
await expect.poll(() => engine.queueDepth).toBe(1);

// Naming "agent-7" from a session that is neither "host" (the recorded owner) nor admin is
// FORBIDDEN, even though "agent-7" is the pending request's own requesterId -- the ADR
// incoherence this fixes: `lease.cancel` is no longer gated on `requesterId === principal`.
await expect(
dispatcher.dispatch(
"lease.cancel",
{ requesterId: "agent-7" },
session({ principal: "tok_other", role: "agent" }),
),
).rejects.toMatchObject({ code: "FORBIDDEN" });

// The owning principal ("host") cancels what it created, under the requesterId it created
// it with -- this is the case the old `requesterId === principal` comparison forbade
// outright.
await expect(
dispatcher.dispatch(
"lease.cancel",
{ requesterId: "agent-7" },
session({ principal: "host", role: "agent" }),
),
).resolves.toMatchObject({ result: "cancelled" });

await expect(queuedRequest).rejects.toThrow();
});
});

describe("Dispatcher: error codes", () => {
Expand Down
1 change: 1 addition & 0 deletions src/daemon/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export class Dispatcher {
const context: AuthorizeContext = {
ownerId: (leaseId) =>
this.options.registry.snapshot.leases.find((lease) => lease.id === leaseId)?.ownerId,
pendingRequestOwner: (requesterId) => this.options.queue.pendingRequestOwner(requesterId),
principal: session.principal,
role: session.role,
};
Expand Down
21 changes: 21 additions & 0 deletions src/daemon/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1431,6 +1431,27 @@ describe("DaemonServer roles and ownership (ADR 0003 §2-4)", () => {
await client.close();
});

it("hello reports the resolved principal: the client-supplied one verbatim, or the daemon's default when omitted (ADR §4)", async () => {
const harness = await createHarness();

const explicit = await createClient(harness.socketPath);
const explicitReply = await explicit.request("hello", {
clientVersion: "test",
protocolVersion: DAEMON_PROTOCOL_VERSION,
principal: "host",
});
expect(explicitReply.payload).toMatchObject({ principal: "host" });
await explicit.close();

const omitted = await createClient(harness.socketPath);
const omittedReply = await omitted.request("hello", {
clientVersion: "test",
protocolVersion: DAEMON_PROTOCOL_VERSION,
});
expect(omittedReply.payload).toMatchObject({ principal: "test-process" });
await omitted.close();
});

it("allows an admin session to call the same admin-only operation", async () => {
const harness = await createHarness({ resolveRole: { resolve: () => "admin" } });
const client = await createClient(harness.socketPath);
Expand Down
5 changes: 5 additions & 0 deletions src/daemon/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,11 @@ export class DaemonServer {
daemonProtocolRange: this.#protocolRange,
version: this.options.version,
role: connection.role,
// ADR §4: report the resolved principal back -- see `helloReplySchema`'s comment. Read
// from `connection.principal`, set a few lines above from `payload.principal` or the
// daemon's own default, so this is always the same value every subsequent dispatched
// request is authorized against.
principal: connection.principal,
},
"hello",
);
Expand Down
Loading
Loading