[1430] GET /vaults query does not enforce max limit and allows limit=100000 to OOM the API

[ToryMic]

📋 Description

Goal

GET /api/v1/vaults forwarded req.query.limit straight into Prisma's take after z.coerce.number(), with no upper bound. An unauthenticated caller could ask for ?limit=100000 and force the API to materialise 100k vault rows plus their relations — a memory spike large enough to OOM-kill the 512 MB container.

Closes #1430

Changes

Pagination ceilings (the fix)

• New backend/src/middleware/paginationGuard.ts exporting DEFAULT_PAGE_SIZE (20), MAX_PAGE_SIZE (50), MAX_PAGE (1000), resolvePagination() and the enforcePaginationLimits() middleware.
• New backend/src/routes/vaults.ts — GET /api/v1/vaults, rate limited with the existing reads tier, reading limit + 1 rows for the hasNextPage lookahead and never exposing tenantId.
• parsePaginationQuery gains a maxPage config key and clamps out-of-range pages; PaginationQuerySchema.page now accepts a signed integer so page=-1 / page=1000000 reach the clamp instead of being rejected with a 400.
• enforcePaginationLimits() pins the sanitised limit/page on req.resolvedPagination for the handler (declared in src/types/express.d.ts, alongside the existing request augmentations).

Chosen behaviour — documented, not incidental

limit is rejected, not clamped. A caller asking for limit > 50 has a bug, or is probing. Quietly returning 50 rows hides that behind a paginated response that looks complete, so the request fails fast with 400 and code: 'LIMIT_EXCEEDED'; the response body repeats maxLimit so clients can self-correct. page is the opposite case — an out-of-range page number is a benign mistake — so it is clamped into 1..1000 and the effective value is echoed back in pagination.currentPage.

This is documented in three places that are kept in sync by a test: the route's module docblock, components.parameters.pageSize in the OpenAPI definition, and the test suite.

OpenAPI + contract snapshots

• components.parameters.pageSize / pageNumber carry the ceilings (maximum: 50 and maximum: 1000) and are $ref-ed from the new GET /api/v1/vaults path, which also documents the 400 / LIMIT_EXCEEDED response with a full example.
• PaginationMeta now publishes limit and currentPage bounds.
• GET /api/v1/vaults joins CRITICAL_ENDPOINTS in apiContractSnapshots.ts with a committed schema-snapshots/get-_api_v1_vaults.json, so the response shape is covered by the existing backward-compatibility check.
• openapi.json regenerated (npm run generate:openapi), which also restores /api/v1/vaults/{id}/apy — that path was documented in the committed spec but had been dropped from swagger.ts by a bad merge, so the "Verify OpenAPI documentation" job was failing on main.

Prerequisite: main does not build or test

The first commit repairs pre-existing breakage so this branch can be validated at all. It is kept separate from the fix itself and is not part of the issue's scope:

• prisma/schema.prisma: WalletTenantAssociation was missing its closing } and ran into model IdempotencyKey, so prisma generate failed and @prisma/client stayed an uninitialised stub — every test suite crashed on import. Also adds SessionAuditLog, Transaction.deletedAt and WebhookEndpoint.tenantId (all already queried by sessionAudit.ts / the tenant guard), plus the matching migration and a refreshed prisma/dev.db.
• readsLimiter was used by GET /receipts in vaultEndpoints.ts without being imported.
• The idempotency replay cache that transferOrchestrator.ts / vaultEndpoints.ts / index.ts / idempotencyRetention.ts import is restored as idempotencyStore.ts and re-exported from idempotency.ts.
• An unterminated if in diffSchemaShapes and a possibly-undefined baseline.properties read in apiContractSnapshots.ts.
• OTel resource construction now resolves whichever factory the installed @opentelemetry/resources major exposes; ZodObject._shape → .shape for Zod 4; strategy-switch 200 response returned; receipts ordered by timestamp; session metadata serialised.
• Restores summary / errors / 404 path on the error envelope, which a bad merge had dropped.
• issues711.test.ts "newly added required fields" mutated the live schema instead of the baseline snapshot, so it asserted a scenario that can never produce the expected diff.

---

🔗 Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• 🔒 Security improvement

---

🛡️ Risk Assessment

Risk Level

• 🟡 Medium: API enhancement, frontend workflow update, non-critical dependency upgrade

Blast Radius & Impact Analysis

• Contract storage layout / data key migration involved
• Value transfer, deposit/withdraw flow, or vault share calculation affected
• External integration (Oracle, Soroban RPC, Bridge, Token contract) affected
• Database schema migration or data backfill required
• Breaking API or interface change affecting downstream clients
• Zero blast radius (isolated tooling / documentation only)

Detailed Risk & Blast Radius Notes:

GET /api/v1/vaults is a new, read-only, unauthenticated route. Nothing
previously depended on it, so the only behavioural change to an existing
contract is the shared pagination parser.

1. limit > 50 now 400s on routes that use enforcePaginationLimits(). Today
   that is only /api/v1/vaults. Existing list routes keep their documented
   ceilings (transactions/portfolio maxLimit 100, vault history 365) and are
   deliberately NOT changed here — that would be a breaking change for
   documented API and frontend callers, and belongs in its own PR.

2. page is now clamped to 1..1000 instead of being passed through, and
   page=-1 / page=1000000 are clamped rather than rejected. Both were
   already unreachable in a meaningful way (a page of -1 resolved to page 1
   in parsePaginationQuery; a page of 1000000 skipped past the end of the
   table), so no client can observe a behaviour change other than
   pagination.currentPage reporting the clamped value.

3. The new route runs two indexed queries (count + findMany) bounded at
   51 rows. It is behind the existing `reads` rate-limit tier, so it
   inherits the same per-IP budget as the other public read endpoints.

4. The prerequisite commit adds nullable columns (Transaction.deletedAt,
   WebhookEndpoint.tenantId) and one new table (SessionAuditLog). Nullable
   columns with no DEFAULT are canary-safe: old code keeps inserting rows
   and reads NULL. Existing data is untouched.

---

🔄 Rollback Plan

Rollback Strategy & Feasibility

• Clean Git Revert: Revertable with zero persistent state drift
• Database Migration Revert: Reversible migration down-script tested and verified
• Contract Upgrade Rollback: Tested rollback to previous contract WASM hash / implementation
• Feature Flag / Circuit Breaker: Feature can be toggled off instantly without redeployment
• Emergency Pause: Contract pause / freeze mechanism available to halt affected functions
• Forward-Only / Irreversible: State migration cannot be cleanly reversed; emergency recovery runbook linked below

Rollback Trigger Criteria

- GET /api/v1/vaults p95 latency above 200ms
- Any 500 from VAULTS_LIST_FAILED
- A legitimate client that needs limit > 50 for a single request (the
  response body names maxLimit, so this is easy to detect from 4xx logs)

Step-by-Step Rollback Procedure

1. git revert the two commits (or revert the tip; the prerequisite repair is safe to keep).
2. If the new columns/table need removing:
   DROP TABLE "SessionAuditLog"; ALTER TABLE "Transaction" DROP COLUMN "deletedAt"; ALTER TABLE "WebhookEndpoint" DROP COLUMN "tenantId";
3. cd backend && npx prisma migrate deploy && npm run prisma:generate to resync the client.

---

⚡ Performance Impact

Performance & Resource Assessment

• Smart contract gas / compute units benchmarked (no regression > 5%, or justified below)
• Backend API latency (p95/p99) and database query execution plans verified
• Database indexing verified for newly queried columns (no table scans)
• Frontend bundle size and Time to Interactive (TTI) verified
• Memory allocation and leak checks verified (no memory leaks in long-running services)
• No measurable performance impact (documentation, tests, or trivial changes)

Performance & Gas Profiling Summary:

The fix is a hard upper bound on work per request, so it strictly reduces
the worst case:

  before: take = 100000 (caller-controlled)  -> ~100k rows materialised
  after:  take = min(limit, 50) + 1          -> <= 51 rows

  skip  = (min(page, 1000) - 1) * limit      -> <= 49_950

Other list routes: unchanged ceilings, plus a page clamp that only ever
shrinks the offset. Every other read in the API is untouched.

---

🔒 SECURITY REVIEW

Smart-contract sections (Reentrancy, CEI, Slither, gas limits) do not apply — this PR touches no Solidity.

• Input validation: limit/page are parsed as exact base-10 integers; anything else (floats, 1e5, arrays, negatives) falls back to the default rather than being coerced. Repeated query parameters are ignored rather than guessed at.
• Access control: the new route is read-only and public, matching the existing public read tier. tenantId is not part of the response projection.
• Boundary / edge-case tests: limit=50 (accepted), limit=51 and limit=100000 (400), limit=abc / 0 / -5 / 1.5 / '' (default), page=-1 → 1, page=100000 → 1000.
• Unbounded loop / DoS vector: eliminated. A test asserts via a Prisma spy that no read is ever issued with take > 51, and that an oversized limit is rejected before any query is issued.

Option A: Fixed in This PR ✅

• Vulnerability identified and resolved
• Test case added to verify fix
• Explain fix below:

`GET /vaults` did `findMany({ take: z.coerce.number() })` with no upper
bound. `z.coerce.number()` also turns "" into 0 and "1e5" into 100000, so
the parameter was never even restricted to integers.

enforcePaginationLimits() now runs before the handler: it parses `limit`
with an exact /^-?\d+$/ test, rejects anything above MAX_PAGE_SIZE with
400 LIMIT_EXCEEDED, clamps `page` into 1..MAX_PAGE, and hands the result
to the route. The route additionally caps `take` at MAX_PAGE_SIZE + 1 so
the guard's ceiling holds even if a future caller forgets to apply it.

---

📝 Testing

Functional Testing

• Unit tests added/updated for changes
• Integration tests passing
• End-to-end (E2E) tests passing — no frontend surface changed
• Manual testing completed and documented below

cd backend
npx prisma generate
npx jest --runInBand
# Test Suites: 92 passed, 92 total
# Tests:       1342 passed, 1342 total

npm run lint          # 0 errors (pre-existing warnings only)
npm run build         # tsc, 0 errors
npm run snapshots:check
npm run generate:openapi && git diff --exit-code openapi.json

Security Testing

• Access control test: unauthorized request rejected — the 400 path is asserted, and the Prisma spy proves no data access occurs
• Boundary / edge-case test: limits, zero-amounts, and rounding behavior — MAX_PAGE_SIZE, MAX_PAGE_SIZE + 1, 100000, and non-integer input

Test Coverage

• All new code paths have test coverage — src/middleware/paginationGuard.ts 95% stmts / 94% branches, src/routes/vaults.ts 94% stmts
• Security-critical paths have comprehensive test cases — backend/src/__tests__/vaultsListLimits.test.ts, 23 tests

New tests in vaultsListLimits.test.ts:

• default page size is 20; limit=50 accepted; limit=51 and limit=100000 → 400 LIMIT_EXCEEDED
• Prisma spy: across limit=100000 / 999999 / 51 / 50 / 1 / unset, no findMany call ever has take > 51
• Prisma spy: limit=100000 issues zero findMany and zero count calls
• page clamped to 1..1000, and the derived skip is capped at (1000 - 1) * limit
• response matches the committed get-_api_v1_vaults.json contract snapshot; tenantId never leaks
• unit tests for resolvePagination and the middleware (including repeated query parameters)
• OpenAPI: pageSize/pageNumber carry the ceilings, /api/v1/vaults $refs them, the 400 documents LIMIT_EXCEEDED, and the committed openapi.json still matches specs

---

🚀 Deployment Notes

Mainnet Readiness

• This code is ready for production deployment
• All critical tests pass — see the known-issues note below
• Security review approved
• No temporary debug code
• No TODO comments

Known issues inherited from main (out of scope, flagged for maintainers)

These fail identically on main and are unrelated to this issue. Listed so review is not blocked by them:

1. npm test coverage gate. jest.config.js requires 80% global coverage; the repo currently sits at ~67% (7248/10828 statements) because large parts of src/ have no tests at all (operationalMetrics.ts, eventPollingService.ts, impersonationSessionService.ts, writeAheadAuditLog.ts, src/tests/*, …). Closing that gap is a repo-wide programme, not a pagination fix. All 92 suites / 1342 tests themselves pass.
2. npm run prisma:schema-check. ScopedAdminToken has both keyId @unique and @@index([keyId]); the committed migration created the uniqueness as an implicit sqlite_autoindex, so prisma migrate diff wants to redefine it. Making it zero-diff requires a DROP INDEX, which scripts/check-migrations.js classifies as an error — so the two gates cannot both be satisfied without a maintainer decision.
3. scripts/validate-*.ts at the repo root pass; dependency-security.yml's pnpm audit and CodeQL also fail on main and depend on repository settings/network, not on this diff.

---

✅ Reviewer Checklist

• PR author completed Risk Assessment and Rollback Plan ✓
• Performance and gas impact evaluated and verified ✓
• PR author completed security checklist ✓ (non-contract sections)
• All findings documented and categorized (fixed/false positive/excluded)
• Inline security comments are clear and justified
• Tests cover security-critical code paths
• No external calls bypass return value checks
• Access control is properly enforced
• State updates follow CEI pattern
• Input validation is comprehensive
• Follow-up actions (if any) tracked in issues

---

📞 Questions or Issues?

• 🤔 Confused about security checklist? → See docs/SECURITY_CHECKLIST.md
• 🔍 Marking finding as false positive? → Follow docs/FALSE_POSITIVE_HANDLING.md
• 🆘 Need security review help? → Tag @security-team in comments

[drips-wave]

@ToryMic Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[ToryMic]

---

🧭 CI triage — which red checks are mine and which are not

main is currently red across the board (verified against the main push run 36430911829 and the run list for main at 711c4332). Every check below fails identically on main; none is caused by this diff. Measured locally against a clean main worktree:

Check	main	This branch	Blocked by
Backend test suites	33 failed suites / 49 failed tests	0 failed / 93 suites, 1371 tests	— fixed here
Backend global coverage	37.11%	67.85%	jest.config.js requires 80%
Backend build (tsc)	fails (tsc error TS1005)	✅ pass	— fixed here
Backend lint + test	❌	❌	npm audit --audit-level=high (pre-existing: 5 high, needs breaking majors)
Backend Governance	❌	❌	prisma:schema-check (pre-existing ScopedAdminToken index)
Backend Test Coverage (>= 80%)	❌	❌	coverage threshold
Frontend Test Coverage (>= 70%)	❌	❌	frontend untouched by this PR
CodeQL (TypeScript)	❌	❌	frontend/package-lock.json out of sync with package.json
CodeQL (Rust) / Cargo Security Audit	❌	❌	cargo build failure in contracts/ (untouched)
Governance & PR Standards / Dependency Security Scan / Dependency Vulnerability Audit	❌	❌	repo has no pnpm-lock.yaml, so pnpm install --frozen-lockfile can never succeed
NPM Audit (Frontend & Backend)	❌	❌	same dependency advisories
Backend build, Shared API schemas typecheck + test, gitleaks, Slither, pnpm version check, Security Review Summary	✅	✅	—

Green on this branch and worth calling out

• Backend build — main does not type-check at all; a missing } in prisma/schema.prisma made prisma generate fail, leaving @prisma/client an uninitialised stub.
• The backend test suite goes from 49 failures to 0, and global coverage from 37% to 68%.
• Verify OpenAPI documentation (git status --porcelain openapi.json after regeneration) passes again — the committed spec and swagger.ts had drifted.
• ci:governance's check-migrations.js, check:migrations:canary, snapshots:check and check-adrs.js all pass.

If a maintainer wants a follow-up PR (deliberately kept out of this one, each is a separate concern):

1. Raise real test coverage for the untested half of src/ — the only way past the 80% gate.
2. Reconcile ScopedAdminToken's keyId @unique + @@index([keyId]) with its migration. prisma migrate diff wants a DROP INDEX, which scripts/check-migrations.js classifies as an error, so the two gates currently contradict each other.
3. Commit a pnpm-lock.yaml, or drop --frozen-lockfile from the four workflows that use it.
4. Sync frontend/package-lock.json and bump the advisories flagged by npm audit (both need breaking majors).

[ToryMic]

---

✅ Final local verification on this branch

cd backend
npx prisma generate
npm run build                                   # tsc → 0 errors
npm run lint                                    # 0 errors (422 pre-existing warnings)
npx jest --runInBand
#   Test Suites: 94 passed, 94 total
#   Tests:       1397 passed, 1397 total        (0 failures)

npm run generate:openapi && git diff --exit-code openapi.json   # in sync
npm run snapshots:check                                         # ✅
node scripts/check-migrations.js                                # ✅ (warnings only)
npm run check:migrations:canary                                 # ✅ (warnings only)
node scripts/check-adrs.js                                      # ✅
npm run prisma:generate                                         # ✅

Coverage moved with it: main 37.11% → this branch 68.21% global statements. The 80% gate is still red because a large part of src/ has no tests at all — see the triage table above. backend/src/idempotencyStore.ts went 70.9% → 94.5% statements once its Redis path was covered.

Branch status: upstream/main is an ancestor of both branches (git rev-list --count HEAD..upstream/main → 0), so both PRs are MERGEABLE with zero conflicts. #1516 is stacked on #1515 and carries its commits; after #1515 merges, #1516's diff collapses to its own three commits.