From 524ca8a1c8c02393a6e53dd8e189055adfc6b133 Mon Sep 17 00:00:00 2001 From: Brian Richter Date: Fri, 31 Jul 2026 23:28:23 -0700 Subject: [PATCH 1/2] fix(did): authorize DID log writes; anyone could overwrite anyone's did.jsonl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A did:webvh log is what the world resolves to learn a user's keys, and the write path had two independent doors open: 1. POST /api/did/log checked only that the Authorization header *started with* "Bearer ". The token was never verified and `userDid` came from the request body, unbound to the caller. `Authorization: Bearer x` passed. 2. `api.didLogs.upsertDidLog` was a public mutation, so a caller could skip the HTTP handler entirely. VITE_CONVEX_URL ships in the browser bundle. Either one let anyone overwrite or blank any user's served DID log. The SCID is self-certifying, so a verifying resolver rejects a forged log rather than accepting a fake identity — the exposure is defacement and DoS of resolution for every honest consumer, with an attacker-controlled path→log mapping. Present since 00e4234 (2026-02-19), not a regression from the Turnkey work. The binding needs no new state: the client derives its serving path as `user-` (toUserSlug) and mints did:webvh:::, so both are recomputable from the JWT's `sub` alone. assertDidLogOwnership recomputes them and rejects a mismatch — the body is checked, never trusted. It matches the DID's trailing segment as `:` rather than parsing by index, so a dev domain carrying a port (localhost%3A5173) still binds and `evil` still doesn't. Also pins iss/aud/exp in verifyAuthToken. signAuthToken has set issuer `originals-auth` and audience `originals-api` since 2026-04-04 and it is the only minter, so every live token (30d max) already carries them — no one gets logged out. Without the pin, any other HS256 token sharing JWT_SECRET, or one with no expiry at all, passed as a session. Reads are untouched: GET /api/did/log and getDidLogByPath stay public, and existing rows keep resolving. The one client caller (useAuth.tsx:195) already sends the token, so no client change is needed. didLogAuth duplicates toUserSlug because Convex modules cannot import from src/; the test asserts the two stay in agreement. Pre-existing typecheck failures in didCreation.ts, lib/turnkeySigner.ts and siteActions.ts (didwebvh-ts/noble type drift) are untouched and unrelated. Co-Authored-By: Claude Opus 5 (1M context) --- convex/_generated/api.d.ts | 2 + convex/didLogs.ts | 8 +- convex/didLogsHttp.ts | 35 ++++-- convex/lib/didLogAuth.ts | 61 +++++++++ convex/lib/jwt.ts | 7 +- scripts/did-log-auth.test.mjs | 228 ++++++++++++++++++++++++++++++++++ 6 files changed, 327 insertions(+), 14 deletions(-) create mode 100644 convex/lib/didLogAuth.ts create mode 100644 scripts/did-log-auth.test.mjs diff --git a/convex/_generated/api.d.ts b/convex/_generated/api.d.ts index 5b9ddbe..e7888d9 100644 --- a/convex/_generated/api.d.ts +++ b/convex/_generated/api.d.ts @@ -45,6 +45,7 @@ import type * as lib_apiKeyHelpers from "../lib/apiKeyHelpers.js"; import type * as lib_auth from "../lib/auth.js"; import type * as lib_authUser from "../lib/authUser.js"; import type * as lib_bucket from "../lib/bucket.js"; +import type * as lib_didLogAuth from "../lib/didLogAuth.js"; import type * as lib_httpResponses from "../lib/httpResponses.js"; import type * as lib_itemCategories from "../lib/itemCategories.js"; import type * as lib_jwt from "../lib/jwt.js"; @@ -123,6 +124,7 @@ declare const fullApi: ApiFromModules<{ "lib/auth": typeof lib_auth; "lib/authUser": typeof lib_authUser; "lib/bucket": typeof lib_bucket; + "lib/didLogAuth": typeof lib_didLogAuth; "lib/httpResponses": typeof lib_httpResponses; "lib/itemCategories": typeof lib_itemCategories; "lib/jwt": typeof lib_jwt; diff --git a/convex/didLogs.ts b/convex/didLogs.ts index 8f796ec..113ae66 100644 --- a/convex/didLogs.ts +++ b/convex/didLogs.ts @@ -6,12 +6,16 @@ */ import { v } from "convex/values"; -import { mutation, query } from "./_generated/server"; +import { internalMutation, query } from "./_generated/server"; /** * Store or update a user's DID log. + * + * Internal: it writes whatever `userDid` it is handed, so the caller owns the + * ownership check. As a public mutation this was directly callable by anyone + * with the deployment URL. Go through didLogsHttp.storeDidLog. */ -export const upsertDidLog = mutation({ +export const upsertDidLog = internalMutation({ args: { userDid: v.string(), path: v.string(), diff --git a/convex/didLogsHttp.ts b/convex/didLogsHttp.ts index b6b827b..91a63f3 100644 --- a/convex/didLogsHttp.ts +++ b/convex/didLogsHttp.ts @@ -6,7 +6,9 @@ */ import { httpAction } from "./_generated/server"; -import { api } from "./_generated/api"; +import { api, internal } from "./_generated/api"; +import { requireAuth, AuthError } from "./lib/auth"; +import { assertDidLogOwnership, DidLogOwnershipError } from "./lib/didLogAuth"; function getCorsHeaders(request: Request): Record { const origin = request.headers.get("Origin") || "*"; @@ -19,20 +21,17 @@ function getCorsHeaders(request: Request): Record { } /** - * Store/update a DID log. Requires JWT auth. + * Store/update a DID log for the authenticated caller. + * + * The body's `userDid`/`path` are checked against the ones derivable from the + * caller's token, never trusted — this endpoint decides what the world resolves + * for an identity. */ export const storeDidLog = httpAction(async (ctx, request) => { const corsHeaders = getCorsHeaders(request); try { - // Verify auth via Authorization header - const authHeader = request.headers.get("Authorization"); - if (!authHeader?.startsWith("Bearer ")) { - return new Response(JSON.stringify({ error: "Unauthorized" }), { - status: 401, - headers: { "Content-Type": "application/json", ...corsHeaders }, - }); - } + const auth = await requireAuth(request); const body = await request.json(); const { userDid, path, log } = body as { userDid: string; path: string; log: string }; @@ -44,13 +43,27 @@ export const storeDidLog = httpAction(async (ctx, request) => { }); } - await ctx.runMutation(api.didLogs.upsertDidLog, { userDid, path, log }); + assertDidLogOwnership({ subOrgId: auth.turnkeySubOrgId, userDid, path }); + + await ctx.runMutation(internal.didLogs.upsertDidLog, { userDid, path, log }); return new Response(JSON.stringify({ ok: true }), { status: 200, headers: { "Content-Type": "application/json", ...corsHeaders }, }); } catch (error) { + if (error instanceof AuthError) { + return new Response(JSON.stringify({ error: error.message }), { + status: 401, + headers: { "Content-Type": "application/json", ...corsHeaders }, + }); + } + if (error instanceof DidLogOwnershipError) { + return new Response(JSON.stringify({ error: error.message }), { + status: 403, + headers: { "Content-Type": "application/json", ...corsHeaders }, + }); + } console.error("[didLogsHttp] Store error:", error); return new Response(JSON.stringify({ error: "Failed to store DID log" }), { status: 500, diff --git a/convex/lib/didLogAuth.ts b/convex/lib/didLogAuth.ts new file mode 100644 index 0000000..71dc18f --- /dev/null +++ b/convex/lib/didLogAuth.ts @@ -0,0 +1,61 @@ +/** + * Binds a DID log write to the caller's own identity. + * + * A did:webvh log is what the world resolves to learn a user's keys, so the + * write path must not take the target identity on trust. It doesn't have to: + * the client derives its serving path as `user-` + * (toUserSlug in src/lib/webvh.ts) and mints did:webvh:::, + * so both are recomputable from the JWT alone. + * + * Duplicated rather than imported because Convex modules cannot reach into + * src/ — keep in step with toUserSlug; the test asserts they agree. + */ + +export class DidLogOwnershipError extends Error { + constructor(message: string) { + super(message); + this.name = "DidLogOwnershipError"; + } +} + +/** The only path this sub-org may serve a DID log at. */ +export function didLogPathForSubOrg(subOrgId: string): string { + return `user-${subOrgId.slice(0, 16)}`; +} + +/** + * Throws unless `userDid` and `path` are the ones this sub-org owns. + * + * Matches the DID's trailing path as a whole segment (`:`) rather than + * parsing by index, so a dev domain carrying a port (`localhost%3A5173`) still + * binds and `evil` still doesn't. + */ +export function assertDidLogOwnership(params: { + subOrgId: string; + userDid: string; + path: string; +}): void { + const { subOrgId, userDid, path } = params; + + if (!subOrgId) { + throw new DidLogOwnershipError("Authenticated caller has no sub-organization ID"); + } + + const expectedPath = didLogPathForSubOrg(subOrgId); + + if (path !== expectedPath) { + throw new DidLogOwnershipError( + `Path "${path}" is not this account's DID log path` + ); + } + + if (!userDid.startsWith("did:webvh:")) { + throw new DidLogOwnershipError(`Expected a did:webvh, got "${userDid}"`); + } + + if (!userDid.endsWith(`:${expectedPath}`)) { + throw new DidLogOwnershipError( + `DID "${userDid}" does not belong to this account` + ); + } +} diff --git a/convex/lib/jwt.ts b/convex/lib/jwt.ts index 17e1e1c..56f95f6 100644 --- a/convex/lib/jwt.ts +++ b/convex/lib/jwt.ts @@ -50,9 +50,14 @@ export async function verifyAuthToken(token: string): Promise // Encode secret as Uint8Array for jose const secret = new TextEncoder().encode(jwtSecret); - // Verify the token + // Pin issuer/audience/exp to what signAuthToken mints (authInternal.ts). + // Without them any other HS256 token sharing JWT_SECRET — different + // service, different audience, or no expiry at all — passes as a session. const { payload } = await jose.jwtVerify(token, secret, { algorithms: ["HS256"], + issuer: "originals-auth", + audience: "originals-api", + requiredClaims: ["exp"], }); const jwtPayload = payload as unknown as JWTPayload; diff --git a/scripts/did-log-auth.test.mjs b/scripts/did-log-auth.test.mjs new file mode 100644 index 0000000..9d9b46b --- /dev/null +++ b/scripts/did-log-auth.test.mjs @@ -0,0 +1,228 @@ +/** + * DID log write authorization. + * + * Two independent doors used to be open: POST /api/did/log accepted any + * `Authorization: Bearer ` without verifying the token, and + * `api.didLogs.upsertDidLog` was a public mutation callable directly. Either + * let anyone overwrite anyone's served did.jsonl, keyed on a userDid they + * supplied themselves. + * + * These tests cover the two pure seams that close it: the token verifier's + * claim pinning, and the binding of a write to the caller's own sub-org. + */ + +import assert from "node:assert/strict"; +import { mkdir, rm } from "node:fs/promises"; +import { pathToFileURL } from "node:url"; +import { build } from "esbuild"; +import * as jose from "jose"; + +const outdir = "tmp/did-log-auth-test"; + +async function bundle(entry, name) { + await build({ + entryPoints: [entry], + outfile: `${outdir}/${name}.mjs`, + bundle: true, + platform: "node", + format: "esm", + target: "node20", + external: ["jose"], + }); + return import(pathToFileURL(`${process.cwd()}/${outdir}/${name}.mjs`).href); +} + +await rm(outdir, { recursive: true, force: true }); +await mkdir(outdir, { recursive: true }); + +const JWT_SECRET = "test-secret-at-least-32-characters-long!!"; +process.env.JWT_SECRET = JWT_SECRET; +const secret = new TextEncoder().encode(JWT_SECRET); + +const jwt = await bundle("convex/lib/jwt.ts", "jwt"); +const didLogAuth = await bundle("convex/lib/didLogAuth.ts", "didLogAuth"); + +const SUB_ORG = "abcdef0123456789fedcba9876543210"; +const SLUG = "user-abcdef0123456789"; + +/** A token exactly as convex/authInternal.ts mints one. */ +function mintToken(overrides = {}) { + const { + issuer = "originals-auth", + audience = "originals-api", + expiry = "30d", + sub = SUB_ORG, + email = "user@example.com", + signingSecret = secret, + } = overrides; + + let builder = new jose.SignJWT({ sub, email }) + .setProtectedHeader({ alg: "HS256" }) + .setIssuedAt(); + if (expiry !== null) builder = builder.setExpirationTime(expiry); + if (issuer !== null) builder = builder.setIssuer(issuer); + if (audience !== null) builder = builder.setAudience(audience); + return builder.sign(signingSecret); +} + +async function rejects(promise, description) { + await assert.rejects(promise, description); +} + +// --- verifyAuthToken: a genuine token still works ------------------------- + +{ + const payload = await jwt.verifyAuthToken(await mintToken()); + assert.equal(payload.turnkeySubOrgId, SUB_ORG); + assert.equal(payload.email, "user@example.com"); +} + +// --- verifyAuthToken: claim pinning --------------------------------------- + +// The minter sets iss/aud; a token from any other service signed with the same +// JWT_SECRET must not be accepted as a boop session. +{ + await rejects( + jwt.verifyAuthToken(await mintToken({ issuer: "some-other-service" })), + /"iss"/ + ); + await rejects(jwt.verifyAuthToken(await mintToken({ issuer: null })), /"iss"/); +} + +{ + await rejects( + jwt.verifyAuthToken(await mintToken({ audience: "some-other-api" })), + /"aud"/ + ); + await rejects(jwt.verifyAuthToken(await mintToken({ audience: null })), /"aud"/); +} + +// An unexpiring session token is not a session. +{ + await rejects(jwt.verifyAuthToken(await mintToken({ expiry: null })), /"exp"/); +} + +{ + await rejects(jwt.verifyAuthToken(await mintToken({ expiry: "-1h" })), /expired/i); +} + +// Signature still has to hold. +{ + const wrongSecret = new TextEncoder().encode("a-completely-different-secret-key-32!!"); + await rejects(jwt.verifyAuthToken(await mintToken({ signingSecret: wrongSecret })), /.*/); +} + +// alg:none must never be accepted. +{ + const unsecured = new jose.UnsecuredJWT({ sub: SUB_ORG, email: "user@example.com" }) + .setIssuedAt() + .setIssuer("originals-auth") + .setAudience("originals-api") + .setExpirationTime("30d") + .encode(); + await rejects(jwt.verifyAuthToken(unsecured), /.*/); +} + +{ + await rejects(jwt.verifyAuthToken(""), /required/i); + await rejects(jwt.verifyAuthToken("not-a-jwt"), /.*/); +} + +// --- didLogPathForSubOrg mirrors the client's toUserSlug ------------------ + +// src/lib/webvh.ts toUserSlug: `user-${subOrgId.slice(0, 16)}`. If these ever +// disagree, every honest write starts failing. +{ + assert.equal(didLogAuth.didLogPathForSubOrg(SUB_ORG), SLUG); + assert.equal(didLogAuth.didLogPathForSubOrg("short"), "user-short"); +} + +// --- assertDidLogOwnership: the honest client passes ---------------------- + +{ + didLogAuth.assertDidLogOwnership({ + subOrgId: SUB_ORG, + userDid: `did:webvh:QmScid123:boop.ad:${SLUG}`, + path: SLUG, + }); +} + +// A dev domain carrying a port must not break the binding. +{ + didLogAuth.assertDidLogOwnership({ + subOrgId: SUB_ORG, + userDid: `did:webvh:QmScid123:localhost%3A5173:${SLUG}`, + path: SLUG, + }); +} + +// --- assertDidLogOwnership: the attacks ---------------------------------- + +// Squatting another user's serving path. +{ + assert.throws( + () => + didLogAuth.assertDidLogOwnership({ + subOrgId: SUB_ORG, + userDid: `did:webvh:QmScid123:boop.ad:${SLUG}`, + path: "user-victimsuborg1234", + }), + /path/i + ); +} + +// Authenticating as yourself but writing a log under the victim's DID. +{ + assert.throws( + () => + didLogAuth.assertDidLogOwnership({ + subOrgId: SUB_ORG, + userDid: "did:webvh:QmScid123:boop.ad:user-victimsuborg1234", + path: SLUG, + }), + /did/i + ); +} + +// A DID whose path merely ends with the slug as a substring, not a segment. +{ + assert.throws( + () => + didLogAuth.assertDidLogOwnership({ + subOrgId: SUB_ORG, + userDid: `did:webvh:QmScid123:boop.ad:evil${SLUG}`, + path: SLUG, + }), + /did/i + ); +} + +// Only did:webvh logs belong here. +{ + assert.throws( + () => + didLogAuth.assertDidLogOwnership({ + subOrgId: SUB_ORG, + userDid: `did:key:z6MkTest:${SLUG}`, + path: SLUG, + }), + /did:webvh/i + ); +} + +// A caller with no sub-org can never derive a path. +{ + assert.throws( + () => + didLogAuth.assertDidLogOwnership({ + subOrgId: "", + userDid: `did:webvh:QmScid123:boop.ad:${SLUG}`, + path: SLUG, + }), + /sub-organization/i + ); +} + +await rm(outdir, { recursive: true, force: true }); + +console.log("did-log-auth: all assertions passed"); From 05667509c40b5e88775644a7b03e7e335cac92ee Mon Sep 17 00:00:00 2001 From: Brian Richter Date: Sun, 16 Aug 2026 00:36:18 -0700 Subject: [PATCH 2/2] fix(did): bind the re-mint log write to the caller too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #217 added a second way into the didLogs table. /api/user/remintDid runs requireAuth(), so the account it rewrites is taken from the token — but the `path` it hands to remintUserDidDb.storeDidLog comes straight from the body, and that mutation patches whichever row matches `path`. So an authenticated user could post their own freshly minted log with `path: "user-"` and take over what the world resolves for that account. Weaker than the unauthenticated hole in the previous commit — it costs an account — but the same defacement primitive, and it landed after that fix was written. assertDidLogOwnership runs before applyRemint rather than next to the write: rejecting afterwards would leave every row already moved to the new DID with the log write refused. The test now also scans convex/ and requires any file running a didLogs-writing mutation to import the check. The helper's unit tests were all passing while this door stood open — a new call site is exactly the regression they cannot see. Verified non-vacuous: it matches userHttp.ts and didLogsHttp.ts today. Also corrects a stale reference in jwt.ts — the minter is signJwtToken. Co-Authored-By: Claude Opus 5 (1M context) --- convex/lib/jwt.ts | 2 +- convex/userHttp.ts | 11 +++++++++++ scripts/did-log-auth.test.mjs | 24 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/convex/lib/jwt.ts b/convex/lib/jwt.ts index 56f95f6..26a40eb 100644 --- a/convex/lib/jwt.ts +++ b/convex/lib/jwt.ts @@ -50,7 +50,7 @@ export async function verifyAuthToken(token: string): Promise // Encode secret as Uint8Array for jose const secret = new TextEncoder().encode(jwtSecret); - // Pin issuer/audience/exp to what signAuthToken mints (authInternal.ts). + // Pin issuer/audience/exp to what signJwtToken mints (authInternal.ts). // Without them any other HS256 token sharing JWT_SECRET — different // service, different audience, or no expiry at all — passes as a session. const { payload } = await jose.jwtVerify(token, secret, { diff --git a/convex/userHttp.ts b/convex/userHttp.ts index 97503f0..33c5acf 100644 --- a/convex/userHttp.ts +++ b/convex/userHttp.ts @@ -12,6 +12,7 @@ import { unauthorizedResponseWithCors, } from "./lib/auth"; import { jsonResponse, errorResponse } from "./lib/httpResponses"; +import { assertDidLogOwnership, DidLogOwnershipError } from "./lib/didLogAuth"; /** The domain encoded in a did:webvh, percent-decoded. Null if not a did:webvh. */ function didWebvhDomain(did: string): string | null { @@ -139,6 +140,13 @@ export const remintUserDID = httpAction(async (ctx, request) => { }); } + // Checked before applyRemint: storeDidLog patches the didLogs row matching + // `path`, so an unchecked body could point any other account's serving path + // at this caller's log. Rejecting afterwards would leave rows already moved. + if (didLog && path) { + assertDidLogOwnership({ subOrgId: auth.turnkeySubOrgId, userDid: newDid, path }); + } + const { rewritten } = await ctx.runMutation( internal.migrations.remintUserDidDb.applyRemint, { userId: user._id, oldDid: user.did, newDid } @@ -160,6 +168,9 @@ export const remintUserDID = httpAction(async (ctx, request) => { if (err instanceof AuthError) { return unauthorizedResponseWithCors(request, err.message); } + if (err instanceof DidLogOwnershipError) { + return errorResponse(request, err.message, 403); + } console.error("[userHttp] Re-mint error:", err); return errorResponse( request, diff --git a/scripts/did-log-auth.test.mjs b/scripts/did-log-auth.test.mjs index 9d9b46b..f3aa31e 100644 --- a/scripts/did-log-auth.test.mjs +++ b/scripts/did-log-auth.test.mjs @@ -223,6 +223,30 @@ async function rejects(promise, description) { ); } +// --- every didLogs write path is behind the ownership check --------------- + +// The helper being correct proves nothing if a new endpoint writes the table +// without calling it — which is exactly how the re-mint path (#217) opened a +// second door. Any file that runs a didLogs-writing mutation must import the +// check; a bare unit test would not have caught this. +{ + const { readdir, readFile } = await import("node:fs/promises"); + const files = (await readdir("convex", { recursive: true })).filter( + (f) => f.endsWith(".ts") && !f.startsWith("_generated") + ); + + const writers = /runMutation\(\s*internal\.[\w.]*(?:upsertDidLog|storeDidLog)/; + + for (const file of files) { + const src = await readFile(`convex/${file}`, "utf8"); + if (!writers.test(src)) continue; + assert.ok( + src.includes("assertDidLogOwnership"), + `convex/${file} writes a didLogs row without asserting ownership` + ); + } +} + await rm(outdir, { recursive: true, force: true }); console.log("did-log-auth: all assertions passed");