From 25fc579ccc9382cf389af7d2b2978fc9576e7da0 Mon Sep 17 00:00:00 2001 From: Miracle656 Date: Sun, 8 Nov 2026 09:46:54 +0100 Subject: [PATCH] fix(wallet): stop giving Veil names away for free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /api/names/claim` is deployed, open to every origin, and granted a name on the strength of a `paymentTxHash` that was never checked against the ledger. The registry's default verifiers were stubs that accepted: paymentVerifier ?? (async (txHash) => Boolean(txHash && txHash.trim().length > 0)) signatureVerifier ?? (async () => true) and `globalNamesRegistry` is constructed with no options, so those stubs were the production policy. Any caller could take any unreserved name by sending a one-character hash, bound to whatever `passkeyPublicKey` they chose. The signature was only checked `if (signature)`, so omitting it skipped the check rather than failing it — nothing proved possession of the passkey the name was being bound to. The route could not have honoured a real payment either. The registry is a set of `Map`s behind a module singleton: every serverless instance gets its own and instances are recycled, so a claim is invisible to the next request and gone after a deploy. Three changes: - **The verifier defaults now refuse.** A stub standing in for money has to fail closed, or the day something is wired to it is the day names become free. Callers that want permissive behaviour — the unit tests covering charset, reserved names and claim concurrency — now pass their own verifiers and say so at the construction site. - **Both routes return 503**, via `lib/namesApiGate.ts`. Deliberately not an env flag: a flag is a switch somebody eventually flips, and what is behind it hands out names. `GET /api/names/available` is shut for its own reason — it answered from the always-empty registry, so every name came back `available: true`, with `Cache-Control: public, max-age=10` inviting clients to cache that. - **The route tests assert the gate.** They previously asserted a 201 for `paymentTxHash: 'tx_valid_jupiter'`, which encoded the hole as intended behaviour. Reserved/taken/charset coverage already lives in `lib/__tests__/namesRegistry.test.ts`, so nothing unique left coverage. The real registry is `contracts/veil-names`, where `register` requires `buyer` auth so the ledger proves possession, and a name is rented by the year in length bands rather than sold once for a flat 5 USDC. `namesApiGate.ts` records what lifting the gate requires. Not fixed, and noted in place instead: the optional-signature gap above. Hardening a module that the on-chain registry retires is wasted work. No caller anywhere in the repo reaches these routes — only their own tests — so nothing regresses. `npx tsc --noEmit` clean; wallet suite 100 suites / 1563 passed. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB --- .../api/names/__tests__/claimRoute.test.ts | 176 ++++++------------ .../wallet/app/api/names/available/route.ts | 49 ++--- frontend/wallet/app/api/names/claim/route.ts | 140 +++----------- .../lib/__tests__/namesRegistry.test.ts | 11 +- frontend/wallet/lib/namesApiGate.ts | 64 +++++++ frontend/wallet/lib/namesRegistry.ts | 26 ++- 6 files changed, 193 insertions(+), 273 deletions(-) create mode 100644 frontend/wallet/lib/namesApiGate.ts diff --git a/frontend/wallet/app/api/names/__tests__/claimRoute.test.ts b/frontend/wallet/app/api/names/__tests__/claimRoute.test.ts index 4c564067f..ba8a1d34c 100644 --- a/frontend/wallet/app/api/names/__tests__/claimRoute.test.ts +++ b/frontend/wallet/app/api/names/__tests__/claimRoute.test.ts @@ -2,114 +2,50 @@ * @jest-environment node */ +/** + * The names API is closed (`lib/namesApiGate.ts`). + * + * This file previously drove the in-process registry through the routes and + * asserted a 201 on `paymentTxHash: 'tx_valid_jupiter'` — encoding, as + * intended behaviour, that any non-empty string buys a name. Those cases are + * gone with the behaviour. What the registry itself does with a reserved + * name, a taken name or a bad charset is covered directly in + * `lib/__tests__/namesRegistry.test.ts`, so nothing unique moved out of + * coverage here. + * + * What is left is the thing that must not regress: both routes refuse, and + * they refuse for every shape of request, including a well-formed one. + */ + import { NextRequest } from 'next/server' -import { POST as claimPOST, OPTIONS as claimOPTIONS } from '../claim/route' -import { GET as availableGET } from '../available/route' -import { globalNamesRegistry } from '@/lib/namesRegistry' -describe('/api/names/claim and /api/names/available routes (Issue #1073)', () => { - beforeEach(() => { - globalNamesRegistry.reset() - }) +import { POST as claimPOST, OPTIONS as claimOPTIONS } from '../claim/route' +import { GET as availableGET, OPTIONS as availableOPTIONS } from '../available/route' +describe('the names API is closed until the registry is on-chain', () => { describe('POST /api/names/claim', () => { - it('returns 400 when required fields are missing', async () => { - const req = new NextRequest('https://app.useveilapp.xyz/api/names/claim', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ name: 'myname' }), - }) - - const res = await claimPOST(req) - expect(res.status).toBe(400) - const data = await res.json() - expect(data.error).toContain('Missing required fields') - }) + it('refuses a well-formed claim with 503', async () => { + const res = await claimPOST() + expect(res.status).toBe(503) - it('returns 402 for unpaid claims', async () => { - const req = new NextRequest('https://app.useveilapp.xyz/api/names/claim', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - name: 'cooperative', - walletAddress: 'GA_WALLET_123', - passkeyPublicKey: 'pk_unpaid_route', - paymentTxHash: '', - }), - }) - - const res = await claimPOST(req) - expect(res.status).toBe(402) const data = await res.json() - expect(data.code).toBe('UNPAID_CLAIM') + expect(data.code).toBe('NAMES_NOT_AVAILABLE') }) - it('returns 403 when claiming a reserved name', async () => { - const req = new NextRequest('https://app.useveilapp.xyz/api/names/claim', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - name: 'admin', - walletAddress: 'GA_SQUATTER', - passkeyPublicKey: 'pk_squatter', - paymentTxHash: 'tx_hash_squatter', - }), - }) - - const res = await claimPOST(req) - expect(res.status).toBe(403) - const data = await res.json() - expect(data.code).toBe('RESERVED_NAME') + it('never answers 201, whatever the body says', async () => { + // The body is ignored entirely — the route takes no request argument. + // Asserting the status directly is the point: there is no combination + // of fields that reaches the registry any more. + const res = await claimPOST() + expect(res.status).not.toBe(201) }) - it('returns 201 when claim succeeds', async () => { - const req = new NextRequest('https://app.useveilapp.xyz/api/names/claim', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - name: 'jupiter', - walletAddress: 'GA_JUPITER_WALLET', - passkeyPublicKey: 'pk_jupiter_pk', - paymentTxHash: 'tx_valid_jupiter', - }), - }) - - const res = await claimPOST(req) - expect(res.status).toBe(201) - const data = await res.json() - expect(data.success).toBe(true) - expect(data.record.name).toBe('jupiter') - expect(data.record.stellar_address).toBe('jupiter*useveilapp.xyz') - expect(data.record.tokenId).toBe('1') + it('is not cached, so lifting the gate takes effect immediately', async () => { + const res = await claimPOST() + expect(res.headers.get('Cache-Control')).toBe('no-store') }) - it('returns 409 when claiming an already taken name', async () => { - // First claim - await globalNamesRegistry.claimName({ - name: 'saturn', - walletAddress: 'GA_SATURN', - passkeyPublicKey: 'pk_saturn', - paymentTxHash: 'tx_saturn', - }) - - const req = new NextRequest('https://app.useveilapp.xyz/api/names/claim', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - name: 'saturn', - walletAddress: 'GA_SATURN_2', - passkeyPublicKey: 'pk_saturn_2', - paymentTxHash: 'tx_saturn_2', - }), - }) - - const res = await claimPOST(req) - expect(res.status).toBe(409) - const data = await res.json() - expect(data.code).toBe('NAME_ALREADY_TAKEN') - }) - - it('responds to OPTIONS preflight with 204 and CORS headers', async () => { + it('answers an OPTIONS preflight with 204 and CORS headers', async () => { const res = await claimOPTIONS() expect(res.status).toBe(204) expect(res.headers.get('Access-Control-Allow-Origin')).toBe('*') @@ -117,34 +53,36 @@ describe('/api/names/claim and /api/names/available routes (Issue #1073)', () => }) describe('GET /api/names/available', () => { - it('returns 400 if name parameter is missing', async () => { - const req = new NextRequest('https://app.useveilapp.xyz/api/names/available') - const res = await availableGET(req) - expect(res.status).toBe(400) - }) + it('refuses with 503 rather than claiming a name is free', async () => { + const res = await availableGET() + expect(res.status).toBe(503) - it('returns available: true for untaken valid name', async () => { - const req = new NextRequest('https://app.useveilapp.xyz/api/names/available?name=available-name') - const res = await availableGET(req) - expect(res.status).toBe(200) const data = await res.json() - expect(data.available).toBe(true) + expect(data.code).toBe('NAMES_NOT_AVAILABLE') + // The old route returned `available: true` for every name, because the + // in-process registry is always empty in production. + expect(data.available).toBeUndefined() }) - it('returns available: false for taken name', async () => { - await globalNamesRegistry.claimName({ - name: 'pluto', - walletAddress: 'GA_PLUTO', - passkeyPublicKey: 'pk_pluto', - paymentTxHash: 'tx_pluto', - }) + it('does not let a client cache the refusal as an answer', async () => { + const res = await availableGET() + expect(res.headers.get('Cache-Control')).toBe('no-store') + }) - const req = new NextRequest('https://app.useveilapp.xyz/api/names/available?name=pluto') - const res = await availableGET(req) - expect(res.status).toBe(200) - const data = await res.json() - expect(data.available).toBe(false) - expect(data.reason).toBe('Name is already taken') + it('answers an OPTIONS preflight with 204 and CORS headers', async () => { + const res = await availableOPTIONS() + expect(res.status).toBe(204) + expect(res.headers.get('Access-Control-Allow-Origin')).toBe('*') + }) + }) + + describe('the request type still compiles against NextRequest', () => { + it('constructs a request the route would have received', () => { + // Kept so the import does not drift out of the file while the routes + // take no argument; when the gate lifts, these handlers take a + // NextRequest again and this is the shape they get. + const req = new NextRequest('https://app.useveilapp.xyz/api/names/available?name=adebayo') + expect(new URL(req.url).searchParams.get('name')).toBe('adebayo') }) }) }) diff --git a/frontend/wallet/app/api/names/available/route.ts b/frontend/wallet/app/api/names/available/route.ts index eecbaa072..ef667f8cd 100644 --- a/frontend/wallet/app/api/names/available/route.ts +++ b/frontend/wallet/app/api/names/available/route.ts @@ -1,45 +1,24 @@ -import { NextRequest, NextResponse } from 'next/server' -import { globalNamesRegistry } from '@/lib/namesRegistry' +/** + * `GET /api/names/available` — closed. + * + * It used to answer from the in-process registry in `lib/namesRegistry.ts`, + * which is always empty in production, so every name came back + * `available: true` with `Cache-Control: public, max-age=10` on it. A client + * could cache a yes for a name that is registered on-chain. See + * `lib/namesApiGate.ts` for why this is shut rather than flagged. + */ -export async function GET(req: NextRequest) { - const { searchParams } = new URL(req.url) - const name = searchParams.get('name') +import { NextResponse } from 'next/server' - if (!name) { - return NextResponse.json( - { error: 'Query parameter "name" is required.' }, - { status: 400, headers: corsHeaders() }, - ) - } +import { namesCorsHeaders, namesUnavailable } from '@/lib/namesApiGate' - const result = globalNamesRegistry.checkAvailability(name) - - return NextResponse.json( - { - name, - ...result, - }, - { - status: 200, - headers: { - ...corsHeaders(), - 'Cache-Control': 'public, max-age=10, s-maxage=10', - }, - }, - ) +export async function GET() { + return namesUnavailable() } export async function OPTIONS() { return new NextResponse(null, { status: 204, - headers: corsHeaders(), + headers: namesCorsHeaders(), }) } - -function corsHeaders(): Record { - return { - 'Access-Control-Allow-Origin': '*', - 'Access-Control-Allow-Methods': 'GET, OPTIONS', - 'Access-Control-Allow-Headers': 'Content-Type', - } -} diff --git a/frontend/wallet/app/api/names/claim/route.ts b/frontend/wallet/app/api/names/claim/route.ts index e16539035..15955a64c 100644 --- a/frontend/wallet/app/api/names/claim/route.ts +++ b/frontend/wallet/app/api/names/claim/route.ts @@ -1,121 +1,33 @@ -import { NextRequest, NextResponse } from 'next/server' -import { - globalNamesRegistry, - InvalidNameError, - ReservedNameError, - NameAlreadyTakenError, - UnpaidClaimError, - PasskeyAlreadyClaimedError, - InvalidSignatureError, -} from '@/lib/namesRegistry' - -export async function POST(req: NextRequest) { - try { - const body = await req.json().catch(() => null) - if (!body || typeof body !== 'object') { - return NextResponse.json( - { error: 'Invalid JSON request body.' }, - { status: 400, headers: corsHeaders() }, - ) - } - - const { name, walletAddress, passkeyPublicKey, paymentTxHash, signature } = body - - if (!name || !walletAddress || !passkeyPublicKey) { - return NextResponse.json( - { - error: - 'Missing required fields. "name", "walletAddress", and "passkeyPublicKey" are required.', - }, - { status: 400, headers: corsHeaders() }, - ) - } - - const record = await globalNamesRegistry.claimName({ - name, - walletAddress, - passkeyPublicKey, - paymentTxHash, - signature, - }) - - return NextResponse.json( - { - success: true, - record: { - name: record.name, - stellar_address: `${record.name}*useveilapp.xyz`, - tokenId: record.tokenId, - walletAddress: record.walletAddress, - passkeyPublicKey: record.passkeyPublicKey, - claimedAt: record.claimedAt, - }, - }, - { - status: 201, - headers: { - ...corsHeaders(), - 'Cache-Control': 'no-store', - }, - }, - ) - } catch (err: unknown) { - if (err instanceof InvalidNameError) { - return NextResponse.json( - { error: err.message, code: 'INVALID_NAME' }, - { status: 400, headers: corsHeaders() }, - ) - } - if (err instanceof PasskeyAlreadyClaimedError) { - return NextResponse.json( - { error: err.message, code: 'PASSKEY_ALREADY_CLAIMED' }, - { status: 400, headers: corsHeaders() }, - ) - } - if (err instanceof InvalidSignatureError) { - return NextResponse.json( - { error: err.message, code: 'INVALID_SIGNATURE' }, - { status: 400, headers: corsHeaders() }, - ) - } - if (err instanceof UnpaidClaimError) { - return NextResponse.json( - { error: err.message, code: 'UNPAID_CLAIM' }, - { status: 402, headers: corsHeaders() }, - ) - } - if (err instanceof ReservedNameError) { - return NextResponse.json( - { error: err.message, code: 'RESERVED_NAME' }, - { status: 403, headers: corsHeaders() }, - ) - } - if (err instanceof NameAlreadyTakenError) { - return NextResponse.json( - { error: err.message, code: 'NAME_ALREADY_TAKEN' }, - { status: 409, headers: corsHeaders() }, - ) - } - - const message = err instanceof Error ? err.message : 'Internal server error' - return NextResponse.json( - { error: message, code: 'SERVER_ERROR' }, - { status: 500, headers: corsHeaders() }, - ) - } +/** + * `POST /api/names/claim` — closed. + * + * This route was deployed, open to any origin, and granted a name on the + * strength of a `paymentTxHash` that was never checked against the ledger: + * the default verifier in `lib/namesRegistry.ts` returned true for any + * non-empty string, and the default signature verifier returned true + * unconditionally. Any caller could take any unreserved name for free, bound + * to any `passkeyPublicKey` they chose, and — because the registry is + * in-process — the record would not survive the next cold start anyway. + * + * Those defaults now fail closed, so the stub cannot grant anything even if + * something is wired to it later. This route is shut on top of that, because + * the registry that matters is `contracts/veil-names` and it is not deployed. + * + * See `lib/namesApiGate.ts` for the reasoning and for what lifting the gate + * requires. + */ + +import { NextResponse } from 'next/server' + +import { namesCorsHeaders, namesUnavailable } from '@/lib/namesApiGate' + +export async function POST() { + return namesUnavailable() } export async function OPTIONS() { return new NextResponse(null, { status: 204, - headers: corsHeaders(), + headers: namesCorsHeaders(), }) } - -function corsHeaders(): Record { - return { - 'Access-Control-Allow-Origin': '*', - 'Access-Control-Allow-Methods': 'POST, OPTIONS', - 'Access-Control-Allow-Headers': 'Content-Type, Authorization', - } -} diff --git a/frontend/wallet/lib/__tests__/namesRegistry.test.ts b/frontend/wallet/lib/__tests__/namesRegistry.test.ts index cd431bd12..793cca182 100644 --- a/frontend/wallet/lib/__tests__/namesRegistry.test.ts +++ b/frontend/wallet/lib/__tests__/namesRegistry.test.ts @@ -15,7 +15,16 @@ describe('NamesRegistry (Issue #1073)', () => { let registry: NamesRegistry beforeEach(() => { - registry = new NamesRegistry() + // Both verifiers are stated explicitly rather than inherited. The + // constructor defaults refuse — a stub standing in for a payment has to + // fail closed, because the deployed claim route inherited the old + // permissive ones and handed out names for free. These tests are about + // charset, reserved names and claim concurrency, so they opt in to + // "payment accepted" and say so here. + registry = new NamesRegistry({ + paymentVerifier: async (txHash) => Boolean(txHash && txHash.trim().length > 0), + signatureVerifier: async () => true, + }) }) describe('Criterion 1: A name cannot be claimed twice under concurrent requests (tested concurrently)', () => { diff --git a/frontend/wallet/lib/namesApiGate.ts b/frontend/wallet/lib/namesApiGate.ts new file mode 100644 index 000000000..669b77257 --- /dev/null +++ b/frontend/wallet/lib/namesApiGate.ts @@ -0,0 +1,64 @@ +/** + * The gate on the off-chain names API. + * + * `lib/namesRegistry.ts` was the first cut at Veil Names: an in-process + * registry with a 5 USDC flat price, reached through `POST /api/names/claim` + * and `GET /api/names/available`. Both routes are deployed. Neither can + * answer correctly, for two independent reasons: + * + * 1. **It cannot hold a claim.** The registry is a set of `Map`s behind a + * module singleton. Each serverless instance gets its own, and instances + * are recycled, so a name claimed on one request is invisible on the next + * and gone after a deploy. There is no database behind it. + * + * 2. **It is the wrong price.** The flat 5 USDC predates the respec. A name + * is now *rented* by the year in length bands — 3 USDC for 3–5 characters, + * 5 for 6–10, 2 for 11–20 — for a one-to-five year term, with a 90-day + * recovery window. `contracts/veil-names` implements that and is the + * source of truth. Quoting 5 USDC flat against a contract that charges by + * band means two live prices for the same name. + * + * So the routes return 503 until the registry is read from the chain. This is + * deliberately not an env flag: a flag is a switch somebody eventually flips, + * and what is behind it hands out names for free (see the fail-closed + * verifier defaults in `namesRegistry.ts` and why they had to change). + * + * To lift the gate: point the routes at the deployed `veil-names` contract — + * `resolve`, `state` and `quote` answer availability and price, and + * `register` takes the payment — then delete this module and the in-process + * registry with it. + */ + +import { NextResponse } from 'next/server' + +/** CORS headers shared by the names routes. */ +export function namesCorsHeaders(): Record { + return { + 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', + 'Access-Control-Allow-Headers': 'Content-Type, Authorization', + } +} + +/** + * The 503 every names route returns while the on-chain registry is not wired. + * + * `Retry-After` is omitted on purpose: there is no date to give, and a number + * invented here would be a promise the deploy schedule has not made. + */ +export function namesUnavailable(): NextResponse { + return NextResponse.json( + { + error: + 'Veil Names is not open yet. Names are registered on-chain and the registry contract is not deployed, so this endpoint cannot reserve or price a name.', + code: 'NAMES_NOT_AVAILABLE', + }, + { + status: 503, + headers: { + ...namesCorsHeaders(), + 'Cache-Control': 'no-store', + }, + }, + ) +} diff --git a/frontend/wallet/lib/namesRegistry.ts b/frontend/wallet/lib/namesRegistry.ts index 0b8a865fc..23b627018 100644 --- a/frontend/wallet/lib/namesRegistry.ts +++ b/frontend/wallet/lib/namesRegistry.ts @@ -287,9 +287,20 @@ export class NamesRegistry { signatureVerifier?: PasskeySignatureVerifier initialRecords?: ClaimedNameRecord[] }) { - this.paymentVerifier = - options?.paymentVerifier ?? (async (txHash) => Boolean(txHash && txHash.trim().length > 0)) - this.signatureVerifier = options?.signatureVerifier ?? (async () => true) + // Both defaults refuse. They used to accept — `paymentVerifier` returned + // true for any non-empty string and `signatureVerifier` returned true + // unconditionally — and because `globalNamesRegistry` is constructed with + // no options, that made the deployed `POST /api/names/claim` hand out any + // unreserved name to any caller who sent a one-character `paymentTxHash`, + // bound to whatever `passkeyPublicKey` they supplied, with + // `Access-Control-Allow-Origin: *`. + // + // A stub that stands in for money has to fail closed, or the day someone + // wires a route to it is the day names become free. A caller that wants + // permissive behaviour — a unit test exercising charset or reserved-list + // logic — passes its own verifier and says so explicitly. + this.paymentVerifier = options?.paymentVerifier ?? (async () => false) + this.signatureVerifier = options?.signatureVerifier ?? (async () => false) if (options?.initialRecords) { for (const rec of options?.initialRecords) { @@ -438,7 +449,14 @@ export class NamesRegistry { throw new UnpaidClaimError('Payment verification failed for 5 USDC.') } - // 2. Verify passkey signature if provided or required + // 2. Verify the passkey signature when one is supplied. + // + // Only when supplied — omitting `signature` skips the check rather than + // failing it, so this proves nothing about possession of the passkey the + // name is being bound to. That is a real gap and it is left here + // deliberately: the on-chain registry (`contracts/veil-names`) requires + // `buyer` auth for `register`, so possession is proven by the ledger + // rather than by this module, which the route gate below retires. if (signature) { const payload = `${normalized}:${walletAddress}:${paymentTxHash}` const isValidSignature = await this.signatureVerifier(passkeyPublicKey, payload, signature)