diff --git a/src/contract/AGENTS.md b/src/contract/AGENTS.md index c42a026ec..6123d9d19 100644 --- a/src/contract/AGENTS.md +++ b/src/contract/AGENTS.md @@ -32,7 +32,7 @@ park on readiness (all but `status.get`) → handler → parse output. every operation but `daemon.stop`, which `DaemonServer` answers. - `agent`: `catalog.get`, `status.get`, `component.list`, `lease.request`, `lease.cancel`, - `lease.renew`, `lease.release`, `lease.list`, `driver.passthrough`, `device.exec`, + `lease.renew`, `lease.release`, `lease.list`, `lease.get`, `driver.passthrough`, `device.exec`, `doctor.run` without `fix` or `purgeOrphans`. - `admin`: every other operation (`list.get`, `config.get`, `daemon.stop`, `events.*`, `token.*`, `worker.*`, `component.install`, `device.add`, `relay.request`, ...). @@ -56,7 +56,7 @@ Three identities: ## Protocol version -- `PROTOCOL_VERSION_RANGE` in `protocol.ts` holds it, now `{ min: 57, max: 57 }`. `DAEMON_PROTOCOL_VERSION` (`src/daemon-protocol/`) is its `max`. +- `PROTOCOL_VERSION_RANGE` in `protocol.ts` holds it, now `{ min: 58, max: 58 }`. `DAEMON_PROTOCOL_VERSION` (`src/daemon-protocol/`) is its `max`. - Every change to a contract shape bumps it by one in the same PR. Move `min` too, unless a compatibility path stays side by side. `.agents/scripts/protocol-bump-check.mjs` checks it on push and in CI; it counts every file here as a shape but tests, `index.ts` and the diff --git a/src/contract/operations.test.ts b/src/contract/operations.test.ts index 20dc2367b..bc082ecd8 100644 --- a/src/contract/operations.test.ts +++ b/src/contract/operations.test.ts @@ -29,6 +29,7 @@ const ROLE_MATRIX: ReadonlyArray<{ { name: "lease.renew", input: { leaseId: "lease_1" }, role: "agent" }, { name: "lease.release", input: { leaseId: "lease_1" }, role: "agent" }, { name: "lease.list", input: {}, role: "agent" }, + { name: "lease.get", input: { leaseId: "lse_1" }, role: "agent" }, { name: "doctor.run", input: { fix: false }, role: "agent" }, { name: "doctor.run", input: {}, role: "agent" }, { name: "doctor.run", input: { fix: true }, role: "admin" }, @@ -116,6 +117,7 @@ const EFFECT_MATRIX: ReadonlyArray<{ { name: "lease.renew", input: { leaseId: "lease_1" }, effect: "write" }, { name: "lease.release", input: { leaseId: "lease_1" }, effect: "write" }, { name: "lease.list", input: {}, effect: "read" }, + { name: "lease.get", input: { leaseId: "lse_1" }, effect: "read" }, { name: "doctor.run", input: {}, effect: "read" }, { name: "doctor.run", input: { fix: false }, effect: "read" }, { name: "doctor.run", input: { fix: true }, effect: "write" }, diff --git a/src/contract/operations.ts b/src/contract/operations.ts index fe34a38e0..28c24fba1 100644 --- a/src/contract/operations.ts +++ b/src/contract/operations.ts @@ -535,6 +535,16 @@ export const leaseList = defineOperation({ output: z.object({ leases: z.array(statusLeaseSchema) }), }); +// why: no `authorize` hook -- the handler answers `UNKNOWN_LEASE` for a lease the caller does not own, as `lease.list` hides it (ADR 0044 §3), so "not yours" and "not there" look the same. +// fallow-ignore-next-line unused-export -- consumed only through the OPERATIONS registry, not by name; still public contract surface. +export const leaseGet = defineOperation({ + name: "lease.get", + role: "agent", + effect: "read", + input: z.object({ leaseId: z.string() }), + output: z.object({ lease: statusLeaseSchema }), +}); + // ---- doctor.run ------------------------------------------------------------------------------- const doctorRunInputSchema = z.object({ @@ -1124,6 +1134,7 @@ export const OPERATIONS = { "lease.renew": leaseRenew, "lease.release": leaseRelease, "lease.list": leaseList, + "lease.get": leaseGet, "driver.passthrough": driverPassthrough, "device.exec": deviceExec, "doctor.run": doctorRun, diff --git a/src/contract/protocol.ts b/src/contract/protocol.ts index fff1f508e..309942908 100644 --- a/src/contract/protocol.ts +++ b/src/contract/protocol.ts @@ -10,7 +10,7 @@ export interface ProtocolRange { const protocolRangeSchema = z.object({ min: z.number().int(), max: z.number().int() }); /** Widen only once a second version is kept alive side by side with the first; a bump that leaves no shim behind moves both ends. */ -export const PROTOCOL_VERSION_RANGE: ProtocolRange = { min: 57, max: 57 }; +export const PROTOCOL_VERSION_RANGE: ProtocolRange = { min: 58, max: 58 }; const LEGACY_DAEMON_PROTOCOL_VERSION = 2; diff --git a/src/contract/schemas.ts b/src/contract/schemas.ts index 3c8ca510e..b65392a5c 100644 --- a/src/contract/schemas.ts +++ b/src/contract/schemas.ts @@ -1383,6 +1383,8 @@ export const workerViewSchema = z.object({ export const statusLeaseSchema = leaseRecordSchema.extend({ /** Which worker holds this lease; absent on a worker's own answer, set by a gateway. */ workerId: z.string().optional(), + // why: the part of a gateway lease id after `.`, so a relayed event naming the worker's lease can be matched; set only by a gateway's `lease.list`, `lease.get` and `list.get kind=leases`. + workerLeaseId: z.string().optional(), }); const USAGE_MAX_WINDOW_MS = 90 * 24 * 60 * 60 * 1000; diff --git a/src/daemon-protocol/index.test.ts b/src/daemon-protocol/index.test.ts index ecd7507c2..f7e37cb58 100644 --- a/src/daemon-protocol/index.test.ts +++ b/src/daemon-protocol/index.test.ts @@ -9,7 +9,7 @@ import { describe("daemon protocol", () => { it("keeps the current protocol version and newline framing", () => { - expect(DAEMON_PROTOCOL_VERSION).toBe(57); + expect(DAEMON_PROTOCOL_VERSION).toBe(58); expect(serializeFrame({ id: 1, type: "hello" })).toBe('{"id":1,"type":"hello"}\n'); }); diff --git a/src/daemon/AGENTS.md b/src/daemon/AGENTS.md index 16ee6a679..2bb2d7f77 100644 --- a/src/daemon/AGENTS.md +++ b/src/daemon/AGENTS.md @@ -58,7 +58,7 @@ Its stdout lines move the state; a failed line, a non-zero exit or a failed laun 2. **Hello** — first frame must be `hello`, else `HANDSHAKE_REQUIRED` and close. Role: the uplink connection is `admin` by acceptance; otherwise the resolver. Principal: `hello.principal`, else `SIMLOCK_AGENT_ID`, else the daemon's pid. Both are fixed for the connection's life. 3. **Version** — ranges overlap → highest common version. `PROTOCOL_VERSION_RANGE` - is `{ min: 57, max: 57 }` (`src/contract/protocol.ts`). No overlap → the + is `{ min: 58, max: 58 }` (`src/contract/protocol.ts`). No overlap → the connection stays open, and every request but `daemon.stop` gets `PROTOCOL_VERSION_UNSUPPORTED`. 4. **Request** — one frame → one `dispatch()` call. While stopping: `DAEMON_STOPPING`. diff --git a/src/daemon/dispatcher.test.ts b/src/daemon/dispatcher.test.ts index 8e3bdae13..43000594a 100644 --- a/src/daemon/dispatcher.test.ts +++ b/src/daemon/dispatcher.test.ts @@ -63,6 +63,7 @@ import { openMemoryStateDatabase } from "../auth/testing.js"; import type { DispatcherOptions, DispatchSession } from "./dispatcher.js"; import { Dispatcher, DispatchError } from "./dispatcher.js"; import { classifyError } from "./error-code.js"; +import { UnknownLeaseError } from "../core/index.js"; import { RequestCancelledError } from "../leasing/index.js"; import { createTestEngine, type TestEngine } from "../leasing/testing.js"; @@ -1166,6 +1167,7 @@ describe("Dispatcher: ownership", () => { session({ principal: "tok_owner", role: "agent" }), ); expect(own.leases).toHaveLength(1); + expect(own.leases[0]).not.toHaveProperty("workerLeaseId"); const otherAgent = await dispatcher.dispatch( "lease.list", @@ -1178,6 +1180,33 @@ describe("Dispatcher: ownership", () => { expect(admin.leases).toHaveLength(1); }); + it("lease.get: an agent reads its own lease, another agent's or an unknown id is UNKNOWN_LEASE, admin reads any", async () => { + const { dispatcher } = await buildDispatcher(); + const leaseId = await grantLease(dispatcher); + const owner = session({ principal: "tok_owner", role: "agent" }); + + const own = await dispatcher.dispatch("lease.get", { leaseId }, owner); + expect(own).toEqual({ lease: (await dispatcher.dispatch("lease.list", {}, owner)).leases[0] }); + expect(own.lease).toMatchObject({ id: leaseId, ownerId: "tok_owner" }); + await expect( + dispatcher.dispatch( + "lease.get", + { leaseId }, + session({ principal: "tok_other", role: "agent" }), + ), + ).rejects.toBeInstanceOf(UnknownLeaseError); + await expect( + dispatcher.dispatch("lease.get", { leaseId: "lse_nope" }, owner), + ).rejects.toBeInstanceOf(UnknownLeaseError); + await expect( + dispatcher.dispatch( + "lease.get", + { leaseId }, + session({ principal: "tok_other", role: "admin" }), + ), + ).resolves.toMatchObject({ lease: { id: leaseId } }); + }); + it("lease.cancel: naming a requesterId with no pending request always passes (not-found), admin bypasses too", async () => { const { dispatcher } = await buildDispatcher(); diff --git a/src/daemon/dispatcher.ts b/src/daemon/dispatcher.ts index 17419b428..d5a9f20c8 100644 --- a/src/daemon/dispatcher.ts +++ b/src/daemon/dispatcher.ts @@ -304,6 +304,7 @@ export class Dispatcher { "lease.renew": this.#leaseRenew, "lease.release": this.#leaseRelease, "lease.list": this.#leaseList, + "lease.get": this.#leaseGet, "driver.passthrough": this.#driverPassthrough, "device.exec": this.#deviceExec, "relay.request": this.#relayRequest, @@ -624,6 +625,16 @@ export class Dispatcher { return { leaseIds: [...leaseIds] }; }; + #leaseGet: Handler<"lease.get"> = (input, session) => { + const lease = this.options.registry.snapshot.leases.find( + (candidate) => candidate.id === input.leaseId, + ); + if (lease === undefined || (session.role !== "admin" && lease.ownerId !== session.principal)) { + throw new UnknownLeaseError(input.leaseId); + } + return { lease }; + }; + #leaseList: Handler<"lease.list"> = (_input, session) => { const leases = this.options.registry.snapshot.leases.filter( (lease) => session.role === "admin" || lease.ownerId === session.principal, diff --git a/src/daemon/server.test.ts b/src/daemon/server.test.ts index b8c85a357..a12c8e937 100644 --- a/src/daemon/server.test.ts +++ b/src/daemon/server.test.ts @@ -2935,6 +2935,33 @@ describe("DaemonServer roles and ownership (ADR 0003 §2-4)", () => { await admin.close(); }); + it("lease.get over the socket answers the caller's own lease and UNKNOWN_LEASE for another agent's", async () => { + const harness = await createHarness({ resolveRole: { resolve: () => "agent" } }); + const alice = await createClient(harness.socketPath); + await helloAs(alice, "alice"); + const grant = await alice.request("lease.request", { + model: "iPhone 16", + osVersion: "26.5", + platform: "ios", + }); + const leaseId = leaseIdOf(grant); + const bob = await createClient(harness.socketPath); + await helloAs(bob, "bob"); + + const own = await alice.request("lease.get", { leaseId }); + expect(own.ok).toBe(true); + expect((own.payload as { lease: { id: string } }).lease).toMatchObject({ + id: leaseId, + ownerId: "alice", + }); + const other = await bob.request("lease.get", { leaseId }); + expect(other.ok).toBe(false); + expect(other.error).toMatchObject({ code: "UNKNOWN_LEASE" }); + + await alice.close(); + await bob.close(); + }); + it("forwards ttlMs as the initial TTL of any lease, and stores it as the lease's width", async () => { const harness = await createHarness({ resolveRole: { resolve: () => "agent" } }); const client = await createClient(harness.socketPath); diff --git a/src/daemon/server.ts b/src/daemon/server.ts index 6ca208275..263f2fec7 100644 --- a/src/daemon/server.ts +++ b/src/daemon/server.ts @@ -1053,6 +1053,12 @@ export class DaemonServer { frame.payload ?? {}, this.#session(connection), ); + case "lease.get": + return this.#dispatcher.dispatch( + "lease.get", + frame.payload ?? {}, + this.#session(connection), + ); case "lease.list": return this.#dispatcher.dispatch( "lease.list", diff --git a/src/gateway/AGENTS.md b/src/gateway/AGENTS.md index 64a5ffe42..1dc572d14 100644 --- a/src/gateway/AGENTS.md +++ b/src/gateway/AGENTS.md @@ -99,9 +99,9 @@ missing from 2 snapshots in a row → dropped; kept across a disconnect, forgott 4. Worker: `relay.*` only on the uplink session (`FORBIDDEN` elsewhere; a gateway refuses them from everyone). Target from `relayTargets`; unknown → `RELAY_TARGET_MISSING`. -**Reads.** `status.get`, `catalog.get`, `list.get`, `lease.list` (admin: all; else own) answer from -views. `worker.install-component` fans out. `nuke.run`, `cleanup.run`, `doctor.run`, -`driver.passthrough`, `component.*`, `device.add`, `device.remove` → `UNSUPPORTED_IN_GATEWAY_MODE`. `usage.get` reads this gateway's `history.db` rows through `core/usage`: fleet totals, one entry per registry or named worker; `dispose` stops its read thread. +**Reads.** `status.get`, `catalog.get`, `list.get`, `lease.list` (admin: all; else own) and `lease.get` +(one lease, as `lease.list` would hold it for the caller) answer from views; a gateway lease record +also carries `workerLeaseId`, the worker's own id (not on `status.get`). `worker.install-component` fans out. `nuke.run`, `cleanup.run`, `doctor.run`, `driver.passthrough`, `component.*`, `device.add`, `device.remove` → `UNSUPPORTED_IN_GATEWAY_MODE`. `usage.get` reads this gateway's `history.db` rows through `core/usage`: fleet totals, one entry per registry or named worker; `dispose` stops its read thread. **Uplink drop.** View → `disconnected`, kept; `worker.disconnected`. In-flight calls → `WORKER_UNREACHABLE`, an in-flight lease request → `worker-failed`. `token.revoke` closes its uplinks. diff --git a/src/gateway/dispatcher.test.ts b/src/gateway/dispatcher.test.ts index 2fea31b6e..e2c1f42cb 100644 --- a/src/gateway/dispatcher.test.ts +++ b/src/gateway/dispatcher.test.ts @@ -836,6 +836,97 @@ describe("GatewayDispatcher", () => { ); }); + it("lease.get reads one lease by its gateway id for its owner or an admin, a worker's own local lease for an admin only, and answers UNKNOWN_LEASE otherwise", async () => { + const { dispatcher, leaseIndex, workers } = harness(); + workers.connected("wrk_1", "mac-mini-1", undefined); + const issued = { + ...leaseFixture("lease_1", "dev_1"), + ownerId: "agent-1", + requesterId: `${GATEWAY_REQUESTER_PREFIX}agent-1`, + }; + const local = { ...leaseFixture("local_1", "dev_2"), ownerId: "worker-agent" }; + workers.refresh("wrk_1", { leases: [issued, local] }); + const owner = session({ principal: "agent-1", role: "agent" }); + const stranger = session({ principal: "agent-2", role: "agent" }); + const get = (leaseId: string, who: ReturnType) => + dispatcher.dispatch("lease.get", { leaseId }, who); + + await expect(get("wrk_1.lease_1", owner)).resolves.toEqual({ + lease: expect.objectContaining({ id: "wrk_1.lease_1", workerId: "wrk_1" }), + }); + await expect(get("wrk_1.lease_1", session())).resolves.toMatchObject({ + lease: { id: "wrk_1.lease_1" }, + }); + await expect(get("local_1", session())).resolves.toMatchObject({ + lease: { id: "local_1", workerId: "wrk_1" }, + }); + const unknown = { code: "UNKNOWN_LEASE", details: { leaseId: expect.any(String) } }; + await expect(get("wrk_1.lease_1", stranger)).rejects.toMatchObject(unknown); + await expect(get("local_1", stranger)).rejects.toMatchObject(unknown); + await expect(get("wrk_1.lease_nope", session())).rejects.toMatchObject(unknown); + await expect(get("lease_1", session())).rejects.toMatchObject(unknown); + // why: an index entry can outlive its worker view, or the view its lease. + for (const [workerId, workerLeaseId] of [ + ["wrk_gone", "lease_9"], + ["wrk_1", "lease_gone"], + ] as const) { + leaseIndex.add({ + gatewayLeaseId: `${workerId}.${workerLeaseId}`, + grantedAt: 1, + ownerId: "agent-1", + requesterId: `${GATEWAY_REQUESTER_PREFIX}agent-1`, + workerId, + workerLeaseId, + }); + await expect(get(`${workerId}.${workerLeaseId}`, session())).rejects.toMatchObject(unknown); + await expect(get(`${workerId}.${workerLeaseId}`, owner)).rejects.toMatchObject(unknown); + } + }); + + it("names the worker's own lease id as workerLeaseId on the leases of lease.list, lease.get and list.get kind=leases, never on status.get", async () => { + const { dispatcher, workers } = harness(); + workers.connected("wrk_1", "mac-mini-1", undefined); + const issued = (id: string, chosen: boolean) => ({ + ...leaseFixture(id, `dev_${id}`), + idChosenByRequester: chosen, + ownerId: "agent-1", + requesterId: `${GATEWAY_REQUESTER_PREFIX}agent-1`, + }); + workers.refresh("wrk_1", { + leases: [ + issued("lse_42", false), + issued("my-lease", true), + leaseFixture("local_1", "dev_local"), + ], + }); + const expected = [ + { id: "wrk_1.lse_42", workerLeaseId: "lse_42" }, + { id: "my-lease", workerLeaseId: "my-lease" }, + { id: "local_1", workerLeaseId: "local_1" }, + ]; + const pick = (leases: readonly { id: string; workerLeaseId?: string | undefined }[]) => + leases.map(({ id, workerLeaseId }) => ({ id, workerLeaseId })); + + const listed = await dispatcher.dispatch("lease.list", {}, session()); + expect(pick(listed.leases)).toEqual(expected); + const fleet = await dispatcher.dispatch("list.get", { kind: "leases" }, session()); + expect(pick(fleet as never)).toEqual(expected); + const owned = await dispatcher.dispatch( + "lease.list", + {}, + session({ principal: "agent-1", role: "agent" }), + ); + expect(pick(owned.leases)).toEqual(expected.slice(0, 2)); + for (const { id, workerLeaseId } of expected) { + const { lease } = await dispatcher.dispatch("lease.get", { leaseId: id }, session()); + expect(lease).toMatchObject({ id, workerLeaseId }); + } + const status = await dispatcher.dispatch("status.get", {}, session()); + expect((status.leases ?? []).map((lease) => Object.keys(lease))).toEqual( + expected.map(() => expect.not.arrayContaining(["workerLeaseId"])), + ); + }); + it("lists no device or lease for a worker that is starting, whether the session is an admin or the owner of a lease the gateway indexed on it, and answers worker.list with the view as it is", async () => { const { dispatcher, workers } = harness(); workers.connected("wrk_1", "mac-mini-1", undefined); diff --git a/src/gateway/dispatcher.ts b/src/gateway/dispatcher.ts index 8e201e71b..7f2a361f1 100644 --- a/src/gateway/dispatcher.ts +++ b/src/gateway/dispatcher.ts @@ -158,7 +158,7 @@ export interface GatewayDispatcherOptions { >; readonly leaseIndex: Pick< FleetLeaseIndex, - "project" | "all" | "isGatewayRequester" | "requesterPrefix" + "project" | "all" | "isGatewayRequester" | "requesterPrefix" | "resolve" | "findByWorkerLease" >; } @@ -252,6 +252,7 @@ export class GatewayDispatcher { "daemon.update": this.#daemonUpdate, "worker.install-component": this.#workerInstallComponent, "lease.list": this.#leaseList, + "lease.get": this.#leaseGet, "list.get": this.#listGet, "identity.get": identityGet, @@ -519,6 +520,56 @@ export class GatewayDispatcher { session.role === "admin" ? this.#fleetLeases() : this.#ownedFleetLeases(session.principal), }); + // why: the one place a gateway lease record gets `workerLeaseId` (the worker's own id), so `lease.list`, `lease.get` and `list.get kind=leases` agree and `status.get` (which calls `project` directly) does not carry it. + #fleetLease( + raw: Lease, + view: { readonly id: string; readonly label?: string | undefined }, + ) { + return { + ...this.options.leaseIndex.project(raw, view.id, view.label), + workerLeaseId: raw.id, + }; + } + + // why: finds the record `lease.list` would hold for this session without building the list -- a gateway-issued lease through the index (its owner, or an admin), a worker's own local lease for an admin only, by a raw id the index does not rename. + #leaseGet: Handler<"lease.get"> = (input, session) => { + const isAdmin = session.role === "admin"; + const entry = this.options.leaseIndex.resolve(input.leaseId); + const lease = + entry === undefined + ? isAdmin + ? this.#localLease(input.leaseId) + : undefined + : isAdmin || entry.ownerId === session.principal + ? this.#issuedLease(entry) + : undefined; + if (lease === undefined) { + throw new DispatchError("UNKNOWN_LEASE", `Unknown lease: ${input.leaseId}`, { + leaseId: input.leaseId, + }); + } + return { lease }; + }; + + #issuedLease(entry: { readonly workerId: string; readonly workerLeaseId: string }) { + const view = this.options.workers.view(entry.workerId); + const raw = view?.leases?.find((lease) => lease.id === entry.workerLeaseId); + return view === undefined || raw === undefined ? undefined : this.#fleetLease(raw, view); + } + + #localLease(leaseId: string) { + for (const view of this.options.workers.views()) { + const raw = view.leases?.find((lease) => lease.id === leaseId); + if ( + raw !== undefined && + this.options.leaseIndex.findByWorkerLease(view.id, raw.id) === undefined + ) { + return this.#fleetLease(raw, view); + } + } + return undefined; + } + /** Every lease this gateway issued to `ownerId`, projected to its fleet-facing form. Looks up * each entry's current record from the owning worker's view rather than caching one on the * index itself, so a renewed TTL/`lastRenewedAt` is always current. An entry whose worker view @@ -531,7 +582,7 @@ export class GatewayDispatcher { const view = this.options.workers.view(entry.workerId); const raw = view?.leases?.find((lease) => lease.id === entry.workerLeaseId); if (view === undefined || raw === undefined) continue; - leases.push(this.options.leaseIndex.project(raw, view.id, view.label)); + leases.push(this.#fleetLease(raw, view)); } return leases; } @@ -585,11 +636,7 @@ export class GatewayDispatcher { #fleetLeases() { return this.options.workers .views() - .flatMap((view) => - (view.leases ?? []).map((lease) => - this.options.leaseIndex.project(lease, view.id, view.label), - ), - ); + .flatMap((view) => (view.leases ?? []).map((lease) => this.#fleetLease(lease, view))); } /** diff --git a/src/http/AGENTS.md b/src/http/AGENTS.md index 797a93857..9e18173d2 100644 --- a/src/http/AGENTS.md +++ b/src/http/AGENTS.md @@ -140,7 +140,8 @@ until `POST /v1/leases/{id}/renew` drains them into `notices`. `lease_lost` clea daemon restart, so a polling client loses the health facts it had not drained. It needs a durable record of why a lease ended first. - **Single-lease reads answer 404, not 403, for an unowned lease.** `GET /v1/leases/{id}` - and its `/events` filter `lease.list` (`findOwnedLease`); there is no `lease.get`. + and its `/events` dispatch `lease.get` (`findOwnedLease`), which answers `UNKNOWN_LEASE` for a + lease that is not there and for one the caller does not own alike. Renew and release dispatch directly and answer 403. - **Error codes outside the contract union.** `UNAUTHENTICATED`, `UNKNOWN_LEASE_REQUEST`, `REQUEST_NOT_CANCELLABLE`, `REQUEST_CANCELLED` (`errors.ts`) have no `ERROR_TABLE` row, so diff --git a/src/http/app.test.ts b/src/http/app.test.ts index f49ecd0b1..0901ea646 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -96,6 +96,16 @@ function buildHarness( ), }); + dispatcher.handlers["lease.get"] = (input: { leaseId: string }, session) => { + const lease = registry.leases.find( + (candidate) => + candidate.id === input.leaseId && + (session.role === "admin" || candidate.ownerId === session.principal), + ); + if (lease === undefined) throw new UnknownLeaseError(input.leaseId); + return { lease }; + }; + const app = createHttpApp({ clock, config, @@ -280,7 +290,36 @@ describe("authentication and role/ownership (via the shared dispatcher)", () => expect(response.status).toBe(403); }); - it("404s (not 403) an agent fetching another requester's lease -- ownership is lease.list's own filter now, not a separate check", async () => { + it.each([ + ["GET /v1/leases/{id}", "/v1/leases/lse_1"], + ["GET /v1/leases/{id}/events", "/v1/leases/lse_1/events"], + ])( + "%s finds the lease with lease.get under the caller's session, and a lease.get that answers UNKNOWN_LEASE is a 404", + async (_route, path) => { + const { app, dispatcher, registry } = buildHarness(); + registry.devices = [makeDevice({ id: "dev_1" })]; + registry.leases = [makeLease({ deviceId: "dev_1", id: "lse_1", ownerId: "tok_agent" })]; + + const found = await app.request(path, { headers: agentAuth }); + expect(found.status).toBe(200); + expect(dispatcher.calls).toEqual([ + expect.objectContaining({ + input: { leaseId: "lse_1" }, + operation: "lease.get", + session: expect.objectContaining({ principal: "tok_agent", role: "agent" }), + }), + ]); + + registry.leases = []; + const missing = await app.request(path, { headers: agentAuth }); + expect(missing.status).toBe(404); + expect(((await missing.json()) as { error: { code: string } }).error.code).toBe( + "UNKNOWN_LEASE", + ); + }, + ); + + it("404s (not 403) an agent fetching another requester's lease -- ownership is lease.get's own answer, not a separate check", async () => { const { app, registry } = buildHarness(); registry.devices = [makeDevice({ id: "dev_1" })]; registry.leases = [makeLease({ deviceId: "dev_1", id: "lse_1", ownerId: "tok_other" })]; @@ -1363,14 +1402,12 @@ describe("lease routes", () => { snapshot: { devices: [device("wrk_a", "UDID-A"), device("wrk_b", "UDID-B")] }, }, }); - dispatcher.handlers["lease.list"] = () => ({ - leases: [ - { - ...makeLease({ deviceId: "dev_1", id: "wrk_b.lse_1", ownerId: "tok_agent" }), - worker: { id: "wrk_b", label: "mac-b" }, - workerId: "wrk_b", - }, - ], + dispatcher.handlers["lease.get"] = () => ({ + lease: { + ...makeLease({ deviceId: "dev_1", id: "wrk_b.lse_1", ownerId: "tok_agent" }), + worker: { id: "wrk_b", label: "mac-b" }, + workerId: "wrk_b", + }, }); const response = await app.request("/v1/leases/wrk_b.lse_1", { headers: agentAuth }); @@ -1445,14 +1482,12 @@ describe("lease routes", () => { }, }, }); - dispatcher.handlers["lease.list"] = () => ({ - leases: [ - { - ...makeLease({ deviceId: "dev_phys", id: "wrk_a.lse_p", ownerId: "tok_agent" }), - worker: { id: "wrk_a" }, - workerId: "wrk_a", - }, - ], + dispatcher.handlers["lease.get"] = () => ({ + lease: { + ...makeLease({ deviceId: "dev_phys", id: "wrk_a.lse_p", ownerId: "tok_agent" }), + worker: { id: "wrk_a" }, + workerId: "wrk_a", + }, }); const response = await app.request("/v1/leases/wrk_a.lse_p", { headers: agentAuth }); diff --git a/src/http/app.ts b/src/http/app.ts index 1e11342ec..915704bb4 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -406,9 +406,9 @@ export function createHttpApp(deps: HttpGatewayDeps): OpenAPIHono & HttpApp const id = c.req.valid("param").id; const { ttlMs } = c.req.valid("json"); - // why: Dispatched directly (not via `findOwnedLease`/`lease.list`) so `lease.renew`'s own + // why: Dispatched directly (not via `findOwnedLease`/`lease.get`) so `lease.renew`'s own // `ownsLease` authorize hook answers -- an unowned lease is `FORBIDDEN`/403, matching the - // socket transport, rather than the `UNKNOWN_LEASE`/404 a `lease.list` filter would produce + // socket transport, rather than the `UNKNOWN_LEASE`/404 `lease.get`'s hidden-lease answer would produce // (S6). An unknown id still surfaces as `UNKNOWN_LEASE`/404 from the handler itself, and a // `ttlMs` above `lease.maxTtlMs` as `BAD_REQUEST`/400 -- the cap lives in the shared // dispatcher, and this route awaits it, so there is nothing to restate here. Answering it @@ -444,10 +444,10 @@ export function createHttpApp(deps: HttpGatewayDeps): OpenAPIHono & HttpApp // unix socket or a future uplink gets the same `FORBIDDEN` this route now inherits // rather than defining. - // Dispatched directly, like `renew`/`release` and unlike the single-lease *reads*: this + // why: dispatched directly, like `renew`/`release` and unlike the single-lease *reads*: this // route mutates a device, so it answers `device.exec`'s own ownership hook -- 403 for // another requester's lease, 404 for an id that names none -- rather than the 404-for-both - // a `lease.list` filter would produce (see `findOwnedLease`'s comment). + // `lease.get` would produce (see `findOwnedLease`'s comment). // Where a chunk goes at each stage of this request's life, including after the client // disconnects -- see `OutputRelay`, which is where the "retain nothing then" rule lives // and is tested. @@ -595,14 +595,13 @@ export function createHttpApp(deps: HttpGatewayDeps): OpenAPIHono & HttpApp const session = buildHttpSession(identity); const id = c.req.valid("param").id; - // Best-effort lookup, only to decorate the response with the device id/state -- + // why: best-effort lookup, only to decorate the response with the device id/state -- // `lease.release`'s output schema is `{ leaseId }` alone (no `deviceId`), so this is still // needed for the response body. It is *not* the authorization decision: `lease.release` // below is dispatched directly and is the sole source of truth for whether this request is // allowed (S6). Its `ownsLease` hook answers `FORBIDDEN`/403 for someone else's lease and // the handler answers `UNKNOWN_LEASE`/404 for a nonexistent one, matching the socket - // transport exactly -- unlike the `lease.list`-filtered `findOwnedLease`, which always - // answered 404 for both cases. + // transport exactly. const lease = await findLeaseForDecoration(deps, session, id); await deps.dispatch("lease.release", { leaseId: id }, session); @@ -796,22 +795,13 @@ function requireOwnRequest(identity: Identity, ownerId: string): void { throw forbidden("Not permitted to access another requester's resource"); } -/** Read-only single-lease lookup for `GET /v1/leases/:id` and `GET /v1/leases/:id/events`. - * There is no `lease.get` operation in the contract -- only `lease.renew`/`lease.release` - * carry an `ownsLease` authorize hook to compare against -- so these two routes stay on - * `lease.list`'s own filter (own leases; admin sees all): an unauthorized id simply isn't in - * the list, and `unknownLease` covers both "doesn't exist" and "not yours" the same way - * `lease.list` itself does not distinguish them, at 404. This is a deliberate, narrower - * divergence than the one S6 fixed for the mutating routes below -- see the known gaps in - * https://github.com/callstackincubator/simlock/blob/main/src/http/AGENTS.md. */ +// why: `lease.get` answers `UNKNOWN_LEASE` for a lease that does not exist and for one the session does not own alike, so the 404 does not tell them apart (the narrower, deliberate divergence from the mutating routes below that the known gaps in src/http/AGENTS.md describe). async function findOwnedLease( deps: HttpGatewayDeps, session: ReturnType, id: string, ) { - const { leases } = await deps.dispatch("lease.list", {}, session); - const lease = leases.find((candidate) => candidate.id === id); - if (lease === undefined) throw unknownLease(id); + const { lease } = await deps.dispatch("lease.get", { leaseId: id }, session); return lease; } diff --git a/website/src/content/docs/docs/using-devices/remote-agents.mdx b/website/src/content/docs/docs/using-devices/remote-agents.mdx index 8d352e418..aadf80072 100644 --- a/website/src/content/docs/docs/using-devices/remote-agents.mdx +++ b/website/src/content/docs/docs/using-devices/remote-agents.mdx @@ -165,7 +165,7 @@ export SIMLOCK_TOKEN=slk_d2FybS1wb29sLWFnZW50LXRva2VuLWV4YW1wbGUtMDM | `DELETE /v1/lease-requests/` | Cancel a request that is still queued. `204` on success; `409 REQUEST_NOT_CANCELLABLE` once device work has started or it was granted (release the lease instead). | | `GET /v1/leases/` | Read the lease again, for example after your agent restarts. `expiresAt` is the current deadline. | | `GET /v1/leases//events` | Server-Sent Events for `device_unhealthy`, `device_recovered` and `lease_lost`. The stream ends after `lease_lost`. | -| `GET /v1/leases` | Your leases. | +| `GET /v1/leases` | Your leases. On a gateway each one also carries `workerLeaseId`, the id its worker knows it by. | | `GET /v1/catalog?platform=ios` | The models and runtimes you can ask for, as `simlock catalog --json` prints them. | | `GET /v1/identity` | Who the token is. |