diff --git a/packages/extension/src/manager/key-name.ts b/packages/extension/src/manager/key-name.ts new file mode 100644 index 0000000..9ebdc1c --- /dev/null +++ b/packages/extension/src/manager/key-name.ts @@ -0,0 +1,86 @@ +// What a key may be called, and what the console must say before it renames one. +// +// `keys/rename/0.1` says only that `newKeyId` is a non-empty string — the +// agent's gate is narrower than the specification's, and it is the one that +// answers. `vti_common::identifier::validate_identifier` accepts +// `[A-Za-z0-9._-]` up to 64 bytes and rejects everything else, because a key id +// lands as a store key and a caller who can inject `:` or `/` collides with an +// adjacent keyspace. +// +// So this module exists for the same reason `grant-command.ts` does: the rule +// is the agent's, but a console that does not apply it hands the operator a +// refusal instead of a form. It is a **pre-flight, not the decision** — the +// agent still answers, and its refusal is still rendered. Nothing here may +// widen the rule; a value this module accepts and the agent refuses is a +// message the operator can act on, while the reverse is a name the agent would +// have taken and the console pretended it would not. +// +// The shape that made this necessary: every key this agent mints for a DID is +// named by a DID URL (`did:webvh:…#key-0`), which is 86 bytes of `:` and `#` — +// not a name the rename gate can ever accept. The old editor pre-filled the +// field with exactly that, so the only thing pressing Save could produce was +// `new_key_id is 86 bytes; maximum is 64`. + +/** The agent's cap, in bytes — `validate_identifier`'s `MAX_IDENTIFIER_LEN`. */ +export const MAX_KEY_NAME_BYTES = 64; + +const ALLOWED = /^[A-Za-z0-9._-]+$/; + +/** + * Why the agent would refuse this name, or `null` if it would take it. + * + * Phrased for the operator rather than as the agent's own wording: the agent + * reports bytes and a character class, which is an accurate description of a + * gate and a poor description of what to type instead. + */ +export function keyNameProblem(name: string): string | null { + if (name === "") return "A name cannot be empty."; + // The common mistake is pasting the id that is already on screen. It fails + // the character rule *and* the length one, and neither message says the + // thing worth knowing: a name is not an address. + if (name.startsWith("did:")) { + return "A name is not a DID URL — your agent stores one as a plain label: letters, digits, and . - _ only."; + } + if (!ALLOWED.test(name)) { + return "Your agent accepts letters, digits, and . - _ only."; + } + const bytes = new TextEncoder().encode(name).length; + if (bytes > MAX_KEY_NAME_BYTES) { + return `That is ${bytes} characters; your agent accepts at most ${MAX_KEY_NAME_BYTES}.`; + } + return null; +} + +/** Whether a key's current id is one the agent would accept as a new name — + * false for every DID-bound key, which is most of them. */ +export function isNameable(keyId: string): boolean { + return keyNameProblem(keyId) === null; +} + +/** + * A name to offer for a key that has none — derived from the DID URL it is + * addressed by today. + * + * `did:webvh:Qm…:webvh.storm.ws:vdr-host#key-0` becomes `vdr-host-key-0`: the + * last path segment, which is what a person calls that identity, and the + * fragment, which is what distinguishes its keys from each other. Offered as a + * *placeholder* rather than a value — a pre-filled suggestion is one press away + * from a rename nobody chose, and a rename detaches the key from the document + * that names it. + * + * Returns `""` when nothing usable survives, which the caller must treat as + * "no suggestion" rather than as a name. + */ +export function suggestKeyName(keyId: string): string { + const hash = keyId.indexOf("#"); + const address = hash === -1 ? keyId : keyId.slice(0, hash); + const fragment = hash === -1 ? "" : keyId.slice(hash + 1); + const tail = address.split(":").filter(Boolean).pop() ?? ""; + const joined = [tail, fragment].filter(Boolean).join("-"); + const cleaned = joined + .replace(/[^A-Za-z0-9._-]+/g, "-") + .replace(/-{2,}/g, "-") + .replace(/^[-._]+|[-._]+$/g, ""); + // Cleaned is ASCII, so bytes and characters agree and a slice is safe. + return cleaned.slice(0, MAX_KEY_NAME_BYTES).replace(/[-._]+$/, ""); +} diff --git a/packages/extension/src/manager/panes/keys.tsx b/packages/extension/src/manager/panes/keys.tsx index 57fc04a..ecccf24 100644 --- a/packages/extension/src/manager/panes/keys.tsx +++ b/packages/extension/src/manager/panes/keys.tsx @@ -11,7 +11,7 @@ // key is not an administration task, it is use, and a console that offers a // "sign this" box turns an audit trail of key management into an oracle. -import { useCallback, useState } from "react"; +import { useCallback, useEffect, useId, useRef, useState } from "react"; import { keysCreate, keysList, @@ -29,6 +29,12 @@ import { Loading, LoadError, Table, Truncated, type Column } from "../table.js"; import { useAsync } from "../use-async.js"; import { formatDate } from "../format.js"; import { hasRole, type Authority, type Parties } from "../use-vta.js"; +import { + isNameable, + keyNameProblem, + suggestKeyName, + MAX_KEY_NAME_BYTES, +} from "../key-name.js"; import type { ContextSelection } from "../context-column.js"; const PAGE = 50; @@ -198,37 +204,101 @@ function CreateKey({ ); } -function RenameKey({ +/** + * Rename a key — the editor, which opens beneath the row rather than inside the + * actions column. + * + * Two things this is shaped by, both of which the previous version got wrong. + * + * **A name is not the id.** Every key an agent mints for a DID is addressed by + * a DID URL, and `keys/rename` will not take one back: the agent's gate is + * `[A-Za-z0-9._-]` up to 64 bytes (`key-name.ts`), so the pre-filled current id + * was the one value guaranteed to be refused — an operator pressing Save got + * `new_key_id is 86 bytes; maximum is 64` and no way to read what would work. + * The field now starts empty for such a key, with a suggestion as placeholder, + * and the rule is stated before it is broken. + * + * **And it is a one-way door.** A DID document addresses this key by the id it + * has today; the agent cannot be given that id again, because it contains `:` + * and `#`. So the warning is not decoration — nothing undoes it. + */ +function RenameEditor({ parties, record, + onClose, onDone, }: { parties: Parties; record: KeyRecord; + onClose: () => void; onDone: () => void; }) { - const [open, setOpen] = useState(false); - const [next, setNext] = useState(record.keyId); + const nameable = isNameable(record.keyId); + const [next, setNext] = useState(nameable ? record.keyId : ""); const [busy, setBusy] = useState(false); const [error, setError] = useState(null); const [pending, setPending] = useState(null); + const fieldId = useId(); + const input = useRef(null); - if (!open) { - return ( - - ); - } + // Opened from a button several columns to the right; without this the caret + // is nowhere and the row that just appeared looks like a decoration. + useEffect(() => { + input.current?.focus(); + }, []); + + const typed = next.trim(); + const suggestion = nameable ? "" : suggestKeyName(record.keyId); + // Nothing typed is not a problem to report — it is the state the field opens + // in. Reporting it would put a red line under an empty box. + const problem = typed === "" ? null : keyNameProblem(typed); + const unchanged = typed === record.keyId; return ( -
- setNext(e.target.value)} /> +
+ + setNext(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Escape") onClose(); + }} + /> + + Letters, digits, and . - _ — up to{" "} + {MAX_KEY_NAME_BYTES} characters. Your agent refuses anything else. + {suggestion && ( + <> + {" "} + The field is empty on purpose —{" "} + {suggestion} is a suggestion, not a + value. + + )} + + + {!nameable && ( + + This key is addressed as {record.keyId}, + which is how a DID document names it. A name cannot contain{" "} + : or{" "} + #, so renaming it detaches the key from + that document and your agent will not accept the old id back. + + )} + {problem && {problem}} {error && {error}} {pending && } +
-
@@ -274,6 +344,10 @@ export function KeysPane({ contextHeading?: string | undefined; }) { const [limit, setLimit] = useState(PAGE); + // Which key is being renamed, held here rather than in the row: the editor + // renders as the table's expanded row, which is a sibling of the cells, not + // a child of one. + const [renaming, setRenaming] = useState(null); const list = useAsync( () => @@ -329,7 +403,12 @@ export function KeysPane({ revoked ) : (
- + label="Revoke" disabledReason={revokeDenied} @@ -372,6 +451,19 @@ export function KeysPane({ columns={columns} rows={list.data.keys} rowKey={(k) => k.keyId} + // Beneath the key it renames, full width — the actions column is + // the narrowest in the table and a DID-shaped field squeezed into + // it shows about eight characters of the thing being edited. + expanded={(k) => + renaming === k.keyId ? ( + setRenaming(null)} + onDone={list.reload} + /> + ) : null + } empty={ contextId ? `No keys in ${contextId}. Keys minted into this context appear here.` diff --git a/packages/extension/src/manager/panes/rooms-create.tsx b/packages/extension/src/manager/panes/rooms-create.tsx index 5b7a661..2a37359 100644 --- a/packages/extension/src/manager/panes/rooms-create.tsx +++ b/packages/extension/src/manager/panes/rooms-create.tsx @@ -68,6 +68,18 @@ // records by that prefix — a convention of this agent rather than a contract, // which is why typing one stays possible. // +// **The key record's id and the DID document's verification method are not the +// same string for a templated mint, and here they disagree by one.** The `room` +// and `room-host` built-in templates (`vta-sdk/templates/`) number their +// verification methods `#key-1` (signing) and `#key-2` (key agreement), while +// the create path saves the key records as `#key-0` and `#key-1` — so the +// document's `#key-1` names the signing key and the keystore's names the +// x25519 one. The public keys still line up positionally, which is why a room +// credential (the VTA signs it naming `{room_did}#key-1`, matching the +// template) verifies; what does not survive is *addressing a key by the id the +// document publishes*. Hence the filter below: the offer is by prefix and +// `keyType !== "x25519"`, never by fragment number. Reported upstream. +// // Minting can succeed and registration fail. The DID is real and the key // identifier is the half nobody writes down, so `Minted` keeps both on screen // and the flow switches to the existing-identity path pre-filled: the retry diff --git a/packages/extension/tests/key-name.test.mts b/packages/extension/tests/key-name.test.mts new file mode 100644 index 0000000..2f765d3 --- /dev/null +++ b/packages/extension/tests/key-name.test.mts @@ -0,0 +1,78 @@ +// What the console lets an operator type into the rename field. +// +// The rule is the agent's (`vti_common::identifier`), applied here so a refusal +// arrives as a sentence under the field rather than as +// `new_key_id is 86 bytes; maximum is 64` after the fact. Every assertion is a +// shape the live console produced: a DID-URL key id pre-filled into the field, +// and the suggestion offered in its place. + +import { strict as assert } from "node:assert"; +import { test } from "node:test"; + +import { + isNameable, + keyNameProblem, + suggestKeyName, + MAX_KEY_NAME_BYTES, +} from "../src/manager/key-name.js"; + +const DID_KEY = "did:webvh:QmRqxWCVUE6Y474EPdNZmR9m3giTRyDgmXSdidvEy7iSwq:webvh.storm.ws:vdr-host#key-1"; + +test("an ordinary slug is accepted", () => { + for (const ok of ["app-signing-key-2026", "vdr.host", "_private", "CamelCase", "0"]) { + assert.equal(keyNameProblem(ok), null, ok); + assert.equal(isNameable(ok), true, ok); + } +}); + +test("the id the field used to be pre-filled with is refused, and says why", () => { + // The bug this module ends: the only thing pressing Save could do was fail. + assert.equal(isNameable(DID_KEY), false); + const why = keyNameProblem(DID_KEY); + assert.ok(why?.includes("not a DID URL"), why ?? "no reason given"); +}); + +test("a separator is refused whatever its length", () => { + // These are the agent's own attack shapes — a key id lands as a store key. + for (const bad of ["global:evil", "../../etc", "my/ctx", "with space", "quote\"it"]) { + assert.notEqual(keyNameProblem(bad), null, bad); + } +}); + +test("the length cap is the agent's, and counts bytes", () => { + assert.equal(keyNameProblem("a".repeat(MAX_KEY_NAME_BYTES)), null); + const over = keyNameProblem("a".repeat(MAX_KEY_NAME_BYTES + 1)); + assert.ok(over?.includes(String(MAX_KEY_NAME_BYTES)), over ?? "no reason given"); +}); + +test("empty is a state, not a violation of the character rule", () => { + assert.ok(keyNameProblem("")?.includes("empty")); +}); + +test("the suggestion names the identity and the key, and would be accepted", () => { + const s = suggestKeyName(DID_KEY); + assert.equal(s, "vdr-host-key-1"); + assert.equal(keyNameProblem(s), null); +}); + +test("a suggestion is derived from the last path segment, not the SCID", () => { + const s = suggestKeyName( + "did:webvh:Qma9EKoeEJqtWpvAdGdrsZMcEFL5QpKrVYsJDyVxd87pCd:webvh.storm.ws:rooms:open-demo#key-0", + ); + assert.equal(s, "open-demo-key-0"); +}); + +test("every suggestion is a name the agent would take", () => { + // The placeholder is read as an offer. One the agent refuses is worse than + // none — it teaches the rule wrongly at exactly the moment it is being read. + for (const id of [ + DID_KEY, + "did:key:z6MkExample#z6MkExample", + "did:webvh:Qm:host#key-0", + "did:peer:2.Ez6LS.Vz6Mk#key-1", + ]) { + const s = suggestKeyName(id); + assert.notEqual(s, "", id); + assert.equal(keyNameProblem(s), null, `${id} → ${s}`); + } +}); diff --git a/packages/extension/tests/keys-pane.render.test.mts b/packages/extension/tests/keys-pane.render.test.mts new file mode 100644 index 0000000..ba2e258 --- /dev/null +++ b/packages/extension/tests/keys-pane.render.test.mts @@ -0,0 +1,117 @@ +// The keys pane, rendered — and specifically the rename editor. +// +// Both bugs this pins were on screen in the live console and invisible to the +// type checker and to `key-name.ts`'s own tests. +// +// **The field was pre-filled with the key's id**, which for every DID-bound key +// is a DID URL: `:` and `#` the agent's identifier gate refuses, at 86 bytes +// against a 64-byte cap. The only thing pressing Save could produce was +// `keys/rename/0.1 failed: … new_key_id is 86 bytes; maximum is 64`, and +// nothing on screen said what would have worked. +// +// **And it rendered inside the actions column**, the narrowest in the table, so +// the field showed about eight characters of the value being edited. It belongs +// under the row, where the width is the pane's. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { agent, h, render, PARTIES } from "./harness/dom.mjs"; +import { KeysPane } from "../src/manager/panes/keys.js"; + +const LIST = "keys/list/0.1"; +const RENAME = "keys/rename/0.1"; + +const DID = "did:webvh:QmRqxWCVUE6Y474EPdNZmR9m3giTRyDgmXSdidvEy7iSwq:webvh.storm.ws:vdr-host"; +const DID_KEY = `${DID}#key-0`; + +const key = (keyId: string, keyType = "ed25519") => ({ + keyId, + keyType, + status: "active", + origin: "derived", + createdAt: "2026-09-11T00:00:00Z", + contextId: "vdr", +}); + +const mount = async (keys: unknown[]) => { + const a = agent({ + [LIST]: { keys, total: keys.length, offset: 0, limit: 50 }, + [RENAME]: { keyId: "renamed", updatedAt: "2026-09-14T00:00:00Z" }, + }); + const screen = await render( + h(KeysPane, { parties: PARTIES, authority: null, contextId: "vdr" } as never), + { chrome: { runtime: { sendMessage: a.sendMessage } } }, + ); + return { a, screen }; +}; + +/** The rename field — the only text input inside the table. */ +const field = (screen: { all: (s: string) => Element[] }) => + screen.all("td input[type='text'], td input:not([type])")[0] as + | (Element & { value: string }) + | undefined; + +test("the editor opens beneath the key, not inside the actions column", async () => { + const { screen } = await mount([key(DID_KEY)]); + await screen.click(screen.button("Rename")); + const input = field(screen); + assert.ok(input, "no field appeared"); + // The full-width detail row is a ``; a field in the actions + // column has no such ancestor and is as wide as "Rename" is. + const cell = input.closest("td"); + assert.ok(cell?.hasAttribute("colspan"), "the editor is not in the table's full-width row"); +}); + +test("a DID-bound key opens with an empty field, and offers a name that would work", async () => { + const { screen } = await mount([key(DID_KEY)]); + await screen.click(screen.button("Rename")); + const input = field(screen)!; + assert.equal(input.value, "", "the field is pre-filled with a value the agent always refuses"); + assert.equal(input.getAttribute("placeholder"), "vdr-host-key-0"); + // Nothing typed: there is nothing to save, and the button says so rather + // than sending a refusal the operator has to read as an error. + assert.ok((screen.button("Save") as HTMLButtonElement).disabled); +}); + +test("the rule is on screen before it is broken", async () => { + const { screen } = await mount([key(DID_KEY)]); + await screen.click(screen.button("Rename")); + const text = screen.text(); + assert.match(text, /up to 64 characters/); + // And the one-way door: the agent cannot be given a DID URL back. + assert.match(text, /will not accept the old id back/); +}); + +test("pasting the id back is refused here, with the reason, and nothing is sent", async () => { + const { a, screen } = await mount([key(DID_KEY)]); + await screen.click(screen.button("Rename")); + await screen.type(field(screen)!, DID_KEY); + assert.match(screen.text(), /not a DID URL/); + assert.ok((screen.button("Save") as HTMLButtonElement).disabled); + assert.equal( + a.calls.filter((c: { type: string }) => c.type.endsWith(RENAME)).length, + 0, + "a name the agent would refuse was sent anyway", + ); +}); + +test("a valid name is sent as typed, against the id the key has today", async () => { + const { a, screen } = await mount([key(DID_KEY)]); + await screen.click(screen.button("Rename")); + await screen.type(field(screen)!, "vdr-host-signing"); + await screen.click(screen.button("Save")); + await screen.settle(); + const call = a.calls.find((c: { type: string }) => c.type.endsWith(RENAME)); + assert.ok(call, "nothing was sent"); + assert.deepEqual(call.payload, { keyId: DID_KEY, newKeyId: "vdr-host-signing" }); +}); + +test("a key that already has a name opens on it, and Save waits for a change", async () => { + const { screen } = await mount([key("app-signing-key")]); + await screen.click(screen.button("Rename")); + assert.equal(field(screen)!.value, "app-signing-key"); + assert.ok( + (screen.button("Save") as HTMLButtonElement).disabled, + "renaming a key to the name it has is a write with nothing in it", + ); +}); diff --git a/packages/extension/tests/manager-form-state.test.mts b/packages/extension/tests/manager-form-state.test.mts index 54aabef..c87381a 100644 --- a/packages/extension/tests/manager-form-state.test.mts +++ b/packages/extension/tests/manager-form-state.test.mts @@ -41,7 +41,7 @@ const ROOT = fileURLToPath(new URL("../src/manager", import.meta.url)); * that adding a key is inconvenient. */ const KEYED_ELSEWHERE: Record = { - RenameKey: "rendered per row by `Table`, which keys each on rowKey", + RenameEditor: "rendered as `Table`'s expanded row, inside the fragment keyed on rowKey", ChangeRole: "rendered per row by `Table`, which keys each on rowKey", }; @@ -75,8 +75,12 @@ function seededComponents(): { name: string; where: string }[] { const body = src.slice(m.index + m[0].length); const end = body.search(/\n(?:export )?function /); const scope = end === -1 ? body : body.slice(0, end); + // The prop may be read anywhere inside the `useState(…)` call, not only + // as its whole argument: `useState(nameable ? record.keyId : "")` is + // seeded from `record` exactly as much as `useState(record.keyId)` is, + // and the narrower match read it as a component with no state at all. const seeds = props.some((p) => - new RegExp(`useState\\(\\s*${p}\\.`).test(scope) || new RegExp(`useState\\(\\s*${p}\\?\\.`).test(scope), + new RegExp(`useState\\([^;]*?\\b${p}\\??\\.`).test(scope), ); if (seeds) found.push({ name, where: rel }); }