Api key guard hammers

context/progress-tracker.md

@@ -6,6 +6,49 @@ pure chore/docs commits). Direct pushes to main must also be logged here.
 
 ---
 
+## 2026-08-28
+
+- Fixed TOCTOU nonce reuse in `AuthService.verifySignature()` (src/modules/auth/auth.service.ts:111):
+  - **Atomic nonce consumption** — replaced `SELECT → verify → UPDATE` with atomic
+    conditional claim `UPDATE nonces SET used_at = now() WHERE id = ? AND used_at IS NULL`
+    executed **before** signature verification. Only the winner of the race gets
+    `count === 1` / `data.length === 1`; losers get `count === 0` and are rejected
+    with `AUTH_NONCE_NOT_FOUND`. This guarantees a given `(wallet, nonce)` can
+    produce at most one successful verification ever, even under concurrent
+    `POST /auth/verify` requests carrying the same stolen pair.
+  - **Burn-on-failure tradeoff documented in code** — if verification fails
+    (invalid signature, bad StrKey, or `expires_at` in the past) the nonce stays
+    burned. The caller must request a fresh nonce; this converts replay attacks
+    into DoS-on-self (one wasted challenge) versus unlimited session creation.
+    Chosen over RPC/locking because a single conditional `UPDATE` is natively atomic
+    in Postgres and fits the existing `SupabaseService.getServiceRoleClient()`
+    pattern without a new migration.
+  - **Per-wallet throttling on `POST /auth/verify`** — new `AuthWalletThrottlerGuard`
+    (src/modules/auth/auth-throttler.guard.ts) keys `@nestjs/throttler` on
+    `req.body.wallet` (fallback to `req.user.wallet` / IP) and is applied via
+    `@UseGuards(AuthWalletThrottlerGuard)` alongside the existing global
+    IP-based `ThrottlerGuard`. Route limit stays `5 req / 60 s` per wallet **and**
+    per IP, preventing offline-style brute force of the SEP-0043 fallback space
+    at network speed. `WalletThrottlerGuard` was also hardened to type-check
+    wallet strings and accept `body.wallet` so the same infrastructure is reused.
+  - **Tests** — extended `test/unit/modules/auth/auth.service.spec.ts` to prove
+    atomicity: parallel double-verify → exactly one success, replay after success
+    fails, replay after failure stays burned (`AUTH_SIGNATURE_INVALID` → `AUTH_NONCE_NOT_FOUND`),
+    expired nonce rejected and stays burned, atomic race via `count === 0` rejected.
+    Added `test/unit/modules/auth/auth-throttler.guard.spec.ts` for the wallet-keyed
+    throttler and updated `auth.controller.spec.ts` to mock the guard. `npm run build`
+    and `npm test` green (38 suites, 425 tests).
+
+- Hardened `ApiKeyGuard` hot path (`src/auth/guards/api-key.guard.ts:29`):
+  - **Cache key records by hash** — `CACHE_MANAGER` (Redis via `cache-manager` + `ioredis`, same pattern as `src/modules/liquidity/liquidity.service.ts:54` and `src/modules/transactions/transactions.service.ts:121`) stores `ApiKeyRecord` under `apikey:record:<keyHash>` (never the raw key) with `60s` TTL. Steady-state vendor traffic now causes ≤1 `SELECT` per TTL per key instead of 2 DB round-trips per request (lookup + unconditional `last_used_at` update). Negative lookups are not cached to avoid polluting the store; enumeration is handled by unified errors.
+  - **Collapsed `last_used_at` writes** — cache-guarded dirty flag `apikey:last_used:<keyId>` with `300s` TTL ensures at-most-once-per-5-minutes-per-key DB `UPDATE`, eliminating 1:1 write amplification. Fire-and-forget `maybeUpdateLastUsed()` logs but never blocks the request.
+  - **Normalized failure responses** — `API_KEY_INVALID`, `API_KEY_INACTIVE`, `API_KEY_EXPIRED`, and missing/malformed headers all map to a single `API_KEY_UNAUTHORIZED` (401) with `message: 'Invalid API key.'`. Server-side `Logger.warn` retains distinct reasons (`hash 8-char prefix`, `keyId`) for forensics, preventing enumeration of revoked vs expired vs nonexistent keys. `API_KEY_INSUFFICIENT_PERMISSIONS` (403) and `API_KEY_RATE_LIMITED` (429) remain distinct.
+  - **Per-key sliding-window rate limiting** — cache-backed counter `apikey:rate:<keyId>` with `60s` window and `60` req limit (structured `429` `API_KEY_RATE_LIMITED` via `HttpException`). Wired through the repo's established `CACHE_MANAGER` guard pattern (not a new BullMQ queue), consistent with `ThrottlerGuard` per-wallet limits. Trips and resets with TTL are tested.
+  - **Revocation invalidation** — `VendorsService.revokeApiKey()` (`src/modules/vendors/vendors.service.ts:636`) now selects `key_hash` alongside `id`, performs the `is_active=false` update, then `await cacheManager.del` for `apikey:record:<hash>`, `apikey:rate:<keyId>`, and `apikey:last_used:<keyId>`, guaranteeing visibility within one TTL. `VendorsService` now injects `CACHE_MANAGER` (`@Inject(CACHE_MANAGER)`) and `src/app.module.ts:13` registers a global `CacheModule` (`isGlobal: true`) via `getRedisConfig` so `ApiKeyGuard` and `VendorsService` share the same Redis/in-memory store.
+  - **Tests** — rewrote `test/unit/modules/auth/api-key.guard.spec.ts:7` to assert cache hit avoids DB (mock `select` call counts and `never store full keys`), revocation invalidation (manual `del` then DB re-check), rate-limit trips (`60` → `429`) and resets after TTL, and enumeration uniformity (missing/invalid/inactive/expired all `API_KEY_UNAUTHORIZED`). Updated `test/unit/modules/vendors/vendors.service.spec.ts:14` to provide `CACHE_MANAGER` mock and verify `revokeApiKey` deletes the three cache keys and tolerates cache failures. `npm run build` and `npm test` green (38 suites, 434 tests).
+
+---
+
 ## 2026-08-27
 
 - Closed the audit gaps on `POST /transactions/submit` (#117):

src/app.module.ts

@@ -1,8 +1,10 @@
 import { MiddlewareConsumer, Module, NestModule, OnModuleInit } from '@nestjs/common';
 import { APP_GUARD, APP_FILTER, APP_INTERCEPTOR } from '@nestjs/core';
-import { ConfigModule } from '@nestjs/config';
+import { ConfigModule, ConfigService } from '@nestjs/config';
+import { CacheModule } from '@nestjs/cache-manager';
 import { ThrottlerModule, ThrottlerGuard } from '@nestjs/throttler';
 import { ScheduleModule } from '@nestjs/schedule';
+import { getRedisConfig } from './config/redis.config';
 import { SentryModule, SentryGlobalFilter } from '@sentry/nestjs/setup';
 import { AuthModule } from './modules/auth/auth.module';
 import { HealthModule } from './modules/health/health.module';
@@ -36,6 +38,12 @@ import { AuditInterceptor } from './common/interceptors/audit.interceptor';
 @Module({
   imports: [
     ConfigModule.forRoot({ isGlobal: true }),
+    CacheModule.registerAsync({
+      isGlobal: true,
+      imports: [ConfigModule],
+      inject: [ConfigService],
+      useFactory: getRedisConfig,
+    }),
     SentryModule.forRoot(),
     ScheduleModule.forRoot(),
     LoggerModule,

src/auth/guards/api-key.guard.ts

@@ -4,9 +4,14 @@ import {
   ExecutionContext,
   UnauthorizedException,
   ForbiddenException,
+  HttpException,
+  HttpStatus,
   Inject,
+  Logger,
 } from '@nestjs/common';
 import { Reflector } from '@nestjs/core';
+import { CACHE_MANAGER } from '@nestjs/cache-manager';
+import { Cache } from 'cache-manager';
 import { createHash } from 'crypto';
 import { SupabaseService } from '../../database/supabase.client';
 import { API_KEY_PERMISSIONS_KEY } from './api-key-permissions.decorator';
@@ -25,60 +30,180 @@ interface ApiKeyRecord {
   updated_at: string;
 }
 
+/**
+ * Cache and rate-limit constants.
+ *
+ * - API_KEY_CACHE_TTL: short TTL for key records (≤1 DB lookup per TTL per key in steady state).
+ * - API_KEY_LAST_USED_TTL: collapse last_used_at writes to at-most-once-per-N-minutes-per-key.
+ * - API_KEY_RATE_LIMIT_* : per-key sliding-window (counts live in cache-manager, not DB).
+ * - API_KEY_NEGATIVE_CACHE_TTL: short TTL for non-existent key hashes to avoid DB hammer
+ *   on random-key floods. Intentionally small so enumeration attempts are still
+ *   eventually re-checked.
+ * - API_KEY_IP_RATE_LIMIT_* : per-IP burst protection for unauthenticated/unknown-key
+ *   floods. Complements the global ThrottlerGuard (100 req/60s per IP) with a
+ *   tighter per-IP budget for the API-key hot path so a single IP cannot
+ *   saturate the Supabase pool with random keys.
+ */
+const API_KEY_CACHE_TTL_SECONDS = 60;
+const API_KEY_LAST_USED_TTL_SECONDS = 300;
+const API_KEY_RATE_LIMIT_WINDOW_SECONDS = 60;
+const API_KEY_RATE_LIMIT_MAX_REQUESTS = 60;
+const API_KEY_NEGATIVE_CACHE_TTL_SECONDS = 30;
+const API_KEY_IP_RATE_LIMIT_WINDOW_SECONDS = 60;
+const API_KEY_IP_RATE_LIMIT_MAX_REQUESTS = 30;
+
+function getRecordCacheKey(keyHash: string): string {
+  return `apikey:record:${keyHash}`;
+}
+
+function getNegativeCacheKey(keyHash: string): string {
+  return `apikey:negative:${keyHash}`;
+}
+
+function getLastUsedCacheKey(keyId: string): string {
+  return `apikey:last_used:${keyId}`;
+}
+
+function getRateLimitCacheKey(keyId: string): string {
+  return `apikey:rate:${keyId}`;
+}
+
+function getIpRateLimitCacheKey(ip: string): string {
+  return `apikey:ip:rate:${ip}`;
+}
+
 @Injectable()
 export class ApiKeyGuard implements CanActivate {
+  private readonly logger = new Logger(ApiKeyGuard.name);
+
   constructor(
+    @Inject(CACHE_MANAGER) private readonly cacheManager: Cache,
     private readonly supabaseService: SupabaseService,
     private readonly reflector: Reflector,
   ) {}
 
+  private getClientIp(request: { headers: Record<string, string | string[] | undefined>; ip?: string }): string {
+    const forwarded = request.headers['x-forwarded-for'];
+    if (typeof forwarded === 'string' && forwarded.length > 0) {
+      return forwarded.split(',')[0].trim();
+    }
+    if (Array.isArray(forwarded) && forwarded.length > 0 && typeof forwarded[0] === 'string') {
+      return forwarded[0].split(',')[0].trim();
+    }
+    if (typeof request.ip === 'string' && request.ip.length > 0) return request.ip;
+    return 'unknown';
+  }
+
   async canActivate(context: ExecutionContext): Promise<boolean> {
     const request = context.switchToHttp().getRequest<{
       headers: Record<string, string | string[] | undefined>;
+      ip?: string;
       apiKey?: ApiKeyRecord;
     }>();
 
     const apiKeyHeader = request.headers['x-api-key'];
+    const clientIp = this.getClientIp(request as unknown as { headers: Record<string, string | string[] | undefined>; ip?: string });
 
     if (!apiKeyHeader || typeof apiKeyHeader !== 'string') {
+      // Even missing/malformed keys count toward per-IP burst protection so a
+      // headerless flood cannot bypass the DB guard.
+      await this.enforceIpRateLimit(clientIp);
+      this.logger.warn('API key missing or malformed header');
       throw new UnauthorizedException({
-        code: 'API_KEY_MISSING',
-        message: 'X-API-Key header is required.',
+        code: 'API_KEY_UNAUTHORIZED',
+        message: 'Invalid API key.',
       });
     }
 
     const keyHash = createHash('sha256').update(apiKeyHeader).digest('hex');
+    const recordCacheKey = getRecordCacheKey(keyHash);
+    const negativeCacheKey = getNegativeCacheKey(keyHash);
 
-    const client = this.supabaseService.getServiceRoleClient();
-    const { data, error } = await client
-      .from('api_keys')
-      .select('*')
-      .eq('key_hash', keyHash)
-      .single();
+    let keyRecord: ApiKeyRecord | undefined;
 
-    if (error || !data) {
-      throw new UnauthorizedException({
-        code: 'API_KEY_INVALID',
-        message: 'Invalid API key.',
-      });
+    try {
+      keyRecord = await this.cacheManager.get<ApiKeyRecord>(recordCacheKey);
+    } catch (error) {
+      this.logger.warn(`API key cache read failed for ${keyHash.slice(0, 8)}...: ${(error as Error).message}`);
     }
 
-    const keyRecord = data as unknown as ApiKeyRecord;
+    if (!keyRecord) {
+      // Negative cache hit: recently observed non-existent hash, avoid DB.
+      try {
+        const isNegative = await this.cacheManager.get<boolean>(negativeCacheKey);
+        if (isNegative) {
+          await this.enforceIpRateLimit(clientIp);
+          this.logger.warn(`API key negative-cache hit for hash ${keyHash.slice(0, 8)}... (IP ${clientIp})`);
+          throw new UnauthorizedException({
+            code: 'API_KEY_UNAUTHORIZED',
+            message: 'Invalid API key.',
+          });
+        }
+      } catch (error) {
+        if (error instanceof UnauthorizedException) throw error;
+        this.logger.warn(`API key negative-cache read failed for ${keyHash.slice(0, 8)}...: ${(error as Error).message}`);
+      }
+
+      // Per-IP burst protection before hitting Supabase for unknown keys.
+      await this.enforceIpRateLimit(clientIp);
+
+      const client = this.supabaseService.getServiceRoleClient();
+      const { data, error } = await client.from('api_keys').select('*').eq('key_hash', keyHash).single();
+
+      if (error || !data) {
+        // Cache the negative result briefly so a random-key flood does not
+        // hammer the DB once per request. TTL is intentionally short.
+        try {
+          await this.cacheManager.set(negativeCacheKey, true, API_KEY_NEGATIVE_CACHE_TTL_SECONDS);
+        } catch (cacheError) {
+          this.logger.warn(`API key negative-cache write failed for ${keyHash.slice(0, 8)}...: ${(cacheError as Error).message}`);
+        }
+        this.logger.warn(
+          `API key lookup failed for hash ${keyHash.slice(0, 8)}...: ${error?.message ?? 'not found'}`,
+        );
+        throw new UnauthorizedException({
+          code: 'API_KEY_UNAUTHORIZED',
+          message: 'Invalid API key.',
+        });
+      }
+
+      keyRecord = data as unknown as ApiKeyRecord;
+    }
 
+    // Unified validation: is_active and expires_at both map to the same
+    // API_KEY_UNAUTHORIZED response to prevent enumeration of revoked vs
+    // expired vs nonexistent keys. Details are logged server-side only.
     if (!keyRecord.is_active) {
+      this.logger.warn(`API key inactive: ${keyRecord.id} (hash ${keyHash.slice(0, 8)}...)`);
       throw new UnauthorizedException({
-        code: 'API_KEY_INACTIVE',
-        message: 'API key has been revoked.',
+        code: 'API_KEY_UNAUTHORIZED',
+        message: 'Invalid API key.',
       });
     }
 
     if (keyRecord.expires_at && new Date(keyRecord.expires_at) < new Date()) {
+      this.logger.warn(`API key expired: ${keyRecord.id} (hash ${keyHash.slice(0, 8)}...)`);
       throw new UnauthorizedException({
-        code: 'API_KEY_EXPIRED',
-        message: 'API key has expired.',
+        code: 'API_KEY_UNAUTHORIZED',
+        message: 'Invalid API key.',
       });
     }
 
+    // Cache the validated record for steady-state traffic (≤1 lookup per TTL per key).
+    // Only cache after successful validation so inactive/expired records are not
+    // served from cache; revocation explicitly invalidates via VendorsService.
+    try {
+      const cached = await this.cacheManager.get<ApiKeyRecord>(recordCacheKey);
+      if (!cached) {
+        await this.cacheManager.set(recordCacheKey, keyRecord, API_KEY_CACHE_TTL_SECONDS);
+      }
+    } catch (error) {
+      this.logger.warn(`API key cache write failed for ${keyRecord.id}: ${(error as Error).message}`);
+    }
+
+    // Per-key sliding-window rate limit (cache-backed, not DB).
+    await this.enforceRateLimit(keyRecord.id, keyHash);
+
     const requiredPermissions = this.reflector.get<string[]>(
       API_KEY_PERMISSIONS_KEY,
       context.getHandler(),
@@ -95,21 +220,79 @@ export class ApiKeyGuard implements CanActivate {
       }
     }
 
-    this.updateLastUsed(keyRecord.id);
+    // Throttled last_used_at: at-most-once-per-N-minutes-per-key.
+    // Fire-and-forget is intentionally not awaited to avoid adding latency to
+    // the hot path; errors are logged.
+    void this.maybeUpdateLastUsed(keyRecord.id);
 
     request.apiKey = keyRecord;
     return true;
   }
 
-  private async updateLastUsed(keyId: string): Promise<void> {
+  private async enforceRateLimit(keyId: string, keyHash: string): Promise<void> {
+    const rateKey = getRateLimitCacheKey(keyId);
+    try {
+      const current = (await this.cacheManager.get<number>(rateKey)) ?? 0;
+      if (current >= API_KEY_RATE_LIMIT_MAX_REQUESTS) {
+        this.logger.warn(
+          `API key rate limited: ${keyId} (hash ${keyHash.slice(0, 8)}...) — ${current}/${API_KEY_RATE_LIMIT_MAX_REQUESTS} per ${API_KEY_RATE_LIMIT_WINDOW_SECONDS}s`,
+        );
+        throw new HttpException(
+          {
+            code: 'API_KEY_RATE_LIMITED',
+            message: 'Too many requests for this API key. Please retry after a short delay.',
+          },
+          HttpStatus.TOO_MANY_REQUESTS,
+        );
+      }
+      const next = current + 1;
+      // Sliding window: each increment resets TTL to full window. For a fixed
+      // window we would preserve the original TTL, but sliding is simpler and
+      // matches the per-key burst protection needed here.
+      await this.cacheManager.set(rateKey, next, API_KEY_RATE_LIMIT_WINDOW_SECONDS);
+    } catch (error) {
+      if (error instanceof HttpException) throw error;
+      // Cache failures should not block legitimate traffic; log and allow.
+      this.logger.warn(`API key rate-limit cache error for ${keyId}: ${(error as Error).message}`);
+    }
+  }
+
+  private async enforceIpRateLimit(ip: string): Promise<void> {
+    // Skip IP throttling for unknown IP (conservative: allow rather than block).
+    if (!ip || ip === 'unknown') return;
+    const ipKey = getIpRateLimitCacheKey(ip);
     try {
+      const current = (await this.cacheManager.get<number>(ipKey)) ?? 0;
+      if (current >= API_KEY_IP_RATE_LIMIT_MAX_REQUESTS) {
+        this.logger.warn(`API key IP rate limited: ${ip} — ${current}/${API_KEY_IP_RATE_LIMIT_MAX_REQUESTS} per ${API_KEY_IP_RATE_LIMIT_WINDOW_SECONDS}s`);
+        throw new HttpException(
+          {
+            code: 'API_KEY_RATE_LIMITED',
+            message: 'Too many requests for this API key. Please retry after a short delay.',
+          },
+          HttpStatus.TOO_MANY_REQUESTS,
+        );
+      }
+      const next = current + 1;
+      await this.cacheManager.set(ipKey, next, API_KEY_IP_RATE_LIMIT_WINDOW_SECONDS);
+    } catch (error) {
+      if (error instanceof HttpException) throw error;
+      this.logger.warn(`API key IP rate-limit cache error for ${ip}: ${(error as Error).message}`);
+    }
+  }
+
+  private async maybeUpdateLastUsed(keyId: string): Promise<void> {
+    const lastUsedKey = getLastUsedCacheKey(keyId);
+    try {
+      const flagged = await this.cacheManager.get<boolean>(lastUsedKey);
+      if (flagged) {
+        return;
+      }
       const client = this.supabaseService.getServiceRoleClient();
-      await client
-        .from('api_keys')
-        .update({ last_used_at: new Date().toISOString() })
-        .eq('id', keyId);
-    } catch {
-      // Fire-and-forget — failure to update last_used_at should not block the request
+      await client.from('api_keys').update({ last_used_at: new Date().toISOString() }).eq('id', keyId);
+      await this.cacheManager.set(lastUsedKey, true, API_KEY_LAST_USED_TTL_SECONDS);
+    } catch (error) {
+      this.logger.warn(`Failed to update last_used_at for ${keyId}: ${(error as Error).message}`);
     }
   }
 }

src/config/redis.config.ts

@@ -21,8 +21,15 @@ export const getRedisConfig = async (configService: ConfigService): Promise<Redi
   const redisUrl = configService.get<string>('REDIS_URL');
   const ttl = configService.get<number>('REPUTATION_CACHE_TTL', 300);
 
-  // If we are in test mode or no Redis URL is provided, fall back to in-memory store
+  // If we are in test mode or no Redis URL is provided, fall back to in-memory store.
+  // In-memory is per-instance and therefore NOT suitable for ApiKeyGuard's
+  // shared invalidation/rate-limiting (revocation and 429 counters become
+  // per-instance). Production must set REDIS_URL; we warn loudly when falling
+  // back outside tests.
   if (isTest || !redisUrl) {
+    if (!isTest && !redisUrl) {
+      pino().warn('REDIS_URL is not set — falling back to in-memory cache. ApiKeyGuard revocation and per-key/per-IP rate limiting will be per-instance only and should not be used in production.');
+    }
     return {
       ttl,
     };