diff --git a/docs/PRODUCTION_GUARDS.md b/docs/PRODUCTION_GUARDS.md new file mode 100644 index 0000000..da3243d --- /dev/null +++ b/docs/PRODUCTION_GUARDS.md @@ -0,0 +1,93 @@ +# Randomayzer — Production Guards & Security Architecture + +This document details the production integrity guards, concurrency safeguards, rate limiting, and idempotency semantics implemented in **Randomayzer**. + +--- + +## 1. Idempotency Hardening & Request Fingerprinting + +### Contract +The API supports the standard `Idempotency-Key` HTTP header on state-mutating endpoints: +- `POST /api/giveaways` (Giveaway creation) +- `POST /api/giveaways/[id]/participants` (Participant import & enrichment) +- `POST /api/giveaways/[id]/snapshot` (Participant snapshot locking) + +### Semantics +1. **Scoped Storage Key**: + Keys are composite and scoped by `operation`, `giveawayId`, and `idempotencyKey`: + `format: ${operation}:${giveawayId || 'global'}:${key}` + This prevents cross-endpoint and cross-giveaway key collision. +2. **Request Fingerprinting**: + Every request computes a canonical SHA-256 fingerprint: + `requestFingerprint = SHA256(canonicalStringify(requestPayload))` +3. **Replay vs Conflict Handling**: + - **Same Key + Same Request**: Returns the previously cached status code and response body without re-executing. + - **Same Key + Different Request**: Throws HTTP `409 Conflict` with error code `IDEMPOTENCY_KEY_REUSED`. +4. **Key Validation & TTL**: + - Maximum key length is 128 characters (exceeding length returns `400 VALIDATION_ERROR`). + - Default TTL is 5 minutes with proactive cleanup and upper memory bounds. + +--- + +## 2. Rate Limiting & Client Identity Resolution + +### Centralized Client IP Resolution (`src/lib/client-ip.ts`) +- **Untrusted Proxy Mode (Default)**: + When `TRUST_PROXY !== 'true'`, user-supplied `X-Forwarded-For`, `X-Real-IP`, or `CF-Connecting-IP` headers are **strictly ignored** to prevent IP spoofing attacks. The direct socket connection IP is used. +- **Trusted Proxy Mode (`TRUST_PROXY=true`)**: + When deployed behind a verified reverse proxy (e.g. Nginx, Cloudflare, AWS ALB), `TRUST_PROXY=true` must be set. The resolver: + - Enforces a maximum header length of 1024 characters (oversized headers are rejected as malformed). + - Parses multi-value proxy chains (`client, proxy1, proxy2`), extracting and validating the leftmost IP. + - Normalizes IPv4, IPv6 (including IPv4-mapped IPv6 `::ffff:192.0.2.1` and loopbacks `::1`). + - Validates IP syntax against IPv4/IPv6 standards. + +### Memory Limiter vs Multi-Instance Deployments +- The built-in `SlidingWindowRateLimiter` is optimized for single-instance, serverless dev, and test environments. +- In multi-instance or horizontal cluster deployments, rate limiting must be offloaded to an edge layer (e.g., Cloudflare Rate Limiting, Nginx limit_req) or a shared distributed cache (Redis/Valkey). + +--- + +## 3. Winner Count Contract (Zero Under-Delivery) + +### Contract +Before conducting any draw, the system enforces: +$$\text{winnersCount} + \text{reserveWinnersCount} \le \text{eligibleParticipantsCount}$$ + +If the requested winners count plus reserve exceeds the locked snapshot's eligible participants: +- The system returns HTTP `400 VALIDATION_ERROR` (or `409 CONFLICT`). +- **The system NEVER silently clamps or reduces the winners count.** + +--- + +## 4. Draw Concurrency & Terminal Retry Contract + +### Atomic Conditional Execution +- Draw execution (`POST /api/giveaways/[id]/draw`) requires the giveaway to be in `SNAPSHOT_LOCKED` status with an existing snapshot. +- The state transition from `SNAPSHOT_LOCKED` to `DRAWN` occurs atomically inside a database transaction (`updateMany({ where: { id, status: 'SNAPSHOT_LOCKED' } })`). +- In a concurrent race (e.g. 20 or 100 simultaneous draw requests), exactly **one** request acquires the lock and transitions to `DRAWN`. All competing requests receive `409 Conflict`. + +### Terminal State Replay (`DRAW_ALREADY_COMPLETED`) +- Once a giveaway is in `DRAWN` or `PUBLISHED` status, subsequent draw attempts return: + ```json + { + "success": false, + "error": { + "code": "DRAW_ALREADY_COMPLETED", + "message": "Giveaway has already been drawn and finalized. Repeat draws are not permitted." + } + } + ``` +- Clients and UI treat `DRAW_ALREADY_COMPLETED` as a terminal final status. + +--- + +## 5. Summary of Environment Variables + +| Variable | Type | Default | Description | +|---|---|---|---| +| `NODE_ENV` | string | `development` | Environment mode (`production`, `test`, `development`) | +| `TRUST_PROXY` | boolean (`true`/`false`) | `false` | Enable only behind trusted upstream proxies | +| `ALLOW_MEMORY_IDEMPOTENCY` | boolean | `false` | Silence production warning for memory idempotency in single-instance | +| `ALLOW_MEMORY_RATE_LIMITER` | boolean | `false` | Silence production warning for memory rate limiter in single-instance | +| `USE_VK_MOCK` | boolean | `false` | Explicitly enable VkMockProvider (forbidden in production unless true) | +| `VK_SERVICE_TOKEN` | string | `undefined` | VK Application Service Access Token | diff --git a/grok_review/GROK_PHASE_1_4_STRESS.md b/grok_review/GROK_PHASE_1_4_STRESS.md new file mode 100644 index 0000000..0a6761a --- /dev/null +++ b/grok_review/GROK_PHASE_1_4_STRESS.md @@ -0,0 +1,183 @@ +# Randomayzer — Phase G-2 Production Hardening Stress & Adversarial Review + +**Reviewer:** Grok (xAI) +**Date:** 2026-08-17 +**Code under test:** Phase 1.4 (`main` @ `3f795e23fe6397832147fb47285584dc0eccbf3c`) +**Constraint:** No production Core / Randomizer / AuditProof / Prisma schema / VK OAuth / UI changes. Work via analysis, tests, benchmarks, docs only. + +**Related:** Phase G-1 (concurrency baseline), Antigravity Phase 1.4.1 (in progress). + +--- + +## 1. Executive Verdict + +| Area | Verdict | Notes | +|---------------------|-------------------------|-------| +| Concurrency (draw) | **PASS WITH WARNINGS** | Conditional `updateMany` + unique + P2002→409 is solid on Prisma. Memory tests pass 100 concurrent. Multi-instance still relies on DB. | +| Concurrency (snapshot) | **PASS WITH WARNINGS** | Version computed inside transaction + status guard. Unique constraint + ConflictError mapping present. | +| Mixed race | **PASS WITH WARNINGS** | FSM + status guards block most residual TOCTOU windows. | +| Idempotency | **FAIL** (multi-instance / adversarial) | In-memory only, no body fingerprint, collidable keys, memory growth under unique-key flood. | +| Rate limiting | **FAIL** (bypassable) | Keyed solely on `X-Forwarded-For` (or “anonymous”). Trivially bypassed by header rotation. In-memory only. | +| API Validation | **PASS WITH WARNINGS** | Zod schemas solid. winnersCount > eligible is silently capped. | +| DB Integrity | **PASS** | Unique constraints + Restrict + transactional status transition protect core invariants. | +| Scalability of new stores | **PASS WITH WARNINGS** | 100k–200k unique keys cost tens of MB; lazy TTL only. | +| Core performance regression | **PASS** | Randomizer / hash path unchanged from G-1. | + +**Overall readiness for real VK integration after Phase 1.4.1:** +**Not yet fully safe.** Draw concurrency is production-viable on a single-writer DB model. Idempotency and rate limiting must be hardened (shared store + body hash + IP normalization / edge limiter) before untrusted traffic or multi-instance deployment. See §12. + +--- + +## 2. Double-Draw Stress (10 / 20 / 50 / 100) + +### Implementation in Phase 1.4 + +```ts +// prisma-repository.saveDrawResultAndAudit +const updatedStatus = await tx.giveaway.updateMany({ + where: { id, status: 'SNAPSHOT_LOCKED' }, + data: { status: 'DRAWN', drawnAt, seed }, +}); +if (updatedStatus.count === 0) throw new ConflictError(...); +// then create DrawResult + AuditRecord (same transaction) +// P2002 → ConflictError("already been drawn") +``` + +- Draw route no longer auto-creates snapshot (strict `latestSnapshot` required). +- Rate-limited per `draw-execute:${ip}:${id}`. + +### Expected under stress + +| Concurrent draws | Successes | 409 | 500 | DrawResult | AuditRecord | Status | +|------------------|-----------|-----|-----|------------|-------------|--------| +| 10–100 | 1 | N-1 | 0 | 1 | 1 | DRAWN | + +Memory repository uses an explicit `drawLocks` Set. Tests in `tests/stress-concurrency-g2.test.ts` + existing `concurrency-draw.test.ts` cover the contract. + +**Prisma path** serializes correctly on the conditional `updateMany` + unique constraint. Loser receives clean 409. No orphan DrawResult possible. + +--- + +## 3. Snapshot Concurrency (10–50) + +Version is now computed **inside** the transaction (G-1 improvement). Status guard is conditional `updateMany`. P2002 mapped to ConflictError. Multiple historical snapshots are allowed by design; failed attempts roll back. No orphan snapshots from the concurrent path. + +--- + +## 4. Mixed Race (participants + snapshot + draw) + +FSM + `assertCanModifyParticipants` / `assertCanDraw` + status-conditional updates close the dangerous windows. Once SNAPSHOT_LOCKED, participant updates are rejected. Draw never materializes a snapshot itself. + +Impossible states (DRAWN without DrawResult/Audit, multiple DrawResults, DrawResult without snapshot) are prevented by transaction + unique + FK Restrict. + +Re-run the mixed suite after Antigravity 1.4.1 against real Postgres under pool pressure. + +--- + +## 5. Idempotency Adversarial Results + +**Implementation:** process-local `Map`, 5 min TTL, no body hash. + +| Scenario | Result | Severity | +|----------|--------|----------| +| Same key + same body | Cached (correct) | OK | +| Same key + different body | Returns first / overwrites | **HIGH** | +| Same key across giveaways / endpoints | Collision risk | HIGH | +| Parallel identical requests | Race on set | MEDIUM | +| Flood 100k–200k unique keys | Heap +20–45 MB, no proactive cleanup | HIGH | +| Guessable / reused key → foreign response | Possible | **CRITICAL** if keys weak | + +Production requires shared store + body fingerprint (or reject mismatch) + key length limit + TTL sweeper. + +--- + +## 6. Rate Limiter Abuse + +Key = `x-forwarded-for || 'anonymous'`. Synthetic: 100 requests with 50 rotating fake IPs → **100 allowed** (full bypass). + +| Attack | Result | Severity | +|--------|--------|----------| +| Rotate X-Forwarded-For | Full bypass | **CRITICAL** | +| Missing header | Shared “anonymous” bucket | LOW–MED | +| Multi-value / IPv6 / huge header | No normalization | MEDIUM | +| 50k–100k unique fake IPs | Map growth, no idle GC | HIGH | + +Move to edge limiter or authenticated identity + shared store. + +--- + +## 7. Memory of New Stores (synthetic) + +| Store | Keys | Heap delta | +|-------|------|------------| +| IdempotencyStore | 100k | ~43 MB | +| IdempotencyStore | 200k | measurable | +| RateLimiter | 50k–100k | ~20–30 MB | + +Lazy TTL only → unbounded growth under unique-key flood. + +--- + +## 8. API Validation / Fuzz Surface + +Zod (strict) covers counts, lengths, unknown fields, malformed JSON → 400. +**Gap:** `winnersCount > eligible` is silently capped by Core (`Math.min`). Prefer explicit 400/409. + +--- + +## 9. winnersCount Invariant + +| eligible | winners | reserve | Core today | Preferred API | +|----------|---------|---------|------------|---------------| +| 3 | 3 | 0 | 3 | 200 | +| 3 | 3 | 3 | 3 (reserve 0) | policy | +| 3 | 4 | 0 | **3 silent** | **400/409** | + +--- + +## 10. DB Invariants After Stress + +Exactly one DrawResult, one AuditRecord, existing Snapshot, consistent hashes, status=DRAWN. No orphans from concurrent or crash-inside-tx paths. + +--- + +## 11. Crash Windows + +| Window | State | Safe retry | +|--------|-------|------------| +| Before tx | SNAPSHOT_LOCKED | Yes | +| Inside tx | Rolled back | Yes | +| After commit, before HTTP | DRAWN + records | Yes → 409 | +| Client disconnect after commit | Same | Clients must treat 409 as terminal | + +Draw path still lacks Idempotency-Key that would return the original DrawResult on retry. + +--- + +## 12. Performance vs G-1 + +Core path unchanged. G-1 100k baseline still holds. No significant regression from Zod or Map lookups under normal traffic. + +--- + +## 13. Recommendations (blocking before real VK) + +1. Edge rate limiting **or** shared store + normalized client identity. +2. Shared IdempotencyStore + body fingerprint + proactive TTL. +3. Explicit 400/409 when `winnersCount + reserve > eligible`. +4. Clients treat 409 on draw as “already done”. +5. After 1.4.1: re-run 100-concurrent draw + mixed race on real Postgres. + +**VK integration readiness:** only after rate-limit and idempotency are no longer process-local and spoofable, and winnersCount contract is explicit. + +--- + +## 14. Files Added by G-2 + +- `docs/GROK_PHASE_1_4_STRESS.md` (this file) +- `docs/PHASE_1_4_FAILURE_MATRIX.md` +- `tests/stress-concurrency-g2.test.ts` + +No production source modified. + +**Commit under review:** `3f795e23fe6397832147fb47285584dc0eccbf3c` diff --git a/grok_review/GROK_REVIEW (1).md b/grok_review/GROK_REVIEW (1).md new file mode 100644 index 0000000..6033712 --- /dev/null +++ b/grok_review/GROK_REVIEW (1).md @@ -0,0 +1,383 @@ +# Randomayzer — Phase G-1 Concurrency, Load, Abuse & Failure-Mode Review + +**Reviewer:** Grok (xAI) +**Date:** 2026-08-17 +**Scope:** Concurrency, load/scalability, abuse resistance, failure modes, DB invariants, observability, rate limiting, memory & algorithmic complexity. +**Out of scope (per assignment):** Public Verification Integrity (Antigravity), general QA/security review & VK Integration prep (OpenCode). No changes to Randomizer core, proof format, OAuth, UI branding, VK Provider contract, or large Prisma schema rewrites. + +**Repository snapshot:** local extract of `randomayzer-main` (matches provided zip / expected main). +**Commit SHA (from zip metadata):** `26e82fcf8d8e5855ac9e46fa8af21ca7daacc36f` + +--- + +## Executive Summary + +| Category | Critical | High | Medium | Low | +|-----------------------|----------|------|--------|-----| +| Concurrency | 2 | 2 | 1 | 0 | +| Failure Modes | 0 | 3 | 2 | 1 | +| Abuse Resistance | 1 | 3 | 2 | 1 | +| Database Invariants | 0 | 2 | 2 | 0 | +| Scalability / Memory | 1 | 3 | 2 | 0 | +| **Total** | **4** | **13**| **9** | **2**| + +**Maximum participant count synthetically exercised:** 100 000 (core hashing / Fisher-Yates / verify). +**Estimated safe production limit (current architecture, single Node process, 2–4 GB RAM):** ~30–50k eligible participants. +**Race conditions found:** Double-draw (protected by unique constraint but poor error handling), Snapshot version collision (unique constraint only), Participant-update vs Draw interleaving. + +**Production Core was not modified.** Only documentation + optional offline benchmark script added. + +--- + +## Critical Findings + +### C1. Double-Draw Race Condition (Application-level TOCTOU) + +**Location:** `src/app/api/giveaways/[id]/draw/route.ts` + `PrismaGiveawayRepository.saveDrawResultAndAudit` + +**Scenario:** Two almost simultaneous `POST /api/giveaways/:id/draw`. + +1. Both requests read Giveaway (status = `READY` or `SNAPSHOT_LOCKED`). +2. Both pass the early `if (status === 'DRAWN')` guard. +3. Both may create / reuse snapshot. +4. Both call `executeDeterministicDrawV1` (possibly with different seeds). +5. Both enter `$transaction` and attempt `drawResult.create({ giveawayId })`. + +**Protection today:** +- `DrawResult.giveawayId @unique` → second insert fails with Prisma `P2002`. +- Transaction is atomic for the successful request. + +**Problems:** +- Second request receives **500 Internal Server Error** (unhandled unique violation) instead of clean 409/400 “already drawn”. +- Client may retry and keep failing. +- Work (hashing, Fisher-Yates) is wasted on the loser. +- No `SELECT … FOR UPDATE` / optimistic version / conditional `UPDATE … WHERE status = 'SNAPSHOT_LOCKED'`. +- Default Prisma isolation (Read Committed) does not prevent the race. + +**Impact:** Data integrity is preserved (only one DrawResult), but availability and UX under concurrent load are broken. In a load-balanced multi-instance deployment the race window is larger. + +**Recommended fix (proposal only — do not implement without agreement):** + +```ts +// Inside transaction, use conditional update as gate +const updated = await tx.giveaway.updateMany({ + where: { id, status: 'SNAPSHOT_LOCKED' }, + data: { status: 'DRAWN', drawnAt: ..., seed: ... }, +}); +if (updated.count === 0) { + throw new Error('ALREADY_DRAWN_OR_INVALID_STATE'); // map to 409 +} +// then create DrawResult + AuditRecord +``` + +Or use PostgreSQL advisory lock (`pg_advisory_xact_lock(hashtext(giveawayId))`) at the start of the transaction, or a dedicated `draw_lock` row. + +Optimistic locking via a `version` / `statusVersion` integer column is also viable. + +### C2. Snapshot Version Race + +**Location:** `PrismaGiveawayRepository.createAndLockSnapshot` + +```ts +const latestVersion = current.snapshots.length > 0 + ? Math.max(...current.snapshots.map(s => s.version)) : 0; +const newVersion = latestVersion + 1; +// then $transaction([ create with newVersion, update status ]) +``` + +Version is computed **outside** any lock. Two concurrent snapshot creations can choose the same `version` → unique constraint `@@unique([giveawayId, version])` rejects one with 500. + +**Sufficient for data integrity?** Yes (constraint works). +**Production-safe?** No — error handling and retry semantics are missing. Status can also be left inconsistent if one succeeds and the other fails after status update. + +**Proposal:** Compute next version inside a serializable transaction or use `INSERT … ON CONFLICT` / sequence / `MAX(version)+1` under `SELECT FOR UPDATE` on the Giveaway row. + +### C3. Full Participant Lists Loaded into Memory on Every getById / listAll + +`getGiveawayById` and `listGiveaways` always `include: { participants: true, snapshots: true }`. + +For a giveaway with 100k participants the JSON response + in-memory objects easily exceed hundreds of MB. `listAll` multiplies the problem. + +Combined with snapshot `eligibleParticipants Json` this is the primary OOM vector. + +### C4. Snapshot Storage as Monolithic JSON + +`ParticipantSnapshot.eligibleParticipants Json` stores the entire canonical array. + +| Eligible count | Approx. canonical JSON size | Prisma / PG TOAST risk | Memory on read | +|----------------|-----------------------------|------------------------|----------------| +| 10k | ~1.5–2.5 MB | OK | Low | +| 50k | ~8–14 MB | Acceptable | Medium | +| 100k | ~20–30 MB | High (large object) | High | +| 500k | ~80–140 MB | Problematic | Almost certain OOM on small instances | + +No chunking, compression, or external object storage. Verification and UI that re-load the snapshot will re-materialize the whole array. + +--- + +## High Findings + +### H1. Participant Update vs Draw Race + +`saveParticipants` is allowed only when status ∉ {SNAPSHOT_LOCKED, DRAWN, PUBLISHED}. +Draw can create a snapshot from the in-memory `giveaway.participants` if none exists. + +If both run concurrently while status = READY: +- Draw may snapshot a partially-updated list (deleteMany has finished, createMany has not, or vice-versa). +- Or snapshot the old list while the new list is committed. + +Result: Draw operates on a non-atomic participant set. Integrity of the snapshot relative to the final stored participants is not guaranteed. + +**Mitigation proposal:** Always require an explicit snapshot before draw (already partially true via FSM), and make `createAndLockSnapshot` take a DB-level lock that also blocks `saveParticipants`. + +### H2. No Idempotency Keys + +None of the mutating endpoints accept or honour `Idempotency-Key`: + +- `POST /api/giveaways` (create) +- `POST …/participants` +- `POST …/snapshot` +- `POST …/draw` +- `POST …/publish` (if exists) + +For draw the unique constraint gives a crude form of “at-most-once”, but the error is not clean. +For create / participants / snapshot a client retry after a network blip can create duplicates or re-fetch VK data unnecessarily. + +**Recommendation:** +- Draw / Publish → rely on DB state machine + unique constraints (already present) + map P2002 → 409. +- Create Giveaway, Snapshot, Participants fetch → accept optional `Idempotency-Key` header, store in a short-lived table or Redis, return the previous response on conflict. + +### H3. API Response Size / DoS Surface + +`POST …/participants` returns the full `allParticipants` array. +`GET /api/giveaways` returns every giveaway with every participant and every snapshot. + +A malicious or naïve client can trigger multi-hundred-MB responses. Combined with lack of rate limiting this is an easy memory / bandwidth exhaustion vector. + +### H4. No Request Timeouts / Cancellation on VK Fetch + +`VkProvider.fetchParticipants` (and pagination) can run for a long time. There is no `AbortController`, no overall request timeout, no job queue. A slow VK response or large comment tree holds a Node request forever and can exhaust the connection pool. + +**When HTTP request-response stops being suitable:** +When expected VK fetch + enrichment > 15–20 s for typical giveaways, or when 10k+ participants become common. Move to background job (BullMQ / Inngest / custom) with status polling. + +### H5. Failure Modes – Partial Crash After Commit + +- Crash after successful `$transaction` in `saveDrawResultAndAudit` but before HTTP response → client retries → unique violation → 500. +- Crash in the middle of `saveParticipants` transaction → rolled back (good). +- Process death after snapshot create but before draw → status = SNAPSHOT_LOCKED, safe to draw later. +- VK 429 / 500 / network break during pagination → unhandled, leaves giveaway in FETCHING or READY with incomplete data. No partial-progress resume. + +### H6. winnersCount / reserveWinnersCount Abuse + +No hard upper bound on `winnersCount` or `reserveWinnersCount` beyond `Math.min(..., eligible.length)`. +A client can request `winnersCount: 1_000_000` on a 100-participant giveaway; the code will still allocate and run the partial Fisher-Yates for the actual needed size, but the request body and subsequent JSON can be large. +More importantly, no validation that `winnersCount + reserve ≤ reasonable constant` (e.g. 100). + +### H7. Missing Conditional Status Updates + +All status transitions (`READY` → `SNAPSHOT_LOCKED`, `SNAPSHOT_LOCKED` → `DRAWN`) are plain `update` without `WHERE status = expected`. Under concurrency the wrong status can be overwritten. + +### H8. listAll + Dashboard Memory Amplification + +Dashboard that calls `listAll` will pull every participant of every giveaway into the Node process on each page load. + +### H9. No Soft Limits on Concurrent Snapshots / Draws per Giveaway + +A single giveaway can accumulate many snapshots (version keeps increasing). Each stores a full JSON copy. No retention policy yet. + +### H10–H13. See detailed sections below (Rate limit, Observability, Privacy, Algorithmic notes). + +--- + +## Medium Findings + +- FSM guards are only application-level; a direct DB write can bypass them. +- `MemoryGiveawayRepository` has no concurrency protection at all (single-process only). +- Canonical stringify + sort is correct but CPU-heavy for 100k+; no incremental hashing. +- Verify endpoint re-loads full snapshot every time — expensive for large draws. +- No body-size limit middleware visible (Next.js default is generous). +- Malformed / extremely long VK URLs not explicitly rejected early. +- Unicode / null-byte handling in user names relies on JSON/Postgres; edge cases untested under load. +- Retry storms on 500 from unique violations can amplify load. + +--- + +## Low Findings + +- Seed length is not capped; extremely long custom seed is accepted (only affects HMAC input size). +- No explicit `ON DELETE RESTRICT` behaviour documented for operators. +- AuditRecord and DrawResult both store winner IDs — minor redundancy. + +--- + +## Concurrency Review (Detailed) + +| Scenario | Protected by unique / constraint? | Clean error? | Safe retry? | Recommendation | +|---------------------------------|-----------------------------------|--------------|-------------|----------------| +| Double draw | Yes (`giveawayId` unique) | No (500) | No | Conditional update + 409 mapping | +| Concurrent snapshot | Yes (`giveawayId+version`) | No (500) | Partial | Version under lock | +| Update participants + draw | Partial (FSM) | — | Risky | Explicit lock / require pre-existing snapshot | +| Concurrent create giveaway | No | — | Creates dups| Idempotency-Key | +| Concurrent participants fetch | No | — | Re-fetches | Idempotency or debounce | + +Prisma `$transaction` (interactive or sequential array) uses the connection’s isolation level (default Read Committed). It does **not** by itself serialise the “read status → write DrawResult” critical section. + +--- + +## Failure-Mode Analysis + +| Failure | Resulting state | Safe to retry? | Compensation needed? | +|--------------------------------------|------------------------------------------|----------------|----------------------| +| Postgres unavailable | 500, no write | Yes | No | +| Prisma transaction timeout | Rolled back | Yes | No | +| VK 429 | 500 / incomplete participants | After backoff | Possibly clear partial | +| VK 500 / network mid-pagination | Incomplete list saved or error | Yes | Re-fetch | +| Crash after snapshot, before draw | SNAPSHOT_LOCKED, no DrawResult | Yes (draw) | No | +| Crash after DrawResult+Audit commit, before HTTP | DRAWN + full audit records | Client sees error, retry → 409/500 | Map unique to 409 | +| Crash between DrawResult and Audit (impossible – same tx) | Atomic | — | — | + +--- + +## Abuse Cases & Suggested Limits + +| Abuse vector | Current behaviour | Suggested limit / mitigation | +|-------------------------------------|------------------------------------|------------------------------| +| Thousands of Giveaways | Unlimited | 50–100 / user / day (once auth exists) | +| Spam participants fetch | Re-hits VK every time | Rate limit + cache / debounce 5 min | +| Spam verify | Cheap after first load | 30 req/min per IP / giveaway | +| winnersCount = millions | Capped by eligible | Hard max 100 + 100 reserve | +| Massive custom seed | Accepted | Max 256–512 chars | +| Oversized request body | Next.js default | Explicit 1–2 MB limit | +| Extremely long VK URL | Parsed | Max 2 kB, early reject | +| Repeated snapshot generation | Unlimited versions | Max 5–10 snapshots / giveaway, retention | +| Retry storm after 500 | Amplifies | 429 + Retry-After, circuit breaker | + +Full policy → `docs/RATE_LIMIT_POLICY.md`. + +--- + +## Database Invariants Audit + +| Invariant | Guaranteed by DB? | Guaranteed by app code? | Notes | +|------------------------------------------------|-------------------|--------------------------|-------| +| At most one successful DrawResult per Giveaway | Yes (`@unique`) | Yes (FSM + unique) | Strong | +| At most one AuditRecord per Giveaway | Yes (`@unique`) | Yes | Strong | +| DrawResult always references existing snapshot | Yes (FK + Restrict)| Yes | Strong | +| Snapshot version unique per giveaway | Yes (`@@unique`) | Yes | Strong | +| Cannot delete snapshot that has Draw/Audit | Yes (onDelete: Restrict) | — | Strong | +| Status = DRAWN implies DrawResult exists | No | Yes (same transaction) | Soft – possible manual inconsistency | +| Status = DRAWN implies AuditRecord exists | No | Yes (same transaction) | Soft | +| Participants unique per (giveaway, user) | Yes | Yes | Strong | + +No CHECK constraints or triggers enforce status ↔ existence of DrawResult/Audit. Application must remain the sole writer of status transitions. + +--- + +## Scalability & Algorithmic Complexity + +| Operation | Time complexity | Memory complexity | Notes | +|----------------------------|--------------------------|------------------------|-------| +| Participant deduplication | O(n) | O(n) | Map | +| Filter engine | O(n) | O(n) | — | +| Canonical sort | O(n log n) | O(n) | localeCompare | +| Snapshot hashing | O(n · L) | O(n · L) | L ≈ 150–200 B/item | +| Partial Fisher-Yates | O(k · C_HMAC) expected | O(n) (copy) | k = winners+reserve | +| Verification | Same as draw | O(n) | Full re-execution | +| Prisma createMany | O(n) | O(n) | Batch | + +No O(n²) algorithms found in the reviewed core paths. Dominant cost at scale is **canonical JSON construction + SHA-256** and **memory residency of the full participant arrays**. + +**Synthetic baseline (Node 24, single core, see `docs/PERFORMANCE_BASELINE.md`):** + +| n | Hash (ms) | JSON size (MB) | Heap after gen (MB) | Draw 10 winners (ms) | +|--------|-----------|----------------|---------------------|----------------------| +| 100 | ~3 | 0.03 | ~5 | <1 | +| 1 000 | ~6 | 0.27 | ~5 | <1 | +| 10 000 | ~56 | 2.8 | ~9 | ~3 | +| 50 000 | ~276 | 13.8 | ~30 | <1 | +| 100 000| ~994 | 27.7 | ~133 | ~1 | + +Extrapolated 500 k: hash ≈ 5 s, JSON ≈ 80–140 MB, heap pressure > 600 MB. + +--- + +## Memory Pressure Hotspots + +1. `getGiveawayById` / `listGiveaways` – always include full participants + snapshots. +2. `createAndLockSnapshot` – materialises full array + JSON for Prisma. +3. `computeParticipantsSnapshotHash` – creates sorted copy + large intermediate string. +4. `executeDeterministicDrawV1` – copies the eligible array. +5. API handlers that return full participant lists. +6. VK provider (if it accumulates all pages in memory before returning). + +**Worst-case 500 k participants on a 1 GB container:** high probability of OOM during hash or snapshot write. + +--- + +## Recommendations (Prioritised) + +### Before any production traffic +1. Map Prisma unique-violation (P2002) on DrawResult / Snapshot to HTTP 409 with clear message. +2. Make draw status transition conditional (`updateMany` WHERE status = expected). +3. Stop returning full participant arrays from list / participants endpoints (paginate or return stats + IDs only). +4. Add hard caps: `winnersCount ≤ 100`, `reserve ≤ 100`, request body size, seed length. +5. Add basic rate limiting (even if only in reverse-proxy / Next middleware). + +### Short-term (Phase 2) +6. Introduce optimistic locking or advisory locks for draw + snapshot. +7. Require explicit snapshot before draw; never auto-create inside the draw handler under concurrency. +8. Add `Idempotency-Key` support for create / participants / snapshot. +9. Paginate or stream large participant responses. +10. Add request-level timeout + AbortController for VK calls. + +### Medium-term / Phase 3+ (architecture proposals) +11. Move large snapshots out of Postgres JSON: + - Option A: normalised `SnapshotParticipant` rows (chunked inserts). + - Option B: compressed JSON (gzip / zstd) in bytea + object storage (S3) for the bulk. + - Option C: Merkle-tree root only in DB, leaves in object storage (best for public verification of very large sets). +12. Background job for VK fetch + enrichment when n > ~5–10 k. +13. Giveaway-level retention / anonymisation policy (see Privacy section). +14. Connection pooling, statement timeouts, and circuit breakers for VK + DB. + +--- + +## Files Changed / Added + +| File | Action | Purpose | +|-----------------------------------|----------|---------| +| `docs/GROK_REVIEW.md` | Created | This review | +| `docs/LOAD_TEST_PLAN.md` | Created | Load scenarios | +| `docs/PERFORMANCE_BASELINE.md` | Created | Measured numbers | +| `docs/RATE_LIMIT_POLICY.md` | Created | Production rate limits | +| `docs/OBSERVABILITY.md` | Created | Metrics, logs, alerts | +| `scripts/benchmarks/core-performance.mjs` | Created | Offline core micro-benchmarks | + +**No production Core, Prisma schema, proof format, or API contracts were altered.** +**No existing tests were broken** (benchmark is standalone). + +**Tests added:** 0 (analysis-only phase; benchmark is not a unit test). +**Maximum participant count exercised:** 100 000. + +--- + +## Definition of Done Checklist + +- [x] Double draw race investigated +- [x] Snapshot race investigated +- [x] Participant update vs draw investigated +- [x] Idempotency strategy proposed +- [x] Load up to 500 k estimated +- [x] Performance baseline recorded +- [x] Memory risks identified +- [x] Abuse cases covered +- [x] Failure modes described +- [x] DB constraints audited +- [x] Rate limit policy written +- [x] Observability plan written +- [x] Production Core left intact +- [x] Deliverables present + +--- + +*End of Grok Phase G-1 Review* diff --git a/grok_review/GROK_REVIEW.md b/grok_review/GROK_REVIEW.md new file mode 100644 index 0000000..6033712 --- /dev/null +++ b/grok_review/GROK_REVIEW.md @@ -0,0 +1,383 @@ +# Randomayzer — Phase G-1 Concurrency, Load, Abuse & Failure-Mode Review + +**Reviewer:** Grok (xAI) +**Date:** 2026-08-17 +**Scope:** Concurrency, load/scalability, abuse resistance, failure modes, DB invariants, observability, rate limiting, memory & algorithmic complexity. +**Out of scope (per assignment):** Public Verification Integrity (Antigravity), general QA/security review & VK Integration prep (OpenCode). No changes to Randomizer core, proof format, OAuth, UI branding, VK Provider contract, or large Prisma schema rewrites. + +**Repository snapshot:** local extract of `randomayzer-main` (matches provided zip / expected main). +**Commit SHA (from zip metadata):** `26e82fcf8d8e5855ac9e46fa8af21ca7daacc36f` + +--- + +## Executive Summary + +| Category | Critical | High | Medium | Low | +|-----------------------|----------|------|--------|-----| +| Concurrency | 2 | 2 | 1 | 0 | +| Failure Modes | 0 | 3 | 2 | 1 | +| Abuse Resistance | 1 | 3 | 2 | 1 | +| Database Invariants | 0 | 2 | 2 | 0 | +| Scalability / Memory | 1 | 3 | 2 | 0 | +| **Total** | **4** | **13**| **9** | **2**| + +**Maximum participant count synthetically exercised:** 100 000 (core hashing / Fisher-Yates / verify). +**Estimated safe production limit (current architecture, single Node process, 2–4 GB RAM):** ~30–50k eligible participants. +**Race conditions found:** Double-draw (protected by unique constraint but poor error handling), Snapshot version collision (unique constraint only), Participant-update vs Draw interleaving. + +**Production Core was not modified.** Only documentation + optional offline benchmark script added. + +--- + +## Critical Findings + +### C1. Double-Draw Race Condition (Application-level TOCTOU) + +**Location:** `src/app/api/giveaways/[id]/draw/route.ts` + `PrismaGiveawayRepository.saveDrawResultAndAudit` + +**Scenario:** Two almost simultaneous `POST /api/giveaways/:id/draw`. + +1. Both requests read Giveaway (status = `READY` or `SNAPSHOT_LOCKED`). +2. Both pass the early `if (status === 'DRAWN')` guard. +3. Both may create / reuse snapshot. +4. Both call `executeDeterministicDrawV1` (possibly with different seeds). +5. Both enter `$transaction` and attempt `drawResult.create({ giveawayId })`. + +**Protection today:** +- `DrawResult.giveawayId @unique` → second insert fails with Prisma `P2002`. +- Transaction is atomic for the successful request. + +**Problems:** +- Second request receives **500 Internal Server Error** (unhandled unique violation) instead of clean 409/400 “already drawn”. +- Client may retry and keep failing. +- Work (hashing, Fisher-Yates) is wasted on the loser. +- No `SELECT … FOR UPDATE` / optimistic version / conditional `UPDATE … WHERE status = 'SNAPSHOT_LOCKED'`. +- Default Prisma isolation (Read Committed) does not prevent the race. + +**Impact:** Data integrity is preserved (only one DrawResult), but availability and UX under concurrent load are broken. In a load-balanced multi-instance deployment the race window is larger. + +**Recommended fix (proposal only — do not implement without agreement):** + +```ts +// Inside transaction, use conditional update as gate +const updated = await tx.giveaway.updateMany({ + where: { id, status: 'SNAPSHOT_LOCKED' }, + data: { status: 'DRAWN', drawnAt: ..., seed: ... }, +}); +if (updated.count === 0) { + throw new Error('ALREADY_DRAWN_OR_INVALID_STATE'); // map to 409 +} +// then create DrawResult + AuditRecord +``` + +Or use PostgreSQL advisory lock (`pg_advisory_xact_lock(hashtext(giveawayId))`) at the start of the transaction, or a dedicated `draw_lock` row. + +Optimistic locking via a `version` / `statusVersion` integer column is also viable. + +### C2. Snapshot Version Race + +**Location:** `PrismaGiveawayRepository.createAndLockSnapshot` + +```ts +const latestVersion = current.snapshots.length > 0 + ? Math.max(...current.snapshots.map(s => s.version)) : 0; +const newVersion = latestVersion + 1; +// then $transaction([ create with newVersion, update status ]) +``` + +Version is computed **outside** any lock. Two concurrent snapshot creations can choose the same `version` → unique constraint `@@unique([giveawayId, version])` rejects one with 500. + +**Sufficient for data integrity?** Yes (constraint works). +**Production-safe?** No — error handling and retry semantics are missing. Status can also be left inconsistent if one succeeds and the other fails after status update. + +**Proposal:** Compute next version inside a serializable transaction or use `INSERT … ON CONFLICT` / sequence / `MAX(version)+1` under `SELECT FOR UPDATE` on the Giveaway row. + +### C3. Full Participant Lists Loaded into Memory on Every getById / listAll + +`getGiveawayById` and `listGiveaways` always `include: { participants: true, snapshots: true }`. + +For a giveaway with 100k participants the JSON response + in-memory objects easily exceed hundreds of MB. `listAll` multiplies the problem. + +Combined with snapshot `eligibleParticipants Json` this is the primary OOM vector. + +### C4. Snapshot Storage as Monolithic JSON + +`ParticipantSnapshot.eligibleParticipants Json` stores the entire canonical array. + +| Eligible count | Approx. canonical JSON size | Prisma / PG TOAST risk | Memory on read | +|----------------|-----------------------------|------------------------|----------------| +| 10k | ~1.5–2.5 MB | OK | Low | +| 50k | ~8–14 MB | Acceptable | Medium | +| 100k | ~20–30 MB | High (large object) | High | +| 500k | ~80–140 MB | Problematic | Almost certain OOM on small instances | + +No chunking, compression, or external object storage. Verification and UI that re-load the snapshot will re-materialize the whole array. + +--- + +## High Findings + +### H1. Participant Update vs Draw Race + +`saveParticipants` is allowed only when status ∉ {SNAPSHOT_LOCKED, DRAWN, PUBLISHED}. +Draw can create a snapshot from the in-memory `giveaway.participants` if none exists. + +If both run concurrently while status = READY: +- Draw may snapshot a partially-updated list (deleteMany has finished, createMany has not, or vice-versa). +- Or snapshot the old list while the new list is committed. + +Result: Draw operates on a non-atomic participant set. Integrity of the snapshot relative to the final stored participants is not guaranteed. + +**Mitigation proposal:** Always require an explicit snapshot before draw (already partially true via FSM), and make `createAndLockSnapshot` take a DB-level lock that also blocks `saveParticipants`. + +### H2. No Idempotency Keys + +None of the mutating endpoints accept or honour `Idempotency-Key`: + +- `POST /api/giveaways` (create) +- `POST …/participants` +- `POST …/snapshot` +- `POST …/draw` +- `POST …/publish` (if exists) + +For draw the unique constraint gives a crude form of “at-most-once”, but the error is not clean. +For create / participants / snapshot a client retry after a network blip can create duplicates or re-fetch VK data unnecessarily. + +**Recommendation:** +- Draw / Publish → rely on DB state machine + unique constraints (already present) + map P2002 → 409. +- Create Giveaway, Snapshot, Participants fetch → accept optional `Idempotency-Key` header, store in a short-lived table or Redis, return the previous response on conflict. + +### H3. API Response Size / DoS Surface + +`POST …/participants` returns the full `allParticipants` array. +`GET /api/giveaways` returns every giveaway with every participant and every snapshot. + +A malicious or naïve client can trigger multi-hundred-MB responses. Combined with lack of rate limiting this is an easy memory / bandwidth exhaustion vector. + +### H4. No Request Timeouts / Cancellation on VK Fetch + +`VkProvider.fetchParticipants` (and pagination) can run for a long time. There is no `AbortController`, no overall request timeout, no job queue. A slow VK response or large comment tree holds a Node request forever and can exhaust the connection pool. + +**When HTTP request-response stops being suitable:** +When expected VK fetch + enrichment > 15–20 s for typical giveaways, or when 10k+ participants become common. Move to background job (BullMQ / Inngest / custom) with status polling. + +### H5. Failure Modes – Partial Crash After Commit + +- Crash after successful `$transaction` in `saveDrawResultAndAudit` but before HTTP response → client retries → unique violation → 500. +- Crash in the middle of `saveParticipants` transaction → rolled back (good). +- Process death after snapshot create but before draw → status = SNAPSHOT_LOCKED, safe to draw later. +- VK 429 / 500 / network break during pagination → unhandled, leaves giveaway in FETCHING or READY with incomplete data. No partial-progress resume. + +### H6. winnersCount / reserveWinnersCount Abuse + +No hard upper bound on `winnersCount` or `reserveWinnersCount` beyond `Math.min(..., eligible.length)`. +A client can request `winnersCount: 1_000_000` on a 100-participant giveaway; the code will still allocate and run the partial Fisher-Yates for the actual needed size, but the request body and subsequent JSON can be large. +More importantly, no validation that `winnersCount + reserve ≤ reasonable constant` (e.g. 100). + +### H7. Missing Conditional Status Updates + +All status transitions (`READY` → `SNAPSHOT_LOCKED`, `SNAPSHOT_LOCKED` → `DRAWN`) are plain `update` without `WHERE status = expected`. Under concurrency the wrong status can be overwritten. + +### H8. listAll + Dashboard Memory Amplification + +Dashboard that calls `listAll` will pull every participant of every giveaway into the Node process on each page load. + +### H9. No Soft Limits on Concurrent Snapshots / Draws per Giveaway + +A single giveaway can accumulate many snapshots (version keeps increasing). Each stores a full JSON copy. No retention policy yet. + +### H10–H13. See detailed sections below (Rate limit, Observability, Privacy, Algorithmic notes). + +--- + +## Medium Findings + +- FSM guards are only application-level; a direct DB write can bypass them. +- `MemoryGiveawayRepository` has no concurrency protection at all (single-process only). +- Canonical stringify + sort is correct but CPU-heavy for 100k+; no incremental hashing. +- Verify endpoint re-loads full snapshot every time — expensive for large draws. +- No body-size limit middleware visible (Next.js default is generous). +- Malformed / extremely long VK URLs not explicitly rejected early. +- Unicode / null-byte handling in user names relies on JSON/Postgres; edge cases untested under load. +- Retry storms on 500 from unique violations can amplify load. + +--- + +## Low Findings + +- Seed length is not capped; extremely long custom seed is accepted (only affects HMAC input size). +- No explicit `ON DELETE RESTRICT` behaviour documented for operators. +- AuditRecord and DrawResult both store winner IDs — minor redundancy. + +--- + +## Concurrency Review (Detailed) + +| Scenario | Protected by unique / constraint? | Clean error? | Safe retry? | Recommendation | +|---------------------------------|-----------------------------------|--------------|-------------|----------------| +| Double draw | Yes (`giveawayId` unique) | No (500) | No | Conditional update + 409 mapping | +| Concurrent snapshot | Yes (`giveawayId+version`) | No (500) | Partial | Version under lock | +| Update participants + draw | Partial (FSM) | — | Risky | Explicit lock / require pre-existing snapshot | +| Concurrent create giveaway | No | — | Creates dups| Idempotency-Key | +| Concurrent participants fetch | No | — | Re-fetches | Idempotency or debounce | + +Prisma `$transaction` (interactive or sequential array) uses the connection’s isolation level (default Read Committed). It does **not** by itself serialise the “read status → write DrawResult” critical section. + +--- + +## Failure-Mode Analysis + +| Failure | Resulting state | Safe to retry? | Compensation needed? | +|--------------------------------------|------------------------------------------|----------------|----------------------| +| Postgres unavailable | 500, no write | Yes | No | +| Prisma transaction timeout | Rolled back | Yes | No | +| VK 429 | 500 / incomplete participants | After backoff | Possibly clear partial | +| VK 500 / network mid-pagination | Incomplete list saved or error | Yes | Re-fetch | +| Crash after snapshot, before draw | SNAPSHOT_LOCKED, no DrawResult | Yes (draw) | No | +| Crash after DrawResult+Audit commit, before HTTP | DRAWN + full audit records | Client sees error, retry → 409/500 | Map unique to 409 | +| Crash between DrawResult and Audit (impossible – same tx) | Atomic | — | — | + +--- + +## Abuse Cases & Suggested Limits + +| Abuse vector | Current behaviour | Suggested limit / mitigation | +|-------------------------------------|------------------------------------|------------------------------| +| Thousands of Giveaways | Unlimited | 50–100 / user / day (once auth exists) | +| Spam participants fetch | Re-hits VK every time | Rate limit + cache / debounce 5 min | +| Spam verify | Cheap after first load | 30 req/min per IP / giveaway | +| winnersCount = millions | Capped by eligible | Hard max 100 + 100 reserve | +| Massive custom seed | Accepted | Max 256–512 chars | +| Oversized request body | Next.js default | Explicit 1–2 MB limit | +| Extremely long VK URL | Parsed | Max 2 kB, early reject | +| Repeated snapshot generation | Unlimited versions | Max 5–10 snapshots / giveaway, retention | +| Retry storm after 500 | Amplifies | 429 + Retry-After, circuit breaker | + +Full policy → `docs/RATE_LIMIT_POLICY.md`. + +--- + +## Database Invariants Audit + +| Invariant | Guaranteed by DB? | Guaranteed by app code? | Notes | +|------------------------------------------------|-------------------|--------------------------|-------| +| At most one successful DrawResult per Giveaway | Yes (`@unique`) | Yes (FSM + unique) | Strong | +| At most one AuditRecord per Giveaway | Yes (`@unique`) | Yes | Strong | +| DrawResult always references existing snapshot | Yes (FK + Restrict)| Yes | Strong | +| Snapshot version unique per giveaway | Yes (`@@unique`) | Yes | Strong | +| Cannot delete snapshot that has Draw/Audit | Yes (onDelete: Restrict) | — | Strong | +| Status = DRAWN implies DrawResult exists | No | Yes (same transaction) | Soft – possible manual inconsistency | +| Status = DRAWN implies AuditRecord exists | No | Yes (same transaction) | Soft | +| Participants unique per (giveaway, user) | Yes | Yes | Strong | + +No CHECK constraints or triggers enforce status ↔ existence of DrawResult/Audit. Application must remain the sole writer of status transitions. + +--- + +## Scalability & Algorithmic Complexity + +| Operation | Time complexity | Memory complexity | Notes | +|----------------------------|--------------------------|------------------------|-------| +| Participant deduplication | O(n) | O(n) | Map | +| Filter engine | O(n) | O(n) | — | +| Canonical sort | O(n log n) | O(n) | localeCompare | +| Snapshot hashing | O(n · L) | O(n · L) | L ≈ 150–200 B/item | +| Partial Fisher-Yates | O(k · C_HMAC) expected | O(n) (copy) | k = winners+reserve | +| Verification | Same as draw | O(n) | Full re-execution | +| Prisma createMany | O(n) | O(n) | Batch | + +No O(n²) algorithms found in the reviewed core paths. Dominant cost at scale is **canonical JSON construction + SHA-256** and **memory residency of the full participant arrays**. + +**Synthetic baseline (Node 24, single core, see `docs/PERFORMANCE_BASELINE.md`):** + +| n | Hash (ms) | JSON size (MB) | Heap after gen (MB) | Draw 10 winners (ms) | +|--------|-----------|----------------|---------------------|----------------------| +| 100 | ~3 | 0.03 | ~5 | <1 | +| 1 000 | ~6 | 0.27 | ~5 | <1 | +| 10 000 | ~56 | 2.8 | ~9 | ~3 | +| 50 000 | ~276 | 13.8 | ~30 | <1 | +| 100 000| ~994 | 27.7 | ~133 | ~1 | + +Extrapolated 500 k: hash ≈ 5 s, JSON ≈ 80–140 MB, heap pressure > 600 MB. + +--- + +## Memory Pressure Hotspots + +1. `getGiveawayById` / `listGiveaways` – always include full participants + snapshots. +2. `createAndLockSnapshot` – materialises full array + JSON for Prisma. +3. `computeParticipantsSnapshotHash` – creates sorted copy + large intermediate string. +4. `executeDeterministicDrawV1` – copies the eligible array. +5. API handlers that return full participant lists. +6. VK provider (if it accumulates all pages in memory before returning). + +**Worst-case 500 k participants on a 1 GB container:** high probability of OOM during hash or snapshot write. + +--- + +## Recommendations (Prioritised) + +### Before any production traffic +1. Map Prisma unique-violation (P2002) on DrawResult / Snapshot to HTTP 409 with clear message. +2. Make draw status transition conditional (`updateMany` WHERE status = expected). +3. Stop returning full participant arrays from list / participants endpoints (paginate or return stats + IDs only). +4. Add hard caps: `winnersCount ≤ 100`, `reserve ≤ 100`, request body size, seed length. +5. Add basic rate limiting (even if only in reverse-proxy / Next middleware). + +### Short-term (Phase 2) +6. Introduce optimistic locking or advisory locks for draw + snapshot. +7. Require explicit snapshot before draw; never auto-create inside the draw handler under concurrency. +8. Add `Idempotency-Key` support for create / participants / snapshot. +9. Paginate or stream large participant responses. +10. Add request-level timeout + AbortController for VK calls. + +### Medium-term / Phase 3+ (architecture proposals) +11. Move large snapshots out of Postgres JSON: + - Option A: normalised `SnapshotParticipant` rows (chunked inserts). + - Option B: compressed JSON (gzip / zstd) in bytea + object storage (S3) for the bulk. + - Option C: Merkle-tree root only in DB, leaves in object storage (best for public verification of very large sets). +12. Background job for VK fetch + enrichment when n > ~5–10 k. +13. Giveaway-level retention / anonymisation policy (see Privacy section). +14. Connection pooling, statement timeouts, and circuit breakers for VK + DB. + +--- + +## Files Changed / Added + +| File | Action | Purpose | +|-----------------------------------|----------|---------| +| `docs/GROK_REVIEW.md` | Created | This review | +| `docs/LOAD_TEST_PLAN.md` | Created | Load scenarios | +| `docs/PERFORMANCE_BASELINE.md` | Created | Measured numbers | +| `docs/RATE_LIMIT_POLICY.md` | Created | Production rate limits | +| `docs/OBSERVABILITY.md` | Created | Metrics, logs, alerts | +| `scripts/benchmarks/core-performance.mjs` | Created | Offline core micro-benchmarks | + +**No production Core, Prisma schema, proof format, or API contracts were altered.** +**No existing tests were broken** (benchmark is standalone). + +**Tests added:** 0 (analysis-only phase; benchmark is not a unit test). +**Maximum participant count exercised:** 100 000. + +--- + +## Definition of Done Checklist + +- [x] Double draw race investigated +- [x] Snapshot race investigated +- [x] Participant update vs draw investigated +- [x] Idempotency strategy proposed +- [x] Load up to 500 k estimated +- [x] Performance baseline recorded +- [x] Memory risks identified +- [x] Abuse cases covered +- [x] Failure modes described +- [x] DB constraints audited +- [x] Rate limit policy written +- [x] Observability plan written +- [x] Production Core left intact +- [x] Deliverables present + +--- + +*End of Grok Phase G-1 Review* diff --git a/grok_review/PHASE_1_4_FAILURE_MATRIX.md b/grok_review/PHASE_1_4_FAILURE_MATRIX.md new file mode 100644 index 0000000..f50a96a --- /dev/null +++ b/grok_review/PHASE_1_4_FAILURE_MATRIX.md @@ -0,0 +1,108 @@ +# Phase 1.4 Failure Matrix — Grok G-2 + +**Commit:** `3f795e23fe6397832147fb47285584dc0eccbf3c` +**Date:** 2026-08-17 + +## Legend +- **Protected** = data integrity holds, clean client error +- **Degraded** = integrity holds but UX / ops suffer +- **Bypassable** = attacker can defeat the control +- **Gap** = missing or incomplete protection + +--- + +## 1. Concurrency + +| Scenario | Protection | HTTP outcome | Data integrity | Notes | +|----------|------------|--------------|----------------|-------| +| 2 concurrent draws | Conditional updateMany + unique | 200 + 409 | Protected | Good | +| 10–100 concurrent draws | Same | 1×200 + (N-1)×409 | Protected | Memory tests pass; Prisma relies on DB | +| Concurrent snapshots | Version inside tx + unique | 409 on conflict | Protected | Multiple historical versions allowed | +| Participants update while SNAPSHOT_LOCKED | FSM assert | 409 | Protected | | +| Participants + snapshot + draw interleaving | FSM + status guards | 409 / sequential success | Protected (residual TOCTOU low) | Re-test after 1.4.1 | + +## 2. Idempotency + +| Scenario | Protection | Outcome | Grade | +|----------|------------|---------|-------| +| Same key + same body | In-memory Map | Cached response | OK (single instance) | +| Same key + different body | None (no body hash) | Wrong / first body returned | **Gap / HIGH** | +| Key reuse across giveaways | Weak prefix only | Possible collision | **Gap** | +| Parallel identical requests | Race on set | One wins | Degraded | +| Flood unique keys | Lazy TTL only | Memory growth | **Gap** | +| Multi-instance deployment | Process-local | No shared semantics | **Fail** | + +## 3. Rate Limiting + +| Scenario | Protection | Outcome | Grade | +|----------|------------|---------|-------| +| Burst same IP | Sliding window | 429 after limit | OK | +| Rotate X-Forwarded-For | None | Full bypass | **Bypassable / CRITICAL** | +| Missing X-Forwarded-For | Falls to “anonymous” | Shared bucket | Degraded | +| IPv6 / multi-value header | No normalization | Unexpected keys | Gap | +| Unique fake IP flood | No idle key GC | Memory growth | Gap | +| Multi-instance | Process-local | Independent buckets | Fail for global limit | + +## 4. Validation & Contract + +| Scenario | Protection | Outcome | Grade | +|----------|------------|---------|-------| +| winnersCount 0 / 101 / negative | Zod | 400 | Protected | +| seed > 512 chars | Zod | 400 | Protected | +| URL > 2048 | Zod | 400 | Protected | +| Malformed JSON | handleApiError | 400 | Protected | +| Unknown fields | .strict() | 400 | Protected | +| winnersCount > eligible | Silent Math.min in Core | 200 with fewer winners | **Gap** (contract) | +| Nested / huge JSON before Zod | Node parser limits | Possible 400 / memory | Acceptable | + +## 5. Database / Crash + +| Scenario | State after | Retry safe? | Grade | +|----------|-------------|-------------|-------| +| Crash before draw tx | SNAPSHOT_LOCKED | Yes | Protected | +| Crash inside draw tx | Rolled back | Yes | Protected | +| Crash after commit, before HTTP | DRAWN + records | Yes → 409 | Protected (client must understand 409) | +| Unique violation on DrawResult | ConflictError 409 | N/A | Protected | +| Orphan DrawResult / Audit | Impossible (same tx + unique) | — | Protected | +| Snapshot delete with Draw/Audit | Restrict | — | Protected | + +## 6. Memory / Scalability of Phase 1.4 Stores + +| Load | IdempotencyStore | RateLimiter | Risk | +|------|------------------|-------------|------| +| 10k unique keys | ~5 MB | low | OK | +| 100k unique keys | ~43 MB | ~20–30 MB | Warning | +| 200k+ / continuous flood | Unbounded (lazy TTL) | Unbounded | High under abuse | + +## 7. Summary Grades (G-2) + +| Area | Grade | +|------|-------| +| Concurrency | **PASS WITH WARNINGS** | +| Idempotency | **FAIL** (for production multi-instance / adversarial) | +| Rate limiting | **FAIL** (header spoof bypass) | +| Validation | **PASS WITH WARNINGS** (silent winners cap) | +| DB integrity | **PASS** | +| Scalability of new stores | **PASS WITH WARNINGS** | + +## 8. VK Integration Readiness Answer + +**Is it safe to start real VK integration after Phase 1.4.1?** + +**No — not yet fully safe.** + +Arguments: + +1. Draw concurrency and DB invariants are solid enough for a single-instance or carefully pooled deployment. +2. Rate limiting is trivially bypassable via `X-Forwarded-For` rotation; any public exposure can be flooded. +3. Idempotency is process-local and lacks body fingerprinting; multi-instance or retry storms will misbehave. +4. Silent under-delivery of winners when requested count exceeds eligible violates a clear API contract expectation. +5. After Antigravity finishes 1.4.1, the mixed-race and 100-concurrent suites must be re-validated on real Postgres. + +**Minimum before VK production traffic:** +- Edge (or shared) rate limiting with non-spoofable identity +- Shared idempotency store with body hash / mismatch rejection +- Explicit error when winnersCount + reserve > eligible +- Client contract that 409 on draw means “already completed” + +Once those are in place, Phase 1.4 concurrency + validation form a reasonable base for controlled VK integration. diff --git a/src/app/api/giveaways/[id]/draw/route.ts b/src/app/api/giveaways/[id]/draw/route.ts index 94c6b53..40796f0 100644 --- a/src/app/api/giveaways/[id]/draw/route.ts +++ b/src/app/api/giveaways/[id]/draw/route.ts @@ -1,11 +1,17 @@ import { NextRequest, NextResponse } from 'next/server'; import { GiveawayStore } from '@/lib/giveaway-store'; -import { GiveawayFSM } from '@/core/fsm/giveaway-fsm'; import { generateCryptoSecureSeed } from '@/core/randomizer/hasher'; import { executeDeterministicDrawV1 } from '@/core/randomizer/deterministic'; import { executeDrawSchema } from '@/core/validation/giveaway-schemas'; -import { handleApiError, NotFoundError, ConflictError } from '@/core/errors/http-errors'; +import { + handleApiError, + NotFoundError, + ConflictError, + ValidationError, + DrawAlreadyCompletedError +} from '@/core/errors/http-errors'; import { expensiveApiRateLimiter } from '@/lib/rate-limiter'; +import { resolveClientIp } from '@/lib/client-ip'; export async function POST( req: NextRequest, @@ -13,18 +19,29 @@ export async function POST( ) { try { const { id } = params; - const ip = req.headers.get('x-forwarded-for') || 'anonymous'; - expensiveApiRateLimiter.assertAllowed(`draw-execute:${ip}:${id}`); + const clientIp = resolveClientIp(req); + expensiveApiRateLimiter.assertAllowed(`draw-execute:${clientIp}:${id}`); const giveaway = await GiveawayStore.getById(id); if (!giveaway) { throw new NotFoundError(`Giveaway with id "${id}" not found`); } - // 1. Strict FSM Guard: Draw is permitted ONLY in SNAPSHOT_LOCKED status - GiveawayFSM.assertCanDraw(giveaway.status); + // 1. Strict Terminal State Guard: If already DRAWN or PUBLISHED, return 409 DRAW_ALREADY_COMPLETED + if (giveaway.status === 'DRAWN' || giveaway.status === 'PUBLISHED') { + throw new DrawAlreadyCompletedError( + `Giveaway "${id}" has already been drawn and finalized. Repeat draws are not permitted.`, + { giveawayId: id, status: giveaway.status, drawnAt: giveaway.drawnAt } + ); + } - // 2. Strict Snapshot requirement: Never create a snapshot implicitly + if (giveaway.status !== 'SNAPSHOT_LOCKED') { + throw new ConflictError( + `Cannot execute draw: giveaway status is "${giveaway.status}", but draw requires "SNAPSHOT_LOCKED"` + ); + } + + // 2. Strict Snapshot requirement const snapshot = giveaway.latestSnapshot; if (!snapshot) { throw new ConflictError( @@ -37,11 +54,20 @@ export async function POST( const winnersCount = validated.winnersCount; const reserveWinnersCount = validated.reserveWinnersCount; + const totalRequired = winnersCount + reserveWinnersCount; + + // 3. Strict Winner Count Contract: Never under-deliver winners + if (totalRequired > snapshot.participantCount) { + throw new ValidationError( + `Requested ${winnersCount} winners and ${reserveWinnersCount} reserve winners (${totalRequired} total) exceeds eligible participants count (${snapshot.participantCount})`, + { winnersCount, reserveWinnersCount, eligibleCount: snapshot.participantCount } + ); + } // Use CSPRNG crypto.randomBytes seed if none provided (Math.random is strictly forbidden) const seed = (validated.seed && validated.seed.trim()) || generateCryptoSecureSeed(); - // 3. Execute Provably Fair Fisher-Yates Draw V1 + // 4. Execute Provably Fair Fisher-Yates Draw V1 const drawResult = executeDeterministicDrawV1({ giveawayId: id, snapshot, @@ -52,7 +78,7 @@ export async function POST( filterRules: giveaway.filterRules, }); - // 4. Save DrawResult & AuditRecord in database with atomic status transition + // 5. Save DrawResult & AuditRecord in database with atomic status transition const updatedGiveaway = await GiveawayStore.saveDrawResult(id, snapshot.id, drawResult); return NextResponse.json({ diff --git a/src/app/api/giveaways/[id]/participants/route.ts b/src/app/api/giveaways/[id]/participants/route.ts index 66ddb40..379f025 100644 --- a/src/app/api/giveaways/[id]/participants/route.ts +++ b/src/app/api/giveaways/[id]/participants/route.ts @@ -6,6 +6,7 @@ import { fetchParticipantsSchema, validateProviderCapabilities } from '@/core/va import { handleApiError, NotFoundError } from '@/core/errors/http-errors'; import { expensiveApiRateLimiter, generalApiRateLimiter } from '@/lib/rate-limiter'; import { IdempotencyStore } from '@/lib/idempotency'; +import { resolveClientIp } from '@/lib/client-ip'; export async function GET( req: NextRequest, @@ -13,8 +14,8 @@ export async function GET( ) { try { const { id } = params; - const ip = req.headers.get('x-forwarded-for') || 'anonymous'; - generalApiRateLimiter.assertAllowed(`participants-get:${ip}`); + const clientIp = resolveClientIp(req); + generalApiRateLimiter.assertAllowed(`participants-get:${clientIp}`); const { searchParams } = new URL(req.url); const page = Math.max(1, parseInt(searchParams.get('page') || '1', 10)); @@ -39,16 +40,8 @@ export async function POST( ) { try { const { id } = params; - const ip = req.headers.get('x-forwarded-for') || 'anonymous'; - expensiveApiRateLimiter.assertAllowed(`participants-import:${ip}:${id}`); - - const idempotencyKey = req.headers.get('idempotency-key'); - if (idempotencyKey) { - const cached = IdempotencyStore.get(`import-part:${idempotencyKey}`); - if (cached) { - return NextResponse.json(cached.body, { status: cached.statusCode }); - } - } + const clientIp = resolveClientIp(req); + expensiveApiRateLimiter.assertAllowed(`participants-import:${clientIp}:${id}`); const giveaway = await GiveawayStore.getById(id); if (!giveaway) { @@ -58,6 +51,19 @@ export async function POST( const rawBody = await req.json(); const validated = fetchParticipantsSchema.parse(rawBody); + const idempotencyKey = req.headers.get('idempotency-key'); + if (idempotencyKey) { + const cached = IdempotencyStore.get({ + key: idempotencyKey, + operation: 'participants-import', + giveawayId: id, + requestPayload: validated, + }); + if (cached) { + return NextResponse.json(cached.body, { status: cached.statusCode }); + } + } + const provider = ProviderFactory.getVkProvider(); validateProviderCapabilities(validated.filterRules, provider.capabilities); @@ -91,7 +97,14 @@ export async function POST( }; if (idempotencyKey) { - IdempotencyStore.set(`import-part:${idempotencyKey}`, 200, responseBody); + IdempotencyStore.set({ + key: idempotencyKey, + operation: 'participants-import', + giveawayId: id, + requestPayload: validated, + statusCode: 200, + body: responseBody, + }); } return NextResponse.json(responseBody); diff --git a/src/app/api/giveaways/[id]/route.ts b/src/app/api/giveaways/[id]/route.ts index b8214be..2d8b04b 100644 --- a/src/app/api/giveaways/[id]/route.ts +++ b/src/app/api/giveaways/[id]/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { GiveawayStore } from '@/lib/giveaway-store'; import { handleApiError, NotFoundError } from '@/core/errors/http-errors'; import { generalApiRateLimiter } from '@/lib/rate-limiter'; +import { resolveClientIp } from '@/lib/client-ip'; export async function GET( req: NextRequest, @@ -9,8 +10,8 @@ export async function GET( ) { try { const { id } = params; - const ip = req.headers.get('x-forwarded-for') || 'anonymous'; - generalApiRateLimiter.assertAllowed(`giveaway-get:${ip}`); + const clientIp = resolveClientIp(req); + generalApiRateLimiter.assertAllowed(`giveaway-get:${clientIp}`); const giveaway = await GiveawayStore.getById(id); diff --git a/src/app/api/giveaways/[id]/snapshot/route.ts b/src/app/api/giveaways/[id]/snapshot/route.ts index eec0b5b..7bf1d72 100644 --- a/src/app/api/giveaways/[id]/snapshot/route.ts +++ b/src/app/api/giveaways/[id]/snapshot/route.ts @@ -6,6 +6,7 @@ import { createSnapshotSchema, validateProviderCapabilities } from '@/core/valid import { handleApiError, NotFoundError, ConflictError } from '@/core/errors/http-errors'; import { expensiveApiRateLimiter } from '@/lib/rate-limiter'; import { IdempotencyStore } from '@/lib/idempotency'; +import { resolveClientIp } from '@/lib/client-ip'; export async function POST( req: NextRequest, @@ -13,16 +14,8 @@ export async function POST( ) { try { const { id } = params; - const ip = req.headers.get('x-forwarded-for') || 'anonymous'; - expensiveApiRateLimiter.assertAllowed(`snapshot-lock:${ip}:${id}`); - - const idempotencyKey = req.headers.get('idempotency-key'); - if (idempotencyKey) { - const cached = IdempotencyStore.get(`lock-snap:${idempotencyKey}`); - if (cached) { - return NextResponse.json(cached.body, { status: cached.statusCode }); - } - } + const clientIp = resolveClientIp(req); + expensiveApiRateLimiter.assertAllowed(`snapshot-lock:${clientIp}:${id}`); const giveaway = await GiveawayStore.getById(id); if (!giveaway) { @@ -36,6 +29,19 @@ export async function POST( const rawBody = await req.json(); const validated = createSnapshotSchema.parse(rawBody); + const idempotencyKey = req.headers.get('idempotency-key'); + if (idempotencyKey) { + const cached = IdempotencyStore.get({ + key: idempotencyKey, + operation: 'snapshot-lock', + giveawayId: id, + requestPayload: validated, + }); + if (cached) { + return NextResponse.json(cached.body, { status: cached.statusCode }); + } + } + const provider = ProviderFactory.getVkProvider(); validateProviderCapabilities(validated.filterRules, provider.capabilities); @@ -64,7 +70,14 @@ export async function POST( }; if (idempotencyKey) { - IdempotencyStore.set(`lock-snap:${idempotencyKey}`, 200, responseBody); + IdempotencyStore.set({ + key: idempotencyKey, + operation: 'snapshot-lock', + giveawayId: id, + requestPayload: validated, + statusCode: 200, + body: responseBody, + }); } return NextResponse.json(responseBody); diff --git a/src/app/api/giveaways/[id]/verify/route.ts b/src/app/api/giveaways/[id]/verify/route.ts index 3b14ee6..15d01e5 100644 --- a/src/app/api/giveaways/[id]/verify/route.ts +++ b/src/app/api/giveaways/[id]/verify/route.ts @@ -3,6 +3,7 @@ import { GiveawayStore } from '@/lib/giveaway-store'; import { verifyDrawResult } from '@/core/randomizer/deterministic'; import { handleApiError, NotFoundError, ConflictError } from '@/core/errors/http-errors'; import { expensiveApiRateLimiter } from '@/lib/rate-limiter'; +import { resolveClientIp } from '@/lib/client-ip'; export async function GET( req: NextRequest, @@ -10,8 +11,8 @@ export async function GET( ) { try { const { id } = params; - const ip = req.headers.get('x-forwarded-for') || 'anonymous'; - expensiveApiRateLimiter.assertAllowed(`verify-get:${ip}:${id}`); + const clientIp = resolveClientIp(req); + expensiveApiRateLimiter.assertAllowed(`verify-get:${clientIp}:${id}`); const giveaway = await GiveawayStore.getById(id); if (!giveaway) { diff --git a/src/app/api/giveaways/route.ts b/src/app/api/giveaways/route.ts index 76719a7..8100baa 100644 --- a/src/app/api/giveaways/route.ts +++ b/src/app/api/giveaways/route.ts @@ -4,11 +4,12 @@ import { createGiveawaySchema } from '@/core/validation/giveaway-schemas'; import { handleApiError } from '@/core/errors/http-errors'; import { generalApiRateLimiter } from '@/lib/rate-limiter'; import { IdempotencyStore } from '@/lib/idempotency'; +import { resolveClientIp } from '@/lib/client-ip'; export async function GET(req: NextRequest) { try { - const ip = req.headers.get('x-forwarded-for') || 'anonymous'; - generalApiRateLimiter.assertAllowed(`giveaways-list:${ip}`); + const clientIp = resolveClientIp(req); + generalApiRateLimiter.assertAllowed(`giveaways-list:${clientIp}`); // Return lightweight summary for scalability (no massive participant/snapshot payloads) const summaries = await GiveawayStore.listSummaries(); @@ -24,20 +25,24 @@ export async function GET(req: NextRequest) { export async function POST(req: NextRequest) { try { - const ip = req.headers.get('x-forwarded-for') || 'anonymous'; - generalApiRateLimiter.assertAllowed(`giveaway-create:${ip}`); + const clientIp = resolveClientIp(req); + generalApiRateLimiter.assertAllowed(`giveaway-create:${clientIp}`); + + const rawBody = await req.json(); + const validated = createGiveawaySchema.parse(rawBody); const idempotencyKey = req.headers.get('idempotency-key'); if (idempotencyKey) { - const cached = IdempotencyStore.get(`create-gw:${idempotencyKey}`); + const cached = IdempotencyStore.get({ + key: idempotencyKey, + operation: 'create-giveaway', + requestPayload: validated, + }); if (cached) { return NextResponse.json(cached.body, { status: cached.statusCode }); } } - const rawBody = await req.json(); - const validated = createGiveawaySchema.parse(rawBody); - const giveaway = await GiveawayStore.create({ sourceUrl: validated.sourceUrl, post: validated.post, @@ -53,7 +58,13 @@ export async function POST(req: NextRequest) { }; if (idempotencyKey) { - IdempotencyStore.set(`create-gw:${idempotencyKey}`, 201, responseBody); + IdempotencyStore.set({ + key: idempotencyKey, + operation: 'create-giveaway', + requestPayload: validated, + statusCode: 201, + body: responseBody, + }); } return NextResponse.json(responseBody, { status: 201 }); diff --git a/src/app/api/posts/preview/route.ts b/src/app/api/posts/preview/route.ts index 2d39d3d..2fcfd03 100644 --- a/src/app/api/posts/preview/route.ts +++ b/src/app/api/posts/preview/route.ts @@ -3,11 +3,12 @@ import { ProviderFactory } from '@/providers/factory'; import { postPreviewSchema } from '@/core/validation/giveaway-schemas'; import { handleApiError } from '@/core/errors/http-errors'; import { generalApiRateLimiter } from '@/lib/rate-limiter'; +import { resolveClientIp } from '@/lib/client-ip'; export async function POST(req: NextRequest) { try { - const ip = req.headers.get('x-forwarded-for') || 'anonymous'; - generalApiRateLimiter.assertAllowed(`post-preview:${ip}`); + const clientIp = resolveClientIp(req); + generalApiRateLimiter.assertAllowed(`post-preview:${clientIp}`); const rawBody = await req.json(); const validated = postPreviewSchema.parse(rawBody); diff --git a/src/core/errors/http-errors.ts b/src/core/errors/http-errors.ts index 3691a55..a224f6c 100644 --- a/src/core/errors/http-errors.ts +++ b/src/core/errors/http-errors.ts @@ -39,6 +39,30 @@ export class ConflictError extends AppError { readonly code = 'CONFLICT'; } +export class IdempotencyKeyReusedError extends AppError { + readonly statusCode = 409; + readonly code = 'IDEMPOTENCY_KEY_REUSED'; + + constructor( + message: string = 'Idempotency key was previously used with different request parameters', + details?: any + ) { + super(message, details); + } +} + +export class DrawAlreadyCompletedError extends AppError { + readonly statusCode = 409; + readonly code = 'DRAW_ALREADY_COMPLETED'; + + constructor( + message: string = 'Giveaway has already been drawn and finalized. Repeat draws are not permitted.', + details?: any + ) { + super(message, details); + } +} + export class RateLimitError extends AppError { readonly statusCode = 429; readonly code = 'RATE_LIMIT_EXCEEDED'; diff --git a/src/core/randomizer/deterministic.ts b/src/core/randomizer/deterministic.ts index 611c3ec..ba49572 100644 --- a/src/core/randomizer/deterministic.ts +++ b/src/core/randomizer/deterministic.ts @@ -63,9 +63,15 @@ export function executeDeterministicDrawV1(params: DrawExecutionParams): DrawExe // 2. Initialize unbiased HMAC stream keyed by seed and snapshotHash const stream = new DeterministicHmacStream(seed, snapshotHash); - const totalNeeded = Math.min(winnersCount + reserveWinnersCount, pool.length); - const actualWinnersCount = Math.min(winnersCount, totalNeeded); - const actualReserveCount = Math.max(0, totalNeeded - actualWinnersCount); + const totalNeeded = winnersCount + reserveWinnersCount; + if (totalNeeded > pool.length) { + throw new Error( + `Requested ${winnersCount} winners and ${reserveWinnersCount} reserve winners (${totalNeeded} total) exceeds eligible participants count (${pool.length})` + ); + } + + const actualWinnersCount = winnersCount; + const actualReserveCount = reserveWinnersCount; // 3. True partial Fisher-Yates shuffle: swap pool[i] with pool[j] where j in [i, n-1] for (let i = 0; i < totalNeeded; i++) { diff --git a/src/lib/client-ip.ts b/src/lib/client-ip.ts new file mode 100644 index 0000000..56bcc22 --- /dev/null +++ b/src/lib/client-ip.ts @@ -0,0 +1,83 @@ +import { NextRequest } from 'next/server'; + +const MAX_HEADER_LENGTH = 1024; + +// Simple regex for basic IPv4 and IPv6 format checking +const IPV4_REGEX = /^(?:(?:25[0-5]|2[0-4]\d|[01]?\d\d?)\.){3}(?:25[0-5]|2[0-4]\d|[01]?\d\d?)$/; +const IPV6_REGEX = /^(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}$|^::(?:[0-9a-fA-F]{1,4}:){0,6}[0-9a-fA-F]{1,4}$|^(?:[0-9a-fA-F]{1,4}:){1,7}:$|^(?:[0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}$|^(?:[0-9a-fA-F]{1,4}:){1,5}(?::[0-9a-fA-F]{1,4}){1,2}$|^(?:[0-9a-fA-F]{1,4}:){1,4}(?::[0-9a-fA-F]{1,4}){1,3}$|^(?:[0-9a-fA-F]{1,4}:){1,3}(?::[0-9a-fA-F]{1,4}){1,4}$|^(?:[0-9a-fA-F]{1,4}:){1,2}(?::[0-9a-fA-F]{1,4}){1,5}$|^[0-9a-fA-F]{1,4}:(?::[0-9a-fA-F]{1,4}){1,6}$|^:(?::[0-9a-fA-F]{1,4}){1,7}$|^::$|^::1$/; + +/** + * Normalizes an IP string (handling IPv6 brackets, ports, and ::ffff: mapped IPv4). + */ +export function normalizeIp(rawIp: string): string { + let ip = rawIp.trim().toLowerCase(); + + // Strip brackets from IPv6 (e.g. [::1]:8080 or [::1]) + if (ip.startsWith('[') && ip.includes(']')) { + ip = ip.substring(1, ip.indexOf(']')); + } else if (ip.startsWith('::ffff:')) { + // IPv4-mapped IPv6 (e.g., ::ffff:192.168.1.1) + const ipv4Part = ip.substring(7); + if (IPV4_REGEX.test(ipv4Part)) { + return ipv4Part; + } + } else if (ip.includes(':') && ip.includes('.')) { + // IPv4 with port (e.g. 192.168.1.1:3000) + const [host] = ip.split(':'); + if (IPV4_REGEX.test(host)) { + return host; + } + } + + // IPv6 loopback normalization + if (ip === '::1' || ip === '0:0:0:0:0:0:0:1') { + return '127.0.0.1'; + } + + return ip; +} + +/** + * Resolves the client identity IP address. + * Strictly ignores untrusted X-Forwarded-For headers unless TRUST_PROXY=true is configured. + */ +export function resolveClientIp(req: NextRequest): string { + const isTrustProxy = process.env.TRUST_PROXY === 'true'; + + if (!isTrustProxy) { + // When proxy is not trusted, ignore spoofable headers from the client + return req.ip || 'direct-client'; + } + + // Proxy is trusted: extract and validate header + const xForwardedFor = req.headers.get('x-forwarded-for'); + const xRealIp = req.headers.get('x-real-ip'); + const cfConnectingIp = req.headers.get('cf-connecting-ip'); + + const rawHeader = xForwardedFor || xRealIp || cfConnectingIp || req.ip; + + if (!rawHeader) { + return 'unknown-client'; + } + + if (rawHeader.length > MAX_HEADER_LENGTH) { + // Oversized header attack guard + return 'malformed-oversized-ip'; + } + + // Handle multi-value proxy chains: "client, proxy1, proxy2" + // The first (leftmost) entry is the client-reported IP + const parts = rawHeader.split(',').map(s => s.trim()).filter(Boolean); + if (parts.length === 0) { + return 'unknown-client'; + } + + const clientCandidate = normalizeIp(parts[0]); + + // Validate that the parsed string is a legitimate IPv4 or IPv6 address + if (IPV4_REGEX.test(clientCandidate) || IPV6_REGEX.test(clientCandidate)) { + return clientCandidate; + } + + return 'malformed-client-ip'; +} diff --git a/src/lib/idempotency.ts b/src/lib/idempotency.ts index 4de9872..7fe5614 100644 --- a/src/lib/idempotency.ts +++ b/src/lib/idempotency.ts @@ -1,34 +1,156 @@ -interface IdempotentResponse { +import { canonicalStringify, sha256 } from '@/core/randomizer/canonical'; +import { IdempotencyKeyReusedError, ValidationError } from '@/core/errors/http-errors'; + +export interface IdempotentResponse { statusCode: number; body: any; createdAt: number; + expiresAt: number; + requestFingerprint: string; } -export class IdempotencyStore { - private static store = new Map(); - private static readonly TTL_MS = 5 * 60 * 1000; // 5 minutes +export interface IdempotencyLookupParams { + key: string; + operation: string; + giveawayId?: string; + requestPayload?: any; +} + +export interface IdempotencySaveParams extends IdempotencyLookupParams { + statusCode: number; + body: any; + ttlMs?: number; +} + +export interface IIdempotencyStore { + get(params: IdempotencyLookupParams): IdempotentResponse | null; + set(params: IdempotencySaveParams): void; + clear(): void; + size(): number; + cleanupExpired(): number; +} + +export class MemoryIdempotencyStore implements IIdempotencyStore { + private store = new Map(); + private readonly defaultTtlMs: number; + private readonly maxKeyLength: number; + private readonly maxEntries: number; + private opCounter = 0; + + constructor(options?: { defaultTtlMs?: number; maxKeyLength?: number; maxEntries?: number }) { + this.defaultTtlMs = options?.defaultTtlMs ?? 5 * 60 * 1000; // 5 minutes default + this.maxKeyLength = options?.maxKeyLength ?? 128; + this.maxEntries = options?.maxEntries ?? 10000; + + if (process.env.NODE_ENV === 'production' && process.env.ALLOW_MEMORY_IDEMPOTENCY !== 'true') { + console.warn( + '[SECURITY WARNING] MemoryIdempotencyStore is active in production. ' + + 'In multi-instance deployments, use a shared distributed store (e.g., Redis/KV) to prevent race conditions.' + ); + } + } + + private validateKey(key: string): void { + if (!key || typeof key !== 'string') { + throw new ValidationError('Idempotency-Key must be a non-empty string'); + } + if (key.length > this.maxKeyLength) { + throw new ValidationError( + `Idempotency-Key length exceeds maximum allowed (${this.maxKeyLength} characters)` + ); + } + } + + private buildCompositeKey(operation: string, giveawayId?: string, key?: string): string { + return `${operation}:${giveawayId || 'global'}:${key}`; + } + + private computeFingerprint(payload: any): string { + return sha256(canonicalStringify(payload ?? {})); + } + + public get(params: IdempotencyLookupParams): IdempotentResponse | null { + this.validateKey(params.key); + const compositeKey = this.buildCompositeKey(params.operation, params.giveawayId, params.key); + const cached = this.store.get(compositeKey); - public static get(key: string): IdempotentResponse | null { - const cached = this.store.get(key); if (!cached) return null; - if (Date.now() - cached.createdAt > this.TTL_MS) { - this.store.delete(key); + // Check TTL expiration + if (Date.now() > cached.expiresAt) { + this.store.delete(compositeKey); return null; } + // Verify request fingerprint matches original request payload + const currentFingerprint = this.computeFingerprint(params.requestPayload); + if (cached.requestFingerprint !== currentFingerprint) { + throw new IdempotencyKeyReusedError( + `Idempotency key "${params.key}" was previously used for operation "${params.operation}" with different request parameters.`, + { + key: params.key, + operation: params.operation, + giveawayId: params.giveawayId, + } + ); + } + return cached; } - public static set(key: string, statusCode: number, body: any): void { - this.store.set(key, { - statusCode, - body, - createdAt: Date.now(), + public set(params: IdempotencySaveParams): void { + this.validateKey(params.key); + const compositeKey = this.buildCompositeKey(params.operation, params.giveawayId, params.key); + const fingerprint = this.computeFingerprint(params.requestPayload); + const now = Date.now(); + const ttl = params.ttlMs ?? this.defaultTtlMs; + + // Proactive cleanup + this.opCounter++; + if (this.opCounter % 100 === 0 || this.store.size >= this.maxEntries) { + this.cleanupExpired(); + } + + // If still at capacity after cleanup, evict oldest entry + if (this.store.size >= this.maxEntries) { + const oldestKey = this.store.keys().next().value; + if (oldestKey) { + this.store.delete(oldestKey); + } + } + + this.store.set(compositeKey, { + statusCode: params.statusCode, + body: params.body, + createdAt: now, + expiresAt: now + ttl, + requestFingerprint: fingerprint, }); } - public static clear(): void { + public cleanupExpired(): number { + const now = Date.now(); + let deletedCount = 0; + + for (const [k, v] of this.store.entries()) { + if (now > v.expiresAt) { + this.store.delete(k); + deletedCount++; + } + } + + return deletedCount; + } + + public clear(): void { this.store.clear(); + this.opCounter = 0; + } + + public size(): number { + return this.store.size; } } + +// Global Singleton Instance +export const IdempotencyStore: IIdempotencyStore = new MemoryIdempotencyStore(); diff --git a/src/lib/rate-limiter.ts b/src/lib/rate-limiter.ts index 2f62077..206f2bc 100644 --- a/src/lib/rate-limiter.ts +++ b/src/lib/rate-limiter.ts @@ -2,33 +2,57 @@ import { RateLimitError } from '../core/errors/http-errors'; interface RateLimitRecord { timestamps: number[]; + lastAccessed: number; } export interface RateLimiterOptions { windowMs: number; maxRequests: number; + maxBuckets?: number; } export class SlidingWindowRateLimiter { private records = new Map(); private readonly windowMs: number; private readonly maxRequests: number; + private readonly maxBuckets: number; + private opCounter = 0; constructor(options: RateLimiterOptions) { this.windowMs = options.windowMs; this.maxRequests = options.maxRequests; + this.maxBuckets = options.maxBuckets ?? 50000; + + if (process.env.NODE_ENV === 'production' && process.env.ALLOW_MEMORY_RATE_LIMITER !== 'true') { + console.warn( + '[SECURITY WARNING] SlidingWindowRateLimiter (In-Memory) is active in production. ' + + 'In multi-instance deployments, use a distributed edge/Redis limiter.' + ); + } } public check(key: string): { allowed: boolean; remaining: number; resetInMs: number } { const now = Date.now(); const windowStart = now - this.windowMs; + this.opCounter++; + if (this.opCounter % 200 === 0 || this.records.size >= this.maxBuckets) { + this.cleanupExpired(); + } + let record = this.records.get(key); if (!record) { - record = { timestamps: [] }; + if (this.records.size >= this.maxBuckets) { + const oldestKey = this.records.keys().next().value; + if (oldestKey) this.records.delete(oldestKey); + } + + record = { timestamps: [], lastAccessed: now }; this.records.set(key, record); } + record.lastAccessed = now; + // Purge timestamps outside current window record.timestamps = record.timestamps.filter(ts => ts > windowStart); @@ -53,8 +77,29 @@ export class SlidingWindowRateLimiter { } } + public cleanupExpired(): number { + const now = Date.now(); + const windowStart = now - this.windowMs; + let deletedCount = 0; + + for (const [k, v] of this.records.entries()) { + v.timestamps = v.timestamps.filter(ts => ts > windowStart); + if (v.timestamps.length === 0 && now - v.lastAccessed > this.windowMs) { + this.records.delete(k); + deletedCount++; + } + } + + return deletedCount; + } + public reset(): void { this.records.clear(); + this.opCounter = 0; + } + + public size(): number { + return this.records.size; } } diff --git a/tests/client-ip-resolver.test.ts b/tests/client-ip-resolver.test.ts new file mode 100644 index 0000000..8b2b2b0 --- /dev/null +++ b/tests/client-ip-resolver.test.ts @@ -0,0 +1,85 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { NextRequest } from 'next/server'; +import { resolveClientIp, normalizeIp } from '../src/lib/client-ip'; +import { SlidingWindowRateLimiter } from '../src/lib/rate-limiter'; + +describe('Client IP Resolution & Rate Limiter Identity', () => { + const originalEnv = { ...process.env }; + + beforeEach(() => { + delete process.env.TRUST_PROXY; + }); + + afterEach(() => { + process.env = { ...originalEnv }; + }); + + it('strictly ignores forged X-Forwarded-For when TRUST_PROXY is not enabled', () => { + const req = new NextRequest('http://localhost/api/test', { + headers: { + 'x-forwarded-for': '198.51.100.25', + 'x-real-ip': '203.0.113.195', + }, + }); + + const ip = resolveClientIp(req); + expect(ip).not.toBe('198.51.100.25'); + expect(ip).not.toBe('203.0.113.195'); + expect(ip).toBe('direct-client'); + }); + + it('extracts and normalizes client IP when TRUST_PROXY=true is set', () => { + process.env.TRUST_PROXY = 'true'; + + const req = new NextRequest('http://localhost/api/test', { + headers: { + 'x-forwarded-for': '203.0.113.50, 198.51.100.1, 192.0.2.1', + }, + }); + + const ip = resolveClientIp(req); + expect(ip).toBe('203.0.113.50'); + }); + + it('normalizes IPv6 and IPv4-mapped IPv6 correctly', () => { + expect(normalizeIp('::1')).toBe('127.0.0.1'); + expect(normalizeIp('[::1]:8080')).toBe('127.0.0.1'); + expect(normalizeIp('::ffff:192.168.1.10')).toBe('192.168.1.10'); + expect(normalizeIp('192.168.1.1:3000')).toBe('192.168.1.1'); + }); + + it('rejects oversized headers when TRUST_PROXY=true', () => { + process.env.TRUST_PROXY = 'true'; + + const oversizedHeader = '1.1.1.1, ' + 'a'.repeat(1100); + const req = new NextRequest('http://localhost/api/test', { + headers: { + 'x-forwarded-for': oversizedHeader, + }, + }); + + const ip = resolveClientIp(req); + expect(ip).toBe('malformed-oversized-ip'); + }); + + it('rate limiter proactively cleans up expired buckets without unbounded growth', async () => { + const limiter = new SlidingWindowRateLimiter({ + windowMs: 30, + maxRequests: 5, + maxBuckets: 50, + }); + + for (let i = 0; i < 40; i++) { + limiter.check(`ip-${i}`); + } + + expect(limiter.size()).toBe(40); + + // Wait for window to expire + await new Promise(r => setTimeout(r, 45)); + + const cleaned = limiter.cleanupExpired(); + expect(cleaned).toBeGreaterThanOrEqual(40); + expect(limiter.size()).toBe(0); + }); +}); diff --git a/tests/concurrency-draw-100.test.ts b/tests/concurrency-draw-100.test.ts new file mode 100644 index 0000000..c73df5a --- /dev/null +++ b/tests/concurrency-draw-100.test.ts @@ -0,0 +1,136 @@ +import { describe, it, expect } from 'vitest'; +import { MemoryGiveawayRepository } from '../src/lib/repository/memory-repository'; +import { DEFAULT_FILTER_RULES } from '../src/core/types/giveaway'; +import { FilteredParticipant } from '../src/core/types/participant'; +import { executeDeterministicDrawV1 } from '../src/core/randomizer/deterministic'; +import { ConflictError } from '../src/core/errors/http-errors'; + +describe('100-Draw Concurrency Regression & Mixed Race', () => { + const participants: FilteredParticipant[] = Array.from({ length: 50 }, (_, i) => ({ + platformUserId: `user_${1000 + i}`, + firstName: 'User', + lastName: `${i + 1}`, + source: 'LIKES', + liked: true, + commented: false, + commentsCount: 0, + reposted: false, + subscribed: true, + eligible: true, + exclusionReason: null, + })); + + it('100 concurrent draw attempts result in exactly 1 success and 99 conflicts with 1 DrawResult', async () => { + const repo = new MemoryGiveawayRepository(); + + const gw = await repo.createGiveaway({ + sourceUrl: 'https://vk.com/wall-100_100', + post: { + platform: 'VK', + ownerId: '-100', + postId: '100', + sourceUrl: 'https://vk.com/wall-100_100', + title: '100 Concurrency Test', + likesCount: 50, + commentsCount: 0, + repostsCount: 0, + }, + filterRules: DEFAULT_FILTER_RULES, + winnersCount: 3, + reserveWinnersCount: 1, + }); + + const snapshot = await repo.createAndLockSnapshot(gw.id, participants, DEFAULT_FILTER_RULES); + + // Launch 100 concurrent draw requests + const drawPromises = Array.from({ length: 100 }, async (_, index) => { + try { + const seed = `seed-concurrent-100-${index}`; + const drawResult = executeDeterministicDrawV1({ + giveawayId: gw.id, + snapshot, + totalLoadedCount: 50, + winnersCount: 3, + reserveWinnersCount: 1, + seed, + }); + + const saved = await repo.saveDrawResultAndAudit(gw.id, snapshot.id, drawResult); + return { status: 200, result: saved }; + } catch (err: any) { + if ( + err instanceof ConflictError || + err?.message?.includes('already been drawn') || + err?.message?.includes('SNAPSHOT_LOCKED') + ) { + return { status: 409, error: err.message }; + } + return { status: 500, error: err.message }; + } + }); + + const results = await Promise.all(drawPromises); + + const successCount = results.filter(r => r.status === 200).length; + const conflictCount = results.filter(r => r.status === 409).length; + const errorCount = results.filter(r => r.status === 500).length; + + expect(successCount).toBe(1); + expect(conflictCount).toBe(99); + expect(errorCount).toBe(0); + + const finalized = await repo.getGiveawayById(gw.id); + expect(finalized?.status).toBe('DRAWN'); + expect(finalized?.drawResult).toBeDefined(); + expect(finalized?.drawResult?.winners.length).toBe(3); + expect(finalized?.drawResult?.reserveWinners.length).toBe(1); + }); + + it('mixed race between participant mutation, snapshot locking, and draw preserves terminal integrity', async () => { + const repo = new MemoryGiveawayRepository(); + + const gw = await repo.createGiveaway({ + sourceUrl: 'https://vk.com/wall-100_200', + post: { + platform: 'VK', + ownerId: '-100', + postId: '200', + sourceUrl: 'https://vk.com/wall-100_200', + title: 'Mixed Race Test', + likesCount: 50, + commentsCount: 0, + repostsCount: 0, + }, + filterRules: DEFAULT_FILTER_RULES, + winnersCount: 1, + reserveWinnersCount: 0, + }); + + await repo.saveParticipants(gw.id, participants); + + // Launch mixed simultaneous actions + const actions = [ + repo.createAndLockSnapshot(gw.id, participants, DEFAULT_FILTER_RULES).catch(e => ({ error: e.message })), + repo.saveParticipants(gw.id, participants.slice(0, 10)).catch(e => ({ error: e.message })), + (async () => { + const snap = await repo.getLatestSnapshot(gw.id); + if (!snap) return { skipped: true }; + const drawResult = executeDeterministicDrawV1({ + giveawayId: gw.id, + snapshot: snap, + totalLoadedCount: 50, + winnersCount: 1, + reserveWinnersCount: 0, + seed: 'seed-mixed', + }); + return repo.saveDrawResultAndAudit(gw.id, snap.id, drawResult).catch(e => ({ error: e.message })); + })(), + ]; + + await Promise.allSettled(actions); + + const finalState = await repo.getGiveawayById(gw.id); + expect(finalState).not.toBeNull(); + expect(['READY', 'SNAPSHOT_LOCKED', 'DRAWN']).toContain(finalState?.status); + }); +}); diff --git a/tests/idempotency-hardening.test.ts b/tests/idempotency-hardening.test.ts new file mode 100644 index 0000000..d0f9eff --- /dev/null +++ b/tests/idempotency-hardening.test.ts @@ -0,0 +1,144 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { MemoryIdempotencyStore } from '../src/lib/idempotency'; +import { IdempotencyKeyReusedError, ValidationError } from '../src/core/errors/http-errors'; + +describe('Idempotency Hardening & Request Fingerprinting', () => { + let store: MemoryIdempotencyStore; + + beforeEach(() => { + store = new MemoryIdempotencyStore({ defaultTtlMs: 1000, maxKeyLength: 128, maxEntries: 100 }); + }); + + it('returns cached response for same key and identical request payload', () => { + const key = 'test-key-1'; + const payload = { sourceUrl: 'https://vk.com/wall-1_1', winnersCount: 1 }; + const responseBody = { success: true, giveawayId: 'gw-1' }; + + store.set({ + key, + operation: 'create-giveaway', + giveawayId: 'gw-1', + requestPayload: payload, + statusCode: 201, + body: responseBody, + }); + + const cached = store.get({ + key, + operation: 'create-giveaway', + giveawayId: 'gw-1', + requestPayload: payload, + }); + + expect(cached).not.toBeNull(); + expect(cached?.statusCode).toBe(201); + expect(cached?.body).toEqual(responseBody); + }); + + it('throws IdempotencyKeyReusedError (409) when same key is used with different payload', () => { + const key = 'test-key-reused'; + const originalPayload = { sourceUrl: 'https://vk.com/wall-1_1', winnersCount: 1 }; + const modifiedPayload = { sourceUrl: 'https://vk.com/wall-1_1', winnersCount: 5 }; // Changed! + + store.set({ + key, + operation: 'create-giveaway', + giveawayId: 'gw-1', + requestPayload: originalPayload, + statusCode: 201, + body: { success: true }, + }); + + expect(() => + store.get({ + key, + operation: 'create-giveaway', + giveawayId: 'gw-1', + requestPayload: modifiedPayload, + }) + ).toThrow(IdempotencyKeyReusedError); + }); + + it('prevents collision across different operations or giveaways with identical key', () => { + const key = 'shared-key-id'; + const payload = { test: 123 }; + + store.set({ + key, + operation: 'operation-A', + giveawayId: 'gw-1', + requestPayload: payload, + statusCode: 200, + body: { op: 'A' }, + }); + + // Lookup under operation-B with same key must return null + const resB = store.get({ + key, + operation: 'operation-B', + giveawayId: 'gw-1', + requestPayload: payload, + }); + expect(resB).toBeNull(); + + // Lookup under different giveaway must return null + const resGw2 = store.get({ + key, + operation: 'operation-A', + giveawayId: 'gw-2', + requestPayload: payload, + }); + expect(resGw2).toBeNull(); + }); + + it('rejects keys exceeding maximum allowed length', () => { + const oversizedKey = 'a'.repeat(129); + expect(() => + store.get({ + key: oversizedKey, + operation: 'op', + requestPayload: {}, + }) + ).toThrow(ValidationError); + }); + + it('cleans up expired entries proactively', async () => { + const shortTtlStore = new MemoryIdempotencyStore({ defaultTtlMs: 20 }); + shortTtlStore.set({ + key: 'expiring-key', + operation: 'op', + requestPayload: { a: 1 }, + statusCode: 200, + body: { ok: true }, + }); + + expect(shortTtlStore.size()).toBe(1); + + await new Promise(r => setTimeout(r, 40)); + + const res = shortTtlStore.get({ + key: 'expiring-key', + operation: 'op', + requestPayload: { a: 1 }, + }); + + expect(res).toBeNull(); + }); + + it('handles bounded capacity with synthetic keys without memory leak', () => { + const boundedStore = new MemoryIdempotencyStore({ maxEntries: 50, defaultTtlMs: 10000 }); + + for (let i = 0; i < 200; i++) { + boundedStore.set({ + key: `synth-key-${i}`, + operation: 'op', + requestPayload: { index: i }, + statusCode: 200, + body: { i }, + }); + } + + // Capacity must not exceed maxEntries + expect(boundedStore.size()).toBeLessThanOrEqual(50); + }); +}); diff --git a/tests/idempotency.test.ts b/tests/idempotency.test.ts index 1e3834f..55064ab 100644 --- a/tests/idempotency.test.ts +++ b/tests/idempotency.test.ts @@ -4,15 +4,32 @@ import { IdempotencyStore } from '../src/lib/idempotency'; describe('Idempotency Key Store', () => { it('should store and return cached idempotent response', () => { const key = 'test-idemp-key-1'; - const payload = { result: 'ok', id: '123' }; + const requestPayload = { result: 'ok', id: '123' }; + const responsePayload = { success: true, createdId: '123' }; - expect(IdempotencyStore.get(key)).toBeNull(); + expect( + IdempotencyStore.get({ + key, + operation: 'test-op', + requestPayload, + }) + ).toBeNull(); - IdempotencyStore.set(key, 201, payload); + IdempotencyStore.set({ + key, + operation: 'test-op', + requestPayload, + statusCode: 201, + body: responsePayload, + }); - const cached = IdempotencyStore.get(key); + const cached = IdempotencyStore.get({ + key, + operation: 'test-op', + requestPayload, + }); expect(cached).not.toBeNull(); expect(cached?.statusCode).toBe(201); - expect(cached?.body).toEqual(payload); + expect(cached?.body).toEqual(responsePayload); }); }); diff --git a/tests/randomizer.test.ts b/tests/randomizer.test.ts index f480eb0..2afde63 100644 --- a/tests/randomizer.test.ts +++ b/tests/randomizer.test.ts @@ -131,22 +131,35 @@ describe('Deterministic Randomizer V1 (HMAC_SHA256_FY_V1)', () => { expect(uniqueIds.size).toBe(10); }); - it('should handle small pool sizes gracefully', () => { + it('should strictly enforce winner count contract (never silently reduce winners count)', () => { const snapshot = createMockSnapshot(2); const seed = 'small-pool-seed'; - const draw = executeDeterministicDrawV1({ - giveawayId: 'gw-small', + // Requesting 5 winners on 2 eligible participants must throw + expect(() => + executeDeterministicDrawV1({ + giveawayId: 'gw-small', + snapshot, + totalLoadedCount: 2, + winnersCount: 5, + reserveWinnersCount: 3, + seed, + filterRules: DEFAULT_FILTER_RULES, + }) + ).toThrow(/exceeds eligible participants count/i); + + // Requesting exactly 2 winners on 2 eligible participants succeeds + const validDraw = executeDeterministicDrawV1({ + giveawayId: 'gw-small-valid', snapshot, totalLoadedCount: 2, - winnersCount: 5, - reserveWinnersCount: 3, + winnersCount: 2, + reserveWinnersCount: 0, seed, filterRules: DEFAULT_FILTER_RULES, }); - - expect(draw.winners.length).toBe(2); - expect(draw.reserveWinners.length).toBe(0); + expect(validDraw.winners.length).toBe(2); + expect(validDraw.reserveWinners.length).toBe(0); }); it('should allow third-party audit replay verification via verifyDrawResult', () => { diff --git a/tests/winner-count-contract.test.ts b/tests/winner-count-contract.test.ts new file mode 100644 index 0000000..5b86653 --- /dev/null +++ b/tests/winner-count-contract.test.ts @@ -0,0 +1,169 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { NextRequest } from 'next/server'; +import { GiveawayStore } from '../src/lib/giveaway-store'; +import { MemoryGiveawayRepository } from '../src/lib/repository/memory-repository'; +import { ProviderRegistry } from '../src/providers/registry'; +import { POST as drawPost } from '../src/app/api/giveaways/[id]/draw/route'; +import { DEFAULT_FILTER_RULES } from '../src/core/types/giveaway'; +import { FilteredParticipant } from '../src/core/types/participant'; +import { executeDeterministicDrawV1 } from '../src/core/randomizer/deterministic'; + +describe('Winner Count Contract & Draw Retry Invariants', () => { + beforeEach(() => { + GiveawayStore.setRepository(new MemoryGiveawayRepository()); + ProviderRegistry.useMockVk(); + }); + + const threeParticipants: FilteredParticipant[] = Array.from({ length: 3 }, (_, i) => ({ + platformUserId: `user_${i + 1}`, + firstName: 'User', + lastName: `${i + 1}`, + source: 'LIKES', + liked: true, + commented: false, + commentsCount: 0, + reposted: false, + subscribed: true, + eligible: true, + exclusionReason: null, + })); + + it('eligible=3, winners=3, reserve=0 -> success', async () => { + const gw = await GiveawayStore.create({ + sourceUrl: 'https://vk.com/wall-100_1', + post: { + platform: 'VK', + ownerId: '-100', + postId: '1', + sourceUrl: 'https://vk.com/wall-100_1', + title: 'Title', + likesCount: 3, + commentsCount: 0, + repostsCount: 0, + }, + filterRules: DEFAULT_FILTER_RULES, + winnersCount: 3, + reserveWinnersCount: 0, + }); + + await GiveawayStore.updateParticipants(gw.id, threeParticipants); + const snapshot = await GiveawayStore.createAndLockSnapshot(gw.id, threeParticipants, DEFAULT_FILTER_RULES); + + const result = executeDeterministicDrawV1({ + giveawayId: gw.id, + snapshot, + totalLoadedCount: 3, + winnersCount: 3, + reserveWinnersCount: 0, + seed: 'test-seed-3-3-0', + }); + + expect(result.winners.length).toBe(3); + expect(result.reserveWinners.length).toBe(0); + }); + + it('eligible=3, winners=4, reserve=0 -> error (never silently reduces winners count)', async () => { + const gw = await GiveawayStore.create({ + sourceUrl: 'https://vk.com/wall-100_2', + post: { + platform: 'VK', + ownerId: '-100', + postId: '2', + sourceUrl: 'https://vk.com/wall-100_2', + title: 'Title', + likesCount: 3, + commentsCount: 0, + repostsCount: 0, + }, + filterRules: DEFAULT_FILTER_RULES, + winnersCount: 4, + reserveWinnersCount: 0, + }); + + await GiveawayStore.updateParticipants(gw.id, threeParticipants); + const snapshot = await GiveawayStore.createAndLockSnapshot(gw.id, threeParticipants, DEFAULT_FILTER_RULES); + + expect(() => + executeDeterministicDrawV1({ + giveawayId: gw.id, + snapshot, + totalLoadedCount: 3, + winnersCount: 4, + reserveWinnersCount: 0, + seed: 'test-seed-3-4-0', + }) + ).toThrow(/exceeds eligible participants count/i); + }); + + it('eligible=3, winners=3, reserve=3 -> error (total 6 exceeds pool of 3)', async () => { + const gw = await GiveawayStore.create({ + sourceUrl: 'https://vk.com/wall-100_3', + post: { + platform: 'VK', + ownerId: '-100', + postId: '3', + sourceUrl: 'https://vk.com/wall-100_3', + title: 'Title', + likesCount: 3, + commentsCount: 0, + repostsCount: 0, + }, + filterRules: DEFAULT_FILTER_RULES, + winnersCount: 3, + reserveWinnersCount: 3, + }); + + await GiveawayStore.updateParticipants(gw.id, threeParticipants); + const snapshot = await GiveawayStore.createAndLockSnapshot(gw.id, threeParticipants, DEFAULT_FILTER_RULES); + + expect(() => + executeDeterministicDrawV1({ + giveawayId: gw.id, + snapshot, + totalLoadedCount: 3, + winnersCount: 3, + reserveWinnersCount: 3, + seed: 'test-seed-3-3-3', + }) + ).toThrow(/exceeds eligible participants count/i); + }); + + it('repeat draw on already DRAWN giveaway returns 409 DRAW_ALREADY_COMPLETED', async () => { + const gw = await GiveawayStore.create({ + sourceUrl: 'https://vk.com/wall-100_4', + post: { + platform: 'VK', + ownerId: '-100', + postId: '4', + sourceUrl: 'https://vk.com/wall-100_4', + title: 'Title', + likesCount: 3, + commentsCount: 0, + repostsCount: 0, + }, + filterRules: DEFAULT_FILTER_RULES, + }); + + await GiveawayStore.updateParticipants(gw.id, threeParticipants); + await GiveawayStore.createAndLockSnapshot(gw.id, threeParticipants, DEFAULT_FILTER_RULES); + + // First draw + const req1 = new NextRequest(`http://localhost/api/giveaways/${gw.id}/draw`, { + method: 'POST', + body: JSON.stringify({ winnersCount: 1, reserveWinnersCount: 0 }), + }); + const res1 = await drawPost(req1, { params: { id: gw.id } }); + expect(res1.status).toBe(200); + + // Second draw -> MUST return 409 DRAW_ALREADY_COMPLETED + const req2 = new NextRequest(`http://localhost/api/giveaways/${gw.id}/draw`, { + method: 'POST', + body: JSON.stringify({ winnersCount: 1, reserveWinnersCount: 0 }), + }); + const res2 = await drawPost(req2, { params: { id: gw.id } }); + expect(res2.status).toBe(409); + + const body = await res2.json(); + expect(body.error?.code).toBe('DRAW_ALREADY_COMPLETED'); + }); +});