Skip to content

Commit cc7aa75

Browse files
committed
test(gateway,daemon,http): cover §27a's admin owner gate; make HTTP reject rather than drop it (H7)
The gate itself was already correct (daemon/dispatcher.ts, gateway/ dispatcher.ts both FORBIDDEN a non-admin naming owner) but untested: no test proved a non-admin naming owner was refused, nor that an admin's owner is what the resulting lease ends up owned by. Added both to daemon/dispatcher.test.ts and gateway/dispatcher.test.ts; each verified to fail when the corresponding gate is temporarily removed (the non-admin test fails outright / times out, since the request proceeds instead of being refused). Separately: src/http/app.ts's leaseRequestBodySchema had no owner field at all and wasn't .strict(), so a non-admin caller naming owner over HTTP got silence where every other transport answers FORBIDDEN -- the exact anti-pattern this PR's own operations.ts comment condemns for device.exec's requesterId (round 4, F4: read-then-silently-ignore is answering as if an identity was never named). Threaded owner through the schema, LeaseRequestInput, and the tracker's dispatch call instead, so the shared dispatcher's own gate decides -- the same fix that precedent already applied to requesterId. New test proves the field now reaches the dispatched lease.request rather than being dropped in transit (the gate itself is exercised at the dispatcher level, once, not per transport); verified to fail against the pre-fix schema/tracker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MA98m7ua7qvDFZjxFaww6Z
1 parent 105d1f0 commit cc7aa75

5 files changed

Lines changed: 113 additions & 0 deletions

File tree

src/daemon/dispatcher.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,45 @@ describe("Dispatcher: ownership", () => {
367367
return (grant as { lease: { id: string } }).lease.id;
368368
}
369369

370+
it("lease.request: rejects a non-admin session naming owner with FORBIDDEN, never silently ignoring it (ADR §27a, H7)", async () => {
371+
const { dispatcher } = await buildDispatcher();
372+
373+
await expect(
374+
dispatcher.dispatch(
375+
"lease.request",
376+
{ model: "iPhone 17 Pro", osVersion: "26.5", owner: "someone-else", platform: "ios" },
377+
session({ principal: "tok_agent", role: "agent" }),
378+
),
379+
).rejects.toMatchObject({ code: "FORBIDDEN" });
380+
});
381+
382+
it("lease.request: an admin session's owner is who the granted lease ends up owned by, not the admin's own principal (ADR §27a, H7)", async () => {
383+
const { dispatcher } = await buildDispatcher();
384+
385+
const grant = await dispatcher.dispatch(
386+
"lease.request",
387+
{ model: "iPhone 17 Pro", osVersion: "26.5", owner: "agent-7", platform: "ios" },
388+
session({ principal: "tok_gateway", role: "admin" }),
389+
);
390+
391+
expect((grant as { lease: { ownerId: string } }).lease.ownerId).toBe("agent-7");
392+
// The lease is now gated on the named owner, not the admin session that requested it.
393+
await expect(
394+
dispatcher.dispatch(
395+
"lease.renew",
396+
{ leaseId: (grant as { lease: { id: string } }).lease.id },
397+
session({ principal: "tok_gateway", role: "agent" }),
398+
),
399+
).rejects.toMatchObject({ code: "FORBIDDEN" });
400+
await expect(
401+
dispatcher.dispatch(
402+
"lease.renew",
403+
{ leaseId: (grant as { lease: { id: string } }).lease.id },
404+
session({ principal: "agent-7", role: "agent" }),
405+
),
406+
).resolves.toMatchObject({ id: (grant as { lease: { id: string } }).lease.id });
407+
});
408+
370409
it("lease.renew: rejects a non-owner with FORBIDDEN, admits the owner, admin bypasses", async () => {
371410
const { dispatcher } = await buildDispatcher();
372411
const leaseId = await grantLease(dispatcher);

src/gateway/dispatcher.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,49 @@ describe("GatewayDispatcher", () => {
492492
),
493493
).rejects.toMatchObject({ code: "FORBIDDEN" });
494494
});
495+
496+
it("lease.request: rejects a non-admin session naming owner with FORBIDDEN, never silently ignoring it (ADR §27a, H7)", async () => {
497+
const { dispatcher, workers } = harness();
498+
workers.connected("wrk_1", undefined, "0.3.0");
499+
workers.refresh("wrk_1", {
500+
capacity: statusFixture().capacity,
501+
catalog: catalogFixture([{ models: ["iPhone 17"], platform: "ios", runtimes: ["26.0"] }])
502+
.platforms,
503+
downloads: { policy: "on-request" },
504+
});
505+
506+
await expect(
507+
dispatcher.dispatch(
508+
"lease.request",
509+
{ model: "iPhone 17", owner: "someone-else", platform: "ios" },
510+
session({ principal: "agent-1", role: "agent" }),
511+
),
512+
).rejects.toMatchObject({ code: "FORBIDDEN" });
513+
});
514+
515+
it("lease.request: an admin session's owner is who the granted lease ends up owned by (ADR §27a, H7)", async () => {
516+
const { directory, dispatcher, leaseIndex, workers } = harness();
517+
const client = new ScriptedWorkerClient();
518+
directory.add("wrk_1", client);
519+
workers.connected("wrk_1", undefined, "0.3.0");
520+
workers.refresh("wrk_1", {
521+
capacity: statusFixture().capacity,
522+
catalog: catalogFixture([{ models: ["iPhone 17"], platform: "ios", runtimes: ["26.0"] }])
523+
.platforms,
524+
downloads: { policy: "on-request" },
525+
});
526+
client.requestLeaseQueue.push({ grant: grantFixture(), kind: "grant" });
527+
528+
const grant = await dispatcher.dispatch(
529+
"lease.request",
530+
{ model: "iPhone 17", noWait: true, owner: "agent-7", platform: "ios" },
531+
session({ principal: "gw:instance-1", role: "admin" }),
532+
);
533+
534+
const gatewayLeaseId = (grant as { lease: { id: string } }).lease.id;
535+
expect((grant as { lease: { ownerId: string } }).lease.ownerId).toBe("agent-7");
536+
expect(leaseIndex.ownerId(gatewayLeaseId)).toBe("agent-7");
537+
});
495538
});
496539

497540
it("has an answer for every operation the contract declares", async () => {

src/http/app.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,21 @@ describe("POST /v1/lease-requests", () => {
277277
expect(dispatcher.calls).toHaveLength(0);
278278
});
279279

280+
it("forwards a body's owner field to the dispatched lease.request rather than silently dropping it (ADR §27a, H7)", async () => {
281+
// Before this, `leaseRequestBodySchema` had no `owner` field at all, so a caller naming one
282+
// was answered as though it had named none -- the same anti-pattern this codebase's own
283+
// `device.exec`'s `requesterId` precedent already rejected: an identity named and answered
284+
// as if it had not is the kind of silence that reads like authorization. The gate itself
285+
// (FORBIDDEN for a non-admin token) lives in the shared dispatcher, exercised once for every
286+
// transport in `daemon/dispatcher.test.ts`/`gateway/dispatcher.test.ts` -- this only proves
287+
// the field actually reaches that dispatcher from HTTP instead of being dropped in transit.
288+
const { app, dispatcher } = buildHarness();
289+
void postLeaseRequest(app, { ...defaultBody, owner: "someone-else" });
290+
const call = await waitForDispatch(dispatcher, "lease.request");
291+
292+
expect((call.input as { owner?: string }).owner).toBe("someone-else");
293+
});
294+
280295
it("maps a fast RequesterAlreadyLeasedError to 409, naming the existing lease", async () => {
281296
const { app, dispatcher } = buildHarness();
282297
const responsePromise = postLeaseRequest(app, defaultBody);

src/http/app.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ const leaseRequestBodySchema = z.object({
6666
device: z.string().min(1),
6767
full: z.boolean().optional(),
6868
noWait: z.boolean().optional(),
69+
// ADR §27a (H7, round 2 review): declared and forwarded, not silently dropped -- the shared
70+
// dispatcher's own `lease.request` handler is the one place that decides whether this token
71+
// may set it (`FORBIDDEN` for non-admin), the same gate every other transport is held to.
72+
owner: z.string().min(1).optional(),
6973
os: z.string().min(1).optional(),
7074
platform: z.enum(["ios", "android"]),
7175
timeoutMs: z.number().int().positive().optional(),
@@ -111,6 +115,7 @@ function toLeaseRequestInput(body: z.infer<typeof leaseRequestBodySchema>): Leas
111115
...(body.noWait === undefined ? {} : { noWait: body.noWait }),
112116
...(body.allowDownload === undefined ? {} : { allowDownload: body.allowDownload }),
113117
...(body.full === undefined ? {} : { full: body.full }),
118+
...(body.owner === undefined ? {} : { owner: body.owner }),
114119
};
115120
}
116121

src/http/tracker.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ export interface LeaseRequestInput {
1414
readonly noWait?: boolean;
1515
readonly allowDownload?: boolean;
1616
readonly full?: boolean;
17+
/** ADR §27a. Threaded straight through to the shared dispatcher's own `lease.request` input --
18+
* the same gate every other transport is held to (`FORBIDDEN` for a non-admin token) decides
19+
* this, not this route (H7, round 2 review: before this, `leaseRequestBodySchema` had no
20+
* `owner` field at all and quietly discarded one a caller sent, the anti-pattern this
21+
* codebase's own `device.exec`'s `requesterId` precedent already rejected for the same reason
22+
* -- an identity named and answered as though it had not been is the kind of silence that
23+
* reads like authorization). */
24+
readonly owner?: string;
1725
}
1826

1927
/** Matches the issue's lease object exactly; `dataPlane` is reserved and always `null` in v1. */
@@ -215,6 +223,9 @@ export class LeaseRequestTracker {
215223
// old grant-then-immediately-renew hack. Under ADR 0004 the daemon then stores
216224
// that width on the lease, so nothing here has to remember it either.
217225
...(body.ttlMs === undefined ? {} : { ttlMs: body.ttlMs }),
226+
// ADR §27a (H7, round 2 review): forwarded as-is -- the shared dispatcher's own
227+
// `lease.request` handler is what rejects a non-admin token naming this.
228+
...(body.owner === undefined ? {} : { owner: body.owner }),
218229
},
219230
session,
220231
)

0 commit comments

Comments
 (0)