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
29 changes: 27 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ a second copy. `rp-login/step-up.ts` re-exports every name, and
`tests/rp-login.step-up.mjs` passes unchanged, which is what says the move was
non-breaking.

**Reveal lives in `FactValue`'s own state**, per value, and nowhere else. Lifted
**Reveal lives in `AttributeValue`'s own state**, per value, and nowhere else. Lifted
to the pane and keyed by fact id it would be a store of "things unhidden" that
outlives the card the person was looking at and is one refactor from a *Show
all*. Component state cannot become that: it dies with the element, so leaving
Expand All @@ -538,14 +538,39 @@ again, or catching the refusal and rethrowing it; matching the step-up code in
`unverifiedApproveRequest` instead of the verified `context`; dropping the
`previewId` cross-check; moving the verify/sign half back up beside the RP
flow; a surface that formats a value itself instead of rendering
`FactValue` (the second surface is always the one added later, and a value
`AttributeValue` (the second surface is always the one added later, and a value
masked on the card and printed in the strip is masked nowhere); greying a mask
with `c.faint`, which is this pane's word for "the agent sent no value" and so
makes a fact the holder has look like one they do not; reintroducing a compiled
table as a fallback for a registry that has not loaded; or letting a UI string
imply the console does not hold
what it hides.

**A listing is read to the end, and that is the client's job.** Every
`persona/…/list` task is cursor-paginated, and the specification is explicit: a
producer MUST NOT infer exhaustion from a short page — only an absent
`nextCursor` means the end. `personaAttributeList`, `personaProfileList` and
`listBindings` returned the first page and dropped the cursor, which nothing
downstream could detect, because a short array is indistinguishable from a
complete one: past the agent's page size (100 by default) the identity map drew
a face pointing at attributes that were not in its own list, under counts that
agreed with each other because they counted the same truncated array. All three
now follow the cursor through `collectPages` (`core/src/util/pages.ts`), and
`listBindings` returns a document with no `nextCursor` because there is nothing
left to fetch. `limit` on those calls is the **page size to ask for**, never a
cap on the result.

**The bound throws rather than truncating.** `MAX_PAGES` guards against a far
side that will not end — a cursor that repeats, or a pool beyond anything a
picture can draw — and returning what had been collected would reintroduce the
defect one layer down and with a longer array. It is set high enough (50 pages,
25,000 records at the maximum page size) that reaching it means a fault rather
than a large pool.

**What breaks it:** reading `.attributes` / `.profiles` / `.personas` off a
single response again; treating `limit` as a cap; or catching the bound's error
and returning a partial list.

**The console's components are rendered in tests, and this is how.**
`tests/harness/` holds module hooks and a DOM so `node --test` can mount a
pane. Two things it does that are not obvious: it resolves a `./thing.js`
Expand Down
65 changes: 44 additions & 21 deletions packages/core/src/admin/persona.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
// context. A context never pulls, and there is no task in this file that would
// let it.

import { collectPages } from "../util/pages.js";
import type { TaskParty, TrustTaskSender } from "../vta/channel.js";
import { buildTrustTask } from "../vta/trust-task.js";

Expand Down Expand Up @@ -202,11 +203,27 @@ export interface AttributeListParams extends PersonaHolderParams {
* needs to see that something went stale rather than have it quietly omitted.
*/
includeStale?: boolean;
/**
* The page size to ask for, **not** a cap on what comes back: this call
* follows `nextCursor` to the end. Left unset the agent picks (100 today).
*/
limit?: PersonaAttributeListPayload["limit"];
/** Where to start. Everything from there is returned, not one page of it. */
cursor?: PersonaAttributeListPayload["cursor"];
}

/** Enumerate the pool. Metadata only unless `includeValues` is set. */
/**
* Enumerate the pool. Metadata only unless `includeValues` is set.
*
* **Reads to the end**, following `nextCursor`. It used to return the first page
* and drop the cursor, which the specification names directly as the mistake —
* "a producer MUST NOT infer exhaustion from a short page — only an absent
* `nextCursor` means the end" — and which is invisible from the outside: a
* holder past the agent's page size got a silently short pool, and the console's
* identity map drew a face pointing at attributes that were not in it.
*
* See `collectPages` for what happens when the far side will not end.
*/
export async function personaAttributeList(
sender: TrustTaskSender,
params: AttributeListParams,
Expand All @@ -219,15 +236,17 @@ export async function personaAttributeList(
...(params.limit !== undefined ? { limit: params.limit } : {}),
...(params.cursor !== undefined ? { cursor: params.cursor } : {}),
};
const res = await holderCall<PersonaAttributeListPayload, PersonaAttributeListResponsePayload>(
sender,
params,
ATTRIBUTE_LIST,
ATTRIBUTE_LIST_RESPONSE,
"persona/attribute/list/1.0",
payload,
);
return res.attributes ?? [];
return collectPages("persona/attribute/list", async (cursor) => {
const res = await holderCall<PersonaAttributeListPayload, PersonaAttributeListResponsePayload>(
sender,
params,
ATTRIBUTE_LIST,
ATTRIBUTE_LIST_RESPONSE,
"persona/attribute/list/1.0",
cursor === undefined ? payload : { ...payload, cursor },
);
return { items: res.attributes ?? [], nextCursor: res.nextCursor };
});
}

export interface AttributePutParams extends PersonaHolderParams {
Expand Down Expand Up @@ -369,8 +388,10 @@ export async function personaAttributeDelete(

// ── Profiles ────────────────────────────────────────────────────────────────

/** Every profile the holder has. Names and entries; never resolved values —
* see {@link personaProfileGet} for why there is no `resolve` here. */
/** Every profile the holder has — **to the end of the listing**, like
* {@link personaAttributeList}. Names and entries; never resolved values, see
* {@link personaProfileGet} for why there is no `resolve` here. `limit` is the
* page size to ask for, not a cap on the result. */
export async function personaProfileList(
sender: TrustTaskSender,
params: PersonaHolderParams & { limit?: PersonaProfileListPayload["limit"]; cursor?: string },
Expand All @@ -379,15 +400,17 @@ export async function personaProfileList(
...(params.limit !== undefined ? { limit: params.limit } : {}),
...(params.cursor !== undefined ? { cursor: params.cursor } : {}),
};
const res = await holderCall<PersonaProfileListPayload, PersonaProfileListResponsePayload>(
sender,
params,
PROFILE_LIST,
PROFILE_LIST_RESPONSE,
"persona/profile/list/1.0",
payload,
);
return res.profiles ?? [];
return collectPages("persona/profile/list", async (cursor) => {
const res = await holderCall<PersonaProfileListPayload, PersonaProfileListResponsePayload>(
sender,
params,
PROFILE_LIST,
PROFILE_LIST_RESPONSE,
"persona/profile/list/1.0",
cursor === undefined ? payload : { ...payload, cursor },
);
return { items: res.profiles ?? [], nextCursor: res.nextCursor };
});
}

export interface ProfileGetParams extends PersonaHolderParams {
Expand Down
39 changes: 30 additions & 9 deletions packages/core/src/persona/bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import {
type PersonaBindingListResponsePayload,
} from "@openvtc/trust-tasks/persona/binding/list/1.0/payload";

import { collectPages } from "../util/pages.js";

import { call, type PersonaCallerParams } from "./call.js";

export type PersonaBinding = PersonaBindingGetResponsePayload;
Expand Down Expand Up @@ -67,7 +69,22 @@ export interface ListBindingsParams extends PersonaCallerParams {
cursor?: PersonaBindingListPayload["cursor"];
}

/** Every persona bound in this context. */
/**
* Every persona present in this context — **to the end of the listing**.
*
* Not "every persona *bound*": the response carries `bound` per entry precisely
* because an unbound persona is still present, and a caller deciding what a
* context knows of the holder needs both kinds.
*
* The returned document carries no `nextCursor`, because there is nothing left
* to fetch. That absence is the honest report of what this now does, and a
* caller that used to ignore the member is correct by construction rather than
* by luck — which is what it was before: two console surfaces read `.personas`
* off the first page and drew the result as the whole truth.
*
* `limit` is the page size to ask for; `cursor` is where to start. Neither caps
* the result.
*/
export async function listBindings(
sender: TrustTaskSender,
params: ListBindingsParams,
Expand All @@ -77,12 +94,16 @@ export async function listBindings(
...(params.limit !== undefined ? { limit: params.limit } : {}),
...(params.cursor !== undefined ? { cursor: params.cursor } : {}),
};
return call<PersonaBindingListPayload, PersonaBindingListResponsePayload>(
sender,
params,
BINDING_LIST,
BINDING_LIST_RESPONSE,
"persona/binding/list",
payload,
);
const personas = await collectPages("persona/binding/list", async (cursor) => {
const res = await call<PersonaBindingListPayload, PersonaBindingListResponsePayload>(
sender,
params,
BINDING_LIST,
BINDING_LIST_RESPONSE,
"persona/binding/list",
cursor === undefined ? payload : { ...payload, cursor },
);
return { items: res.personas ?? [], nextCursor: res.nextCursor };
});
return { personas };
}
1 change: 1 addition & 0 deletions packages/core/src/util/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
// `util` is the bottom of the layering, and the boundary test enforces it.

export * from "./base64url.js";
export * from "./pages.js";
export * from "./timing.js";
74 changes: 74 additions & 0 deletions packages/core/src/util/pages.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Reading a cursor-paginated listing to the end.
//
// ## Why a helper rather than a loop at each call site
//
// Every `persona/…/list` task in the persona family answers with a page and a
// `nextCursor`, and the specification is explicit about how that must be read:
// "a producer MUST NOT infer exhaustion from a short page — only an absent
// `nextCursor` means the end." The clients here were named `...List`, documented
// as *enumerate the pool* and *every persona bound in this context*, and
// returned the first page while dropping the cursor on the floor.
//
// That failure is invisible in exactly the way that matters. Nothing errors,
// nothing looks partial, and the console's identity map — whose whole premise is
// one picture of everything — draws a face pointing at attributes that are not
// in the list, under counts that agree with each other because they are all
// counting the same truncated array. A picture that reads as complete while
// being partial is the one wrong answer that pane must never give.
//
// ## The bound is not a page limit, it is a loop guard
//
// `maxPages` exists because a cursor comes from the far side: an agent that
// returns the same cursor forever, or one whose pool genuinely exceeds what any
// caller could draw, must not spin this in a service worker. It is set high
// enough that reaching it means something is wrong rather than something is
// large.
//
// **Reaching it throws.** Returning what was collected would reintroduce the
// defect this exists to fix, one layer down and with a longer array — and a
// caller cannot tell a short answer from a complete one, which is the whole
// problem. An error names the situation and the surface can say so.

/** One page, in the shape every `…/list` response shares. */
export interface Page<T> {
items: T[];
nextCursor?: string | undefined;
}

/** How many pages `collectPages` will read before deciding the far side is
* misbehaving. At the specification's maximum page size of 500 this is 25,000
* records — beyond any pool a person curates by hand, which is the point. */
export const MAX_PAGES = 50;

/**
* Follow `nextCursor` until it is absent, and return everything.
*
* `fetchPage` is called with `undefined` first and with each cursor after it.
* `what` names the listing in the error, because "too many pages" without a
* subject is a message nobody can act on.
*/
export async function collectPages<T>(
what: string,
fetchPage: (cursor?: string | undefined) => Promise<Page<T>>,
maxPages: number = MAX_PAGES,
): Promise<T[]> {
const all: T[] = [];
let cursor: string | undefined;
for (let page = 0; page < maxPages; page += 1) {
const next: Page<T> = await fetchPage(cursor);
all.push(...next.items);
// Absence is the only exhaustion signal — a short page is not one, and an
// empty page carrying a cursor is a legal answer this must keep following.
if (next.nextCursor === undefined) return all;
// A cursor that does not move is the far side looping. Caught here rather
// than by the page bound alone so the message says which fault it was.
if (next.nextCursor === cursor) {
throw new Error(`${what}: your agent returned the same page cursor twice, so the listing cannot be finished`);
}
cursor = next.nextCursor;
}
throw new Error(
`${what}: still more after ${maxPages} pages — refusing to keep asking, because a listing this long is ` +
`an agent misbehaving rather than a pool this size`,
);
}
80 changes: 80 additions & 0 deletions packages/core/tests/admin.persona.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
personasBlockingDelete,
PROFILE_DELETE_BOUND,
} from "../dist/admin/index.js";
import { listBindings } from "../dist/persona/index.js";

const HOLDER = { did: "did:key:zHolder" };
const SERVICE = { did: "did:webvh:QmAgent:agent.example" };
Expand Down Expand Up @@ -411,3 +412,82 @@ test("a values listing can ask for the sensitive ones, and does not by default",
await personaAttributeList(plain, { ...PARTIES, includeValues: true });
assert.ok(!("includeSensitive" in plain.sent[0].envelope.payload));
});

// ── A listing is read to the end, not to the first page ────────────────────
//
// `*List` clients returned the first page and dropped `nextCursor`, which the
// specification names as the mistake — "a producer MUST NOT infer exhaustion
// from a short page" — and which nothing downstream could detect: a short array
// is indistinguishable from a complete one. The console's identity map drew the
// result as the whole truth.

/** Answers with each reply in turn, recording what it was asked. */
function pages(...replies) {
const sent = [];
return {
sent,
send(envelope) {
sent.push({ envelope });
return Promise.resolve(replies[sent.length - 1] ?? replies[replies.length - 1]);
},
};
}

const poolAttribute = (id) => ({
attributeId: id,
type: "name.legal",
valueType: "string",
provenance: { kind: "selfAsserted" },
version: 1,
updatedAt: "x",
});

test("the pool is read to the end, and the cursor goes back with the next request", async () => {
const r = pages(
{ attributes: [poolAttribute("a1")], nextCursor: "c1" },
{ attributes: [poolAttribute("a2")] },
);
const all = await personaAttributeList(r, { ...PARTIES });
assert.deepEqual(all.map((a) => a.attributeId), ["a1", "a2"]);
assert.equal(r.sent.length, 2);
assert.equal(r.sent[0].envelope.payload.cursor, undefined, "the first request invents no cursor");
assert.equal(r.sent[1].envelope.payload.cursor, "c1");
});

test("paging preserves the rest of the request, so a narrowed listing stays narrowed", async () => {
// The second page of a `typePrefix` query that forgot the prefix would return
// the whole pool — and `reveal-value.ts` matches by id, so it would quietly
// read every value the holder has to answer a question about one.
const r = pages(
{ attributes: [poolAttribute("a1")], nextCursor: "c1" },
{ attributes: [poolAttribute("a2")] },
);
await personaAttributeList(r, { ...PARTIES, typePrefix: "phone", includeValues: true, includeSensitive: true });
const second = r.sent[1].envelope.payload;
assert.equal(second.typePrefix, "phone");
assert.equal(second.includeValues, true);
assert.equal(second.includeSensitive, true);
});

test("faces are read to the end too", async () => {
const r = pages(
{ profiles: [{ profileId: "p1", name: "One", entries: [], version: 1, updatedAt: "x" }], nextCursor: "c1" },
{ profiles: [{ profileId: "p2", name: "Two", entries: [], version: 1, updatedAt: "x" }] },
);
const all = await personaProfileList(r, { ...PARTIES });
assert.deepEqual(all.map((p) => p.profileId), ["p1", "p2"]);
});

test("every persona in a context is read to the end, and no cursor comes back", async () => {
// The returned document carries no `nextCursor` because there is nothing left
// to fetch — the two console surfaces that ignored the member are correct by
// construction now rather than by luck.
const r = pages(
{ personas: [{ personaDid: "did:key:zA", bound: true }], nextCursor: "c1" },
{ personas: [{ personaDid: "did:key:zB", bound: false }] },
);
const res = await listBindings(r, { ...PARTIES, contextId: "openvtc" });
assert.deepEqual(res.personas.map((p) => p.personaDid), ["did:key:zA", "did:key:zB"]);
assert.equal(res.nextCursor, undefined);
assert.equal(r.sent[1].envelope.payload.contextId, "openvtc", "the context survives the second request");
});
Loading
Loading