From 23434b6e10bd71b2620be3512219dc9511c3e71d Mon Sep 17 00:00:00 2001 From: Brian Richter Date: Wed, 29 Jul 2026 02:54:43 -0700 Subject: [PATCH] =?UTF-8?q?fix(did):=20pass=20bare=20multibase=20in=20upda?= =?UTF-8?q?teKeys=20=E2=80=94=20DID=20minting=20was=20fully=20broken?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No did:webvh could be created at all since the didwebvh-ts 2.8 upgrade. Every mint threw: Key did:key:z6Mk… is not authorized to update. 2.8's isKeyAuthorized parses the proof's verificationMethod down to its keyMultibase and compares by exact equality: updateKeys.some((k) => k === parsed.keyMultibase) Every call site passed `signer.getVerificationMethodId()` — the full `did:key:z6Mk…` URI — so the comparison could never match. didwebvh-ts verifies the log it has just created, so creation failed closed. Reproduced with a brand-new key and empty localStorage, which ruled out the earlier theory that this was about a lost controller key: it failed identically for a fresh identity. That also explains why the re-mint hook never reached Convex — it died in the browser at the mint step, before any request. Fixed at all four sites: user DIDs (lib/webvh.ts), server-side DID creation (didCreation.ts), and both site paths (siteActions.ts createDID + updateDID) — site publishing carried the same defect. Adds scripts/webvh-mint.test.mjs, which performs a real mint. didwebvh-ts verifies its own output, so a malformed updateKeys fails the test exactly as it failed in the browser. The harness installs browser globals via defineProperty (bun's localStorage is readonly) and restores them, since bun shares globals across test files. Also removes convex/remintDid.ts. It was meant to go in #217 — the `rm` was in a command the sandbox blocked, so it merged as dead code; the client now uses the JWT-authorized /api/user/remintDid endpoint. Co-Authored-By: Claude Opus 5 (1M context) --- convex/_generated/api.d.ts | 2 - convex/didCreation.ts | 4 +- convex/remintDid.ts | 109 ------------------------------------ convex/siteActions.ts | 10 ++-- scripts/webvh-mint.test.mjs | 103 ++++++++++++++++++++++++++++++++++ src/lib/webvh.ts | 6 +- 6 files changed, 117 insertions(+), 117 deletions(-) delete mode 100644 convex/remintDid.ts create mode 100644 scripts/webvh-mint.test.mjs diff --git a/convex/_generated/api.d.ts b/convex/_generated/api.d.ts index 73fff4c..1232226 100644 --- a/convex/_generated/api.d.ts +++ b/convex/_generated/api.d.ts @@ -65,7 +65,6 @@ import type * as presenceHttp from "../presenceHttp.js"; import type * as publication from "../publication.js"; import type * as rateLimits from "../rateLimits.js"; import type * as referrals from "../referrals.js"; -import type * as remintDid from "../remintDid.js"; import type * as siteActions from "../siteActions.js"; import type * as siteAssets from "../siteAssets.js"; import type * as siteInternals from "../siteInternals.js"; @@ -142,7 +141,6 @@ declare const fullApi: ApiFromModules<{ publication: typeof publication; rateLimits: typeof rateLimits; referrals: typeof referrals; - remintDid: typeof remintDid; siteActions: typeof siteActions; siteAssets: typeof siteAssets; siteInternals: typeof siteInternals; diff --git a/convex/didCreation.ts b/convex/didCreation.ts index fc48522..78b81df 100644 --- a/convex/didCreation.ts +++ b/convex/didCreation.ts @@ -38,7 +38,9 @@ async function createDIDRecord( domain, signer, verifier: signer, - updateKeys: [verificationMethodId], + // Bare multibase, not the did:key URI — see lib/webvh.ts (didwebvh-ts 2.8 + // compares updateKeys against the parsed keyMultibase by exact equality). + updateKeys: [address], verificationMethods: [ { id: "#key-0", diff --git a/convex/remintDid.ts b/convex/remintDid.ts deleted file mode 100644 index 6378f64..0000000 --- a/convex/remintDid.ts +++ /dev/null @@ -1,109 +0,0 @@ -"use node"; - -/** - * Client-driven re-mint of a user's did:webvh onto the current WEBVH_DOMAIN. - * - * The mint has to happen in the browser: a user DID is signed by an Ed25519 key - * held in that browser's localStorage (see getOrCreateKeyPair in lib/webvh.ts), - * and the server has never held it. The server's job is to authorize the swap - * and rewrite the rows. - * - * AUTHORIZATION — this matters, because applyRemint reassigns every row owned by - * one DID to another, which is an account-takeover primitive if it is callable - * with arbitrary arguments. The app's other mutations trust client-supplied - * DIDs; this one must not. Instead: - * - * 1. The submitted log is RESOLVED, not merely parsed. resolveDIDFromLog - * verifies each entry's Data Integrity proof, so a log cannot be forged - * without the signing key. - * 2. Its update keys must intersect the stored log's update keys for oldDid. - * Re-mint reuses the same localStorage key, so the honest client always - * matches, while an attacker would need the victim's private key. - * 3. The new DID must land on this deployment's WEBVH_DOMAIN, so this can only - * ever move an identity onto the canonical domain. - * - * Any failure throws before a single row is touched. - */ - -import { v } from "convex/values"; -import { action } from "./_generated/server"; -import { internal } from "./_generated/api"; -import { resolveDIDFromLog } from "didwebvh-ts"; -import type { DIDLog } from "didwebvh-ts"; - -function parseLog(jsonl: string): DIDLog { - const entries = jsonl - .trim() - .split("\n") - .filter((line) => line.trim().length > 0) - .map((line) => JSON.parse(line)); - if (entries.length === 0) throw new Error("DID log is empty"); - return entries as DIDLog; -} - -function domainOf(did: string): string | null { - const parts = did.split(":"); - if (parts.length < 5 || parts[1] !== "webvh") return null; - try { - return decodeURIComponent(parts[3]); - } catch { - return parts[3]; - } -} - -export const remintUserDid = action({ - args: { - oldDid: v.string(), - newDidLog: v.string(), - path: v.string(), - }, - handler: async ( - ctx, - args - ): Promise<{ newDid: string; rewritten: number } | { skipped: string }> => { - const targetDomain = process.env.WEBVH_DOMAIN; - if (!targetDomain) throw new Error("WEBVH_DOMAIN is not set on this deployment"); - - // (1) Resolve — this verifies every entry's proof, not just its shape. - const { did: newDid, meta: newMeta } = await resolveDIDFromLog(parseLog(args.newDidLog)); - - if (newDid === args.oldDid) return { skipped: "already on the current DID" }; - - // (3) Only ever move onto this deployment's canonical domain. - const newDomain = domainOf(newDid); - if (newDomain !== targetDomain) { - throw new Error(`Re-mint must target ${targetDomain}, got ${newDomain ?? "unknown"}`); - } - - const user = await ctx.runQuery(internal.migrations.remintUserDidDb.findUserByDid, { - did: args.oldDid, - }); - if (!user) throw new Error("No user holds that DID"); - - // (2) Prove the caller controls the key that controls the OLD DID. - const oldRecord = await ctx.runQuery(internal.migrations.remintUserDidDb.getDidLogFor, { - userDid: args.oldDid, - }); - if (!oldRecord) throw new Error("No stored DID log for the current DID; cannot authorize"); - - const { meta: oldMeta } = await resolveDIDFromLog(parseLog(oldRecord.log)); - const shared = newMeta.updateKeys.filter((k) => oldMeta.updateKeys.includes(k)); - if (shared.length === 0) { - throw new Error("New DID is not controlled by the current DID's update key"); - } - - const { rewritten }: { rewritten: number; skipped: boolean } = await ctx.runMutation( - internal.migrations.remintUserDidDb.applyRemint, - { userId: user._id, oldDid: args.oldDid, newDid } - ); - - await ctx.runMutation(internal.migrations.remintUserDidDb.storeDidLog, { - userDid: newDid, - path: args.path, - log: args.newDidLog, - }); - - console.log(`[remintDid] ${args.oldDid} -> ${newDid} (${rewritten} rows)`); - return { newDid, rewritten }; - }, -}); diff --git a/convex/siteActions.ts b/convex/siteActions.ts index 54e182b..2bb1810 100644 --- a/convex/siteActions.ts +++ b/convex/siteActions.ts @@ -248,13 +248,14 @@ export const createSiteFromUpload = action({ const { privateKey, publicKeyMultibase } = await createSiteKey(); const signer = new SiteWebVHSigner(privateKey, publicKeyMultibase); - const verificationMethodId = signer.getVerificationMethodId(); const didResult = await createDID({ domain: hostname, signer, verifier: signer, - updateKeys: [verificationMethodId], + // Bare multibase, not the did:key URI — see lib/webvh.ts (didwebvh-ts 2.8 + // compares updateKeys against the parsed keyMultibase by exact equality). + updateKeys: [publicKeyMultibase], verificationMethods: [ { id: "#key-0", @@ -337,7 +338,6 @@ export const migrateVerifiedCustomDomain = action({ encryptionSecret ); const signer = new SiteWebVHSigner(privateKey, record.key.publicKeyMultibase); - const verificationMethodId = signer.getVerificationMethodId(); const migratedDid = `did:webvh:${record.site.scid}:${hostname}`; const currentLog = record.didLogEntries.map((entry) => JSON.parse(entry.entryJsonl)); @@ -347,7 +347,9 @@ export const migrateVerifiedCustomDomain = action({ verifier: signer, domain: hostname, controller: migratedDid, - updateKeys: [verificationMethodId], + // Bare multibase, not the did:key URI — see lib/webvh.ts (didwebvh-ts 2.8 + // compares updateKeys against the parsed keyMultibase by exact equality). + updateKeys: [record.key.publicKeyMultibase], verificationMethods: [ { id: "#key-0", diff --git a/scripts/webvh-mint.test.mjs b/scripts/webvh-mint.test.mjs new file mode 100644 index 0000000..b782d5d --- /dev/null +++ b/scripts/webvh-mint.test.mjs @@ -0,0 +1,103 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdir, rm } from "node:fs/promises"; +import { pathToFileURL } from "node:url"; +import { build } from "esbuild"; + +const outdir = "tmp/webvh-mint-test"; + +/** + * Exercises the real mint. didwebvh-ts verifies the log it just created, so a + * malformed `updateKeys` fails here exactly as it does in the browser. + */ +async function loadWebvh() { + await rm(outdir, { recursive: true, force: true }); + await mkdir(outdir, { recursive: true }); + await build({ + entryPoints: ["src/lib/webvh.ts"], + outfile: `${outdir}/webvh.mjs`, + bundle: true, + platform: "node", + format: "esm", + target: "node20", + define: { "import.meta.env.VITE_WEBVH_DOMAIN": '"boop.ad"' }, + plugins: [ + { + name: "stub-capacitor", + setup(b) { + b.onResolve({ filter: /^@capacitor\/core$/ }, () => ({ path: "cap", namespace: "s" })); + b.onLoad({ filter: /.*/, namespace: "s" }, () => ({ + contents: "export const Capacitor={isNativePlatform:()=>false};", + loader: "js", + })); + }, + }, + ], + }); + return import(pathToFileURL(`${process.cwd()}/${outdir}/webvh.mjs`).href); +} + +/** + * Installs browser globals for the duration of `fn`, then restores them. + * defineProperty, not assignment: bun exposes a readonly `localStorage`. + * Restoring matters because bun shares globals across test files. + */ +function withBrowserGlobals(fn) { + const store = new Map(); + const saved = ["localStorage", "window"].map((name) => ({ + name, + descriptor: Object.getOwnPropertyDescriptor(globalThis, name), + })); + + const localStorage = { + getItem: (k) => (store.has(k) ? store.get(k) : null), + setItem: (k, v) => store.set(k, String(v)), + removeItem: (k) => store.delete(k), + }; + const set = (name, value) => + Object.defineProperty(globalThis, name, { value, configurable: true, writable: true }); + + set("localStorage", localStorage); + set("window", { location: { host: "boop.ad" }, localStorage }); + + const restore = () => { + for (const { name, descriptor } of saved) { + if (descriptor) Object.defineProperty(globalThis, name, descriptor); + else delete globalThis[name]; + } + }; + return Promise.resolve(fn()).finally(restore); +} + +const webvh = await loadWebvh(); + +test("createUserWebVHDid mints a verifiable did:webvh", async () => { + await withBrowserGlobals(async () => { + // Regression: updateKeys carried the `did:key:` URI while didwebvh-ts 2.8 + // compares the parsed keyMultibase, so every mint threw + // "Key did:key:… is not authorized to update" — no account could get a DID. + const result = await webvh.createUserWebVHDid({ + email: "someone@example.com", + subOrgId: "20ed9d43-2d31-44f8-9b02-2242c2749a58", + }); + + assert.match(result.did, /^did:webvh:/); + assert.ok(result.did.includes(":boop.ad:"), "must be minted on the current domain"); + assert.equal(result.path, "user-20ed9d43-2d31-44"); + assert.ok(result.didLogJsonl.trim().length > 0, "a log must be produced for the server"); + }); +}); + +test("the minted DID is not stale and yields an openable share URL", async () => { + await withBrowserGlobals(async () => { + const result = await webvh.createUserWebVHDid({ + email: "someone@example.com", + subOrgId: "abc1234567890123456", + }); + assert.equal(webvh.isStaleDidDomain(result.did), false, "a fresh mint must not re-trigger"); + assert.match( + webvh.buildListResourceUrl(result.did, "l1"), + /^https:\/\/boop\.ad\/user-abc1234567890123\/resources\/list-l1$/ + ); + }); +}); diff --git a/src/lib/webvh.ts b/src/lib/webvh.ts index d3d4042..60dffce 100644 --- a/src/lib/webvh.ts +++ b/src/lib/webvh.ts @@ -177,7 +177,11 @@ export async function createUserWebVHDid(params: { domain, signer, verifier: signer, - updateKeys: [signer.getVerificationMethodId()], + // Bare multibase, NOT the did:key URI. didwebvh-ts 2.8's isKeyAuthorized + // parses the proof's verificationMethod down to its keyMultibase and + // compares by exact equality, so a `did:key:z6Mk…` entry never matches and + // every mint fails with "Key … is not authorized to update". + updateKeys: [publicKeyMultibase], verificationMethods: [ { id: "#key-0",