From 1560ff34b74b5a1e5011c4e83f04b95e5fd63b0a Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 12:07:04 +0200 Subject: [PATCH 01/12] feat(world-id): defensively claim unmigrated apps' on-chain rp_ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Salting new rp_ids cannot protect the installed base: every app created so far has a public app_id, so its uint64(keccak256(app_id)) rp_id is predictable forever, and on-chain register() is permissionless, zero-fee and first-come with no reclaim path (H1 #3910854). The only defense left for those apps is to hold the id ourselves until the app is ready to use it. POST /api/_pre-register-rp-ids claims the ids of a caller-supplied list of apps, registering each to the Portal's shared manager key with a placeholder signer. It creates no rp_registration rows — on-chain state is the record of what we hold, and inventing rows would make the Portal claim apps are registered when their owners never asked, which is what proof-context serves from. Because every claim spends L2 gas and the registry's WLD fee, the safeguards are the point: a kill switch that makes the endpoint inert, dry-run by default so spending requires asking for it, a hard 25-app ceiling per call, and a per-outcome breakdown in the response and the logs so a run that skipped everything for an unexpected reason cannot read as a successful sweep. Claims never happen on a failed on-chain read, and re-running is idempotent. Adoption is what keeps a claimed app onboardable. submitManagedRpRegistration already refused an rp_id that was taken on-chain; it now distinguishes a squatter from our own claim by the manager address — the one role only our KMS key can sign for — and rotates the placeholder signer to the developer's real one instead of submitting a register() the contract would reject. Adoption is pinned to the shared key regardless of ENABLE_SHARED_KEY_RP_REGISTRATION, because a dedicated key would be a manager the contract has never seen and every later update would revert. Self-managed apps cannot adopt yet: the developer registers from their own wallet, so a claimed id means their register() reverts. register_rp now fails that case loudly with rp_id_taken instead of inserting a row that polls pending forever. Transferring the manager to the developer is the missing piece (submitTransferManagerTransaction exists, nothing drives it), so ids should not be claimed for apps expected to self-manage until that lands. Co-Authored-By: Claude --- web/.env.example | 10 + web/api/_pre-register-rp-ids/index.ts | 301 ++++++++++++++++++ web/api/hasura/register-rp/index.ts | 44 +++ web/api/helpers/rp-registration-flows.ts | 113 ++++++- web/api/helpers/rp-utils.ts | 7 +- .../api/helpers/rp-registration-flows.test.ts | 49 ++- web/tests/api/pre-register-rp-ids.test.ts | 268 ++++++++++++++++ 7 files changed, 780 insertions(+), 12 deletions(-) create mode 100644 web/api/_pre-register-rp-ids/index.ts create mode 100644 web/tests/api/pre-register-rp-ids.test.ts diff --git a/web/.env.example b/web/.env.example index 626173d12..f53feac86 100644 --- a/web/.env.example +++ b/web/.env.example @@ -90,6 +90,16 @@ RP_REGISTRY_SAFE_4337_MODULE_ADDRESS=0x75cf11467937ce3F2f357CE24ffc3DBF8fD5c226 RP_REGISTRY_KMS_REGION=eu-west-1 RP_REGISTRY_UPDATE_RP_TYPEHASH=0xe52c0a4424a7d2015a13748c3738793d6b1dd1774e4ee82516251372da305465 RP_REGISTRY_DOMAIN_SEPARATOR=0x150a3a11bbb5ea6c4cfbd7cedc050d67bb55333eb8700bec241368de512607f1 +# Kill switch for POST /api/_pre-register-rp-ids, which defensively claims the +# on-chain rp_id of apps that haven't migrated yet so nobody can squat it. Every +# claim spends L2 gas and the registry's WLD fee, so leave this unset unless a +# run is actually intended. The endpoint dry-runs unless asked not to. +ENABLE_RP_ID_PRE_REGISTRATION= +# Placeholder signer recorded on a defensively claimed rp_id. Must be a valid +# non-zero address that CANNOT sign — nothing should be able to verify against a +# claimed-but-unadopted RP. Replaced with the developer's real signer when the +# app registers for real. +RP_ID_PRE_REGISTRATION_SIGNER= # Verifier Contract VERIFIER_CONTRACT_ADDRESS=0x0000000000000000000000000000000000000000 diff --git a/web/api/_pre-register-rp-ids/index.ts b/web/api/_pre-register-rp-ids/index.ts new file mode 100644 index 000000000..860da4869 --- /dev/null +++ b/web/api/_pre-register-rp-ids/index.ts @@ -0,0 +1,301 @@ +import { getSdk as getAppInfoSdk } from "@/api/hasura/register-rp/graphql/get-app-info.generated"; +import { getSdk as getFetchRpSdk } from "@/api/helpers/graphql/fetch-rp-registration.generated"; +import { getAPIServiceGraphqlClient } from "@/api/helpers/graphql"; +import { getKMSClient } from "@/api/helpers/kms"; +import { resolveManagerAddress } from "@/api/helpers/rp-manager"; +import { submitRegisterRpTransaction } from "@/api/helpers/rp-transactions"; +import { + generateRpIdString, + getRpRegistryConfig, + isZeroAddress, + parseRpId, +} from "@/api/helpers/rp-utils"; +import { getRpFromContract } from "@/api/helpers/temporal-rpc"; +import { protectInternalEndpoint } from "@/api/helpers/utils"; +import { validateRequestSchema } from "@/api/helpers/validate-request-schema"; +import { logger } from "@/lib/logger"; +import { isAddress } from "ethers"; +import { NextRequest, NextResponse } from "next/server"; +import * as yup from "yup"; + +/** + * Hard ceiling on how many apps one call may claim on-chain. Each claim is a + * UserOp that costs L2 gas and pulls the registry's WLD fee from our Safe, so + * an accidental "pre-register everything" must not be one request away. Drain a + * larger backlog across repeated calls. + */ +const MAX_APPS_PER_CALL = 25; + +const schema = yup + .object({ + app_ids: yup + .array() + .of(yup.string().strict().required()) + .min(1) + .max(MAX_APPS_PER_CALL) + .required(), + /** + * Defaults to true: the only way to spend gas is to ask for it explicitly. + * A dry run reports exactly what a real run would submit. + */ + dry_run: yup.boolean().default(true), + }) + .noUnknown(); + +type Outcome = + | "would_claim" + | "claimed" + | "skipped_already_registered_in_portal" + | "skipped_already_claimed_by_us" + | "skipped_taken_by_foreign_manager" + | "skipped_staging" + | "skipped_app_not_found" + | "failed_rpc" + | "failed_submission"; + +/** + * Defensively claim the on-chain rp_id of apps that have not migrated to World + * ID 4.0 yet, so nobody else can. + * + * For every app created before unpredictable rp_ids, `rp_id` is + * `uint64(keccak256(app_id))` over a *public* app_id, and on-chain `register()` + * is permissionless, zero-fee and first-come. Those ids are public and + * predictable forever, so salting new registrations cannot protect the existing + * installed base — the only defense left is to hold the id ourselves until the + * app is ready to use it (H1 #3910854). + * + * A claim registers the rp_id to the Portal's SHARED manager key with a + * placeholder signer. Two consequences worth being explicit about: + * + * - No `rp_registration` row is created. On-chain state is the record of what + * we hold, and `submitManagedRpRegistration` reads it to decide whether to + * adopt. Inventing rows would make the Portal claim apps are registered when + * their owners never asked, and `proof-context` serves off that row. + * - The signer is a placeholder that must never be able to sign. Nothing can + * verify against a claimed-but-unadopted rp_id, which is the intent. + * + * Adoption: when the app later registers in managed mode, + * `submitManagedRpRegistration` sees our own manager on-chain and rotates the + * signer instead of registering. SELF-MANAGED apps cannot adopt yet — the + * developer would need the manager transferred to them + * (`submitTransferManagerTransaction` exists but no flow drives it), so + * `register_rp` fails them loudly with an actionable error rather than leaving + * a row that polls forever. Do not claim ids for apps expected to self-manage + * until that flow lands. + */ +export async function POST(request: NextRequest) { + const { isAuthenticated, errorResponse } = protectInternalEndpoint(request); + if (!isAuthenticated) { + return errorResponse; + } + + // Kill switch. Off means this endpoint cannot spend anything, whatever it is + // called with. + if (process.env.ENABLE_RP_ID_PRE_REGISTRATION !== "true") { + logger.warn("RP id pre-registration is disabled"); + return NextResponse.json( + { error: "RP id pre-registration is disabled." }, + { status: 503 }, + ); + } + + const body = await request.json().catch(() => null); + const { isValid, parsedParams } = await validateRequestSchema({ + value: body, + schema, + }); + if (!isValid || !parsedParams) { + return NextResponse.json( + { + error: `Invalid request body. Expected { app_ids: string[] (1..${MAX_APPS_PER_CALL}), dry_run?: boolean }.`, + }, + { status: 400 }, + ); + } + + const { app_ids: appIds, dry_run: dryRun } = parsedParams; + + const config = getRpRegistryConfig(); + if (!config) { + logger.error("RP Registry is not configured for pre-registration"); + return NextResponse.json( + { error: "RP Registry is not configured." }, + { status: 500 }, + ); + } + + // Deliberately the shared key, not a dedicated per-RP key: a claim is not a + // registration, and minting a KMS key per unmigrated app would be thousands + // of keys we may never adopt. It is also what makes adoption decidable — + // `submitManagedRpRegistration` compares the on-chain manager against this + // one address. + const managerKmsKeyId = process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; + if (!managerKmsKeyId) { + logger.error( + "RP_REGISTRY_MANAGER_KMS_KEY_ID is required for pre-registration", + ); + return NextResponse.json( + { error: "Shared manager key is not configured." }, + { status: 500 }, + ); + } + + const placeholderSigner = process.env.RP_ID_PRE_REGISTRATION_SIGNER; + if ( + !placeholderSigner || + !isAddress(placeholderSigner) || + isZeroAddress(placeholderSigner) + ) { + logger.error( + "RP_ID_PRE_REGISTRATION_SIGNER must be a valid non-zero address", + ); + return NextResponse.json( + { error: "Placeholder signer is not configured." }, + { status: 500 }, + ); + } + + const managerAddress = await resolveManagerAddress( + managerKmsKeyId, + config.kmsRegion, + ); + if (!managerAddress) { + // Without our own manager address we cannot tell "already ours" from + // "someone else's", and claiming blindly could overwrite nothing but would + // report nonsense. Fail the run instead. + logger.error("Could not resolve the shared manager address from KMS"); + return NextResponse.json( + { error: "Could not resolve the manager address." }, + { status: 503 }, + ); + } + + const client = await getAPIServiceGraphqlClient(); + const results: { app_id: string; outcome: Outcome; rp_id?: string }[] = []; + + for (const appId of appIds) { + const rpIdString = generateRpIdString(appId); + const rpId = parseRpId(rpIdString); + + const { app } = await getAppInfoSdk(client).GetAppInfo({ app_id: appId }); + const appInfo = app?.[0]; + if (!appInfo) { + results.push({ app_id: appId, outcome: "skipped_app_not_found" }); + continue; + } + if (appInfo.is_staging) { + // Staging apps never migrate to World ID 4.0, so there is nothing to + // protect and their rp_ids are not worth spending gas on. + results.push({ app_id: appId, outcome: "skipped_staging" }); + continue; + } + + // An app that already has a row owns its rp_id through the normal flow; + // claiming on top of it would at best waste a submission and at worst + // interfere with an in-flight registration. + const { rp_registration } = await getFetchRpSdk(client).FetchRpRegistration( + { + app_id: appId, + }, + ); + if (rp_registration?.length) { + results.push({ + app_id: appId, + outcome: "skipped_already_registered_in_portal", + rp_id: rpIdString, + }); + continue; + } + + let onChain; + try { + onChain = await getRpFromContract(rpId, config.contractAddress); + } catch (error) { + // Never claim on a failed read: `register()` reverts with IdAlreadyInUse + // if the id is taken, so a blind submission burns gas and, worse, a read + // failure is indistinguishable from "free" here. + logger.warn("Could not read on-chain RP state; skipping", { + error, + app_id: appId, + rpIdString, + }); + results.push({ app_id: appId, outcome: "failed_rpc", rp_id: rpIdString }); + continue; + } + + if (onChain.initialized) { + const isOurs = + onChain.manager?.toLowerCase() === managerAddress.toLowerCase(); + if (!isOurs) { + // Already squatted. Nothing this endpoint can do — the contract has no + // reclaim path — but it is the single most important thing to surface. + logger.error("rp_id is already held by a foreign manager", { + app_id: appId, + rpIdString, + onChainManager: onChain.manager, + onChainSigner: onChain.signer, + }); + } + results.push({ + app_id: appId, + outcome: isOurs + ? "skipped_already_claimed_by_us" + : "skipped_taken_by_foreign_manager", + rp_id: rpIdString, + }); + continue; + } + + if (dryRun) { + results.push({ + app_id: appId, + outcome: "would_claim", + rp_id: rpIdString, + }); + continue; + } + + try { + const kmsClient = await getKMSClient(config.kmsRegion); + const operationHash = await submitRegisterRpTransaction(config, { + rpId, + managerAddress, + signerAddress: placeholderSigner, + appName: appInfo.app_metadata?.[0]?.name || "", + kmsClient, + }); + logger.info("Claimed rp_id defensively", { + app_id: appId, + rpIdString, + operationHash, + }); + results.push({ app_id: appId, outcome: "claimed", rp_id: rpIdString }); + } catch (error) { + logger.error("Failed to claim rp_id", { + error, + app_id: appId, + rpIdString, + }); + results.push({ + app_id: appId, + outcome: "failed_submission", + rp_id: rpIdString, + }); + } + } + + const counts = results.reduce>((acc, r) => { + acc[r.outcome] = (acc[r.outcome] ?? 0) + 1; + return acc; + }, {}); + + // Log the breakdown rather than only the total: a run that skipped everything + // for an unexpected reason must not read as a successful sweep. + logger.info("RP id pre-registration run finished", { + dry_run: dryRun, + requested: appIds.length, + counts, + }); + + return NextResponse.json({ dry_run: dryRun, counts, results }); +} diff --git a/web/api/hasura/register-rp/index.ts b/web/api/hasura/register-rp/index.ts index 1fbd9be58..eeb8bb2d3 100644 --- a/web/api/hasura/register-rp/index.ts +++ b/web/api/hasura/register-rp/index.ts @@ -7,10 +7,13 @@ import { } from "@/api/helpers/rp-registration-flows"; import { generateRpIdString, + getRpRegistryConfig, isZeroAddress, normalizeAddress, + parseRpId, RpRegistrationStatus, } from "@/api/helpers/rp-utils"; +import { getRpFromContract } from "@/api/helpers/temporal-rpc"; import { protectInternalEndpoint } from "@/api/helpers/utils"; import { validateRequestSchema } from "@/api/helpers/validate-request-schema"; import { logger } from "@/lib/logger"; @@ -153,6 +156,47 @@ export const POST = async (req: NextRequest) => { // Self-managed: just create the DB record. No KMS / on-chain work. if (mode === "self_managed") { const rpIdString = generateRpIdString(app_id); + + // A self-managed developer registers the rp_id from their own wallet, so if + // the id is already claimed on-chain their `register()` has already reverted + // (or is about to). Inserting the row anyway would leave the dashboard + // polling `pending` forever with no explanation. This covers both a + // squatter and an id the Portal claimed defensively via + // _pre-register-rp-ids — the latter needs the manager transferred to them, + // which no flow drives yet, so both cases route to support. + // + // Best-effort: a read failure must not block onboarding for the common case + // where the id is free. + const primaryConfig = getRpRegistryConfig(); + if (primaryConfig) { + try { + const onChain = await getRpFromContract( + parseRpId(rpIdString), + primaryConfig.contractAddress, + ); + if (onChain.initialized) { + logger.warn("Self-managed rp_id is already claimed on-chain", { + app_id, + rpIdString, + onChainManager: onChain.manager, + }); + return errorHasuraQuery({ + req, + detail: + "This app's RP ID is already registered on-chain. Portal cannot manage it — contact support.", + code: "rp_id_taken", + app_id, + }); + } + } catch (error) { + logger.warn("Could not pre-check on-chain RP ownership; continuing", { + error, + app_id, + rpIdString, + }); + } + } + const { insert_rp_registration_one: claimedSlot } = await getClaimRpSdk( client, ).ClaimRpRegistration({ diff --git a/web/api/helpers/rp-registration-flows.ts b/web/api/helpers/rp-registration-flows.ts index b5c5e0f6a..ff3cf87e4 100644 --- a/web/api/helpers/rp-registration-flows.ts +++ b/web/api/helpers/rp-registration-flows.ts @@ -24,12 +24,14 @@ import { getSdk as getRevertToggleSdk } from "@/api/hasura/toggle-rp-active/grap import { getSdk as getUpdateToggleSdk } from "@/api/hasura/toggle-rp-active/graphql/update-toggle-result.generated"; import { getKMSClient, scheduleKeyDeletion } from "@/api/helpers/kms"; import { createManagerKey, getEthAddressFromKMS } from "@/api/helpers/kms-eth"; +import { resolveManagerAddress } from "@/api/helpers/rp-manager"; import { submitRegisterRpTransaction, submitRotateSignerTransaction, submitToggleRpActiveTransaction, } from "@/api/helpers/rp-transactions"; import { + addressesEqual, generateRpIdString, getRpRegistryConfig, getStagingRpRegistryConfig, @@ -151,9 +153,8 @@ export async function submitManagedRpRegistration({ // dependency to a flow that works fine without it. // // Only the read is inside the try. A wider catch would swallow a failure from - // the slot release below and fall through to KMS and a UserOp for an rp_id we - // have already proven is taken — the exact wedged row this check exists to - // avoid. + // a slot release below and fall through to KMS and a UserOp for an rp_id we + // have already resolved, which is the wedged row this check exists to avoid. let existingOnChainRp: Awaited> | null = null; try { @@ -169,6 +170,81 @@ export async function submitManagedRpRegistration({ }); } + // Releasing the slot must never abort the decision that led here: the row is + // ours to clean up, and a failure leaves it wedged for ops rather than changing + // what the caller should be told. + const releaseSlot = async () => { + try { + await getDeleteRpSdk(client).DeleteRpRegistration({ rp_id: rpIdString }); + } catch (error) { + logger.error("Failed to release the registration slot", { + error, + app_id: appId, + rpIdString, + }); + } + }; + + // The claim above means the Portal held no row for this app, so an existing + // on-chain entry is either a squatter's or one WE claimed defensively via + // _pre-register-rp-ids. Those look identical except for the manager, which only + // our KMS key can sign for — so the manager is what tells them apart. + // + // Three outcomes, and the difference between them is what the caller can do: + // - manager is ours -> adopt: rotate the placeholder signer to the + // real one instead of registering an id we already hold + // - manager is someone else -> rp_id_taken, terminal, needs support + // - manager unresolvable -> kms_error, RETRYABLE. A KMS outage must not be + // reported as "someone else owns your id, contact support"; that would send + // developers of pre-claimed apps to support over a transient failure. + let adoptExistingClaim = false; + if (existingOnChainRp?.initialized) { + const sharedManagerKeyId = process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; + const ourManagerAddress = sharedManagerKeyId + ? await resolveManagerAddress(sharedManagerKeyId, primaryConfig.kmsRegion) + : null; + + if (sharedManagerKeyId && !ourManagerAddress) { + logger.error( + "Cannot tell whether an initialized rp_id is a Portal pre-claim", + { app_id: appId, rpIdString }, + ); + await releaseSlot(); + return { + ok: false, + code: "kms_error", + detail: "Failed to resolve manager key. Please try again.", + }; + } + + adoptExistingClaim = Boolean( + ourManagerAddress && + addressesEqual(existingOnChainRp.manager, ourManagerAddress), + ); + + if (!adoptExistingClaim) { + logger.warn("rp_id already registered on-chain by a foreign manager", { + app_id: appId, + rpIdString, + onChainManager: existingOnChainRp.manager, + onChainSigner: existingOnChainRp.signer, + }); + await releaseSlot(); + return { + ok: false, + code: "rp_id_taken", + detail: + "This app's RP ID is already registered on-chain by another party. Portal cannot manage it — contact support.", + }; + } + + logger.info("Adopting an rp_id the Portal claimed defensively", { + app_id: appId, + rpIdString, + onChainSigner: existingOnChainRp.signer, + }); + } + if (existingOnChainRp?.initialized) { logger.warn("rp_id already registered on-chain by a foreign manager", { app_id: appId, @@ -238,7 +314,12 @@ export async function submitManagedRpRegistration({ let managerAddress: string; let isUniqueManagerKey: boolean; + // Adoption has no choice of key: the manager already recorded on-chain is the + // shared one we claimed with, and only that key can sign the rotation. Minting + // a dedicated key here would produce a manager the contract has never heard + // of, and every update would revert. const useSharedManagerKey = + adoptExistingClaim || process.env.ENABLE_SHARED_KEY_RP_REGISTRATION === "true"; if (useSharedManagerKey) { @@ -291,17 +372,29 @@ export async function submitManagedRpRegistration({ let operationHash: string; try { - operationHash = await submitRegisterRpTransaction(primaryConfig, { - rpId, - managerAddress, - signerAddress, - appName, - kmsClient, - }); + operationHash = adoptExistingClaim + ? // The id is already registered to our manager with a placeholder + // signer, so registering again would revert with IdAlreadyInUse. + // Rotating installs the developer's real signer and completes the + // handover in one transaction. + await submitRotateSignerTransaction(primaryConfig, { + rpId, + newSignerAddress: signerAddress, + managerKmsKeyId, + kmsClient, + }) + : await submitRegisterRpTransaction(primaryConfig, { + rpId, + managerAddress, + signerAddress, + appName, + kmsClient, + }); } catch (error) { logger.error("Failed to submit registration transaction", { error, app_id: appId, + adopting: adoptExistingClaim, }); if (isUniqueManagerKey) { await scheduleKeyDeletion(kmsClient, managerKmsKeyId); diff --git a/web/api/helpers/rp-utils.ts b/web/api/helpers/rp-utils.ts index 41999c25c..5ca59c472 100644 --- a/web/api/helpers/rp-utils.ts +++ b/web/api/helpers/rp-utils.ts @@ -85,7 +85,12 @@ export function isZeroAddress(address: string): boolean { return normalizeAddress(address).toLowerCase() === ZERO_ADDRESS; } -function addressesEqual(a: string, b: string): boolean { +/** + * Case- and checksum-insensitive address comparison. Exported so ownership + * checks outside this module can't drift into their own `.toLowerCase()` + * comparison that skips normalization. + */ +export function addressesEqual(a: string, b: string): boolean { return ( normalizeAddress(a).toLowerCase() === normalizeAddress(b).toLowerCase() ); diff --git a/web/tests/api/helpers/rp-registration-flows.test.ts b/web/tests/api/helpers/rp-registration-flows.test.ts index 6ac024c31..af91377c5 100644 --- a/web/tests/api/helpers/rp-registration-flows.test.ts +++ b/web/tests/api/helpers/rp-registration-flows.test.ts @@ -92,12 +92,14 @@ jest.mock("@/api/helpers/temporal-rpc", () => ({ const submitToggleRpActiveTransactionMock = jest.fn(); const submitRegisterRpTransactionMock = jest.fn(); +const submitRotateSignerTransactionMock = jest.fn(); jest.mock("@/api/helpers/rp-transactions", () => ({ submitToggleRpActiveTransaction: (...args: unknown[]) => submitToggleRpActiveTransactionMock(...args), submitRegisterRpTransaction: (...args: unknown[]) => submitRegisterRpTransactionMock(...args), - submitRotateSignerTransaction: jest.fn(), + submitRotateSignerTransaction: (...args: unknown[]) => + submitRotateSignerTransactionMock(...args), })); jest.mock("@/api/helpers/kms", () => ({ @@ -208,6 +210,7 @@ beforeEach(() => { rp_registration_by_pk: { rp_id: rpId, is_unique_manager_key: false }, }); submitRegisterRpTransactionMock.mockResolvedValue("0xregophash"); + submitRotateSignerTransactionMock.mockResolvedValue("0xrotateophash"); (createManagerKey as jest.Mock).mockResolvedValue({ keyId: dedicatedManagerKeyId, address: managerAddress, @@ -1143,6 +1146,50 @@ describe("submitManagedRpRegistration [rp_id collision guard]", () => { expect(getRpFromContractMock).not.toHaveBeenCalled(); }); + it("adopts an rp_id the Portal claimed defensively by rotating its signer", async () => { + // Pre-registration (_pre-register-rp-ids) claims the id with the SHARED + // manager key and a placeholder signer. Registering again would revert with + // IdAlreadyInUse, so the real registration has to rotate instead — otherwise + // every pre-claimed app is permanently locked out of onboarding. + process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = sharedManagerKeyArn; + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: managerAddress, + signer: "0x000000000000000000000000000000000000dead", + }); + + const res = await register(); + + expect(res).toMatchObject({ ok: true, operationHash: "0xrotateophash" }); + expect(submitRotateSignerTransactionMock).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ newSignerAddress: signerAddress }), + ); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + // Adoption must sign with the key already recorded on-chain, so it cannot + // mint a dedicated one even though shared mode is off. + expect(createManagerKey as jest.Mock).not.toHaveBeenCalled(); + }); + + it("still refuses a foreign claim when the shared manager key is configured", async () => { + // The manager is what distinguishes our own claim from a squatter's, so the + // adoption path must not widen into "any initialized RP is ours". + process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = sharedManagerKeyArn; + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: "0x00000000000000000000000000000000000000ff", + signer: "0x00000000000000000000000000000000000000ee", + }); + + const res = await register(); + + expect(res).toMatchObject({ ok: false, code: "rp_id_taken" }); + expect(submitRotateSignerTransactionMock).not.toHaveBeenCalled(); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + it("still reports rp_id_taken when releasing the slot fails", async () => { // The read's non-fatal catch must not extend over the slot release: swallowing // that failure would fall through to KMS and a UserOp for an rp_id already diff --git a/web/tests/api/pre-register-rp-ids.test.ts b/web/tests/api/pre-register-rp-ids.test.ts new file mode 100644 index 000000000..ba2957271 --- /dev/null +++ b/web/tests/api/pre-register-rp-ids.test.ts @@ -0,0 +1,268 @@ +import { POST } from "@/api/_pre-register-rp-ids"; +import { NextRequest } from "next/server"; + +// #region Mocks +const GetAppInfo = jest.fn(); +jest.mock("@/api/hasura/register-rp/graphql/get-app-info.generated", () => ({ + getSdk: () => ({ GetAppInfo }), +})); + +const FetchRpRegistration = jest.fn(); +jest.mock("@/api/helpers/graphql/fetch-rp-registration.generated", () => ({ + getSdk: () => ({ FetchRpRegistration }), +})); + +jest.mock("@/api/helpers/graphql", () => ({ + getAPIServiceGraphqlClient: jest.fn().mockResolvedValue({}), +})); + +jest.mock("@/api/helpers/kms", () => ({ + getKMSClient: jest.fn().mockResolvedValue({}), +})); + +const resolveManagerAddressMock = jest.fn(); +jest.mock("@/api/helpers/rp-manager", () => ({ + resolveManagerAddress: (...args: unknown[]) => + resolveManagerAddressMock(...args), +})); + +const submitRegisterRpTransactionMock = jest.fn(); +jest.mock("@/api/helpers/rp-transactions", () => ({ + submitRegisterRpTransaction: (...args: unknown[]) => + submitRegisterRpTransactionMock(...args), +})); + +const getRpFromContractMock = jest.fn(); +jest.mock("@/api/helpers/temporal-rpc", () => ({ + getRpFromContract: (...args: unknown[]) => getRpFromContractMock(...args), +})); + +const getRpRegistryConfigMock = jest.fn(); +jest.mock("@/api/helpers/rp-utils", () => { + const actual = jest.requireActual("@/api/helpers/rp-utils"); + return { + ...actual, + getRpRegistryConfig: () => getRpRegistryConfigMock(), + }; +}); + +jest.mock("@/lib/logger", () => ({ + logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, +})); +// #endregion + +// #region Test Data +const appId = "app_00000000000000000000000000000001"; +const managerAddress = "0x2222222222222222222222222222222222222222"; +const placeholderSigner = "0x000000000000000000000000000000000000dEaD"; + +const createRequest = (body: unknown) => + new NextRequest("https://cdn.test.com/api/_pre-register-rp-ids", { + method: "POST", + headers: new Headers({ + "Content-Type": "application/json", + authorization: `Bearer ${process.env.INTERNAL_ENDPOINTS_SECRET}`, + }), + body: JSON.stringify(body), + }); + +// The handler's return type is nullable because protectInternalEndpoint's +// errorResponse is; every call here is authenticated, so narrow once. +const post = async (body: unknown) => { + const res = await POST(createRequest(body)); + expect(res).not.toBeNull(); + return res!; +}; +// #endregion + +beforeEach(() => { + jest.clearAllMocks(); + process.env.INTERNAL_ENDPOINTS_SECRET = "test_secret"; + process.env.ENABLE_RP_ID_PRE_REGISTRATION = "true"; + process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = + "arn:aws:kms:eu-west-1:000000000000:key/shared-manager"; + process.env.RP_ID_PRE_REGISTRATION_SIGNER = placeholderSigner; + + getRpRegistryConfigMock.mockReturnValue({ + contractAddress: "0xcontract", + kmsRegion: "eu-west-1", + }); + resolveManagerAddressMock.mockResolvedValue(managerAddress); + GetAppInfo.mockResolvedValue({ + app: [{ id: appId, is_staging: false, app_metadata: [{ name: "Test" }] }], + }); + FetchRpRegistration.mockResolvedValue({ rp_registration: [] }); + getRpFromContractMock.mockResolvedValue({ + initialized: false, + active: false, + manager: `0x${"0".repeat(40)}`, + signer: `0x${"0".repeat(40)}`, + }); + submitRegisterRpTransactionMock.mockResolvedValue("0xclaimop"); +}); + +// #region Kill switch and configuration guards +describe("/api/_pre-register-rp-ids [guards]", () => { + it("refuses to run when the kill switch is off", async () => { + delete process.env.ENABLE_RP_ID_PRE_REGISTRATION; + + const res = await post({ app_ids: [appId] }); + + expect(res.status).toBe(503); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("refuses to run without a shared manager key", async () => { + delete process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; + + const res = await post({ app_ids: [appId] }); + + expect(res.status).toBe(500); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("refuses a zero-address placeholder signer", async () => { + // A zero signer would be accepted by the contract and leave an RP nobody + // can ever sign for. + process.env.RP_ID_PRE_REGISTRATION_SIGNER = `0x${"0".repeat(40)}`; + + const res = await post({ app_ids: [appId] }); + + expect(res.status).toBe(500); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("refuses to claim when our own manager address cannot be resolved", async () => { + // Without it, "already ours" is indistinguishable from "someone else's". + resolveManagerAddressMock.mockResolvedValue(null); + + const res = await post({ app_ids: [appId] }); + + expect(res.status).toBe(503); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("rejects a batch larger than the per-call ceiling", async () => { + const tooMany = Array.from( + { length: 26 }, + (_, i) => `app_${String(i).padStart(32, "0")}`, + ); + + const res = await post({ app_ids: tooMany }); + + expect(res.status).toBe(400); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); +}); +// #endregion + +// #region Dry run is the default +describe("/api/_pre-register-rp-ids [dry run]", () => { + it("reports what it would claim without submitting anything", async () => { + const res = await post({ app_ids: [appId] }); + + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.dry_run).toBe(true); + expect(body.counts).toEqual({ would_claim: 1 }); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("only submits when dry_run is explicitly false", async () => { + const res = await post({ app_ids: [appId], dry_run: false }); + + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.counts).toEqual({ claimed: 1 }); + expect(submitRegisterRpTransactionMock).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + managerAddress, + signerAddress: placeholderSigner, + }), + ); + }); +}); +// #endregion + +// #region Per-app skip decisions +describe("/api/_pre-register-rp-ids [skips]", () => { + const run = () => post({ app_ids: [appId], dry_run: false }); + + it("skips an app that already has a Portal registration", async () => { + FetchRpRegistration.mockResolvedValue({ + rp_registration: [{ rp_id: "rp_0123456789abcdef" }], + }); + + const body = await (await run()).json(); + + expect(body.counts).toEqual({ skipped_already_registered_in_portal: 1 }); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("skips staging apps, which never migrate", async () => { + GetAppInfo.mockResolvedValue({ + app: [{ id: appId, is_staging: true, app_metadata: [{ name: "Test" }] }], + }); + + const body = await (await run()).json(); + + expect(body.counts).toEqual({ skipped_staging: 1 }); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("skips an id we already claimed, so repeated runs are idempotent", async () => { + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: managerAddress, + signer: placeholderSigner, + }); + + const body = await (await run()).json(); + + expect(body.counts).toEqual({ skipped_already_claimed_by_us: 1 }); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("reports an id already squatted by a foreign manager", async () => { + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: "0x00000000000000000000000000000000000000ff", + signer: "0x00000000000000000000000000000000000000ee", + }); + + const body = await (await run()).json(); + + expect(body.counts).toEqual({ skipped_taken_by_foreign_manager: 1 }); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("does not claim on a failed on-chain read", async () => { + // register() reverts with IdAlreadyInUse if the id is taken, so a blind + // submission after a failed read just burns gas. + getRpFromContractMock.mockRejectedValue(new Error("rpc timeout")); + + const body = await (await run()).json(); + + expect(body.counts).toEqual({ failed_rpc: 1 }); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("records a submission failure without aborting the rest of the batch", async () => { + const secondAppId = "app_00000000000000000000000000000002"; + submitRegisterRpTransactionMock + .mockRejectedValueOnce(new Error("bundler down")) + .mockResolvedValueOnce("0xclaimop"); + + const res = await post({ + app_ids: [appId, secondAppId], + dry_run: false, + }); + const body = await res.json(); + + expect(body.counts).toEqual({ failed_submission: 1, claimed: 1 }); + }); +}); +// #endregion From cc3bb0219647d1711731a5016b1a06b0e44ae658 Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 12:14:17 +0200 Subject: [PATCH 02/12] fix(world-id): route the pre-registration endpoint and dedupe its input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Codex findings, both real: Handlers under web/api are only reachable through a matching app-router re-export, and the leading-underscore endpoints use %5F-escaped route folders (see web/app/api/%5Fdeactivate-deleted-app-rps). Without one, POST /api/_pre-register-rp-ids 404s and the tool only ever worked from unit tests. `next build` now lists the route next to the existing cron. A repeated app_id was processed twice. submitRegisterRpTransaction returns once the UserOp is submitted rather than mined, so the second pass would read the chain as still uninitialized, submit another register() for the same rp_id with a fresh nonce, and report a second claim — spending gas twice and defeating the per-call ceiling the endpoint exists to enforce. Deduped before the loop, and the drop is logged rather than silent. Co-Authored-By: Claude --- web/api/_pre-register-rp-ids/index.ts | 15 ++++++++++++++- web/app/api/%5Fpre-register-rp-ids/route.ts | 1 + web/tests/api/pre-register-rp-ids.test.ts | 11 +++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 web/app/api/%5Fpre-register-rp-ids/route.ts diff --git a/web/api/_pre-register-rp-ids/index.ts b/web/api/_pre-register-rp-ids/index.ts index 860da4869..a178f4382 100644 --- a/web/api/_pre-register-rp-ids/index.ts +++ b/web/api/_pre-register-rp-ids/index.ts @@ -113,7 +113,20 @@ export async function POST(request: NextRequest) { ); } - const { app_ids: appIds, dry_run: dryRun } = parsedParams; + const { app_ids: rawAppIds, dry_run: dryRun } = parsedParams; + + // Dedupe before the loop. submitRegisterRpTransaction returns once the UserOp + // is submitted, not once it is mined, so a repeated app_id would read the + // chain as still uninitialized, submit a second register() for the same rp_id + // with a fresh nonce, and report two claims — double-spending gas and defeating + // the per-call ceiling this endpoint exists to enforce. + const appIds = Array.from(new Set(rawAppIds)); + if (appIds.length !== rawAppIds.length) { + logger.warn("Dropped duplicate app_ids from pre-registration request", { + requested: rawAppIds.length, + unique: appIds.length, + }); + } const config = getRpRegistryConfig(); if (!config) { diff --git a/web/app/api/%5Fpre-register-rp-ids/route.ts b/web/app/api/%5Fpre-register-rp-ids/route.ts new file mode 100644 index 000000000..67f9f93ae --- /dev/null +++ b/web/app/api/%5Fpre-register-rp-ids/route.ts @@ -0,0 +1 @@ +export { POST } from "@/api/_pre-register-rp-ids"; diff --git a/web/tests/api/pre-register-rp-ids.test.ts b/web/tests/api/pre-register-rp-ids.test.ts index ba2957271..9f555cd57 100644 --- a/web/tests/api/pre-register-rp-ids.test.ts +++ b/web/tests/api/pre-register-rp-ids.test.ts @@ -250,6 +250,17 @@ describe("/api/_pre-register-rp-ids [skips]", () => { expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); }); + it("claims a repeated app_id only once", async () => { + // submitRegisterRpTransaction returns on submission, not on mining, so a + // duplicate would read the chain as still free and submit a second + // register() for the same rp_id — double-spending gas. + const res = await post({ app_ids: [appId, appId], dry_run: false }); + + const body = await res.json(); + expect(body.counts).toEqual({ claimed: 1 }); + expect(submitRegisterRpTransactionMock).toHaveBeenCalledTimes(1); + }); + it("records a submission failure without aborting the rest of the batch", async () => { const secondAppId = "app_00000000000000000000000000000002"; submitRegisterRpTransactionMock From 7ea7e404d7b478e1ea6656679f73344afaa9198b Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 12:22:41 +0200 Subject: [PATCH 03/12] fix(world-id): don't reject a self-managed developer's own registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard added with pre-registration failed any self-managed rp_id that was already initialized on-chain. That is backwards: the self-managed developer runs register() from their own wallet on the instructions screen, and this mutation is the "Continue" that follows — so an initialized id is the HEALTHY state, and the guard would have broken every legitimate self-managed completion, leaving those apps with no row for status or proof-context reconciliation. Only an id held by the Portal's own shared manager should fail: that is the defensive pre-claim, the developer's register() reverted against it, and handing it over needs a manager transfer no flow drives yet. Any other manager is the developer's own registration — or a squatter's, which for self-managed the Portal cannot distinguish either way, unchanged by this PR since it stores no expected roles for self-managed rows. Tests now pin both directions, since the existing suite passed only because the guard is skipped when no shared manager key is configured. Codex independently flagged the same inversion. Co-Authored-By: Claude --- web/api/hasura/register-rp/index.ts | 60 +++++++++++------- web/tests/api/hasura/register-rp.test.ts | 79 ++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 22 deletions(-) diff --git a/web/api/hasura/register-rp/index.ts b/web/api/hasura/register-rp/index.ts index eeb8bb2d3..7eb63af6d 100644 --- a/web/api/hasura/register-rp/index.ts +++ b/web/api/hasura/register-rp/index.ts @@ -5,7 +5,9 @@ import { submitManagedRpRegistration, type ManagedRegistrationResult, } from "@/api/helpers/rp-registration-flows"; +import { resolveManagerAddress } from "@/api/helpers/rp-manager"; import { + addressesEqual, generateRpIdString, getRpRegistryConfig, isZeroAddress, @@ -157,36 +159,50 @@ export const POST = async (req: NextRequest) => { if (mode === "self_managed") { const rpIdString = generateRpIdString(app_id); - // A self-managed developer registers the rp_id from their own wallet, so if - // the id is already claimed on-chain their `register()` has already reverted - // (or is about to). Inserting the row anyway would leave the dashboard - // polling `pending` forever with no explanation. This covers both a - // squatter and an id the Portal claimed defensively via - // _pre-register-rp-ids — the latter needs the manager transferred to them, - // which no flow drives yet, so both cases route to support. + // A self-managed developer runs `register()` from their own wallet BEFORE + // reaching this mutation — the instructions screen hands them the calldata + // and this is the "Continue" that follows. So an initialized rp_id is the + // HEALTHY state here and must not be treated as a conflict. // - // Best-effort: a read failure must not block onboarding for the common case - // where the id is free. + // The one case that has to fail is an id the Portal claimed defensively via + // _pre-register-rp-ids: the developer's own `register()` reverted against it, + // and handing them the id needs the manager transferred to them, which no + // flow drives yet. Our shared manager address is what identifies that case; + // any other manager is the developer's own registration (or a squatter's, + // which for self-managed we cannot tell apart either way — unchanged by + // this PR, since the Portal stores no expected roles for self-managed). + // + // Best-effort: a read failure must not block onboarding. const primaryConfig = getRpRegistryConfig(); - if (primaryConfig) { + const sharedManagerKeyId = process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; + if (primaryConfig && sharedManagerKeyId) { try { const onChain = await getRpFromContract( parseRpId(rpIdString), primaryConfig.contractAddress, ); if (onChain.initialized) { - logger.warn("Self-managed rp_id is already claimed on-chain", { - app_id, - rpIdString, - onChainManager: onChain.manager, - }); - return errorHasuraQuery({ - req, - detail: - "This app's RP ID is already registered on-chain. Portal cannot manage it — contact support.", - code: "rp_id_taken", - app_id, - }); + const ourManagerAddress = await resolveManagerAddress( + sharedManagerKeyId, + primaryConfig.kmsRegion, + ); + if ( + ourManagerAddress && + addressesEqual(onChain.manager, ourManagerAddress) + ) { + logger.warn("Self-managed rp_id is held by the Portal's manager", { + app_id, + rpIdString, + onChainManager: onChain.manager, + }); + return errorHasuraQuery({ + req, + detail: + "This app's RP ID is held by Portal and cannot be self-managed yet — contact support.", + code: "rp_id_taken", + app_id, + }); + } } } catch (error) { logger.warn("Could not pre-check on-chain RP ownership; continuing", { diff --git a/web/tests/api/hasura/register-rp.test.ts b/web/tests/api/hasura/register-rp.test.ts index 227a6cb26..e51bdc733 100644 --- a/web/tests/api/hasura/register-rp.test.ts +++ b/web/tests/api/hasura/register-rp.test.ts @@ -14,6 +14,26 @@ jest.mock("@/api/helpers/rp-registration-flows", () => ({ submitManagedRpRegistrationMock(...args), })); +const getRpFromContractMock = jest.fn(); +jest.mock("@/api/helpers/temporal-rpc", () => ({ + getRpFromContract: (...args: unknown[]) => getRpFromContractMock(...args), +})); + +const resolveManagerAddressMock = jest.fn(); +jest.mock("@/api/helpers/rp-manager", () => ({ + resolveManagerAddress: (...args: unknown[]) => + resolveManagerAddressMock(...args), +})); + +const getRpRegistryConfigMock = jest.fn(); +jest.mock("@/api/helpers/rp-utils", () => { + const actual = jest.requireActual("@/api/helpers/rp-utils"); + return { + ...actual, + getRpRegistryConfig: () => getRpRegistryConfigMock(), + }; +}); + jest.mock("@/lib/logger", () => ({ logger: { info: jest.fn(), @@ -62,12 +82,30 @@ const createMockRequest = (input: Record) => }); // #endregion +const portalManagerAddress = "0x2222222222222222222222222222222222222222"; + beforeEach(() => { jest.clearAllMocks(); process.env.INTERNAL_ENDPOINTS_SECRET = "internal-secret"; + process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = + "arn:aws:kms:eu-west-1:000000000000:key/shared-manager"; appIsStaging = false; authorizedTeam = [{ id: teamId }]; + getRpRegistryConfigMock.mockReturnValue({ + contractAddress: "0xcontract", + kmsRegion: "eu-west-1", + }); + resolveManagerAddressMock.mockResolvedValue(portalManagerAddress); + // Default: the self-managed developer has already run register() themselves, + // which is the state this mutation is reached in. + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: "0xDeveloperOwnedManager", + signer: "0xDeveloperOwnedSigner", + }); + submitManagedRpRegistrationMock.mockResolvedValue({ ok: true, rpIdString: "rp_abc123", @@ -153,6 +191,47 @@ describe("/api/hasura/register-rp [success]", () => { }); // #endregion +// #region Self-managed vs a defensively claimed rp_id +describe("/api/hasura/register-rp [self-managed on-chain ownership]", () => { + const selfManaged = () => + POST(createMockRequest({ app_id: appId, mode: "self_managed" })); + + it("succeeds when the developer has already registered the id themselves", async () => { + // This mutation is the "Continue" AFTER the instructions screen, so the id + // being initialized on-chain is the healthy state. Treating any initialized + // id as a conflict would break every legitimate self-managed completion. + const res = (await selfManaged())!; + + expect(res.status).toBe(200); + expect((await res.json()).status).toBe("pending"); + }); + + it("refuses when the id is held by the Portal's own manager", async () => { + // Defensively pre-claimed: the developer's register() reverted against it, + // and handing the id over needs a manager transfer that no flow drives yet. + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: portalManagerAddress, + signer: "0x000000000000000000000000000000000000dEaD", + }); + + const res = (await selfManaged())!; + const body = await res.json(); + + expect(body.code ?? body.extensions?.code).toBe("rp_id_taken"); + }); + + it("still succeeds when the on-chain read fails", async () => { + getRpFromContractMock.mockRejectedValue(new Error("rpc timeout")); + + const res = (await selfManaged())!; + + expect(res.status).toBe(200); + }); +}); +// #endregion + // #region Staging app migration (product guard — kept) describe("/api/hasura/register-rp [staging app migration]", () => { it("rejects managed RP registration for staging apps", async () => { From 0837084a775a79a3bae8b620b3036c692c6c5e13 Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 12:37:43 +0200 Subject: [PATCH 04/12] fix(world-id): recognise a pre-claim without depending on KMS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The self-managed guard identified a Portal pre-claim by resolving our shared manager key and comparing addresses. Codex pointed out the fail-open: when resolveManagerAddress returns null during a KMS outage the comparison is false, the handler inserts the row anyway, and because rp-status trusts self-managed rows by mode it later promotes that row against the pre-claim's placeholder signer — leaving the developer a registration that can never sign. Failing closed instead would be worse. An initialized rp_id is the NORMAL state at this point (the developer just registered it themselves), so requiring a resolved manager would break every legitimate self-managed completion whenever KMS is unavailable — and KMS has no business in this flow at all, since the Portal holds no keys for self-managed apps. Compare the placeholder signer instead. It is a plain env var we control, no remote call is involved, so the check cannot fail open or fail closed on someone else's outage. A pre-claim is the only thing that carries it: only we could rotate it away, and doing so creates a row, which this path answers with already_registered from the DB claim. Co-Authored-By: Claude --- web/api/hasura/register-rp/index.ts | 64 +++++++++++++----------- web/tests/api/hasura/register-rp.test.ts | 26 +++++++++- 2 files changed, 58 insertions(+), 32 deletions(-) diff --git a/web/api/hasura/register-rp/index.ts b/web/api/hasura/register-rp/index.ts index 7eb63af6d..f55a25b37 100644 --- a/web/api/hasura/register-rp/index.ts +++ b/web/api/hasura/register-rp/index.ts @@ -5,7 +5,6 @@ import { submitManagedRpRegistration, type ManagedRegistrationResult, } from "@/api/helpers/rp-registration-flows"; -import { resolveManagerAddress } from "@/api/helpers/rp-manager"; import { addressesEqual, generateRpIdString, @@ -167,44 +166,49 @@ export const POST = async (req: NextRequest) => { // The one case that has to fail is an id the Portal claimed defensively via // _pre-register-rp-ids: the developer's own `register()` reverted against it, // and handing them the id needs the manager transferred to them, which no - // flow drives yet. Our shared manager address is what identifies that case; - // any other manager is the developer's own registration (or a squatter's, - // which for self-managed we cannot tell apart either way — unchanged by - // this PR, since the Portal stores no expected roles for self-managed). + // flow drives yet. // - // Best-effort: a read failure must not block onboarding. + // The PLACEHOLDER SIGNER identifies that case, not the manager address. Both + // would work, but resolving our manager means a KMS call, and KMS has no + // business in a self-managed registration — the Portal holds no keys for + // these apps. Depending on it would mean that during a KMS outage this check + // either blocks every legitimate self-managed completion (initialized is the + // normal state here) or silently falls through and lets a pre-claimed id + // through, which rp-status then promotes against the placeholder signer, + // leaving the developer a registration that can never sign. The placeholder + // is a plain env var we control, so the comparison cannot fail open. + // + // Any other signer is the developer's own registration — or a squatter's, + // which for self-managed the Portal cannot tell apart either way, unchanged + // by this PR since it stores no expected roles for self-managed rows. const primaryConfig = getRpRegistryConfig(); - const sharedManagerKeyId = process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; - if (primaryConfig && sharedManagerKeyId) { + const placeholderSigner = process.env.RP_ID_PRE_REGISTRATION_SIGNER; + if (primaryConfig && placeholderSigner) { try { const onChain = await getRpFromContract( parseRpId(rpIdString), primaryConfig.contractAddress, ); - if (onChain.initialized) { - const ourManagerAddress = await resolveManagerAddress( - sharedManagerKeyId, - primaryConfig.kmsRegion, - ); - if ( - ourManagerAddress && - addressesEqual(onChain.manager, ourManagerAddress) - ) { - logger.warn("Self-managed rp_id is held by the Portal's manager", { - app_id, - rpIdString, - onChainManager: onChain.manager, - }); - return errorHasuraQuery({ - req, - detail: - "This app's RP ID is held by Portal and cannot be self-managed yet — contact support.", - code: "rp_id_taken", - app_id, - }); - } + if ( + onChain.initialized && + addressesEqual(onChain.signer, placeholderSigner) + ) { + logger.warn("Self-managed rp_id is held by a Portal pre-claim", { + app_id, + rpIdString, + onChainManager: onChain.manager, + }); + return errorHasuraQuery({ + req, + detail: + "This app's RP ID is held by Portal and cannot be self-managed yet — contact support.", + code: "rp_id_taken", + app_id, + }); } } catch (error) { + // A read failure must not block onboarding: the common case is an id the + // developer just registered themselves. logger.warn("Could not pre-check on-chain RP ownership; continuing", { error, app_id, diff --git a/web/tests/api/hasura/register-rp.test.ts b/web/tests/api/hasura/register-rp.test.ts index e51bdc733..02440e613 100644 --- a/web/tests/api/hasura/register-rp.test.ts +++ b/web/tests/api/hasura/register-rp.test.ts @@ -83,12 +83,14 @@ const createMockRequest = (input: Record) => // #endregion const portalManagerAddress = "0x2222222222222222222222222222222222222222"; +const placeholderSigner = "0x000000000000000000000000000000000000dEaD"; beforeEach(() => { jest.clearAllMocks(); process.env.INTERNAL_ENDPOINTS_SECRET = "internal-secret"; process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = "arn:aws:kms:eu-west-1:000000000000:key/shared-manager"; + process.env.RP_ID_PRE_REGISTRATION_SIGNER = placeholderSigner; appIsStaging = false; authorizedTeam = [{ id: teamId }]; @@ -206,14 +208,14 @@ describe("/api/hasura/register-rp [self-managed on-chain ownership]", () => { expect((await res.json()).status).toBe("pending"); }); - it("refuses when the id is held by the Portal's own manager", async () => { + it("refuses when the id carries the Portal's pre-claim placeholder signer", async () => { // Defensively pre-claimed: the developer's register() reverted against it, // and handing the id over needs a manager transfer that no flow drives yet. getRpFromContractMock.mockResolvedValue({ initialized: true, active: true, manager: portalManagerAddress, - signer: "0x000000000000000000000000000000000000dEaD", + signer: placeholderSigner, }); const res = (await selfManaged())!; @@ -222,6 +224,26 @@ describe("/api/hasura/register-rp [self-managed on-chain ownership]", () => { expect(body.code ?? body.extensions?.code).toBe("rp_id_taken"); }); + it("does not depend on KMS to recognise a pre-claim", async () => { + // The check compares the placeholder signer, an env var we control, rather + // than resolving our manager key. A KMS outage must not decide between + // blocking every legitimate self-managed completion and silently admitting a + // pre-claimed id that rp-status would then promote against a dead signer. + resolveManagerAddressMock.mockResolvedValue(null); + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: portalManagerAddress, + signer: placeholderSigner, + }); + + const res = (await selfManaged())!; + const body = await res.json(); + + expect(body.code ?? body.extensions?.code).toBe("rp_id_taken"); + expect(resolveManagerAddressMock).not.toHaveBeenCalled(); + }); + it("still succeeds when the on-chain read fails", async () => { getRpFromContractMock.mockRejectedValue(new Error("rpc timeout")); From f09c1df70ffcf9543b08e7ed32596f8ff6327919 Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 12:49:26 +0200 Subject: [PATCH 05/12] fix(world-id): stop reporting unknown pre-claim state as a terminal error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Codex findings, both the same shape: an unknown state was being reported as something permanent. Managed adoption converted an unresolved shared manager address straight to "not ours", so a transient KMS failure told the developer of a pre-claimed app that someone else owns their rp_id and to contact support. Unresolvable is now its own outcome — a retryable kms_error, with the claimed slot released so the retry is not met with already_registered. The self-managed placeholder-signer guard failed open whenever it could not run: no placeholder configured, or getRpRegistryConfig() null for an unrelated reason. For an app the Portal has pre-claimed that inserts a row which rp-status promotes (it trusts self-managed rows by mode) against a signer that can never sign. What skipping means now depends on whether pre-claims can exist at all: - pre-registration enabled but misconfigured -> config_error, a deploy fault - placeholder set but the chain unreadable -> rpc_error, retryable - placeholder unset, never enabled -> skip; no claims exist That last case is every environment that has not run the tool, so it must stay open — the alternative is making self-managed registration depend on config it has no reason to need. Noted the operational invariant in the code: once pre-registration has run somewhere, RP_ID_PRE_REGISTRATION_SIGNER has to stay set there, because the claims outlive the flag. Rebased onto #2221, resolving the overlap where the collision check moved after the DB claim: adoption keeps the slot, both failure paths release it. Co-Authored-By: Claude --- web/api/hasura/register-rp/index.ts | 92 ++++++++++++++----- web/tests/api/hasura/register-rp.test.ts | 33 ++++++- .../api/helpers/rp-registration-flows.test.ts | 26 ++++++ 3 files changed, 127 insertions(+), 24 deletions(-) diff --git a/web/api/hasura/register-rp/index.ts b/web/api/hasura/register-rp/index.ts index f55a25b37..65466d602 100644 --- a/web/api/hasura/register-rp/index.ts +++ b/web/api/hasura/register-rp/index.ts @@ -181,39 +181,85 @@ export const POST = async (req: NextRequest) => { // Any other signer is the developer's own registration — or a squatter's, // which for self-managed the Portal cannot tell apart either way, unchanged // by this PR since it stores no expected roles for self-managed rows. + // + // When the check CANNOT run, whether skipping is safe depends entirely on + // whether pre-claims can exist: + // - pre-registration enabled but misconfigured -> config_error. Claims are + // being made and we cannot recognise them, which is a deploy fault, not a + // developer's problem to absorb silently. + // - placeholder configured but the chain unreadable -> rpc_error, RETRYABLE. + // A pre-claimed id admitted here becomes a row rp-status promotes (it + // trusts self-managed rows by mode) against a signer that can never sign. + // A retryable error is recoverable; that registration is not. + // - placeholder unset and pre-registration never enabled -> skip. No claims + // exist, so there is nothing to recognise. This is every environment that + // has not run the tool. + // + // OPERATIONAL INVARIANT: once pre-registration has been run in an + // environment, RP_ID_PRE_REGISTRATION_SIGNER must stay set there forever. + // The claims outlive the flag. const primaryConfig = getRpRegistryConfig(); const placeholderSigner = process.env.RP_ID_PRE_REGISTRATION_SIGNER; - if (primaryConfig && placeholderSigner) { + const preRegistrationEnabled = + process.env.ENABLE_RP_ID_PRE_REGISTRATION === "true"; + const canRecognisePreClaims = Boolean( + primaryConfig && + placeholderSigner && + isAddress(placeholderSigner) && + !isZeroAddress(placeholderSigner), + ); + + if (!canRecognisePreClaims && preRegistrationEnabled) { + logger.error( + "Pre-registration is enabled but its placeholder signer is unusable", + { app_id, hasConfig: Boolean(primaryConfig) }, + ); + return errorHasuraQuery({ + req, + detail: "RP Registry is not configured correctly.", + code: "config_error", + app_id, + }); + } + + if (canRecognisePreClaims) { + let onChain; try { - const onChain = await getRpFromContract( + onChain = await getRpFromContract( parseRpId(rpIdString), - primaryConfig.contractAddress, + primaryConfig!.contractAddress, ); - if ( - onChain.initialized && - addressesEqual(onChain.signer, placeholderSigner) - ) { - logger.warn("Self-managed rp_id is held by a Portal pre-claim", { - app_id, - rpIdString, - onChainManager: onChain.manager, - }); - return errorHasuraQuery({ - req, - detail: - "This app's RP ID is held by Portal and cannot be self-managed yet — contact support.", - code: "rp_id_taken", - app_id, - }); - } } catch (error) { - // A read failure must not block onboarding: the common case is an id the - // developer just registered themselves. - logger.warn("Could not pre-check on-chain RP ownership; continuing", { + logger.warn("Could not read on-chain RP state for a self-managed id", { error, app_id, rpIdString, }); + return errorHasuraQuery({ + req, + detail: + "Could not verify this app's RP ID on-chain. Please try again.", + code: "rpc_error", + app_id, + }); + } + + if ( + onChain.initialized && + addressesEqual(onChain.signer, placeholderSigner!) + ) { + logger.warn("Self-managed rp_id is held by a Portal pre-claim", { + app_id, + rpIdString, + onChainManager: onChain.manager, + }); + return errorHasuraQuery({ + req, + detail: + "This app's RP ID is held by Portal and cannot be self-managed yet — contact support.", + code: "rp_id_taken", + app_id, + }); } } diff --git a/web/tests/api/hasura/register-rp.test.ts b/web/tests/api/hasura/register-rp.test.ts index 02440e613..5782bfb18 100644 --- a/web/tests/api/hasura/register-rp.test.ts +++ b/web/tests/api/hasura/register-rp.test.ts @@ -91,6 +91,7 @@ beforeEach(() => { process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = "arn:aws:kms:eu-west-1:000000000000:key/shared-manager"; process.env.RP_ID_PRE_REGISTRATION_SIGNER = placeholderSigner; + delete process.env.ENABLE_RP_ID_PRE_REGISTRATION; appIsStaging = false; authorizedTeam = [{ id: teamId }]; @@ -244,12 +245,42 @@ describe("/api/hasura/register-rp [self-managed on-chain ownership]", () => { expect(resolveManagerAddressMock).not.toHaveBeenCalled(); }); - it("still succeeds when the on-chain read fails", async () => { + it("returns a retryable error when the chain cannot be read and pre-claims are possible", async () => { + // Admitting a pre-claimed id here creates a row that rp-status promotes (it + // trusts self-managed rows by mode) against a signer that can never sign. + // A retryable error is recoverable; that registration is not. + getRpFromContractMock.mockRejectedValue(new Error("rpc timeout")); + + const res = (await selfManaged())!; + const body = await res.json(); + + expect(body.code ?? body.extensions?.code).toBe("rpc_error"); + }); + + it("skips the check entirely where pre-claims cannot exist", async () => { + // No placeholder configured and pre-registration never enabled: there is + // nothing to recognise, so a read failure must not block onboarding. This is + // every environment that has not run the tool. + delete process.env.RP_ID_PRE_REGISTRATION_SIGNER; + delete process.env.ENABLE_RP_ID_PRE_REGISTRATION; getRpFromContractMock.mockRejectedValue(new Error("rpc timeout")); const res = (await selfManaged())!; expect(res.status).toBe(200); + expect(getRpFromContractMock).not.toHaveBeenCalled(); + }); + + it("refuses to proceed when pre-registration is on but its signer is unusable", async () => { + // Claims are being made and we cannot recognise them — a deploy fault, not + // something a developer should absorb as a broken registration. + process.env.ENABLE_RP_ID_PRE_REGISTRATION = "true"; + process.env.RP_ID_PRE_REGISTRATION_SIGNER = `0x${"0".repeat(40)}`; + + const res = (await selfManaged())!; + const body = await res.json(); + + expect(body.code ?? body.extensions?.code).toBe("config_error"); }); }); // #endregion diff --git a/web/tests/api/helpers/rp-registration-flows.test.ts b/web/tests/api/helpers/rp-registration-flows.test.ts index af91377c5..deacfc0a8 100644 --- a/web/tests/api/helpers/rp-registration-flows.test.ts +++ b/web/tests/api/helpers/rp-registration-flows.test.ts @@ -1172,6 +1172,32 @@ describe("submitManagedRpRegistration [rp_id collision guard]", () => { expect(createManagerKey as jest.Mock).not.toHaveBeenCalled(); }); + it("returns a retryable kms_error, not rp_id_taken, when our manager cannot be resolved", async () => { + // A KMS outage must not be reported as "someone else owns your id, contact + // support" — that sends developers of pre-claimed apps to support over a + // transient failure they could just retry. + process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = sharedManagerKeyArn; + (getEthAddressFromKMS as jest.Mock).mockRejectedValue( + new Error("kms unavailable"), + ); + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: managerAddress, + signer: "0x000000000000000000000000000000000000dead", + }); + + const res = await register(); + + expect(res).toMatchObject({ ok: false, code: "kms_error" }); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + expect(submitRotateSignerTransactionMock).not.toHaveBeenCalled(); + // The slot is released so a retry is not answered with already_registered. + expect(DeleteRpRegistration).toHaveBeenCalledWith({ + rp_id: expect.stringMatching(/^rp_/), + }); + }); + it("still refuses a foreign claim when the shared manager key is configured", async () => { // The manager is what distinguishes our own claim from a squatter's, so the // adoption path must not widen into "any initialized RP is ours". From 84d26cee631c1474707863ee0b6c56956b0b1121 Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 13:00:44 +0200 Subject: [PATCH 06/12] fix(world-id): claim and adopt the staging registry alongside production MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A managed registration mirrors onto the staging registry on production deployments, but the defensive sweep only claimed the primary contract. That left the same predictable rp_id free on the staging side for a squatter to take, which then makes the real migration's staging registration revert with IdAlreadyInUse and records staging `failed` — and the endpoint had already reported the app as `claimed`, so the gap was invisible. The sweep now walks both configured registries and reports per-registry outcomes, so a half-claimed app shows up as such in the counts rather than collapsing into a single `claimed`. The staging mirror in submitManagedRpRegistration gained the same adoption branch as production, decided per contract because the two registries are claimed independently and can legitimately disagree. Raised as a P2 by Codex. Co-Authored-By: Claude --- web/api/_pre-register-rp-ids/index.ts | 188 ++++++++++++++-------- web/api/helpers/rp-registration-flows.ts | 44 +++-- web/tests/api/pre-register-rp-ids.test.ts | 74 ++++++++- 3 files changed, 221 insertions(+), 85 deletions(-) diff --git a/web/api/_pre-register-rp-ids/index.ts b/web/api/_pre-register-rp-ids/index.ts index a178f4382..c21c7e906 100644 --- a/web/api/_pre-register-rp-ids/index.ts +++ b/web/api/_pre-register-rp-ids/index.ts @@ -5,8 +5,10 @@ import { getKMSClient } from "@/api/helpers/kms"; import { resolveManagerAddress } from "@/api/helpers/rp-manager"; import { submitRegisterRpTransaction } from "@/api/helpers/rp-transactions"; import { + addressesEqual, generateRpIdString, getRpRegistryConfig, + getStagingRpRegistryConfig, isZeroAddress, parseRpId, } from "@/api/helpers/rp-utils"; @@ -183,8 +185,20 @@ export async function POST(request: NextRequest) { ); } + // Only populated on production deployments, matching the managed registration + // flow's own staging mirror condition. + const stagingConfig = + process.env.NEXT_PUBLIC_APP_ENV === "production" + ? getStagingRpRegistryConfig() + : null; + const client = await getAPIServiceGraphqlClient(); - const results: { app_id: string; outcome: Outcome; rp_id?: string }[] = []; + const results: { + app_id: string; + registry?: "production" | "staging"; + outcome: Outcome; + rp_id?: string; + }[] = []; for (const appId of appIds) { const rpIdString = generateRpIdString(appId); @@ -220,85 +234,121 @@ export async function POST(request: NextRequest) { continue; } - let onChain; - try { - onChain = await getRpFromContract(rpId, config.contractAddress); - } catch (error) { - // Never claim on a failed read: `register()` reverts with IdAlreadyInUse - // if the id is taken, so a blind submission burns gas and, worse, a read - // failure is indistinguishable from "free" here. - logger.warn("Could not read on-chain RP state; skipping", { - error, - app_id: appId, - rpIdString, - }); - results.push({ app_id: appId, outcome: "failed_rpc", rp_id: rpIdString }); - continue; - } + // Both registries have to be claimed. A managed registration mirrors onto the + // staging registry on production deployments, so leaving that side free lets a + // squatter take it and make the later migration's staging registration fail — + // and this endpoint would still have reported the app as `claimed`. + const registries = [ + { label: "production" as const, config }, + ...(stagingConfig + ? [{ label: "staging" as const, config: stagingConfig }] + : []), + ]; - if (onChain.initialized) { - const isOurs = - onChain.manager?.toLowerCase() === managerAddress.toLowerCase(); - if (!isOurs) { - // Already squatted. Nothing this endpoint can do — the contract has no - // reclaim path — but it is the single most important thing to surface. - logger.error("rp_id is already held by a foreign manager", { + for (const registry of registries) { + let onChain; + try { + onChain = await getRpFromContract( + rpId, + registry.config.contractAddress, + ); + } catch (error) { + // Never claim on a failed read: `register()` reverts with IdAlreadyInUse + // if the id is taken, so a blind submission burns gas and, worse, a read + // failure is indistinguishable from "free" here. + logger.warn("Could not read on-chain RP state; skipping", { + error, app_id: appId, rpIdString, - onChainManager: onChain.manager, - onChainSigner: onChain.signer, + registry: registry.label, + }); + results.push({ + app_id: appId, + registry: registry.label, + outcome: "failed_rpc", + rp_id: rpIdString, }); + continue; } - results.push({ - app_id: appId, - outcome: isOurs - ? "skipped_already_claimed_by_us" - : "skipped_taken_by_foreign_manager", - rp_id: rpIdString, - }); - continue; - } - if (dryRun) { - results.push({ - app_id: appId, - outcome: "would_claim", - rp_id: rpIdString, - }); - continue; - } + if (onChain.initialized) { + const isOurs = addressesEqual(onChain.manager, managerAddress); + if (!isOurs) { + // Already squatted. Nothing this endpoint can do — the contract has no + // reclaim path — but it is the single most important thing to surface. + logger.error("rp_id is already held by a foreign manager", { + app_id: appId, + rpIdString, + registry: registry.label, + onChainManager: onChain.manager, + onChainSigner: onChain.signer, + }); + } + results.push({ + app_id: appId, + registry: registry.label, + outcome: isOurs + ? "skipped_already_claimed_by_us" + : "skipped_taken_by_foreign_manager", + rp_id: rpIdString, + }); + continue; + } - try { - const kmsClient = await getKMSClient(config.kmsRegion); - const operationHash = await submitRegisterRpTransaction(config, { - rpId, - managerAddress, - signerAddress: placeholderSigner, - appName: appInfo.app_metadata?.[0]?.name || "", - kmsClient, - }); - logger.info("Claimed rp_id defensively", { - app_id: appId, - rpIdString, - operationHash, - }); - results.push({ app_id: appId, outcome: "claimed", rp_id: rpIdString }); - } catch (error) { - logger.error("Failed to claim rp_id", { - error, - app_id: appId, - rpIdString, - }); - results.push({ - app_id: appId, - outcome: "failed_submission", - rp_id: rpIdString, - }); + if (dryRun) { + results.push({ + app_id: appId, + registry: registry.label, + outcome: "would_claim", + rp_id: rpIdString, + }); + continue; + } + + try { + const kmsClient = await getKMSClient(registry.config.kmsRegion); + const operationHash = await submitRegisterRpTransaction( + registry.config, + { + rpId, + managerAddress, + signerAddress: placeholderSigner, + appName: appInfo.app_metadata?.[0]?.name || "", + kmsClient, + }, + ); + logger.info("Claimed rp_id defensively", { + app_id: appId, + rpIdString, + registry: registry.label, + operationHash, + }); + results.push({ + app_id: appId, + registry: registry.label, + outcome: "claimed", + rp_id: rpIdString, + }); + } catch (error) { + logger.error("Failed to claim rp_id", { + error, + app_id: appId, + rpIdString, + registry: registry.label, + }); + results.push({ + app_id: appId, + registry: registry.label, + outcome: "failed_submission", + rp_id: rpIdString, + }); + } } } const counts = results.reduce>((acc, r) => { - acc[r.outcome] = (acc[r.outcome] ?? 0) + 1; + const key = r.registry ? `${r.registry}:${r.outcome}` : r.outcome; + acc[key] = (acc[key] ?? 0) + 1; return acc; }, {}); diff --git a/web/api/helpers/rp-registration-flows.ts b/web/api/helpers/rp-registration-flows.ts index ff3cf87e4..dd65e844f 100644 --- a/web/api/helpers/rp-registration-flows.ts +++ b/web/api/helpers/rp-registration-flows.ts @@ -416,16 +416,42 @@ export async function submitManagedRpRegistration({ const stagingConfig = getStagingRpRegistryConfig(); if (stagingConfig) { try { - stagingOperationHash = await submitRegisterRpTransaction( - stagingConfig, - { + // The staging mirror needs the same adoption branch as production: if the + // defensive sweep claimed this rp_id on the staging registry too, a + // register() here reverts with IdAlreadyInUse and staging is recorded + // `failed`, leaving the developer unable to use staging actions. Decided + // per-contract because the two registries are claimed independently and + // can disagree. + let adoptStagingClaim = false; + try { + const existingStagingRp = await getRpFromContract( rpId, - managerAddress, - signerAddress, - appName, - kmsClient, - }, - ); + stagingConfig.contractAddress, + ); + adoptStagingClaim = + existingStagingRp.initialized && + addressesEqual(existingStagingRp.manager, managerAddress); + } catch (error) { + logger.warn("Could not pre-check the staging rp_id; registering", { + error, + rpIdString, + }); + } + + stagingOperationHash = adoptStagingClaim + ? await submitRotateSignerTransaction(stagingConfig, { + rpId, + newSignerAddress: signerAddress, + managerKmsKeyId, + kmsClient, + }) + : await submitRegisterRpTransaction(stagingConfig, { + rpId, + managerAddress, + signerAddress, + appName, + kmsClient, + }); stagingStatus = RpRegistrationStatus.Pending; logger.info("Staging registration submitted", { rpIdString, diff --git a/web/tests/api/pre-register-rp-ids.test.ts b/web/tests/api/pre-register-rp-ids.test.ts index 9f555cd57..b78137406 100644 --- a/web/tests/api/pre-register-rp-ids.test.ts +++ b/web/tests/api/pre-register-rp-ids.test.ts @@ -38,11 +38,13 @@ jest.mock("@/api/helpers/temporal-rpc", () => ({ })); const getRpRegistryConfigMock = jest.fn(); +const getStagingRpRegistryConfigMock = jest.fn(); jest.mock("@/api/helpers/rp-utils", () => { const actual = jest.requireActual("@/api/helpers/rp-utils"); return { ...actual, getRpRegistryConfig: () => getRpRegistryConfigMock(), + getStagingRpRegistryConfig: () => getStagingRpRegistryConfigMock(), }; }); @@ -87,6 +89,8 @@ beforeEach(() => { contractAddress: "0xcontract", kmsRegion: "eu-west-1", }); + delete process.env.NEXT_PUBLIC_APP_ENV; + getStagingRpRegistryConfigMock.mockReturnValue(null); resolveManagerAddressMock.mockResolvedValue(managerAddress); GetAppInfo.mockResolvedValue({ app: [{ id: appId, is_staging: false, app_metadata: [{ name: "Test" }] }], @@ -164,7 +168,7 @@ describe("/api/_pre-register-rp-ids [dry run]", () => { expect(res.status).toBe(200); const body = await res.json(); expect(body.dry_run).toBe(true); - expect(body.counts).toEqual({ would_claim: 1 }); + expect(body.counts).toEqual({ "production:would_claim": 1 }); expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); }); @@ -173,7 +177,7 @@ describe("/api/_pre-register-rp-ids [dry run]", () => { expect(res.status).toBe(200); const body = await res.json(); - expect(body.counts).toEqual({ claimed: 1 }); + expect(body.counts).toEqual({ "production:claimed": 1 }); expect(submitRegisterRpTransactionMock).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ @@ -221,7 +225,9 @@ describe("/api/_pre-register-rp-ids [skips]", () => { const body = await (await run()).json(); - expect(body.counts).toEqual({ skipped_already_claimed_by_us: 1 }); + expect(body.counts).toEqual({ + "production:skipped_already_claimed_by_us": 1, + }); expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); }); @@ -235,7 +241,9 @@ describe("/api/_pre-register-rp-ids [skips]", () => { const body = await (await run()).json(); - expect(body.counts).toEqual({ skipped_taken_by_foreign_manager: 1 }); + expect(body.counts).toEqual({ + "production:skipped_taken_by_foreign_manager": 1, + }); expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); }); @@ -246,7 +254,7 @@ describe("/api/_pre-register-rp-ids [skips]", () => { const body = await (await run()).json(); - expect(body.counts).toEqual({ failed_rpc: 1 }); + expect(body.counts).toEqual({ "production:failed_rpc": 1 }); expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); }); @@ -257,10 +265,59 @@ describe("/api/_pre-register-rp-ids [skips]", () => { const res = await post({ app_ids: [appId, appId], dry_run: false }); const body = await res.json(); - expect(body.counts).toEqual({ claimed: 1 }); + expect(body.counts).toEqual({ "production:claimed": 1 }); expect(submitRegisterRpTransactionMock).toHaveBeenCalledTimes(1); }); + it("claims the staging registry too when it is configured", async () => { + // A managed registration mirrors onto staging on production deployments, so + // leaving that side free lets a squatter take it and make the later + // migration's staging registration fail — while this endpoint still reported + // the app as claimed. + process.env.NEXT_PUBLIC_APP_ENV = "production"; + getStagingRpRegistryConfigMock.mockReturnValue({ + contractAddress: "0xstagingcontract", + kmsRegion: "eu-west-1", + }); + + const body = await (await run()).json(); + + expect(body.counts).toEqual({ + "production:claimed": 1, + "staging:claimed": 1, + }); + expect(submitRegisterRpTransactionMock).toHaveBeenCalledTimes(2); + const contracts = submitRegisterRpTransactionMock.mock.calls.map( + (call) => call[0].contractAddress, + ); + expect(contracts).toEqual(["0xcontract", "0xstagingcontract"]); + }); + + it("reports the registries separately when only one side is free", async () => { + // Half-claimed must be visible in the counts rather than collapsing into a + // single "claimed". + process.env.NEXT_PUBLIC_APP_ENV = "production"; + getStagingRpRegistryConfigMock.mockReturnValue({ + contractAddress: "0xstagingcontract", + kmsRegion: "eu-west-1", + }); + getRpFromContractMock.mockImplementation( + async (_rpId: bigint, contractAddress: string) => ({ + initialized: contractAddress === "0xstagingcontract", + active: contractAddress === "0xstagingcontract", + manager: "0x00000000000000000000000000000000000000ff", + signer: "0x00000000000000000000000000000000000000ee", + }), + ); + + const body = await (await run()).json(); + + expect(body.counts).toEqual({ + "production:claimed": 1, + "staging:skipped_taken_by_foreign_manager": 1, + }); + }); + it("records a submission failure without aborting the rest of the batch", async () => { const secondAppId = "app_00000000000000000000000000000002"; submitRegisterRpTransactionMock @@ -273,7 +330,10 @@ describe("/api/_pre-register-rp-ids [skips]", () => { }); const body = await res.json(); - expect(body.counts).toEqual({ failed_submission: 1, claimed: 1 }); + expect(body.counts).toEqual({ + "production:failed_submission": 1, + "production:claimed": 1, + }); }); }); // #endregion From 8576ea905dcc07405d3ace668b46e68825c49a5b Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 13:13:18 +0200 Subject: [PATCH 07/12] fix(world-id): require a real JSON boolean to opt out of the dry run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit yup cast the string "false" to boolean false, so a hand-built curl or wrapper script could make this endpoint spend real gas without ever passing a JSON boolean — and explicit opt-out is the whole safety property. Not fixed with .strict(): yup applies defaults during casting, which strict mode skips, so .strict().default(true) leaves an ABSENT dry_run as undefined and therefore falsy. That inverts the guarantee instead of tightening it — the local test suite caught it immediately. The check now runs on the raw body before yup sees it, and a test pins the omitted case so the same trap cannot be reintroduced. Also drops a duplicated foreign-manager block the previous rebase left behind. It sat after the adoption branch and returned rp_id_taken unconditionally, so adoption could never take effect — caught by the adoption test failing. Rebased onto #2221's slot-release fix; the release is now a shared helper used by both the kms_error and rp_id_taken paths. Co-Authored-By: Claude --- web/api/_pre-register-rp-ids/index.ts | 20 ++++++++++++++++ web/api/helpers/rp-registration-flows.ts | 29 ----------------------- web/tests/api/pre-register-rp-ids.test.ts | 20 ++++++++++++++++ 3 files changed, 40 insertions(+), 29 deletions(-) diff --git a/web/api/_pre-register-rp-ids/index.ts b/web/api/_pre-register-rp-ids/index.ts index c21c7e906..ba76b8d93 100644 --- a/web/api/_pre-register-rp-ids/index.ts +++ b/web/api/_pre-register-rp-ids/index.ts @@ -39,6 +39,12 @@ const schema = yup /** * Defaults to true: the only way to spend gas is to ask for it explicitly. * A dry run reports exactly what a real run would submit. + * + * NOT `.strict()`, deliberately — yup applies defaults during casting, which + * strict mode skips, so `.strict().default(true)` would leave an ABSENT + * dry_run as undefined and therefore falsy. That inverts the safety property. + * Non-boolean values are rejected by an explicit check on the raw body + * instead; see below. */ dry_run: yup.boolean().default(true), }) @@ -102,6 +108,20 @@ export async function POST(request: NextRequest) { } const body = await request.json().catch(() => null); + + // Checked on the RAW body, before yup casts. Opting out of the dry run is the + // only thing that lets this endpoint spend gas, so it has to be an actual JSON + // boolean — yup would happily coerce the string "false", which a hand-built + // curl or wrapper script produces by accident. + const rawDryRun = (body as { dry_run?: unknown } | null)?.dry_run; + if (rawDryRun !== undefined && typeof rawDryRun !== "boolean") { + logger.warn("Rejected a non-boolean dry_run", { type: typeof rawDryRun }); + return NextResponse.json( + { error: "dry_run must be a JSON boolean." }, + { status: 400 }, + ); + } + const { isValid, parsedParams } = await validateRequestSchema({ value: body, schema, diff --git a/web/api/helpers/rp-registration-flows.ts b/web/api/helpers/rp-registration-flows.ts index dd65e844f..d00ef641d 100644 --- a/web/api/helpers/rp-registration-flows.ts +++ b/web/api/helpers/rp-registration-flows.ts @@ -245,35 +245,6 @@ export async function submitManagedRpRegistration({ }); } - if (existingOnChainRp?.initialized) { - logger.warn("rp_id already registered on-chain by a foreign manager", { - app_id: appId, - rpIdString, - onChainManager: existingOnChainRp.manager, - onChainSigner: existingOnChainRp.signer, - }); - // Release the slot: the app is not registered, and holding the row would - // make every later attempt report `already_registered` instead. A failure - // here leaves the row wedged, which is an ops problem — but the id really is - // taken, so that stays the answer either way, and continuing is not an - // option. - try { - await getDeleteRpSdk(client).DeleteRpRegistration({ rp_id: rpIdString }); - } catch (error) { - logger.error("Failed to release the slot for a taken rp_id", { - error, - app_id: appId, - rpIdString, - }); - } - return { - ok: false, - code: "rp_id_taken", - detail: - "This app's RP ID is already registered on-chain by another party. Portal cannot manage it — contact support.", - }; - } - // is_unique_manager_key is written together with the manager key at the end // of this flow. If the migration adding it has not been applied yet, fail // here rather than after the on-chain transaction has been submitted. diff --git a/web/tests/api/pre-register-rp-ids.test.ts b/web/tests/api/pre-register-rp-ids.test.ts index b78137406..39ced5705 100644 --- a/web/tests/api/pre-register-rp-ids.test.ts +++ b/web/tests/api/pre-register-rp-ids.test.ts @@ -146,6 +146,26 @@ describe("/api/_pre-register-rp-ids [guards]", () => { expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); }); + it('rejects the string "false" for dry_run rather than spending gas on it', async () => { + // yup would otherwise cast it to boolean false. Opting out of the dry run has + // to be an explicit JSON boolean — that is the whole safety property. + const res = await post({ app_ids: [appId], dry_run: "false" }); + + expect(res.status).toBe(400); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("still dry-runs when dry_run is omitted entirely", async () => { + // Guards against fixing the cast by making the field strict, which would skip + // yup's default and leave an absent dry_run undefined — falsy, so it would + // spend gas. Exactly backwards. + const res = await post({ app_ids: [appId] }); + const body = await res.json(); + + expect(body.dry_run).toBe(true); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + it("rejects a batch larger than the per-call ceiling", async () => { const tooMany = Array.from( { length: 26 }, From 9630f600c443c1237df707c58a99cbe79d25ca43 Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 13:21:27 +0200 Subject: [PATCH 08/12] fix(world-id): decide the manager key from both registries, not just production MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sweep can succeed on staging and fail on production. A later managed registration in dedicated-key mode then minted a dedicated manager for production and only afterwards compared staging against it — so a staging rp_id already held by our shared pre-claim looked foreign, register() reverted on an initialized id, and staging was left permanently failed. Every subsequent staging status check and retry compares against the dedicated key recorded on the row and reads the shared pre-claim as foreign too, so it never recovers. The row stores ONE manager_kms_key_id, so a pre-claim on either registry has to decide the key for both. Staging is now read before the key is chosen, and a staging-only pre-claim forces the shared key just as a production one does. Both adoption flags are then just consulted at submission time. Only production remains authoritative for whether the app can register at all — a foreign staging claim means staging cannot mirror, not that onboarding is blocked. Verified the new test fails without the fix and passes with it, rather than assuming it discriminates. Raised as a P2 by Codex. Co-Authored-By: Claude --- web/api/helpers/rp-registration-flows.ts | 75 ++++++++++++------- .../api/helpers/rp-registration-flows.test.ts | 35 +++++++++ 2 files changed, 85 insertions(+), 25 deletions(-) diff --git a/web/api/helpers/rp-registration-flows.ts b/web/api/helpers/rp-registration-flows.ts index d00ef641d..9e514cd7a 100644 --- a/web/api/helpers/rp-registration-flows.ts +++ b/web/api/helpers/rp-registration-flows.ts @@ -197,8 +197,38 @@ export async function submitManagedRpRegistration({ // - manager unresolvable -> kms_error, RETRYABLE. A KMS outage must not be // reported as "someone else owns your id, contact support"; that would send // developers of pre-claimed apps to support over a transient failure. + // Staging is read here too, not later at submission time. The row stores ONE + // manager_kms_key_id, so a pre-claim on EITHER registry has to decide the key + // for both. Deciding staging after a dedicated key was already minted for + // production leaves staging permanently failed: its register() reverts on an + // initialized id, and every later staging status/retry compares against the + // dedicated key and reads the shared pre-claim as foreign. + const stagingConfigForAdoption = + process.env.NEXT_PUBLIC_APP_ENV === "production" + ? getStagingRpRegistryConfig() + : null; + let existingStagingRp: Awaited> | null = + null; + if (stagingConfigForAdoption) { + try { + existingStagingRp = await getRpFromContract( + rpId, + stagingConfigForAdoption.contractAddress, + ); + } catch (error) { + // Non-fatal for the same reason as production: staging is a best-effort + // mirror, so a failed read falls through to registering. + logger.warn("Could not pre-check the staging rp_id; registering", { + error, + app_id: appId, + rpIdString, + }); + } + } + let adoptExistingClaim = false; - if (existingOnChainRp?.initialized) { + let adoptStagingClaim = false; + if (existingOnChainRp?.initialized || existingStagingRp?.initialized) { const sharedManagerKeyId = process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; const ourManagerAddress = sharedManagerKeyId ? await resolveManagerAddress(sharedManagerKeyId, primaryConfig.kmsRegion) @@ -217,11 +247,22 @@ export async function submitManagedRpRegistration({ }; } + adoptStagingClaim = Boolean( + existingStagingRp?.initialized && + ourManagerAddress && + addressesEqual(existingStagingRp.manager, ourManagerAddress), + ); + adoptExistingClaim = Boolean( - ourManagerAddress && + existingOnChainRp?.initialized && + ourManagerAddress && addressesEqual(existingOnChainRp.manager, ourManagerAddress), ); + } + // Only the PRODUCTION registry is authoritative for whether this app can be + // registered at all; a foreign staging claim just means staging cannot mirror. + if (existingOnChainRp?.initialized) { if (!adoptExistingClaim) { logger.warn("rp_id already registered on-chain by a foreign manager", { app_id: appId, @@ -288,9 +329,11 @@ export async function submitManagedRpRegistration({ // Adoption has no choice of key: the manager already recorded on-chain is the // shared one we claimed with, and only that key can sign the rotation. Minting // a dedicated key here would produce a manager the contract has never heard - // of, and every update would revert. + // of, and every update would revert. A staging-only pre-claim forces it too — + // the row records one manager key for both registries. const useSharedManagerKey = adoptExistingClaim || + adoptStagingClaim || process.env.ENABLE_SHARED_KEY_RP_REGISTRATION === "true"; if (useSharedManagerKey) { @@ -387,28 +430,10 @@ export async function submitManagedRpRegistration({ const stagingConfig = getStagingRpRegistryConfig(); if (stagingConfig) { try { - // The staging mirror needs the same adoption branch as production: if the - // defensive sweep claimed this rp_id on the staging registry too, a - // register() here reverts with IdAlreadyInUse and staging is recorded - // `failed`, leaving the developer unable to use staging actions. Decided - // per-contract because the two registries are claimed independently and - // can disagree. - let adoptStagingClaim = false; - try { - const existingStagingRp = await getRpFromContract( - rpId, - stagingConfig.contractAddress, - ); - adoptStagingClaim = - existingStagingRp.initialized && - addressesEqual(existingStagingRp.manager, managerAddress); - } catch (error) { - logger.warn("Could not pre-check the staging rp_id; registering", { - error, - rpIdString, - }); - } - + // adoptStagingClaim was decided before the manager key was chosen — see + // there for why. If the sweep claimed this rp_id on staging, a register() + // here would revert with IdAlreadyInUse and record staging `failed`, + // leaving the developer unable to use staging actions. stagingOperationHash = adoptStagingClaim ? await submitRotateSignerTransaction(stagingConfig, { rpId, diff --git a/web/tests/api/helpers/rp-registration-flows.test.ts b/web/tests/api/helpers/rp-registration-flows.test.ts index deacfc0a8..b2e25f13b 100644 --- a/web/tests/api/helpers/rp-registration-flows.test.ts +++ b/web/tests/api/helpers/rp-registration-flows.test.ts @@ -1216,6 +1216,41 @@ describe("submitManagedRpRegistration [rp_id collision guard]", () => { expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); }); + it("uses the shared key when only STAGING was pre-claimed", async () => { + // The sweep can succeed on staging and not production. The row stores one + // manager key, so minting a dedicated one for production would leave staging + // permanently failed: its register() reverts on an initialized id, and every + // later staging status/retry compares against the dedicated key and reads the + // shared pre-claim as foreign. + process.env.NEXT_PUBLIC_APP_ENV = "production"; + process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = sharedManagerKeyArn; + mockGetStagingRpRegistryConfig.mockReturnValue({ + contractAddress: "0xstagingcontract", + kmsRegion: "us-east-1", + }); + getRpFromContractMock.mockImplementation( + async (_rpId: bigint, contractAddress: string) => ({ + // Free on production, already ours on staging. + initialized: contractAddress === "0xstagingcontract", + active: contractAddress === "0xstagingcontract", + manager: managerAddress, + signer: "0x000000000000000000000000000000000000dead", + }), + ); + + const res = await register(); + + expect(res).toMatchObject({ ok: true }); + // Shared key, not a dedicated one, even though shared mode is off. + expect(createManagerKey as jest.Mock).not.toHaveBeenCalled(); + // Production registers (it was free); staging rotates (already ours). + expect(submitRegisterRpTransactionMock).toHaveBeenCalledTimes(1); + expect(submitRotateSignerTransactionMock).toHaveBeenCalledWith( + expect.objectContaining({ contractAddress: "0xstagingcontract" }), + expect.objectContaining({ newSignerAddress: signerAddress }), + ); + }); + it("still reports rp_id_taken when releasing the slot fails", async () => { // The read's non-fatal catch must not extend over the slot release: swallowing // that failure would fall through to KMS and a UserOp for an rp_id already From 45fa93ca915a0f9cad31ed2e9cfe2c4ef6b684a8 Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 13:30:10 +0200 Subject: [PATCH 09/12] fix(world-id): don't resubmit settling claims, don't guess staging ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Codex findings on the pre-registration path. An operator repeating a real run before the previous UserOp mined resubmitted: submitRegisterRpTransaction returns on submission, the on-chain read still shows the id as free, and the UserOp nonce carries per-attempt randomness — so both can be accepted and one later reverts, burning gas. The payload-level dedupe never covered this. A Redis marker now reserves each rp_id per registry for the UserOp validity window plus margin, mirroring the throttle in rp-status, and reports skipped_claim_in_flight. Fails OPEN without Redis: this is an operator tool that dry-runs by default, so refusing to work without a cache would be worse than the duplicate it prevents, and the on-chain read still catches anything mined. The staging pre-check's non-fatal catch left ownership as "not ours". With dedicated keys that mints a dedicated manager, staging's register() then reverts on an initialized id, and every later staging status check and retry compares against the dedicated key and reads the shared pre-claim as foreign — that side never recovers. Unknown now forces the shared key, which is always valid for a fresh registration. Scoped to environments that actually pre-claim (they keep RP_ID_PRE_REGISTRATION_SIGNER set), so dedicated keys keep their isolation everywhere else, and a staging RPC blip still never blocks a production registration. Both new tests were checked against the un-fixed code first to confirm they actually fail without the change. Co-Authored-By: Claude --- web/api/_pre-register-rp-ids/index.ts | 59 +++++++++++++++++++ web/api/helpers/rp-registration-flows.ts | 18 +++++- .../api/helpers/rp-registration-flows.test.ts | 37 ++++++++++++ web/tests/api/pre-register-rp-ids.test.ts | 19 +++++- 4 files changed, 130 insertions(+), 3 deletions(-) diff --git a/web/api/_pre-register-rp-ids/index.ts b/web/api/_pre-register-rp-ids/index.ts index ba76b8d93..a58fa278e 100644 --- a/web/api/_pre-register-rp-ids/index.ts +++ b/web/api/_pre-register-rp-ids/index.ts @@ -13,6 +13,7 @@ import { parseRpId, } from "@/api/helpers/rp-utils"; import { getRpFromContract } from "@/api/helpers/temporal-rpc"; +import { USER_OP_MAX_VALIDITY_MS } from "@/api/helpers/user-operation"; import { protectInternalEndpoint } from "@/api/helpers/utils"; import { validateRequestSchema } from "@/api/helpers/validate-request-schema"; import { logger } from "@/lib/logger"; @@ -28,6 +29,47 @@ import * as yup from "yup"; */ const MAX_APPS_PER_CALL = 25; +/** + * How long a submitted claim is remembered so a repeat run does not resubmit it. + * `submitRegisterRpTransaction` returns once the UserOp is submitted, not mined, + * and the UserOp nonce carries per-attempt randomness — so a second register() + * for the same rp_id can be accepted concurrently and one of them later reverts, + * burning gas. Covers the UserOp validity window plus margin, after which the + * on-chain read is authoritative again. + */ +const CLAIM_IN_FLIGHT_TTL_SECONDS = Math.ceil( + (USER_OP_MAX_VALIDITY_MS + 5 * 60 * 1000) / 1000, +); +const CLAIM_IN_FLIGHT_KEY_PREFIX = "rp_claim_in_flight:"; + +/** + * Reserves an rp_id for this run. False means a claim was submitted recently and + * has not settled, so skip it. + * + * Fails OPEN when Redis is unavailable: the endpoint is operator-driven and + * dry-run by default, so refusing to work without Redis would be worse than the + * duplicate submission this guards against. The on-chain read still catches + * anything that has actually mined. + */ +async function reserveClaim(rpIdString: string): Promise { + const redis = global.RedisClient; + if (!redis) { + return true; + } + try { + const reserved = await redis.set( + `${CLAIM_IN_FLIGHT_KEY_PREFIX}${rpIdString}`, + "1", + "EX", + CLAIM_IN_FLIGHT_TTL_SECONDS, + "NX", + ); + return reserved === "OK"; + } catch { + return true; + } +} + const schema = yup .object({ app_ids: yup @@ -55,6 +97,7 @@ type Outcome = | "claimed" | "skipped_already_registered_in_portal" | "skipped_already_claimed_by_us" + | "skipped_claim_in_flight" | "skipped_taken_by_foreign_manager" | "skipped_staging" | "skipped_app_not_found" @@ -325,6 +368,22 @@ export async function POST(request: NextRequest) { continue; } + if (!(await reserveClaim(`${registry.label}:${rpIdString}`))) { + // A claim for this id was submitted recently and has not settled yet. + logger.warn("Skipping an rp_id with a claim still in flight", { + app_id: appId, + rpIdString, + registry: registry.label, + }); + results.push({ + app_id: appId, + registry: registry.label, + outcome: "skipped_claim_in_flight", + rp_id: rpIdString, + }); + continue; + } + try { const kmsClient = await getKMSClient(registry.config.kmsRegion); const operationHash = await submitRegisterRpTransaction( diff --git a/web/api/helpers/rp-registration-flows.ts b/web/api/helpers/rp-registration-flows.ts index 9e514cd7a..dbd2b3f1e 100644 --- a/web/api/helpers/rp-registration-flows.ts +++ b/web/api/helpers/rp-registration-flows.ts @@ -209,6 +209,7 @@ export async function submitManagedRpRegistration({ : null; let existingStagingRp: Awaited> | null = null; + let stagingOwnershipUnknown = false; if (stagingConfigForAdoption) { try { existingStagingRp = await getRpFromContract( @@ -216,8 +217,14 @@ export async function submitManagedRpRegistration({ stagingConfigForAdoption.contractAddress, ); } catch (error) { - // Non-fatal for the same reason as production: staging is a best-effort - // mirror, so a failed read falls through to registering. + // Non-fatal: staging is a best-effort mirror, so blocking a production + // registration on a staging RPC failure would be the wrong trade. But + // "unknown" must not silently become "not ours" — if a pre-claim exists on + // staging and we mint a dedicated key here, staging's register() reverts on + // an initialized id and every later staging status/retry reads the shared + // pre-claim as foreign, so that side never recovers. Force the shared key + // instead, which is always a valid choice for a fresh registration. + stagingOwnershipUnknown = true; logger.warn("Could not pre-check the staging rp_id; registering", { error, app_id: appId, @@ -331,9 +338,16 @@ export async function submitManagedRpRegistration({ // a dedicated key here would produce a manager the contract has never heard // of, and every update would revert. A staging-only pre-claim forces it too — // the row records one manager key for both registries. + // Scoped to environments that actually pre-claim: RP_ID_PRE_REGISTRATION_SIGNER + // stays configured once claims exist there (see _pre-register-rp-ids). Elsewhere + // an unreadable staging registry changes nothing, so dedicated keys keep their + // isolation. + const preClaimsPossible = Boolean(process.env.RP_ID_PRE_REGISTRATION_SIGNER); + const useSharedManagerKey = adoptExistingClaim || adoptStagingClaim || + (stagingOwnershipUnknown && preClaimsPossible) || process.env.ENABLE_SHARED_KEY_RP_REGISTRATION === "true"; if (useSharedManagerKey) { diff --git a/web/tests/api/helpers/rp-registration-flows.test.ts b/web/tests/api/helpers/rp-registration-flows.test.ts index b2e25f13b..7d26a0844 100644 --- a/web/tests/api/helpers/rp-registration-flows.test.ts +++ b/web/tests/api/helpers/rp-registration-flows.test.ts @@ -168,6 +168,7 @@ beforeEach(() => { process.env.NEXT_PUBLIC_APP_ENV = "test"; delete process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; delete process.env.ENABLE_SHARED_KEY_RP_REGISTRATION; + delete process.env.RP_ID_PRE_REGISTRATION_SIGNER; mockGetRpRegistryConfig.mockReturnValue({ contractAddress: "0xcontract", kmsRegion: "us-east-1", @@ -1251,6 +1252,42 @@ describe("submitManagedRpRegistration [rp_id collision guard]", () => { ); }); + it("uses the shared key when staging ownership cannot be determined", async () => { + // The staging read is best-effort, so it must not block a production + // registration — but "unknown" must not become "not ours" either. If staging + // is in fact pre-claimed and we mint a dedicated key, staging's register() + // reverts on an initialized id and every later staging status/retry reads the + // shared pre-claim as foreign, so that side never recovers. + process.env.NEXT_PUBLIC_APP_ENV = "production"; + process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = sharedManagerKeyArn; + // Set only where pre-registration has actually been run, which is what scopes + // this fallback. + process.env.RP_ID_PRE_REGISTRATION_SIGNER = + "0x000000000000000000000000000000000000dEaD"; + mockGetStagingRpRegistryConfig.mockReturnValue({ + contractAddress: "0xstagingcontract", + kmsRegion: "us-east-1", + }); + getRpFromContractMock.mockImplementation( + async (_rpId: bigint, contractAddress: string) => { + if (contractAddress === "0xstagingcontract") { + throw new Error("staging rpc timeout"); + } + return { + initialized: false, + active: false, + manager: `0x${"0".repeat(40)}`, + signer: `0x${"0".repeat(40)}`, + }; + }, + ); + + const res = await register(); + + expect(res).toMatchObject({ ok: true }); + expect(createManagerKey as jest.Mock).not.toHaveBeenCalled(); + }); + it("still reports rp_id_taken when releasing the slot fails", async () => { // The read's non-fatal catch must not extend over the slot release: swallowing // that failure would fall through to KMS and a UserOp for an rp_id already diff --git a/web/tests/api/pre-register-rp-ids.test.ts b/web/tests/api/pre-register-rp-ids.test.ts index 39ced5705..8dccce9a7 100644 --- a/web/tests/api/pre-register-rp-ids.test.ts +++ b/web/tests/api/pre-register-rp-ids.test.ts @@ -77,8 +77,11 @@ const post = async (body: unknown) => { }; // #endregion -beforeEach(() => { +beforeEach(async () => { jest.clearAllMocks(); + // The in-flight claim markers are real Redis keys; without this the second test + // to touch an rp_id would be skipped as still settling. + await global.RedisClient?.flushall(); process.env.INTERNAL_ENDPOINTS_SECRET = "test_secret"; process.env.ENABLE_RP_ID_PRE_REGISTRATION = "true"; process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = @@ -338,6 +341,20 @@ describe("/api/_pre-register-rp-ids [skips]", () => { }); }); + it("does not resubmit a claim that is still settling", async () => { + // submitRegisterRpTransaction returns on submission, not on mining, so a + // repeat run sees the id as still uninitialized. The UserOp nonce carries + // per-attempt randomness, so both can be accepted and one later reverts, + // burning gas. Dedupe within one payload does not cover this. + const first = await (await run()).json(); + expect(first.counts).toEqual({ "production:claimed": 1 }); + + const second = await (await run()).json(); + + expect(second.counts).toEqual({ "production:skipped_claim_in_flight": 1 }); + expect(submitRegisterRpTransactionMock).toHaveBeenCalledTimes(1); + }); + it("records a submission failure without aborting the rest of the batch", async () => { const secondAppId = "app_00000000000000000000000000000002"; submitRegisterRpTransactionMock From 8c255ccd4c95be5053b1e56bac85bc1115413cd5 Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 13:38:57 +0200 Subject: [PATCH 10/12] fix(world-id): give the in-flight claim marker a reader, and release it on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The marker added last round had no reader outside the claim tool, so a developer starting managed registration while an operator's claim was still settling saw the id as free and submitted a competing register(). If the pre-claim won on-chain with the shared manager while the row recorded a dedicated one, every later status check and retry read the shared claim as foreign and the registration never reconciled. Registration now checks the marker and asks the developer to retry — by then the claim is visible and the adoption path takes over. Reuses the retryable submission_error code rather than adding a new one to four error maps. Writer and reader now live in one module (rp-claims.ts) so the key format and TTL cannot drift between them. A marker nobody sees is exactly the failure this round was about. Also releases the reservation when getKMSClient or the submission throws before a UserOp is accepted. Otherwise a repeat run reported skipped_claim_in_flight for the full TTL on an operation that never existed, leaving the id unclaimed. (P3.) Both new tests were checked against the un-fixed code first; the first one passed initially only because it reserved the wrong key, which is why that check matters. Co-Authored-By: Claude --- web/api/_pre-register-rp-ids/index.ts | 49 +------- web/api/helpers/rp-claims.ts | 107 ++++++++++++++++++ web/api/helpers/rp-registration-flows.ts | 25 ++++ .../api/helpers/rp-registration-flows.test.ts | 28 ++++- web/tests/api/pre-register-rp-ids.test.ts | 15 +++ 5 files changed, 180 insertions(+), 44 deletions(-) create mode 100644 web/api/helpers/rp-claims.ts diff --git a/web/api/_pre-register-rp-ids/index.ts b/web/api/_pre-register-rp-ids/index.ts index a58fa278e..8fdf2d079 100644 --- a/web/api/_pre-register-rp-ids/index.ts +++ b/web/api/_pre-register-rp-ids/index.ts @@ -12,8 +12,8 @@ import { isZeroAddress, parseRpId, } from "@/api/helpers/rp-utils"; +import { releaseClaim, reserveClaim } from "@/api/helpers/rp-claims"; import { getRpFromContract } from "@/api/helpers/temporal-rpc"; -import { USER_OP_MAX_VALIDITY_MS } from "@/api/helpers/user-operation"; import { protectInternalEndpoint } from "@/api/helpers/utils"; import { validateRequestSchema } from "@/api/helpers/validate-request-schema"; import { logger } from "@/lib/logger"; @@ -29,47 +29,6 @@ import * as yup from "yup"; */ const MAX_APPS_PER_CALL = 25; -/** - * How long a submitted claim is remembered so a repeat run does not resubmit it. - * `submitRegisterRpTransaction` returns once the UserOp is submitted, not mined, - * and the UserOp nonce carries per-attempt randomness — so a second register() - * for the same rp_id can be accepted concurrently and one of them later reverts, - * burning gas. Covers the UserOp validity window plus margin, after which the - * on-chain read is authoritative again. - */ -const CLAIM_IN_FLIGHT_TTL_SECONDS = Math.ceil( - (USER_OP_MAX_VALIDITY_MS + 5 * 60 * 1000) / 1000, -); -const CLAIM_IN_FLIGHT_KEY_PREFIX = "rp_claim_in_flight:"; - -/** - * Reserves an rp_id for this run. False means a claim was submitted recently and - * has not settled, so skip it. - * - * Fails OPEN when Redis is unavailable: the endpoint is operator-driven and - * dry-run by default, so refusing to work without Redis would be worse than the - * duplicate submission this guards against. The on-chain read still catches - * anything that has actually mined. - */ -async function reserveClaim(rpIdString: string): Promise { - const redis = global.RedisClient; - if (!redis) { - return true; - } - try { - const reserved = await redis.set( - `${CLAIM_IN_FLIGHT_KEY_PREFIX}${rpIdString}`, - "1", - "EX", - CLAIM_IN_FLIGHT_TTL_SECONDS, - "NX", - ); - return reserved === "OK"; - } catch { - return true; - } -} - const schema = yup .object({ app_ids: yup @@ -368,7 +327,7 @@ export async function POST(request: NextRequest) { continue; } - if (!(await reserveClaim(`${registry.label}:${rpIdString}`))) { + if (!(await reserveClaim(registry.label, rpIdString))) { // A claim for this id was submitted recently and has not settled yet. logger.warn("Skipping an rp_id with a claim still in flight", { app_id: appId, @@ -409,6 +368,10 @@ export async function POST(request: NextRequest) { rp_id: rpIdString, }); } catch (error) { + // Nothing was submitted, so holding the reservation would report + // `skipped_claim_in_flight` for the whole TTL on an operation that does + // not exist. + await releaseClaim(registry.label, rpIdString); logger.error("Failed to claim rp_id", { error, app_id: appId, diff --git a/web/api/helpers/rp-claims.ts b/web/api/helpers/rp-claims.ts new file mode 100644 index 000000000..6add1f9e0 --- /dev/null +++ b/web/api/helpers/rp-claims.ts @@ -0,0 +1,107 @@ +import "server-only"; + +/** + * Tracking for defensive rp_id claims that have been submitted but not settled. + * + * `submitRegisterRpTransaction` returns once the UserOp is submitted, not once it + * is mined, so for a window afterwards an on-chain read still shows the rp_id as + * free. Two things must not act on that stale reading: + * + * - `_pre-register-rp-ids` re-running would submit a second `register()`. The + * UserOp nonce carries per-attempt randomness, so both can be accepted and one + * later reverts, burning gas. + * - `submitManagedRpRegistration` would submit a competing `register()`. If the + * pre-claim wins on-chain with the shared manager while the row records a + * dedicated one, every later status check and retry reads the shared claim as + * foreign and that registration never reconciles. + * + * Both live here so the key format and TTL cannot drift between the writer and + * the reader — the failure mode would be a marker nobody sees. + */ + +import { USER_OP_MAX_VALIDITY_MS } from "@/api/helpers/user-operation"; + +/** + * Covers the UserOp validity window plus the same margin the status endpoint uses + * before calling an unsettled op dead. After it, the on-chain read is + * authoritative again. + */ +const CLAIM_IN_FLIGHT_TTL_SECONDS = Math.ceil( + (USER_OP_MAX_VALIDITY_MS + 5 * 60 * 1000) / 1000, +); + +export type ClaimRegistry = "production" | "staging"; + +const claimKey = (registry: ClaimRegistry, rpIdString: string) => + `rp_claim_in_flight:${registry}:${rpIdString}`; + +/** + * Reserves an rp_id for a claim about to be submitted. False means a claim is + * already settling and this one should be skipped. + * + * Fails OPEN when Redis is unavailable or errors. The claim tool is + * operator-driven and dry-runs by default, and registration must not depend on a + * cache being up; the on-chain read still catches anything that has mined. + */ +export async function reserveClaim( + registry: ClaimRegistry, + rpIdString: string, +): Promise { + const redis = global.RedisClient; + if (!redis) { + return true; + } + try { + const reserved = await redis.set( + claimKey(registry, rpIdString), + "1", + "EX", + CLAIM_IN_FLIGHT_TTL_SECONDS, + "NX", + ); + return reserved === "OK"; + } catch { + return true; + } +} + +/** + * Drops a reservation whose submission never happened, so a retry is not blocked + * for the whole TTL waiting on an operation that does not exist. + */ +export async function releaseClaim( + registry: ClaimRegistry, + rpIdString: string, +): Promise { + const redis = global.RedisClient; + if (!redis) { + return; + } + try { + await redis.del(claimKey(registry, rpIdString)); + } catch { + // Best-effort: the TTL clears it either way. + } +} + +/** + * Whether a claim for this rp_id was submitted recently and may still land. + * + * Fails CLOSED — reports "not in flight" — when Redis is unavailable, matching + * `reserveClaim`'s fail-open: neither the claim tool nor registration may be + * blocked by a cache outage. + */ +export async function isClaimInFlight( + registry: ClaimRegistry, + rpIdString: string, +): Promise { + const redis = global.RedisClient; + if (!redis) { + return false; + } + try { + return (await redis.exists(claimKey(registry, rpIdString))) === 1; + } catch { + return false; + } +} diff --git a/web/api/helpers/rp-registration-flows.ts b/web/api/helpers/rp-registration-flows.ts index dbd2b3f1e..9fc8a97f8 100644 --- a/web/api/helpers/rp-registration-flows.ts +++ b/web/api/helpers/rp-registration-flows.ts @@ -24,6 +24,7 @@ import { getSdk as getRevertToggleSdk } from "@/api/hasura/toggle-rp-active/grap import { getSdk as getUpdateToggleSdk } from "@/api/hasura/toggle-rp-active/graphql/update-toggle-result.generated"; import { getKMSClient, scheduleKeyDeletion } from "@/api/helpers/kms"; import { createManagerKey, getEthAddressFromKMS } from "@/api/helpers/kms-eth"; +import { isClaimInFlight } from "@/api/helpers/rp-claims"; import { resolveManagerAddress } from "@/api/helpers/rp-manager"; import { submitRegisterRpTransaction, @@ -267,6 +268,30 @@ export async function submitManagedRpRegistration({ ); } + // A defensive claim may have been submitted moments ago and not mined yet, in + // which case the read above still shows the id as free. Registering into that + // window means two competing register() calls: if the pre-claim wins with the + // shared manager while this row records a dedicated one, every later status + // check and retry reads the shared claim as foreign and the registration never + // reconciles. Ask the developer to retry instead — by then the claim is visible + // and the adoption path takes over. + if ( + !existingOnChainRp?.initialized && + (await isClaimInFlight("production", rpIdString)) + ) { + logger.warn("Registration raced an in-flight defensive claim", { + app_id: appId, + rpIdString, + }); + await releaseSlot(); + return { + ok: false, + code: "submission_error", + detail: + "This app's RP ID is being prepared. Please try again in a few minutes.", + }; + } + // Only the PRODUCTION registry is authoritative for whether this app can be // registered at all; a foreign staging claim just means staging cannot mirror. if (existingOnChainRp?.initialized) { diff --git a/web/tests/api/helpers/rp-registration-flows.test.ts b/web/tests/api/helpers/rp-registration-flows.test.ts index 7d26a0844..b8e8eb7fc 100644 --- a/web/tests/api/helpers/rp-registration-flows.test.ts +++ b/web/tests/api/helpers/rp-registration-flows.test.ts @@ -161,8 +161,9 @@ const sharedManagerKeyArn = "arn:aws:kms:eu-west-1:000000000000:key/shared-manager"; const dedicatedManagerKeyId = "dedicated-kms-key"; -beforeEach(() => { +beforeEach(async () => { jest.clearAllMocks(); + await global.RedisClient?.flushall(); // Non-production by default so the staging mirror is out of scope; the // staging suite opts in explicitly. process.env.NEXT_PUBLIC_APP_ENV = "test"; @@ -1288,6 +1289,31 @@ describe("submitManagedRpRegistration [rp_id collision guard]", () => { expect(createManagerKey as jest.Mock).not.toHaveBeenCalled(); }); + it("refuses to race a defensive claim that has not mined yet", async () => { + // The claim was submitted moments ago, so the on-chain read still shows the id + // as free. Registering here means two competing register() calls; if the + // pre-claim wins with the shared manager while this row records a dedicated + // one, status and retry read the shared claim as foreign forever. + const { reserveClaim } = jest.requireActual("@/api/helpers/rp-claims"); + const { generateRpIdString } = jest.requireActual("@/lib/rp"); + await reserveClaim("production", generateRpIdString(appId)); + + getRpFromContractMock.mockResolvedValue({ + initialized: false, + active: false, + manager: `0x${"0".repeat(40)}`, + signer: `0x${"0".repeat(40)}`, + }); + + const res = await register(); + + expect(res).toMatchObject({ ok: false, code: "submission_error" }); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + expect(DeleteRpRegistration).toHaveBeenCalledWith({ + rp_id: expect.stringMatching(/^rp_/), + }); + }); + it("still reports rp_id_taken when releasing the slot fails", async () => { // The read's non-fatal catch must not extend over the slot release: swallowing // that failure would fall through to KMS and a UserOp for an rp_id already diff --git a/web/tests/api/pre-register-rp-ids.test.ts b/web/tests/api/pre-register-rp-ids.test.ts index 8dccce9a7..331f1406d 100644 --- a/web/tests/api/pre-register-rp-ids.test.ts +++ b/web/tests/api/pre-register-rp-ids.test.ts @@ -355,6 +355,21 @@ describe("/api/_pre-register-rp-ids [skips]", () => { expect(submitRegisterRpTransactionMock).toHaveBeenCalledTimes(1); }); + it("releases the reservation when the submission fails", async () => { + // Nothing was submitted, so holding the reservation would report + // skipped_claim_in_flight for the whole TTL on an operation that never + // existed, leaving the id unclaimed by the tool. + submitRegisterRpTransactionMock.mockRejectedValueOnce( + new Error("bundler down"), + ); + const first = await (await run()).json(); + expect(first.counts).toEqual({ "production:failed_submission": 1 }); + + const second = await (await run()).json(); + + expect(second.counts).toEqual({ "production:claimed": 1 }); + }); + it("records a submission failure without aborting the rest of the batch", async () => { const secondAppId = "app_00000000000000000000000000000002"; submitRegisterRpTransactionMock From 598e996df3e897bae76a32088e738b26b320a5e0 Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 13:45:36 +0200 Subject: [PATCH 11/12] fix(world-id): complete the ownership decision for staging races and missing keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more gaps in the same decision, both the incompleteness this file keeps producing. The in-flight reader checked only "production" while the writer reserves both registries, so a settling staging claim let a dedicated key be minted and a competing staging register() go out; if the shared-manager pre-claim landed first, staging status and retry read it as foreign and that side never recovered. A settling staging claim now forces the shared key — staging is best-effort, so it does not fail the registration, matching how an unreadable staging registry is already handled. A missing RP_REGISTRY_MANAGER_KMS_KEY_ID also fell through to a terminal rp_id_taken, because the retryable guard only fired when the variable was present. For a pre-claimed app that reports a deploy problem as "someone else owns your id, contact support". Absent-but-pre-claims-possible is now a retryable config_error, while a foreign id in an environment that never pre-claimed still gets rp_id_taken — pinned by a test that passes either way on purpose, since it guards behaviour that must not change. Co-Authored-By: Claude --- web/api/helpers/rp-registration-flows.ts | 47 ++++++++++--- .../api/helpers/rp-registration-flows.test.ts | 68 +++++++++++++++++++ 2 files changed, 105 insertions(+), 10 deletions(-) diff --git a/web/api/helpers/rp-registration-flows.ts b/web/api/helpers/rp-registration-flows.ts index 9fc8a97f8..c91748e50 100644 --- a/web/api/helpers/rp-registration-flows.ts +++ b/web/api/helpers/rp-registration-flows.ts @@ -234,6 +234,12 @@ export async function submitManagedRpRegistration({ } } + // Scoped to environments that actually pre-claim: RP_ID_PRE_REGISTRATION_SIGNER + // stays configured once claims exist there (see _pre-register-rp-ids). Elsewhere + // an unreadable or racing staging registry changes nothing, so dedicated keys + // keep their isolation. + const preClaimsPossible = Boolean(process.env.RP_ID_PRE_REGISTRATION_SIGNER); + let adoptExistingClaim = false; let adoptStagingClaim = false; if (existingOnChainRp?.initialized || existingStagingRp?.initialized) { @@ -242,16 +248,27 @@ export async function submitManagedRpRegistration({ ? await resolveManagerAddress(sharedManagerKeyId, primaryConfig.kmsRegion) : null; - if (sharedManagerKeyId && !ourManagerAddress) { + // Two ways ownership can be UNKNOWN rather than foreign, and both must stay + // retryable: the key is configured but KMS cannot resolve it, or the key is not + // configured at all in an environment that does pre-claim. Reporting either as + // rp_id_taken sends a developer whose app we hold to support over a deploy + // problem. + if (!ourManagerAddress && (sharedManagerKeyId || preClaimsPossible)) { logger.error( "Cannot tell whether an initialized rp_id is a Portal pre-claim", - { app_id: appId, rpIdString }, + { + app_id: appId, + rpIdString, + sharedManagerKeyConfigured: Boolean(sharedManagerKeyId), + }, ); await releaseSlot(); return { ok: false, - code: "kms_error", - detail: "Failed to resolve manager key. Please try again.", + code: sharedManagerKeyId ? "kms_error" : "config_error", + detail: sharedManagerKeyId + ? "Failed to resolve manager key. Please try again." + : "RP Registry is not configured correctly. Please try again later.", }; } @@ -275,6 +292,22 @@ export async function submitManagedRpRegistration({ // check and retry reads the shared claim as foreign and the registration never // reconciles. Ask the developer to retry instead — by then the claim is visible // and the adoption path takes over. + // Staging is checked as well — the writer reserves both registries. A settling + // staging claim only means staging cannot be decided yet, and staging is a + // best-effort mirror, so it forces the shared key rather than failing the whole + // registration (same treatment as an unreadable staging registry above). + if ( + stagingConfigForAdoption && + !existingStagingRp?.initialized && + (await isClaimInFlight("staging", rpIdString)) + ) { + stagingOwnershipUnknown = true; + logger.warn("A staging defensive claim is still settling", { + app_id: appId, + rpIdString, + }); + } + if ( !existingOnChainRp?.initialized && (await isClaimInFlight("production", rpIdString)) @@ -363,12 +396,6 @@ export async function submitManagedRpRegistration({ // a dedicated key here would produce a manager the contract has never heard // of, and every update would revert. A staging-only pre-claim forces it too — // the row records one manager key for both registries. - // Scoped to environments that actually pre-claim: RP_ID_PRE_REGISTRATION_SIGNER - // stays configured once claims exist there (see _pre-register-rp-ids). Elsewhere - // an unreadable staging registry changes nothing, so dedicated keys keep their - // isolation. - const preClaimsPossible = Boolean(process.env.RP_ID_PRE_REGISTRATION_SIGNER); - const useSharedManagerKey = adoptExistingClaim || adoptStagingClaim || diff --git a/web/tests/api/helpers/rp-registration-flows.test.ts b/web/tests/api/helpers/rp-registration-flows.test.ts index b8e8eb7fc..0cab18d4f 100644 --- a/web/tests/api/helpers/rp-registration-flows.test.ts +++ b/web/tests/api/helpers/rp-registration-flows.test.ts @@ -1314,6 +1314,74 @@ describe("submitManagedRpRegistration [rp_id collision guard]", () => { }); }); + it("returns a retryable config error when the shared key is missing but pre-claims exist", async () => { + // A missing RP_REGISTRY_MANAGER_KMS_KEY_ID is a deploy problem, not proof that + // someone else owns the id. Reporting rp_id_taken sends a developer whose app + // we are holding to support over a config outage. + process.env.RP_ID_PRE_REGISTRATION_SIGNER = + "0x000000000000000000000000000000000000dEaD"; + delete process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: managerAddress, + signer: "0x000000000000000000000000000000000000dead", + }); + + const res = await register(); + + expect(res).toMatchObject({ ok: false, code: "config_error" }); + expect(submitRegisterRpTransactionMock).not.toHaveBeenCalled(); + }); + + it("still reports rp_id_taken for a foreign id where pre-claims are impossible", async () => { + // The retryable path above must not swallow the genuine squatter case in an + // environment that has never pre-claimed anything. + delete process.env.RP_ID_PRE_REGISTRATION_SIGNER; + delete process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: "0x00000000000000000000000000000000000000ff", + signer: "0x00000000000000000000000000000000000000ee", + }); + + const res = await register(); + + expect(res).toMatchObject({ ok: false, code: "rp_id_taken" }); + }); + + it("forces the shared key when a STAGING claim is still settling", async () => { + // The writer reserves both registries; a reader that only checks production + // lets a dedicated key be minted while the staging pre-claim lands, which + // leaves staging unrecoverable. Staging is best-effort, so this forces the + // shared key rather than failing the registration. + process.env.NEXT_PUBLIC_APP_ENV = "production"; + process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID = sharedManagerKeyArn; + process.env.RP_ID_PRE_REGISTRATION_SIGNER = + "0x000000000000000000000000000000000000dEaD"; + mockGetStagingRpRegistryConfig.mockReturnValue({ + contractAddress: "0xstagingcontract", + kmsRegion: "us-east-1", + }); + const { reserveClaim } = jest.requireActual("@/api/helpers/rp-claims"); + const { generateRpIdString } = jest.requireActual("@/lib/rp"); + await reserveClaim("staging", generateRpIdString(appId)); + + // Both registries read as free — the staging claim has not mined. + getRpFromContractMock.mockResolvedValue({ + initialized: false, + active: false, + manager: `0x${"0".repeat(40)}`, + signer: `0x${"0".repeat(40)}`, + }); + + const res = await register(); + + expect(res).toMatchObject({ ok: true }); + expect(createManagerKey as jest.Mock).not.toHaveBeenCalled(); + }); + it("still reports rp_id_taken when releasing the slot fails", async () => { // The read's non-fatal catch must not extend over the slot release: swallowing // that failure would fall through to KMS and a UserOp for an rp_id already From b5957497d65582f815b73ee6b97781d4da4d9685 Mon Sep 17 00:00:00 2001 From: Dmitry Lugovoy Date: Wed, 5 Aug 2026 13:54:44 +0200 Subject: [PATCH 12/12] fix(world-id): recognise a pre-claim after the kill switch goes back off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The self-managed guard's fail-closed path was keyed to ENABLE_RP_ID_PRE_REGISTRATION — the kill switch — while the comment three lines above it stated that defensive claims outlive that flag. So the exact state the comment described as an invariant (sweep has run, flag turned back off, placeholder unset) was silently unguarded: a Portal-held placeholder RP would be inserted as self_managed, and rp-status trusts self-managed rows by mode, so it would be promoted against a signer that can never sign. There are two independent tells that Portal holds an id, and the guard now uses both in cost order. The placeholder signer stays preferred: a plain env var, no remote call, so it cannot fail open or closed on someone else's outage. The shared manager address is the fallback, paid for only when the cheap tell is unavailable or negative — KMS has no business in a self-managed registration, but a silently broken registration is worse than a retryable error. Unresolvable manager is retryable, not terminal. With neither tell there is no in-band signal left, so the guard cannot run: a config error while claims are being made, a no-op where nothing was ever claimed. Verified the fallback test fails without the fallback, and that the existing "does not depend on KMS" test still passes with the fallback removed — so the cheap path is genuinely still preferred rather than incidentally exercised. Co-Authored-By: Claude --- web/api/hasura/register-rp/index.ts | 101 +++++++++++++++-------- web/tests/api/hasura/register-rp.test.ts | 49 +++++++++-- 2 files changed, 110 insertions(+), 40 deletions(-) diff --git a/web/api/hasura/register-rp/index.ts b/web/api/hasura/register-rp/index.ts index 65466d602..07fa93c4f 100644 --- a/web/api/hasura/register-rp/index.ts +++ b/web/api/hasura/register-rp/index.ts @@ -14,6 +14,7 @@ import { parseRpId, RpRegistrationStatus, } from "@/api/helpers/rp-utils"; +import { resolveManagerAddress } from "@/api/helpers/rp-manager"; import { getRpFromContract } from "@/api/helpers/temporal-rpc"; import { protectInternalEndpoint } from "@/api/helpers/utils"; import { validateRequestSchema } from "@/api/helpers/validate-request-schema"; @@ -182,36 +183,41 @@ export const POST = async (req: NextRequest) => { // which for self-managed the Portal cannot tell apart either way, unchanged // by this PR since it stores no expected roles for self-managed rows. // - // When the check CANNOT run, whether skipping is safe depends entirely on - // whether pre-claims can exist: - // - pre-registration enabled but misconfigured -> config_error. Claims are - // being made and we cannot recognise them, which is a deploy fault, not a - // developer's problem to absorb silently. - // - placeholder configured but the chain unreadable -> rpc_error, RETRYABLE. - // A pre-claimed id admitted here becomes a row rp-status promotes (it - // trusts self-managed rows by mode) against a signer that can never sign. - // A retryable error is recoverable; that registration is not. - // - placeholder unset and pre-registration never enabled -> skip. No claims - // exist, so there is nothing to recognise. This is every environment that - // has not run the tool. + // Two independent tells that Portal holds this id, in cost order: // - // OPERATIONAL INVARIANT: once pre-registration has been run in an - // environment, RP_ID_PRE_REGISTRATION_SIGNER must stay set there forever. - // The claims outlive the flag. + // 1. the PLACEHOLDER SIGNER — a plain env var, no remote call, so it cannot + // fail open or fail closed on someone else's outage. Preferred. + // 2. the SHARED MANAGER address — needs a KMS round trip, so it is only a + // fallback. KMS has no business in a self-managed registration, but a + // silently broken registration is worse than a retryable error. + // + // The fallback exists because defensive claims OUTLIVE the kill switch. Keying + // the guard to ENABLE_RP_ID_PRE_REGISTRATION would leave the exact state this + // comment used to describe as an invariant — sweep has run, flag turned back + // off, placeholder unset — silently unguarded, and a Portal-held placeholder RP + // would be inserted as self_managed. rp-status trusts self-managed rows by + // mode, so it would then be promoted against a signer that can never sign. + // + // With neither tell available there is no in-band signal left, so the guard + // cannot run at all: that is a config error while claims are being made, and a + // no-op in an environment that has never claimed anything. const primaryConfig = getRpRegistryConfig(); const placeholderSigner = process.env.RP_ID_PRE_REGISTRATION_SIGNER; + const sharedManagerKeyId = process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; const preRegistrationEnabled = process.env.ENABLE_RP_ID_PRE_REGISTRATION === "true"; - const canRecognisePreClaims = Boolean( + + const canCheckBySigner = Boolean( primaryConfig && placeholderSigner && isAddress(placeholderSigner) && !isZeroAddress(placeholderSigner), ); + const canCheckByManager = Boolean(primaryConfig && sharedManagerKeyId); - if (!canRecognisePreClaims && preRegistrationEnabled) { + if (!canCheckBySigner && !canCheckByManager && preRegistrationEnabled) { logger.error( - "Pre-registration is enabled but its placeholder signer is unusable", + "Pre-registration is enabled but nothing identifies a Portal pre-claim", { app_id, hasConfig: Boolean(primaryConfig) }, ); return errorHasuraQuery({ @@ -222,7 +228,7 @@ export const POST = async (req: NextRequest) => { }); } - if (canRecognisePreClaims) { + if (canCheckBySigner || canCheckByManager) { let onChain; try { onChain = await getRpFromContract( @@ -244,22 +250,47 @@ export const POST = async (req: NextRequest) => { }); } - if ( - onChain.initialized && - addressesEqual(onChain.signer, placeholderSigner!) - ) { - logger.warn("Self-managed rp_id is held by a Portal pre-claim", { - app_id, - rpIdString, - onChainManager: onChain.manager, - }); - return errorHasuraQuery({ - req, - detail: - "This app's RP ID is held by Portal and cannot be self-managed yet — contact support.", - code: "rp_id_taken", - app_id, - }); + if (onChain.initialized) { + let heldByPortal = + canCheckBySigner && + addressesEqual(onChain.signer, placeholderSigner!); + + // Only pay for KMS when the cheap tell was unavailable or negative. + if (!heldByPortal && canCheckByManager) { + const ourManagerAddress = await resolveManagerAddress( + sharedManagerKeyId!, + primaryConfig!.kmsRegion, + ); + if (!ourManagerAddress) { + logger.error( + "Cannot tell whether a self-managed rp_id is a Portal pre-claim", + { app_id, rpIdString }, + ); + return errorHasuraQuery({ + req, + detail: + "Could not verify this app's RP ID. Please try again shortly.", + code: "kms_error", + app_id, + }); + } + heldByPortal = addressesEqual(onChain.manager, ourManagerAddress); + } + + if (heldByPortal) { + logger.warn("Self-managed rp_id is held by a Portal pre-claim", { + app_id, + rpIdString, + onChainManager: onChain.manager, + }); + return errorHasuraQuery({ + req, + detail: + "This app's RP ID is held by Portal and cannot be self-managed yet — contact support.", + code: "rp_id_taken", + app_id, + }); + } } } diff --git a/web/tests/api/hasura/register-rp.test.ts b/web/tests/api/hasura/register-rp.test.ts index 5782bfb18..277b865db 100644 --- a/web/tests/api/hasura/register-rp.test.ts +++ b/web/tests/api/hasura/register-rp.test.ts @@ -258,10 +258,11 @@ describe("/api/hasura/register-rp [self-managed on-chain ownership]", () => { }); it("skips the check entirely where pre-claims cannot exist", async () => { - // No placeholder configured and pre-registration never enabled: there is - // nothing to recognise, so a read failure must not block onboarding. This is - // every environment that has not run the tool. + // Neither tell available and pre-registration never enabled: there is nothing + // to recognise, so a read failure must not block onboarding. This is every + // environment that has not run the tool. delete process.env.RP_ID_PRE_REGISTRATION_SIGNER; + delete process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; delete process.env.ENABLE_RP_ID_PRE_REGISTRATION; getRpFromContractMock.mockRejectedValue(new Error("rpc timeout")); @@ -271,17 +272,55 @@ describe("/api/hasura/register-rp [self-managed on-chain ownership]", () => { expect(getRpFromContractMock).not.toHaveBeenCalled(); }); - it("refuses to proceed when pre-registration is on but its signer is unusable", async () => { - // Claims are being made and we cannot recognise them — a deploy fault, not + it("refuses to proceed when pre-registration is on and nothing identifies a claim", async () => { + // Claims are being made and neither tell is available — a deploy fault, not // something a developer should absorb as a broken registration. process.env.ENABLE_RP_ID_PRE_REGISTRATION = "true"; process.env.RP_ID_PRE_REGISTRATION_SIGNER = `0x${"0".repeat(40)}`; + delete process.env.RP_REGISTRY_MANAGER_KMS_KEY_ID; const res = (await selfManaged())!; const body = await res.json(); expect(body.code ?? body.extensions?.code).toBe("config_error"); }); + + it("falls back to the shared manager when the placeholder signer is unset", async () => { + // Defensive claims outlive the kill switch, so the guard cannot be keyed to it: + // sweep has run, flag turned back off, placeholder unset. Without the manager + // fallback a Portal-held placeholder RP would be inserted as self_managed and + // rp-status would promote it against a signer that can never sign. + delete process.env.RP_ID_PRE_REGISTRATION_SIGNER; + delete process.env.ENABLE_RP_ID_PRE_REGISTRATION; + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: portalManagerAddress, + signer: "0xSomeOtherSigner", + }); + + const res = (await selfManaged())!; + const body = await res.json(); + + expect(body.code ?? body.extensions?.code).toBe("rp_id_taken"); + expect(resolveManagerAddressMock).toHaveBeenCalled(); + }); + + it("returns a retryable error when the manager fallback cannot resolve", async () => { + delete process.env.RP_ID_PRE_REGISTRATION_SIGNER; + resolveManagerAddressMock.mockResolvedValue(null); + getRpFromContractMock.mockResolvedValue({ + initialized: true, + active: true, + manager: portalManagerAddress, + signer: "0xSomeOtherSigner", + }); + + const res = (await selfManaged())!; + const body = await res.json(); + + expect(body.code ?? body.extensions?.code).toBe("kms_error"); + }); }); // #endregion