From f58ebcfdeb18cb0288f6e299ec338bbff027b9db Mon Sep 17 00:00:00 2001 From: CNduka001 Date: Wed, 21 Oct 2026 07:18:19 +0100 Subject: [PATCH 1/2] fix: validate POST /admin/keys body with zod (#506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rates and quota cents (ratePerMin, ratePerDay, monthlyQuotaCents, dailyQuotaCents) went to the database exactly as sent: a negative, a zero, a fraction, or an out-of-range number were all accepted and only surfaced later as a key that could never call anything or was never limited. overagePolicy's TypeScript union is erased at runtime, so any string was stored and metering silently fell back to 'block'. Validate the body with zod and reject rather than coerce: - Every numeric field is a positive integer in [1, 2_147_483_647] (the Int column ceiling); zero, negatives and fractions are rejected. - overagePolicy is restricted to the three values metering understands. - ratePerDay must be >= ratePerMin — a per-day cap below the per-minute cap can never bind. - Errors go through the shared formatValidationError so they name the offending field, matching the other routes. Tests cover a negative, a fractional, a zero, an out-of-range value, an unknown policy, a contradictory rate pair and a blank label. Co-Authored-By: Claude Opus 4.8 --- src/__tests__/auth.test.ts | 69 +++++++++++++++++++++++++ src/api/admin.ts | 101 ++++++++++++++++++++++++++++++------- 2 files changed, 151 insertions(+), 19 deletions(-) diff --git a/src/__tests__/auth.test.ts b/src/__tests__/auth.test.ts index ea3237c4..3d57ed25 100644 --- a/src/__tests__/auth.test.ts +++ b/src/__tests__/auth.test.ts @@ -647,6 +647,75 @@ describe('admin endpoints', () => { expect(mockFindMany).not.toHaveBeenCalled() }) + // ── Body validation (issue #506): reject bad quotas/policies, don't coerce ── + async function postKey(payload: unknown) { + const app = await buildAdminApp() + return app.inject({ + method: 'POST', + url: '/admin/keys', + headers: { 'x-admin-token': 'admin-secret' }, + payload: payload as object, + }) + } + + it('rejects a negative rate and names the field, without touching the database', async () => { + const res = await postKey({ label: 'acme', ratePerMin: -5 }) + expect(res.statusCode).toBe(400) + expect(res.json().error).toMatch(/ratePerMin/) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it('rejects a fractional quota', async () => { + const res = await postKey({ label: 'acme', monthlyQuotaCents: 10.5 }) + expect(res.statusCode).toBe(400) + expect(res.json().error).toMatch(/monthlyQuotaCents/) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it('rejects a zero rate (a key that can never call anything)', async () => { + const res = await postKey({ label: 'acme', ratePerDay: 0 }) + expect(res.statusCode).toBe(400) + expect(res.json().error).toMatch(/ratePerDay/) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it('rejects an out-of-range value above the Int column ceiling', async () => { + const res = await postKey({ label: 'acme', dailyQuotaCents: 9_999_999_999 }) + expect(res.statusCode).toBe(400) + expect(res.json().error).toMatch(/dailyQuotaCents/) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it('rejects an unknown overagePolicy instead of silently defaulting to block', async () => { + const res = await postKey({ label: 'acme', overagePolicy: 'refund_everything' }) + expect(res.statusCode).toBe(400) + expect(res.json().error).toMatch(/overagePolicy/) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it('rejects a per-day rate below the per-minute rate', async () => { + const res = await postKey({ label: 'acme', ratePerMin: 100, ratePerDay: 50 }) + expect(res.statusCode).toBe(400) + expect(res.json().error).toMatch(/ratePerDay/) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it('rejects a blank (whitespace-only) label', async () => { + const res = await postKey({ label: ' ' }) + expect(res.statusCode).toBe(400) + expect(res.json().error).toMatch(/label/) + expect(mockCreate).not.toHaveBeenCalled() + }) + + it('accepts valid rates and policy at the boundary', async () => { + mockCreate.mockImplementation(async ({ data }: any) => ({ + id: 'new-id', createdAt: new Date(), ratePerMin: 60, ratePerDay: 10000, ...data, + })) + const res = await postKey({ label: 'acme', ratePerMin: 60, ratePerDay: 60, overagePolicy: 'charge_402' }) + expect(res.statusCode).toBe(201) + expect(mockCreate).toHaveBeenCalledTimes(1) + }) + it('revokes a key by id', async () => { mockFindUnique.mockResolvedValue({ id: 'key-1', revokedAt: null }) mockUpdate.mockResolvedValue({ id: 'key-1', revokedAt: new Date() }) diff --git a/src/api/admin.ts b/src/api/admin.ts index 295636a1..69f490dc 100644 --- a/src/api/admin.ts +++ b/src/api/admin.ts @@ -1,7 +1,9 @@ import type { FastifyInstance, FastifyRequest, FastifyReply } from 'fastify' import { randomBytes } from 'crypto' +import { z } from 'zod' import { prisma } from '../db' import { admin_auth_failures_total } from '../metrics' +import { formatValidationError } from '../validation' import { hashApiKey, isAdminAuthorized, checkAdminAuth, invalidateApiKey } from './auth' import { copyUsageHistory } from '../x402/metering' @@ -25,6 +27,80 @@ interface RotateKeyBody { overlapHours?: number } +/** The only overage policies `src/x402/metering.ts` understands. */ +const OVERAGE_POLICIES = ['block', 'charge_402', 'allow_overage'] as const + +// Rates and quota cents are stored in Postgres `Int` columns (32-bit signed), so +// their value cannot exceed this without overflowing the column anyway. +const MAX_INT32 = 2_147_483_647 + +/** + * A rate or quota: a positive integer within the `Int` column's range. Rejects + * zero, negatives, and fractions rather than coercing them, so a mistake fails + * loudly at creation instead of silently minting a key that can never call + * anything or one that is never limited. + */ +const quotaField = (name: string) => + z + .number({ error: `${name} must be a positive integer` }) + .int({ error: `${name} must be a positive integer` }) + .min(1, { error: `${name} must be a positive integer` }) + .max(MAX_INT32, { error: `${name} must be at most ${MAX_INT32}` }) + +/** + * Validates the `POST /admin/keys` body and rejects rather than coerces. Every + * field a caller can send is checked here so nothing reaches the database + * unvalidated (see issue #506). + */ +const CreateKeySchema = z + .object({ + label: z.string({ error: 'label is required' }).trim().min(1, { error: 'label is required' }), + expiresAt: z.string({ error: 'expiresAt must be a valid future date' }).optional(), + ratePerMin: quotaField('ratePerMin').optional(), + ratePerDay: quotaField('ratePerDay').optional(), + monthlyQuotaCents: quotaField('monthlyQuotaCents').optional(), + dailyQuotaCents: quotaField('dailyQuotaCents').optional(), + overagePolicy: z + .enum(OVERAGE_POLICIES, { + error: `overagePolicy must be one of: ${OVERAGE_POLICIES.join(', ')}`, + }) + .optional(), + // Scopes are optional, but an unknown-key strip would silently mint a key + // with no scopes at all — so they must be in the schema. The transform does + // the same de-dupe/trim/drop-empty normalisation the route used to do by + // hand, keeping a single source of truth for what gets stored (issue #506). + scopes: z + .array(z.string(), { error: 'scopes must be an array of strings' }) + .optional() + .transform((s) => + s === undefined ? undefined : Array.from(new Set(s.map((v) => v.trim()).filter(Boolean))), + ), + }) + .superRefine((data, ctx) => { + if (data.expiresAt !== undefined) { + const t = new Date(data.expiresAt).getTime() + if (!Number.isFinite(t) || t <= Date.now()) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['expiresAt'], + message: 'expiresAt must be a valid future date', + }) + } + } + // A per-day cap below the per-minute cap can never bind and is always a mistake. + if ( + data.ratePerMin !== undefined && + data.ratePerDay !== undefined && + data.ratePerDay < data.ratePerMin + ) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['ratePerDay'], + message: 'ratePerDay must be greater than or equal to ratePerMin', + }) + } + }) + /** * Registers admin-only endpoints for minting, listing, and revoking API keys. * @@ -76,39 +152,26 @@ export async function registerAdminRoutes(app: FastifyInstance) { }) } - const { label, expiresAt, ratePerMin, ratePerDay, monthlyQuotaCents, dailyQuotaCents, overagePolicy, scopes } = req.body ?? {} - if (!label || typeof label !== 'string' || label.trim().length === 0) { - return reply.status(400).send({ error: 'label is required' }) - } - if (expiresAt !== undefined && typeof expiresAt !== 'string') { - return reply.status(400).send({ error: 'expiresAt must be a valid future date' }) + const parsed = CreateKeySchema.safeParse(req.body ?? {}) + if (!parsed.success) { + return reply.status(400).send(formatValidationError(parsed.error)) } + const { label, expiresAt, ratePerMin, ratePerDay, monthlyQuotaCents, dailyQuotaCents, overagePolicy, scopes } = parsed.data const expiration = expiresAt === undefined ? undefined : new Date(expiresAt) - if (expiration && (!Number.isFinite(expiration.getTime()) || expiration <= new Date())) { - return reply.status(400).send({ error: 'expiresAt must be a valid future date' }) - } - if (scopes !== undefined) { - if (!Array.isArray(scopes) || scopes.some(s => typeof s !== 'string')) { - return reply.status(400).send({ error: 'scopes must be an array of strings' }) - } - } - const normalizedScopes = scopes !== undefined - ? Array.from(new Set(scopes.map(s => s.trim()).filter(Boolean))) - : undefined // Generate the key, store only its hash, and return the plaintext once. const plaintext = generateApiKey() const record = await prisma.apiKey.create({ data: { hash: hashApiKey(plaintext), - label: label.trim(), + label, ...(expiration ? { expiresAt: expiration } : {}), ...(ratePerMin !== undefined ? { ratePerMin } : {}), ...(ratePerDay !== undefined ? { ratePerDay } : {}), ...(monthlyQuotaCents !== undefined ? { monthlyQuotaCents } : {}), ...(dailyQuotaCents !== undefined ? { dailyQuotaCents } : {}), ...(overagePolicy !== undefined ? { overagePolicy } : {}), - ...(normalizedScopes !== undefined ? { scopes: normalizedScopes } : {}), + ...(scopes !== undefined ? { scopes } : {}), }, }) From 71b1400e22df39df978106744033c98c99603c75 Mon Sep 17 00:00:00 2001 From: CNduka001 Date: Sat, 24 Oct 2026 10:22:20 +0100 Subject: [PATCH 2/2] fix(openapi): define missing BadRequest response so client:gen succeeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /rates endpoint (added by #453) references #/components/responses/BadRequest, but that response component was never defined. openapi-typescript aborts with 'Can't resolve $ref at #/paths/~1rates/get/responses/400', so 'npm run client:gen' — the first step of CI's Typecheck & build job — fails before tsc runs, turning the check red for every open PR. Define BadRequest with the same minimal shape every other 400 in the spec uses (description: Invalid parameters) and regenerate the committed artifacts: - openapi.json via openapi:gen - clients/typescript/src/schema.d.ts via client:gen, which had gone stale (missing the whole /rates path) precisely because generation was aborting, so the 'git diff --exit-code -- clients/typescript' gate could never pass either. Co-Authored-By: Claude Opus 4.8 --- clients/typescript/src/schema.d.ts | 88 ++++++++++++++++++++++++++++++ openapi.json | 3 + openapi.yaml | 2 + 3 files changed, 93 insertions(+) diff --git a/clients/typescript/src/schema.d.ts b/clients/typescript/src/schema.d.ts index 6b07abba..38095490 100644 --- a/clients/typescript/src/schema.d.ts +++ b/clients/typescript/src/schema.d.ts @@ -705,10 +705,60 @@ export interface paths { patch?: never; trace?: never; }; + "/rates": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** + * Query latest lending and borrow rates across pools + * @description Returns the latest supply and borrow rates, utilisation, and reserves for lending markets. + */ + get: operations["getLendingRates"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; } export type webhooks = Record; export interface components { schemas: { + LendingRateEntry: { + id: string; + /** @example blend */ + venue: string; + /** @example blend */ + source: string; + /** @example mainnet */ + network: string; + poolId: string; + poolName?: string | null; + /** @example USDC */ + asset: string; + assetAddress?: string | null; + supplyApy: number; + borrowApy: number; + supplyApr?: number | null; + borrowApr?: number | null; + /** @description Borrowed / supplied as a fraction. Null for venues that publish rates only. */ + utilization?: number | null; + totalSupply?: number | null; + totalBorrowed?: number | null; + reserves?: number | null; + ledger?: number; + /** Format: date-time */ + timestamp: string; + }; + LendingRatesResponse: { + network: string; + rates: components["schemas"]["LendingRateEntry"][]; + }; InternalErrorResponse: { /** * @example internal_error @@ -1456,6 +1506,13 @@ export interface components { }; }; responses: { + /** @description Invalid parameters */ + BadRequest: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; /** @description Payment Required */ PaymentRequired: { headers: { @@ -3118,4 +3175,35 @@ export interface operations { 402: components["responses"]["PaymentRequired"]; }; }; + getLendingRates: { + parameters: { + query?: { + /** @description Filter by asset symbol (e.g. USDC, XLM) */ + asset?: string; + /** @description Filter by pool contract ID */ + poolId?: string; + /** @description Stellar network to query (testnet or mainnet) */ + network?: "testnet" | "mainnet"; + /** @description Maximum number of rate records to return */ + limit?: number; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Latest lending rates */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["LendingRatesResponse"]; + }; + }; + 400: components["responses"]["BadRequest"]; + 429: components["responses"]["TooManyRequests"]; + }; + }; } diff --git a/openapi.json b/openapi.json index 0c401f5e..3e9890dc 100644 --- a/openapi.json +++ b/openapi.json @@ -2904,6 +2904,9 @@ } }, "responses": { + "BadRequest": { + "description": "Invalid parameters" + }, "PaymentRequired": { "description": "Payment Required", "content": { diff --git a/openapi.yaml b/openapi.yaml index 72d5778b..eeb4de41 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1974,6 +1974,8 @@ components: default: testnet responses: + BadRequest: + description: Invalid parameters PaymentRequired: description: Payment Required content: